| Message ID | 20250404-iio-imu-inv-icm42600-rework-interrupt-using-names-v1-1-72ed5100da14@tdk.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: imu: inv_icm42600: switch to use generic name irq get | expand |
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 > >
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>; > }; >
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
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>; };
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(+)