On Fri, Aug 09, 2013 at 08:20:45AM +0200, Sascha Hauer wrote:
> On Thu, Aug 08, 2013 at 06:19:43PM +0800, Peter Chen wrote:
> > > > - compatible = "fsl,imx25-usbmisc";
> > > > - clocks = <&clks 9>, <&clks 70>, <&clks 8>;
> > > > - clock-names = "ipg", "ahb", "per";
> > > > - reg = <0x53ff4600 0x00f>;
> > > > - status = "disabled";
> > > > + noncore: usb-non-core@53ff4600 {
> > > > + compatible = "fsl,imx-usb-non-core", "syscon";
> > > > + reg = <0x53ff4600 0xf>;
> > > > };
> > >
> > > This is bullshit for multiple reasons.
> > >
> > > It's the usbmisc unit that changes between different SoCs, not the
> > > chipidea core. So pretending the usbmisc unit is generic by removing the
> > > SoC specific compatible and instead leaking in the SoC type from the
> > > chipidea device nodes is just crazy.
> > >
> >
> > Please do not call usbmisc as "unit", it should not be a driver, it is
> > a "lib", it is "usbmisc-lib" to implement functions for controller
> > driver.
> >
> > At current design, there are two drivers (ci_hdrc_imx & usbmisc_imx) to
> > control one hardware (one imx usb controller). You will need to control
> > the same clocks at two drivers, it is not a good idea.
>
> It's more meaningful to talk about devices instead of drivers. The USB
> core consists of three (on most i.MX) devices, namely three identical
> chipidea cores. The clocks are shared between all these devices and the
> clock framework handles this just fine due to reference counting.
>
Three devices? What do you mean? host, device, and otg at former fsl
usb driver framework?
You know it is better together the resources, something like register
mapping, clocks, etc. The less place we put clock we the less error
we will meet especially when we introduce runtime power management.
> > Besides, there
> > are dependencies between two drivers, the usbmisc will be unloaded
> > first, how usbmisc can supply the functions after it is unloaded?
> > Remember, it is one hardware, why one driver can forbid another to use
> > controller's function?
> >
> > > What you are doing here is to model the DT after how the Linux driver is
> > > programmed. This really is a bad idea. Remember that the bindings have
> > > to be OS agnostic. They can't be changed just because you want to
> > > resolve your module probe dependencies.
> >
> > I am a beginner for DT, how DT binding can't be changed if the driver
> > has re-designed? If I understand correctly, you say "bindings have to
> > be OS agnostic", you mean the same DT can be used at all late Linux
> > versions after the binding has finished, is it correct?
>
> And not only Linux, but also *BSD and others. Currently I use the same
> bindings for barebox which means I would have to change the bindings
> there aswell once you change them in the Kernel.
>
Understand, just like other ABIs, such as sys ctrl, ioctl, etc.
So the binding review should be careful and no regression.
> >
> > Again, binding is for driver, if it is not suitable as a driver, what
> > the meaningful for the existent of binding?
>
> Maybe I would believe you if you removed the need for a driver, but what
> you do is to replace a 'usbmisc driver' handling a 'usbmisc device' with
> a 'syscon driver' handling a 'syscon device'. The real change here is
> that the implementation of the driver logic is moved from the usbmisc
> driver to the chipidea driver.
>
Yes, since all chipidea devices need to visit the same register mapping.
--
Best Regards,
Peter Chen
--
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
