Re: [PATCH] ARM: mach-qcom: fix support for ipq806x

From: Linus Walleij <linus.walleij@linaro.org>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>,
	Russell King <linux@armlinux.org.uk>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Arnd Bergmann <arnd@arndb.de>, Ard Biesheuvel <ardb@kernel.org>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	Nick Hawkins <nick.hawkins@hpe.com>,
	John Crispin <john@phrozen.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] ARM: mach-qcom: fix support for ipq806x
Date: Wed, 26 Oct 2022 10:19:21 +0200	[thread overview]
Message-ID: <CACRpkdb4iqazgVerHCPU0VqZKYoB5kJeDSaL+ek67L=2Txem-A@mail.gmail.com> (raw)
In-Reply-To: <6357240c.170a0220.999b2.23d6@mx.google.com>

On Tue, Oct 25, 2022 at 1:47 AM Christian Marangi <ansuelsmth@gmail.com> wrote:

> bad news... yesterday I tested this binding and it's problematic. It
> does work and the router correctly boot...

That's actually partly good news :D

> problem is that SMEM is
> broken with such configuration... I assume with this binding, by the
> system view ram starts from 0x42000000 instead of 0x40000000 and this
> cause SMEM to fail probe with the error "SBL didn't init SMEM".

We need to fix this.

> This is the location of SMEM entry in ram
>
>                 smem: smem@41000000 {
>                         compatible = "qcom,smem";
>                         reg = <0x41000000 0x200000>;
>                         no-map;
>
>                         hwlocks = <&sfpb_mutex 3>;
>                 };
(...)
> Wonder if you have other ideas about this.

So the problem is that the resource is outside of the system RAM?

I don't understand why that triggers it since this is per definition not
system RAM, it is SMEM after all. And it is no different in esssence
from any memory mapped IO or other things that are outside of
the system RAM.

The SMEM node is special since it is created without children thanks
to the hack in drivers/of/platform.c.

Then the driver in drivers/soc/qcom/smem.c
contains things like this:

        rmem = of_reserved_mem_lookup(pdev->dev.of_node);
        if (rmem) {
                smem->regions[0].aux_base = rmem->base;
                smem->regions[0].size = rmem->size;
        } else {
                /*
                 * Fall back to the memory-region reference, if we're not a
                 * reserved-memory node.
                 */
                ret = qcom_smem_resolve_mem(smem, "memory-region",
&smem->regions[0]);
                if (ret)
                        return ret;
        }

However it is treated as memory-mapped IO later:

        for (i = 1; i < num_regions; i++) {
                smem->regions[i].virt_base = devm_ioremap_wc(&pdev->dev,

smem->regions[i].aux_base,

smem->regions[i].size);
                if (!smem->regions[i].virt_base) {
                        dev_err(&pdev->dev, "failed to remap %pa\n",
&smem->regions[i].aux_base);
                        return -ENOMEM;
                }
        }

As a first hack I would check:

1. Is it the of_reserved_mem_lookup() or qcom_smem_resolve_smem() stuff
   in drivers/soc/qcom/smem.c that is failing?

If yes then:

2. Add a fallback path just using of_iomap(node) for aux_base and size
  with some comment like /* smem is outside of the main memory map */
  and see if that works.

Yours,
Linus Walleij