On Mon, Mar 25, 2013 at 01:47:19PM -0400, Alan Stern wrote:
> On Mon, 25 Mar 2013, Felipe Balbi wrote:
> 
> > > @@ -62,22 +64,26 @@ static const struct ehci_driver_overrides 
> > > platform_overrides __initdata = {
> > >   .reset =        ehci_platform_reset,
> > >  };
> > >  
> > > +static struct usb_ehci_pdata ehci_platform_defaults;
> > 
> > this ehci_platform_defaults is quite a hack. Would be much better to see
> > a proper re-factoring of the code so that it actually learns about DT
> > *and* platform_data.
> > 
> > So, if dev->dev.platform_data is NULL, you shouldn't error, rather you
> > should just assume the default, rather than this quick little hack.
> > 
> > Alan has final saying though.
> 
> IMO, using ehci_platform_defaults _is_ a way of assuming the default.  
> In other words, it's not a bad hack.  I'm okay with this this approach
> (in fact, it was my suggestion originally).
> 
> On the other hand, it would be nice to have a clearer way of indicating 
> that the driver was invoked because of a DT match, something better 
> than just noticing that dev->dev.platform_data is NULL.  But I guess 
> this is a legitimate option even for regular platform drivers -- if 

usually we check that dev->of_node is a valid pointer.

> they don't have any specific requirements, they may as well pass a NULL 
> pointer instead of a pointer to an empty structure.

that was my point.

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to