Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1625849 > unrolled thread

[RFC] usb-phy-generic: Add support to SMSC USB3315

Back to article view | Back to linux.kernel

Contents

  [RFC] usb-phy-generic: Add support to SMSC USB3315 Peter Senna Tschudin <peter.senna@collabora.com> - 2017-04-19 08:20 +0200
    Re: [RFC] usb-phy-generic: Add support to SMSC USB3315 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-19 12:10 +0200
      Re: [RFC] usb-phy-generic: Add support to SMSC USB3315 Peter Senna Tschudin <peter.senna@collabora.com> - 2017-04-19 12:30 +0200
        Re: [RFC] usb-phy-generic: Add support to SMSC USB3315 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-19 19:30 +0200

#1625849 — [RFC] usb-phy-generic: Add support to SMSC USB3315

We need the SMSC USB3315 clock and regulator to always be initialized.
We also need the PHY driver to take the PHY out of reset. This patch
extends the existing USB generic nop phy driver to include a new
initialization path.

A new compatible string "smsc,usb3315" is used to decide which
initialization path to use.

CC: Peter Chen <peter.chen@nxp.com>
CC: Stephen Boyd <sboyd@codeaurora.org>
CC: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---

This is a follow-up of previous discussion:
  https://www.spinics.net/lists/linux-usb/msg146680.html

 drivers/usb/phy/phy-generic.c | 33 +++++++++++++++++++++++++++++----
 drivers/usb/phy/phy-generic.h |  1 +
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 89d6e7a..6ea9ce4 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -151,6 +151,9 @@ int usb_gen_phy_init(struct usb_phy *phy)
 	struct usb_phy_generic *nop = dev_get_drvdata(phy->dev);
 	int ret;
 
+	if (nop->init_done)
+		return 0;
+
 	if (!IS_ERR(nop->vcc)) {
 		if (regulator_enable(nop->vcc))
 			dev_err(phy->dev, "Failed to enable power\n");
@@ -164,6 +167,8 @@ int usb_gen_phy_init(struct usb_phy *phy)
 
 	nop_reset(nop);
 
+	nop->init_done = true;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usb_gen_phy_init);
@@ -216,18 +221,29 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
 	otg->host = host;
 	return 0;
 }
+int smsc_usb3315_init(struct usb_phy_generic *nop)
+{
+	/*
+	 * If the gpio for controlling reset state is not available, try again
+	 * later
+	 */
+	if(!nop->gpiod_reset)
+		return -EPROBE_DEFER;
+
+	return usb_gen_phy_init(&nop->phy);
+}
 
 int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
 		struct usb_phy_generic_platform_data *pdata)
 {
+	struct device_node *node = NULL;
 	enum usb_phy_type type = USB_PHY_TYPE_USB2;
 	int err = 0;
-
 	u32 clk_rate = 0;
 	bool needs_vcc = false;
 
 	if (dev->of_node) {
-		struct device_node *node = dev->of_node;
+		node = dev->of_node;
 
 		if (of_property_read_u32(node, "clock-frequency", &clk_rate))
 			clk_rate = 0;
@@ -304,6 +320,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
 	nop->phy.otg->set_host		= nop_set_host;
 	nop->phy.otg->set_peripheral	= nop_set_peripheral;
 
+	if(node && of_device_is_compatible(node, "smsc,usb3315")) {
+		err = smsc_usb3315_init(nop);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usb_phy_gen_create_phy);
@@ -318,6 +340,10 @@ static int usb_phy_generic_probe(struct platform_device *pdev)
 	if (!nop)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, nop);
+
+	nop->init_done = false;
+
 	err = usb_phy_gen_create_phy(dev, nop, dev_get_platdata(&pdev->dev));
 	if (err)
 		return err;
@@ -346,8 +372,6 @@ static int usb_phy_generic_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	platform_set_drvdata(pdev, nop);
-
 	return 0;
 }
 
@@ -362,6 +386,7 @@ static int usb_phy_generic_remove(struct platform_device *pdev)
 
 static const struct of_device_id nop_xceiv_dt_ids[] = {
 	{ .compatible = "usb-nop-xceiv" },
+	{ .compatible = "smsc,usb3315" },
 	{ }
 };
 
diff --git a/drivers/usb/phy/phy-generic.h b/drivers/usb/phy/phy-generic.h
index 0d0eadd..db4ade6 100644
--- a/drivers/usb/phy/phy-generic.h
+++ b/drivers/usb/phy/phy-generic.h
@@ -14,6 +14,7 @@ struct usb_phy_generic {
 	struct gpio_desc *gpiod_vbus;
 	struct regulator *vbus_draw;
 	bool vbus_draw_enabled;
+	bool init_done;
 	unsigned long mA;
 	unsigned int vbus;
 };
-- 
2.9.3

[toc] | [next] | [standalone]

#1625985

Hello!

On 4/19/2017 9:14 AM, Peter Senna Tschudin wrote:

> We need the SMSC USB3315 clock and regulator to always be initialized.
> We also need the PHY driver to take the PHY out of reset. This patch
> extends the existing USB generic nop phy driver to include a new
> initialization path.
>
> A new compatible string "smsc,usb3315" is used to decide which
> initialization path to use.
>
> CC: Peter Chen <peter.chen@nxp.com>
> CC: Stephen Boyd <sboyd@codeaurora.org>
> CC: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> ---
>
> This is a follow-up of previous discussion:
>   https://www.spinics.net/lists/linux-usb/msg146680.html
>
>  drivers/usb/phy/phy-generic.c | 33 +++++++++++++++++++++++++++++----
>  drivers/usb/phy/phy-generic.h |  1 +
>  2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index 89d6e7a..6ea9ce4 100644
> --- a/drivers/usb/phy/phy-generic.c
> +++ b/drivers/usb/phy/phy-generic.c
[...]
> @@ -216,18 +221,29 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>  	otg->host = host;
>  	return 0;
>  }

    Need empty line here.

> +int smsc_usb3315_init(struct usb_phy_generic *nop)
> +{
> +	/*
> +	 * If the gpio for controlling reset state is not available, try again
> +	 * later
> +	 */
> +	if(!nop->gpiod_reset)

    You hadn't run the patch thru scripts/checkpatch.pl before posting -- need 
space between *if* and (.

[...]
> @@ -304,6 +320,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
>  	nop->phy.otg->set_host		= nop_set_host;
>  	nop->phy.otg->set_peripheral	= nop_set_peripheral;
>
> +	if(node && of_device_is_compatible(node, "smsc,usb3315")) {

    Same here.

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1625998

On Wed, Apr 19, 2017 at 01:03:23PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 4/19/2017 9:14 AM, Peter Senna Tschudin wrote:
> 
> > We need the SMSC USB3315 clock and regulator to always be initialized.
> > We also need the PHY driver to take the PHY out of reset. This patch
> > extends the existing USB generic nop phy driver to include a new
> > initialization path.
> > 
> > A new compatible string "smsc,usb3315" is used to decide which
> > initialization path to use.
> > 
> > CC: Peter Chen <peter.chen@nxp.com>
> > CC: Stephen Boyd <sboyd@codeaurora.org>
> > CC: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> > ---
> > 
> > This is a follow-up of previous discussion:
> >   https://www.spinics.net/lists/linux-usb/msg146680.html
> > 
> >  drivers/usb/phy/phy-generic.c | 33 +++++++++++++++++++++++++++++----
> >  drivers/usb/phy/phy-generic.h |  1 +
> >  2 files changed, 30 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> > index 89d6e7a..6ea9ce4 100644
> > --- a/drivers/usb/phy/phy-generic.c
> > +++ b/drivers/usb/phy/phy-generic.c
> [...]
> > @@ -216,18 +221,29 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
> >  	otg->host = host;
> >  	return 0;
> >  }
> 
>    Need empty line here.
> 
> > +int smsc_usb3315_init(struct usb_phy_generic *nop)
> > +{
> > +	/*
> > +	 * If the gpio for controlling reset state is not available, try again
> > +	 * later
> > +	 */
> > +	if(!nop->gpiod_reset)
> 
>    You hadn't run the patch thru scripts/checkpatch.pl before posting --
> need space between *if* and (.
> 
> [...]
> > @@ -304,6 +320,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
> >  	nop->phy.otg->set_host		= nop_set_host;
> >  	nop->phy.otg->set_peripheral	= nop_set_peripheral;
> > 
> > +	if(node && of_device_is_compatible(node, "smsc,usb3315")) {
> 
>    Same here.
> 
> [...]
> 
> MBR, Sergei

Thank you for the review Sergei! Should I send V2 of this RFC fixing
these issues or wait for comments on the validity of this approach?


> 

[toc] | [prev] | [next] | [standalone]

#1626629

On 04/19/2017 01:24 PM, Peter Senna Tschudin wrote:

>>> We need the SMSC USB3315 clock and regulator to always be initialized.
>>> We also need the PHY driver to take the PHY out of reset. This patch
>>> extends the existing USB generic nop phy driver to include a new
>>> initialization path.
>>>
>>> A new compatible string "smsc,usb3315" is used to decide which
>>> initialization path to use.
>>>
>>> CC: Peter Chen <peter.chen@nxp.com>
>>> CC: Stephen Boyd <sboyd@codeaurora.org>
>>> CC: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
>>> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
>>> ---
>>>
>>> This is a follow-up of previous discussion:
>>>   https://www.spinics.net/lists/linux-usb/msg146680.html
>>>
>>>  drivers/usb/phy/phy-generic.c | 33 +++++++++++++++++++++++++++++----
>>>  drivers/usb/phy/phy-generic.h |  1 +
>>>  2 files changed, 30 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
>>> index 89d6e7a..6ea9ce4 100644
>>> --- a/drivers/usb/phy/phy-generic.c
>>> +++ b/drivers/usb/phy/phy-generic.c
>> [...]
>>> @@ -216,18 +221,29 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>>>  	otg->host = host;
>>>  	return 0;
>>>  }
>>
>>    Need empty line here.
>>
>>> +int smsc_usb3315_init(struct usb_phy_generic *nop)
>>> +{
>>> +	/*
>>> +	 * If the gpio for controlling reset state is not available, try again
>>> +	 * later
>>> +	 */
>>> +	if(!nop->gpiod_reset)
>>
>>    You hadn't run the patch thru scripts/checkpatch.pl before posting --
>> need space between *if* and (.
>>
>> [...]
>>> @@ -304,6 +320,12 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
>>>  	nop->phy.otg->set_host		= nop_set_host;
>>>  	nop->phy.otg->set_peripheral	= nop_set_peripheral;
>>>
>>> +	if(node && of_device_is_compatible(node, "smsc,usb3315")) {
>>
>>    Same here.
>>
>> [...]
>>
>> MBR, Sergei
>
> Thank you for the review Sergei! Should I send V2 of this RFC fixing
> these issues or wait for comments on the validity of this approach?

    Wait, of course, if this is RFC...

WBR, Sergei

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web