On FU540, the management IP block is tightly coupled with the Cadence
MACB IP block. It manages many of the boundary signals from the MACB IP
This patchset controls the tx_clk input signal to the MACB IP. It
switches between the local TX clock (125MHz) and PHY TX clocks. This
is necessary to toggle between 1Gb and 100/10Mb speeds.
Future patches may add support for monitoring or controlling other IP
boundary signals.
This patchset is mostly based on work done by
Wesley Terpstra <[email protected]>
This patchset is based on Linux v5.2-rc1 and tested on HiFive Unleashed
board with additional board related patches needed for testing can be
found at dev/yashs/ethernet branch of:
https://github.com/yashshah7/riscv-linux.git
Yash Shah (2):
net/macb: bindings doc: add sifive fu540-c000 binding
net: macb: Add support for SiFive FU540-C000
Documentation/devicetree/bindings/net/macb.txt | 3 +
drivers/net/ethernet/cadence/macb_main.c | 118 +++++++++++++++++++++++++
2 files changed, 121 insertions(+)
--
1.9.1
Add the compatibility string documentation for SiFive FU540-C0000
interface.
On the FU540, this driver also needs to read and write registers in a
management IP block that monitors or drives boundary signals for the
GEMGXL IP block that are not directly mapped to GEMGXL registers.
Therefore, add additional range to "reg" property for SiFive GEMGXL
management IP registers.
Signed-off-by: Yash Shah <[email protected]>
---
Documentation/devicetree/bindings/net/macb.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 9c5e944..91a2a66 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible: Should be "cdns,[<chip>-]{macb|gem}"
Use "cdns,at91rm9200-emac" Atmel at91rm9200 SoC.
Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs.
+ Use "cdns,fu540-macb" for SiFive FU540-C000 SoC.
Use "cdns,sam9x60-macb" for Microchip sam9x60 SoC.
Use "cdns,np4-macb" for NP4 SoC devices.
Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
@@ -17,6 +18,8 @@ Required properties:
Use "cdns,zynqmp-gem" for Zynq Ultrascale+ MPSoC.
Or the generic form: "cdns,emac".
- reg: Address and length of the register set for the device
+ For "cdns,fu540-macb", second range is required to specify the
+ address and length of the registers for GEMGXL Management block.
- interrupts: Should contain macb interrupt
- phy-mode: See ethernet.txt file in the same directory.
- clock-names: Tuple listing input clock names.
--
1.9.1
The management IP block is tightly coupled with the Cadence MACB IP
block on the FU540, and manages many of the boundary signals from the
MACB IP. This patch only controls the tx_clk input signal to the MACB
IP. Future patches may add support for monitoring or controlling other
IP boundary signals.
Signed-off-by: Yash Shah <[email protected]>
---
drivers/net/ethernet/cadence/macb_main.c | 118 +++++++++++++++++++++++++++++++
1 file changed, 118 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c049410..a9e5227 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -10,6 +10,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/clk.h>
+#include <linux/clk-provider.h>
#include <linux/crc32.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
@@ -40,6 +41,15 @@
#include <linux/pm_runtime.h>
#include "macb.h"
+/* This structure is only used for MACB on SiFive FU540 devices */
+struct sifive_fu540_macb_mgmt {
+ void __iomem *reg;
+ unsigned long rate;
+ struct clk_hw hw;
+};
+
+static struct sifive_fu540_macb_mgmt *mgmt;
+
#define MACB_RX_BUFFER_SIZE 128
#define RX_BUFFER_MULTIPLE 64 /* bytes */
@@ -3903,6 +3913,113 @@ static int at91ether_init(struct platform_device *pdev)
return 0;
}
+static unsigned long fu540_macb_tx_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return mgmt->rate;
+}
+
+static long fu540_macb_tx_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ if (WARN_ON(rate < 2500000))
+ return 2500000;
+ else if (rate == 2500000)
+ return 2500000;
+ else if (WARN_ON(rate < 13750000))
+ return 2500000;
+ else if (WARN_ON(rate < 25000000))
+ return 25000000;
+ else if (rate == 25000000)
+ return 25000000;
+ else if (WARN_ON(rate < 75000000))
+ return 25000000;
+ else if (WARN_ON(rate < 125000000))
+ return 125000000;
+ else if (rate == 125000000)
+ return 125000000;
+
+ WARN_ON(rate > 125000000);
+
+ return 125000000;
+}
+
+static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate);
+ iowrite32(rate != 125000000, mgmt->reg);
+ mgmt->rate = rate;
+
+ return 0;
+}
+
+static const struct clk_ops fu540_c000_ops = {
+ .recalc_rate = fu540_macb_tx_recalc_rate,
+ .round_rate = fu540_macb_tx_round_rate,
+ .set_rate = fu540_macb_tx_set_rate,
+};
+
+static int fu540_c000_clk_init(struct platform_device *pdev, struct clk **pclk,
+ struct clk **hclk, struct clk **tx_clk,
+ struct clk **rx_clk, struct clk **tsu_clk)
+{
+ struct clk_init_data init;
+ int err = 0;
+
+ err = macb_clk_init(pdev, pclk, hclk, tx_clk, rx_clk, tsu_clk);
+ if (err)
+ return err;
+
+ mgmt = devm_kzalloc(&pdev->dev, sizeof(*mgmt), GFP_KERNEL);
+ if (!mgmt)
+ return -ENOMEM;
+
+ init.name = "sifive-gemgxl-mgmt";
+ init.ops = &fu540_c000_ops;
+ init.flags = 0;
+ init.num_parents = 0;
+
+ mgmt->rate = 0;
+ mgmt->hw.init = &init;
+
+ *tx_clk = clk_register(NULL, &mgmt->hw);
+ if (IS_ERR(*tx_clk))
+ return PTR_ERR(*tx_clk);
+
+ err = clk_prepare_enable(*tx_clk);
+ if (err)
+ dev_err(&pdev->dev, "failed to enable tx_clk (%u)\n", err);
+ else
+ dev_info(&pdev->dev, "Registered clk switch '%s'\n", init.name);
+
+ return 0;
+}
+
+static int fu540_c000_init(struct platform_device *pdev)
+{
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!res)
+ return -ENODEV;
+
+ mgmt->reg = ioremap(res->start, resource_size(res));
+ if (!mgmt->reg)
+ return -ENOMEM;
+
+ return macb_init(pdev);
+}
+
+static const struct macb_config fu540_c000_config = {
+ .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
+ MACB_CAPS_GEM_HAS_PTP,
+ .dma_burst_length = 16,
+ .clk_init = fu540_c000_clk_init,
+ .init = fu540_c000_init,
+ .jumbo_max_len = 10240,
+};
+
static const struct macb_config at91sam9260_config = {
.caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
.clk_init = macb_clk_init,
@@ -3980,6 +4097,7 @@ static int at91ether_init(struct platform_device *pdev)
{ .compatible = "cdns,at32ap7000-macb" },
{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
{ .compatible = "cdns,macb" },
+ { .compatible = "cdns,fu540-macb", .data = &fu540_c000_config },
{ .compatible = "cdns,np4-macb", .data = &np4_config },
{ .compatible = "cdns,pc302-gem", .data = &pc302gem_config },
{ .compatible = "cdns,gem", .data = &pc302gem_config },
--
1.9.1
On Mai 23 2019, Yash Shah <[email protected]> wrote:
> On FU540, the management IP block is tightly coupled with the Cadence
> MACB IP block. It manages many of the boundary signals from the MACB IP
> This patchset controls the tx_clk input signal to the MACB IP. It
> switches between the local TX clock (125MHz) and PHY TX clocks. This
> is necessary to toggle between 1Gb and 100/10Mb speeds.
Doesn't work for me:
[ 365.842801] macb: probe of 10090000.ethernet failed with error -17
Andreas.
--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
> +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate);
> + iowrite32(rate != 125000000, mgmt->reg);
That looks odd. Writing the result of a comparison to a register?
Andrew
Please be consistent in your subsystem prefixes used in your Subject lines.
You use "net: macb:" then "net/macb:" Really, plain "macb: " is sufficient.
Thank you.
On Thu, May 23, 2019 at 6:46 AM Yash Shah <[email protected]> wrote:
>
> Add the compatibility string documentation for SiFive FU540-C0000
> interface.
> On the FU540, this driver also needs to read and write registers in a
> management IP block that monitors or drives boundary signals for the
> GEMGXL IP block that are not directly mapped to GEMGXL registers.
> Therefore, add additional range to "reg" property for SiFive GEMGXL
> management IP registers.
>
> Signed-off-by: Yash Shah <[email protected]>
> ---
> Documentation/devicetree/bindings/net/macb.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> index 9c5e944..91a2a66 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -4,6 +4,7 @@ Required properties:
> - compatible: Should be "cdns,[<chip>-]{macb|gem}"
> Use "cdns,at91rm9200-emac" Atmel at91rm9200 SoC.
> Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs.
> + Use "cdns,fu540-macb" for SiFive FU540-C000 SoC.
This pattern that Atmel started isn't really correct. The vendor
prefix here should be sifive. 'cdns' would be appropriate for a
fallback.
> Use "cdns,sam9x60-macb" for Microchip sam9x60 SoC.
> Use "cdns,np4-macb" for NP4 SoC devices.
> Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
> @@ -17,6 +18,8 @@ Required properties:
> Use "cdns,zynqmp-gem" for Zynq Ultrascale+ MPSoC.
> Or the generic form: "cdns,emac".
> - reg: Address and length of the register set for the device
> + For "cdns,fu540-macb", second range is required to specify the
> + address and length of the registers for GEMGXL Management block.
> - interrupts: Should contain macb interrupt
> - phy-mode: See ethernet.txt file in the same directory.
> - clock-names: Tuple listing input clock names.
> --
> 1.9.1
>
Hi Andreas,
On Thu, May 23, 2019 at 6:19 PM Andreas Schwab <[email protected]> wrote:
>
> On Mai 23 2019, Yash Shah <[email protected]> wrote:
>
> > On FU540, the management IP block is tightly coupled with the Cadence
> > MACB IP block. It manages many of the boundary signals from the MACB IP
> > This patchset controls the tx_clk input signal to the MACB IP. It
> > switches between the local TX clock (125MHz) and PHY TX clocks. This
> > is necessary to toggle between 1Gb and 100/10Mb speeds.
>
> Doesn't work for me:
>
> [ 365.842801] macb: probe of 10090000.ethernet failed with error -17
>
Make sure you have applied all the patches needed for testing found at
dev/yashs/ethernet branch of:
https://github.com/yashshah7/riscv-linux.git
In addition to that, make sure in your kernel config GPIO_SIFIVE=y
In v2 of this patch, I will add this select GPIO_SIFIVE config in the
Cadence Kconfig file.
- Yash
On Thu, May 23, 2019 at 8:24 PM Andrew Lunn <[email protected]> wrote:
>
> > +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate);
> > + iowrite32(rate != 125000000, mgmt->reg);
>
> That looks odd. Writing the result of a comparison to a register?
The idea was to write "1" to the register if the value of rate is
anything else than 125000000.
To make it easier to read, I will change this to below:
- iowrite32(rate != 125000000, mgmt->reg);
+ if (rate != 125000000)
+ iowrite32(1, mgmt->reg);
+ else
+ iowrite32(0, mgmt->reg);
Hope that's fine. Thanks for your comment
- Yash
>
> Andrew
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, May 23, 2019 at 9:58 PM David Miller <[email protected]> wrote:
>
>
> Please be consistent in your subsystem prefixes used in your Subject lines.
> You use "net: macb:" then "net/macb:" Really, plain "macb: " is sufficient.
Sure, Will take care of this in the next revision of this patch.
Thanks for your comment.
- Yash
On Fri, May 24, 2019 at 2:20 AM Rob Herring <[email protected]> wrote:
>
> On Thu, May 23, 2019 at 6:46 AM Yash Shah <[email protected]> wrote:
> >
> > Add the compatibility string documentation for SiFive FU540-C0000
> > interface.
> > On the FU540, this driver also needs to read and write registers in a
> > management IP block that monitors or drives boundary signals for the
> > GEMGXL IP block that are not directly mapped to GEMGXL registers.
> > Therefore, add additional range to "reg" property for SiFive GEMGXL
> > management IP registers.
> >
> > Signed-off-by: Yash Shah <[email protected]>
> > ---
> > Documentation/devicetree/bindings/net/macb.txt | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> > index 9c5e944..91a2a66 100644
> > --- a/Documentation/devicetree/bindings/net/macb.txt
> > +++ b/Documentation/devicetree/bindings/net/macb.txt
> > @@ -4,6 +4,7 @@ Required properties:
> > - compatible: Should be "cdns,[<chip>-]{macb|gem}"
> > Use "cdns,at91rm9200-emac" Atmel at91rm9200 SoC.
> > Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs.
> > + Use "cdns,fu540-macb" for SiFive FU540-C000 SoC.
>
> This pattern that Atmel started isn't really correct. The vendor
> prefix here should be sifive. 'cdns' would be appropriate for a
> fallback.
Ok sure. WIll change it to "sifive,fu540-macb"
Thanks for your comment.
- Yash
On Fri, May 24, 2019 at 10:22:06AM +0530, Yash Shah wrote:
> On Thu, May 23, 2019 at 8:24 PM Andrew Lunn <[email protected]> wrote:
> >
> > > +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate,
> > > + unsigned long parent_rate)
> > > +{
> > > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate);
> > > + iowrite32(rate != 125000000, mgmt->reg);
> >
> > That looks odd. Writing the result of a comparison to a register?
>
> The idea was to write "1" to the register if the value of rate is
> anything else than 125000000.
I'm not a language lawyer. Is it guaranteed that an expression like
this returns 1? Any value !0 is true, so maybe it actually returns 42?
> To make it easier to read, I will change this to below:
> - iowrite32(rate != 125000000, mgmt->reg);
> + if (rate != 125000000)
> + iowrite32(1, mgmt->reg);
> + else
> + iowrite32(0, mgmt->reg);
>
> Hope that's fine. Thanks for your comment
Yes, that is good.
Andrew
On Mai 24 2019, Yash Shah <[email protected]> wrote:
> Hi Andreas,
>
> On Thu, May 23, 2019 at 6:19 PM Andreas Schwab <[email protected]> wrote:
>>
>> On Mai 23 2019, Yash Shah <[email protected]> wrote:
>>
>> > On FU540, the management IP block is tightly coupled with the Cadence
>> > MACB IP block. It manages many of the boundary signals from the MACB IP
>> > This patchset controls the tx_clk input signal to the MACB IP. It
>> > switches between the local TX clock (125MHz) and PHY TX clocks. This
>> > is necessary to toggle between 1Gb and 100/10Mb speeds.
>>
>> Doesn't work for me:
>>
>> [ 365.842801] macb: probe of 10090000.ethernet failed with error -17
>>
>
> Make sure you have applied all the patches needed for testing found at
> dev/yashs/ethernet branch of:
Nope, try reloading the module.
Andreas.
--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
On Mon, May 27, 2019 at 1:34 PM Andreas Schwab <[email protected]> wrote:
>
> On Mai 24 2019, Yash Shah <[email protected]> wrote:
>
> > Hi Andreas,
> >
> > On Thu, May 23, 2019 at 6:19 PM Andreas Schwab <[email protected]> wrote:
> >>
> >> On Mai 23 2019, Yash Shah <[email protected]> wrote:
> >>
> >> > On FU540, the management IP block is tightly coupled with the Cadence
> >> > MACB IP block. It manages many of the boundary signals from the MACB IP
> >> > This patchset controls the tx_clk input signal to the MACB IP. It
> >> > switches between the local TX clock (125MHz) and PHY TX clocks. This
> >> > is necessary to toggle between 1Gb and 100/10Mb speeds.
> >>
> >> Doesn't work for me:
> >>
> >> [ 365.842801] macb: probe of 10090000.ethernet failed with error -17
> >>
> >
> > Make sure you have applied all the patches needed for testing found at
> > dev/yashs/ethernet branch of:
>
> Nope, try reloading the module.
Yes, I could see the error on reloading the module.
Thanks for the catch. I will fix this in the next version of this patch.
- Yash
On Fri, 24 May 2019 06:48:47 PDT (-0700), [email protected] wrote:
> On Fri, May 24, 2019 at 10:22:06AM +0530, Yash Shah wrote:
>> On Thu, May 23, 2019 at 8:24 PM Andrew Lunn <[email protected]> wrote:
>> >
>> > > +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate,
>> > > + unsigned long parent_rate)
>> > > +{
>> > > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate);
>> > > + iowrite32(rate != 125000000, mgmt->reg);
>> >
>> > That looks odd. Writing the result of a comparison to a register?
>>
>> The idea was to write "1" to the register if the value of rate is
>> anything else than 125000000.
>
> I'm not a language lawyer. Is it guaranteed that an expression like
> this returns 1? Any value !0 is true, so maybe it actually returns 42?
From Stack Overflow: https://stackoverflow.com/questions/18097922/return-value-of-operator-in-c
"C11(ISO/IEC 9899:201x) ยง6.5.8 Relational operators
Each of the operators < (less than), > (greater than), <= (less than or equal
to), and >= (greater than or equal to) shall yield 1 if the specified relation
is true and 0 if it is false. The result has type int."
>> To make it easier to read, I will change this to below:
>> - iowrite32(rate != 125000000, mgmt->reg);
>> + if (rate != 125000000)
>> + iowrite32(1, mgmt->reg);
>> + else
>> + iowrite32(0, mgmt->reg);
>>
>> Hope that's fine. Thanks for your comment
>
> Yes, that is good.
>
> Andrew
On 23/05/2019 at 22:50, Rob Herring wrote:
> On Thu, May 23, 2019 at 6:46 AM Yash Shah <[email protected]> wrote:
>>
>> Add the compatibility string documentation for SiFive FU540-C0000
>> interface.
>> On the FU540, this driver also needs to read and write registers in a
>> management IP block that monitors or drives boundary signals for the
>> GEMGXL IP block that are not directly mapped to GEMGXL registers.
>> Therefore, add additional range to "reg" property for SiFive GEMGXL
>> management IP registers.
>>
>> Signed-off-by: Yash Shah <[email protected]>
>> ---
>> Documentation/devicetree/bindings/net/macb.txt | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
>> index 9c5e944..91a2a66 100644
>> --- a/Documentation/devicetree/bindings/net/macb.txt
>> +++ b/Documentation/devicetree/bindings/net/macb.txt
>> @@ -4,6 +4,7 @@ Required properties:
>> - compatible: Should be "cdns,[<chip>-]{macb|gem}"
>> Use "cdns,at91rm9200-emac" Atmel at91rm9200 SoC.
>> Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs.
>> + Use "cdns,fu540-macb" for SiFive FU540-C000 SoC.
>
> This pattern that Atmel started isn't really correct. The vendor
> prefix here should be sifive. 'cdns' would be appropriate for a
> fallback.
Ok, we missed this for the sam9x60 SoC that we added recently then.
Anyway a little too late, coming back to this machine, and talking to
Yash, isn't "sifive,fu540-c000-macb" more specific and a better match
for being future proof? I would advice for the most specific possible
with other compatible strings on the same line in the DT, like:
"sifive,fu540-c000-macb", "sifive,fu540-macb"
Moreover, is it really a "macb" or a "gem" type of interface from
Cadence? Not a big deal, but just to discuss the topic to the bone...
Note that I'm fine if you consider that what you have in net-next new is
correct.
Regards,
Nicolas
>> Use "cdns,sam9x60-macb" for Microchip sam9x60 SoC.
>> Use "cdns,np4-macb" for NP4 SoC devices.
>> Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
>> @@ -17,6 +18,8 @@ Required properties:
>> Use "cdns,zynqmp-gem" for Zynq Ultrascale+ MPSoC.
>> Or the generic form: "cdns,emac".
>> - reg: Address and length of the register set for the device
>> + For "cdns,fu540-macb", second range is required to specify the
>> + address and length of the registers for GEMGXL Management block.
>> - interrupts: Should contain macb interrupt
>> - phy-mode: See ethernet.txt file in the same directory.
>> - clock-names: Tuple listing input clock names.
>> --
>> 1.9.1
>>
>
--
Nicolas Ferre
On Mon, Jun 24, 2019 at 9:08 PM <[email protected]> wrote:
>
> On 23/05/2019 at 22:50, Rob Herring wrote:
> > On Thu, May 23, 2019 at 6:46 AM Yash Shah <[email protected]> wrote:
> >>
> >> Add the compatibility string documentation for SiFive FU540-C0000
> >> interface.
> >> On the FU540, this driver also needs to read and write registers in a
> >> management IP block that monitors or drives boundary signals for the
> >> GEMGXL IP block that are not directly mapped to GEMGXL registers.
> >> Therefore, add additional range to "reg" property for SiFive GEMGXL
> >> management IP registers.
> >>
> >> Signed-off-by: Yash Shah <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/net/macb.txt | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> >> index 9c5e944..91a2a66 100644
> >> --- a/Documentation/devicetree/bindings/net/macb.txt
> >> +++ b/Documentation/devicetree/bindings/net/macb.txt
> >> @@ -4,6 +4,7 @@ Required properties:
> >> - compatible: Should be "cdns,[<chip>-]{macb|gem}"
> >> Use "cdns,at91rm9200-emac" Atmel at91rm9200 SoC.
> >> Use "cdns,at91sam9260-macb" for Atmel at91sam9 SoCs.
> >> + Use "cdns,fu540-macb" for SiFive FU540-C000 SoC.
> >
> > This pattern that Atmel started isn't really correct. The vendor
> > prefix here should be sifive. 'cdns' would be appropriate for a
> > fallback.
>
> Ok, we missed this for the sam9x60 SoC that we added recently then.
>
> Anyway a little too late, coming back to this machine, and talking to
> Yash, isn't "sifive,fu540-c000-macb" more specific and a better match
> for being future proof? I would advice for the most specific possible
> with other compatible strings on the same line in the DT, like:
>
> "sifive,fu540-c000-macb", "sifive,fu540-macb"
>
Yes, I agree that "sifive,fu540-c000-macb" is a better match.
> Moreover, is it really a "macb" or a "gem" type of interface from
> Cadence? Not a big deal, but just to discuss the topic to the bone...
I believe it should be "gem". I will plan to submit the patch for
these changes. Thanks for pointing it out.
- Yash
>
> Note that I'm fine if you consider that what you have in net-next new is
> correct.
>
> Regards,
> Nicolas
>
> >> Use "cdns,sam9x60-macb" for Microchip sam9x60 SoC.
> >> Use "cdns,np4-macb" for NP4 SoC devices.
> >> Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
> >> @@ -17,6 +18,8 @@ Required properties:
> >> Use "cdns,zynqmp-gem" for Zynq Ultrascale+ MPSoC.
> >> Or the generic form: "cdns,emac".
> >> - reg: Address and length of the register set for the device
> >> + For "cdns,fu540-macb", second range is required to specify the
> >> + address and length of the registers for GEMGXL Management block.
> >> - interrupts: Should contain macb interrupt
> >> - phy-mode: See ethernet.txt file in the same directory.
> >> - clock-names: Tuple listing input clock names.
> >> --
> >> 1.9.1
> >>
> >
>
>
> --
> Nicolas Ferre