Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 3/3] pmdomain: arm_scmi: add support for domain hierarchies
From: Kevin Hilman (TI) @ 2026-04-10 23:44 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: Geert Uytterhoeven, linux-pm, devicetree, linux-kernel, arm-scmi,
	linux-arm-kernel
In-Reply-To: <20260410-topic-lpm-pmdomain-child-ids-v2-0-83396e4b5f8b@baylibre.com>

After primary SCMI pmdomain is created, use new of_genpd helper which
checks for child domain mappings defined in power-domains-child-ids.

Also remove any child domain mappings when SCMI domain is removed.

Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 drivers/pmdomain/arm/scmi_pm_domain.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/arm/scmi_pm_domain.c b/drivers/pmdomain/arm/scmi_pm_domain.c
index b5e2ffd5ea64..6d33b3d62ef3 100644
--- a/drivers/pmdomain/arm/scmi_pm_domain.c
+++ b/drivers/pmdomain/arm/scmi_pm_domain.c
@@ -114,6 +114,14 @@ static int scmi_pm_domain_probe(struct scmi_device *sdev)
 
 	dev_set_drvdata(dev, scmi_pd_data);
 
+	/*
+	 * Parse (optional) power-domains-child-ids property to
+	 * establish parent-child relationships
+	 */
+	ret = of_genpd_add_child_ids(np, scmi_pd_data);
+	if (ret < 0 && ret != -ENOENT)
+		dev_err(dev, "Failed to add child domain hierarchy: %d\n", ret);
+
 	return 0;
 err_rm_genpds:
 	for (i = num_domains - 1; i >= 0; i--)
@@ -129,9 +137,13 @@ static void scmi_pm_domain_remove(struct scmi_device *sdev)
 	struct device *dev = &sdev->dev;
 	struct device_node *np = dev->of_node;
 
+	scmi_pd_data = dev_get_drvdata(dev);
+
+	/* Remove any parent-child relationships established at probe time */
+	of_genpd_remove_child_ids(np, scmi_pd_data);
+
 	of_genpd_del_provider(np);
 
