Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1610300 > unrolled thread

[PATCH v6 0/5] i2c: aspeed: added driver for Aspeed I2C

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/5] i2c: aspeed: added driver for Aspeed I2C Brendan Higgins <brendanhiggins@google.com> - 2017-03-28 07:20 +0200
    [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2017-03-28 07:20 +0200
      Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C  driver Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-03-28 11:00 +0200
        Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2017-03-29 12:30 +0200
        Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C  driver Rob Herring <robh@kernel.org> - 2017-04-03 16:30 +0200
      Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C  driver Rob Herring <robh@kernel.org> - 2017-04-03 16:30 +0200
    [PATCH v6 5/5] i2c: aspeed: added slave support for Aspeed I2C driver Brendan Higgins <brendanhiggins@google.com> - 2017-03-28 07:20 +0200
    Re: [PATCH v6 0/5] i2c: aspeed: added driver for Aspeed I2C Andrew Jeffery <andrew@aj.id.au> - 2017-03-31 02:10 +0200

#1610300 — [PATCH v6 0/5] i2c: aspeed: added driver for Aspeed I2C

Sorry for the delay, I went on a long vacation prior to receiving feedback and
got back in the middle of a hardware bring up that consumed all of my attention
for an extended period of time. I will try to plan upstream submissions around
my other responsibilities better in the future.

Addressed comments from:
  - Vladimir in: https://www.spinics.net/lists/linux-i2c/msg27387.html
    and: https://www.spinics.net/lists/linux-i2c/msg27386.html
  - Wolfram in: https://www.spinics.net/lists/linux-i2c/msg27476.html
    and: https://www.spinics.net/lists/linux-i2c/msg27483.html

Changes since previous update:
  - No longer arbitrarily restrict bus to be slave xor master.
  - Pulled out "struct aspeed_i2c_controller" as a interrupt controller.
  - Pulled out slave support into its own commit.
  - Rewrote code that sets clock divider register because the original version
    set it incorrectly.
  - Discovered and fixed issue in implementation that caused certain slave
    devices to misbehave; the cause was that the master IRQ handler would return
    control to the requesting thread after the last RX or TX command was handled
    such that the requesting thread would issue either a repeated start or stop.
    This was incorrect because the time taken to complete the completion was too
    great. I fixed this by rewriting the master IRQ handler so that it now
    manages the entire transaction only returning control to the requesting
    thread once the entire transaction is complete.
  - Rewrote the aspeed_i2c_master_irq handler because the old method of
    completing a completion in between restarts was too slow causing devices to
    misbehave.
  - Added support for I2C_M_RECV_LEN which I had incorrectly said was supported
    before.
  - Addressed other comments from Vladimir.

Changes have been tested on the Aspeed 2500 evaluation board, as before, and now
on a real platform with an Aspeed 2520.

[toc] | [next] | [standalone]

#1610301 — [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

Added device tree binding documentation for Aspeed I2C busses.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
  - None
Changes for v3:
  - Removed reference to "bus" device tree param
Changes for v4:
  - None
Changes for v5:
  - None
Changes for v6:
  - Replaced the controller property with and interrupt controller, leaving only
    the busses in the I2C documentation.
---
 .../devicetree/bindings/i2c/i2c-aspeed.txt         | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 000000000000..fbcc501706b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,49 @@
+Device tree configuration for the I2C busses on the AST24XX and AST25XX SoCs.
+
+Required Properties:
+- #address-cells	: should be 1
+- #size-cells		: should be 0
+- reg			: address offset and range of bus
+- compatible		: should be "aspeed,ast2400-i2c-bus"
+			  or "aspeed,ast2500-i2c-bus"
+- clocks		: root clock of bus, should reference the APB
+			  clock
+- interrupts		: interrupt number
+- interrupt-parent	: interrupt controller for bus, should reference a
+			  aspeed,ast2400-i2c-ic or aspeed,ast2500-i2c-ic
+			  interrupt controller
+
+Optional Properties:
+- clock-frequency	: frequency of the bus clock in Hz
+			  defaults to 100 kHz when not specified
+
+Example:
+
+i2c {
+	compatible = "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0 0x1e78a000 0x1000>;
+
+	i2c_ic: interrupt-controller@0 {
+		#interrupt-cells = <1>;
+		compatible = "aspeed,ast2400-i2c-ic";
+		reg = <0x0 0x40>;
+		interrupts = <12>;
+		interrupt-controller;
+	};
+
+	i2c0: i2c-bus@40 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#interrupt-cells = <1>;
+		reg = <0x40 0x40>;
+		compatible = "aspeed,ast2400-i2c-bus";
+		bus = <0>;
+		clocks = <&clk_apb>;
+		clock-frequency = <100000>;
+		status = "disabled";
+		interrupts = <0>;
+		interrupt-parent = <&i2c_ic>;
+	};
+};
-- 
2.12.2.564.g063fe858b8-goog

[toc] | [prev] | [next] | [standalone]

#1610424 — Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

On Mon, 2017-03-27 at 22:12 -0700, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C busses.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

> +	i2c0: i2c-bus@40 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		#interrupt-cells = <1>;
> +		reg = <0x40 0x40>;
> +		compatible = "aspeed,ast2400-i2c-bus";
> +		bus = <0>;
> +		clocks = <&clk_apb>;
> +		clock-frequency = <100000>;

For busses it's more traditional to make this "bus-frequency" but
that's a nit and Linux/fdt has not respected that tradition terribly
well. If you respin, it might be work changing.

The clock-frequency tends to be the frequency of the controller itself.

> +		status = "disabled";
> +		interrupts = <0>;
> +		interrupt-parent = <&i2c_ic>;
> +	};
> +};

[toc] | [prev] | [next] | [standalone]

#1611786 — Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

>> +             bus = <0>;
>> +             clocks = <&clk_apb>;
>> +             clock-frequency = <100000>;
>
> For busses it's more traditional to make this "bus-frequency" but
> that's a nit and Linux/fdt has not respected that tradition terribly
> well. If you respin, it might be work changing.
>
> The clock-frequency tends to be the frequency of the controller itself.

Ah, okay. I will change this on the next revision.

[toc] | [prev] | [next] | [standalone]

#1615243 — Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

On Tue, Mar 28, 2017 at 07:54:43PM +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2017-03-27 at 22:12 -0700, Brendan Higgins wrote:
> > Added device tree binding documentation for Aspeed I2C busses.
> > 
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> 
> > +	i2c0: i2c-bus@40 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		#interrupt-cells = <1>;
> > +		reg = <0x40 0x40>;
> > +		compatible = "aspeed,ast2400-i2c-bus";
> > +		bus = <0>;
> > +		clocks = <&clk_apb>;
> > +		clock-frequency = <100000>;
> 
> For busses it's more traditional to make this "bus-frequency" but
> that's a nit and Linux/fdt has not respected that tradition terribly
> well. If you respin, it might be work changing.

Makes sense, but that's news to me. I'm still new to this DT stuff.

> The clock-frequency tends to be the frequency of the controller itself.

The clock binding has largely replaced that.

Rob

[toc] | [prev] | [next] | [standalone]

#1615247 — Re: [PATCH v6 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

On Mon, Mar 27, 2017 at 10:12:24PM -0700, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C busses.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> ---
> Changes for v2:
>   - None
> Changes for v3:
>   - Removed reference to "bus" device tree param
> Changes for v4:
>   - None
> Changes for v5:
>   - None
> Changes for v6:
>   - Replaced the controller property with and interrupt controller, leaving only
>     the busses in the I2C documentation.
> ---
>  .../devicetree/bindings/i2c/i2c-aspeed.txt         | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> new file mode 100644
> index 000000000000..fbcc501706b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
> @@ -0,0 +1,49 @@
> +Device tree configuration for the I2C busses on the AST24XX and AST25XX SoCs.
> +
> +Required Properties:
> +- #address-cells	: should be 1
> +- #size-cells		: should be 0
> +- reg			: address offset and range of bus
> +- compatible		: should be "aspeed,ast2400-i2c-bus"
> +			  or "aspeed,ast2500-i2c-bus"
> +- clocks		: root clock of bus, should reference the APB
> +			  clock
> +- interrupts		: interrupt number
> +- interrupt-parent	: interrupt controller for bus, should reference a
> +			  aspeed,ast2400-i2c-ic or aspeed,ast2500-i2c-ic
> +			  interrupt controller
> +
> +Optional Properties:
> +- clock-frequency	: frequency of the bus clock in Hz
> +			  defaults to 100 kHz when not specified
> +
> +Example:
> +
> +i2c {
> +	compatible = "simple-bus";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges = <0 0x1e78a000 0x1000>;
> +
> +	i2c_ic: interrupt-controller@0 {
> +		#interrupt-cells = <1>;
> +		compatible = "aspeed,ast2400-i2c-ic";
> +		reg = <0x0 0x40>;
> +		interrupts = <12>;
> +		interrupt-controller;
> +	};
> +
> +	i2c0: i2c-bus@40 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		#interrupt-cells = <1>;
> +		reg = <0x40 0x40>;
> +		compatible = "aspeed,ast2400-i2c-bus";
> +		bus = <0>;

Not documented and what's it for?

> +		clocks = <&clk_apb>;
> +		clock-frequency = <100000>;
> +		status = "disabled";

Drop status from examples.

> +		interrupts = <0>;
> +		interrupt-parent = <&i2c_ic>;
> +	};
> +};
> -- 
> 2.12.2.564.g063fe858b8-goog
> 

[toc] | [prev] | [next] | [standalone]

#1610302 — [PATCH v6 5/5] i2c: aspeed: added slave support for Aspeed I2C driver

Added slave support for Aspeed I2C controller. Supports fourteen busses
present in AST24XX and AST25XX BMC SoCs by Aspeed.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Added in v6:
  - Pulled slave support out of initial driver commit into its own commit.
  - No longer arbitrarily restrict bus to be slave xor master.
---
 drivers/i2c/busses/i2c-aspeed.c | 186 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 186 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 04266acc6c46..a9ee58a2c4e2 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -49,6 +49,7 @@
 #define ASPEED_I2CD_SDA_DRIVE_1T_EN			BIT(8)
 #define ASPEED_I2CD_M_SDA_DRIVE_1T_EN			BIT(7)
 #define ASPEED_I2CD_M_HIGH_SPEED_EN			BIT(6)
+#define ASPEED_I2CD_SLAVE_EN				BIT(1)
 #define ASPEED_I2CD_MASTER_EN				BIT(0)
 
 /* 0x04 : I2CD Clock and AC Timing Control Register #1 */
@@ -69,6 +70,7 @@
  */
 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(14)
 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
+#define ASPEED_I2CD_INTR_SLAVE_MATCH			BIT(7)
 #define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
 #define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
 #define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
@@ -106,6 +108,9 @@
 #define ASPEED_I2CD_M_TX_CMD				BIT(1)
 #define ASPEED_I2CD_M_START_CMD				BIT(0)
 
+/* 0x18 : I2CD Slave Device Address Register   */
+#define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_START,
 	ASPEED_I2C_MASTER_TX_FIRST,
@@ -115,6 +120,15 @@ enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 };
 
+enum aspeed_i2c_slave_state {
+	ASPEED_I2C_SLAVE_START,
+	ASPEED_I2C_SLAVE_READ_REQUESTED,
+	ASPEED_I2C_SLAVE_READ_PROCESSED,
+	ASPEED_I2C_SLAVE_WRITE_REQUESTED,
+	ASPEED_I2C_SLAVE_WRITE_RECEIVED,
+	ASPEED_I2C_SLAVE_STOP,
+};
+
 struct aspeed_i2c_bus {
 	struct i2c_adapter		adap;
 	struct device			*dev;
@@ -207,6 +221,110 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
 	return ret;
 }
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
+{
+	u32 command, irq_status, status_ack = 0;
+	struct i2c_client *slave = bus->slave;
+	bool irq_handled = true;
+	u8 value;
+
+	spin_lock(&bus->lock);
+	if (!slave) {
+		irq_handled = false;
+		goto out;
+	}
+
+	command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+	irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+
+	/* Slave was requested, restart state machine. */
+	if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+		status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
+		bus->slave_state = ASPEED_I2C_SLAVE_START;
+	}
+
+	/* Slave is not currently active, irq was for someone else. */
+	if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+		irq_handled = false;
+		goto out;
+	}
+
+	dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
+		irq_status, command);
+
+	/* Slave was sent something. */
+	if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+		value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		/* Handle address frame. */
+		if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
+			if (value & 0x1)
+				bus->slave_state =
+						ASPEED_I2C_SLAVE_READ_REQUESTED;
+			else
+				bus->slave_state =
+						ASPEED_I2C_SLAVE_WRITE_REQUESTED;
+		}
+		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+	}
+
+	/* Slave was asked to stop. */
+	if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+		status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	}
+	if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+		status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	}
+
+	switch (bus->slave_state) {
+	case ASPEED_I2C_SLAVE_READ_REQUESTED:
+		if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+			dev_err(bus->dev, "Unexpected ACK on read request.\n");
+		bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
+
+		i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+		aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+		aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+		break;
+	case ASPEED_I2C_SLAVE_READ_PROCESSED:
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+			dev_err(bus->dev,
+				"Expected ACK after processed read.\n");
+		i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
+		aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+		aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+		break;
+	case ASPEED_I2C_SLAVE_WRITE_REQUESTED:
+		bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+		break;
+	case ASPEED_I2C_SLAVE_WRITE_RECEIVED:
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+		break;
+	case ASPEED_I2C_SLAVE_STOP:
+		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+		break;
+	default:
+		dev_err(bus->dev, "unhandled slave_state: %d\n",
+			bus->slave_state);
+		break;
+	}
+
+	if (status_ack != irq_status)
+		dev_err(bus->dev,
+			"irq handled != irq. expected %x, but was %x\n",
+			irq_status, status_ack);
+	aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
+
+out:
+	spin_unlock(&bus->lock);
+	return irq_handled;
+}
+#endif
+
 static void do_start(struct aspeed_i2c_bus *bus)
 {
 	u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
@@ -371,6 +489,14 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 {
 	struct aspeed_i2c_bus *bus = dev_id;
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	if (aspeed_i2c_slave_irq(bus)) {
+		dev_dbg(bus->dev, "irq handled by slave.\n");
+		return IRQ_HANDLED;
+	}
+#endif
+
+	dev_dbg(bus->dev, "irq handled by master.\n");
 	aspeed_i2c_master_irq(bus);
 	return IRQ_HANDLED;
 }
@@ -426,9 +552,69 @@ static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
 }
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+	u32 addr_reg_val, func_ctrl_reg_val;
+	struct aspeed_i2c_bus *bus;
+	unsigned long flags;
+
+	bus = client->adapter->algo_data;
+	spin_lock_irqsave(&bus->lock, flags);
+	if (bus->slave) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		return -EINVAL;
+	}
+
+	/* Set slave addr. */
+	addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
+	addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+	addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
+	aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
+
+	/* Turn on slave mode. */
+	func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+	aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+	bus->slave = client;
+	bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return 0;
+}
+
+static int aspeed_i2c_unreg_slave(struct i2c_client *client)
+{
+	struct aspeed_i2c_bus *bus = client->adapter->algo_data;
+	u32 func_ctrl_reg_val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	if (!bus->slave) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		return -EINVAL;
+	}
+
+	/* Turn off slave mode. */
+	func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+	aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+	bus->slave = NULL;
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return 0;
+}
+#endif
+
 static const struct i2c_algorithm aspeed_i2c_algo = {
 	.master_xfer	= aspeed_i2c_master_xfer,
 	.functionality	= aspeed_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	.reg_slave	= aspeed_i2c_reg_slave,
+	.unreg_slave	= aspeed_i2c_unreg_slave,
+#endif
 };
 
 static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
-- 
2.12.2.564.g063fe858b8-goog

[toc] | [prev] | [next] | [standalone]

#1613558

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2017-03-27 at 22:12 -0700, Brendan Higgins wrote:
> Sorry for the delay, I went on a long vacation prior to receiving feedback and
> got back in the middle of a hardware bring up that consumed all of my attention
> for an extended period of time. I will try to plan upstream submissions around
> my other responsibilities better in the future.
> 
> Addressed comments from:
>   - Vladimir in: https://www.spinics.net/lists/linux-i2c/msg27387.html
>     and: https://www.spinics.net/lists/linux-i2c/msg27386.html
>   - Wolfram in: https://www.spinics.net/lists/linux-i2c/msg27476.html
>     and: https://www.spinics.net/lists/linux-i2c/msg27483.html
> 
> Changes since previous update:
>   - No longer arbitrarily restrict bus to be slave xor master.
>   - Pulled out "struct aspeed_i2c_controller" as a interrupt controller.
>   - Pulled out slave support into its own commit.
>   - Rewrote code that sets clock divider register because the original version
>     set it incorrectly.
>   - Discovered and fixed issue in implementation that caused certain slave
>     devices to misbehave; the cause was that the master IRQ handler would return
>     control to the requesting thread after the last RX or TX command was handled
>     such that the requesting thread would issue either a repeated start or stop.
>     This was incorrect because the time taken to complete the completion was too
>     great. I fixed this by rewriting the master IRQ handler so that it now
>     manages the entire transaction only returning control to the requesting
>     thread once the entire transaction is complete.
>   - Rewrote the aspeed_i2c_master_irq handler because the old method of
>     completing a completion in between restarts was too slow causing devices to
>     misbehave.
>   - Added support for I2C_M_RECV_LEN which I had incorrectly said was supported
>     before.
>   - Addressed other comments from Vladimir.
> 
> Changes have been tested on the Aspeed 2500 evaluation board, as before, and now
> on a real platform with an Aspeed 2520.

Looks like there's going to be another revision of the series, but
regardless, I've applied and tested v6 and had no issues. So:

Tested-by: Andrew Jeffery <andrew@aj.id.au>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web