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

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

[Brendan Higgins]

Addressed comments from:
  - Marc in: http://www.spinics.net/lists/devicetree/msg179432.html
  - Ben in: http://www.spinics.net/lists/devicetree/msg179491.html

Changes since previous update:
  - I2C irqchip: Unallocate resources on init failure
  - I2C driver: Added master_xfer_result field to struct aspeed_i2c_bus

As before, tested on Aspeed 2500 evaluation board and a real platform with an
Aspeed 2520.

[PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller

[Brendan Higgins]

Added device tree binding documentation for Aspeed I2C Interrupt
Controller.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Added in v6:
  - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
    what it actually does.
Changes for v7:
  - None
Changes for v8:
  - None
Changes for v9:
  - None
Changes for v10:
  - None
---
 .../interrupt-controller/aspeed,ast2400-i2c-ic.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
new file mode 100644
index 000000000000..033cc82e5684
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
@@ -0,0 +1,25 @@
+Device tree configuration for the I2C Interrupt Controller on the AST24XX and
+AST25XX SoCs.
+
+Required Properties:
+- #address-cells	: should be 1
+- #size-cells 		: should be 1
+- #interrupt-cells 	: should be 1
+- compatible 		: should be "aspeed,ast2400-i2c-ic"
+			  or "aspeed,ast2500-i2c-ic"
+- reg			: address start and range of controller
+- interrupts		: interrupt number
+- interrupt-controller	: denotes that the controller receives and fires
+			  new interrupts for child busses
+
+Example:
+
+i2c_ic: interrupt-controller@0 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	#interrupt-cells = <1>;
+	compatible = "aspeed,ast2400-i2c-ic";
+	reg = <0x0 0x40>;
+	interrupts = <12>;
+	interrupt-controller;
+};
-- 
2.13.0.506.g27d5fe0cd-goog

Re: [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller

[Joel Stanley]

On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> Added device tree binding documentation for Aspeed I2C Interrupt
> Controller.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Acked-by: Rob Herring <robh@kernel.org>

Acked-by: Joel Stanley <joel@jms.id.au>

Re: [PATCH v10 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller

[Marc Zyngier]

On 03/06/17 02:29, Brendan Higgins wrote:
> Added device tree binding documentation for Aspeed I2C Interrupt
> Controller.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Acked-by: Rob Herring <robh@kernel.org>

I've queued this (and the next patch) for 4.13.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed

[Brendan Higgins]

The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
separate I2C busses. This adds a dummy irqchip which maps the single
hardware interrupt to software interrupts for each of the busses.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Added in v6:
  - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
    what it actually does.
Changes for v7:
  - Renamed irq domain for consistency
Changes for v8:
  - None
Changes for v9:
  - None
Changes for v10:
  - Unallocate resources on init failure
  - Changed formatting of a function call
---
 drivers/irqchip/Makefile            |   2 +-
 drivers/irqchip/irq-aspeed-i2c-ic.c | 115 ++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b64c59b838a0..e067f9839b33 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -73,6 +73,6 @@ obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
 obj-$(CONFIG_MVEBU_PIC)			+= irq-mvebu-pic.o
 obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
 obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
-obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
+obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
 obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
new file mode 100644
index 000000000000..815b88dd18f2
--- /dev/null
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -0,0 +1,115 @@
+/*
+ *  Aspeed 24XX/25XX I2C Interrupt Controller.
+ *
+ *  Copyright (C) 2012-2017 ASPEED Technology Inc.
+ *  Copyright 2017 IBM Corporation
+ *  Copyright 2017 Google, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+
+
+#define ASPEED_I2C_IC_NUM_BUS 14
+
+struct aspeed_i2c_ic {
+	void __iomem		*base;
+	int			parent_irq;
+	struct irq_domain	*irq_domain;
+};
+
+/*
+ * The aspeed chip provides a single hardware interrupt for all of the I2C
+ * busses, so we use a dummy interrupt chip to translate this single interrupt
+ * into multiple interrupts, each associated with a single I2C bus.
+ */
+static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
+{
+	struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long bit, status;
+	unsigned int bus_irq;
+
+	chained_irq_enter(chip, desc);
+	status = readl(i2c_ic->base);
+	for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
+		bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
+		generic_handle_irq(bus_irq);
+	}
+	chained_irq_exit(chip, desc);
+}
+
+/*
+ * Set simple handler and mark IRQ as valid. Nothing interesting to do here
+ * since we are using a dummy interrupt chip.
+ */
+static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
+					unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, domain->host_data);
+
+	return 0;
+}
+
+static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
+	.map = aspeed_i2c_ic_map_irq_domain,
+};
+
+static int __init aspeed_i2c_ic_of_init(struct device_node *node,
+					struct device_node *parent)
+{
+	struct aspeed_i2c_ic *i2c_ic;
+	int ret = 0;
+
+	i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
+	if (!i2c_ic)
+		return -ENOMEM;
+
+	i2c_ic->base = of_iomap(node, 0);
+	if (IS_ERR(i2c_ic->base)) {
+		ret = PTR_ERR(i2c_ic->base);
+		goto err_free_ic;
+	}
+
+	i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
+	if (i2c_ic->parent_irq < 0) {
+		ret = i2c_ic->parent_irq;
+		goto err_iounmap;
+	}
+
+	i2c_ic->irq_domain = irq_domain_add_linear(node, ASPEED_I2C_IC_NUM_BUS,
+						   &aspeed_i2c_ic_irq_domain_ops,
+						   NULL);
+	if (!i2c_ic->irq_domain) {
+		ret = -ENOMEM;
+		goto err_iounmap;
+	}
+
+	i2c_ic->irq_domain->name = "aspeed-i2c-domain";
+
+	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
+					 aspeed_i2c_ic_irq_handler, i2c_ic);
+
+	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
+
+	return 0;
+
+err_iounmap:
+	iounmap(i2c_ic->base);
+err_free_ic:
+	kfree(i2c_ic);
+	return ret;
+}
+
+IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
+IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);
-- 
2.13.0.506.g27d5fe0cd-goog

Re: [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 5618 bytes --]

On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
> The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
> separate I2C busses. This adds a dummy irqchip which maps the single
> hardware interrupt to software interrupts for each of the busses.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

I exercised the patch on an AST2500-based machine with good results.

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

> ---
> Added in v6:
>   - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
>     what it actually does.
> Changes for v7:
>   - Renamed irq domain for consistency
> Changes for v8:
>   - None
> Changes for v9:
>   - None
> Changes for v10:
>   - Unallocate resources on init failure
>   - Changed formatting of a function call
> ---
>  drivers/irqchip/Makefile            |   2 +-
>  drivers/irqchip/irq-aspeed-i2c-ic.c | 115 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c
> 
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b64c59b838a0..e067f9839b33 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> > @@ -73,6 +73,6 @@ obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
> >  obj-$(CONFIG_MVEBU_PIC)			+= irq-mvebu-pic.o
> >  obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
> >  obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
> > -obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
> > +obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
> >  obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
> >  obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
> new file mode 100644
> index 000000000000..815b88dd18f2
> --- /dev/null
> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
> @@ -0,0 +1,115 @@
> +/*
> + *  Aspeed 24XX/25XX I2C Interrupt Controller.
> + *
> + *  Copyright (C) 2012-2017 ASPEED Technology Inc.
> + *  Copyright 2017 IBM Corporation
> + *  Copyright 2017 Google, Inc.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/io.h>
> +
> +
> +#define ASPEED_I2C_IC_NUM_BUS 14
> +
> +struct aspeed_i2c_ic {
> > > +	void __iomem		*base;
> > > +	int			parent_irq;
> > > +	struct irq_domain	*irq_domain;
> +};
> +
> +/*
> + * The aspeed chip provides a single hardware interrupt for all of the I2C
> + * busses, so we use a dummy interrupt chip to translate this single interrupt
> + * into multiple interrupts, each associated with a single I2C bus.
> + */
> +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
> +{
> > +	struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	unsigned long bit, status;
> > +	unsigned int bus_irq;
> +
> > +	chained_irq_enter(chip, desc);
> > +	status = readl(i2c_ic->base);
> > +	for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
> > +		bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
> > +		generic_handle_irq(bus_irq);
> > +	}
> > +	chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * Set simple handler and mark IRQ as valid. Nothing interesting to do here
> + * since we are using a dummy interrupt chip.
> + */
> +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
> > +					unsigned int irq, irq_hw_number_t hwirq)
> +{
> > +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> > +	irq_set_chip_data(irq, domain->host_data);
> +
> > +	return 0;
> +}
> +
> +static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
> > +	.map = aspeed_i2c_ic_map_irq_domain,
> +};
> +
> +static int __init aspeed_i2c_ic_of_init(struct device_node *node,
> > +					struct device_node *parent)
> +{
> > +	struct aspeed_i2c_ic *i2c_ic;
> > +	int ret = 0;
> +
> > +	i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
> > +	if (!i2c_ic)
> > +		return -ENOMEM;
> +
> > +	i2c_ic->base = of_iomap(node, 0);
> > +	if (IS_ERR(i2c_ic->base)) {
> > +		ret = PTR_ERR(i2c_ic->base);
> > +		goto err_free_ic;
> > +	}
> +
> > +	i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
> > +	if (i2c_ic->parent_irq < 0) {
> > +		ret = i2c_ic->parent_irq;
> > +		goto err_iounmap;
> > +	}
> +
> > +	i2c_ic->irq_domain = irq_domain_add_linear(node, ASPEED_I2C_IC_NUM_BUS,
> > +						   &aspeed_i2c_ic_irq_domain_ops,
> > +						   NULL);
> > +	if (!i2c_ic->irq_domain) {
> > +		ret = -ENOMEM;
> > +		goto err_iounmap;
> > +	}
> +
> > +	i2c_ic->irq_domain->name = "aspeed-i2c-domain";
> +
> > +	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
> > +					 aspeed_i2c_ic_irq_handler, i2c_ic);
> +
> > +	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
> +
> > +	return 0;
> +
> +err_iounmap:
> > +	iounmap(i2c_ic->base);
> +err_free_ic:
> > +	kfree(i2c_ic);
> > +	return ret;
> +}
> +
> +IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
> +IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v10 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 5618 bytes --]

On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
> The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
> separate I2C busses. This adds a dummy irqchip which maps the single
> hardware interrupt to software interrupts for each of the busses.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

I exercised the patch on an AST2500-based machine with good results.

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

> ---
> Added in v6:
>   - Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
>     what it actually does.
> Changes for v7:
>   - Renamed irq domain for consistency
> Changes for v8:
>   - None
> Changes for v9:
>   - None
> Changes for v10:
>   - Unallocate resources on init failure
>   - Changed formatting of a function call
> ---
>  drivers/irqchip/Makefile            |   2 +-
>  drivers/irqchip/irq-aspeed-i2c-ic.c | 115 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c
> 
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b64c59b838a0..e067f9839b33 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> > @@ -73,6 +73,6 @@ obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
> >  obj-$(CONFIG_MVEBU_PIC)			+= irq-mvebu-pic.o
> >  obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
> >  obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
> > -obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o
> > +obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
> >  obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
> >  obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
> new file mode 100644
> index 000000000000..815b88dd18f2
> --- /dev/null
> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
> @@ -0,0 +1,115 @@
> +/*
> + *  Aspeed 24XX/25XX I2C Interrupt Controller.
> + *
> + *  Copyright (C) 2012-2017 ASPEED Technology Inc.
> + *  Copyright 2017 IBM Corporation
> + *  Copyright 2017 Google, Inc.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/io.h>
> +
> +
> +#define ASPEED_I2C_IC_NUM_BUS 14
> +
> +struct aspeed_i2c_ic {
> > > +	void __iomem		*base;
> > > +	int			parent_irq;
> > > +	struct irq_domain	*irq_domain;
> +};
> +
> +/*
> + * The aspeed chip provides a single hardware interrupt for all of the I2C
> + * busses, so we use a dummy interrupt chip to translate this single interrupt
> + * into multiple interrupts, each associated with a single I2C bus.
> + */
> +static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
> +{
> > +	struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	unsigned long bit, status;
> > +	unsigned int bus_irq;
> +
> > +	chained_irq_enter(chip, desc);
> > +	status = readl(i2c_ic->base);
> > +	for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
> > +		bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
> > +		generic_handle_irq(bus_irq);
> > +	}
> > +	chained_irq_exit(chip, desc);
> +}
> +
> +/*
> + * Set simple handler and mark IRQ as valid. Nothing interesting to do here
> + * since we are using a dummy interrupt chip.
> + */
> +static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
> > +					unsigned int irq, irq_hw_number_t hwirq)
> +{
> > +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> > +	irq_set_chip_data(irq, domain->host_data);
> +
> > +	return 0;
> +}
> +
> +static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
> > +	.map = aspeed_i2c_ic_map_irq_domain,
> +};
> +
> +static int __init aspeed_i2c_ic_of_init(struct device_node *node,
> > +					struct device_node *parent)
> +{
> > +	struct aspeed_i2c_ic *i2c_ic;
> > +	int ret = 0;
> +
> > +	i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
> > +	if (!i2c_ic)
> > +		return -ENOMEM;
> +
> > +	i2c_ic->base = of_iomap(node, 0);
> > +	if (IS_ERR(i2c_ic->base)) {
> > +		ret = PTR_ERR(i2c_ic->base);
> > +		goto err_free_ic;
> > +	}
> +
> > +	i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
> > +	if (i2c_ic->parent_irq < 0) {
> > +		ret = i2c_ic->parent_irq;
> > +		goto err_iounmap;
> > +	}
> +
> > +	i2c_ic->irq_domain = irq_domain_add_linear(node, ASPEED_I2C_IC_NUM_BUS,
> > +						   &aspeed_i2c_ic_irq_domain_ops,
> > +						   NULL);
> > +	if (!i2c_ic->irq_domain) {
> > +		ret = -ENOMEM;
> > +		goto err_iounmap;
> > +	}
> +
> > +	i2c_ic->irq_domain->name = "aspeed-i2c-domain";
> +
> > +	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
> > +					 aspeed_i2c_ic_irq_handler, i2c_ic);
> +
> > +	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
> +
> > +	return 0;
> +
> +err_iounmap:
> > +	iounmap(i2c_ic->base);
> +err_free_ic:
> > +	kfree(i2c_ic);
> > +	return ret;
> +}
> +
> +IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
> +IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

[Brendan Higgins]

Added device tree binding documentation for Aspeed I2C busses.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Rob Herring <robh@kernel.org>
---
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.
Changes for v7:
  - Changed clock-frequency to bus-frequency in device tree
Changes for v8:
  - Added multi-master property to device tree
Changes for v9:
  - None
Changes for v10:
  - None
---
 .../devicetree/bindings/i2c/i2c-aspeed.txt         | 48 ++++++++++++++++++++++
 1 file changed, 48 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..bd6480b19535
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,48 @@
+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:
+- bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
+		  specified
+- multi-master	: states that there is another master active on this bus.
+
+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";
+		clocks = <&clk_apb>;
+		bus-frequency = <100000>;
+		interrupts = <0>;
+		interrupt-parent = <&i2c_ic>;
+	};
+};
-- 
2.13.0.506.g27d5fe0cd-goog

Re: [PATCH v10 3/5] i2c: aspeed: added documentation for Aspeed I2C driver

[Joel Stanley]

On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> Added device tree binding documentation for Aspeed I2C busses.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Acked-by: Rob Herring <robh@kernel.org>

Acked-by: Joel Stanley <joel@jms.id.au>

[PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

Added initial master 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>
---
Changes for v2:
  - Added single module_init (multiple was breaking some builds).
Changes for v3:
  - Removed "bus" device tree param; now extracted from bus address offset
Changes for v4:
  - I2C adapter number is now generated dynamically unless specified in alias.
Changes for v5:
  - Removed irq_chip used to multiplex IRQ and replaced it with dummy_irq_chip
    along with some other IRQ cleanup.
  - Addressed comments from Cedric, and Vladimir, mostly stylistic things and
    using devm managed resources.
  - Increased max clock frequency before the bus is put in HighSpeed mode, as
    per Kachalov's comment.
Changes for v6:
  - 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.
  - 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 for v7:
  - Changed clock-frequency to bus-frequency
  - Made some fixes to clock divider code
  - Added hardware reset function
  - Marked functions that need to be called with the lock held as "unlocked"
  - Did a bunch of clean up
Changes for v8:
  - ACK IRQ status bits before doing anything else
  - Added multi-master device tree property
  - Do not send STOP commands after interrupt errors
  - Fix SMBUS_QUICK emulation handling
  - Removed highspeed clock code (I will do it in a later patch set)
  - Use the platform_device name for the adapter name
  - Reset for all failed recoveries
  - Removed the "__" prefix from all of the non-thread safe functions
Changes for v9:
  - No longer debug log after every address NACK
  - Clear errors after reset
  - No longer access clock while holding lock
  - Ack all interrupts durring bus reset and initialization
Changes for v10:
  - Added master_xfer_result field to struct aspeed_i2c_bus
---
 drivers/i2c/busses/Kconfig      |  10 +
 drivers/i2c/busses/Makefile     |   1 +
 drivers/i2c/busses/i2c-aspeed.c | 681 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 692 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-aspeed.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 144cbadc7c72..280f84a0d7d1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -326,6 +326,16 @@ config I2C_POWERMAC
 
 comment "I2C system bus drivers (mostly embedded / system-on-chip)"
 
+config I2C_ASPEED
+	tristate "Aspeed I2C Controller"
+	depends on ARCH_ASPEED
+	help
+	  If you say yes to this option, support will be included for the
+	  Aspeed I2C controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-aspeed.
+
 config I2C_AT91
 	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
 	depends on ARCH_AT91
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 30b60855fbcd..e84604b9bf3b 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 
 # Embedded system I2C/SMBus host controller drivers
+obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
 obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_AXXIA)		+= i2c-axxia.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
new file mode 100644
index 000000000000..a04021e3b3ab
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,681 @@
+/*
+ *  Aspeed 24XX/25XX I2C Controller.
+ *
+ *  Copyright (C) 2012-2017 ASPEED Technology Inc.
+ *  Copyright 2017 IBM Corporation
+ *  Copyright 2017 Google, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* I2C Register */
+#define ASPEED_I2C_FUN_CTRL_REG				0x00
+#define ASPEED_I2C_AC_TIMING_REG1			0x04
+#define ASPEED_I2C_AC_TIMING_REG2			0x08
+#define ASPEED_I2C_INTR_CTRL_REG			0x0c
+#define ASPEED_I2C_INTR_STS_REG				0x10
+#define ASPEED_I2C_CMD_REG				0x14
+#define ASPEED_I2C_DEV_ADDR_REG				0x18
+#define ASPEED_I2C_BYTE_BUF_REG				0x20
+
+/* Global Register Definition */
+/* 0x00 : I2C Interrupt Status Register  */
+/* 0x08 : I2C Interrupt Target Assignment  */
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register  */
+#define ASPEED_I2CD_MULTI_MASTER_DIS			BIT(15)
+#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_MASTER_EN				BIT(0)
+
+/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
+#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT			16
+#define ASPEED_I2CD_TIME_SCL_HIGH_MASK			GENMASK(19, 16)
+#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT			12
+#define ASPEED_I2CD_TIME_SCL_LOW_MASK			GENMASK(15, 12)
+#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK		GENMASK(3, 0)
+#define ASPEED_I2CD_TIME_SCL_REG_MAX			GENMASK(3, 0)
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define ASPEED_NO_TIMEOUT_CTRL				0
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(14)
+#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
+#define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
+#define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
+#define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
+#define ASPEED_I2CD_INTR_ARBIT_LOSS			BIT(3)
+#define ASPEED_I2CD_INTR_RX_DONE			BIT(2)
+#define ASPEED_I2CD_INTR_TX_NAK				BIT(1)
+#define ASPEED_I2CD_INTR_TX_ACK				BIT(0)
+#define ASPEED_I2CD_INTR_ERROR						       \
+		(ASPEED_I2CD_INTR_ARBIT_LOSS |				       \
+		 ASPEED_I2CD_INTR_ABNORMAL |				       \
+		 ASPEED_I2CD_INTR_SCL_TIMEOUT |				       \
+		 ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
+#define ASPEED_I2CD_INTR_ALL						       \
+		(ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |			       \
+		 ASPEED_I2CD_INTR_BUS_RECOVER_DONE |			       \
+		 ASPEED_I2CD_INTR_SCL_TIMEOUT |				       \
+		 ASPEED_I2CD_INTR_ABNORMAL |				       \
+		 ASPEED_I2CD_INTR_NORMAL_STOP |				       \
+		 ASPEED_I2CD_INTR_ARBIT_LOSS |				       \
+		 ASPEED_I2CD_INTR_RX_DONE |				       \
+		 ASPEED_I2CD_INTR_TX_NAK |				       \
+		 ASPEED_I2CD_INTR_TX_ACK)
+
+/* 0x14 : I2CD Command/Status Register   */
+#define ASPEED_I2CD_SCL_LINE_STS			BIT(18)
+#define ASPEED_I2CD_SDA_LINE_STS			BIT(17)
+#define ASPEED_I2CD_BUS_BUSY_STS			BIT(16)
+#define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
+
+/* Command Bit */
+#define ASPEED_I2CD_M_STOP_CMD				BIT(5)
+#define ASPEED_I2CD_M_S_RX_CMD_LAST			BIT(4)
+#define ASPEED_I2CD_M_RX_CMD				BIT(3)
+#define ASPEED_I2CD_S_TX_CMD				BIT(2)
+#define ASPEED_I2CD_M_TX_CMD				BIT(1)
+#define ASPEED_I2CD_M_START_CMD				BIT(0)
+
+enum aspeed_i2c_master_state {
+	ASPEED_I2C_MASTER_START,
+	ASPEED_I2C_MASTER_TX_FIRST,
+	ASPEED_I2C_MASTER_TX,
+	ASPEED_I2C_MASTER_RX_FIRST,
+	ASPEED_I2C_MASTER_RX,
+	ASPEED_I2C_MASTER_STOP,
+	ASPEED_I2C_MASTER_INACTIVE,
+};
+
+struct aspeed_i2c_bus {
+	struct i2c_adapter		adap;
+	struct device			*dev;
+	void __iomem			*base;
+	/* Synchronizes I/O mem access to base. */
+	spinlock_t			lock;
+	struct completion		cmd_complete;
+	int				irq;
+	unsigned long			parent_clk_frequency;
+	u32				bus_frequency;
+	/* Transaction state. */
+	enum aspeed_i2c_master_state	master_state;
+	struct i2c_msg			*msgs;
+	size_t				buf_index;
+	size_t				msgs_index;
+	size_t				msgs_count;
+	bool				send_stop;
+	int				cmd_err;
+	/* Protected only by i2c_lock_bus */
+	int				master_xfer_result;
+};
+
+static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
+
+static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
+{
+	unsigned long time_left, flags;
+	int ret = 0;
+	u32 command;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	command = readl(bus->base + ASPEED_I2C_CMD_REG);
+
+	if (command & ASPEED_I2CD_SDA_LINE_STS) {
+		/* Bus is idle: no recovery needed. */
+		if (command & ASPEED_I2CD_SCL_LINE_STS)
+			goto out;
+		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+			command);
+
+		reinit_completion(&bus->cmd_complete);
+		writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
+		spin_unlock_irqrestore(&bus->lock, flags);
+
+		time_left = wait_for_completion_timeout(
+				&bus->cmd_complete, bus->adap.timeout);
+
+		spin_lock_irqsave(&bus->lock, flags);
+		if (time_left == 0)
+			goto reset_out;
+		else if (bus->cmd_err)
+			goto reset_out;
+		/* Recovery failed. */
+		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+			   ASPEED_I2CD_SCL_LINE_STS))
+			goto reset_out;
+	/* Bus error. */
+	} else {
+		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+			command);
+
+		reinit_completion(&bus->cmd_complete);
+		writel(ASPEED_I2CD_BUS_RECOVER_CMD,
+		       bus->base + ASPEED_I2C_CMD_REG);
+		spin_unlock_irqrestore(&bus->lock, flags);
+
+		time_left = wait_for_completion_timeout(
+				&bus->cmd_complete, bus->adap.timeout);
+
+		spin_lock_irqsave(&bus->lock, flags);
+		if (time_left == 0)
+			goto reset_out;
+		else if (bus->cmd_err)
+			goto reset_out;
+		/* Recovery failed. */
+		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+			   ASPEED_I2CD_SDA_LINE_STS))
+			goto reset_out;
+	}
+
+out:
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return ret;
+
+reset_out:
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return aspeed_i2c_reset(bus);
+}
+
+/* precondition: bus.lock has been acquired. */
+static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
+{
+	u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
+	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
+	u8 slave_addr = msg->addr << 1;
+
+	bus->master_state = ASPEED_I2C_MASTER_START;
+	bus->buf_index = 0;
+
+	if (msg->flags & I2C_M_RD) {
+		slave_addr |= 1;
+		command |= ASPEED_I2CD_M_RX_CMD;
+		/* Need to let the hardware know to NACK after RX. */
+		if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
+			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+	}
+
+	writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+	writel(command, bus->base + ASPEED_I2C_CMD_REG);
+}
+
+/* precondition: bus.lock has been acquired. */
+static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
+{
+	bus->master_state = ASPEED_I2C_MASTER_STOP;
+	writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
+}
+
+/* precondition: bus.lock has been acquired. */
+static void aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
+{
+	if (bus->msgs_index + 1 < bus->msgs_count) {
+		bus->msgs_index++;
+		aspeed_i2c_do_start(bus);
+	} else {
+		aspeed_i2c_do_stop(bus);
+	}
+}
+
+static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
+{
+	u32 irq_status, status_ack = 0, command = 0;
+	struct i2c_msg *msg;
+	u8 recv_byte;
+
+	spin_lock(&bus->lock);
+	irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+	/* Ack all interrupt bits. */
+	writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+	if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
+		goto out_complete;
+	}
+
+	/*
+	 * We encountered an interrupt that reports an error: the hardware
+	 * should clear the command queue effectively taking us back to the
+	 * INACTIVE state.
+	 */
+	if (irq_status & ASPEED_I2CD_INTR_ERROR) {
+		dev_dbg(bus->dev, "received error interrupt: 0x%08x",
+			irq_status);
+		bus->cmd_err = -EIO;
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		goto out_complete;
+	}
+
+	/* We are in an invalid state; reset bus to a known state. */
+	if (!bus->msgs && bus->master_state != ASPEED_I2C_MASTER_STOP) {
+		dev_err(bus->dev, "bus in unknown state");
+		bus->cmd_err = -EIO;
+		aspeed_i2c_do_stop(bus);
+		goto out_no_complete;
+	}
+	msg = &bus->msgs[bus->msgs_index];
+
+	/*
+	 * START is a special case because we still have to handle a subsequent
+	 * TX or RX immediately after we handle it, so we handle it here and
+	 * then update the state and handle the new state below.
+	 */
+	if (bus->master_state == ASPEED_I2C_MASTER_START) {
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+			pr_devel("no slave present at %02x", msg->addr);
+			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+			goto error_and_stop;
+		}
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		if (msg->len == 0) { /* SMBUS_QUICK */
+			aspeed_i2c_do_stop(bus);
+			goto out_no_complete;
+		}
+		if (msg->flags & I2C_M_RD)
+			bus->master_state = ASPEED_I2C_MASTER_RX_FIRST;
+		else
+			bus->master_state = ASPEED_I2C_MASTER_TX_FIRST;
+	}
+
+	switch (bus->master_state) {
+	case ASPEED_I2C_MASTER_TX:
+		if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
+			dev_dbg(bus->dev, "slave NACKed TX");
+			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+			goto error_and_stop;
+		} else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+			dev_err(bus->dev, "slave failed to ACK TX");
+			goto error_and_stop;
+		}
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		/* fallthrough intended */
+	case ASPEED_I2C_MASTER_TX_FIRST:
+		if (bus->buf_index < msg->len) {
+			bus->master_state = ASPEED_I2C_MASTER_TX;
+			writel(msg->buf[bus->buf_index++],
+			       bus->base + ASPEED_I2C_BYTE_BUF_REG);
+			writel(ASPEED_I2CD_M_TX_CMD,
+			       bus->base + ASPEED_I2C_CMD_REG);
+		} else {
+			aspeed_i2c_next_msg_or_stop(bus);
+		}
+		goto out_no_complete;
+	case ASPEED_I2C_MASTER_RX_FIRST:
+		/* RX may not have completed yet (only address cycle) */
+		if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
+			goto out_no_complete;
+		/* fallthrough intended */
+	case ASPEED_I2C_MASTER_RX:
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
+			dev_err(bus->dev, "master failed to RX");
+			goto error_and_stop;
+		}
+		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+
+		recv_byte = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		msg->buf[bus->buf_index++] = recv_byte;
+
+		if (msg->flags & I2C_M_RECV_LEN) {
+			if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
+				bus->cmd_err = -EPROTO;
+				aspeed_i2c_do_stop(bus);
+				goto out_no_complete;
+			}
+			msg->len = recv_byte +
+					((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
+			msg->flags &= ~I2C_M_RECV_LEN;
+		}
+
+		if (bus->buf_index < msg->len) {
+			bus->master_state = ASPEED_I2C_MASTER_RX;
+			command = ASPEED_I2CD_M_RX_CMD;
+			if (bus->buf_index + 1 == msg->len)
+				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+			writel(command, bus->base + ASPEED_I2C_CMD_REG);
+		} else {
+			aspeed_i2c_next_msg_or_stop(bus);
+		}
+		goto out_no_complete;
+	case ASPEED_I2C_MASTER_STOP:
+		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
+			dev_err(bus->dev, "master failed to STOP");
+			bus->cmd_err = -EIO;
+			/* Do not STOP as we have already tried. */
+		} else {
+			status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+		}
+
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		goto out_complete;
+	case ASPEED_I2C_MASTER_INACTIVE:
+		dev_err(bus->dev,
+			"master received interrupt 0x%08x, but is inactive",
+			irq_status);
+		bus->cmd_err = -EIO;
+		/* Do not STOP as we should be inactive. */
+		goto out_complete;
+	default:
+		WARN(1, "unknown master state\n");
+		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+		bus->cmd_err = -EIO;
+		goto out_complete;
+	}
+error_and_stop:
+	bus->cmd_err = -EIO;
+	aspeed_i2c_do_stop(bus);
+	goto out_no_complete;
+out_complete:
+	bus->msgs = NULL;
+	if (bus->cmd_err)
+		bus->master_xfer_result = bus->cmd_err;
+	else
+		bus->master_xfer_result = bus->msgs_index + 1;
+	complete(&bus->cmd_complete);
+out_no_complete:
+	if (irq_status != status_ack)
+		dev_err(bus->dev,
+			"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
+			irq_status, status_ack);
+	spin_unlock(&bus->lock);
+	return !!irq_status;
+}
+
+static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
+{
+	struct aspeed_i2c_bus *bus = dev_id;
+
+	if (aspeed_i2c_master_irq(bus))
+		return IRQ_HANDLED;
+	else
+		return IRQ_NONE;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+				  struct i2c_msg *msgs, int num)
+{
+	struct aspeed_i2c_bus *bus = adap->algo_data;
+	unsigned long time_left, flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	bus->cmd_err = 0;
+
+	/* If bus is busy, attempt recovery. We assume a single master
+	 * environment.
+	 */
+	if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		ret = aspeed_i2c_recover_bus(bus);
+		if (ret)
+			return ret;
+		spin_lock_irqsave(&bus->lock, flags);
+	}
+
+	bus->cmd_err = 0;
+	bus->msgs = msgs;
+	bus->msgs_index = 0;
+	bus->msgs_count = num;
+
+	reinit_completion(&bus->cmd_complete);
+	aspeed_i2c_do_start(bus);
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	time_left = wait_for_completion_timeout(&bus->cmd_complete,
+						bus->adap.timeout);
+
+	if (time_left == 0)
+		return -ETIMEDOUT;
+	else
+		return bus->master_xfer_result;
+}
+
+static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm aspeed_i2c_algo = {
+	.master_xfer	= aspeed_i2c_master_xfer,
+	.functionality	= aspeed_i2c_functionality,
+};
+
+static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
+{
+	u32 base_clk, clk_high, clk_low, tmp;
+
+	/*
+	 * The actual clock frequency of SCL is:
+	 *	SCL_freq = APB_freq / (base_freq * (SCL_high + SCL_low))
+	 *		 = APB_freq / divisor
+	 * where base_freq is a programmable clock divider; its value is
+	 *	base_freq = 1 << base_clk
+	 * SCL_high is the number of base_freq clock cycles that SCL stays high
+	 * and SCL_low is the number of base_freq clock cycles that SCL stays
+	 * low for a period of SCL.
+	 * The actual register has a minimum SCL_high and SCL_low minimum of 1;
+	 * thus, they start counting at zero. So
+	 *	SCL_high = clk_high + 1
+	 *	SCL_low	 = clk_low + 1
+	 * Thus,
+	 *	SCL_freq = APB_freq /
+	 *		((1 << base_clk) * (clk_high + 1 + clk_low + 1))
+	 * The documentation recommends clk_high >= 8 and clk_low >= 7 when
+	 * possible; this last constraint gives us the following solution:
+	 */
+	base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
+	tmp = divisor / (1 << base_clk);
+	clk_high = tmp / 2 + tmp % 2;
+	clk_low = tmp - clk_high;
+
+	clk_high -= 1;
+	clk_low -= 1;
+
+	return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
+		& ASPEED_I2CD_TIME_SCL_HIGH_MASK)
+			| ((clk_low << ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
+			   & ASPEED_I2CD_TIME_SCL_LOW_MASK)
+			| (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
+}
+
+/* precondition: bus.lock has been acquired. */
+static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
+{
+	u32 divisor, clk_reg_val;
+
+	divisor = bus->parent_clk_frequency / bus->bus_frequency;
+	clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
+	writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
+	writel(ASPEED_NO_TIMEOUT_CTRL, bus->base + ASPEED_I2C_AC_TIMING_REG2);
+
+	return 0;
+}
+
+/* precondition: bus.lock has been acquired. */
+static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
+			     struct platform_device *pdev)
+{
+	u32 fun_ctrl_reg = ASPEED_I2CD_MASTER_EN;
+	int ret;
+
+	/* Disable everything. */
+	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+	ret = aspeed_i2c_init_clk(bus);
+	if (ret < 0)
+		return ret;
+
+	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
+
+	/* Enable Master Mode */
+	writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) | fun_ctrl_reg,
+	       bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+	/* Set interrupt generation of I2C controller */
+	writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+	return 0;
+}
+
+static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
+{
+	struct platform_device *pdev = to_platform_device(bus->dev);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&bus->lock, flags);
+
+	/* Disable and ack all interrupts. */
+	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+	ret = aspeed_i2c_init(bus, pdev);
+
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return ret;
+}
+
+static int aspeed_i2c_probe_bus(struct platform_device *pdev)
+{
+	struct aspeed_i2c_bus *bus;
+	struct clk *parent_clk;
+	struct resource *res;
+	int ret;
+
+	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+	if (!bus)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bus->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(bus->base))
+		return PTR_ERR(bus->base);
+
+	parent_clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(parent_clk))
+		return PTR_ERR(parent_clk);
+	bus->parent_clk_frequency = clk_get_rate(parent_clk);
+	/* We just need the clock rate, we don't actually use the clk object. */
+	devm_clk_put(&pdev->dev, parent_clk);
+
+	ret = of_property_read_u32(pdev->dev.of_node,
+				   "bus-frequency", &bus->bus_frequency);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Could not read bus-frequency property\n");
+		bus->bus_frequency = 100000;
+	}
+
+	/* Initialize the I2C adapter */
+	spin_lock_init(&bus->lock);
+	init_completion(&bus->cmd_complete);
+	bus->adap.owner = THIS_MODULE;
+	bus->adap.retries = 0;
+	bus->adap.timeout = 5 * HZ;
+	bus->adap.algo = &aspeed_i2c_algo;
+	bus->adap.algo_data = bus;
+	bus->adap.dev.parent = &pdev->dev;
+	bus->adap.dev.of_node = pdev->dev.of_node;
+	strlcpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
+
+	bus->dev = &pdev->dev;
+
+	/* Clean up any left over interrupt state. */
+	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
+	/*
+	 * bus.lock does not need to be held because the interrupt handler has
+	 * not been enabled yet.
+	 */
+	ret = aspeed_i2c_init(bus, pdev);
+	if (ret < 0)
+		return ret;
+
+	bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
+			       0, dev_name(&pdev->dev), bus);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_add_adapter(&bus->adap);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, bus);
+
+	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
+		 bus->adap.nr, bus->irq);
+
+	return 0;
+}
+
+static int aspeed_i2c_remove_bus(struct platform_device *pdev)
+{
+	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&bus->lock, flags);
+
+	/* Disable everything. */
+	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	i2c_del_adapter(&bus->adap);
+
+	return 0;
+}
+
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {
+	{ .compatible = "aspeed,ast2400-i2c-bus", },
+	{ .compatible = "aspeed,ast2500-i2c-bus", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
+
+static struct platform_driver aspeed_i2c_bus_driver = {
+	.probe		= aspeed_i2c_probe_bus,
+	.remove		= aspeed_i2c_remove_bus,
+	.driver		= {
+		.name		= "aspeed-i2c-bus",
+		.of_match_table	= aspeed_i2c_bus_of_table,
+	},
+};
+module_platform_driver(aspeed_i2c_bus_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.13.0.506.g27d5fe0cd-goog

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Joel Stanley]

On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> Added initial master 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>

Reviewed-by: Joel Stanley <joel@jms.id.au>

I have also tested this on an ast2500evb.

Cheers,

Joel

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Joel Stanley]

On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> Added initial master support for Aspeed I2C controller. Supports
> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
>
> Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Reviewed-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

I have also tested this on an ast2500evb.

Cheers,

Joel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 27716 bytes --]

On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
> Added initial master 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>

I exercised this patch on an AST2500-based machine. The results were
good, and the implementation copes fine with a workload that causes an
oops in our (older, unrelated) out-of-tree bus driver.

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

> ---
> Changes for v2:
>   - Added single module_init (multiple was breaking some builds).
> Changes for v3:
>   - Removed "bus" device tree param; now extracted from bus address
> offset
> Changes for v4:
>   - I2C adapter number is now generated dynamically unless specified
> in alias.
> Changes for v5:
>   - Removed irq_chip used to multiplex IRQ and replaced it with
> dummy_irq_chip
>     along with some other IRQ cleanup.
>   - Addressed comments from Cedric, and Vladimir, mostly stylistic
> things and
>     using devm managed resources.
>   - Increased max clock frequency before the bus is put in HighSpeed
> mode, as
>     per Kachalov's comment.
> Changes for v6:
>   - 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.
>   - 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 for v7:
>   - Changed clock-frequency to bus-frequency
>   - Made some fixes to clock divider code
>   - Added hardware reset function
>   - Marked functions that need to be called with the lock held as
> "unlocked"
>   - Did a bunch of clean up
> Changes for v8:
>   - ACK IRQ status bits before doing anything else
>   - Added multi-master device tree property
>   - Do not send STOP commands after interrupt errors
>   - Fix SMBUS_QUICK emulation handling
>   - Removed highspeed clock code (I will do it in a later patch set)
>   - Use the platform_device name for the adapter name
>   - Reset for all failed recoveries
>   - Removed the "__" prefix from all of the non-thread safe functions
> Changes for v9:
>   - No longer debug log after every address NACK
>   - Clear errors after reset
>   - No longer access clock while holding lock
>   - Ack all interrupts durring bus reset and initialization
> Changes for v10:
>   - Added master_xfer_result field to struct aspeed_i2c_bus
> ---
>  drivers/i2c/busses/Kconfig      |  10 +
>  drivers/i2c/busses/Makefile     |   1 +
>  drivers/i2c/busses/i2c-aspeed.c | 681
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 692 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-aspeed.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -326,6 +326,16 @@ config I2C_POWERMAC
>  
>  comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>  
> +config I2C_ASPEED
> +	tristate "Aspeed I2C Controller"
> +	depends on ARCH_ASPEED
> +	help
> +	  If you say yes to this option, support will be included
> for the
> +	  Aspeed I2C controller.
> +
> +	  This driver can also be built as a module.  If so, the
> module
> +	  will be called i2c-aspeed.
> +
>  config I2C_AT91
>  	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
>  	depends on ARCH_AT91
> diff --git a/drivers/i2c/busses/Makefile
> b/drivers/i2c/busses/Makefile
> index 30b60855fbcd..e84604b9bf3b 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-
> hydra.o
>  obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
>  
>  # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
>  obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
>  obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
>  obj-$(CONFIG_I2C_AXXIA)		+= i2c-axxia.o
> diff --git a/drivers/i2c/busses/i2c-aspeed.c
> b/drivers/i2c/busses/i2c-aspeed.c
> new file mode 100644
> index 000000000000..a04021e3b3ab
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -0,0 +1,681 @@
> +/*
> + *  Aspeed 24XX/25XX I2C Controller.
> + *
> + *  Copyright (C) 2012-2017 ASPEED Technology Inc.
> + *  Copyright 2017 IBM Corporation
> + *  Copyright 2017 Google, Inc.
> + *
> + *  This program is free software; you can redistribute it and/or
> modify
> + *  it under the terms of the GNU General Public License version 2
> as
> + *  published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/* I2C Register */
> +#define ASPEED_I2C_FUN_CTRL_REG				0x00
> +#define ASPEED_I2C_AC_TIMING_REG1			0x04
> +#define ASPEED_I2C_AC_TIMING_REG2			0x08
> +#define ASPEED_I2C_INTR_CTRL_REG			0x0c
> +#define ASPEED_I2C_INTR_STS_REG				0x10
> +#define ASPEED_I2C_CMD_REG				0x14
> +#define ASPEED_I2C_DEV_ADDR_REG				0x18
> +#define ASPEED_I2C_BYTE_BUF_REG				0x20
> +
> +/* Global Register Definition */
> +/* 0x00 : I2C Interrupt Status Register  */
> +/* 0x08 : I2C Interrupt Target Assignment  */
> +
> +/* Device Register Definition */
> +/* 0x00 : I2CD Function Control Register  */
> +#define ASPEED_I2CD_MULTI_MASTER_DIS			BIT(15)
> +#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_MASTER_EN				BIT(0)
> +
> +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
> +#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT			16
> +#define ASPEED_I2CD_TIME_SCL_HIGH_MASK			GENMAS
> K(19, 16)
> +#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT			12
> +#define ASPEED_I2CD_TIME_SCL_LOW_MASK			GENMASK
> (15, 12)
> +#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK		GENMASK(3,
> 0)
> +#define ASPEED_I2CD_TIME_SCL_REG_MAX			GENMASK(
> 3, 0)
> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
> +#define ASPEED_NO_TIMEOUT_CTRL				0
> +
> +/* 0x0c : I2CD Interrupt Control Register &
> + * 0x10 : I2CD Interrupt Status Register
> + *
> + * These share bit definitions, so use the same values for the
> enable &
> + * status bits.
> + */
> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(1
> 4)
> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
> +#define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
> +#define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
> +#define ASPEED_I2CD_INTR_ARBIT_LOSS			BIT(3)
> +#define ASPEED_I2CD_INTR_RX_DONE			BIT(2)
> +#define ASPEED_I2CD_INTR_TX_NAK				BIT(1
> )
> +#define ASPEED_I2CD_INTR_TX_ACK				BIT(0
> )
> +#define ASPEED_I2CD_INTR_ERROR					
> 	       \
> +		(ASPEED_I2CD_INTR_ARBIT_LOSS |			
> 	       \
> +		 ASPEED_I2CD_INTR_ABNORMAL |				
>        \
> +		 ASPEED_I2CD_INTR_SCL_TIMEOUT |			
> 	       \
> +		 ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
> +#define ASPEED_I2CD_INTR_ALL						
>        \
> +		(ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |			
>        \
> +		 ASPEED_I2CD_INTR_BUS_RECOVER_DONE |			
>        \
> +		 ASPEED_I2CD_INTR_SCL_TIMEOUT |			
> 	       \
> +		 ASPEED_I2CD_INTR_ABNORMAL |				
>        \
> +		 ASPEED_I2CD_INTR_NORMAL_STOP |			
> 	       \
> +		 ASPEED_I2CD_INTR_ARBIT_LOSS |			
> 	       \
> +		 ASPEED_I2CD_INTR_RX_DONE |				
>        \
> +		 ASPEED_I2CD_INTR_TX_NAK |				
>        \
> +		 ASPEED_I2CD_INTR_TX_ACK)
> +
> +/* 0x14 : I2CD Command/Status Register   */
> +#define ASPEED_I2CD_SCL_LINE_STS			BIT(18)
> +#define ASPEED_I2CD_SDA_LINE_STS			BIT(17)
> +#define ASPEED_I2CD_BUS_BUSY_STS			BIT(16)
> +#define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
> +
> +/* Command Bit */
> +#define ASPEED_I2CD_M_STOP_CMD				BIT(5)
> +#define ASPEED_I2CD_M_S_RX_CMD_LAST			BIT(4)
> +#define ASPEED_I2CD_M_RX_CMD				BIT(3)
> +#define ASPEED_I2CD_S_TX_CMD				BIT(2)
> +#define ASPEED_I2CD_M_TX_CMD				BIT(1)
> +#define ASPEED_I2CD_M_START_CMD				BIT(0
> )
> +
> +enum aspeed_i2c_master_state {
> +	ASPEED_I2C_MASTER_START,
> +	ASPEED_I2C_MASTER_TX_FIRST,
> +	ASPEED_I2C_MASTER_TX,
> +	ASPEED_I2C_MASTER_RX_FIRST,
> +	ASPEED_I2C_MASTER_RX,
> +	ASPEED_I2C_MASTER_STOP,
> +	ASPEED_I2C_MASTER_INACTIVE,
> +};
> +
> +struct aspeed_i2c_bus {
> +	struct i2c_adapter		adap;
> +	struct device			*dev;
> +	void __iomem			*base;
> +	/* Synchronizes I/O mem access to base. */
> +	spinlock_t			lock;
> +	struct completion		cmd_complete;
> +	int				irq;
> +	unsigned long			parent_clk_frequency;
> +	u32				bus_frequency;
> +	/* Transaction state. */
> +	enum aspeed_i2c_master_state	master_state;
> +	struct i2c_msg			*msgs;
> +	size_t				buf_index;
> +	size_t				msgs_index;
> +	size_t				msgs_count;
> +	bool				send_stop;
> +	int				cmd_err;
> +	/* Protected only by i2c_lock_bus */
> +	int				master_xfer_result;
> +};
> +
> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
> +
> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> +{
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +	u32 command;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	command = readl(bus->base + ASPEED_I2C_CMD_REG);
> +
> +	if (command & ASPEED_I2CD_SDA_LINE_STS) {
> +		/* Bus is idle: no recovery needed. */
> +		if (command & ASPEED_I2CD_SCL_LINE_STS)
> +			goto out;
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting
> recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
> ASPEED_I2C_CMD_REG);
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus-
> >adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SCL_LINE_STS))
> +			goto reset_out;
> +	/* Bus error. */
> +	} else {
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting
> recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_BUS_RECOVER_CMD,
> +		       bus->base + ASPEED_I2C_CMD_REG);
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus-
> >adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SDA_LINE_STS))
> +			goto reset_out;
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +
> +reset_out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return aspeed_i2c_reset(bus);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
> +{
> +	u32 command = ASPEED_I2CD_M_START_CMD |
> ASPEED_I2CD_M_TX_CMD;
> +	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
> +	u8 slave_addr = msg->addr << 1;
> +
> +	bus->master_state = ASPEED_I2C_MASTER_START;
> +	bus->buf_index = 0;
> +
> +	if (msg->flags & I2C_M_RD) {
> +		slave_addr |= 1;
> +		command |= ASPEED_I2CD_M_RX_CMD;
> +		/* Need to let the hardware know to NACK after RX.
> */
> +		if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
> +			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
> +	}
> +
> +	writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
> +	writel(command, bus->base + ASPEED_I2C_CMD_REG);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
> +{
> +	bus->master_state = ASPEED_I2C_MASTER_STOP;
> +	writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
> ASPEED_I2C_CMD_REG);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static void aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
> +{
> +	if (bus->msgs_index + 1 < bus->msgs_count) {
> +		bus->msgs_index++;
> +		aspeed_i2c_do_start(bus);
> +	} else {
> +		aspeed_i2c_do_stop(bus);
> +	}
> +}
> +
> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> +{
> +	u32 irq_status, status_ack = 0, command = 0;
> +	struct i2c_msg *msg;
> +	u8 recv_byte;
> +
> +	spin_lock(&bus->lock);
> +	irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
> +	/* Ack all interrupt bits. */
> +	writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
> +
> +	if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
> +		goto out_complete;
> +	}
> +
> +	/*
> +	 * We encountered an interrupt that reports an error: the
> hardware
> +	 * should clear the command queue effectively taking us back
> to the
> +	 * INACTIVE state.
> +	 */
> +	if (irq_status & ASPEED_I2CD_INTR_ERROR) {
> +		dev_dbg(bus->dev, "received error interrupt:
> 0x%08x",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		goto out_complete;
> +	}
> +
> +	/* We are in an invalid state; reset bus to a known state.
> */
> +	if (!bus->msgs && bus->master_state !=
> ASPEED_I2C_MASTER_STOP) {
> +		dev_err(bus->dev, "bus in unknown state");
> +		bus->cmd_err = -EIO;
> +		aspeed_i2c_do_stop(bus);
> +		goto out_no_complete;
> +	}
> +	msg = &bus->msgs[bus->msgs_index];
> +
> +	/*
> +	 * START is a special case because we still have to handle a
> subsequent
> +	 * TX or RX immediately after we handle it, so we handle it
> here and
> +	 * then update the state and handle the new state below.
> +	 */
> +	if (bus->master_state == ASPEED_I2C_MASTER_START) {
> +		if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_TX_ACK))) {
> +			pr_devel("no slave present at %02x", msg-
> >addr);
> +			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> +			goto error_and_stop;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> +		if (msg->len == 0) { /* SMBUS_QUICK */
> +			aspeed_i2c_do_stop(bus);
> +			goto out_no_complete;
> +		}
> +		if (msg->flags & I2C_M_RD)
> +			bus->master_state =
> ASPEED_I2C_MASTER_RX_FIRST;
> +		else
> +			bus->master_state =
> ASPEED_I2C_MASTER_TX_FIRST;
> +	}
> +
> +	switch (bus->master_state) {
> +	case ASPEED_I2C_MASTER_TX:
> +		if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK))
> {
> +			dev_dbg(bus->dev, "slave NACKed TX");
> +			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> +			goto error_and_stop;
> +		} else if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_TX_ACK))) {
> +			dev_err(bus->dev, "slave failed to ACK TX");
> +			goto error_and_stop;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> +		/* fallthrough intended */
> +	case ASPEED_I2C_MASTER_TX_FIRST:
> +		if (bus->buf_index < msg->len) {
> +			bus->master_state = ASPEED_I2C_MASTER_TX;
> +			writel(msg->buf[bus->buf_index++],
> +			       bus->base + ASPEED_I2C_BYTE_BUF_REG);
> +			writel(ASPEED_I2CD_M_TX_CMD,
> +			       bus->base + ASPEED_I2C_CMD_REG);
> +		} else {
> +			aspeed_i2c_next_msg_or_stop(bus);
> +		}
> +		goto out_no_complete;
> +	case ASPEED_I2C_MASTER_RX_FIRST:
> +		/* RX may not have completed yet (only address
> cycle) */
> +		if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
> +			goto out_no_complete;
> +		/* fallthrough intended */
> +	case ASPEED_I2C_MASTER_RX:
> +		if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_RX_DONE))) {
> +			dev_err(bus->dev, "master failed to RX");
> +			goto error_and_stop;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> +
> +		recv_byte = readl(bus->base +
> ASPEED_I2C_BYTE_BUF_REG) >> 8;
> +		msg->buf[bus->buf_index++] = recv_byte;
> +
> +		if (msg->flags & I2C_M_RECV_LEN) {
> +			if (unlikely(recv_byte >
> I2C_SMBUS_BLOCK_MAX)) {
> +				bus->cmd_err = -EPROTO;
> +				aspeed_i2c_do_stop(bus);
> +				goto out_no_complete;
> +			}
> +			msg->len = recv_byte +
> +					((msg->flags &
> I2C_CLIENT_PEC) ? 2 : 1);
> +			msg->flags &= ~I2C_M_RECV_LEN;
> +		}
> +
> +		if (bus->buf_index < msg->len) {
> +			bus->master_state = ASPEED_I2C_MASTER_RX;
> +			command = ASPEED_I2CD_M_RX_CMD;
> +			if (bus->buf_index + 1 == msg->len)
> +				command |=
> ASPEED_I2CD_M_S_RX_CMD_LAST;
> +			writel(command, bus->base +
> ASPEED_I2C_CMD_REG);
> +		} else {
> +			aspeed_i2c_next_msg_or_stop(bus);
> +		}
> +		goto out_no_complete;
> +	case ASPEED_I2C_MASTER_STOP:
> +		if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_NORMAL_STOP))) {
> +			dev_err(bus->dev, "master failed to STOP");
> +			bus->cmd_err = -EIO;
> +			/* Do not STOP as we have already tried. */
> +		} else {
> +			status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
> +		}
> +
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		goto out_complete;
> +	case ASPEED_I2C_MASTER_INACTIVE:
> +		dev_err(bus->dev,
> +			"master received interrupt 0x%08x, but is
> inactive",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		/* Do not STOP as we should be inactive. */
> +		goto out_complete;
> +	default:
> +		WARN(1, "unknown master state\n");
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		bus->cmd_err = -EIO;
> +		goto out_complete;
> +	}
> +error_and_stop:
> +	bus->cmd_err = -EIO;
> +	aspeed_i2c_do_stop(bus);
> +	goto out_no_complete;
> +out_complete:
> +	bus->msgs = NULL;
> +	if (bus->cmd_err)
> +		bus->master_xfer_result = bus->cmd_err;
> +	else
> +		bus->master_xfer_result = bus->msgs_index + 1;
> +	complete(&bus->cmd_complete);
> +out_no_complete:
> +	if (irq_status != status_ack)
> +		dev_err(bus->dev,
> +			"irq handled != irq. expected 0x%08x, but
> was 0x%08x\n",
> +			irq_status, status_ack);
> +	spin_unlock(&bus->lock);
> +	return !!irq_status;
> +}
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> +	struct aspeed_i2c_bus *bus = dev_id;
> +
> +	if (aspeed_i2c_master_irq(bus))
> +		return IRQ_HANDLED;
> +	else
> +		return IRQ_NONE;
> +}
> +
> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> +				  struct i2c_msg *msgs, int num)
> +{
> +	struct aspeed_i2c_bus *bus = adap->algo_data;
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	bus->cmd_err = 0;
> +
> +	/* If bus is busy, attempt recovery. We assume a single
> master
> +	 * environment.
> +	 */
> +	if (readl(bus->base + ASPEED_I2C_CMD_REG) &
> ASPEED_I2CD_BUS_BUSY_STS) {
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +		ret = aspeed_i2c_recover_bus(bus);
> +		if (ret)
> +			return ret;
> +		spin_lock_irqsave(&bus->lock, flags);
> +	}
> +
> +	bus->cmd_err = 0;
> +	bus->msgs = msgs;
> +	bus->msgs_index = 0;
> +	bus->msgs_count = num;
> +
> +	reinit_completion(&bus->cmd_complete);
> +	aspeed_i2c_do_start(bus);
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	time_left = wait_for_completion_timeout(&bus->cmd_complete,
> +						bus->adap.timeout);
> +
> +	if (time_left == 0)
> +		return -ETIMEDOUT;
> +	else
> +		return bus->master_xfer_result;
> +}
> +
> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
> +
> +static const struct i2c_algorithm aspeed_i2c_algo = {
> +	.master_xfer	= aspeed_i2c_master_xfer,
> +	.functionality	= aspeed_i2c_functionality,
> +};
> +
> +static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> +{
> +	u32 base_clk, clk_high, clk_low, tmp;
> +
> +	/*
> +	 * The actual clock frequency of SCL is:
> +	 *	SCL_freq = APB_freq / (base_freq * (SCL_high +
> SCL_low))
> +	 *		 = APB_freq / divisor
> +	 * where base_freq is a programmable clock divider; its
> value is
> +	 *	base_freq = 1 << base_clk
> +	 * SCL_high is the number of base_freq clock cycles that SCL
> stays high
> +	 * and SCL_low is the number of base_freq clock cycles that
> SCL stays
> +	 * low for a period of SCL.
> +	 * The actual register has a minimum SCL_high and SCL_low
> minimum of 1;
> +	 * thus, they start counting at zero. So
> +	 *	SCL_high = clk_high + 1
> +	 *	SCL_low	 = clk_low + 1
> +	 * Thus,
> +	 *	SCL_freq = APB_freq /
> +	 *		((1 << base_clk) * (clk_high + 1 + clk_low
> + 1))
> +	 * The documentation recommends clk_high >= 8 and clk_low >=
> 7 when
> +	 * possible; this last constraint gives us the following
> solution:
> +	 */
> +	base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
> +	tmp = divisor / (1 << base_clk);
> +	clk_high = tmp / 2 + tmp % 2;
> +	clk_low = tmp - clk_high;
> +
> +	clk_high -= 1;
> +	clk_low -= 1;
> +
> +	return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
> +		& ASPEED_I2CD_TIME_SCL_HIGH_MASK)
> +			| ((clk_low <<
> ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
> +			   & ASPEED_I2CD_TIME_SCL_LOW_MASK)
> +			| (base_clk &
> ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
> +{
> +	u32 divisor, clk_reg_val;
> +
> +	divisor = bus->parent_clk_frequency / bus->bus_frequency;
> +	clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
> +	writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
> +	writel(ASPEED_NO_TIMEOUT_CTRL, bus->base +
> ASPEED_I2C_AC_TIMING_REG2);
> +
> +	return 0;
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
> +			     struct platform_device *pdev)
> +{
> +	u32 fun_ctrl_reg = ASPEED_I2CD_MASTER_EN;
> +	int ret;
> +
> +	/* Disable everything. */
> +	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +
> +	ret = aspeed_i2c_init_clk(bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!of_property_read_bool(pdev->dev.of_node, "multi-
> master"))
> +		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
> +
> +	/* Enable Master Mode */
> +	writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) |
> fun_ctrl_reg,
> +	       bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +
> +	/* Set interrupt generation of I2C controller */
> +	writel(ASPEED_I2CD_INTR_ALL, bus->base +
> ASPEED_I2C_INTR_CTRL_REG);
> +
> +	return 0;
> +}
> +
> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
> +{
> +	struct platform_device *pdev = to_platform_device(bus->dev);
> +	unsigned long flags;
> +	int ret;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +
> +	/* Disable and ack all interrupts. */
> +	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> +	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
> +
> +	ret = aspeed_i2c_init(bus, pdev);
> +
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +}
> +
> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> +{
> +	struct aspeed_i2c_bus *bus;
> +	struct clk *parent_clk;
> +	struct resource *res;
> +	int ret;
> +
> +	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	bus->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(bus->base))
> +		return PTR_ERR(bus->base);
> +
> +	parent_clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(parent_clk))
> +		return PTR_ERR(parent_clk);
> +	bus->parent_clk_frequency = clk_get_rate(parent_clk);
> +	/* We just need the clock rate, we don't actually use the
> clk object. */
> +	devm_clk_put(&pdev->dev, parent_clk);
> +
> +	ret = of_property_read_u32(pdev->dev.of_node,
> +				   "bus-frequency", &bus-
> >bus_frequency);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not read bus-frequency property\n");
> +		bus->bus_frequency = 100000;
> +	}
> +
> +	/* Initialize the I2C adapter */
> +	spin_lock_init(&bus->lock);
> +	init_completion(&bus->cmd_complete);
> +	bus->adap.owner = THIS_MODULE;
> +	bus->adap.retries = 0;
> +	bus->adap.timeout = 5 * HZ;
> +	bus->adap.algo = &aspeed_i2c_algo;
> +	bus->adap.algo_data = bus;
> +	bus->adap.dev.parent = &pdev->dev;
> +	bus->adap.dev.of_node = pdev->dev.of_node;
> +	strlcpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
> +
> +	bus->dev = &pdev->dev;
> +
> +	/* Clean up any left over interrupt state. */
> +	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> +	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
> +	/*
> +	 * bus.lock does not need to be held because the interrupt
> handler has
> +	 * not been enabled yet.
> +	 */
> +	ret = aspeed_i2c_init(bus, pdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	ret = devm_request_irq(&pdev->dev, bus->irq,
> aspeed_i2c_bus_irq,
> +			       0, dev_name(&pdev->dev), bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_add_adapter(&bus->adap);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, bus);
> +
> +	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
> +		 bus->adap.nr, bus->irq);
> +
> +	return 0;
> +}
> +
> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
> +{
> +	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +
> +	/* Disable everything. */
> +	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> +
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	i2c_del_adapter(&bus->adap);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> +	{ .compatible = "aspeed,ast2400-i2c-bus", },
> +	{ .compatible = "aspeed,ast2500-i2c-bus", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> +
> +static struct platform_driver aspeed_i2c_bus_driver = {
> +	.probe		= aspeed_i2c_probe_bus,
> +	.remove		= aspeed_i2c_remove_bus,
> +	.driver		= {
> +		.name		= "aspeed-i2c-bus",
> +		.of_match_table	= aspeed_i2c_bus_of_table,
> +	},
> +};
> +module_platform_driver(aspeed_i2c_bus_driver);
> +
> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
> +MODULE_LICENSE("GPL v2");

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 27716 bytes --]

On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
> Added initial master 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>

I exercised this patch on an AST2500-based machine. The results were
good, and the implementation copes fine with a workload that causes an
oops in our (older, unrelated) out-of-tree bus driver.

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

> ---
> Changes for v2:
>   - Added single module_init (multiple was breaking some builds).
> Changes for v3:
>   - Removed "bus" device tree param; now extracted from bus address
> offset
> Changes for v4:
>   - I2C adapter number is now generated dynamically unless specified
> in alias.
> Changes for v5:
>   - Removed irq_chip used to multiplex IRQ and replaced it with
> dummy_irq_chip
>     along with some other IRQ cleanup.
>   - Addressed comments from Cedric, and Vladimir, mostly stylistic
> things and
>     using devm managed resources.
>   - Increased max clock frequency before the bus is put in HighSpeed
> mode, as
>     per Kachalov's comment.
> Changes for v6:
>   - 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.
>   - 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 for v7:
>   - Changed clock-frequency to bus-frequency
>   - Made some fixes to clock divider code
>   - Added hardware reset function
>   - Marked functions that need to be called with the lock held as
> "unlocked"
>   - Did a bunch of clean up
> Changes for v8:
>   - ACK IRQ status bits before doing anything else
>   - Added multi-master device tree property
>   - Do not send STOP commands after interrupt errors
>   - Fix SMBUS_QUICK emulation handling
>   - Removed highspeed clock code (I will do it in a later patch set)
>   - Use the platform_device name for the adapter name
>   - Reset for all failed recoveries
>   - Removed the "__" prefix from all of the non-thread safe functions
> Changes for v9:
>   - No longer debug log after every address NACK
>   - Clear errors after reset
>   - No longer access clock while holding lock
>   - Ack all interrupts durring bus reset and initialization
> Changes for v10:
>   - Added master_xfer_result field to struct aspeed_i2c_bus
> ---
>  drivers/i2c/busses/Kconfig      |  10 +
>  drivers/i2c/busses/Makefile     |   1 +
>  drivers/i2c/busses/i2c-aspeed.c | 681
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 692 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-aspeed.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -326,6 +326,16 @@ config I2C_POWERMAC
>  
>  comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>  
> +config I2C_ASPEED
> +	tristate "Aspeed I2C Controller"
> +	depends on ARCH_ASPEED
> +	help
> +	  If you say yes to this option, support will be included
> for the
> +	  Aspeed I2C controller.
> +
> +	  This driver can also be built as a module.  If so, the
> module
> +	  will be called i2c-aspeed.
> +
>  config I2C_AT91
>  	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
>  	depends on ARCH_AT91
> diff --git a/drivers/i2c/busses/Makefile
> b/drivers/i2c/busses/Makefile
> index 30b60855fbcd..e84604b9bf3b 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-
> hydra.o
>  obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
>  
>  # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
>  obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
>  obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
>  obj-$(CONFIG_I2C_AXXIA)		+= i2c-axxia.o
> diff --git a/drivers/i2c/busses/i2c-aspeed.c
> b/drivers/i2c/busses/i2c-aspeed.c
> new file mode 100644
> index 000000000000..a04021e3b3ab
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -0,0 +1,681 @@
> +/*
> + *  Aspeed 24XX/25XX I2C Controller.
> + *
> + *  Copyright (C) 2012-2017 ASPEED Technology Inc.
> + *  Copyright 2017 IBM Corporation
> + *  Copyright 2017 Google, Inc.
> + *
> + *  This program is free software; you can redistribute it and/or
> modify
> + *  it under the terms of the GNU General Public License version 2
> as
> + *  published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/* I2C Register */
> +#define ASPEED_I2C_FUN_CTRL_REG				0x00
> +#define ASPEED_I2C_AC_TIMING_REG1			0x04
> +#define ASPEED_I2C_AC_TIMING_REG2			0x08
> +#define ASPEED_I2C_INTR_CTRL_REG			0x0c
> +#define ASPEED_I2C_INTR_STS_REG				0x10
> +#define ASPEED_I2C_CMD_REG				0x14
> +#define ASPEED_I2C_DEV_ADDR_REG				0x18
> +#define ASPEED_I2C_BYTE_BUF_REG				0x20
> +
> +/* Global Register Definition */
> +/* 0x00 : I2C Interrupt Status Register  */
> +/* 0x08 : I2C Interrupt Target Assignment  */
> +
> +/* Device Register Definition */
> +/* 0x00 : I2CD Function Control Register  */
> +#define ASPEED_I2CD_MULTI_MASTER_DIS			BIT(15)
> +#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_MASTER_EN				BIT(0)
> +
> +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
> +#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT			16
> +#define ASPEED_I2CD_TIME_SCL_HIGH_MASK			GENMAS
> K(19, 16)
> +#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT			12
> +#define ASPEED_I2CD_TIME_SCL_LOW_MASK			GENMASK
> (15, 12)
> +#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK		GENMASK(3,
> 0)
> +#define ASPEED_I2CD_TIME_SCL_REG_MAX			GENMASK(
> 3, 0)
> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
> +#define ASPEED_NO_TIMEOUT_CTRL				0
> +
> +/* 0x0c : I2CD Interrupt Control Register &
> + * 0x10 : I2CD Interrupt Status Register
> + *
> + * These share bit definitions, so use the same values for the
> enable &
> + * status bits.
> + */
> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(1
> 4)
> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
> +#define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
> +#define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
> +#define ASPEED_I2CD_INTR_ARBIT_LOSS			BIT(3)
> +#define ASPEED_I2CD_INTR_RX_DONE			BIT(2)
> +#define ASPEED_I2CD_INTR_TX_NAK				BIT(1
> )
> +#define ASPEED_I2CD_INTR_TX_ACK				BIT(0
> )
> +#define ASPEED_I2CD_INTR_ERROR					
> 	       \
> +		(ASPEED_I2CD_INTR_ARBIT_LOSS |			
> 	       \
> +		 ASPEED_I2CD_INTR_ABNORMAL |				
>        \
> +		 ASPEED_I2CD_INTR_SCL_TIMEOUT |			
> 	       \
> +		 ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
> +#define ASPEED_I2CD_INTR_ALL						
>        \
> +		(ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |			
>        \
> +		 ASPEED_I2CD_INTR_BUS_RECOVER_DONE |			
>        \
> +		 ASPEED_I2CD_INTR_SCL_TIMEOUT |			
> 	       \
> +		 ASPEED_I2CD_INTR_ABNORMAL |				
>        \
> +		 ASPEED_I2CD_INTR_NORMAL_STOP |			
> 	       \
> +		 ASPEED_I2CD_INTR_ARBIT_LOSS |			
> 	       \
> +		 ASPEED_I2CD_INTR_RX_DONE |				
>        \
> +		 ASPEED_I2CD_INTR_TX_NAK |				
>        \
> +		 ASPEED_I2CD_INTR_TX_ACK)
> +
> +/* 0x14 : I2CD Command/Status Register   */
> +#define ASPEED_I2CD_SCL_LINE_STS			BIT(18)
> +#define ASPEED_I2CD_SDA_LINE_STS			BIT(17)
> +#define ASPEED_I2CD_BUS_BUSY_STS			BIT(16)
> +#define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
> +
> +/* Command Bit */
> +#define ASPEED_I2CD_M_STOP_CMD				BIT(5)
> +#define ASPEED_I2CD_M_S_RX_CMD_LAST			BIT(4)
> +#define ASPEED_I2CD_M_RX_CMD				BIT(3)
> +#define ASPEED_I2CD_S_TX_CMD				BIT(2)
> +#define ASPEED_I2CD_M_TX_CMD				BIT(1)
> +#define ASPEED_I2CD_M_START_CMD				BIT(0
> )
> +
> +enum aspeed_i2c_master_state {
> +	ASPEED_I2C_MASTER_START,
> +	ASPEED_I2C_MASTER_TX_FIRST,
> +	ASPEED_I2C_MASTER_TX,
> +	ASPEED_I2C_MASTER_RX_FIRST,
> +	ASPEED_I2C_MASTER_RX,
> +	ASPEED_I2C_MASTER_STOP,
> +	ASPEED_I2C_MASTER_INACTIVE,
> +};
> +
> +struct aspeed_i2c_bus {
> +	struct i2c_adapter		adap;
> +	struct device			*dev;
> +	void __iomem			*base;
> +	/* Synchronizes I/O mem access to base. */
> +	spinlock_t			lock;
> +	struct completion		cmd_complete;
> +	int				irq;
> +	unsigned long			parent_clk_frequency;
> +	u32				bus_frequency;
> +	/* Transaction state. */
> +	enum aspeed_i2c_master_state	master_state;
> +	struct i2c_msg			*msgs;
> +	size_t				buf_index;
> +	size_t				msgs_index;
> +	size_t				msgs_count;
> +	bool				send_stop;
> +	int				cmd_err;
> +	/* Protected only by i2c_lock_bus */
> +	int				master_xfer_result;
> +};
> +
> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
> +
> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> +{
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +	u32 command;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	command = readl(bus->base + ASPEED_I2C_CMD_REG);
> +
> +	if (command & ASPEED_I2CD_SDA_LINE_STS) {
> +		/* Bus is idle: no recovery needed. */
> +		if (command & ASPEED_I2CD_SCL_LINE_STS)
> +			goto out;
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting
> recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
> ASPEED_I2C_CMD_REG);
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus-
> >adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SCL_LINE_STS))
> +			goto reset_out;
> +	/* Bus error. */
> +	} else {
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting
> recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_BUS_RECOVER_CMD,
> +		       bus->base + ASPEED_I2C_CMD_REG);
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus-
> >adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SDA_LINE_STS))
> +			goto reset_out;
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +
> +reset_out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return aspeed_i2c_reset(bus);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
> +{
> +	u32 command = ASPEED_I2CD_M_START_CMD |
> ASPEED_I2CD_M_TX_CMD;
> +	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
> +	u8 slave_addr = msg->addr << 1;
> +
> +	bus->master_state = ASPEED_I2C_MASTER_START;
> +	bus->buf_index = 0;
> +
> +	if (msg->flags & I2C_M_RD) {
> +		slave_addr |= 1;
> +		command |= ASPEED_I2CD_M_RX_CMD;
> +		/* Need to let the hardware know to NACK after RX.
> */
> +		if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
> +			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
> +	}
> +
> +	writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
> +	writel(command, bus->base + ASPEED_I2C_CMD_REG);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
> +{
> +	bus->master_state = ASPEED_I2C_MASTER_STOP;
> +	writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
> ASPEED_I2C_CMD_REG);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static void aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
> +{
> +	if (bus->msgs_index + 1 < bus->msgs_count) {
> +		bus->msgs_index++;
> +		aspeed_i2c_do_start(bus);
> +	} else {
> +		aspeed_i2c_do_stop(bus);
> +	}
> +}
> +
> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> +{
> +	u32 irq_status, status_ack = 0, command = 0;
> +	struct i2c_msg *msg;
> +	u8 recv_byte;
> +
> +	spin_lock(&bus->lock);
> +	irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
> +	/* Ack all interrupt bits. */
> +	writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
> +
> +	if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
> +		goto out_complete;
> +	}
> +
> +	/*
> +	 * We encountered an interrupt that reports an error: the
> hardware
> +	 * should clear the command queue effectively taking us back
> to the
> +	 * INACTIVE state.
> +	 */
> +	if (irq_status & ASPEED_I2CD_INTR_ERROR) {
> +		dev_dbg(bus->dev, "received error interrupt:
> 0x%08x",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		goto out_complete;
> +	}
> +
> +	/* We are in an invalid state; reset bus to a known state.
> */
> +	if (!bus->msgs && bus->master_state !=
> ASPEED_I2C_MASTER_STOP) {
> +		dev_err(bus->dev, "bus in unknown state");
> +		bus->cmd_err = -EIO;
> +		aspeed_i2c_do_stop(bus);
> +		goto out_no_complete;
> +	}
> +	msg = &bus->msgs[bus->msgs_index];
> +
> +	/*
> +	 * START is a special case because we still have to handle a
> subsequent
> +	 * TX or RX immediately after we handle it, so we handle it
> here and
> +	 * then update the state and handle the new state below.
> +	 */
> +	if (bus->master_state == ASPEED_I2C_MASTER_START) {
> +		if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_TX_ACK))) {
> +			pr_devel("no slave present at %02x", msg-
> >addr);
> +			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> +			goto error_and_stop;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> +		if (msg->len == 0) { /* SMBUS_QUICK */
> +			aspeed_i2c_do_stop(bus);
> +			goto out_no_complete;
> +		}
> +		if (msg->flags & I2C_M_RD)
> +			bus->master_state =
> ASPEED_I2C_MASTER_RX_FIRST;
> +		else
> +			bus->master_state =
> ASPEED_I2C_MASTER_TX_FIRST;
> +	}
> +
> +	switch (bus->master_state) {
> +	case ASPEED_I2C_MASTER_TX:
> +		if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK))
> {
> +			dev_dbg(bus->dev, "slave NACKed TX");
> +			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> +			goto error_and_stop;
> +		} else if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_TX_ACK))) {
> +			dev_err(bus->dev, "slave failed to ACK TX");
> +			goto error_and_stop;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> +		/* fallthrough intended */
> +	case ASPEED_I2C_MASTER_TX_FIRST:
> +		if (bus->buf_index < msg->len) {
> +			bus->master_state = ASPEED_I2C_MASTER_TX;
> +			writel(msg->buf[bus->buf_index++],
> +			       bus->base + ASPEED_I2C_BYTE_BUF_REG);
> +			writel(ASPEED_I2CD_M_TX_CMD,
> +			       bus->base + ASPEED_I2C_CMD_REG);
> +		} else {
> +			aspeed_i2c_next_msg_or_stop(bus);
> +		}
> +		goto out_no_complete;
> +	case ASPEED_I2C_MASTER_RX_FIRST:
> +		/* RX may not have completed yet (only address
> cycle) */
> +		if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
> +			goto out_no_complete;
> +		/* fallthrough intended */
> +	case ASPEED_I2C_MASTER_RX:
> +		if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_RX_DONE))) {
> +			dev_err(bus->dev, "master failed to RX");
> +			goto error_and_stop;
> +		}
> +		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> +
> +		recv_byte = readl(bus->base +
> ASPEED_I2C_BYTE_BUF_REG) >> 8;
> +		msg->buf[bus->buf_index++] = recv_byte;
> +
> +		if (msg->flags & I2C_M_RECV_LEN) {
> +			if (unlikely(recv_byte >
> I2C_SMBUS_BLOCK_MAX)) {
> +				bus->cmd_err = -EPROTO;
> +				aspeed_i2c_do_stop(bus);
> +				goto out_no_complete;
> +			}
> +			msg->len = recv_byte +
> +					((msg->flags &
> I2C_CLIENT_PEC) ? 2 : 1);
> +			msg->flags &= ~I2C_M_RECV_LEN;
> +		}
> +
> +		if (bus->buf_index < msg->len) {
> +			bus->master_state = ASPEED_I2C_MASTER_RX;
> +			command = ASPEED_I2CD_M_RX_CMD;
> +			if (bus->buf_index + 1 == msg->len)
> +				command |=
> ASPEED_I2CD_M_S_RX_CMD_LAST;
> +			writel(command, bus->base +
> ASPEED_I2C_CMD_REG);
> +		} else {
> +			aspeed_i2c_next_msg_or_stop(bus);
> +		}
> +		goto out_no_complete;
> +	case ASPEED_I2C_MASTER_STOP:
> +		if (unlikely(!(irq_status &
> ASPEED_I2CD_INTR_NORMAL_STOP))) {
> +			dev_err(bus->dev, "master failed to STOP");
> +			bus->cmd_err = -EIO;
> +			/* Do not STOP as we have already tried. */
> +		} else {
> +			status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
> +		}
> +
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		goto out_complete;
> +	case ASPEED_I2C_MASTER_INACTIVE:
> +		dev_err(bus->dev,
> +			"master received interrupt 0x%08x, but is
> inactive",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		/* Do not STOP as we should be inactive. */
> +		goto out_complete;
> +	default:
> +		WARN(1, "unknown master state\n");
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		bus->cmd_err = -EIO;
> +		goto out_complete;
> +	}
> +error_and_stop:
> +	bus->cmd_err = -EIO;
> +	aspeed_i2c_do_stop(bus);
> +	goto out_no_complete;
> +out_complete:
> +	bus->msgs = NULL;
> +	if (bus->cmd_err)
> +		bus->master_xfer_result = bus->cmd_err;
> +	else
> +		bus->master_xfer_result = bus->msgs_index + 1;
> +	complete(&bus->cmd_complete);
> +out_no_complete:
> +	if (irq_status != status_ack)
> +		dev_err(bus->dev,
> +			"irq handled != irq. expected 0x%08x, but
> was 0x%08x\n",
> +			irq_status, status_ack);
> +	spin_unlock(&bus->lock);
> +	return !!irq_status;
> +}
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> +	struct aspeed_i2c_bus *bus = dev_id;
> +
> +	if (aspeed_i2c_master_irq(bus))
> +		return IRQ_HANDLED;
> +	else
> +		return IRQ_NONE;
> +}
> +
> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> +				  struct i2c_msg *msgs, int num)
> +{
> +	struct aspeed_i2c_bus *bus = adap->algo_data;
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	bus->cmd_err = 0;
> +
> +	/* If bus is busy, attempt recovery. We assume a single
> master
> +	 * environment.
> +	 */
> +	if (readl(bus->base + ASPEED_I2C_CMD_REG) &
> ASPEED_I2CD_BUS_BUSY_STS) {
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +		ret = aspeed_i2c_recover_bus(bus);
> +		if (ret)
> +			return ret;
> +		spin_lock_irqsave(&bus->lock, flags);
> +	}
> +
> +	bus->cmd_err = 0;
> +	bus->msgs = msgs;
> +	bus->msgs_index = 0;
> +	bus->msgs_count = num;
> +
> +	reinit_completion(&bus->cmd_complete);
> +	aspeed_i2c_do_start(bus);
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	time_left = wait_for_completion_timeout(&bus->cmd_complete,
> +						bus->adap.timeout);
> +
> +	if (time_left == 0)
> +		return -ETIMEDOUT;
> +	else
> +		return bus->master_xfer_result;
> +}
> +
> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
> +
> +static const struct i2c_algorithm aspeed_i2c_algo = {
> +	.master_xfer	= aspeed_i2c_master_xfer,
> +	.functionality	= aspeed_i2c_functionality,
> +};
> +
> +static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> +{
> +	u32 base_clk, clk_high, clk_low, tmp;
> +
> +	/*
> +	 * The actual clock frequency of SCL is:
> +	 *	SCL_freq = APB_freq / (base_freq * (SCL_high +
> SCL_low))
> +	 *		 = APB_freq / divisor
> +	 * where base_freq is a programmable clock divider; its
> value is
> +	 *	base_freq = 1 << base_clk
> +	 * SCL_high is the number of base_freq clock cycles that SCL
> stays high
> +	 * and SCL_low is the number of base_freq clock cycles that
> SCL stays
> +	 * low for a period of SCL.
> +	 * The actual register has a minimum SCL_high and SCL_low
> minimum of 1;
> +	 * thus, they start counting at zero. So
> +	 *	SCL_high = clk_high + 1
> +	 *	SCL_low	 = clk_low + 1
> +	 * Thus,
> +	 *	SCL_freq = APB_freq /
> +	 *		((1 << base_clk) * (clk_high + 1 + clk_low
> + 1))
> +	 * The documentation recommends clk_high >= 8 and clk_low >=
> 7 when
> +	 * possible; this last constraint gives us the following
> solution:
> +	 */
> +	base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
> +	tmp = divisor / (1 << base_clk);
> +	clk_high = tmp / 2 + tmp % 2;
> +	clk_low = tmp - clk_high;
> +
> +	clk_high -= 1;
> +	clk_low -= 1;
> +
> +	return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
> +		& ASPEED_I2CD_TIME_SCL_HIGH_MASK)
> +			| ((clk_low <<
> ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
> +			   & ASPEED_I2CD_TIME_SCL_LOW_MASK)
> +			| (base_clk &
> ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
> +{
> +	u32 divisor, clk_reg_val;
> +
> +	divisor = bus->parent_clk_frequency / bus->bus_frequency;
> +	clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
> +	writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
> +	writel(ASPEED_NO_TIMEOUT_CTRL, bus->base +
> ASPEED_I2C_AC_TIMING_REG2);
> +
> +	return 0;
> +}
> +
> +/* precondition: bus.lock has been acquired. */
> +static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
> +			     struct platform_device *pdev)
> +{
> +	u32 fun_ctrl_reg = ASPEED_I2CD_MASTER_EN;
> +	int ret;
> +
> +	/* Disable everything. */
> +	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +
> +	ret = aspeed_i2c_init_clk(bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!of_property_read_bool(pdev->dev.of_node, "multi-
> master"))
> +		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
> +
> +	/* Enable Master Mode */
> +	writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) |
> fun_ctrl_reg,
> +	       bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +
> +	/* Set interrupt generation of I2C controller */
> +	writel(ASPEED_I2CD_INTR_ALL, bus->base +
> ASPEED_I2C_INTR_CTRL_REG);
> +
> +	return 0;
> +}
> +
> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
> +{
> +	struct platform_device *pdev = to_platform_device(bus->dev);
> +	unsigned long flags;
> +	int ret;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +
> +	/* Disable and ack all interrupts. */
> +	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> +	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
> +
> +	ret = aspeed_i2c_init(bus, pdev);
> +
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +}
> +
> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> +{
> +	struct aspeed_i2c_bus *bus;
> +	struct clk *parent_clk;
> +	struct resource *res;
> +	int ret;
> +
> +	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	bus->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(bus->base))
> +		return PTR_ERR(bus->base);
> +
> +	parent_clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(parent_clk))
> +		return PTR_ERR(parent_clk);
> +	bus->parent_clk_frequency = clk_get_rate(parent_clk);
> +	/* We just need the clock rate, we don't actually use the
> clk object. */
> +	devm_clk_put(&pdev->dev, parent_clk);
> +
> +	ret = of_property_read_u32(pdev->dev.of_node,
> +				   "bus-frequency", &bus-
> >bus_frequency);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Could not read bus-frequency property\n");
> +		bus->bus_frequency = 100000;
> +	}
> +
> +	/* Initialize the I2C adapter */
> +	spin_lock_init(&bus->lock);
> +	init_completion(&bus->cmd_complete);
> +	bus->adap.owner = THIS_MODULE;
> +	bus->adap.retries = 0;
> +	bus->adap.timeout = 5 * HZ;
> +	bus->adap.algo = &aspeed_i2c_algo;
> +	bus->adap.algo_data = bus;
> +	bus->adap.dev.parent = &pdev->dev;
> +	bus->adap.dev.of_node = pdev->dev.of_node;
> +	strlcpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
> +
> +	bus->dev = &pdev->dev;
> +
> +	/* Clean up any left over interrupt state. */
> +	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> +	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
> +	/*
> +	 * bus.lock does not need to be held because the interrupt
> handler has
> +	 * not been enabled yet.
> +	 */
> +	ret = aspeed_i2c_init(bus, pdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +	ret = devm_request_irq(&pdev->dev, bus->irq,
> aspeed_i2c_bus_irq,
> +			       0, dev_name(&pdev->dev), bus);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_add_adapter(&bus->adap);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, bus);
> +
> +	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
> +		 bus->adap.nr, bus->irq);
> +
> +	return 0;
> +}
> +
> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
> +{
> +	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +
> +	/* Disable everything. */
> +	writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
> +
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	i2c_del_adapter(&bus->adap);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> +	{ .compatible = "aspeed,ast2400-i2c-bus", },
> +	{ .compatible = "aspeed,ast2500-i2c-bus", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> +
> +static struct platform_driver aspeed_i2c_bus_driver = {
> +	.probe		= aspeed_i2c_probe_bus,
> +	.remove		= aspeed_i2c_remove_bus,
> +	.driver		= {
> +		.name		= "aspeed-i2c-bus",
> +		.of_match_table	= aspeed_i2c_bus_of_table,
> +	},
> +};
> +module_platform_driver(aspeed_i2c_bus_driver);
> +
> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
> +MODULE_LICENSE("GPL v2");

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

On Sun, Jun 4, 2017 at 10:02 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
>> Added initial master 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>
>
> I exercised this patch on an AST2500-based machine. The results were
> good, and the implementation copes fine with a workload that causes an
> oops in our (older, unrelated) out-of-tree bus driver.
>
> Tested-by: Andrew Jeffery <andrew@aj.id.au>

Wolfram, could you take a look at these patches? Ben, Joel and others
seem to be happy with it and Marc said he would take the irqchip
stuff.

Cheers

>
>> ---
>> Changes for v2:
>>   - Added single module_init (multiple was breaking some builds).
>> Changes for v3:
>>   - Removed "bus" device tree param; now extracted from bus address
>> offset
>> Changes for v4:
>>   - I2C adapter number is now generated dynamically unless specified
>> in alias.
>> Changes for v5:
>>   - Removed irq_chip used to multiplex IRQ and replaced it with
>> dummy_irq_chip
>>     along with some other IRQ cleanup.
>>   - Addressed comments from Cedric, and Vladimir, mostly stylistic
>> things and
>>     using devm managed resources.
>>   - Increased max clock frequency before the bus is put in HighSpeed
>> mode, as
>>     per Kachalov's comment.
>> Changes for v6:
>>   - 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.
>>   - 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 for v7:
>>   - Changed clock-frequency to bus-frequency
>>   - Made some fixes to clock divider code
>>   - Added hardware reset function
>>   - Marked functions that need to be called with the lock held as
>> "unlocked"
>>   - Did a bunch of clean up
>> Changes for v8:
>>   - ACK IRQ status bits before doing anything else
>>   - Added multi-master device tree property
>>   - Do not send STOP commands after interrupt errors
>>   - Fix SMBUS_QUICK emulation handling
>>   - Removed highspeed clock code (I will do it in a later patch set)
>>   - Use the platform_device name for the adapter name
>>   - Reset for all failed recoveries
>>   - Removed the "__" prefix from all of the non-thread safe functions
>> Changes for v9:
>>   - No longer debug log after every address NACK
>>   - Clear errors after reset
>>   - No longer access clock while holding lock
>>   - Ack all interrupts durring bus reset and initialization
>> Changes for v10:
>>   - Added master_xfer_result field to struct aspeed_i2c_bus
>> ---
>>  drivers/i2c/busses/Kconfig      |  10 +
>>  drivers/i2c/busses/Makefile     |   1 +
>>  drivers/i2c/busses/i2c-aspeed.c | 681
>> ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 692 insertions(+)
>>  create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 144cbadc7c72..280f84a0d7d1 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -326,6 +326,16 @@ config I2C_POWERMAC
>>
>>  comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>>
>> +config I2C_ASPEED
>> +     tristate "Aspeed I2C Controller"
>> +     depends on ARCH_ASPEED
>> +     help
>> +       If you say yes to this option, support will be included
>> for the
>> +       Aspeed I2C controller.
>> +
>> +       This driver can also be built as a module.  If so, the
>> module
>> +       will be called i2c-aspeed.
>> +
>>  config I2C_AT91
>>       tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
>>       depends on ARCH_AT91
>> diff --git a/drivers/i2c/busses/Makefile
>> b/drivers/i2c/busses/Makefile
>> index 30b60855fbcd..e84604b9bf3b 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA)             += i2c-
>> hydra.o
>>  obj-$(CONFIG_I2C_POWERMAC)   += i2c-powermac.o
>>
>>  # Embedded system I2C/SMBus host controller drivers
>> +obj-$(CONFIG_I2C_ASPEED)     += i2c-aspeed.o
>>  obj-$(CONFIG_I2C_AT91)               += i2c-at91.o
>>  obj-$(CONFIG_I2C_AU1550)     += i2c-au1550.o
>>  obj-$(CONFIG_I2C_AXXIA)              += i2c-axxia.o
>> diff --git a/drivers/i2c/busses/i2c-aspeed.c
>> b/drivers/i2c/busses/i2c-aspeed.c
>> new file mode 100644
>> index 000000000000..a04021e3b3ab
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -0,0 +1,681 @@
>> +/*
>> + *  Aspeed 24XX/25XX I2C Controller.
>> + *
>> + *  Copyright (C) 2012-2017 ASPEED Technology Inc.
>> + *  Copyright 2017 IBM Corporation
>> + *  Copyright 2017 Google, Inc.
>> + *
>> + *  This program is free software; you can redistribute it and/or
>> modify
>> + *  it under the terms of the GNU General Public License version 2
>> as
>> + *  published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/completion.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +/* I2C Register */
>> +#define ASPEED_I2C_FUN_CTRL_REG                              0x00
>> +#define ASPEED_I2C_AC_TIMING_REG1                    0x04
>> +#define ASPEED_I2C_AC_TIMING_REG2                    0x08
>> +#define ASPEED_I2C_INTR_CTRL_REG                     0x0c
>> +#define ASPEED_I2C_INTR_STS_REG                              0x10
>> +#define ASPEED_I2C_CMD_REG                           0x14
>> +#define ASPEED_I2C_DEV_ADDR_REG                              0x18
>> +#define ASPEED_I2C_BYTE_BUF_REG                              0x20
>> +
>> +/* Global Register Definition */
>> +/* 0x00 : I2C Interrupt Status Register  */
>> +/* 0x08 : I2C Interrupt Target Assignment  */
>> +
>> +/* Device Register Definition */
>> +/* 0x00 : I2CD Function Control Register  */
>> +#define ASPEED_I2CD_MULTI_MASTER_DIS                 BIT(15)
>> +#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_MASTER_EN                                BIT(0)
>> +
>> +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
>> +#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT                      16
>> +#define ASPEED_I2CD_TIME_SCL_HIGH_MASK                       GENMAS
>> K(19, 16)
>> +#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT                       12
>> +#define ASPEED_I2CD_TIME_SCL_LOW_MASK                        GENMASK
>> (15, 12)
>> +#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK           GENMASK(3,
>> 0)
>> +#define ASPEED_I2CD_TIME_SCL_REG_MAX                 GENMASK(
>> 3, 0)
>> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
>> +#define ASPEED_NO_TIMEOUT_CTRL                               0
>> +
>> +/* 0x0c : I2CD Interrupt Control Register &
>> + * 0x10 : I2CD Interrupt Status Register
>> + *
>> + * These share bit definitions, so use the same values for the
>> enable &
>> + * status bits.
>> + */
>> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT                      BIT(1
>> 4)
>> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE            BIT(13)
>> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT                 BIT(6)
>> +#define ASPEED_I2CD_INTR_ABNORMAL                    BIT(5)
>> +#define ASPEED_I2CD_INTR_NORMAL_STOP                 BIT(4)
>> +#define ASPEED_I2CD_INTR_ARBIT_LOSS                  BIT(3)
>> +#define ASPEED_I2CD_INTR_RX_DONE                     BIT(2)
>> +#define ASPEED_I2CD_INTR_TX_NAK                              BIT(1
>> )
>> +#define ASPEED_I2CD_INTR_TX_ACK                              BIT(0
>> )
>> +#define ASPEED_I2CD_INTR_ERROR
>>              \
>> +             (ASPEED_I2CD_INTR_ARBIT_LOSS |
>>              \
>> +              ASPEED_I2CD_INTR_ABNORMAL |
>>        \
>> +              ASPEED_I2CD_INTR_SCL_TIMEOUT |
>>              \
>> +              ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
>> +#define ASPEED_I2CD_INTR_ALL
>>        \
>> +             (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
>>        \
>> +              ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
>>        \
>> +              ASPEED_I2CD_INTR_SCL_TIMEOUT |
>>              \
>> +              ASPEED_I2CD_INTR_ABNORMAL |
>>        \
>> +              ASPEED_I2CD_INTR_NORMAL_STOP |
>>              \
>> +              ASPEED_I2CD_INTR_ARBIT_LOSS |
>>              \
>> +              ASPEED_I2CD_INTR_RX_DONE |
>>        \
>> +              ASPEED_I2CD_INTR_TX_NAK |
>>        \
>> +              ASPEED_I2CD_INTR_TX_ACK)
>> +
>> +/* 0x14 : I2CD Command/Status Register   */
>> +#define ASPEED_I2CD_SCL_LINE_STS                     BIT(18)
>> +#define ASPEED_I2CD_SDA_LINE_STS                     BIT(17)
>> +#define ASPEED_I2CD_BUS_BUSY_STS                     BIT(16)
>> +#define ASPEED_I2CD_BUS_RECOVER_CMD                  BIT(11)
>> +
>> +/* Command Bit */
>> +#define ASPEED_I2CD_M_STOP_CMD                               BIT(5)
>> +#define ASPEED_I2CD_M_S_RX_CMD_LAST                  BIT(4)
>> +#define ASPEED_I2CD_M_RX_CMD                         BIT(3)
>> +#define ASPEED_I2CD_S_TX_CMD                         BIT(2)
>> +#define ASPEED_I2CD_M_TX_CMD                         BIT(1)
>> +#define ASPEED_I2CD_M_START_CMD                              BIT(0
>> )
>> +
>> +enum aspeed_i2c_master_state {
>> +     ASPEED_I2C_MASTER_START,
>> +     ASPEED_I2C_MASTER_TX_FIRST,
>> +     ASPEED_I2C_MASTER_TX,
>> +     ASPEED_I2C_MASTER_RX_FIRST,
>> +     ASPEED_I2C_MASTER_RX,
>> +     ASPEED_I2C_MASTER_STOP,
>> +     ASPEED_I2C_MASTER_INACTIVE,
>> +};
>> +
>> +struct aspeed_i2c_bus {
>> +     struct i2c_adapter              adap;
>> +     struct device                   *dev;
>> +     void __iomem                    *base;
>> +     /* Synchronizes I/O mem access to base. */
>> +     spinlock_t                      lock;
>> +     struct completion               cmd_complete;
>> +     int                             irq;
>> +     unsigned long                   parent_clk_frequency;
>> +     u32                             bus_frequency;
>> +     /* Transaction state. */
>> +     enum aspeed_i2c_master_state    master_state;
>> +     struct i2c_msg                  *msgs;
>> +     size_t                          buf_index;
>> +     size_t                          msgs_index;
>> +     size_t                          msgs_count;
>> +     bool                            send_stop;
>> +     int                             cmd_err;
>> +     /* Protected only by i2c_lock_bus */
>> +     int                             master_xfer_result;
>> +};
>> +
>> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
>> +
>> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>> +{
>> +     unsigned long time_left, flags;
>> +     int ret = 0;
>> +     u32 command;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +     command = readl(bus->base + ASPEED_I2C_CMD_REG);
>> +
>> +     if (command & ASPEED_I2CD_SDA_LINE_STS) {
>> +             /* Bus is idle: no recovery needed. */
>> +             if (command & ASPEED_I2CD_SCL_LINE_STS)
>> +                     goto out;
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting
>> recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
>> ASPEED_I2C_CMD_REG);
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus-
>> >adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SCL_LINE_STS))
>> +                     goto reset_out;
>> +     /* Bus error. */
>> +     } else {
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting
>> recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_BUS_RECOVER_CMD,
>> +                    bus->base + ASPEED_I2C_CMD_REG);
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus-
>> >adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SDA_LINE_STS))
>> +                     goto reset_out;
>> +     }
>> +
>> +out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return ret;
>> +
>> +reset_out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return aspeed_i2c_reset(bus);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
>> +{
>> +     u32 command = ASPEED_I2CD_M_START_CMD |
>> ASPEED_I2CD_M_TX_CMD;
>> +     struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
>> +     u8 slave_addr = msg->addr << 1;
>> +
>> +     bus->master_state = ASPEED_I2C_MASTER_START;
>> +     bus->buf_index = 0;
>> +
>> +     if (msg->flags & I2C_M_RD) {
>> +             slave_addr |= 1;
>> +             command |= ASPEED_I2CD_M_RX_CMD;
>> +             /* Need to let the hardware know to NACK after RX.
>> */
>> +             if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
>> +                     command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
>> +     }
>> +
>> +     writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
>> +     writel(command, bus->base + ASPEED_I2C_CMD_REG);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
>> +{
>> +     bus->master_state = ASPEED_I2C_MASTER_STOP;
>> +     writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
>> ASPEED_I2C_CMD_REG);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static void aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
>> +{
>> +     if (bus->msgs_index + 1 < bus->msgs_count) {
>> +             bus->msgs_index++;
>> +             aspeed_i2c_do_start(bus);
>> +     } else {
>> +             aspeed_i2c_do_stop(bus);
>> +     }
>> +}
>> +
>> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> +{
>> +     u32 irq_status, status_ack = 0, command = 0;
>> +     struct i2c_msg *msg;
>> +     u8 recv_byte;
>> +
>> +     spin_lock(&bus->lock);
>> +     irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
>> +     /* Ack all interrupt bits. */
>> +     writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>> +
>> +     if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
>> +             goto out_complete;
>> +     }
>> +
>> +     /*
>> +      * We encountered an interrupt that reports an error: the
>> hardware
>> +      * should clear the command queue effectively taking us back
>> to the
>> +      * INACTIVE state.
>> +      */
>> +     if (irq_status & ASPEED_I2CD_INTR_ERROR) {
>> +             dev_dbg(bus->dev, "received error interrupt:
>> 0x%08x",
>> +                     irq_status);
>> +             bus->cmd_err = -EIO;
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             goto out_complete;
>> +     }
>> +
>> +     /* We are in an invalid state; reset bus to a known state.
>> */
>> +     if (!bus->msgs && bus->master_state !=
>> ASPEED_I2C_MASTER_STOP) {
>> +             dev_err(bus->dev, "bus in unknown state");
>> +             bus->cmd_err = -EIO;
>> +             aspeed_i2c_do_stop(bus);
>> +             goto out_no_complete;
>> +     }
>> +     msg = &bus->msgs[bus->msgs_index];
>> +
>> +     /*
>> +      * START is a special case because we still have to handle a
>> subsequent
>> +      * TX or RX immediately after we handle it, so we handle it
>> here and
>> +      * then update the state and handle the new state below.
>> +      */
>> +     if (bus->master_state == ASPEED_I2C_MASTER_START) {
>> +             if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_TX_ACK))) {
>> +                     pr_devel("no slave present at %02x", msg-
>> >addr);
>> +                     status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> +                     goto error_and_stop;
>> +             }
>> +             status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> +             if (msg->len == 0) { /* SMBUS_QUICK */
>> +                     aspeed_i2c_do_stop(bus);
>> +                     goto out_no_complete;
>> +             }
>> +             if (msg->flags & I2C_M_RD)
>> +                     bus->master_state =
>> ASPEED_I2C_MASTER_RX_FIRST;
>> +             else
>> +                     bus->master_state =
>> ASPEED_I2C_MASTER_TX_FIRST;
>> +     }
>> +
>> +     switch (bus->master_state) {
>> +     case ASPEED_I2C_MASTER_TX:
>> +             if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK))
>> {
>> +                     dev_dbg(bus->dev, "slave NACKed TX");
>> +                     status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> +                     goto error_and_stop;
>> +             } else if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_TX_ACK))) {
>> +                     dev_err(bus->dev, "slave failed to ACK TX");
>> +                     goto error_and_stop;
>> +             }
>> +             status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> +             /* fallthrough intended */
>> +     case ASPEED_I2C_MASTER_TX_FIRST:
>> +             if (bus->buf_index < msg->len) {
>> +                     bus->master_state = ASPEED_I2C_MASTER_TX;
>> +                     writel(msg->buf[bus->buf_index++],
>> +                            bus->base + ASPEED_I2C_BYTE_BUF_REG);
>> +                     writel(ASPEED_I2CD_M_TX_CMD,
>> +                            bus->base + ASPEED_I2C_CMD_REG);
>> +             } else {
>> +                     aspeed_i2c_next_msg_or_stop(bus);
>> +             }
>> +             goto out_no_complete;
>> +     case ASPEED_I2C_MASTER_RX_FIRST:
>> +             /* RX may not have completed yet (only address
>> cycle) */
>> +             if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
>> +                     goto out_no_complete;
>> +             /* fallthrough intended */
>> +     case ASPEED_I2C_MASTER_RX:
>> +             if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_RX_DONE))) {
>> +                     dev_err(bus->dev, "master failed to RX");
>> +                     goto error_and_stop;
>> +             }
>> +             status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> +
>> +             recv_byte = readl(bus->base +
>> ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> +             msg->buf[bus->buf_index++] = recv_byte;
>> +
>> +             if (msg->flags & I2C_M_RECV_LEN) {
>> +                     if (unlikely(recv_byte >
>> I2C_SMBUS_BLOCK_MAX)) {
>> +                             bus->cmd_err = -EPROTO;
>> +                             aspeed_i2c_do_stop(bus);
>> +                             goto out_no_complete;
>> +                     }
>> +                     msg->len = recv_byte +
>> +                                     ((msg->flags &
>> I2C_CLIENT_PEC) ? 2 : 1);
>> +                     msg->flags &= ~I2C_M_RECV_LEN;
>> +             }
>> +
>> +             if (bus->buf_index < msg->len) {
>> +                     bus->master_state = ASPEED_I2C_MASTER_RX;
>> +                     command = ASPEED_I2CD_M_RX_CMD;
>> +                     if (bus->buf_index + 1 == msg->len)
>> +                             command |=
>> ASPEED_I2CD_M_S_RX_CMD_LAST;
>> +                     writel(command, bus->base +
>> ASPEED_I2C_CMD_REG);
>> +             } else {
>> +                     aspeed_i2c_next_msg_or_stop(bus);
>> +             }
>> +             goto out_no_complete;
>> +     case ASPEED_I2C_MASTER_STOP:
>> +             if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_NORMAL_STOP))) {
>> +                     dev_err(bus->dev, "master failed to STOP");
>> +                     bus->cmd_err = -EIO;
>> +                     /* Do not STOP as we have already tried. */
>> +             } else {
>> +                     status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
>> +             }
>> +
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             goto out_complete;
>> +     case ASPEED_I2C_MASTER_INACTIVE:
>> +             dev_err(bus->dev,
>> +                     "master received interrupt 0x%08x, but is
>> inactive",
>> +                     irq_status);
>> +             bus->cmd_err = -EIO;
>> +             /* Do not STOP as we should be inactive. */
>> +             goto out_complete;
>> +     default:
>> +             WARN(1, "unknown master state\n");
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             bus->cmd_err = -EIO;
>> +             goto out_complete;
>> +     }
>> +error_and_stop:
>> +     bus->cmd_err = -EIO;
>> +     aspeed_i2c_do_stop(bus);
>> +     goto out_no_complete;
>> +out_complete:
>> +     bus->msgs = NULL;
>> +     if (bus->cmd_err)
>> +             bus->master_xfer_result = bus->cmd_err;
>> +     else
>> +             bus->master_xfer_result = bus->msgs_index + 1;
>> +     complete(&bus->cmd_complete);
>> +out_no_complete:
>> +     if (irq_status != status_ack)
>> +             dev_err(bus->dev,
>> +                     "irq handled != irq. expected 0x%08x, but
>> was 0x%08x\n",
>> +                     irq_status, status_ack);
>> +     spin_unlock(&bus->lock);
>> +     return !!irq_status;
>> +}
>> +
>> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
>> +{
>> +     struct aspeed_i2c_bus *bus = dev_id;
>> +
>> +     if (aspeed_i2c_master_irq(bus))
>> +             return IRQ_HANDLED;
>> +     else
>> +             return IRQ_NONE;
>> +}
>> +
>> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> +                               struct i2c_msg *msgs, int num)
>> +{
>> +     struct aspeed_i2c_bus *bus = adap->algo_data;
>> +     unsigned long time_left, flags;
>> +     int ret = 0;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +     bus->cmd_err = 0;
>> +
>> +     /* If bus is busy, attempt recovery. We assume a single
>> master
>> +      * environment.
>> +      */
>> +     if (readl(bus->base + ASPEED_I2C_CMD_REG) &
>> ASPEED_I2CD_BUS_BUSY_STS) {
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +             ret = aspeed_i2c_recover_bus(bus);
>> +             if (ret)
>> +                     return ret;
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +     }
>> +
>> +     bus->cmd_err = 0;
>> +     bus->msgs = msgs;
>> +     bus->msgs_index = 0;
>> +     bus->msgs_count = num;
>> +
>> +     reinit_completion(&bus->cmd_complete);
>> +     aspeed_i2c_do_start(bus);
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     time_left = wait_for_completion_timeout(&bus->cmd_complete,
>> +                                             bus->adap.timeout);
>> +
>> +     if (time_left == 0)
>> +             return -ETIMEDOUT;
>> +     else
>> +             return bus->master_xfer_result;
>> +}
>> +
>> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
>> +{
>> +     return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
>> I2C_FUNC_SMBUS_BLOCK_DATA;
>> +}
>> +
>> +static const struct i2c_algorithm aspeed_i2c_algo = {
>> +     .master_xfer    = aspeed_i2c_master_xfer,
>> +     .functionality  = aspeed_i2c_functionality,
>> +};
>> +
>> +static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
>> +{
>> +     u32 base_clk, clk_high, clk_low, tmp;
>> +
>> +     /*
>> +      * The actual clock frequency of SCL is:
>> +      *      SCL_freq = APB_freq / (base_freq * (SCL_high +
>> SCL_low))
>> +      *               = APB_freq / divisor
>> +      * where base_freq is a programmable clock divider; its
>> value is
>> +      *      base_freq = 1 << base_clk
>> +      * SCL_high is the number of base_freq clock cycles that SCL
>> stays high
>> +      * and SCL_low is the number of base_freq clock cycles that
>> SCL stays
>> +      * low for a period of SCL.
>> +      * The actual register has a minimum SCL_high and SCL_low
>> minimum of 1;
>> +      * thus, they start counting at zero. So
>> +      *      SCL_high = clk_high + 1
>> +      *      SCL_low  = clk_low + 1
>> +      * Thus,
>> +      *      SCL_freq = APB_freq /
>> +      *              ((1 << base_clk) * (clk_high + 1 + clk_low
>> + 1))
>> +      * The documentation recommends clk_high >= 8 and clk_low >=
>> 7 when
>> +      * possible; this last constraint gives us the following
>> solution:
>> +      */
>> +     base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
>> +     tmp = divisor / (1 << base_clk);
>> +     clk_high = tmp / 2 + tmp % 2;
>> +     clk_low = tmp - clk_high;
>> +
>> +     clk_high -= 1;
>> +     clk_low -= 1;
>> +
>> +     return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> +             & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> +                     | ((clk_low <<
>> ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> +                        & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> +                     | (base_clk &
>> ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
>> +{
>> +     u32 divisor, clk_reg_val;
>> +
>> +     divisor = bus->parent_clk_frequency / bus->bus_frequency;
>> +     clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
>> +     writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
>> +     writel(ASPEED_NO_TIMEOUT_CTRL, bus->base +
>> ASPEED_I2C_AC_TIMING_REG2);
>> +
>> +     return 0;
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
>> +                          struct platform_device *pdev)
>> +{
>> +     u32 fun_ctrl_reg = ASPEED_I2CD_MASTER_EN;
>> +     int ret;
>> +
>> +     /* Disable everything. */
>> +     writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
>> +
>> +     ret = aspeed_i2c_init_clk(bus);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     if (!of_property_read_bool(pdev->dev.of_node, "multi-
>> master"))
>> +             fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
>> +
>> +     /* Enable Master Mode */
>> +     writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) |
>> fun_ctrl_reg,
>> +            bus->base + ASPEED_I2C_FUN_CTRL_REG);
>> +
>> +     /* Set interrupt generation of I2C controller */
>> +     writel(ASPEED_I2CD_INTR_ALL, bus->base +
>> ASPEED_I2C_INTR_CTRL_REG);
>> +
>> +     return 0;
>> +}
>> +
>> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
>> +{
>> +     struct platform_device *pdev = to_platform_device(bus->dev);
>> +     unsigned long flags;
>> +     int ret;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +
>> +     /* Disable and ack all interrupts. */
>> +     writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>> +     writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
>> +
>> +     ret = aspeed_i2c_init(bus, pdev);
>> +
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return ret;
>> +}
>> +
>> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> +{
>> +     struct aspeed_i2c_bus *bus;
>> +     struct clk *parent_clk;
>> +     struct resource *res;
>> +     int ret;
>> +
>> +     bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>> +     if (!bus)
>> +             return -ENOMEM;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     bus->base = devm_ioremap_resource(&pdev->dev, res);
>> +     if (IS_ERR(bus->base))
>> +             return PTR_ERR(bus->base);
>> +
>> +     parent_clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(parent_clk))
>> +             return PTR_ERR(parent_clk);
>> +     bus->parent_clk_frequency = clk_get_rate(parent_clk);
>> +     /* We just need the clock rate, we don't actually use the
>> clk object. */
>> +     devm_clk_put(&pdev->dev, parent_clk);
>> +
>> +     ret = of_property_read_u32(pdev->dev.of_node,
>> +                                "bus-frequency", &bus-
>> >bus_frequency);
>> +     if (ret < 0) {
>> +             dev_err(&pdev->dev,
>> +                     "Could not read bus-frequency property\n");
>> +             bus->bus_frequency = 100000;
>> +     }
>> +
>> +     /* Initialize the I2C adapter */
>> +     spin_lock_init(&bus->lock);
>> +     init_completion(&bus->cmd_complete);
>> +     bus->adap.owner = THIS_MODULE;
>> +     bus->adap.retries = 0;
>> +     bus->adap.timeout = 5 * HZ;
>> +     bus->adap.algo = &aspeed_i2c_algo;
>> +     bus->adap.algo_data = bus;
>> +     bus->adap.dev.parent = &pdev->dev;
>> +     bus->adap.dev.of_node = pdev->dev.of_node;
>> +     strlcpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
>> +
>> +     bus->dev = &pdev->dev;
>> +
>> +     /* Clean up any left over interrupt state. */
>> +     writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>> +     writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
>> +     /*
>> +      * bus.lock does not need to be held because the interrupt
>> handler has
>> +      * not been enabled yet.
>> +      */
>> +     ret = aspeed_i2c_init(bus, pdev);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
>> +     ret = devm_request_irq(&pdev->dev, bus->irq,
>> aspeed_i2c_bus_irq,
>> +                            0, dev_name(&pdev->dev), bus);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     ret = i2c_add_adapter(&bus->adap);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     platform_set_drvdata(pdev, bus);
>> +
>> +     dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
>> +              bus->adap.nr, bus->irq);
>> +
>> +     return 0;
>> +}
>> +
>> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
>> +{
>> +     struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +
>> +     /* Disable everything. */
>> +     writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
>> +     writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>> +
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     i2c_del_adapter(&bus->adap);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
>> +     { .compatible = "aspeed,ast2400-i2c-bus", },
>> +     { .compatible = "aspeed,ast2500-i2c-bus", },
>> +     { },
>> +};
>> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
>> +
>> +static struct platform_driver aspeed_i2c_bus_driver = {
>> +     .probe          = aspeed_i2c_probe_bus,
>> +     .remove         = aspeed_i2c_remove_bus,
>> +     .driver         = {
>> +             .name           = "aspeed-i2c-bus",
>> +             .of_match_table = aspeed_i2c_bus_of_table,
>> +     },
>> +};
>> +module_platform_driver(aspeed_i2c_bus_driver);
>> +
>> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
>> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
>> +MODULE_LICENSE("GPL v2");

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

On Sun, Jun 4, 2017 at 10:02 PM, Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org> wrote:
> On Fri, 2017-06-02 at 18:29 -0700, Brendan Higgins wrote:
>> Added initial master support for Aspeed I2C controller. Supports
>> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> I exercised this patch on an AST2500-based machine. The results were
> good, and the implementation copes fine with a workload that causes an
> oops in our (older, unrelated) out-of-tree bus driver.
>
> Tested-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>

Wolfram, could you take a look at these patches? Ben, Joel and others
seem to be happy with it and Marc said he would take the irqchip
stuff.

Cheers

>
>> ---
>> Changes for v2:
>>   - Added single module_init (multiple was breaking some builds).
>> Changes for v3:
>>   - Removed "bus" device tree param; now extracted from bus address
>> offset
>> Changes for v4:
>>   - I2C adapter number is now generated dynamically unless specified
>> in alias.
>> Changes for v5:
>>   - Removed irq_chip used to multiplex IRQ and replaced it with
>> dummy_irq_chip
>>     along with some other IRQ cleanup.
>>   - Addressed comments from Cedric, and Vladimir, mostly stylistic
>> things and
>>     using devm managed resources.
>>   - Increased max clock frequency before the bus is put in HighSpeed
>> mode, as
>>     per Kachalov's comment.
>> Changes for v6:
>>   - 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.
>>   - 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 for v7:
>>   - Changed clock-frequency to bus-frequency
>>   - Made some fixes to clock divider code
>>   - Added hardware reset function
>>   - Marked functions that need to be called with the lock held as
>> "unlocked"
>>   - Did a bunch of clean up
>> Changes for v8:
>>   - ACK IRQ status bits before doing anything else
>>   - Added multi-master device tree property
>>   - Do not send STOP commands after interrupt errors
>>   - Fix SMBUS_QUICK emulation handling
>>   - Removed highspeed clock code (I will do it in a later patch set)
>>   - Use the platform_device name for the adapter name
>>   - Reset for all failed recoveries
>>   - Removed the "__" prefix from all of the non-thread safe functions
>> Changes for v9:
>>   - No longer debug log after every address NACK
>>   - Clear errors after reset
>>   - No longer access clock while holding lock
>>   - Ack all interrupts durring bus reset and initialization
>> Changes for v10:
>>   - Added master_xfer_result field to struct aspeed_i2c_bus
>> ---
>>  drivers/i2c/busses/Kconfig      |  10 +
>>  drivers/i2c/busses/Makefile     |   1 +
>>  drivers/i2c/busses/i2c-aspeed.c | 681
>> ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 692 insertions(+)
>>  create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 144cbadc7c72..280f84a0d7d1 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -326,6 +326,16 @@ config I2C_POWERMAC
>>
>>  comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>>
>> +config I2C_ASPEED
>> +     tristate "Aspeed I2C Controller"
>> +     depends on ARCH_ASPEED
>> +     help
>> +       If you say yes to this option, support will be included
>> for the
>> +       Aspeed I2C controller.
>> +
>> +       This driver can also be built as a module.  If so, the
>> module
>> +       will be called i2c-aspeed.
>> +
>>  config I2C_AT91
>>       tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
>>       depends on ARCH_AT91
>> diff --git a/drivers/i2c/busses/Makefile
>> b/drivers/i2c/busses/Makefile
>> index 30b60855fbcd..e84604b9bf3b 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA)             += i2c-
>> hydra.o
>>  obj-$(CONFIG_I2C_POWERMAC)   += i2c-powermac.o
>>
>>  # Embedded system I2C/SMBus host controller drivers
>> +obj-$(CONFIG_I2C_ASPEED)     += i2c-aspeed.o
>>  obj-$(CONFIG_I2C_AT91)               += i2c-at91.o
>>  obj-$(CONFIG_I2C_AU1550)     += i2c-au1550.o
>>  obj-$(CONFIG_I2C_AXXIA)              += i2c-axxia.o
>> diff --git a/drivers/i2c/busses/i2c-aspeed.c
>> b/drivers/i2c/busses/i2c-aspeed.c
>> new file mode 100644
>> index 000000000000..a04021e3b3ab
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -0,0 +1,681 @@
>> +/*
>> + *  Aspeed 24XX/25XX I2C Controller.
>> + *
>> + *  Copyright (C) 2012-2017 ASPEED Technology Inc.
>> + *  Copyright 2017 IBM Corporation
>> + *  Copyright 2017 Google, Inc.
>> + *
>> + *  This program is free software; you can redistribute it and/or
>> modify
>> + *  it under the terms of the GNU General Public License version 2
>> as
>> + *  published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/completion.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +/* I2C Register */
>> +#define ASPEED_I2C_FUN_CTRL_REG                              0x00
>> +#define ASPEED_I2C_AC_TIMING_REG1                    0x04
>> +#define ASPEED_I2C_AC_TIMING_REG2                    0x08
>> +#define ASPEED_I2C_INTR_CTRL_REG                     0x0c
>> +#define ASPEED_I2C_INTR_STS_REG                              0x10
>> +#define ASPEED_I2C_CMD_REG                           0x14
>> +#define ASPEED_I2C_DEV_ADDR_REG                              0x18
>> +#define ASPEED_I2C_BYTE_BUF_REG                              0x20
>> +
>> +/* Global Register Definition */
>> +/* 0x00 : I2C Interrupt Status Register  */
>> +/* 0x08 : I2C Interrupt Target Assignment  */
>> +
>> +/* Device Register Definition */
>> +/* 0x00 : I2CD Function Control Register  */
>> +#define ASPEED_I2CD_MULTI_MASTER_DIS                 BIT(15)
>> +#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_MASTER_EN                                BIT(0)
>> +
>> +/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
>> +#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT                      16
>> +#define ASPEED_I2CD_TIME_SCL_HIGH_MASK                       GENMAS
>> K(19, 16)
>> +#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT                       12
>> +#define ASPEED_I2CD_TIME_SCL_LOW_MASK                        GENMASK
>> (15, 12)
>> +#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK           GENMASK(3,
>> 0)
>> +#define ASPEED_I2CD_TIME_SCL_REG_MAX                 GENMASK(
>> 3, 0)
>> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
>> +#define ASPEED_NO_TIMEOUT_CTRL                               0
>> +
>> +/* 0x0c : I2CD Interrupt Control Register &
>> + * 0x10 : I2CD Interrupt Status Register
>> + *
>> + * These share bit definitions, so use the same values for the
>> enable &
>> + * status bits.
>> + */
>> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT                      BIT(1
>> 4)
>> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE            BIT(13)
>> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT                 BIT(6)
>> +#define ASPEED_I2CD_INTR_ABNORMAL                    BIT(5)
>> +#define ASPEED_I2CD_INTR_NORMAL_STOP                 BIT(4)
>> +#define ASPEED_I2CD_INTR_ARBIT_LOSS                  BIT(3)
>> +#define ASPEED_I2CD_INTR_RX_DONE                     BIT(2)
>> +#define ASPEED_I2CD_INTR_TX_NAK                              BIT(1
>> )
>> +#define ASPEED_I2CD_INTR_TX_ACK                              BIT(0
>> )
>> +#define ASPEED_I2CD_INTR_ERROR
>>              \
>> +             (ASPEED_I2CD_INTR_ARBIT_LOSS |
>>              \
>> +              ASPEED_I2CD_INTR_ABNORMAL |
>>        \
>> +              ASPEED_I2CD_INTR_SCL_TIMEOUT |
>>              \
>> +              ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
>> +#define ASPEED_I2CD_INTR_ALL
>>        \
>> +             (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
>>        \
>> +              ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
>>        \
>> +              ASPEED_I2CD_INTR_SCL_TIMEOUT |
>>              \
>> +              ASPEED_I2CD_INTR_ABNORMAL |
>>        \
>> +              ASPEED_I2CD_INTR_NORMAL_STOP |
>>              \
>> +              ASPEED_I2CD_INTR_ARBIT_LOSS |
>>              \
>> +              ASPEED_I2CD_INTR_RX_DONE |
>>        \
>> +              ASPEED_I2CD_INTR_TX_NAK |
>>        \
>> +              ASPEED_I2CD_INTR_TX_ACK)
>> +
>> +/* 0x14 : I2CD Command/Status Register   */
>> +#define ASPEED_I2CD_SCL_LINE_STS                     BIT(18)
>> +#define ASPEED_I2CD_SDA_LINE_STS                     BIT(17)
>> +#define ASPEED_I2CD_BUS_BUSY_STS                     BIT(16)
>> +#define ASPEED_I2CD_BUS_RECOVER_CMD                  BIT(11)
>> +
>> +/* Command Bit */
>> +#define ASPEED_I2CD_M_STOP_CMD                               BIT(5)
>> +#define ASPEED_I2CD_M_S_RX_CMD_LAST                  BIT(4)
>> +#define ASPEED_I2CD_M_RX_CMD                         BIT(3)
>> +#define ASPEED_I2CD_S_TX_CMD                         BIT(2)
>> +#define ASPEED_I2CD_M_TX_CMD                         BIT(1)
>> +#define ASPEED_I2CD_M_START_CMD                              BIT(0
>> )
>> +
>> +enum aspeed_i2c_master_state {
>> +     ASPEED_I2C_MASTER_START,
>> +     ASPEED_I2C_MASTER_TX_FIRST,
>> +     ASPEED_I2C_MASTER_TX,
>> +     ASPEED_I2C_MASTER_RX_FIRST,
>> +     ASPEED_I2C_MASTER_RX,
>> +     ASPEED_I2C_MASTER_STOP,
>> +     ASPEED_I2C_MASTER_INACTIVE,
>> +};
>> +
>> +struct aspeed_i2c_bus {
>> +     struct i2c_adapter              adap;
>> +     struct device                   *dev;
>> +     void __iomem                    *base;
>> +     /* Synchronizes I/O mem access to base. */
>> +     spinlock_t                      lock;
>> +     struct completion               cmd_complete;
>> +     int                             irq;
>> +     unsigned long                   parent_clk_frequency;
>> +     u32                             bus_frequency;
>> +     /* Transaction state. */
>> +     enum aspeed_i2c_master_state    master_state;
>> +     struct i2c_msg                  *msgs;
>> +     size_t                          buf_index;
>> +     size_t                          msgs_index;
>> +     size_t                          msgs_count;
>> +     bool                            send_stop;
>> +     int                             cmd_err;
>> +     /* Protected only by i2c_lock_bus */
>> +     int                             master_xfer_result;
>> +};
>> +
>> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
>> +
>> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>> +{
>> +     unsigned long time_left, flags;
>> +     int ret = 0;
>> +     u32 command;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +     command = readl(bus->base + ASPEED_I2C_CMD_REG);
>> +
>> +     if (command & ASPEED_I2CD_SDA_LINE_STS) {
>> +             /* Bus is idle: no recovery needed. */
>> +             if (command & ASPEED_I2CD_SCL_LINE_STS)
>> +                     goto out;
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting
>> recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
>> ASPEED_I2C_CMD_REG);
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus-
>> >adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SCL_LINE_STS))
>> +                     goto reset_out;
>> +     /* Bus error. */
>> +     } else {
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting
>> recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_BUS_RECOVER_CMD,
>> +                    bus->base + ASPEED_I2C_CMD_REG);
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus-
>> >adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SDA_LINE_STS))
>> +                     goto reset_out;
>> +     }
>> +
>> +out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return ret;
>> +
>> +reset_out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return aspeed_i2c_reset(bus);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
>> +{
>> +     u32 command = ASPEED_I2CD_M_START_CMD |
>> ASPEED_I2CD_M_TX_CMD;
>> +     struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
>> +     u8 slave_addr = msg->addr << 1;
>> +
>> +     bus->master_state = ASPEED_I2C_MASTER_START;
>> +     bus->buf_index = 0;
>> +
>> +     if (msg->flags & I2C_M_RD) {
>> +             slave_addr |= 1;
>> +             command |= ASPEED_I2CD_M_RX_CMD;
>> +             /* Need to let the hardware know to NACK after RX.
>> */
>> +             if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
>> +                     command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
>> +     }
>> +
>> +     writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
>> +     writel(command, bus->base + ASPEED_I2C_CMD_REG);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static void aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
>> +{
>> +     bus->master_state = ASPEED_I2C_MASTER_STOP;
>> +     writel(ASPEED_I2CD_M_STOP_CMD, bus->base +
>> ASPEED_I2C_CMD_REG);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static void aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
>> +{
>> +     if (bus->msgs_index + 1 < bus->msgs_count) {
>> +             bus->msgs_index++;
>> +             aspeed_i2c_do_start(bus);
>> +     } else {
>> +             aspeed_i2c_do_stop(bus);
>> +     }
>> +}
>> +
>> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> +{
>> +     u32 irq_status, status_ack = 0, command = 0;
>> +     struct i2c_msg *msg;
>> +     u8 recv_byte;
>> +
>> +     spin_lock(&bus->lock);
>> +     irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
>> +     /* Ack all interrupt bits. */
>> +     writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>> +
>> +     if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
>> +             goto out_complete;
>> +     }
>> +
>> +     /*
>> +      * We encountered an interrupt that reports an error: the
>> hardware
>> +      * should clear the command queue effectively taking us back
>> to the
>> +      * INACTIVE state.
>> +      */
>> +     if (irq_status & ASPEED_I2CD_INTR_ERROR) {
>> +             dev_dbg(bus->dev, "received error interrupt:
>> 0x%08x",
>> +                     irq_status);
>> +             bus->cmd_err = -EIO;
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             goto out_complete;
>> +     }
>> +
>> +     /* We are in an invalid state; reset bus to a known state.
>> */
>> +     if (!bus->msgs && bus->master_state !=
>> ASPEED_I2C_MASTER_STOP) {
>> +             dev_err(bus->dev, "bus in unknown state");
>> +             bus->cmd_err = -EIO;
>> +             aspeed_i2c_do_stop(bus);
>> +             goto out_no_complete;
>> +     }
>> +     msg = &bus->msgs[bus->msgs_index];
>> +
>> +     /*
>> +      * START is a special case because we still have to handle a
>> subsequent
>> +      * TX or RX immediately after we handle it, so we handle it
>> here and
>> +      * then update the state and handle the new state below.
>> +      */
>> +     if (bus->master_state == ASPEED_I2C_MASTER_START) {
>> +             if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_TX_ACK))) {
>> +                     pr_devel("no slave present at %02x", msg-
>> >addr);
>> +                     status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> +                     goto error_and_stop;
>> +             }
>> +             status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> +             if (msg->len == 0) { /* SMBUS_QUICK */
>> +                     aspeed_i2c_do_stop(bus);
>> +                     goto out_no_complete;
>> +             }
>> +             if (msg->flags & I2C_M_RD)
>> +                     bus->master_state =
>> ASPEED_I2C_MASTER_RX_FIRST;
>> +             else
>> +                     bus->master_state =
>> ASPEED_I2C_MASTER_TX_FIRST;
>> +     }
>> +
>> +     switch (bus->master_state) {
>> +     case ASPEED_I2C_MASTER_TX:
>> +             if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK))
>> {
>> +                     dev_dbg(bus->dev, "slave NACKed TX");
>> +                     status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> +                     goto error_and_stop;
>> +             } else if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_TX_ACK))) {
>> +                     dev_err(bus->dev, "slave failed to ACK TX");
>> +                     goto error_and_stop;
>> +             }
>> +             status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> +             /* fallthrough intended */
>> +     case ASPEED_I2C_MASTER_TX_FIRST:
>> +             if (bus->buf_index < msg->len) {
>> +                     bus->master_state = ASPEED_I2C_MASTER_TX;
>> +                     writel(msg->buf[bus->buf_index++],
>> +                            bus->base + ASPEED_I2C_BYTE_BUF_REG);
>> +                     writel(ASPEED_I2CD_M_TX_CMD,
>> +                            bus->base + ASPEED_I2C_CMD_REG);
>> +             } else {
>> +                     aspeed_i2c_next_msg_or_stop(bus);
>> +             }
>> +             goto out_no_complete;
>> +     case ASPEED_I2C_MASTER_RX_FIRST:
>> +             /* RX may not have completed yet (only address
>> cycle) */
>> +             if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
>> +                     goto out_no_complete;
>> +             /* fallthrough intended */
>> +     case ASPEED_I2C_MASTER_RX:
>> +             if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_RX_DONE))) {
>> +                     dev_err(bus->dev, "master failed to RX");
>> +                     goto error_and_stop;
>> +             }
>> +             status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> +
>> +             recv_byte = readl(bus->base +
>> ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> +             msg->buf[bus->buf_index++] = recv_byte;
>> +
>> +             if (msg->flags & I2C_M_RECV_LEN) {
>> +                     if (unlikely(recv_byte >
>> I2C_SMBUS_BLOCK_MAX)) {
>> +                             bus->cmd_err = -EPROTO;
>> +                             aspeed_i2c_do_stop(bus);
>> +                             goto out_no_complete;
>> +                     }
>> +                     msg->len = recv_byte +
>> +                                     ((msg->flags &
>> I2C_CLIENT_PEC) ? 2 : 1);
>> +                     msg->flags &= ~I2C_M_RECV_LEN;
>> +             }
>> +
>> +             if (bus->buf_index < msg->len) {
>> +                     bus->master_state = ASPEED_I2C_MASTER_RX;
>> +                     command = ASPEED_I2CD_M_RX_CMD;
>> +                     if (bus->buf_index + 1 == msg->len)
>> +                             command |=
>> ASPEED_I2CD_M_S_RX_CMD_LAST;
>> +                     writel(command, bus->base +
>> ASPEED_I2C_CMD_REG);
>> +             } else {
>> +                     aspeed_i2c_next_msg_or_stop(bus);
>> +             }
>> +             goto out_no_complete;
>> +     case ASPEED_I2C_MASTER_STOP:
>> +             if (unlikely(!(irq_status &
>> ASPEED_I2CD_INTR_NORMAL_STOP))) {
>> +                     dev_err(bus->dev, "master failed to STOP");
>> +                     bus->cmd_err = -EIO;
>> +                     /* Do not STOP as we have already tried. */
>> +             } else {
>> +                     status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
>> +             }
>> +
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             goto out_complete;
>> +     case ASPEED_I2C_MASTER_INACTIVE:
>> +             dev_err(bus->dev,
>> +                     "master received interrupt 0x%08x, but is
>> inactive",
>> +                     irq_status);
>> +             bus->cmd_err = -EIO;
>> +             /* Do not STOP as we should be inactive. */
>> +             goto out_complete;
>> +     default:
>> +             WARN(1, "unknown master state\n");
>> +             bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> +             bus->cmd_err = -EIO;
>> +             goto out_complete;
>> +     }
>> +error_and_stop:
>> +     bus->cmd_err = -EIO;
>> +     aspeed_i2c_do_stop(bus);
>> +     goto out_no_complete;
>> +out_complete:
>> +     bus->msgs = NULL;
>> +     if (bus->cmd_err)
>> +             bus->master_xfer_result = bus->cmd_err;
>> +     else
>> +             bus->master_xfer_result = bus->msgs_index + 1;
>> +     complete(&bus->cmd_complete);
>> +out_no_complete:
>> +     if (irq_status != status_ack)
>> +             dev_err(bus->dev,
>> +                     "irq handled != irq. expected 0x%08x, but
>> was 0x%08x\n",
>> +                     irq_status, status_ack);
>> +     spin_unlock(&bus->lock);
>> +     return !!irq_status;
>> +}
>> +
>> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
>> +{
>> +     struct aspeed_i2c_bus *bus = dev_id;
>> +
>> +     if (aspeed_i2c_master_irq(bus))
>> +             return IRQ_HANDLED;
>> +     else
>> +             return IRQ_NONE;
>> +}
>> +
>> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> +                               struct i2c_msg *msgs, int num)
>> +{
>> +     struct aspeed_i2c_bus *bus = adap->algo_data;
>> +     unsigned long time_left, flags;
>> +     int ret = 0;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +     bus->cmd_err = 0;
>> +
>> +     /* If bus is busy, attempt recovery. We assume a single
>> master
>> +      * environment.
>> +      */
>> +     if (readl(bus->base + ASPEED_I2C_CMD_REG) &
>> ASPEED_I2CD_BUS_BUSY_STS) {
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +             ret = aspeed_i2c_recover_bus(bus);
>> +             if (ret)
>> +                     return ret;
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +     }
>> +
>> +     bus->cmd_err = 0;
>> +     bus->msgs = msgs;
>> +     bus->msgs_index = 0;
>> +     bus->msgs_count = num;
>> +
>> +     reinit_completion(&bus->cmd_complete);
>> +     aspeed_i2c_do_start(bus);
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     time_left = wait_for_completion_timeout(&bus->cmd_complete,
>> +                                             bus->adap.timeout);
>> +
>> +     if (time_left == 0)
>> +             return -ETIMEDOUT;
>> +     else
>> +             return bus->master_xfer_result;
>> +}
>> +
>> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
>> +{
>> +     return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
>> I2C_FUNC_SMBUS_BLOCK_DATA;
>> +}
>> +
>> +static const struct i2c_algorithm aspeed_i2c_algo = {
>> +     .master_xfer    = aspeed_i2c_master_xfer,
>> +     .functionality  = aspeed_i2c_functionality,
>> +};
>> +
>> +static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
>> +{
>> +     u32 base_clk, clk_high, clk_low, tmp;
>> +
>> +     /*
>> +      * The actual clock frequency of SCL is:
>> +      *      SCL_freq = APB_freq / (base_freq * (SCL_high +
>> SCL_low))
>> +      *               = APB_freq / divisor
>> +      * where base_freq is a programmable clock divider; its
>> value is
>> +      *      base_freq = 1 << base_clk
>> +      * SCL_high is the number of base_freq clock cycles that SCL
>> stays high
>> +      * and SCL_low is the number of base_freq clock cycles that
>> SCL stays
>> +      * low for a period of SCL.
>> +      * The actual register has a minimum SCL_high and SCL_low
>> minimum of 1;
>> +      * thus, they start counting at zero. So
>> +      *      SCL_high = clk_high + 1
>> +      *      SCL_low  = clk_low + 1
>> +      * Thus,
>> +      *      SCL_freq = APB_freq /
>> +      *              ((1 << base_clk) * (clk_high + 1 + clk_low
>> + 1))
>> +      * The documentation recommends clk_high >= 8 and clk_low >=
>> 7 when
>> +      * possible; this last constraint gives us the following
>> solution:
>> +      */
>> +     base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
>> +     tmp = divisor / (1 << base_clk);
>> +     clk_high = tmp / 2 + tmp % 2;
>> +     clk_low = tmp - clk_high;
>> +
>> +     clk_high -= 1;
>> +     clk_low -= 1;
>> +
>> +     return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> +             & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> +                     | ((clk_low <<
>> ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> +                        & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> +                     | (base_clk &
>> ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus)
>> +{
>> +     u32 divisor, clk_reg_val;
>> +
>> +     divisor = bus->parent_clk_frequency / bus->bus_frequency;
>> +     clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
>> +     writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
>> +     writel(ASPEED_NO_TIMEOUT_CTRL, bus->base +
>> ASPEED_I2C_AC_TIMING_REG2);
>> +
>> +     return 0;
>> +}
>> +
>> +/* precondition: bus.lock has been acquired. */
>> +static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
>> +                          struct platform_device *pdev)
>> +{
>> +     u32 fun_ctrl_reg = ASPEED_I2CD_MASTER_EN;
>> +     int ret;
>> +
>> +     /* Disable everything. */
>> +     writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
>> +
>> +     ret = aspeed_i2c_init_clk(bus);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     if (!of_property_read_bool(pdev->dev.of_node, "multi-
>> master"))
>> +             fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
>> +
>> +     /* Enable Master Mode */
>> +     writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) |
>> fun_ctrl_reg,
>> +            bus->base + ASPEED_I2C_FUN_CTRL_REG);
>> +
>> +     /* Set interrupt generation of I2C controller */
>> +     writel(ASPEED_I2CD_INTR_ALL, bus->base +
>> ASPEED_I2C_INTR_CTRL_REG);
>> +
>> +     return 0;
>> +}
>> +
>> +static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
>> +{
>> +     struct platform_device *pdev = to_platform_device(bus->dev);
>> +     unsigned long flags;
>> +     int ret;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +
>> +     /* Disable and ack all interrupts. */
>> +     writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>> +     writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
>> +
>> +     ret = aspeed_i2c_init(bus, pdev);
>> +
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return ret;
>> +}
>> +
>> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> +{
>> +     struct aspeed_i2c_bus *bus;
>> +     struct clk *parent_clk;
>> +     struct resource *res;
>> +     int ret;
>> +
>> +     bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>> +     if (!bus)
>> +             return -ENOMEM;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     bus->base = devm_ioremap_resource(&pdev->dev, res);
>> +     if (IS_ERR(bus->base))
>> +             return PTR_ERR(bus->base);
>> +
>> +     parent_clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(parent_clk))
>> +             return PTR_ERR(parent_clk);
>> +     bus->parent_clk_frequency = clk_get_rate(parent_clk);
>> +     /* We just need the clock rate, we don't actually use the
>> clk object. */
>> +     devm_clk_put(&pdev->dev, parent_clk);
>> +
>> +     ret = of_property_read_u32(pdev->dev.of_node,
>> +                                "bus-frequency", &bus-
>> >bus_frequency);
>> +     if (ret < 0) {
>> +             dev_err(&pdev->dev,
>> +                     "Could not read bus-frequency property\n");
>> +             bus->bus_frequency = 100000;
>> +     }
>> +
>> +     /* Initialize the I2C adapter */
>> +     spin_lock_init(&bus->lock);
>> +     init_completion(&bus->cmd_complete);
>> +     bus->adap.owner = THIS_MODULE;
>> +     bus->adap.retries = 0;
>> +     bus->adap.timeout = 5 * HZ;
>> +     bus->adap.algo = &aspeed_i2c_algo;
>> +     bus->adap.algo_data = bus;
>> +     bus->adap.dev.parent = &pdev->dev;
>> +     bus->adap.dev.of_node = pdev->dev.of_node;
>> +     strlcpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
>> +
>> +     bus->dev = &pdev->dev;
>> +
>> +     /* Clean up any left over interrupt state. */
>> +     writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>> +     writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
>> +     /*
>> +      * bus.lock does not need to be held because the interrupt
>> handler has
>> +      * not been enabled yet.
>> +      */
>> +     ret = aspeed_i2c_init(bus, pdev);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
>> +     ret = devm_request_irq(&pdev->dev, bus->irq,
>> aspeed_i2c_bus_irq,
>> +                            0, dev_name(&pdev->dev), bus);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     ret = i2c_add_adapter(&bus->adap);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     platform_set_drvdata(pdev, bus);
>> +
>> +     dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
>> +              bus->adap.nr, bus->irq);
>> +
>> +     return 0;
>> +}
>> +
>> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
>> +{
>> +     struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +
>> +     /* Disable everything. */
>> +     writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
>> +     writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>> +
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     i2c_del_adapter(&bus->adap);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
>> +     { .compatible = "aspeed,ast2400-i2c-bus", },
>> +     { .compatible = "aspeed,ast2500-i2c-bus", },
>> +     { },
>> +};
>> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
>> +
>> +static struct platform_driver aspeed_i2c_bus_driver = {
>> +     .probe          = aspeed_i2c_probe_bus,
>> +     .remove         = aspeed_i2c_remove_bus,
>> +     .driver         = {
>> +             .name           = "aspeed-i2c-bus",
>> +             .of_match_table = aspeed_i2c_bus_of_table,
>> +     },
>> +};
>> +module_platform_driver(aspeed_i2c_bus_driver);
>> +
>> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
>> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
>> +MODULE_LICENSE("GPL v2");
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 5152 bytes --]

Hi Brendan,

here is my review. Only minor stuff, no real show-stopper.

> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -326,6 +326,16 @@ config I2C_POWERMAC
>  
>  comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>  
> +config I2C_ASPEED
> +	tristate "Aspeed I2C Controller"
> +	depends on ARCH_ASPEED

|| COMPILE_TEST?

> +	help
> +	  If you say yes to this option, support will be included for the
> +	  Aspeed I2C controller.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-aspeed.
> +
>  config I2C_AT91
>  	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
>  	depends on ARCH_AT91
> +struct aspeed_i2c_bus {
> +	struct i2c_adapter		adap;
> +	struct device			*dev;
> +	void __iomem			*base;
> +	/* Synchronizes I/O mem access to base. */
> +	spinlock_t			lock;
> +	struct completion		cmd_complete;
> +	int				irq;

'irq' not really needed, I'd think.

> +	unsigned long			parent_clk_frequency;
> +	u32				bus_frequency;
> +	/* Transaction state. */
> +	enum aspeed_i2c_master_state	master_state;
> +	struct i2c_msg			*msgs;
> +	size_t				buf_index;
> +	size_t				msgs_index;
> +	size_t				msgs_count;
> +	bool				send_stop;
> +	int				cmd_err;
> +	/* Protected only by i2c_lock_bus */
> +	int				master_xfer_result;
> +};

...

> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> +{
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +	u32 command;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	command = readl(bus->base + ASPEED_I2C_CMD_REG);
> +
> +	if (command & ASPEED_I2CD_SDA_LINE_STS) {
> +		/* Bus is idle: no recovery needed. */
> +		if (command & ASPEED_I2CD_SCL_LINE_STS)
> +			goto out;
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus->adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SCL_LINE_STS))
> +			goto reset_out;
> +	/* Bus error. */
> +	} else {
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> +			command);

Same dbg message as in the condition? Move it out of the 'if'?

> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_BUS_RECOVER_CMD,
> +		       bus->base + ASPEED_I2C_CMD_REG);

Out of interest: What does the RECOVER_CMD do?

> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus->adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SDA_LINE_STS))
> +			goto reset_out;
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +
> +reset_out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return aspeed_i2c_reset(bus);
> +}

...

> +	case ASPEED_I2C_MASTER_INACTIVE:
> +		dev_err(bus->dev,
> +			"master received interrupt 0x%08x, but is inactive",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		/* Do not STOP as we should be inactive. */
> +		goto out_complete;
> +	default:
> +		WARN(1, "unknown master state\n");
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		bus->cmd_err = -EIO;
> +		goto out_complete;
> +	}
> +error_and_stop:
> +	bus->cmd_err = -EIO;
> +	aspeed_i2c_do_stop(bus);
> +	goto out_no_complete;
> +out_complete:
> +	bus->msgs = NULL;
> +	if (bus->cmd_err)
> +		bus->master_xfer_result = bus->cmd_err;
> +	else
> +		bus->master_xfer_result = bus->msgs_index + 1;
> +	complete(&bus->cmd_complete);
> +out_no_complete:
> +	if (irq_status != status_ack)
> +		dev_err(bus->dev,
> +			"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
> +			irq_status, status_ack);
> +	spin_unlock(&bus->lock);
> +	return !!irq_status;
> +}

You return in the interrupt handler -EIO always in case of errors. Can
you check Documentation/i2c/fault-codes and see if you can follow those
guidelines? Especially ENXIO on NACK and EAGAIN...

> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> +	struct aspeed_i2c_bus *bus = dev_id;
> +
> +	if (aspeed_i2c_master_irq(bus))
> +		return IRQ_HANDLED;
> +	else
> +		return IRQ_NONE;

Ternary operator? Your choice, though...

And one more question: Why do you use adapter->algo_data instead of
i2c_{get|set}_adapdata?

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 5152 bytes --]

Hi Brendan,

here is my review. Only minor stuff, no real show-stopper.

> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 144cbadc7c72..280f84a0d7d1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -326,6 +326,16 @@ config I2C_POWERMAC
>  
>  comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>  
> +config I2C_ASPEED
> +	tristate "Aspeed I2C Controller"
> +	depends on ARCH_ASPEED

|| COMPILE_TEST?

> +	help
> +	  If you say yes to this option, support will be included for the
> +	  Aspeed I2C controller.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-aspeed.
> +
>  config I2C_AT91
>  	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
>  	depends on ARCH_AT91
> +struct aspeed_i2c_bus {
> +	struct i2c_adapter		adap;
> +	struct device			*dev;
> +	void __iomem			*base;
> +	/* Synchronizes I/O mem access to base. */
> +	spinlock_t			lock;
> +	struct completion		cmd_complete;
> +	int				irq;

'irq' not really needed, I'd think.

> +	unsigned long			parent_clk_frequency;
> +	u32				bus_frequency;
> +	/* Transaction state. */
> +	enum aspeed_i2c_master_state	master_state;
> +	struct i2c_msg			*msgs;
> +	size_t				buf_index;
> +	size_t				msgs_index;
> +	size_t				msgs_count;
> +	bool				send_stop;
> +	int				cmd_err;
> +	/* Protected only by i2c_lock_bus */
> +	int				master_xfer_result;
> +};

...

> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
> +{
> +	unsigned long time_left, flags;
> +	int ret = 0;
> +	u32 command;
> +
> +	spin_lock_irqsave(&bus->lock, flags);
> +	command = readl(bus->base + ASPEED_I2C_CMD_REG);
> +
> +	if (command & ASPEED_I2CD_SDA_LINE_STS) {
> +		/* Bus is idle: no recovery needed. */
> +		if (command & ASPEED_I2CD_SCL_LINE_STS)
> +			goto out;
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> +			command);
> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus->adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SCL_LINE_STS))
> +			goto reset_out;
> +	/* Bus error. */
> +	} else {
> +		dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> +			command);

Same dbg message as in the condition? Move it out of the 'if'?

> +
> +		reinit_completion(&bus->cmd_complete);
> +		writel(ASPEED_I2CD_BUS_RECOVER_CMD,
> +		       bus->base + ASPEED_I2C_CMD_REG);

Out of interest: What does the RECOVER_CMD do?

> +		spin_unlock_irqrestore(&bus->lock, flags);
> +
> +		time_left = wait_for_completion_timeout(
> +				&bus->cmd_complete, bus->adap.timeout);
> +
> +		spin_lock_irqsave(&bus->lock, flags);
> +		if (time_left == 0)
> +			goto reset_out;
> +		else if (bus->cmd_err)
> +			goto reset_out;
> +		/* Recovery failed. */
> +		else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> +			   ASPEED_I2CD_SDA_LINE_STS))
> +			goto reset_out;
> +	}
> +
> +out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return ret;
> +
> +reset_out:
> +	spin_unlock_irqrestore(&bus->lock, flags);
> +
> +	return aspeed_i2c_reset(bus);
> +}

...

> +	case ASPEED_I2C_MASTER_INACTIVE:
> +		dev_err(bus->dev,
> +			"master received interrupt 0x%08x, but is inactive",
> +			irq_status);
> +		bus->cmd_err = -EIO;
> +		/* Do not STOP as we should be inactive. */
> +		goto out_complete;
> +	default:
> +		WARN(1, "unknown master state\n");
> +		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> +		bus->cmd_err = -EIO;
> +		goto out_complete;
> +	}
> +error_and_stop:
> +	bus->cmd_err = -EIO;
> +	aspeed_i2c_do_stop(bus);
> +	goto out_no_complete;
> +out_complete:
> +	bus->msgs = NULL;
> +	if (bus->cmd_err)
> +		bus->master_xfer_result = bus->cmd_err;
> +	else
> +		bus->master_xfer_result = bus->msgs_index + 1;
> +	complete(&bus->cmd_complete);
> +out_no_complete:
> +	if (irq_status != status_ack)
> +		dev_err(bus->dev,
> +			"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
> +			irq_status, status_ack);
> +	spin_unlock(&bus->lock);
> +	return !!irq_status;
> +}

You return in the interrupt handler -EIO always in case of errors. Can
you check Documentation/i2c/fault-codes and see if you can follow those
guidelines? Especially ENXIO on NACK and EAGAIN...

> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> +	struct aspeed_i2c_bus *bus = dev_id;
> +
> +	if (aspeed_i2c_master_irq(bus))
> +		return IRQ_HANDLED;
> +	else
> +		return IRQ_NONE;

Ternary operator? Your choice, though...

And one more question: Why do you use adapter->algo_data instead of
i2c_{get|set}_adapdata?

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 42 bytes --]


And any chance for a MAINTAINERS entry?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

On Mon, Jun 19, 2017 at 7:21 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
> And any chance for a MAINTAINERS entry?
>

We already have one:
http://elixir.free-electrons.com/linux/v4.11.5/source/MAINTAINERS#L1090

However, if you would like me to own this driver, I could add an entry
for myself.

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

On Mon, Jun 19, 2017 at 7:21 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>
> And any chance for a MAINTAINERS entry?
>

We already have one:
http://elixir.free-electrons.com/linux/v4.11.5/source/MAINTAINERS#L1090

However, if you would like me to own this driver, I could add an entry
for myself.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Mon, Jun 19, 2017 at 05:58:19PM -0700, Brendan Higgins wrote:
> On Mon, Jun 19, 2017 at 7:21 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >
> > And any chance for a MAINTAINERS entry?
> >
> 
> We already have one:
> http://elixir.free-electrons.com/linux/v4.11.5/source/MAINTAINERS#L1090
> 
> However, if you would like me to own this driver, I could add an entry
> for myself.

As far as get_maintainers.pl prints out someone responsibe, I am very
happy. The above line won't work for I2C (and maybe some other
subsystems), though,  because we are one directory level deeper
(drivers/i2c/busses/*aspeed*). That might want fixing.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

I thought all of the comments made sense and will be addressed in my
next revision,
except the following:

>> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>> +{
>> +     unsigned long time_left, flags;
>> +     int ret = 0;
>> +     u32 command;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +     command = readl(bus->base + ASPEED_I2C_CMD_REG);
>> +
>> +     if (command & ASPEED_I2CD_SDA_LINE_STS) {
>> +             /* Bus is idle: no recovery needed. */
>> +             if (command & ASPEED_I2CD_SCL_LINE_STS)
>> +                     goto out;
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus->adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SCL_LINE_STS))
>> +                     goto reset_out;
>> +     /* Bus error. */
>> +     } else {
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
>> +                     command);
>
> Same dbg message as in the condition? Move it out of the 'if'?

Message is the same; that's true; however, I only want to print this
if I am actually doing
the recovery. There is a case in the first condition where we actually
don't attempt recovery.
(See both SDA and SCL high). Nevertheless, now that I am looking at
this. I think it might
make sense to make the dbg statements different since I can explain
what type of recovery
I am attempting.

>
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_BUS_RECOVER_CMD,
>> +                    bus->base + ASPEED_I2C_CMD_REG);
>
> Out of interest: What does the RECOVER_CMD do?

According to the documentation, it attempts to create 1 to 8 SCL
cycles to force any stuck
device to let go of the SDA line. It then puts the hardware in a sane
state once it detects
that the bus has been recovered. I will put a comment to this effect.

>
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus->adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SDA_LINE_STS))
>> +                     goto reset_out;
>> +     }
>> +
>> +out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return ret;
>> +
>> +reset_out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return aspeed_i2c_reset(bus);
>> +}

Cheers!

Re: [PATCH v10 4/5] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

I thought all of the comments made sense and will be addressed in my
next revision,
except the following:

>> +static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>> +{
>> +     unsigned long time_left, flags;
>> +     int ret = 0;
>> +     u32 command;
>> +
>> +     spin_lock_irqsave(&bus->lock, flags);
>> +     command = readl(bus->base + ASPEED_I2C_CMD_REG);
>> +
>> +     if (command & ASPEED_I2CD_SDA_LINE_STS) {
>> +             /* Bus is idle: no recovery needed. */
>> +             if (command & ASPEED_I2CD_SCL_LINE_STS)
>> +                     goto out;
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
>> +                     command);
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus->adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SCL_LINE_STS))
>> +                     goto reset_out;
>> +     /* Bus error. */
>> +     } else {
>> +             dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
>> +                     command);
>
> Same dbg message as in the condition? Move it out of the 'if'?

Message is the same; that's true; however, I only want to print this
if I am actually doing
the recovery. There is a case in the first condition where we actually
don't attempt recovery.
(See both SDA and SCL high). Nevertheless, now that I am looking at
this. I think it might
make sense to make the dbg statements different since I can explain
what type of recovery
I am attempting.

>
>> +
>> +             reinit_completion(&bus->cmd_complete);
>> +             writel(ASPEED_I2CD_BUS_RECOVER_CMD,
>> +                    bus->base + ASPEED_I2C_CMD_REG);
>
> Out of interest: What does the RECOVER_CMD do?

According to the documentation, it attempts to create 1 to 8 SCL
cycles to force any stuck
device to let go of the SDA line. It then puts the hardware in a sane
state once it detects
that the bus has been recovered. I will put a comment to this effect.

>
>> +             spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +             time_left = wait_for_completion_timeout(
>> +                             &bus->cmd_complete, bus->adap.timeout);
>> +
>> +             spin_lock_irqsave(&bus->lock, flags);
>> +             if (time_left == 0)
>> +                     goto reset_out;
>> +             else if (bus->cmd_err)
>> +                     goto reset_out;
>> +             /* Recovery failed. */
>> +             else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
>> +                        ASPEED_I2CD_SDA_LINE_STS))
>> +                     goto reset_out;
>> +     }
>> +
>> +out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return ret;
>> +
>> +reset_out:
>> +     spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> +     return aspeed_i2c_reset(bus);
>> +}

Cheers!
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver

[Brendan Higgins]

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.
Changes for v7:
  - Added hardware reset function
  - Marked functions that need to be called with the lock held as "unlocked"
  - Did some cleanup
Changes for v8:
  - None
Changes for v9:
  - None
Changes for v10:
  - None
---
 drivers/i2c/busses/i2c-aspeed.c | 202 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 202 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index a04021e3b3ab..6516addf5bb8 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,
@@ -116,6 +121,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;
@@ -136,6 +150,10 @@ struct aspeed_i2c_bus {
 	int				cmd_err;
 	/* Protected only by i2c_lock_bus */
 	int				master_xfer_result;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	struct i2c_client		*slave;
+	enum aspeed_i2c_slave_state	slave_state;
+#endif /* CONFIG_I2C_SLAVE */
 };
 
 static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
@@ -207,6 +225,110 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
 	return aspeed_i2c_reset(bus);
 }
 
+#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 = readl(bus->base + ASPEED_I2C_CMD_REG);
+	irq_status = readl(bus->base + 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 = readl(bus->base + 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);
+		writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+		writel(ASPEED_I2CD_S_TX_CMD, bus->base + 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);
+		writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+		writel(ASPEED_I2CD_S_TX_CMD, bus->base + 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);
+	writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+out:
+	spin_unlock(&bus->lock);
+	return irq_handled;
+}
+#endif /* CONFIG_I2C_SLAVE */
+
 /* precondition: bus.lock has been acquired. */
 static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 {
@@ -415,6 +537,13 @@ 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 /* CONFIG_I2C_SLAVE */
+
 	if (aspeed_i2c_master_irq(bus))
 		return IRQ_HANDLED;
 	else
@@ -465,9 +594,76 @@ 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)
+/* precondition: bus.lock has been acquired. */
+static void __aspeed_i2c_reg_slave(struct aspeed_i2c_bus *bus, u16 slave_addr)
+{
+	u32 addr_reg_val, func_ctrl_reg_val;
+
+	/* Set slave addr. */
+	addr_reg_val = readl(bus->base + ASPEED_I2C_DEV_ADDR_REG);
+	addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+	addr_reg_val |= slave_addr & ASPEED_I2CD_DEV_ADDR_MASK;
+	writel(addr_reg_val, bus->base + ASPEED_I2C_DEV_ADDR_REG);
+
+	/* Turn on slave mode. */
+	func_ctrl_reg_val = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+	writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+}
+
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+	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;
+	}
+
+	__aspeed_i2c_reg_slave(bus, client->addr);
+
+	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 = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+	writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+	bus->slave = NULL;
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return 0;
+}
+#endif /* CONFIG_I2C_SLAVE */
+
 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 /* CONFIG_I2C_SLAVE */
 };
 
 static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
@@ -542,6 +738,12 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) | fun_ctrl_reg,
 	       bus->base + ASPEED_I2C_FUN_CTRL_REG);
 
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	/* If slave has already been registered, re-enable it. */
+	if (bus->slave)
+		__aspeed_i2c_reg_slave(bus, bus->slave->addr);
+#endif /* CONFIG_I2C_SLAVE */
+
 	/* Set interrupt generation of I2C controller */
 	writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
 
-- 
2.13.0.506.g27d5fe0cd-goog

Re: [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver

[Joel Stanley]

On Sat, Jun 3, 2017 at 10:59 AM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> 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>

I wired up the first and second buses on the ast2500evb, and performed
the following test:

echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-1/new_device
echo 24c02 0x64 > /sys/bus/i2c/devices/i2c-0/new_device

echo "How about a nice game of chess? 0xDEADBEEF" >
/sys/bus/i2c/devices/1-1064/slave-eeprom

hexdump -C /sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus/i2c-0/0-0064/eeprom
00000000  48 6f 77 20 61 62 6f 75  74 20 61 20 6e 69 63 65  |How about a nice|
00000010  20 67 61 6d 65 20 6f 66  20 63 68 65 73 73 3f 20  | game of chess? |
00000020  30 78 44 45 41 44 42 45  45 46 0a 00 00 00 00 00  |0xDEADBEEF......|
00000030  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100

I also tested the other direction:

# echo "hack the planet" >
/sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus/i2c-0/0-0064/eeprom
# hexdump -C /sys/bus/i2c/devices/1-1064/slave-eeprom
00000000  68 61 63 6b 20 74 68 65  20 70 6c 61 6e 65 74 0a  |hack the planet.|
00000010  20 67 61 6d 65 20 6f 66  20 63 68 65 73 73 3f 20  | game of chess? |
00000020  30 78 44 45 41 44 42 45  45 46 0a 00 00 00 00 00  |0xDEADBEEF......|
00000030  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100

# md5sum /sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus
/i2c-0/0-0064/eeprom
0ae09da1dcbeb410cd311579fb6bd3b4
/sys/devices/platform/ahb/ahb:apb/ahb:apb:i2c@1e78a000/1e78a040.i2c-bus/i2c-0/0-0064/eeprom
# md5sum /sys/bus/i2c/devices/1-1064/slave-eeprom
0ae09da1dcbeb410cd311579fb6bd3b4  /sys/bus/i2c/devices/1-1064/slave-eeprom

Tested-by: Joel Stanley <joel@jms.id.au>

Cheers,

Joel

> ---
> 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.
> Changes for v7:
>   - Added hardware reset function
>   - Marked functions that need to be called with the lock held as "unlocked"
>   - Did some cleanup
> Changes for v8:
>   - None
> Changes for v9:
>   - None
> Changes for v10:
>   - None
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 202 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 202 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index a04021e3b3ab..6516addf5bb8 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,
> @@ -116,6 +121,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;
> @@ -136,6 +150,10 @@ struct aspeed_i2c_bus {
>         int                             cmd_err;
>         /* Protected only by i2c_lock_bus */
>         int                             master_xfer_result;
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +       struct i2c_client               *slave;
> +       enum aspeed_i2c_slave_state     slave_state;
> +#endif /* CONFIG_I2C_SLAVE */
>  };
>
>  static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
> @@ -207,6 +225,110 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>         return aspeed_i2c_reset(bus);
>  }
>
> +#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 = readl(bus->base + ASPEED_I2C_CMD_REG);
> +       irq_status = readl(bus->base + 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 = readl(bus->base + 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);
> +               writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
> +               writel(ASPEED_I2CD_S_TX_CMD, bus->base + 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);
> +               writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
> +               writel(ASPEED_I2CD_S_TX_CMD, bus->base + 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);
> +       writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
> +
> +out:
> +       spin_unlock(&bus->lock);
> +       return irq_handled;
> +}
> +#endif /* CONFIG_I2C_SLAVE */
> +
>  /* precondition: bus.lock has been acquired. */
>  static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
>  {
> @@ -415,6 +537,13 @@ 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 /* CONFIG_I2C_SLAVE */
> +
>         if (aspeed_i2c_master_irq(bus))
>                 return IRQ_HANDLED;
>         else
> @@ -465,9 +594,76 @@ 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)
> +/* precondition: bus.lock has been acquired. */
> +static void __aspeed_i2c_reg_slave(struct aspeed_i2c_bus *bus, u16 slave_addr)
> +{
> +       u32 addr_reg_val, func_ctrl_reg_val;
> +
> +       /* Set slave addr. */
> +       addr_reg_val = readl(bus->base + ASPEED_I2C_DEV_ADDR_REG);
> +       addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
> +       addr_reg_val |= slave_addr & ASPEED_I2CD_DEV_ADDR_MASK;
> +       writel(addr_reg_val, bus->base + ASPEED_I2C_DEV_ADDR_REG);
> +
> +       /* Turn on slave mode. */
> +       func_ctrl_reg_val = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +       func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> +       writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +}
> +
> +static int aspeed_i2c_reg_slave(struct i2c_client *client)
> +{
> +       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;
> +       }
> +
> +       __aspeed_i2c_reg_slave(bus, client->addr);
> +
> +       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 = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +       func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
> +       writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
> +
> +       bus->slave = NULL;
> +       spin_unlock_irqrestore(&bus->lock, flags);
> +
> +       return 0;
> +}
> +#endif /* CONFIG_I2C_SLAVE */
> +
>  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 /* CONFIG_I2C_SLAVE */
>  };
>
>  static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
> @@ -542,6 +738,12 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
>         writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) | fun_ctrl_reg,
>                bus->base + ASPEED_I2C_FUN_CTRL_REG);
>
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +       /* If slave has already been registered, re-enable it. */
> +       if (bus->slave)
> +               __aspeed_i2c_reg_slave(bus, bus->slave->addr);
> +#endif /* CONFIG_I2C_SLAVE */
> +
>         /* Set interrupt generation of I2C controller */
>         writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
>
> --
> 2.13.0.506.g27d5fe0cd-goog
>

Re: [PATCH v10 5/5] i2c: aspeed: added slave support for Aspeed I2C driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

On Fri, Jun 02, 2017 at 06:29:52PM -0700, Brendan Higgins wrote:
> 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>

Nice, looks good to me!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]