[PATCH v1] i2c: octeon_i2c: Add I2C controller driver for Octeon

[Stefan Roese <sr@denx.de>]

Hi Simon,

On 19.05.20 14:32, Simon Glass wrote:
> Hi,
> 
> On Thu, 14 May 2020 at 01:23, Stefan Roese <sr@denx.de> wrote:
>>
>> From: Suneel Garapati <sgarapati@marvell.com>
>>
>> Add support for I2C controllers found on Octeon II/III and Octeon TX
>> TX2 SoC platforms.
>>
>> Signed-off-by: Aaron Williams <awilliams@marvell.com>
>> Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>> Cc: Aaron Williams <awilliams@marvell.com>
>> Cc: Chandrakala Chavva <cchavva@marvell.com>
>> ---
>> RFC -> v1 (Stefan):
>> - Separated this patch from the OcteonTX/TX2 RFC patch series into a
>>    single patch. This is useful, as the upcoming MIPS Octeon support will
>>    use this I2C driver.
>> - Added MIPS Octeon II/III support (big endian). Rename driver and its
>>    function names from "octeontx" to "octeon" to better match all Octeon
>>    platforms.
>> - Moved from union to defines / bitmasks as suggested by Simon. This makes
>>    the driver usage on little- and big-endian platforms much easier.
>> - Enhanced Kconfig text
>> - Removed all clock macros (use values from DT)
>> - Removed long driver debug strings. This is only available when a debug
>>    version of this driver is built. The user / developer can lookup the
>>    descriptive error messages in the driver in this case anyway.
>> - Removed static "last_id"
>> - Dropped misc blank lines. Misc reformatting.
>> - Dropped "!= 0"
>> - Added missing function comments
>> - Added missing strut comments
>> - Changed comment style
>> - Renames "result" to "ret"
>> - Hex numbers uppercase
>> - Minor other changes
>> - Reword commit text and subject
>>
>>   drivers/i2c/Kconfig      |  10 +
>>   drivers/i2c/Makefile     |   1 +
>>   drivers/i2c/octeon_i2c.c | 803 +++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 814 insertions(+)
>>   create mode 100644 drivers/i2c/octeon_i2c.c
>>
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>> index e42b6516bf..1330b36698 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -374,6 +374,16 @@ config SYS_I2C_SANDBOX
>>            bus. Devices can be attached to the bus using the device tree
>>            which specifies the driver to use.  See sandbox.dts as an example.
>>
>> +config SYS_I2C_OCTEON
>> +       bool "Octeon II/III/TX/TX2 I2C driver"
>> +       depends on (ARCH_OCTEON || ARCH_OCTEONTX || ARCH_OCTEONTX2) && DM_I2C
>> +       default y
>> +       help
>> +         Add support for the Marvell Octeon I2C driver. This is used with
>> +         various Octeon parts such as Octeon II/III and OcteonTX/TX2. All
>> +         chips have several I2C ports and all are provided, controlled by
>> +         the device tree.
>> +
>>   config SYS_I2C_S3C24X0
>>          bool "Samsung I2C driver"
>>          depends on ARCH_EXYNOS4 && DM_I2C
>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>> index 62935b7ebc..2b58aae892 100644
>> --- a/drivers/i2c/Makefile
>> +++ b/drivers/i2c/Makefile
>> @@ -27,6 +27,7 @@ obj-$(CONFIG_SYS_I2C_LPC32XX) += lpc32xx_i2c.o
>>   obj-$(CONFIG_SYS_I2C_MESON) += meson_i2c.o
>>   obj-$(CONFIG_SYS_I2C_MVTWSI) += mvtwsi.o
>>   obj-$(CONFIG_SYS_I2C_MXC) += mxc_i2c.o
>> +obj-$(CONFIG_SYS_I2C_OCTEON) += octeon_i2c.o
>>   obj-$(CONFIG_SYS_I2C_OMAP24XX) += omap24xx_i2c.o
>>   obj-$(CONFIG_SYS_I2C_RCAR_I2C) += rcar_i2c.o
>>   obj-$(CONFIG_SYS_I2C_RCAR_IIC) += rcar_iic.o
>> diff --git a/drivers/i2c/octeon_i2c.c b/drivers/i2c/octeon_i2c.c
>> new file mode 100644
>> index 0000000000..210f98655e
>> --- /dev/null
>> +++ b/drivers/i2c/octeon_i2c.c
>> @@ -0,0 +1,803 @@
>> +// SPDX-License-Identifier:    GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Marvell International Ltd.
>> + */
>> +
>> +#include <common.h>
>> +#include <i2c.h>
>> +#include <dm.h>
>> +#include <pci_ids.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/clock.h>
>> +#include <linux/bitfield.h>
>> +
>> +/*
>> + * Octeon II/III (MIPS) have different register offsets than the ARM based
>> + * Octeon TX/TX2 SoCs
>> + */
>> +#if defined(CONFIG_ARCH_OCTEON)
>> +#define REG_OFFS               0x0000
>> +#else
>> +#define REG_OFFS               0x1000
>> +#endif
>> +
>> +#define TWSI_SW_TWSI           (REG_OFFS + 0x00)
>> +#define TWSI_TWSI_SW           (REG_OFFS + 0x08)
>> +#define TWSI_INT               (REG_OFFS + 0x10)
>> +#define TWSI_SW_TWSI_EXT       (REG_OFFS + 0x18)
>> +
>> +#define TWSI_SW_DATA_MASK      GENMASK_ULL(31, 0)
>> +#define TWSI_SW_EOP_IA_MASK    GENMASK_ULL(34, 32)
>> +#define TWSI_SW_IA_MASK                GENMASK_ULL(39, 35)
>> +#define TWSI_SW_ADDR_MASK      GENMASK_ULL(49, 40)
>> +#define TWSI_SW_SCR_MASK       GENMASK_ULL(51, 50)
>> +#define TWSI_SW_SIZE_MASK      GENMASK_ULL(54, 52)
>> +#define TWSI_SW_SOVR           BIT_ULL(55)
>> +#define TWSI_SW_R              BIT_ULL(56)
>> +#define TWSI_SW_OP_MASK                GENMASK_ULL(60, 57)
>> +#define TWSI_SW_EIA            GENMASK_ULL(61)
>> +#define TWSI_SW_SLONLY         BIT_ULL(62)
>> +#define TWSI_SW_V              BIT_ULL(63)
>> +
>> +#define TWSI_INT_SDA_OVR       BIT_ULL(8)
>> +#define TWSI_INT_SCL_OVR       BIT_ULL(9)
>> +#define TWSI_INT_SDA           BIT_ULL(10)
>> +#define TWSI_INT_SCL           BIT_ULL(11)
>> +
>> +enum {
>> +       TWSI_OP_WRITE   = 0,
>> +       TWSI_OP_READ    = 1,
>> +};
>> +
>> +enum {
>> +       TWSI_EOP_SLAVE_ADDR = 0,
>> +       TWSI_EOP_CLK_CTL = 3,
>> +       TWSI_SW_EOP_IA   = 6,
>> +};
>> +
>> +enum {
>> +       TWSI_SLAVEADD     = 0,
>> +       TWSI_DATA         = 1,
>> +       TWSI_CTL          = 2,
>> +       TWSI_CLKCTL       = 3,
>> +       TWSI_STAT         = 3,
>> +       TWSI_SLAVEADD_EXT = 4,
>> +       TWSI_RST          = 7,
>> +};
>> +
>> +enum {
>> +       TWSI_CTL_AAK    = BIT(2),
>> +       TWSI_CTL_IFLG   = BIT(3),
>> +       TWSI_CTL_STP    = BIT(4),
>> +       TWSI_CTL_STA    = BIT(5),
>> +       TWSI_CTL_ENAB   = BIT(6),
>> +       TWSI_CTL_CE     = BIT(7),
>> +};
>> +
>> +/*
>> + * Internal errors. When debugging is enabled, the driver will report the
>> + * error number and the user / developer can check the table below for the
>> + * detailed error description.
>> + */
>> +enum {
>> +       /** Bus error */
>> +       TWSI_STAT_BUS_ERROR             = 0x00,
>> +       /** Start condition transmitted */
>> +       TWSI_STAT_START                 = 0x08,
>> +       /** Repeat start condition transmitted */
>> +       TWSI_STAT_RSTART                = 0x10,
>> +       /** Address + write bit transmitted, ACK received */
>> +       TWSI_STAT_TXADDR_ACK            = 0x18,
>> +       /** Address + write bit transmitted, /ACK received */
>> +       TWSI_STAT_TXADDR_NAK            = 0x20,
>> +       /** Data byte transmitted in master mode, ACK received */
>> +       TWSI_STAT_TXDATA_ACK            = 0x28,
>> +       /** Data byte transmitted in master mode, ACK received */
>> +       TWSI_STAT_TXDATA_NAK            = 0x30,
>> +       /** Arbitration lost in address or data byte */
>> +       TWSI_STAT_TX_ARB_LOST           = 0x38,
>> +       /** Address + read bit transmitted, ACK received */
>> +       TWSI_STAT_RXADDR_ACK            = 0x40,
>> +       /** Address + read bit transmitted, /ACK received */
>> +       TWSI_STAT_RXADDR_NAK            = 0x48,
>> +       /** Data byte received in master mode, ACK transmitted */
>> +       TWSI_STAT_RXDATA_ACK_SENT       = 0x50,
>> +       /** Data byte received, NACK transmitted */
>> +       TWSI_STAT_RXDATA_NAK_SENT       = 0x58,
>> +       /** Slave address received, sent ACK */
>> +       TWSI_STAT_SLAVE_RXADDR_ACK      = 0x60,
>> +       /**
>> +        * Arbitration lost in address as master, slave address + write bit
>> +        * received, ACK transmitted
>> +        */
>> +       TWSI_STAT_TX_ACK_ARB_LOST       = 0x68,
>> +       /** General call address received, ACK transmitted */
>> +       TWSI_STAT_RX_GEN_ADDR_ACK       = 0x70,
>> +       /**
>> +        * Arbitration lost in address as master, general call address
>> +        * received, ACK transmitted
>> +        */
>> +       TWSI_STAT_RX_GEN_ADDR_ARB_LOST  = 0x78,
>> +       /** Data byte received after slave address received, ACK transmitted */
>> +       TWSI_STAT_SLAVE_RXDATA_ACK      = 0x80,
>> +       /** Data byte received after slave address received, /ACK transmitted */
>> +       TWSI_STAT_SLAVE_RXDATA_NAK      = 0x88,
>> +       /**
>> +        * Data byte received after general call address received, ACK
>> +        * transmitted
>> +        */
>> +       TWSI_STAT_GEN_RXADDR_ACK        = 0x90,
>> +       /**
>> +        * Data byte received after general call address received, /ACK
>> +        * transmitted
>> +        */
>> +       TWSI_STAT_GEN_RXADDR_NAK        = 0x98,
>> +       /** STOP or repeated START condition received in slave mode */
>> +       TWSI_STAT_STOP_MULTI_START      = 0xa0,
>> +       /** Slave address + read bit received, ACK transmitted */
>> +       TWSI_STAT_SLAVE_RXADDR2_ACK     = 0xa8,
>> +       /**
>> +        * Arbitration lost in address as master, slave address + read bit
>> +        * received, ACK transmitted
>> +        */
>> +       TWSI_STAT_RXDATA_ACK_ARB_LOST   = 0xb0,
>> +       /** Data byte transmitted in slave mode, ACK received */
>> +       TWSI_STAT_SLAVE_TXDATA_ACK      = 0xb8,
>> +       /** Data byte transmitted in slave mode, /ACK received */
>> +       TWSI_STAT_SLAVE_TXDATA_NAK      = 0xc0,
>> +       /** Last byte transmitted in slave mode, ACK received */
>> +       TWSI_STAT_SLAVE_TXDATA_END_ACK  = 0xc8,
>> +       /** Second address byte + write bit transmitted, ACK received */
>> +       TWSI_STAT_TXADDR2DATA_ACK       = 0xd0,
>> +       /** Second address byte + write bit transmitted, /ACK received */
>> +       TWSI_STAT_TXADDR2DATA_NAK       = 0xd8,
>> +       /** No relevant status information */
>> +       TWSI_STAT_IDLE                  = 0xf8
>> +};
>> +
>> +#ifndef CONFIG_SYS_I2C_OCTEON_SLAVE_ADDR
>> +# define CONFIG_SYS_I2C_OCTEON_SLAVE_ADDR      0x77
> 
> This should be a device-tree property.

