Re: [PATCH v2 2/4] serdev: add private data to serdev_device

[Markus Probst]

On Fri, 2026-03-06 at 11:49 -0800, Randy Dunlap wrote:
> Hi--
> 
> On 3/6/26 11:35 AM, Markus Probst wrote:
> > Add private data to `struct serdev_device`, as it is required by the
> > rust abstraction added in the following commit
> > (rust: add basic serial device bus abstractions).
> > 
> > Signed-off-by: Markus Probst <markus.pro...@posteo.de>
> > ---
> >  include/linux/serdev.h | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> > index 5654c58eb73c..b591af23faf0 100644
> > --- a/include/linux/serdev.h
> > +++ b/include/linux/serdev.h
> > @@ -33,12 +33,13 @@ struct serdev_device_ops {
> >  
> >  /**
> >   * struct serdev_device - Basic representation of an serdev device
> > - * @dev:   Driver model representation of the device.
> > - * @nr:            Device number on serdev bus.
> > - * @ctrl:  serdev controller managing this device.
> > - * @ops:   Device operations.
> > - * @write_comp     Completion used by serdev_device_write() internally
> > - * @write_lock     Lock to serialize access when writing data
> > + * @dev:    Driver model representation of the device.
> > + * @nr:             Device number on serdev bus.
> > + * @ctrl:   serdev controller managing this device.
> > + * @ops:    Device operations.
> > + * @write_comp      Completion used by serdev_device_write() internally
> > + * @write_lock      Lock to serialize access when writing data
> > + * @private_data Private data for the device driver.
> >   */
> 
> I don't quite get why each changed line has an extra tab added to it. ?
So all the fields have the same indentation.
Otherwise the description of private_data, because of its length, would
be 1 character more to the right.

> Also, struct member names in kernel-doc should with a colon  (':'), e.g.,
> 
>  * @private_data: Private data for the device driver.
> 
> Please correct that and the 2 lines above it also.
> And maybe test it to check for warnings.
I can do that.

> 
> >  struct serdev_device {
> >     struct device dev;
> > @@ -47,6 +48,7 @@ struct serdev_device {
> >     const struct serdev_device_ops *ops;
> >     struct completion write_comp;
> >     struct mutex write_lock;
> > +   void *private_data;
> >  };
> >  
> >  static inline struct serdev_device *to_serdev_device(struct device *d)
> > 
> 
> thanks.

Thanks
- Markus Probst

signature.asc
Description: This is a digitally signed message part