| Message ID | 20240826150840.25497-5-wsa+renesas@sang-engineering.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: rcar: add SMBAlert support | expand |
Hi Geert, > IIUIC, this is not a property of the hardware, but a side-channel > independent from the actual I2C controller hardware? Then a generic > "smbus-alert-gpios" property sounds more appropriate to me. It is not generic. While it is true that most I2C controllers do need GPIOs as a side channel, there are controllers having... > BTW, are you aware of any I2C controller having a dedicated input pin > for this? ... this. Check 'i2c-stm32f7.c' or 'i2c-xlp9xx.c'. Thank you for your comments! Wolfram
Hi Wolfram, On Sat, Aug 31, 2024 at 5:31 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > IIUIC, this is not a property of the hardware, but a side-channel > > independent from the actual I2C controller hardware? Then a generic > > "smbus-alert-gpios" property sounds more appropriate to me. > > It is not generic. While it is true that most I2C controllers do need > GPIOs as a side channel, there are controllers having... > > > BTW, are you aware of any I2C controller having a dedicated input pin > > for this? > > ... this. Check 'i2c-stm32f7.c' or 'i2c-xlp9xx.c'. OK... Still, this interrupt is not a property of the R-Car i2C hardware block, so it should not be modelled as such. To me, this looks similar to hardware flow control on serial ports: some ports support this in hardware, other ports need to use GPIOs, or board designers may still decide to use a GPIO anyway. See Documentation/devicetree/bindings/serial/serial.yaml and drivers/tty/serial/serial_mctrl_gpio.c, which uses GPIO interrupts. Gr{oetje,eeting}s, Geert
> Still, this interrupt is not a property of the R-Car i2C hardware block, > so it should not be modelled as such. Hmm, you are probably right, given that I need this in the board DTS: === &i2c3 { pinctrl-0 = <&i2c3_pins>; pinctrl-names = "i2c-pwr"; + + /delete-property/ interrupts; + interrupts-extended = <&gic GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>, <&gpio1 26 IRQ_TYPE_EDGE_FALLING>; + interrupt-names = "main", "smbus_alert"; + + smbus; }; === I have to admit this is not exactly pretty. Pity, though, the I2C core is all prepared for the above. Seems I have to update the core for "alert-gpios", after all. Happy hacking, Wolfram
diff --git a/Documentation/devicetree/bindings/i2c/renesas,rcar-i2c.yaml b/Documentation/devicetree/bindings/i2c/renesas,rcar-i2c.yaml index 6cc60c3f61cd..2eed3ae7c57d 100644 --- a/Documentation/devicetree/bindings/i2c/renesas,rcar-i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/renesas,rcar-i2c.yaml @@ -60,7 +60,20 @@ properties: maxItems: 1 interrupts: - maxItems: 1 + minItems: 1 + maxItems: 2 + description: + Without interrupt-names, the first interrupt listed must be the one + of the IP core, the second optional interrupt listed must handle + SMBALERT#, likely a GPIO. + + interrupt-names: + minItems: 1 + maxItems: 2 + items: + enum: + - main + - smbus_alert clock-frequency: description: @@ -155,7 +168,8 @@ examples: i2c0: i2c@e6508000 { compatible = "renesas,i2c-r8a7791", "renesas,rcar-gen2-i2c"; reg = <0xe6508000 0x40>; - interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>; + interrupts-extended = <&gic GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>, <&gpio5 1 IRQ_TYPE_EDGE_FALLING>; + interrupt-names = "main", "smbus_alert"; clock-frequency = <400000>; clocks = <&cpg CPG_MOD 931>; power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- Question: Should I remove 'smbus_alert' from the enum of 'interrupt-names'? It is already documented here: https://github.com/devicetree-org/dt-schema/commit/c51125d571cac9596048e888a856d70650e400e0 .../bindings/i2c/renesas,rcar-i2c.yaml | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)