linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5]
@ 2019-02-08 16:11 Federico Vaga
  2019-02-08 16:11 ` [PATCH v3 1/5] i2c:ocores: stop transfer on timeout Federico Vaga
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Federico Vaga @ 2019-02-08 16:11 UTC (permalink / raw)
  To: Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel

This patch set provides improvements to the i2c-ocore driver.

[V2 -> V3]
- fix error condition on platform_get_irq(). Copied from
  https://patchwork.ozlabs.org/patch/1038409/

[V1 -> V2]
- replaced usleep_range() with udelay() so that the polling version can be
  used in atomic context.
- added dedicated patch for minor style issues
- fixed delay computation
- use spin_lock_irqsave(), instead of spin_trylock_irqsave(). IACK is always
  necessary and a trylock would generate an extra interrupt for nothing
- make the driver ready for an eventual master_xfer_irqless()


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 1/5] i2c:ocores: stop transfer on timeout
  2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
@ 2019-02-08 16:11 ` Federico Vaga
  2019-02-08 16:58   ` Andrew Lunn
  2019-02-08 16:11 ` [PATCH v3 2/5] i2c:ocores: do not handle IRQ if IF is not set Federico Vaga
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Federico Vaga @ 2019-02-08 16:11 UTC (permalink / raw)
  To: Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel, Federico Vaga

Detecting a timeout is ok, but we also need to assert a STOP command on
the bus in order to prevent it from generating interrupts when there are
no on going transfers.

Example: very long transmission.

1. ocores_xfer: START a transfer
2. ocores_isr : handle byte by byte the transfer
3. ocores_xfer: goes in timeout [[bugfix here]]
4. ocores_xfer: return to I2C subsystem and to the I2C driver
5. I2C driver : it may clean up the i2c_msg memory
6. ocores_isr : receives another interrupt (pending bytes to be
                transferred) but the i2c_msg memory is invalid now

So, since the transfer was too long, we have to detect the timeout and
STOP the transfer.

Another point is that we have a critical region here. When handling the
timeout condition we may have a running IRQ handler. For this reason I
introduce a spinlock.

In order to make easier to understan locking I have:
- added a new function to handle timeout
- modified the current ocores_process() function in order to be protected
  by the new spinlock
Like this it is obvious at first sight that this locking serializes
the execution of ocores_process() and ocores_process_timeout()

Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
---
 drivers/i2c/busses/i2c-ocores.c | 54 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 87f9caa..aa85202 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -25,7 +25,12 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/log2.h>
+#include <linux/spinlock.h>
 
+/**
+ * @process_lock: protect I2C transfer process.
+ *     ocores_process() and ocores_process_timeout() can't run in parallel.
+ */
 struct ocores_i2c {
 	void __iomem *base;
 	u32 reg_shift;
@@ -36,6 +41,7 @@ struct ocores_i2c {
 	int pos;
 	int nmsgs;
 	int state; /* see STATE_ */
+	spinlock_t process_lock;
 	struct clk *clk;
 	int ip_clock_khz;
 	int bus_clock_khz;
@@ -141,19 +147,26 @@ static void ocores_process(struct ocores_i2c *i2c)
 {
 	struct i2c_msg *msg = i2c->msg;
 	u8 stat = oc_getreg(i2c, OCI2C_STATUS);
+	unsigned long flags;
+
+	/*
+	 * If we spin here is because we are in timeout, so we are going
+	 * to be in STATE_ERROR. See ocores_process_timeout()
+	 */
+	spin_lock_irqsave(&i2c->process_lock, flags);
 
 	if ((i2c->state == STATE_DONE) || (i2c->state == STATE_ERROR)) {
 		/* stop has been sent */
 		oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
 		wake_up(&i2c->wait);
-		return;
+		goto out;
 	}
 
 	/* error? */
 	if (stat & OCI2C_STAT_ARBLOST) {
 		i2c->state = STATE_ERROR;
 		oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
-		return;
+		goto out;
 	}
 
 	if ((i2c->state == STATE_START) || (i2c->state == STATE_WRITE)) {
@@ -163,7 +176,7 @@ static void ocores_process(struct ocores_i2c *i2c)
 		if (stat & OCI2C_STAT_NACK) {
 			i2c->state = STATE_ERROR;
 			oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
-			return;
+			goto out;
 		}
 	} else
 		msg->buf[i2c->pos++] = oc_getreg(i2c, OCI2C_DATA);
@@ -184,14 +197,14 @@ static void ocores_process(struct ocores_i2c *i2c)
 
 				oc_setreg(i2c, OCI2C_DATA, addr);
 				oc_setreg(i2c, OCI2C_CMD,  OCI2C_CMD_START);
-				return;
+				goto out;
 			} else
 				i2c->state = (msg->flags & I2C_M_RD)
 					? STATE_READ : STATE_WRITE;
 		} else {
 			i2c->state = STATE_DONE;
 			oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
-			return;
+			goto out;
 		}
 	}
 
@@ -202,6 +215,9 @@ static void ocores_process(struct ocores_i2c *i2c)
 		oc_setreg(i2c, OCI2C_DATA, msg->buf[i2c->pos++]);
 		oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_WRITE);
 	}
+
+out:
+	spin_unlock_irqrestore(&i2c->process_lock, flags);
 }
 
 static irqreturn_t ocores_isr(int irq, void *dev_id)
@@ -213,9 +229,24 @@ static irqreturn_t ocores_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+/**
+ * Process timeout event
+ * @i2c: ocores I2C device instance
+ */
+static void ocores_process_timeout(struct ocores_i2c *i2c)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&i2c->process_lock, flags);
+	i2c->state = STATE_ERROR;
+	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
+	spin_unlock_irqrestore(&i2c->process_lock, flags);
+}
+
 static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct ocores_i2c *i2c = i2c_get_adapdata(adap);
+	int ret;
 
 	i2c->msg = msgs;
 	i2c->pos = 0;
@@ -225,11 +256,14 @@ static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	oc_setreg(i2c, OCI2C_DATA, i2c_8bit_addr_from_msg(i2c->msg));
 	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_START);
 
-	if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
-			       (i2c->state == STATE_DONE), HZ))
-		return (i2c->state == STATE_DONE) ? num : -EIO;
-	else
+	ret = wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
+				 (i2c->state == STATE_DONE), HZ);
+	if (ret == 0) {
+		ocores_process_timeout(i2c);
 		return -ETIMEDOUT;
+	}
+
+	return (i2c->state == STATE_DONE) ? num : -EIO;
 }
 
 static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
@@ -422,6 +456,8 @@ static int ocores_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
+	spin_lock_init(&i2c->process_lock);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	i2c->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(i2c->base))
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 2/5] i2c:ocores: do not handle IRQ if IF is not set
  2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
  2019-02-08 16:11 ` [PATCH v3 1/5] i2c:ocores: stop transfer on timeout Federico Vaga
@ 2019-02-08 16:11 ` Federico Vaga
  2019-02-08 17:00   ` Andrew Lunn
  2019-02-08 16:11 ` [PATCH v3 3/5] i2c:ocores: add polling interface Federico Vaga
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Federico Vaga @ 2019-02-08 16:11 UTC (permalink / raw)
  To: Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel, Federico Vaga

