Re: [PATCH v6 12/13] leds: flash: mt6370: Add MediaTek MT6370 flashlight support

From: szuni chen
Date: Tue Jul 26 2022 - 00:15:59 EST


Hi Andy,

>
> > +#define MT6370_ITORCH_MIN_UA 25000
> > +#define MT6370_ITORCH_STEP_UA 12500
> > +#define MT6370_ITORCH_MAX_UA 400000
> > +#define MT6370_ITORCH_DOUBLE_MAX_UA 800000
> > +#define MT6370_ISTRB_MIN_UA 50000
> > +#define MT6370_ISTRB_STEP_UA 12500
> > +#define MT6370_ISTRB_MAX_UA 1500000
> > +#define MT6370_ISTRB_DOUBLE_MAX_UA 3000000
>
> Perhaps _uA would be better and consistent across your series
> regarding current units.
>

Yes, _uA will be more consistent, but in general, using upper case in
the define macro is a convention, doesn't it?

>
> > + /*
> > + * For the flash to turn on/off, need to wait for HW ramping up/down time
>
> we need
>
> > + * 5ms/500us to prevent the unexpected problem.
> > + */
> > + if (!prev && curr)
> > + usleep_range(5000, 6000);
> > + else if (prev && !curr)
> > + udelay(500);
>
> This still remains unanswered, why in the first place we allow
> switching, and a busy loop in the other place?

If I refine the description to
"For the flash to turn on/off, need to wait for 5ms/500us analog settling time.
If any flash led is already used, then the analog is settled done, we
don't need to wait again."
is it answer the question?


Best regards,
Alice