Hi Marek,
I think your patch is right here. I was confused by the
devm_pinctrl_get_select_default() which returns a pointer to pinctrl
instant.
But there is no code use that pinctrl. From the documentation and some
examples in drivers/, it helps to set proper pin configuration for
GPIO usage which depends on the DT setting and pinctrl maps. So we
need setup such pinctrl maps in the right place, otherwise this API
call will return error.
Looks like we only use *pinctrl as a return value of
devm_pinctrl_get_select_default() instead of a real handler for
further pinctrl operation.
I'm OK with this patch now and will apply it to my for-next branch.
Add Stephen to this loop, since he introduce this API.
Thanks,
-Bryan
On Mon, Aug 27, 2012 at 6:05 PM, Marek Vasut <marek.vasut@xxxxxxxxx> wrote:
> Dear Bryan Wu,
>
> [...]
>
>> >> Maybe I don't understand how to use this 'pinctrl' in this patch,
>> >> since I didn't see any code to use that.
>> >> Could you please post code to configure the pins in gpio-leds driver?
>> >
>> > It's not code that uses this, it's a DT property. But then, if the DT
>> > property is not present, this fails ... so this patch is wrong. The
>> > pinctrl is used by simply specifying the "pinctrl-names" and "pinctrl-0"
>> > properties in the DT file.
>>
>> I know this. What I don't understand of this patch is "how to use
>> 'struct pinctrl *pinctrl;' which you get from DT tree"?
>> So if there is no such property in DT, it will return an error, I
>> don't think this is necessary.
>
> Indeed, so the error should be converted into warning, correct?
>
>> And if there is a property in DT, the *pinctrl will point to some
>> instant. but there is no code to use this instant to control any pins
>> in leds-gpio driver.
>
> The way I understand it is that the devm_pinctrl_get_select_default() call
> configures the pin multiplexing, therefore further operation with the device is
> possible. In this case, the pins are multiplexed as GPIO.
>
>> And why we need this to control pins in leds-gpio
>> driver, since we can use gpio generic API to do that?
>
> You mean the hog gpios? I guess that's indeed possible, but I expected putting
> this under the leds-gpio would be more correct or maybe more systematic. Am I
> wrong?
>
>> Thanks,
>> -Bryan
>>
>> > But I wonder how to go about this.
>> >
>> >> -Bryan
>> >>
>> >> >> > Signed-off-by: Marek Vasut <marex@xxxxxxx>
>> >> >> > Cc: Bryan Wu <bryan.wu@xxxxxxxxxxxxx>
>> >> >> > Cc: Richard Purdie <rpurdie@xxxxxxxxx>
>> >> >> > Cc: Grant Likely <grant.likely@xxxxxxxxxxxx>
>> >> >> > Cc: Rob Herring <rob.herring@xxxxxxxxxxx>
>> >> >> > Cc: linux-leds@xxxxxxxxxxxxxxx
>> >> >> > ---
>> >> >> >
>> >> >> > drivers/leds/leds-gpio.c | 6 ++++++
>> >> >> > 1 file changed, 6 insertions(+)
>> >> >> >
>> >> >> > diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
>> >> >> > index cde85ba..877d69e 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;
>> >> >> >
>> >> >> > @@ -234,8 +235,13 @@ 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))
>> >> >> > + return PTR_ERR(pinctrl);
>> >> >> > +
>> >> >> >
>> >> >> > if (pdata && pdata->num_leds) {
>> >> >> >
>> >> >> > priv = devm_kzalloc(&pdev->dev,
>> >> >> >
>> >> >> > sizeof_gpio_leds_priv(pdata->num_le
>> >> >> > ds) ,
>> >> >> >
>> >> >> > --
>> >> >> > 1.7.10.4
>> >> >
>> >> > Best regards,
>> >> > Marek Vasut
>> >
>> > Best regards,
>> > Marek Vasut
--
Bryan Wu <bryan.wu@xxxxxxxxxxxxx>
Kernel Developer +86.186-168-78255 Mobile
Canonical Ltd. www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
