[PATCH] ARM: EXYNOS: Avoid early use of of_machine_is_compatible()

[Doug Anderson]

The recent commit "ARM: EXYNOS: add support for EXYNOS5440 SoC" broke
support for exynos5250 because of_machine_is_compatible() was used too
early in the boot process.  It also probably meant that the exynos5440
failed to use the proper iotable.  Switch to use
of_flat_dt_is_compatible() in both of these cases.

The failure I was seeing in exynos5250 because of this was:
  Division by zero in kernel.
  [<80015ed4>] (unwind_backtrace+0x0/0xec) from [<8045c7a4>] 
(dump_stack+0x20/0x24)
  [<8045c7a4>] (dump_stack+0x20/0x24) from [<80012990>] (__div0+0x20/0x28)
  [<80012990>] (__div0+0x20/0x28) from [<8021ab04>] (Ldiv0_64+0x8/0x18)
  [<8021ab04>] (Ldiv0_64+0x8/0x18) from [<80068560>] 
(__clocksource_updatefreq_scale+0x54/0x134)
  [<80068560>] (__clocksource_updatefreq_scale+0x54/0x134) from [<8006865c>] 
(__clocksource_register_scale+0x1c/0x54)
  [<8006865c>] (__clocksource_register_scale+0x1c/0x54) from [<80612a18>] 
(exynos_timer_init+0x100/0x1e8)
  [<80612a18>] (exynos_timer_init+0x100/0x1e8) from [<8060d184>] 
(time_init+0x28/0x38)
  [<8060d184>] (time_init+0x28/0x38) from [<8060a754>] 
(start_kernel+0x1e0/0x3c8)
  [<8060a754>] (start_kernel+0x1e0/0x3c8) from [<40008078>] (0x40008078)

Signed-off-by: Doug Anderson 

---
 arch/arm/mach-exynos/common.c  |5 -
 arch/arm/mach-exynos/mach-exynos5-dt.c |4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 73b940f..b919f5f 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -335,8 +336,10 @@ void __init exynos_init_late(void)
 
 void __init exynos_init_io(struct map_desc *mach_desc, int size)
 {
+   unsigned long root = of_get_flat_dt_root();
+
/* initialize the io descriptors we need for initialization */
-   if (of_machine_is_compatible("samsung,exynos5440"))
+   if (of_flat_dt_is_compatible(root, "samsung,exynos5440"))
iotable_init(exynos5440_iodesc, ARRAY_SIZE(exynos5440_iodesc));
else
iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc));
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c 
b/arch/arm/mach-exynos/mach-exynos5-dt.c
index 67fa3c2..7b76c3f 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -122,9 +122,11 @@ static const struct of_dev_auxdata 
exynos5440_auxdata_lookup[] __initconst = {
 
 static void __init exynos5_dt_map_io(void)
 {
+   unsigned long root = of_get_flat_dt_root();
+
exynos_init_io(NULL, 0);
 
-   if (of_machine_is_compatible("samsung,exynos5250"))
+   if (of_flat_dt_is_compatible(root, "samsung,exynos5250"))
s3c24xx_init_clocks(2400);
 }
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: EXYNOS: Avoid early use of of_machine_is_compatible()

[Doug Anderson]

Olof / Kukjin,

On Tue, Nov 27, 2012 at 10:05 PM, Olof Johansson  wrote:
> On Wed, Nov 28, 2012 at 02:23:09PM +0900, Kukjin Kim wrote:
>> Olof Johansson wrote:
>> >
>> > On Tue, Nov 27, 2012 at 2:27 PM, Kukjin Kim  wrote:
>> > > On 11/28/12 07:11, Olof Johansson wrote:
>> > >>
>> > >> On Tue, Nov 27, 2012 at 11:53 AM, Doug Anderson
>> > >> wrote:
>> > >>>
>> > >>> The recent commit "ARM: EXYNOS: add support for EXYNOS5440 SoC" broke
>> > >>> support for exynos5250 because of_machine_is_compatible() was used too
>> > >>> early in the boot process.  It also probably meant that the exynos5440
>> > >>> failed to use the proper iotable.  Switch to use
>> > >>> of_flat_dt_is_compatible() in both of these cases.
>> > >>>
>> > >>> The failure I was seeing in exynos5250 because of this was:
>> > >>>Division by zero in kernel.
>> > >>>[<80015ed4>] (unwind_backtrace+0x0/0xec) from [<8045c7a4>]
>> > >>> (dump_stack+0x20/0x24)
>> > >>>[<8045c7a4>] (dump_stack+0x20/0x24) from [<80012990>]
>> > >>> (__div0+0x20/0x28)
>> > >>>[<80012990>] (__div0+0x20/0x28) from [<8021ab04>]
>> (Ldiv0_64+0x8/0x18)
>> > >>>[<8021ab04>] (Ldiv0_64+0x8/0x18) from [<80068560>]
>> > >>> (__clocksource_updatefreq_scale+0x54/0x134)
>> > >>>[<80068560>] (__clocksource_updatefreq_scale+0x54/0x134) from
>> > >>> [<8006865c>] (__clocksource_register_scale+0x1c/0x54)
>> > >>>[<8006865c>] (__clocksource_register_scale+0x1c/0x54) from
>> > >>> [<80612a18>] (exynos_timer_init+0x100/0x1e8)
>> > >>>[<80612a18>] (exynos_timer_init+0x100/0x1e8) from [<8060d184>]
>> > >>> (time_init+0x28/0x38)
>> > >>>[<8060d184>] (time_init+0x28/0x38) from [<8060a754>]
>> > >>> (start_kernel+0x1e0/0x3c8)
>> > >>>[<8060a754>] (start_kernel+0x1e0/0x3c8) from [<40008078>]
>> > (0x40008078)
>> > >>>
>> > >>> Signed-off-by: Doug Anderson
>> > >>
>> > >>
>> > >> Thanks Doug.
>> > >>
>> > >> Kukjin, I'll apply this directly on top of the previous branch in
>> > >> arm-soc, if that's OK with you.
>> > >>
>> > > Sure, go ahead with my ack if you want,
>> > >
>> > > Acked-by: Kukjin Kim 
>> > >
>> > > Note, actually there was a fix which uses soc_is_exynos5440() in my
>> > local
>> > > :-) I'm not sure which one is better at this moment, but I'm OK on this.
>> >
>> > Ok, applied. Thanks all.
>> >
>> Olof, just note, happens build error with exynos4_defconfig because of
>> non-DT.
>
> Ick, thanks for catching that.

Sorry for this!  I will try to be more diligent about trying
exynos4_defconfig before submitting future patches to these files.

>>
>> Following can resolve it or we should create null function for
>> of_get_flat_dt_root() and of_flat_dt_is_compatible()...
>>
>> 8<---
>> From: Kukjin Kim 
>> Subject: ARM: EXYNOS: fix a build error with non-DT for exynos4
>>
>> This fixes following in case of non-DT:
>> arch/arm/mach-exynos/common.c: In function 'exynos_init_io':
>> arch/arm/mach-exynos/common.c:339: error: implicit declaration of function
>> 'of_get_flat_dt_root'
>> arch/arm/mach-exynos/common.c:342: error: implicit declaration of function
>> 'of_flat_dt_is_compatible'
>> make[1]: *** [arch/arm/mach-exynos/common.o] Error 1
>>
>> Signed-off-by: Kukjin Kim 
>> ---
>> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
>> index b919f5f..2110091 100644
>> --- a/arch/arm/mach-exynos/common.c
>> +++ b/arch/arm/mach-exynos/common.c
>> @@ -336,12 +336,14 @@ void __init exynos_init_late(void)
>>
>>  void __init exynos_init_io(struct map_desc *mach_desc, int size)
>>  {
>> +#ifdef CONFIG_OF
>>   unsigned long root = of_get_flat_dt_root();
>>
>>   /* initialize the io descriptors we need for initialization */
>>   if (of_flat_dt_is_compatible(root, "samsung,exynos5440"))
>>   iotable_i

Re: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Doug Anderson]

Seungwon,

Thanks for the review.  See below for comments.  If you'd like me to
respin then please let me know.  Otherwise I look forward to your ack.

On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon  wrote:
> Yes. pin of write protection is common property.
> This change is good. I have some suggestion below.
> Could you check it?
>
> On Friday, November 23, 2012, Doug Anderson wrote:
>> The exynos code claimed wp-gpio with devm_gpio_request() but never did
>> anything with it.  That meant that anyone using a write protect GPIO
>> would effectively be write protected all the time.
>>
>> A future change will move the wp-gpio support to the core dw_mmc.c
>> file.  Now the exynos-specific code won't claim the GPIO but will
>> just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
>> won't be used.
>>
>> Signed-off-by: Doug Anderson 
>>
>> ---
>> Changes in v2:
>> - Nothing new in this patch
>>
>>  drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
>> b/drivers/mmc/host/dw_mmc-exynos.c
>> index 4d50da6..58cc03e 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
>>   }
>>   }
>>
>> - gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
>> - if (gpio_is_valid(gpio)) {
>> - if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
>> - dev_info(host->dev, "gpio [%d] request failed\n",
>> - gpio);
>> - } else {
>> + /*
>> +  * If there are no write-protect GPIOs present then we assume no write
>> +  * protect.  The mci_readl() in dw_mmc.c won't work since it's not
>> +  * hooked up on exynos.
>> +  */
>> + if (!of_find_property(slot_np, "wp-gpios", NULL)) {
>>   dev_info(host->dev, "wp gpio not available");
>>   host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>>   }
> All card types need this quirk in case wp-gpio property is empty?
> I think wp-pin is valid for SD card, not eMMC/SDIO.

Right.  It is only checked right now by the SD code (mmc/core/sd.c).
It doesn't particularly hurt to set it the quirk in other cases though
and it seems nice not to add special cases.  I could imagine someone
extending the MMC code at some point to support write protect (via
GPIO) for eMMC, so there's even a slight justification for avoiding
the special case.


> Of course, I know origin code did it.
> How about removing whole checking routine?
> Instead, new definition for this quirk can be added into 
> 'dw_mci_of_quirks'(dw_mmc.c) and dts file.

On _exynos_ all SD cards need this quirk if there is no wp-gpio
property.  However this is not generally true for all users of dw_mmc.
 The DesignWare IP Block actually has a write protect input that can
be read with "mci_readl(slot->host, WRTPRT)" but on exynos the
DesignWare write protect line isn't exposed on any physical pins.
That means that the only possible way to do write protect on exynos is
using a GPIO.

The above means that on exynos if the GPIO isn't defined we will
assume no write protect.  On other platforms if the GPIO isn't defined
we'll assume that the "mci_readl" will work and we'll use that.

If people would prefer it I can code up an alternate solution that
doesn't touch any exynos code but that would introduce a new device
tree binding.  We could accomplish what's needed for exynos using a
property like "broken-internal-wp".

Please let me know if you'd like me to submit a new patch with this
solution or if you like the existing solution.


> Thanks,
> Seungwon Jeon
>> --
>> 1.7.7.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/4] mmc: dw_mmc: Add "disable-wp" device tree property

[Doug Anderson]

The "disable-wp" property is used to specify that a given SD card slot
doesn't have a concept of write protect.  This eliminates the need for
special case code for SD slots that should never be write protected
(like a micro SD slot or a dev board).

The dw_mmc driver is special in needing to specify "disable-wp"
because the lack of a "wp-gpios" property means to use the special
purpose write protect line.  On some other mmc devices the lack of
"wp-gpios" means that write protect should be disabled.

Signed-off-by: Doug Anderson 
---
Changes in v3:
- New for this version of the patch series.  Chose "disable-wp" rather
  than the discussed "broken-internal-wp" since it mapped more cleanly
  to an existing quirk (and the only reason to specify that the
  internal wp is broken is if you're disabling the write protect
  anyway).

 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |   12 +-
 drivers/mmc/host/dw_mmc.c  |   36 +++-
 include/linux/mmc/dw_mmc.h |4 ++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt 
b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index 06cd32d08..726fd21 100644
--- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
@@ -26,8 +26,16 @@ Required Properties:
* bus-width: as documented in mmc core bindings.
 
* wp-gpios: specifies the write protect gpio line. The format of the
- gpio specifier depends on the gpio controller. If the write-protect
- line is not available, this property is optional.
+ gpio specifier depends on the gpio controller. If a GPIO is not used
+ for write-protect, this property is optional.
+
+   * disable-wp: If the wp-gpios property isn't present then (by default)
+ we'd assume that the write protect is hooked up directly to the
+ controller's special purpose write protect line (accessible via
+ the WRTPRT register).  However, it's possible that we simply don't
+ want write protect.  In that case specify 'disable-wp'.
+ NOTE: This property is not required for slots known to always
+ connect to eMMC or SDIO cards.
 
 Optional properties:
 
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7342029..b47b1e9 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -74,6 +74,7 @@ struct idmac_desc {
  * struct dw_mci_slot - MMC slot state
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
+ * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  * processed, or NULL when the slot is idle.
@@ -88,6 +89,8 @@ struct dw_mci_slot {
struct mmc_host *mmc;
struct dw_mci   *host;
 
+   int quirks;
+
u32 ctype;
 
struct mmc_request  *mrq;
@@ -828,7 +831,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
struct dw_mci_board *brd = slot->host->pdata;
 
/* Use platform get_ro function, else try on board write protect */
-   if (brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT)
+   if ((brd->quirks & DW_MCI_QUIRK_NO_WRITE_PROTECT) ||
+   (slot->quirks & DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT))
read_only = 0;
else if (brd->get_ro)
read_only = brd->get_ro(slot->id);
@@ -1788,6 +1792,30 @@ static struct device_node 
*dw_mci_of_find_slot_node(struct device *dev, u8 slot)
return NULL;
 }
 
+static struct dw_mci_of_slot_quirks {
+   char *quirk;
+   int id;
+} of_slot_quirks[] = {
+   {
+   .quirk  = "disable-wp",
+   .id = DW_MCI_SLOT_QUIRK_NO_WRITE_PROTECT,
+   },
+};
+
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+   struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+   int quirks = 0;
+   int idx;
+
+   /* get quirks */
+   for (idx = 0; idx < ARRAY_SIZE(of_slot_quirks); idx++)
+   if (of_get_property(np, of_slot_quirks[idx].quirk, NULL))
+   quirks |= of_slot_quirks[idx].id;
+
+   return quirks;
+}
+
 /* find out bus-width for a given slot */
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
@@ -1803,6 +1831,10 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 
slot)
return bus_wd;
 }
 #else /* CONFIG_OF */
+static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
+{
+   return 0;
+}
 static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
 {
r

[PATCH v3 3/4] mmc: dw_mmc: exynos: Remove code for wp-gpios

[Doug Anderson]

The exynos code claimed the write protect with devm_gpio_request() but
never did anything with it.  That meant that anyone using a write
protect GPIO would effectively be write protected all the time.

The handling for wp-gpios belongs in the main dw_mmc driver and has
been moved there.

Signed-off-by: Doug Anderson 
---
Changes in v3:
- Totally removed wp-gpios handling from exynos code.

Changes in v2: None

 drivers/mmc/host/dw_mmc-exynos.c |   10 --
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 4d50da6..72fd0f2 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -175,16 +175,6 @@ static int dw_mci_exynos_setup_bus(struct dw_mci *host,
}
}
 
-   gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
-   if (gpio_is_valid(gpio)) {
-   if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
-   dev_info(host->dev, "gpio [%d] request failed\n",
-   gpio);
-   } else {
-   dev_info(host->dev, "wp gpio not available");
-   host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
-   }
-
if (host->pdata->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION)
return 0;
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/2] mmc: dw_mmc: exynos: Stop claiming wp-gpio

[Doug Anderson]

Seungwon,


On Wed, Nov 28, 2012 at 11:46 PM, Seungwon Jeon  wrote:
> Hi Doug,
>
> On Thursday, November 29, 2012, Doug Anderson wrote:
>> Seungwon,
>>
>> Thanks for the review.  See below for comments.  If you'd like me to
>> respin then please let me know.  Otherwise I look forward to your ack.
>>
>> On Wed, Nov 28, 2012 at 1:29 AM, Seungwon Jeon  wrote:
>> > Yes. pin of write protection is common property.
>> > This change is good. I have some suggestion below.
>> > Could you check it?
>> >
>> > On Friday, November 23, 2012, Doug Anderson wrote:
>> >> The exynos code claimed wp-gpio with devm_gpio_request() but never did
>> >> anything with it.  That meant that anyone using a write protect GPIO
>> >> would effectively be write protected all the time.
>> >>
>> >> A future change will move the wp-gpio support to the core dw_mmc.c
>> >> file.  Now the exynos-specific code won't claim the GPIO but will
>> >> just set the DW_MCI_QUIRK_NO_WRITE_PROTECT quirk if write protect
>> >> won't be used.
>> >>
>> >> Signed-off-by: Doug Anderson 
>> >>
>> >> ---
>> >> Changes in v2:
>> >> - Nothing new in this patch
>> >>
>> >>  drivers/mmc/host/dw_mmc-exynos.c |   12 ++--
>> >>  1 files changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
>> >> b/drivers/mmc/host/dw_mmc-exynos.c
>> >> index 4d50da6..58cc03e 100644
>> >> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> >> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> >> @@ -175,12 +175,12 @@ static int dw_mci_exynos_setup_bus(struct dw_mci 
>> >> *host,
>> >>   }
>> >>   }
>> >>
>> >> - gpio = of_get_named_gpio(slot_np, "wp-gpios", 0);
>> >> - if (gpio_is_valid(gpio)) {
>> >> - if (devm_gpio_request(host->dev, gpio, "dw-mci-wp"))
>> >> - dev_info(host->dev, "gpio [%d] request failed\n",
>> >> - gpio);
>> >> - } else {
>> >> + /*
>> >> +  * If there are no write-protect GPIOs present then we assume no 
>> >> write
>> >> +  * protect.  The mci_readl() in dw_mmc.c won't work since it's not
>> >> +  * hooked up on exynos.
>> >> +  */
>> >> + if (!of_find_property(slot_np, "wp-gpios", NULL)) {
>> >>   dev_info(host->dev, "wp gpio not available");
>> >>   host->pdata->quirks |= DW_MCI_QUIRK_NO_WRITE_PROTECT;
>> >>   }
>> > All card types need this quirk in case wp-gpio property is empty?
>> > I think wp-pin is valid for SD card, not eMMC/SDIO.
>>
>> Right.  It is only checked right now by the SD code (mmc/core/sd.c).
>> It doesn't particularly hurt to set it the quirk in other cases though
>> and it seems nice not to add special cases.  I could imagine someone
>> extending the MMC code at some point to support write protect (via
>> GPIO) for eMMC, so there's even a slight justification for avoiding
>> the special case.
>>
>>
>> > Of course, I know origin code did it.
>> > How about removing whole checking routine?
>> > Instead, new definition for this quirk can be added into 
>> > 'dw_mci_of_quirks'(dw_mmc.c) and dts file.
>>
>> On _exynos_ all SD cards need this quirk if there is no wp-gpio
>> property.  However this is not generally true for all users of dw_mmc.
>>  The DesignWare IP Block actually has a write protect input that can
>> be read with "mci_readl(slot->host, WRTPRT)" but on exynos the
>> DesignWare write protect line isn't exposed on any physical pins.
>> That means that the only possible way to do write protect on exynos is
>> using a GPIO.
>>
>> The above means that on exynos if the GPIO isn't defined we will
>> assume no write protect.  On other platforms if the GPIO isn't defined
>> we'll assume that the "mci_readl" will work and we'll use that.
>>
>> If people would prefer it I can code up an alternate solution that
>> doesn't touch any exynos code but that would introduce a new device
>> tree binding.  We could accomplish what's needed for exynos using a
>> property like "b

[PATCH v3 4/4] mmc: dw_mmc: Handle wp-gpios from device tree

[Doug Anderson]

On some SoCs (like exynos5250) you need to use an external GPIO for
write protect.  Add support for wp-gpios to the core dw_mmc driver
since it could be useful across multiple SoCs.

With this change I am able to make use of the write protect for the
external SD slot on exynos5250-snow.

Signed-off-by: Doug Anderson 
---
Changes in v3: None
Changes in v2:
- Fixed return type from u32 to int
- Return -EINVAL instead of -1

 drivers/mmc/host/dw_mmc.c |   34 ++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b47b1e9..8f8bac5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "dw_mmc.h"
 
@@ -75,6 +76,7 @@ struct idmac_desc {
  * @mmc: The mmc_host representing this slot.
  * @host: The MMC controller this slot is using.
  * @quirks: Slot-level quirks (DW_MCI_SLOT_QUIRK_XXX)
+ * @wp_gpio: If gpio_is_valid() we'll use this to read write protect.
  * @ctype: Card type for this slot.
  * @mrq: mmc_request currently being processed or waiting to be
  * processed, or NULL when the slot is idle.
@@ -90,6 +92,7 @@ struct dw_mci_slot {
struct dw_mci   *host;
 
int quirks;
+   int wp_gpio;
 
u32 ctype;
 
@@ -836,6 +839,8 @@ static int dw_mci_get_ro(struct mmc_host *mmc)
read_only = 0;
else if (brd->get_ro)
read_only = brd->get_ro(slot->id);
+   else if (gpio_is_valid(slot->wp_gpio))
+   read_only = gpio_get_value(slot->wp_gpio);
else
read_only =
mci_readl(slot->host, WRTPRT) & (1 << slot->id) ? 1 : 0;
@@ -1830,6 +1835,29 @@ static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 
slot)
   " as 1\n");
return bus_wd;
 }
