On Wed, Feb 19, 2025 at 11:06:02AM +0200, Dmitry Baryshkov wrote:
> On Tue, Feb 18, 2025 at 08:29:46PM +0100, Jerome Brunet wrote:
> > Add helper functions to create a device on the auxiliary bus.
> > 
> > This is meant for fairly simple usage of the auxiliary bus, to avoid having
> > the same code repeated in the different drivers.
> > 
> > Suggested-by: Stephen Boyd <sb...@kernel.org>
> > Cc: Arnd Bergmann <a...@arndb.de>
> > Signed-off-by: Jerome Brunet <jbru...@baylibre.com>
> > ---
> >  drivers/base/auxiliary.c      | 108 
> > ++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/auxiliary_bus.h |  17 +++++++
> >  2 files changed, 125 insertions(+)
> > 
> > diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> > index 
> > afa4df4c5a3f371b91d8dd8c4325495d32ad1291..a6d46c2759be81a0739f07528d5959c2a76eb8a8
> >  100644
> > --- a/drivers/base/auxiliary.c
> > +++ b/drivers/base/auxiliary.c
> > @@ -385,6 +385,114 @@ void auxiliary_driver_unregister(struct 
> > auxiliary_driver *auxdrv)
> >  }
> >  EXPORT_SYMBOL_GPL(auxiliary_driver_unregister);
> >  
> > +static void auxiliary_device_release(struct device *dev)
> > +{
> > +   struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
> > +
> > +   kfree(auxdev);
> > +}
> > +
> > +/**
> > + * auxiliary_device_create - create a device on the auxiliary bus
> > + * @dev: parent device
> > + * @modname: module name used to create the auxiliary driver name.
> > + * @devname: auxiliary bus device name
> > + * @platform_data: auxiliary bus device platform data
> > + * @id: auxiliary bus device id
> > + *
> > + * Helper to create an auxiliary bus device.
> > + * The device created matches driver 'modname.devname' on the auxiliary 
> > bus.
> > + */
> > +struct auxiliary_device *auxiliary_device_create(struct device *dev,
> > +                                            const char *modname,
> > +                                            const char *devname,
> > +                                            void *platform_data,
> > +                                            int id)
> > +{
> > +   struct auxiliary_device *auxdev;
> > +   int ret;
> > +
> > +   auxdev = kzalloc(sizeof(*auxdev), GFP_KERNEL);
> > +   if (!auxdev)
> > +           return NULL;
> > +
> > +   auxdev->id = id;
> > +   auxdev->name = devname;
> > +   auxdev->dev.parent = dev;
> > +   auxdev->dev.platform_data = platform_data;
> > +   auxdev->dev.release = auxiliary_device_release;
> > +   device_set_of_node_from_dev(&auxdev->dev, dev);
> > +
> > +   ret = auxiliary_device_init(auxdev);
> > +   if (ret) {
> > +           kfree(auxdev);
> > +           return NULL;
> > +   }
> > +
> > +   ret = __auxiliary_device_add(auxdev, modname);
> > +   if (ret) {
> 
> This loses possible error return values from __auxiliary_device_add().

Why does that really matter?

> I'd suggest to return ERR_PTR(ret) here and in the
> auxiliary_device_init() chunks and ERR_PTR(-ENOMEM) in case of kzalloc()
> failure.

Will the caller do something different based on the error value here?
All we care is that this worked or not, the specific error isn't going
to matter for device creation like this.

thanks,

greg k-h

Reply via email to