mbox series

[v5,0/5] ARM: Add GXP I2C Support

Message ID 20230217155054.99757-1-nick.hawkins@hpe.com
Headers show
Series ARM: Add GXP I2C Support | expand

Message

Hawkins, Nick Feb. 17, 2023, 3:50 p.m. UTC
From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP SoC supports 10 I2C engines. Each I2C engine is completely
independent and can function both as an I2C master and I2C slave. The
I2C master can operate in a multi master environment. The engines support
a scalable speed from 8kHZ to 1.5 Mhz.

---
Changes since v4:
 *Removed use of i2c_global_init_done
 *Removed use of if else case for inline conditional
 *Removed error messages for timeouts and NACKS
 *Added bit definitions to replace magic values
 *Fix build error
 *Relocate Kconfig definition
Changes since v3:
 *Switch engine variable to u32
 *Disable IRQ on device remove with register write instead
 *Provided even greater description with the use of Phandle
Changes since v2:
 *Disable IRQ on a device remove
 *Remove use of I2C_CLASS_DEPRECATED
 *Use i2c_parse_fw_timings instead of of_property_read_u32
 *Remove redundant dev_err_probe as platform_get_irq already has one
 *Used __iomem instead of res->start to find physical address
 *Use BIT in gxp_i2c_irq_handler
 *Made value u8 instead of u16 for u8 read
 *Provided a better description of Phandle in yaml
Changes since v1:
 *Removed yaml documentation of hpe,gxp-sysreg as it has been
  applied to syscon.yaml
 *Made i2cX a generic node name i2c in dts file
 *Added status field to the dtsi and the dts for i2c bus
 *Removed unnecessary size-cells and address-cells from yaml
 *Removed phandle from hpe,sysreg-phandle
 *Changed hpe,i2c-max-bus-freq to clock-frequency
 *Removed rogue tab in structure definition
 *Removed use of __iomem *base local variables as it was
  unnecessary
 *Switched #if IS_ENABLED() -> if (IS_ENABLED()) inside
  functions
 *Removed use of pr_* functions
 *Removed informational prints in register and unregister
  functions
 *Removed print from interrupt handler
 *Removed informational prints from probe function
 *Switched dev_err -> dev_err_probe in probe function
 *Used the respective helper for mapping the resource to
  __iomem*

Nick Hawkins (5):
  i2c: hpe: Add GXP SoC I2C Controller
  dt-bindings: i2c: Add hpe,gxp-i2c
  ARM: dts: hpe: Add I2C Topology
  ARM: multi_v7_defconfig: add gxp i2c module
  MAINTAINERS: Add HPE GXP I2C Support

 .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml  |  59 ++
 MAINTAINERS                                   |   2 +
 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts      | 109 +++
 arch/arm/boot/dts/hpe-gxp.dtsi                | 125 ++++
 arch/arm/configs/multi_v7_defconfig           |   1 +
 drivers/i2c/busses/Kconfig                    |   7 +
 drivers/i2c/busses/Makefile                   |   1 +
 drivers/i2c/busses/i2c-gxp.c                  | 620 ++++++++++++++++++
 8 files changed, 924 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
 create mode 100644 drivers/i2c/busses/i2c-gxp.c

Comments

Wolfram Sang Feb. 17, 2023, 10:08 p.m. UTC | #1
On Fri, Feb 17, 2023 at 09:50:50AM -0600, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The GXP SoC supports 10 I2C engines. Each I2C engine is completely
> independent and can function both as an I2C master and I2C slave. The
> I2C master can operate in a multi master environment. The engines support
> a scalable speed from 8kHZ to 1.5 Mhz.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 

Thank you for adding the #defines, it makes the code much easier to
understand. I noticed one thing to fix, but I will send an incremental
patch for it.

Applied to for-next, thanks!
Krzysztof Kozlowski Feb. 18, 2023, 10:10 a.m. UTC | #2
On 17/02/2023 16:50, nick.hawkins@hpe.com wrote:
> diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
> index cf735b3c4f35..3bc071149bae 100644
> --- a/arch/arm/boot/dts/hpe-gxp.dtsi
> +++ b/arch/arm/boot/dts/hpe-gxp.dtsi
> @@ -122,6 +122,131 @@
>  				interrupts = <6>;
>  				interrupt-parent = <&vic0>;
>  			};
> +
> +			sysreg_system_controller: syscon@f8 {
> +				compatible = "hpe,gxp-sysreg", "syscon";
> +				reg = <0xf8 0x8>;
> +			};
> +
> +			i2c0: i2c@2000 {
> +				compatible = "hpe,gxp-i2c";
> +				reg = <0x2000 0x70>;
> +				interrupts = <9>;
> +				interrupt-parent = <&vic0>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				status = "disabled";
> +				hpe,sysreg = <&sysreg_system_controller>;
> +				clock-frequency = <100000>;

clock-frequency is a property of the board. If it is fixed in the SoC,
then make it fixed in the driver and there is no point for this property
in DT.

Best regards,
Krzysztof
Hawkins, Nick Feb. 23, 2023, 9:02 p.m. UTC | #3
> clock-frequency is a property of the board. If it is fixed in the SoC,
> then make it fixed in the driver and there is no point for this property
> in DT.

Greetings Krzysztof,

It can be other values however if this property is missing the code will
default to this value.

I will remove it.

Thanks,

-Nick Hawkins
Krzysztof Kozlowski Feb. 24, 2023, 8:25 a.m. UTC | #4
On 23/02/2023 22:02, Hawkins, Nick wrote:
>> clock-frequency is a property of the board. If it is fixed in the SoC,
>> then make it fixed in the driver and there is no point for this property
>> in DT.
> 
> Greetings Krzysztof,
> 
> It can be other values however

Hm, are you sure? On the same SoC, same or different I2C controllers
will have different frequency, entirely independent of board configuration?

Best regards,
Krzysztof