Linux I2C development
 help / color / mirror / Atom feed
* [PATCH v2 3/8] i2c: bcm2835: Use ratelimited logging on transfer errors
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
  To: wsa, swarren, eric
  Cc: linux-kernel, linux-i2c, linux-arm-kernel, linux-rpi-kernel,
	Noralf Trønnes
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>

Writing to an AT24C32 generates on average 2x i2c transfer errors per
32-byte page write. Which amounts to a lot for a 4k write. This is due
to the fact that the chip doesn't respond during it's internal write
cycle when the at24 driver tries and retries the next write.
Reduce this flooding of the log by using dev_err_ratelimited().

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
 drivers/i2c/busses/i2c-bcm2835.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index df036ed..370a322 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -207,7 +207,8 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
 	    (msg->flags & I2C_M_IGNORE_NAK))
 		return 0;
 
-	dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
+	dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
+			    i2c_dev->msg_err);
 
 	if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
 		return -EREMOTEIO;
-- 
2.8.2

^ permalink raw reply related

* [PATCH v2 5/8] i2c: bcm2835: Add support for Repeated Start Condition
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
  To: wsa, swarren, eric
  Cc: linux-kernel, linux-i2c, linux-arm-kernel, linux-rpi-kernel,
	Noralf Trønnes
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>

Documentation/i2c/i2c-protocol states that Combined transactions should
separate messages with a Start bit and end the whole transaction with a
Stop bit. This patch adds support for issuing only a Start between
messages instead of a Stop followed by a Start.

This implementation differs from downstream i2c-bcm2708 in 2 respects:
- it uses an interrupt to detect that the transfer is active instead
  of using polling. There is no interrupt for Transfer Active, but by
  not prefilling the FIFO it's possible to use the TXW interrupt.
- when resetting/disabling the controller between transfers it writes
  CLEAR to the control register instead of just zero.
  Using just zero gave many errors. This might be the reason why
  downstream had to disable this feature and make it available with a
  module parameter.

I have run thousands of transfers to a DS1307 (rtc), MMA8451 (accel)
and AT24C32 (eeprom) in parallel without problems.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/i2c/busses/i2c-bcm2835.c | 101 ++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 38 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 4e08add..f3a1472 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -63,6 +63,7 @@ struct bcm2835_i2c_dev {
 	struct i2c_adapter adapter;
 	struct completion completion;
 	struct i2c_msg *curr_msg;
+	int num_msgs;
 	u32 msg_err;
 	u8 *msg_buf;
 	size_t msg_buf_remaining;
@@ -109,6 +110,45 @@ static void bcm2835_drain_rxfifo(struct bcm2835_i2c_dev *i2c_dev)
 	}
 }
 
+/*
+ * Repeated Start Condition (Sr)
+ * The BCM2835 ARM Peripherals datasheet mentions a way to trigger a Sr when it
+ * talks about reading from a slave with 10 bit address. This is achieved by
+ * issuing a write, poll the I2CS.TA flag and wait for it to be set, and then
+ * issue a read.
+ * A comment in https://github.com/raspberrypi/linux/issues/254 shows how the
+ * firmware actually does it using polling and says that it's a workaround for
+ * a problem in the state machine.
+ * It turns out that it is possible to use the TXW interrupt to know when the
+ * transfer is active, provided the FIFO has not been prefilled.
+ */
+
+static void bcm2835_i2c_start_transfer(struct bcm2835_i2c_dev *i2c_dev)
+{
+	u32 c = BCM2835_I2C_C_ST | BCM2835_I2C_C_I2CEN;
+	struct i2c_msg *msg = i2c_dev->curr_msg;
+	bool last_msg = (i2c_dev->num_msgs == 1);
+
+	if (!i2c_dev->num_msgs)
+		return;
+
+	i2c_dev->num_msgs--;
+	i2c_dev->msg_buf = msg->buf;
+	i2c_dev->msg_buf_remaining = msg->len;
+
+	if (msg->flags & I2C_M_RD)
+		c |= BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
+	else
+		c |= BCM2835_I2C_C_INTT;
+
+	if (last_msg)
+		c |= BCM2835_I2C_C_INTD;
+
+	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
+	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
+	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+}
+
 static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
 {
 	struct bcm2835_i2c_dev *i2c_dev = data;
@@ -142,6 +182,12 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
 		}
 
 		bcm2835_fill_txfifo(i2c_dev);
+
+		if (i2c_dev->num_msgs && !i2c_dev->msg_buf_remaining) {
+			i2c_dev->curr_msg++;
+			bcm2835_i2c_start_transfer(i2c_dev);
+		}
+
 		return IRQ_HANDLED;
 	}
 
@@ -166,30 +212,25 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
-				struct i2c_msg *msg)
+static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+			    int num)
 {
-	u32 c;
+	struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	unsigned long time_left;
+	int i;
 
-	i2c_dev->curr_msg = msg;
-	i2c_dev->msg_buf = msg->buf;
-	i2c_dev->msg_buf_remaining = msg->len;
-	reinit_completion(&i2c_dev->completion);
-
-	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, BCM2835_I2C_C_CLEAR);
+	for (i = 0; i < (num - 1); i++)
+		if (msgs[i].flags & I2C_M_RD) {
+			dev_warn_once(i2c_dev->dev,
+				      "only one read message supported, has to be last\n");
+			return -EOPNOTSUPP;
+		}
 
-	if (msg->flags & I2C_M_RD) {
-		c = BCM2835_I2C_C_READ | BCM2835_I2C_C_INTR;
-	} else {
-		c = BCM2835_I2C_C_INTT;
-		bcm2835_fill_txfifo(i2c_dev);
-	}
-	c |= BCM2835_I2C_C_ST | BCM2835_I2C_C_INTD | BCM2835_I2C_C_I2CEN;
+	i2c_dev->curr_msg = msgs;
+	i2c_dev->num_msgs = num;
+	reinit_completion(&i2c_dev->completion);
 
-	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_A, msg->addr);
-	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DLEN, msg->len);
-	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, c);
+	bcm2835_i2c_start_transfer(i2c_dev);
 
 	time_left = wait_for_completion_timeout(&i2c_dev->completion,
 						BCM2835_I2C_TIMEOUT);
@@ -200,32 +241,16 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
 		return -ETIMEDOUT;
 	}
 
-	if (likely(!i2c_dev->msg_err))
-		return 0;
+	if (!i2c_dev->msg_err)
+		return num;
 
 	dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
 			    i2c_dev->msg_err);
 
 	if (i2c_dev->msg_err & BCM2835_I2C_S_ERR)
 		return -EREMOTEIO;
-	else
-		return -EIO;
-}
-
-static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
-			    int num)
-{
-	struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
-	int i;
-	int ret = 0;
-
-	for (i = 0; i < num; i++) {
-		ret = bcm2835_i2c_xfer_msg(i2c_dev, &msgs[i]);
-		if (ret)
-			break;
-	}
 
-	return ret ?: i;
+	return -EIO;
 }
 
 static u32 bcm2835_i2c_func(struct i2c_adapter *adap)
-- 
2.8.2

^ permalink raw reply related

* [PATCH v2 7/8] i2c: bcm2835: Add support for dynamic clock
From: Noralf Trønnes @ 2016-09-27 11:57 UTC (permalink / raw)
  To: wsa, swarren, eric
  Cc: linux-kernel, linux-i2c, linux-arm-kernel, linux-rpi-kernel,
	Noralf Trønnes
In-Reply-To: <1474977426-3272-1-git-send-email-noralf@tronnes.org>

Support a dynamic clock by reading the frequency and setting the
divisor in the transfer function instead of during probe.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/i2c/busses/i2c-bcm2835.c | 51 +++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
index 79dd15f..5aed514 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -58,6 +58,7 @@ struct bcm2835_i2c_dev {
 	void __iomem *regs;
 	struct clk *clk;
 	int irq;
+	u32 bus_clk_rate;
 	struct i2c_adapter adapter;
 	struct completion completion;
 	struct i2c_msg *curr_msg;
@@ -78,6 +79,30 @@ static inline u32 bcm2835_i2c_readl(struct bcm2835_i2c_dev *i2c_dev, u32 reg)
 	return readl(i2c_dev->regs + reg);
 }
 
+static int bcm2835_i2c_set_divider(struct bcm2835_i2c_dev *i2c_dev)
+{
+	u32 divider;
+
+	divider = DIV_ROUND_UP(clk_get_rate(i2c_dev->clk),
+			       i2c_dev->bus_clk_rate);
+	/*
+	 * Per the datasheet, the register is always interpreted as an even
+	 * number, by rounding down. In other words, the LSB is ignored. So,
+	 * if the LSB is set, increment the divider to avoid any issue.
+	 */
+	if (divider & 1)
+		divider++;
+	if ((divider < BCM2835_I2C_CDIV_MIN) ||
+	    (divider > BCM2835_I2C_CDIV_MAX)) {
+		dev_err_ratelimited(i2c_dev->dev, "Invalid clock-frequency\n");
+		return -EINVAL;
+	}
+
+	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
+
+	return 0;
+}
+
 static void bcm2835_fill_txfifo(struct bcm2835_i2c_dev *i2c_dev)
 {
 	u32 val;
@@ -215,7 +240,7 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 {
 	struct bcm2835_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	unsigned long time_left;
-	int i;
+	int i, ret;
 
 	for (i = 0; i < (num - 1); i++)
 		if (msgs[i].flags & I2C_M_RD) {
@@ -224,6 +249,10 @@ static int bcm2835_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
 			return -EOPNOTSUPP;
 		}
 
+	ret = bcm2835_i2c_set_divider(i2c_dev);
+	if (ret)
+		return ret;
+
 	i2c_dev->curr_msg = msgs;
 	i2c_dev->num_msgs = num;
 	reinit_completion(&i2c_dev->completion);
@@ -274,7 +303,6 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
 {
 	struct bcm2835_i2c_dev *i2c_dev;
 	struct resource *mem, *irq;
-	u32 bus_clk_rate, divider;
 	int ret;
 	struct i2c_adapter *adap;
 
@@ -298,27 +326,12 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
 	}
 
 	ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-				   &bus_clk_rate);
+				   &i2c_dev->bus_clk_rate);
 	if (ret < 0) {
 		dev_warn(&pdev->dev,
 			 "Could not read clock-frequency property\n");
-		bus_clk_rate = 100000;
-	}
-
-	divider = DIV_ROUND_UP(clk_get_rate(i2c_dev->clk), bus_clk_rate);
-	/*
-	 * Per the datasheet, the register is always interpreted as an even
-	 * number, by rounding down. In other words, the LSB is ignored. So,
-	 * if the LSB is set, increment the divider to avoid any issue.
-	 */
-	if (divider & 1)
-		divider++;
-	if ((divider < BCM2835_I2C_CDIV_MIN) ||
-	    (divider > BCM2835_I2C_CDIV_MAX)) {
-		dev_err(&pdev->dev, "Invalid clock-frequency\n");
-		return -ENODEV;
+		i2c_dev->bus_clk_rate = 100000;
 	}
