linux-kernel - Re: [PATCHv3 net-next] net: modernize IRQ resource acquisition

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] [day] [month] [year] [list]

Message-ID: <CAGRGNgXhtPy_G9O0n7dEhcAX3sWN=08tF9tgFpLs8V---uELYg@mail.gmail.com>
Date: Wed, 13 Nov 2024 20:48:24 +1100
From: Julian Calaby <julian.calaby@...il.com>
To: Rosen Penev <rosenp@...il.com>
Cc: netdev@...r.kernel.org, Marc Kleine-Budde <mkl@...gutronix.de>, 
	Vincent Mailhol <mailhol.vincent@...adoo.fr>, Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Florian Fainelli <florian.fainelli@...adcom.com>, Vladimir Oltean <olteanv@...il.com>, 
	Chen-Yu Tsai <wens@...e.org>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Samuel Holland <samuel@...lland.org>, Pantelis Antoniou <pantelis.antoniou@...il.com>, 
	Marcin Wojtas <marcin.s.wojtas@...il.com>, Byungho An <bh74.an@...sung.com>, 
	Kevin Brace <kevinbrace@...cecomputerlab.com>, Francois Romieu <romieu@...zoreil.com>, 
	Michal Simek <michal.simek@....com>, Heiner Kallweit <hkallweit1@...il.com>, 
	Russell King <linux@...linux.org.uk>, Zhao Qiang <qiang.zhao@....com>, 
	"open list:CAN NETWORK DRIVERS" <linux-can@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>, 
	"moderated list:ARM/Allwinner sunXi SoC support" <linux-arm-kernel@...ts.infradead.org>, 
	"open list:ARM/Allwinner sunXi SoC support" <linux-sunxi@...ts.linux.dev>, 
	"open list:FREESCALE SOC FS_ENET DRIVER" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCHv3 net-next] net: modernize IRQ resource acquisition

Hi Rosen,

On Wed, Nov 13, 2024 at 8:14 AM Rosen Penev <rosenp@...il.com> wrote:
>
> In probe, np == pdev->dev.of_node. It's easier to pass pdev directly.
>
> Replace irq_of_parse_and_map() by platform_get_irq() to do so. Requires
> removing the error message as well as fixing the return type.
>
> Replace of_address_to_resource() with platform_get_resource() for the
> same reason.

Sorry for the drive-by review, but I have to question the utility of
this conversion.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index cdf0ec9fa7f3..48e93c3445e7 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1673,9 +1673,8 @@ static int grcan_probe(struct platform_device *ofdev)
>                 goto exit_error;
>         }
>
> -       irq = irq_of_parse_and_map(np, GRCAN_IRQIX_IRQ);
> -       if (!irq) {
> -               dev_err(&ofdev->dev, "no irq found\n");
> +       irq = platform_get_irq(ofdev, GRCAN_IRQIX_IRQ);
> +       if (irq < 0) {

In this change and a lot of the others, you're not removing the "np"
variable, so you're basically replacing one wrapper with another.

>                 err = -ENODEV;
>                 goto exit_error;
>         }
> diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
> index be63293511d9..9006137e3a55 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
> @@ -83,8 +83,8 @@ static int do_pd_setup(struct fs_enet_private *fep)
>         struct fs_platform_info *fpi = fep->fpi;
>         int ret = -EINVAL;
>
> -       fep->interrupt = irq_of_parse_and_map(ofdev->dev.of_node, 0);
> -       if (!fep->interrupt)
> +       fep->interrupt = platform_get_irq(ofdev, 0);
> +       if (fep->interrupt < 0)

This one and others like it are fine: it's much cleaner to use the
"platform_irq()" function instead of reaching deep into the structure
to grab the "of_node" property.

That said, in this case and probably a few others this is a driver for
an OF device so I'm still not sure this actually makes sense.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@...il.com
Profile: http://www.google.com/profiles/julian.calaby/