[{"id":11839,"web_url":"https://patchwork.libcamera.org/comment/11839/","msgid":"<20200804162601.GE6075@pendragon.ideasonboard.com>","date":"2020-08-04T16:26:01","subject":"Re: [libcamera-devel] [PATCH v7 1/9] libcamera: sysfs: Add new\n\tnamespace to interact with sysfs","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Tue, Aug 04, 2020 at 06:13:50PM +0200, Niklas Söderlund wrote:\n> To support stable and unique camera IDs access to information in sysfs\n> is needed. Add a new libcamera::sysfs namespace to contain such sysfs\n> helpers.\n> \n> As a first helper add a method to lookup a character device sysfs path.\n\nI would have turned this the other way around (\"Add a helper function to\nlookup the sysfs path of a character device. Store the function in a new\nlibcamera::sysfs namespace as there is not class to host it.\") but it\ndoesn't matter much.\n\n> Suggested-by: Laurent Pinchart \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/internal/sysfs.h | 22 ++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/sysfs.cpp | 48 ++++++++++++++++++++++++++\n> 4 files changed, 72 insertions(+)\n> create mode 100644 include/libcamera/internal/sysfs.h\n> create mode 100644 src/libcamera/sysfs.cpp\n> \n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index d868eff47f920da0..150103388fdb219d 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -25,6 +25,7 @@ libcamera_internal_headers = files([\n> 'process.h',\n> 'pub_key.h',\n> 'semaphore.h',\n> + 'sysfs.h',\n> 'thread.h',\n> 'utils.h',\n> 'v4l2_controls.h',\n> diff --git a/include/libcamera/internal/sysfs.h b/include/libcamera/internal/sysfs.h\n> new file mode 100644\n> index 0000000000000000..72f436205d8d30e8\n> --- /dev/null\n> +++ b/include/libcamera/internal/sysfs.h\n> @@ -0,0 +1,22 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * sysfs.h - Miscellaneous utility functions to access sysfs\n> + */\n> +#ifndef __LIBCAMERA_INTERNAL_SYSFS_H__\n> +#define __LIBCAMERA_INTERNAL_SYSFS_H__\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +namespace sysfs {\n> +\n> +std::string charDevPath(const std::string &devicePath);\n\ns/devicePath/deviceNode/\n\n> +\n> +} /* namespace sysfs */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_INTERNAL_SYSFS_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 3aad4386ffc296db..bada45bcb11e9e88 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -41,6 +41,7 @@ libcamera_sources = files([\n> 'semaphore.cpp',\n> 'signal.cpp',\n> 'stream.cpp',\n> + 'sysfs.cpp',\n> 'thread.cpp',\n> 'timer.cpp',\n> 'utils.cpp',\n> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> new file mode 100644\n> index 0000000000000000..3b2920663e9c3bcc\n> --- /dev/null\n> +++ b/src/libcamera/sysfs.cpp\n> @@ -0,0 +1,48 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * sysfs.cpp - Miscellaneous utility functions to access sysfs\n> + */\n> +\n> +#include \"libcamera/internal/sysfs.h\"\n> +\n\n#include \n\nfor std::ostringstream.\n\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +/**\n> + * \\file sysfs.h\n> + * \\brief Miscellaneous utility functions to access sysfs\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(SYSFS);\n\nSysFs ? Up to you.\n\n> +\n> +namespace sysfs {\n> +\n> +/**\n> + * \\brief Retrive the sysfs path for a characther device\n\ns/Retrive/Retrieve/\ns/characther/character/\n\n> + * \\param[in] devicePath Path to charachter device\n\ns/devicePath/deviceNode/\ns/charachter/character/\ns/device$/device node/\n\n> + * \\return The sysfs path on success or empty string on failure\n\ns/empty/an empty/\n\n> + */\n> +std::string charDevPath(const std::string &devicePath)\n\ns/devicePath/deviceNode/\n\nReviewed-by: Laurent Pinchart \n\n> +{\n> +\tstruct stat st;\n> +\tint ret = stat(devicePath.c_str(), &st);\n> +\tif (ret < 0) {\n> +\t\tLOG(SYSFS, Error)\n> +\t\t\t<< \"Unable to stat '\" << devicePath << \"'\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tstd::ostringstream dev(\"/sys/dev/char/\", std::ios_base::ate);\n> +\tdev << major(st.st_rdev) << \":\" << minor(st.st_rdev);\n> +\treturn dev.str();\n> +}\n> +\n> +} /* namespace sysfs */\n> +\n> +} /* namespace libcamera */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1F588BD86F\n\tfor ;\n\tTue, 4 Aug 2020 16:26:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AFE566054A;\n\tTue, 4 Aug 2020 18:26:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7274D60545\n\tfor ;\n\tTue, 4 Aug 2020 18:26:13 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D5A2127B;\n\tTue, 4 Aug 2020 18:26:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"aFyfMPVY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596558373;\n\tbh=7lQQmlzv7s4Ki7b2DA5l7sRD8LUSc8cM9wLdklP8FUM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aFyfMPVYIsVMKcLZjzGhufYkR1DSz+AI0lt3Rn0Jxv/Y0UNGWhywzLxjJVJt6Hdxu\n\tjtk4K8NoUYNUkb/rJTbhrKZWe44pmrXb202YtAVlDbBuAj+DoA0ymDOiwBRgUSKpcP\n\tYHtDmyKfI5Eys/QBBojocVASP7qptMu2fPm1RJ0Y=","Date":"Tue, 4 Aug 2020 19:26:01 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200804162601.GE6075@pendragon.ideasonboard.com>","References":"<20200804161358.1628962-1-niklas.soderlund@ragnatech.se>\n\t<20200804161358.1628962-2-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200804161358.1628962-2-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v7 1/9] libcamera: sysfs: Add new\n\tnamespace to interact with sysfs","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11863,"web_url":"https://patchwork.libcamera.org/comment/11863/","msgid":"<20200805072140.glmaqqnmbthwbz4f@uno.localdomain>","date":"2020-08-05T07:21:40","subject":"Re: [libcamera-devel] [PATCH v7 1/9] libcamera: sysfs: Add new\n\tnamespace to interact with sysfs","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Aug 04, 2020 at 07:26:01PM +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n>\n> Thank you for the patch.\n>\n> On Tue, Aug 04, 2020 at 06:13:50PM +0200, Niklas Söderlund wrote:\n> > To support stable and unique camera IDs access to information in sysfs\n> > is needed. Add a new libcamera::sysfs namespace to contain such sysfs\n> > helpers.\n> >\n> > As a first helper add a method to lookup a character device sysfs path.\n>\n> I would have turned this the other way around (\"Add a helper function to\n> lookup the sysfs path of a character device. Store the function in a new\n> libcamera::sysfs namespace as there is not class to host it.\") but it\n> doesn't matter much.\n>\n> > Suggested-by: Laurent Pinchart \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/internal/sysfs.h | 22 ++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/libcamera/sysfs.cpp | 48 ++++++++++++++++++++++++++\n> > 4 files changed, 72 insertions(+)\n> > create mode 100644 include/libcamera/internal/sysfs.h\n> > create mode 100644 src/libcamera/sysfs.cpp\n> >\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index d868eff47f920da0..150103388fdb219d 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -25,6 +25,7 @@ libcamera_internal_headers = files([\n> > 'process.h',\n> > 'pub_key.h',\n> > 'semaphore.h',\n> > + 'sysfs.h',\n> > 'thread.h',\n> > 'utils.h',\n> > 'v4l2_controls.h',\n> > diff --git a/include/libcamera/internal/sysfs.h b/include/libcamera/internal/sysfs.h\n> > new file mode 100644\n> > index 0000000000000000..72f436205d8d30e8\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/sysfs.h\n> > @@ -0,0 +1,22 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * sysfs.h - Miscellaneous utility functions to access sysfs\n> > + */\n> > +#ifndef __LIBCAMERA_INTERNAL_SYSFS_H__\n> > +#define __LIBCAMERA_INTERNAL_SYSFS_H__\n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace sysfs {\n> > +\n> > +std::string charDevPath(const std::string &devicePath);\n>\n> s/devicePath/deviceNode/\n>\n> > +\n> > +} /* namespace sysfs */\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_INTERNAL_SYSFS_H__ */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 3aad4386ffc296db..bada45bcb11e9e88 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -41,6 +41,7 @@ libcamera_sources = files([\n> > 'semaphore.cpp',\n> > 'signal.cpp',\n> > 'stream.cpp',\n> > + 'sysfs.cpp',\n> > 'thread.cpp',\n> > 'timer.cpp',\n> > 'utils.cpp',\n> > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp\n> > new file mode 100644\n> > index 0000000000000000..3b2920663e9c3bcc\n> > --- /dev/null\n> > +++ b/src/libcamera/sysfs.cpp\n> > @@ -0,0 +1,48 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * sysfs.cpp - Miscellaneous utility functions to access sysfs\n> > + */\n> > +\n> > +#include \"libcamera/internal/sysfs.h\"\n> > +\n>\n> #include \n>\n> for std::ostringstream.\n>\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/log.h\"\n> > +\n> > +/**\n> > + * \\file sysfs.h\n> > + * \\brief Miscellaneous utility functions to access sysfs\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(SYSFS);\n>\n> SysFs ? Up to you.\n>\n> > +\n> > +namespace sysfs {\n> > +\n> > +/**\n> > + * \\brief Retrive the sysfs path for a characther device\n>\n> s/Retrive/Retrieve/\n> s/characther/character/\n>\n> > + * \\param[in] devicePath Path to charachter device\n>\n> s/devicePath/deviceNode/\n> s/charachter/character/\n> s/device$/device node/\n>\n> > + * \\return The sysfs path on success or empty string on failure\n>\n> s/empty/an empty/\n>\n> > + */\n> > +std::string charDevPath(const std::string &devicePath)\n>\n> s/devicePath/deviceNode/\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > +{\n> > +\tstruct stat st;\n> > +\tint ret = stat(devicePath.c_str(), &st);\n> > +\tif (ret < 0) {\n> > +\t\tLOG(SYSFS, Error)\n> > +\t\t\t<< \"Unable to stat '\" << devicePath << \"'\";\n\nAs you have to rework it, I think the stat error is relevant to be\nprinted out\n\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\tstd::ostringstream dev(\"/sys/dev/char/\", std::ios_base::ate);\n> > +\tdev << major(st.st_rdev) << \":\" << minor(st.st_rdev);\n\nUsually we have an empty line before return.\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> > +\treturn dev.str();\n> > +}\n> > +\n> > +} /* namespace sysfs */\n> > +\n> > +} /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D5342BD87A\n\tfor ;\n\tWed, 5 Aug 2020 07:18:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F8886055E;\n\tWed, 5 Aug 2020 09:18:01 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F223160554\n\tfor ;\n\tWed, 5 Aug 2020 09:17:59 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 4DB78240019;\n\tWed, 5 Aug 2020 07:17:59 +0000 (UTC)"],"Date":"Wed, 5 Aug 2020 09:21:40 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200805072140.glmaqqnmbthwbz4f@uno.localdomain>","References":"<20200804161358.1628962-1-niklas.soderlund@ragnatech.se>\n\t<20200804161358.1628962-2-niklas.soderlund@ragnatech.se>\n\t<20200804162601.GE6075@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200804162601.GE6075@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v7 1/9] libcamera: sysfs: Add new\n\tnamespace to interact with sysfs","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]