RE: [PATCH 2/2] regulator: core: Ensure we are at least in bounds for our constraints

From: Haibo Chen
Date: Wed Mar 30 2016 - 05:40:57 EST


Hi Brown,

I also meet the similar issue on i.MX6 platforms.

With your patch ---> regulator: core: Ensure we are at least in bounds for our constraints
When I insert an SD3.0 card, shows the following log:

root@imx6qdlsolo:~# [ 59.733941] sdhci-esdhc-imx 2190000.usdhc: could not set regulator OCR (-22)
[ 60.829911] sdhci-esdhc-imx 2190000.usdhc: could not set regulator OCR (-22)
[ 61.917951] sdhci-esdhc-imx 2190000.usdhc: could not set regulator OCR (-22)
[ 63.009498] sdhci-esdhc-imx 2190000.usdhc: could not set regulator OCR (-22)

I did a quick debug, and find when I change the operator && to != , this issue gone.
- if (constraints->min_uV != constraints->max_uV) {
+ if (constraints->min_uV && constraints->max_uV) {


In our sdhci.c, we call the function
regulator_set_voltage ---> regulator_set_voltage_unlocked(struct regulator *regulator, int min_uV, int max_uV)
here, the parameter min_uV is 3300000, and the max_uV is 3400000

currently with your patch (the upper operator is &&), when insert a SD3.0 card,
it will do the sanity check, and return -EINVAL

but when I change the upper operator from && to !=,
before the sanity check, it will first get the current_uV, and then go to out.

I'm not familiar with regulator common code. Hope the upper describe can help you debug this issue.

The following attach our dts piece code.

126 &usdhc1 {
127 pinctrl-names = "default", "state_100mhz", "state_200mhz";
128 pinctrl-0 = <&pinctrl_usdhc1>;
129 pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
130 pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
131 cd-gpios = <&gpio1 19 GPIO_ACTIVE_LOW>;
132 keep-power-in-suspend;
133 wakeup-source;
134 vmmc-supply = <&reg_sd1_vmmc>;
135 status = "okay";
136 };

regulators {
26 compatible = "simple-bus";
27 #address-cells = <1>;
28 #size-cells = <0>;
29
30 reg_sd1_vmmc: sd1_regulator {
31 compatible = "regulator-fixed";
32 regulator-name = "VSD_3V3";
33 regulator-min-microvolt = <3300000>;
34 regulator-max-microvolt = <3300000>;
35 gpio = <&gpio1 9 GPIO_ACTIVE_HIGH>;
36 enable-active-high;
37 };
38 };

Best Regards
Haibo Chen



> -----Original Message-----
> From: linux-mmc-owner@xxxxxxxxxxxxxxx [mailto:linux-mmc-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Mark Brown
> Sent: Wednesday, March 30, 2016 2:27 AM
> To: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Cc: Bjorn Andersson <bjorn@xxxxxxx>; Krzysztof Kozlowski
> <k.kozlowski@xxxxxxxxxxx>; Ivaylo Dimitrov <ivo.g.dimitrov.75@xxxxxxxxx>;
> Liam Girdwood <lgirdwood@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Ulf
> Hansson <ulf.hansson@xxxxxxxxxx>; linux-mmc <linux-mmc@xxxxxxxxxxxxxxx>;
> linux-samsung-soc <linux-samsung-soc@xxxxxxxxxxxxxxx>; Javier Martinez
> Canillas <javier@xxxxxxxxxxxxxxx>; Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/2] regulator: core: Ensure we are at least in bounds for
> our constraints
>
> On Tue, Mar 29, 2016 at 08:05:34PM +0200, Geert Uytterhoeven wrote:
>
> > sh_mobile_sdhi ee100000.sd: Got WP GPIO ======> sh_mobile_sdhi
> > ee100000.sd: could not set regulator OCR (-22)
> > gpio_rcar e6055400.gpio: sense irq = 6, type = 3
> > sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 clock rate
> > 97 MHz
>
> > The line marked with the arrow is introduced by the changed check, and
> > looks to be the origin of the failure.
>
> This isn't making any sense. Why would a change in how we apply voltage
> constraints on initial probe of the regulator have an impact here? The changed
> code shouldn't even be executing at the point where the SDHCI driver is trying
> to use the regulator. There's something else going on here.