[PATCH net-next v1 3/7] ptp: ocp: Refactor ptp_ocp_i2c_notifier_call()

Andy Shevchenko posted 7 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH net-next v1 3/7] ptp: ocp: Refactor ptp_ocp_i2c_notifier_call()
Posted by Andy Shevchenko 2 months, 4 weeks ago
Refactor ptp_ocp_i2c_notifier_call() to avoid unneeded local variable.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ptp/ptp_ocp.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 28243fb1d78f..1dbbca4197bc 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -4872,16 +4872,6 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
 {
 	struct device *dev, *child = data;
 	struct ptp_ocp *bp;
-	bool add;
-
-	switch (action) {
-	case BUS_NOTIFY_ADD_DEVICE:
-	case BUS_NOTIFY_DEL_DEVICE:
-		add = action == BUS_NOTIFY_ADD_DEVICE;
-		break;
-	default:
-		return 0;
-	}
 
 	if (!i2c_verify_adapter(child))
 		return 0;
@@ -4894,10 +4884,17 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
 
 found:
 	bp = dev_get_drvdata(dev);
-	if (add)
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
 		ptp_ocp_symlink(bp, child, "i2c");
-	else
+		break;
+	case BUS_NOTIFY_DEL_DEVICE:
 		sysfs_remove_link(&bp->dev.kobj, "i2c");
+		break;
+	default:
+		return 0;
+	}
 
 	return 0;
 }
-- 
2.50.1
Re: [PATCH net-next v1 3/7] ptp: ocp: Refactor ptp_ocp_i2c_notifier_call()
Posted by Vadim Fedorenko 2 months, 3 weeks ago
On 11/11/2025 16:52, Andy Shevchenko wrote:
> Refactor ptp_ocp_i2c_notifier_call() to avoid unneeded local variable.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/ptp/ptp_ocp.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 28243fb1d78f..1dbbca4197bc 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -4872,16 +4872,6 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
>   {
>   	struct device *dev, *child = data;
>   	struct ptp_ocp *bp;
> -	bool add;
> -
> -	switch (action) {
> -	case BUS_NOTIFY_ADD_DEVICE:
> -	case BUS_NOTIFY_DEL_DEVICE:
> -		add = action == BUS_NOTIFY_ADD_DEVICE;
> -		break;
> -	default:
> -		return 0;
> -	}

the reason we've done it is to avoid iterating over devices and do
string comparisons for actions we don't care about. We can still avoid
local variable by changing if condition later in the code, but I'm not
sure this refactor gives any benefits.

>   
>   	if (!i2c_verify_adapter(child))
>   		return 0;
> @@ -4894,10 +4884,17 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
>   
>   found:
>   	bp = dev_get_drvdata(dev);
> -	if (add)
> +
> +	switch (action) {
> +	case BUS_NOTIFY_ADD_DEVICE:
>   		ptp_ocp_symlink(bp, child, "i2c");
> -	else
> +		break;
> +	case BUS_NOTIFY_DEL_DEVICE:
>   		sysfs_remove_link(&bp->dev.kobj, "i2c");
> +		break;
> +	default:
> +		return 0;
> +	}
>   
>   	return 0;
>   }
Re: [PATCH net-next v1 3/7] ptp: ocp: Refactor ptp_ocp_i2c_notifier_call()
Posted by Andy Shevchenko 2 months, 3 weeks ago
On Wed, Nov 12, 2025 at 01:45:31PM +0000, Vadim Fedorenko wrote:
> On 11/11/2025 16:52, Andy Shevchenko wrote:
> > Refactor ptp_ocp_i2c_notifier_call() to avoid unneeded local variable.

...

> the reason we've done it is to avoid iterating over devices and do
> string comparisons for actions we don't care about. We can still avoid
> local variable by changing if condition later in the code, but I'm not
> sure this refactor gives any benefits.

This is definitely a slow path and having less LoCs and straightforward code
flow is important for readability and maintenance. I find my proposed change
useful.

-- 
With Best Regards,
Andy Shevchenko