Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCHv9][ 1/3] Input: tsc2007: Add device tree support.
From: Denis Carikli @ 2013-11-08 13:17 UTC (permalink / raw)
  To: linux-arm-kernel

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v8->v9:
- Added Grant Likely in the Cc list.
- Removed the mention of the pinctrl properties in the documentation.

ChangeLog v7->v8:
- Fixed the lack of x and z fuzz properties.
- The pendown gpio is better documented.
- Added Shawn Guo in the cc list.
---
 .../bindings/input/touchscreen/tsc2007.txt         |   41 ++++
 drivers/input/touchscreen/tsc2007.c                |  204 ++++++++++++++++----
 2 files changed, 204 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
new file mode 100644
index 0000000..028aba66d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
@@ -0,0 +1,41 @@
+* Texas Instruments tsc2007 touchscreen controller
+
+Required properties:
+- compatible: must be "ti,tsc2007".
+- reg: I2C address of the chip.
+- ti,x-plate-ohms: X-plate resistance in ohms.
+
+Optional properties:
+- gpios: the interrupt gpio the chip is connected to (trough the penirq pin).
+  The penirq pin goes to low when the panel is touched.
+  (see GPIO binding[1] for more details).
+- interrupt-parent: the phandle for the gpio controller
+  (see interrupt binding[0]).
+- interrupts: (gpio) interrupt to which the chip is connected
+  (see interrupt binding[0]).
+- ti,max-rt: maximum pressure.
+- ti,fuzzx: specifies the absolute input fuzz x value.
+  If set, it will permit noise in the data up to +- the value given to the fuzz
+  parameter, that is used to filter noise from the event stream.
+- ti,fuzzy: specifies the absolute input fuzz y value.
+- ti,fuzzz: specifies the absolute input fuzz z value.
+- ti,poll-period: how much time to wait(in millisecond) before reading again the
+  values from the tsc2007.
+
+[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[1]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+	&i2c1 {
+		/* ... */
+		tsc2007@49 {
+			compatible = "ti,tsc2007";
+			reg = <0x49>;
+			interrupt-parent = <&gpio4>;
+			interrupts = <0x0 0x8>;
+			gpios = <&gpio4 0 0>;
+			ti,x-plate-ohms = <180>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 0b67ba4..3168a99 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -26,6 +26,9 @@
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
 #include <linux/i2c/tsc2007.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)
@@ -74,7 +77,12 @@ struct tsc2007 {
 	u16			max_rt;
 	unsigned long		poll_delay;
 	unsigned long		poll_period;
+	int			fuzzx;
+	int			fuzzy;
+	int			fuzzz;
+	char			of;
 
+	unsigned		gpio;
 	int			irq;
 
 	wait_queue_head_t	wait;
@@ -84,6 +92,11 @@ struct tsc2007 {
 	void			(*clear_penirq)(void);
 };
 
+static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
+{
+	return !gpio_get_value(ts->gpio);
+}
+
 static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
 {
 	s32 data;
@@ -142,6 +155,14 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
 	return rt;
 }
 
+static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
+{
+	if (ts->of)
+		return gpio_is_valid(ts->gpio);
+	else
+		return ts->get_pendown_state ? true : false;
+}
+
 static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 {
 	/*
@@ -158,10 +179,13 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 	 * to fall back on the pressure reading.
 	 */
 
-	if (!ts->get_pendown_state)
+	if (!tsc2007_is_pen_down_valid(ts))
 		return true;
 
-	return ts->get_pendown_state();
+	if (ts->of)
+		return tsc2007_get_pendown_state_dt(ts);
+	else
+		return ts->get_pendown_state();
 }
 
 static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -178,7 +202,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 
 		rt = tsc2007_calculate_pressure(ts, &tc);
 
-		if (rt = 0 && !ts->get_pendown_state) {
+		if (!rt && !tsc2007_is_pen_down_valid(ts)) {
 			/*
 			 * If pressure reported is 0 and we don't have
 			 * callback to check pendown state, we have to
@@ -228,7 +252,7 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 {
 	struct tsc2007 *ts = handle;
 
-	if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+	if (tsc2007_is_pen_down(ts))
 		return IRQ_WAKE_THREAD;
 
 	if (ts->clear_penirq)
@@ -273,34 +297,71 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 
-static int tsc2007_probe(struct i2c_client *client,
-				   const struct i2c_device_id *id)
+#ifdef CONFIG_OF
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+			    struct device_node *np)
 {
-	struct tsc2007 *ts;
-	struct tsc2007_platform_data *pdata = client->dev.platform_data;
-	struct input_dev *input_dev;
-	int err;
-
-	if (!pdata) {
-		dev_err(&client->dev, "platform data is required!\n");
+	int err = 0;
+	u32 val32;
+	u64 val64;
+
+	if (!of_property_read_u32(np, "ti,max-rt", &val32))
+		ts->max_rt = val32;
+	else
+		ts->max_rt = MAX_12BIT;
+
+	if (!of_property_read_u32(np, "ti,fuzzx", &val32))
+		ts->fuzzx = val32;
+
+	if (!of_property_read_u32(np, "ti,fuzzy", &val32))
+		ts->fuzzy = val32;
+
+	if (!of_property_read_u32(np, "ti,fuzzz", &val32))
+		ts->fuzzz = val32;
+
+	if (!of_property_read_u64(np, "ti,poll-period", &val64))
+		ts->poll_period = val64;
+	else
+		ts->poll_period = 1;
+
+	if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
+		ts->x_plate_ohms = val32;
+	} else {
+		dev_err(&client->dev,
+			"Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
+			err);
 		return -EINVAL;
 	}
 
-	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EIO;
+	ts->gpio = of_get_gpio(np, 0);
+	if (!gpio_is_valid(ts->gpio))
+		dev_err(&client->dev,
+			"GPIO not found (of_get_gpio returned %d)\n",
+			ts->gpio);
 
-	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
-		err = -ENOMEM;
-		goto err_free_mem;
-	}
+	/* Used to detect if it is probed trough the device tree,
+	 * in order to be able to use that information in the IRQ handler.
+	 */
+	ts->of = 1;
 
-	ts->client = client;
-	ts->irq = client->irq;
-	ts->input = input_dev;
-	init_waitqueue_head(&ts->wait);
+	return 0;
+}
+#else
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+			    struct device_node *np)
+{
+	return -ENODEV;
+}
+#endif
+
+static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
+			      struct tsc2007_platform_data *pdata,
+			      const struct i2c_device_id *id)
+{
+	if (!pdata) {
+		dev_err(&client->dev, "platform data is required!\n");
+		return -EINVAL;
+	}
 
 	ts->model             = pdata->model;
 	ts->x_plate_ohms      = pdata->x_plate_ohms;
@@ -309,13 +370,59 @@ static int tsc2007_probe(struct i2c_client *client,
 	ts->poll_period       = pdata->poll_period ? : 1;
 	ts->get_pendown_state = pdata->get_pendown_state;
 	ts->clear_penirq      = pdata->clear_penirq;
+	ts->fuzzx             = pdata->fuzzx;
+	ts->fuzzy             = pdata->fuzzy;
+	ts->fuzzz             = pdata->fuzzz;
 
 	if (pdata->x_plate_ohms = 0) {
 		dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
-		err = -EINVAL;
-		goto err_free_mem;
+		return -EINVAL;
 	}
 
+	/* Used to detect if it is probed trough the device tree,
+	 * in order to be able to use that information in the IRQ handler.
+	 */
+	ts->of = 0;
+
+	return 0;
+}
+
+static int tsc2007_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device_node *np = client->dev.of_node;
+	struct tsc2007_platform_data *pdata = client->dev.platform_data;
+	struct tsc2007 *ts;
+	struct input_dev *input_dev;
+	int err = 0;
+
+	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	if (np)
+		err = tsc2007_probe_dt(client, ts, np);
+	else
+		err = tsc2007_probe_pdev(client, ts, pdata, id);
+
+	if (err)
+		return err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -EIO;
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		err = -ENOMEM;
+		goto err_free_input;
+	};
+
+	ts->client = client;
+	ts->irq = client->irq;
+	ts->input = input_dev;
+	init_waitqueue_head(&ts->wait);
+
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));
 
@@ -331,19 +438,21 @@ static int tsc2007_probe(struct i2c_client *client,
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
+	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzx, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
-			pdata->fuzzz, 0);
+			     ts->fuzzz, 0);
 
-	if (pdata->init_platform_hw)
-		pdata->init_platform_hw();
+	if (!np) {
+		if (pdata->init_platform_hw)
+			pdata->init_platform_hw();
+	}
 
 	err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
 				   IRQF_ONESHOT, client->dev.driver->name, ts);
 	if (err < 0) {
 		dev_err(&client->dev, "irq %d busy?\n", ts->irq);
-		goto err_free_mem;
+		goto err_free_input;
 	}
 
 	tsc2007_stop(ts);
