On Thu, Jul 2, 2020 at 8:08 PM Robin Murphy <robin.murphy@xxxxxxx> wrote: > > On 2020-07-02 10:05, Jagan Teki wrote: > > rk3288 has usb host0 ohci controller but doesn't actually work > > on real hardware but it works with new revision chip rk3288w. > > > > So, disable ohci for rk3288. > > > > For rk3288w chips the compatible update code is handled by bootloader. > > > > Cc: William Wu <william.wu@xxxxxxxxxxxxxx> > > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> > > --- > > Note: > > - U-Boot patch for compatible update > > https://patchwork.ozlabs.org/project/uboot/patch/20200702084820.35942-1-jagan@xxxxxxxxxxxxxxxxxxxx/ > > > > drivers/usb/host/ohci-platform.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c > > index 7addfc2cbadc..24655ed6a7e0 100644 > > --- a/drivers/usb/host/ohci-platform.c > > +++ b/drivers/usb/host/ohci-platform.c > > @@ -96,7 +96,7 @@ static int ohci_platform_probe(struct platform_device *dev) > > struct ohci_hcd *ohci; > > int err, irq, clk = 0; > > > > - if (usb_disabled()) > > + if (usb_disabled() || of_machine_is_compatible("rockchip,rk3288")) > > This seems unnecessary to me - if we've even started probing a driver > for a broken piece of hardware to the point that we need magic checks to > bail out again, then something is already fundamentally wrong. > > Old boards only sold with the original SoC variant have no reason to > enable the OHCI (since it never worked originally), thus will never > execute this check. > > New boards designed around the W variant to make use of the OHCI can > freely enable it either way. > > The only relative-edge-case where it might matter is older board designs > still in production which have shipped with both SoC variants. Enabling > OHCI can't be *necessary* given that it's still broken on a lot of > deployed boards, so at best it must be an opportunistic nice-to-have. > Since we're already having to rely on the bootloader to patch up the > devicetree for other low-level differences in this case, it should be > part of that responsibility for it to only enable the OHCI on the > appropriate SoC variant too. Statically enabling it in the DTS for a > board where it may well not work is just bad. You mean enable OHCI by identifying revision W with dts status "okay"? doesn't it complex for the bootloader to update all effecting changes? Jagan.
