Re: [PATCH] USB: apple-mfi-fastcharge: Use devm_kzalloc and simplify the code

[Greg Kroah-Hartman]

On Sat, Nov 14, 2020 at 02:17:48PM +0000, Lucas Tanure wrote:
> On Sat, Nov 14, 2020 at 12:56 PM Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Sat, Nov 14, 2020 at 12:42:49PM +0000, Lucas Tanure wrote:
> > > Signed-off-by: Lucas Tanure <tanure@xxxxxxxxx>
> >
> > I can't take patches without any changelog text, sorry.
> >
> > > ---
> > >  drivers/usb/misc/apple-mfi-fastcharge.c | 17 +++++------------
> > >  1 file changed, 5 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/usb/misc/apple-mfi-fastcharge.c b/drivers/usb/misc/apple-mfi-fastcharge.c
> > > index 9de0171b5177..de86e389a008 100644
> > > --- a/drivers/usb/misc/apple-mfi-fastcharge.c
> > > +++ b/drivers/usb/misc/apple-mfi-fastcharge.c
> > > @@ -178,16 +178,13 @@ static int mfi_fc_probe(struct usb_device *udev)
> > >  {
> > >       struct power_supply_config battery_cfg = {};
> > >       struct mfi_device *mfi = NULL;
> > > -     int err;
> > >
> > >       if (!mfi_fc_match(udev))
> > >               return -ENODEV;
> > >
> > > -     mfi = kzalloc(sizeof(struct mfi_device), GFP_KERNEL);
> > > -     if (!mfi) {
> > > -             err = -ENOMEM;
> > > -             goto error;
> > > -     }
> > > +     mfi = devm_kzalloc(&udev->dev, sizeof(*mfi), GFP_KERNEL);
> > > +     if (!mfi)
> > > +             return -ENOMEM;
> > >
> > >       battery_cfg.drv_data = mfi;
> > >
> > > @@ -197,8 +194,7 @@ static int mfi_fc_probe(struct usb_device *udev)
> > >                                               &battery_cfg);
> > >       if (IS_ERR(mfi->battery)) {
> > >               dev_err(&udev->dev, "Can't register battery\n");
> > > -             err = PTR_ERR(mfi->battery);
> > > -             goto error;
> > > +             return PTR_ERR(mfi->battery);
> > >       }
> > >
> > >       mfi->udev = usb_get_dev(udev);
> > > @@ -206,9 +202,6 @@ static int mfi_fc_probe(struct usb_device *udev)
> > >
> > >       return 0;
> > >
> > > -error:
> > > -     kfree(mfi);
> > > -     return err;
> > >  }
> > >
> > >  static void mfi_fc_disconnect(struct usb_device *udev)
> > > @@ -220,7 +213,7 @@ static void mfi_fc_disconnect(struct usb_device *udev)
> > >               power_supply_unregister(mfi->battery);
> > >       dev_set_drvdata(&udev->dev, NULL);
> > >       usb_put_dev(mfi->udev);
> > > -     kfree(mfi);
> > > +     devm_kfree(&udev->dev, mfi);
> >
> > Are you sure about this?
> I think so, as the probe will allocate again that struct, the
> disconnect should free the previous one.

Why do you need to manually free it here like this?

Why are you trying to convert this file to this api anyway?

thanks,

greg k-h