linux-kernel - Re: [PATCH 04/21] ata: libahci_platform: Convert to using handy devm-ioremap methods

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Mar 2022 10:11:09 +0900
From:   Damien Le Moal <damien.lemoal@...nsource.wdc.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Hans de Goede <hdegoede@...hat.com>,
        Jens Axboe <axboe@...nel.dk>
Cc:     Serge Semin <fancer.lancer@...il.com>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Rob Herring <robh+dt@...nel.org>, linux-ide@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 04/21] ata: libahci_platform: Convert to using handy
 devm-ioremap methods

On 3/24/22 09:16, Serge Semin wrote:
> Currently the IOMEM AHCI registers space is mapped by means of the
> two functions invocation: platform_get_resource() is used to get the very
> first memory resource and devm_ioremap_resource() is called to remap that
> resource. Device-managed kernel API provides a handy wrapper to perform
> the same in single function call: devm_platform_ioremap_resource().

> 
> While at it seeing many AHCI platform drivers rely on having the AHCI CSR
> space marked with "ahci" name let's first try to find and remap the CSR
> IO-mem with that name and only if it fails fallback to getting the very
> first registers space platform resource.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> ---
>  drivers/ata/libahci_platform.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 1bd2f1686239..8eabbb5f208c 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -404,11 +404,13 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
>  
>  	devres_add(dev, hpriv);
>  
> -	hpriv->mmio = devm_ioremap_resource(dev,
> -			      platform_get_resource(pdev, IORESOURCE_MEM, 0));
> +	hpriv->mmio = devm_platform_ioremap_resource_byname(pdev, "ahci");

See __devm_ioremap_resource(): if there is no resource named "ahci" found,
then this will print an error message ("invalid resource\n"). That may
confuse users as this error message was not present before. So you may
want to change this code to something like this:

/*
 * If the DT provided an "ahci" named resource, use it. Otherwise,
 * fallback to using the default first resource for the device node.
 */
if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci"))
	hpriv->mmio = devm_platform_ioremap_resource_byname(pdev, "ahci");
else
	hpriv->mmio = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(hpriv->mmio)) {
	rc = PTR_ERR(hpriv->mmio);
	goto err_out;
}

>  	if (IS_ERR(hpriv->mmio)) {
> -		rc = PTR_ERR(hpriv->mmio);
> -		goto err_out;
> +		hpriv->mmio = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(hpriv->mmio)) {
> +			rc = PTR_ERR(hpriv->mmio);
> +			goto err_out;
> +		}
>  	}
>  
>  	for (i = 0; i < AHCI_MAX_CLKS; i++) {


-- 
Damien Le Moal
Western Digital Research