* [PATCHv1 0/2] mmc: Move broken-hpi also to the chipset level @ 2016-04-19 7:12 Olliver Schinagl 2016-04-19 7:12 ` [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree Olliver Schinagl 2016-04-19 7:12 ` [PATCH 2/2] mmc: sunxi: Mark the HPI of the mmc controller broken by default Olliver Schinagl 0 siblings, 2 replies; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 7:12 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek Cc: linux-mmc, linux-kernel, linux-arm-kernel, Olliver Schinagl A few months ago, hans de Goede and I where talking about the behavior of HPI on eMMC modules. We came to the conclusion that more often then not, the mmc chipset might be the problem and not the eMMC module itself. To remedy this the broken-hpi parameter is now also checked on the chipset level. On the sunxi platform, all the eMMC users with emmc modules that support HPI need the broken-hpi flag, indicating that the sunxi mmc controller is likely not properly supporting HPI. We disable this on the chipset level for these chipsets. As for the broken-hpi flag in the devicetree's, those should in theory be removed from the boards in question in a separate patch when in agreement, as technically, the eMMC module supports HPI just fine and thus the we are lying about what really is broken. This was tested on an OLinuXino Lime2 with 4GB industrial grade Micron eMMC flash. Olliver Schinagl (2): mmc: core: Improve marking broken HPI through devicetree mmc: sunxi: Mark the HPI of the mmc controller broken by default drivers/mmc/core/host.c | 2 ++ drivers/mmc/core/mmc.c | 2 +- drivers/mmc/host/sunxi-mmc.c | 4 ++-- include/linux/mmc/host.h | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) -- 2.8.0.rc3 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 7:12 [PATCHv1 0/2] mmc: Move broken-hpi also to the chipset level Olliver Schinagl @ 2016-04-19 7:12 ` Olliver Schinagl 2016-04-19 9:29 ` Ulf Hansson 2016-04-19 10:52 ` Sergei Shtylyov 2016-04-19 7:12 ` [PATCH 2/2] mmc: sunxi: Mark the HPI of the mmc controller broken by default Olliver Schinagl 1 sibling, 2 replies; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 7:12 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek Cc: linux-mmc, linux-kernel, linux-arm-kernel, Olliver Schinagl In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc device as to having an broken HPI implementation. After talking some with Hans, we now think it is actually the mmc controller that can be broken and not support broken HPI's. This patch adds a new capability, mmc-broken-hpi, which allows us to mark a broken hpi implementation on the host level. Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> --- drivers/mmc/core/host.c | 2 ++ drivers/mmc/core/mmc.c | 2 +- include/linux/mmc/host.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index 6e4c55a..9b63b36 100644 --- a/drivers/mmc/core/host.c +++ b/drivers/mmc/core/host.c @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) host->caps |= MMC_CAP_HW_RESET; if (of_property_read_bool(np, "cap-sdio-irq")) host->caps |= MMC_CAP_SDIO_IRQ; + if (of_property_read_bool(np, "mmc-broken-hpi")) + host->caps |= MMC_CAP_BROKEN_HPI; if (of_property_read_bool(np, "full-pwr-cycle")) host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; if (of_property_read_bool(np, "keep-power-in-suspend")) diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 4dbe3df..9a19562 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, /* * Enable HPI feature (if supported) */ - if (card->ext_csd.hpi) { + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HPI_MGMT, 1, card->ext_csd.generic_cmd6_time); diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 8dd4d29..20f758e 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -264,6 +264,7 @@ struct mmc_host { #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ -- 2.8.0.rc3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 7:12 ` [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree Olliver Schinagl @ 2016-04-19 9:29 ` Ulf Hansson 2016-04-19 9:42 ` Olliver Schinagl 2016-04-19 10:52 ` Sergei Shtylyov 1 sibling, 1 reply; 15+ messages in thread From: Ulf Hansson @ 2016-04-19 9:29 UTC (permalink / raw) To: Olliver Schinagl Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: > In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc > device as to having an broken HPI implementation. After talking some > with Hans, we now think it is actually the mmc controller that can be > broken and not support broken HPI's. I don't want us to invent a DT binding for something you *think* is a HW controller issue. Have you really excluded that this isn't a software issue? Me personally haven't been using HPI that much so I can't really tell about the code robustness from the mmc core (mmc protocol point of view). Kind regards Uffe > > This patch adds a new capability, mmc-broken-hpi, which allows us to > mark a broken hpi implementation on the host level. > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > --- > drivers/mmc/core/host.c | 2 ++ > drivers/mmc/core/mmc.c | 2 +- > include/linux/mmc/host.h | 1 + > 3 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c > index 6e4c55a..9b63b36 100644 > --- a/drivers/mmc/core/host.c > +++ b/drivers/mmc/core/host.c > @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) > host->caps |= MMC_CAP_HW_RESET; > if (of_property_read_bool(np, "cap-sdio-irq")) > host->caps |= MMC_CAP_SDIO_IRQ; > + if (of_property_read_bool(np, "mmc-broken-hpi")) > + host->caps |= MMC_CAP_BROKEN_HPI; > if (of_property_read_bool(np, "full-pwr-cycle")) > host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; > if (of_property_read_bool(np, "keep-power-in-suspend")) > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 4dbe3df..9a19562 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > /* > * Enable HPI feature (if supported) > */ > - if (card->ext_csd.hpi) { > + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { > err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > EXT_CSD_HPI_MGMT, 1, > card->ext_csd.generic_cmd6_time); > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index 8dd4d29..20f758e 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -264,6 +264,7 @@ struct mmc_host { > #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ > #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ > #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ > +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ > #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ > #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ > > -- > 2.8.0.rc3 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 9:29 ` Ulf Hansson @ 2016-04-19 9:42 ` Olliver Schinagl 2016-04-19 9:49 ` Jaehoon Chung ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 9:42 UTC (permalink / raw) To: Ulf Hansson Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel Hi Ulf, On 19-04-16 11:29, Ulf Hansson wrote: > On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >> device as to having an broken HPI implementation. After talking some >> with Hans, we now think it is actually the mmc controller that can be >> broken and not support broken HPI's. > I don't want us to invent a DT binding for something you *think* is a > HW controller issue. > > Have you really excluded that this isn't a software issue? Me > personally haven't been using HPI that much so I can't really tell > about the code robustness from the mmc core (mmc protocol point of > view). Well this patch goes hand in hand so to speak with the broken-hpi patch introduced by him, he did most of the investigation. We just discussed how to handle it and asked me to cook up the patch. @hans, what do you think? > > Kind regards > Uffe > >> This patch adds a new capability, mmc-broken-hpi, which allows us to >> mark a broken hpi implementation on the host level. >> >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >> --- >> drivers/mmc/core/host.c | 2 ++ >> drivers/mmc/core/mmc.c | 2 +- >> include/linux/mmc/host.h | 1 + >> 3 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >> index 6e4c55a..9b63b36 100644 >> --- a/drivers/mmc/core/host.c >> +++ b/drivers/mmc/core/host.c >> @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) >> host->caps |= MMC_CAP_HW_RESET; >> if (of_property_read_bool(np, "cap-sdio-irq")) >> host->caps |= MMC_CAP_SDIO_IRQ; >> + if (of_property_read_bool(np, "mmc-broken-hpi")) >> + host->caps |= MMC_CAP_BROKEN_HPI; >> if (of_property_read_bool(np, "full-pwr-cycle")) >> host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; >> if (of_property_read_bool(np, "keep-power-in-suspend")) >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 4dbe3df..9a19562 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >> /* >> * Enable HPI feature (if supported) >> */ >> - if (card->ext_csd.hpi) { >> + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { >> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> EXT_CSD_HPI_MGMT, 1, >> card->ext_csd.generic_cmd6_time); >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 8dd4d29..20f758e 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -264,6 +264,7 @@ struct mmc_host { >> #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ >> #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ >> #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ >> +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ >> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ >> #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ >> >> -- >> 2.8.0.rc3 >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 9:42 ` Olliver Schinagl @ 2016-04-19 9:49 ` Jaehoon Chung 2016-04-19 12:20 ` Olliver Schinagl 2016-04-19 9:51 ` Ulf Hansson 2016-04-19 11:22 ` Hans de Goede 2 siblings, 1 reply; 15+ messages in thread From: Jaehoon Chung @ 2016-04-19 9:49 UTC (permalink / raw) To: Olliver Schinagl, Ulf Hansson Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel Hi On 04/19/2016 06:42 PM, Olliver Schinagl wrote: > Hi Ulf, > > On 19-04-16 11:29, Ulf Hansson wrote: >> On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >>> device as to having an broken HPI implementation. After talking some >>> with Hans, we now think it is actually the mmc controller that can be >>> broken and not support broken HPI's. >> I don't want us to invent a DT binding for something you *think* is a >> HW controller issue. >> >> Have you really excluded that this isn't a software issue? Me >> personally haven't been using HPI that much so I can't really tell >> about the code robustness from the mmc core (mmc protocol point of >> view). > Well this patch goes hand in hand so to speak with the broken-hpi patch introduced by him, he did most of the investigation. We just discussed how to handle it and asked me to cook up the patch. I didn't understand why add this property. Is this same patch? commit 81f8a7be6642b4c26ab681b2e0f4c4120a6de1b0 Author: Hans de Goede <hdegoede@redhat.com> Date: Wed Apr 1 17:26:23 2015 +0200 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. > > @hans, what do you think? >> >> Kind regards >> Uffe >> >>> This patch adds a new capability, mmc-broken-hpi, which allows us to >>> mark a broken hpi implementation on the host level. >>> >>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>> --- >>> drivers/mmc/core/host.c | 2 ++ >>> drivers/mmc/core/mmc.c | 2 +- >>> include/linux/mmc/host.h | 1 + >>> 3 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >>> index 6e4c55a..9b63b36 100644 >>> --- a/drivers/mmc/core/host.c >>> +++ b/drivers/mmc/core/host.c >>> @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) >>> host->caps |= MMC_CAP_HW_RESET; >>> if (of_property_read_bool(np, "cap-sdio-irq")) >>> host->caps |= MMC_CAP_SDIO_IRQ; >>> + if (of_property_read_bool(np, "mmc-broken-hpi")) >>> + host->caps |= MMC_CAP_BROKEN_HPI; >>> if (of_property_read_bool(np, "full-pwr-cycle")) >>> host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; >>> if (of_property_read_bool(np, "keep-power-in-suspend")) >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>> index 4dbe3df..9a19562 100644 >>> --- a/drivers/mmc/core/mmc.c >>> +++ b/drivers/mmc/core/mmc.c >>> @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >>> /* >>> * Enable HPI feature (if supported) >>> */ >>> - if (card->ext_csd.hpi) { >>> + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { >>> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>> EXT_CSD_HPI_MGMT, 1, >>> card->ext_csd.generic_cmd6_time); >>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>> index 8dd4d29..20f758e 100644 >>> --- a/include/linux/mmc/host.h >>> +++ b/include/linux/mmc/host.h >>> @@ -264,6 +264,7 @@ struct mmc_host { >>> #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ >>> #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ >>> #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ >>> +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ >>> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ >>> #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ >>> >>> -- >>> 2.8.0.rc3 >>> > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 9:49 ` Jaehoon Chung @ 2016-04-19 12:20 ` Olliver Schinagl 2016-04-19 12:44 ` Jaehoon Chung 0 siblings, 1 reply; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 12:20 UTC (permalink / raw) To: Jaehoon Chung, Ulf Hansson Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel Hey Jaehoon, On 19-04-16 11:49, Jaehoon Chung wrote: > Hi > > On 04/19/2016 06:42 PM, Olliver Schinagl wrote: >> Hi Ulf, >> >> On 19-04-16 11:29, Ulf Hansson wrote: >>> On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >>>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >>>> device as to having an broken HPI implementation. After talking some >>>> with Hans, we now think it is actually the mmc controller that can be >>>> broken and not support broken HPI's. >>> I don't want us to invent a DT binding for something you *think* is a >>> HW controller issue. >>> >>> Have you really excluded that this isn't a software issue? Me >>> personally haven't been using HPI that much so I can't really tell >>> about the code robustness from the mmc core (mmc protocol point of >>> view). >> Well this patch goes hand in hand so to speak with the broken-hpi patch introduced by him, he did most of the investigation. We just discussed how to handle it and asked me to cook up the patch. > I didn't understand why add this property. Is this same patch? > > commit 81f8a7be6642b4c26ab681b2e0f4c4120a6de1b0 > Author: Hans de Goede <hdegoede@redhat.com> > Date: Wed Apr 1 17:26:23 2015 +0200 > > 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. Similar, this does it on the card level, e.g. you mark a card as having a broken HPI implementation. The truth is, the card may not be broken at all, but the mmc controller so we add a property on the controller level. What Ulf is pondering, if this is not a software stack problem, rather then a hardware implementation, which I don't know. >> @hans, what do you think? >>> Kind regards >>> Uffe >>> >>>> This patch adds a new capability, mmc-broken-hpi, which allows us to >>>> mark a broken hpi implementation on the host level. >>>> >>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>>> --- >>>> drivers/mmc/core/host.c | 2 ++ >>>> drivers/mmc/core/mmc.c | 2 +- >>>> include/linux/mmc/host.h | 1 + >>>> 3 files changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >>>> index 6e4c55a..9b63b36 100644 >>>> --- a/drivers/mmc/core/host.c >>>> +++ b/drivers/mmc/core/host.c >>>> @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) >>>> host->caps |= MMC_CAP_HW_RESET; >>>> if (of_property_read_bool(np, "cap-sdio-irq")) >>>> host->caps |= MMC_CAP_SDIO_IRQ; >>>> + if (of_property_read_bool(np, "mmc-broken-hpi")) >>>> + host->caps |= MMC_CAP_BROKEN_HPI; >>>> if (of_property_read_bool(np, "full-pwr-cycle")) >>>> host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; >>>> if (of_property_read_bool(np, "keep-power-in-suspend")) >>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>>> index 4dbe3df..9a19562 100644 >>>> --- a/drivers/mmc/core/mmc.c >>>> +++ b/drivers/mmc/core/mmc.c >>>> @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >>>> /* >>>> * Enable HPI feature (if supported) >>>> */ >>>> - if (card->ext_csd.hpi) { >>>> + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { >>>> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>>> EXT_CSD_HPI_MGMT, 1, >>>> card->ext_csd.generic_cmd6_time); >>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>>> index 8dd4d29..20f758e 100644 >>>> --- a/include/linux/mmc/host.h >>>> +++ b/include/linux/mmc/host.h >>>> @@ -264,6 +264,7 @@ struct mmc_host { >>>> #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ >>>> #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ >>>> #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ >>>> +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ >>>> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ >>>> #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ >>>> >>>> -- >>>> 2.8.0.rc3 >>>> >> >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 12:20 ` Olliver Schinagl @ 2016-04-19 12:44 ` Jaehoon Chung 0 siblings, 0 replies; 15+ messages in thread From: Jaehoon Chung @ 2016-04-19 12:44 UTC (permalink / raw) To: Olliver Schinagl, Ulf Hansson Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel Hi Olliver, On 04/19/2016 09:20 PM, Olliver Schinagl wrote: > Hey Jaehoon, > > On 19-04-16 11:49, Jaehoon Chung wrote: >> Hi >> >> On 04/19/2016 06:42 PM, Olliver Schinagl wrote: >>> Hi Ulf, >>> >>> On 19-04-16 11:29, Ulf Hansson wrote: >>>> On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >>>>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >>>>> device as to having an broken HPI implementation. After talking some >>>>> with Hans, we now think it is actually the mmc controller that can be >>>>> broken and not support broken HPI's. >>>> I don't want us to invent a DT binding for something you *think* is a >>>> HW controller issue. >>>> >>>> Have you really excluded that this isn't a software issue? Me >>>> personally haven't been using HPI that much so I can't really tell >>>> about the code robustness from the mmc core (mmc protocol point of >>>> view). >>> Well this patch goes hand in hand so to speak with the broken-hpi patch introduced by him, he did most of the investigation. We just discussed how to handle it and asked me to cook up the patch. >> I didn't understand why add this property. Is this same patch? >> >> commit 81f8a7be6642b4c26ab681b2e0f4c4120a6de1b0 >> Author: Hans de Goede <hdegoede@redhat.com> >> Date: Wed Apr 1 17:26:23 2015 +0200 >> >> 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. > Similar, this does it on the card level, e.g. you mark a card as having a broken HPI implementation. > > The truth is, the card may not be broken at all, but the mmc controller so we add a property on the controller level. > > What Ulf is pondering, if this is not a software stack problem, rather then a hardware implementation, which I don't know. Hmm..problem is mmc controller side..it's strange. How did you know this problem is on mmc controller side? Well, i should be also missing something for HPI feature..but it's not relevant to mmc controller. Ok. i will check more.. Best Regards, Jaehoon Chung >>> @hans, what do you think? >>>> Kind regards >>>> Uffe >>>> >>>>> This patch adds a new capability, mmc-broken-hpi, which allows us to >>>>> mark a broken hpi implementation on the host level. >>>>> >>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>>>> --- >>>>> drivers/mmc/core/host.c | 2 ++ >>>>> drivers/mmc/core/mmc.c | 2 +- >>>>> include/linux/mmc/host.h | 1 + >>>>> 3 files changed, 4 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >>>>> index 6e4c55a..9b63b36 100644 >>>>> --- a/drivers/mmc/core/host.c >>>>> +++ b/drivers/mmc/core/host.c >>>>> @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) >>>>> host->caps |= MMC_CAP_HW_RESET; >>>>> if (of_property_read_bool(np, "cap-sdio-irq")) >>>>> host->caps |= MMC_CAP_SDIO_IRQ; >>>>> + if (of_property_read_bool(np, "mmc-broken-hpi")) >>>>> + host->caps |= MMC_CAP_BROKEN_HPI; >>>>> if (of_property_read_bool(np, "full-pwr-cycle")) >>>>> host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; >>>>> if (of_property_read_bool(np, "keep-power-in-suspend")) >>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>>>> index 4dbe3df..9a19562 100644 >>>>> --- a/drivers/mmc/core/mmc.c >>>>> +++ b/drivers/mmc/core/mmc.c >>>>> @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >>>>> /* >>>>> * Enable HPI feature (if supported) >>>>> */ >>>>> - if (card->ext_csd.hpi) { >>>>> + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { >>>>> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>>>> EXT_CSD_HPI_MGMT, 1, >>>>> card->ext_csd.generic_cmd6_time); >>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>>>> index 8dd4d29..20f758e 100644 >>>>> --- a/include/linux/mmc/host.h >>>>> +++ b/include/linux/mmc/host.h >>>>> @@ -264,6 +264,7 @@ struct mmc_host { >>>>> #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ >>>>> #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ >>>>> #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ >>>>> +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ >>>>> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ >>>>> #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ >>>>> >>>>> -- >>>>> 2.8.0.rc3 >>>>> >>> >>> > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 9:42 ` Olliver Schinagl 2016-04-19 9:49 ` Jaehoon Chung @ 2016-04-19 9:51 ` Ulf Hansson 2016-04-19 11:22 ` Hans de Goede 2 siblings, 0 replies; 15+ messages in thread From: Ulf Hansson @ 2016-04-19 9:51 UTC (permalink / raw) To: Olliver Schinagl Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel On 19 April 2016 at 11:42, Olliver Schinagl <oliver@schinagl.nl> wrote: > Hi Ulf, > > On 19-04-16 11:29, Ulf Hansson wrote: >> >> On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >>> >>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >>> device as to having an broken HPI implementation. After talking some >>> with Hans, we now think it is actually the mmc controller that can be >>> broken and not support broken HPI's. >> >> I don't want us to invent a DT binding for something you *think* is a >> HW controller issue. >> >> Have you really excluded that this isn't a software issue? Me >> personally haven't been using HPI that much so I can't really tell >> about the code robustness from the mmc core (mmc protocol point of >> view). > > Well this patch goes hand in hand so to speak with the broken-hpi patch > introduced by him, he did most of the investigation. We just discussed how > to handle it and asked me to cook up the patch. Well, my point is that it's more understandable about having a broken HPI implementation for eMMC cards, but for host controllers I am not so sure. I don't think there is an electrical change required by the host controller to support HPI, is just like any other command, right? Unless I am missing something, of course. Kind regards Uffe ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 9:42 ` Olliver Schinagl 2016-04-19 9:49 ` Jaehoon Chung 2016-04-19 9:51 ` Ulf Hansson @ 2016-04-19 11:22 ` Hans de Goede 2016-04-19 12:45 ` Olliver Schinagl 2 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2016-04-19 11:22 UTC (permalink / raw) To: Olliver Schinagl, Ulf Hansson Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Jaehoon Chung, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel Hi, On 19-04-16 11:42, Olliver Schinagl wrote: > Hi Ulf, > > On 19-04-16 11:29, Ulf Hansson wrote: >> On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >>> device as to having an broken HPI implementation. After talking some >>> with Hans, we now think it is actually the mmc controller that can be >>> broken and not support broken HPI's. >> >> I don't want us to invent a DT binding for something you *think* is a >> HW controller issue. I agree we should not add a dt flag for this, we can simply set the flag in the host driver if we believe this is a host issue. >> Have you really excluded that this isn't a software issue? Me >> personally haven't been using HPI that much so I can't really tell >> about the code robustness from the mmc core (mmc protocol point of >> view). > Well this patch goes hand in hand so to speak with the broken-hpi patch introduced by him, he did most of the investigation. We just discussed how to handle it and asked me to cook up the patch. > > @hans, what do you think? When we discussed this a while back we had a pretty small sample set of sunxi boards with emmc, and it seemed that hpi was broken on all of them. But recently we've been seeing eMMC-s on a lot more boards and they all work fine, except the one on my Utoo A13 tablets, and the one you are using, so this does really seem be an eMMC specific problem. That or it is a problem with the host on sun4i/sun5i/sun7i which is not present on sun6i, sun8i and later ... But given how rare eMMC-s are on sun4i/sun5i/sun7i I think the current solution where we set a flag on the emmc dt node rather then on the host node / in the host driver is fine. Taking your case into account too, that will bring us up to 2 cases where we set the broken-hpi flag on the emmc node, which does not really seem like a number to worry about. TL;DR: Thanks for writing this patch set, but given recent developments I believe that it is best to keep handling broken-hpi as we are doing in current kernels and no changes are necessary. Regards, Hans >> >> Kind regards >> Uffe >> >>> This patch adds a new capability, mmc-broken-hpi, which allows us to >>> mark a broken hpi implementation on the host level. >>> >>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>> --- >>> drivers/mmc/core/host.c | 2 ++ >>> drivers/mmc/core/mmc.c | 2 +- >>> include/linux/mmc/host.h | 1 + >>> 3 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >>> index 6e4c55a..9b63b36 100644 >>> --- a/drivers/mmc/core/host.c >>> +++ b/drivers/mmc/core/host.c >>> @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) >>> host->caps |= MMC_CAP_HW_RESET; >>> if (of_property_read_bool(np, "cap-sdio-irq")) >>> host->caps |= MMC_CAP_SDIO_IRQ; >>> + if (of_property_read_bool(np, "mmc-broken-hpi")) >>> + host->caps |= MMC_CAP_BROKEN_HPI; >>> if (of_property_read_bool(np, "full-pwr-cycle")) >>> host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; >>> if (of_property_read_bool(np, "keep-power-in-suspend")) >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>> index 4dbe3df..9a19562 100644 >>> --- a/drivers/mmc/core/mmc.c >>> +++ b/drivers/mmc/core/mmc.c >>> @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >>> /* >>> * Enable HPI feature (if supported) >>> */ >>> - if (card->ext_csd.hpi) { >>> + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { >>> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>> EXT_CSD_HPI_MGMT, 1, >>> card->ext_csd.generic_cmd6_time); >>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>> index 8dd4d29..20f758e 100644 >>> --- a/include/linux/mmc/host.h >>> +++ b/include/linux/mmc/host.h >>> @@ -264,6 +264,7 @@ struct mmc_host { >>> #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports Driver Type A */ >>> #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports Driver Type C */ >>> #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports Driver Type D */ >>> +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for HPI is broken */ >>> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ >>> #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ >>> >>> -- >>> 2.8.0.rc3 >>> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 11:22 ` Hans de Goede @ 2016-04-19 12:45 ` Olliver Schinagl 2016-04-19 19:20 ` Ulf Hansson 0 siblings, 1 reply; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 12:45 UTC (permalink / raw) To: Hans de Goede, Ulf Hansson Cc: Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Jaehoon Chung, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel Hey Hans, On 19-04-16 13:22, Hans de Goede wrote: > Hi, > > On 19-04-16 11:42, Olliver Schinagl wrote: >> Hi Ulf, >> >> On 19-04-16 11:29, Ulf Hansson wrote: >>> On 19 April 2016 at 09:12, Olliver Schinagl <oliver@schinagl.nl> wrote: >>>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an >>>> mmc >>>> device as to having an broken HPI implementation. After talking some >>>> with Hans, we now think it is actually the mmc controller that can be >>>> broken and not support broken HPI's. > >> >>> I don't want us to invent a DT binding for something you *think* is a >>> HW controller issue. > > I agree we should not add a dt flag for this, we can simply set the > flag in the host driver if we believe this is a host issue. Okay, I can remove the dt flag > >>> Have you really excluded that this isn't a software issue? Me >>> personally haven't been using HPI that much so I can't really tell >>> about the code robustness from the mmc core (mmc protocol point of >>> view). >> Well this patch goes hand in hand so to speak with the broken-hpi >> patch introduced by him, he did most of the investigation. We just >> discussed how to handle it and asked me to cook up the patch. >> >> @hans, what do you think? > > When we discussed this a while back we had a pretty small sample > set of sunxi boards with emmc, and it seemed that hpi was broken > on all of them. But recently we've been seeing eMMC-s on a lot more > boards and they all work fine, except the one on my Utoo A13 tablets, > and the one you are using, so this does really seem be an eMMC > specific problem. That or it is a problem with the host on > sun4i/sun5i/sun7i > which is not present on sun6i, sun8i and later ... Well I think we still have a very small sample size, the sun4i, sun5i and sun7i boards (all using the same mmc controller afaik) have the broken-hpi set, the sun6i and sun8i seem to be working fine (different mmc controller?). I'm not so sure it is an eMMC specific problem though. The module we're using is a high-end micron part, industrial grade. Granted, it could be still broken there, but I find it less likely. Micron has an interessting technical document: "TN-52-05 e.EMMC Linux Enablement" talking specifically about HPI on eMMC. It also mentions HPI is a mmc/jedic 4.41 thing, afaik our controller is only 4.3 (which might not even matter according to Ulf if it is just a command). The datasheet of the chip I use, MTFC4GACAANA, also mentions explicitly that it supports HPI. Granted, it could still be broken, but I have doubts. > > But given how rare eMMC-s are on sun4i/sun5i/sun7i I think the current > solution where we set a flag on the emmc dt node rather then on the > host node / in the host driver is fine. Yeah it is very limited, that is true, and I suppose I can live with that. > > Taking your case into account too, that will bring us up to 2 cases > where we set the broken-hpi flag on the emmc node, which does not > really seem like a number to worry about. Actually, 4 :), there are 3 sun5i (tablets?) devices that suffer from this and my device now. The sun6i and sun8i devices are only 2 (the sinlinx devices in the current kernel) that a very quick grep (mmc-card ) showed. Grepping for non-removable yielded a bit more, like the chip sun5i-like device with a "non-removable" mmc, not sure what to make of that though. > > TL;DR: Thanks for writing this patch set, but given recent developments > I believe that it is best to keep handling broken-hpi as we are doing > in current kernels and no changes are necessary. I would still recommend to add the capability and raise the flag for the sun[457]i devices though, as my gut thinks it's a problem with the sunxi IP there. But with the emmc-card level work around, it does solve/fix it, so what is the best way? Olliver > > Regards, > > Hans > > >>> >>> Kind regards >>> Uffe >>> >>>> This patch adds a new capability, mmc-broken-hpi, which allows us to >>>> mark a broken hpi implementation on the host level. >>>> >>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >>>> --- >>>> drivers/mmc/core/host.c | 2 ++ >>>> drivers/mmc/core/mmc.c | 2 +- >>>> include/linux/mmc/host.h | 1 + >>>> 3 files changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >>>> index 6e4c55a..9b63b36 100644 >>>> --- a/drivers/mmc/core/host.c >>>> +++ b/drivers/mmc/core/host.c >>>> @@ -270,6 +270,8 @@ int mmc_of_parse(struct mmc_host *host) >>>> host->caps |= MMC_CAP_HW_RESET; >>>> if (of_property_read_bool(np, "cap-sdio-irq")) >>>> host->caps |= MMC_CAP_SDIO_IRQ; >>>> + if (of_property_read_bool(np, "mmc-broken-hpi")) >>>> + host->caps |= MMC_CAP_BROKEN_HPI; >>>> if (of_property_read_bool(np, "full-pwr-cycle")) >>>> host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE; >>>> if (of_property_read_bool(np, "keep-power-in-suspend")) >>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>>> index 4dbe3df..9a19562 100644 >>>> --- a/drivers/mmc/core/mmc.c >>>> +++ b/drivers/mmc/core/mmc.c >>>> @@ -1592,7 +1592,7 @@ static int mmc_init_card(struct mmc_host >>>> *host, u32 ocr, >>>> /* >>>> * Enable HPI feature (if supported) >>>> */ >>>> - if (card->ext_csd.hpi) { >>>> + if (card->ext_csd.hpi && !(host->caps & MMC_CAP_BROKEN_HPI)) { >>>> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>>> EXT_CSD_HPI_MGMT, 1, >>>> card->ext_csd.generic_cmd6_time); >>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>>> index 8dd4d29..20f758e 100644 >>>> --- a/include/linux/mmc/host.h >>>> +++ b/include/linux/mmc/host.h >>>> @@ -264,6 +264,7 @@ struct mmc_host { >>>> #define MMC_CAP_DRIVER_TYPE_A (1 << 23) /* Host supports >>>> Driver Type A */ >>>> #define MMC_CAP_DRIVER_TYPE_C (1 << 24) /* Host supports >>>> Driver Type C */ >>>> #define MMC_CAP_DRIVER_TYPE_D (1 << 25) /* Host supports >>>> Driver Type D */ >>>> +#define MMC_CAP_BROKEN_HPI (1 << 29) /* Host support for >>>> HPI is broken */ >>>> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 >>>> supported. */ >>>> #define MMC_CAP_HW_RESET (1 << 31) /* Hardware reset */ >>>> >>>> -- >>>> 2.8.0.rc3 >>>> >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 12:45 ` Olliver Schinagl @ 2016-04-19 19:20 ` Ulf Hansson 0 siblings, 0 replies; 15+ messages in thread From: Ulf Hansson @ 2016-04-19 19:20 UTC (permalink / raw) To: Olliver Schinagl Cc: Hans de Goede, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Jaehoon Chung, Michal Suchanek, linux-mmc, linux-kernel, linux-arm-kernel [...] > Well I think we still have a very small sample size, the sun4i, sun5i and > sun7i boards (all using the same mmc controller afaik) have the broken-hpi > set, the sun6i and sun8i seem to be working fine (different mmc > controller?). > I'm not so sure it is an eMMC specific problem though. The module we're > using is a high-end micron part, industrial grade. Granted, it could be > still broken there, but I find it less likely. Micron has an interessting > technical document: > "TN-52-05 e.EMMC Linux Enablement" > talking specifically about HPI on eMMC. > It also mentions HPI is a mmc/jedic 4.41 thing, afaik our controller is only > 4.3 (which might not even matter according to Ulf if it is just a command). > > The datasheet of the chip I use, MTFC4GACAANA, also mentions explicitly that > it supports HPI. > > Granted, it could still be broken, but I have doubts. Perhaps the same eMMC is used without errors on other controllers? > >> >> But given how rare eMMC-s are on sun4i/sun5i/sun7i I think the current >> solution where we set a flag on the emmc dt node rather then on the >> host node / in the host driver is fine. > > Yeah it is very limited, that is true, and I suppose I can live with that. >> >> >> Taking your case into account too, that will bring us up to 2 cases >> where we set the broken-hpi flag on the emmc node, which does not >> really seem like a number to worry about. > > Actually, 4 :), there are 3 sun5i (tablets?) devices that suffer from this > and my device now. The sun6i and sun8i devices are only 2 (the sinlinx > devices in the current kernel) that a very quick grep (mmc-card ) showed. > Grepping for non-removable yielded a bit more, like the chip sun5i-like > device with a "non-removable" mmc, not sure what to make of that though. >> >> >> TL;DR: Thanks for writing this patch set, but given recent developments >> I believe that it is best to keep handling broken-hpi as we are doing >> in current kernels and no changes are necessary. > > I would still recommend to add the capability and raise the flag for the > sun[457]i devices though, as my gut thinks it's a problem with the sunxi IP > there. But with the emmc-card level work around, it does solve/fix it, so > what is the best way? The best is clearly to make a proper debug investigation before we decide to add a DT binding for the host. I don't know on what level you are able to measure signals on the HW, but if not, perhaps the newly TRACE support in the MMC core can help. Kind regards Uffe ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 7:12 ` [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree Olliver Schinagl 2016-04-19 9:29 ` Ulf Hansson @ 2016-04-19 10:52 ` Sergei Shtylyov 2016-04-19 12:21 ` Olliver Schinagl 1 sibling, 1 reply; 15+ messages in thread From: Sergei Shtylyov @ 2016-04-19 10:52 UTC (permalink / raw) To: Olliver Schinagl, Ulf Hansson, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek Cc: linux-mmc, linux-kernel, linux-arm-kernel Hello. On 4/19/2016 10:12 AM, Olliver Schinagl wrote: > In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc scripts/checkpatch.pl now enforces certain commit citing style, the commit summary should be specified too. > device as to having an broken HPI implementation. After talking some > with Hans, we now think it is actually the mmc controller that can be > broken and not support broken HPI's. > > This patch adds a new capability, mmc-broken-hpi, which allows us to > mark a broken hpi implementation on the host level. > > Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> [...] MBR, Sergei ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 10:52 ` Sergei Shtylyov @ 2016-04-19 12:21 ` Olliver Schinagl 2016-04-19 13:23 ` Sergei Shtylyov 0 siblings, 1 reply; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 12:21 UTC (permalink / raw) To: Sergei Shtylyov, Ulf Hansson, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek Cc: linux-mmc, linux-kernel, linux-arm-kernel Hey Sergei, On 19-04-16 12:52, Sergei Shtylyov wrote: > Hello. > > On 4/19/2016 10:12 AM, Olliver Schinagl wrote: > >> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc > > scripts/checkpatch.pl now enforces certain commit citing style, the > commit summary should be specified too. I used checkpatch from 4.6.0-rc2(or3) and did not notice it. But I'll add it to v2. > >> device as to having an broken HPI implementation. After talking some >> with Hans, we now think it is actually the mmc controller that can be >> broken and not support broken HPI's. >> >> This patch adds a new capability, mmc-broken-hpi, which allows us to >> mark a broken hpi implementation on the host level. >> >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> > [...] > > MBR, Sergei > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree 2016-04-19 12:21 ` Olliver Schinagl @ 2016-04-19 13:23 ` Sergei Shtylyov 0 siblings, 0 replies; 15+ messages in thread From: Sergei Shtylyov @ 2016-04-19 13:23 UTC (permalink / raw) To: Olliver Schinagl, Ulf Hansson, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek Cc: linux-mmc, linux-kernel, linux-arm-kernel On 4/19/2016 3:21 PM, Olliver Schinagl wrote: >>> In patch 81f8a7be66 Hans de Goede added a patch to allow marking an mmc >> >> scripts/checkpatch.pl now enforces certain commit citing style, the >> commit summary should be specified too. > I used checkpatch from 4.6.0-rc2(or3) and did not notice it. But I'll add it > to v2. Maybe it just didn't recognize your commit citing? Anyway, the format is the same as for the "Fixes:" tag. >>> device as to having an broken HPI implementation. After talking some >>> with Hans, we now think it is actually the mmc controller that can be >>> broken and not support broken HPI's. >>> >>> This patch adds a new capability, mmc-broken-hpi, which allows us to >>> mark a broken hpi implementation on the host level. >>> >>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> >> [...] MBR, Sergei ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] mmc: sunxi: Mark the HPI of the mmc controller broken by default 2016-04-19 7:12 [PATCHv1 0/2] mmc: Move broken-hpi also to the chipset level Olliver Schinagl 2016-04-19 7:12 ` [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree Olliver Schinagl @ 2016-04-19 7:12 ` Olliver Schinagl 1 sibling, 0 replies; 15+ messages in thread From: Olliver Schinagl @ 2016-04-19 7:12 UTC (permalink / raw) To: Ulf Hansson, Maxime Ripard, Chen-Yu Tsai, Venu Byravarasu, Adrian Hunter, Michal Hocko, Lars-Peter Clausen, Sudeep Holla, Sergei Shtylyov, Wolfram Sang, Wenkai Du, Chaotian Jing, Kuninori Morimoto, Hans de Goede, Jaehoon Chung, Michal Suchanek Cc: linux-mmc, linux-kernel, linux-arm-kernel, Olliver Schinagl So far it seems all sunxi MMC controllers have a broken HPI. Until proved otherwise, mark all sunxi mmc controllers as having a broken HPI. Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> --- drivers/mmc/host/sunxi-mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 8372a41..acf24fb 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -1126,8 +1126,8 @@ static int sunxi_mmc_probe(struct platform_device *pdev) mmc->f_min = 400000; mmc->f_max = 52000000; mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED | - MMC_CAP_1_8V_DDR | - MMC_CAP_ERASE | MMC_CAP_SDIO_IRQ; + MMC_CAP_1_8V_DDR | MMC_CAP_ERASE | + MMC_CAP_SDIO_IRQ | MMC_CAP_BROKEN_HPI; ret = mmc_of_parse(mmc); if (ret) -- 2.8.0.rc3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-04-19 19:20 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-19 7:12 [PATCHv1 0/2] mmc: Move broken-hpi also to the chipset level Olliver Schinagl 2016-04-19 7:12 ` [PATCH 1/2] mmc: core: Improve marking broken HPI through devicetree Olliver Schinagl 2016-04-19 9:29 ` Ulf Hansson 2016-04-19 9:42 ` Olliver Schinagl 2016-04-19 9:49 ` Jaehoon Chung 2016-04-19 12:20 ` Olliver Schinagl 2016-04-19 12:44 ` Jaehoon Chung 2016-04-19 9:51 ` Ulf Hansson 2016-04-19 11:22 ` Hans de Goede 2016-04-19 12:45 ` Olliver Schinagl 2016-04-19 19:20 ` Ulf Hansson 2016-04-19 10:52 ` Sergei Shtylyov 2016-04-19 12:21 ` Olliver Schinagl 2016-04-19 13:23 ` Sergei Shtylyov 2016-04-19 7:12 ` [PATCH 2/2] mmc: sunxi: Mark the HPI of the mmc controller broken by default Olliver Schinagl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).