-	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_DIV, divider);
 
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!irq) {
-- 
2.8.2

^ permalink raw reply related

* Re: [PATCH v2 3/8] i2c: bcm2835: Use ratelimited logging on transfer errors
From: Martin Sperl @ 2016-09-27 13:01 UTC (permalink / raw)
  To: Noralf Trønnes
  Cc: wsa, swarren, eric, linux-rpi-kernel, linux-kernel,
	linux-arm-kernel, linux-i2c
In-Reply-To: <1474977426-3272-4-git-send-email-noralf@tronnes.org>


> On 27 Sep 2016, at 13:57, Noralf Trønnes <noralf@tronnes.org> wrote:
> 
> Writing to an AT24C32 generates on average 2x i2c transfer errors per
> 32-byte page write. Which amounts to a lot for a 4k write. This is due
> to the fact that the chip doesn't respond during it's internal write
> cycle when the at24 driver tries and retries the next write.
> Reduce this flooding of the log by using dev_err_ratelimited().
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> ---
> drivers/i2c/busses/i2c-bcm2835.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
> index df036ed..370a322 100644
> --- a/drivers/i2c/busses/i2c-bcm2835.c
> +++ b/drivers/i2c/busses/i2c-bcm2835.c
> @@ -207,7 +207,8 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
>        (msg->flags & I2C_M_IGNORE_NAK))
>        return 0;
> 
> -    dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
> +    dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
> +                i2c_dev->msg_err);
Do we really need this error message at all?

Maybe just remove it instead, because error messages during 
"normal"/successfull operations of at24 seems  strange.

Or make it a debug message instead.

Martin

^ permalink raw reply

* Re: [PATCH v2 8/8] ARM: bcm2835: Disable i2c2 in the Device Tree
From: Stefan Wahren @ 2016-09-27 17:25 UTC (permalink / raw)
  To: eric, wsa, swarren, Noralf Trønnes
  Cc: linux-kernel, linux-i2c, linux-rpi-kernel, linux-arm-kernel
In-Reply-To: <1474977426-3272-9-git-send-email-noralf@tronnes.org>


> Noralf Trønnes <noralf@tronnes.org> hat am 27. September 2016 um 13:57
> geschrieben:
> 
> 
> i2c2 is connected to the HDMI connector and is controlled by the
> firmware. Disable it to stay out of harms way.

Until this point the commit message is okay, the rest is more confusing.

Btw this should avoid a warning about missing clock frequency.

> 
> From the downstream commit:
> i2c-bcm2708/BCM270X_DT: Add support for I2C2
> 
> The third I2C bus (I2C2) is normally reserved for HDMI use. Careless
> use of this bus can break an attached display - use with caution.
> 
> It is recommended to disable accesses by VideoCore by setting
> hdmi_ignore_edid=1 or hdmi_edid_file=1 in config.txt.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> index e9b47b2..8bffbee 100644
> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
> @@ -59,10 +59,6 @@
>  	clock-frequency = <100000>;
>  };
>  
> -&i2c2 {
> -	status = "okay";
> -};
> -

I'm not sure if this the right fix. According to bcm283x.dtsi the 3 i2c busses
have the same compatible string "brcm,bcm2835-i2c", but the changelog suggests
that this bus is "special".

Shouldn't we use a different compatible string? Our intention isn't to disable
i2c2 but avoid any claims of the usual i2c driver.

>  &sdhci {
>  	status = "okay";
>  	bus-width = <4>;
> -- 
> 2.8.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 8/8] ARM: bcm2835: Disable i2c2 in the Device Tree
From: Jan Kandziora @ 2016-09-27 18:53 UTC (permalink / raw)
  To: Stefan Wahren, eric, wsa, swarren, Noralf Trønnes
  Cc: linux-kernel, linux-i2c, linux-rpi-kernel, linux-arm-kernel
In-Reply-To: <256118042.194076.147d5d83-f402-4ba4-b7de-4ab57a0e29ad.open-xchange@email.1und1.de>

Am 27.09.2016 um 19:25 schrieb Stefan Wahren:
> 
>> Noralf Trønnes <noralf@tronnes.org> hat am 27. September 2016 um 13:57
>> geschrieben:
>>
>>
>> i2c2 is connected to the HDMI connector and is controlled by the
>> firmware. Disable it to stay out of harms way.
> 
> Until this point the commit message is okay, the rest is more confusing.
> 
> Btw this should avoid a warning about missing clock frequency.
> 
>>
>> From the downstream commit:
>> i2c-bcm2708/BCM270X_DT: Add support for I2C2
>>
>> The third I2C bus (I2C2) is normally reserved for HDMI use. Careless
>> use of this bus can break an attached display - use with caution.
>>
>> It is recommended to disable accesses by VideoCore by setting
>> hdmi_ignore_edid=1 or hdmi_edid_file=1 in config.txt.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> b/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> index e9b47b2..8bffbee 100644
>> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> @@ -59,10 +59,6 @@
>>  	clock-frequency = <100000>;
>>  };
>>  
>> -&i2c2 {
>> -	status = "okay";
>> -};
>> -
> 
> I'm not sure if this the right fix. According to bcm283x.dtsi the 3 i2c busses
> have the same compatible string "brcm,bcm2835-i2c", but the changelog suggests
> that this bus is "special".
> 
> Shouldn't we use a different compatible string? Our intention isn't to disable
> i2c2 but avoid any claims of the usual i2c driver.
> 
i2c2 should not be generally disabled.

There's dtparam=i2c2_iknowwhatimdoing which enables CPU access to this
bus. It's useful for reading out the monitor EDID by the CPU, and for
accessing controls (backlight, volume) on certain monitors.

And I have a I2C touchscreen controller on HDMI DDC.



>>  &sdhci {
>>  	status = "okay";
>>  	bus-width = <4>;
>> -- 
>> 2.8.2
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH v2 8/8] ARM: bcm2835: Disable i2c2 in the Device Tree
From: Noralf Trønnes @ 2016-09-27 19:23 UTC (permalink / raw)
  To: Stefan Wahren, eric, wsa, swarren
  Cc: linux-kernel, linux-i2c, linux-rpi-kernel, linux-arm-kernel
In-Reply-To: <256118042.194076.147d5d83-f402-4ba4-b7de-4ab57a0e29ad.open-xchange@email.1und1.de>


Den 27.09.2016 19:25, skrev Stefan Wahren:
>> Noralf Trønnes <noralf@tronnes.org> hat am 27. September 2016 um 13:57
>> geschrieben:
>>
>>
>> i2c2 is connected to the HDMI connector and is controlled by the
>> firmware. Disable it to stay out of harms way.
> Until this point the commit message is okay, the rest is more confusing.
>
> Btw this should avoid a warning about missing clock frequency.
>
>>  From the downstream commit:
>> i2c-bcm2708/BCM270X_DT: Add support for I2C2
>>
>> The third I2C bus (I2C2) is normally reserved for HDMI use. Careless
>> use of this bus can break an attached display - use with caution.
>>
>> It is recommended to disable accesses by VideoCore by setting
>> hdmi_ignore_edid=1 or hdmi_edid_file=1 in config.txt.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   arch/arm/boot/dts/bcm2835-rpi.dtsi | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> b/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> index e9b47b2..8bffbee 100644
>> --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
>> @@ -59,10 +59,6 @@
>>   	clock-frequency = <100000>;
>>   };
>>   
>> -&i2c2 {
>> -	status = "okay";
>> -};
>> -
> I'm not sure if this the right fix. According to bcm283x.dtsi the 3 i2c busses
> have the same compatible string "brcm,bcm2835-i2c", but the changelog suggests
> that this bus is "special".

I just rounded up all the differences from downstream that I knew about
into this patchset. But looking closer I see that the vc4 driver uses
i2c2. So I'll drop this patch.


Noralf.


> Shouldn't we use a different compatible string? Our intention isn't to disable
> i2c2 but avoid any claims of the usual i2c driver.
>
>>   &sdhci {
>>   	status = "okay";
>>   	bus-width = <4>;
>> -- 
>> 2.8.2
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 3/8] i2c: bcm2835: Use ratelimited logging on transfer errors
From: Noralf Trønnes @ 2016-09-27 19:27 UTC (permalink / raw)
  To: Martin Sperl
  Cc: wsa, swarren, eric, linux-rpi-kernel, linux-kernel,
	linux-arm-kernel, linux-i2c
In-Reply-To: <74D9ED33-1B99-4ED6-88B5-879950D160C8@martin.sperl.org>


