lkml.org 
[lkml]   [2014]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/3] usb: host: st-hcd: Add USB HCD support for STi SoCs
Hi Arnd,

Thanks for reviewing, see my comments below: -

> Unfortunately, this seems to be done in a rather strange way,
> I suspect you'll have to start over, but I'll let Alan and Greg
> weigh in.
>
> > +
> > +struct st_hcd_dev {
> > + int port_nr;
> > + struct platform_device *ehci_device;
> > + struct platform_device *ohci_device;
> > + struct clk *ic_clk;
> > + struct clk *ohci_clk;
> > + struct reset_control *pwr;
> > + struct reset_control *rst;
> > + struct phy *phy;
> > +};
>
> The way you do this apparently is to create a device that encapsulates
> the OHCI and the EHCI and then goes on to create child devices that
> are bound to the real drivers.

Yes, although this isn't the first driver to take that approach USB_HCD_BCMA
(bcma-hcd.c) and USB_HCD_SSB (ssb-hcd.c) do much the same thing.

>
> The way it should be done however is to put the two host controllers
> into DT directly and describe their resources (phy, clock, reset, ...)
> using the DT bindings for those.

I'm of course happy to change it if required. I see looking through that a lot
of other platforms do it the way you describe with a

ehci-<platname>.c and ohci-<platname>.c
>
> Depending on what kind of special requirements the ST version has,
> this can be done either by using the generic ohci/ehci bindings
> with extensions where necessary, or by creating a new binding and
> new driver files that use 'ohci_init_driver'/'ehci_init_driver'
> to inherit from the common code.
>
> The first of the two approaches is preferred.

We don't have anything particularly special, just a couple of reset lines,
clock, phy, etc.
>
> > + pdev->dev.parent = &parent->dev;
> > + pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> > + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>
> This is something we shouldn't ever do these days, the DMA settings
> should come directly from DT without driver interaction.

Ok, I'll wait to hear the outcome of Greg/Alans views before either fixing
it or starting over.

regards,

Peter.


\
 
 \ /
  Last update: 2014-07-24 15:01    [W:0.068 / U:12.408 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site