Re: [U-Boot] [RESEND][PATCH v1] phy: add support for STM32 usb phy controller

[Marek Vasut]

On 04/27/2018 09:50 AM, Patrice CHOTARD wrote:
> Hi Marek
> 
> On 04/26/2018 05:13 PM, Marek Vasut wrote:
>> On 04/26/2018 04:23 PM, Patrice Chotard wrote:
>>> From: Christophe Kerello 
>>>
>>> This patch adds phy tranceiver driver for STM32 USB PHY
>>> Controller (usbphyc) that provides dual port High-Speed
>>> phy for OTG (single port) and EHCI/OHCI host controller
>>> (two ports).
>>> One port of the phy is shared between the two USB controllers
>>> through a UTMI+ switch.
>>>
>>> Signed-off-by: Christophe Kerello 
>>> Signed-off-by: Patrice Chotard 
>>
>> [...]
>>
>>> +struct pll_params {
>>> +   u8 ndiv;
>>> +   u16 frac;
>>> +};
>>> +
>>> +struct stm32_usbphyc {
>>> +   fdt_addr_t base;
>>> +   struct clk clk;
>>> +   struct stm32_usbphyc_phy {
>>> +   struct udevice *vdd;
>>> +   struct udevice *vdda1v1;
>>> +   struct udevice *vdda1v8;
>>> +   int index;
>>> +   bool init;
>>> +   bool powered;
>>> +   } phys[MAX_PHYS];
>>
>> Shouldn't there be one driver instance per PHY ?
> 
> This driver manages a PHY provider + PHY child sub nodes as requested by 
> Kernel maintainer.
> 
> more details here : https://lkml.org/lkml/2018/3/2/670

What am I looking for ?

> If you want i can add more details in v2 by including the DT bindings 
> documentation ?

Yes, please do, it's quite inobvious.

>>
>>> +};
>>> +
>>> +void stm32_usbphyc_get_pll_params(u32 clk_rate, struct pll_params 
>>> *pll_params)
>>> +{
>>> +   unsigned long long fvco, ndiv, frac;
>>> +
>>> +   /*
>>> +*| FVCO = INFF*2*(NDIV + FRACT/2^16 ) when DITHER_DISABLE[1] = 1
>>> +*| FVCO = 2880MHz
>>> +*| NDIV = integer part of input bits to set the LDF
>>> +*| FRACT = fractional part of input bits to set the LDF
>>> +*  =>  PLLNDIV = integer part of (FVCO / (INFF*2))
>>> +*  =>  PLLFRACIN = fractional part of(FVCO / INFF*2) * 2^16
>>> +* <=>  PLLFRACIN = ((FVCO / (INFF*2)) - PLLNDIV) * 2^16
>>> +*/
>>> +   fvco = (unsigned long long)PLL_FVCO * 100; /* In Hz */
>>> +
>>> +   ndiv = fvco;
>>> +   do_div(ndiv, (clk_rate * 2));
>>> +   pll_params->ndiv = (u8)ndiv;
>>> +
>>> +   frac = fvco * (1 << 16);
>>> +   do_div(frac, (clk_rate * 2));
>>> +   frac = frac - (ndiv * (1 << 16));
>>> +   pll_params->frac = (u16)frac;
>>> +}
>>> +
>>> +static int stm32_usbphyc_pll_init(struct stm32_usbphyc *usbphyc)
>>> +{
>>> +   struct pll_params pll_params;
>>> +   u32 clk_rate = clk_get_rate(>clk);
>>> +   u32 usbphyc_pll;
>>> +
>>> +   if ((clk_rate < PLL_INFF_MIN_RATE) || (clk_rate > PLL_INFF_MAX_RATE)) {
>>> +   pr_debug("%s: input clk freq (%dHz) out of range\n",
>>> +__func__, clk_rate);
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   stm32_usbphyc_get_pll_params(clk_rate, _params);
>>> +
>>> +   usbphyc_pll = PLLDITHEN1 | PLLDITHEN0 | PLLSTRBYP;
>>> +   usbphyc_pll |= ((pll_params.ndiv << PLLNDIV_SHIFT) & PLLNDIV);
>>> +
>>> +   if (pll_params.frac) {
>>> +   usbphyc_pll |= PLLFRACCTL;
>>> +   usbphyc_pll |= ((pll_params.frac << PLLFRACIN_SHIFT)
>>> +& PLLFRACIN);
>>> +   }
>>> +
>>> +   writel(usbphyc_pll, usbphyc->base + STM32_USBPHYC_PLL);
>>> +
>>> +   pr_debug("%s: input clk freq=%dHz, ndiv=%d, frac=%d\n", __func__,
>>
>> dev_dbg
> 
> There's no access to udevice struct here that's why pr_debug() is used.
> 
> Thanks
> 
>>
>>> +clk_rate, pll_params.ndiv, pll_params.frac);
>>> +
>>> +   return 0;
>>> +}
>> [...]


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RESEND][PATCH v1] phy: add support for STM32 usb phy controller

[Patrice CHOTARD]

Hi Marek

On 04/26/2018 05:13 PM, Marek Vasut wrote:
> On 04/26/2018 04:23 PM, Patrice Chotard wrote:
>> From: Christophe Kerello 
>>
>> This patch adds phy tranceiver driver for STM32 USB PHY
>> Controller (usbphyc) that provides dual port High-Speed
>> phy for OTG (single port) and EHCI/OHCI host controller
>> (two ports).
>> One port of the phy is shared between the two USB controllers
>> through a UTMI+ switch.
>>
>> Signed-off-by: Christophe Kerello 
>> Signed-off-by: Patrice Chotard 
> 
> [...]
> 
>> +struct pll_params {
>> +u8 ndiv;
>> +u16 frac;
>> +};
>> +
>> +struct stm32_usbphyc {
>> +fdt_addr_t base;
>> +struct clk clk;
>> +struct stm32_usbphyc_phy {
>> +struct udevice *vdd;
>> +struct udevice *vdda1v1;
>> +struct udevice *vdda1v8;
>> +int index;
>> +bool init;
>> +bool powered;
>> +} phys[MAX_PHYS];
> 
> Shouldn't there be one driver instance per PHY ?

This driver manages a PHY provider + PHY child sub nodes as requested by 
Kernel maintainer.

more details here : https://lkml.org/lkml/2018/3/2/670

If you want i can add more details in v2 by including the DT bindings 
documentation ?

> 
>> +};
>> +
>> +void stm32_usbphyc_get_pll_params(u32 clk_rate, struct pll_params 
>> *pll_params)
>> +{
>> +unsigned long long fvco, ndiv, frac;
>> +
>> +/*
>> + *| FVCO = INFF*2*(NDIV + FRACT/2^16 ) when DITHER_DISABLE[1] = 1
>> + *| FVCO = 2880MHz
>> + *| NDIV = integer part of input bits to set the LDF
>> + *| FRACT = fractional part of input bits to set the LDF
>> + *  =>  PLLNDIV = integer part of (FVCO / (INFF*2))
>> + *  =>  PLLFRACIN = fractional part of(FVCO / INFF*2) * 2^16
>> + * <=>  PLLFRACIN = ((FVCO / (INFF*2)) - PLLNDIV) * 2^16
>> + */
>> +fvco = (unsigned long long)PLL_FVCO * 100; /* In Hz */
>> +
>> +ndiv = fvco;
>> +do_div(ndiv, (clk_rate * 2));
>> +pll_params->ndiv = (u8)ndiv;
>> +
>> +frac = fvco * (1 << 16);
>> +do_div(frac, (clk_rate * 2));
>> +frac = frac - (ndiv * (1 << 16));
>> +pll_params->frac = (u16)frac;
>> +}
>> +
>> +static int stm32_usbphyc_pll_init(struct stm32_usbphyc *usbphyc)
>> +{
>> +struct pll_params pll_params;
>> +u32 clk_rate = clk_get_rate(>clk);
>> +u32 usbphyc_pll;
>> +
>> +if ((clk_rate < PLL_INFF_MIN_RATE) || (clk_rate > PLL_INFF_MAX_RATE)) {
>> +pr_debug("%s: input clk freq (%dHz) out of range\n",
>> + __func__, clk_rate);
>> +return -EINVAL;
>> +}
>> +
>> +stm32_usbphyc_get_pll_params(clk_rate, _params);
>> +
>> +usbphyc_pll = PLLDITHEN1 | PLLDITHEN0 | PLLSTRBYP;
>> +usbphyc_pll |= ((pll_params.ndiv << PLLNDIV_SHIFT) & PLLNDIV);
>> +
>> +if (pll_params.frac) {
>> +usbphyc_pll |= PLLFRACCTL;
>> +usbphyc_pll |= ((pll_params.frac << PLLFRACIN_SHIFT)
>> + & PLLFRACIN);
>> +}
>> +
>> +writel(usbphyc_pll, usbphyc->base + STM32_USBPHYC_PLL);
>> +
>> +pr_debug("%s: input clk freq=%dHz, ndiv=%d, frac=%d\n", __func__,
> 
> dev_dbg

There's no access to udevice struct here that's why pr_debug() is used.

Thanks

> 
>> + clk_rate, pll_params.ndiv, pll_params.frac);
>> +
>> +return 0;
>> +}
> [...]
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RESEND][PATCH v1] phy: add support for STM32 usb phy controller

