Re: [PATCHv2] media: i2c/adp1653: devicetree support for adp1653

2015-01-04 Thread Pavel Machek
Hi!

> Thanks for the patch! A few comments below.
> 
> On Wed, Dec 24, 2014 at 11:34:34PM +0100, Pavel Machek wrote:
> > 
> > We are moving to device tree support on OMAP3, but that currently
> > breaks ADP1653 driver. This adds device tree support, plus required
> > documentation.
> > 
> > Signed-off-by: Pavel Machek 
> > 
> > ---
> > 
> > Changed -microsec to -us, as requested by devicetree people.
> > 
> > Fixed checkpatch issues.
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/common.txt 
> > b/Documentation/devicetree/bindings/leds/common.txt
> > index 2d88816..2c6c7c5 100644
> > --- a/Documentation/devicetree/bindings/leds/common.txt
> > +++ b/Documentation/devicetree/bindings/leds/common.txt
> > @@ -14,6 +14,15 @@ Optional properties for child nodes:
> >   "ide-disk" - LED indicates disk activity
> >   "timer" - LED flashes at a fixed, configurable rate
> >  
> > +- max-microamp : maximum intensity in microamperes of the LED
> > +(torch LED for flash devices)
> 
> s/torch LED/torch mode/
> 
> > +- flash-max-microamp : maximum intensity in microamperes of the
> > +   flash LED; it is mandatory if the LED should
> > +  support the flash mode
> > +- flash-timeout-microsec : timeout in microseconds after which the flash
> > +   LED is turned off
> 
> These should go to a different patch.

Actually these both should not be in this patch in the first place.

> > +  - reg: I2C slave address
> > +
> > +  - gpios: References to the GPIO that controls the power for the chip.
> > +
> > +There are two led outputs available - flash and indicator. One led is
> > +represented by one child node, nodes need to be named "flash" and 
> > "indicator".
> 
> 80 characters per line.

Count them. It is.

> > +
> > +Required properties of the LED child node:
> > +- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
> > +
> > +Required properties of the flash LED child node:
> > +
> > +- flash-max-microamp : see 
> > Documentation/devicetree/bindings/leds/common.txt
> > +- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
> > +
> > +Example:
> > +
> > +adp1653: led-controller@30 {
> > +compatible = "adi,adp1653";
> > +   reg = <0x30>;
> > +gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
> 
> Please use tabs for indentation above (and below).

Ok.

> > --- a/arch/arm/boot/dts/omap3-n900.dts
> > +++ b/arch/arm/boot/dts/omap3-n900.dts
> > @@ -553,6 +558,22 @@
> >  
> > ti,usb-charger-detection = <&isp1704>;
> > };
> > +
> > +   adp1653: led-controller@30 {
> > +   compatible = "adi,adp1653";
> > +   reg = <0x30>;
> > +   gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
> > +
> > +   flash {
> > +   flash-timeout-us = <50>;
> > +   flash-max-microamp = <32>;
> > +   max-microamp = <5>;
> > +   };
> > +
> > +   indicator {
> > +   max-microamp = <17500>;
> > +   };
> > +   };
> 
> This should go to a separate patch as well.

How many patches do I need to do for one trivial change? :-(.


> > +   if (flash->platform_data->power)
> > +   ret = flash->platform_data->power(&flash->subdev, on);
> 
> if () {
> } else {
> }

Ok.

> > +   else {
> > +   gpio_set_value(flash->platform_data->power_gpio, on);
> 
> Shouldn't you add this to the platform data struct?

I don't see what you mean.

> power_gpio is actually a poor name for this, as is the "power" callback.
> This is really "EN" gpio in the spec, I'd call it perhaps just "gpio", or
> "enable_gpio".

Feel free to clean that that up in followup patch.

> > +   if (on) {
> > +   /* Some delay is apparently required. */
> > +   udelay(20);
> 
> The driver should always handle the delay, platform data or not. This
> reminds me --- is there a need to retain the support for platform data? I
> don't think it's being used anywhere. I'm fine with both keeping and
> removing it.

Lets do that in followup patch, if needed.

> > +   child = of_get_child_by_name(node, "indicator");
> > +   if (!child)
> > +   return -EINVAL;
> 
> Do you require an indicator to be connected? I think it shouldn't be
> mandatory, at least the driver should work without it, even if it
> exposes
> the control (making that conditional would be a subject for another patch,
> but that doesn't need to be done now).

Another patch, if someone needs it, yes.

> > +   if (of_property_read_u32(child, "max-microamp", &val))
> > +   return -EINVAL;
> > +   pd->max_indicator_intensity = val;
> > +
> > +   if (!of_find_property(node, "gpios", NULL)) {
> > +   dev_err(&client->dev, "No gpio node\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   gpio = of_get_gpio_flags(node, 0, &flags);
> 
> You could assign t

[PATCHv3] media: i2c/adp1653: devicetree support for adp1653

2015-01-04 Thread Pavel Machek

We are moving to device tree support on OMAP3, but that currently
breaks ADP1653 driver. This adds device tree support, plus required
documentation.

Signed-off-by: Pavel Machek 

---

Please apply,
Pavel

diff --git a/Documentation/devicetree/bindings/media/i2c/adp1653.txt 
b/Documentation/devicetree/bindings/media/i2c/adp1653.txt
new file mode 100644
index 000..0fc28a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/adp1653.txt
@@ -0,0 +1,37 @@
+* Analog Devices ADP1653 flash LED driver
+
+Required Properties:
+
+  - compatible: Must contain be "adi,adp1653"
+
+  - reg: I2C slave address
+
+  - gpios: References to the GPIO that controls the power for the chip.
+
+There are two led outputs available - flash and indicator. One led is
+represented by one child node, nodes need to be named "flash" and "indicator".
+
+Required properties of the LED child node:
+- max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+
+Required properties of the flash LED child node:
+
+- flash-max-microamp : see Documentation/devicetree/bindings/leds/common.txt
+- flash-timeout-us : see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+   adp1653: led-controller@30 {
+   compatible = "adi,adp1653";
+   reg = <0x30>;
+   gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>; /* 88 */
+
+   flash {
+   flash-timeout-us = <50>;
+   flash-max-microamp = <32>;
+   max-microamp = <5>;
+   };
+   indicator {
+   max-microamp = <17500>;
+   };
+   };
diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index 873fe19..0341009 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -8,6 +8,7 @@
  * Contributors:
  * Sakari Ailus 
  * Tuukka Toivonen 
+ * Pavel Machek 
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -34,6 +35,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 
@@ -306,9 +309,17 @@ adp1653_init_device(struct adp1653_flash *flash)
 static int
 __adp1653_set_power(struct adp1653_flash *flash, int on)
 {
-   int ret;
+   int ret = 0;
+
+   if (flash->platform_data->power) {
+   ret = flash->platform_data->power(&flash->subdev, on);
+   } else {
+   gpio_set_value(flash->platform_data->power_gpio, on);
+   if (on)
+   /* Some delay is apparently required. */
+   udelay(20);
+   }
 
-   ret = flash->platform_data->power(&flash->subdev, on);
if (ret < 0)
return ret;
 
@@ -316,8 +327,13 @@ __adp1653_set_power(struct adp1653_flash *flash, int on)
return 0;
 
ret = adp1653_init_device(flash);
-   if (ret < 0)
+   if (ret >= 0)
+   return ret;
+
+   if (flash->platform_data->power)
flash->platform_data->power(&flash->subdev, 0);
+   else
+   gpio_set_value(flash->platform_data->power_gpio, 0);
 
return ret;
 }
@@ -407,21 +423,77 @@ static int adp1653_resume(struct device *dev)
 
 #endif /* CONFIG_PM */
 
+static int adp1653_of_init(struct i2c_client *client,
+  struct adp1653_flash *flash,
+  struct device_node *node)
+{
+   u32 val;
+   struct adp1653_platform_data *pd;
+   enum of_gpio_flags flags;
+   int gpio;
+   struct device_node *child;
+
+   if (!node)
+   return -EINVAL;
+
+   pd = devm_kzalloc(&client->dev, sizeof(*pd), GFP_KERNEL);
+   if (!pd)
+   return -ENOMEM;
+   flash->platform_data = pd;
+
+   child = of_get_child_by_name(node, "flash");
+   if (!child)
+   return -EINVAL;
+   if (of_property_read_u32(child, "flash-timeout-microsec", &val))
+   return -EINVAL;
+
+   pd->max_flash_timeout = val;
+   if (of_property_read_u32(child, "flash-max-microamp", &val))
+   return -EINVAL;
+   pd->max_flash_intensity = val/1000;
+
+   if (of_property_read_u32(child, "max-microamp", &val))
+   return -EINVAL;
+   pd->max_torch_intensity = val/1000;
+
+   child = of_get_child_by_name(node, "indicator");
+   if (!child)
+   return -EINVAL;
+   if (of_property_read_u32(child, "max-microamp", &val))
+   return -EINVAL;
+   pd->max_indicator_intensity = val;
+
+   if (!of_find_property(node, "gpios", NULL)) {
+   dev_err(&client->dev, "No gpio node\n");
+   return -EINVAL;
+   }
+
+   pd->power_gpio = of_get_gpio_flags(node, 0, &flags);
+   if (pd->power_gpio < 0) {
+   dev_err(&client->dev, "Error getting 

Re: OMAP baseline test results for v3.19-rc1

2015-01-04 Thread Arnd Bergmann
On Saturday 03 January 2015 15:44:38 Aaro Koskinen wrote:
> 
> On Sat, Jan 03, 2015 at 01:51:53PM +0100, Arnd Bergmann wrote:
> > On Saturday 03 January 2015 07:21:49 Paul Walmsley wrote:
> > > Another ~300KB kernel object size increase for omap2plus_defconfig 
> > > kernels.
> > 
> > 300kb seems like a lot for a single release. Have you looked into
> > where this is coming from?
> 
> In 3.19-rc1, all initrd decompressors are by default enabled...
> 
>  CONFIG_RD_GZIP=y
> -# CONFIG_RD_BZIP2 is not set
> -# CONFIG_RD_LZMA is not set
> -# CONFIG_RD_XZ is not set
> -# CONFIG_RD_LZO is not set
> -# CONFIG_RD_LZ4 is not set
> +CONFIG_RD_BZIP2=y
> +CONFIG_RD_LZMA=y
> +CONFIG_RD_XZ=y
> +CONFIG_RD_LZO=y
> +CONFIG_RD_LZ4=y
> 

Interesting, this seems to be the opposite of the intention of the
change that caused it, which tried to let you disable the algorithms
without selecting CONFIG_EXPERT. For some reason that is impossible
to know now, they already defaulted to 'n' when EXPERT was set, but
now they default to 'y'.

Does this indeed cause the entire change in size?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: OMAP baseline test results for v3.19-rc1

2015-01-04 Thread Arnd Bergmann
On Saturday 03 January 2015 20:28:28 Paul Walmsley wrote:
> On Sat, 3 Jan 2015, Arnd Bergmann wrote:
> 
> > On Saturday 03 January 2015 07:21:49 Paul Walmsley wrote:
> > > Another ~300KB kernel object size increase for omap2plus_defconfig 
> > > kernels.
> > 
> > 300kb seems like a lot for a single release. 
> 
> +300KB is high, but not exceptionally so.
> 
> The data, since I started keeping track of it, is below.  The last numeric 
> column represents the total kernel object size delta in bytes.

I see.

> http://www.pwsan.com/omap/testlogs/
> 
> test_v3.7-rc1/20121017205513/build/size.txt:+128332   -67728+2144   
> +62748  omap2plus_defconfig
> test_v3.8-rc1/20121228031713/build/size.txt:+168843   +25288 +900  
> +195031  omap2plus_defconfig
> test_v3.9-rc1/20130312100243/build/size.txt:+195310   +37968+1364  
> +234642  omap2plus_defconfig
> test_v3.10-rc1/20130518212204/build/size.txt: -59854   -98552   +37136  
> -121270  omap2plus_defconfig
> test_v3.11-rc1/20130721020309/build/size.txt:+159173-2456+1680  
> +158397  omap2plus_defconfig
> test_v3.12-rc1/20130922202452/build_z/size.txt:+237402   +47344 +760  
> +285506  omap2plus_defconfig
> test_v3.13-rc1/20131208173326/build_z/size.txt:-276029-5584+5008  
> -276605  omap2plus_defconfig
> test_v3.14-rc1/20140210035354/build_z/size.txt: +94801   -17448-9016   
> +68337  omap2plus_defconfig
> test_v3.15-rc1/20140421113253/build_z/size.txt: +30722   +11392+4312   
> +46426  omap2plus_defconfig
> test_v3.16-rc1/20140629224344/build_z/size.txt:+133191   +20776  +2750728  
> +2904695  omap2plus_defconfig
> test_v3.17-rc1/20140821122707/build_z/size.txt: +54469   -34424+1728   
> +21773  omap2plus_defconfig
> test_v3.18-rc1/20141020095901/build_z/size.txt:+677950   +32008   +54408  
> +764366  omap2plus_defconfig
> test_v3.19-rc1/20150102151849/build_z/size.txt:+297698+9648-4528  
> +302818  omap2plus_defconfig

So 3.16 was by far the largest increase, almost 10x bigger than the 3.19 
increase, but
others are quite big as well.

Do you know what happened in 3.16?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] hsi: nokia-modem: fix uninitialized device pointer

2015-01-04 Thread Aaro Koskinen
modem->device was never initialized. This resulted in logs such as:

[  241.386322] (NULL device *): CMT rst line change detected

Signed-off-by: Aaro Koskinen 
---
 drivers/hsi/clients/nokia-modem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hsi/clients/nokia-modem.c 
b/drivers/hsi/clients/nokia-modem.c
index f0c2145..eb4dc63 100644
--- a/drivers/hsi/clients/nokia-modem.c
+++ b/drivers/hsi/clients/nokia-modem.c
@@ -162,6 +162,7 @@ static int nokia_modem_probe(struct device *dev)
return -ENOMEM;
}
dev_set_drvdata(dev, modem);
+   modem->device = dev;
 
irq = irq_of_parse_and_map(np, 0);
if (!irq) {
-- 
2.2.0

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


Re: OMAP baseline test results for v3.19-rc1

2015-01-04 Thread Paul Walmsley
On Sun, 4 Jan 2015, Arnd Bergmann wrote:

> So 3.16 was by far the largest increase, almost 10x bigger than the 3.19 
> increase, but
> others are quite big as well.
> 
> Do you know what happened in 3.16?

It was commit 1413c03893332366e5b4d1e26f942ada25f3e82a ("lockdep: Increase 
static allocations").


- Paul

commit 1413c03893332366e5b4d1e26f942ada25f3e82a
Author: Sasha Levin 
Date:   Wed Jan 8 14:21:46 2014 -0500

lockdep: Increase static allocations

Fuzzing a recent kernel with a large configuration hits the static
allocation limits and disables lockdep.

This patch doubles the limits.

Signed-off-by: Sasha Levin 
Signed-off-by: Peter Zijlstra 
Link: 
http://lkml.kernel.org/r/1389208906-24338-1-git-send-email-sasha.le...@oracle.com
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Ingo Molnar 

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


Re: [PATCH] hsi: nokia-modem: fix uninitialized device pointer

2015-01-04 Thread Sebastian Reichel
Hi Aaro,

On Sun, Jan 04, 2015 at 07:34:13PM +0200, Aaro Koskinen wrote:
> modem->device was never initialized. This resulted in logs such as:
> [  241.386322] (NULL device *): CMT rst line change detected

Thanks, applied to for-next:

https://git.kernel.org/cgit/linux/kernel/git/sre/linux-hsi.git/commit/?h=for-next&id=67e9a2ce6e070b1e5db2dfe9c61890fc30ae954d

-- Sebastian


signature.asc
Description: Digital signature


Re: OMAP baseline test results for v3.19-rc1

2015-01-04 Thread Aaro Koskinen
Hi,

On Sun, Jan 04, 2015 at 05:46:31PM +0100, Arnd Bergmann wrote:
> On Saturday 03 January 2015 15:44:38 Aaro Koskinen wrote:
> > On Sat, Jan 03, 2015 at 01:51:53PM +0100, Arnd Bergmann wrote:
> > > On Saturday 03 January 2015 07:21:49 Paul Walmsley wrote:
> > > > Another ~300KB kernel object size increase for omap2plus_defconfig 
> > > > kernels.
> > > 
> > > 300kb seems like a lot for a single release. Have you looked into
> > > where this is coming from?
> > 
> > In 3.19-rc1, all initrd decompressors are by default enabled...
> 
> Interesting, this seems to be the opposite of the intention of the
> change that caused it, which tried to let you disable the algorithms
> without selecting CONFIG_EXPERT. For some reason that is impossible
> to know now, they already defaulted to 'n' when EXPERT was set, but
> now they default to 'y'.
> 
> Does this indeed cause the entire change in size?

Actually, no. It seems a bigger contributor is enabling SATA
in omap2plus_defconfig. But there still something else... Below is some
data based on quick checks.

v3.18 / omap2plus_defconfig:

text data   bss dec hex filename
8811980  790500 8423536 180260161130e20 vmlinux

v3.19-rc1 / omap2plus_defconfig:

text data   bss dec hex filename
9093858  800316 8419072 18313246117701e vmlinux

v3.19-rc1 / omap2plus_defconfig + revert ec72c666fb34 ("usr/Kconfig: make
initrd compression algorithm selection not expert"):

text data   bss dec hex filename
9070939  800260 8419072 18290271117165f vmlinux

v3.19-rc1 / omap2plus_defconfig + revert 661ea91b676b ("ARM:
omap2plus_defconfig: Enable AHCI_PLATFORM driver"):

text data   bss dec hex filename
8928148  792700 8414784 18135632114ba50 vmlinux

v3.19-rc1 / omap2plus_defconfig + revert ec72c666fb34 & 661ea91b676b:

text data   bss dec hex filename
8901129  792644 8414784 18108557114508d vmlinux

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] ARM: OMAP: hwmod fixes for v3.19-rc

2015-01-04 Thread Paul Walmsley
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi Tony

The following changes since commit 97bf6af1f928216fd6c5a66e8a57bfa95a659672:

  Linux 3.19-rc1 (2014-12-20 17:08:50 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pjw/omap-pending.git 
tags/for-v3.19-rc/omap-fixes-a

for you to fetch changes up to 99d076b747455449b2eec9e37f3fb0bfdf51af32:

  MAINTAINERS: add maintainer for OMAP hwmod data (2015-01-02 16:24:27 -0700)

- 
For v3.19-rc, fix some hwmod structure details for the OMAP DSS modules
for DRA7xx and AM43xx.  Also update the MAINTAINERS file to encourage
folks to cc me on hwmod data patches.

Basic build, boot, and PM testlogs are available here:

http://www.pwsan.com/omap/testlogs/omap-fixes-a-for-v3.19-rc/20150103144242/

- 
Paul Walmsley (1):
  MAINTAINERS: add maintainer for OMAP hwmod data

Tomi Valkeinen (2):
  ARM: AM43xx: hwmod: set DSS submodule parent hwmods
  ARM: DRA7xx: hwmod: set DSS submodule parent hwmods

 MAINTAINERS| 6 ++
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c | 2 ++
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c  | 2 ++
 3 files changed, 10 insertions(+)
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQIcBAEBAgAGBQJUqc29AAoJEMePsQ0LvSpLwHMP/iwMppVchp7aBom6wAJXf5Cf
3d6M8j+5UNex01TDnBAj9fSWC0ea/HkEKpOSVp+TOsLX1cnMPQfC6wBmJ4nZ4HLG
l+7e1x6S0eKCvoMh6yvTzf5hJfmHCs0ULcJJ2E87fNhuSJ+t7ESCaVFSFqCvT2Fi
Rq+9Ba/9Mwe2UHotmwRpt1wNFSzIWuHfi2La5SOwUdPZ/6snpy5blmGpipG4vUJL
Oe3zvY0NVo0Ee/i8Q89UqmE56jStsq6/Lo4rH1yfVTLAnFdR43X8lC8u5uKvaQB9
EELRUvY+chISVVahHFAt2DmuenPBnYIQqZ6I4rgf+RirT7Ee2qEL4Chty5bwrz61
OfGiWEvZ/JmnVvn6SYicZ9wbKL58AL1eVMG6ylHBtjiAkJc2eg5QeS+uNnEBFjIG
6UeVLBQQ1ldvYt/n8L3vyxqeKnf4f9/THg/4hvgYLvhs2Vpj8dL2syknwhfRb1pG
7yLH7pD3wxibAK5gMVLNR8NdDZmfUeI/EyF++CqY5qj5YzN4qTUk2mvpdQI96bg0
mgPs/t1mNg54M2W6TnWLAF7CnowK3UTAu3QxmxQuaxXf5uu0mWmxyAijzl7LTeoj
6Gm/qyLALvE7cTAH/10PjkrvvP0MpZ6Fs9PWLEc3t1/6RoDXZA6BdzQARSjvhgMM
Y2mY3tU2B7IUbycZQIkS
=4el2
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: ethernet: cpsw: fix hangs with interrupts

2015-01-04 Thread David Miller
From: Felipe Balbi 
Date: Fri, 2 Jan 2015 16:15:59 -0600

> The CPSW IP implements pulse-signaled interrupts. Due to
> that we must write a correct, pre-defined value to the
> CPDMA_MACEOIVECTOR register so the controller generates
> a pulse on the correct IRQ line to signal the End Of
> Interrupt.
> 
> The way the driver is written today, all four IRQ lines
> are requested using the same IRQ handler and, because of
> that, we could fall into situations where a TX IRQ fires
> but we tell the controller that we ended an RX IRQ (or
> vice-versa). This situation triggers an IRQ storm on the
> reserved IRQ 127 of INTC which will in turn call ack_bad_irq()
> which will, then, print a ton of:
 ...
> Reported-by: Yegor Yefremov 
> Fixes: 510a1e7 (drivers: net: davinci_cpdma: acknowledge interrupt properly)
> Cc:  # v3.9+
> Signed-off-by: Felipe Balbi 

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines

2015-01-04 Thread Kishon Vijay Abraham I
Hi,

On Thursday 18 December 2014 07:41 PM, Nishanth Menon wrote:
> On 12/18/2014 12:18 AM, Kishon Vijay Abraham I wrote:
>>
>>
>> On Tuesday 16 December 2014 02:20 AM, Nishanth Menon wrote:
>>> On 12/12/2014 02:06 AM, Kishon Vijay Abraham I wrote:
 The reset values for all the PCF lines are high and hence on shutdown
 we should drive all the lines high in order to bring it to the reset state.

 This is actually required since pcf doesn't have a reset line and even 
 after
 warm reset (by invoking "reboot" in prompt) the pcf lines maintains it's
 previous programmed state. This becomes a problem if the boards are 
 designed
 to work with the default initial state.

 DRA7XX_evm uses PCF8575 and one of the PCF output lines feeds to MMC/SD and
 this line should be driven high in order for the MMC/SD to be detected.
 This line is modelled as regulator and the hsmmc driver takes care of 
 enabling
 and disabling it. In the case of 'reboot', during shutdown path as part of 
 it's
 cleanup process the hsmmc driver disables this regulator. This makes MMC 
 boot
 not functional.

 Fixed it by driving high all the pcf lines.

 Signed-off-by: Kishon Vijay Abraham I 
 ---
  drivers/gpio/gpio-pcf857x.c |9 +
  1 file changed, 9 insertions(+)

 diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
 index 236708a..00b15b2 100644
 --- a/drivers/gpio/gpio-pcf857x.c
 +++ b/drivers/gpio/gpio-pcf857x.c
 @@ -448,6 +448,14 @@ static int pcf857x_remove(struct i2c_client *client)
return status;
  }
  
 +static void pcf857x_shutdown(struct i2c_client *client)
 +{
 +  struct pcf857x *gpio = i2c_get_clientdata(client);
 +
 +  /* Drive all the I/O lines high */
 +  gpio->write(gpio->client, BIT(gpio->chip.ngpio) - 1);
>>>
>>> you might force a contention here - depending on System configuration.
>>> example:
>>> +---+
>>> |   |
>>> |  U1   | +--+  +---+
>>> |   +->  |  |   |
>>> +---+ |  |  |   |
>>>   | Switch<-+SoC|
>>> +---+ |  |  |   |
>>> |   | |  |  |   |
>>> | U2<-+--^---+  +---+
>>> |   ||
>>> |   ||
>>> +---+|
>>>   +--+--+
>>>   | |
>>>   | PCF |
>>>   | |
>>>   +-+
>>>
>>> At low, SoC pin is connected to U2 as drive. when reset to high, you
>>> now have U1 driving to the same pin that SoC has, potentially
>>> resulting in contention.
>>>
>>>
>>> Unfortunately, at this level, you do not know what the state of the
>>> system is, blindly forcing a pin level will potentially cause
>>> contention risk depending on pin configuration.
>>
>> Assume we are doing a reset when the system is powered on, irrespective of 
>> the
>> state of the system, we'll be forcing the pin level to the default state.
> 
> Yes, I dont deny that system will be fine *after* reset sequence is
> started or completed. However there is a duration between the pcf
> shutdown handler is called and the final reset handler is invoked -
> that is the duration when  the contention might cause device behavior.
> Essentially ignoring the state various drivers have asked PCF to setup
> the pins and doing a hands down configuration may have side effects we
> cant properly expect.

The solution might be to invoke the shutdown handler of the various drivers
using the PCF before the shutdown handler of the PCF driver itself gets
invoked? But I'm not sure how can that be achieved in linux kernel :-(

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html