+
+/* find the write protect gpio for a given slot; or -1 if none specified */
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+   struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
+   int gpio;
+
+   if (!np)
+   return -EINVAL;
+
+   gpio = of_get_named_gpio(np, "wp-gpios", 0);
+
+   /* Having a missing entry is valid; return silently */
+   if (!gpio_is_valid(gpio))
+   return -EINVAL;
+
+   if (devm_gpio_request(dev, gpio, "dw-mci-wp")) {
+   dev_warn(dev, "gpio [%d] request failed\n", gpio);
+   return -EINVAL;
+   }
+
+   return gpio;
+}
 #else /* CONFIG_OF */
 static int dw_mci_of_get_slot_quirks(struct device *dev, u8 slot)
 {
@@ -1843,6 +1871,10 @@ static struct device_node 
*dw_mci_of_find_slot_node(struct device *dev, u8 slot)
 {
return NULL;
 }
+static int dw_mci_of_get_wp_gpio(struct device *dev, u8 slot)
+{
+   return -EINVAL;
+}
 #endif /* CONFIG_OF */
 
 static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -1960,6 +1992,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned 
int id)
else
clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
 
+   slot->wp_gpio = dw_mci_of_get_wp_gpio(host->dev, slot->id);
+
mmc_add_host(mmc);
 
 #if defined(CONFIG_DEBUG_FS)
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 2/4] ARM: dts: Add disable-wp for sd card slot on smdk5250

[Doug Anderson]

The next change will remove the code from the dw_mmc-exynos that added
the DW_MCI_QUIRK_NO_WRITE_PROTECT.  Keep existing functionality of
having no write protect pin on smdk5250 by adding the disable-wp
property.

Signed-off-by: Doug Anderson 
---
Changes in v3:
- New for this version of the patch series.

 arch/arm/boot/dts/exynos5250-smdk5250.dts |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts 
b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index f30ca18..5538b13 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -146,6 +146,7 @@
reg = <0>;
bus-width = <4>;
samsung,cd-pinmux-gpio = <&gpc3 2 2 3 3>;
+   disable-wp;
gpios = <&gpc3 0 2 0 3>, <&gpc3 1 2 0 3>,
<&gpc3 3 2 3 3>, <&gpc3 4 2 3 3>,
<&gpc3 5 2 3 3>, <&gpc3 6 2 3 3>,
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/2] These two patches to s3c_pm_arch_prepare_irqs() were part of the work

[Doug Anderson]

to make suspend/resume reliable on the ARM Chromebook
(exynos5250-snow).

A few more details:
- The first patch is not strictly needed but was a nice cleanup.  Our
  understanding was that EINT0 was originally turned on for exynos
  evt0 silicon and not needed for evt1.
- The second patch is more important and (also) more obvious.  The
  function was modifying the S5P_WAKEUP_MASK register and then
  clobbering its own modifications.

For some history, see:
- https://gerrit.chromium.org/gerrit/31337
- https://gerrit.chromium.org/gerrit/31341


Jonathan Kliegman (2):
  arm: exynos: Remove hardcode wakeup unmask for EINT_0
  arm: exynos: Clear ENABLE_WAKEUP_SW bit when entering suspend

 arch/arm/mach-exynos/include/mach/pm-core.h | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] arm: exynos: Clear ENABLE_WAKEUP_SW bit when entering suspend

[Doug Anderson]

From: Jonathan Kliegman 

Setting this bit to 0 causes the system to wait until suspended
to use the wakeup masks.  With it being set high previously,
masked interrupts were being received and processed before the
EINT_WAKEUP_MASK was configured.

Signed-off-by: Jonathan Kliegman 
Signed-off-by: Doug Anderson 
Reviewed-by: Doug Anderson 
---
 arch/arm/mach-exynos/include/mach/pm-core.h | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h 
b/arch/arm/mach-exynos/include/mach/pm-core.h
index 9d8da51e3..7dbbfec 100644
--- a/arch/arm/mach-exynos/include/mach/pm-core.h
+++ b/arch/arm/mach-exynos/include/mach/pm-core.h
@@ -27,13 +27,8 @@ static inline void s3c_pm_debug_init_uart(void)
 
 static inline void s3c_pm_arch_prepare_irqs(void)
 {
-   unsigned int tmp;
-   tmp = __raw_readl(S5P_WAKEUP_MASK);
-   tmp &= ~(1 << 31);
-   __raw_writel(tmp, S5P_WAKEUP_MASK);
-
-   __raw_writel(s3c_irqwake_intmask, S5P_WAKEUP_MASK);
__raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
+   __raw_writel(s3c_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
 }
 
 static inline void s3c_pm_arch_stop_clocks(void)
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] arm: exynos: Remove hardcode wakeup unmask for EINT_0

[Doug Anderson]

From: Jonathan Kliegman 

For legacy reasons EINT_0 was being forced on for all
exynos systems as a wake interrupt.  For boards that need
EINT_0 they should probably enable it with enable_irq_wake

Signed-off-by: Jonathan Kliegman 
Signed-off-by: Doug Anderson 
Reviewed-by: Doug Anderson 
Reviewed-by: Thomas Abraham 
---
 arch/arm/mach-exynos/include/mach/pm-core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h 
b/arch/arm/mach-exynos/include/mach/pm-core.h
index a67ecfa..9d8da51e3 100644
--- a/arch/arm/mach-exynos/include/mach/pm-core.h
+++ b/arch/arm/mach-exynos/include/mach/pm-core.h
@@ -33,7 +33,7 @@ static inline void s3c_pm_arch_prepare_irqs(void)
__raw_writel(tmp, S5P_WAKEUP_MASK);
 
__raw_writel(s3c_irqwake_intmask, S5P_WAKEUP_MASK);
-   __raw_writel(s3c_irqwake_eintmask & 0xFFFE, S5P_EINT_WAKEUP_MASK);
+   __raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
 }
 
 static inline void s3c_pm_arch_stop_clocks(void)
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
i2c0 = &i2c_0;
i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson 

---
Changes in v3:
- Addressed Wolfram's feedback; rebased atop idr-cleanup series.

Changes in v2: None

 drivers/i2c/i2c-core.c | 54 +-
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 0b599f2..5204818 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -921,13 +921,41 @@ out_list:
 }
 
 /**
+ * __i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int __i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+   int id;
+
+   /* Handled by wrappers */
+   if (WARN_ON(adap->nr == -1))
+   return -EINVAL;
+
+   if (adap->nr & ~MAX_IDR_MASK)
+   return -EINVAL;
+
+   mutex_lock(&core_lock);
+   id = idr_alloc(&i2c_adapter_idr, adap, adap->nr, adap->nr + 1,
+  GFP_KERNEL);
+   mutex_unlock(&core_lock);
+   if (id < 0)
+   return id == -ENOSPC ? -EBUSY : id;
+   return i2c_register_adapter(adap);
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -935,8 +963,17 @@ out_list:
  */
 int i2c_add_adapter(struct i2c_adapter *adapter)
 {
+   struct device *dev = &adapter->dev;
int res;
 
+   if (dev->of_node) {
+   res = of_alias_get_id(dev->of_node, "i2c");
+   if (res >= 0) {
+   adapter->nr = res;
+   return __i2c_add_numbered_adapter(adapter);
+   }
+   }
+
mutex_lock(&core_lock);
res = idr_alloc(&i2c_adapter_idr, adapter,
__i2c_first_dynamic_bus_num, 0, GFP_KERNEL);
@@ -974,20 +1011,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-   int id;
-
if (adap->nr == -1) /* -1 means dynamically assign bus id */
return i2c_add_adapter(adap);
-   if (adap->nr & ~MAX_IDR_MASK)
-   return -EINVAL;
-
-   mutex_lock(&core_lock);
-   id = idr_alloc(&i2c_adapter_idr, adap, adap->nr, adap->nr + 1,
-  GFP_KERNEL);
-   mutex_unlock(&core_lock);
-   if (id < 0)
-   return id == -ENOSPC ? -EBUSY : id;
-   return i2c_register_adapter(adap);
+   return __i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson 
---
Changes in v3: None
Changes in v2:
- No longer tweak pdev->id as per Sylwester Nawrocki.
- No longer add the dev ID to the adap.name.  Other drivers don't
  include the device ID here and it doesn't make sense with
  dynamically (or automatically) allocated IDs.
- Use dev_name(&dev->dev) to register for the IRQ; this matches what
  the i2c-s3c2410.c does and handles dynamically allocated IDs.
- This change was only compile-tested (corgi_defconfig), since I don't
  have access to a board that uses this driver.

 drivers/i2c/busses/i2c-pxa.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..705cc9f 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,13 @@ static int i2c_pxa_probe_dt(struct platform_device 
*pdev, struct pxa_i2c *i2c,
struct device_node *np = pdev->dev.of_node;
const struct of_device_id *of_id =
of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-   int ret;
 
if (!of_id)
return 1;
-   ret = of_alias_get_id(np, "i2c");
-   if (ret < 0) {
-   dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-   return ret;
-   }
-   pdev->id = ret;
+
+   /* For device tree we always use the dynamic or alias-assigned ID */
+   i2c->adap.nr = -1;
+
if (of_get_property(np, "mrvl,i2c-polling", NULL))
i2c->use_pio = 1;
if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
@@ -1100,6 +1097,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
goto emalloc;
}
 
+   /* Default adapter num to device id; i2c_pxa_probe_dt can override. */
+   i2c->adap.nr = dev->id;
+
ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
if (ret > 0)
ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
@@ -1124,9 +1124,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
spin_lock_init(&i2c->lock);
init_waitqueue_head(&i2c->wait);
 
-   i2c->adap.nr = dev->id;
-   snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
-i2c->adap.nr);
+   strlcpy(i2c->adap.name, "pxa_i2c-i2c", sizeof(i2c->adap.name));
 
i2c->clk = clk_get(&dev->dev, NULL);
if (IS_ERR(i2c->clk)) {
@@ -1169,7 +1167,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
} else {
i2c->adap.algo = &i2c_pxa_algorithm;
ret = request_irq(irq, i2c_pxa_handler, IRQF_SHARED,
- i2c->adap.name, i2c);
+ dev_name(&dev->dev), i2c);
if (ret)
goto ereqirq;
}
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

Wolfram,

Thanks for the review.  New patch was just sent.  :)

On Sun, Feb 10, 2013 at 4:19 AM, Wolfram Sang  wrote:
>> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)
>
> i2c_get_id_from_dt()?

Done.


>> + if (!dev->of_node)
>> + return -1;
>
> -ESOMETHING?

Function has been removed, as per below.


>> +
>> + id = of_alias_get_id(dev->of_node, "i2c");
>> + if (id < 0)
>> + return -1;
>> + return id;
>
> Simply 'return of_alias_get_id(...)'? Even more, since this function
> boils down to calling of_alias_get_id only and is only used once, I'd
> think we can implement that directly and drop this function. That
> shouldn't hurt readability.

Good point.  Done.

>> +/**
>> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
>> + * @adap: the adapter to register (with adap->nr initialized)
>> + * Context: can sleep
>> + *
>> + * See i2c_add_numbered_adapter() for details.
>> + */
>> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
>
> All other internal functions are prefixed with '__'.

Done.


>> +{
>> + int id;
>> + int status;
>> +
>> + /* Handled by wrappers */
>> + BUG_ON(adap->nr == -1);
>
> Is that a reason to halt the kernel? WARN and bailing out would do IMO.

Done.


>> +
>> + if (adap->nr & ~MAX_IDR_MASK)
>> + return -EINVAL;
>
> Note the idr-cleanup series from Tejun Heo. Given that his series is
> scheduled for v3.9, I'd like to have your patches on top of his.

Done.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality.
The i2c-pxa driver changes have only been compile-tested and are just
for cleanup purposes.

Changes in v3:
- Addressed Wolfram's feedback; rebased atop idr-cleanup series.

Changes in v2:
- No longer tweak pdev->id as per Sylwester Nawrocki.
- No longer add the dev ID to the adap.name.  Other drivers don't
  include the device ID here and it doesn't make sense with
  dynamically (or automatically) allocated IDs.
- Use dev_name(&dev->dev) to register for the IRQ; this matches what
  the i2c-s3c2410.c does and handles dynamically allocated IDs.
- This change was only compile-tested (corgi_defconfig), since I don't
  have access to a board that uses this driver.

Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c | 20 
 drivers/i2c/i2c-core.c   | 54 
 2 files changed, 49 insertions(+), 25 deletions(-)

-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC: PATCH 2/2] iio: adc: exynos_adc: Handle timeout and race conditions

[Doug Anderson]

Lars,

On Sat, Mar 16, 2013 at 7:41 AM, Lars-Peter Clausen  wrote:
> I think you still need the mutex for serialization, otherwise the requests
> would just cancel each other out. Btw. what happens if you start a conversion
> while another is still in progress? Is it possible to abort a conversion?

I was thinking that the spinlock would just replace the mutex for the
purposes of serialization.

I stepped back a bit, though, and I'm wondering if we're over-thinking
things.  The timeout case should certainly be handled properly (thanks
for pointing it out), but getting a timeout is really not expected and
adding a lot of extra overhead to handle it elegantly seems a bit
much?

Specifically, the mutex means that we have one user of the ADC at a
time, and ADC conversion has nothing variable about it.  The user
manual that I have access to talks about 12-bit conversion happening
in 1 microsecond with a 5MHz input clock or 5 microseconds with a 1MHz
input clock.  Even if someone has clocks configured very differently,
it would be hard to imagine a conversion actually taking a full
second.

...so that means that if the timeout actually fires then something
else fairly drastic has gone wrong.  It's _very_ unlikely that the IRQ
will still go off for this conversion sometime in the future.

To me, total modifications to what's landed already ought to be:

* Change timeout to long (from unsigned long)

* Make sure we return errors (negative results) from
wait_for_completion_interruptible_timeout() properly.

* If we get back a value of 0 from
wait_for_completion_interruptible_timeout() then we should print a
warning and attempt machinations to reset the ADC.  Without ever
seeing real-world situtations that would cause a real timeout these
machinations would be a bit of a guess (is resetting the adc useful
when it's more likely that someone accidentally messed with the clock
tree or power gated the ADC?)...  ...or perhaps a warning and a TODO
in the code would be enough?


Thoughts?

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/2] These two patches to s3c_pm_arch_prepare_irqs() were part of the work

[Doug Anderson]

Kukjin,

On Tue, Apr 2, 2013 at 7:16 PM, Kukjin Kim  wrote:
>> Applied with 1st one, BTW, do you want to send this for stable tree?

I don't have any need for it to be in stable tree.  The ARM Chromebook
hasn't reached critical functionality on any released/upstram Linux
versions so it doesn't make much sense to backport fixes.  If someone
else wants it in stable (and can confirm that it helps them) then I
certainly wouldn't object!


> One more note, just now I discussed Jaecheol Lee about the bit,
> ENABLE_WAKEUP_SW, as the patch fixed, it should be cleared but used to be
> set s3c_irqwake_intmask. Let me check again, then if any updates I'll let
> you know.

OK, thanks.  If there is a reason that ENABLE_WAKEUP_SW needs to be
set then it would be good to understand that case.  :)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC: PATCH 2/2] iio: adc: exynos_adc: Handle timeout and race conditions

[Doug Anderson]

Lars,

On Fri, Apr 5, 2013 at 1:53 AM, Lars-Peter Clausen  wrote:
> Since we sleep inside the protected section we need to use a mutex.

Ah, good point.

> It's not the timeout case I'm worried about, but the case where the transfer
> is interrupted by the user. Even though it is rather unlikely for the
> problem to occur we should still try to avoid it, this is one of these
> annoying heisenbugs that happen once in a while and nobody is able to
> reproduce them.

Yes, of course.  Then we can also get extra confidence that the reset
logic works well by stressing out this case...  :)

This makes me think, though.  Given how fast we expect the ADC
transaction to finish, would there be any benefit to making the wait
non-interruptible and then shortening the timeout a whole lot.  If we
shortened to 1ms then we're really not "non-interruptible" for very
long and there's less chance of subtle bugs in the way that reset
works.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] iio: adc: add exynos5 adc driver under iio framwork

[Doug Anderson]

Lars,

Thank you for your comments / thoughts...


On Thu, Jan 24, 2013 at 1:54 AM, Lars-Peter Clausen  wrote:
> adc: adc@12D1 {
>
> #io-channel-cells = <1>;
> io-channel-output-names = "adc1", "adc2", ...;
>
> ncp15wb473@0 {
> compatible = "ntc,ncp15wb473";
> ...
> io-channels = <&adc 0>; // First ADC channel

I'm not an expert, but I think the typical way is:
* No need to include a handle to &adc.  It's logically our parent.  In
a similar way i2c devices don't specify their parent bus--they are
just listed under it.
* The "0" should be specified with reg = <0>

Everything else about this syntax looks good.

To implement this I'd imagine that we'll need a new API call, right?
In this case the thermistor driver won't know the name of the channel.
 It can find the ADC (the struct device and probably other things) and
knows a channel index.  Am I understanding properly?

I think Naveen expressed the same question, though he said it a bit
differently than me.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

Wolfram,

On Mon, Feb 11, 2013 at 4:48 PM, Doug Anderson  wrote:
> This was suggested by Mark Brown in response to a patch for adding
> this functionality only for the s3c2410 bus:
>   https://lkml.org/lkml/2012/11/20/681
>
> I have also modified the i2c-pxa driver to use this new functionality.
> The i2c-pxa driver changes have only been compile-tested and are just
> for cleanup purposes.
>
> Changes in v3:
> - Addressed Wolfram's feedback; rebased atop idr-cleanup series.
>
> Changes in v2:
> - No longer tweak pdev->id as per Sylwester Nawrocki.
> - No longer add the dev ID to the adap.name.  Other drivers don't
>   include the device ID here and it doesn't make sense with
>   dynamically (or automatically) allocated IDs.
> - Use dev_name(&dev->dev) to register for the IRQ; this matches what
>   the i2c-s3c2410.c does and handles dynamically allocated IDs.
> - This change was only compile-tested (corgi_defconfig), since I don't
>   have access to a board that uses this driver.
>
> Doug Anderson (2):
>   i2c-core: dt: Pick i2c bus number from i2c alias if present
>   i2c: pxa: Use i2c-core to get bus number now
>
>  drivers/i2c/busses/i2c-pxa.c | 20 
>  drivers/i2c/i2c-core.c   | 54 
> 
>  2 files changed, 49 insertions(+), 25 deletions(-)

Is there anything else needed for this patch series?  It's been
hanging around for quite a while and it would be nice to see it land.
If you're waiting for additional acks or reviews I'll ask around and
see if I can get them...

Thanks!  :)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

Wolfram,

On Thu, Feb 28, 2013 at 3:25 PM, Wolfram Sang  wrote:
>
> Regarding patch 1, I was waiting for the idr changes to hit mainline.
> They are mainline now, but since the removal of MAX_IDR_MASK your patch
> doesn't apply anymore :( Can you rebase and retest, please? I'd like to
> get it into 3.9, still.

Yes, I'll rebase/retest tomorrow.  I'll let you make the call between
3.9 and 3.10.  Obviously I'd love to see it in 3.9 but I'm OK with
either.  I was just hoping not to find out at the end of the 3.10
cycle that it needs fixes and need to wait for 3.11.  I wanted to make
sure all my ducks were in a row.  ;-)


> For patch 2, some Tested-by would be nice; but it will probably make it
> into my for-next somewhen after rc1 is out.

I have no urgent need for patch 2 to land but it seemed nice to tidy
up the pxa driver and I felt it was my obligation to post it together
with patch set #1 (plus it's always nice to remove code).  Putting it
into your for-next seems like a very good solution.  That gives people
some time to test it and make sure it works for them.


Thanks!

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] iio: adc: exynos5_adc: fix compilation warnings

[Doug Anderson]

Naveen,

On Wed, Mar 6, 2013 at 7:09 PM, Naveen Krishna Chatradhi
 wrote:
> -   unsigned intversion;
> +   unsigned intversion;

Given that you've changed exynos_adc_get_version() to return an int,
shouldn't this be an int too (not unsigned)?


> -static inline unsigned int exynos_adc_get_version(struct platform_device 
> *pdev)
> +static inline int exynos_adc_get_version(struct platform_device *pdev)
>  {
> const struct of_device_id *match;
>
> match = of_match_node(exynos_adc_match, pdev->dev.of_node);
> -   return (unsigned int)match->data;
> +   return (int)match->data;

Given that you're now checking for an error code below it seems like
you ought to generate one here.  ;)
...AKA: return an error if match is NULL--don't dereference NULL.


>  static int exynos_read_raw(struct iio_dev *indio_dev,
> @@ -117,7 +117,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> long mask)
>  {
> struct exynos_adc *info = iio_priv(indio_dev);
> -   unsigned long timeout;
> +   int timeout;

Why change this to an "int" when
wait_for_completion_interruptible_timeout() returns a long?  I agree
with the removal of the "unsigned", though.


> +   version = exynos_adc_get_version(pdev);
> +   if (version < 0) {
> +   dev_err(&pdev->dev, "no matching of_node, err = %d\n", 
> version);
> +   ret = version;
> +   goto err_iio;
> +   }
> +
> +   info->version = version;

Optional (and perhaps a matter of preference): I'd eliminate the
"version" variable here and just re-use "ret" for storing the result
of exynos_adc_get_version().


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] ARM: dts: Add adc and thermistors for exynos5250-snow

[Doug Anderson]

Hook up the exynos5250-snow thermistors via the device tree now that
there's a driver available to use them.

Signed-off-by: Doug Anderson 
---
 arch/arm/boot/dts/cros5250-common.dtsi |  4 
 arch/arm/boot/dts/exynos5250-snow.dts  | 31 +++
 2 files changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/cros5250-common.dtsi 
b/arch/arm/boot/dts/cros5250-common.dtsi
index 62eceb4..cb48981 100644
--- a/arch/arm/boot/dts/cros5250-common.dtsi
+++ b/arch/arm/boot/dts/cros5250-common.dtsi
@@ -305,6 +305,10 @@
status = "disabled";
};
 
+   adc@12D1 {
+   vdd-supply = <&buck5_reg>;
+   };
+
hdmi {
hpd-gpio = <&gpx3 7 0xf 1 3>;
};
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
b/arch/arm/boot/dts/exynos5250-snow.dts
index babd9f9..1cab5fd 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -51,4 +51,35 @@
clock-frequency = <2400>;
};
};
+
+   adc@12D1 {
+   ncp15wb473@3 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 3>;
+   };
+   ncp15wb473@4 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 4>;
+   };
+   ncp15wb473@5 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 5>;
+   };
+   ncp15wb473@6 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uV = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 6>;
+   };
+   };
 };
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] iio: adc: Add dt support for turning on the phy in exynos-adc

[Doug Anderson]

Without this change the exynos adc controller needed to have its phy
enabled in some out-of-driver C code.  Add support for specifying the
phy enable register by listing it in the reg list.

Signed-off-by: Doug Anderson 
---
 .../devicetree/bindings/arm/samsung/exynos-adc.txt |  4 ++--
 drivers/iio/adc/exynos_adc.c   | 14 +-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index 96db940..05be151 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -15,7 +15,7 @@ Required properties:
Must be "samsung,exynos-adc-v2" for
future controllers.
 - reg: Contains ADC register address range (base address and
-   length).
+   length) and the address of the phy enable register.
 - interrupts:  Contains the interrupt information for the timer. The
format is being dependent on which interrupt controller
the Samsung device uses.
@@ -30,7 +30,7 @@ Example: adding device info in dtsi file
 
 adc: adc@12D1 {
compatible = "samsung,exynos-adc-v1";
-   reg = <0x12D1 0x100>;
+   reg = <0x12D1 0x100>, <0x10040718 0x4>;
interrupts = <0 106 0>;
#io-channel-cells = <1>;
io-channel-ranges;
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index ed6fdd7..5ab0dfd 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -85,6 +85,7 @@ enum adc_version {
 
 struct exynos_adc {
void __iomem*regs;
+   void __iomem*enable_reg;
struct clk  *clk;
unsigned intirq;
struct regulator*vdd;
@@ -269,13 +270,19 @@ static int exynos_adc_probe(struct platform_device *pdev)
info = iio_priv(indio_dev);
 
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
info->regs = devm_request_and_ioremap(&pdev->dev, mem);
if (!info->regs) {
ret = -ENOMEM;
goto err_iio;
}
 
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+   info->enable_reg = devm_request_and_ioremap(&pdev->dev, mem);
+   if (!info->enable_reg) {
+   ret = -ENOMEM;
+   goto err_iio;
+   }
+
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "no irq resource?\n");
@@ -295,6 +302,8 @@ static int exynos_adc_probe(struct platform_device *pdev)
goto err_iio;
}
 
+   writel(1, info->enable_reg);
+
info->clk = devm_clk_get(&pdev->dev, "adc");
if (IS_ERR(info->clk)) {
dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
@@ -370,6 +379,7 @@ static int exynos_adc_remove(struct platform_device *pdev)
exynos_adc_remove_devices);
regulator_disable(info->vdd);
clk_disable_unprepare(info->clk);
+   writel(0, info->enable_reg);
iio_device_unregister(indio_dev);
free_irq(info->irq, info);
iio_device_free(indio_dev);
@@ -395,6 +405,7 @@ static int exynos_adc_suspend(struct device *dev)
}
 
clk_disable_unprepare(info->clk);
+   writel(0, info->enable_reg);
regulator_disable(info->vdd);
 
return 0;
@@ -410,6 +421,7 @@ static int exynos_adc_resume(struct device *dev)
if (ret)
return ret;
 
+   writel(1, info->enable_reg);
clk_prepare_enable(info->clk);
 
exynos_adc_hw_init(info);
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] ARM: dts: Add adc to exynos5250 device tree file

[Doug Anderson]

Add the device tree entry for the device-tree enabled ADC driver that
recently landed in the iio tree.

Signed-off-by: Doug Anderson 
---
 arch/arm/boot/dts/exynos5250.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 24c52e6..3773feb 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -115,6 +115,17 @@
interrupts = <1 2>, <22 4>;
};
 
+   adc: adc@12D1 {
+   compatible = "samsung,exynos-adc-v1";
+   reg = <0x12D1 0x100>, <0x10040718 0x4>;
+   interrupts = <0 106 0>;
+   #io-channel-cells = <1>;
+   io-channel-ranges;
+
+   clocks = <&clock 303>;
+   clock-names = "adc";
+   };
+
watchdog {
compatible = "samsung,s3c2410-wdt";
reg = <0x101D 0x100>;
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] iio: adc: Document the regulator/clocks for exynos-adc

[Doug Anderson]

The exynos ADC won't work without a regulator called "vdd" and a clock
called "adc".  Document this fact in the device tree bindings.

Signed-off-by: Doug Anderson 
---
 Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index f686378..96db940 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -20,6 +20,9 @@ Required properties:
format is being dependent on which interrupt controller
the Samsung device uses.
 - #io-channel-cells = <1>; As ADC has multiple outputs
+- clocks   From common clock binding: handle to adc clock.
+- clock-names  From common clock binding: Shall be "adc".
+- vdd-supply   VDD input supply.
 
 Note: child nodes can be added for auto probing from device tree.
 
@@ -31,6 +34,11 @@ adc: adc@12D1 {
interrupts = <0 106 0>;
#io-channel-cells = <1>;
io-channel-ranges;
+
+   clocks = <&clock 303>;
+   clock-names = "adc";
+
+   vdd-supply = <&buck5_reg>;
 };
 
 
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/4] Get exynos adc driver running on on exynos5250-snow

[Doug Anderson]

This set of patches is based on Naveen Krishna Chatradhi's recent work
(some of which is still in-flight) and does the final touches to get
things working on exynos5250-snow (the ARM Chromebook).

These patches were tested on next-20130312 (which includes support for
the common clock framework on exynos) plus the current linux-iio/togreg:
  95783f2 iio: adc: add exynos adc driver under iio framwork
  298489f iio:common: Use spi_sync_transfer() in STMicroelectronics ...
  1d9a4cb IIO ADC support for AD7923
  9a282b0 iio: Add OF support
  3d277fc3 staging:iio: Remove adt7410 driver

...plus Naveen's recent ADC cleanup and NTC thermistor patch.

Doug Anderson (4):
  iio: adc: Document the regulator/clocks for exynos-adc
  iio: adc: Add dt support for turning on the phy in exynos-adc
  ARM: dts: Add adc to exynos5250 device tree file
  ARM: dts: Add adc and thermistors for exynos5250-snow

 .../devicetree/bindings/arm/samsung/exynos-adc.txt | 12 +++--
 arch/arm/boot/dts/cros5250-common.dtsi |  4 +++
 arch/arm/boot/dts/exynos5250-snow.dts  | 31 ++
 arch/arm/boot/dts/exynos5250.dtsi  | 11 
 drivers/iio/adc/exynos_adc.c   | 14 +-
 5 files changed, 69 insertions(+), 3 deletions(-)

-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] hwmon: ntc: Add DT with IIO support to NTC thermistor driver

[Doug Anderson]

Hi,

On Tue, Mar 12, 2013 at 6:45 AM, Guenter Roeck  wrote:
> On Tue, Mar 12, 2013 at 02:09:26PM +0530, Naveen Krishna Chatradhi wrote:
>> This patch adds DT support to NTC driver to parse the
>> platform data.
>>
>> Also adds the support to work as an iio device.
>>
>> During the probe ntc driver gets the respective channels of ADC
>> and uses iio_raw_read calls to get the ADC converted value.
>>
>> Signed-off-by: Naveen Krishna Chatradhi 

I know that there is still work to do to address Guenter's comments,
but I spent a little time playing with this today to see what else was
missing and actually got it working.  :)  I'll add a "Tested-by" once
Naveen reposts.

I sent up my patches that I needed to the ADC + device trees to get it
working, but I just realized that I forgot to CC Guenter.  Doh!  In
case you're curious, you can find them at:
* https://patchwork.kernel.org/patch/2260231/
* https://patchwork.kernel.org/patch/2260211/
* https://patchwork.kernel.org/patch/2260221/
* https://patchwork.kernel.org/patch/2260201/


> You'll still need the conditional dependency on IIO in Kconfig.
> Something like
> depends on IIO if OF
> if that works, or alternatively
> select IIO if OF
> if that is acceptable.

This was important for me.  I had IIO setup as a module and adc
configured an in-kernel.  Without the above I got a nice linker error.
 ;)
The syntax that worked for me was 'select IIO if OF'.  I don't think
you can do a conditional depends on (it didn't work for me and I
couldn't find any examples like that).


>> +Read more about iio bindings at
>> + Documentation/devicetree/bindings/iio/iio-bindings.txt
>> +

BTW: the above line had a whitespace error on it.  Double-check that
it's gone for the next version.


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow

[Doug Anderson]

Kukjin,

On Wed, Mar 13, 2013 at 12:30 AM, Kukjin Kim  wrote:
> BTW, Doug, I think, this should be re-worked to use pinctrl. Can you?

Yes, I've already got this locally.  I hadn't sent it up yet since I
wasn't sure whether pinctrl would land before or after this.  I'll
send it up today against my local cherry-pick of Thomas's pinctrl
stuff (since it doesn't seem to be in linux-next nor in
arm-soc/for-next).

Naveen: I'm going to keep your Tested-by on all 3 patches if that's
OK.  The only difference will be the gpio specifier in the device tree
and the change is trivial.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 3/3] ARM: dts: Add sbs-battery for exynos5250-snow

[Doug Anderson]

Now that we have i2c-arbitrator in place on bus 4 we can add the
sbs-battery driver.  Future devices will be added onto bus 4 once
drivers are in good shape.

Signed-off-by: Doug Anderson 
Tested-by: Naveen Krishna Chatradhi 
---
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/exynos5250-snow.dts | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
b/arch/arm/boot/dts/exynos5250-snow.dts
index bf16353..7aa66c8 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -66,6 +66,12 @@
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
+
+   battery: sbs-battery@b {
+   compatible = "sbs,sbs-battery";
+   reg = <0xb>;
+   sbs,poll-retry-count = <1>;
+   };
};
};
 
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow

[Doug Anderson]

We need to use the i2c-arbitrator to talk to any of the devices on i2c
bus 4 on exynos5250-snow so that we don't confuse the embedded
controller (EC).  Add the i2c-arbitrator to the device tree.  As we
add future devices (keyboard, sbs, tps65090) we'll add them on top of
this.

The arbitrated bus is numbered 104 simply as a convenience to make it
easier for people poking around to guess that it might have something
to do with the physical bus 4.

The addition is split between the cros5250-common and the snow device
tree file since not all cros5250-class devices use arbitration.

Signed-off-by: Doug Anderson 
Tested-by: Naveen Krishna Chatradhi 
---
Changes in v4:
- Changed mux gpio syntax to work atop Thomas's "ARM: dts: add pin
  state information in client nodes for Exynos5 platforms"; avoid
  adding gpios property to i2c@12CA for the same reason.

Changes in v3: None
Changes in v2:
- Use new device tree property names / compatible string.
- Include that the GPIOs for arbitration are active low.

 arch/arm/boot/dts/cros5250-common.dtsi |  3 ++-
 arch/arm/boot/dts/exynos5250-snow.dts  | 25 +
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/cros5250-common.dtsi 
b/arch/arm/boot/dts/cros5250-common.dtsi
index 8a5b3a6..0a61bbb 100644
--- a/arch/arm/boot/dts/cros5250-common.dtsi
+++ b/arch/arm/boot/dts/cros5250-common.dtsi
@@ -193,7 +193,8 @@
};
 
i2c@12CA {
-   status = "disabled";
+   samsung,i2c-sda-delay = <100>;
+   samsung,i2c-max-bus-freq = <66000>;
};
 
i2c@12CB {
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
b/arch/arm/boot/dts/exynos5250-snow.dts
index 581ffae..bf16353 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -16,6 +16,10 @@
model = "Google Snow";
compatible = "google,snow", "samsung,exynos5250";
 
+   aliases {
+   i2c104 = &i2c_104;
+   };
+
pinctrl@1140 {
sd3_clk: sd3-clk {
samsung,pin-drv = <0>;
@@ -44,6 +48,27 @@
};
};
 
+   i2c-arbitrator {
+   compatible = "i2c-arbitrator-cros-ec";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   i2c-parent = <&{/i2c@12CA}>;
+
+   ap-claim-gpio = <&gpf0 3 1>;
+   ec-claim-gpio = <&gpe0 4 1>;
+   slew-delay-us = <10>;
+   wait-retry-us = <3000>;
+   wait-free-us = <5>;
+
+   /* Use ID 104 as a hint that we're on physical bus 4 */
+   i2c_104: i2c@0 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+
/*
 * On Snow we've got SIP WiFi and so can keep drive strengths low to
 * reduce EMI.
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Doug Anderson]

The i2c-arbitrator-cros-ec driver implements the arbitration scheme
that the Embedded Controller (EC) on the ARM Chromebook expects to use
for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
be used in other places where standard I2C bus arbitration can't be
used and two extra GPIOs are available for arbitration.

This driver is based on code that Simon Glass added to the i2c-s3c2410
driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
mux driver is as suggested by Grant Likely.  See
<https://patchwork.kernel.org/patch/1877311/> for some history.

Signed-off-by: Doug Anderson 
Signed-off-by: Simon Glass 
Signed-off-by: Naveen Krishna Chatradhi 
Reviewed-by: Stephen Warren 
Tested-by: Naveen Krishna Chatradhi 
---
Changes in v4: None
Changes in v3:
- Handle of_find_i2c_adapter_by_node() failure more properly by
  changing init order.
- Don't warn on -EPROBE_DEFER from calls that could return it.
- Move to module_platform_driver().  As we pull in parts of the system
  that rely on devices under this i2c bus we'll need to make sure they
  can handle the fact that they'll be initted later now.

Changes in v2:
- Renamed to i2c-arbitrator-cros-ec.
- Documented "microsecond" properties as optional; removed
  "bus-arbitration" prefix since it was just extra wordy.
- Split GPIOs into two properties to make it cleaner.
- Capitalized I2C in freeform text.
- Get 'active low' from device tree.

 .../bindings/i2c/i2c-arbitrator-cros-ec.txt|  76 +++
 drivers/i2c/muxes/Kconfig  |  11 +
 drivers/i2c/muxes/Makefile |   2 +
 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c | 222 +
 4 files changed, 311 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
 create mode 100644 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt 
b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
new file mode 100644
index 000..1f893e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
@@ -0,0 +1,76 @@
+GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow)
+===
+This uses GPIO lines between the AP (Application Processor) and an attached
+EC (Embedded Controller) which both want to talk on the same I2C bus as master.
+
+The AP and EC each have a 'bus claim' line, which is an output that the
+other can see. These are both active low, with pull-ups enabled.
+
+- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus
+- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus
+
+This mechanism is used instead of standard I2C multimaster to avoid some of the
+subtle driver and silicon bugs that are often present with I2C multimaster.
+
+
+Algorithm:
+
+The basic algorithm is to assert your line when you want the bus, then make
+sure that the other side doesn't want it also. A detailed explanation is best
+done with an example.
+
+Let's say the AP wants to claim the bus. It:
+1. Asserts AP_CLAIM.
+2. Waits a little bit for the other side to notice (slew time, say 10
+   microseconds).
+3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we are
+   done.
+4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released.
+5. If not, back off, release the claim and wait for a few more milliseconds.
+6. Go back to 1 (until retry time has expired).
+
+
+Required properties:
+- compatible: i2c-arbitrator-cros-ec
+- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus.
+- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the bus.
+- Standard I2C mux properties. See mux.txt in this directory.
+- Single I2C child bus node at reg 0. See mux.txt in this directory.
+
+Optional properties:
+- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us.
+- wait-retry-us: we'll attempt another claim after this many microseconds.
+Default is 3000 us.
+- wait-free-us: we'll give up after this many microseconds. Default is 5 
us.
+
+
+Example:
+   i2c@12CA {
+   compatible = "acme,some-i2c-device";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
+   i2c-arbitrator {
+   compatible = "i2c-arbitrator-cros-ec";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   i2c-parent = <&{/i2c@12CA}>;
+
+   ap-claim-gpio = <&gpf0 3 1 0x1 0>;
+   ec-claim-gpio = <&gpe0 4 0 0x10003 0>;
+   slew-delay-us = <10>;
+   wait-retry-us = <3000>;
+  

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Doug Anderson]

Stephen,

On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren  wrote:
>> Changes in v4: None
>
> Isn't this 'PATCH V3 REPOST' then?

In this case part 2 in the patch series changes but not parts 1 and 3.
 I could have just reposted part 2 with a higher version, but that
makes it a little harder to piece together all of the parts of the
series so I decided to repost all 3.  I can do differently in the
future if you prefer, but my understanding was that it was a matter of
preference/judgement call.

Thanks!  :)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ARM: dts: add usb 2.0 clock references to device tree

[Doug Anderson]

This is a fixup to two device tree nodes that have already landed but
without clock nodes since the transition to common clock happened at
the same time.

Signed-off-by: Doug Anderson 
---
 arch/arm/boot/dts/exynos5250.dtsi | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 24c52e6..59be603 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -402,12 +402,18 @@
compatible = "samsung,exynos4210-ehci";
reg = <0x1211 0x100>;
interrupts = <0 71 0>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
 
usb@1212 {
compatible = "samsung,exynos4210-ohci";
reg = <0x1212 0x100>;
interrupts = <0 71 0>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
 
amba {
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ARM: dts: add usb 2.0 clock references to exynos5250 device tree

[Doug Anderson]

This is a fixup to two device tree nodes that have already landed but
without clock nodes since the transition to common clock happened at
the same time.

Signed-off-by: Doug Anderson 
---
 arch/arm/boot/dts/exynos5250.dtsi | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 24c52e6..59be603 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -402,12 +402,18 @@
compatible = "samsung,exynos4210-ehci";
reg = <0x1211 0x100>;
interrupts = <0 71 0>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
 
usb@1212 {
compatible = "samsung,exynos4210-ohci";
reg = <0x1212 0x100>;
interrupts = <0 71 0>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
 
amba {
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: dts: add usb 2.0 clock references to device tree

[Doug Anderson]

Argh...

On Wed, Mar 13, 2013 at 10:17 AM, Doug Anderson  wrote:
> This is a fixup to two device tree nodes that have already landed but
> without clock nodes since the transition to common clock happened at
> the same time.
>
> Signed-off-by: Doug Anderson 

Please forgive my fat finger and see the version with the updated
description: "[PATCH] ARM: dts: add usb 2.0 clock references to
exynos5250 device tree".  There is no difference other than the title.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] USB: ehci-s5p: Fix phy reset

[Doug Anderson]

Alexander,

On Tue, Mar 12, 2013 at 6:09 PM, Alexander Graf  wrote:
> -   err = gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, "ehci_vbus_gpio");
> -   if (err)
> +   /* reset pulls the line down, then up again */
> +   err = gpio_request_one(gpio, GPIOF_OUT_INIT_LOW, "ehci_vbus_gpio");
> +   if (err) {
> dev_err(&pdev->dev, "can't request ehci vbus gpio %d", gpio);
> +   return;
> +   }
> +   mdelay(1);
> +   __gpio_set_value(gpio, 1);
> +   gpio_free(gpio);

Freeing the gpio is a little on the iffy side since you actually care
about keeping the value.  Perhaps you can change this to
devm_gpio_request_one() and avoid the free?  I was about to submit a
patch to do just that (since otherwise you run into trouble if you
ever defer the probe) but then ran across your patch.

Thanks!

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] USB: ehci-s5p: Fix phy reset

[Doug Anderson]

Alexander,

On Wed, Mar 13, 2013 at 10:45 AM, Alexander Graf  wrote:
>
>>> +   gpio_free(gpio);
>>
>> Freeing the gpio is a little on the iffy side since you actually care
>> about keeping the value.  Perhaps you can change this to
>> devm_gpio_request_one() and avoid the free?  I was about to submit a
>> patch to do just that (since otherwise you run into trouble if you
>> ever defer the probe) but then ran across your patch.
>
> I could also just return it when the function exits and only free it when we 
> exit the probe function with a negative value. The reason I put it in here 
> was that on probe deferral, the pin simply gets blocked.
>
> However, I could probably also just completely take the gpio_free() out of 
> this patch and resubmit, as it should be pretty much unrelated. Then you can 
> patch it properly.

Sure, if you want to resubmit without the gpio_free() I'll submit a
new patch atop yours with the change to devm and people can see if
they like it...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] iio: adc: exynos5_adc: fix compilation warnings

[Doug Anderson]

