All of lore.kernel.org
 help / color / mirror / Atom feed
From: balbi@kernel.org (Felipe Balbi)
To: linus-amlogic@lists.infradead.org
Subject: [PATCH 2/3] usb: dwc3: of-simple: add support for shared and pulsed reset lines
Date: Mon, 29 Jan 2018 10:18:28 +0200	[thread overview]
Message-ID: <87fu6pjcx7.fsf@linux.intel.com> (raw)
In-Reply-To: <20180128200333.20093-3-martin.blumenstingl@googlemail.com>


Hi,

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> Some SoCs (such as Amlogic Meson GXL for example) share the reset line
> with other components (in case of the Meson GXL example there's a shared
> reset line between the USB2 PHYs, USB3 PHYs and the dwc3 controller).
> Additionally SoC implementations may prefer a reset pulse over level
> resets.
>
> Add an internal per-of_device_id struct which can be used to configure
> whether the reset lines are shared and whether they use level or pulse
> resets.
>
> For now this falls back to the old defaults, which are:
> - reset lines are exclusive
> - level resets are being used
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/usb/dwc3/dwc3-of-simple.c | 65 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> index 7ae0eefc7cc7..ceb9f0cd822a 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -22,11 +22,22 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  
> +/**
> + * struct dwc3_of_simple_params - hardware specific parameters
> + * @shared_resets: indicates that the resets are shared or exclusive
> + * @pulse_resets: use a reset pulse instead of level based resets
> + */
> +struct dwc3_of_simple_params {
> +	bool			shared_resets;
> +	bool			pulse_resets;
> +};
> +
>  struct dwc3_of_simple {
>  	struct device		*dev;
>  	struct clk		**clks;
>  	int			num_clocks;
>  	struct reset_control	*resets;
> +	const struct dwc3_of_simple_params	*params;

instead, you can add these two fields here:

	bool shared_resets;
        bool pulse_resets;

and ...

> @@ -90,17 +101,26 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, simple);
>  	simple->dev = dev;
> +	simple->params = of_device_get_match_data(dev);
>  
> -	simple->resets = of_reset_control_array_get_optional_exclusive(np);
> +	simple->resets = of_reset_control_array_get(np,
> +						simple->params->shared_resets,
> +						true);

wrap this with a of_device_is_compatible() check:

	if (of_device_is_compatible(dev->of_node, "foobar")) {
        	simple->shared_resets = true;
		simple->pulse_resets = true;
	}

or something like that. Then we don't need to add a new
dwc3_of_simple_params for everybody.

Also, the why isn't the reset type (pulse vs level) handled by reset
framework itself? Why does dwc3-of-simple need to know about it?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20180129/dd2feb7d/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	gregkh@linuxfoundation.org, robh+dt@kernel.org,
	mark.rutland@arm.com, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org
Cc: linux-amlogic@lists.infradead.org
Subject: [2/3] usb: dwc3: of-simple: add support for shared and pulsed reset lines
Date: Mon, 29 Jan 2018 10:18:28 +0200	[thread overview]
Message-ID: <87fu6pjcx7.fsf@linux.intel.com> (raw)

Hi,

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> Some SoCs (such as Amlogic Meson GXL for example) share the reset line
> with other components (in case of the Meson GXL example there's a shared
> reset line between the USB2 PHYs, USB3 PHYs and the dwc3 controller).
> Additionally SoC implementations may prefer a reset pulse over level
> resets.
>
> Add an internal per-of_device_id struct which can be used to configure
> whether the reset lines are shared and whether they use level or pulse
> resets.
>
> For now this falls back to the old defaults, which are:
> - reset lines are exclusive
> - level resets are being used
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/usb/dwc3/dwc3-of-simple.c | 65 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> index 7ae0eefc7cc7..ceb9f0cd822a 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -22,11 +22,22 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  
> +/**
> + * struct dwc3_of_simple_params - hardware specific parameters
> + * @shared_resets: indicates that the resets are shared or exclusive
> + * @pulse_resets: use a reset pulse instead of level based resets
> + */
> +struct dwc3_of_simple_params {
> +	bool			shared_resets;
> +	bool			pulse_resets;
> +};
> +
>  struct dwc3_of_simple {
>  	struct device		*dev;
>  	struct clk		**clks;
>  	int			num_clocks;
>  	struct reset_control	*resets;
> +	const struct dwc3_of_simple_params	*params;

instead, you can add these two fields here:

	bool shared_resets;
        bool pulse_resets;

and ...

> @@ -90,17 +101,26 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, simple);
>  	simple->dev = dev;
> +	simple->params = of_device_get_match_data(dev);
>  
> -	simple->resets = of_reset_control_array_get_optional_exclusive(np);
> +	simple->resets = of_reset_control_array_get(np,
> +						simple->params->shared_resets,
> +						true);

