Message ID | 20231212115151.20016-4-quic_luoj@quicinc.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | support ipq5332 platform | expand |
Hello, I have some more minor comments for yoi :) On Tue, 12 Dec 2023 19:51:48 +0800 Luo Jie <quic_luoj@quicinc.com> wrote: > The reference clock of CMN PLL block is selectable, the internal > 48MHZ is used by default. > > The output clock of CMN PLL block is for providing the clock > source of ethernet device(such as qca8084), there are 1 * 25MHZ > and 3 * 50MHZ output clocks available for the ethernet devices. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- [...] > +/* For the CMN PLL block, the reference clock can be configured according to > + * the device tree property "cmn-reference-clock", the internal 48MHZ is used > + * by default on the ipq533 platform. > + * > + * The output clock of CMN PLL block is provided to the ethernet devices, > + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default. > + * > + * Such as the output 50M clock for the qca8084 ethernet PHY. > + */ > +static int ipq_cmn_clock_config(struct mii_bus *bus) > +{ > + int ret; > + u32 reg_val, src_sel, ref_clk; > + struct ipq4019_mdio_data *priv; Here you should also use reverse christmas-tree notation [...] > @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) > } > } > > + /* The CMN block resource is for providing clock source to ethernet, > + * which can be optionally configured on the platform ipq9574 and > + * ipq5332. > + */ > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk"); > + if (res) { > + priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(priv->cmn_membase)) > + return PTR_ERR(priv->cmn_membase); > + } > + And here you can simplify a bit by using devm_platform_ioremap_resource_byname() Thanks, Maxime
On Tue, Dec 12, 2023 at 01:54:17PM +0100, Maxime Chevallier wrote: > Hello, > > I have some more minor comments for yoi :) > > On Tue, 12 Dec 2023 19:51:48 +0800 > Luo Jie <quic_luoj@quicinc.com> wrote: > > + /* The CMN block resource is for providing clock source to ethernet, > > + * which can be optionally configured on the platform ipq9574 and > > + * ipq5332. > > + */ > > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk"); > > + if (res) { > > + priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(priv->cmn_membase)) > > + return PTR_ERR(priv->cmn_membase); > > + } > > + > > And here you can simplify a bit by using > devm_platform_ioremap_resource_byname() Not if the resource is optional.
On 12/12/2023 8:54 PM, Maxime Chevallier wrote: > Hello, > > I have some more minor comments for yoi :) > > On Tue, 12 Dec 2023 19:51:48 +0800 > Luo Jie <quic_luoj@quicinc.com> wrote: > >> The reference clock of CMN PLL block is selectable, the internal >> 48MHZ is used by default. >> >> The output clock of CMN PLL block is for providing the clock >> source of ethernet device(such as qca8084), there are 1 * 25MHZ >> and 3 * 50MHZ output clocks available for the ethernet devices. >> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> >> --- > > [...] > >> +/* For the CMN PLL block, the reference clock can be configured according to >> + * the device tree property "cmn-reference-clock", the internal 48MHZ is used >> + * by default on the ipq533 platform. >> + * >> + * The output clock of CMN PLL block is provided to the ethernet devices, >> + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default. >> + * >> + * Such as the output 50M clock for the qca8084 ethernet PHY. >> + */ >> +static int ipq_cmn_clock_config(struct mii_bus *bus) >> +{ >> + int ret; >> + u32 reg_val, src_sel, ref_clk; >> + struct ipq4019_mdio_data *priv; > > Here you should also use reverse christmas-tree notation Ok, will correct this, thanks. > > [...] > >> @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) >> } >> } >> >> + /* The CMN block resource is for providing clock source to ethernet, >> + * which can be optionally configured on the platform ipq9574 and >> + * ipq5332. >> + */ >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk"); >> + if (res) { >> + priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(priv->cmn_membase)) >> + return PTR_ERR(priv->cmn_membase); >> + } >> + > > And here you can simplify a bit by using > devm_platform_ioremap_resource_byname() > > Thanks, > > Maxime > As Russell mentioned, since this resource is optional, so devm_platform_ioremap_resource_byname can't be used here.
On Wed, 13 Dec 2023 16:09:53 +0800 Jie Luo <quic_luoj@quicinc.com> wrote: > On 12/12/2023 8:54 PM, Maxime Chevallier wrote: > > Hello, > > > > I have some more minor comments for yoi :) > > > > On Tue, 12 Dec 2023 19:51:48 +0800 > > Luo Jie <quic_luoj@quicinc.com> wrote: > > > >> The reference clock of CMN PLL block is selectable, the internal > >> 48MHZ is used by default. > >> > >> The output clock of CMN PLL block is for providing the clock > >> source of ethernet device(such as qca8084), there are 1 * 25MHZ > >> and 3 * 50MHZ output clocks available for the ethernet devices. > >> > >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > >> --- > > > > [...] > > > >> +/* For the CMN PLL block, the reference clock can be configured according to > >> + * the device tree property "cmn-reference-clock", the internal 48MHZ is used > >> + * by default on the ipq533 platform. > >> + * > >> + * The output clock of CMN PLL block is provided to the ethernet devices, > >> + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default. > >> + * > >> + * Such as the output 50M clock for the qca8084 ethernet PHY. > >> + */ > >> +static int ipq_cmn_clock_config(struct mii_bus *bus) > >> +{ > >> + int ret; > >> + u32 reg_val, src_sel, ref_clk; > >> + struct ipq4019_mdio_data *priv; > > > > Here you should also use reverse christmas-tree notation > > Ok, will correct this, thanks. > > > > > [...] > > > >> @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) > >> } > >> } > >> > >> + /* The CMN block resource is for providing clock source to ethernet, > >> + * which can be optionally configured on the platform ipq9574 and > >> + * ipq5332. > >> + */ > >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk"); > >> + if (res) { > >> + priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res); > >> + if (IS_ERR(priv->cmn_membase)) > >> + return PTR_ERR(priv->cmn_membase); > >> + } > >> + > > > > And here you can simplify a bit by using > > devm_platform_ioremap_resource_byname() > > > > Thanks, > > > > Maxime > > > As Russell mentioned, since this resource is optional, > so devm_platform_ioremap_resource_byname can't be used here. > Indeed, my bad I missed that point. Sorry for the noise :/ Thanks, Maxime
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c index 582e41ab0990..8d3c6bae379f 100644 --- a/drivers/net/mdio/mdio-ipq4019.c +++ b/drivers/net/mdio/mdio-ipq4019.c @@ -44,6 +44,25 @@ /* Maximum SOC PCS(uniphy) number on IPQ platform */ #define ETH_LDO_RDY_CNT 3 +#define CMN_PLL_REFERENCE_SOURCE_SEL 0x28 +#define CMN_PLL_REFCLK_SOURCE_DIV GENMASK(9, 8) + +#define CMN_PLL_REFERENCE_CLOCK 0x784 +#define CMN_PLL_REFCLK_EXTERNAL BIT(9) +#define CMN_PLL_REFCLK_DIV GENMASK(8, 4) +#define CMN_PLL_REFCLK_INDEX GENMASK(3, 0) + +#define CMN_PLL_POWER_ON_AND_RESET 0x780 +#define CMN_ANA_EN_SW_RSTN BIT(6) + +#define CMN_REFCLK_INTERNAL_48MHZ 0 +#define CMN_REFCLK_EXTERNAL_25MHZ 1 +#define CMN_REFCLK_EXTERNAL_31250KHZ 2 +#define CMN_REFCLK_EXTERNAL_40MHZ 3 +#define CMN_REFCLK_EXTERNAL_48MHZ 4 +#define CMN_REFCLK_EXTERNAL_50MHZ 5 +#define CMN_REFCLK_INTERNAL_96MHZ 6 + enum mdio_clk_id { MDIO_CLK_MDIO_AHB, MDIO_CLK_UNIPHY0_AHB, @@ -55,6 +74,7 @@ enum mdio_clk_id { struct ipq4019_mdio_data { void __iomem *membase; + void __iomem *cmn_membase; void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT]; struct clk *clk[MDIO_CLK_CNT]; }; @@ -227,12 +247,116 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum, return 0; } +/* For the CMN PLL block, the reference clock can be configured according to + * the device tree property "cmn-reference-clock", the internal 48MHZ is used + * by default on the ipq533 platform. + * + * The output clock of CMN PLL block is provided to the ethernet devices, + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default. + * + * Such as the output 50M clock for the qca8084 ethernet PHY. + */ +static int ipq_cmn_clock_config(struct mii_bus *bus) +{ + int ret; + u32 reg_val, src_sel, ref_clk; + struct ipq4019_mdio_data *priv; + + priv = bus->priv; + if (priv->cmn_membase) { + reg_val = readl(priv->cmn_membase + CMN_PLL_REFERENCE_CLOCK); + + /* Select reference clock source */ + ret = of_property_read_u32(bus->parent->of_node, + "cmn-reference-clock", + &ref_clk); + if (!ret) { + switch (ref_clk) { + case CMN_REFCLK_INTERNAL_48MHZ: + reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL | + CMN_PLL_REFCLK_INDEX); + reg_val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7); + break; + case CMN_REFCLK_EXTERNAL_25MHZ: + reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL | + CMN_PLL_REFCLK_INDEX); + reg_val |= (CMN_PLL_REFCLK_EXTERNAL | + FIELD_PREP(CMN_PLL_REFCLK_INDEX, 3)); + break; + case CMN_REFCLK_EXTERNAL_31250KHZ: + reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL | + CMN_PLL_REFCLK_INDEX); + reg_val |= (CMN_PLL_REFCLK_EXTERNAL | + FIELD_PREP(CMN_PLL_REFCLK_INDEX, 4)); + break; + case CMN_REFCLK_EXTERNAL_40MHZ: + reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL | + CMN_PLL_REFCLK_INDEX); + reg_val |= (CMN_PLL_REFCLK_EXTERNAL | + FIELD_PREP(CMN_PLL_REFCLK_INDEX, 6)); + break; + case CMN_REFCLK_EXTERNAL_48MHZ: + reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL | + CMN_PLL_REFCLK_INDEX); + reg_val |= (CMN_PLL_REFCLK_EXTERNAL | + FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7)); + break; + case CMN_REFCLK_EXTERNAL_50MHZ: + reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL | + CMN_PLL_REFCLK_INDEX); + reg_val |= (CMN_PLL_REFCLK_EXTERNAL | + FIELD_PREP(CMN_PLL_REFCLK_INDEX, 8)); + break; + case CMN_REFCLK_INTERNAL_96MHZ: + src_sel = readl(priv->cmn_membase + + CMN_PLL_REFERENCE_SOURCE_SEL); + src_sel &= ~CMN_PLL_REFCLK_SOURCE_DIV; + src_sel |= FIELD_PREP(CMN_PLL_REFCLK_SOURCE_DIV, 0); + writel(src_sel, priv->cmn_membase + + CMN_PLL_REFERENCE_SOURCE_SEL); + + reg_val &= ~CMN_PLL_REFCLK_DIV; + reg_val |= FIELD_PREP(CMN_PLL_REFCLK_DIV, 2); + break; + default: + return -EINVAL; + } + } else if (ret == -EINVAL) { + /* If the cmn-reference-clock is not specified, + * the internal 48MHZ is selected by default. + */ + reg_val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7); + } else { + return ret; + } + + writel(reg_val, priv->cmn_membase + CMN_PLL_REFERENCE_CLOCK); + + /* assert CMN PLL */ + reg_val = readl(priv->cmn_membase + CMN_PLL_POWER_ON_AND_RESET); + reg_val &= ~CMN_ANA_EN_SW_RSTN; + writel(reg_val, priv->cmn_membase); + fsleep(IPQ_PHY_SET_DELAY_US); + + /* deassert CMN PLL */ + reg_val |= CMN_ANA_EN_SW_RSTN; + writel(reg_val, priv->cmn_membase + CMN_PLL_POWER_ON_AND_RESET); + fsleep(IPQ_PHY_SET_DELAY_US); + } + + return 0; +} + static int ipq_mdio_reset(struct mii_bus *bus) { struct ipq4019_mdio_data *priv = bus->priv; int ret, index; unsigned long rate; + ret = ipq_cmn_clock_config(bus); + if (ret) + return ret; + /* For the platform ipq5332, there are two SoC uniphies available * for connecting with ethernet PHY, the SoC uniphy gcc clock * should be enabled for resetting the connected device such @@ -296,7 +420,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) /* This resource are optional */ for (index = 0; index < ETH_LDO_RDY_CNT; index++) { res = platform_get_resource(pdev, IORESOURCE_MEM, index + 1); - if (res) { + if (res && strcmp(res->name, "cmn_blk")) { priv->eth_ldo_rdy[index] = devm_ioremap(&pdev->dev, res->start, resource_size(res)); @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) } } + /* The CMN block resource is for providing clock source to ethernet, + * which can be optionally configured on the platform ipq9574 and + * ipq5332. + */ + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk"); + if (res) { + priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(priv->cmn_membase)) + return PTR_ERR(priv->cmn_membase); + } + for (index = 0; index < MDIO_CLK_CNT; index++) { priv->clk[index] = devm_clk_get_optional(&pdev->dev, mdio_clk_name[index]);
The reference clock of CMN PLL block is selectable, the internal 48MHZ is used by default. The output clock of CMN PLL block is for providing the clock source of ethernet device(such as qca8084), there are 1 * 25MHZ and 3 * 50MHZ output clocks available for the ethernet devices. Signed-off-by: Luo Jie <quic_luoj@quicinc.com> --- drivers/net/mdio/mdio-ipq4019.c | 137 +++++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-)