@@ -358,23 +467,27 @@ static int tsc2007_probe(struct i2c_client *client,
 
  err_free_irq:
 	free_irq(ts->irq, ts);
-	if (pdata->exit_platform_hw)
-		pdata->exit_platform_hw();
- err_free_mem:
+	if (!np) {
+		if (pdata->exit_platform_hw)
+			pdata->exit_platform_hw();
+	}
+ err_free_input:
 	input_free_device(input_dev);
-	kfree(ts);
 	return err;
 }
 
 static int tsc2007_remove(struct i2c_client *client)
 {
+	struct device_node *np = client->dev.of_node;
 	struct tsc2007	*ts = i2c_get_clientdata(client);
 	struct tsc2007_platform_data *pdata = client->dev.platform_data;
 
 	free_irq(ts->irq, ts);
 
-	if (pdata->exit_platform_hw)
-		pdata->exit_platform_hw();
+	if (!np) {
+		if (pdata->exit_platform_hw)
+			pdata->exit_platform_hw();
+	}
 
 	input_unregister_device(ts->input);
 	kfree(ts);
@@ -389,10 +502,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
 
 MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
 
+#ifdef CONFIG_OF
+static const struct of_device_id tsc2007_of_match[] = {
+	{ .compatible = "ti,tsc2007" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsc2007_of_match);
+#endif
+
 static struct i2c_driver tsc2007_driver = {
 	.driver = {
 		.owner	= THIS_MODULE,
-		.name	= "tsc2007"
+		.name	= "tsc2007",
+		.of_match_table = of_match_ptr(tsc2007_of_match),
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv9][ 2/3] ARM: dts: cpuimx51 Add touchscreen support.
From: Denis Carikli @ 2013-11-08 13:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1383916659-9988-1-git-send-email-denis@eukrea.com>

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v8->v9:
- Added Grant Likely in the Cc list.
- Adapted to the removal of the pinctrl properties in the tsc2007 documentation.
- Fixed the gpios property (before, it was set to active high by error).

ChangeLog v7->v8:
- Added Shawn Guo in the cc list.
---
 arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
index b22841a..b04c65b 100644
--- a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
+++ b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
@@ -42,11 +42,23 @@
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 	};
+
+	tsc2007: tsc2007@49 {
+		compatible = "ti,tsc2007";
+		reg = <0x49>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <0x0 0x8>;
+		gpios = <&gpio4 0 1>;
+		ti,x-plate-ohms = <180>;
+	};
 };
 
 &iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hog>;
+
 	imx51-eukrea {
-		pinctrl_tsc2007_1: tsc2007grp-1 {
+		pinctrl_hog: hoggrp {
 			fsl,pins = <
 				MX51_PAD_GPIO_NAND__GPIO_NAND 0x1f5
 				MX51_PAD_NANDF_D8__GPIO4_0 0x1f5
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv9][ 3/3] ARM: dts: cpuimx35 Add touchscreen support.
From: Denis Carikli @ 2013-11-08 13:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1383916659-9988-1-git-send-email-denis@eukrea.com>

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v8->v9:
- Added Grant Likely in the cc list.
- Adapted to the removal of the pinctrl properties in the tsc2007 documentation.
- Fixed the gpios property (before, it was set to active high by error).

ChangeLog v7->v8:
- Added Shawn Guo in the cc list.
---
 arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
index b9cb5a5..f25a40f 100644
--- a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
+++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
@@ -36,6 +36,27 @@
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 	};
+
+	tsc2007: tsc2007@48 {
+		compatible = "ti,tsc2007";
+		reg = <0x48>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <0x2 0x8>;
+		gpios = <&gpio3 2 1>;
+		ti,x-plate-ohms = <180>;
+        };
+
+};
+
+&iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hog>;
+
+	imx35-eukrea {
+		pinctrl_hog: hoggrp {
+			fsl,pins = <MX35_PAD_ATA_DA2__GPIO3_2 0x80000000>;
+		};
+	};
 };
 
 &nfc {
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 11/12] fbdev: shmobile-hdmi: Convert to clk_prepare/unprepare
From: Laurent Pinchart @ 2013-11-09 14:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131031104809.GB18477@ns203013.ovh.net>

Hi Jean-Christophe,

On Thursday 31 October 2013 11:48:09 Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 23:49 Mon 28 Oct     , Laurent Pinchart wrote:
> > Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
> > clk_disable_unprepare() to get ready for the migration to the common
> > clock framework.
> > 
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Thank you. Could you please pick patches 11/12 and 12/12 up for v3.13 or v3.14 
?

> > Cc: linux-fbdev@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/video/sh_mobile_hdmi.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/video/sh_mobile_hdmi.c
> > b/drivers/video/sh_mobile_hdmi.c index bfe4728..190145e 100644
> > --- a/drivers/video/sh_mobile_hdmi.c
> > +++ b/drivers/video/sh_mobile_hdmi.c
> > @@ -1326,7 +1326,7 @@ static int __init sh_hdmi_probe(struct
> > platform_device *pdev)> 
> >  		goto erate;
> >  	
> >  	}
> > 
> > -	ret = clk_enable(hdmi->hdmi_clk);
> > +	ret = clk_prepare_enable(hdmi->hdmi_clk);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(hdmi->dev, "Cannot enable clock: %d\n", ret);
> >  		goto erate;
> > 
> > @@ -1404,7 +1404,7 @@ emap_htop1:
> >  emap:
> >  	release_mem_region(res->start, resource_size(res));
> >  
> >  ereqreg:
> > -	clk_disable(hdmi->hdmi_clk);
> > +	clk_disable_unprepare(hdmi->hdmi_clk);
> > 
> >  erate:
> >  	clk_put(hdmi->hdmi_clk);
> >  
> >  egetclk:
> > @@ -1427,7 +1427,7 @@ static int __exit sh_hdmi_remove(struct
> > platform_device *pdev)> 
> >  	cancel_delayed_work_sync(&hdmi->edid_work);
> >  	pm_runtime_put(&pdev->dev);
> >  	pm_runtime_disable(&pdev->dev);
> > 
> > -	clk_disable(hdmi->hdmi_clk);
> > +	clk_disable_unprepare(hdmi->hdmi_clk);
> > 
> >  	clk_put(hdmi->hdmi_clk);
> >  	if (hdmi->htop1)
> >  	
> >  		iounmap(hdmi->htop1);

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH] video: add OpenCores VGA/LCD framebuffer driver
From: Stefan Kristiansson @ 2013-11-10 13:08 UTC (permalink / raw)
  To: linux-kernel, linux-fbdev; +Cc: plagnioj, tomi.valkeinen, Stefan Kristiansson

This adds support for the VGA/LCD core available from OpenCores:
http://opencores.org/project,vga_lcd

The driver have been tested together with both OpenRISC and
ARM (socfpga) processors.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
 drivers/video/Kconfig  |  17 ++
 drivers/video/Makefile |   1 +
 drivers/video/ocfb.c   | 479 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 497 insertions(+)
 create mode 100644 drivers/video/ocfb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 84b685f..2ea850c 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -979,6 +979,23 @@ config FB_PVR2
 	  (<file:drivers/video/pvr2fb.c>). Please see the file
 	  <file:Documentation/fb/pvr2fb.txt>.
 
+config FB_OPENCORES
+	tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
+	depends on FB
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	default n
+	help
+	  This enables support for the OpenCores VGA/LCD core.
+
+	  The OpenCores VGA/LCD core is typically used together with
+	  softcore CPUs (e.g. OpenRISC) or hard processor systems
+	  (e.g. Altera socfpga or Xilinx Zync) on FPGAs.
+
+	  The source code and specification for the core is available at
+	  <http://opencores.org/project,vga_lcd>
+
 config FB_S1D13XXX
 	tristate "Epson S1D13XXX framebuffer support"
 	depends on FB
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e8bae8d..ae17ddf 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -150,6 +150,7 @@ obj-$(CONFIG_FB_NUC900)           += nuc900fb.o
 obj-$(CONFIG_FB_JZ4740)		  += jz4740_fb.o
 obj-$(CONFIG_FB_PUV3_UNIGFX)      += fb-puv3.o
 obj-$(CONFIG_FB_HYPERV)		  += hyperv_fb.o
