Re: [PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

2013-03-21 Thread Wolfram Sang 
On Mon, Mar 04, 2013 at 07:12:53PM +0530, Thomas Abraham wrote:
 With device core now able to setup the default pin configuration,
 the call to devm_pinctrl_get_select_default can be removed. And
 the pin configuration code based on the deprecated Samsung specific
 gpio bindings is also removed.
 
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
 
 Hi Heiko, Tomasz,
 
 We have to make a choice on the path forward for pinctrl support
 on Samsung platforms. We have three cases to deal with.
 
 A. Samsung platforms without DT support.
 B. Samsung platforms with DT support using old Samsung specific
gpio bindings for pin-configuration (s3c24xx and s3x64xx).
 C. Samsung platforms with DT support using using pinctrl based
pin-configurations (Exynos4 and Exynos5).
 
 For [A], we just let the platform specific callbacks handle the
 pin setup. For [B] and [C], based on Linus Walleij's pin grab
 by device core patch and subsequent discussions with him on the
 mailing list, would it be acceptable that we discontinue support
 for [B] in Samsung SoC device drivers. This will impact your
 current DT work on s3c24xx and s3c64xx platforms. Pinctrl is
 inevitable and we have to migrate to it. Instead of workarounds
 to maintain support for [B], it might be better that we migrate
 s3c24xx and s3c64xx platforms to pinctrl.
 
 Please do let us know your opinion on this.

Dropping [B] sounds good, but I think it makes sense to wait with this
patch until pinctrl support for s3c24xx is in place?

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

Re: [PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

2013-03-21 Thread Heiko Stübner 
Am Donnerstag, 21. März 2013, 11:00:32 schrieb Wolfram Sang:
 On Mon, Mar 04, 2013 at 07:12:53PM +0530, Thomas Abraham wrote:
  With device core now able to setup the default pin configuration,
  the call to devm_pinctrl_get_select_default can be removed. And
  the pin configuration code based on the deprecated Samsung specific
  gpio bindings is also removed.
  
  Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
  ---
  
  Hi Heiko, Tomasz,
  
  We have to make a choice on the path forward for pinctrl support
  on Samsung platforms. We have three cases to deal with.
  
  A. Samsung platforms without DT support.
  B. Samsung platforms with DT support using old Samsung specific
  
 gpio bindings for pin-configuration (s3c24xx and s3x64xx).
  
  C. Samsung platforms with DT support using using pinctrl based
  
 pin-configurations (Exynos4 and Exynos5).
  
  For [A], we just let the platform specific callbacks handle the
  pin setup. For [B] and [C], based on Linus Walleij's pin grab
  by device core patch and subsequent discussions with him on the
  mailing list, would it be acceptable that we discontinue support
  for [B] in Samsung SoC device drivers. This will impact your
  current DT work on s3c24xx and s3c64xx platforms. Pinctrl is
  inevitable and we have to migrate to it. Instead of workarounds
  to maintain support for [B], it might be better that we migrate
  s3c24xx and s3c64xx platforms to pinctrl.
  
  Please do let us know your opinion on this.
 
 Dropping [B] sounds good, but I think it makes sense to wait with this
 patch until pinctrl support for s3c24xx is in place?

s3c24xx does not currently have dt support in mainline at all and the non-dt 
init is not touched at all. Also it seems the legacy gpio based dt init does 
not work at all currently, because the pinctrl function are no-ops and 
therefore the legacy gpio init is never reached on such systems.

So I would vote for dropping [B]. I'm already working a bit on a pinctrl 
driver for s3c24xx, so when I reintroduce s3c24xx dt support again, it should 
already have pinctrl support then.


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

Re: [PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

2013-03-06 Thread Linus Walleij 
On Mon, Mar 4, 2013 at 2:42 PM, Thomas Abraham
thomas.abra...@linaro.org wrote:

 With device core now able to setup the default pin configuration,
 the call to devm_pinctrl_get_select_default can be removed. And
 the pin configuration code based on the deprecated Samsung specific
 gpio bindings is also removed.

 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

Acked-by: Linus Walleij linus.wall...@linaro.org

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

2013-03-04 Thread Thomas Abraham 
With device core now able to setup the default pin configuration,
the call to devm_pinctrl_get_select_default can be removed. And
the pin configuration code based on the deprecated Samsung specific
gpio bindings is also removed.

Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
---

Hi Heiko, Tomasz,

We have to make a choice on the path forward for pinctrl support
on Samsung platforms. We have three cases to deal with.

A. Samsung platforms without DT support.
B. Samsung platforms with DT support using old Samsung specific
   gpio bindings for pin-configuration (s3c24xx and s3x64xx).
C. Samsung platforms with DT support using using pinctrl based
   pin-configurations (Exynos4 and Exynos5).

For [A], we just let the platform specific callbacks handle the
pin setup. For [B] and [C], based on Linus Walleij's pin grab
by device core patch and subsequent discussions with him on the
mailing list, would it be acceptable that we discontinue support
for [B] in Samsung SoC device drivers. This will impact your
current DT work on s3c24xx and s3c64xx platforms. Pinctrl is
inevitable and we have to migrate to it. Instead of workarounds
to maintain support for [B], it might be better that we migrate
s3c24xx and s3c64xx platforms to pinctrl.

Please do let us know your opinion on this.

Thanks,
Thomas.

 drivers/i2c/busses/i2c-s3c2410.c |   67 +-
 1 files changed, 1 insertions(+), 66 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index f6b880b..703272c 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -37,8 +37,6 @@
 #include linux/slab.h
 #include linux/io.h
 #include linux/of_i2c.h
-#include linux/of_gpio.h
-#include linux/pinctrl/consumer.h
 
 #include asm/irq.h
 
@@ -84,8 +82,6 @@ struct s3c24xx_i2c {
struct i2c_adapter  adap;
 
struct s3c2410_platform_i2c *pdata;
-   int gpios[2];
-   struct pinctrl  *pctrl;
 #ifdef CONFIG_CPU_FREQ
struct notifier_block   freq_transition;
 #endif
@@ -856,57 +852,6 @@ static inline void s3c24xx_i2c_deregister_cpufreq(struct 
s3c24xx_i2c *i2c)
 }
 #endif
 
-#ifdef CONFIG_OF
-static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
-{
-   int idx, gpio, ret;
-
-   if (i2c-quirks  QUIRK_NO_GPIO)
-   return 0;
-
-   for (idx = 0; idx  2; idx++) {
-   gpio = of_get_gpio(i2c-dev-of_node, idx);
-   if (!gpio_is_valid(gpio)) {
-   dev_err(i2c-dev, invalid gpio[%d]: %d\n, idx, gpio);
-   goto free_gpio;
-   }
-   i2c-gpios[idx] = gpio;
-
-   ret = gpio_request(gpio, i2c-bus);
-   if (ret) {
-   dev_err(i2c-dev, gpio [%d] request failed\n, gpio);
-   goto free_gpio;
-   }
-   }
-   return 0;
-
-free_gpio:
-   while (--idx = 0)
-   gpio_free(i2c-gpios[idx]);
-   return -EINVAL;
-}
-
-static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
-{
-   unsigned int idx;
-
-   if (i2c-quirks  QUIRK_NO_GPIO)
-   return;
-
-   for (idx = 0; idx  2; idx++)
-   gpio_free(i2c-gpios[idx]);
-}
-#else
-static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
-{
-   return 0;
-}
-
-static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
-{
-}
-#endif
-
 /* s3c24xx_i2c_init
  *
  * initialise the controller, set the IO lines and frequency
@@ -1054,15 +999,9 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
i2c-adap.algo_data = i2c;
i2c-adap.dev.parent = pdev-dev;
 
-   i2c-pctrl = devm_pinctrl_get_select_default(i2c-dev);
-
/* inititalise the i2c gpio lines */
-
-   if (i2c-pdata-cfg_gpio) {
+   if (i2c-pdata-cfg_gpio)
i2c-pdata-cfg_gpio(to_platform_device(i2c-dev));
-   } else if (IS_ERR(i2c-pctrl)  s3c24xx_i2c_parse_dt_gpio(i2c)) {
-   return -EINVAL;
-   }
 
/* initialise the i2c controller */
 
@@ -1140,10 +1079,6 @@ static int s3c24xx_i2c_remove(struct platform_device 
*pdev)
i2c_del_adapter(i2c-adap);
 
clk_disable_unprepare(i2c-clk);
-
-   if (pdev-dev.of_node  IS_ERR(i2c-pctrl))
-   s3c24xx_i2c_dt_gpio_free(i2c);
-
return 0;
 }
 
-- 
1.6.6.rc2

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

Re: [PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

2013-03-04 Thread Heiko Stübner 
Hi Thomas,

Am Montag, 4. März 2013, 14:42:53 schrieb Thomas Abraham:
 With device core now able to setup the default pin configuration,
 the call to devm_pinctrl_get_select_default can be removed. And
 the pin configuration code based on the deprecated Samsung specific
 gpio bindings is also removed.
 
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
 
 Hi Heiko, Tomasz,
 
 We have to make a choice on the path forward for pinctrl support
 on Samsung platforms. We have three cases to deal with.
 
 A. Samsung platforms without DT support.
 B. Samsung platforms with DT support using old Samsung specific
gpio bindings for pin-configuration (s3c24xx and s3x64xx).
 C. Samsung platforms with DT support using using pinctrl based
pin-configurations (Exynos4 and Exynos5).
 
 For [A], we just let the platform specific callbacks handle the
 pin setup. For [B] and [C], based on Linus Walleij's pin grab
 by device core patch and subsequent discussions with him on the
 mailing list, would it be acceptable that we discontinue support
 for [B] in Samsung SoC device drivers. This will impact your
 current DT work on s3c24xx and s3c64xx platforms. Pinctrl is
 inevitable and we have to migrate to it. Instead of workarounds
 to maintain support for [B], it might be better that we migrate
 s3c24xx and s3c64xx platforms to pinctrl.
 
 Please do let us know your opinion on this.

As discusses in the other thread, I'm in favor of going forward this way and 
not to invest unnecessary energy in keeping the non-pinctrl stuff alive.

Pinctrl for at least the s3c2416 [0] is already on my playground-wishlist, but 
I'm still in the process of getting to know it.

So for this patch:
Acked-by: Heiko Stuebner he...@sntech.de


Heiko


[0] in contrast to what the gpio-samsung driver implies, the s3c24xx pin banks 
are not uniform between SoCs, making this more difficult still :-)


  drivers/i2c/busses/i2c-s3c2410.c |   67
 +- 1 files changed, 1 insertions(+),
 66 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-s3c2410.c
 b/drivers/i2c/busses/i2c-s3c2410.c index f6b880b..703272c 100644
 --- a/drivers/i2c/busses/i2c-s3c2410.c
 +++ b/drivers/i2c/busses/i2c-s3c2410.c
 @@ -37,8 +37,6 @@
  #include linux/slab.h
  #include linux/io.h
  #include linux/of_i2c.h
 -#include linux/of_gpio.h
 -#include linux/pinctrl/consumer.h
 
  #include asm/irq.h
 
 @@ -84,8 +82,6 @@ struct s3c24xx_i2c {
   struct i2c_adapter  adap;
 
   struct s3c2410_platform_i2c *pdata;
 - int gpios[2];
 - struct pinctrl  *pctrl;
  #ifdef CONFIG_CPU_FREQ
   struct notifier_block   freq_transition;
  #endif
 @@ -856,57 +852,6 @@ static inline void
 s3c24xx_i2c_deregister_cpufreq(struct s3c24xx_i2c *i2c) }
  #endif
 
 -#ifdef CONFIG_OF
 -static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 -{
 - int idx, gpio, ret;
 -
 - if (i2c-quirks  QUIRK_NO_GPIO)
 - return 0;
 -
 - for (idx = 0; idx  2; idx++) {
 - gpio = of_get_gpio(i2c-dev-of_node, idx);
 - if (!gpio_is_valid(gpio)) {
 - dev_err(i2c-dev, invalid gpio[%d]: %d\n, idx, gpio);
 - goto free_gpio;
 - }
 - i2c-gpios[idx] = gpio;
 -
 - ret = gpio_request(gpio, i2c-bus);
 - if (ret) {
 - dev_err(i2c-dev, gpio [%d] request failed\n, gpio);
 - goto free_gpio;
 - }
 - }
 - return 0;
 -
 -free_gpio:
 - while (--idx = 0)
 - gpio_free(i2c-gpios[idx]);
 - return -EINVAL;
 -}
 -
 -static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 -{
 - unsigned int idx;
 -
 - if (i2c-quirks  QUIRK_NO_GPIO)
 - return;
 -
 - for (idx = 0; idx  2; idx++)
 - gpio_free(i2c-gpios[idx]);
 -}
 -#else
 -static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 -{
 - return 0;
 -}
 -
 -static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 -{
 -}
 -#endif
 -
  /* s3c24xx_i2c_init
   *
   * initialise the controller, set the IO lines and frequency
 @@ -1054,15 +999,9 @@ static int s3c24xx_i2c_probe(struct platform_device
 *pdev) i2c-adap.algo_data = i2c;
   i2c-adap.dev.parent = pdev-dev;
 
 - i2c-pctrl = devm_pinctrl_get_select_default(i2c-dev);
 -
   /* inititalise the i2c gpio lines */
 -
 - if (i2c-pdata-cfg_gpio) {
 + if (i2c-pdata-cfg_gpio)
   i2c-pdata-cfg_gpio(to_platform_device(i2c-dev));
 - } else if (IS_ERR(i2c-pctrl)  s3c24xx_i2c_parse_dt_gpio(i2c)) {
 - return -EINVAL;
 - }
 
   /* initialise the i2c controller */
 
 @@ -1140,10 +1079,6 @@ static int s3c24xx_i2c_remove(struct platform_device
 *pdev) i2c_del_adapter(i2c-adap);
 
   clk_disable_unprepare(i2c-clk);
 -
 - if (pdev-dev.of_node  IS_ERR(i2c-pctrl))
 - s3c24xx_i2c_dt_gpio_free(i2c);
 -
   return 0;
  }

--
To

Re: [PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

2013-03-04 Thread Thomas Abraham 
On 4 March 2013 19:33, Heiko Stübner he...@sntech.de wrote:
 Hi Thomas,

 Am Montag, 4. März 2013, 14:42:53 schrieb Thomas Abraham:
 With device core now able to setup the default pin configuration,
 the call to devm_pinctrl_get_select_default can be removed. And
 the pin configuration code based on the deprecated Samsung specific
 gpio bindings is also removed.

 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---

 Hi Heiko, Tomasz,

 We have to make a choice on the path forward for pinctrl support
 on Samsung platforms. We have three cases to deal with.

 A. Samsung platforms without DT support.
 B. Samsung platforms with DT support using old Samsung specific
gpio bindings for pin-configuration (s3c24xx and s3x64xx).
 C. Samsung platforms with DT support using using pinctrl based
pin-configurations (Exynos4 and Exynos5).

 For [A], we just let the platform specific callbacks handle the
 pin setup. For [B] and [C], based on Linus Walleij's pin grab
 by device core patch and subsequent discussions with him on the
 mailing list, would it be acceptable that we discontinue support
 for [B] in Samsung SoC device drivers. This will impact your
 current DT work on s3c24xx and s3c64xx platforms. Pinctrl is
 inevitable and we have to migrate to it. Instead of workarounds
 to maintain support for [B], it might be better that we migrate
 s3c24xx and s3c64xx platforms to pinctrl.

 Please do let us know your opinion on this.

 As discusses in the other thread, I'm in favor of going forward this way and
 not to invest unnecessary energy in keeping the non-pinctrl stuff alive.

 Pinctrl for at least the s3c2416 [0] is already on my playground-wishlist, but
 I'm still in the process of getting to know it.

 So for this patch:
 Acked-by: Heiko Stuebner he...@sntech.de


 Heiko


 [0] in contrast to what the gpio-samsung driver implies, the s3c24xx pin banks
 are not uniform between SoCs, making this more difficult still :-)

Hi Heiko,

Thanks for the Ack. If the common samsung pinctrl support
(pinctrl-samsung.c) is not well suited for s3c24xx platforms, we could
write a separate pinctrl driver for s3c24xx platforms. But I guess we
might be able to support s3c24xx which some changes to existing code.
If there is anything that I can help with here, please do let me know.

Thanks,
Thomas.



  drivers/i2c/busses/i2c-s3c2410.c |   67
 +- 1 files changed, 1 insertions(+),
 66 deletions(-)

 diff --git a/drivers/i2c/busses/i2c-s3c2410.c
 b/drivers/i2c/busses/i2c-s3c2410.c index f6b880b..703272c 100644
 --- a/drivers/i2c/busses/i2c-s3c2410.c
 +++ b/drivers/i2c/busses/i2c-s3c2410.c
 @@ -37,8 +37,6 @@
  #include linux/slab.h
  #include linux/io.h
  #include linux/of_i2c.h
 -#include linux/of_gpio.h
 -#include linux/pinctrl/consumer.h

  #include asm/irq.h

 @@ -84,8 +82,6 @@ struct s3c24xx_i2c {
   struct i2c_adapter  adap;

   struct s3c2410_platform_i2c *pdata;
 - int gpios[2];
 - struct pinctrl  *pctrl;
  #ifdef CONFIG_CPU_FREQ
   struct notifier_block   freq_transition;
  #endif
 @@ -856,57 +852,6 @@ static inline void
 s3c24xx_i2c_deregister_cpufreq(struct s3c24xx_i2c *i2c) }
  #endif

 -#ifdef CONFIG_OF
 -static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 -{
 - int idx, gpio, ret;
 -
 - if (i2c-quirks  QUIRK_NO_GPIO)
 - return 0;
 -
 - for (idx = 0; idx  2; idx++) {
 - gpio = of_get_gpio(i2c-dev-of_node, idx);
 - if (!gpio_is_valid(gpio)) {
 - dev_err(i2c-dev, invalid gpio[%d]: %d\n, idx, gpio);
 - goto free_gpio;
 - }
 - i2c-gpios[idx] = gpio;
 -
 - ret = gpio_request(gpio, i2c-bus);
 - if (ret) {
 - dev_err(i2c-dev, gpio [%d] request failed\n, gpio);
 - goto free_gpio;
 - }
 - }
 - return 0;
 -
 -free_gpio:
 - while (--idx = 0)
 - gpio_free(i2c-gpios[idx]);
 - return -EINVAL;
 -}
 -
 -static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 -{
 - unsigned int idx;
 -
 - if (i2c-quirks  QUIRK_NO_GPIO)
 - return;
 -
 - for (idx = 0; idx  2; idx++)
 - gpio_free(i2c-gpios[idx]);
 -}
 -#else
 -static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
 -{
 - return 0;
 -}
 -
 -static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
 -{
 -}
 -#endif
 -
  /* s3c24xx_i2c_init
   *
   * initialise the controller, set the IO lines and frequency
 @@ -1054,15 +999,9 @@ static int s3c24xx_i2c_probe(struct platform_device
 *pdev) i2c-adap.algo_data = i2c;
   i2c-adap.dev.parent = pdev-dev;

 - i2c-pctrl = devm_pinctrl_get_select_default(i2c-dev);
 -
   /* inititalise the i2c gpio lines */
 -
 - if (i2c-pdata-cfg_gpio) {
 + if (i2c-pdata-cfg_gpio)
   i2c-pdata-cfg_gpio(to_platform_device(i2c-dev));
 - } else if