Hi Lee,
On 14-11-2022 02:41 pm, Lee Jones wrote:
On Fri, 11 Nov 2022, Naresh Solanki wrote:
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>
[...]
Patrick & Marcello worked on the patch. I'm upstreaming it. Will update
it as:
Signed-off-by: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx>
Co-Developed-by: Marcello Sylvester Bauer <sylv@xxxxxxx>
Signed-off-by: Naresh Solanki <Naresh.Solanki@xxxxxxxxxxxxx>
ok?
+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?
The .data isn't used.
Then why add it?
Yes. Will remove that in next version.
[...]
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+/* Number of switch based on chip variant */
This comment is superfluous.
You mean this comment should be removed ?
I do.
Sure will remove.
+#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?
MAX5978 & single power switch wheres MAX5970 has two.
That's not what this enum means.
You are just describing the type to be matched on.
The value should be arbitrary, no?
Yes you are right. And thats how its being used.
Setting the first enum value to 1 was to avoid zero.
[...]
+#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?
Shared reg.
Where else are they used?
The respective cell regulator/iio & led driver.
Regards,
Naresh
