RE: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits workaround

2010-08-02 Thread Zang Roy-R61911
 

 -Original Message-
 From: Anton Vorontsov [mailto:cbouatmai...@gmail.com] 
 Sent: Friday, July 30, 2010 15:06 PM
 To: Zang Roy-R61911
 Cc: linux-...@vger.kernel.org; linuxppc-...@ozlabs.org; 
 a...@linux-foundation.org
 Subject: Re: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits 
 workaround
 
 On Fri, Jul 30, 2010 at 11:52:57AM +0800, Roy Zang wrote:
  P4080 ESDHC controller does not support 1.8V and 3.0V 
 voltage. but the
  host controller capabilities register wrongly set the bits.
  This patch adds the workaround to correct the weird voltage 
 setting bits.
  
  Signed-off-by: Roy Zang tie-fei.z...@freescale.com
  ---
 [...]
  diff --git a/drivers/mmc/host/sdhci-of-core.c 
 b/drivers/mmc/host/sdhci-of-core.c
  index 0c30242..1f3913d 100644
  --- a/drivers/mmc/host/sdhci-of-core.c
  +++ b/drivers/mmc/host/sdhci-of-core.c
  @@ -164,6 +164,10 @@ static int __devinit 
 sdhci_of_probe(struct of_device *ofdev,
  if (sdhci_of_wp_inverted(np))
  host-quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
   
  +   if (of_device_is_compatible(np, fsl,p4080-esdhc))
  +   host-quirks |= (SDHCI_QUIRK_QORIQ_NO_VDD_180
  +   |SDHCI_QUIRK_QORIQ_NO_VDD_300);
  +
 
 It should be two properties, something like sdhci,no-vdd-180
 and sdhci,no-vdd-300. But it might be even better: we have
 voltage-ranges for mmc-spi case, see
 Documentation/powerpc/dts-bindings/mmc-spi-slot.txt.
 
 If voltage-ranges specified, then we use it, not capabilities
 register.
 
 For p4080 it will be 'voltage-ranges = 3200 3400;'. So, with
 voltage-ranges we can do fine grained VDD control without
 introducing anything new.
why not
   voltage-ranges = 3300 3300;
?
Roy

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits workaround

2010-08-02 Thread Anton Vorontsov
On Mon, Aug 02, 2010 at 02:19:58PM +0800, Zang Roy-R61911 wrote:
[...]
  For p4080 it will be 'voltage-ranges = 3200 3400;'. So, with
  voltage-ranges we can do fine grained VDD control without
  introducing anything new.
 why not
voltage-ranges = 3300 3300;

Right you are, both will be 3300.

Thanks,

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 3/3 v2] mmc: Add ESDHC weird voltage bits workaround

2010-07-30 Thread Anton Vorontsov
On Fri, Jul 30, 2010 at 11:52:57AM +0800, Roy Zang wrote:
 P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the
 host controller capabilities register wrongly set the bits.
 This patch adds the workaround to correct the weird voltage setting bits.
 
 Signed-off-by: Roy Zang tie-fei.z...@freescale.com
 ---
[...]
 diff --git a/drivers/mmc/host/sdhci-of-core.c 
 b/drivers/mmc/host/sdhci-of-core.c
 index 0c30242..1f3913d 100644
 --- a/drivers/mmc/host/sdhci-of-core.c
 +++ b/drivers/mmc/host/sdhci-of-core.c
 @@ -164,6 +164,10 @@ static int __devinit sdhci_of_probe(struct of_device 
 *ofdev,
   if (sdhci_of_wp_inverted(np))
   host-quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
  
 + if (of_device_is_compatible(np, fsl,p4080-esdhc))
 + host-quirks |= (SDHCI_QUIRK_QORIQ_NO_VDD_180
 + |SDHCI_QUIRK_QORIQ_NO_VDD_300);
 +

It should be two properties, something like sdhci,no-vdd-180
and sdhci,no-vdd-300. But it might be even better: we have
voltage-ranges for mmc-spi case, see
Documentation/powerpc/dts-bindings/mmc-spi-slot.txt.

If voltage-ranges specified, then we use it, not capabilities
register.

For p4080 it will be 'voltage-ranges = 3200 3400;'. So, with
voltage-ranges we can do fine grained VDD control without
introducing anything new.

As for implementation, you might just factor out voltage-ranges
parsing from drivers/mmc/host/of_mmc_spi.c, and then in sdhci
driver you could do.

if (host-ocr_avail)
mmc-ocr_avail = host-ocr_avail.

   clk = of_get_property(np, clock-frequency, size);
   if (clk  size == sizeof(*clk)  *clk)
   of_host-clock = *clk;
 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index 1424d08..a667790 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -1699,6 +1699,14 @@ int sdhci_add_host(struct sdhci_host *host)
  
   caps = sdhci_readl(host, SDHCI_CAPABILITIES);
  
 +  /* Workaround for P4080 host controller capabilities
 +   * 1.8V and 3.0V do not supported*/
 + if (host-quirks  SDHCI_QUIRK_QORIQ_NO_VDD_180)

The point of making NO_VDD stuff is to make these quirks
chip-agnostic. Ideally, sdhci.c should never know about
particular chips.

So, you shouldn't name quirks with QORIQ.

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev