On Thu, Apr 11, 2024 at 11:55:55AM +0000, Volodymyr Babchuk wrote: > There are multiple issues with SDHC2 configuration for SA8155P-ADP, > which prevent use of SDHC2 and causes issues with ethernet: > > - Card Detect pin for SHDC2 on SA8155P-ADP is connected to gpio4 of > PMM8155AU_1, not to SoC itself. SoC's gpio4 is used for DWMAC > TX. If sdhc driver probes after dwmac driver, it reconfigures > gpio4 and this breaks Ethernet MAC. > > - pinctrl configuration mentions gpio96 as CD pin. It seems it was > copied from some SM8150 example, because as mentioned above, > correct CD pin is gpio4 on PMM8155AU_1. > > - L13C voltage regulator limits minimal voltage to 2.504V, which > prevents use 1.8V to power SD card, which in turns does not allow > card to work in UHS mode. > > This patch fixes all the mentioned issues. > > Fixes: 0deb2624e2d0 ("arm64: dts: qcom: sa8155p-adp: Add support for uSD card") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx> > > --- > > In v2: > - Added "Fixes:" tag > - CCed stable ML > - Fixed pinctrl configuration > - Extended voltage range for L13C voltage regulator > --- > arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 32 +++++++++++------------- > 1 file changed, 14 insertions(+), 18 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > index 5e4287f8c8cd1..b9d56bda96759 100644 > --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts > @@ -283,7 +283,7 @@ vreg_l12c_1p808: ldo12 { > > vreg_l13c_2p96: ldo13 { > regulator-name = "vreg_l13c_2p96"; > - regulator-min-microvolt = <2504000>; > + regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <2960000>; > regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > }; > @@ -384,10 +384,10 @@ &remoteproc_cdsp { > &sdhc_2 { > status = "okay"; > > - cd-gpios = <&tlmm 4 GPIO_ACTIVE_LOW>; > + cd-gpios = <&pmm8155au_1_gpios 4 GPIO_ACTIVE_LOW>; > pinctrl-names = "default", "sleep"; > - pinctrl-0 = <&sdc2_on>; > - pinctrl-1 = <&sdc2_off>; > + pinctrl-0 = <&sdc2_on &pmm8155au_1_sdc2_cd>; > + pinctrl-1 = <&sdc2_off &pmm8155au_1_sdc2_cd>; > vqmmc-supply = <&vreg_l13c_2p96>; /* IO line power */ > vmmc-supply = <&vreg_l17a_2p96>; /* Card power line */ > bus-width = <4>; > @@ -505,13 +505,6 @@ data-pins { > bias-pull-up; /* pull up */ > drive-strength = <16>; /* 16 MA */ > }; > - > - sd-cd-pins { > - pins = "gpio96"; > - function = "gpio"; > - bias-pull-up; /* pull up */ > - drive-strength = <2>; /* 2 MA */ > - }; > }; > > sdc2_off: sdc2-off-state { > @@ -532,13 +525,6 @@ data-pins { > bias-pull-up; /* pull up */ > drive-strength = <2>; /* 2 MA */ > }; > - > - sd-cd-pins { > - pins = "gpio96"; > - function = "gpio"; > - bias-pull-up; /* pull up */ > - drive-strength = <2>; /* 2 MA */ > - }; > }; > > usb2phy_ac_en1_default: usb2phy-ac-en1-default-state { > @@ -604,3 +590,13 @@ phy-reset-pins { > }; > }; > }; > + > +&pmm8155au_1_gpios { > + pmm8155au_1_sdc2_cd: pmm8155au_1-sdc2-cd { > + pins = "gpio4"; > + function = "normal"; > + input-enable; > + bias-pull-up; > + power-source = <0>; Nitpick: There is one indentation level too much here (remove a tab). Barely worth mentioning, but I guess there will be a v3 to address Krzysztof's comments. :) Thanks, Stephan