[PATCH v3 0/2] iio: imu: inv_icm42600: switch to use generic name irq get

[PATCH v3 0/2] iio: imu: inv_icm42600: switch to use generic name irq get

[Jean-Baptiste Maneyrol via B4 Relay]

The purpose of this series is to switch to fwnode_irq_get_by_name()
in the core module instead of using irq from the bus parsing.
Add interrupt naming and support only INT1.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
Changes in v3:
- Update dt-binding to report support of the 2 interrupts and delete
  remark about driver feature support.
- Link to v2: https://lore.kernel.org/r/20250407-iio-imu-inv-icm42600-rework-interrupt-using-names-v2-0-c278acf587b2@tdk.com

Changes in v2:
- Add INT2 in interrupt-names enum and fix enum
- Add fallback to first interrupt if naming is not here to ensure
  backward compatibility
- Link to v1: https://lore.kernel.org/r/20250404-iio-imu-inv-icm42600-rework-interrupt-using-names-v1-0-72ed5100da14@tdk.com

---
Jean-Baptiste Maneyrol (2):
      dt-bindings: iio: imu: icm42600: add interrupt naming support
      iio: imu: inv_icm42600: switch to use generic name irq get

 .../bindings/iio/imu/invensense,icm42600.yaml           | 13 ++++++++++++-
 drivers/iio/imu/inv_icm42600/inv_icm42600.h             |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c        | 17 +++++++++++++++--
 drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c         |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c         |  2 +-
 5 files changed, 30 insertions(+), 6 deletions(-)
---
base-commit: 1c2409fe38d5c19015d69851d15ba543d1911932
change-id: 20250325-iio-imu-inv-icm42600-rework-interrupt-using-names-b397ced72835

Best regards,
-- 
Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

[PATCH v3 1/2] dt-bindings: iio: imu: icm42600: add interrupt naming support

[Jean-Baptiste Maneyrol via B4 Relay]

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add interrupt-names field for specifying interrupt pin configured.
Chips are supporting 2 interrupt pins, change interrupt to support 2
entries.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 .../devicetree/bindings/iio/imu/invensense,icm42600.yaml    | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
index 7e4492bbd0278a336587dc5ac04da7153453da29..d4d4e5c3d8562523872a737864610c26c8fccd82 100644
--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
@@ -39,7 +39,16 @@ properties:
     maxItems: 1
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - INT1
+        - INT2
 
   drive-open-drain:
     type: boolean
@@ -76,6 +85,7 @@ examples:
             reg = <0x68>;
             interrupt-parent = <&gpio2>;
             interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "INT1";
             vdd-supply = <&vdd>;
             vddio-supply = <&vddio>;
         };
@@ -95,6 +105,7 @@ examples:
             spi-cpol;
             interrupt-parent = <&gpio1>;
             interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "INT1";
             vdd-supply = <&vdd>;
             vddio-supply = <&vddio>;
         };

-- 
2.49.0

Re: [PATCH v3 1/2] dt-bindings: iio: imu: icm42600: add interrupt naming support

[Krzysztof Kozlowski]

On Wed, Apr 09, 2025 at 05:14:31PM GMT, Jean-Baptiste Maneyrol wrote:
> Add interrupt-names field for specifying interrupt pin configured.
> Chips are supporting 2 interrupt pins, change interrupt to support 2
> entries.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> ---
>  .../devicetree/bindings/iio/imu/invensense,icm42600.yaml    | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index 7e4492bbd0278a336587dc5ac04da7153453da29..d4d4e5c3d8562523872a737864610c26c8fccd82 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -39,7 +39,16 @@ properties:
>      maxItems: 1
>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - INT1
> +        - INT2

Why INT2+INT1 should be valid? Why is this flexible? Commit msg should
explain that, otherwise we expect fixed number of interrupts. Actually
commit msg is here specific - always 2 interrupts, so I do not
understand why 1 is allowed.

Best regards,
Krzysztof

[PATCH v3 2/2] iio: imu: inv_icm42600: switch to use generic name irq get