wrap this with a of_device_is_compatible() check:

	if (of_device_is_compatible(dev->of_node, "foobar")) {
        	simple->shared_resets = true;
		simple->pulse_resets = true;
	}

or something like that. Then we don't need to add a new
dwc3_of_simple_params for everybody.

Also, the why isn't the reset type (pulse vs level) handled by reset
framework itself? Why does dwc3-of-simple need to know about it?

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Martin Blumenstingl
	<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Subject: Re: [PATCH 2/3] usb: dwc3: of-simple: add support for shared and pulsed reset lines
Date: Mon, 29 Jan 2018 10:18:28 +0200	[thread overview]
Message-ID: <87fu6pjcx7.fsf@linux.intel.com> (raw)
In-Reply-To: <20180128200333.20093-3-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

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


Hi,

Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> writes:
> Some SoCs (such as Amlogic Meson GXL for example) share the reset line
> with other components (in case of the Meson GXL example there's a shared
> reset line between the USB2 PHYs, USB3 PHYs and the dwc3 controller).
> Additionally SoC implementations may prefer a reset pulse over level
> resets.
>
> Add an internal per-of_device_id struct which can be used to configure
> whether the reset lines are shared and whether they use level or pulse
> resets.
>
> For now this falls back to the old defaults, which are:
> - reset lines are exclusive
> - level resets are being used
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  drivers/usb/dwc3/dwc3-of-simple.c | 65 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> index 7ae0eefc7cc7..ceb9f0cd822a 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -22,11 +22,22 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  
> +/**
> + * struct dwc3_of_simple_params - hardware specific parameters
> + * @shared_resets: indicates that the resets are shared or exclusive
> + * @pulse_resets: use a reset pulse instead of level based resets
> + */
> +struct dwc3_of_simple_params {
> +	bool			shared_resets;
> +	bool			pulse_resets;
> +};
> +
>  struct dwc3_of_simple {
>  	struct device		*dev;
>  	struct clk		**clks;
>  	int			num_clocks;
>  	struct reset_control	*resets;
> +	const struct dwc3_of_simple_params	*params;

instead, you can add these two fields here:

	bool shared_resets;
        bool pulse_resets;

and ...

> @@ -90,17 +101,26 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, simple);
>  	simple->dev = dev;
> +	simple->params = of_device_get_match_data(dev);
>  
> -	simple->resets = of_reset_control_array_get_optional_exclusive(np);
> +	simple->resets = of_reset_control_array_get(np,
> +						simple->params->shared_resets,
> +						true);

wrap this with a of_device_is_compatible() check:

	if (of_device_is_compatible(dev->of_node, "foobar")) {
        	simple->shared_resets = true;
		simple->pulse_resets = true;
	}

or something like that. Then we don't need to add a new
dwc3_of_simple_params for everybody.

Also, the why isn't the reset type (pulse vs level) handled by reset
framework itself? Why does dwc3-of-simple need to know about it?

-- 
balbi

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

  reply	other threads:[~2018-01-29  8:18 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-28 20:03 [PATCH 0/3] DWC3 support for Amlogic Meson AXG and GXL SoCs Martin Blumenstingl
2018-01-28 20:03 ` Martin Blumenstingl
2018-01-28 20:03 ` [PATCH 1/3] dt-bindings: usb: add support for dwc3 controller on Amlogic Meson GX Martin Blumenstingl
2018-01-28 20:03   ` Martin Blumenstingl
2018-01-28 20:03   ` [1/3] " Martin Blumenstingl
2018-02-05  6:07   ` [PATCH 1/3] " Rob Herring
2018-02-05  6:07     ` Rob Herring
2018-02-05  6:07     ` [1/3] " Rob Herring
2018-01-28 20:03 ` [PATCH 2/3] usb: dwc3: of-simple: add support for shared and pulsed reset lines Martin Blumenstingl
2018-01-28 20:03   ` Martin Blumenstingl
2018-01-28 20:03   ` [2/3] " Martin Blumenstingl
2018-01-29  8:18   ` Felipe Balbi [this message]
2018-01-29  8:18     ` [PATCH 2/3] " Felipe Balbi
2018-01-29  8:18     ` [2/3] " Felipe Balbi
2018-02-03 20:00     ` [PATCH 2/3] " Martin Blumenstingl
2018-02-03 20:00       ` Martin Blumenstingl
2018-02-03 20:00       ` [2/3] " Martin Blumenstingl
2018-01-28 20:03 ` [PATCH 3/3] usb: dwc3: of-simple: add support for the Amlogic Meson GXL and AXG SoCs Martin Blumenstingl
2018-01-28 20:03   ` Martin Blumenstingl
2018-01-28 20:03   ` [3/3] " Martin Blumenstingl

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=87fu6pjcx7.fsf@linux.intel.com \
    --to=balbi@kernel.org \
    --cc=linus-amlogic@lists.infradead.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.