Re: [patch 2/4] Base support for the Cypress c67x00 multi-role USB controllers

>>>>> "David" == David Brownell <[email protected]> writes:

Hi,

David> At a quick read, this looks like the right kind of direction.

Great!

David> Chips this complex need their own driver subdirectory, they
David> can't go in either host or gadget (they'd need to be both!)  so
David> that result is foredoomed.

Hence the drivers/usb/c67x00.

David> On Thursday 26 April 2007, Peter Korsgaard wrote:
>> Communication with the controller is done through the link control
>> protocol (LCP) over either HPI, SPI or HSS; Sofar only HPI is
>> supported.

David> Comments explaining HPI and HSS would be useful.  Does your
David> implementation strategy defer SPI and HSS till the whole thing
David> works?  It might be nice to flesh out at least one of the two
David> before things get too far along, to help avoid inadvertently
David> hard-wiring bad assumptions.

Sorry, I'll add that when I update the patch.
HPI = Host port interface (16bit parallel SRAM like interface)
HSS = High speed serial interface (serial 9600bps-2Mbps)

SPI and HSS support should in theory only be a question of adding
lowlevel drivers (c67x00-ll-{spi,hss}.c) with the same interface and
select the right interface depending on platform data.

I don't have hardware to verify this though.

David> Your platform_data should for example say how the particular
David> platform works rather than assuming HPI; and there should be
David> some CONFIG_* variable saying whether to link in the HPI glue
David> or some other kind(s).  Default to HPI until some other glue is
David> working, of course.

Ok.

David> Given that any dual-role controller can run in three modes,
David> I'm curious why you don't give config options here.  The three
David> mode being: host-only, peripheral-only, and OTG.

David> That choice would naturally be part of this core patch; the
David> host patch would affect two roles, and the gadget patch would
David> also affect two roles.

Please see patch 3 and 4 in the series for host/peripheral options. I
can move the Kconfig stuff to the base patch if you prefer.

David> Again, see how the musb_hdrc code handles this.  It's a bit
David> messy (Kconfig seems to glitch in some cases), but giving
David> customers the option of how to configure things makes a
David> difference with embedded configs that may not want to have a
David> few hundred KBytes of never-used kernel code...

Exactly. The musb stuff seems to be conceptually similar to what I've
done.

>> +       /* Fill in needed attributes */ +       sie->pdev =
>> platform_device_register_simple("c67x00_sie",
>> +                                                   id++, NULL, 0);

David> This seems wrong.  Not only should drivers not create device
David> nodes themselves, but the only way this could be the right way
David> to fit into the driver model would be if each SIE were its own
David> host (or OTG) bus segment with independent driver.  In your
David> case, one driver would handle each usb_hostN bus.

I know. I'm unfortunately pretty clueless of the finer details of the
driver model.
The chips have 2 (more-or-less) independent SIEs. The idea was to have
seperate hcd/udc's for each SIE, which I guess means seperate struct
devices?

-- 
Bye, Peter Korsgaard

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel