Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/2] Add macb support for SiFive FU540-C000
@ 2019-06-17  4:19 Yash Shah
  2019-06-17  4:19 ` [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding Yash Shah
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Yash Shah @ 2019-06-17  4:19 UTC (permalink / raw)
  To: davem, devicetree, netdev, linux-kernel, linux-riscv
  Cc: robh+dt, mark.rutland, nicolas.ferre, palmer, aou, paul.walmsley,
	ynezz, sachin.ghadi, Yash Shah

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 <wesley@sifive.com>

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

Change History:
V2:
- Change compatible string from "cdns,fu540-macb" to "sifive,fu540-macb"
- Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
  driver. This is needed because on FU540, the macb driver depends on
  SiFive GPIO driver.
- Avoid writing the result of a comparison to a register.
- Fix the issue of probe fail on reloading the module reported by:
  Andreas Schwab <schwab@suse.de>

Yash Shah (2):
  macb: bindings doc: add sifive fu540-c000 binding
  macb: Add support for SiFive FU540-C000

 Documentation/devicetree/bindings/net/macb.txt |   3 +
 drivers/net/ethernet/cadence/Kconfig           |   6 ++
 drivers/net/ethernet/cadence/macb_main.c       | 129 +++++++++++++++++++++++++
 3 files changed, 138 insertions(+)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding
  2019-06-17  4:19 [PATCH v2 0/2] Add macb support for SiFive FU540-C000 Yash Shah
@ 2019-06-17  4:19 ` Yash Shah
  2019-06-17  8:56   ` Paul Walmsley
  2019-06-17  4:19 ` [PATCH v2 2/2] macb: Add support for SiFive FU540-C000 Yash Shah
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Yash Shah @ 2019-06-17  4:19 UTC (permalink / raw)
  To: davem, devicetree, netdev, linux-kernel, linux-riscv
  Cc: robh+dt, mark.rutland, nicolas.ferre, palmer, aou, paul.walmsley,
	ynezz, sachin.ghadi, Yash Shah

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 <yash.shah@sifive.com>
---
 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..63c73fa 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -15,8 +15,11 @@ Required properties:
   Use "atmel,sama5d4-gem" for the GEM IP (10/100) available on Atmel sama5d4 SoCs.
   Use "cdns,zynq-gem" Xilinx Zynq-7xxx SoC.
   Use "cdns,zynqmp-gem" for Zynq Ultrascale+ MPSoC.
+  Use "sifive,fu540-macb" for SiFive FU540-C000 SoC.
   Or the generic form: "cdns,emac".
 - reg: Address and length of the register set for the device