-	scmi_pd_data = dev_get_drvdata(dev);
 	for (i = 0; i < scmi_pd_data->num_domains; i++) {
 		if (!scmi_pd_data->domains[i])
 			continue;

-- 
2.51.0


^ permalink raw reply related

* [PATCH v2 2/3] pmdomain: core: add support for power-domains-child-ids
From: Kevin Hilman (TI) @ 2026-04-10 23:44 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: Geert Uytterhoeven, linux-pm, devicetree, linux-kernel, arm-scmi,
	linux-arm-kernel
In-Reply-To: <20260410-topic-lpm-pmdomain-child-ids-v2-0-83396e4b5f8b@baylibre.com>

Currently, PM domains can only support hierarchy for simple
providers (e.g. ones with #power-domain-cells = 0).

Add support for oncell providers as well by adding a new property
`power-domains-child-ids` to describe the parent/child relationship.

For example, an SCMI PM domain provider has multiple domains, each of
which might be a child of diffeent parent domains. In this example,
the parent domains are MAIN_PD and WKUP_PD:

    scmi_pds: protocol@11 {
        reg = <0x11>;
        #power-domain-cells = <1>;
        power-domains = <&MAIN_PD>, <&WKUP_PD>;
        power-domains-child-ids = <15>, <19>;
    };

With this example using the new property, SCMI PM domain 15 becomes a
child domain of MAIN_PD, and SCMI domain 19 becomes a child domain of
WKUP_PD.

To support this feature, add two new core functions

- of_genpd_add_child_ids()
- of_genpd_remove_child_ids()

which can be called by pmdomain providers to add/remove child domains
if they support the new property power-domains-child-ids.

The add function is "all or nothing".  If it cannot add all of the
child domains in the list, it will unwind any additions already made
and report a failure.

Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 drivers/pmdomain/core.c   | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h |  16 ++++++++++++++++
 2 files changed, 182 insertions(+)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 61c2277c9ce3..f978477dd546 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2909,6 +2909,172 @@ static struct generic_pm_domain *genpd_get_from_provider(
 	return genpd;
 }
 
+/**
+ * of_genpd_add_child_ids() - Parse power-domains-child-ids property
+ * @np: Device node pointer associated with the PM domain provider.
+ * @data: Pointer to the onecell data associated with the PM domain provider.
+ *
+ * Parse the power-domains and power-domains-child-ids properties to establish
+ * parent-child relationships for PM domains. The power-domains property lists
+ * parent domains, and power-domains-child-ids lists which child domain IDs
+ * should be associated with each parent.
+ *
+ * Uses "all or nothing" semantics: either all relationships are established
+ * successfully, or none are (any partially-added relationships are unwound
+ * on error).
+ *
+ * Returns 0 on success, -ENOENT if properties don't exist, or negative error code.
+ */
+int of_genpd_add_child_ids(struct device_node *np,
+			   struct genpd_onecell_data *data)
+{
+	struct of_phandle_args parent_args;
+	struct generic_pm_domain *parent_genpd, *child_genpd;
+	struct generic_pm_domain **pairs; /* pairs[2*i]=parent, pairs[2*i+1]=child */
+	u32 child_id;
+	int i, ret, count, child_count, added = 0;
+
+	/* Check if both properties exist */
+	count = of_count_phandle_with_args(np, "power-domains", "#power-domain-cells");
+	if (count <= 0)
+		return -ENOENT;
+
+	child_count = of_property_count_u32_elems(np, "power-domains-child-ids");
+	if (child_count < 0)
+		return -ENOENT;
+	if (child_count != count)
+		return -EINVAL;
+
+	/* Allocate tracking array for error unwind (parent/child pairs) */
+	pairs = kmalloc_array(count * 2, sizeof(*pairs), GFP_KERNEL);
+	if (!pairs)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		ret = of_property_read_u32_index(np, "power-domains-child-ids",
+						 i, &child_id);
+		if (ret)
+			goto err_unwind;
+
+		/* Validate child ID is within bounds */
+		if (child_id >= data->num_domains) {
+			pr_err("Child ID %u out of bounds (max %u) for %pOF\n",
+			       child_id, data->num_domains - 1, np);
+			ret = -EINVAL;
+			goto err_unwind;
+		}
+
+		/* Get the child domain */
+		child_genpd = data->domains[child_id];
+		if (!child_genpd) {
+			pr_err("Child domain %u is NULL for %pOF\n", child_id, np);
+			ret = -EINVAL;
+			goto err_unwind;
+		}
+
+		ret = of_parse_phandle_with_args(np, "power-domains",
+						 "#power-domain-cells", i,
+						 &parent_args);
+		if (ret)
+			goto err_unwind;
+
+		/* Get the parent domain */
+		parent_genpd = genpd_get_from_provider(&parent_args);
+		of_node_put(parent_args.np);
+		if (IS_ERR(parent_genpd)) {
+			pr_err("Failed to get parent domain for %pOF: %ld\n",
+			       np, PTR_ERR(parent_genpd));
+			ret = PTR_ERR(parent_genpd);
+			goto err_unwind;
+		}
+
+		/* Establish parent-child relationship */
+		ret = pm_genpd_add_subdomain(parent_genpd, child_genpd);
+		if (ret) {
+			pr_err("Failed to add child domain %u to parent in %pOF: %d\n",
+			       child_id, np, ret);
+			goto err_unwind;
+		}
+
+		/* Track for potential unwind */
+		pairs[2 * added] = parent_genpd;
+		pairs[2 * added + 1] = child_genpd;
+		added++;
+
+		pr_debug("Added child domain %u (%s) to parent %s for %pOF\n",
+			 child_id, child_genpd->name, parent_genpd->name, np);
+	}
+
+	kfree(pairs);
+	return 0;
+
+err_unwind:
+	/* Reverse all previously established relationships */
+	while (added-- > 0)
+		pm_genpd_remove_subdomain(pairs[2 * added], pairs[2 * added + 1]);
+	kfree(pairs);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_genpd_add_child_ids);
+
+/**
+ * of_genpd_remove_child_ids() - Remove parent-child PM domain relationships
+ * @np: Device node pointer associated with the PM domain provider.
+ * @data: Pointer to the onecell data associated with the PM domain provider.
+ *
+ * Reverses the effect of of_genpd_add_child_ids() by parsing the same
+ * power-domains and power-domains-child-ids properties and calling
+ * pm_genpd_remove_subdomain() for each established relationship.
+ *
+ * Returns 0 on success, -ENOENT if properties don't exist, or negative error
+ * code on failure.
+ */
+int of_genpd_remove_child_ids(struct device_node *np,
+			   struct genpd_onecell_data *data)
+{
+	struct of_phandle_args parent_args;
+	struct generic_pm_domain *parent_genpd, *child_genpd;
+	u32 child_id;
+	int i, ret, count, child_count;
+
+	/* Check if both properties exist */
+	count = of_count_phandle_with_args(np, "power-domains", "#power-domain-cells");
+	if (count <= 0)
+		return -ENOENT;
+
+	child_count = of_property_count_u32_elems(np, "power-domains-child-ids");
+	if (child_count < 0)
+		return -ENOENT;
+	if (child_count != count)
+		return -EINVAL;
+
+	for (i = 0; i < count; i++) {
+		if (of_property_read_u32_index(np, "power-domains-child-ids",
+					       i, &child_id))
+			continue;
+
+		if (child_id >= data->num_domains || !data->domains[child_id])
+			continue;
+
+		ret = of_parse_phandle_with_args(np, "power-domains",
+						 "#power-domain-cells", i,
+						 &parent_args);
+		if (ret)
+			continue;
+
+		parent_genpd = genpd_get_from_provider(&parent_args);
+		of_node_put(parent_args.np);
+		if (IS_ERR(parent_genpd))
+			continue;
+
+		child_genpd = data->domains[child_id];
+		pm_genpd_remove_subdomain(parent_genpd, child_genpd);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_genpd_remove_child_ids);
+
 /**
  * of_genpd_add_device() - Add a device to an I/O PM domain
  * @genpdspec: OF phandle args to use for look-up PM domain
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f67a2cb7d781..b44615d79af6 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -465,6 +465,10 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
 int of_genpd_parse_idle_states(struct device_node *dn,
 			       struct genpd_power_state **states, int *n);
 void of_genpd_sync_state(struct device_node *np);
+int of_genpd_add_child_ids(struct device_node *np,
+			   struct genpd_onecell_data *data);
+int of_genpd_remove_child_ids(struct device_node *np,
+			      struct genpd_onecell_data *data);
 
 int genpd_dev_pm_attach(struct device *dev);
 struct device *genpd_dev_pm_attach_by_id(struct device *dev,
@@ -534,6 +538,18 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 {
 	return ERR_PTR(-EOPNOTSUPP);
 }
+
+static inline int of_genpd_add_child_ids(struct device_node *np,
+					 struct genpd_onecell_data *data)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int of_genpd_remove_child_ids(struct device_node *np,
+					    struct genpd_onecell_data *data)
+{
+	return -EOPNOTSUPP;
+}
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 #ifdef CONFIG_PM

-- 
2.51.0


^ permalink raw reply related

* [PATCH v2 1/3] dt-bindings: power: Add power-domains-child-ids property
From: Kevin Hilman (TI) @ 2026-04-10 23:44 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: Geert Uytterhoeven, linux-pm, devicetree, linux-kernel, arm-scmi,
	linux-arm-kernel
In-Reply-To: <20260410-topic-lpm-pmdomain-child-ids-v2-0-83396e4b5f8b@baylibre.com>

Add binding documentation for the new power-domains-child-ids property,
which works in conjunction with the existing power-domains property to
establish parent-child relationships between a multi-domain power domain
provider and external parent domains.

Each element in the uint32 array identifies the child domain
ID (index) within the provider that should be made a child domain of
the corresponding phandle entry in power-domains. The two arrays must
have the same number of elements.

Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
 Documentation/devicetree/bindings/power/power-domain.yaml | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/power-domain.yaml b/Documentation/devicetree/bindings/power/power-domain.yaml
index b1147dbf2e73..163b0af158fd 100644
--- a/Documentation/devicetree/bindings/power/power-domain.yaml
+++ b/Documentation/devicetree/bindings/power/power-domain.yaml
@@ -68,6 +68,21 @@ properties:
       by the given provider should be subdomains of the domain specified
       by this binding.
 
+  power-domains-child-ids:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      An array of child domain IDs that correspond to the power-domains
+      property. This property is only applicable to power domain providers
+      with "#power-domain-cells" > 0 (i.e., providers that supply multiple
+      power domains). It specifies which of the provider's child domains
+      should be associated with each parent domain listed in the power-domains
+      property. The number of elements in this array must match the number of
+      phandles in the power-domains property. Each element specifies the child
+      domain ID (index) that should be made a child domain of the corresponding
+      parent domain. This enables hierarchical power domain structures where
+      different child domains from the same provider can have different
+      parent domains.
+
 required:
   - "#power-domain-cells"
 
@@ -133,3 +148,22 @@ examples:
             min-residency-us = <7000>;
         };
     };
+
+  - |
+    // Example: SCMI domain 15 -> MAIN_PD, SCMI domain 19 -> WKUP_PD
+    MAIN_PD: power-controller-main {
+        compatible = "foo,power-controller";
+        #power-domain-cells = <0>;
+    };
+
+    WKUP_PD: power-controller-wkup {
+        compatible = "foo,power-controller";
+        #power-domain-cells = <0>;
+    };
+
+    scmi_pds: power-controller-scmi {
+        compatible = "foo,power-controller";
+        #power-domain-cells = <1>;
+        power-domains = <&MAIN_PD>, <&WKUP_PD>;
+        power-domains-child-ids = <15>, <19>;
+    };

-- 
2.51.0


^ permalink raw reply related

* [PATCH v2 0/3] pmdomain: core: add support for domain hierarchies in DT
From: Kevin Hilman (TI) @ 2026-04-10 23:44 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: Geert Uytterhoeven, linux-pm, devicetree, linux-kernel, arm-scmi,
	linux-arm-kernel

Currently, PM domains can only support hierarchy for simple
providers (e.g. ones with #power-domain-cells = 0).

Add support for oncell providers as well by adding a new property
`power-domains-child-ids` to describe the parent/child relationship.

Also adds the first user of the new API: the Arm SCMI PM domain driver.

Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
---
This idea was previously discussed on the arm-scmi mailing list[1]
where this approach was proposed by Ulf, and then an initial RFC[2]
implementation was made.  From there, it was suggested by Rob[3] to
use a nexus node map instead, which led to several more versions
attempting to implement that, culminating in v5[4], where Rob and
Geert then had second thoughts about the power-domain-map approach.

Therefore, I've gone back to the approach in the initial RFC[2] to use
the child-ids approach.

Changes compared to initial RFC[2]
- dropped RFC
- rewrote the parse/add function to use iterators/helpers from of.h
- add a remove function for cleanup
- use child domain language instead of subdomain

[1] https://lore.kernel.org/arm-scmi/CAPDyKFo_P129sVirHHYjOQT+QUmpymcRJme9obzKJeRgO7B-1A@mail.gmail.com/
[2] https://lore.kernel.org/all/20250528-pmdomain-hierarchy-onecell-v1-1-851780700c68@baylibre.com/
[3] https://lore.kernel.org/all/20250528203532.GA704342-robh@kernel.org/
[4] https://lore.kernel.org/r/20260122-pmdomain-hierarchy-onecell-v5-0-76855ec856bd@baylibre.com

Changes in v2:
- dt-bindings: fix warinings from make dt_binding_check
- scmi_pm_domain: switch to dev_err()
- pmdomain: core: fix locking around add/remove domains
- pmdomain: error unwind if any children fail to be added
- pmdomain: fix node reference leak
- pmdomain: ensure power-domains and child-ids properties are same
  length before iterating
- Link to v1: https://patch.msgid.link/20260310-topic-lpm-pmdomain-child-ids-v1-0-5361687a18ff@baylibre.com

---
Kevin Hilman (TI) (3):
      dt-bindings: power: Add power-domains-child-ids property
      pmdomain: core: add support for power-domains-child-ids
      pmdomain: arm_scmi: add support for domain hierarchies

 Documentation/devicetree/bindings/power/power-domain.yaml |  34 ++++++++++++++++++++++++++++++++++
 drivers/pmdomain/arm/scmi_pm_domain.c                     |  14 +++++++++++++-
 drivers/pmdomain/core.c                                   | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h                                 |  16 ++++++++++++++++
 4 files changed, 229 insertions(+), 1 deletion(-)
---
base-commit: f7b88edb52c8dd01b7e576390d658ae6eef0e134
change-id: 20260310-topic-lpm-pmdomain-child-ids-e3d57ae57040

Best regards,
--  
Kevin Hilman (TI) <khilman@baylibre.com>


^ permalink raw reply

* Re: [GIT PULL] RISC-V SpacemiT Devicetrees for v7.1
From: Yixun Lan @ 2026-04-10 23:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: soc, Yixun Lan, Arnd Bergmann, spacemit, linux-riscv, devicetree,
	linux-kernel
In-Reply-To: <CAD++jLkQfUwhvDOCkhK2NtRyXrb6y9rwpDckR6h0G3+dzYwW3A@mail.gmail.com>

Hi Linus,

On 00:19 Sat 11 Apr     , Linus Walleij wrote:
> Hi Yixun,
> 
> I looked into this pull request.
> 
> I'm sorry if I do stupid mistakes in handling it, I'm new to maintaining
> the SoC tree. Bear with me.
> 
> On Fri, Apr 3, 2026 at 2:32 PM Yixun Lan <dlan@kernel.org> wrote:
> 
> > Aurelien Jarno (7):
> >       riscv: dts: spacemit: drop incorrect pinctrl for combo PHY
> (...)
> > Yixun Lan (9):
> >       riscv: dts: spacemit: pcie: fix missing power regulator
> 
> [Fixes]
> fatal: Not a valid object name linus/master
Could be the problem that your master branch isn't up-to-date?

> Commit: c68360c0d636 ("riscv: dts: spacemit: drop incorrect pinctrl
> for combo PHY")
..
>     Fixes tag: Fixes: 0be016a4b5d1b9 ("riscv: dts: spacemit: PCIe and
> PHY-related updates")
Above commit was merged for v7.0 cycle, while this PR is for v7.1
The fix isn't that critical and didn't cause any run-time issue, so
I do the fix in this v7.1 cycle..

>     Has these problem(s):
>         - Inspect: Target SHA is not ancestor of Linus' master branch,
> which means it is fixing commit in your branch
> fatal: Not a valid object name linus/master
> Commit: 8a9071299dec ("riscv: dts: spacemit: pcie: fix missing power regulator")
>     Fixes tag: Fixes: 0be016a4b5d1 ("riscv: dts: spacemit: PCIe and
> PHY-related updates")
>     Has these problem(s):
>         - Inspect: Target SHA is not ancestor of Linus' master branch,
> which means it is fixing commit in your branch
> 
> So this means you introduced bugs and fix them in the same pull request?
> 
> Why?
> 
> The practice is to squash such fixes into the offending patches when
> presenting pull requests. But I went ahead anyway, trying to not be so
> picky. (The commits are there, in your branch indeed.)
> 
Glad to learn this, I will keep it in mind..

> - Checked that it was in linux-next OK
Right, I think the commit is actually in v7.0-rc1

> - built DTBS OK
> 
> Pulled in, thanks.
Thank you

> 
> Yours,
> Linus Walleij

-- 
Yixun Lan (dlan)

^ permalink raw reply

* Re: [PATCH v7 0/8] hwmon: (ina3221) Various improvement and add support for SQ52210
From: Guenter Roeck @ 2026-04-10 23:30 UTC (permalink / raw)
  To: Wenliang Yan, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Jonathan Corbet, linux-hwmon, devicetree, linux-kernel
In-Reply-To: <20260402081350.65559-1-wenliang202407@163.com>

Hi,

On 4/2/26 01:13, Wenliang Yan wrote:
> Changes in v7:
> - Fixed unnecessary semicolon in ina3221_read_value()
>    (reported by kernel test robot)
> 
> I will address any additional feedback in the next version.
> Thank you for your time
> 
Sashiko reports lots of issues with this series. Some of it is irrelevant
(for example, enum values for chip types are always lower case in the hwmon subsystem),
but many are real problems. Please take a look.

Thanks,
Guenter

> ---
> v6: https://lore.kernel.org/linux-hwmon/20260225090324.112145-1-wenliang202407@163.com/
> v5: https://lore.kernel.org/linux-hwmon/20260119121446.17469-1-wenliang202407@163.com/
> v4: https://lore.kernel.org/linux-hwmon/20260114081741.111340-1-wenliang202407@163.com/
> v3: https://lore.kernel.org/linux-hwmon/20251120081921.39412-1-wenliang202407@163.com/
> v2: https://lore.kernel.org/linux-hwmon/20251118125148.95603-1-wenliang202407@163.com/
> v1: https://lore.kernel.org/linux-hwmon/20251111080546.32421-1-wenliang202407@163.com/
> 
> Wenliang Yan (8):
>    dt-bindings: hwmon: ti,ina3221: Add SQ52210
>    hwmon: (ina3221) Add support for SQ52210
>    hwmon: (ina3221) Pre-calculate current and power LSB
>    hwmon: (ina3221) Support alert configuration
>    hwmon: (ina3221) Introduce power attribute and alert characteristics
>    hwmon: (ina3221) Modify the 'ina3221_read_value' function
>    hwmon: (ina3221) Support alert_limit_write function and write/read
>      functions for 'power' attribute
>    hwmon: (ina3221) Modify write/read functions for 'in' and 'curr'
>      attribute
> 
>   .../devicetree/bindings/hwmon/ti,ina3221.yaml |  15 +-
>   Documentation/hwmon/ina3221.rst               |  24 +
>   drivers/hwmon/ina3221.c                       | 548 +++++++++++++++++-
>   3 files changed, 571 insertions(+), 16 deletions(-)
> 


^ permalink raw reply

* Re: [PATCH v9 2/3] hwmon: ltc4283: Add support for the LTC4283 Swap Controller
From: Guenter Roeck @ 2026-04-10 23:27 UTC (permalink / raw)
  To: nuno.sa, linux-gpio, linux-hwmon, devicetree, linux-doc
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Linus Walleij, Bartosz Golaszewski
In-Reply-To: <20260406-ltc4283-support-v9-2-b66cfc749261@analog.com>

On 4/6/26 07:31, Nuno Sá via B4 Relay wrote:
> From: Nuno Sá <nuno.sa@analog.com>
> 
> Support the LTC4283 Hot Swap Controller. The device features programmable
> current limit with foldback and independently adjustable inrush current to
> optimize the MOSFET safe operating area (SOA). The SOA timer limits MOSFET
> temperature rise for reliable protection against overstresses.
> 
> An I2C interface and onboard ADC allow monitoring of board current,
> voltage, power, energy, and fault status.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

The patch still has some issues. Please see

https://sashiko.dev/#/patchset/20260406-ltc4283-support-v9-0-b66cfc749261%40analog.com

Specifically:

- regmap_clear_bits() may not cause problems, but it is not the best
   choice either because the register was already read.
   It might be better to just write the value to be masked since
   both the register value and the mask are known.

- I can't comment on the energy accuracy lost. That is your call.

- Clamping before multiplying is indeed wrong.
   You'll need to clamp before multiplying (and then possibly
   clamp again).

-  %*ph: The AI seems to have a point.

- debugfs: False positive. I'll need to check if the guidance ever made it into the
   Agent's prompts.

Thanks,
Guenter

> ---
>   Documentation/hwmon/index.rst   |    1 +
>   Documentation/hwmon/ltc4283.rst |  266 ++++++
>   MAINTAINERS                     |    1 +
>   drivers/hwmon/Kconfig           |   12 +
>   drivers/hwmon/Makefile          |    1 +
>   drivers/hwmon/ltc4283.c         | 1808 +++++++++++++++++++++++++++++++++++++++
>   6 files changed, 2089 insertions(+)
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 199f35a75282..d54dda83ab6e 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -144,6 +144,7 @@ Hardware Monitoring Kernel Drivers
>      ltc4260
>      ltc4261
>      ltc4282
> +   ltc4283
>      ltc4286
>      macsmc-hwmon
>      max127
> diff --git a/Documentation/hwmon/ltc4283.rst b/Documentation/hwmon/ltc4283.rst
> new file mode 100644
> index 000000000000..ba88445e45f4
> --- /dev/null
> +++ b/Documentation/hwmon/ltc4283.rst
> @@ -0,0 +1,266 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Kernel drivers ltc4283
> +==========================================
> +
> +Supported chips:
> +
> +  * Analog Devices LTC4283
> +
> +    Prefix: 'ltc4283'
> +
> +    Addresses scanned: -
> +
> +    Datasheet:
> +
> +        https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4283.pdf
> +
> +Author: Nuno Sá <nuno.sa@analog.com>
> +
> +Description
> +___________
> +
> +The LTC4283 negative voltage hot swap controller drives an external N-channel
> +MOSFET to allow a board to be safely inserted and removed from a live backplane.
> +The device features programmable current limit with foldback and independently
> +adjustable inrush current to optimize the MOSFET safe operating area (SOA). The
> +SOA timer limits MOSFET temperature rise for reliable protection against
> +overstresses. An I2C interface and onboard gear-shift ADC allow monitoring of
> +board current, voltage, power, energy, and fault status.  Additional features
> +respond to input UV/OV, interrupt the host when a fault has occurred, notify
> +when output power is good, detect insertion of a board, turn off the MOSFET
> +if an external supply monitor fails to indicate power good within a timeout
> +period, and auto-reboot after a programmable delay following a host commanded
> +turn-off.
> +
> +Sysfs entries
> +_____________
> +
> +The following attributes are supported. Limits are read-write and all the other
> +attributes are read-only. Note that the VADIOx channels might not be available
> +if the ADIO pins are used as GPIOs (naturally also affects the respective
> +differential channels).
> +
> +======================= ==========================================
> +in0_lcrit_alarm         Critical Undervoltage alarm
> +in0_crit_alarm          Critical Overvoltage alarm
> +in0_label		Channel label (VIN)
> +
> +in1_input		Output voltage (mV).
> +in1_min			Undervoltage threshold
> +in1_max			Overvoltage threshold
> +in1_lowest		Lowest measured voltage
> +in1_highest		Highest measured voltage
> +in1_reset_history	Write 1 to reset history.
> +in1_min_alarm		Undervoltage alarm
> +in1_max_alarm		Overvoltage alarm
> +in1_label		Channel label (VPWR)
> +
> +in2_input		Output voltage (mV).
> +in2_min			Undervoltage threshold
> +in2_max			Overvoltage threshold
> +in2_lowest		Lowest measured voltage
> +in2_highest		Highest measured voltage
> +in2_reset_history	Write 1 to reset history.
> +in2_min_alarm		Undervoltage alarm
> +in2_max_alarm		Overvoltage alarm
> +in2_enable		Enable/Disable monitoring.
> +in2_label		Channel label (VADI1)
> +
> +in3_input		Output voltage (mV).
> +in3_min			Undervoltage threshold
> +in3_max			Overvoltage threshold
> +in3_lowest		Lowest measured voltage
> +in3_highest		Highest measured voltage
> +in3_reset_history	Write 1 to reset history.
> +in3_min_alarm		Undervoltage alarm
> +in3_max_alarm		Overvoltage alarm
> +in3_enable		Enable/Disable monitoring.
> +in3_label		Channel label (VADI2)
> +
> +in4_input		Output voltage (mV).
> +in4_min			Undervoltage threshold
> +in4_max			Overvoltage threshold
> +in4_lowest		Lowest measured voltage
> +in4_highest		Highest measured voltage
> +in4_reset_history	Write 1 to reset history.
> +in4_min_alarm		Undervoltage alarm
> +in4_max_alarm		Overvoltage alarm
> +in4_enable		Enable/Disable monitoring.
> +in4_label		Channel label (VADI3)
> +
> +in5_input		Output voltage (mV).
> +in5_min			Undervoltage threshold
> +in5_max			Overvoltage threshold
> +in5_lowest		Lowest measured voltage
> +in5_highest		Highest measured voltage
> +in5_reset_history	Write 1 to reset history.
> +in5_min_alarm		Undervoltage alarm
> +in5_max_alarm		Overvoltage alarm
> +in5_enable		Enable/Disable monitoring.
> +in5_label		Channel label (VADI4)
> +
> +in6_input		Output voltage (mV).
> +in6_min			Undervoltage threshold
> +in6_max			Overvoltage threshold
> +in6_lowest		Lowest measured voltage
> +in6_highest		Highest measured voltage
> +in6_reset_history	Write 1 to reset history.
> +in6_min_alarm		Undervoltage alarm
> +in6_max_alarm		Overvoltage alarm
> +in6_enable		Enable/Disable monitoring.
> +in6_label		Channel label (VADIO1)
> +
> +in7_input		Output voltage (mV).
> +in7_min			Undervoltage threshold
> +in7_max			Overvoltage threshold
> +in7_lowest		Lowest measured voltage
> +in7_highest		Highest measured voltage
> +in7_reset_history	Write 1 to reset history.
> +in7_min_alarm		Undervoltage alarm
> +in7_max_alarm		Overvoltage alarm
> +in7_enable		Enable/Disable monitoring.
> +in7_label		Channel label (VADIO2)
> +
> +in8_input		Output voltage (mV).
> +in8_min			Undervoltage threshold
> +in8_max			Overvoltage threshold
> +in8_lowest		Lowest measured voltage
> +in8_highest		Highest measured voltage
> +in8_reset_history	Write 1 to reset history.
> +in8_min_alarm		Undervoltage alarm
> +in8_max_alarm		Overvoltage alarm
> +in8_enable		Enable/Disable monitoring.
> +in8_label		Channel label (VADIO3)
> +
> +in9_input		Output voltage (mV).
> +in9_min			Undervoltage threshold
> +in9_max			Overvoltage threshold
> +in9_lowest		Lowest measured voltage
> +in9_highest		Highest measured voltage
> +in9_reset_history	Write 1 to reset history.
> +in9_min_alarm		Undervoltage alarm
> +in9_max_alarm		Overvoltage alarm
> +in9_enable		Enable/Disable monitoring.
> +in9_label		Channel label (VADIO4)
> +
> +in10_input		Output voltage (mV).
> +in10_min		Undervoltage threshold
> +in10_max		Overvoltage threshold
> +in10_lowest		Lowest measured voltage
> +in10_highest		Highest measured voltage
> +in10_reset_history	Write 1 to reset history.
> +in10_min_alarm		Undervoltage alarm
> +in10_max_alarm		Overvoltage alarm
> +in10_enable		Enable/Disable monitoring.
> +in10_label		Channel label (DRNS)
> +
> +in11_input		Output voltage (mV).
> +in11_min		Undervoltage threshold
> +in11_max		Overvoltage threshold
> +in11_lowest		Lowest measured voltage
> +in11_highest		Highest measured voltage
> +in11_reset_history	Write 1 to reset history.
> +			Also clears fet bad and short fault logs.
> +in11_min_alarm		Undervoltage alarm
> +in11_max_alarm		Overvoltage alarm
> +in11_enable		Enable/Disable monitoring
> +in11_fault		Failure in the MOSFET. Either bad or shorted FET.
> +in11_label		Channel label (DRAIN)
> +
> +in12_input		Output voltage (mV).
> +in12_min		Undervoltage threshold
> +in12_max		Overvoltage threshold
> +in12_lowest		Lowest measured voltage
> +in12_highest		Highest measured voltage
> +in12_reset_history	Write 1 to reset history.
> +in12_min_alarm		Undervoltage alarm
> +in12_max_alarm		Overvoltage alarm
> +in12_enable		Enable/Disable monitoring.
> +in12_label		Channel label (ADIN2-ADIN1)
> +
> +in13_input		Output voltage (mV).
> +in13_min		Undervoltage threshold
> +in13_max		Overvoltage threshold
> +in13_lowest		Lowest measured voltage
> +in13_highest		Highest measured voltage
> +in13_reset_history	Write 1 to reset history.
> +in13_min_alarm		Undervoltage alarm
> +in13_max_alarm		Overvoltage alarm
> +in13_enable		Enable/Disable monitoring.
> +in13_label		Channel label (ADIN4-ADIN3)
> +
> +in14_input		Output voltage (mV).
> +in14_min		Undervoltage threshold
> +in14_max		Overvoltage threshold
> +in14_lowest		Lowest measured voltage
> +in14_highest		Highest measured voltage
> +in14_reset_history	Write 1 to reset history.
> +in14_min_alarm		Undervoltage alarm
> +in14_max_alarm		Overvoltage alarm
> +in14_enable		Enable/Disable monitoring.
> +in14_label		Channel label (ADIO2-ADIO1)
> +
> +in15_input		Output voltage (mV).
> +in15_min		Undervoltage threshold
> +in15_max		Overvoltage threshold
> +in15_lowest		Lowest measured voltage
> +in15_highest		Highest measured voltage
> +in15_reset_history	Write 1 to reset history.
> +in15_min_alarm		Undervoltage alarm
> +in15_max_alarm		Overvoltage alarm
> +in15_enable		Enable/Disable monitoring.
> +in15_label		Channel label (ADIO4-ADIO3)
> +
> +curr1_input		Sense current (mA)
> +curr1_min		Undercurrent threshold
> +curr1_max		Overcurrent threshold
> +curr1_lowest		Lowest measured current
> +curr1_highest		Highest measured current
> +curr1_reset_history	Write 1 to reset curr1 history.
> +			Also clears overcurrent fault logs.
> +curr1_min_alarm		Undercurrent alarm
> +curr1_max_alarm		Overcurrent alarm
> +curr1_crit_alarm        Critical Overcurrent alarm
> +curr1_label		Channel label (ISENSE)
> +
> +power1_input		Power (in uW)
> +power1_min		Low power threshold
> +power1_max		High power threshold
> +power1_input_lowest	Historical minimum power use
> +power1_input_highest	Historical maximum power use
> +power1_reset_history	Write 1 to reset power1 history.
> +			Also clears power fault logs.
> +power1_min_alarm	Low power alarm
> +power1_max_alarm	High power alarm
> +power1_label		Channel label (Power)
> +
> +energy1_input		Measured energy over time (in microJoule)
> +energy1_enable		Enable/Disable Energy accumulation
> +======================= ==========================================
> +
> +DebugFs entries
> +_______________
> +
> +The chip also has a fault log register where failures can be logged. Hence,
> +as these are logging events, we give access to them in debugfs. Note that
> +even if some failure is detected in these logs, it does necessarily mean
> +that the failure is still present. As mentioned in the proper Sysfs entries,
> +these logs can be cleared by writing in the proper reset_history attribute.
> +
> +.. warning:: The debugfs interface is subject to change without notice
> +             and is only available when the kernel is compiled with
> +             ``CONFIG_DEBUG_FS`` defined.
> +
> +``/sys/kernel/debug/i2c/i2c-[X]/[X]-addr/``
> +contains the following attributes:
> +
> +=======================		==========================================
> +power1_failed_fault_log		Set to 1 by a power1 fault occurring.
> +power1_good_input_fault_log	Set to 1 by a power1 good input fault occurring at PGIO3.
> +in11_fet_short_fault_log	Set to 1 when a FET-short fault occurs.
> +in11_fet_bad_fault_log		Set to 1 when a FET-BAD fault occurs.
> +in0_lcrit_fault_log		Set to 1 by a VIN undervoltage fault occurring.
> +in0_crit_fault_log		Set to 1 by a VIN overvoltage fault occurring.
> +curr1_crit_fault_log		Set to 1 by an overcurrent fault occurring.
> +======================= 	==========================================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3f727d7fdfa4..a63833b6fe8b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15166,6 +15166,7 @@ M:	Nuno Sá <nuno.sa@analog.com>
>   L:	linux-hwmon@vger.kernel.org
>   S:	Supported
>   F:	Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml
> +F:	drivers/hwmon/ltc4283.c
>   
>   LTC4286 HARDWARE MONITOR DRIVER
>   M:	Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index fb847ab40ab4..4d9f500ae6ee 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1157,6 +1157,18 @@ config SENSORS_LTC4282
>   	  This driver can also be built as a module. If so, the module will
>   	  be called ltc4282.
>   
> +config SENSORS_LTC4283
> +	tristate "Analog Devices LTC4283"
> +	depends on I2C
> +	select REGMAP_I2C
> +	select AUXILIARY_BUS
> +	help
> +	  If you say yes here you get support for Analog Devices LTC4283
> +	  Negative Voltage Hot Swap Controller I2C interface.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called ltc4283.
> +
>   config SENSORS_LTQ_CPUTEMP
>   	bool "Lantiq cpu temperature sensor driver"
>   	depends on SOC_XWAY
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 0fce31b43eb1..b9d7b0287b9c 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_LTC4245)	+= ltc4245.o
>   obj-$(CONFIG_SENSORS_LTC4260)	+= ltc4260.o
>   obj-$(CONFIG_SENSORS_LTC4261)	+= ltc4261.o
>   obj-$(CONFIG_SENSORS_LTC4282)	+= ltc4282.o
> +obj-$(CONFIG_SENSORS_LTC4283)	+= ltc4283.o
>   obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
>   obj-$(CONFIG_SENSORS_MACSMC_HWMON)	+= macsmc-hwmon.o
>   obj-$(CONFIG_SENSORS_MAX1111)	+= max1111.o
> diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c
> new file mode 100644
> index 000000000000..2a2674a55167
> --- /dev/null
> +++ b/drivers/hwmon/ltc4283.c
> @@ -0,0 +1,1808 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices LTC4283 I2C Negative Voltage Hot Swap Controller (HWMON)
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/bits.h>
> +
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/device/devres.h>
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/math.h>
> +#include <linux/math64.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>
> +
> +#include <linux/mod_devicetable.h>
> +#include <linux/overflow.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> +#define LTC4283_SYSTEM_STATUS		0x00
> +#define LTC4283_FAULT_STATUS		0x03
> +#define   LTC4283_OV_MASK		BIT(0)
> +#define   LTC4283_UV_MASK		BIT(1)
> +#define   LTC4283_OC_MASK		BIT(2)
> +#define   LTC4283_FET_BAD_MASK		BIT(3)
> +#define   LTC4283_FET_SHORT_MASK	BIT(6)
> +#define LTC4283_FAULT_LOG		0x04
> +#define   LTC4283_OV_FAULT_MASK		BIT(0)
> +#define   LTC4283_UV_FAULT_MASK		BIT(1)
> +#define   LTC4283_OC_FAULT_MASK		BIT(2)
> +#define   LTC4283_FET_BAD_FAULT_MASK	BIT(3)
> +#define   LTC4283_PGI_FAULT_MASK	BIT(4)
> +#define   LTC4283_PWR_FAIL_FAULT_MASK	BIT(5)
> +#define   LTC4283_FET_SHORT_FAULT_MASK	BIT(6)
> +#define LTC4283_ADC_ALM_LOG_1		0x05
> +#define   LTC4283_POWER_LOW_ALM		BIT(0)
> +#define   LTC4283_POWER_HIGH_ALM	BIT(1)
> +#define   LTC4283_SENSE_LOW_ALM		BIT(4)
> +#define   LTC4283_SENSE_HIGH_ALM	BIT(5)
> +#define LTC4283_ADC_ALM_LOG_2		0x06
> +#define LTC4283_ADC_ALM_LOG_3		0x07
> +#define LTC4283_ADC_ALM_LOG_4		0x08
> +#define LTC4283_ADC_ALM_LOG_5		0x09
> +#define LTC4283_CONTROL_1		0x0a
> +#define   LTC4283_RW_PAGE_MASK		BIT(0)
> +#define   LTC4283_PIGIO2_ACLB_MASK	BIT(2)
> +#define   LTC4283_PWRGD_RST_CTRL_MASK	BIT(3)
> +#define   LTC4283_FET_BAD_OFF_MASK	BIT(4)
> +#define   LTC4283_THERM_TMR_MASK	BIT(5)
> +#define   LTC4283_DVDT_MASK		BIT(6)
> +#define LTC4283_CONTROL_2		0x0b
> +#define   LTC4283_OV_RETRY_MASK		BIT(0)
> +#define   LTC4283_UV_RETRY_MASK		BIT(1)
> +#define   LTC4283_OC_RETRY_MASK		GENMASK(3, 2)
> +#define   LTC4283_FET_BAD_RETRY_MASK	GENMASK(5, 4)
> +#define   LTC4283_EXT_FAULT_RETRY_MASK	BIT(7)
> +#define LTC4283_RESERVED_OC		0x0c
> +#define LTC4283_CONFIG_1		0x0d
> +#define   LTC4283_FB_MASK		GENMASK(3, 2)
> +#define   LTC4283_ILIM_MASK		GENMASK(7, 4)
> +#define LTC4283_CONFIG_2		0x0e
> +#define   LTC4283_COOLING_DL_MASK	GENMASK(3, 1)
> +#define   LTC4283_FTBD_DL_MASK		GENMASK(5, 4)
> +#define LTC4283_CONFIG_3		0x0f
> +#define   LTC4283_VPWR_DRNS_MASK	BIT(6)
> +#define   LTC4283_EXTFLT_TURN_OFF_MASK	BIT(7)
> +#define LTC4283_PGIO_CONFIG		0x10
> +#define   LTC4283_PGIO1_CFG_MASK	GENMASK(1, 0)
> +#define   LTC4283_PGIO2_CFG_MASK	GENMASK(3, 2)
> +#define   LTC4283_PGIO3_CFG_MASK	GENMASK(5, 4)
> +#define   LTC4283_PGIO4_CFG_MASK	GENMASK(7, 6)
> +#define LTC4283_PGIO_CONFIG_2		0x11
> +#define   LTC4283_ADC_MASK		GENMASK(2, 0)
> +#define LTC4283_ADC_SELECT(c)		(0x13 + (c) / 8)
> +#define   LTC4283_ADC_SELECT_MASK(c)	BIT((c) % 8)
> +#define LTC4283_SENSE_MIN_TH		0x1b
> +#define LTC4283_SENSE_MAX_TH		0x1c
> +#define LTC4283_VPWR_MIN_TH		0x1d
> +#define LTC4283_VPWR_MAX_TH		0x1e
> +#define LTC4283_POWER_MIN_TH		0x1f
> +#define LTC4283_POWER_MAX_TH		0x20
> +#define LTC4283_ADC_2_MIN_TH(c)		(0x21 + (c) * 2)
> +#define LTC4283_ADC_2_MAX_TH(c)		(0x22 + (c) * 2)
> +#define LTC4283_ADC_2_MIN_TH_DIFF(c)	(0x39 + (c) * 2)
> +#define LTC4283_ADC_2_MAX_TH_DIFF(c)	(0x3a + (c) * 2)
> +#define LTC4283_SENSE			0x41
> +#define LTC4283_SENSE_MIN		0x42
> +#define LTC4283_SENSE_MAX		0x43
> +#define LTC4283_VPWR			0x44
> +#define LTC4283_VPWR_MIN		0x45
> +#define LTC4283_VPWR_MAX		0x46
> +#define LTC4283_POWER			0x47
> +#define LTC4283_POWER_MIN		0x48
> +#define LTC4283_POWER_MAX		0x49
> +#define LTC4283_RESERVED_68		0x68
> +#define LTC4283_RESERVED_6D		0x6D
> +/* get channels from ADC 2 */
> +#define LTC4283_ADC_2(c)		(0x4a + (c) * 3)
> +#define LTC4283_ADC_2_MIN(c)		(0x4b + (c) * 3)
> +#define LTC4283_ADC_2_MAX(c)		(0x4c + (c) * 3)
> +#define LTC4283_ADC_2_DIFF(c)		(0x6e + (c) * 3)
> +#define LTC4283_ADC_2_MIN_DIFF(c)	(0x6f + (c) * 3)
> +#define LTC4283_ADC_2_MAX_DIFF(c)	(0x70 + (c) * 3)
> +#define LTC4283_ENERGY			0x7a
> +#define LTC4283_METER_CONTROL		0x84
> +#define   LTC4283_INTEGRATE_I_MASK	BIT(0)
> +#define   LTC4283_METER_HALT_MASK	BIT(6)
> +#define LTC4283_RESERVED_86		0x86
> +#define LTC4283_RESERVED_8F		0x8F
> +#define LTC4283_FAULT_LOG_CTRL		0x90
> +#define   LTC4283_FAULT_LOG_EN_MASK	BIT(7)
> +#define LTC4283_RESERVED_91		0x91
> +#define LTC4283_RESERVED_A1		0xA1
> +#define LTC4283_RESERVED_A3		0xA3
> +#define LTC4283_RESERVED_AC		0xAC
> +#define LTC4283_POWER_PLAY_MSB		0xE7
> +#define LTC4283_POWER_PLAY_LSB		0xE8
> +#define LTC4283_RESERVED_F1		0xF1
> +#define LTC4283_RESERVED_FF		0xFF
> +
> +/* also applies for differential channels */
> +#define LTC4283_ADC1_FS_uV		32768
> +#define LTC4283_ADC2_FS_mV		2048
> +#define LTC4283_TCONV_uS		64103
> +#define LTC4283_VILIM_MIN_uV		15000
> +#define LTC4283_VILIM_MAX_uV		30000
> +#define LTC4283_VILIM_RANGE	\
> +	(LTC4283_VILIM_MAX_uV - LTC4283_VILIM_MIN_uV + 1)
> +
> +#define LTC4283_PGIO_FUNC_GPIO		2
> +#define LTC4283_PGIO2_FUNC_ACLB		3
> +
> +/*
> + * Maximum value for rsense in nano ohms. The reasoning for this value is that
> + * it's the max value for which multiplying by 256 does not overflow long on
> + * 32bits. For the minimum value, is a sane minimum rsense for which power_max
> + * does not overflow 32bits.
> + */
> +#define LTC4283_MAX_RSENSE	1677721599
> +#define LTC4283_MIN_RSENSE	50000
> +
> +/* voltage channels */
> +enum {
> +	LTC4283_CHAN_VIN,
> +	LTC4283_CHAN_VPWR,
> +	LTC4283_CHAN_ADI_1,
> +	LTC4283_CHAN_ADI_2,
> +	LTC4283_CHAN_ADI_3,
> +	LTC4283_CHAN_ADI_4,
> +	LTC4283_CHAN_ADIO_1,
> +	LTC4283_CHAN_ADIO_2,
> +	LTC4283_CHAN_ADIO_3,
> +	LTC4283_CHAN_ADIO_4,
> +	LTC4283_CHAN_DRNS,
> +	LTC4283_CHAN_DRAIN,
> +	/* differential channels */
> +	LTC4283_CHAN_ADIN12,
> +	LTC4283_CHAN_ADIN34,
> +	LTC4283_CHAN_ADIO12,
> +	LTC4283_CHAN_ADIO34,
> +	LTC4283_CHAN_MAX
> +};
> +
> +/* Just for ease of use on the regmap  */
> +#define LTC4283_ADIO34_MAX \
> +	LTC4283_ADC_2_MAX_DIFF(LTC4283_CHAN_ADIO34 - LTC4283_CHAN_ADIN12)
> +
> +struct ltc4283_hwmon {
> +	struct regmap *map;
> +	struct i2c_client *client;
> +	unsigned long gpio_mask;
> +	unsigned long ch_enable_mask;
> +	/* in microwatt */
> +	long power_max;
> +	/* in millivolt */
> +	u32 vsense_max;
> +	/* in tenths of microohm*/
> +	u32 rsense;
> +	bool energy_en;
> +	bool ext_fault;
> +};
> +
> +static int ltc4283_read_voltage_word(const struct ltc4283_hwmon *st,
> +				     u32 reg, u32 fs, long *val)
> +{
> +	unsigned int __raw;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &__raw);
> +	if (ret)
> +		return ret;
> +
> +	*val = DIV_ROUND_CLOSEST(__raw * fs, BIT(16));
> +	return 0;
> +}
> +
> +static int ltc4283_read_voltage_byte(const struct ltc4283_hwmon *st,
> +				     u32 reg, u32 fs, long *val)
> +{
> +	int ret;
> +	u32 in;
> +
> +	ret = regmap_read(st->map, reg, &in);
> +	if (ret)
> +		return ret;
> +
> +	*val = DIV_ROUND_CLOSEST(in * fs, BIT(8));
> +	return 0;
> +}
> +
> +static u32 ltc4283_in_reg(u32 attr, u32 channel)
> +{
> +	switch (attr) {
> +	case hwmon_in_input:
> +		if (channel == LTC4283_CHAN_VPWR)
> +			return LTC4283_VPWR;
> +		if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN)
> +			return LTC4283_ADC_2(channel - LTC4283_CHAN_ADI_1);
> +		return LTC4283_ADC_2_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	case hwmon_in_highest:
> +		if (channel == LTC4283_CHAN_VPWR)
> +			return LTC4283_VPWR_MAX;
> +		if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN)
> +			return LTC4283_ADC_2_MAX(channel - LTC4283_CHAN_ADI_1);
> +		return LTC4283_ADC_2_MAX_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	case hwmon_in_lowest:
> +		if (channel == LTC4283_CHAN_VPWR)
> +			return LTC4283_VPWR_MIN;
> +		if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN)
> +			return LTC4283_ADC_2_MIN(channel - LTC4283_CHAN_ADI_1);
> +		return LTC4283_ADC_2_MIN_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	case hwmon_in_max:
> +		if (channel == LTC4283_CHAN_VPWR)
> +			return LTC4283_VPWR_MAX_TH;
> +		if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN)
> +			return LTC4283_ADC_2_MAX_TH(channel - LTC4283_CHAN_ADI_1);
> +		return LTC4283_ADC_2_MAX_TH_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	default:
> +		if (channel == LTC4283_CHAN_VPWR)
> +			return LTC4283_VPWR_MIN_TH;
> +		if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN)
> +			return LTC4283_ADC_2_MIN_TH(channel - LTC4283_CHAN_ADI_1);
> +		return LTC4283_ADC_2_MIN_TH_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	}
> +}
> +
> +static int ltc4283_read_in_vals(const struct ltc4283_hwmon *st,
> +				u32 attr, u32 channel, long *val)
> +{
> +	u32 reg = ltc4283_in_reg(attr, channel);
> +	int ret;
> +
> +	if (channel < LTC4283_CHAN_ADIN12) {
> +		if (attr != hwmon_in_max && attr != hwmon_in_min)
> +			return ltc4283_read_voltage_word(st, reg,
> +							 LTC4283_ADC2_FS_mV,
> +							 val);
> +
> +		return ltc4283_read_voltage_byte(st, reg,
> +						 LTC4283_ADC2_FS_mV, val);
> +	}
> +
> +	if (attr != hwmon_in_max && attr != hwmon_in_min)
> +		ret = ltc4283_read_voltage_word(st, reg,
> +						LTC4283_ADC1_FS_uV, val);
> +	else
> +		ret = ltc4283_read_voltage_byte(st, reg,
> +						LTC4283_ADC1_FS_uV, val);
> +	if (ret)
> +		return ret;
> +
> +	*val = DIV_ROUND_CLOSEST(*val, MILLI);
> +	return 0;
> +}
> +
> +static int ltc4283_read_alarm(struct ltc4283_hwmon *st, u32 reg,
> +			      u32 mask, long *val)
> +{
> +	u32 alarm;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &alarm);
> +	if (ret)
> +		return ret;
> +
> +	*val = !!(alarm & mask);
> +
> +	/* If not status/fault logs, clear the alarm after reading it. */
> +	if (reg != LTC4283_FAULT_STATUS && reg != LTC4283_FAULT_LOG)
> +		return regmap_clear_bits(st->map, reg, mask);
> +
> +	return 0;
> +}
> +
> +static int ltc4283_read_in_alarm(struct ltc4283_hwmon *st, u32 channel,
> +				 bool max_alm, long *val)
> +{
> +	if (channel == LTC4283_VPWR)
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1,
> +					  BIT(2 + max_alm), val);
> +
> +	if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_ADI_4) {
> +		u32 bit = (channel - LTC4283_CHAN_ADI_1) * 2;
> +		/*
> +		 * Lower channels go to higher bits. We also want to go +1 down
> +		 * in the min_alarm case.
> +		 */
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_2,
> +					  BIT(7 - bit - !max_alm), val);
> +	}
> +
> +	if (channel >= LTC4283_CHAN_ADIO_1 && channel <= LTC4283_CHAN_ADIO_4) {
> +		u32 bit = (channel - LTC4283_CHAN_ADIO_1) * 2;
> +
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_3,
> +					  BIT(7 - bit - !max_alm), val);
> +	}
> +
> +	if (channel >= LTC4283_CHAN_ADIN12 && channel <= LTC4283_CHAN_ADIO34) {
> +		u32 bit = (channel - LTC4283_CHAN_ADIN12) * 2;
> +
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_5,
> +					  BIT(7 - bit - !max_alm), val);
> +	}
> +
> +	if (channel == LTC4283_CHAN_DRNS)
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_4,
> +					  BIT(6 + max_alm), val);
> +
> +	return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_4, BIT(4 + max_alm),
> +				  val);
> +}
> +
> +static int ltc4283_read_in(struct ltc4283_hwmon *st, u32 attr, u32 channel,
> +			   long *val)
> +{
> +	switch (attr) {
> +	case hwmon_in_input:
> +		if (!test_bit(channel, &st->ch_enable_mask))
> +			return -ENODATA;
> +
> +		return ltc4283_read_in_vals(st, attr, channel, val);
> +	case hwmon_in_highest:
> +	case hwmon_in_lowest:
> +	case hwmon_in_max:
> +	case hwmon_in_min:
> +		return ltc4283_read_in_vals(st, attr, channel, val);
> +	case hwmon_in_max_alarm:
> +		return ltc4283_read_in_alarm(st, channel, true, val);
> +	case hwmon_in_min_alarm:
> +		return ltc4283_read_in_alarm(st, channel, false, val);
> +	case hwmon_in_crit_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_FAULT_STATUS,
> +					  LTC4283_OV_MASK, val);
> +	case hwmon_in_lcrit_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_FAULT_STATUS,
> +					  LTC4283_UV_MASK, val);
> +	case hwmon_in_fault:
> +		/*
> +		 * We report failure if we detect either a fer_bad or a
> +		 * fet_short in the status register.
> +		 */
> +		return ltc4283_read_alarm(st, LTC4283_FAULT_STATUS,
> +					  LTC4283_FET_BAD_MASK | LTC4283_FET_SHORT_MASK, val);
> +	case hwmon_in_enable:
> +		*val = test_bit(channel, &st->ch_enable_mask);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +	return 0;
> +}
> +
> +static int ltc4283_read_current_word(const struct ltc4283_hwmon *st, u32 reg,
> +				     long *val)
> +{
> +	u64 temp = (u64)LTC4283_ADC1_FS_uV * DECA * MILLI;
> +	unsigned int __raw;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &__raw);
> +	if (ret)
> +		return ret;
> +
> +	*val = DIV64_U64_ROUND_CLOSEST(__raw * temp,
> +				       BIT_ULL(16) * st->rsense);
> +
> +	return 0;
> +}
> +
> +static int ltc4283_read_current_byte(const struct ltc4283_hwmon *st, u32 reg,
> +				     long *val)
> +{
> +	u64 temp = (u64)LTC4283_ADC1_FS_uV * DECA * MILLI;
> +	u32 curr;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &curr);
> +	if (ret)
> +		return ret;
> +
> +	*val = DIV_ROUND_CLOSEST_ULL(curr * temp, BIT(8) * st->rsense);
> +	return 0;
> +}
> +
> +static int ltc4283_read_curr(struct ltc4283_hwmon *st, u32 attr, long *val)
> +{
> +	switch (attr) {
> +	case hwmon_curr_input:
> +		return ltc4283_read_current_word(st, LTC4283_SENSE, val);
> +	case hwmon_curr_highest:
> +		return ltc4283_read_current_word(st, LTC4283_SENSE_MAX, val);
> +	case hwmon_curr_lowest:
> +		return ltc4283_read_current_word(st, LTC4283_SENSE_MIN, val);
> +	case hwmon_curr_max:
> +		return ltc4283_read_current_byte(st, LTC4283_SENSE_MAX_TH, val);
> +	case hwmon_curr_min:
> +		return ltc4283_read_current_byte(st, LTC4283_SENSE_MIN_TH, val);
> +	case hwmon_curr_max_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1,
> +					  LTC4283_SENSE_HIGH_ALM, val);
> +	case hwmon_curr_min_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1,
> +					  LTC4283_SENSE_LOW_ALM, val);
> +	case hwmon_curr_crit_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_FAULT_STATUS,
> +					  LTC4283_OC_MASK, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ltc4283_read_power_word(const struct ltc4283_hwmon *st,
> +				   u32 reg, long *val)
> +{
> +	u64 temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI;
> +	unsigned int __raw;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &__raw);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Power is given by:
> +	 *     P = CODE(16b) * 32.768mV * 2.048V / (2^16 * Rsense)
> +	 */
> +	*val = DIV64_U64_ROUND_CLOSEST(temp * __raw, BIT_ULL(16) * st->rsense);
> +
> +	return 0;
> +}
> +
> +static int ltc4283_read_power_byte(const struct ltc4283_hwmon *st,
> +				   u32 reg, long *val)
> +{
> +	u64 temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI;
> +	u32 power;
> +	int ret;
> +
> +	ret = regmap_read(st->map, reg, &power);
> +	if (ret)
> +		return ret;
> +
> +	*val = DIV_ROUND_CLOSEST_ULL(power * temp, BIT(8) * st->rsense);
> +
> +	return 0;
> +}
> +
> +static int ltc4283_read_power(struct ltc4283_hwmon *st, u32 attr, long *val)
> +{
> +	switch (attr) {
> +	case hwmon_power_input:
> +		return ltc4283_read_power_word(st, LTC4283_POWER, val);
> +	case hwmon_power_input_highest:
> +		return ltc4283_read_power_word(st, LTC4283_POWER_MAX, val);
> +	case hwmon_power_input_lowest:
> +		return ltc4283_read_power_word(st, LTC4283_POWER_MIN, val);
> +	case hwmon_power_max_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1,
> +					  LTC4283_POWER_HIGH_ALM, val);
> +	case hwmon_power_min_alarm:
> +		return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1,
> +					  LTC4283_POWER_LOW_ALM, val);
> +	case hwmon_power_max:
> +		return ltc4283_read_power_byte(st, LTC4283_POWER_MAX_TH, val);
> +	case hwmon_power_min:
> +		return ltc4283_read_power_byte(st, LTC4283_POWER_MIN_TH, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ltc4283_read_energy(struct ltc4283_hwmon *st, u32 attr, s64 *val)
> +{
> +	u64 temp = LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV, energy, temp_2;
> +	u8 raw[8] = {};
> +	int ret;
> +
> +	if (!st->energy_en)
> +		return -ENODATA;
> +
> +	ret = i2c_smbus_read_i2c_block_data(st->client, LTC4283_ENERGY, 6, raw);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 6)
> +		return -EIO;
> +
> +	energy = get_unaligned_be64(raw) >> 16;
> +
> +	/*
> +	 * The formula for energy is given by:
> +	 *	E = CODE(48b) * 32.768mV * 2.048V * Tconv / 2^24 * Rsense
> +	 *
> +	 * As Rsense can have tenths of micro-ohm resolution, we need to
> +	 * multiply by DECA to get microjoule.
> +	 */
> +	if (check_mul_overflow(temp * LTC4283_TCONV_uS, energy, &temp_2)) {
> +		/*
> +		 * We multiply again by 1000 to make sure that we don't get 0
> +		 * in the following division which could happen for big rsense
> +		 * values. OTOH, we then divide energy first by 1000 so that
> +		 * we do not overflow u64 again for very small rsense values.
> +		 * We add 100 factor for proper conversion to microjoule.
> +		 */
> +		temp_2 = DIV64_U64_ROUND_CLOSEST(temp * LTC4283_TCONV_uS * MILLI,
> +						 BIT_ULL(24) * st->rsense);
> +		energy = DIV_ROUND_CLOSEST_ULL(energy, MILLI * CENTI) * temp_2;
> +	} else {
> +		/* Put rsense back into nanoohm so we get microjoule. */
> +		energy = DIV64_U64_ROUND_CLOSEST(temp_2, BIT_ULL(24) * st->rsense * CENTI);
> +	}
> +
> +	*val = energy;
> +	return 0;
> +}
> +
> +static int ltc4283_read(struct device *dev, enum hwmon_sensor_types type,
> +			u32 attr, int channel, long *val)
> +{
> +	struct ltc4283_hwmon *st = dev_get_drvdata(dev);
> +
> +	switch (type) {
> +	case hwmon_in:
> +		return ltc4283_read_in(st, attr, channel, val);
> +	case hwmon_curr:
> +		return ltc4283_read_curr(st, attr, val);
> +	case hwmon_power:
> +		return ltc4283_read_power(st, attr, val);
> +	case hwmon_energy:
> +		*val = st->energy_en;
> +		return 0;
> +	case hwmon_energy64:
> +		return ltc4283_read_energy(st, attr, (s64 *)val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ltc4283_write_power_byte(const struct ltc4283_hwmon *st, u32 reg,
> +				    long val)
> +{
> +	u64 temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI;
> +	u32 __raw;
> +
> +	val = clamp_val(val, 0, st->power_max);
> +	__raw = DIV64_U64_ROUND_CLOSEST(val * BIT_ULL(8) * st->rsense, temp);
> +
> +	return regmap_write(st->map, reg, __raw);
> +}
> +
> +static int ltc4283_write_power_word(const struct ltc4283_hwmon *st,
> +				    u32 reg, long val)
> +{
> +	u64 temp = st->rsense * BIT_ULL(16), temp_2;
> +	u16 __raw;
> +
> +	if (check_mul_overflow(val, temp, &temp_2)) {
> +		temp = DIV_ROUND_CLOSEST_ULL(temp, DECA * MILLI);
> +		__raw = DIV_ROUND_CLOSEST_ULL(temp * val, LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV);
> +	} else {
> +		temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI;
> +		__raw = DIV64_U64_ROUND_CLOSEST(temp_2, temp);
> +	}
> +
> +	return regmap_write(st->map, reg, __raw);
> +}
> +
> +static int ltc4283_reset_power_hist(struct ltc4283_hwmon *st)
> +{
> +	int ret;
> +
> +	ret = ltc4283_write_power_word(st, LTC4283_POWER_MIN, st->power_max);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc4283_write_power_word(st, LTC4283_POWER_MAX, 0);
> +	if (ret)
> +		return ret;
> +
> +	/* Clear possible power faults. */
> +	return regmap_clear_bits(st->map, LTC4283_FAULT_LOG,
> +				 LTC4283_PWR_FAIL_FAULT_MASK | LTC4283_PGI_FAULT_MASK);
> +}
> +
> +static int ltc4283_write_power(struct ltc4283_hwmon *st, u32 attr, long val)
> +{
> +	switch (attr) {
> +	case hwmon_power_max:
> +		return ltc4283_write_power_byte(st, LTC4283_POWER_MAX_TH, val);
> +	case hwmon_power_min:
> +		return ltc4283_write_power_byte(st, LTC4283_POWER_MIN_TH, val);
> +	case hwmon_power_reset_history:
> +		return ltc4283_reset_power_hist(st);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ltc4283_write_in_history(struct ltc4283_hwmon *st, u32 reg,
> +				    long lowest, u32 fs)
> +{
> +	u32 __raw;
> +	int ret;
> +
> +	__raw = DIV_ROUND_CLOSEST(BIT(16) * lowest, fs);
> +	if (__raw == BIT(16))
> +		__raw = U16_MAX;
> +
> +	ret = regmap_write(st->map, reg, __raw);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->map, reg + 1, 0);
> +}
> +
> +static int ltc4283_write_in_byte(const struct ltc4283_hwmon *st,
> +				 u32 reg, u32 fs, long val)
> +{
> +	u32 __raw;
> +
> +	val = clamp_val(val, 0, fs);
> +	__raw = DIV_ROUND_CLOSEST(val * BIT(8), fs);
> +	if (__raw == BIT(8))
> +		__raw = U8_MAX;
> +
> +	return regmap_write(st->map, reg, __raw);
> +}
> +
> +static int ltc4283_reset_in_hist(struct ltc4283_hwmon *st, u32 channel)
> +{
> +	u32 reg, fs;
> +	int ret;
> +
> +	/*
> +	 * Make sure to clear possible under/over voltage faults. Otherwise the
> +	 * chip won't latch on again.
> +	 */
> +	if (channel == LTC4283_CHAN_VIN)
> +		return regmap_clear_bits(st->map, LTC4283_FAULT_LOG,
> +					 LTC4283_OV_FAULT_MASK | LTC4283_UV_FAULT_MASK);
> +
> +	if (channel == LTC4283_CHAN_VPWR)
> +		return ltc4283_write_in_history(st, LTC4283_VPWR_MIN,
> +						LTC4283_ADC2_FS_mV,
> +						LTC4283_ADC2_FS_mV);
> +
> +	if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN) {
> +		fs = LTC4283_ADC2_FS_mV;
> +		reg = LTC4283_ADC_2_MIN(channel - LTC4283_CHAN_ADI_1);
> +	} else {
> +		fs = LTC4283_ADC1_FS_uV;
> +		reg = LTC4283_ADC_2_MIN_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	}
> +
> +	ret = ltc4283_write_in_history(st, reg, fs, fs);
> +	if (ret)
> +		return ret;
> +	if (channel != LTC4283_CHAN_DRAIN)
> +		return 0;
> +
> +	/* Then, let's also clear possible fet faults. Same as above. */
> +	return regmap_clear_bits(st->map, LTC4283_FAULT_LOG,
> +				 LTC4283_FET_BAD_FAULT_MASK | LTC4283_FET_SHORT_FAULT_MASK);
> +}
> +
> +static int ltc4283_write_in_en(struct ltc4283_hwmon *st, u32 channel, bool en)
> +{
> +	unsigned int bit, adc_idx = channel - LTC4283_CHAN_ADI_1;
> +	unsigned int reg = LTC4283_ADC_SELECT(adc_idx);
> +	int ret;
> +
> +	bit = LTC4283_ADC_SELECT_MASK(adc_idx);
> +	if (channel > LTC4283_CHAN_DRAIN)
> +		/* Account for two reserved fields after DRAIN. */
> +		bit <<= 2;
> +
> +	if (en)
> +		ret = regmap_set_bits(st->map, reg, bit);
> +	else
> +		ret = regmap_clear_bits(st->map, reg, bit);
> +	if (ret)
> +		return ret;
> +
> +	__assign_bit(channel, &st->ch_enable_mask, en);
> +	return 0;
> +}
> +
> +static int ltc4283_write_minmax(struct ltc4283_hwmon *st, long val,
> +				u32 channel, bool is_max)
> +{
> +	u32 reg;
> +
> +	if (channel == LTC4283_CHAN_VPWR) {
> +		if (is_max)
> +			return ltc4283_write_in_byte(st, LTC4283_VPWR_MAX_TH,
> +						     LTC4283_ADC2_FS_mV, val);
> +
> +		return ltc4283_write_in_byte(st, LTC4283_VPWR_MIN_TH,
> +					     LTC4283_ADC2_FS_mV, val);
> +	}
> +
> +	if (channel >= LTC4283_CHAN_ADI_1 && channel <= LTC4283_CHAN_DRAIN) {
> +		if (is_max) {
> +			reg = LTC4283_ADC_2_MAX_TH(channel - LTC4283_CHAN_ADI_1);
> +			return ltc4283_write_in_byte(st, reg,
> +						     LTC4283_ADC2_FS_mV, val);
> +		}
> +
> +		reg = LTC4283_ADC_2_MIN_TH(channel - LTC4283_CHAN_ADI_1);
> +		return ltc4283_write_in_byte(st, reg, LTC4283_ADC2_FS_mV, val);
> +	}
> +
> +	/* Just sanity check we do not overflow val for 32bit */
> +	val = clamp_val(val * MILLI, 0, LTC4283_ADC1_FS_uV);
> +
> +	if (is_max) {
> +		reg = LTC4283_ADC_2_MAX_TH_DIFF(channel - LTC4283_CHAN_ADIN12);
> +		return ltc4283_write_in_byte(st, reg, LTC4283_ADC1_FS_uV, val);
> +	}
> +
> +	reg = LTC4283_ADC_2_MIN_TH_DIFF(channel - LTC4283_CHAN_ADIN12);
> +	return ltc4283_write_in_byte(st, reg, LTC4283_ADC1_FS_uV, val);
> +}
> +
> +static int ltc4283_write_in(struct ltc4283_hwmon *st, u32 attr, long val,
> +			    int channel)
> +{
> +	switch (attr) {
> +	case hwmon_in_max:
> +		return ltc4283_write_minmax(st, val, channel, true);
> +	case hwmon_in_min:
> +		return ltc4283_write_minmax(st, val, channel, false);
> +	case hwmon_in_reset_history:
> +		return ltc4283_reset_in_hist(st, channel);
> +	case hwmon_in_enable:
> +		return ltc4283_write_in_en(st, channel, !!val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ltc4283_write_curr_byte(const struct ltc4283_hwmon *st,
> +				   u32 reg, long val)
> +{
> +	u32 temp = LTC4283_ADC1_FS_uV * DECA * MILLI;
> +	u32 reg_val, isense_max;
> +
> +	isense_max = DIV_ROUND_CLOSEST(st->vsense_max * MICRO * DECA, st->rsense);
> +	val = clamp_val(val, 0, isense_max);
> +	reg_val = DIV_ROUND_CLOSEST_ULL(val * BIT_ULL(8) * st->rsense, temp);
> +
> +	return regmap_write(st->map, reg, reg_val);
> +}
> +
> +static int ltc4283_write_curr_history(struct ltc4283_hwmon *st)
> +{
> +	int ret;
> +
> +	ret = ltc4283_write_in_history(st, LTC4283_SENSE_MIN,
> +				       st->vsense_max * MILLI,
> +				       LTC4283_ADC1_FS_uV);
> +	if (ret)
> +		return ret;
> +
> +	/* Now, let's also clear possible overcurrent logs. */
> +	return regmap_clear_bits(st->map, LTC4283_FAULT_LOG,
> +				 LTC4283_OC_FAULT_MASK);
> +}
> +
> +static int ltc4283_write_curr(struct ltc4283_hwmon *st, u32 attr, long val)
> +{
> +	switch (attr) {
> +	case hwmon_curr_max:
> +		return ltc4283_write_curr_byte(st, LTC4283_SENSE_MAX_TH, val);
> +	case hwmon_curr_min:
> +		return ltc4283_write_curr_byte(st, LTC4283_SENSE_MIN_TH, val);
> +	case hwmon_curr_reset_history:
> +		return ltc4283_write_curr_history(st);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int ltc4283_energy_enable_set(struct ltc4283_hwmon *st, long val)
> +{
> +	int ret;
> +
> +	/* Setting the bit halts the meter. */
> +	val = !!val;
> +	ret = regmap_update_bits(st->map, LTC4283_METER_CONTROL,
> +				 LTC4283_METER_HALT_MASK,
> +				 FIELD_PREP(LTC4283_METER_HALT_MASK, !val));
> +	if (ret)
> +		return ret;
> +
> +	st->energy_en = val;
> +
> +	return 0;
> +}
> +
> +static int ltc4283_write(struct device *dev, enum hwmon_sensor_types type,
> +			 u32 attr, int channel, long val)
> +{
> +	struct ltc4283_hwmon *st = dev_get_drvdata(dev);
> +
> +	switch (type) {
> +	case hwmon_power:
> +		return ltc4283_write_power(st, attr, val);
> +	case hwmon_in:
> +		return ltc4283_write_in(st, attr, val, channel);
> +	case hwmon_curr:
> +		return ltc4283_write_curr(st, attr, val);
> +	case hwmon_energy:
> +		return ltc4283_energy_enable_set(st, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static umode_t ltc4283_in_is_visible(const struct ltc4283_hwmon *st,
> +				     u32 attr, int channel)
> +{
> +	/* If ADIO is set as a GPIO, don´t make it visible. */
> +	if (channel >= LTC4283_CHAN_ADIO_1 && channel <= LTC4283_CHAN_ADIO_4) {
> +		/* ADIOX pins come at index 0 in the gpio mask. */
> +		channel -= LTC4283_CHAN_ADIO_1;
> +		if (test_bit(channel, &st->gpio_mask))
> +			return 0;
> +	}
> +
> +	/* Also take care of differential channels. */
> +	if (channel >= LTC4283_CHAN_ADIO12 && channel <= LTC4283_CHAN_ADIO34) {
> +		channel -= LTC4283_CHAN_ADIO12;
> +		/* If one channel in the pair is used, make it invisible. */
> +		if (test_bit(channel * 2, &st->gpio_mask) ||
> +		    test_bit(channel * 2 + 1, &st->gpio_mask))
> +			return 0;
> +	}
> +
> +	switch (attr) {
> +	case hwmon_in_input:
> +	case hwmon_in_highest:
> +	case hwmon_in_lowest:
> +	case hwmon_in_max_alarm:
> +	case hwmon_in_min_alarm:
> +	case hwmon_in_label:
> +	case hwmon_in_lcrit_alarm:
> +	case hwmon_in_crit_alarm:
> +	case hwmon_in_fault:
> +		return 0444;
> +	case hwmon_in_max:
> +	case hwmon_in_min:
> +	case hwmon_in_enable:
> +		return 0644;
> +	case hwmon_in_reset_history:
> +		return 0200;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static umode_t ltc4283_curr_is_visible(u32 attr)
> +{
> +	switch (attr) {
> +	case hwmon_curr_input:
> +	case hwmon_curr_highest:
> +	case hwmon_curr_lowest:
> +	case hwmon_curr_max_alarm:
> +	case hwmon_curr_min_alarm:
> +	case hwmon_curr_crit_alarm:
> +	case hwmon_curr_label:
> +		return 0444;
> +	case hwmon_curr_max:
> +	case hwmon_curr_min:
> +		return 0644;
> +	case hwmon_curr_reset_history:
> +		return 0200;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static umode_t ltc4283_power_is_visible(u32 attr)
> +{
> +	switch (attr) {
> +	case hwmon_power_input:
> +	case hwmon_power_input_highest:
> +	case hwmon_power_input_lowest:
> +	case hwmon_power_label:
> +	case hwmon_power_max_alarm:
> +	case hwmon_power_min_alarm:
> +		return 0444;
> +	case hwmon_power_max:
> +	case hwmon_power_min:
> +		return 0644;
> +	case hwmon_power_reset_history:
> +		return 0200;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static umode_t ltc4283_is_visible(const void *data,
> +				  enum hwmon_sensor_types type,
> +				  u32 attr, int channel)
> +{
> +	switch (type) {
> +	case hwmon_in:
> +		return ltc4283_in_is_visible(data, attr, channel);
> +	case hwmon_curr:
> +		return ltc4283_curr_is_visible(attr);
> +	case hwmon_power:
> +		return ltc4283_power_is_visible(attr);
> +	case hwmon_energy:
> +		/* hwmon_energy_enable */
> +		return 0644;
> +	case hwmon_energy64:
> +		/* hwmon_energy_input */
> +		return 0444;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static const char * const ltc4283_in_strs[] = {
> +	"VIN", "VPWR", "VADI1", "VADI2", "VADI3", "VADI4", "VADIO1", "VADIO2",
> +	"VADIO3", "VADIO4", "DRNS", "DRAIN", "ADIN2-ADIN1", "ADIN4-ADIN3",
> +	"ADIO2-ADIO1", "ADIO4-ADIO3"
> +};
> +
> +static int ltc4283_read_labels(struct device *dev,
> +			       enum hwmon_sensor_types type,
> +			       u32 attr, int channel, const char **str)
> +{
> +	switch (type) {
> +	case hwmon_in:
> +		*str = ltc4283_in_strs[channel];
> +		return 0;
> +	case hwmon_curr:
> +		*str = "ISENSE";
> +		return 0;
> +	case hwmon_power:
> +		*str = "Power";
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +/*
> + * Set max limits for ISENSE and Power as that depends on the max voltage on
> + * rsense that is defined in ILIM_ADJUST. This is specially important for power
> + * because for some rsense and vfsout values, if we allow the default raw 255
> + * value, that would overflow long in 32bit archs when reading back the max
> + * power limit.
> + */
> +static int ltc4283_set_max_limits(struct ltc4283_hwmon *st, struct device *dev)
> +{
> +	u32 temp = st->vsense_max * DECA * MICRO;
> +	int ret;
> +
> +	ret = ltc4283_write_in_byte(st, LTC4283_SENSE_MAX_TH, LTC4283_ADC1_FS_uV,
> +				    st->vsense_max * MILLI);
> +	if (ret)
> +		return ret;
> +
> +	/* Power is given by ISENSE * Vout. */
> +	st->power_max = DIV_ROUND_CLOSEST(temp, st->rsense) * LTC4283_ADC2_FS_mV;
> +	return ltc4283_write_power_byte(st, LTC4283_POWER_MAX_TH, st->power_max);
> +}
> +
> +static int ltc4283_parse_array_prop(const struct ltc4283_hwmon *st,
> +				    struct device *dev, const char *prop,
> +				    const u32 *vals, u32 n_vals)
> +{
> +	u32 prop_val;
> +	int ret;
> +	u32 i;
> +
> +	ret = device_property_read_u32(dev, prop, &prop_val);
> +	if (ret)
> +		return n_vals;
> +
> +	for (i = 0; i < n_vals; i++) {
> +		if (prop_val != vals[i])
> +			continue;
> +
> +		return i;
> +	}
> +
> +	return dev_err_probe(dev, -EINVAL,
> +			     "Invalid %s property value %u, expected one of: %*ph\n",
> +			     prop, prop_val, n_vals, vals);
> +}
> +
> +static int ltc4283_get_defaults(struct ltc4283_hwmon *st)
> +{
> +	u32 reg_val, ilm_adjust, c;
> +	int ret;
> +
> +	ret = regmap_read(st->map, LTC4283_METER_CONTROL, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	st->energy_en = !FIELD_GET(LTC4283_METER_HALT_MASK, reg_val);
> +
> +	ret = regmap_read(st->map, LTC4283_CONFIG_1, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	ilm_adjust = FIELD_GET(LTC4283_ILIM_MASK, reg_val);
> +	st->vsense_max = LTC4283_VILIM_MIN_uV / MILLI + ilm_adjust;
> +
> +	ret = regmap_read(st->map, LTC4283_PGIO_CONFIG, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	/* Can be latter overwritten in ltc4283_pgio_config() */
> +	if (FIELD_GET(LTC4283_PGIO4_CFG_MASK, reg_val) < LTC4283_PGIO_FUNC_GPIO)
> +		st->ext_fault = true;
> +
> +	/* VPWR and VIN are always enabled */
> +	__set_bit(LTC4283_CHAN_VIN, &st->ch_enable_mask);
> +	__set_bit(LTC4283_CHAN_VPWR, &st->ch_enable_mask);
> +	for (c = LTC4283_CHAN_ADI_1; c < LTC4283_CHAN_MAX; c++) {
> +		u32 chan = c - LTC4283_CHAN_ADI_1, bit;
> +
> +		ret = regmap_read(st->map, LTC4283_ADC_SELECT(chan), &reg_val);
> +		if (ret)
> +			return ret;
> +
> +		bit = LTC4283_ADC_SELECT_MASK(chan);
> +		if (c > LTC4283_CHAN_DRAIN)
> +			/* account for two reserved fields after DRAIN */
> +			bit <<= 2;
> +
> +		if (!(bit & reg_val))
> +			continue;
> +
> +		__set_bit(c, &st->ch_enable_mask);
> +	}
> +
> +	return 0;
> +}
> +
> +static const char * const ltc4283_pgio1_funcs[] = {
> +	"inverted_power_good", "power_good", "gpio"
> +};
> +
> +static const char * const ltc4283_pgio2_funcs[] = {
> +	 "inverted_power_good", "power_good", "gpio", "active_current_limiting"
> +};
> +
> +static const char * const ltc4283_pgio3_funcs[] = {
> +	"inverted_power_good_input", "power_good_input", "gpio"
> +};
> +
> +static const char * const ltc4283_pgio4_funcs[] = {
> +	"inverted_external_fault", "external_fault", "gpio"
> +};
> +
> +enum {
> +	LTC4283_PIN_ADIO1,
> +	LTC4283_PIN_ADIO2,
> +	LTC4283_PIN_ADIO3,
> +	LTC4283_PIN_ADIO4,
> +	LTC4283_PIN_PGIO1,
> +	LTC4283_PIN_PGIO2,
> +	LTC4283_PIN_PGIO3,
> +	LTC4283_PIN_PGIO4,
> +};
> +
> +static int ltc4283_pgio_config(struct ltc4283_hwmon *st, struct device *dev)
> +{
> +	int ret, func;
> +
> +	func = device_property_match_property_string(dev, "adi,pgio1-func",
> +						     ltc4283_pgio1_funcs,
> +						     ARRAY_SIZE(ltc4283_pgio1_funcs));
> +	if (func < 0 && func != -EINVAL)
> +		return dev_err_probe(dev, func,
> +				     "Invalid adi,pgio1-func property\n");
> +	if (func >= 0) {
> +		if (func == LTC4283_PGIO_FUNC_GPIO) {
> +			__set_bit(LTC4283_PIN_PGIO1, &st->gpio_mask);
> +			/* If GPIO, default to an input pin. */
> +			func++;
> +		}
> +
> +		ret = regmap_update_bits(st->map, LTC4283_PGIO_CONFIG,
> +					 LTC4283_PGIO1_CFG_MASK,
> +					 FIELD_PREP(LTC4283_PGIO1_CFG_MASK, func));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	func = device_property_match_property_string(dev, "adi,pgio2-func",
> +						     ltc4283_pgio2_funcs,
> +						     ARRAY_SIZE(ltc4283_pgio2_funcs));
> +
> +	if (func < 0 && func != -EINVAL)
> +		return dev_err_probe(dev, func,
> +				     "Invalid adi,pgio2-func property\n");
> +	if (func >= 0) {
> +		if (func != LTC4283_PGIO2_FUNC_ACLB) {
> +			if (func == LTC4283_PGIO_FUNC_GPIO)  {
> +				__set_bit(LTC4283_PIN_PGIO2, &st->gpio_mask);
> +				func++;
> +			}
> +
> +			ret = regmap_update_bits(st->map, LTC4283_PGIO_CONFIG,
> +						 LTC4283_PGIO2_CFG_MASK,
> +						 FIELD_PREP(LTC4283_PGIO2_CFG_MASK, func));
> +		} else {
> +			ret = regmap_set_bits(st->map, LTC4283_CONTROL_1,
> +					      LTC4283_PIGIO2_ACLB_MASK);
> +		}
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	func = device_property_match_property_string(dev, "adi,pgio3-func",
> +						     ltc4283_pgio3_funcs,
> +						     ARRAY_SIZE(ltc4283_pgio3_funcs));
> +
> +	if (func < 0 && func != -EINVAL)
> +		return dev_err_probe(dev, func,
> +				     "Invalid adi,pgio3-func property\n");
> +	if (func >= 0) {
> +		if (func == LTC4283_PGIO_FUNC_GPIO) {
> +			__set_bit(LTC4283_PIN_PGIO3, &st->gpio_mask);
> +			func++;
> +		}
> +
> +		ret = regmap_update_bits(st->map, LTC4283_PGIO_CONFIG,
> +					 LTC4283_PGIO3_CFG_MASK,
> +					 FIELD_PREP(LTC4283_PGIO3_CFG_MASK, func));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	func = device_property_match_property_string(dev, "adi,pgio4-func",
> +						     ltc4283_pgio4_funcs,
> +						     ARRAY_SIZE(ltc4283_pgio4_funcs));
> +
> +	if (func < 0 && func != -EINVAL)
> +		return dev_err_probe(dev, func,
> +				     "Invalid adi,pgio4-func property\n");
> +	if (func >= 0) {
> +		if (func == LTC4283_PGIO_FUNC_GPIO) {
> +			__set_bit(LTC4283_PIN_PGIO4, &st->gpio_mask);
> +			func++;
> +			st->ext_fault = false;
> +		} else {
> +			st->ext_fault = true;
> +		}
> +
> +		ret = regmap_update_bits(st->map, LTC4283_PGIO_CONFIG,
> +					 LTC4283_PGIO4_CFG_MASK,
> +					 FIELD_PREP(LTC4283_PGIO4_CFG_MASK, func));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ltc4283_adio_config(struct ltc4283_hwmon *st, struct device *dev,
> +			       const char *prop, u32 pin)
> +{
> +	u32 adc_idx;
> +	int ret;
> +
> +	if (!device_property_read_bool(dev, prop))
> +		return 0;
> +
> +	adc_idx = LTC4283_CHAN_ADIO_1 - LTC4283_CHAN_ADI_1 + pin;
> +	ret = regmap_clear_bits(st->map, LTC4283_ADC_SELECT(adc_idx),
> +				LTC4283_ADC_SELECT_MASK(adc_idx));
> +	if (ret)
> +		return ret;
> +
> +	__set_bit(pin, &st->gpio_mask);
> +	return 0;
> +}
> +
> +static int ltc4283_pin_config(struct ltc4283_hwmon *st, struct device *dev)
> +{
> +	int ret;
> +
> +	ret = ltc4283_pgio_config(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc4283_adio_config(st, dev, "adi,gpio-on-adio1", LTC4283_PIN_ADIO1);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc4283_adio_config(st, dev, "adi,gpio-on-adio2", LTC4283_PIN_ADIO2);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc4283_adio_config(st, dev, "adi,gpio-on-adio3", LTC4283_PIN_ADIO3);
> +	if (ret)
> +		return ret;
> +
> +	return ltc4283_adio_config(st, dev, "adi,gpio-on-adio4", LTC4283_PIN_ADIO4);
> +}
> +
> +static const char * const ltc4283_oc_fet_retry[] = {
> +	"latch-off", "1", "7", "unlimited"
> +};
> +
> +static const u32 ltc4283_fb_factor[] = {
> +	100, 50, 20, 10
> +};
> +
> +static const u32 ltc4283_cooling_dl[] = {
> +	512, 1002, 2005, 4100, 8190, 16400, 32800, 65600
> +};
> +
> +static const u32 ltc4283_fet_bad_delay[] = {
> +	256, 512, 1002, 2005
> +};
> +
> +static int ltc4283_setup(struct ltc4283_hwmon *st, struct device *dev)
> +{
> +	u32 val;
> +	int ret;
> +
> +	/* The part has an eeprom so let's get the needed defaults from it */
> +	ret = ltc4283_get_defaults(st);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Default to LTC4283_MIN_RSENSE so we can probe without FW properties.
> +	 */
> +	st->rsense = LTC4283_MIN_RSENSE;
> +	ret = device_property_read_u32(dev, "adi,rsense-nano-ohms",
> +				       &st->rsense);
> +	if (!ret) {
> +		if (st->rsense < LTC4283_MIN_RSENSE || st->rsense > LTC4283_MAX_RSENSE)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "adi,rsense-nano-ohms(%u) too small or too large [%u %u]\n",
> +					     st->rsense, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE);
> +	}
> +
> +	/*
> +	 * The resolution for rsense is tenths of micro (eg: 62.5 uOhm) which
> +	 * means we need nano in the bindings. However, to make things easier to
> +	 * handle (with respect to overflows) we divide it by 100 as we don't
> +	 * really need the last two digits.
> +	 */
> +	st->rsense /= CENTI;
> +
> +	ret = device_property_read_u32(dev, "adi,current-limit-sense-microvolt",
> +				       &st->vsense_max);
> +	if (!ret) {
> +		u32 reg_val;
> +
> +		if (!in_range(st->vsense_max, LTC4283_VILIM_MIN_uV,
> +			      LTC4283_VILIM_RANGE)) {
> +			return dev_err_probe(dev, -EINVAL,
> +					     "adi,current-limit-sense-microvolt (%u) out of range [%u %u]\n",
> +					     st->vsense_max, LTC4283_VILIM_MIN_uV,
> +					     LTC4283_VILIM_MAX_uV);
> +		}
> +
> +		st->vsense_max /= MILLI;
> +		reg_val = FIELD_PREP(LTC4283_ILIM_MASK,
> +				     st->vsense_max - LTC4283_VILIM_MIN_uV / MILLI);
> +		ret = regmap_update_bits(st->map, LTC4283_CONFIG_1,
> +					 LTC4283_ILIM_MASK, reg_val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ltc4283_parse_array_prop(st, dev, "adi,current-limit-foldback-factor",
> +				       ltc4283_fb_factor, ARRAY_SIZE(ltc4283_fb_factor));
> +	if (ret < 0)
> +		return ret;
> +	if (ret < ARRAY_SIZE(ltc4283_fb_factor)) {
> +		ret = regmap_update_bits(st->map, LTC4283_CONFIG_1, LTC4283_FB_MASK,
> +					 FIELD_PREP(LTC4283_FB_MASK, ret));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ltc4283_parse_array_prop(st, dev, "adi,cooling-delay-ms",
> +				       ltc4283_cooling_dl, ARRAY_SIZE(ltc4283_cooling_dl));
> +	if (ret < 0)
> +		return ret;
> +	if (ret < ARRAY_SIZE(ltc4283_cooling_dl)) {
> +		ret = regmap_update_bits(st->map, LTC4283_CONFIG_2, LTC4283_COOLING_DL_MASK,
> +					 FIELD_PREP(LTC4283_COOLING_DL_MASK, ret));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ltc4283_parse_array_prop(st, dev, "adi,fet-bad-timer-delay-ms",
> +				       ltc4283_fet_bad_delay, ARRAY_SIZE(ltc4283_fet_bad_delay));
> +	if (ret < 0)
> +		return ret;
> +	if (ret < ARRAY_SIZE(ltc4283_fet_bad_delay)) {
> +		ret = regmap_update_bits(st->map, LTC4283_CONFIG_2, LTC4283_FTBD_DL_MASK,
> +					 FIELD_PREP(LTC4283_FTBD_DL_MASK, ret));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = ltc4283_set_max_limits(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = ltc4283_pin_config(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	if (device_property_read_bool(dev, "adi,power-good-reset-on-fet")) {
> +		ret = regmap_clear_bits(st->map, LTC4283_CONTROL_1,
> +					LTC4283_PWRGD_RST_CTRL_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,fet-turn-off-disable")) {
> +		ret = regmap_clear_bits(st->map, LTC4283_CONTROL_1,
> +					LTC4283_FET_BAD_OFF_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,tmr-pull-down-disable")) {
> +		ret = regmap_set_bits(st->map, LTC4283_CONTROL_1,
> +				      LTC4283_THERM_TMR_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,dvdt-inrush-control-disable")) {
> +		ret = regmap_clear_bits(st->map, LTC4283_CONTROL_1,
> +					LTC4283_DVDT_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,undervoltage-retry-disable")) {
> +		ret = regmap_clear_bits(st->map, LTC4283_CONTROL_2,
> +					LTC4283_UV_RETRY_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,overvoltage-retry-disable")) {
> +		ret = regmap_clear_bits(st->map, LTC4283_CONTROL_2,
> +					LTC4283_OV_RETRY_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,external-fault-retry-enable")) {
> +		if (!st->ext_fault)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "adi,external-fault-retry-enable set but PGIO4 not configured\n");
> +		ret = regmap_set_bits(st->map, LTC4283_CONTROL_2,
> +				      LTC4283_EXT_FAULT_RETRY_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,fault-log-enable")) {
> +		ret = regmap_set_bits(st->map, LTC4283_FAULT_LOG_CTRL,
> +				      LTC4283_FAULT_LOG_EN_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = device_property_match_property_string(dev, "adi,overcurrent-retries",
> +						    ltc4283_oc_fet_retry,
> +						    ARRAY_SIZE(ltc4283_oc_fet_retry));
> +	/* We still want to catch when an invalid string is given. */
> +	if (ret < 0 && ret != -EINVAL)
> +		return dev_err_probe(dev, ret,
> +				     "adi,overcurrent-retries invalid value\n");
> +	if (ret >= 0) {
> +		ret = regmap_update_bits(st->map, LTC4283_CONTROL_2,
> +					 LTC4283_OC_RETRY_MASK,
> +					 FIELD_PREP(LTC4283_OC_RETRY_MASK, ret));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = device_property_match_property_string(dev, "adi,fet-bad-retries",
> +						    ltc4283_oc_fet_retry,
> +						    ARRAY_SIZE(ltc4283_oc_fet_retry));
> +	if (ret < 0 && ret != -EINVAL)
> +		return dev_err_probe(dev, ret,
> +				     "adi,fet-bad-retries invalid value\n");
> +	if (ret >= 0) {
> +		ret = regmap_update_bits(st->map, LTC4283_CONTROL_2,
> +					 LTC4283_FET_BAD_RETRY_MASK,
> +					 FIELD_PREP(LTC4283_FET_BAD_RETRY_MASK, ret));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,external-fault-fet-off-enable")) {
> +		if (!st->ext_fault)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "adi,external-fault-fet-off-enable set but PGIO4 not configured\n");
> +		ret = regmap_set_bits(st->map, LTC4283_CONFIG_3,
> +				      LTC4283_EXTFLT_TURN_OFF_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (device_property_read_bool(dev, "adi,vpower-drns-enable")) {
> +		u32 chan = LTC4283_CHAN_DRNS - LTC4283_CHAN_ADI_1;
> +
> +		__clear_bit(LTC4283_CHAN_DRNS, &st->ch_enable_mask);
> +		/*
> +		 * Then, let's by default disable DRNS from ADC2 given that it
> +		 * is already being monitored by the VPWR channel. One can still
> +		 * enable it later on if needed.
> +		 */
> +		ret = regmap_clear_bits(st->map, LTC4283_ADC_SELECT(chan),
> +					LTC4283_ADC_SELECT_MASK(chan));
> +		if (ret)
> +			return ret;
> +
> +		val = 1;
> +	} else {
> +		val = 0;
> +	}
> +
> +	ret = regmap_update_bits(st->map, LTC4283_CONFIG_3,
> +				 LTC4283_VPWR_DRNS_MASK,
> +				 FIELD_PREP(LTC4283_VPWR_DRNS_MASK, val));
> +	if (ret)
> +		return ret;
> +
> +	/* Make sure the ADC has 12bit resolution since we're assuming that. */
> +	ret = regmap_update_bits(st->map, LTC4283_PGIO_CONFIG_2,
> +				 LTC4283_ADC_MASK,
> +				 FIELD_PREP(LTC4283_ADC_MASK, 3));
> +	if (ret)
> +		return ret;
> +
> +	/* Energy reads (which are 6 byte block reads) rely on page access */
> +	ret = regmap_set_bits(st->map, LTC4283_CONTROL_1, LTC4283_RW_PAGE_MASK);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Make sure we are integrating power as we only support reporting
> +	 * consumed energy.
> +	 */
> +	return regmap_clear_bits(st->map, LTC4283_METER_CONTROL,
> +				 LTC4283_INTEGRATE_I_MASK);
> +}
> +
> +static const struct hwmon_channel_info * const ltc4283_info[] = {
> +	HWMON_CHANNEL_INFO(in,
> +			   HWMON_I_LCRIT_ALARM | HWMON_I_CRIT_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_MAX_ALARM | HWMON_I_RESET_HISTORY |
> +			   HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_FAULT | HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL,
> +			   HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST |
> +			   HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM |
> +			   HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM |
> +			   HWMON_I_ENABLE | HWMON_I_LABEL),
> +	HWMON_CHANNEL_INFO(curr,
> +			   HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST |
> +			   HWMON_C_MAX | HWMON_C_MIN | HWMON_C_MIN_ALARM |
> +			   HWMON_C_MAX_ALARM | HWMON_C_CRIT_ALARM |
> +			   HWMON_C_RESET_HISTORY | HWMON_C_LABEL),
> +	HWMON_CHANNEL_INFO(power,
> +			   HWMON_P_INPUT | HWMON_P_INPUT_LOWEST |
> +			   HWMON_P_INPUT_HIGHEST | HWMON_P_MAX | HWMON_P_MIN |
> +			   HWMON_P_MAX_ALARM | HWMON_P_MIN_ALARM |
> +			   HWMON_P_RESET_HISTORY | HWMON_P_LABEL),
> +	HWMON_CHANNEL_INFO(energy,
> +			   HWMON_E_ENABLE),
> +	HWMON_CHANNEL_INFO(energy64,
> +			   HWMON_E_INPUT),
> +	NULL
> +};
> +
> +static const struct hwmon_ops ltc4283_ops = {
> +	.read = ltc4283_read,
> +	.write = ltc4283_write,
> +	.is_visible = ltc4283_is_visible,
> +	.read_string = ltc4283_read_labels,
> +};
> +
> +static const struct hwmon_chip_info ltc4283_chip_info = {
> +	.ops = &ltc4283_ops,
> +	.info = ltc4283_info,
> +};
> +
> +static int ltc4283_show_fault_log(void *arg, u64 *val, u32 mask)
> +{
> +	struct ltc4283_hwmon *st = arg;
> +	long alarm;
> +	int ret;
> +
> +	ret = ltc4283_read_alarm(st, LTC4283_FAULT_LOG, mask, &alarm);
> +	if (ret)
> +		return ret;
> +
> +	*val = alarm;
> +
> +	return 0;
> +}
> +
> +static int ltc4283_show_in0_lcrit_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_UV_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_in0_lcrit_fault_log,
> +			 ltc4283_show_in0_lcrit_fault_log, NULL, "%llu\n");
> +
> +static int ltc4283_show_in0_crit_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_OV_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_in0_crit_fault_log,
> +			 ltc4283_show_in0_crit_fault_log, NULL, "%llu\n");
> +
> +static int ltc4283_show_fet_bad_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_FET_BAD_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_fet_bad_fault_log,
> +			 ltc4283_show_fet_bad_fault_log, NULL, "%llu\n");
> +
> +static int ltc4283_show_fet_short_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_FET_SHORT_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_fet_short_fault_log,
> +			 ltc4283_show_fet_short_fault_log, NULL, "%llu\n");
> +
> +static int ltc4283_show_curr1_crit_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_OC_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_curr1_crit_fault_log,
> +			 ltc4283_show_curr1_crit_fault_log, NULL, "%llu\n");
> +
> +static int ltc4283_show_power1_failed_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_PWR_FAIL_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_power1_failed_fault_log,
> +			 ltc4283_show_power1_failed_fault_log, NULL, "%llu\n");
> +
> +static int ltc4283_show_power1_good_input_fault_log(void *arg, u64 *val)
> +{
> +	return ltc4283_show_fault_log(arg, val, LTC4283_PGI_FAULT_MASK);
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(ltc4283_power1_good_input_fault_log,
> +			 ltc4283_show_power1_good_input_fault_log, NULL, "%llu\n");
> +
> +static void ltc4283_debugfs_init(struct ltc4283_hwmon *st, struct i2c_client *i2c)
> +{
> +	debugfs_create_file_unsafe("in0_crit_fault_log", 0400, i2c->debugfs, st,
> +				   &ltc4283_in0_crit_fault_log);
> +	debugfs_create_file_unsafe("in0_lcrit_fault_log", 0400, i2c->debugfs, st,
> +				   &ltc4283_in0_lcrit_fault_log);
> +	debugfs_create_file_unsafe("in0_fet_bad_fault_log", 0400, i2c->debugfs, st,
> +				   &ltc4283_fet_bad_fault_log);
> +	debugfs_create_file_unsafe("in0_fet_short_fault_log", 0400, i2c->debugfs, st,
> +				   &ltc4283_fet_short_fault_log);
> +	debugfs_create_file_unsafe("curr1_crit_fault_log", 0400, i2c->debugfs, st,
> +				   &ltc4283_curr1_crit_fault_log);
> +	debugfs_create_file_unsafe("power1_failed_fault_log", 0400, i2c->debugfs, st,
> +				   &ltc4283_power1_failed_fault_log);
> +	debugfs_create_file_unsafe("power1_good_input_fault_log", 0400, i2c->debugfs,
> +				   st, &ltc4283_power1_good_input_fault_log);
> +}
> +
> +static bool ltc4283_is_word_reg(unsigned int reg)
> +{
> +	return reg >= LTC4283_SENSE && reg <= LTC4283_ADIO34_MAX;
> +}
> +
> +static int ltc4283_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct i2c_client *client = context;
> +	int ret;
> +
> +	if (ltc4283_is_word_reg(reg))
> +		ret = i2c_smbus_read_word_swapped(client, reg);
> +	else
> +		ret = i2c_smbus_read_byte_data(client, reg);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = ret;
> +	return 0;
> +}
> +
> +static int ltc4283_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct i2c_client *client = context;
> +
> +	if (ltc4283_is_word_reg(reg))
> +		return i2c_smbus_write_word_swapped(client, reg, val);
> +
> +	return i2c_smbus_write_byte_data(client, reg, val);
> +}
> +
> +static const struct regmap_bus ltc4283_regmap_bus = {
> +	.reg_read = ltc4283_reg_read,
> +	.reg_write = ltc4283_reg_write,
> +};
> +
> +static bool ltc4283_writable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case LTC4283_SYSTEM_STATUS ... LTC4283_FAULT_STATUS:
> +		return false;
> +	case LTC4283_RESERVED_OC:
> +		return false;
> +	case LTC4283_RESERVED_86 ... LTC4283_RESERVED_8F:
> +		return false;
> +	case LTC4283_RESERVED_91 ... LTC4283_RESERVED_A1:
> +		return false;
> +	case LTC4283_RESERVED_A3:
> +		return false;
> +	case LTC4283_RESERVED_AC:
> +		return false;
> +	case LTC4283_POWER_PLAY_MSB ... LTC4283_POWER_PLAY_LSB:
> +		return false;
> +	case LTC4283_RESERVED_F1 ... LTC4283_RESERVED_FF:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
> +static const struct regmap_config ltc4283_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.max_register = 0xFF,
> +	.writeable_reg = ltc4283_writable_reg,
> +};
> +
> +static int ltc4283_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev, *hwmon;
> +	struct auxiliary_device *adev;
> +	struct ltc4283_hwmon *st;
> +	int ret, id;
> +
> +	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> +	if (!st)
> +		return -ENOMEM;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_READ_I2C_BLOCK))
> +		return -EOPNOTSUPP;
> +
> +	st->client = client;
> +	st->map = devm_regmap_init(dev, &ltc4283_regmap_bus, client,
> +				   &ltc4283_regmap_config);
> +	if (IS_ERR(st->map))
> +		return dev_err_probe(dev, PTR_ERR(st->map),
> +				     "Failed to create regmap\n");
> +
> +	ret = ltc4283_setup(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	hwmon = devm_hwmon_device_register_with_info(dev, "ltc4283", st,
> +						     &ltc4283_chip_info, NULL);
> +
> +	if (IS_ERR(hwmon))
> +		return PTR_ERR(hwmon);
> +
> +	ltc4283_debugfs_init(st, client);
> +
> +	if (!st->gpio_mask)
> +		return 0;
> +
> +	id = (client->adapter->nr << 10) | client->addr;
> +	adev = __devm_auxiliary_device_create(dev, KBUILD_MODNAME, "gpio",
> +					      NULL, id);
> +	if (!adev)
> +		return dev_err_probe(dev, -ENODEV, "Failed to add GPIO device\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ltc4283_of_match[] = {
> +	{ .compatible = "adi,ltc4283" },
> +	{ }
> +};
> +
> +static const struct i2c_device_id ltc4283_i2c_id[] = {
> +	{ "ltc4283" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc4283_i2c_id);
> +
> +static struct i2c_driver ltc4283_driver = {
> +	.driver	= {
> +		.name = "ltc4283",
> +		.of_match_table = ltc4283_of_match,
> +	},
> +	.probe = ltc4283_probe,
> +	.id_table = ltc4283_i2c_id,
> +};
> +module_i2c_driver(ltc4283_driver);
> +
> +MODULE_AUTHOR("Nuno Sá <nuno.sa@analog.com>");
> +MODULE_DESCRIPTION("LTC4283 Hot Swap Controller driver");
> +MODULE_LICENSE("GPL");
> 


^ permalink raw reply

* Re: [PATCH V3 6/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607
From: David Lechner @ 2026-04-10 22:59 UTC (permalink / raw)
  To: Chris Morgan, linux-iio
  Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
	devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
	Chris Morgan
In-Reply-To: <20260330195853.392877-7-macroalpha82@gmail.com>

On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add icm42607 accelerometer sensor for icm42607.
> 

...

> +static const unsigned long inv_icm42607_accel_scan_masks[] = {
> +	/* 3-axis accel + temperature */
> +	INV_ICM42607_SCAN_MASK_ACCEL_3AXIS | INV_ICM42607_SCAN_MASK_TEMP,

This is going to make it so that the temperature channel is always read
even if it isn't enabled and additional work is needed when pushing to
buffers to remove it again.

It looks like it is possible to read accel and temp separatly, so
there shuold be two more lines here,

	INV_ICM42607_SCAN_MASK_ACCEL_3AXIS,
	INV_ICM42607_SCAN_MASK_TEMP,

I forget what the correct order is though.

> +	0,
> +};
> +
> +/* enable accelerometer sensor and FIFO write */
> +static int inv_icm42607_accel_update_scan_mode(struct iio_dev *indio_dev,
> +					       const unsigned long *scan_mask)
> +{
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +	struct inv_icm42607_sensor_state *accel_st = iio_priv(indio_dev);
> +	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> +	unsigned int fifo_en = 0;
> +	unsigned int sleep_temp = 0;
> +	unsigned int sleep_accel = 0;
> +	unsigned int sleep;
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +
> +	if (*scan_mask & INV_ICM42607_SCAN_MASK_TEMP) {
> +		/* enable temp sensor */
> +		ret = inv_icm42607_set_temp_conf(st, true, &sleep_temp);
> +		if (ret)
> +			goto out_unlock;
> +		fifo_en |= INV_ICM42607_SENSOR_TEMP;
> +	}
> +
> +	if (*scan_mask & INV_ICM42607_SCAN_MASK_ACCEL_3AXIS) {
> +		/* enable accel sensor */
> +		conf.mode = accel_st->power_mode;
> +		conf.filter = accel_st->filter;
> +		ret = inv_icm42607_set_accel_conf(st, &conf, &sleep_accel);
> +		if (ret)
> +			goto out_unlock;
> +		fifo_en |= INV_ICM42607_SENSOR_ACCEL;
> +	}
> +
> +	/* update data FIFO write */
> +	ret = inv_icm42607_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> +
> +out_unlock:
> +	mutex_unlock(&st->lock);
> +	/* sleep maximum required time */

Would be better if the comment explain _why_ we need to sleep.

The code is pretty obvious that it does what the comment says, so
it doesn't add much.

> +	sleep = max(sleep_accel, sleep_temp);
> +	if (sleep)

Probably don't need the if here as msleep() should handle 0 without actually
sleeping.

> +		msleep(sleep);
> +	return ret;
> +}
> +
> +static int inv_icm42607_accel_read_sensor(struct iio_dev *indio_dev,
> +					  struct iio_chan_spec const *chan,
> +					  s16 *val)
> +{
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +	struct inv_icm42607_sensor_state *accel_st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->map);
> +	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> +	unsigned int reg;
> +	__be16 *data;
> +	int ret;
> +
> +	if (chan->type != IIO_ACCEL)
> +		return -EINVAL;
> +
> +	switch (chan->channel2) {
> +	case IIO_MOD_X:
> +		reg = INV_ICM42607_REG_ACCEL_DATA_X1;
> +		break;
> +	case IIO_MOD_Y:
> +		reg = INV_ICM42607_REG_ACCEL_DATA_Y1;
> +		break;
> +	case IIO_MOD_Z:
> +		reg = INV_ICM42607_REG_ACCEL_DATA_Z1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> +	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> +		return -ENXIO;
> +
> +	guard(mutex)(&st->lock);
> +
> +	/* enable accel sensor */
> +	conf.mode = accel_st->power_mode;
> +	conf.filter = accel_st->filter;
> +	ret = inv_icm42607_set_accel_conf(st, &conf, NULL);
> +	if (ret)
> +		return ret;
> +
> +	/* read accel register data */
> +	data = (__be16 *)&st->buffer[0];
> +	ret = regmap_bulk_read(st->map, reg, data, sizeof(*data));
> +	if (ret)
> +		return ret;
> +
> +	*val = (int16_t)be16_to_cpup(data);

We don't use int16_t in the kernel (ideally). Stick with s16.

Although cast isn't needed here since val is already s16.

> +	if (*val == INV_ICM42607_DATA_INVALID)
> +		ret = -EINVAL;
> +
> +	return ret;
> +}
> +
> +/* IIO format int + nano */

Usually we make these 2-D arrays for readability and then cast to int * if needed.

> +static const int inv_icm42607_accel_scale[] = {
> +	/* +/- 16G => 0.004788403 m/s-2 */
> +	[2 * INV_ICM42607_ACCEL_FS_16G] = 0,
> +	[2 * INV_ICM42607_ACCEL_FS_16G + 1] = 4788403,
> +	/* +/- 8G => 0.002394202 m/s-2 */
> +	[2 * INV_ICM42607_ACCEL_FS_8G] = 0,
> +	[2 * INV_ICM42607_ACCEL_FS_8G + 1] = 2394202,
> +	/* +/- 4G => 0.001197101 m/s-2 */
> +	[2 * INV_ICM42607_ACCEL_FS_4G] = 0,
> +	[2 * INV_ICM42607_ACCEL_FS_4G + 1] = 1197101,
> +	/* +/- 2G => 0.000598550 m/s-2 */
> +	[2 * INV_ICM42607_ACCEL_FS_2G] = 0,
> +	[2 * INV_ICM42607_ACCEL_FS_2G + 1] = 598550,
> +};
> +

...

> +static int inv_icm42607_accel_read_calibbias(struct inv_icm42607_state *st,
> +					     struct iio_chan_spec const *chan,
> +					     int *val, int *val2)
> +{
> +	/* Not actually supported in the ICM-42607P registers */
> +	return -EOPNOTSUPP;
> +}

Can we just not create the attribute instead of returning an error?


> +static int inv_icm42607_accel_write_raw_get_fmt(struct iio_dev *indio_dev,
> +						struct iio_chan_spec const *chan,
> +						long mask)
> +{
> +	if (chan->type != IIO_ACCEL)
> +		return -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		return IIO_VAL_INT_PLUS_MICRO;

Can write this as:

	case IIO_CHAN_INFO_SAMP_FREQ:
	case IIO_CHAN_INFO_CALIBBIAS:
		return IIO_VAL_INT_PLUS_MICRO;

> +	default:
> +		return -EINVAL;
> +	}
> +}
> +

...

> +int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,
> +				struct inv_icm42607_sensor_conf *conf,
> +				unsigned int *sleep_ms)
> +{
> +	struct inv_icm42607_sensor_conf *oldconf = &st->conf.accel;
> +	unsigned int val;
> +	int ret;
> +
> +	if (conf->mode < 0)
> +		conf->mode = oldconf->mode;
> +	if (conf->fs < 0)
> +		conf->fs = oldconf->fs;
> +	if (conf->odr < 0)
> +		conf->odr = oldconf->odr;
> +	if (conf->filter < 0)
> +		conf->filter = oldconf->filter;
> +
> +	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {

We could use the regmap cache feature to avoid having to manual keep
track of old values. Or just always write the same values anyway. I
find that is nice when debugging hardware with a logic analyzer. Unless
there is some measureable performance improvlment here?

> +		val = INV_ICM42607_ACCEL_CONFIG0_FS_SEL(conf->fs) |
> +		INV_ICM42607_ACCEL_CONFIG0_ODR(conf->odr);
> +		ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG0, val);
> +		if (ret)
> +			return ret;
> +		oldconf->fs = conf->fs;
> +		oldconf->odr = conf->odr;
> +	}
> +
> +	if (conf->filter != oldconf->filter) {
> +		if (conf->mode == INV_ICM42607_SENSOR_MODE_LOW_POWER) {
> +			val = INV_ICM42607_ACCEL_CONFIG1_AVG(conf->filter);
> +			ret = regmap_update_bits(st->map, INV_ICM42607_REG_ACCEL_CONFIG1,
> +						 INV_ICM42607_ACCEL_CONFIG1_AVG_MASK, val);
> +		} else {
> +			val = INV_ICM42607_ACCEL_CONFIG1_FILTER(conf->filter);
> +			ret = regmap_update_bits(st->map, INV_ICM42607_REG_ACCEL_CONFIG1,
> +						 INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK, val);
> +		}
> +		if (ret)
> +			return ret;
> +		oldconf->filter = conf->filter;
> +	}
> +
> +	return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
> +					  st->conf.temp_en, sleep_ms);
> +}
> +

^ permalink raw reply

* [PATCH] dt-bindings: thermal: Fix false warning with 'phandle' in trips nodes
From: Rob Herring (Arm) @ 2026-04-10 22:36 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-pm, devicetree, linux-kernel

A pattern property matching essentially anything doesn't work if there
are implicit properties such as 'phandle' which can occur on any node.
One such example popped up recently:

arch/arm64/boot/dts/qcom/sm8650-hdk.dtb: thermal-zones: gpuss0-thermal:trips:phandle: 531 is not of type 'object'
        from schema $id: http://devicetree.org/schemas/thermal/thermal-zones.yaml

Instead of a pattern property, use an "additionalProperties" schema
instead which is the fallback in case of no matching property.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
Daniel, Please pick this up for v7.1 as the above warning is in next. Or 
if you prefer, I can take it.

 .../bindings/thermal/thermal-zones.yaml       | 111 +++++++++---------
 1 file changed, 54 insertions(+), 57 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
index 0de0a9757ccc..07d9f576ffe7 100644
--- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
+++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
@@ -129,63 +129,60 @@ patternProperties:
           which the thermal framework needs to take action. The actions to
           be taken are defined in another node called cooling-maps.
 
-        patternProperties:
-          "^[a-zA-Z][a-zA-Z0-9\\-_]{0,63}$":
-            type: object
-
-            properties:
-              temperature:
-                $ref: /schemas/types.yaml#/definitions/int32
-                minimum: -273000
-                maximum: 200000
-                description:
-                  An integer expressing the trip temperature in millicelsius.
-
-              hysteresis:
-                $ref: /schemas/types.yaml#/definitions/uint32
-                description:
-                  An unsigned integer expressing the hysteresis delta with
-                  respect to the trip temperature property above, also in
-                  millicelsius. Any cooling action initiated by the framework is
-                  maintained until the temperature falls below
-                  (trip temperature - hysteresis). This potentially prevents a
-                  situation where the trip gets constantly triggered soon after
-                  cooling action is removed.
-
-              type:
-                $ref: /schemas/types.yaml#/definitions/string
-                enum:
-                  - active   # enable active cooling e.g. fans
-                  - passive  # enable passive cooling e.g. throttling cpu
-                  - hot      # send notification to driver
-                  - critical # send notification to driver, trigger shutdown
-                description: |
-                  There are four valid trip types: active, passive, hot,
-                  critical.
-
-                  The critical trip type is used to set the maximum
-                  temperature threshold above which the HW becomes
-                  unstable and underlying firmware might even trigger a
-                  reboot. Hitting the critical threshold triggers a system
-                  shutdown.
-
-                  The hot trip type can be used to send a notification to
-                  the thermal driver (if a .notify callback is registered).
-                  The action to be taken is left to the driver.
-
-                  The passive trip type can be used to slow down HW e.g. run
-                  the CPU, GPU, bus at a lower frequency.
-
-                  The active trip type can be used to control other HW to
-                  help in cooling e.g. fans can be sped up or slowed down
-
-            required:
-              - temperature
-              - hysteresis
-              - type
-            additionalProperties: false
-
-        additionalProperties: false
+        additionalProperties:
+          type: object
+          additionalProperties: false
+
+          properties:
+            temperature:
+              $ref: /schemas/types.yaml#/definitions/int32
+              minimum: -273000
+              maximum: 200000
+              description:
+                An integer expressing the trip temperature in millicelsius.
+
+            hysteresis:
+              $ref: /schemas/types.yaml#/definitions/uint32
+              description:
+                An unsigned integer expressing the hysteresis delta with
+                respect to the trip temperature property above, also in
+                millicelsius. Any cooling action initiated by the framework is
+                maintained until the temperature falls below
+                (trip temperature - hysteresis). This potentially prevents a
+                situation where the trip gets constantly triggered soon after
+                cooling action is removed.
+
+            type:
+              $ref: /schemas/types.yaml#/definitions/string
+              enum:
+                - active   # enable active cooling e.g. fans
+                - passive  # enable passive cooling e.g. throttling cpu
+                - hot      # send notification to driver
+                - critical # send notification to driver, trigger shutdown
+              description: |
+                There are four valid trip types: active, passive, hot,
+                critical.
+
+                The critical trip type is used to set the maximum
+                temperature threshold above which the HW becomes
+                unstable and underlying firmware might even trigger a
+                reboot. Hitting the critical threshold triggers a system
+                shutdown.
+
+                The hot trip type can be used to send a notification to
+                the thermal driver (if a .notify callback is registered).
+                The action to be taken is left to the driver.
+
+                The passive trip type can be used to slow down HW e.g. run
+                the CPU, GPU, bus at a lower frequency.
+
+                The active trip type can be used to control other HW to
+                help in cooling e.g. fans can be sped up or slowed down
+
+          required:
+            - temperature
+            - hysteresis
+            - type
 
       cooling-maps:
         type: object
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH V3 5/9] iio: imu: inv_icm42607: Add Temperature Support in icm42607
From: David Lechner @ 2026-04-10 22:34 UTC (permalink / raw)
  To: Chris Morgan, linux-iio
  Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
	devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
	Chris Morgan
In-Reply-To: <20260330195853.392877-6-macroalpha82@gmail.com>

On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add functions for reading temperature sensor data.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  3 +
>  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 17 ++++
>  .../iio/imu/inv_icm42607/inv_icm42607_temp.c  | 81 +++++++++++++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_temp.h  | 30 +++++++
>  4 files changed, 131 insertions(+)
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
> 
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index 5530fd3bc03f..086848c8fd3b 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -433,6 +433,9 @@ extern const struct dev_pm_ops inv_icm42607_pm_ops;
>  
>  u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr);
>  
> +int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
> +			       unsigned int *sleep_ms);
> +
>  int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
>  			     unsigned int writeval, unsigned int *readval);
>  
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 344071089042..735a262dc103 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -164,6 +164,23 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
>  	return 0;
>  }
>  
> +int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
> +			       unsigned int *sleep_ms)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	val = INV_ICM42607_TEMP_CONFIG0_FILTER(INV_ICM42607_FILTER_BW_34HZ);
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_TEMP_CONFIG0,
> +				 INV_ICM42607_TEMP_CONFIG0_FILTER_MASK, val);
> +	if (ret)
> +		return ret;
> +
> +	return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode,
> +					  st->conf.accel.mode, enable,
> +					  sleep_ms);
> +}
> +
>  int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
>  			     unsigned int writeval, unsigned int *readval)
>  {
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> new file mode 100644
> index 000000000000..b42eb78cd960
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +
> +#include "inv_icm42607.h"
> +#include "inv_icm42607_temp.h"
> +
> +static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
> +{
> +	struct device *dev = regmap_get_device(st->map);
> +	__be16 *raw;
> +	int ret;
> +
> +	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> +	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> +		return -ENXIO;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = inv_icm42607_set_temp_conf(st, true, NULL);
> +	if (ret)
> +		return ret;
> +
> +	raw = (__be16 *)&st->buffer[0];

Can we make buffer __be16 buffer[] to avoid cast and ensure proper alignment?

> +	ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1, raw, sizeof(*raw));
> +	if (ret)
> +		return ret;
> +
> +	*temp = (s16)be16_to_cpup(raw);

cast is not needed. temp is already s16.

> +	if (*temp == INV_ICM42607_DATA_INVALID)
> +		ret = -EINVAL;
> +
> +	return ret;
> +}
> +
> +int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +	s16 temp;
> +	int ret;
> +
> +	if (chan->type != IIO_TEMP)
> +		return -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (!iio_device_claim_direct(indio_dev))
> +			return -EBUSY;
> +		ret = inv_icm42607_temp_read(st, &temp);
> +		iio_device_release_direct(indio_dev);
> +		if (ret)
> +			return ret;
> +		*val = temp;
> +		return IIO_VAL_INT;
> +	/*
> +	 * T°C = (temp / 128) + 25
> +	 * Tm°C = 1000 * ((temp * 100 / 12800) + 25)
> +	 * scale: 100000 / 12800 ~= 7.8125
> +	 * offset: 25000
> +	 */
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 7;
> +		*val2 = 812500;
> +		return IIO_VAL_INT_PLUS_MICRO;

Could use IIO_VAL_INT_PLUS_NANO to get more exact value.

> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 25000;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}

^ permalink raw reply

* Re: [PATCH 2/3] pmdomain: core: add support for power-domains-child-ids
From: Kevin Hilman @ 2026-04-10 22:25 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Geert Uytterhoeven, linux-pm, devicetree,
	linux-kernel, arm-scmi, linux-arm-kernel
In-Reply-To: <CAPDyKFrR2zyMFXTAkKs1XRgB-u5jSP256g730s=7SLuOZKsKVg@mail.gmail.com>

Ulf Hansson <ulf.hansson@linaro.org> writes:

> On Fri, 10 Apr 2026 at 02:45, Kevin Hilman <khilman@baylibre.com> wrote:
>>
>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>
>> > On Wed, 11 Mar 2026 at 01:19, Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>> >>
>> >> Currently, PM domains can only support hierarchy for simple
>> >> providers (e.g. ones with #power-domain-cells = 0).
>> >>
>> >> Add support for oncell providers as well by adding a new property
>> >> `power-domains-child-ids` to describe the parent/child relationship.
>> >>
>> >> For example, an SCMI PM domain provider has multiple domains, each of
>> >> which might be a child of diffeent parent domains. In this example,
>> >> the parent domains are MAIN_PD and WKUP_PD:
>> >>
>> >>     scmi_pds: protocol@11 {
>> >>         reg = <0x11>;
>> >>         #power-domain-cells = <1>;
>> >>         power-domains = <&MAIN_PD>, <&WKUP_PD>;
>> >>         power-domains-child-ids = <15>, <19>;
>> >>     };
>> >>
>> >> With this example using the new property, SCMI PM domain 15 becomes a
>> >> child domain of MAIN_PD, and SCMI domain 19 becomes a child domain of
>> >> WKUP_PD.
>> >>
>> >> To support this feature, add two new core functions
>> >>
>> >> - of_genpd_add_child_ids()
>> >> - of_genpd_remove_child_ids()
>> >>
>> >> which can be called by pmdomain providers to add/remove child domains
>> >> if they support the new property power-domains-child-ids.
>> >>
>> >> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
>> >
>> > Thanks for working on this! It certainly is a missing feature!
>>
>> You're welcome, thanks for the detailed review.
>>
>> >> ---
>> >>  drivers/pmdomain/core.c   | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  include/linux/pm_domain.h |  16 ++++++++++++++++
>> >>  2 files changed, 185 insertions(+)
>> >>
>> >> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
>> >> index 61c2277c9ce3..acb45dd540b7 100644
>> >> --- a/drivers/pmdomain/core.c
>> >> +++ b/drivers/pmdomain/core.c
>> >> @@ -2909,6 +2909,175 @@ static struct generic_pm_domain *genpd_get_from_provider(
>> >>         return genpd;
>> >>  }
>> >>
>> >> +/**
>> >> + * of_genpd_add_child_ids() - Parse power-domains-child-ids property
>> >> + * @np: Device node pointer associated with the PM domain provider.
>> >> + * @data: Pointer to the onecell data associated with the PM domain provider.
>> >> + *
>> >> + * Parse the power-domains and power-domains-child-ids properties to establish
>> >> + * parent-child relationships for PM domains. The power-domains property lists
>> >> + * parent domains, and power-domains-child-ids lists which child domain IDs
>> >> + * should be associated with each parent.
>> >> + *
>> >> + * Returns 0 on success, -ENOENT if properties don't exist, or negative error code.
>> >
>> > I think we should avoid returning specific error codes for specific
>> > errors, simply because it usually becomes messy.
>> >
>> > If I understand correctly the intent here is to allow the caller to
>> > check for -ENOENT and potentially avoid bailing out as it may not
>> > really be an error, right?
>>
>> Right, -ENOENT is not an error of parsing, it's to indicate that there
>> are no child-ids to be parsed.
>>
>> > Perhaps a better option is to return the number of children for whom
>> > we successfully assigned parents. Hence 0 or a positive value allows
>> > the caller to understand what happened. More importantly, a negative
>> > error code then really becomes an error for the caller to consider.
>>
>> I explored this a bit, but it gets messy quick.  It means we have to
>> track cases where only some of the children were added as well as when
>> all children were added.   Personally, I think this should be an "all or
>> nothing" thing.  If all the children cannot be parsed/added, then none
>> of them should be added.
>>
>> This also allows the remove to not have to care about how many were
>> added, and just remove them all, with the additional benefit of not
>> having to track the state of how many children were successfully added.
>>
>
> I fully agree, it should be all or nothing. Failing with one
> child/parent should end up with an error code being returned.
>
> That said, it still seems to make perfect sense to return the number
> of children for whom we assigned parents for, no?

No, because what will the caller use that number for?  If we are
assuming "all or nothing", what would we use it for (other than a debug print?)

It also makes it a bit confusing what a zero return value means.  Does
that mean success?  Or that zero children were added (which would be
fail.)

I prefer to keep it as is.

Kevin

^ permalink raw reply

* Re: [PATCH V3 3/9] iio: imu: inv_icm42607: Add I2C and SPI For icm42607
From: David Lechner @ 2026-04-10 22:21 UTC (permalink / raw)
  To: Chris Morgan, linux-iio
  Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
	devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
	Chris Morgan
In-Reply-To: <20260330195853.392877-4-macroalpha82@gmail.com>

On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add I2C and SPI driver support for InvenSense ICM-42607 devices.

> Include runtime power management on each device.

Power management seems unrelated, so likey belongs in a separate patch.

> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  14 ++
>  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 204 ++++++++++++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |  93 ++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_spi.c   | 100 +++++++++
>  4 files changed, 411 insertions(+)
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> 
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index 609188c40ffc..7d13091aa8df 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -11,6 +11,7 @@
>  #include <linux/regmap.h>
>  #include <linux/mutex.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/pm.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/common/inv_sensors_timestamp.h>
>  
> @@ -21,6 +22,16 @@ enum inv_icm42607_chip {
>  	INV_CHIP_NB,
>  };
>  
> +/* serial bus slew rates */
> +enum inv_icm42607_slew_rate {
> +	INV_ICM42607_SLEW_RATE_20_60NS,
> +	INV_ICM42607_SLEW_RATE_12_36NS,
> +	INV_ICM42607_SLEW_RATE_6_18NS,
> +	INV_ICM42607_SLEW_RATE_4_12NS,
> +	INV_ICM42607_SLEW_RATE_2_6NS,
> +	INV_ICM42607_SLEW_RATE_INF_2NS,
> +};
> +
>  enum inv_icm42607_sensor_mode {
>  	INV_ICM42607_SENSOR_MODE_OFF,
>  	INV_ICM42607_SENSOR_MODE_STANDBY,
> @@ -413,6 +424,9 @@ struct inv_icm42607_sensor_state {
>  
>  typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
>  
> +extern const struct regmap_config inv_icm42607_regmap_config;
> +extern const struct dev_pm_ops inv_icm42607_pm_ops;
> +
>  u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr);
>  
>  int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 6b7078387568..da04c820dab2 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -12,12 +12,33 @@
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/iio/iio.h>
>  
>  #include "inv_icm42607.h"
>  
> +static const struct regmap_range_cfg inv_icm42607_regmap_ranges[] = {
> +	{
> +		.name = "user bank",
> +		.range_min = 0x0000,
> +		.range_max = 0x00FF,
> +		.window_start = 0,
> +		.window_len = 0x0100,
> +	},
> +};
> +
> +const struct regmap_config inv_icm42607_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0x00FF,
> +	.ranges = inv_icm42607_regmap_ranges,
> +	.num_ranges = ARRAY_SIZE(inv_icm42607_regmap_ranges),
> +	.cache_type = REGCACHE_NONE,
> +};
> +EXPORT_SYMBOL_NS_GPL(inv_icm42607_regmap_config, "IIO_ICM42607");

It would make more sense to include the regmap config in the first patch
since it is shared.

> +
>  struct inv_icm42607_hw {
>  	uint8_t whoami;
>  	const char *name;
> @@ -86,6 +107,62 @@ u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr)
>  	return odr_periods[odr];
>  }
>  
> +static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> +				      enum inv_icm42607_sensor_mode gyro,
> +				      enum inv_icm42607_sensor_mode accel,
> +				      bool temp, unsigned int *sleep_ms)
> +{
> +	enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
> +	enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
> +	bool oldtemp = st->conf.temp_en;
> +	unsigned int sleepval;
> +	unsigned int val;
> +	int ret;
> +
> +	if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> +		return 0;
> +
> +	val = INV_ICM42607_PWR_MGMT0_GYRO(gyro) |
> +	INV_ICM42607_PWR_MGMT0_ACCEL(accel);
> +	if (!temp)
> +		val |= INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL;
> +	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> +	if (ret)
> +		return ret;
> +
> +	st->conf.gyro.mode = gyro;
> +	st->conf.accel.mode = accel;
> +	st->conf.temp_en = temp;
> +
> +	sleepval = 0;
> +	if (temp && !oldtemp) {
> +		if (sleepval < INV_ICM42607_TEMP_STARTUP_TIME_MS)
> +			sleepval = INV_ICM42607_TEMP_STARTUP_TIME_MS;
> +	}
> +	if (accel != oldaccel && oldaccel == INV_ICM42607_SENSOR_MODE_OFF) {
> +		usleep_range(200, 300);
> +		if (sleepval < INV_ICM42607_ACCEL_STARTUP_TIME_MS)
> +			sleepval = INV_ICM42607_ACCEL_STARTUP_TIME_MS;
> +	}
> +	if (gyro != oldgyro) {
> +		if (oldgyro == INV_ICM42607_SENSOR_MODE_OFF) {
> +			usleep_range(200, 300);
> +			if (sleepval < INV_ICM42607_GYRO_STARTUP_TIME_MS)
> +				sleepval = INV_ICM42607_GYRO_STARTUP_TIME_MS;
> +		} else if (gyro == INV_ICM42607_SENSOR_MODE_OFF) {
> +			if (sleepval < INV_ICM42607_GYRO_STOP_TIME_MS)
> +				sleepval = INV_ICM42607_GYRO_STOP_TIME_MS;
> +		}
> +	}
> +
> +	if (sleep_ms)
> +		*sleep_ms = sleepval;
> +	else if (sleepval)
> +		msleep(sleepval);
> +
> +	return 0;
> +}
> +
>  int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
>  			     unsigned int writeval, unsigned int *readval)
>  {
> @@ -219,6 +296,10 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
>  static void inv_icm42607_disable_vddio_reg(void *_data)
>  {
>  	struct inv_icm42607_state *st = _data;
> +	struct device *dev = regmap_get_device(st->map);
> +
> +	if (pm_runtime_status_suspended(dev))
> +		return;
>  
>  	regulator_disable(st->vddio_supply);
>  }
> @@ -289,11 +370,134 @@ int inv_icm42607_core_probe(struct regmap *regmap, int chip,
>  
>  	/* Setup chip registers (includes WHOAMI check, reset check, bus setup) */
>  	ret = inv_icm42607_setup(st, bus_setup);
> +	if (ret)
> +		return ret; /* Return error from setup (e.g., WHOAMI fail) */
> +
> +	/* Setup runtime power management */

Would add a blank line here if this is meant to apply to more than the
following line. Otherwise it doesn't add much.

> +	ret = devm_pm_runtime_set_active_enabled(dev);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
> +	pm_runtime_use_autosuspend(dev);
>  
>  	return ret;
>  }
>  EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
>  

...

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> new file mode 100644
> index 000000000000..eb72973debc5
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#include "inv_icm42607.h"
> +
> +static int inv_icm42607_i2c_bus_setup(struct inv_icm42607_state *st)
> +{
> +	unsigned int mask, val;
> +	int ret;
> +
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> +				 INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN |
> +				 INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN, 0);

regmap_clear_bits()

> +	if (ret)
> +		return ret;
> +
> +	mask = INV_ICM42607_DRIVE_CONFIG2_I2C_MASK;

Local mask variable isn't helping much.

> +	val = INV_ICM42607_DRIVE_CONFIG2_I2C(INV_ICM42607_SLEW_RATE_12_36NS);
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_DRIVE_CONFIG2,
> +				 mask, val);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> +				  INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK,
> +				  INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS);
> +}
> +
> +static int inv_icm42607_probe(struct i2c_client *client)
> +{
> +	const void *match;
> +	enum inv_icm42607_chip chip;
> +	struct regmap *regmap;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> +		return -EOPNOTSUPP;
> +
> +	match = device_get_match_data(&client->dev);

Should be i2c_get_match_data(). And we recently decided to standardize
on not checking for NULL return.

> +	if (!match)
> +		return -EINVAL;
> +	chip = (uintptr_t)match;
> +
> +	regmap = devm_regmap_init_i2c(client, &inv_icm42607_regmap_config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return inv_icm42607_core_probe(regmap, chip, inv_icm42607_i2c_bus_setup);
> +}
> +
> +static const struct i2c_device_id inv_icm42607_id[] = {
> +	{ "icm42607", INV_CHIP_ICM42607 },
> +	{ "icm42607p", INV_CHIP_ICM42607P },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, inv_icm42607_id);
> +
> +static const struct of_device_id inv_icm42607_of_matches[] = {
> +	{
> +		.compatible = "invensense,icm42607",
> +		.data = (void *)INV_CHIP_ICM42607,
> +	}, {
> +		.compatible = "invensense,icm42607p",
> +		.data = (void *)INV_CHIP_ICM42607P,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm42607_of_matches);
> +
> +static struct i2c_driver inv_icm42607_driver = {
> +	.driver = {
> +		.name = "inv-icm42607-i2c",
> +		.of_match_table = inv_icm42607_of_matches,
> +		.pm = pm_ptr(&inv_icm42607_pm_ops),
> +	},
> +	.id_table = inv_icm42607_id,
> +	.probe = inv_icm42607_probe,
> +};
> +module_i2c_driver(inv_icm42607_driver);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense ICM-42607x I2C driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_ICM42607");
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> new file mode 100644
> index 000000000000..51ce3deeb706
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/spi/spi.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#include "inv_icm42607.h"
> +
> +static int inv_icm42607_spi_bus_setup(struct inv_icm42607_state *st)
> +{
> +	unsigned int mask, val;
> +	int ret;
> +
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_DEVICE_CONFIG,
> +				 INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE,
> +				 INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE);

regmap_set_bits()

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> +				 INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN |
> +				 INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN, 0);

regmap_clear_bits()

> +	if (ret)
> +		return ret;
> +
> +	mask = INV_ICM42607_DRIVE_CONFIG3_SPI_MASK;
> +	val = INV_ICM42607_DRIVE_CONFIG3_SPI(INV_ICM42607_SLEW_RATE_INF_2NS);
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_DRIVE_CONFIG3,
> +				 mask, val);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> +				  INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK,
> +				  INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS);
> +}
> +
> +static int inv_icm42607_probe(struct spi_device *spi)
> +{
> +	const void *match;
> +	enum inv_icm42607_chip chip;
> +	struct regmap *regmap;
> +
> +	match = device_get_match_data(&spi->dev);
> +	if (!match)
> +		return -EINVAL;

Should be spi_get_device_match_data(). And we recently decided to standardize
on not checking for NULL return.

> +	chip = (uintptr_t)match;

uintptr_t is frowned upon in the kernel. Stick with kernel_ulong_t.

> +
> +	regmap = devm_regmap_init_spi(spi, &inv_icm42607_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(&spi->dev, PTR_ERR(regmap),
> +				     "Failed to register spi regmap %ld\n",
> +				     PTR_ERR(regmap));
> +
> +	return inv_icm42607_core_probe(regmap, chip,
> +				       inv_icm42607_spi_bus_setup);
> +}
> +
> +static const struct of_device_id inv_icm42607_of_matches[] = {
> +	{
> +		.compatible = "invensense,icm42607",
> +		.data = (void *)INV_CHIP_ICM42607,
> +	},
> +	{
> +		.compatible = "invensense,icm42607p",
> +		.data = (void *)INV_CHIP_ICM42607P,
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm42607_of_matches);
> +
> +static const struct spi_device_id inv_icm42607_spi_id_table[] = {
> +	{ "icm42607", INV_CHIP_ICM42607 },
> +	{ "icm42607p", INV_CHIP_ICM42607P },
> +	{ },

No trailing comma.

> +};
> +MODULE_DEVICE_TABLE(spi, inv_icm42607_spi_id_table);
> +
> +static struct spi_driver inv_icm42607_driver = {
> +	.driver = {
> +		.name = "inv-icm42607-spi",
> +		.of_match_table = inv_icm42607_of_matches,
> +		.pm = &inv_icm42607_pm_ops,
> +	},
> +	.id_table = inv_icm42607_spi_id_table,
> +	.probe = inv_icm42607_probe,
> +};
> +module_spi_driver(inv_icm42607_driver);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense ICM-42607x SPI driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_ICM42607");


^ permalink raw reply

* Re: [GIT PULL] RISC-V SpacemiT Devicetrees for v7.1
From: Linus Walleij @ 2026-04-10 22:19 UTC (permalink / raw)
  To: Yixun Lan
  Cc: soc, Yixun Lan, Arnd Bergmann, spacemit, linux-riscv, devicetree,
	linux-kernel
In-Reply-To: <20260403123040-KYC0145825@kernel.org>

Hi Yixun,

I looked into this pull request.

I'm sorry if I do stupid mistakes in handling it, I'm new to maintaining
the SoC tree. Bear with me.

On Fri, Apr 3, 2026 at 2:32 PM Yixun Lan <dlan@kernel.org> wrote:

> Aurelien Jarno (7):
>       riscv: dts: spacemit: drop incorrect pinctrl for combo PHY
(...)
> Yixun Lan (9):
>       riscv: dts: spacemit: pcie: fix missing power regulator

[Fixes]
fatal: Not a valid object name linus/master
Commit: c68360c0d636 ("riscv: dts: spacemit: drop incorrect pinctrl
for combo PHY")
    Fixes tag: Fixes: 0be016a4b5d1b9 ("riscv: dts: spacemit: PCIe and
PHY-related updates")
    Has these problem(s):
        - Inspect: Target SHA is not ancestor of Linus' master branch,
which means it is fixing commit in your branch
fatal: Not a valid object name linus/master
Commit: 8a9071299dec ("riscv: dts: spacemit: pcie: fix missing power regulator")
    Fixes tag: Fixes: 0be016a4b5d1 ("riscv: dts: spacemit: PCIe and
PHY-related updates")
    Has these problem(s):
        - Inspect: Target SHA is not ancestor of Linus' master branch,
which means it is fixing commit in your branch

So this means you introduced bugs and fix them in the same pull request?

Why?

The practice is to squash such fixes into the offending patches when
presenting pull requests. But I went ahead anyway, trying to not be so
picky. (The commits are there, in your branch indeed.)

- Checked that it was in linux-next OK
- built DTBS OK

Pulled in, thanks.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH V3 2/9] iio: imu: inv_icm42607: Add Core for inv_icm42607 Driver
From: David Lechner @ 2026-04-10 22:06 UTC (permalink / raw)
  To: Chris Morgan, linux-iio
  Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
	devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
	Chris Morgan
In-Reply-To: <20260330195853.392877-3-macroalpha82@gmail.com>

On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add the core component of a new inv_icm42607 driver. This includes
> a few setup functions and the full register definition in the
> header file.
> 
> +#define INV_ICM42607_REG_PWR_MGMT0			0x1F
> +#define INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL		BIT(7)
> +#define INV_ICM42607_PWR_MGMT0_IDLE			BIT(4)
> +#define INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK		GENMASK(3, 2)
> +#define INV_ICM42607_PWR_MGMT0_GYRO(_mode)		\
> +	FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, (_mode))

We usually try to avoid macros that hide FIELD_PREP(). IMHO, it makes
the code harder to read because you have to jump back and forth to
definitions to see if it really is that.

> +#define INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK		GENMASK(1, 0)
> +#define INV_ICM42607_PWR_MGMT0_ACCEL(_mode)		\
> +	FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, (_mode))
> +

...


> +u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr)
> +{
> +	static u32 odr_periods[INV_ICM42607_ODR_NB] = {
> +		/* Reserved values */
> +		0, 0, 0, 0, 0,
> +		/* 1600Hz */
> +		625000,
> +		/* 800Hz */
> +		1250000,
> +		/* 400Hz */
> +		2500000,
> +		/* 200Hz */
> +		5000000,
> +		/* 100 Hz */
> +		10000000,
> +		/* 50Hz */
> +		20000000,
> +		/* 25Hz */
> +		40000000,
> +		/* 12.5Hz */
> +		80000000,
> +		/* 6.25Hz */
> +		160000000,
> +		/* 3.125Hz */
> +		320000000,
> +		/* 1.5625Hz */
> +		640000000,
> +	};

No range checking to avoid out-of-bouds access?

> +
> +	return odr_periods[odr];
> +}
> +
> +int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> +			     unsigned int writeval, unsigned int *readval)
> +{
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +
> +	guard(mutex)(&st->lock);
> +
> +	if (readval)
> +		return regmap_read(st->map, reg, readval);
> +
> +	return regmap_write(st->map, reg, writeval);
> +}
> +
> +static int inv_icm42607_set_conf(struct inv_icm42607_state *st,
> +				 const struct inv_icm42607_conf *conf)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	val = INV_ICM42607_PWR_MGMT0_GYRO(conf->gyro.mode) |
> +	INV_ICM42607_PWR_MGMT0_ACCEL(conf->accel.mode);

Indent wrapped lines one tab or line up with INV_ after `= `.

> +	/*
> +	 * No temperature enable reg in datasheet, but BSP driver
> +	 * selected RC oscillator clock in LP mode when temperature
> +	 * was disabled.
> +	 */
> +	if (!conf->temp_en)
> +		val |= INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL;
> +	ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> +	if (ret)
> +		return ret;
> +
> +	val = INV_ICM42607_GYRO_CONFIG0_FS_SEL(conf->gyro.fs) |
> +	INV_ICM42607_GYRO_CONFIG0_ODR(conf->gyro.odr);
> +	ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> +	if (ret)
> +		return ret;
> +
> +	val = INV_ICM42607_ACCEL_CONFIG0_FS_SEL(conf->accel.fs) |
> +	INV_ICM42607_ACCEL_CONFIG0_ODR(conf->accel.odr);
> +	ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG0, val);
> +	if (ret)
> +		return ret;
> +
> +	val = INV_ICM42607_GYRO_CONFIG1_FILTER(conf->gyro.filter);
> +	ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG1, val);
> +	if (ret)
> +		return ret;
> +
> +	val = INV_ICM42607_ACCEL_CONFIG1_FILTER(conf->accel.filter);
> +	ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG1, val);
> +	if (ret)
> +		return ret;
> +
> +	st->conf = *conf;
> +
> +	return 0;
> +}
> +
> +/**
> + *  inv_icm42607_setup() - check and setup chip
> + *  @st:	driver internal state
> + *  @bus_setup:	callback for setting up bus specific registers
> + *
> + *  Returns 0 on success, a negative error code otherwise.
> + */
> +static int inv_icm42607_setup(struct inv_icm42607_state *st,
> +			      inv_icm42607_bus_setup bus_setup)
> +{
> +	const struct inv_icm42607_hw *hw = &inv_icm42607_hw[st->chip];
> +	const struct device *dev = regmap_get_device(st->map);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(st->map, INV_ICM42607_REG_WHOAMI, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val != hw->whoami)
> +		dev_warn_probe(dev, -ENODEV,
> +			       "invalid whoami %#02x expected %#02x (%s)\n",
> +			       val, hw->whoami, hw->name);
> +
> +	st->name = hw->name;
> +
> +	ret = regmap_write(st->map, INV_ICM42607_REG_SIGNAL_PATH_RESET,
> +			   INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET);
> +	if (ret)
> +		return ret;

nit: Add blank line here.

> +	msleep(INV_ICM42607_RESET_TIME_MS);
> +
> +	ret = regmap_read(st->map, INV_ICM42607_REG_INT_STATUS, &val);
> +	if (ret)
> +		return ret;
> +	if (!(val & INV_ICM42607_INT_STATUS_RESET_DONE))
> +		return dev_err_probe(dev, -ENODEV,
> +				     "reset error, reset done bit not set\n");

Could also replace this and msleep with regmap_read_poll_timeout().

> +
> +	ret = bus_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> +				 INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN,
> +				 INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN);

Simplify with regmap_set_bits().

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> +				 INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK,
> +				 INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL);
> +	if (ret)
> +		return ret;
> +
> +	return inv_icm42607_set_conf(st, hw->conf);
> +}
> +
> +static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
> +{
> +	int ret;
> +
> +	ret = regulator_enable(st->vddio_supply);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(3000, 4000);

Use fsleep() and add a comment to explain why the duration
was chosen.

> +
> +	return 0;
> +}
> +
> +static void inv_icm42607_disable_vddio_reg(void *_data)
> +{
> +	struct inv_icm42607_state *st = _data;
> +
> +	regulator_disable(st->vddio_supply);
> +}
> +
> +int inv_icm42607_core_probe(struct regmap *regmap, int chip,
> +			    inv_icm42607_bus_setup bus_setup)
> +{
> +	struct device *dev = regmap_get_device(regmap);
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
> +	struct inv_icm42607_state *st;
> +	int irq, irq_type;
> +	bool open_drain;
> +	int ret;
> +
> +	if (chip < INV_CHIP_INVALID || chip >= INV_CHIP_NB)

Only two chips are defined in regmap_read_poll_timeout, so this range
checking seems wrong.

> +		dev_warn_probe(dev, -ENODEV,
> +			       "Invalid chip = %d\n", chip);
> +
> +	/* get INT1 only supported interrupt or fallback to first interrupt */
> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> +	if (irq < 0 && irq != -EPROBE_DEFER) {
> +		dev_info(dev, "no INT1 interrupt defined, fallback to first interrupt\n");
> +		irq = fwnode_irq_get(fwnode, 0);
> +	}
> +	if (irq < 0)
> +		return dev_err_probe(dev, irq, "error missing INT1 interrupt\n");
> +
> +	irq_type = irq_get_trigger_type(irq);
> +	if (!irq_type)
> +		irq_type = IRQF_TRIGGER_FALLING;
> +
> +	open_drain = device_property_read_bool(dev, "drive-open-drain");
> +
> +	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> +	if (!st)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, st);
> +	mutex_init(&st->lock);
> +	st->chip = chip;
> +	st->map = regmap;
> +	st->irq = irq;
> +
> +	ret = iio_read_mount_matrix(dev, &st->orientation);
> +	if (ret) {
> +		dev_err(dev, "failed to retrieve mounting matrix %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to get vdd regulator\n");
> +
> +	msleep(INV_ICM42607_POWER_UP_TIME_MS);
> +
> +	st->vddio_supply = devm_regulator_get(dev, "vddio");

If we aren't implementing power managament, we can just use
devm_regulator_get_enabled() and avoid the devm_add_action_or_reset().

> +	if (IS_ERR(st->vddio_supply))
> +		return PTR_ERR(st->vddio_supply);
> +
> +	ret = inv_icm42607_enable_vddio_reg(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, inv_icm42607_disable_vddio_reg, st);
> +	if (ret)
> +		return ret;
> +
> +	/* Setup chip registers (includes WHOAMI check, reset check, bus setup) */

This comment would be better as part of the function doc comment.

> +	ret = inv_icm42607_setup(st, bus_setup);
> +
> +	return ret;
> +}

^ permalink raw reply

* Re: [PATCH v2] dt-binding: leds: publish common bindings under dual license
From: Gergo Koteles @ 2026-04-10 21:35 UTC (permalink / raw)
  To: Corvin Köhne, linux-kernel
  Cc: open list:LED SUBSYSTEM, Pavel Machek,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Corvin Köhne, Ashley Towns, Dan Murphy, INAGAKI Hiroshi,
	Jacek Anaszewski, Olliver Schinagl, Pavel Machek,
	Rafał Miłecki, Roderick Colenbrander,
	Krzysztof Kozlowski
In-Reply-To: <20260408062942.7128-1-corvin.koehne@gmail.com>

On Wed, 2026-04-08 at 08:29 +0200, Corvin Köhne wrote:
> From: Corvin Köhne <c.koehne@beckhoff.com>
> 
> Changes leds/common.h DT binding header file to be published under GPLv2
> or BSD-2-Clause license terms. This change allows this common LED
> bindings header file to be used in software components as bootloaders
> and OSes that are not published under GPLv2 terms.
> 
> All contributors to leds/common.h file in copy.
> 
> Cc: Ashley Towns <mail@ashleytowns.id.au>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Gergo Koteles <soyer@irl.hu>
> Cc: INAGAKI Hiroshi <musashino.open@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Olliver Schinagl <oliver@schinagl.nl>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> ---
>  include/dt-bindings/leds/common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 4f017bea0123..b7bafbaf7df3 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
>  /*
>   * This header provides macros for the common LEDs device tree bindings.
>   *

I don't know if a single line change counts, but it's fine with me.

Acked-by: Gergo Koteles <soyer@irl.hu>

^ permalink raw reply

* Re: [PATCH v7 6/6] docs: iio: adc: ad4691: add driver documentation
From: David Lechner @ 2026-04-10 21:38 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel, Jonathan Corbet, Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-6-be375d4df2c5@analog.com>

On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add RST documentation for the AD4691 family ADC driver covering
> supported devices, IIO channels, operating modes, oversampling,
> reference voltage, LDO supply, reset, GP pins, SPI offload support,
> and buffer data format.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  Documentation/iio/ad4691.rst | 283 +++++++++++++++++++++++++++++++++++++++++++
>  Documentation/iio/index.rst  |   1 +
>  MAINTAINERS                  |   1 +
>  3 files changed, 285 insertions(+)
> 
> diff --git a/Documentation/iio/ad4691.rst b/Documentation/iio/ad4691.rst
> new file mode 100644
> index 000000000000..a1012c8b78a3
> --- /dev/null
> +++ b/Documentation/iio/ad4691.rst
> @@ -0,0 +1,283 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=============
> +AD4691 driver
> +=============


One overall comment. This goes into driver implementation details quite a bit.
I think that is really better done as comments in the driver itself. And this
document should just focus on how to use the driver from the userspace point
of view.


> +Buffer data format
> +==================
> +
> +The IIO buffer data format (``in_voltageN_type``) is the same across all
> +paths: 16-bit unsigned big-endian samples with no shift.
> +
> ++-------------------------+-------------+----------+-------+
> +| Path                    | storagebits | realbits | shift |
> ++=========================+=============+==========+=======+
> +| Triggered buffer        | 16          | 16       | 0     |
> ++-------------------------+-------------+----------+-------+
> +| CNV Burst offload (DMA) | 16          | 16       | 0     |
> ++-------------------------+-------------+----------+-------+
> +| Manual offload (DMA)    | 16          | 16       | 0     |
> ++-------------------------+-------------+----------+-------+

Not sure this table is helpful since all values are the same everywhere.

Also, doesn't SPI offload have storagebits == 32?

> +
> +In the triggered-buffer path the SPI rx_buf for each transfer points directly
> +into the scan buffer, so the 16-bit big-endian result is written in place with
> +no additional copying.
> +


^ permalink raw reply

* Re: [PATCH v4 2/2] hwmon:(pmbus/xdp720) Add support for efuse xdp720
From: Guenter Roeck @ 2026-04-10 21:20 UTC (permalink / raw)
  To: ASHISH YADAV
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-hwmon,
	devicetree, linux-kernel, Ashish Yadav
In-Reply-To: <20260410070154.3313-3-Ashish.Yadav@infineon.com>

On Fri, Apr 10, 2026 at 12:31:54PM +0530, ASHISH YADAV wrote:
> From: Ashish Yadav <ashish.yadav@infineon.com>
> 
> Add the pmbus driver for Infineon XDP720 Digital eFuse Controller.
> 
> Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>

Applied.

Thanks,
Guenter

> ---
> XDP720 Digital eFuse Controller provides accurate system telemetry
> (V, I, P, T) and reports analog current at the IMON pin for post-processing.
> 
> The Current and Power measurement depends on the RIMON and GIMON values.
> Please look into data sheet sections 5.4.2 and 5.4.4 for more details:
> https://www.infineon.com/assets/row/public/documents/24/49/infineon-xdp720-001-datasheet-en.pdf
> 
> The GIMON (microA/A) depends on the 10th bit of TELEMETRY_AVG PMBUS Register.
> The value of RIMON (kohm) can be provided by the user through device tree using
> infineon,rimon-micro-ohms  property.
> ---
>  drivers/hwmon/pmbus/Kconfig  |   9 +++
>  drivers/hwmon/pmbus/Makefile |   1 +
>  drivers/hwmon/pmbus/xdp720.c | 128 +++++++++++++++++++++++++++++++++++
>  3 files changed, 138 insertions(+)
>  create mode 100644 drivers/hwmon/pmbus/xdp720.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index fc1273abe357..c419e3ecce90 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -702,6 +702,15 @@ config SENSORS_XDP710
>  	  This driver can also be built as a module. If so, the module will
>  	  be called xdp710.
>  
> +config SENSORS_XDP720
> +	tristate "Infineon XDP720 family"
> +	help
> +	  If you say yes here you get hardware monitoring support for Infineon
> +	  XDP720.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called xdp720.
> +
>  config SENSORS_XDPE152
>  	tristate "Infineon XDPE152 family"
>  	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index d6c86924f887..1cac7ccae79f 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_SENSORS_TPS546D24)	+= tps546d24.o
>  obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
>  obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
>  obj-$(CONFIG_SENSORS_XDP710)	+= xdp710.o
> +obj-$(CONFIG_SENSORS_XDP720)	+= xdp720.o
>  obj-$(CONFIG_SENSORS_XDPE122)	+= xdpe12284.o
>  obj-$(CONFIG_SENSORS_XDPE152)	+= xdpe152c4.o
>  obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
> diff --git a/drivers/hwmon/pmbus/xdp720.c b/drivers/hwmon/pmbus/xdp720.c
> new file mode 100644
> index 000000000000..8729a771f216
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/xdp720.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon XDP720 Digital eFuse Controller
> + *
> + * Copyright (c) 2026 Infineon Technologies. All rights reserved.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/of_device.h>
> +#include <linux/bitops.h>
> +#include <linux/math64.h>
> +#include "pmbus.h"
> +
> +/*
> + * The IMON resistor required to generate the system overcurrent protection.
> + * Arbitrary default Rimon value: 2k Ohm
> + */
> +#define XDP720_DEFAULT_RIMON 2000000000 /* 2k ohm */
> +#define XDP720_TELEMETRY_AVG 0xE9
> +
> +static struct pmbus_driver_info xdp720_info = {
> +	.pages = 1,
> +	.format[PSC_VOLTAGE_IN] = direct,
> +	.format[PSC_VOLTAGE_OUT] = direct,
> +	.format[PSC_CURRENT_OUT] = direct,
> +	.format[PSC_POWER] = direct,
> +	.format[PSC_TEMPERATURE] = direct,
> +
> +	.m[PSC_VOLTAGE_IN] = 4653,
> +	.b[PSC_VOLTAGE_IN] = 0,
> +	.R[PSC_VOLTAGE_IN] = -2,
> +	.m[PSC_VOLTAGE_OUT] = 4653,
> +	.b[PSC_VOLTAGE_OUT] = 0,
> +	.R[PSC_VOLTAGE_OUT] = -2,
> +	/*
> +	 * Current and Power measurement depends on the RIMON (kOhm) and
> +	 * GIMON(microA/A) values.
> +	 */
> +	.m[PSC_CURRENT_OUT] = 24668,
> +	.b[PSC_CURRENT_OUT] = 0,
> +	.R[PSC_CURRENT_OUT] = -4,
> +	.m[PSC_POWER] = 4486,
> +	.b[PSC_POWER] = 0,
> +	.R[PSC_POWER] = -1,
> +	.m[PSC_TEMPERATURE] = 54,
> +	.b[PSC_TEMPERATURE] = 22521,
> +	.R[PSC_TEMPERATURE] = -1,
> +
> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_PIN |
> +		   PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_INPUT |
> +		   PMBUS_HAVE_STATUS_TEMP,
> +};
> +
> +static int xdp720_probe(struct i2c_client *client)
> +{
> +	struct pmbus_driver_info *info;
> +	int ret;
> +	u32 rimon;
> +	int gimon;
> +
> +	info = devm_kmemdup(&client->dev, &xdp720_info, sizeof(*info),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	ret = devm_regulator_get_enable(&client->dev, "vdd-vin");
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +			"failed to enable vdd-vin supply\n");
> +
> +	ret = i2c_smbus_read_word_data(client, XDP720_TELEMETRY_AVG);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Can't get TELEMETRY_AVG\n");
> +		return ret;
> +	}
> +
> +	ret >>= 10; /* 10th bit of TELEMETRY_AVG REG for GIMON Value */
> +	ret &= GENMASK(0, 0);
> +	if (ret == 1)
> +		gimon = 18200; /* output gain 18.2 microA/A */
> +	else
> +		gimon = 9100; /* output gain 9.1 microA/A */
> +
> +	if (of_property_read_u32(client->dev.of_node,
> +				 "infineon,rimon-micro-ohms", &rimon))
> +		rimon = XDP720_DEFAULT_RIMON; /* Default if not set via DT */
> +	if (rimon == 0)
> +		return -EINVAL;
> +
> +	/* Adapt the current and power scale for each instance */
> +	info->m[PSC_CURRENT_OUT] = DIV64_U64_ROUND_CLOSEST((u64)
> +		info->m[PSC_CURRENT_OUT] * rimon * gimon, 1000000000000ULL);
> +	info->m[PSC_POWER] = DIV64_U64_ROUND_CLOSEST((u64)
> +		info->m[PSC_POWER] * rimon * gimon, 1000000000000000ULL);
> +
> +	return pmbus_do_probe(client, info);
> +}
> +
> +static const struct of_device_id xdp720_of_match[] = {
> +	{ .compatible = "infineon,xdp720" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, xdp720_of_match);
> +
> +static const struct i2c_device_id xdp720_id[] = {
> +	{ "xdp720" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, xdp720_id);
> +
> +static struct i2c_driver xdp720_driver = {
> +	.driver = {
> +		   .name = "xdp720",
> +		   .of_match_table = xdp720_of_match,
> +	},
> +	.probe = xdp720_probe,
> +	.id_table = xdp720_id,
> +};
> +
> +module_i2c_driver(xdp720_driver);
> +
> +MODULE_AUTHOR("Ashish Yadav <ashish.yadav@infineon.com>");
> +MODULE_DESCRIPTION("PMBus driver for Infineon XDP720 Digital eFuse Controller");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");

^ permalink raw reply

* Re: [PATCH v4 1/2] dt-bindings: hwmon/pmbus: Add Infineon XDP720
From: Guenter Roeck @ 2026-04-10 21:19 UTC (permalink / raw)
  To: ASHISH YADAV
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-hwmon,
	devicetree, linux-kernel, Ashish Yadav
In-Reply-To: <20260410070154.3313-2-Ashish.Yadav@infineon.com>

On Fri, Apr 10, 2026 at 12:31:53PM +0530, ASHISH YADAV wrote:
> From: Ashish Yadav <ashish.yadav@infineon.com>
> 
> Add documentation for the device tree binding of the XDP720 eFuse.
> 
> Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Applied.

Thanks,
Guenter

^ permalink raw reply

* Re: [PATCH v7 5/6] iio: adc: ad4691: add oversampling support
From: David Lechner @ 2026-04-10 21:15 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel, Jonathan Corbet, Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-5-be375d4df2c5@analog.com>

On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add per-channel oversampling ratio (OSR) support for CNV burst mode.
> The accumulator depth register (ACC_DEPTH_IN) is programmed with the
> selected OSR at buffer enable time and before each single-shot read.
> 
> Supported OSR values: 1, 2, 4, 8, 16, 32.
> 
> Introduce AD4691_MANUAL_CHANNEL() for manual mode channels, which do
> not expose the oversampling ratio attribute since OSR is not applicable
> in that mode. A separate manual_channels array is added to
> struct ad4691_channel_info and selected at probe time; offload paths
> reuse the same arrays with num_channels capping access before the soft
> timestamp entry.
> 
> The reported sampling frequency accounts for the active OSR:
> effective_freq = oscillator_freq / osr

Technically, the way this is implemented is fine according to IIO ABI
rules. Writing any attribute can cause others to change. It does
introduce a potential pitfall though. Currently, changing the OSR will
change the sampling frequency, so you have to always write oversampling_ratio
first, then write sampling_frequency to get what you asked for. If you want
to change the OSR and keep the same sample rate, you still have to write both
attributes again.

In other drivers, I've implemented it so that the requested sampling frequency
is stored any you always get the closest sampling frequency available based on
the oversampling ratio. This way, it doesn't matter which order you write
the attributes. In that case, the actual periodic trigger source isn't set up
until we actually start sampling.

> 
> OSR defaults to 1 (no accumulation) for all channels.
> 

...

> @@ -499,7 +570,7 @@ static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
>  	if (ret)
>  		return ret;
>  
> -	*val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
> +	*val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)] / osr;

I guess we don't have to worry about fractional values here?

>  	return IIO_VAL_INT;
>  }
>  
> @@ -536,6 +607,11 @@ static int ad4691_read_avail(struct iio_dev *indio_dev,
>  		*type = IIO_VAL_INT;
>  		*length = ARRAY_SIZE(ad4691_osc_freqs_Hz) - start;
>  		return IIO_AVAIL_LIST;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*vals = ad4691_oversampling_ratios;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(ad4691_oversampling_ratios);
> +		return IIO_AVAIL_LIST;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -566,6 +642,11 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
>  	if (ret)
>  		return ret;
>  
> +	ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(chan->channel),
> +			   st->osr[chan->channel]);
> +	if (ret)
> +		return ret;
> +
>  	ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, &reg_val);
>  	if (ret)
>  		return ret;
> @@ -575,8 +656,9 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
>  		return ret;
>  
>  	osc_idx = FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val);
> -	/* Wait 2 oscillator periods for the conversion to complete. */
> -	period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC, ad4691_osc_freqs_Hz[osc_idx]);
> +	/* Wait osr oscillator periods for all accumulator samples to complete. */

Why did we need to way 2 before and only 1 now when OSR == 1?

> +	period_us = DIV_ROUND_UP((unsigned long)st->osr[chan->channel] * USEC_PER_SEC,
> +				 ad4691_osc_freqs_Hz[osc_idx]);

^ permalink raw reply

* Re: [PATCH v7 4/6] iio: adc: ad4691: add SPI offload support
From: David Lechner @ 2026-04-10 21:00 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel, Jonathan Corbet, Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-4-be375d4df2c5@analog.com>

On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add SPI offload support to enable DMA-based, CPU-independent data
> acquisition using the SPI Engine offload framework.
> 
> When an SPI offload is available (devm_spi_offload_get() succeeds),
> the driver registers a DMA engine IIO buffer and uses dedicated buffer
> setup operations. If no offload is available the existing software
> triggered buffer path is used unchanged.
> 
> Both CNV Burst Mode and Manual Mode support offload, but use different
> trigger mechanisms:
> 
> CNV Burst Mode: the SPI Engine is triggered by the ADC's DATA_READY
> signal on the GP pin specified by the trigger-source consumer reference
> in the device tree (one cell = GP pin number 0-3). For this mode the
> driver acts as both an SPI offload consumer (DMA RX stream, message
> optimization) and a trigger source provider: it registers the
> GP/DATA_READY output via devm_spi_offload_trigger_register() so the
> offload framework can match the '#trigger-source-cells' phandle and
> automatically fire the SPI Engine DMA transfer at end-of-conversion.
> 
> Manual Mode: the SPI Engine is triggered by a periodic trigger at
> the configured sampling frequency. The pre-built SPI message uses
> the pipelined CNV-on-CS protocol: N+1 16-bit transfers are issued
> for N active channels (the first result is discarded as garbage from
> the pipeline flush) and the remaining N results are captured by DMA.
> 
> All offload transfers use 16-bit frames (bits_per_word=16, len=2).
> The channel scan_type (storagebits=16, shift=0, IIO_BE) is shared
> between the software triggered-buffer and offload paths; no separate
> scan_type or channel array is needed for the offload case. The
> ad4691_manual_channels[] array introduced in the triggered-buffer
> commit is reused here: it hides the IIO_CHAN_INFO_OVERSAMPLING_RATIO
> attribute, which is not applicable in Manual Mode.
> 
> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  drivers/iio/adc/Kconfig  |   2 +
>  drivers/iio/adc/ad4691.c | 398 ++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 395 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index d498f16c0816..fdc6565933c5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -143,8 +143,10 @@ config AD4691
>  	tristate "Analog Devices AD4691 Family ADC Driver"
>  	depends on SPI
>  	select IIO_BUFFER
> +	select IIO_BUFFER_DMAENGINE
>  	select IIO_TRIGGERED_BUFFER
>  	select REGMAP
> +	select SPI_OFFLOAD
>  	help
>  	  Say yes here to build support for Analog Devices AD4691 Family MuxSAR
>  	  SPI analog to digital converters (ADC).
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 3e5caa0972eb..839ea7f44c78 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
> @@ -22,6 +22,8 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/spi/spi.h>
> +#include <linux/spi/offload/consumer.h>
> +#include <linux/spi/offload/provider.h>
>  #include <linux/units.h>
>  #include <linux/unaligned.h>
>  
> @@ -43,6 +45,11 @@
>  
>  #define AD4691_CNV_DUTY_CYCLE_NS		380
>  #define AD4691_CNV_HIGH_TIME_NS			430
> +/*
> + * Conservative default for the manual offload periodic trigger. Low enough
> + * to work safely out of the box across all OSR and channel count combinations.
> + */
> +#define AD4691_OFFLOAD_INITIAL_TRIGGER_HZ	(100 * HZ_PER_KHZ)
>  
>  #define AD4691_SPI_CONFIG_A_REG			0x000
>  #define AD4691_SW_RESET				(BIT(7) | BIT(0))
> @@ -95,6 +102,8 @@
>  #define AD4691_ACC_IN(n)			(0x252 + (3 * (n)))
>  #define AD4691_ACC_STS_DATA(n)			(0x283 + (4 * (n)))
>  
> +#define AD4691_OFFLOAD_BITS_PER_WORD		16

This is just the same as realbits in scan info. So could use that
directly instead.

> +
>  static const char * const ad4691_supplies[] = { "avdd", "vio" };
>  
>  enum ad4691_ref_ctrl {
> @@ -114,6 +123,7 @@ struct ad4691_chip_info {
>  	const char *name;
>  	unsigned int max_rate;
>  	const struct ad4691_channel_info *sw_info;
> +	const struct ad4691_channel_info *offload_info;
>  };
>  
>  #define AD4691_CHANNEL(ch)						\
> @@ -177,6 +187,18 @@ static const struct ad4691_channel_info ad4693_sw_info = {
>  	.num_channels = ARRAY_SIZE(ad4693_channels),
>  };
>  
> +static const struct ad4691_channel_info ad4691_offload_info = {
> +	.channels = ad4691_channels,
> +	/* No soft timestamp; num_channels caps access to 16. */
> +	.num_channels = 16,

`ARRAY_SIZE(ad4691_channels) - 1` would make sense too.

> +};
> +
> +static const struct ad4691_channel_info ad4693_offload_info = {
> +	.channels = ad4693_channels,
> +	/* No soft timestamp; num_channels caps access to 8. */
> +	.num_channels = 8,
> +};
> +
>  /*
>   * Internal oscillator frequency table. Index is the OSC_FREQ_REG[3:0] value.
>   * Index 0 (1 MHz) is only valid for AD4692/AD4694; AD4691/AD4693 support
> @@ -207,24 +229,36 @@ static const struct ad4691_chip_info ad4691_chip_info = {
>  	.name = "ad4691",
>  	.max_rate = 500 * HZ_PER_KHZ,
>  	.sw_info = &ad4691_sw_info,
> +	.offload_info = &ad4691_offload_info,
>  };
>  
>  static const struct ad4691_chip_info ad4692_chip_info = {
>  	.name = "ad4692",
>  	.max_rate = 1 * HZ_PER_MHZ,
>  	.sw_info = &ad4691_sw_info,
> +	.offload_info = &ad4691_offload_info,
>  };
>  
>  static const struct ad4691_chip_info ad4693_chip_info = {
>  	.name = "ad4693",
>  	.max_rate = 500 * HZ_PER_KHZ,
>  	.sw_info = &ad4693_sw_info,
> +	.offload_info = &ad4693_offload_info,
>  };
>  
>  static const struct ad4691_chip_info ad4694_chip_info = {
>  	.name = "ad4694",
>  	.max_rate = 1 * HZ_PER_MHZ,
>  	.sw_info = &ad4693_sw_info,
> +	.offload_info = &ad4693_offload_info,
> +};
> +
> +struct ad4691_offload_state {
> +	struct spi_offload *spi;

I would call this "offload" or "instance". "spi" is usally the SPI
device handle.

> +	struct spi_offload_trigger *trigger;
> +	u64 trigger_hz;
> +	u8 tx_cmd[17][2];
> +	u8 tx_reset[4];
>  };
>  

...

> +
> +static int ad4691_cnv_burst_offload_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	struct ad4691_offload_state *offload = st->offload;
> +	int ret;
> +
> +	spi_offload_trigger_disable(offload->spi, offload->trigger);
> +
> +	ret = ad4691_sampling_enable(st, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> +			   AD4691_SEQ_ALL_CHANNELS_OFF);

Why this extra step? We don't have it when unwinding in the
error path of the postenable function.

> +	if (ret)
> +		return ret;
> +
> +	spi_unoptimize_message(&st->scan_msg);
> +
> +	return ad4691_exit_conversion_mode(st);
> +}
> +
> +static const struct iio_buffer_setup_ops ad4691_cnv_burst_offload_buffer_setup_ops = {
> +	.postenable = &ad4691_cnv_burst_offload_buffer_postenable,
> +	.predisable = &ad4691_cnv_burst_offload_buffer_predisable,
> +};
> +
>  static ssize_t sampling_frequency_show(struct device *dev,
>  				       struct device_attribute *attr,
>  				       char *buf)

^ permalink raw reply

* Re: [PATCH net-next v3 00/12] net: airoha: Support multiple net_devices connected to the same GDM port
From: Jakub Kicinski @ 2026-04-10 20:49 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Christian Marangi,
	Benjamin Larsson, linux-arm-kernel, linux-mediatek, netdev,
	devicetree, Xuegang Lu
In-Reply-To: <adiop-9Mo4ADfvfw@lore-desk>

On Fri, 10 Apr 2026 09:37:11 +0200 Lorenzo Bianconi wrote:
> > On Mon, 06 Apr 2026 12:34:05 +0200 Lorenzo Bianconi wrote:  
> > > EN7581 or AN7583 SoCs support connecting multiple external SerDes (e.g.
> > > Ethernet or USB SerDes) to GDM3 or GDM4 ports via a hw arbiter that
> > > manages the traffic in a TDM manner. As a result multiple net_devices can
> > > connect to the same GDM{3,4} port and there is a theoretical "1:n"
> > > relation between GDM ports and net_devices.  
> > 
> > Looks like this driver uses page pool.
> > If you're sharing the same page pool across multiple netdevs
> > it must not be linked to a netdev.  
> 
> are you referring to slow.netdev pointer? If so, this is not set in airoha_eth
> driver.

Yes. Alright, thanks for checking. Pretty sure I saw it set somewhere 
in a file called airoha* but must be another component :)

^ permalink raw reply

* Re: [PATCH v7 3/6] iio: adc: ad4691: add triggered buffer support
From: David Lechner @ 2026-04-10 20:46 UTC (permalink / raw)
  To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
	Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
	Philipp Zabel, Jonathan Corbet, Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
	linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-3-be375d4df2c5@analog.com>

On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add buffered capture support using the IIO triggered buffer framework.
> 

...

> @@ -201,8 +245,45 @@ struct ad4691_state {
>  	 * atomicity of consecutive SPI operations.
>  	 */
>  	struct mutex lock;
> +	/*
> +	 * Per-buffer-enable lifetime resources:
> +	 * Manual Mode - a pre-built SPI message that clocks out N+1
> +	 *		 transfers in one go.
> +	 * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
> +	 *		    transfers in one go.
> +	 */
> +	struct spi_message scan_msg;
> +	/* max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst). */
> +	struct spi_transfer scan_xfers[34];
> +	/*
> +	 * CNV burst: 16 AVG_IN addresses + state-reset address + state-reset
> +	 * value = 18.  Manual: 16 channel cmds + 1 NOOP = 17.
> +	 */
> +	__be16 scan_tx[18];

Needs __aligned(IIO_DMA_MINALIGN) since it is used with SPI.

> +	/* Scan buffer: one BE16 slot per channel (rx'd directly), plus timestamp */
> +	struct {
> +		__be16 vals[16];
> +		aligned_s64 ts;
> +	} scan;

Unless it is required that all channels are always enabled:

	IIO_DECLARE_BUFFER_WITH_TS(__be16, scan_rx, 16);

In any case, needs to be DMA-safe for SPI.

>  };
>  



> +static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct ad4691_state *st = iio_priv(indio_dev);
> +	unsigned int n_active;
> +	unsigned int k, i;
> +	int ret;
> +
> +	n_active = bitmap_weight(indio_dev->active_scan_mask, iio_get_masklength(indio_dev));
> +
> +	memset(st->scan_xfers, 0, (2 * n_active + 2) * sizeof(st->scan_xfers[0]));
> +	memset(st->scan_tx, 0, (n_active + 2) * sizeof(st->scan_tx[0]));

Maybe simpler to just clear the whole thing? (same with other preenable)

> +
> +	spi_message_init(&st->scan_msg);
> +

^ permalink raw reply

* Re: [PATCH v7 2/2] iio: dac: ad5706r: Add support for AD5706R DAC
From: David Lechner @ 2026-04-10 20:28 UTC (permalink / raw)
  To: Alexis Czezar Torreno, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-kernel, Andy Shevchenko
In-Reply-To: <20260410-dev_ad5706r-v7-2-af93a4caa186@analog.com>

On 4/10/26 1:48 AM, Alexis Czezar Torreno wrote:
> Add support for the Analog Devices AD5706R, a 4-channel 16-bit
> current output digital-to-analog converter with SPI interface.
> 

...

> +static int ad5706r_regmap_write(void *context, const void *data, size_t count)
> +{
> +	struct ad5706r_state *st = context;
> +	unsigned int num_bytes, val;
> +	u16 reg;
> +
> +	if (count != 4)
> +		return -EINVAL;
> +
> +	reg = get_unaligned_be16(data);
> +	num_bytes = ad5706r_reg_len(reg);
> +
> +	struct spi_transfer xfer = {
> +		.tx_buf = st->tx_buf,
> +		.len = num_bytes + 2,
> +	};
> +
> +	val = get_unaligned_be32(data);
> +	put_unaligned_be32(val, &st->tx_buf[0]);

Can't we just do memcpy() instead of swapping the byte order twice?

> +
> +	/* For single byte, copy the data to the correct position */
> +	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> +		st->tx_buf[2] = st->tx_buf[3];
> +
> +	return spi_sync_transfer(st->spi, &xfer, 1);

There isn't any special paramters in the xfer struct, so spi_write() should
work here and save a bit of code.

	return spi_write(st->spi, data, num_bytes);


> +}
> +
> +static int ad5706r_regmap_read(void *context, const void *reg_buf,
> +			       size_t reg_size, void *val_buf, size_t val_size)
> +{
> +	struct ad5706r_state *st = context;
> +	unsigned int num_bytes;
> +	u16 reg, cmd, val;
> +	int ret;
> +
> +	if (reg_size != 2 || val_size != 2)
> +		return -EINVAL;
> +
> +	reg = get_unaligned_be16(reg_buf);
> +	num_bytes = ad5706r_reg_len(reg);
> +
> +	/* Full duplex, device responds immediately after command */
> +	struct spi_transfer xfer = {
> +		.tx_buf = st->tx_buf,
> +		.rx_buf = st->rx_buf,
> +		.len = 2 + num_bytes,
> +	};
> +
> +	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> +	put_unaligned_be16(cmd, &st->tx_buf[0]);
> +	put_unaligned_be16(0, &st->tx_buf[2]);

Do we actually need to write 0s while reading? 

Usually, we would just do a spi_write_then_read for something like
this.

> +
> +	ret = spi_sync_transfer(st->spi, &xfer, 1);
> +	if (ret)
> +		return ret;
> +


> +	/* Extract value from response (skip 2-byte command echo) */
> +	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> +		val = st->rx_buf[2];
> +	else if (num_bytes == AD5706R_DOUBLE_BYTE_LEN)
> +		val = get_unaligned_be16(&st->rx_buf[2]);
> +	else
> +		return -EINVAL;
> +
> +	put_unaligned_be16(val, val_buf);

Can't this all be simplified to memcpy(val_buf, &st->rx_buf[2], num_bytes); ?

Or the whole thing simplified to:

	return spi_write_then_read(st->spi, reg_buf, 2, val_buf, num_bytes);

> +
> +	return 0;
> +}
> +

^ permalink raw reply

* Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver
From: David Lechner @ 2026-04-10 20:02 UTC (permalink / raw)
  To: Hardik Phalet, Greg Kroah-Hartman
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan,
	linux-iio, devicetree, linux-kernel, linux-staging
In-Reply-To: <20260409210639.3197576-4-hardik.phalet@pm.me>

On 4/9/26 4:07 PM, Hardik Phalet wrote:
> Add an IIO driver for the QST QMC5883P, a 3-axis anisotropic
> magneto-resistive (AMR) magnetometer with a 16-bit ADC, communicating
> over I2C. There is no existing upstream driver for this device.
> 
> The driver supports:
>  - Raw magnetic field readings on X, Y, and Z axes
>  - Four full-scale ranges (+/-2 G, +/-8 G, +/-12 G, +/-30 G)
>  - Configurable output data rate (10, 50, 100, 200 Hz)
>  - Configurable oversampling ratio (1, 2, 4, 8)
>  - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
>    attribute
>  - Optional vdd-supply regulator management
>  - Runtime PM with a 2 s autosuspend delay
>  - System suspend/resume with full chip reinitialisation and regmap
>    cache resync to handle regulator power-loss

This is a little bit much to review all in one patch. Could be nice
to split out power management to a separate patch.

Oversampling/downsampling could be split to a separate patch or two
as well.

> 
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
> bit fields are accessed via regmap_field to avoid read-modify-write
> races. The STATUS register is marked precious so regmap never reads
> it speculatively and clears the DRDY/OVFL bits unexpectedly.
> 
> 
> The probe-time init sequence is: soft reset, wait 1 ms, deassert
> reset, configure SET/RESET control, apply default ODR/OSR/DSR/RNG,
> enter normal mode. This ordering was determined empirically on
> hardware to produce reliable, non-zero axis readings.
> 
> Cleanup is fully devm-managed: devm_pm_runtime_enable() handles
> runtime PM teardown, and a devm action registered before
> devm_iio_device_register() puts the chip to sleep on removal,
> ensuring the IIO interface is unregistered before the hardware is
> suspended.
> 
> The driver is placed under drivers/staging/iio/magnetometer/ with a
> TODO file tracking the remaining work before it can graduate:
>  - Triggered buffer support (iio_triggered_buffer_setup)
>  - DRDY interrupt support
>  - Self-test implementation
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
>  drivers/staging/iio/Kconfig                 |   1 +
>  drivers/staging/iio/Makefile                |   1 +
>  drivers/staging/iio/magnetometer/Kconfig    |  20 +
>  drivers/st
> aging/iio/magnetometer/Makefile   |   7 +
>  drivers/staging/iio/magnetometer/TODO       |   5 +
>  drivers/staging/iio/magnetometer/qmc5883p.c | 830 ++++++++++++++++++++
>  6 files changed, 864 insertions(+)
>  create mode 100644 drivers/staging/iio/magnetometer/Kconfig
>  create mode 100644 drivers/staging/iio/magnetometer/Makefile
>  create mode 100644 drivers/staging/iio/magnetometer/TODO
>  create mode 100644 drivers/staging/iio/magnetometer/qmc5883p.c
> 
> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
> index a60631c1f449..d363e163d248 100644
> --- a/drivers/staging/iio/Kconfig
> +++ b/drivers/staging/iio/Kconfig
> @@ -10,5 +10,6 @@ source "drivers/staging/iio/adc/Kconfig"
>  source "drivers/staging/iio/addac/Kconfig"
>  source "drivers/staging/iio/frequency/Kconfig"
>  source "drivers/staging/iio/impedance-analyzer/Kconfig"
> +source "drivers/staging/iio/magnetometer/Kconfig"
>  
>  endmenu
> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
> index 62
> 8583535393..7dcbb75d43f0 100644
> --- a/drivers/staging/iio/Makefile
> +++ b/drivers/staging/iio/Makefile
> @@ -8,3 +8,4 @@ obj-y += adc/
>  obj-y += addac/
>  obj-y += frequency/
>  obj-y += impedance-analyzer/
> +obj-y += magnetometer/
> diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
> new file mode 100644
> index 000000000000..d631da9578a1
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Magnetometer sensors
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Magnetometer sensors"
> +
> +config QMC5883P
> +	tristate "QMC5883P 3-Axis Magnetometer"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for QMC5883P I2C-based
> +	  3-axis magnetometer chip.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called qmc5883p.
> +
> +endmenu
> diff --git a/drivers/staging/iio/magnetometer/Ma
> kefile b/drivers/staging/iio/magnetometer/Makefile
> new file mode 100644
> index 000000000000..8e650f2e3b02
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for staging industrial I/O Magnetometer sensor devices
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +obj-$(CONFIG_QMC5883P)	+= qmc5883p.o
> diff --git a/drivers/staging/iio/magnetometer/TODO b/drivers/staging/iio/magnetometer/TODO
> new file mode 100644
> index 000000000000..6a8084c0dded
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/TODO
> @@ -0,0 +1,5 @@
> +TODO
> +====
> +- Implement triggered buffer support (iio_triggered_buffer_setup)
> +- Add interrupt (DRDY) support
> +- Implement self-test (selftest regmap field is unused)
> diff --git a/drivers/staging/iio/magnetometer/qmc5883p.c b/drivers/staging/iio/magnetometer/qmc5883p.c
> new file mode 100644
> index 000000000000..d9758f1e0f4d
> --- /dev/null
> +++ b/drivers/
> staging/iio/magnetometer/qmc5883p.c
> @@ -0,0 +1,830 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * qmc5883p.c - QMC5883P magnetometer driver
> + *
> + * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/types.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/unaligned.h>
> +
> +/* Register definition */
> +#define QMC5883P_REG_CHIP_ID 0x00
> +#define QMC5883P_REG_X_LSB 0x01
> +#define QMC5883P_REG_X_MSB 0x02
> +#define QMC5883P_REG_Y_LSB 0x03
> +#define QMC5883P_REG_Y_MSB 0x04
> +#define QMC58
> 83P_REG_Z_LSB 0x05

Were you manually editing the patch?

> +#define QMC5883P_REG_Z_MSB 0x06
> +#define QMC5883P_REG_STATUS 0x09
> +#define QMC5883P_REG_CTRL_1 0x0A
> +#define QMC5883P_REG_CTRL_2 0x0B
> +
> +/* Value definition */
> +#define QMC5883P_MODE_SUSPEND 0x00
> +#define QMC5883P_MODE_NORMAL 0x01
> +#define QMC5883P_MODE_SINGLE 0x02
> +#define QMC5883P_MODE_CONTINUOUS 0x03
> +
> +/* Output data rate */
> +#define QMC5883P_ODR_10 0x00
> +#define QMC5883P_ODR_50 0x01
> +#define QMC5883P_ODR_100 0x02
> +#define QMC5883P_ODR_200 0x03
> +
> +/* Oversampling rate */
> +#define QMC5883P_OSR_8 0x00
> +#define QMC5883P_OSR_4 0x01
> +#define QMC5883P_OSR_2 0x02
> +#define QMC5883P_OSR_1 0x03
> +
> +/* Downsampling rate */
> +#define QMC5883P_DSR_1 0x00
> +#define QMC5883P_DSR_2 0x01
> +#define QMC5883P_DSR_4 0x02
> +#define QMC5883P_DSR_8 0x03
> +
> +#define QMC5883P_RSTCTRL_SET_RESET \
> +	0x00 /* Set and reset on, i.e. the offset of device is renewed */
> +#define QMC5883P_RSTCTRL_SET_ONLY 0x01 /* Set only on */
> +#define QMC5883P_RSTCTRL_OFF 0x02 /* Set 
> and reset off */

Or maybe you mail client mangled the patch? These wraps are
happening in many places.

> +
> +#define QMC5883P_RNG_30G 0x00
> +#define QMC5883P_RNG_12G 0x01
> +#define QMC5883P_RNG_08G 0x02
> +#define QMC5883P_RNG_02G 0x03
> +
> +#define QMC5883P_DEFAULT_ODR QMC5883P_ODR_100
> +#define QMC5883P_DEFAULT_OSR QMC5883P_OSR_4
> +#define QMC5883P_DEFAULT_DSR QMC5883P_DSR_4
> +#define QMC5883P_DEFAULT_RNG QMC5883P_RNG_08G
> +
> +#define QMC5883P_DRDY_POLL_US 1000
> +
> +#define QMC5883P_CHIP_ID 0x80
> +
> +#define QMC5883P_STATUS_DRDY BIT(0)
> +#define QMC5883P_STATUS_OVFL BIT(1)
> +
> +/*
> + * Scale factors in T/LSB for IIO_VAL_FRACTIONAL (val/val2), derived from
> + * datasheet Table 2 sensitivities (LSB/G) converted to LSB/T (1 G = 1e-4 T):
> + *   sensitivity_T = sensitivity_G * 10000
> + *   scale = 1 / sensitivity_T
> + *
> + * Index matches register value: RNG<1:0> = 0b00..0b11
> + */
> +static const int qmc5883p_scale[][2] = {
> +	[QMC5883P_RNG_30G] = { 1, 10000000 },
> +	[QMC5883P_RNG_12G] = { 1, 25000000 },
> +	[QMC5883P_RNG_08G] = { 1, 37500000 },
> +	[QMC5883P_RNG_02G] = { 1, 15
> 0000000 },
> +};
> +
> +static const int qmc5883p_odr[] = {
> +	[QMC5883P_ODR_10] = 10,
> +	[QMC5883P_ODR_50] = 50,
> +	[QMC5883P_ODR_100] = 100,
> +	[QMC5883P_ODR_200] = 200,
> +};
> +
> +static const int qmc5883p_osr[] = {
> +	[QMC5883P_OSR_1] = 1,
> +	[QMC5883P_OSR_2] = 2,
> +	[QMC5883P_OSR_4] = 4,
> +	[QMC5883P_OSR_8] = 8,
> +};
> +
> +static const unsigned int qmc5883p_dsr[] = {
> +	[QMC5883P_DSR_1] = 1,
> +	[QMC5883P_DSR_2] = 2,
> +	[QMC5883P_DSR_4] = 4,
> +	[QMC5883P_DSR_8] = 8,
> +};
> +
> +struct qmc5883p_rf {
> +	struct regmap_field *osr;
> +	struct regmap_field *dsr;
> +	struct regmap_field *odr;
> +	struct regmap_field *mode;
> +	struct regmap_field *rng;
> +	struct regmap_field *rstctrl;
> +	struct regmap_field *sftrst;
> +	struct regmap_field *selftest;
> +	struct regmap_field *chip_id;
> +};
> +
> +static const struct regmap_range qmc5883p_readable_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_CHIP_ID, QMC5883P_REG_STATUS),
> +	regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
> +};
> +
> +static con
> st struct regmap_range qmc5883p_writable_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
> +};
> +
> +/*
> + * Volatile registers: hardware updates these independently of the driver.
> + * regmap will never serve these from cache.
> + */
> +static const struct regmap_range qmc5883p_volatile_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_X_LSB, QMC5883P_REG_Z_MSB),
> +	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
> +};
> +
> +/*
> + * Precious registers: reading has a side effect (clears DRDY/OVFL bits).
> + * regmap will never read these speculatively.
> + */
> +static const struct regmap_range qmc5883p_precious_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
> +};
> +
> +static const struct regmap_access_table qmc5883p_readable_table = {
> +	.yes_ranges = qmc5883p_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_readable_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_writable_table = {
> +	.yes_ranges = qmc5
> 883p_writable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_writable_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_volatile_table = {
> +	.yes_ranges = qmc5883p_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_volatile_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_precious_table = {
> +	.yes_ranges = qmc5883p_precious_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_precious_ranges),
> +};
> +
> +static const struct regmap_config qmc5883p_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0x0B,
> +	.cache_type = REGCACHE_RBTREE,
> +	.rd_table = &qmc5883p_readable_table,
> +	.wr_table = &qmc5883p_writable_table,
> +	.volatile_table = &qmc5883p_volatile_table,
> +	.precious_table = &qmc5883p_precious_table,
> +};
> +
> +struct qmc5883p_data {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct mutex mutex; /* protects regmap and rf field accesses */
> +	struct qmc5883p_rf rf;
> +};
> +
> +enum qmc5883p_channels {
> +	AXIS_X = 0
> ,
> +	AXIS_Y,
> +	AXIS_Z,
> +};
> +
> +static const struct reg_field qmc5883p_rf_osr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 4, 5);
> +static const struct reg_field qmc5883p_rf_dsr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 6, 7);
> +static const struct reg_field qmc5883p_rf_odr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 2, 3);
> +static const struct reg_field qmc5883p_rf_mode =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 0, 1);
> +static const struct reg_field qmc5883p_rf_rng =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 2, 3);
> +static const struct reg_field qmc5883p_rf_rstctrl =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 0, 1);
> +static const struct reg_field qmc5883p_rf_sftrst =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
> +static const struct reg_field qmc5883p_rf_selftest =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 6, 6);
> +static const struct reg_field qmc5883p_rf_chip_id =
> +	REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);
> +
> +static int qmc5883p_rf_init(struct qmc5883p_data *data)
> +{
> +	struct regmap *regmap = data->regmap;
> +	struct device *dev = d
> ata->dev;
> +	struct qmc5883p_rf *rf = &data->rf;
> +
> +	rf->osr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_osr);
> +	if (IS_ERR(rf->osr))
> +		return PTR_ERR(rf->osr);
> +
> +	rf->dsr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_dsr);
> +	if (IS_ERR(rf->dsr))
> +		return PTR_ERR(rf->dsr);
> +
> +	rf->odr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_odr);
> +	if (IS_ERR(rf->odr))
> +		return PTR_ERR(rf->odr);
> +
> +	rf->mode = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_mode);
> +	if (IS_ERR(rf->mode))
> +		return PTR_ERR(rf->mode);
> +
> +	rf->rng = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rng);
> +	if (IS_ERR(rf->rng))
> +		return PTR_ERR(rf->rng);
> +
> +	rf->rstctrl = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rstctrl);
> +	if (IS_ERR(rf->rstctrl))
> +		return PTR_ERR(rf->rstctrl);
> +
> +	rf->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
> +	if (IS_ERR(rf->sftrst))
> +		return PTR_ERR(rf->sftrst);
> +
> +	rf->selftest =
> +		devm_regmap_field_alloc(de
> v, regmap, qmc5883p_rf_selftest);
> +	if (IS_ERR(rf->selftest))
> +		return PTR_ERR(rf->selftest);
> +
> +	rf->chip_id = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_chip_id);
> +	if (IS_ERR(rf->chip_id))
> +		return PTR_ERR(rf->chip_id);
> +
> +	return 0;
> +}
> +
> +static int qmc5883p_verify_chip_id(struct qmc5883p_data *data)
> +{
> +	int ret, regval;
> +
> +	ret = regmap_field_read(data->rf.chip_id, &regval);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "failed to read chip ID\n");
> +
> +	if (regval != QMC5883P_CHIP_ID)
> +		return dev_err_probe(data->dev, -ENODEV,

We don't consider ID match an error. It has happened too many times
that there is a compatible part with a different ID. This can just
be dev_info() and return success.

> +				     "unexpected chip ID 0x%02x, expected 0x%02x\n",
> +				     regval, QMC5883P_CHIP_ID);
> +	return ret;
> +}
> +
> +static int qmc5883p_chip_init(struct qmc5883p_data *data)
> +{
> +	int ret;
> +
> +	ret = regmap_field_write(data->rf.sftrst, 1);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(1000, 2000);

Use fsleep() instead and add a comment explaining why this specific duration
was selected.

> +
> +	ret = regmap_field_write(data->rf.sftrst, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write
> (data->rf.rstctrl, QMC5883P_RSTCTRL_SET_RESET);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.rng, QMC5883P_DEFAULT_RNG);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.osr, QMC5883P_DEFAULT_OSR);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.dsr, QMC5883P_DEFAULT_DSR);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.odr, QMC5883P_DEFAULT_ODR);
> +	if (ret)
> +		return ret;

Since we just reset the chip, why do we need to set everything to a
new default instead of using the chip's default? If there is a good
reason, add a comment, otherwise we can leave this out.

> +
> +	return regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);

Does this start sampling? Seems like it could be out of place here.

> +}
> +
> +/*
> + * qmc5883p_get_measure - read all three axes.
> + * Must be called with data->mutex held.
> + * Handles PM internally: resumes device, reads data, schedules autosuspend.
> + */
> +static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
> +				s16 *z)
> +{
> +	int ret;
> +	u8 reg_data[6];
> +	unsigned int status;
> +
> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Poll the status register until DR
> DY is set or timeout.
> +	 * Read the whole register in one shot so that OVFL is captured from
> +	 * the same read: reading 0x09 clears both DRDY and OVFL, so a second
> +	 * read would always see OVFL=0.
> +	 * At ODR=10Hz one period is 100ms; use 150ms as a safe upper bound.
> +	 */
> +	ret = regmap_read_poll_timeout(data->regmap, QMC5883P_REG_STATUS,
> +				       status, status & QMC5883P_STATUS_DRDY,
> +				       QMC5883P_DRDY_POLL_US, 150000);

Numbers with lots of 0s are easier to read as 150 * (MICRO / MILLI).

> +	if (ret)
> +		goto out;
> +
> +	if (status & QMC5883P_STATUS_OVFL) {
> +		dev_warn_ratelimited(data->dev,
> +				     "data overflow, consider reducing field range\n");
> +		ret = -ERANGE;
> +		goto out;
> +	}
> +
> +	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
> +			       ARRAY_SIZE(reg_data));
> +	if (ret)
> +		goto out;
> +
> +	*x = (s16)get_unaligned_le16(&reg_data[0]);
> +	*y = (s16)get_unaligned_le16(&reg_data[2]);
> +	*z = (s16)get_unaligned_le16(&reg_data[4]);
> +
> +out:
> +	pm_runtime_mark_last_busy(data->dev);
> +	pm_runtime_put_
> autosuspend(data->dev);
> +	return ret;
> +}
> +
> +static int qmc5883p_write_scale(struct qmc5883p_data *data, int val, int val2)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_scale); i++) {
> +		if (qmc5883p_scale[i][0] == val && qmc5883p_scale[i][1] == val2)
> +			return regmap_field_write(data->rf.rng, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int qmc5883p_write_odr(struct qmc5883p_data *data, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_odr); i++) {
> +		if (qmc5883p_odr[i] == val)
> +			return regmap_field_write(data->rf.odr, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int qmc5883p_write_osr(struct qmc5883p_data *data, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_osr); i++) {
> +		if (qmc5883p_osr[i] == val)
> +			return regmap_field_write(data->rf.osr, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static ssize_t downsampling_ratio_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio
> _dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	unsigned int regval;
> +	int ret;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	ret = regmap_field_read(data->rf.dsr, &regval);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%u\n", qmc5883p_dsr[regval]);
> +}
> +
> +static ssize_t downsampling_ratio_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	unsigned int val;
> +	int i, ret, restore;
> +
> +	ret = kstrtouint(buf, 10, &val);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	if (ret)
> +		goto out;
> +
> +	ret = -EINVAL;
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_dsr); i++) {
> +		if (qmc5883p_dsr[i] == val) {
> +			ret = regmap_field_wr
> ite(data->rf.dsr, i);
> +			break;
> +		}
> +	}
> +
> +	restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> +	if (restore && !ret)
> +		ret = restore;
> +
> +out:
> +	pm_runtime_mark_last_busy(data->dev);
> +	pm_runtime_put_autosuspend(data->dev);
> +	return ret ? ret : (ssize_t)len;
> +}
> +
> +static int qmc5883p_read_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	s16 x, y, z;
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret;
> +	unsigned int regval;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = qmc5883p_get_measure(data, &x, &y, &z);
> +		if (ret < 0)
> +			return ret;
> +		switch (chan->address) {
> +		case AXIS_X:
> +			*val = x;
> +			break;
> +		case AXIS_Y:
> +			*val = y;
> +			break;
> +		case AXIS_Z:
> +			*val = z;
> +			break;
> +		}
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regmap_field_read(data->rf.rng, &regval);
> +		if (
> ret < 0)
> +			return ret;
> +		*val = qmc5883p_scale[regval][0];
> +		*val2 = qmc5883p_scale[regval][1];
> +		return IIO_VAL_FRACTIONAL;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_field_read(data->rf.odr, &regval);
> +		if (ret < 0)
> +			return ret;
> +		*val = qmc5883p_odr[regval];
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		ret = regmap_field_read(data->rf.osr, &regval);
> +		if (ret < 0)
> +			return ret;
> +		*val = qmc5883p_osr[regval];
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int qmc5883p_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan, int val,
> +			      int val2, long mask)
> +{
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret, restore;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	if (ret)
> +		goto out;
> +
> +	switch (mask) {
> +	case IIO_
> CHAN_INFO_SAMP_FREQ:
> +		ret = qmc5883p_write_odr(data, val);
> +		break;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		ret = qmc5883p_write_osr(data, val);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = qmc5883p_write_scale(data, val, val2);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> +	if (restore && !ret)
> +		ret = restore;
> +
> +out:
> +	pm_runtime_mark_last_busy(data->dev);
> +	pm_runtime_put_autosuspend(data->dev);
> +	return ret;
> +}
> +
> +/*
> + * qmc5883p_read_avail - expose available values to userspace.
> + *
> + * Creates the _available sysfs attributes automatically:
> + *   in_magn_sampling_frequency_available
> + *   in_magn_oversampling_ratio_available
> + *   in_magn_scale_available
> + */
> +static int qmc5883p_read_avail(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       const int **vals, int *type, int *length,
> +			       long mask)
> +{
> +	switch (mask) {
> 
> 
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*vals = qmc5883p_odr;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(qmc5883p_odr);
> +		return IIO_AVAIL_LIST;
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*vals = qmc5883p_osr;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(qmc5883p_osr);
> +		return IIO_AVAIL_LIST;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*vals = (const int *)qmc5883p_scale;
> +		*type = IIO_VAL_FRACTIONAL;
> +		*length = ARRAY_SIZE(qmc5883p_scale) * 2;
> +		return IIO_AVAIL_LIST;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static IIO_DEVICE_ATTR(downsampling_ratio, 0644, downsampling_ratio_show,
> +		       downsampling_ratio_store, 0);
> +static IIO_CONST_ATTR(downsampling_ratio_available, "1 2 4 8");

As mentioned in the cover letter, we'd like to know more about what
this actually does. If there is a good reason it doesn't fit with
any existing filter attribute, then we'll need a patch to document
the sysfs ABI as well.

> +
> +static struct attribute *qmc5883p_attributes[] = {
> +	&iio_dev_attr_downsampling_ratio.dev_attr.attr,
> +	&iio_const_attr_downsampling_ratio_available.dev_attr.attr, NULL
> +};
> +
> +static const struct attribute_group qmc5883p_attribute_group = {
> +	.attrs = qmc5883p_attribu
> tes,
> +};
> +
> +static const struct iio_info qmc5883p_info = {
> +	.attrs = &qmc5883p_attribute_group,
> +	.read_raw = qmc5883p_read_raw,
> +	.write_raw = qmc5883p_write_raw,
> +	.read_avail = qmc5883p_read_avail,
> +};
> +
> +static const struct iio_chan_spec qmc5883p_channels[] = {
> +	{
> +		.type = IIO_MAGN,
> +		.channel2 = IIO_MOD_X,
> +		.modified = 1,
> +		.address = AXIS_X,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_type =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_type_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},

Could save some dupilcation by making a macro that takes X/Y/Z
as parameter. e.g.

#define QMC5883P_CHAN(ch) \
	...				\
	.channel2 = IIO_MOD_##ch,	\
	...				\
	.address = AXIS_##ch,		\
	...

> +	{
> +		.type = IIO_MAGN,
> +		.channel2 = IIO_MOD_Y,
> +		.modified = 1,
> +		.address = AXIS_Y,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_IN
> FO_SCALE),
> +		.info_mask_shared_by_type =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_type_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +	{
> +		.type = IIO_MAGN,
> +		.channel2 = IIO_MOD_Z,
> +		.modified = 1,
> +		.address = AXIS_Z,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_type =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_type_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +};
> +
> +static void qmc5883p_suspend_action(void *arg)
> +{
> +	struct qmc5883p_data *data = arg;
> +
> +	regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +}
> +
> +static int qmc5883p_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->d
> ev;
> +	struct qmc5883p_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &qmc5883p_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "regmap initialization failed\n");
> +
> +	data = iio_priv(indio_dev);
> +	data->dev = dev;
> +	data->regmap = regmap;
> +	mutex_init(&data->mutex);
> +
> +	ret = devm_regulator_get_enable_optional(dev, "vdd");
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret,
> +				"failed to get vdd regulator\n");
> +
> +	/* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
> +	fsleep(50000);
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	ret = qmc5883p_rf_init(data);

Would be more logical to move this up right after regmap is declared.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to initialize regmap fields\n");
> +
> +	ret = qmc5883p_verify_chip_id(data);
> +	if (ret)
> +		retur
> n ret;
> +
> +	ret = qmc5883p_chip_init(data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize chip\n");
> +
> +	indio_dev->name = "qmc5883p";
> +	indio_dev->info = &qmc5883p_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = qmc5883p_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(qmc5883p_channels);
> +
> +	pm_runtime_set_autosuspend_delay(dev, 2000);
> +	pm_runtime_use_autosuspend(dev);
> +
> +	pm_runtime_set_active(dev);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_mark_last_busy(dev);
> +
> +	ret = devm_add_action_or_reset(dev, qmc5883p_suspend_action, data);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);

Usually, we just return directly here. This pretty much doesn't ever fail.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to register IIO device\n");
> +	return 0;
> +}
> +
> +static int qmc5883p_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indi
> o_dev);
> +
> +	return regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +}
> +
> +static int qmc5883p_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(10000, 11000);

Again, fsleep() and comment.

> +	return 0;
> +}
> +
> +static int qmc5883p_system_suspend(struct device *dev)
> +{
> +	return pm_runtime_force_suspend(dev);
> +}
> +
> +static int qmc5883p_system_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * If the regulator was cut during system suspend, POR will have
> +	 * reset all registers. Reinitialise the chip and resync the
> +	 * regmap cache so that cached control register values are pushed
> +	 * back to har
> dware.
> +	 */
> +	ret = qmc5883p_chip_init(data);
> +	if (ret)
> +		return ret;
> +
> +	regcache_mark_dirty(data->regmap);
> +	return regcache_sync(data->regmap);
> +}
> +
> +static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(qmc5883p_system_suspend, qmc5883p_system_resume)
> +	RUNTIME_PM_OPS(qmc5883p_runtime_suspend, qmc5883p_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id qmc5883p_of_match[] = {
> +	{ .compatible = "qst,qmc5883p" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qmc5883p_of_match);
> +
> +static const struct i2c_device_id qmc5883p_id[] = {
> +	{ "qmc5883p", 0 },
> +	{},

IIO style for this is:

	{ }

space between braces and no trailing comma.

> +};
> +MODULE_DEVICE_TABLE(i2c, qmc5883p_id);
> +

^ permalink raw reply

* Re: [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver
From: David Lechner @ 2026-04-10 19:32 UTC (permalink / raw)
  To: Hardik Phalet, Greg Kroah-Hartman
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan,
	linux-iio, devicetree, linux-kernel, linux-staging
In-Reply-To: <20260409210639.3197576-5-hardik.phalet@pm.me>

On 4/9/26 4:07 PM, Hardik Phalet wrote:
> Add a MAINTAINERS entry for the QST QMC5883P staging IIO driver,
> covering the driver source and its device tree binding.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa16..d0b9bfceb283 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20274,6 +20274,13 @@ F:	Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
>  F:	drivers/bus/fsl-mc/
>  F:	include/uapi/linux/fsl_mc.h
>  
> +QST QMC5883P MAGNETOMETER DRIVER
> +M:	Hardik Phalet <hardik.phalet@pm.me>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
> +F:	drivers/staging/iio/magnetometer/
> +
>  QT1010 MEDIA DRIVER
>  L:	linux-media@vger.kernel.org
>  S:	Orphan

This should be split up and added in the patches that actually
add the F: files. Most of it will go with the dt-bingings patch
and the one line added later with the driver.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox