RE: [PATCH v7 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

[Roleda, Jan carlo]

Hello Uwe,

Thank you for the review.

> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> Sent: Thursday, July 16, 2026 10:56 PM
> To: Roleda, Jan carlo <Jancarlo.Roleda@xxxxxxxxxx>
> Cc: Lee Jones <lee@xxxxxxxxxx>; Pavel Machek <pavel@xxxxxxxxxx>; Rob
> Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor
> Dooley <conor+dt@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; linux-
> leds@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v7 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay
> LED Driver
> 
> [External]
> 
> Hello,
> 
> On Wed, Jul 08, 2026 at 03:00:07PM +0800, Jan Carlo Roleda wrote:
> > +#include <linux/bitfield.h>
> > +#include <linux/errno.h>
> > +#include <linux/i2c.h>
> > +#include <linux/leds.h>
> > +#include <linux/mod_devicetable.h>
> 
> Please don't include this header. The the device-id structs you use are available
> via <linux/i2c.h> already.
> 

Noted, Thank you for the information!

> > +#include <linux/module.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +#include <linux/types.h>
> > [...]
> > +static const struct reg_default ltc3208_reg_defaults[LTC3208_REG_G_OPT]
> = {
> > +	{LTC3208_REG_A_GRNRED, 0},
> > +	{LTC3208_REG_B_AUXBLU, 0},
> > +	{LTC3208_REG_C_MAIN, 0},
> > +	{LTC3208_REG_D_SUB, 0},
> > +	{LTC3208_REG_E_AUX_SELECT, 0},
> > +	{LTC3208_REG_F_CAM, 0},
> > +	{LTC3208_REG_G_OPT, 0}
> 
> Space after opening { and before } please.
> 

Noted on these changes.

> > +};
> > [...]
> > +	device_for_each_child_node_scoped(&client->dev, child) {
> > +		struct ltc3208_led *led;
> > +		struct led_init_data init_data = {};
> 
> s/{}/{ }/ is the more usual option.
> 
> > +		u32 chan;
> 
> Best regards
> Uwe

I will combine these suggestions with those of Lee Jones in the next patch.

Regards,
Carlo