Commit 52e01c0b authored by Vadim Pasternak's avatar Vadim Pasternak Committed by Hans de Goede
Browse files

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: 62f9529b ("platform/mellanox: mlxreg-lc: Add initial support for Nvidia line card devices")
Reported-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: default avatarVadim Pasternak <vadimp@nvidia.com>
Link: https://lore.kernel.org/r/20220823201937.46855-3-vadimp@nvidia.com


Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
parent 16b36a95

drivers/platform/mellanox/mlxreg-lc.c

+25 −12
Original line number Diff line number Diff line
@@ -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, &regval);

	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, &regval);

	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