Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 2 Jun 2022 11:17:40 +0200 | Subject | Re: [PATCH 06/14] leds: mt6370: Add Mediatek MT6370 Indicator support |
| |
On Thu, Jun 2, 2022 at 8:27 AM ChiYuan Huang <u0084500@gmail.com> wrote: > On Wed, Jun 01, 2022 at 11:48:58AM +0200, Andy Shevchenko wrote: > > On Tue, May 31, 2022 at 1:16 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
...
> > What indicator? > It's RGB curent sink type LED driver (maximum supported current is only 24mA).
Make your commit messages a slightly more verbose.
...
> > > +#include <linux/of.h> > > > > Are you sure this is the correct header? Seems you need > > mod_devicetable.h instead. > > > It's the correct header and be used for the struct 'of_device_id'.
Nope. Run the following command $ git grep -n 'struct of_device_id {' -- include/linux/
...
> > > +struct mt6370_priv { > > > + struct mutex lock; > > > > Do you use regmap locking? > > > MFD regmap register already the access lock. > > This lock is just to guarantee only one user can access the RGB register > part. > > Sorry, from the comment, do you want us to rename or remove this lock?
My point is, since you have two locks, explain why you need each of them.
> > > + struct device *dev; > > > > > + struct regmap *regmap; > > > > > + struct regmap_field *fields[F_MAX_FIELDS]; > > > + const struct reg_field *reg_fields; > > > + const struct linear_range *ranges; > > > + struct reg_cfg *reg_cfgs; > > > + unsigned int leds_count; > > > + unsigned int leds_active; > > > + bool is_mt6372; > > > + struct mt6370_led leds[]; > > > +};
-- With Best Regards, Andy Shevchenko
| |