Re: [PATCH 1/4] phy: sun4i-usb: add support for V3s USB PHY

Re: [PATCH 1/4] phy: sun4i-usb: add support for V3s USB PHY

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Tue, Jan 03, 2017 at 11:25:31PM +0800, Icenowy Zheng wrote:
> Allwinner V3s come with a USB PHY controller slightly different to other
> SoCs, with only one PHY.
> 
> Add support for it.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/4] phy: sun4i-usb: add support for V3s USB PHY

[Kishon Vijay Abraham I]

On Friday 06 January 2017 07:26 PM, Maxime Ripard wrote:
> On Tue, Jan 03, 2017 at 11:25:31PM +0800, Icenowy Zheng wrote:
>> Allwinner V3s come with a USB PHY controller slightly different to other
>> SoCs, with only one PHY.
>>
>> Add support for it.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> 
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

merged, thanks!

-Kishon
> 
> Thanks,
> Maxime
> 

Re: [PATCH 2/4] musb: sunxi: add support for the variant in H3/V3s SoC

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

On Tue, Jan 03, 2017 at 11:25:32PM +0800, Icenowy Zheng wrote:
> Allwinner H3/V3s features a variant of MUSB controller, which lacks one
> endpoint.
> 
> Add support for it.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/4] musb: sunxi: add support for the variant in H3/V3s SoC

[Bin Liu]

On Fri, Jan 06, 2017 at 02:57:15PM +0100, Maxime Ripard wrote:
> On Tue, Jan 03, 2017 at 11:25:32PM +0800, Icenowy Zheng wrote:
> > Allwinner H3/V3s features a variant of MUSB controller, which lacks one
> > endpoint.
> > 
> > Add support for it.
> > 
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> 
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied. Thanks.
-Bin.

Re: [PATCH 3/4] ARM: dts: sunxi: add usb_otg and usbphy node for V3s SoC

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

On Tue, Jan 03, 2017 at 11:25:33PM +0800, Icenowy Zheng wrote:
> V3s SoC features a USB PHY controller and a MUSB OTG controller.
> 
> Add device nodes for them.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

This can be merged in your other DTSI patch.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Bin Liu]

On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> Lichee Pi Zero features a USB OTG port.
> 
> Add support for it.
> 
> Note: in order to use the Host mode, the board must be powered via the
> +5V and GND pins.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
>  arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> index 0099affc6ce3..3d9168cbaeca 100644
> --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> @@ -71,3 +71,13 @@
>  	pinctrl-names = "default";
>  	status = "okay";
>  };
> +
> +&usb_otg {
> +	dr_mode = "otg";

Why not set this default mode in dtsi instead?

Regards,
-Bin.

> +	status = "okay";
> +};
> +
> +&usbphy {
> +	usb0_id_det-gpio = <&pio 5 6 GPIO_ACTIVE_HIGH>;
> +	status = "okay";
> +};
> -- 
> 2.11.0
> 

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Bin Liu]

On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> 
> 
> 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> >>  Lichee Pi Zero features a USB OTG port.
> >>
> >>  Add support for it.
> >>
> >>  Note: in order to use the Host mode, the board must be powered via the
> >>  +5V and GND pins.
> >>
> >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >>  ---
> >>   arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> >>   1 file changed, 10 insertions(+)
> >>
> >>  diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> >>  index 0099affc6ce3..3d9168cbaeca 100644
> >>  --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> >>  +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> >>  @@ -71,3 +71,13 @@
> >>           pinctrl-names = "default";
> >>           status = "okay";
> >>   };
> >>  +
> >>  +&usb_otg {
> >>  + dr_mode = "otg";
> >
> > Why not set this default mode in dtsi instead?
> >
> > Regards,
> > -Bin.
> 
> There's possibly boards which do not have OTG functions.

That is board specific.

You'd better to define the default dr_mode which the musb _controller_
supports in the dtsi, and then override it in a specific board dts if
necessary.

Regards,
-Bin.

> 
> Even the official CDR design of V3s uses the USB controller to
> connect a UVC webcam to make the design a dual-cam design
> (V3s itself has a CSI).
> 
> >
> >>  + status = "okay";
> >>  +};
> >>  +
> >>  +&usbphy {
> >>  + usb0_id_det-gpio = <&pio 5 6 GPIO_ACTIVE_HIGH>;
> >>  + status = "okay";
> >>  +};
> >>  --
> >>  2.11.0

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Bin Liu]