+	For "sifive,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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 2/2] macb: Add support for SiFive FU540-C000
  2019-06-17  4:19 [PATCH v2 0/2] Add macb support for SiFive FU540-C000 Yash Shah
  2019-06-17  4:19 ` [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding Yash Shah
@ 2019-06-17  4:19 ` Yash Shah
  2019-06-17 15:58   ` Andrew Lunn
  2019-06-17  4:43 ` [PATCH v2 0/2] Add macb " Yash Shah
  2019-06-17  8:25 ` Andreas Schwab
  3 siblings, 1 reply; 24+ messages in thread
From: Yash Shah @ 2019-06-17  4:19 UTC (permalink / raw)
  To: davem, devicetree, netdev, linux-kernel, linux-riscv
  Cc: robh+dt, mark.rutland, nicolas.ferre, palmer, aou, paul.walmsley,
	ynezz, sachin.ghadi, Yash Shah

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 <yash.shah@sifive.com>
---
 drivers/net/ethernet/cadence/Kconfig     |   6 ++
 drivers/net/ethernet/cadence/macb_main.c | 129 +++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index b998401..d478fae 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -48,4 +48,10 @@ config MACB_PCI
 	  To compile this driver as a module, choose M here: the module
 	  will be called macb_pci.
 
+config MACB_SIFIVE_FU540
+	bool "Cadence MACB/GEM support for SiFive FU540 SoC"
+	depends on MACB && GPIO_SIFIVE
+	help
+	  Enable the Cadence MACB/GEM support for SiFive FU540 SoC.
+
 endif # NET_VENDOR_CADENCE
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c049410..275b5e8 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,116 @@ 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);
+	if (rate != 125000000)
+		iowrite32(1, mgmt->reg);
+	else
+		iowrite32(0, 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,
@@ -3992,6 +4112,9 @@ static int at91ether_init(struct platform_device *pdev)
 	{ .compatible = "cdns,emac", .data = &emac_config },
 	{ .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config},
 	{ .compatible = "cdns,zynq-gem", .data = &zynq_config },
+#ifdef CONFIG_MACB_SIFIVE_FU540
+	{ .compatible = "sifive,fu540-macb", .data = &fu540_c000_config },
+#endif
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, macb_dt_ids);
@@ -4199,6 +4322,9 @@ static int macb_probe(struct platform_device *pdev)
 
 err_disable_clocks:
 	clk_disable_unprepare(tx_clk);
+#ifdef CONFIG_MACB_SIFIVE_FU540
+	clk_unregister(tx_clk);
+#endif
 	clk_disable_unprepare(hclk);
 	clk_disable_unprepare(pclk);
 	clk_disable_unprepare(rx_clk);
@@ -4233,6 +4359,9 @@ static int macb_remove(struct platform_device *pdev)
 		pm_runtime_dont_use_autosuspend(&pdev->dev);
 		if (!pm_runtime_suspended(&pdev->dev)) {
 			clk_disable_unprepare(bp->tx_clk);
+#ifdef CONFIG_MACB_SIFIVE_FU540
+			clk_unregister(bp->tx_clk);
+#endif
 			clk_disable_unprepare(bp->hclk);
 			clk_disable_unprepare(bp->pclk);
 			clk_disable_unprepare(bp->rx_clk);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17  4:19 [PATCH v2 0/2] Add macb support for SiFive FU540-C000 Yash Shah
  2019-06-17  4:19 ` [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding Yash Shah
  2019-06-17  4:19 ` [PATCH v2 2/2] macb: Add support for SiFive FU540-C000 Yash Shah
@ 2019-06-17  4:43 ` " Yash Shah
  2019-06-17  8:25 ` Andreas Schwab
  3 siblings, 0 replies; 24+ messages in thread
From: Yash Shah @ 2019-06-17  4:43 UTC (permalink / raw)
  To: David Miller, devicetree, netdev, linux-kernel, linux-riscv
  Cc: Rob Herring, Mark Rutland, Nicolas Ferre, Palmer Dabbelt,
	Albert Ou, Paul Walmsley, Petr Štetiar, Sachin Ghadi

On Mon, Jun 17, 2019 at 9:49 AM Yash Shah <yash.shah@sifive.com> 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.
>
> Future patches may add support for monitoring or controlling other IP
> boundary signals.
>
> This patchset is mostly based on work done by
> Wesley Terpstra <wesley@sifive.com>
>
> 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:

Correction in branch name: dev/yashs/ethernet_v2

> https://github.com/yashshah7/riscv-linux.git
>
> Change History:
> V2:
> - Change compatible string from "cdns,fu540-macb" to "sifive,fu540-macb"
> - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
>   driver. This is needed because on FU540, the macb driver depends on
>   SiFive GPIO driver.
> - Avoid writing the result of a comparison to a register.
> - Fix the issue of probe fail on reloading the module reported by:
>   Andreas Schwab <schwab@suse.de>
>
> Yash Shah (2):
>   macb: bindings doc: add sifive fu540-c000 binding
>   macb: Add support for SiFive FU540-C000
>
>  Documentation/devicetree/bindings/net/macb.txt |   3 +
>  drivers/net/ethernet/cadence/Kconfig           |   6 ++
>  drivers/net/ethernet/cadence/macb_main.c       | 129 +++++++++++++++++++++++++
>  3 files changed, 138 insertions(+)
>
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17  4:19 [PATCH v2 0/2] Add macb support for SiFive FU540-C000 Yash Shah
                   ` (2 preceding siblings ...)
  2019-06-17  4:43 ` [PATCH v2 0/2] Add macb " Yash Shah
@ 2019-06-17  8:25 ` Andreas Schwab
  2019-06-17  9:58   ` Paul Walmsley
  3 siblings, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2019-06-17  8:25 UTC (permalink / raw)
  To: Yash Shah
  Cc: davem, devicetree, netdev, linux-kernel, linux-riscv, robh+dt,
	mark.rutland, nicolas.ferre, palmer, aou, paul.walmsley, ynezz,
	sachin.ghadi

On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote:

> - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
>   driver. This is needed because on FU540, the macb driver depends on
>   SiFive GPIO driver.

This of course requires that the GPIO driver is upstreamed first.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding
  2019-06-17  4:19 ` [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding Yash Shah
@ 2019-06-17  8:56   ` Paul Walmsley
  0 siblings, 0 replies; 24+ messages in thread
From: Paul Walmsley @ 2019-06-17  8:56 UTC (permalink / raw)
  To: Yash Shah
  Cc: davem, devicetree, netdev, linux-kernel, linux-riscv, robh+dt,
	mark.rutland, nicolas.ferre, palmer, aou, ynezz, sachin.ghadi

On Mon, 17 Jun 2019, Yash Shah 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 <yash.shah@sifive.com>

Reviewed-by: Paul Walmsley <paul.walmsley@sifive.com>


- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17  8:25 ` Andreas Schwab
@ 2019-06-17  9:58   ` Paul Walmsley
  2019-06-17 10:01     ` Andreas Schwab
  2019-06-17 10:22     ` Yash Shah
  0 siblings, 2 replies; 24+ messages in thread
From: Paul Walmsley @ 2019-06-17  9:58 UTC (permalink / raw)
  To: Yash Shah
  Cc: Andreas Schwab, davem, devicetree, netdev, linux-kernel,
	linux-riscv, robh+dt, mark.rutland, nicolas.ferre, palmer, aou,
	ynezz, sachin.ghadi

Hi Yash,

On Mon, 17 Jun 2019, Andreas Schwab wrote:

> On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote:
> 
> > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
> >   driver. This is needed because on FU540, the macb driver depends on
> >   SiFive GPIO driver.
> 
> This of course requires that the GPIO driver is upstreamed first.

What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO 
driver isn't present?  (After modifying the Kconfig "depends" line 
appropriately.)

Looks to me that it shouldn't have an impact unless the DT string is 
present, and even then, the impact might simply be that the MACB driver 
may not work?


- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17  9:58   ` Paul Walmsley
@ 2019-06-17 10:01     ` Andreas Schwab
  2019-06-17 10:05       ` Paul Walmsley
  2019-06-17 10:22     ` Yash Shah
  1 sibling, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2019-06-17 10:01 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Yash Shah, davem, devicetree, netdev, linux-kernel, linux-riscv,
	robh+dt, mark.rutland, nicolas.ferre, palmer, aou, ynezz,
	sachin.ghadi

On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:

> Looks to me that it shouldn't have an impact unless the DT string is 
> present, and even then, the impact might simply be that the MACB driver 
> may not work?

If the macb driver doesn't work you have an unusable system, of course.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 10:01     ` Andreas Schwab
@ 2019-06-17 10:05       ` Paul Walmsley
  2019-06-17 10:14         ` Andreas Schwab
  0 siblings, 1 reply; 24+ messages in thread
From: Paul Walmsley @ 2019-06-17 10:05 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Yash Shah, davem, devicetree, netdev, linux-kernel, linux-riscv,
	robh+dt, mark.rutland, nicolas.ferre, palmer, aou, ynezz,
	sachin.ghadi

On Mon, 17 Jun 2019, Andreas Schwab wrote:

> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
> 
> > Looks to me that it shouldn't have an impact unless the DT string is 
> > present, and even then, the impact might simply be that the MACB driver 
> > may not work?
> 
> If the macb driver doesn't work you have an unusable system, of course.

Why?

- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 10:05       ` Paul Walmsley
@ 2019-06-17 10:14         ` Andreas Schwab
  2019-06-17 11:34           ` Paul Walmsley
  0 siblings, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2019-06-17 10:14 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Yash Shah, davem, devicetree, netdev, linux-kernel, linux-riscv,
	robh+dt, mark.rutland, nicolas.ferre, palmer, aou, ynezz,
	sachin.ghadi

On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:

> On Mon, 17 Jun 2019, Andreas Schwab wrote:
>
>> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
>> 
>> > Looks to me that it shouldn't have an impact unless the DT string is 
>> > present, and even then, the impact might simply be that the MACB driver 
>> > may not work?
>> 
>> If the macb driver doesn't work you have an unusable system, of course.
>
> Why?

Because a system is useless without network.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17  9:58   ` Paul Walmsley
  2019-06-17 10:01     ` Andreas Schwab
@ 2019-06-17 10:22     ` Yash Shah
  2019-06-17 10:28       ` Paul Walmsley
  1 sibling, 1 reply; 24+ messages in thread
From: Yash Shah @ 2019-06-17 10:22 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Andreas Schwab, David Miller, devicetree, netdev, linux-kernel,
	linux-riscv, Rob Herring, Mark Rutland, Nicolas Ferre,
	Palmer Dabbelt, Albert Ou, Petr Štetiar, Sachin Ghadi

On Mon, Jun 17, 2019 at 3:28 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> Hi Yash,
>
> On Mon, 17 Jun 2019, Andreas Schwab wrote:
>
> > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote:
> >
> > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
> > >   driver. This is needed because on FU540, the macb driver depends on
> > >   SiFive GPIO driver.
> >
> > This of course requires that the GPIO driver is upstreamed first.
>
> What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO
> driver isn't present?  (After modifying the Kconfig "depends" line
> appropriately.)
>
> Looks to me that it shouldn't have an impact unless the DT string is
> present, and even then, the impact might simply be that the MACB driver
> may not work?

Yes, there won't be an impact other than MACB driver not working.
In any case, without GPIO driver, PHY won't get reset and the network
interface won't come up.

>
>
> - Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 10:22     ` Yash Shah
@ 2019-06-17 10:28       ` Paul Walmsley
  2019-06-17 11:15         ` Yash Shah
  0 siblings, 1 reply; 24+ messages in thread
From: Paul Walmsley @ 2019-06-17 10:28 UTC (permalink / raw)
  To: Yash Shah
  Cc: Andreas Schwab, David Miller, devicetree, netdev, linux-kernel,
	linux-riscv, Rob Herring, Mark Rutland, Nicolas Ferre,
	Palmer Dabbelt, Albert Ou, Petr Štetiar, Sachin Ghadi,
	sagar.kadam

On Mon, 17 Jun 2019, Yash Shah wrote:

> On Mon, Jun 17, 2019 at 3:28 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> > On Mon, 17 Jun 2019, Andreas Schwab wrote:
> >
> > > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote:
> > >
> > > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
> > > >   driver. This is needed because on FU540, the macb driver depends on
> > > >   SiFive GPIO driver.
> > >
> > > This of course requires that the GPIO driver is upstreamed first.
> >
> > What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO
> > driver isn't present?  (After modifying the Kconfig "depends" line
> > appropriately.)
> >
> > Looks to me that it shouldn't have an impact unless the DT string is
> > present, and even then, the impact might simply be that the MACB driver
> > may not work?
> 
> Yes, there won't be an impact other than MACB driver not working.

OK.  In that case, there doesn't seem much point to adding the Kconfig 
option.  Could you please post a new version without it?

> In any case, without GPIO driver, PHY won't get reset and the network
> interface won't come up.

Naturally, in the medium term, we want Linux to handle the reset.  But if 
there's no GPIO driver present, and the bootloader handles the PHY reset 
before the kernel starts, would the network driver work in that case?
 

- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 10:28       ` Paul Walmsley
@ 2019-06-17 11:15         ` Yash Shah
  0 siblings, 0 replies; 24+ messages in thread
From: Yash Shah @ 2019-06-17 11:15 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Andreas Schwab, David Miller, devicetree, netdev, linux-kernel,
	linux-riscv, Rob Herring, Mark Rutland, Nicolas Ferre,
	Palmer Dabbelt, Albert Ou, Petr Štetiar, Sachin Ghadi,
	Sagar Kadam

On Mon, Jun 17, 2019 at 3:58 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> On Mon, 17 Jun 2019, Yash Shah wrote:
>
> > On Mon, Jun 17, 2019 at 3:28 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
> >
> > > On Mon, 17 Jun 2019, Andreas Schwab wrote:
> > >
> > > > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote:
> > > >
> > > > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb
> > > > >   driver. This is needed because on FU540, the macb driver depends on
> > > > >   SiFive GPIO driver.
> > > >
> > > > This of course requires that the GPIO driver is upstreamed first.
> > >
> > > What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO
> > > driver isn't present?  (After modifying the Kconfig "depends" line
> > > appropriately.)
> > >
> > > Looks to me that it shouldn't have an impact unless the DT string is
> > > present, and even then, the impact might simply be that the MACB driver
> > > may not work?
> >
> > Yes, there won't be an impact other than MACB driver not working.
>
> OK.  In that case, there doesn't seem much point to adding the Kconfig
> option.  Could you please post a new version without it?