[Jean-Baptiste Maneyrol via B4 Relay]

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Use generic fwnode_irq_get_byname() for getting interrupt pin using
interrupt name. Only INT1 is supported by the driver currently.

If not found fallback to first defined interrupt to keep compatibility.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_icm42600/inv_icm42600.h      |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 17 +++++++++++++++--
 drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c  |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c  |  2 +-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
index 18787a43477b89db12caee597ab040af5c8f52d5..f893dbe6996506a33eb5d3be47e6765a923665c9 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
@@ -426,7 +426,7 @@ int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
 int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
 			     unsigned int writeval, unsigned int *readval);
 
-int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
+int inv_icm42600_core_probe(struct regmap *regmap, int chip,
 			    inv_icm42600_bus_setup bus_setup);
 
 struct iio_dev *inv_icm42600_gyro_init(struct inv_icm42600_state *st);
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index ef9875d3b79db116f9fb4f6d881a7979292c1792..8ce817a4d5aa78cc8b228ee3064083b336c2c357 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -683,12 +683,13 @@ static void inv_icm42600_disable_pm(void *_data)
 	pm_runtime_disable(dev);
 }
 
-int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
+int inv_icm42600_core_probe(struct regmap *regmap, int chip,
 			    inv_icm42600_bus_setup bus_setup)
 {
 	struct device *dev = regmap_get_device(regmap);
 	struct inv_icm42600_state *st;
-	int irq_type;
+	struct fwnode_handle *fwnode;
+	int irq, irq_type;
 	bool open_drain;
 	int ret;
 
@@ -697,6 +698,18 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
 		return -ENODEV;
 	}
 
+	/* get INT1 only supported interrupt or fallback to first interrupt */
+	fwnode = dev_fwnode(dev);
+	if (!fwnode)
+		return -ENODEV;
+	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;
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
index 04e440fe023aa3869529b0f0be003ea0544bfb8d..38cc0d7834fcb96dabc401f29d613cf9fc75b8f5 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
@@ -67,7 +67,7 @@ static int inv_icm42600_probe(struct i2c_client *client)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	return inv_icm42600_core_probe(regmap, chip, client->irq,
+	return inv_icm42600_core_probe(regmap, chip,
 				       inv_icm42600_i2c_bus_setup);
 }
 
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
index 2bd2c4c8e50c3fe081e882aca6c64736510b474c..f40a09c4cbfc673e76922d13d61a3634785300ec 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c
@@ -64,7 +64,7 @@ static int inv_icm42600_probe(struct spi_device *spi)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	return inv_icm42600_core_probe(regmap, chip, spi->irq,
+	return inv_icm42600_core_probe(regmap, chip,
 				       inv_icm42600_spi_bus_setup);
 }
 

-- 
2.49.0

Re: [PATCH v3 2/2] iio: imu: inv_icm42600: switch to use generic name irq get

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 05:14:32PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Use generic fwnode_irq_get_byname() for getting interrupt pin using
> interrupt name. Only INT1 is supported by the driver currently.
> 
> If not found fallback to first defined interrupt to keep compatibility.

...

> -int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
>  			    inv_icm42600_bus_setup bus_setup);

If you use 100 limit, it fits now on one line.

...

> -int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
>  			    inv_icm42600_bus_setup bus_setup)

Ditto.

...

> +	struct fwnode_handle *fwnode;

Do you need to include property.h?

...

> +	/* get INT1 only supported interrupt or fallback to first interrupt */
> +	fwnode = dev_fwnode(dev);

> +	if (!fwnode)
> +		return -ENODEV;

Unneeded check, the below will do it for you,

> +	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");

...

