On Tue, Jul 26, 2022 at 8:18 PM Andy Shevchenko
[off-list ref] wrote:

...

quoted

Just for saving memory space.
Because these led_classdevs do not be used at the same time.
Or do you think it would be better to rewrite it as follows?
-------------------------------------------------------------------------------------
struct mt6370_led {
       struct led_classdev isink;
       struct led_classdev_mc mc;
       struct mt6370_priv *priv;
       u32 default_state;
       u32 index;
};
-------------------------------------------------------------------------------------

You obviously didn't get what I'm talking about...
Each union to work properly should have an associated variable that
holds the information of which field of the union is in use. Do you
have such a variable? If not, how does your code know which one to
use? If yes, add a proper comment there.

Ummm... from my understanding,
if the colors of these four LEDs are set to 'LED_COLOR_ID_RGB' or
'LED_COLOR_ID_MULTI' in DT,
their 'led->index' will be set to 'MT6370_VIRTUAL_MULTICOLOR' in
'mt6370_leds_probe()'.
If so, these led devices will be set as 'struct led_classdev_mc' and
use related ops functions in 'mt6370_init_led_properties()'.
Instead, they whose 'led->index' is not 'MT6370_VIRTUAL_MULTICOLOR'
will be set as 'struct led_classdev'.
So, maybe the member 'index' of the 'struct mt6370_led' is what you
describe the information of which field of the union is in use?
I will add the proper comment here to describe this thing. I'm so
sorry for misunderstanding your mean last time.
Thanks again for your review.

-- 
Best Regards,
ChiaEn Wu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel