2021-04-03 13:08:07

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] docs: driver-model: Update the documentation for device class

On Sat, Apr 03, 2021 at 05:30:50PM +0530, Manivannan Sadhasivam wrote:
> The current documentation about the device class is out of date such
> that it refers to non-existent APIs and structures. This commit updates
> them to the current device class APIs and structures, removes wordings
> that no longer valid while trying to keep the original content intact.

Thanks for working on this!

One thing that instantly jumped out at me:

> -Class drivers can export attributes using the DEVCLASS_ATTR macro that works
> -similarly to the DEVICE_ATTR macro for devices. For example, a definition
> +Class drivers can export attributes using the CLASS_ATTR_* macros that works
> +similarly to the DEVICE_ATTR_* macros for devices. For example, a definition
> like this::
>
> - static DEVCLASS_ATTR(debug,0644,show_debug,store_debug);
> + static CLASS_ATTR_RW(debug, 0644, show_debug, store_debug);

CLASS_ATTR_RW(debug);
is the correct way to write the above, what you added there will not
build.

But a meta-comment, should stuff like this go directly into the .c files
itself so that the documentation is created automatically? the fact
that this lives so "far away" from the source ensures that it will
always be out of date. I know other subsystems (graphics, v4l2) have
tied the documentation into their code files much better so I think the
build and markup infrastructure is there today to do this.

thanks,

greg k-h


2021-04-03 13:41:32

by 'Manivannan Sadhasivam'

[permalink] [raw]
Subject: Re: [PATCH] docs: driver-model: Update the documentation for device class

On Sat, Apr 03, 2021 at 03:04:42PM +0200, Greg KH wrote:
> On Sat, Apr 03, 2021 at 05:30:50PM +0530, Manivannan Sadhasivam wrote:
> > The current documentation about the device class is out of date such
> > that it refers to non-existent APIs and structures. This commit updates
> > them to the current device class APIs and structures, removes wordings
> > that no longer valid while trying to keep the original content intact.
>
> Thanks for working on this!
>
> One thing that instantly jumped out at me:
>
> > -Class drivers can export attributes using the DEVCLASS_ATTR macro that works
> > -similarly to the DEVICE_ATTR macro for devices. For example, a definition
> > +Class drivers can export attributes using the CLASS_ATTR_* macros that works
> > +similarly to the DEVICE_ATTR_* macros for devices. For example, a definition
> > like this::
> >
> > - static DEVCLASS_ATTR(debug,0644,show_debug,store_debug);
> > + static CLASS_ATTR_RW(debug, 0644, show_debug, store_debug);
>
> CLASS_ATTR_RW(debug);
> is the correct way to write the above, what you added there will not
> build.
>

Oops... I just did a blind replace there, thanks for spotting.

> But a meta-comment, should stuff like this go directly into the .c files
> itself so that the documentation is created automatically? the fact
> that this lives so "far away" from the source ensures that it will
> always be out of date. I know other subsystems (graphics, v4l2) have
> tied the documentation into their code files much better so I think the
> build and markup infrastructure is there today to do this.
>

Well you're right that this documentation is far from its implementation but
that applies to most of the stuffs inside kernel, right? Also, I think if we
move these into .c file, then it will flood the whole file IMO.

We already have the kernel doc for most of the APIs/structures and that should
be enough for the .c/.h code in my perspective. If a developer wants to obtain
more information other than the API/struct definitions, then they should land
in documentation.

It should be responsibility of the maintainer to keep the doc up-to date :)

Thanks,
Mani

> thanks,
>
> greg k-h