If the Interrupt Flag (IF) is not set, we should not handle the IRQ:
- the line can be shared with other devices
- it can be a spurious interrupt

To avoid reading twice the status register, the ocores_process() function
expects it to be read by the caller.

Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
Acked-by: Peter Korsgaard <peter@korsgaard.com>
---
 drivers/i2c/busses/i2c-ocores.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index aa85202..fcc2558 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -143,10 +143,9 @@ static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 	return i2c->getreg(i2c, reg);
 }
 
-static void ocores_process(struct ocores_i2c *i2c)
+static void ocores_process(struct ocores_i2c *i2c, u8 stat)
 {
 	struct i2c_msg *msg = i2c->msg;
-	u8 stat = oc_getreg(i2c, OCI2C_STATUS);
 	unsigned long flags;
 
 	/*
@@ -223,8 +222,12 @@ out:
 static irqreturn_t ocores_isr(int irq, void *dev_id)
 {
 	struct ocores_i2c *i2c = dev_id;
+	u8 stat = oc_getreg(i2c, OCI2C_STATUS);
+
+	if (!(stat & OCI2C_STAT_IF))
+		return IRQ_NONE;
 
-	ocores_process(i2c);
+	ocores_process(i2c, stat);
 
 	return IRQ_HANDLED;
 }
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 3/5] i2c:ocores: add polling interface
  2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
  2019-02-08 16:11 ` [PATCH v3 1/5] i2c:ocores: stop transfer on timeout Federico Vaga
  2019-02-08 16:11 ` [PATCH v3 2/5] i2c:ocores: do not handle IRQ if IF is not set Federico Vaga
@ 2019-02-08 16:11 ` Federico Vaga
  2019-02-09 21:33   ` Andrew Lunn
  2019-02-08 16:12 ` [PATCH v3 4/5] i2c:ocores: add SPDX tag Federico Vaga
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Federico Vaga @ 2019-02-08 16:11 UTC (permalink / raw)
  To: Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel, Federico Vaga

This driver assumes that an interrupt line is always available for
the I2C master. This is not always the case and this patch adds support
for a polling version.

Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
---
 drivers/i2c/busses/i2c-ocores.c | 176 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 156 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index fcc2558..bbe3e96 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -26,6 +27,9 @@
 #include <linux/io.h>
 #include <linux/log2.h>
 #include <linux/spinlock.h>
+#include <linux/jiffies.h>
+
+#define OCORES_FLAG_POLL BIT(0)
 
 /**
  * @process_lock: protect I2C transfer process.
@@ -35,6 +39,7 @@ struct ocores_i2c {
 	void __iomem *base;
 	u32 reg_shift;
 	u32 reg_io_width;
+	unsigned long flags;
 	wait_queue_head_t wait;
 	struct i2c_adapter adap;
 	struct i2c_msg *msg;
@@ -246,10 +251,113 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
 	spin_unlock_irqrestore(&i2c->process_lock, flags);
 }
 
-static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+/**
+ * Wait until something change in a given register
+ * @i2c: ocores I2C device instance
+ * @reg: register to query
+ * @mask: bitmask to apply on register value
+ * @val: expected result
+ * @timeout: timeout in jiffies
+ *
+ * Timeout is necessary to avoid to stay here forever when the chip
+ * does not answer correctly.
+ *
+ * Return: 0 on success, -ETIMEDOUT on timeout
+ */
+static int ocores_wait(struct ocores_i2c *i2c,
+		       int reg, u8 mask, u8 val,
+		       const unsigned long timeout)
+{
+	unsigned long j;
+
+	j = jiffies + timeout;
+	while (1) {
+		u8 status = oc_getreg(i2c, reg);
+
+		if ((status & mask) == val)
+			break;
+
+		if (time_after(jiffies, j))
+			return -ETIMEDOUT;
+	}
+	return 0;
+}
+
+/**
+ * Wait until is possible to process some data
+ * @i2c: ocores I2C device instance
+ *
+ * Used when the device is in polling mode (interrupts disabled).
+ *
+ * Return: 0 on success, -ETIMEDOUT on timeout
+ */
+static int ocores_poll_wait(struct ocores_i2c *i2c)
+{
+	u8 mask;
+	int err;
+
+	if (i2c->state == STATE_DONE || i2c->state == STATE_ERROR) {
+		/* transfer is over */
+		mask = OCI2C_STAT_BUSY;
+	} else {
+		/* on going transfer */
+		mask = OCI2C_STAT_TIP;
+		udelay((8 * 1000) / i2c->bus_clock_khz);
+	}
+
+	/*
+	 * once we are here we expect to get the expected result immediately
+	 * so if after 1ms we timeout then something is broken.
+	 */
+	err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
+	if (err)
+		dev_warn(i2c->adap.dev.parent,
+			 "%s: STATUS timeout, bit 0x%x did not clear in 1ms\n",
+			 __func__, mask);
+	return err;
+}
+
+
+/**
+ * It handles an IRQ-less transfer
+ * @i2c: ocores I2C device instance
+ *
+ * Even if IRQ are disabled, the I2C OpenCore IP behavior is exactly the same
+ * (only that IRQ are not produced). This means that we can re-use entirely
+ * ocores_isr(), we just add our polling code around it.
+ *
+ * It can run in atomic context
+ */
+static void ocores_process_polling(struct ocores_i2c *i2c)
+{
+	while (1) {
+		irqreturn_t ret;
+		int err;
+
+		err = ocores_poll_wait(i2c);
+		if (err) {
+			i2c->state = STATE_ERROR;
+			break; /* timeout */
+		}
+
+		ret = ocores_isr(-1, i2c);
+		if (ret == IRQ_NONE)
+			break; /* all messages have been transfered */
+	}
+}
+
+static int ocores_xfer_core(struct ocores_i2c *i2c,
+			    struct i2c_msg *msgs, int num,
+			    bool polling)
 {
-	struct ocores_i2c *i2c = i2c_get_adapdata(adap);
 	int ret;
+	u8 ctrl;
+
+	ctrl = oc_getreg(i2c, OCI2C_CONTROL);
+	if (polling)
+		oc_setreg(i2c, OCI2C_CONTROL, ctrl & ~OCI2C_CTRL_IEN);
+	else
+		oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_IEN);
 
 	i2c->msg = msgs;
 	i2c->pos = 0;
@@ -259,16 +367,37 @@ static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	oc_setreg(i2c, OCI2C_DATA, i2c_8bit_addr_from_msg(i2c->msg));
 	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_START);
 
