Re: [PATCH v2 2/9] phy: stm32: Add support for ST STM32MP25 USB2-FEMTO PHY
From: Marek Vasut <hidden>
Date: 2026-08-18 11:43:24
Also in:
linux-arm-kernel, linux-phy, linux-usb, lkml
On 8/18/26 11:28 AM, Fabrice Gasnier wrote: Hello Fabrice,
quoted
quoted
quoted
+static int stm32_usb2phy_enable(struct stm32_usb2phy *phy_dev) +{ + const struct stm32mp2_usb2phy_hw_data *phy_data = phy_dev->hw_data; + unsigned long rate; + int refsel, ret;Hello Marek, Just noticed refsel should be unsigned ?
It makes no difference in this case, since the value can be either 0/1/2, but fixed.
quoted
quoted
quoted
+ + /* Check if a phy is already init or clk48 in use */ + if (atomic_inc_return(&phy_dev->en_refcnt) > 1) + return 0; + + rate = clk_get_rate(phy_dev->phyref); + if (rate == 19200000) + refsel = 0; + else if (rate == 20000000) + refsel = 1; + else if (rate == 24000000) + refsel = 2; + else + return -EINVAL;
[...]
quoted
quoted
As you mention the downstream driver, please see there a specific comment regarding the 2nd clock for OHCI: /* * USB2PHY provides several clocks used either by either USHB (EHCI/OHCI), OTG or USB3DR. * In case of OHCI, CMN bit must be cleared (clkohci_hw). This clock is required to access * the registers, to resume the controller from suspended state. * So declare two clocks, the PLL used in all case, and the OHCI clocks used by OHCI * controller. */Is this what you have in mind ?Yes, with one addition, please see next comment
[...]
quoted
static int stm32_usb2phy_probe(struct platform_device *pdev) { - struct clk_init_data init = { .ops = &stm32_usb2phy_clk48_ops }; + struct clk_init_data clk48init = { .ops = &stm32_usb2phy_clk48_ops }; + struct clk_init_data clkcmninit = { .ops = &stm32_usb2phy_clkcmn_ops };clkcmninit should be a child of clk48 which basically represent the PLL (480MHz) as it is still needed as parent. See below. BTW, mainly a nit: could rename clk48 to clkpll and update frequency to 480M.
Fixed in V3.
quoted
+ phy_dev->clk48_hw.init = &clk48init; ret = devm_clk_hw_register(phy_dev->dev, &phy_dev->clk48_hw); if (ret) return dev_err_probe(phy_dev->dev, ret, "Failed to register 48 MHz clock\n"); - ret = devm_of_clk_add_hw_provider(phy_dev->dev, of_clk_hw_simple_get, &phy_dev->clk48_hw); + phy_dev->clkcmn_hw.init = &clkcmninit;Would initialize with (to adapt) : + phy_dev->clkcmn_hw.init = CLK_HW_INIT_HW(name, &phy_dev->clk48_hw, + &stm32_usb2phy_clkcmn_ops, 0); +
That's nice, also added to V3, thanks ! [...]