Re: [PATCH v2] mmc: sunxi-mmc: check ocr_avail on resource request

[Ulf Hansson]

On Wed, 12 Jan 2022 at 10:34, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, Jan 04, 2022 at 02:03:25PM +0800, Michael Wu wrote:
> > Some platforms have no regulator, discrete power devices are used instead.
>
> Is it really the case? vmmc at least should be mandatory so a platform
> not having a regulator would violate the binding itself.
>
> > However, sunxi_mmc_probe does not catch this exception when regulator is
> > absent in DTS. This leads to sd or eMMC init failure.
>
> This will still happen with your patch though?
>
> > To solve this, a fixed vmmc regulator must be hooked up in DTS, like this:
> > reg_dummy_vmmc: dummy_vmmc {
> >       compatible = "regulator-fixed";
> >       regulator-name = "dummy-vmmc";
> >       regulator-min-microvolt = <3300000>;
> >       regulator-max-microvolt = <3300000>;
> > };
> >
> > mmc0:mmc@4020000 {
> >       compatible = "allwinner,sun50i-a100-emmc";
> >       device_type = "mmc0";
> >       vmmc-supply = <&reg_dummy_vmmc>;
> > }
> >
> > In this patch, we print an error message and abort the probe process if
> > the regulator is not specified in DTS.
>
> I'm fine with the patch itself, but it's really not clear to me what
> situation is being fixed or improved here.
>
> You're first mentioning that this is fixing the driver probing even if a
> regulator is absent, but then states (rightfully) that in such a case we
> should use a fixed regulator. So we should always have a regulator then?
>
> I assume that you want the driver to properly error out instead of going
> on if either a regulator is missing or if its voltages are out of range?
>
> If the former, then we should probably check if host->mmc->supply.vmmc
> returned an error. If the latter, then yes, checking ocr_avail is
> probably fine but we should make it clearer in the error message that
> it's what it's about.

Just wanted to mention that I just had a very similar discussion
around another patch [1]. Please have a look at that discussion to get
my opinion on this.

Kind regards
Uffe

[1]
https://patchwork.kernel.org/project/linux-mmc/patch/20211215130711.111186-4-gsomlo@xxxxxxxxx/