Sure, will do that.

>
> > In any case, without GPIO driver, PHY won't get reset and the network
> > interface won't come up.
>
> Naturally, in the medium term, we want Linux to handle the reset.  But if
> there's no GPIO driver present, and the bootloader handles the PHY reset
> before the kernel starts, would the network driver work in that case?

Yes, if bootloader handles the PHY reset then the network driver will
work in that case.
I will post a new version without the GPIO driver dependency.

>
>
> - Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 10:14         ` Andreas Schwab
@ 2019-06-17 11:34           ` Paul Walmsley
  2019-06-17 14:14             ` Troy Benjegerdes
  0 siblings, 1 reply; 24+ messages in thread
From: Paul Walmsley @ 2019-06-17 11:34 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Yash Shah, davem, devicetree, netdev, linux-kernel, linux-riscv,
	robh+dt, mark.rutland, nicolas.ferre, palmer, aou, ynezz,
	sachin.ghadi

On Mon, 17 Jun 2019, Andreas Schwab wrote:

> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
> 
> > On Mon, 17 Jun 2019, Andreas Schwab wrote:
> >
> >> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
> >> 
> >> > Looks to me that it shouldn't have an impact unless the DT string is 
> >> > present, and even then, the impact might simply be that the MACB driver 
> >> > may not work?
> >> 
> >> If the macb driver doesn't work you have an unusable system, of course.
> >
> > Why?
> 
> Because a system is useless without network.

From an upstream Linux point of view, Yash's patches should be an 
improvement over the current mainline kernel situation, since there's 
currently no upstream support for the (SiFive-specific) TX clock switch 
register.  With the right DT data, and a bootloader that handles the PHY 
reset, I think networking should work after his patches are upstream -- 
although I myself haven't tried this yet.


- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 11:34           ` Paul Walmsley
@ 2019-06-17 14:14             ` Troy Benjegerdes
  2019-06-17 14:20               ` Paul Walmsley
  2019-06-17 18:42               ` Alistair Francis
  0 siblings, 2 replies; 24+ messages in thread
From: Troy Benjegerdes @ 2019-06-17 14:14 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Andreas Schwab, Mark Rutland, devicetree, Albert Ou, netdev,
	Palmer Dabbelt, Linux Kernel Mailing List, nicolas.ferre,
	Sachin Ghadi, Yash Shah, robh+dt, ynezz, linux-riscv, davem,
	Jim Jacobsen



> On Jun 17, 2019, at 6:34 AM, Paul Walmsley <paul.walmsley@sifive.com> wrote:
> 
> On Mon, 17 Jun 2019, Andreas Schwab wrote:
> 
>> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
>> 
>>> On Mon, 17 Jun 2019, Andreas Schwab wrote:
>>> 
>>>> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
>>>> 
>>>>> Looks to me that it shouldn't have an impact unless the DT string is 
>>>>> present, and even then, the impact might simply be that the MACB driver 
>>>>> may not work?
>>>> 
>>>> If the macb driver doesn't work you have an unusable system, of course.
>>> 
>>> Why?
>> 
>> Because a system is useless without network.
> 
> From an upstream Linux point of view, Yash's patches should be an 
> improvement over the current mainline kernel situation, since there's 
> currently no upstream support for the (SiFive-specific) TX clock switch 
> register.  With the right DT data, and a bootloader that handles the PHY 
> reset, I think networking should work after his patches are upstream -- 
> although I myself haven't tried this yet.
> 

Have we documented this tx clock switch register in something with a
direct URL link (rather than a PDF)?

I’d like to update freedom-u-sdk (or yocto) to create bootable images
with a working U-boot (upstream or not, I don’t care, as long as it works),
and what I have right now is the old legacy HiFive U-boot[1] and a 4.19
kernel with a bunch of extra patches.

The legacy M-mode U-boot handles the phy reset already, and I’ve been
able to load upstream S-mode uboot as a payload via TFTP, and then 
load and boot a 4.19 kernel. 

It would be nice to get this all working with 5.x, however there are still
several missing pieces to really have it work well.


[1] https://github.com/sifive/HiFive_U-Boot

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 14:14             ` Troy Benjegerdes
@ 2019-06-17 14:20               ` Paul Walmsley
  2019-06-17 18:42               ` Alistair Francis
  1 sibling, 0 replies; 24+ messages in thread
