ARM: dts: utilite-pro: add mmc card slot support

PATCH New christopher.spinrath at rwth-aachen.de 3 participants 6 messages Jun 12, 2016 – Jun 16, 2016 3 messages not in the archive

Subsystems (2)

Device tree bindings Arm i.MX and Layerscape

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

christopher.spinrath at rwth-aachen.de

author

2016-06-12 23:24 UTC #1 lore raw

PATCH v3 2/2 ARM: dts: utilite-pro: add mmc card slot support

From: Christopher Spinrath <[email protected]>

The Utilite Pro has a mmc card slot connected to the usdhc3 controller. There is no card detection until hardware revision 1.3.

Add support for it and signal the controller with the broken-cd property that polling has to be used to detect a card.

Signed-off-by Christopher Spinrath <[email protected]>

Reviewed-by Fabio Estevam <[email protected]>

---

Notes:
Changes since v2:
- add Fabio's Reviewed-By

Changes since v1:
- enhance commit message to explain to the broken-cd property

 arch/arm/boot/dts/imx6q-utilite-pro.dts | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

arch/arm/boot/dts/imx6q-utilite-pro.dts +44 -0

index 7219745..6199063 100644
@@ -130,6 +130,39 @@
 			MX6QDL_PAD_SD4_DAT6__UART2_CTS_B 0x1b0b1
 		>;
 	};
+
+	pinctrl_usdhc3: usdhc3grp {
+		fsl,pins = <
+			MX6QDL_PAD_SD3_CMD__SD3_CMD	0x17059
+			MX6QDL_PAD_SD3_CLK__SD3_CLK	0x10059
+			MX6QDL_PAD_SD3_DAT0__SD3_DATA0	0x17059
+			MX6QDL_PAD_SD3_DAT1__SD3_DATA1	0x17059
+			MX6QDL_PAD_SD3_DAT2__SD3_DATA2	0x17059
+			MX6QDL_PAD_SD3_DAT3__SD3_DATA3	0x17059
+		>;
+	};
+
+	pinctrl_usdhc3_100mhz: usdhc3grp-100mhz {
+		fsl,pins = <
+			MX6QDL_PAD_SD3_CMD__SD3_CMD	0x170B9
+			MX6QDL_PAD_SD3_CLK__SD3_CLK	0x100B9
+			MX6QDL_PAD_SD3_DAT0__SD3_DATA0	0x170B9
+			MX6QDL_PAD_SD3_DAT1__SD3_DATA1	0x170B9
+			MX6QDL_PAD_SD3_DAT2__SD3_DATA2	0x170B9
+			MX6QDL_PAD_SD3_DAT3__SD3_DATA3	0x170B9
+		>;
+	};
+
+	pinctrl_usdhc3_200mhz: usdhc3grp-200mhz {
+		fsl,pins = <
+			MX6QDL_PAD_SD3_CMD__SD3_CMD	0x170F9
+			MX6QDL_PAD_SD3_CLK__SD3_CLK	0x100F9
+			MX6QDL_PAD_SD3_DAT0__SD3_DATA0	0x170F9
+			MX6QDL_PAD_SD3_DAT1__SD3_DATA1	0x170F9
+			MX6QDL_PAD_SD3_DAT2__SD3_DATA2	0x170F9
+			MX6QDL_PAD_SD3_DAT3__SD3_DATA3	0x170F9
+		>;
+	};
 };
 
 &pcie {

@@ -151,3 +184,14 @@
 	uart-has-rtscts;
 	status = "okay";
 };
+
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	no-1-8-v;
+	broken-cd;
+	keep-power-in-suspend;
+	status = "okay";
+};

Signature · 1 line

2.8.3

Igor Grinberg

2016-06-15 06:40 UTC #2 lore raw

PATCH v3 2/2 ARM: dts: utilite-pro: add mmc card slot support

Hi Christopher,

Quoted 26 lines · christopher.spinrath at rwth-aachen.de

From: Christopher Spinrath <[email protected]>
The Utilite Pro has a mmc card slot connected to the usdhc3 controller. There is no card detection until hardware revision 1.3.
Add support for it and signal the controller with the broken-cd property that polling has to be used to detect a card.
Signed-off-by Christopher Spinrath <[email protected]>
Reviewed-by Fabio Estevam <[email protected]>
---
Notes:
Changes since v2:
- add Fabio's Reviewed-By
Changes since v1:
- enhance commit message to explain to the broken-cd property
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +0 -0
index 7219745..6199063 100644

[...]

arch/arm/boot/dts/imx6q-utilite-pro.dts line 194

Quoted 12 lines

arch/arm/boot/dts/imx6q-utilite-pro.dts +8 -0

