All of lore.kernel.org
 help / color / mirror / Atom feed
* [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers
@ 2021-03-16 12:02 Vadim Pasternak
  2021-03-19 21:36 ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Vadim Pasternak @ 2021-03-16 12:02 UTC (permalink / raw)
  To: linux; +Cc: linux-hwmon, Vadim Pasternak

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@nvidia.com>
---
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", t
   clarify that "FAN presence" can be changed dynamically.
---
 drivers/hwmon/mlxreg-fan.c | 48 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
index ed8d59d4eecb..8a69044140a6 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 not physically connected, zero - otherwise.
+			 */
+			if (tacho->prsnt) {
+				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;
@@ -388,9 +412,9 @@ static int mlxreg_fan_speed_divider_get(struct mlxreg_fan *fan,
 static int mlxreg_fan_config(struct mlxreg_fan *fan,
 			     struct mlxreg_core_platform_data *pdata)
 {
+	int tacho_num = 0, regval, drwr_avail = 0, tacho_avail = 0, i;
 	struct mlxreg_core_data *data = pdata->data;
 	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,26 @@ static int mlxreg_fan_config(struct mlxreg_fan *fan,
 		}
 	}
 
+	if (pdata->capability) {
+		/* 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) {
+			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;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers
  2021-03-16 12:02 [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers Vadim Pasternak
@ 2021-03-19 21:36 ` Guenter Roeck
  2021-03-21  9:39   ` Vadim Pasternak
  0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-03-19 21:36 UTC (permalink / raw)
  To: Vadim Pasternak; +Cc: linux-hwmon

On 3/16/21 5:02 AM, 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@nvidia.com>

I had asked for a number of changes to v2. Resending v2 without
addressing anything isn't really helpful.

Guenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers
  2021-03-19 21:36 ` Guenter Roeck
@ 2021-03-21  9:39   ` Vadim Pasternak
  2021-03-21 16:12     ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Vadim Pasternak @ 2021-03-21  9:39 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon



> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Friday, March 19, 2021 11:36 PM
> To: Vadim Pasternak <vadimp@nvidia.com>
> Cc: linux-hwmon@vger.kernel.org
> Subject: Re: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add
> support for fan drawers capability and present registers
> 
> On 3/16/21 5:02 AM, 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@nvidia.com>
> 
> I had asked for a number of changes to v2. Resending v2 without addressing
> anything isn't really helpful.

Hi Guenter,

I am sorry, I missed the change related to your comment: 
"The outer double (( )) is pointless.".
I'll fix it.

I think I handled your other comments or I am wrong?

Should I send patch as v3 or as v2?

Thanks,
Vadim.

> 
> Guenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers
  2021-03-21  9:39   ` Vadim Pasternak
@ 2021-03-21 16:12     ` Guenter Roeck
  2021-03-21 16:23       ` Vadim Pasternak
  0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-03-21 16:12 UTC (permalink / raw)
  To: Vadim Pasternak; +Cc: linux-hwmon

On 3/21/21 2:39 AM, Vadim Pasternak wrote:
> 
> 
>> -----Original Message-----
>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>> Sent: Friday, March 19, 2021 11:36 PM
>> To: Vadim Pasternak <vadimp@nvidia.com>
>> Cc: linux-hwmon@vger.kernel.org
>> Subject: Re: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add
>> support for fan drawers capability and present registers
>>
>> On 3/16/21 5:02 AM, 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@nvidia.com>
>>
>> I had asked for a number of changes to v2. Resending v2 without addressing
>> anything isn't really helpful.
> 
> Hi Guenter,
> 
> I am sorry, I missed the change related to your comment: 
> "The outer double (( )) is pointless.".
> I'll fix it.
> 
> I think I handled your other comments or I am wrong?
> 

None of the comments I sent as response to the original v2
have been addressed.

> Should I send patch as v3 or as v2?
> 
Any change requires a new version.

Guenter

> Thanks,
> Vadim.
> 
>>
>> Guenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers
  2021-03-21 16:12     ` Guenter Roeck
@ 2021-03-21 16:23       ` Vadim Pasternak
  0 siblings, 0 replies; 5+ messages in thread
From: Vadim Pasternak @ 2021-03-21 16:23 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon



> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Sunday, March 21, 2021 6:13 PM
> To: Vadim Pasternak <vadimp@nvidia.com>
> Cc: linux-hwmon@vger.kernel.org
> Subject: Re: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add
> support for fan drawers capability and present registers
> 
> On 3/21/21 2:39 AM, Vadim Pasternak wrote:
> >
> >
> >> -----Original Message-----
> >> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> >> Sent: Friday, March 19, 2021 11:36 PM
> >> To: Vadim Pasternak <vadimp@nvidia.com>
> >> Cc: linux-hwmon@vger.kernel.org
> >> Subject: Re: [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan)
> >> Add support for fan drawers capability and present registers
> >>
> >> On 3/16/21 5:02 AM, 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@nvidia.com>
> >>
> >> I had asked for a number of changes to v2. Resending v2 without
> >> addressing anything isn't really helpful.
> >
> > Hi Guenter,
> >
> > I am sorry, I missed the change related to your comment:
> > "The outer double (( )) is pointless.".
> > I'll fix it.
> >
> > I think I handled your other comments or I am wrong?
> >
> 
> None of the comments I sent as response to the original v2 have been
> addressed.
> 
> > Should I send patch as v3 or as v2?
> >
> Any change requires a new version.

Ah, I understood now.
From some reason I didn't get your comments for v2 to my
mail box and I thought it was just missed.

Now, I found you review at:
https://www.spinics.net/lists/linux-hwmon/msg10195.html

I'll go over your comments and prepare v3.
Very sorry.

Thanks,
Vadim.

> 
> Guenter
> 
> > Thanks,
> > Vadim.
> >
> >>
> >> Guenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-21 16:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 12:02 [Re-send PATCH hwmon-next v2 1/1] hwmon: (mlxreg-fan) Add support for fan drawers capability and present registers Vadim Pasternak
2021-03-19 21:36 ` Guenter Roeck
2021-03-21  9:39   ` Vadim Pasternak
2021-03-21 16:12     ` Guenter Roeck
2021-03-21 16:23       ` Vadim Pasternak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.