[PATCH v1 1/1] device property: Add a note to the fwnode.h

[PATCH v1 1/1] device property: Add a note to the fwnode.h

[Andy Shevchenko]

Add a note to the fwnode.h that the header should not be used
directly in the leaf drivers, they all should use the higher
level APIs and the respective headers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/fwnode.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 6fa0a268d538..da537c7c30dc 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -2,6 +2,12 @@
 /*
  * fwnode.h - Firmware device node object handle type definition.
  *
+ * Note, this header is not meant to be used by the leaf drivers.
+ * It provides the low level data types and definitions for the firmware
+ * and device property providers. The respective API headers should
+ * guarantee all the required data types and definitions without including
+ * this header directly.
+ *
  * Copyright (C) 2015, Intel Corporation
  * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  */
-- 
2.47.2

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Andy Shevchenko]

On Mon, Mar 31, 2025 at 07:32:27PM +0300, Andy Shevchenko wrote:
> Add a note to the fwnode.h that the header should not be used
> directly in the leaf drivers, they all should use the higher
> level APIs and the respective headers.

I re-read it and it's ambiguous, I fixed this in v2 that has been just sent.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Rafael J. Wysocki]

On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Add a note to the fwnode.h that the header should not be used
> directly in the leaf drivers, they all should use the higher
> level APIs and the respective headers.

This sounds like a solution to a problem, but the problem statement is missing.

What's your motivation?

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/fwnode.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 6fa0a268d538..da537c7c30dc 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -2,6 +2,12 @@
>  /*
>   * fwnode.h - Firmware device node object handle type definition.
>   *
> + * Note, this header is not meant to be used by the leaf drivers.
> + * It provides the low level data types and definitions for the firmware
> + * and device property providers. The respective API headers should
> + * guarantee all the required data types and definitions without including
> + * this header directly.
> + *
>   * Copyright (C) 2015, Intel Corporation
>   * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>   */
> --
> 2.47.2
>
>

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Andy Shevchenko]

On Mon, Apr 07, 2025 at 08:17:17PM +0200, Rafael J. Wysocki wrote:
> On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Add a note to the fwnode.h that the header should not be used
> > directly in the leaf drivers, they all should use the higher
> > level APIs and the respective headers.
> 
> This sounds like a solution to a problem, but the problem statement is missing.

> What's your motivation?

Found a few drivers that are mistakenly include fwnode.h while they meant to
have either of.h or more likely property.h.

...

> > + * Note, this header is not meant to be used by the leaf drivers.
> > + * It provides the low level data types and definitions for the firmware
> > + * and device property providers. The respective API headers should
> > + * guarantee all the required data types and definitions without including
> > + * this header directly.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Rafael J. Wysocki]

On Mon, Apr 7, 2025 at 8:36 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Apr 07, 2025 at 08:17:17PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > Add a note to the fwnode.h that the header should not be used
> > > directly in the leaf drivers, they all should use the higher
> > > level APIs and the respective headers.
> >
> > This sounds like a solution to a problem, but the problem statement is missing.
>
> > What's your motivation?
>
> Found a few drivers that are mistakenly include fwnode.h while they meant to
> have either of.h or more likely property.h.

I see.

I would then say

"This header file provides low-level data types and definitions for
firmware and device property providers.  The respective API header
files supplied by them should contain all of the requisite data types
and definitions for end users, so including it directly should not be
necessary."

And I would mention that the purpose is to give guidance to driver
writers to avoid repeating a common mistake.

>
> ...
>
> > > + * Note, this header is not meant to be used by the leaf drivers.
> > > + * It provides the low level data types and definitions for the firmware
> > > + * and device property providers. The respective API headers should
> > > + * guarantee all the required data types and definitions without including
> > > + * this header directly.
>
> --

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Andy Shevchenko]

On Mon, Apr 07, 2025 at 08:44:20PM +0200, Rafael J. Wysocki wrote:
> On Mon, Apr 7, 2025 at 8:36 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Apr 07, 2025 at 08:17:17PM +0200, Rafael J. Wysocki wrote:
> > > On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > Add a note to the fwnode.h that the header should not be used
> > > > directly in the leaf drivers, they all should use the higher
> > > > level APIs and the respective headers.
> > >
> > > This sounds like a solution to a problem, but the problem statement is missing.
> >
> > > What's your motivation?
> >
> > Found a few drivers that are mistakenly include fwnode.h while they meant to
> > have either of.h or more likely property.h.
> 
> I see.
> 
> I would then say
> 
> "This header file provides low-level data types and definitions for
> firmware and device property providers.  The respective API header
> files supplied by them should contain all of the requisite data types
> and definitions for end users, so including it directly should not be
> necessary."
> 
> And I would mention that the purpose is to give guidance to driver
> writers to avoid repeating a common mistake.

A-ha, thanks for the suggestion, since there is also a v2, which almost
the same, I will incorporate it into v3.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Rafael J. Wysocki]

On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Add a note to the fwnode.h that the header should not be used
> directly in the leaf drivers, they all should use the higher
> level APIs and the respective headers.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  include/linux/fwnode.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 6fa0a268d538..da537c7c30dc 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -2,6 +2,12 @@
>  /*
>   * fwnode.h - Firmware device node object handle type definition.
>   *
> + * Note, this header is not meant to be used by the leaf drivers.
> + * It provides the low level data types and definitions for the firmware
> + * and device property providers. The respective API headers should
> + * guarantee all the required data types and definitions without including
> + * this header directly.
> + *
>   * Copyright (C) 2015, Intel Corporation
>   * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>   */
> --
> 2.47.2
>
>

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:19:03PM +0200, Rafael J. Wysocki wrote:
> On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Add a note to the fwnode.h that the header should not be used
> > directly in the leaf drivers, they all should use the higher
> > level APIs and the respective headers.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thank you, but you already commented on this and proposed the better wording
which is in v2. So, what should I do now?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Rafael J. Wysocki]

On Wed, Apr 9, 2025 at 4:46 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Apr 09, 2025 at 04:19:03PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > Add a note to the fwnode.h that the header should not be used
> > > directly in the leaf drivers, they all should use the higher
> > > level APIs and the respective headers.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Thank you, but you already commented on this and proposed the better wording
> which is in v2. So, what should I do now?

The tag is for the v2, sorry.  Let me add it there.

Re: [PATCH v1 1/1] device property: Add a note to the fwnode.h

[Rafael J. Wysocki]

On Wed, Apr 9, 2025 at 5:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Apr 9, 2025 at 4:46 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Wed, Apr 09, 2025 at 04:19:03PM +0200, Rafael J. Wysocki wrote:
> > > On Mon, Mar 31, 2025 at 6:32 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > Add a note to the fwnode.h that the header should not be used
> > > > directly in the leaf drivers, they all should use the higher
> > > > level APIs and the respective headers.
> > > >
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >
> > > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Thank you, but you already commented on this and proposed the better wording
> > which is in v2. So, what should I do now?
>
> The tag is for the v2, sorry.  Let me add it there.

For the v3 rather, but never mind.  Already sent.