Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 07/23] drivers/fsi: Implement slave initialisation
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mingo-H+wXaHxf7aLQT0dZR+AlfA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Jeremy Kerr, joel-U3u1mxZcP9KHXe+LvDLADg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, andrew-zrmu5oMJ5Fs,
	alistair-Y4h6yKqj69EXC2x5gXVKYQ,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, Chris Bostic
In-Reply-To: <20170405020607.79939-1-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

Implement fsi_slave_init: if we can read a chip ID, create fsi_slave
devices and register with the driver core.

Includes changes from Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>.

Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
---
 drivers/fsi/fsi-core.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 6e1cfdf..c705ca2 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -17,9 +17,12 @@
 #include <linux/fsi.h>
 #include <linux/idr.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #include "fsi-master.h"
 
+#define FSI_SLAVE_SIZE_23b		0x800000
+
 static DEFINE_IDA(master_ida);
 
 struct fsi_slave {
@@ -114,11 +117,70 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
 			addr, val, size);
 }
 
+static void fsi_slave_release(struct device *dev)
+{
+	struct fsi_slave *slave = to_fsi_slave(dev);
+
+	kfree(slave);
+}
+
 static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 {
-	/* todo: initialise slave device, perform engine scan */
+	struct fsi_slave *slave;
+	uint32_t chip_id;
+	uint8_t crc;
+	int rc;
+
+	/* Currently, we only support single slaves on a link, and use the
+	 * full 23-bit address range
+	 */
+	if (id != 0)
+		return -EINVAL;
+
+	rc = fsi_master_read(master, link, id, 0, &chip_id, sizeof(chip_id));
+	if (rc) {
+		dev_warn(&master->dev, "can't read slave %02x:%02x %d\n",
+				link, id, rc);
+		return -ENODEV;
+	}
+	chip_id = be32_to_cpu(chip_id);
+
+	crc = fsi_crc4(0, chip_id, 32);
+	if (crc) {
+		dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n",
+				link, id);
+		return -EIO;
+	}
+
+	dev_info(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n",
+			chip_id, master->idx, link, id);
+
+	/* We can communicate with a slave; create the slave device and
+	 * register.
+	 */
+	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
+	if (!slave)
+		return -ENOMEM;
+
+	slave->master = master;
+	slave->dev.parent = &master->dev;
+	slave->dev.release = fsi_slave_release;
+	slave->link = link;
+	slave->id = id;
+	slave->size = FSI_SLAVE_SIZE_23b;
+
+	dev_set_name(&slave->dev, "slave@%02x:%02x", link, id);
+	rc = device_register(&slave->dev);
+	if (rc < 0) {
+		dev_warn(&master->dev, "failed to create slave device: %d\n",
+				rc);
+		put_device(&slave->dev);
+		return rc;
+	}
+
+	/* todo: perform engine scan */
 
-	return -ENODEV;
+	return rc;
 }
 
 /* FSI master support */
-- 
1.8.2.2

--
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 v5 06/23] drivers/fsi: Set up links for slave communication
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Chris Bostic, joel, linux-kernel, andrew, alistair, benh
In-Reply-To: <20170405020607.79939-1-cbostic@linux.vnet.ibm.com>

From: Chris Bostic <cbostic@linux.vnet.ibm.com>

Enable each link and send a break command, and try to detect a slave by
reading from the SMODE register.

Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 32698ed..6e1cfdf 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -157,12 +157,45 @@ static int fsi_master_write(struct fsi_master *master, int link,
 	return master->write(master, link, slave_id, addr, val, size);
 }
 
+static int fsi_master_link_enable(struct fsi_master *master, int link)
+{
+	if (master->link_enable)
+		return master->link_enable(master, link);
+
+	return 0;
+}
+
+/*
+ * Issue a break command on this link
+ */
+static int fsi_master_break(struct fsi_master *master, int link)
+{
+	if (master->send_break)
+		return master->send_break(master, link);
+
+	return 0;
+}
+
 static int fsi_master_scan(struct fsi_master *master)
 {
-	int link;
+	int link, rc;
+
+	for (link = 0; link < master->n_links; link++) {
+		rc = fsi_master_link_enable(master, link);
+		if (rc) {
+			dev_dbg(&master->dev,
+				"enable link %d failed: %d\n", link, rc);
+			continue;
+		}
+		rc = fsi_master_break(master, link);
+		if (rc) {
+			dev_dbg(&master->dev,
+				"break to link %d failed: %d\n", link, rc);
+			continue;
+		}
 
-	for (link = 0; link < master->n_links; link++)
 		fsi_slave_init(master, link, 0);
+	}
 
 	return 0;
 }
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v5 05/23] drivers/fsi: Add slave & master read/write APIs
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Jeremy Kerr, joel, linux-kernel, andrew, alistair, benh,
	Chris Bostic
In-Reply-To: <20170405020607.79939-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Introduce functions to perform reads/writes on the slave address space;
these simply pass the request on the slave's master with the correct
link and slave ID.

We implement these on top of similar helpers for the master.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
---
 drivers/fsi/fsi-core.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 4bbca95..32698ed 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,6 +32,11 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static int fsi_master_read(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, void *val, size_t size);
+static int fsi_master_write(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, const void *val, size_t size);
+
 /* crc helpers */
 static const uint8_t crc4_tab[] = {
 	0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
@@ -57,6 +62,58 @@ uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
 EXPORT_SYMBOL_GPL(fsi_crc4);
 
 /* FSI slave support */
+static int fsi_slave_calc_addr(struct fsi_slave *slave, uint32_t *addrp,
+		uint8_t *idp)
+{
+	uint32_t addr = *addrp;
+	uint8_t id = *idp;
+
+	if (addr > slave->size)
+		return -EINVAL;
+
+	/* For 23 bit addressing, we encode the extra two bits in the slave
+	 * id (and the slave's actual ID needs to be 0).
+	 */
+	if (addr > 0x1fffff) {
+		if (slave->id != 0)
+			return -EINVAL;
+		id = (addr >> 21) & 0x3;
+		addr &= 0x1fffff;
+	}
+
+	*addrp = addr;
+	*idp = id;
+	return 0;
+}
+
+static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
+			void *val, size_t size)
+{
+	uint8_t id = slave->id;
+	int rc;
+
+	rc = fsi_slave_calc_addr(slave, &addr, &id);
+	if (rc)
+		return rc;
+
+	return fsi_master_read(slave->master, slave->link, id,
+			addr, val, size);
+}
+
+static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
+			const void *val, size_t size)
+{
+	uint8_t id = slave->id;
+	int rc;
+
+	rc = fsi_slave_calc_addr(slave, &addr, &id);
+	if (rc)
+		return rc;
+
+	return fsi_master_write(slave->master, slave->link, id,
+			addr, val, size);
+}
+
 static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 {
 	/* todo: initialise slave device, perform engine scan */
@@ -65,6 +122,41 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 }
 
 /* FSI master support */
+static int fsi_check_access(uint32_t addr, size_t size)
+{
+	if (size != 1 && size != 2 && size != 4)
+		return -EINVAL;
+
+	if ((addr & 0x3) != (size & 0x3))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int fsi_master_read(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, void *val, size_t size)
+{
+	int rc;
+
+	rc = fsi_check_access(addr, size);
+	if (rc)
+		return rc;
+
+	return master->read(master, link, slave_id, addr, val, size);
+}
+
+static int fsi_master_write(struct fsi_master *master, int link,
+		uint8_t slave_id, uint32_t addr, const void *val, size_t size)
+{
+	int rc;
+
+	rc = fsi_check_access(addr, size);
+	if (rc)
+		return rc;
+
+	return master->write(master, link, slave_id, addr, val, size);
+}
+
 static int fsi_master_scan(struct fsi_master *master)
 {
 	int link;
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v5 04/23] drivers/fsi: Add crc4 helpers
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Jeremy Kerr, joel, linux-kernel, andrew, alistair, benh,
	Chris Bostic
In-Reply-To: <20170405020607.79939-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Add some helpers for the crc checks for the slave configuration table.
This works 4-bits-at-a-time, using a simple table approach.

We will need this in the FSI core code, as well as any master
implementations that need to calculate CRCs in software.

We add this to the fsi code (rather than lib/), as we need a specific
polynomial for FSI CRCs.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c   | 24 ++++++++++++++++++++++++
 drivers/fsi/fsi-master.h | 21 +++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index e90d45d..4bbca95 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,6 +32,30 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* crc helpers */
+static const uint8_t crc4_tab[] = {
+	0x0, 0x7, 0xe, 0x9, 0xb, 0xc, 0x5, 0x2,
+	0x1, 0x6, 0xf, 0x8, 0xa, 0xd, 0x4, 0x3,
+};
+
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits)
+{
+	int i;
+
+	/* Align to 4-bits */
+	bits = (bits + 3) & ~0x3;
+
+	/* mask off anything above the top bit */
+	x &= (1ull << bits) - 1;
+
+	/* Calculate crc4 over four-bit nibbles, starting at the MSbit */
+	for (i = bits - 4; i >= 0; i -= 4)
+		c = crc4_tab[c ^ ((x >> i) & 0xf)];
+
+	return c;
+}
+EXPORT_SYMBOL_GPL(fsi_crc4);
+
 /* FSI slave support */
 static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
 {
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 7764b00..d6a4885 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -38,4 +38,25 @@ struct fsi_master {
 extern int fsi_master_register(struct fsi_master *master);
 extern void fsi_master_unregister(struct fsi_master *master);
 
+/**
+ * crc4 helper: Given a starting crc4 state @c, calculate the crc4 vaue of @x,
+ * which is @bits in length. This may be required by master implementations
+ * that do not provide their own hardware checksums.
+ *
+ * The crc4 is performed on 4-bit chunks (which is all we need for FSI
+ * calculations). Typically, we'll want a starting state of 0:
+ *
+ *  c = fsi_crc4(0, msg, len);
+ *
+ * To crc4 a message that includes a single start bit, initialise crc4 state
+ * with:
+ *
+ *  c = fsi_crc4(0, 1, 1);
+ *
+ * Then update with message data:
+ *
+ *  c = fsi_crc4(c, msg, len);
+ */
+uint8_t fsi_crc4(uint8_t c, uint64_t x, int bits);
+
 #endif /* DRIVERS_FSI_MASTER_H */
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v5 03/23] drivers/fsi: Add empty master scan
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mingo-H+wXaHxf7aLQT0dZR+AlfA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Jeremy Kerr, joel-U3u1mxZcP9KHXe+LvDLADg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, andrew-zrmu5oMJ5Fs,
	alistair-Y4h6yKqj69EXC2x5gXVKYQ,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, Chris Bostic
In-Reply-To: <20170405020607.79939-1-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

When a new fsi master is added, we will need to scan its links, and
slaves attached to those links. This change introduces a little shell to
iterate the links, which we will populate with the actual slave scan in
a later change.

Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
---
 drivers/fsi/fsi-core.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 2f19509..e90d45d 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -32,7 +32,25 @@ struct fsi_slave {
 
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+/* FSI slave support */
+static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
+{
+	/* todo: initialise slave device, perform engine scan */
+
+	return -ENODEV;
+}
+
 /* FSI master support */
+static int fsi_master_scan(struct fsi_master *master)
+{
+	int link;
+
+	for (link = 0; link < master->n_links; link++)
+		fsi_slave_init(master, link, 0);
+
+	return 0;
+}
+
 int fsi_master_register(struct fsi_master *master)
 {
 	int rc;
@@ -44,10 +62,13 @@ int fsi_master_register(struct fsi_master *master)
 	dev_set_name(&master->dev, "fsi%d", master->idx);
 
 	rc = device_register(&master->dev);
-	if (rc)
+	if (rc) {
 		ida_simple_remove(&master_ida, master->idx);
+		return rc;
+	}
 
-	return rc;
+	fsi_master_scan(master);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(fsi_master_register);
 
-- 
1.8.2.2

--
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 v5 02/23] drivers/fsi: Add slave definition
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Jeremy Kerr, joel, linux-kernel, andrew, alistair, benh,
	Chris Bostic
In-Reply-To: <20170405020607.79939-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Add the initial fsi slave device, which is private to the core code.
This will be a child of the master, and parent to endpoint devices.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ca02913..2f19509 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -22,6 +22,16 @@
 
 static DEFINE_IDA(master_ida);
 
+struct fsi_slave {
+	struct device		dev;
+	struct fsi_master	*master;
+	int			id;
+	int			link;
+	uint32_t		size;	/* size of slave address space */
+};
+
+#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
+
 /* FSI master support */
 int fsi_master_register(struct fsi_master *master)
 {
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v5 01/23] drivers/fsi: Add fsi master definition
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, rostedt, mingo, gregkh, devicetree,
	linux-arm-kernel
  Cc: Jeremy Kerr, joel, linux-kernel, andrew, alistair, benh,
	Chris Bostic
In-Reply-To: <20170405020607.79939-1-cbostic@linux.vnet.ibm.com>

From: Jeremy Kerr <jk@ozlabs.org>

Add a `struct fsi_master` to represent a FSI master controller.

FSI master drivers register one of these structs to provide
device-specific of the standard operations: read/write/term/break and
link control.

Includes changes from Edward A. James <eajames@us.ibm.com> & Jeremy Kerr
<jk@ozlabs.org>.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-core.c   | 35 +++++++++++++++++++++++++++++++++++
 drivers/fsi/fsi-master.h | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 drivers/fsi/fsi-master.h

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 3d55bd5..ca02913 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -15,8 +15,43 @@
 
 #include <linux/device.h>
 #include <linux/fsi.h>
+#include <linux/idr.h>
 #include <linux/module.h>
 
+#include "fsi-master.h"
+
+static DEFINE_IDA(master_ida);
+
+/* FSI master support */
+int fsi_master_register(struct fsi_master *master)
+{
+	int rc;
+
+	if (!master)
+		return -EINVAL;
+
+	master->idx = ida_simple_get(&master_ida, 0, INT_MAX, GFP_KERNEL);
+	dev_set_name(&master->dev, "fsi%d", master->idx);
+
+	rc = device_register(&master->dev);
+	if (rc)
+		ida_simple_remove(&master_ida, master->idx);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(fsi_master_register);
+
+void fsi_master_unregister(struct fsi_master *master)
+{
+	if (master->idx >= 0) {
+		ida_simple_remove(&master_ida, master->idx);
+		master->idx = -1;
+	}
+
+	device_unregister(&master->dev);
+}
+EXPORT_SYMBOL_GPL(fsi_master_unregister);
+
 /* FSI core & Linux bus type definitions */
 
 static int fsi_bus_match(struct device *dev, struct device_driver *drv)
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
new file mode 100644
index 0000000..7764b00
--- /dev/null
+++ b/drivers/fsi/fsi-master.h
@@ -0,0 +1,41 @@
+/*
+ * FSI master definitions. These comprise the core <--> master interface,
+ * to allow the core to interact with the (hardware-specific) masters.
+ *
+ * Copyright (C) IBM Corporation 2016
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef DRIVERS_FSI_MASTER_H
+#define DRIVERS_FSI_MASTER_H
+
+#include <linux/device.h>
+
+struct fsi_master {
+	struct device	dev;
+	int		idx;
+	int		n_links;
+	int		flags;
+	int		(*read)(struct fsi_master *, int link, uint8_t id,
+				uint32_t addr, void *val, size_t size);
+	int		(*write)(struct fsi_master *, int link, uint8_t id,
+				uint32_t addr, const void *val, size_t size);
+	int		(*term)(struct fsi_master *, int link, uint8_t id);
+	int		(*send_break)(struct fsi_master *, int link);
+	int		(*link_enable)(struct fsi_master *, int link);
+};
+
+#define dev_to_fsi_master(d) container_of(d, struct fsi_master, dev)
+
+extern int fsi_master_register(struct fsi_master *master);
+extern void fsi_master_unregister(struct fsi_master *master);
+
+#endif /* DRIVERS_FSI_MASTER_H */
-- 
1.8.2.2

^ permalink raw reply related

* [PATCH v5 00/23] FSI device driver implementation
From: Christopher Bostic @ 2017-04-05  2:05 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, rostedt-nx8X9YLhiw1AfugRpC6u6w,
	mingo-H+wXaHxf7aLQT0dZR+AlfA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Christopher Bostic, joel-U3u1mxZcP9KHXe+LvDLADg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, andrew-zrmu5oMJ5Fs,
	alistair-Y4h6yKqj69EXC2x5gXVKYQ,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r

Implementation of the IBM 'Flexible Support Interface' (FSI) bus device
driver. FSI is a high fan out serial bus consisting of a clock and a serial
data line capable of running at speeds up to 166 MHz.

This set provides the basic framework to add FSI extensions to the
Linux bus and device models. Master specific implementations are
defined to utilize the core FSI function.

In Linux, we have a core FSI "bus type", along with drivers for FSI
masters and engines.

The FSI master drivers expose a read/write interface to the bus address
space. The master drivers are under drivers/fsi/fsi-master-*.c.

The core handles probing and discovery of slaves and slave
engines, using those read/write interfaces. It is responsible for
creating the endpoint Linux devices corresponding to the discovered
engines on each slave.

Slave engines are identified by an 'engine' type, and an optional
version. Engine, a.k.a. client, drivers are matched and bound to these
engines during discovery.

This patch set does not include extended FSI function such as:
    *  Cascaded master support
    *  Application layer hot plug notification
    *  Application layer FSI bus status interface

Common FSI terminology:

* Master
    Controller of the FSI bus.  Only the master is allowed to control the
    clock line and is the initiator of all transactions on a bus.

* Slave
    The receiver or target of a master initiated transaction.  The slave
    cannot initiate communications on a bus and must respond to any
    master requests for data.

* CFAM
    Stands for Common Field replaceable unit Access Macro.  A CFAM is an
    ASIC residing in any device requiring FSI communications. CFAMs
    consist of an array of hardware 'engines' used for various purposes.
    I2C masters, UARTs, General Purpose IO hardware are common types of
    these engines.

* Configuration Space / Table
    A table contained at the beginning of each CFAM address space.
    This table lists information such as the CFAM's ID, which engine types
    and versions it has available, as well as its addressing range.

* FSI Engine driver
    A device driver that registers with the FSI core so that it can access
    devices it owns on an FSI bus.

* Hub
    An FSI master that connects to an upstream 'primary' master allowing
    high fanout of target devices.

----
Changes in v5:
   - Remove explicit kfree of struct fsi_master in fsi_master_gpio.
   - Remove Palmetto and Romulus dts device tree file udates for FSI gpio
     master

Changes in v4:
   - endianness: the _read() and _write() APIs are now all *bus endian*,
     so will be the same on all platforms (the previous fsi patches
     exposed as (BMC/FSP) CPU endian, which is variable).
   - device tree: Remove the "ibm," prefix for the fsi core and GPIO
     master compatibility strings, as they're not describing
     IBM-specific
   - device model: Create separate struct devices for each FSI
     master, which fits better with the Linux device model, and allows
     addition of sysfs attributes that are implemented by the fsi core
   - sysfs: there are now sysfs facilities for break and term. Raw
     file supports reads and writes of arbitrary sizes.
   - GPIO master: split the xfer() logic out a little, so that the
     response handling & DPOLL retry mechanism is more obvious
   - GPIO master: simplifications for message construction
   - GPIO master: fixes for some CRC calculations
   - GPIO master: issue TERM in response to DPOLL busy-loops
   - Error handling: rather than handle errors on (potentially) an entire
     cascaded read or write, the error handling is now down on a per-slave
     basis, where we try to reestablish communication in a more "gradual"
     manner, rather than sending a break immediately. May need to add a
     hook to percolate error recovery up to a slave's master but no need
     seen for that at present.
   - Hub master: this is now implemented as a fsi engine driver, as the
     fsi_slave_{read,write}() functions are exported (and the port count
     is available in the hMFSI configuration register)
     This means we need fewer special-cases in the fsi core.
   - Tracepoints: Add tracepoints for FSI core read & write, and another
     set for low-level GPIO in/out operations.

Changes in v3:
    - Patch set contained an invalid 18/18 test patch not
      meant for community review, corrected.

Changes in v2:
    - Change from atomic global for master number to ida simple
      interface.
    - Add valid pointer checks on register and unregister utils.
    - Move CRC calculation utilities out of driver to lib path.
    - Clean up white space issues.
    - Remove added list management of master devices and use
      instead the device_for_each_child method available in the
      bus.
    - Add new patch to document FSI bus functionality.
    - Add new patch documenting FSI gpio master.
    - Rearrage patch set to have documentation earlier than code
      implementing it.
    - Document all compatible strings used in device tree bindings.
    - Elaborate documentation definition of FSI GPIO master.
    - Describe in more detail what each GPIO FSI master pin is for.
    - Re-order compatible strings in example binding so that most
      specific device comes first.
    - Indicate proper activation order of all FSI GPIO master pins.
    - Fix an unmatched '>' bracket in the example for binding.
    - Bracket each element of the example bindings individually.
    - Add new patch documenting sysfs-bus-fsi attributes.
    - Merge FSI GPIO master init into probe function.
    - Set pin initial values at time of pin request.
    - Assign value of master->master.dev at probe time.
    - Use get_optional interface for all optional GPIO pins.

Chris Bostic (9):
  drivers/fsi: Set up links for slave communication
  drivers/fsi: Set slave SMODE to init communication
  drivers/fsi: Add master unscan
  drivers/fsi: Add documentation for GPIO bindings
  drivers/fsi: Add client driver register utilities
  drivers/fsi: Document FSI master sysfs files in ABI
  drivers/fsi: Add GPIO based FSI master
  drivers/fsi: Add SCOM FSI client device driver
  drivers/fsi: Add hub master support

Jeremy Kerr (14):
  drivers/fsi: Add fsi master definition
  drivers/fsi: Add slave definition
  drivers/fsi: Add empty master scan
  drivers/fsi: Add crc4 helpers
  drivers/fsi: Add slave & master read/write APIs
  drivers/fsi: Implement slave initialisation
  drivers/fsi: scan slaves & register devices
  drivers/fsi: Add device read/write/peek API
  drivers/fsi: Add sysfs files for FSI master & slave accesses
  drivers/fsi: expose direct-access slave API
  drivers/fsi: Add tracepoints for low-level operations
  drivers/fsi: Add error handling for slave communication errors
  drivers/fsi/gpio: Add tracepoints for GPIO master
  drivers/fsi: Use asynchronous slave mode

 Documentation/ABI/testing/sysfs-bus-fsi            |   6 +
 .../devicetree/bindings/fsi/fsi-master-gpio.txt    |  24 +
 drivers/fsi/Kconfig                                |  26 +
 drivers/fsi/Makefile                               |   3 +
 drivers/fsi/fsi-core.c                             | 835 +++++++++++++++++++++
 drivers/fsi/fsi-master-gpio.c                      | 620 +++++++++++++++
 drivers/fsi/fsi-master-hub.c                       | 327 ++++++++
 drivers/fsi/fsi-master.h                           |  64 ++
 drivers/fsi/fsi-scom.c                             | 263 +++++++
 include/linux/fsi.h                                |  35 +-
 include/trace/events/fsi.h                         | 127 ++++
 include/trace/events/fsi_master_gpio.h             |  68 ++
 12 files changed, 2397 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-fsi
 create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
 create mode 100644 drivers/fsi/fsi-master-gpio.c
 create mode 100644 drivers/fsi/fsi-master-hub.c
 create mode 100644 drivers/fsi/fsi-master.h
 create mode 100644 drivers/fsi/fsi-scom.c
 create mode 100644 include/trace/events/fsi.h
 create mode 100644 include/trace/events/fsi_master_gpio.h

-- 
1.8.2.2

--
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] clk/axs10x: introduce AXS10X pll driver
From: Stephen Boyd @ 2017-04-05  1:35 UTC (permalink / raw)
  To: Vlad Zakharov
  Cc: linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Jose Abreu, Michael Turquette,
	Mark Rutland, Rob Herring
In-Reply-To: <1487682670-4164-1-git-send-email-vzakhar-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On 02/21, Vlad Zakharov wrote:
> diff --git a/Documentation/devicetree/bindings/clock/snps,pll-clock.txt b/Documentation/devicetree/bindings/clock/snps,pll-clock.txt
> new file mode 100644
> index 0000000..5706246
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/snps,pll-clock.txt
> @@ -0,0 +1,28 @@
> +Binding for the AXS10X Generic PLL clock
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible: should be "snps,axs10x-<name>-pll-clock"
> +  "snps,axs10x-arc-pll-clock"
> +  "snps,axs10x-pgu-pll-clock"
> +- reg: should always contain 2 pairs address - length: first for PLL config
> +registers and second for corresponding LOCK CGU register.
> +- clocks: shall be the input parent clock phandle for the PLL.
> +- #clock-cells: from common clock binding; Should always be set to 0.
> +
> +Example:
> +	input-clk: input-clk {
> +		clock-frequency = <33333333>;
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +
> +	core-clk: core-clk@80 {
> +		compatible = "snps,axs10x-arc-pll-clock";
> +		reg = <0x80 0x10 0x100 0x10>;

Can you add the braces around register pairs in the example?
Makes it clearer with a quick glance.

> +		#clock-cells = <0>;
> +		clocks = <&input-clk>;
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3960e7f..5805833 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11910,6 +11910,12 @@ F:	arch/arc/plat-axs10x
>  F:	arch/arc/boot/dts/ax*
>  F:	Documentation/devicetree/bindings/arc/axs10*
>  
> +SYNOPSYS ARC SDP clock driver

This also includes the existing driver there. Jose can ack this?

> +M:	Vlad Zakharov <vzakhar-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> +S:	Supported
> +F:	drivers/clk/axs10x/*
> +F:	Documentation/devicetree/bindings/clock/snps,pll-clock.txt
> +
>  SYSTEM CONFIGURATION (SYSCON)
>  M:	Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>  M:	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> diff --git a/drivers/clk/axs10x/Makefile b/drivers/clk/axs10x/Makefile
> index 01996b8..d747dea 100644
> --- a/drivers/clk/axs10x/Makefile
> +++ b/drivers/clk/axs10x/Makefile
> @@ -1 +1,2 @@
>  obj-y += i2s_pll_clock.o
> +obj-y += pll_clock.o
> diff --git a/drivers/clk/axs10x/pll_clock.c b/drivers/clk/axs10x/pll_clock.c
> new file mode 100644
> index 0000000..784a0a2
> --- /dev/null
> +++ b/drivers/clk/axs10x/pll_clock.c
> @@ -0,0 +1,384 @@
> +/*
> + * Synopsys AXS10X SDP Generic PLL clock driver
> + *
> + * Copyright (C) 2017 Synopsys
> + *
> + * 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.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/device.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +/* PLL registers addresses */
> +#define PLL_REG_IDIV	0x0
> +#define PLL_REG_FBDIV	0x4
> +#define PLL_REG_ODIV	0x8
> +
> +/*
> + * Bit fields of the PLL IDIV/FBDIV/ODIV registers:
> + *  ________________________________________________________________________
> + * |31                15|    14    |   13   |  12  |11         6|5         0|
> + * |-------RESRVED------|-NOUPDATE-|-BYPASS-|-EDGE-|--HIGHTIME--|--LOWTIME--|
> + * |____________________|__________|________|______|____________|___________|
> + *
> + * Following macros detirmine the way of access to these registers

s/detirmine/determine/

> + * They should be set up only using the macros.
> + * reg should be and uint32_t variable.

s/and/an/?

> + */
> +
> +#define PLL_REG_GET_LOW(reg)			\
> +	(((reg) & (0x3F << 0)) >> 0)
> +#define PLL_REG_GET_HIGH(reg)			\
> +	(((reg) & (0x3F << 6)) >> 6)
> +#define PLL_REG_GET_EDGE(reg)			\
> +	(((reg) & (BIT(12))) ? 1 : 0)
> +#define PLL_REG_GET_BYPASS(reg)			\
> +	(((reg) & (BIT(13))) ? 1 : 0)
> +#define PLL_REG_GET_NOUPD(reg)			\
> +	(((reg) & (BIT(14))) ? 1 : 0)
> +#define PLL_REG_GET_PAD(reg)			\
> +	(((reg) & (0x1FFFF << 15)) >> 15)
> +
> +#define PLL_REG_SET_LOW(reg, value)		\
> +	{ reg |= (((value) & 0x3F) << 0); }
> +#define PLL_REG_SET_HIGH(reg, value)	\
> +	{ reg |= (((value) & 0x3F) << 6); }
> +#define PLL_REG_SET_EDGE(reg, value)	\
> +	{ reg |= (((value) & 0x01) << 12); }
> +#define PLL_REG_SET_BYPASS(reg, value)	\
> +	{ reg |= (((value) & 0x01) << 13); }
> +#define PLL_REG_SET_NOUPD(reg, value)	\
> +	{ reg |= (((value) & 0x01) << 14); }
> +#define PLL_REG_SET_PAD(reg, value)		\
> +	{ reg |= (((value) & 0x1FFFF) << 15); }
> +
> +#define PLL_LOCK	0x1
> +#define PLL_MAX_LOCK_TIME 100 /* 100 us */
> +
> +struct pll_cfg {
> +	u32 rate;
> +	u32 idiv;
> +	u32 fbdiv;
> +	u32 odiv;
> +};
> +
> +struct pll_of_table {
> +	unsigned long prate;
> +	struct pll_cfg *pll_cfg_table;

const?

> +};
> +
> +struct pll_of_data {
> +	struct pll_of_table *pll_table;
> +};

Why the structure for another structure pointer? Just use
pll_of_table for now?

> +
> +static struct pll_of_data pgu_pll_data = {

const?

> +	.pll_table = (struct pll_of_table []){
> +		{
> +			.prate = 27000000,

Can this be another clk in the framework instead of hardcoding
the parent rate?

> +			.pll_cfg_table = (struct pll_cfg []){
> +				{ 25200000, 1, 84, 90 },
> +				{ 50000000, 1, 100, 54 },
> +				{ 74250000, 1, 44, 16 },
> +				{ },
> +			},
> +		},
> +		/* Used as list limiter */
> +		{ },

There's only ever one, so I'm confused why we're making a list.

> +	},
> +};
> +
> +static struct pll_of_data arc_pll_data = {

const?

> +	.pll_table = (struct pll_of_table []){
> +		{
> +			.prate = 33333333,
> +			.pll_cfg_table = (struct pll_cfg []){
> +				{ 33333333,  1, 1,  1 },
> +				{ 50000000,  1, 30, 20 },
> +				{ 75000000,  2, 45, 10 },
> +				{ 90000000,  2, 54, 10 },
> +				{ 100000000, 1, 30, 10 },
> +				{ 125000000, 2, 45, 6 },
> +				{ },
> +			},
> +		},
> +		/* Used as list limiter */
> +		{ },
> +	},
> +};
> +
> +struct pll_clk {
> +	void __iomem *base;
> +	void __iomem *lock;
> +	const struct pll_of_data *pll_data;
> +	struct clk_hw hw;
> +	struct device *dev;
> +};
> +
> +static inline void pll_write(struct pll_clk *clk, unsigned int reg,
> +		unsigned int val)
> +{
> +	iowrite32(val, clk->base + reg);
> +}
> +
> +static inline u32 pll_read(struct pll_clk *clk,
> +		unsigned int reg)

reg can go on the same line as clk?

> +{
> +	return ioread32(clk->base + reg);
> +}
> +
> +static inline struct pll_clk *to_pll_clk(struct clk_hw *hw)
> +{
> +	return container_of(hw, struct pll_clk, hw);
> +}
> +
> +static inline u32 div_get_value(unsigned int reg)
> +{
> +	if (PLL_REG_GET_BYPASS(reg))
> +		return 1;
> +
> +	return (PLL_REG_GET_HIGH(reg) + PLL_REG_GET_LOW(reg));

Useless parenthesis.

> +}
> +
> +static inline u32 encode_div(unsigned int id, int upd)
> +{
> +	uint32_t div = 0;
> +
> +	PLL_REG_SET_LOW(div, (id%2 == 0) ? id >> 1 : (id >> 1) + 1);
> +	PLL_REG_SET_HIGH(div, id >> 1);
> +	PLL_REG_SET_EDGE(div, id%2);
> +	PLL_REG_SET_BYPASS(div, id == 1 ? 1 : 0);
> +	PLL_REG_SET_NOUPD(div, !upd);
> +
> +	return div;
> +}
> +
> +static const struct pll_cfg *pll_get_cfg(unsigned long prate,
> +		const struct pll_of_table *pll_table)
> +{
> +	int i;
> +
> +	for (i = 0; pll_table[i].prate != 0; i++)
> +		if (pll_table[i].prate == prate)
> +			return pll_table[i].pll_cfg_table;
> +
> +	return NULL;
> +}
> +
> +static unsigned long pll_recalc_rate(struct clk_hw *hw,
> +			unsigned long parent_rate)
> +{
> +	u64 rate;
> +	u32 idiv, fbdiv, odiv;
> +	struct pll_clk *clk = to_pll_clk(hw);
> +
> +	idiv = div_get_value(pll_read(clk, PLL_REG_IDIV));
> +	fbdiv = div_get_value(pll_read(clk, PLL_REG_FBDIV));
> +	odiv = div_get_value(pll_read(clk, PLL_REG_ODIV));
> +
> +	rate = (u64)parent_rate * fbdiv;
> +	do_div(rate, idiv * odiv);
> +
> +	return (unsigned long)rate;

Useless cast?

> +}
> +
> +static long pll_round_rate(struct clk_hw *hw, unsigned long rate,
> +			unsigned long *prate)
> +{
> +	int i;
> +	long best_rate;
> +	struct pll_clk *clk = to_pll_clk(hw);
> +	const struct pll_cfg *pll_cfg = pll_get_cfg(*prate,
> +			clk->pll_data->pll_table);
> +
> +	if (!pll_cfg) {
> +		dev_err(clk->dev, "invalid parent rate=%ld\n", *prate);
> +		return -EINVAL;
> +	}
> +
> +	if (pll_cfg[0].rate == 0)
> +		return -EINVAL;
> +
> +	best_rate = pll_cfg[0].rate;
> +
> +	for (i = 1; pll_cfg[i].rate != 0; i++) {
> +		if (abs(rate - pll_cfg[i].rate) < abs(rate - best_rate))
> +			best_rate = pll_cfg[i].rate;
> +	}
> +
> +	return best_rate;
> +}
> +
> +static int pll_set_rate(struct clk_hw *hw, unsigned long rate,
> +			unsigned long parent_rate)
> +{
> +	int i;
> +	struct pll_clk *clk = to_pll_clk(hw);
> +	const struct pll_cfg *pll_cfg = pll_get_cfg(parent_rate,
> +			clk->pll_data->pll_table);
> +
> +	if (!pll_cfg) {
> +		dev_err(clk->dev, "invalid parent rate=%ld\n", parent_rate);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; pll_cfg[i].rate != 0; i++) {
> +		if (pll_cfg[i].rate == rate) {
> +			pll_write(clk, PLL_REG_IDIV,
> +					encode_div(pll_cfg[i].idiv, 0));
> +			pll_write(clk, PLL_REG_FBDIV,
> +					encode_div(pll_cfg[i].fbdiv, 0));
> +			pll_write(clk, PLL_REG_ODIV,
> +					encode_div(pll_cfg[i].odiv, 1));
> +
> +			/*
> +			 * Wait until CGU relocks.
> +			 * If after timeout CGU is unlocked yet return error
> +			 */
> +			udelay(PLL_MAX_LOCK_TIME);
> +			if (ioread32(clk->lock) & PLL_LOCK)
> +				return 0;
> +			else
> +				return -ETIMEDOUT;

Can just be a return without the else part.

> +		}
> +	}
> +
> +	dev_err(clk->dev, "invalid rate=%ld, parent_rate=%ld\n", rate,
> +			parent_rate);
> +	return -EINVAL;
> +}
> +
> +static const struct clk_ops pll_ops = {
> +	.recalc_rate = pll_recalc_rate,
> +	.round_rate = pll_round_rate,
> +	.set_rate = pll_set_rate,
> +};
> +
> +static int pll_clk_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const char *parent_name;
> +	struct clk *clk;
> +	struct pll_clk *pll_clk;
> +	struct resource *mem;
> +	struct clk_init_data init = { };
> +
> +	pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL);
> +	if (!pll_clk)
> +		return -ENOMEM;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	pll_clk->base = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(pll_clk->base))
> +		return PTR_ERR(pll_clk->base);
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	pll_clk->lock = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(pll_clk->lock))
> +		return PTR_ERR(pll_clk->base);
> +
> +	init.name = dev->of_node->name;
> +	init.ops = &pll_ops;
> +	parent_name = of_clk_get_parent_name(dev->of_node, 0);
> +	init.parent_names = &parent_name;
> +	init.num_parents = 1;
> +	pll_clk->hw.init = &init;
> +	pll_clk->dev = dev;
> +	pll_clk->pll_data = of_device_get_match_data(dev);
> +
> +	if (!pll_clk->pll_data) {
> +		dev_err(dev, "No OF match data provided\n");
> +			return -EINVAL;
> +	}
> +
> +	clk = devm_clk_register(dev, &pll_clk->hw);
> +	if (IS_ERR(clk)) {
> +		dev_err(dev, "failed to register %s clock (%ld)\n",
> +				init.name, PTR_ERR(clk));
> +		return PTR_ERR(clk);
> +	}
> +
> +	return of_clk_add_provider(dev->of_node, of_clk_src_simple_get, clk);
> +}
> +
> +static int pll_clk_remove(struct platform_device *pdev)
> +{
> +	of_clk_del_provider(pdev->dev.of_node);
> +	return 0;
> +}
> +
> +static void __init of_pll_clk_setup(struct device_node *node)
> +{
> +	const char *parent_name;
> +	struct clk *clk;
> +	struct pll_clk *pll_clk;
> +	struct clk_init_data init = { };
> +
> +	pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL);
> +	if (!pll_clk)
> +		return;
> +
> +	pll_clk->base = of_iomap(node, 0);
> +	if (!pll_clk->base) {
> +		pr_err("failed to map pll div registers\n");
> +		iounmap(pll_clk->base);
> +		return;
> +	}
> +
> +	pll_clk->lock = of_iomap(node, 1);
> +	if (!pll_clk->lock) {
> +		pr_err("failed to map pll lock register\n");
> +		iounmap(pll_clk->lock);
> +		return;
> +	}
> +
> +	init.name = node->name;
> +	init.ops = &pll_ops;
> +	parent_name = of_clk_get_parent_name(node, 0);
> +	init.parent_names = &parent_name;
> +	init.num_parents = parent_name ? 1 : 0;
> +	pll_clk->hw.init = &init;
> +	pll_clk->pll_data = &arc_pll_data;
> +
> +	clk = clk_register(NULL, &pll_clk->hw);
> +	if (IS_ERR(clk)) {
> +		pr_err("failed to register %s clock (%ld)\n",
> +				node->name, PTR_ERR(clk));
> +		kfree(pll_clk);
> +		return;
> +	}
> +
> +	of_clk_add_provider(node, of_clk_src_simple_get, clk);

