Re: [PATCH v2] leds: leds-gpio: adopt pinctrl support

Dear Bryan Wu,

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to properly 
investigate this.

But without proper DT setting, it will also give us warning I think.
or we can provide some dummy functions as a temp solution as Shawn
pointed out before.

But this driver is also used on hardware that's not yet coverted to DT, so I'd 
say dev_warn() if CONFIG_OF is enabled and otherwise simply go on ? Actually, 
can we not skip whole this pinctrl thing if CONFIG_OF is disabled? Actually (2), 
what's the relationship between OF and pinctrl?

-Bryan

quoted

Signed-off-by: AnilKumar Ch <redacted>
---

Changes from v1:
        - Seperated from "Add DT for AM33XX devices" patch series
        - Incorporated Tony's comments on v1
        
          * Changed to warning message instead od error return
 
 drivers/leds/leds-gpio.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index c032b21..ad577f4 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c

@@ -20,6 +20,7 @@

 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/module.h>

+#include <linux/pinctrl/consumer.h>

 struct gpio_led_data {
 
        struct led_classdev cdev;

@@ -236,8 +237,14 @@ static int __devinit gpio_led_probe(struct

platform_device *pdev)

 {
 
        struct gpio_led_platform_data *pdata = pdev->dev.platform_data;
        struct gpio_leds_priv *priv;

+       struct pinctrl *pinctrl;

        int i, ret = 0;

+       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+       if (IS_ERR(pinctrl))
+               dev_warn(&pdev->dev,
+                       "pins are not configured from the driver\n");
+

        if (pdata && pdata->num_leds) {
        
                priv = devm_kzalloc(&pdev->dev,
                
                                sizeof_gpio_leds_priv(pdata->num_leds),

--
1.7.9.5

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

Best regards,
Marek Vasut

* Marek Vasut [off-list ref] [120904 20:13]:

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to properly 
investigate this.

quoted

But without proper DT setting, it will also give us warning I think.
or we can provide some dummy functions as a temp solution as Shawn
pointed out before.

But this driver is also used on hardware that's not yet coverted to DT, so I'd 
say dev_warn() if CONFIG_OF is enabled and otherwise simply go on ? Actually, 
can we not skip whole this pinctrl thing if CONFIG_OF is disabled? Actually (2), 
what's the relationship between OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Regards,

Tony

Hi Tony,

* Marek Vasut [off-list ref] [120904 20:13]:

quoted

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to properly
investigate this.

quoted

But without proper DT setting, it will also give us warning I think.
or we can provide some dummy functions as a temp solution as Shawn
pointed out before.

But this driver is also used on hardware that's not yet coverted to DT,
so I'd say dev_warn() if CONFIG_OF is enabled and otherwise simply go on
? Actually, can we not skip whole this pinctrl thing if CONFIG_OF is
disabled? Actually (2), what's the relationship between OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems where 
pinctrl is disabled.

Regards,

Tony

Best regards,
Marek Vasut

* Marek Vasut [off-list ref] [120905 19:05]:

Hi Tony,

quoted

* Marek Vasut [off-list ref] [120904 20:13]:

quoted

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to properly
investigate this.

quoted

But without proper DT setting, it will also give us warning I think.
or we can provide some dummy functions as a temp solution as Shawn
pointed out before.

But this driver is also used on hardware that's not yet coverted to DT,
so I'd say dev_warn() if CONFIG_OF is enabled and otherwise simply go on
? Actually, can we not skip whole this pinctrl thing if CONFIG_OF is
disabled? Actually (2), what's the relationship between OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems where 
pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Or do you get some warning if CONFIG_PINCTRL is not selected for your
hardware?

Tony

Dear Tony Lindgren,

* Marek Vasut [off-list ref] [120905 19:05]:

quoted

Hi Tony,

quoted

* Marek Vasut [off-list ref] [120904 20:13]:

quoted

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to
properly investigate this.

quoted

But without proper DT setting, it will also give us warning I
think. or we can provide some dummy functions as a temp solution
as Shawn pointed out before.

But this driver is also used on hardware that's not yet coverted to
DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise
simply go on ? Actually, can we not skip whole this pinctrl thing if
CONFIG_OF is disabled? Actually (2), what's the relationship between
OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems where
pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Oh all right then.

Or do you get some warning if CONFIG_PINCTRL is not selected for your
hardware?

No, I don't have much hardware like such anymore :-(

Best regards,
Marek Vasut

On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote:

Dear Tony Lindgren,

quoted

* Marek Vasut [off-list ref] [120905 19:05]:

quoted

Hi Tony,

quoted

* Marek Vasut [off-list ref] [120904 20:13]:

quoted

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to
properly investigate this.

quoted

But without proper DT setting, it will also give us warning I
think. or we can provide some dummy functions as a temp solution
as Shawn pointed out before.

But this driver is also used on hardware that's not yet coverted to
DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise
simply go on ? Actually, can we not skip whole this pinctrl thing if
CONFIG_OF is disabled? Actually (2), what's the relationship between
OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems where
pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Oh all right then.

Bryan,

If this patch looks fine, can you queue this for 3.7?

Thanks
AnilKumar

Dear AnilKumar, Chimata,

On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote:

quoted

Dear Tony Lindgren,

quoted

* Marek Vasut [off-list ref] [120905 19:05]:

quoted

Hi Tony,

quoted

* Marek Vasut [off-list ref] [120904 20:13]:

quoted

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] 

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to
properly investigate this.

quoted

But without proper DT setting, it will also give us warning I
think. or we can provide some dummy functions as a temp
solution as Shawn pointed out before.

But this driver is also used on hardware that's not yet coverted
to DT, so I'd say dev_warn() if CONFIG_OF is enabled and
otherwise simply go on ? Actually, can we not skip whole this
pinctrl thing if CONFIG_OF is disabled? Actually (2), what's the
relationship between OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may
not be device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems
where pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h
if CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Oh all right then.

Bryan,

If this patch looks fine, can you queue this for 3.7?

Looks good to me.

Thanks
AnilKumar

Best regards,
Marek Vasut

On Fri, Sep 7, 2012 at 3:59 PM, AnilKumar, Chimata [off-list ref] wrote:

On Fri, Sep 07, 2012 at 05:39:35, Marek Vasut wrote:

quoted

Dear Tony Lindgren,

quoted

* Marek Vasut [off-list ref] [120905 19:05]:

quoted

Hi Tony,

quoted

* Marek Vasut [off-list ref] [120904 20:13]:

quoted

Dear Bryan Wu,

quoted

On Sat, Sep 1, 2012 at 4:16 PM, AnilKumar Ch [off-list ref] wrote:

quoted

Adopt pinctrl support to leds-gpio driver based on leds-gpio
device pointer, pinctrl driver configure SoC pins to GPIO
mode according to definitions provided in .dts file.

Thanks for this, actually Marek Vasut submitted a similar patch
before. I'm pretty fine with this patch.

Thanks for submitting this actually ... I didn't have time to
properly investigate this.

quoted

But without proper DT setting, it will also give us warning I
think. or we can provide some dummy functions as a temp solution
as Shawn pointed out before.

But this driver is also used on hardware that's not yet coverted to
DT, so I'd say dev_warn() if CONFIG_OF is enabled and otherwise
simply go on ? Actually, can we not skip whole this pinctrl thing if
CONFIG_OF is disabled? Actually (2), what's the relationship between
OF and pinctrl?

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems where
pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

Oh all right then.

Bryan,

If this patch looks fine, can you queue this for 3.7?

I've applied this to my for-next branch.

Thanks,
-Bryan

On Thu, Sep 6, 2012 at 7:45 PM, Tony Lindgren [off-list ref] wrote:

quoted

quoted

The warning should be pinctrl related as the pinctrl drivers may not be
device tree based drivers.

Exactly my concern. Also the warning shouldnt be present on systems where
pinctrl is disabled.

But pinctrl_get_select() returns 0 in include/linux/pinctrl/consumer.h if
CONFIG_PINCTRL is not selected, so no warning is produced AFAIK ;)

This is correct, nothing to worry about.

The one troublesome case is if a pinctrl driver is present but not
being used, then you might need to call pinctrl_provide_dummies().

Yours,
Linus Walleij