Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH V2 1/6] ARM: sa1100: add cpu clock
From: Dmitry Eremin-Solenikov @ 2014-11-24 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416870336-30114-1-git-send-email-dbaryshkov@gmail.com>

Both SA1100 framebuffer and PCMCIA drivers require knowledge of cpu
frequency to correctly program timings.  Currently they receive timing
information by calling cpufreq_get(0).  However if cpu frequency driver
is not enabled (e.g. due to unsupported DRAM chip/board on sa1110)
cpufreq_get(0) returns 0, causing incorrect timings to be programmed.

Add cpu clock returning cpu frequency, to be used by sa11x0 fb and
pcmcia drivers.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/clock.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-sa1100/clock.c b/arch/arm/mach-sa1100/clock.c
index 9fa6a99..53f750d 100644
--- a/arch/arm/mach-sa1100/clock.c
+++ b/arch/arm/mach-sa1100/clock.c
@@ -15,10 +15,12 @@
 #include <linux/clkdev.h>
 
 #include <mach/hardware.h>
+#include <mach/generic.h>
 
 struct clkops {
 	void			(*enable)(struct clk *);
 	void			(*disable)(struct clk *);
+	unsigned long		(*get_rate)(struct clk *);
 };
 
 struct clk {
@@ -33,13 +35,6 @@ struct clk clk_##_name = {				\
 
 static DEFINE_SPINLOCK(clocks_lock);
 
-/* Dummy clk routine to build generic kernel parts that may be using them */
-unsigned long clk_get_rate(struct clk *clk)
-{
-	return 0;
-}
-EXPORT_SYMBOL(clk_get_rate);
-
 static void clk_gpio27_enable(struct clk *clk)
 {
 	/*
@@ -58,6 +53,19 @@ static void clk_gpio27_disable(struct clk *clk)
 	GAFR &= ~GPIO_32_768kHz;
 }
 
+static void clk_cpu_enable(struct clk *clk)
+{
+}
+
+static void clk_cpu_disable(struct clk *clk)
+{
+}
+
+static unsigned long clk_cpu_get_rate(struct clk *clk)
+{
+	return sa11x0_getspeed(0) * 1000;
+}
+
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
@@ -87,16 +95,35 @@ void clk_disable(struct clk *clk)
 }
 EXPORT_SYMBOL(clk_disable);
 
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (clk && clk->ops && clk->ops->get_rate)
+		return clk->ops->get_rate(clk);
+
+	return 0;
+}
+EXPORT_SYMBOL(clk_get_rate);
+
 const struct clkops clk_gpio27_ops = {
 	.enable		= clk_gpio27_enable,
 	.disable	= clk_gpio27_disable,
 };
 
+const struct clkops clk_cpu_ops = {
+	.enable		= clk_cpu_enable,
+	.disable	= clk_cpu_disable,
+	.get_rate	= clk_cpu_get_rate,
+};
+
 static DEFINE_CLK(gpio27, &clk_gpio27_ops);
 
+static DEFINE_CLK(cpu, &clk_cpu_ops);
+
 static struct clk_lookup sa11xx_clkregs[] = {
 	CLKDEV_INIT("sa1111.0", NULL, &clk_gpio27),
 	CLKDEV_INIT("sa1100-rtc", NULL, NULL),
+	CLKDEV_INIT("sa11x0-fb", NULL, &clk_cpu),
+	CLKDEV_INIT("sa11x0-pcmcia", NULL, &clk_cpu),
 };
 
 static int __init sa11xx_clk_init(void)
-- 
2.1.3


^ permalink raw reply related

* [PATCH V2 2/6] ARM: sa1100: add a clock alias for sa1111 pcmcia device
From: Dmitry Eremin-Solenikov @ 2014-11-24 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416870336-30114-1-git-send-email-dbaryshkov@gmail.com>

SA-1111 uses internal MMIO space offsets as a device name, so device
name for sa1111 pcmcia is 1800 (PCMCIA is at offset 0x1800).

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/clock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-sa1100/clock.c b/arch/arm/mach-sa1100/clock.c
index 53f750d..03c75a8 100644
--- a/arch/arm/mach-sa1100/clock.c
+++ b/arch/arm/mach-sa1100/clock.c
@@ -124,6 +124,8 @@ static struct clk_lookup sa11xx_clkregs[] = {
 	CLKDEV_INIT("sa1100-rtc", NULL, NULL),
 	CLKDEV_INIT("sa11x0-fb", NULL, &clk_cpu),
 	CLKDEV_INIT("sa11x0-pcmcia", NULL, &clk_cpu),
+	/* sa1111 names devices using internal offsets, PCMCIA is at 0x1800 */
+	CLKDEV_INIT("1800", NULL, &clk_cpu),
 };
 
 static int __init sa11xx_clk_init(void)
-- 
2.1.3


^ permalink raw reply related

* [PATCH V2 3/6] fbdev: sa1100fb: make use of device clock
From: Dmitry Eremin-Solenikov @ 2014-11-24 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416870336-30114-1-git-send-email-dbaryshkov@gmail.com>

Use per-device clock (instead of calling cpufreq_get(0), which can
return 0 if no cpu frequency driver is selected) to program timings.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/fbdev/sa1100fb.c | 30 +++++++++++++++++++++++-------
 drivers/video/fbdev/sa1100fb.h |  1 +
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c
index 580c444e..2133cf1 100644
--- a/drivers/video/fbdev/sa1100fb.c
+++ b/drivers/video/fbdev/sa1100fb.c
@@ -178,6 +178,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/mutex.h>
 #include <linux/io.h>
+#include <linux/clk.h>
 
 #include <video/sa1100fb.h>
 
@@ -413,9 +414,9 @@ sa1100fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 		var->transp.offset);
 
 #ifdef CONFIG_CPU_FREQ
-	dev_dbg(fbi->dev, "dma period = %d ps, clock = %d kHz\n",
+	dev_dbg(fbi->dev, "dma period = %d ps, clock = %ld kHz\n",
 		sa1100fb_display_dma_period(var),
-		cpufreq_get(smp_processor_id()));
+		clk_get_rate(fbi->clk) / 1000);
 #endif
 
 	return 0;
