[PATCH v5 00/21] Variety of fixes and new features for mr75203 driver

[PATCH v5 00/21] Variety of fixes and new features for mr75203 driver

[Eliav Farber]

List of fixes:
 - Fix "intel,vm-map" property to be optional.
 - Fix VM sensor allocation when "intel,vm-map" not defined.
 - Fix multi-channel voltage reading.
 - Fix voltage equation for negative source input.
 - Modify the temperature equation according to series 5 datasheet.
 - Fix coding style issue.

List of new features:
 - Modify "reset" property to be optional.
 - Add optional "moortec,vm-active-channels" property to define the number
   of active channels per VM.
 - Add support for mr76006 pre-scaler to multiply the voltage result by 2.
 - Add support for series 6 temperature equation.
 - Add coefficient properties to fine tune the temperature equation.
 - Add debugfs to read and write temperature coefficients

---------

Changes between v4 and v5:
- 0004: Add detailed comment in code explaining the cast and the div instead of
  right shift or use of BIT().
- 0011: Fix typo in description (nou --> not).
- 0020: Return j coefficient to use debugfs_create_file() instead of
  debugfs_create_u32() because j is signed.
- 0021: Move the coding style patch to be last in the series (it does not fix
  the code), and remove the "Fixes:" tag.

Changes between v3 and v4:
*) Provide a Fixes tag for all fixes in the series.
*) Start series with fixes.
*) New patch to add description in moortec,mr75203.yaml.
*) New patch to add moortec to vendor-prefixes.
*) Fix moortec,mr75203.yaml checker errors.
*) Remove validation of device-tree parameters.
*) Fix per patch specific comments (detailed in each patch).

Changes between v2 and v3:
*) Add "moortec" prefix to all new device-tree properties.
*) Change order of patches.
*) Add explanations to better understand the changes.
*) Change "reset" property to be optional and remove the
  "reset-control-skip" property.
*) Split the patch for "fix multi-channel voltage reading" to two
   patches.
*) Change pre-scaler property format and fix typo (scalar --> scaler).
*) Fix voltage equation to support negative values instead of limiting
   value to zero.
*) Temperature equation - protect from overflow and add clamping.
*) Add new "moortec,ts-series" property to select between temperature
   equation of series 5 or series 6.

Changes between v1 and v2:
 *) Fix compilation error for patch 08/16:
    "warning: ISO C90 forbids variable length array"

---------

Eliav Farber (21):
  dt-bindings: hwmon: (mr75203) fix "intel,vm-map" property to be
    optional
  hwmon: (mr75203) fix VM sensor allocation when "intel,vm-map" not
    defined
  hwmon: (mr75203) update pvt->v_num and vm_num to the actual number of
    used sensors
  hwmon: (mr75203) fix voltage equation for negative source input
  hwmon: (mr75203) fix multi-channel voltage reading
  hwmon: (mr75203) enable polling for all VM channels
  dt-bindings: hwmon: (mr75203) add description for Moortec's PVT
    controller
  dt-bindings: hwmon: (mr75203) change "resets" property to be optional
  hwmon: (mr75203) skip reset-control deassert for SOCs that don't
    support it
  dt-bindings: vendor-prefixes: add vendor prefix for Moortec
  dt-bindings: hwmon: (mr75203) add "moortec,vm-active-channels"
    property
  hwmon: (mr75203) add VM active channel support
  dt-bindings: hwmon: (mr75203) add "moortec,vm-pre-scaler-x2" property
  hwmon: (mr75203) add VM pre-scaler x2 support
  hwmon: (mr75203) modify the temperature equation according to series 5
    datasheet
  dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property
  hwmon: (mr75203) add support for series 6 temperature equation
  dt-bindings: hwmon: (mr75203) add coefficient properties for the
    thermal equation
  hwmon: (mr75203) parse temperature coefficients from device-tree
  hwmon: (mr75203) add debugfs to read and write temperature
    coefficients
  hwmon: (mr75203) fix coding style space errors

 .../bindings/hwmon/moortec,mr75203.yaml       |  97 +++-
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/hwmon/mr75203.c                       | 433 +++++++++++++++---
 3 files changed, 466 insertions(+), 66 deletions(-)

-- 
2.37.1

[PATCH v5 01/21] dt-bindings: hwmon: (mr75203) fix "intel,vm-map" property to be optional

[Eliav Farber]

Change "intel,vm-map" property to be optional instead of required.

The driver implementation indicates it is not mandatory to have
"intel,vm-map" in the device tree:
 - probe doesn't fail in case it is absent.
 - explicit comment in code - "Incase intel,vm-map property is not
   defined, we assume incremental channel numbers".

Fixes: 748022ef093f ("hwmon: Add DT bindings schema for PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Acked-by: Rob Herring <robh@kernel.org>
---
V3 -> V2:
- Change this patch to be first in the series.
- Add explanation why "intel,vm-map" is not required.

 Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index b79f069a04c2..8ea97e774364 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -48,7 +48,6 @@ required:
   - compatible
   - reg
   - reg-names
-  - intel,vm-map
   - clocks
   - resets
   - "#thermal-sensor-cells"
-- 
2.37.1

Re: [PATCH v5 01/21] dt-bindings: hwmon: (mr75203) fix "intel,vm-map" property to be optional

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:29PM +0000, Eliav Farber wrote:
> Change "intel,vm-map" property to be optional instead of required.
> 
> The driver implementation indicates it is not mandatory to have
> "intel,vm-map" in the device tree:
>  - probe doesn't fail in case it is absent.
>  - explicit comment in code - "Incase intel,vm-map property is not
>    defined, we assume incremental channel numbers".
> 
> Fixes: 748022ef093f ("hwmon: Add DT bindings schema for PVT controller")
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Acked-by: Rob Herring <robh@kernel.org>

Applied, together with patches 2..6 of the series (all fixes).

Thanks,
Guenter

> ---
> V3 -> V2:
> - Change this patch to be first in the series.
> - Add explanation why "intel,vm-map" is not required.
> 
>  Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> index b79f069a04c2..8ea97e774364 100644
> --- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> @@ -48,7 +48,6 @@ required:
>    - compatible
>    - reg
>    - reg-names
> -  - intel,vm-map
>    - clocks
>    - resets
>    - "#thermal-sensor-cells"

[PATCH v5 02/21] hwmon: (mr75203) fix VM sensor allocation when "intel,vm-map" not defined

[Eliav Farber]

Bug - in case "intel,vm-map" is missing in device-tree ,'num' is set
to 0, and no voltage channel infos are allocated.

The reason num is set to 0 when "intel,vm-map" is missing is to set the
entire pvt->vm_idx[] with incremental channel numbers, but it didn't
take into consideration that same num is used later in devm_kcalloc().

If "intel,vm-map" does exist there is no need to set the unspecified
channels with incremental numbers, because the unspecified channels
can't be accessed in pvt_read_in() which is the only other place besides
the probe functions that uses pvt->vm_idx[].

This change fixes the bug by moving the incremental channel numbers
setting to be done only if "intel,vm-map" property is defined (starting
loop from 0), and removing 'num = 0'.

Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> v3:
- Simplify the fix by not removing the local num variable (it is removed as
  part of a later commit).

 drivers/hwmon/mr75203.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 046523d47c29..81ccb4c6fa5c 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -594,7 +594,12 @@ static int mr75203_probe(struct platform_device *pdev)
 		ret = device_property_read_u8_array(dev, "intel,vm-map",
 						    pvt->vm_idx, vm_num);
 		if (ret) {
-			num = 0;
+			/*
+			 * Incase intel,vm-map property is not defined, we
+			 * assume incremental channel numbers.
+			 */
+			for (i = 0; i < vm_num; i++)
+				pvt->vm_idx[i] = i;
 		} else {
 			for (i = 0; i < vm_num; i++)
 				if (pvt->vm_idx[i] >= vm_num ||
@@ -604,13 +609,6 @@ static int mr75203_probe(struct platform_device *pdev)
 				}
 		}
 
-		/*
-		 * Incase intel,vm-map property is not defined, we assume
-		 * incremental channel numbers.
-		 */
-		for (i = num; i < vm_num; i++)
-			pvt->vm_idx[i] = i;
-
 		in_config = devm_kcalloc(dev, num + 1,
 					 sizeof(*in_config), GFP_KERNEL);
 		if (!in_config)
-- 
2.37.1

[PATCH v5 03/21] hwmon: (mr75203) update pvt->v_num and vm_num to the actual number of used sensors

[Eliav Farber]

This issue is relevant when "intel,vm-map" is set in device-tree, and
defines a lower number of VMs than actually supported.

This change is needed for all places that use pvt->v_num or vm_num
later on in the code.

Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> v3:
- Update also vm_num to actual number of VMs.

 drivers/hwmon/mr75203.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 81ccb4c6fa5c..62df0c9498f3 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -605,6 +605,8 @@ static int mr75203_probe(struct platform_device *pdev)
 				if (pvt->vm_idx[i] >= vm_num ||
 				    pvt->vm_idx[i] == 0xff) {
 					num = i;
+					pvt->v_num = i;
+					vm_num = i;
 					break;
 				}
 		}
-- 
2.37.1

[PATCH v5 04/21] hwmon: (mr75203) fix voltage equation for negative source input

[Eliav Farber]

According to Moortec Embedded Voltage Monitor (MEVM) series 3 data
sheet, the minimum input signal is -100mv and maximum input signal
is +1000mv.

The equation used to convert the digital word to voltage uses mixed
types (*val signed and n unsigned), and on 64 bit machines also has
different size, since sizeof(u32) = 4 and sizeof(long) = 8.

So when measuring a negative input, n will be small enough, such that
PVT_N_CONST * n < PVT_R_CONST, and the result of
(PVT_N_CONST * n - PVT_R_CONST) will overflow to a very big positive
32 bit number. Then when storing the result in *val it will be the same
value just in 64 bit (instead of it representing a negative number which
will what happen when sizeof(long) = 4).

When -1023 <= (PVT_N_CONST * n - PVT_R_CONST) <= -1
dividing the number by 1024 should result of in 0, but because ">> 10"
is used, and the sign bit is used to fill the vacated bit positions, it
results in -1 (0xf...fffff) which is wrong.

This change fixes the sign problem and supports negative values by
casting n to long and replacing the shift right with div operation.

Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V5 -> V4:
- Add detailed comment in code explaining the cast and the div instead of right
  shift or use of BIT().

V4 -> V3:
- Remove unrelated change (add of empty line).

V3 -> V2:
- Fix equation to support negative values instead of limiting value to zero.

 drivers/hwmon/mr75203.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 62df0c9498f3..8211d463495d 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -201,8 +201,18 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 			return ret;
 
 		n &= SAMPLE_DATA_MSK;
-		/* Convert the N bitstream count into voltage */
-		*val = (PVT_N_CONST * n - PVT_R_CONST) >> PVT_CONV_BITS;
+		/*
+		 * Convert the N bitstream count into voltage.
+		 * To support negative voltage calculation for 64bit machines
+		 * n must be cast to long, since n and *val differ both in
+		 * signedness and in size.
+		 * Division is used instead of right shift, because for signed
+		 * numbers, the sign bit is used to fill the vacated bit
+		 * positions, and if the number is negative, 1 is used.
+		 * BIT(x) may not be used instead of (1 << x) because it's
+		 * unsigned.
+		 */
+		*val = (PVT_N_CONST * (long)n - PVT_R_CONST) / (1 << PVT_CONV_BITS);
 
 		return 0;
 	default:
-- 
2.37.1

[PATCH v5 05/21] hwmon: (mr75203) fix multi-channel voltage reading

[Eliav Farber]

Fix voltage allocation and reading to support all channels in all VMs.
Prior to this change allocation and reading were done only for the first
channel in each VM.
This change counts the total number of channels for allocation, and takes
into account the channel offset when reading the sample data register.

Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> V3:
- Keep lines sorted according to length.