-	ret = wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
-				 (i2c->state == STATE_DONE), HZ);
-	if (ret == 0) {
-		ocores_process_timeout(i2c);
-		return -ETIMEDOUT;
+	if (polling) {
+		ocores_process_polling(i2c);
+	} else {
+		ret = wait_event_timeout(i2c->wait,
+					 (i2c->state == STATE_ERROR) ||
+					 (i2c->state == STATE_DONE), HZ);
+		if (ret == 0) {
+			ocores_process_timeout(i2c);
+			return -ETIMEDOUT;
+		}
 	}
 
 	return (i2c->state == STATE_DONE) ? num : -EIO;
 }
 
+static int ocores_xfer_polling(struct i2c_adapter *adap,
+			       struct i2c_msg *msgs, int num)
+{
+	return ocores_xfer_core(i2c_get_adapdata(adap), msgs, num, true);
+}
+
+static int ocores_xfer(struct i2c_adapter *adap,
+		       struct i2c_msg *msgs, int num)
+{
+	struct ocores_i2c *i2c = i2c_get_adapdata(adap);
+
+	if (i2c->flags & OCORES_FLAG_POLL)
+		return ocores_xfer_polling(adap, msgs, num);
+	return ocores_xfer_core(i2c, msgs, num, false);
+}
+
 static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
 {
 	int prescale;
@@ -294,7 +423,7 @@ static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
 
 	/* Init the device */
 	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
-	oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_IEN | OCI2C_CTRL_EN);
+	oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_EN);
 
 	return 0;
 }
@@ -451,10 +580,6 @@ static int ocores_i2c_probe(struct platform_device *pdev)
 	int ret;
 	int i;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c)
 		return -ENOMEM;
@@ -509,18 +634,29 @@ static int ocores_i2c_probe(struct platform_device *pdev)
 		}
 	}
 
+	init_waitqueue_head(&i2c->wait);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq == -ENXIO) {
+		i2c->flags |= OCORES_FLAG_POLL;
+	} else {
+		if (irq < 0)
+			return irq;
+	}
+
+	if (!(i2c->flags & OCORES_FLAG_POLL)) {
+		ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
+				       pdev->name, i2c);
+		if (ret) {
+			dev_err(&pdev->dev, "Cannot claim IRQ\n");
+			goto err_clk;
+		}
+	}
+
 	ret = ocores_init(&pdev->dev, i2c);
 	if (ret)
 		goto err_clk;
 
-	init_waitqueue_head(&i2c->wait);
-	ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
-			       pdev->name, i2c);
-	if (ret) {
-		dev_err(&pdev->dev, "Cannot claim IRQ\n");
-		goto err_clk;
-	}
-
 	/* hook up driver to tree */
 	platform_set_drvdata(pdev, i2c);
 	i2c->adap = ocores_adapter;
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 4/5] i2c:ocores: add SPDX tag
  2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
                   ` (2 preceding siblings ...)
  2019-02-08 16:11 ` [PATCH v3 3/5] i2c:ocores: add polling interface Federico Vaga
@ 2019-02-08 16:12 ` Federico Vaga
  2019-02-08 17:01   ` Andrew Lunn
  2019-02-08 17:16   ` Peter Rosin
  2019-02-08 16:12 ` [PATCH v3 5/5] i2c:ocores: checkpatch fixes Federico Vaga
  2019-02-09 21:41 ` [PATCH v3 0/5] Andrew Lunn
  5 siblings, 2 replies; 24+ messages in thread
From: Federico Vaga @ 2019-02-08 16:12 UTC (permalink / raw)
  To: Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel, Federico Vaga

It adds the SPDX tag and it removes the old text about the GPLv2.

Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
---
 drivers/i2c/busses/i2c-ocores.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bbe3e96..5b80190 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * i2c-ocores.c: I2C bus driver for OpenCores I2C controller
  * (https://opencores.org/project/i2c/overview)
@@ -7,9 +8,8 @@
  * Support for the GRLIB port of the controller by
  * Andreas Larsson <andreas@gaisler.com>
  *
- * This file is licensed under the terms of the GNU General Public License
- * version 2.  This program is licensed "as is" without any warranty of any
- * kind, whether express or implied.
+ * This program is licensed "as is" without any warranty of any kind,
+ * whether express or implied.
  */
 
 #include <linux/clk.h>
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 5/5] i2c:ocores: checkpatch fixes
  2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
                   ` (3 preceding siblings ...)
  2019-02-08 16:12 ` [PATCH v3 4/5] i2c:ocores: add SPDX tag Federico Vaga
@ 2019-02-08 16:12 ` Federico Vaga
  2019-02-08 17:03   ` Andrew Lunn
  2019-02-09 21:41 ` [PATCH v3 0/5] Andrew Lunn
  5 siblings, 1 reply; 24+ messages in thread
From: Federico Vaga @ 2019-02-08 16:12 UTC (permalink / raw)
  To: Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel, Federico Vaga

Miscellaneous style fixes from checkpatch

Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
---
 drivers/i2c/busses/i2c-ocores.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 5b80190..ba35d2a 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -182,8 +182,9 @@ static void ocores_process(struct ocores_i2c *i2c, u8 stat)
 			oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
 			goto out;
 		}