On Thu, Jan 12, 2017 at 04:13:34AM +0800, Icenowy Zheng wrote:
> 
> 
> 12.01.2017, 04:08, "Bin Liu" <b-liu@ti.com>:
> > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> >>  11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> >>  > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> >>  >>  Lichee Pi Zero features a USB OTG port.
> >>  >>
> >>  >>  Add support for it.
> >>  >>
> >>  >>  Note: in order to use the Host mode, the board must be powered via the
> >>  >>  +5V and GND pins.
> >>  >>
> >>  >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >>  >>  ---
> >>  >>   arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> >>  >>   1 file changed, 10 insertions(+)
> >>  >>
> >>  >>  diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> >>  >>  index 0099affc6ce3..3d9168cbaeca 100644
> >>  >>  --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> >>  >>  +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> >>  >>  @@ -71,3 +71,13 @@
> >>  >>           pinctrl-names = "default";
> >>  >>           status = "okay";
> >>  >>   };
> >>  >>  +
> >>  >>  +&usb_otg {
> >>  >>  + dr_mode = "otg";
> >>  >
> >>  > Why not set this default mode in dtsi instead?
> >>  >
> >>  > Regards,
> >>  > -Bin.
> >>
> >>  There's possibly boards which do not have OTG functions.
> >
> > That is board specific.
> >
> > You'd better to define the default dr_mode which the musb _controller_
> > supports in the dtsi, and then override it in a specific board dts if
> > necessary.
> 
> Is there MUSB controllers which do not support a certain mode?

I am not aware of any. That is why I recommended to set "otg" in dtsi,
then override it in board dts if a port is specically designed to
host-only or device-only mode.

> 
> (I remembered my omap3-n900 which do not work under OTG mode...)

I belive it is n900 board specific. omap3 itself doesn't have such
limitation, AFAIK.

Regards,
-Bin.

> 
> >
> > Regards,
> > -Bin.
> >
> >>  Even the official CDR design of V3s uses the USB controller to
> >>  connect a UVC webcam to make the design a dual-cam design
> >>  (V3s itself has a CSI).
> >>
> >>  >
> >>  >>  + status = "okay";
> >>  >>  +};
> >>  >>  +
> >>  >>  +&usbphy {
> >>  >>  + usb0_id_det-gpio = <&pio 5 6 GPIO_ACTIVE_HIGH>;
> >>  >>  + status = "okay";
> >>  >>  +};
> >>  >>  --
> >>  >>  2.11.0

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > 
> > 
> > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > >>  Lichee Pi Zero features a USB OTG port.
> > >>
> > >>  Add support for it.
> > >>
> > >>  Note: in order to use the Host mode, the board must be powered via the
> > >>  +5V and GND pins.
> > >>
> > >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > >>  ---
> > >>   arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > >>   1 file changed, 10 insertions(+)
> > >>
> > >>  diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > >>  index 0099affc6ce3..3d9168cbaeca 100644
> > >>  --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > >>  +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > >>  @@ -71,3 +71,13 @@
> > >>           pinctrl-names = "default";
> > >>           status = "okay";
> > >>   };
> > >>  +
> > >>  +&usb_otg {
> > >>  + dr_mode = "otg";
> > >
> > > Why not set this default mode in dtsi instead?
> > >
> > > Regards,
> > > -Bin.
> > 
> > There's possibly boards which do not have OTG functions.
> 
> That is board specific.

Exactly, and this is why it should be done in the board DT.

The controller in the Allwinner SoCs do not handle directly the ID pin
and VBUS, but rather rely on a GPIO to do so.

So boards with OTG will need setup anyway, at least to tell which
GPIOs are used. There's no point in enforcing a default if it doesn't
work by default.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Bin Liu]

On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > > 
> > > 
> > > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > >>  Lichee Pi Zero features a USB OTG port.
> > > >>
> > > >>  Add support for it.
> > > >>
> > > >>  Note: in order to use the Host mode, the board must be powered via the
> > > >>  +5V and GND pins.
> > > >>
> > > >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > >>  ---
> > > >>   arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > > >>   1 file changed, 10 insertions(+)
> > > >>
> > > >>  diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > >>  index 0099affc6ce3..3d9168cbaeca 100644
> > > >>  --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > >>  +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > >>  @@ -71,3 +71,13 @@
> > > >>           pinctrl-names = "default";
> > > >>           status = "okay";
> > > >>   };
> > > >>  +
> > > >>  +&usb_otg {
> > > >>  + dr_mode = "otg";
> > > >
> > > > Why not set this default mode in dtsi instead?
> > > >
> > > > Regards,
> > > > -Bin.
> > > 
> > > There's possibly boards which do not have OTG functions.
> > 
> > That is board specific.
> 
> Exactly, and this is why it should be done in the board DT.

I am just suggesting based on the common practice. If a .dtsi exists for
a family, the .dtsi describes the device and common properties for all
possible boards, and each board .dts adds or overrides its specific
implementation. Kernel has many devices/boards done in this way - define
the default dr_mode in .dtsi.

In this case, I suggest to set the common dr_mode in .dtsi, then each
board .dts only overrides it if the implementation is different. 

> 
> The controller in the Allwinner SoCs do not handle directly the ID pin
> and VBUS, but rather rely on a GPIO to do so.
> 
> So boards with OTG will need setup anyway, at least to tell which
> GPIOs are used. There's no point in enforcing a default if it doesn't
> work by default.

Then define a default which supposes to work for most boards.

Why I suggest this, is because defining a default dr_mode which works
for most cases in dtsi could prevent a little surprise in MUSB function.
If someone designs a new board but forgets to define dr_mode in the new
board DT, the MUSB driver will default to org mode, which might not be
intended.

Regards,
-Bin.

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3329 bytes --]

Hi Bin,