Den 27.09.2016 15:01, skrev Martin Sperl:
>> On 27 Sep 2016, at 13:57, Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> Writing to an AT24C32 generates on average 2x i2c transfer errors per
>> 32-byte page write. Which amounts to a lot for a 4k write. This is due
>> to the fact that the chip doesn't respond during it's internal write
>> cycle when the at24 driver tries and retries the next write.
>> Reduce this flooding of the log by using dev_err_ratelimited().
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Reviewed-by: Eric Anholt <eric@anholt.net>
>> ---
>> drivers/i2c/busses/i2c-bcm2835.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
>> index df036ed..370a322 100644
>> --- a/drivers/i2c/busses/i2c-bcm2835.c
>> +++ b/drivers/i2c/busses/i2c-bcm2835.c
>> @@ -207,7 +207,8 @@ static int bcm2835_i2c_xfer_msg(struct bcm2835_i2c_dev *i2c_dev,
>>         (msg->flags & I2C_M_IGNORE_NAK))
>>         return 0;
>>
>> -    dev_err(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
>> +    dev_err_ratelimited(i2c_dev->dev, "i2c transfer failed: %x\n",
>> +                i2c_dev->msg_err);
> Do we really need this error message at all?
>
> Maybe just remove it instead, because error messages during
> "normal"/successfull operations of at24 seems  strange.
>
> Or make it a debug message instead.

I have looked through 64 i2c bus drivers, 8 use dev_err and 2 use dev_warn
on transfer errors (not timeouts). Several use dev_dbg.

I'll change it to dev_dbg instead. Thanks.

Noralf.

^ permalink raw reply

* RE: [PATCH] i2c: imx: defer probe if bus recovery GPIOs are not ready
From: Leo Li @ 2016-09-27 18:31 UTC (permalink / raw)
  To: Stefan Agner, wsa@the-dreams.de
  Cc: linux-i2c@vger.kernel.org, u.kleine-koenig@pengutronix.de,
	linux-kernel@vger.kernel.org
In-Reply-To: <20160927001858.11601-1-stefan@agner.ch>



> -----Original Message-----
> From: Stefan Agner [mailto:stefan@agner.ch]
> Sent: Monday, September 26, 2016 7:19 PM
> To: wsa@the-dreams.de
> Cc: Leo Li <leoyang.li@nxp.com>; linux-i2c@vger.kernel.org; u.kleine-
> koenig@pengutronix.de; linux-kernel@vger.kernel.org; Stefan Agner
> <stefan@agner.ch>
> Subject: [PATCH] i2c: imx: defer probe if bus recovery GPIOs are not ready
> 
> Some SoC might load the GPIO driver after the I2C driver and
> using the I2C bus recovery mechanism via GPIOs. In this case
> it is crucial to defer probing if the GPIO request functions
> do so, otherwise the I2C driver gets loaded without recovery
> mechanisms enabled.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Li Yang <leoyang.li@nxp.com>

> ---
> This is an actual issue on NXP Vybrid devices on which the GPIO
> driver gets loaded rather later.
> 
> --
> Stefan
> 
>  drivers/i2c/busses/i2c-imx.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 592a8f2..47fc1f1 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1009,10 +1009,13 @@ static int i2c_imx_init_recovery_info(struct
> imx_i2c_struct *i2c_imx,
>  	rinfo->sda_gpio = of_get_named_gpio(pdev->dev.of_node, "sda-gpios",
> 0);
>  	rinfo->scl_gpio = of_get_named_gpio(pdev->dev.of_node, "scl-gpios",
> 0);
> 
> -	if (!gpio_is_valid(rinfo->sda_gpio) ||
> -	    !gpio_is_valid(rinfo->scl_gpio) ||
> -	    IS_ERR(i2c_imx->pinctrl_pins_default) ||
> -	    IS_ERR(i2c_imx->pinctrl_pins_gpio)) {
> +	if (rinfo->sda_gpio == -EPROBE_DEFER ||
> +	    rinfo->scl_gpio == -EPROBE_DEFER) {
> +		return -EPROBE_DEFER;
> +	} else if (!gpio_is_valid(rinfo->sda_gpio) ||
> +		   !gpio_is_valid(rinfo->scl_gpio) ||
> +		   IS_ERR(i2c_imx->pinctrl_pins_default) ||
> +		   IS_ERR(i2c_imx->pinctrl_pins_gpio)) {
>  		dev_dbg(&pdev->dev, "recovery information incomplete\n");
>  		return 0;
>  	}
> --
> 2.10.0

^ permalink raw reply

* [PATCH v3 03/10] i2c: i801: minor formatting issues
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

No functional changes, just typos and remove unused #define.

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

no changes in v2
---
 drivers/i2c/busses/i2c-i801.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 7334831..4696319bf 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -186,10 +186,10 @@
 #define SMBHSTSTS_INTR		0x02
 #define SMBHSTSTS_HOST_BUSY	0x01
 
-/* Host Notify Status registers bits */
+/* Host Notify Status register bits */
 #define SMBSLVSTS_HST_NTFY_STS	1
 
-/* Host Notify Command registers bits */
+/* Host Notify Command register bits */
 #define SMBSLVCMD_HST_NTFY_INTREN	0x01
 
 #define STATUS_ERROR_FLAGS	(SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
@@ -270,8 +270,6 @@ struct i801_priv {
 	struct smbus_host_notify *host_notify;
 };
 
-#define SMBHSTNTFY_SIZE		8
-
 #define FEATURE_SMBUS_PEC	(1 << 0)
 #define FEATURE_BLOCK_BUFFER	(1 << 1)
 #define FEATURE_BLOCK_PROC	(1 << 2)
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 07/10] i2c: i2c-smbus: remove double warning message
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

i2c_handle_smbus_host_notify() returns an error code when it fails.
If the caller wants to warn when an error is returned, we should not
partially warn in the function itself.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

new in v2
---
 drivers/i2c/i2c-smbus.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 35e4f1a..819e5f2 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -347,7 +347,6 @@ int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
 
 	if (host_notify->pending) {
 		spin_unlock_irqrestore(&host_notify->lock, flags);
-		dev_warn(&adapter->dev, "Host Notify already scheduled.\n");
 		return -EBUSY;
 	}
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 08/10] i2c: i2c-smbus: fix return value of i2c_handle_smbus_host_notify()
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

schedule_work() returns a boolean (true on success, false if the work
was already queued).
This doesn't match the "negative number on error" return model that the
function exports.

Given that schedule_work() will always return true (we have an internal
.pending check protected by a spinlock), just return false and ignore
the return value of schedule_work().

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

new in v2
---
 drivers/i2c/i2c-smbus.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 819e5f2..f6780de 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -357,7 +357,10 @@ int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
 	host_notify->pending = true;
 	spin_unlock_irqrestore(&host_notify->lock, flags);
 
-	return schedule_work(&host_notify->work);
+	/* schedule_work is called if .pending is false, so it can't fail. */
+	schedule_work(&host_notify->work);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 09/10] i2c: i801: check if Host Notify is available during interrupt
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

There is a chance we get called while Host Notify is not available (yet),
so we need to clear the Host Notify bit in those rare case.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v3:
- renamed from "i2c: i801: warn on i2c_handle_smbus_host_notify() errors"
- removed the warnings and just kept the test
- added some comments on why the errors are not treated

no changes in v2 (comments addressed in the previous patches)

i2c: i2c-i801: remove warnings on the handle host notify

add a comment on i2c-i801
---
 drivers/i2c/busses/i2c-i801.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 422cce2..38e2421 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -580,11 +580,20 @@ static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
 	unsigned short addr;
 	unsigned int data;
 
+	if (unlikely(!priv->host_notify))
+		goto out;
+
 	addr = inb_p(SMBNTFDADD(priv)) >> 1;
 	data = inw_p(SMBNTFDDAT(priv));
 
+	/*
+	 * We don't notify about missed host notify events when the adapter
+	 * lags behind. The operation is still scheduled and will get
+	 * eventually processed.
+	 */
 	i2c_handle_smbus_host_notify(priv->host_notify, addr, data);
 
+out:
 	/* clear Host Notify bit and return */
 	outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
 	return IRQ_HANDLED;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 00/10] i2c: Host Notify / i801 fixes
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel

Hi,

this is mostly a resend of the series.

The only change is in patch 9/10, where I removed the dev_warn that could
be an issue on some laggish operations on Synaptics RMI4 devices (when
retrieving the heatmap). It also appeared that these warning were sent quite
regularly with the Elantech touchpads, so let's just disable those given
that they are mostly harmless.

Cheers,
Benjamin

Benjamin Tissoires (10):
  i2c: i2c-smbus: prevent races on remove when Host Notify is used
  i2c: i801: store and restore the SLVCMD register at load and unload
  i2c: i801: minor formatting issues
  i2c: i801: use BIT() macro for bits definition
  i2c: i801: use the BIT() macro for FEATURES_* also
  i2c: i801: do not report an error if FEATURE_HOST_NOTIFY is not set
  i2c: i2c-smbus: remove double warning message
  i2c: i2c-smbus: fix return value of i2c_handle_smbus_host_notify()
  i2c: i801: check if Host Notify is available during interrupt
  i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0

 drivers/i2c/busses/i2c-i801.c | 123 +++++++++++++++++++++++++-----------------
 drivers/i2c/i2c-smbus.c       |  25 ++++++++-
 include/linux/i2c-smbus.h     |   1 +
 3 files changed, 99 insertions(+), 50 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH v3 01/10] i2c: i2c-smbus: prevent races on remove when Host Notify is used
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

struct host_notify contains its own workqueue, so there is a race when
the adapter gets removed:
- the adapter schedules a notification
- the notification is on hold
- the adapter gets removed and all its children too
- the worker fires and access illegal memory

Add an API to actually kill the workqueue and prevent it to access such
illegal memory. I couldn't find a reliable way of automatically calling
this, so it's the responsibility of the adapter driver to clean up after
itself.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

changes in v2:
- changed i801_disable_host_notify() parameter
- changed the comments to actually match the behavior
---
 drivers/i2c/busses/i2c-i801.c | 13 +++++++++++++
 drivers/i2c/i2c-smbus.c       | 19 +++++++++++++++++++
 include/linux/i2c-smbus.h     |  1 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 22a0ed4..b494a85 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -959,6 +959,18 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
 	return 0;
 }
 