-	} else
+	} else {
 		msg->buf[i2c->pos++] = oc_getreg(i2c, OCI2C_DATA);
+	}
 
 	/* end of msg? */
 	if (i2c->pos == msg->len) {
@@ -202,9 +203,9 @@ static void ocores_process(struct ocores_i2c *i2c, u8 stat)
 				oc_setreg(i2c, OCI2C_DATA, addr);
 				oc_setreg(i2c, OCI2C_CMD,  OCI2C_CMD_START);
 				goto out;
-			} else
-				i2c->state = (msg->flags & I2C_M_RD)
-					? STATE_READ : STATE_WRITE;
+			}
+			i2c->state = (msg->flags & I2C_M_RD)
+				? STATE_READ : STATE_WRITE;
 		} else {
 			i2c->state = STATE_DONE;
 			oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
@@ -405,7 +406,8 @@ static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
 	u8 ctrl = oc_getreg(i2c, OCI2C_CONTROL);
 
 	/* make sure the device is disabled */
-	oc_setreg(i2c, OCI2C_CONTROL, ctrl & ~(OCI2C_CTRL_EN|OCI2C_CTRL_IEN));
+	ctrl &= ~(OCI2C_CTRL_EN | OCI2C_CTRL_IEN);
+	oc_setreg(i2c, OCI2C_CONTROL, ctrl);
 
 	prescale = (i2c->ip_clock_khz / (5 * i2c->bus_clock_khz)) - 1;
 	prescale = clamp(prescale, 0, 0xffff);
@@ -462,11 +464,13 @@ MODULE_DEVICE_TABLE(of, ocores_i2c_match);
 #ifdef CONFIG_OF
 /* Read and write functions for the GRLIB port of the controller. Registers are
  * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
- * register. The subsequent registers has their offset decreased accordingly. */
+ * register. The subsequent registers has their offset decreased accordingly.
+ */
 static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
 {
 	u32 rd;
 	int rreg = reg;
+
 	if (reg != OCI2C_PRELOW)
 		rreg--;
 	rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
@@ -480,6 +484,7 @@ static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 value)
 {
 	u32 curr, wr;
 	int rreg = reg;
+
 	if (reg != OCI2C_PRELOW)
 		rreg--;
 	if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
@@ -568,7 +573,7 @@ static int ocores_i2c_of_probe(struct platform_device *pdev,
 	return 0;
 }
 #else
-#define ocores_i2c_of_probe(pdev,i2c) -ENODEV
+#define ocores_i2c_of_probe(pdev, i2c) -ENODEV
 #endif
 
 static int ocores_i2c_probe(struct platform_device *pdev)
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/5] i2c:ocores: stop transfer on timeout
  2019-02-08 16:11 ` [PATCH v3 1/5] i2c:ocores: stop transfer on timeout Federico Vaga
@ 2019-02-08 16:58   ` Andrew Lunn
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2019-02-08 16:58 UTC (permalink / raw)
  To: Federico Vaga; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

On Fri, Feb 08, 2019 at 05:11:57PM +0100, Federico Vaga wrote:
> Detecting a timeout is ok, but we also need to assert a STOP command on
> the bus in order to prevent it from generating interrupts when there are
> no on going transfers.
> 
> Example: very long transmission.
> 
> 1. ocores_xfer: START a transfer
> 2. ocores_isr : handle byte by byte the transfer
> 3. ocores_xfer: goes in timeout [[bugfix here]]
> 4. ocores_xfer: return to I2C subsystem and to the I2C driver
> 5. I2C driver : it may clean up the i2c_msg memory
> 6. ocores_isr : receives another interrupt (pending bytes to be
>                 transferred) but the i2c_msg memory is invalid now
> 
> So, since the transfer was too long, we have to detect the timeout and
> STOP the transfer.
> 
> Another point is that we have a critical region here. When handling the
> timeout condition we may have a running IRQ handler. For this reason I
> introduce a spinlock.
> 
> In order to make easier to understan locking I have:
> - added a new function to handle timeout
> - modified the current ocores_process() function in order to be protected
>   by the new spinlock
> Like this it is obvious at first sight that this locking serializes
> the execution of ocores_process() and ocores_process_timeout()
> 
> Signed-off-by: Federico Vaga <federico.vaga@cern.ch>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 2/5] i2c:ocores: do not handle IRQ if IF is not set
  2019-02-08 16:11 ` [PATCH v3 2/5] i2c:ocores: do not handle IRQ if IF is not set Federico Vaga
@ 2019-02-08 17:00   ` Andrew Lunn
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2019-02-08 17:00 UTC (permalink / raw)
  To: Federico Vaga; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

On Fri, Feb 08, 2019 at 05:11:58PM +0100, Federico Vaga wrote:
> If the Interrupt Flag (IF) is not set, we should not handle the IRQ:
> - the line can be shared with other devices
> - it can be a spurious interrupt
> 
> To avoid reading twice the status register, the ocores_process() function
> expects it to be read by the caller.
> 
> Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
> Acked-by: Peter Korsgaard <peter@korsgaard.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 4/5] i2c:ocores: add SPDX tag
  2019-02-08 16:12 ` [PATCH v3 4/5] i2c:ocores: add SPDX tag Federico Vaga
@ 2019-02-08 17:01   ` Andrew Lunn
  2019-02-08 17:16   ` Peter Rosin
  1 sibling, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2019-02-08 17:01 UTC (permalink / raw)
  To: Federico Vaga; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

On Fri, Feb 08, 2019 at 05:12:00PM +0100, Federico Vaga wrote:
> It adds the SPDX tag and it removes the old text about the GPLv2.
> 
> Signed-off-by: Federico Vaga <federico.vaga@cern.ch>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 5/5] i2c:ocores: checkpatch fixes
  2019-02-08 16:12 ` [PATCH v3 5/5] i2c:ocores: checkpatch fixes Federico Vaga
@ 2019-02-08 17:03   ` Andrew Lunn
  0 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2019-02-08 17:03 UTC (permalink / raw)
  To: Federico Vaga; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

On Fri, Feb 08, 2019 at 05:12:01PM +0100, Federico Vaga wrote:
> Miscellaneous style fixes from checkpatch
> 
> Signed-off-by: Federico Vaga <federico.vaga@cern.ch>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 4/5] i2c:ocores: add SPDX tag
  2019-02-08 16:12 ` [PATCH v3 4/5] i2c:ocores: add SPDX tag Federico Vaga
  2019-02-08 17:01   ` Andrew Lunn
@ 2019-02-08 17:16   ` Peter Rosin
  1 sibling, 0 replies; 24+ messages in thread
From: Peter Rosin @ 2019-02-08 17:16 UTC (permalink / raw)
  To: Federico Vaga, Peter Korsgaard, Andrew Lunn; +Cc: linux-i2c, linux-kernel

On 2019-02-08 17:12, Federico Vaga wrote:
> It adds the SPDX tag and it removes the old text about the GPLv2.
> 
> Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
> ---
>  drivers/i2c/busses/i2c-ocores.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index bbe3e96..5b80190 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * i2c-ocores.c: I2C bus driver for OpenCores I2C controller
>   * (https://opencores.org/project/i2c/overview)
> @@ -7,9 +8,8 @@
>   * Support for the GRLIB port of the controller by
>   * Andreas Larsson <andreas@gaisler.com>
>   *
> - * This file is licensed under the terms of the GNU General Public License
> - * version 2.  This program is licensed "as is" without any warranty of any
> - * kind, whether express or implied.
> + * This program is licensed "as is" without any warranty of any kind,
> + * whether express or implied.
>   */
>  
>  #include <linux/clk.h>
> 

If you feel that you need to keep some license boilerplate after adding
the SPDX tag, then the SPDX tag is per default wrong. The SPDX tag should be
a full description of the applicable license. What's the point otherwise?
In this case though, GPLv2 already contains this gist of the boilerplate
text (in section 11) so please just kill the whole paragraph.

Cheers,
Peter

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/5] i2c:ocores: add polling interface
  2019-02-08 16:11 ` [PATCH v3 3/5] i2c:ocores: add polling interface Federico Vaga
@ 2019-02-09 21:33   ` Andrew Lunn
  2019-02-11  8:20     ` Federico Vaga
  0 siblings, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2019-02-09 21:33 UTC (permalink / raw)
  To: Federico Vaga; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

> +static int ocores_poll_wait(struct ocores_i2c *i2c)
> +{
> +	u8 mask;
> +	int err;
> +
> +	if (i2c->state == STATE_DONE || i2c->state == STATE_ERROR) {
> +		/* transfer is over */
> +		mask = OCI2C_STAT_BUSY;
> +	} else {
> +		/* on going transfer */
> +		mask = OCI2C_STAT_TIP;
> +		udelay((8 * 1000) / i2c->bus_clock_khz);
> +	}
> +
> +	/*
> +	 * once we are here we expect to get the expected result immediately
> +	 * so if after 1ms we timeout then something is broken.
> +	 */
> +	err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));

Hi Federico

I did some timing tests for this. On my box, we request a udelay of
80uS. The kernel actually delays for about 79uS. We then spin in
ocores_wait() for an additional 10-11uS, which is 3 to 4 iterations.

There are actually 9 bits on the wire, not 8, since there is an
ACK/NACK bit after the actual data transfer. So i changed the delay to
(9 * 1000) / i2c->bus_clock_khz. That resulted in ocores_wait() mostly
not looping at all. But for reading an 4K AT24 EEPROM, it increased
the read time by 10ms, from 424ms to 434ms. So we should probably keep
with 8.

Tested-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/5]
  2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
                   ` (4 preceding siblings ...)
  2019-02-08 16:12 ` [PATCH v3 5/5] i2c:ocores: checkpatch fixes Federico Vaga
@ 2019-02-09 21:41 ` Andrew Lunn
  5 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2019-02-09 21:41 UTC (permalink / raw)
  To: Federico Vaga; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

