Re: [PATCH 1/3] hw/arm: Add EHCI/OHCI controllers to Allwinner R40 and Bananapi board

[Guenter Roeck <linux@roeck-us.net>]

On 1/15/24 03:02, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 13/1/24 20:16, Guenter Roeck wrote:
>> Allwinner R40 supports two USB host ports shared between a USB 2.0 EHCI
>> host controller and a USB 1.1 OHCI host controller. Add support for both
>> of them.
>>
>> If machine USB support is not enabled, create unimplemented devices
>> for the USB memory ranges to avoid crashes when booting Linux.
> 
> I never really understood the reason for machine_usb() and had on my
> TODO to do some archeology research to figure it out since quite some
> time. Having to map an UnimpDevice due to CLI options seems like an
> anti-pattern when the device is indeed implemented in the repository.
> 

Me not either. I copied the code from aw_a10_init(), trying to use the
same pattern. I am perfectly fine with making it unconditional, but then
I would argue that it should be unconditional for Allwinner A10 as well
(not that I really care much, just for consistency).

The "-usb" option says "enable on-board USB host controller (if not
enabled by default)". Unfortunately, that doesn't tell me much,
and most specifically it doesn't tell me how to enable it by default.
One option I can think of would be to enable it on the machine level,
i.e., from bananapi_m2u.c, but then, again, I don't see if/how
that is done for other boards. Any suggestions ?

Of course, I could discuss this with the person who implemented this
code for A10, but it turns out that was me, for no good reason than
that I tried to follow the pattern I had seen elsewhere without really
understanding what I was doing.

So should I drop the conditional from H40 and send a separate patch
to drop it from the A10 code as well, following your line of argument ?
Or drop it and leave A10 alone ?

Thanks,
Guenter