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

[PATCH 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>
---
Jean-Baptiste Maneyrol (2):
      dt-bindings: iio: imu: icm42600: add interrupt naming support
      iio: imu: inv_icm42600: switch to use generic name irq get

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

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

[PATCH 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 used.
Only INT1 is supported by the driver currently.
---
 .../devicetree/bindings/iio/imu/invensense,icm42600.yaml          | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
index 7e4492bbd0278a336587dc5ac04da7153453da29..f19cdfd7450e04e7c984dc987f3c23f5ad89a248 100644
--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
@@ -41,6 +41,12 @@ properties:
   interrupts:
     maxItems: 1
 
+  interrupt-names:
+    enum:
+      - INT1
+    description: |
+      set to "INT1" the only supported interrupts pin
+
   drive-open-drain:
     type: boolean
 
@@ -76,6 +82,7 @@ examples:
             reg = <0x68>;
             interrupt-parent = <&gpio2>;
             interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "INT1";
             vdd-supply = <&vdd>;
             vddio-supply = <&vddio>;
         };
@@ -95,6 +102,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

[PATCH 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.
---
 drivers/iio/imu/inv_icm42600/inv_icm42600.h      |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 13 +++++++++++--
 drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c  |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c  |  2 +-
 4 files changed, 14 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..bfdd7cd5fafae87ad7c6204f6dd3bef17935b3f9 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,14 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
 		return -ENODEV;
 	}
 
+	/* get INT1 only supported interrupt */
+	fwnode = dev_fwnode(dev);
+	if (!fwnode)
+		return -ENODEV;
+	irq = fwnode_irq_get_byname(fwnode, "INT1");
+	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 1/2] dt-bindings: iio: imu: icm42600: add interrupt naming support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

On Fri, Apr 04, 2025 at 05:52:02PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add interrupt-names field for specifying interrupt used.
> Only INT1 is supported by the driver currently.
> ---
>  .../devicetree/bindings/iio/imu/invensense,icm42600.yaml          | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index 7e4492bbd0278a336587dc5ac04da7153453da29..f19cdfd7450e04e7c984dc987f3c23f5ad89a248 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -41,6 +41,12 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  interrupt-names:
> +    enum:
> +      - INT1

An enum with one entry is just a const.

> +    description: |
> +      set to "INT1" the only supported interrupts pin

The commit message suggests there are others, you should list them all
here (if there are) even if the driver doesn't use them.

> +
>    drive-open-drain:
>      type: boolean
>  
> @@ -76,6 +82,7 @@ examples:
>              reg = <0x68>;
>              interrupt-parent = <&gpio2>;
>              interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "INT1";
>              vdd-supply = <&vdd>;
>              vddio-supply = <&vddio>;
>          };
> @@ -95,6 +102,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
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Jonathan Cameron]

On Fri, 04 Apr 2025 17:52:02 +0200
Jean-Baptiste Maneyrol via B4 Relay <devnull+jean-baptiste.maneyrol.tdk.com@kernel.org> wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add interrupt-names field for specifying interrupt used.
> Only INT1 is supported by the driver currently.
No SoB?

Also, these are tricky to retrofit.  Arguably this is a binding
fix because there was not documentation to see the interrupt was INT1,
so perhaps we don't need to care in the binding (beyond adding that
INT2 is valid as well).

However we don't want the driver to fail on DT out in the field so
we need to fallback to assuming that if the interrupt name is not supplied
we just grab the one interrupt there and assume it is INT1.

Jonathan

> ---
>  .../devicetree/bindings/iio/imu/invensense,icm42600.yaml          | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index 7e4492bbd0278a336587dc5ac04da7153453da29..f19cdfd7450e04e7c984dc987f3c23f5ad89a248 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -41,6 +41,12 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  interrupt-names:
> +    enum:
> +      - INT1
> +    description: |
> +      set to "INT1" the only supported interrupts pin
> +
>    drive-open-drain:
>      type: boolean
>  
> @@ -76,6 +82,7 @@ examples:
>              reg = <0x68>;
>              interrupt-parent = <&gpio2>;
>              interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "INT1";
>              vdd-supply = <&vdd>;
>              vddio-supply = <&vddio>;
>          };
> @@ -95,6 +102,7 @@ examples:
>              spi-cpol;
>              interrupt-parent = <&gpio1>;
>              interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "INT1";
>              vdd-supply = <&vdd>;
>              vddio-supply = <&vddio>;
>          };
> 

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