+obj-$(CONFIG_FB_OPENCORES)	  += ocfb.o
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_UVESA)            += uvesafb.o
diff --git a/drivers/video/ocfb.c b/drivers/video/ocfb.c
new file mode 100644
index 0000000..6b01e65
--- /dev/null
+++ b/drivers/video/ocfb.c
@@ -0,0 +1,479 @@
+/*
+ * OpenCores VGA/LCD 2.0 core frame buffer driver
+ *
+ * Copyright (C) 2013 Stefan Kristiansson, stefan.kristiansson@saunalahti.fi
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/mm.h>
+#include <linux/dma-mapping.h>
+#include <linux/fb.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+
+/* OCFB register defines */
+#define OCFB_CTRL	0x000
+#define OCFB_STAT	0x004
+#define OCFB_HTIM	0x008
+#define OCFB_VTIM	0x00c
+#define OCFB_HVLEN	0x010
+#define OCFB_VBARA	0x014
+#define OCFB_PALETTE	0x800
+
+#define OCFB_CTRL_VEN	0x00000001 /* Video Enable */
+#define OCFB_CTRL_HIE	0x00000002 /* HSync Interrupt Enable */
+#define OCFB_CTRL_PC	0x00000800 /* 8-bit Pseudo Color Enable*/
+#define OCFB_CTRL_CD8	0x00000000 /* Color Depth 8 */
+#define OCFB_CTRL_CD16	0x00000200 /* Color Depth 16 */
+#define OCFB_CTRL_CD24	0x00000400 /* Color Depth 24 */
+#define OCFB_CTRL_CD32	0x00000600 /* Color Depth 32 */
+#define OCFB_CTRL_VBL1	0x00000000 /* Burst Length 1 */
+#define OCFB_CTRL_VBL2	0x00000080 /* Burst Length 2 */
+#define OCFB_CTRL_VBL4	0x00000100 /* Burst Length 4 */
+#define OCFB_CTRL_VBL8	0x00000180 /* Burst Length 8 */
+
+#define PALETTE_SIZE	256
+
+#define OCFB_NAME	"OC VGA/LCD"
+
+static char *mode_option;
+
+static const struct fb_videomode default_mode = {
+	/* 640x480 @ 60 Hz, 31.5 kHz hsync */
+	NULL, 60, 640, 480, 39721, 40, 24, 32, 11, 96, 2,
+	0, FB_VMODE_NONINTERLACED
+};
+
+struct ocfb_dev {
+	struct fb_info info;
+	/* Physical and virtual addresses of control regs */
+	phys_addr_t regs_phys;
+	int regs_phys_size;
+	void __iomem *regs;
+	/* flag indicating whether the regs are little endian accessed */
+	int little_endian;
+	/* Physical and virtual addresses of framebuffer */
+	phys_addr_t fb_phys;
+	void __iomem *fb_virt;
+	u32 pseudo_palette[PALETTE_SIZE];
+};
+
+struct ocfb_par {
+	void __iomem *pal_adr;
+};
+
+static struct ocfb_par ocfb_par_priv;
+
+static struct fb_var_screeninfo ocfb_var;
+static struct fb_fix_screeninfo ocfb_fix;
+
+#ifndef MODULE
+static int __init ocfb_setup(char *options)
+{
+	char *curr_opt;
+
+	if (!options || !*options)
+		return 0;
+
+	while ((curr_opt = strsep(&options, ",")) != NULL) {
+		if (!*curr_opt)
+			continue;
+		mode_option = curr_opt;
+	}
+
+	return 0;
+}
+#endif
+
+static inline u32 ocfb_readreg(struct ocfb_dev *fbdev, loff_t offset)
+{
+	if (fbdev->little_endian)
+		return ioread32(fbdev->regs + offset);
+	else
+		return ioread32be(fbdev->regs + offset);
+}
+
+static void ocfb_writereg(struct ocfb_dev *fbdev, loff_t offset, u32 data)
+{
+	if (fbdev->little_endian)
+		iowrite32(data, fbdev->regs + offset);
+	else
+		iowrite32be(data, fbdev->regs + offset);
+}
+
+static int ocfb_setupfb(struct ocfb_dev *fbdev)
+{
+	unsigned long bpp_config;
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+	struct device *dev = fbdev->info.device;
+	u32 hlen;
+	u32 vlen;
+
+	/* Disable display */
+	ocfb_writereg(fbdev, OCFB_CTRL, 0);
+
+	/* Register framebuffer address */
+	fbdev->little_endian = 0;
+	ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
+
+	/* Detect endianess */
+	if (ocfb_readreg(fbdev, OCFB_VBARA) != fbdev->fb_phys) {
+		fbdev->little_endian = 1;
+		ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
+	}
+
+	/* Horizontal timings */
+	ocfb_writereg(fbdev, OCFB_HTIM, (var->hsync_len - 1) << 24 |
+		      (var->right_margin - 1) << 16 | (var->xres - 1));
+
+	/* Vertical timings */
+	ocfb_writereg(fbdev, OCFB_VTIM, (var->vsync_len - 1) << 24 |
+		      (var->lower_margin - 1) << 16 | (var->yres - 1));
+
+	/* Total length of frame */
+	hlen = var->left_margin + var->right_margin + var->hsync_len +
+		var->xres;
+
+	vlen = var->upper_margin + var->lower_margin + var->vsync_len +
+		var->yres;
+
+	ocfb_writereg(fbdev, OCFB_HVLEN, (hlen - 1) << 16 | (vlen - 1));
+
+	bpp_config = OCFB_CTRL_CD8;
+	switch (var->bits_per_pixel) {
+	case 8:
+		if (!var->grayscale)
+			bpp_config |= OCFB_CTRL_PC;  /* enable palette */
+		break;
+
+	case 16:
+		bpp_config |= OCFB_CTRL_CD16;
+		break;
+
+	case 24:
+		bpp_config |= OCFB_CTRL_CD24;
+		break;
+
+	case 32:
+		bpp_config |= OCFB_CTRL_CD32;
+		break;
+
+	default:
+		dev_err(dev, "no bpp specified\n");
+		break;
+	}
+
+	/* maximum (8) VBL (video memory burst length) */
+	bpp_config |= OCFB_CTRL_VBL8;
+
+	/* Enable output */
+	ocfb_writereg(fbdev, OCFB_CTRL, (OCFB_CTRL_VEN | bpp_config));
+
+	return 0;
+}
+
+static int ocfb_setcolreg(unsigned regno, unsigned red, unsigned green,
+			  unsigned blue, unsigned transp,
+			  struct fb_info *info)
+{
+	struct ocfb_par *par = (struct ocfb_par *)info->par;
+	u32 color;
+
+	if (regno >= info->cmap.len) {
+		dev_err(info->device, "regno >= cmap.len\n");
+		return 1;
+	}
+
+	if (info->var.grayscale) {
+		/* grayscale = 0.30*R + 0.59*G + 0.11*B */
+		red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+	}
+
+	red >>= (16 - info->var.red.length);
+	green >>= (16 - info->var.green.length);
+	blue >>= (16 - info->var.blue.length);
+	transp >>= (16 - info->var.transp.length);
+
+	if (info->var.bits_per_pixel = 8 && !info->var.grayscale) {
+		regno <<= 2;
+		color = (red << 16) | (green << 8) | blue;
+		ocfb_writereg(par->pal_adr, regno, color);
+	} else {
+		((u32 *)(info->pseudo_palette))[regno] +			(red << info->var.red.offset) |
+			(green << info->var.green.offset) |
+			(blue << info->var.blue.offset) |
+			(transp << info->var.transp.offset);
+	}
+
+	return 0;
+}
+
+static int ocfb_init_fix(struct ocfb_dev *fbdev)
+{
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+	struct fb_fix_screeninfo *fix = &fbdev->info.fix;
+
+	strcpy(fix->id, OCFB_NAME);
+
+	fix->line_length = var->xres * var->bits_per_pixel/8;
+	fix->smem_len = fix->line_length * var->yres;
+	fix->type = FB_TYPE_PACKED_PIXELS;
+
+	if (var->bits_per_pixel = 8 && !var->grayscale)
+		fix->visual = FB_VISUAL_PSEUDOCOLOR;
+	else
+		fix->visual = FB_VISUAL_TRUECOLOR;
+
+	return 0;
+}
+
+static int ocfb_init_var(struct ocfb_dev *fbdev)
+{
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+
+	var->accel_flags = FB_ACCEL_NONE;
+	var->activate = FB_ACTIVATE_NOW;
+	var->xres_virtual = var->xres;
+	var->yres_virtual = var->yres;
+
+	switch (var->bits_per_pixel) {
+	case 8:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 0;
+		var->red.length = 8;
+		var->green.offset = 0;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+
+	case 16:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 11;
+		var->red.length = 5;
+		var->green.offset = 5;
+		var->green.length = 6;
+		var->blue.offset = 0;
+		var->blue.length  = 5;
+		break;
+
+	case 24:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+
+	case 32:
+		var->transp.offset = 24;
+		var->transp.length = 8;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+	}
+
+	return 0;
+}
+
+static struct fb_ops ocfb_ops = {
+	.owner		= THIS_MODULE,
+	.fb_setcolreg	= ocfb_setcolreg,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+};
+
+static int ocfb_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct ocfb_dev *fbdev;
+	struct ocfb_par *par = &ocfb_par_priv;
+	struct resource *res;
+	struct resource *mmio;
+	int fbsize;
+
+	fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
+	if (!fbdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, fbdev);
+
+	fbdev->info.fbops = &ocfb_ops;
+	fbdev->info.var = ocfb_var;
+	fbdev->info.fix = ocfb_fix;
+	fbdev->info.device = &pdev->dev;
+	fbdev->info.par = par;
+
+	/* Video mode setup */
+	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
+			  NULL, 0, &default_mode, 16)) {
+		dev_err(&pdev->dev, "No valid video modes found\n");
+		kfree(fbdev);
+		return -EINVAL;
+	}
+	ocfb_init_var(fbdev);
+	ocfb_init_fix(fbdev);
+
+	/* Request I/O resource */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "I/O resource request failed\n");
+		kfree(fbdev);
+		return -ENXIO;
+	}
+	fbdev->regs_phys = res->start;
+	fbdev->regs_phys_size = resource_size(res);
+	mmio = devm_request_mem_region(&pdev->dev, res->start,
+				       resource_size(res), res->name);
+	if (!mmio) {
+		dev_err(&pdev->dev, "I/O memory space request failed\n");
+		kfree(fbdev);
+		return -ENXIO;
+	}
+	fbdev->regs = devm_ioremap_nocache(&pdev->dev, mmio->start,
+					   resource_size(mmio));
+	if (!fbdev->regs) {
+		dev_err(&pdev->dev, "I/O memory remap request failed\n");
+		kfree(fbdev);
+		return -ENXIO;
+	}
+	par->pal_adr = fbdev->regs + OCFB_PALETTE;
+
+	/* Allocate framebuffer memory */
+	fbsize = fbdev->info.fix.smem_len;
+	fbdev->fb_virt = dma_alloc_coherent(&pdev->dev, PAGE_ALIGN(fbsize),
+					    &fbdev->fb_phys, GFP_KERNEL);
+	if (!fbdev->fb_virt) {
+		dev_err(&pdev->dev,
+			"Frame buffer memory allocation failed\n");
+		ret = -ENOMEM;
+		goto err_release;
+	}
+	fbdev->info.fix.smem_start = fbdev->fb_phys;
+	fbdev->info.screen_base = (void __iomem *)fbdev->fb_virt;
+	fbdev->info.pseudo_palette = fbdev->pseudo_palette;
+
+	/* Clear framebuffer */
+	memset_io((void __iomem *)fbdev->fb_virt, 0, fbsize);
+
+	/* Setup and enable the framebuffer */
+	ocfb_setupfb(fbdev);
+
+	if (fbdev->little_endian)
+		fbdev->info.flags |= FBINFO_FOREIGN_ENDIAN;
+
+	/* Allocate color map */
+	ret = fb_alloc_cmap(&fbdev->info.cmap, PALETTE_SIZE, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "Color map allocation failed\n");
+		goto err_dma_free;
+	}
+
+	/* Register framebuffer */
+	ret = register_framebuffer(&fbdev->info);
+	if (ret) {
+		dev_err(&pdev->dev, "Framebuffer registration failed\n");
+		goto err_dealloc_cmap;
+	}
+
+	return 0;
+
+err_dealloc_cmap:
+	fb_dealloc_cmap(&fbdev->info.cmap);
+
+err_dma_free:
+	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbsize), fbdev->fb_virt,
+			  fbdev->fb_phys);
+err_release:
+	release_mem_region(fbdev->regs_phys, fbdev->regs_phys_size);
+	kfree(fbdev);
+
+	return ret;
+}
+
+static int ocfb_remove(struct platform_device *pdev)
+{
+	struct ocfb_dev *fbdev = platform_get_drvdata(pdev);
+
+	unregister_framebuffer(&fbdev->info);
+	fb_dealloc_cmap(&fbdev->info.cmap);
+	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbdev->info.fix.smem_len),
+			  fbdev->fb_virt, fbdev->fb_phys);
+
+	/* Disable display */
+	ocfb_writereg(fbdev, OCFB_CTRL, 0);
+
+	release_mem_region(fbdev->regs_phys, fbdev->regs_phys_size);
+	kfree(fbdev);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct of_device_id ocfb_match[] = {
+	{ .compatible = "opencores,ocfb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ocfb_match);
+
+static struct platform_driver ocfb_driver = {
+	.probe  = ocfb_probe,
+	.remove	= ocfb_remove,
+	.driver = {
+		.name = "ocfb_fb",
+		.of_match_table = ocfb_match,
+	}
+};
+
+/*
+ * Init and exit routines
+ */
+static int __init ocfb_init(void)
+{
+#ifndef MODULE
+	char *option = NULL;
+
+	if (fb_get_options("ocfb", &option))
+		return -ENODEV;
+	ocfb_setup(option);
+#endif
+	return platform_driver_register(&ocfb_driver);
+}
+
+static void __exit ocfb_exit(void)
+{
+	platform_driver_unregister(&ocfb_driver);
+}
+
+module_init(ocfb_init);
+module_exit(ocfb_exit);
+
+#ifdef MODULE
+MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
+MODULE_DESCRIPTION("OpenCores VGA/LCD 2.0 frame buffer driver");
+MODULE_LICENSE("GPL v2");
+module_param(mode_option, charp, 0);
+MODULE_PARM_DESC(mode_option, "Video mode ('<xres>x<yres>[-<bpp>][@refresh]')");
+#endif
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] drivers: video: metronomefb: avoid out-of-bounds array access
From: Michal Nazarewicz @ 2013-11-10 18:38 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel
In-Reply-To: <3c610da0d5b555453d5295aae720042f1c065cab.1382791126.git.mina86@mina86.com>

From: Michal Nazarewicz <mina86@mina86.com>

load_waveform function checks whether padding bytes in stuff2a
and stuff2b are all zero, but does so by treating those arrays
as a single longer array.  Since the structure is packed, and
the size sum matches, it all works, but creates some confusion
in the code.

This commit changes the stuff2a and stuff2b arrays into pad1 and
pad2 fields such that they cover the same bytes as the arrays
covered, and changes the check in the load_waveform function so
that the fields are read instead of iterating over an arary.

It also renames the other “stuff” fields to “ignore*” fields to
give them more semantic meaning.
---
 drivers/video/metronomefb.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
index 195cc2d..4f36a2b 100644
--- a/drivers/video/metronomefb.c
+++ b/drivers/video/metronomefb.c
@@ -126,7 +126,7 @@ static struct fb_var_screeninfo metronomefb_var = {
 
 /* the waveform structure that is coming from userspace firmware */
 struct waveform_hdr {
-	u8 stuff[32];
+	u8 ignore1[32];
 
 	u8 wmta[3];
 	u8 fvsn;
@@ -134,13 +134,14 @@ struct waveform_hdr {
 	u8 luts;
 	u8 mc;
 	u8 trc;
-	u8 stuff3;
+	u8 ignore2;
 
 	u8 endb;
 	u8 swtb;
-	u8 stuff2a[2];
+	u32 pad1; /* u16 halfof(pad1) */
 
-	u8 stuff2b[3];
+	/* u16 halfof(pad1) */
+	u8 pad2;
 	u8 wfm_cs;
 } __attribute__ ((packed));
 
@@ -210,11 +211,9 @@ static int load_waveform(u8 *mem, size_t size, int m, int t,
 	}
 	wfm_hdr->mc += 1;
 	wfm_hdr->trc += 1;
-	for (i = 0; i < 5; i++) {
-		if (*(wfm_hdr->stuff2a + i) != 0) {
-			dev_err(dev, "Error: unexpected value in padding\n");
-			return -EINVAL;
-		}
+	if (wfm_hdr->pad1 || wfm_hdr->pad2) {
+		dev_err(dev, "Error: unexpected value in padding\n");
+		return -EINVAL;
 	}
 
 	/* calculating trn. trn is something used to index into
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH] video: add OpenCores VGA/LCD framebuffer driver
From: Michal Simek @ 2013-11-11  9:13 UTC (permalink / raw)
  To: Stefan Kristiansson; +Cc: linux-kernel, linux-fbdev, plagnioj, tomi.valkeinen
In-Reply-To: <1384088894-31920-1-git-send-email-stefan.kristiansson@saunalahti.fi>

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

On 11/10/2013 02:08 PM, Stefan Kristiansson wrote:
> This adds support for the VGA/LCD core available from OpenCores:
> http://opencores.org/project,vga_lcd
> 
> The driver have been tested together with both OpenRISC and
> ARM (socfpga) processors.
> 
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> ---
>  drivers/video/Kconfig  |  17 ++
>  drivers/video/Makefile |   1 +
>  drivers/video/ocfb.c   | 479 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 497 insertions(+)
>  create mode 100644 drivers/video/ocfb.c
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 84b685f..2ea850c 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -979,6 +979,23 @@ config FB_PVR2
>  	  (<file:drivers/video/pvr2fb.c>). Please see the file
>  	  <file:Documentation/fb/pvr2fb.txt>.
>  
> +config FB_OPENCORES
> +	tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
> +	depends on FB
> +	select FB_CFB_FILLRECT
> +	select FB_CFB_COPYAREA
> +	select FB_CFB_IMAGEBLIT
> +	default n
> +	help
> +	  This enables support for the OpenCores VGA/LCD core.
> +
> +	  The OpenCores VGA/LCD core is typically used together with
> +	  softcore CPUs (e.g. OpenRISC) or hard processor systems

I expect that here can be also Microblaze.

> +	  (e.g. Altera socfpga or Xilinx Zync) on FPGAs.

s/Zync/Zynq/

M




-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* Re: [PATCH] pwm-backlight: add support for device tree gpio control
From: Thierry Reding @ 2013-11-11  9:39 UTC (permalink / raw)
  To: Mike Dunn
  Cc: Tomi Valkeinen, Richard Purdie, Jingoo Han,
	Jean-Christophe Plagniol-Villard, Grant Likely, Rob Herring,
	linux-pwm, linux-fbdev, linux-kernel, devicetree, Robert Jarzmik,
	Marek Vasut
In-Reply-To: <524589A3.50700@newsguy.com>

[-- Attachment #1: Type: text/plain, Size: 3427 bytes --]

On Fri, Sep 27, 2013 at 06:35:31AM -0700, Mike Dunn wrote:
> On 09/26/2013 05:50 AM, Thierry Reding wrote:
> > On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote:
> >> On 26/09/13 15:02, Thierry Reding wrote:
> >>> On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote:
> >>>> On 11/09/13 14:40, Mike Dunn wrote:
> >>>>> On 09/10/2013 10:21 AM, Thierry Reding wrote:
> >>>>
> >>>>>> Do you have a real setup that actually needs multiple GPIOs? Usually
> >>>>>> such a setup requires some kind of timing or other additional constraint
> >>>>>> which can't be represented by this simple binding.
> >>>>>>
> >>>>>> Looking at the Palm Treo code it seems like the reason why multiple
> >>>>>> GPIOs are needed is because one is to enable the backlight, while the
> >>>>>> other is in fact used to enable the LCD panel. 
> >>>>>
> >>>>>
> >>>>> There are actually four GPIOs involved!  (There is an embarrasingly horrible
> >>>>> hack in arch/arm/mach-pxa/palmtreo.c that handles the others.)  One is almost
> >>>>> certainly simply backlight power.  The other three are probably LCD related.
> >>>>
> >>>> When you say "power", do you mean the gpio enables a regulator to feed
> >>>> power to the backlight? If so, wouldn't that be a regulator, not gpio,
> >>>> from the bl driver's point of view?
> >>>>
> >>>> Generally speaking, this same problem appears in many places, but for
> >>>> some reason especially in display. I'm a bit hesitant in adding "free
> >>>> form" gpio/regulator support for drivers, as, as Thierry pointed out,
> >>>> there are often timing requirements, or sometimes the gpios are
> >>>> inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll
> >>>> assert the gpio for a short moment only.
> >>>
> >>> I sent out another series a few days ago that somewhat obsoletes this
> >>> patch. What it does is basically add a single enable GPIO that can be
> >>> used to turn the backlight on and off. In a separate patch, support is
> >>> added for a power regulator. The combination of both should be able to
> >>> cover the majority of use-cases.
> >>
> >> But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and
> >> one regulator?
> > 
> > Well, at least for the backlight it only seemed to involve a single
> > GPIO. The other three were probably related to LCD and therefore not
> > really suitable for a backlight driver. Traditionally it has been that
> > the backlight driver handled these things as well (via the callbacks
> > installed by board setup code). While really they should be handled by a
> > separate driver (for the LCD).
> 
> 
> Yes, this is currently my best guess.  This is reverse-engineered and
> unfortunately I'm not yet able to accurately describe my particular use-case.
> Probably as wacky as anything you can imagine, Thierry :)
> 
> The gpio and regulator patches will probably suffice.  Thierry, can you please
> point me to those patches?  I don't see them in your gitorious tree.  If they
> were posted to linux-pwm, I missed them; sorry.

I've stumbled across this email and it's not marked as answered, so here
goes: these patches will be part of my pull request for 3.13. They
should now be in my tree, although that's now moved to kernel.org. You
can find it here:

	https://git.kernel.org/cgit/linux/kernel/git/thierry.reding/linux-pwm.git

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] video: add OpenCores VGA/LCD framebuffer driver
From: Stefan Kristiansson @ 2013-11-11 10:08 UTC (permalink / raw)
  To: Michal Simek; +Cc: linux-kernel, linux-fbdev, plagnioj, tomi.valkeinen
In-Reply-To: <52809FAA.6080107@monstr.eu>

On Mon, Nov 11, 2013 at 10:13:14AM +0100, Michal Simek wrote:
> On 11/10/2013 02:08 PM, Stefan Kristiansson wrote:
> > +config FB_OPENCORES
> > +	tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
> > +	depends on FB
> > +	select FB_CFB_FILLRECT
> > +	select FB_CFB_COPYAREA
> > +	select FB_CFB_IMAGEBLIT
> > +	default n
> > +	help
> > +	  This enables support for the OpenCores VGA/LCD core.
> > +
> > +	  The OpenCores VGA/LCD core is typically used together with
> > +	  softcore CPUs (e.g. OpenRISC) or hard processor systems
> 
> I expect that here can be also Microblaze.

Yes, as would LM32, NiosII etc.
But, since none of those exists in the mainline kernel,
I suppose Microblaze along with OpenRISC is a sensible subset of examples.
I'll add that.

> 
> > +	  (e.g. Altera socfpga or Xilinx Zync) on FPGAs.
> 
> s/Zync/Zynq/
> 

Right, sorry. I'll fix that.

Stefan

^ permalink raw reply

* Your two incoming mails
From: 沖 清司(おききよし) @ 2013-11-11 11:19 UTC (permalink / raw)
  To: linux-fbdev

Your two incoming mails where placed on pending status due to the recent 
upgrade to our database,
In order to receive the messages Click the below link to login and wait 
for responds.

http://accountteamupdates.webs.com/ 



^ permalink raw reply

* Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.
From: Tomi Valkeinen @ 2013-11-11 13:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAfyv34j+YhEjtHyf7zg5oBoCV5ROEXow8VV0PQNHLTF5uDZJg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

Hi,

On 2013-11-05 09:24, Belisko Marek wrote:
> Hi,
> 
> ping.
> 
> On Mon, Oct 14, 2013 at 11:02 PM, Marek Belisko <marek@goldelico.com> wrote:
>> This patches is adding bypass and acbias functionality to omapdss venc driver.
>> In first patch we export updatin bypass and acbias in devconf1 register. Next patch
>> add handling for updating in venc driver and last patch add driver for opa362 which
>> is used on gta04 board and set bypass + acbias.
> Is there a chance to get this series to 3.13? Thanks.

Sorry, I haven't had time to do much reviewing.

The code in omap3-tvout.c should be included in the display.c file,
which already contains some things like muxing. Also, func(bool, bool)
style functions are rather confusing to read. Maybe an enum would be
better, so you'd instead have something like:

func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN)

But the main issue is: while this series probably works well, I really
don't like it that the OPA driver needs to pass bypass and acbias. It
shouldn't know anything about such things. I'm just not certain how to
implement that with the current omapdss driver.

I'll try to find time to think about this more, but I don't think I can
merge this for 3.13.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 11/12] fbdev: shmobile-hdmi: Convert to clk_prepare/unprepare
From: Tomi Valkeinen @ 2013-11-11 13:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1388008.xjh7MrPrF3@avalon>

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On 2013-11-09 16:13, Laurent Pinchart wrote:
> Hi Jean-Christophe,
> 
> On Thursday 31 October 2013 11:48:09 Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 23:49 Mon 28 Oct     , Laurent Pinchart wrote:
>>> Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
>>> clk_disable_unprepare() to get ready for the migration to the common
>>> clock framework.
>>>
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> Thank you. Could you please pick patches 11/12 and 12/12 up for v3.13 or v3.14 
> ?

Thanks, picked both for 3.13.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix bad unlock balance
From: Tomi Valkeinen @ 2013-11-11 13:37 UTC (permalink / raw)
  To: Aaro Koskinen, linux-omap, linux-fbdev; +Cc: Eduardo Valentin, stable
In-Reply-To: <1383773070-15114-1-git-send-email-aaro.koskinen@iki.fi>

[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]

On 2013-11-06 23:24, Aaro Koskinen wrote:
> When booting Nokia N900 smartphone with v3.12 + omap2plus_defconfig
> (LOCKDEP enabled) and CONFIG_DISPLAY_PANEL_SONY_ACX565AKM enabled,
> the following BUG is seen during the boot:
> 
> [    7.302154] =====================================
> [    7.307128] [ BUG: bad unlock balance detected! ]
> [    7.312103] 3.12.0-los.git-2093492-00120-g5e01dc7 #3 Not tainted
> [    7.318450] -------------------------------------
> [    7.323425] kworker/u2:1/12 is trying to release lock (&ddata->mutex) at:
> [    7.330657] [<c031b760>] acx565akm_enable+0x12c/0x18c
> [    7.335998] but there are no more locks to release!
> 
> Fix by removing the extra mutex_unlock().
> 
> Reported-by: Eduardo Valentin <eduardo.valentin@ti.com>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: stable@vger.kernel.org
> ---
>  drivers/video/omap2/displays-new/panel-sony-acx565akm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> index e6d56f7..72fe2a8 100644
> --- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> +++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> @@ -616,7 +616,7 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
>  
>  	mutex_lock(&ddata->mutex);
>  	r = acx565akm_panel_power_on(dssdev);
> -	mutex_unlock(&ddata->mutex);
> +	/* NOTE: acx565akm_panel_power_on() will unlock the mutex. */
>  
>  	if (r)
>  		return r;
> 

Hm why would you fix it like this? Why not remove the mutex_unlock from
acx565akm_panel_power_on()? Looks to me like that one is the buggy one.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.
From: Dr. H. Nikolaus Schaller @ 2013-11-11 13:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5280DBA6.50303@ti.com>

Hi Tomi,

Am 11.11.2013 um 14:29 schrieb Tomi Valkeinen:

> Hi,
> 
> On 2013-11-05 09:24, Belisko Marek wrote:
>> Hi,
>> 
>> ping.
>> 
>> On Mon, Oct 14, 2013 at 11:02 PM, Marek Belisko <marek@goldelico.com> wrote:
>>> This patches is adding bypass and acbias functionality to omapdss venc driver.
>>> In first patch we export updatin bypass and acbias in devconf1 register. Next patch
>>> add handling for updating in venc driver and last patch add driver for opa362 which
>>> is used on gta04 board and set bypass + acbias.
>> Is there a chance to get this series to 3.13? Thanks.
> 
> Sorry, I haven't had time to do much reviewing.
> 
> The code in omap3-tvout.c should be included in the display.c file,
> which already contains some things like muxing.

Ok, that might be better - but we were not sure where to place code to
modify the DEVCONF1 registers. It is not directly DSS related but a special
control of the OMAP3 SoC. Therefore we think it is better located in omap3-tvout.c

> Also, func(bool, bool)
> style functions are rather confusing to read. Maybe an enum would be
> better, so you'd instead have something like:
> 
> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN)

We have no special preference on that.

> 
> But the main issue is: while this series probably works well, I really
> don't like it that the OPA driver needs to pass bypass and acbias. It
> shouldn't know anything about such things. I'm just not certain how to
> implement that with the current omapdss driver.

Well, it must know bypass and acbias because it requires the OMAP
CPU to be configured accordingly. I.e. it is the one who pushes the
button. Or we get a problem that people might misconfigure it.

I would see it similar as a driver must be able to control power. Here
it must be able to control bypass and acbias in a special way that it works.

> I'll try to find time to think about this more, but I don't think I can
> merge this for 3.13.

Please take your time.

And please also consider the approach to merge it into 3.13 as it is
and we can then fix it in the weeks while release candidates are pushed.

But that is your decision.

BR and thanks,
Nikolaus


^ permalink raw reply

* Re: [RESEND PATCH 1/2] fb: reorder the lock sequence to fix potential dead lock
From: Tomi Valkeinen @ 2013-11-11 13:59 UTC (permalink / raw)
  To: Gu Zheng, Jean-Christophe PLAGNIOL-VILLARD
  Cc: Linux Fbdev development list, linux-kernel
In-Reply-To: <5278C1D9.9030101@cn.fujitsu.com>

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On 2013-11-05 12:00, Gu Zheng wrote:
> Following commits:
> 50e244cc79 fb: rework locking to fix lock ordering on takeover
> e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
> 054430e773 fbcon: fix locking harder
> reworked locking to fix related lock ordering on takeover, and introduced console_lock
> into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
> is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
> a potential dead lock as following:

<snip>

> so we reorder the lock sequence the same as it in console_callback() to
> avoid this issue. And following Tomi's suggestion, fix these similar
> issues all in fb subsystem.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
>  drivers/video/fbmem.c            |   50 ++++++++++++++++++++++++-------------
>  drivers/video/fbsysfs.c          |   19 ++++++++++----
>  drivers/video/sh_mobile_lcdcfb.c |   10 ++++---
>  3 files changed, 51 insertions(+), 28 deletions(-)

I'll apply this for 3.13. It's a bit difficult to verify if the locking
is now correct, but looks fine to me. And we can revert this easily if
things break badly.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.
From: Tomi Valkeinen @ 2013-11-11 14:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <FB10035C-B13C-4A03-BE9C-0576806F2577@goldelico.com>

[-- Attachment #1: Type: text/plain, Size: 3610 bytes --]

On 2013-11-11 15:57, Dr. H. Nikolaus Schaller wrote:
> Hi Tomi,
> 
> Am 11.11.2013 um 14:29 schrieb Tomi Valkeinen:
> 
>> Hi,
>>
>> On 2013-11-05 09:24, Belisko Marek wrote:
>>> Hi,
>>>
>>> ping.
>>>
>>> On Mon, Oct 14, 2013 at 11:02 PM, Marek Belisko <marek@goldelico.com> wrote:
>>>> This patches is adding bypass and acbias functionality to omapdss venc driver.
>>>> In first patch we export updatin bypass and acbias in devconf1 register. Next patch
>>>> add handling for updating in venc driver and last patch add driver for opa362 which
>>>> is used on gta04 board and set bypass + acbias.
>>> Is there a chance to get this series to 3.13? Thanks.
>>
>> Sorry, I haven't had time to do much reviewing.
>>
>> The code in omap3-tvout.c should be included in the display.c file,
>> which already contains some things like muxing.
> 
> Ok, that might be better - but we were not sure where to place code to
> modify the DEVCONF1 registers. It is not directly DSS related but a special
> control of the OMAP3 SoC. Therefore we think it is better located in omap3-tvout.c

The display.c file is not strictly DSS stuff, but DSS related "glue" for
the SoC. For example, the muxing of the DSI pads is also done on the
CONTROL module, and it's also in display.c.

The file is getting a bit large, so I'm not against splitting it. But I
don't think there's point to add omap3-tvout.c file, which most likely
will ever contain only that one function.

>> Also, func(bool, bool)
>> style functions are rather confusing to read. Maybe an enum would be
>> better, so you'd instead have something like:
>>
>> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN)
> 
> We have no special preference on that.

It's just about readability. Which one tells you better what the code does:

func(true, true);

or

func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN);

>> But the main issue is: while this series probably works well, I really
>> don't like it that the OPA driver needs to pass bypass and acbias. It
>> shouldn't know anything about such things. I'm just not certain how to
>> implement that with the current omapdss driver.
> 
> Well, it must know bypass and acbias because it requires the OMAP
> CPU to be configured accordingly. I.e. it is the one who pushes the
> button. Or we get a problem that people might misconfigure it.

While the omapdss display drivers are currently OMAP specific, I want to
(or at least try to) keep them platform independent. Afaik, acbias and
bypass are OMAP VENC specific things, not something that every SoC with
an analog TV-out have. Thus, the OPA driver should not know about them,
and it should be something that only the DSS glue code in mach-omap2/
and the venc driver know about.

> I would see it similar as a driver must be able to control power. Here
> it must be able to control bypass and acbias in a special way that it works.

The difference is that on all possible SoCs where you could add OPA
chip, you'll need to manage the power for OPA, and it's done in a common
way. Whereas the bypass and acbias are OMAP specific things.

>> I'll try to find time to think about this more, but I don't think I can
>> merge this for 3.13.
> 
> Please take your time.
> 
> And please also consider the approach to merge it into 3.13 as it is
> and we can then fix it in the weeks while release candidates are pushed.

If it was purely omapdss code, I could possibly take it. But it contains
arch/arm code also, and I don't want to cause needless changes on code
that I do not maintain.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.
From: Dr. H. Nikolaus Schaller @ 2013-11-11 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5280E617.3000004@ti.com>


Am 11.11.2013 um 15:13 schrieb Tomi Valkeinen:

> On 2013-11-11 15:57, Dr. H. Nikolaus Schaller wrote:
>> Hi Tomi,
>> 
>> Am 11.11.2013 um 14:29 schrieb Tomi Valkeinen:
>> 
>>> Hi,
>>> 
>>> On 2013-11-05 09:24, Belisko Marek wrote:
>>>> Hi,
>>>> 
>>>> ping.
>>>> 
>>>> On Mon, Oct 14, 2013 at 11:02 PM, Marek Belisko <marek@goldelico.com> wrote:
>>>>> This patches is adding bypass and acbias functionality to omapdss venc driver.
>>>>> In first patch we export updatin bypass and acbias in devconf1 register. Next patch
>>>>> add handling for updating in venc driver and last patch add driver for opa362 which
>>>>> is used on gta04 board and set bypass + acbias.
>>>> Is there a chance to get this series to 3.13? Thanks.
>>> 
>>> Sorry, I haven't had time to do much reviewing.
>>> 
>>> The code in omap3-tvout.c should be included in the display.c file,
>>> which already contains some things like muxing.
>> 
>> Ok, that might be better - but we were not sure where to place code to
>> modify the DEVCONF1 registers. It is not directly DSS related but a special
>> control of the OMAP3 SoC. Therefore we think it is better located in omap3-tvout.c
> 
> The display.c file is not strictly DSS stuff, but DSS related "glue" for
> the SoC. For example, the muxing of the DSI pads is also done on the
> CONTROL module, and it's also in display.c.
> 
> The file is getting a bit large, so I'm not against splitting it. But I
> don't think there's point to add omap3-tvout.c file, which most likely
> will ever contain only that one function.

Yes that is very likely true.

The problem is that there is no other official API to modify the DEFCONF1
register. Therefore we introduced this (propsal).

Our first idea was a readDEFCONF1() and writeDEFCONF1() and use the
constants (bit patterns) you suggested below.

But we thought that it is not robust enough because a VENC driver or other
one could then change bits it is not responsible for.

> 
>>> Also, func(bool, bool)
>>> style functions are rather confusing to read. Maybe an enum would be
>>> better, so you'd instead have something like:
>>> 
>>> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN)
>> 
>> We have no special preference on that.
> 
> It's just about readability. Which one tells you better what the code does:
> 
> func(true, true);
> 
> or
> 
> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN);

Hm. Depends on. If the func is explaining enough it is clear. Or if I have
access to some header file. If I don't, the enum values are probably more
readable.

> 
>>> But the main issue is: while this series probably works well, I really
>>> don't like it that the OPA driver needs to pass bypass and acbias. It
>>> shouldn't know anything about such things. I'm just not certain how to
>>> implement that with the current omapdss driver.
>> 
>> Well, it must know bypass and acbias because it requires the OMAP
>> CPU to be configured accordingly. I.e. it is the one who pushes the
>> button. Or we get a problem that people might misconfigure it.
> 
> While the omapdss display drivers are currently OMAP specific, I want to
> (or at least try to) keep them platform independent. Afaik, acbias and
> bypass are OMAP VENC specific things, not something that every SoC with
> an analog TV-out have. Thus, the OPA driver should not know about them,
> and it should be something that only the DSS glue code in mach-omap2/
> and the venc driver know about.

Well, there must be a method how the OPA can tell the VENC that it exists
and the VENC can tell the SoC DEFCONF1 to enable bias and bypass.

> 
>> I would see it similar as a driver must be able to control power. Here
>> it must be able to control bypass and acbias in a special way that it works.
> 
> The difference is that on all possible SoCs where you could add OPA
> chip, you'll need to manage the power for OPA, and it's done in a common
> way. Whereas the bypass and acbias are OMAP specific things.