Okay, will move to DT.

>> +#endif
>> +
>> +/**
>> + * struct octeon_twsi - Private data of this driver
>> + *
>> + * @base:      Base address of i2c registers
>> + */
>> +struct octeon_twsi {
>> +       void __iomem *base;
>> +};
>> +
>> +static void twsi_unblock(void *base);
>> +static int twsi_stop(void *base);
>> +
>> +#if defined(CONFIG_ARCH_OCTEON)
> 
> Needs a clock driver.

Right. I'll try to add clock driver support with the first Octeon base
port.

>> +static int get_io_clock(void)
>> +{
>> +       return octeon_get_io_clock();
>> +}
>> +#else
>> +static int get_io_clock(void)
>> +{
>> +       return octeontx_get_io_clock();
>> +}
>> +#endif
>> +
>> +static int twsi_thp(void)
>> +{
>> +       if (IS_ENABLED(CONFIG_ARCH_OCTEON) || IS_ENABLED(CONFIG_ARCH_OCTEONTX))
>> +               return 24;
>> +       else
>> +               return 3;
>> +}
>> +
> 
> [..]
> 
>> +/**
>> + * Calculate the divisor values
>> + *
>> + * @speed      Speed to set
>> + * @m_div      Pointer to M divisor
>> + * @n_div      Pointer to N divisor
>> + * @return     0 for success, otherwise error
>> + */
>> +static void twsi_calc_div(unsigned int speed, int *m_div, int *n_div)
>> +{
>> +       int io_clock_hz;
>> +       int tclk, fsamp;
>> +       int ndiv, mdiv;
>> +
>> +#if defined(CONFIG_ARCH_OCTEON) || defined(CONFIG_ARCH_OCTEONTX)
>> +       io_clock_hz = get_io_clock();
>> +       tclk = io_clock_hz / (2 * (twsi_thp() + 1));
>> +#elif defined(CONFIG_ARCH_OCTEONTX2)
>> +       /* Refclk src in mode register defaults to 100MHz clock */
>> +       io_clock_hz = 100000000; /* 100 Mhz */
>> +       tclk = io_clock_hz / (twsi_thp() + 2);
>> +#endif
> 
> Needs a clock driver. If different logic is needed it should use the
> compatible string / driver data rather than #ifdefs.

Yes, will update.

>> +       debug("%s( io_clock %u tclk %u)\n", __func__, io_clock_hz, tclk);
>> +
>> +       /*
>> +        * Compute the clocks M divider:
>> +        *
>> +        * TWSI freq = (core freq) / (10 x (M+1) x 2 * (thp+1) x 2^N)
>> +        * M = ((core freq) / (10 x (TWSI freq) x 2 * (thp+1) x 2^N)) - 1
>> +        *
>> +        * For OcteonTX2 -
>> +        * TWSI freq = (core freq) / (10 x (M+1) x (thp+2) x 2^N)
>> +        * M = ((core freq) / (10 x (TWSI freq) x (thp+2) x 2^N)) - 1
>> +        */
>> +       for (ndiv = 0; ndiv < 8; ndiv++) {
>> +               fsamp = tclk / (1 << ndiv);
>> +               mdiv = fsamp / speed / 10;
>> +               mdiv -= 1;
>> +               if (mdiv < 16)
>> +                       break;
>> +       }
>> +
>> +       *m_div = mdiv;
>> +       *n_div = ndiv;
>> +}
>> +
>> +/**
> 
> [..]
> 
>> +/**
>> + * Driver probe function
>> + *
>> + * @dev                I2C device to probe
>> + * @return     0 for success, otherwise error
>> + */
>> +static int octeon_pci_i2c_probe(struct udevice *dev)
>> +{
>> +       struct octeon_twsi *twsi = dev_get_priv(dev);
>> +#if !defined(CONFIG_ARCH_OCTEON)
> 
> Again should use dev_get_driver_data()

Yes, will update in v2.

Thanks,
Stefan