| Message ID | 20201224010907.263125-7-djrscally@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows | expand |
On Thu, Dec 24, 2020 at 3:12 AM Daniel Scally <djrscally@gmail.com> wrote: > > OF, ACPI and software_nodes all implement graphs including nodes for ports > and endpoints. These are all intended to be named with a common schema, > as "port@n" and "endpoint@n" where n is an unsigned int representing the > index of the node. To ensure commonality across the subsystems, provide a > set of macros to define the format. Nitpicks below, but in general that's what I meant, thanks! Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> (after addressing nitpicks) > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > Changes in v3 > - Patch introduced > > include/linux/fwnode.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > index 9506f8ec0974..52889efceb7d 100644 > --- a/include/linux/fwnode.h > +++ b/include/linux/fwnode.h > @@ -32,6 +32,19 @@ struct fwnode_endpoint { > const struct fwnode_handle *local_fwnode; > }; > > +/* > + * ports and endpoints defined in OF, ACPI and as software_nodes should all > + * follow a common naming scheme; use these macros to ensure commonality across > + * the subsystems. > + * > + * The *PREFIX_LEN macros refer to the length of the "port@" and "endpoint@" *PREFIX_LEN -> *_PREFIX_LEN > + * sections of the naming scheme. > + */ > +#define FWNODE_GRAPH_PORT_NAME_FORMAT "port@%u" > +#define FWNODE_GRAPH_PORT_NAME_PREFIX_LEN 5 > +#define FWNODE_GRAPH_ENDPOINT_NAME_FORMAT "endpoint@%u" > +#define FWNODE_GRAPH_ENDPOINT_PREFIX_LEN 9 _FORMAT -> _FMT (however, V4L2 guys may correct me, because IIRC _FMT suffix is also used for other things in v4l2. > #define NR_FWNODE_REFERENCE_ARGS 8 > > /**
Hi Daniel, Thank you for the patch. On Thu, Dec 24, 2020 at 02:17:07PM +0200, Andy Shevchenko wrote: > On Thu, Dec 24, 2020 at 3:12 AM Daniel Scally wrote: > > > > OF, ACPI and software_nodes all implement graphs including nodes for ports > > and endpoints. These are all intended to be named with a common schema, > > as "port@n" and "endpoint@n" where n is an unsigned int representing the > > index of the node. To ensure commonality across the subsystems, provide a > > set of macros to define the format. > > Nitpicks below, but in general that's what I meant, thanks! > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > (after addressing nitpicks) > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > > --- > > Changes in v3 > > - Patch introduced > > > > include/linux/fwnode.h | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > > index 9506f8ec0974..52889efceb7d 100644 > > --- a/include/linux/fwnode.h > > +++ b/include/linux/fwnode.h > > @@ -32,6 +32,19 @@ struct fwnode_endpoint { > > const struct fwnode_handle *local_fwnode; > > }; > > > > +/* > > + * ports and endpoints defined in OF, ACPI and as software_nodes should all > > + * follow a common naming scheme; use these macros to ensure commonality across > > + * the subsystems. > > + * > > + * The *PREFIX_LEN macros refer to the length of the "port@" and "endpoint@" > > *PREFIX_LEN -> *_PREFIX_LEN > > > + * sections of the naming scheme. > > + */ > > +#define FWNODE_GRAPH_PORT_NAME_FORMAT "port@%u" > > +#define FWNODE_GRAPH_PORT_NAME_PREFIX_LEN 5 > > +#define FWNODE_GRAPH_ENDPOINT_NAME_FORMAT "endpoint@%u" > > +#define FWNODE_GRAPH_ENDPOINT_PREFIX_LEN 9 > > _FORMAT -> _FMT (however, V4L2 guys may correct me, because IIRC _FMT > suffix is also used for other things in v4l2. This isn't related to V4L2, so it doesn't matter much :-) I personally prefer spelling names out in full when that wouldn't result in too long lines, but it's really a matter of personal preference, I don't mind either way. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > #define NR_FWNODE_REFERENCE_ARGS 8 > > > > /**
Hi Daniel, Andy, On Thu, Dec 24, 2020 at 01:08:59AM +0000, Daniel Scally wrote: > OF, ACPI and software_nodes all implement graphs including nodes for ports > and endpoints. These are all intended to be named with a common schema, > as "port@n" and "endpoint@n" where n is an unsigned int representing the > index of the node. To ensure commonality across the subsystems, provide a > set of macros to define the format. > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > Changes in v3 > - Patch introduced > > include/linux/fwnode.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > index 9506f8ec0974..52889efceb7d 100644 > --- a/include/linux/fwnode.h > +++ b/include/linux/fwnode.h > @@ -32,6 +32,19 @@ struct fwnode_endpoint { > const struct fwnode_handle *local_fwnode; > }; > > +/* > + * ports and endpoints defined in OF, ACPI and as software_nodes should all > + * follow a common naming scheme; use these macros to ensure commonality across > + * the subsystems. > + * > + * The *PREFIX_LEN macros refer to the length of the "port@" and "endpoint@" > + * sections of the naming scheme. > + */ > +#define FWNODE_GRAPH_PORT_NAME_FORMAT "port@%u" > +#define FWNODE_GRAPH_PORT_NAME_PREFIX_LEN 5 > +#define FWNODE_GRAPH_ENDPOINT_NAME_FORMAT "endpoint@%u" > +#define FWNODE_GRAPH_ENDPOINT_PREFIX_LEN 9 > + > #define NR_FWNODE_REFERENCE_ARGS 8 I'd keep such definitions local to the swnode implementation as neither ACPI nor DT have an apparent need for them. They do use the naming, but don't appear to format such strings.
On Mon, Dec 28, 2020 at 06:30:24PM +0200, Sakari Ailus wrote: > Hi Daniel, Andy, > > On Thu, Dec 24, 2020 at 01:08:59AM +0000, Daniel Scally wrote: > > OF, ACPI and software_nodes all implement graphs including nodes for ports > > and endpoints. These are all intended to be named with a common schema, > > as "port@n" and "endpoint@n" where n is an unsigned int representing the > > index of the node. To ensure commonality across the subsystems, provide a > > set of macros to define the format. > > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > > --- > > Changes in v3 > > - Patch introduced > > > > include/linux/fwnode.h | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h > > index 9506f8ec0974..52889efceb7d 100644 > > --- a/include/linux/fwnode.h > > +++ b/include/linux/fwnode.h > > @@ -32,6 +32,19 @@ struct fwnode_endpoint { > > const struct fwnode_handle *local_fwnode; > > }; > > > > +/* > > + * ports and endpoints defined in OF, ACPI and as software_nodes should all > > + * follow a common naming scheme; use these macros to ensure commonality across > > + * the subsystems. > > + * > > + * The *PREFIX_LEN macros refer to the length of the "port@" and "endpoint@" > > + * sections of the naming scheme. > > + */ > > +#define FWNODE_GRAPH_PORT_NAME_FORMAT "port@%u" > > +#define FWNODE_GRAPH_PORT_NAME_PREFIX_LEN 5 > > +#define FWNODE_GRAPH_ENDPOINT_NAME_FORMAT "endpoint@%u" > > +#define FWNODE_GRAPH_ENDPOINT_PREFIX_LEN 9 > > + > > #define NR_FWNODE_REFERENCE_ARGS 8 > > I'd keep such definitions local to the swnode implementation as neither > ACPI nor DT have an apparent need for them. They do use the naming, but > don't appear to format such strings. Ah, I noticed these are used by the later patches. Please ignore that comment then. But these are still definitions, so I'd use SWNODE prefix rather than FWNODE. That should also be reflected in the comment. ACPI does not have a format port concept so the definitions that are being used can be said to be specific to Linux.
Hi Sakari On 28/12/2020 17:11, Sakari Ailus wrote: > On Mon, Dec 28, 2020 at 06:30:24PM +0200, Sakari Ailus wrote: >> Hi Daniel, Andy, >> >> On Thu, Dec 24, 2020 at 01:08:59AM +0000, Daniel Scally wrote: >>> OF, ACPI and software_nodes all implement graphs including nodes for ports >>> and endpoints. These are all intended to be named with a common schema, >>> as "port@n" and "endpoint@n" where n is an unsigned int representing the >>> index of the node. To ensure commonality across the subsystems, provide a >>> set of macros to define the format. >>> >>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> Signed-off-by: Daniel Scally <djrscally@gmail.com> >>> --- >>> Changes in v3 >>> - Patch introduced >>> >>> include/linux/fwnode.h | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h >>> index 9506f8ec0974..52889efceb7d 100644 >>> --- a/include/linux/fwnode.h >>> +++ b/include/linux/fwnode.h >>> @@ -32,6 +32,19 @@ struct fwnode_endpoint { >>> const struct fwnode_handle *local_fwnode; >>> }; >>> >>> +/* >>> + * ports and endpoints defined in OF, ACPI and as software_nodes should all >>> + * follow a common naming scheme; use these macros to ensure commonality across >>> + * the subsystems. >>> + * >>> + * The *PREFIX_LEN macros refer to the length of the "port@" and "endpoint@" >>> + * sections of the naming scheme. >>> + */ >>> +#define FWNODE_GRAPH_PORT_NAME_FORMAT "port@%u" >>> +#define FWNODE_GRAPH_PORT_NAME_PREFIX_LEN 5 >>> +#define FWNODE_GRAPH_ENDPOINT_NAME_FORMAT "endpoint@%u" >>> +#define FWNODE_GRAPH_ENDPOINT_PREFIX_LEN 9 >>> + >>> #define NR_FWNODE_REFERENCE_ARGS 8 >> >> I'd keep such definitions local to the swnode implementation as neither >> ACPI nor DT have an apparent need for them. They do use the naming, but >> don't appear to format such strings. > > Ah, I noticed these are used by the later patches. Please ignore that > comment then. > > But these are still definitions, so I'd use SWNODE prefix rather than > FWNODE. That should also be reflected in the comment. Fine by me; I shall change that. > ACPI does not have a format port concept so the definitions that are being > used can be said to be specific to Linux. >
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 9506f8ec0974..52889efceb7d 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -32,6 +32,19 @@ struct fwnode_endpoint { const struct fwnode_handle *local_fwnode; }; +/* + * ports and endpoints defined in OF, ACPI and as software_nodes should all + * follow a common naming scheme; use these macros to ensure commonality across + * the subsystems. + * + * The *PREFIX_LEN macros refer to the length of the "port@" and "endpoint@" + * sections of the naming scheme. + */ +#define FWNODE_GRAPH_PORT_NAME_FORMAT "port@%u" +#define FWNODE_GRAPH_PORT_NAME_PREFIX_LEN 5 +#define FWNODE_GRAPH_ENDPOINT_NAME_FORMAT "endpoint@%u" +#define FWNODE_GRAPH_ENDPOINT_PREFIX_LEN 9 + #define NR_FWNODE_REFERENCE_ARGS 8 /**
OF, ACPI and software_nodes all implement graphs including nodes for ports and endpoints. These are all intended to be named with a common schema, as "port@n" and "endpoint@n" where n is an unsigned int representing the index of the node. To ensure commonality across the subsystems, provide a set of macros to define the format. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Daniel Scally <djrscally@gmail.com> --- Changes in v3 - Patch introduced include/linux/fwnode.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)