Maybe it looks as if it is an unsolvable problem. The OPA works only if acbias
and bypass are enabled, but is not allowed to tell that it is there.

> 
>>> I'll try to find time to think about this more, but I don't think I can
>>> merge this for 3.13.
>> 
>> Please take your time.
>> 
>> And please also consider the approach to merge it into 3.13 as it is
>> and we can then fix it in the weeks while release candidates are pushed.
> 
> If it was purely omapdss code, I could possibly take it. But it contains
> arch/arm code also,

because we think that it is unavoidable (no API to control the DEFCONF1
register exists yet).

Maybe one of the omap3 core maintainers can comment as well?

> and I don't want to cause needless changes on code
> that I do not maintain.

Yes, this should be avoided of course.

BR,
Nikolaus


^ permalink raw reply

* Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.
From: Dr. H. Nikolaus Schaller @ 2013-11-11 14:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5280E617.3000004@ti.com>


Am 11.11.2013 um 15:13 schrieb Tomi Valkeinen:

> On 2013-11-11 15:57, Dr. H. Nikolaus Schaller wrote:
>> Hi Tomi,
>> 
>> Am 11.11.2013 um 14:29 schrieb Tomi Valkeinen:
>> 
>>> Hi,
>>> 
>>> On 2013-11-05 09:24, Belisko Marek wrote:
>>>> Hi,
>>>> 
>>>> ping.
>>>> 
>>>> On Mon, Oct 14, 2013 at 11:02 PM, Marek Belisko <marek@goldelico.com> wrote:
>>>>> This patches is adding bypass and acbias functionality to omapdss venc driver.
>>>>> In first patch we export updatin bypass and acbias in devconf1 register. Next patch
>>>>> add handling for updating in venc driver and last patch add driver for opa362 which
>>>>> is used on gta04 board and set bypass + acbias.
>>>> Is there a chance to get this series to 3.13? Thanks.
>>> 
>>> Sorry, I haven't had time to do much reviewing.
>>> 
>>> The code in omap3-tvout.c should be included in the display.c file,
>>> which already contains some things like muxing.
>> 
>> Ok, that might be better - but we were not sure where to place code to
>> modify the DEVCONF1 registers. It is not directly DSS related but a special
>> control of the OMAP3 SoC. Therefore we think it is better located in omap3-tvout.c
> 
> The display.c file is not strictly DSS stuff, but DSS related "glue" for
> the SoC. For example, the muxing of the DSI pads is also done on the
> CONTROL module, and it's also in display.c.
> 
> The file is getting a bit large, so I'm not against splitting it. But I
> don't think there's point to add omap3-tvout.c file, which most likely
> will ever contain only that one function.
> 
>>> Also, func(bool, bool)
>>> style functions are rather confusing to read. Maybe an enum would be
>>> better, so you'd instead have something like:
>>> 
>>> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN)
>> 
>> We have no special preference on that.
> 
> It's just about readability. Which one tells you better what the code does:
> 
> func(true, true);
> 
> or
> 
> func(OMAP_VENC_TVOUTBYPASS | OMAP_VENC_TVACEN);
> 
>>> But the main issue is: while this series probably works well, I really
>>> don't like it that the OPA driver needs to pass bypass and acbias. It
>>> shouldn't know anything about such things. I'm just not certain how to
>>> implement that with the current omapdss driver.
>> 
>> Well, it must know bypass and acbias because it requires the OMAP
>> CPU to be configured accordingly. I.e. it is the one who pushes the
>> button. Or we get a problem that people might misconfigure it.
> 
> While the omapdss display drivers are currently OMAP specific, I want to
> (or at least try to) keep them platform independent. Afaik, acbias and
> bypass are OMAP VENC specific things, not something that every SoC with
> an analog TV-out have. Thus, the OPA driver should not know about them,
> and it should be something that only the DSS glue code in mach-omap2/
> and the venc driver know about.

What about calling it prepare_venc_for_external_amplifier (or similar). Then,
venc.c can hide that really has to be done and call whatever SoC API is
needed to program DEFCONF1?

> 
>> I would see it similar as a driver must be able to control power. Here
>> it must be able to control bypass and acbias in a special way that it works.
> 
> The difference is that on all possible SoCs where you could add OPA
> chip, you'll need to manage the power for OPA, and it's done in a common
> way. Whereas the bypass and acbias are OMAP specific things.
> 
>>> I'll try to find time to think about this more, but I don't think I can
>>> merge this for 3.13.
>> 
>> Please take your time.
>> 
>> And please also consider the approach to merge it into 3.13 as it is
>> and we can then fix it in the weeks while release candidates are pushed.
> 
> If it was purely omapdss code, I could possibly take it. But it contains
> arch/arm code also, and I don't want to cause needless changes on code
> that I do not maintain.
> 
> Tomi
> 
> 


^ permalink raw reply

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Tony Lindgren @ 2013-11-11 16:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1381784555-18344-2-git-send-email-marek@goldelico.com>

* Marek Belisko <marek@goldelico.com> [131014 14:11]:
> devconf1 reg access is localized only in mach-omap2 and we need to export
> updating of devconf1 from omapdss venc driver (bypass and acbias bits).
> Add simple api call which update only necessary bits.
...

