вт, 9 черв. 2026 р. о 22:23 Andy Shevchenko <[email protected]> пише:
>
> On Sat, Jun 06, 2026 at 07:57:38AM +0300, Svyatoslav Ryhel wrote:
> > Add Control Bank to HVLED/LVLED muxing support based on the led-sources
> > defined in the device tree.
>
> ...
>
> > static int lm3533_led_setup(struct lm3533_led *led)
> > {
> > - int ret;
> > + u32 output_cfg_shift = 0;
>
> No need to assign the default to this.
>
This is a personal preference, compiler will optimize it if this is not needed.
> > + u32 output_cfg_val = 0;
> > + u32 output_cfg_mask = 0;
> > + int ret, i;
>
> No need to add 'i'.
>
This is personal preference as well. There is no strict rule that
iteration variable must be defined strictly in the for loop.
> > + if (led->num_leds) {
> > + for (i = 0; i < led->num_leds; i++) {
>
> for (unsigned int i = 0; i < led->num_leds; i++) {
>
> > + if (led->leds[i] >= LM3533_LVCTRLBANK_MAX)
> > + continue;
> > +
> > + output_cfg_shift = led->leds[i] * 2;
> > + output_cfg_val |= led->id << output_cfg_shift;
> > + output_cfg_mask |= OUTPUT_LVLED_MASK <<
> > output_cfg_shift;
> > + }
> > +
> > + /* LVLED1, LVLED2 and LVLED3 */
> > + ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF1,
> > + output_cfg_mask <<
> > OUTPUT_CONF1_SHIFT,
> > + output_cfg_val <<
> > OUTPUT_CONF1_SHIFT);
> > + if (ret)
> > + return ret;
> > +
> > + /* LVLED4 and LVLED5 */
> > + ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF2,
> > + output_cfg_mask >>
> > OUTPUT_CONF2_SHIFT,
> > + output_cfg_val >>
> > OUTPUT_CONF2_SHIFT);
> > + if (ret)
> > + return ret;
> > + }
>
> ...
>
> > + if (led->num_leds > 0) {
> > + ret = device_property_read_u32_array(&pdev->dev,
> > "led-sources",
> > + led->leds,
> > led->num_leds);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to get led-sources\n");
> > + goto err_deregister;
> > + }
> > + }
>
> This and other pieces may benefit from local variable
>
> struct device *dev = &pdev->dev;
>
> defined at the top of the function.
>
Yes, but this would require an additional patch, which I will not add
to this already overinflated series.
> ...
>
> > static int lm3533_bl_setup(struct lm3533_bl *bl)
>
> As per above.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
