Re: [PATCH v6 07/12] usb: chipidea: add a usb2 driver for ci13xxx

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Wed, Sep 24, 2014 at 02:23:38PM +0200, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 19:29:05 Peter Chen wrote:
> > 
> > So, it is IP CORE LIB (you suggest) vs IP CORE Platform Driver
> > (dwc3, musb, chipidea) you are talking about, right? Except for
> > creating another platform driver as well as related DT node (optional),
> > are there any advantages compared to current IP core driver structure?
> 
> Having a library module usually requires less code, and is more
> consistent with other drivers, which helps new developers understand
> what the driver is doing.

I beg to differ. You end-up having to pass function pointers through
platform_data to handle differences which could be handled by the core
IP.

> The most important aspect though is the DT binding: once the structure
> with separate kernel drivers leaks out into the DT format, you can't
> easily change the driver any more, e.g. to make a property that was
> introduced for one glue driver more general so it can get handled by
> the common part. Having a single node lets us convert to the library
> model later, so that would be a strong reason to keep the DT binding
> simple, without child nodes.
> 
> Following that logic, it turns into another reason for using the library
> model to start with, because otherwise the child device does not have
> any DT properties itself and has to rely on the parent device properties,
> which somewhat complicates the driver structure.

this is bullcrap. Take a look at
Documentation/devicetree/bindings/usb/dwc3.txt:

synopsys DWC3 CORE

DWC3- USB3 CONTROLLER

Required properties:
 - compatible: must be "snps,dwc3"
 - reg : Address and length of the register set for the device
 - interrupts: Interrupts used by the dwc3 controller.

Optional properties:
 - usb-phy : array of phandle for the PHY device.  The first element
   in the array is expected to be a handle to the USB2/HS PHY and
   the second element is expected to be a handle to the USB3/SS PHY
 - phys: from the *Generic PHY* bindings
 - phy-names: from the *Generic PHY* bindings
 - tx-fifo-resize: determines if the FIFO *has* to be reallocated.

This is usually a subnode to DWC3 glue to which it is connected.

dwc3@4a030000 {
	compatible = "snps,dwc3";
	reg = <0x4a030000 0xcfff>;
	interrupts = <0 92 4>
	usb-phy = <&usb2_phy>, <&usb3,phy>;
	tx-fifo-resize;
};

This contains all the attributes it needs to work. In fact, this could
be used without any glue.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]