Naveen,

On Tue, Mar 12, 2013 at 9:48 PM, Naveen Krishna Chatradhi
 wrote:
> Doug, There was a comment from Lars regarding the match not
> being NULL, if driver depends on CONFIG_OF. So, i've removed
> the NULL check in v2 of this patch.
> https://patchwork.kernel.org/patch/841/
>
> I'm checking the return value of get_version() for -ve values before
> assigning to info->version. So, i left the (unsigned int) unchanged.

Hmmm, I guess this was the point that confused me.  I went back and
agree with Lars--it can't be NULL.  ...but that means that
exynos_adc_get_version() can't return an error, so why are we checking
for an error?

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[REPOST PATCH] spi: Unlock a spinlock before calling into the controller driver.

[Doug Anderson]

From: Bryan Freed 

spi_pump_messages() calls into a controller driver with
unprepare_transfer_hardware() which is documented as "This may sleep".
As in the prepare_transfer_hardware() call below, we should release the
queue_lock spinlock before making the call.
Rework the logic a bit to hold queue_lock to protect the 'busy' flag,
then release it to call unprepare_transfer_hardware().

Signed-off-by: Bryan Freed 
Reviewed-by: Doug Anderson 
Signed-off-by: Doug Anderson 
Acked-by: Linus Walleij 
---
During a rebase we noticed that this old patch never actually landed
anywhere.  I haven't gone through an reproduced the original bug on
ToT but I believe that this patch is still required.  Perhaps it could
land somewhere?  ;)  Thanks!

 drivers/spi/spi.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index f996c60..5b96250 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -543,17 +543,16 @@ static void spi_pump_messages(struct kthread_work *work)
/* Lock queue and check for queue work */
spin_lock_irqsave(&master->queue_lock, flags);
if (list_empty(&master->queue) || !master->running) {
-   if (master->busy && master->unprepare_transfer_hardware) {
-   ret = master->unprepare_transfer_hardware(master);
-   if (ret) {
-   spin_unlock_irqrestore(&master->queue_lock, 
flags);
-   dev_err(&master->dev,
-   "failed to unprepare transfer 
hardware\n");
-   return;
-   }
+   if (!master->busy) {
+   spin_unlock_irqrestore(&master->queue_lock, flags);
+   return;
}
master->busy = false;
spin_unlock_irqrestore(&master->queue_lock, flags);
+   if (master->unprepare_transfer_hardware &&
+   master->unprepare_transfer_hardware(master))
+   dev_err(&master->dev,
+   "failed to unprepare transfer hardware\n");
return;
}
 
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] iio: adc: exynos5_adc: fix compilation warnings

[Doug Anderson]

Lars,

On Wed, Mar 13, 2013 at 11:11 AM, Lars-Peter Clausen  wrote:
> Agreed. Adding the dependency on OF in Kconfig should be all that is needed.

I think changing the timeout from 'unsigned long' to 'long' is also
legit (to match the actual type returned) and a good idea.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] iio: adc: exynos5_adc: fix compilation warnings

[Doug Anderson]

Lars,

On Wed, Mar 13, 2013 at 11:40 AM, Lars-Peter Clausen  wrote:
>> Yes, but that's a different issue and to be honest I didn't even realize
>> that the patch was trying to fix this as well. In my opinion it's best to
>> split this up into two patches one which fixes the OF dependency. The other
>> fixing the timeout issue. Cause there is more to it than just changing the 
>> type.

Sounds fair to split into two patches.  ;)


>> wait_for_completion_interruptible_timeout() may return
>>   1) 0, if there was a timeout, waiting for the completion
>>   2) > 0, if the completion was completeted, the returned value
>>  the  remaining time.
>>   3) < 0, If it was interrupt while waiting for the completion
>>
>> The code currently only handles 1) and 2), but it also needs to handle 3).
>> Since the completion has not been completed in case 3.
>>
>> E.g. something like this should work:
>>
>>   if (timeout == 0)
>>   return -ETIMEDOUT;
>>   else if(timeout < 0)
>>   return timeout;
>>   return 0;

Good catch.  Yes, that looks right to me.


> I just saw, there is another issue related to this. The driver should call
> INIT_COMPLETION(&info->completion) before starting the conversion. Otherwise
> there may be a problem if we got interrupted while waiting for the
> interrupt. E.g. imagine the following sequence.
>
> 1) Start conversion
> 2) Wait for completion
> 3) Abort waiting
> 4) Interrupt kicks in and marks the completion as done
>
> Now if another conversion is started the completion will already be
> completed and wait_for_completion_interruptible_timeout() will return right
> away without waiting for the conversion to be finished.

Another good catch.  ...but are you sure that your solution is enough?
 It seems like we'd also want to lock a spinlock in the ISR and to
consider the completion protected by the lock.  If we don't do that it
seems like you could get an interrupt _right_ after you re-init the
completion.

Notes:
* I thought we could perhaps just disable the interrupt after
wait_for_completion_interruptible_timeout() returns, but I'm not sure
that's guaranteed to work in a multiprocessor environment.

* I don't see any other iio/adc drivers using a spin_lock so maybe
there's a better way to do it (or I'm misunderstanding).  A quick scan
shows only two other iio/adc drivers even use request_irq().
at91_adc.c appears to suffer from similar problems to what we're
discussing here (only init the completion in probe).  ad_sigma_delta
at lest calls INIT_COMPLETION before waiting but seems like it still
has a small window of race (I'd have to dig more to be sure).


Perhaps someone will tell me that I'm just confused.  ;)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/4] iio: adc: Add dt support for turning on the phy in exynos-adc

[Doug Anderson]

Without this change the exynos adc controller needed to have its phy
enabled in some out-of-driver C code.  Add support for specifying the
phy enable register by listing it in the reg list.

Signed-off-by: Doug Anderson 
---
Changes in v2: None

 .../devicetree/bindings/arm/samsung/exynos-adc.txt |  4 ++--
 drivers/iio/adc/exynos_adc.c   | 14 +-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index 96db940..05be151 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -15,7 +15,7 @@ Required properties:
Must be "samsung,exynos-adc-v2" for
future controllers.
 - reg: Contains ADC register address range (base address and
-   length).
+   length) and the address of the phy enable register.
 - interrupts:  Contains the interrupt information for the timer. The
format is being dependent on which interrupt controller
the Samsung device uses.
@@ -30,7 +30,7 @@ Example: adding device info in dtsi file
 
 adc: adc@12D1 {
compatible = "samsung,exynos-adc-v1";
-   reg = <0x12D1 0x100>;
+   reg = <0x12D1 0x100>, <0x10040718 0x4>;
interrupts = <0 106 0>;
#io-channel-cells = <1>;
io-channel-ranges;
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index ed6fdd7..5ab0dfd 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -85,6 +85,7 @@ enum adc_version {
 
 struct exynos_adc {
void __iomem*regs;
+   void __iomem*enable_reg;
struct clk  *clk;
unsigned intirq;
struct regulator*vdd;
@@ -269,13 +270,19 @@ static int exynos_adc_probe(struct platform_device *pdev)
info = iio_priv(indio_dev);
 
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
info->regs = devm_request_and_ioremap(&pdev->dev, mem);
if (!info->regs) {
ret = -ENOMEM;
goto err_iio;
}
 
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+   info->enable_reg = devm_request_and_ioremap(&pdev->dev, mem);
+   if (!info->enable_reg) {
+   ret = -ENOMEM;
+   goto err_iio;
+   }
+
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "no irq resource?\n");
@@ -295,6 +302,8 @@ static int exynos_adc_probe(struct platform_device *pdev)
goto err_iio;
}
 
+   writel(1, info->enable_reg);
+
info->clk = devm_clk_get(&pdev->dev, "adc");
if (IS_ERR(info->clk)) {
dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
@@ -370,6 +379,7 @@ static int exynos_adc_remove(struct platform_device *pdev)
exynos_adc_remove_devices);
regulator_disable(info->vdd);
clk_disable_unprepare(info->clk);
+   writel(0, info->enable_reg);
iio_device_unregister(indio_dev);
free_irq(info->irq, info);
iio_device_free(indio_dev);
@@ -395,6 +405,7 @@ static int exynos_adc_suspend(struct device *dev)
}
 
clk_disable_unprepare(info->clk);
+   writel(0, info->enable_reg);
regulator_disable(info->vdd);
 
return 0;
@@ -410,6 +421,7 @@ static int exynos_adc_resume(struct device *dev)
if (ret)
return ret;
 
+   writel(1, info->enable_reg);
clk_prepare_enable(info->clk);
 
exynos_adc_hw_init(info);
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 1/4] iio: adc: Document the regulator/clocks for exynos-adc

[Doug Anderson]

The exynos ADC won't work without a regulator called "vdd" and a clock
called "adc".  Document this fact in the device tree bindings.

Signed-off-by: Doug Anderson 
---
Changes in v2: None

 Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index f686378..96db940 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -20,6 +20,9 @@ Required properties:
format is being dependent on which interrupt controller
the Samsung device uses.
 - #io-channel-cells = <1>; As ADC has multiple outputs
+- clocks   From common clock binding: handle to adc clock.
+- clock-names  From common clock binding: Shall be "adc".
+- vdd-supply   VDD input supply.
 
 Note: child nodes can be added for auto probing from device tree.
 
@@ -31,6 +34,11 @@ adc: adc@12D1 {
interrupts = <0 106 0>;
#io-channel-cells = <1>;
io-channel-ranges;
+
+   clocks = <&clock 303>;
+   clock-names = "adc";
+
+   vdd-supply = <&buck5_reg>;
 };
 
 
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 3/4] ARM: dts: Add adc to exynos5250 device tree file

[Doug Anderson]

Add the device tree entry for the device-tree enabled ADC driver that
recently landed in the iio tree.

Signed-off-by: Doug Anderson 
---
Changes in v2: None

 arch/arm/boot/dts/exynos5250.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 3acf594..1642062 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -69,6 +69,17 @@
 <0 28 0>, <0 29 0>, <0 30 0>, <0 31 0>;
};
 
+   adc: adc@12D1 {
+   compatible = "samsung,exynos-adc-v1";
+   reg = <0x12D1 0x100>, <0x10040718 0x4>;
+   interrupts = <0 106 0>;
+   #io-channel-cells = <1>;
+   io-channel-ranges;
+
+   clocks = <&clock 303>;
+   clock-names = "adc";
+   };
+
watchdog {
compatible = "samsung,s3c2410-wdt";
reg = <0x101D 0x100>;
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 4/4] ARM: dts: Add adc and thermistors for exynos5250-snow

[Doug Anderson]

Hook up the exynos5250-snow thermistors via the device tree now that
there's a driver available to use them.

Signed-off-by: Doug Anderson 
---
Changes in v2:
- Match 'uV' -> 'uv' change in Naveen's bindings.

 arch/arm/boot/dts/cros5250-common.dtsi |  4 
 arch/arm/boot/dts/exynos5250-snow.dts  | 31 +++
 2 files changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/cros5250-common.dtsi 
b/arch/arm/boot/dts/cros5250-common.dtsi
index 46c0980..f7cc835 100644
--- a/arch/arm/boot/dts/cros5250-common.dtsi
+++ b/arch/arm/boot/dts/cros5250-common.dtsi
@@ -167,6 +167,10 @@
status = "disabled";
};
 
+   adc@12D1 {
+   vdd-supply = <&buck5_reg>;
+   };
+
hdmi {
hpd-gpio = <&gpx3 7 0xf 1 3>;
};
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts 
b/arch/arm/boot/dts/exynos5250-snow.dts
index 17dd951..b78da7c 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -40,4 +40,35 @@
<&gpc4 5 2 3 0>, <&gpc4 6 2 3 0>;
};
};
+
+   adc@12D1 {
+   ncp15wb473@3 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uv = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 3>;
+   };
+   ncp15wb473@4 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uv = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 4>;
+   };
+   ncp15wb473@5 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uv = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 5>;
+   };
+   ncp15wb473@6 {
+   compatible = "ntc,ncp15wb473";
+   pullup-uv = <180>;
+   pullup-ohm = <47000>;
+   pulldown-ohm = <0>;
+   io-channels = <&adc 6>;
+   };
+   };
 };
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 0/4] This set of patches is based on Naveen Krishna Chatradhi's recent work

[Doug Anderson]

(some of which is still in-flight) and does the final touches to get
things working on exynos5250-snow (the ARM Chromebook).

These patches were tested on next-20130312 (which includes support for
the common clock framework on exynos) plus the current linux-iio/togreg:
  95783f2 iio: adc: add exynos adc driver under iio framwork
  298489f iio:common: Use spi_sync_transfer() in STMicroelectronics ...
  1d9a4cb IIO ADC support for AD7923
  9a282b0 iio: Add OF support
  3d277fc3 staging:iio: Remove adt7410 driver

Changes in v2:
- Match 'uV' -> 'uv' change in Naveen's bindings.

Doug Anderson (4):
  iio: adc: Document the regulator/clocks for exynos-adc
  iio: adc: Add dt support for turning on the phy in exynos-adc
  ARM: dts: Add adc to exynos5250 device tree file
  ARM: dts: Add adc and thermistors for exynos5250-snow

 .../devicetree/bindings/arm/samsung/exynos-adc.txt | 12 +++--
 arch/arm/boot/dts/cros5250-common.dtsi |  4 +++
 arch/arm/boot/dts/exynos5250-snow.dts  | 31 ++
 arch/arm/boot/dts/exynos5250.dtsi  | 11 
 drivers/iio/adc/exynos_adc.c   | 14 +-
 5 files changed, 69 insertions(+), 3 deletions(-)

-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4] hwmon: ntc: Add DT with IIO support to NTC thermistor driver

[Doug Anderson]

Hi,

On Tue, Mar 12, 2013 at 9:08 PM, Naveen Krishna Chatradhi
 wrote:
> This patch adds DT support to NTC driver to parse the
> platform data.
>
> Also adds the support to work as an iio device.
>
> During the probe ntc driver gets the respective channels of ADC
> and uses iio_raw_read calls to get the ADC converted value.
>
> Signed-off-by: Naveen Krishna Chatradhi 
> ---
> Changes since v3:
> 1. Added a NULL check before iio_channel_release
> 2. Modified a return statement
>
> Guenter Roeck, Its wonderful working with you. Thanks.
>
>  .../devicetree/bindings/hwmon/ntc_thermistor.txt   |   29 
>  drivers/hwmon/Kconfig  |1 +
>  drivers/hwmon/ntc_thermistor.c |  145 
> +---
>  include/linux/platform_data/ntc_thermistor.h   |8 +-
>  4 files changed, 163 insertions(+), 20 deletions(-)

I haven't done a full review of this code, but it I built it in (along
with other CLs to enable it on exynos5250-snow) and the thermistors
are accessible and report reasonable values.

localhost hwmon # grep "" /sys/class/hwmon/*/device/temp1_input
/sys/class/hwmon/hwmon0/device/temp1_input:37890
/sys/class/hwmon/hwmon1/device/temp1_input:38393
/sys/class/hwmon/hwmon2/device/temp1_input:37148
/sys/class/hwmon/hwmon3/device/temp1_input:38059

Tested-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] USB: ehci-s5p: Fix phy reset

[Doug Anderson]

Alexander,

On Wed, Mar 13, 2013 at 4:59 PM, Alexander Graf  wrote:
> On my Exynos 5 based Arndale system, I need to pull the reset line down
> and then let it go up again to actually perform a reset. Without that
> reset, I can't find any USB hubs on my bus, rendering the USB controller
> useless.
>
> We also only need to reset the line after the phy node has been found.
> This way we don't accidently reserve the vbus GPIO pin, but later on
> defer the creation of our controller, because the phy device tree node
> hasn't been probed yet.
>
> This patch implements the above logic, making EHCI and OHCI work on
> Arndale systems for me.
>
> Signed-off-by: Alexander Graf 
> CC: Vivek Gautam 
> CC: Jingoo Han 
> CC: Alan Stern 
> CC: Kukjin Kim 
> CC: Felipe Balbi 
> CC: Greg Kroah-Hartman 
> CC: Doug Anderson 
>
> ---
>
> v1 -> v2:
>
>   - remove gpio_free call
>   - move reset logic after phy node search

Seems fine to me.  I guess the earlier problem you wrote about was the
probe failure, then?  I think that the reason I don't tend to get the
probe failure is that I've got my device tree ordered differently so
that the phy gets initted in a different order.

I'll send up the devm_ patch atop this.

Reviewed-by: Doug Anderson 

Thanks!  :)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] USB: ehci-s5p: Fix phy reset

[Doug Anderson]

Hi,

On Thu, Mar 14, 2013 at 7:58 AM, Thomas Abraham
 wrote:
>> I can see your point, but as I mentioned earlier there seems to be some 
>> timing issue here. By simply doing the reset a few ms earlier (in the first 
>> probe, before the driver detects that it needs to defer probing), I already 
>> can't find the hub on the bus later.
>>
>> So I'm assuming that the same thing would also happen if I put it even 
>> earlier in machine init.
>
> True, I missed that point. The usb hub connected over hsic interface,
> after power-on-reset, might have initiated the 'connect' state on
> seeing the idle condition on the bus and since the host/phy controller
> is not ready yet, the connect might have failed.
>
> So the correct sequence would be, after the usb host controller and
> the phy controllers are initialized, the 'reset' pin on the on-board
> usb hub should be asserted. Upon releasing that reset, the usb hub
> would initiate the 'connect' state on the HSIC bus.

I think we ran into similar issues on one of our boards that had a hub
attached to the HSIC port.  We had to make our fix in a version of the
code that's nowhere near what landed upstream (so our change is not
relevant), but we're definitely interested in whatever solution you
come up with.  :)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] usb: Document clocks in samsung,exynos4210-ehci/ohci bindings

[Doug Anderson]

The exynox4210-ehci and exynos4210-ohci nodes need a clock specified
using the common clock framework.  Document it.

Signed-off-by: Doug Anderson 
---
 Documentation/devicetree/bindings/usb/exynos-usb.txt | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt 
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index f66fcdd..d557cf7 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -10,6 +10,8 @@ Required properties:
  - reg: physical base address of the controller and length of memory mapped
region.
  - interrupts: interrupt number to the cpu.
+ - clocks: from common clock binding: handle to adc clock.
+ - clock-names: from common clock binding: Shall be "usbhost".
 
 Optional properties:
  - samsung,vbus-gpio:  if present, specifies the GPIO that
@@ -22,6 +24,9 @@ Example:
reg = <0x1211 0x100>;
interrupts = <0 71 0>;
samsung,vbus-gpio = <&gpx2 6 1 3 3>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
 
 OHCI
@@ -31,10 +36,15 @@ Required properties:
  - reg: physical base address of the controller and length of memory mapped
region.
  - interrupts: interrupt number to the cpu.
+ - clocks: from common clock binding: handle to adc clock.
+ - clock-names: from common clock binding: Shall be "usbhost".
 
 Example:
usb@1212 {
compatible = "samsung,exynos4210-ohci";
reg = <0x1212 0x100>;
interrupts = <0 71 0>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: dts: add usb 2.0 clock references to exynos5250 device tree

[Doug Anderson]

Vivek,

On Wed, Mar 13, 2013 at 11:22 PM, Vivek Gautam
 wrote:
> It will be nice if you can please update relevant information
> alongwith this, in the bindings doc for "exynos-usb".

Sure.  It always feels like device tree additions ought to be separate
patches from bindings patches, so I'll just send something separately.
 Here it is: 

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] usb: ehci-s5p: Use devm for requesting ehci_vbus_gpio

[Doug Anderson]

The ehci_vbus_gpio is requested but never freed.  This can cause
problems with deferred probes and would cause problems if
s5p_ehci_remove was ever called.  Use devm to fix this.

Signed-off-by: Doug Anderson 
---
 drivers/usb/host/ehci-s5p.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 20ebf6a..a464197 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -92,6 +92,7 @@ static void s5p_ehci_phy_disable(struct s5p_ehci_hcd 
*s5p_ehci)
 
 static void s5p_setup_vbus_gpio(struct platform_device *pdev)
 {
+   struct device *dev = &pdev->dev;
int err;
int gpio;
 
@@ -103,7 +104,8 @@ static void s5p_setup_vbus_gpio(struct platform_device 
*pdev)
if (!gpio_is_valid(gpio))
return;
 
-   err = gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, "ehci_vbus_gpio");
+   err = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_HIGH,
+   "ehci_vbus_gpio");
if (err)
dev_err(&pdev->dev, "can't request ehci vbus gpio %d", gpio);
 }
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] usb: Document clocks in samsung,exynos4210-ehci/ohci bindings

[Doug Anderson]

The exynox4210-ehci and exynos4210-ohci nodes need a clock specified
using the common clock framework.  Document it.

Signed-off-by: Doug Anderson 
---
Changes in v2:
- Fixed embarrassing typo adc=>usb.  Thanks Jingoo!

 Documentation/devicetree/bindings/usb/exynos-usb.txt | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt 
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index f66fcdd..b3abde7 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -10,6 +10,8 @@ Required properties:
  - reg: physical base address of the controller and length of memory mapped
region.
  - interrupts: interrupt number to the cpu.
+ - clocks: from common clock binding: handle to usb clock.
+ - clock-names: from common clock binding: Shall be "usbhost".
 
 Optional properties:
  - samsung,vbus-gpio:  if present, specifies the GPIO that
@@ -22,6 +24,9 @@ Example:
reg = <0x1211 0x100>;
interrupts = <0 71 0>;
samsung,vbus-gpio = <&gpx2 6 1 3 3>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
 
 OHCI
@@ -31,10 +36,15 @@ Required properties:
  - reg: physical base address of the controller and length of memory mapped
region.
  - interrupts: interrupt number to the cpu.
+ - clocks: from common clock binding: handle to usb clock.
+ - clock-names: from common clock binding: Shall be "usbhost".
 
 Example:
usb@1212 {
compatible = "samsung,exynos4210-ohci";
reg = <0x1212 0x100>;
interrupts = <0 71 0>;
+
+   clocks = <&clock 285>;
+   clock-names = "usbhost";
};
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] usb: ehci-s5p: Use devm for requesting ehci_vbus_gpio

[Doug Anderson]

Jingoo,

On Thu, Mar 14, 2013 at 5:30 PM, Jingoo Han  wrote:
> Would you replace other '&pdev->dev' with 'dev' in s5p_setup_vbus_gpio()
> as below? It seems to be better for readability.

Yes, of course.  That was silly of me not to add the "dev" local and
not update the other places...  Thanks for your review!

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] usb: ehci-s5p: Use devm for requesting ehci_vbus_gpio

[Doug Anderson]

The ehci_vbus_gpio is requested but never freed.  This can cause
problems with deferred probes and would cause problems if
s5p_ehci_remove was ever called.  Use devm to fix this.

Signed-off-by: Doug Anderson 
---
Changes in v2:
- &pdev->dev => dev elsewhere in s5p_setup_vbus_gpio()

 drivers/usb/host/ehci-s5p.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 20ebf6a..738490e 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -92,20 +92,21 @@ static void s5p_ehci_phy_disable(struct s5p_ehci_hcd 
*s5p_ehci)
 
 static void s5p_setup_vbus_gpio(struct platform_device *pdev)
 {
+   struct device *dev = &pdev->dev;
int err;
int gpio;
 
-   if (!pdev->dev.of_node)
+   if (!dev->of_node)
return;
 
-   gpio = of_get_named_gpio(pdev->dev.of_node,
-   "samsung,vbus-gpio", 0);
+   gpio = of_get_named_gpio(dev->of_node, "samsung,vbus-gpio", 0);
if (!gpio_is_valid(gpio))
return;
 
-   err = gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, "ehci_vbus_gpio");
+   err = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_HIGH,
+   "ehci_vbus_gpio");
if (err)
-   dev_err(&pdev->dev, "can't request ehci vbus gpio %d", gpio);
+   dev_err(dev, "can't request ehci vbus gpio %d", gpio);
 }
 
 static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] iio: adc: Kconfig: exynos_adc depends on CONFIG_OF

[Doug Anderson]

Naveen,

On Fri, Mar 15, 2013 at 9:23 AM, Naveen Krishna Chatradhi
 wrote:
> As the exynos_adc driver only supports device tree registration.
> Making driver depend on CONFIG_OF solves possible errors during probe.
>
> Signed-off-by: Naveen Krishna Chatradhi 
> Reported-by: Dan Carpenter 
> Cc: Doug Anderson 
> Cc: Lars-Peter Clausen 
> ---
> Discussion thread for this patch can be found at
> http://www.gossamer-threads.com/lists/linux/kernel/1693284?page=last
>
>  drivers/iio/adc/Kconfig |1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] RFC: mmc: dw_mmc: Always go to STATE_DATA_BUSY from STATE_DATA_ERROR

[Doug Anderson]

On a flaky piece of hardware that seems good at generating CRC errors,
we have found that often times the CRC errors don't get reported
properly when using CONFIG_MMC_DW_IDMAC (they get reported OK when
using pio).

The flow that happens is:
1. dw_mci_interrupt() fires and status=80b8, pending=8088 so that
   we hit (pending & DW_MCI_DATA_ERROR_FLAGS).  We store 8088 in
   data_status and set EVENT_DATA_ERROR in host->pending_events
2. We schedule the tasklet and it runs.
3. We're in STATE_SENDING_DATA in the tasklet and see
   EVENT_DATA_ERROR so we dw_mci_stop_dma().
4. dw_mci_stop_dma() calls dw_mci_idmac_stop_dma() and
   dw_mci_dma_cleanup().  These stop dma but _don't_ set
   EVENT_XFER_COMPLETE (since we're host->using_dma).
5. data->stop is NULL so we don't send a stop command.
6. We move onto STATE_DATA_ERROR and loop again in the tasklet.
7. We hit STATE_DATA_ERROR but the transfer isn't done, so the tasklet
   stops.

We never seem to get any additional DMA interrupts that cause
EVENT_XFER_COMPLETE and restart the tasklet so we just hang.  That
doesn't seem surprising given that we've stopped DMA.

We did put a print at the end of dw_mci_interrupt() to show the result
of the "mci_readl(host, IDSTS)" and saw 0xa000 in the case of the
above CRC error.

A proposed fix for this is to ignore (but still clear) the
EVENT_XFER_COMPLETE in STATE_DATA_ERROR in the tasklet.

Reported-by: Bing Zhao 
Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 9834221..696b3bb 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1137,10 +1137,7 @@ static void dw_mci_tasklet_func(unsigned long priv)
goto unlock;
 
case STATE_DATA_ERROR:
-   if (!test_and_clear_bit(EVENT_XFER_COMPLETE,
-   &host->pending_events))
-   break;
-
+   clear_bit(EVENT_XFER_COMPLETE, &host->pending_events);
state = STATE_DATA_BUSY;
break;
}
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] RFC: mmc: dw_mmc: Don't clear errors we aren't handling

[Doug Anderson]

Although there are no known cases of this being a problem (and it may
be technically impossible for the hardware to report more errors once
already in the error state), it seems unwise for us to be clearing
error interrupts that we didn't actually read.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 9834221..84ae704 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1583,7 +1583,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
break;
 
if (pending & DW_MCI_CMD_ERROR_FLAGS) {
-   mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS);
+   mci_writel(host, RINTSTS,
+  pending & DW_MCI_CMD_ERROR_FLAGS);
host->cmd_status = pending;
smp_wmb();
set_bit(EVENT_CMD_COMPLETE, &host->pending_events);
@@ -1591,7 +1592,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
 
if (pending & DW_MCI_DATA_ERROR_FLAGS) {
/* if there is an error report DATA_ERROR */
-   mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS);
+   mci_writel(host, RINTSTS,
+  pending & DW_MCI_DATA_ERROR_FLAGS);
host->data_status = pending;
smp_wmb();
set_bit(EVENT_DATA_ERROR, &host->pending_events);
-- 
1.8.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC: PATCH 2/2] iio: adc: exynos_adc: Handle timeout and race conditions

[Doug Anderson]

On Fri, Mar 15, 2013 at 2:53 PM, Lars-Peter Clausen  wrote:
> What exactly is the spinlock protecting against here? Concurrent runs of
> exynos_adc_isr? This is probably not issue in the first place.
>
> What you want to protect against is that completion is completed between the
> call to INIT_COMPLETION() and the start of a new conversion. So the sections
> that need to be under the spinlock are the complete call here and the point
> from INIT_COMPLETION until the transfer is started in exynos_read_raw(). Make
> sure to use spin_lock_irq there.

...and at that point I _think_ you won't also need the mutex.

A reasonable way to test to see if you've got this all correct would be to:

* Start two processes that are reading from different ADCs that will
report very different values (maybe add a device tree node for adc1 or
adc7 and use those since they're not really connected to
thermistors?).

* Have your two processes read as fast as they can.  This could just
be "while true; do cat /sys/class/hwmon/hwmon0/device/temp1_input;
done"

* Decrease your timeout and maybe(?) sprinkle some random udelays in
the irq handler so that the timeouts happen sometimes but not others.

* Periodically cancel one of the readers with Ctrl-C

If all is working well then you should always get back the right value
from the right reader (and get no crashes).

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] serial: samsung: Add poll_get_char & poll_put_char

[Doug Anderson]

From: Julien Pichon 

The following patch allows users to use KGDB over serial console on
board based on Samsung SOC. It has been tested on a board using
exynos5.

Signed-off-by: Julien Pichon 
Signed-off-by: Doug Anderson 
(dianders changed poll to return NO_POLL_CHAR, which appears to
fix 'help' in kgdb; also updated commit message)
---
This is pulled from an email on 
from Feb 18, 2012. It's never landed anywhere. I've made a small modification
that make it so that kgdb's "help" doesn't crash.

See: http://comments.gmane.org/gmane.linux.kernel.samsung-soc/9477

 drivers/tty/serial/samsung.c |   47 ++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 8eef114..7f04717 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -876,11 +876,24 @@ s3c24xx_serial_verify_port(struct uart_port *port, struct 
serial_struct *ser)
 
 static struct console s3c24xx_serial_console;
 
+static int __init s3c24xx_serial_console_init(void)
+{
+   register_console(&s3c24xx_serial_console);
+   return 0;
+}
+console_initcall(s3c24xx_serial_console_init);
+
 #define S3C24XX_SERIAL_CONSOLE &s3c24xx_serial_console
 #else
 #define S3C24XX_SERIAL_CONSOLE NULL
 #endif
 
+#ifdef CONFIG_CONSOLE_POLL
+static int s3c24xx_serial_get_poll_char(struct uart_port *port);
+static void s3c24xx_serial_put_poll_char(struct uart_port *port,
+unsigned char c);
+#endif
+
 static struct uart_ops s3c24xx_serial_ops = {
.pm = s3c24xx_serial_pm,
.tx_empty   = s3c24xx_serial_tx_empty,
@@ -899,6 +912,10 @@ static struct uart_ops s3c24xx_serial_ops = {
.request_port   = s3c24xx_serial_request_port,
.config_port= s3c24xx_serial_config_port,
.verify_port= s3c24xx_serial_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+   .poll_get_char = s3c24xx_serial_get_poll_char,
+   .poll_put_char = s3c24xx_serial_put_poll_char,
+#endif
 };
 
 static struct uart_driver s3c24xx_uart_drv = {
@@ -1316,6 +1333,36 @@ s3c24xx_serial_console_txrdy(struct uart_port *port, 
unsigned int ufcon)
return (utrstat & S3C2410_UTRSTAT_TXE) ? 1 : 0;
 }
 
+#ifdef CONFIG_CONSOLE_POLL
+/*
+ * Console polling routines for writing and reading from the uart while
+ * in an interrupt or debug context.
+ */
+
+static int s3c24xx_serial_get_poll_char(struct uart_port *port)
+{
+   struct s3c24xx_uart_port *ourport = to_ourport(port);
+   unsigned int ufstat;
+
+   ufstat = rd_regl(port, S3C2410_UFSTAT);
+   if (s3c24xx_serial_rx_fifocnt(ourport, ufstat) == 0)
+   return NO_POLL_CHAR;
+
+   return rd_regb(port, S3C2410_URXH);
+}
+
+static void s3c24xx_serial_put_poll_char(struct uart_port *port,
+   unsigned char c)
+{
+   unsigned int ufcon = rd_regl(cons_uart, S3C2410_UFCON);
+
+   while (!s3c24xx_serial_console_txrdy(port, ufcon))
+   cpu_relax();
+   wr_regb(cons_uart, S3C2410_UTXH, c);
+}
+
+#endif /* CONFIG_CONSOLE_POLL */
+
 static void
 s3c24xx_serial_console_putchar(struct uart_port *port, int ch)
 {
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 2/3] mmc: dw_mmc: Honor requests to set the clock to 0 (turn off clock)

[Doug Anderson]

Jaehoon,

On Sun, Aug 25, 2013 at 6:31 PM, Jaehoon Chung  wrote:
> Hi Doug,
>
> On 08/24/2013 05:40 AM, Doug Anderson wrote:
>> Jaehoon,
>>
>> On Fri, Aug 23, 2013 at 6:21 AM, Jaehoon Chung  
>> wrote:
>>> Hi Doug,
>>>
>>> If the clock-gating is enabled, then maybe it's continuously printed the 
>>> kernel message for Bus_speed.
>>
>> Can you explain?  I don't think dw_mmc has support for clock gating
>> right now.  ...or are there some patches that I'm not aware of?  I
>> could believe that if you've got some non-upstream clock gating
>> patches that these would need to be modified to handle it...  ...but
>> unless those are slated to land upstream it seems like I can't take
>> them into account, can I?
> If i enabled the CONFIG_MMC_CLK_GATE, the i have found the below message 
> whenever some operation is run.
> I will test more with your patch.

Ah, sorry!  I wasn't aware of that config option.  I was thinking of
automatic clock gating based on something like the common clock
framework.  When there are no more users of a gate clock it will get
turned off.  To have that work dw_mmc would need to release its biu /
ciu clocks at some point.

If I had to guess, I'd speculate that perhaps we should just change
the printout to a dev_debug(), though I do find that printout
incredibly useful.  If I had to guess I'd say that the mmc core is
switching between a clock of 0 and a full speed clock constantly.  If
that's true then it means that dw_mmc used to treat that like a no-op.
 Now it actually gates the clock.  If you comment out the printout, do
things still work?  Does your power consumption go down?

Let me know if you find anything.  Otherwise I can try to reproduce this week.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 2/3] mmc: dw_mmc: Honor requests to set the clock to 0 (turn off clock)

[Doug Anderson]

Jaehoon / Seungwon,

On Mon, Aug 26, 2013 at 2:06 AM, Jaehoon Chung  wrote:
> On 08/26/2013 01:34 PM, Seungwon Jeon wrote:
>> On Fri, August 23, 2013, Doug Anderson wrote:
>>> Previously the dw_mmc driver would ignore any requests to disable the
>>> card's clock.  This doesn't seem like a good thing in general, but had
>>> one extra bad side effect in the following situtation:
>>> * mmc core would set clk to 400kHz at boot time while initting
>>> * mmc core would set clk to 0 since no card, but it would be ignored.
>>> * suspend to ram and resume; clocks in the dw_mmc IP block are now 0
>>>   but dw_mmc thinks that they're 400kHz (it ignored the set to 0).
>>> * insert card
>>> * mmc core would set clk to 400kHz which would be considered a no-op.
>>>
>>> Note that if there is no card in the slot and we do a suspend/resume
>>> cycle, we _do_ still end up with differences in a dw_mmc register
>>> dump, but the differences are clock related and we've got the clock
>>> disabled both before and after, so this should be OK.
>>>
>>> Signed-off-by: Doug Anderson 
>>> ---
>>> Changes in v6:
>>> - Replaces previous pathes that ensured saving/restoring clocks.
>>>
>>>  drivers/mmc/host/dw_mmc.c | 21 +++--
>>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index ee5f167..f6c7545 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -635,7 +635,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, 
>>> bool force_clkinit)
>>>  u32 div;
>>>  u32 clk_en_a;
>>>
>>> -if (slot->clock != host->current_speed || force_clkinit) {
>>> +if (slot->clock == 0) {
>>> +mci_writel(host, CLKENA, 0);
>>> +mci_send_cmd(slot,
>>> + SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>> Basically dw_mmc driver uses host's low power mode(auto clock gating)
>> So, how about keeping origin code rather than programming clock setting to 
>> '0'?
> Right, Dw-mmc controller can use the Low-power mode.
> This mode is functionality like clock-gating. Well, i didn't know what 
> benefit we get, if set to 0.

Ah, right.  ...so it's unlikely that we'd save any power because we're
already gating the clock.

I'd really still rather honor the MMC subsystem's request.  It
shouldn't _hurt_ to turn the clock off when the subsystem requests it,
right?  One reason to honor the mmc core is that it will make things
cleaner if/when we support a voltage change operation.  The MMC core
has the logic for the voltage change, and part of that involves
turning off the clock.  We'll already need a bunch of special case
code in dw_mmc for voltage change, but it would be nice to avoid one
extra bit.

Another option would be to add a core MMC quirk to disable MMC_CLKGATE
on a per-driver basis.  In the "single zImage" world that seems like
the right thing to do, since the MMC_CLKGATE description seems to
indicate that enabling that won't really work on all drivers anyway.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] iio: exynos_adc: fix wrong structure extration in suspend and resume

[Doug Anderson]

Naveen

On Tue, Aug 27, 2013 at 10:33 PM, Naveen Krishna Ch
 wrote:
>> I would like to know any comments on
>>
>> https://patchwork.kernel.org/patch/2513361/
>>
>> Its been pending for a while now.
>>
>> Thanks,
>> Naveen
> Ping
> Any comments please

I assume you're asking for a ping for the _other_ patch (AKA: "Handle
timeout issues"), not the one you're replying to here.

Can you please just do a REPOST of the patch you'd like comments on?

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 2/3] mmc: dw_mmc: Honor requests to set the clock to 0 (turn off clock)

[Doug Anderson]

Seungwon,

On Thu, Aug 29, 2013 at 12:04 AM, Seungwon Jeon  wrote:
>> I'd really still rather honor the MMC subsystem's request.  It
>> shouldn't _hurt_ to turn the clock off when the subsystem requests it,
> Even though turning off by clock programming doesn't hurt,
> it is costly behavior when considering low power mode of host's own support.

It is costly?  We are talking about these two commands, right?

  mci_writel(host, CLKENA, 0);
  mci_send_cmd(slot,
  SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);

Do you have a reason to believe that these are more costly than all of
the rest of the code that's executed when the user defines
CONFIG_MMC_CLKGATE?  You're still proposing doing all of the updates
of the clock when slot->clock is non-zero, right?  ...so at best
skipping this code will be 33% faster since the re-enable code
disables and then reenables the clock.  If it's the
"SDMMC_CMD_PRV_DAT_WAIT" that you're worried about then skipping this
code will only be 25% faster since there are already three calls with
SDMMC_CMD_PRV_DAT_WAIT in the enable code.


> Just now, how about focusing on the problem clock isn't updated properly 
> after suspend/resume?

I tried to do that in the original patches, but you pointed out
(correctly) that we should do the correct fix rather than a hackier
fix.  IMHO the most correct fix is to honor the MMC core's request to
turn the clock off.  Partially honoring the MMC core (as you suggest)
is certainly less hacky that my original proposal but I still think
turning the clock off is better.


>> right?  One reason to honor the mmc core is that it will make things
>> cleaner if/when we support a voltage change operation.  The MMC core
>> has the logic for the voltage change, and part of that involves
>> turning off the clock.  We'll already need a bunch of special case
>> code in dw_mmc for voltage change, but it would be nice to avoid one
>> extra bit.
> Turning off clock during voltage switching would be another procedure.
> I guess it could be discussed later.

Agreed that we're not trying to get voltage switching done here, but
forward thinking is nice.  If there's no reason _not_ to turn the
clock off and it will help us later, let's do it. Also, we've already
agreed that MMC_CLKGATE isn't so useful for dw_mmc, so trying to do
something awkward to make MMC_CLKGATE slightly faster doesn't seem
worth it.


> I want to fix some minor change to prevent frequent message that Jaehoon 
> pointed.

As far as I can tell, the frequent messages and whether or not to
actually turn the clock off are unrelated.  I will send up a patch
that fixes the frequent messages by caching the last value printed and
only printing if it changed.  I have verified that this works and that
the system still functions OK (can boot to prompt) with
CONFIG_MMC_CLKGATE.


Note: re-reading over some of the previous messages, it sounds like
you're proposing using the patch from your email directly, AKA:

http://article.gmane.org/gmane.linux.kernel/1542482

Did you test that patch?  Did it work for you?  It doesn't actually
compile cleanly for me (you removed the "force_clkinit" param in the
function but not the callers).  That's easy to fix, but implies that
this patch was just a proposal and not a tested solution.

...but aside from not compiling cleanly, I don't think it will work
for the same reasons that the original code didn't work.  Specifically
it doesn't address the core problem that we need to update
host->current_speed when the clock is 0.  Otherwise we won't re-init
and we run into the original problem, right?  To be certain I took
your patch and applied it, then fixed the callers of
dw_mci_setup_bus() not to pass a second parameter.  I did a
suspend/resume with no card in and then plugged a card in.  It didn't
work.


As I said above, new patch coming shortly.  As always: feel free to
point out any glaring mistakes I made in the above.  ;)  Note that I
will be out of communication for the next week or so and buried
beneath email for a few days after that, so my response might be a
little delayed.



-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v7 3/3] mmc: dw_mmc: Set timeout to max upon resume

[Doug Anderson]

The TMOUT register is initted to 0x at probe time but isn't
initted after suspend/resume.  Add an init of this value.

No problems were observed without this (it will also get initted in
__dw_mci_start_request if there is data to send), but it makes the
register dump before and after suspend cleaner.

Signed-off-by: Doug Anderson 
Acked-by: Seungwon Jeon 
Reviewed-by: Tomasz Figa 
---
Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 1584705..f8d891a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2513,6 +2513,9 @@ int dw_mci_resume(struct dw_mci *host)
/* Restore the old value at FIFOTH register */
mci_writel(host, FIFOTH, host->fifoth_val);
 
+   /* Put in max timeout */
+   mci_writel(host, TMOUT, 0x);
+
mci_writel(host, RINTSTS, 0x);
mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
-- 
1.8.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v7 0/3] mmc: dw_mmc: fixes for suspend/resume on exynos

[Doug Anderson]

This series of patches addresses some suspend/resume problems with
dw_mmc on exynos platforms, espeically exynos5420.  This patchset was
tested on the current ToT Chromeos 3.8 tree (which has lots of
backports from 3.10/3.11) and on ToT Linux (v3.11-rc6).  I have
confirmed basic booting and that SD cards work across suspend/resume
(both if they are plugged in and if they are not plugged in).

I have received confirmation from Samsung that the problem solved for
exynos5420 is a silicon errata and that this is a good fix.

Changes in v7:
- Avoid printing the same clock over and over again w/ MMC_CLKGATE.

Changes in v6:
- Took out TODO comment copied from main platform code.
- Replaces previous pathes that ensured saving/restoring clocks.

Changes in v5:
- Cleaned up dw_mci_exynos_resume_noirq() comment as per Seungwon.
- Don't memcpy dev_pm_ops structure, define a new one.

Changes in v4:
- Take Seungwon's suggestion and don't add any dw_mmc-pltfm code.

Changes in v3:
- Add freeze/thaw and poweroff/restore noirq entries.

Changes in v2:
- Use suspend_noirq as per James Hogan.

Doug Anderson (3):
  mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT
  mmc: dw_mmc: Honor requests to set the clock to 0 (turn off clock)
  mmc: dw_mmc: Set timeout to max upon resume

 drivers/mmc/host/dw_mmc-exynos.c | 53 +++-
 drivers/mmc/host/dw_mmc.c| 39 ++---
 2 files changed, 77 insertions(+), 15 deletions(-)

-- 
1.8.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v7 2/3] mmc: dw_mmc: Honor requests to set the clock to 0 (turn off clock)

[Doug Anderson]

Previously the dw_mmc driver would ignore any requests to disable the
card's clock.  This doesn't seem like a good thing in general, but had
one extra bad side effect in the following situtation:
* mmc core would set clk to 400kHz at boot time while initting
* mmc core would set clk to 0 since no card, but it would be ignored.
* suspend to ram and resume; clocks in the dw_mmc IP block are now 0
  but dw_mmc thinks that they're 400kHz (it ignored the set to 0).
* insert card
* mmc core would set clk to 400kHz which would be considered a no-op.

Note that if there is no card in the slot and we do a suspend/resume
cycle, we _do_ still end up with differences in a dw_mmc register
dump, but the differences are clock related and we've got the clock
disabled both before and after, so this should be OK.

Signed-off-by: Doug Anderson 
---
Changes in v7:
- Avoid printing the same clock over and over again w/ MMC_CLKGATE.

Changes in v6:
- Replaces previous pathes that ensured saving/restoring clocks.

 drivers/mmc/host/dw_mmc.c | 36 ++--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ee5f167..1584705 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -88,6 +88,8 @@ struct idmac_desc {
  * @queue_node: List node for placing this node in the @queue list of
  * &struct dw_mci.
  * @clock: Clock rate configured by set_ios(). Protected by host->lock.
+ * @last_printed_clock: The last clock we printed.  Keeping track of this helps
+ * us to avoid spamming the console with CONFIG_MMC_CLKGATE.
  * @flags: Random state bits associated with the slot.
  * @id: Number of this slot.
  * @last_detect_state: Most recently observed card detect state.
@@ -105,6 +107,7 @@ struct dw_mci_slot {
struct list_headqueue_node;
 
unsigned intclock;
+   unsigned intlast_printed_clock;
unsigned long   flags;
 #define DW_MMC_CARD_PRESENT0
 #define DW_MMC_CARD_NEED_INIT  1
@@ -635,7 +638,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, 
bool force_clkinit)
u32 div;
u32 clk_en_a;
 
-   if (slot->clock != host->current_speed || force_clkinit) {
+   if (slot->clock == 0) {
+   mci_writel(host, CLKENA, 0);
+   mci_send_cmd(slot,
+SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+   } else if (slot->clock != host->current_speed || force_clkinit) {
div = host->bus_hz / slot->clock;
if (host->bus_hz % slot->clock && host->bus_hz > slot->clock)
/*
@@ -646,10 +653,14 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, 
bool force_clkinit)
 
div = (host->bus_hz != slot->clock) ? DIV_ROUND_UP(div, 2) : 0;
 
-   dev_info(&slot->mmc->class_dev,
-"Bus speed (slot %d) = %dHz (slot req %dHz, actual 
%dHZ"
-" div = %d)\n", slot->id, host->bus_hz, slot->clock,
-div ? ((host->bus_hz / div) >> 1) : host->bus_hz, div);
+   if (slot->clock != slot->last_printed_clock || force_clkinit) {
+   dev_info(&slot->mmc->class_dev,
+"Bus speed (slot %d) = %dHz (slot req %dHz, 
actual %dHZ div = %d)\n",
+slot->id, host->bus_hz, slot->clock,
+div ? ((host->bus_hz / div) >> 1) :
+host->bus_hz, div);
+   slot->last_printed_clock = slot->clock;
+   }
 
/* disable clock */
mci_writel(host, CLKENA, 0);
@@ -675,9 +686,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool 
force_clkinit)
/* inform CIU */
mci_send_cmd(slot,
 SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
-
-   host->current_speed = slot->clock;
}
+   host->current_speed = slot->clock;
 
/* Set the current slot bus width */
mci_writel(host, CTYPE, (slot->ctype << slot->id));
@@ -807,13 +817,11 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
 
mci_writel(slot->host, UHS_REG, regs);
 
-   if (ios->clock) {
-   /*
-* Use mirror of ios->clock to prevent race with mmc
-* core ios update when finding the minimum.
-*/
-   slot->clock = ios->clock;
-   }
+   /*
+* Use mirror of ios->clock to prevent race with mmc
+* core ios update when finding the minimum.
+*/
+   slot->cloc

[PATCH v7 1/3] mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT

[Doug Anderson]

If the WAKEUP_INT is asserted at wakeup and not cleared, we'll end up
looping around forever.  This has been seen to happen on exynos5420
silicon despite the fact that we haven't enabled any wakeup events due
to a silicon errata.  It is safe to do on all exynos variants.

Signed-off-by: Doug Anderson 
---
Changes in v7: None
Changes in v6:
- Took out TODO comment copied from main platform code.

Changes in v5:
- Cleaned up dw_mci_exynos_resume_noirq() comment as per Seungwon.
- Don't memcpy dev_pm_ops structure, define a new one.

Changes in v4:
- Take Seungwon's suggestion and don't add any dw_mmc-pltfm code.

Changes in v3:
- Add freeze/thaw and poweroff/restore noirq entries.

Changes in v2:
- Use suspend_noirq as per James Hogan.

 drivers/mmc/host/dw_mmc-exynos.c | 53 +++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 866edef..e8c6cc9 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -30,6 +30,7 @@
 #define SDMMC_CLKSEL_TIMING(x, y, z)   (SDMMC_CLKSEL_CCLK_SAMPLE(x) |  \
SDMMC_CLKSEL_CCLK_DRIVE(y) |\
SDMMC_CLKSEL_CCLK_DIVIDER(z))
+#define SDMMC_CLKSEL_WAKEUP_INTBIT(11)
 
 #define EXYNOS4210_FIXED_CIU_CLK_DIV   2
 #define EXYNOS4412_FIXED_CIU_CLK_DIV   4
@@ -100,6 +101,49 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int dw_mci_exynos_suspend(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+
+   return dw_mci_suspend(host);
+}
+
+static int dw_mci_exynos_resume(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+
+   return dw_mci_resume(host);
+}
+
+/**
+ * dw_mci_exynos_resume_noirq - Exynos-specific resume code
+ *
+ * On exynos5420 there is a silicon errata that will sometimes leave the
+ * WAKEUP_INT bit in the CLKSEL register asserted.  This bit is 1 to indicate
+ * that it fired and we can clear it by writing a 1 back.  Clear it to prevent
+ * interrupts from going off constantly.
+ *
+ * We run this code on all exynos variants because it doesn't hurt.
+ */
+
+static int dw_mci_exynos_resume_noirq(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+   u32 clksel;
+
+   clksel = mci_readl(host, CLKSEL);
+   if (clksel & SDMMC_CLKSEL_WAKEUP_INT)
+   mci_writel(host, CLKSEL, clksel);
+
+   return 0;
+}
+#else
+#define dw_mci_exynos_suspend  NULL
+#define dw_mci_exynos_resume   NULL
+#define dw_mci_exynos_resume_noirq NULL
+#endif /* CONFIG_PM_SLEEP */
+
 static void dw_mci_exynos_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
/*
@@ -187,13 +231,20 @@ static int dw_mci_exynos_probe(struct platform_device 
*pdev)
return dw_mci_pltfm_register(pdev, drv_data);
 }
 
+const struct dev_pm_ops dw_mci_exynos_pmops = {
+   SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
+   .resume_noirq = dw_mci_exynos_resume_noirq,
+   .thaw_noirq = dw_mci_exynos_resume_noirq,
+   .restore_noirq = dw_mci_exynos_resume_noirq,
+};
+
 static struct platform_driver dw_mci_exynos_pltfm_driver = {
.probe  = dw_mci_exynos_probe,
.remove = __exit_p(dw_mci_pltfm_remove),
.driver = {
.name   = "dwmmc_exynos",
.of_match_table = dw_mci_exynos_match,
-   .pm = &dw_mci_pltfm_pmops,
+   .pm = &dw_mci_exynos_pmops,
},
 };
 
-- 
1.8.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/5] mmc: dw_mmc: Add suspend/resume callbacks; disable irq during suspend

[Doug Anderson]

On some platforms (like exynos5420) the dw_mmc controller may be in a
strange state after we wake up from sleep.  Add callbacks to allow for
dealing with these quirks.  Prevent interrupts from firing when we're
suspended since this strange state may cause interrupts to fire.

In my case I saw the WAKEUP_INT interrupt firing upon resume and
needed to add some code to handle this.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 12 
 drivers/mmc/host/dw_mmc.h |  4 
 2 files changed, 16 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index f20273e..2aaa93f 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2434,6 +2434,7 @@ EXPORT_SYMBOL(dw_mci_remove);
  */
 int dw_mci_suspend(struct dw_mci *host)
 {
+   const struct dw_mci_drv_data *drv_data = host->drv_data;
int i, ret = 0;
 
for (i = 0; i < host->num_slots; i++) {
@@ -2454,14 +2455,25 @@ int dw_mci_suspend(struct dw_mci *host)
if (host->vmmc)
regulator_disable(host->vmmc);
 
+   disable_irq(host->irq);
+
+   if (drv_data && drv_data->suspend)
+   drv_data->suspend(host);
+
return 0;
 }
 EXPORT_SYMBOL(dw_mci_suspend);
 
 int dw_mci_resume(struct dw_mci *host)
 {
+   const struct dw_mci_drv_data *drv_data = host->drv_data;
int i, ret;
 
+   if (drv_data && drv_data->resume)
+   drv_data->resume(host);
+
+   enable_irq(host->irq);
+
if (host->vmmc) {
ret = regulator_enable(host->vmmc);
if (ret) {
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 0b74189..52a3266 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -190,6 +190,8 @@ extern int dw_mci_resume(struct dw_mci *host);
  * @prepare_command: handle CMD register extensions.
  * @set_ios: handle bus specific extensions.
  * @parse_dt: parse implementation specific device tree properties.
+ * @suspend: called late in the suspend process
+ * @resume: called early in the resume process
  *
  * Provide controller implementation specific extensions. The usage of this
  * data structure is fully optional and usage of each member in this structure
@@ -202,5 +204,7 @@ struct dw_mci_drv_data {
void(*prepare_command)(struct dw_mci *host, u32 *cmdr);
void(*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
+   void(*suspend)(struct dw_mci *host);
+   void(*resume)(struct dw_mci *host);
 };
 #endif /* _DW_MMC_H_ */
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/5] mmc: dw_mmc: Always setup the bus after suspend/resume

[Doug Anderson]

After suspend/resume all of the dw_mmc registers are reset to
defaults.  We restore most of them, but specifically don't setup the
clock registers after resume unless we've got a powered card.  Things
still work because the core will eventually call set_ios() and we'll
set things up.

There doesn't seem to be any reason that I can see _not_ to set things
up after resume.  Restoring this state makes the code easier to reason
about and should help prevent bugs.  It also allows us to do a
register dump before and after suspend/resume to confirm that we've
set things up OK.

I examined the state of the dw_mmc instance before and after suspend
after this patch.  I had no card inserted in an SD card slot.

Before this patch, differences were:
* CLKDIV (0x08)
* CLKENA (0x10)
* TMOUT (0x14)
* CMD (0x2C) - difference is not important
* CLKSEL (0x9C - exynos specific)

After this patch, only TMOUT was different.  I have a separate patch
for that.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2aaa93f..a0a07df 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2510,9 +2510,7 @@ int dw_mci_resume(struct dw_mci *host)
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
continue;
-   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
-   dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
-   }
+   dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
 
ret = mmc_resume_host(host->slot[i]->mmc);
if (ret < 0)
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/5] mmc: dw_mmc: Add exynos resume callback to clear WAKEUP_INT

[Doug Anderson]

If the WAKEUP_INT is asserted at wakeup and not cleared, we'll end up
looping around forever.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc-exynos.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index f013e7e..84d3b78 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -30,6 +30,7 @@
 #define SDMMC_CLKSEL_TIMING(x, y, z)   (SDMMC_CLKSEL_CCLK_SAMPLE(x) |  \
SDMMC_CLKSEL_CCLK_DRIVE(y) |\
SDMMC_CLKSEL_CCLK_DIVIDER(z))
+#define SDMMC_CLKSEL_WAKEUP_INTBIT(11)
 
 #define SDMMC_CMD_USE_HOLD_REG BIT(29)
 
@@ -102,6 +103,27 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
return 0;
 }
 
+/**
+ * dw_mci_exynos_resume - Exynos-specific resume code
+ *
+ * We have seen cases (at least on the exynos5420) where turning off the INT
+ * power rail during suspend will leave the WAKEUP_INT bit in the CLKSEL
+ * register asserted.  This bit is 1 to indicate that it fired and we can
+ * clear it by writing a 1 back.  Clear it to prevent interrupts from going off
+ * constantly.
+ */
+
+static int dw_mci_exynos_resume(struct dw_mci *host)
+{
+   u32 clksel;
+
+   clksel = mci_readl(host, CLKSEL);
+   if (clksel & SDMMC_CLKSEL_WAKEUP_INT)
+   mci_writel(host, CLKSEL, clksel);
+
+   return 0;
+}
+
 static void dw_mci_exynos_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
/*
@@ -165,6 +187,7 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.caps   = exynos_dwmmc_caps,
.init   = dw_mci_exynos_priv_init,
.setup_clock= dw_mci_exynos_setup_clock,
+   .resume = dw_mci_exynos_resume,
.prepare_command= dw_mci_exynos_prepare_command,
.set_ios= dw_mci_exynos_set_ios,
.parse_dt   = dw_mci_exynos_parse_dt,
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/5] mmc: dw_mmc: Set timeout to max upon resume

[Doug Anderson]

The TMOUT register is initted to 0x at probe time but isn't
initted after suspend/resume.  Add an init of this value.

No problems were observed without this (it will also get initted in
__dw_mci_start_request if there is data to send), but it makes the
register dump before and after suspend clean.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index a0a07df..eedb517 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2494,6 +2494,9 @@ int dw_mci_resume(struct dw_mci *host)
/* Restore the old value at FIFOTH register */
mci_writel(host, FIFOTH, host->fifoth_val);
 
+   /* Put in max timeout */
+   mci_writel(host, TMOUT, 0x);
+
mci_writel(host, RINTSTS, 0x);
mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/5] mmc: dw_mmc: Invalidate cache of current_speed after suspend/resume

[Doug Anderson]

The dw_mmc driver keeps a cache of the current slot->clock in order to
avoid doing a whole lot of work every time set_ios() is called.
However, after suspend/resume the register values are bogus so we need
to ensure that the cached value is invalidated.

In many cases we got by without this since the core mmc code fiddles
with the clock a lot.  If we've got a card present we're probably
running it at something like 50MHz and the core will temporarily
switch us to 400kHz after resume.  One case that didn't work (for me)
is the case of having no card in the slot.  The slot is initted to
400kHz at boot time.  After suspend/resume the slot thinks it's still
at 400kHz (due to the cache) so doesn't adjust timing.  When it tries
to send the command at probe time it just times out and gets left in a
bad state.

Invalidating the current_speed also means that we don't need to call:
  dw_mci_setup_bus(slot, true);
...to force an update of the clock in the case when the slot was left
powered.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bc3a1bc..f20273e 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2488,13 +2488,18 @@ int dw_mci_resume(struct dw_mci *host)
   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
 
+   /*
+* Invalidate the 'current_speed' value since CLKDIV has some up in
+* default state and our cache is incorrect.
+*/
+   host->current_speed = 0x;
+
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
continue;
if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
-   dw_mci_setup_bus(slot, true);
}
 
ret = mmc_resume_host(host->slot[i]->mmc);
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/5] mmc: dw_mmc: fixes for suspend/resume on exynos

[Doug Anderson]

This series of patches addresses some suspend/resume problems with
dw_mmc on exynos platforms.  Since suspend/resume is not fully working
on ToT Linux (3.10) on exynos5250-snow, this series was tested against
the current ToT ChromeOS 3.8 tree.  I have confirmed basic booting
and eMMC / SD card usage (and compiling, honest!) against ToT Linux.


Doug Anderson (5):
  mmc: dw_mmc: Invalidate cache of current_speed after suspend/resume
  mmc: dw_mmc: Add suspend/resume callbacks; disable irq during suspend
  mmc: dw_mmc: Add exynos resume callback to clear WAKEUP_INT
  mmc: dw_mmc: Always setup the bus after suspend/resume
  mmc: dw_mmc: Set timeout to max upon resume

 drivers/mmc/host/dw_mmc-exynos.c | 23 +++
 drivers/mmc/host/dw_mmc.c| 26 ++
 drivers/mmc/host/dw_mmc.h|  4 
 3 files changed, 49 insertions(+), 4 deletions(-)

-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/5] mmc: dw_mmc: Add exynos resume callback to clear WAKEUP_INT

[Doug Anderson]

Hi,

On Tue, Jul 9, 2013 at 10:31 AM, Doug Anderson  wrote:
> If the WAKEUP_INT is asserted at wakeup and not cleared, we'll end up
> looping around forever.
>
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mmc/host/dw_mmc-exynos.c | 23 +++
>  1 file changed, 23 insertions(+)

Grant just pointed out that the WAKEUP_INT is supposed to only be
enabled if bits 8, 9, or 10 are 1.  Our driver never sets those so we
_should_ never get a WAKEUP_INT.  Bits 8-10 are marked as RESERVED on
the exynos5420 manual, so the current guess is that they're broken on
that silicon but that sometimes the interrupt fires anyway.

In any case, it is still a reasonable thing to clear this interrupt at
wakeup if it has fired, even if we're on an exynos device without any
problems.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mmc: dw_mmc: Handle DW_MCI_QUIRK_IDMAC_DTO properly

[Doug Anderson]

In (1fb5f68 mmc: dw_mmc: Don't loop when handling an interrupt), the
code for handling DW_MCI_QUIRK_IDMAC_DTO became dead code.  Move it to
where it ought to live.

Found by code inspection and compile-tested only--I don't know of any
boards that need DW_MCI_QUIRK_IDMAC_DTO.

Signed-off-by: Doug Anderson 
---
 drivers/mmc/host/dw_mmc.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bc3a1bc..cdc0940 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1595,18 +1595,17 @@ static irqreturn_t dw_mci_interrupt(int irq, void 
*dev_id)
 
pending = mci_readl(host, MINTSTS); /* read-only mask reg */
 
-   if (pending) {
-
-   /*
-* DTO fix - version 2.10a and below, and only if internal DMA
-* is configured.
-*/
-   if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO) {
-   if (!pending &&
-   ((mci_readl(host, STATUS) >> 17) & 0x1fff))
-   pending |= SDMMC_INT_DATA_OVER;
-   }
+   /*
+* DTO fix - version 2.10a and below, and only if internal DMA
+* is configured.
+*/
+   if (host->quirks & DW_MCI_QUIRK_IDMAC_DTO) {
+   if (!pending &&
+   ((mci_readl(host, STATUS) >> 17) & 0x1fff))
+   pending |= SDMMC_INT_DATA_OVER;
+   }
 
+   if (pending) {
if (pending & DW_MCI_CMD_ERROR_FLAGS) {
mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS);
host->cmd_status = pending;
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/5] mmc: dw_mmc: Add suspend/resume callbacks; disable irq during suspend

[Doug Anderson]

James,

On Tue, Jul 9, 2013 at 2:17 PM, James Hogan  wrote:
> Hi Doug,
>
> On 9 July 2013 18:31, Doug Anderson  wrote:
>> On some platforms (like exynos5420) the dw_mmc controller may be in a
>> strange state after we wake up from sleep.  Add callbacks to allow for
>> dealing with these quirks.  Prevent interrupts from firing when we're
>> suspended since this strange state may cause interrupts to fire.
>>
>> In my case I saw the WAKEUP_INT interrupt firing upon resume and
>> needed to add some code to handle this.
>>
>> Signed-off-by: Doug Anderson 
>
> Would it make sense to take advantage of the {suspend,resume}_noirq
> power management callbacks to clear that WAKEUP_INT before interrupts
> are re-enabled, rather than explicitly disabling and enabling the
> interrupt at the suspend/resume stage?

That's a good suggestion.  Let me give it a shot and get back to you
after I validate that it works.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 5/5] mmc: dw_mmc: Set timeout to max upon resume

[Doug Anderson]

The TMOUT register is initted to 0x at probe time but isn't
initted after suspend/resume.  Add an init of this value.

