List of fixes:
- Fix for VM sensor allocation
- Fix multi-channel voltage reading
- Protect from negative voltage value
- Fix temperature equation and coefficients
List of new features:
- Add option to skip reset controller
- Add VM active channel support
- Add VM pre-scalar support
- Add parsing of thermal coefficients from device-tree
- Add debugfs to updater temperature coefficients on the fly
Changes between v1 and v2:
*) Fix compilation error for patch 08/16:
"warning: ISO C90 forbids variable length array"
Eliav Farber (16):
hwmon: (mr75203) fix VM sensor allocation when "intel,vm-map" not
defined
hwmon: (mr75203) update pvt->v_num to the actual number of used
sensors
hwmon: (mr75203) update Moortec PVT controller intel,vm-map property
hwmon: (mr75203) add Moortec PVT controller reset-control-skip
property
hwmon: (mr75203) add option to skip reset controller
hwmon: (mr75203) fix multi-channel voltage reading
hwmon: (mr75203) add VM active channels property for Moortec PVT
controller
hwmon: (mr75203) add VM active channel support
hwmon: (mr75203) add VM pre-scalar property for Moortec PVT controller
hwmon: (mr75203) add VM pre-scalar support
hwmon: (mr75203) add protection for negative voltage value
hwmon: (mr75203) modify the temperature equation
hwmon: (mr75203) add thermal coefficient properties for Moortec PVT
controller
hwmon: (mr75203) parse thermal coefficients from device-tree
hwmon: (mr75203) fix coding style space errors
hwmon: (mr75203) add debugfs to read and write temperature
coefficients
.../bindings/hwmon/moortec,mr75203.yaml | 60 ++-
drivers/hwmon/mr75203.c | 467 +++++++++++++++---
2 files changed, 461 insertions(+), 66 deletions(-)
--
2.37.1
Fix: "ERROR: space required before the open parenthesis '('"
Signed-off-by: Eliav Farber <[email protected]>
---
drivers/hwmon/mr75203.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index e54a4d1803e4..2898565afaab 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -174,7 +174,7 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
return ret;
ret = regmap_read(t_map, SDIF_DATA(channel), &nbs);
- if(ret < 0)
+ if (ret < 0)
return ret;
nbs &= SAMPLE_DATA_MSK;
@@ -219,7 +219,7 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
return ret;
ret = regmap_read(v_map, VM_SDIF_DATA(vm_idx, ch_idx), &n);
- if(ret < 0)
+ if (ret < 0)
return ret;
n &= SAMPLE_DATA_MSK;
@@ -322,19 +322,19 @@ static int pvt_init(struct pvt_device *pvt)
if (t_num) {
ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(t_map, SDIF_HALT, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(t_map, CLK_SYNTH, clk_synth);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(t_map, SDIF_DISABLE, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_read_poll_timeout(t_map, SDIF_STAT,
@@ -347,7 +347,7 @@ static int pvt_init(struct pvt_device *pvt)
val = CFG0_MODE_2 | CFG0_PARALLEL_OUT | CFG0_12_BIT |
IP_CFG << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG;
ret = regmap_write(t_map, SDIF_W, val);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_read_poll_timeout(t_map, SDIF_STAT,
@@ -360,7 +360,7 @@ static int pvt_init(struct pvt_device *pvt)
val = POWER_DELAY_CYCLE_256 | IP_TMR << SDIF_ADDR_SFT |
SDIF_WRN_W | SDIF_PROG;
ret = regmap_write(t_map, SDIF_W, val);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_read_poll_timeout(t_map, SDIF_STAT,
@@ -374,39 +374,39 @@ static int pvt_init(struct pvt_device *pvt)
IP_CTRL << SDIF_ADDR_SFT |
SDIF_WRN_W | SDIF_PROG;
ret = regmap_write(t_map, SDIF_W, val);
- if(ret < 0)
+ if (ret < 0)
return ret;
}
if (p_num) {
ret = regmap_write(p_map, SDIF_HALT, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(p_map, SDIF_DISABLE, BIT(p_num) - 1);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(p_map, CLK_SYNTH, clk_synth);
- if(ret < 0)
+ if (ret < 0)
return ret;
}
if (v_num) {
ret = regmap_write(v_map, SDIF_SMPL_CTRL, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(v_map, SDIF_HALT, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(v_map, CLK_SYNTH, clk_synth);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_write(v_map, SDIF_DISABLE, 0x0);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_read_poll_timeout(v_map, SDIF_STAT,
@@ -434,7 +434,7 @@ static int pvt_init(struct pvt_device *pvt)
CFG1_14_BIT | IP_CFG << SDIF_ADDR_SFT |
SDIF_WRN_W | SDIF_PROG;
ret = regmap_write(v_map, SDIF_W, val);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_read_poll_timeout(v_map, SDIF_STAT,
@@ -447,7 +447,7 @@ static int pvt_init(struct pvt_device *pvt)
val = POWER_DELAY_CYCLE_64 | IP_TMR << SDIF_ADDR_SFT |
SDIF_WRN_W | SDIF_PROG;
ret = regmap_write(v_map, SDIF_W, val);
- if(ret < 0)
+ if (ret < 0)
return ret;
ret = regmap_read_poll_timeout(v_map, SDIF_STAT,
@@ -461,7 +461,7 @@ static int pvt_init(struct pvt_device *pvt)
IP_CTRL << SDIF_ADDR_SFT |
SDIF_WRN_W | SDIF_PROG;
ret = regmap_write(v_map, SDIF_W, val);
- if(ret < 0)
+ if (ret < 0)
return ret;
}
@@ -586,7 +586,7 @@ static int mr75203_probe(struct platform_device *pdev)
}
ret = regmap_read(pvt->c_map, PVT_IP_CONFIG, &val);
- if(ret < 0)
+ if (ret < 0)
return ret;
ts_num = (val & TS_NUM_MSK) >> TS_NUM_SFT;
--
2.37.1
Modify the equation and coefficients to convert the digital output to
temperature according to series 5 of the Moortec Embedded Temperature
Sensor (METS) datasheet:
T = G + H * (n / cal5 - 0.5) + J * F
The G, H and J coefficients are multiplied by 1000 to get the temperature
in milli-Celsius.
Signed-off-by: Eliav Farber <[email protected]>
---
drivers/hwmon/mr75203.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index e3191f590167..e500897585e4 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -102,9 +102,10 @@
#define PVT_POLL_DELAY_US 20
#define PVT_POLL_TIMEOUT_US 20000
-#define PVT_H_CONST 100000
-#define PVT_CAL5_CONST 2047
-#define PVT_G_CONST 40000
+#define PVT_H_CONST 60000
+#define PVT_G_CONST 200000
+#define PVT_J_CONST -100
+#define PVT_CAL5_CONST 4094
#define PVT_CONV_BITS 10
#define PVT_N_CONST 90
#define PVT_R_CONST 245805
@@ -158,7 +159,6 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
struct regmap *t_map = pvt->t_map;
u32 stat, nbs;
int ret;
- u64 tmp;
switch (attr) {
case hwmon_temp_input:
@@ -176,12 +176,13 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
nbs &= SAMPLE_DATA_MSK;
/*
- * Convert the register value to
- * degrees centigrade temperature
+ * Convert the register value to degrees centigrade temperature:
+ * T = G + H * (n / cal5 - 0.5) + J * F
*/
- tmp = nbs * PVT_H_CONST;
- do_div(tmp, PVT_CAL5_CONST);
- *val = tmp - PVT_G_CONST - pvt->ip_freq;
+ *val = PVT_G_CONST;
+ *val += PVT_H_CONST * nbs / PVT_CAL5_CONST;
+ *val -= PVT_H_CONST / 2;
+ *val += PVT_J_CONST * pvt->ip_freq / HZ_PER_MHZ;
return 0;
default:
@@ -313,7 +314,7 @@ static int pvt_init(struct pvt_device *pvt)
(key >> 1) << CLK_SYNTH_HI_SFT |
(key >> 1) << CLK_SYNTH_HOLD_SFT | CLK_SYNTH_EN;
- pvt->ip_freq = sys_freq * 100 / (key + 2);
+ pvt->ip_freq = clk_get_rate(pvt->clk) / (key + 2);
if (t_num) {
ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0);
--
2.37.1
vm-pre-scalar-ch# is a per channel optional parameter that can be
used to normalzie the voltage output results.
Signed-off-by: Eliav Farber <[email protected]>
---
.../devicetree/bindings/hwmon/moortec,mr75203.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index 6111b5069b3c..e2a55001eefc 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -56,6 +56,12 @@ properties:
A value of 0 means that the entire VM sensor is nou used.
$ref: /schemas/types.yaml#definitions/uint8-array
+ vm-pre-scalar-ch#:
+ description:
+ vm-active-channels defines the pre-scalar per channel value
+ used to normalzie the voltage output results.
+ $ref: /schemas/types.yaml#definitions/uint32
+
required:
- compatible
- reg
@@ -68,6 +74,7 @@ additionalProperties:
- intel,vm-map
- reset-control-skip
- vm-active-channels
+ - vm-pre-scalar-ch#
examples:
- |
@@ -82,5 +89,6 @@ examples:
clocks = <&osc0>;
resets = <&rcu0 0x40 7>;
vm-active-channels = [08 10 02];
+ vm-pre-scalar-ch5 = <2>;
#thermal-sensor-cells = <1>;
};
--
2.37.1
Adding a "reset-control-skip" bool property to the mr75203 node will
avoid looking up and obtaining a reference to a reset controller.
Signed-off-by: Eliav Farber <[email protected]>
---
drivers/hwmon/mr75203.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index f89f7bb5d698..bec63b611eb4 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/regmap.h>
@@ -525,14 +526,19 @@ static int mr75203_probe(struct platform_device *pdev)
return ret;
}
- pvt->rst = devm_reset_control_get_exclusive(dev, NULL);
- if (IS_ERR(pvt->rst))
- return dev_err_probe(dev, PTR_ERR(pvt->rst),
- "failed to get reset control\n");
+ if (of_property_read_bool(dev->of_node, "reset-control-skip")) {
+ dev_info(dev, "skipping reset-control\n");
+ } else {
+ pvt->rst = devm_reset_control_get_exclusive(dev, NULL);
+ if (IS_ERR(pvt->rst))
+ return dev_err_probe(dev, PTR_ERR(pvt->rst),
+ "failed to get reset control\n");
- ret = pvt_reset_control_deassert(dev, pvt);
- if (ret)
- return dev_err_probe(dev, ret, "cannot deassert reset control\n");
+ ret = pvt_reset_control_deassert(dev, pvt);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "cannot deassert reset control\n");
+ }
ret = regmap_read(pvt->c_map, PVT_IP_CONFIG, &val);
if(ret < 0)
--
2.37.1
On Wed, Aug 17, 2022 at 05:43:14AM +0000, Eliav Farber wrote:
> vm-pre-scalar-ch# is a per channel optional parameter that can be
> used to normalzie the voltage output results.
>
> Signed-off-by: Eliav Farber <[email protected]>
> ---
> .../devicetree/bindings/hwmon/moortec,mr75203.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> index 6111b5069b3c..e2a55001eefc 100644
> --- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> @@ -56,6 +56,12 @@ properties:
> A value of 0 means that the entire VM sensor is nou used.
> $ref: /schemas/types.yaml#definitions/uint8-array
>
> + vm-pre-scalar-ch#:
Is that how such properties are implemented ? Seems to me that
results in a lot of decode complexity.
Why not use an array property like the other properties ?
Guenter
> + description:
> + vm-active-channels defines the pre-scalar per channel value
> + used to normalzie the voltage output results.
> + $ref: /schemas/types.yaml#definitions/uint32
> +
> required:
> - compatible
> - reg
> @@ -68,6 +74,7 @@ additionalProperties:
> - intel,vm-map
> - reset-control-skip
> - vm-active-channels
> + - vm-pre-scalar-ch#
>
> examples:
> - |
> @@ -82,5 +89,6 @@ examples:
> clocks = <&osc0>;
> resets = <&rcu0 0x40 7>;
> vm-active-channels = [08 10 02];
> + vm-pre-scalar-ch5 = <2>;
> #thermal-sensor-cells = <1>;
> };
On Wed, Aug 17, 2022 at 05:43:17AM +0000, Eliav Farber wrote:
> Modify the equation and coefficients to convert the digital output to
> temperature according to series 5 of the Moortec Embedded Temperature
> Sensor (METS) datasheet:
> T = G + H * (n / cal5 - 0.5) + J * F
>
> The G, H and J coefficients are multiplied by 1000 to get the temperature
> in milli-Celsius.
>
This is, at the very least, confusing. It doesn't explain the discrepancy
to the old code nor the change in constant values. I have no idea if this
change would result in erroneous readings on some other system where
the existing calculation may be the correct one.
On top of that, it seems overflow-prune in 32 bit systems.
Guenter
> Signed-off-by: Eliav Farber <[email protected]>
> ---
> drivers/hwmon/mr75203.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
> index e3191f590167..e500897585e4 100644
> --- a/drivers/hwmon/mr75203.c
> +++ b/drivers/hwmon/mr75203.c
> @@ -102,9 +102,10 @@
>
> #define PVT_POLL_DELAY_US 20
> #define PVT_POLL_TIMEOUT_US 20000
> -#define PVT_H_CONST 100000
> -#define PVT_CAL5_CONST 2047
> -#define PVT_G_CONST 40000
> +#define PVT_H_CONST 60000
> +#define PVT_G_CONST 200000
> +#define PVT_J_CONST -100
> +#define PVT_CAL5_CONST 4094
> #define PVT_CONV_BITS 10
> #define PVT_N_CONST 90
> #define PVT_R_CONST 245805
> @@ -158,7 +159,6 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
> struct regmap *t_map = pvt->t_map;
> u32 stat, nbs;
> int ret;
> - u64 tmp;
>
> switch (attr) {
> case hwmon_temp_input:
> @@ -176,12 +176,13 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
> nbs &= SAMPLE_DATA_MSK;
>
> /*
> - * Convert the register value to
> - * degrees centigrade temperature
> + * Convert the register value to degrees centigrade temperature:
> + * T = G + H * (n / cal5 - 0.5) + J * F
> */
> - tmp = nbs * PVT_H_CONST;
> - do_div(tmp, PVT_CAL5_CONST);
> - *val = tmp - PVT_G_CONST - pvt->ip_freq;
> + *val = PVT_G_CONST;
> + *val += PVT_H_CONST * nbs / PVT_CAL5_CONST;
> + *val -= PVT_H_CONST / 2;
> + *val += PVT_J_CONST * pvt->ip_freq / HZ_PER_MHZ;
>
> return 0;
> default:
> @@ -313,7 +314,7 @@ static int pvt_init(struct pvt_device *pvt)
> (key >> 1) << CLK_SYNTH_HI_SFT |
> (key >> 1) << CLK_SYNTH_HOLD_SFT | CLK_SYNTH_EN;
>
> - pvt->ip_freq = sys_freq * 100 / (key + 2);
> + pvt->ip_freq = clk_get_rate(pvt->clk) / (key + 2);
>
> if (t_num) {
> ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0);
On 8/18/2022 11:11 PM, Guenter Roeck wrote:
> Is that how such properties are implemented ? Seems to me that
> results in a lot of decode complexity.
>
> Why not use an array property like the other properties ?
Each VM has up to 16 inputs and there might be more than one VM.
Assuming an example of 2 VMs, and channels 5 and 6 in first VM have pre-
scalar of 2, while channel 2 in the second VM has pre-scalar of 3 and
channel 11 has pre-scalar of 2, the alternative was to do something like
this:
vm-pre-scalar-0=[01 01 01 01 01 02 02 01 01 01 01 01 01 01 01 01];
vm-pre-scalar-1=[01 01 03 01 01 01 01 01 01 01 01 02 01 01 01 01];
Most of the inputs are 01, which are anyway the default.
I don't see a difference in decoding complexity between the different
approaches but if you prefer this I'll modify my patches.
On 8/18/2022 11:23 PM, Guenter Roeck wrote:
> On Wed, Aug 17, 2022 at 05:43:17AM +0000, Eliav Farber wrote:
>> Modify the equation and coefficients to convert the digital output to
>> temperature according to series 5 of the Moortec Embedded Temperature
>> Sensor (METS) datasheet:
>> T = G + H * (n / cal5 - 0.5) + J * F
>>
>> The G, H and J coefficients are multiplied by 1000 to get the
>> temperature
>> in milli-Celsius.
>>
>
> This is, at the very least, confusing. It doesn't explain the discrepancy
> to the old code nor the change in constant values. I have no idea if this
> change would result in erroneous readings on some other system where
> the existing calculation may be the correct one.
When I tested the driver it was also not clear to me why the equation
and coefficients in the code don't match the specifications in the data
sheet.
I reached out to Maxlinear engineers (@rtanwar) and they also couldn't
explain the discrepancy.
After further correspondence I aligned both the equation and coefficients
in the driver code to the equation and coefficients defined in series 5
of the Moortec Embedded Temperature Sensor (METS) datasheet which they
provided.
> On top of that, it seems overflow-prune in 32 bit systems.
I'll check if it can overflow, and if it can I'll fix in next version.
--
Regards, Eliav
On Fri, Aug 19, 2022 at 10:44:06AM +0300, Farber, Eliav wrote:
> On 8/18/2022 11:23 PM, Guenter Roeck wrote:
> > On Wed, Aug 17, 2022 at 05:43:17AM +0000, Eliav Farber wrote:
> > > Modify the equation and coefficients to convert the digital output to
> > > temperature according to series 5 of the Moortec Embedded Temperature
> > > Sensor (METS) datasheet:
> > > T = G + H * (n / cal5 - 0.5) + J * F
> > >
> > > The G, H and J coefficients are multiplied by 1000 to get the
> > > temperature
> > > in milli-Celsius.
> > >
> >
> > This is, at the very least, confusing. It doesn't explain the discrepancy
> > to the old code nor the change in constant values. I have no idea if this
> > change would result in erroneous readings on some other system where
> > the existing calculation may be the correct one.
>
> When I tested the driver it was also not clear to me why the equation
> and coefficients in the code don't match the specifications in the data
> sheet.
> I reached out to Maxlinear engineers (@rtanwar) and they also couldn't
> explain the discrepancy.
> After further correspondence I aligned both the equation and coefficients
> in the driver code to the equation and coefficients defined in series 5
> of the Moortec Embedded Temperature Sensor (METS) datasheet which they
> provided.
>
At least some of the discrepancy is because the original code is more
optimized and avoids overflow. Either case, the above needs to be explained
in the commit description.
> > On top of that, it seems overflow-prune in 32 bit systems.
>
> I'll check if it can overflow, and if it can I'll fix in next version.
>
> --
> Regards, Eliav
>