Re: [PATCH 10/13] phy: airoha: Add support for Airoha AN7581 USB PHY

Vinod Koul
2025-03-11T11:40:44Z

On 09-03-25, 14:29, Christian Marangi wrote:
> Add support for Airoha AN7581 USB PHY driver. AN7581 supports up to 2
> USB port with USB 2.0 mode always supported and USB 3.0 mode available
> only if the Serdes port is correctly configured for USB 3.0.
>
> On xLate probe, the Serdes mode is validated and the driver return error
> if the Serdes mode doesn't reflect the expected mode. This is required
> as Serdes mode are controlled by the SCU SSR bits and can be either USB
> 3.0 mode or HSGMII or PCIe 2. In such case USB 3.0 won't work.
>
> If the USB 3.0 mode is not supported, the modes needs to be also
> disabled in the xHCI node or the driver will report unsable clock and
> fail probe.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> MAINTAINERS | 1 +
> drivers/phy/Kconfig | 1 +
> drivers/phy/Makefile | 3 +-
> drivers/phy/airoha/Kconfig | 13 +
> drivers/phy/airoha/Makefile | 3 +
> drivers/phy/airoha/phy-airoha-usb.c | 554 ++++++++++++++++++++++++++++
> 6 files changed, 574 insertions(+), 1 deletion(-)
> create mode 100644 drivers/phy/airoha/Kconfig
> create mode 100644 drivers/phy/airoha/Makefile
> create mode 100644 drivers/phy/airoha/phy-airoha-usb.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c2dd385e9165..1835e488ccaa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -758,6 +758,7 @@ M: Christian Marangi <ansuelsmth@gmail.com>
> L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> S: Maintained
> F: Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yam
> +F: drivers/phy/airoha/phy-airoha-usb.c
>
> AIRSPY MEDIA DRIVER
> L: linux-media@vger.kernel.org
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 8d58efe998ec..19c9c518fc3d 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -93,6 +93,7 @@ config PHY_NXP_PTN3222
> schemes. It supports all three USB 2.0 data rates: Low Speed, Full
> Speed and High Speed.
>
> +source "drivers/phy/airoha/Kconfig"
> source "drivers/phy/allwinner/Kconfig"
> source "drivers/phy/amlogic/Kconfig"
> source "drivers/phy/broadcom/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index e281442acc75..71708c6865b8 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -12,7 +12,8 @@ obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
> obj-$(CONFIG_USB_LGM_PHY) += phy-lgm-usb.o
> obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
> obj-$(CONFIG_PHY_NXP_PTN3222) += phy-nxp-ptn3222.o
> -obj-y += allwinner/ \
> +obj-y += airoha/ \

Directory for a single driver? Also move the PCIe driver in, if you
really want to do this

> +struct airoha_usb_phy_priv {
> + struct device *dev;
> +
> + struct regmap *regmap;
> +
> + unsigned int id;
> +
> + struct airoha_usb_phy_instance *phys[AIROHA_USB_PHY_MAX_INSTANCE];

No need for empty lines here?

> +static int airoha_usb_phy_u2_slew_rate_calibration(struct airoha_usb_phy_priv *priv)
> +{
> + u32 fm_out;
> + u32 srctrl;
> +
> + /* Enable HS TX SR calibration */
> + regmap_set_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR5,
> + AIROHA_USB_PHY_USB20_HSTX_SRCAL_EN);
> +
> + usleep_range(1000, 1500);
> +
> + /* Enable Free run clock */
> + regmap_set_bits(priv->regmap, AIROHA_USB_PHY_FMMONR1,
> + AIROHA_USB_PHY_FRCK_EN);
> +
> + /* Select Monitor Clock */
> + regmap_update_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
> + AIROHA_USB_PHY_MONCLK_SEL,
> + priv->id == 0 ? AIROHA_USB_PHY_MONCLK_SEL0 :
> + AIROHA_USB_PHY_MONCLK_SEL1);
> +
> + /* Set cyclecnt */
> + regmap_update_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
> + AIROHA_USB_PHY_CYCLECNT,
> + FIELD_PREP(AIROHA_USB_PHY_CYCLECNT,
> + AIROHA_USB_PHY_U2_FM_DET_CYCLE_CNT));
> +
> + /* Enable Frequency meter */
> + regmap_set_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
> + AIROHA_USB_PHY_FREQDET_EN);
> +
> + /* Timeout can happen and we will apply workaround at the end */
> + regmap_read_poll_timeout(priv->regmap, AIROHA_USB_PHY_FMMONR0, fm_out,
> + fm_out, AIROHA_USB_PHY_FREQDET_SLEEP,
> + AIROHA_USB_PHY_FREQDET_TIMEOUT);
> +
> + /* Disable Frequency meter */
> + regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_FMCR0,
> + AIROHA_USB_PHY_FREQDET_EN);
> +
> + /* Disable Free run clock */
> + regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_FMMONR1,
> + AIROHA_USB_PHY_FRCK_EN);
> +
> + /* Disable HS TX SR calibration */
> + regmap_clear_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR5,
> + AIROHA_USB_PHY_USB20_HSTX_SRCAL_EN);
> +
> + usleep_range(1000, 1500);
> +
> + /* Frequency was not detected, use default SR calibration value */
> + if (!fm_out) {
> + srctrl = 0x5;

magic?

> + dev_err(priv->dev, "Frequency not detected, using default SR calibration.\n");
> + /* (1024 / FM_OUT) * REF_CK * U2_SR_COEF (round to the nearest digits) */
> + } else {
> + srctrl = AIROHA_USB_PHY_REF_CK * AIROHA_USB_PHY_U2_SR_COEF;
> + srctrl = (srctrl * AIROHA_USB_PHY_U2_FM_DET_CYCLE_CNT) / fm_out;
> + srctrl = DIV_ROUND_CLOSEST(srctrl, AIROHA_USB_PHY_U2_SR_COEF_DIVISOR);
> + dev_dbg(priv->dev, "SR calibration applied: %x\n", srctrl);
> + }
> +
> + regmap_update_bits(priv->regmap, AIROHA_USB_PHY_USBPHYACR5,
> + AIROHA_USB_PHY_USB20_HSTX_SRCTRL,
> + FIELD_PREP(AIROHA_USB_PHY_USB20_HSTX_SRCTRL, srctrl));
> +
> + return 0;

This is the only return... consider making it void return then?
Here and other places

> +static int airoha_usb_phy_exit(struct phy *phy)
> +{
> + return 0;
> +}

you can drop this

> +static int airoha_usb_phy_power_on(struct phy *phy)
> +{
> + struct airoha_usb_phy_instance *instance = phy_get_drvdata(phy);
> + struct airoha_usb_phy_priv *priv = dev_get_drvdata(phy->dev.parent);
> +
> + if (instance->type == PHY_TYPE_USB2)
> + return airoha_usb_phy_u2_power_on(priv);
> +
> + return airoha_usb_phy_u3_power_on(priv);

for non USB2, you power both why?

> +static int airoha_usb_phy_u2_set_mode(struct airoha_usb_phy_priv *priv,
> + enum phy_mode mode)
> +{
> + u32 val = 0;
> +
> + /*
> + * For Device and Host mode, enable force IDDIG.
> + * For Device set IDDIG, for Host clear IDDIG.
> + * For OTG disable force and clear IDDIG bit while at it.
> + */
> + switch (mode) {
> + case PHY_MODE_USB_DEVICE:
> + val |= AIROHA_USB_PHY_IDDIG;
> + fallthrough;
> + case PHY_MODE_USB_HOST:
> + val |= AIROHA_USB_PHY_FORCE_IDDIG;
> + break;
> + case PHY_MODE_USB_OTG:
> + break;

Not expecting to set this?
--
~Vinod