On Fri, Feb 08, 2019 at 05:11:56PM +0100, Federico Vaga wrote:
> This patch set provides improvements to the i2c-ocore driver.

Hi Federico

Please could you fixup the SPDX patch, add my review/tested by tags,
and i think we are good to go.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/5] i2c:ocores: add polling interface
  2019-02-09 21:33   ` Andrew Lunn
@ 2019-02-11  8:20     ` Federico Vaga
  0 siblings, 0 replies; 24+ messages in thread
From: Federico Vaga @ 2019-02-11  8:20 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Peter Korsgaard, linux-i2c, linux-kernel

On Saturday, February 9, 2019 10:33:53 PM CET Andrew Lunn wrote:
> > +static int ocores_poll_wait(struct ocores_i2c *i2c)
> > +{
> > +	u8 mask;
> > +	int err;
> > +
> > +	if (i2c->state == STATE_DONE || i2c->state == STATE_ERROR) {
> > +		/* transfer is over */
> > +		mask = OCI2C_STAT_BUSY;
> > +	} else {
> > +		/* on going transfer */
> > +		mask = OCI2C_STAT_TIP;
> > +		udelay((8 * 1000) / i2c->bus_clock_khz);
> > +	}
> > +
> > +	/*
> > +	 * once we are here we expect to get the expected result immediately
> > +	 * so if after 1ms we timeout then something is broken.
> > +	 */
> > +	err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
> 
> Hi Federico
> 
> I did some timing tests for this. On my box, we request a udelay of
> 80uS. The kernel actually delays for about 79uS. We then spin in
> ocores_wait() for an additional 10-11uS, which is 3 to 4 iterations.
> 
> There are actually 9 bits on the wire, not 8, since there is an
> ACK/NACK bit after the actual data transfer. So i changed the delay to
> (9 * 1000) / i2c->bus_clock_khz. That resulted in ocores_wait() mostly
> not looping at all. But for reading an 4K AT24 EEPROM, it increased
> the read time by 10ms, from 424ms to 434ms. So we should probably keep
> with 8.
> 
> Tested-by: Andrew Lunn <andrew@lunn.ch>

I had a similar experience. I will add a comment in the code to explain that 8 
is not a mistake but a conscious decision. Then I will add what you wrote here
in the patch changelog

> 
>     Andrew





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/5]
  2023-05-29 15:35 Bernhard Rosenkränzer
@ 2023-05-29 16:30 ` Matthias Brugger
  0 siblings, 0 replies; 24+ messages in thread
From: Matthias Brugger @ 2023-05-29 16:30 UTC (permalink / raw)
  To: Bernhard Rosenkränzer, daniel.lezcano,
	angelogioacchino.delregno, rafael, amitk, rui.zhang, robh+dt,
	krzystof.kozlowski+dt, rdunlap, ye.xingchen, p.zabel
  Cc: linux-pm, linux-kernel, linux-arm-kernel, linux-mediatek,
	devicetree, wenst, james.lo, rex-bc.chen, nfraprado, abailon,
	amergnat, khilman

Hi Bernhard,

Please resend with a subject line for the cover letter.

Regards,
Matthias

On 29/05/2023 17:35, Bernhard Rosenkränzer wrote:
> From: Balsam CHIHI <bchihi@baylibre.com>
> 
> Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
> Also, add Suspend and Resume support to LVTS Driver (all SoCs),
> and update the documentation that describes the Calibration Data Offsets.
> 
> Changelog:
>      v3 :
>          - Rebased :
>              base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
>          - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <nfraprado@collabora.com>:
>            Use filtered mode to make sure threshold interrupts are triggered,
>            protocol documentation, cosmetics
>          - I (bero@baylibre.com) will be taking care of this patchset
>            from now on, since Balsam has left BayLibre. Thanks for
>            getting it almost ready, Balsam!
> 
>      v2 :
>          - Based on top of thermal/linux-next :
>              base-commit: 7ac82227ee046f8234471de4c12a40b8c2d3ddcc
>          - Squash "add thermal zones and thermal nodes" and
>              "add temperature mitigation threshold" commits together to form
>              "arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones" commit.
>          - Add Suspend and Resume support to LVTS Driver.
>          - Update Calibration Data documentation.
>          - Fix calibration data offsets for mt8192
>              (Thanks to "Chen-Yu Tsai" and "Nícolas F. R. A. Prado").
>          https://lore.kernel.org/all/20230425133052.199767-1-bchihi@baylibre.com/
>          Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> 
>      v1 :
>          - The initial series "Add LVTS support for mt8192" :
>              "https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/".
> 
> Balsam CHIHI (5):
>    dt-bindings: thermal: mediatek: Add LVTS thermal controller definition
>      for mt8192
>    thermal/drivers/mediatek/lvts_thermal: Add suspend and resume
>    thermal/drivers/mediatek/lvts_thermal: Add mt8192 support
>    arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones
>    thermal/drivers/mediatek/lvts_thermal: Update calibration data
>      documentation
> 
>   arch/arm64/boot/dts/mediatek/mt8192.dtsi      | 454 ++++++++++++++++++
>   drivers/thermal/mediatek/lvts_thermal.c       | 160 +++++-
>   .../thermal/mediatek,lvts-thermal.h           |  19 +
>   3 files changed, 631 insertions(+), 2 deletions(-)
> 
> base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/5]
@ 2023-05-29 15:35 Bernhard Rosenkränzer
  2023-05-29 16:30 ` Matthias Brugger
  0 siblings, 1 reply; 24+ messages in thread
From: Bernhard Rosenkränzer @ 2023-05-29 15:35 UTC (permalink / raw)
  To: daniel.lezcano, angelogioacchino.delregno, rafael, amitk,
	rui.zhang, matthias.bgg, robh+dt, krzystof.kozlowski+dt, rdunlap,
	ye.xingchen, p.zabel
  Cc: linux-pm, linux-kernel, linux-arm-kernel, linux-mediatek,
	devicetree, wenst, james.lo, rex-bc.chen, nfraprado, abailon,
	amergnat, khilman

