On Wed 06 Jan 19:50 CST 2021, Tzung-Bi Shih wrote:
> On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> <mathieu.poirier@xxxxxxxxxx> wrote:
> >
> > > 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.
Thanks,
Bjorn
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> >
> > > + scp->l1tcm_base = devm_ioremap_resource(dev, res);
> > > + if (IS_ERR((__force void *)scp->l1tcm_base)) {
> > > + dev_err(dev, "Failed to parse and map l1tcm memory\n");
> > > + ret = PTR_ERR((__force void *)scp->l1tcm_base);
> > > + goto free_rproc;
> > > + }
> > > + scp->l1tcm_size = resource_size(res);
> > > + scp->l1tcm_phys = res->start;
> > > + }
