On 13/05/2022 21:04, Kyle Swenson wrote: > The Awinic AW21024 LED controller is a 24-channel RGB LED controller. > Each LED on the controller can be controlled individually or grouped > with other LEDs on the controller to form a multi-color LED. Arbitrary > combinations of individual and grouped LED control should be possible. > > Signed-off-by: Kyle Swenson <kyle.swenson@xxxxxxxx> Thank you for your patch. There is something to discuss/improve. > + > +static const struct i2c_device_id aw21024_id[] = { > + { "aw21024", 0 }, /* 24 Channel */ > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, aw21024_id); > + > +static const struct of_device_id of_aw21024_leds_match[] = { > + { .compatible = "awinic,aw21024", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, of_aw21024_leds_match); > + > +static struct i2c_driver aw21024_driver = { > + .driver = { > + .name = "aw21024", > + .of_match_table = of_match_ptr(of_aw21024_leds_match), of_match_ptr causes this being unused. kbuild robot probably pointed this out... if not - of_match_ptr goes with maybe_unused. You need both or none, depending on intended usage. > + }, > + .probe_new = aw21024_probe, > + .remove = aw21024_remove, > + .id_table = aw21024_id, Why other places are indented but this not? > +}; > +module_i2c_driver(aw21024_driver); > + > +MODULE_AUTHOR("Kyle Swenson <kyle.swenson@xxxxxxxx>"); > +MODULE_DESCRIPTION("Awinic AW21024 LED driver"); > +MODULE_LICENSE("GPL"); Best regards, Krzysztof