Devicetree
 help / color / mirror / Atom feed
* [PATCH V5 05/10] PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
From: Viresh Kumar @ 2016-11-29  6:36 UTC (permalink / raw)
  To: Rafael Wysocki, nm, sboyd, Viresh Kumar
  Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
	d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>

Pass the entire supply structure instead of all of its fields.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/base/power/opp/core.c | 44 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 8d6006151c9a..37fad2eb0f47 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -542,8 +542,7 @@ static struct clk *_get_opp_clk(struct device *dev)
 }
 
 static int _set_opp_voltage(struct device *dev, struct regulator *reg,
-			    unsigned long u_volt, unsigned long u_volt_min,
-			    unsigned long u_volt_max)
+			    struct dev_pm_opp_supply *supply)
 {
 	int ret;
 
@@ -554,14 +553,15 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
 		return 0;
 	}
 
-	dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__, u_volt_min,
-		u_volt, u_volt_max);
+	dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__,
+		supply->u_volt_min, supply->u_volt, supply->u_volt_max);
 
-	ret = regulator_set_voltage_triplet(reg, u_volt_min, u_volt,
-					    u_volt_max);
+	ret = regulator_set_voltage_triplet(reg, supply->u_volt_min,
+					    supply->u_volt, supply->u_volt_max);
 	if (ret)
 		dev_err(dev, "%s: failed to set voltage (%lu %lu %lu mV): %d\n",
-			__func__, u_volt_min, u_volt, u_volt_max, ret);
+			__func__, supply->u_volt_min, supply->u_volt,
+			supply->u_volt_max, ret);
 
 	return ret;
 }
@@ -583,8 +583,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 	struct regulator *reg;
 	struct clk *clk;
 	unsigned long freq, old_freq;
-	unsigned long u_volt, u_volt_min, u_volt_max;
-	unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
+	struct dev_pm_opp_supply old_supply, new_supply;
 	int ret;
 
 	if (unlikely(!target_freq)) {
@@ -634,17 +633,12 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		return ret;
 	}
 
-	if (IS_ERR(old_opp)) {
-		old_u_volt = 0;
-	} else {
-		old_u_volt = old_opp->supply.u_volt;
-		old_u_volt_min = old_opp->supply.u_volt_min;
-		old_u_volt_max = old_opp->supply.u_volt_max;
-	}
+	if (IS_ERR(old_opp))
+		old_supply.u_volt = 0;
+	else
+		memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
 
-	u_volt = opp->supply.u_volt;
-	u_volt_min = opp->supply.u_volt_min;
-	u_volt_max = opp->supply.u_volt_max;
+	memcpy(&new_supply, &opp->supply, sizeof(new_supply));
 
 	reg = opp_table->regulator;
 
@@ -652,8 +646,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	/* Scaling up? Scale voltage before frequency */
 	if (freq > old_freq) {
-		ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
-				       u_volt_max);
+		ret = _set_opp_voltage(dev, reg, &new_supply);
 		if (ret)
 			goto restore_voltage;
 	}
@@ -672,8 +665,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	/* Scaling down? Scale voltage after frequency */
 	if (freq < old_freq) {
-		ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
-				       u_volt_max);
+		ret = _set_opp_voltage(dev, reg, &new_supply);
 		if (ret)
 			goto restore_freq;
 	}
@@ -686,10 +678,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 			__func__, old_freq);
 restore_voltage:
 	/* This shouldn't harm even if the voltages weren't updated earlier */
-	if (old_u_volt) {
-		_set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
-				 old_u_volt_max);
-	}
+	if (old_supply.u_volt)
+		_set_opp_voltage(dev, reg, &old_supply);
 
 	return ret;
 }
-- 
2.7.1.410.g6faf27b


^ permalink raw reply related

* [PATCH V5 06/10] PM / OPP: Add infrastructure to manage multiple regulators
From: Viresh Kumar @ 2016-11-29  6:36 UTC (permalink / raw)
  To: Rafael Wysocki, nm-l0cyMroinI0, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	Viresh Kumar
  Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

This patch adds infrastructure to manage multiple regulators and updates
the only user (cpufreq-dt) of dev_pm_opp_set{put}_regulator().

This is preparatory work for adding full support for devices with
multiple regulators.

Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>

---
V4->V5:
- Don't allocate from within rcu locked section.
- s/+//
---
 drivers/base/power/opp/core.c    | 246 +++++++++++++++++++++++++++------------
 drivers/base/power/opp/debugfs.c |  52 +++++++--
 drivers/base/power/opp/of.c      | 103 +++++++++++-----
 drivers/base/power/opp/opp.h     |  10 +-
 drivers/cpufreq/cpufreq-dt.c     |   9 +-
 include/linux/pm_opp.h           |   8 +-
 6 files changed, 301 insertions(+), 127 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 37fad2eb0f47..89a3fd720724 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -93,6 +93,8 @@ struct opp_table *_find_opp_table(struct device *dev)
  * Return: voltage in micro volt corresponding to the opp, else
  * return 0
  *
+ * This is useful only for devices with single power supply.
+ *
  * Locking: This function must be called under rcu_read_lock(). opp is a rcu
  * protected pointer. This means that opp which could have been fetched by
  * opp_find_freq_{exact,ceil,floor} functions is valid as long as we are
@@ -112,7 +114,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 	if (IS_ERR_OR_NULL(tmp_opp))
 		pr_err("%s: Invalid parameters\n", __func__);
 	else
-		v = tmp_opp->supply.u_volt;
+		v = tmp_opp->supplies[0].u_volt;
 
 	return v;
 }
@@ -210,6 +212,24 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
 
+static int _get_regulator_count(struct device *dev)
+{
+	struct opp_table *opp_table;
+	int count;
+
+	rcu_read_lock();
+
+	opp_table = _find_opp_table(dev);
+	if (!IS_ERR(opp_table))
+		count = opp_table->regulator_count;
+	else
+		count = 0;
+
+	rcu_read_unlock();
+
+	return count;
+}
+
 /**
  * dev_pm_opp_get_max_volt_latency() - Get max voltage latency in nanoseconds
  * @dev: device for which we do this operation
@@ -222,34 +242,51 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
 {
 	struct opp_table *opp_table;
 	struct dev_pm_opp *opp;
-	struct regulator *reg;
+	struct regulator *reg, **regulators;
 	unsigned long latency_ns = 0;
-	unsigned long min_uV = ~0, max_uV = 0;
-	int ret;
+	int ret, i, count;
+	struct {
+		unsigned long min;
+		unsigned long max;
+	} *uV;
+
+	count = _get_regulator_count(dev);
+
+	/* Regulator may not be required for the device */
+	if (!count)
+		return 0;
+
+	regulators = kmalloc_array(count, sizeof(*regulators), GFP_KERNEL);
+	if (!regulators)
+		return 0;
+
+	uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
+	if (!uV)
+		goto free_regulators;
 
 	rcu_read_lock();
 
 	opp_table = _find_opp_table(dev);
 	if (IS_ERR(opp_table)) {
 		rcu_read_unlock();
-		return 0;
+		goto free_uV;
 	}
 
-	reg = opp_table->regulator;
-	if (IS_ERR(reg)) {
-		/* Regulator may not be required for device */
-		rcu_read_unlock();
-		return 0;
-	}
+	memcpy(regulators, opp_table->regulators, count * sizeof(*regulators));
 
-	list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
-		if (!opp->available)
-			continue;
+	for (i = 0; i < count; i++) {
+		uV[i].min = ~0;
+		uV[i].max = 0;
 
-		if (opp->supply.u_volt_min < min_uV)
-			min_uV = opp->supply.u_volt_min;
-		if (opp->supply.u_volt_max > max_uV)
-			max_uV = opp->supply.u_volt_max;
+		list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
+			if (!opp->available)
+				continue;
+
+			if (opp->supplies[i].u_volt_min < uV[i].min)
+				uV[i].min = opp->supplies[i].u_volt_min;
+			if (opp->supplies[i].u_volt_max > uV[i].max)
+				uV[i].max = opp->supplies[i].u_volt_max;
+		}
 	}
 
 	rcu_read_unlock();
@@ -258,9 +295,16 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
 	 * The caller needs to ensure that opp_table (and hence the regulator)
 	 * isn't freed, while we are executing this routine.
 	 */
-	ret = regulator_set_voltage_time(reg, min_uV, max_uV);
-	if (ret > 0)
-		latency_ns = ret * 1000;
+	for (i = 0; reg = regulators[i], i < count; i++) {
+		ret = regulator_set_voltage_time(reg, uV[i].min, uV[i].max);
+		if (ret > 0)
+			latency_ns += ret * 1000;
+	}
+
+free_uV:
+	kfree(uV);
+free_regulators:
+	kfree(regulators);
 
 	return latency_ns;
 }
@@ -580,7 +624,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
 	struct opp_table *opp_table;
 	struct dev_pm_opp *old_opp, *opp;
-	struct regulator *reg;
+	struct regulator *reg = ERR_PTR(-ENXIO);
 	struct clk *clk;
 	unsigned long freq, old_freq;
 	struct dev_pm_opp_supply old_supply, new_supply;
@@ -633,14 +677,23 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		return ret;
 	}
 
+	if (opp_table->regulators) {
+		/* This function only supports single regulator per device */
+		if (WARN_ON(opp_table->regulator_count > 1)) {
+			dev_err(dev, "multiple regulators not supported\n");
+			rcu_read_unlock();
+			return -EINVAL;
+		}
+
+		reg = opp_table->regulators[0];
+	}
+
 	if (IS_ERR(old_opp))
 		old_supply.u_volt = 0;
 	else
-		memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
-
-	memcpy(&new_supply, &opp->supply, sizeof(new_supply));
+		memcpy(&old_supply, old_opp->supplies, sizeof(old_supply));
 
-	reg = opp_table->regulator;
+	memcpy(&new_supply, opp->supplies, sizeof(new_supply));
 
 	rcu_read_unlock();
 
@@ -764,9 +817,6 @@ static struct opp_table *_add_opp_table(struct device *dev)
 
 	_of_init_opp_table(opp_table, dev);
 
-	/* Set regulator to a non-NULL error value */
-	opp_table->regulator = ERR_PTR(-ENXIO);
-
 	/* Find clk for the device */
 	opp_table->clk = clk_get(dev, NULL);
 	if (IS_ERR(opp_table->clk)) {
@@ -815,7 +865,7 @@ static void _remove_opp_table(struct opp_table *opp_table)
 	if (opp_table->prop_name)
 		return;
 
-	if (!IS_ERR(opp_table->regulator))
+	if (opp_table->regulators)
 		return;
 
 	/* Release clk */
@@ -924,35 +974,50 @@ struct dev_pm_opp *_allocate_opp(struct device *dev,
 				 struct opp_table **opp_table)
 {
 	struct dev_pm_opp *opp;
+	int count, supply_size;
+	struct opp_table *table;
 
-	/* allocate new OPP node */
-	opp = kzalloc(sizeof(*opp), GFP_KERNEL);
-	if (!opp)
+	table = _add_opp_table(dev);
+	if (!table)
 		return NULL;
 
-	INIT_LIST_HEAD(&opp->node);
+	/* Allocate space for at least one supply */
+	count = table->regulator_count ? table->regulator_count : 1;
+	supply_size = sizeof(*opp->supplies) * count;
 
-	*opp_table = _add_opp_table(dev);
-	if (!*opp_table) {
-		kfree(opp);
+	/* allocate new OPP node and supplies structures */
+	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
+	if (!opp) {
+		kfree(table);
 		return NULL;
 	}
 
+	/* Put the supplies at the end of the OPP structure as an empty array */
+	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
+	INIT_LIST_HEAD(&opp->node);
+
+	*opp_table = table;
+
 	return opp;
 }
 
 static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
 					 struct opp_table *opp_table)
 {
-	struct regulator *reg = opp_table->regulator;
-
-	if (!IS_ERR(reg) &&
-	    !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
-					    opp->supply.u_volt_max)) {
-		pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
-			__func__, opp->supply.u_volt_min,
-			opp->supply.u_volt_max);
-		return false;
+	struct regulator *reg;
+	int i;
+
+	for (i = 0; i < opp_table->regulator_count; i++) {
+		reg = opp_table->regulators[i];
+
+		if (!regulator_is_supported_voltage(reg,
+					opp->supplies[i].u_volt_min,
+					opp->supplies[i].u_volt_max)) {
+			pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
+				__func__, opp->supplies[i].u_volt_min,
+				opp->supplies[i].u_volt_max);
+			return false;
+		}
 	}
 
 	return true;
@@ -984,12 +1049,13 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 
 		/* Duplicate OPPs */
 		dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
-			 __func__, opp->rate, opp->supply.u_volt,
-			 opp->available, new_opp->rate, new_opp->supply.u_volt,
-			 new_opp->available);
+			 __func__, opp->rate, opp->supplies[0].u_volt,
+			 opp->available, new_opp->rate,
+			 new_opp->supplies[0].u_volt, new_opp->available);
 
+		/* Should we compare voltages for all regulators here ? */
 		return opp->available &&
-		       new_opp->supply.u_volt == opp->supply.u_volt ? 0 : -EEXIST;
+		       new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? 0 : -EEXIST;
 	}
 
 	new_opp->opp_table = opp_table;
@@ -1056,9 +1122,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 	/* populate the opp table */
 	new_opp->rate = freq;
 	tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
-	new_opp->supply.u_volt = u_volt;
-	new_opp->supply.u_volt_min = u_volt - tol;
-	new_opp->supply.u_volt_max = u_volt + tol;
+	new_opp->supplies[0].u_volt = u_volt;
+	new_opp->supplies[0].u_volt_min = u_volt - tol;
+	new_opp->supplies[0].u_volt_max = u_volt + tol;
 	new_opp->available = true;
 	new_opp->dynamic = dynamic;
 
@@ -1303,12 +1369,14 @@ void dev_pm_opp_put_prop_name(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
 
 /**
- * dev_pm_opp_set_regulator() - Set regulator name for the device
+ * dev_pm_opp_set_regulators() - Set regulator names for the device
  * @dev: Device for which regulator name is being set.
- * @name: Name of the regulator.
+ * @names: Array of pointers to the names of the regulator.
+ * @count: Number of regulators.
  *
  * In order to support OPP switching, OPP layer needs to know the name of the
- * device's regulator, as the core would be required to switch voltages as well.
+ * device's regulators, as the core would be required to switch voltages as
+ * well.
  *
  * This must be called before any OPPs are initialized for the device.
  *
@@ -1318,11 +1386,12 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-int dev_pm_opp_set_regulator(struct device *dev, const char *name)
+int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
+			      unsigned int count)
 {
 	struct opp_table *opp_table;
 	struct regulator *reg;
-	int ret;
+	int ret, i;
 
 	mutex_lock(&opp_table_lock);
 
@@ -1338,26 +1407,44 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name)
 		goto err;
 	}
 
-	/* Already have a regulator set */
-	if (WARN_ON(!IS_ERR(opp_table->regulator))) {
+	/* Already have regulators set */
+	if (WARN_ON(opp_table->regulators)) {
 		ret = -EBUSY;
 		goto err;
 	}
-	/* Allocate the regulator */
-	reg = regulator_get_optional(dev, name);
-	if (IS_ERR(reg)) {
-		ret = PTR_ERR(reg);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "%s: no regulator (%s) found: %d\n",
-				__func__, name, ret);
+
+	opp_table->regulators = kmalloc_array(count,
+					      sizeof(*opp_table->regulators),
+					      GFP_KERNEL);
+	if (!opp_table->regulators) {
+		ret = -ENOMEM;
 		goto err;
 	}
 
-	opp_table->regulator = reg;
+	for (i = 0; i < count; i++) {
+		reg = regulator_get_optional(dev, names[i]);
+		if (IS_ERR(reg)) {
+			ret = PTR_ERR(reg);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "%s: regulator (%s) not found: %d\n",
+					__func__, names[i], ret);
+			goto free_regulators;
+		}
+
+		opp_table->regulators[i] = reg;
+	}
+
+	opp_table->regulator_count = count;
 
 	mutex_unlock(&opp_table_lock);
 	return 0;
 
+free_regulators:
+	while (i != 0)
+		regulator_put(opp_table->regulators[--i]);
+
+	kfree(opp_table->regulators);
+	opp_table->regulators = NULL;
 err:
 	_remove_opp_table(opp_table);
 unlock:
@@ -1365,11 +1452,11 @@ int dev_pm_opp_set_regulator(struct device *dev, const char *name)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulators);
 
 /**
- * dev_pm_opp_put_regulator() - Releases resources blocked for regulator
- * @dev: Device for which regulator was set.
+ * dev_pm_opp_put_regulators() - Releases resources blocked for regulators
+ * @dev: Device for which regulators were set.
  *
  * Locking: The internal opp_table and opp structures are RCU protected.
  * Hence this function internally uses RCU updater strategy with mutex locks
@@ -1377,9 +1464,10 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-void dev_pm_opp_put_regulator(struct device *dev)
+void dev_pm_opp_put_regulators(struct device *dev)
 {
 	struct opp_table *opp_table;
+	int i;
 
 	mutex_lock(&opp_table_lock);
 
@@ -1391,16 +1479,20 @@ void dev_pm_opp_put_regulator(struct device *dev)
 		goto unlock;
 	}
 
-	if (IS_ERR(opp_table->regulator)) {
-		dev_err(dev, "%s: Doesn't have regulator set\n", __func__);
+	if (!opp_table->regulators) {
+		dev_err(dev, "%s: Doesn't have regulators set\n", __func__);
 		goto unlock;
 	}
 
 	/* Make sure there are no concurrent readers while updating opp_table */
 	WARN_ON(!list_empty(&opp_table->opp_list));
 
-	regulator_put(opp_table->regulator);
-	opp_table->regulator = ERR_PTR(-ENXIO);
+	for (i = opp_table->regulator_count - 1; i >= 0; i--)
+		regulator_put(opp_table->regulators[i]);
+
+	kfree(opp_table->regulators);
+	opp_table->regulators = NULL;
+	opp_table->regulator_count = 0;
 
 	/* Try freeing opp_table if this was the last blocking resource */
 	_remove_opp_table(opp_table);
@@ -1408,7 +1500,7 @@ void dev_pm_opp_put_regulator(struct device *dev)
 unlock:
 	mutex_unlock(&opp_table_lock);
 }
-EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulator);
+EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
 
 /**
  * dev_pm_opp_add()  - Add an OPP table from a table definitions
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index c897676ca35f..95f433db4ac7 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/limits.h>
+#include <linux/slab.h>
 
 #include "opp.h"
 
@@ -34,6 +35,46 @@ void opp_debug_remove_one(struct dev_pm_opp *opp)
 	debugfs_remove_recursive(opp->dentry);
 }
 
+static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
+				      struct opp_table *opp_table,
+				      struct dentry *pdentry)
+{
+	struct dentry *d;
+	int i = 0;
+	char *name;
+
+	/* Always create at least supply-0 directory */
+	do {
+		name = kasprintf(GFP_KERNEL, "supply-%d", i);
+
+		/* Create per-opp directory */
+		d = debugfs_create_dir(name, pdentry);
+
+		kfree(name);
+
+		if (!d)
+			return false;
+
+		if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d,
+					  &opp->supplies[i].u_volt))
+			return false;
+
+		if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d,
+					  &opp->supplies[i].u_volt_min))
+			return false;
+
+		if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d,
+					  &opp->supplies[i].u_volt_max))
+			return false;
+
+		if (!debugfs_create_ulong("u_amp", S_IRUGO, d,
+					  &opp->supplies[i].u_amp))
+			return false;
+	} while (++i < opp_table->regulator_count);
+
+	return true;
+}
+
 int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
 {
 	struct dentry *pdentry = opp_table->dentry;
@@ -63,16 +104,7 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
 	if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
 		return -ENOMEM;
 
-	if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->supply.u_volt))
-		return -ENOMEM;
-
-	if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->supply.u_volt_min))
-		return -ENOMEM;
-
-	if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->supply.u_volt_max))
-		return -ENOMEM;
-
-	if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->supply.u_amp))
+	if (!opp_debug_create_supplies(opp, opp_table, d))
 		return -ENOMEM;
 
 	if (!debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index bdf409d42126..3f7d2591b173 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/device.h>
 #include <linux/of.h>
+#include <linux/slab.h>
 #include <linux/export.h>
 
 #include "opp.h"
@@ -101,16 +102,16 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
 	return true;
 }
 
-/* TODO: Support multiple regulators */
 static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
 			      struct opp_table *opp_table)
 {
-	u32 microvolt[3] = {0};
-	u32 val;
-	int count, ret;
+	u32 *microvolt, *microamp = NULL;
+	int supplies, vcount, icount, ret, i, j;
 	struct property *prop = NULL;
 	char name[NAME_MAX];
 
+	supplies = opp_table->regulator_count ? opp_table->regulator_count : 1;
+
 	/* Search for "opp-microvolt-<name>" */
 	if (opp_table->prop_name) {
 		snprintf(name, sizeof(name), "opp-microvolt-%s",
@@ -128,34 +129,29 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
 			return 0;
 	}
 
-	count = of_property_count_u32_elems(opp->np, name);
-	if (count < 0) {
+	vcount = of_property_count_u32_elems(opp->np, name);
+	if (vcount < 0) {
 		dev_err(dev, "%s: Invalid %s property (%d)\n",
-			__func__, name, count);
-		return count;
+			__func__, name, vcount);
+		return vcount;
 	}
 
-	/* There can be one or three elements here */
-	if (count != 1 && count != 3) {
-		dev_err(dev, "%s: Invalid number of elements in %s property (%d)\n",
-			__func__, name, count);
+	/* There can be one or three elements per supply */
+	if (vcount != supplies && vcount != supplies * 3) {
+		dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+			__func__, name, vcount, supplies);
 		return -EINVAL;
 	}
 
-	ret = of_property_read_u32_array(opp->np, name, microvolt, count);
+	microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL);
+	if (!microvolt)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(opp->np, name, microvolt, vcount);
 	if (ret) {
 		dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
-		return -EINVAL;
-	}
-
-	opp->supply.u_volt = microvolt[0];
-
-	if (count == 1) {
-		opp->supply.u_volt_min = opp->supply.u_volt;
-		opp->supply.u_volt_max = opp->supply.u_volt;
-	} else {
-		opp->supply.u_volt_min = microvolt[1];
-		opp->supply.u_volt_max = microvolt[2];
+		ret = -EINVAL;
+		goto free_microvolt;
 	}
 
 	/* Search for "opp-microamp-<name>" */
@@ -172,10 +168,59 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
 		prop = of_find_property(opp->np, name, NULL);
 	}
 
