Re: [PATCH 2/5] mmc: pwrseq_simple: Extend to support more pins

2015-01-28 Thread Srinivas Kandagatla

Hi Javier,

You are in a lead of 3 hrs from me..
Surprisingly I send very much same patch just few Mins ago :-)
May be we can merge goods in both :-)

On 28/01/15 10:10, Javier Martinez Canillas wrote:

Many WLAN attached to a SDIO/MMC interface, needs more than one pin for
their reset sequence. For example, is very common for chips to have two
pins: one for reset and one for power enable.

This patch adds support for more reset pins to the pwrseq_simple driver
and instead hardcoding a fixed number, it uses the of_gpio_named_count()
since the MMC power sequence is only built when CONFIG_OF is enabled.

Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
---
  drivers/mmc/core/pwrseq_simple.c | 54 ++--
  1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 0958c696137f..9e51fe1051c5 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -11,6 +11,7 @@
  #include linux/slab.h
  #include linux/device.h
  #include linux/err.h
+#include linux/of_gpio.h
  #include linux/gpio/consumer.h

  #include linux/mmc/host.h
@@ -19,34 +20,44 @@

  struct mmc_pwrseq_simple {
struct mmc_pwrseq pwrseq;
-   struct gpio_desc *reset_gpio;
+   struct gpio_desc **reset_gpio;


May be renaming it to reset_gpios makes more sense..

If you make this struct gpio_desc *reset_gpios[0]; You can aviod an 
extra kmalloc and free ..




+   int nr_gpios;
  };

  static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
  {


[...

struct mmc_pwrseq_simple *pwrseq = container_of(host-pwrseq,
struct mmc_pwrseq_simple, pwrseq);
+   int i;

-   if (!IS_ERR(pwrseq-reset_gpio))
-   gpiod_set_value_cansleep(pwrseq-reset_gpio, 1);
+   for (i = 0; i  pwrseq-nr_gpios; i++)
+   if (!IS_ERR(pwrseq-reset_gpio[i]))
+   gpiod_set_value_cansleep(pwrseq-reset_gpio[i], 1);


...]


  }

  static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
  {


[...


struct mmc_pwrseq_simple *pwrseq = container_of(host-pwrseq,
struct mmc_pwrseq_simple, pwrseq);
+   int i;

-   if (!IS_ERR(pwrseq-reset_gpio))
-   gpiod_set_value_cansleep(pwrseq-reset_gpio, 0);
+   for (i = 0; i  pwrseq-nr_gpios; i++)
+   if (!IS_ERR(pwrseq-reset_gpio[i]))
+   gpiod_set_value_cansleep(pwrseq-reset_gpio[i], 0);

...]

Now that we have more code in mmc_pwrseq_simple_post_power_on() and 
mmc_pwrseq_simple_pre_power_on(), Just move most of them into a common 
function like:


static void __mmc_pwrseq_simple_power_on_off(struct mmc_host *host,
  bool on)
{
struct mmc_pwrseq_simple *pwrseq = container_of(host-pwrseq,
struct mmc_pwrseq_simple, pwrseq);
int i;

if (!IS_ERR(pwrseq-reset_gpios)) {
for (i = 0; i  pwrseq-ngpios; i++)
gpiod_set_value_cansleep(pwrseq-reset_gpios[i],
 on ? : 0);
}
}

static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
{
__mmc_pwrseq_simple_power_on_off(host, true);
}

static void mmc_pwrseq_simple_post_power_on(struct mmc_host *host)
{
__mmc_pwrseq_simple_power_on_off(host, false);
}



  }

  static void mmc_pwrseq_simple_free(struct mmc_host *host)
  {
struct mmc_pwrseq_simple *pwrseq = container_of(host-pwrseq,
struct mmc_pwrseq_simple, pwrseq);
+   int i;

-   if (!IS_ERR(pwrseq-reset_gpio))
-   gpiod_put(pwrseq-reset_gpio);
+   if (pwrseq-nr_gpios  0) {
+   for (i = 0; i  pwrseq-nr_gpios; i++)
+   if (!IS_ERR(pwrseq-reset_gpio[i]))
+   gpiod_put(pwrseq-reset_gpio[i]);
+   kfree(pwrseq-reset_gpio);
+   }

kfree(pwrseq);
host-pwrseq = NULL;
@@ -63,17 +74,27 @@ int mmc_pwrseq_simple_alloc(struct mmc_host *host, struct 
device *dev)
  {
struct mmc_pwrseq_simple *pwrseq;
int ret = 0;
+   int i;

pwrseq = kzalloc(sizeof(struct mmc_pwrseq_simple), GFP_KERNEL);
if (!pwrseq)
return -ENOMEM;

-   pwrseq-reset_gpio = gpiod_get_index(dev, reset, 0, GPIOD_OUT_HIGH);
-   if (IS_ERR(pwrseq-reset_gpio) 
-   PTR_ERR(pwrseq-reset_gpio) != -ENOENT 
-   PTR_ERR(pwrseq-reset_gpio) != -ENOSYS) {
-   ret = PTR_ERR(pwrseq-reset_gpio);
-   goto free;
+   pwrseq-nr_gpios = of_gpio_named_count(dev-of_node, reset-gpios);
+   if (pwrseq-nr_gpios  0) {


What happens if there are no gpios? This fuction should return -ENOENT 
and should not even try to allocate 

Re: [PATCH V2] drm/exynos: Add match table for drm platform device

2012-09-21 Thread Srinivas KANDAGATLA
On 21/09/12 19:37, Leela Krishna Amudala wrote:
 This patch is a part of moving the driver to support DT style probing
 of exynos drm device. The compatible name should match with the
 entry in the dtsi file.

 Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_drm_drv.c |   11 +++
  1 files changed, 11 insertions(+), 0 deletions(-)

 diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
 b/drivers/gpu/drm/exynos/exynos_drm_drv.c
 index d070719..495be89 100644
 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
 +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
 @@ -294,12 +294,23 @@ static int exynos_drm_platform_remove(struct 
 platform_device *pdev)
   return 0;
  }
  
 +#ifdef CONFIG_OF
 +static const struct of_device_id drm_device_dt_match[] = {
 + { .compatible = samsung,exynos-drm-device},
 + {},
 +};
 +MODULE_DEVICE_TABLE(of, drm_device_dt_match);
 +#else
 +#define drm_device_dt_match NULL
 +#endif

No need of else here as you are using of_match_ptr.

 +
  static struct platform_driver exynos_drm_platform_driver = {
   .probe  = exynos_drm_platform_probe,
   .remove = __devexit_p(exynos_drm_platform_remove),
   .driver = {
   .owner  = THIS_MODULE,
   .name   = exynos-drm,
 + .of_match_table = of_match_ptr(drm_device_dt_match),
   },
  };
  

--
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