The 04/06/2021 09:38, Patrick Delaunay wrote:

Hi Patrick,

I have applied your patches and I have done a basic test on jr2_switch. It seems to work fine. I got some warnings from the function '__of_translate_address' which I need to figure out.

But one more important thing is that also luton_switch should be updated.

Today of_address_to_resource() is called only in ofnode_read_resource() for livetree support and fdt_get_resource() is called when livetree is not supported.

The fdt_get_resource() doesn't do the address translation so when it is required, but the address translation is done by ofnode_read_resource() caller, for example in drivers/firmware/scmi/smt.c::scmi_dt_get_smt_buffer() { ... ret = ofnode_read_resource(args.node, 0, &resource); if (ret) return ret;

    faddr = cpu_to_fdt32(resource.start);
    paddr = ofnode_translate_address(args.node, &faddr);

...

The both behavior should be aligned and the address translation must be called in fdt_get_resource() and removed for each caller.

Fixes: a44810123f9e ("dm: core: Add dev_read_resource() to read device resources") Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com

---

This patch allows to remove the workaround in smci/smt.c introduced by [1].

But it impact with all user of

• ofnode_read_resource
• ofnode_read_resource_byname
• dev_read_resource
• dev_read_resource_byname

After my first check, the only impacts are in drivers/net/mscc_eswitch => I remove the unnecessary translate after code review, this patch need to be verify on real hardware

I proposed to merge the workaround [1] as soon as possible to avoid issue on stm32mp1 platform and this patch can be merged when it will be acked by mscc_eswitch maintainers and other API users.

[1] "scmi: translate the resource only when livetree is not activated" http://patchwork.ozlabs.org/project/uboot/list/?series=236526&state=*

drivers/firmware/scmi/smt.c | 12 +----------- drivers/net/mscc_eswitch/jr2_switch.c | 4 +--- drivers/net/mscc_eswitch/ocelot_switch.c | 4 +--- drivers/net/mscc_eswitch/serval_switch.c | 4 +--- drivers/net/mscc_eswitch/servalt_switch.c | 4 +--- lib/fdtdec.c | 6 +++++- 6 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c index f1915c0074..e60c2aebc8 100644 --- a/drivers/firmware/scmi/smt.c +++ b/drivers/firmware/scmi/smt.c @@ -30,8 +30,6 @@ int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt) int ret; struct ofnode_phandle_args args; struct resource resource;

•   fdt32_t faddr;
  

•   phys_addr_t paddr;
  
    ret = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
    if (ret)
  

@@ -41,21 +39,13 @@ int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt) if (ret) return ret;

•   /* TEMP workaround for ofnode_read_resource translation issue */
  

•   if (of_live_active()) {
  

•           paddr = resource.start;
  

•   } else {
  

•           faddr = cpu_to_fdt32(resource.start);
  

•           paddr = ofnode_translate_address(args.node, &faddr);
  

•   }
  

•   smt->size = resource_size(&resource);
    if (smt->size < sizeof(struct scmi_smt_header)) {
            dev_err(dev, "Shared memory buffer too small\n");
            return -EINVAL;
    }
  
  

•   smt->buf = devm_ioremap(dev, paddr, smt->size);
  

•   smt->buf = devm_ioremap(dev, resource.start, smt->size);
    if (!smt->buf)
            return -ENOMEM;
  
  

diff --git a/drivers/net/mscc_eswitch/jr2_switch.c b/drivers/net/mscc_eswitch/jr2_switch.c index 128d7f21ce..bf3e9b56ec 100644 --- a/drivers/net/mscc_eswitch/jr2_switch.c +++ b/drivers/net/mscc_eswitch/jr2_switch.c @@ -842,7 +842,6 @@ static int jr2_probe(struct udevice *dev) int i; int ret; struct resource res;

•   fdt32_t faddr;
    phys_addr_t addr_base;
    unsigned long addr_size;
    ofnode eth_node, node, mdio_node;
  

@@ -893,9 +892,8 @@ static int jr2_probe(struct udevice *dev)

            if (ofnode_read_resource(mdio_node, 0, &res))
                    return -ENOMEM;

•           faddr = cpu_to_fdt32(res.start);
  
  

•           addr_base = ofnode_translate_address(mdio_node, &faddr);
  

•           addr_base = res.start;
            addr_size = res.end - res.start;
  
            /* If the bus is new then create a new bus */
  

diff --git a/drivers/net/mscc_eswitch/ocelot_switch.c b/drivers/net/mscc_eswitch/ocelot_switch.c index 19e725c6f9..d1d0a489ab 100644 --- a/drivers/net/mscc_eswitch/ocelot_switch.c +++ b/drivers/net/mscc_eswitch/ocelot_switch.c @@ -530,7 +530,6 @@ static int ocelot_probe(struct udevice *dev) struct ocelot_private *priv = dev_get_priv(dev); int i, ret; struct resource res;

•   fdt32_t faddr;
    phys_addr_t addr_base;
    unsigned long addr_size;
    ofnode eth_node, node, mdio_node;
  

@@ -578,9 +577,8 @@ static int ocelot_probe(struct udevice *dev)

            if (ofnode_read_resource(mdio_node, 0, &res))
                    return -ENOMEM;

•           faddr = cpu_to_fdt32(res.start);
  
  

•           addr_base = ofnode_translate_address(mdio_node, &faddr);
  

•           addr_base = res.start;
            addr_size = res.end - res.start;
  
            /* If the bus is new then create a new bus */
  

diff --git a/drivers/net/mscc_eswitch/serval_switch.c b/drivers/net/mscc_eswitch/serval_switch.c index 09ce33452d..c4b81f7529 100644 --- a/drivers/net/mscc_eswitch/serval_switch.c +++ b/drivers/net/mscc_eswitch/serval_switch.c @@ -482,7 +482,6 @@ static int serval_probe(struct udevice *dev) struct serval_private *priv = dev_get_priv(dev); int i, ret; struct resource res;

•   fdt32_t faddr;
    phys_addr_t addr_base;
    unsigned long addr_size;
    ofnode eth_node, node, mdio_node;
  

@@ -533,9 +532,8 @@ static int serval_probe(struct udevice *dev)

            if (ofnode_read_resource(mdio_node, 0, &res))
                    return -ENOMEM;

•           faddr = cpu_to_fdt32(res.start);
  
  

•           addr_base = ofnode_translate_address(mdio_node, &faddr);
  

•           addr_base = res.start;
            addr_size = res.end - res.start;
  
            /* If the bus is new then create a new bus */
  

diff --git a/drivers/net/mscc_eswitch/servalt_switch.c b/drivers/net/mscc_eswitch/servalt_switch.c index 4a4e9e4054..f114086ece 100644 --- a/drivers/net/mscc_eswitch/servalt_switch.c +++ b/drivers/net/mscc_eswitch/servalt_switch.c @@ -412,7 +412,6 @@ static int servalt_probe(struct udevice *dev) struct servalt_private *priv = dev_get_priv(dev); int i; struct resource res;

•   fdt32_t faddr;
    phys_addr_t addr_base;
    unsigned long addr_size;
    ofnode eth_node, node, mdio_node;
  

@@ -461,9 +460,8 @@ static int servalt_probe(struct udevice *dev)

            if (ofnode_read_resource(mdio_node, 0, &res))
                    return -ENOMEM;

•           faddr = cpu_to_fdt32(res.start);
  
  

•           addr_base = ofnode_translate_address(mdio_node, &faddr);
  

•           addr_base = res.start;
            addr_size = res.end - res.start;
  
            /* If the bus is new then create a new bus */
  

diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 864589193b..4b097fb588 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -942,7 +942,11 @@ int fdt_get_resource(const void *fdt, int node, const char *property,

    while (ptr + na + ns <= end) {
            if (i == index) {

•                   res->start = fdtdec_get_number(ptr, na);
  

•                   if (CONFIG_IS_ENABLED(OF_TRANSLATE))
  

•                           res->start = fdt_translate_address(fdt, node, ptr);
  

•                   else
  

•                           res->start = fdtdec_get_number(ptr, na);
  

•                   res->end = res->start;
                    res->end += fdtdec_get_number(&ptr[na], ns) - 1;
                    return 0;
  

-- 2.17.1