Re: [PATCH 1/6] usb: chipidea: Add dynamic pinctrl selection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23 August 2018 at 12:11, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Tue, Aug 21, 2018 at 4:57 PM Loic Poulain <loic.poulain@xxxxxxxxxx> wrote:
>>
>> Some hardware implementations require to configure pins differently
>> according to the USB role (host/device), this can be an update of the
>> pins routing or a simple GPIO value change.
>>
>> This patch introduces new optional "host" and "device" pinctrls.
>> If these pinctrls are defined by the device, they are respectively
>> selected on host/device role start.
>>
>> If a default pinctrl exist, it is restored on host/device role stop.
>
>>  #include <linux/extcon.h>
>>  #include <linux/phy/phy.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pinctrl/consumer.h>
>
> A nit: Even in this context it would be nice to keep *some* order.
>

Ok.

>>  #include <linux/module.h>
>>  #include <linux/idr.h>
>>  #include <linux/interrupt.h>
>
>> +               p = pinctrl_lookup_state(platdata->pctl, "default");
>> +               p = pinctrl_lookup_state(platdata->pctl, "host");
>> +               p = pinctrl_lookup_state(platdata->pctl, "device");
>
> I'm wondering if we have any rules applied to these names.

I suppose i have document this in the chipidea DT binding doc.

>
>>  #include <linux/usb/hcd.h>
>>  #include <linux/usb/chipidea.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/pinctrl/consumer.h>
>
> Ditto about ordering.
>
>> +       if (ci->platdata->pins_host)
>> +               pinctrl_select_state(ci->platdata->pctl,
>> +                                    ci->platdata->pins_host);
>
> Hmm... Do we need to have a condition above?
>

Yes, since these pinctrls are optional and can be null.

>> +       if (ci->platdata->pins_host && ci->platdata->pins_default)
>> +               pinctrl_select_state(ci->platdata->pctl,
>> +                                    ci->platdata->pins_default);
>
> Ditto about conditional.
>
>>  #include <linux/usb/gadget.h>
>>  #include <linux/usb/otg-fsm.h>
>>  #include <linux/usb/chipidea.h>
>> +#include <linux/pinctrl/consumer.h>
>
> Ditto about ordering.
>
>> +       if (ci->platdata->pins_device)
>> +               pinctrl_select_state(ci->platdata->pctl,
>> +                                    ci->platdata->pins_device);
>
>
>> +       if (ci->platdata->pins_device && ci->platdata->pins_default)
>> +               pinctrl_select_state(ci->platdata->pctl,
>> +                                    ci->platdata->pins_default);
>
> Ditto about conditional.
>
> --
> With Best Regards,
> Andy Shevchenko

Regards,
Loic



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux