From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Date: Wed, 24 Mar 2021 10:01:28 +0800
Subject: [PATCH v2 1/2] power: regulator: add driver for ANATOP regulator
In-Reply-To: <872fc093-626d-8324-fa2c-38d12714329f@seco.com>
References: <20210323134836.99512-1-grandpaul@gmail.com>
 <20210323134836.99512-2-grandpaul@gmail.com>
 <872fc093-626d-8324-fa2c-38d12714329f@seco.com>
Message-ID: <058eec11-4d4f-e6b9-a1a4-05106979a59b@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Sean,

Thanks for the review. I fix almost of the issues. Will upload the v3 soon.

Still have some questions.


Sean Anderson ? 2021/3/23 ??11:06 ??:
>
>
>
> ????if (anatop_reg->supply) {
> ??????? ret = regulator_set_value(anatop_reg->supply, uV + 150000);
> ??????? if (ret)
> ??????????? return ret;
> ????}
>

What is 150000? Is it the min_dropout_uV?

Should I set it to 125000 instead?



>
>
> ????
> ????ret = regulator_set_enable(sreg->supply, true);
> ????if (ret)
> ??????? return ret;
>

Since vin-supply is optional, I change it to

??????? ret = device_get_supply_regulator(dev, "vin-supply",
????????????????????????????????????????? &anatop_reg->supply);
??????? if (!ret) {
??????????????? ret = regulator_set_enable(anatop_reg->supply, true);
??????????????? if (ret)
??????????????????????? return ret;
??????? }

Is this ok?

> > +
> > +??? ret = ofnode_read_u32(dev_ofnode(dev),
>
> > +U_BOOT_DRIVER(anatop_regulator) = {
> > +??? .name = "anatop_regulator",
> > +??? .id = UCLASS_REGULATOR,
> > +??? .ops = &anatop_regulator_ops,
> > +??? .of_match = of_anatop_regulator_match_tbl,
> > +??? .plat_auto = sizeof(struct anatop_regulator),
> > +??? .probe = anatop_regulator_probe,
> > +};
> >


Yours,
Paul