Can you please use the clk_hw based provider and clk registration
functions?

> +}
> +
> +CLK_OF_DECLARE(axs10x_pll_clock, "snps,axs10x-arc-pll-clock", of_pll_clk_setup);

Does this need to be CLK_OF_DECLARE_DRIVER? I mean does the
driver need to probe and also have this of declare happen? Is the
PLL special and needs to be used for the timers?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Christopher Bostic @ 2017-04-05  1:24 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Joel Stanley
  Cc: Rob Herring, Mark Rutland, Russell King,
	rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
	Greg KH, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Linux Kernel Mailing List, Andrew Jeffery, Alistair Popple,
	Edward A . James, Jeremy Kerr
In-Reply-To: <1491344360.4166.68.camel-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>



On 4/4/17 5:19 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-04 at 12:32 -0500, Christopher Bostic wrote:
>> Agreed that there is room for improvement.   I intend to look further
>> into your suggestions from here and our private conversation on the
>> matter and make changes as appropriate.  I have an open issue to track
>> this.  As it exists in this patch reads/writes from master to slave
>> fundamentally work.
> My understanding is they "seem to work if you get lucky with the timing
> and fall apart under load". Or did I hear wrong ?
>
>>   Given the pervasiveness and time to fully evaluate
>> and test any protocol updates I intend address this in the near future
>> with a separate follow on patch.
> Please try the simple change I proposed in my email. It's a 4 or 5
> lines change max to your clock_toggle function and how it's called in
> send and receive. It should be trivial to check if things still "seem
> to work" to begin with.
>
> Do you have some kind of test mechanism that hammers the FSI
> continuously ? Such as doing a series of putmemproc/getmemproc &
> checking the values ?
>
> Then you can run that while hammering the LPC bus and generally putting
> the BMC under load and you'll quickly see if it's reliable or not.

Hi Ben,

I do now have a mechanism that puts the bus under heavy load.  I'll look 
into your changes tomorrow.

Thanks,
Chris
>
> Cheers,
> Ben.
>

--
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 V10 00/12] IOMMU probe deferral support
From: Rob Herring @ 2017-04-05  1:23 UTC (permalink / raw)
  To: Sricharan R
  Cc: Robin Murphy, Will Deacon, Joerg Roedel, Lorenzo Pieralisi,
	Linux IOMMU, linux-arm-kernel@lists.infradead.org, linux-arm-msm,
	Marek Szyprowski, bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-acpi@vger.kernel.org, Tomasz Nowicki, Hanjun Guo,
	Sinan Kaya, Frank Rowand, devicetree@vger.kernel.org,
	linux-kernel
In-Reply-To: <1491301105-5274-1-git-send-email-sricharan@codeaurora.org>

On Tue, Apr 4, 2017 at 5:18 AM, Sricharan R <sricharan@codeaurora.org> wrote:
> This series calls the dma ops configuration for the devices
> at a generic place so that it works for all busses.
> The dma_configure_ops for a device is now called during
> the device_attach callback just before the probe of the
> bus/driver is called. Similarly dma_deconfigure is called during
> device/driver_detach path.

For patches 3, 4, 6, 7, 8:

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 1/2] extcon: cros-ec: Add extcon-cros-ec driver to support display out.
From: Chanwoo Choi @ 2017-04-05  1:21 UTC (permalink / raw)
  To: Enric Balletbo i Serra, MyungJoo Ham, Rob Herring
  Cc: Lee Jones, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Benson Leung
In-Reply-To: <58B7C9BF.1040708-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Hi Enric,

On 2017년 03월 02일 16:29, Chanwoo Choi wrote:
> Hi,
> 
> On 2017년 03월 01일 20:19, Enric Balletbo i Serra wrote:
>> From: Benson Leung <bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>
>> This is the driver for the USB Type C cable detection mechanism
>> built into the ChromeOS Embedded Controller on systems that
>> have USB Type-C ports.
>>
>> At present, this allows for the presence of display out, but in
>> future, it may also be used to notify host and device type cables
>> and the presence of power.
>>
>> Signed-off-by: Benson Leung <bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
>> ---
>> Changes since v1:
>>  Requested by Chanwoo Choi
>>  - Rename files changing _ for -
>>  - Remove the unneeded blank line on bottom of header.
>>  - Remove kobject.h and cros_ec_commands.h includes.
>>  - Remove the debug message as is not necessary.
>>  - Use the tab for indentation instead of space for if sentence.
>>  - Define each variable on different lines when the variables should be
>>    initialized.
>>  - Remove EXTCON_USB and EXTCON_USB_HOST as are not really used for now.
>>  - Add one blank line to split out between state and property setting.
>>  - Add the author information (header and module)
>>
>>  Enric Balletbo
>>  - As Rob suggested to rename the compatible name to something indicating that
>>    is USB Type C related I also renamed the file names, extcon-cros-ec ->
>>    extcon-usbc-cros-ec, I think it's more clear.
>>
>>  drivers/extcon/Kconfig               |   7 +
>>  drivers/extcon/Makefile              |   1 +
>>  drivers/extcon/extcon-usbc-cros-ec.c | 415 +++++++++++++++++++++++++++++++++++
>>  include/linux/mfd/cros_ec_commands.h |  75 +++++++
>>  4 files changed, 498 insertions(+)
>>  create mode 100644 drivers/extcon/extcon-usbc-cros-ec.c
>>
> 
> Looks good to me.
> Acked-by: Chanwoo Choi <cw00.chio-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> I think this patch should be handled with patches[1].
> [1] https://lkml.org/lkml/2017/2/14/655
> 
> I think that one maintainer among following subsystems
> (mfd, chrome h/w platform, rtc and extcon)
> will apply their git repository, and then one maintainer
> will send the pull request of immutable branch for these patches.
> 

As I mentioned, these patch should be handled with related patches[1].
[1] https://lkml.org/lkml/2017/2/14/655

So, I can't apply these patch on extcon git because there is a merge conflict
and we should handle these patches with immutable branch between subsystem maintainer.

The v4.11-rc5 was released, if you want to apply this patch to the v4.12-rc1,
please take care of these patches.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics
--
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

* [PATCH linux v7 2/2] drivers: hwmon: Support for ASPEED PWM/Fan tach
From: Jaghathiswari Rankappagounder Natarajan @ 2017-04-05  0:52 UTC (permalink / raw)
  To: joel, jdelvare, linux, linux-hwmon, linux-kernel, openbmc, corbet,
	linux-doc, robh+dt, mark.rutland, devicetree
  Cc: Jaghathiswari Rankappagounder Natarajan
In-Reply-To: <20170405005241.8794-1-jaghu@google.com>

The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
The ASPEED AST2400/2500 Fan tach controller supports 16 tachometer
inputs.
The device driver matches on the device tree node. The configuration
values are read from the device tree and written to the respective
registers.
The driver provides a sysfs entries through which the user can
configure the duty-cycle value (ranging from 0 to 100 percent) and read
the fan tach rpm value.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 v7:
- Made corrections for the sparse errors

 v6:
- Corrected odd line breaks
- Changed upto to up to
- Dropped unrelated changes
- Removed struct and used regs pointer directly
- Made groups to be null terminated
- Made correction in calculation of val/raw_data
- Removed else after return
- Removed unnecessary continuation lines

 v5:
- Changed the driver to suit the changes in the device tree documentation

 v4:
- Modified this driver to suit the representation in the devicetree

 v3:
- Only sent out device tree documentation; did not send this driver

 v2:
- Used BIT()
- Used regmap
- Avoided division when raw data is 0
- Removed empty lines between declaration
- Removed macros; Used two attribute groups and used is_visible callback
- Returned error when properties are undefined
- Removed .owner field
- Used PTR_ERR_OR_ZERO
- Removed explicit of_node_put for child nodes

 Documentation/hwmon/aspeed-pwm-tacho |  22 +
 drivers/hwmon/Kconfig                |   9 +
 drivers/hwmon/Makefile               |   1 +
 drivers/hwmon/aspeed-pwm-tacho.c     | 835 +++++++++++++++++++++++++++++++++++
 4 files changed, 867 insertions(+)
 create mode 100644 Documentation/hwmon/aspeed-pwm-tacho
 create mode 100644 drivers/hwmon/aspeed-pwm-tacho.c

diff --git a/Documentation/hwmon/aspeed-pwm-tacho b/Documentation/hwmon/aspeed-pwm-tacho
new file mode 100644
index 000000000000..7cfb34977460
--- /dev/null
+++ b/Documentation/hwmon/aspeed-pwm-tacho
@@ -0,0 +1,22 @@
+Kernel driver aspeed-pwm-tacho
+==============================
+
+Supported chips:
+	ASPEED AST2400/2500
+
+Authors:
+	<jaghu@google.com>
+
+Description:
+------------
+This driver implements support for ASPEED AST2400/2500 PWM and Fan Tacho
+controller. The PWM controller supports upto 8 PWM outputs. The Fan tacho
+controller supports up to 16 tachometer inputs.
+
+The driver provides the following sensor accesses in sysfs:
+
+fanX_input	ro	provide current fan rotation value in RPM as reported
+			by the fan to the device.
+
+pwmX		rw	get or set PWM fan control value. This is an integer
+			value between 0(off) and 255(full speed).
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 45cef3d2c75c..757b5b0705bf 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -341,6 +341,15 @@ config SENSORS_ASB100
 	  This driver can also be built as a module.  If so, the module
 	  will be called asb100.

+config SENSORS_ASPEED
+	tristate "ASPEED AST2400/AST2500 PWM and Fan tach driver"
+	help
+	  This driver provides support for ASPEED AST2400/AST2500 PWM
+	  and Fan Tacho controllers.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called aspeed_pwm_tacho.
+
 config SENSORS_ATXP1
 	tristate "Attansic ATXP1 VID controller"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index aecf4ba17460..83025cc9bb45 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_SENSORS_ADT7475)	+= adt7475.o
 obj-$(CONFIG_SENSORS_APPLESMC)	+= applesmc.o
 obj-$(CONFIG_SENSORS_ARM_SCPI)	+= scpi-hwmon.o
 obj-$(CONFIG_SENSORS_ASC7621)	+= asc7621.o
+obj-$(CONFIG_SENSORS_ASPEED)	+= aspeed-pwm-tacho.o
 obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
 obj-$(CONFIG_SENSORS_CORETEMP)	+= coretemp.o
 obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