-	if (prop && !of_property_read_u32(opp->np, name, &val))
-		opp->supply.u_amp = val;
+	if (prop) {
+		icount = of_property_count_u32_elems(opp->np, name);
+		if (icount < 0) {
+			dev_err(dev, "%s: Invalid %s property (%d)\n", __func__,
+				name, icount);
+			ret = icount;
+			goto free_microvolt;
+		}
 
-	return 0;
+		if (icount != supplies) {
+			dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+				__func__, name, icount, supplies);
+			ret = -EINVAL;
+			goto free_microvolt;
+		}
+
+		microamp = kmalloc_array(icount, sizeof(*microamp), GFP_KERNEL);
+		if (!microamp) {
+			ret = -EINVAL;
+			goto free_microvolt;
+		}
+
+		ret = of_property_read_u32_array(opp->np, name, microamp,
+						 icount);
+		if (ret) {
+			dev_err(dev, "%s: error parsing %s: %d\n", __func__,
+				name, ret);
+			ret = -EINVAL;
+			goto free_microamp;
+		}
+	}
+
+	for (i = 0, j = 0; i < supplies; i++) {
+		opp->supplies[i].u_volt = microvolt[j++];
+
+		if (vcount == supplies) {
+			opp->supplies[i].u_volt_min = opp->supplies[i].u_volt;
+			opp->supplies[i].u_volt_max = opp->supplies[i].u_volt;
+		} else {
+			opp->supplies[i].u_volt_min = microvolt[j++];
+			opp->supplies[i].u_volt_max = microvolt[j++];
+		}
+
+		if (microamp)
+			opp->supplies[i].u_amp = microamp[i];
+	}
+
+free_microamp:
+	kfree(microamp);
+free_microvolt:
+	kfree(microvolt);
+
+	return ret;
 }
 
 /**
@@ -304,8 +349,8 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 
 	pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
 		 __func__, new_opp->turbo, new_opp->rate,
-		 new_opp->supply.u_volt, new_opp->supply.u_volt_min,
-		 new_opp->supply.u_volt_max, new_opp->clock_latency_ns);
+		 new_opp->supplies[0].u_volt, new_opp->supplies[0].u_volt_min,
+		 new_opp->supplies[0].u_volt_max, new_opp->clock_latency_ns);
 
 	/*
 	 * Notify the changes in the availability of the operable
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 8a02516542c2..5b0f7e53bede 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -61,7 +61,7 @@ extern struct list_head opp_tables;
  * @turbo:	true if turbo (boost) OPP
  * @suspend:	true if suspend OPP
  * @rate:	Frequency in hertz
- * @supply:	Power supply voltage/current values
+ * @supplies:	Power supplies voltage/current values
  * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
  *		frequency from any other OPP's frequency.
  * @opp_table:	points back to the opp_table struct this opp belongs to
@@ -80,7 +80,7 @@ struct dev_pm_opp {
 	bool suspend;
 	unsigned long rate;
 
-	struct dev_pm_opp_supply supply;
+	struct dev_pm_opp_supply *supplies;
 
 	unsigned long clock_latency_ns;
 
@@ -139,7 +139,8 @@ enum opp_table_access {
  * @supported_hw_count: Number of elements in supported_hw array.
  * @prop_name: A name to postfix to many DT properties, while parsing them.
  * @clk: Device's clock handle
- * @regulator: Supply regulator
+ * @regulators: Supply regulators
+ * @regulator_count: Number of power supply regulators
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
  *
@@ -174,7 +175,8 @@ struct opp_table {
 	unsigned int supported_hw_count;
 	const char *prop_name;
 	struct clk *clk;
-	struct regulator *regulator;
+	struct regulator **regulators;
+	unsigned int regulator_count;
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 5c07ae05d69a..15cb26118dc7 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -186,7 +186,10 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	 */
 	name = find_supply_name(cpu_dev);
 	if (name) {
-		ret = dev_pm_opp_set_regulator(cpu_dev, name);
+		const char *names[] = {name};
+
+		ret = dev_pm_opp_set_regulators(cpu_dev, names,
+						ARRAY_SIZE(names));
 		if (ret) {
 			dev_err(cpu_dev, "Failed to set regulator for cpu%d: %d\n",
 				policy->cpu, ret);
@@ -285,7 +288,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 out_free_opp:
 	dev_pm_opp_of_cpumask_remove_table(policy->cpus);
 	if (name)
-		dev_pm_opp_put_regulator(cpu_dev);
+		dev_pm_opp_put_regulators(cpu_dev);
 out_put_clk:
 	clk_put(cpu_clk);
 
@@ -300,7 +303,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
 	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
 	dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
 	if (priv->reg_name)
-		dev_pm_opp_put_regulator(priv->cpu_dev);
+		dev_pm_opp_put_regulators(priv->cpu_dev);
 
 	clk_put(policy->clk);
 	kfree(priv);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index f69126e2bb59..27eea9bfc5ed 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -78,8 +78,8 @@ int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
 void dev_pm_opp_put_supported_hw(struct device *dev);
 int dev_pm_opp_set_prop_name(struct device *dev, const char *name);
 void dev_pm_opp_put_prop_name(struct device *dev);
-int dev_pm_opp_set_regulator(struct device *dev, const char *name);
-void dev_pm_opp_put_regulator(struct device *dev);
+int dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count);
+void dev_pm_opp_put_regulators(struct device *dev);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
 int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -186,12 +186,12 @@ static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
 
 static inline void dev_pm_opp_put_prop_name(struct device *dev) {}
 
-static inline int dev_pm_opp_set_regulator(struct device *dev, const char *name)
+static inline int dev_pm_opp_set_regulators(struct device *dev, const char *names[], unsigned int count)
 {
 	return -ENOTSUPP;
 }
 
-static inline void dev_pm_opp_put_regulator(struct device *dev) {}
+static inline void dev_pm_opp_put_regulators(struct device *dev) {}
 
 static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
-- 
2.7.1.410.g6faf27b

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

^ permalink raw reply related

* [PATCH V5 07/10] PM / OPP: Separate out _generic_set_opp()
From: Viresh Kumar @ 2016-11-29  6:36 UTC (permalink / raw)
  To: Rafael Wysocki, nm, sboyd, Viresh Kumar
  Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
	d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>

Later patches would add support for custom set_opp() callbacks. This
patch separates out the code for _generic_set_opp() handler in order to
prepare for that.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

---
V4->V5:
- Make 'dev' part of struct dev_pm_set_opp_data
- Fix commit log: s/opp_set_rate/set_opp
- Same in a comment as well
---
 drivers/base/power/opp/core.c | 181 +++++++++++++++++++++++++++++-------------
 drivers/base/power/opp/opp.h  |   3 +
 include/linux/pm_opp.h        |  35 ++++++++
 3 files changed, 166 insertions(+), 53 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 89a3fd720724..3a0b9d993c42 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -610,6 +610,69 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
 	return ret;
 }
 
+static inline int
+_generic_set_opp_clk_only(struct device *dev, struct clk *clk,
+			  unsigned long old_freq, unsigned long freq)
+{
+	int ret;
+
+	ret = clk_set_rate(clk, freq);
+	if (ret) {
+		dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
+			ret);
+	}
+
+	return ret;
+}
+
+static int _generic_set_opp(struct dev_pm_set_opp_data *data)
+{
+	struct dev_pm_opp_supply *old_supply = data->old_opp.supplies;
+	struct dev_pm_opp_supply *new_supply = data->new_opp.supplies;
+	unsigned long old_freq = data->old_opp.rate, freq = data->new_opp.rate;
+	struct regulator *reg = data->regulators[0];
+	struct device *dev= data->dev;
+	int ret;
+
+	/* This function only supports single regulator per device */
+	if (WARN_ON(data->regulator_count > 1)) {
+		dev_err(dev, "multiple regulators are not supported\n");
+		return -EINVAL;
+	}
+
+	/* Scaling up? Scale voltage before frequency */
+	if (freq > old_freq) {
+		ret = _set_opp_voltage(dev, reg, new_supply);
+		if (ret)
+			goto restore_voltage;
+	}
+
+	/* Change frequency */
+	ret = _generic_set_opp_clk_only(dev, data->clk, old_freq, freq);
+	if (ret)
+		goto restore_voltage;
+
+	/* Scaling down? Scale voltage after frequency */
+	if (freq < old_freq) {
+		ret = _set_opp_voltage(dev, reg, new_supply);
+		if (ret)
+			goto restore_freq;
+	}
+
+	return 0;
+
+restore_freq:
+	if (_generic_set_opp_clk_only(dev, data->clk, freq, old_freq))
+		dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+			__func__, old_freq);
+restore_voltage:
+	/* This shouldn't harm even if the voltages weren't updated earlier */
+	if (old_supply->u_volt)
+		_set_opp_voltage(dev, reg, old_supply);
+
+	return ret;
+}
+
 /**
  * dev_pm_opp_set_rate() - Configure new OPP based on frequency
  * @dev:	 device for which we do this operation
@@ -623,12 +686,12 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
 	struct opp_table *opp_table;
+	unsigned long freq, old_freq;
 	struct dev_pm_opp *old_opp, *opp;
-	struct regulator *reg = ERR_PTR(-ENXIO);
+	struct regulator **regulators;
+	struct dev_pm_set_opp_data *data;
 	struct clk *clk;
-	unsigned long freq, old_freq;
-	struct dev_pm_opp_supply old_supply, new_supply;
-	int ret;
+	int ret, size;
 
 	if (unlikely(!target_freq)) {
 		dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
@@ -677,64 +740,36 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		return ret;
 	}
 
-	if (opp_table->regulators) {
-		/* This function only supports single regulator per device */
-		if (WARN_ON(opp_table->regulator_count > 1)) {
-			dev_err(dev, "multiple regulators not supported\n");
-			rcu_read_unlock();
-			return -EINVAL;
-		}
+	dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,
+		old_freq, freq);
 
-		reg = opp_table->regulators[0];
+	regulators = opp_table->regulators;
+
+	/* Only frequency scaling */
+	if (!regulators) {
+		rcu_read_unlock();
+		return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
 	}
 
+	data = opp_table->set_opp_data;
+	data->regulators = regulators;
+	data->regulator_count = opp_table->regulator_count;
+	data->clk = clk;
+	data->dev = dev;
+
+	data->old_opp.rate = old_freq;
+	size = sizeof(*opp->supplies) * opp_table->regulator_count;
 	if (IS_ERR(old_opp))
-		old_supply.u_volt = 0;
+		memset(data->old_opp.supplies, 0, size);
 	else
-		memcpy(&old_supply, old_opp->supplies, sizeof(old_supply));
+		memcpy(data->old_opp.supplies, old_opp->supplies, size);
 
-	memcpy(&new_supply, opp->supplies, sizeof(new_supply));
+	data->new_opp.rate = freq;
+	memcpy(data->new_opp.supplies, opp->supplies, size);
 
 	rcu_read_unlock();
 
-	/* Scaling up? Scale voltage before frequency */
-	if (freq > old_freq) {
-		ret = _set_opp_voltage(dev, reg, &new_supply);
-		if (ret)
-			goto restore_voltage;
-	}
-
-	/* Change frequency */
-
-	dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n",
-		__func__, old_freq, freq);
-
-	ret = clk_set_rate(clk, freq);
-	if (ret) {
-		dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
-			ret);
-		goto restore_voltage;
-	}
-
-	/* Scaling down? Scale voltage after frequency */
-	if (freq < old_freq) {
-		ret = _set_opp_voltage(dev, reg, &new_supply);
-		if (ret)
-			goto restore_freq;
-	}
-
-	return 0;
-
-restore_freq:
-	if (clk_set_rate(clk, old_freq))
-		dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
-			__func__, old_freq);
-restore_voltage:
-	/* This shouldn't harm even if the voltages weren't updated earlier */
-	if (old_supply.u_volt)
-		_set_opp_voltage(dev, reg, &old_supply);
-
-	return ret;
+	return _generic_set_opp(data);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
 
@@ -1368,6 +1403,38 @@ void dev_pm_opp_put_prop_name(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
 
+static int _allocate_set_opp_data(struct opp_table *opp_table)
+{
+	struct dev_pm_set_opp_data *data;
+	int len, count = opp_table->regulator_count;
+
+	if (WARN_ON(!count))
+		return -EINVAL;
+
+	/* space for set_opp_data */
+	len = sizeof(*data);
+
+	/* space for old_opp.supplies and new_opp.supplies */
+	len += 2 * sizeof(struct dev_pm_opp_supply) * count;
+
+	data = kzalloc(len, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->old_opp.supplies = (void *)(data + 1);
+	data->new_opp.supplies = data->old_opp.supplies + count;
+
+	opp_table->set_opp_data = data;
+
+	return 0;
+}
+
+static void _free_set_opp_data(struct opp_table *opp_table)
+{
+	kfree(opp_table->set_opp_data);
+	opp_table->set_opp_data = NULL;
+}
+
 /**
  * dev_pm_opp_set_regulators() - Set regulator names for the device
  * @dev: Device for which regulator name is being set.
@@ -1436,6 +1503,11 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
 
 	opp_table->regulator_count = count;
 
+	/* Allocate block only once to pass to set_opp() routines */
+	ret = _allocate_set_opp_data(opp_table);
+	if (ret)
+		goto free_regulators;
+
 	mutex_unlock(&opp_table_lock);
 	return 0;
 
@@ -1445,6 +1517,7 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
 
 	kfree(opp_table->regulators);
 	opp_table->regulators = NULL;
+	opp_table->regulator_count = 0;
 err:
 	_remove_opp_table(opp_table);
 unlock:
@@ -1490,6 +1563,8 @@ void dev_pm_opp_put_regulators(struct device *dev)
 	for (i = opp_table->regulator_count - 1; i >= 0; i--)
 		regulator_put(opp_table->regulators[i]);
 
+	_free_set_opp_data(opp_table);
+
 	kfree(opp_table->regulators);
 	opp_table->regulators = NULL;
 	opp_table->regulator_count = 0;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 5b0f7e53bede..a05e43912c6b 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -141,6 +141,7 @@ enum opp_table_access {
  * @clk: Device's clock handle
  * @regulators: Supply regulators
  * @regulator_count: Number of power supply regulators
+ * @set_opp_data: Data to be passed to set_opp callback
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
  *
@@ -178,6 +179,8 @@ struct opp_table {
 	struct regulator **regulators;
 	unsigned int regulator_count;
 
+	struct dev_pm_set_opp_data *set_opp_data;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	char dentry_name[NAME_MAX];
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 27eea9bfc5ed..4723625bc16b 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -17,6 +17,8 @@
 #include <linux/err.h>
 #include <linux/notifier.h>
 
+struct clk;
+struct regulator;
 struct dev_pm_opp;
 struct device;
 
@@ -40,6 +42,39 @@ struct dev_pm_opp_supply {
 	unsigned long u_amp;
 };
 
+/**
+ * struct dev_pm_opp_info - OPP freq/voltage/current values
+ * @rate:	Target clk rate in hz
+ * @supplies:	Array of voltage/current values for all power supplies
+ *
+ * This structure stores the freq/voltage/current values for a single OPP.
+ */
+struct dev_pm_opp_info {
+	unsigned long rate;
+	struct dev_pm_opp_supply *supplies;
+};
+
+/**
+ * struct dev_pm_set_opp_data - Set OPP data
+ * @old_opp:	Old OPP info
+ * @new_opp:	New OPP info
+ * @regulators:	Array of regulator pointers
+ * @regulator_count: Number of regulators
+ * @clk:	Pointer to clk
+ * @dev:	Pointer to the struct device
+ *
+ * This structure contains all information required for setting an OPP.
+ */
+struct dev_pm_set_opp_data {
+	struct dev_pm_opp_info old_opp;
+	struct dev_pm_opp_info new_opp;
+
+	struct regulator **regulators;
+	unsigned int regulator_count;
+	struct clk *clk;
+	struct device *dev;
+};
+
 #if defined(CONFIG_PM_OPP)
 
 unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
-- 
2.7.1.410.g6faf27b


^ permalink raw reply related

* [PATCH V5 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks
From: Viresh Kumar @ 2016-11-29  6:36 UTC (permalink / raw)
  To: Rafael Wysocki, nm, sboyd, Viresh Kumar
  Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
	d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>

The generic set_opp() handler isn't sufficient for platforms with
complex DVFS.  For example, some TI platforms have multiple regulators
for a CPU device. The order in which various supplies need to be
programmed is only known to the platform code and its best to leave it
to it.

This patch implements APIs to register platform specific set_opp()
callback.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>

---
V4->V5:
- s/custom OPP set rate/custom set OPP/
- set_opp() doesn't have a separate 'dev' argument now.
---
 drivers/base/power/opp/core.c | 114 +++++++++++++++++++++++++++++++++++++++++-
 drivers/base/power/opp/opp.h  |   2 +
 include/linux/pm_opp.h        |  10 ++++
 3 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 3a0b9d993c42..ddd4915ffd4f 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -687,6 +687,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
 	struct opp_table *opp_table;
 	unsigned long freq, old_freq;
+	int (*set_opp)(struct dev_pm_set_opp_data *data);
 	struct dev_pm_opp *old_opp, *opp;
 	struct regulator **regulators;
 	struct dev_pm_set_opp_data *data;
@@ -751,6 +752,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
 	}
 
+	if (opp_table->set_opp)
+		set_opp = opp_table->set_opp;
+	else
+		set_opp = _generic_set_opp;
+
 	data = opp_table->set_opp_data;
 	data->regulators = regulators;
 	data->regulator_count = opp_table->regulator_count;
@@ -769,7 +775,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	rcu_read_unlock();
 
-	return _generic_set_opp(data);
+	return set_opp(data);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
 
@@ -903,6 +909,9 @@ static void _remove_opp_table(struct opp_table *opp_table)
 	if (opp_table->regulators)
 		return;
 
+	if (opp_table->set_opp)
+		return;
+
 	/* Release clk */
 	if (!IS_ERR(opp_table->clk))
 		clk_put(opp_table->clk);
@@ -1578,6 +1587,109 @@ void dev_pm_opp_put_regulators(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
 
 /**
+ * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
+ * @dev: Device for which the helper is getting registered.
+ * @set_opp: Custom set OPP helper.
+ *
+ * This is useful to support complex platforms (like platforms with multiple
+ * regulators per device), instead of the generic OPP set rate helper.
+ *
+ * This must be called before any OPPs are initialized for the device.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_register_set_opp_helper(struct device *dev,
+			int (*set_opp)(struct dev_pm_set_opp_data *data))
+{
+	struct opp_table *opp_table;
+	int ret;
+
+	if (!set_opp)
+		return -EINVAL;
+
+	mutex_lock(&opp_table_lock);
+
+	opp_table = _add_opp_table(dev);
+	if (!opp_table) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	/* This should be called before OPPs are initialized */
+	if (WARN_ON(!list_empty(&opp_table->opp_list))) {
+		ret = -EBUSY;
+		goto err;
+	}
+
+	/* Already have custom set_opp helper */
+	if (WARN_ON(opp_table->set_opp)) {
+		ret = -EBUSY;
+		goto err;
+	}
+
+	opp_table->set_opp = set_opp;
+
+	mutex_unlock(&opp_table_lock);
+	return 0;
+
+err:
+	_remove_opp_table(opp_table);
+unlock:
+	mutex_unlock(&opp_table_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_set_opp_helper);
+
+/**
+ * dev_pm_opp_register_put_opp_helper() - Releases resources blocked for
+ *					   set_opp helper
+ * @dev: Device for which custom set_opp helper has to be cleared.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_register_put_opp_helper(struct device *dev)
+{
+	struct opp_table *opp_table;
+
+	mutex_lock(&opp_table_lock);
+
+	/* Check for existing table for 'dev' first */
+	opp_table = _find_opp_table(dev);
+	if (IS_ERR(opp_table)) {
+		dev_err(dev, "Failed to find opp_table: %ld\n",
+			PTR_ERR(opp_table));
+		goto unlock;
+	}
+
+	if (!opp_table->set_opp) {
+		dev_err(dev, "%s: Doesn't have custom set_opp helper set\n",
+			__func__);
+		goto unlock;
+	}
+
+	/* Make sure there are no concurrent readers while updating opp_table */
+	WARN_ON(!list_empty(&opp_table->opp_list));
+
+	opp_table->set_opp = NULL;
+
+	/* Try freeing opp_table if this was the last blocking resource */
+	_remove_opp_table(opp_table);
+
+unlock:
+	mutex_unlock(&opp_table_lock);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_put_opp_helper);
+
+/**
  * dev_pm_opp_add()  - Add an OPP table from a table definitions
  * @dev:	device for which we do this operation
  * @freq:	Frequency in Hz for this OPP
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index a05e43912c6b..af9f2b849a66 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -141,6 +141,7 @@ enum opp_table_access {
  * @clk: Device's clock handle
  * @regulators: Supply regulators
  * @regulator_count: Number of power supply regulators
+ * @set_opp: Platform specific set_opp callback
  * @set_opp_data: Data to be passed to set_opp callback
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
@@ -179,6 +180,7 @@ struct opp_table {
 	struct regulator **regulators;
 	unsigned int regulator_count;
 
+	int (*set_opp)(struct dev_pm_set_opp_data *data);
 	struct dev_pm_set_opp_data *set_opp_data;
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 4723625bc16b..d62bd05d3b1d 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -115,6 +115,8 @@ int dev_pm_opp_set_prop_name(struct device *dev, const char *name);
 void dev_pm_opp_put_prop_name(struct device *dev);
 int dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count);
 void dev_pm_opp_put_regulators(struct device *dev);
+int dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
+void dev_pm_opp_register_put_opp_helper(struct device *dev);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
 int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -214,6 +216,14 @@ static inline int dev_pm_opp_set_supported_hw(struct device *dev,
 
 static inline void dev_pm_opp_put_supported_hw(struct device *dev) {}
 
+static inline int dev_pm_opp_register_set_opp_helper(struct device *dev,
+			int (*set_opp)(struct dev_pm_set_opp_data *data))
+{
+	return -ENOTSUPP;
+}
+
+static inline void dev_pm_opp_register_put_opp_helper(struct device *dev) {}
+
 static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
 {
 	return -ENOTSUPP;
-- 
2.7.1.410.g6faf27b


^ permalink raw reply related

* [PATCH V5 09/10] PM / OPP: Don't WARN on multiple calls to dev_pm_opp_set_regulators()
From: Viresh Kumar @ 2016-11-29  6:36 UTC (permalink / raw)
  To: Rafael Wysocki, nm, sboyd, Viresh Kumar
  Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh,
	d-gerlach, broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar@linaro.org>

If a platform specific OPP driver has called this routine first and set
the regulators, then the second call from cpufreq-dt driver will hit the
WARN_ON(). Remove the WARN_ON(), but continue to return error in such
cases.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/base/power/opp/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index ddd4915ffd4f..3c9f223709ab 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -1484,7 +1484,7 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
 	}
 
 	/* Already have regulators set */
-	if (WARN_ON(opp_table->regulators)) {
+	if (opp_table->regulators) {
 		ret = -EBUSY;
 		goto err;
 	}
-- 
2.7.1.410.g6faf27b


^ permalink raw reply related

* [PATCH V5 10/10] PM / OPP: Don't assume platform doesn't have regulators
From: Viresh Kumar @ 2016-11-29  6:36 UTC (permalink / raw)
  To: Rafael Wysocki, nm-l0cyMroinI0, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	Viresh Kumar
  Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Viresh Kumar
In-Reply-To: <cover.1480401041.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

If the regulators aren't set explicitly by the platform, the OPP core
assumes that the platform doesn't have any regulator and uses the
clk-only callback.

If the platform failed to register a regulator with the core, then this
can turn out to be a dangerous assumption as the OPP core will try to
change clk without changing regulators.

Handle that properly by making sure that the DT didn't have any entries
for supply voltages as well.

Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/base/power/opp/core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 3c9f223709ab..f7a5fb4dbf11 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -748,7 +748,20 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	/* Only frequency scaling */
 	if (!regulators) {
+		unsigned long u_volt = opp->supplies[0].u_volt;
+
 		rcu_read_unlock();
+
+		/*
+		 * DT contained supply ratings? Consider platform failed to set
+		 * regulators.
+		 */
+		if (unlikely(u_volt)) {
+			dev_err(dev, "%s: Regulator not registered with OPP core\n",
+				__func__);
+			return -EINVAL;
+		}
+
 		return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
 	}
 
-- 
2.7.1.410.g6faf27b

--
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 1/2] PM / Domains: Introduce domain-performance-state binding
From: Viresh Kumar @ 2016-11-29  6:57 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Kevin Hilman, Vincent Guittot, Rob Herring, Rafael Wysocki,
	linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
	linux-kernel, Mark Rutland, Ulf Hansson, Lina Iyer,
	devicetree@vger.kernel.org, Nayak Rajendra
In-Reply-To: <4f815e31-22d0-fef7-953c-257fa2bbcb9d@codeaurora.org>

On 28-11-16, 10:27, Stephen Boyd wrote:
> On 11/23/2016 08:40 PM, Viresh Kumar wrote:
> > But even in these cases we wouldn't be using the voltage values within the
> > kernel as we will be giving only a performance state to the M3 core, right?
> 
> Nope. In these cases we need to set a certain voltage and we do that by
> requesting it via the M3 core.

Don't we need something like this then ?

	parent: power-controller@12340000 {
		compatible = "foo,power-controller";
		reg = <0x12340000 0x1000>;
		#power-domain-cells = <0>;
		domain-performance-states = <&perf_state0>;
	};

	perf_state0: performance_states {
		pstate1: pstate@1 {
			index = <1>;
			/* Optional */
			microvolt = <970000 975000 985000>;
		};
		pstate2: pstate@2 {
			index = <2>;
			/* Optional */
			microvolt = <970000 975000 985000>;
		};
		pstate3: pstate@3 {
			index = <3>;
			/* Optional */
			microvolt = <970000 975000 985000>;
		};
	}

	cpus {
		cpu@0 {
			...
			power-domain = <&parent>;
			operating-points-v2 = <&cpu0_opp_table>;
		};
	};

	cpu0_opp_table: opp_table0 {
		compatible = "operating-points-v2";
		opp-shared;

		opp@1000000000 {
			opp-hz = /bits/ 64 <1000000000>;
			domain-performance-state = <&pstate1>;
		};
		opp@1100000000 {
			opp-hz = /bits/ 64 <1100000000>;
			domain-performance-state = <&pstate2>;
		};
		opp@1200000000 {
			opp-hz = /bits/ 64 <1200000000>;
			domain-performance-state = <&pstate3>;
		};
	};

-- 
viresh

^ permalink raw reply

* Re: [PATCH] arm64: dts: exynos: Add support for s6e3ha2 panel device for TM2
From: Marek Szyprowski @ 2016-11-29  7:12 UTC (permalink / raw)
  To: Hoegeun Kwon, kgene, krzk, devicetree
  Cc: linux-samsung-soc, Hyungwon Hwang, Andrzej Hajda, Chanwoo Choi
In-Reply-To: <1480400182-4485-1-git-send-email-hoegeun.kwon@samsung.com>

Hi Hyungwon,


On 2016-11-29 07:16, Hoegeun Kwon wrote:
> From: Hyungwon Hwang <human.hwang@samsung.com>
>
> This patch adds the Panel Device Tree node for s6e3ha2 display
> controller to Exynos5433 SoC dts.
>
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>   arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 35 +++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)

Please send an updated driver with dt binding description first, then 
this patch.

> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index db879f4..d27f27d 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -252,11 +252,46 @@
>   			reg = <1>;
>   
>   			dsi_out: endpoint {
> +				remote-endpoint = <&dsi_in>;
>   				samsung,burst-clock-frequency = <512000000>;
>   				samsung,esc-clock-frequency = <16000000>;
>   			};
>   		};
>   	};
> +
> +	panel@0 {
> +		compatible = "samsung,s6e3ha2";
> +		reg = <0>;
> +		vdd3-supply = <&ldo27_reg>;
> +		vci-supply = <&ldo28_reg>;
> +		reset-gpios = <&gpg0 0 0>;

Typically reset gpio is used with GPIO_ACTIVE_LOW flag, so handling this 
pin should be changed in the panel driver.

> +		panel-en-gpios = <&gpf1 5 0>;

This should be renamed to "enable-gpios", as pointed in 
https://patchwork.kernel.org/patch/5714111/

> +		te-gpios = <&gpf1 3 1>;
> +		power-on-delay= <5>;
> +		init-delay = <120>;
> +		panel-width-mm = <71>;
> +		panel-height-mm = <125>;
> +
> +		display-timings {
> +			timing-0 {
> +				clock-frequency = <14874444>;
> +				hactive = <1440>;
> +				vactive = <2560>;
> +				hfront-porch = <1>;
> +				hback-porch = <1>;
> +				hsync-len = <1>;
> +				vfront-porch = <1>;
> +				vback-porch = <15>;
> +				vsync-len = <1>;
> +			};
> +		};
> +
> +		port {
> +			dsi_in: endpoint {
> +				remote-endpoint = <&dsi_out>;
> +			};
> +		};
> +	};
>   };
>   
>   &hsi2c_0 {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply

* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-29  7:49 UTC (permalink / raw)
  To: Ziji Hu
  Cc: Gregory CLEMENT, Adrian Hunter,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
	Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Thomas Petazzoni,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
	Victor Gu, Wei(SOCP) Liu, Wilson Ding
In-Reply-To: <c30cead8-17b6-48b0-7355-cd82268842e1-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>

On 29 November 2016 at 03:53, Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> wrote:
> Hi Ulf,
>
> On 2016/11/28 23:16, Ulf Hansson wrote:
>> On 28 November 2016 at 12:38, Ziji Hu <huziji-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> wrote:
>>> Hi Ulf,
>>>
>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>>
>>>>>     As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>>>     send commands for testing current sampling point set in our host PHY.
>>>>>
>>>>>     According to my test result, it shows that mmc_send_tuning() can only support
>>>>>     tuning command (CMD21/CMD19).
>>>>>     As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>>>     which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>>>     SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>>>     speed modes.
>>>>>
>>>>>     Could you please provide suggestions for the speed mode in which tuning is
>>>>>     not available?
>>>>>
>>>>
>>>> Normally the mmc host driver shouldn't have to care about what the
>>>> card supports, as that is the responsibility of the mmc core to
>>>> manage.
>>>>
>>>> The host should only need to implement the ->execute_tuning() ops,
>>>> which gets called when the card supports tuning (CMD19/21). Does it
>>>> make sense?
>>>>
>>>    I think it is irrelevant to tuning procedure.
>>>
>>>    Our host requires to adjust PHY setting after each time ios setting
>>>    (SDCLK/bus width/speed mode) is changed.
>>>    The simplified sequence is:
>>>    mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>>>    adjust PHY setting.
>>>    During PHY setting adjustment, out host driver has to send commands to
>>>    test current sampling point. Tuning is another independent step.
>>
>> For those speed modes (or other ios changes) that *don't* requires
>> tuning, then what will you do when you send the command to confirm the
>> change of PHY setting and it fails?
>>
>> My assumption is that you will fail anyway, by propagating the error
>> to the mmc core. At least that what was my understanding from your
>> earlier replies, right!?
>>
>> Then, I think there are no point having the host driver sending a
>> command to confirm the PHY settings, as the mmc core will anyway
>> discover if something goes wrong when the next command is sent.
>>
>> Please correct me if I am wrong!
>>
>
>    Sorry that I didn't make myself clear.
>
>    Our host PHY delay line consists of hundreds of sampling points.
>    Each sampling point represents a different phase shift.
>
>    In lower speed mode, our host driver will scan the delay line.
>    It will select and test multiple sampling points, other than testing
>    only single sampling point.
>
>    If a sampling point fails to transfer cmd/data, our host driver will
>    move to test next sampling point, until we find out a group of successful
>    sampling points which can transfer cmd/data. At last we will select
>    a perfect one from them.

Ahh, I see. Unfortunate, this is going to be very hard to implement properly.

The main problem is that the host driver has *no* knowledge about the
internal state of the card, as that is the responsibility of the mmc
core to keep track of.

If the host driver would send a command during every update of the
"ios" setting, from ->set_ios(), for sure it would lead to commands
being sent that are "forbidden" in the current internal state of the
card.
This would lead to that the card initialization sequence fails,
because the card may move to an unknown internal state and the mmc
core would have no knowledge about what happened.

Hmm..

Can you specify, *exactly*, under which "ios updates" you need to
verify updated PHY setting changes by sending a cmd/data? Also, please
specify if it's enough to only test the CMD line or also DATA lines.

Kind regards
Uffe
--
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 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Archit Taneja @ 2016-11-29  8:11 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-arm-msm, robh, dri-devel, devicetree
In-Reply-To: <2390919.s2CzTWN1li@avalon>



On 11/29/2016 12:03 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> Thank you for the patch.
>
> On Tuesday 29 Nov 2016 11:37:41 Archit Taneja wrote:
>> Add the regulator supply properties needed by ADV7511 and ADV7533.
>>
>> The regulators are specified as optional properties since there can
>> be boards which have a fixed supply directly routed to the pins, and
>> these may not be modelled as regulator supplies.
>
> That's why we have support for dummy supplies in the kernel, isn't it ? Isn't
> it better to make the supplies mandatory in the bindings (and obviously
> handling them as optional in the driver for backward-compatibility) ?

I'm a bit unclear on this.

I thought we couldn't add mandatory properties once the device is already
present in DT for one or more platforms.

Say, if we do make it mandatory for future additions, we would need to have
DT property for the supplies for the new platforms. If the regulators on
these boards are fixed supplies, they would be need to be modeled
using "regulator-fixed", possibly without any input supply. Is that
what you're suggesting?

Thanks,
Archit

>
> Apart from that,
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>> Cc: devicetree@vger.kernel.org
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>> v3:
>> - Revert back to having a common avdd-supply property for the 1.8V
>>   supplies
>>
>> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 9 ++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
>> 6532a59..13d53bc 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> @@ -56,6 +56,15 @@ Optional properties:
>>  - adi,disable-timing-generator: Only for ADV7533. Disables the internal
>> timing generator. The chip will rely on the sync signals in the DSI data
>> lanes, rather than generate its own timings for HDMI output.
>> +- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
>> +  pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also
>> powers
>> +  up the A2VDD pin.
>> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
>> +  ADV7511 and V3P3 on ADV7533.
>> +
>> +ADV7533 specific supplies:
>> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
>> +  either 1.2V or 1.8V.
>>
>>  Required nodes:
>

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

^ permalink raw reply

* Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
From: Laurent Pinchart @ 2016-11-29  8:27 UTC (permalink / raw)
  To: dri-devel
  Cc: Rob Herring, linux-renesas-soc, Tomi Valkeinen, Laurent Pinchart,
	devicetree
In-Reply-To: <2307840.2mk6E40XHl@avalon>

Hi Rob,

On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> > On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >> Document properties common to several display panels in a central
> >> location that can be referenced by the panel device tree bindings.
> > 
> > Looks good. Just one comment...
> > 
> > [...]
> > 
> >> +Connectivity
> >> +------------
> >> +
> >> +- ports: Panels receive video data through one or multiple connections.
> >> While
> >> +  the nature of those connections is specific to the panel type, the
> >> +  connectivity is expressed in a standard fashion using ports as
> >> specified in
> >> +  the device graph bindings defined in
> >> +  Documentation/devicetree/bindings/graph.txt.
> > 
> > We allow panels to either use graph binding or be a child of the display
> > controller.
> 
> I knew that some display controllers use a phandle to the panel (see the
> fsl,panel and nvidia,panel properties), but I didn't know we had panels as
> children of display controller nodes. I don't think we should allow that for
> anything but DSI panels, as the DT hierarchy is based on control buses. Are
> you sure we have other panels instantiated through that mechanism ?

Ping ?

Please note that this file documents properties common to multiple panel DT 
bindings, but in no way makes it mandatory to use the OF graph bindings for 
panels. The decision is left to individual bindings.

> > Using the graph is preferred, but in the simple cases just a child node is
> > sufficient. This should be described here or somewhere in this doc.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH v7 3/8] drm: sun8i: add HDMI video support to A83T and H3
From: Jean-Francois Moine @ 2016-11-29  8:39 UTC (permalink / raw)
  To: Dave Airlie, Maxime Ripard, Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <cover.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>

Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
---
 drivers/gpu/drm/sun8i/Kconfig       |   7 +
 drivers/gpu/drm/sun8i/Makefile      |   2 +
 drivers/gpu/drm/sun8i/de2_hdmi.c    | 440 +++++++++++++++++++
 drivers/gpu/drm/sun8i/de2_hdmi.h    |  51 +++
 drivers/gpu/drm/sun8i/de2_hdmi_io.c | 843 ++++++++++++++++++++++++++++++++++++
 5 files changed, 1343 insertions(+)
 create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.c
 create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.h
 create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi_io.c

diff --git a/drivers/gpu/drm/sun8i/Kconfig b/drivers/gpu/drm/sun8i/Kconfig
index 6940895..5c4607b 100644
--- a/drivers/gpu/drm/sun8i/Kconfig
+++ b/drivers/gpu/drm/sun8i/Kconfig
@@ -17,3 +17,10 @@ config DRM_SUN8I_DE2
 	  Choose this option if your Allwinner chipset has the DE2 interface
 	  as the A64, A83T and H3. If M is selected the module will be called
 	  sun8i-de2-drm.
+
+config DRM_SUN8I_DE2_HDMI
+	tristate "Support for DE2 HDMI"
+	depends on DRM_SUN8I_DE2
+	help
+	  Choose this option if you use want HDMI on DE2.
+	  If M is selected the module will be called sun8i-de2-hdmi.
diff --git a/drivers/gpu/drm/sun8i/Makefile b/drivers/gpu/drm/sun8i/Makefile
index f107919..6ba97c2 100644
--- a/drivers/gpu/drm/sun8i/Makefile
+++ b/drivers/gpu/drm/sun8i/Makefile
@@ -3,5 +3,7 @@
 #
 
 sun8i-de2-drm-objs := de2_drv.o de2_crtc.o de2_plane.o
+sun8i-de2-hdmi-objs := de2_hdmi.o de2_hdmi_io.o
 
 obj-$(CONFIG_DRM_SUN8I_DE2) += sun8i-de2-drm.o
+obj-$(CONFIG_DRM_SUN8I_DE2_HDMI) += sun8i-de2-hdmi.o
diff --git a/drivers/gpu/drm/sun8i/de2_hdmi.c b/drivers/gpu/drm/sun8i/de2_hdmi.c
new file mode 100644
index 0000000..9ff6132
--- /dev/null
+++ b/drivers/gpu/drm/sun8i/de2_hdmi.c
@@ -0,0 +1,440 @@
+/*
+ * Allwinner DRM driver - HDMI
+ *
+ * Copyright (C) 2016 Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
+ *
+ * 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/module.h>
+#include <linux/component.h>
+#include <linux/clk.h>
+#include <linux/hdmi.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_of.h>
+
+#include "de2_hdmi.h"
+
+static const struct of_device_id de2_hdmi_dt_ids[] = {
+	{ .compatible = "allwinner,sun8i-a83t-hdmi",
+					.data = (void *) SOC_A83T },
+	{ .compatible = "allwinner,sun8i-h3-hdmi",
+					.data = (void *) SOC_H3 },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, de2_hdmi_dt_ids);
+
+#define conn_to_priv(x) \
+	container_of(x, struct de2_hdmi_priv, connector)
+
+#define enc_to_priv(x) \
+	container_of(x, struct de2_hdmi_priv, encoder)
+
+/* --- encoder functions --- */
+
+static int de2_hdmi_set_clock(struct de2_hdmi_priv *priv,
+				int rate)
+{
+	struct clk *parent_clk;
+	u32 parent_rate;
+	int ret;
+
+	/* determine and set the best rate for the parent clock (pll-video) */
+	if ((270000 * 2) % rate == 0)
+		parent_rate = 270000000;
+	else if (297000 % rate == 0)
+		parent_rate = 297000000;
+	else
+		return -EINVAL;			/* unsupported clock */
+
+	parent_clk = clk_get_parent(priv->clk);
+
+	ret = clk_set_rate(parent_clk, parent_rate);
+	if (ret) {
+		dev_err(priv->dev, "set parent rate failed %d\n", ret);
+		return ret;
+	}
+	ret = clk_set_rate(priv->clk, rate * 1000);
+	if (ret)
+		dev_err(priv->dev, "set rate failed %d\n", ret);
+
+	return ret;
+}
+
+static void de2_hdmi_encoder_mode_set(struct drm_encoder *encoder,
+				      struct drm_display_mode *mode,
+				      struct drm_display_mode *adjusted_mode)
+{
+	struct de2_hdmi_priv *priv = enc_to_priv(encoder);
+
+	priv->cea_mode = drm_match_cea_mode(mode);
+
+	DRM_DEBUG_DRIVER("cea_mode %d\n", priv->cea_mode);
+
+	if (de2_hdmi_set_clock(priv, mode->clock) < 0)
+		return;
+
+	mutex_lock(&priv->mutex);
+	hdmi_io_mode_set(priv, mode);
+	mutex_unlock(&priv->mutex);
+}
+
+static void de2_hdmi_encoder_enable(struct drm_encoder *encoder)
+{
+	struct de2_hdmi_priv *priv = enc_to_priv(encoder);
+
+	mutex_lock(&priv->mutex);
+	hdmi_io_video_on(priv);
+	mutex_unlock(&priv->mutex);
+}
+
+static void de2_hdmi_encoder_disable(struct drm_encoder *encoder)
+{
+	struct de2_hdmi_priv *priv = enc_to_priv(encoder);
+
+	mutex_lock(&priv->mutex);
+	hdmi_io_video_off(priv);
+	mutex_unlock(&priv->mutex);
+}
+
+static const struct drm_encoder_helper_funcs de2_hdmi_encoder_helper_funcs = {
+	.mode_set = de2_hdmi_encoder_mode_set,
+	.enable = de2_hdmi_encoder_enable,
+	.disable = de2_hdmi_encoder_disable,
+};
+
+static const struct drm_encoder_funcs de2_hdmi_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+/* --- connector functions --- */
+
+static int de2_hdmi_connector_mode_valid(struct drm_connector *connector,
+					struct drm_display_mode *mode)
+{
+	int cea_mode = drm_match_cea_mode(mode);
+
+	return hdmi_io_mode_valid(cea_mode) < 0 ? MODE_NOMODE : MODE_OK;
+}
+
+static enum drm_connector_status de2_hdmi_connector_detect(
+				struct drm_connector *connector, bool force)
+{
+	struct de2_hdmi_priv *priv = conn_to_priv(connector);
+	int ret;
+
+	mutex_lock(&priv->mutex);
+	ret = hdmi_io_get_hpd(priv);
+	mutex_unlock(&priv->mutex);
+
+	return ret ? connector_status_connected :
+			connector_status_disconnected;
+}
+
+static int read_edid_block(void *data, u8 *buf,
+			   unsigned int blk, size_t length)
+{
+	struct de2_hdmi_priv *priv = data;
+	int ret;
+
+	mutex_lock(&priv->mutex);
+	ret = hdmi_io_ddc_read(priv, blk / 2, (blk & 1) ? 128 : 0,
+				length, buf);
+	mutex_unlock(&priv->mutex);
+
+	return ret;
+}
+
+/* values duplicated from edid_cea_modes[] */
+static const struct drm_display_mode lmodes_tb[] = {
+	/* 2 - 720x480@60Hz */
+	{ DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 27000, 720, 736,
+		   798, 858, 0, 480, 489, 495, 525, 0,
+		   DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
+	  .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, },
+	/* 4 - 1280x720@60Hz */
+	{ DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390,
+		   1430, 1650, 0, 720, 725, 730, 750, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+	/* 16 - 1920x1080@60Hz */
+	{ DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2008,
+		   2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+	  .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+};
+
+static int de2_hdmi_connector_get_modes(struct drm_connector *connector)
+{
+	struct de2_hdmi_priv *priv = conn_to_priv(connector);
+	struct drm_display_mode *mode;
+	const struct drm_display_mode *lmode;
+	struct edid *edid;
+	int n;
+
+	edid = drm_do_get_edid(connector, read_edid_block, priv);
+
+	if (!edid) {
+		dev_warn(priv->dev, "failed to read EDID\n");
+		if (!connector->cmdline_mode.specified)
+			return 0;
+
+		if (connector->cmdline_mode.xres == 1920 &&
+		    connector->cmdline_mode.yres == 1080)
+			lmode = &lmodes_tb[2];
+		else if (connector->cmdline_mode.xres == 1280 &&
+			 connector->cmdline_mode.yres == 720)
+			lmode = &lmodes_tb[1];
+		else
+			lmode = &lmodes_tb[0];
+
+		mode = drm_mode_duplicate(connector->dev, lmode);
+		if (!mode)
+			return 0;
+		drm_mode_probed_add(connector, mode);
+
+		return 1;
+	}
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	n = drm_add_edid_modes(connector, edid);
+
+	drm_edid_to_eld(connector, edid);
+
+	kfree(edid);
+
+	DRM_DEBUG_DRIVER("%s EDID ok %d modes\n",
+		connector->eld[0] ? "HDMI" : "DVI", n);
+
+	return n;
+}
+
+static const
+struct drm_connector_helper_funcs de2_hdmi_connector_helper_funcs = {
+	.get_modes = de2_hdmi_connector_get_modes,
+	.mode_valid = de2_hdmi_connector_mode_valid,
+};
+
+static const struct drm_connector_funcs de2_hdmi_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = de2_hdmi_connector_detect,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static void de2_hdmi_cleanup(struct de2_hdmi_priv *priv)
+{
+	clk_disable_unprepare(priv->clk_ddc);
+	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->gate);
+	reset_control_assert(priv->reset1);
+	reset_control_assert(priv->reset0);
+}
+
+static int de2_hdmi_bind(struct device *dev, struct device *master, void *data)
+{
+	struct drm_device *drm = data;
+	struct de2_hdmi_priv *priv = dev_get_drvdata(dev);
+	struct drm_encoder *encoder = &priv->encoder;
+	struct drm_connector *connector = &priv->connector;
+	int ret;
+
+	encoder->possible_crtcs =
+			drm_of_find_possible_crtcs(drm, dev->of_node);
+
+	/* if no CRTC, delay */
+	if (encoder->possible_crtcs == 0)
+		return -EPROBE_DEFER;
+
+	/* HDMI init */
+	ret = reset_control_deassert(priv->reset0);
+	if (ret)
+		goto err;
+	ret = reset_control_deassert(priv->reset1);
+	if (ret)
+		goto err;
+
+	ret = clk_prepare_enable(priv->clk_ddc);
+	if (ret)
+		goto err;
+
+	de2_hdmi_set_clock(priv, 147500);	/* set a valid clock rate */
+	ret = clk_prepare_enable(priv->gate);
+	if (ret)
+		goto err;
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		goto err;
+
+	mutex_lock(&priv->mutex);
+	hdmi_io_init(priv);
+	mutex_unlock(&priv->mutex);
+
+	/* encoder init */
+	ret = drm_encoder_init(drm, encoder, &de2_hdmi_encoder_funcs,
+			       DRM_MODE_ENCODER_TMDS, NULL);
+	if (ret)
+		goto err;
+
+	drm_encoder_helper_add(encoder, &de2_hdmi_encoder_helper_funcs);
+
+	/* connector init */
+	ret = drm_connector_init(drm, connector,
+				 &de2_hdmi_connector_funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret)
+		goto err_connector;
+
+	connector->interlace_allowed = 1;
+	connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+				 DRM_CONNECTOR_POLL_DISCONNECT;
+	drm_connector_helper_add(connector,
+				 &de2_hdmi_connector_helper_funcs);
+
+	drm_mode_connector_attach_encoder(connector, encoder);
+
+	return 0;
+
+err_connector:
+	drm_encoder_cleanup(encoder);
+err:
+	dev_err(dev, "err %d\n", ret);
+	return ret;
+}
+
+static void de2_hdmi_unbind(struct device *dev, struct device *master,
+			   void *data)
+{
+	struct de2_hdmi_priv *priv = dev_get_drvdata(dev);
+
+	if (priv->connector.dev)
+		drm_connector_cleanup(&priv->connector);
+	drm_encoder_cleanup(&priv->encoder);
+	de2_hdmi_cleanup(priv);
+}
+
+static const struct component_ops de2_hdmi_ops = {
+	.bind = de2_hdmi_bind,
+	.unbind = de2_hdmi_unbind,
+};
+
+static int de2_hdmi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct de2_hdmi_priv *priv;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, priv);
+	priv->dev = dev;
+
+	mutex_init(&priv->mutex);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "failed to get memory resource\n");
+		return -ENXIO;
+	}
+	priv->mmio = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->mmio)) {
+		ret = PTR_ERR(priv->mmio);
+		dev_err(dev, "failed to map registers err %d\n", ret);
+		return ret;
+	}
+
+	priv->gate = devm_clk_get(dev, "bus");
+	if (IS_ERR(priv->gate)) {
+		ret = PTR_ERR(priv->gate);
+		dev_err(dev, "gate clock err %d\n", ret);
+		return ret;
+	}
+
+	priv->clk = devm_clk_get(dev, "clock");
+	if (IS_ERR(priv->clk)) {
+		ret = PTR_ERR(priv->clk);
+		dev_err(dev, "hdmi clock err %d\n", ret);
+		return ret;
+	}
+
+	priv->clk_ddc = devm_clk_get(dev, "ddc-clock");
+	if (IS_ERR(priv->clk_ddc)) {
+		ret = PTR_ERR(priv->clk_ddc);
+		dev_err(dev, "hdmi-ddc clock err %d\n", ret);
+		return ret;
+	}
+
+	priv->reset0 = devm_reset_control_get(dev, "hdmi0");
+	if (IS_ERR(priv->reset0)) {
+		ret = PTR_ERR(priv->reset0);
+		dev_err(dev, "reset controller err %d\n", ret);
+		return ret;
+	}
+
+	priv->reset1 = devm_reset_control_get(dev, "hdmi1");
+	if (IS_ERR(priv->reset1)) {
+		ret = PTR_ERR(priv->reset1);
+		dev_err(dev, "reset controller err %d\n", ret);
+		return ret;
+	}
+
+	priv->soc_type = (int) of_match_device(de2_hdmi_dt_ids,
+						&pdev->dev)->data;
+
+	de2_hdmi_audio_register(dev);
+
+	return component_add(dev, &de2_hdmi_ops);
+}
+
+static int de2_hdmi_remove(struct platform_device *pdev)
+{
+	de2_hdmi_audio_unregister(&pdev->dev);
+	component_del(&pdev->dev, &de2_hdmi_ops);
+
+	return 0;
+}
+
+static struct platform_driver de2_hdmi_driver = {
+	.probe = de2_hdmi_probe,
+	.remove = de2_hdmi_remove,
+	.driver = {
+		.name = "sun8i-de2-hdmi",
+		.of_match_table = of_match_ptr(de2_hdmi_dt_ids),
+	},
+};
+
+/* create the video HDMI driver and the sound card driver */
+static int __init de2_hdmi_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&de2_hdmi_driver);
+
+	return ret;
+}
+
+static void __exit de2_hdmi_fini(void)
+{
+	platform_driver_unregister(&de2_hdmi_driver);
+}
+
+module_init(de2_hdmi_init);
+module_exit(de2_hdmi_fini);
+
+MODULE_AUTHOR("Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>");
+MODULE_DESCRIPTION("Allwinner DE2 HDMI encoder/connector");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/sun8i/de2_hdmi.h b/drivers/gpu/drm/sun8i/de2_hdmi.h
new file mode 100644
index 0000000..6711a76
--- /dev/null
+++ b/drivers/gpu/drm/sun8i/de2_hdmi.h
@@ -0,0 +1,51 @@
+#ifndef __DE2_HDMI_H__
+#define __DE2_HDMI_H__
+/*
+ * Copyright (C) 2016 Jean-François Moine
+ *
+ * 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/platform_device.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <drm/drmP.h>
+
+/* SoC types */
+#define SOC_A83T 0
+#define SOC_H3 1
+
+struct de2_hdmi_priv {
+	struct device *dev;
+	void __iomem *mmio;
+
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+
+	struct clk *clk;
+	struct clk *clk_ddc;
+	struct clk *gate;
+	struct reset_control *reset0;
+	struct reset_control *reset1;
+
+	struct mutex mutex;
+	u8 soc_type;
+	u8 cea_mode;
+};
+
+/* in de2_hdmi_io.c */
+void hdmi_io_init(struct de2_hdmi_priv *priv);
+void hdmi_io_video_on(struct de2_hdmi_priv *priv);
+void hdmi_io_video_off(struct de2_hdmi_priv *priv);
+void hdmi_io_mode_set(struct de2_hdmi_priv *priv,
+			struct drm_display_mode *mode);
+int hdmi_io_ddc_read(struct de2_hdmi_priv *priv,
+			char pointer, char offset,
+			int nbyte, char *pbuf);
+int hdmi_io_get_hpd(struct de2_hdmi_priv *priv);
+int hdmi_io_mode_valid(int cea_mode);
+
+#endif /* __DE2_HDMI_H__ */
diff --git a/drivers/gpu/drm/sun8i/de2_hdmi_io.c b/drivers/gpu/drm/sun8i/de2_hdmi_io.c
new file mode 100644
index 0000000..b746a52
--- /dev/null
+++ b/drivers/gpu/drm/sun8i/de2_hdmi_io.c
@@ -0,0 +1,843 @@
+/*
+ * Allwinner A83T and H3 HDMI lowlevel functions
+ *
+ * Copyright (C) 2016 Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
+ * Adapted from the sun8iw6 and sun8iw7 disp2 drivers
+ *	Copyright (c) 2016 Allwinnertech Co., Ltd.
+ *
+ * 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.
+ */
+
+/*
+ * The HDMI controller in the A83T and H3 seems to be a
+ * Synopsys DesignWare HDMI controller.
+ * The PHYs are unknown.
+ * Documentation:
+ *	https://linux-sunxi.org/DWC_HDMI_Controller
+ *	https://www.synopsys.com/dw/doc.php/ds/c/dwc_hdmi_tx_csds.pdf
+ */
+
+#include <linux/hdmi.h>
+
+#include "de2_hdmi.h"
+
+static int hdmi_mode = 1;
+MODULE_PARM_DESC(de2_hdmi_mode, "Force HDMI mode.\n"
+"When set, if the display device is connected by HDMI, switch to this mode.\n"
+"When unset, stay in DVI mode (useful when screen overscan).\n");
+module_param_named(de2_hdmi_mode, hdmi_mode, int, 0644);
+
+/* guessed PHY registers */
+#define HDMI_PHY_LOCK_READ_REG	0x10010
+#define HDMI_PHY_CTRL_REG	0x10020
+#define HDMI_PHY_24_REG		0x10024
+#define HDMI_PHY_28_REG		0x10028
+#define HDMI_PHY_PLL_REG	0x1002c
+#define HDMI_PHY_CLK_REG	0x10030
+#define HDMI_PHY_34_REG		0x10034
+#define HDMI_PHY_STATUS_REG	0x10038
+
+/* DW registers (obfuscated addresses) */
+
+/* Interrupt Registers */
+#define R_0100_HDMI_IH_FC_STAT0 0x0010
+#define R_0101_HDMI_IH_FC_STAT1 0x0011
+#define R_0102_HDMI_IH_FC_STAT2 0x8010
+#define R_0103_HDMI_IH_AS_STAT0 0x8011
+#define R_0104_HDMI_IH_PHY_STAT0 0x0012
+#define R_0105_HDMI_IH_I2CM_STAT0 0x0013
+#define R_0106_HDMI_IH_CEC_STAT0 0x8012
+#define R_0107_HDMI_IH_VP_STAT0 0x8013
+#define R_0108_HDMI_IH_I2CMPHY_STAT0 0x4010
+#define R_01ff_HDMI_IH_MUTE 0xf01f
+
+/* Video Sample Registers */
+#define R_0200_HDMI_TX_INVID0 0x0800
+#define R_0201_HDMI_TX_INSTUFFING 0x0801
+#define R_0202_HDMI_TX_GYDATA0 0x8800
+#define R_0203_HDMI_TX_GYDATA1 0x8801
+#define R_0204_HDMI_TX_RCRDATA0 0x0802
+#define R_0205_HDMI_TX_RCRDATA1 0x0803
+#define R_0206_HDMI_TX_BCBDATA0 0x8802
+#define R_0207_HDMI_TX_BCBDATA1 0x8803
+
+/* Video Packetizer Registers */
+#define R_0801_HDMI_VP_PR_CD 0x0401
+#define R_0802_HDMI_VP_STUFF 0x8400
+#define R_0803_HDMI_VP_REMAP 0x8401
+#define R_0804_HDMI_VP_CONF 0x0402
+#define R_0807_HDMI_VP_MASK 0x8403
+
+/* Frame Composer Registers */
+#define R_1000_HDMI_FC_INVIDCONF 0x0040
+#define		HDMI_FC_INVIDCONF_DE_IN_POLARITY_ACTIVE_HIGH 0x10
+#define		HDMI_FC_INVIDCONF_HDMI_MODE 0x08
+#define R_1001_HDMI_FC_INHACTV0 0x0041
+#define R_1002_HDMI_FC_INHACTV1 0x8040
+#define R_1003_HDMI_FC_INHBLANK0 0x8041
+#define R_1004_HDMI_FC_INHBLANK1 0x0042
+#define R_1005_HDMI_FC_INVACTV0 0x0043
+#define R_1006_HDMI_FC_INVACTV1 0x8042
+#define R_1007_HDMI_FC_INVBLANK 0x8043
+#define R_1008_HDMI_FC_HSYNCINDELAY0 0x4040
+#define R_1009_HDMI_FC_HSYNCINDELAY1 0x4041
+#define R_100a_HDMI_FC_HSYNCINWIDTH0 0xc040
+#define R_100b_HDMI_FC_HSYNCINWIDTH1 0xc041
+#define R_100c_HDMI_FC_VSYNCINDELAY 0x4042
+#define R_100d_HDMI_FC_VSYNCINWIDTH 0x4043
+#define R_1011_HDMI_FC_CTRLDUR 0x0045
+#define R_1012_HDMI_FC_EXCTRLDUR 0x8044
+#define R_1013_HDMI_FC_EXCTRLSPAC 0x8045
+#define R_1014_HDMI_FC_CH0PREAM 0x0046
+#define R_1015_HDMI_FC_CH1PREAM 0x0047
+#define R_1016_HDMI_FC_CH2PREAM 0x8046
+#define R_1018_HDMI_FC_GCP 0x4044
+#define R_1019_HDMI_FC_AVICONF0 0x4045
+#define		HDMI_FC_AVICONF0_SCAN_INFO_UNDERSCAN 0x20
+#define R_101a_HDMI_FC_AVICONF1 0xc044
+#define R_101b_HDMI_FC_AVICONF2 0xc045
+#define R_101c_HDMI_FC_AVIVID 0x4046
+#define R_1025_HDMI_FC_AUDICONF0 0x2043
+#define R_1026_HDMI_FC_AUDICONF1 0xa042
+#define R_1027_HDMI_FC_AUDICONF2 0xa043
+#define R_1028_HDMI_FC_AUDICONF3 0x6040
+#define R_1029_HDMI_FC_VSDIEEEID0 0x6041
+#define R_1030_HDMI_FC_VSDIEEEID1 0x2044
+#define R_1031_HDMI_FC_VSDIEEEID2 0x2045
+#define R_1032_HDMI_FC_VSDPAYLOAD0 0xa044
+#define R_1033_HDMI_FC_VSDPAYLOAD1 0xa045
+#define R_1034_HDMI_FC_VSDPAYLOAD2 0x2046
+#define R_1063_HDMI_FC_AUDSCONF 0xa049
+#define R_1065_HDMI_FC_AUDSV 0x204b
+#define R_1066_HDMI_FC_AUDSU 0xa04a
+#define R_1067_HDMI_FC_AUDSCHNLS0 0xa04b
+#define		HDMI_FC_AUDSCHNLS0_CGMSA 0x30
+#define R_1068_HDMI_FC_AUDSCHNLS1 0x6048
+#define R_1069_HDMI_FC_AUDSCHNLS2 0x6049
+#define R_106a_HDMI_FC_AUDSCHNLS3 0xe048
+#define		HDMI_FC_AUDSCHNLS3_OIEC_CH0(v) (v)
+#define		HDMI_FC_AUDSCHNLS3_OIEC_CH1(v) (v << 4)
+#define R_106b_HDMI_FC_AUDSCHNLS4 0xe049
+#define		HDMI_FC_AUDSCHNLS4_OIEC_CH2(v) (v)
+#define		HDMI_FC_AUDSCHNLS4_OIEC_CH3(v) (v << 4)
+#define R_106c_HDMI_FC_AUDSCHNLS5 0x604a
+#define		HDMI_FC_AUDSCHNLS5_OIEC_CH0(v) (v)
+#define		HDMI_FC_AUDSCHNLS5_OIEC_CH1(v) (v << 4)
+#define R_106d_HDMI_FC_AUDSCHNLS6 0x604b
+#define		HDMI_FC_AUDSCHNLS6_OIEC_CH2(v) (v)
+#define		HDMI_FC_AUDSCHNLS6_OIEC_CH3(v) (v << 4)
+#define R_106e_HDMI_FC_AUDSCHNLS7 0xe04a
+#define R_106f_HDMI_FC_AUDSCHNLS8 0xe04b
+#define		HDMI_FC_AUDSCHNLS8_WORDLENGTH(v) (v)
+#define R_10b3_HDMI_FC_DATAUTO0 0xb045
+#define R_10b4_HDMI_FC_DATAUTO1 0x3046
+#define R_10b5_HDMI_FC_DATAUTO2 0x3047
+#define R_10d2_HDMI_FC_MASK0 0x904c
+#define R_10d6_HDMI_FC_MASK1 0x904e
+#define R_10da_HDMI_FC_MASK2 0xd04c
+#define R_10e0_HDMI_FC_PRCONF 0x3048
+#define R_1103_HDMI_FC_GMD_CONF 0x8051
+#define R_1104_HDMI_FC_GMD_HB 0x0052
+#define R_1200_HDMI_FC_DBGFORCE 0x0840
+#define		HDMI_FC_DBGFORCE_FORCEAUDIO BIT(4)
+#define		HDMI_FC_DBGFORCE_FORCEVIDEO BIT(0)
+#define R_1219_HDMI_FC_DBGTMDS0 0x4845
+
+/* HDMI Source PHY Registers */
+#define R_3000_HDMI_PHY_CONF0 0x0240
+#define		HDMI_PHY_CONF0_PDZ BIT(7)
+#define		HDMI_PHY_CONF0_ENTMDS BIT(6)
+#define		HDMI_PHY_CONF0_SPARECTRL BIT(5)
+#define		HDMI_PHY_CONF0_GEN2_PDDQ BIT(4)
+#define		HDMI_PHY_CONF0_GEN2_TXPWRON BIT(3)
+#define		HDMI_PHY_CONF0_GEN2_ENHPDRXSENSE BIT(2)
+#define		HDMI_PHY_CONF0_SELDATAENPOL BIT(1)
+#define		HDMI_PHY_CONF0_SELDIPIF BIT(0)
+#define R_3001_HDMI_PHY_TST0 0x0241
+#define		HDMI_PHY_TST0_TSTCLR BIT(5)
+#define R_3005_HDMI_PHY_INT0 0x0243
+#define R_3006_HDMI_PHY_MASK0 0x8242
+
+/* HDMI Master PHY Registers */
+#define R_3020_HDMI_PHY_I2CM_SLAVE_ADDR 0x2240
+#define		HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2 0x69
+#define R_3021_HDMI_PHY_I2CM_ADDRESS_ADDR 0x2241
+#define R_3022_HDMI_PHY_I2CM_DATAO_1_ADDR 0xa240
+#define R_3023_HDMI_PHY_I2CM_DATAO_0_ADDR 0xa241
+#define R_3026_HDMI_PHY_I2CM_OPERATION_ADDR 0xa242
+#define		HDMI_PHY_I2CM_OPERATION_ADDR_WRITE 0x10
+#define R_3027_HDMI_PHY_I2CM_INT_ADDR 0xa243
+#define R_3028_HDMI_PHY_I2CM_CTLINT_ADDR 0x6240
+
+/* Audio Sampler Registers */
+#define R_3100_HDMI_AUD_CONF0 0x0250
+#define		HDMI_AUD_CONF0_SW_RESET 0x80
+#define		HDMI_AUD_CONF0_I2S_ALL_ENABLE 0x2f
+#define R_3101_HDMI_AUD_CONF1 0x0251
+#define R_3102_HDMI_AUD_INT 0x8250
+#define R_3103_HDMI_AUD_CONF2 0x8251
+#define R_3200_HDMI_AUD_N1 0x0a40
+#define R_3201_HDMI_AUD_N2 0x0a41
+#define R_3202_HDMI_AUD_N3 0x8a40
+#define R_3205_HDMI_AUD_CTS3 0x0a43
+#define R_3206_HDMI_AUD_INPUTCLKFS 0x8a42
+#define		HDMI_AUD_INPUTCLKFS_64FS 0x04
+#define R_3302_HDMI_AUD_SPDIFINT 0x8a50
+
+/* Generic Parallel Audio Interface Registers */
+#define R_3506_HDMI_GP_POL 0x8272
+
+/* Main Controller Registers */
+#define R_4001_HDMI_MC_CLKDIS 0x0081
+#define		HDMI_MC_CLKDIS_HDCPCLK_DISABLE BIT(6)
+#define		HDMI_MC_CLKDIS_AUDCLK_DISABLE BIT(3)
+#define		HDMI_MC_CLKDIS_TMDSCLK_DISABLE BIT(1)
+#define R_4002_HDMI_MC_SWRSTZ 0x8080
+#define R_4004_HDMI_MC_FLOWCTRL 0x0082
+#define R_4005_HDMI_MC_PHYRSTZ 0x0083
+#define		HDMI_MC_PHYRSTZ_DEASSERT BIT(0)
+
+/* HDCP Encryption Engine Registers */
+#define R_5000_HDMI_A_HDCPCFG0 0x00c0
+#define R_5001_HDMI_A_HDCPCFG1 0x00c1
+#define		HDMI_A_HDCPCFG1_PH2UPSHFTENC BIT(2)
+#define		HDMI_A_HDCPCFG1_ENCRYPTIONDISABLE BIT(1)
+#define		HDMI_A_HDCPCFG1_SWRESET BIT(0)
+#define R_5008_HDMI_A_APIINTMSK 0x40c0
+#define R_5009_HDMI_A_VIDPOLCFG 0x40c1
+#define		HDMI_A_VIDPOLCFG_DATAENPOL_ACTIVE_HIGH BIT(4)
+
+/* CEC Engine Registers */
+#define R_7d02_HDMI_CEC_MASK 0x86f0
+
+/* I2C Master Registers (E-DDC) */
+#define R_7e00_HDMI_I2CM_SLAVE 0x0ee0
+#define R_7e01_HDMI_I2CM_ADDRESS 0x0ee1
+#define R_7e03_HDMI_I2CM_DATAI 0x8ee1
+#define R_7e04_HDMI_I2CM_OPERATION 0x0ee2
+#define		HDMI_I2CM_OPERATION_DDC_READ 0x02
+#define R_7e05_HDMI_I2CM_INT 0x0ee3
+#define R_7e06_HDMI_I2CM_CTLINT 0x8ee2
+#define R_7e07_HDMI_I2CM_DIV 0x8ee3
+#define R_7e08_HDMI_I2CM_SEGADDR 0x4ee0
+#define R_7e09_HDMI_I2CM_SOFTRSTZ 0x4ee1
+#define R_7e0a_HDMI_I2CM_SEGPTR 0xcee0
+#define R_7e0c_HDMI_I2CM_SS_SCL_HCNT_0_ADDR 0x4ee2
+#define R_7e0e_HDMI_I2CM_SS_SCL_LCNT_0_ADDR 0xcee2
+
+#define VIC_720x480_60		2
+#define VIC_1280x720_60		4
+#define VIC_1920x1080i_60	5
+#define VIC_720x480i_60		6
+#define VIC_1920x1080_60	16
+#define VIC_720x576_50		17
+#define VIC_1280x720_50		19
+#define VIC_1920x1080i_50	20
+#define VIC_720x576i_50		21
+#define VIC_1920x1080_50	31
+#define VIC_1920x1080_24	32
+#define VIC_1920x1080_25	33
+#define VIC_1920x1080_30	34
+
+static inline u8 hdmi_readb(struct de2_hdmi_priv *priv, u32 addr)
+{
+	return readb_relaxed(priv->mmio + addr);
+}
+
+static inline u32 hdmi_readl(struct de2_hdmi_priv *priv, u32 addr)
+{
+	return readl_relaxed(priv->mmio + addr);
+}
+
+static inline void hdmi_writeb(struct de2_hdmi_priv *priv, u32 addr, u8 data)
+{
+	writeb_relaxed(data, priv->mmio + addr);
+}
+
+static inline void hdmi_writel(struct de2_hdmi_priv *priv, u32 addr, u32 data)
+{
+	writel_relaxed(data, priv->mmio + addr);
+}
+
+static inline void hdmi_orb(struct de2_hdmi_priv *priv, u32 addr, u8 data)
+{
+	writeb_relaxed(readb_relaxed(priv->mmio + addr) | data,
+			priv->mmio + addr);
+}
+
+static inline void hdmi_orl(struct de2_hdmi_priv *priv, u32 addr, u32 data)
+{
+	writel_relaxed(readl_relaxed(priv->mmio + addr) | data,
+			priv->mmio + addr);
+}
+
+static inline void hdmi_andl(struct de2_hdmi_priv *priv, u32 addr, u32 data)
+{
+	writel_relaxed(readl_relaxed(priv->mmio + addr) & data,
+			priv->mmio + addr);
+}
+
+/* read on/off functions */
+static inline void hdmi_read_on(struct de2_hdmi_priv *priv)
+{
+	hdmi_writel(priv, HDMI_PHY_LOCK_READ_REG, 0x54524545);
+}
+static inline void hdmi_read_off(struct de2_hdmi_priv *priv)
+{
+	hdmi_writel(priv, HDMI_PHY_LOCK_READ_REG, 0x57415452);
+}
+
+static void hdmi_inner_init(struct de2_hdmi_priv *priv)
+{
+	u8 clkdis = priv->soc_type == SOC_H3 ?
+				~HDMI_MC_CLKDIS_TMDSCLK_DISABLE : 0xff;
+
+	hdmi_read_on(priv);
+
+	/* software reset */
+	hdmi_writeb(priv, R_4002_HDMI_MC_SWRSTZ,  0x00);
+	udelay(2);
+
+	/* mask all interrupts */
+	hdmi_writeb(priv, R_01ff_HDMI_IH_MUTE, 0x00);
+	hdmi_writeb(priv, R_0807_HDMI_VP_MASK, 0xff);
+	hdmi_writeb(priv, R_10d2_HDMI_FC_MASK0, 0xff);
+	hdmi_writeb(priv, R_10d6_HDMI_FC_MASK1, 0xff);
+	hdmi_writeb(priv, R_10da_HDMI_FC_MASK2, 0xff);
+	hdmi_writeb(priv, R_3102_HDMI_AUD_INT, 0xff);
+	hdmi_writeb(priv, R_3302_HDMI_AUD_SPDIFINT, 0xff);
+	hdmi_writeb(priv, R_3506_HDMI_GP_POL, 0xff);
+	hdmi_writeb(priv, R_5008_HDMI_A_APIINTMSK, 0xff);
+	hdmi_writeb(priv, R_7d02_HDMI_CEC_MASK, 0xff);
+	hdmi_writeb(priv, R_7e05_HDMI_I2CM_INT, 0xff);
+	hdmi_writeb(priv, R_7e06_HDMI_I2CM_CTLINT, 0xff);
+
+	hdmi_writeb(priv, R_1063_HDMI_FC_AUDSCONF, 0xf0);
+	hdmi_writeb(priv, R_10b3_HDMI_FC_DATAUTO0, 0x1e);
+	hdmi_writeb(priv, R_5001_HDMI_A_HDCPCFG1, 0x00);
+	hdmi_writeb(priv, R_5001_HDMI_A_HDCPCFG1,
+				HDMI_A_HDCPCFG1_ENCRYPTIONDISABLE |
+				HDMI_A_HDCPCFG1_SWRESET);
+	hdmi_writeb(priv, R_5000_HDMI_A_HDCPCFG0, 0x00);
+	hdmi_writeb(priv, R_5009_HDMI_A_VIDPOLCFG,
+				HDMI_A_VIDPOLCFG_DATAENPOL_ACTIVE_HIGH);
+	hdmi_writeb(priv, R_4001_HDMI_MC_CLKDIS, clkdis);
+	hdmi_writeb(priv, R_4001_HDMI_MC_CLKDIS, 0x00);
+	hdmi_writeb(priv, R_4001_HDMI_MC_CLKDIS, clkdis);
+	hdmi_writeb(priv, R_0100_HDMI_IH_FC_STAT0, 0xff);
+	hdmi_writeb(priv, R_0101_HDMI_IH_FC_STAT1, 0xff);
+	hdmi_writeb(priv, R_0102_HDMI_IH_FC_STAT2, 0xff);
+	hdmi_writeb(priv, R_0103_HDMI_IH_AS_STAT0, 0xff);
+	hdmi_writeb(priv, R_0105_HDMI_IH_I2CM_STAT0, 0xff);
+	hdmi_writeb(priv, R_0106_HDMI_IH_CEC_STAT0, 0xff);
+	hdmi_writeb(priv, R_0107_HDMI_IH_VP_STAT0, 0xff);
+}
+
+static void hdmi_phy_init_a83t(struct de2_hdmi_priv *priv)
+{
+	hdmi_inner_init(priv);
+
+	hdmi_writeb(priv, 0x10000, 0x01);
+	hdmi_writeb(priv, 0x10001, 0x00);
+	hdmi_writeb(priv, 0x10002, HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2);
+	hdmi_writeb(priv, 0x10003, 0x00);
+	hdmi_writeb(priv, 0x10007, 0xa0);
+	hdmi_writeb(priv, R_4005_HDMI_MC_PHYRSTZ,
+					HDMI_MC_PHYRSTZ_DEASSERT);
+	udelay(1);
+	hdmi_writeb(priv, R_3000_HDMI_PHY_CONF0,
+					HDMI_PHY_CONF0_GEN2_ENHPDRXSENSE |
+					HDMI_PHY_CONF0_SELDATAENPOL);
+	hdmi_writeb(priv, R_3000_HDMI_PHY_CONF0,
+					HDMI_PHY_CONF0_GEN2_PDDQ |
+					HDMI_PHY_CONF0_GEN2_ENHPDRXSENSE |
+					HDMI_PHY_CONF0_SELDATAENPOL);
+	hdmi_writeb(priv, R_3000_HDMI_PHY_CONF0,
+					HDMI_PHY_CONF0_GEN2_PDDQ |
+					HDMI_PHY_CONF0_SELDATAENPOL);
+	hdmi_writeb(priv, R_3006_HDMI_PHY_MASK0, 0xf0);
+	hdmi_writeb(priv, R_3027_HDMI_PHY_I2CM_INT_ADDR, 0xff);
+	hdmi_writeb(priv, R_3028_HDMI_PHY_I2CM_CTLINT_ADDR, 0xff);
+	hdmi_writeb(priv, R_0104_HDMI_IH_PHY_STAT0, 0xff);
+	hdmi_writeb(priv, R_0108_HDMI_IH_I2CMPHY_STAT0, 0xff);
+	hdmi_writeb(priv, R_4005_HDMI_MC_PHYRSTZ, 0x00);
+	hdmi_writeb(priv, R_3000_HDMI_PHY_CONF0,
+					HDMI_PHY_CONF0_GEN2_PDDQ |
+					HDMI_PHY_CONF0_GEN2_ENHPDRXSENSE |
+					HDMI_PHY_CONF0_SELDATAENPOL);
+	hdmi_writeb(priv, R_3000_HDMI_PHY_CONF0,
+					HDMI_PHY_CONF0_GEN2_ENHPDRXSENSE |
+					HDMI_PHY_CONF0_SELDATAENPOL);
+	hdmi_writeb(priv, R_3001_HDMI_PHY_TST0, HDMI_PHY_TST0_TSTCLR);
+	hdmi_writeb(priv, R_3020_HDMI_PHY_I2CM_SLAVE_ADDR,
+					HDMI_PHY_I2CM_SLAVE_ADDR_PHY_GEN2);
+	hdmi_writeb(priv, R_3001_HDMI_PHY_TST0, 0x00);
+}
+
+static void hdmi_phy_init_h3(struct de2_hdmi_priv *priv)
+{
+	int to_cnt;
+	u32 tmp;
+
+	hdmi_writel(priv, HDMI_PHY_CTRL_REG, 0);
+	hdmi_writel(priv, HDMI_PHY_CTRL_REG, 1 << 0);
+	udelay(5);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 16);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 1);
+	udelay(10);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 2);
+	udelay(5);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 3);
+	usleep_range(40, 50);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 19);
+	usleep_range(100, 120);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 18);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 7 << 4);
+
+	to_cnt = 10;
+	while (1) {
+		if (hdmi_readl(priv, HDMI_PHY_STATUS_REG) & 0x80)
+			break;
+		usleep_range(200, 250);
+		if (--to_cnt == 0) {
+			dev_err(priv->dev, "hdmi phy init timeout\n");
+			break;
+		}
+	}
+
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 0xf << 8);
+	hdmi_orl(priv, HDMI_PHY_CTRL_REG, 1 << 7);
+
+	hdmi_writel(priv, HDMI_PHY_PLL_REG, 0x39dc5040);
+	hdmi_writel(priv, HDMI_PHY_CLK_REG, 0x80084343);
+	msleep(20);
+	hdmi_writel(priv, HDMI_PHY_34_REG, 0x00000001);
+	hdmi_orl(priv, HDMI_PHY_PLL_REG, 0x02000000);
+	msleep(100);
+	tmp = hdmi_readl(priv, HDMI_PHY_STATUS_REG);
+	hdmi_orl(priv, HDMI_PHY_PLL_REG, 0xc0000000);
+	hdmi_orl(priv, HDMI_PHY_PLL_REG, (tmp >> 11) & 0x3f);
+	hdmi_writel(priv, HDMI_PHY_CTRL_REG, 0x01ff0f7f);
+	hdmi_writel(priv, HDMI_PHY_24_REG, 0x80639000);
+	hdmi_writel(priv, HDMI_PHY_28_REG, 0x0f81c405);
+
+	hdmi_inner_init(priv);
+}
+
+static int get_divider(int rate)
+{
+	if (rate <= 27000)
+		return 11;
+	if (rate <= 74250)
+		return 4;
+	if (rate <= 148500)
+		return 2;
+	return 1;
+}
+
+static void hdmi_i2cm_write(struct de2_hdmi_priv *priv,
+			    int addr, u8 valh, u8 vall)
+{
+	hdmi_writeb(priv, R_3021_HDMI_PHY_I2CM_ADDRESS_ADDR, addr);
+	hdmi_writeb(priv, R_3022_HDMI_PHY_I2CM_DATAO_1_ADDR, valh);
+	hdmi_writeb(priv, R_3023_HDMI_PHY_I2CM_DATAO_0_ADDR, vall);
+	hdmi_writeb(priv, R_3026_HDMI_PHY_I2CM_OPERATION_ADDR,
+					HDMI_PHY_I2CM_OPERATION_ADDR_WRITE);
+	usleep_range(2000, 2500);
+}
+
+static void hdmi_phy_set_a83t(struct de2_hdmi_priv *priv,
+				struct drm_display_mode *mode)
+{
+	switch (get_divider(mode->clock)) {
+	case 1:
+		hdmi_i2cm_write(priv, 0x06, 0x00, 0x00);
+		hdmi_i2cm_write(priv, 0x15, 0x00, 0x0f);
+		hdmi_i2cm_write(priv, 0x10, 0x00, 0x00);
+		hdmi_i2cm_write(priv, 0x19, 0x00, 0x02);
+		hdmi_i2cm_write(priv, 0x0e, 0x00, 0x00);
+		hdmi_i2cm_write(priv, 0x09, 0x80, 0x2b);
+		break;
+	case 2:				/* 1080P @ 60 & 50 */
+		hdmi_i2cm_write(priv, 0x06, 0x04, 0xa0);
+		hdmi_i2cm_write(priv, 0x15, 0x00, 0x0a);
+		hdmi_i2cm_write(priv, 0x10, 0x00, 0x00);
+		hdmi_i2cm_write(priv, 0x19, 0x00, 0x02);
+		hdmi_i2cm_write(priv, 0x0e, 0x00, 0x21);
+		hdmi_i2cm_write(priv, 0x09, 0x80, 0x29);
+		break;
+	case 4:				/* 720P @ 50 & 60, 1080I, 1080 */
+		hdmi_i2cm_write(priv, 0x06, 0x05, 0x40);
+		hdmi_i2cm_write(priv, 0x15, 0x00, 0x05);
+		hdmi_i2cm_write(priv, 0x10, 0x00, 0x00);
+		hdmi_i2cm_write(priv, 0x19, 0x00, 0x07);
+		hdmi_i2cm_write(priv, 0x0e, 0x02, 0xb5);
+		hdmi_i2cm_write(priv, 0x09, 0x80, 0x09);
+		break;
+/*	case 11:			* 480P/576P */
+	default:
+		hdmi_i2cm_write(priv, 0x06, 0x01,
+			mode->flags & DRM_MODE_FLAG_DBLCLK ? 0xe3 : 0xe0);
+		hdmi_i2cm_write(priv, 0x15, 0x00, 0x00);
+		hdmi_i2cm_write(priv, 0x10, 0x08, 0xda);
+		hdmi_i2cm_write(priv, 0x19, 0x00, 0x07);
+		hdmi_i2cm_write(priv, 0x0e, 0x03, 0x18);
+		hdmi_i2cm_write(priv, 0x09, 0x80, 0x09);
+		break;
+	}
+	hdmi_i2cm_write(priv, 0x1e, 0x00, 0x00);
+	hdmi_i2cm_write(priv, 0x13, 0x00, 0x00);
+	hdmi_i2cm_write(priv, 0x17, 0x00, 0x00);
+	hdmi_writeb(priv, R_3000_HDMI_PHY_CONF0,
+				HDMI_PHY_CONF0_GEN2_TXPWRON |
+				HDMI_PHY_CONF0_GEN2_ENHPDRXSENSE |
+				HDMI_PHY_CONF0_SELDATAENPOL);
+}
+
+static void hdmi_phy_set_h3(struct de2_hdmi_priv *priv,
+			struct drm_display_mode *mode)
+{
+	u32 tmp;
+
+	hdmi_andl(priv, HDMI_PHY_CTRL_REG, ~0xf000);
+
+	switch (get_divider(mode->clock)) {
+	case 1:
+		hdmi_writel(priv, HDMI_PHY_PLL_REG, 0x31dc5fc0);
+						/* or 0x30dc5fc0 ? */
+		hdmi_writel(priv, HDMI_PHY_CLK_REG, 0x800863c0);
+		msleep(20);
+		hdmi_writel(priv, HDMI_PHY_34_REG, 0x00000001);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0x02000000);
+		msleep(200);
+		tmp = (hdmi_readl(priv, HDMI_PHY_STATUS_REG) >> 11) & 0x3f;
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0xc0000000);
+		if (tmp < 0x3d)
+			tmp += 2;
+		else
+			tmp = 0x3f;
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, tmp);
+		msleep(100);
+		hdmi_writel(priv, HDMI_PHY_CTRL_REG, 0x01ffff7f);
+		hdmi_writel(priv, HDMI_PHY_24_REG, 0x8063b000);
+		hdmi_writel(priv, HDMI_PHY_28_REG, 0x0f8246b5);
+		break;
+	case 2:				/* 1080P @ 60 & 50 */
+		hdmi_writel(priv, HDMI_PHY_PLL_REG, 0x39dc5040);
+		hdmi_writel(priv, HDMI_PHY_CLK_REG, 0x80084381);
+		msleep(20);
+		hdmi_writel(priv, HDMI_PHY_34_REG, 0x00000001);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0x02000000);
+		msleep(100);
+		tmp = (hdmi_readl(priv, HDMI_PHY_STATUS_REG) >> 11) & 0x3f;
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0xc0000000);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, tmp);
+		hdmi_writel(priv, HDMI_PHY_CTRL_REG, 0x01ffff7f);
+		hdmi_writel(priv, HDMI_PHY_24_REG, 0x8063a800);
+		hdmi_writel(priv, HDMI_PHY_28_REG, 0x0f81c485);
+		break;
+	case 4:				/* 720P @ 50 & 60, 1080I, 1080 */
+		hdmi_writel(priv, HDMI_PHY_PLL_REG, 0x39dc5040);
+		hdmi_writel(priv, HDMI_PHY_CLK_REG, 0x80084343);
+		msleep(20);
+		hdmi_writel(priv, HDMI_PHY_34_REG, 0x00000001);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0x02000000);
+		msleep(100);
+		tmp = (hdmi_readl(priv, HDMI_PHY_STATUS_REG) >> 11) & 0x3f;
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0xc0000000);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, tmp);
+		hdmi_writel(priv, HDMI_PHY_CTRL_REG, 0x01ffff7f);
+		hdmi_writel(priv, HDMI_PHY_24_REG, 0x8063b000);
+		hdmi_writel(priv, HDMI_PHY_28_REG, 0x0f81c405);
+		break;
+	default:
+/*	case 11:				* 480P/576P */
+		hdmi_writel(priv, HDMI_PHY_PLL_REG, 0x39dc5040);
+		hdmi_writel(priv, HDMI_PHY_CLK_REG, 0x8008430a);
+		msleep(20);
+		hdmi_writel(priv, HDMI_PHY_34_REG, 0x00000001);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0x02000000);
+		msleep(100);
+		tmp = (hdmi_readl(priv, HDMI_PHY_STATUS_REG) >> 11) & 0x3f;
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, 0xc0000000);
+		hdmi_orl(priv, HDMI_PHY_PLL_REG, tmp);
+		hdmi_writel(priv, HDMI_PHY_CTRL_REG, 0x01ffff7f);
+		hdmi_writel(priv, HDMI_PHY_24_REG, 0x8063b000);
+		hdmi_writel(priv, HDMI_PHY_28_REG, 0x0f81c405);
+		break;
+	}
+}
+
+/* HDMI functions */
+
+/* hardware init */
+void hdmi_io_init(struct de2_hdmi_priv *priv)
+{
+	if (priv->soc_type == SOC_H3)
+		hdmi_phy_init_h3(priv);
+	else
+		hdmi_phy_init_a83t(priv);
+
+	/* disable hdcp */
+	hdmi_writeb(priv, R_5001_HDMI_A_HDCPCFG1,
+					HDMI_A_HDCPCFG1_PH2UPSHFTENC);
+	hdmi_writeb(priv, R_4001_HDMI_MC_CLKDIS,
+					HDMI_MC_CLKDIS_HDCPCLK_DISABLE);
+}
+
+/* check if the resolution is supported */
+int hdmi_io_mode_valid(int cea_mode)
+{
+	switch (cea_mode) {
+	case VIC_720x480_60:
+	case VIC_1280x720_60:
+	case VIC_1920x1080i_60:
+	case VIC_720x480i_60:
+	case VIC_1920x1080_60:
+	case VIC_720x576_50:
+	case VIC_1280x720_50:
+	case VIC_1920x1080i_50:
+	case VIC_720x576i_50:
+	case VIC_1920x1080_50:
+	case VIC_1920x1080_24:
+	case VIC_1920x1080_25:
+	case VIC_1920x1080_30:
+		return 1;
+	}
+	return -1;
+}
+
+/* output init */
+void hdmi_io_mode_set(struct de2_hdmi_priv *priv,
+			struct drm_display_mode *mode)
+{
+	int avi_d2;			/* AVI InfoFrame Data Byte 2 */
+	int h_blank, h_sync_w, h_front_p;
+	int invidconf;
+
+	/* colorimetry and aspect ratio */
+	switch (priv->cea_mode) {
+	case 0:
+		return;			/* bad mode */
+	case VIC_720x480_60:
+	case VIC_720x480i_60:
+	case VIC_720x576_50:
+	case VIC_720x576i_50:
+		avi_d2 = (HDMI_COLORIMETRY_ITU_601 << 6) |
+			(HDMI_PICTURE_ASPECT_4_3 << 4) | 0x08;
+		break;
+	default:
+		avi_d2 = (HDMI_COLORIMETRY_ITU_709 << 6) |
+			(HDMI_PICTURE_ASPECT_16_9 << 4) | 0x08;
+		break;
+	}
+
+	h_blank = mode->htotal - mode->hdisplay;
+	h_sync_w = mode->hsync_end - mode->hsync_start;
+	h_front_p = mode->hsync_start - mode->hdisplay;
+
+	invidconf = 0;
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+		invidconf |= 0x01;
+	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+		invidconf |= 0x20;
+	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+		invidconf |= 0x40;
+
+	if (priv->soc_type == SOC_H3) {
+		hdmi_phy_set_h3(priv, mode);
+		hdmi_inner_init(priv);
+	} else {
+		hdmi_phy_init_a83t(priv);
+	}
+
+	hdmi_writeb(priv, R_1200_HDMI_FC_DBGFORCE,
+					HDMI_FC_DBGFORCE_FORCEVIDEO);
+	hdmi_writeb(priv, R_1219_HDMI_FC_DBGTMDS0, 0x00);
+	hdmi_writeb(priv, R_1000_HDMI_FC_INVIDCONF,
+				invidconf |
+				HDMI_FC_INVIDCONF_DE_IN_POLARITY_ACTIVE_HIGH);
+	hdmi_writeb(priv, 0x10001, invidconf < 0x60 ? 0x03 : 0x00);
+	hdmi_writeb(priv, R_1002_HDMI_FC_INHACTV1, mode->hdisplay >> 8);
+	hdmi_writeb(priv, R_100d_HDMI_FC_VSYNCINWIDTH,
+			mode->vsync_end - mode->vsync_start);
+	hdmi_writeb(priv, R_1006_HDMI_FC_INVACTV1, mode->vdisplay >> 8);
+	hdmi_writeb(priv, R_1004_HDMI_FC_INHBLANK1, h_blank >> 8);
+	hdmi_writeb(priv, R_100c_HDMI_FC_VSYNCINDELAY,
+			mode->vsync_start - mode->vdisplay);
+	hdmi_writeb(priv, R_1009_HDMI_FC_HSYNCINDELAY1, h_front_p >> 8);
+	hdmi_writeb(priv, R_100b_HDMI_FC_HSYNCINWIDTH1, h_sync_w >> 8);
+	hdmi_writeb(priv, R_1001_HDMI_FC_INHACTV0, mode->hdisplay);
+	hdmi_writeb(priv, R_1003_HDMI_FC_INHBLANK0, h_blank);
+	hdmi_writeb(priv, R_1008_HDMI_FC_HSYNCINDELAY0, h_front_p);
+	hdmi_writeb(priv, R_100a_HDMI_FC_HSYNCINWIDTH0, h_sync_w);
+	hdmi_writeb(priv, R_1005_HDMI_FC_INVACTV0, mode->vdisplay);
+	hdmi_writeb(priv, R_1007_HDMI_FC_INVBLANK,
+			mode->vtotal - mode->vdisplay);
+	hdmi_writeb(priv, R_1011_HDMI_FC_CTRLDUR, 12);
+	hdmi_writeb(priv, R_1012_HDMI_FC_EXCTRLDUR, 32);
+	hdmi_writeb(priv, R_1013_HDMI_FC_EXCTRLSPAC, 1);
+	hdmi_writeb(priv, R_1014_HDMI_FC_CH0PREAM, 0x0b);
+	hdmi_writeb(priv, R_1015_HDMI_FC_CH1PREAM, 0x16);
+	hdmi_writeb(priv, R_1016_HDMI_FC_CH2PREAM, 0x21);
+	hdmi_writeb(priv, R_10e0_HDMI_FC_PRCONF,
+			mode->flags & DRM_MODE_FLAG_DBLCLK ? 0x21 : 0x10);
+	hdmi_writeb(priv, R_0801_HDMI_VP_PR_CD,
+			mode->flags & DRM_MODE_FLAG_DBLCLK ? 0x41 : 0x40);
+	hdmi_writeb(priv, R_0802_HDMI_VP_STUFF, 0x07);
+	hdmi_writeb(priv, R_0803_HDMI_VP_REMAP, 0x00);
+	hdmi_writeb(priv, R_0804_HDMI_VP_CONF, 0x47);
+	hdmi_writeb(priv, R_0200_HDMI_TX_INVID0, 0x01);
+	hdmi_writeb(priv, R_0201_HDMI_TX_INSTUFFING, 0x07);
+	hdmi_writeb(priv, R_0202_HDMI_TX_GYDATA0, 0x00);
+	hdmi_writeb(priv, R_0203_HDMI_TX_GYDATA1, 0x00);
+	hdmi_writeb(priv, R_0204_HDMI_TX_RCRDATA0, 0x00);
+	hdmi_writeb(priv, R_0205_HDMI_TX_RCRDATA1, 0x00);
+	hdmi_writeb(priv, R_0206_HDMI_TX_BCBDATA0, 0x00);
+	hdmi_writeb(priv, R_0207_HDMI_TX_BCBDATA1, 0x00);
+
+	if (priv->connector.eld[0]) {		/* if audio/HDMI */
+		hdmi_writeb(priv, R_10b3_HDMI_FC_DATAUTO0, 0x08);
+		hdmi_writeb(priv, R_1031_HDMI_FC_VSDIEEEID2, 0x00);
+		hdmi_writeb(priv, R_1030_HDMI_FC_VSDIEEEID1,
+						HDMI_IEEE_OUI >> 8);
+		hdmi_writeb(priv, R_1029_HDMI_FC_VSDIEEEID0,
+						HDMI_IEEE_OUI & 0xff);
+		hdmi_writeb(priv, R_1032_HDMI_FC_VSDPAYLOAD0, 0x00);
+		hdmi_writeb(priv, R_1033_HDMI_FC_VSDPAYLOAD1, 0x00);
+		hdmi_writeb(priv, R_1034_HDMI_FC_VSDPAYLOAD2, 0x00);
+		hdmi_writeb(priv, R_10b4_HDMI_FC_DATAUTO1, 0x01);
+		hdmi_writeb(priv, R_10b5_HDMI_FC_DATAUTO2, 0x11);
+		hdmi_writeb(priv, R_1018_HDMI_FC_GCP, 0x00);
+		hdmi_writeb(priv, R_1104_HDMI_FC_GMD_HB, 0x00);
+		hdmi_writeb(priv, R_1103_HDMI_FC_GMD_CONF, 0x11);
+
+		/* switch to HDMI mode */
+		if (hdmi_mode) {
+			hdmi_read_on(priv);
+			hdmi_orb(priv, R_1000_HDMI_FC_INVIDCONF,
+					HDMI_FC_INVIDCONF_HDMI_MODE);
+			hdmi_read_off(priv);
+		}
+
+		/* AVI */
+		hdmi_writeb(priv, R_1019_HDMI_FC_AVICONF0,
+					HDMI_FC_AVICONF0_SCAN_INFO_UNDERSCAN);
+		hdmi_writeb(priv, R_101a_HDMI_FC_AVICONF1, avi_d2);
+		hdmi_writeb(priv, R_101b_HDMI_FC_AVICONF2, 0x08);
+		hdmi_writeb(priv, R_101c_HDMI_FC_AVIVID, priv->cea_mode);
+	}
+
+	hdmi_writeb(priv, R_4004_HDMI_MC_FLOWCTRL, 0x00);
+	hdmi_writeb(priv, R_4001_HDMI_MC_CLKDIS, 0x00);	/* enable all clocks */
+
+	if (priv->soc_type != SOC_H3)
+		hdmi_phy_set_a83t(priv, mode);
+
+	hdmi_writeb(priv, R_1200_HDMI_FC_DBGFORCE, 0x00);
+
+}
+
+void hdmi_io_video_on(struct de2_hdmi_priv *priv)
+{
+	if (!priv->cea_mode)
+		return;
+pr_info("*jfm* hdmi video on\n");
+	if (priv->soc_type == SOC_H3)
+		hdmi_orl(priv, HDMI_PHY_CTRL_REG, 0x0f << 12);
+}
+
+void hdmi_io_video_off(struct de2_hdmi_priv *priv)
+{
+	if (!priv->cea_mode)
+		return;
+pr_info("*jfm* hdmi video off\n");
+	if (priv->soc_type == SOC_H3)
+		hdmi_andl(priv, HDMI_PHY_CTRL_REG, ~(0x0f << 12));
+}
+
+/* get a block of EDID */
+int hdmi_io_ddc_read(struct de2_hdmi_priv *priv,
+			char pointer, char off,
+			int nbyte, char *pbuf)
+{
+	unsigned int to_cnt;
+	u8 reg;
+	int ret = 0;
+
+	hdmi_read_on(priv);
+	hdmi_writeb(priv, R_7e09_HDMI_I2CM_SOFTRSTZ, 0x00);
+	to_cnt = 50;
+	while (!(hdmi_readb(priv, R_7e09_HDMI_I2CM_SOFTRSTZ) & 0x01)) {
+		udelay(10);
+		if (--to_cnt == 0) {	/* wait for 500us for timeout */
+			dev_err(priv->dev, "hdmi ddc reset timeout\n");
+			break;
+		}
+	}
+
+	hdmi_writeb(priv, R_7e07_HDMI_I2CM_DIV, 0x05);
+	hdmi_writeb(priv, R_7e05_HDMI_I2CM_INT, 0x08);
+	hdmi_writeb(priv, R_7e0c_HDMI_I2CM_SS_SCL_HCNT_0_ADDR, 0xd8);
+	hdmi_writeb(priv, R_7e0e_HDMI_I2CM_SS_SCL_LCNT_0_ADDR, 0xfe);
+
+	while (nbyte > 0) {
+		hdmi_writeb(priv, R_7e00_HDMI_I2CM_SLAVE, 0xa0 >> 1);
+		hdmi_writeb(priv, R_7e01_HDMI_I2CM_ADDRESS, off);
+		hdmi_writeb(priv, R_7e08_HDMI_I2CM_SEGADDR, 0x60 >> 1);
+		hdmi_writeb(priv, R_7e0a_HDMI_I2CM_SEGPTR, pointer);
+		hdmi_writeb(priv, R_7e04_HDMI_I2CM_OPERATION,
+					HDMI_I2CM_OPERATION_DDC_READ);
+
+		to_cnt = 200;				/* timeout 100ms */
+		while (1) {
+			reg = hdmi_readb(priv, R_0105_HDMI_IH_I2CM_STAT0);
+			hdmi_writeb(priv, R_0105_HDMI_IH_I2CM_STAT0, reg);
+			if (reg & 0x02) {
+				*pbuf++ = hdmi_readb(priv,
+						R_7e03_HDMI_I2CM_DATAI);
+				break;
+			}
+			if (reg & 0x01) {
+				dev_err(priv->dev, "hdmi ddc read error\n");
+				ret = -1;
+				break;
+			}
+			if (--to_cnt == 0) {
+				if (!ret) {
+					dev_err(priv->dev,
+						"hdmi ddc read timeout\n");
+					ret = -1;
+				}
+				break;
+			}
+			usleep_range(500, 800);
+		}
+		if (ret)
+			break;
+		nbyte--;
+		off++;
+	}
+	hdmi_read_off(priv);
+
+	return ret;
+}
+
+int hdmi_io_get_hpd(struct de2_hdmi_priv *priv)
+{
+	int ret;
+
+	hdmi_read_on(priv);
+
+	if (priv->soc_type == SOC_H3)
+		ret = hdmi_readl(priv, HDMI_PHY_STATUS_REG) & 0x80000;
+	else
+		ret = hdmi_readb(priv, R_3005_HDMI_PHY_INT0) & 0x02;
+
+	hdmi_read_off(priv);
+
+	return ret != 0;
+}
-- 
2.10.2

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply related

* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
From: Ulf Hansson @ 2016-11-29  8:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lina Iyer, Kevin Hilman, Rafael J. Wysocki,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Andy Gross, Stephen Boyd,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161110195832.4nz7lxlmshaemcbb@rob-hp-laptop>

On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > Re-using idle state definition provided by arm,idle-state for domain
>> > idle states creates a lot of confusion and limits further evolution of
>> > the domain idle definition. To keep things clear and simple, define a
>> > idle states for domain using a new compatible "domain-idle-state".
>> >
>> > Fix existing PM domains code to look for the newly defined compatible.
>> >
>> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>> >  drivers/base/power/domain.c                        |  2 +-
>> >  3 files changed, 38 insertions(+), 5 deletions(-)
>> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > new file mode 100644
>> > index 0000000..eefc7ed
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > @@ -0,0 +1,33 @@
>> > +PM Domain Idle State Node:
>> > +
>> > +A domain idle state node represents the state parameters that will be used to
>> > +select the state when there are no active components in the domain.
>> > +
>> > +The state node has the following parameters -
>> > +
>> > +- compatible:
>> > +       Usage: Required
>> > +       Value type: <string>
>> > +       Definition: Must be "domain-idle-state".
>> > +
>> > +- entry-latency-us
>> > +       Usage: Required
>> > +       Value type: <prop-encoded-array>
>> > +       Definition: u32 value representing worst case latency in
>> > +                   microseconds required to enter the idle state.
>> > +                   The exit-latency-us duration may be guaranteed
>> > +                   only after entry-latency-us has passed.
>>
>> As we anyway are going to change this, why not use an u64 and have the
>> value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.

Rob, are you fine with this? I thought it would be great to get this
in for 4.10 rc1.

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

^ permalink raw reply

* Re: [PATCH v2 1/7] MFD: add bindings for stm32 general purpose timer driver
From: Benjamin Gaignard @ 2016-11-29  8:48 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lee Jones, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
	alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, Thierry Reding,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, knaack.h-Mmb7MZpHnFY,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
	Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <b78a21f7-38a1-5a40-b96e-d1c9156aee68-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

2016-11-27 16:41 GMT+01:00 Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On 27/11/16 14:10, Jonathan Cameron wrote:
>> On 24/11/16 15:14, Benjamin Gaignard wrote:
>>> Add bindings information for stm32 general purpose timer
>>>
>>> version 2:
>>> - rename stm32-mfd-timer to stm32-gptimer
>>> - only keep one compatible string
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
>>> ---
>>>  .../bindings/mfd/stm32-general-purpose-timer.txt   | 43 ++++++++++++++++++++++
>>>  1 file changed, 43 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>> new file mode 100644
>>> index 0000000..2f10e67
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>> @@ -0,0 +1,43 @@
>>> +STM32 general purpose timer driver
>>> +
>>> +Required parameters:
>>> +- compatible: must be "st,stm32-gptimer"
>>> +
>>> +- reg:                      Physical base address and length of the controller's
>>> +                    registers.
>>> +- clock-names:              Set to "clk_int".
>>> +- clocks:           Phandle to the clock used by the timer module.
>>> +                    For Clk properties, please refer to ../clock/clock-bindings.txt
>>> +
>>> +Optional parameters:
>>> +- resets:           Phandle to the parent reset controller.
>>> +                    See ..reset/st,stm32-rcc.txt
>>> +
>>> +Optional subnodes:
>>> +- pwm:                      See ../pwm/pwm-stm32.txt
>>> +- iiotimer:         See ../iio/timer/stm32-iio-timer.txt
>> Naming issue here.  Can't mention IIO as that's a linux subsystem and all
>> bindings must be independent of OS.
>>
>> Perhaps adc-trigger-timer?
>>> +
>>> +Example:
>>> +    gptimer1: gptimer1@40010000 {
>>> +            compatible = "st,stm32-gptimer";
>>> +            reg = <0x40010000 0x400>;
>>> +            clocks = <&rcc 0 160>;
>>> +            clock-names = "clk_int";
>>> +
>>> +            pwm1@0 {
>>> +                    compatible = "st,stm32-pwm";
>>> +                    st,pwm-num-chan = <4>;
>>> +                    st,breakinput;
>>> +                    st,complementary;
>>> +            };
>>> +
>>> +            iiotimer1@0 {
>>> +                    compatible = "st,stm32-iio-timer";
>> Again, avoid the use of iio in here (same issue you had with mfd in the previous
>> version).
>>> +                    interrupts = <27>;
>>> +                    st,input-triggers-names = TIM5_TRGO,
>> Docs for these should be introduced before they are used in an example.
>> Same for the PWM ones above.  Expand the detail fo the example as you add
>> the other elements.
>
> I've just dived into the datasheet for these timers.
> http://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf
>

I really appreciate that you do this effort, thanks.

>
> I think you need a binding that describes the capabilities of each of the timers
> explicitly.   Down to the level of whether there is a repetition counter or not.
> Each should exists as a separate entity in device tree.
>
> They then have an existence as timers separate to the description of what they
> are used for.
>
> Here the only way we are saying they exist is by their use which doesn't feel
> right to me.
>
> So I think you need to move back to what you had in the first place.  The key
> thing is that ever timer needs describing fully.  They are different enough
> that for example the datasheet doesn't even try to describe them in one section.
> (it has 4 separate chapters covering different sets of these hardware blocks).
> The naming isn't really based on index, we are talking different hardware
> that the datasheet authors have decided not to give different names to!

Even if the hardware are named differently in the documentation they
all share the
same registers definitions and mapping but configurations are
different for each device.

>
> If they'd called them
> advanced timers
> generic timers
> basic timers
> really basic timers meant for driving the DAC (6 and 7)
>
> We'd all have been quite happy with different compatible strings giving away
> what they can do.

4 compatible strings will not be enough to describe devices
configuration, for example
in the documentation general purpose timers could have a 16 or 32 bit
counter, for 1 to 4
pwm channels and triggers (accepted or generated by the device) are
different for each device.

DAC could be drive by timers 2, 4, 5, 6, 7 and 8.
ADC could be driver by 32 triggers

> What you have here is far too specific to what you are trying to do with them
> right now.
>
> These things are separately capable of timing capture (which is I guess where
> the IIO device later comes in).
>
> So my expectation is that we end up potentially instantiating:
>
> 1) An MFD to handle the shared elements of the timers.
> 2) Up to 12ish timers each with separate existence as a device in the driver model
> and in device tree.
> (nasty corner cases here are using timers as perscalers for other timers - I'd be
> tempted to leave that for now)
> Note that each of these devices has a different register set I think? Any shared
> bits are handled via the mfd above (if we even need that MFD which I'm starting
> to doubt looking at the datasheet).
>

pwm and trigger share the same registers but not the same bits.
With regmap write functions I don't have sharing problems.

> 3) Up to N pwms again with there own existence in the device model.  These don't
> do much other than wrap the timer and stick it in output mode.
> 4) Up to N iio triggers - this is basically using the timer as a periodic interrupt
> (though without the interrupt having visibility to the kernel) which fires off
> sampling on associated ADCs.
> 5) Up to N iio capture devices for all channels that support timing capture.
> Note there is also hardware encoder capture support in these which should be
> correctly handled as well.  This comes back to an ancient discussion on the
> TI ecap units which have similar capabilities (driver never went anywhere but
> I think that was because the author left TI).
>
> Certainly for the IIO devices these should no be bound up into one instance
> as you have done here.
>
> Anyhow, I fear that right now this discussion is missing the key ingredient
> that the hardware is horrendously variable in it's capabilities and really
> is 4-5 different types of hardware that just happen to share a few bits of
> their offsets in their register maps.

Hardware really share the same registers mapping that why I have wrote
one only driver
per framework. Only the configurations are different

>
> So after all that I'm almost more confused than I was at the start!
>
> Jonathan
>
>
>>> +                                              TIM2_TRGO,
>>> +                                              TIM4_TRGO,
>>> +                                              TIM3_TRGO;
>>> +                    st,output-triggers-names = TIM1_TRGO;
>>> +            };
>>> +    };
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v3 05/13] drm: bridge: Add LVDS encoder DT bindings
From: Laurent Pinchart @ 2016-11-29  9:04 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Archit Taneja,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480410283-28698-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

The DT bindings support parallel to LVDS encoders that don't require any
configuration, similarly to the dumb VGA DAC DT bindings.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 .../bindings/display/bridge/lvds-transmitter.txt   | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
new file mode 100644
index 000000000000..fd39ad34c383
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -0,0 +1,64 @@
+Parallel to LVDS Encoder
+------------------------
+
+This binding supports the parallel to LVDS encoders that don't require any
+configuration.
+
+LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A. Multiple
+incompatible data link layers have been used over time to transmit image data
+to LVDS panels. This binding targets devices compatible with the following
+specifications only.
+
+[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999, February
+1999 (Version 1.0), Japan Electronic Industry Development Association (JEIDA)
+[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
+Semiconductor
+[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0), Video
+Electronics Standards Association (VESA)
+
+Those devices have been marketed under the FPD-Link and FlatLink brand names
+among others.
+
+
+Required properties:
+
+- compatible: Must be "lvds-encoder"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for parallel input
+- Video port 1 for LVDS output
+
+
+Example
+-------
+
+lvds-encoder {
+	compatible = "lvds-encoder";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			lvds_enc_in: endpoint {
+				remote-endpoint = <&display_out_rgb>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			lvds_enc_out: endpoint {
+				remote-endpoint = <&lvds_panel_in>;
+			};
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart

--
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 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Jean-Francois Moine @ 2016-11-29  9:08 UTC (permalink / raw)
  To: Dave Airlie, Maxime Ripard, Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <cover.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>

Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
---
 .../devicetree/bindings/display/sunxi/hdmi.txt     | 56 ++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt

diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
new file mode 100644
index 0000000..1e107cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
@@ -0,0 +1,56 @@
+Allwinner HDMI Transmitter
+==========================
+
+The Allwinner HDMI transmitters are included in the SoCs.
+They support audio and video.
+
+Required properties:
+ - compatible : should be one of
+		"allwinner,sun8i-a83t-hdmi"
+		"allwinner,sun8i-h3-hdmi"
+ - reg: base address and size of the I/O memory
+ - clocks : phandles to the HDMI clocks as described in
+	Documentation/devicetree/bindings/clock/clock-bindings.txt
+ - clock-names : must be
+		"bus" : bus gate
+		"clock" : streaming clock
+		"ddc-clock" : DDC clock
+ - resets : One or two phandles to the HDMI resets
+ - reset-names : when 2 phandles, must be
+		"hdmi0" and "hdmi1"
+ - #address-cells : should be <1>
+ - #size-cells : should be <0>
+
+Required nodes:
+ - port: Audio and video input port nodes with endpoint definitions
+	as defined in Documentation/devicetree/bindings/graph.txt.
+	port@0 is video and port@1 is audio.
+
+Example:
+
+	hdmi: hdmi@01ee0000 {
+		compatible = "allwinner,sun8i-a83t-hdmi";
+		reg = <0x01ee0000 0x20000>;
+		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
+			 <&ccu CLK_HDMI_DDC>;
+		clock-names = "bus", "clock", "ddc-clock";
+		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
+		reset-names = "hdmi0", "hdmi1";
+		pinctrl-names = "default";
+		pinctrl-0 = <&hdmi_pins_a>;
+		status = "disabled";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@0 {			/* video */
+			reg = <0>;
+			hdmi_tcon1: endpoint {
+				remote-endpoint = <&tcon1_hdmi>;
+			};
+		};
+		port@1 {			/* audio */
+			reg = <1>;
+			hdmi_i2s2: endpoint {
+				remote-endpoint = <&i2s2_hdmi>;
+			};
+		};
+	};
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH v2 11/13] clocksource: export the clocks_calc_mult_shift to use by timestamp code
From: Thomas Gleixner @ 2016-11-29  9:08 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
	Richard Cochran, Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok,
	John Stultz
In-Reply-To: <20161128230337.6731-12-grygorii.strashko-l0cyMroinI0@public.gmane.org>

On Mon, 28 Nov 2016, Grygorii Strashko wrote:

> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> 
> The CPSW CPTS driver is capable of doing timestamping on tx/rx packets and
> requires to know mult and shift factors for timestamp conversion from raw
> value to nanoseconds (ptp clock). Now these mult and shift factors are
> calculated manually and provided through DT, which makes very hard to
> support of a lot number of platforms, especially if CPTS refclk is not the
> same for some kind of boards and depends on efuse settings (Keystone 2
> platforms). Hence, export clocks_calc_mult_shift() to allow drivers like
> CPSW CPTS (and other ptp drivesr) to benefit from automaitc calculation of
> mult and shift factors.
> 
> Cc: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>

Acked-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>

> ---
>  kernel/time/clocksource.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 7e4fad7..150242c 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -89,6 +89,7 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 maxsec)
>  	*mult = tmp;
>  	*shift = sft;
>  }
> +EXPORT_SYMBOL_GPL(clocks_calc_mult_shift);
>  
>  /*[Clocksource internal variables]---------
>   * curr_clocksource:
> -- 
> 2.10.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

* Re: [PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Laurent Pinchart @ 2016-11-29  9:11 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-arm-msm, devicetree, Mark Brown, dri-devel
In-Reply-To: <b779c014-fc20-1fbc-dea9-cb90dbde5b71@codeaurora.org>

Hi Archit,

(CC'ing Mark Brown)

On Tuesday 29 Nov 2016 13:41:33 Archit Taneja wrote:
> On 11/29/2016 12:03 PM, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 11:37:41 Archit Taneja wrote:
> >> Add the regulator supply properties needed by ADV7511 and ADV7533.
> >> 
> >> The regulators are specified as optional properties since there can
> >> be boards which have a fixed supply directly routed to the pins, and
> >> these may not be modelled as regulator supplies.
> > 
> > That's why we have support for dummy supplies in the kernel, isn't it ?
> > Isn't it better to make the supplies mandatory in the bindings (and
> > obviously handling them as optional in the driver for
> > backward-compatibility) ?
>
> I'm a bit unclear on this.
> 
> I thought we couldn't add mandatory properties once the device is already
> present in DT for one or more platforms.

You can, as long as you treat them as optional in the driver to retain 
backward compatibility. The DT bindings should document the properties 
expected from a new platform (older versions of the bindings will always be 
available in the git history).

> Say, if we do make it mandatory for future additions, we would need to have
> DT property for the supplies for the new platforms. If the regulators on
> these boards are fixed supplies, they would be need to be modeled
> using "regulator-fixed", possibly without any input supply. Is that
> what you're suggesting?

That's the idea, yes. Clock maintainers have a similar opinion regarding the 
clock bindings, where a clock that is not optional at the hardware level 
should be specified in DT even if it's always present.

Mark, any opinion ?

> > Apart from that,
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> >> Cc: devicetree@vger.kernel.org
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> >> ---
> >> v3:
> >> - Revert back to having a common avdd-supply property for the 1.8V
> >>   supplies
> >> 
> >> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 9 ++++
> >> 1 file changed, 9 insertions(+)
> >> 
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> >> 6532a59..13d53bc 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> 
> >> @@ -56,6 +56,15 @@ Optional properties:
> >>  - adi,disable-timing-generator: Only for ADV7533. Disables the internal
> >> 
> >> timing generator. The chip will rely on the sync signals in the DSI data
> >> lanes, rather than generate its own timings for HDMI output.
> >> +- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and
> >> PVDD
> >> +  pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also
> >> powers
> >> +  up the A2VDD pin.
> >> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
> >> +  ADV7511 and V3P3 on ADV7533.
> >> +
> >> +ADV7533 specific supplies:
> >> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can
> >> be
> >> +  either 1.2V or 1.8V.
> >> 
> >>  Required nodes:

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] arm64: dts: Add symlinks for cros-ec-keyboard and cros-ec-sbs
From: Heiko Stübner @ 2016-11-29  9:16 UTC (permalink / raw)
  To: Brian Norris
  Cc: Douglas Anderson, olof-nZhT3qVonbNeoWH0uzbU5w, Arnd Bergmann,
	robh-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161128235132.GA71295-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Am Montag, 28. November 2016, 15:51:35 schrieb Brian Norris:
> Hi Olof, Arnd,
> 
> On Thu, May 12, 2016 at 03:02:10PM -0700, Doug Anderson wrote:
> > We'd like to be able to use the cros-ec-keyboard.dtsi and
> > cros-ec-sbs.dtsi snippets for arm64 devices.  Currently those files live
> > in the arm/boot/dts directory.
> > 
> > Let's follow the convention set by commit 8ee57b8182c4 ("ARM64: dts:
> > vexpress: Use a symlink to vexpress-v2m-rs1.dtsi from arch=arm") and use
> > a symlink.  Note that in this case we put the files in a new
> > "include/common" directory since these snippets may need to be
> > referenced by dts files in many different subdirectories.
> > 
> > Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > ---
> > Note that, as of right now, there are no users of this.  However, given
> > development happening it is almost 100% certain that users will arrive
> > soon.  If we need to wait for the first user before landing this we can
> > leave this on the back burner.  If it's OK to have no users (yet), let's
> > land.
> 
> This may not fit your definition of "soon", but I'm looking to start
> using these files in arch/arm64/. I'll probably carry along this patch
> and resend when I'm ready, but it'd be just as well if you'd merge it
> now (or express a preference for a different directory structure).

Till now I was expecting to just pick up this patch as well once the first 
(rk3399-)users arrived. But of course it could also be picked up separately.


> >  arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi | 1 +
> >  arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi      | 1 +
> >  2 files changed, 2 insertions(+)
> >  create mode 120000
> >  arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> >  create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> > 
> > \ No newline at end of file
> > 
> > diff --git a/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> > b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi new file mode
> > 120000
> > index 000000000000..1c1889f0a791
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> > @@ -0,0 +1 @@
> > +../../../../../arm/boot/dts/cros-ec-keyboard.dtsi
> > \ No newline at end of file
> > diff --git a/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> > b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi new file mode
> > 120000
> > index 000000000000..3d7ae9c88bcd
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> > @@ -0,0 +1 @@
> > +../../../../../arm/boot/dts/cros-ec-sbs.dtsi
> 
> FWIW:
> 
> Reviewed-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

I'm also running with this on my rk3399-gru and it sucessfully enables the 
cros-ec keyboard :-) ,so if anyone picks it up before me they can add

Reviewed-by: Heiko Stueber <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Tested-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

--
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: Question regarding clocks in the DW-HDMI DT bindings
From: Laurent Pinchart @ 2016-11-29  9:18 UTC (permalink / raw)
  To: Michael Turquette
  Cc: Andy Yan, Vladimir Zapolskiy, Fabio Estevam, DRI mailing list,
	Linux-DT, nickey.yang-TNX95d0MmH7DzftRWevZcw, Stephen Boyd
In-Reply-To: <CAEG3pNCTH5YUjO8m-jd1rfRRKRus7c39W4rYK56c6NdfQsOcyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Mike,

On Monday 28 Nov 2016 22:27:01 Michael Turquette wrote:
> On Mon, Nov 28, 2016 at 10:04 PM, Laurent Pinchart wrote:
> > On Monday 28 Nov 2016 13:56:11 Michael Turquette wrote:
> >> On Fri, Nov 25, 2016 at 7:22 AM, Laurent Pinchart wrote:
> >>> On Friday 25 Nov 2016 10:56:53 Andy Yan wrote:
> >>>> On 2016年11月25日 07:26, Laurent Pinchart wrote:
> >>>>> On Friday 25 Nov 2016 00:16:00 Vladimir Zapolskiy wrote:
> >>>>>> On 11/25/2016 12:07 AM, Fabio Estevam wrote:
> >>>>>>> On Thu, Nov 24, 2016 at 7:16 PM, Laurent Pinchart wrote:
> >>>>>>>> Hi Andy,
> >>>>>>>> 
> >>>>>>>> As the author of the DW-HDMI DT bindings this question is
> >>>>>>>> addressed to you, but information from anyone is more than welcome.
> >>>>>>>> 
> >>>>>>>> The DT bindings specify two clocks named "iahb" and "isfr" but
> >>>>>>>> don't describe them. While I assume that the "isfr" clock
> >>>>>>>> corresponds to the "isfrclk" input signal of the DW HDMI, there is
> >>>>>>>> no "iahb" clock described in the IP core datasheet.
> >>>>>>> 
> >>>>>>> i.MX6Q has a DW-HDMI IP block.
> >>>>>>> 
> >>>>>>> The names in the devicetree binding matches the ones listed at the
> >>>>>>> i.MX6Q Reference Manual - Table 33-1. HDMI Clocks
> >>>>>> 
> >>>>>> correct, for your convenience the table is copied below:
> >>>>>> 
> >>>>>> Clock name |     Clock Root     | Description
> >>>>>> -----------+--------------------+-----------------------------------
> >>>>>>   iahbclk  | ahb_clk_root       | Bus clock
> >>>>>>   icecclk  | ckil_sync_clk_root | CEC low-frequency clock (32kHZ)
> >>>>>>   ihclk    | ahb_clk_root       | Module clock
> >>>>>>   isfrclk  | video_27m_clk_root | Internal SFR clock (video clock
> >>>>>>   27MHz)
> >>>>>> 
> >>>>>> Here AHB stands for ARM Advanced High-performance Bus.
> >>>>> 
> >>>>> That's what I suspected. I believe the "iahb" name is wrong, as the
> >>>>> DW HDMI TX IP core clearly documents the bus clock as being called
> >>>>> "iapbclk". We could rename that in the DT bindings (with
> >>>>> compatibility code in the driver to keep supporting the old name) but
> >>>>> it might not be worth it. The bindings should however document that
> >>>>> the "iahb" clock is the IP core's "iapbclk" bus clock.
> >>>> 
> >>>> I got the clock name from I.MX6Q TRM, I also checked the name again
> >>>> with Rockchip IC design team now, hope to get some new information
> >>>> soon.
> >>> 
> >>> Thank you. While at it, could you ask them which version of the DW HDMI
> >>> IP used in the SoC ?
> >>> 
> >>>>> Another question I have about the bus clock (CC'ing the devicetree
> >>>>> mailing list as well as the clock maintainers) is whether it should
> >>>>> be made optional. The clock is obviously mandatory from a hardware
> >>>>> point of view (given that APB is a synchronous bus and thus requires a
> >>>>> clock), but in some SoCs (specifically for the Renesas SoCs) that
> >>>>> clock is always on and can't be controlled. We already omit bus clocks
> >>>>> in DT for most IP cores when the clock can never be controlled (and we
> >>>>> also omit a bunch of other clocks that we don't even know exist), so
> >>>>> it could make sense to make the clock optional. Otherwise there would
> >>>>> be runtime overhead trying to handle a clock that can't be controlled.
> >>>> 
> >>>> If this is the case on Renesas SOCs, we can consider make the clock as
> >>>> optional. Or move all the clock operations to platform specific
> >>>> code(dw_hdmi-rockchip.c/dw_hdmi-imx.c)?
> >>> 
> >>> I'd prefer keeping the code generic, otherwise we'd end up with
> >>> platform-specific code that would perform the same operations on most
> >>> platforms. I'll submit a patch soon to make the clock optional, we can
> >>> discuss it then.
> >> 
> >> Yes, let's keep the code generic. Absence of a "standard' clock is OK
> >> and we should accept the small overhead incurred in providing a
> >> solution that works for everyone. This prevents hardware-specific
> >> hacks in the driver.
> >> 
> >> Related: we really should model bus clocks whenever possible. I've
> >> seen other attempts to merge functional/logic and bus clocks into a
> >> single entity (e.g. a single struct clk_hw/clk_core that turns both
> >> clocks on and off) and this defeats some fine-grained power management
> >> scenarios that the hardware designers had in mind when creating
> >> separate controls for the clocks.
> > 
> > Sure, but that wasn't really the question :-) When the bus clock is
> > separately controllable then I agree it should be modelled separately in
> > DT. In my case the bus clock is always on, and I'm thus wondering whether
> > it would be better to make it optional in DT to reduce the runtime
> > overhead incurred by trying to control something that can't be
> > controlled.
> 
> I thought I answered this, but maybe not directly enough :-)
> 
> We should make the clock mandatory in DT if the physical line must be
> there. This is regardless of whether a given chip/IP variant has
> control over that clock; so long as the physical clock line always
> exists then it is not really "optional".
> 
> In the case where there is an absence of the physical clock line, then
> making it optional in DT makes sense.
> 
> As an aside, we did discuss the fact that the vast majority of clocks
> are not modeled in DT, and I'm not saying that we transcribe the RTL
> into DT. I'm just saying that if there is a debate over whether or not
> to make a clock optional in DT, when it is always physically there,
> then don't make it optional. Whether or not the control is exposed on
> a particular chip is less important.
> 
> Anyways, this is more DT ridiculousness and I won't block either
> method getting merged. I'm just picking my favorite color to paint the
> bikeshed.

Thanks for sharing your opinion. I'll keep the clock mandatory and specify it 
in DT.

> >>>>>> By the way while we're discussing DW HDMI bindings specific to iMX,
> >>>>>> I would recommend to remove utterly hackish and iMX only "gpr"
> >>>>>> property from the example in bindings/display/bridge/dw_hdmi.txt

-- 
Regards,

Laurent Pinchart

--
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 v5 8/8] iommu/rockchip: Enable Rockchip IOMMU on ARM64
From: Heiko Stübner @ 2016-11-29  9:22 UTC (permalink / raw)
  To: Brian Norris
  Cc: Shunqian Zheng, joro-zLv9SwRftAIdnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, mark.yao-TNX95d0MmH7DzftRWevZcw,
	airlied-cv59FeDIM0c, tfiga-hpIqsD4AKlfQT0dZR+AlfA,
	xxm-TNX95d0MmH7DzftRWevZcw, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomasz Figa,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161129004225.GA109697-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Am Montag, 28. November 2016, 16:42:27 schrieb Brian Norris:
> Hi,
> 
> On Fri, Jun 24, 2016 at 10:13:33AM +0800, Shunqian Zheng wrote:
> > From: Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > 
> > This patch makes it possible to compile the rockchip-iommu driver on
> > ARM64, so that it can be used with 64-bit SoCs equipped with this type
> > of IOMMU.
> > 
> > Signed-off-by: Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > Signed-off-by: Shunqian Zheng <zhengsq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > Signed-off-by: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > ---
> > 
> >  drivers/iommu/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index ad08603..5572621 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG
> > 
> >  config ROCKCHIP_IOMMU
> >  
> >  	bool "Rockchip IOMMU Support"
> > 
> > -	depends on ARM
> > +	depends on ARM || ARM64
> > 
> >  	depends on ARCH_ROCKCHIP || COMPILE_TEST
> >  	select IOMMU_API
> >  	select ARM_DMA_USE_IOMMU
> 
> Whatever happened with the rest of this series? Some of the IOMMU bits
> made it, but the DRM fixes never did, and so this didn't get applied.
> This leaves the whole DRM stack unusable on ARM64 Rockchip systems.
> 
> The patch context has changed a bit on patch 7 (and maybe 6?), so
> somebody will need to refresh those on the latest upstream.

I've tried forward-porting these when going through Caesars VOP dts patches, 
but was somewhat unsucessful in my small attempt at the time - see [0].


Heiko

[0] https://lkml.org/lkml/2016/11/14/519

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

^ permalink raw reply

* Re: [PATCH  v2 02/13] net: ethernet: ti: allow cpts to be built separately
From: Richard Cochran @ 2016-11-29  9:37 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
	linux-omap, Rob Herring, devicetree, Murali Karicheri,
	Wingman Kwok
In-Reply-To: <20161128230337.6731-3-grygorii.strashko@ti.com>

On Mon, Nov 28, 2016 at 05:03:26PM -0600, Grygorii Strashko wrote:

> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c

> +EXPORT_SYMBOL_GPL(cpts_unregister);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("TI CPTS ALE driver");

ALE?

Also, MODULE_AUTHOR (that's me) is missing.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH  v2 01/13] net: ethernet: ti: cpts: switch to readl/writel_relaxed()
From: Richard Cochran @ 2016-11-29  9:38 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
	linux-omap, Rob Herring, devicetree, Murali Karicheri,
	Wingman Kwok
In-Reply-To: <20161128230337.6731-2-grygorii.strashko@ti.com>

On Mon, Nov 28, 2016 at 05:03:25PM -0600, Grygorii Strashko wrote:
> Switch to readl/writel_relaxed() APIs, because this is recommended
> API and the CPTS IP is reused on Keystone 2 SoCs
> where LE/BE modes are supported.
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Acked-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply

* Re: [PATCH v3] iommu/ipmmu-vmsa: Add r8a7796 DT binding
From: Geert Uytterhoeven @ 2016-11-29  9:46 UTC (permalink / raw)
  To: Magnus Damm
  Cc: devicetree@vger.kernel.org, Mark Rutland, Laurent Pinchart,
	Geert Uytterhoeven, Joerg Roedel, linux-kernel@vger.kernel.org,
	Linux-Renesas, iommu, Rob Herring, Simon Horman
In-Reply-To: <20161123144054.11919.22936.sendpatchset@little-apple>

On Wed, Nov 23, 2016 at 3:40 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
>
> Update the IPMMU DT binding documentation to include the r8a7796 compat
> string for R-Car M3-W.
>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Simon Horman <horms+renesas@verge.net.au>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 6/7] IIO: add STM32 IIO timer driver
From: Benjamin Gaignard @ 2016-11-29  9:46 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lee Jones, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
	alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, Thierry Reding,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, knaack.h-Mmb7MZpHnFY,
	Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
	Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <3e2bce3d-c607-d397-487f-2439a0ba7b0b-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

2016-11-27 16:42 GMT+01:00 Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> I delved into the datasheet after trying to figure this out, so I think
> I now sort of understand your intent, but please do answer the questions
> inline.
>
> On 24/11/16 15:14, Benjamin Gaignard wrote:
>> Timers IPs can be used to generate triggers for other IPs like
>> DAC, ADC or other timers.
>> Each trigger may result of timer internals signals like counter enable,
>> reset or edge, this configuration could be done through "master_mode"
>> device attribute.
>>
>> A timer device could be triggered by other timers, we use the trigger
>> name and is_stm32_iio_timer_trigger() function to distinguish them
>> and configure IP input switch.
> The presence of an IIO device in here was a suprise.. What is it actually for?

IIO device is needed to be able to valid the input triggers, which
aren't the same than
those generated by the device.
Since I use triggers name to distinguish them I have introduced
is_stm32_iio_timer_trigger()
function to be sure that triggers are coming for a valid hardware and
not from a fake one
using the same name.

>
> I think this needs some examples of usage to make it clear what the aim is.

In the hardware block there is switch in input to select which trigger
will drive the IP.
For example that allow to start multiple pwm exactly that the same
time or to start/stop
it on master edges.

>
> I was basically expecting to see a driver instantiating one iio trigger
> per timer that can act as a trigger.  Those would each have sampling frequency
> controls and basica enable / disable.

An hardware device could have up to 5 triggers: timX_trgo, timX_ch1, timX_ch2,
timX_ch3, timX_ch4.
Until now I have try to simplify the problem and just use timX_trgo trigger.
I have added a "sampling_frequency" attribute on the trigger to
control the frequence
and I use trigger set_state function to enable disable it.

>
> I'm seeing something much more complex here so additional explanation is
> needed.
>>
>> Timer may also decide on which event (edge, level) they could
>> be activated by a trigger, this configuration is done by writing in
>> "slave_mode" device attribute.
> Really?  Sounds like magic numbers in sysfs which is never a good idea.
> Please document those attributes / or break them up into elements that
> don't require magic numbers.

I would like to use strings here, it is possible to use IIO_CONST_ATTR
to describe them ?

>>
>> Since triggers could also be used by DAC or ADC their names are defined
>> in include/dt-bindings/iio/timer/st,stm32-iio-timer.h so those IPs will be able
>> to configure themselves in valid_trigger function
>>
>> Trigger have a "sampling_frequency" attribute which allow to configure
>> timer sampling frequency without using pwm interface
>>
>> version 2:
>> - keep only one compatible
> Hmm. I'm not sure I like this as such.  We are actually dealing with lots
> of instances of a hardware block with only a small amount of shared
> infrastrcuture (which is classic mfd teritory). So to my mind we
> should have a separate device for each.

Registers mapping and offset are the same, from triggers point of view
only the configuration of the input switch is different.

>
>> - use st,input-triggers-names and st,output-triggers-names
>>   to know which triggers are accepted and/or create by the device
> I'm not following why we have this cascade setup?
>
> These are triggers, not devices in the IIO context.  We need some detailed
> description of why you have it setup like this. This would include the
> ABI with examples of how you are using it.

I had put example of usage on the cover letter, I will duplicate them
in this commit
message.

>
> Basically I don't currently understand what you are doing :(
>
>
> Thanks,
>
> Jonathan
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
>> ---
>>  drivers/iio/Kconfig                                |   2 +-
>>  drivers/iio/Makefile                               |   1 +
>>  drivers/iio/timer/Kconfig                          |  15 +
>>  drivers/iio/timer/Makefile                         |   1 +
>>  drivers/iio/timer/stm32-iio-timer.c                | 448 +++++++++++++++++++++
>>  drivers/iio/trigger/Kconfig                        |   1 -
>>  include/dt-bindings/iio/timer/st,stm32-iio-timer.h |  23 ++
>>  include/linux/iio/timer/stm32-iio-timers.h         |  16 +
>>  8 files changed, 505 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/iio/timer/Kconfig
>>  create mode 100644 drivers/iio/timer/Makefile
>>  create mode 100644 drivers/iio/timer/stm32-iio-timer.c
>>  create mode 100644 include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>  create mode 100644 include/linux/iio/timer/stm32-iio-timers.h
>>
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 6743b18..2de2a80 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig"
>>  source "drivers/iio/pressure/Kconfig"
>>  source "drivers/iio/proximity/Kconfig"
>>  source "drivers/iio/temperature/Kconfig"
>> -
>> +source "drivers/iio/timer/Kconfig"
>>  endif # IIO
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 87e4c43..b797c08 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -32,4 +32,5 @@ obj-y += potentiometer/
>>  obj-y += pressure/
>>  obj-y += proximity/
>>  obj-y += temperature/
>> +obj-y += timer/
>>  obj-y += trigger/
>> diff --git a/drivers/iio/timer/Kconfig b/drivers/iio/timer/Kconfig
>> new file mode 100644
>> index 0000000..7a73bc6
>> --- /dev/null
>> +++ b/drivers/iio/timer/Kconfig
>> @@ -0,0 +1,15 @@
>> +#
>> +# Timers drivers
>> +
>> +menu "Timers"
>> +
>> +config IIO_STM32_TIMER
>> +     tristate "stm32 iio timer"
>> +     depends on ARCH_STM32
>> +     depends on OF
>> +     select IIO_TRIGGERED_EVENT
>> +     select MFD_STM32_GP_TIMER
>> +     help
>> +       Select this option to enable stm32 timers hardware IPs
>> +
>> +endmenu
>> diff --git a/drivers/iio/timer/Makefile b/drivers/iio/timer/Makefile
>> new file mode 100644
>> index 0000000..a360c9f
>> --- /dev/null
>> +++ b/drivers/iio/timer/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_IIO_STM32_TIMER) += stm32-iio-timer.o
>> diff --git a/drivers/iio/timer/stm32-iio-timer.c b/drivers/iio/timer/stm32-iio-timer.c
>> new file mode 100644
>> index 0000000..35f2687
>> --- /dev/null
>> +++ b/drivers/iio/timer/stm32-iio-timer.c
>> @@ -0,0 +1,448 @@
>> +/*
>> + * stm32-iio-timer.c
>> + *
>> + * Copyright (C) STMicroelectronics 2016
>> + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org> for STMicroelectronics.
>> + * License terms:  GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/timer/stm32-iio-timers.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/triggered_event.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/mfd/stm32-gptimer.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define DRIVER_NAME "stm32-iio-timer"
>> +
>> +struct stm32_iio_timer_dev {
>> +     struct device *dev;
>> +     struct regmap *regmap;
>> +     struct clk *clk;
>> +     int irq;
>> +     bool own_timer;
>> +     unsigned int sampling_frequency;
>> +     struct iio_trigger *active_trigger;
>> +};
>> +
>> +static ssize_t _store_frequency(struct device *dev,
>> +                             struct device_attribute *attr,
>> +                             const char *buf, size_t len)
>> +{
>> +     struct iio_trigger *trig = to_iio_trigger(dev);
>> +     struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>> +     unsigned int freq;
>> +     int ret;
>> +
>> +     ret = kstrtouint(buf, 10, &freq);
>> +     if (ret)
>> +             return ret;
>> +
>> +     stm32->sampling_frequency = freq;
>> +
>> +     return len;
>> +}
>> +
>> +static ssize_t _read_frequency(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +     struct iio_trigger *trig = to_iio_trigger(dev);
>> +     struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>> +     unsigned long long freq = stm32->sampling_frequency;
>> +     u32 psc, arr, cr1;
>> +
>> +     regmap_read(stm32->regmap, TIM_CR1, &cr1);
>> +     regmap_read(stm32->regmap, TIM_PSC, &psc);
>> +     regmap_read(stm32->regmap, TIM_ARR, &arr);
>> +
>> +     if (psc && arr && (cr1 & TIM_CR1_CEN)) {
>> +             freq = (unsigned long long)clk_get_rate(stm32->clk);
>> +             do_div(freq, psc);
>> +             do_div(freq, arr);
>> +     }
>> +
>> +     return sprintf(buf, "%d\n", (unsigned int)freq);
>> +}
>> +
>> +static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
>> +                           _read_frequency,
>> +                           _store_frequency);
>> +
>> +static struct attribute *stm32_trigger_attrs[] = {
>> +     &iio_dev_attr_sampling_frequency.dev_attr.attr,
>> +     NULL,
>> +};
>> +
>> +static const struct attribute_group stm32_trigger_attr_group = {
>> +     .attrs = stm32_trigger_attrs,
>> +};
>> +
>> +static const struct attribute_group *stm32_trigger_attr_groups[] = {
>> +     &stm32_trigger_attr_group,
>> +     NULL,
>> +};
>> +
>> +static
>> +ssize_t _show_master_mode(struct device *dev,
>> +                       struct device_attribute *attr, char *buf)
>> +{
>> +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +     struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>> +     u32 cr2;
>> +
>> +     regmap_read(stm32->regmap, TIM_CR2, &cr2);
>> +
>> +     return snprintf(buf, PAGE_SIZE, "%d\n", (cr2 >> 4) & 0x7);
>> +}
>> +
>> +static
>> +ssize_t _store_master_mode(struct device *dev,
>> +                        struct device_attribute *attr,
>> +                        const char *buf, size_t len)
>> +{
>> +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +     struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>> +     u8 mode;
>> +     int ret;
>> +
>> +     ret = kstrtou8(buf, 10, &mode);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (mode > 0x7)
>> +             return -EINVAL;
>> +
>> +     regmap_update_bits(stm32->regmap, TIM_CR2, TIM_CR2_MMS, mode << 4);
>> +
>> +     return len;
>> +}
>> +
>> +static IIO_DEVICE_ATTR(master_mode, S_IRUGO | S_IWUSR,
>> +                    _show_master_mode,
>> +                    _store_master_mode,
>> +                    0);
>> +
>> +static
>> +ssize_t _show_slave_mode(struct device *dev,
>> +                      struct device_attribute *attr, char *buf)
>> +{
>> +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +     struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>> +     u32 smcr;
>> +
>> +     regmap_read(stm32->regmap, TIM_SMCR, &smcr);
>> +
>> +     return snprintf(buf, PAGE_SIZE, "%d\n", smcr & 0x3);
>> +}
>> +
>> +static
>> +ssize_t _store_slave_mode(struct device *dev,
>> +                       struct device_attribute *attr,
>> +                       const char *buf, size_t len)
>> +{
>> +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +     struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>> +     u8 mode;
>> +     int ret;
>> +
>> +     ret = kstrtou8(buf, 10, &mode);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (mode > 0x7)
>> +             return -EINVAL;
> How is something called slave mode going to be fed a number between 0 and 7?
> Rule of thumb is no magic numbers in sysfs and right now this is looking
> rather cryptic to say the least!

I would like to use strings here, it is possible to use IIO_CONST_ATTR
to describe them ?
In documentation slave modes are describe that this:
000: Slave mode disabled - if CEN = ‘1’ then the prescaler is clocked
directly by the internal clock.
001: Encoder mode 1 - Counter counts up/down on TI2FP1 edge depending
on TI1FP2 level.
010: Encoder mode 2 - Counter counts up/down on TI1FP2 edge depending
on TI2FP1 level.
011: Encoder mode 3 - Counter counts up/down on both TI1FP1 and TI2FP2
edges depending on the level of the other input.
100: Reset Mode - Rising edge of the selected trigger input (TRGI)
reinitializes the counter and generates an update of the registers.
101: Gated Mode - The counter clock is enabled when the trigger input
(TRGI) is high.
        The counter stops (but is not reset) as soon as the trigger becomes low.
         Both start and stop of the counter are controlled.
110: Trigger Mode - The counter starts at a rising edge of the trigger
TRGI (but it is notreset).
         Only the start of the counter is controlled.
111: External Clock Mode 1 - Rising edges of the selected trigger
(TRGI) clock the counter.

>> +
>> +     regmap_update_bits(stm32->regmap, TIM_SMCR, TIM_SMCR_SMS, mode);
>> +
>> +     return len;
>> +}
>> +
>> +static IIO_DEVICE_ATTR(slave_mode, S_IRUGO | S_IWUSR,
> There is an iritating move (in terms of noise it's generating) to use values
> directly instead fo these defines.  Still if you don't fix it here I'll only
> get a patch 'fixing' it soon after...

I will fix at in version 3

>
>> +                    _show_slave_mode,
>> +                    _store_slave_mode,
>> +                    0);
>> +
>> +static struct attribute *stm32_timer_attrs[] = {
>> +     &iio_dev_attr_master_mode.dev_attr.attr,
>> +     &iio_dev_attr_slave_mode.dev_attr.attr,
> New ABI so must be documented under Documentation/ABI/testing/sysfs-bus-iio-*

OK

>> +     NULL,
>> +};
>> +
>> +static const struct attribute_group stm32_timer_attr_group = {
>> +     .attrs = stm32_timer_attrs,
>> +};
>> +
>> +static int stm32_timer_start(struct stm32_iio_timer_dev *stm32)
>> +{
>> +     unsigned long long prd, div;
>> +     int prescaler = 0;
>> +     u32 max_arr = 0xFFFF, cr1;
>> +
>> +     if (stm32->sampling_frequency == 0)
>> +             return 0;
>> +
>> +     /* Period and prescaler values depends of clock rate */
>> +     div = (unsigned long long)clk_get_rate(stm32->clk);
>> +
>> +     do_div(div, stm32->sampling_frequency);
>> +
>> +     prd = div;
>> +
>> +     while (div > max_arr) {
>> +             prescaler++;
>> +             div = prd;
>> +             do_div(div, (prescaler + 1));
>> +     }
>> +     prd = div;
>> +
>> +     if (prescaler > MAX_TIM_PSC) {
>> +             dev_err(stm32->dev, "prescaler exceeds the maximum value\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* Check that we own the timer */
>> +     regmap_read(stm32->regmap, TIM_CR1, &cr1);
>> +     if ((cr1 & TIM_CR1_CEN) && !stm32->own_timer)
>> +             return -EBUSY;
>> +
>> +     if (!stm32->own_timer) {
>> +             stm32->own_timer = true;
>> +             clk_enable(stm32->clk);
>> +     }
>> +
>> +     regmap_write(stm32->regmap, TIM_PSC, prescaler);
>> +     regmap_write(stm32->regmap, TIM_ARR, prd - 1);
>> +     regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>> +
>> +     /* Force master mode to update mode */
>> +     regmap_update_bits(stm32->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
>> +
>> +     /* Make sure that registers are updated */
>> +     regmap_update_bits(stm32->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
>> +
>> +     /* Enable interrupt */
>> +     regmap_write(stm32->regmap, TIM_SR, 0);
>> +     regmap_update_bits(stm32->regmap, TIM_DIER, TIM_DIER_UIE, TIM_DIER_UIE);
>> +
>> +     /* Enable controller */
>> +     regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_timer_stop(struct stm32_iio_timer_dev *stm32)
>> +{
>> +     if (!stm32->own_timer)
>> +             return 0;
>> +
>> +     /* Stop timer */
>> +     regmap_update_bits(stm32->regmap, TIM_DIER, TIM_DIER_UIE, 0);
>> +     regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_CEN, 0);
>> +     regmap_write(stm32->regmap, TIM_PSC, 0);
>> +     regmap_write(stm32->regmap, TIM_ARR, 0);
>> +
>> +     clk_disable(stm32->clk);
>> +
>> +     stm32->own_timer = false;
>> +     stm32->active_trigger = NULL;
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_set_trigger_state(struct iio_trigger *trig, bool state)
>> +{
>> +     struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>> +
>> +     stm32->active_trigger = trig;
>> +
>> +     if (state)
>> +             return stm32_timer_start(stm32);
>> +     else
>> +             return stm32_timer_stop(stm32);
>> +}
>> +
>> +static irqreturn_t stm32_timer_irq_handler(int irq, void *private)
>> +{
>> +     struct stm32_iio_timer_dev *stm32 = private;
>> +     u32 sr;
>> +
>> +     regmap_read(stm32->regmap, TIM_SR, &sr);
>> +     regmap_write(stm32->regmap, TIM_SR, 0);
>> +
>> +     if ((sr & TIM_SR_UIF) && stm32->active_trigger)
>> +             iio_trigger_poll(stm32->active_trigger);
> This is acting like a trigger cascading off another trigger?

Not only a trigger but ADC or DAC too.

>
> Normally this interrupt handler would be directly associated with the
> trigger hardware - in this case the timer.

>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static const struct iio_trigger_ops timer_trigger_ops = {
>> +     .owner = THIS_MODULE,
>> +     .set_trigger_state = stm32_set_trigger_state,
>> +};
>> +
>> +static int stm32_setup_iio_triggers(struct stm32_iio_timer_dev *stm32)
>> +{
>> +     int ret;
>> +     struct property *p;
>> +     const char *cur = NULL;
>> +
>> +     p = of_find_property(stm32->dev->of_node,
>> +                          "st,output-triggers-names", NULL);
>> +
>> +     while ((cur = of_prop_next_string(p, cur)) != NULL) {
>> +             struct iio_trigger *trig;
>> +
>> +             trig = devm_iio_trigger_alloc(stm32->dev, "%s", cur);
>> +             if  (!trig)
>> +                     return -ENOMEM;
>> +
>> +             trig->dev.parent = stm32->dev->parent;
>> +             trig->ops = &timer_trigger_ops;
>> +             trig->dev.groups = stm32_trigger_attr_groups;
>> +             iio_trigger_set_drvdata(trig, stm32);
>> +
>> +             ret = devm_iio_trigger_register(stm32->dev, trig);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +/**
>> + * is_stm32_iio_timer_trigger
>> + * @trig: trigger to be checked
>> + *
>> + * return true if the trigger is a valid stm32 iio timer trigger
>> + * either return false
>> + */
>> +bool is_stm32_iio_timer_trigger(struct iio_trigger *trig)
>> +{
>> +     return (trig->ops == &timer_trigger_ops);
>> +}
>> +EXPORT_SYMBOL(is_stm32_iio_timer_trigger);
>> +
>> +static int stm32_validate_trigger(struct iio_dev *indio_dev,
>> +                               struct iio_trigger *trig)
>> +{
>> +     struct stm32_iio_timer_dev *dev = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     if (!is_stm32_iio_timer_trigger(trig))
>> +             return -EINVAL;
>> +
>> +     ret = of_property_match_string(dev->dev->of_node,
>> +                                    "st,input-triggers-names",
>> +                                    trig->name);
>> +
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     regmap_update_bits(dev->regmap, TIM_SMCR, TIM_SMCR_TS, ret << 4);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct iio_info stm32_trigger_info = {
>> +     .driver_module = THIS_MODULE,
>> +     .validate_trigger = stm32_validate_trigger,
>> +     .attrs = &stm32_timer_attr_group,
>> +};
>> +
>> +static struct stm32_iio_timer_dev *stm32_setup_iio_device(struct device *dev)
>> +{
>> +     struct iio_dev *indio_dev;
>> +     int ret;
>> +
>> +     indio_dev = devm_iio_device_alloc(dev, sizeof(struct stm32_iio_timer_dev));
>> +     if (!indio_dev)
>> +             return NULL;
> This is 'unusual'.  Why does a trigger driver need an iio_dev at all?

Trigger doesn't need it but for configuring the input switch when
validating the triggers I need a device

>> +
>> +     indio_dev->name = dev_name(dev);
>> +     indio_dev->dev.parent = dev;
>> +     indio_dev->info = &stm32_trigger_info;
>> +     indio_dev->modes = INDIO_EVENT_TRIGGERED;
>> +     indio_dev->num_channels = 0;
>> +     indio_dev->dev.of_node = dev->of_node;
>> +
>> +     ret = iio_triggered_event_setup(indio_dev,
>> +                                     NULL,
>> +                                     stm32_timer_irq_handler);
> So the iio_dev exists to provide the ability to fire this interrupt from
> another trigger?  Why do you want to do this?

I need interrupt because I use set_trigger_state() to enable/disable
the sampling frequency.
As far I understand and test set_trigger_state() is only called when
indio_dev->modes = INDIO_EVENT_TRIGGERED
and iio_triggered_event_setup have been called to create register an
irq handler.
I just need irq declaration for this last condition, I don't need the
irq to fire in kernel to drive other hardware block.

If I could use set_trigger_state() without calling using
iio_triggered_event_setup() I should remove all
irq code from the driver.

One possible solution would be to add calls to set_trigger_state() in
iio_trigger_write_current() function
at the same level than iio_trigger_detach_poll_func() or
iio_trigger_attach_poll_func() calls:

if (indio_dev->modes = INDIO_DIRECT_MODE && oldtrig->ops->set_trigger_state)
     oldtrig->ops->set_trigger_state(oldtrig, false);

if (indio_dev->modes = INDIO_DIRECT_MODE &&
indio_dev->trig->ops->set_trigger_state)
     indio_dev->trig->ops->set_trigger_state(indio_dev->trig, true);

I'm to new in IIO framework to understand if that it possible or not

>> +     if (ret)
>> +             return NULL;
>> +
>> +     ret = devm_iio_device_register(dev, indio_dev);
>> +     if (ret) {
>> +             iio_triggered_event_cleanup(indio_dev);
>> +             return NULL;
>> +     }
>> +
>> +     return iio_priv(indio_dev);
>> +}
>> +
>> +static int stm32_iio_timer_probe(struct platform_device *pdev)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     struct stm32_iio_timer_dev *stm32;
>> +     struct stm32_gptimer_dev *mfd = dev_get_drvdata(pdev->dev.parent);
>> +     int ret;
>> +
>> +     stm32 = stm32_setup_iio_device(dev);
>> +     if (!stm32)
>> +             return -ENOMEM;
>> +
>> +     stm32->dev = dev;
>> +     stm32->regmap = mfd->regmap;
>> +     stm32->clk = mfd->clk;
>> +
>> +     stm32->irq = platform_get_irq(pdev, 0);
>> +     if (stm32->irq < 0)
>> +             return -EINVAL;
>> +
>> +     ret = devm_request_irq(stm32->dev, stm32->irq,
>> +                            stm32_timer_irq_handler, IRQF_SHARED,
>> +                            "iiotimer_event", stm32);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = stm32_setup_iio_triggers(stm32);
>> +     if (ret)
>> +             return ret;
>> +
>> +     platform_set_drvdata(pdev, stm32);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_iio_timer_remove(struct platform_device *pdev)
>> +{
>> +     struct stm32_iio_timer_dev *stm32 = platform_get_drvdata(pdev);
>> +
>> +     iio_triggered_event_cleanup((struct iio_dev *)stm32);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct of_device_id stm32_trig_of_match[] = {
>> +     {
>> +             .compatible = "st,stm32-iio-timer",
>> +     },
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
>> +
>> +static struct platform_driver stm32_iio_timer_driver = {
>> +     .probe = stm32_iio_timer_probe,
>> +     .remove = stm32_iio_timer_remove,
>> +     .driver = {
>> +             .name = DRIVER_NAME,
>> +             .of_match_table = stm32_trig_of_match,
>> +     },
>> +};
>> +module_platform_driver(stm32_iio_timer_driver);
>> +
>> +MODULE_ALIAS("platform:" DRIVER_NAME);
>> +MODULE_DESCRIPTION("STMicroelectronics STM32 iio timer driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
>> index 809b2e7..f2af4fe 100644
>> --- a/drivers/iio/trigger/Kconfig
>> +++ b/drivers/iio/trigger/Kconfig
>> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER
>>
>>         To compile this driver as a module, choose M here: the
>>         module will be called iio-trig-sysfs.
>> -
> Clear this out...
>>  endmenu
>> diff --git a/include/dt-bindings/iio/timer/st,stm32-iio-timer.h b/include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>> new file mode 100644
>> index 0000000..d39bf16
>> --- /dev/null
>> +++ b/include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + * st,stm32-iio-timer.h
>> + *
>> + * Copyright (C) STMicroelectronics 2016
>> + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org> for STMicroelectronics.
>> + * License terms:  GNU General Public License (GPL), version 2
>> + */
>> +
>> +#ifndef _DT_BINDINGS_IIO_TIMER_H_
>> +#define _DT_BINDINGS_IIO_TIMER_H_
>> +
>> +#define TIM1_TRGO    "tim1_trgo"
>> +#define TIM2_TRGO    "tim2_trgo"
>> +#define TIM3_TRGO    "tim3_trgo"
>> +#define TIM4_TRGO    "tim4_trgo"
>> +#define TIM5_TRGO    "tim5_trgo"
>> +#define TIM6_TRGO    "tim6_trgo"
>> +#define TIM7_TRGO    "tim7_trgo"
>> +#define TIM8_TRGO    "tim8_trgo"
>> +#define TIM9_TRGO    "tim9_trgo"
>> +#define TIM12_TRGO   "tim12_trgo"
>> +
>> +#endif
>> diff --git a/include/linux/iio/timer/stm32-iio-timers.h b/include/linux/iio/timer/stm32-iio-timers.h
>> new file mode 100644
>> index 0000000..5d1b86c
>> --- /dev/null
>> +++ b/include/linux/iio/timer/stm32-iio-timers.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * stm32-iio-timers.h
>> + *
>> + * Copyright (C) STMicroelectronics 2016
>> + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org> for STMicroelectronics.
>> + * License terms:  GNU General Public License (GPL), version 2
>> + */
>> +
>> +#ifndef _STM32_IIO_TIMERS_H_
>> +#define _STM32_IIO_TIMERS_H_
>> +
>> +#include <dt-bindings/iio/timer/st,stm32-iio-timer.h>
>> +
>> +bool is_stm32_iio_timer_trigger(struct iio_trigger *trig);
>> +
>> +#endif
>>
>
--
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


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