[U-Boot] [PATCH 10/13] i2c: mvtwsi: Add compatibility to DM

[Stefan Roese]

Hi Mario,

first, thanks for this very nice patch series. Really appreciated.
One comment below...

On 18.07.2016 10:27, Mario Six wrote:
> This patch adds the necessary functions and Kconfig entry to make the
> MVTWSI I2C driver compatible with the driver model.
>
> A possible device tree entry might look like this:
>
> i2c at 11100 {
> 	compatible = "marvell,mv64xxx-i2c";
> 	reg = <0x11000 0x20>;
> 	clock-frequency = <100000>;
> 	u-boot,i2c-slave-addr = <0x0>;
> };
>
> Signed-off-by: Mario Six <mario.six at gdsys.cc>
> ---
>  drivers/i2c/Kconfig  |   7 +++
>  drivers/i2c/mvtwsi.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 123 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index 6e22bba..b3e8405 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -154,6 +154,13 @@ config SYS_I2C_UNIPHIER_F
>  	  Support for UniPhier FIFO-builtin I2C controller driver.
>  	  This I2C controller is used on PH1-Pro4 or newer UniPhier SoCs.
>
> +config SYS_I2C_MVTWSI
> +	bool "Marvell I2C driver"
> +	depends on DM_I2C
> +	help
> +	  Support for Marvell I2C controllers as used on the orion5x and
> +	  kirkwood SoC families.
> +
>  source "drivers/i2c/muxes/Kconfig"
>
>  endmenu
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index 3325c4b..c81e9d4 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -12,12 +12,19 @@
>  #include <i2c.h>
>  #include <asm/errno.h>
>  #include <asm/io.h>
> +#ifdef CONFIG_DM_I2C
> +#include <dm.h>
> +#include <mapmem.h>
> +#endif
> +
> +DECLARE_GLOBAL_DATA_PTR;
>
>  /*
>   * Include a file that will provide CONFIG_I2C_MVTWSI_BASE*, and possibly other
>   * settings
>   */
>
> +#ifndef CONFIG_DM_I2C
>  #if defined(CONFIG_ORION5X)
>  #include <asm/arch/orion5x.h>
>  #elif (defined(CONFIG_KIRKWOOD) || defined(CONFIG_ARCH_MVEBU))
> @@ -27,6 +34,7 @@
>  #else
>  #error Driver mvtwsi not supported by SoC or board
>  #endif
> +#endif /* CONFIG_DM_I2C */
>
>  /*
>   * TWSI register structure
> @@ -61,6 +69,19 @@ struct  mvtwsi_registers {
>
>  #endif
>
> +#ifdef CONFIG_DM_I2C
> +struct mvtwsi_i2c_dev {
> +	/* TWSI Register base for the device */
> +	struct mvtwsi_registers *base;
> +	/* Number of the device (determined from cell-index property) */
> +	int index;
> +	/* The I2C slave address for the device */
> +	u8 slaveadd;
> +	/* The configured I2C speed in Hz */
> +	uint speed;
> +};
> +#endif /* CONFIG_DM_I2C */
> +
>  /*
>   * enum mvtwsi_ctrl_register_fields - Bit masks for flags in the control
>   * register
> @@ -134,6 +155,7 @@ enum mvtwsi_ack_flags {
>  	MVTWSI_READ_ACK = 1,
>  };
>
> +#ifndef CONFIG_DM_I2C
>  /*
>   * MVTWSI controller base
>   */
> @@ -172,6 +194,7 @@ static struct mvtwsi_registers *twsi_get_base(struct i2c_adapter *adap)
>
>  	return NULL;
>  }
> +#endif
>
>  /*
>   * enum mvtwsi_error_class - types of I2C errors
> @@ -358,7 +381,7 @@ static uint __twsi_i2c_set_bus_speed(struct mvtwsi_registers *twsi,
>  }
>
>  static void __twsi_i2c_init(struct mvtwsi_registers *twsi, int speed,
> -			  int slaveadd)
> +			    int slaveadd)
>  {
>  	/* Reset controller */
>  	twsi_reset(twsi);
> @@ -472,6 +495,7 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi, uchar chip,
>  	return status != 0 ? status : stop_status;
>  }
>
> +#ifndef CONFIG_DM_I2C
>  static void twsi_i2c_init(struct i2c_adapter *adap, int speed,
>  			  int slaveadd)
>  {
> @@ -561,3 +585,94 @@ U_BOOT_I2C_ADAP_COMPLETE(twsi5, twsi_i2c_init, twsi_i2c_probe,
>  			 CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE, 5)
>
>  #endif
> +#else /* CONFIG_DM_I2C */
> +
> +static int mvtwsi_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
> +				 u32 chip_flags)
> +{
> +	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
> +	return __twsi_i2c_probe_chip(dev->base, chip_addr);
> +}
> +
> +static int mvtwsi_i2c_set_bus_speed(struct udevice *bus, uint speed)
> +{
> +	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
> +	return __twsi_i2c_set_bus_speed(dev->base, speed);
> +}
> +
> +static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus)
> +{
> +	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
> +	fdt_addr_t addr;
> +	fdt_size_t size;
> +
> +	addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, bus->of_offset,
> +						  "reg", 0, &size);
> +
> +	dev->base = map_sysmem(SOC_REGS_PHY_BASE + addr, size);
> +
> +	if (!dev->base)
> +		return -ENOMEM;


SOC_REGS_PHY_BASE is only defined for MVEBU / Armada XP / 38x. You
should use dev_get_addr() instead here. This will do all the
address translation you need:

	addr = dev_get_addr(bus);

Or if you need a ptr:

	dev->base = dev_get_addr_ptr(bus);

Otherwise your patch serial look good, so please add my:

Reviewed-by: Stefan Roese <sr at denx.de>

to all the patches.

Thanks,
Stefan