Re: [PATCH v2 5/5] arm: dts: imx: Delete usbmisc_imx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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.

> 
> > 
> > Also the existing binding with referencing the usbmisc instance with
> > <&usbmisc number> is good and convenient. Instead depending on the alias
> > number of the chipidea node is quite non-obvious.
> > 
> 
> 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.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux