On Fri, Jun 15, 2018 at 10:54:45PM +0200, Jacek Anaszewski wrote:
> This one requires USB maintainer ack.
I hate to be that person, but:
>
> Adding Greg.
>
> On 06/14/2018 11:16 PM, Uwe Kleine-König wrote:
> > The led trigger core learned a few things that allow to simplify the trigger drivers.
> > Make use of automated trigger attributes and error checking of the activate callback.
> > Also use the wrappers to set and get trigger_data.
> >
> > Acked-by: Pavel Machek <pavel@xxxxxx>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > ---
> > drivers/usb/core/ledtrig-usbport.c | 31 +++++++++++-------------------
> > 1 file changed, 11 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
> > index aa1d51458da7..dc7f7fd71684 100644
> > --- a/drivers/usb/core/ledtrig-usbport.c
> > +++ b/drivers/usb/core/ledtrig-usbport.c
> > @@ -113,11 +113,17 @@ static ssize_t usbport_trig_port_store(struct device *dev,
> > static struct attribute *ports_attrs[] = {
> > NULL,
> > };
> > +
Ok, that looks nice...
> > static const struct attribute_group ports_group = {
> > .name = "ports",
> > .attrs = ports_attrs,
> > };
> > +static const struct attribute_group *ports_groups[] = {
> > + &ports_group,
> > + NULL
> > +};
No extra line there?
And what about the ATTRIBUTE_GROUPS() macro?
> > +
> > /***************************************
> > * Adding & removing ports
> > ***************************************/
> > @@ -301,59 +307,44 @@ static int usbport_trig_notify(struct notifier_block *nb, unsigned long action,
> > static int usbport_trig_activate(struct led_classdev *led_cdev)
> > {
> > struct usbport_trig_data *usbport_data;
> > - int err;
Leave a blank line here please.
> > usbport_data = kzalloc(sizeof(*usbport_data), GFP_KERNEL);
> > if (!usbport_data)
> > - return 0;
> > + return -ENOMEM;
This is a separate bug fix.
Anyway, all minor things, feel free to fix them up later on:
Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
And remember to run your patches through checkpatch.pl, it would have
caught some of this already...
thanks,
greg k-h