From: Paul Walmsley @ 2019-06-17 14:20 UTC (permalink / raw)
  To: Troy Benjegerdes
  Cc: Andreas Schwab, Mark Rutland, devicetree, Albert Ou, netdev,
	Palmer Dabbelt, Linux Kernel Mailing List, nicolas.ferre,
	Sachin Ghadi, Yash Shah, robh+dt, ynezz, linux-riscv, davem,
	Jim Jacobsen

On Mon, 17 Jun 2019, Troy Benjegerdes wrote:

> Have we documented this tx clock switch register in something with a
> direct URL link (rather than a PDF)?

The SiFive FU540 user manual PDF is the canonical public reference:

https://static.dev.sifive.com/FU540-C000-v1.0.pdf

This practice aligns with other SoC vendors, who also release PDFs.

The relevant Ethernet documentation, including register maps, is in 
Chapter 19.


- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] macb: Add support for SiFive FU540-C000
  2019-06-17  4:19 ` [PATCH v2 2/2] macb: Add support for SiFive FU540-C000 Yash Shah
@ 2019-06-17 15:58   ` Andrew Lunn
  2019-06-18  4:04     ` Yash Shah
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2019-06-17 15:58 UTC (permalink / raw)
  To: Yash Shah
  Cc: davem, devicetree, netdev, linux-kernel, linux-riscv, robh+dt,
	mark.rutland, nicolas.ferre, palmer, aou, paul.walmsley, ynezz,
	sachin.ghadi

On Mon, Jun 17, 2019 at 09:49:27AM +0530, Yash Shah wrote:
> 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 <yash.shah@sifive.com>
> ---
>  drivers/net/ethernet/cadence/Kconfig     |   6 ++
>  drivers/net/ethernet/cadence/macb_main.c | 129 +++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index b998401..d478fae 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -48,4 +48,10 @@ config MACB_PCI
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called macb_pci.
>  
> +config MACB_SIFIVE_FU540
> +	bool "Cadence MACB/GEM support for SiFive FU540 SoC"
> +	depends on MACB && GPIO_SIFIVE
> +	help
> +	  Enable the Cadence MACB/GEM support for SiFive FU540 SoC.
> +
>  endif # NET_VENDOR_CADENCE
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index c049410..275b5e8 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,116 @@ 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);
> +	if (rate != 125000000)
> +		iowrite32(1, mgmt->reg);
> +	else
> +		iowrite32(0, 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,
> @@ -3992,6 +4112,9 @@ static int at91ether_init(struct platform_device *pdev)
>  	{ .compatible = "cdns,emac", .data = &emac_config },
>  	{ .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config},
>  	{ .compatible = "cdns,zynq-gem", .data = &zynq_config },
> +#ifdef CONFIG_MACB_SIFIVE_FU540
> +	{ .compatible = "sifive,fu540-macb", .data = &fu540_c000_config },
> +#endif

This #ifdef should not be needed.

>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, macb_dt_ids);
> @@ -4199,6 +4322,9 @@ static int macb_probe(struct platform_device *pdev)
>  
>  err_disable_clocks:
>  	clk_disable_unprepare(tx_clk);
> +#ifdef CONFIG_MACB_SIFIVE_FU540
> +	clk_unregister(tx_clk);
> +#endif

So long as tx_clk is NULL, you can call clk_unregister(). So please
remove the #ifdef.


>  	clk_disable_unprepare(hclk);
>  	clk_disable_unprepare(pclk);
>  	clk_disable_unprepare(rx_clk);
> @@ -4233,6 +4359,9 @@ static int macb_remove(struct platform_device *pdev)
>  		pm_runtime_dont_use_autosuspend(&pdev->dev);
>  		if (!pm_runtime_suspended(&pdev->dev)) {
>  			clk_disable_unprepare(bp->tx_clk);
> +#ifdef CONFIG_MACB_SIFIVE_FU540
> +			clk_unregister(bp->tx_clk);
> +#endif

Same here.

In general try to avoid #ifdef in C code.

   Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 14:14             ` Troy Benjegerdes
  2019-06-17 14:20               ` Paul Walmsley
@ 2019-06-17 18:42               ` Alistair Francis
  2019-06-18  3:26                 ` Paul Walmsley
  1 sibling, 1 reply; 24+ messages in thread
From: Alistair Francis @ 2019-06-17 18:42 UTC (permalink / raw)
  To: paul.walmsley, troy.benjegerdes
  Cc: jamez, linux-riscv, davem, schwab, nicolas.ferre, mark.rutland,
	devicetree, linux-kernel, aou, sachin.ghadi, netdev, ynezz,
	palmer, yash.shah, robh+dt

On Mon, 2019-06-17 at 09:14 -0500, Troy Benjegerdes wrote:
> > On Jun 17, 2019, at 6:34 AM, Paul Walmsley <
> > paul.walmsley@sifive.com> wrote:
> > 
> > On Mon, 17 Jun 2019, Andreas Schwab wrote:
> > 
> > > On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
> > > 
> > > > On Mon, 17 Jun 2019, Andreas Schwab wrote:
> > > > 
> > > > > On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com>
> > > > > wrote:
> > > > > 
> > > > > > Looks to me that it shouldn't have an impact unless the DT
> > > > > > string is 
> > > > > > present, and even then, the impact might simply be that the
> > > > > > MACB driver 
> > > > > > may not work?
> > > > > 
> > > > > If the macb driver doesn't work you have an unusable system,
> > > > > of course.
> > > > 
> > > > Why?
> > > 
> > > Because a system is useless without network.
> > 
> > From an upstream Linux point of view, Yash's patches should be an 
> > improvement over the current mainline kernel situation, since
> > there's 
> > currently no upstream support for the (SiFive-specific) TX clock
> > switch 
> > register.  With the right DT data, and a bootloader that handles
> > the PHY 
> > reset, I think networking should work after his patches are
> > upstream -- 
> > although I myself haven't tried this yet.
> > 
> 
> Have we documented this tx clock switch register in something with a
> direct URL link (rather than a PDF)?
> 
> I’d like to update freedom-u-sdk (or yocto) to create bootable images
> with a working U-boot (upstream or not, I don’t care, as long as it
> works),
> and what I have right now is the old legacy HiFive U-boot[1] and a
> 4.19
> kernel with a bunch of extra patches.

Yocto/OpenEmbedded does this today. TFTP boot works with the 2019.04 U-
Boot (+ some patches ontop for SMP support). We use the latest 5.1
stable kernel plus 5 or so patches to boot on the Unleased. Networking,
display and audio are all working with the Microsemi expansion board as
well. Let me know if there is something else missing and I'll add it
in. There are probably documentation fixes that are needed as well.

I was thinking of skipping the 5.2 release though as I thought the DT
stuff wasn't going to make it [1]. I will probably re-evaluate that
decision though when 5.2 comes out as it looks like it's all going to
work :)

With U-boot 2017.09 and Linux 5.2/5.3 we should finally be upstream
only!

> 
> The legacy M-mode U-boot handles the phy reset already, and I’ve been
> able to load upstream S-mode uboot as a payload via TFTP, and then 
> load and boot a 4.19 kernel. 
> 
> It would be nice to get this all working with 5.x, however there are
> still
> several missing pieces to really have it work well.

Let me know what is still missing/doesn't work and I can add it. At the
moment the only known issue I know of is a missing SD card driver in U-
Boot.

1: https://github.com/riscv/meta-riscv/issues/143

Alistair

> 
> 
> [1] https://github.com/sifive/HiFive_U-Boot
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-17 18:42               ` Alistair Francis
@ 2019-06-18  3:26                 ` Paul Walmsley
  2019-06-18  9:32                   ` Anup Patel
  2019-06-18 23:11                   ` Alistair Francis
  0 siblings, 2 replies; 24+ messages in thread
From: Paul Walmsley @ 2019-06-18  3:26 UTC (permalink / raw)
  To: Alistair Francis
  Cc: troy.benjegerdes, jamez, linux-riscv, davem, schwab,
	nicolas.ferre, mark.rutland, devicetree, linux-kernel, aou,
	sachin.ghadi, netdev, ynezz, palmer, yash.shah, robh+dt

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On Mon, 17 Jun 2019, Alistair Francis wrote:

> > The legacy M-mode U-boot handles the phy reset already, and I’ve been
> > able to load upstream S-mode uboot as a payload via TFTP, and then 
> > load and boot a 4.19 kernel. 
> > 
> > It would be nice to get this all working with 5.x, however there are
> > still
> > several missing pieces to really have it work well.
> 
> Let me know what is still missing/doesn't work and I can add it. At the
> moment the only known issue I know of is a missing SD card driver in U-
> Boot.

The DT data has changed between the non-upstream data that people 
developed against previously, vs. the DT data that just went upstream 
here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852

and

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205

So Upstream U-Boot is going to need several patches to get things working 
again.  Clock identifiers and Ethernet are two known areas.


- Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] macb: Add support for SiFive FU540-C000
  2019-06-17 15:58   ` Andrew Lunn