From: Balsam CHIHI <bchihi@baylibre.com>

Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.
Also, add Suspend and Resume support to LVTS Driver (all SoCs),
and update the documentation that describes the Calibration Data Offsets.

Changelog:
    v3 : 
        - Rebased :
            base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
        - Fix issues in v2 pointed out by Nícolas F. R. A. Prado <nfraprado@collabora.com>:
          Use filtered mode to make sure threshold interrupts are triggered,
          protocol documentation, cosmetics
        - I (bero@baylibre.com) will be taking care of this patchset
          from now on, since Balsam has left BayLibre. Thanks for
          getting it almost ready, Balsam!

    v2 :
        - Based on top of thermal/linux-next :
            base-commit: 7ac82227ee046f8234471de4c12a40b8c2d3ddcc
        - Squash "add thermal zones and thermal nodes" and
            "add temperature mitigation threshold" commits together to form
            "arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones" commit.
        - Add Suspend and Resume support to LVTS Driver.
        - Update Calibration Data documentation.
        - Fix calibration data offsets for mt8192
            (Thanks to "Chen-Yu Tsai" and "Nícolas F. R. A. Prado").
        https://lore.kernel.org/all/20230425133052.199767-1-bchihi@baylibre.com/
        Tested-by: Chen-Yu Tsai <wenst@chromium.org>

    v1 :
        - The initial series "Add LVTS support for mt8192" :
            "https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/".

Balsam CHIHI (5):
  dt-bindings: thermal: mediatek: Add LVTS thermal controller definition
    for mt8192
  thermal/drivers/mediatek/lvts_thermal: Add suspend and resume
  thermal/drivers/mediatek/lvts_thermal: Add mt8192 support
  arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones
  thermal/drivers/mediatek/lvts_thermal: Update calibration data
    documentation

 arch/arm64/boot/dts/mediatek/mt8192.dtsi      | 454 ++++++++++++++++++
 drivers/thermal/mediatek/lvts_thermal.c       | 160 +++++-
 .../thermal/mediatek,lvts-thermal.h           |  19 +
 3 files changed, 631 insertions(+), 2 deletions(-)

base-commit: 6a3d37b4d885129561e1cef361216f00472f7d2e
-- 
2.41.0.rc2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/5]
  2018-05-18 14:39 Marc Zyngier
  2018-05-18 16:29 ` Vince Weaver
@ 2018-05-21 18:19 ` Will Deacon
  1 sibling, 0 replies; 24+ messages in thread
From: Will Deacon @ 2018-05-21 18:19 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, Mark Rutland, Russell King,
	Vladimir Murzin, Vince Weaver, Peter Zijlstra, Stefan Wahren,
	Eric Anholt, Florian Fainelli

Hi Marc,

Thanks for this.

On Fri, May 18, 2018 at 03:39:08PM +0100, Marc Zyngier wrote:
> PMUv3 has been introduced with ARMv8 and, while it has only been used
> on 64bit systems so far, it would definitely be useful for 32bit
> guests running under KVM/arm64, for example.
> 
> There is also the case of people natively running 32bit kernels on
> 64bit HW and trying to upstream unspeakable hacks, hoping that the
> stars will align and that they'll win the lottery (see [1]).
> 
> So let's try again, and make the PMUv3 driver usable for everyone.
> 
> This is done in three steps:
> (1) Move the driver from arch/arm64 to drivers/perf
> (2) Add a handful of system register accessors so that we can reuse
>     the driver on 32bit
> (3) Provide the same accessors on 32bit, enable compilation, and
>     make it the default selection for mach-virt.
> 
> Tested on a Seattle box with 32bit guests.

I think we should go ahead with something like this, but I don't think
we're quite there with these patches. If we're going to move the arch code
out into drivers, let's do that for the perf_event* files under arch/arm/
as well. Then we could have a structure along the lines of:


  drivers/perf/arm_pmu.c			- As it is today
  drivers/perf/arm_cpu/xscale_pmu.c		- Only builds for 32-bit
  drivers/perf/arm_cpu/armv6_pmu.c		- Only builds for 32-bit
  drivers/perf/arm_cpu/arch_pmu.c		- Works for v7/v8 on
                                                  both 32-bit and 64-bit

The latter can then pull in whatever accessors it needs from the arch
code headers.

I know it's more of an invasive change, but this way we always end up
running the same code on the two architectures and it will be much easier
to maintain.

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/5]
  2018-05-18 16:41   ` Marc Zyngier
@ 2018-05-18 17:39     ` Stefan Wahren
  0 siblings, 0 replies; 24+ messages in thread
From: Stefan Wahren @ 2018-05-18 17:39 UTC (permalink / raw)
  To: Marc Zyngier, Vince Weaver
  Cc: Peter Zijlstra, Florian Fainelli, Vladimir Murzin, Eric Anholt,
	Mark Rutland, Russell King, Will Deacon, linux-kernel,
	linux-arm-kernel

> Marc Zyngier <marc.zyngier@arm.com> hat am 18. Mai 2018 um 18:41 geschrieben:
> 
> 
> [/me beats himself for not writing a subject line...]
> 
> On 18/05/18 17:29, Vince Weaver wrote:
> > On Fri, 18 May 2018, Marc Zyngier wrote:
> > 
> >> There is also the case of people natively running 32bit kernels on
> >> 64bit HW and trying to upstream unspeakable hacks, hoping that the
> >> stars will align and that they'll win the lottery (see [1]).
> > 
> > I've tested these patches on a Raspberry Pi 3B running a 32-bit upstream 
> > (4.17-rc5-git) kernel and they work.
> > 
> > [    0.472906] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available
> > 
> > I only needed to add this to the devicetree
> > 
> > 	arm-pmu {
> > 		compatible = "arm,cortex-a53-pmu";
> > 		interrupt-parent = <&local_intc>;
> > 		interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
> > 	};
> 
> That's definitely the sensible thing to have on such hardware. Why isn't
> it in the upstream DT already, irrespective of the state of the kernel
> support?

I remember that Vince point out the absence. He asked about how to implement it and i wasn't sure about it. At this time we hadn't IRQ polarity support. So we wanted to get this puzzle piece before. In march i put it on my TODO list, but then RPI 3 B+ support had higher prio to get into 4.18.

In general we have the problem that most of the users take the downstream kernel and don't know about the differences. Luckily more distributions switch to the upstream kernel, which increases the feedback.

> 
> > Tested-by: Vince Weaver <vincent.weaver@maine.edu>

Thanks again
Stefan