@@ -586,9 +587,10 @@ static struct fb_ops sa1100fb_ops = {
  * Calculate the PCD value from the clock rate (in picoseconds).
  * We take account of the PPCR clock setting.
  */
-static inline unsigned int get_pcd(unsigned int pixclock, unsigned int cpuclock)
+static inline unsigned int get_pcd(struct sa1100fb_info *fbi,
+		unsigned int pixclock)
 {
-	unsigned int pcd = cpuclock / 100;
+	unsigned int pcd = clk_get_rate(fbi->clk) / 100 / 1000;
 
 	pcd *= pixclock;
 	pcd /= 10000000;
@@ -667,7 +669,7 @@ static int sa1100fb_activate_var(struct fb_var_screeninfo *var, struct sa1100fb_
 		LCCR2_BegFrmDel(var->upper_margin) +
 		LCCR2_EndFrmDel(var->lower_margin);
 
-	pcd = get_pcd(var->pixclock, cpufreq_get(0));
+	pcd = get_pcd(fbi, var->pixclock);
 	new_regs.lccr3 = LCCR3_PixClkDiv(pcd) | fbi->inf->lccr3 |
 		(var->sync & FB_SYNC_HOR_HIGH_ACT ? LCCR3_HorSnchH : LCCR3_HorSnchL) |
 		(var->sync & FB_SYNC_VERT_HIGH_ACT ? LCCR3_VrtSnchH : LCCR3_VrtSnchL);
@@ -781,6 +783,9 @@ static void sa1100fb_enable_controller(struct sa1100fb_info *fbi)
 	fbi->palette_cpu[0] &= 0xcfff;
 	fbi->palette_cpu[0] |= palette_pbs(&fbi->fb.var);
 
+	/* enable LCD controller clock */
+	clk_prepare_enable(fbi->clk);
+
 	/* Sequence from 11.7.10 */
 	writel_relaxed(fbi->reg_lccr3, fbi->base + LCCR3);
 	writel_relaxed(fbi->reg_lccr2, fbi->base + LCCR2);
@@ -825,6 +830,9 @@ static void sa1100fb_disable_controller(struct sa1100fb_info *fbi)
 
 	schedule_timeout(20 * HZ / 1000);
 	remove_wait_queue(&fbi->ctrlr_wait, &wait);
+
+	/* disable LCD controller clock */
+	clk_disable_unprepare(fbi->clk);
 }
 
 /*
@@ -1003,7 +1011,6 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val,
 			 void *data)
 {
 	struct sa1100fb_info *fbi = TO_INF(nb, freq_transition);
-	struct cpufreq_freqs *f = data;
 	u_int pcd;
 
 	switch (val) {
@@ -1012,7 +1019,7 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val,
 		break;
 
 	case CPUFREQ_POSTCHANGE:
-		pcd = get_pcd(fbi->fb.var.pixclock, f->new);
+		pcd = get_pcd(fbi, fbi->fb.var.pixclock);
 		fbi->reg_lccr3 = (fbi->reg_lccr3 & ~0xff) | LCCR3_PixClkDiv(pcd);
 		set_ctrlr_state(fbi, C_ENABLE_CLKCHANGE);
 		break;
@@ -1219,6 +1226,13 @@ static int sa1100fb_probe(struct platform_device *pdev)
 	if (!fbi)
 		goto failed;
 
+	fbi->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(fbi->clk)) {
+		ret = PTR_ERR(fbi->clk);
+		fbi->clk = NULL;
+		goto failed;
+	}
+
 	fbi->base = ioremap(res->start, resource_size(res));
 	if (!fbi->base)
 		goto failed;
@@ -1271,6 +1285,8 @@ static int sa1100fb_probe(struct platform_device *pdev)
  failed:
 	if (fbi)
 		iounmap(fbi->base);
+	if (fbi->clk)
+		clk_put(fbi->clk);
 	kfree(fbi);
 	release_mem_region(res->start, resource_size(res));
 	return ret;
diff --git a/drivers/video/fbdev/sa1100fb.h b/drivers/video/fbdev/sa1100fb.h
index fc5d429..0139d13 100644
--- a/drivers/video/fbdev/sa1100fb.h
+++ b/drivers/video/fbdev/sa1100fb.h
@@ -68,6 +68,7 @@ struct sa1100fb_info {
 #endif
 
 	const struct sa1100fb_mach_info *inf;
+	struct clk *clk;
 };
 
 #define TO_INF(ptr,member)	container_of(ptr,struct sa1100fb_info,member)
-- 
2.1.3


^ permalink raw reply related

* [PATCH V2 4/6] pcmcia: soc-common: enable/disable socket clocks
From: Dmitry Eremin-Solenikov @ 2014-11-24 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416870336-30114-1-git-send-email-dbaryshkov@gmail.com>

Call clk_prepare_enable() during hw_init() and clk_disable_unprepare()
during hw_shutdown() to ensure that the clock rates returned by
clk_get_rate() are correct.

It is safe to call enable/disable functions even on NULL clock, so this
patch will not break cases when the socket clock is not set.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/pcmcia/soc_common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index a2bc6ee..933f465 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -120,6 +120,8 @@ static void __soc_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt,
 
 	if (skt->ops->hw_shutdown)
 		skt->ops->hw_shutdown(skt);
+
+	clk_disable_unprepare(skt->clk);
 }
 
 static void soc_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt)
@@ -131,6 +133,8 @@ static int soc_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 {
 	int ret = 0, i;
 
+	clk_prepare_enable(skt->clk);
+
 	if (skt->ops->hw_init) {
 		ret = skt->ops->hw_init(skt);
 		if (ret)
-- 
2.1.3


^ permalink raw reply related

* [PATCH V2 5/6] pcmcia: sa1111: provide device clock
From: Dmitry Eremin-Solenikov @ 2014-11-24 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416870336-30114-1-git-send-email-dbaryshkov@gmail.com>

Both pxa2xx (long ago) and sa1100 (now) make use of clock device to get
the cpu speed. Make sa1111 glue code provide clock to platform layer.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/pcmcia/sa1111_generic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pcmcia/sa1111_generic.c b/drivers/pcmcia/sa1111_generic.c
index 65b02c3..7bae7e5 100644
--- a/drivers/pcmcia/sa1111_generic.c
+++ b/drivers/pcmcia/sa1111_generic.c
@@ -145,6 +145,12 @@ int sa1111_pcmcia_add(struct sa1111_dev *dev, struct pcmcia_low_level *ops,
 			return -ENOMEM;
 
 		s->soc.nr = ops->first + i;
+		s->soc.clk = clk_get(&dev->dev, NULL);
+		if (IS_ERR(s->soc.clk)) {
+			ret = PTR_ERR(s->soc.clk);
+			kfree(s);
+			return ret;
+		}
 		soc_pcmcia_init_one(&s->soc, ops, &dev->dev);
 		s->dev = dev;
 		if (s->soc.nr) {
@@ -220,6 +226,7 @@ static int pcmcia_remove(struct sa1111_dev *dev)
 	for (; s; s = next) {
 		next = s->next;
 		soc_pcmcia_remove_one(&s->soc);
+		clk_put(s->soc.clk);
 		kfree(s);
 	}
 
-- 
2.1.3


^ permalink raw reply related

* [PATCH V2 6/6] pcmcia: sa1100: make use of device clock
From: Dmitry Eremin-Solenikov @ 2014-11-24 23:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416870336-30114-1-git-send-email-dbaryshkov@gmail.com>

Use per-device clock (instead of calling cpufreq_get(0), which can
return 0 if no cpu frequency driver is selected) to program timings.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/pcmcia/sa1100_generic.c |  1 +
 drivers/pcmcia/sa11xx_base.c    | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/pcmcia/sa1100_generic.c b/drivers/pcmcia/sa1100_generic.c
index ff8a027..d2ab060 100644
--- a/drivers/pcmcia/sa1100_generic.c
+++ b/drivers/pcmcia/sa1100_generic.c
@@ -93,6 +93,7 @@ static int sa11x0_drv_pcmcia_remove(struct platform_device *dev)
 	for (i = 0; i < sinfo->nskt; i++)
 		soc_pcmcia_remove_one(&sinfo->skt[i]);
 
+	clk_put(sinfo->clk);
 	kfree(sinfo);
 	return 0;
 }
diff --git a/drivers/pcmcia/sa11xx_base.c b/drivers/pcmcia/sa11xx_base.c
index 54d3089..cf6de2c 100644
--- a/drivers/pcmcia/sa11xx_base.c
+++ b/drivers/pcmcia/sa11xx_base.c
@@ -135,14 +135,16 @@ sa1100_pcmcia_frequency_change(struct soc_pcmcia_socket *skt,
 static int
 sa1100_pcmcia_set_timing(struct soc_pcmcia_socket *skt)
 {
-	return sa1100_pcmcia_set_mecr(skt, cpufreq_get(0));
+	unsigned long clk = clk_get_rate(skt->clk);
+
+	return sa1100_pcmcia_set_mecr(skt, clk / 1000);
 }
 
 static int
 sa1100_pcmcia_show_timing(struct soc_pcmcia_socket *skt, char *buf)
 {
 	struct soc_pcmcia_timing timing;
-	unsigned int clock = cpufreq_get(0);
+	unsigned int clock = clk_get_rate(skt->clk);
 	unsigned long mecr = MECR;
 	char *p = buf;
 
@@ -218,6 +220,11 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	struct skt_dev_info *sinfo;
 	struct soc_pcmcia_socket *skt;
 	int i, ret = 0;
+	struct clk *clk;
+
+	clk = clk_get(dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
 
 	sa11xx_drv_pcmcia_ops(ops);
 
@@ -226,12 +233,14 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 		return -ENOMEM;
 
 	sinfo->nskt = nr;
+	sinfo->clk = clk;
 
 	/* Initialize processor specific parameters */
 	for (i = 0; i < nr; i++) {
 		skt = &sinfo->skt[i];
 
 		skt->nr = first + i;
+		skt->clk = clk;
 		soc_pcmcia_init_one(skt, ops, dev);
 
 		ret = sa11xx_drv_pcmcia_add_one(skt);
@@ -242,6 +251,7 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops,
 	if (ret) {
 		while (--i >= 0)
 			soc_pcmcia_remove_one(&sinfo->skt[i]);
+		clk_put(clk);
 		kfree(sinfo);
 	} else {
 		dev_set_drvdata(dev, sinfo);
-- 
2.1.3


^ permalink raw reply related

* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Tomi Valkeinen @ 2014-11-25  9:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
	liam.r.girdwood
In-Reply-To: <20141124173920.GL7712@sirena.org.uk>

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

On 24/11/14 19:39, Mark Brown wrote:

> OK...  this is all telling me that I *really* need to scrub this in
> detail.  It's all sounding very vague, it's an area which seems to cause
> lots of problems and I don't want to be sitting here next time around
> trying to figure out if another rewrite makes things better or worse or
> if another driver should look similar or different.

I was vague as I don't have the details. I don't know much about the
audio side, except from the users side. Jyri can perhaps fill in the
details.

But a proper review for sound/ side would be appreciated.

For what it's worth, I can say from a user's perspective that with this
series the OMAP HDMI audio finally seems to work so that I'm satisfied
with it. While there's still things to improve, it works and I can keep
it enabled in the kernel while developing the video side and it doesn't
get on my way.

>> The whole series is about HDMI audio, not video.
> 
> You know exactly what I mean - the early patches are in drivers/video,
> don't touch anything outside of that and have no obvious dependencies.

My point was, there's no particular reason to apply those patches
individually. It's an independent series, about HDMI audio, and applying
only some of the earlier patches does not improve the kernel in visible
manner.

>> And in any case, I don't like applying individual patches from a series.
>> Usually that just complicates things. If I would apply some of the early
>> patches to fbdev, then this series would no longer work on plain
>> mainline kernel, and would instead depend on fbdev tree. The situation
> 
> But I thought everyone was saying this hardware doesn't work anyway and
> that you want to apply all these changes to fbdev?

I don't particularly _want_ to apply these via fbdev, but I think it's
the easiest way. Most of the files touched are in drivers/video/, so
hopefully merging via fbdev reduces the chances of conflicts.

And this needs your ack before it can be merged via fbdev tree. Until
you've said that you're fine getting this via fbdev, we don't know how
this will be merged, and I can't create dependencies to fbdev.

> This means we're all then stuck reading reposts of the same enormous
> series over and over again - as a reviewer a really big series that
> appears from the subject lines to be mostly about another system is
> really offputting.  If you're going to do something like this please at

Well, yes, I see your point. And I agree that patches that the rest of
the series does not depend on should normally be applied individually if
the series starts getting multiple revisions (although even those
patches could cause conflicts if the rest of the patches touch code nearby).

But that's not the case here, as all the patches are needed to get a
working HDMI audio.

> least reply to the messages, that way it's clearer that there's not
> going to be a dependency problem getting the patches applied (which is
> part of what makes things offputting).

Yes, lack of communication was my mistake.

In any case, how do you want to proceed? As I said, I very much would
like to get this into the next merge window and from my point of view
the current versions looks good. If you find something to be fixed in
the sound/ side, and you're fine with merging this via fbdev, I can
start merging the earlier patches in the series so that the next
revision is easier to review.

But the merge window is getting close. If you find something very wrong
with the series, we can skip this merge window. But if there are only
issues that can be easily fixed with follow-up patches, I'd rather merge
this revision than do more full review rounds.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 3/6] video: fbdev-MMP: Fix typos for the word "destroy"
From: Tomi Valkeinen @ 2014-11-25 10:55 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, kernel-janitors,
	trivial, LKML
In-Reply-To: <547368BB.2050401@users.sourceforge.net>

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

On 24/11/14 19:19, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 24 Nov 2014 15:50:15 +0100
> 
> Two mistyped words were corrected in the description for
> the mmp_unregister_path() function.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/video/fbdev/mmp/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/mmp/core.c b/drivers/video/fbdev/mmp/core.c
> index b563b92..43ed576 100644
> --- a/drivers/video/fbdev/mmp/core.c
> +++ b/drivers/video/fbdev/mmp/core.c
> @@ -223,10 +223,10 @@ struct mmp_path *mmp_register_path(struct mmp_path_info *info)
>  EXPORT_SYMBOL_GPL(mmp_register_path);
>  
>  /*
> - * mmp_unregister_path - unregister and destory path
> + * mmp_unregister_path - unregister and destroy path
>   * @p: path to be destoried.
>   *
> - * this function registers path and destorys it.
> + * this function registers path and destroys it.
>   */
>  void mmp_unregister_path(struct mmp_path *path)
>  {
> 

Thanks, applied this and the patch 5/6 to fbdev tree.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Tomi Valkeinen @ 2014-11-25 12:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416309051-26784-3-git-send-email-hdegoede@redhat.com>

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

On 18/11/14 13:10, Hans de Goede wrote:
> If pre-filled framebuffer nodes are used, the firmware may need extra
> properties to find the right node. This documents the properties to use
> for this on sunxi platforms.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> ---
>  .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> new file mode 100644
> index 0000000..c46ba64
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> @@ -0,0 +1,33 @@
> +Sunxi specific Simple Framebuffer bindings
> +
> +This binding documents sunxi specific extensions to the simple-framebuffer
> +bindings. The sunxi simplefb u-boot code relies on the devicetree containing
> +pre-populated simplefb nodes.
> +
> +These extensions are intended so that u-boot can select the right node based
> +on which pipeline is being used. As such they are solely intended for
> +firmware / bootloader use, and the OS should ignore them.
> +
> +Required properties:
> +- compatible: "allwinner,simple-framebuffer"
> +- allwinner,pipeline, one of:

Sorry my ignorance, but what's sunxi and what's allwinner? Both names
are mixed here.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v3 0/2] dt-bindings: simplefb: Drop the advice about using a specific path for nodes
From: Tomi Valkeinen @ 2014-11-25 12:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1416309051-26784-1-git-send-email-hdegoede@redhat.com>

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

Hi,

On 18/11/14 13:10, Hans de Goede wrote:
> Hi Tomi,
> 
> So it turns out we needed a v3, as I mistakenly went with "sunxi," as vendor
> prefix for the allwinner specific properties, but the registered vendor prefix
> for allwinner is "allwinnner,", this version fixes this.
> 
> Changes in v3: Use proper "allwinnner," for the compatible string and vendor
> specific properties.
> 
> Changes in v2: Changed the simplefb-sunxi bindings to use a single
> sunxi,pipeline string property instead of a sunxi,pipeline int and a
> sunxi,output string property.
> 
> This patch set is all acked up, and a fix to the binding changes you've
> already queued for 3.19, so please queue these 2 patches for 3.19.

The discussions about this have continued in the earlier version of this
series. Is this series still ok for everyone?

The series looks fine to me, and has Grant's acks, so I'll merge it
tomorrow to fbdev tree if no one objects (and my comment for the patch 2
about sunxi/allwinner was just my ignorance).

 Tomi



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

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Hans de Goede @ 2014-11-25 12:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <547476DA.2030004@ti.com>

Hi,

On 11/25/2014 01:32 PM, Tomi Valkeinen wrote:
> On 18/11/14 13:10, Hans de Goede wrote:
>> If pre-filled framebuffer nodes are used, the firmware may need extra
>> properties to find the right node. This documents the properties to use
>> for this on sunxi platforms.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> ---
>>   .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>> new file mode 100644
>> index 0000000..c46ba64
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>> @@ -0,0 +1,33 @@
>> +Sunxi specific Simple Framebuffer bindings
>> +
>> +This binding documents sunxi specific extensions to the simple-framebuffer
>> +bindings. The sunxi simplefb u-boot code relies on the devicetree containing
>> +pre-populated simplefb nodes.
>> +
>> +These extensions are intended so that u-boot can select the right node based
>> +on which pipeline is being used. As such they are solely intended for
>> +firmware / bootloader use, and the OS should ignore them.
>> +
>> +Required properties:
>> +- compatible: "allwinner,simple-framebuffer"
>> +- allwinner,pipeline, one of:
>
> Sorry my ignorance, but what's sunxi and what's allwinner? Both names
> are mixed here.

sunxi is the sun#i SoCs from Allwinner, Allwinner is the manufacturer and the
SoC "code" names used everywhere in the kernel for their SoCs are sun4i, sun5i,
sun6i, etc. Most people refer to these SoCs as sunxi. This is also what the
linux-sunxi mailinglist in the Cc is about.

The official devicetree vendor prefix for Allwinner is allwinner, hence the
allwinner in the compatible name, see e.g. also

Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt

Which also uses sunxi / sun4i everywhere except in the compatible vendor prefix.

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Tomi Valkeinen @ 2014-11-25 13:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <54747B72.1050406@redhat.com>

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

On 25/11/14 14:52, Hans de Goede wrote:

>>> +Required properties:
>>> +- compatible: "allwinner,simple-framebuffer"
>>> +- allwinner,pipeline, one of:
>>
>> Sorry my ignorance, but what's sunxi and what's allwinner? Both names
>> are mixed here.
> 
> sunxi is the sun#i SoCs from Allwinner, Allwinner is the manufacturer
> and the
> SoC "code" names used everywhere in the kernel for their SoCs are sun4i,
> sun5i,
> sun6i, etc. Most people refer to these SoCs as sunxi. This is also what the
> linux-sunxi mailinglist in the Cc is about.
> 
> The official devicetree vendor prefix for Allwinner is allwinner, hence the
> allwinner in the compatible name, see e.g. also
> 
> Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> 
> Which also uses sunxi / sun4i everywhere except in the compatible vendor
> prefix.

Alright, thanks for explanation.

Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
to differentiate from some other SoC Allwinner has or might create in
the future? That is, presuming you're confident enough that a single
compatible string covers all the current and forthcoming sunxi SoCs.

Perhaps simplefb is a bit special case, but I usually feel better if the
compatible string is defined in a more specific manner. In this case I'd
have:

allwinner,sun4i-simple-framebuffer
allwinner,sun5i-simple-framebuffer
allwinner,sun6i-simple-framebuffer

so that if sun7i has totally different display controller, there would
be no conflict.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Hans de Goede @ 2014-11-25 13:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <54747DCF.9010502@ti.com>

Hi,

On 11/25/2014 02:02 PM, Tomi Valkeinen wrote:
> On 25/11/14 14:52, Hans de Goede wrote:
>
>>>> +Required properties:
>>>> +- compatible: "allwinner,simple-framebuffer"
>>>> +- allwinner,pipeline, one of:
>>>
>>> Sorry my ignorance, but what's sunxi and what's allwinner? Both names
>>> are mixed here.
>>
>> sunxi is the sun#i SoCs from Allwinner, Allwinner is the manufacturer
>> and the
>> SoC "code" names used everywhere in the kernel for their SoCs are sun4i,
>> sun5i,
>> sun6i, etc. Most people refer to these SoCs as sunxi. This is also what the
>> linux-sunxi mailinglist in the Cc is about.
>>
>> The official devicetree vendor prefix for Allwinner is allwinner, hence the
>> allwinner in the compatible name, see e.g. also
>>
>> Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
>>
>> Which also uses sunxi / sun4i everywhere except in the compatible vendor
>> prefix.
>
> Alright, thanks for explanation.
>
> Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
> to differentiate from some other SoC Allwinner has or might create in
> the future? That is, presuming you're confident enough that a single
> compatible string covers all the current and forthcoming sunxi SoCs.

This was discussed in an earlier thread, we (Ian Campbell, Grant and me)
decided to settle on allwinner,simple-framebuffer to make it clear that
these are allwinner extensions to the standard simple-framebuffer bindings,
and that the node otherwise is simple-framebuffer compatible.

We were afraid that e.g. sun4i-simple-framebuffer would signal that it
is not a normal simple-framebuffer node, so we decided to go with just
the allwinner, prefix to indicate that it uses allwinner specific
extensions.

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Tomi Valkeinen @ 2014-11-25 13:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5474824F.8080000@redhat.com>

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

On 25/11/14 15:21, Hans de Goede wrote:

>> Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
>> to differentiate from some other SoC Allwinner has or might create in
>> the future? That is, presuming you're confident enough that a single
>> compatible string covers all the current and forthcoming sunxi SoCs.
> 
> This was discussed in an earlier thread, we (Ian Campbell, Grant and me)

Okay. Sorry for not having time at the moment to follow the discussions
properly. =)

> decided to settle on allwinner,simple-framebuffer to make it clear that
> these are allwinner extensions to the standard simple-framebuffer bindings,
> and that the node otherwise is simple-framebuffer compatible.
> 
> We were afraid that e.g. sun4i-simple-framebuffer would signal that it
> is not a normal simple-framebuffer node, so we decided to go with just
> the allwinner, prefix to indicate that it uses allwinner specific
> extensions.

Wouldn't

compatible = "allwinner,sun4i-simple-framebuffer", "simple-framebuffer";

tell that it's a simple-framebuffer, with allwinner's sun4i extensions?

I guess you can have just "allwinner,simple-framebuffer", and then if a
new Allwinner SoC has a totally different display controller, the
documentation would specify that this property is for that SoC, and this
another property is for that another SoC. But isn't the compatible
string what's supposed to use in cases like this?

And if the new SoC is not sunxi, but some totally other family, there's
need for a new compatible string anyway, as
"simple-framebuffer-sunxi.txt" is for sunxi only.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Hans de Goede @ 2014-11-25 13:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5474863A.5040801@ti.com>

Hi,

On 11/25/2014 02:38 PM, Tomi Valkeinen wrote:
> On 25/11/14 15:21, Hans de Goede wrote:
>
>>> Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
>>> to differentiate from some other SoC Allwinner has or might create in
>>> the future? That is, presuming you're confident enough that a single
>>> compatible string covers all the current and forthcoming sunxi SoCs.
>>
>> This was discussed in an earlier thread, we (Ian Campbell, Grant and me)
>
> Okay. Sorry for not having time at the moment to follow the discussions
> properly. =)
>
>> decided to settle on allwinner,simple-framebuffer to make it clear that
>> these are allwinner extensions to the standard simple-framebuffer bindings,
>> and that the node otherwise is simple-framebuffer compatible.
>>
>> We were afraid that e.g. sun4i-simple-framebuffer would signal that it
>> is not a normal simple-framebuffer node, so we decided to go with just
>> the allwinner, prefix to indicate that it uses allwinner specific
>> extensions.
>
> Wouldn't
>
> compatible = "allwinner,sun4i-simple-framebuffer", "simple-framebuffer";
>
> tell that it's a simple-framebuffer, with allwinner's sun4i extensions?
>
> I guess you can have just "allwinner,simple-framebuffer", and then if a
> new Allwinner SoC has a totally different display controller, the
> documentation would specify that this property is for that SoC, and this
> another property is for that another SoC. But isn't the compatible
> string what's supposed to use in cases like this?

The only soc specific thing in the binding is the pipeline property string
values, and we can always add new values to that, the rest is all generic,
as simplefb is generic.

As said Ian Campbell, Grant and me have decided on using this, and currently
patches are already queued up for both the dts files and u-boot to use this,
so unless there are really strong reasons to change it at this point I would
prefer to keep this as is.

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Mark Brown @ 2014-11-25 18:10 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
	liam.r.girdwood
In-Reply-To: <54744B4C.2030105@ti.com>

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

On Tue, Nov 25, 2014 at 11:26:36AM +0200, Tomi Valkeinen wrote:
> On 24/11/14 19:39, Mark Brown wrote:

> >> The whole series is about HDMI audio, not video.

> > You know exactly what I mean - the early patches are in drivers/video,
> > don't touch anything outside of that and have no obvious dependencies.

> My point was, there's no particular reason to apply those patches
> individually. It's an independent series, about HDMI audio, and applying
> only some of the earlier patches does not improve the kernel in visible
> manner.

Can you please apply them, I'll try to get round to reviewing the audio
bits soon but either these will need applying if the rest of it's OK or
if there does turn out to be a problem they cut down on the size of the
series.  I really do want to read it closely, but hopefully by the
weekend.

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

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Tomi Valkeinen @ 2014-11-26  8:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5474880F.8040908@redhat.com>

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

On 25/11/14 15:45, Hans de Goede wrote:

>> Wouldn't
>>
>> compatible = "allwinner,sun4i-simple-framebuffer", "simple-framebuffer";
>>
>> tell that it's a simple-framebuffer, with allwinner's sun4i extensions?
>>
>> I guess you can have just "allwinner,simple-framebuffer", and then if a
>> new Allwinner SoC has a totally different display controller, the
>> documentation would specify that this property is for that SoC, and this
>> another property is for that another SoC. But isn't the compatible
>> string what's supposed to use in cases like this?
> 
> The only soc specific thing in the binding is the pipeline property string
> values, and we can always add new values to that, the rest is all generic,
> as simplefb is generic.

The thing I don't understand is that the compatible string states that
"this covers all Allwinner SoCs", even if we have no idea what kind of
SoCs those may be. And if it covers all kinds of SoCs, then it might as
well be fully generic, not Allwinner specific.

And if it's not fully generic, then having it cover all possible
Allwinner SoCs doesn't make sense either.

> As said Ian Campbell, Grant and me have decided on using this, and
> currently
> patches are already queued up for both the dts files and u-boot to use
> this,
> so unless there are really strong reasons to change it at this point I
> would
> prefer to keep this as is.

Ok. Well, as I said, it does not look correct to me, but if everybody
else agrees on it (and I see I didn't get any replies during the night),
I'll be applying this today.

 Tomi



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

^ permalink raw reply

* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Tomi Valkeinen @ 2014-11-26 11:02 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
	liam.r.girdwood
In-Reply-To: <20141125181020.GJ7712@sirena.org.uk>

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

On 25/11/14 20:10, Mark Brown wrote:
> On Tue, Nov 25, 2014 at 11:26:36AM +0200, Tomi Valkeinen wrote:
>> On 24/11/14 19:39, Mark Brown wrote:
> 
>>>> The whole series is about HDMI audio, not video.
> 
>>> You know exactly what I mean - the early patches are in drivers/video,
>>> don't touch anything outside of that and have no obvious dependencies.
> 
>> My point was, there's no particular reason to apply those patches
>> individually. It's an independent series, about HDMI audio, and applying
>> only some of the earlier patches does not improve the kernel in visible
>> manner.
> 
> Can you please apply them, I'll try to get round to reviewing the audio
> bits soon but either these will need applying if the rest of it's OK or
> if there does turn out to be a problem they cut down on the size of the
> series.  I really do want to read it closely, but hopefully by the
> weekend.

Ok, I've picked the following patches:

OMAPDSS: hdmi.h: Add members to hdmi drvdata for audio implementation
OMAPDSS: hdmi: Add pdev pointer for audio_pdev in HDMI DRV data
OMAPDSS: hdmi: Make hdmi structure public
OMAPDSS: hdmi_wp: Add function for getting audio dma address
OMAPDSS: hdmi4_core: Remove unused hdmi4_audio_get_dma_port()
OMAPDSS: hdmi: Remove most of OMAP[45]_DSS_HDMI_AUDIO ifdefs
OMAPDSS: hdmi.h: Add HDMI_AUDIO_LAYOUT_6CH enum value
OMAPDSS: hdmi5_core: Initialize mandatory sample_order parameter
OMAPDSS: hdmi_wp: Protect reserved bits in hdmi_wp_audio_config_format()

 Tomi



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

^ permalink raw reply

* [PATCH 1/2] backlight/lp855x: Refactor dt parsing code
From: Sean Paul @ 2014-11-26 19:11 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, milo.kim-l0cyMroinI0
  Cc: seanpaul-F7+t8E8rja9g9hUCZPvPmw, Stéphane Marchesin

This patch refactors the dt parsing code to avoid setting platform_data,
instead just setting lp->pdata directly. This facilitates easier
probe deferral since the current scheme would require us to clear out
dev->platform_data before deferring.

Cc: Stéphane Marchesin <marcheu@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/video/backlight/lp855x_bl.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 25fb8e3..257b3ba 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -341,8 +341,10 @@ static const struct attribute_group lp855x_attr_group = {
 };
 
 #ifdef CONFIG_OF
-static int lp855x_parse_dt(struct device *dev, struct device_node *node)
+static int lp855x_parse_dt(struct lp855x *lp)
 {
+	struct device *dev = lp->dev;
+	struct device_node *node = dev->of_node;
 	struct lp855x_platform_data *pdata;
 	int rom_length;
 
@@ -381,12 +383,12 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
 		pdata->rom_data = &rom[0];
 	}
 
-	dev->platform_data = pdata;
+	lp->pdata = pdata;
 
 	return 0;
 }
 #else
-static int lp855x_parse_dt(struct device *dev, struct device_node *node)
+static int lp855x_parse_dt(struct lp855x *lp)
 {
 	return -EINVAL;
 }
@@ -395,18 +397,8 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
 static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 {
 	struct lp855x *lp;
-	struct lp855x_platform_data *pdata = dev_get_platdata(&cl->dev);
-	struct device_node *node = cl->dev.of_node;
 	int ret;
 
-	if (!pdata) {
-		ret = lp855x_parse_dt(&cl->dev, node);
-		if (ret < 0)
-			return ret;
-
-		pdata = dev_get_platdata(&cl->dev);
-	}
-
 	if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
 		return -EIO;
 
@@ -414,16 +406,23 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	if (!lp)
 		return -ENOMEM;
 
-	if (pdata->period_ns > 0)
-		lp->mode = PWM_BASED;
-	else
-		lp->mode = REGISTER_BASED;
-
 	lp->client = cl;
 	lp->dev = &cl->dev;
-	lp->pdata = pdata;
 	lp->chipname = id->name;
 	lp->chip_id = id->driver_data;
+	lp->pdata = dev_get_platdata(&cl->dev);
+
+	if (!lp->pdata) {
+		ret = lp855x_parse_dt(lp);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (lp->pdata->period_ns > 0)
+		lp->mode = PWM_BASED;
+	else
+		lp->mode = REGISTER_BASED;
+
 	i2c_set_clientdata(cl, lp);
 
 	ret = lp855x_configure(lp);
-- 
2.1.1


^ permalink raw reply related

* [PATCH 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Sean Paul @ 2014-11-26 19:11 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, milo.kim-l0cyMroinI0
  Cc: seanpaul-F7+t8E8rja9g9hUCZPvPmw, Stéphane Marchesin,
	Aaron Durbin
In-Reply-To: <1417029064-12460-1-git-send-email-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

This patch adds a supply regulator to the lp855x platform data to facilitate
powering on/off the 3V rail attached to the controller.

Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Aaron Durbin <adurbin@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 .../devicetree/bindings/video/backlight/lp855x.txt      |  2 ++
 drivers/video/backlight/lp855x_bl.c                     | 17 +++++++++++++++++
 include/linux/platform_data/lp855x.h                    |  2 ++
 3 files changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/backlight/lp855x.txt b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
index 96e83a5..0a3ecbc 100644
--- a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
+++ b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
@@ -12,6 +12,7 @@ Optional properties:
   - pwm-period: PWM period value. Set only PWM input mode used (u32)
   - rom-addr: Register address of ROM area to be updated (u8)
   - rom-val: Register value to be updated (u8)
+  - power-supply: Regulator which controls the 3V rail
 
 Example:
 
@@ -56,6 +57,7 @@ Example:
 	backlight@2c {
 		compatible = "ti,lp8557";
 		reg = <0x2c>;
+		power-supply = <&backlight_vdd>;
 
 		dev-ctrl = /bits/ 8 <0x41>;
 		init-brt = /bits/ 8 <0x0a>;
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 257b3ba..8021009 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -17,6 +17,7 @@
 #include <linux/of.h>
 #include <linux/platform_data/lp855x.h>
 #include <linux/pwm.h>
+#include <linux/regulator/consumer.h>
 
 /* LP8550/1/2/3/6 Registers */
 #define LP855X_BRIGHTNESS_CTRL		0x00
@@ -383,6 +384,13 @@ static int lp855x_parse_dt(struct lp855x *lp)
 		pdata->rom_data = &rom[0];
 	}
 
+	pdata->supply = devm_regulator_get(dev, "power");
+	if (IS_ERR(pdata->supply)) {
+		if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		pdata->supply = NULL;
+	}
+
 	lp->pdata = pdata;
 
 	return 0;
@@ -423,6 +431,14 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	else
 		lp->mode = REGISTER_BASED;
 
+	if (lp->pdata->supply) {
+		ret = regulator_enable(lp->pdata->supply);
+		if (ret < 0) {
+			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
+			return ret;
+		}
+	}
+
 	i2c_set_clientdata(cl, lp);
 
 	ret = lp855x_configure(lp);
@@ -454,6 +470,7 @@ static int lp855x_remove(struct i2c_client *cl)
 
 	lp->bl->props.brightness = 0;
 	backlight_update_status(lp->bl);
+	regulator_disable(lp->pdata->supply);
 	sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
 
 	return 0;
diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
index 1b2ba24..9c7fd1e 100644
--- a/include/linux/platform_data/lp855x.h
+++ b/include/linux/platform_data/lp855x.h
@@ -136,6 +136,7 @@ struct lp855x_rom_data {
 		Only valid when mode is PWM_BASED.
  * @size_program : total size of lp855x_rom_data
  * @rom_data : list of new eeprom/eprom registers
+ * @supply : regulator that supplies 3V input
  */
 struct lp855x_platform_data {
 	const char *name;
@@ -144,6 +145,7 @@ struct lp855x_platform_data {
 	unsigned int period_ns;
 	int size_program;
 	struct lp855x_rom_data *rom_data;
+	struct regulator *supply;
 };
 
 #endif
-- 
2.1.1


^ permalink raw reply related

* [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Lad, Prabhakar @ 2014-11-26 22:07 UTC (permalink / raw)
  To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
  Cc: linux-fbdev, linux-kernel, Lad, Prabhakar

this patch fixes following build warning:
drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  par->state.vgabase = (void __iomem *) vga_res.start;
                       ^
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 drivers/video/fbdev/vt8623fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index 5c7cbc6..ea7f056 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
 
-	par->state.vgabase = (void __iomem *) vga_res.start;
+	par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
 
 	/* Find how many physical memory there is on card */
 	memsize1 = (vga_rseq(par->state.vgabase, 0x34) + 1) >> 1;
-- 
1.9.1


^ permalink raw reply related

* [PATCH 2/3] video: fbdev: s3fb: suppress build warning
From: Lad, Prabhakar @ 2014-11-26 22:07 UTC (permalink / raw)
  To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
  Cc: linux-fbdev, linux-kernel, Lad, Prabhakar
In-Reply-To: <1417039645-5313-1-git-send-email-prabhakar.csengg@gmail.com>

this patch fixes following build warning:
drivers/video/fbdev/s3fb.c: In function ‘s3_pci_probe’:
drivers/video/fbdev/s3fb.c:1185:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  par->state.vgabase = (void __iomem *) vga_res.start;
                       ^
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 drivers/video/fbdev/s3fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index c43b969..f0ae61a 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -1182,7 +1182,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
 
-	par->state.vgabase = (void __iomem *) vga_res.start;
+	par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
 
 	/* Unlock regs */
 	cr38 = vga_rcrt(par->state.vgabase, 0x38);
-- 
1.9.1


^ permalink raw reply related

* [PATCH 3/3] video: fbdev: arkfb: suppress build warning
From: Lad, Prabhakar @ 2014-11-26 22:07 UTC (permalink / raw)
  To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
  Cc: linux-fbdev, linux-kernel, Lad, Prabhakar
In-Reply-To: <1417039645-5313-1-git-send-email-prabhakar.csengg@gmail.com>

this patch fixes following build warning:

drivers/video/fbdev/arkfb.c: In function ‘ark_pci_probe’:
drivers/video/fbdev/arkfb.c:1019:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  par->state.vgabase = (void __iomem *) vga_res.start;
                       ^
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 drivers/video/fbdev/arkfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index adc4ea2..b305a1e 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -1016,7 +1016,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
 
-	par->state.vgabase = (void __iomem *) vga_res.start;
+	par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
 
 	/* FIXME get memsize */
 	regval = vga_rseq(par->state.vgabase, 0x10);
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH 1/2] backlight/lp855x: Refactor dt parsing code
From: Kim, Milo @ 2014-11-27  1:32 UTC (permalink / raw)
  To: Sean Paul, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Stéphane Marchesin, jg1.han-Sze3O3UU22JBDgjK7y7TUQ,
	cooloney-Re5JQEeQqe8AvxtiuMwx3w, lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1417029064-12460-1-git-send-email-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

(Looping backlight class subsystem maintainers)

Hi Sean,

Thanks for checking this. I'd like to describe why the original code is 
preferred.

The original code is copying the values of the lp855x platform data from 
the DT in advance of allocating lp855x data.
It guarantees returning quickly in case of invalid platform data.
In other words, no memory allocation of lp855x proceeds if parsing the 
DT gets failed.
However, this patch allocates the lp855x first and checking the DT.
Of course, devm_kzalloc() will free allocated memory later but original 
code does NOT try to allocate it.
So I'd prefer to keep this way.

Best regards,
Milo

On 11/27/2014 4:11 AM, Sean Paul wrote:
> This patch refactors the dt parsing code to avoid setting platform_data,
> instead just setting lp->pdata directly. This facilitates easier
> probe deferral since the current scheme would require us to clear out
> dev->platform_data before deferring.
>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>   drivers/video/backlight/lp855x_bl.c | 37 ++++++++++++++++++-------------------
>   1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 25fb8e3..257b3ba 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -341,8 +341,10 @@ static const struct attribute_group lp855x_attr_group = {
>   };
>
>   #ifdef CONFIG_OF
> -static int lp855x_parse_dt(struct device *dev, struct device_node *node)
> +static int lp855x_parse_dt(struct lp855x *lp)
>   {
> +	struct device *dev = lp->dev;
> +	struct device_node *node = dev->of_node;
>   	struct lp855x_platform_data *pdata;
>   	int rom_length;
>
> @@ -381,12 +383,12 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
>   		pdata->rom_data = &rom[0];
>   	}
>
> -	dev->platform_data = pdata;
> +	lp->pdata = pdata;
>
>   	return 0;
>   }
>   #else
> -static int lp855x_parse_dt(struct device *dev, struct device_node *node)
> +static int lp855x_parse_dt(struct lp855x *lp)
>   {
>   	return -EINVAL;
>   }
> @@ -395,18 +397,8 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
>   static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>   {
>   	struct lp855x *lp;
> -	struct lp855x_platform_data *pdata = dev_get_platdata(&cl->dev);
> -	struct device_node *node = cl->dev.of_node;
>   	int ret;
>
> -	if (!pdata) {
> -		ret = lp855x_parse_dt(&cl->dev, node);
> -		if (ret < 0)
> -			return ret;
> -
> -		pdata = dev_get_platdata(&cl->dev);
> -	}
> -
>   	if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
>   		return -EIO;
>
> @@ -414,16 +406,23 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>   	if (!lp)
>   		return -ENOMEM;
>
> -	if (pdata->period_ns > 0)
> -		lp->mode = PWM_BASED;
> -	else
> -		lp->mode = REGISTER_BASED;
> -
>   	lp->client = cl;
>   	lp->dev = &cl->dev;
> -	lp->pdata = pdata;
>   	lp->chipname = id->name;
>   	lp->chip_id = id->driver_data;
> +	lp->pdata = dev_get_platdata(&cl->dev);
> +
> +	if (!lp->pdata) {
> +		ret = lp855x_parse_dt(lp);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	if (lp->pdata->period_ns > 0)
> +		lp->mode = PWM_BASED;
> +	else
> +		lp->mode = REGISTER_BASED;
> +
>   	i2c_set_clientdata(cl, lp);
>
>   	ret = lp855x_configure(lp);
>

^ permalink raw reply

* Re: [PATCH 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Kim, Milo @ 2014-11-27  1:32 UTC (permalink / raw)
  To: Sean Paul, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Stéphane Marchesin, Aaron Durbin,
	jg1.han-Sze3O3UU22JBDgjK7y7TUQ, cooloney-Re5JQEeQqe8AvxtiuMwx3w,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1417029064-12460-2-git-send-email-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

(Looping backlight class subsystem maintainers)

Hi Sean,

Please see my comments below. Thanks!
On 11/27/2014 4:11 AM, Sean Paul wrote:
> This patch adds a supply regulator to the lp855x platform data to facilitate
> powering on/off the 3V rail attached to the controller.
>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: Aaron Durbin <adurbin@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>   .../devicetree/bindings/video/backlight/lp855x.txt      |  2 ++
>   drivers/video/backlight/lp855x_bl.c                     | 17 +++++++++++++++++
>   include/linux/platform_data/lp855x.h                    |  2 ++
>   3 files changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/video/backlight/lp855x.txt b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
> index 96e83a5..0a3ecbc 100644
> --- a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
> +++ b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
> @@ -12,6 +12,7 @@ Optional properties:
>     - pwm-period: PWM period value. Set only PWM input mode used (u32)
>     - rom-addr: Register address of ROM area to be updated (u8)
>     - rom-val: Register value to be updated (u8)
> +  - power-supply: Regulator which controls the 3V rail
>
>   Example:
>
> @@ -56,6 +57,7 @@ Example:
>   	backlight@2c {
>   		compatible = "ti,lp8557";
>   		reg = <0x2c>;
> +		power-supply = <&backlight_vdd>;
>
>   		dev-ctrl = /bits/ 8 <0x41>;
>   		init-brt = /bits/ 8 <0x0a>;
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 257b3ba..8021009 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -17,6 +17,7 @@
>   #include <linux/of.h>
>   #include <linux/platform_data/lp855x.h>
>   #include <linux/pwm.h>
> +#include <linux/regulator/consumer.h>
>
>   /* LP8550/1/2/3/6 Registers */
>   #define LP855X_BRIGHTNESS_CTRL		0x00
> @@ -383,6 +384,13 @@ static int lp855x_parse_dt(struct lp855x *lp)
>   		pdata->rom_data = &rom[0];
>   	}
>
> +	pdata->supply = devm_regulator_get(dev, "power");
> +	if (IS_ERR(pdata->supply)) {
> +		if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		pdata->supply = NULL;

This optional data, 'supply' is set to NULL.
Please refer to my comments below.

> +	}
> +
>   	lp->pdata = pdata;
>
>   	return 0;
> @@ -423,6 +431,14 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>   	else
>   		lp->mode = REGISTER_BASED;
>
> +	if (lp->pdata->supply) {
> +		ret = regulator_enable(lp->pdata->supply);
> +		if (ret < 0) {
> +			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
>   	i2c_set_clientdata(cl, lp);
>
>   	ret = lp855x_configure(lp);
> @@ -454,6 +470,7 @@ static int lp855x_remove(struct i2c_client *cl)
>
>   	lp->bl->props.brightness = 0;
>   	backlight_update_status(lp->bl);
> +	regulator_disable(lp->pdata->supply);

The 'lp->pdata->supply' is optional, what happens if it is NULL?
This will cause the kernel OOPS inside regulator_disable() API.

int regulator_disable(struct regulator *regulator)
{
	struct regulator_dev *rdev = regulator->rdev;
...
}

>   	sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
>
>   	return 0;
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> index 1b2ba24..9c7fd1e 100644
> --- a/include/linux/platform_data/lp855x.h
> +++ b/include/linux/platform_data/lp855x.h
> @@ -136,6 +136,7 @@ struct lp855x_rom_data {
>   		Only valid when mode is PWM_BASED.
>    * @size_program : total size of lp855x_rom_data
>    * @rom_data : list of new eeprom/eprom registers
> + * @supply : regulator that supplies 3V input
>    */
>   struct lp855x_platform_data {
>   	const char *name;
> @@ -144,6 +145,7 @@ struct lp855x_platform_data {
>   	unsigned int period_ns;
>   	int size_program;
>   	struct lp855x_rom_data *rom_data;
> +	struct regulator *supply;
>   };
>
>   #endif
>

Best regards,
Milo

^ 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