@ 2019-06-18  4:04     ` Yash Shah
  0 siblings, 0 replies; 24+ messages in thread
From: Yash Shah @ 2019-06-18  4:04 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Miller, devicetree, netdev, linux-kernel, linux-riscv,
	Rob Herring, Mark Rutland, Nicolas Ferre, Palmer Dabbelt,
	Albert Ou, Paul Walmsley, Petr Štetiar, Sachin Ghadi

On Mon, Jun 17, 2019 at 9:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Jun 17, 2019 at 09:49:27AM +0530, Yash Shah wrote:
...
> >  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,
> > @@ -3992,6 +4112,9 @@ static int at91ether_init(struct platform_device *pdev)
> >       { .compatible = "cdns,emac", .data = &emac_config },
> >       { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config},
> >       { .compatible = "cdns,zynq-gem", .data = &zynq_config },
> > +#ifdef CONFIG_MACB_SIFIVE_FU540
> > +     { .compatible = "sifive,fu540-macb", .data = &fu540_c000_config },
> > +#endif
>
> This #ifdef should not be needed.
>
> >       { /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, macb_dt_ids);
> > @@ -4199,6 +4322,9 @@ static int macb_probe(struct platform_device *pdev)
> >
> >  err_disable_clocks:
> >       clk_disable_unprepare(tx_clk);
> > +#ifdef CONFIG_MACB_SIFIVE_FU540
> > +     clk_unregister(tx_clk);
> > +#endif
>
> So long as tx_clk is NULL, you can call clk_unregister(). So please
> remove the #ifdef.
>
>
> >       clk_disable_unprepare(hclk);
> >       clk_disable_unprepare(pclk);
> >       clk_disable_unprepare(rx_clk);
> > @@ -4233,6 +4359,9 @@ static int macb_remove(struct platform_device *pdev)
> >               pm_runtime_dont_use_autosuspend(&pdev->dev);
> >               if (!pm_runtime_suspended(&pdev->dev)) {
> >                       clk_disable_unprepare(bp->tx_clk);
> > +#ifdef CONFIG_MACB_SIFIVE_FU540
> > +                     clk_unregister(bp->tx_clk);
> > +#endif
>
> Same here.
>
> In general try to avoid #ifdef in C code.

Will remove all the #ifdef in v3.
Thanks for your comments.

- Yash

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-18  3:26                 ` Paul Walmsley
@ 2019-06-18  9:32                   ` Anup Patel
  2019-06-18 16:49                     ` Troy Benjegerdes
  2019-06-18 23:40                     ` Atish Patra
  2019-06-18 23:11                   ` Alistair Francis
  1 sibling, 2 replies; 24+ messages in thread
From: Anup Patel @ 2019-06-18  9:32 UTC (permalink / raw)
  To: Paul Walmsley, palmer
  Cc: Alistair Francis, troy.benjegerdes, jamez, linux-riscv, davem,
	schwab, nicolas.ferre, mark.rutland, devicetree, linux-kernel,
	aou, sachin.ghadi, netdev, ynezz, yash.shah, robh+dt,
	Atish Patra, Bin Meng, Lukas Auer

On Tue, Jun 18, 2019 at 8:56 AM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> On Mon, 17 Jun 2019, Alistair Francis wrote:
>
> > > The legacy M-mode U-boot handles the phy reset already, and I’ve been
> > > able to load upstream S-mode uboot as a payload via TFTP, and then
> > > load and boot a 4.19 kernel.
> > >
> > > It would be nice to get this all working with 5.x, however there are
> > > still
> > > several missing pieces to really have it work well.
> >
> > Let me know what is still missing/doesn't work and I can add it. At the
> > moment the only known issue I know of is a missing SD card driver in U-
> > Boot.
>
> The DT data has changed between the non-upstream data that people
> developed against previously, vs. the DT data that just went upstream
> here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852
>
> and
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205
>
> So Upstream U-Boot is going to need several patches to get things working
> again.  Clock identifiers and Ethernet are two known areas.

Done.

I just send-out few patches to fix U-Boot SiFive Clock driver.

The U-Boot SiFive Clock driver fix series can be found in
riscv_unleashed_clk_sync_v1 branch of:
https://github.com/avpatel/u-boot.git

Users will also require OpenSBI DTB fix which can be found
in sifive_unleashed_dtb_fix_v1 branch of:
https://github.com/avpatel/opensbi.git

With above fixes, we can now use same DTB for both U-Boot
and Linux kernel (5.2-rc1). Although, users are free to pass a
different DTB to Linux kernel via TFTP.

I have tested SiFive serial and Cadance MACB ethernet on
both U-Boot and Linux (5.2-rc1)

Regards,
Anup

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-18  9:32                   ` Anup Patel
@ 2019-06-18 16:49                     ` Troy Benjegerdes
  2019-06-18 23:40                     ` Atish Patra
  1 sibling, 0 replies; 24+ messages in thread
From: Troy Benjegerdes @ 2019-06-18 16:49 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, palmer, Alistair Francis, jamez, linux-riscv,
	davem, schwab, nicolas.ferre, mark.rutland, devicetree,
	linux-kernel, aou, sachin.ghadi, netdev, ynezz, yash.shah,
	robh+dt, Atish Patra, Bin Meng, Lukas Auer



> On Jun 18, 2019, at 4:32 AM, Anup Patel <anup@brainfault.org> wrote:
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852

I added your patches, along with two of mine, and rebased them
to the latest U-boot master, and put them on the ‘to-upstream’ branch
at https://github.com/sifive/u-boot/tree/to-upstream

I am most interested in review of the patch that adds the DTS files
from Linux to U-boot, along with a ‘-u-boot.dtsi’ file which includes
several extra things, most notably an ethernet entry [1] which does
not match the new proposed changes for the MacB driver that Yash
is working on.

How close are we to consensus on the new “sifive,fu540-macb”
device tree entry format? Is this something that is stable enough to
start basing some work in M-mode U-boot on yet, or do we expect
more changes?

[1] https://github.com/sifive/u-boot/commit/35e4168e36139722f30143a0ca0aa8637dd3ee04#diff-27d2d375ddac52f1bca71594075e1be4R93

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-18  3:26                 ` Paul Walmsley
  2019-06-18  9:32                   ` Anup Patel
@ 2019-06-18 23:11                   ` Alistair Francis
  1 sibling, 0 replies; 24+ messages in thread
From: Alistair Francis @ 2019-06-18 23:11 UTC (permalink / raw)
  To: paul.walmsley
  Cc: davem, palmer, nicolas.ferre, aou, yash.shah, linux-kernel,
	sachin.ghadi, devicetree, ynezz, jamez, mark.rutland, robh+dt,
	netdev, schwab, troy.benjegerdes, linux-riscv

