Re: [PATCH 09/18] mtd: rawnand: davinci: Convert to devm_platform_ioremap_resource()

Yangtao Li
2023-07-13T08:24:49Z

Hi Mique,

On 2023/7/12 20:09, Miquel Raynal wrote:
> Hi Yangtao,
>
> frank.li@vivo.com wrote on Fri, 7 Jul 2023 12:06:13 +0800:
>
>> Use devm_platform_ioremap_resource() to simplify code.
>>
>> Signed-off-by: Yangtao Li <frank.li@vivo.com>
>> ---
>> drivers/mtd/nand/raw/davinci_nand.c | 21 +++++++++------------
>> 1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
>> index 415d6aaa8255..2db1cd1d3d03 100644
>> --- a/drivers/mtd/nand/raw/davinci_nand.c
>> +++ b/drivers/mtd/nand/raw/davinci_nand.c
>> @@ -710,8 +710,7 @@ static int nand_davinci_probe(struct platform_device *pdev)
>> {
>> struct davinci_nand_pdata *pdata;
>> struct davinci_nand_info *info;
>> - struct resource *res1;
>> - struct resource *res2;
>> + struct resource *res;
>> void __iomem *vaddr;
>> void __iomem *base;
>> int ret;
>> @@ -736,26 +735,24 @@ static int nand_davinci_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, info);
>>
>> - res1 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> - if (!res1 || !res2) {
>> - dev_err(&pdev->dev, "resource missing\n");
>> - return -EINVAL;
>> - }
>> -
>> - vaddr = devm_ioremap_resource(&pdev->dev, res1);
>> + vaddr = devm_platform_ioremap_resource(pdev, 0);
>> if (IS_ERR(vaddr))
>> return PTR_ERR(vaddr);
>>
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + if (!res) {
>> + dev_err(&pdev->dev, "resource missing\n");
>> + return -EINVAL;
>> + }
>> /*
>> * This registers range is used to setup NAND settings. In case with
>> * TI AEMIF driver, the same memory address range is requested already
>> * by AEMIF, so we cannot request it twice, just ioremap.
>> * The AEMIF and NAND drivers not use the same registers in this range.
>> */
>> - base = devm_ioremap(&pdev->dev, res2->start, resource_size(res2));
>> + base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>> if (!base) {
>> - dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res2);
>> + dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res);
> I believe this is the only use of the resource, I am in favor of just
> using the regular devm_platform_ioremap_resource() helper here and just
> drop the reference to the resource from the message.

I guess maybe you can apply this patch directly?

It should be noted that devm_ioremap_resource is not the same as
devm_ioremap,

here is devm_ioremap (devm_request_mem_region will not be called, and
there are comments explaining why this interface is needed),

which means that we cannot simply replace it with
devm_platform_ioremap_resource.

MBR,

Yangtao

>
> I will apply all other patches, please send a v2 only for this one
> once improved.
>
>> return -EADDRNOTAVAIL;
>> }
>>
>
> Thanks,
> Miquèl