[PATCH v2 2/3] USB: add of_platform glue driver for FSL USB DR controller

[Anatolij Gustschin]

Hi Grant,

On Tue, 28 Sep 2010 19:01:28 +0900
Grant Likely <grant.likely at secretlab.ca> wrote:
...
> Looks pretty good.  Comments below.  Main comment is that with the
> recent changes in mainline, this no longer needs to be an
> of_platform_driver.  It can be a plain old platform_driver instead.
> It gets registered as a platform_driver anyway, but of_platform_driver
> is a shim that adds a bit of overhead, so it is best to avoid it.
> I'll detail the changes you need to make in my comments below.

Thanks. I'll change to platform_driver. My reply below.

...
> > +{
> > +       struct device_node *np = ofdev->dev.of_node;
> > +       struct platform_device *usb_dev;
> > +       struct fsl_usb2_platform_data data, *pdata;
> > +       struct fsl_usb2_dev_data *dev_data;
> > +       const unsigned char *prop;
> > +       static unsigned int idx;
> > +       int i;
> > +
> > +       if (!of_device_is_available(np))
> > +               return -ENODEV;
> > +
> > +       pdata = match->data;
> > +       if (!pdata) {
> > +               memset(&data, 0, sizeof(data));
> > +               pdata = &data;
> > +       }
> 
> This is bad behaviour.  *match->data must not be modified in probe
> because multiple instances of the device can exist.  The target of
> pdata is modified later in the probe routine.
> 
> However, the above 4 lines can be removed entirely since none of the
> fsl_usb2_mph_dr_of_match entries actually set the data pointer.  The
> local 'data' structure can be used directly.

A match entry for mpc5121 is added by the third patch. I'll fix
this so that only local 'data' structure is modified later in the
probe routine.

Thanks,
Anatolij