@@ -151,3 +184,14 @@
 	uart-has-rtscts;
 	status = "okay";
 };
+
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	no-1-8-v;
+	broken-cd;

A wast majority of boards produced are of revision >=1.3. Can we please have the default as revision 1.3 with cd? And let the patch you have submitted to U-Boot do the job for older revisions?

Quoted 4 lines

arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
+	keep-power-in-suspend;
+	status = "okay";
+};

Signature · 2 lines

Regards,
Igor.

A message that is not in the archive (sent off-list or not yet received): [email protected]

Christopher Spinrath

author

2016-06-15 09:16 UTC #4 lore raw

PATCH v3 2/2 ARM: dts: utilite-pro: add mmc card slot support

Hi Igor,

Quoted 41 lines · Igor Grinberg

Hi Christopher,
Quoted 26 lines · christopher.spinrath at rwth-aachen.de
From: Christopher Spinrath <[email protected]>
The Utilite Pro has a mmc card slot connected to the usdhc3 controller. There is no card detection until hardware revision 1.3.
Add support for it and signal the controller with the broken-cd property that polling has to be used to detect a card.
Signed-off-by Christopher Spinrath <[email protected]>
Reviewed-by Fabio Estevam <[email protected]>
---
Notes:
Changes since v2:
- add Fabio's Reviewed-By
Changes since v1:
- enhance commit message to explain to the broken-cd property
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +0 -0
index 7219745..6199063 100644
[...]
Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +5 -0
@@ -151,3 +184,14 @@
 	uart-has-rtscts;
 	status = "okay";
 };
+
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;

Quoted 9 lines

arch/arm/boot/dts/imx6q-utilite-pro.dts line 194
Quoted 3 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	no-1-8-v;
+	broken-cd;
A wast majority of boards produced are of revision >=1.3. Can we please have the default as revision 1.3 with cd? And let the patch you have submitted to U-Boot do the job for older revisions?

Well, my board has revision 1.0. So I cannot test that and feel uneasy to put my Signed-off-by under such a patch. IMHO the best solution would be that someone with a revision >= 1.3 board sends a follow-up patch adding the cd-gpios.

If I resend the patch with cd-gpios could you test it and provide a Tested-By?

The other question is: should the dts provide a working/sane configuration for all boards (which the broken-cd approach is) independently of the bootloader? Is it ok to put both, the cd-gpios and the broken-cd property into the dts and let the bootloader remove the broken-cd property for revision >= 1.3 (due to the documentation it is not but the driver favourites the broken cd property - hence, it would work)?

Thanks,
Christopher

Quoted 5 lines

Quoted 4 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
+	keep-power-in-suspend;
+	status = "okay";
+};

Igor Grinberg

2016-06-15 10:33 UTC #5 lore raw

PATCH v3 2/2 ARM: dts: utilite-pro: add mmc card slot support

Hi Christopher,

Quoted 54 lines · Christopher Spinrath

Hi Igor,
Quoted 41 lines · Igor Grinberg
Hi Christopher,
Quoted 26 lines · christopher.spinrath at rwth-aachen.de
From: Christopher Spinrath <[email protected]>
The Utilite Pro has a mmc card slot connected to the usdhc3 controller. There is no card detection until hardware revision 1.3.
Add support for it and signal the controller with the broken-cd property that polling has to be used to detect a card.
Signed-off-by Christopher Spinrath <[email protected]>
Reviewed-by Fabio Estevam <[email protected]>
---
Notes:
Changes since v2:
- add Fabio's Reviewed-By
Changes since v1:
- enhance commit message to explain to the broken-cd property
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +0 -0
index 7219745..6199063 100644
[...]
Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +5 -0
@@ -151,3 +184,14 @@
 	uart-has-rtscts;
 	status = "okay";
 };
+
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts line 194
Quoted 3 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	no-1-8-v;
+	broken-cd;
A wast majority of boards produced are of revision >=1.3. Can we please have the default as revision 1.3 with cd? And let the patch you have submitted to U-Boot do the job for older revisions?

Quoted 7 lines

Well, my board has revision 1.0. So I cannot test that and feel uneasy to put my Signed-off-by under such a patch. IMHO the best solution would be that someone with a revision >= 1.3 board sends a follow-up patch adding the cd-gpios.
If I resend the patch with cd-gpios could you test it and provide a Tested-By?

Yes. Absolutely.

Quoted 4 lines

The other question is: should the dts provide a working/sane configuration for all boards (which the broken-cd approach is) independently of the bootloader?

Well, no, I wouldn't go that way, as "why wouldn't we just use the broken-cd approach on all boards and not bother with gpio-cds"... We do want the gpio-cd to work as expected where it can.

Quoted 5 lines

Is it ok to put both, the cd-gpios and
the broken-cd property into the dts and let the bootloader remove the broken-cd property for revision >= 1.3 (due to the documentation it is not but the driver favourites the broken cd property - hence, it would work)?

That sounds sane from my POV (unless I'm missing something), but will have to have comments explaining what is going on.

There are two main reasons why I would not want to have the broken-cd as a default:
1) It is not broken... It just does not exist prior to revision 1.3.
2) There are more boards of revision >=1.3 then older ones outside and we would want them to work as expected even with older U-Boot versions.

Signature · 2 lines

Regards,
Igor.

A message that is not in the archive (sent off-list or not yet received): [email protected]

Christopher Spinrath

author

2016-06-15 14:40 UTC #7 lore raw

PATCH v3 2/2 ARM: dts: utilite-pro: add mmc card slot support

Hi Shawn,

Quoted 62 lines · Igor Grinberg

Hi Christopher,
Quoted 59 lines · Christopher Spinrath
Hi Igor,
Quoted 41 lines · Igor Grinberg
Hi Christopher,
Quoted 26 lines · christopher.spinrath at rwth-aachen.de
From: Christopher Spinrath <[email protected]>
The Utilite Pro has a mmc card slot connected to the usdhc3 controller. There is no card detection until hardware revision 1.3.
Add support for it and signal the controller with the broken-cd property that polling has to be used to detect a card.
Signed-off-by Christopher Spinrath <[email protected]>
Reviewed-by Fabio Estevam <[email protected]>
---
Notes:
Changes since v2:
- add Fabio's Reviewed-By
Changes since v1:
- enhance commit message to explain to the broken-cd property
 arch/arm/boot/dts/imx6q-utilite-pro.dts | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
arch/arm/boot/dts/imx6q-utilite-pro.dts +0 -0
index 7219745..6199063 100644
[...]
Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +5 -0
@@ -151,3 +184,14 @@
 	uart-has-rtscts;
 	status = "okay";
 };
+
+&usdhc3 {
+	pinctrl-names = "default", "state_100mhz", "state_200mhz";
+	pinctrl-0 = <&pinctrl_usdhc3>;
+	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
Quoted 9 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts line 194
Quoted 3 lines
arch/arm/boot/dts/imx6q-utilite-pro.dts +3 -0
+	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
+	no-1-8-v;
+	broken-cd;
A wast majority of boards produced are of revision >=1.3. Can we please have the default as revision 1.3 with cd? And let the patch you have submitted to U-Boot do the job for older revisions?
Well, my board has revision 1.0. So I cannot test that and feel uneasy to put my Signed-off-by under such a patch. IMHO the best solution would be that someone with a revision >= 1.3 board sends a follow-up patch adding the cd-gpios.

Quoted 5 lines

Quoted 2 lines
If I resend the patch with cd-gpios could you test it and provide a Tested-By?
Yes. Absolutely.

Can we have your opinion on that matter?

Quoted 24 lines

Quoted 4 lines
The other question is: should the dts provide a working/sane configuration for all boards (which the broken-cd approach is) independently of the bootloader?
Well, no, I wouldn't go that way, as "why wouldn't we just use the broken-cd approach on all boards and not bother with gpio-cds"... We do want the gpio-cd to work as expected where it can.
Quoted 5 lines
Is it ok to put both, the cd-gpios and
the broken-cd property into the dts and let the bootloader remove the broken-cd property for revision >= 1.3 (due to the documentation it is not but the driver favourites the broken cd property - hence, it would work)?
That sounds sane from my POV (unless I'm missing something), but will have to have comments explaining what is going on.
There are two main reasons why I would not want to have the broken-cd as a default:
1) It is not broken... It just does not exist prior to revision 1.3.
2) There are more boards of revision >=1.3 then older ones outside and we would want them to work as expected even with older U-Boot versions.

Shawn Guo

2016-06-16 01:42 UTC #8 lore raw

PATCH v3 2/2 ARM: dts: utilite-pro: add mmc card slot support

Quoted 4 lines · Christopher Spinrath

Quoted 4 lines
Quoted 4 lines
Well, my board has revision 1.0. So I cannot test that and feel uneasy to put my Signed-off-by under such a patch. IMHO the best solution would be that someone with a revision >= 1.3 board sends a follow-up patch adding the cd-gpios.

This approach seems reasonable to me, so I just went ahead to apply the patch, and wait a follow-up patch from someone who can test on 1.3 board.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
If I resend the patch with cd-gpios could you test it and provide a Tested-By?
Yes. Absolutely.
Can we have your opinion on that matter?

Shawn