> 
> Thanks a lot for testing.
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/5]
  2018-05-18 16:29 ` Vince Weaver
@ 2018-05-18 16:41   ` Marc Zyngier
  2018-05-18 17:39     ` Stefan Wahren
  0 siblings, 1 reply; 24+ messages in thread
From: Marc Zyngier @ 2018-05-18 16:41 UTC (permalink / raw)
  To: Vince Weaver
  Cc: linux-arm-kernel, linux-kernel, Will Deacon, Mark Rutland,
	Russell King, Vladimir Murzin, Peter Zijlstra, Stefan Wahren,
	Eric Anholt, Florian Fainelli

[/me beats himself for not writing a subject line...]

On 18/05/18 17:29, Vince Weaver wrote:
> On Fri, 18 May 2018, Marc Zyngier wrote:
> 
>> There is also the case of people natively running 32bit kernels on
>> 64bit HW and trying to upstream unspeakable hacks, hoping that the
>> stars will align and that they'll win the lottery (see [1]).
> 
> I've tested these patches on a Raspberry Pi 3B running a 32-bit upstream 
> (4.17-rc5-git) kernel and they work.
> 
> [    0.472906] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available
> 
> I only needed to add this to the devicetree
> 
> 	arm-pmu {
> 		compatible = "arm,cortex-a53-pmu";
> 		interrupt-parent = <&local_intc>;
> 		interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
> 	};

That's definitely the sensible thing to have on such hardware. Why isn't
it in the upstream DT already, irrespective of the state of the kernel
support?

> Tested-by: Vince Weaver <vincent.weaver@maine.edu>

Thanks a lot for testing.

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/5]
  2018-05-18 14:39 Marc Zyngier
@ 2018-05-18 16:29 ` Vince Weaver
  2018-05-18 16:41   ` Marc Zyngier
  2018-05-21 18:19 ` Will Deacon
  1 sibling, 1 reply; 24+ messages in thread
From: Vince Weaver @ 2018-05-18 16:29 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, Will Deacon, Mark Rutland,
	Russell King, Vladimir Murzin, Vince Weaver, Peter Zijlstra,
	Stefan Wahren, Eric Anholt, Florian Fainelli

On Fri, 18 May 2018, Marc Zyngier wrote:

> There is also the case of people natively running 32bit kernels on
> 64bit HW and trying to upstream unspeakable hacks, hoping that the
> stars will align and that they'll win the lottery (see [1]).

I've tested these patches on a Raspberry Pi 3B running a 32-bit upstream 
(4.17-rc5-git) kernel and they work.

[    0.472906] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available

I only needed to add this to the devicetree

	arm-pmu {
		compatible = "arm,cortex-a53-pmu";
		interrupt-parent = <&local_intc>;
		interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
	};


Tested-by: Vince Weaver <vincent.weaver@maine.edu>

Vince

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/5]
@ 2018-05-18 14:39 Marc Zyngier
  2018-05-18 16:29 ` Vince Weaver
  2018-05-21 18:19 ` Will Deacon
  0 siblings, 2 replies; 24+ messages in thread
From: Marc Zyngier @ 2018-05-18 14:39 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Mark Rutland, Russell King, Vladimir Murzin,
	Vince Weaver, Peter Zijlstra, Stefan Wahren, Eric Anholt,
	Florian Fainelli

PMUv3 has been introduced with ARMv8 and, while it has only been used
on 64bit systems so far, it would definitely be useful for 32bit
guests running under KVM/arm64, for example.

There is also the case of people natively running 32bit kernels on
64bit HW and trying to upstream unspeakable hacks, hoping that the
stars will align and that they'll win the lottery (see [1]).

So let's try again, and make the PMUv3 driver usable for everyone.

This is done in three steps:
(1) Move the driver from arch/arm64 to drivers/perf
(2) Add a handful of system register accessors so that we can reuse
    the driver on 32bit
(3) Provide the same accessors on 32bit, enable compilation, and
    make it the default selection for mach-virt.

Tested on a Seattle box with 32bit guests.

* From v1:
  - Fixed encodings for some CP15 accessors
  - Added a terse note saying that CPU_V7 also covers ARMv8
  - Rebased on v4.12-rc5

* From v2:
  - SPDX tags on new and moved files. Yeah!
  - Annual rebase on 4.17-rc5

[1] https://patchwork.kernel.org/patch/10406793/

Marc Zyngier (5):
  arm64: perf: Move PMUv3 driver to drivers/perf
  arm64: perf: Abstract system register accesses away
  ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations
  ARM: perf: Allow the use of the PMUv3 driver on 32bit ARM
  ARM: mach-virt: Select PMUv3 driver by default

 arch/arm/Kconfig                                   |   1 +
 arch/arm/include/asm/arm_pmuv3.h                   | 125 +++++++++++++++++++++
 arch/arm/mm/Kconfig                                |   2 +-
 arch/arm64/include/asm/arm_pmuv3.h                 | 111 ++++++++++++++++++
 arch/arm64/include/asm/perf_event.h                |  55 ---------
 arch/arm64/kernel/Makefile                         |   1 -
 drivers/perf/Kconfig                               |   8 ++
 drivers/perf/Makefile                              |   1 +
 .../perf_event.c => drivers/perf/arm_pmuv3.c       |  42 ++++---
 include/kvm/arm_pmu.h                              |   2 +-
 include/linux/perf/arm_pmuv3.h                     |  78 +++++++++++++
 11 files changed, 346 insertions(+), 80 deletions(-)
 create mode 100644 arch/arm/include/asm/arm_pmuv3.h
 create mode 100644 arch/arm64/include/asm/arm_pmuv3.h
 rename arch/arm64/kernel/perf_event.c => drivers/perf/arm_pmuv3.c (97%)
 create mode 100644 include/linux/perf/arm_pmuv3.h

-- 
2.14.2

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/5]
@ 2015-07-12  5:10 Taeung Song
  0 siblings, 0 replies; 24+ messages in thread
From: Taeung Song @ 2015-07-12  5:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, jolsa, namhyung, Ingo Molnar, Taeung Song

Changes in v3:
	- builtin-config.c: Add a config variable ’kmem.default’ with a default value into ‘struct default_configset’
	which has default config variables and values.
	- cmd_config(): Add a option ‘—global’ and ‘—local’ to enable  config file location to  be selected

Changes in v2:
	- Renaming variables a more suitable name
	  1. ’—list-all' instead of '--all'
	  2. ’name' instead of 'subkey'
	  3. 'section, name, value' instead of 'given_section,subkey,value'
	- Documentation/perf-config.txt: Correct small infelicities or typing errors in a perf-config documention.
	- Documentation/perf-config.txt: Remove a part description of report.children because it was duplicated
	in Documentation/callchain-overhead-calculation.txt
	- builtin-config.c: Use a variable ’int actions’ instead of struct params which has ‘bool list_action’,
	‘bool get_action’ and etc. , to simplify a branching statement for perf-config options 
	- builtin-config.c: Declaration a global variable ‘static struct default_configsets’ has config variables
	with default values instead of using a 'util/PERFCONFIG-DEFAULT' file and remove functions merge()
	and perse_key() to get perf config default values.
	- normalize_value(): Add a function to normalize a value and check data type of it.
	- cmd_config(): Simplify parsing arguments as arguments is just divided by '=' and then in front of '.' is a section,
	between '.' and '=' is a name, and behind '=' is a value.
	- show_all_config(): Print config variables ‘struct default_configsets’ haven't 
	- cmd_config(): Make a command ’perf config' without a option work as with a option ’—list’ instead of ‘—list-all’.

