Re: [PATCH hwmon-next v3 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 22, 2021 at 07:22:37PM +0200, Vadim Pasternak wrote:
> Add support for fan drawer's capability and present registers in order
> to set mapping between the fan drawers and tachometers. Some systems
> are equipped with fan drawers with one tachometer inside. Others with
> fan drawers with several tachometers inside. Using present register
> along with tachometer-to-drawer mapping allows to skip reading missed
> tachometers and expose input for them as zero, instead of exposing
> fault code returned by hardware.
> 
> Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxx>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> v2->v3:
>  Comments pointed out by Guenter:
>  - Remove unnecessary braces mlxreg_fan_read() (missed in v2).
>  - In order to avoid divide by 0 error, validate 'tachos_per_drwr' is
>    set prior mapping tachometer to drawer in mlxreg_fan_read().
>  - Change 'regval' to unsigned integer in mlxreg_fan_config().
>    Move it inside the 'if' statement, where it is used.
>  - No need to initialize 'drwr_avail' in mlxreg_fan_config().
>    Move it inside the 'if' statement, where it is used.
>  - Return error in case number of fan drawers less the number of
>    tachometers, since this is not valid.
>  Added by Vadim:
>  - Fix comment in mlxreg_fan_read().
> v1->v2:
>  Comments pointed out by Guenter:
>  - Simplify drawers number calculation in mlxreg_fan_config().
>  - Validate that the number of drawers and tachometers, supported on
>    system, both are not zero. Otherwise return error.
>  Added by Vadim:
>  - Change comment in mlxreg_fan_read() regarding FAN presence - use
>    "FAN is inserted" instead of "FAN is physically connected", to
>    clarify that "FAN presence" can be changed dynamically.
> ---
>  drivers/hwmon/mlxreg-fan.c | 51 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> index ed8d59d4eecb..116681fde33d 100644
> --- a/drivers/hwmon/mlxreg-fan.c
> +++ b/drivers/hwmon/mlxreg-fan.c
> @@ -67,11 +67,13 @@
>   * @connected: indicates if tachometer is connected;
>   * @reg: register offset;
>   * @mask: fault mask;
> + * @prsnt: present register offset;
>   */
>  struct mlxreg_fan_tacho {
>  	bool connected;
>  	u32 reg;
>  	u32 mask;
> +	u32 prsnt;
>  };
>  
>  /*
> @@ -92,6 +94,7 @@ struct mlxreg_fan_pwm {
>   * @regmap: register map of parent device;
>   * @tacho: tachometer data;
>   * @pwm: PWM data;
> + * @tachos_per_drwr - number of tachometers per drawer;
>   * @samples: minimum allowed samples per pulse;
>   * @divider: divider value for tachometer RPM calculation;
>   * @cooling: cooling device levels;
> @@ -103,6 +106,7 @@ struct mlxreg_fan {
>  	struct mlxreg_core_platform_data *pdata;
>  	struct mlxreg_fan_tacho tacho[MLXREG_FAN_MAX_TACHO];
>  	struct mlxreg_fan_pwm pwm;
> +	int tachos_per_drwr;
>  	int samples;
>  	int divider;
>  	u8 cooling_levels[MLXREG_FAN_MAX_STATE + 1];
> @@ -123,6 +127,26 @@ mlxreg_fan_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		tacho = &fan->tacho[channel];
>  		switch (attr) {
>  		case hwmon_fan_input:
> +			/*
> +			 * Check FAN presence: FAN related bit in presence register is one,
> +			 * if FAN is physically connected, zero - otherwise.
> +			 */
> +			if (tacho->prsnt && fan->tachos_per_drwr) {
> +				err = regmap_read(fan->regmap, tacho->prsnt, &regval);
> +				if (err)
> +					return err;
> +
> +				/*
> +				 * Map channel to presence bit - drawer can be equipped with
> +				 * one or few FANs, while presence is indicated per drawer.
> +				 */
> +				if (BIT(channel / fan->tachos_per_drwr) & regval) {
> +					/* FAN is not connected - return zero for FAN speed. */
> +					*val = 0;
> +					return 0;
> +				}
> +			}
> +
>  			err = regmap_read(fan->regmap, tacho->reg, &regval);
>  			if (err)
>  				return err;
> @@ -389,8 +413,8 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
>  			     struct mlxreg_core_platform_data *pdata)
>  {
>  	struct mlxreg_core_data *data = pdata->data;
> +	int tacho_num = 0, tacho_avail = 0, i;
>  	bool configured = false;
> -	int tacho_num = 0, i;
>  	int err;
>  
>  	fan->samples = MLXREG_FAN_TACHO_SAMPLES_PER_PULSE_DEF;
> @@ -415,7 +439,9 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
>  
>  			fan->tacho[tacho_num].reg = data->reg;
>  			fan->tacho[tacho_num].mask = data->mask;
> +			fan->tacho[tacho_num].prsnt = data->reg_prsnt;
>  			fan->tacho[tacho_num++].connected = true;
> +			tacho_avail++;
>  		} else if (strnstr(data->label, "pwm", sizeof(data->label))) {
>  			if (fan->pwm.connected) {
>  				dev_err(fan->dev, "duplicate pwm entry: %s\n",
> @@ -453,6 +479,29 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
>  		}
>  	}
>  
> +	if (pdata->capability) {
> +		int drwr_avail;
> +		u32 regval;
> +
> +		/* Obtain the number of FAN drawers, supported by system. */
> +		err = regmap_read(fan->regmap, pdata->capability, &regval);
> +		if (err) {
> +			dev_err(fan->dev, "Failed to query capability register 0x%08x\n",
> +				pdata->capability);
> +			return err;
> +		}
> +
> +		drwr_avail = hweight32(regval);
> +		if (!tacho_avail || !drwr_avail || tacho_avail < drwr_avail) {
> +			dev_err(fan->dev, "Configuration is invalid: drawers num %d tachos num %d\n",
> +				drwr_avail, tacho_avail);
> +			return -EINVAL;
> +		}
> +
> +		/* Set the number of tachometers per one drawer. */
> +		fan->tachos_per_drwr = tacho_avail / drwr_avail;
> +	}
> +
>  	/* Init cooling levels per PWM state. */
>  	for (i = 0; i < MLXREG_FAN_SPEED_MIN_LEVEL; i++)
>  		fan->cooling_levels[i] = MLXREG_FAN_SPEED_MIN_LEVEL;



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux