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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Jul 2022 08:10:55 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     szuni chen <szunichen@...il.com>
Cc:     ChiaEn Wu <peterwu.pub@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Jingoo Han <jingoohan1@...il.com>, Pavel Machek <pavel@....cz>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sebastian Reichel <sre@...nel.org>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Guenter Roeck <linux@...ck-us.net>,
        "Krogerus, Heikki" <heikki.krogerus@...ux.intel.com>,
        Helge Deller <deller@....de>,
        ChiaEn Wu <chiaen_wu@...htek.com>,
        Alice Chen <alice_chen@...htek.com>,
        cy_huang <cy_huang@...htek.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Linux LED Subsystem <linux-leds@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        "open list:FRAMEBUFFER LAYER" <linux-fbdev@...r.kernel.org>
Subject: Re: [PATCH v6 12/13] leds: flash: mt6370: Add MediaTek MT6370
 flashlight support

On Tue, Jul 26, 2022 at 6:15 AM szuni chen <szunichen@...il.com> wrote:

...

> > > +#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?

There is general convention, but there are also:
1) common sense;
2) usage in practice (e.g. _US, etc for *seconds and _HZ for *frequency).

My common sense tells me that it is convenient to use mA,uA, etc.
Plus "in practice" it's related to use as in your series and elsewhere.

But of course it's minor to me, decide yourself.

...

> > > +       /*
> > > +        * 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?

No. I'm talking from the Linux APIs perspective. There is a huge
difference between those branches. Please, conduct research, read
documentation to understand what my question is about.

-- 
With Best Regards,
Andy Shevchenko