Hi,
The patchset is based on David Miller's "net" repo.
The patchset extends the Broadcom iProc mdio mux to add support for
suspend/resume and the ability to configure the internal clock
divider.
The base address of the mdio-mux-bcm-iproc is modified to point to the
start of the mdio block's address space, to be able to access all the
mdio's registers. The missing registers are required to configure the
internal clock divider registers in some of the Broadcom SoC's.
Thanks
Arun
Arun Parameswaran (7):
dt-bindings: net: Fix Broadcom iProc mdio mux driver base address
net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
arm64: dts: Fix the base address of the Broadcom iProc mdio mux
dt-bindings: net: Add clock handle to Broadcom iProc mdio mux
net: phy: Add support to configure clock in Broadcom iProc mdio mux
net: phy: Add pm support to Broadcom iProc mdio mux driver
net: phy: Add pm support for scan ctrl register to bcm mdio mux
.../bindings/net/brcm,mdio-mux-iproc.txt | 7 +-
arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi | 4 +-
.../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 4 +-
drivers/net/phy/mdio-mux-bcm-iproc.c | 89 ++++++++++++++++++++--
4 files changed, 92 insertions(+), 12 deletions(-)
--
1.9.1
Add support for saving and restoring the 'scan control' register
in the Broadcom iProc mdio mux driver.
Signed-off-by: Arun Parameswaran <[email protected]>
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 89c18d6..d1fce1e 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -26,6 +26,9 @@
#define MDIO_RATE_ADJ_INT_OFFSET 0x004
#define MDIO_RATE_ADJ_DIVIDENT_SHIFT 16
+#define MDIO_SCAN_CTRL_OFFSET 0x008
+#define MDIO_SCAN_CTRL_OVERRIDE_EXT_MSTR 28
+
#define MDIO_PARAM_OFFSET 0x23c
#define MDIO_PARAM_MIIM_CYCLE 29
#define MDIO_PARAM_INTERNAL_SEL 25
@@ -51,7 +54,7 @@
#define MDIO_OPERATING_FREQUENCY 11000000
#define MDIO_RATE_ADJ_DIVIDENT 1
-#define MDIO_NUM_OF_REGS_TO_RESTORE 2
+#define MDIO_NUM_OF_REGS_TO_RESTORE 3
struct iproc_mdiomux_desc {
void *mux_handle;
@@ -68,6 +71,7 @@ struct iproc_mdiomux_desc {
static const u16 restore_reg_offsets[MDIO_NUM_OF_REGS_TO_RESTORE] = {
MDIO_RATE_ADJ_EXT_OFFSET,
MDIO_RATE_ADJ_INT_OFFSET,
+ MDIO_SCAN_CTRL_OVERRIDE_EXT_MSTR,
};
#endif
--
1.9.1
Modify the base address passed to the Broadcom iProc MDIO mux driver
to point to the start of the block's register address space.
Fixes: ce8d5dbfd64f ("Add DT binding doc for Broadcom MDIO bus multiplexer")
Signed-off-by: Arun Parameswaran <[email protected]>
---
Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
index dfe287a..dc8aa68 100644
--- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
+++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
@@ -18,9 +18,9 @@ at- Documentation/devicetree/bindings/net/mdio-mux.txt
for example:
- mdio_mux_iproc: mdio-mux@6602023c {
+ mdio_mux_iproc: mdio-mux@66020000 {
compatible = "brcm,mdio-mux-iproc";
- reg = <0x6602023c 0x14>;
+ reg = <0x66020000 0x250>;
#address-cells = <1>;
#size-cells = <0>;
--
1.9.1
Add support for suspend and resume to the Broadcom iProc mdio
mux driver.
Signed-off-by: Arun Parameswaran <[email protected]>
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 42 ++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 6b400dd..89c18d6 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -51,13 +51,25 @@
#define MDIO_OPERATING_FREQUENCY 11000000
#define MDIO_RATE_ADJ_DIVIDENT 1
+#define MDIO_NUM_OF_REGS_TO_RESTORE 2
+
struct iproc_mdiomux_desc {
void *mux_handle;
void __iomem *base;
struct device *dev;
struct mii_bus *mii_bus;
struct clk *core_clk;
+#ifdef CONFIG_PM_SLEEP
+ u32 restore_regs[MDIO_NUM_OF_REGS_TO_RESTORE];
+#endif
+};
+
+#ifdef CONFIG_PM_SLEEP
+static const u16 restore_reg_offsets[MDIO_NUM_OF_REGS_TO_RESTORE] = {
+ MDIO_RATE_ADJ_EXT_OFFSET,
+ MDIO_RATE_ADJ_INT_OFFSET,
};
+#endif
static void mdio_mux_iproc_config_clk(struct iproc_mdiomux_desc *md)
{
@@ -258,6 +270,35 @@ static int mdio_mux_iproc_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int mdio_mux_iproc_suspend(struct device *dev)
+{
+ int i;
+ struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
+
+ for (i = 0; i < MDIO_NUM_OF_REGS_TO_RESTORE; i++)
+ md->restore_regs[i] = readl(md->base +
+ restore_reg_offsets[i]);
+
+ return 0;
+}
+
+static int mdio_mux_iproc_resume(struct device *dev)
+{
+ int i;
+ struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
+
+ for (i = 0; i < MDIO_NUM_OF_REGS_TO_RESTORE; i++)
+ writel(md->restore_regs[i],
+ md->base + restore_reg_offsets[i]);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(mdio_mux_iproc_pm_ops,
+ mdio_mux_iproc_suspend, mdio_mux_iproc_resume);
+
static const struct of_device_id mdio_mux_iproc_match[] = {
{
.compatible = "brcm,mdio-mux-iproc",
@@ -270,6 +311,7 @@ static int mdio_mux_iproc_remove(struct platform_device *pdev)
.driver = {
.name = "mdio-mux-iproc",
.of_match_table = mdio_mux_iproc_match,
+ .pm = &mdio_mux_iproc_pm_ops,
},
.probe = mdio_mux_iproc_probe,
.remove = mdio_mux_iproc_remove,
--
1.9.1
Modify the register offsets in the Broadcom iProc mdio mux to start
from the top of the register address space.
Earlier the base address specified was from the middle of the block's
register space. The base address will now point to the start of the
mdio's address space. The offsets have been fixed to match this.
Fixes: 98bc865a1ec8 ("net: mdio-mux: Add MDIO mux driver for iProc SoCs")
Signed-off-by: Arun Parameswaran <[email protected]>
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 0831b71..dc65e95 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -22,7 +22,7 @@
#include <linux/mdio-mux.h>
#include <linux/delay.h>
-#define MDIO_PARAM_OFFSET 0x00
+#define MDIO_PARAM_OFFSET 0x23c
#define MDIO_PARAM_MIIM_CYCLE 29
#define MDIO_PARAM_INTERNAL_SEL 25
#define MDIO_PARAM_BUS_ID 22
@@ -30,15 +30,15 @@
#define MDIO_PARAM_PHY_ID 16
#define MDIO_PARAM_PHY_DATA 0
-#define MDIO_READ_OFFSET 0x04
+#define MDIO_READ_OFFSET 0x240
#define MDIO_READ_DATA_MASK 0xffff
-#define MDIO_ADDR_OFFSET 0x08
+#define MDIO_ADDR_OFFSET 0x244
-#define MDIO_CTRL_OFFSET 0x0C
+#define MDIO_CTRL_OFFSET 0x248
#define MDIO_CTRL_WRITE_OP 0x1
#define MDIO_CTRL_READ_OP 0x2
-#define MDIO_STAT_OFFSET 0x10
+#define MDIO_STAT_OFFSET 0x24c
#define MDIO_STAT_DONE 1
#define BUS_MAX_ADDR 32
--
1.9.1
Add clock phandle, of the core clock driving the mdio block, as an
optional property to the Broadcom iProc mdio mux block.
The clock, when specified, will be used to setup the rate adjust registers
in the mdio to derrive the mdio's operating frequency.
Signed-off-by: Arun Parameswaran <[email protected]>
---
Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
index dc8aa68..b58843f 100644
--- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
+++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
@@ -13,6 +13,9 @@ MDIO multiplexer node:
Every non-ethernet PHY requires a compatible so that it could be probed based
on this compatible string.
+Optional properties:
+- clocks: phandle of the core clock which drives the mdio block.
+
Additional information regarding generic multiplexer properties can be found
at- Documentation/devicetree/bindings/net/mdio-mux.txt
--
1.9.1
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 <[email protected]>
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index dc65e95..6b400dd 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_PARAM_OFFSET 0x23c
#define MDIO_PARAM_MIIM_CYCLE 29
#define MDIO_PARAM_INTERNAL_SEL 25
@@ -44,13 +48,32 @@
#define BUS_MAX_ADDR 32
#define EXT_BUS_START_ADDR 16
+#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_clk(struct iproc_mdiomux_desc *md)
+{
+ u32 val;
+ u32 divisor;
+
+ if (md->core_clk) {
+ 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)
{
unsigned int timeout = 1000; /* loop for 1s */
@@ -175,6 +198,12 @@ 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)) {
+ dev_info(&pdev->dev, "core_clk not specified\n");
+ md->core_clk = NULL;
+ }
+
md->mii_bus = mdiobus_alloc();
if (!md->mii_bus) {
dev_err(&pdev->dev, "mdiomux bus alloc failed\n");
@@ -206,6 +235,8 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
goto out_register;
}
+ mdio_mux_iproc_config_clk(md);
+
dev_info(md->dev, "iProc mdiomux registered\n");
return 0;
--
1.9.1
Modify the base address of the mdio mux driver to point to the start of
the mdio mux block's register address space.
Fixes: fd898f75dac7 ("arm64: dts: Add MDIO multiplexer DT node for Stingray")
Signed-off-by: Arun Parameswaran <[email protected]>
---
arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi | 4 ++--
arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
index 4057197..1a406a7 100644
--- a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi
@@ -482,9 +482,9 @@
status = "disabled";
};
- mdio_mux_iproc: mdio-mux@6602023c {
+ mdio_mux_iproc: mdio-mux@66020000 {
compatible = "brcm,mdio-mux-iproc";
- reg = <0x6602023c 0x14>;
+ reg = <0x66020000 0x250>;
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index b203152..a70e8dd 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -278,9 +278,9 @@
#include "stingray-pinctrl.dtsi"
- mdio_mux_iproc: mdio-mux@2023c {
+ mdio_mux_iproc: mdio-mux@20000 {
compatible = "brcm,mdio-mux-iproc";
- reg = <0x0002023c 0x14>;
+ reg = <0x00020000 0x250>;
#address-cells = <1>;
#size-cells = <0>;
--
1.9.1
On Thu, Jul 26, 2018 at 11:36:17AM -0700, Arun Parameswaran wrote:
> Hi,
> The patchset is based on David Miller's "net" repo.
Hi Arun
"net" is intended for bug fixes. This looks like new features, so
should be against net-next". Please read the netdev FAQ.
Andrew
On 18-07-26 11:52 AM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:17AM -0700, Arun Parameswaran wrote:
>> Hi,
>> The patchset is based on David Miller's "net" repo.
>
> Hi Arun
>
> "net" is intended for bug fixes. This looks like new features, so
> should be against net-next". Please read the netdev FAQ.
>
> Andrew
>
Hi Andrew
The first three patches are 'fixes', and the subsequent patches
(which are new features) depend on the first 3 patches.
Since I had the 'fixes', and the dependency, I used the 'net'
repo.
Please advise if I should still use 'net-next' for all of them.
Thanks
Arun
On Thu, Jul 26, 2018 at 11:36:18AM -0700, Arun Parameswaran wrote:
> Modify the base address passed to the Broadcom iProc MDIO mux driver
> to point to the start of the block's register address space.
>
> Fixes: ce8d5dbfd64f ("Add DT binding doc for Broadcom MDIO bus multiplexer")
Hi Arun
Fixes generally means something explodes, throws an Opps, userspace
does the wrong thing. Documentation/process/stable-kernel-rules.rst
says:
- It must fix a real bug that bothers people (not a, "This could be a
problem..." type thing).
Please explain how this is a real problem and what people it bothers.
I also don't see any attempt to keep backwards compatibility with
older device tree blobs. Is it intentional you will break such old
blobs?
Thanks
Andrew
> Signed-off-by: Arun Parameswaran <[email protected]>
> ---
> Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
> index dfe287a..dc8aa68 100644
> --- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
> +++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
> @@ -18,9 +18,9 @@ at- Documentation/devicetree/bindings/net/mdio-mux.txt
>
>
> for example:
> - mdio_mux_iproc: mdio-mux@6602023c {
> + mdio_mux_iproc: mdio-mux@66020000 {
> compatible = "brcm,mdio-mux-iproc";
> - reg = <0x6602023c 0x14>;
> + reg = <0x66020000 0x250>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> --
> 1.9.1
>
On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
> Modify the register offsets in the Broadcom iProc mdio mux to start
> from the top of the register address space.
>
> Earlier the base address specified was from the middle of the block's
> register space. The base address will now point to the start of the
> mdio's address space. The offsets have been fixed to match this.
Hi Arun
Did you consider a change something like:
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 0831b7142df7..2d53e609498c 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -169,6 +169,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
md->dev = &pdev->dev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ if (res->start & 0xfff != 0) {
+ dev_info(&pdev->dev, "Please upgrade your device tree blob.\n");
+ res->start &= ~0xfff;
+ }
+
md->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(md->base)) {
dev_err(&pdev->dev, "failed to ioremap register\n");
Andrew
> @@ -175,6 +198,12 @@ 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)) {
> + dev_info(&pdev->dev, "core_clk not specified\n");
> + md->core_clk = NULL;
> + }
> +
In the binding, you say the clock is optional. This is a rather strong
message for something which is optional. I think it would be better to
not output anything.
Andrew
Hi Andrew
On 18-07-26 12:01 PM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:18AM -0700, Arun Parameswaran wrote:
>> Modify the base address passed to the Broadcom iProc MDIO mux driver
>> to point to the start of the block's register address space.
>>
>> Fixes: ce8d5dbfd64f ("Add DT binding doc for Broadcom MDIO bus multiplexer")
>
> Hi Arun
>
> Fixes generally means something explodes, throws an Opps, userspace
> does the wrong thing. Documentation/process/stable-kernel-rules.rst
> says:
>
> - It must fix a real bug that bothers people (not a, "This could be a
> problem..." type thing).
>
> Please explain how this is a real problem and what people it bothers.
>
It is only relevant when we need to modify the registers which are not
currently accessible.
I will remove the 'fixes' and use the 'net-next' repo for the change
set.
> I also don't see any attempt to keep backwards compatibility with
> older device tree blobs. Is it intentional you will break such old
> blobs?
>
I will modify the patch to keep backward compatibility like you
suggested in the other patch.
Thanks
Arun
> Thanks
> Andrew
>
>
>> Signed-off-by: Arun Parameswaran <[email protected]>
>> ---
>> Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
>> index dfe287a..dc8aa68 100644
>> --- a/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
>> +++ b/Documentation/devicetree/bindings/net/brcm,mdio-mux-iproc.txt
>> @@ -18,9 +18,9 @@ at- Documentation/devicetree/bindings/net/mdio-mux.txt
>>
>>
>> for example:
>> - mdio_mux_iproc: mdio-mux@6602023c {
>> + mdio_mux_iproc: mdio-mux@66020000 {
>> compatible = "brcm,mdio-mux-iproc";
>> - reg = <0x6602023c 0x14>;
>> + reg = <0x66020000 0x250>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> --
>> 1.9.1
>>
On 18-07-26 12:06 PM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
>> Modify the register offsets in the Broadcom iProc mdio mux to start
>> from the top of the register address space.
>>
>> Earlier the base address specified was from the middle of the block's
>> register space. The base address will now point to the start of the
>> mdio's address space. The offsets have been fixed to match this.
>
> Hi Arun
>
> Did you consider a change something like:
That looks good. I will make this change to the patch.
Thanks
Arun
>
> diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
> index 0831b7142df7..2d53e609498c 100644
> --- a/drivers/net/phy/mdio-mux-bcm-iproc.c
> +++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
> @@ -169,6 +169,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
> md->dev = &pdev->dev;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + if (res->start & 0xfff != 0) {
> + dev_info(&pdev->dev, "Please upgrade your device tree blob.\n");
> + res->start &= ~0xfff;
> + }
> +
> md->base = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(md->base)) {
> dev_err(&pdev->dev, "failed to ioremap register\n");
>
>
> Andrew
>
On Thu, Jul 26, 2018 at 11:36:23AM -0700, Arun Parameswaran wrote:
> Add support for suspend and resume to the Broadcom iProc mdio
> mux driver.
> +#ifdef CONFIG_PM_SLEEP
> +static int mdio_mux_iproc_suspend(struct device *dev)
> +{
> + int i;
> + struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
> +
> + for (i = 0; i < MDIO_NUM_OF_REGS_TO_RESTORE; i++)
> + md->restore_regs[i] = readl(md->base +
> + restore_reg_offsets[i]);
> +
> + return 0;
> +}
> +
> +static int mdio_mux_iproc_resume(struct device *dev)
> +{
> + int i;
> + struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
> +
> + for (i = 0; i < MDIO_NUM_OF_REGS_TO_RESTORE; i++)
> + writel(md->restore_regs[i],
> + md->base + restore_reg_offsets[i]);
> +
> + return 0;
> +}
Hi Arun
Would it not be simpler to just call mdio_mux_iproc_config_clk() on
resume?
Andrew
On 18-07-26 12:13 PM, Andrew Lunn wrote:
>> @@ -175,6 +198,12 @@ 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)) {
>> + dev_info(&pdev->dev, "core_clk not specified\n");
>> + md->core_clk = NULL;
>> + }
>> +
>
> In the binding, you say the clock is optional. This is a rather strong
> message for something which is optional. I think it would be better to
> not output anything.
>
> Andrew
>
Will remove the message.
Thanks
Arun
On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
> Add support for saving and restoring the 'scan control' register
> in the Broadcom iProc mdio mux driver.
Hi Arun
I don't see anything setting this register. So why is it necessary to
save it over suspend/resume?
Andrew
On 7/26/2018 12:16 PM, Arun Parameswaran wrote:
>
>
> On 18-07-26 12:06 PM, Andrew Lunn wrote:
>> On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
>>> Modify the register offsets in the Broadcom iProc mdio mux to start
>>> from the top of the register address space.
>>>
>>> Earlier the base address specified was from the middle of the block's
>>> register space. The base address will now point to the start of the
>>> mdio's address space. The offsets have been fixed to match this.
>>
>> Hi Arun
>>
>> Did you consider a change something like:
> That looks good. I will make this change to the patch.
>
> Thanks
> Arun
To make it backward compatible, then length of the resource also needs
to be adjusted from 0x14 to 0x250 in the driver?
Otherwise you will end up accessing areas out of 0x14 defined in old DT
that is not mapped?
Thanks,
Ray
>>
>> diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
>> index 0831b7142df7..2d53e609498c 100644
>> --- a/drivers/net/phy/mdio-mux-bcm-iproc.c
>> +++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
>> @@ -169,6 +169,12 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
>> md->dev = &pdev->dev;
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> + if (res->start & 0xfff != 0) {
>> + dev_info(&pdev->dev, "Please upgrade your device tree blob.\n");
>> + res->start &= ~0xfff;
>> + }
>> +
>> md->base = devm_ioremap_resource(&pdev->dev, res);
>> if (IS_ERR(md->base)) {
>> dev_err(&pdev->dev, "failed to ioremap register\n");
>>
>>
>> Andrew
>>
> +static void mdio_mux_iproc_config_clk(struct iproc_mdiomux_desc *md)
> +{
> + u32 val;
> + u32 divisor;
> +
> + if (md->core_clk) {
> + divisor = clk_get_rate(md->core_clk) / MDIO_OPERATING_FREQUENCY;
/**
* clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
* This is only valid once the clock source has been enabled.
* @clk: clock source
*/
unsigned long clk_get_rate(struct clk *clk);
It is generally good practice to call clk_prepare_enable() sometime
before clk_get_rate() to ensure the clock is ticking, and to show this
driver is making use of the clock, so it does not get turned off.
> + 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);
> + }
> +}
On Thu, Jul 26, 2018 at 12:25:24PM -0700, Ray Jui wrote:
>
>
> On 7/26/2018 12:16 PM, Arun Parameswaran wrote:
> >
> >
> >On 18-07-26 12:06 PM, Andrew Lunn wrote:
> >>On Thu, Jul 26, 2018 at 11:36:19AM -0700, Arun Parameswaran wrote:
> >>>Modify the register offsets in the Broadcom iProc mdio mux to start
> >>>from the top of the register address space.
> >>>
> >>>Earlier the base address specified was from the middle of the block's
> >>>register space. The base address will now point to the start of the
> >>>mdio's address space. The offsets have been fixed to match this.
> >>
> >>Hi Arun
> >>
> >>Did you consider a change something like:
> >That looks good. I will make this change to the patch.
> >
> >Thanks
> >Arun
>
> To make it backward compatible, then length of the resource also needs to be
> adjusted from 0x14 to 0x250 in the driver?
>
> Otherwise you will end up accessing areas out of 0x14 defined in old DT that
> is not mapped?
struct resource {
resource_size_t start;
resource_size_t end;
const char *name;
unsigned long flags;
unsigned long desc;
struct resource *parent, *sibling, *child;
};
"end" suggests an address, not a length. But it would be good to look
deeper into the code to be sure.
Andrew
On 18-07-26 12:20 PM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
>> Add support for saving and restoring the 'scan control' register
>> in the Broadcom iProc mdio mux driver.
>
> Hi Arun
>
> I don't see anything setting this register. So why is it necessary to
> save it over suspend/resume?
>
> Andrew
>
Hi Andrew
In the Omega SoC (only), this register was defaulted incorrectly (to use
a test mode) by the hardware. So we added the setup of the register to
the early bootloader (ATF).
We still need to restore this register while resuming from sleep.
Thanks
Arun
On Thu, Jul 26, 2018 at 12:33:05PM -0700, Arun Parameswaran wrote:
>
>
> On 18-07-26 12:20 PM, Andrew Lunn wrote:
> > On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
> >> Add support for saving and restoring the 'scan control' register
> >> in the Broadcom iProc mdio mux driver.
> >
> > Hi Arun
> >
> > I don't see anything setting this register. So why is it necessary to
> > save it over suspend/resume?
> >
> > Andrew
> >
> Hi Andrew
> In the Omega SoC (only), this register was defaulted incorrectly (to use
> a test mode) by the hardware. So we added the setup of the register to
> the early bootloader (ATF).
>
> We still need to restore this register while resuming from sleep.
Hi Arun
It is better to not rely on the bootloader and have the driver do it.
That makes it easier for people to swap to alternative bootloaders.
Andrew
Hi Andrew,
On 18-07-26 12:36 PM, Andrew Lunn wrote:
> On Thu, Jul 26, 2018 at 12:33:05PM -0700, Arun Parameswaran wrote:
>>
>>
>> On 18-07-26 12:20 PM, Andrew Lunn wrote:
>>> On Thu, Jul 26, 2018 at 11:36:24AM -0700, Arun Parameswaran wrote:
>>>> Add support for saving and restoring the 'scan control' register
>>>> in the Broadcom iProc mdio mux driver.
>>>
>>> Hi Arun
>>>
>>> I don't see anything setting this register. So why is it necessary to
>>> save it over suspend/resume?
>>>
>>> Andrew
>>>
>> Hi Andrew
>> In the Omega SoC (only), this register was defaulted incorrectly (to use
>> a test mode) by the hardware. So we added the setup of the register to
>> the early bootloader (ATF).
>>
>> We still need to restore this register while resuming from sleep.
>
> Hi Arun
>
> It is better to not rely on the bootloader and have the driver do it.
> That makes it easier for people to swap to alternative bootloaders.
>
> Andrew
>
I will make the change to set the register in the driver.
Thanks
Arun
Hi Andrew
On 18-07-26 12:26 PM, Andrew Lunn wrote:
>> +static void mdio_mux_iproc_config_clk(struct iproc_mdiomux_desc *md)
>> +{
>> + u32 val;
>> + u32 divisor;
>> +
>> + if (md->core_clk) {
>> + divisor = clk_get_rate(md->core_clk) / MDIO_OPERATING_FREQUENCY;
>
> /**
> * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
> * This is only valid once the clock source has been enabled.
> * @clk: clock source
> */
> unsigned long clk_get_rate(struct clk *clk);
>
> It is generally good practice to call clk_prepare_enable() sometime
> before clk_get_rate() to ensure the clock is ticking, and to show this
> driver is making use of the clock, so it does not get turned off.
Will add 'clk_prepare_enable()' to the probe.
Thanks
Arun
>
>> + 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);
>> + }
>> +}