Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM

On Fri, Jan 8, 2021 at 2:02 AM Bjorn Andersson
[off-list ref] wrote:

On Wed 06 Jan 19:50 CST 2021, Tzung-Bi Shih wrote:

quoted

On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
[off-list ref] wrote:

quoted

quoted

 static void mt8183_scp_stop(struct mtk_scp *scp)
 {
      /* Disable SCP watchdog */

@@ -714,6 +749,19 @@ static int scp_probe(struct platform_device *pdev)
              goto free_rproc;
      }
      scp->sram_size = resource_size(res);
+     scp->sram_phys = res->start;
+
+     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
+     if (res) {

As far as I can tell the if() condition isn't needed since
platform_get_resource_byname() returns NULL on error and devm_ioremap_resource()
is capable of handling that condition.  As such the code to parse "l1tcm" can be
the same as what is done for "sram".

The "l1tcm" memory region is optional.  The if() condition is for: if
DTS doesn't provide the memory region, kernel can skip the code block.

People are actively looking for platform_get_resource_byname +
devm_ioremap_resource() pairs to replace with
devm_platform_ioremap_resource_byname(), so we're probably going to have
someone try to patch this soon...

So please change the pair to devm_platform_ioremap_resource_byname() and
treat a returned -EINVAL as the memory isn't specified and other
IS_ERR() as errors.

Will do but what if we need to access the "res"?

For example:
scp->sram_size = resource_size(res);

By using devm_platform_ioremap_resource_byname() which will drop the
access to res.  Shall we add a new variant of
devm_platform_ioremap_resource_byname()?