Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <f650d4944a291197467e594c75103243f9943527.camel@fi.rohmeurope.com>
Subject: Re: [PATCH v4 2/2] power: supply: mt6360_charger: add MT6360
 charger support
From:   Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reply-To: matti.vaittinen@fi.rohmeurope.com
To:     Gene Chen <gene.chen.richtek@gmail.com>
Cc:     sre@kernel.org, Matthias Brugger <matthias.bgg@gmail.com>,
        Rob Herring <robh+dt@kernel.org>, linux-pm@vger.kernel.org,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Gene Chen <gene_chen@richtek.com>, Wilma.Wu@mediatek.com,
        shufan_lee@richtek.com, ChiYuan Huang <cy_huang@richtek.com>,
        benjamin.chao@mediatek.com
In-Reply-To: <CAE+NS36skw0XRCnNzHp8KSvdS+YPCAgwBNM-F7Wg=dxuiF5z1w@mail.gmail.com>
References: <1610973703-676-1-git-send-email-gene.chen.richtek@gmail.com>
         <1610973703-676-3-git-send-email-gene.chen.richtek@gmail.com>
         <771c7da0584cf37da6ba370207a89a7401a20c33.camel@fi.rohmeurope.com>
         <CAE+NS35BOC6rr4U9rtBkNph8mZCVV=5MGbh0VQXBvhLRkBaAvQ@mail.gmail.com>
         <a4646a277d692f4b4a04382b7367b641b2ff0fc6.camel@fi.rohmeurope.com>
         <CAE+NS36skw0XRCnNzHp8KSvdS+YPCAgwBNM-F7Wg=dxuiF5z1w@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
Date:   Thu, 27 May 2021 13:22:33 +0300
User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) 
Content-Transfer-Encoding: 8bit
Precedence: bulk


On Thu, 2021-05-27 at 17:58 +0800, Gene Chen wrote:
> Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> 於 2021年5月27日 週四
> 下午12:25寫道：
> > 
> > On Wed, 2021-05-26 at 17:40 +0800, Gene Chen wrote:
> > > Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> 於 2021年3月30日
> > > 週二
> > > 下午7:48寫道：
> > > > On Mon, 2021-01-18 at 20:41 +0800, Gene Chen wrote:
> > > > > From: Gene Chen <gene_chen@richtek.com>
> > > > > 
> > > > > Add basic support for the battery charger for MT6360 PMIC
> > > > > 
> > > > > Signed-off-by: Gene Chen <gene_chen@richtek.com>
> > > > > ---
> > > > >  drivers/power/supply/Kconfig          |  10 +
> > > > >  drivers/power/supply/Makefile         |   1 +
> > > > >  drivers/power/supply/mt6360_charger.c | 914
> > > > > ++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 925 insertions(+)
> > > > >  create mode 100644 drivers/power/supply/mt6360_charger.c
> > > > > 
> > > > 
> > > > Thanks for the contribution :)
> > > > 
> > > > Few comments which I am not demanding to be 'fixed' - but which
> > > > might
> > > > be good to be checked. Eg, please consider my comments as
> > > > 'nit's.
> > > > 
> > > > ...
> > > > 
> > > > > +static unsigned int mt6360_map_reg_sel(u32 data, u32 min,
> > > > > u32
> > > > > max,
> > > > > u32 step)
> > > > > +{
> > > > > +     u32 target = 0, max_sel;
> > > > > +
> > > > > +     if (data >= min) {
> > > > > +             target = (data - min) / step;
> > > > > +             max_sel = (max - min) / step;
> > > > > +             if (target > max_sel)
> > > > > +                     target = max_sel;
> > > > > +     }
> > > > > +     return target;
> > > > > +}
> > > > 
> > > > lib/linear_ranges.c might already implement this ...
> > > > 
> > > 
> > > I found we are neither linear_range_get_selector_high or
> > > linear_range_get_selector_low.
> > > When value lower than min_value, choose min_sel. If higher than
> > > max_value, choose max_sel.
> > 
> > Ah, correct.
> > 
> > > Should I create linear_range_get_selector() for this?
> > 
> > My suggestion would be yes, but I am not insisting on it.
> > 
> 
> Maybe this is a special choosing mechanism, I will keep it.
> Could I also keep mt6360_map_real_val?

I think adding generic function to linear range code for others to re-
use would be nice. But if you choose to keep the mt6360_map_reg_sel()
then it I agree that you should also keep the mt6360_map_real_val().

Best regards
	Matti Vaittinen