[PATCH linux dev-4.10 1/6] drivers: i2c: Add FSI-attached I2C master algorithm
Eddie James
eajames at linux.vnet.ibm.com
Thu May 11 00:21:48 AEST 2017
On 05/10/2017 03:11 AM, Joel Stanley wrote:
> On Fri, May 5, 2017 at 5:38 AM, Eddie James <eajames at linux.vnet.ibm.com> wrote:
>> From: "Edward A. James" <eajames at us.ibm.com>
>>
>> Initial startup code for the I2C algorithm to drive the I2C master
>> located on POWER CPUs over FSI bus.
>>
>> Signed-off-by: Edward A. James <eajames at us.ibm.com>
>> ---
>> drivers/Makefile | 2 +-
>> drivers/i2c/busses/Kconfig | 11 ++
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-fsi.c | 275 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 288 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/i2c/busses/i2c-fsi.c
>>
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index 67ce51d..278f109 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -105,6 +105,7 @@ obj-$(CONFIG_SERIO) += input/serio/
>> obj-$(CONFIG_GAMEPORT) += input/gameport/
>> obj-$(CONFIG_INPUT) += input/
>> obj-$(CONFIG_RTC_LIB) += rtc/
>> +obj-$(CONFIG_FSI) += fsi/
>> obj-y += i2c/ media/
>> obj-$(CONFIG_PPS) += pps/
>> obj-y += ptp/
>> @@ -173,4 +174,3 @@ obj-$(CONFIG_STM) += hwtracing/stm/
>> obj-$(CONFIG_ANDROID) += android/
>> obj-$(CONFIG_NVMEM) += nvmem/
>> obj-$(CONFIG_FPGA) += fpga/
>> -obj-$(CONFIG_FSI) += fsi/
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 7e34901d..0c714e0 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -1245,4 +1245,15 @@ config I2C_OPAL
>> This driver can also be built as a module. If so, the module will be
>> called as i2c-opal.
>>
>> +config I2C_FSI
>> + tristate "FSI I2C driver"
>> + depends on FSI
>> + help
>> + Driver for FSI bus attached I2C masters. These are I2C masters that
>> + are connected to the system over a FSI bus, instead of the more
>> + common PCI or MMIO interface.
>> +
>> + This driver can also be built as a module. If so, the module will be
>> + called as i2c-fsi.
>> +
>> endmenu
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 5798b4e..547773b 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -124,5 +124,6 @@ obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
>> obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
>> obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
>> obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
>> +obj-$(CONFIG_I2C_FSI) += i2c-fsi.o
>>
>> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
>> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
>> new file mode 100644
>> index 0000000..bb12fa9
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-fsi.c
>> @@ -0,0 +1,275 @@
>> +/*
>> + * Copyright 2017 IBM Corporation
>> + *
>> + * Eddie James <eajames at us.ibm.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +#include <linux/fsi.h>
>> +#include <linux/i2c.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/sched.h>
>> +#include <linux/semaphore.h>
>> +#include <linux/wait.h>
>> +
>> +#define FSI_ENGID_I2C_FSI 0x7
> Is the engine ID something that is part of the FSI spec? If so, we
> should put the IDs in the FSI headers.
It's defined in the engine code everywhere else... SCOM driver, hub
master code, etc. So I think we should leave it here for now.
>
>> +
>> +/* Find left shift from first set bit in m */
>> +#define MASK_TO_LSH(m) (__builtin_ffsll(m) - 1ULL)
>> +
>> +/* Extract field m from v */
>> +#define GETFIELD(m, v) (((v) & (m)) >> MASK_TO_LSH(m))
>> +
>> +/* Set field m of v to val */
>> +#define SETFIELD(m, v, val) \
>> + (((v) & ~(m)) | ((((typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
> My eyes :)
>
> Are these going to be used by all of our FSI drivers? Perhaps we could
> get Chris to put them in the common FSI header.
Agreed, it's ugly :) I haven't seen these in any other FSI drivers
though... would prefer to leave it here.
>
>> +
>> +#define I2C_DEFAULT_CLK_DIV 6
>> +#define I2C_DEFAULT_CLK 333333
>> +
>> +/* i2c registers */
>> +#define I2C_FSI_FIFO 0x00
>> +#define I2C_FSI_CMD 0x04
>> +#define I2C_FSI_MODE 0x08
>> +#define I2C_FSI_WATER_MARK 0x0C
>> +#define I2C_FSI_INT_MASK 0x10
>> +#define I2C_FSI_INT_COND 0x14
>> +#define I2C_FSI_OR_INT_MASK 0x14
>> +#define I2C_FSI_INTS 0x18
>> +#define I2C_FSI_AND_INT_MASK 0x18
>> +#define I2C_FSI_STAT 0x1C
>> +#define I2C_FSI_RESET_I2C 0x1C
>> +#define I2C_FSI_ESTAT 0x20
>> +#define I2C_FSI_RESET_ERR 0x20
>> +#define I2C_FSI_RESID_LEN 0x24
>> +#define I2C_FSI_SET_SCL 0x24
>> +#define I2C_FSI_PORT_BUSY 0x28
>> +#define I2C_FSI_RESET_SCL 0x2C
>> +#define I2C_FSI_SET_SDA 0x30
>> +#define I2C_FSI_RESET_SDA 0x34
>> +
>> +/* cmd register */
>> +#define I2C_CMD_WITH_START 0x80000000
>> +#define I2C_CMD_WITH_ADDR 0x40000000
>> +#define I2C_CMD_RD_CONT 0x20000000
>> +#define I2C_CMD_WITH_STOP 0x10000000
>> +#define I2C_CMD_FORCELAUNCH 0x08000000
>> +#define I2C_CMD_ADDR 0x00fe0000
>> +#define I2C_CMD_READ 0x00010000
>> +#define I2C_CMD_LEN 0x0000ffff
>> +
>> +/* mode register */
>> +#define I2C_MODE_CLKDIV 0xffff0000
>> +#define I2C_MODE_PORT 0x0000fc00
>> +#define I2C_MODE_ENHANCED 0x00000008
>> +#define I2C_MODE_DIAG 0x00000004
>> +#define I2C_MODE_PACE_ALLOW 0x00000002
>> +#define I2C_MODE_WRAP 0x00000001
>> +
>> +/* watermark register */
>> +#define I2C_WATERMARK_HI 0x0000f000
>> +#define I2C_WATERMARK_LO 0x000000f0
>> +
>> +#define I2C_FIFO_HI_LVL 4
>> +#define I2C_FIFO_LO_LVL 4
>> +
>> +/* interrupt register */
>> +#define I2C_INT_INV_CMD 0x00008000
>> +#define I2C_INT_PARITY 0x00004000
>> +#define I2C_INT_BE_OVERRUN 0x00002000
>> +#define I2C_INT_BE_ACCESS 0x00001000
>> +#define I2C_INT_LOST_ARB 0x00000800
>> +#define I2C_INT_NACK 0x00000400
>> +#define I2C_INT_DAT_REQ 0x00000200
>> +#define I2C_INT_CMD_COMP 0x00000100
>> +#define I2C_INT_STOP_ERR 0x00000080
>> +#define I2C_INT_BUSY 0x00000040
>> +#define I2C_INT_IDLE 0x00000020
>> +
>> +#define I2C_INT_ENABLE 0x0000ff80
>> +#define I2C_INT_ERR 0x0000fcc0
>> +
>> +/* status register */
>> +#define I2C_STAT_INV_CMD 0x80000000
>> +#define I2C_STAT_PARITY 0x40000000
>> +#define I2C_STAT_BE_OVERRUN 0x20000000
>> +#define I2C_STAT_BE_ACCESS 0x10000000
>> +#define I2C_STAT_LOST_ARB 0x08000000
>> +#define I2C_STAT_NACK 0x04000000
>> +#define I2C_STAT_DAT_REQ 0x02000000
>> +#define I2C_STAT_CMD_COMP 0x01000000
>> +#define I2C_STAT_STOP_ERR 0x00800000
>> +#define I2C_STAT_MAX_PORT 0x000f0000
>> +#define I2C_STAT_ANY_INT 0x00008000
>> +#define I2C_STAT_SCL_IN 0x00000800
>> +#define I2C_STAT_SDA_IN 0x00000400
>> +#define I2C_STAT_PORT_BUSY 0x00000200
>> +#define I2C_STAT_SELF_BUSY 0x00000100
>> +#define I2C_STAT_FIFO_COUNT 0x000000ff
>> +
>> +#define I2C_STAT_ERR 0xfc800000
>> +#define I2C_STAT_ANY_RESP 0xff800000
>> +
>> +/* extended status register */
>> +#define I2C_ESTAT_FIFO_SZ 0xff000000
>> +#define I2C_ESTAT_SCL_IN_SY 0x00008000
>> +#define I2C_ESTAT_SDA_IN_SY 0x00004000
>> +#define I2C_ESTAT_S_SCL 0x00002000
>> +#define I2C_ESTAT_S_SDA 0x00001000
>> +#define I2C_ESTAT_M_SCL 0x00000800
>> +#define I2C_ESTAT_M_SDA 0x00000400
>> +#define I2C_ESTAT_HI_WATER 0x00000200
>> +#define I2C_ESTAT_LO_WATER 0x00000100
>> +#define I2C_ESTAT_PORT_BUSY 0x00000080
>> +#define I2C_ESTAT_SELF_BUSY 0x00000040
>> +#define I2C_ESTAT_VERSION 0x0000001f
>> +
>> +struct fsi_i2c_master {
>> + struct fsi_device *fsi;
>> + u8 fifo_size;
>> + unsigned long clk;
>> +};
>> +
>> +static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
>> + u32 *data)
>> +{
>> + int rc;
>> + u32 _data;
> Not a fan of the underscores. Perhaps raw_data?
Sure.
>
>> +
>> + rc = fsi_device_read(fsi, reg, &_data, sizeof(_data));
>> + if (rc)
>> + return rc;
>> +
>> + *data = be32_to_cpu(_data);
>> +
>> + return 0;
>> +}
>> +
>> +static int fsi_i2c_write_reg(struct fsi_device *fsi, unsigned int reg,
>> + u32 *data)
>> +{
>> + u32 _data = cpu_to_be32(*data);
>> +
>> + return fsi_device_write(fsi, reg, &_data, sizeof(_data));
> It would be nice if the FSI api checked that you were passing it data
> in the correct endiannes.
Hmm, I disagree. Mainly because the API accepts data of any size, so
byte-swapping words doesn't make sense. (although in reality only 1, 2,
or 4 byte access is accepted). When this driver reads from the FIFO
register, it should not swap the endian so that it returns the data
as-is off the device. However we do need to swap the endian for the
other registers, so that we're reading/setting the right bits. So it's
really usage-dependent.
>
>> +}
>> +
>> +static unsigned long fsi_i2c_get_clk_div(struct fsi_i2c_master *i2c)
>> +{
>> + unsigned long clk_div = I2C_DEFAULT_CLK_DIV;
>> +
>> + return clk_div;
>> +}
> A bit of overkill. You could just use the #define where you need it.
I did this so that we could actually calculate clock div based on fsi
and i2c clocks in the future... I'll just use the #define for now then.
>
>> +
>> +static int fsi_i2c_dev_init(struct fsi_i2c_master *i2c)
>> +{
>> + int rc;
>> + unsigned long clk_div;
>> + u32 mode = I2C_MODE_ENHANCED, extended_status, watermark = 0;
>> + u32 interrupt = 0;
>> +
>> + /* since we use polling, disable interrupts */
>> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_INT_MASK, &interrupt);
>> + if (rc)
>> + return rc;
>> +
>> + clk_div = fsi_i2c_get_clk_div(i2c);
>> + mode = SETFIELD(I2C_MODE_CLKDIV, mode, clk_div);
>> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
>> + if (rc)
>> + return rc;
>> +
>> + rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_ESTAT, &extended_status);
>> + if (rc)
>> + return rc;
>> +
>> + i2c->fifo_size = GETFIELD(I2C_ESTAT_FIFO_SZ, extended_status);
>> + watermark = SETFIELD(I2C_WATERMARK_HI, watermark,
>> + i2c->fifo_size - I2C_FIFO_HI_LVL);
>> + watermark = SETFIELD(I2C_WATERMARK_LO, watermark,
>> + I2C_FIFO_LO_LVL);
>> +
>> + rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_WATER_MARK, &watermark);
>> +
>> + return rc;
>> +}
>> +
>> +static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>> + int num)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> +static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
>> +{
>> + return I2C_FUNC_I2C | I2C_FUNC_PROTOCOL_MANGLING | I2C_FUNC_10BIT_ADDR;
>> +}
>> +
>> +static const struct i2c_algorithm fsi_i2c_algorithm = {
>> + .master_xfer = fsi_i2c_xfer,
>> + .functionality = fsi_i2c_functionality,
>> +};
>> +
>> +static int fsi_i2c_probe(struct device *dev)
>> +{
>> + struct fsi_i2c_master *i2c;
>> + int rc;
>> + u32 clk;
>> +
>> + i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
>> + if (!i2c)
>> + return -ENOMEM;
>> +
>> + i2c->fsi = to_fsi_dev(dev);
>> + i2c->clk = I2C_DEFAULT_CLK;
>> +
>> + if (dev->of_node) {
>> + if (!of_property_read_u32(dev->of_node, "clock-frequency",
>> + &clk))
>> + i2c->clk = clk;
>> + }
>> +
>> + rc = fsi_i2c_dev_init(i2c);
>> + if (rc)
>> + return rc;
>> +
>> + dev_set_drvdata(dev, i2c);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct fsi_device_id fsi_i2c_ids[] = {
>> + { FSI_ENGID_I2C_FSI, FSI_VERSION_ANY },
>> + { 0 }
>> +};
>> +
>> +static struct fsi_driver fsi_i2c_driver = {
>> + .id_table = fsi_i2c_ids,
>> + .drv = {
>> + .name = "i2c_master_fsi",
>> + .bus = &fsi_bus_type,
>> + .probe = fsi_i2c_probe,
>> + },
>> +};
>> +
>> +static int fsi_i2c_init(void)
>> +{
>> + return fsi_driver_register(&fsi_i2c_driver);
>> +}
>> +
>> +static void fsi_i2c_exit(void)
>> +{
>> + fsi_driver_unregister(&fsi_i2c_driver);
>> +}
>> +
>> +module_init(fsi_i2c_init);
>> +module_exit(fsi_i2c_exit);
> Please use module_fsi_driver() instead.
Sure.
Thanks for the review, Joel!
Eddie
>
>
>> +
>> +MODULE_AUTHOR("Eddie James <eajames at us.ibm.com>");
>> +MODULE_DESCRIPTION("FSI attached I2C master");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.8.3.1
>>
More information about the openbmc
mailing list