Hi Stephan,
Stephan Gerhold <stephan@xxxxxxxxxxx> writes:
> On Wed, Apr 10, 2024 at 01:41:30PM +0000, Volodymyr Babchuk wrote:
>> Card Detect pin for SHDC2 on SA8155P-ADP is connected to GPIO4 of
>> PMM8155AU_1, not to internal TLMM. This change fixes two issues at
>> once: not working ethernet (because GPIO4 is used for MAC TX) and SD
>> detection.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
>> ---
>> arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>> index 5e4287f8c8cd1..6b08ce246b78c 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
>> @@ -384,7 +384,7 @@ &remoteproc_cdsp {
>> &sdhc_2 {
>> status = "okay";
>>
>> - cd-gpios = <&tlmm 4 GPIO_ACTIVE_LOW>;
>> + cd-gpios = <&pmm8155au_1_gpios 4 GPIO_ACTIVE_LOW>;
>
> Good catch!
Yeah... It took time to understand why ethernet is not working
sometimes. It appeared that there were race between SDHC and MAC
initialization.
>
>> pinctrl-names = "default", "sleep";
>> pinctrl-0 = <&sdc2_on>;
>> pinctrl-1 = <&sdc2_off>;
>
> These two pinctrl configure "gpio96" for "sd-cd-pins". Yet another wrong
> GPIO? Should probably drop that and add proper pinctrl for the actual
> GPIO in the PMIC. Seems like Qualcomm configured the PMIC GPIO with
> pull-up downstream [1] (not sure if this is the right file). It might be
> redundant if there is an external pull-up on the board, but only the
> schematic could tell that for sure.
If I only had schematic for this board... gpio96 puzzles me as well. I
can understand where wrong GPIO4 come from. But gpio96 origin is
completely unclear. I have user manual for the board, it mention
functions of some GPIOs, but there is nothing about gpio96. Anyways, I
removed it from the DTS (see diff below) and I see no issues with SD card.
> FWIW: Looking closer at the broken commit, the regulator voltage setup
> of &sdhc_2 looks suspicious too. Typically one would want a 1.8V capable
> regulator for the vqmmc-supply to properly use ultra high-speed modes,
> but &vreg_l13c_2p96 seems to be configured with 2.504V-2.960V at the
> moment. On downstream it seems to be 1.8V-2.96V [2] (again, not sure if
> this is the right file). I would recommend re-checking this too and
> testing if UHS cards are detected correctly (if you have the board).
This is actually a good catch. I changed the voltage range to 1.8V-2.96V and
now my card detects in UHS/SDR104 mode. Prior to this change it worked only
in HS mode.
> &vreg_l17a_2p96 has the same 2.504V-2.960V, but has 2.704V-2.960V
> downstream [3]. This is close at least, might be fine as-is (but I'm not
> convinced there is a good reason to differ there).
>
Well, I believe that downstream configuration is more correct, but I
can't prove it, so I'll leave it as is.
Diff for additional changes looks like this:
diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index 6b08ce246b78c..b2a3496ff68ad 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>;
};
@@ -386,8 +386,8 @@ &sdhc_2 {
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_on>;
+ pinctrl-1 = <&sdc2_off &pmm8155au_1_sdc2_off>;
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,25 @@ phy-reset-pins {
};
};
};
+
+&pmm8155au_1_gpios {
+ pmm8155au_1_sdc2_on: pmm8155au_1-sdc2-on-state {
+ sd-cd-pin {
+ pins = "gpio4";
+ function = "normal";
+ input-enable;
+ bias-pull-up;
+ power-source = <0>;
+ };
+ };
+
+ pmm8155au_1_sdc2_off: pmm8155au_1-sdc2-off-state {
+ sd-cd-pin {
+ pins = "gpio4";
+ function = "normal";
+ input-enable;
+ bias-pull-up;
+ power-source = <0>;
+ };
+ };
+};
I am planning to send v2 tomorrow.
--
WBR, Volodymyr
