| Message ID | 1346137397-32374-7-git-send-email-richard.zhao@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > >
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; > +}
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 >
> 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
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 --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;
Signed-off-by: Richard Zhao <richard.zhao@freescale.com> --- drivers/usb/otg/mxs-phy.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)