new file mode 100644
index 000000000000..48403a2115be
--- /dev/null
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -0,0 +1,835 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+#include <linux/regmap.h>
+
+/* ASPEED PWM & FAN Tach Register Definition */
+#define ASPEED_PTCR_CTRL		0x00
+#define ASPEED_PTCR_CLK_CTRL		0x04
+#define ASPEED_PTCR_DUTY0_CTRL		0x08
+#define ASPEED_PTCR_DUTY1_CTRL		0x0c
+#define ASPEED_PTCR_TYPEM_CTRL		0x10
+#define ASPEED_PTCR_TYPEM_CTRL1		0x14
+#define ASPEED_PTCR_TYPEN_CTRL		0x18
+#define ASPEED_PTCR_TYPEN_CTRL1		0x1c
+#define ASPEED_PTCR_TACH_SOURCE		0x20
+#define ASPEED_PTCR_TRIGGER		0x28
+#define ASPEED_PTCR_RESULT		0x2c
+#define ASPEED_PTCR_INTR_CTRL		0x30
+#define ASPEED_PTCR_INTR_STS		0x34
+#define ASPEED_PTCR_TYPEM_LIMIT		0x38
+#define ASPEED_PTCR_TYPEN_LIMIT		0x3C
+#define ASPEED_PTCR_CTRL_EXT		0x40
+#define ASPEED_PTCR_CLK_CTRL_EXT	0x44
+#define ASPEED_PTCR_DUTY2_CTRL		0x48
+#define ASPEED_PTCR_DUTY3_CTRL		0x4c
+#define ASPEED_PTCR_TYPEO_CTRL		0x50
+#define ASPEED_PTCR_TYPEO_CTRL1		0x54
+#define ASPEED_PTCR_TACH_SOURCE_EXT	0x60
+#define ASPEED_PTCR_TYPEO_LIMIT		0x78
+
+/* ASPEED_PTCR_CTRL : 0x00 - General Control Register */
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1	15
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2	6
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK	(BIT(7) | BIT(15))
+
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1	14
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2	5
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK	(BIT(6) | BIT(14))
+
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1	13
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2	4
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK	(BIT(5) | BIT(13))
+
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1	12
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2	3
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK	(BIT(4) | BIT(12))
+
+#define	ASPEED_PTCR_CTRL_FAN_NUM_EN(x)	BIT(16 + (x))
+
+#define	ASPEED_PTCR_CTRL_PWMD_EN	BIT(11)
+#define	ASPEED_PTCR_CTRL_PWMC_EN	BIT(10)
+#define	ASPEED_PTCR_CTRL_PWMB_EN	BIT(9)
+#define	ASPEED_PTCR_CTRL_PWMA_EN	BIT(8)
+
+#define	ASPEED_PTCR_CTRL_CLK_SRC	BIT(1)
+#define	ASPEED_PTCR_CTRL_CLK_EN		BIT(0)
+
+/* ASPEED_PTCR_CLK_CTRL : 0x04 - Clock Control Register */
+/* TYPE N */
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_MASK		GENMASK(31, 16)
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT		24
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_H		20
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_L		16
+/* TYPE M */
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_MASK         GENMASK(15, 0)
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT		8
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_H		4
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_L		0
+
+/*
+ * ASPEED_PTCR_DUTY_CTRL/1/2/3 : 0x08/0x0C/0x48/0x4C - PWM-FAN duty control
+ * 0/1/2/3 register
+ */
+#define DUTY_CTRL_PWM2_FALL_POINT	24
+#define DUTY_CTRL_PWM2_RISE_POINT	16
+#define DUTY_CTRL_PWM2_RISE_FALL_MASK	GENMASK(31, 16)
+#define DUTY_CTRL_PWM1_FALL_POINT	8
+#define DUTY_CTRL_PWM1_RISE_POINT	0
+#define DUTY_CTRL_PWM1_RISE_FALL_MASK   GENMASK(15, 0)
+
+/* ASPEED_PTCR_TYPEM_CTRL : 0x10/0x18/0x50 - Type M/N/O Ctrl 0 Register */
+#define TYPE_CTRL_FAN_MASK		(GENMASK(5, 1) | GENMASK(31, 16))
+#define TYPE_CTRL_FAN1_MASK		GENMASK(31, 0)
+#define TYPE_CTRL_FAN_PERIOD		16
+#define TYPE_CTRL_FAN_MODE		4
+#define TYPE_CTRL_FAN_DIVISION		1
+#define TYPE_CTRL_FAN_TYPE_EN		1
+
+/* ASPEED_PTCR_TACH_SOURCE : 0x20/0x60 - Tach Source Register */
+/* bit [0,1] at 0x20, bit [2] at 0x60 */
+#define TACH_PWM_SOURCE_BIT01(x)	((x) * 2)
+#define TACH_PWM_SOURCE_BIT2(x)		((x) * 2)
+#define TACH_PWM_SOURCE_MASK_BIT01(x)	(0x3 << ((x) * 2))
+#define TACH_PWM_SOURCE_MASK_BIT2(x)	BIT((x) * 2)
+
+/* ASPEED_PTCR_RESULT : 0x2c - Result Register */
+#define RESULT_STATUS_MASK		BIT(31)
+#define RESULT_VALUE_MASK		0xfffff
+
+/* ASPEED_PTCR_CTRL_EXT : 0x40 - General Control Extension #1 Register */
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1	15
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2	6
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK	(BIT(7) | BIT(15))
+
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1	14
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2	5
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK	(BIT(6) | BIT(14))
+
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1	13
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2	4
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK	(BIT(5) | BIT(13))
+
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1	12
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2	3
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK	(BIT(4) | BIT(12))
+
+#define	ASPEED_PTCR_CTRL_PWMH_EN	BIT(11)
+#define	ASPEED_PTCR_CTRL_PWMG_EN	BIT(10)
+#define	ASPEED_PTCR_CTRL_PWMF_EN	BIT(9)
+#define	ASPEED_PTCR_CTRL_PWME_EN	BIT(8)
+
+/* ASPEED_PTCR_CLK_EXT_CTRL : 0x44 - Clock Control Extension #1 Register */
+/* TYPE O */
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_MASK         GENMASK(15, 0)
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT		8
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_H		4
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_L		0
+
+#define PWM_MAX 255
+
+#define M_PWM_DIV_H 0x00
+#define M_PWM_DIV_L 0x05
+#define M_PWM_PERIOD 0x5F
+#define M_TACH_CLK_DIV 0x00
+#define M_TACH_MODE 0x00
+#define M_TACH_UNIT 0x1000
+#define INIT_FAN_CTRL 0xFF
+
+struct aspeed_pwm_tacho_data {
+	struct regmap *regmap;
+	unsigned long clk_freq;
+	bool pwm_present[8];
+	bool fan_tach_present[16];
+	u8 type_pwm_clock_unit[3];
+	u8 type_pwm_clock_division_h[3];
+	u8 type_pwm_clock_division_l[3];
+	u8 type_fan_tach_clock_division[3];
+	u16 type_fan_tach_unit[3];
+	u8 pwm_port_type[8];
+	u8 pwm_port_fan_ctrl[8];
+	u8 fan_tach_ch_source[16];
+	const struct attribute_group *groups[3];
+};
+
+enum type { TYPEM, TYPEN, TYPEO };
+
+struct type_params {
+	u32 l_value;
+	u32 h_value;
+	u32 unit_value;
+	u32 clk_ctrl_mask;
+	u32 clk_ctrl_reg;
+	u32 ctrl_reg;
+	u32 ctrl_reg1;
+};
+
+static const struct type_params type_params[] = {
+	[TYPEM] = {
+		.l_value = ASPEED_PTCR_CLK_CTRL_TYPEM_L,
+		.h_value = ASPEED_PTCR_CLK_CTRL_TYPEM_H,
+		.unit_value = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT,
+		.clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_MASK,
+		.clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL,
+		.ctrl_reg = ASPEED_PTCR_TYPEM_CTRL,
+		.ctrl_reg1 = ASPEED_PTCR_TYPEM_CTRL1,
+	},
+	[TYPEN] = {
+		.l_value = ASPEED_PTCR_CLK_CTRL_TYPEN_L,
+		.h_value = ASPEED_PTCR_CLK_CTRL_TYPEN_H,
+		.unit_value = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT,
+		.clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_MASK,
+		.clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL,
+		.ctrl_reg = ASPEED_PTCR_TYPEN_CTRL,
+		.ctrl_reg1 = ASPEED_PTCR_TYPEN_CTRL1,
+	},
+	[TYPEO] = {
+		.l_value = ASPEED_PTCR_CLK_CTRL_TYPEO_L,
+		.h_value = ASPEED_PTCR_CLK_CTRL_TYPEO_H,
+		.unit_value = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT,
+		.clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_MASK,
+		.clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL_EXT,
+		.ctrl_reg = ASPEED_PTCR_TYPEO_CTRL,
+		.ctrl_reg1 = ASPEED_PTCR_TYPEO_CTRL1,
+	}
+};
+
+enum pwm_port { PWMA, PWMB, PWMC, PWMD, PWME, PWMF, PWMG, PWMH };
+
+struct pwm_port_params {
+	u32 pwm_en;
+	u32 ctrl_reg;
+	u32 type_part1;
+	u32 type_part2;
+	u32 type_mask;
+	u32 duty_ctrl_rise_point;
+	u32 duty_ctrl_fall_point;
+	u32 duty_ctrl_reg;
+	u32 duty_ctrl_rise_fall_mask;
+};
+
+static const struct pwm_port_params pwm_port_params[] = {
+	[PWMA] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMA_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY0_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+	},
+	[PWMB] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMB_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY0_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+	},
+	[PWMC] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMC_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY1_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+	},
+	[PWMD] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMD_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY1_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+	},
+	[PWME] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWME_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY2_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+	},
+	[PWMF] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMF_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY2_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+	},
+	[PWMG] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMG_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY3_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+	},
+	[PWMH] = {
+		.pwm_en = ASPEED_PTCR_CTRL_PWMH_EN,
+		.ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+		.type_part1 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1,
+		.type_part2 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2,
+		.type_mask = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK,
+		.duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+		.duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+		.duty_ctrl_reg = ASPEED_PTCR_DUTY3_CTRL,
+		.duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+	}
+};
+
+static int regmap_aspeed_pwm_tacho_reg_write(void *context, unsigned int reg,
+					     unsigned int val)
+{
+	void __iomem *regs = (void __iomem *)context;
+
+	writel(val, regs + reg);
+	return 0;
+}
+
+static int regmap_aspeed_pwm_tacho_reg_read(void *context, unsigned int reg,
+					    unsigned int *val)
+{
+	void __iomem *regs = (void __iomem *)context;
+
+	*val = readl(regs + reg);
+	return 0;
+}
+
+static const struct regmap_config aspeed_pwm_tacho_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = ASPEED_PTCR_TYPEO_LIMIT,
+	.reg_write = regmap_aspeed_pwm_tacho_reg_write,
+	.reg_read = regmap_aspeed_pwm_tacho_reg_read,
+	.fast_io = true,
+};
+
+static void aspeed_set_clock_enable(struct regmap *regmap, bool val)
+{
+	regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
+			   ASPEED_PTCR_CTRL_CLK_EN,
+			   val ? ASPEED_PTCR_CTRL_CLK_EN : 0);
+}
+
+static void aspeed_set_clock_source(struct regmap *regmap, int val)
+{
+	regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
+			   ASPEED_PTCR_CTRL_CLK_SRC,
+			   val ? ASPEED_PTCR_CTRL_CLK_SRC : 0);
+}
+
+static void aspeed_set_pwm_clock_values(struct regmap *regmap, u8 type,
+					u8 div_high, u8 div_low, u8 unit)
+{
+	u32 reg_value = ((div_high << type_params[type].h_value) |
+			 (div_low << type_params[type].l_value) |
+			 (unit << type_params[type].unit_value));
+
+	regmap_update_bits(regmap, type_params[type].clk_ctrl_reg,
+			   type_params[type].clk_ctrl_mask, reg_value);
+}
+
+static void aspeed_set_pwm_port_enable(struct regmap *regmap, u8 pwm_port,
+				       bool enable)
+{
+	regmap_update_bits(regmap, pwm_port_params[pwm_port].ctrl_reg,
+			   pwm_port_params[pwm_port].pwm_en,
+			   enable ? pwm_port_params[pwm_port].pwm_en : 0);
+}
+
+static void aspeed_set_pwm_port_type(struct regmap *regmap,
+				     u8 pwm_port, u8 type)
+{
+	u32 reg_value = (type & 0x1) << pwm_port_params[pwm_port].type_part1;
+
+	reg_value |= (type & 0x2) << pwm_port_params[pwm_port].type_part2;
+
+	regmap_update_bits(regmap, pwm_port_params[pwm_port].ctrl_reg,
+			   pwm_port_params[pwm_port].type_mask, reg_value);
+}
+
+static void aspeed_set_pwm_port_duty_rising_falling(struct regmap *regmap,
+						    u8 pwm_port, u8 rising,
+						    u8 falling)
+{
+	u32 reg_value = (rising <<
+			 pwm_port_params[pwm_port].duty_ctrl_rise_point);
+	reg_value |= (falling <<
+		      pwm_port_params[pwm_port].duty_ctrl_fall_point);
+
+	regmap_update_bits(regmap, pwm_port_params[pwm_port].duty_ctrl_reg,
+			   pwm_port_params[pwm_port].duty_ctrl_rise_fall_mask,
+			   reg_value);
+}
+
+static void aspeed_set_tacho_type_enable(struct regmap *regmap, u8 type,
+					 bool enable)
+{
+	regmap_update_bits(regmap, type_params[type].ctrl_reg,
+			   TYPE_CTRL_FAN_TYPE_EN,
+			   enable ? TYPE_CTRL_FAN_TYPE_EN : 0);
+}
+
+static void aspeed_set_tacho_type_values(struct regmap *regmap, u8 type,
+					 u8 mode, u16 unit, u8 division)
+{
+	u32 reg_value = ((mode << TYPE_CTRL_FAN_MODE) |
+			 (unit << TYPE_CTRL_FAN_PERIOD) |
+			 (division << TYPE_CTRL_FAN_DIVISION));
+
+	regmap_update_bits(regmap, type_params[type].ctrl_reg,
+			   TYPE_CTRL_FAN_MASK, reg_value);
+	regmap_update_bits(regmap, type_params[type].ctrl_reg1,
+			   TYPE_CTRL_FAN1_MASK, unit << 16);
+}
+
+static void aspeed_set_fan_tach_ch_enable(struct regmap *regmap, u8 fan_tach_ch,
+					  bool enable)
+{
+	regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
+			   ASPEED_PTCR_CTRL_FAN_NUM_EN(fan_tach_ch),
+			   enable ?
+			   ASPEED_PTCR_CTRL_FAN_NUM_EN(fan_tach_ch) : 0);
+}
+
+static void aspeed_set_fan_tach_ch_source(struct regmap *regmap, u8 fan_tach_ch,
+					  u8 fan_tach_ch_source)
+{
+	u32 reg_value1 = ((fan_tach_ch_source & 0x3) <<
+			  TACH_PWM_SOURCE_BIT01(fan_tach_ch));
+	u32 reg_value2 = (((fan_tach_ch_source & 0x4) >> 2) <<
+			  TACH_PWM_SOURCE_BIT2(fan_tach_ch));
+
+	regmap_update_bits(regmap, ASPEED_PTCR_TACH_SOURCE,
+			   TACH_PWM_SOURCE_MASK_BIT01(fan_tach_ch),
+			   reg_value1);
+
+	regmap_update_bits(regmap, ASPEED_PTCR_TACH_SOURCE_EXT,
+			   TACH_PWM_SOURCE_MASK_BIT2(fan_tach_ch),
+			   reg_value2);
+}
+
+static void aspeed_set_pwm_port_fan_ctrl(struct aspeed_pwm_tacho_data *priv,
+					 u8 index, u8 fan_ctrl)
+{
+	u16 period, dc_time_on;
+
+	period = priv->type_pwm_clock_unit[priv->pwm_port_type[index]];
+	period += 1;
+	dc_time_on = (fan_ctrl * period) / PWM_MAX;
+
+	if (dc_time_on == 0) {
+		aspeed_set_pwm_port_enable(priv->regmap, index, false);
+	} else {
+		if (dc_time_on == period)
+			dc_time_on = 0;
+
+		aspeed_set_pwm_port_duty_rising_falling(priv->regmap, index, 0,
+							dc_time_on);
+		aspeed_set_pwm_port_enable(priv->regmap, index, true);
+	}
+}
+
+static u32 aspeed_get_fan_tach_ch_measure_period(struct aspeed_pwm_tacho_data
+						 *priv, u8 type)
+{
+	u32 clk;
+	u16 tacho_unit;
+	u8 clk_unit, div_h, div_l, tacho_div;
+
+	clk = priv->clk_freq;
+	clk_unit = priv->type_pwm_clock_unit[type];
+	div_h = priv->type_pwm_clock_division_h[type];
+	div_h = 0x1 << div_h;
+	div_l = priv->type_pwm_clock_division_l[type];
+	if (div_l == 0)
+		div_l = 1;
+	else
+		div_l = div_l * 2;
+
+	tacho_unit = priv->type_fan_tach_unit[type];
+	tacho_div = priv->type_fan_tach_clock_division[type];
+
+	tacho_div = 0x4 << (tacho_div * 2);
+	return clk / (clk_unit * div_h * div_l * tacho_div * tacho_unit);
+}
+
+static u32 aspeed_get_fan_tach_ch_rpm(struct aspeed_pwm_tacho_data *priv,
+				      u8 fan_tach_ch)
+{
+	u32 raw_data, tach_div, clk_source, sec, val;
+	u8 fan_tach_ch_source, type;
+
+	regmap_write(priv->regmap, ASPEED_PTCR_TRIGGER, 0);
+	regmap_write(priv->regmap, ASPEED_PTCR_TRIGGER, 0x1 << fan_tach_ch);
+
+	fan_tach_ch_source = priv->fan_tach_ch_source[fan_tach_ch];
+	type = priv->pwm_port_type[fan_tach_ch_source];
+
+	sec = (1000 / aspeed_get_fan_tach_ch_measure_period(priv, type));
+	msleep(sec);
+
+	regmap_read(priv->regmap, ASPEED_PTCR_RESULT, &val);
+	raw_data = val & RESULT_VALUE_MASK;
+	tach_div = priv->type_fan_tach_clock_division[type];
+	tach_div = 0x4 << (tach_div * 2);
+	clk_source = priv->clk_freq;
+
+	if (raw_data == 0)
+		return 0;
+
+	return (clk_source * 60) / (2 * raw_data * tach_div);
+}
+
+static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
+		       const char *buf, size_t count)
+{
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	int index = sensor_attr->index;
+	int ret;
+	struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+	long fan_ctrl;
+
+	ret = kstrtol(buf, 10, &fan_ctrl);
+	if (ret != 0)
+		return ret;
+
+	if (fan_ctrl < 0 || fan_ctrl > PWM_MAX)
+		return -EINVAL;
+
+	if (priv->pwm_port_fan_ctrl[index] == fan_ctrl)
+		return count;
+
+	priv->pwm_port_fan_ctrl[index] = fan_ctrl;
+	aspeed_set_pwm_port_fan_ctrl(priv, index, fan_ctrl);
+
+	return count;
+}
+
+static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	int index = sensor_attr->index;
+	struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", priv->pwm_port_fan_ctrl[index]);
+}
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	int index = sensor_attr->index;
+	u32 rpm;
+	struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+	rpm = aspeed_get_fan_tach_ch_rpm(priv, index);
+
+	return sprintf(buf, "%u\n", rpm);
+}
+
+static umode_t pwm_is_visible(struct kobject *kobj,
+			      struct attribute *a, int index)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+	if (!priv->pwm_present[index])
+		return 0;
+	return a->mode;
+}
+
+static umode_t fan_dev_is_visible(struct kobject *kobj,
+				  struct attribute *a, int index)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+	if (!priv->fan_tach_present[index])
+		return 0;
+	return a->mode;
+}
+
+static SENSOR_DEVICE_ATTR(pwm0, 0644,
+			show_pwm, set_pwm, 0);
+static SENSOR_DEVICE_ATTR(pwm1, 0644,
+			show_pwm, set_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2, 0644,
+			show_pwm, set_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3, 0644,
+			show_pwm, set_pwm, 3);
+static SENSOR_DEVICE_ATTR(pwm4, 0644,
+			show_pwm, set_pwm, 4);
+static SENSOR_DEVICE_ATTR(pwm5, 0644,
+			show_pwm, set_pwm, 5);
+static SENSOR_DEVICE_ATTR(pwm6, 0644,
+			show_pwm, set_pwm, 6);
+static SENSOR_DEVICE_ATTR(pwm7, 0644,
+			show_pwm, set_pwm, 7);
+static struct attribute *pwm_dev_attrs[] = {
+	&sensor_dev_attr_pwm0.dev_attr.attr,
+	&sensor_dev_attr_pwm1.dev_attr.attr,
+	&sensor_dev_attr_pwm2.dev_attr.attr,
+	&sensor_dev_attr_pwm3.dev_attr.attr,
+	&sensor_dev_attr_pwm4.dev_attr.attr,
+	&sensor_dev_attr_pwm5.dev_attr.attr,
+	&sensor_dev_attr_pwm6.dev_attr.attr,
+	&sensor_dev_attr_pwm7.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group pwm_dev_group = {
+	.attrs = pwm_dev_attrs,
+	.is_visible = pwm_is_visible,
+};
+
+static SENSOR_DEVICE_ATTR(fan0_input, 0444,
+		show_rpm, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, 0444,
+		show_rpm, NULL, 1);
+static SENSOR_DEVICE_ATTR(fan2_input, 0444,
+		show_rpm, NULL, 2);
+static SENSOR_DEVICE_ATTR(fan3_input, 0444,
+		show_rpm, NULL, 3);
+static SENSOR_DEVICE_ATTR(fan4_input, 0444,
+		show_rpm, NULL, 4);
+static SENSOR_DEVICE_ATTR(fan5_input, 0444,
+		show_rpm, NULL, 5);
+static SENSOR_DEVICE_ATTR(fan6_input, 0444,
+		show_rpm, NULL, 6);
+static SENSOR_DEVICE_ATTR(fan7_input, 0444,
+		show_rpm, NULL, 7);
+static SENSOR_DEVICE_ATTR(fan8_input, 0444,
+		show_rpm, NULL, 8);
+static SENSOR_DEVICE_ATTR(fan9_input, 0444,
+		show_rpm, NULL, 9);
+static SENSOR_DEVICE_ATTR(fan10_input, 0444,
+		show_rpm, NULL, 10);
+static SENSOR_DEVICE_ATTR(fan11_input, 0444,
+		show_rpm, NULL, 11);
+static SENSOR_DEVICE_ATTR(fan12_input, 0444,
+		show_rpm, NULL, 12);
+static SENSOR_DEVICE_ATTR(fan13_input, 0444,
+		show_rpm, NULL, 13);
+static SENSOR_DEVICE_ATTR(fan14_input, 0444,
+		show_rpm, NULL, 14);
+static SENSOR_DEVICE_ATTR(fan15_input, 0444,
+		show_rpm, NULL, 15);
+static struct attribute *fan_dev_attrs[] = {
+	&sensor_dev_attr_fan0_input.dev_attr.attr,
+	&sensor_dev_attr_fan1_input.dev_attr.attr,
+	&sensor_dev_attr_fan2_input.dev_attr.attr,
+	&sensor_dev_attr_fan3_input.dev_attr.attr,
+	&sensor_dev_attr_fan4_input.dev_attr.attr,
+	&sensor_dev_attr_fan5_input.dev_attr.attr,
+	&sensor_dev_attr_fan6_input.dev_attr.attr,
+	&sensor_dev_attr_fan7_input.dev_attr.attr,
+	&sensor_dev_attr_fan8_input.dev_attr.attr,
+	&sensor_dev_attr_fan9_input.dev_attr.attr,
+	&sensor_dev_attr_fan10_input.dev_attr.attr,
+	&sensor_dev_attr_fan11_input.dev_attr.attr,
+	&sensor_dev_attr_fan12_input.dev_attr.attr,
+	&sensor_dev_attr_fan13_input.dev_attr.attr,
+	&sensor_dev_attr_fan14_input.dev_attr.attr,
+	&sensor_dev_attr_fan15_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group fan_dev_group = {
+	.attrs = fan_dev_attrs,
+	.is_visible = fan_dev_is_visible,
+};
+
+/*
+ * The clock type is type M :
+ * The PWM frequency = 24MHz / (type M clock division L bit *
+ * type M clock division H bit * (type M PWM period bit + 1))
+ */
+static void aspeed_create_type(struct aspeed_pwm_tacho_data *priv)
+{
+	priv->type_pwm_clock_division_h[TYPEM] = M_PWM_DIV_H;
+	priv->type_pwm_clock_division_l[TYPEM] = M_PWM_DIV_L;
+	priv->type_pwm_clock_unit[TYPEM] = M_PWM_PERIOD;
+	aspeed_set_pwm_clock_values(priv->regmap, TYPEM, M_PWM_DIV_H,
+				    M_PWM_DIV_L, M_PWM_PERIOD);
+	aspeed_set_tacho_type_enable(priv->regmap, TYPEM, true);
+	priv->type_fan_tach_clock_division[TYPEM] = M_TACH_CLK_DIV;
+	priv->type_fan_tach_unit[TYPEM] = M_TACH_UNIT;
+	aspeed_set_tacho_type_values(priv->regmap, TYPEM, M_TACH_MODE,
+				     M_TACH_UNIT, M_TACH_CLK_DIV);
+}
+
+static void aspeed_create_pwm_port(struct aspeed_pwm_tacho_data *priv,
+				   u8 pwm_port)
+{
+	aspeed_set_pwm_port_enable(priv->regmap, pwm_port, true);
+	priv->pwm_present[pwm_port] = true;
+
+	priv->pwm_port_type[pwm_port] = TYPEM;
+	aspeed_set_pwm_port_type(priv->regmap, pwm_port, TYPEM);
+
+	priv->pwm_port_fan_ctrl[pwm_port] = INIT_FAN_CTRL;
+	aspeed_set_pwm_port_fan_ctrl(priv, pwm_port, INIT_FAN_CTRL);
+}
+
+static void aspeed_create_fan_tach_channel(struct aspeed_pwm_tacho_data *priv,
+					   u8 *fan_tach_ch,
+					   int count,
+					   u8 pwm_source)
+{
+	u8 val, index;
+
+	for (val = 0; val < count; val++) {
+		index = fan_tach_ch[val];
+		aspeed_set_fan_tach_ch_enable(priv->regmap, index, true);
+		priv->fan_tach_present[index] = true;
+		priv->fan_tach_ch_source[index] = pwm_source;
+		aspeed_set_fan_tach_ch_source(priv->regmap, index, pwm_source);
+	}
+}
+
+static int aspeed_create_fan(struct device *dev,
+			     struct device_node *child,
+			     struct aspeed_pwm_tacho_data *priv)
+{
+	u8 *fan_tach_ch;
+	u32 pwm_port;
+	int ret, count;
+
+	ret = of_property_read_u32(child, "reg", &pwm_port);
+	if (ret)
+		return ret;
+	aspeed_create_pwm_port(priv, (u8)pwm_port);
+
+	count = of_property_count_u8_elems(child, "aspeed,fan-tach-ch");
+	if (count < 1)
+		return -EINVAL;
+	fan_tach_ch = devm_kzalloc(dev, sizeof(*fan_tach_ch) * count,
+				   GFP_KERNEL);
+	if (!fan_tach_ch)
+		return -ENOMEM;
+	ret = of_property_read_u8_array(child, "aspeed,fan-tach-ch",
+					fan_tach_ch, count);
+	if (ret)
+		return ret;
+	aspeed_create_fan_tach_channel(priv, fan_tach_ch, count, pwm_port);
+
+	return 0;
+}
+
+static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np, *child;
+	struct aspeed_pwm_tacho_data *priv;
+	void __iomem *regs;
+	struct resource *res;
+	struct device *hwmon;
+	struct clk *clk;
+	int ret;
+
+	np = dev->of_node;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENOENT;
+	regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs,
+			&aspeed_pwm_tacho_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+	regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE, 0);
+	regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE_EXT, 0);
+
+	clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(clk))
+		return -ENODEV;
+	priv->clk_freq = clk_get_rate(clk);
+	aspeed_set_clock_enable(priv->regmap, true);
+	aspeed_set_clock_source(priv->regmap, 0);
+
+	aspeed_create_type(priv);
+
+	for_each_child_of_node(np, child) {
+		ret = aspeed_create_fan(dev, child, priv);
+		of_node_put(child);
+		if (ret)
+			return ret;
+	}
+	of_node_put(np);
+
+	priv->groups[0] = &pwm_dev_group;
+	priv->groups[1] = &fan_dev_group;
+	priv->groups[2] = NULL;
+	hwmon = devm_hwmon_device_register_with_groups(dev,
+						       "aspeed_pwm_tacho",
+						       priv, priv->groups);
+	return PTR_ERR_OR_ZERO(hwmon);
+}
+
+static const struct of_device_id of_pwm_tacho_match_table[] = {
+	{ .compatible = "aspeed,ast2400-pwm-tacho", },
+	{ .compatible = "aspeed,ast2500-pwm-tacho", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_tacho_match_table);
+
+static struct platform_driver aspeed_pwm_tacho_driver = {
+	.probe		= aspeed_pwm_tacho_probe,
+	.driver		= {
+		.name	= "aspeed_pwm_tacho",
+		.of_match_table = of_pwm_tacho_match_table,
+	},
+};
+
+module_platform_driver(aspeed_pwm_tacho_driver);
+
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("ASPEED PWM and Fan Tacho device driver");
+MODULE_LICENSE("GPL");
--
2.12.2.715.g7642488e1d-goog


^ permalink raw reply related

* [PATCH linux v7 1/2] Documentation: dt-bindings: Document bindings for ASPEED AST2400/AST2500 PWM and Fan tach controller device driver
From: Jaghathiswari Rankappagounder Natarajan @ 2017-04-05  0:52 UTC (permalink / raw)
  To: joel, jdelvare, linux, linux-hwmon, linux-kernel, openbmc, corbet,
	linux-doc, robh+dt, mark.rutland, devicetree
  Cc: Jaghathiswari Rankappagounder Natarajan
In-Reply-To: <20170405005241.8794-1-jaghu@google.com>

This binding provides interface for adding values related to ASPEED
AST2400/2500 PWM and Fan tach controller support.
The PWM controller can support upto 8 PWM output ports.
The Fan tach controller can support upto 16 tachometer inputs.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 v7:
- Added a semicolon at the end of pwm_tacho_fixed_clk node in Examples section

 v6:
- Changed to aspeed,fan-tach-ch

 v5:
- Changed the naming scheme to aspeed,ast2400/2500-pwm-tacho
- Removed gpio pin muxing
- Added aspeed vendor prefix for fan-tach-ch
- Changed to fan@0/1
- Changed reg to 32 bits

 v4:
- Used 'reg'

 v3:
- Made the structure more common

 v2:
- Removed '_' in node and property names
- Gave some explanation for the properties used

 .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 68 ++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt

diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
new file mode 100644
index 000000000000..cf4460564adb
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
@@ -0,0 +1,68 @@
+ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver
+
+The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho
+controller can support upto 16 Fan tachometer inputs.
+
+There can be upto 8 fans supported. Each fan can have one PWM output and
+one/two Fan tach inputs.
+
+Required properties for pwm-tacho node:
+- #address-cells : should be 1.
+
+- #size-cells : should be 1.
+
+- reg : address and length of the register set for the device.
+
+- pinctrl-names : a pinctrl state named "default" must be defined.
+
+- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
+
+- compatible : should be "aspeed,ast2400-pwm-tacho" for AST2400 and
+	       "aspeed,ast2500-pwm-tacho" for AST2500.
+
+- clocks : a fixed clock providing input clock frequency(PWM
+	   and Fan Tach clock)
+
+fan subnode format:
+===================
+Under fan subnode there can upto 8 child nodes, with each child node
+representing a fan. If there are 8 fans each fan can have one PWM port and
+one/two Fan tach inputs.
+
+Required properties for each child node:
+- reg : should specify PWM source port.
+	integer value in the range 0 to 7 with 0 indicating PWM port A and
+	7 indicating PWM port H.
+
+- aspeed,fan-tach-ch : should specify the Fan tach input channel.
+                integer value in the range 0 through 15, with 0 indicating
+		Fan tach channel 0 and 15 indicating Fan tach channel 15.
+		Atleast one Fan tach input channel is required.
+
+Examples:
+
+pwm_tacho_fixed_clk: fixedclk {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <24000000>;
+};
+
+pwm_tacho: pwmtachocontroller@1e786000 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	reg = <0x1E786000 0x1000>;
+	compatible = "aspeed,ast2500-pwm-tacho";
+	clocks = <&pwm_tacho_fixed_clk>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>;
+
+	fan@0 {
+		reg = <0x00>;
+		aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+	};
+
+	fan@1 {
+		reg = <0x01>;
+		aspeed,fan-tach-ch = /bits/ 8 <0x01 0x02>;
+	};
+};
--
2.12.2.715.g7642488e1d-goog


^ permalink raw reply related

* [PATCH linux v7 0/2] Support for ASPEED AST2400/AST2500 PWM and Fan Tach driver
From: Jaghathiswari Rankappagounder Natarajan @ 2017-04-05  0:52 UTC (permalink / raw)
  To: joel, jdelvare, linux, linux-hwmon, linux-kernel, openbmc, corbet,
	linux-doc, robh+dt, mark.rutland, devicetree
  Cc: Jaghathiswari Rankappagounder Natarajan

Support for ASPEED AST2400/AST2500 PWM and Fan Tach driver.
Patches based on the upstream tag 4.9. Changes made in Version 4 are indicated
in the individual patches.
The AST2400/AST2500 PWM controller can support 8 PWM output ports.
The AST2400/AST2500 Fan Tach controller can support 16 tachometer inputs.
The hwmon driver provides sysfs entries through which the user can configure the
duty cycle for the particular PWM output port and read the fan rpm value for
the particular tachometer input.
Added devicetree binding documentation for AST2400/AST2500 PWM and Fan Tach
support.

Tested on Zaius board (which has AST2500 chip) and observed that when
duty cycle is lowered then the fan speed is lowered and lower fan rpm value(
corresponding to the duty cycle) is reported and when the duty cycle is
increased then the fan speed increases and higher fan rpm value(corresponding
to the duty cycle) is reported.

Jaghathiswari Rankappagounder Natarajan (2):
  Documentation: dt-bindings: Document bindings for ASPEED
    AST2400/AST2500 PWM and Fan tach controller device driver
  drivers: hwmon: Support for ASPEED PWM/Fan tach

 .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt |  68 ++
 Documentation/hwmon/aspeed-pwm-tacho               |  22 +
 drivers/hwmon/Kconfig                              |   9 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/aspeed-pwm-tacho.c                   | 835 +++++++++++++++++++++
 5 files changed, 935 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
 create mode 100644 Documentation/hwmon/aspeed-pwm-tacho
 create mode 100644 drivers/hwmon/aspeed-pwm-tacho.c

--
2.12.2.715.g7642488e1d-goog


^ permalink raw reply

* Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Steve Longerbeam @ 2017-04-05  0:44 UTC (permalink / raw)
  To: Russell King - ARM Linux, Philipp Zabel
  Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, mchehab,
	hverkuil, nick, markus.heiser, laurent.pinchart+renesas, bparrot,
	geert, arnd, sudipm.mukherjee, minghsiu.tsai, tiffany.lin,
	jean-christophe.trotin, horms+renesas, niklas.soderlund+renesas,
	robert.jarzmik, songjun.wu, andrew-ct.chen, gregkh, shuah,
	sakari.ailus, pavel, devicetree, linux-kernel, linux-arm-kernel,
	linux-media, devel
In-Reply-To: <19f0ce92-cad6-8950-8018-e3224e2bf266@gmail.com>



On 04/04/2017 05:40 PM, Steve Longerbeam wrote:
>
>
> On 04/04/2017 04:10 PM, Russell King - ARM Linux wrote:
>> On Thu, Mar 30, 2017 at 07:25:49PM +0200, Philipp Zabel wrote:
>>> The TVP5150 DT bindings specify a single output port (port 0) that
>>> corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT).
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> ---
>>> I'm trying to get this to work with a TVP5150 analog TV decoder, and the
>>> first problem is that this device doesn't have pad 0 as its single
>>> output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for
>>> pads (input, video out, vbi out, audio out), and video out is pad 1,
>>> whereas the device tree only defines a single port (0).
>>
>> Looking at the patch, it's highlighted another review point with
>> Steve's driver.
>>
>>> diff --git a/drivers/staging/media/imx/imx-media-dev.c
>>> b/drivers/staging/media/imx/imx-media-dev.c
>>> index 17e2386a3ca3a..c52d6ca797965 100644
>>> --- a/drivers/staging/media/imx/imx-media-dev.c
>>> +++ b/drivers/staging/media/imx/imx-media-dev.c
>>> @@ -267,6 +267,15 @@ static int imx_media_create_link(struct
>>> imx_media_dev *imxmd,
>>>      source_pad = link->local_pad;
>>>      sink_pad = link->remote_pad;
>>>
>>> +    /*
>>> +     * If the source subdev is an analog video decoder with a single
>>> source
>>> +     * port, assume that this port 0 corresponds to the
>>> DEMOD_PAD_VID_OUT
>>> +     * entity pad.
>>> +     */
>>> +    if (source->entity.function == MEDIA_ENT_F_ATV_DECODER &&
>>> +        local_sd->num_sink_pads == 0 && local_sd->num_src_pads == 1)
>>> +        source_pad = DEMOD_PAD_VID_OUT;
>>> +
>>>      v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
>>>            source->name, source_pad, sink->name, sink_pad);
>>
>> What is "local" and what is "remote" here?  It seems that, in the case of
>> a link being created with the sensor(etc), it's all back to front.
>>
>> Eg, I see locally:
>>
>> imx-media: imx_media_create_link: imx219 0-0010:0 -> imx6-mipi-csi2:0
>>
>> So here, "source" is the imx219 (the sensor), and sink is
>> "imx6-mipi-csi2"
>> (part of the iMX6 capture.)  However, this makes "local_sd" the subdev of
>> the sensor, and "remote_sd" the subdev of the CSI2 interface - which is
>> totally back to front - this code is part of the iMX6 capture system,
>> so "local" implies that it should be part of that, and the "remote" thing
>> would be the sensor.
>>
>> Hence, this seems completely confused.  I'd suggest that:
>>
>> (a) the "pad->pad.flags & MEDIA_PAD_FL_SINK" test in
>> imx_media_create_link()
>>     is moved into imx_media_create_links(), and placed here instead:
>>
>>         for (j = 0; j < num_pads; j++) {
>>             pad = &local_sd->pad[j];
>>
>>             if (pad->pad.flags & MEDIA_PAD_FL_SINK)
>>                 continue;
>>
>>             ...
>>         }
>>
>>     as the pad isn't going to spontaneously change this flag while we
>>     consider each individual link.
>
>
> Sure, I can do that. It would avoid iterating unnecessarily through the
> pad's links if the pad is a sink.
>
>
>>  However, maybe the test should be:
>>
>>             if (!(pad->pad.flags & MEDIA_PAD_FL_SOURCE))
>>
>>     ?
>>
>
> maybe that is more intuitive.
>
>
>> (b) the terms "local" and "remote" in imx_media_create_link() are
>>     replaced with "source" and "sink" respectively, since this will
>>     now be called with a guaranteed source pad.
>
> Agreed. I'll change the arg and local var names.
>
>>
>> As for Philipp's solution, I'm not sure what the correct solution for
>> something like this is.  It depends how you view "hardware interface"
>> as defined by video-interfaces.txt, and whether the pads on the TVP5150
>> represent the hardware interfaces.  If we take the view that the pads
>> do represent hardware interfaces, then using the reg= property on the
>> port node will solve this problem.
>
> And the missing port nodes would have to actually be defined first.
> According to Philipp they aren't, only a single output port 0.
>
>
>>
>> If not, it would mean that we would have to have the iMX capture code
>> scan the pads on the source device, looking for outputs - but that
>> runs into a problem - if the source device has multiple outputs, does
>> the reg= property specify the output pad index or the pad number,
>
> And how do we even know the data direction of a DT port. Is it an input,
> an output, bidirectional? The OF graph parsing in imx-media-of.c can't
> determine a port's direction if it encounters a device it doesn't
> recognize that has multiple ports. For now that is not really a problem
> because upstream from the video mux and csi-2 receiver it's expected
> there will only be original sources of video with only one source port.
> But it can become a limitation later. For example a device that has
> multiple output bus interfaces, which would require multiple output
> ports.
>

Actually what was I thinking, the TVP5150 is already an example of
such a device.

All of this could be solved if there was some direction information
in port nodes.

Steve

^ permalink raw reply

* Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Steve Longerbeam @ 2017-04-05  0:40 UTC (permalink / raw)
  To: Russell King - ARM Linux, Philipp Zabel
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	fabio.estevam-3arQi8VN3Tc, mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw, nick-gcszYUEDH4VrovVCs/uTlw,
	markus.heiser-O6JHGLzbNUwb1SvskN2V4Q,
	laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
	bparrot-l0cyMroinI0, geert-Td1EMuHUCqxL1ZNQvxDV9g,
	arnd-r2nGTMty4D4, sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
	minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
	tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
	jean-christophe.trotin-qxv4g6HH51o,
	horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
	niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw,
	robert.jarzmik-GANU6spQydw, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA,
	andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	shuah-DgEjT+Ai2ygdnm+yROfE0A, sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
	pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
In-Reply-To: <20170404231053.GE7909-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>



On 04/04/2017 04:10 PM, Russell King - ARM Linux wrote:
> On Thu, Mar 30, 2017 at 07:25:49PM +0200, Philipp Zabel wrote:
>> The TVP5150 DT bindings specify a single output port (port 0) that
>> corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT).
>>
>> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> ---
>> I'm trying to get this to work with a TVP5150 analog TV decoder, and the
>> first problem is that this device doesn't have pad 0 as its single
>> output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for
>> pads (input, video out, vbi out, audio out), and video out is pad 1,
>> whereas the device tree only defines a single port (0).
>
> Looking at the patch, it's highlighted another review point with
> Steve's driver.
>
>> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
>> index 17e2386a3ca3a..c52d6ca797965 100644
>> --- a/drivers/staging/media/imx/imx-media-dev.c
>> +++ b/drivers/staging/media/imx/imx-media-dev.c
>> @@ -267,6 +267,15 @@ static int imx_media_create_link(struct imx_media_dev *imxmd,
>>  	source_pad = link->local_pad;
>>  	sink_pad = link->remote_pad;
>>
>> +	/*
>> +	 * If the source subdev is an analog video decoder with a single source
>> +	 * port, assume that this port 0 corresponds to the DEMOD_PAD_VID_OUT
>> +	 * entity pad.
>> +	 */
>> +	if (source->entity.function == MEDIA_ENT_F_ATV_DECODER &&
>> +	    local_sd->num_sink_pads == 0 && local_sd->num_src_pads == 1)
>> +		source_pad = DEMOD_PAD_VID_OUT;
>> +
>>  	v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
>>  		  source->name, source_pad, sink->name, sink_pad);
>
> What is "local" and what is "remote" here?  It seems that, in the case of
> a link being created with the sensor(etc), it's all back to front.
>
> Eg, I see locally:
>
> imx-media: imx_media_create_link: imx219 0-0010:0 -> imx6-mipi-csi2:0
>
> So here, "source" is the imx219 (the sensor), and sink is "imx6-mipi-csi2"
> (part of the iMX6 capture.)  However, this makes "local_sd" the subdev of
> the sensor, and "remote_sd" the subdev of the CSI2 interface - which is
> totally back to front - this code is part of the iMX6 capture system,
> so "local" implies that it should be part of that, and the "remote" thing
> would be the sensor.
>
> Hence, this seems completely confused.  I'd suggest that:
>
> (a) the "pad->pad.flags & MEDIA_PAD_FL_SINK" test in imx_media_create_link()
>     is moved into imx_media_create_links(), and placed here instead:
>
> 		for (j = 0; j < num_pads; j++) {
> 			pad = &local_sd->pad[j];
>
> 			if (pad->pad.flags & MEDIA_PAD_FL_SINK)
> 				continue;
>
> 			...
> 		}
>
>     as the pad isn't going to spontaneously change this flag while we
>     consider each individual link.


Sure, I can do that. It would avoid iterating unnecessarily through the
pad's links if the pad is a sink.


>  However, maybe the test should be:
>
> 			if (!(pad->pad.flags & MEDIA_PAD_FL_SOURCE))
>
>     ?
>

maybe that is more intuitive.


> (b) the terms "local" and "remote" in imx_media_create_link() are
>     replaced with "source" and "sink" respectively, since this will
>     now be called with a guaranteed source pad.

Agreed. I'll change the arg and local var names.

>
> As for Philipp's solution, I'm not sure what the correct solution for
> something like this is.  It depends how you view "hardware interface"
> as defined by video-interfaces.txt, and whether the pads on the TVP5150
> represent the hardware interfaces.  If we take the view that the pads
> do represent hardware interfaces, then using the reg= property on the
> port node will solve this problem.

And the missing port nodes would have to actually be defined first.
According to Philipp they aren't, only a single output port 0.


