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. > #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. > #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? > + 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