| Message ID | 1532726613-6483-7-git-send-email-arun.parameswaran@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add clock config and pm support to bcm iProc mdio mux | expand |
> static void mdio_mux_iproc_config(struct iproc_mdiomux_desc *md) > { > u32 val; > + u32 divisor; Hi Arun Reverse Christmas tree please. Andrew
> @@ -198,10 +219,22 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) > return PTR_ERR(md->base); > } > > + md->core_clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(md->core_clk)) { > + md->core_clk = NULL; > + } else { > + rc = clk_prepare_enable(md->core_clk); > + if (rc) { > + dev_err(&pdev->dev, "failed to enable core clk\n"); > + return rc; > + } > + } > + > md->mii_bus = mdiobus_alloc(); > if (!md->mii_bus) { > dev_err(&pdev->dev, "mdiomux bus alloc failed\n"); > - return -ENOMEM; > + rc = -ENOMEM; > + goto out; Hi Arun There is a devm_mdiobus_alloc() which could make the cleanup on error simple. > } > > bus = md->mii_bus; > @@ -217,7 +250,7 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) > rc = mdiobus_register(bus); > if (rc) { > dev_err(&pdev->dev, "mdiomux registration failed\n"); > - goto out; > + goto out_alloc; > } > > platform_set_drvdata(pdev, md); > @@ -236,8 +269,11 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) > > out_register: > mdiobus_unregister(bus); > -out: > +out_alloc: > mdiobus_free(bus); > +out: > + if (md->core_clk) > + clk_disable_unprepare(md->core_clk); > return rc; > } Andrew
Hi Andrew On 18-07-27 05:03 PM, Andrew Lunn wrote: >> static void mdio_mux_iproc_config(struct iproc_mdiomux_desc *md) >> { >> u32 val; >> + u32 divisor; > > Hi Arun > > Reverse Christmas tree please. > > Andrew > Ok, will send out an updated patch. Thanks Arun
Hi Andrew, On 18-07-28 02:22 PM, Andrew Lunn wrote: >> @@ -198,10 +219,22 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) >> return PTR_ERR(md->base); >> } >> >> + md->core_clk = devm_clk_get(&pdev->dev, NULL); >> + if (IS_ERR(md->core_clk)) { >> + md->core_clk = NULL; >> + } else { >> + rc = clk_prepare_enable(md->core_clk); >> + if (rc) { >> + dev_err(&pdev->dev, "failed to enable core clk\n"); >> + return rc; >> + } >> + } >> + >> md->mii_bus = mdiobus_alloc(); >> if (!md->mii_bus) { >> dev_err(&pdev->dev, "mdiomux bus alloc failed\n"); >> - return -ENOMEM; >> + rc = -ENOMEM; >> + goto out; > > Hi Arun > > There is a devm_mdiobus_alloc() which could make the cleanup on error > simple. > Should I do this as a separate patch ? Thanks Arun >> } >> >> bus = md->mii_bus; >> @@ -217,7 +250,7 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) >> rc = mdiobus_register(bus); >> if (rc) { >> dev_err(&pdev->dev, "mdiomux registration failed\n"); >> - goto out; >> + goto out_alloc; >> } >> >> platform_set_drvdata(pdev, md); >> @@ -236,8 +269,11 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) >> >> out_register: >> mdiobus_unregister(bus); >> -out: >> +out_alloc: >> mdiobus_free(bus); >> +out: >> + if (md->core_clk) >> + clk_disable_unprepare(md->core_clk); >> return rc; >> } > > Andrew >
> > Hi Arun > > > > There is a devm_mdiobus_alloc() which could make the cleanup on error > > simple. > > > Should I do this as a separate patch ? A separate patch is easier to review. And if something breaks, a git bisect gives you a better idea what actually broke it, if you have lots of small patches. Andrew
On 07/27/2018 02:23 PM, Arun Parameswaran wrote: > Add support to configure the internal rate adjust register based on the > core clock supplied through device tree in the Broadcom iProc mdio mux. > > The operating frequency of the mdio mux block is 11MHz. This is derrived > by dividing the clock to the mdio mux with the rate adjust register. > > In some SoC's the default values of the rate adjust register do not yield > 11MHz. These SoC's are required to specify the clock via the device tree > for proper operation. > > Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com> > --- [snip] > static int iproc_mdio_wait_for_idle(void __iomem *base, bool result) > @@ -198,10 +219,22 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) > return PTR_ERR(md->base); > } > > + md->core_clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(md->core_clk)) { > + md->core_clk = NULL; You need to specifically check for -EPROBE_DEFER here and propagate that to the caller, other errors could be ignored like you do it here, same comment as in patch 7, if you make md->core_clk NULL, then you can drop testing for that pointer being valid.
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c index c36ce4b..51d1003 100644 --- a/drivers/net/phy/mdio-mux-bcm-iproc.c +++ b/drivers/net/phy/mdio-mux-bcm-iproc.c @@ -13,7 +13,7 @@ * You should have received a copy of the GNU General Public License * version 2 (GPLv2) along with this source code. */ - +#include <linux/clk.h> #include <linux/platform_device.h> #include <linux/device.h> #include <linux/of_mdio.h> @@ -22,6 +22,10 @@ #include <linux/mdio-mux.h> #include <linux/delay.h> +#define MDIO_RATE_ADJ_EXT_OFFSET 0x000 +#define MDIO_RATE_ADJ_INT_OFFSET 0x004 +#define MDIO_RATE_ADJ_DIVIDENT_SHIFT 16 + #define MDIO_SCAN_CTRL_OFFSET 0x008 #define MDIO_SCAN_CTRL_OVRIDE_EXT_MSTR 28 @@ -49,21 +53,38 @@ #define MDIO_REG_ADDR_SPACE_SIZE 0x250 +#define MDIO_OPERATING_FREQUENCY 11000000 +#define MDIO_RATE_ADJ_DIVIDENT 1 + struct iproc_mdiomux_desc { void *mux_handle; void __iomem *base; struct device *dev; struct mii_bus *mii_bus; + struct clk *core_clk; }; static void mdio_mux_iproc_config(struct iproc_mdiomux_desc *md) { u32 val; + u32 divisor; /* Disable external mdio master access */ val = readl(md->base + MDIO_SCAN_CTRL_OFFSET); val |= BIT(MDIO_SCAN_CTRL_OVRIDE_EXT_MSTR); writel(val, md->base + MDIO_SCAN_CTRL_OFFSET); + + if (md->core_clk) { + /* use rate adjust regs to derrive the mdio's operating + * frequency from the specified core clock + */ + divisor = clk_get_rate(md->core_clk) / MDIO_OPERATING_FREQUENCY; + divisor = divisor / (MDIO_RATE_ADJ_DIVIDENT + 1); + val = divisor; + val |= MDIO_RATE_ADJ_DIVIDENT << MDIO_RATE_ADJ_DIVIDENT_SHIFT; + writel(val, md->base + MDIO_RATE_ADJ_EXT_OFFSET); + writel(val, md->base + MDIO_RATE_ADJ_INT_OFFSET); + } } static int iproc_mdio_wait_for_idle(void __iomem *base, bool result) @@ -198,10 +219,22 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) return PTR_ERR(md->base); } + md->core_clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(md->core_clk)) { + md->core_clk = NULL; + } else { + rc = clk_prepare_enable(md->core_clk); + if (rc) { + dev_err(&pdev->dev, "failed to enable core clk\n"); + return rc; + } + } + md->mii_bus = mdiobus_alloc(); if (!md->mii_bus) { dev_err(&pdev->dev, "mdiomux bus alloc failed\n"); - return -ENOMEM; + rc = -ENOMEM; + goto out; } bus = md->mii_bus; @@ -217,7 +250,7 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) rc = mdiobus_register(bus); if (rc) { dev_err(&pdev->dev, "mdiomux registration failed\n"); - goto out; + goto out_alloc; } platform_set_drvdata(pdev, md); @@ -236,8 +269,11 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev) out_register: mdiobus_unregister(bus); -out: +out_alloc: mdiobus_free(bus); +out: + if (md->core_clk) + clk_disable_unprepare(md->core_clk); return rc; } @@ -248,6 +284,8 @@ static int mdio_mux_iproc_remove(struct platform_device *pdev) mdio_mux_uninit(md->mux_handle); mdiobus_unregister(md->mii_bus); mdiobus_free(md->mii_bus); + if (md->core_clk) + clk_disable_unprepare(md->core_clk); return 0; }
Add support to configure the internal rate adjust register based on the core clock supplied through device tree in the Broadcom iProc mdio mux. The operating frequency of the mdio mux block is 11MHz. This is derrived by dividing the clock to the mdio mux with the rate adjust register. In some SoC's the default values of the rate adjust register do not yield 11MHz. These SoC's are required to specify the clock via the device tree for proper operation. Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com> --- drivers/net/phy/mdio-mux-bcm-iproc.c | 46 ++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-)