mbox series

[0/4] mmc: Add sdhci workaround stability enhencement

Message ID 20191202144104.5069-1-jun.nie@linaro.org
Headers show
Series mmc: Add sdhci workaround stability enhencement | expand

Message

Jun Nie Dec. 2, 2019, 2:41 p.m. UTC
This patch set made three changes:
 1. set enhence full power cycle stability.
 2. Add dt property to support DMA memory address boundary workaround.
 3. Add dt property to non-standard HS400 mode value in ctrl register.


Jun Nie (4):
  mmc: sdhci: Add delay after power off
  mmc: sdhci: dt: Add DMA boundary and HS400 properties
  mmc: sdhci: Set ctrl_hs400 value in dts
  mmc: sdhci: Add DMA memory boundary workaround

 .../devicetree/bindings/mmc/sdhci.txt         |  8 +++++
 drivers/mmc/host/sdhci.c                      | 36 +++++++++++++++++--
 drivers/mmc/host/sdhci.h                      |  9 ++++-
 3 files changed, 50 insertions(+), 3 deletions(-)

-- 
2.17.1

Comments

Adrian Hunter Dec. 3, 2019, 7:26 a.m. UTC | #1
On 2/12/19 4:41 pm, Jun Nie wrote:
> Add delay after power off to ensure that full power cycle is

> successful. Otherwise, some controllers, at lease for Hisilicon

> eMMC controller, may not be unstable sometimes for full power

> cycle operation.

> 

> Signed-off-by: Jun Nie <jun.nie@linaro.org>

> ---

>  drivers/mmc/host/sdhci.c | 7 +++++++

>  drivers/mmc/host/sdhci.h | 2 ++

>  2 files changed, 9 insertions(+)

> 

> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c

> index 3140fe2e5dba..a654f0aeb438 100644

> --- a/drivers/mmc/host/sdhci.c

> +++ b/drivers/mmc/host/sdhci.c

> @@ -1761,6 +1761,13 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,

>  		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);

>  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)

>  			sdhci_runtime_pm_bus_off(host);

> +

> +		/*

> +		 * Some controllers need an extra 100ms delay to secure

> +		 * full power cycle is successful.

> +		 */

> +		if (host->quirks2 & SDHCI_QUIRK2_DELAY_AFTER_POWER_OFF)

> +			msleep(100);


Please use the ->set_power() callback and do this in your own driver.

>  	} else {

>  		/*

>  		 * Spec says that we should clear the power reg before setting

> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h

> index 0ed3e0eaef5f..0e6f97eaa796 100644

> --- a/drivers/mmc/host/sdhci.h

> +++ b/drivers/mmc/host/sdhci.h

> @@ -482,6 +482,8 @@ struct sdhci_host {

>   * block count.

>   */

>  #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT			(1<<18)

> +/* Controllers need an extra 100ms delay to make sure power off completely */

> +#define SDHCI_QUIRK2_DELAY_AFTER_POWER_OFF		(1<<19)

>  

>  	int irq;		/* Device IRQ */

>  	void __iomem *ioaddr;	/* Mapped address */

>
Jun Nie Dec. 17, 2019, 2:56 p.m. UTC | #2
Rob Herring <robh@kernel.org> 于2019年12月14日周六 上午7:01写道:
>

> On Mon, Dec 02, 2019 at 10:41:02PM +0800, Jun Nie wrote:

> > DMA memory cannot cross specific boundary on some controller, such as 128MB

> > on SDHCI Designware. Add sdhci-dma-mem-boundary property to split DMA

> > operation in such case.

> >

> > sdhci-ctrl-hs400 specify the HS400 mode setting for register

> > SDHCI_HOST_CONTROL2(offset 0x3E:bit[2:0]). Because this value is not

> > defined in SDHC Standard specification.

> >

> > Signed-off-by: Jun Nie <jun.nie@linaro.org>

> > ---

> >  Documentation/devicetree/bindings/mmc/sdhci.txt | 8 ++++++++

> >  1 file changed, 8 insertions(+)

> >

> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci.txt b/Documentation/devicetree/bindings/mmc/sdhci.txt

> > index 0e9923a64024..e6d7feb9a741 100644

> > --- a/Documentation/devicetree/bindings/mmc/sdhci.txt

> > +++ b/Documentation/devicetree/bindings/mmc/sdhci.txt

> > @@ -11,3 +11,11 @@ Optional properties:

> >  - sdhci-caps: The sdhci capabilities register is incorrect. This 64bit

> >    property corresponds to the bits in the sdhci capability register. If the

> >    bit is on in the property then the bit should be turned on.

> > +- sdhci-dma-mem-boundary: The sdhci controller DMA memory space boundary.

> > +  If the controller's DMA cannot cross a specific memory space boundary,

> > +  such as 128MB, set this value in dt and driver will split the DMA

> > +  operation when crossing such boundary.

>

> This should be implied by the compatible string.

>

> > +- sdhci-ctrl-hs400: The HS400 is not defined in SDHC Standard specification

> > +  for SDHCI_HOST_CONTROL2(offset 0x3E:bit[2:0]). Different controllers have

> > +  have different value for HS400 mode. If 0x5 is not the HS400 mode value

> > +  for your controller, you should specify the value with this property.

>

> This too, unless it needs to be tuned per board.

>

> Can you be more specific as to what the possible values are and what

> they do?


It is specific to SoC or specific to controller. HS400 mode value on
DWC3 of new Hisilicon
 SoC is 7, not 5. This same is to DMA buffer memory address boundary.
Do you mean you
want the boundary value and HS400 mode value should bundled with compatible,
ie. specific SoC or controller, to set a value in sdhci layer from
platform glue driver?

>

> Rob