On Mon, 2014-11-17 at 09:46 +0800, Peter Chen wrote:
> On Fri, Nov 14, 2014 at 04:33:22PM +0200, Andy Shevchenko wrote:
> > Since PHY for Chipidea is optional (not all SoCs having PHY for Chipidea should
> > be programmed), we register 'nop' PHY for platforms that does not have
> > programmable PHY.
> >
My answers below.
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > drivers/usb/chipidea/ci_hdrc_pci.c | 32 +++++++++++++++++++++++++-------
> > 1 file changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
> > index 241ae34..7e0b701 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_pci.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_pci.c
> > @@ -16,10 +16,16 @@
> > #include <linux/interrupt.h>
> > #include <linux/usb/gadget.h>
> > #include <linux/usb/chipidea.h>
> > +#include <linux/usb/usb_phy_generic.h>
> >
> > /* driver name */
> > #define UDC_DRIVER_NAME "ci_hdrc_pci"
> >
> > +struct ci_hdrc_pci {
> > + struct platform_device *ci;
> > + struct platform_device *phy;
> > +};
> > +
> > /******************************************************************************
> > * PCI block
> > *****************************************************************************/
> > @@ -52,7 +58,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> > {
> > struct ci_hdrc_platform_data *platdata = (void *)id->driver_data;
> > - struct platform_device *plat_ci;
> > + struct ci_hdrc_pci *ci;
> > struct resource res[3];
> > int retval = 0, nres = 2;
> >
> > @@ -61,6 +67,10 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
> > return -ENODEV;
> > }
> >
> > + ci = devm_kzalloc(&pdev->dev, sizeof(*ci), GFP_KERNEL);
> > + if (!ci)
> > + return -ENOMEM;
> > +
> > retval = pcim_enable_device(pdev);
> > if (retval)
> > return retval;
> > @@ -73,20 +83,27 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
> > pci_set_master(pdev);
> > pci_try_set_mwi(pdev);
> >
> > + /* register a nop PHY */
> > + ci->phy = usb_phy_generic_register();
> > + if (!ci->phy)
> > + return -ENOMEM;
>
> Rebase my next tree where antoine's generic phy support for chipidea
> driver in it, and for usb phy, the "phy" changes to "usb_phy".
Okay, I'm going to change the name in this structure, otherwise how the
new patches in your tree helps me (PCI driver)?
>
> > +
> > memset(res, 0, sizeof(res));
> > +
>
> Why this blank line is needed?
Not needed.
>
> > res[0].start = pci_resource_start(pdev, 0);
> > res[0].end = pci_resource_end(pdev, 0);
> > res[0].flags = IORESOURCE_MEM;
> > res[1].start = pdev->irq;
> > res[1].flags = IORESOURCE_IRQ;
> >
> > - plat_ci = ci_hdrc_add_device(&pdev->dev, res, nres, platdata);
> > - if (IS_ERR(plat_ci)) {
> > + ci->ci = ci_hdrc_add_device(&pdev->dev, res, nres, platdata);
> > + if (IS_ERR(ci->ci)) {
> > dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> > - return PTR_ERR(plat_ci);
> > + usb_phy_generic_unregister(ci->phy);
> > + return PTR_ERR(ci->ci);
> > }
> >
> > - pci_set_drvdata(pdev, plat_ci);
> > + pci_set_drvdata(pdev, ci);
> >
> > return 0;
> > }
> > @@ -101,9 +118,10 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
> > */
> > static void ci_hdrc_pci_remove(struct pci_dev *pdev)
> > {
> > - struct platform_device *plat_ci = pci_get_drvdata(pdev);
> > + struct ci_hdrc_pci *ci = pci_get_drvdata(pdev);
> >
> > - ci_hdrc_remove_device(plat_ci);
> > + ci_hdrc_remove_device(ci->ci);
> > + usb_phy_generic_unregister(ci->phy);
> > }
> >
> > /**
> > --
> > 2.1.3
> >
>
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
