Devicetree
 help / color / mirror / Atom feed
* [PATCH v6 7/8] power: Adds support for Smart Battery System Manager
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, sre, peda, preid, linux-i2c,
	devicetree, linux-pm, benjamin.tissoires
  Cc: Karl-Heinz Schneider
In-Reply-To: <1493716346-58517-1-git-send-email-preid@electromag.com.au>

From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>

This patch adds support for Smart Battery System Manager.
A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of
communicating up to four I2C smart battery devices. All smart battery
devices are listening at address 0x0b, so the SBSM muliplexes between
them. The driver makes use of the I2C-Mux framework to allow smart
batteries to be bound via device tree, i.e. the sbs-battery driver.

Via sysfs interface the online state and charge type are presented. If
the driver is bound as ltc1760 (an implementation of a Dual Smart Battery
System Manager) the charge type can also be changed from trickle to fast.

Tested-by: Phil Reid <preid@electromag.com.au>
Reviewed-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/power/supply/Kconfig       |  13 ++
 drivers/power/supply/Makefile      |   1 +
 drivers/power/supply/sbs-manager.c | 317 +++++++++++++++++++++++++++++++++++++
 3 files changed, 331 insertions(+)
 create mode 100644 drivers/power/supply/sbs-manager.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index da54ac8..a435415 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -170,6 +170,19 @@ config CHARGER_SBS
         help
 	  Say Y to include support for SBS compilant battery chargers.
 
+config MANAGER_SBS
+	tristate "Smart Battery System Manager"
+	depends on I2C && I2C_MUX
+	help
+	  Say Y here to include support for Smart Battery System Manager
+	  ICs. The driver reports online and charging status via sysfs.
+	  It presents itself also as I2C mux which allows to bind
+	  smart battery driver to its ports.
+	  Supported is for example LTC1760.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called sbs-manager.
+
 config BATTERY_BQ27XXX
 	tristate "BQ27xxx battery driver"
 	help
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 3789a2c..350c0bb 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
 obj-$(CONFIG_CHARGER_SBS)	+= sbs-charger.o
+obj-$(CONFIG_MANAGER_SBS)	+= sbs-manager.o
 obj-$(CONFIG_BATTERY_BQ27XXX)	+= bq27xxx_battery.o
 obj-$(CONFIG_BATTERY_BQ27XXX_I2C) += bq27xxx_battery_i2c.o
 obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
new file mode 100644
index 0000000..8842f6e
--- /dev/null
+++ b/drivers/power/supply/sbs-manager.c
@@ -0,0 +1,317 @@
+/*
+ * Driver for SBS compliant Smart Battery System Managers
+ *
+ * The device communicates via i2c at address 0x0a and multiplexes access to up
+ * to four smart batteries at address 0x0b.
+ *
+ * Via sysfs interface the online state and charge type are presented.
+ *
+ * Datasheet SBSM:    http://sbs-forum.org/specs/sbsm100b.pdf
+ * Datasheet LTC1760: http://cds.linear.com/docs/en/datasheet/1760fb.pdf
+ *
+ * Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/of_device.h>
+#include <linux/power_supply.h>
+
+#define SBSM_MAX_BATS  4
+#define SBSM_RETRY_CNT 3
+
+/* registers addresses */
+#define SBSM_CMD_BATSYSSTATE     0x01
+#define SBSM_CMD_BATSYSSTATECONT 0x02
+#define SBSM_CMD_BATSYSINFO      0x04
+#define SBSM_CMD_LTC             0x3c
+
+struct sbsm_data {
+	struct i2c_client *client;
+	struct i2c_mux_core *muxc;
+
+	struct power_supply *psy;
+
+	u8 cur_chan;          /* currently selected channel */
+	bool is_ltc1760;      /* special capabilities */
+};
+
+static enum power_supply_property sbsm_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+};
+
+static int sbsm_read_word(struct i2c_client *client, u8 address)
+{
+	int reg, retries;
+
+	for (retries = SBSM_RETRY_CNT; retries > 0; retries--) {
+		reg = i2c_smbus_read_word_data(client, address);
+		if (reg >= 0)
+			break;
+	}
+
+	if (reg < 0) {
+		dev_err(&client->dev, "failed to read register 0x%02x\n",
+			address);
+	}
+
+	return reg;
+}
+
+static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
+{
+	int ret, retries;
+
+	for (retries = SBSM_RETRY_CNT; retries > 0; retries--) {
+		ret = i2c_smbus_write_word_data(client, address, word);
+		if (ret >= 0)
+			break;
+	}
+	if (ret < 0)
+		dev_err(&client->dev, "failed to write to register 0x%02x\n",
+			address);
+
+	return ret;
+}
+
+static int sbsm_get_property(struct power_supply *psy,
+			     enum power_supply_property psp,
+			     union power_supply_propval *val)
+{
+	struct sbsm_data *data = power_supply_get_drvdata(psy);
+	int regval = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT);
+		if (regval < 0)
+			return regval;
+		val->intval = !!(regval & 0x1);
+		break;
+
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+		if (regval < 0)
+			return regval;
+
+		if ((regval & 0x00f0) == 0) {
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+			return 0;
+		}
+		val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+
+		if (data->is_ltc1760) {
+			/* charge mode fast if turbo is active */
+			regval = sbsm_read_word(data->client, SBSM_CMD_LTC);
+			if (regval < 0)
+				return regval;
+			else if (regval & 0x80)
+				val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		}
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int sbsm_prop_is_writeable(struct power_supply *psy,
+				  enum power_supply_property psp)
+{
+	struct sbsm_data *data = power_supply_get_drvdata(psy);
+
+	return (psp == POWER_SUPPLY_PROP_CHARGE_TYPE) && data->is_ltc1760;
+}
+
+static int sbsm_set_property(struct power_supply *psy,
+			     enum power_supply_property psp,
+			     const union power_supply_propval *val)
+{
+	struct sbsm_data *data = power_supply_get_drvdata(psy);
+	int ret = -EINVAL;
+	u16 regval;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		/* write 1 to TURBO if type fast is given */
+		if (!data->is_ltc1760)
+			break;
+		regval = val->intval ==
+			 POWER_SUPPLY_CHARGE_TYPE_FAST ? (0x1 << 7) : 0;
+		ret = sbsm_write_word(data->client, SBSM_CMD_LTC, regval);
+		break;
+
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * Switch to battery
+ * Parameter chan is directly the content of SMB_BAT* nibble
+ */
+static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct sbsm_data *data = i2c_mux_priv(muxc);
+	struct device *dev = &data->client->dev;
+	int ret = 0;
+	u16 reg;
+
+	if (data->cur_chan == chan)
+		return ret;
+
+	/* chan goes from 1 ... 4 */
+	reg = 1 << (11 + chan);
+	ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg);
+	if (ret)
+		dev_err(dev, "Failed to select channel %i\n", chan);
+	else
+		data->cur_chan = chan;
+
+	return ret;
+}
+
+static const struct of_device_id sbsm_dt_ids[] = {
+	{ .compatible = "sbs,sbs-manager" },
+	{ .compatible = "lltc,ltc1760" },
+	{ }
+};
+
+static const struct power_supply_desc sbsm_default_psy_desc = {
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.properties = sbsm_props,
+	.num_properties = ARRAY_SIZE(sbsm_props),
+	.get_property = &sbsm_get_property,
+	.set_property = &sbsm_set_property,
+	.property_is_writeable = &sbsm_prop_is_writeable,
+};
+
+static int sbsm_probe(struct i2c_client *client,
+		      const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct sbsm_data *data;
+	struct device *dev = &client->dev;
+	struct power_supply_desc *psy_desc;
+	struct power_supply_config psy_cfg = {};
+	int ret = 0, i, supported_bats;
+
+	/* Device listens only at address 0x0a */
+	if (client->addr != 0x0a)
+		return -EINVAL;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
+		return -EPFNOSUPPORT;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, data);
+
+	data->client = client;
+	data->is_ltc1760 = !!strstr(id->name, "ltc1760");
+
+	ret  = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
+	if (ret < 0)
+		return ret;
+	supported_bats = ret & 0xf;
+
+	data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
+				   I2C_MUX_LOCKED, &sbsm_select, NULL);
+	if (!data->muxc) {
+		dev_err(dev, "failed to alloc i2c mux\n");
+		ret = -ENOMEM;
+		goto err_mux_alloc;
+	}
+	data->muxc->priv = data;
+
+	/* register muxed i2c channels. One for each supported battery */
+	for (i = 0; i < SBSM_MAX_BATS; ++i) {
+		if (supported_bats & BIT(i)) {
+			ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
+			if (ret) {
+				dev_err(dev,
+					"failed to register i2c mux channel %d\n",
+					i + 1);
+				goto err_mux_register;
+			}
+		}
+	}
+
+	psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc,
+				sizeof(struct power_supply_desc),
+				GFP_KERNEL);
+	if (!psy_desc) {
+		ret = -ENOMEM;
+		goto err_psy;
+	}
+
+	psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s",
+					dev_name(&client->dev));
+	if (!psy_desc->name) {
+		ret = -ENOMEM;
+		goto err_psy;
+	}
+
+	psy_cfg.drv_data = data;
+	psy_cfg.of_node = dev->of_node;
+	data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
+	if (IS_ERR(data->psy)) {
+		ret = PTR_ERR(data->psy);
+		dev_err(dev, "failed to register power supply %s\n",
+			psy_desc->name);
+		goto err_psy;
+	}
+
+	dev_info(dev, "sbsm registered\n");
+	return 0;
+
+err_psy:
+err_mux_register:
+	i2c_mux_del_adapters(data->muxc);
+
+err_mux_alloc:
+	return ret;
+}
+
+static int sbsm_remove(struct i2c_client *client)
+{
+	struct sbsm_data *data = i2c_get_clientdata(client);
+
+	i2c_mux_del_adapters(data->muxc);
+	return 0;
+}
+
+static const struct i2c_device_id sbsm_ids[] = {
+	{ "sbs-manager", 0 },
+	{ "ltc1760",     0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sbsm_ids);
+MODULE_DEVICE_TABLE(of, sbsm_dt_ids);
+
+static struct i2c_driver sbsm_driver = {
+	.driver = {
+		.name = "sbsm",
+		.of_match_table = of_match_ptr(sbsm_dt_ids),
+	},
+	.probe		= sbsm_probe,
+	.remove		= sbsm_remove,
+	.id_table	= sbsm_ids
+};
+module_i2c_driver(sbsm_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz@schneider-inet.de>");
+MODULE_DESCRIPTION("SBSM Smart Battery System Manager");
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v6 6/8] Documentation: Add sbs-manager device tree node documentation
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, sre, peda, preid, linux-i2c,
	devicetree, linux-pm, benjamin.tissoires
  Cc: Karl-Heinz Schneider
In-Reply-To: <1493716346-58517-1-git-send-email-preid@electromag.com.au>

From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>

This patch adds device tree documentation for the sbs-manager

Reviewed-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 .../bindings/power/supply/sbs,sbs-manager.txt      | 66 ++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt

diff --git a/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
new file mode 100644
index 0000000..389016c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
@@ -0,0 +1,66 @@
+Binding for sbs-manager
+
+Required properties:
+- compatible: "<vendor>,<part-number>", "sbs,sbs-charger" as fallback. The part
+  number compatible string might be used in order to take care of vendor
+  specific registers.
+- reg: integer, i2c address of the device. Should be <0xa>.
+Optional properties:
+- gpio-controller: Marks the port as GPIO controller.
+  See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
+- #gpio-cells: Should be <2>. The first cell is the pin number, the second cell
+  is used to specify optional parameters:
+  See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
+
+From OS view the device is basically an i2c-mux used to communicate with up to
+four smart battery devices at address 0xb. The driver actually implements this
+behaviour. So standard i2c-mux nodes can be used to register up to four slave
+batteries. Channels will be numerated starting from 1 to 4.
+
+Example:
+
+batman@0a {
+    compatible = "lltc,ltc1760", "sbs,sbs-manager";
+    reg = <0x0a>;
+    #address-cells = <1>;
+    #size-cells = <0>;
+
+    gpio-controller;
+    #gpio-cells = <2>;
+
+    i2c@1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <1>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 1 1>;
+        };
+    };
+
+    i2c@2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <2>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 2 1>;
+        };
+    };
+
+    i2c@3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <3>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 3 1>;
+        };
+    };
+};
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v6 5/8] i2c: mux: pca954x: Call request irq after adding mux segments
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, sre, peda, preid, linux-i2c,
	devicetree, linux-pm, benjamin.tissoires
In-Reply-To: <1493716346-58517-1-git-send-email-preid@electromag.com.au>

The pca954x device do not have the ability to mask interrupts. For
i2c slave devices that also don't have masking ability (eg ltc1760
smbalert output) delay registering the irq until after the mux
segments have been configured. During the mux add_adaptor call the
core i2c system can register an smbalert handler which would then
be called immediately when the irq is registered. This smbalert
handler will then clear the pending irq.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 52 +++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index ad31d21..4299738 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -294,7 +294,7 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
-	int c, err, irq;
+	int c, irq;
 
 	if (!data->chip->has_irq || client->irq <= 0)
 		return 0;
@@ -314,24 +314,22 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 			handle_simple_irq);
 	}
 
-	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
-					pca954x_irq_handler,
-					IRQF_ONESHOT | IRQF_SHARED,
-					"pca954x", data);
-	if (err)
-		goto err_req_irq;
+	return 0;
+}
 
-	disable_irq(data->client->irq);
+static void pca954x_cleanup(struct i2c_mux_core *muxc)
+{
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int c, irq;
 
-	return 0;
-err_req_irq:
-	for (c = 0; c < data->chip->nchans; c++) {
-		irq = irq_find_mapping(data->irq, c);
-		irq_dispose_mapping(irq);
+	if (data->irq) {
+		for (c = 0; c < data->chip->nchans; c++) {
+			irq = irq_find_mapping(data->irq, c);
+			irq_dispose_mapping(irq);
+		}
+		irq_domain_remove(data->irq);
 	}
-	irq_domain_remove(data->irq);
-
-	return err;
+	i2c_mux_del_adapters(muxc);
 }
 
 /*
@@ -422,6 +420,14 @@ static int pca954x_probe(struct i2c_client *client,
 		}
 	}
 
+	if (data->chip->has_irq || client->irq > 0) {
+		ret = devm_request_threaded_irq(&client->dev, data->client->irq,
+		NULL, pca954x_irq_handler, IRQF_ONESHOT | IRQF_SHARED,
+		"pca954x", data);
+		if (ret)
+			goto fail_del_adapters;
+	}
+
 	dev_info(&client->dev,
 		 "registered %d multiplexed busses for I2C %s %s\n",
 		 num, data->chip->muxtype == pca954x_ismux
@@ -430,25 +436,15 @@ static int pca954x_probe(struct i2c_client *client,
 	return 0;
 
 fail_del_adapters:
-	i2c_mux_del_adapters(muxc);
+	pca954x_cleanup(muxc);
 	return ret;
 }
 
 static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
-	struct pca954x *data = i2c_mux_priv(muxc);
-	int c, irq;
 
-	if (data->irq) {
-		for (c = 0; c < data->chip->nchans; c++) {
-			irq = irq_find_mapping(data->irq, c);
-			irq_dispose_mapping(irq);
-		}
-		irq_domain_remove(data->irq);
-	}
-
-	i2c_mux_del_adapters(muxc);
+	pca954x_cleanup(muxc);
 	return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v6 4/8] i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, sre, peda, preid, linux-i2c,
	devicetree, linux-pm, benjamin.tissoires
In-Reply-To: <1493716346-58517-1-git-send-email-preid@electromag.com.au>

Add a call to of_i2c_setup_smbus_alert when a i2c adapter is registered
so the the smbalert driver can be registered.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d2402bb..626471b 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -40,6 +40,7 @@
 #include <linux/gpio.h>
 #include <linux/hardirq.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/irqflags.h>
@@ -2045,6 +2046,9 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 		dev_warn(&adap->dev,
 			 "Failed to create compatibility class link\n");
 #endif
+	res = of_i2c_setup_smbus_alert(adap);
+	if (res)
+		goto out_list;
 
 	i2c_init_recovery(adap);
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v6 3/8] i2c: i2c-smbus: add of_i2c_setup_smbus_alert
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, sre-DgEjT+Ai2ygdnm+yROfE0A,
	peda-koto5C5qi+TLoDKTGw+V6w,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1493716346-58517-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

This commit adds of_i2c_setup_smbus_alert which allows the smbalert
driver to be attached to an i2c adapter via the device tree.

Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c.txt |  4 +--
 drivers/i2c/i2c-smbus.c                       | 35 +++++++++++++++++++++++++++
 include/linux/i2c-smbus.h                     |  9 +++++++
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index cee9d50..1126398 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -59,8 +59,8 @@ wants to support one of the below features, it should adapt the bindings below.
 	interrupts used by the device.
 
 - interrupt-names
-	"irq" and "wakeup" names are recognized by I2C core, other names are
-	left to individual drivers.
+	"irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
+	other names are	left to individual drivers.
 
 - host-notify
 	device uses SMBus host notify protocol instead of interrupt line.
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index df0e2fa..a8f8439 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -21,6 +21,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_irq.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 
@@ -238,6 +239,40 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
 }
 EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
 
+int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
+{
+	struct i2c_client *client;
+	struct i2c_smbus_alert_setup *setup;
+	struct i2c_board_info info = {
+		I2C_BOARD_INFO("smbus_alert", 0x0c),
+	};
+	int irq;
+
+	if (!adap->dev.of_node)
+		return 0;
+
+	irq = of_irq_get_byname(adap->dev.of_node, "smbus_alert");
+	if (irq == -EINVAL || irq == -ENODATA)
+		return 0;
+	else if (irq < 0)
+		return irq;
+
+	setup = devm_kzalloc(&adap->dev, sizeof(struct i2c_smbus_alert_setup),
+		GFP_KERNEL);
+	if (!setup)
+		return -ENOMEM;
+
+	setup->irq = irq;
+	info.platform_data = setup;
+
+	client = i2c_new_device(adap, &info);
+	if (!client)
+		return -ENODEV;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert);
+
 /**
  * i2c_handle_smbus_alert - Handle an SMBus alert
  * @ara: the ARA client on the relevant adapter
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index a138502..4732d09 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -50,4 +50,13 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
 					 struct i2c_smbus_alert_setup *setup);
 int i2c_handle_smbus_alert(struct i2c_client *ara);
 
+#if IS_ENABLED(CONFIG_I2C_SMBUS)
+int of_i2c_setup_smbus_alert(struct i2c_adapter *adap);
+#else
+static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
+{
+	return 0;
+}
+#endif
+
 #endif /* _LINUX_I2C_SMBUS_H */
-- 
1.8.3.1

--
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 v6 2/8] i2c: i2c-smbus: Add null ptr guard in smb_alert_probe
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, sre, peda, preid, linux-i2c,
	devicetree, linux-pm, benjamin.tissoires
In-Reply-To: <1493716346-58517-1-git-send-email-preid@electromag.com.au>

If platdata is not correct setup before registering the device
a null ptr derefence can occur.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-smbus.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b272493..df0e2fa 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -151,6 +151,11 @@ static int smbalert_probe(struct i2c_client *ara,
 	struct i2c_adapter *adapter = ara->adapter;
 	int res;
 
+	if (!setup) {
+		dev_err(&adapter->dev, "setup not defined\n");
+		return -ENODATA;
+	}
+
 	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
 			     GFP_KERNEL);
 	if (!alert)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v6 1/8] i2c: i2c-smbus: Support threaded irqs.
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland, sre, peda, preid, linux-i2c,
	devicetree, linux-pm, benjamin.tissoires
In-Reply-To: <1493716346-58517-1-git-send-email-preid@electromag.com.au>

handle_nested_irq calls the threaded irq handler. So if the smbus_alert
irq is being generated via this an null address is dereferenced. Split
irq up into separate functions to allow thread / non thread irq to work
correctly.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-smbus.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index f9271c7..b272493 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -72,13 +72,12 @@ static int smbus_do_alert(struct device *dev, void *addrp)
  * The alert IRQ handler needs to hand work off to a task which can issue
  * SMBus calls, because those sleeping calls can't be made in IRQ context.
  */
-static void smbus_alert(struct work_struct *work)
+static irqreturn_t smbus_alert(int irq, void *d)
 {
-	struct i2c_smbus_alert *alert;
+	struct i2c_smbus_alert *alert = d;
 	struct i2c_client *ara;
 	unsigned short prev_addr = 0;	/* Not a valid address */
 
-	alert = container_of(work, struct i2c_smbus_alert, alert);
 	ara = alert->ara;
 
 	for (;;) {
@@ -115,6 +114,17 @@ static void smbus_alert(struct work_struct *work)
 		prev_addr = data.addr;
 	}
 
+	return IRQ_HANDLED;
+}
+
+static void smbalert_work(struct work_struct *work)
+{
+	struct i2c_smbus_alert *alert;
+
+	alert = container_of(work, struct i2c_smbus_alert, alert);
+
+	smbus_alert(alert->irq, alert);
+
 	/* We handled all alerts; re-enable level-triggered IRQs */
 	if (!alert->alert_edge_triggered)
 		enable_irq(alert->irq);
@@ -148,12 +158,14 @@ static int smbalert_probe(struct i2c_client *ara,
 
 	alert->alert_edge_triggered = setup->alert_edge_triggered;
 	alert->irq = setup->irq;
-	INIT_WORK(&alert->alert, smbus_alert);
+	INIT_WORK(&alert->alert, smbalert_work);
 	alert->ara = ara;
 
 	if (setup->irq > 0) {
-		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
-				       0, "smbus_alert", alert);
+		res = devm_request_threaded_irq(&ara->dev, alert->irq,
+						smbalert_irq, smbus_alert,
+						IRQF_SHARED | IRQF_ONESHOT,
+						"smbus_alert", alert);
 		if (res)
 			return res;
 	}
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v6 0/8] Add sbs-manager with smbalert support
From: Phil Reid @ 2017-05-02  9:12 UTC (permalink / raw)
  To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, sre-DgEjT+Ai2ygdnm+yROfE0A,
	peda-koto5C5qi+TLoDKTGw+V6w,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA

This is another go of the sbs-manager driver using smbalert for
irq support from a while ago. 

Enables the existing smbalert driver to be loaded via the device tree.
Only need to add smbus_alert interrupt to the i2c bus segement in the
devicetree and the core will then enable the alert driver.

Reorders the rquest irq call in the pca954x driver to ensure each
muxed i2c segment can handle service smbalerts on that segment before
irq's are enabled. The pca954x can't mask individual irq's routed thru
them.

Add the sbs-manager from Karl-Heinz.
Add the alert call back and gpio interface to allow the battery detect
logic in the existing sbs-battery driver to work.

Changes from v5:
- Documentation: Add sbs-manager device tree node documentation
  - Use same style as sbs-charger for compatible property. 
- power: Adds support for Smart Battery System Manager
  - reorder kconfig / makefile
  - remove errouinous le16 to cpu conversions
  - while loops to for loops
  - formating changes to error messages
  - changed sbsm_set_proprty indentation (hopefully I got it right)
  - removed CONFIG_OF conditional around of_device_id table
  - ENODEV -> EINVAL in probe function for mismatched address
  - Use BIT() macro in probe function
  - add of_node assignment in probe function
  - remove owner assignament and set of_match_table
- power: supply: sbs-battery: Add alert callback
  - Removed patch as Sebastian has queued it.
- power: supply: sbs-manager: Add alert callback and battery change notification
  - Use device_property_present instead of of_get_property
  - Add depends on GPIOLIB






Karl-Heinz Schneider (2):
  Documentation: Add sbs-manager device tree node documentation
  power: Adds support for Smart Battery System Manager

Phil Reid (6):
  i2c: i2c-smbus: Support threaded irqs.
  i2c: i2c-smbus: Add null ptr guard in smb_alert_probe
  i2c: i2c-smbus: add of_i2c_setup_smbus_alert
  i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
  i2c: mux: pca954x: Call request irq after adding mux segments
  power: supply: sbs-manager: Add alert callback and battery change
    notification

 Documentation/devicetree/bindings/i2c/i2c.txt      |   4 +-
 .../bindings/power/supply/sbs,sbs-manager.txt      |  66 ++++
 drivers/i2c/i2c-core.c                             |   4 +
 drivers/i2c/i2c-smbus.c                            |  64 ++-
 drivers/i2c/muxes/i2c-mux-pca954x.c                |  52 ++-
 drivers/power/supply/Kconfig                       |  14 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/sbs-manager.c                 | 438 +++++++++++++++++++++
 include/linux/i2c-smbus.h                          |   9 +
 9 files changed, 616 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
 create mode 100644 drivers/power/supply/sbs-manager.c

-- 
1.8.3.1

--
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 v7 7/7] arm64: dts: qcom: msm8916: Add debug unit
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

Add debug unit on Qualcomm msm8916 based platforms, including the
DragonBoard 410c board.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 68a8e67..3af814b 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1104,6 +1104,38 @@
 			};
 		};
 
+		debug@850000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0x850000 0x1000>;
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&CPU0>;
+		};
+
+		debug@852000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0x852000 0x1000>;
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&CPU1>;
+		};
+
+		debug@854000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0x854000 0x1000>;
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&CPU2>;
+		};
+
+		debug@856000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0x856000 0x1000>;
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&CPU3>;
+		};
+
 		etm@85c000 {
 			compatible = "arm,coresight-etm4x", "arm,primecell";
 			reg = <0x85c000 0x1000>;
-- 
2.7.4


^ permalink raw reply related

* [PATCH v7 6/7] arm64: dts: hi6220: register debug module
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

Bind debug module driver for Hi6220.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 64 +++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 470461d..467aa15 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -913,5 +913,69 @@
 				};
 			};
 		};
+
+		debug@f6590000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf6590000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu0>;
+		};
+
+		debug@f6592000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf6592000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu1>;
+		};
+
+		debug@f6594000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf6594000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu2>;
+		};
+
+		debug@f6596000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf6596000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu3>;
+		};
+
+		debug@f65d0000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf65d0000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu4>;
+		};
+
+		debug@f65d2000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf65d2000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu5>;
+		};
+
+		debug@f65d4000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf65d4000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu6>;
+		};
+
+		debug@f65d6000 {
+			compatible = "arm,coresight-cpu-debug","arm,primecell";
+			reg = <0 0xf65d6000 0 0x1000>;
+			clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+			clock-names = "apb_pclk";
+			cpu = <&cpu7>;
+		};
 	};
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 5/7] coresight: add support for CPU debug module
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

Coresight includes debug module and usually the module connects with CPU
debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
description for related info in "Part H: External Debug".

Chapter H7 "The Sample-based Profiling Extension" introduces several
sampling registers, e.g. we can check program counter value with
combined CPU exception level, secure state, etc. So this is helpful for
analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
loop with IRQ disabled. In this case the CPU cannot switch context and
handle any interrupt (including IPIs), as the result it cannot handle
SMP call for stack dump.

This patch is to enable coresight debug module, so firstly this driver
is to bind apb clock for debug module and this is to ensure the debug
module can be accessed from program or external debugger. And the driver
uses sample-based registers for debug purpose, e.g. when system triggers
panic, the driver will dump program counter and combined context
registers (EDCIDSR, EDVIDSR); by parsing context registers so can
quickly get to know CPU secure state, exception level, etc.

Some of the debug module registers are located in CPU power domain, so
this requires the CPU power domain stays on when access related debug
registers, but the power management for CPU power domain is quite
dependent on SoC integration for power management. For the platforms
which with sane power controller implementations, this driver follows
the method to set EDPRCR to try to pull the CPU out of low power state
and then set 'no power down request' bit so the CPU has no chance to
lose power.

If the SoC has not followed up this design well for power management
controller, the user should use the command line parameter or sysfs
to constrain all or partial idle states to ensure the CPU power
domain is enabled and access coresight CPU debug component safely.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/Kconfig               |  14 +
 drivers/hwtracing/coresight/Makefile              |   1 +
 drivers/hwtracing/coresight/coresight-cpu-debug.c | 671 ++++++++++++++++++++++
 3 files changed, 686 insertions(+)
 create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 130cb21..8d55d6d 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -89,4 +89,18 @@ config CORESIGHT_STM
 	  logging useful software events or data coming from various entities
 	  in the system, possibly running different OSs
 
+config CORESIGHT_CPU_DEBUG
+	tristate "CoreSight CPU Debug driver"
+	depends on ARM || ARM64
+	depends on DEBUG_FS
+	help
+	  This driver provides support for coresight debugging module. This
+	  is primarily used to dump sample-based profiling registers when
+	  system triggers panic, the driver will parse context registers so
+	  can quickly get to know program counter (PC), secure state,
+	  exception level, etc. Before use debugging functionality, platform
+	  needs to ensure the clock domain and power domain are enabled
+	  properly, please refer Documentation/trace/coresight-cpu-debug.txt
+	  for detailed description and the example for usage.
+
 endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index af480d9..433d590 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
 					coresight-etm4x-sysfs.o
 obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
 obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
+obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c
new file mode 100644
index 0000000..38e3794
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
@@ -0,0 +1,671 @@
+/*
+ * Copyright (c) 2017 Linaro Limited. All rights reserved.
+ *
+ * Author: Leo Yan <leo.yan@linaro.org>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#include <linux/amba/bus.h>
+#include <linux/coresight.h>
+#include <linux/cpu.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/pm_qos.h>
+#include <linux/slab.h>
+#include <linux/smp.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "coresight-priv.h"
+
+#define EDPCSR				0x0A0
+#define EDCIDSR				0x0A4
+#define EDVIDSR				0x0A8
+#define EDPCSR_HI			0x0AC
+#define EDOSLAR				0x300
+#define EDPRCR				0x310
+#define EDPRSR				0x314
+#define EDDEVID1			0xFC4
+#define EDDEVID				0xFC8
+
+#define EDPCSR_PROHIBITED		0xFFFFFFFF
+
+/* bits definition for EDPCSR */
+#define EDPCSR_THUMB			BIT(0)
+#define EDPCSR_ARM_INST_MASK		GENMASK(31, 2)
+#define EDPCSR_THUMB_INST_MASK		GENMASK(31, 1)
+
+/* bits definition for EDPRCR */
+#define EDPRCR_COREPURQ			BIT(3)
+#define EDPRCR_CORENPDRQ		BIT(0)
+
+/* bits definition for EDPRSR */
+#define EDPRSR_DLK			BIT(6)
+#define EDPRSR_PU			BIT(0)
+
+/* bits definition for EDVIDSR */
+#define EDVIDSR_NS			BIT(31)
+#define EDVIDSR_E2			BIT(30)
+#define EDVIDSR_E3			BIT(29)
+#define EDVIDSR_HV			BIT(28)
+#define EDVIDSR_VMID			GENMASK(7, 0)
+
+/*
+ * bits definition for EDDEVID1:PSCROffset
+ *
+ * NOTE: armv8 and armv7 have different definition for the register,
+ * so consolidate the bits definition as below:
+ *
+ * 0b0000 - Sample offset applies based on the instruction state, we
+ *          rely on EDDEVID to check if EDPCSR is implemented or not
+ * 0b0001 - No offset applies.
+ * 0b0010 - No offset applies, but do not use in AArch32 mode
+ *
+ */
+#define EDDEVID1_PCSR_OFFSET_MASK	GENMASK(3, 0)
+#define EDDEVID1_PCSR_OFFSET_INS_SET	(0x0)
+#define EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32	(0x2)
+
+/* bits definition for EDDEVID */
+#define EDDEVID_PCSAMPLE_MODE		GENMASK(3, 0)
+#define EDDEVID_IMPL_EDPCSR		(0x1)
+#define EDDEVID_IMPL_EDPCSR_EDCIDSR	(0x2)
+#define EDDEVID_IMPL_FULL		(0x3)
+
+#define DEBUG_WAIT_SLEEP		1000
+#define DEBUG_WAIT_TIMEOUT		32000
+
+struct debug_drvdata {
+	void __iomem	*base;
+	struct device	*dev;
+	int		cpu;
+
+	bool		edpcsr_present;
+	bool		edcidsr_present;
+	bool		edvidsr_present;
+	bool		pc_has_offset;
+
+	u32		edpcsr;
+	u32		edpcsr_hi;
+	u32		edprsr;
+	u32		edvidsr;
+	u32		edcidsr;
+};
+
+static DEFINE_MUTEX(debug_lock);
+static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
+static int debug_count;
+static struct dentry *debug_debugfs_dir;
+
+static bool debug_enable;
+module_param_named(enable, debug_enable, bool, 0600);
+MODULE_PARM_DESC(enable, "Knob to enable debug functionality "
+		 "(default is 0, which means is disabled by default)");
+
+static void debug_os_unlock(struct debug_drvdata *drvdata)
+{
+	/* Unlocks the debug registers */
+	writel_relaxed(0x0, drvdata->base + EDOSLAR);
+
+	/* Make sure the registers are unlocked before accessing */
+	wmb();
+}
+
+/*
+ * According to ARM DDI 0487A.k, before access external debug
+ * registers should firstly check the access permission; if any
+ * below condition has been met then cannot access debug
+ * registers to avoid lockup issue:
+ *
+ * - CPU power domain is powered off;
+ * - The OS Double Lock is locked;
+ *
+ * By checking EDPRSR can get to know if meet these conditions.
+ */
+static bool debug_access_permitted(struct debug_drvdata *drvdata)
+{
+	/* CPU is powered off */
+	if (!(drvdata->edprsr & EDPRSR_PU))
+		return false;
+
+	/* The OS Double Lock is locked */
+	if (drvdata->edprsr & EDPRSR_DLK)
+		return false;
+
+	return true;
+}
+
+static void debug_force_cpu_powered_up(struct debug_drvdata *drvdata)
+{
+	u32 edprcr;
+
+try_again:
+
+	/*
+	 * Send request to power management controller and assert
+	 * DBGPWRUPREQ signal; if power management controller has
+	 * sane implementation, it should enable CPU power domain
+	 * in case CPU is in low power state.
+	 */
+	edprcr = readl_relaxed(drvdata->base + EDPRCR);
+	edprcr |= EDPRCR_COREPURQ;
+	writel_relaxed(edprcr, drvdata->base + EDPRCR);
+
+	/* Wait for CPU to be powered up (timeout~=32ms) */
+	if (readx_poll_timeout_atomic(readl_relaxed, drvdata->base + EDPRSR,
+			drvdata->edprsr, (drvdata->edprsr & EDPRSR_PU),
+			DEBUG_WAIT_SLEEP, DEBUG_WAIT_TIMEOUT)) {
+		/*
+		 * Unfortunately the CPU cannot be powered up, so return
+		 * back and later has no permission to access other
+		 * registers. For this case, should disable CPU low power
+		 * states to ensure CPU power domain is enabled!
+		 */
+		pr_err("%s: power up request for CPU%d failed\n",
+			__func__, drvdata->cpu);
+		return;
+	}
+
+	/*
+	 * At this point the CPU is powered up, so set the no powerdown
+	 * request bit so we don't lose power and emulate power down.
+	 */
+	edprcr = readl_relaxed(drvdata->base + EDPRCR);
+	edprcr |= EDPRCR_COREPURQ | EDPRCR_CORENPDRQ;
+	writel_relaxed(edprcr, drvdata->base + EDPRCR);
+
+	drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
+
+	/* The core power domain got switched off on use, try again */
+	if (unlikely(!drvdata->edprsr & EDPRSR_PU))
+		goto try_again;
+}
+
+static void debug_read_regs(struct debug_drvdata *drvdata)
+{
+	u32 save_edprcr;
+
+	CS_UNLOCK(drvdata->base);
+
+	/* Unlock os lock */
+	debug_os_unlock(drvdata);
+
+	/* Save EDPRCR register */
+	save_edprcr = readl_relaxed(drvdata->base + EDPRCR);
+
+	/*
+	 * Ensure CPU power domain is enabled to let registers
+	 * are accessiable.
+	 */
+	debug_force_cpu_powered_up(drvdata);
+
+	if (!debug_access_permitted(drvdata))
+		goto out;
+
+	drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
+
+	/*
+	 * As described in ARM DDI 0487A.k, if the processing
+	 * element (PE) is in debug state, or sample-based
+	 * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
+	 * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
+	 * UNKNOWN state. So directly bail out for this case.
+	 */
+	if (drvdata->edpcsr == EDPCSR_PROHIBITED)
+		goto out;
+
+	/*
+	 * A read of the EDPCSR normally has the side-effect of
+	 * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
+	 * at this point it's safe to read value from them.
+	 */
+	if (IS_ENABLED(CONFIG_64BIT))
+		drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
+
+	if (drvdata->edcidsr_present)
+		drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
+
+	if (drvdata->edvidsr_present)
+		drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
+
+out:
+	/* Restore EDPRCR register */
+	writel_relaxed(save_edprcr, drvdata->base + EDPRCR);
+
+	CS_LOCK(drvdata->base);
+}
+
+static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata)
+{
+	unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
+	unsigned long pc;
+
+	if (IS_ENABLED(CONFIG_64BIT))
+		return (unsigned long)drvdata->edpcsr_hi << 32 |
+		       (unsigned long)drvdata->edpcsr;
+
+	pc = (unsigned long)drvdata->edpcsr;
+
+	if (drvdata->pc_has_offset) {
+		arm_inst_offset = 8;
+		thumb_inst_offset = 4;
+	}
+
+	/* Handle thumb instruction */
+	if (pc & EDPCSR_THUMB) {
+		pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
+		return pc;
+	}
+
+	/*
+	 * Handle arm instruction offset, if the arm instruction
+	 * is not 4 byte alignment then it's possible the case
+	 * for implementation defined; keep original value for this
+	 * case and print info for notice.
+	 */
+	if (pc & BIT(1))
+		pr_emerg("Instruction offset is implementation defined\n");
+	else
+		pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
+
+	return pc;
+}
+
+static void debug_dump_regs(struct debug_drvdata *drvdata)
+{
+	unsigned long pc;
+
+	pr_emerg("\tEDPRSR:  %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
+		 drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
+		 drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
+
+	if (!debug_access_permitted(drvdata)) {
+		pr_emerg("No permission to access debug registers!\n");
+		return;
+	}
+
+	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
+		pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
+		return;
+	}
+
+	pc = debug_adjust_pc(drvdata);
+	pr_emerg("\tEDPCSR:  [<%p>] %pS\n", (void *)pc, (void *)pc);
+
+	if (drvdata->edcidsr_present)
+		pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
+
+	if (drvdata->edvidsr_present)
+		pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%dbits VMID:%x)\n",
+			 drvdata->edvidsr,
+			 drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
+			 drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
+				(drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
+			 drvdata->edvidsr & EDVIDSR_HV ? 64 : 32,
+			 drvdata->edvidsr & (u32)EDVIDSR_VMID);
+}
+
+static void debug_init_arch_data(void *info)
+{
+	struct debug_drvdata *drvdata = info;
+	u32 mode, pcsr_offset;
+	u32 eddevid, eddevid1;
+
+	CS_UNLOCK(drvdata->base);
+
+	/* Read device info */
+	eddevid  = readl_relaxed(drvdata->base + EDDEVID);
+	eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
+
+	CS_LOCK(drvdata->base);
+
+	/* Parse implementation feature */
+	mode = eddevid & EDDEVID_PCSAMPLE_MODE;
+	pcsr_offset = eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
+
+	drvdata->edpcsr_present  = false;
+	drvdata->edcidsr_present = false;
+	drvdata->edvidsr_present = false;
+	drvdata->pc_has_offset   = false;
+
+	switch (mode) {
+	case EDDEVID_IMPL_FULL:
+		drvdata->edvidsr_present = true;
+		/* Fall through */
+	case EDDEVID_IMPL_EDPCSR_EDCIDSR:
+		drvdata->edcidsr_present = true;
+		/* Fall through */
+	case EDDEVID_IMPL_EDPCSR:
+		/*
+		 * In ARM DDI 0487A.k, the EDDEVID1.PCSROffset is used to
+		 * define if has the offset for PC sampling value; if read
+		 * back EDDEVID1.PCSROffset == 0x2, then this means the debug
+		 * module does not sample the instruction set state when
+		 * armv8 CPU in AArch32 state.
+		 */
+		drvdata->edpcsr_present =
+			((IS_ENABLED(CONFIG_64BIT) && pcsr_offset != 0) ||
+			 (pcsr_offset != EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32));
+
+		drvdata->pc_has_offset =
+			(pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
+		break;
+	default:
+		break;
+	}
+}
+
+/*
+ * Dump out information on panic.
+ */
+static int debug_notifier_call(struct notifier_block *self,
+			       unsigned long v, void *p)
+{
+	int cpu;
+	struct debug_drvdata *drvdata;
+
+	mutex_lock(&debug_lock);
+
+	/* Bail out if the functionality is disabled */
+	if (!debug_enable)
+		goto skip_dump;
+
+	pr_emerg("ARM external debug module:\n");
+
+	for_each_possible_cpu(cpu) {
+		drvdata = per_cpu(debug_drvdata, cpu);
+		if (!drvdata)
+			continue;
+
+		pr_emerg("CPU[%d]:\n", drvdata->cpu);
+
+		debug_read_regs(drvdata);
+		debug_dump_regs(drvdata);
+	}
+
+skip_dump:
+	mutex_unlock(&debug_lock);
+	return 0;
+}
+
+static struct notifier_block debug_notifier = {
+	.notifier_call = debug_notifier_call,
+};
+
+static int debug_enable_func(void)
+{
+	struct debug_drvdata *drvdata;
+	int cpu, ret = 0;
+
+	for_each_possible_cpu(cpu) {
+		drvdata = per_cpu(debug_drvdata, cpu);
+		if (!drvdata)
+			continue;
+
+		ret = pm_runtime_get_sync(drvdata->dev);
+		if (ret < 0) {
+			pm_runtime_put_noidle(drvdata->dev);
+			goto err;
+		}
+	}
+
+err:
+	return ret;
+}
+
+static int debug_disable_func(void)
+{
+	struct debug_drvdata *drvdata;
+	int cpu, ret = 0;
+
+	for_each_possible_cpu(cpu) {
+		drvdata = per_cpu(debug_drvdata, cpu);
+		if (!drvdata)
+			continue;
+
+		ret = pm_runtime_put(drvdata->dev);
+		if (ret < 0)
+			goto err;
+	}
+
+err:
+	return ret;
+}
+
+static ssize_t debug_func_knob_write(struct file *f,
+		const char __user *buf, size_t count, loff_t *ppos)
+{
+	u8 val;
+	int ret;
+
+	ret = kstrtou8_from_user(buf, count, 2, &val);
+	if (ret)
+		return ret;
+
+	mutex_lock(&debug_lock);
+
+	if (val == debug_enable)
+		goto out;
+
+	if (val)
+		ret = debug_enable_func();
+	else
+		ret = debug_disable_func();
+
+	if (ret) {
+		pr_err("%s: unable to %s debug function: %d\n",
+		       __func__, val ? "enable" : "disable", ret);
+		goto err;
+	}
+
+	debug_enable = val;
+out:
+	ret = count;
+err:
+	mutex_unlock(&debug_lock);
+	return ret;
+}
+
+static ssize_t debug_func_knob_read(struct file *f,
+		char __user *ubuf, size_t count, loff_t *ppos)
+{
+	ssize_t ret;
+	char buf[3];
+
+	mutex_lock(&debug_lock);
+
+	snprintf(buf, sizeof(buf), "%d\n", debug_enable);
+	ret = simple_read_from_buffer(ubuf, count, ppos, buf, sizeof(buf));
+
+	mutex_unlock(&debug_lock);
+	return ret;
+}
+
+static const struct file_operations debug_func_knob_fops = {
+	.open	= simple_open,
+	.read	= debug_func_knob_read,
+	.write	= debug_func_knob_write,
+};
+
+static int debug_func_init(void)
+{
+	struct dentry *file;
+	int ret;
+
+	/* Create debugfs node */
+	debug_debugfs_dir = debugfs_create_dir("coresight_cpu_debug", NULL);
+	if (!debug_debugfs_dir) {
+		pr_err("%s: unable to create debugfs directory\n", __func__);
+		return -ENOMEM;
+	}
+
+	file = debugfs_create_file("enable", 0644, debug_debugfs_dir, NULL,
+				   &debug_func_knob_fops);
+	if (!file) {
+		pr_err("%s: unable to create enable knob file\n", __func__);
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	/* Register function to be called for panic */
+	ret = atomic_notifier_chain_register(&panic_notifier_list,
+					     &debug_notifier);
+	if (ret) {
+		pr_err("%s: unable to register notifier: %d\n",
+		       __func__, ret);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	debugfs_remove_recursive(debug_debugfs_dir);
+	return ret;
+}
+
+static void debug_func_exit(void)
+{
+	debugfs_remove_recursive(debug_debugfs_dir);
+
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &debug_notifier);
+}
+
+static int debug_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	void __iomem *base;
+	struct device *dev = &adev->dev;
+	struct debug_drvdata *drvdata;
+	struct resource *res = &adev->res;
+	struct device_node *np = adev->dev.of_node;
+	int ret;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
+	if (per_cpu(debug_drvdata, drvdata->cpu)) {
+		dev_err(dev, "CPU%d drvdata has been initialized, "
+			"may be caused by binding wrong CPU node in the DT\n",
+			drvdata->cpu);
+		return -EBUSY;
+	}
+
+	drvdata->dev = &adev->dev;
+	amba_set_drvdata(adev, drvdata);
+
+	/* Validity for the resource is already checked by the AMBA core */
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	drvdata->base = base;
+
+	get_online_cpus();
+	per_cpu(debug_drvdata, drvdata->cpu) = drvdata;
+	ret = smp_call_function_single(drvdata->cpu, debug_init_arch_data,
+				       drvdata, 1);
+	put_online_cpus();
+
+	if (ret) {
+		dev_err(dev, "CPU%d debug arch init failed\n", drvdata->cpu);
+		goto err;
+	}
+
+	if (!drvdata->edpcsr_present) {
+		dev_err(dev, "CPU%d sample-based profiling isn't implemented\n",
+			drvdata->cpu);
+		ret = -ENXIO;
+		goto err;
+	}
+
+	if (!debug_count++) {
+		ret = debug_func_init();
+		if (ret)
+			goto err_func_init;
+	}
+
+	mutex_lock(&debug_lock);
+	if (!debug_enable)
+		pm_runtime_put(dev);
+	mutex_unlock(&debug_lock);
+
+	dev_info(dev, "Coresight debug-CPU%d initialized\n", drvdata->cpu);
+	return 0;
+
+err_func_init:
+	debug_count--;
+err:
+	per_cpu(debug_drvdata, drvdata->cpu) = NULL;
+	return ret;
+}
+
+static int debug_remove(struct amba_device *adev)
+{
+	struct device *dev = &adev->dev;
+	struct debug_drvdata *drvdata = amba_get_drvdata(adev);
+
+	per_cpu(debug_drvdata, drvdata->cpu) = NULL;
+
+	mutex_lock(&debug_lock);
+	if (debug_enable)
+		pm_runtime_put(dev);
+	mutex_unlock(&debug_lock);
+
+	if (!--debug_count)
+		debug_func_exit();
+
+	return 0;
+}
+
+static struct amba_id debug_ids[] = {
+	{       /* Debug for Cortex-A53 */
+		.id	= 0x000bbd03,
+		.mask	= 0x000fffff,
+	},
+	{       /* Debug for Cortex-A57 */
+		.id	= 0x000bbd07,
+		.mask	= 0x000fffff,
+	},
+	{       /* Debug for Cortex-A72 */
+		.id	= 0x000bbd08,
+		.mask	= 0x000fffff,
+	},
+	{ 0, 0 },
+};
+
+static struct amba_driver debug_driver = {
+	.drv = {
+		.name   = "coresight-cpu-debug",
+		.suppress_bind_attrs = true,
+	},
+	.probe		= debug_probe,
+	.remove		= debug_remove,
+	.id_table	= debug_ids,
+};
+
+module_amba_driver(debug_driver);
+
+MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
+MODULE_DESCRIPTION("ARM Coresight CPU Debug Driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 4/7] coresight: refactor with function of_coresight_get_cpu
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, Mike Leach, Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

This is refactor to add function of_coresight_get_cpu(), so it's used to
retrieve CPU id for coresight component. Finally can use it as a common
function for multiple places.

Suggested-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/hwtracing/coresight/of_coresight.c | 43 +++++++++++++++++++-----------
 include/linux/coresight.h                  |  2 ++
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index de7e8ce..46eec0f 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -101,15 +101,39 @@ static int of_coresight_alloc_memory(struct device *dev,
 	return 0;
 }
 
+int of_coresight_get_cpu(const struct device_node *node)
+{
+	int cpu;
+	bool found;
+	struct device_node *dn, *np;
+
+	dn = of_parse_phandle(node, "cpu", 0);
+
+	/* Affinity defaults to CPU0 */
+	if (!dn)
+		return 0;
+
+	for_each_possible_cpu(cpu) {
+		np = of_cpu_device_node_get(cpu);
+		found = (dn == np);
+		of_node_put(np);
+		if (found)
+			break;
+	}
+	of_node_put(dn);
+
+	/* Affinity to CPU0 if no cpu nodes are found */
+	return found ? cpu : 0;
+}
+EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+
 struct coresight_platform_data *of_get_coresight_platform_data(
 			struct device *dev, const struct device_node *node)
 {
-	int i = 0, ret = 0, cpu;
+	int i = 0, ret = 0;
 	struct coresight_platform_data *pdata;
 	struct of_endpoint endpoint, rendpoint;
 	struct device *rdev;
-	bool found;
-	struct device_node *dn, *np;
 	struct device_node *ep = NULL;
 	struct device_node *rparent = NULL;
 	struct device_node *rport = NULL;
@@ -176,18 +200,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
 		} while (ep);
 	}
 
-	dn = of_parse_phandle(node, "cpu", 0);
-	for_each_possible_cpu(cpu) {
-		np = of_cpu_device_node_get(cpu);
-		found = (dn == np);
-		of_node_put(np);
-		if (found)
-			break;
-	}
-	of_node_put(dn);
-
-	/* Affinity to CPU0 if no cpu nodes are found */
-	pdata->cpu = found ? cpu : 0;
+	pdata->cpu = of_coresight_get_cpu(node);
 
 	return pdata;
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 769f2c8..4915254 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -263,9 +263,11 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
 #endif
 
 #ifdef CONFIG_OF
+extern int of_coresight_get_cpu(const struct device_node *node);
 extern struct coresight_platform_data *of_get_coresight_platform_data(
 	struct device *dev, const struct device_node *node);
 #else
+static inline int of_coresight_get_cpu(const struct device_node *node) { return 0; }
 static inline struct coresight_platform_data *of_get_coresight_platform_data(
 	struct device *dev, const struct device_node *node) { return NULL; }
 #endif
-- 
2.7.4

--
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 v7 3/7] coresight: of_get_coresight_platform_data: Add missing of_node_put
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

From: Suzuki K Poulose <suzuki.poulose@arm.com>

The of_get_coresight_platform_data iterates over the possible CPU nodes
to find a given cpu phandle. However it does not drop the reference
to the node pointer returned by the of_get_coresight_platform_data.

This patch also introduces another minor fix is to use
of_cpu_device_node_get() to replace of_get_cpu_node().

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Leo: minor tweaks for of_get_coresight_platform_data]
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/of_coresight.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index 859ad49..de7e8ce 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -108,7 +108,8 @@ struct coresight_platform_data *of_get_coresight_platform_data(
 	struct coresight_platform_data *pdata;
 	struct of_endpoint endpoint, rendpoint;
 	struct device *rdev;
-	struct device_node *dn;
+	bool found;
+	struct device_node *dn, *np;
 	struct device_node *ep = NULL;
 	struct device_node *rparent = NULL;
 	struct device_node *rport = NULL;
@@ -175,17 +176,19 @@ struct coresight_platform_data *of_get_coresight_platform_data(
 		} while (ep);
 	}
 