>
> If not, it would mean that we would have to have the iMX capture code
> scan the pads on the source device, looking for outputs - but that
> runs into a problem - if the source device has multiple outputs, does
> the reg= property specify the output pad index or the pad number,

And how do we even know the data direction of a DT port. Is it an input,
an output, bidirectional? The OF graph parsing in imx-media-of.c can't
determine a port's direction if it encounters a device it doesn't
recognize that has multiple ports. For now that is not really a problem
because upstream from the video mux and csi-2 receiver it's expected
there will only be original sources of video with only one source port.
But it can become a limitation later. For example a device that has
multiple output bus interfaces, which would require multiple output
ports.


Steve



> and what if one binding for a device specifies it one way and another
> device's binding specifies it a different way.
>
> There's lots of scope for making things really painful here, and
> ending up with needing sensor-specific code in capture drivers to
> work around different decisions on this.
>
> I think someone needs to nail this down, if it's not already too late.
>
--
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 1/2] doc: Add bindings document for Xilinx LogiCore PR Decoupler
From: Moritz Fischer @ 2017-04-04 23:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: Alan Tull, Greg Kroah-Hartman, Linux Kernel Mailing List,
	linux-fpga-u79uwXL29TY76Z2rM5mHXA, Moritz Fischer, Michal Simek,
	Sören Brinkmann, Devicetree List
In-Reply-To: <20170330224429.yo42hgnr7nhm2ljv@rob-hp-laptop>

On Thu, Mar 30, 2017 at 05:44:29PM -0500, Rob Herring wrote:
> On Fri, Mar 24, 2017 at 10:33:20AM -0500, Alan Tull wrote:
> > From: Moritz Fischer <mdf-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Please use "dt-bindings: fpga: ..." for the subject.
>
>
> >
> > This adds the binding documentation for the Xilinx LogiCORE PR
> > Decoupler soft core.
> >
> > Signed-off-by: Moritz Fischer <mdf-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> > Acked-by: Alan Tull <atull-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> I'm confused why you are sending these instead of Moritz? If it goes
> through you, then it should have your S-o-B too.

Do you want me to resend this Alan (with Rob's suggestions)?
>
> > Cc: Sören Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > ---
> >  .../bindings/fpga/xilinx-pr-decoupler.txt          | 35 ++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
> >
> > diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
> > new file mode 100644
> > index 000000000000..2c527ac30398
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
> > @@ -0,0 +1,35 @@
> > +Xilinx LogiCORE Partial Reconfig Decoupler Softcore
> > +
> > +The Xilinx LogiCORE Partial Reconfig Decoupler manages one or more
> > +decouplers / fpga bridges.
> > +The controller can decouple/disable the bridges which prevents signal
> > +changes from passing through the bridge.  The controller can also
> > +couple / enable the bridges which allows traffic to pass through the
> > +bridge normally.
> > +
> > +The Driver supports only MMIO handling. A PR region can have multiple
> > +PR Decouplers which can be handled independently or chained via decouple/
> > +decouple_status signals.
> > +
> > +Required properties:
> > +- compatible : Should contain "xlnx,pr-decoupler-1.00" or "xlnx,pr-decoupler"
>
> I'd drop xlnx,pr-decoupler, but in any case, it should not be OR rather
> "followed by". Plus the example has both.

Michal wanted to have both, so I put both. Personally I don't care. I
think they have some downstream stuff that relied on it.

>
> > +- regs : base address and size for decoupler module
> > +- clocks : input clock to IP
> > +- clock-names : should contain "aclk"
> > +
> > +Optional properties:
> > +- bridge-enable : 0 if driver should disable bridge at startup
> > +  1 if driver should enable bridge at startup
> > +  Default is to leave bridge in current state.
> > +
> > +See Documentation/devicetree/bindings/fpga/fpga-region.txt for generic bindings.
> > +
> > +Example:
> > + fpga-bridge@100000450 {
> > + compatible = "xlnx,pr-decoupler-1.00",
> > +     "xlnx-pr-decoupler";
> > + regs = <0x10000045 0x10>;
> > + clocks = <&clkc 15>;
> > + clock-names = "aclk";
> > + bridge-enable = <0>;
> > + };
> > --
> > 2.11.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

Thanks,
Moritz
--
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: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Russell King - ARM Linux @ 2017-04-04 23:10 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Steve Longerbeam, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, fabio.estevam-3arQi8VN3Tc,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	nick-gcszYUEDH4VrovVCs/uTlw, markus.heiser-O6JHGLzbNUwb1SvskN2V4Q,
	laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
	bparrot-l0cyMroinI0, geert-Td1EMuHUCqxL1ZNQvxDV9g,
	arnd-r2nGTMty4D4, sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
	minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
	tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
	jean-christophe.trotin-qxv4g6HH51o,
	horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
	niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw,
	robert.jarzmik-GANU6spQydw, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA,
	andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	shuah-DgEjT+Ai2ygdnm+yROfE0A, sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
	pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-media
In-Reply-To: <1490894749.2404.33.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On Thu, Mar 30, 2017 at 07:25:49PM +0200, Philipp Zabel wrote:
> The TVP5150 DT bindings specify a single output port (port 0) that
> corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT).
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> I'm trying to get this to work with a TVP5150 analog TV decoder, and the
> first problem is that this device doesn't have pad 0 as its single
> output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for
> pads (input, video out, vbi out, audio out), and video out is pad 1,
> whereas the device tree only defines a single port (0).

Looking at the patch, it's highlighted another review point with
Steve's driver.

> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
> index 17e2386a3ca3a..c52d6ca797965 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -267,6 +267,15 @@ static int imx_media_create_link(struct imx_media_dev *imxmd,
>  	source_pad = link->local_pad;
>  	sink_pad = link->remote_pad;
>  
> +	/*
> +	 * If the source subdev is an analog video decoder with a single source
> +	 * port, assume that this port 0 corresponds to the DEMOD_PAD_VID_OUT
> +	 * entity pad.
> +	 */
> +	if (source->entity.function == MEDIA_ENT_F_ATV_DECODER &&
> +	    local_sd->num_sink_pads == 0 && local_sd->num_src_pads == 1)
> +		source_pad = DEMOD_PAD_VID_OUT;
> +
>  	v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
>  		  source->name, source_pad, sink->name, sink_pad);

What is "local" and what is "remote" here?  It seems that, in the case of
a link being created with the sensor(etc), it's all back to front.

Eg, I see locally:

imx-media: imx_media_create_link: imx219 0-0010:0 -> imx6-mipi-csi2:0

So here, "source" is the imx219 (the sensor), and sink is "imx6-mipi-csi2"
(part of the iMX6 capture.)  However, this makes "local_sd" the subdev of
the sensor, and "remote_sd" the subdev of the CSI2 interface - which is
totally back to front - this code is part of the iMX6 capture system,
so "local" implies that it should be part of that, and the "remote" thing
would be the sensor.

Hence, this seems completely confused.  I'd suggest that:

(a) the "pad->pad.flags & MEDIA_PAD_FL_SINK" test in imx_media_create_link()
    is moved into imx_media_create_links(), and placed here instead:

		for (j = 0; j < num_pads; j++) {
			pad = &local_sd->pad[j];

			if (pad->pad.flags & MEDIA_PAD_FL_SINK)
				continue;

			...
		}

    as the pad isn't going to spontaneously change this flag while we
    consider each individual link.  However, maybe the test should be:

			if (!(pad->pad.flags & MEDIA_PAD_FL_SOURCE))

    ?

(b) the terms "local" and "remote" in imx_media_create_link() are
    replaced with "source" and "sink" respectively, since this will
    now be called with a guaranteed source pad.

As for Philipp's solution, I'm not sure what the correct solution for
something like this is.  It depends how you view "hardware interface"
as defined by video-interfaces.txt, and whether the pads on the TVP5150
represent the hardware interfaces.  If we take the view that the pads
do represent hardware interfaces, then using the reg= property on the
port node will solve this problem.

If not, it would mean that we would have to have the iMX capture code
scan the pads on the source device, looking for outputs - but that
runs into a problem - if the source device has multiple outputs, does
the reg= property specify the output pad index or the pad number, and
what if one binding for a device specifies it one way and another
device's binding specifies it a different way.

There's lots of scope for making things really painful here, and
ending up with needing sensor-specific code in capture drivers to
work around different decisions on this.

I think someone needs to nail this down, if it's not already too late.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Benjamin Herrenschmidt @ 2017-04-04 22:19 UTC (permalink / raw)
  To: Christopher Bostic, Joel Stanley
  Cc: Mark Rutland, devicetree, Andrew Jeffery, Greg KH, Russell King,
	rostedt, Linux Kernel Mailing List, Rob Herring, Jeremy Kerr,
	Edward A . James, Alistair Popple, mingo, linux-arm-kernel
In-Reply-To: <93b21624-11fc-b71b-aa78-6cb4371c87ae@linux.vnet.ibm.com>

On Tue, 2017-04-04 at 12:32 -0500, Christopher Bostic wrote:
> Agreed that there is room for improvement.   I intend to look further 
> into your suggestions from here and our private conversation on the 
> matter and make changes as appropriate.  I have an open issue to track 
> this.  As it exists in this patch reads/writes from master to slave 
> fundamentally work.  

My understanding is they "seem to work if you get lucky with the timing
and fall apart under load". Or did I hear wrong ?

>  Given the pervasiveness and time to fully evaluate 
> and test any protocol updates I intend address this in the near future 
> with a separate follow on patch.

Please try the simple change I proposed in my email. It's a 4 or 5
lines change max to your clock_toggle function and how it's called in
send and receive. It should be trivial to check if things still "seem
to work" to begin with.

Do you have some kind of test mechanism that hammers the FSI
continuously ? Such as doing a series of putmemproc/getmemproc &
checking the values ?

Then you can run that while hammering the LPC bus and generally putting
the BMC under load and you'll quickly see if it's reliable or not.

Cheers,
Ben.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Steve Longerbeam @ 2017-04-04 22:11 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
	mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
	bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
	tiffany.lin, jean-christophe.trotin, horms+renesas,
	niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
	andrew-ct.chen, gregkh, shuah, sakari.ailus, pavel, devicetree,
	linux-kernel, linux-arm-kernel, linux-media
In-Reply-To: <1490894749.2404.33.camel@pengutronix.de>



On 03/30/2017 10:25 AM, Philipp Zabel wrote:
> The TVP5150 DT bindings specify a single output port (port 0) that
> corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT).
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> I'm trying to get this to work with a TVP5150 analog TV decoder, and the
> first problem is that this device doesn't have pad 0 as its single
> output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for
> pads (input, video out, vbi out, audio out), and video out is pad 1,
> whereas the device tree only defines a single port (0).

Shouldn't the DT bindings define ports for these other pads?
I haven't seen this documented anywhere, but shouldn't there
be a 1:1 correspondence between DT ports and media pads?

Steve


> ---
>
>  drivers/staging/media/imx/imx-media-dev.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
> index 17e2386a3ca3a..c52d6ca797965 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -267,6 +267,15 @@ static int imx_media_create_link(struct imx_media_dev *imxmd,
>  	source_pad = link->local_pad;
>  	sink_pad = link->remote_pad;
>
> +	/*
> +	 * If the source subdev is an analog video decoder with a single source
> +	 * port, assume that this port 0 corresponds to the DEMOD_PAD_VID_OUT
> +	 * entity pad.
> +	 */
> +	if (source->entity.function == MEDIA_ENT_F_ATV_DECODER &&
> +	    local_sd->num_sink_pads == 0 && local_sd->num_src_pads == 1)
> +		source_pad = DEMOD_PAD_VID_OUT;
> +
>  	v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
>  		  source->name, source_pad, sink->name, sink_pad);
>
>

^ permalink raw reply

* Re: [PATCH v5 7/9] clk: hi6220: add debug APB clock
From: Stephen Boyd @ 2017-04-04 21:51 UTC (permalink / raw)
  To: Leo Yan
  Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Michael Turquette, Mathieu Poirier, Guodong Xu, John Stultz,
	linux-doc, linux-kernel, devicetree, linux-arm-kernel,
	linux-arm-msm, linux-soc, linux-clk, mike.leach, Suzuki.Poulose,
	sudeep.holla
In-Reply-To: <1490466197-29163-8-git-send-email-leo.yan@linaro.org>

On 03/26, Leo Yan wrote:
> The debug APB clock is absent in hi6220 driver, so this patch is to add
> support for it.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---

Applied to clk-next. I suspect we don't need a topic branch for
the DT header because arm-soc won't be taking the dts side of the
changes?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v5 5/9] coresight: use const for device_node structures
From: Stephen Boyd @ 2017-04-04 21:48 UTC (permalink / raw)
  To: Leo Yan
  Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Michael Turquette, Mathieu Poirier, Guodong Xu, John Stultz,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	mike.leach-QSEj5FYQhm4dnm+yROfE0A, Suzuki.Poulose-5wv7dgnIgG8,
	sudeep.holla-5wv7dgnIgG8
In-Reply-To: <1490466197-29163-6-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 03/26, Leo Yan wrote:
> Almost low level functions from open firmware have used const to
> qualify device_node structures, so add const for device_node
> parameters in of_coresight related functions.
> 
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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: [GIT PULL] PCI: Support for configurable PCI endpoint
From: Bjorn Helgaas @ 2017-04-04 20:36 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Bjorn Helgaas, Joao Pinto, linux-pci, linux-doc, linux-kernel,
	devicetree, linux-omap, linux-arm-kernel, hch, nsekhar
In-Reply-To: <20170327094520.3129-1-kishon@ti.com>

On Mon, Mar 27, 2017 at 03:14:56PM +0530, Kishon Vijay Abraham I wrote:
> Hi Bjorn,
> 
> Please find the pull request for PCI endpoint support below. I've
> also included all the history here.

I tentatively applied this to pci/host-designware with the mostly trival
textual changes below.  If you post the series again, please include them.

I saw some acks to prior revisions, but few of them were included in this
series.   Can you collect them up?  If there are no other substantial
changes, I can insert them into my branch manually.

Bjorn



diff --git a/Documentation/PCI/00-INDEX b/Documentation/PCI/00-INDEX
index fd533c79fa19..00c9a90b6f38 100644
--- a/Documentation/PCI/00-INDEX
+++ b/Documentation/PCI/00-INDEX
@@ -15,10 +15,10 @@ pcieaer-howto.txt
 endpoint/pci-endpoint.txt
 	- guide to add endpoint controller driver and endpoint function driver.
 endpoint/pci-endpoint-cfs.txt
-	- guide to use configfs to configure the pci endpoint function.
+	- guide to use configfs to configure the PCI endpoint function.
 endpoint/pci-test-function.txt
-	- specification of *pci test* function device.
+	- specification of *PCI test* function device.
 endpoint/pci-test-howto.txt
 	- userguide for PCI endpoint test function.
 endpoint/function/binding/
-	- binding documentation for pci endpoint function
+	- binding documentation for PCI endpoint function
diff --git a/Documentation/PCI/endpoint/function/binding/pci-test.txt b/Documentation/PCI/endpoint/function/binding/pci-test.txt
index c44fc18d78cc..3b68b955fb50 100644
--- a/Documentation/PCI/endpoint/function/binding/pci-test.txt
+++ b/Documentation/PCI/endpoint/function/binding/pci-test.txt
@@ -13,5 +13,5 @@ cache_line_size	 : don't care
 subsys_vendor_id : don't care
 subsys_id	 : don't care
 interrupt_pin	 : Should be 1 - INTA, 2 - INTB, 3 - INTC, 4 -INTD
-msi_interrupts	 : Should be 1 to 32 depending on the number of msi interrupts
+msi_interrupts	 : Should be 1 to 32 depending on the number of MSI interrupts
 		   to test
diff --git a/Documentation/PCI/endpoint/pci-endpoint-cfs.txt b/Documentation/PCI/endpoint/pci-endpoint-cfs.txt
index 8b2a8280b131..d740f29960a4 100644
--- a/Documentation/PCI/endpoint/pci-endpoint-cfs.txt
+++ b/Documentation/PCI/endpoint/pci-endpoint-cfs.txt
@@ -1,10 +1,10 @@
                    CONFIGURING PCI ENDPOINT USING CONFIGFS
                     Kishon Vijay Abraham I <kishon@ti.com>
 
-The PCI Endpoint Core exposes configfs entry (pci_ep) in order to configure the
-PCI endpoint function and in order to bind the endpoint function
+The PCI Endpoint Core exposes configfs entry (pci_ep) to configure the
+PCI endpoint function and to bind the endpoint function
 with the endpoint controller. (For introducing other mechanisms to
-configure the PCI Endpoint Function refer [1]).
+configure the PCI Endpoint Function refer to [1]).
 
 *) Mounting configfs
 
diff --git a/Documentation/PCI/endpoint/pci-endpoint.txt b/Documentation/PCI/endpoint/pci-endpoint.txt
index 4a3e4388b37b..9b1d66829290 100644
--- a/Documentation/PCI/endpoint/pci-endpoint.txt
+++ b/Documentation/PCI/endpoint/pci-endpoint.txt
@@ -2,27 +2,27 @@
 		    Kishon Vijay Abraham I <kishon@ti.com>
 
 This document is a guide to use the PCI Endpoint Framework in order to create
-endpoint controller driver, endpoint function driver and using configfs
+endpoint controller driver, endpoint function driver, and using configfs
 interface to bind the function driver to the controller driver.
 
 1. Introduction
 
-*Linux* has a comprehensive PCI subsystem to support PCI controllers that
+Linux has a comprehensive PCI subsystem to support PCI controllers that
 operates in Root Complex mode. The subsystem has capability to scan PCI bus,
-assign memory resources and irq resources, load PCI driver (based on
-vendorid, deviceid), support other services like hot-plug, power management,
+assign memory resources and IRQ resources, load PCI driver (based on
+vendor ID, device ID), support other services like hot-plug, power management,
 advanced error reporting and virtual channels.
 
-However PCI controller IPs integrated in certain SoC is capable of operating
+However the PCI controller IP integrated in some SoCs is capable of operating
 either in Root Complex mode or Endpoint mode. PCI Endpoint Framework will
-add endpoint mode support in *Linux*. This will help to run Linux in an
+add endpoint mode support in Linux. This will help to run Linux in an
 EP system which can have a wide variety of use cases from testing or
-validation, co-processor accelerator etc..
+validation, co-processor accelerator, etc.
 
 2. PCI Endpoint Core
 
-The PCI Endpoint Core layer comprises of 3 components: the Endpoint Controller
-library, the Endpoint Function library and the configfs layer to bind the
+The PCI Endpoint Core layer comprises 3 components: the Endpoint Controller
+library, the Endpoint Function library, and the configfs layer to bind the
 endpoint function with the endpoint controller.
 
 2.1 PCI Endpoint Controller(EPC) Library
@@ -42,20 +42,20 @@ by the PCI controller driver.
 	 * write_header: ops to populate configuration space header
 	 * set_bar: ops to configure the BAR
 	 * clear_bar: ops to reset the BAR
-	 * alloc_addr_space: ops to allocate *in* PCI controller address space
+	 * alloc_addr_space: ops to allocate in PCI controller address space
 	 * free_addr_space: ops to free the allocated address space
 	 * raise_irq: ops to raise a legacy or MSI interrupt
 	 * start: ops to start the PCI link
 	 * stop: ops to stop the PCI link
 
    The PCI controller driver can then create a new EPC device by invoking
-   devm_pci_epc_create/pci_epc_create.
+   devm_pci_epc_create()/pci_epc_create().
 
 *) devm_pci_epc_destroy()/pci_epc_destroy()
 
    The PCI controller driver can destroy the EPC device created by either
-   devm_pci_epc_create or pci_epc_create using devm_pci_epc_destroy() or
-   /pci_epc_destroy()
+   devm_pci_epc_create() or pci_epc_create() using devm_pci_epc_destroy() or
+   pci_epc_destroy().
 
 *) pci_epc_linkup()
 
@@ -112,27 +112,27 @@ by the PCI endpoint function driver.
 2.1.3 Other APIs
 
 There are other APIs provided by the EPC library. These are used for binding
-the epf device with epc device. pci-ep-cfs.c can be used as reference for
+the EPF device with EPC device. pci-ep-cfs.c can be used as reference for
 using these APIs.
 
 *) pci_epc_get()
 
-   Get a reference to the pci endpoint controller based on the device name of
+   Get a reference to the PCI endpoint controller based on the device name of
    the controller.
 
 *) pci_epc_put()
 
-   Release the reference to the pci endpoint controller obtained using
+   Release the reference to the PCI endpoint controller obtained using
    pci_epc_get()
 
 *) pci_epc_add_epf()
 
-   Add a pci endpoint function to a pci endpoint controller. A pcie device
-   can have upto 8 functions according to the specification.
+   Add a PCI endpoint function to a PCI endpoint controller. A PCIe device
+   can have up to 8 functions according to the specification.
 
 *) pci_epc_remove_epf()
 
-   Remove the pci endpoint function from pci endpoint controller.
+   Remove the PCI endpoint function from PCI endpoint controller.
 
 *) pci_epc_start()
 
@@ -147,7 +147,7 @@ using these APIs.
 2.2 PCI Endpoint Function(EPF) Library
 
 The EPF library provides APIs to be used by the function driver and the EPC
-library in order to provide endpoint mode functionality.
+library to provide endpoint mode functionality.
 
 2.2.1 APIs for the PCI Endpoint Function Driver
 
diff --git a/Documentation/PCI/endpoint/pci-test-function.txt b/Documentation/PCI/endpoint/pci-test-function.txt
index 1324376ec6a3..0c519c9bf94a 100644
--- a/Documentation/PCI/endpoint/pci-test-function.txt
+++ b/Documentation/PCI/endpoint/pci-test-function.txt
@@ -32,8 +32,8 @@ This register will be used by the host driver to indicate the function
 that the endpoint device must perform.
 
 Bitfield Description:
-  Bit 0		: raise legacy irq
-  Bit 1		: raise MSI irq
+  Bit 0		: raise legacy IRQ
+  Bit 1		: raise MSI IRQ
   Bit 2 - 7	: MSI interrupt number
   Bit 8		: read command (read data from RC buffer)
   Bit 9		: write command (write data to RC buffer)
@@ -51,7 +51,7 @@ Bitfield Description:
   Bit 3		: write fail
   Bit 4		: copy success
   Bit 5		: copy fail
-  Bit 6		: irq raised
+  Bit 6		: IRQ raised
   Bit 7		: source address is invalid
   Bit 8		: destination address is invalid
 
diff --git a/Documentation/PCI/endpoint/pci-test-howto.txt b/Documentation/PCI/endpoint/pci-test-howto.txt
index 730b70c73055..75f48c3bb191 100644
--- a/Documentation/PCI/endpoint/pci-test-howto.txt
+++ b/Documentation/PCI/endpoint/pci-test-howto.txt
@@ -49,7 +49,7 @@ configurable fields.
 	  cache_line_size	msi_interrupts	subclass_code	vendorid
 	  deviceid          	progif_code	subsys_id
 
-The pci endpoint function driver populates these entries with default values
+The PCI endpoint function driver populates these entries with default values
 when the device is bound to the driver. The pci-epf-test driver populates
 vendorid with 0xffff and interrupt_pin with 0x0001
 
diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
index b69dd7dbd29e..6a07c96227e0 100644
--- a/Documentation/devicetree/bindings/pci/ti-pci.txt
+++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
@@ -14,7 +14,7 @@ HOST MODE
 =========
  - reg : Two register ranges as listed in the reg-names property
  - reg-names : The first entry must be "ti-conf" for the TI specific registers
-	       The second entry must be "rc-dbics" for the designware pcie
+	       The second entry must be "rc-dbics" for the DesignWare PCIe
 	       registers
 	       The third entry must be "config" for the PCIe configuration space
  - interrupts : Two interrupt entries must be specified. The first one is for
@@ -39,7 +39,7 @@ DEVICE MODE
  - interrupts : one interrupt entries must be specified for main interrupt.
  - num-ib-windows : number of inbound address translation windows
  - num-ob-windows : number of outbound address translation windows
- - ti,syscon-unaligned-access: phandle to the syscon dt node. The 1st argument
+ - ti,syscon-unaligned-access: phandle to the syscon DT node. The 1st argument
 			       should contain the register offset within syscon
 			       and the 2nd argument should contain the bit field
 			       for setting the bit to enable unaligned
@@ -48,10 +48,10 @@ DEVICE MODE
 Optional Property:
  - gpios : Should be added if a gpio line is required to drive PERST# line
 
-NOTE: Two dt nodes may be added for each PCI controller; one for host
+NOTE: Two DT nodes may be added for each PCI controller; one for host
 mode and another for device mode. So in order for PCI to
-work in host mode, EP mode dt node should be disabled and in order to PCI to
-work in EP mode, host mode dt node should be disabled. And host mode and EP
+work in host mode, EP mode DT node should be disabled and in order to PCI to
+work in EP mode, host mode DT node should be disabled. Host mode and EP
 mode are mutually exclusive.
 
 Example:
diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
index 438571898d27..4ebc3594b32c 100644
--- a/Documentation/misc-devices/pci-endpoint-test.txt
+++ b/Documentation/misc-devices/pci-endpoint-test.txt
@@ -1,7 +1,7 @@
 Driver for PCI Endpoint Test Function
 
 This driver should be used as a host side driver if the root complex is
-connected to a configurable pci endpoint running *pci_epf_test* function
+connected to a configurable PCI endpoint running *pci_epf_test* function
 driver configured according to [1].
 
 The "pci_endpoint_test" driver can be used to perform the following tests.
@@ -20,10 +20,10 @@ should be used to perform the above tests.
 
 ioctl
 -----
- PCITEST_BAR: Tests the BAR. The number of the BAR that has to be tested
+ PCITEST_BAR: Tests the BAR. The number of the BAR to be tested
 	      should be passed as argument.
  PCITEST_LEGACY_IRQ: Tests legacy IRQ
- PCITEST_MSI: Tests message signalled interrupts. The MSI number that has
+ PCITEST_MSI: Tests message signalled interrupts. The MSI number
 	      to be tested should be passed as argument.
  PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
 		as argument.
diff --git a/MAINTAINERS b/MAINTAINERS
index 3c1b947811e2..15ed84389092 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9581,7 +9581,7 @@ F:	include/linux/pci*
 F:	arch/x86/pci/
 F:	arch/x86/kernel/quirks.c
 
-PCI EP SUBSYSTEM
+PCI ENDPOINT SUBSYSTEM
 M:	Kishon Vijay Abraham I <kishon@ti.com>
 L:	linux-pci@vger.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kishon/pci-endpoint.git
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 5f4f8f8ff1c2..09c10f426b64 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -424,7 +424,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	err = devm_request_irq(dev, pdev->irq, pci_endpoint_test_irqhandler,
 			       IRQF_SHARED, DRV_MODULE_NAME, test);
 	if (err) {
-		dev_err(dev, "failed to request irq\n");
+		dev_err(dev, "failed to request IRQ %d\n", pdev->irq);
 		goto err_disable_msi;
 	}
 
@@ -433,8 +433,8 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 				       pci_endpoint_test_irqhandler,
 				       IRQF_SHARED, DRV_MODULE_NAME, test);
 		if (err)
-			dev_err(dev, "failed to request irq for MSI %d\n",
-				i + 1);
+			dev_err(dev, "failed to request IRQ %d for MSI %d\n",
+				pdev->irq + i, i + 1);
 	}
 
 	for (bar = BAR_0; bar <= BAR_5; bar++) {
diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
index 345ad67dca3c..b7e15526d676 100644
--- a/drivers/pci/dwc/Kconfig
+++ b/drivers/pci/dwc/Kconfig
@@ -21,8 +21,8 @@ config PCI_DRA7XX
 	help
 	 Enables support for the PCIe controller in the DRA7xx SoC. There
 	 are two instances of PCIe controller in DRA7xx. This controller can
-	 work either as EP or RC. In order to enable host specific features
-	 PCI_DRA7XX_HOST must be selected and in order to enable device
+	 work either as EP or RC. In order to enable host-specific features
+	 PCI_DRA7XX_HOST must be selected and in order to enable device-
 	 specific features PCI_DRA7XX_EP must be selected. This uses
 	 the Designware core.
 
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 29cbe9a73b30..9ae9e59b2a74 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -43,7 +43,7 @@ static int devm_pci_epc_match(struct device *dev, void *res, void *match_data)
 }
 
 /**
- * pci_epc_put() - release the pci endpoint controller
+ * pci_epc_put() - release the PCI endpoint controller
  * @epc: epc returned by pci_epc_get()
  *
  * release the refcount the caller obtained by invoking pci_epc_get()
@@ -59,7 +59,7 @@ void pci_epc_put(struct pci_epc *epc)
 EXPORT_SYMBOL_GPL(pci_epc_put);
 
 /**
- * pci_epc_get() - get the pci endpoint controller
+ * pci_epc_get() - get the PCI endpoint controller
  * @epc_name: device name of the endpoint controller
  *
  * Invoke to get struct pci_epc * corresponding to the device name of the
@@ -225,11 +225,11 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 interrupts)
 EXPORT_SYMBOL_GPL(pci_epc_set_msi);
 
 /**
- * pci_epc_unmap_addr() - unmap cpu address from pci address
+ * pci_epc_unmap_addr() - unmap CPU address from PCI address
  * @epc: the EPC device on which address is allocated
  * @phys_addr: physical address of the local system
  *
- * Invoke to unmap the cpu address from pci address.
+ * Invoke to unmap the CPU address from PCI address.
  */
 void pci_epc_unmap_addr(struct pci_epc *epc, phys_addr_t phys_addr)
 {
@@ -248,13 +248,13 @@ void pci_epc_unmap_addr(struct pci_epc *epc, phys_addr_t phys_addr)
 EXPORT_SYMBOL_GPL(pci_epc_unmap_addr);
 
 /**
- * pci_epc_map_addr() - map cpu address to pci address
+ * pci_epc_map_addr() - map CPU address to PCI address
  * @epc: the EPC device on which address is allocated
  * @phys_addr: physical address of the local system
- * @pci_addr: pci address to which the physical address should be mapped
+ * @pci_addr: PCI address to which the physical address should be mapped
  * @size: the size of the allocation
  *
- * Invoke to map cpu address with pci address.
+ * Invoke to map CPU address with PCI address.
  */
 int pci_epc_map_addr(struct pci_epc *epc, phys_addr_t phys_addr,
 		     u64 pci_addr, size_t size)
@@ -279,7 +279,7 @@ EXPORT_SYMBOL_GPL(pci_epc_map_addr);
 /**
  * pci_epc_clear_bar() - reset the BAR
  * @epc: the EPC device for which the BAR has to be cleared
- * @bar: the bar number that has to be reset
+ * @bar: the BAR number that has to be reset
  *
  * Invoke to reset the BAR of the endpoint device.
  */
@@ -302,7 +302,7 @@ EXPORT_SYMBOL_GPL(pci_epc_clear_bar);
 /**
  * pci_epc_set_bar() - configure BAR in order for host to assign PCI addr space
  * @epc: the EPC device on which BAR has to be configured
- * @bar: the bar number that has to be configured
+ * @bar: the BAR number that has to be configured
  * @size: the size of the addr space
  * @flags: specify memory allocation/io allocation/32bit address/64 bit address
  *
@@ -358,13 +358,13 @@ int pci_epc_write_header(struct pci_epc *epc, struct pci_epf_header *header)
 EXPORT_SYMBOL_GPL(pci_epc_write_header);
 
 /**
- * pci_epc_add_epf() - bind pci endpoint function to an endpoint controller
+ * pci_epc_add_epf() - bind PCI endpoint function to an endpoint controller
  * @epc: the EPC device to which the endpoint function should be added
  * @epf: the endpoint function to be added
  *
  * A PCI endpoint device can have one or more functions. In the case of PCIe,
- * the specification allows upto 8 PCIe endpoint functions. Invoke
- * pci_epc_add_epf() to add a pci endpoint function to an endpoint controller.
+ * the specification allows up to 8 PCIe endpoint functions. Invoke
+ * pci_epc_add_epf() to add a PCI endpoint function to an endpoint controller.
  */
 int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf)
 {
@@ -392,11 +392,11 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf)
 EXPORT_SYMBOL_GPL(pci_epc_add_epf);
 
 /**
- * pci_epc_remove_epf() - remove pci endpoint function from endpoint controller
+ * pci_epc_remove_epf() - remove PCI endpoint function from endpoint controller
  * @epc: the EPC device from which the endpoint function should be removed
  * @epf: the endpoint function to be removed
  *
- * Invoke to remove pci endpoint function from the endpoint controller.
+ * Invoke to remove PCI endpoint function from the endpoint controller.
  */
 void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf)
 {
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 9ec1639ef749..92db7dcd911c 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -87,7 +87,7 @@ EXPORT_SYMBOL_GPL(pci_epf_bind);
 /**
  * pci_epf_free_space() - free the allocated PCI EPF register space
  * @addr: the virtual address of the PCI EPF register space
- * @bar: the bar number corresponding to the register space
+ * @bar: the BAR number corresponding to the register space
  *
  * Invoke to free the allocated PCI EPF register space.
  */
@@ -109,7 +109,7 @@ EXPORT_SYMBOL_GPL(pci_epf_free_space);
 /**
  * pci_epf_alloc_space() - allocate memory for the PCI EPF register space
  * @size: the size of the memory that has to be allocated
- * @bar: the bar number corresponding to the allocated register space
+ * @bar: the BAR number corresponding to the allocated register space
  *
  * Invoke to allocate memory for the PCI EPF register space.
  */
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index d6eb32291462..af5edbf3eea3 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -27,8 +27,8 @@ enum pci_epc_irq_type {
  * @write_header: ops to populate configuration space header
  * @set_bar: ops to configure the BAR
  * @clear_bar: ops to reset the BAR
- * @map_addr: ops to map cpu address to pci address
- * @unmap_addr: ops to unmap cpu address and pci address
+ * @map_addr: ops to map CPU address to PCI address
+ * @unmap_addr: ops to unmap CPU address and PCI address
  * @set_msi: ops to set the requested number of MSI interrupts in the MSI
  *	     capability register
  * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
@@ -58,9 +58,9 @@ struct pci_epc_ops {
 
 /**
  * struct pci_epc_mem - address space of the endpoint controller
- * @phys_base: physical base address of the pci address space
- * @size: the size of the pci address space
- * @bitmap: bitmap to manage the pci address space
+ * @phys_base: physical base address of the PCI address space
+ * @size: the size of the PCI address space
+ * @bitmap: bitmap to manage the PCI address space
  * @pages: number of bits representing the address region
  */
 struct pci_epc_mem {
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 5ff2c5a592c3..0d529cb90143 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -38,7 +38,7 @@ enum pci_barno {
  * struct pci_epf_header - represents standard configuration header
  * @vendorid: identifies device manufacturer
  * @deviceid: identifies a particular device
- * @revid: specifies a device specific revision identifier
+ * @revid: specifies a device-specific revision identifier
  * @progif_code: identifies a specific register-level programming interface
  * @subclass_code: identifies more specifically the function of the device
  * @baseclass_code: broadly classifies the type of function the device performs
@@ -115,11 +115,11 @@ struct pci_epf_bar {
  * @name: the name of the PCI EPF device
  * @header: represents standard configuration header
  * @bar: represents the BAR of EPF device
- * @msi_interrupts: number of msi interrupts required by this function
+ * @msi_interrupts: number of MSI interrupts required by this function
  * @func_no: unique function number within this endpoint device
  * @epc: the EPC device to which this EPF device is bound
  * @driver: the EPF driver to which this EPF device is bound
- * @list: to add pci_epf as a list of pci endpoint functions to pci_epc
+ * @list: to add pci_epf as a list of PCI endpoint functions to pci_epc
  */
 struct pci_epf {
 	struct device		dev;
diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 39b5b0ba569e..ad54a58d7dda 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -170,7 +170,7 @@ int main(int argc, char **argv)
 		fprintf(stderr,
 			"usage: %s [options]\n"
 			"Options:\n"
-			"\t-D <dev>		pci endpoint test device {default: /dev/pci-endpoint-test.0}\n"
+			"\t-D <dev>		PCI endpoint test device {default: /dev/pci-endpoint-test.0}\n"
 			"\t-b <bar num>		BAR test (bar number between 0..5)\n"
 			"\t-m <msi num>		MSI test (msi number between 1..32)\n"
 			"\t-r			Read buffer test\n"




--- cl.orig	2017-04-04 15:26:34.555152663 -0500
+++ cl.new	2017-04-04 15:26:47.575278312 -0500
@@ -1,67 +1,67 @@
-commit 485847badf93
+commit 2f81b96ef43e
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:20 2017 +0530
 
     ARM: DRA7: clockdomain: Change the CLKTRCTRL of CM_PCIE_CLKSTCTRL to SW_WKUP
     
-    The PCIe programming sequence in TRM suggests CLKSTCTRL of PCIe should
-    be set to SW_WKUP. There are no issues when CLKSTCTRL is set to HW_AUTO
-    in RC mode. However in EP mode, the host system is not able to access the
+    The PCIe programming sequence in TRM suggests CLKSTCTRL of PCIe should be
+    set to SW_WKUP. There are no issues when CLKSTCTRL is set to HW_AUTO in RC
+    mode. However in EP mode, the host system is not able to access the
     MEMSPACE and setting the CLKSTCTRL to SW_WKUP fixes it.
     
     Acked-by: Tony Lindgren <tony@atomide.com>
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit ea4a8033fd42
+commit 4d53ed89fe50
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:19 2017 +0530
 
-    MAINTAINERS: add PCI EP maintainer
+    MAINTAINERS: Add PCI Endpoint maintainer
     
-    Add maintainer for the newly introduced PCI EP framework.
+    Add maintainer for the newly introduced PCI Endpoint framework.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 9c3eb6b9db09
+commit a11812c0dacc
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:18 2017 +0530
 
     Documentation: PCI: Add userguide for PCI endpoint test function
     
-    Add documentation to help users use pci-epf-test function driver
-    and pci_endpoint_test host driver for testing PCI.
+    Add documentation to help users use pci-epf-test function driver and
+    pci_endpoint_test host driver for testing PCI.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit ce58c0dca0fc
+commit c7ce6959f0a5
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:17 2017 +0530
 
     tools: PCI: Add sample test script to invoke pcitest
     
-    Add a simple test script that invokes the pcitest userspace tool
-    to perform all the PCI endpoint tests (BAR tests, interrupt tests,
-    read tests, write tests and copy tests).
+    Add a simple test script that invokes the pcitest userspace tool to perform
+    all the PCI endpoint tests (BAR tests, interrupt tests, read tests, write
+    tests and copy tests).
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit fdc27ef5bde2
+commit f9ee26a038d8
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:16 2017 +0530
 
     tools: PCI: Add a userspace tool to test PCI endpoint
     
-    Add a userspace tool to invoke the ioctls exposed by the
-    PCI endpoint test driver to perform various PCI tests.
+    Add a userspace tool to invoke the ioctls exposed by the PCI endpoint test
+    driver to perform various PCI tests.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit d323b479a02e
+commit fea31fd3cf56
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:15 2017 +0530
 
@@ -72,21 +72,21 @@
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit d26ec30392dd
+commit bedcd782937f
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:14 2017 +0530
 
-    misc: Add host side pci driver for pci test function device
+    misc: Add host side PCI driver for PCI test function device
     
-    Add PCI endpoint test driver that can verify base address
-    register, legacy interrupt/MSI interrupt and read/write/copy
-    buffers between host and device. The corresponding pci-epf-test
-    function driver should be used on the EP side.
+    Add PCI endpoint test driver that can verify base address register, legacy
+    interrupt/MSI interrupt and read/write/copy buffers between host and
+    device. The corresponding pci-epf-test function driver should be used on
+    the EP side.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 5b43200ddaa7
+commit 8066f3c52feb
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:13 2017 +0530
 
@@ -98,115 +98,115 @@
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit a5090d8e9668
+commit 25eef24289ee
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:12 2017 +0530
 
-    dt-bindings: PCI: dra7xx: Add dt bindings to enable unaligned access
+    dt-bindings: PCI: dra7xx: Add DT bindings to enable unaligned access
     
-    Update device tree binding documentation of TI's dra7xx PCI
-    controller to include property for enabling unaligned mem access.
+    Update device tree binding documentation of TI's dra7xx PCI controller to
+    include property for enabling unaligned mem access.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 3f080640d53f
+commit ebe506162817
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:11 2017 +0530
 
     PCI: dwc: dra7xx: Workaround for errata id i870
     
-    According to errata i870, access to the PCIe slave port
-    that are not 32-bit aligned will result in incorrect mapping
-    to TLP Address and Byte enable fields.
+    According to errata i870, access to the PCIe slave port that are not 32-bit
+    aligned will result in incorrect mapping to TLP Address and Byte enable
+    fields.
     
     Accessing non 32-bit aligned data causes incorrect data in the target
-    buffer if memcpy is used. Implement the workaround for this
-    errata here.
+    buffer if memcpy is used. Implement the workaround for this errata here.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 24365ea32418
+commit 9ff4601c7765
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:10 2017 +0530
 
     dt-bindings: mfd: syscon: Add documentation for #syscon-cells property
     
-    Add documentation for the optional #syscon-cells property to determine
-    the number of cells that should be given in the phandle while
-    referencing the syscon-node.
+    Add documentation for the optional #syscon-cells property to determine the
+    number of cells that should be given in the phandle while referencing the
+    syscon-node.
     
     Suggested-by: Rob Herring <robh@kernel.org>
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 27cb5fde6551
+commit 1329cf15fe87
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:09 2017 +0530
 
-    dt-bindings: PCI: dra7xx: Add dt bindings for pci dra7xx EP mode
+    dt-bindings: PCI: dra7xx: Add DT bindings for PCI dra7xx EP mode
     
-    Add device tree binding documentation for pci dra7xx EP mode.
+    Add device tree binding documentation for PCI dra7xx EP mode.
     
     Acked-by: Rob Herring <robh@kernel.org>
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit e8be32886fd1
+commit 6259ce5e8f4a
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:08 2017 +0530
 
     PCI: dwc: dra7xx: Add EP mode support
     
-    The PCIe controller integrated in dra7xx SoCs is capable of operating
-    in endpoint mode. Add endpoint mode support to dra7xx driver.
+    The PCIe controller integrated in dra7xx SoCs is capable of operating in
+    endpoint mode. Add endpoint mode support to dra7xx driver.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 3138c72f71f2
+commit 9567c35d6566
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:07 2017 +0530
 
-    PCI: dwc: dra7xx: Facilitate wrapper and msi interrupts to be enabled independently
+    PCI: dwc: dra7xx: Facilitate wrapper and MSI interrupts to be enabled independently
     
-    No functional change. Split dra7xx_pcie_enable_interrupts into
-    dra7xx_pcie_enable_wrapper_interrupts and dra7xx_pcie_enable_msi_interrupts
-    so that wrapper interrupts and msi interrupts can be enabled independently.
-    This is in preparation for adding EP mode support to dra7xx driver since
-    EP mode doesn't have to enable msi_interrupts.
+    No functional change. Split dra7xx_pcie_enable_interrupts() into
+    dra7xx_pcie_enable_wrapper_interrupts() and
+    dra7xx_pcie_enable_msi_interrupts() so that wrapper interrupts and MSI
+    interrupts can be enabled independently.  This is in preparation for adding
+    EP mode support to dra7xx driver since EP mode doesn't have to enable
+    msi_interrupts.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 8ba73ff9284c
+commit c10abab4c7ca
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:06 2017 +0530
 
-    dt-bindings: PCI: Add dt bindings for pci designware EP mode
+    dt-bindings: PCI: Add DT bindings for PCI designware EP mode
     
-    Add device tree binding documentation for pci designware EP mode.
+    Add device tree binding documentation for PCI designware EP mode.
     
-    Acked-by: Rob Herring <robh@kernel.org>
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
+    Acked-by: Rob Herring <robh@kernel.org>
 
-commit b6251498f9bc
+commit 98d63c77d6a0
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:05 2017 +0530
 
     PCI: dwc: designware: Add EP mode support
     
-    Add endpoint mode support to designware driver. This uses the
-    EP Core layer introduced recently to add endpoint mode support.
-    *Any* function driver can now use this designware device
-    in order to achieve the EP functionality.
+    Add endpoint mode support to designware driver. This uses the EP Core layer
+    introduced recently to add endpoint mode support.  *Any* function driver
+    can now use this designware device in order to achieve the EP
+    functionality.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 86d768a59594
+commit 36ecd387d736
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:04 2017 +0530
 
@@ -218,71 +218,70 @@
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 41c1e46c71b9
+commit b8116f7d8cec
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:03 2017 +0530
 
     PCI: endpoint: functions: Add an EP function to test PCI
     
-    Adds a new endpoint function driver (to program the virtual
-    test device) making use of the EP-core library.
+    Adds a new endpoint function driver (to program the virtual test device)
+    making use of the EP-core library.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 05e78b3dee04
+commit 1a5946105d8d
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:02 2017 +0530
 
-    Documentation: PCI: Add specification for the *pci test* function device
+    Documentation: PCI: Add specification for the *PCI test* function device
     
-    Add specification for the *pci test* virtual function device. The endpoint
-    function driver and the host pci driver should be created based on this
+    Add specification for the *PCI test* virtual function device. The endpoint
+    function driver and the host PCI driver should be created based on this
     specification.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 5904951f2a07
+commit f24b3959ecbc
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:01 2017 +0530
 
     PCI: endpoint: Create configfs entry for EPC device and EPF driver
     
-    Invoke API's provided by pci-ep-cfs to create configfs entry for
-    every EPC device and EPF driver to help users in creating EPF device
-    and binding the EPF device to the EPC device.
+    Invoke APIs provided by pci-ep-cfs to create configfs entry for every EPC
+    device and EPF driver to help users in creating EPF device and binding the
+    EPF device to the EPC device.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit c782a62981cd
+commit afbfe69d612b
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:15:00 2017 +0530
 
-    Documentation: PCI: Guide to use pci endpoint configfs
+    Documentation: PCI: Guide to use PCI endpoint configfs
     
-    Add Documentation to help users use pci endpoint to configure
-    pci endpoint function and to bind the endpoint function
-    with endpoint controller.
+    Add Documentation to help users use PCI endpoint to configure PCI endpoint
+    function and to bind the endpoint function with endpoint controller.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit 9b703e4292d1
+commit 7049ceb16f8d
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:14:59 2017 +0530
 
     PCI: endpoint: Introduce configfs entry for configuring EP functions
     
     Introduce a new configfs entry to configure the EP function (like
-    configuring the standard configuration header entries) and to
-    bind the EP function with EP controller.
+    configuring the standard configuration header entries) and to bind the EP
+    function with EP controller.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit d03d46a9d9fc
+commit e025fb44364f
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:14:58 2017 +0530
 
@@ -294,18 +293,17 @@
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
 
-commit ff50e9e4a512
+commit 79e4effb2a36
 Author: Kishon Vijay Abraham I <kishon@ti.com>
 Date:   Mon Mar 27 15:14:57 2017 +0530
 
     PCI: endpoint: Add EP core layer to enable EP controller and EP functions
     
-    Introduce a new EP core layer in order to support endpoint functions
-    in linux kernel. This comprises of EPC library
-    (Endpoint Controller Library) and EPF library (Endpoint
-    Function Library). EPC library implements functions that is specific
-    to an endpoint controller and EPF library implements functions
-    that is specific to an endpoint function.
+    Introduce a new EP core layer in order to support endpoint functions in
+    linux kernel. This comprises the EPC library (Endpoint Controller Library)
+    and EPF library (Endpoint Function Library). EPC library implements
+    functions specific to an endpoint controller and EPF library implements
+    functions specific to an endpoint function.
     
     Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

^ permalink raw reply related

* Re: Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver
From: Priit Laes @ 2017-04-04 20:09 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng, Russell King,
	Chen-Yu Tsai, Mark Rutland, Rob Herring, Stephen Boyd,
	Michael Turquette, Philipp Zabel
In-Reply-To: <20170327075438.cw3d6s7zyeemenwr@lukather>

On Mon, Mar 27, 2017 at 09:54:38AM +0200, Maxime Ripard wrote:
> Hi,
> 
> Thanks a lot for working on this.
> 
> On Sun, Mar 26, 2017 at 08:20:16PM +0300, Priit Laes wrote:
> > Introduce a clock controller driver for sun4i A10 and sun7i A20
> > series SoCs.
> > 
> > Signed-off-by: Priit Laes <plaes-q/aMd4JkU83YtjvyW6yDsg@public.gmane.org>
> > ---
> >  drivers/clk/sunxi-ng/Kconfig                  |   13 +-
> >  drivers/clk/sunxi-ng/Makefile                 |    1 +-
> >  drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c      | 1532 ++++++++++++++++++-
> >  drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h      |   59 +-
> >  include/dt-bindings/clock/sunxi-a10-a20-ccu.h |  208 ++-
> >  include/dt-bindings/reset/sunxi-a10-a20-ccu.h |   66 +-
> 
> I'm not too fond of those sunxi-<all the SoCs supported>. We're not
> doing that for any other driver, I don't really know why this has
> became a trend lately.
> 
> You can call them ccu-sun4i-a10.h, and it will work just fine.

OK, will do!

> 
> > +/* Not documented on A10 */
> > +static SUNXI_CCU_GATE(pll_periph_sata_clk, "pll-periph-sata", "pll-periph",
> > +		      0x028, BIT(14), 0);
> 
> The rate doesn't come from pll-periph directly, does it?

So it uses hosc (24MHz parent clock) instead of pll-periph?

> 
> > +#define SUN4I_AHB_REG		0x054
> > +static struct ccu_mux cpu_clk = {
> > +	.mux		= {
> > +		.shift		= 16,
> > +		.width		= 2,
> > +		.fixed_predivs	= cpu_predivs,
> > +		.n_predivs	= ARRAY_SIZE(cpu_predivs),
> > +	},
> > +	.common		= {
> > +		.reg		= 0x054,
> 
> Why did you define this one, even though you don't seem to be using it
> anywhere?

Leftover from when I also included A10 support.

> 
> > +static const char *const ahb_parents[] = { "axi", "pll-periph",
> > +					   "pll-periph-2x" };
> > +static const struct ccu_mux_fixed_prediv ahb_predivs[] = {
> > +	{ .index = 2, .div = 2, },
> > +};
> 
> This seems to be only true for the A20, and not the A10.
> 
> Are you sure here? The pll-periph-2x seem to be only used in the MBUS
> clock in our current code.

Nope...
> 
> And then, using pll-periph-2x, and then dividing it by 2 just gives us
> pll-periph, which is also our previous parent :)

...will investigate.

> 
> > +/* Undocumented on A10 */
> > +static SUNXI_CCU_PHASE(mmc0_output_clk, "mmc0_output", "mmc0",
> > +		       0x088, 8, 3, 0);
> > +/* Undocumented on A10 */
> > +static SUNXI_CCU_PHASE(mmc0_sample_clk, "mmc0_sample", "mmc0",
> > +		       0x088, 20, 3, 0);
> 
> The A10 doesn't have them.

Are you sure? Although, they weren't listed in datasheet, they are defined
in the sun4i-a10.dtsi:

mmc0_clk: clk@01c20088 {
        #clock-cells = <1>;
        compatible = "allwinner,sun4i-a10-mmc-clk";
        reg = <0x01c20088 0x4>;
        clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
        clock-output-names = "mmc0",
                             "mmc0_output",
                             "mmc0_sample";
};

> > +/* TODO: Check whether A10 actually supports osc32k as 4th parent? */
> > +static const char *const ir_parents_sun4i[] = { "hosc", "pll-periph",
> > +						"pll-ddr-other" };
> 
> What does the BSP say about this?

sun7i datasheet mentions osc32k, but BSP code for sun4i, sun5i and sun7i
is identical and supports only 3 first parents without osc32k.


> > +/* Undocumented on A10 */
> > +static SUNXI_CCU_MUX_WITH_GATE(spdif_clk, "spdif", audio_parents,
> > +			       0x0c0, 16, 2, BIT(31), CLK_SET_RATE_PARENT);
> 
> This doesn't seem to exist at all on the A10

Wasn't listed in datasheet, but it's in BSP and also in sun4i-a10.dtsi:

spdif_clk: clk@01c200c0 {
        #clock-cells = <0>;
        compatible = "allwinner,sun4i-a10-mod1-clk";
        reg = <0x01c200c0 0x4>;
        clocks = <&pll2 SUN4I_A10_PLL2_8X>,
                 <&pll2 SUN4I_A10_PLL2_4X>,
                 <&pll2 SUN4I_A10_PLL2_2X>,
                 <&pll2 SUN4I_A10_PLL2_1X>;
        clock-output-names = "spdif";
};


> 
> > +/*
> > + * TODO: SATA clock also supports external clock as parent via BIT(24)
> > + * The external clock is probably an optional crystal or oscillator
> > + * that can be connected to the SATA-CLKM / SATA-CLKP pins.
> > + */
> > +static SUNXI_CCU_GATE(sata_clk, "sata", "pll-periph-sata",
> > +		      0x0c8, BIT(31), 0);
> 
> The rate won't be good here either. This is supposed to be 100MHz.

Hmm.. I tested SATA with Cubietruck. Or what do you mean?

> > +static const char *const csi_isp_parents[] = { "pll-video0", "pll-ve",
> > +					       "pll-ddr-other", "pll-sata" };
> > +
> > +static SUNXI_CCU_M_WITH_MUX_GATE(csi_isp_clk, "csi-isp",
> > +				 csi_isp_parents,
> > +				 0x120, 0, 4, 24, 2, BIT(31), 0);
> 
> We've been calling it sclk in the other SoC iirc. Any particular
> reason to call it differently?

It's called ISP in BSP and A10 manual.
In A20 it's indeed Special Clock Register (SCLK).

> > +static const char *const out_parents[] = { "hosc", "osc32k", "hosc" };
> > +static SUNXI_CCU_MP_WITH_MUX_GATE(out_a_clk, "out-a", out_parents,
> > +				  0x1f0, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> > +static SUNXI_CCU_MP_WITH_MUX_GATE(out_b_clk, "out-b", out_parents,
> > +				  0x1f4, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> 
> There's a fixed pre-divider on the first hosc of 750.

Nice catch.

So it should be something like this:

[snip]
static const char *const out_parents[] = { "osc24M", "osc32k", "osc24M" };
static const struct ccu_mux_fixed_prediv out_prediv = {
        .index = 0, .div = 750
};

static struct ccu_mp out_a_clk = {
        .enable         = BIT(31),
        .m              = _SUNXI_CCU_DIV(8, 5),
        .p              = _SUNXI_CCU_DIV(20, 2),
        .mux            = {
                .shift          = 24,
                .width          = 2,
                .fixed_predivs  = &out_prediv,
                .n_predivs      = ARRAY_SIZE(out_prediv),
        },
        .common         = {
                .reg            = 0x1f0,
                .features       = CCU_FEATURE_FIXED_PREDIV,
                .hw.init        = CLK_HW_INIT_PARENTS("out-a",
                                                      out_parents,
                                                      &ccu_mp_ops,
                                                      0),
        },
};
[/snip]


> > +static void init_clocks(void __iomem *reg)
> > +{
> > +	u32 val;
> > +
> > +	/* Force the PLL-Audio-1x divider to 4 */
> > +	val = readl(reg + SUN4I_PLL_AUDIO_REG);
> > +	val &= ~GENMASK(19, 16);
> > +	writel(val | (3 << 16), reg + SUN4I_PLL_AUDIO_REG);
> > +
> > +	/* Use PLL6 as parent for AHB */
> > +	val = readl(reg + SUN4I_AHB_REG);
> > +	val &= ~GENMASK(7, 6);
> > +	writel(val | (2 << 6), reg + SUN4I_AHB_REG);
> 
> Keeping some kind of comment similar to what was in the DT would be
> great, otherwise we lose *why* we need to do so.

OK

> > +}
> > +
> > +static void __init sun4i_a10_ccu_setup(struct device_node *node)
> > +{
> > +	void __iomem *reg;
> > +
> > +	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> > +	if (IS_ERR(reg)) {
> > +		pr_err("%s: Could not map the clock registers\n",
> > +		       of_node_full_name(node));
> > +		return;
> > +	}
> > +
> > +	init_clocks(reg);
> > +
> > +	sunxi_ccu_probe(node, reg, &sun4i_a10_ccu_desc);
> 
> Can't you move the request_and_map / probe in the common function?

Will do.

> > +#ifndef _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> > +#define _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> > +
> > +#define CLK_HOSC		1
> > +#define CLK_PLL_PERIPH_SATA	16
> 
> That one looks suspicious. I don't see why we would need the PLL,
> while we have a perfectly functional SATA clock below. Have you tried
> gating the bit31 of the register 0xc8 to see if it has any impact?

Will try it...

> Thanks!
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
> 
> -- 
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

^ 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