On Mon, 2019-06-17 at 20:26 -0700, Paul Walmsley wrote:
> On Mon, 17 Jun 2019, Alistair Francis wrote:
> 
> > > The legacy M-mode U-boot handles the phy reset already, and I’ve
> > > been
> > > able to load upstream S-mode uboot as a payload via TFTP, and
> > > then 
> > > load and boot a 4.19 kernel. 
> > > 
> > > It would be nice to get this all working with 5.x, however there
> > > are
> > > still
> > > several missing pieces to really have it work well.
> > 
> > Let me know what is still missing/doesn't work and I can add it. At
> > the
> > moment the only known issue I know of is a missing SD card driver
> > in U-
> > Boot.
> 
> The DT data has changed between the non-upstream data that people 
> developed against previously, vs. the DT data that just went
> upstream 
> here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852
> 
> and
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205
> 
> So Upstream U-Boot is going to need several patches to get things
> working 
> again.  Clock identifiers and Ethernet are two known areas.

Yep, Anup has sent patches to U-Boot and OpenSBI.

Alistair

> 
> 
> - Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Add macb support for SiFive FU540-C000
  2019-06-18  9:32                   ` Anup Patel
  2019-06-18 16:49                     ` Troy Benjegerdes
@ 2019-06-18 23:40                     ` Atish Patra
  1 sibling, 0 replies; 24+ messages in thread
From: Atish Patra @ 2019-06-18 23:40 UTC (permalink / raw)
  To: anup, paul.walmsley, palmer
  Cc: bmeng.cn, davem, Alistair Francis, nicolas.ferre, aou, yash.shah,
	linux-kernel, ynezz, devicetree, sachin.ghadi, jamez,
	mark.rutland, robh+dt, netdev, schwab, lukas.auer,
	troy.benjegerdes, linux-riscv

On Tue, 2019-06-18 at 15:02 +0530, Anup Patel wrote:
> On Tue, Jun 18, 2019 at 8:56 AM Paul Walmsley <
> paul.walmsley@sifive.com> wrote:
> > On Mon, 17 Jun 2019, Alistair Francis wrote:
> > 
> > > > The legacy M-mode U-boot handles the phy reset already, and
> > > > I’ve been
> > > > able to load upstream S-mode uboot as a payload via TFTP, and
> > > > then
> > > > load and boot a 4.19 kernel.
> > > > 
> > > > It would be nice to get this all working with 5.x, however
> > > > there are
> > > > still
> > > > several missing pieces to really have it work well.
> > > 
> > > Let me know what is still missing/doesn't work and I can add it.
> > > At the
> > > moment the only known issue I know of is a missing SD card driver
> > > in U-
> > > Boot.
> > 
> > The DT data has changed between the non-upstream data that people
> > developed against previously, vs. the DT data that just went
> > upstream
> > here:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852
> > 
> > and
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205
> > 
> > So Upstream U-Boot is going to need several patches to get things
> > working
> > again.  Clock identifiers and Ethernet are two known areas.
> 
> Done.
> 
> I just send-out few patches to fix U-Boot SiFive Clock driver.
> 
> The U-Boot SiFive Clock driver fix series can be found in
> riscv_unleashed_clk_sync_v1 branch of:
> https://github.com/avpatel/u-boot.git
> 
> Users will also require OpenSBI DTB fix which can be found
> in sifive_unleashed_dtb_fix_v1 branch of:
> https://github.com/avpatel/opensbi.git
> 

Additionally, user can also use FW_PAYLOAD_FDT_PATH argument during
build to include the dtb built from kernel.

example:
make -j 32 PLATFORM=sifive/fu540 FW_PAYLOAD_PATH=<u-boot path>/u-
boot.bin FW_PAYLOAD_FDT_PATH=<kernel_dtb_path>


> With above fixes, we can now use same DTB for both U-Boot
> and Linux kernel (5.2-rc1). Although, users are free to pass a
> different DTB to Linux kernel via TFTP.
> 
> I have tested SiFive serial and Cadance MACB ethernet on
> both U-Boot and Linux (5.2-rc1)
> 
> Regards,
> Anup
-- 
Regards,
Atish

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17  4:19 [PATCH v2 0/2] Add macb support for SiFive FU540-C000 Yash Shah
2019-06-17  4:19 ` [PATCH v2 1/2] macb: bindings doc: add sifive fu540-c000 binding Yash Shah
2019-06-17  8:56   ` Paul Walmsley
2019-06-17  4:19 ` [PATCH v2 2/2] macb: Add support for SiFive FU540-C000 Yash Shah
2019-06-17 15:58   ` Andrew Lunn
2019-06-18  4:04     ` Yash Shah
2019-06-17  4:43 ` [PATCH v2 0/2] Add macb " Yash Shah
2019-06-17  8:25 ` Andreas Schwab
2019-06-17  9:58   ` Paul Walmsley
2019-06-17 10:01     ` Andreas Schwab
2019-06-17 10:05       ` Paul Walmsley
2019-06-17 10:14         ` Andreas Schwab
2019-06-17 11:34           ` Paul Walmsley
2019-06-17 14:14             ` Troy Benjegerdes
2019-06-17 14:20               ` Paul Walmsley
2019-06-17 18:42               ` Alistair Francis
2019-06-18  3:26                 ` Paul Walmsley
2019-06-18  9:32                   ` Anup Patel
2019-06-18 16:49                     ` Troy Benjegerdes
2019-06-18 23:40                     ` Atish Patra
2019-06-18 23:11                   ` Alistair Francis
2019-06-17 10:22     ` Yash Shah
2019-06-17 10:28       ` Paul Walmsley
2019-06-17 11:15         ` Yash Shah

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox