Re: [PATCH v2 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-17 Thread Peter Chen 
On Sat, Jul 16, 2016 at 05:30:57PM -0500, Rob Herring wrote:
> On Wed, Jul 13, 2016 at 10:06:45AM +0800, Peter Chen wrote:
> > Add binding doc for generic power sequence library.
> 
> I'd written a review on last version, but forgot to send it out. Anyway, 
> I mostly had the same comments as Philipp and Joshua. A couple of things 
> they missed...
> 
> > 
> > Signed-off-by: Peter Chen 
> > ---
> >  .../bindings/power/pwrseq/pwrseq-generic.txt   | 53 
> > ++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
> > b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > new file mode 100644
> > index 000..186c58c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > @@ -0,0 +1,53 @@
> > +The generic power sequence library
> > +
> > +Some hard-wired USB/MMC devices need to do power sequence to let the
> > +device work normally, the typical power sequence like: enable USB
> > +PHY clock, toggle reset pin, etc. But current Linux USB driver
> > +lacks of such code to do it, it may cause some hard-wired USB devices
> > +works abnormal or can't be recognized by controller at all. The
> > +power sequence will be done before this device can be found at USB
> > +bus.
> > +
> > +The power sequence properties is under the device node.
> > +
> > +Required properties:
> > +- power-sequence: this device needs to do power sequence before enumeration
> > +
> > +Optional properties:
> > +- clocks: the input clock for device.
> > +- reset-gpios: Should specify the GPIO for reset.
> > +- reset-duration-us: the duration in microsecond for assert reset signal.
> > +
> > +Below is the example of USB power sequence properties on USB device
> > +nodes which have two level USB hubs.
> > +
> > + {
> > +   vbus-supply = <_usb_otg1_vbus>;
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_usb_otg1_id>;
> > +   status = "okay";
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   hub: genesys@1 {
> 
> genesys: hub@1
> 
> > +   compatible = "usb5e3,608";
> > +   reg = <1>;
> > +
> > +   power-sequence;
> > +   clocks = < IMX6SX_CLK_CKO>;
> > +   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> > +   reset-duration-us = <10>;
> > +
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   ethernet: asix@1 {
> 
> asix: ethernet@1
> 
> > +   compatible = "usbb95,1708";
> > +   reg = <1>;
> > +
> > +   power-sequence;
> > +   clocks = < IMX6SX_CLK_IPG>;
> > +   reset-gpios = < 6 GPIO_ACTIVE_LOW>; /* 
> > ethernet_rst */
> > +   reset-duration-us = <15>;
> > +   };
> > +   };
> > +};

Will change, thanks.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-16 Thread Rob Herring 
On Wed, Jul 13, 2016 at 10:06:45AM +0800, Peter Chen wrote:
> Add binding doc for generic power sequence library.

I'd written a review on last version, but forgot to send it out. Anyway, 
I mostly had the same comments as Philipp and Joshua. A couple of things 
they missed...

> 
> Signed-off-by: Peter Chen 
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt   | 53 
> ++
>  1 file changed, 53 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
> b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> new file mode 100644
> index 000..186c58c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,53 @@
> +The generic power sequence library
> +
> +Some hard-wired USB/MMC devices need to do power sequence to let the
> +device work normally, the typical power sequence like: enable USB
> +PHY clock, toggle reset pin, etc. But current Linux USB driver
> +lacks of such code to do it, it may cause some hard-wired USB devices
> +works abnormal or can't be recognized by controller at all. The
> +power sequence will be done before this device can be found at USB
> +bus.
> +
> +The power sequence properties is under the device node.
> +
> +Required properties:
> +- power-sequence: this device needs to do power sequence before enumeration
> +
> +Optional properties:
> +- clocks: the input clock for device.
> +- reset-gpios: Should specify the GPIO for reset.
> +- reset-duration-us: the duration in microsecond for assert reset signal.
> +
> +Below is the example of USB power sequence properties on USB device
> +nodes which have two level USB hubs.
> +
> + {
> + vbus-supply = <_usb_otg1_vbus>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_usb_otg1_id>;
> + status = "okay";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + hub: genesys@1 {

genesys: hub@1

> + compatible = "usb5e3,608";
> + reg = <1>;
> +
> + power-sequence;
> + clocks = < IMX6SX_CLK_CKO>;
> + reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> + reset-duration-us = <10>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ethernet: asix@1 {

asix: ethernet@1

> + compatible = "usbb95,1708";
> + reg = <1>;
> +
> + power-sequence;
> + clocks = < IMX6SX_CLK_IPG>;
> + reset-gpios = < 6 GPIO_ACTIVE_LOW>; /* 
> ethernet_rst */
> + reset-duration-us = <15>;
> + };
> + };
> +};
> -- 
> 1.9.1
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-14 Thread Peter Chen 
On Wed, Jul 13, 2016 at 04:20:06PM -0700, Joshua Clayton wrote:
> 
> 
> On 07/13/2016 01:42 AM, Peter Chen wrote:
> > On Wed, Jul 13, 2016 at 09:27:31AM +0200, Philipp Zabel wrote:
> >> Am Mittwoch, den 13.07.2016, 10:06 +0800 schrieb Peter Chen:
> >>> Add binding doc for generic power sequence library.
> >>>
> >>> Signed-off-by: Peter Chen 
> >>> ---
> >>>  .../bindings/power/pwrseq/pwrseq-generic.txt   | 53 
> >>> ++
> >>>  1 file changed, 53 insertions(+)
> >>>  create mode 100644 
> >>> Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> >>>
> >>> diff --git 
> >>> a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
> >>> b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> >>> new file mode 100644
> >>> index 000..186c58c
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> >>> @@ -0,0 +1,53 @@
> >>> +The generic power sequence library
> >>> +
> >>> +Some hard-wired USB/MMC devices need to do power sequence to let the
> >>> +device work normally,
> >> I would replace "to let the device work normally" with "before the
> >> device can be enumerated [on the bus]" here.
> >>
> > Ok.
> >
> >>>  the typical power sequence like: enable USB
> >>> +PHY clock, toggle reset pin, etc. But current Linux USB driver
> >>> +lacks of such code to do it, it may cause some hard-wired USB devices
> >>> +works abnormal or can't be recognized by controller at all. The
> >>> +power sequence will be done before this device can be found at USB
> >>> +bus.
> >>> +
> >>> +The power sequence properties is under the device node.
> >>> +
> >>> +Required properties:
> >>> +- power-sequence: this device needs to do power sequence before 
> >>> enumeration
> >> As Joshua pointed out, is this even needed at all?
> >>
> > If no, how we decide whether allocates pwrseq instance through pwrseq
> > library or not?
> >
> The pwrseq driver is Linux specific. The dts is supposed to be OS agnostic.
> It seems to me that If a driver supports pwrseq and the dts elements
> are there, it should use them, e.g. if there is a clock, enable the clock.
> if there is a reset gpio then take the device into and out of reset during 
> probe.
> 

I agree with you, will delete this property.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-13 Thread Joshua Clayton 


On 07/13/2016 01:42 AM, Peter Chen wrote:
> On Wed, Jul 13, 2016 at 09:27:31AM +0200, Philipp Zabel wrote:
>> Am Mittwoch, den 13.07.2016, 10:06 +0800 schrieb Peter Chen:
>>> Add binding doc for generic power sequence library.
>>>
>>> Signed-off-by: Peter Chen 
>>> ---
>>>  .../bindings/power/pwrseq/pwrseq-generic.txt   | 53 
>>> ++
>>>  1 file changed, 53 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
>>> b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>>> new file mode 100644
>>> index 000..186c58c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>>> @@ -0,0 +1,53 @@
>>> +The generic power sequence library
>>> +
>>> +Some hard-wired USB/MMC devices need to do power sequence to let the
>>> +device work normally,
>> I would replace "to let the device work normally" with "before the
>> device can be enumerated [on the bus]" here.
>>
> Ok.
>
>>>  the typical power sequence like: enable USB
>>> +PHY clock, toggle reset pin, etc. But current Linux USB driver
>>> +lacks of such code to do it, it may cause some hard-wired USB devices
>>> +works abnormal or can't be recognized by controller at all. The
>>> +power sequence will be done before this device can be found at USB
>>> +bus.
>>> +
>>> +The power sequence properties is under the device node.
>>> +
>>> +Required properties:
>>> +- power-sequence: this device needs to do power sequence before enumeration
>> As Joshua pointed out, is this even needed at all?
>>
> If no, how we decide whether allocates pwrseq instance through pwrseq
> library or not?
>
The pwrseq driver is Linux specific. The dts is supposed to be OS agnostic.
It seems to me that If a driver supports pwrseq and the dts elements
are there, it should use them, e.g. if there is a clock, enable the clock.
if there is a reset gpio then take the device into and out of reset during 
probe.

Can you see a  problem with that approach?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-13 Thread Peter Chen 
On Wed, Jul 13, 2016 at 09:27:31AM +0200, Philipp Zabel wrote:
> Am Mittwoch, den 13.07.2016, 10:06 +0800 schrieb Peter Chen:
> > Add binding doc for generic power sequence library.
> > 
> > Signed-off-by: Peter Chen 
> > ---
> >  .../bindings/power/pwrseq/pwrseq-generic.txt   | 53 
> > ++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
> > b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > new file mode 100644
> > index 000..186c58c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> > @@ -0,0 +1,53 @@
> > +The generic power sequence library
> > +
> > +Some hard-wired USB/MMC devices need to do power sequence to let the
> > +device work normally,
> 
> I would replace "to let the device work normally" with "before the
> device can be enumerated [on the bus]" here.
> 

Ok.

> >  the typical power sequence like: enable USB
> > +PHY clock, toggle reset pin, etc. But current Linux USB driver
> > +lacks of such code to do it, it may cause some hard-wired USB devices
> > +works abnormal or can't be recognized by controller at all. The
> > +power sequence will be done before this device can be found at USB
> > +bus.
> > +
> > +The power sequence properties is under the device node.
> > +
> > +Required properties:
> > +- power-sequence: this device needs to do power sequence before enumeration
> 
> As Joshua pointed out, is this even needed at all?
> 

If no, how we decide whether allocates pwrseq instance through pwrseq
library or not?

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-13 Thread Philipp Zabel 
Am Mittwoch, den 13.07.2016, 10:06 +0800 schrieb Peter Chen:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen 
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt   | 53 
> ++
>  1 file changed, 53 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
> b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> new file mode 100644
> index 000..186c58c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,53 @@
> +The generic power sequence library
> +
> +Some hard-wired USB/MMC devices need to do power sequence to let the
> +device work normally,

I would replace "to let the device work normally" with "before the
device can be enumerated [on the bus]" here.

>  the typical power sequence like: enable USB
> +PHY clock, toggle reset pin, etc. But current Linux USB driver
> +lacks of such code to do it, it may cause some hard-wired USB devices
> +works abnormal or can't be recognized by controller at all. The
> +power sequence will be done before this device can be found at USB
> +bus.
> +
> +The power sequence properties is under the device node.
> +
> +Required properties:
> +- power-sequence: this device needs to do power sequence before enumeration

As Joshua pointed out, is this even needed at all?

> +Optional properties:
> +- clocks: the input clock for device.
> +- reset-gpios: Should specify the GPIO for reset.
> +- reset-duration-us: the duration in microsecond for assert reset signal.

With the above two issues sorted out,
Acked-by: Philipp Zabel 

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html