[Jonathan Cameron]

On Fri, 04 Apr 2025 17:52:03 +0200
Jean-Baptiste Maneyrol via B4 Relay <devnull+jean-baptiste.maneyrol.tdk.com@kernel.org> 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.
> ---
>  drivers/iio/imu/inv_icm42600/inv_icm42600.h      |  2 +-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 13 +++++++++++--
>  drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c  |  2 +-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_spi.c  |  2 +-
>  4 files changed, 14 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..bfdd7cd5fafae87ad7c6204f6dd3bef17935b3f9 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,14 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
>  		return -ENODEV;
>  	}
>  
> +	/* get INT1 only supported interrupt */
> +	fwnode = dev_fwnode(dev);
> +	if (!fwnode)
> +		return -ENODEV;
> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> +	if (irq < 0)
> +		return dev_err_probe(dev, irq, "error missing INT1 interrupt\n");
This needs to 'then' - possibly with a dev_info() - fallback to
using 
	fwnode_irq_get(fwnode, 0);
to get what would have previously been in spi->irq , i2c->irq etc
as I assume there is DT out in the wild without the names provided.

Jonathan

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

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

[Krzysztof Kozlowski]

On 04/04/2025 18:53, Conor Dooley wrote:
> On Fri, Apr 04, 2025 at 05:52:02PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
>> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
>>
>> Add interrupt-names field for specifying interrupt used.
>> Only INT1 is supported by the driver currently.


Please run scripts/checkpatch.pl and fix reported warnings. After that,
run also `scripts/checkpatch.pl --strict` and (probably) fix more
warnings. Some warnings can be ignored, especially from --strict run,
but the code here looks like it needs a fix. Feel free to get in touch
if the warning is not clear.

>> ---
>>  .../devicetree/bindings/iio/imu/invensense,icm42600.yaml          | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>> index 7e4492bbd0278a336587dc5ac04da7153453da29..f19cdfd7450e04e7c984dc987f3c23f5ad89a248 100644
>> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>> @@ -41,6 +41,12 @@ properties:
>>    interrupts:
>>      maxItems: 1
>>  
>> +  interrupt-names:
>> +    enum:
>> +      - INT1
> 
> An enum with one entry is just a const.

It's not one entry and that's the problem. Instead it should be items
with one const.



Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 04/04/2025 17:52, Jean-Baptiste Maneyrol via B4 Relay wrote:
>  
> +	/* get INT1 only supported interrupt */
> +	fwnode = dev_fwnode(dev);
> +	if (!fwnode)
> +		return -ENODEV;
> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> +	if (irq < 0)
> +		return dev_err_probe(dev, irq, "error missing INT1 interrupt\n");

That's an ABI break.

Best regards,
Krzysztof

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Fri, Apr 04, 2025 at 07:21:13PM +0200, Krzysztof Kozlowski wrote:
> On 04/04/2025 18:53, Conor Dooley wrote:
> > On Fri, Apr 04, 2025 at 05:52:02PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
> >> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> >>
> >> Add interrupt-names field for specifying interrupt used.
> >> Only INT1 is supported by the driver currently.
> 
> 
> Please run scripts/checkpatch.pl and fix reported warnings. After that,
> run also `scripts/checkpatch.pl --strict` and (probably) fix more
> warnings. Some warnings can be ignored, especially from --strict run,
> but the code here looks like it needs a fix. Feel free to get in touch
> if the warning is not clear.
> 
> >> ---
> >>  .../devicetree/bindings/iio/imu/invensense,icm42600.yaml          | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >> index 7e4492bbd0278a336587dc5ac04da7153453da29..f19cdfd7450e04e7c984dc987f3c23f5ad89a248 100644
> >> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >> @@ -41,6 +41,12 @@ properties:
> >>    interrupts:
> >>      maxItems: 1
> >>  
> >> +  interrupt-names:
> >> +    enum:
> >> +      - INT1
> > 
> > An enum with one entry is just a const.
> 
> It's not one entry and that's the problem. Instead it should be items
> with one const.

That was kinda meant to be a comment about the general problem of people
using single-item enums, given the other comment I made about there
being more than one interrupt the result was always going to end up
being an items list with more than one possibility anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]