-	/* Affinity defaults to CPU0 */
-	pdata->cpu = 0;
 	dn = of_parse_phandle(node, "cpu", 0);
-	for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
-		if (dn == of_get_cpu_node(cpu, NULL)) {
-			pdata->cpu = cpu;
+	for_each_possible_cpu(cpu) {
+		np = of_cpu_device_node_get(cpu);
+		found = (dn == np);
+		of_node_put(np);
+		if (found)
 			break;
-		}
 	}
 	of_node_put(dn);
 
+	/* Affinity to CPU0 if no cpu nodes are found */
+	pdata->cpu = found ? cpu : 0;
+
 	return pdata;
 }
 EXPORT_SYMBOL_GPL(of_get_coresight_platform_data);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 2/7] doc: Add documentation for Coresight CPU debug
From: Leo Yan @ 2017-05-02  8:30 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

Update kernel-parameters.txt to add new parameter:
coresight_cpu_debug.enable is a knob to enable debugging at boot time.

Add detailed documentation, which contains the implementation, Mike
Leach excellent summary for "clock and power domain". At the end some
examples on how to enable the debugging functionality are provided.

Suggested-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   7 +
 Documentation/trace/coresight-cpu-debug.txt     | 174 ++++++++++++++++++++++++
 2 files changed, 181 insertions(+)
 create mode 100644 Documentation/trace/coresight-cpu-debug.txt

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index facc20a..cf90146 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -650,6 +650,13 @@
 			/proc/<pid>/coredump_filter.
 			See also Documentation/filesystems/proc.txt.
 
+	coresight_cpu_debug.enable
+			[ARM,ARM64]
+			Format: <bool>
+			Enable/disable the CPU sampling based debugging.
+			0: default value, disable debugging
+			1: enable debugging at boot time
+
 	cpuidle.off=1	[CPU_IDLE]
 			disable the cpuidle sub-system
 
diff --git a/Documentation/trace/coresight-cpu-debug.txt b/Documentation/trace/coresight-cpu-debug.txt
new file mode 100644
index 0000000..fd3f07d
--- /dev/null
+++ b/Documentation/trace/coresight-cpu-debug.txt
@@ -0,0 +1,174 @@
+		Coresight CPU Debug Module
+		==========================
+
+   Author:   Leo Yan <leo.yan@linaro.org>
+   Date:     April 5th, 2017
+
+Introduction
+------------
+
+Coresight CPU debug module is defined in ARMv8-a architecture reference manual
+(ARM DDI 0487A.k) Chapter 'Part H: External debug', the CPU can integrate
+debug module and it is mainly used for two modes: self-hosted debug and
+external debug. Usually the external debug mode is well known as the external
+debugger connects with SoC from JTAG port; on the other hand the program can
+explore debugging method which rely on self-hosted debug mode, this document
+is to focus on this part.
+
+The debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc; usually
+every CPU has one dedicated debug module to be connected. Based on self-hosted
+debug mechanism, Linux kernel can access these related registers from mmio
+region when the kernel panic happens. The callback notifier for kernel panic
+will dump related registers for every CPU; finally this is good for assistant
+analysis for panic.
+
+
+Implementation
+--------------
+
+- During driver registration, use EDDEVID and EDDEVID1 two device ID
+  registers to decide if sample-based profiling is implemented or not. On some
+  platforms this hardware feature is fully or partialy implemented; and if
+  this feature is not supported then registration will fail.
+
+- When write this doc, the debug driver mainly relies on three sampling
+  registers. The kernel panic callback notifier gathers info from EDPCSR
+  EDVIDSR and EDCIDSR; from EDPCSR we can get program counter, EDVIDSR has
+  information for secure state, exception level, bit width, etc; EDCIDSR is
+  context ID value which contains the sampled value of CONTEXTIDR_EL1.
+
+- The driver supports CPU running mode with either AArch64 or AArch32. The
+  registers naming convention is a bit different between them, AArch64 uses
+  'ED' for register prefix (ARM DDI 0487A.k, chapter H9.1) and AArch32 uses
+  'DBG' as prefix (ARM DDI 0487A.k, chapter G5.1). The driver is unified to
+  use AArch64 naming convention.
+
+- ARMv8-a (ARM DDI 0487A.k) and ARMv7-a (ARM DDI 0406C.b) have different
+  register bits definition. So the driver consolidates two difference:
+
+  If PCSROffset=0b0000, on ARMv8-a the feature of EDPCSR is not implemented;
+  but ARMv7-a defines "PCSR samples are offset by a value that depends on the
+  instruction set state". For ARMv7-a, the driver checks furthermore if CPU
+  runs with ARM or thumb instruction set and calibrate PCSR value, the
+  detailed description for offset is in ARMv7-a ARM (ARM DDI 0406C.b) chapter
+  C11.11.34 "DBGPCSR, Program Counter Sampling Register".
+
+  If PCSROffset=0b0010, ARMv8-a defines "EDPCSR implemented, and samples have
+  no offset applied and do not sample the instruction set state in AArch32
+  state". So on ARMv8 if EDDEVID1.PCSROffset is 0b0010 and the CPU operates
+  in AArch32 state, EDPCSR is not sampled; when the CPU operates in AArch64
+  state EDPCSR is sampled and no offset are applied.
+
+
+Clock and power domain
+----------------------
+
+Before accessing debug registers, we should ensure the clock and power domain
+have been enabled properly. In ARMv8-a ARM (ARM DDI 0487A.k) chapter 'H9.1
+Debug registers', the debug registers are spread into two domains: the debug
+domain and the CPU domain.
+
+                                +---------------+
+                                |               |
+                                |               |
+                     +----------+--+            |
+          dbg_clk -->|          |**|            |<-- cpu_clk
+                     |    Debug |**|   CPU      |
+           dbg_pd -->|          |**|            |<-- cpu_pd
+                     +----------+--+            |
+                                |               |
+                                |               |
+                                +---------------+
+
+For debug domain, the user uses DT binding "clocks" and "power-domains" to
+specify the corresponding clock source and power supply for the debug logic.
+The driver calls the pm_runtime_{put|get} operations as needed to handle the
+debug power domain.
+
+For CPU domain, the different SoC designs have different power management
+schemes and finally this heavily impacts external debug module. So we can
+divide into below cases:
+
+- On systems with a sane power controller which can behave correctly with
+  respect to CPU power domain, the CPU power domain can be controlled by
+  register EDPRCR in driver. The driver firstly writes bit EDPRCR.COREPURQ
+  to power up the CPU, and then writes bit EDPRCR.CORENPDRQ for emulation
+  of CPU power down. As result, this can ensure the CPU power domain is
+  powered on properly during the period when access debug related registers;
+
+- Some designs will power down an entire cluster if all CPUs on the cluster
+  are powered down - including the parts of the debug registers that should
+  remain powered in the debug power domain. The bits in EDPRCR are not
+  respected in these cases, so these designs do not support debug over
+  power down in the way that the CoreSight / Debug designers anticipated.
+  This means that even checking EDPRSR has the potential to cause a bus hang
+  if the target register is unpowered.
+
+  In this case, accessing to the debug registers while they are not powered
+  is a recipe for disaster; so we need preventing CPU low power states at boot
+  time or when user enable module at the run time. Please see chapter
+  "How to use the module" for detailed usage info for this.
+
+
+Device Tree Bindings
+--------------------
+
+See Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt for details.
+
+
+How to use the module
+---------------------
+
+If you want to enable debugging functionality at boot time, you can add
+"coresight_cpu_debug.enable=1" to the kernel command line parameter.
+
+The driver also can work as module, so can enable the debugging when insmod
+module:
+# insmod coresight_cpu_debug.ko debug=1
+
+When boot time or insmod module you have not enabled the debugging, the driver
+uses the debugfs file system to provide a knob to dynamically enable or disable
+debugging:
+
+To enable it, write a '1' into /sys/kernel/debug/coresight_cpu_debug/enable:
+# echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable
+
+To disable it, write a '0' into /sys/kernel/debug/coresight_cpu_debug/enable:
+# echo 0 > /sys/kernel/debug/coresight_cpu_debug/enable
+
+As explained in chapter "Clock and power domain", if you are working on one
+platform which has idle states to power off debug logic and the power
+controller cannot work well for the request from EDPRCR, then you should
+firstly constraint CPU idle states before enable CPU debugging feature; so can
+ensure the accessing to debug logic.
+
+If you want to limit idle states at boot time, you can use "nohlt" or
+"cpuidle.off=1" in the kernel command line.
+
+At the runtime you can disable idle states with below methods:
+
+Set latency request to /dev/cpu_dma_latency to disable all CPUs specific idle
+states (if latency = 0uS then disable all idle states):
+# echo "what_ever_latency_you_need_in_uS" > /dev/cpu_dma_latency
+
+Disable specific CPU's specific idle state:
+# echo 1 > /sys/devices/system/cpu/cpu$cpu/cpuidle/state$state/disable
+
+
+Output format
+-------------
+
+Here is an example of the debugging output format:
+
+ARM external debug module:
+CPU[0]:
+ EDPRSR:  0000000b (Power:On DLK:Unlock)
+ EDPCSR:  [<ffff00000808eb54>] handle_IPI+0xe4/0x150
+ EDCIDSR: 00000000
+ EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
+CPU[1]:
+ EDPRSR:  0000000b (Power:On DLK:Unlock)
+ EDPCSR:  [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
+ EDCIDSR: 00000000
+ EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
-- 
2.7.4


^ permalink raw reply related

* [PATCH v7 1/7] coresight: bindings for CPU debug module
From: Leo Yan @ 2017-05-02  8:29 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan
In-Reply-To: <1493713805-26920-1-git-send-email-leo.yan@linaro.org>

According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
Chapter 'Part H: External debug', the CPU can integrate debug module
and it can support self-hosted debug and external debug. Especially
for supporting self-hosted debug, this means the program can access
the debug module from mmio region; and usually the mmio region is
integrated with coresight.

So add document for binding debug component, includes binding to APB
clock; and also need specify the CPU node which the debug module is
dedicated to specific CPU.

Suggested-by: Mike Leach <mike.leach@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 .../bindings/arm/coresight-cpu-debug.txt           | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt

diff --git a/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
new file mode 100644
index 0000000..2982912
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
@@ -0,0 +1,49 @@
+* CoreSight CPU Debug Component:
+
+CoreSight CPU debug component are compliant with the ARMv8 architecture
+reference manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The
+external debug module is mainly used for two modes: self-hosted debug and
+external debug, and it can be accessed from mmio region from Coresight
+and eventually the debug module connects with CPU for debugging. And the
+debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc;
+usually every CPU has one dedicated debug module to be connected.
+
+Required properties:
+
+- compatible : should be "arm,coresight-cpu-debug"; supplemented with
+               "arm,primecell" since this driver is using the AMBA bus
+	       interface.
+
+- reg : physical base address and length of the register set.
+
+- clocks : the clock associated to this component.
+
+- clock-names : the name of the clock referenced by the code. Since we are
+                using the AMBA framework, the name of the clock providing
+		the interconnect should be "apb_pclk" and the clock is
+		mandatory. The interface between the debug logic and the
+		processor core is clocked by the internal CPU clock, so it
+		is enabled with CPU clock by default.
+
+- cpu : the CPU phandle the debug module is affined to. When omitted
+	the module is considered to belong to CPU0.
+
+Optional properties:
+
+- power-domains: a phandle to the debug power domain. We use "power-domains"
+                 binding to turn on the debug logic if it has own dedicated
+		 power domain and if necessary to use "cpuidle.off=1" or
+		 "nohlt" in the kernel command line or sysfs node to
+		 constrain idle states to ensure registers in the CPU power
+		 domain are accessible.
+
+Example:
+
+	debug@f6590000 {
+		compatible = "arm,coresight-cpu-debug","arm,primecell";
+		reg = <0 0xf6590000 0 0x1000>;
+		clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu0>;
+	};
-- 
2.7.4


^ permalink raw reply related

* [PATCH v7 0/7] coresight: enable debug module
From: Leo Yan @ 2017-05-02  8:29 UTC (permalink / raw)
  To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Mathieu Poirier, Greg Kroah-Hartman, Suzuki K Poulose,
	Stephen Boyd, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, Mike Leach,
	Sudeep Holla
  Cc: Leo Yan

ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.

This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.

Patch 0001 is to document the dt binding; patch 0002 is to document
boot parameters used in kernel command line and add one detailed
document to describe the Coresight debug module implementation, the
clock and power domain impaction on the driver, some examples for
usage.

Patch 0003 is used to fix the func of_get_coresight_platform_data()
doesn't properly drop the reference to the CPU node pointer; and
patch 0004 is refactor to add new function of_coresight_get_cpu().

Patch 0005 is the driver for CPU debug module.

Patch 0006 in this series are to enable debug unit on 96boards Hikey,
Patch 0007 is to enable debug on 96boards DB410c. Have verified on both
two boards.

We can enable debugging with two methods, adding parameters into kernel
command line for build-in module:
  coresight_cpu_debug.enable=1

Or we can wait the system has booted up to use debugfs nodes to enable
debugging:
  # echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable

As result we can get below log after input command:
echo c > /proc/sysrq-trigger:

ARM external debug module:
CPU[0]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff00000808eb54>] handle_IPI+0xe4/0x150
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
CPU[1]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)

[...]

Changes from v6:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to install panic notifier when insmod module; refined function
  debug_force_cpu_powered_up() for CPU power state checking; some minor
  fixing for output log, adding comments for memory barrier, code
  alignment.

Changes from v5:
* According to Suzuki and Mathieu suggestions, refined debug module
  driver to drop unused structure members, refactored initialization
  code to distinguish hardware implementation features, refactored
  flow for forcing CPU powered up, supported pm_runtime operations.
* Added one new doc file: Documentation/trace/coresight-cpu-debug.txt,
  which is used to describe detailed info for implementation, clock
  and power domain impaction on debug module, and exmaples for common
  usage.
* Removed "idle constraints" from debug driver.

Changes from v4:
* This version is mainly credit to ARM colleagues many contribution
  ideas for better quality (Thanks a lot Suzuki, Mike and Sudeep!).
* According to Suzuki suggestion, refined debug module driver to avoid
  memory leak for drvdata struct, handle PCSAMPLE_MODE=1, use flag
  drvdata.pc_has_offset to indicate if PCSR has offset, minor fixes.
* According to Mathieu suggestion, refined dt binding description.
* Changed driver to support module mode;
* According to Mike suggestion and very appreciate the pseudo code,
  added support to force CPU powered up with register EDPRCR;
* According to discussions, added command line and debugfs nodes to
  support enabling debugging for boot time, or later can dynamically
  enable/disable debugging by debugfs.
* According to Rob Herring suggestion, one minor fixes in DT binding.
* According to Stephen Boyd suggestion, add const quality to structure
  device_node. And used use of_cpu_device_node_get() to replace
  of_get_cpu_node() in patch 0003.

Changes from v3:
* Added Suzuki K Poulose's patch to fix issue for the func
  of_get_coresight_platform_data() doesn't properly drop the reference
  to the CPU node pointer.
* According to Suzuki suggestion, added code to handl the corner case
  for ARMv8 CPU with aarch32 mode.
* According to Suzuki suggestion, changed compatible string to
  "arm,coresight-cpu-debug".
* According to Mathieu suggestion, added "power-domains" as optional
  properties.

Changes from v2:
* According to Mathieu Poirier suggestion, applied some minor fixes.
* Added two extra patches for enabling debug module on Hikey.

Changes from v1:
* According to Mike Leach suggestion, removed the binding for debug
  module clocks which have been directly provided by CPU clocks.
* According to Mathieu Poirier suggestion, added function
  of_coresight_get_cpu() and some minor refactors for debug module
  driver.

Changes from RFC:
* According to Mike Leach suggestion, added check for EDPRSR to avoid
  lockup; added supporting EDVIDSR and EDCIDSR registers.
* According to Mark Rutland and Mathieu Poirier suggestion, rewrote
  the documentation for DT binding.
* According to Mark and Mathieu suggestion, refined debug driver.


Leo Yan (6):
  coresight: bindings for CPU debug module
  doc: Add documentation for Coresight CPU debug
  coresight: refactor with function of_coresight_get_cpu
  coresight: add support for CPU debug module
  arm64: dts: hi6220: register debug module
  arm64: dts: qcom: msm8916: Add debug unit

Suzuki K Poulose (1):
  coresight: of_get_coresight_platform_data: Add missing of_node_put

 Documentation/admin-guide/kernel-parameters.txt    |   7 +
 .../bindings/arm/coresight-cpu-debug.txt           |  49 ++
 Documentation/trace/coresight-cpu-debug.txt        | 174 ++++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  64 ++
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |  32 +
 drivers/hwtracing/coresight/Kconfig                |  14 +
 drivers/hwtracing/coresight/Makefile               |   1 +
 drivers/hwtracing/coresight/coresight-cpu-debug.c  | 671 +++++++++++++++++++++
 drivers/hwtracing/coresight/of_coresight.c         |  40 +-
 include/linux/coresight.h                          |   2 +
 10 files changed, 1042 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
 create mode 100644 Documentation/trace/coresight-cpu-debug.txt
 create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx: add Gateworks Ventana GW5600 support
From: Shawn Guo @ 2017-05-02  8:22 UTC (permalink / raw)
  To: Tim Harvey
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492197481-16638-1-git-send-email-tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>