V3 -> V2:
- Remove configuration of ip-polling register to a separate commit.
- Explain the fix.

 drivers/hwmon/mr75203.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 8211d463495d..87a14713e124 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -68,8 +68,9 @@
 
 /* VM Individual Macro Register */
 #define VM_COM_REG_SIZE	0x200
-#define VM_SDIF_DONE(n)	(VM_COM_REG_SIZE + 0x34 + 0x200 * (n))
-#define VM_SDIF_DATA(n)	(VM_COM_REG_SIZE + 0x40 + 0x200 * (n))
+#define VM_SDIF_DONE(vm)	(VM_COM_REG_SIZE + 0x34 + 0x200 * (vm))
+#define VM_SDIF_DATA(vm, ch)	\
+	(VM_COM_REG_SIZE + 0x40 + 0x200 * (vm) + 0x4 * (ch))
 
 /* SDA Slave Register */
 #define IP_CTRL			0x00
@@ -115,6 +116,7 @@ struct pvt_device {
 	u32			t_num;
 	u32			p_num;
 	u32			v_num;
+	u32			c_num;
 	u32			ip_freq;
 	u8			*vm_idx;
 };
@@ -178,14 +180,15 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct pvt_device *pvt = dev_get_drvdata(dev);
 	struct regmap *v_map = pvt->v_map;
+	u8 vm_idx, ch_idx;
 	u32 n, stat;
-	u8 vm_idx;
 	int ret;
 
-	if (channel >= pvt->v_num)
+	if (channel >= pvt->v_num * pvt->c_num)
 		return -EINVAL;
 
-	vm_idx = pvt->vm_idx[channel];
+	vm_idx = pvt->vm_idx[channel / pvt->c_num];
+	ch_idx = channel % pvt->c_num;
 
 	switch (attr) {
 	case hwmon_in_input:
@@ -196,7 +199,7 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 		if (ret)
 			return ret;
 
-		ret = regmap_read(v_map, VM_SDIF_DATA(vm_idx), &n);
+		ret = regmap_read(v_map, VM_SDIF_DATA(vm_idx, ch_idx), &n);
 		if(ret < 0)
 			return ret;
 
@@ -509,8 +512,8 @@ static int pvt_reset_control_deassert(struct device *dev, struct pvt_device *pvt
 
 static int mr75203_probe(struct platform_device *pdev)
 {
+	u32 ts_num, vm_num, pd_num, ch_num, val, index, i;
 	const struct hwmon_channel_info **pvt_info;
-	u32 ts_num, vm_num, pd_num, val, index, i;
 	struct device *dev = &pdev->dev;
 	u32 *temp_config, *in_config;
 	struct device *hwmon_dev;
@@ -551,9 +554,11 @@ static int mr75203_probe(struct platform_device *pdev)
 	ts_num = (val & TS_NUM_MSK) >> TS_NUM_SFT;
 	pd_num = (val & PD_NUM_MSK) >> PD_NUM_SFT;
 	vm_num = (val & VM_NUM_MSK) >> VM_NUM_SFT;
+	ch_num = (val & CH_NUM_MSK) >> CH_NUM_SFT;
 	pvt->t_num = ts_num;
 	pvt->p_num = pd_num;
 	pvt->v_num = vm_num;
+	pvt->c_num = ch_num;
 	val = 0;
 	if (ts_num)
 		val++;
@@ -590,7 +595,7 @@ static int mr75203_probe(struct platform_device *pdev)
 	}
 
 	if (vm_num) {
-		u32 num = vm_num;
+		u32 total_ch;
 
 		ret = pvt_get_regmap(pdev, "vm", pvt);
 		if (ret)
@@ -614,20 +619,20 @@ static int mr75203_probe(struct platform_device *pdev)
 			for (i = 0; i < vm_num; i++)
 				if (pvt->vm_idx[i] >= vm_num ||
 				    pvt->vm_idx[i] == 0xff) {
-					num = i;
 					pvt->v_num = i;
 					vm_num = i;
 					break;
 				}
 		}
 
-		in_config = devm_kcalloc(dev, num + 1,
+		total_ch = ch_num * vm_num;
+		in_config = devm_kcalloc(dev, total_ch + 1,
 					 sizeof(*in_config), GFP_KERNEL);
 		if (!in_config)
 			return -ENOMEM;
 
-		memset32(in_config, HWMON_I_INPUT, num);
-		in_config[num] = 0;
+		memset32(in_config, HWMON_I_INPUT, total_ch);
+		in_config[total_ch] = 0;
 		pvt_in.config = in_config;
 
 		pvt_info[index++] = &pvt_in;
-- 
2.37.1

[PATCH v5 06/21] hwmon: (mr75203) enable polling for all VM channels

[Eliav Farber]

Configure ip-polling register to enable polling for all voltage monitor
channels.
This enables reading the voltage values for all inputs other than just
input 0.

Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V3 -> V2:
- Move configuration of ip-polling register from previous patch to a
  separate commit.

V4 -> V3:
- Replace GENMASK(pvt->c_num - 1, 0)  with (BIT(pvt->c_num) - 1).

 drivers/hwmon/mr75203.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 87a14713e124..41e3d3b54baf 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -398,6 +398,19 @@ static int pvt_init(struct pvt_device *pvt)
 		if (ret)
 			return ret;
 
+		val = (BIT(pvt->c_num) - 1) | VM_CH_INIT |
+		      IP_POLL << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG;
+		ret = regmap_write(v_map, SDIF_W, val);
+		if (ret < 0)
+			return ret;
+
+		ret = regmap_read_poll_timeout(v_map, SDIF_STAT,
+					       val, !(val & SDIF_BUSY),
+					       PVT_POLL_DELAY_US,
+					       PVT_POLL_TIMEOUT_US);
+		if (ret)
+			return ret;
+
 		val = CFG1_VOL_MEAS_MODE | CFG1_PARALLEL_OUT |
 		      CFG1_14_BIT | IP_CFG << SDIF_ADDR_SFT |
 		      SDIF_WRN_W | SDIF_PROG;
-- 
2.37.1

[PATCH v5 07/21] dt-bindings: hwmon: (mr75203) add description for Moortec's PVT controller

[Eliav Farber]

This changes adds a detailed description for the mr75203 controller and
for some of the analog IPs controlled by it.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V4 -> V3:
- New patch to add description.

 .../bindings/hwmon/moortec,mr75203.yaml       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index 8ea97e774364..5e41aed5891a 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -9,6 +9,32 @@ title: Moortec Semiconductor MR75203 PVT Controller bindings
 maintainers:
   - Rahul Tanwar <rtanwar@maxlinear.com>
 
+description: |
+  A Moortec PVT (Process, Voltage, Temperature) monitoring logic design can
+  include many different units.
+  Such a design will usually consists of several Moortec's embedded analog IPs,
+  and a single Moortec controller (mr75203) to configure and control the IPs.
+
+  Some of the Moortec's analog hard IPs that can be used in a design:
+  *) Temperature Sensor (TS) - used to monitor core temperature (e.g. mr74137).
+  *) Voltage Monitor (VM) - used to monitor voltage levels (e.g. mr74138).
+  *) Process Detector (PD) - used to assess silicon speed (e.g. mr74139).
+  *) Delay Chain - ring oscillator connected to the PD, used to measure IO
+     based transistors (e.g. mr76008 ring oscillator at 1.1V, mr76007 ring
+     oscillator at 1.8V).
+  *) Pre Scaler - provides divide-by-X scaling of input voltage, which can then
+     be presented for VM for measurement within its range (e.g. mr76006 -
+     divide by 2 pre-scaler).
+
+  TS, VM & PD also include a digital interface, which consists of configuration
+  inputs and measurement outputs.
+
+  Some of the units have number of series, each series can have slightly
+  different characteristics.
+
+  The mr75203 binding describes configuration for the controller unit, but also
+  for some of the analog IPs.
+
 properties:
   compatible:
     const: moortec,mr75203
-- 
2.37.1

Re: [PATCH v5 07/21] dt-bindings: hwmon: (mr75203) add description for Moortec's PVT controller

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:35PM +0000, Eliav Farber wrote:
> This changes adds a detailed description for the mr75203 controller and
> for some of the analog IPs controlled by it.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>

Applied to hwmon-next, with Rob's Acked-by: from v4.

Guenter

[PATCH v5 08/21] dt-bindings: hwmon: (mr75203) change "resets" property to be optional

[Eliav Farber]

Change "resets" property to be optional instead of required, for SOCs
that don't support a reset controller.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V4 -> V3:
- Fix "reset" to "resets".

V3 -> v2:
- Change "reset" property to be optional instead of adding new
  "reset-control-skip" property.

 Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index 5e41aed5891a..9454576ebb73 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -75,7 +75,6 @@ required:
   - reg
   - reg-names
   - clocks
-  - resets
   - "#thermal-sensor-cells"
 
 additionalProperties: false
-- 
2.37.1

Re: [PATCH v5 08/21] dt-bindings: hwmon: (mr75203) change "resets" property to be optional

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:36PM +0000, Eliav Farber wrote:
> Change "resets" property to be optional instead of required, for SOCs
> that don't support a reset controller.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>

Applied, with Rob's Acked-by: from v4 added.

Guenter

[PATCH v5 09/21] hwmon: (mr75203) skip reset-control deassert for SOCs that don't support it

[Eliav Farber]

Don't fail the probe function and don't deassert the reset controller if
a "reset" property doesn't exist in the device tree.

Change is done for SOCs that don't support a reset controller.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V3 -> v2:
- Change "reset" property to be optional instead of skipping it.

 drivers/hwmon/mr75203.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 41e3d3b54baf..19ec9fb91202 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -551,14 +551,17 @@ static int mr75203_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	pvt->rst = devm_reset_control_get_exclusive(dev, NULL);
+	pvt->rst = devm_reset_control_get_optional_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");
+	if (pvt->rst) {
+		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

Re: [PATCH v5 09/21] hwmon: (mr75203) skip reset-control deassert for SOCs that don't support it

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:37PM +0000, Eliav Farber wrote:
> Don't fail the probe function and don't deassert the reset controller if
> a "reset" property doesn't exist in the device tree.
> 
> Change is done for SOCs that don't support a reset controller.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> V3 -> v2:
> - Change "reset" property to be optional instead of skipping it.
> 
>  drivers/hwmon/mr75203.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
> index 41e3d3b54baf..19ec9fb91202 100644
> --- a/drivers/hwmon/mr75203.c
> +++ b/drivers/hwmon/mr75203.c
> @@ -551,14 +551,17 @@ static int mr75203_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	pvt->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	pvt->rst = devm_reset_control_get_optional_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");
> +	if (pvt->rst) {
> +		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)

[PATCH v5 10/21] dt-bindings: vendor-prefixes: add vendor prefix for Moortec

[Eliav Farber]

Add device-tree vendor prefix for Moortec Semiconductor Ltd.
Website: https://moortec.com/

Moortec were acquired by Synopsys so link above leads to:
https://www.synopsys.com/solutions/silicon-lifecycle-management.html

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V4 -> V3:
- New patch to add moortec to vendor-prefixes.

 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2f0151e9f6be..9bf7c2cd7e89 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -819,6 +819,8 @@ patternProperties:
     description: MNT Research GmbH
   "^modtronix,.*":
     description: Modtronix Engineering
+  "^moortec,.*":
+    description: Moortec Semiconductor Ltd.
   "^mosaixtech,.*":
     description: Mosaix Technologies, Inc.
   "^motorola,.*":
-- 
2.37.1

Re: [PATCH v5 10/21] dt-bindings: vendor-prefixes: add vendor prefix for Moortec

[Rob Herring]

On Thu, 08 Sep 2022 15:24:38 +0000, Eliav Farber wrote:
> Add device-tree vendor prefix for Moortec Semiconductor Ltd.
> Website: https://moortec.com/
> 
> Moortec were acquired by Synopsys so link above leads to:
> https://www.synopsys.com/solutions/silicon-lifecycle-management.html
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> V4 -> V3:
> - New patch to add moortec to vendor-prefixes.
> 
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 10/21] dt-bindings: vendor-prefixes: add vendor prefix for Moortec

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:38PM +0000, Eliav Farber wrote:
> Add device-tree vendor prefix for Moortec Semiconductor Ltd.
> Website: https://moortec.com/
> 
> Moortec were acquired by Synopsys so link above leads to:
> https://www.synopsys.com/solutions/silicon-lifecycle-management.html
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Acked-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 11/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-active-channels" property

[Eliav Farber]

Add optional "moortec,vm-active-channels" property to define the number
of active channels per VM.

This shall be useful to avoid exposing sysfs for reading inputs that are
not connected to any voltage source.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V5 -> V4:
- Fix typo in description (nou --> not).

V4 -> V3:
- Fix DT checker errors.

V3 -> V2:
- Add "moortec" prefix to property name.
- Add explanation why this change is needed.

 .../devicetree/bindings/hwmon/moortec,mr75203.yaml     | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index 9454576ebb73..5d4bf0edea5c 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -70,6 +70,15 @@ properties:
   "#thermal-sensor-cells":
     const: 1
 
+  moortec,vm-active-channels:
+    description:
+      Defines the number of channels per VM that are actually used and are
+      connected to some input source.
+      Maximum number of items - number of VMs.
+      Maximum value of each item - number of channels.
+      Minimum value of each item - 0 (which means entire VM sensor is not used).
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+
 required:
   - compatible
   - reg
@@ -91,5 +100,6 @@ examples:
         intel,vm-map = [03 01 04 ff ff];
         clocks = <&osc0>;
         resets = <&rcu0 0x40 7>;
+        moortec,vm-active-channels = /bits/ 8 <0x10 0x05>;
         #thermal-sensor-cells = <1>;
     };
-- 
2.37.1

Re: [PATCH v5 11/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-active-channels" property

[Rob Herring]

On Thu, 08 Sep 2022 15:24:39 +0000, Eliav Farber wrote:
> Add optional "moortec,vm-active-channels" property to define the number
> of active channels per VM.
> 
> This shall be useful to avoid exposing sysfs for reading inputs that are
> not connected to any voltage source.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> V5 -> V4:
> - Fix typo in description (nou --> not).
> 
> V4 -> V3:
> - Fix DT checker errors.
> 
> V3 -> V2:
> - Add "moortec" prefix to property name.
> - Add explanation why this change is needed.
> 
>  .../devicetree/bindings/hwmon/moortec,mr75203.yaml     | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 11/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-active-channels" property

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:39PM +0000, Eliav Farber wrote:
> Add optional "moortec,vm-active-channels" property to define the number
> of active channels per VM.
> 
> This shall be useful to avoid exposing sysfs for reading inputs that are
> not connected to any voltage source.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 12/21] hwmon: (mr75203) add VM active channel support

[Eliav Farber]

Add active channel support per voltage monitor.
The number of active channels is read from the device-tree.
When absent in device-tree, all channels are assumed to be used.

This shall be useful to expose sysfs only for inputs that are connected
to a voltage source.

Setting number of active channels to 0, means that entire VM sensor is
not used.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> V3:
- Convert comments to kernel doc.
- Repalce for loop with memset.
- Add {} to outer for loop.

V3 -> V2:
- Refactor the code changes (move code to a new function and group
  parameters in dedicated structure).

V2 -> V1:
- Fix compilation error for patch 08/16:
  "warning: ISO C90 forbids variable length array"

 drivers/hwmon/mr75203.c | 121 ++++++++++++++++++++++++++++++++--------
 1 file changed, 99 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 19ec9fb91202..56e19c430919 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -29,6 +29,8 @@
 #define CH_NUM_MSK	GENMASK(31, 24)
 #define CH_NUM_SFT	24
 
+#define VM_NUM_MAX	(VM_NUM_MSK >> VM_NUM_SFT)
+
 /* Macro Common Register */
 #define CLK_SYNTH		0x00
 #define CLK_SYNTH_LO_SFT	0
@@ -106,6 +108,31 @@
 #define PVT_N_CONST		90
 #define PVT_R_CONST		245805
 
+/**
+ * struct voltage_device - VM single input parameters.
+ * @vm_map: Map channel number to VM index.
+ * @ch_map: Map channel number to channel index.
+ *
+ * The structure provides mapping between channel-number (0..N-1) to VM-index
+ * (0..num_vm-1) and channel-index (0..ch_num-1) where N = num_vm * ch_num.
+ */
+struct voltage_device {
+	u32 vm_map;
+	u32 ch_map;
+};
+
+/**
+ * struct voltage_channels - VM channel count.
+ * @total: Total number of channels in all VMs.
+ * @max: Maximum number of channels among all VMs.
+ *
+ * The structure provides channel count information across all VMs.
+ */
+struct voltage_channels {
+	u32 total;
+	u8 max;
+};
+
 struct pvt_device {
 	struct regmap		*c_map;
 	struct regmap		*t_map;
@@ -113,12 +140,12 @@ struct pvt_device {
 	struct regmap		*v_map;
 	struct clk		*clk;
 	struct reset_control	*rst;
+	struct voltage_device	*vd;
+	struct voltage_channels	vm_channels;
 	u32			t_num;
 	u32			p_num;
 	u32			v_num;
-	u32			c_num;
 	u32			ip_freq;
-	u8			*vm_idx;
 };
 
 static umode_t pvt_is_visible(const void *data, enum hwmon_sensor_types type,
@@ -184,11 +211,11 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 	u32 n, stat;
 	int ret;
 
-	if (channel >= pvt->v_num * pvt->c_num)
+	if (channel >= pvt->vm_channels.total)
 		return -EINVAL;
 
-	vm_idx = pvt->vm_idx[channel / pvt->c_num];
-	ch_idx = channel % pvt->c_num;
+	vm_idx = pvt->vd[channel].vm_map;
+	ch_idx = pvt->vd[channel].ch_map;
 
 	switch (attr) {
 	case hwmon_in_input:
@@ -398,7 +425,7 @@ static int pvt_init(struct pvt_device *pvt)
 		if (ret)
 			return ret;
 
-		val = (BIT(pvt->c_num) - 1) | VM_CH_INIT |
+		val = (BIT(pvt->vm_channels.max) - 1) | VM_CH_INIT |
 		      IP_POLL << SDIF_ADDR_SFT | SDIF_WRN_W | SDIF_PROG;
 		ret = regmap_write(v_map, SDIF_W, val);
 		if (ret < 0)
@@ -523,6 +550,60 @@ static int pvt_reset_control_deassert(struct device *dev, struct pvt_device *pvt
 	return devm_add_action_or_reset(dev, pvt_reset_control_assert, pvt);
 }
 
+static int pvt_get_active_channel(struct device *dev, struct pvt_device *pvt,
+				  u32 vm_num, u32 ch_num, u8 *vm_idx)
+{
+	u8 vm_active_ch[VM_NUM_MAX];
+	int ret, i, j, k;
+
+	ret = device_property_read_u8_array(dev, "moortec,vm-active-channels",
+					    vm_active_ch, vm_num);
+	if (ret) {
+		/*
+		 * Incase "moortec,vm-active-channels" property is not defined,
+		 * we assume each VM sensor has all of its channels active.
+		 */
+		memset(vm_active_ch, ch_num, vm_num);
+		pvt->vm_channels.max = ch_num;
+		pvt->vm_channels.total = ch_num * vm_num;
+	} else {
+		for (i = 0; i < vm_num; i++) {
+			if (vm_active_ch[i] > ch_num) {
+				dev_err(dev, "invalid active channels: %u\n",
+					vm_active_ch[i]);
+				return -EINVAL;
+			}
+
+			pvt->vm_channels.total += vm_active_ch[i];
+
+			if (vm_active_ch[i] > pvt->vm_channels.max)
+				pvt->vm_channels.max = vm_active_ch[i];
+		}
+	}
+
+	/*
+	 * Map between the channel-number to VM-index and channel-index.
+	 * Example - 3 VMs, "moortec,vm_active_ch" = <5 2 4>:
+	 * vm_map = [0 0 0 0 0 1 1 2 2 2 2]
+	 * ch_map = [0 1 2 3 4 0 1 0 1 2 3]
+	 */
+	pvt->vd = devm_kcalloc(dev, pvt->vm_channels.total, sizeof(*pvt->vd),
+			       GFP_KERNEL);
+	if (!pvt->vd)
+		return -ENOMEM;
+
+	k = 0;
+	for (i = 0; i < vm_num; i++) {
+		for (j = 0; j < vm_active_ch[i]; j++) {
+			pvt->vd[k].vm_map = vm_idx[i];
+			pvt->vd[k].ch_map = j;
+			k++;
+		}
+	}
+
+	return 0;
+}
+
 static int mr75203_probe(struct platform_device *pdev)
 {
 	u32 ts_num, vm_num, pd_num, ch_num, val, index, i;
@@ -574,7 +655,6 @@ static int mr75203_probe(struct platform_device *pdev)
 	pvt->t_num = ts_num;
 	pvt->p_num = pd_num;
 	pvt->v_num = vm_num;
-	pvt->c_num = ch_num;
 	val = 0;
 	if (ts_num)
 		val++;
@@ -611,44 +691,41 @@ static int mr75203_probe(struct platform_device *pdev)
 	}
 
 	if (vm_num) {
-		u32 total_ch;
+		u8 vm_idx[VM_NUM_MAX];
 
 		ret = pvt_get_regmap(pdev, "vm", pvt);
 		if (ret)
 			return ret;
 
-		pvt->vm_idx = devm_kcalloc(dev, vm_num, sizeof(*pvt->vm_idx),
-					   GFP_KERNEL);
-		if (!pvt->vm_idx)
-			return -ENOMEM;
-
-		ret = device_property_read_u8_array(dev, "intel,vm-map",
-						    pvt->vm_idx, vm_num);
+		ret = device_property_read_u8_array(dev, "intel,vm-map", vm_idx,
+						    vm_num);
 		if (ret) {
 			/*
 			 * Incase intel,vm-map property is not defined, we
 			 * assume incremental channel numbers.
 			 */
 			for (i = 0; i < vm_num; i++)
-				pvt->vm_idx[i] = i;
+				vm_idx[i] = i;
 		} else {
 			for (i = 0; i < vm_num; i++)
-				if (pvt->vm_idx[i] >= vm_num ||
-				    pvt->vm_idx[i] == 0xff) {
+				if (vm_idx[i] >= vm_num || vm_idx[i] == 0xff) {
 					pvt->v_num = i;
 					vm_num = i;
 					break;
 				}
 		}
 
-		total_ch = ch_num * vm_num;
-		in_config = devm_kcalloc(dev, total_ch + 1,
+		ret = pvt_get_active_channel(dev, pvt, vm_num, ch_num, vm_idx);
+		if (ret)
+			return ret;
+
+		in_config = devm_kcalloc(dev, pvt->vm_channels.total + 1,
 					 sizeof(*in_config), GFP_KERNEL);
 		if (!in_config)
 			return -ENOMEM;
 
-		memset32(in_config, HWMON_I_INPUT, total_ch);
-		in_config[total_ch] = 0;
+		memset32(in_config, HWMON_I_INPUT, pvt->vm_channels.total);
+		in_config[pvt->vm_channels.total] = 0;
 		pvt_in.config = in_config;
 
 		pvt_info[index++] = &pvt_in;
-- 
2.37.1

Re: [PATCH v5 12/21] hwmon: (mr75203) add VM active channel support

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:40PM +0000, Eliav Farber wrote:
> Add active channel support per voltage monitor.
> The number of active channels is read from the device-tree.
> When absent in device-tree, all channels are assumed to be used.
> 
> This shall be useful to expose sysfs only for inputs that are connected
> to a voltage source.
> 
> Setting number of active channels to 0, means that entire VM sensor is
> not used.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 13/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-pre-scaler-x2" property

[Eliav Farber]

Add support for mr76006 pre-scaler which provides divide-by-2 scaling of
the input voltage, so that it can be  presented to the VM for measurement
within its range (the VM input range is limited to -0.1V to 1V).

The new "moortec,vm-pre-scaler-x2" property lists the channels that use
the mr76006 pre-scaler.

The driver will use this list to multiply the voltage result by 2, to
present to the user with the actual voltage input source.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V4 -> V3:
- Rename "moortec,vm-pre-scaler" to "moortec,vm-pre-scaler-x2".
- Added mximum number if items in description.

V3 -> V2:
- Add "moortec" prefix to property name.
- Change property format to be a single u8 array.
- Fix typo: scalar --> scaler.

 .../devicetree/bindings/hwmon/moortec,mr75203.yaml  | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index 5d4bf0edea5c..d6b03a6d7043 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -79,6 +79,18 @@ properties:
       Minimum value of each item - 0 (which means entire VM sensor is not used).
     $ref: /schemas/types.yaml#/definitions/uint8-array
 
+  moortec,vm-pre-scaler-x2:
+    description:
+      Defines the channels that use a mr76006 pre-scaler to divide the input
+      source by 2.
+      The pre-scaler is used for input sources that exceed the VM input range.
+      The driver uses this information to present to the user with the actual
+      value of the voltage source.
+      For channels that are not listed, no pre-scaler is assumed.
+      Maximum number of items - total number of channels in all VMs.
+      Each channel should not appear more than once.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+
 required:
   - compatible
   - reg
@@ -101,5 +113,6 @@ examples:
         clocks = <&osc0>;
         resets = <&rcu0 0x40 7>;
         moortec,vm-active-channels = /bits/ 8 <0x10 0x05>;
+        moortec,vm-pre-scaler-x2 = /bits/ 8 <5 6 20>;
         #thermal-sensor-cells = <1>;
     };
-- 
2.37.1

Re: [PATCH v5 13/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-pre-scaler-x2" property

[Rob Herring]

On Thu, 08 Sep 2022 15:24:41 +0000, Eliav Farber wrote:
> Add support for mr76006 pre-scaler which provides divide-by-2 scaling of
> the input voltage, so that it can be  presented to the VM for measurement
> within its range (the VM input range is limited to -0.1V to 1V).
> 
> The new "moortec,vm-pre-scaler-x2" property lists the channels that use
> the mr76006 pre-scaler.
> 
> The driver will use this list to multiply the voltage result by 2, to
> present to the user with the actual voltage input source.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> V4 -> V3:
> - Rename "moortec,vm-pre-scaler" to "moortec,vm-pre-scaler-x2".
> - Added mximum number if items in description.
> 
> V3 -> V2:
> - Add "moortec" prefix to property name.
> - Change property format to be a single u8 array.
> - Fix typo: scalar --> scaler.
> 
>  .../devicetree/bindings/hwmon/moortec,mr75203.yaml  | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 13/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-pre-scaler-x2" property

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:41PM +0000, Eliav Farber wrote:
> Add support for mr76006 pre-scaler which provides divide-by-2 scaling of
> the input voltage, so that it can be  presented to the VM for measurement
> within its range (the VM input range is limited to -0.1V to 1V).
> 
> The new "moortec,vm-pre-scaler-x2" property lists the channels that use
> the mr76006 pre-scaler.
> 
> The driver will use this list to multiply the voltage result by 2, to
> present to the user with the actual voltage input source.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 14/21] hwmon: (mr75203) add VM pre-scaler x2 support

[Eliav Farber]

Add support for mr76006 pre-scaler which provides divide-by-2 scaling
of the input voltage, so that it can be  presented to the VM for
measurement within its range (the VM input range is limited from -0.1V
to 1V).

The driver reads from the device-tree all the channels that use the
mr76006 pre-scaler and multiplies the voltage result by a factor of 2,
to represent to the user with the actual voltage input source.

Channels that are not in the device-tree are multiplied by a factor
of 1.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> V3:
- Replace of_property_count_u8_elems() with device_property_count_u8().
- Remove unnecessary blank line.
- Remove code that validated the YAML.

V3 -> V2:
- Modify code according to new property format.

 drivers/hwmon/mr75203.c | 55 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 56e19c430919..8a6ae72fed20 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -17,6 +17,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
+#include <linux/slab.h>
 
 /* PVT Common register */
 #define PVT_IP_CONFIG	0x04
@@ -108,17 +109,24 @@
 #define PVT_N_CONST		90
 #define PVT_R_CONST		245805
 
+#define PRE_SCALER_X1	1
+#define PRE_SCALER_X2	2
+
 /**
  * struct voltage_device - VM single input parameters.
  * @vm_map: Map channel number to VM index.
  * @ch_map: Map channel number to channel index.
+ * @pre_scaler: Pre scaler value (1 or 2) used to normalize the voltage output
+ *              result.
  *
  * The structure provides mapping between channel-number (0..N-1) to VM-index
  * (0..num_vm-1) and channel-index (0..ch_num-1) where N = num_vm * ch_num.
+ * It also provides normalization factor for the VM equation.
  */
 struct voltage_device {
 	u32 vm_map;
 	u32 ch_map;
+	u32 pre_scaler;
 };
 
 /**
@@ -207,8 +215,8 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct pvt_device *pvt = dev_get_drvdata(dev);
 	struct regmap *v_map = pvt->v_map;
+	u32 n, stat, pre_scaler;
 	u8 vm_idx, ch_idx;
-	u32 n, stat;
 	int ret;
 
 	if (channel >= pvt->vm_channels.total)
@@ -231,6 +239,7 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 			return ret;
 
 		n &= SAMPLE_DATA_MSK;
+		pre_scaler = pvt->vd[channel].pre_scaler;
 		/*
 		 * Convert the N bitstream count into voltage.
 		 * To support negative voltage calculation for 64bit machines
@@ -242,7 +251,8 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 		 * BIT(x) may not be used instead of (1 << x) because it's
 		 * unsigned.
 		 */
-		*val = (PVT_N_CONST * (long)n - PVT_R_CONST) / (1 << PVT_CONV_BITS);
+		*val = pre_scaler * (PVT_N_CONST * (long)n - PVT_R_CONST) /
+			(1 << PVT_CONV_BITS);
 
 		return 0;
 	default:
@@ -604,6 +614,43 @@ static int pvt_get_active_channel(struct device *dev, struct pvt_device *pvt,
 	return 0;
 }
 
+static int pvt_get_pre_scaler(struct device *dev, struct pvt_device *pvt)
+{
+	u8 *pre_scaler_ch_list;
+	int i, ret, num_ch;
+	u32 channel;
+
+	/* Set default pre-scaler value to be 1. */
+	for (i = 0; i < pvt->vm_channels.total; i++)
+		pvt->vd[i].pre_scaler = PRE_SCALER_X1;
+
+	/* Get number of channels configured in "moortec,vm-pre-scaler-x2". */
+	num_ch = device_property_count_u8(dev, "moortec,vm-pre-scaler-x2");
+	if (num_ch <= 0)
+		return 0;
+
+	pre_scaler_ch_list = kcalloc(num_ch, sizeof(*pre_scaler_ch_list),
+				     GFP_KERNEL);
+	if (!pre_scaler_ch_list)
+		return -ENOMEM;
+
+	/* Get list of all channels that have pre-scaler of 2. */
+	ret = device_property_read_u8_array(dev, "moortec,vm-pre-scaler-x2",
+					    pre_scaler_ch_list, num_ch);
+	if (ret)
+		goto out;
+
+	for (i = 0; i < num_ch; i++) {
+		channel = pre_scaler_ch_list[i];
+		pvt->vd[channel].pre_scaler = PRE_SCALER_X2;
+	}
+
+out:
+	kfree(pre_scaler_ch_list);
+
+	return ret;
+}
+
 static int mr75203_probe(struct platform_device *pdev)
 {
 	u32 ts_num, vm_num, pd_num, ch_num, val, index, i;
@@ -719,6 +766,10 @@ static int mr75203_probe(struct platform_device *pdev)
 		if (ret)
 			return ret;
 
+		ret = pvt_get_pre_scaler(dev, pvt);
+		if (ret)
+			return ret;
+
 		in_config = devm_kcalloc(dev, pvt->vm_channels.total + 1,
 					 sizeof(*in_config), GFP_KERNEL);
 		if (!in_config)
-- 
2.37.1

Re: [PATCH v5 14/21] hwmon: (mr75203) add VM pre-scaler x2 support

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:42PM +0000, Eliav Farber wrote:
> Add support for mr76006 pre-scaler which provides divide-by-2 scaling
> of the input voltage, so that it can be  presented to the VM for
> measurement within its range (the VM input range is limited from -0.1V
> to 1V).
> 
> The driver reads from the device-tree all the channels that use the
> mr76006 pre-scaler and multiplies the voltage result by a factor of 2,
> to represent to the user with the actual voltage input source.
> 
> Channels that are not in the device-tree are multiplied by a factor
> of 1.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied, after resolving a minor conflict against commit d59eacaac953
("hwmon/drivers/mr75203: use HZ macros") from September 2021.
In the future, please base your patch series on the latest upstream
kernel version.

Thanks,
Guenter

[PATCH v5 15/21] hwmon: (mr75203) modify the temperature equation according to series 5 datasheet

[Eliav Farber]

Modify the equation and coefficients used 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

Where:
*) G = 60, H = 200, cal5 = 4094, J = -0.1.
*) F = frequency clock in MHz.
*) n is the digital output.

In code, the G, H and J coefficients are multiplied by a factor of 1000
to get the temperature in milli-Celsius.
Final result is clamped in case it exceeds min/max thresholds.

Change is done since it is unclear where the current equation and
coefficients came from.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> V3:
- Change 'not clear' to 'unclear' in commit message.
- Add _mC prefix to temperature mix/max defines.
- Add SERIES5 to coefficient defines.

V3 -> V2:
- Protect from overflow.
- Add temperature clamping.
- Add better documentation.

 drivers/hwmon/mr75203.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 8a6ae72fed20..0de64642cc42 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -102,13 +102,19 @@
 
 #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_CONV_BITS		10
 #define PVT_N_CONST		90
 #define PVT_R_CONST		245805
 
+#define PVT_TEMP_MIN_mC		-40000
+#define PVT_TEMP_MAX_mC		125000
+
+/* Temperature coefficients for series 5 */
+#define PVT_SERIES5_H_CONST	200000
+#define PVT_SERIES5_G_CONST	60000
+#define PVT_SERIES5_J_CONST	-100
+#define PVT_SERIES5_CAL5_CONST	4094
+
 #define PRE_SCALER_X1	1
 #define PRE_SCALER_X2	2
 
@@ -174,13 +180,26 @@ static umode_t pvt_is_visible(const void *data, enum hwmon_sensor_types type,
 	return 0;
 }
 
+static long pvt_calc_temp(struct pvt_device *pvt, u32 nbs)
+{
+	/*
+	 * Convert the register value to degrees centigrade temperature:
+	 * T = G + H * (n / cal5 - 0.5) + J * F
+	 */
+	s64 tmp = PVT_SERIES5_G_CONST +
+		PVT_SERIES5_H_CONST * (s64)nbs / PVT_SERIES5_CAL5_CONST -
+		PVT_SERIES5_H_CONST / 2 +
+		PVT_SERIES5_J_CONST * (s64)pvt->ip_freq / HZ_PER_MHZ;
+
+	return clamp_val(tmp, PVT_TEMP_MIN_mC, PVT_TEMP_MAX_mC);
+}
+
 static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct pvt_device *pvt = dev_get_drvdata(dev);
 	struct regmap *t_map = pvt->t_map;
 	u32 stat, nbs;
 	int ret;
-	u64 tmp;
 
 	switch (attr) {
 	case hwmon_temp_input:
@@ -201,9 +220,7 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val)
 		 * Convert the register value to
 		 * degrees centigrade temperature
 		 */
-		tmp = nbs * PVT_H_CONST;
-		do_div(tmp, PVT_CAL5_CONST);
-		*val = tmp - PVT_G_CONST - pvt->ip_freq;
+		*val = pvt_calc_temp(pvt, nbs);
 
 		return 0;
 	default:
@@ -337,7 +354,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

Re: [PATCH v5 15/21] hwmon: (mr75203) modify the temperature equation according to series 5 datasheet

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:43PM +0000, Eliav Farber wrote:
> Modify the equation and coefficients used 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
> 
> Where:
> *) G = 60, H = 200, cal5 = 4094, J = -0.1.
> *) F = frequency clock in MHz.
> *) n is the digital output.
> 
> In code, the G, H and J coefficients are multiplied by a factor of 1000
> to get the temperature in milli-Celsius.
> Final result is clamped in case it exceeds min/max thresholds.
> 
> Change is done since it is unclear where the current equation and
> coefficients came from.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property

[Eliav Farber]

Add optional "moortec,ts-series" property to define the temperature
equation and coefficients that shall be used to convert the digital
output to value in milli-Celsius.
Supported series: 5 (default) and 6.

Series 5:
  T = G + H * (n / cal5 - 0.5) + J * F
Where: G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz

Series 6:
   T = G + H * (n / cal5 - 0.5)
Where: G = 57.4, H = 249.4, cal5 = 4096

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V4 -> V3:
- Remove constraints in free-form text descriptions.

V3 -> V2:
- New patch to introduce "moortec,ts-series" property.

 .../devicetree/bindings/hwmon/moortec,mr75203.yaml       | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index d6b03a6d7043..df849517464e 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -91,6 +91,15 @@ properties:
       Each channel should not appear more than once.
     $ref: /schemas/types.yaml#/definitions/uint8-array
 
+  moortec,ts-series:
+    description:
+      Definition of the temperature equation and coefficients that shall be
+      used to convert the digital output to value in milli-Celsius.
+    minimum: 5
+    maximum: 6
+    default: 5
+    $ref: /schemas/types.yaml#/definitions/uint32
+
 required:
   - compatible
   - reg
-- 
2.37.1

Re: [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:44PM +0000, Eliav Farber wrote:
> Add optional "moortec,ts-series" property to define the temperature
> equation and coefficients that shall be used to convert the digital
> output to value in milli-Celsius.
> Supported series: 5 (default) and 6.
> 
> Series 5:
>   T = G + H * (n / cal5 - 0.5) + J * F
> Where: G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz
> 
> Series 6:
>    T = G + H * (n / cal5 - 0.5)
> Where: G = 57.4, H = 249.4, cal5 = 4096
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> V4 -> V3:
> - Remove constraints in free-form text descriptions.
> 
> V3 -> V2:
> - New patch to introduce "moortec,ts-series" property.
> 
>  .../devicetree/bindings/hwmon/moortec,mr75203.yaml       | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> index d6b03a6d7043..df849517464e 100644
> --- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
> @@ -91,6 +91,15 @@ properties:
>        Each channel should not appear more than once.
>      $ref: /schemas/types.yaml#/definitions/uint8-array
>  
> +  moortec,ts-series:
> +    description:
> +      Definition of the temperature equation and coefficients that shall be
> +      used to convert the digital output to value in milli-Celsius.
> +    minimum: 5
> +    maximum: 6
> +    default: 5
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +

I am not a dt expert, but I wonder if this should be handled
with different "compatible" properties. Sorry if this was already
addressed and I missed it.

Thanks,
Guenter

Re: [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec, ts-series" property

[Farber, Eliav]

On 9/8/2022 10:02 PM, Guenter Roeck wrote:
> On Thu, Sep 08, 2022 at 03:24:44PM +0000, Eliav Farber wrote:
>> Add optional "moortec,ts-series" property to define the temperature
>> equation and coefficients that shall be used to convert the digital
>> output to value in milli-Celsius.
>> Supported series: 5 (default) and 6.
>>
>> Series 5:
>>   T = G + H * (n / cal5 - 0.5) + J * F
>> Where: G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in 
>> MHz
>>
>> Series 6:
>>    T = G + H * (n / cal5 - 0.5)
>> Where: G = 57.4, H = 249.4, cal5 = 4096
>>
>> Signed-off-by: Eliav Farber <farbere@amazon.com>
>> ---
>> V4 -> V3:
>> - Remove constraints in free-form text descriptions.
>>
>> V3 -> V2:
>> - New patch to introduce "moortec,ts-series" property.
>>
>>  .../devicetree/bindings/hwmon/moortec,mr75203.yaml       | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml 
>> b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
>> index d6b03a6d7043..df849517464e 100644
>> --- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
>> @@ -91,6 +91,15 @@ properties:
>>        Each channel should not appear more than once.
>>      $ref: /schemas/types.yaml#/definitions/uint8-array
>>
>> +  moortec,ts-series:
>> +    description:
>> +      Definition of the temperature equation and coefficients that 
>> shall be
>> +      used to convert the digital output to value in milli-Celsius.
>> +    minimum: 5
>> +    maximum: 6
>> +    default: 5
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>
> I am not a dt expert, but I wonder if this should be handled
> with different "compatible" properties. Sorry if this was already
> addressed and I missed it.

It was already addressed in [PATCH v3 14/19] by Philipp.
It is indeed confusing which is why I added a detailed description
to moortec,mr75203.yaml in [PATCH v5 07/21].
Shortly, the driver is for the controller unit (mr75203), while the
"series" is a parameter of the temperature unit (mr74137).
Therefore I added a new TS property and not a different compatible.

--
Regards, Eliav

Re: [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec, ts-series" property

[Guenter Roeck]

On 9/8/22 13:18, Farber, Eliav wrote:
> On 9/8/2022 10:02 PM, Guenter Roeck wrote:
>> On Thu, Sep 08, 2022 at 03:24:44PM +0000, Eliav Farber wrote:
>>> Add optional "moortec,ts-series" property to define the temperature
>>> equation and coefficients that shall be used to convert the digital
>>> output to value in milli-Celsius.
>>> Supported series: 5 (default) and 6.
>>>
>>> Series 5:
>>>   T = G + H * (n / cal5 - 0.5) + J * F
>>> Where: G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz
>>>
>>> Series 6:
>>>    T = G + H * (n / cal5 - 0.5)
>>> Where: G = 57.4, H = 249.4, cal5 = 4096
>>>
>>> Signed-off-by: Eliav Farber <farbere@amazon.com>
>>> ---
>>> V4 -> V3:
>>> - Remove constraints in free-form text descriptions.
>>>
>>> V3 -> V2:
>>> - New patch to introduce "moortec,ts-series" property.
>>>
>>>  .../devicetree/bindings/hwmon/moortec,mr75203.yaml       | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
>>> index d6b03a6d7043..df849517464e 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
>>> @@ -91,6 +91,15 @@ properties:
>>>        Each channel should not appear more than once.
>>>      $ref: /schemas/types.yaml#/definitions/uint8-array
>>>
>>> +  moortec,ts-series:
>>> +    description:
>>> +      Definition of the temperature equation and coefficients that shall be
>>> +      used to convert the digital output to value in milli-Celsius.
>>> +    minimum: 5
>>> +    maximum: 6
>>> +    default: 5
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +
>>
>> I am not a dt expert, but I wonder if this should be handled
>> with different "compatible" properties. Sorry if this was already
>> addressed and I missed it.
> 
> It was already addressed in [PATCH v3 14/19] by Philipp.
> It is indeed confusing which is why I added a detailed description
> to moortec,mr75203.yaml in [PATCH v5 07/21].
> Shortly, the driver is for the controller unit (mr75203), while the
> "series" is a parameter of the temperature unit (mr74137).
> Therefore I added a new TS property and not a different compatible.
> 

Thanks a lot for the clarification.

Guenter

Re: [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property

[Rob Herring]

On Thu, 08 Sep 2022 15:24:44 +0000, Eliav Farber wrote:
> Add optional "moortec,ts-series" property to define the temperature
> equation and coefficients that shall be used to convert the digital
> output to value in milli-Celsius.
> Supported series: 5 (default) and 6.
> 
> Series 5:
>   T = G + H * (n / cal5 - 0.5) + J * F
> Where: G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz
> 
> Series 6:
>    T = G + H * (n / cal5 - 0.5)
> Where: G = 57.4, H = 249.4, cal5 = 4096
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> V4 -> V3:
> - Remove constraints in free-form text descriptions.
> 
> V3 -> V2:
> - New patch to introduce "moortec,ts-series" property.
> 
>  .../devicetree/bindings/hwmon/moortec,mr75203.yaml       | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:44PM +0000, Eliav Farber wrote:
> Add optional "moortec,ts-series" property to define the temperature
> equation and coefficients that shall be used to convert the digital
> output to value in milli-Celsius.
> Supported series: 5 (default) and 6.
> 
> Series 5:
>   T = G + H * (n / cal5 - 0.5) + J * F
> Where: G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz
> 
> Series 6:
>    T = G + H * (n / cal5 - 0.5)
> Where: G = 57.4, H = 249.4, cal5 = 4096
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 17/21] hwmon: (mr75203) add support for series 6 temperature equation

[Eliav Farber]

The current equation used in code is aligned to series 5:
T = G + H * (n / cal5 - 0.5) + J * F
Where:
G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz

Series 6 has a slightly different equation:
T = G + H * (n / cal5 - 0.5)
and a different set of coefficients:
G = 57.4, H = 249.4, cal5 = 4096

This change supports equation and coefficients for both series.
(for series 6, J is set to 0).

The series is determined according to “moortec,ts-series” property in
the device tree.
If absent, series 5 is assumed to be the default.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> V3:
- Replace of_property_read_u32() with device_property_read_u32().
- Use switch-case instead of if-else.

V3 -> V2:
- New patch to support temperature sensor series 6 instead of having to
  set all 4 coefficients.

 drivers/hwmon/mr75203.c | 66 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 0de64642cc42..bbb59dd7e7cb 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -115,6 +115,15 @@
 #define PVT_SERIES5_J_CONST	-100
 #define PVT_SERIES5_CAL5_CONST	4094
 
+/* Temperature coefficients for series 6 */
+#define PVT_SERIES6_H_CONST	249400
+#define PVT_SERIES6_G_CONST	57400
+#define PVT_SERIES6_J_CONST	0
+#define PVT_SERIES6_CAL5_CONST	4096
+
+#define TEMPERATURE_SENSOR_SERIES_5	5
+#define TEMPERATURE_SENSOR_SERIES_6	6
+
 #define PRE_SCALER_X1	1
 #define PRE_SCALER_X2	2
 
@@ -147,6 +156,13 @@ struct voltage_channels {
 	u8 max;
 };
 
+struct temp_coeff {
+	u32 h;
+	u32 g;
+	u32 cal5;
+	s32 j;
+};
+
 struct pvt_device {
 	struct regmap		*c_map;
 	struct regmap		*t_map;
@@ -156,6 +172,7 @@ struct pvt_device {
 	struct reset_control	*rst;
 	struct voltage_device	*vd;
 	struct voltage_channels	vm_channels;
+	struct temp_coeff	ts_coeff;
 	u32			t_num;
 	u32			p_num;
 	u32			v_num;
@@ -186,10 +203,12 @@ static long pvt_calc_temp(struct pvt_device *pvt, u32 nbs)
 	 * Convert the register value to degrees centigrade temperature:
 	 * T = G + H * (n / cal5 - 0.5) + J * F
 	 */
-	s64 tmp = PVT_SERIES5_G_CONST +
-		PVT_SERIES5_H_CONST * (s64)nbs / PVT_SERIES5_CAL5_CONST -
-		PVT_SERIES5_H_CONST / 2 +
-		PVT_SERIES5_J_CONST * (s64)pvt->ip_freq / HZ_PER_MHZ;
+	struct temp_coeff *ts_coeff = &pvt->ts_coeff;
+
+	s64 tmp = ts_coeff->g +
+		ts_coeff->h * (s64)nbs / ts_coeff->cal5 -
+		ts_coeff->h / 2 +
+		ts_coeff->j * (s64)pvt->ip_freq / HZ_PER_MHZ;
 
 	return clamp_val(tmp, PVT_TEMP_MIN_mC, PVT_TEMP_MAX_mC);
 }
@@ -668,6 +687,41 @@ static int pvt_get_pre_scaler(struct device *dev, struct pvt_device *pvt)
 	return ret;
 }
 
+static int pvt_set_temp_coeff(struct device *dev, struct pvt_device *pvt)
+{
+	struct temp_coeff *ts_coeff = &pvt->ts_coeff;
+	u32 series;
+	int ret;
+
+	/* Incase ts-series property is not defined, use default 5. */
+	ret = device_property_read_u32(dev, "moortec,ts-series", &series);
+	if (ret)
+		series = TEMPERATURE_SENSOR_SERIES_5;
+
+	switch (series) {
+	case TEMPERATURE_SENSOR_SERIES_5:
+		ts_coeff->h = PVT_SERIES5_H_CONST;
+		ts_coeff->g = PVT_SERIES5_G_CONST;
+		ts_coeff->j = PVT_SERIES5_J_CONST;
+		ts_coeff->cal5 = PVT_SERIES5_CAL5_CONST;
+		break;
+	case TEMPERATURE_SENSOR_SERIES_6:
+		ts_coeff->h = PVT_SERIES6_H_CONST;
+		ts_coeff->g = PVT_SERIES6_G_CONST;
+		ts_coeff->j = PVT_SERIES6_J_CONST;
+		ts_coeff->cal5 = PVT_SERIES6_CAL5_CONST;
+		break;
+	default:
+		dev_err(dev, "invalid temperature sensor series (%u)\n",
+			series);
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "temperature sensor series = %u\n", series);
+
+	return 0;
+}
+
 static int mr75203_probe(struct platform_device *pdev)
 {
 	u32 ts_num, vm_num, pd_num, ch_num, val, index, i;
@@ -738,6 +792,10 @@ static int mr75203_probe(struct platform_device *pdev)
 		if (ret)
 			return ret;
 
+		ret = pvt_set_temp_coeff(dev, pvt);
+		if (ret)
+			return ret;
+
 		temp_config = devm_kcalloc(dev, ts_num + 1,
 					   sizeof(*temp_config), GFP_KERNEL);
 		if (!temp_config)
-- 
2.37.1

Re: [PATCH v5 17/21] hwmon: (mr75203) add support for series 6 temperature equation

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:45PM +0000, Eliav Farber wrote:
> The current equation used in code is aligned to series 5:
> T = G + H * (n / cal5 - 0.5) + J * F
> Where:
> G = 60, H = 200, cal5 = 4094, J = -0.1, F = frequency clock in MHz
> 
> Series 6 has a slightly different equation:
> T = G + H * (n / cal5 - 0.5)
> and a different set of coefficients:
> G = 57.4, H = 249.4, cal5 = 4096
> 
> This change supports equation and coefficients for both series.
> (for series 6, J is set to 0).
> 
> The series is determined according to “moortec,ts-series” property in
> the device tree.
> If absent, series 5 is assumed to be the default.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 18/21] dt-bindings: hwmon: (mr75203) add coefficient properties for the thermal equation

[Eliav Farber]

Add optional temperature coefficient properties:
 *) moortec,ts-coeff-g
 *) moortec,ts-coeff-h
 *) moortec,ts-coeff-cal5
 *) moortec,ts-coeff-j
If defined they shall be used instead of defaults.

The coefficients were added to device tree on top of the series property
(which can be used to select between series 5 and series 6), because
coefficients can vary between product and product, and code defaults might
not be accurate enough.

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V4 -> V3:
- Add 'multipleOf: 1000' instead of plain text.
- Add minimum/maximum for some of the new properties.

V3 -> V2:
- Add "moortec" prefix to property name.

 .../bindings/hwmon/moortec,mr75203.yaml       | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
index df849517464e..d0d549749208 100644
--- a/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
+++ b/Documentation/devicetree/bindings/hwmon/moortec,mr75203.yaml
@@ -100,6 +100,41 @@ properties:
     default: 5
     $ref: /schemas/types.yaml#/definitions/uint32
 
+  moortec,ts-coeff-g:
+    description:
+      G coefficient for temperature equation.
+      Default for series 5 = 60000
+      Default for series 6 = 57400
+    multipleOf: 1000
+    minimum: 1000
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  moortec,ts-coeff-h:
+    description:
+      H coefficient for temperature equation.
+      Default for series 5 = 200000
+      Default for series 6 = 249400
+    multipleOf: 1000
+    minimum: 1000
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  moortec,ts-coeff-cal5:
+    description:
+      cal5 coefficient for temperature equation.
+      Default for series 5 = 4094
+      Default for series 6 = 4096
+    minimum: 1
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  moortec,ts-coeff-j:
+    description:
+      J coefficient for temperature equation.
+      Default for series 5 = -100
+      Default for series 6 = 0
+    multipleOf: 1000
+    maximum: 0
+    $ref: /schemas/types.yaml#/definitions/int32
+
 required:
   - compatible
   - reg
@@ -123,5 +158,7 @@ examples:
         resets = <&rcu0 0x40 7>;
         moortec,vm-active-channels = /bits/ 8 <0x10 0x05>;
         moortec,vm-pre-scaler-x2 = /bits/ 8 <5 6 20>;
+        moortec,ts-coeff-g = <61400>;
+        moortec,ts-coeff-h = <253700>;
         #thermal-sensor-cells = <1>;
     };
-- 
2.37.1

Re: [PATCH v5 18/21] dt-bindings: hwmon: (mr75203) add coefficient properties for the thermal equation

[Rob Herring]

On Thu, 08 Sep 2022 15:24:46 +0000, Eliav Farber wrote:
> Add optional temperature coefficient properties:
>  *) moortec,ts-coeff-g
>  *) moortec,ts-coeff-h
>  *) moortec,ts-coeff-cal5
>  *) moortec,ts-coeff-j
> If defined they shall be used instead of defaults.
> 
> The coefficients were added to device tree on top of the series property
> (which can be used to select between series 5 and series 6), because
> coefficients can vary between product and product, and code defaults might
> not be accurate enough.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
> V4 -> V3:
> - Add 'multipleOf: 1000' instead of plain text.
> - Add minimum/maximum for some of the new properties.
> 
> V3 -> V2:
> - Add "moortec" prefix to property name.
> 
>  .../bindings/hwmon/moortec,mr75203.yaml       | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 18/21] dt-bindings: hwmon: (mr75203) add coefficient properties for the thermal equation

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:46PM +0000, Eliav Farber wrote:
> Add optional temperature coefficient properties:
>  *) moortec,ts-coeff-g
>  *) moortec,ts-coeff-h
>  *) moortec,ts-coeff-cal5
>  *) moortec,ts-coeff-j
> If defined they shall be used instead of defaults.
> 
> The coefficients were added to device tree on top of the series property
> (which can be used to select between series 5 and series 6), because
> coefficients can vary between product and product, and code defaults might
> not be accurate enough.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 19/21] hwmon: (mr75203) parse temperature coefficients from device-tree

[Eliav Farber]

Use thermal coefficients from the device tree if they exist.
Otherwise, use default values according to the series (5 or 6).
All coefficients can be used or only part of them.

The coefficients shall be used for fine tuning the default values since
coefficients can vary between product and product.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V4 -> V3:
- Replace of_property_read_u32() with device_property_read_u32().
- Fix "Code shouldn't be a YAML validator".
- Read directly to ts_coeff-> parameter to avoid conditional if.

 drivers/hwmon/mr75203.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index bbb59dd7e7cb..07668545c3ae 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -719,6 +719,15 @@ static int pvt_set_temp_coeff(struct device *dev, struct pvt_device *pvt)
 
 	dev_dbg(dev, "temperature sensor series = %u\n", series);
 
+	/* Override ts-coeff-h/g/j/cal5 if they are defined. */
+	device_property_read_u32(dev, "moortec,ts-coeff-h", &ts_coeff->h);
+	device_property_read_u32(dev, "moortec,ts-coeff-g", &ts_coeff->g);
+	device_property_read_u32(dev, "moortec,ts-coeff-j", &ts_coeff->j);
+	device_property_read_u32(dev, "moortec,ts-coeff-cal5", &ts_coeff->cal5);
+
+	dev_dbg(dev, "ts-coeff: h = %u, g = %u, j = %d, cal5 = %u\n",
+		ts_coeff->h, ts_coeff->g, ts_coeff->j, ts_coeff->cal5);
+
 	return 0;
 }
 
-- 
2.37.1

Re: [PATCH v5 19/21] hwmon: (mr75203) parse temperature coefficients from device-tree

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:47PM +0000, Eliav Farber wrote:
> Use thermal coefficients from the device tree if they exist.
> Otherwise, use default values according to the series (5 or 6).
> All coefficients can be used or only part of them.
> 
> The coefficients shall be used for fine tuning the default values since
> coefficients can vary between product and product.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Eliav Farber]

This change adds debugfs to read and write temperature sensor coefficients
- g, h, j and cal5.

The coefficients can vary between product and product, so it can be very
useful to be able to modify them on the fly during the calibration
process.

e.g.:

cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5
4096

echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g

Signed-off-by: Eliav Farber <farbere@amazon.com>
---
V5 -> V4:
- Return j coefficient to use debugfs_create_file() instead of
  debugfs_create_u32() because j is signed.

