Hi Stefan,
On Thu, Jul 21, 2016 at 9:05 AM, Stefan Roese <s...@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@11100 {
>> compatible = "marvell,mv64xxx-i2c";
>> reg = <0x11000 0x20>;
>> clock-frequency = <100000>;
>> u-boot,i2c-slave-addr = <0x0>;
>> };
>>
>> Signed-off-by: Mario Six <mario....@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 <s...@denx.de>
>
> to all the patches.
>
Thanks for reviewing!
> Thanks,
> Stefan
Best regards,
Mario
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