+static void i801_disable_host_notify(struct i801_priv *priv)
+{
+
+	if (!(priv->features & FEATURE_HOST_NOTIFY))
+		return;
+
+	/* disable Host Notify... */
+	outb_p(0, SMBSLVCMD(priv));
+	/* ...and process the already queued notifications */
+	i2c_cancel_smbus_host_notify(priv->host_notify);
+}
+
 static const struct i2c_algorithm smbus_algorithm = {
 	.smbus_xfer	= i801_access,
 	.functionality	= i801_func,
@@ -1647,6 +1659,7 @@ static void i801_remove(struct pci_dev *dev)
 	pm_runtime_forbid(&dev->dev);
 	pm_runtime_get_noresume(&dev->dev);
 
+	i801_disable_host_notify(priv);
 	i801_del_mux(priv);
 	i2c_del_adapter(&priv->adapter);
 	i801_acpi_remove(priv);
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b0d2679..35e4f1a 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -279,6 +279,8 @@ static void smbus_host_notify_work(struct work_struct *work)
  * Returns a struct smbus_host_notify pointer on success, and NULL on failure.
  * The resulting smbus_host_notify must not be freed afterwards, it is a
  * managed resource already.
+ * To prevent races on remove, the caller needs to stop the embedded worker
+ * by calling i2c_cancel_smbus_host_notify().
  */
 struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap)
 {
@@ -299,6 +301,23 @@ struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap)
 EXPORT_SYMBOL_GPL(i2c_setup_smbus_host_notify);
 
 /**
+ * i2c_cancel_smbus_host_notify - Terminate any active Host Notification.
+ * @host_notify: the host_notify object to terminate
+ *
+ * Process any pending Host Notifcation and prevent new ones to be added.
+ * Must be called to ensure no races between the adaptor being removed and
+ * the Host Notification being processed.
+ */
+void i2c_cancel_smbus_host_notify(struct smbus_host_notify *host_notify)
+{
+	if (!host_notify)
+		return;
+
+	cancel_work_sync(&host_notify->work);
+}
+EXPORT_SYMBOL_GPL(i2c_cancel_smbus_host_notify);
+
+/**
  * i2c_handle_smbus_host_notify - Forward a Host Notify event to the correct
  * I2C client.
  * @host_notify: the struct host_notify attached to the relevant adapter
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index c2e3324..ac02827 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -76,5 +76,6 @@ struct smbus_host_notify {
 struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap);
 int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
 				 unsigned short addr, unsigned int data);
+void i2c_cancel_smbus_host_notify(struct smbus_host_notify *host_notify);
 
 #endif /* _LINUX_I2C_SMBUS_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 02/10] i2c: i801: store and restore the SLVCMD register at load and unload
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

Also do not override any other configuration in this register.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

new in v2
---
 drivers/i2c/busses/i2c-i801.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b494a85..7334831 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -240,6 +240,7 @@ struct i801_priv {
 	struct i2c_adapter adapter;
 	unsigned long smba;
 	unsigned char original_hstcfg;
+	unsigned char original_slvcmd;
 	struct pci_dev *pci_dev;
 	unsigned int features;
 
@@ -952,7 +953,12 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
 	if (!priv->host_notify)
 		return -ENOMEM;
 
-	outb_p(SMBSLVCMD_HST_NTFY_INTREN, SMBSLVCMD(priv));
+	priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
+
+	if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd))
+		outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd,
+		       SMBSLVCMD(priv));
+
 	/* clear Host Notify bit to allow a new notification */
 	outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
 
@@ -961,12 +967,11 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
 
 static void i801_disable_host_notify(struct i801_priv *priv)
 {
-
 	if (!(priv->features & FEATURE_HOST_NOTIFY))
 		return;
 
 	/* disable Host Notify... */
-	outb_p(0, SMBSLVCMD(priv));
+	outb_p(priv->original_slvcmd, SMBSLVCMD(priv));
 	/* ...and process the already queued notifications */
 	i2c_cancel_smbus_host_notify(priv->host_notify);
 }
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 04/10] i2c: i801: use BIT() macro for bits definition
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

i801 mixes hexadecimal and decimal values for defining bits. However,
we have a nice BIT() macro for this exact purpose.

No functional changes, cleanup only.

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

no changes in v2
---
 drivers/i2c/busses/i2c-i801.c | 50 +++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 4696319bf..05ce265 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -136,26 +136,26 @@
 #define SBREG_SMBCTRL		0xc6000c
 
 /* Host status bits for SMBPCISTS */
-#define SMBPCISTS_INTS		0x08
+#define SMBPCISTS_INTS		BIT(3)
 
 /* Control bits for SMBPCICTL */
-#define SMBPCICTL_INTDIS	0x0400
+#define SMBPCICTL_INTDIS	BIT(10)
 
 /* Host configuration bits for SMBHSTCFG */
-#define SMBHSTCFG_HST_EN	1
-#define SMBHSTCFG_SMB_SMI_EN	2
-#define SMBHSTCFG_I2C_EN	4
+#define SMBHSTCFG_HST_EN	BIT(0)
+#define SMBHSTCFG_SMB_SMI_EN	BIT(1)
+#define SMBHSTCFG_I2C_EN	BIT(2)
 
 /* TCO configuration bits for TCOCTL */
-#define TCOCTL_EN		0x0100
+#define TCOCTL_EN		BIT(8)
 
 /* Auxiliary status register bits, ICH4+ only */
-#define SMBAUXSTS_CRCE		1
-#define SMBAUXSTS_STCO		2
+#define SMBAUXSTS_CRCE		BIT(0)
+#define SMBAUXSTS_STCO		BIT(1)
 
 /* Auxiliary control register bits, ICH4+ only */
-#define SMBAUXCTL_CRC		1
-#define SMBAUXCTL_E32B		2
+#define SMBAUXCTL_CRC		BIT(0)
+#define SMBAUXCTL_E32B		BIT(1)
 
 /* Other settings */
 #define MAX_RETRIES		400
@@ -170,27 +170,27 @@
 #define I801_I2C_BLOCK_DATA	0x18	/* ICH5 and later */
 
 /* I801 Host Control register bits */
-#define SMBHSTCNT_INTREN	0x01
-#define SMBHSTCNT_KILL		0x02
-#define SMBHSTCNT_LAST_BYTE	0x20
-#define SMBHSTCNT_START		0x40
-#define SMBHSTCNT_PEC_EN	0x80	/* ICH3 and later */
+#define SMBHSTCNT_INTREN	BIT(0)
+#define SMBHSTCNT_KILL		BIT(1)
+#define SMBHSTCNT_LAST_BYTE	BIT(5)
+#define SMBHSTCNT_START		BIT(6)
+#define SMBHSTCNT_PEC_EN	BIT(7)	/* ICH3 and later */
 
 /* I801 Hosts Status register bits */
-#define SMBHSTSTS_BYTE_DONE	0x80
-#define SMBHSTSTS_INUSE_STS	0x40
-#define SMBHSTSTS_SMBALERT_STS	0x20
-#define SMBHSTSTS_FAILED	0x10
-#define SMBHSTSTS_BUS_ERR	0x08
-#define SMBHSTSTS_DEV_ERR	0x04
-#define SMBHSTSTS_INTR		0x02
-#define SMBHSTSTS_HOST_BUSY	0x01
+#define SMBHSTSTS_BYTE_DONE	BIT(7)
+#define SMBHSTSTS_INUSE_STS	BIT(6)
+#define SMBHSTSTS_SMBALERT_STS	BIT(5)
+#define SMBHSTSTS_FAILED	BIT(4)
+#define SMBHSTSTS_BUS_ERR	BIT(3)
+#define SMBHSTSTS_DEV_ERR	BIT(2)
+#define SMBHSTSTS_INTR		BIT(1)
+#define SMBHSTSTS_HOST_BUSY	BIT(0)
 
 /* Host Notify Status register bits */
-#define SMBSLVSTS_HST_NTFY_STS	1
+#define SMBSLVSTS_HST_NTFY_STS	BIT(0)
 
 /* Host Notify Command register bits */
-#define SMBSLVCMD_HST_NTFY_INTREN	0x01
+#define SMBSLVCMD_HST_NTFY_INTREN	BIT(0)
 
 #define STATUS_ERROR_FLAGS	(SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
 				 SMBHSTSTS_DEV_ERR)
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 05/10] i2c: i801: use the BIT() macro for FEATURES_* also
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

no functional changes

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

new in v2
---
 drivers/i2c/busses/i2c-i801.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 05ce265..55fa6e1 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -270,15 +270,15 @@ struct i801_priv {
 	struct smbus_host_notify *host_notify;
 };
 
-#define FEATURE_SMBUS_PEC	(1 << 0)
-#define FEATURE_BLOCK_BUFFER	(1 << 1)
-#define FEATURE_BLOCK_PROC	(1 << 2)
-#define FEATURE_I2C_BLOCK_READ	(1 << 3)
-#define FEATURE_IRQ		(1 << 4)
-#define FEATURE_HOST_NOTIFY	(1 << 5)
+#define FEATURE_SMBUS_PEC	BIT(0)
+#define FEATURE_BLOCK_BUFFER	BIT(1)
+#define FEATURE_BLOCK_PROC	BIT(2)
+#define FEATURE_I2C_BLOCK_READ	BIT(3)
+#define FEATURE_IRQ		BIT(4)
+#define FEATURE_HOST_NOTIFY	BIT(5)
 /* Not really a feature, but it's convenient to handle it as such */
-#define FEATURE_IDF		(1 << 15)
-#define FEATURE_TCO		(1 << 16)
+#define FEATURE_IDF		BIT(15)
+#define FEATURE_TCO		BIT(16)
 
 static const char *i801_feature_names[] = {
 	"SMBus PEC",
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 06/10] i2c: i801: do not report an error if FEATURE_HOST_NOTIFY is not set
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

