Matti Vaittinen <mazziesaccount@xxxxxxxxx> 於 2022年4月28日 週四 下午7:51寫道: > > Hi ChiYuan! > > On Thu, Apr 28, 2022 at 1:03 PM cy_huang <u0084500@xxxxxxxxx> wrote: > > From: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > Add mt6360 isnk channel hardware breath mode support. > > > > Signed-off-by: ChiYuan Huang <cy_huang@xxxxxxxxxxx> > > > > +static int mt6360_gen_breath_reg_config(struct led_pattern *pattern, u32 len, > > + u8 *vals, int val_len) > > +{ > > + static const struct linear_range tranges[MT6360_ILED_RANGE_MAX] = { > > + { 125, 0, 15, 250 }, /* tr/f12 and ton, unit: millisecond */ > > + { 250, 0, 15, 500 }, /* toff, unit: millisecond */ > > + }; > > It's nice to see you are using the linear_ranges helpers here! Just a > minor remark - do you think you could use field names in linear_ranges > initializations? That would make it less likely the driver breaks if > someone changes the struct linear_range definition. Eg, use something > like: > > static const struct linear_range tranges[MT6360_ILED_RANGE_MAX] = { > /* tr/f12 and ton, unit: millisecond */ > { .min = 125, .min_sel = 0, .max_sel = 15, .step = 250 }, > /* toff, unit: millisecond */ > { .min = 250, .min_sel = 0, .max_sel = 15, .step = 500 }, > }; > > Do you think that would work? Sure, it works. To specify the field name can be compatible if the struct changes in the future. Thanks. > > Best Regards > -- Matti > > -- > > Matti Vaittinen > Linux kernel developer at ROHM Semiconductors > Oulu Finland > > ~~ When things go utterly wrong vim users can always type :help! ~~ > > Discuss - Estimate - Plan - Report and finally accomplish this: > void do_work(int time) __attribute__ ((const));