On Fri, Apr 14, 2017 at 12:18:01PM -0700, Tim Harvey wrote:
> The Gateworks Ventana GW5600 is a media-centric single-board computer based on
> the NXP IMX6 SoC with the following features:
>  * PoE (emulated 802.3af)
>  * IMX6 DualLite Soc (supports IMX6S,IMX6DL,IMX6Q)
>  * 1GiB DDR3 DRAM (supports up to 4GiB)
>  * 8GB eMMC
>  * 1x microSD connector
>  * Gateworks System Controller:
>   - hardware watchdog
>   - hardware monitor
>   - pushbutton controller
>   - EEPROM storage
>   - power control
>  * 1x bi-color USER LED
>  * 1x front-panel pushbutton
>  * 1x front-panel GbE
>  * 2x front panel USB 2.0
>  * 1x front panel USB OTG
>  * 1x SIM socket
>  * 1x miniPCIe socket with SATA (mSATA)
>  * 1x miniPCIe socket with USB 2.0 (Modem)
>  * 1x miniPCIe socket with PCIe, USB 2.0, and SIM
>  * RS232/RS485 serial
>   - 2x RS232 UARTs (off-board connector)
>   - 1x RS485 (loading option)
>  * 4x digital I/O signals (PWM/I2C/GPIO/5V/3.3V options)
>  * 1x analog input (0 to 5V)
>  * 1x CAN (loading option)
>  * off-board LVDS:
>   - I2C
>   - 12V
>   - LED driver (4x 330mA strings)
>   - matrix keypad controller (8row x 10col)
>   - I2S
>   - dual-channel LVDS
>   - PWM
>  * off-board video input:
>   - 16bit parallel / MIPI (IPU1_CSI0)
>  * GPS (loading option)
>  * Analog Video Input (CVBS) 3 inputs (1 active at a time)
>  * Analog Audio Input/Output (2ch Line level, optional MIC/HP drivers)
>  * HDMI out
>  * JTAG programmable
>  * Inertial Module
> 
> Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile            |   2 +
>  arch/arm/boot/dts/imx6dl-gw560x.dts   |  55 +++
>  arch/arm/boot/dts/imx6q-gw560x.dts    |  59 +++
>  arch/arm/boot/dts/imx6qdl-gw560x.dtsi | 761 ++++++++++++++++++++++++++++++++++
>  4 files changed, 877 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6dl-gw560x.dts
>  create mode 100644 arch/arm/boot/dts/imx6q-gw560x.dts
>  create mode 100644 arch/arm/boot/dts/imx6qdl-gw560x.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 0bff8e7..0c9cfbb 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -352,6 +352,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6dl-gw551x.dtb \
>  	imx6dl-gw552x.dtb \
>  	imx6dl-gw553x.dtb \
> +	imx6dl-gw560x.dtb \
>  	imx6dl-gw5903.dtb \
>  	imx6dl-gw5904.dtb \
>  	imx6dl-hummingboard.dtb \
> @@ -397,6 +398,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6q-gw551x.dtb \
>  	imx6q-gw552x.dtb \
>  	imx6q-gw553x.dtb \
> +	imx6q-gw560x.dtb \
>  	imx6q-gw5903.dtb \
>  	imx6q-gw5904.dtb \
>  	imx6q-h100.dtb \
> diff --git a/arch/arm/boot/dts/imx6dl-gw560x.dts b/arch/arm/boot/dts/imx6dl-gw560x.dts
> new file mode 100644
> index 0000000..21bdfaf
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6dl-gw560x.dts
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright 2017 Gateworks Corporation
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of
> + *     the License, or (at your option) any later version.
> + *
> + *     This file 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.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "imx6dl.dtsi"
> +#include "imx6qdl-gw560x.dtsi"
> +
> +/ {
> +	model = "Gateworks Ventana i.MX6 DualLite/Solo GW560X";
> +	compatible = "gw,imx6dl-gw560x", "gw,ventana", "fsl,imx6dl";
> +};
> diff --git a/arch/arm/boot/dts/imx6q-gw560x.dts b/arch/arm/boot/dts/imx6q-gw560x.dts
> new file mode 100644
> index 0000000..735f2bb
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6q-gw560x.dts
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright 2017 Gateworks Corporation
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of
> + *     the License, or (at your option) any later version.
> + *
> + *     This file 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.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "imx6q.dtsi"
> +#include "imx6qdl-gw560x.dtsi"
> +
> +/ {
> +	model = "Gateworks Ventana i.MX6 Dual/Quad GW560X";
> +	compatible = "gw,imx6q-gw560x", "gw,ventana", "fsl,imx6q";
> +};
> +
> +&sata {
> +	status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/imx6qdl-gw560x.dtsi b/arch/arm/boot/dts/imx6qdl-gw560x.dtsi
> new file mode 100644
> index 0000000..dc52b87
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6qdl-gw560x.dtsi
> @@ -0,0 +1,761 @@
> +/*
> + * Copyright 2017 Gateworks Corporation
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of
> + *     the License, or (at your option) any later version.
> + *
> + *     This file 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.
> + *
> + *     You should have received a copy of the GNU General Public
> + *     License along with this file; if not, write to the Free
> + *     Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + *     MA 02110-1301 USA
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	/* these are used by bootloader for disabling nodes */
> +	aliases {
> +		led0 = &led0;
> +		led1 = &led1;
> +		led2 = &led2;
> +		ssi0 = &ssi1;
> +		usb0 = &usbh1;
> +		usb1 = &usbotg;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart2;
> +	};
> +
> +	backlight_display {

Can we consistently use hyphen instead of underscore in node name?

> +		compatible = "pwm-backlight";
> +		pwms = <&pwm4 0 5000000>;
> +		brightness-levels = <
> +			0  1  2  3  4  5  6  7  8  9
> +			10 11 12 13 14 15 16 17 18 19
> +			20 21 22 23 24 25 26 27 28 29
> +			30 31 32 33 34 35 36 37 38 39
> +			40 41 42 43 44 45 46 47 48 49
> +			50 51 52 53 54 55 56 57 58 59
> +			60 61 62 63 64 65 66 67 68 69
> +			70 71 72 73 74 75 76 77 78 79
> +			80 81 82 83 84 85 86 87 88 89
> +			90 91 92 93 94 95 96 97 98 99
> +			100
> +			>;
> +		default-brightness-level = <100>;
> +	};
> +
> +	backlight_keypad {

Ditto

> +		compatible = "gpio-backlight";
> +		gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>;
> +		default-on;
> +	};
> +
> +	clocks {
> +		codec_osc: codec_osc {

I cannot find how this fixed clock is used/referenced.

> +			#clock-cells = <0>;
> +			compatible = "fixed-clock";
> +			clock-frequency = <12000000>;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpio_leds>;
> +
> +		led0: user1 {
> +			label = "user1";
> +			gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>; /* MX6_PANLEDG */
> +			default-state = "on";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
> +		led1: user2 {
> +			label = "user2";
> +			gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>; /* MX6_PANLEDR */
> +			default-state = "off";
> +		};
> +
> +		led2: user3 {
> +			label = "user3";
> +			gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; /* MX6_LOCLED# */
> +			default-state = "off";
> +		};
> +	};
> +
> +	memory {

memory@10000000

> +		reg = <0x10000000 0x40000000>;
> +	};
> +
> +	pps {
> +		compatible = "pps-gpio";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pps>;
> +		gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_2p5v: regulator-2p5v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "2P5V";
> +		regulator-min-microvolt = <2500000>;
> +		regulator-max-microvolt = <2500000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3P3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_5p0v: regulator-5p0v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "5P0V";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_12p0v: regulator-12p0v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "12P0V";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_1p4v: regulator-vddsoc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_soc";
> +		regulator-min-microvolt = <1400000>;
> +		regulator-max-microvolt = <1400000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_usb_h1_vbus: regulator-usb-h1-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_h1_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_usb_otg_vbus: regulator-usb-otg-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_otg_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio3 22 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	sound {
> +		compatible = "fsl,imx6q-ventana-sgtl5000",
> +			     "fsl,imx-audio-sgtl5000";
> +		model = "sgtl5000-audio";
> +		ssi-controller = <&ssi1>;
> +		audio-codec = <&sgtl5000>;
> +		audio-routing =
> +			"MIC_IN", "Mic Jack",
> +			"Mic Jack", "Mic Bias",
> +			"Headphone Jack", "HP_OUT";
> +		mux-int-port = <1>;
> +		mux-ext-port = <4>;
> +	};
> +};
> +
> +&audmux {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_audmux>;
> +	status = "okay";
> +};
> +
> +&ecspi3 {
> +	cs-gpios = <&gpio4 24 GPIO_ACTIVE_HIGH>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi3>;
> +	status = "okay";
> +};
> +
> +&can1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_flexcan>;
> +	status = "okay";
> +};
> +
> +&clks {
> +	assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
> +			  <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
> +	assigned-clock-parents = <&clks IMX6QDL_CLK_PLL3_USB_OTG>,
> +				 <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> +};
> +
> +&fec {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_enet>;
> +	phy-mode = "rgmii-id";
> +	phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&hdmi {
> +	ddc-i2c-bus = <&i2c3>;
> +	status = "okay";
> +};
> +
> +&i2c1 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +
> +	eeprom1: eeprom@50 {
> +		compatible = "atmel,24c02";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};
> +
> +	eeprom2: eeprom@51 {
> +		compatible = "atmel,24c02";
> +		reg = <0x51>;
> +		pagesize = <16>;
> +	};
> +
> +	eeprom3: eeprom@52 {
> +		compatible = "atmel,24c02";
> +		reg = <0x52>;
> +		pagesize = <16>;
> +	};
> +
> +	eeprom4: eeprom@53 {
> +		compatible = "atmel,24c02";
> +		reg = <0x53>;
> +		pagesize = <16>;
> +	};
> +
> +	pca9555: gpio@23 {
> +		compatible = "nxp,pca9555";
> +		reg = <0x23>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> +
> +	ds1672: rtc@68 {
> +		compatible = "dallas,ds1672";
> +		reg = <0x68>;
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	status = "okay";
> +
> +	sgtl5000: codec@0a {

Drop the leading zero in unit-address.

> +		compatible = "fsl,sgtl5000";
> +		reg = <0x0a>;
> +		clocks = <&clks IMX6QDL_CLK_CKO>;
> +		VDDA-supply = <&reg_1p8v>;
> +		VDDIO-supply = <&reg_3p3v>;
> +	};
> +
> +	tca8418: keypad@34 {
> +		compatible = "ti,tca8418";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_keypad>;
> +		reg = <0x34>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
> +		linux,keymap = <0x00010100	/* BTN_0 HOME */
> +			        0x00000101	/* BTN_1 MENU */
> +			        0x01010102	/* BTN_2 ESCAPE */
> +			        0x01000103	/* BTN_3 BACK */
> +			        0x02000104	/* BTN_4 SEARCH */
> +			        0x00030105	/* BTN_5 UP */
> +			        0x00020106	/* BTN_6 RIGHT */
> +			        0x01030107	/* BTN_7 LEFT */
> +			        0x01020108	/* BTN_8 DOWN */
> +			        0x02020109>;	/* BTN_9 ENTER */

Please use MATRIX_KEY() and defines in linux-event-codes.h, so that we
can save the comments.

> +		keypad,num-rows = <4>;
> +		keypad,num-columns = <4>;
> +	};
> +
> +	ltc3676: pmic@3c {
> +		compatible = "lltc,ltc3676";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		reg = <0x3c>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
> +
> +		regulators {
> +			/* VDD_DDR (1+R1/R2 = 2.105) */
> +			reg_vdd_ddr: sw2 {
> +				regulator-name = "vddddr";
> +				regulator-min-microvolt = <868310>;
> +				regulator-max-microvolt = <1684000>;
> +				lltc,fb-voltage-divider = <221000 200000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			/* VDD_ARM (1+R1/R2 = 1.931) */
> +			reg_vdd_arm: sw3 {
> +				regulator-name = "vddarm";
> +				regulator-min-microvolt = <796551>;
> +				regulator-max-microvolt = <1544827>;
> +				lltc,fb-voltage-divider = <243000 261000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				linux,phandle = <&reg_vdd_arm>;
> +			};
> +
> +			/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
> +			reg_1p8v: sw4 {
> +				regulator-name = "vdd1p8";
> +				regulator-min-microvolt = <1033310>;
> +				regulator-max-microvolt = <2004000>;
> +				lltc,fb-voltage-divider = <301000 200000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			/* VDD_1P0 (1+R1/R2 = 1.39): PCIe/ENET-PHY */
> +			reg_1p0v: ldo2 {
> +				regulator-name = "vdd1p0";
> +				regulator-min-microvolt = <950000>;
> +				regulator-max-microvolt = <1050000>;
> +				lltc,fb-voltage-divider = <78700 200000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			/* VDD_AUD_1P8: Audio codec */
> +			reg_aud_1p8v: ldo3 {
> +				regulator-name = "vdd1p8a";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-boot-on;
> +			};
> +
> +			/* VDD_HIGH (1+R1/R2 = 4.17) */
> +			reg_3p0v: ldo4 {
> +				regulator-name = "vdd3p0";
> +				regulator-min-microvolt = <3023250>;
> +				regulator-max-microvolt = <3023250>;
> +				lltc,fb-voltage-divider = <634000 200000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +};
> +
> +&i2c3 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	status = "okay";
> +
> +	egalax_ts: touchscreen@04 {

Drop the leading zero in unit-address.

> +		compatible = "eeti,egalax_ts";
> +		reg = <0x04>;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
> +		wakeup-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +&ldb {
> +	fsl,dual-channel;
> +	status = "okay";
> +
> +	lvds-channel@0 {
> +		fsl,data-mapping = "spwg";
> +		fsl,data-width = <18>;
> +		status = "okay";
> +
> +		display-timings {
> +			native-mode = <&timing0>;
> +			timing0: hsd100pxn1 {
> +				clock-frequency = <65000000>;
> +				hactive = <1024>;
> +				vactive = <768>;
> +				hback-porch = <220>;
> +				hfront-porch = <40>;
> +				vback-porch = <21>;
> +				vfront-porch = <7>;
> +				hsync-len = <60>;
> +				vsync-len = <10>;
> +			};
> +		};
> +	};
> +};
> +
> +&pcie {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pcie>;
> +	reset-gpio = <&gpio4 31 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
> +&pwm2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm2>; /* MX6_DIO1 */
> +	status = "disabled";
> +};
> +
> +&pwm3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm3>; /* MX6_DIO2 */
> +	status = "disabled";
> +};
> +
> +&pwm4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm4>;
> +	status = "okay";
> +};
> +
> +&ssi1 {
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	uart-has-rtscts;
> +	rts-gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +};
> +
> +&uart5 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart5>;
> +	status = "okay";
> +};
> +
> +&usbotg {
> +	vbus-supply = <&reg_usb_otg_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbotg>;
> +	disable-over-current;
> +	status = "okay";
> +};
> +
> +&usbh1 {
> +	vbus-supply = <&reg_usb_h1_vbus>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbh1>;
> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	bus-width = <8>;
> +	vmmc-supply = <&reg_3p3v>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usdhc3 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc3>;
> +	pinctrl-1 = <&pinctrl_usdhc3_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
> +	cd-gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> +	vmmc-supply = <&reg_3p3v>;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +};
> +
> +&iomuxc {
> +	imx6qdl-gw560x {

Drop this container node.

> +		pinctrl_audmux: audmuxgrp {
> +			fsl,pins = <
> +				/* AUD4 */
> +				MX6QDL_PAD_DISP0_DAT20__AUD4_TXC	0x130b0
> +				MX6QDL_PAD_DISP0_DAT21__AUD4_TXD	0x110b0
> +				MX6QDL_PAD_DISP0_DAT22__AUD4_TXFS	0x130b0
> +				MX6QDL_PAD_DISP0_DAT23__AUD4_RXD	0x130b0
> +				MX6QDL_PAD_GPIO_0__CCM_CLKO1		0x130b0 /* AUD4_MCK */
> +				/* AUD6 */
> +				MX6QDL_PAD_DI0_PIN2__AUD6_TXD		0x130b0
> +				MX6QDL_PAD_DI0_PIN3__AUD6_TXFS		0x130b0
> +				MX6QDL_PAD_DI0_PIN4__AUD6_RXD		0x130b0
> +				MX6QDL_PAD_DI0_PIN15__AUD6_TXC		0x130b0
> +			>;
> +		};
> +
> +		pinctrl_ecspi3: escpi3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT0__ECSPI3_SCLK	0x100b1
> +				MX6QDL_PAD_DISP0_DAT1__ECSPI3_MOSI	0x100b1
> +				MX6QDL_PAD_DISP0_DAT2__ECSPI3_MISO	0x100b1
> +				MX6QDL_PAD_DISP0_DAT3__GPIO4_IO24	0x100b1
> +			>;
> +		};
> +
> +		pinctrl_enet: enetgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_RGMII_RXC__RGMII_RXC		0x1b030
> +				MX6QDL_PAD_RGMII_RD0__RGMII_RD0		0x1b030
> +				MX6QDL_PAD_RGMII_RD1__RGMII_RD1		0x1b030
> +				MX6QDL_PAD_RGMII_RD2__RGMII_RD2		0x1b030
> +				MX6QDL_PAD_RGMII_RD3__RGMII_RD3		0x1b030
> +				MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x1b030
> +				MX6QDL_PAD_RGMII_TXC__RGMII_TXC		0x1b030
> +				MX6QDL_PAD_RGMII_TD0__RGMII_TD0		0x1b030
> +				MX6QDL_PAD_RGMII_TD1__RGMII_TD1		0x1b030
> +				MX6QDL_PAD_RGMII_TD2__RGMII_TD2		0x1b030
> +				MX6QDL_PAD_RGMII_TD3__RGMII_TD3		0x1b030
> +				MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL	0x1b030
> +				MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK	0x1b0b0
> +				MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> +				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
> +				MX6QDL_PAD_GPIO_16__ENET_REF_CLK	0x4001b0a8
> +				MX6QDL_PAD_ENET_TXD0__GPIO1_IO30	0x4001b0b0 /* PHY_RST# */
> +			>;
> +		};
> +
> +		pinctrl_flexcan: flexcangrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_ROW2__FLEXCAN1_RX	0x1b0b1
> +				MX6QDL_PAD_KEY_COL2__FLEXCAN1_TX	0x1b0b1
> +				MX6QDL_PAD_GPIO_2__GPIO1_IO02		0x4001b0b0 /* CAN_STBY */
> +			>;
> +		};
> +
> +		pinctrl_gpio_leds: gpioledsgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL0__GPIO4_IO06		0x1b0b0
> +				MX6QDL_PAD_KEY_ROW0__GPIO4_IO07		0x1b0b0
> +				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15		0x1b0b0
> +			>;
> +		};
> +
> +		pinctrl_i2c1: i2c1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_EIM_D21__I2C1_SCL		0x4001b8b1
> +				MX6QDL_PAD_EIM_D28__I2C1_SDA		0x4001b8b1
> +			>;
> +		};
> +
> +		pinctrl_usbh1: usbh1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_9__GPIO1_IO09		0x4001b0b0 /* USBHUB_RST# */
> +			>;
> +		};

Please sort the pinctrl nodes alphabetically.

> +
> +		pinctrl_i2c2: i2c2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL3__I2C2_SCL		0x4001b8b1
> +				MX6QDL_PAD_KEY_ROW3__I2C2_SDA		0x4001b8b1
> +			>;
> +		};
> +
> +		pinctrl_pmic: pmicgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_8__GPIO1_IO08		0x0001b0b0 /* PMIC_IRQ# */
> +			>;
> +		};
> +
> +		pinctrl_i2c3: i2c3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_3__I2C3_SCL		0x4001b8b1
> +				MX6QDL_PAD_GPIO_6__I2C3_SDA		0x4001b8b1
> +

Drop this newline.

> +				/* Misc */
> +				MX6QDL_PAD_GPIO_19__GPIO4_IO05		0x4001b0b0 /* DIOI2C_DIS# */
> +

Ditto

Shawn

> +				/* Backlight / Touch Connector */
> +				MX6QDL_PAD_DISP0_DAT18__GPIO5_IO12	0x0001b0b0 /* LVDS_TOUCH_IRQ# */
> +				MX6QDL_PAD_DISP0_DAT19__GPIO5_IO13	0x0001b0b0 /* LVDS_BACKEN */
> +			>;
> +		};
> +
> +		pinctrl_keypad: keypadgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT17__GPIO5_IO11	0x0001b0b0 /* KEYPAD_IRQ# */
> +				MX6QDL_PAD_DISP0_DAT9__GPIO4_IO30	0x0001b0b0 /* KEYPAD_LED_EN */
> +			>;
> +		};
> +
> +		pinctrl_pcie: pciegrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT10__GPIO4_IO31	0x1b0b0    /* PCI_RST# */
> +				MX6QDL_PAD_GPIO_17__GPIO7_IO12		0x4001b0b0 /* PCIESKT_WDIS# */
> +			>;
> +		};
> +
> +		pinctrl_pps: ppsgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_ENET_RXD1__GPIO1_IO26	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_pwm2: pwm2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD1_DAT2__PWM2_OUT		0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_pwm3: pwm3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD1_DAT1__PWM3_OUT		0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_pwm4: pwm4grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD1_CMD__PWM4_OUT		0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
> +			>;
> +		};
> +
> +		pinctrl_uart2: uart2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD4_DAT7__UART2_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD4_DAT4__UART2_RX_DATA	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_uart5: uart5grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL1__UART5_TX_DATA	0x1b0b1
> +				MX6QDL_PAD_KEY_ROW1__UART5_RX_DATA	0x1b0b1
> +			>;
> +		};
> +
> +		pinctrl_usbotg: usbotggrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_GPIO_1__USB_OTG_ID		0x17059
> +				MX6QDL_PAD_EIM_D22__GPIO3_IO22		0x1b0b0 /* PWR_EN */
> +				MX6QDL_PAD_KEY_COL4__GPIO4_IO14		0x1b0b0 /* OC */
> +			>;
> +		};
> +
> +		pinctrl_usdhc2: usdhc2grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD2_CMD__SD2_CMD		0x170f9
> +				MX6QDL_PAD_SD2_CLK__SD2_CLK		0x100f9
> +				MX6QDL_PAD_SD2_DAT0__SD2_DATA0		0x170f9
> +				MX6QDL_PAD_SD2_DAT1__SD2_DATA1		0x170f9
> +				MX6QDL_PAD_SD2_DAT2__SD2_DATA2		0x170f9
> +				MX6QDL_PAD_SD2_DAT3__SD2_DATA3		0x170f9
> +				MX6QDL_PAD_NANDF_D4__SD2_DATA4		0x170f9
> +				MX6QDL_PAD_NANDF_D5__SD2_DATA5		0x170f9
> +				MX6QDL_PAD_NANDF_D6__SD2_DATA6		0x170f9
> +				MX6QDL_PAD_NANDF_D7__SD2_DATA7		0x170f9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3: usdhc3grp {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x17059
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x10059
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x17059
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x17059
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x17059
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x17059
> +				MX6QDL_PAD_SD3_DAT5__GPIO7_IO00		0x17059 /* CD */
> +				MX6QDL_PAD_NANDF_CS1__SD3_VSELECT	0x17059
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_100mhz: usdhc3grp100mhz {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x170b9
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x100b9
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x170b9
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x170b9
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x170b9
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x170b9
> +				MX6QDL_PAD_SD3_DAT5__GPIO7_IO00		0x170b9 /* CD */
> +				MX6QDL_PAD_NANDF_CS1__SD3_VSELECT	0x170b9
> +			>;
> +		};
> +
> +		pinctrl_usdhc3_200mhz: usdhc3grp200mhz {
> +			fsl,pins = <
> +				MX6QDL_PAD_SD3_CMD__SD3_CMD		0x170f9
> +				MX6QDL_PAD_SD3_CLK__SD3_CLK		0x100f9
> +				MX6QDL_PAD_SD3_DAT0__SD3_DATA0		0x170f9
> +				MX6QDL_PAD_SD3_DAT1__SD3_DATA1		0x170f9
> +				MX6QDL_PAD_SD3_DAT2__SD3_DATA2		0x170f9
> +				MX6QDL_PAD_SD3_DAT3__SD3_DATA3		0x170f9
> +				MX6QDL_PAD_SD3_DAT5__GPIO7_IO00		0x170f9 /* CD */
> +				MX6QDL_PAD_NANDF_CS1__SD3_VSELECT	0x170f9
> +			>;
> +		};
> +
> +		pinctrl_wdog: wdoggrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_DISP0_DAT8__WDOG1_B		0x1b0b0
> +			>;
> +		};
> +	};
> +};
> -- 
> 2.7.4
> 
--
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 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
From: Daniel Vetter @ 2017-05-02  8:16 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	Linux Kernel Mailing List, dri-devel, Rob Herring
In-Reply-To: <CADnq5_OEX2kzLJ5ABfBs6fZnmhFmJAPbg1XjJWx9Pt_pu=bTEA@mail.gmail.com>

On Mon, Apr 24, 2017 at 10:26:45AM -0400, Alex Deucher wrote:
> On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt <eric@anholt.net> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric@anholt.net> wrote:
> >>> Daniel Vetter <daniel@ffwll.ch> writes:
> >>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
> >>>>> The FBDEV initialization would throw an error in dmesg, when we just
> >>>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
> >>>>>
> >>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
> >>>>
> >>>> Hm, this shouldn't be an error really, you might want to hotplug more
> >>>> connectors later on. What exactly complains?
> >>>
> >>> drm_fb_helper_init() throws an error if the passed in connector count is
> >>> 0, so drm_fb_cma_helper() printks an error.
> >>
> >> Oh, _that_ thing. The error in there is correct, but (almost) everyone
> >> gets this parameter wrong. This isn't the max number of connectors the
> >> fb helper will light up, but just the max number of connectors _per_
> >> crtc when driving in hw clone mode. There's two problems with that:
> >> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
> >> active connector
> >> - I mentioned that everyone gets this wrong?
> >>
> >> If you're moderately bored it'd be great to nuke the max_connector
> >> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
> >> comment explaining that this needs to be changed, probably with
> >> dynamic reallocation, once someone gets around to implementing hw
> >> clone mode).
> >>
> >> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
> >> TODO.rst entry would be great in that case.
> >
> > If I'm driving a GPU with no display subsystem at all, it seems like I
> > shouldn't initialize fbdev for it, right?
> 
> That's what we do for radeon/amdgpu on hw without display blocks.

Yeah I got slightly distracted with the error :-)

Still might make sense to put this logic into the helpers, if there's no
crtc then don't bother initializing the fbcon. But imo the better cleanup
is getting rid of the connectors parameter, since most drivers get it
wrong, then respin the vc4 patch to explain what it's really doing (it's
not about the error really, it's about not registering and fbdev that's
not doing anything).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH] ARM: dts: omap4-droid4: Add isl29030 ALS/proximity sensor
From: Sebastian Reichel @ 2017-05-02  8:03 UTC (permalink / raw)
  To: Sebastian Reichel, Tony Lindgren
  Cc: Rob Herring, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sebastian Reichel

The Droid 4 has a isl29030 to measure ambient light (e.g. for
automatically adapting display brightness) and proximity.

Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
Hi,

The (trivial) driver changes have already been queued to iio-testing:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=e530cc8408770e6451e4d5c5e8a2a45132689cc1

-- Sebastian
---
 arch/arm/boot/dts/omap4-droid4-xt894.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index 89eb607f4a9e..1ad442ee6f30 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -348,6 +348,17 @@
 		interrupt-names = "irq", "wakeup";
 		wakeup-source;
 	};
+
+	isl29030@44 {
+		compatible = "isil,isl29030";
+		reg = <0x44>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&als_proximity_pins>;
+
+		interrupt-parent = <&gpio6>;
+		interrupts = <17 IRQ_TYPE_LEVEL_LOW>; /* gpio177 */
+	};
 };
 
 &omap4_pmx_core {
@@ -395,6 +406,12 @@
 		>;
 	};
 
+	als_proximity_pins: pinmux_als_proximity_pins {
+		pinctrl-single,pins = <
+		OMAP4_IOPAD(0x18c, PIN_INPUT_PULLUP | MUX_MODE3)
+		>;
+	};
+
 	usb_ulpi_pins: pinmux_usb_ulpi_pins {
 		pinctrl-single,pins = <
 		OMAP4_IOPAD(0x196, MUX_MODE7)
-- 
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

^ permalink raw reply related

* Re: [PATCH v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Benjamin Herrenschmidt @ 2017-05-02  7:59 UTC (permalink / raw)
  To: Joel Stanley, Greg Kroah-Hartman, Jiri Slaby
  Cc: Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko, Mark Rutland,
	Rob Herring, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170502074543.1380-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Tue, 2017-05-02 at 17:15 +0930, Joel Stanley wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> 
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
> 
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
> 
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
> 
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
> 
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
> 
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Reviewed-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>

> ---
> v4:
>  - Reorder if statements
>  - Remove uncessary comment
> 
> v3:
>  - remove whitespace in header
>  - reformat comment
>  - don't check for reg-io-width property; we don't need any special
>    accessors for reading/writing bytes
>  - move file to 8250_aspeed_vuart.c
> 
> v2:
>  - Use attribute groups and DEVICE_ATTR_RW
>  - Use platform_get_resource/devm_ioremap_resource
>  - of_find_property -> of_property_read_bool
>  - Drop unncessary 0xff mask
>  - Fix comment style
>  - Use BIT and GENMASK where pssible
>  - Move to 8250 directory
>  - Rename ast -> aspeed to match other Aspeed drivers
>  - Add documentation of sysfs file
>  - Add detail to the commit message
> 
>  Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
>  Documentation/devicetree/bindings/serial/8250.txt  |   2 +
>  drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig                    |  10 +
>  drivers/tty/serial/8250/Makefile                   |   1 +
>  5 files changed, 351 insertions(+)
>  create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
>  create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
> 
> diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> new file mode 100644
> index 000000000000..8062953ce77b
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> @@ -0,0 +1,15 @@
> +What:		/sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
> +Date:		April 2017
> +Contact:	Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:	Configures which IO port the host side of the UART
> +		will appear on the host <-> BMC LPC bus.
> +Users:		OpenBMC.  Proposed changes should be mailed to
> +		openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> +
> +What:		/sys/bus/platform/drivers/aspeed-vuart*/sirq
> +Date:		April 2017
> +Contact:	Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:	Configures which interrupt number the host side of
> +		the UART will appear on the host <-> BMC LPC bus.
> +Users:		OpenBMC.  Proposed changes should be mailed to
> +		openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 10276a46ecef..656733949309 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -20,6 +20,8 @@ Required properties:
>  	- "fsl,16550-FIFO64"
>  	- "fsl,ns16550"
>  	- "ti,da830-uart"
> +	- "aspeed,ast2400-vuart"
> +	- "aspeed,ast2500-vuart"
>  	- "serial" if the port type is unknown.
>  - reg : offset and length of the register set for the device.
>  - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> new file mode 100644
> index 000000000000..822be4906763
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -0,0 +1,323 @@
> +/*
> + *  Serial Port driver for Aspeed VUART device
> + *
> + *    Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + *    Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License
> + *  as published by the Free Software Foundation; either version
> + *  2 of the License, or (at your option) any later version.
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250.h"
> +
> +#define ASPEED_VUART_GCRA		0x20
> +#define ASPEED_VUART_GCRA_VUART_EN		BIT(0)
> +#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
> +#define ASPEED_VUART_GCRB		0x24
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK	GENMASK(7, 4)
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT	4
> +#define ASPEED_VUART_ADDRL		0x28
> +#define ASPEED_VUART_ADDRH		0x2c
> +
> +struct aspeed_vuart {
> +	struct device		*dev;
> +	void __iomem		*regs;
> +	struct clk		*clk;
> +	int			line;
> +};
> +
> +/*
> + * The VUART is basically two UART 'front ends' connected by their FIFO
> + * (no actual serial line in between). One is on the BMC side (management
> + * controller) and one is on the host CPU side.
> + *
> + * It allows the BMC to provide to the host a "UART" that pipes into
> + * the BMC itself and can then be turned by the BMC into a network console
> + * of some sort for example.
> + *
> + * This driver is for the BMC side. The sysfs files allow the BMC
> + * userspace which owns the system configuration policy, to specify
> + * at what IO port and interrupt number the host side will appear
> + * to the host on the Host <-> BMC LPC bus. It could be different on a
> + * different system (though most of them use 3f8/4).
> + */
> +
> +static ssize_t lpc_address_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	u16 addr;
> +
> +	addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
> +		(readb(vuart->regs + ASPEED_VUART_ADDRL));
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t lpc_address_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	unsigned long val;
> +	int err;
> +
> +	err = kstrtoul(buf, 0, &val);
> +	if (err)
> +		return err;
> +
> +	writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> +	writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(lpc_address);
> +
> +static ssize_t sirq_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	u8 reg;
> +
> +	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +	reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +	reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t count)
> +{
> +	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +	unsigned long val;
> +	int err;
> +	u8 reg;
> +
> +	err = kstrtoul(buf, 0, &val);
> +	if (err)
> +		return err;
> +
> +	val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +	val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> +	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +	reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +	reg |= val;
> +	writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(sirq);
> +
> +static struct attribute *aspeed_vuart_attrs[] = {
> +	&dev_attr_sirq.attr,
> +	&dev_attr_lpc_address.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group aspeed_vuart_attr_group = {
> +	.attrs = aspeed_vuart_attrs,
> +};
> +
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> +	u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +	if (enabled)
> +		reg |= ASPEED_VUART_GCRA_VUART_EN;
> +	else
> +		reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> +
> +	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> +					     bool discard)
> +{
> +	u8 reg;
> +
> +	reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +	/* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> +	if (!discard)
> +		reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +	else
> +		reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +
> +	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static int aspeed_vuart_startup(struct uart_port *uart_port)
> +{
> +	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +	int rc;
> +
> +	rc = serial8250_do_startup(uart_port);
> +	if (rc)
> +		return rc;
> +
> +	aspeed_vuart_set_host_tx_discard(vuart, false);
> +
> +	return 0;
> +}
> +
> +static void aspeed_vuart_shutdown(struct uart_port *uart_port)
> +{
> +	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +
> +	aspeed_vuart_set_host_tx_discard(vuart, true);
> +
> +	serial8250_do_shutdown(uart_port);
> +}
> +
> +static int aspeed_vuart_probe(struct platform_device *pdev)
> +{
> +	struct uart_8250_port port;
> +	struct aspeed_vuart *vuart;
> +	struct device_node *np;
> +	struct resource *res;
> +	u32 clk, prop;
> +	int rc;
> +
> +	np = pdev->dev.of_node;
> +
> +	vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> +	if (!vuart)
> +		return -ENOMEM;
> +
> +	vuart->dev = &pdev->dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	vuart->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(vuart->regs))
> +		return PTR_ERR(vuart->regs);
> +
> +	memset(&port, 0, sizeof(port));
> +	port.port.private_data = vuart;
> +	port.port.membase = vuart->regs;
> +	port.port.mapbase = res->start;
> +	port.port.mapsize = resource_size(res);
> +	port.port.startup = aspeed_vuart_startup;
> +	port.port.shutdown = aspeed_vuart_shutdown;
> +	port.port.dev = &pdev->dev;
> +
> +	rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (of_property_read_u32(np, "clock-frequency", &clk)) {
> +		vuart->clk = devm_clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(vuart->clk)) {
> +			dev_warn(&pdev->dev,
> +				"clk or clock-frequency not defined\n");
> +			return PTR_ERR(vuart->clk);
> +		}
> +
> +		rc = clk_prepare_enable(vuart->clk);
> +		if (rc < 0)
> +			return rc;
> +
> +		clk = clk_get_rate(vuart->clk);
> +	}
> +
> +	/* If current-speed was set, then try not to change it. */
> +	if (of_property_read_u32(np, "current-speed", &prop) == 0)
> +		port.port.custom_divisor = clk / (16 * prop);
> +
> +	/* Check for shifted address mapping */
> +	if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> +		port.port.mapbase += prop;
> +
> +	/* Check for registers offset within the devices address range */
> +	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> +		port.port.regshift = prop;
> +
> +	/* Check for fifo size */
> +	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> +		port.port.fifosize = prop;
> +
> +	/* Check for a fixed line number */
> +	rc = of_alias_get_id(np, "serial");
> +	if (rc >= 0)
> +		port.port.line = rc;
> +
> +	port.port.irq = irq_of_parse_and_map(np, 0);
> +	port.port.irqflags = IRQF_SHARED;
> +	port.port.iotype = UPIO_MEM;
> +	port.port.type = PORT_16550A;
> +	port.port.uartclk = clk;
> +	port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> +		| UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> +	if (of_property_read_bool(np, "no-loopback-test"))
> +		port.port.flags |= UPF_SKIP_TEST;
> +
> +	if (port.port.fifosize)
> +		port.capabilities = UART_CAP_FIFO;
> +
> +	if (of_property_read_bool(np, "auto-flow-control"))
> +		port.capabilities |= UART_CAP_AFE;
> +
> +	rc = serial8250_register_8250_port(&port);
> +	if (rc < 0)
> +		goto err_clk_disable;
> +
> +	vuart->line = rc;
> +
> +	aspeed_vuart_set_enabled(vuart, true);
> +	aspeed_vuart_set_host_tx_discard(vuart, true);
> +	platform_set_drvdata(pdev, vuart);
> +
> +	return 0;
> +
> +err_clk_disable:
> +	clk_disable_unprepare(vuart->clk);
> +	irq_dispose_mapping(port.port.irq);
> +	return rc;
> +}
> +
> +static int aspeed_vuart_remove(struct platform_device *pdev)
> +{
> +	struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
> +
> +	aspeed_vuart_set_enabled(vuart, false);
> +	serial8250_unregister_port(vuart->line);
> +	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +	clk_disable_unprepare(vuart->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id aspeed_vuart_table[] = {
> +	{ .compatible = "aspeed,ast2400-vuart" },
> +	{ .compatible = "aspeed,ast2500-vuart" },
> +	{ },
> +};
> +
> +static struct platform_driver aspeed_vuart_driver = {
> +	.driver = {
> +		.name = "aspeed-vuart",
> +		.of_match_table = aspeed_vuart_table,
> +	},
> +	.probe = aspeed_vuart_probe,
> +	.remove = aspeed_vuart_remove,
> +};
> +
> +module_platform_driver(aspeed_vuart_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Driver for Aspeed VUART device");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 0e3f529d50e9..a1161ec0256f 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called 8250_accent.
>  
> +config SERIAL_8250_ASPEED_VUART
> +	tristate "Aspeed Virtual UART"
> +	depends on SERIAL_8250
> +	depends on OF
> +	help
> +	  If you want to use the virtual UART (VUART) device on Aspeed
> +	  BMC platforms, enable this option. This enables the 16550A-
> +	  compatible device on the local LPC bus, giving a UART device
> +	  with no physical RS232 connections.
> +
>  config SERIAL_8250_BOCA
>  	tristate "Support Boca cards"
>  	depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 2f30f9ecdb1b..a44a99a3e623 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR)		+= 8250_exar.o
>  obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
>  obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
>  obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_ASPEED_VUART)	+= 8250_aspeed_vuart.o
>  obj-$(CONFIG_SERIAL_8250_BCM2835AUX)	+= 8250_bcm2835aux.o
>  obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
>  obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
--
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 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Andy Shevchenko @ 2017-05-02  7:51 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Greg Kroah-Hartman, Jiri Slaby, Jeremy Kerr,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree,
	Benjamin Herrenschmidt, Mark Rutland, Rob Herring,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170502074543.1380-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Tue, May 2, 2017 at 10:45 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> The VUART is two UART 'front ends' connected by their FIFO (no actual
> serial line in between). One is on the BMC side (management controller)
> and one is on the host CPU side.
>
> This driver is for the BMC side. The sysfs files allow the BMC
> userspace, which owns the system configuration policy, to specify at
> what IO port and interrupt number the host side will appear to the host
> on the Host <-> BMC LPC bus. It could be different on a different system
> (though most of them use 3f8/4).
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>

I have reviewed the driver and found it's quality good, though one
minor about IRQ getting is left untouched. Let's Greg or others to
decide, from my side FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
> v4:
>  - Reorder if statements
>  - Remove uncessary comment
>
> v3:
>  - remove whitespace in header
>  - reformat comment
>  - don't check for reg-io-width property; we don't need any special
>    accessors for reading/writing bytes
>  - move file to 8250_aspeed_vuart.c
>
> v2:
>  - Use attribute groups and DEVICE_ATTR_RW
>  - Use platform_get_resource/devm_ioremap_resource
>  - of_find_property -> of_property_read_bool
>  - Drop unncessary 0xff mask
>  - Fix comment style
>  - Use BIT and GENMASK where pssible
>  - Move to 8250 directory
>  - Rename ast -> aspeed to match other Aspeed drivers
>  - Add documentation of sysfs file
>  - Add detail to the commit message
>
>  Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
>  Documentation/devicetree/bindings/serial/8250.txt  |   2 +
>  drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig                    |  10 +
>  drivers/tty/serial/8250/Makefile                   |   1 +
>  5 files changed, 351 insertions(+)
>  create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
>  create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
>
> diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> new file mode 100644
> index 000000000000..8062953ce77b
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
> @@ -0,0 +1,15 @@
> +What:          /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
> +Date:          April 2017
> +Contact:       Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:   Configures which IO port the host side of the UART
> +               will appear on the host <-> BMC LPC bus.
> +Users:         OpenBMC.  Proposed changes should be mailed to
> +               openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> +
> +What:          /sys/bus/platform/drivers/aspeed-vuart*/sirq
> +Date:          April 2017
> +Contact:       Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> +Description:   Configures which interrupt number the host side of
> +               the UART will appear on the host <-> BMC LPC bus.
> +Users:         OpenBMC.  Proposed changes should be mailed to
> +               openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index 10276a46ecef..656733949309 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -20,6 +20,8 @@ Required properties:
>         - "fsl,16550-FIFO64"
>         - "fsl,ns16550"
>         - "ti,da830-uart"
> +       - "aspeed,ast2400-vuart"
> +       - "aspeed,ast2500-vuart"
>         - "serial" if the port type is unknown.
>  - reg : offset and length of the register set for the device.
>  - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> new file mode 100644
> index 000000000000..822be4906763
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -0,0 +1,323 @@
> +/*
> + *  Serial Port driver for Aspeed VUART device
> + *
> + *    Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + *    Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU General Public License
> + *  as published by the Free Software Foundation; either version
> + *  2 of the License, or (at your option) any later version.
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250.h"
> +
> +#define ASPEED_VUART_GCRA              0x20
> +#define ASPEED_VUART_GCRA_VUART_EN             BIT(0)
> +#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
> +#define ASPEED_VUART_GCRB              0x24
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK       GENMASK(7, 4)
> +#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT      4
> +#define ASPEED_VUART_ADDRL             0x28
> +#define ASPEED_VUART_ADDRH             0x2c
> +
> +struct aspeed_vuart {
> +       struct device           *dev;
> +       void __iomem            *regs;
> +       struct clk              *clk;
> +       int                     line;
> +};
> +
> +/*
> + * The VUART is basically two UART 'front ends' connected by their FIFO
> + * (no actual serial line in between). One is on the BMC side (management
> + * controller) and one is on the host CPU side.
> + *
> + * It allows the BMC to provide to the host a "UART" that pipes into
> + * the BMC itself and can then be turned by the BMC into a network console
> + * of some sort for example.
> + *
> + * This driver is for the BMC side. The sysfs files allow the BMC
> + * userspace which owns the system configuration policy, to specify
> + * at what IO port and interrupt number the host side will appear
> + * to the host on the Host <-> BMC LPC bus. It could be different on a
> + * different system (though most of them use 3f8/4).
> + */
> +
> +static ssize_t lpc_address_show(struct device *dev,
> +                               struct device_attribute *attr, char *buf)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       u16 addr;
> +
> +       addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
> +               (readb(vuart->regs + ASPEED_VUART_ADDRL));
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t lpc_address_store(struct device *dev,
> +                                struct device_attribute *attr,
> +                                const char *buf, size_t count)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       unsigned long val;
> +       int err;
> +
> +       err = kstrtoul(buf, 0, &val);
> +       if (err)
> +               return err;
> +
> +       writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> +       writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(lpc_address);
> +
> +static ssize_t sirq_show(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       u8 reg;
> +
> +       reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +       reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +       reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
> +                         const char *buf, size_t count)
> +{
> +       struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> +       unsigned long val;
> +       int err;
> +       u8 reg;
> +
> +       err = kstrtoul(buf, 0, &val);
> +       if (err)
> +               return err;
> +
> +       val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> +       val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> +       reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> +       reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +       reg |= val;
> +       writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(sirq);
> +
> +static struct attribute *aspeed_vuart_attrs[] = {
> +       &dev_attr_sirq.attr,
> +       &dev_attr_lpc_address.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group aspeed_vuart_attr_group = {
> +       .attrs = aspeed_vuart_attrs,
> +};
> +
> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
> +{
> +       u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +       if (enabled)
> +               reg |= ASPEED_VUART_GCRA_VUART_EN;
> +       else
> +               reg &= ~ASPEED_VUART_GCRA_VUART_EN;
> +
> +       writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
> +                                            bool discard)
> +{
> +       u8 reg;
> +
> +       reg = readb(vuart->regs + ASPEED_VUART_GCRA);
> +
> +       /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
> +       if (!discard)
> +               reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +       else
> +               reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
> +
> +       writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
> +}
> +
> +static int aspeed_vuart_startup(struct uart_port *uart_port)
> +{
> +       struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +       struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +       int rc;
> +
> +       rc = serial8250_do_startup(uart_port);
> +       if (rc)
> +               return rc;
> +
> +       aspeed_vuart_set_host_tx_discard(vuart, false);
> +
> +       return 0;
> +}
> +
> +static void aspeed_vuart_shutdown(struct uart_port *uart_port)
> +{
> +       struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> +       struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
> +
> +       aspeed_vuart_set_host_tx_discard(vuart, true);
> +
> +       serial8250_do_shutdown(uart_port);
> +}
> +
> +static int aspeed_vuart_probe(struct platform_device *pdev)
> +{
> +       struct uart_8250_port port;
> +       struct aspeed_vuart *vuart;
> +       struct device_node *np;
> +       struct resource *res;
> +       u32 clk, prop;
> +       int rc;
> +
> +       np = pdev->dev.of_node;
> +
> +       vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> +       if (!vuart)
> +               return -ENOMEM;
> +
> +       vuart->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       vuart->regs = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(vuart->regs))
> +               return PTR_ERR(vuart->regs);
> +
> +       memset(&port, 0, sizeof(port));
> +       port.port.private_data = vuart;
> +       port.port.membase = vuart->regs;
> +       port.port.mapbase = res->start;
> +       port.port.mapsize = resource_size(res);
> +       port.port.startup = aspeed_vuart_startup;
> +       port.port.shutdown = aspeed_vuart_shutdown;
> +       port.port.dev = &pdev->dev;
> +
> +       rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +       if (rc < 0)
> +               return rc;
> +
> +       if (of_property_read_u32(np, "clock-frequency", &clk)) {
> +               vuart->clk = devm_clk_get(&pdev->dev, NULL);
> +               if (IS_ERR(vuart->clk)) {
> +                       dev_warn(&pdev->dev,
> +                               "clk or clock-frequency not defined\n");
> +                       return PTR_ERR(vuart->clk);
> +               }
> +
> +               rc = clk_prepare_enable(vuart->clk);
> +               if (rc < 0)
> +                       return rc;
> +
> +               clk = clk_get_rate(vuart->clk);
> +       }
> +
> +       /* If current-speed was set, then try not to change it. */
> +       if (of_property_read_u32(np, "current-speed", &prop) == 0)
> +               port.port.custom_divisor = clk / (16 * prop);
> +
> +       /* Check for shifted address mapping */
> +       if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> +               port.port.mapbase += prop;
> +
> +       /* Check for registers offset within the devices address range */
> +       if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> +               port.port.regshift = prop;
> +
> +       /* Check for fifo size */
> +       if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> +               port.port.fifosize = prop;
> +
> +       /* Check for a fixed line number */
> +       rc = of_alias_get_id(np, "serial");
> +       if (rc >= 0)
> +               port.port.line = rc;
> +
> +       port.port.irq = irq_of_parse_and_map(np, 0);
> +       port.port.irqflags = IRQF_SHARED;
> +       port.port.iotype = UPIO_MEM;
> +       port.port.type = PORT_16550A;
> +       port.port.uartclk = clk;
> +       port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> +               | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> +       if (of_property_read_bool(np, "no-loopback-test"))
> +               port.port.flags |= UPF_SKIP_TEST;
> +
> +       if (port.port.fifosize)
> +               port.capabilities = UART_CAP_FIFO;
> +
> +       if (of_property_read_bool(np, "auto-flow-control"))
> +               port.capabilities |= UART_CAP_AFE;
> +
> +       rc = serial8250_register_8250_port(&port);
> +       if (rc < 0)
> +               goto err_clk_disable;
> +
> +       vuart->line = rc;
> +
> +       aspeed_vuart_set_enabled(vuart, true);
> +       aspeed_vuart_set_host_tx_discard(vuart, true);
> +       platform_set_drvdata(pdev, vuart);
> +
> +       return 0;
> +
> +err_clk_disable:
> +       clk_disable_unprepare(vuart->clk);
> +       irq_dispose_mapping(port.port.irq);
> +       return rc;
> +}
> +
> +static int aspeed_vuart_remove(struct platform_device *pdev)
> +{
> +       struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
> +
> +       aspeed_vuart_set_enabled(vuart, false);
> +       serial8250_unregister_port(vuart->line);
> +       sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
> +       clk_disable_unprepare(vuart->clk);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id aspeed_vuart_table[] = {
> +       { .compatible = "aspeed,ast2400-vuart" },
> +       { .compatible = "aspeed,ast2500-vuart" },
> +       { },
> +};
> +
> +static struct platform_driver aspeed_vuart_driver = {
> +       .driver = {
> +               .name = "aspeed-vuart",
> +               .of_match_table = aspeed_vuart_table,
> +       },
> +       .probe = aspeed_vuart_probe,
> +       .remove = aspeed_vuart_remove,
> +};
> +
> +module_platform_driver(aspeed_vuart_driver);
> +
> +MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Driver for Aspeed VUART device");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 0e3f529d50e9..a1161ec0256f 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
>           To compile this driver as a module, choose M here: the module
>           will be called 8250_accent.
>
> +config SERIAL_8250_ASPEED_VUART
> +       tristate "Aspeed Virtual UART"
> +       depends on SERIAL_8250
> +       depends on OF
> +       help
> +         If you want to use the virtual UART (VUART) device on Aspeed
> +         BMC platforms, enable this option. This enables the 16550A-
> +         compatible device on the local LPC bus, giving a UART device
> +         with no physical RS232 connections.
> +
>  config SERIAL_8250_BOCA
>         tristate "Support Boca cards"
>         depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 2f30f9ecdb1b..a44a99a3e623 100644
> --- a/drivers/tty/serial/8250/Makefile
> +++ b/drivers/tty/serial/8250/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR)                += 8250_exar.o
>  obj-$(CONFIG_SERIAL_8250_HP300)                += 8250_hp300.o
>  obj-$(CONFIG_SERIAL_8250_CS)           += serial_cs.o
>  obj-$(CONFIG_SERIAL_8250_ACORN)                += 8250_acorn.o
> +obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
>  obj-$(CONFIG_SERIAL_8250_BCM2835AUX)   += 8250_bcm2835aux.o
>  obj-$(CONFIG_SERIAL_8250_CONSOLE)      += 8250_early.o
>  obj-$(CONFIG_SERIAL_8250_FOURPORT)     += 8250_fourport.o
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko
--
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 v4 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Jeremy Kerr, linux-serial, linux-kernel, devicetree,
	Andy Shevchenko, Benjamin Herrenschmidt, Mark Rutland,
	Rob Herring, openbmc
In-Reply-To: <20170502074543.1380-1-joel@jms.id.au>

From: Jeremy Kerr <jk@ozlabs.org>

This change adds a driver for the 16550-based Aspeed virtual UART
device. We use a similar process to the of_serial driver for device
probe, but expose some VUART-specific functions through sysfs too.

The VUART is two UART 'front ends' connected by their FIFO (no actual
serial line in between). One is on the BMC side (management controller)
and one is on the host CPU side.

This driver is for the BMC side. The sysfs files allow the BMC
userspace, which owns the system configuration policy, to specify at
what IO port and interrupt number the host side will appear to the host
on the Host <-> BMC LPC bus. It could be different on a different system
(though most of them use 3f8/4).

OpenPOWER host firmware doesn't like it when the host-side of the
VUART's FIFO is not drained. This driver only disables host TX discard
mode when the port is in use. We set the VUART enabled bit when we bind
to the device, and clear it on unbind.

We don't want to do this on open/release, as the host may be using this
bit to configure serial output modes, which is independent of whether
the devices has been opened by BMC userspace.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Rob Herring <robh@kernel.org>

---
v4:
 - Reorder if statements
 - Remove uncessary comment

v3:
 - remove whitespace in header
 - reformat comment
 - don't check for reg-io-width property; we don't need any special
   accessors for reading/writing bytes
 - move file to 8250_aspeed_vuart.c

v2:
 - Use attribute groups and DEVICE_ATTR_RW
 - Use platform_get_resource/devm_ioremap_resource
 - of_find_property -> of_property_read_bool
 - Drop unncessary 0xff mask
 - Fix comment style
 - Use BIT and GENMASK where pssible
 - Move to 8250 directory
 - Rename ast -> aspeed to match other Aspeed drivers
 - Add documentation of sysfs file
 - Add detail to the commit message

 Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
 Documentation/devicetree/bindings/serial/8250.txt  |   2 +
 drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig                    |  10 +
 drivers/tty/serial/8250/Makefile                   |   1 +
 5 files changed, 351 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
 create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c

diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
new file mode 100644
index 000000000000..8062953ce77b
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
@@ -0,0 +1,15 @@
+What:		/sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
+Date:		April 2017
+Contact:	Jeremy Kerr <jk@ozlabs.org>
+Description:	Configures which IO port the host side of the UART
+		will appear on the host <-> BMC LPC bus.
+Users:		OpenBMC.  Proposed changes should be mailed to
+		openbmc@lists.ozlabs.org
+
+What:		/sys/bus/platform/drivers/aspeed-vuart*/sirq
+Date:		April 2017
+Contact:	Jeremy Kerr <jk@ozlabs.org>
+Description:	Configures which interrupt number the host side of
+		the UART will appear on the host <-> BMC LPC bus.
+Users:		OpenBMC.  Proposed changes should be mailed to
+		openbmc@lists.ozlabs.org
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 10276a46ecef..656733949309 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -20,6 +20,8 @@ Required properties:
 	- "fsl,16550-FIFO64"
 	- "fsl,ns16550"
 	- "ti,da830-uart"
+	- "aspeed,ast2400-vuart"
+	- "aspeed,ast2500-vuart"
 	- "serial" if the port type is unknown.
 - reg : offset and length of the register set for the device.
 - interrupts : should contain uart interrupt.
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
new file mode 100644
index 000000000000..822be4906763
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -0,0 +1,323 @@
+/*
+ *  Serial Port driver for Aspeed VUART device
+ *
+ *    Copyright (C) 2016 Jeremy Kerr <jk@ozlabs.org>, IBM Corp.
+ *    Copyright (C) 2006 Arnd Bergmann <arnd@arndb.de>, IBM Corp.
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+
+#include "8250.h"
+
+#define ASPEED_VUART_GCRA		0x20
+#define ASPEED_VUART_GCRA_VUART_EN		BIT(0)
+#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
+#define ASPEED_VUART_GCRB		0x24
+#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK	GENMASK(7, 4)
+#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT	4
+#define ASPEED_VUART_ADDRL		0x28
+#define ASPEED_VUART_ADDRH		0x2c
+
+struct aspeed_vuart {
+	struct device		*dev;
+	void __iomem		*regs;
+	struct clk		*clk;
+	int			line;
+};
+
+/*
+ * The VUART is basically two UART 'front ends' connected by their FIFO
+ * (no actual serial line in between). One is on the BMC side (management
+ * controller) and one is on the host CPU side.
+ *
+ * It allows the BMC to provide to the host a "UART" that pipes into
+ * the BMC itself and can then be turned by the BMC into a network console
+ * of some sort for example.
+ *
+ * This driver is for the BMC side. The sysfs files allow the BMC
+ * userspace which owns the system configuration policy, to specify
+ * at what IO port and interrupt number the host side will appear
+ * to the host on the Host <-> BMC LPC bus. It could be different on a
+ * different system (though most of them use 3f8/4).
+ */
+
+static ssize_t lpc_address_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	u16 addr;
+
+	addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
+		(readb(vuart->regs + ASPEED_VUART_ADDRL));
+
+	return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
+}
+
+static ssize_t lpc_address_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	unsigned long val;
+	int err;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+
+	writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+	writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(lpc_address);
+
+static ssize_t sirq_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	u8 reg;
+
+	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+	reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+	reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+
+	return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
+}
+
+static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+	unsigned long val;
+	int err;
+	u8 reg;
+
+	err = kstrtoul(buf, 0, &val);
+	if (err)
+		return err;
+
+	val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+	val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+	reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+	reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+	reg |= val;
+	writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(sirq);
+
+static struct attribute *aspeed_vuart_attrs[] = {
+	&dev_attr_sirq.attr,
+	&dev_attr_lpc_address.attr,
+	NULL,
+};
+
+static const struct attribute_group aspeed_vuart_attr_group = {
+	.attrs = aspeed_vuart_attrs,
+};
+
+static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
+{
+	u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+	if (enabled)
+		reg |= ASPEED_VUART_GCRA_VUART_EN;
+	else
+		reg &= ~ASPEED_VUART_GCRA_VUART_EN;
+
+	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
+					     bool discard)
+{
+	u8 reg;
+
+	reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+	/* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
+	if (!discard)
+		reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+	else
+		reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+
+	writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static int aspeed_vuart_startup(struct uart_port *uart_port)
+{
+	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+	int rc;
+
+	rc = serial8250_do_startup(uart_port);
+	if (rc)
+		return rc;
+
+	aspeed_vuart_set_host_tx_discard(vuart, false);
+
+	return 0;
+}
+
+static void aspeed_vuart_shutdown(struct uart_port *uart_port)
+{
+	struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+	struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+
+	aspeed_vuart_set_host_tx_discard(vuart, true);
+
+	serial8250_do_shutdown(uart_port);
+}
+
+static int aspeed_vuart_probe(struct platform_device *pdev)
+{
+	struct uart_8250_port port;
+	struct aspeed_vuart *vuart;
+	struct device_node *np;
+	struct resource *res;
+	u32 clk, prop;
+	int rc;
+
+	np = pdev->dev.of_node;
+
+	vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
+	if (!vuart)
+		return -ENOMEM;
+
+	vuart->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vuart->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(vuart->regs))
+		return PTR_ERR(vuart->regs);
+
+	memset(&port, 0, sizeof(port));
+	port.port.private_data = vuart;
+	port.port.membase = vuart->regs;
+	port.port.mapbase = res->start;
+	port.port.mapsize = resource_size(res);
+	port.port.startup = aspeed_vuart_startup;
+	port.port.shutdown = aspeed_vuart_shutdown;
+	port.port.dev = &pdev->dev;
+
+	rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+	if (rc < 0)
+		return rc;
+
+	if (of_property_read_u32(np, "clock-frequency", &clk)) {
+		vuart->clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(vuart->clk)) {
+			dev_warn(&pdev->dev,
+				"clk or clock-frequency not defined\n");
+			return PTR_ERR(vuart->clk);
+		}
+
+		rc = clk_prepare_enable(vuart->clk);
+		if (rc < 0)
+			return rc;
+
+		clk = clk_get_rate(vuart->clk);
+	}
+
+	/* If current-speed was set, then try not to change it. */
+	if (of_property_read_u32(np, "current-speed", &prop) == 0)
+		port.port.custom_divisor = clk / (16 * prop);
+
+	/* Check for shifted address mapping */
+	if (of_property_read_u32(np, "reg-offset", &prop) == 0)
+		port.port.mapbase += prop;
+
+	/* Check for registers offset within the devices address range */
+	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
+		port.port.regshift = prop;
+
+	/* Check for fifo size */
+	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+		port.port.fifosize = prop;
+
+	/* Check for a fixed line number */
+	rc = of_alias_get_id(np, "serial");
+	if (rc >= 0)
+		port.port.line = rc;
+
+	port.port.irq = irq_of_parse_and_map(np, 0);
+	port.port.irqflags = IRQF_SHARED;
+	port.port.iotype = UPIO_MEM;
+	port.port.type = PORT_16550A;
+	port.port.uartclk = clk;
+	port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
+		| UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
+
+	if (of_property_read_bool(np, "no-loopback-test"))
+		port.port.flags |= UPF_SKIP_TEST;
+
+	if (port.port.fifosize)
+		port.capabilities = UART_CAP_FIFO;
+
+	if (of_property_read_bool(np, "auto-flow-control"))
+		port.capabilities |= UART_CAP_AFE;
+
+	rc = serial8250_register_8250_port(&port);
+	if (rc < 0)
+		goto err_clk_disable;
+
+	vuart->line = rc;
+
+	aspeed_vuart_set_enabled(vuart, true);
+	aspeed_vuart_set_host_tx_discard(vuart, true);
+	platform_set_drvdata(pdev, vuart);
+
+	return 0;
+
+err_clk_disable:
+	clk_disable_unprepare(vuart->clk);
+	irq_dispose_mapping(port.port.irq);
+	return rc;
+}
+
+static int aspeed_vuart_remove(struct platform_device *pdev)
+{
+	struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
+
+	aspeed_vuart_set_enabled(vuart, false);
+	serial8250_unregister_port(vuart->line);
+	sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+	clk_disable_unprepare(vuart->clk);
+
+	return 0;
+}
+
+static const struct of_device_id aspeed_vuart_table[] = {
+	{ .compatible = "aspeed,ast2400-vuart" },
+	{ .compatible = "aspeed,ast2500-vuart" },
+	{ },
+};
+
+static struct platform_driver aspeed_vuart_driver = {
+	.driver = {
+		.name = "aspeed-vuart",
+		.of_match_table = aspeed_vuart_table,
+	},
+	.probe = aspeed_vuart_probe,
+	.remove = aspeed_vuart_remove,
+};
+
+module_platform_driver(aspeed_vuart_driver);
+
+MODULE_AUTHOR("Jeremy Kerr <jk@ozlabs.org>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for Aspeed VUART device");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 0e3f529d50e9..a1161ec0256f 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -224,6 +224,16 @@ config SERIAL_8250_ACCENT
 	  To compile this driver as a module, choose M here: the module
 	  will be called 8250_accent.
 
+config SERIAL_8250_ASPEED_VUART
+	tristate "Aspeed Virtual UART"
+	depends on SERIAL_8250
+	depends on OF
+	help
+	  If you want to use the virtual UART (VUART) device on Aspeed
+	  BMC platforms, enable this option. This enables the 16550A-
+	  compatible device on the local LPC bus, giving a UART device
+	  with no physical RS232 connections.
+
 config SERIAL_8250_BOCA
 	tristate "Support Boca cards"
 	depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 2f30f9ecdb1b..a44a99a3e623 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR)		+= 8250_exar.o
 obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
 obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
 obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_ASPEED_VUART)	+= 8250_aspeed_vuart.o
 obj-$(CONFIG_SERIAL_8250_BCM2835AUX)	+= 8250_bcm2835aux.o
 obj-$(CONFIG_SERIAL_8250_CONSOLE)	+= 8250_early.o
 obj-$(CONFIG_SERIAL_8250_FOURPORT)	+= 8250_fourport.o
