On Wed, Mar 27, 2024 at 7:06 PM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > On Wed, Mar 27, 2024 at 9:58 AM Kate Hsuan <hpa@xxxxxxxxxx> wrote: > > On Mon, Mar 25, 2024 at 3:30 AM Andy Shevchenko > > <andy.shevchenko@xxxxxxxxx> wrote: > > > On Sun, Mar 24, 2024 at 5:02 PM Kate Hsuan <hpa@xxxxxxxxxx> wrote: > > ... > > > > > +/* main fwnode for ktd2026 */ > > > > +static const struct software_node ktd2026_node = { > > > > + .name = "ktd2026" > > > > > > Leave a comma, this is not a terminator. > > > > > > > +}; > > > > > > When I asked about the name I relied on the fact that you have an idea > > > how it works. So, assuming my understanding is correct, this platform > > > may not have more than a single LED of this type. Dunno if we need a > > > comment about this. > > > > I'll make a comment to describe the configuration. > > This LED controller can be configured to an RGB LED like this. Also, > > it can be configured as three single-color (RGB) LEDs to show red, > > green, and blue only. > > I think the name can be "ktd2026-multi-color". Is it good for you? > > My point here is that the name is static and if you have more than one > LED in the system, the second one won't be registered due to sysfs > name collisions. Question here is how many of these types of LEDs are > possible on the platform? If more than one, the name has to be > dropped. Writing this I think a comment would be good to have in any > case. Only one RGB LED controller on this platform so we can name it. I also tested the LED with and without the name and the LED worked properly. I think the name can be dropped and put a comment there to describe the usage and configuration of the LED controller. Thank you :) > > ... > > > > > +static int __init xiaomi_mipad2_init(void) > > > > +{ > > > > + return software_node_register_node_group(ktd2026_node_group); > > > > +} > > > > + > > > > +static void xiaomi_mipad2_exit(void) > > > > > > __exit ? > > No need. > > x86-andriod-tablet is based on platform_driver and platform_device so > > it doesn't need __exit. > > > > I put __exit and the compiler complained about the warning. > > === > > WARNING: modpost: > > drivers/platform/x86/x86-android-tablets/x86-android-tablets: section > > mismatch in reference: xiaomi_mipad2_info+0x50 (section: .init.rodata) > > -> xiaomi_mipad2_exit (section: .exit.text) > > === > > This is interesting. Why then do we call them symmetrically? > > Hans, do we need to have anything here been amended? > > -- > With Best Regards, > Andy Shevchenko > -- BR, Kate