All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joshua Clayton <stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	oscar-Bdbr4918Nnnk1uMJSBkQmQ@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mail-APzI5cXaD1zVlRWJc41N0YvC60bnQu0Y@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org,
	festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library
Date: Thu, 7 Jul 2016 16:53:07 -0700	[thread overview]
Message-ID: <577EEB63.3020007@gmail.com> (raw)
In-Reply-To: <1467884834.4236.36.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>



On 07/07/2016 02:47 AM, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
>> Add binding doc for generic power sequence library.
>>
>> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
>> ---
>>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
>>  1 file changed, 56 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 0000000..4b23834
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>> @@ -0,0 +1,56 @@
>> +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.
>> +- clock-name: must be "pwrseq-clk"
> The "-clk" in the clock name is redundant.
>
>> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
>> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> I understand you want to make it explicit that this GPIO is for the
> pwrseq library, but are we really gaining anything over just calling
> these reset-gpios and reset-duration-us?
> The same applies to the clock name above.
using  reset-gpios makes sense to  me too.
The above "power-sequence" might then be better called "reset-on-init",
But really, if a device has a reset gpio shouldn't the default behavior be to
reset it on boot and when coming back from sleep?
Is a special property even needed?
>
>> +Below is the example of USB power sequence properties on USB device
>> +nodes which have two level USB hubs.
>> +
>> +&usbotg1 {
>> +	vbus-supply = <&reg_usb_otg1_vbus>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_usb_otg1_id>;
>> +	status = "okay";
>> +
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	hub: genesys@1 {
>> +		compatible = "usb5e3,608";
>> +		reg = <1>;
>> +
>> +		power-sequence;
>> +		clocks = <&clks IMX6SX_CLK_CKO>;
>> +		clock-names = "pwrseq-clk";
>> +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
>> +		pwrseq-reset-duration-us = <10>;
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		ethernet: asix@1 {
>> +			compatible = "usbb95,1708";
>> +			reg = <1>;
>> +
>> +			power-sequence;
>> +			clocks = <&clks IMX6SX_CLK_IPG>;
>> +			clock-names = "pwrseq-clk";
>> +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>> +			pwrseq-reset-duration-us = <15>;
>> +		};
> This looks weird. The hub and ethernet chips don't have "pwrseq" clock
> and reset input pins. I'd remove the clock-names and pwrseq- reset
> prefix.
>
> regards
> Philipp
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

WARNING: multiple messages have this Message-ID (diff)
From: stillcompiling@gmail.com (Joshua Clayton)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library
Date: Thu, 7 Jul 2016 16:53:07 -0700	[thread overview]
Message-ID: <577EEB63.3020007@gmail.com> (raw)
In-Reply-To: <1467884834.4236.36.camel@pengutronix.de>



On 07/07/2016 02:47 AM, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 17:14 +0800 schrieb Peter Chen:
>> Add binding doc for generic power sequence library.
>>
>> Signed-off-by: Peter Chen <peter.chen@nxp.com>
>> ---
>>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 56 ++++++++++++++++++++++
>>  1 file changed, 56 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 0000000..4b23834
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
>> @@ -0,0 +1,56 @@
>> +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.
>> +- clock-name: must be "pwrseq-clk"
> The "-clk" in the clock name is redundant.
>
>> +- pwrseq-reset-gpios: Should specify the GPIO for reset.
>> +- pwrseq-reset-duration-us: the duration in microsecond for assert reset signal.
> I understand you want to make it explicit that this GPIO is for the
> pwrseq library, but are we really gaining anything over just calling
> these reset-gpios and reset-duration-us?
> The same applies to the clock name above.
using  reset-gpios makes sense to  me too.
The above "power-sequence" might then be better called "reset-on-init",
But really, if a device has a reset gpio shouldn't the default behavior be to
reset it on boot and when coming back from sleep?
Is a special property even needed?
>
>> +Below is the example of USB power sequence properties on USB device
>> +nodes which have two level USB hubs.
>> +
>> +&usbotg1 {
>> +	vbus-supply = <&reg_usb_otg1_vbus>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_usb_otg1_id>;
>> +	status = "okay";
>> +
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	hub: genesys at 1 {
>> +		compatible = "usb5e3,608";
>> +		reg = <1>;
>> +
>> +		power-sequence;
>> +		clocks = <&clks IMX6SX_CLK_CKO>;
>> +		clock-names = "pwrseq-clk";
>> +		pwrseq-reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
>> +		pwrseq-reset-duration-us = <10>;
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		ethernet: asix at 1 {
>> +			compatible = "usbb95,1708";
>> +			reg = <1>;
>> +
>> +			power-sequence;
>> +			clocks = <&clks IMX6SX_CLK_IPG>;
>> +			clock-names = "pwrseq-clk";
>> +			pwrseq-reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
>> +			pwrseq-reset-duration-us = <15>;
>> +		};
> This looks weird. The hub and ethernet chips don't have "pwrseq" clock
> and reset input pins. I'd remove the clock-names and pwrseq- reset
> prefix.
>
> regards
> Philipp
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2016-07-07 23:53 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-07  9:14 [PATCH 0/6] power: add power sequence library Peter Chen
2016-07-07  9:14 ` Peter Chen
     [not found] ` <1467882892-27589-1-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>
2016-07-07  9:14   ` [PATCH 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic " Peter Chen
2016-07-07  9:14     ` Peter Chen
     [not found]     ` <1467882892-27589-2-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>
2016-07-07  9:47       ` Philipp Zabel
2016-07-07  9:47         ` Philipp Zabel
     [not found]         ` <1467884834.4236.36.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2016-07-07 23:53           ` Joshua Clayton [this message]
2016-07-07 23:53             ` Joshua Clayton
2016-07-08  2:39             ` Peter Chen
2016-07-08  2:39               ` Peter Chen
2016-07-08  2:37         ` Peter Chen
2016-07-08  2:37           ` Peter Chen
2016-07-07  9:14 ` [PATCH 2/6] power: add " Peter Chen
2016-07-07  9:14   ` Peter Chen
2016-07-07  9:14 ` [PATCH 3/6] binding-doc: usb: usb-device: add optional properties for power sequence Peter Chen
2016-07-07  9:14   ` Peter Chen
2016-07-07  9:14 ` [PATCH 4/6] usb: core: add power sequence handling for USB devices Peter Chen
2016-07-07  9:14   ` Peter Chen
2016-07-07  9:14 ` [PATCH 5/6] usb: chipidea: host: let the hcd know's parent device node Peter Chen
2016-07-07  9:14   ` Peter Chen
2016-07-07 22:56   ` Stephen Boyd
2016-07-07 22:56     ` Stephen Boyd
2016-07-08  1:54     ` Peter Chen
2016-07-08  1:54       ` Peter Chen
2016-07-07  9:14 ` [PATCH 6/6] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property Peter Chen
2016-07-07  9:14   ` Peter Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=577EEB63.3020007@gmail.com \
    --to=stillcompiling-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mail-APzI5cXaD1zVlRWJc41N0YvC60bnQu0Y@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=oscar-Bdbr4918Nnnk1uMJSBkQmQ@public.gmane.org \
    --cc=p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=peter.chen-3arQi8VN3Tc@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    --cc=troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org \
    --cc=ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.