Skip to contentSkip to conversation

fsl/fman: Clarification for implementation details in dtsec_config()

PATCH Applied SF Markus Elfring 4 participants 6 messages Jan 13, 2016 – Jan 14, 2016

Subsystems (4)
Networking Network drivers Kernel janitors Freescale QorIQ DPAA fman driver
lore
Share & tools
lore

SF Markus Elfring

author
#0 lore raw

Hello,

The Coccinelle semantic patch "deref_null.cocci" pointed out a potential problem in the implementation of the function "dtsec_config" (lines 1434-1439):
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/net/ethernet/freescale/fman/fman_dtsec.c?idåa03bfd873c29eb786655ef2e95e53ed242b404#n1434
…

	dtsec->tbiphy = of_phy_find_device(params->internal_phy_node);
	if (!dtsec->tbiphy) {
		pr_err("of_phy_find_device (TBI PHY) failed\n");
		put_device(&dtsec->tbiphy->mdio.dev);
		goto err_dtsec_drv_param;
	}

…

If the pointer "dtsec->tbiphy" is NULL, then it will especially matter if the shown address computation will succeed.
Should the call of the function "put_device" be removed,
or was another argument intended?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw

I think you can remove the put_device() line.

regards,
dan carpenter

#2 lore raw
Quoted 1 line · Dan Carpenter

I think you can remove the put_device() line.

Agreed.

of_phy_find_device() only does a get_device() when it finds a matching device.

Andrew

author
#3 lore raw

PATCH 1/1 fsl/fman: Delete one function call "put_device" in dtsec_config()

From: Markus Elfring <[email protected]>
Date: Thu, 14 Jan 2016 15:46:28 +0100

The Coccinelle semantic patch script "deref_null.cocci" pointed a problem out in the implementation of the function "dtsec_config".

A null pointer was assigned to the data structure member "tbiphy" of the variable "dtsec" if a matching device was not found.
A call of the function "put_device" was unnecessary then because a previous call of the function "get_device" was not triggered. Thus remove the function call "put_device" after the printing of the desired error message.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/net/ethernet/freescale/fman/fman_dtsec.c | 1 -
 1 file changed, 1 deletion(-)
drivers/net/ethernet/freescale/fman/fman_dtsec.c +0 -1
index 6b1261c..7c92eb8 100644
@@ -1434,7 +1434,6 @@ struct fman_mac *dtsec_config(struct fman_mac_params *params)
 	dtsec->tbiphy = of_phy_find_device(params->internal_phy_node);
 	if (!dtsec->tbiphy) {
 		pr_err("of_phy_find_device (TBI PHY) failed\n");
-		put_device(&dtsec->tbiphy->mdio.dev);
 		goto err_dtsec_drv_param;
 	}
Signature · 1 line
2.6.3
#4 lore raw

Re: [PATCH] fsl/fman: Delete one function call "put_device" in dtsec_config()

Quoted 6 lines · SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Thu, 14 Jan 2016 15:46:28 +0100

The Coccinelle semantic patch script "deref_null.cocci" pointed a problem out in the implementation of the function "dtsec_config".

Quoted 8 lines

A null pointer was assigned to the data structure member "tbiphy" of the variable "dtsec" if a matching device was not found.
A call of the function "put_device" was unnecessary then because a previous call of the function "get_device" was not triggered. Thus remove the function call "put_device" after the printing of the desired error message.

Signed-off-by Markus Elfring <[email protected]>
Fixes 57ba4c9b56d8 ("fsl/fman: Add FMan MAC support")
Reviewd-by Andrew Lunn <[email protected]>

Andrew

Quoted 19 lines
---
 drivers/net/ethernet/freescale/fman/fman_dtsec.c | 1 -
 1 file changed, 1 deletion(-)
drivers/net/ethernet/freescale/fman/fman_dtsec.c +0 -1
index 6b1261c..7c92eb8 100644
@@ -1434,7 +1434,6 @@ struct fman_mac *dtsec_config(struct fman_mac_params *params)
 	dtsec->tbiphy = of_phy_find_device(params->internal_phy_node);
 	if (!dtsec->tbiphy) {
 		pr_err("of_phy_find_device (TBI PHY) failed\n");
-		put_device(&dtsec->tbiphy->mdio.dev);
 		goto err_dtsec_drv_param;
 	}
Signature · 1 line
2.6.3
maintainer Applied
#5 lore raw

Re: [PATCH] fsl/fman: Delete one function call "put_device" in dtsec_config()

In reply to SF Markus Elfring

From: SF Markus Elfring <[email protected]>
Date: Thu, 14 Jan 2016 16:00:46 +0100

Quoted 6 lines

From: Markus Elfring <[email protected]>
Date: Thu, 14 Jan 2016 15:46:28 +0100

The Coccinelle semantic patch script "deref_null.cocci" pointed a problem out in the implementation of the function "dtsec_config".

Quoted 8 lines

A null pointer was assigned to the data structure member "tbiphy" of the variable "dtsec" if a matching device was not found.
A call of the function "put_device" was unnecessary then because a previous call of the function "get_device" was not triggered. Thus remove the function call "put_device" after the printing of the desired error message.

Signed-off-by Markus Elfring <[email protected]>

Applied, thanks.