[PATCH v1 6/6] arm64: dts: meson: a1: add eMMC controller and its pins

Dmitry Rokosov posted 6 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH v1 6/6] arm64: dts: meson: a1: add eMMC controller and its pins
Posted by Dmitry Rokosov 2 years, 8 months ago
From: Jan Dakinevich <yvdakinevich@sberdevices.ru>

The definition is inspired by a similar one for AXG SoC family.
'sdio_pins' and 'sdio_clk_gate_pins' pinctrls are supposed to be used as
"default" and "clk-gate" in board-specific device trees.

'meson-gx' driver during initialization sets clock to safe low-frequency
value (400kHz). However, both source clocks ("clkin0" and "clkin1") are
high-frequency by default, and using of eMMC's internal divider is not
enough to achieve so low values. To provide low-frequency source,
reparent "sd_emmc_sel2" clock using 'assigned-clocks' property.

Signed-off-by: Jan Dakinevich <yvdakinevich@sberdevices.ru>
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 43 +++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 3eb6aa9c00e0..a25170c61462 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -134,6 +134,32 @@ mux {
 						bias-pull-down;
 					};
 				};
+
+				sdio_pins: sdio {
+					mux-0 {
+						groups = "sdcard_d0_x",
+							 "sdcard_d1_x",
+							 "sdcard_d2_x",
+							 "sdcard_d3_x",
+							 "sdcard_cmd_x";
+						function = "sdcard";
+						bias-pull-up;
+					};
+
+					mux-1 {
+						groups = "sdcard_clk_x";
+						function = "sdcard";
+						bias-disable;
+					};
+				};
+
+				sdio_clk_gate_pins: sdio_clk_gate {
+					mux {
+						groups = "sdcard_clk_x";
+						function = "sdcard";
+						bias-pull-down;
+					};
+				};
 			};
 
 			uart_AO: serial@1c00 {
@@ -200,6 +226,23 @@ usb2_phy1: phy@4000 {
 				#phy-cells = <0>;
 				power-domains = <&pwrc PWRC_USB_ID>;
 			};
+
+			sd_emmc: sd@10000 {
+				compatible = "amlogic,meson-axg-mmc";
+				reg = <0x0 0x10000 0x0 0x800>;
+				interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clkc_periphs CLKID_SD_EMMC_A>,
+					 <&clkc_periphs CLKID_SD_EMMC>,
+					 <&clkc_pll CLKID_FCLK_DIV2>;
+				clock-names = "core",
+					      "clkin0",
+					      "clkin1";
+				assigned-clocks = <&clkc_periphs CLKID_SD_EMMC_SEL2>;
+				assigned-clock-parents = <&xtal>;
+				resets = <&reset RESET_SD_EMMC_A>;
+				power-domains = <&pwrc PWRC_SD_EMMC_ID>;
+				status = "disabled";
+			};
 		};
 
 		gic: interrupt-controller@ff901000 {
-- 
2.36.0
Re: [PATCH v1 6/6] arm64: dts: meson: a1: add eMMC controller and its pins
Posted by Martin Blumenstingl 2 years, 7 months ago
On Wed, Jun 7, 2023 at 10:16 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
>
> From: Jan Dakinevich <yvdakinevich@sberdevices.ru>
>
> The definition is inspired by a similar one for AXG SoC family.
> 'sdio_pins' and 'sdio_clk_gate_pins' pinctrls are supposed to be used as
> "default" and "clk-gate" in board-specific device trees.
Let's wait for Neil's response on the other patch for the question
about pin mux settings

> 'meson-gx' driver during initialization sets clock to safe low-frequency
> value (400kHz). However, both source clocks ("clkin0" and "clkin1") are
> high-frequency by default, and using of eMMC's internal divider is not
> enough to achieve so low values. To provide low-frequency source,
> reparent "sd_emmc_sel2" clock using 'assigned-clocks' property.
Even if the pinctrl part should be postponed then I think it's worth
adding &sd_emmc
Re: [PATCH v1 6/6] arm64: dts: meson: a1: add eMMC controller and its pins
Posted by neil.armstrong@linaro.org 2 years, 7 months ago
Hi,

On 25/06/2023 23:11, Martin Blumenstingl wrote:
> On Wed, Jun 7, 2023 at 10:16 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
>>
>> From: Jan Dakinevich <yvdakinevich@sberdevices.ru>
>>
>> The definition is inspired by a similar one for AXG SoC family.
>> 'sdio_pins' and 'sdio_clk_gate_pins' pinctrls are supposed to be used as
>> "default" and "clk-gate" in board-specific device trees.
> Let's wait for Neil's response on the other patch for the question
> about pin mux settings
> 
>> 'meson-gx' driver during initialization sets clock to safe low-frequency
>> value (400kHz). However, both source clocks ("clkin0" and "clkin1") are
>> high-frequency by default, and using of eMMC's internal divider is not
>> enough to achieve so low values. To provide low-frequency source,
>> reparent "sd_emmc_sel2" clock using 'assigned-clocks' property.
> Even if the pinctrl part should be postponed then I think it's worth
> adding &sd_emmc

Yeah it's weird to add HW definition and to not enable them,
so please enable them in the board if you add them in the DTSI.

Neil
Re: [PATCH v1 6/6] arm64: dts: meson: a1: add eMMC controller and its pins
Posted by Dmitry Rokosov 2 years, 7 months ago
Hello Neil,

Thank you for the review!

On Mon, Jun 26, 2023 at 03:36:23PM +0200, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 25/06/2023 23:11, Martin Blumenstingl wrote:
> > On Wed, Jun 7, 2023 at 10:16 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
> > > 
> > > From: Jan Dakinevich <yvdakinevich@sberdevices.ru>
> > > 
> > > The definition is inspired by a similar one for AXG SoC family.
> > > 'sdio_pins' and 'sdio_clk_gate_pins' pinctrls are supposed to be used as
> > > "default" and "clk-gate" in board-specific device trees.
> > Let's wait for Neil's response on the other patch for the question
> > about pin mux settings
> > 
> > > 'meson-gx' driver during initialization sets clock to safe low-frequency
> > > value (400kHz). However, both source clocks ("clkin0" and "clkin1") are
> > > high-frequency by default, and using of eMMC's internal divider is not
> > > enough to achieve so low values. To provide low-frequency source,
> > > reparent "sd_emmc_sel2" clock using 'assigned-clocks' property.
> > Even if the pinctrl part should be postponed then I think it's worth
> > adding &sd_emmc
> 
> Yeah it's weird to add HW definition and to not enable them,
> so please enable them in the board if you add them in the DTSI.

Unfortunately, I'm unable to provide our internal board DTS. However, I
have an AD401 reference board on hand, so it's possible to test
everything there. I'll include these changes in the next version.

-- 
Thank you,
Dmitry