[v9,04/10] clk: Add Lynx 10G SerDes PLL driver

This adds support for the PLLs found in Lynx 10G "SerDes" devices found on
various NXP QorIQ SoCs. There are two PLLs in each SerDes. This driver has
been split from the main PHY driver to allow for better review, even though
these PLLs are not present anywhere else besides the SerDes. An auxiliary
device is not used as it offers no benefits over a function call (and there
is no need to have a separate device).

The PLLs are modeled as clocks proper to let us take advantage of the
existing clock infrastructure. I have not given the same treatment to the
per-lane clocks because they need to be programmed in-concert with the rest
of the lane settings. One tricky thing is that the VCO (PLL) rate exceeds
2^32 (maxing out at around 5GHz). This will be a problem on 32-bit
platforms, since clock rates are stored as unsigned longs. To work around
this, the pll clock rate is generally treated in units of kHz.

The PLLs are configured rather interestingly. Instead of the usual direct
programming of the appropriate divisors, the input and output clock rates
are selected directly. Generally, the only restriction is that the input
and output must be integer multiples of each other. This suggests some kind
of internal look-up table. The datasheets generally list out the supported
combinations explicitly, and not all input/output combinations are
documented. I'm not sure if this is due to lack of support, or due to an
oversight. If this becomes an issue, then some combinations can be
blacklisted (or whitelisted). This may also be necessary for other SoCs
which have more stringent clock requirements.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>

---

Changes in v9:
- Convert some u32s to unsigned long to match arguments
- Switch from round_rate to determine_rate
- Drop explicit reference to reference clock
- Use .parent_names when requesting parents
- Use devm_clk_hw_get_clk to pass clocks back to serdes
- Fix indentation
- Split off from following patch to allow for better review

 MAINTAINERS                    |   7 +
 drivers/clk/Makefile           |   1 +
 drivers/clk/clk-fsl-lynx-10g.c | 509 +++++++++++++++++++++++++++++++++
 drivers/phy/freescale/Kconfig  |   6 +
 include/linux/phy/lynx-10g.h   |  16 ++
 5 files changed, 539 insertions(+)
 create mode 100644 drivers/clk/clk-fsl-lynx-10g.c
 create mode 100644 include/linux/phy/lynx-10g.h

Quoting Sean Anderson (2022-12-29 16:01:33)
> This adds support for the PLLs found in Lynx 10G "SerDes" devices found on
> various NXP QorIQ SoCs. There are two PLLs in each SerDes. This driver has
> been split from the main PHY driver to allow for better review, even though
> these PLLs are not present anywhere else besides the SerDes. An auxiliary
> device is not used as it offers no benefits over a function call (and there
> is no need to have a separate device).
> 
> The PLLs are modeled as clocks proper to let us take advantage of the
> existing clock infrastructure.

What advantage do we gain?

> I have not given the same treatment to the
> per-lane clocks because they need to be programmed in-concert with the rest
> of the lane settings. One tricky thing is that the VCO (PLL) rate exceeds
> 2^32 (maxing out at around 5GHz). This will be a problem on 32-bit
> platforms, since clock rates are stored as unsigned longs. To work around
> this, the pll clock rate is generally treated in units of kHz.

This looks like a disadvantage. Are we reporting the frequency in kHz to
the clk framework?

> 
> The PLLs are configured rather interestingly. Instead of the usual direct
> programming of the appropriate divisors, the input and output clock rates
> are selected directly. Generally, the only restriction is that the input
> and output must be integer multiples of each other. This suggests some kind
> of internal look-up table. The datasheets generally list out the supported
> combinations explicitly, and not all input/output combinations are
> documented. I'm not sure if this is due to lack of support, or due to an
> oversight. If this becomes an issue, then some combinations can be
> blacklisted (or whitelisted). This may also be necessary for other SoCs
> which have more stringent clock requirements.

I'm wondering if a clk provider should be created at all here. Who is
the consumer of the clk? The phy driver itself? Does the clk provided
need to interact with other clks in the system? Or is the clk tree
wholly self-contained?

Can the phy consumer configure the output frequency directly via
phy_configure() or when the phy is enabled? I'm thinking the phy driver
can call clk_set_rate() on the parent 'rfclk' before or after setting
the bits to control the output rate, and use clk_round_rate() to figure
out what input frequencies are supported for the output frequency
desired. This would avoid kHz overflowing 32-bits, and the big clk lock
getting blocked on some other clk in the system changing rates.

BTW, what sort of phy is this? Some networking device?

> 
> diff --git a/drivers/clk/clk-fsl-lynx-10g.c b/drivers/clk/clk-fsl-lynx-10g.c
> new file mode 100644
> index 000000000000..61f68b5ae675
> --- /dev/null
> +++ b/drivers/clk/clk-fsl-lynx-10g.c
> @@ -0,0 +1,509 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Sean Anderson <sean.anderson@seco.com>
> + *
> + * This file contains the implementation for the PLLs found on Lynx 10G phys.
> + *
> + * XXX: The VCO rate of the PLLs can exceed ~4GHz, which is the maximum rate
> + * expressable in an unsigned long. To work around this, rates are specified in
> + * kHz. This is as if there was a division by 1000 in the PLL.
> + */
> +
> +#include <linux/clk.h>

Is this include used? If not, please remove.

> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/bitfield.h>
> +#include <linux/math64.h>
> +#include <linux/phy/lynx-10g.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/units.h>
> +#include <dt-bindings/clock/fsl,lynx-10g.h>
> +
> +#define PLL_STRIDE     0x20
> +#define PLLa(a, off)   ((a) * PLL_STRIDE + (off))
> +#define PLLaRSTCTL(a)  PLLa(a, 0x00)
> +#define PLLaCR0(a)     PLLa(a, 0x04)
> +
> +#define PLLaRSTCTL_RSTREQ      BIT(31)
> +#define PLLaRSTCTL_RST_DONE    BIT(30)
> +#define PLLaRSTCTL_RST_ERR     BIT(29)
[...]
> +
> +static int lynx_clk_init(struct clk_hw_onecell_data *hw_data,
> +                        struct device *dev, struct regmap *regmap,
> +                        unsigned int index)
> +{
> +       const struct clk_hw *ex_dly_parents;
> +       struct clk_parent_data pll_parents[1] = { };
> +       struct clk_init_data pll_init = {
> +               .ops = &lynx_pll_clk_ops,
> +               .parent_data = pll_parents,
> +               .num_parents = 1,
> +               .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT |

Why is the nocache flag used?

> +                        CLK_OPS_PARENT_ENABLE,
> +       };
> +       struct clk_init_data ex_dly_init = {
> +               .ops = &lynx_ex_dly_clk_ops,
> +               .parent_hws = &ex_dly_parents,
> +               .num_parents = 1,
> +       };
> +       struct lynx_clk *clk;
> +       int ret;

On 1/27/23 15:59, Stephen Boyd wrote:
> Quoting Sean Anderson (2022-12-29 16:01:33)
>> This adds support for the PLLs found in Lynx 10G "SerDes" devices found on
>> various NXP QorIQ SoCs. There are two PLLs in each SerDes. This driver has
>> been split from the main PHY driver to allow for better review, even though
>> these PLLs are not present anywhere else besides the SerDes. An auxiliary
>> device is not used as it offers no benefits over a function call (and there
>> is no need to have a separate device).
>> 
>> The PLLs are modeled as clocks proper to let us take advantage of the
>> existing clock infrastructure.
> 
> What advantage do we gain?

The handling of rate changes, locking, etc is all done by the clock
subsystem. We can use the existing debugfs stuff as well. And everything
is organized by existing API boundaries.

>> I have not given the same treatment to the
>> per-lane clocks because they need to be programmed in-concert with the rest
>> of the lane settings. One tricky thing is that the VCO (PLL) rate exceeds
>> 2^32 (maxing out at around 5GHz). This will be a problem on 32-bit
>> platforms, since clock rates are stored as unsigned longs. To work around
>> this, the pll clock rate is generally treated in units of kHz.
> 
> This looks like a disadvantage. Are we reporting the frequency in kHz to
> the clk framework?

It is, and yes.

>> 
>> The PLLs are configured rather interestingly. Instead of the usual direct
>> programming of the appropriate divisors, the input and output clock rates
>> are selected directly. Generally, the only restriction is that the input
>> and output must be integer multiples of each other. This suggests some kind
>> of internal look-up table. The datasheets generally list out the supported
>> combinations explicitly, and not all input/output combinations are
>> documented. I'm not sure if this is due to lack of support, or due to an
>> oversight. If this becomes an issue, then some combinations can be
>> blacklisted (or whitelisted). This may also be necessary for other SoCs
>> which have more stringent clock requirements.
> 
> I'm wondering if a clk provider should be created at all here. Who is
> the consumer of the clk? The phy driver itself? Does the clk provided
> need to interact with other clks in the system? Or is the clk tree
> wholly self-contained?

It's wholly self-contained, aside from an undocumented mode where you
can output a fixed frequency (for testing). The provider exists so you
can use assigned-clocks to ensure a particular PLL is used for a
particular frequency. This prevents a problem where e.g. you have
reference clocks for 100 Hz and 156.25 Hz, one lane requests 5 GHz and
the PLL with the 156.25 Hz clock gets used. Then, later another lane
requests 5.15625 GHz and the remaining 100 Hz PLL can't provide it. To
prevent this, you can assign the PLLs their rates up front and the lanes
will use the existing rates. Most of the time this should be done by the
RCW, but there are cases where the RCW can't set up the clocks properly.

> Can the phy consumer configure the output frequency directly via
> phy_configure() or when the phy is enabled? I'm thinking the phy driver
> can call clk_set_rate() on the parent 'rfclk' before or after setting
> the bits to control the output rate,

This is what currently happens. See lynx_set_mode in the next patch.

> and use clk_round_rate() to figure
> out what input frequencies are supported for the output frequency
> desired. This would avoid kHz overflowing 32-bits, and the big clk lock
> getting blocked on some other clk in the system changing rates.

Can you describe this in a bit more detail? Are you suggesting to skip
the clock framework?

> BTW, what sort of phy is this? Some networking device?

Hi Stephen,

On 1/27/23 16:51, Sean Anderson wrote:
> On 1/27/23 15:59, Stephen Boyd wrote:
>> Quoting Sean Anderson (2022-12-29 16:01:33)
>>> This adds support for the PLLs found in Lynx 10G "SerDes" devices found on
>>> various NXP QorIQ SoCs. There are two PLLs in each SerDes. This driver has
>>> been split from the main PHY driver to allow for better review, even though
>>> these PLLs are not present anywhere else besides the SerDes. An auxiliary
>>> device is not used as it offers no benefits over a function call (and there
>>> is no need to have a separate device).
>>> 
>>> The PLLs are modeled as clocks proper to let us take advantage of the
>>> existing clock infrastructure.
>> 
>> What advantage do we gain?
> 
> The handling of rate changes, locking, etc is all done by the clock
> subsystem. We can use the existing debugfs stuff as well. And everything
> is organized by existing API boundaries.
> 
>>> I have not given the same treatment to the
>>> per-lane clocks because they need to be programmed in-concert with the rest
>>> of the lane settings. One tricky thing is that the VCO (PLL) rate exceeds
>>> 2^32 (maxing out at around 5GHz). This will be a problem on 32-bit
>>> platforms, since clock rates are stored as unsigned longs. To work around
>>> this, the pll clock rate is generally treated in units of kHz.
>> 
>> This looks like a disadvantage. Are we reporting the frequency in kHz to
>> the clk framework?
> 
> It is, and yes.
> 
>>> 
>>> The PLLs are configured rather interestingly. Instead of the usual direct
>>> programming of the appropriate divisors, the input and output clock rates
>>> are selected directly. Generally, the only restriction is that the input
>>> and output must be integer multiples of each other. This suggests some kind
>>> of internal look-up table. The datasheets generally list out the supported
>>> combinations explicitly, and not all input/output combinations are
>>> documented. I'm not sure if this is due to lack of support, or due to an
>>> oversight. If this becomes an issue, then some combinations can be
>>> blacklisted (or whitelisted). This may also be necessary for other SoCs
>>> which have more stringent clock requirements.
>> 
>> I'm wondering if a clk provider should be created at all here. Who is
>> the consumer of the clk? The phy driver itself? Does the clk provided
>> need to interact with other clks in the system? Or is the clk tree
>> wholly self-contained?
> 
> It's wholly self-contained, aside from an undocumented mode where you
> can output a fixed frequency (for testing). The provider exists so you
> can use assigned-clocks to ensure a particular PLL is used for a
> particular frequency. This prevents a problem where e.g. you have
> reference clocks for 100 Hz and 156.25 Hz, one lane requests 5 GHz and
> the PLL with the 156.25 Hz clock gets used. Then, later another lane
> requests 5.15625 GHz and the remaining 100 Hz PLL can't provide it. To
> prevent this, you can assign the PLLs their rates up front and the lanes
> will use the existing rates. Most of the time this should be done by the
> RCW, but there are cases where the RCW can't set up the clocks properly.
> 
>> Can the phy consumer configure the output frequency directly via
>> phy_configure() or when the phy is enabled? I'm thinking the phy driver
>> can call clk_set_rate() on the parent 'rfclk' before or after setting
>> the bits to control the output rate,
> 
> This is what currently happens. See lynx_set_mode in the next patch.
> 
>> and use clk_round_rate() to figure
>> out what input frequencies are supported for the output frequency
>> desired. This would avoid kHz overflowing 32-bits, and the big clk lock
>> getting blocked on some other clk in the system changing rates.
> 
> Can you describe this in a bit more detail? Are you suggesting to skip
> the clock framework?
> 
>> BTW, what sort of phy is this? Some networking device?
> 
> From the Kconfig in the next commit:
> 
> This adds support for the Lynx "SerDes" devices found on various QorIQ
> SoCs. There may be up to four SerDes devices on each SoC, and each
> device supports up to eight lanes. The SerDes is configured by
> default by the RCW, but this module is necessary in order to support
> some modes (such as 2.5G SGMII or 1000BASE-KX), or clock setups (as
> only as subset of clock configurations are supported by the RCW).
> The hardware supports a variety of protocols, including Ethernet,
> SATA, PCIe, and more exotic links such as Interlaken and Aurora. This
> driver only supports Ethernet, but it will try not to touch lanes
> configured for other protocols.
> 
>>> 
>>> diff --git a/drivers/clk/clk-fsl-lynx-10g.c b/drivers/clk/clk-fsl-lynx-10g.c
>>> new file mode 100644
>>> index 000000000000..61f68b5ae675
>>> --- /dev/null
>>> +++ b/drivers/clk/clk-fsl-lynx-10g.c
>>> @@ -0,0 +1,509 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2022 Sean Anderson <sean.anderson@seco.com>
>>> + *
>>> + * This file contains the implementation for the PLLs found on Lynx 10G phys.
>>> + *
>>> + * XXX: The VCO rate of the PLLs can exceed ~4GHz, which is the maximum rate
>>> + * expressable in an unsigned long. To work around this, rates are specified in
>>> + * kHz. This is as if there was a division by 1000 in the PLL.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>> 
>> Is this include used? If not, please remove.
> 
> OK
> 
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/device.h>
>>> +#include <linux/bitfield.h>
>>> +#include <linux/math64.h>
>>> +#include <linux/phy/lynx-10g.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/units.h>
>>> +#include <dt-bindings/clock/fsl,lynx-10g.h>
>>> +
>>> +#define PLL_STRIDE     0x20
>>> +#define PLLa(a, off)   ((a) * PLL_STRIDE + (off))
>>> +#define PLLaRSTCTL(a)  PLLa(a, 0x00)
>>> +#define PLLaCR0(a)     PLLa(a, 0x04)
>>> +
>>> +#define PLLaRSTCTL_RSTREQ      BIT(31)
>>> +#define PLLaRSTCTL_RST_DONE    BIT(30)
>>> +#define PLLaRSTCTL_RST_ERR     BIT(29)
>> [...]
>>> +
>>> +static int lynx_clk_init(struct clk_hw_onecell_data *hw_data,
>>> +                        struct device *dev, struct regmap *regmap,
>>> +                        unsigned int index)
>>> +{
>>> +       const struct clk_hw *ex_dly_parents;
>>> +       struct clk_parent_data pll_parents[1] = { };
>>> +       struct clk_init_data pll_init = {
>>> +               .ops = &lynx_pll_clk_ops,
>>> +               .parent_data = pll_parents,
>>> +               .num_parents = 1,
>>> +               .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT |
>> 
>> Why is the nocache flag used?
> 
> PCIe rate selection can automatically modify the frequency of the
> PLL. From the reference manual:
> 
> | For PCIe 8 Gbaud, the auto-negotiation also involves a combination of
> | switching PLL selects, and/or reconfiguring a PLL. There are two
> | scenarios for PLL reconfiguration:
> |
> | * PCIe starts on any PLL which runs at 5GHz, and the other PLL is off.
> | PHY wrapper will switch to the other PLL as part of Gen 3 speed
> | switch. In this case, both PLLs must be supplied with valid reference
> | clocks.
> | * PCIe starts on any PLL which runs at 5GHz, and the other PLL is
> | being used for other protocols. PHY wrapper will reconfigure the
> | current 5GHz PLL as part of Gen 3 speed switch. PCIe stays on the same
> | PLL in this case. This also applies to the scenario where one PCIe
> | port runs on all lanes.
> 
> As far as I know there's no way to configure or disable this. PCIe isn't
> implemented yet, but it would likely need some way to mark PLLs for
> exclusive use (not just exclusive rate). I figured it would be better
> not to cache the rate so that things like the above would get detected
> properly.
> 
> --Sean
> 
>>> +                        CLK_OPS_PARENT_ENABLE,
>>> +       };
>>> +       struct clk_init_data ex_dly_init = {
>>> +               .ops = &lynx_ex_dly_clk_ops,
>>> +               .parent_hws = &ex_dly_parents,
>>> +               .num_parents = 1,
>>> +       };
>>> +       struct lynx_clk *clk;
>>> +       int ret;

I am going to try and send out v10 soon. Do you have any comments on my
reply here? At the moment the only change I am planning to make is to
remove the clk.h include.

--Sean