lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z9AhN9T8s1oogCUn@vaman>
Date: Tue, 11 Mar 2025 12:40:39 +0100
From: Vinod Koul <vkoul@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Daniel Danzberger <dd@...edd.com>, Arnd Bergmann <arnd@...db.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	Nikita Shubin <nikita.shubin@...uefel.me>,
	Guo Ren <guoren@...nel.org>, Yangyu Chen <cyy@...self.name>,
	Ben Hutchings <ben@...adent.org.uk>, Felix Fietkau <nbd@....name>,
	linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-phy@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
	linux-usb@...r.kernel.org, upstream@...oha.com
Subject: Re: [PATCH 10/13] phy: airoha: Add support for Airoha AN7581 USB PHY

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@...il.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@...il.com>
>  L:	linux-arm-kernel@...ts.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@...r.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