From 16b36a953bc7fc4843568abcdb7b32f92cc65457 Mon Sep 17 00:00:00 2001 From: Vadim Pasternak Date: Tue, 23 Aug 2022 23:19:34 +0300 Subject: platform/mellanox: mlxreg-lc: Fix coverity warning Fix smatch warning: drivers/platform/mellanox/mlxreg-lc.c:866 mlxreg_lc_probe() warn: passing zero to 'PTR_ERR' by removing 'err = PTR_ERR(regmap)'. Fixes: b4b830a34d80 ("platform/mellanox: mlxreg-lc: Fix error flow and extend verbosity") Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Vadim Pasternak Link: https://lore.kernel.org/r/20220823201937.46855-2-vadimp@nvidia.com Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede --- drivers/platform/mellanox/mlxreg-lc.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/platform/mellanox') diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c index 55834ccb4ac7..9a1bfcd24317 100644 --- a/drivers/platform/mellanox/mlxreg-lc.c +++ b/drivers/platform/mellanox/mlxreg-lc.c @@ -863,7 +863,6 @@ static int mlxreg_lc_probe(struct platform_device *pdev) if (err) { dev_err(&pdev->dev, "Failed to sync regmap for client %s at bus %d at addr 0x%02x\n", data->hpdev.brdinfo->type, data->hpdev.nr, data->hpdev.brdinfo->addr); - err = PTR_ERR(regmap); goto regcache_sync_fail; } -- cgit v1.2.3 From 52e01c0b1d80b0a7e8d9970456e10b788398e633 Mon Sep 17 00:00:00 2001 From: Vadim Pasternak Date: Tue, 23 Aug 2022 23:19:35 +0300 Subject: platform/mellanox: mlxreg-lc: Fix locking issue Fix locking issues: - mlxreg_lc_state_update() takes a lock when set or clear "MLXREG_LC_POWERED". - All the devices can be deleted before MLXREG_LC_POWERED flag is cleared. To fix it: - Add lock() / unlock() at the beginning / end of mlxreg_lc_event_handler() and remove locking from mlxreg_lc_power_on_off() and mlxreg_lc_enable_disable() - Add locked version of mlxreg_lc_state_update() - mlxreg_lc_state_update_locked() for using outside mlxreg_lc_event_handler(). (2) Remove redundant NULL check for of if 'data->notifier'. Fixes: 62f9529b8d5c87b ("platform/mellanox: mlxreg-lc: Add initial support for Nvidia line card devices") Reported-by: Dan Carpenter Signed-off-by: Vadim Pasternak Link: https://lore.kernel.org/r/20220823201937.46855-3-vadimp@nvidia.com Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede --- drivers/platform/mellanox/mlxreg-lc.c | 37 +++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) (limited to 'drivers/platform/mellanox') diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c index 9a1bfcd24317..e578c7bc060b 100644 --- a/drivers/platform/mellanox/mlxreg-lc.c +++ b/drivers/platform/mellanox/mlxreg-lc.c @@ -460,8 +460,6 @@ static int mlxreg_lc_power_on_off(struct mlxreg_lc *mlxreg_lc, u8 action) u32 regval; int err; - mutex_lock(&mlxreg_lc->lock); - err = regmap_read(mlxreg_lc->par_regmap, mlxreg_lc->data->reg_pwr, ®val); if (err) goto regmap_read_fail; @@ -474,7 +472,6 @@ static int mlxreg_lc_power_on_off(struct mlxreg_lc *mlxreg_lc, u8 action) err = regmap_write(mlxreg_lc->par_regmap, mlxreg_lc->data->reg_pwr, regval); regmap_read_fail: - mutex_unlock(&mlxreg_lc->lock); return err; } @@ -491,8 +488,6 @@ static int mlxreg_lc_enable_disable(struct mlxreg_lc *mlxreg_lc, bool action) * line card which is already has been enabled. Disabling does not affect the disabled line * card. */ - mutex_lock(&mlxreg_lc->lock); - err = regmap_read(mlxreg_lc->par_regmap, mlxreg_lc->data->reg_ena, ®val); if (err) goto regmap_read_fail; @@ -505,7 +500,6 @@ static int mlxreg_lc_enable_disable(struct mlxreg_lc *mlxreg_lc, bool action) err = regmap_write(mlxreg_lc->par_regmap, mlxreg_lc->data->reg_ena, regval); regmap_read_fail: - mutex_unlock(&mlxreg_lc->lock); return err; } @@ -537,6 +531,15 @@ mlxreg_lc_sn4800_c16_config_init(struct mlxreg_lc *mlxreg_lc, void *regmap, static void mlxreg_lc_state_update(struct mlxreg_lc *mlxreg_lc, enum mlxreg_lc_state state, u8 action) +{ + if (action) + mlxreg_lc->state |= state; + else + mlxreg_lc->state &= ~state; +} + +static void +mlxreg_lc_state_update_locked(struct mlxreg_lc *mlxreg_lc, enum mlxreg_lc_state state, u8 action) { mutex_lock(&mlxreg_lc->lock); @@ -560,8 +563,11 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, dev_info(mlxreg_lc->dev, "linecard#%d state %d event kind %d action %d\n", mlxreg_lc->data->slot, mlxreg_lc->state, kind, action); - if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED)) + mutex_lock(&mlxreg_lc->lock); + if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED)) { + mutex_unlock(&mlxreg_lc->lock); return 0; + } switch (kind) { case MLXREG_HOTPLUG_LC_SYNCED: @@ -574,7 +580,7 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, if (!(mlxreg_lc->state & MLXREG_LC_POWERED) && action) { err = mlxreg_lc_power_on_off(mlxreg_lc, 1); if (err) - return err; + goto mlxreg_lc_power_on_off_fail; } /* In case line card is configured - enable it. */ if (mlxreg_lc->state & MLXREG_LC_CONFIGURED && action) @@ -588,12 +594,13 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, /* In case line card is configured - enable it. */ if (mlxreg_lc->state & MLXREG_LC_CONFIGURED) err = mlxreg_lc_enable_disable(mlxreg_lc, 1); + mutex_unlock(&mlxreg_lc->lock); return err; } err = mlxreg_lc_create_static_devices(mlxreg_lc, mlxreg_lc->main_devs, mlxreg_lc->main_devs_num); if (err) - return err; + goto mlxreg_lc_create_static_devices_fail; /* In case line card is already in ready state - enable it. */ if (mlxreg_lc->state & MLXREG_LC_CONFIGURED) @@ -620,6 +627,10 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, break; } +mlxreg_lc_power_on_off_fail: +mlxreg_lc_create_static_devices_fail: + mutex_unlock(&mlxreg_lc->lock); + return err; } @@ -665,7 +676,7 @@ static int mlxreg_lc_completion_notify(void *handle, struct i2c_adapter *parent, if (err) goto mlxreg_lc_create_static_devices_failed; - mlxreg_lc_state_update(mlxreg_lc, MLXREG_LC_POWERED, 1); + mlxreg_lc_state_update_locked(mlxreg_lc, MLXREG_LC_POWERED, 1); } /* Verify if line card is synchronized. */ @@ -676,7 +687,7 @@ static int mlxreg_lc_completion_notify(void *handle, struct i2c_adapter *parent, /* Power on line card if necessary. */ if (regval & mlxreg_lc->data->mask) { mlxreg_lc->state |= MLXREG_LC_SYNCED; - mlxreg_lc_state_update(mlxreg_lc, MLXREG_LC_SYNCED, 1); + mlxreg_lc_state_update_locked(mlxreg_lc, MLXREG_LC_SYNCED, 1); if (mlxreg_lc->state & ~MLXREG_LC_POWERED) { err = mlxreg_lc_power_on_off(mlxreg_lc, 1); if (err) @@ -684,7 +695,7 @@ static int mlxreg_lc_completion_notify(void *handle, struct i2c_adapter *parent, } } - mlxreg_lc_state_update(mlxreg_lc, MLXREG_LC_INITIALIZED, 1); + mlxreg_lc_state_update_locked(mlxreg_lc, MLXREG_LC_INITIALIZED, 1); return 0; @@ -904,6 +915,8 @@ static int mlxreg_lc_remove(struct platform_device *pdev) struct mlxreg_core_data *data = dev_get_platdata(&pdev->dev); struct mlxreg_lc *mlxreg_lc = platform_get_drvdata(pdev); + mlxreg_lc_state_update_locked(mlxreg_lc, MLXREG_LC_INITIALIZED, 0); + /* * Probing and removing are invoked by hotplug events raised upon line card insertion and * removing. If probing procedure fails all data is cleared. However, hotplug event still -- cgit v1.2.3 From 1c8ee06b637f0b0146bbcda776d131e4822432c3 Mon Sep 17 00:00:00 2001 From: Vadim Pasternak Date: Tue, 23 Aug 2022 23:19:36 +0300 Subject: platform/mellanox: Remove unnecessary code Remove redundant 'NULL' check for of if 'data->notifier'. Replace 'return err' by 'return 0' in mlxreg_lc_probe(). Fixes: 62f9529b8d5c87b ("platform/mellanox: mlxreg-lc: Add initial support for Nvidia line card devices") Reported-by: Dan Carpenter Signed-off-by: Vadim Pasternak Link: https://lore.kernel.org/r/20220823201937.46855-4-vadimp@nvidia.com Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede --- drivers/platform/mellanox/mlxreg-lc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/platform/mellanox') diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c index e578c7bc060b..1e0c3ddc46cd 100644 --- a/drivers/platform/mellanox/mlxreg-lc.c +++ b/drivers/platform/mellanox/mlxreg-lc.c @@ -825,10 +825,9 @@ static int mlxreg_lc_probe(struct platform_device *pdev) mutex_init(&mlxreg_lc->lock); /* Set event notification callback. */ - if (data->notifier) { - data->notifier->user_handler = mlxreg_lc_event_handler; - data->notifier->handle = mlxreg_lc; - } + data->notifier->user_handler = mlxreg_lc_event_handler; + data->notifier->handle = mlxreg_lc; + data->hpdev.adapter = i2c_get_adapter(data->hpdev.nr); if (!data->hpdev.adapter) { dev_err(&pdev->dev, "Failed to get adapter for bus %d\n", @@ -888,7 +887,7 @@ static int mlxreg_lc_probe(struct platform_device *pdev) if (err) goto mlxreg_lc_config_init_fail; - return err; + return 0; mlxreg_lc_config_init_fail: regcache_sync_fail: -- cgit v1.2.3 From 059209fd902f5ad4bec8124931c258e62fddf74f Mon Sep 17 00:00:00 2001 From: Vadim Pasternak Date: Tue, 23 Aug 2022 23:19:37 +0300 Subject: platform/mellanox: Remove redundant 'NULL' check Remove 'NULL' check for 'data->hpdev.client' in error flow of mlxreg_lc_probe(). It cannot be 'NULL' at this point. Fixes: b4b830a34d80 ("platform/mellanox: mlxreg-lc: Fix error flow and extend verbosity") Reported-by: Dan Carpenter Signed-off-by: Vadim Pasternak Link: https://lore.kernel.org/r/20220823201937.46855-5-vadimp@nvidia.com Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede --- drivers/platform/mellanox/mlxreg-lc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/platform/mellanox') diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c index 1e0c3ddc46cd..1e071df4c9f5 100644 --- a/drivers/platform/mellanox/mlxreg-lc.c +++ b/drivers/platform/mellanox/mlxreg-lc.c @@ -893,10 +893,8 @@ mlxreg_lc_config_init_fail: regcache_sync_fail: regmap_write_fail: devm_regmap_init_i2c_fail: - if (data->hpdev.client) { - i2c_unregister_device(data->hpdev.client); - data->hpdev.client = NULL; - } + i2c_unregister_device(data->hpdev.client); + data->hpdev.client = NULL; i2c_new_device_fail: i2c_put_adapter(data->hpdev.adapter); data->hpdev.adapter = NULL; -- cgit v1.2.3 From 072aba58c9a4fa5edbfa1db92f78951cbb65d9b9 Mon Sep 17 00:00:00 2001 From: Vadim Pasternak Date: Sun, 4 Sep 2022 17:11:13 +0300 Subject: platform/mellanox: mlxreg-lc: Make error handling flow consistent Use 'goto' statement in error flow of mlxreg_lc_event_handler() at all places for consistency. This follow-up patch implementing comments from https://www.spinics.net/lists/platform-driver-x86/msg34587.html Signed-off-by: Vadim Pasternak Link: https://lore.kernel.org/r/20220904141113.49048-1-vadimp@nvidia.com Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede --- drivers/platform/mellanox/mlxreg-lc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/platform/mellanox') diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c index 1e071df4c9f5..8d833836a6d3 100644 --- a/drivers/platform/mellanox/mlxreg-lc.c +++ b/drivers/platform/mellanox/mlxreg-lc.c @@ -564,10 +564,8 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, mlxreg_lc->data->slot, mlxreg_lc->state, kind, action); mutex_lock(&mlxreg_lc->lock); - if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED)) { - mutex_unlock(&mlxreg_lc->lock); - return 0; - } + if (!(mlxreg_lc->state & MLXREG_LC_INITIALIZED)) + goto mlxreg_lc_non_initialzed_exit; switch (kind) { case MLXREG_HOTPLUG_LC_SYNCED: @@ -594,8 +592,8 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, /* In case line card is configured - enable it. */ if (mlxreg_lc->state & MLXREG_LC_CONFIGURED) err = mlxreg_lc_enable_disable(mlxreg_lc, 1); - mutex_unlock(&mlxreg_lc->lock); - return err; + + goto mlxreg_lc_enable_disable_exit; } err = mlxreg_lc_create_static_devices(mlxreg_lc, mlxreg_lc->main_devs, mlxreg_lc->main_devs_num); @@ -627,8 +625,10 @@ static int mlxreg_lc_event_handler(void *handle, enum mlxreg_hotplug_kind kind, break; } +mlxreg_lc_enable_disable_exit: mlxreg_lc_power_on_off_fail: mlxreg_lc_create_static_devices_fail: +mlxreg_lc_non_initialzed_exit: mutex_unlock(&mlxreg_lc->lock); return err; -- cgit v1.2.3