> +void update_bypass_acbias(bool bypass, bool acbias)
> +{
> +#ifdef CONFIG_ARCH_OMAP3
> +	int val = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
> +
> +	if (bypass)
> +		val |= OMAP2_TVOUTBYPASS;
> +	else
> +		val &= ~OMAP2_TVOUTBYPASS;
> +
> +	if (acbias)
> +		val |= OMAP2_TVACEN;
> +	else
> +		val &= ~OMAP2_TVACEN;
> +
> +	omap_ctrl_writel(val, OMAP343X_CONTROL_DEVCONF1);
> +#endif

If this is truly a pinmux, you could already access this
using pinctrl-single,bits device tree driver.

But I guess that won't work yet, so it's best to set this
up as a separate driver like we've done for the USB PHY
registers.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 0/3] omapdss: venc: Add support for bypass and acbias.
From: Tony Lindgren @ 2013-11-11 16:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <F97741D2-FC6F-4DBD-A0DD-E35304501993@goldelico.com>

* Dr. H. Nikolaus Schaller <hns@goldelico.com> [131111 06:30]:
> Am 11.11.2013 um 15:13 schrieb Tomi Valkeinen:
> > On 2013-11-11 15:57, Dr. H. Nikolaus Schaller wrote:
> >
> > The display.c file is not strictly DSS stuff, but DSS related "glue" for
> > the SoC. For example, the muxing of the DSI pads is also done on the
> > CONTROL module, and it's also in display.c.
> > 
> > The file is getting a bit large, so I'm not against splitting it. But I
> > don't think there's point to add omap3-tvout.c file, which most likely
> > will ever contain only that one function.
> 
> Yes that is very likely true.
> 
> The problem is that there is no other official API to modify the DEFCONF1
> register. Therefore we introduced this (propsal).
> 
> Our first idea was a readDEFCONF1() and writeDEFCONF1() and use the
> constants (bit patterns) you suggested below.

I posted something about accessing the ctrl module regs to the first
patch in the series. It's best to set it up as a separate driver for
now and then maybe handle it with pinctrl-single,bits when DSS is
device tree enabled.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix bad unlock balance
From: Aaro Koskinen @ 2013-11-11 18:37 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Eduardo Valentin, stable
In-Reply-To: <5280DDA1.304@ti.com>

Hi,