No problems were observed without this (it will also get initted in
__dw_mci_start_request if there is data to send), but it makes the
register dump before and after suspend clean.

Signed-off-by: Doug Anderson 
---
Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index be095b7..d2c5db3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2482,6 +2482,9 @@ int dw_mci_resume(struct dw_mci *host)
/* Restore the old value at FIFOTH register */
mci_writel(host, FIFOTH, host->fifoth_val);
 
+   /* Put in max timeout */
+   mci_writel(host, TMOUT, 0x);
+
mci_writel(host, RINTSTS, 0x);
mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 1/5] mmc: dw_mmc: Invalidate cache of current_speed after suspend/resume

[Doug Anderson]

The dw_mmc driver keeps a cache of the current slot->clock in order to
avoid doing a whole lot of work every time set_ios() is called.
However, after suspend/resume the register values are bogus so we need
to ensure that the cached value is invalidated.

In many cases we got by without this since the core mmc code fiddles
with the clock a lot.  If we've got a card present we're probably
running it at something like 50MHz and the core will temporarily
switch us to 400kHz after resume.  One case that didn't work (for me)
is the case of having no card in the slot.  The slot is initted to
400kHz at boot time.  After suspend/resume the slot thinks it's still
at 400kHz (due to the cache) so doesn't adjust timing.  When it tries
to send the command at probe time it just times out and gets left in a
bad state.

Invalidating the current_speed also means that we don't need to call:
  dw_mci_setup_bus(slot, true);
...to force an update of the clock in the case when the slot was left
powered.

Signed-off-by: Doug Anderson 
---
Changes in v2:
- Fix typo (some -> come)
- Use ~0 instead of 0x; add comment about value

 drivers/mmc/host/dw_mmc.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bc3a1bc..7a5ce6a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2488,13 +2488,19 @@ int dw_mci_resume(struct dw_mci *host)
   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
 
+   /*
+* Invalidate the 'current_speed' value since CLKDIV has come up in
+* default state and our cache is incorrect; set to something we know
+* slot->clock won't be.
+*/
+   host->current_speed = ~0;
+
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
continue;
if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
-   dw_mci_setup_bus(slot, true);
}
 
ret = mmc_resume_host(host->slot[i]->mmc);
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 3/5] mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT

[Doug Anderson]

If the WAKEUP_INT is asserted at wakeup and not cleared, we'll end up
looping around forever.  This has been seen to happen on exynos5420
silicon despite the fact that we haven't enabled any wakeup events.

Signed-off-by: Doug Anderson 
---
Changes in v2:
- Use suspend_noirq as per James Hogan.

 drivers/mmc/host/dw_mmc-exynos.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index f013e7e..36b9620 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -30,6 +30,7 @@
 #define SDMMC_CLKSEL_TIMING(x, y, z)   (SDMMC_CLKSEL_CCLK_SAMPLE(x) |  \
SDMMC_CLKSEL_CCLK_DRIVE(y) |\
SDMMC_CLKSEL_CCLK_DIVIDER(z))
+#define SDMMC_CLKSEL_WAKEUP_INTBIT(11)
 
 #define SDMMC_CMD_USE_HOLD_REG BIT(29)
 
@@ -102,6 +103,27 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
return 0;
 }
 
+/**
+ * dw_mci_exynos_resume_noirq - Exynos-specific resume code
+ *
+ * We have seen cases (at least on the exynos5420) where turning off the INT
+ * power rail during suspend will leave the WAKEUP_INT bit in the CLKSEL
+ * register asserted.  This bit is 1 to indicate that it fired and we can
+ * clear it by writing a 1 back.  Clear it to prevent interrupts from going off
+ * constantly.
+ */
+
+static int dw_mci_exynos_resume_noirq(struct dw_mci *host)
+{
+   u32 clksel;
+
+   clksel = mci_readl(host, CLKSEL);
+   if (clksel & SDMMC_CLKSEL_WAKEUP_INT)
+   mci_writel(host, CLKSEL, clksel);
+
+   return 0;
+}
+
 static void dw_mci_exynos_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
/*
@@ -165,6 +187,7 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.caps   = exynos_dwmmc_caps,
.init   = dw_mci_exynos_priv_init,
.setup_clock= dw_mci_exynos_setup_clock,
+   .resume_noirq   = dw_mci_exynos_resume_noirq,
.prepare_command= dw_mci_exynos_prepare_command,
.set_ios= dw_mci_exynos_set_ios,
.parse_dt   = dw_mci_exynos_parse_dt,
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 4/5] mmc: dw_mmc: Always setup the bus after suspend/resume

[Doug Anderson]

After suspend/resume all of the dw_mmc registers are reset to
defaults.  We restore most of them, but specifically don't setup the
clock registers after resume unless we've got a powered card.  Things
still work because the core will eventually call set_ios() and we'll
set things up.

There doesn't seem to be any reason that I can see _not_ to set things
up after resume.  Restoring this state makes the code easier to reason
about and should help prevent bugs.  It also allows us to do a
register dump before and after suspend/resume to confirm that we've
set things up OK.

I examined the state of the dw_mmc instance before and after suspend
after this patch.  I had no card inserted in an SD card slot.

Before this patch, differences were:
* CLKDIV (0x08)
* CLKENA (0x10)
* TMOUT (0x14)
* CMD (0x2C) - difference is not important
* CLKSEL (0x9C - exynos specific)

After this patch, only TMOUT was different.  I have a separate patch
for that.

Signed-off-by: Doug Anderson 
---
Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7a5ce6a..be095b7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2499,9 +2499,7 @@ int dw_mci_resume(struct dw_mci *host)
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
continue;
-   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
-   dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
-   }
+   dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
 
ret = mmc_resume_host(host->slot[i]->mmc);
if (ret < 0)
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 0/5] mmc: dw_mmc: fixes for suspend/resume on exynos

[Doug Anderson]

This series of patches addresses some suspend/resume problems with
dw_mmc on exynos platforms.  Since suspend/resume is not fully working
on ToT Linux (3.10) on exynos5250-snow, this series was tested against
the current ToT ChromeOS 3.8 tree.  I have confirmed basic booting
and eMMC / SD card usage (and compiling, honest!) against ToT Linux.

Changes in v2:
- Fix typo (some -> come)
- Use ~0 instead of 0x; add comment about value
- Use suspend_noirq as per James Hogan.

Doug Anderson (5):
  mmc: dw_mmc: Invalidate cache of current_speed after suspend/resume
  mmc: dw_mmc: Add suspend_noirq/resume_noirq callbacks for dw_mmc-pltfm
  mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT
  mmc: dw_mmc: Always setup the bus after suspend/resume
  mmc: dw_mmc: Set timeout to max upon resume

 drivers/mmc/host/dw_mmc-exynos.c | 23 +++
 drivers/mmc/host/dw_mmc-pltfm.c  | 37 ++---
 drivers/mmc/host/dw_mmc.c| 15 +++
 drivers/mmc/host/dw_mmc.h|  4 
 4 files changed, 72 insertions(+), 7 deletions(-)

-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/5] mmc: dw_mmc: Add suspend_noirq/resume_noirq callbacks for dw_mmc-pltfm

[Doug Anderson]

On some devices (like exynos5420) the dw_mmc controller may be in a
strange state after we wake up from sleep.  Add callbacks to allow for
dealing with these quirks.  We use the "_noirq" versions of the
callbacks since in the case of exynos5420 the strange state caused
interrupts to fire so we need to deal with it while interrupts are
still off.

At the moment this support is only added to dw_mmc-pltfm which calls
straight to the callback, since nobody but exynos needs it.  We can
add some levels of indirection (a call into the generic dw_mmc code)
when someone finds a need.

Signed-off-by: Doug Anderson 
---
Changes in v2:
- Use suspend_noirq as per James Hogan.

 drivers/mmc/host/dw_mmc-pltfm.c | 37 ++---
 drivers/mmc/host/dw_mmc.h   |  4 
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index 41c27b7..220568c 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -105,12 +105,43 @@ static int dw_mci_pltfm_resume(struct device *dev)
 
return 0;
 }
+
+static int dw_mci_pltfm_suspend_noirq(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+   const struct dw_mci_drv_data *drv_data = host->drv_data;
+
+   if (drv_data && drv_data->suspend_noirq)
+   return drv_data->suspend_noirq(host);
+
+   return 0;
+}
+
+static int dw_mci_pltfm_resume_noirq(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+   const struct dw_mci_drv_data *drv_data = host->drv_data;
+
+   if (drv_data && drv_data->resume_noirq)
+   return drv_data->resume_noirq(host);
+
+   return 0;
+}
+
+
 #else
-#define dw_mci_pltfm_suspend   NULL
-#define dw_mci_pltfm_resumeNULL
+#define dw_mci_pltfm_suspend   NULL
+#define dw_mci_pltfm_resumeNULL
+#define dw_mci_pltfm_suspend_noirq NULL
+#define dw_mci_pltfm_resume_noirq  NULL
 #endif /* CONFIG_PM_SLEEP */
 
-SIMPLE_DEV_PM_OPS(dw_mci_pltfm_pmops, dw_mci_pltfm_suspend, 
dw_mci_pltfm_resume);
+const struct dev_pm_ops dw_mci_pltfm_pmops = {
+   SET_SYSTEM_SLEEP_PM_OPS(dw_mci_pltfm_suspend, dw_mci_pltfm_resume)
+   .suspend_noirq = dw_mci_pltfm_suspend_noirq,
+   .resume_noirq = dw_mci_pltfm_resume_noirq,
+};
+
 EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
 
 static const struct of_device_id dw_mci_pltfm_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 0b74189..5d0398f 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -190,6 +190,8 @@ extern int dw_mci_resume(struct dw_mci *host);
  * @prepare_command: handle CMD register extensions.
  * @set_ios: handle bus specific extensions.
  * @parse_dt: parse implementation specific device tree properties.
+ * @suspend_noirq: called late in the suspend process
+ * @resume_noirq: called early in the resume process
  *
  * Provide controller implementation specific extensions. The usage of this
  * data structure is fully optional and usage of each member in this structure
@@ -202,5 +204,7 @@ struct dw_mci_drv_data {
void(*prepare_command)(struct dw_mci *host, u32 *cmdr);
void(*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
+   int (*suspend_noirq)(struct dw_mci *host);
+   int (*resume_noirq)(struct dw_mci *host);
 };
 #endif /* _DW_MMC_H_ */
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 3/5] mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT

[Doug Anderson]

Seungwon,

On Wed, Jul 10, 2013 at 7:54 AM, Seungwon Jeon  wrote:
> On Wed, July 10, 2013, Doug Anderson wrote:
>> If the WAKEUP_INT is asserted at wakeup and not cleared, we'll end up
>> looping around forever.  This has been seen to happen on exynos5420
>> silicon despite the fact that we haven't enabled any wakeup events.
>>
>> Signed-off-by: Doug Anderson 
>> ---
>> Changes in v2:
>> - Use suspend_noirq as per James Hogan.
>>
>>  drivers/mmc/host/dw_mmc-exynos.c | 23 +++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
>> b/drivers/mmc/host/dw_mmc-exynos.c
>> index f013e7e..36b9620 100644
>> --- a/drivers/mmc/host/dw_mmc-exynos.c
>> +++ b/drivers/mmc/host/dw_mmc-exynos.c
>> @@ -30,6 +30,7 @@
>>  #define SDMMC_CLKSEL_TIMING(x, y, z) (SDMMC_CLKSEL_CCLK_SAMPLE(x) |  \
>>   SDMMC_CLKSEL_CCLK_DRIVE(y) |\
>>   SDMMC_CLKSEL_CCLK_DIVIDER(z))
>> +#define SDMMC_CLKSEL_WAKEUP_INT  BIT(11)
>>
>>  #define SDMMC_CMD_USE_HOLD_REG   BIT(29)
>>
>> @@ -102,6 +103,27 @@ static int dw_mci_exynos_setup_clock(struct dw_mci 
>> *host)
>>   return 0;
>>  }
>>
>> +/**
>> + * dw_mci_exynos_resume_noirq - Exynos-specific resume code
>> + *
>> + * We have seen cases (at least on the exynos5420) where turning off the INT
>> + * power rail during suspend will leave the WAKEUP_INT bit in the CLKSEL
>> + * register asserted.  This bit is 1 to indicate that it fired and we can
>> + * clear it by writing a 1 back.  Clear it to prevent interrupts from going 
>> off
>> + * constantly.
>> + */
> As I know this bit is auto-cleared.
> Did you find the cause of this problem?
> How about your GPIO setting in sleep?
> Currently, we don't know why the problem is happened.
> At least, we should make it clear.

Yes, the documentation that I have says that this bit is "auto
cleared" as well but doesn't indicate under what conditions it is auto
cleared.  From testing how this bit reacts I have found that writing a
1 to it clears the bit--in other words it behaves like bits in
RINTSTS.  That's a terrible design for a bit in a register with shared
config but appears to be how it works.

Note: in a sense it will be "auto cleared" because doing a
read-modify-write of any other bits in this register will clear the
interrupt.

I have asked for official confirmation.

We have found that on exynos5420 bits 8-10 of CLKSEL are marked as
RESERVED.  Those bits are documented to enable the WAKEUP_INT on
exynos5250.  My best guess is that these bits are not used on
exynos5420 and the WAKEUP_INT line is left floating, which means it
can fire randomly.  I have also asked for official confirmation about
this.


I will likely merge this change locally in our kernel tree while
waiting for a response.  If you would like to wait before Acking
that's very reasonable.  Do you have any other problems with this
change assuming my understanding above is correct?

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/5] mmc: dw_mmc: Add suspend_noirq/resume_noirq callbacks for dw_mmc-pltfm

[Doug Anderson]

James,

On Wed, Jul 10, 2013 at 1:37 AM, James Hogan  wrote:
>> -SIMPLE_DEV_PM_OPS(dw_mci_pltfm_pmops, dw_mci_pltfm_suspend, 
>> dw_mci_pltfm_resume);
>> +const struct dev_pm_ops dw_mci_pltfm_pmops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(dw_mci_pltfm_suspend, dw_mci_pltfm_resume)
>> + .suspend_noirq = dw_mci_pltfm_suspend_noirq,
>> + .resume_noirq = dw_mci_pltfm_resume_noirq,
>> +};
>
> Does Exynos support hibernation? I see that SET_SYSTEM_SLEEP_PM_OPS sets
> freeze, thaw, poweroff, and restore callbacks too. You may not need the
> hibernation specific _noirq callbacks though in which case it's probably
> fine as it is.

Thank you for your review and good suggestions.  You're right that I
should add the other "noirq" variants in here.  Even if hibernation
isn't supported now that's the right thing to do.  I will fix that and
send v3 with your "Reviewed-by".


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/5] mmc: dw_mmc: Invalidate cache of current_speed after suspend/resume

[Doug Anderson]

The dw_mmc driver keeps a cache of the current slot->clock in order to
avoid doing a whole lot of work every time set_ios() is called.
However, after suspend/resume the register values are bogus so we need
to ensure that the cached value is invalidated.

In many cases we got by without this since the core mmc code fiddles
with the clock a lot.  If we've got a card present we're probably
running it at something like 50MHz and the core will temporarily
switch us to 400kHz after resume.  One case that didn't work (for me)
is the case of having no card in the slot.  The slot is initted to
400kHz at boot time.  After suspend/resume the slot thinks it's still
at 400kHz (due to the cache) so doesn't adjust timing.  When it tries
to send the command at probe time it just times out and gets left in a
bad state.

Invalidating the current_speed also means that we don't need to call:
  dw_mci_setup_bus(slot, true);
...to force an update of the clock in the case when the slot was left
powered.

Signed-off-by: Doug Anderson 
---
Changes in v3: None
Changes in v2:
- Fix typo (some -> come)
- Use ~0 instead of 0x; add comment about value

 drivers/mmc/host/dw_mmc.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bc3a1bc..7a5ce6a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2488,13 +2488,19 @@ int dw_mci_resume(struct dw_mci *host)
   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
mci_writel(host, CTRL, SDMMC_CTRL_INT_ENABLE);
 
+   /*
+* Invalidate the 'current_speed' value since CLKDIV has come up in
+* default state and our cache is incorrect; set to something we know
+* slot->clock won't be.
+*/
+   host->current_speed = ~0;
+
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
continue;
if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
-   dw_mci_setup_bus(slot, true);
}
 
ret = mmc_resume_host(host->slot[i]->mmc);
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 4/5] mmc: dw_mmc: Always setup the bus after suspend/resume

[Doug Anderson]

After suspend/resume all of the dw_mmc registers are reset to
defaults.  We restore most of them, but specifically don't setup the
clock registers after resume unless we've got a powered card.  Things
still work because the core will eventually call set_ios() and we'll
set things up.

There doesn't seem to be any reason that I can see _not_ to set things
up after resume.  Restoring this state makes the code easier to reason
about and should help prevent bugs.  It also allows us to do a
register dump before and after suspend/resume to confirm that we've
set things up OK.

I examined the state of the dw_mmc instance before and after suspend
after this patch.  I had no card inserted in an SD card slot.

Before this patch, differences were:
* CLKDIV (0x08)
* CLKENA (0x10)
* TMOUT (0x14)
* CMD (0x2C) - difference is not important
* CLKSEL (0x9C - exynos specific)

After this patch, only TMOUT was different.  I have a separate patch
for that.

Signed-off-by: Doug Anderson 
---
Changes in v3: None
Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7a5ce6a..be095b7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2499,9 +2499,7 @@ int dw_mci_resume(struct dw_mci *host)
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
continue;
-   if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
-   dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
-   }
+   dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
 
ret = mmc_resume_host(host->slot[i]->mmc);
if (ret < 0)
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 5/5] mmc: dw_mmc: Set timeout to max upon resume

[Doug Anderson]

The TMOUT register is initted to 0x at probe time but isn't
initted after suspend/resume.  Add an init of this value.

No problems were observed without this (it will also get initted in
__dw_mci_start_request if there is data to send), but it makes the
register dump before and after suspend clean.

Signed-off-by: Doug Anderson 
Acked-by: Seungwon Jeon 
---
Changes in v3: None
Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index be095b7..d2c5db3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2482,6 +2482,9 @@ int dw_mci_resume(struct dw_mci *host)
/* Restore the old value at FIFOTH register */
mci_writel(host, FIFOTH, host->fifoth_val);
 
+   /* Put in max timeout */
+   mci_writel(host, TMOUT, 0x);
+
mci_writel(host, RINTSTS, 0x);
mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 2/5] mmc: dw_mmc: Add suspend_noirq/resume_noirq callbacks for dw_mmc-pltfm

[Doug Anderson]

On some devices (like exynos5420) the dw_mmc controller may be in a
strange state after we wake up from sleep.  Add callbacks to allow for
dealing with these quirks.  We use the "_noirq" versions of the
callbacks since in the case of exynos5420 the strange state caused
interrupts to fire so we need to deal with it while interrupts are
still off.

At the moment this support is only added to dw_mmc-pltfm which calls
straight to the callback, since nobody but exynos needs it.  We can
add some levels of indirection (a call into the generic dw_mmc code)
when someone finds a need.

Signed-off-by: Doug Anderson 
Reviewed-by: James Hogan 
---
Changes in v3:
- Add freeze/thaw and poweroff/restore noirq entries.

Changes in v2:
- Use suspend_noirq as per James Hogan.

 drivers/mmc/host/dw_mmc-pltfm.c | 41 ++---
 drivers/mmc/host/dw_mmc.h   |  4 
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index 41c27b7..742ef76 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -105,12 +105,47 @@ static int dw_mci_pltfm_resume(struct device *dev)
 
return 0;
 }
+
+static int dw_mci_pltfm_suspend_noirq(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+   const struct dw_mci_drv_data *drv_data = host->drv_data;
+
+   if (drv_data && drv_data->suspend_noirq)
+   return drv_data->suspend_noirq(host);
+
+   return 0;
+}
+
+static int dw_mci_pltfm_resume_noirq(struct device *dev)
+{
+   struct dw_mci *host = dev_get_drvdata(dev);
+   const struct dw_mci_drv_data *drv_data = host->drv_data;
+
+   if (drv_data && drv_data->resume_noirq)
+   return drv_data->resume_noirq(host);
+
+   return 0;
+}
+
+
 #else
-#define dw_mci_pltfm_suspend   NULL
-#define dw_mci_pltfm_resumeNULL
+#define dw_mci_pltfm_suspend   NULL
+#define dw_mci_pltfm_resumeNULL
+#define dw_mci_pltfm_suspend_noirq NULL
+#define dw_mci_pltfm_resume_noirq  NULL
 #endif /* CONFIG_PM_SLEEP */
 
-SIMPLE_DEV_PM_OPS(dw_mci_pltfm_pmops, dw_mci_pltfm_suspend, 
dw_mci_pltfm_resume);
+const struct dev_pm_ops dw_mci_pltfm_pmops = {
+   SET_SYSTEM_SLEEP_PM_OPS(dw_mci_pltfm_suspend, dw_mci_pltfm_resume)
+   .suspend_noirq = dw_mci_pltfm_suspend_noirq,
+   .resume_noirq = dw_mci_pltfm_resume_noirq,
+   .freeze_noirq = dw_mci_pltfm_suspend_noirq,
+   .thaw_noirq = dw_mci_pltfm_resume_noirq,
+   .poweroff_noirq = dw_mci_pltfm_suspend_noirq,
+   .restore_noirq = dw_mci_pltfm_resume_noirq,
+};
+
 EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
 
 static const struct of_device_id dw_mci_pltfm_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 0b74189..5d0398f 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -190,6 +190,8 @@ extern int dw_mci_resume(struct dw_mci *host);
  * @prepare_command: handle CMD register extensions.
  * @set_ios: handle bus specific extensions.
  * @parse_dt: parse implementation specific device tree properties.
