2023-02-11 03:18:44

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 00/12] Enable networking support for StarFive JH7100 SoC

This patch series adds ethernet support for the StarFive JH7100 SoC and
makes it available for the StarFive VisionFive V1 and BeagleV Starlight
boards, although I could only validate on the former SBC.

The work is heavily based on the reference implementation [1] and requires
the non-coherent DMA support provided by Emil via the Sifive Composable
Cache controller.

Also note there is an overlap in "[PATCH 08/12] net: stmmac: Add glue layer
for StarFive JH7100 SoC" with the Yanhong Wang's upstreaming attempt [2]:
"[PATCH v4 5/7] net: stmmac: Add glue layer for StarFive JH7110 SoCs".

Since I cannot test the JH7110 SoC, I dropped the support for it from Emil's
variant of the stmmac glue layer. Hence, we might need a bit of coordination
in order to get this properly merged.

[1] https://github.com/starfive-tech/linux/commits/visionfive
[2] https://lore.kernel.org/linux-riscv/[email protected]/

Cristian Ciocaltea (7):
dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100
SoC
dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property
dt-bindings: net: Add StarFive JH7100 SoC
riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC
riscv: dts: starfive: jh7100: Add ccache DT node
riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes
riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

Emil Renner Berthing (5):
soc: sifive: ccache: Add StarFive JH7100 support
soc: sifive: ccache: Add non-coherent DMA handling
riscv: Implement non-coherent DMA support via SiFive cache flushing
dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible
net: stmmac: Add glue layer for StarFive JH7100 SoC

.../devicetree/bindings/mfd/syscon.yaml | 1 +
.../devicetree/bindings/net/snps,dwmac.yaml | 15 +-
.../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++
.../bindings/riscv/sifive,ccache0.yaml | 33 +++-
MAINTAINERS | 6 +
arch/riscv/Kconfig | 6 +-
.../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++
arch/riscv/boot/dts/starfive/jh7100.dtsi | 55 +++++++
arch/riscv/mm/dma-noncoherent.c | 37 ++++-
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++
drivers/soc/sifive/Kconfig | 1 +
drivers/soc/sifive/sifive_ccache.c | 71 +++++++-
include/soc/sifive/sifive_ccache.h | 21 +++
15 files changed, 587 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c

--
2.39.1



2023-02-11 03:18:51

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

Document the compatible for the SiFive Composable Cache Controller found
on the StarFive JH7100 SoC.

This also requires extending the 'reg' property to handle distinct
ranges, as specified via 'reg-names'.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../bindings/riscv/sifive,ccache0.yaml | 28 ++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
index 31d20efaa6d3..2b864b2f12c9 100644
--- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
+++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
@@ -25,6 +25,7 @@ select:
- sifive,ccache0
- sifive,fu540-c000-ccache
- sifive,fu740-c000-ccache
+ - starfive,jh7100-ccache

required:
- compatible
@@ -37,6 +38,7 @@ properties:
- sifive,ccache0
- sifive,fu540-c000-ccache
- sifive,fu740-c000-ccache
+ - starfive,jh7100-ccache
- const: cache
- items:
- const: starfive,jh7110-ccache
@@ -70,7 +72,13 @@ properties:
- description: DirFail interrupt

reg:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: control
+ - const: sideband

next-level-cache: true

@@ -89,6 +97,7 @@ allOf:
contains:
enum:
- sifive,fu740-c000-ccache
+ - starfive,jh7100-ccache
- starfive,jh7110-ccache
- microchip,mpfs-ccache

@@ -106,12 +115,29 @@ allOf:
Must contain entries for DirError, DataError and DataFail signals.
maxItems: 3

+ - if:
+ properties:
+ compatible:
+ contains:
+ const: starfive,jh7100-ccache
+
+ then:
+ properties:
+ reg:
+ maxItems: 2
+
+ else:
+ properties:
+ reg:
+ maxItems: 1
+
- if:
properties:
compatible:
contains:
enum:
- sifive,fu740-c000-ccache
+ - starfive,jh7100-ccache
- starfive,jh7110-ccache

then:
--
2.39.1


2023-02-11 03:19:03

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

Add the 'uncached-offset' property to be used for specifying the
uncached memory offset required for handling non-coherent DMA
transactions.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
index 2b864b2f12c9..60cd87a2810a 100644
--- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
+++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
@@ -82,6 +82,11 @@ properties:

next-level-cache: true

+ uncached-offset:
+ $ref: /schemas/types.yaml#/definitions/uint64
+ description: |
+ Uncached memory offset for handling non-coherent DMA transactions.
+
memory-region:
maxItems: 1
description: |
--
2.39.1


2023-02-11 03:19:06

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 03/12] soc: sifive: ccache: Add StarFive JH7100 support

From: Emil Renner Berthing <[email protected]>

This adds support for the StarFive JH7100 SoC which also feature this
SiFive cache controller.

Unfortunately the interrupt for uncorrected data is broken on the JH7100
and fires continuously, so add a quirk to not register a handler for it.

Signed-off-by: Emil Renner Berthing <[email protected]>
[drop JH7110, rework Kconfig]
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
drivers/soc/sifive/Kconfig | 1 +
drivers/soc/sifive/sifive_ccache.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
index e86870be34c9..867cf16273a4 100644
--- a/drivers/soc/sifive/Kconfig
+++ b/drivers/soc/sifive/Kconfig
@@ -4,6 +4,7 @@ if SOC_SIFIVE || SOC_STARFIVE

config SIFIVE_CCACHE
bool "Sifive Composable Cache controller"
+ default SOC_STARFIVE
help
Support for the composable cache controller on SiFive platforms.

diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
index 3684f5b40a80..676468c35859 100644
--- a/drivers/soc/sifive/sifive_ccache.c
+++ b/drivers/soc/sifive/sifive_ccache.c
@@ -106,6 +106,7 @@ static void ccache_config_read(void)
static const struct of_device_id sifive_ccache_ids[] = {
{ .compatible = "sifive,fu540-c000-ccache" },
{ .compatible = "sifive,fu740-c000-ccache" },
+ { .compatible = "starfive,jh7100-ccache", .data = (void *)BIT(DATA_UNCORR) },
{ .compatible = "sifive,ccache0" },
{ /* end of table */ }
};
@@ -210,11 +211,15 @@ static int __init sifive_ccache_init(void)
struct device_node *np;
struct resource res;
int i, rc, intr_num;
+ const struct of_device_id *match;
+ unsigned long broken_irqs;

- np = of_find_matching_node(NULL, sifive_ccache_ids);
+ np = of_find_matching_node_and_match(NULL, sifive_ccache_ids, &match);
if (!np)
return -ENODEV;

+ broken_irqs = (uintptr_t)match->data;
+
if (of_address_to_resource(np, 0, &res)) {
rc = -ENODEV;
goto err_node_put;
@@ -240,6 +245,10 @@ static int __init sifive_ccache_init(void)

for (i = 0; i < intr_num; i++) {
g_irq[i] = irq_of_parse_and_map(np, i);
+
+ if (broken_irqs & BIT(i))
+ continue;
+
rc = request_irq(g_irq[i], ccache_int_handler, 0, "ccache_ecc",
NULL);
if (rc) {
--
2.39.1


2023-02-11 03:19:11

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

From: Emil Renner Berthing <[email protected]>

Add functions to flush the caches and handle non-coherent DMA.

Signed-off-by: Emil Renner Berthing <[email protected]>
[replace <asm/cacheflush.h> with <linux/cacheflush.h>]
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
drivers/soc/sifive/sifive_ccache.c | 60 +++++++++++++++++++++++++++++-
include/soc/sifive/sifive_ccache.h | 21 +++++++++++
2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
index 676468c35859..0062635d845f 100644
--- a/drivers/soc/sifive/sifive_ccache.c
+++ b/drivers/soc/sifive/sifive_ccache.c
@@ -8,13 +8,16 @@

#define pr_fmt(fmt) "CCACHE: " fmt

+#include <linux/align.h>
#include <linux/debugfs.h>
#include <linux/interrupt.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/device.h>
#include <linux/bitfield.h>
+#include <linux/cacheflush.h>
#include <asm/cacheinfo.h>
+#include <asm/page.h>
#include <soc/sifive/sifive_ccache.h>

#define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
@@ -39,10 +42,14 @@
#define SIFIVE_CCACHE_CONFIG_SETS_MASK GENMASK_ULL(23, 16)
#define SIFIVE_CCACHE_CONFIG_BLKS_MASK GENMASK_ULL(31, 24)

+#define SIFIVE_CCACHE_FLUSH64 0x200
+#define SIFIVE_CCACHE_FLUSH32 0x240
+
#define SIFIVE_CCACHE_WAYENABLE 0x08
#define SIFIVE_CCACHE_ECCINJECTERR 0x40

#define SIFIVE_CCACHE_MAX_ECCINTR 4
+#define SIFIVE_CCACHE_LINE_SIZE 64

static void __iomem *ccache_base;
static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
@@ -125,6 +132,47 @@ int unregister_sifive_ccache_error_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL_GPL(unregister_sifive_ccache_error_notifier);

+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+static phys_addr_t uncached_offset;
+DEFINE_STATIC_KEY_FALSE(sifive_ccache_handle_noncoherent_key);
+
+void sifive_ccache_flush_range(phys_addr_t start, size_t len)
+{
+ phys_addr_t end = start + len;
+ phys_addr_t line;
+
+ if (!len)
+ return;
+
+ mb();
+ for (line = ALIGN_DOWN(start, SIFIVE_CCACHE_LINE_SIZE); line < end;
+ line += SIFIVE_CCACHE_LINE_SIZE) {
+#ifdef CONFIG_32BIT
+ writel(line >> 4, ccache_base + SIFIVE_CCACHE_FLUSH32);
+#else
+ writeq(line, ccache_base + SIFIVE_CCACHE_FLUSH64);
+#endif
+ mb();
+ }
+}
+EXPORT_SYMBOL_GPL(sifive_ccache_flush_range);
+
+void *sifive_ccache_set_uncached(void *addr, size_t size)
+{
+ phys_addr_t phys_addr = __pa(addr) + uncached_offset;
+ void *mem_base;
+
+ mem_base = memremap(phys_addr, size, MEMREMAP_WT);
+ if (!mem_base) {
+ pr_err("%s memremap failed for addr %p\n", __func__, addr);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return mem_base;
+}
+EXPORT_SYMBOL_GPL(sifive_ccache_set_uncached);
+#endif /* CONFIG_RISCV_DMA_NONCOHERENT */
+
static int ccache_largest_wayenabled(void)
{
return readl(ccache_base + SIFIVE_CCACHE_WAYENABLE) & 0xFF;
@@ -213,6 +261,7 @@ static int __init sifive_ccache_init(void)
int i, rc, intr_num;
const struct of_device_id *match;
unsigned long broken_irqs;
+ u64 __maybe_unused offset;

np = of_find_matching_node_and_match(NULL, sifive_ccache_ids, &match);
if (!np)
@@ -258,6 +307,15 @@ static int __init sifive_ccache_init(void)
}
of_node_put(np);

+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+ if (!of_property_read_u64(np, "uncached-offset", &offset)) {
+ uncached_offset = offset;
+ static_branch_enable(&sifive_ccache_handle_noncoherent_key);
+ riscv_cbom_block_size = SIFIVE_CCACHE_LINE_SIZE;
+ riscv_noncoherent_supported();
+ }
+#endif
+
ccache_config_read();

ccache_cache_ops.get_priv_group = ccache_get_priv_group;
@@ -278,4 +336,4 @@ static int __init sifive_ccache_init(void)
return rc;
}

-device_initcall(sifive_ccache_init);
+arch_initcall(sifive_ccache_init);
diff --git a/include/soc/sifive/sifive_ccache.h b/include/soc/sifive/sifive_ccache.h
index 4d4ed49388a0..d349ccb3969b 100644
--- a/include/soc/sifive/sifive_ccache.h
+++ b/include/soc/sifive/sifive_ccache.h
@@ -7,10 +7,31 @@
#ifndef __SOC_SIFIVE_CCACHE_H
#define __SOC_SIFIVE_CCACHE_H

+#include <linux/io.h>
+#include <linux/jump_label.h>
+
extern int register_sifive_ccache_error_notifier(struct notifier_block *nb);
extern int unregister_sifive_ccache_error_notifier(struct notifier_block *nb);

#define SIFIVE_CCACHE_ERR_TYPE_CE 0
#define SIFIVE_CCACHE_ERR_TYPE_UE 1

+DECLARE_STATIC_KEY_FALSE(sifive_ccache_handle_noncoherent_key);
+
+static inline bool sifive_ccache_handle_noncoherent(void)
+{
+#ifdef CONFIG_SIFIVE_CCACHE
+ return static_branch_unlikely(&sifive_ccache_handle_noncoherent_key);
+#else
+ return false;
+#endif
+}
+
+void sifive_ccache_flush_range(phys_addr_t start, size_t len);
+void *sifive_ccache_set_uncached(void *addr, size_t size);
+static inline void sifive_ccache_clear_uncached(void *addr, size_t size)
+{
+ memunmap(addr);
+}
+
#endif /* __SOC_SIFIVE_CCACHE_H */
--
2.39.1


2023-02-11 03:19:20

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

From: Emil Renner Berthing <[email protected]>

This variant is used on the StarFive JH7100 SoC.

Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
arch/riscv/Kconfig | 6 ++++--
arch/riscv/mm/dma-noncoherent.c | 37 +++++++++++++++++++++++++++++++--
2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 9c687da7756d..05f6c77faf6f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -232,12 +232,14 @@ config LOCKDEP_SUPPORT
def_bool y

config RISCV_DMA_NONCOHERENT
- bool
+ bool "Support non-coherent DMA"
+ default SOC_STARFIVE
select ARCH_HAS_DMA_PREP_COHERENT
+ select ARCH_HAS_DMA_SET_UNCACHED
+ select ARCH_HAS_DMA_CLEAR_UNCACHED
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_HAS_SYNC_DMA_FOR_CPU
select ARCH_HAS_SETUP_DMA_OPS
- select DMA_DIRECT_REMAP

config AS_HAS_INSN
def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
index d919efab6eba..e07e53aea537 100644
--- a/arch/riscv/mm/dma-noncoherent.c
+++ b/arch/riscv/mm/dma-noncoherent.c
@@ -9,14 +9,21 @@
#include <linux/dma-map-ops.h>
#include <linux/mm.h>
#include <asm/cacheflush.h>
+#include <soc/sifive/sifive_ccache.h>

static bool noncoherent_supported;

void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
enum dma_data_direction dir)
{
- void *vaddr = phys_to_virt(paddr);
+ void *vaddr;

+ if (sifive_ccache_handle_noncoherent()) {
+ sifive_ccache_flush_range(paddr, size);
+ return;
+ }
+
+ vaddr = phys_to_virt(paddr);
switch (dir) {
case DMA_TO_DEVICE:
ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
@@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
enum dma_data_direction dir)
{
- void *vaddr = phys_to_virt(paddr);
+ void *vaddr;
+
+ if (sifive_ccache_handle_noncoherent()) {
+ sifive_ccache_flush_range(paddr, size);
+ return;
+ }

+ vaddr = phys_to_virt(paddr);
switch (dir) {
case DMA_TO_DEVICE:
break;
@@ -49,10 +62,30 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
}
}

+void *arch_dma_set_uncached(void *addr, size_t size)
+{
+ if (sifive_ccache_handle_noncoherent())
+ return sifive_ccache_set_uncached(addr, size);
+
+ return addr;
+}
+
+void arch_dma_clear_uncached(void *addr, size_t size)
+{
+ if (sifive_ccache_handle_noncoherent())
+ sifive_ccache_clear_uncached(addr, size);
+}
+
void arch_dma_prep_coherent(struct page *page, size_t size)
{
void *flush_addr = page_address(page);

+ if (sifive_ccache_handle_noncoherent()) {
+ memset(flush_addr, 0, size);
+ sifive_ccache_flush_range(__pa(flush_addr), size);
+ return;
+ }
+
ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size);
}

--
2.39.1


2023-02-11 03:19:30

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 06/12] dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible

From: Emil Renner Berthing <[email protected]>

Document StarFive JH7100 SoC compatible for sysmain registers.

Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index c828c4f5e4a7..43f564be709f 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -67,6 +67,7 @@ properties:
- rockchip,rk3568-qos
- rockchip,rk3588-qos
- rockchip,rv1126-qos
+ - starfive,jh7100-sysmain

- const: syscon

--
2.39.1


2023-02-11 03:19:33

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

Add DT bindings documentation for the Synopsys DesignWare MAC found on
the StarFive JH7100 SoC.

Adjust 'reset' and 'reset-names' properties to allow using 'ahb' instead
of the 'stmmaceth' reset signal, as required by JH7100.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 15 ++-
.../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++++++++
MAINTAINERS | 5 +
3 files changed, 122 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e88a86623fce..71522a2cd7a4 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -89,6 +89,7 @@ properties:
- snps,dwmac-5.10a
- snps,dwxgmac
- snps,dwxgmac-2.10
+ - starfive,jh7100-dwmac

reg:
minItems: 1
@@ -131,12 +132,17 @@ properties:
- ptp_ref

resets:
- maxItems: 1
- description:
- MAC Reset signal.
+ minItems: 1
+ items:
+ - description: MAC Reset signal
+ - description: AHB Reset signal

reset-names:
- const: stmmaceth
+ minItems: 1
+ contains:
+ enum:
+ - stmmaceth
+ - ahb

power-domains:
maxItems: 1
@@ -578,6 +584,7 @@ allOf:
- snps,dwxgmac
- snps,dwxgmac-2.10
- st,spear600-gmac
+ - starfive,jh7100-dwmac

then:
properties:
diff --git a/Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
new file mode 100644
index 000000000000..6afe30690172
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 StarFive Technology Co., Ltd.
+# Copyright (C) 2022 Emil Renner Berthing <[email protected]>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/starfive,jh7100-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7100 DWMAC Ethernet Controller
+
+maintainers:
+ - Emil Renner Berthing <[email protected]>
+
+# We need a select here so we don't match all nodes with 'snps,dwmac'
+select:
+ properties:
+ compatible:
+ contains:
+ const: starfive,jh7100-dwmac
+ required:
+ - compatible
+
+allOf:
+ - $ref: snps,dwmac.yaml#
+
+properties:
+ compatible:
+ items:
+ - const: starfive,jh7100-dwmac
+ - const: snps,dwmac
+
+ clocks:
+ items:
+ - description: GMAC main clock
+ - description: GMAC AHB clock
+ - description: PTP clock
+ - description: GTX clock
+ - description: TX clock
+
+ clock-names:
+ items:
+ - const: stmmaceth
+ - const: pclk
+ - const: ptp_ref
+ - const: gtxc
+ - const: tx
+
+ resets:
+ description: AHB Reset signal
+
+ reset-names:
+ const: ahb
+
+ starfive,syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: phandle to the syscon node
+
+ starfive,gtxclk-dlychain:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: GTX clock delay chain setting
+
+required:
+ - compatible
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/starfive-jh7100.h>
+ #include <dt-bindings/reset/starfive-jh7100.h>
+
+ gmac: ethernet@10020000 {
+ compatible = "starfive,jh7100-dwmac", "snps,dwmac";
+ reg = <0x0 0x10020000 0x0 0x10000>;
+ clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
+ <&clkgen JH7100_CLK_GMAC_AHB>,
+ <&clkgen JH7100_CLK_GMAC_PTP_REF>,
+ <&clkgen JH7100_CLK_GMAC_GTX>,
+ <&clkgen JH7100_CLK_GMAC_TX_INV>;
+ clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
+ resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
+ reset-names = "ahb";
+ interrupts = <6>, <7>;
+ interrupt-names = "macirq", "eth_wake_irq";
+ max-frame-size = <9000>;
+ phy-mode = "rgmii-txid";
+ snps,multicast-filter-bins = <32>;
+ snps,perfect-filter-entries = <128>;
+ starfive,syscon = <&sysmain>;
+ rx-fifo-depth = <32768>;
+ tx-fifo-depth = <16384>;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,fixed-burst;
+ snps,force_thresh_dma_mode;
+ snps,no-pbl-x8;
+
+ stmmac_axi_setup: stmmac-axi-config {
+ snps,wr_osr_lmt = <0xf>;
+ snps,rd_osr_lmt = <0xf>;
+ snps,blen = <256 128 64 32 0 0 0>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index abed40db41f0..d48468b81b94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19816,6 +19816,11 @@ M: Emil Renner Berthing <[email protected]>
S: Maintained
F: arch/riscv/boot/dts/starfive/

+STARFIVE DWMAC GLUE LAYER
+M: Emil Renner Berthing <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
+
STARFIVE JH7100 CLOCK DRIVERS
M: Emil Renner Berthing <[email protected]>
S: Maintained
--
2.39.1


2023-02-11 03:19:45

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

From: Emil Renner Berthing <[email protected]>

This adds a glue layer for the Synopsys DesignWare MAC IP core on the
StarFive JH7100 SoC.

Signed-off-by: Emil Renner Berthing <[email protected]>
[drop references to JH7110, update JH7100 compatible string]
Signed-off-by: Cristian Ciocaltea <[email protected]>
---
MAINTAINERS | 1 +
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++
4 files changed, 169 insertions(+)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d48468b81b94..defedaff6041 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19820,6 +19820,7 @@ STARFIVE DWMAC GLUE LAYER
M: Emil Renner Berthing <[email protected]>
S: Maintained
F: Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
+F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c

STARFIVE JH7100 CLOCK DRIVERS
M: Emil Renner Berthing <[email protected]>
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index f77511fe4e87..2c81aa594291 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
for the stmmac device driver. This driver is used for
arria5 and cyclone5 FPGA SoCs.

+config DWMAC_STARFIVE
+ tristate "StarFive DWMAC support"
+ default m if SOC_STARFIVE
+ depends on SOC_STARFIVE || COMPILE_TEST
+ select MFD_SYSCON
+ help
+ Support for ethernet controller on StarFive SOCs.
+
+ This selects StarFive SoC glue layer support for the stmmac device
+ driver. This driver is used for the JH71x0 series GMAC ethernet
+ controller.
+
config DWMAC_STI
tristate "STi GMAC support"
default ARCH_STI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 057e4bab5c08..8738fdbb4b2d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
+obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
new file mode 100644
index 000000000000..d4c81f1a5482
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dwmac-starfive.c - DWMAC glue layer for StarFive JH7100 SoC
+ *
+ * Copyright (C) 2021 Emil Renner Berthing <[email protected]>
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+
+#define JH7100_SYSMAIN_REGISTER28 0x70
+/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
+#define JH7100_SYSMAIN_REGISTER49 0xc8
+
+struct dwmac_starfive {
+ struct device *dev;
+ struct clk *gtxc;
+};
+
+static int dwmac_starfive_jh7100_syscon_init(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct regmap *sysmain;
+ u32 gtxclk_dlychain;
+ int ret;
+
+ sysmain = syscon_regmap_lookup_by_phandle(np, "starfive,syscon");
+ if (IS_ERR(sysmain))
+ return dev_err_probe(dev, PTR_ERR(sysmain),
+ "error getting sysmain registers\n");
+
+ /* Choose RGMII interface to the phy.
+ * TODO: support other interfaces once we know the meaning of other
+ * values in the register
+ */
+ ret = regmap_update_bits(sysmain, JH7100_SYSMAIN_REGISTER28, 0x7, 1);
+ if (ret)
+ return dev_err_probe(dev, ret, "error selecting gmac interface\n");
+
+ if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
+ ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
+ if (ret)
+ return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
+ }
+
+ return 0;
+}
+
+static void dwmac_starfive_fix_mac_speed(void *data, unsigned int speed)
+{
+ struct dwmac_starfive *dwmac = data;
+ unsigned long rate;
+ int ret;
+
+ switch (speed) {
+ case SPEED_1000:
+ rate = 125000000;
+ break;
+ case SPEED_100:
+ rate = 25000000;
+ break;
+ case SPEED_10:
+ rate = 2500000;
+ break;
+ default:
+ dev_warn(dwmac->dev, "unsupported link speed %u\n", speed);
+ return;
+ }
+
+ ret = clk_set_rate(dwmac->gtxc, rate);
+ if (ret)
+ dev_err(dwmac->dev, "error setting gtx clock rate: %d\n", ret);
+}
+
+static int dwmac_starfive_probe(struct platform_device *pdev)
+{
+ struct stmmac_resources stmmac_res;
+ struct plat_stmmacenet_data *plat;
+ struct dwmac_starfive *dwmac;
+ struct clk *txclk;
+ int (*syscon_init)(struct device *dev);
+ int ret;
+
+ dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+ if (!dwmac)
+ return -ENOMEM;
+
+ ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (ret)
+ return ret;
+
+ syscon_init = of_device_get_match_data(&pdev->dev);
+ if (syscon_init) {
+ ret = syscon_init(&pdev->dev);
+ if (ret)
+ return ret;
+ }
+
+ dwmac->gtxc = devm_clk_get_enabled(&pdev->dev, "gtxc");
+ if (IS_ERR(dwmac->gtxc))
+ return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->gtxc),
+ "error getting/enabling gtxc clock\n");
+
+ txclk = devm_clk_get_enabled(&pdev->dev, "tx");
+ if (IS_ERR(txclk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(txclk),
+ "error getting/enabling tx clock\n");
+
+ plat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+ if (IS_ERR(plat))
+ return dev_err_probe(&pdev->dev, PTR_ERR(plat),
+ "dt configuration failed\n");
+
+ dwmac->dev = &pdev->dev;
+ plat->bsp_priv = dwmac;
+ plat->fix_mac_speed = dwmac_starfive_fix_mac_speed;
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res);
+ if (ret) {
+ stmmac_remove_config_dt(pdev, plat);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id dwmac_starfive_match[] = {
+ {
+ .compatible = "starfive,jh7100-dwmac",
+ .data = dwmac_starfive_jh7100_syscon_init,
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dwmac_starfive_match);
+
+static struct platform_driver dwmac_starfive_driver = {
+ .probe = dwmac_starfive_probe,
+ .remove = stmmac_pltfr_remove,
+ .driver = {
+ .name = "dwmac-starfive",
+ .pm = &stmmac_pltfr_pm_ops,
+ .of_match_table = dwmac_starfive_match,
+ },
+};
+module_platform_driver(dwmac_starfive_driver);
+
+MODULE_AUTHOR("Emil Renner Berthing <[email protected]>");
+MODULE_DESCRIPTION("StarFive DWMAC Glue Layer");
+MODULE_LICENSE("GPL");
--
2.39.1


2023-02-11 03:19:56

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 09/12] riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC

The RISC-V architecture is by default coherent, as indicated by
CONFIG_OF_DMA_DEFAULT_COHERENT, but the StarFive JH7100 is not, hence
provide the dma-noncoherent property to the soc DT node.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7100.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 000447482aca..7109e70fdab8 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -114,6 +114,7 @@ soc {
#address-cells = <2>;
#size-cells = <2>;
ranges;
+ dma-noncoherent;

clint: clint@2000000 {
compatible = "starfive,jh7100-clint", "sifive,clint0";
--
2.39.1


2023-02-11 03:19:58

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 10/12] riscv: dts: starfive: jh7100: Add ccache DT node

Provide a DT node for the Sifive Composable Cache controller found on
the StarFive JH7100 SoC.

Note this is also used to support non-coherent DMA.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7100.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 7109e70fdab8..88f91bc5753b 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -32,6 +32,7 @@ U74_0: cpu@0 {
i-tlb-sets = <1>;
i-tlb-size = <32>;
mmu-type = "riscv,sv39";
+ next-level-cache = <&ccache>;
riscv,isa = "rv64imafdc";
tlb-split;

@@ -57,6 +58,7 @@ U74_1: cpu@1 {
i-tlb-sets = <1>;
i-tlb-size = <32>;
mmu-type = "riscv,sv39";
+ next-level-cache = <&ccache>;
riscv,isa = "rv64imafdc";
tlb-split;

@@ -116,6 +118,20 @@ soc {
ranges;
dma-noncoherent;

+ ccache: cache-controller@2010000 {
+ compatible = "starfive,jh7100-ccache", "cache";
+ reg = <0x0 0x2010000 0x0 0x1000>,
+ <0x0 0x8000000 0x0 0x2000000>;
+ reg-names = "control", "sideband";
+ interrupts = <128>, <130>, <131>, <129>;
+ cache-block-size = <64>;
+ cache-level = <2>;
+ cache-sets = <2048>;
+ cache-size = <2097152>;
+ cache-unified;
+ uncached-offset = <0xf 0x80000000>;
+ };
+
clint: clint@2000000 {
compatible = "starfive,jh7100-clint", "sifive,clint0";
reg = <0x0 0x2000000 0x0 0x10000>;
--
2.39.1


2023-02-11 03:20:00

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 11/12] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

Provide the sysmain and gmac DT nodes supporting the DWMAC found on the
StarFive JH7100 SoC.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
arch/riscv/boot/dts/starfive/jh7100.dtsi | 38 ++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 88f91bc5753b..0918af7b6eb0 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -164,6 +164,44 @@ rstgen: reset-controller@11840000 {
#reset-cells = <1>;
};

+ sysmain: syscon@11850000 {
+ compatible = "starfive,jh7100-sysmain", "syscon";
+ reg = <0x0 0x11850000 0x0 0x10000>;
+ };
+
+ gmac: ethernet@10020000 {
+ compatible = "starfive,jh7100-dwmac", "snps,dwmac";
+ reg = <0x0 0x10020000 0x0 0x10000>;
+ clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
+ <&clkgen JH7100_CLK_GMAC_AHB>,
+ <&clkgen JH7100_CLK_GMAC_PTP_REF>,
+ <&clkgen JH7100_CLK_GMAC_GTX>,
+ <&clkgen JH7100_CLK_GMAC_TX_INV>;
+ clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
+ resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
+ reset-names = "ahb";
+ interrupts = <6>, <7>;
+ interrupt-names = "macirq", "eth_wake_irq";
+ max-frame-size = <9000>;
+ phy-mode = "rgmii-txid";
+ snps,multicast-filter-bins = <32>;
+ snps,perfect-filter-entries = <128>;
+ starfive,syscon = <&sysmain>;
+ rx-fifo-depth = <32768>;
+ tx-fifo-depth = <16384>;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,fixed-burst;
+ snps,force_thresh_dma_mode;
+ snps,no-pbl-x8;
+ status = "disabled";
+
+ stmmac_axi_setup: stmmac-axi-config {
+ snps,wr_osr_lmt = <0xf>;
+ snps,rd_osr_lmt = <0xf>;
+ snps,blen = <256 128 64 32 0 0 0>;
+ };
+ };
+
i2c0: i2c@118b0000 {
compatible = "snps,designware-i2c";
reg = <0x0 0x118b0000 0x0 0x10000>;
--
2.39.1


2023-02-11 03:20:20

by Cristian Ciocaltea

[permalink] [raw]
Subject: [PATCH 12/12] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

Add pinmux configuration for the DWMAC found on the JH7100 based boards
and enable the gmac DT node.

Signed-off-by: Cristian Ciocaltea <[email protected]>
---
.../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++++++++++++
1 file changed, 78 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index b93ce351a90f..9927e7462e9f 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -41,7 +41,85 @@ led-ack {
};
};

+&gmac {
+ starfive,gtxclk-dlychain = <4>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&gmac_pins>;
+ status = "okay";
+};
+
&gpio {
+ gmac_pins: gmac-0 {
+ gtxclk-pins {
+ pins = <PAD_FUNC_SHARE(115)>;
+ bias-pull-up;
+ drive-strength = <35>;
+ input-enable;
+ input-schmitt-enable;
+ slew-rate = <0>;
+ };
+ miitxclk-pins {
+ pins = <PAD_FUNC_SHARE(116)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ tx-pins {
+ pins = <PAD_FUNC_SHARE(117)>,
+ <PAD_FUNC_SHARE(119)>,
+ <PAD_FUNC_SHARE(120)>,
+ <PAD_FUNC_SHARE(121)>,
+ <PAD_FUNC_SHARE(122)>,
+ <PAD_FUNC_SHARE(123)>,
+ <PAD_FUNC_SHARE(124)>,
+ <PAD_FUNC_SHARE(125)>,
+ <PAD_FUNC_SHARE(126)>;
+ bias-pull-up;
+ drive-strength = <35>;
+ input-disable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ rxclk-pins {
+ pins = <PAD_FUNC_SHARE(127)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-disable;
+ slew-rate = <6>;
+ };
+ rxer-pins {
+ pins = <PAD_FUNC_SHARE(129)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ rx-pins {
+ pins = <PAD_FUNC_SHARE(128)>,
+ <PAD_FUNC_SHARE(130)>,
+ <PAD_FUNC_SHARE(131)>,
+ <PAD_FUNC_SHARE(132)>,
+ <PAD_FUNC_SHARE(133)>,
+ <PAD_FUNC_SHARE(134)>,
+ <PAD_FUNC_SHARE(135)>,
+ <PAD_FUNC_SHARE(136)>,
+ <PAD_FUNC_SHARE(137)>,
+ <PAD_FUNC_SHARE(138)>,
+ <PAD_FUNC_SHARE(139)>,
+ <PAD_FUNC_SHARE(140)>,
+ <PAD_FUNC_SHARE(141)>;
+ bias-pull-up;
+ drive-strength = <14>;
+ input-enable;
+ input-schmitt-enable;
+ slew-rate = <0>;
+ };
+ };
+
i2c0_pins: i2c0-0 {
i2c-pins {
pinmux = <GPIOMUX(62, GPO_LOW,
--
2.39.1


2023-02-11 11:12:00

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 00/12] Enable networking support for StarFive JH7100 SoC

Hey Cristian!

+CC Arnd, Prabhakar

On Sat, Feb 11, 2023 at 05:18:09AM +0200, Cristian Ciocaltea wrote:
> This patch series adds ethernet support for the StarFive JH7100 SoC and
> makes it available for the StarFive VisionFive V1 and BeagleV Starlight
> boards, although I could only validate on the former SBC.
>
> The work is heavily based on the reference implementation [1] and requires
> the non-coherent DMA support provided by Emil via the Sifive Composable
> Cache controller.
>
> Also note there is an overlap in "[PATCH 08/12] net: stmmac: Add glue layer
> for StarFive JH7100 SoC" with the Yanhong Wang's upstreaming attempt [2]:
> "[PATCH v4 5/7] net: stmmac: Add glue layer for StarFive JH7110 SoCs".
>
> Since I cannot test the JH7110 SoC, I dropped the support for it from Emil's
> variant of the stmmac glue layer. Hence, we might need a bit of coordination
> in order to get this properly merged.

To be honest, that one is the least of your worries sequencing wise.
Anything doing non-coherent DMA on RISC-V that doesn't use instructions is
dependant on Prabhakar's series:
https://lore.kernel.org/linux-riscv/[email protected]/#t
That's kinda stalled out though, waiting on Arnd to make some changes to
the cross-arch DMA code:
https://lore.kernel.org/linux-riscv/[email protected]/

I was talking to Emil about the non-coherent support at FOSDEM actually,
and I see no real reason not to bring the JH7100 non-coherent support in
if we are doing it for other SoCs.

So yeah, hopefully we shall get there at some point soonTM...

Sorry,
Conor.

> [1] https://github.com/starfive-tech/linux/commits/visionfive
> [2] https://lore.kernel.org/linux-riscv/[email protected]/
>
> Cristian Ciocaltea (7):
> dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100
> SoC
> dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property
> dt-bindings: net: Add StarFive JH7100 SoC
> riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC
> riscv: dts: starfive: jh7100: Add ccache DT node
> riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes
> riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac
>
> Emil Renner Berthing (5):
> soc: sifive: ccache: Add StarFive JH7100 support
> soc: sifive: ccache: Add non-coherent DMA handling
> riscv: Implement non-coherent DMA support via SiFive cache flushing
> dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible
> net: stmmac: Add glue layer for StarFive JH7100 SoC
>
> .../devicetree/bindings/mfd/syscon.yaml | 1 +
> .../devicetree/bindings/net/snps,dwmac.yaml | 15 +-
> .../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++
> .../bindings/riscv/sifive,ccache0.yaml | 33 +++-
> MAINTAINERS | 6 +
> arch/riscv/Kconfig | 6 +-
> .../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++
> arch/riscv/boot/dts/starfive/jh7100.dtsi | 55 +++++++
> arch/riscv/mm/dma-noncoherent.c | 37 ++++-
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++
> drivers/soc/sifive/Kconfig | 1 +
> drivers/soc/sifive/sifive_ccache.c | 71 +++++++-
> include/soc/sifive/sifive_ccache.h | 21 +++
> 15 files changed, 587 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>
> --
> 2.39.1
>


Attachments:
(No filename) (3.70 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-11 11:54:54

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 00/12] Enable networking support for StarFive JH7100 SoC

Hi Conor,

On 2/11/23 13:11, Conor Dooley wrote:
> Hey Cristian!
>
> +CC Arnd, Prabhakar
>
> On Sat, Feb 11, 2023 at 05:18:09AM +0200, Cristian Ciocaltea wrote:
>> This patch series adds ethernet support for the StarFive JH7100 SoC and
>> makes it available for the StarFive VisionFive V1 and BeagleV Starlight
>> boards, although I could only validate on the former SBC.
>>
>> The work is heavily based on the reference implementation [1] and requires
>> the non-coherent DMA support provided by Emil via the Sifive Composable
>> Cache controller.
>>
>> Also note there is an overlap in "[PATCH 08/12] net: stmmac: Add glue layer
>> for StarFive JH7100 SoC" with the Yanhong Wang's upstreaming attempt [2]:
>> "[PATCH v4 5/7] net: stmmac: Add glue layer for StarFive JH7110 SoCs".
>>
>> Since I cannot test the JH7110 SoC, I dropped the support for it from Emil's
>> variant of the stmmac glue layer. Hence, we might need a bit of coordination
>> in order to get this properly merged.
>
> To be honest, that one is the least of your worries sequencing wise.
> Anything doing non-coherent DMA on RISC-V that doesn't use instructions is
> dependant on Prabhakar's series:
> https://lore.kernel.org/linux-riscv/[email protected]/#t
> That's kinda stalled out though, waiting on Arnd to make some changes to
> the cross-arch DMA code:
> https://lore.kernel.org/linux-riscv/[email protected]/

Thank you for pointing this out, I wasn't aware of it!

> I was talking to Emil about the non-coherent support at FOSDEM actually,
> and I see no real reason not to bring the JH7100 non-coherent support in
> if we are doing it for other SoCs.
>
> So yeah, hopefully we shall get there at some point soonTM...

That would be great, I'll try to monitor the progress and re-spin the
series as soon as the non-coherent support is figured out.

Regards,
Cristian

> Sorry,
> Conor.

>> [1] https://github.com/starfive-tech/linux/commits/visionfive
>> [2] https://lore.kernel.org/linux-riscv/[email protected]/
>>
>> Cristian Ciocaltea (7):
>> dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100
>> SoC
>> dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property
>> dt-bindings: net: Add StarFive JH7100 SoC
>> riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC
>> riscv: dts: starfive: jh7100: Add ccache DT node
>> riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes
>> riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac
>>
>> Emil Renner Berthing (5):
>> soc: sifive: ccache: Add StarFive JH7100 support
>> soc: sifive: ccache: Add non-coherent DMA handling
>> riscv: Implement non-coherent DMA support via SiFive cache flushing
>> dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible
>> net: stmmac: Add glue layer for StarFive JH7100 SoC
>>
>> .../devicetree/bindings/mfd/syscon.yaml | 1 +
>> .../devicetree/bindings/net/snps,dwmac.yaml | 15 +-
>> .../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++
>> .../bindings/riscv/sifive,ccache0.yaml | 33 +++-
>> MAINTAINERS | 6 +
>> arch/riscv/Kconfig | 6 +-
>> .../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++
>> arch/riscv/boot/dts/starfive/jh7100.dtsi | 55 +++++++
>> arch/riscv/mm/dma-noncoherent.c | 37 ++++-
>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++
>> drivers/soc/sifive/Kconfig | 1 +
>> drivers/soc/sifive/sifive_ccache.c | 71 +++++++-
>> include/soc/sifive/sifive_ccache.h | 21 +++
>> 15 files changed, 587 insertions(+), 11 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>
>> --
>> 2.39.1
>>

2023-02-11 16:02:01

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

> + starfive,gtxclk-dlychain:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: GTX clock delay chain setting

Please could you add more details to this. Is this controlling the
RGMII delays? 0ns or 2ns?

> + gmac: ethernet@10020000 {
> + compatible = "starfive,jh7100-dwmac", "snps,dwmac";
> + reg = <0x0 0x10020000 0x0 0x10000>;
> + clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
> + <&clkgen JH7100_CLK_GMAC_AHB>,
> + <&clkgen JH7100_CLK_GMAC_PTP_REF>,
> + <&clkgen JH7100_CLK_GMAC_GTX>,
> + <&clkgen JH7100_CLK_GMAC_TX_INV>;
> + clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
> + resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
> + reset-names = "ahb";
> + interrupts = <6>, <7>;
> + interrupt-names = "macirq", "eth_wake_irq";
> + max-frame-size = <9000>;
> + phy-mode = "rgmii-txid";

This is unusual. Does your board have a really long RX clock line to
insert the 2ns delay needed on the RX side?

Andrew

2023-02-11 16:11:57

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

> +
> +#define JH7100_SYSMAIN_REGISTER28 0x70
> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
> +#define JH7100_SYSMAIN_REGISTER49 0xc8

Seems like the comment should be one line earlier?

There is value in basing the names on the datasheet, but you could
append something meaningful on the end:

#define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8

???

> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
> + if (ret)
> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
> + }

You should probably document that if starfive,gtxclk-dlychain is not
found in the DT blob, the value for the delay chain is undefined. It
would actually be better to define it, set it to 0 for example. That
way, you know you don't have any dependency on the bootloader for
example.

Andrew

2023-02-13 09:20:13

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Document the compatible for the SiFive Composable Cache Controller found
> on the StarFive JH7100 SoC.
>
> This also requires extending the 'reg' property to handle distinct
> ranges, as specified via 'reg-names'.


Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-02-13 09:23:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Add the 'uncached-offset' property to be used for specifying the
> uncached memory offset required for handling non-coherent DMA
> transactions.

Only one offset can be non-coherent? If this is for DMA, why
dma-noncoherent cannot be used?

Best regards,
Krzysztof


2023-02-13 09:23:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 06/12] dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> From: Emil Renner Berthing <[email protected]>
>
> Document StarFive JH7100 SoC compatible for sysmain registers.
>


Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-02-13 09:27:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Add DT bindings documentation for the Synopsys DesignWare MAC found on
> the StarFive JH7100 SoC.
>
> Adjust 'reset' and 'reset-names' properties to allow using 'ahb' instead
> of the 'stmmaceth' reset signal, as required by JH7100.
>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 15 ++-
> .../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++++++++


FYI, there is conflicting work:

https://lore.kernel.org/all/[email protected]/

It's almost the same, thus this should be dropped.

Best regards,
Krzysztof


2023-02-13 09:29:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> From: Emil Renner Berthing <[email protected]>
>
> This adds a glue layer for the Synopsys DesignWare MAC IP core on the
> StarFive JH7100 SoC.
>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> [drop references to JH7110, update JH7100 compatible string]
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++
> 4 files changed, 169 insertions(+)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d48468b81b94..defedaff6041 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19820,6 +19820,7 @@ STARFIVE DWMAC GLUE LAYER
> M: Emil Renner Berthing <[email protected]>
> S: Maintained
> F: Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>
> STARFIVE JH7100 CLOCK DRIVERS
> M: Emil Renner Berthing <[email protected]>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index f77511fe4e87..2c81aa594291 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
> for the stmmac device driver. This driver is used for
> arria5 and cyclone5 FPGA SoCs.
>
> +config DWMAC_STARFIVE
> + tristate "StarFive DWMAC support"

Bring only one driver.

https://lore.kernel.org/all/[email protected]/

Best regards,
Krzysztof


2023-02-13 09:30:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 11/12] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Provide the sysmain and gmac DT nodes supporting the DWMAC found on the
> StarFive JH7100 SoC.
>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> arch/riscv/boot/dts/starfive/jh7100.dtsi | 38 ++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index 88f91bc5753b..0918af7b6eb0 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -164,6 +164,44 @@ rstgen: reset-controller@11840000 {
> #reset-cells = <1>;
> };
>
> + sysmain: syscon@11850000 {
> + compatible = "starfive,jh7100-sysmain", "syscon";
> + reg = <0x0 0x11850000 0x0 0x10000>;
> + };
> +
> + gmac: ethernet@10020000 {

Aren't the nodes ordered by address?

Best regards,
Krzysztof


2023-02-13 10:12:09

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

On 11/02/2023 03:18, Cristian Ciocaltea wrote:
> From: Emil Renner Berthing <[email protected]>
>
> This variant is used on the StarFive JH7100 SoC.
>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> arch/riscv/Kconfig | 6 ++++--
> arch/riscv/mm/dma-noncoherent.c | 37 +++++++++++++++++++++++++++++++--
> 2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 9c687da7756d..05f6c77faf6f 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -232,12 +232,14 @@ config LOCKDEP_SUPPORT
> def_bool y
>
> config RISCV_DMA_NONCOHERENT
> - bool
> + bool "Support non-coherent DMA"
> + default SOC_STARFIVE
> select ARCH_HAS_DMA_PREP_COHERENT
> + select ARCH_HAS_DMA_SET_UNCACHED
> + select ARCH_HAS_DMA_CLEAR_UNCACHED
> select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> select ARCH_HAS_SYNC_DMA_FOR_CPU
> select ARCH_HAS_SETUP_DMA_OPS
> - select DMA_DIRECT_REMAP
>
> config AS_HAS_INSN
> def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero)
> diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
> index d919efab6eba..e07e53aea537 100644
> --- a/arch/riscv/mm/dma-noncoherent.c
> +++ b/arch/riscv/mm/dma-noncoherent.c
> @@ -9,14 +9,21 @@
> #include <linux/dma-map-ops.h>
> #include <linux/mm.h>
> #include <asm/cacheflush.h>
> +#include <soc/sifive/sifive_ccache.h>
>
> static bool noncoherent_supported;
>
> void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
> enum dma_data_direction dir)
> {
> - void *vaddr = phys_to_virt(paddr);
> + void *vaddr;
>
> + if (sifive_ccache_handle_noncoherent()) {
> + sifive_ccache_flush_range(paddr, size);
> + return;
> + }
> +
> + vaddr = phys_to_virt(paddr);
> switch (dir) {
> case DMA_TO_DEVICE:
> ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
> @@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
> void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
> enum dma_data_direction dir)
> {
> - void *vaddr = phys_to_virt(paddr);
> + void *vaddr;
> +
> + if (sifive_ccache_handle_noncoherent()) {
> + sifive_ccache_flush_range(paddr, size);
> + return;
> + }

ok, what happens if we have an system where the ccache and another level
of cache also requires maintenance operations?

>
> + vaddr = phys_to_virt(paddr);
> switch (dir) {
> case DMA_TO_DEVICE:
> break;
> @@ -49,10 +62,30 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
> }
> }
>
> +void *arch_dma_set_uncached(void *addr, size_t size)
> +{
> + if (sifive_ccache_handle_noncoherent())
> + return sifive_ccache_set_uncached(addr, size);
> +
> + return addr;
> +}
> +
> +void arch_dma_clear_uncached(void *addr, size_t size)
> +{
> + if (sifive_ccache_handle_noncoherent())
> + sifive_ccache_clear_uncached(addr, size);
> +}
> +
> void arch_dma_prep_coherent(struct page *page, size_t size)
> {
> void *flush_addr = page_address(page);
>
> + if (sifive_ccache_handle_noncoherent()) {
> + memset(flush_addr, 0, size);
> + sifive_ccache_flush_range(__pa(flush_addr), size);
> + return;
> + }
> +
> ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size);
> }
>

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html


2023-02-14 17:58:28

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

On 2/13/23 11:23, Krzysztof Kozlowski wrote:
> On 11/02/2023 04:18, Cristian Ciocaltea wrote:
>> Add the 'uncached-offset' property to be used for specifying the
>> uncached memory offset required for handling non-coherent DMA
>> transactions.
>
> Only one offset can be non-coherent? If this is for DMA, why
> dma-noncoherent cannot be used?

As Conor already mentioned in [1], the handling of non-coherent DMA on
RISC-V is currently being worked on, so I expect this patch will be dropped.

[1] https://lore.kernel.org/lkml/Y+d36nz0xdfXmDI1@spud/

Thanks for reviewing,
Cristian

>
> Best regards,
> Krzysztof
>

2023-02-14 18:07:01

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

On 2/13/23 10:30, Ben Dooks wrote:
> On 11/02/2023 03:18, Cristian Ciocaltea wrote:
>> From: Emil Renner Berthing <[email protected]>
>>
>> This variant is used on the StarFive JH7100 SoC.
>>
>> Signed-off-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>>   arch/riscv/Kconfig              |  6 ++++--
>>   arch/riscv/mm/dma-noncoherent.c | 37 +++++++++++++++++++++++++++++++--
>>   2 files changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 9c687da7756d..05f6c77faf6f 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -232,12 +232,14 @@ config LOCKDEP_SUPPORT
>>       def_bool y
>>   config RISCV_DMA_NONCOHERENT
>> -    bool
>> +    bool "Support non-coherent DMA"
>> +    default SOC_STARFIVE
>>       select ARCH_HAS_DMA_PREP_COHERENT
>> +    select ARCH_HAS_DMA_SET_UNCACHED
>> +    select ARCH_HAS_DMA_CLEAR_UNCACHED
>>       select ARCH_HAS_SYNC_DMA_FOR_DEVICE
>>       select ARCH_HAS_SYNC_DMA_FOR_CPU
>>       select ARCH_HAS_SETUP_DMA_OPS
>> -    select DMA_DIRECT_REMAP
>>   config AS_HAS_INSN
>>       def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma)
>> t0$(comma) t0$(comma) zero)
>> diff --git a/arch/riscv/mm/dma-noncoherent.c
>> b/arch/riscv/mm/dma-noncoherent.c
>> index d919efab6eba..e07e53aea537 100644
>> --- a/arch/riscv/mm/dma-noncoherent.c
>> +++ b/arch/riscv/mm/dma-noncoherent.c
>> @@ -9,14 +9,21 @@
>>   #include <linux/dma-map-ops.h>
>>   #include <linux/mm.h>
>>   #include <asm/cacheflush.h>
>> +#include <soc/sifive/sifive_ccache.h>
>>   static bool noncoherent_supported;
>>   void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
>>                     enum dma_data_direction dir)
>>   {
>> -    void *vaddr = phys_to_virt(paddr);
>> +    void *vaddr;
>> +    if (sifive_ccache_handle_noncoherent()) {
>> +        sifive_ccache_flush_range(paddr, size);
>> +        return;
>> +    }
>> +
>> +    vaddr = phys_to_virt(paddr);
>>       switch (dir) {
>>       case DMA_TO_DEVICE:
>>           ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
>> @@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr,
>> size_t size,
>>   void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
>>                  enum dma_data_direction dir)
>>   {
>> -    void *vaddr = phys_to_virt(paddr);
>> +    void *vaddr;
>> +
>> +    if (sifive_ccache_handle_noncoherent()) {
>> +        sifive_ccache_flush_range(paddr, size);
>> +        return;
>> +    }
>
> ok, what happens if we have an system where the ccache and another level
> of cache also requires maintenance operations?

According to [1], the handling of non-coherent DMA on RISC-V is
currently being worked on, so I will respin the series as soon as the
proper support arrives.

[1] https://lore.kernel.org/lkml/Y+d36nz0xdfXmDI1@spud/


>> +    vaddr = phys_to_virt(paddr);
>>       switch (dir) {
>>       case DMA_TO_DEVICE:
>>           break;
>> @@ -49,10 +62,30 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr,
>> size_t size,
>>       }
>>   }
>> +void *arch_dma_set_uncached(void *addr, size_t size)
>> +{
>> +    if (sifive_ccache_handle_noncoherent())
>> +        return sifive_ccache_set_uncached(addr, size);
>> +
>> +    return addr;
>> +}
>> +
>> +void arch_dma_clear_uncached(void *addr, size_t size)
>> +{
>> +    if (sifive_ccache_handle_noncoherent())
>> +        sifive_ccache_clear_uncached(addr, size);
>> +}
>> +
>>   void arch_dma_prep_coherent(struct page *page, size_t size)
>>   {
>>       void *flush_addr = page_address(page);
>> +    if (sifive_ccache_handle_noncoherent()) {
>> +        memset(flush_addr, 0, size);
>> +        sifive_ccache_flush_range(__pa(flush_addr), size);
>> +        return;
>> +    }
>> +
>>       ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size);
>>   }
>

2023-02-14 18:12:52

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

On 2/13/23 11:26, Krzysztof Kozlowski wrote:
> On 11/02/2023 04:18, Cristian Ciocaltea wrote:
>> From: Emil Renner Berthing <[email protected]>
>>
>> This adds a glue layer for the Synopsys DesignWare MAC IP core on the
>> StarFive JH7100 SoC.
>>
>> Signed-off-by: Emil Renner Berthing <[email protected]>
>> [drop references to JH7110, update JH7100 compatible string]
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> MAINTAINERS | 1 +
>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++
>> 4 files changed, 169 insertions(+)
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d48468b81b94..defedaff6041 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19820,6 +19820,7 @@ STARFIVE DWMAC GLUE LAYER
>> M: Emil Renner Berthing <[email protected]>
>> S: Maintained
>> F: Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
>> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
>>
>> STARFIVE JH7100 CLOCK DRIVERS
>> M: Emil Renner Berthing <[email protected]>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index f77511fe4e87..2c81aa594291 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
>> for the stmmac device driver. This driver is used for
>> arria5 and cyclone5 FPGA SoCs.
>>
>> +config DWMAC_STARFIVE
>> + tristate "StarFive DWMAC support"
>
> Bring only one driver.
>
> https://lore.kernel.org/all/[email protected]/

Already mentioned in the cover letter that we have this overlap (will be
merged into a single driver).

Thanks,
Cristian

>
> Best regards,
> Krzysztof
>
>

2023-02-14 18:16:09

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 11/12] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

On 2/13/23 11:26, Krzysztof Kozlowski wrote:
> On 11/02/2023 04:18, Cristian Ciocaltea wrote:
>> Provide the sysmain and gmac DT nodes supporting the DWMAC found on the
>> StarFive JH7100 SoC.
>>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> arch/riscv/boot/dts/starfive/jh7100.dtsi | 38 ++++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
>> index 88f91bc5753b..0918af7b6eb0 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
>> @@ -164,6 +164,44 @@ rstgen: reset-controller@11840000 {
>> #reset-cells = <1>;
>> };
>>
>> + sysmain: syscon@11850000 {
>> + compatible = "starfive,jh7100-sysmain", "syscon";
>> + reg = <0x0 0x11850000 0x0 0x10000>;
>> + };
>> +
>> + gmac: ethernet@10020000 {
>
> Aren't the nodes ordered by address?

Right, I missed the ordering trying to keep the related nodes together.
Will fix.

Thanks,
Cristian

> Best regards,
> Krzysztof
>
>

2023-02-14 18:18:01

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

On Tue, Feb 14, 2023 at 08:06:49PM +0200, Cristian Ciocaltea wrote:
> On 2/13/23 10:30, Ben Dooks wrote:
> > On 11/02/2023 03:18, Cristian Ciocaltea wrote:
> > > From: Emil Renner Berthing <[email protected]>

> > > diff --git a/arch/riscv/mm/dma-noncoherent.c
> > > b/arch/riscv/mm/dma-noncoherent.c
> > > index d919efab6eba..e07e53aea537 100644
> > > --- a/arch/riscv/mm/dma-noncoherent.c
> > > +++ b/arch/riscv/mm/dma-noncoherent.c
> > > @@ -9,14 +9,21 @@
> > > ? #include <linux/dma-map-ops.h>
> > > ? #include <linux/mm.h>
> > > ? #include <asm/cacheflush.h>
> > > +#include <soc/sifive/sifive_ccache.h>
> > > ? static bool noncoherent_supported;
> > > ? void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
> > > ??????????????????? enum dma_data_direction dir)
> > > ? {
> > > -??? void *vaddr = phys_to_virt(paddr);
> > > +??? void *vaddr;
> > > +??? if (sifive_ccache_handle_noncoherent()) {
> > > +??????? sifive_ccache_flush_range(paddr, size);
> > > +??????? return;
> > > +??? }
> > > +
> > > +??? vaddr = phys_to_virt(paddr);
> > > ????? switch (dir) {
> > > ????? case DMA_TO_DEVICE:
> > > ????????? ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
> > > @@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr,
> > > size_t size,
> > > ? void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
> > > ???????????????? enum dma_data_direction dir)
> > > ? {
> > > -??? void *vaddr = phys_to_virt(paddr);
> > > +??? void *vaddr;
> > > +
> > > +??? if (sifive_ccache_handle_noncoherent()) {
> > > +??????? sifive_ccache_flush_range(paddr, size);
> > > +??????? return;
> > > +??? }
> >
> > ok, what happens if we have an system where the ccache and another level
> > of cache also requires maintenance operations?

TBH, I'd hope that a system with that complexity is also not trying to
manage the cache in this manner!

> According to [1], the handling of non-coherent DMA on RISC-V is currently
> being worked on, so I will respin the series as soon as the proper support
> arrives.

But yeah, once that stuff lands we can carry out these operations only
for the platforms that need/"need" it.

Cheers,
Conor.


Attachments:
(No filename) (2.10 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-14 20:40:49

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

Hey all,

On Sat, Feb 11, 2023 at 05:18:10AM +0200, Cristian Ciocaltea wrote:
> Document the compatible for the SiFive Composable Cache Controller found
> on the StarFive JH7100 SoC.
>
> This also requires extending the 'reg' property to handle distinct
> ranges, as specified via 'reg-names'.
>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> .../bindings/riscv/sifive,ccache0.yaml | 28 ++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> index 31d20efaa6d3..2b864b2f12c9 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> @@ -25,6 +25,7 @@ select:
> - sifive,ccache0
> - sifive,fu540-c000-ccache
> - sifive,fu740-c000-ccache
> + - starfive,jh7100-ccache
>
> required:
> - compatible
> @@ -37,6 +38,7 @@ properties:
> - sifive,ccache0
> - sifive,fu540-c000-ccache
> - sifive,fu740-c000-ccache
> + - starfive,jh7100-ccache
> - const: cache
> - items:
> - const: starfive,jh7110-ccache
> @@ -70,7 +72,13 @@ properties:
> - description: DirFail interrupt
>
> reg:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: control
> + - const: sideband

So why is this called "sideband"?
In the docs for the JH7100 it is called LIM & it's called LIM in our
docs for the PolarFire SoC (at the same address btw) and we run the HSS
out of it! LIM being "loosely integrated memory", which by the limit
hits on Google may be a SiFive-ism?

I'm not really sure if adding it as a "reg" section is the right thing
to do as it's not "just" a register bank.
Perhaps Rob/Krzysztof have a take on that one?

>
> next-level-cache: true
>
> @@ -89,6 +97,7 @@ allOf:
> contains:
> enum:
> - sifive,fu740-c000-ccache
> + - starfive,jh7100-ccache
> - starfive,jh7110-ccache
> - microchip,mpfs-ccache
>
> @@ -106,12 +115,29 @@ allOf:
> Must contain entries for DirError, DataError and DataFail signals.
> maxItems: 3
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7100-ccache
> +
> + then:
> + properties:
> + reg:
> + maxItems: 2
> +
> + else:
> + properties:
> + reg:
> + maxItems: 1
> +
> - if:
> properties:
> compatible:
> contains:
> enum:
> - sifive,fu740-c000-ccache
> + - starfive,jh7100-ccache
> - starfive,jh7110-ccache
>
> then:
> --
> 2.39.1
>


Attachments:
(No filename) (2.90 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-15 00:08:12

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

On 2/11/23 18:11, Andrew Lunn wrote:
>> +
>> +#define JH7100_SYSMAIN_REGISTER28 0x70
>> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
>> +#define JH7100_SYSMAIN_REGISTER49 0xc8
>
> Seems like the comment should be one line earlier?
>
> There is value in basing the names on the datasheet, but you could
> append something meaningful on the end:
>
> #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8
>
> ???

Unfortunately the JH7100 datasheet I have access to doesn't provide any
information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil
could provide some details here?

>> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
>> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
>> + }
>
> You should probably document that if starfive,gtxclk-dlychain is not
> found in the DT blob, the value for the delay chain is undefined. It
> would actually be better to define it, set it to 0 for example. That
> way, you know you don't have any dependency on the bootloader for
> example.

Sure, I will set it to 0.

>
> Andrew

Thanks for reviewing,
Cristian

2023-02-15 00:34:41

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 2/11/23 18:01, Andrew Lunn wrote:
>> + starfive,gtxclk-dlychain:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: GTX clock delay chain setting
>
> Please could you add more details to this. Is this controlling the
> RGMII delays? 0ns or 2ns?

This is what gets written to JH7100_SYSMAIN_REGISTER49 and it's
currently set to 4 in patch 12/12. As already mentioned, I don't have
the register information in the datasheet, but I'll update this as soon
as we get some details.

>> + gmac: ethernet@10020000 {
>> + compatible = "starfive,jh7100-dwmac", "snps,dwmac";
>> + reg = <0x0 0x10020000 0x0 0x10000>;
>> + clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
>> + <&clkgen JH7100_CLK_GMAC_AHB>,
>> + <&clkgen JH7100_CLK_GMAC_PTP_REF>,
>> + <&clkgen JH7100_CLK_GMAC_GTX>,
>> + <&clkgen JH7100_CLK_GMAC_TX_INV>;
>> + clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
>> + resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
>> + reset-names = "ahb";
>> + interrupts = <6>, <7>;
>> + interrupt-names = "macirq", "eth_wake_irq";
>> + max-frame-size = <9000>;
>> + phy-mode = "rgmii-txid";
>
> This is unusual. Does your board have a really long RX clock line to
> insert the 2ns delay needed on the RX side?

Just tested with "rgmii" and didn't notice any issues. If I'm not
missing anything, I'll do the change in the next revision.

> Andrew

Thanks,
Cristian

2023-02-15 11:21:33

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea
<[email protected]> wrote:
>
> On 2/11/23 18:11, Andrew Lunn wrote:
> >> +
> >> +#define JH7100_SYSMAIN_REGISTER28 0x70
> >> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
> >> +#define JH7100_SYSMAIN_REGISTER49 0xc8
> >
> > Seems like the comment should be one line earlier?

Well yes, the very generic register names are also bad, but this
comment refers to the fact that it kind of makes sense that register
28 has the offset
28 * 4 bytes pr. register = 0x70
..but then register 49 is oddly out of place at offset 0xc8 instead of
49 * 4 bytes pr. register = 0xc4

> > There is value in basing the names on the datasheet, but you could
> > append something meaningful on the end:
> >
> > #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8
>
> Unfortunately the JH7100 datasheet I have access to doesn't provide any
> information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil
> could provide some details here?

This is reverse engineered from the auto generated headers in their u-boot:
https://github.com/starfive-tech/u-boot/blob/JH7100_VisionFive_devel/arch/riscv/include/asm/arch-jh7100/syscon_sysmain_ctrl_macro.h

Christian, I'm happy that you're working on this, but mess like this
and waiting for the non-coherent dma to be sorted is why I didn't send
it upstream yet.

> >> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
> >> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
> >> + if (ret)
> >> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
> >> + }
> >
> > You should probably document that if starfive,gtxclk-dlychain is not
> > found in the DT blob, the value for the delay chain is undefined. It
> > would actually be better to define it, set it to 0 for example. That
> > way, you know you don't have any dependency on the bootloader for
> > example.
>
> Sure, I will set it to 0.
>
> >
> > Andrew
>
> Thanks for reviewing,
> Cristian
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-15 11:51:37

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

On 2/15/23 13:20, Emil Renner Berthing wrote:
> On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea
> <[email protected]> wrote:
>>
>> On 2/11/23 18:11, Andrew Lunn wrote:
>>>> +
>>>> +#define JH7100_SYSMAIN_REGISTER28 0x70
>>>> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
>>>> +#define JH7100_SYSMAIN_REGISTER49 0xc8
>>>
>>> Seems like the comment should be one line earlier?
>
> Well yes, the very generic register names are also bad, but this
> comment refers to the fact that it kind of makes sense that register
> 28 has the offset
> 28 * 4 bytes pr. register = 0x70
> ..but then register 49 is oddly out of place at offset 0xc8 instead of
> 49 * 4 bytes pr. register = 0xc4
>
>>> There is value in basing the names on the datasheet, but you could
>>> append something meaningful on the end:
>>>
>>> #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8
>>
>> Unfortunately the JH7100 datasheet I have access to doesn't provide any
>> information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil
>> could provide some details here?
>
> This is reverse engineered from the auto generated headers in their u-boot:
> https://github.com/starfive-tech/u-boot/blob/JH7100_VisionFive_devel/arch/riscv/include/asm/arch-jh7100/syscon_sysmain_ctrl_macro.h
>
> Christian, I'm happy that you're working on this, but mess like this
> and waiting for the non-coherent dma to be sorted is why I didn't send
> it upstream yet.

Thank you for clarifying this and for all the work you have done so far,
Emil! If you don't mind, I would be glad to continue helping with this
mainlining effort.

>>>> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
>>>> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
>>>> + if (ret)
>>>> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
>>>> + }
>>>
>>> You should probably document that if starfive,gtxclk-dlychain is not
>>> found in the DT blob, the value for the delay chain is undefined. It
>>> would actually be better to define it, set it to 0 for example. That
>>> way, you know you don't have any dependency on the bootloader for
>>> example.
>>
>> Sure, I will set it to 0.
>>
>>>
>>> Andrew
>>
>> Thanks for reviewing,
>> Cristian
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-15 12:52:27

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

On Wed, Feb 15, 2023 at 02:08:01AM +0200, Cristian Ciocaltea wrote:
> On 2/11/23 18:11, Andrew Lunn wrote:
> > > +
> > > +#define JH7100_SYSMAIN_REGISTER28 0x70
> > > +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
> > > +#define JH7100_SYSMAIN_REGISTER49 0xc8
> >
> > Seems like the comment should be one line earlier?
> >
> > There is value in basing the names on the datasheet, but you could
> > append something meaningful on the end:
> >
> > #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8
> >
> > ???
>
> Unfortunately the JH7100 datasheet I have access to doesn't provide any
> information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil
> could provide some details here?

If you have no reliable source of naming, just make a name up from how
the register is used. This is why i suggested adding _DLYCHAIN,
because that is what is written to it. You should be able to do the
same with register 28.

Andrew

2023-02-15 13:02:24

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On Wed, Feb 15, 2023 at 02:34:23AM +0200, Cristian Ciocaltea wrote:
> On 2/11/23 18:01, Andrew Lunn wrote:
> > > + starfive,gtxclk-dlychain:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + description: GTX clock delay chain setting
> >
> > Please could you add more details to this. Is this controlling the
> > RGMII delays? 0ns or 2ns?
>
> This is what gets written to JH7100_SYSMAIN_REGISTER49 and it's currently
> set to 4 in patch 12/12. As already mentioned, I don't have the register
> information in the datasheet, but I'll update this as soon as we get some
> details.

I have seen what happens to this value, but i have no idea what it
actually means. And without knowing what it means, i cannot say if it
is being used correctly or not. And it could be related to the next
part of my comment...

>
> > > + gmac: ethernet@10020000 {
> > > + compatible = "starfive,jh7100-dwmac", "snps,dwmac";
> > > + reg = <0x0 0x10020000 0x0 0x10000>;
> > > + clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
> > > + <&clkgen JH7100_CLK_GMAC_AHB>,
> > > + <&clkgen JH7100_CLK_GMAC_PTP_REF>,
> > > + <&clkgen JH7100_CLK_GMAC_GTX>,
> > > + <&clkgen JH7100_CLK_GMAC_TX_INV>;
> > > + clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
> > > + resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
> > > + reset-names = "ahb";
> > > + interrupts = <6>, <7>;
> > > + interrupt-names = "macirq", "eth_wake_irq";
> > > + max-frame-size = <9000>;
> > > + phy-mode = "rgmii-txid";
> >
> > This is unusual. Does your board have a really long RX clock line to
> > insert the 2ns delay needed on the RX side?
>
> Just tested with "rgmii" and didn't notice any issues. If I'm not missing
> anything, I'll do the change in the next revision.

rgmii-id is generally the value to be used. That indicates the board
needs 2ns delays adding by something, either the MAC or the PHY. And
then i always recommend the MAC driver does nothing, pass the value to
the PHY and let the PHY add the delays.

So try both rgmii and rgmii-id and do a lot of bi directional
transfers. Then look at the reported ethernet frame check sum error
counts, both local and the link peer. I would expect one setting gives
you lots of errors, and the other works much better.

Andrew

2023-02-15 13:11:48

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

On Tue, 14 Feb 2023 at 21:42, Conor Dooley <[email protected]> wrote:
>
> Hey all,
>
> On Sat, Feb 11, 2023 at 05:18:10AM +0200, Cristian Ciocaltea wrote:
> > Document the compatible for the SiFive Composable Cache Controller found
> > on the StarFive JH7100 SoC.
> >
> > This also requires extending the 'reg' property to handle distinct
> > ranges, as specified via 'reg-names'.
> >
> > Signed-off-by: Cristian Ciocaltea <[email protected]>
> > ---
> > .../bindings/riscv/sifive,ccache0.yaml | 28 ++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> > index 31d20efaa6d3..2b864b2f12c9 100644
> > --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> > @@ -25,6 +25,7 @@ select:
> > - sifive,ccache0
> > - sifive,fu540-c000-ccache
> > - sifive,fu740-c000-ccache
> > + - starfive,jh7100-ccache
> >
> > required:
> > - compatible
> > @@ -37,6 +38,7 @@ properties:
> > - sifive,ccache0
> > - sifive,fu540-c000-ccache
> > - sifive,fu740-c000-ccache
> > + - starfive,jh7100-ccache
> > - const: cache
> > - items:
> > - const: starfive,jh7110-ccache
> > @@ -70,7 +72,13 @@ properties:
> > - description: DirFail interrupt
> >
> > reg:
> > - maxItems: 1
> > + minItems: 1
> > + maxItems: 2
> > +
> > + reg-names:
> > + items:
> > + - const: control
> > + - const: sideband
>
> So why is this called "sideband"?
> In the docs for the JH7100 it is called LIM & it's called LIM in our
> docs for the PolarFire SoC (at the same address btw) and we run the HSS
> out of it! LIM being "loosely integrated memory", which by the limit
> hits on Google may be a SiFive-ism?
>
> I'm not really sure if adding it as a "reg" section is the right thing
> to do as it's not "just" a register bank.
> Perhaps Rob/Krzysztof have a take on that one?

Yes, this seems to be a leftover I didn't manage to clean up yet. The
"sideband" range is called L2 LIM in the datasheet and seems to be a
way to use the cache directly. The Sifive docs read "When cache ways
are disabled, they are addressable in the L2 Loosely-Integrated Memory
(L2 LIM) address space [..]". This feature is not used by Linux on the
JH7100, so can just be removed here.

/Emil

> >
> > next-level-cache: true
> >
> > @@ -89,6 +97,7 @@ allOf:
> > contains:
> > enum:
> > - sifive,fu740-c000-ccache
> > + - starfive,jh7100-ccache
> > - starfive,jh7110-ccache
> > - microchip,mpfs-ccache
> >
> > @@ -106,12 +115,29 @@ allOf:
> > Must contain entries for DirError, DataError and DataFail signals.
> > maxItems: 3
> >
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7100-ccache
> > +
> > + then:
> > + properties:
> > + reg:
> > + maxItems: 2
> > +
> > + else:
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > - if:
> > properties:
> > compatible:
> > contains:
> > enum:
> > - sifive,fu740-c000-ccache
> > + - starfive,jh7100-ccache
> > - starfive,jh7110-ccache
> >
> > then:
> > --
> > 2.39.1
> >
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-16 15:51:48

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 2/15/23 15:01, Andrew Lunn wrote:
> On Wed, Feb 15, 2023 at 02:34:23AM +0200, Cristian Ciocaltea wrote:
>> On 2/11/23 18:01, Andrew Lunn wrote:
>>>> + starfive,gtxclk-dlychain:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + description: GTX clock delay chain setting
>>>
>>> Please could you add more details to this. Is this controlling the
>>> RGMII delays? 0ns or 2ns?
>>
>> This is what gets written to JH7100_SYSMAIN_REGISTER49 and it's currently
>> set to 4 in patch 12/12. As already mentioned, I don't have the register
>> information in the datasheet, but I'll update this as soon as we get some
>> details.
>
> I have seen what happens to this value, but i have no idea what it
> actually means. And without knowing what it means, i cannot say if it
> is being used correctly or not. And it could be related to the next
> part of my comment...
>
>>
>>>> + gmac: ethernet@10020000 {
>>>> + compatible = "starfive,jh7100-dwmac", "snps,dwmac";
>>>> + reg = <0x0 0x10020000 0x0 0x10000>;
>>>> + clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
>>>> + <&clkgen JH7100_CLK_GMAC_AHB>,
>>>> + <&clkgen JH7100_CLK_GMAC_PTP_REF>,
>>>> + <&clkgen JH7100_CLK_GMAC_GTX>,
>>>> + <&clkgen JH7100_CLK_GMAC_TX_INV>;
>>>> + clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
>>>> + resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
>>>> + reset-names = "ahb";
>>>> + interrupts = <6>, <7>;
>>>> + interrupt-names = "macirq", "eth_wake_irq";
>>>> + max-frame-size = <9000>;
>>>> + phy-mode = "rgmii-txid";
>>>
>>> This is unusual. Does your board have a really long RX clock line to
>>> insert the 2ns delay needed on the RX side?
>>
>> Just tested with "rgmii" and didn't notice any issues. If I'm not missing
>> anything, I'll do the change in the next revision.
>
> rgmii-id is generally the value to be used. That indicates the board
> needs 2ns delays adding by something, either the MAC or the PHY. And
> then i always recommend the MAC driver does nothing, pass the value to
> the PHY and let the PHY add the delays.
>
> So try both rgmii and rgmii-id and do a lot of bi directional
> transfers. Then look at the reported ethernet frame check sum error
> counts, both local and the link peer. I would expect one setting gives
> you lots of errors, and the other works much better.

I gave "rgmii-id" a try and it's not usable, I get too many errors. So
"rgmii" should be the right choice here.

Thanks,
Cristian

> Andrew
>

2023-02-16 17:55:21

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

> I gave "rgmii-id" a try and it's not usable, I get too many errors. So
> "rgmii" should be the right choice here.

I would actually say it shows we don't understand what is going on
with delays. "rgmii" is not every often the correct value. The fact it
works suggests the MAC is adding delays.

What value are you using for starfive,gtxclk-dlychain ? Try 0 and then
"rgmii-id"

Andrew


2023-02-16 18:50:36

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

Emil,

+CC Daire

On Sat, Feb 11, 2023 at 05:18:13AM +0200, Cristian Ciocaltea wrote:
> From: Emil Renner Berthing <[email protected]>
>
> Add functions to flush the caches and handle non-coherent DMA.
>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> [replace <asm/cacheflush.h> with <linux/cacheflush.h>]
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---

> +void *sifive_ccache_set_uncached(void *addr, size_t size)
> +{
> + phys_addr_t phys_addr = __pa(addr) + uncached_offset;
> + void *mem_base;
> +
> + mem_base = memremap(phys_addr, size, MEMREMAP_WT);
> + if (!mem_base) {
> + pr_err("%s memremap failed for addr %p\n", __func__, addr);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return mem_base;
> +}

The rest of this I either get b/c we did it, or will become moot so I
amn't worried about it, but can you please explain this, in particular
the memremap that you're doing here?

Cheers,
Conor.


Attachments:
(No filename) (946.00 B)
signature.asc (228.00 B)
Download all attachments

2023-02-16 21:54:49

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

Hey all,

On Sat, Feb 11, 2023 at 05:18:11AM +0200, Cristian Ciocaltea wrote:
> Add the 'uncached-offset' property to be used for specifying the
> uncached memory offset required for handling non-coherent DMA
> transactions.
>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> index 2b864b2f12c9..60cd87a2810a 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> @@ -82,6 +82,11 @@ properties:
>
> next-level-cache: true
>
> + uncached-offset:
> + $ref: /schemas/types.yaml#/definitions/uint64
> + description: |
> + Uncached memory offset for handling non-coherent DMA transactions.

Firstly, this pretty tied to the StarFive stuff, where there is only one
"bank" of memory that neatly maps to one bank of non-cached memory.
On PolarFire SoC, where we would also like to make use of non-coherent
DMA for some transfers using the FPGA fabric, things are a bit more
complex.
Instead of a region & a non-cached alias, we have 2 regions and 2
non-cached regions.
These regions lie at 0x8000_0000 & 0x10_0000_0000 and the non-cached
regions are at 0xc000_0000 & 0x14_0000_0000. As you can tell, one fixed
offset isn't going to work there!

The other bit of a problem is that there is no fixed concept of aliases,
as seems to be the case on the jh7100. Instead, where the regions
"point" to in physical DDR is something that is configurable at runtime.
Practically speaking, it is set by firmware very early on in boot & is
fixed from there out, but will vary between boards and FPGA fabric
configuration. Effectively that means that from the PoV of a Devicetree
it is constant, but a good bit of flexibility is going to be needed.

What we have been doing on PolarFire SoC (although mostly internally at
this point) is, rather than creating a property like uncached-offset, we
instead are using the dma-ranges properties to induce the same affect.

In an example configuration with memory at:
reg = <0x0 0x80000000 0x0 0x4000000>;
reg = <0x0 0x8a000000 0x0 0x8000000>;
reg = <0x0 0xc4000000 0x0 0x6000000>;
reg = <0x10 0x22000000 0x0 0x5e000000>;
reg = <0x14 0x12000000 0x0 0x10000000>;

a reserved memory section then covering the non-cached region at
0x14_0000_0000:
dma_non_cached_high: non-cached-high-buffer {
compatible = "shared-dma-pool";
size = <0x0 0x10000000>;
no-map;
linux,dma-default;
alloc-ranges = <0x14 0x12000000 0x0 0x10000000>;
};

and dma-ranges:
dma-ranges = <0x14 0x0 0x0 0x80000000 0x0 0x4000000>,
<0x14 0x4000000 0x0 0xc4000000 0x0 0x6000000>,
<0x14 0xa000000 0x0 0x8a000000 0x0 0x8000000>,

In this configuration, 0x8000_0000, 0x10_0000_0000, 0xc000_0000 &
0x14_0000_0000 are all aliases of the same address.
With this setup, we're able to do non-coherent DMA to the FPGA fabric,
to the PCI for example.
The DTS does grow a bit of complexity, with reserved memory regions and
dma-ranges - but at least they're standard properties!

Emil, if you want to take a look at that it is here:
https://github.com/linux4microchip/linux linux-5.15-mchp
I think I said to you before that it was based on one of Atish's early
approaches, the one from the 5.15 development cycle IIRC since we're
using that LTS.
Obviously it'll need changes to be upstreamable so we're not wedded to
this approach. For instance, it's being controlled by a compile time
option at the moment, so that clearly needs to become runtime for
upstream (and realistically needs to be one in our vendor tree too...)

I'll try to hack that approach into the visionfive v1 soonTM and see how
it goes, but it'll not be this side of March before I have time to do
that.

Cheers,
Conor.


Attachments:
(No filename) (3.89 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-17 00:33:00

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 2/16/23 19:54, Andrew Lunn wrote:
>> I gave "rgmii-id" a try and it's not usable, I get too many errors. So
>> "rgmii" should be the right choice here.
>
> I would actually say it shows we don't understand what is going on
> with delays. "rgmii" is not every often the correct value. The fact it
> works suggests the MAC is adding delays.
>
> What value are you using for starfive,gtxclk-dlychain ?

This is set to '4' in patch 12/12.

> Try 0 and then "rgmii-id"

I made some more tests and it seems the only stable configuration is
"rgmii" with "starfive,gtxclk-dlychain" set to 4:

phy-mode | dlychain | status
---------+----------+--------------------------------------------
rgmii | 4 | OK (no issues observed)
rgmii-id | 4 | BROKEN (errors reported [1])
rgmii | 0 | UNRELIABLE (no errors, but frequent stalls)
rgmii-id | 0 | BROKEN (errors reported)

[1] Reported errors in case of BROKEN status:
$ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'

/sys/class/net/eth0/statistics/rx_crc_errors:6
/sys/class/net/eth0/statistics/rx_errors:6
/sys/class/net/eth0/statistics/tx_bytes:10836
/sys/class/net/eth0/statistics/tx_packets:46

> Andrew
>

2023-02-17 13:58:46

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

> > I would actually say it shows we don't understand what is going on
> > with delays. "rgmii" is not every often the correct value. The fact it
> > works suggests the MAC is adding delays.
> >
> > What value are you using for starfive,gtxclk-dlychain ?
>
> This is set to '4' in patch 12/12.
>
> > Try 0 and then "rgmii-id"
>
> I made some more tests and it seems the only stable configuration is "rgmii"
> with "starfive,gtxclk-dlychain" set to 4:
>
> phy-mode | dlychain | status
> ---------+----------+--------------------------------------------
> rgmii | 4 | OK (no issues observed)
> rgmii-id | 4 | BROKEN (errors reported [1])
> rgmii | 0 | UNRELIABLE (no errors, but frequent stalls)
> rgmii-id | 0 | BROKEN (errors reported)
>
> [1] Reported errors in case of BROKEN status:
> $ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'

Thanks for the testing.

So it seems like something is adding delays when it probably should
not. Ideally we want to know what.

There is a danger here, something which has happened in the past. A
PHY which ignored "rgmii" and actually did power on defaults which was
"rgmii-id". As a result, lots of boards put "rmgii" in there DT blob,
which 'worked'. Until a board came along which really did need
"rgmii". The developer bringing that board up debugged the PHY, found
the problem and made it respect "rgmii" so their board worked. And the
fix broke a number of 'working' boards which had the wrong "rgmii"
instead of "rgmii-id".

So you have a choice. Go with 4 and "rgmii", but put in a big fat
warning, "Works somehow but is technically wrong and will probably
break sometime in the future". Or try to understand what is really
going on here, were are the delays coming from, and fix the issue.

Andrew

2023-02-17 15:25:24

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 2/17/23 15:30, Andrew Lunn wrote:
>>> I would actually say it shows we don't understand what is going on
>>> with delays. "rgmii" is not every often the correct value. The fact it
>>> works suggests the MAC is adding delays.
>>>
>>> What value are you using for starfive,gtxclk-dlychain ?
>>
>> This is set to '4' in patch 12/12.
>>
>>> Try 0 and then "rgmii-id"
>>
>> I made some more tests and it seems the only stable configuration is "rgmii"
>> with "starfive,gtxclk-dlychain" set to 4:
>>
>> phy-mode | dlychain | status
>> ---------+----------+--------------------------------------------
>> rgmii | 4 | OK (no issues observed)
>> rgmii-id | 4 | BROKEN (errors reported [1])
>> rgmii | 0 | UNRELIABLE (no errors, but frequent stalls)
>> rgmii-id | 0 | BROKEN (errors reported)
>>
>> [1] Reported errors in case of BROKEN status:
>> $ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'
>
> Thanks for the testing.
>
> So it seems like something is adding delays when it probably should
> not. Ideally we want to know what.
>
> There is a danger here, something which has happened in the past. A
> PHY which ignored "rgmii" and actually did power on defaults which was
> "rgmii-id". As a result, lots of boards put "rmgii" in there DT blob,
> which 'worked'. Until a board came along which really did need
> "rgmii". The developer bringing that board up debugged the PHY, found
> the problem and made it respect "rgmii" so their board worked. And the
> fix broke a number of 'working' boards which had the wrong "rgmii"
> instead of "rgmii-id".

Thanks for the heads-up.

> So you have a choice. Go with 4 and "rgmii", but put in a big fat
> warning, "Works somehow but is technically wrong and will probably
> break sometime in the future". Or try to understand what is really
> going on here, were are the delays coming from, and fix the issue.
>
> Andrew

I will try to analyze this further.

Regards,
Cristian

2023-02-19 21:33:38

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

On Thu, 16 Feb 2023 at 19:51, Conor Dooley <[email protected]> wrote:
>
> Emil,
>
> +CC Daire
>
> On Sat, Feb 11, 2023 at 05:18:13AM +0200, Cristian Ciocaltea wrote:
> > From: Emil Renner Berthing <[email protected]>
> >
> > Add functions to flush the caches and handle non-coherent DMA.
> >
> > Signed-off-by: Emil Renner Berthing <[email protected]>
> > [replace <asm/cacheflush.h> with <linux/cacheflush.h>]
> > Signed-off-by: Cristian Ciocaltea <[email protected]>
> > ---
>
> > +void *sifive_ccache_set_uncached(void *addr, size_t size)
> > +{
> > + phys_addr_t phys_addr = __pa(addr) + uncached_offset;
> > + void *mem_base;
> > +
> > + mem_base = memremap(phys_addr, size, MEMREMAP_WT);
> > + if (!mem_base) {
> > + pr_err("%s memremap failed for addr %p\n", __func__, addr);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + return mem_base;
> > +}
>
> The rest of this I either get b/c we did it, or will become moot so I
> amn't worried about it, but can you please explain this, in particular
> the memremap that you're doing here?

No, I can't really. As we talked about it's also based on a prototype
by Atish. I'm sure you know that the general idea is that we want to
return a pointer that accesses the same physical memory, but through
the uncached alias. I can't tell you exactly why it's done this way
though, sorry.

/Emil

> Cheers,
> Conor.
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-20 11:44:34

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

On Sun, Feb 19, 2023 at 10:32:52PM +0100, Emil Renner Berthing wrote:
> On Thu, 16 Feb 2023 at 19:51, Conor Dooley <[email protected]> wrote:
> >
> > Emil,
> >
> > +CC Daire
> >
> > On Sat, Feb 11, 2023 at 05:18:13AM +0200, Cristian Ciocaltea wrote:
> > > From: Emil Renner Berthing <[email protected]>
> > >
> > > Add functions to flush the caches and handle non-coherent DMA.
> > >
> > > Signed-off-by: Emil Renner Berthing <[email protected]>
> > > [replace <asm/cacheflush.h> with <linux/cacheflush.h>]
> > > Signed-off-by: Cristian Ciocaltea <[email protected]>
> > > ---
> >
> > > +void *sifive_ccache_set_uncached(void *addr, size_t size)
> > > +{
> > > + phys_addr_t phys_addr = __pa(addr) + uncached_offset;
> > > + void *mem_base;
> > > +
> > > + mem_base = memremap(phys_addr, size, MEMREMAP_WT);
> > > + if (!mem_base) {
> > > + pr_err("%s memremap failed for addr %p\n", __func__, addr);
> > > + return ERR_PTR(-EINVAL);
> > > + }
> > > +
> > > + return mem_base;
> > > +}
> >
> > The rest of this I either get b/c we did it, or will become moot so I
> > amn't worried about it, but can you please explain this, in particular
> > the memremap that you're doing here?
>
> No, I can't really. As we talked about it's also based on a prototype
> by Atish. I'm sure you know that the general idea is that we want to
> return a pointer that accesses the same physical memory, but through
> the uncached alias.

Yah, I follow all the rest of what's going on - it's just this bit of it
that I don't.

> I can't tell you exactly why it's done this way
> though, sorry.

I had a bit of a look on lore, but don't really see anything there that
contained any discussion of what was going on here.

Adding Atish in the off-chance that he remembers!


Attachments:
(No filename) (1.77 kB)
signature.asc (228.00 B)
Download all attachments

2023-03-03 11:52:50

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 06/12] dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible

On Sat, 11 Feb 2023, Cristian Ciocaltea wrote:

> From: Emil Renner Berthing <[email protected]>
>
> Document StarFive JH7100 SoC compatible for sysmain registers.
>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Cristian Ciocaltea <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
> 1 file changed, 1 insertion(+)

Applied, thanks

--
Lee Jones [李琼斯]

2023-03-06 23:32:47

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 03/12] soc: sifive: ccache: Add StarFive JH7100 support

On Sat, Feb 11, 2023 at 05:18:12AM +0200, Cristian Ciocaltea wrote:
> From: Emil Renner Berthing <[email protected]>
>
> This adds support for the StarFive JH7100 SoC which also feature this
> SiFive cache controller.
>
> Unfortunately the interrupt for uncorrected data is broken on the JH7100
> and fires continuously, so add a quirk to not register a handler for it.
>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> [drop JH7110, rework Kconfig]
> Signed-off-by: Cristian Ciocaltea <[email protected]>

This driver doesn't really do very much of anything as things stand, so
I don't see really see all that much value in picking it up right now,
since the non-coherent bits aren't usable yet.

> ---
> drivers/soc/sifive/Kconfig | 1 +
> drivers/soc/sifive/sifive_ccache.c | 11 ++++++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
> index e86870be34c9..867cf16273a4 100644
> --- a/drivers/soc/sifive/Kconfig
> +++ b/drivers/soc/sifive/Kconfig
> @@ -4,6 +4,7 @@ if SOC_SIFIVE || SOC_STARFIVE
>
> config SIFIVE_CCACHE
> bool "Sifive Composable Cache controller"
> + default SOC_STARFIVE

I don't think this should have a default set w/ the support that this
patch brings in. Perhaps later we should be doing defaulting, but not at
this point in the series.
Other than that, this is fine by me:
Reviewed-by: Conor Dooley <[email protected]>

Thanks,
Conor.


Attachments:
(No filename) (1.46 kB)
signature.asc (228.00 B)
Download all attachments

2023-03-06 23:46:24

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 03/12] soc: sifive: ccache: Add StarFive JH7100 support

On 3/7/23 01:32, Conor Dooley wrote:
> On Sat, Feb 11, 2023 at 05:18:12AM +0200, Cristian Ciocaltea wrote:
>> From: Emil Renner Berthing <[email protected]>
>>
>> This adds support for the StarFive JH7100 SoC which also feature this
>> SiFive cache controller.
>>
>> Unfortunately the interrupt for uncorrected data is broken on the JH7100
>> and fires continuously, so add a quirk to not register a handler for it.
>>
>> Signed-off-by: Emil Renner Berthing <[email protected]>
>> [drop JH7110, rework Kconfig]
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>
> This driver doesn't really do very much of anything as things stand, so
> I don't see really see all that much value in picking it up right now,
> since the non-coherent bits aren't usable yet.
>
>> ---
>> drivers/soc/sifive/Kconfig | 1 +
>> drivers/soc/sifive/sifive_ccache.c | 11 ++++++++++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig
>> index e86870be34c9..867cf16273a4 100644
>> --- a/drivers/soc/sifive/Kconfig
>> +++ b/drivers/soc/sifive/Kconfig
>> @@ -4,6 +4,7 @@ if SOC_SIFIVE || SOC_STARFIVE
>>
>> config SIFIVE_CCACHE
>> bool "Sifive Composable Cache controller"
>> + default SOC_STARFIVE
>
> I don't think this should have a default set w/ the support that this
> patch brings in. Perhaps later we should be doing defaulting, but not at
> this point in the series.

I will handle this is v2 as soon as the non-coherency stuff is ready.

> Other than that, this is fine by me:
> Reviewed-by: Conor Dooley <[email protected]>

Thanks for reviewing,
Cristian

> Thanks,
> Conor.

2023-03-20 23:48:13

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

On Tue, 14 Feb 2023 12:40:35 PST (-0800), Conor Dooley wrote:
> Hey all,
>
> On Sat, Feb 11, 2023 at 05:18:10AM +0200, Cristian Ciocaltea wrote:
>> Document the compatible for the SiFive Composable Cache Controller found
>> on the StarFive JH7100 SoC.
>>
>> This also requires extending the 'reg' property to handle distinct
>> ranges, as specified via 'reg-names'.
>>
>> Signed-off-by: Cristian Ciocaltea <[email protected]>
>> ---
>> .../bindings/riscv/sifive,ccache0.yaml | 28 ++++++++++++++++++-
>> 1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
>> index 31d20efaa6d3..2b864b2f12c9 100644
>> --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
>> @@ -25,6 +25,7 @@ select:
>> - sifive,ccache0
>> - sifive,fu540-c000-ccache
>> - sifive,fu740-c000-ccache
>> + - starfive,jh7100-ccache
>>
>> required:
>> - compatible
>> @@ -37,6 +38,7 @@ properties:
>> - sifive,ccache0
>> - sifive,fu540-c000-ccache
>> - sifive,fu740-c000-ccache
>> + - starfive,jh7100-ccache
>> - const: cache
>> - items:
>> - const: starfive,jh7110-ccache
>> @@ -70,7 +72,13 @@ properties:
>> - description: DirFail interrupt
>>
>> reg:
>> - maxItems: 1
>> + minItems: 1
>> + maxItems: 2
>> +
>> + reg-names:
>> + items:
>> + - const: control
>> + - const: sideband
>
> So why is this called "sideband"?
> In the docs for the JH7100 it is called LIM & it's called LIM in our
> docs for the PolarFire SoC (at the same address btw) and we run the HSS

IIRC it's both: "LIM" is the memory, "sideband" is the port. I can't
find any proper documentation of "sideband" outside of DT and errata,
but there's a hanful of references to it in the bootloader for the
fu540:
<https://github.com/sifive/freedom-u540-c000-bootloader/search?q=sideband>.

It's not really clear which is more correct here: sideband accesses are
only useful when the cache is configured as an LIM, at least for general
software. IIRC the accesses to the LIM only go through the sideband
port for the E core, but I might be wrong about that.

> out of it! LIM being "loosely integrated memory", which by the limit
> hits on Google may be a SiFive-ism?

Yep: TIM is the SiFive version of Arm's TCM (tightly coupled memory),
and LIM is the flavor that's farther away (L2 instead of L1).

> I'm not really sure if adding it as a "reg" section is the right thing
> to do as it's not "just" a register bank.
> Perhaps Rob/Krzysztof have a take on that one?
>
>>
>> next-level-cache: true
>>
>> @@ -89,6 +97,7 @@ allOf:
>> contains:
>> enum:
>> - sifive,fu740-c000-ccache
>> + - starfive,jh7100-ccache
>> - starfive,jh7110-ccache
>> - microchip,mpfs-ccache
>>
>> @@ -106,12 +115,29 @@ allOf:
>> Must contain entries for DirError, DataError and DataFail signals.
>> maxItems: 3
>>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: starfive,jh7100-ccache
>> +
>> + then:
>> + properties:
>> + reg:
>> + maxItems: 2
>> +
>> + else:
>> + properties:
>> + reg:
>> + maxItems: 1
>> +
>> - if:
>> properties:
>> compatible:
>> contains:
>> enum:
>> - sifive,fu740-c000-ccache
>> + - starfive,jh7100-ccache
>> - starfive,jh7110-ccache
>>
>> then:
>> --
>> 2.39.1
>>

2023-10-27 14:56:56

by Cristian Ciocaltea

[permalink] [raw]
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 2/17/23 17:25, Cristian Ciocaltea wrote:
> On 2/17/23 15:30, Andrew Lunn wrote:
>>>> I would actually say it shows we don't understand what is going on
>>>> with delays. "rgmii" is not every often the correct value. The fact it
>>>> works suggests the MAC is adding delays.
>>>>
>>>> What value are you using for starfive,gtxclk-dlychain ?
>>>
>>> This is set to '4' in patch 12/12.
>>>
>>>> Try 0 and then "rgmii-id"
>>>
>>> I made some more tests and it seems the only stable configuration is
>>> "rgmii"
>>> with "starfive,gtxclk-dlychain" set to 4:
>>>
>>> phy-mode | dlychain | status
>>> ---------+----------+--------------------------------------------
>>> rgmii    |        4 | OK (no issues observed)
>>> rgmii-id |        4 | BROKEN (errors reported [1])
>>> rgmii    |        0 | UNRELIABLE (no errors, but frequent stalls)
>>> rgmii-id |        0 | BROKEN (errors reported)
>>>
>>> [1] Reported errors in case of BROKEN status:
>>> $ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'
>>
>> Thanks for the testing.
>>
>> So it seems like something is adding delays when it probably should
>> not. Ideally we want to know what.
>>
>> There is a danger here, something which has happened in the past. A
>> PHY which ignored "rgmii" and actually did power on defaults which was
>> "rgmii-id". As a result, lots of boards put "rmgii" in there DT blob,
>> which 'worked'. Until a board came along which really did need
>> "rgmii". The developer bringing that board up debugged the PHY, found
>> the problem and made it respect "rgmii" so their board worked. And the
>> fix broke a number of 'working' boards which had the wrong "rgmii"
>> instead of "rgmii-id".
>
> Thanks for the heads-up.
>
>> So you have a choice. Go with 4 and "rgmii", but put in a big fat
>> warning, "Works somehow but is technically wrong and will probably
>> break sometime in the future". Or try to understand what is really
>> going on here, were are the delays coming from, and fix the issue.
>>
>>        Andrew
>
> I will try to analyze this further.

As the non-coherent DMA work this series depended on has been completed,
I started to investigate further the "rgmii-id" issue.
I couldn't spot anything wrong in the Motorcomm PHY driver, but
eventually got this working by adjusting rx-internal-delay-ps.

Will do some more testing before submitting v2.

Thanks,
Cristian