> -	return inv_icm42600_core_probe(regmap, chip, client->irq,
> +	return inv_icm42600_core_probe(regmap, chip,
>  				       inv_icm42600_i2c_bus_setup);

This is now one line (81 characters which is fine independently on your choice
of the limit).

...

> -	return inv_icm42600_core_probe(regmap, chip, spi->irq,
> +	return inv_icm42600_core_probe(regmap, chip,
>  				       inv_icm42600_spi_bus_setup);

One line.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: imu: inv_icm42600: switch to use generic name irq get

[Jonathan Cameron]

On Wed, 9 Apr 2025 20:07:43 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Wed, Apr 09, 2025 at 05:14:32PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
> > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> > 
> > Use generic fwnode_irq_get_byname() for getting interrupt pin using
> > interrupt name. Only INT1 is supported by the driver currently.
> > 
> > If not found fallback to first defined interrupt to keep compatibility.  
> 
> ...
> 
> > -int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> > +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
> >  			    inv_icm42600_bus_setup bus_setup);  
> 
> If you use 100 limit, it fits now on one line.

I'd rather stick to 'just over 80' where it really helps. Rather than
generally switch to 100.  Maybe that day will come but I'm not sure it is
yet.

> 
> ...
> 
> > -int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> > +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
> >  			    inv_icm42600_bus_setup bus_setup)  
> 
> Ditto.
> 
> ...
> 
> > +	struct fwnode_handle *fwnode;  
> 
> Do you need to include property.h?
> 
> ...
> 
> > +	/* get INT1 only supported interrupt or fallback to first interrupt */
> > +	fwnode = dev_fwnode(dev);  
> 
> > +	if (!fwnode)
> > +		return -ENODEV;  
> 
> Unneeded check, the below will do it for you,
> 
> > +	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");  
> 
> ...
> 
> > -	return inv_icm42600_core_probe(regmap, chip, client->irq,
> > +	return inv_icm42600_core_probe(regmap, chip,
> >  				       inv_icm42600_i2c_bus_setup);  
> 
> This is now one line (81 characters which is fine independently on your choice
> of the limit).
> 
> ...
> 
> > -	return inv_icm42600_core_probe(regmap, chip, spi->irq,
> > +	return inv_icm42600_core_probe(regmap, chip,
> >  				       inv_icm42600_spi_bus_setup);  
> 
> One line.
> 

Re: [PATCH v3 2/2] iio: imu: inv_icm42600: switch to use generic name irq get

[Jean-Baptiste Maneyrol]

Hello Andy,

I tried to respond inline, but my apologies it is not working well with the email client I'm obliged to use.

To sum-up, property.h is already included and I will simplify the fwnode usage to use the API calls directly.

I will just keep the over 80 limit following Jonathan recommendations.

Thanks,
JB

________________________________________
From: Andy Shevchenko <andy@kernel.org>
Sent: Wednesday, April 9, 2025 19:07
To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen <lars@metafoo.de>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; David Lechner <dlechner@baylibre.com>; Nuno Sá <nuno.sa@analog.com>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 2/2] iio: imu: inv_icm42600: switch to use generic name irq get
 
This Message Is From an External Sender
This message came from outside your organization.
 
On Wed, Apr 09, 2025 at 05:14:32PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Use generic fwnode_irq_get_byname() for getting interrupt pin using
> interrupt name. Only INT1 is supported by the driver currently.
> 
> If not found fallback to first defined interrupt to keep compatibility.

...

> -int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
>  			    inv_icm42600_bus_setup bus_setup);

If you use 100 limit, it fits now on one line.

...

> -int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
>  			    inv_icm42600_bus_setup bus_setup)

Ditto.

...

> +	struct fwnode_handle *fwnode;

Do you need to include property.h?

property.h is already included.

...

> +	/* get INT1 only supported interrupt or fallback to first interrupt */
> +	fwnode = dev_fwnode(dev);

> +	if (!fwnode)
> +		return -ENODEV;

Unneeded check, the below will do it for you,

OK, I will simplify and user directly the fwnode functions.

> +	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");

...

> -	return inv_icm42600_core_probe(regmap, chip, client->irq,
> +	return inv_icm42600_core_probe(regmap, chip,
>  				       inv_icm42600_i2c_bus_setup);

This is now one line (81 characters which is fine independently on your choice
of the limit).

...

> -	return inv_icm42600_core_probe(regmap, chip, spi->irq,
> +	return inv_icm42600_core_probe(regmap, chip,
>  				       inv_icm42600_spi_bus_setup);

One line.

-- 
With Best Regards,
Andy Shevchenko