[libcamera-devel,v7,1/9] libcamera: sysfs: Add new namespace to interact with sysfs
diff mbox series

Message ID 20200804161358.1628962-2-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Generate unique and stable camera IDs
Related show

Commit Message

Niklas Söderlund Aug. 4, 2020, 4:13 p.m. UTC 
To support stable and unique camera IDs access to information in sysfs
is needed. Add a new libcamera::sysfs namespace to contain such sysfs
helpers.

As a first helper add a method to lookup a character device sysfs path.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/internal/meson.build |  1 +
 include/libcamera/internal/sysfs.h     | 22 ++++++++++++
 src/libcamera/meson.build              |  1 +
 src/libcamera/sysfs.cpp                | 48 ++++++++++++++++++++++++++
 4 files changed, 72 insertions(+)
 create mode 100644 include/libcamera/internal/sysfs.h
 create mode 100644 src/libcamera/sysfs.cpp

Comments

Laurent Pinchart Aug. 4, 2020, 4:26 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Tue, Aug 04, 2020 at 06:13:50PM +0200, Niklas Söderlund wrote:
> To support stable and unique camera IDs access to information in sysfs
> is needed. Add a new libcamera::sysfs namespace to contain such sysfs
> helpers.
> 
> As a first helper add a method to lookup a character device sysfs path.

I would have turned this the other way around ("Add a helper function to
lookup the sysfs path of a character device. Store the function in a new
libcamera::sysfs namespace as there is not class to host it.") but it
doesn't matter much.

> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/libcamera/internal/meson.build |  1 +
>  include/libcamera/internal/sysfs.h     | 22 ++++++++++++
>  src/libcamera/meson.build              |  1 +
>  src/libcamera/sysfs.cpp                | 48 ++++++++++++++++++++++++++
>  4 files changed, 72 insertions(+)
>  create mode 100644 include/libcamera/internal/sysfs.h
>  create mode 100644 src/libcamera/sysfs.cpp
> 
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index d868eff47f920da0..150103388fdb219d 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -25,6 +25,7 @@ libcamera_internal_headers = files([
>      'process.h',
>      'pub_key.h',
>      'semaphore.h',
> +    'sysfs.h',
>      'thread.h',
>      'utils.h',
>      'v4l2_controls.h',
> diff --git a/include/libcamera/internal/sysfs.h b/include/libcamera/internal/sysfs.h
> new file mode 100644
> index 0000000000000000..72f436205d8d30e8
> --- /dev/null
> +++ b/include/libcamera/internal/sysfs.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * sysfs.h - Miscellaneous utility functions to access sysfs
> + */
> +#ifndef __LIBCAMERA_INTERNAL_SYSFS_H__
> +#define __LIBCAMERA_INTERNAL_SYSFS_H__
> +
> +#include <string>
> +
> +namespace libcamera {
> +
> +namespace sysfs {
> +
> +std::string charDevPath(const std::string &devicePath);

s/devicePath/deviceNode/

> +
> +} /* namespace sysfs */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_SYSFS_H__ */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 3aad4386ffc296db..bada45bcb11e9e88 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -41,6 +41,7 @@ libcamera_sources = files([
>      'semaphore.cpp',
>      'signal.cpp',
>      'stream.cpp',
> +    'sysfs.cpp',
>      'thread.cpp',
>      'timer.cpp',
>      'utils.cpp',
> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp
> new file mode 100644
> index 0000000000000000..3b2920663e9c3bcc
> --- /dev/null
> +++ b/src/libcamera/sysfs.cpp
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * sysfs.cpp - Miscellaneous utility functions to access sysfs
> + */
> +
> +#include "libcamera/internal/sysfs.h"
> +

#include <sstream>

for std::ostringstream.

> +#include <sys/stat.h>
> +#include <sys/sysmacros.h>
> +
> +#include "libcamera/internal/log.h"
> +
> +/**
> + * \file sysfs.h
> + * \brief Miscellaneous utility functions to access sysfs
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(SYSFS);

SysFs ? Up to you.

> +
> +namespace sysfs {
> +
> +/**
> + * \brief Retrive the sysfs path for a characther device

s/Retrive/Retrieve/
s/characther/character/

> + * \param[in] devicePath Path to charachter device

s/devicePath/deviceNode/
s/charachter/character/
s/device$/device node/

> + * \return The sysfs path on success or empty string on failure

s/empty/an empty/

> + */
> +std::string charDevPath(const std::string &devicePath)

s/devicePath/deviceNode/

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +{
> +	struct stat st;
> +	int ret = stat(devicePath.c_str(), &st);
> +	if (ret < 0) {
> +		LOG(SYSFS, Error)
> +			<< "Unable to stat '" << devicePath << "'";
> +		return {};
> +	}
> +
> +	std::ostringstream dev("/sys/dev/char/", std::ios_base::ate);
> +	dev << major(st.st_rdev) << ":" << minor(st.st_rdev);
> +	return dev.str();
> +}
> +
> +} /* namespace sysfs */
> +
> +} /* namespace libcamera */
Jacopo Mondi Aug. 5, 2020, 7:21 a.m. UTC | #2 
Hi Niklas,

On Tue, Aug 04, 2020 at 07:26:01PM +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Tue, Aug 04, 2020 at 06:13:50PM +0200, Niklas Söderlund wrote:
> > To support stable and unique camera IDs access to information in sysfs
> > is needed. Add a new libcamera::sysfs namespace to contain such sysfs
> > helpers.
> >
> > As a first helper add a method to lookup a character device sysfs path.
>
> I would have turned this the other way around ("Add a helper function to
> lookup the sysfs path of a character device. Store the function in a new
> libcamera::sysfs namespace as there is not class to host it.") but it
> doesn't matter much.
>
> > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  include/libcamera/internal/meson.build |  1 +
> >  include/libcamera/internal/sysfs.h     | 22 ++++++++++++
> >  src/libcamera/meson.build              |  1 +
> >  src/libcamera/sysfs.cpp                | 48 ++++++++++++++++++++++++++
> >  4 files changed, 72 insertions(+)
> >  create mode 100644 include/libcamera/internal/sysfs.h
> >  create mode 100644 src/libcamera/sysfs.cpp
> >
> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > index d868eff47f920da0..150103388fdb219d 100644
> > --- a/include/libcamera/internal/meson.build
> > +++ b/include/libcamera/internal/meson.build
> > @@ -25,6 +25,7 @@ libcamera_internal_headers = files([
> >      'process.h',
> >      'pub_key.h',
> >      'semaphore.h',
> > +    'sysfs.h',
> >      'thread.h',
> >      'utils.h',
> >      'v4l2_controls.h',
> > diff --git a/include/libcamera/internal/sysfs.h b/include/libcamera/internal/sysfs.h
> > new file mode 100644
> > index 0000000000000000..72f436205d8d30e8
> > --- /dev/null
> > +++ b/include/libcamera/internal/sysfs.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * sysfs.h - Miscellaneous utility functions to access sysfs
> > + */
> > +#ifndef __LIBCAMERA_INTERNAL_SYSFS_H__
> > +#define __LIBCAMERA_INTERNAL_SYSFS_H__
> > +
> > +#include <string>
> > +
> > +namespace libcamera {
> > +
> > +namespace sysfs {
> > +
> > +std::string charDevPath(const std::string &devicePath);
>
> s/devicePath/deviceNode/
>
> > +
> > +} /* namespace sysfs */
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_INTERNAL_SYSFS_H__ */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 3aad4386ffc296db..bada45bcb11e9e88 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -41,6 +41,7 @@ libcamera_sources = files([
> >      'semaphore.cpp',
> >      'signal.cpp',
> >      'stream.cpp',
> > +    'sysfs.cpp',
> >      'thread.cpp',
> >      'timer.cpp',
> >      'utils.cpp',
> > diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp
> > new file mode 100644
> > index 0000000000000000..3b2920663e9c3bcc
> > --- /dev/null
> > +++ b/src/libcamera/sysfs.cpp
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * sysfs.cpp - Miscellaneous utility functions to access sysfs
> > + */
> > +
> > +#include "libcamera/internal/sysfs.h"
> > +
>
> #include <sstream>
>
> for std::ostringstream.
>
> > +#include <sys/stat.h>
> > +#include <sys/sysmacros.h>
> > +
> > +#include "libcamera/internal/log.h"
> > +
> > +/**
> > + * \file sysfs.h
> > + * \brief Miscellaneous utility functions to access sysfs
> > + */
> > +
> > +namespace libcamera {
> > +
> > +LOG_DEFINE_CATEGORY(SYSFS);
>
> SysFs ? Up to you.
>
> > +
> > +namespace sysfs {
> > +
> > +/**
> > + * \brief Retrive the sysfs path for a characther device
>
> s/Retrive/Retrieve/
> s/characther/character/
>
> > + * \param[in] devicePath Path to charachter device
>
> s/devicePath/deviceNode/
> s/charachter/character/
> s/device$/device node/
>
> > + * \return The sysfs path on success or empty string on failure
>
> s/empty/an empty/
>
> > + */
> > +std::string charDevPath(const std::string &devicePath)
>
> s/devicePath/deviceNode/
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > +{
> > +	struct stat st;
> > +	int ret = stat(devicePath.c_str(), &st);
> > +	if (ret < 0) {
> > +		LOG(SYSFS, Error)
> > +			<< "Unable to stat '" << devicePath << "'";

As you have to rework it, I think the stat error is relevant to be
printed out

> > +		return {};
> > +	}
> > +
> > +	std::ostringstream dev("/sys/dev/char/", std::ios_base::ate);
> > +	dev << major(st.st_rdev) << ":" << minor(st.st_rdev);

Usually we have an empty line before return.

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> > +	return dev.str();
> > +}
> > +
> > +} /* namespace sysfs */
> > +
> > +} /* namespace libcamera */
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
index d868eff47f920da0..150103388fdb219d 100644
--- a/include/libcamera/internal/meson.build
+++ b/include/libcamera/internal/meson.build
@@ -25,6 +25,7 @@  libcamera_internal_headers = files([
     'process.h',
     'pub_key.h',
     'semaphore.h',
+    'sysfs.h',
     'thread.h',
     'utils.h',
     'v4l2_controls.h',
diff --git a/include/libcamera/internal/sysfs.h b/include/libcamera/internal/sysfs.h
new file mode 100644
index 0000000000000000..72f436205d8d30e8
--- /dev/null
+++ b/include/libcamera/internal/sysfs.h
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Google Inc.
+ *
+ * sysfs.h - Miscellaneous utility functions to access sysfs
+ */
+#ifndef __LIBCAMERA_INTERNAL_SYSFS_H__
+#define __LIBCAMERA_INTERNAL_SYSFS_H__
+
+#include <string>
+
+namespace libcamera {
+
+namespace sysfs {
+
+std::string charDevPath(const std::string &devicePath);
+
+} /* namespace sysfs */
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_INTERNAL_SYSFS_H__ */
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 3aad4386ffc296db..bada45bcb11e9e88 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -41,6 +41,7 @@  libcamera_sources = files([
     'semaphore.cpp',
     'signal.cpp',
     'stream.cpp',
+    'sysfs.cpp',
     'thread.cpp',
     'timer.cpp',
     'utils.cpp',
diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp
new file mode 100644
index 0000000000000000..3b2920663e9c3bcc
--- /dev/null
+++ b/src/libcamera/sysfs.cpp
@@ -0,0 +1,48 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Google Inc.
+ *
+ * sysfs.cpp - Miscellaneous utility functions to access sysfs
+ */
+
+#include "libcamera/internal/sysfs.h"
+
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+
+#include "libcamera/internal/log.h"
+
+/**
+ * \file sysfs.h
+ * \brief Miscellaneous utility functions to access sysfs
+ */
+
+namespace libcamera {
+
+LOG_DEFINE_CATEGORY(SYSFS);
+
+namespace sysfs {
+
+/**
+ * \brief Retrive the sysfs path for a characther device
+ * \param[in] devicePath Path to charachter device
+ * \return The sysfs path on success or empty string on failure
+ */
+std::string charDevPath(const std::string &devicePath)
+{
+	struct stat st;
+	int ret = stat(devicePath.c_str(), &st);
+	if (ret < 0) {
+		LOG(SYSFS, Error)
+			<< "Unable to stat '" << devicePath << "'";
+		return {};
+	}
+
+	std::ostringstream dev("/sys/dev/char/", std::ios_base::ate);
+	dev << major(st.st_rdev) << ":" << minor(st.st_rdev);
+	return dev.str();
+}
+
+} /* namespace sysfs */
+
+} /* namespace libcamera */