Re: [PATCH v2] USB: ohci-nxp: Fix error handling in ohci-hcd-nxp driver

[Vladimir Zapolskiy]

On 11/17/25 16:56, Alan Stern wrote:
> On Mon, Nov 17, 2025 at 09:53:21AM +0100, Arnd Bergmann wrote:
> > On Mon, Nov 17, 2025, at 02:34, Ma Ke wrote:
> > > When obtaining the ISP1301 I2C client through the device tree, the
> > > driver does not release the device reference in the probe failure path
> > > or in the remove function. This could cause a reference count leak,
> > > which may prevent the device from being properly unbound or freed,
> > > leading to resource leakage.
> > >
> > > Fix this by storing whether the client was obtained via device tree
> > > and only releasing the reference in that case.
> > >
> > > Found by code review.
> > >
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Fixes: 73108aa90cbf ("USB: ohci-nxp: Use isp1301 driver")
> > > Signed-off-by: Ma Ke <make24@xxxxxxxxxxx>
> >
> > The patch looks fine in principle, however I don't see any way
> > this driver would be probed without devicetree, and I think
> > it would be better to remove all the traces of the pre-DT
> > logic in it.
> >
> > The lpc32xx platform was converted to DT back in 2012, so
> > any reference to the old variant is dead code. Something like
> > the patch below should work here.
> >
> > Other thoughts on this driver, though I I'm not sure anyone
> > is going to have the energy to implement these:
> >
> >   - the reference to isp1301_i2c_client should be kept in
> >     the hcd private data, after allocating a structure, by
> >     setting driver->hcd_priv_size.
> >   - instead of looking for the i2c device, I would suppose
> >     it should look for a usb_phy instead, as there is no
> >     guarantee on the initialization being ordered at the
> >     moment.
> >   - instead of a usb_phy, the driver should probably use
> >     a generic phy (a much larger rework).

Since I'm one of the remaining users and holders of the LPC32xx powered
boards, I should take this task.

> Considering what the comments at the start of the file say:
>
>   * Currently supported OHCI host devices:
>   * - NXP LPC32xx
>
>   * NOTE: This driver does not have suspend/resume functionality
>   * This driver is intended for engineering development purposes only
>
> I wonder whether any existing systems actually use this driver.

The LPC32xx OHCI host device works fine with the driver, noteworthy
there were some issues with the LPC32xx UDC though.

Any pre-dt leftovers should be removed from the driver, as Arnd suggested.

--
Best wishes,
Vladimir