we can skip one test when calling i801_enable_host_notify(). Given
that we call it all the time, it's better to consider the fact that
the adapter doesn't support Host Notify as not an error.

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

no changes in v2
---
 drivers/i2c/busses/i2c-i801.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 55fa6e1..422cce2 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -944,12 +944,13 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
 	struct i801_priv *priv = i2c_get_adapdata(adapter);
 
 	if (!(priv->features & FEATURE_HOST_NOTIFY))
-		return -ENOTSUPP;
+		return 0; /* not an error actually */
 
-	if (!priv->host_notify)
+	if (!priv->host_notify) {
 		priv->host_notify = i2c_setup_smbus_host_notify(adapter);
-	if (!priv->host_notify)
-		return -ENOMEM;
+		if (!priv->host_notify)
+			return -ENOMEM;
+	}
 
 	priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
 
@@ -1638,7 +1639,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	 * is not used if i2c_add_adapter() fails.
 	 */
 	err = i801_enable_host_notify(&priv->adapter);
-	if (err && err != -ENOTSUPP)
+	if (err)
 		dev_warn(&dev->dev, "Unable to enable SMBus Host Notify\n");
 
 	i801_probe_optional_slaves(priv);
@@ -1693,7 +1694,7 @@ static int i801_resume(struct device *dev)
 	int err;
 
 	err = i801_enable_host_notify(&priv->adapter);
-	if (err && err != -ENOTSUPP)
+	if (err)
 		dev_warn(dev, "Unable to enable SMBus Host Notify\n");
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 10/10] i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0
From: Benjamin Tissoires @ 2016-09-28  8:15 UTC (permalink / raw)
  To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
In-Reply-To: <1475050549-25542-1-git-send-email-benjamin.tissoires@redhat.com>

On the platform tested, reading SMBNTFDDAT always returns 0 (using 1 read
of a word or 2 of 2 bytes). Given that we are not sure why and that we
don't need to rely on the data parameter in the current users of Host
Notify, remove this part of the code.

If someone wants to re-enable it, just revert this commit and data should
be available.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v3

new in v2
---
 drivers/i2c/busses/i2c-i801.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 38e2421..0e3aa44 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -117,7 +117,6 @@
 #define SMBSLVSTS(p)	(16 + (p)->smba)	/* ICH3 and later */
 #define SMBSLVCMD(p)	(17 + (p)->smba)	/* ICH3 and later */
 #define SMBNTFDADD(p)	(20 + (p)->smba)	/* ICH3 and later */
-#define SMBNTFDDAT(p)	(22 + (p)->smba)	/* ICH3 and later */
 
 /* PCI Address Constants */
 #define SMBBAR		4
@@ -578,20 +577,22 @@ static void i801_isr_byte_done(struct i801_priv *priv)
 static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
 {
 	unsigned short addr;
-	unsigned int data;
 
 	if (unlikely(!priv->host_notify))
 		goto out;
 
 	addr = inb_p(SMBNTFDADD(priv)) >> 1;
-	data = inw_p(SMBNTFDDAT(priv));
 
 	/*
+	 * With the tested platforms, reading SMBNTFDDAT (22 + (p)->smba)
+	 * always returns 0 and is safe to read.
+	 * We just use 0 given we have no use of the data right now.
+	 *
 	 * We don't notify about missed host notify events when the adapter
 	 * lags behind. The operation is still scheduled and will get
 	 * eventually processed.
 	 */
-	i2c_handle_smbus_host_notify(priv->host_notify, addr, data);
+	i2c_handle_smbus_host_notify(priv->host_notify, addr, 0);
 
 out:
 	/* clear Host Notify bit and return */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 1/2] Documentation: tpm: add the IBM Virtual TPM device tree binding documentation
From: Nayna Jain @ 2016-09-28  8:30 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, wsa-z923LK4zBo2bacvFa/9K2g,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	peterhuewe-Mmb7MZpHnFY, tpmdd-yWjUBOtONefk1uMJSBkQmQ,
	jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA,
	hellerda-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	ltcgcw-r/Jw6+rmf7HQT0dZR+AlfA,
	cclaudio-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	honclo-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Nayna Jain

Virtual TPM, which is being used on IBM POWER7+ and POWER8 systems running
POWERVM, is currently supported by tpm device driver but lacks the
documentation. This patch adds the missing documentation for the existing
support.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
Changelog v2:

- New Patch

 .../devicetree/bindings/security/tpm/ibmvtpm.txt   | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/ibmvtpm.txt

diff --git a/Documentation/devicetree/bindings/security/tpm/ibmvtpm.txt b/Documentation/devicetree/bindings/security/tpm/ibmvtpm.txt
new file mode 100644
index 0000000..d89f999
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/ibmvtpm.txt
@@ -0,0 +1,41 @@
+* Device Tree Bindings for IBM Virtual Trusted Platform Module(vtpm)
+
+Required properties:
+
+- compatible            : property name that conveys the platform architecture
+                          identifiers, as 'IBM,vtpm'
+- device_type           : specifies type of virtual device
+- interrupts            : property specifying the interrupt source number and
+                          sense code associated with this virtual I/O Adapters
+- ibm,my-drc-index      : integer index for the connector between the device
+                          and its parent - present only if Dynamic
+                          Reconfiguration(DR) Connector is enabled
+- ibm,#dma-address-cells: specifies the number of cells that are used to
+                          encode the physical address field of dma-window
+                          properties
+- ibm,#dma-size-cells   : specifies the number of cells that are used to
+                          encode the size field of dma-window properties
+- ibm,my-dma-window     : specifies DMA window associated with this virtual
+                          IOA
+- ibm,loc-code          : specifies the unique and persistent location code
+                          associated with this virtual I/O Adapters
+- linux,sml-base        : 64-bit base address of the reserved memory allocated
+                          for the firmware event log
+- linux,sml-size        : size of the memory allocated for the firmware event log
+
+Example (IBM Virtual Trusted Platform Module)
+---------------------------------------------
+
+                vtpm@30000003 {
+                        ibm,#dma-size-cells = <0x2>;
+                        compatible = "IBM,vtpm";
+                        device_type = "IBM,vtpm";
+                        ibm,my-drc-index = <0x30000003>;
+                        ibm,#dma-address-cells = <0x2>;
+                        linux,sml-base = <0xc60e 0x0>;
+                        interrupts = <0xa0003 0x0>;
+                        ibm,my-dma-window = <0x10000003 0x0 0x0 0x0 0x10000000>;
+                        ibm,loc-code = "U8286.41A.10082DV-V3-C3";
+                        reg = <0x30000003>;
+                        linux,sml-size = <0xbce10200>;
+                };
-- 
2.5.0

--
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

^ permalink raw reply related

* [PATCH v2 2/2] Documentation: tpm: add the Physical TPM device tree binding documentation
From: Nayna Jain @ 2016-09-28  8:30 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, wsa-z923LK4zBo2bacvFa/9K2g,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	peterhuewe-Mmb7MZpHnFY, tpmdd-yWjUBOtONefk1uMJSBkQmQ,
	jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA,
	hellerda-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	ltcgcw-r/Jw6+rmf7HQT0dZR+AlfA,
	cclaudio-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	honclo-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Nayna Jain
In-Reply-To: <1475051441-23008-1-git-send-email-nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Newly added support of TPM 2.0 eventlog securityfs pseudo files in tpm
device driver consumes device tree bindings representing I2C based
Physical TPM. This patch adds the documentation for corresponding device
tree bindings of I2C based Physical TPM. These bindings are similar to
vtpm device tree bindings being used on IBM Power7+ and Power8 Systems
running PowerVM.

Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
Changelog v2:

- Include review feedbacks.
  - Move the doc within bindings/security/tpm.
  - Add example for compatible property in description.
  - Delete implicit properties like status, label from description.
  - Redefine linux,sml-base description.

 .../devicetree/bindings/security/tpm/tpm-i2c.txt     | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt

diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
new file mode 100644
index 0000000..16df8bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
@@ -0,0 +1,20 @@
+* Device Tree Bindings for I2C based Trusted Platform Module(TPM)
+
+Required properties:
+
+- compatible     : 'manufacturer,model', eg. nuvoton,npct650
+- linux,sml-base : 64-bit base address of the reserved memory allocated for
+                   the firmware event log
+- linux,sml-size : size of the memory allocated for the firmware event log
+
+Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
+----------------------------------------------------------
+
+tpm@57 {
+	reg = <0x57>;
+	label = "tpm";
+	compatible = "nuvoton,npct650", "nuvoton,npct601";
+	linux,sml-base = <0x7f 0xfd450000>;
+	linux,sml-size = <0x10000>;
+	status = "okay";
+};
-- 
2.5.0

--
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

^ permalink raw reply related

* Re: [tpmdd-devel] [PATCH] Documentation: tpm: Adds the TPM device tree node documentation
From: Nayna @ 2016-09-28  8:40 UTC (permalink / raw)
  To: Rob Herring, Jason Gunthorpe
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pawel Moll, Ian Campbell,
	wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	gcwilson-r/Jw6+rmf7HQT0dZR+AlfA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kumar Gala
In-Reply-To: <CAL_Jsq+jmAHSGTJPxWZgc_87NTA8uvWZSegx9E7krXKzDcgBmw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 09/02/2016 11:27 PM, Rob Herring wrote:
> On Fri, Sep 2, 2016 at 12:06 PM, Jason Gunthorpe
> <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>> On Fri, Sep 02, 2016 at 09:51:21AM -0500, Rob Herring wrote:
>>>> +- linux,sml-base : base address of the Event Log. It is a physical address.
>>>> +              sml stands for shared memory log.
>>>
>>> How is it a physical address on an i2c device? Why 2 cells (which needs
>>> to be documented also)?
>>
>> To be clear, as I understand it, this mechanism is a hand off from the
>> boot firmware to Linux.
>>
>> The boot firmware talks i2c to the device, does some stuff, writes it
>> to memory and then linux reads that stuff. I agree it seems crazy to
>> include a random physical address like that.
>
> I'd put that in reserved-memory then if designing this from scratch...

Thanks for the review comments.

Yes, it is in reserved-memory. I modified the explanation for 
linux,sml-base property to be more descriptive now in my v2 version of 
the patch, posted just now.
>
> Must not be completely random as somehow the kernel doesn't use that memory.
>
>> The linux,sml-* names appear to have been used by IBM for a long time
>> on their enterprise PPC platforms (see drivers/char/tpm/tpm_of.c), so
>> I've expected we have to keep them?
>
> Yes. I wasn't aware of that.
>
>> I asked Nayna to document this stuff IBM is doing so the rest of us
>> in TPM land can have a hope of maintaining it...
>>
>> Jason
>> --
>> 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
>

--
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

^ permalink raw reply

* Re: [tpmdd-devel] [PATCH] Documentation: tpm: Adds the TPM device tree node documentation
From: Nayna @ 2016-09-28  8:44 UTC (permalink / raw)
  To: Jason Gunthorpe, Rob Herring
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	pawel.moll-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	wsa-z923LK4zBo2bacvFa/9K2g,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	gcwilson-r/Jw6+rmf7HQT0dZR+AlfA, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	galak-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <20160902170613.GA5024-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>



On 09/02/2016 10:36 PM, Jason Gunthorpe wrote:
> On Fri, Sep 02, 2016 at 09:51:21AM -0500, Rob Herring wrote:
>>> +- linux,sml-base : base address of the Event Log. It is a physical address.
>>> +		   sml stands for shared memory log.
>>
>> How is it a physical address on an i2c device? Why 2 cells (which needs
>> to be documented also)?
>
> To be clear, as I understand it, this mechanism is a hand off from the
> boot firmware to Linux.
>
> The boot firmware talks i2c to the device, does some stuff, writes it
> to memory and then linux reads that stuff. I agree it seems crazy to
> include a random physical address like that.
>
> The linux,sml-* names appear to have been used by IBM for a long time
> on their enterprise PPC platforms (see drivers/char/tpm/tpm_of.c), so
> I've expected we have to keep them?
>
> I asked Nayna to document this stuff IBM is doing so the rest of us
> in TPM land can have a hope of maintaining it...

Thanks Jason !!

In my v2 version for device tree documentation, I have posted the 
documentation for both vtpm and physical TPM.

>
> Jason
>

--
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

^ permalink raw reply

* RE: [patch v2] i2c: add master driver for mellanox systems
From: Vadim Pasternak @ 2016-09-28 11:50 UTC (permalink / raw)
  To: Vadim Pasternak, wsa@the-dreams.de
  Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	jiri@resnulli.us, Michael Shych
In-Reply-To: <1474297231-36264-1-git-send-email-vadimp@mellanox.com>

Hi,

I'd like to kindly clarify whether the patch I sent could be applied, or I need to modify something?

Best regards,
Vadim.