-- 
2.11.0

^ permalink raw reply related

* [PATCH v4 1/2] serial: 8250: Add flag so drivers can avoid THRE probe
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, devicetree, Andy Shevchenko,
	Benjamin Herrenschmidt, Jeremy Kerr, Mark Rutland, Rob Herring,
	openbmc
In-Reply-To: <20170502074543.1380-1-joel@jms.id.au>

The probing of THRE irq behaviour assumes the other end will be reading
bytes out of the buffer in order to probe the port at driver init. In
some cases the other end cannot be relied upon to read these bytes, so
provide a flag for them to skip this step.

Bit 19 was chosen as the flags are a int and the top bits are taken.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>

---
v3:
 - No change

v3:
 - Correct the bit number in the changelog

v2:
 - No change

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/tty/serial/8250/8250_port.c | 2 +-
 include/linux/serial_core.h         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..60a6c247340f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2229,7 +2229,7 @@ int serial8250_do_startup(struct uart_port *port)
 		}
 	}
 
-	if (port->irq) {
+	if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
 		unsigned char iir1;
 		/*
 		 * Test for UARTs that do not reassert THRE when the
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 58484fb35cc8..260245deec94 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -195,6 +195,7 @@ struct uart_port {
 #define UPF_NO_TXEN_TEST	((__force upf_t) (1 << 15))
 #define UPF_MAGIC_MULTIPLIER	((__force upf_t) ASYNC_MAGIC_MULTIPLIER /* 16 */ )
 
+#define UPF_NO_THRE_TEST	((__force upf_t) (1 << 19))
 /* Port has hardware-assisted h/w flow control */
 #define UPF_AUTO_CTS		((__force upf_t) (1 << 20))
 #define UPF_AUTO_RTS		((__force upf_t) (1 << 21))
-- 
2.11.0

^ permalink raw reply related

* [PATCH v4 0/2] drivers: serial: Aspeed VUART driver
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
	Benjamin Herrenschmidt, Jeremy Kerr, Mark Rutland, Rob Herring,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ

This is v4 of a driver for the Aspeed VUART. This version addresses feedback
from Andy and Greg, and includes Rob's ack for the bindings change.

The VUART is a serial device on the BMC side of the LPC bus that connects a BMC
to it's host processor.

We add a flag to the serial core to allow the driver to skip probing of the
THRE irq behaviour, which could hang due to the host not reading bytes out of
the buffer.

We've been using this on systems for over a year, so it has seen a good amount
of testing.

Cheers,

Joel

Jeremy Kerr (1):
  drivers/serial: Add driver for Aspeed virtual UART

Joel Stanley (1):
  serial: 8250: Add flag so drivers can avoid THRE probe

 Documentation/ABI/stable/sysfs-driver-aspeed-vuart |  15 +
 Documentation/devicetree/bindings/serial/8250.txt  |   2 +
 drivers/tty/serial/8250/8250_aspeed_vuart.c        | 323 +++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c                |   2 +-
 drivers/tty/serial/8250/Kconfig                    |  10 +
 drivers/tty/serial/8250/Makefile                   |   1 +
 include/linux/serial_core.h                        |   1 +
 7 files changed, 353 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
 create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c

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

^ permalink raw reply

* Re: [PATCH v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02  7:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
	Jeremy Kerr, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree, Benjamin Herrenschmidt,
	OpenBMC Maillist
In-Reply-To: <CAHp75Vd=oONhzF34Pe8WwYKBWWxcsaQtgJy66aFD68iNKTxEfg@mail.gmail.com>

On Tue, Apr 11, 2017 at 9:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Apr 10, 2017 at 7:04 AM, Joel Stanley <joel@jms.id.au> wrote:
>> From: Jeremy Kerr <jk@ozlabs.org>
>>
>> This change adds a driver for the 16550-based Aspeed virtual UART
>> device. We use a similar process to the of_serial driver for device
>> probe, but expose some VUART-specific functions through sysfs too.
>>
>> The VUART is two UART 'front ends' connected by their FIFO (no actual
>> serial line in between). One is on the BMC side (management controller)
>> and one is on the host CPU side.
>>
>> This driver is for the BMC side. The sysfs files allow the BMC
>> userspace, which owns the system configuration policy, to specify at
>> what IO port and interrupt number the host side will appear to the host
>> on the Host <-> BMC LPC bus. It could be different on a different system
>> (though most of them use 3f8/4).
>>
>> OpenPOWER host firmware doesn't like it when the host-side of the
>> VUART's FIFO is not drained. This driver only disables host TX discard
>> mode when the port is in use. We set the VUART enabled bit when we bind
>> to the device, and clear it on unbind.
>>
>> We don't want to do this on open/release, as the host may be using this
>> bit to configure serial output modes, which is independent of whether
>> the devices has been opened by BMC userspace.
>
>> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
>> +{
>> +       u8 reg;
>> +
>> +       reg = readb(vuart->regs + ASPEED_VUART_GCRA);
>
>> +       reg &= ~ASPEED_VUART_GCRA_VUART_EN;
>> +       if (enabled)
>> +               reg |= ASPEED_VUART_GCRA_VUART_EN;
>
> Usually the pattern is
> if (something)
>  set x bit;
> else
>  clear x bit;
>
> It would make it one operation in any case and a bit more understandable.

I have made these ordering changes you requested.

>
>> +       port.port.irq = irq_of_parse_and_map(np, 0);
>
> The benefit of use platform_get_irq() is to get rid of some OF specific headers.

It's an of driver, and this function does exactly what we require. I
have left it in for v4.

Cheers,

Joel

^ 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