On 09-03-25, 14:29, Christian Marangi wrote:

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