linux-kernel - Re: [PATCH net-next 1/4] net: ibm: emac: tah: use devm and dev

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: <76049027-f24b-4b7d-9bcc-5dae001233d3@lunn.ch>
Date: Tue, 10 Sep 2024 18:37:37 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Rosen Penev <rosenp@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, linux-kernel@...r.kernel.org,
	jacob.e.keller@...el.com, horms@...nel.org, sd@...asysnail.net,
	chunkeey@...il.com
Subject: Re: [PATCH net-next 1/4] net: ibm: emac: tah: use devm and dev_err

On Sat, Sep 07, 2024 at 03:21:44PM -0700, Rosen Penev wrote:
> Simplifies the driver by removing manual frees and using dev_err instead
> of printk. pdev->dev has the of_node name in it. eg.
> 
> TAH /plb/opb/emac-tah@...01350 initialized
> 
> vs
> 
> emac-tah 4ef601350.emac-tah: initialized
> 
> close enough.

There are lots of different things going on in this patch. It would be
better to split it up.

> -void tah_reset(struct platform_device *ofdev)
> +void tah_reset(struct platform_device *pdev)

Search/replace would be one patch.

>  {
> -	struct tah_instance *dev = platform_get_drvdata(ofdev);
> +	struct tah_instance *dev = platform_get_drvdata(pdev);
>  	struct tah_regs __iomem *p = dev->base;
>  	int n;
>  
> @@ -56,7 +56,7 @@ void tah_reset(struct platform_device *ofdev)
>  		--n;
>  
>  	if (unlikely(!n))
> -		printk(KERN_ERR "%pOF: reset timeout\n", ofdev->dev.of_node);
> +		dev_err(&pdev->dev, "reset timeout");

printk() to dev_err() another patch.

> -	rc = -ENOMEM;
> -	dev = kzalloc(sizeof(struct tah_instance), GFP_KERNEL);
> -	if (dev == NULL)
> -		goto err_gone;
> +	dev = devm_kzalloc(&pdev->dev, sizeof(struct tah_instance), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
>  

devm another patch.

>  	mutex_init(&dev->lock);
> -	dev->ofdev = ofdev;
> +	dev->ofdev = pdev;

It seems odd to not also rename dev->ofdev to dev->pdev, so it is
consistent.

    Andrew

---
pw-bot: cr