On Thu, Jun 01, 2023 at 02:24:28AM +0000, Stanley Chang[昌育德] wrote:

Hi Conor,

quoted

quoted

quoted

You have device-specific compatibles, which is great, but you also allow
only those two generic ones. I had a _brief_ look at the driver, and it
seems like there is no decision making done based on the compatibles,
only on the properties. Is that correct?
If it is, I would understand having "realtek,usb3phy" as a fallback
compatible for "realtek,rtd1619-usb3phy", but I do not get the current
setup.

This driver is compatible with all Realtek RTD SoCs without specifying different settings.
So use "realtek,usb3phy" as fallback compatible for all SoCs.
This is the compatible name we use.
Other compatible names simply indicate that the driver supports the SoCs.

Then you should write the binding such that having fallback compatibles
is permitted. Try plugging
compatible = "realtek,rtd1295-usb2phy", "realtek,rtd-usb2phy", "realtek,usb2phy";
into your example below and see what happens.

quoted

The name "usbNphy" and "rtd-usbNphy" seem to be more generic for all RTD SoCs,
but they are not device-specific compatible.
Do you have a better suggestion?

Write the binding so that having fallback compatibles in the DT actually
works, don't add the SoC-specific ones merely as indicators that those
SoCs are supported and don't permit "realtek,usbNphy" or
"realtek,rtd-usbNphy" in isolation ;)

As far as I understand what you mean.
I should follow other docs to define compatible.
Reference:
  Documentation/devicetree/bindings/phy/mediatek,xsphy.yaml
  drivers/phy/mediatek/phy-mtk-xsphy.c

For example:

  compatible:
    items:
      - enum:
          - realtek,rtd1295-usb2phy
          - realtek,rtd1395-usb2phy
          - realtek,rtd1619-usb2phy
          - realtek,rtd1319-usb2phy
          - realtek,rtd1619b-usb2phy
          - realtek,rtd1312c-usb2phy
          - realtek,rtd1319d-usb2phy
          - realtek,rtd1315e-usb2phy
      - const: realtek,usb2phy

examples:
  -
    dwc3_u3drd_usb2phy: dwc3_u3drd_usb2phy@98013e14 {
        compatible = "realtek,rtd1319-usb2phy", "realtek,usb2phy";
  
And use only "Realtek, usb2phy" in the driver.
static const struct of_device_id usbphy_rtk_dt_match[] = {
        { .compatible = "realtek,usb2phy", },
        {},
};

Yes, this would be a vast improvement, thanks.