[PATCH v2 1/3] dt-bindings: clock: gcc-sdm660: Add missing SDCC resets

[PATCH v2 1/3] dt-bindings: clock: gcc-sdm660: Add missing SDCC resets

[Alexey Minnekhanov]

Add resets for eMMC/SD card blocks that were missed during initial
driver submission.

Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
Fixes: f2a76a2955c0 ("clk: qcom: Add Global Clock controller (GCC) driver for SDM660")
---
 include/dt-bindings/clock/qcom,gcc-sdm660.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,gcc-sdm660.h b/include/dt-bindings/clock/qcom,gcc-sdm660.h
index df8a6f3d367e..74c22f67da21 100644
--- a/include/dt-bindings/clock/qcom,gcc-sdm660.h
+++ b/include/dt-bindings/clock/qcom,gcc-sdm660.h
@@ -153,5 +153,7 @@
 #define GCC_USB_30_BCR			7
 #define GCC_USB_PHY_CFG_AHB2PHY_BCR	8
 #define GCC_MSS_RESTART			9
+#define GCC_SDCC1_BCR			10
+#define GCC_SDCC2_BCR			11
 
 #endif
-- 
2.45.3

[PATCH v2 2/3] clk: qcom: gcc-sdm660: Add missing SDCC block resets

[Alexey Minnekhanov]

This will allow linux to properly reset eMMC/SD blocks.

Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
Fixes: f2a76a2955c0 ("clk: qcom: Add Global Clock controller (GCC) driver for SDM660")
---
 drivers/clk/qcom/gcc-sdm660.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/qcom/gcc-sdm660.c b/drivers/clk/qcom/gcc-sdm660.c
index df79298a1a25..01a76f1b5b4c 100644
--- a/drivers/clk/qcom/gcc-sdm660.c
+++ b/drivers/clk/qcom/gcc-sdm660.c
@@ -2420,6 +2420,8 @@ static struct gdsc *gcc_sdm660_gdscs[] = {
 static const struct qcom_reset_map gcc_sdm660_resets[] = {
 	[GCC_QUSB2PHY_PRIM_BCR] = { 0x12000 },
 	[GCC_QUSB2PHY_SEC_BCR] = { 0x12004 },
+	[GCC_SDCC2_BCR] = { 0x14000 },
+	[GCC_SDCC1_BCR] = { 0x16000 },
 	[GCC_UFS_BCR] = { 0x75000 },
 	[GCC_USB3_DP_PHY_BCR] = { 0x50028 },
 	[GCC_USB3_PHY_BCR] = { 0x50020 },
-- 
2.45.3

Re: [PATCH v2 2/3] clk: qcom: gcc-sdm660: Add missing SDCC block resets

[Konrad Dybcio]

On 29.01.2025 10:23 PM, Alexey Minnekhanov wrote:
> This will allow linux to properly reset eMMC/SD blocks.
> 
> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
> Fixes: f2a76a2955c0 ("clk: qcom: Add Global Clock controller (GCC) driver for SDM660")
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

[PATCH v2 3/3] arm64: dts: qcom: sdm630: Add missing resets to mmc blocks

[Alexey Minnekhanov]

Add resets to eMMC/SD card blocks so linux can properly reset
them during initialization.

Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
---
 arch/arm64/boot/dts/qcom/sdm630.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index a2c079bac1a7..3722e405a97c 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -1379,6 +1379,7 @@ sdhc_2: mmc@c084000 {
 					<&xo_board>;
 			clock-names = "iface", "core", "xo";
 
+			resets = <&gcc GCC_SDCC2_BCR>;
 
 			interconnects = <&a2noc 3 &a2noc 10>,
 					<&gnoc 0 &cnoc 28>;
@@ -1433,6 +1434,8 @@ sdhc_1: mmc@c0c4000 {
 				 <&gcc GCC_SDCC1_ICE_CORE_CLK>;
 			clock-names = "iface", "core", "xo", "ice";
 
+			resets = <&gcc GCC_SDCC1_BCR>;
+
 			interconnects = <&a2noc 2 &a2noc 10>,
 					<&gnoc 0 &cnoc 27>;
 			interconnect-names = "sdhc-ddr", "cpu-sdhc";
-- 
2.45.3

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm630: Add missing resets to mmc blocks

[Konrad Dybcio]

On 29.01.2025 10:23 PM, Alexey Minnekhanov wrote:
> Add resets to eMMC/SD card blocks so linux can properly reset
> them during initialization.
> 
> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
> ---

Thanks!

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2 1/3] dt-bindings: clock: gcc-sdm660: Add missing SDCC resets

[Krzysztof Kozlowski]

On Thu, Jan 30, 2025 at 12:23:25AM +0300, Alexey Minnekhanov wrote:
> Add resets for eMMC/SD card blocks that were missed during initial
> driver submission.
> 
> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
> Fixes: f2a76a2955c0 ("clk: qcom: Add Global Clock controller (GCC) driver for SDM660")

Your SoB is always the last tag, but repeating what I asked during
previous review - what sort of bug is here? Missing feature is not a bug
per se.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: clock: gcc-sdm660: Add missing SDCC resets

[Alexey Minnekhanov]

On 1/30/25 11:35 AM, Krzysztof Kozlowski wrote:
> On Thu, Jan 30, 2025 at 12:23:25AM +0300, Alexey Minnekhanov wrote:
>> Add resets for eMMC/SD card blocks that were missed during initial
>> driver submission.
>>
>> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
>> Fixes: f2a76a2955c0 ("clk: qcom: Add Global Clock controller (GCC) driver for SDM660")
> 
> Your SoB is always the last tag, but repeating what I asked during
> previous review - what sort of bug is here? Missing feature is not a bug
> per se.
> 
> Best regards,
> Krzysztof
> 

I was hesitant about adding fixes tag myself, because there is no any 
user-observable bug being fixed here. In fact the behavior is unchanged, 
eMMC and SD worked fine before this and still work fine with this patch.
 From Linux side this is mostly for consistency with other 
younger/sibling SoCs, like MSM8998 & 8996 all have these resets 
specified. This will probably be much more useful for U-Boot port.

Perhaps fixes tag should be removed completely?

--
Regards,
Alexey

Re: [PATCH v2 1/3] dt-bindings: clock: gcc-sdm660: Add missing SDCC resets

[Krzysztof Kozlowski]

On 30/01/2025 12:29, Alexey Minnekhanov wrote:
> On 1/30/25 11:35 AM, Krzysztof Kozlowski wrote:
>> On Thu, Jan 30, 2025 at 12:23:25AM +0300, Alexey Minnekhanov wrote:
>>> Add resets for eMMC/SD card blocks that were missed during initial
>>> driver submission.
>>>
>>> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
>>> Fixes: f2a76a2955c0 ("clk: qcom: Add Global Clock controller (GCC) driver for SDM660")
>>
>> Your SoB is always the last tag, but repeating what I asked during
>> previous review - what sort of bug is here? Missing feature is not a bug
>> per se.
>>
>> Best regards,
>> Krzysztof
>>
> 
> I was hesitant about adding fixes tag myself, because there is no any 
> user-observable bug being fixed here. In fact the behavior is unchanged, 
> eMMC and SD worked fine before this and still work fine with this patch.
>  From Linux side this is mostly for consistency with other 
> younger/sibling SoCs, like MSM8998 & 8996 all have these resets 
> specified. This will probably be much more useful for U-Boot port.
> 
> Perhaps fixes tag should be removed completely?

Yes, drop the Fixes tag.

Best regards,
Krzysztof