On Thu, Jan 12, 2017 at 08:50:14AM -0600, Bin Liu wrote:
> On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> > On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > > > 
> > > > 
> > > > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > > >>  Lichee Pi Zero features a USB OTG port.
> > > > >>
> > > > >>  Add support for it.
> > > > >>
> > > > >>  Note: in order to use the Host mode, the board must be powered via the
> > > > >>  +5V and GND pins.
> > > > >>
> > > > >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > > >>  ---
> > > > >>   arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > > > >>   1 file changed, 10 insertions(+)
> > > > >>
> > > > >>  diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > >>  index 0099affc6ce3..3d9168cbaeca 100644
> > > > >>  --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > >>  +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > >>  @@ -71,3 +71,13 @@
> > > > >>           pinctrl-names = "default";
> > > > >>           status = "okay";
> > > > >>   };
> > > > >>  +
> > > > >>  +&usb_otg {
> > > > >>  + dr_mode = "otg";
> > > > >
> > > > > Why not set this default mode in dtsi instead?
> > > > >
> > > > > Regards,
> > > > > -Bin.
> > > > 
> > > > There's possibly boards which do not have OTG functions.
> > > 
> > > That is board specific.
> > 
> > Exactly, and this is why it should be done in the board DT.
> 
> I am just suggesting based on the common practice. If a .dtsi exists for
> a family, the .dtsi describes the device and common properties for all
> possible boards, and each board .dts adds or overrides its specific
> implementation. Kernel has many devices/boards done in this way - define
> the default dr_mode in .dtsi.
> 
> In this case, I suggest to set the common dr_mode in .dtsi, then each
> board .dts only overrides it if the implementation is different. 
> 
> > 
> > The controller in the Allwinner SoCs do not handle directly the ID pin
> > and VBUS, but rather rely on a GPIO to do so.
> > 
> > So boards with OTG will need setup anyway, at least to tell which
> > GPIOs are used. There's no point in enforcing a default if it doesn't
> > work by default.
> 
> Then define a default which supposes to work for most boards.
> 
> Why I suggest this, is because defining a default dr_mode which works
> for most cases in dtsi could prevent a little surprise in MUSB function.
> If someone designs a new board but forgets to define dr_mode in the new
> board DT, the MUSB driver will default to org mode, which might not be
> intended.

The point is that there is no sensible default. Some boards don't have
an ID pin and no VBUS (peripheral), some don't have an ID pin but VBUS
(host), and some have an ID pin but no controllable VBUS, some have an
ID pin and a controllable VBUS, but we have no idea which GPIOs are
used.

There's no way we can have something that works on most cases.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero

[Bin Liu]

On Thu, Jan 12, 2017 at 06:39:38PM +0100, Maxime Ripard wrote:
> Hi Bin,
> 
> On Thu, Jan 12, 2017 at 08:50:14AM -0600, Bin Liu wrote:
> > On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> > > On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > > > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > > > > 
> > > > > 
> > > > > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > > > >>  Lichee Pi Zero features a USB OTG port.
> > > > > >>
> > > > > >>  Add support for it.
> > > > > >>
> > > > > >>  Note: in order to use the Host mode, the board must be powered via the
> > > > > >>  +5V and GND pins.
> > > > > >>
> > > > > >>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > > > >>  ---
> > > > > >>   arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > > > > >>   1 file changed, 10 insertions(+)
> > > > > >>
> > > > > >>  diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >>  index 0099affc6ce3..3d9168cbaeca 100644
> > > > > >>  --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >>  +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >>  @@ -71,3 +71,13 @@
> > > > > >>           pinctrl-names = "default";
> > > > > >>           status = "okay";
> > > > > >>   };
> > > > > >>  +
> > > > > >>  +&usb_otg {
> > > > > >>  + dr_mode = "otg";
> > > > > >
> > > > > > Why not set this default mode in dtsi instead?
> > > > > >
> > > > > > Regards,
> > > > > > -Bin.
> > > > > 
> > > > > There's possibly boards which do not have OTG functions.
> > > > 
> > > > That is board specific.
> > > 
> > > Exactly, and this is why it should be done in the board DT.
> > 
> > I am just suggesting based on the common practice. If a .dtsi exists for
> > a family, the .dtsi describes the device and common properties for all
> > possible boards, and each board .dts adds or overrides its specific
> > implementation. Kernel has many devices/boards done in this way - define
> > the default dr_mode in .dtsi.
> > 
> > In this case, I suggest to set the common dr_mode in .dtsi, then each
> > board .dts only overrides it if the implementation is different. 
> > 
> > > 
> > > The controller in the Allwinner SoCs do not handle directly the ID pin
> > > and VBUS, but rather rely on a GPIO to do so.
> > > 
> > > So boards with OTG will need setup anyway, at least to tell which
> > > GPIOs are used. There's no point in enforcing a default if it doesn't
> > > work by default.
> > 
> > Then define a default which supposes to work for most boards.
> > 
> > Why I suggest this, is because defining a default dr_mode which works
> > for most cases in dtsi could prevent a little surprise in MUSB function.
> > If someone designs a new board but forgets to define dr_mode in the new
> > board DT, the MUSB driver will default to org mode, which might not be
> > intended.
> 
> The point is that there is no sensible default. Some boards don't have
> an ID pin and no VBUS (peripheral), some don't have an ID pin but VBUS
> (host), and some have an ID pin but no controllable VBUS, some have an
> ID pin and a controllable VBUS, but we have no idea which GPIOs are
> used.
> 
> There's no way we can have something that works on most cases.

Ok, understood.

Regards,
-Bin.