Messages in this thread | | | Date | Mon, 15 Mar 2021 12:19:15 +0100 | From | Pavel Machek <> | Subject | Re: [PATCH v2 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs |
| |
> > + struct led_classdev cdev; > > +}; > > + > > +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > > + {1 << 15, "simatic-ipc:green:" LED_FUNCTION_STATUS "-1" }, > > + {1 << 7, "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-1" }, > > + {1 << 14, "simatic-ipc:red:" LED_FUNCTION_STATUS "-2" }, > > + {1 << 6, "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-2" }, > > + {1 << 13, "simatic-ipc:red:" LED_FUNCTION_STATUS "-3" }, > > + {1 << 5, "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-3" }, > > Can you use BIT() macro here? And can it be sorted by the bit order?
There's nothing wrong with << and this order is fine.
But I still don't like the naming. simantic-ipc: prefix is useless. Having 6 status leds is not good, either.
> > + struct simatic_ipc_led *led = > > + container_of(led_cd, struct simatic_ipc_led, cdev); > > One line?
80 columns. It is fine as it is.
Best regards,
Pavel -- http://www.livejournal.com/~pavelmachek [unhandled content-type:application/pgp-signature]
| |