V4 -> V3:
- Remove check of the debugfs_create_dir() return value.
- Use debugfs_create_u32() instead of debugfs_create_file().
- Return devm_add_action_or_reset() without checking return value and printing
  an error message on failure.

 drivers/hwmon/mr75203.c | 62 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 07668545c3ae..e6b49f810307 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -9,6 +9,7 @@
  */
 #include <linux/bits.h>
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/hwmon.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
@@ -170,6 +171,7 @@ struct pvt_device {
 	struct regmap		*v_map;
 	struct clk		*clk;
 	struct reset_control	*rst;
+	struct dentry		*dbgfs_dir;
 	struct voltage_device	*vd;
 	struct voltage_channels	vm_channels;
 	struct temp_coeff	ts_coeff;
@@ -179,6 +181,64 @@ struct pvt_device {
 	u32			ip_freq;
 };
 
+static ssize_t pvt_ts_coeff_j_read(struct file *file, char __user *user_buf,
+				   size_t count, loff_t *ppos)
+{
+	struct pvt_device *pvt = file->private_data;
+	unsigned int len;
+	char buf[13];
+
+	len = scnprintf(buf, sizeof(buf), "%d\n", pvt->ts_coeff.j);
+
+	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t pvt_ts_coeff_j_write(struct file *file,
+				    const char __user *user_buf,
+				    size_t count, loff_t *ppos)
+{
+	struct pvt_device *pvt = file->private_data;
+	int ret;
+
+	ret = kstrtos32_from_user(user_buf, count, 0, &pvt->ts_coeff.j);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations pvt_ts_coeff_j_fops = {
+	.read = pvt_ts_coeff_j_read,
+	.write = pvt_ts_coeff_j_write,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
+static void devm_pvt_ts_dbgfs_remove(void *data)
+{
+	struct pvt_device *pvt = (struct pvt_device *)data;
+
+	debugfs_remove_recursive(pvt->dbgfs_dir);
+	pvt->dbgfs_dir = NULL;
+}
+
+static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev)
+{
+	pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
+
+	debugfs_create_u32("ts_coeff_h", 0644, pvt->dbgfs_dir,
+			   &pvt->ts_coeff.h);
+	debugfs_create_u32("ts_coeff_g", 0644, pvt->dbgfs_dir,
+			   &pvt->ts_coeff.g);
+	debugfs_create_u32("ts_coeff_cal5", 0644, pvt->dbgfs_dir,
+			   &pvt->ts_coeff.cal5);
+	debugfs_create_file("ts_coeff_j", 0644, pvt->dbgfs_dir, pvt,
+			    &pvt_ts_coeff_j_fops);
+
+	return devm_add_action_or_reset(dev, devm_pvt_ts_dbgfs_remove, pvt);
+}
+
 static umode_t pvt_is_visible(const void *data, enum hwmon_sensor_types type,
 			      u32 attr, int channel)
 {
@@ -813,6 +873,8 @@ static int mr75203_probe(struct platform_device *pdev)
 		memset32(temp_config, HWMON_T_INPUT, ts_num);
 		pvt_temp.config = temp_config;
 		pvt_info[index++] = &pvt_temp;
+
+		pvt_ts_dbgfs_create(pvt, dev);
 	}
 
 	if (pd_num) {
-- 
2.37.1

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Andy Shevchenko]

On Thu, Sep 08, 2022 at 03:24:48PM +0000, Eliav Farber wrote:
> This change adds debugfs to read and write temperature sensor coefficients
> - g, h, j and cal5.
> 
> The coefficients can vary between product and product, so it can be very
> useful to be able to modify them on the fly during the calibration
> process.
> 
> e.g.:
> 
> cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5
> 4096
> 
> echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g

...

> - Return j coefficient to use debugfs_create_file() instead of
>   debugfs_create_u32() because j is signed.

You can use

DEFINE_DEBUGFS_ATTRIBUTE(ts_coeff_j, ts_coeff_j_get, ts_coeff_j_set, "%lld\n");

which still makes code compact.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Farber, Eliav]

On 9/8/2022 9:11 PM, Andy Shevchenko wrote:
> On Thu, Sep 08, 2022 at 03:24:48PM +0000, Eliav Farber wrote:
>> This change adds debugfs to read and write temperature sensor 
>> coefficients
>> - g, h, j and cal5.
>>
>> The coefficients can vary between product and product, so it can be very
>> useful to be able to modify them on the fly during the calibration
>> process.
>>
>> e.g.:
>>
>> cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5
>> 4096
>>
>> echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g
>
> ...
>
>> - Return j coefficient to use debugfs_create_file() instead of
>>   debugfs_create_u32() because j is signed.
>
> You can use
>
> DEFINE_DEBUGFS_ATTRIBUTE(ts_coeff_j, ts_coeff_j_get, ts_coeff_j_set, 
> "%lld\n");
>
> which still makes code compact. 


I tried your suggestion to use DEFINE_DEBUGFS_ATTRIBUTE but I can't set
j to be a negative value:

root@alpine:~# cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
0
root@alpine:~# echo 100 > /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
root@alpine:~# cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
100
root@alpine:~# echo -100 > /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
sh: write error: Invalid argument

This is the code I added:

static int ts_coeff_j_set(void *data, u64 val)
{
     struct pvt_device *pvt = data;

     pvt->ts_coeff.j = val;
     return 0;
}

static int ts_coeff_j_get(void *data, u64 *val)
{
     struct pvt_device *pvt = data;

     *val = pvt->ts_coeff.j;
     return 0;
}

DEFINE_DEBUGFS_ATTRIBUTE(ts_coeff_j_fops, ts_coeff_j_get,
              ts_coeff_j_set, "%lld\n");

static void devm_pvt_ts_dbgfs_remove(void *data)
{
     struct pvt_device *pvt = (struct pvt_device *)data;

     debugfs_remove_recursive(pvt->dbgfs_dir);
     pvt->dbgfs_dir = NULL;
}

static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev)
{
     ...
     debugfs_create_file("ts_coeff_j", 0644, pvt->dbgfs_dir, pvt,
                 &ts_coeff_j_fops);
     ...

I'm using kernel 5.10.112.
Can you please see if I'm did anything wrong?

--
Thanks, Eliav

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Farber, Eliav]

On 9/13/2022 4:06 PM, Farber, Eliav wrote:
> On 9/8/2022 9:11 PM, Andy Shevchenko wrote:
>> On Thu, Sep 08, 2022 at 03:24:48PM +0000, Eliav Farber wrote:
>>> This change adds debugfs to read and write temperature sensor 
>>> coefficients
>>> - g, h, j and cal5.
>>>
>>> The coefficients can vary between product and product, so it can be 
>>> very
>>> useful to be able to modify them on the fly during the calibration
>>> process.
>>>
>>> e.g.:
>>>
>>> cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5
>>> 4096
>>>
>>> echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g
>>
>> ...
>>
>>> - Return j coefficient to use debugfs_create_file() instead of
>>>   debugfs_create_u32() because j is signed.
>>
>> You can use
>>
>> DEFINE_DEBUGFS_ATTRIBUTE(ts_coeff_j, ts_coeff_j_get, ts_coeff_j_set, 
>> "%lld\n");
>>
>> which still makes code compact. 
>
>
> I tried your suggestion to use DEFINE_DEBUGFS_ATTRIBUTE but I can't set
> j to be a negative value:
>
> root@alpine:~# cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
> 0
> root@alpine:~# echo 100 > /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
> root@alpine:~# cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
> 100
> root@alpine:~# echo -100 > /sys/kernel/debug/940f23d0000.pvt/ts_coeff_j
> sh: write error: Invalid argument
>
> This is the code I added:
>
> static int ts_coeff_j_set(void *data, u64 val)
> {
>     struct pvt_device *pvt = data;
>
>     pvt->ts_coeff.j = val;
>     return 0;
> }
>
> static int ts_coeff_j_get(void *data, u64 *val)
> {
>     struct pvt_device *pvt = data;
>
>     *val = pvt->ts_coeff.j;
>     return 0;
> }
>
> DEFINE_DEBUGFS_ATTRIBUTE(ts_coeff_j_fops, ts_coeff_j_get,
>              ts_coeff_j_set, "%lld\n");
>
> static void devm_pvt_ts_dbgfs_remove(void *data)
> {
>     struct pvt_device *pvt = (struct pvt_device *)data;
>
>     debugfs_remove_recursive(pvt->dbgfs_dir);
>     pvt->dbgfs_dir = NULL;
> }
>
> static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device 
> *dev)
> {
>     ...
>     debugfs_create_file("ts_coeff_j", 0644, pvt->dbgfs_dir, pvt,
>                 &ts_coeff_j_fops);
>     ...
>
> I'm using kernel 5.10.112.
> Can you please see if I'm did anything wrong? 

It seems like debugfs_attr_write() calls simple_attr_write() and it uses
kstrtoull(), which is why it fails when setting a negative value.
This is the same also in v6.0-rc5.

debugfs_attr_read() on the other hand does show the correct value also
when j is negative.

--
Regards, Eliav

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Andy Shevchenko]

On Tue, Sep 13, 2022 at 05:40:16PM +0300, Farber, Eliav wrote:
> On 9/13/2022 4:06 PM, Farber, Eliav wrote:

...

> It seems like debugfs_attr_write() calls simple_attr_write() and it uses
> kstrtoull(), which is why it fails when setting a negative value.
> This is the same also in v6.0-rc5.
> 
> debugfs_attr_read() on the other hand does show the correct value also
> when j is negative.

Which puzzles me since there is a few drivers that use %lld.
Yeah, changing it to

	ret = sscanf(attr->set_buf, attr->fmt, &val);
	if (ret != 1)
		ret = -EINVAL;

probably can fix that. Dunno if debugfs maintainer is okay with this.

P.S. This needs revisiting all format strings to see if there are no additional
characters, otherwise that needs to be addressed first, if feasible.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Farber, Eliav]

On 9/13/2022 8:01 PM, Andy Shevchenko wrote:
> On Tue, Sep 13, 2022 at 05:40:16PM +0300, Farber, Eliav wrote:
>> On 9/13/2022 4:06 PM, Farber, Eliav wrote:
>
> ...
>
>> It seems like debugfs_attr_write() calls simple_attr_write() and it uses
>> kstrtoull(), which is why it fails when setting a negative value.
>> This is the same also in v6.0-rc5.
>>
>> debugfs_attr_read() on the other hand does show the correct value also
>> when j is negative.
>
> Which puzzles me since there is a few drivers that use %lld.
> Yeah, changing it to
>
>        ret = sscanf(attr->set_buf, attr->fmt, &val);
>        if (ret != 1)
>                ret = -EINVAL;
>
> probably can fix that. Dunno if debugfs maintainer is okay with this.
>
> P.S. This needs revisiting all format strings to see if there are no 
> additional
> characters, otherwise that needs to be addressed first, if feasible.

I was thinking of making such a correction:

-       ret = kstrtoull(attr->set_buf, 0, &val);
+       if (attr->set_buf[0] == '-')
+               ret = kstrtoll(attr->set_buf, 0, &val);
+       else
+               ret = kstrtoull(attr->set_buf, 0, &val);

and when I tested the change it worked, but then I noticed this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/libfs.c?h=v6.0-rc5&id=488dac0c9237647e9b8f788b6a342595bfa40bda

According to this, it previously used simple_strtoll() which supports
negative values, but was changed to use kstrtoull() to deliberately
return '-EINVAL' if it gets a negative value.

So I’m not sure debugfs maintainers will be okay with a fix that
basically reverts the commit I mentioned.
Hence, what do you suggest to do with my commit?
Is it ok to leave it as it is today?

--
Thanks, Eliav

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Andy Shevchenko]

On Wed, Sep 14, 2022 at 07:26:36AM +0300, Farber, Eliav wrote:
> On 9/13/2022 8:01 PM, Andy Shevchenko wrote:
> > On Tue, Sep 13, 2022 at 05:40:16PM +0300, Farber, Eliav wrote:
> > > On 9/13/2022 4:06 PM, Farber, Eliav wrote:

...

> > > It seems like debugfs_attr_write() calls simple_attr_write() and it uses
> > > kstrtoull(), which is why it fails when setting a negative value.
> > > This is the same also in v6.0-rc5.
> > > 
> > > debugfs_attr_read() on the other hand does show the correct value also
> > > when j is negative.
> > 
> > Which puzzles me since there is a few drivers that use %lld.
> > Yeah, changing it to
> > 
> >        ret = sscanf(attr->set_buf, attr->fmt, &val);
> >        if (ret != 1)
> >                ret = -EINVAL;
> > 
> > probably can fix that. Dunno if debugfs maintainer is okay with this.
> > 
> > P.S. This needs revisiting all format strings to see if there are no
> > additional
> > characters, otherwise that needs to be addressed first, if feasible.
> 
> I was thinking of making such a correction:
> 
> -       ret = kstrtoull(attr->set_buf, 0, &val);
> +       if (attr->set_buf[0] == '-')
> +               ret = kstrtoll(attr->set_buf, 0, &val);
> +       else
> +               ret = kstrtoull(attr->set_buf, 0, &val);
> 
> and when I tested the change it worked, but then I noticed this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/libfs.c?h=v6.0-rc5&id=488dac0c9237647e9b8f788b6a342595bfa40bda
> 
> According to this, it previously used simple_strtoll() which supports
> negative values, but was changed to use kstrtoull() to deliberately
> return '-EINVAL' if it gets a negative value.
> 
> So I’m not sure debugfs maintainers will be okay with a fix that
> basically reverts the commit I mentioned.
> Hence, what do you suggest to do with my commit?
> Is it ok to leave it as it is today?

Meanwhile asking is not a problem, at least we will know for sure.
And yes, leave it as is, but point to the thread where you asking
the clarification.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Andy Shevchenko]

On Wed, Sep 14, 2022 at 12:32:47PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 14, 2022 at 07:26:36AM +0300, Farber, Eliav wrote:
> > On 9/13/2022 8:01 PM, Andy Shevchenko wrote:
> > > On Tue, Sep 13, 2022 at 05:40:16PM +0300, Farber, Eliav wrote:
> > > > On 9/13/2022 4:06 PM, Farber, Eliav wrote:

