Re: [PATCH] extcon: add MAX3355 driver

[Sergei Shtylyov]

Hello.

On 11/10/2015 2:52 AM, Chanwoo Choi wrote:

> I received the reply from you after too long time (17~18day).
> You better to reply it more a little more quickly
> if you have the question or new patches.

   I've replied as soon as I had my new idea.

> > > > > > > MAX3355E chip integrates a charge pump and comparators to enable a system with
> > > > > > > an integrated USB OTG dual-role transceiver to function as a USB OTG dual-role
> > > > > > > device. In addition to sensing/controlling Vbus, the chip also passes thru the
> > > > > > > ID signal from the USB OTG connector.  On some Renesas boards, this signal  is
> > > > > > > just  fed into the SoC thru a GPIO pin -- there's no real OTG controller, only
> > > > > > > host and gadget USB controllers sharing the same USB bus; however,  we'd  like
> > > > > > > to allow host or gadget drivers to be loaded depending on the cable type,
> > > > > > > hence
> > > > > > > the need for the MAX3355 extcon driver. The Vbus status signals are also wired
> > > > > > > to GPIOs (however, we're not currently intested in them), the  OFFVBUS# signal
> > > > > > > is controlled  by the host controllers, there's also the SHDN# signal wired to
> > > > > > > GPIO, which should be high for normal operation.
> > > > >
> > > > > > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
> > > > >
> > > > > > > ---
> > > > > > > The patch is against the 'extcon-next' branch of the 'extcon.git' repo.
> > > > >
> > > > > > >     Documentation/devicetree/bindings/extcon/extcon-max3355.txt |   21 ++
> > > > > > >     drivers/extcon/Kconfig                                      |    6
> > > > > > >     drivers/extcon/Makefile                                     |    1
> > > > > > >     drivers/extcon/extcon-max3355.c                             |  122
> > > > > > > ++++++++++++
> > > > > > >     4 files changed, 150 insertions(+)
> > > > >
> > > > > > > Index: extcon/Documentation/devicetree/bindings/extcon/extcon-max3355.txt
> > > > > > > ===================================================================
> > > > > > > --- /dev/null
> > > > > > > +++ extcon/Documentation/devicetree/bindings/extcon/extcon-max3355.txt
> > > > > > > @@ -0,0 +1,21 @@
> > > > > > > +MAX3355 USB OTG chip
> > > > >
> > > > > > Need manufactor information as following :
> > > > > >      -> Maxim MAX3355
> > > > >
> > > > >       Would be Maxim enough? Or perhaps I should use Maxim Integrated [Products]?
> > > >
> > > >      You haven't replied to my questions.
> > > >
> > > > > > > +--------------------
> > > > > > > +
> > > > > > > +MAX3355 integrates a charge pump and comparators to enable a system with an
> > > > > > > +integrated USB OTG dual-role transceiver to function as a USB OTG dual-role
> > > > > > > +device.
> > > > > > > +
> > > > > > > +Required properties:
> > > > > > > +- compatible: should be "maxim,max3355";
> > > > > > > +- maxim,shdn-gpio: should contain a phandle and GPIO specifier for the
> > > > > > > GPIO pin
> > > > > > > +  connected to the MAX3355's SHDN# pin;
> > > > > > > +- maxim,id-gpio: should contain a phandle and GPIO specifier for the GPIO pin
> > > > > > > +  connected to the MAX3355's ID_OUT pin.
> > > > > > > +
> > > > > > > +Example (Koelsch board):
> > > > > > > +
> > > > > > > +    usb-otg {
> > > > > > > +        compatible = "maxim,max3355";
> > > > > > > +        maxim,shdn-gpio = <&gpio2 4 GPIO_ACTIVE_LOW>;
> > > > > > > +        maxim,id-gpio = <&gpio5 31 GPIO_ACTIVE_HIGH>;
> > > > >
> > > > > > Kernel already supported the gpio helper function to get gpio from devicetree.
> > > > > > I prefer use follwoing style by using of_get_gpio()/of_get_gpio_flags() in
> > > > > > include/linux/of_gpio.h.
> > > > >
> > > > > >           gpios = <&gpio2 4 GPIO_ACTIVE_LOW>, <&gpio5 31 GPIO_ACTIVE_HIGH>;
> > > > >
> > > > >       OK, though Documentation/devicetree/bindings/gpio/gpio.txt does not seem
> > > > > to insist on using this one...
> > > >
> > > >      Moreover, it now says "gpios" isn't allowed for the new bindings. So I have to strongly disagree here...
> > >
> > > OK. But, I recommend you use the 'gpiod' with devm_gpiod_get
> > > instead of using devm_gpio_request_one() directly as following:
> > > You can refer drivers/extcon/extcon-usb-gpio.c about using gpiod.
> > >
> > > For example,
> > >      data->shdn_gpiod = devm_gpiod_get(dev, "maxim,shdn", GPIOD_IN);
> > >      data->id_gpiod = devm_gpiod_get(dev, "maxim,id", GPIOD_IN);
> >
> >     Thanks, done now. I just had another idea: how about I add an optional "enable-gpio" property to the 'extcon-usb-gpio' driver? I wouldn't need my own driver then at all... :-)
>
> What is meaning of 'enable-gpio' property?
> You better to explain your goal about 'enable-gpio' property

   This optional property would serve for enabling the valid signal on the ID 
GPIO, the same way I'm using the SHDN# GPIO in the MAX3355 driver.

> Also, If you think that it is generic way about
> adding 'enable-gpio' property to extcon-usb-gpio.c,
> you can try it.

   Yes, I think it would be generic enough.

> Thanks,
> Chanwoo Choi

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/