* [PATCH v4 0/5] ARM: Add GXP I2C Support @ 2023-01-25 18:44 nick.hawkins 2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins ` (4 more replies) 0 siblings, 5 replies; 15+ messages in thread From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw) To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel From: Nick Hawkins <nick.hawkins@hpe.com> The GXP SoC supports 10 I2C engines. Each I2C engine is completely independent and can function both as an I2C master and I2C slave. The I2C master can operate in a multi master environment. The engines support a scalable speed from 8kHZ to 1.5 Mhz. --- Changes since v3: *Switch engine variable to u32 *Disable IRQ on device remove with register write instead *Provided even greater description with the use of Phandle Changes since v2: *Disable IRQ on a device remove *Remove use of I2C_CLASS_DEPRECATED *Use i2c_parse_fw_timings instead of of_property_read_u32 *Remove redundant dev_err_probe as platform_get_irq already has one *Used __iomem instead of res->start to find physical address *Use BIT in gxp_i2c_irq_handler *Made value u8 instead of u16 for u8 read *Provided a better description of Phandle in yaml Changes since v1: *Removed yaml documentation of hpe,gxp-sysreg as it has been applied to syscon.yaml *Made i2cX a generic node name i2c in dts file *Added status field to the dtsi and the dts for i2c bus *Removed unnecessary size-cells and address-cells from yaml *Removed phandle from hpe,sysreg-phandle *Changed hpe,i2c-max-bus-freq to clock-frequency *Removed rogue tab in structure definition *Removed use of __iomem *base local variables as it was unnecessary *Switched #if IS_ENABLED() -> if (IS_ENABLED()) inside functions *Removed use of pr_* functions *Removed informational prints in register and unregister functions *Removed print from interrupt handler *Removed informational prints from probe function *Switched dev_err -> dev_err_probe in probe function *Used the respective helper for mapping the resource to __iomem Nick Hawkins (5): i2c: hpe: Add GXP SoC I2C Controller dt-bindings: i2c: Add hpe,gxp-i2c ARM: dts: hpe: Add I2C Topology ARM: multi_v7_defconfig: add gxp i2c module MAINTAINERS: Add HPE GXP I2C Support .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml | 59 ++ MAINTAINERS | 2 + arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 109 ++++ arch/arm/boot/dts/hpe-gxp.dtsi | 125 ++++ arch/arm/configs/multi_v7_defconfig | 1 + drivers/i2c/busses/Kconfig | 7 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-gxp.c | 603 ++++++++++++++++++ 8 files changed, 907 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml create mode 100644 drivers/i2c/busses/i2c-gxp.c -- 2.17.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller 2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins @ 2023-01-25 18:44 ` nick.hawkins 2023-02-15 20:29 ` Wolfram Sang 2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw) To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel From: Nick Hawkins <nick.hawkins@hpe.com> The GXP SoC supports 10 I2C engines. Each I2C engine is completely independent and can function both as an I2C master and I2C slave. The I2C master can operate in a multi master environment. The engines support a scalable speed from 8kHZ to 1.5 Mhz. Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> Reviewed-by: Joel Stanley <joel@jms.id.au> --- v4: *Disable IRQ on device remove by register write *Switch engine variable to u32 v3: *Disable IRQ on device remove *Remove use of I2C_CLASS_DEPRECATED *Use i2c_parse_fw_timings instead of of_property_read_u32 *Remove redundant dev_err_probe as platform_get_irq already has one *Used __iomem instead of res->start to find physical address *Use BIT in gxp_i2c_irq_handler *Made value u8 instead of u16 v2: *Removed rogue tab in structure *Removed use of __iomem base local variables as it was unnecessary *Switched #if IS_ENABLED() -> if (IS_ENABLED()) inside functions *Removed use of pr_* functions *Removed informational prints in register and unregister functions *Removed print from interrupt handler *No informational prints in probe function *Switched dev_err for dev_err_probe in probe function *Use respective helper for mapping the resource to __iomem --- drivers/i2c/busses/Kconfig | 7 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-gxp.c | 603 +++++++++++++++++++++++++++++++++++ 3 files changed, 611 insertions(+) create mode 100644 drivers/i2c/busses/i2c-gxp.c diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index e50f9603d189..8b3951e0ca5c 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -1457,4 +1457,11 @@ config I2C_VIRTIO This driver can also be built as a module. If so, the module will be called i2c-virtio. +config I2C_GXP + tristate "GXP I2C Interface" + depends on ARCH_HPE_GXP || COMPILE_TEST + help + This enables support for GXP I2C interface. The I2C engines can be + either I2C master or I2C slaves. + endmenu diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index e73cdb1d2b5a..dcc96eab6d68 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -127,6 +127,7 @@ obj-$(CONFIG_I2C_THUNDERX) += i2c-thunderx.o obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o +obj-$(CONFIG_I2C_GXP) += i2c-gxp.o # External I2C/SMBus adapter drivers obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o diff --git a/drivers/i2c/busses/i2c-gxp.c b/drivers/i2c/busses/i2c-gxp.c new file mode 100644 index 000000000000..37272261ce8d --- /dev/null +++ b/drivers/i2c/busses/i2c-gxp.c @@ -0,0 +1,603 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */ + +#include <linux/err.h> +#include <linux/io.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> + +#define GXP_MAX_I2C_ENGINE 10 +static const char * const gxp_i2c_name[] = { + "gxp-i2c0", "gxp-i2c1", "gxp-i2c2", "gxp-i2c3", + "gxp-i2c4", "gxp-i2c5", "gxp-i2c6", "gxp-i2c7", + "gxp-i2c8", "gxp-i2c9" }; + +/* GXP I2C Global interrupt status/enable register*/ +#define GXP_I2CINTSTAT 0x00 +#define GXP_I2CINTEN 0x04 + +/* GXP I2C registers */ +#define GXP_I2CSTAT 0x00 +#define MASK_STOP_EVENT 0x20 +#define MASK_ACK 0x08 +#define MASK_RW 0x04 +#define GXP_I2CEVTERR 0x01 +#define MASK_SLAVE_CMD_EVENT 0x01 +#define MASK_SLAVE_DATA_EVENT 0x02 +#define MASK_MASTER_EVENT 0x10 +#define GXP_I2CSNPDAT 0x02 +#define GXP_I2CMCMD 0x04 +#define GXP_I2CSCMD 0x06 +#define GXP_I2CSNPAA 0x09 +#define GXP_I2CADVFEAT 0x0A +#define GXP_I2COWNADR 0x0B +#define GXP_I2CFREQDIV 0x0C +#define GXP_I2CFLTFAIR 0x0D +#define GXP_I2CTMOEDG 0x0E +#define GXP_I2CCYCTIM 0x0F + +static bool i2c_global_init_done; + +enum { + GXP_I2C_IDLE = 0, + GXP_I2C_ADDR_PHASE, + GXP_I2C_RDATA_PHASE, + GXP_I2C_WDATA_PHASE, + GXP_I2C_ADDR_NACK, + GXP_I2C_DATA_NACK, + GXP_I2C_ERROR, + GXP_I2C_COMP +}; + +struct gxp_i2c_drvdata { + struct device *dev; + void __iomem *base; + struct i2c_timings t; + u32 engine; + int irq; + struct completion completion; + struct i2c_adapter adapter; + struct i2c_msg *curr_msg; + int msgs_remaining; + int msgs_num; + u8 *buf; + size_t buf_remaining; + unsigned char state; + struct i2c_client *slave; + unsigned char stopped; +}; + +static struct regmap *i2cg_map; + +static void gxp_i2c_start(struct gxp_i2c_drvdata *drvdata) +{ + u16 value; + + drvdata->buf = drvdata->curr_msg->buf; + drvdata->buf_remaining = drvdata->curr_msg->len; + + /* Note: Address in struct i2c_msg is 7 bits */ + value = drvdata->curr_msg->addr << 9; + + if (drvdata->curr_msg->flags & I2C_M_RD) { + /* Read */ + value |= 0x05; + } else { + /* Write */ + value |= 0x01; + } + + drvdata->state = GXP_I2C_ADDR_PHASE; + writew(value, drvdata->base + GXP_I2CMCMD); +} + +static int gxp_i2c_master_xfer(struct i2c_adapter *adapter, + struct i2c_msg *msgs, int num) +{ + int ret; + struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(adapter); + unsigned long time_left; + + drvdata->msgs_remaining = num; + drvdata->curr_msg = msgs; + drvdata->msgs_num = num; + reinit_completion(&drvdata->completion); + + gxp_i2c_start(drvdata); + + time_left = wait_for_completion_timeout(&drvdata->completion, + adapter->timeout); + ret = num - drvdata->msgs_remaining; + if (time_left == 0) { + switch (drvdata->state) { + case GXP_I2C_WDATA_PHASE: + dev_err(drvdata->dev, + "gxp_i2c_start:write Data phase timeout at msg[%d]\n", + ret); + break; + case GXP_I2C_RDATA_PHASE: + dev_err(drvdata->dev, + "gxp_i2c_start:read Data phase timeout at msg[%d]\n", + ret); + break; + case GXP_I2C_ADDR_PHASE: + dev_err(drvdata->dev, + "gxp_i2c_start:Addr phase timeout\n"); + break; + default: + dev_err(drvdata->dev, + "gxp_i2c_start:i2c transfer timeout state=%d\n", + drvdata->state); + break; + } + return -ETIMEDOUT; + } + + if (drvdata->state == GXP_I2C_ADDR_NACK) { + dev_err(drvdata->dev, + "gxp_i2c_start:No ACK for address phase\n"); + return -EIO; + } else if (drvdata->state == GXP_I2C_DATA_NACK) { + dev_err(drvdata->dev, "gxp_i2c_start:No ACK for data phase\n"); + return -EIO; + } + + return ret; +} + +static u32 gxp_i2c_func(struct i2c_adapter *adap) +{ + if (IS_ENABLED(CONFIG_I2C_SLAVE)) + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SLAVE; + + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; +} + +#if IS_ENABLED(CONFIG_I2C_SLAVE) +static int gxp_i2c_reg_slave(struct i2c_client *slave) +{ + struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(slave->adapter); + + if (drvdata->slave) + return -EBUSY; + + if (slave->flags & I2C_CLIENT_TEN) + return -EAFNOSUPPORT; + + drvdata->slave = slave; + + writeb(slave->addr << 1, drvdata->base + GXP_I2COWNADR); + writeb(0x69, drvdata->base + GXP_I2CSCMD); + + return 0; +} + +static int gxp_i2c_unreg_slave(struct i2c_client *slave) +{ + struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(slave->adapter); + + WARN_ON(!drvdata->slave); + + writeb(0x00, drvdata->base + GXP_I2COWNADR); + writeb(0xF0, drvdata->base + GXP_I2CSCMD); + + drvdata->slave = NULL; + + return 0; +} +#endif + +static const struct i2c_algorithm gxp_i2c_algo = { + .master_xfer = gxp_i2c_master_xfer, + .functionality = gxp_i2c_func, +#if IS_ENABLED(CONFIG_I2C_SLAVE) + .reg_slave = gxp_i2c_reg_slave, + .unreg_slave = gxp_i2c_unreg_slave, +#endif +}; + +static void gxp_i2c_stop(struct gxp_i2c_drvdata *drvdata) +{ + /* Clear event and send stop */ + writeb(0x82, drvdata->base + GXP_I2CMCMD); + + complete(&drvdata->completion); +} + +static void gxp_i2c_restart(struct gxp_i2c_drvdata *drvdata) +{ + u16 value; + + drvdata->buf = drvdata->curr_msg->buf; + drvdata->buf_remaining = drvdata->curr_msg->len; + + value = drvdata->curr_msg->addr << 9; + + if (drvdata->curr_msg->flags & I2C_M_RD) { + /* Read and clear master event */ + value |= 0x85; + } else { + /* Write and clear master event */ + value |= 0x81; + } + + drvdata->state = GXP_I2C_ADDR_PHASE; + + writew(value, drvdata->base + GXP_I2CMCMD); +} + +static void gxp_i2c_chk_addr_ack(struct gxp_i2c_drvdata *drvdata) +{ + u16 value; + + value = readb(drvdata->base + GXP_I2CSTAT); + if (!(value & MASK_ACK)) { + /* Got no ack, stop */ + drvdata->state = GXP_I2C_ADDR_NACK; + gxp_i2c_stop(drvdata); + return; + } + + if (drvdata->curr_msg->flags & I2C_M_RD) { + /* Start to read data from slave */ + if (drvdata->buf_remaining == 0) { + /* No more data to read, stop */ + drvdata->msgs_remaining--; + drvdata->state = GXP_I2C_COMP; + gxp_i2c_stop(drvdata); + return; + } + drvdata->state = GXP_I2C_RDATA_PHASE; + + if (drvdata->buf_remaining == 1) { + /* The last data, do not ack */ + writeb(0x84, drvdata->base + GXP_I2CMCMD); + } else { + /* Read data and ack it */ + writeb(0x8C, drvdata->base + GXP_I2CMCMD); + } + } else { + /* Start to write first data to slave */ + if (drvdata->buf_remaining == 0) { + /* No more data to write, stop */ + drvdata->msgs_remaining--; + drvdata->state = GXP_I2C_COMP; + gxp_i2c_stop(drvdata); + return; + } + value = *drvdata->buf; + value = value << 8; + /* Clear master event */ + value |= 0x80; + drvdata->buf++; + drvdata->buf_remaining--; + drvdata->state = GXP_I2C_WDATA_PHASE; + writew(value, drvdata->base + GXP_I2CMCMD); + } +} + +static void gxp_i2c_ack_data(struct gxp_i2c_drvdata *drvdata) +{ + u8 value; + + /* Store the data returned */ + value = readb(drvdata->base + GXP_I2CSNPDAT); + *drvdata->buf = value; + drvdata->buf++; + drvdata->buf_remaining--; + + if (drvdata->buf_remaining == 0) { + /* No more data, this message is completed. */ + drvdata->msgs_remaining--; + + if (drvdata->msgs_remaining == 0) { + /* No more messages, stop */ + drvdata->state = GXP_I2C_COMP; + gxp_i2c_stop(drvdata); + return; + } + /* Move to next message and start transfer */ + drvdata->curr_msg++; + gxp_i2c_restart(drvdata); + return; + } + + /* Ack the slave to make it send next byte */ + drvdata->state = GXP_I2C_RDATA_PHASE; + if (drvdata->buf_remaining == 1) { + /* The last data, do not ack */ + writeb(0x84, drvdata->base + GXP_I2CMCMD); + } else { + /* Read data and ack it */ + writeb(0x8C, drvdata->base + GXP_I2CMCMD); + } +} + +static void gxp_i2c_chk_data_ack(struct gxp_i2c_drvdata *drvdata) +{ + u16 value; + + value = readb(drvdata->base + GXP_I2CSTAT); + if (!(value & MASK_ACK)) { + /* Received No ack, stop */ + drvdata->state = GXP_I2C_DATA_NACK; + gxp_i2c_stop(drvdata); + return; + } + + /* Got ack, check if there is more data to write */ + if (drvdata->buf_remaining == 0) { + /* No more data, this message is completed */ + drvdata->msgs_remaining--; + + if (drvdata->msgs_remaining == 0) { + /* No more messages, stop */ + drvdata->state = GXP_I2C_COMP; + gxp_i2c_stop(drvdata); + return; + } + /* Move to next message and start transfer */ + drvdata->curr_msg++; + gxp_i2c_restart(drvdata); + return; + } + + /* Write data to slave */ + value = *drvdata->buf; + value = value << 8; + + /* Clear master event */ + value |= 0x80; + drvdata->buf++; + drvdata->buf_remaining--; + drvdata->state = GXP_I2C_WDATA_PHASE; + writew(value, drvdata->base + GXP_I2CMCMD); +} + +#if IS_ENABLED(CONFIG_I2C_SLAVE) +static bool gxp_i2c_slave_irq_handler(struct gxp_i2c_drvdata *drvdata) +{ + u8 value; + u8 buf; + int ret; + + value = readb(drvdata->base + GXP_I2CEVTERR); + + /* Received start or stop event */ + if (value & MASK_SLAVE_CMD_EVENT) { + value = readb(drvdata->base + GXP_I2CSTAT); + /* Master sent stop */ + if (value & MASK_STOP_EVENT) { + if (drvdata->stopped == 0) + i2c_slave_event(drvdata->slave, I2C_SLAVE_STOP, &buf); + writeb(0x69, drvdata->base + GXP_I2CSCMD); + drvdata->stopped = 1; + } else { + /* Master sent start and wants to read */ + drvdata->stopped = 0; + if (value & MASK_RW) { + i2c_slave_event(drvdata->slave, + I2C_SLAVE_READ_REQUESTED, &buf); + value = buf << 8 | 0x61; + writew(value, drvdata->base + GXP_I2CSCMD); + } else { + /* Master wants to write to us */ + ret = i2c_slave_event(drvdata->slave, + I2C_SLAVE_WRITE_REQUESTED, &buf); + if (!ret) { + /* Ack next byte from master */ + writeb(0x69, drvdata->base + GXP_I2CSCMD); + } else { + /* Nack next byte from master */ + writeb(0x61, drvdata->base + GXP_I2CSCMD); + } + } + } + } else if (value & MASK_SLAVE_DATA_EVENT) { + value = readb(drvdata->base + GXP_I2CSTAT); + /* Master wants to read */ + if (value & MASK_RW) { + /* Master wants another byte */ + if (value & MASK_ACK) { + i2c_slave_event(drvdata->slave, + I2C_SLAVE_READ_PROCESSED, &buf); + value = buf << 8 | 0x61; + writew(value, drvdata->base + GXP_I2CSCMD); + } else { + /* No more bytes needed */ + writew(0x69, drvdata->base + GXP_I2CSCMD); + } + } else { + /* Master wants to write to us */ + value = readb(drvdata->base + GXP_I2CSNPDAT); + buf = (uint8_t)value; + ret = i2c_slave_event(drvdata->slave, + I2C_SLAVE_WRITE_RECEIVED, &buf); + if (!ret) { + /* Ack next byte from master */ + writeb(0x69, drvdata->base + GXP_I2CSCMD); + } else { + /* Nack next byte from master */ + writeb(0x61, drvdata->base + GXP_I2CSCMD); + } + } + } else { + return false; + } + + return true; +} +#endif + +static irqreturn_t gxp_i2c_irq_handler(int irq, void *_drvdata) +{ + struct gxp_i2c_drvdata *drvdata = (struct gxp_i2c_drvdata *)_drvdata; + u32 value; + + /* Check if the interrupt is for the current engine */ + regmap_read(i2cg_map, GXP_I2CINTSTAT, &value); + if (!(value & BIT(drvdata->engine))) + return IRQ_NONE; + + value = readb(drvdata->base + GXP_I2CEVTERR); + + /* Error */ + if (value & ~(MASK_MASTER_EVENT | MASK_SLAVE_CMD_EVENT | + MASK_SLAVE_DATA_EVENT)) { + /* Clear all events */ + writeb(0x00, drvdata->base + GXP_I2CEVTERR); + drvdata->state = GXP_I2C_ERROR; + gxp_i2c_stop(drvdata); + return IRQ_HANDLED; + } + + if (IS_ENABLED(CONFIG_I2C_SLAVE)) { + /* Slave mode */ + if (value & (MASK_SLAVE_CMD_EVENT | MASK_SLAVE_DATA_EVENT)) { + if (gxp_i2c_slave_irq_handler(drvdata)) + return IRQ_HANDLED; + return IRQ_NONE; + } + } + + /* Master mode */ + switch (drvdata->state) { + case GXP_I2C_ADDR_PHASE: + gxp_i2c_chk_addr_ack(drvdata); + break; + + case GXP_I2C_RDATA_PHASE: + gxp_i2c_ack_data(drvdata); + break; + + case GXP_I2C_WDATA_PHASE: + gxp_i2c_chk_data_ack(drvdata); + break; + } + + return IRQ_HANDLED; +} + +static void gxp_i2c_init(struct gxp_i2c_drvdata *drvdata) +{ + drvdata->state = GXP_I2C_IDLE; + writeb(2000000 / drvdata->t.bus_freq_hz, + drvdata->base + GXP_I2CFREQDIV); + writeb(0x32, drvdata->base + GXP_I2CFLTFAIR); + writeb(0x0a, drvdata->base + GXP_I2CTMOEDG); + writeb(0x00, drvdata->base + GXP_I2CCYCTIM); + writeb(0x00, drvdata->base + GXP_I2CSNPAA); + writeb(0x00, drvdata->base + GXP_I2CADVFEAT); + writeb(0xF0, drvdata->base + GXP_I2CSCMD); + writeb(0x80, drvdata->base + GXP_I2CMCMD); + writeb(0x00, drvdata->base + GXP_I2CEVTERR); + writeb(0x00, drvdata->base + GXP_I2COWNADR); +} + +static int gxp_i2c_probe(struct platform_device *pdev) +{ + struct gxp_i2c_drvdata *drvdata; + int rc; + struct i2c_adapter *adapter; + + if (!i2c_global_init_done) { + i2cg_map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, + "hpe,sysreg"); + if (IS_ERR(i2cg_map)) { + return dev_err_probe(&pdev->dev, IS_ERR(i2cg_map), + "failed to map i2cg_handle\n"); + } + + /* Disable interrupt */ + regmap_update_bits(i2cg_map, GXP_I2CINTEN, 0x00000FFF, 0); + i2c_global_init_done = true; + } + + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), + GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + platform_set_drvdata(pdev, drvdata); + drvdata->dev = &pdev->dev; + init_completion(&drvdata->completion); + + drvdata->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(drvdata->base)) + return PTR_ERR(drvdata->base); + + /* Use physical memory address to determine which I2C engine this is. */ + drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; + + if (drvdata->engine >= GXP_MAX_I2C_ENGINE) { + return dev_err_probe(&pdev->dev, -EINVAL, "i2c engine% is unsupported\n", + drvdata->engine); + } + + rc = platform_get_irq(pdev, 0); + if (rc < 0) + return rc; + + drvdata->irq = rc; + rc = devm_request_irq(&pdev->dev, drvdata->irq, gxp_i2c_irq_handler, + IRQF_SHARED, gxp_i2c_name[drvdata->engine], drvdata); + if (rc < 0) + return dev_err_probe(&pdev->dev, rc, "irq request failed\n"); + + i2c_parse_fw_timings(&pdev->dev, &drvdata->t, true); + + gxp_i2c_init(drvdata); + + /* Enable interrupt */ + regmap_update_bits(i2cg_map, GXP_I2CINTEN, BIT(drvdata->engine), + BIT(drvdata->engine)); + + adapter = &drvdata->adapter; + i2c_set_adapdata(adapter, drvdata); + + adapter->owner = THIS_MODULE; + strscpy(adapter->name, "HPE GXP I2C adapter", sizeof(adapter->name)); + adapter->algo = &gxp_i2c_algo; + adapter->dev.parent = &pdev->dev; + adapter->dev.of_node = pdev->dev.of_node; + + rc = i2c_add_adapter(adapter); + if (rc) + return dev_err_probe(&pdev->dev, rc, "i2c add adapter failed\n"); + + return 0; +} + +static int gxp_i2c_remove(struct platform_device *pdev) +{ + struct gxp_i2c_drvdata *drvdata = platform_get_drvdata(pdev); + + /* Disable interrupt */ + regmap_update_bits(i2cg_map, GXP_I2CINTEN, BIT(drvdata->engine), 0); + i2c_del_adapter(&drvdata->adapter); + + return 0; +} + +static const struct of_device_id gxp_i2c_of_match[] = { + { .compatible = "hpe,gxp-i2c" }, + {}, +}; +MODULE_DEVICE_TABLE(of, gxp_i2c_of_match); + +static struct platform_driver gxp_i2c_driver = { + .probe = gxp_i2c_probe, + .remove = gxp_i2c_remove, + .driver = { + .name = "gxp-i2c", + .of_match_table = gxp_i2c_of_match, + }, +}; +module_platform_driver(gxp_i2c_driver); + +MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>"); +MODULE_DESCRIPTION("HPE GXP I2C bus driver"); +MODULE_LICENSE("GPL"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller 2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins @ 2023-02-15 20:29 ` Wolfram Sang 2023-02-16 20:02 ` Hawkins, Nick 0 siblings, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2023-02-15 20:29 UTC (permalink / raw) To: nick.hawkins Cc: verdun, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel [-- Attachment #1: Type: text/plain, Size: 7270 bytes --] Hi Nick, > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index e50f9603d189..8b3951e0ca5c 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -1457,4 +1457,11 @@ config I2C_VIRTIO > This driver can also be built as a module. If so, the module > will be called i2c-virtio. > > +config I2C_GXP > + tristate "GXP I2C Interface" > + depends on ARCH_HPE_GXP || COMPILE_TEST > + help > + This enables support for GXP I2C interface. The I2C engines can be > + either I2C master or I2C slaves. > + Shouldn't this be in the "I2C system bus drivers (mostly embedded / system-on-chip)" section? (alphabetically sorted) > +static bool i2c_global_init_done; Can't we avoid this static by checking if i2cg_map is NULL/non-NULL? > + > +enum { > + GXP_I2C_IDLE = 0, > + GXP_I2C_ADDR_PHASE, > + GXP_I2C_RDATA_PHASE, > + GXP_I2C_WDATA_PHASE, > + GXP_I2C_ADDR_NACK, > + GXP_I2C_DATA_NACK, > + GXP_I2C_ERROR, > + GXP_I2C_COMP > +}; > + > +struct gxp_i2c_drvdata { > + struct device *dev; > + void __iomem *base; > + struct i2c_timings t; > + u32 engine; > + int irq; > + struct completion completion; > + struct i2c_adapter adapter; > + struct i2c_msg *curr_msg; > + int msgs_remaining; > + int msgs_num; > + u8 *buf; > + size_t buf_remaining; > + unsigned char state; > + struct i2c_client *slave; > + unsigned char stopped; > +}; > + > +static struct regmap *i2cg_map; > + > +static void gxp_i2c_start(struct gxp_i2c_drvdata *drvdata) > +{ > + u16 value; > + > + drvdata->buf = drvdata->curr_msg->buf; > + drvdata->buf_remaining = drvdata->curr_msg->len; > + > + /* Note: Address in struct i2c_msg is 7 bits */ > + value = drvdata->curr_msg->addr << 9; > + > + if (drvdata->curr_msg->flags & I2C_M_RD) { > + /* Read */ > + value |= 0x05; > + } else { > + /* Write */ > + value |= 0x01; > + } I'd write it more concise as: value |= drvdata->curr_msg->flags & I2C_M_RD ? 0x05 : 0x01; But this is a matter of taste. > + > + drvdata->state = GXP_I2C_ADDR_PHASE; > + writew(value, drvdata->base + GXP_I2CMCMD); > +} > + > +static int gxp_i2c_master_xfer(struct i2c_adapter *adapter, > + struct i2c_msg *msgs, int num) > +{ > + int ret; > + struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(adapter); > + unsigned long time_left; > + > + drvdata->msgs_remaining = num; > + drvdata->curr_msg = msgs; > + drvdata->msgs_num = num; > + reinit_completion(&drvdata->completion); > + > + gxp_i2c_start(drvdata); > + > + time_left = wait_for_completion_timeout(&drvdata->completion, > + adapter->timeout); > + ret = num - drvdata->msgs_remaining; > + if (time_left == 0) { > + switch (drvdata->state) { > + case GXP_I2C_WDATA_PHASE: > + dev_err(drvdata->dev, > + "gxp_i2c_start:write Data phase timeout at msg[%d]\n", > + ret); Please don't write error message to the log on timeouts. They can happen. Think of an EEPROM which is busy because it needs to erase a page. Upper layers need to handle this correctly, the user doesn't need to know about it. > + break; > + case GXP_I2C_RDATA_PHASE: > + dev_err(drvdata->dev, > + "gxp_i2c_start:read Data phase timeout at msg[%d]\n", > + ret); > + break; > + case GXP_I2C_ADDR_PHASE: > + dev_err(drvdata->dev, > + "gxp_i2c_start:Addr phase timeout\n"); > + break; > + default: > + dev_err(drvdata->dev, > + "gxp_i2c_start:i2c transfer timeout state=%d\n", > + drvdata->state); > + break; > + } > + return -ETIMEDOUT; > + } > + > + if (drvdata->state == GXP_I2C_ADDR_NACK) { > + dev_err(drvdata->dev, > + "gxp_i2c_start:No ACK for address phase\n"); > + return -EIO; > + } else if (drvdata->state == GXP_I2C_DATA_NACK) { > + dev_err(drvdata->dev, "gxp_i2c_start:No ACK for data phase\n"); > + return -EIO; Same here for NACK. Otherwise i2cdetect will flood your logfile. > + } > + > + return ret; > +} > + > +static u32 gxp_i2c_func(struct i2c_adapter *adap) > +{ > + if (IS_ENABLED(CONFIG_I2C_SLAVE)) > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SLAVE; > + > + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; > +} > + > +#if IS_ENABLED(CONFIG_I2C_SLAVE) > +static int gxp_i2c_reg_slave(struct i2c_client *slave) > +{ > + struct gxp_i2c_drvdata *drvdata = i2c_get_adapdata(slave->adapter); > + > + if (drvdata->slave) > + return -EBUSY; > + > + if (slave->flags & I2C_CLIENT_TEN) > + return -EAFNOSUPPORT; > + > + drvdata->slave = slave; > + > + writeb(slave->addr << 1, drvdata->base + GXP_I2COWNADR); > + writeb(0x69, drvdata->base + GXP_I2CSCMD); Does it make sense to have #defines for the magic values for I2CSCMD? BTW, is the datasheet public? ... > +static void gxp_i2c_init(struct gxp_i2c_drvdata *drvdata) > +{ > + drvdata->state = GXP_I2C_IDLE; > + writeb(2000000 / drvdata->t.bus_freq_hz, > + drvdata->base + GXP_I2CFREQDIV); > + writeb(0x32, drvdata->base + GXP_I2CFLTFAIR); > + writeb(0x0a, drvdata->base + GXP_I2CTMOEDG); > + writeb(0x00, drvdata->base + GXP_I2CCYCTIM); > + writeb(0x00, drvdata->base + GXP_I2CSNPAA); > + writeb(0x00, drvdata->base + GXP_I2CADVFEAT); > + writeb(0xF0, drvdata->base + GXP_I2CSCMD); > + writeb(0x80, drvdata->base + GXP_I2CMCMD); > + writeb(0x00, drvdata->base + GXP_I2CEVTERR); > + writeb(0x00, drvdata->base + GXP_I2COWNADR); Also here, lots of magic values. Can we do something about it? > +} > + > +static int gxp_i2c_probe(struct platform_device *pdev) > +{ > + struct gxp_i2c_drvdata *drvdata; > + int rc; > + struct i2c_adapter *adapter; > + > + if (!i2c_global_init_done) { > + i2cg_map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > + "hpe,sysreg"); > + if (IS_ERR(i2cg_map)) { > + return dev_err_probe(&pdev->dev, IS_ERR(i2cg_map), > + "failed to map i2cg_handle\n"); > + } > + > + /* Disable interrupt */ > + regmap_update_bits(i2cg_map, GXP_I2CINTEN, 0x00000FFF, 0); > + i2c_global_init_done = true; > + } > + > + drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), > + GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, drvdata); > + drvdata->dev = &pdev->dev; > + init_completion(&drvdata->completion); > + > + drvdata->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(drvdata->base)) > + return PTR_ERR(drvdata->base); > + > + /* Use physical memory address to determine which I2C engine this is. */ > + drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; This breaks on my 64-bit test-build, so it will also fail with COMPILE_TEST. drivers/i2c/busses/i2c-gxp.c: In function ‘gxp_i2c_probe’: drivers/i2c/busses/i2c-gxp.c:533:28: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] 533 | drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; The rest looks good to me. Except for removing the hardcoded values, the other fixes should be fairly simple, I think. So, hardcoded values could be changed incrementally maybe. If this is possible at all. I still plan to have this driver in 6.3. Happy hacking, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller 2023-02-15 20:29 ` Wolfram Sang @ 2023-02-16 20:02 ` Hawkins, Nick 2023-02-16 20:47 ` Wolfram Sang 0 siblings, 1 reply; 15+ messages in thread From: Hawkins, Nick @ 2023-02-16 20:02 UTC (permalink / raw) To: Wolfram Sang Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel > Does it make sense to have #defines for the magic values for I2CSCMD? > BTW, is the datasheet public? I will work on the defines and get rid of all the magic values. Unfortunately, there is no public spec available currently. Hopefully, we will have one someday though. > > + /* Use physical memory address to determine which I2C engine this is. */ > > + drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; > This breaks on my 64-bit test-build, so it will also fail with > COMPILE_TEST. > drivers/i2c/busses/i2c-gxp.c: In function ‘gxp_i2c_probe’: > drivers/i2c/busses/i2c-gxp.c:533:28: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] > 533 | drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; I am currently unable to reproduce this error. I even set W=2. Would replacing (u32) with (unsigned long) resolve it? Thanks, -Nick Hawkins ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller 2023-02-16 20:02 ` Hawkins, Nick @ 2023-02-16 20:47 ` Wolfram Sang 0 siblings, 0 replies; 15+ messages in thread From: Wolfram Sang @ 2023-02-16 20:47 UTC (permalink / raw) To: Hawkins, Nick Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel [-- Attachment #1: Type: text/plain, Size: 770 bytes --] Hi Nick, > I will work on the defines and get rid of all the magic values. Cool, thank you! > Unfortunately, there is no public spec available currently. > Hopefully, we will have one someday though. Then, the #defines are even more helpful! > > drivers/i2c/busses/i2c-gxp.c: In function ‘gxp_i2c_probe’: > > drivers/i2c/busses/i2c-gxp.c:533:28: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] > > 533 | drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; > > I am currently unable to reproduce this error. I even set W=2. Interesting. I have this version: gcc (Debian 10.2.1-6) 10.2.1 20210110 > Would replacing (u32) with (unsigned long) resolve it? Or size_t. Happy hacking, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c 2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins 2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins @ 2023-01-25 18:44 ` nick.hawkins 2023-01-25 21:18 ` Rob Herring 2023-01-25 18:44 ` [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology nick.hawkins ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw) To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel From: Nick Hawkins <nick.hawkins@hpe.com> Document compatibility string to support I2C controller in GXP. Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> --- v4: *Provide even greater description with the use of Phandle *Reorder properties so they match the required order v3: *Provide better description with use of Phandle v2: *Removed uneccessary size-cells and address-cells *Removed phandle from hpe,sysreg-phandle *Changed hpe,i2c-max-bus-freq to clock-frequency --- .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml diff --git a/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml new file mode 100644 index 000000000000..6604dcd47251 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i2c/hpe,gxp-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HPE GXP SoC I2C Controller + +maintainers: + - Nick Hawkins <nick.hawkins@hpe.com> + +allOf: + - $ref: /schemas/i2c/i2c-controller.yaml# + +properties: + compatible: + const: hpe,gxp-i2c + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clock-frequency: + default: 100000 + + hpe,sysreg: + $ref: /schemas/types.yaml#/definitions/phandle + description: + Phandle to the global status and enable interrupt registers shared + between each I2C engine controller instance. It enables the I2C + engine controller to act as both a master or slave by being able to + arm and respond to interrupts from its engine. Each bit in the + registers represent the respective bit position. + +required: + - compatible + - reg + - interrupts + +unevaluatedProperties: false + +examples: + - | + i2c@2600 { + compatible = "hpe,gxp-i2c"; + reg = <0x2500 0x70>; + interrupts = <9>; + #address-cells = <1>; + #size-cells = <0>; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <10000>; + + eeprom@50 { + compatible = "atmel,24c128"; + reg = <0x50>; + }; + }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c 2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins @ 2023-01-25 21:18 ` Rob Herring 2023-01-25 21:31 ` Hawkins, Nick 0 siblings, 1 reply; 15+ messages in thread From: Rob Herring @ 2023-01-25 21:18 UTC (permalink / raw) To: nick.hawkins Cc: verdun, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel On Wed, Jan 25, 2023 at 12:44:35PM -0600, nick.hawkins@hpe.com wrote: > From: Nick Hawkins <nick.hawkins@hpe.com> > > Document compatibility string to support I2C controller > in GXP. > > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> > > --- > v4: > *Provide even greater description with the use > of Phandle > *Reorder properties so they match the required > order > v3: > *Provide better description with use of Phandle > v2: > *Removed uneccessary size-cells and address-cells > *Removed phandle from hpe,sysreg-phandle > *Changed hpe,i2c-max-bus-freq to clock-frequency > --- > .../devicetree/bindings/i2c/hpe,gxp-i2c.yaml | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml > > diff --git a/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml > new file mode 100644 > index 000000000000..6604dcd47251 > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml > @@ -0,0 +1,59 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/i2c/hpe,gxp-i2c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: HPE GXP SoC I2C Controller > + > +maintainers: > + - Nick Hawkins <nick.hawkins@hpe.com> > + > +allOf: > + - $ref: /schemas/i2c/i2c-controller.yaml# > + > +properties: > + compatible: > + const: hpe,gxp-i2c > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clock-frequency: > + default: 100000 > + > + hpe,sysreg: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Phandle to the global status and enable interrupt registers shared > + between each I2C engine controller instance. It enables the I2C > + engine controller to act as both a master or slave by being able to > + arm and respond to interrupts from its engine. Each bit in the > + registers represent the respective bit position. Each bit represents the bit position? AIUI, each I2C instance has a bit in it needs to control. How does the driver know what instance (and therefore the correct bit)? Typically you would have a 2nd cell here with that information. > + > +required: > + - compatible > + - reg > + - interrupts > + > +unevaluatedProperties: false > + > +examples: > + - | > + i2c@2600 { > + compatible = "hpe,gxp-i2c"; > + reg = <0x2500 0x70>; > + interrupts = <9>; > + #address-cells = <1>; > + #size-cells = <0>; > + hpe,sysreg = <&sysreg_system_controller>; > + clock-frequency = <10000>; > + > + eeprom@50 { > + compatible = "atmel,24c128"; > + reg = <0x50>; > + }; > + }; > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c 2023-01-25 21:18 ` Rob Herring @ 2023-01-25 21:31 ` Hawkins, Nick 2023-01-26 13:38 ` Rob Herring 0 siblings, 1 reply; 15+ messages in thread From: Hawkins, Nick @ 2023-01-25 21:31 UTC (permalink / raw) To: Rob Herring Cc: Verdun, Jean-Marie, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel > > + hpe,sysreg: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + Phandle to the global status and enable interrupt registers shared > > + between each I2C engine controller instance. It enables the I2C > > + engine controller to act as both a master or slave by being able to > > + arm and respond to interrupts from its engine. Each bit in the > > + registers represent the respective bit position. > Each bit represents the bit position? Yes what I mean here is that bit 0 represents engine 0, bit 1 represents engine 1 and so on. I will reword this how you have below. > AIUI, each I2C instance has a bit in it needs to control. How does the > driver know what instance (and therefore the correct bit)? Typically you > would have a 2nd cell here with that information. We are currently using the memory area designated reg to determine which engine we are on. Here is a snippet from patch 1 of this patchset that introduces the driver: /* Use physical memory address to determine which I2C engine this is. */ + drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; This works because each engine is 0x100 apart. I would however like to conform to a standard to designate the engine. Is there an existing property I can leverage? Thanks for your feedback, -Nick Hawkins ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c 2023-01-25 21:31 ` Hawkins, Nick @ 2023-01-26 13:38 ` Rob Herring 0 siblings, 0 replies; 15+ messages in thread From: Rob Herring @ 2023-01-26 13:38 UTC (permalink / raw) To: Hawkins, Nick Cc: Verdun, Jean-Marie, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel On Wed, Jan 25, 2023 at 3:32 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote: > > > > + hpe,sysreg: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: > > > + Phandle to the global status and enable interrupt registers shared > > > + between each I2C engine controller instance. It enables the I2C > > > + engine controller to act as both a master or slave by being able to > > > + arm and respond to interrupts from its engine. Each bit in the > > > + registers represent the respective bit position. > > > > Each bit represents the bit position? > > Yes what I mean here is that bit 0 represents engine 0, bit 1 represents > engine 1 and so on. I will reword this how you have below. > > > AIUI, each I2C instance has a bit in it needs to control. How does the > > driver know what instance (and therefore the correct bit)? Typically you > > would have a 2nd cell here with that information. > > We are currently using the memory area designated reg to determine > which engine we are on. > > Here is a snippet from patch 1 of this patchset that introduces the driver: > /* Use physical memory address to determine which I2C engine this is. */ > + drvdata->engine = ((u32)drvdata->base & 0xf00) >> 8; > > This works because each engine is 0x100 apart. Ah, that works fine then. Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology 2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins 2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins 2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins @ 2023-01-25 18:44 ` nick.hawkins 2023-01-25 18:44 ` [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module nick.hawkins 2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins 4 siblings, 0 replies; 15+ messages in thread From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw) To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel From: Nick Hawkins <nick.hawkins@hpe.com> Add 9 I2C Engines, 2 MUXs, and a EEPROM to the device tree. Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> --- v4: *No change v3: *No change v2: *Made i2cX a generic node name i2c in the dts *Added status field to dtsi and dts for i2c bus --- arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 109 ++++++++++++++++++++ arch/arm/boot/dts/hpe-gxp.dtsi | 125 +++++++++++++++++++++++ 2 files changed, 234 insertions(+) diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts index 3a7382ce40ef..1f2547fe9ae3 100644 --- a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts @@ -23,4 +23,113 @@ device_type = "memory"; reg = <0x40000000 0x20000000>; }; + + i2cmux@4 { + compatible = "i2c-mux-reg"; + i2c-parent = <&i2c4>; + reg = <0xd1000074 0x1>; + #address-cells = <1>; + #size-cells = <0>; + + i2c@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@3 { + reg = <3>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@4 { + reg = <4>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; + + i2cmux@6 { + compatible = "i2c-mux-reg"; + i2c-parent = <&i2c6>; + reg = <0xd1000076 0x1>; + #address-cells = <1>; + #size-cells = <0>; + + i2c@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@2 { + reg = <2>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@3 { + reg = <3>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@4 { + reg = <4>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@5 { + reg = <5>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; +}; + +&i2c0 { + status = "okay"; +}; + +&i2c1 { + status = "okay"; +}; + +&i2c2 { + status = "okay"; + eeprom@50 { + compatible = "atmel,24c02"; + pagesize = <8>; + reg = <0x50>; + }; +}; + +&i2c3 { + status = "okay"; +}; + +&i2c4 { + status = "okay"; +}; + +&i2c5 { + status = "okay"; +}; + +&i2c6 { + status = "okay"; +}; + +&i2c7 { + status = "okay"; +}; + +&i2c8 { + status = "okay"; +}; + +&i2c9 { + status = "okay"; }; diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi index cf735b3c4f35..3bc071149bae 100644 --- a/arch/arm/boot/dts/hpe-gxp.dtsi +++ b/arch/arm/boot/dts/hpe-gxp.dtsi @@ -122,6 +122,131 @@ interrupts = <6>; interrupt-parent = <&vic0>; }; + + sysreg_system_controller: syscon@f8 { + compatible = "hpe,gxp-sysreg", "syscon"; + reg = <0xf8 0x8>; + }; + + i2c0: i2c@2000 { + compatible = "hpe,gxp-i2c"; + reg = <0x2000 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c1: i2c@2100 { + compatible = "hpe,gxp-i2c"; + reg = <0x2100 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c2: i2c@2200 { + compatible = "hpe,gxp-i2c"; + reg = <0x2200 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c3: i2c@2300 { + compatible = "hpe,gxp-i2c"; + reg = <0x2300 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c4: i2c@2400 { + compatible = "hpe,gxp-i2c"; + reg = <0x2400 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c5: i2c@2500 { + compatible = "hpe,gxp-i2c"; + reg = <0x2500 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c6: i2c@2600 { + compatible = "hpe,gxp-i2c"; + reg = <0x2600 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c7: i2c@2700 { + compatible = "hpe,gxp-i2c"; + reg = <0x2700 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c8: i2c@2800 { + compatible = "hpe,gxp-i2c"; + reg = <0x2800 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; + + i2c9: i2c@2900 { + compatible = "hpe,gxp-i2c"; + reg = <0x2900 0x70>; + interrupts = <9>; + interrupt-parent = <&vic0>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + hpe,sysreg = <&sysreg_system_controller>; + clock-frequency = <100000>; + }; }; }; }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module 2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins ` (2 preceding siblings ...) 2023-01-25 18:44 ` [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology nick.hawkins @ 2023-01-25 18:44 ` nick.hawkins 2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins 4 siblings, 0 replies; 15+ messages in thread From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw) To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel From: Nick Hawkins <nick.hawkins@hpe.com> Add the CONFIG_I2C_GXP symbol to enable the GXP SoC I2C capabilities. Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> --- v4: *No change v3: *No change v2: *No change --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index b61b2e3d116b..8a19b1dc10d0 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -411,6 +411,7 @@ CONFIG_I2C_DAVINCI=y CONFIG_I2C_DESIGNWARE_PLATFORM=y CONFIG_I2C_DIGICOLOR=m CONFIG_I2C_EMEV2=m +CONFIG_I2C_GXP=m CONFIG_I2C_IMX=y CONFIG_I2C_MESON=y CONFIG_I2C_MV64XXX=y -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support 2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins ` (3 preceding siblings ...) 2023-01-25 18:44 ` [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module nick.hawkins @ 2023-01-25 18:44 ` nick.hawkins 2023-01-28 18:38 ` Wolfram Sang 4 siblings, 1 reply; 15+ messages in thread From: nick.hawkins @ 2023-01-25 18:44 UTC (permalink / raw) To: verdun, nick.hawkins, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel From: Nick Hawkins <nick.hawkins@hpe.com> Add the I2C controller source and bindings. Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> --- v4: *No change v3: *No change v2: *No change --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 1daadaa4d48b..d671a8b6968e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2217,12 +2217,14 @@ M: Jean-Marie Verdun <verdun@hpe.com> M: Nick Hawkins <nick.hawkins@hpe.com> S: Maintained F: Documentation/devicetree/bindings/arm/hpe,gxp.yaml +F: Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml F: Documentation/devicetree/bindings/spi/hpe,gxp-spifi.yaml F: Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml F: arch/arm/boot/dts/hpe-bmc* F: arch/arm/boot/dts/hpe-gxp* F: arch/arm/mach-hpe/ F: drivers/clocksource/timer-gxp.c +F: drivers/i2c/busses/i2c-gxp.c F: drivers/spi/spi-gxp.c F: drivers/watchdog/gxp-wdt.c -- 2.17.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support 2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins @ 2023-01-28 18:38 ` Wolfram Sang 2023-02-07 21:53 ` Hawkins, Nick 0 siblings, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2023-01-28 18:38 UTC (permalink / raw) To: nick.hawkins Cc: verdun, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel [-- Attachment #1: Type: text/plain, Size: 448 bytes --] On Wed, Jan 25, 2023 at 12:44:38PM -0600, nick.hawkins@hpe.com wrote: > From: Nick Hawkins <nick.hawkins@hpe.com> > > Add the I2C controller source and bindings. > > Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com> I assume this better goes via the arm-soc-tree as well to reduce merge conflicts, so: Acked-by: Wolfram Sang <wsa@kernel.org> Let me know if I should pick this instead. Will review the driver in the next days. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support 2023-01-28 18:38 ` Wolfram Sang @ 2023-02-07 21:53 ` Hawkins, Nick 2023-02-15 19:26 ` Hawkins, Nick 0 siblings, 1 reply; 15+ messages in thread From: Hawkins, Nick @ 2023-02-07 21:53 UTC (permalink / raw) To: Wolfram Sang Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel > Let me know if I should pick this instead. Will review the driver in the > next days. Please include it. There may be warnings generated if MAINTAINERS does not list the files at the time of your commit. Thank you, -Nick Hawkins ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support 2023-02-07 21:53 ` Hawkins, Nick @ 2023-02-15 19:26 ` Hawkins, Nick 0 siblings, 0 replies; 15+ messages in thread From: Hawkins, Nick @ 2023-02-15 19:26 UTC (permalink / raw) To: Wolfram Sang Cc: Verdun, Jean-Marie, robh+dt, krzysztof.kozlowski+dt, linux, linux-i2c, devicetree, linux-kernel, linux-arm-kernel, joel > > Let me know if I should pick this instead. Will review the driver in the > > next days. > Please include it. There may be warnings generated if MAINTAINERS does not list the files at the time of your commit. > Thank you, > -Nick Hawkins Greetings Wolfram, Just a gentle reminder on this review. Thank you for your time, -Nick Hawkins ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-02-16 20:47 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-25 18:44 [PATCH v4 0/5] ARM: Add GXP I2C Support nick.hawkins 2023-01-25 18:44 ` [PATCH v4 1/5] i2c: hpe: Add GXP SoC I2C Controller nick.hawkins 2023-02-15 20:29 ` Wolfram Sang 2023-02-16 20:02 ` Hawkins, Nick 2023-02-16 20:47 ` Wolfram Sang 2023-01-25 18:44 ` [PATCH v4 2/5] dt-bindings: i2c: Add hpe,gxp-i2c nick.hawkins 2023-01-25 21:18 ` Rob Herring 2023-01-25 21:31 ` Hawkins, Nick 2023-01-26 13:38 ` Rob Herring 2023-01-25 18:44 ` [PATCH v4 3/5] ARM: dts: hpe: Add I2C Topology nick.hawkins 2023-01-25 18:44 ` [PATCH v4 4/5] ARM: multi_v7_defconfig: add gxp i2c module nick.hawkins 2023-01-25 18:44 ` [PATCH v4 5/5] MAINTAINERS: Add HPE GXP I2C Support nick.hawkins 2023-01-28 18:38 ` Wolfram Sang 2023-02-07 21:53 ` Hawkins, Nick 2023-02-15 19:26 ` Hawkins, Nick
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).