Re: [PATCH v3 2/2] usb: ehci: Add Aspeed AST2700 support

Alan Stern
2025-09-29T14:09:27Z

On Mon, Sep 29, 2025 at 05:56:13AM +0000, Ryan Chen wrote:
> > > @@ -253,8 +256,13 @@ static int ehci_platform_probe(struct
> > platform_device *dev)
> > > if (!pdata)
> > > pdata = &ehci_platform_defaults;
> > >
> > > + dma_mask_64 = pdata->dma_mask_64;
> > > + match = of_match_device(dev->dev.driver->of_match_table, &dev->dev);
> >
> > (I just noticed this.) The "dev->dev.driver->of_match_table" part looks odd.
> > Why not just write "vt8500_ehci_ids"? Do you expect that this could ever
> > have a different value?
> >
> > Alan Stern
> Thanks your feedback.
> I used dev->dev.driver->of_match_table rather than hard-coding vt8500_ehci_ids
> to keep the probe code generic and tied to the driver model, not to a specific symbol.
> Functionally it's the same here, but this pattern avoids coupling the probe to a
> particular table name.
>
> How you think ?

The code doesn't need to be any more generic than the source file it
containing it. This particular probe function will never be called for
a different driver, or a device that matches an OF table different from
vt8500_ehci_ids, right?

Interestingly, there are two other places in drivers/usb/host/ that call
of_match_device(). fsl-mph-dr-of.c uses my approach and xhci-plat.c
uses yours. The difference is that the xhci-platform probe routine
_is_ called by other drivers, namely, by xhci-rcar.c.

I guess it's okay to keep this code as written, even though it's not
strictly necessary

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern