From 17c2bd6bea4c32fe691c1f9ebcc20fd48d77454a 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/mlxreg-lc.c') 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 From 1e092b7faa6b507125b69c92a51bb22c2d549d37 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/mlxreg-lc.c') 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 From 2f92fdd043d548a14287889742e147f8ed4ee03d 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/mlxreg-lc.c') 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 From 791ae8e8960efbf2e331eae7110db65b4f9f9083 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/mlxreg-lc.c') 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