mmc: Add support for marking hpi as broken through devicetree
PATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time);
There seems to be no way to reliable identify eMMC-s which have a broken hpi implementation, but at least for eMMC's which are soldered onto a board we can work around this by specifying that hpi is broken in devicetree.
---
Changes in v2:
-Fix of_node leakage
--- Documentation/devicetree/bindings/mmc/mmc-card.txt | 31 ++++++++++++++++++++++ drivers/mmc/core/mmc.c | 10 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/mmc-card.txt
Documentation/devicetree/bindings/mmc/mmc-card.txt new file +31 -0
@@ -0,0 +1,31 @@ +mmc-card / eMMC bindings +------------------------ + +This documents describes the devicetree bindings for a mmc-host controller +child node describing a mmc-card / an eMMC, see "Use of Function subnodes" +in mmc.txt + +Required properties: +-compatible : Must be "mmc-card" +-reg : Must be <0> + +Optional properties: +-broken-hpi : Use this to indicate that the mmc-card has a broken hpi + implementation, and that hpi should not be used + +Example: + +&mmc2 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc2_pins_a>; + vmmc-supply = <®_vcc3v3>; + bus-width = <8>; + non-removable; + status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; +};
drivers/mmc/core/mmc.c +9 -1
@@ -11,6 +11,7 @@*/ #include <linux/err.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/stat.h> #include <linux/pm_runtime.h>
@@ -336,6 +337,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) {int err =0 , idx;unsigned int part_size; +struct device_node *np; +bool broken_hpi = false;/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */ card->ext_csd.raw_ext_csd_structure = ext_csd[EXT_CSD_STRUCTURE ];
@@ -349,6 +352,11 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) } } + np =mmc_of_find_child_device (card->host,0 ); +if (np &&of_device_is_compatible (np,"mmc-card" )) + broken_hpi =of_property_read_bool (np,"broken-hpi" ); +of_node_put (np); +/* * The EXT_CSD format is meant to be forward compatible. As long * as CSD_STRUCTURE does not change, all values for EXT_CSD_REV
@@ -494,7 +502,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) }/* check whether the eMMC card supports HPI */ -if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 ) { +if (!broken_hpi && (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 )) { card->ext_csd.hpi =1 ;if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x2 ) card->ext_csd.hpi_cmd =MMC_STOP_TRANSMISSION ;
Signature · 1 line
2.3.4
PATCH v2 2/2 ARM: dts: sun5i: Add broken-hpi property for Utoo-P66 eMMC
The eMMC on the A13 based Utoo-P66 tablet does not properly support hpi, and trying to enable it results in the eMMC not working, so add a child-node describing the eMMC, and set the broken-hpi property on it.
---
Changes in v2:
-Fix mmmcard typo
--- arch/arm/boot/dts/sun5i-a13-utoo-p66.dts | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm/boot/dts/sun5i-a13-utoo-p66.dts +6 -0
@@ -135,6 +135,12 @@ bus-width = <8>; non-removable; status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; }; &pio {
Signature · 1 line
2.3.4
Maxime Ripard
AppliedPATCH v2 2/2 ARM: dts: sun5i: Add broken-hpi property for Utoo-P66 eMMC
Quoted 5 lines · Hans de Goede
The eMMC on the A13 based Utoo-P66 tablet does not properly support hpi, and trying to enable it results in the eMMC not working, so add a child-node describing the eMMC, and set the broken-hpi property on it.
Signed-off-by Hans de Goede <[email protected]>
Applied, thanks!
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150402/c938966b/attachment.sig>
Ulf Hansson
AppliedPATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
In reply to Hans de GoedeQuoted 7 lines · Hans de Goede
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time);
Quoted 5 lines
There seems to be no way to reliable identify eMMC-s which have a broken hpi implementation, but at least for eMMC's which are soldered onto a board we can work around this by specifying that hpi is broken in devicetree.
Signed-off-by Hans de Goede <[email protected]>
Thanks! Applied for next.
Kind regards
Uffe
Quoted 91 lines
---
Changes in v2:(quoted diff) +0
-2-Fix of_node leakage --- Documentation/devicetree/bindings/mmc/mmc-card.txt | 31 ++++++++++++++++++++++ drivers/mmc/core/mmc.c | 10 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/mmc-card.txtDocumentation/devicetree/bindings/mmc/mmc-card.txt new file +31
-0@@ -0,0 +1,31 @@ +mmc-card / eMMC bindings +------------------------ + +This documents describes the devicetree bindings for a mmc-host controller +child node describing a mmc-card / an eMMC, see "Use of Function subnodes" +in mmc.txt + +Required properties: +-compatible : Must be "mmc-card" +-reg : Must be <0> + +Optional properties: +-broken-hpi : Use this to indicate that the mmc-card has a broken hpi + implementation, and that hpi should not be used + +Example: + +&mmc2 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc2_pins_a>; + vmmc-supply = <®_vcc3v3>; + bus-width = <8>; + non-removable; + status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; +};drivers/mmc/core/mmc.c +9
-2@@ -11,6 +11,7 @@*/ #include <linux/err.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/stat.h> #include <linux/pm_runtime.h> @@ -336,6 +337,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) {int err =0 , idx;unsigned int part_size; +struct device_node *np; +bool broken_hpi = false;/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */ card->ext_csd.raw_ext_csd_structure = ext_csd[EXT_CSD_STRUCTURE ];@@ -349,6 +352,11 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) } } + np =mmc_of_find_child_device (card->host,0 ); +if (np &&of_device_is_compatible (np,"mmc-card" )) + broken_hpi =of_property_read_bool (np,"broken-hpi" ); +of_node_put (np); +/* * The EXT_CSD format is meant to be forward compatible. As long * as CSD_STRUCTURE does not change, all values for EXT_CSD_REV @@ -494,7 +502,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) }/* check whether the eMMC card supports HPI */ -if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 ) { +if (!broken_hpi && (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 )) { card->ext_csd.hpi =1 ;if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x2 ) card->ext_csd.hpi_cmd =MMC_STOP_TRANSMISSION ; --2.3.4
PATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
In reply to Hans de GoedeHey Hans,
Quoted 10 lines · Hans de Goede
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card,
EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HPI_MGMT, 1,card->ext_csd.generic_cmd6_time);There seems to be no way to reliable identify eMMC-s which have a broken hpi implementation, but at least for eMMC's which are soldered onto a board we can work around this by specifying that hpi is broken in devicetree.
We've been talking about this a little off-list, and I was just triggered again by this so I'm following up on it again.
The same issue bit me in the ass on a Micron 'industrial' grade eMMC (MTFC4GACAANA) and luckily you found this back then to help me.
As we discussed, this may very well be a limitation of the mmc controller, rather then the MMC chip. While we only have a sample size of 2 on 2 different socs (with likely the same mmc IP) (A13/A20) I found in the JEDEC spec in the appendix on changes from 4.4 to 4.41:
Introduce of High Priority Interrupt mechanism, HPI background and one of possible solutions.
Which leaves me to believe, that HPI was added to eMMC in version 4.41 and was not available before. Both the A13 and A20 user manuals state that the MMC controller in these socs is only version 4.3.
Obviously I know far to little as to what does and what does not work with an MMC-host, since it's just a simple data-bus, but I would not be surprised if these things are somewhere somewhat related, so hopefully someone on this list can give their opinion on this.
I already checked the core/mmc.c where the version is read from the mmc controller to see if we can trigger on this based on version, however it seems only the major version number is stored here [0]?
So in my opinion, it is quite likely that this setting be moved up to the mmc-core level, for the SoC, rather then the card itself?
[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L148
Quoted 90 lines
Signed-off-by Hans de Goede <[email protected]>---
Changes in v2:(quoted diff) +0
-2-Fix of_node leakage --- Documentation/devicetree/bindings/mmc/mmc-card.txt | 31 ++++++++++++++++++++++ drivers/mmc/core/mmc.c | 10 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/mmc-card.txtDocumentation/devicetree/bindings/mmc/mmc-card.txt new file +31
-0@@ -0,0 +1,31 @@ +mmc-card / eMMC bindings +------------------------ + +This documents describes the devicetree bindings for a mmc-host controller +child node describing a mmc-card / an eMMC, see "Use of Function subnodes" +in mmc.txt + +Required properties: +-compatible : Must be "mmc-card" +-reg : Must be <0> + +Optional properties: +-broken-hpi : Use this to indicate that the mmc-card has a broken hpi + implementation, and that hpi should not be used + +Example: + +&mmc2 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc2_pins_a>; + vmmc-supply = <®_vcc3v3>; + bus-width = <8>; + non-removable; + status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; +};drivers/mmc/core/mmc.c +9
-1@@ -11,6 +11,7 @@*/ #include <linux/err.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/stat.h> #include <linux/pm_runtime.h> @@ -336,6 +337,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) {int err =0 , idx;unsigned int part_size; +struct device_node *np; +bool broken_hpi = false;/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */ card->ext_csd.raw_ext_csd_structure = ext_csd[EXT_CSD_STRUCTURE ];@@ -349,6 +352,11 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) } } + np =mmc_of_find_child_device (card->host,0 ); +if (np &&of_device_is_compatible (np,"mmc-card" )) + broken_hpi =of_property_read_bool (np,"broken-hpi" ); +of_node_put (np); +/* * The EXT_CSD format is meant to be forward compatible. As long * as CSD_STRUCTURE does not change, all values for EXT_CSD_REV @@ -494,7 +502,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) }/* check whether the eMMC card supports HPI */ -if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 ) { +if (!broken_hpi && (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 )) { card->ext_csd.hpi =1 ;if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x2 ) card->ext_csd.hpi_cmd =MMC_STOP_TRANSMISSION ;
PATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
Hi,
Quoted 20 lines · Olliver Schinagl
Hey Hans,
Quoted 10 lines · Hans de Goede
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time);There seems to be no way to reliable identify eMMC-s which have a broken hpi implementation, but at least for eMMC's which are soldered onto a board we can work around this by specifying that hpi is broken in devicetree.
We've been talking about this a little off-list, and I was just triggered again by this so I'm following up on it again.
The same issue bit me in the ass on a Micron 'industrial' grade eMMC (MTFC4GACAANA) and luckily you found this back then to help me.
As we discussed, this may very well be a limitation of the mmc controller, rather then the MMC chip. While we only have a sample size of 2 on 2 different socs (with likely the same mmc IP) (A13/A20) I found in the JEDEC spec in the appendix on changes from 4.4 to 4.41:
Introduce of High Priority Interrupt mechanism, HPI background and one of possible solutions.
Quoted 7 lines
Which leaves me to believe, that HPI was added to eMMC in version 4.41 and was not available before. Both the A13 and A20 user manuals state that the MMC controller in these socs is only version 4.3.
Obviously I know far to little as to what does and what does not work with an MMC-host, since it's just a simple data-bus, but I would not be surprised if these things are somewhere somewhat related, so hopefully someone on this list can give their opinion on this.
I already checked the
core/mmc.cwhere the version is read from the mmc controller to see if we can trigger on this based on version, however it seems only the major version number is stored here [0]?So in my opinion, it is quite likely that this setting be moved up to the mmc-core level, for the SoC, rather then the card itself?
Yes I was talking to Tsvetan from Olimex and he says they have seen the problem with yet another emmc, so this indeed seems to be a sunxi SoC specific problem, and we just need to disable hpi on all sunxi SoC's.
Do you have time to look into doing a kernel patch for this ?
I'm thinking adding a host-cap called MMC_CAP_BROKEN_HPI and adding support for that to drivers/mmc/core/host.c: mmc_of_parse() and extending the current card check for broken-hpi to also check host->caps
And then we need to decide whether to just hard set that cap in drivers/mmc/host/sunxi_mmc.c or if we are going to add it to our dtsi files. I think we should probably just hard-code it in drivers/mmc/host/sunxi_mmc.c.
Regards,
Hans
Quoted 89 lines
[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L148
Quoted 86 lines
Signed-off-by Hans de Goede <[email protected]>---
Changes in v2:(quoted diff) +0
-2-Fix of_node leakage --- Documentation/devicetree/bindings/mmc/mmc-card.txt | 31 ++++++++++++++++++++++ drivers/mmc/core/mmc.c | 10 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/mmc-card.txtDocumentation/devicetree/bindings/mmc/mmc-card.txt new file +31
-0@@ -0,0 +1,31 @@ +mmc-card / eMMC bindings +------------------------ + +This documents describes the devicetree bindings for a mmc-host controller +child node describing a mmc-card / an eMMC, see "Use of Function subnodes" +in mmc.txt + +Required properties: +-compatible : Must be "mmc-card" +-reg : Must be <0> + +Optional properties: +-broken-hpi : Use this to indicate that the mmc-card has a broken hpi + implementation, and that hpi should not be used + +Example: + +&mmc2 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc2_pins_a>; + vmmc-supply = <®_vcc3v3>; + bus-width = <8>; + non-removable; + status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; +};drivers/mmc/core/mmc.c +9
-1@@ -11,6 +11,7 @@*/ #include <linux/err.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/stat.h> #include <linux/pm_runtime.h> @@ -336,6 +337,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) {int err =0 , idx;unsigned int part_size; +struct device_node *np; +bool broken_hpi = false;/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */ card->ext_csd.raw_ext_csd_structure = ext_csd[EXT_CSD_STRUCTURE ];@@ -349,6 +352,11 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) } } + np =mmc_of_find_child_device (card->host,0 ); +if (np &&of_device_is_compatible (np,"mmc-card" )) + broken_hpi =of_property_read_bool (np,"broken-hpi" ); +of_node_put (np); +/* * The EXT_CSD format is meant to be forward compatible. As long * as CSD_STRUCTURE does not change, all values for EXT_CSD_REV @@ -494,7 +502,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) }/* check whether the eMMC card supports HPI */ -if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 ) { +if (!broken_hpi && (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 )) { card->ext_csd.hpi =1 ;if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x2 ) card->ext_csd.hpi_cmd =MMC_STOP_TRANSMISSION ;
PATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
Hey Hans,
Quoted 60 lines · Hans de Goede
Hi,
Quoted 37 lines · Olliver Schinagl
Hey Hans,
Quoted 7 lines · Hans de Goede
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time);Quoted 5 lines
There seems to be no way to reliable identify eMMC-s which have a broken
hpi implementation, but at least for eMMC's which are soldered onto a board
we can work around this by specifying that hpi is broken in devicetree.We've been talking about this a little off-list, and I was just triggered again by this so I'm following up on it again.
The same issue bit me in the ass on a Micron 'industrial' grade eMMC (MTFC4GACAANA) and luckily you found this back then to help me.
As we discussed, this may very well be a limitation of the mmc controller, rather then the MMC chip. While we only have a sample size of 2 on 2 different socs (with likely the same mmc IP) (A13/A20) I found in the JEDEC spec in the appendix on changes from 4.4 to 4.41:
Introduce of High Priority Interrupt mechanism, HPI background and one of possible solutions.Which leaves me to believe, that HPI was added to eMMC in version 4.41 and was not available before. Both the A13 and A20 user manuals state that the MMC controller in these socs is only version 4.3.
Obviously I know far to little as to what does and what does not work with an MMC-host, since it's just a simple data-bus, but I would not be surprised if these things are somewhere somewhat related, so hopefully someone on this list can give their opinion on this.
Quoted 6 lines
I already checked the
core/mmc.cwhere the version is read from the mmc controller to see if we can trigger on this based on version, however it seems only the major version number is stored here [0]?So in my opinion, it is quite likely that this setting be moved up to the mmc-core level, for the SoC, rather then the card itself?
Yes I was talking to Tsvetan from Olimex and he says they have seen the problem with yet
another emmc, so this indeed seems to be a sunxi SoC specific problem, and we just need to
disable hpi on all sunxi SoC's.Do you have time to look into doing a kernel patch for this ?
I'm thinking adding a host-cap called
MMC_CAP_BROKEN_HPIand adding support for that todrivers/mmc/core/host.c:mmc_of_parse()and extending the current card check for broken-hpi to also checkhost->caps
Quoted 4 lines
And then we need to decide whether to just hard set that cap in
drivers/mmc/host/sunxi_mmc.cor if we are going to add it to our dtsi files. I think we should probably just hard-code it indrivers/mmc/host/sunxi_mmc.c.
Yeah I'll see if I can look into that.
I was also looking if we really cannot detect older MMC versions, and I think we can somewhat. I haven't figured out the details yet though.
The EXT_CSD_REV field does yield a version number matching the various 4 sub-versions, so for now I'm thinking of simply disabeling HPI for anything before 4.41 (unless maybe overriden). Since before 4.41 it shouldn't have been supported. Maybe add an override that allows use of HPI? but then we have the whole backwards compatibility thing.
[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L368
Quoted 108 lines
Regards,
Hans
Quoted 97 lines
[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L148
Quoted 94 lines
Signed-off-by Hans de Goede <[email protected]>---
Changes in v2:(quoted diff) +1
-2-Fix of_node leakage --- Documentation/devicetree/bindings/mmc/mmc-card.txt | 31 ++++++++++++++++++++++ drivers/mmc/core/mmc.c | 10 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mmc/mmc-card.txtDocumentation/devicetree/bindings/mmc/mmc-card.txt new file +4
-0@@ -0,0 +1,31 @@ +mmc-card / eMMC bindings +------------------------ + +This documents describes the devicetree bindings for a mmc-hostcontroller
+child node describing a mmc-card / an eMMC, see "Use of Function subnodes"Documentation/devicetree/bindings/mmc/mmc-card.txt new file +26
-0+in mmc.txt + +Required properties: +-compatible : Must be "mmc-card" +-reg : Must be <0> + +Optional properties: +-broken-hpi : Use this to indicate that the mmc-card has a broken hpi + implementation, and that hpi should not be used + +Example: + +&mmc2 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc2_pins_a>; + vmmc-supply = <®_vcc3v3>; + bus-width = <8>; + non-removable; + status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; +};drivers/mmc/core/mmc.c +3
-0@@ -11,6 +11,7 @@*/ #include <linux/err.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/stat.h> #include <linux/pm_runtime.h> @@ -336,6 +337,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) {int err =0 , idx;unsigned int part_size; +struct device_node *np; +bool broken_hpi = false;/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
card->ext_csd.raw_ext_csd_structure = ext_csd[EXT_CSD_STRUCTURE];drivers/mmc/core/mmc.c +6
-1@@ -349,6 +352,11 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) } } + np =mmc_of_find_child_device (card->host,0 ); +if (np &&of_device_is_compatible (np,"mmc-card" )) + broken_hpi =of_property_read_bool (np,"broken-hpi" ); +of_node_put (np); +/* * The EXT_CSD format is meant to be forward compatible. As long * as CSD_STRUCTURE does not change, all values for EXT_CSD_REV @@ -494,7 +502,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) }/* check whether the eMMC card supports HPI */ -if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 ) { +if (!broken_hpi && (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 )) { card->ext_csd.hpi =1 ;if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x2 ) card->ext_csd.hpi_cmd =MMC_STOP_TRANSMISSION ;
PATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
Hey Hans,
Quoted 76 lines · Olliver Schinagl
Hey Hans,
Quoted 63 lines · Hans de Goede
Hi,
Quoted 40 lines · Olliver Schinagl
Hey Hans,
Quoted 7 lines · Hans de Goede
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time);Quoted 6 lines
There seems to be no way to reliable identify eMMC-s which have a broken
hpi implementation, but at least for eMMC's which are soldered onto a board
we can work around this by specifying that hpi is broken in devicetree.We've been talking about this a little off-list, and I was just triggered again by this so I'm following up on it again.
The same issue bit me in the ass on a Micron 'industrial' grade eMMC (MTFC4GACAANA) and luckily you found this back then to help me.
As we discussed, this may very well be a limitation of the mmc controller, rather then the MMC chip. While we only have a sample size of 2 on 2 different socs (with likely the same mmc IP) (A13/A20) I found in the JEDEC spec in the appendix on changes from 4.4 to 4.41:
Introduce of High Priority Interrupt mechanism, HPI background and one of possible solutions.Which leaves me to believe, that HPI was added to eMMC in version 4.41 and was not available before. Both the A13 and A20 user manuals state that the MMC controller in these socs is only version 4.3.
Obviously I know far to little as to what does and what does not work with an MMC-host, since it's just a simple data-bus, but I would not be surprised if these things are somewhere somewhat related, so hopefully someone on this list can give their opinion on this.
Quoted 6 lines
I already checked the
core/mmc.cwhere the version is read from the mmc controller to see if we can trigger on this based on version, however it seems only the major version number is stored here [0]?So in my opinion, it is quite likely that this setting be moved up to the mmc-core level, for the SoC, rather then the card itself?
Yes I was talking to Tsvetan from Olimex and he says they have seen the problem with yet
another emmc, so this indeed seems to be a sunxi SoC specific problem, and we just need to
disable hpi on all sunxi SoC's.Do you have time to look into doing a kernel patch for this ?
I'm thinking adding a host-cap called
MMC_CAP_BROKEN_HPIand adding support for that todrivers/mmc/core/host.c:mmc_of_parse()and extending the current card check for broken-hpi to also checkhost->capsQuoted 4 lines
And then we need to decide whether to just hard set that cap in
drivers/mmc/host/sunxi_mmc.cor if we are going to add it to our dtsi files. I think we should probably just hard-code it indrivers/mmc/host/sunxi_mmc.c.Yeah I'll see if I can look into that.
I was also looking if we really cannot detect older MMC versions, and I think we can somewhat. I haven't figured out the details yet though.
Quoted 7 lines
The
EXT_CSD_REVfield does yield a version number matching the various 4 sub-versions, so for now I'm thinking of simply disabeling HPI for anything before 4.41 (unless maybe overriden). Since before 4.41 it shouldn't have been supported. Maybe add an override that allows use of HPI? but then we have the whole backwards compatibility thing.[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L368
I re-read what you said and looked at the files and think your idea is probably best, since there is no version information (in sunxi_mmc.c for example) anyway). So there is versioning information on the card level and the mmc core level, but not in the host level.
The quick fix, is to use MMC_CAP_BROKEN_HPI
The better fix that would feel better/proper to use MMC_CAP_HPI instead, to indicate that a chipset actually can do HPI, since nothing says our chipset should do HPI (v4.3 that we have doesn't support it at all).
I'm debating if it would be usefull to have a version field in the mmc host struct, which would set some capabilities by default based on the version level, but I don't think we have that many capabilities right now anyway do we?
P.S. you want to keep the broken-hpi flag (on a card level?) for backwards compatibility with older dts files? Or move it to the host level so that the MMC_CAP_BROKEN_HPI field can be overriden via the dts? So sunxi-mmc.c sets MMC_CAP_BROKEN_HPI, a user could still set broken-hpi to false to indicate the HPI is actually working for him?
Olliver
Quoted 111 lines
Quoted 110 lines
Regards,
Hans
Quoted 99 lines
[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L148
Quoted 96 lines
Signed-off-by Hans de Goede <[email protected]>---
Changes in v2:
-Fixof_nodeleakage--- Documentation/devicetree/bindings/mmc/mmc-card.txt | 31 ++++++++++++++++++++++ drivers/mmc/core/mmc.c | 10 ++++++- 2 files changed, 40 insertions(+), 1 deletion(-)create mode 100644
Documentation/devicetree/bindings/mmc/mmc-card.txtDocumentation/devicetree/bindings/mmc/mmc-card.txt new file +4
-0@@ -0,0 +1,31 @@ +mmc-card / eMMC bindings +------------------------ + +This documents describes the devicetree bindings for a mmc-hostcontroller
+child node describing a mmc-card / an eMMC, see "Use of Function subnodes"Documentation/devicetree/bindings/mmc/mmc-card.txt new file +26
-0+in mmc.txt + +Required properties: +-compatible : Must be "mmc-card" +-reg : Must be <0> + +Optional properties: +-broken-hpi : Use this to indicate that the mmc-card has a broken hpi + implementation, and that hpi should not be used + +Example: + +&mmc2 { + pinctrl-names = "default"; + pinctrl-0 = <&mmc2_pins_a>; + vmmc-supply = <®_vcc3v3>; + bus-width = <8>; + non-removable; + status = "okay"; + + mmccard: mmccard at 0 { + reg = <0>; + compatible = "mmc-card"; + broken-hpi; + }; +};drivers/mmc/core/mmc.c +3
-0@@ -11,6 +11,7 @@*/ #include <linux/err.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/stat.h> #include <linux/pm_runtime.h> @@ -336,6 +337,8 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) {int err =0 , idx;unsigned int part_size; +struct device_node *np; +bool broken_hpi = false;/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
card->ext_csd.raw_ext_csd_structure =ext_csd[EXT_CSD_STRUCTURE];
drivers/mmc/core/mmc.c +6
-1@@ -349,6 +352,11 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) } } + np =mmc_of_find_child_device (card->host,0 ); +if (np &&of_device_is_compatible (np,"mmc-card" )) + broken_hpi =of_property_read_bool (np,"broken-hpi" ); +of_node_put (np); +/* * The EXT_CSD format is meant to be forward compatible. As long * as CSD_STRUCTURE does not change, all values for EXT_CSD_REV @@ -494,7 +502,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) }/* check whether the eMMC card supports HPI */ -if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 ) { +if (!broken_hpi && (ext_csd[EXT_CSD_HPI_FEATURES ] &0x1 )) { card->ext_csd.hpi =1 ;if (ext_csd[EXT_CSD_HPI_FEATURES ] &0x2 ) card->ext_csd.hpi_cmd =MMC_STOP_TRANSMISSION ;
PATCH v2 1/2 mmc: Add support for marking hpi as broken through devicetree
Hi,
Quoted 57 lines · Olliver Schinagl
Hey Hans,
Quoted 54 lines · Olliver Schinagl
Hey Hans,
Quoted 42 lines · Hans de Goede
Hi,
Quoted 20 lines · Olliver Schinagl
Hey Hans,
Quoted 10 lines · Hans de Goede
The eMMC on a tablet I've will stop working / communicating as soon as the kernel executes:
mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time);There seems to be no way to reliable identify eMMC-s which have a broken hpi implementation, but at least for eMMC's which are soldered onto a board we can work around this by specifying that hpi is broken in devicetree.
We've been talking about this a little off-list, and I was just triggered again by this so I'm following up on it again.
The same issue bit me in the ass on a Micron 'industrial' grade eMMC (MTFC4GACAANA) and luckily you found this back then to help me.
As we discussed, this may very well be a limitation of the mmc controller, rather then the MMC chip. While we only have a sample size of 2 on 2 different socs (with likely the same mmc IP) (A13/A20) I found in the JEDEC spec in the appendix on changes from 4.4 to 4.41:
Introduce of High Priority Interrupt mechanism, HPI background and one of possible solutions.Quoted 7 lines
Which leaves me to believe, that HPI was added to eMMC in version 4.41 and was not available before. Both the A13 and A20 user manuals state that the MMC controller in these socs is only version 4.3.
Obviously I know far to little as to what does and what does not work with an MMC-host, since it's just a simple data-bus, but I would not be surprised if these things are somewhere somewhat related, so hopefully someone on this list can give their opinion on this.
I already checked the
core/mmc.cwhere the version is read from the mmc controller to see if we can trigger on this based on version, however it seems only the major version number is stored here [0]?So in my opinion, it is quite likely that this setting be moved up to the mmc-core level, for the SoC, rather then the card itself?
Yes I was talking to Tsvetan from Olimex and he says they have seen the problem with yet another emmc, so this indeed seems to be a sunxi SoC specific problem, and we just need to disable hpi on all sunxi SoC's.
Do you have time to look into doing a kernel patch for this ?
I'm thinking adding a host-cap called
MMC_CAP_BROKEN_HPIand adding support for that todrivers/mmc/core/host.c:mmc_of_parse()and extending the current card check for broken-hpi to also checkhost->capsQuoted 4 lines
And then we need to decide whether to just hard set that cap in
drivers/mmc/host/sunxi_mmc.cor if we are going to add it to our dtsi files. I think we should probably just hard-code it indrivers/mmc/host/sunxi_mmc.c.Yeah I'll see if I can look into that.
I was also looking if we really cannot detect older MMC versions, and I think we can somewhat. I haven't figured out the details yet though.
Quoted 8 lines
Quoted 3 lines
The
EXT_CSD_REVfield does yield a version number matching the various 4 sub-versions, so for now I'm thinking of simply disabeling HPI for anything before 4.41 (unless maybe overriden). Since before 4.41 it shouldn't have been supported. Maybe add an override that allows use of HPI? but then we have the whole backwards compatibility thing.[0] http://lxr.free-electrons.com/source/drivers/mmc/core/mmc.c#L368
I re-read what you said and looked at the files and think your idea is probably best, since there is no version information (in
sunxi_mmc.cfor example) anyway). So there is versioning information on the card level and the mmc core level, but not in the host level.The quick fix, is to use
MMC_CAP_BROKEN_HPI
The better fix that would feel better/proper to useMMC_CAP_HPIinstead, to indicate that a chipset actually can do HPI, since nothing says our chipset should do HPI (v4.3 that we have doesn't support it at all).
Maybe, I think for starters doing a MMC_CAP_BROKEN_HPI is best, and then later maybe do an RFC patch to introduce MMC_CAP_HPI instead, this becomes difficult quickly though, e.g. on which controllers should we enable MMC_CAP_HPI ? Since sunxi currently is the only SoC having issues with this, I think that MMC_CAP_BROKEN_HPI is the best way to deal with this.
Quoted 1 line
I'm debating if it would be usefull to have a version field in the mmc host struct,
I'm not familiar enough with mmc to say for sure, but I've the feeling that a lot of card caps introduced in later specs can be used by older hosts just fine, since they are only a matter of changing the host-driver / mmc-core. I think it is best to just go for the KISS solution here, introduce the MMC_CAP_BROKEN_HPI flag and leave it at that.
Quoted 3 lines
which would set some capabilities by default based on the version level, but I don't think we have that many capabilities right now anyway do we?
P.S. you want to keep the broken-hpi flag (on a card level?) for backwards compatibility with older dts files?
I think we should keep it, it may also be useful for non sunxi boards in some cases.
Regards,
Hans