+ * @suspend_noirq: called late in the suspend process
+ * @resume_noirq: called early in the resume process
  *
  * Provide controller implementation specific extensions. The usage of this
  * data structure is fully optional and usage of each member in this structure
@@ -202,5 +204,7 @@ struct dw_mci_drv_data {
void(*prepare_command)(struct dw_mci *host, u32 *cmdr);
void(*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
+   int (*suspend_noirq)(struct dw_mci *host);
+   int (*resume_noirq)(struct dw_mci *host);
 };
 #endif /* _DW_MMC_H_ */
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 3/5] mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT

[Doug Anderson]

If the WAKEUP_INT is asserted at wakeup and not cleared, we'll end up
looping around forever.  This has been seen to happen on exynos5420
silicon despite the fact that we haven't enabled any wakeup events.

Signed-off-by: Doug Anderson 
---
Changes in v3: None
Changes in v2:
- Use suspend_noirq as per James Hogan.

 drivers/mmc/host/dw_mmc-exynos.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index f013e7e..36b9620 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -30,6 +30,7 @@
 #define SDMMC_CLKSEL_TIMING(x, y, z)   (SDMMC_CLKSEL_CCLK_SAMPLE(x) |  \
SDMMC_CLKSEL_CCLK_DRIVE(y) |\
SDMMC_CLKSEL_CCLK_DIVIDER(z))
+#define SDMMC_CLKSEL_WAKEUP_INTBIT(11)
 
 #define SDMMC_CMD_USE_HOLD_REG BIT(29)
 
@@ -102,6 +103,27 @@ static int dw_mci_exynos_setup_clock(struct dw_mci *host)
return 0;
 }
 
+/**
+ * dw_mci_exynos_resume_noirq - Exynos-specific resume code
+ *
+ * We have seen cases (at least on the exynos5420) where turning off the INT
+ * power rail during suspend will leave the WAKEUP_INT bit in the CLKSEL
+ * register asserted.  This bit is 1 to indicate that it fired and we can
+ * clear it by writing a 1 back.  Clear it to prevent interrupts from going off
+ * constantly.
+ */
+
+static int dw_mci_exynos_resume_noirq(struct dw_mci *host)
+{
+   u32 clksel;
+
+   clksel = mci_readl(host, CLKSEL);
+   if (clksel & SDMMC_CLKSEL_WAKEUP_INT)
+   mci_writel(host, CLKSEL, clksel);
+
+   return 0;
+}
+
 static void dw_mci_exynos_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
/*
@@ -165,6 +187,7 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.caps   = exynos_dwmmc_caps,
.init   = dw_mci_exynos_priv_init,
.setup_clock= dw_mci_exynos_setup_clock,
+   .resume_noirq   = dw_mci_exynos_resume_noirq,
.prepare_command= dw_mci_exynos_prepare_command,
.set_ios= dw_mci_exynos_set_ios,
.parse_dt   = dw_mci_exynos_parse_dt,
-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 0/5] mmc: dw_mmc: fixes for suspend/resume on exynos

[Doug Anderson]

This series of patches addresses some suspend/resume problems with
dw_mmc on exynos platforms.  Since suspend/resume is not fully working
on ToT Linux (3.10) on exynos5250-snow, this series was tested against
the current ToT ChromeOS 3.8 tree.  I have confirmed basic booting
and eMMC / SD card usage (and compiling, honest!) against ToT Linux.

Changes in v3:
- Add freeze/thaw and poweroff/restore noirq entries.

Changes in v2:
- Fix typo (some -> come)
- Use ~0 instead of 0x; add comment about value
- Use suspend_noirq as per James Hogan.

Doug Anderson (5):
  mmc: dw_mmc: Invalidate cache of current_speed after suspend/resume
  mmc: dw_mmc: Add suspend_noirq/resume_noirq callbacks for dw_mmc-pltfm
  mmc: dw_mmc: Add exynos resume_noirq callback to clear WAKEUP_INT
  mmc: dw_mmc: Always setup the bus after suspend/resume
  mmc: dw_mmc: Set timeout to max upon resume

 drivers/mmc/host/dw_mmc-exynos.c | 23 ++
 drivers/mmc/host/dw_mmc-pltfm.c  | 41 +---
 drivers/mmc/host/dw_mmc.c| 15 +++
 drivers/mmc/host/dw_mmc.h|  4 
 4 files changed, 76 insertions(+), 7 deletions(-)

-- 
1.8.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Input: gpio_keys - wakeup_trigger

[Doug Anderson]

Benson,

On Fri, Sep 13, 2013 at 2:52 PM, Benson Leung  wrote:
> Allow wakeup_trigger to be defined per gpio button. Currently, all
> gpio buttons are set up as IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING.
> It may be more appropriate to only wake the system on one edge, for example
> if the gpio is for a Lid Switch.
>
> Signed-off-by: Benson Leung 

As discussed out of band, I'd tend to put the masking/error checking
in gpio_keys_get_devtree_pdata() then just rely on non-DT users not to
pass in nonsense, but I don't feel that strongly about it, so:

Reviewed-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression parsing GPT (EFI) partition tables

[Doug Anderson]

Hi,

Just ran into this same problem and tracked it down to the same
commit.  Luckily Sean found this thread.  :)

On Wed, Oct 9, 2013 at 5:37 PM, Davidlohr Bueso  wrote:
> Hi Josh,
>
> On Wed, 2013-10-09 at 16:26 -0700, Josh Triplett wrote:
>> When testing ChromeOS with a 3.12 kernel from git, I encountered a
>> regression introduced between 3.11 to 3.12: at boot time, the kernel
>> failed to find any partitions on the USB disk I booted from, which uses
>> a GPT partition table with 12 partitions.  This prevented the system
>> from booting.
>>
>> Reverting all the patches to block/partitions/efi.c between 3.11 and
>> 3.12 allowed the system to detect partitions again.  The patches I
>> reverted:
>>
>> 6b02fa5 partitions/efi: loosen check fot pmbr size in lba
>> b4bc4a1 block/partitions/efi.c: consistently use pr_foo()
>> 70f637e partitions/efi: some style cleanups
>> aa054bc partitions/efi: compare first and last usable LBAs
>> 27a7c64 partitions/efi: account for pmbr size in lba
>> b05ebbb partitions/efi: detect hybrid MBRs
>> 3e69ac3 partitions/efi: do not require gpt partition to begin at sector 1
>> 33afd7a partitions/efi: check pmbr record's starting lba
>> c2ebdc2 partitions/efi: use lba-aware partition records
>
> It would be good to bisect this :)
> I suspect it might be caused by 33afd7a otherwise there's something
> wrong with the mbr size in lba (6b02fa5+27a7c64).

So I found that (c2ebdc2 partitions/efi: use lba-aware partition
records) broke things but that breakage is short-lived.  Specifically
in c2ebdc2 we changed from looking at "part->start_sect" to
"part->start_sector", but "part->start_sect" is equivalent to
"part->starting_lba" in the new scheme.

...moving forward, I then found the next breakage was 27a7c64 and that
appears to be the culprit.  Adjusting to further changes on ToT, you
get a fix that looks roughly like:

diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index 1eb09ee..9df329d 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -226,7 +226,8 @@ check_hybrid:
if (ret == GPT_MBR_PROTECTIVE) {
sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
if (sz != (uint32_t) total_sectors - 1 && sz != 0x)
-   ret = 0;
+   pr_warn("%s sz=%u, total_sectors - 1=%u\n", __func__,
+   sz, (uint32_t)((uint32_t) total_sectors - 1));
}
 done:
return ret;

Now, when I boot up I see messages like:

[4.038359] is_pmbr_valid sz=4956095, total_sectors - 1=15523839
[4.043963] GPT:Primary header thinks Alt. header is not at the end
of the disk.
[4.043967] GPT:4956095 != 15523839
[4.043969] GPT:Alternate GPT header not at the end of the disk.
[4.043972] GPT:4956095 != 15523839
[4.043975] GPT: Use GNU Parted to correct GPT errors.

...so basically it looks like we're now considering something an error
that used to be considered a warning.

I don't know __anything__ about how GPT is supposed to work and I'll
leave it to the experts to debate.  It always is possible that the
Chrome OS GPT is somehow wrong, but Bill Richardson (CCed) says he
thinks that what we're doing is OK and that the alternate header is
supposed to be a backup copy (and thus it should only be a warning if
it's missing).


> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index 1eb09ee..d589937 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -157,10 +157,6 @@ static inline int pmbr_part_valid(gpt_mbr_record *part)
> if (part->os_type != EFI_PMBR_OSTYPE_EFI_GPT)
> goto invalid;
>
> -   /* set to 0x0001 (i.e., the LBA of the GPT Partition Header) */
> -   if (le32_to_cpu(part->starting_lba) != 
> GPT_PRIMARY_PARTITION_TABLE_LBA)
> -   goto invalid;
> -
> return GPT_MBR_PROTECTIVE;
>  invalid:
> return 0;
>
>
> If this doesn't work, could you bypass mbr checks with force_gpt option
> and add some printk to the partition_record.size_in_lba, like:
>
> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index 1eb09ee..77cfed2 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -225,6 +225,7 @@ check_hybrid:
>  */
> if (ret == GPT_MBR_PROTECTIVE) {
> sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
> +   printk("gpt sz check: %d, %ld\n", sz, total_sectors);
> if (sz != (uint32_t) total_sectors - 1 && sz != 0x)
> ret = 0;
> }

Just for thoroughness, I did try this patch (without my patch) and it
didn't work for me.



-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression parsing GPT (EFI) partition tables

[Doug Anderson]

Hi,

On Thu, Oct 10, 2013 at 2:26 PM, Davidlohr Bueso  wrote:
> On Thu, 2013-10-10 at 13:15 -0700, Doug Anderson wrote:
>> Hi,
>>
>> Just ran into this same problem and tracked it down to the same
>> commit.  Luckily Sean found this thread.  :)
>>
>> On Wed, Oct 9, 2013 at 5:37 PM, Davidlohr Bueso  wrote:
>> > Hi Josh,
>> >
>> > On Wed, 2013-10-09 at 16:26 -0700, Josh Triplett wrote:
>> >> When testing ChromeOS with a 3.12 kernel from git, I encountered a
>> >> regression introduced between 3.11 to 3.12: at boot time, the kernel
>> >> failed to find any partitions on the USB disk I booted from, which uses
>> >> a GPT partition table with 12 partitions.  This prevented the system
>> >> from booting.
>> >>
>> >> Reverting all the patches to block/partitions/efi.c between 3.11 and
>> >> 3.12 allowed the system to detect partitions again.  The patches I
>> >> reverted:
>> >>
>> >> 6b02fa5 partitions/efi: loosen check fot pmbr size in lba
>> >> b4bc4a1 block/partitions/efi.c: consistently use pr_foo()
>> >> 70f637e partitions/efi: some style cleanups
>> >> aa054bc partitions/efi: compare first and last usable LBAs
>> >> 27a7c64 partitions/efi: account for pmbr size in lba
>> >> b05ebbb partitions/efi: detect hybrid MBRs
>> >> 3e69ac3 partitions/efi: do not require gpt partition to begin at sector 1
>> >> 33afd7a partitions/efi: check pmbr record's starting lba
>> >> c2ebdc2 partitions/efi: use lba-aware partition records
>> >
>> > It would be good to bisect this :)
>> > I suspect it might be caused by 33afd7a otherwise there's something
>> > wrong with the mbr size in lba (6b02fa5+27a7c64).
>>
>> So I found that (c2ebdc2 partitions/efi: use lba-aware partition
>> records) broke things but that breakage is short-lived.  Specifically
>> in c2ebdc2 we changed from looking at "part->start_sect" to
>> "part->start_sector", but "part->start_sect" is equivalent to
>> "part->starting_lba" in the new scheme.
>
> Right, because we're not using CHS addressing anymore for GPT. What do
> you mean by "breakage is short-lived"? I didn't quite get that.

Sorry, it's "short-lived" because the CLs right after this effectively
change it back to looking at "starting_lba" again.


>> ...moving forward, I then found the next breakage was 27a7c64 and that
>> appears to be the culprit.  Adjusting to further changes on ToT, you
>> get a fix that looks roughly like:
>>
>> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
>> index 1eb09ee..9df329d 100644
>> --- a/block/partitions/efi.c
>> +++ b/block/partitions/efi.c
>> @@ -226,7 +226,8 @@ check_hybrid:
>> if (ret == GPT_MBR_PROTECTIVE) {
>> sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
>> if (sz != (uint32_t) total_sectors - 1 && sz != 0x)
>> -   ret = 0;
>> +   pr_warn("%s sz=%u, total_sectors - 1=%u\n", __func__,
>> +   sz, (uint32_t)((uint32_t) total_sectors - 
>> 1));
>> }
>>  done:
>> return ret;
>>
>> Now, when I boot up I see messages like:
>>
>> [4.038359] is_pmbr_valid sz=4956095, total_sectors - 1=15523839
>
> Hmmm, this confuses me: sz represents the size of the disk and should be
> equal to total_sectors - 1 (aka lastlba) in this case, and since you're
> not using the whole 2Tb disk limit, so it'll never be 0x
>
>> [4.043963] GPT:Primary header thinks Alt. header is not at the end
>> of the disk.
>> [4.043967] GPT:4956095 != 15523839
>
> but in reality, sz is equal to where the primary GPT header thinks the
> alternate header is (pgpt->alternate_lba):
>
> if (le64_to_cpu(pgpt->alternate_lba) != lastlba) {
> pr_warn("GPT:Primary header thinks Alt. header is not at the 
> end of the disk.\n");
> pr_warn("GPT:%lld != %lld\n",
> (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
> (unsigned long long)lastlba);
> error_found++;
> }
>
>> [4.043969] GPT:Alternate GPT header not at the end of the disk.
>> [4.043972] GPT:4956095 != 15523839
>> [4.043975] GPT: Use GNU Parted to correct GPT errors.
>
> At least the backup

[PATCH] partitions/efi: treat size mismatch as a warning, not an error

[Doug Anderson]

In (27a7c64 partitions/efi: account for pmbr size in lba) we started
treating bad sizes in lba field of the partition that has the 0xEE
(GPT protective) as errors.  However, we may run into these "bad
sizes" in the real world if someone uses dd to copy an image from a
smaller disk to a bigger disk.  Since this case used to work (even
without using force_gpt), keep it working and treat the size mismatch
as a warning instead of an error.

Reported-by: Josh Triplett 
Reported-by: Sean Paul 
Signed-off-by: Doug Anderson 
---
 block/partitions/efi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index 1eb09ee..ac23dc1 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -222,11 +222,15 @@ check_hybrid:
 * the disk size.
 *
 * Hybrid MBRs do not necessarily comply with this.
+*
+* Consider a bad value here to be a warning to support dd-ing
+* an image from a smaller disk to a bigger disk.
 */
if (ret == GPT_MBR_PROTECTIVE) {
sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
if (sz != (uint32_t) total_sectors - 1 && sz != 0x)
-   ret = 0;
+   pr_warn("%s: mbr size mismatch (%u != %u)\n", __func__,
+   sz, (uint32_t)((uint32_t) total_sectors - 1));
}
 done:
return ret;
-- 
1.8.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Regression parsing GPT (EFI) partition tables

[Doug Anderson]

Davidlohr,

On Thu, Oct 10, 2013 at 3:31 PM, Davidlohr Bueso  wrote:
> Then you should *really* use the force_gpt option, which is there to
> bypass any MBR checks, and you can avoid issues like this :)
>
> Anyway, this is still a regression and I believe we can go ahead and
> just warn the user about the case instead of not recognizing the disk.
>
> Bill/Doug, care to send a formal patch (with corresponding comments)?

OK, I've posted up at .
As I've said, I haven't spent the time to really understand every last
detail (I was just doing dumb git bisects), so if my explanation /
comments don't actually make sense then please correct me.

Thanks for your help in tracking this down!

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ARM: EXYNOS: no duplicate mask/unmask in eint0_15

[Doug Anderson]

On Thu, Sep 6, 2012 at 8:21 AM, Daniel Kurtz  wrote:
> chained_irq_enter/exit() already mask&ack/unmask the chained interrupt.
> There is no need to also explicitly do it in the handler.
>
> Signed-off-by: Daniel Kurtz 
> ---
>  arch/arm/mach-exynos/common.c |7 ---
>  1 files changed, 0 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 4eb39cd..0a85aec 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -965,14 +965,7 @@ static void exynos_irq_eint0_15(unsigned int irq, struct 
> irq_desc *desc)
> struct irq_chip *chip = irq_get_chip(irq);
>
> chained_irq_enter(chip, desc);
> -   chip->irq_mask(&desc->irq_data);
> -
> -   if (chip->irq_ack)
> -   chip->irq_ack(&desc->irq_data);
> -
> generic_handle_irq(*irq_data);
> -
> -   chip->irq_unmask(&desc->irq_data);
> chained_irq_exit(chip, desc);
>  }
>
> --
> 1.7.7.3
>

Acked-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH 3/3] dt: Document: Add optional MAX77686 operating mode bindings

[Doug Anderson]

Abhilash,

Thanks for posting up these patches.  Just going to do my commenting
directly on the documentation patch since they are high-level
comments.

On Sun, Dec 9, 2012 at 10:26 PM, Abhilash Kesavan  wrote:
> Add documenatation for various operating mode capabilities of
> the MAX77686 PMIC.
>
> Signed-off-by: Abhilash Kesavan 
> ---
>  Documentation/devicetree/bindings/mfd/max77686.txt |6 ++

This should probably be squashed with PATCH 2/3.  It seems to be best
to introduce documentation in the same patch as first usage.


>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt 
> b/Documentation/devicetree/bindings/mfd/max77686.txt
> index c6a3469..f2867a9 100644
> --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> @@ -13,6 +13,12 @@ Required properties:
>  - interrupts : This i2c device has an IRQ line connected to the main SoC.
>  - interrupt-parent : The parent interrupt controller.
>
> +optional properties:
> +- max77686-opmode : The regulators of max77686 have various operating mode
> +  capabilities such as low power mode, standby mode (controlled by PWRREQ
> +  signal) etc. Check the regulator CTRL register for the bits setting these
> +  modes.

I have a feeling we're going to want something a little more generic
than just whacking a register value straight into the device tree.

I would propose the following for LDOs to match the generic regulator
define, but someone with more device tree experience may have a better
idea.  The idea is to map to the generic set_suspend_mode() /
set_mode():
  regulator-suspend-mode = 
  regulator-mode = 

The max77686 appears to support NORMAL and IDLE mode for most LDOs, so
the only reasonable values for these two would be 2 and 4.  It appears
that it would also be nonsensical (impossible to map to a register
value) to have REGULATOR_MODE_NORMAL for suspend and
REGULATOR_MODE_IDLE for the normal running mode.


For bucks 1-3, you'd just have a boolean property mapping to
set_suspend_disable().  Perhaps:
  regulator-suspend-en;

That wouldn't allow access to the "Forced low power mode" of bucks
1-3, but I don't think there's any code to support that now so nobody
must be using it.


Given that all of the above maps nicely into regulator constants and
regulator functions, I'd imagine that support could be added straight
into the generic regulator code and you won't have to touch max77686
at all (unless you wanted to add support for set_mode(), which doesn't
seem to be there).


All of the above would set _defaults_ for regulators.  If device-tree
people think it's wise, we could add "-default" into the names.

> +
>  Optional node:
>  - voltage-regulators : The regulators of max77686 have to be instantiated
>under subnode named "voltage-regulators" using the following format.
> --
> 1.7.8.6
>

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [RFC] cpufreq: can't raise max frequency with cpu_thermal

[Doug Anderson]

On Tue, Dec 18, 2012 at 12:29 AM, Sonny Rao  wrote:
> The cpu_thermal generic thermal management code has a bug where once
> max cpu frequency has been lowered in sysfs (scaling_max_freq) it is
> not possible to raise the max back up later.  The bug is that the
> notifer gets called by __cpufreq_set_policy() before the user policy
> max is raised, and is incorrectly trying to enforce the max frequency
> policy even when we are trying to change the policy.  It is also not
> clear why this driver is looking at the user policy since it is
> primarily supposed to enforce thermal policy, not user set policy.
>
> Signed-off-by: Sonny Rao 
> ---
>  drivers/thermal/cpu_cooling.c |4 
>  1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 836828e..63bc708 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -219,10 +219,6 @@ static int cpufreq_thermal_notifier(struct 
> notifier_block *nb,
> if (cpumask_test_cpu(policy->cpu, ¬ify_device->allowed_cpus))
> max_freq = notify_device->cpufreq_val;
>
> -   /* Never exceed user_policy.max*/
> -   if (max_freq > policy->user_policy.max)
> -   max_freq = policy->user_policy.max;
> -
> if (policy->max != max_freq)
> cpufreq_verify_within_limits(policy, 0, max_freq);
>
> --
> 1.7.7.3
>

Sonny's change matches what the "ACPI version" of this code
(drivers/acpi/processor_thermal.c) does as well.  I would certainly be
interested to know why the code was added here in the first place.
Amit: do you know?

Reviewed-by: Doug Anderson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/