Taeung Song (5):
  perf tools: Add 'perf-config' command
  perf config: Add functions which can get or set perf config variables.
  perf config: Add a option 'list-all' to perf-config.
  perf config: Add a option 'remove' to perf-config.
  perf config: Add ‘—system’ and ‘—global’ options to be able to select
    which config file to be used.

 tools/perf/Build                            |   1 +
 tools/perf/Documentation/perf-config.txt    | 401 ++++++++++++++++
 tools/perf/Documentation/perfconfig.example |  70 ++-
 tools/perf/builtin-config.c                 | 708 ++++++++++++++++++++++++++++
 tools/perf/builtin.h                        |   1 +
 tools/perf/command-list.txt                 |   1 +
 tools/perf/perf.c                           |   1 +
 tools/perf/util/cache.h                     |  20 +
 tools/perf/util/config.c                    |  84 +++-
 9 files changed, 1251 insertions(+), 36 deletions(-)
 create mode 100644 tools/perf/Documentation/perf-config.txt
 create mode 100644 tools/perf/builtin-config.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/5]
@ 2014-12-08  9:46 Yunzhi Li
  0 siblings, 0 replies; 24+ messages in thread
From: Yunzhi Li @ 2014-12-08  9:46 UTC (permalink / raw)
  To: heiko, dianders, romain.perier
  Cc: olof, huangtao, zyw, cf, linux-rockchip, Yunzhi Li, devicetree,
	Paul Zimmerman, linux-usb, Kumar Gala, linux-kernel,
	Grant Likely, Ian Campbell, Rob Herring, Pawel Moll,
	Kishon Vijay Abraham I, Mark Rutland, Russell King,
	linux-arm-kernel, Greg Kroah-Hartman

Patches to add support for Rockchip usb phys.Add a new Rockchip
usb phy driver and modify dwc2 controller driver to make dwc2
platform devices support a generic PHY framework driver. This
patch set has been tested on my rk3288-evb and power off the usb
phys would reduce about 60mW power budget in total during sustem
suspend.

Changes in v3:
- Use BIT macro instead of bit shift ops.
- Rename the config entry to PHY_ROCKCHIP_USB.
- Fix coding style: both branches of the if() which only one
  branch of the conditional statement is a single statement should
  have braces.
- No need to test dwc2->phy for NULL before calling generic phy
  APIs.
- Add more context about the changes in the long description.

Yunzhi Li (5):
  phy: add a driver for the Rockchip SoC internal USB2.0 PHY
  Documentation: bindings: add doc for the Rockchip usb PHY
  usb: dwc2: add generic PHY framework support for dwc2 usb    
    controler platform driver.
  ARM: dts: add rk3288 usb PHY
  ARM: dts: Enable usb PHY on rk3288-evb board

 .../devicetree/bindings/phy/rockchip-usb-phy.txt   |  22 +++
 arch/arm/boot/dts/rk3288-evb.dtsi                  |   4 +
 arch/arm/boot/dts/rk3288.dtsi                      |  13 ++
 drivers/phy/Kconfig                                |   7 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-rockchip-usb.c                     | 179 +++++++++++++++++++++
 drivers/usb/dwc2/gadget.c                          |  33 ++--
 drivers/usb/dwc2/platform.c                        |  36 ++++-
 8 files changed, 272 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
 create mode 100644 drivers/phy/phy-rockchip-usb.c

-- 
2.0.0



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/5]
@ 2009-11-10 22:36 Alex Chiang
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Chiang @ 2009-11-10 22:36 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes

This is v3 of the series.

I based it off of Linus's latest tree. 

I did not include David Rientjes's "mm: slab allocate memory section nodemask
for large systems" patch in my series, since it's not necessarily related.

Please consider for inclusion for the next merge window (v2.6.33).

Thanks,
/ac

v2 -> v3:
	- rebased to Linus's latest tree (799dd75b)
	- Added David Rientjes's Acked-by: flags
	- dropped S390 cc's, since they are unaffected by this series

v1 -> v2: http://thread.gmane.org/gmane.linux.kernel.mm/40084/
        Address David Rientjes's comments
        - check return value of sysfs_create_link in register_cpu_under_node
        - do /not/ convert [un]register_cpu_under_node to return void, since
          sparse starts whinging if you ignore sysfs_create_link()'s return
          value and working around sparse makes the code ugly
        - adjust documentation

        Added S390 maintainers to cc: for patch [1/5] as per Kame-san's
        suggestion. S390 may map a memory section to more than one node,
        causing this series to break.


---

Alex Chiang (5):
      mm: add numa node symlink for memory section in sysfs
      mm: refactor register_cpu_under_node()
      mm: refactor unregister_cpu_under_node()
      mm: add numa node symlink for cpu devices in sysfs
      Documentation: ABI: /sys/devices/system/cpu/cpu#/node


 Documentation/ABI/testing/sysfs-devices-memory     |   14 ++++-
 Documentation/ABI/testing/sysfs-devices-system-cpu |   14 +++++
 Documentation/memory-hotplug.txt                   |   11 ++--
 drivers/base/node.c                                |   58 ++++++++++++++------
 4 files changed, 76 insertions(+), 21 deletions(-)


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-05-29 16:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 16:11 [PATCH v3 0/5] Federico Vaga
2019-02-08 16:11 ` [PATCH v3 1/5] i2c:ocores: stop transfer on timeout Federico Vaga
2019-02-08 16:58   ` Andrew Lunn
2019-02-08 16:11 ` [PATCH v3 2/5] i2c:ocores: do not handle IRQ if IF is not set Federico Vaga
2019-02-08 17:00   ` Andrew Lunn
2019-02-08 16:11 ` [PATCH v3 3/5] i2c:ocores: add polling interface Federico Vaga
2019-02-09 21:33   ` Andrew Lunn
2019-02-11  8:20     ` Federico Vaga
2019-02-08 16:12 ` [PATCH v3 4/5] i2c:ocores: add SPDX tag Federico Vaga
2019-02-08 17:01   ` Andrew Lunn
2019-02-08 17:16   ` Peter Rosin
2019-02-08 16:12 ` [PATCH v3 5/5] i2c:ocores: checkpatch fixes Federico Vaga
2019-02-08 17:03   ` Andrew Lunn
2019-02-09 21:41 ` [PATCH v3 0/5] Andrew Lunn
  -- strict thread matches above, loose matches on Subject: below --
2023-05-29 15:35 Bernhard Rosenkränzer
2023-05-29 16:30 ` Matthias Brugger
2018-05-18 14:39 Marc Zyngier
2018-05-18 16:29 ` Vince Weaver
2018-05-18 16:41   ` Marc Zyngier
2018-05-18 17:39     ` Stefan Wahren
2018-05-21 18:19 ` Will Deacon
2015-07-12  5:10 Taeung Song
2014-12-08  9:46 Yunzhi Li
2009-11-10 22:36 Alex Chiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).