> -----Original Message-----
> From: vadimp@mellanox.com [mailto:vadimp@mellanox.com]
> Sent: Monday, September 19, 2016 6:01 PM
> To: wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Vadim Pasternak <vadimp@mellanox.com>; Michael Shych
> <michaelsh@mellanox.com>
> Subject: [patch v2] i2c: add master driver for mellanox systems
> 
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> Device driver for Mellanox I2C controller logic, implemented in Lattice CPLD
> device.
> Device supports:
>  - Master mode
>  - One physical bus
>  - Polling mode
> 
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/busses/Kconfig:config I2C_MLXCPLD
> 
> Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> v1->v2
>  Fixes added by Vadim:
>  - Put new record in Makefile in alphabetic order;
>  - Remove http://www.mellanox.com from MAINTAINERS record;
> ---
>  Documentation/i2c/busses/i2c-mlxcpld |  47 +++
>  MAINTAINERS                          |   8 +
>  drivers/i2c/busses/Kconfig           |  12 +
>  drivers/i2c/busses/Makefile          |   1 +
>  drivers/i2c/busses/i2c-mlxcpld.c     | 597
> +++++++++++++++++++++++++++++++++++
>  5 files changed, 665 insertions(+)
>  create mode 100644 Documentation/i2c/busses/i2c-mlxcpld
>  create mode 100644 drivers/i2c/busses/i2c-mlxcpld.c
> 
> diff --git a/Documentation/i2c/busses/i2c-mlxcpld
> b/Documentation/i2c/busses/i2c-mlxcpld
> new file mode 100644
> index 0000000..0f8678a
> --- /dev/null
> +++ b/Documentation/i2c/busses/i2c-mlxcpld
> @@ -0,0 +1,47 @@
> +Driver i2c-mlxcpld
> +
> +Author: Michael Shych <michaelsh@mellanox.com>
> +
> +This is a for Mellanox I2C controller logic, implemented in Lattice
> +CPLD device.
> +Device supports:
> + - Master mode.
> + - One physical bus.
> + - Polling mode.
> +
> +This controller is equipped within the next Mellanox systems:
> +"msx6710", "msx6720", "msb7700", "msn2700", "msx1410", "msn2410",
> +"msb7800", "msn2740", "msn2100".
> +
> +The next transaction types are supported:
> + - Receive Byte/Block.
> + - Send Byte/Block.
> + - Read Byte/Block.
> + - Write Byte/Block.
> +
> +Registers:
> +CTRL		0x1 - control reg.
> +			Resets all the registers.
> +HALF_CYC	0x4 - cycle reg.
> +			Configure the width of I2C SCL half clock cycle (in 4
> LPC_CLK
> +			units).
> +I2C_HOLD	0x5 - hold reg.
> +			OE (output enable) is delayed by value set to this
> register
> +			(in LPC_CLK units)
> +CMD			0x6 - command reg.
> +			Bit 7(lsb), 0 = write, 1 = read.
> +			Bits [6:0] - the 7bit Address of the I2C device.
> +			It should be written last as it triggers an I2C transaction.
> +NUM_DATA	0x7 - data size reg.
> +			Number of address bytes to write in read transaction
> +NUM_ADDR	0x8 - address reg.
> +			Number of address bytes to write in read transaction.
> +STATUS		0x9 - status reg.
> +			Bit 0 - transaction is completed.
> +			Bit 4 - ACK/NACK.
> +DATAx		0xa - 0x54  - 68 bytes data buffer regs.
> +			For write transaction address is specified in four first
> bytes
> +			(DATA1 - DATA4), data starting from DATA4.
> +			For read transactions address is send in separate
> transaction and
> +			specified in four first bytes (DATA0 - DATA3). Data is
> reading
> +			starting from DATA0.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6781a3f..dc31231 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7667,6 +7667,14 @@ W:	http://www.mellanox.com
>  Q:	http://patchwork.ozlabs.org/project/netdev/list/
>  F:	drivers/net/ethernet/mellanox/mlxsw/
> 
> +MELLANOX MLXCPLD I2C DRIVER
> +M:	Vadim Pasternak <vadimp@mellanox.com>
> +M:	Michael Shych <michaelsh@mellanox.com>
> +L:	linux-i2c@vger.kernel.org
> +S:	Supported
> +F:	drivers/i2c/busses/i2c-mlxcpld.c
> +F:	Documentation/i2c/busses/i2c-mlxcpld
> +
>  SOFT-ROCE DRIVER (rxe)
>  M:	Moni Shoua <monis@mellanox.com>
>  L:	linux-rdma@vger.kernel.org
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index
> 5c3993b..1126142a 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1203,4 +1203,16 @@ config I2C_OPAL
>  	  This driver can also be built as a module. If so, the module will be
>  	  called as i2c-opal.
> 
> +config I2C_MLXCPLD
> +        tristate "Mellanox I2C driver"
> +        depends on X86_64
> +        default y
> +        help
> +	  This exposes the Mellanox platform I2C busses to the linux I2C layer
> +	  for X86 based systems.
> +	  Controller is implemented as CPLD logic.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called as i2c-mlxcpld.
> +
>  endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index
> 37f2819..4df3578 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -118,5 +118,6 @@ obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
>  obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
>  obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
>  obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
> +obj-$(CONFIG_I2C_MLXCPLD)	+= i2c-mlxcpld.o
> 
>  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git
> a/drivers/i2c/busses/i2c-mlxcpld.c b/drivers/i2c/busses/i2c-mlxcpld.c
> new file mode 100644
> index 0000000..dd62190
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-mlxcpld.c
> @@ -0,0 +1,597 @@
> +/*
> + * drivers/i2c/busses/i2c-mlxcpld.c
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + *    contributors may be used to endorse or promote products derived from
> + *    this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of
> +the
> + * GNU General Public License ("GPL") version 2 as published by the
> +Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED
> +TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR
> +PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> +CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> +BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> WHETHER
> +IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> +OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> ADVISED
> +OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +/* General defines */
> +#define MLXPLAT_CPLD_LPC_I2C_BASE_ADRR	0x2000
> +#define MLXCPLD_I2C_DEVICE_NAME		"i2c_mlxcpld"
> +#define MLXCPLD_I2C_VALID_FLAG		(I2C_M_RECV_LEN |
> I2C_M_RD)
> +#define MLXCPLD_I2C_BUS_NUM		1
> +#define MLXCPLD_I2C_DATA_REG_SZ		36
> +#define MLXCPLD_I2C_MAX_ADDR_LEN	4
> +#define MLXCPLD_I2C_RETR_NUM		2
> +#define MLXCPLD_I2C_XFER_TO		500000 /* msec */
> +#define MLXCPLD_I2C_POLL_TIME		2000   /* msec */
> +
> +/* LPC I2C registers */
> +#define MLXCPLD_LPCI2C_LPF_REG		0x0
> +#define MLXCPLD_LPCI2C_CTRL_REG		0x1
> +#define MLXCPLD_LPCI2C_HALF_CYC_REG	0x4
> +#define MLXCPLD_LPCI2C_I2C_HOLD_REG	0x5
> +#define MLXCPLD_LPCI2C_CMD_REG		0x6
> +#define MLXCPLD_LPCI2C_NUM_DAT_REG	0x7
> +#define MLXCPLD_LPCI2C_NUM_ADDR_REG	0x8
> +#define MLXCPLD_LPCI2C_STATUS_REG	0x9
> +#define MLXCPLD_LPCI2C_DATA_REG		0xa
> +
> +/* LPC I2C masks and parametres */
> +#define MLXCPLD_LPCI2C_RST_SEL_MASK	0x1
> +#define MLXCPLD_LPCI2C_LPF_DFLT		0x2
> +#define MLXCPLD_LPCI2C_HALF_CYC_100	0x1f
> +#define MLXCPLD_LPCI2C_I2C_HOLD_100	0x3c
> +#define MLXCPLD_LPCI2C_TRANS_END	0x1
> +#define MLXCPLD_LPCI2C_STATUS_NACK	0x10
> +#define MLXCPLD_LPCI2C_ERR_IND		-1
> +#define MLXCPLD_LPCI2C_NO_IND		0
> +#define MLXCPLD_LPCI2C_ACK_IND		1
> +#define MLXCPLD_LPCI2C_NACK_IND		2
> +
> +/**
> + * mlxcpld_i2c_regs - controller registers:
> + * @half_cyc - half cycle register
> + * @i2c_hold - hold register
> + * @config - config register
> + * @cmd - command register
> + * @cmd - status register
> + * @data - register data
> +**/
> +struct mlxcpld_i2c_regs {
> +	u8 half_cyc;
> +	u8 i2c_hold;
> +	u8 config;
> +	u8 cmd;
> +	u8 status;
> +	u8 data[MLXCPLD_I2C_DATA_REG_SZ];
> +};
> +
> +/**
> + * mlxcpld_i2c_curr_transf - current transaction parameters:
> + * @cmd - command
> + * @addr_width - address width
> + * @data_len - data length
> + * @cmd - command register
> + * @msg_num - message number
> + * @msg - pointer to message buffer
> +**/
> +struct mlxcpld_i2c_curr_transf {
> +	u8 cmd;
> +	u8 addr_width;
> +	u8 data_len;
> +	u8 msg_num;
> +	struct i2c_msg *msg;
> +};
> +
> +/**
> + * mlxcpld_i2c_priv - private controller data:
> + * @lpc_gen_dec_reg - register space
> + * @adap - i2c adapter
> + * @dev_id - device id
> + * @base_addr - base IO address
> + * @poll_time - polling time
> + * @xfer_to - transfer timeout in microsec (500000)
> + * @retr_num - access retries number (2)
> + * @block_sz - maximum data block size (36),
> + * @lock - bus access lock
> + * @lpc_i2c_res - lpc i2c resourse
> + * @lpc_cpld_res - lpc cpld resource
> + * @xfer - current i2c transfer block
> + * @pdev - platform device
> +**/
> +struct mlxcpld_i2c_priv {
> +	struct i2c_adapter adap;
> +	u16 dev_id;
> +	u16 base_addr;
> +	u16 poll_time;
> +	int xfer_to;
> +	int retr_num;
> +	int block_sz;
> +	struct mutex lock;
> +	struct mlxcpld_i2c_curr_transf xfer;
> +	struct platform_device *pdev;
> +};
> +struct platform_device *mlxcpld_i2c_plat_dev;
> +
> +static void mlxcpld_i2c_lpc_write_buf(u8 *data, u8 len, u32 addr) {
> +	int i, nbyte, ndword;
> +
> +	nbyte = len % 4;
> +	ndword = len / 4;
> +	for (i = 0; i < ndword; i++)
> +		outl(*((u32 *)data + i), addr + i * 4);
> +	ndword *= 4;
> +	addr += ndword;
> +	data += ndword;
> +	for (i = 0; i < nbyte; i++)
> +		outb(*(data + i), addr + i);
> +}
> +
> +static void mlxcpld_i2c_lpc_read_buf(u8 *data, u8 len, u32 addr) {
> +	int i, nbyte, ndword;
> +
> +	nbyte = len % 4;
> +	ndword = len / 4;
> +	for (i = 0; i < ndword; i++)
> +		*((u32 *)data + i) = inl(addr + i * 4);
> +	ndword *= 4;
> +	addr += ndword;
> +	data += ndword;
> +	for (i = 0; i < nbyte; i++)
> +		*(data + i) = inb(addr + i);
> +}
> +
> +static void mlxcpld_i2c_read_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
> +				  u8 *data, u8 datalen)
> +{
> +	u32 addr = priv->base_addr + offs;
> +
> +	switch (datalen) {
> +	case 1:
> +		*(data) = inb(addr);
> +		break;
> +	case 2:
> +		*((u16 *)data) = inw(addr);
> +		break;
> +	case 3:
> +		*((u16 *)data) = inw(addr);
> +		*(data + 2) = inb(addr + 2);
> +		break;
> +	case 4:
> +		*((u32 *)data) = inl(addr);
> +		break;
> +	default:
> +		mlxcpld_i2c_lpc_read_buf(data, datalen, addr);
> +		break;
> +	}
> +}
> +
> +static void mlxcpld_i2c_write_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
> +				   u8 *data, u8 datalen)
> +{
> +	u32 addr = priv->base_addr + offs;
> +
> +	switch (datalen) {
> +	case 1:
> +		outb(*(data), addr);
> +		break;
> +	case 2:
> +		outw(*((u16 *)data), addr);
> +		break;
> +	case 3:
> +		outw(*((u16 *)data), addr);
> +		outb(*(data + 2), addr + 2);
> +		break;
> +	case 4:
> +		outl(*((u32 *)data), addr);
> +		break;
> +	default:
> +		mlxcpld_i2c_lpc_write_buf(data, datalen, addr);
> +		break;
> +	}
> +}
> +
> +/* Check validity of current i2c message and all transfer.
> + * Calculate also coomon length of all i2c messages in transfer.
> + */
> +static int mlxcpld_i2c_invalid_len(struct mlxcpld_i2c_priv *priv,
> +				   const struct i2c_msg *msg, u8 *comm_len) {
> +	u8 max_len = msg->flags == I2C_M_RD ? priv->block_sz -
> +		     MLXCPLD_I2C_MAX_ADDR_LEN : priv->block_sz;
> +
> +	if (msg->len < 0 || msg->len > max_len)
> +		return -EINVAL;
> +
> +	*comm_len += msg->len;
> +	if (*comm_len > priv->block_sz)
> +		return -EINVAL;
> +	else
> +		return 0;
> +}
> +
> +/* Check validity of received i2c messages parameters.
> + *  Returns 0 if OK, other - in case of invalid parameters
> + *  or common length of data that should be passed to CPLD  */ static
> +int mlxcpld_i2c_check_msg_params(struct mlxcpld_i2c_priv *priv,
> +					struct i2c_msg *msgs, int num,
> +					u8 *comm_len)
> +{
> +	int i;
> +
> +	if (!num) {
> +		dev_err(&priv->pdev->dev, "Incorrect 0 num of messages\n");
> +		return -EINVAL;
> +	}
> +
> +	if (unlikely(msgs[0].addr > 0x7f)) {
> +		dev_err(&priv->pdev->dev, "Invalid address 0x%03x\n",
> +			msgs[0].addr);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < num; ++i) {
> +		if (unlikely(!msgs[i].buf)) {
> +			dev_err(&priv->pdev->dev, "Invalid buf in msg[%d]\n",
> +				i);
> +			return -EINVAL;
> +		}
> +		if (unlikely(msgs[0].addr != msgs[i].addr)) {
> +			dev_err(&priv->pdev->dev, "Invalid addr in msg[%d]\n",
> +				i);
> +			return -EINVAL;
> +		}
> +		if (unlikely(mlxcpld_i2c_invalid_len(priv, &msgs[i],
> +						     comm_len))) {
> +			dev_err(&priv->pdev->dev, "Invalid len %d msg[%d],
> addr 0x%x, lag %u\n",
> +				msgs[i].len, i, msgs[i].addr, msgs[i].flags);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/* Check if transfer is completed and status of operation.
> + * Returns 0 - transfer completed (both ACK or NACK),
> + * negative - transfer isn't finished.
> + */
> +static int mlxcpld_i2c_check_status(struct mlxcpld_i2c_priv *priv, int
> +*status) {
> +	u8 val;
> +
> +	mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
> +
> +	if (val & MLXCPLD_LPCI2C_TRANS_END) {
> +		if (val & MLXCPLD_LPCI2C_STATUS_NACK)
> +			/* The slave is unable to accept the data. No such
> +			 * slave, command not understood, or unable to accept
> +			 * any more data.
> +			 */
> +			*status = MLXCPLD_LPCI2C_NACK_IND;
> +		else
> +			*status = MLXCPLD_LPCI2C_ACK_IND;
> +		return 0;
> +	}
> +	*status = MLXCPLD_LPCI2C_NO_IND;
> +
> +	return -EIO;
> +}
> +
> +static void mlxcpld_i2c_set_transf_data(struct mlxcpld_i2c_priv *priv,
> +					struct i2c_msg *msgs, int num,
> +					u8 comm_len)
> +{
> +	priv->xfer.msg = msgs;
> +	priv->xfer.msg_num = num;
> +
> +	/*
> +	 * All upper layers currently are never use transfer with more than
> +	 * 2 messages. Actually, it's also not so relevant in Mellanox systems
> +	 * because of HW limitation. Max size of transfer is o more than 20B
> +	 * in current x86 LPCI2C bridge.
> +	 */
> +	priv->xfer.cmd = (msgs[num - 1].flags & I2C_M_RD);
> +
> +	if (priv->xfer.cmd == I2C_M_RD) {
> +		if (comm_len == msgs[0].len) {
> +			/* Special case of addr_width = 0 */
> +			priv->xfer.addr_width = 0;
> +			priv->xfer.data_len = comm_len;
> +		} else {
> +			priv->xfer.addr_width = msgs[0].len;
> +			priv->xfer.data_len = comm_len - priv-
> >xfer.addr_width;
> +		}
> +	} else {
> +		/* Width (I2C_NUM_ADDR reg) isn't used in write command. */
> +		priv->xfer.addr_width = 0;
> +		priv->xfer.data_len = comm_len;
> +	}
> +}
> +
> +/* Reset CPLD LPCI2C block */
> +static void mlxcpld_i2c_reset(struct mlxcpld_i2c_priv *priv) {
> +	u8 val;
> +
> +	mutex_lock(&priv->lock);
> +	mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
> +	val &= ~MLXCPLD_LPCI2C_RST_SEL_MASK;
> +	mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
> +	mutex_unlock(&priv->lock);
> +}
> +
> +/* Make sure the CPLD is ready to start transmitting.
> + * Return 0 if it is, -EBUSY if it is not.
> + */
> +static int mlxcpld_i2c_check_busy(struct mlxcpld_i2c_priv *priv) {
> +	u8 val;
> +
> +	mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
> +
> +	if (val & MLXCPLD_LPCI2C_TRANS_END)
> +		return 0;
> +
> +	return -EIO;
> +}
> +
> +static int mlxcpld_i2c_wait_for_free(struct mlxcpld_i2c_priv *priv) {
> +	int timeout = 0;
> +
> +	do {
> +		if (!mlxcpld_i2c_check_busy(priv))
> +			break;
> +		usleep_range(priv->poll_time/2, priv->poll_time);
> +		timeout += priv->poll_time;
> +	} while (timeout < priv->xfer_to);
> +
> +	if (timeout > priv->xfer_to)
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +/*
> + * Wait for master transfer to complete.
> + * It puts current process to sleep until we get interrupt or timeout expires.
> + * Returns the number of transferred or read bytes or error (<0).
> + */
> +static int mlxcpld_i2c_wait_for_tc(struct mlxcpld_i2c_priv *priv) {
> +	int status, i = 1, timeout = 0;
> +	u8 datalen;
> +	int err = 0;
> +
> +	do {
> +		usleep_range(priv->poll_time / 2, priv->poll_time);
> +		if (!mlxcpld_i2c_check_status(priv, &status))
> +			break;
> +		timeout += priv->poll_time;
> +	} while (status == 0 && timeout < priv->xfer_to);
> +
> +	switch (status) {
> +	case MLXCPLD_LPCI2C_NO_IND:
> +		return -ETIMEDOUT;
> +	case MLXCPLD_LPCI2C_ACK_IND:
> +		if (priv->xfer.cmd == I2C_M_RD) {
> +			/*
> +			 * Actual read data len will be always the same as
> +			 * requested len. 0xff (line pull-up) will be returned
> +			 * if slave has no data to return. Thus don't read
> +			 * MLXCPLD_LPCI2C_NUM_DAT_REG reg from CPLD.
> +			 */
> +			err = datalen = priv->xfer.data_len;
> +			if (priv->xfer.msg_num == 1)
> +				i = 0;
> +
> +			if (!priv->xfer.msg[i].buf)
> +				err = -EINVAL;
> +			else
> +				mlxcpld_i2c_read_comm(priv,
> +
> MLXCPLD_LPCI2C_DATA_REG,
> +						      priv->xfer.msg[i].buf,
> +						      datalen);
> +		} else {
> +			err = priv->xfer.addr_width + priv->xfer.data_len;
> +		}
> +		break;
> +	case MLXCPLD_LPCI2C_NACK_IND:
> +		err = -EAGAIN;
> +		break;
> +	case MLXCPLD_LPCI2C_ERR_IND:
> +		err = -EIO;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return err;
> +}
> +
> +static void mlxcpld_i2c_xfer_msg(struct mlxcpld_i2c_priv *priv) {
> +	int i, len = 0;
> +	u8 cmd;
> +
> +	mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_DAT_REG,
> +			       &priv->xfer.data_len, 1);
> +	mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_ADDR_REG,
> +			       &priv->xfer.addr_width, 1);
> +
> +	for (i = 0; i < priv->xfer.msg_num; i++) {
> +		if ((priv->xfer.msg[i].flags & I2C_M_RD) != I2C_M_RD) {
> +			/* Don't write to CPLD buffer in read transaction */
> +			mlxcpld_i2c_write_comm(priv,
> MLXCPLD_LPCI2C_DATA_REG +
> +					       len, priv->xfer.msg[i].buf,
> +					       priv->xfer.msg[i].len);
> +			len += priv->xfer.msg[i].len;
> +		}
> +	}
> +
> +	/* Set target slave address with command for master transfer.
> +	 * It should be latest executed function before CPLD transaction.
> +	 */
> +	cmd = (priv->xfer.msg[0].addr << 1) | priv->xfer.cmd;
> +	mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CMD_REG, &cmd, 1);
> }
> +
> +/* Generic lpc-i2c transfer.
> + * Returns the number of processed messages or error (<0).
> + */
> +static int mlxcpld_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +			    int num)
> +{
> +	struct mlxcpld_i2c_priv *priv = i2c_get_adapdata(adap);
> +	u8 comm_len = 0;
> +	int err;
> +
> +	err = mlxcpld_i2c_check_msg_params(priv, msgs, num, &comm_len);
> +	if (err) {
> +		dev_err(&priv->pdev->dev, "Incorrect message\n");
> +		return err;
> +	}
> +
> +	/* Check bus state */
> +	if (mlxcpld_i2c_wait_for_free(priv)) {
> +		dev_err(&priv->pdev->dev, "LPCI2C bridge is busy\n");
> +
> +		/*
> +		 * Usually it means something serious has happened.
> +		 * We can not have unfinished previous transfer
> +		 * so it doesn't make any sense to try to stop it.
> +		 * Probably we were not able to recover from the
> +		 * previous error.
> +		 * The only reasonable thing - is soft reset.
> +		 */
> +		mlxcpld_i2c_reset(priv);
> +		if (mlxcpld_i2c_check_busy(priv)) {
> +			dev_err(&priv->pdev->dev, "LPCI2C bridge is busy after
> reset\n");
> +			return -EIO;
> +		}
> +	}
> +
> +	mlxcpld_i2c_set_transf_data(priv, msgs, num, comm_len);
> +
> +	mutex_lock(&priv->lock);
> +	/* Do real transfer. Can't fail */
> +	mlxcpld_i2c_xfer_msg(priv);
> +	/* Wait for transaction complete */
> +	err = mlxcpld_i2c_wait_for_tc(priv);
> +	mutex_unlock(&priv->lock);
> +
> +	return err < 0 ? err : num;
> +}
> +
> +static u32 mlxcpld_i2c_func(struct i2c_adapter *adap) {
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
> +
> +static const struct i2c_algorithm mlxcpld_i2c_algo = {
> +	.master_xfer	= mlxcpld_i2c_xfer,
> +	.functionality	= mlxcpld_i2c_func
> +};
> +
> +static struct i2c_adapter mlxcpld_i2c_adapter = {
> +	.owner          = THIS_MODULE,
> +	.name           = "i2c-mlxcpld",
> +	.class          = I2C_CLASS_HWMON | I2C_CLASS_SPD,
> +	.algo           = &mlxcpld_i2c_algo,
> +};
> +
> +static int mlxcpld_i2c_probe(struct platform_device *pdev) {
> +	struct mlxcpld_i2c_priv *priv;
> +	int err;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct mlxcpld_i2c_priv),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	mutex_init(&priv->lock);
> +	platform_set_drvdata(pdev, priv);
> +	priv->pdev = pdev;
> +	priv->xfer_to = MLXCPLD_I2C_XFER_TO;
> +	priv->retr_num = MLXCPLD_I2C_RETR_NUM;
> +	priv->block_sz = MLXCPLD_I2C_DATA_REG_SZ;
> +	priv->poll_time = MLXCPLD_I2C_POLL_TIME;
> +	/* Register with i2c layer */
> +	priv->adap = mlxcpld_i2c_adapter;
> +	priv->adap.dev.parent = &pdev->dev;
> +	i2c_set_adapdata(&priv->adap, priv);
> +	priv->adap.retries = priv->retr_num;
> +	priv->adap.nr = MLXCPLD_I2C_BUS_NUM;
> +	priv->adap.timeout = usecs_to_jiffies(priv->xfer_to);
> +
> +	err = i2c_add_numbered_adapter(&priv->adap);
> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to add %s adapter (%d)\n",
> +			MLXCPLD_I2C_DEVICE_NAME, err);
> +		goto fail_adapter;
> +	}
> +
> +	priv->base_addr = MLXPLAT_CPLD_LPC_I2C_BASE_ADRR;
> +
> +	return 0;
> +
> +fail_adapter:
> +	mutex_destroy(&priv->lock);
> +	return err;
> +}
> +
> +static int mlxcpld_i2c_remove(struct platform_device *pdev) {
> +	struct mlxcpld_i2c_priv *priv = platform_get_drvdata(pdev);
> +
> +	i2c_del_adapter(&priv->adap);
> +	mutex_destroy(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mlxcpld_i2c_driver = {
> +	.probe		= mlxcpld_i2c_probe,
> +	.remove		= mlxcpld_i2c_remove,
> +	.driver = {
> +		.name = MLXCPLD_I2C_DEVICE_NAME,
> +	},
> +};
> +
> +module_platform_driver(mlxcpld_i2c_driver);
> +
> +MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
> +MODULE_DESCRIPTION("Mellanox I2C-CPLD controller driver");
> +MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:i2c-mlxcpld");
> --
> 2.1.4

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox