| 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
| ||
|
Message-ID: <d743de28a5dd643cf775f4d8a3baaabd09d48c48.camel@pengutronix.de>
Date: Fri, 06 Sep 2024 15:44:01 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Christian Bruel <christian.bruel@...s.st.com>, vkoul@...nel.org,
kishon@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, mcoquelin.stm32@...il.com,
alexandre.torgue@...s.st.com
Cc: linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
fabrice.gasnier@...s.st.com
Subject: Re: [PATCH v5 2/5] phy: stm32: Add support for STM32MP25 COMBOPHY.
Hi,
On Di, 2024-09-03 at 14:13 +0200, Christian Bruel wrote:
> Addition of the COMBOPHY driver found on STM32MP25 platforms
>
> This single lane PHY is shared (exclusive) between the USB3 and PCIE
> controllers.
> Supports 5Gbit/s for PCIE gen2 or 2.5Gbit/s for PCIE gen1.
>
> Supports wakeup-source capability to wakeup system using remote-wakeup
> capable USB device
>
> Signed-off-by: Christian Bruel <christian.bruel@...s.st.com>
> ---
> drivers/phy/st/Kconfig | 11 +
> drivers/phy/st/Makefile | 1 +
> drivers/phy/st/phy-stm32-combophy.c | 590 ++++++++++++++++++++++++++++
> 3 files changed, 602 insertions(+)
> create mode 100644 drivers/phy/st/phy-stm32-combophy.c
>
[...]
> diff --git a/drivers/phy/st/phy-stm32-combophy.c b/drivers/phy/st/phy-stm32-combophy.c
> new file mode 100644
> index 000000000000..305ba124d092
> --- /dev/null
> +++ b/drivers/phy/st/phy-stm32-combophy.c
> @@ -0,0 +1,590 @@
[...]
> +static int stm32_combophy_pll_init(struct stm32_combophy *combophy)
> +{
> + int ret;
> + u32 refclksel, pllmult, propcntrl, val;
> + u32 clk_rate;
> + struct clk *clk;
> +
> + if (combophy->have_pad_clk)
> + clk = combophy->clks[PAD_CLK].clk;
> + else
> + clk = combophy->clks[KER_CLK].clk;
> +
> + clk_rate = clk_get_rate(clk);
> +
> + reset_control_assert(combophy->phy_reset);
> +
> + dev_dbg(combophy->dev, "%s pll init rate %d\n",
> + combophy->have_pad_clk ? "External" : "Ker", clk_rate);
> +
> + /*
> + * vddcombophy is interconnected with vddcore. Isolation bit should be unset
> + * before using the ComboPHY.
> + */
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR2,
> + SYSCFG_COMBOPHY_CR2_ISO_DIS, SYSCFG_COMBOPHY_CR2_ISO_DIS);
> +
> + if (combophy->type != PHY_TYPE_PCIE)
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_REFSSPEN, SYSCFG_COMBOPHY_CR1_REFSSPEN);
Could the multiple accesses to SYSCFG_COMBOPHY_CR1 be consolidated into
a single regmap_update_bits()?
> +
> + if (combophy->type == PHY_TYPE_PCIE && !combophy->have_pad_clk)
> + regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> + STM32MP25_PCIEPRGCR_EN, STM32MP25_PCIEPRGCR_EN);
> +
> + if (of_property_present(combophy->dev->of_node, "st,ssc-on")) {
> + dev_dbg(combophy->dev, "Enabling clock with SSC\n");
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_SSCEN, SYSCFG_COMBOPHY_CR1_SSCEN);
> + }
> +
> + if (!of_property_read_u32(combophy->dev->of_node, "st,rx-equalizer", &val)) {
> + dev_dbg(combophy->dev, "Set RX equalizer %u\n", val);
> + if (val > SYSCFG_COMBOPHY_CR4_RX0_EQ) {
> + dev_err(combophy->dev, "Invalid value %u for rx0 equalizer\n", val);
This path looks like it should deassert the phy_reset as well.
> + return -EINVAL;
> + }
> +
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR4,
> + SYSCFG_COMBOPHY_CR4_RX0_EQ, val);
> + }
> +
> + if (combophy->type == PHY_TYPE_PCIE) {
> + ret = stm32_impedance_tune(combophy);
> + if (ret) {
> + reset_control_deassert(combophy->phy_reset);
> + goto out;
> + }
> +
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_REFUSEPAD,
> + combophy->have_pad_clk ? SYSCFG_COMBOPHY_CR1_REFUSEPAD : 0);
> + }
> +
> + switch (clk_rate) {
> + case 100000000:
> + pllmult = MPLLMULT_100;
> + refclksel = REFCLKSEL_0;
> + propcntrl = 0x8u << 4;
> + break;
> + case 19200000:
> + pllmult = MPLLMULT_19_2;
> + refclksel = REFCLKSEL_1;
> + propcntrl = 0x8u << 4;
> + break;
> + case 25000000:
> + pllmult = MPLLMULT_25;
> + refclksel = REFCLKSEL_0;
> + propcntrl = 0xeu << 4;
> + break;
> + case 24000000:
> + pllmult = MPLLMULT_24;
> + refclksel = REFCLKSEL_1;
> + propcntrl = 0xeu << 4;
> + break;
> + case 20000000:
> + pllmult = MPLLMULT_20;
> + refclksel = REFCLKSEL_0;
> + propcntrl = 0xeu << 4;
> + break;
> + default:
> + dev_err(combophy->dev, "Invalid rate 0x%x\n", clk_rate);
> + reset_control_deassert(combophy->phy_reset);
> + ret = -EINVAL;
> + goto out;
> + };
> +
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_REFCLKDIV2, REFCLDIV_0);
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_REFCLKSEL, refclksel);
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_MPLLMULT, pllmult);
> +
> + /*
> + * Force elasticity buffer to be tuned for the reference clock as
> + * the separated clock model is not supported
> + */
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR5,
> + SYSCFG_COMBOPHY_CR5_COMMON_CLOCKS, SYSCFG_COMBOPHY_CR5_COMMON_CLOCKS);
> +
> + reset_control_deassert(combophy->phy_reset);
> +
> + ret = regmap_read_poll_timeout(combophy->regmap, SYSCFG_COMBOPHY_SR, val,
> + !(val & STM32MP25_PIPE0_PHYSTATUS),
> + 10, 1000);
> + if (ret) {
> + dev_err(combophy->dev, "timeout, cannot lock PLL\n");
> + goto out;
> + }
> +
> + if (combophy->type == PHY_TYPE_PCIE) {
> + val = readl_relaxed(combophy->base + COMBOPHY_SUP_ANA_MPLL_LOOP_CTL);
> + val &= ~COMBOPHY_PROP_CNTRL;
> + val |= propcntrl;
> + writel_relaxed(val, combophy->base + COMBOPHY_SUP_ANA_MPLL_LOOP_CTL);
> + }
> +
> + return 0;
> +
> +out:
> + if (combophy->type == PHY_TYPE_PCIE && !combophy->have_pad_clk)
> + regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> + STM32MP25_PCIEPRGCR_EN, 0);
> +
> + if (combophy->type != PHY_TYPE_PCIE)
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR1,
> + SYSCFG_COMBOPHY_CR1_REFSSPEN, 0);
> +
> + regmap_update_bits(combophy->regmap, SYSCFG_COMBOPHY_CR2,
> + SYSCFG_COMBOPHY_CR2_ISO_DIS, 0);
> +
> + return ret;
> +}
> +
[...]
> +
> +static int stm32_combophy_probe(struct platform_device *pdev)
> +{
> + struct stm32_combophy *combophy;
> + struct device *dev = &pdev->dev;
> + struct phy_provider *phy_provider;
> + int ret, irq;
> +
> + combophy = devm_kzalloc(dev, sizeof(*combophy), GFP_KERNEL);
> + if (!combophy)
> + return -ENOMEM;
> +
> + combophy->dev = dev;
> +
> + dev_set_drvdata(dev, combophy);
> +
> + combophy->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(combophy->base))
> + return PTR_ERR(combophy->base);
> +
> + if (stm32_combophy_get_clocks(combophy))
> + return ret;
> +
> + combophy->phy_reset = devm_reset_control_get(dev, "phy");
Please use devm_reset_control_get_exclusive() directly.
regards
Philipp
Powered by blists - more mailing lists