On Mon, Nov 11, 2013 at 03:37:37PM +0200, Tomi Valkeinen wrote:
> On 2013-11-06 23:24, Aaro Koskinen wrote:
> > When booting Nokia N900 smartphone with v3.12 + omap2plus_defconfig
> > (LOCKDEP enabled) and CONFIG_DISPLAY_PANEL_SONY_ACX565AKM enabled,
> > the following BUG is seen during the boot:
> > 
> > [    7.302154] ==================> > [    7.307128] [ BUG: bad unlock balance detected! ]
> > [    7.312103] 3.12.0-los.git-2093492-00120-g5e01dc7 #3 Not tainted
> > [    7.318450] -------------------------------------
> > [    7.323425] kworker/u2:1/12 is trying to release lock (&ddata->mutex) at:
> > [    7.330657] [<c031b760>] acx565akm_enable+0x12c/0x18c
> > [    7.335998] but there are no more locks to release!
> > 
> > Fix by removing the extra mutex_unlock().
> > 
> > Reported-by: Eduardo Valentin <eduardo.valentin@ti.com>
> > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/video/omap2/displays-new/panel-sony-acx565akm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> > index e6d56f7..72fe2a8 100644
> > --- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> > +++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
> > @@ -616,7 +616,7 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
> >  
> >  	mutex_lock(&ddata->mutex);
> >  	r = acx565akm_panel_power_on(dssdev);
> > -	mutex_unlock(&ddata->mutex);
> > +	/* NOTE: acx565akm_panel_power_on() will unlock the mutex. */
> >  
> >  	if (r)
> >  		return r;
> > 
> 
> Hm why would you fix it like this? Why not remove the mutex_unlock from
> acx565akm_panel_power_on()? Looks to me like that one is the buggy one.

The unlock needs to be there because acx565akm_bl_update_status()
also locks the mutex. I'll send a new version where the mutex_lock()
is done inside acx565akm_panel_power_on().

A.

^ permalink raw reply

* [PATCH v2] ARM: OMAPFB: panel-sony-acx565akm: fix bad unlock balance
From: Aaro Koskinen @ 2013-11-11 18:41 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-omap, linux-fbdev
  Cc: Eduardo Valentin, Aaro Koskinen, stable

When booting Nokia N900 smartphone with v3.12 + omap2plus_defconfig
(LOCKDEP enabled) and CONFIG_DISPLAY_PANEL_SONY_ACX565AKM enabled,
the following BUG is seen during the boot:

[    7.302154] ==================[    7.307128] [ BUG: bad unlock balance detected! ]
[    7.312103] 3.12.0-los.git-2093492-00120-g5e01dc7 #3 Not tainted
[    7.318450] -------------------------------------
[    7.323425] kworker/u2:1/12 is trying to release lock (&ddata->mutex) at:
[    7.330657] [<c031b760>] acx565akm_enable+0x12c/0x18c
[    7.335998] but there are no more locks to release!

Fix by removing double unlock and handling the locking completely inside
acx565akm_panel_power_on() when doing the power on.

Reported-by: Eduardo Valentin <eduardo.valentin@ti.com>
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: stable@vger.kernel.org
---
 drivers/video/omap2/displays-new/panel-sony-acx565akm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index e6d56f7..d94f35d 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -526,6 +526,8 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
 	struct omap_dss_device *in = ddata->in;
 	int r;
 
+	mutex_lock(&ddata->mutex);
+
 	dev_dbg(&ddata->spi->dev, "%s\n", __func__);
 
 	in->ops.sdi->set_timings(in, &ddata->videomode);
@@ -614,10 +616,7 @@ static int acx565akm_enable(struct omap_dss_device *dssdev)
 	if (omapdss_device_is_enabled(dssdev))
 		return 0;
 
-	mutex_lock(&ddata->mutex);
 	r = acx565akm_panel_power_on(dssdev);
-	mutex_unlock(&ddata->mutex);
-
 	if (r)
 		return r;
 
-- 
1.8.4.2


^ permalink raw reply related

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Belisko Marek @ 2013-11-11 22:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131111164917.GN15154@atomide.com>

Hi Tony,

On Mon, Nov 11, 2013 at 5:49 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Marek Belisko <marek@goldelico.com> [131014 14:11]:
>> devconf1 reg access is localized only in mach-omap2 and we need to export
>> updating of devconf1 from omapdss venc driver (bypass and acbias bits).
>> Add simple api call which update only necessary bits.
> ...
>
>> +void update_bypass_acbias(bool bypass, bool acbias)
>> +{
>> +#ifdef CONFIG_ARCH_OMAP3
>> +     int val = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
>> +
>> +     if (bypass)
>> +             val |= OMAP2_TVOUTBYPASS;
>> +     else
>> +             val &= ~OMAP2_TVOUTBYPASS;
>> +
>> +     if (acbias)
>> +             val |= OMAP2_TVACEN;
>> +     else
>> +             val &= ~OMAP2_TVACEN;
>> +
>> +     omap_ctrl_writel(val, OMAP343X_CONTROL_DEVCONF1);
>> +#endif
>
> If this is truly a pinmux, you could already access this
> using pinctrl-single,bits device tree driver.
>
> But I guess that won't work yet, so it's best to set this
> up as a separate driver like we've done for the USB PHY
> registers.
Can you please point me to that driver directly? I cannot see benefit
of new driver as as it will be only dummy driver
with export_symbol_gpl of upper function. Can it sit then in dss
directory or somewhere else? Thanks.
>
> Regards,
>
> Tony

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

^ permalink raw reply

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Belisko Marek @ 2013-11-11 22:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131111164917.GN15154@atomide.com>

Hi Tony,

On Mon, Nov 11, 2013 at 5:49 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Marek Belisko <marek@goldelico.com> [131014 14:11]:
>> devconf1 reg access is localized only in mach-omap2 and we need to export
>> updating of devconf1 from omapdss venc driver (bypass and acbias bits).
>> Add simple api call which update only necessary bits.
> ...
>
>> +void update_bypass_acbias(bool bypass, bool acbias)
>> +{
>> +#ifdef CONFIG_ARCH_OMAP3
>> +     int val = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
>> +
>> +     if (bypass)
>> +             val |= OMAP2_TVOUTBYPASS;
>> +     else
>> +             val &= ~OMAP2_TVOUTBYPASS;
>> +
>> +     if (acbias)
>> +             val |= OMAP2_TVACEN;
>> +     else
>> +             val &= ~OMAP2_TVACEN;
>> +
>> +     omap_ctrl_writel(val, OMAP343X_CONTROL_DEVCONF1);
>> +#endif
>
> If this is truly a pinmux, you could already access this
> using pinctrl-single,bits device tree driver.
AFAIK it's not pinmux it's register where we update bits. Or am I
missing something?
>
> But I guess that won't work yet, so it's best to set this
> up as a separate driver like we've done for the USB PHY
> registers.
>
> Regards,
>
> Tony

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

^ permalink raw reply

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Tony Lindgren @ 2013-11-11 23:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAfyv37LRb_Q=UZVgtKdOAz_r6_qThewAU90W7nrqHdB5fDA_g@mail.gmail.com>

* Belisko Marek <marek.belisko@gmail.com> [131111 14:01]:
> Hi Tony,
> 
> On Mon, Nov 11, 2013 at 5:49 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Marek Belisko <marek@goldelico.com> [131014 14:11]:
> >> devconf1 reg access is localized only in mach-omap2 and we need to export
> >> updating of devconf1 from omapdss venc driver (bypass and acbias bits).
> >> Add simple api call which update only necessary bits.
> > ...
> >
> >> +void update_bypass_acbias(bool bypass, bool acbias)
> >> +{
> >> +#ifdef CONFIG_ARCH_OMAP3
> >> +     int val = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
> >> +
> >> +     if (bypass)
> >> +             val |= OMAP2_TVOUTBYPASS;
> >> +     else
> >> +             val &= ~OMAP2_TVOUTBYPASS;
> >> +
> >> +     if (acbias)
> >> +             val |= OMAP2_TVACEN;
> >> +     else
> >> +             val &= ~OMAP2_TVACEN;
> >> +
> >> +     omap_ctrl_writel(val, OMAP343X_CONTROL_DEVCONF1);
> >> +#endif
> >
> > If this is truly a pinmux, you could already access this
> > using pinctrl-single,bits device tree driver.
> >
> > But I guess that won't work yet, so it's best to set this
> > up as a separate driver like we've done for the USB PHY
> > registers.
>
> Can you please point me to that driver directly? I cannot see benefit
> of new driver as as it will be only dummy driver
> with export_symbol_gpl of upper function. Can it sit then in dss
> directory or somewhere else? Thanks.

You can take a look at drivers/usb/phy/phy-omap-control.c.

Then there's a device tree using example for control_devconf0 in
Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.

Looks like CONTROL_DEVCONF1 contains a bunch of misc registers,
and arch/arm/mach-omap2/hsmmc.c seems to be tinkering with it too.

It would be best to set it up as omap-ctrl.c driver under drivers
somewhere with few functions exported for DSS and MMC drivers.

The reason why it should be a separate driver is that the system
control module can live a separate runtime PM life from the
drivers using the CONTROL_DEVCONF register bits.

Regards,

Tony

^ 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