On Fri, 11 Nov 2022, Naresh Solanki wrote:Patrick & Marcello worked on the patch. I'm upstreaming it. Will update it as:
On 07-11-2022 03:06 pm, Lee Jones wrote:
On Thu, 03 Nov 2022, Naresh Solanki wrote:
From: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx>
Implement a regulator driver with IRQ support for fault management.
This is not a "regulator driver".
Written against documentation [1] and [2] and tested on real hardware.
Every channel has its own regulator supplies nammed 'vss1-supply' and
'vss2-supply'. The regulator supply is used to determine the output
voltage, as the smart switch provides no output regulation.
The driver requires the 'shunt-resistor-micro-ohms' property to be
present in Device Tree to properly calculate current related
values.
Datasheet links:
1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
Signed-off-by: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx>
Signed-off-by: Marcello Sylvester Bauer <sylv@xxxxxxx>
Signed-off-by: Naresh Solanki <Naresh.Solanki@xxxxxxxxxxxxx>
[...]
Yes. Will remove that in next version.The .data isn't used.+static const struct of_device_id max597x_of_match[] = {
+ { .compatible = "maxim,max5970", .data = (void *)MAX597x_TYPE_MAX5970 },
+ { .compatible = "maxim,max5978", .data = (void *)MAX597x_TYPE_MAX5978 },
Where is .data used?
Then why add it?
Sure will remove.
[...]
You mean this comment should be removed ?+#include <linux/device.h>
+#include <linux/regmap.h>
+
+/* Number of switch based on chip variant */
This comment is superfluous.
I do.
Yes you are right. And thats how its being used.
MAX5978 & single power switch wheres MAX5970 has two.+#define MAX5970_NUM_SWITCHES 2
+#define MAX5978_NUM_SWITCHES 1
+/* Both chip variant have 4 indication LEDs used by LED cell */
Here too I think.
+#define MAX597X_NUM_LEDS 4
+
+enum max597x_chip_type {
+ MAX597x_TYPE_MAX5978 = 1,
Why 1?
That's not what this enum means.
You are just describing the type to be matched on.
The value should be arbitrary, no?
The respective cell regulator/iio & led driver.
[...]
Shared reg.+#define OC_STATUS_WARN(ch) BIT(ch)
+
+#define MAX5970_REG_CHXEN 0x3b
+#define CHXEN(ch) (3 << (ch * 2))
+
+#define MAX5970_REG_LED_FLASH 0x43
Do these all need to be shared?
Or can they be isolated into, say, the Regulator driver?
Where else are they used?