diff mbox

[v2,06/11] USB: mxs-phy: add basic otg support

Message ID 1346137397-32374-7-git-send-email-richard.zhao@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Richard Zhao Aug. 28, 2012, 7:03 a.m. UTC 
Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/usb/otg/mxs-phy.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Felipe Balbi Sept. 11, 2012, 9:05 a.m. UTC | #1 
Hi,

On Tue, Aug 28, 2012 at 03:03:12PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>

if you add a commit log you can add my:

Acked-by: Felipe Balbi <balbi@ti.com>

> ---
>  drivers/usb/otg/mxs-phy.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
> index c1a67cb..6a03e97 100644
> --- a/drivers/usb/otg/mxs-phy.c
> +++ b/drivers/usb/otg/mxs-phy.c
> @@ -97,12 +97,24 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
>  	return 0;
>  }
>  
> +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +	return 0;
> +}
> +
> +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> +					struct usb_gadget *gadget)
> +{
> +	return 0;
> +}
> +
>  static int mxs_phy_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	void __iomem *base;
>  	struct clk *clk;
>  	struct mxs_phy *mxs_phy;
> +	struct usb_otg *otg;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
> @@ -139,6 +151,15 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  
>  	mxs_phy->clk = clk;
>  
> +	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
> +	if (!otg)
> +		return -ENOMEM;
> +	otg->phy = &mxs_phy->phy;
> +	otg->set_host = mxs_phy_set_host;
> +	otg->set_peripheral = mxs_phy_set_peripheral;
> +
> +	mxs_phy->phy.otg = otg;
> +
>  	platform_set_drvdata(pdev, &mxs_phy->phy);
>  
>  	return 0;
> -- 
> 1.7.9.5
> 
>
Heikki Krogerus Sept. 12, 2012, 10:39 a.m. UTC | #2 
Hi,

On Tue, Aug 28, 2012 at 03:03:12PM +0800, Richard Zhao wrote:
> +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{

Shouldn't you at least save the host pointer?

otg->host = host;

> +	return 0;
> +}
> +
> +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> +					struct usb_gadget *gadget)
> +{

And the same with the gadget?

> +	return 0;
> +}
Richard Zhao Sept. 14, 2012, 8:30 a.m. UTC | #3 
On Wed, Sep 12, 2012 at 01:39:01PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> On Tue, Aug 28, 2012 at 03:03:12PM +0800, Richard Zhao wrote:
> > +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> > +{
> 
> Shouldn't you at least save the host pointer?
> 
> otg->host = host;
It looks making sense, though it's not called by chipidea driver.

> 
> > +	return 0;
> > +}
> > +
> > +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> > +					struct usb_gadget *gadget)
> > +{
> 
> And the same with the gadget?

Thanks
Richard
> 
> > +	return 0;
> > +}
> 
> -- 
> heikki
>
Chen Peter-B29397 Sept. 14, 2012, 8:56 a.m. UTC | #4 
> diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
> index c1a67cb..6a03e97 100644
> --- a/drivers/usb/otg/mxs-phy.c
> +++ b/drivers/usb/otg/mxs-phy.c
> @@ -97,12 +97,24 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
> int port)
>  	return 0;
>  }
> 
> +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> +{
> +	return 0;
> +}
> +
> +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> +					struct usb_gadget *gadget)
> +{
> +	return 0;
> +}
> +
>  static int mxs_phy_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	void __iomem *base;
>  	struct clk *clk;
>  	struct mxs_phy *mxs_phy;
> +	struct usb_otg *otg;
> 
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
> @@ -139,6 +151,15 @@ static int mxs_phy_probe(struct platform_device
> *pdev)
> 
>  	mxs_phy->clk = clk;
> 
> +	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
> +	if (!otg)
> +		return -ENOMEM;
> +	otg->phy = &mxs_phy->phy;
> +	otg->set_host = mxs_phy_set_host;
> +	otg->set_peripheral = mxs_phy_set_peripheral;
> +
> +	mxs_phy->phy.otg = otg;
> +

Put otg struct at PHY driver is not a good practice.
Only OTG driver who handles ID interrupt cases about host and gadget struct.
PHY is a transceiver, it not cares otg, host and device at all.

>  	platform_set_drvdata(pdev, &mxs_phy->phy);
> 
>  	return 0;
> --
> 1.7.9.5
Richard Zhao Sept. 14, 2012, 10:53 a.m. UTC | #5 
On Fri, Sep 14, 2012 at 04:56:13PM +0800, Chen Peter-B29397 wrote:
>  
> > diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
> > index c1a67cb..6a03e97 100644
> > --- a/drivers/usb/otg/mxs-phy.c
> > +++ b/drivers/usb/otg/mxs-phy.c
> > @@ -97,12 +97,24 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
> > int port)
> >  	return 0;
> >  }
> > 
> > +static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
> > +{
> > +	return 0;
> > +}
> > +
> > +static int mxs_phy_set_peripheral(struct usb_otg *otg,
> > +					struct usb_gadget *gadget)
> > +{
> > +	return 0;
> > +}
> > +
> >  static int mxs_phy_probe(struct platform_device *pdev)
> >  {
> >  	struct resource *res;
> >  	void __iomem *base;
> >  	struct clk *clk;
> >  	struct mxs_phy *mxs_phy;
> > +	struct usb_otg *otg;
> > 
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res) {
> > @@ -139,6 +151,15 @@ static int mxs_phy_probe(struct platform_device
> > *pdev)
> > 
> >  	mxs_phy->clk = clk;
> > 
> > +	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
> > +	if (!otg)
> > +		return -ENOMEM;
> > +	otg->phy = &mxs_phy->phy;
> > +	otg->set_host = mxs_phy_set_host;
> > +	otg->set_peripheral = mxs_phy_set_peripheral;
> > +
> > +	mxs_phy->phy.otg = otg;
> > +
> 
> Put otg struct at PHY driver is not a good practice.
> Only OTG driver who handles ID interrupt cases about host and gadget struct.
> PHY is a transceiver, it not cares otg, host and device at all.
I understand your concern, but this patch don't mean to restructure
common code.

Thanks
Richard
> 
> >  	platform_set_drvdata(pdev, &mxs_phy->phy);
> > 
> >  	return 0;
> > --
> > 1.7.9.5
>
diff mbox

Patch

diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index c1a67cb..6a03e97 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -97,12 +97,24 @@  static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -139,6 +151,15 @@  static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
 	return 0;