[PATCH 2/3] phy: stm32-usbphyc: usbphyc is a clock provider of ck_usbo_48m clock

[Patrick DELAUNAY]

Hi Sean

On 5/8/22 20:23, Sean Anderson wrote:
> On 4/26/22 8:37 AM, Patrick Delaunay wrote:
>> ck_usbo_48m is generated by usbphyc PLL and used by OTG controller
>> for Full-Speed use cases with dedicated Full-Speed transceiver.
>>
>> ck_usbo_48m is available as soon as the PLL is enabled.
>>
>> Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> ---
>>
>>   drivers/phy/phy-stm32-usbphyc.c | 79 +++++++++++++++++++++++++++++++++
>>   1 file changed, 79 insertions(+)
>>
>> diff --git a/drivers/phy/phy-stm32-usbphyc.c 
>> b/drivers/phy/phy-stm32-usbphyc.c
>> index 16c8799eca..e0b8fcb8f2 100644
>> --- a/drivers/phy/phy-stm32-usbphyc.c
>> +++ b/drivers/phy/phy-stm32-usbphyc.c
>> @@ -7,6 +7,7 @@
>>     #include <common.h>
>>   #include <clk.h>
>> +#include <clk-uclass.h>
>>   #include <div64.h>
>>   #include <dm.h>
>>   #include <fdtdec.h>
>> @@ -17,6 +18,7 @@
>>   #include <usb.h>
>>   #include <asm/io.h>
>>   #include <dm/device_compat.h>
>> +#include <dm/lists.h>
>>   #include <linux/bitops.h>
>>   #include <linux/delay.h>
>>   #include <power/regulator.h>
>> @@ -49,6 +51,9 @@
>>   #define PLL_INFF_MIN_RATE    19200000 /* in Hz */
>>   #define PLL_INFF_MAX_RATE    38400000 /* in Hz */
>>   +/* USBPHYC_CLK48 */
>> +#define USBPHYC_CLK48_FREQ    48000000 /* in Hz */
>> +
>>   struct pll_params {
>>       u8 ndiv;
>>       u16 frac;
>> @@ -355,6 +360,16 @@ static const struct phy_ops 
>> stm32_usbphyc_phy_ops = {
>>       .of_xlate = stm32_usbphyc_of_xlate,
>>   };
>>   +static int stm32_usbphyc_bind(struct udevice *dev)
>> +{
>> +    int ret;
>> +
>> +    ret = device_bind_driver_to_node(dev, "stm32-usbphyc-clk", 
>> "ck_usbo_48m",
>> +                     dev_ofnode(dev), NULL);
>> +
>> +    return log_ret(ret);
>> +}
>> +
>>   static int stm32_usbphyc_probe(struct udevice *dev)
>>   {
>>       struct stm32_usbphyc *usbphyc = dev_get_priv(dev);
>> @@ -444,6 +459,70 @@ U_BOOT_DRIVER(stm32_usb_phyc) = {
>>       .id = UCLASS_PHY,
>>       .of_match = stm32_usbphyc_of_match,
>>       .ops = &stm32_usbphyc_phy_ops,
>> +    .bind = stm32_usbphyc_bind,
>>       .probe = stm32_usbphyc_probe,
>>       .priv_auto    = sizeof(struct stm32_usbphyc),
>>   };
>> +
>> +struct stm32_usbphyc_clk {
>> +    bool enable;
>> +};
>> +
>> +static ulong stm32_usbphyc_clk48_get_rate(struct clk *clk)
>> +{
>> +    return USBPHYC_CLK48_FREQ;
>
> What is the relationship between this clock and the PLL?


The output clock of USBPHYC, running a 48MHz is only available when the 
USBPHYC is initialized and the PLL is enabled.

=> ck_usbo_48m is available as soon as the PLL is enabled.

see the associated code in kernel:

https://lore.kernel.org/all/20210208114659.15269-1-amelie.delaunay@foss.st.com/T/


>
>> +}
>> +
>> +static int stm32_usbphyc_clk48_enable(struct clk *clk)
>> +{
>> +    struct stm32_usbphyc_clk *usbphyc_clk = dev_get_priv(clk->dev);
>> +    struct stm32_usbphyc *usbphyc;
>> +    int ret;
>> +
>> +    if (usbphyc_clk->enable)
>> +        return 0;
>> +
>> +    usbphyc = dev_get_priv(clk->dev->parent);
>> +
>> +    /* ck_usbo_48m is generated by usbphyc PLL */
>> +    ret = stm32_usbphyc_pll_enable(usbphyc);
>> +    if (ret)
>> +        return ret;
>> +
>> +    usbphyc_clk->enable = true;
>> +
>> +    return 0;
>> +}
>> +
>> +static int stm32_usbphyc_clk48_disable(struct clk *clk)
>> +{ usbphyc provides a unique clock called ck_usbo_48m.
>> STM32 USB OTG needs a 48Mhz clock (utmifs_clk48) for Full-Speed 
>> operation.
>> ck_usbo_48m is a possible parent clock for USB OTG 48Mhz clock.
>> +    struct stm32_usbphyc_clk *usbphyc_clk = dev_get_priv(clk->dev);
>> +    struct stm32_usbphyc *usbphyc;
>> +    int ret;
>> +
>> +    if (!usbphyc_clk->enable)
>> +        return 0;
>> +
>> +    usbphyc = dev_get_priv(clk->dev->parent);
>> +
>> +    ret = stm32_usbphyc_pll_disable(usbphyc);
>> +    if (ret)
>> +        return ret;
>> +
>> +    usbphyc_clk->enable = false;
>> +
>> +    return 0;
>> +}
>> +
>> +const struct clk_ops usbphyc_clk48_ops = {
>> +    .get_rate = stm32_usbphyc_clk48_get_rate,
>> +    .enable = stm32_usbphyc_clk48_enable,
>> +    .disable = stm32_usbphyc_clk48_disable,
>> +};
>> +
>> +U_BOOT_DRIVER(stm32_usb_phyc_clk) = {
>> +    .name = "stm32-usbphyc-clk",
>> +    .id = UCLASS_CLK,
>> +    .ops = &usbphyc_clk48_ops,
>> +    .priv_auto = sizeof(struct stm32_usbphyc_clk),
>> +};
>>
>
> I see that in the next patch you call this device from 
> drivers/clk/clk_stm32mp1.c
>
> Why is this clock a separate driver from the clock driver?


The clock driver STM32MP1 = drivers/clk/clk_stm32mp1.c manage the clock in

the hardware IP RCC include in STM32MPU

=> 4 PLL and several oscillators LSI / HSI / CSI / HSE / LSE

=> manage clock source for many peripheral clock in STM32 MPU


The USBPHYC drivers manages the USB PHYC hardware block, it manage a PHY for

USB operation (device or host).

This hardware block have a internal PLL for its operation which is managed

directly by USBPHYC registers:

   usbphyc provides a unique clock called ck_usbo_48m.
   STM32 USB OTG needs a 48Mhz clock (utmifs_clk48) for Full-Speed operation.
   ck_usbo_48m is a possible parent clock for USB OTG 48Mhz clock.


This PLL is required for internal operation of USBPHYC (this PLL need to 
be enable

to allow USB detection for example) but also as input clock for RCC 
peripheral clock

USBO is "rcc_ck_usbo_48m" (see RCC_USBCKSELR.USBOSRC : USB OTG kernel clock

source selection)


So in RCC driver, we need to get the rate (it is trivial = 48MHz)

and enable the PLL when the peripheral clock USBO is required.


For this last point I prefer export a generic clock provider = CLK 
UCLASS than

export a specific usbphy function or duplicate USBPHYC code in RCC CLK 
driver.

even if the enable request on "rcc_ck_usbo_48m" is not managed in STM32MP15x

RCC driver...(stm32mp1_clk_enable don't enable the parent clock)


This patch prepares the STM32MP13x RCC driver introduction (coming soon),

based on CCF (common clock framework, ported n U-Boot):

each clock on the system is a CLK UCLASS and the dependencies are correctly

handle (each parent clock is enable when a clock is enable) but without

a CLK UCLASS for each USBO parent, a error occur during STM32MP13x CLK

driver probe (it is linked to drivers/clk/clk.c::clk_register(), with 
unkwon parent).


I hope that it is more clear.


>
> --Sean


Patrick