[Marek Vasut]

On 04/26/2018 04:23 PM, Patrice Chotard wrote:
> From: Christophe Kerello 
> 
> This patch adds phy tranceiver driver for STM32 USB PHY
> Controller (usbphyc) that provides dual port High-Speed
> phy for OTG (single port) and EHCI/OHCI host controller
> (two ports).
> One port of the phy is shared between the two USB controllers
> through a UTMI+ switch.
> 
> Signed-off-by: Christophe Kerello 
> Signed-off-by: Patrice Chotard 

[...]

> +struct pll_params {
> + u8 ndiv;
> + u16 frac;
> +};
> +
> +struct stm32_usbphyc {
> + fdt_addr_t base;
> + struct clk clk;
> + struct stm32_usbphyc_phy {
> + struct udevice *vdd;
> + struct udevice *vdda1v1;
> + struct udevice *vdda1v8;
> + int index;
> + bool init;
> + bool powered;
> + } phys[MAX_PHYS];

Shouldn't there be one driver instance per PHY ?

> +};
> +
> +void stm32_usbphyc_get_pll_params(u32 clk_rate, struct pll_params 
> *pll_params)
> +{
> + unsigned long long fvco, ndiv, frac;
> +
> + /*
> +  *| FVCO = INFF*2*(NDIV + FRACT/2^16 ) when DITHER_DISABLE[1] = 1
> +  *| FVCO = 2880MHz
> +  *| NDIV = integer part of input bits to set the LDF
> +  *| FRACT = fractional part of input bits to set the LDF
> +  *  =>  PLLNDIV = integer part of (FVCO / (INFF*2))
> +  *  =>  PLLFRACIN = fractional part of(FVCO / INFF*2) * 2^16
> +  * <=>  PLLFRACIN = ((FVCO / (INFF*2)) - PLLNDIV) * 2^16
> +  */
> + fvco = (unsigned long long)PLL_FVCO * 100; /* In Hz */
> +
> + ndiv = fvco;
> + do_div(ndiv, (clk_rate * 2));
> + pll_params->ndiv = (u8)ndiv;
> +
> + frac = fvco * (1 << 16);
> + do_div(frac, (clk_rate * 2));
> + frac = frac - (ndiv * (1 << 16));
> + pll_params->frac = (u16)frac;
> +}
> +
> +static int stm32_usbphyc_pll_init(struct stm32_usbphyc *usbphyc)
> +{
> + struct pll_params pll_params;
> + u32 clk_rate = clk_get_rate(>clk);
> + u32 usbphyc_pll;
> +
> + if ((clk_rate < PLL_INFF_MIN_RATE) || (clk_rate > PLL_INFF_MAX_RATE)) {
> + pr_debug("%s: input clk freq (%dHz) out of range\n",
> +  __func__, clk_rate);
> + return -EINVAL;
> + }
> +
> + stm32_usbphyc_get_pll_params(clk_rate, _params);
> +
> + usbphyc_pll = PLLDITHEN1 | PLLDITHEN0 | PLLSTRBYP;
> + usbphyc_pll |= ((pll_params.ndiv << PLLNDIV_SHIFT) & PLLNDIV);
> +
> + if (pll_params.frac) {
> + usbphyc_pll |= PLLFRACCTL;
> + usbphyc_pll |= ((pll_params.frac << PLLFRACIN_SHIFT)
> +  & PLLFRACIN);
> + }
> +
> + writel(usbphyc_pll, usbphyc->base + STM32_USBPHYC_PLL);
> +
> + pr_debug("%s: input clk freq=%dHz, ndiv=%d, frac=%d\n", __func__,

dev_dbg

> +  clk_rate, pll_params.ndiv, pll_params.frac);
> +
> + return 0;
> +}
[...]

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot