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

[Mario Six]

Hi Stefan,

On Thu, Jul 21, 2016 at 9:05 AM, Stefan Roese <sr at denx.de> wrote:
> 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);
>

Ah, yes, I just copied that from the MVEBU_REGISTER macro to get it working,
and never went back to clean it up properly.

Will fix in v2.

> Otherwise your patch serial look good, so please add my:
>
> Reviewed-by: Stefan Roese <sr at denx.de>
>
> to all the patches.
>

Thanks for reviewing!

> Thanks,
> Stefan

Best regards,

Mario