Re: [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller

[Sergio Prado]

On Tue, Dec 13, 2016 at 12:59:15PM -0600, Rob Herring wrote:
> > +Samsung S3C2410 and compatible USB device controller
> > +
> > +Required properties:
> > + - compatible: Should be one of the following
> > +  "samsung,s3c2410-udc"
> > +  "samsung,s3c2440-udc"
> > + - reg: address and length of the controller memory mapped region
> > + - interrupts: interrupt number for the USB device controller
> > + - clocks: Should reference the bus and host clocks
> > + - clock-names: Should contain two strings
> > +   "usb-bus-gadget" for the USB bus clock
> 
> Pretty sure the h/w clock name in the datasheet does not use the Linux 
> term gadget.

You are right. The datasheet calls it UCLK. In the S3c24010 clock driver
(clk-s3c2410.c), there's is a clock alias to UCLK called
"usb-bus-gadget" that was used in the USB device controller's driver.
We can change the driver and the DT binding to use "uclk" to
better reflect the name used in the datasheet. What do you think?

> 
> > +   "usb-device" for the USB device clock
> > +
> > +Optional properties:
> > + - samsung,vbus-gpio: If present, specifies a gpio that needs to be
> > +   activated for the bus to be powered.
> 
> Isn't it the host side that controls Vbus?

Yes. I'll change the description to "specifies a gpio that allows to
detect whether vbus is present (USB is connected)."

> 
> > + - samsung,pullup-gpio: If present, specifies a gpio to control the
> 
> Both GPIOs need to specify the active state.

OK.

> 
> > +   USB D+ pullup.
> > +
> > +usb1: udc@5200 {
> > +   compatible = "samsung,s3c2440-udc";
> > +   reg = <0x5200 0x10>;
> > +   interrupts = <0 0 25 3>;
> > +   clocks = < UCLK>, < HCLK_USBD>;
> > +   clock-names = "usb-bus-gadget", "usb-device";
> > +   samsung,pullup-gpio = < 5 GPIO_ACTIVE_HIGH>;
> > +};
> > -- 
> > 1.9.1
> > 

Best regards,

-- 
Sergio Prado
Embedded Labworks

Re: [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller

[Sergio Prado]

On Tue, Dec 13, 2016 at 12:59:15PM -0600, Rob Herring wrote:
> > +Samsung S3C2410 and compatible USB device controller
> > +
> > +Required properties:
> > + - compatible: Should be one of the following
> > +  "samsung,s3c2410-udc"
> > +  "samsung,s3c2440-udc"
> > + - reg: address and length of the controller memory mapped region
> > + - interrupts: interrupt number for the USB device controller
> > + - clocks: Should reference the bus and host clocks
> > + - clock-names: Should contain two strings
> > +   "usb-bus-gadget" for the USB bus clock
> 
> Pretty sure the h/w clock name in the datasheet does not use the Linux 
> term gadget.

You are right. The datasheet calls it UCLK. In the S3c24010 clock driver
(clk-s3c2410.c), there's is a clock alias to UCLK called
"usb-bus-gadget" that was used in the USB device controller's driver.
We can change the driver and the DT binding to use "uclk" to
better reflect the name used in the datasheet. What do you think?

> 
> > +   "usb-device" for the USB device clock
> > +
> > +Optional properties:
> > + - samsung,vbus-gpio: If present, specifies a gpio that needs to be
> > +   activated for the bus to be powered.
> 
> Isn't it the host side that controls Vbus?

Yes. I'll change the description to "specifies a gpio that allows to
detect whether vbus is present (USB is connected)."

> 
> > + - samsung,pullup-gpio: If present, specifies a gpio to control the
> 
> Both GPIOs need to specify the active state.

OK.

> 
> > +   USB D+ pullup.
> > +
> > +usb1: udc@5200 {
> > +   compatible = "samsung,s3c2440-udc";
> > +   reg = <0x5200 0x10>;
> > +   interrupts = <0 0 25 3>;
> > +   clocks = < UCLK>, < HCLK_USBD>;
> > +   clock-names = "usb-bus-gadget", "usb-device";
> > +   samsung,pullup-gpio = < 5 GPIO_ACTIVE_HIGH>;
> > +};
> > -- 
> > 1.9.1
> > 

Best regards,

-- 
Sergio Prado
Embedded Labworks

Re: [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller

[Rob Herring]

On Fri, Dec 09, 2016 at 05:06:39PM -0200, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C2410 and
> compatible USB device controller.
> 
> Signed-off-by: Sergio Prado 
> ---
>  .../devicetree/bindings/usb/s3c2410-usb.txt| 28 
> ++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt 
> b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> index e45b38ce2986..4d3f9894c2d4 100644
> --- a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> @@ -20,3 +20,31 @@ usb0: ohci@4900 {
>   clocks = < UCLK>, < HCLK_USBH>;
>   clock-names = "usb-bus-host", "usb-host";
>  };
> +
> +Samsung S3C2410 and compatible USB device controller
> +
> +Required properties:
> + - compatible: Should be one of the following
> +"samsung,s3c2410-udc"
> +"samsung,s3c2440-udc"
> + - reg: address and length of the controller memory mapped region
> + - interrupts: interrupt number for the USB device controller
> + - clocks: Should reference the bus and host clocks
> + - clock-names: Should contain two strings
> + "usb-bus-gadget" for the USB bus clock

Pretty sure the h/w clock name in the datasheet does not use the Linux 
term gadget.

> + "usb-device" for the USB device clock
> +
> +Optional properties:
> + - samsung,vbus-gpio: If present, specifies a gpio that needs to be
> +   activated for the bus to be powered.

Isn't it the host side that controls Vbus?

> + - samsung,pullup-gpio: If present, specifies a gpio to control the

Both GPIOs need to specify the active state.

> +   USB D+ pullup.
> +
> +usb1: udc@5200 {
> + compatible = "samsung,s3c2440-udc";
> + reg = <0x5200 0x10>;
> + interrupts = <0 0 25 3>;
> + clocks = < UCLK>, < HCLK_USBD>;
> + clock-names = "usb-bus-gadget", "usb-device";
> + samsung,pullup-gpio = < 5 GPIO_ACTIVE_HIGH>;
> +};
> -- 
> 1.9.1
> 

Re: [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller

[Rob Herring]

On Fri, Dec 09, 2016 at 05:06:39PM -0200, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C2410 and
> compatible USB device controller.
> 
> Signed-off-by: Sergio Prado 
> ---
>  .../devicetree/bindings/usb/s3c2410-usb.txt| 28 
> ++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt 
> b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> index e45b38ce2986..4d3f9894c2d4 100644
> --- a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> @@ -20,3 +20,31 @@ usb0: ohci@4900 {
>   clocks = < UCLK>, < HCLK_USBH>;
>   clock-names = "usb-bus-host", "usb-host";
>  };
> +
> +Samsung S3C2410 and compatible USB device controller
> +
> +Required properties:
> + - compatible: Should be one of the following
> +"samsung,s3c2410-udc"
> +"samsung,s3c2440-udc"
> + - reg: address and length of the controller memory mapped region
> + - interrupts: interrupt number for the USB device controller
> + - clocks: Should reference the bus and host clocks
> + - clock-names: Should contain two strings
> + "usb-bus-gadget" for the USB bus clock

Pretty sure the h/w clock name in the datasheet does not use the Linux 
term gadget.

> + "usb-device" for the USB device clock
> +
> +Optional properties:
> + - samsung,vbus-gpio: If present, specifies a gpio that needs to be
> +   activated for the bus to be powered.

Isn't it the host side that controls Vbus?

> + - samsung,pullup-gpio: If present, specifies a gpio to control the

Both GPIOs need to specify the active state.

> +   USB D+ pullup.
> +
> +usb1: udc@5200 {
> + compatible = "samsung,s3c2440-udc";
> + reg = <0x5200 0x10>;
> + interrupts = <0 0 25 3>;
> + clocks = < UCLK>, < HCLK_USBD>;
> + clock-names = "usb-bus-gadget", "usb-device";
> + samsung,pullup-gpio = < 5 GPIO_ACTIVE_HIGH>;
> +};
> -- 
> 1.9.1
>