...

> > > > It seems like debugfs_attr_write() calls simple_attr_write() and it uses
> > > > kstrtoull(), which is why it fails when setting a negative value.
> > > > This is the same also in v6.0-rc5.
> > > > 
> > > > debugfs_attr_read() on the other hand does show the correct value also
> > > > when j is negative.
> > > 
> > > Which puzzles me since there is a few drivers that use %lld.
> > > Yeah, changing it to
> > > 
> > >        ret = sscanf(attr->set_buf, attr->fmt, &val);
> > >        if (ret != 1)
> > >                ret = -EINVAL;
> > > 
> > > probably can fix that. Dunno if debugfs maintainer is okay with this.
> > > 
> > > P.S. This needs revisiting all format strings to see if there are no
> > > additional
> > > characters, otherwise that needs to be addressed first, if feasible.
> > 
> > I was thinking of making such a correction:
> > 
> > -       ret = kstrtoull(attr->set_buf, 0, &val);
> > +       if (attr->set_buf[0] == '-')
> > +               ret = kstrtoll(attr->set_buf, 0, &val);
> > +       else
> > +               ret = kstrtoull(attr->set_buf, 0, &val);
> > 
> > and when I tested the change it worked, but then I noticed this commit:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/libfs.c?h=v6.0-rc5&id=488dac0c9237647e9b8f788b6a342595bfa40bda
> > 
> > According to this, it previously used simple_strtoll() which supports
> > negative values, but was changed to use kstrtoull() to deliberately
> > return '-EINVAL' if it gets a negative value.
> > 
> > So I’m not sure debugfs maintainers will be okay with a fix that
> > basically reverts the commit I mentioned.
> > Hence, what do you suggest to do with my commit?
> > Is it ok to leave it as it is today?
> 
> Meanwhile asking is not a problem, at least we will know for sure.
> And yes, leave it as is, but point to the thread where you asking
> the clarification.

For the record:

$ git grep -n -A1 -w DEFINE_DEBUGFS_ATTRIBUTE | grep ');' | sed 's,.*\(".*%.*"\).*,\1,' | sort | uniq -c
  1 "%08llx\n"
  5 "0x%016llx\n"
  5 "0x%02llx\n"
  5 "0x%04llx\n"
 13 "0x%08llx\n"
  1 "0x%4.4llx\n"
  3 "0x%.4llx\n"
  4 "0x%llx\n"
  1 "%1lld\n"
 40 "%lld\n"
  2 "%lli\n"
129 "%llu\n"
  1 "%#llx\n"
  2 "%llx\n"

means that sscanf() should work and fix the issue. You may even propose a patch
as a starter for a discussion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Andy Shevchenko]

On Wed, Sep 14, 2022 at 05:03:09PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 14, 2022 at 12:32:47PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 14, 2022 at 07:26:36AM +0300, Farber, Eliav wrote:
> > > On 9/13/2022 8:01 PM, Andy Shevchenko wrote:
> > > > On Tue, Sep 13, 2022 at 05:40:16PM +0300, Farber, Eliav wrote:
> > > > > On 9/13/2022 4:06 PM, Farber, Eliav wrote:

...

> > > > > It seems like debugfs_attr_write() calls simple_attr_write() and it uses
> > > > > kstrtoull(), which is why it fails when setting a negative value.
> > > > > This is the same also in v6.0-rc5.
> > > > > 
> > > > > debugfs_attr_read() on the other hand does show the correct value also
> > > > > when j is negative.
> > > > 
> > > > Which puzzles me since there is a few drivers that use %lld.
> > > > Yeah, changing it to
> > > > 
> > > >        ret = sscanf(attr->set_buf, attr->fmt, &val);
> > > >        if (ret != 1)
> > > >                ret = -EINVAL;
> > > > 
> > > > probably can fix that. Dunno if debugfs maintainer is okay with this.
> > > > 
> > > > P.S. This needs revisiting all format strings to see if there are no
> > > > additional
> > > > characters, otherwise that needs to be addressed first, if feasible.
> > > 
> > > I was thinking of making such a correction:
> > > 
> > > -       ret = kstrtoull(attr->set_buf, 0, &val);
> > > +       if (attr->set_buf[0] == '-')
> > > +               ret = kstrtoll(attr->set_buf, 0, &val);
> > > +       else
> > > +               ret = kstrtoull(attr->set_buf, 0, &val);
> > > 
> > > and when I tested the change it worked, but then I noticed this commit:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/libfs.c?h=v6.0-rc5&id=488dac0c9237647e9b8f788b6a342595bfa40bda
> > > 
> > > According to this, it previously used simple_strtoll() which supports
> > > negative values, but was changed to use kstrtoull() to deliberately
> > > return '-EINVAL' if it gets a negative value.
> > > 
> > > So I’m not sure debugfs maintainers will be okay with a fix that
> > > basically reverts the commit I mentioned.
> > > Hence, what do you suggest to do with my commit?
> > > Is it ok to leave it as it is today?
> > 
> > Meanwhile asking is not a problem, at least we will know for sure.
> > And yes, leave it as is, but point to the thread where you asking
> > the clarification.
> 
> For the record:
> 
> $ git grep -n -A1 -w DEFINE_DEBUGFS_ATTRIBUTE | grep ');' | sed 's,.*\(".*%.*"\).*,\1,' | sort | uniq -c
>   1 "%08llx\n"
>   5 "0x%016llx\n"
>   5 "0x%02llx\n"
>   5 "0x%04llx\n"
>  13 "0x%08llx\n"
>   1 "0x%4.4llx\n"
>   3 "0x%.4llx\n"
>   4 "0x%llx\n"
>   1 "%1lld\n"
>  40 "%lld\n"
>   2 "%lli\n"
> 129 "%llu\n"
>   1 "%#llx\n"
>   2 "%llx\n"
> 
> means that sscanf() should work and fix the issue. You may even propose a patch
> as a starter for a discussion.

Reading the commit 488dac0c9237 ("libfs: fix error cast of negative value in
simple_attr_write()") I think it should be either reverted or fixed, because
u64 is not an issue for negative numbers. The %lld and %llu in any case are
for 64-bit value, representing it as unsigned simplifies the generic code,
but it doesn't mean we can't keep there signed value if we know that (by
supplying proper format string). We have 43 current users of signed integer
and I'm in doubt this patch doesn't break none of them.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Farber, Eliav]

On 9/14/2022 5:08 PM, Andy Shevchenko wrote:
> CAUTION: This email originated from outside of the organization. Do 
> not click links or open attachments unless you can confirm the sender 
> and know the content is safe.
>
>
>
> On Wed, Sep 14, 2022 at 05:03:09PM +0300, Andy Shevchenko wrote:
>> On Wed, Sep 14, 2022 at 12:32:47PM +0300, Andy Shevchenko wrote:
>> > On Wed, Sep 14, 2022 at 07:26:36AM +0300, Farber, Eliav wrote:
>> > > On 9/13/2022 8:01 PM, Andy Shevchenko wrote:
>> > > > On Tue, Sep 13, 2022 at 05:40:16PM +0300, Farber, Eliav wrote:
>> > > > > On 9/13/2022 4:06 PM, Farber, Eliav wrote:
>
> ...
>
>> > > > > It seems like debugfs_attr_write() calls simple_attr_write() 
>> and it uses
>> > > > > kstrtoull(), which is why it fails when setting a negative 
>> value.
>> > > > > This is the same also in v6.0-rc5.
>> > > > >
>> > > > > debugfs_attr_read() on the other hand does show the correct 
>> value also
>> > > > > when j is negative.
>> > > >
>> > > > Which puzzles me since there is a few drivers that use %lld.
>> > > > Yeah, changing it to
>> > > >
>> > > >        ret = sscanf(attr->set_buf, attr->fmt, &val);
>> > > >        if (ret != 1)
>> > > >                ret = -EINVAL;
>> > > >
>> > > > probably can fix that. Dunno if debugfs maintainer is okay with 
>> this.
>> > > >
>> > > > P.S. This needs revisiting all format strings to see if there 
>> are no
>> > > > additional
>> > > > characters, otherwise that needs to be addressed first, if 
>> feasible.
>> > >
>> > > I was thinking of making such a correction:
>> > >
>> > > -       ret = kstrtoull(attr->set_buf, 0, &val);
>> > > +       if (attr->set_buf[0] == '-')
>> > > +               ret = kstrtoll(attr->set_buf, 0, &val);
>> > > +       else
>> > > +               ret = kstrtoull(attr->set_buf, 0, &val);
>> > >
>> > > and when I tested the change it worked, but then I noticed this 
>> commit:
>> > >
>> > > 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/libfs.c?h=v6.0-rc5&id=488dac0c9237647e9b8f788b6a342595bfa40bda
>> > >
>> > > According to this, it previously used simple_strtoll() which 
>> supports
>> > > negative values, but was changed to use kstrtoull() to deliberately
>> > > return '-EINVAL' if it gets a negative value.
>> > >
>> > > So I’m not sure debugfs maintainers will be okay with a fix that
>> > > basically reverts the commit I mentioned.
>> > > Hence, what do you suggest to do with my commit?
>> > > Is it ok to leave it as it is today?
>> >
>> > Meanwhile asking is not a problem, at least we will know for sure.
>> > And yes, leave it as is, but point to the thread where you asking
>> > the clarification.
>>
>> For the record:
>>
>> $ git grep -n -A1 -w DEFINE_DEBUGFS_ATTRIBUTE | grep ');' | sed 
>> 's,.*\(".*%.*"\).*,\1,' | sort | uniq -c
>>   1 "%08llx\n"
>>   5 "0x%016llx\n"
>>   5 "0x%02llx\n"
>>   5 "0x%04llx\n"
>>  13 "0x%08llx\n"
>>   1 "0x%4.4llx\n"
>>   3 "0x%.4llx\n"
>>   4 "0x%llx\n"
>>   1 "%1lld\n"
>>  40 "%lld\n"
>>   2 "%lli\n"
>> 129 "%llu\n"
>>   1 "%#llx\n"
>>   2 "%llx\n"
>>
>> means that sscanf() should work and fix the issue. You may even 
>> propose a patch
>> as a starter for a discussion.


I proposed a patch with the fix you suggested.

--
Thanks, Eliav

Re: [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:48PM +0000, Eliav Farber wrote:
> This change adds debugfs to read and write temperature sensor coefficients
> - g, h, j and cal5.
> 
> The coefficients can vary between product and product, so it can be very
> useful to be able to modify them on the fly during the calibration
> process.
> 
> e.g.:
> 
> cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5
> 4096
> 
> echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>

Applied to hwmon-next.

Thanks,
Guenter

[PATCH v5 21/21] hwmon: (mr75203) fix coding style space errors

[Eliav Farber]

Fix: "ERROR: space required before the open parenthesis '('"

All of the errors were introduced before this series of patches.

Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V5 -> V4:
- 0021: Move this patch to be last in the series (it does not fix the code),
  and remove the "Fixes:" tag.

 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 e6b49f810307..0fea443dc3e1 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -290,7 +290,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;
@@ -331,7 +331,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;
@@ -437,19 +437,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,
@@ -462,7 +462,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,
@@ -475,7 +475,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,
@@ -489,39 +489,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,
@@ -548,7 +548,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,
@@ -561,7 +561,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,
@@ -575,7 +575,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;
 	}
 
@@ -832,7 +832,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

Re: [PATCH v5 21/21] hwmon: (mr75203) fix coding style space errors

[Guenter Roeck]

On Thu, Sep 08, 2022 at 03:24:49PM +0000, Eliav Farber wrote:
> Fix: "ERROR: space required before the open parenthesis '('"
> 
> All of the errors were introduced before this series of patches.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to hwmon-next.

Thanks,
Guenter