* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-08-28 16:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4Oy8X9CSpZjYnTRgZ_MsxXQ=zaP+kxiZ2b7Es4GjN+1iJw@mail.gmail.com>
On 28 August 2014 16:33, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Aug 28, 2014 at 10:20 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Aug 28, 2014 at 3:22 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>>>> 2) We don't want to hardcode these clocks into the kernel (sunxi) clk
>>>> driver, instead the bootloader should tell the kernel about these clocks.
>>>>
>>>> So the only point of discussion left seems to be how to do 2...
>>>
>>> Wouldn't it be a lot simpler just to use existing fbdev (not KMS) and
>>> whip together a device specific driver that claims the proper
>>> resources? And just implement the minimal about of fbdev possible?
>>> fbdev already is a driver library.
>>
>> Like... drivers/video/fbdev/offb.c?
>
> I'd probably reclassify drivers/video/fbdev/simplefb.c as a skeleton
> and use it as a template for making device specific versions of it.
>
> I don't see why there is so much resistance to just making device
> specific fb drivers. Whenever the KMS driver gets written just
> disable the device specific fb driver in the build.
Except that is not the goal here. The simplefb or whatever replacement
is supposed to stay as a generic driver compiled into kernel whereas
the complete platform-specific driver is supposed to be provided as
module and loaded at the init system's leasure sometime during boot.
This way you can have generic distribution kernel which supports many
devices but does not have built-in support for every graphics
hardware.
Thanks
Michal
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-08-28 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdXwDbpcyQuXMBZuNKBF0DmJ+F7BN8vJiupDNe3sqTSqYQ@mail.gmail.com>
On Thu, Aug 28, 2014 at 10:37 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Aug 28, 2014 at 4:33 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>> On Thu, Aug 28, 2014 at 10:20 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Thu, Aug 28, 2014 at 3:22 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>>>>> 2) We don't want to hardcode these clocks into the kernel (sunxi) clk
>>>>> driver, instead the bootloader should tell the kernel about these clocks.
>>>>>
>>>>> So the only point of discussion left seems to be how to do 2...
>>>>
>>>> Wouldn't it be a lot simpler just to use existing fbdev (not KMS) and
>>>> whip together a device specific driver that claims the proper
>>>> resources? And just implement the minimal about of fbdev possible?
>>>> fbdev already is a driver library.
>>>
>>> Like... drivers/video/fbdev/offb.c?
>>
>> I'd probably reclassify drivers/video/fbdev/simplefb.c as a skeleton
>> and use it as a template for making device specific versions of it.
>>
>> I don't see why there is so much resistance to just making device
>> specific fb drivers. Whenever the KMS driver gets written just
>> disable the device specific fb driver in the build.
>
> I explicitly named offb, because it already supports living with the
> video mode initialized by Open Firmware, which is passed to the kernel
> in a device tree.
Not sure how that works. It from a PowerMac
arch/powerpc/platforms/powermac/bootx_init.c
arch/powerpc/kernel/prom_init.c
>
> --
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Geert Uytterhoeven @ 2014-08-28 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4Oy8X9CSpZjYnTRgZ_MsxXQ=zaP+kxiZ2b7Es4GjN+1iJw@mail.gmail.com>
On Thu, Aug 28, 2014 at 4:33 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Aug 28, 2014 at 10:20 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Aug 28, 2014 at 3:22 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>>>> 2) We don't want to hardcode these clocks into the kernel (sunxi) clk
>>>> driver, instead the bootloader should tell the kernel about these clocks.
>>>>
>>>> So the only point of discussion left seems to be how to do 2...
>>>
>>> Wouldn't it be a lot simpler just to use existing fbdev (not KMS) and
>>> whip together a device specific driver that claims the proper
>>> resources? And just implement the minimal about of fbdev possible?
>>> fbdev already is a driver library.
>>
>> Like... drivers/video/fbdev/offb.c?
>
> I'd probably reclassify drivers/video/fbdev/simplefb.c as a skeleton
> and use it as a template for making device specific versions of it.
>
> I don't see why there is so much resistance to just making device
> specific fb drivers. Whenever the KMS driver gets written just
> disable the device specific fb driver in the build.
I explicitly named offb, because it already supports living with the
video mode initialized by Open Firmware, which is passed to the kernel
in a device tree.
--
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-08-28 14:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdU+zxUCU05YOpyuP1fx=Dgb5SZu=vbaJW13sQAaMBwU_w@mail.gmail.com>
On Thu, Aug 28, 2014 at 10:20 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Aug 28, 2014 at 3:22 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>>> 2) We don't want to hardcode these clocks into the kernel (sunxi) clk
>>> driver, instead the bootloader should tell the kernel about these clocks.
>>>
>>> So the only point of discussion left seems to be how to do 2...
>>
>> Wouldn't it be a lot simpler just to use existing fbdev (not KMS) and
>> whip together a device specific driver that claims the proper
>> resources? And just implement the minimal about of fbdev possible?
>> fbdev already is a driver library.
>
> Like... drivers/video/fbdev/offb.c?
I'd probably reclassify drivers/video/fbdev/simplefb.c as a skeleton
and use it as a template for making device specific versions of it.
I don't see why there is so much resistance to just making device
specific fb drivers. Whenever the KMS driver gets written just
disable the device specific fb driver in the build.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Geert Uytterhoeven @ 2014-08-28 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4OyXPjXx4q828kFgMsC_u7u3nqLPLxOp-kRZbS27yRKOEg@mail.gmail.com>
On Thu, Aug 28, 2014 at 3:22 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>> 2) We don't want to hardcode these clocks into the kernel (sunxi) clk
>> driver, instead the bootloader should tell the kernel about these clocks.
>>
>> So the only point of discussion left seems to be how to do 2...
>
> Wouldn't it be a lot simpler just to use existing fbdev (not KMS) and
> whip together a device specific driver that claims the proper
> resources? And just implement the minimal about of fbdev possible?
> fbdev already is a driver library.
Like... drivers/video/fbdev/offb.c?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-08-28 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FF1D6C.6090205@redhat.com>
On Thu, Aug 28, 2014 at 8:15 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 08/27/2014 04:16 PM, Thierry Reding wrote:
>
> <snip>
>
>>> The problems your talking about here all have to do with ordering how
>>> things are enabled, but what we're talking about here is keeping things
>>> enabled which are already enabled by the bootloader, so there is no
>>> ordering problem.
>>
>> See what I did there? I tricked you into saying what I've been saying
>> all along. =)
>
> He he :)
>
>> It's about keeping things enabled which have been enabled
>> by the bootloader. That's the root of the problem.
>
> Ok so we all seem to largely agree on most things here:
>
> 1) Somehow some clks must be marked used so as to not get disabled
> 2) We don't want to hardcode these clocks into the kernel (sunxi) clk
> driver, instead the bootloader should tell the kernel about these clocks.
>
> So the only point of discussion left seems to be how to do 2...
Wouldn't it be a lot simpler just to use existing fbdev (not KMS) and
whip together a device specific driver that claims the proper
resources? And just implement the minimal about of fbdev possible?
fbdev already is a driver library.
---
An independent issue is the early printk equivalent. For that you need
an address and the x/y layout
>
> <snip>
>
>>> Hmm I see, in my mind the problem is not that the clk framework disables
>>> unused clocks, but that no one is marking the clocks in question as used.
>>> Someone should mark these clocks as used so that they do not get disabled.
>>>
>>> We could add a clk_mark_in_use function and have the simplefb patch call
>>> that instead of clk_prepare_and_enable, or maybe even better just only
>>> claim the clks and teach the clk framework to not disable claimed clk
>>> in its cleanup run. That would make it clear that simplefb is not enabling
>>> anything, just claiming resource its need to avoid them getting removed
>>> from underneath simplefb, would that work for you ?
>>
>> That would be more accurate, but it boils down to the same problem where
>> we still need a driver to claim the resources in some way whereas the
>> problem is fundamentally one where the bootloader should be telling the
>> kernel not to disable it. It's in fact the bootloader that's claiming
>> the resources.
>
> Yes, but those resources do not belong to the bootloader in a sense
> that traditional bootloader / firmware claimed resources (e.g. acpi
> reserved resources) do. These traditional resources are claimed for ever.
>
> Where as these resources are claimed by the bootloader to keep the simplefb
> it provides working, as soon as the simplefb is no longer used, they become
> unused.
>
> <snip off-topic generic-ehci discussion>
>
>>> No, there are a lot of other drivers which were written before someone
>>> decided that having 10-20 drivers which were 90% copy and paste of each
>>> other was a bad idea, but we're really going offtopic here.
>>
>> The copy and paste is for code that's platform specific. The clocks have
>> different names, resets are different, supplies are different. The fact
>> that many can currently use the same driver is likely just coincidence
>> rather than design and it's entirely possible that at some point they'll
>> add support for a more advanced feature that makes them incompatible
>> with the rest of the generic drivers. And then you have a big mess
>> because you need to add quirks all over the place.
>>
>> And this isn't all that far off-topic, since simplefb also needs to deal
>> with this kind of situation. And what I've been arguing is that in order
>> to really be generic it has to make assumptions, one of which is that it
>> uses only resources that it doesn't need to explicitly handle.
>
> I can understand that you're worried about generic ?hci drivers dealing with
> clocks / resets / etc. As there may be strict ordering requirements there,
> but for simplefb that is not the case.
>
> All we're asking for is for a way to communicate 2 things to the kernel:
>
> 1) These resources are in use (we are not asking the kernel to do anything
> with them, rather the opposite, we're asking to leave them alone so no
> ordering issues)
>
> 2) Tie these resources to simplefb so that the kernel can know when they
> are no longer in use, and it may e.g. re-use the memory
>
> To me the most logical and also most "correct" way of modelling this is to
> put the resources inside the simplefb dt node.
>
> <snip>
>
>>> The key word here is "the used resources" to me this is not about simlefb
>>> managing resources, but marking them as used as long as it needs them, like
>>> it will need to do for the reserved mem chunk.
>>
>> The difference between the clocks and the memory resource is that the
>> driver needs to directly access the memory (it needs to map it and
>> provide a userspace mapping for it) whereas it doesn't need to touch the
>> clocks (except to workaround a Linux-specific implementation detail).
>
> Erm, no, the need to map the memory and the memory being a resource
> which may be released are an orthogonal problem. E.g. a system with
> dedicated framebuffer memory won't need to use a reserved main-memory
> chunk, nor need to worry about returning that mem when simplefb is no
> longer in use.
>
> Regards,
>
> Hans
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-08-28 12:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827141632.GB32243@ulmo>
Hi,
On 08/27/2014 04:16 PM, Thierry Reding wrote:
<snip>
>> The problems your talking about here all have to do with ordering how
>> things are enabled, but what we're talking about here is keeping things
>> enabled which are already enabled by the bootloader, so there is no
>> ordering problem.
>
> See what I did there? I tricked you into saying what I've been saying
> all along. =)
He he :)
> It's about keeping things enabled which have been enabled
> by the bootloader. That's the root of the problem.
Ok so we all seem to largely agree on most things here:
1) Somehow some clks must be marked used so as to not get disabled
2) We don't want to hardcode these clocks into the kernel (sunxi) clk
driver, instead the bootloader should tell the kernel about these clocks.
So the only point of discussion left seems to be how to do 2...
<snip>
>> Hmm I see, in my mind the problem is not that the clk framework disables
>> unused clocks, but that no one is marking the clocks in question as used.
>> Someone should mark these clocks as used so that they do not get disabled.
>>
>> We could add a clk_mark_in_use function and have the simplefb patch call
>> that instead of clk_prepare_and_enable, or maybe even better just only
>> claim the clks and teach the clk framework to not disable claimed clk
>> in its cleanup run. That would make it clear that simplefb is not enabling
>> anything, just claiming resource its need to avoid them getting removed
>> from underneath simplefb, would that work for you ?
>
> That would be more accurate, but it boils down to the same problem where
> we still need a driver to claim the resources in some way whereas the
> problem is fundamentally one where the bootloader should be telling the
> kernel not to disable it. It's in fact the bootloader that's claiming
> the resources.
Yes, but those resources do not belong to the bootloader in a sense
that traditional bootloader / firmware claimed resources (e.g. acpi
reserved resources) do. These traditional resources are claimed for ever.
Where as these resources are claimed by the bootloader to keep the simplefb
it provides working, as soon as the simplefb is no longer used, they become
unused.
<snip off-topic generic-ehci discussion>
>> No, there are a lot of other drivers which were written before someone
>> decided that having 10-20 drivers which were 90% copy and paste of each
>> other was a bad idea, but we're really going offtopic here.
>
> The copy and paste is for code that's platform specific. The clocks have
> different names, resets are different, supplies are different. The fact
> that many can currently use the same driver is likely just coincidence
> rather than design and it's entirely possible that at some point they'll
> add support for a more advanced feature that makes them incompatible
> with the rest of the generic drivers. And then you have a big mess
> because you need to add quirks all over the place.
>
> And this isn't all that far off-topic, since simplefb also needs to deal
> with this kind of situation. And what I've been arguing is that in order
> to really be generic it has to make assumptions, one of which is that it
> uses only resources that it doesn't need to explicitly handle.
I can understand that you're worried about generic ?hci drivers dealing with
clocks / resets / etc. As there may be strict ordering requirements there,
but for simplefb that is not the case.
All we're asking for is for a way to communicate 2 things to the kernel:
1) These resources are in use (we are not asking the kernel to do anything
with them, rather the opposite, we're asking to leave them alone so no
ordering issues)
2) Tie these resources to simplefb so that the kernel can know when they
are no longer in use, and it may e.g. re-use the memory
To me the most logical and also most "correct" way of modelling this is to
put the resources inside the simplefb dt node.
<snip>
>> The key word here is "the used resources" to me this is not about simlefb
>> managing resources, but marking them as used as long as it needs them, like
>> it will need to do for the reserved mem chunk.
>
> The difference between the clocks and the memory resource is that the
> driver needs to directly access the memory (it needs to map it and
> provide a userspace mapping for it) whereas it doesn't need to touch the
> clocks (except to workaround a Linux-specific implementation detail).
Erm, no, the need to map the memory and the memory being a resource
which may be released are an orthogonal problem. E.g. a system with
dedicated framebuffer memory won't need to use a reserved main-memory
chunk, nor need to worry about returning that mem when simplefb is no
longer in use.
Regards,
Hans
^ permalink raw reply
* [PATCH v2 3/7] Adding Skyworks SKY81452 backlight driver
From: Gyungoh Yoo @ 2014-08-28 10:49 UTC (permalink / raw)
To: jg1.han-Sze3O3UU22JBDgjK7y7TUQ, cooloney-Re5JQEeQqe8AvxtiuMwx3w,
lee.jones-QSEj5FYQhm4dnm+yROfE0A
Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jack.yoo-tjhQNA90jdKqndwCJWfcng,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409222307-23225-1-git-send-email-jack.yoo-tjhQNA90jdKqndwCJWfcng@public.gmane.org>
v2:
Added 'compatible' attribute in child drivers
Added messages for exception or errors.
---
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 347 +++++++++++++++++++++++++++
include/linux/sky81452-backlight.h | 47 ++++
4 files changed, 405 insertions(+)
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/sky81452-backlight.h
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8d03924..2586fdd 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -409,6 +409,16 @@ config BACKLIGHT_PANDORA
If you have a Pandora console, say Y to enable the
backlight driver.
+config BACKLIGHT_SKY81452
+ tristate "Backlight driver for SKY81452"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_SKY81452
+ help
+ If you have a Skyworks SKY81452, say Y to enable the
+ backlight driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sky81452-backlight
+
config BACKLIGHT_TPS65217
tristate "TPS65217 Backlight"
depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index fcd50b73..d67073f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c
new file mode 100644
index 0000000..d292026
--- /dev/null
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -0,0 +1,347 @@
+/*
+ * sky81452-backlight.c SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/backlight.h>
+#include <linux/sky81452-backlight.h>
+
+/* registers */
+#define SKY81452_REG0 0x00
+#define SKY81452_REG1 0x01
+#define SKY81452_REG2 0x02
+#define SKY81452_REG4 0x04
+#define SKY81452_REG5 0x05
+
+/* bit mask */
+#define SKY81452_CS 0xFF
+#define SKY81452_EN 0x3F
+#define SKY81452_IGPW 0x20
+#define SKY81452_PWMMD 0x10
+#define SKY81452_PHASE 0x08
+#define SKY81452_ILIM 0x04
+#define SKY81452_VSHRT 0x03
+#define SKY81452_OCP 0x80
+#define SKY81452_OTMP 0x40
+#define SKY81452_SHRT 0x3F
+#define SKY81452_OPN 0x3F
+
+#define SKY81452_DEFAULT_NAME "lcd-backlight"
+#define SKY81452_MAX_BRIGHTNESS (SKY81452_CS + 1)
+
+#define CTZ(b) __builtin_ctz(b)
+
+static int sky81452_bl_update_status(struct backlight_device *bd)
+{
+ const struct sky81452_bl_platform_data *pdata + dev_get_platdata(bd->dev.parent);
+ const unsigned int brightness = (unsigned int)bd->props.brightness;
+ struct regmap *regmap = bl_get_data(bd);
+ int ret;
+
+ if (brightness > 0) {
+ ret = regmap_write(regmap, SKY81452_REG0, brightness - 1);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ pdata->enable << CTZ(SKY81452_EN));
+ }
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN, 0);
+}
+
+static int sky81452_bl_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static const struct backlight_ops sky81452_bl_ops = {
+ .update_status = sky81452_bl_update_status,
+ .get_brightness = sky81452_bl_get_brightness,
+};
+
+static ssize_t sky81452_bl_store_enable(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned long value;
+ int ret;
+
+ ret = kstrtoul(buf, 16, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ ret = regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ value << CTZ(SKY81452_EN));
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return count;
+}
+
+static ssize_t sky81452_bl_show_open_short(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int reg, value = 0;
+ char tmp[3];
+ int i, ret;
+
+ reg = !strcmp(attr->attr.name, "open") ? SKY81452_REG5 : SKY81452_REG4;
+ ret = regmap_read(regmap, reg, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ if (value & SKY81452_SHRT) {
+ *buf = 0;
+ for (i = 0; i < 6; i++) {
+ if (value & 0x01) {
+ sprintf(tmp, "%d ", i + 1);
+ strcat(buf, tmp);
+ }
+ value >>= 1;
+ }
+ strcat(buf, "\n");
+ } else
+ strcpy(buf, "none\n");
+
+ return strlen(buf);
+}
+
+static ssize_t sky81452_bl_show_fault(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int value = 0;
+ int ret;
+
+ ret = regmap_read(regmap, SKY81452_REG4, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ *buf = 0;
+
+ if (value & SKY81452_OCP)
+ strcat(buf, "over-current ");
+
+ if (value & SKY81452_OTMP)
+ strcat(buf, "over-temperature");
+
+ strcat(buf, "\n");
+ return strlen(buf);
+}
+
+static DEVICE_ATTR(enable, S_IWGRP | S_IWUSR, NULL, sky81452_bl_store_enable);
+static DEVICE_ATTR(open, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(short, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(fault, S_IRUGO, sky81452_bl_show_fault, NULL);
+
+static struct attribute *sky81452_bl_attribute[] = {
+ &dev_attr_enable.attr,
+ &dev_attr_open.attr,
+ &dev_attr_short.attr,
+ &dev_attr_fault.attr,
+ NULL
+};
+
+static const struct attribute_group sky81452_bl_attr_group = {
+ .attrs = sky81452_bl_attribute,
+};
+
+#ifdef CONFIG_OF
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ struct device_node *np = of_node_get(dev->of_node);
+ struct sky81452_bl_platform_data *pdata;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "backlight node not found");
+ return ERR_PTR(-ENODATA);
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ of_node_put(np);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ of_property_read_string(np, "name", &pdata->name);
+ pdata->ignore_pwm = of_property_read_bool(np, "ignore-pwm");
+ pdata->dpwm_mode = of_property_read_bool(np, "dpwm-mode");
+ pdata->phase_shift = of_property_read_bool(np, "phase-shift");
+
+ pdata->gpio_enable = of_get_named_gpio(np, "gpio-enable", 0);
+ if (IS_ERR_VALUE(pdata->gpio_enable))
+ pdata->gpio_enable = -1;
+
+ ret = of_property_read_u32(np, "enable", &pdata->enable);
+ if (IS_ERR_VALUE(ret))
+ pdata->enable = SKY81452_EN >> CTZ(SKY81452_EN);
+
+ ret = of_property_read_u32(np, "short-detection-threshold",
+ &pdata->short_detection_threshold);
+ if (IS_ERR_VALUE(ret))
+ pdata->short_detection_threshold = 7;
+
+ ret = of_property_read_u32(np, "boost-current-limit",
+ &pdata->boost_current_limit);
+ if (IS_ERR_VALUE(ret))
+ pdata->boost_current_limit = 2750;
+
+ of_node_put(np);
+ return pdata;
+}
+#else
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
+
+static int sky81452_bl_init_device(struct regmap *regmap,
+ struct sky81452_bl_platform_data *pdata)
+{
+ unsigned int value;
+
+ value = pdata->ignore_pwm ? SKY81452_IGPW : 0;
+ value |= pdata->dpwm_mode ? SKY81452_PWMMD : 0;
+ value |= pdata->phase_shift ? 0 : SKY81452_PHASE;
+
+ if (pdata->boost_current_limit = 2300)
+ value |= SKY81452_ILIM;
+ else if (pdata->boost_current_limit != 2720)
+ return -EINVAL;
+
+ if (pdata->short_detection_threshold < 4 ||
+ pdata->short_detection_threshold > 7)
+ return -EINVAL;
+ value |= (7 - pdata->short_detection_threshold) << CTZ(SKY81452_VSHRT);
+
+ return regmap_write(regmap, SKY81452_REG2, value);
+}
+
+static int sky81452_bl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap = dev_get_drvdata(dev->parent);
+ struct sky81452_bl_platform_data *pdata = dev_get_platdata(dev);
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ const char *name;
+ int ret;
+
+ if (!pdata) {
+ pdata = sky81452_bl_parse_dt(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
+
+ if (pdata->gpio_enable >= 0) {
+ ret = devm_gpio_request_one(dev, pdata->gpio_enable,
+ GPIOF_OUT_INIT_HIGH, "sky81452-en");
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to request GPIO:%d", ret);
+ return ret;
+ }
+ }
+
+ ret = sky81452_bl_init_device(regmap, pdata);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to initialize device:%d", ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.max_brightness = SKY81452_MAX_BRIGHTNESS,
+ name = pdata->name ? pdata->name : SKY81452_DEFAULT_NAME;
+ bd = devm_backlight_device_register(dev, name, dev, regmap,
+ &sky81452_bl_ops, &props);
+ if (IS_ERR(bd)) {
+ dev_err(dev, "failed to register backlight:%d", PTR_ERR(bd));
+ return PTR_ERR(bd);
+ }
+
+ platform_set_drvdata(pdev, bd);
+
+ ret = sysfs_create_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to create attribute:%d", ret);
+ goto err;
+ }
+
+ return ret;
+err:
+ backlight_device_unregister(bd);
+ return ret;
+}
+
+static int sky81452_bl_remove(struct platform_device *pdev)
+{
+ const struct sky81452_bl_platform_data *pdata + dev_get_platdata(&pdev->dev);
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ sysfs_remove_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+
+ bd->props.power = FB_BLANK_UNBLANK;
+ bd->props.brightness = 0;
+ backlight_update_status(bd);
+
+ if (pdata->gpio_enable >= 0)
+ gpio_set_value_cansleep(pdata->gpio_enable, 0);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_bl_of_match[] = {
+ { .compatible = "skyworks,sky81452-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_bl_of_match);
+#endif
+
+static struct platform_driver sky81452_bl_driver = {
+ .driver = {
+ .name = "sky81452-backlight",
+ .of_match_table = of_match_ptr(sky81452_bl_of_match),
+ },
+ .probe = sky81452_bl_probe,
+ .remove = sky81452_bl_remove,
+};
+
+module_platform_driver(sky81452_bl_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 backlight driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.1");
diff --git a/include/linux/sky81452-backlight.h b/include/linux/sky81452-backlight.h
new file mode 100644
index 0000000..b3635f9
--- /dev/null
+++ b/include/linux/sky81452-backlight.h
@@ -0,0 +1,47 @@
+/*
+ * sky81452.h SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_BACKLIGHT_H
+#define _SKY81452_BACKLIGHT_H
+
+/**
+ * struct sky81452_platform_data
+ * @name: backlight driver name.
+ If it is not defined, default name is lcd-backlight.
+ * @gpio_enable:GPIO number which control EN pin
+ * @enable: Enable mask for current sink channel 1, 2, 3, 4, 5 and 6.
+ * @ignore_pwm: true if DPWMI should be ignored.
+ * @dpwm_mode: true is DPWM dimming mode, otherwise Analog dimming mode.
+ * @phase_shift:true is phase shift mode.
+ * @short_detecion_threshold: It should be one of 4, 5, 6 and 7V.
+ * @boost_current_limit: It should be one of 2300, 2750mA.
+ */
+struct sky81452_bl_platform_data {
+ const char *name;
+ int gpio_enable;
+ unsigned int enable;
+ bool ignore_pwm;
+ bool dpwm_mode;
+ bool phase_shift;
+ unsigned int short_detection_threshold;
+ unsigned int boost_current_limit;
+};
+
+#endif
--
1.9.1
^ permalink raw reply related
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-08-28 10:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140828101140.GB14388@ulmo>
On 28 August 2014 12:11, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 05:42:21PM +0200, Maxime Ripard wrote:
>> On Wed, Aug 27, 2014 at 11:52:48AM +0200, Thierry Reding wrote:
>> > On Wed, Aug 27, 2014 at 10:45:26AM +0200, Maxime Ripard wrote:
>> > > On Wed, Aug 27, 2014 at 08:54:41AM +0200, Thierry Reding wrote:
>> > > > On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
>> > > > > On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
>> > [...]
>> > > > > > > Mike Turquette repeatedly said that he was against such a DT property:
>> > > > > > > https://lkml.org/lkml/2014/5/12/693
>> > > > > >
>> > > > > > Mike says in that email that he's opposing the addition of a property
>> > > > > > for clocks that is the equivalent of regulator-always-on. That's not
>> > > > > > what this is about. If at all it'd be a property to mark a clock that
>> > > > > > should not be disabled by default because it's essential.
>> > > > >
>> > > > > It's just semantic. How is "a clock that should not be disabled by
>> > > > > default because it's essential" not a clock that stays always on?
>> > > >
>> > > > Because a clock that should not be disabled by default can be turned off
>> > > > when appropriate. A clock that is always on can't be turned off.
>> > >
>> > > If a clock is essential, then it should never be disabled. Or we don't
>> > > share the same meaning of essential.
>> >
>> > Essential for the particular use-case.
>>
>> So, how would the clock driver would know about which use case we're
>> in? How would it know about which display engine is currently running?
>> How would it know about which video output is being set?
>>
>> Currently, we have two separate display engines, which can each output
>> either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
>> one of these combinations would require different clocks. What clocks
>> will we put in the driver? All of them?
>
> Ideally the solution wouldn't involve hard-coding this into the clock
> driver at all. There should be a way for firmware to communicate to the
> kernel that a given clock shouldn't be disabled. Then since firmware
> already knows what it set up it can tell the kernel to not touch those.
>
And that way is that it inserts into the simplefb node or whatever
node the list of clocks that it programmed in order to make the
framebuffer work. Do you know a better, more generic way than that?
Thanks
Michal
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-28 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827154221.GX15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]
On Wed, Aug 27, 2014 at 05:42:21PM +0200, Maxime Ripard wrote:
> On Wed, Aug 27, 2014 at 11:52:48AM +0200, Thierry Reding wrote:
> > On Wed, Aug 27, 2014 at 10:45:26AM +0200, Maxime Ripard wrote:
> > > On Wed, Aug 27, 2014 at 08:54:41AM +0200, Thierry Reding wrote:
> > > > On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
> > > > > On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
> > [...]
> > > > > > > Mike Turquette repeatedly said that he was against such a DT property:
> > > > > > > https://lkml.org/lkml/2014/5/12/693
> > > > > >
> > > > > > Mike says in that email that he's opposing the addition of a property
> > > > > > for clocks that is the equivalent of regulator-always-on. That's not
> > > > > > what this is about. If at all it'd be a property to mark a clock that
> > > > > > should not be disabled by default because it's essential.
> > > > >
> > > > > It's just semantic. How is "a clock that should not be disabled by
> > > > > default because it's essential" not a clock that stays always on?
> > > >
> > > > Because a clock that should not be disabled by default can be turned off
> > > > when appropriate. A clock that is always on can't be turned off.
> > >
> > > If a clock is essential, then it should never be disabled. Or we don't
> > > share the same meaning of essential.
> >
> > Essential for the particular use-case.
>
> So, how would the clock driver would know about which use case we're
> in? How would it know about which display engine is currently running?
> How would it know about which video output is being set?
>
> Currently, we have two separate display engines, which can each output
> either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
> one of these combinations would require different clocks. What clocks
> will we put in the driver? All of them?
Ideally the solution wouldn't involve hard-coding this into the clock
driver at all. There should be a way for firmware to communicate to the
kernel that a given clock shouldn't be disabled. Then since firmware
already knows what it set up it can tell the kernel to not touch those.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-28 10:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMqctSY7AJKHthe+aG8--Ok6JiY1g2zM_tN10S2nELOSe=qMw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]
On Wed, Aug 27, 2014 at 10:57:29PM +0200, Michal Suchanek wrote:
> Hello,
>
> On 27 August 2014 17:42, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > On Wed, Aug 27, 2014 at 11:52:48AM +0200, Thierry Reding wrote:
> >> On Wed, Aug 27, 2014 at 10:45:26AM +0200, Maxime Ripard wrote:
> >> > On Wed, Aug 27, 2014 at 08:54:41AM +0200, Thierry Reding wrote:
> >> > > On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
> >> > > > On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
> >> [...]
> >> > > > > > Mike Turquette repeatedly said that he was against such a DT property:
> >> > > > > > https://lkml.org/lkml/2014/5/12/693
> >> > > > >
> >> > > > > Mike says in that email that he's opposing the addition of a property
> >> > > > > for clocks that is the equivalent of regulator-always-on. That's not
> >> > > > > what this is about. If at all it'd be a property to mark a clock that
> >> > > > > should not be disabled by default because it's essential.
> >> > > >
> >> > > > It's just semantic. How is "a clock that should not be disabled by
> >> > > > default because it's essential" not a clock that stays always on?
> >> > >
> >> > > Because a clock that should not be disabled by default can be turned off
> >> > > when appropriate. A clock that is always on can't be turned off.
> >> >
> >> > If a clock is essential, then it should never be disabled. Or we don't
> >> > share the same meaning of essential.
> >>
> >> Essential for the particular use-case.
> >
> > So, how would the clock driver would know about which use case we're
> > in? How would it know about which display engine is currently running?
> > How would it know about which video output is being set?
> >
> > Currently, we have two separate display engines, which can each output
> > either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
> > one of these combinations would require different clocks. What clocks
> > will we put in the driver? All of them?
> >
>
> since simplefb cannot be extended how about adding, say, dtfb which
> claims the resources from dt and then instantiates a simplefb once the
> resources are claimed? That is have a dtfb which has the clocks
> assigned and has simplefb as child dt node.
I don't see how that changes anything. All you do is add another layer
of indirection. The fundamental problem remains the same and isn't
solved.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] backlight: generic_bl: Remove unused function
From: Lee Jones @ 2014-08-28 7:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1409168700-19042-1-git-send-email-festevam@gmail.com>
On Wed, 27 Aug 2014, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> genericbl_limit_intensity() is exported, but it is never
> called anywhere else.
>
> Fix the following sparse warning:
>
> drivers/video/backlight/generic_bl.c:59:6: warning: symbol 'genericbl_limit_intensity' was not declared. Should it be static?
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/video/backlight/generic_bl.c | 18 ------------------
> 1 file changed, 18 deletions(-)
Applied, thanks.
> diff --git a/drivers/video/backlight/generic_bl.c b/drivers/video/backlight/generic_bl.c
> index 5d8d652..67dfb93 100644
> --- a/drivers/video/backlight/generic_bl.c
> +++ b/drivers/video/backlight/generic_bl.c
> @@ -52,24 +52,6 @@ static int genericbl_get_intensity(struct backlight_device *bd)
> return genericbl_intensity;
> }
>
> -/*
> - * Called when the battery is low to limit the backlight intensity.
> - * If limit=0 clear any limit, otherwise limit the intensity
> - */
> -void genericbl_limit_intensity(int limit)
> -{
> - struct backlight_device *bd = generic_backlight_device;
> -
> - mutex_lock(&bd->ops_lock);
> - if (limit)
> - bd->props.state |= GENERICBL_BATTLOW;
> - else
> - bd->props.state &= ~GENERICBL_BATTLOW;
> - backlight_update_status(generic_backlight_device);
> - mutex_unlock(&bd->ops_lock);
> -}
> -EXPORT_SYMBOL(genericbl_limit_intensity);
> -
> static const struct backlight_ops genericbl_ops = {
> .options = BL_CORE_SUSPENDRESUME,
> .get_brightness = genericbl_get_intensity,
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-08-27 20:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827154221.GX15297@lukather>
Hello,
On 27 August 2014 17:42, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Wed, Aug 27, 2014 at 11:52:48AM +0200, Thierry Reding wrote:
>> On Wed, Aug 27, 2014 at 10:45:26AM +0200, Maxime Ripard wrote:
>> > On Wed, Aug 27, 2014 at 08:54:41AM +0200, Thierry Reding wrote:
>> > > On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
>> > > > On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
>> [...]
>> > > > > > Mike Turquette repeatedly said that he was against such a DT property:
>> > > > > > https://lkml.org/lkml/2014/5/12/693
>> > > > >
>> > > > > Mike says in that email that he's opposing the addition of a property
>> > > > > for clocks that is the equivalent of regulator-always-on. That's not
>> > > > > what this is about. If at all it'd be a property to mark a clock that
>> > > > > should not be disabled by default because it's essential.
>> > > >
>> > > > It's just semantic. How is "a clock that should not be disabled by
>> > > > default because it's essential" not a clock that stays always on?
>> > >
>> > > Because a clock that should not be disabled by default can be turned off
>> > > when appropriate. A clock that is always on can't be turned off.
>> >
>> > If a clock is essential, then it should never be disabled. Or we don't
>> > share the same meaning of essential.
>>
>> Essential for the particular use-case.
>
> So, how would the clock driver would know about which use case we're
> in? How would it know about which display engine is currently running?
> How would it know about which video output is being set?
>
> Currently, we have two separate display engines, which can each output
> either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
> one of these combinations would require different clocks. What clocks
> will we put in the driver? All of them?
>
since simplefb cannot be extended how about adding, say, dtfb which
claims the resources from dt and then instantiates a simplefb once the
resources are claimed? That is have a dtfb which has the clocks
assigned and has simplefb as child dt node.
Thanks
Michal
^ permalink raw reply
* [PATCH] backlight: generic_bl: Remove unused function
From: Fabio Estevam @ 2014-08-27 19:45 UTC (permalink / raw)
To: linux-fbdev
From: Fabio Estevam <fabio.estevam@freescale.com>
genericbl_limit_intensity() is exported, but it is never
called anywhere else.
Fix the following sparse warning:
drivers/video/backlight/generic_bl.c:59:6: warning: symbol 'genericbl_limit_intensity' was not declared. Should it be static?
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/video/backlight/generic_bl.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/video/backlight/generic_bl.c b/drivers/video/backlight/generic_bl.c
index 5d8d652..67dfb93 100644
--- a/drivers/video/backlight/generic_bl.c
+++ b/drivers/video/backlight/generic_bl.c
@@ -52,24 +52,6 @@ static int genericbl_get_intensity(struct backlight_device *bd)
return genericbl_intensity;
}
-/*
- * Called when the battery is low to limit the backlight intensity.
- * If limit=0 clear any limit, otherwise limit the intensity
- */
-void genericbl_limit_intensity(int limit)
-{
- struct backlight_device *bd = generic_backlight_device;
-
- mutex_lock(&bd->ops_lock);
- if (limit)
- bd->props.state |= GENERICBL_BATTLOW;
- else
- bd->props.state &= ~GENERICBL_BATTLOW;
- backlight_update_status(generic_backlight_device);
- mutex_unlock(&bd->ops_lock);
-}
-EXPORT_SYMBOL(genericbl_limit_intensity);
-
static const struct backlight_ops genericbl_ops = {
.options = BL_CORE_SUSPENDRESUME,
.get_brightness = genericbl_get_intensity,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 03/16] video: Add DT binding documentation for VGA connector
From: Laurent Pinchart @ 2014-08-27 17:22 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, linux-fbdev@vger.kernel.org,
Laurent Pinchart, dri-devel, SH-Linux
In-Reply-To: <CAL_JsqLOzrgncpTa3SqMjc=XeYuvXhGrvk9dKA+Fgh_g_wycCA@mail.gmail.com>
Hi Rob,
On Wednesday 27 August 2014 12:12:36 Rob Herring wrote:
> On Wed, Aug 27, 2014 at 11:41 AM, Laurent Pinchart wrote:
> > The VGA connector is described by a single input port and an optional
> > DDC bus.
>
> Wasn't there a generic connector binding for DVI, HDMI, etc.?
As far as I know, there are three separate generic bindings for DVI
connectors, HDMI connectors, and analog TV connectors. The VGA connector
doesn't seem to really fit into one of those categories.
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-fbdev@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >
> > .../devicetree/bindings/video/vga-connector.txt | 28
> > ++++++++++++++++++++++ 1 file changed, 28 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/video/vga-connector.txt>
> > diff --git a/Documentation/devicetree/bindings/video/vga-connector.txt
> > b/Documentation/devicetree/bindings/video/vga-connector.txt new file mode
> > 100644
> > index 0000000..9a45ec1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/vga-connector.txt
> > @@ -0,0 +1,28 @@
> > +VGA Connector
> > +=======
> > +
> > +Required properties:
> > +- compatible: "vga-connector"
> > +
> > +Optional properties:
> > +- label: a symbolic name for the connector
>
> ...which corresponds to hardware labels.
>
> > +- ddc-i2c-bus: phandle to the I2C bus that is connected to VGA DDC
> > +
> > +Required nodes:
> > +- Video port for VGA input
>
> A reference to the relevant video graph bindings should be added here.
I'll fix that.
> > +
> > +Example
> > +-------
> > +
> > +vga0: connector@0 {
> > + compatible = "vga-connector";
> > + label = "vga";
> > +
> > + ddc-i2c-bus = <&i2c3>;
> > +
> > + port {
> > + vga_connector_in: endpoint {
> > + remote-endpoint = <&adv7123_out>;
> > + };
> > + };
> > +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 03/16] video: Add DT binding documentation for VGA connector
From: Rob Herring @ 2014-08-27 17:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, SH-Linux, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org
In-Reply-To: <1409157673-4154-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
On Wed, Aug 27, 2014 at 11:41 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The VGA connector is described by a single input port and an optional
> DDC bus.
Wasn't there a generic connector binding for DVI, HDMI, etc.?
>
> Cc: devicetree@vger.kernel.org
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> .../devicetree/bindings/video/vga-connector.txt | 28 ++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/vga-connector.txt
>
> diff --git a/Documentation/devicetree/bindings/video/vga-connector.txt b/Documentation/devicetree/bindings/video/vga-connector.txt
> new file mode 100644
> index 0000000..9a45ec1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/vga-connector.txt
> @@ -0,0 +1,28 @@
> +VGA Connector
> +=======
> +
> +Required properties:
> +- compatible: "vga-connector"
> +
> +Optional properties:
> +- label: a symbolic name for the connector
...which corresponds to hardware labels.
> +- ddc-i2c-bus: phandle to the I2C bus that is connected to VGA DDC
> +
> +Required nodes:
> +- Video port for VGA input
A reference to the relevant video graph bindings should be added here.
> +
> +Example
> +-------
> +
> +vga0: connector@0 {
> + compatible = "vga-connector";
> + label = "vga";
> +
> + ddc-i2c-bus = <&i2c3>;
> +
> + port {
> + vga_connector_in: endpoint {
> + remote-endpoint = <&adv7123_out>;
> + };
> + };
> +};
> --
> 1.8.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 06/16] video: Add DT bindings for the R-Car Display Unit
From: Laurent Pinchart @ 2014-08-27 16:41 UTC (permalink / raw)
To: dri-devel; +Cc: linux-sh, devicetree, linux-fbdev
In-Reply-To: <1409157673-4154-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Aside of the usual boring core properties (compatible, reg, interrupts
and clocks), the bindings use the OF graph bindings to model connections
between the DU output video ports and the on-board and off-board
components.
Cc: devicetree@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../devicetree/bindings/video/renesas,du.txt | 84 ++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/renesas,du.txt
diff --git a/Documentation/devicetree/bindings/video/renesas,du.txt b/Documentation/devicetree/bindings/video/renesas,du.txt
new file mode 100644
index 0000000..5102830
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/renesas,du.txt
@@ -0,0 +1,84 @@
+* Renesas R-Car Display Unit (DU)
+
+Required Properties:
+
+ - compatible: must be one of the following.
+ - "renesas,du-r8a7779" for R8A7779 (R-Car H1) compatible DU
+ - "renesas,du-r8a7790" for R8A7790 (R-Car H2) compatible DU
+ - "renesas,du-r8a7791" for R8A7791 (R-Car M2) compatible DU
+
+ - reg: A list of base address and length of each memory resource, one for
+ each entry in the reg-names property.
+ - reg-names: Name of the memory resources. The DU requires one memory
+ resource for the DU core (named "du") and one memory resource for each
+ LVDS encoder (named "lvds.x" with "x" being the LVDS controller numerical
+ index).
+
+ - interrupt-parent: phandle of the parent interrupt controller.
+ - interrupts: Interrupt specifiers for the DU interrupts.
+
+ - clocks: A list of phandles + clock-specifier pairs, one for each entry in
+ the clock-names property.
+ - clock-names: Name of the clocks. This property is model-dependent.
+ - R8A7779 uses a single functional clock. The clock doesn't need to be
+ named.
+ - R8A7790 and R8A7791 use one functional clock per channel and one clock
+ per LVDS encoder. The functional clocks must be named "du.x" with "x"
+ being the channel numerical index. The LVDS clocks must be named
+ "lvds.x" with "x" being the LVDS encoder numerical index.
+
+Required nodes:
+
+The connections to the DU output video ports are modeled using the OF graph
+bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+The following table lists for each supported model the port number
+corresponding to each DU output.
+
+ Port 0 Port1 Port2
+-----------------------------------------------------------------------------
+ R8A7779 (H1) DPAD 0 DPAD 1 -
+ R8A7790 (H2) DPAD LVDS 0 LVDS 1
+ R8A7791 (M2) DPAD LVDS 0 -
+
+
+Example: R8A7790 (R-Car H2) DU
+
+ du: du@feb00000 {
+ compatible = "renesas,du-r8a7790";
+ reg = <0 0xfeb00000 0 0x70000>,
+ <0 0xfeb90000 0 0x1c>,
+ <0 0xfeb94000 0 0x1c>;
+ reg-names = "du", "lvds.0", "lvds.1";
+ interrupt-parent = <&gic>;
+ interrupts = <0 256 IRQ_TYPE_LEVEL_HIGH>,
+ <0 268 IRQ_TYPE_LEVEL_HIGH>,
+ <0 269 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp7_clks R8A7790_CLK_DU0>,
+ <&mstp7_clks R8A7790_CLK_DU1>,
+ <&mstp7_clks R8A7790_CLK_DU2>,
+ <&mstp7_clks R8A7790_CLK_LVDS0>,
+ <&mstp7_clks R8A7790_CLK_LVDS1>;
+ clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ du_out_rgb: endpoint {
+ };
+ };
+ port@1 {
+ reg = <1>;
+ du_out_lvds0: endpoint {
+ };
+ };
+ port@2 {
+ reg = <2>;
+ du_out_lvds1: endpoint {
+ };
+ };
+ };
+ };
--
1.8.5.5
^ permalink raw reply related
* [PATCH 05/16] video: Add THC63LVDM83D DT bindings documentation
From: Laurent Pinchart @ 2014-08-27 16:41 UTC (permalink / raw)
To: dri-devel; +Cc: devicetree, linux-fbdev, linux-sh
In-Reply-To: <1409157673-4154-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
The THC63LVDM83D is a video LVDS serializer described by an input port,
an output port, and an optional power down GPIO.
Cc: devicetree@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../devicetree/bindings/video/thine,thc63lvdm83d | 50 ++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/thine,thc63lvdm83d
diff --git a/Documentation/devicetree/bindings/video/thine,thc63lvdm83d b/Documentation/devicetree/bindings/video/thine,thc63lvdm83d
new file mode 100644
index 0000000..527e236
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/thine,thc63lvdm83d
@@ -0,0 +1,50 @@
+THine Electronics THC63LVDM83D LVDS serializer
+----------------------------------------------
+
+The THC63LVDM83D is an LVDS serializer designed to support pixel data
+transmission between a host and a flat panel.
+
+Required properties:
+
+- compatible: Should be "thine,thc63lvdm83d"
+
+Optional properties:
+
+- pwdn-gpios: Power down control GPIO
+
+Required nodes:
+
+The THC63LVDM83D has two video ports. Their connections are modeled using the
+OFgraph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for CMOS/TTL input
+- Video port 1 for LVDS output
+
+
+Example
+-------
+
+ lvds_enc: encoder@0 {
+ compatible = "thine,thc63lvdm83d";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ lvds_enc_in: endpoint@0 {
+ remote-endpoint = <&rgb_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ lvds_enc_out: endpoint@0 {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+ };
--
1.8.5.5
^ permalink raw reply related
* [PATCH 04/16] video: Add ADV7123 DT bindings documentation
From: Laurent Pinchart @ 2014-08-27 16:41 UTC (permalink / raw)
To: dri-devel; +Cc: devicetree, linux-fbdev, linux-sh
In-Reply-To: <1409157673-4154-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
The ADV7123 is a video DAC described by an input port, an output port,
and an optional power save GPIO.
Cc: devicetree@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../devicetree/bindings/video/adi,adv7123.txt | 50 ++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/adi,adv7123.txt
diff --git a/Documentation/devicetree/bindings/video/adi,adv7123.txt b/Documentation/devicetree/bindings/video/adi,adv7123.txt
new file mode 100644
index 0000000..a6b2b2b
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/adi,adv7123.txt
@@ -0,0 +1,50 @@
+Analog Device ADV7123 Video DAC
+-------------------------------
+
+The ADV7123 is a digital-to-analog converter that outputs VGA signals from a
+parallel video input.
+
+Required properties:
+
+- compatible: Should be "adi,adv7123"
+
+Optional properties:
+
+- psave-gpios: Power save control GPIO
+
+Required nodes:
+
+The ADV7123 has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for DPI input
+- Video port 1 for VGA output
+
+
+Example
+-------
+
+ adv7123: encoder@0 {
+ compatible = "adi,adv7123";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ adv7123_in: endpoint@0 {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ adv7123_out: endpoint@0 {
+ remote-endpoint = <&vga_connector_in>;
+ };
+ };
+ };
+ };
--
1.8.5.5
^ permalink raw reply related
* [PATCH 03/16] video: Add DT binding documentation for VGA connector
From: Laurent Pinchart @ 2014-08-27 16:41 UTC (permalink / raw)
To: dri-devel; +Cc: devicetree, linux-fbdev, linux-sh
In-Reply-To: <1409157673-4154-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
The VGA connector is described by a single input port and an optional
DDC bus.
Cc: devicetree@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../devicetree/bindings/video/vga-connector.txt | 28 ++++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/vga-connector.txt
diff --git a/Documentation/devicetree/bindings/video/vga-connector.txt b/Documentation/devicetree/bindings/video/vga-connector.txt
new file mode 100644
index 0000000..9a45ec1
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/vga-connector.txt
@@ -0,0 +1,28 @@
+VGA Connector
+=======
+
+Required properties:
+- compatible: "vga-connector"
+
+Optional properties:
+- label: a symbolic name for the connector
+- ddc-i2c-bus: phandle to the I2C bus that is connected to VGA DDC
+
+Required nodes:
+- Video port for VGA input
+
+Example
+-------
+
+vga0: connector@0 {
+ compatible = "vga-connector";
+ label = "vga";
+
+ ddc-i2c-bus = <&i2c3>;
+
+ port {
+ vga_connector_in: endpoint {
+ remote-endpoint = <&adv7123_out>;
+ };
+ };
+};
--
1.8.5.5
^ permalink raw reply related
* [PATCH 00/16] R-Car Display Unit DT bindings
From: Laurent Pinchart @ 2014-08-27 16:40 UTC (permalink / raw)
To: dri-devel; +Cc: devicetree, linux-fbdev, linux-sh
Hello,
This patch series documents and implements DT bindings support for the R-Car
Display Unit (DU).
Unlike the previous attempt that tried to create a new model for composite
display devices and failed to get any real traction from DRM developers, the
approach taken here focuses on DT bindings without requiring core changes
outside of the DU driver.
Aside of the usual boring core properties (compatible, reg, interrupts and
clocks), the proposed bindings use the OF graph bindings to model connections
between the DU output video ports and the on-board and off-board components.
As such they do not depend on any particular implementation or implementation
philosophy.
The series starts by documenting DT bindings for the devices present on the
Marzen, Lager and Koelsch boards not already supported in the mainline kernel
(01/16 to 05/16) and for the DU (06/16). It then reworks the rcar_du platform
data to ease implementation of the DT bindings (07/16). The next step is to
implement support for the DU DT bindings in the R-Car DU DRM driver (08/16).
Finally the remaining patches add the DU DT nodes for all supported SoCs
(09/16 to 11/16), remove DU platform data from the DT-based board files (12/16
and 13/16) and describe the on-board devices connected to the DU outputs for
the Marzen, Lager and Koelsch boards (14/16 to 16/16).
Cc: devicetree@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Laurent Pinchart (16):
devicetree: Add vendor prefix "mitsubishi" to vendor-prefixes.txt
devicetree: Add vendor prefix "thine" to vendor-prefixes.txt
video: Add DT binding documentation for VGA connector
video: Add ADV7123 DT bindings documentation
video: Add THC63LVDM83D DT bindings documentation
video: Add DT bindings for the R-Car Display Unit
drm/rcar-du: Use struct videomode in platform data
drm/rcar-du: Add OF support
ARM: shmobile: r8a7779: Add DU node to device tree
ARM: shmobile: r8a7790: Add DU node to device tree
ARM: shmobile: r8a7791: Add DU node to device tree
ARM: shmobile: lager-reference: Remove DU platform device
ARM: shmobile: koelsch-reference: Remove DU platform device
ARM: shmobile: marzen: Enable DU device in DT
ARM: shmobile: lager: Enable DU device in DT
ARM: shmobile: koelsch: Enable DU device in DT
.../devicetree/bindings/vendor-prefixes.txt | 2 +
.../devicetree/bindings/video/adi,adv7123.txt | 50 +++++
.../devicetree/bindings/video/renesas,du.txt | 84 ++++++++
.../devicetree/bindings/video/thine,thc63lvdm83d | 50 +++++
.../devicetree/bindings/video/vga-connector.txt | 28 +++
arch/arm/boot/dts/r8a7779-marzen.dts | 106 ++++++++++
arch/arm/boot/dts/r8a7779.dtsi | 24 +++
arch/arm/boot/dts/r8a7790-lager.dts | 78 ++++++-
arch/arm/boot/dts/r8a7790.dtsi | 39 ++++
arch/arm/boot/dts/r8a7791-koelsch.dts | 43 +++-
arch/arm/boot/dts/r8a7791.dtsi | 30 +++
arch/arm/mach-shmobile/board-koelsch-reference.c | 74 -------
arch/arm/mach-shmobile/board-koelsch.c | 19 +-
arch/arm/mach-shmobile/board-lager-reference.c | 81 --------
arch/arm/mach-shmobile/board-lager.c | 19 +-
arch/arm/mach-shmobile/board-marzen.c | 19 +-
drivers/gpu/drm/rcar-du/Kconfig | 1 +
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 170 ++++++++-------
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 2 +
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 11 +-
drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 3 +-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 231 +++++++++++++++++++--
drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 43 ++--
drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 3 +-
include/linux/platform_data/rcar-du.h | 4 +-
25 files changed, 904 insertions(+), 310 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/adi,adv7123.txt
create mode 100644 Documentation/devicetree/bindings/video/renesas,du.txt
create mode 100644 Documentation/devicetree/bindings/video/thine,thc63lvdm83d
create mode 100644 Documentation/devicetree/bindings/video/vga-connector.txt
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-27 15:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827095241.GC23186@ulmo>
[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]
On Wed, Aug 27, 2014 at 11:52:48AM +0200, Thierry Reding wrote:
> On Wed, Aug 27, 2014 at 10:45:26AM +0200, Maxime Ripard wrote:
> > On Wed, Aug 27, 2014 at 08:54:41AM +0200, Thierry Reding wrote:
> > > On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
> > > > On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
> [...]
> > > > > > Mike Turquette repeatedly said that he was against such a DT property:
> > > > > > https://lkml.org/lkml/2014/5/12/693
> > > > >
> > > > > Mike says in that email that he's opposing the addition of a property
> > > > > for clocks that is the equivalent of regulator-always-on. That's not
> > > > > what this is about. If at all it'd be a property to mark a clock that
> > > > > should not be disabled by default because it's essential.
> > > >
> > > > It's just semantic. How is "a clock that should not be disabled by
> > > > default because it's essential" not a clock that stays always on?
> > >
> > > Because a clock that should not be disabled by default can be turned off
> > > when appropriate. A clock that is always on can't be turned off.
> >
> > If a clock is essential, then it should never be disabled. Or we don't
> > share the same meaning of essential.
>
> Essential for the particular use-case.
So, how would the clock driver would know about which use case we're
in? How would it know about which display engine is currently running?
How would it know about which video output is being set?
Currently, we have two separate display engines, which can each output
either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
one of these combinations would require different clocks. What clocks
will we put in the driver? All of them?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-27 14:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827135609.GW15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 5509 bytes --]
On Wed, Aug 27, 2014 at 03:56:09PM +0200, Maxime Ripard wrote:
> On Wed, Aug 27, 2014 at 02:56:14PM +0200, Thierry Reding wrote:
> > > >> So second of all, Thierry, what exactly is the technical argument against
> > > >> adding support for dealing with clocks to simplefb ?
> > > >
> > > > I've already stated technical arguments against it. You could just go
> > > > back and read the thread again, or would you rather want me to repeat
> > > > them for your convenience?
> > >
> > > It is a long thread, IIRC your main arguments against this are:
> > >
> > > 1) You don't want this kind of platform-specific knowledge in simplefb
> > > 2) That simplefb is supposed to be simple, and this is not simple
> > >
> > > As for 1. several people have already argued that clocks as such are
> > > an abstract concept, found one many platforms and as such are not
> > > platform specific.
> >
> > That alone doesn't justify this patch. SoCs often have a requirement to
> > enable clocks in a specific order, for example. Other systems may need
> > to coordinate clocks with resets in a specific order or enable power
> > domains and such. All that is very SoC specific and if we don't make it
> > very clear what this driver assumes about the state of the system, then
> > we can't object to anybody adding support for those later on either. The
> > result of that is going to be a driver that needs to handle all kinds of
> > combinations of resources.
>
> Which is not an issue in our case, since the firmware enabled them
> already.
Exactly, so why do you need them at all? You shouldn't have to.
> > So there's a couple of SoCs and boards that actually are generic enough
> > to work with a generic driver. And then there's a whole bunch of other
> > drivers for hardware that's compliant with the same standard yet needs
> > different drivers. To me that's a clear indication that there isn't
> > enough genericity to warrant a generic driver in the first place.
> >
> > The commonality is in the functionality and defined by the standard
> > registers. But there's little to no commonality in how that interface is
> > glued into the SoC. Luckily the above subsystems implement the standard
> > hardware programming in a library, so that non-generic drivers can still
> > make use of most of the generic code.
>
> To be fair, these additions are a couple of release old, and are quite
> recent. You can't really make any judgement based on barely 2 releases
> of history.
Okay, fair enough. Time will tell. It's still kind of odd to declare a
driver to be generic without attempting to make the majority of existing
drivers work with it.
> > > If you look at how almost all dt bindings work, then the dt node for
> > > a device specifies the resources needed, I don't see why simplefb would
> > > be so special that it should be different here. I agree that it is
> > > important to get the abstractions right here, but to me it seems that
> > > the right abstraction is to be consistent with how all other devices
> > > are abstracted and to add needed resources to the dt node for the
> > > simplefb.
> >
> > But simplefb is fundamentally different from other devices. It isn't a
> > physical device at all. It's a virtual device that reuses resources as
> > set up by some other piece of code (firmware). It implies that there's
> > nothing that needs to be managed. It should only create a framebuffer
> > with the given parameters and allow the kernel (and userspace) to render
> > into it.
>
> Nothing should be managed, but everything should stay as is. Again,
> this is something that we all seem to agree on, yet differ completely
> on how to implement that.
I'm arguing that if everything should stay as is, then nobody should
have to do anything.
> > The only way you can deal with such virtual, completely generic devices
> > is by being very explicit about the requirements. For simplefb the
> > assumptions are that firmware set everything up and passes information
> > about what it set up to the kernel so that it can be reused. None of the
> > resources need to be explicitly managed because they have all been
> > properly configured. For that reason, again, I think the right way is
> > for the kernel not to switch off any of the used resources.
>
> So, you're saying that the firmware should inform the kernel about
> what clocks it has set up?
Yes. The problem is that even if we tell the kernel that clocks have
been set up it may still decide to disable them when they are unused.
That's the whole purpose of clk_disable_unused() as I understand it.
Whatever mechanism we introduce needs to specifically imply that the
clock must stay enabled.
> > If you want to equate simplefb to other drivers then it is fundamentally
> > broken anyway. Given only what's in the DTB the simplefb driver won't be
> > able to do anything useful. Consider what would happen if the firmware
> > didn't set up all the resources. Then the DT is missing resets, power
> > supplies and all that to make it work.
>
> That would mean that the framebuffer wasn't working in the first
> place, which breaks the initial assumption, doesn't it?
Exactly. simplefb gets away with an incomplete description of the
hardware in DT because of these assumptions. So you can't draw parallels
to other drivers since they don't make these assumptions and need to be
described completely.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-27 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FDE0CE.5030807@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 8992 bytes --]
On Wed, Aug 27, 2014 at 03:44:46PM +0200, Hans de Goede wrote:
> Hi,
>
> On 08/27/2014 02:56 PM, Thierry Reding wrote:
> > On Wed, Aug 27, 2014 at 12:35:24PM +0200, Hans de Goede wrote:
>
> <snip>
>
> >>>> So second of all, Thierry, what exactly is the technical argument against
> >>>> adding support for dealing with clocks to simplefb ?
> >>>
> >>> I've already stated technical arguments against it. You could just go
> >>> back and read the thread again, or would you rather want me to repeat
> >>> them for your convenience?
> >>
> >> It is a long thread, IIRC your main arguments against this are:
> >>
> >> 1) You don't want this kind of platform-specific knowledge in simplefb
> >> 2) That simplefb is supposed to be simple, and this is not simple
> >>
> >> As for 1. several people have already argued that clocks as such are
> >> an abstract concept, found one many platforms and as such are not
> >> platform specific.
> >
> > That alone doesn't justify this patch. SoCs often have a requirement to
> > enable clocks in a specific order, for example. Other systems may need
> > to coordinate clocks with resets in a specific order or enable power
> > domains and such. All that is very SoC specific and if we don't make it
> > very clear what this driver assumes about the state of the system, then
> > we can't object to anybody adding support for those later on either. The
> > result of that is going to be a driver that needs to handle all kinds of
> > combinations of resources.
>
> The problems your talking about here all have to do with ordering how
> things are enabled, but what we're talking about here is keeping things
> enabled which are already enabled by the bootloader, so there is no
> ordering problem.
See what I did there? I tricked you into saying what I've been saying
all along. =) It's about keeping things enabled which have been enabled
by the bootloader. That's the root of the problem.
> > So while clocks themselves are fairly generic the way in which they need
> > to be used is highly platform-specific.
> >
> > Let me also reiterate what I've said a couple of times already. The
> > issue here isn't that simplefb needs to manage these clocks in any way.
> > Firmware has already set them up so that display works. The reason why
> > you need this patch is so that the clock framework doesn't automatically
> > turn them off. With the proposed patch the driver enables these clocks,
> > but that's not what it needs to do, they are already on. It's a work
> > around to prevent the clock framework into not disabling them.
> >
> > So as far as I'm concerned this points to a fundamental issue with how
> > the clock framework handles unused clocks. Therefore that is the problem
> > that should be solved, not worked around.
>
> Hmm I see, in my mind the problem is not that the clk framework disables
> unused clocks, but that no one is marking the clocks in question as used.
> Someone should mark these clocks as used so that they do not get disabled.
>
> We could add a clk_mark_in_use function and have the simplefb patch call
> that instead of clk_prepare_and_enable, or maybe even better just only
> claim the clks and teach the clk framework to not disable claimed clk
> in its cleanup run. That would make it clear that simplefb is not enabling
> anything, just claiming resource its need to avoid them getting removed
> from underneath simplefb, would that work for you ?
That would be more accurate, but it boils down to the same problem where
we still need a driver to claim the resources in some way whereas the
problem is fundamentally one where the bootloader should be telling the
kernel not to disable it. It's in fact the bootloader that's claiming
the resources.
> >> Any generic hw driver dealing with embedded platforms, be it ahci, ohci,
> >> ehci, etc. has ended up adding abstract support for things like clocks
> >> (and other resources). I say abstract support here, since to all these
> >> drivers the support was added in a way that no platform specific knowledge
> >> is necessary, they just deal with clocks not caring about the specific
> >> underlying clock implementation, clock names, etc.
> >
> > Yes, I know that. You and I both had a very similar discussion not so
> > long ago. But the above aren't quite the same as simplefb. The devices
> > follow at least a well-known standard (EHCI, OHCI, AHCI), so there's
> > bound to be more commonalities between them than between the various
> > display devices that simplefb potentially can support.
> >
> > Interestingly though, if you look at what hardware really is supported
> > by the generic drivers that deal with embedded platforms, there isn't
> > all that much diversity (I've manually stripped out non-DTS occurrences
> > since they aren't relevant for this discussion):
>
> That is because the generic platform drivers were only added recently
> to stop the insanity where each new soc got its own ohci-soc.c /
> ehci-soc.c file.
Insanity is subjective.
> > arch/arm/boot/dts/rk3288.dtsi:199: compatible = "generic-ehci";
> > arch/arm/boot/dts/rk3288.dtsi:210: compatible = "generic-ehci";
>
> (offtopic) And I see that despite the recent addition, we actually already have our
> first non sunxi user, good, I didn't even know the rockchip guys were using this :)
FWIW, the rk3288 entry is broken. It should really contain an SoC
specific compatible string as well.
> And I know that there are quite a few more in the pipeline (waiting for their
> usb phy and dt patches to get merged).
As far as I'm concerned that's all a mistake (and catastrophy waiting to
happen), but hey, there's only so many hours in a day so I've got to
pick my battles.
> > So there's a couple of SoCs and boards that actually are generic enough
> > to work with a generic driver. And then there's a whole bunch of other
> > drivers for hardware that's compliant with the same standard yet needs
> > different drivers.
>
> No, there are a lot of other drivers which were written before someone
> decided that having 10-20 drivers which were 90% copy and paste of each
> other was a bad idea, but we're really going offtopic here.
The copy and paste is for code that's platform specific. The clocks have
different names, resets are different, supplies are different. The fact
that many can currently use the same driver is likely just coincidence
rather than design and it's entirely possible that at some point they'll
add support for a more advanced feature that makes them incompatible
with the rest of the generic drivers. And then you have a big mess
because you need to add quirks all over the place.
And this isn't all that far off-topic, since simplefb also needs to deal
with this kind of situation. And what I've been arguing is that in order
to really be generic it has to make assumptions, one of which is that it
uses only resources that it doesn't need to explicitly handle.
> >> If you look at how almost all dt bindings work, then the dt node for
> >> a device specifies the resources needed, I don't see why simplefb would
> >> be so special that it should be different here. I agree that it is
> >> important to get the abstractions right here, but to me it seems that
> >> the right abstraction is to be consistent with how all other devices
> >> are abstracted and to add needed resources to the dt node for the
> >> simplefb.
> >
> > But simplefb is fundamentally different from other devices. It isn't a
> > physical device at all. It's a virtual device that reuses resources as
> > set up by some other piece of code (firmware). It implies that there's
> > nothing that needs to be managed. It should only create a framebuffer
> > with the given parameters and allow the kernel (and userspace) to render
> > into it.
> >
> > The only way you can deal with such virtual, completely generic devices
> > is by being very explicit about the requirements. For simplefb the
> > assumptions are that firmware set everything up and passes information
> > about what it set up to the kernel so that it can be reused.
>
> > None of the resources need to be explicitly managed because they have all
> > been properly configured. For that reason, again, I think the right way is
> > for the kernel not to switch off any of the used resources.
>
> The key word here is "the used resources" to me this is not about simlefb
> managing resources, but marking them as used as long as it needs them, like
> it will need to do for the reserved mem chunk.
The difference between the clocks and the memory resource is that the
driver needs to directly access the memory (it needs to map it and
provide a userspace mapping for it) whereas it doesn't need to touch the
clocks (except to workaround a Linux-specific implementation detail).
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-27 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827125613.GF23186@ulmo>
[-- Attachment #1: Type: text/plain, Size: 11763 bytes --]
On Wed, Aug 27, 2014 at 02:56:14PM +0200, Thierry Reding wrote:
> > >> So second of all, Thierry, what exactly is the technical argument against
> > >> adding support for dealing with clocks to simplefb ?
> > >
> > > I've already stated technical arguments against it. You could just go
> > > back and read the thread again, or would you rather want me to repeat
> > > them for your convenience?
> >
> > It is a long thread, IIRC your main arguments against this are:
> >
> > 1) You don't want this kind of platform-specific knowledge in simplefb
> > 2) That simplefb is supposed to be simple, and this is not simple
> >
> > As for 1. several people have already argued that clocks as such are
> > an abstract concept, found one many platforms and as such are not
> > platform specific.
>
> That alone doesn't justify this patch. SoCs often have a requirement to
> enable clocks in a specific order, for example. Other systems may need
> to coordinate clocks with resets in a specific order or enable power
> domains and such. All that is very SoC specific and if we don't make it
> very clear what this driver assumes about the state of the system, then
> we can't object to anybody adding support for those later on either. The
> result of that is going to be a driver that needs to handle all kinds of
> combinations of resources.
Which is not an issue in our case, since the firmware enabled them
already.
> So while clocks themselves are fairly generic the way in which they need
> to be used is highly platform-specific.
>
> Let me also reiterate what I've said a couple of times already. The
> issue here isn't that simplefb needs to manage these clocks in any way.
> Firmware has already set them up so that display works. The reason why
> you need this patch is so that the clock framework doesn't automatically
> turn them off. With the proposed patch the driver enables these clocks,
> but that's not what it needs to do, they are already on. It's a work
> around to prevent the clock framework into not disabling them.
>
> So as far as I'm concerned this points to a fundamental issue with how
> the clock framework handles unused clocks. Therefore that is the problem
> that should be solved, not worked around.
Well, this is again a philosophical difference them. I find like the
right thing to do for the clock framework to disable the clocks that
are enabled if no one reports them as used. That also comes down to
all the drivers that require a clock to stay enabled should report to
the clock framework to tell that it uses this clock. Fortunately, we
have everything in place to do so.
> > Any generic hw driver dealing with embedded platforms, be it ahci, ohci,
> > ehci, etc. has ended up adding abstract support for things like clocks
> > (and other resources). I say abstract support here, since to all these
> > drivers the support was added in a way that no platform specific knowledge
> > is necessary, they just deal with clocks not caring about the specific
> > underlying clock implementation, clock names, etc.
>
> Yes, I know that. You and I both had a very similar discussion not so
> long ago. But the above aren't quite the same as simplefb. The devices
> follow at least a well-known standard (EHCI, OHCI, AHCI), so there's
> bound to be more commonalities between them than between the various
> display devices that simplefb potentially can support.
>
> Interestingly though, if you look at what hardware really is supported
> by the generic drivers that deal with embedded platforms, there isn't
> all that much diversity (I've manually stripped out non-DTS occurrences
> since they aren't relevant for this discussion):
>
> $ git grep -n generic-ohci
> arch/arm/boot/dts/sun4i-a10.dtsi:451: compatible = "allwinner,sun4i-a10-ohci", "generic-ohci";
> arch/arm/boot/dts/sun4i-a10.dtsi:492: compatible = "allwinner,sun4i-a10-ohci", "generic-ohci";
> arch/arm/boot/dts/sun5i-a10s.dtsi:403: compatible = "allwinner,sun5i-a10s-ohci", "generic-ohci";
> arch/arm/boot/dts/sun5i-a13.dtsi:376: compatible = "allwinner,sun5i-a13-ohci", "generic-ohci";
> arch/arm/boot/dts/sun6i-a31.dtsi:410: compatible = "allwinner,sun6i-a31-ohci", "generic-ohci";
> arch/arm/boot/dts/sun6i-a31.dtsi:432: compatible = "allwinner,sun6i-a31-ohci", "generic-ohci";
> arch/arm/boot/dts/sun6i-a31.dtsi:443: compatible = "allwinner,sun6i-a31-ohci", "generic-ohci";
> arch/arm/boot/dts/sun7i-a20.dtsi:535: compatible = "allwinner,sun7i-a20-ohci", "generic-ohci";
> arch/arm/boot/dts/sun7i-a20.dtsi:576: compatible = "allwinner,sun7i-a20-ohci", "generic-ohci";
> arch/powerpc/boot/dts/akebono.dts:144: compatible = "ibm,476gtr-ohci", "generic-ohci";
> arch/powerpc/boot/dts/akebono.dts:151: compatible = "ibm,476gtr-ohci", "generic-ohci";
>
> $ git grep -n generic-ehci
> arch/arm/boot/dts/rk3288.dtsi:199: compatible = "generic-ehci";
> arch/arm/boot/dts/rk3288.dtsi:210: compatible = "generic-ehci";
> arch/arm/boot/dts/sun4i-a10.dtsi:441: compatible = "allwinner,sun4i-a10-ehci", "generic-ehci";
> arch/arm/boot/dts/sun4i-a10.dtsi:482: compatible = "allwinner,sun4i-a10-ehci", "generic-ehci";
> arch/arm/boot/dts/sun5i-a10s.dtsi:393: compatible = "allwinner,sun5i-a10s-ehci", "generic-ehci";
> arch/arm/boot/dts/sun5i-a13.dtsi:366: compatible = "allwinner,sun5i-a13-ehci", "generic-ehci";
> arch/arm/boot/dts/sun6i-a31.dtsi:399: compatible = "allwinner,sun6i-a31-ehci", "generic-ehci";
> arch/arm/boot/dts/sun6i-a31.dtsi:421: compatible = "allwinner,sun6i-a31-ehci", "generic-ehci";
> arch/arm/boot/dts/sun7i-a20.dtsi:525: compatible = "allwinner,sun7i-a20-ehci", "generic-ehci";
> arch/arm/boot/dts/sun7i-a20.dtsi:566: compatible = "allwinner,sun7i-a20-ehci", "generic-ehci";
> arch/powerpc/boot/dts/akebono.dts:130: compatible = "ibm,476gtr-ehci", "generic-ehci";
>
> For generic-ahci there aren't any matches, but here are a few that are
> marked compatible with it using different compatible strings:
>
> $ git grep -n snps,spear-ahci
> arch/arm/boot/dts/spear1310.dtsi:60: compatible = "snps,spear-ahci";
> arch/arm/boot/dts/spear1310.dtsi:69: compatible = "snps,spear-ahci";
> arch/arm/boot/dts/spear1310.dtsi:78: compatible = "snps,spear-ahci";
> arch/arm/boot/dts/spear1340.dtsi:43: compatible = "snps,spear-ahci";
>
> $ git grep -n snps,exynos5440-ahci
> arch/arm/boot/dts/exynos5440.dtsi:241: compatible = "snps,exynos5440-ahci";
>
> $ git grep -n 'ibm,476gtr-ahci'
> arch/powerpc/boot/dts/akebono.dts:123: compatible = "ibm,476gtr-ahci";
>
> $ git grep -n 'snps,dwc-ahci'
> arch/arm/boot/dts/dra7.dtsi:1049: compatible = "snps,dwc-ahci";
> arch/arm/boot/dts/exynos5250.dtsi:265: compatible = "snps,dwc-ahci";
> arch/arm/boot/dts/omap5.dtsi:919: compatible = "snps,dwc-ahci";
>
> That looks fairly homogenous to me. There are also things like this
> (from the ahci_platform.c driver):
>
> if (of_device_is_compatible(dev->of_node, "hisilicon,hisi-ahci"))
> hpriv->flags |= AHCI_HFLAG_NO_FBS | AHCI_HFLAG_NO_NCQ;
>
> With more supported hardware there's bound to be more quirks like that.
>
> So there's a couple of SoCs and boards that actually are generic enough
> to work with a generic driver. And then there's a whole bunch of other
> drivers for hardware that's compliant with the same standard yet needs
> different drivers. To me that's a clear indication that there isn't
> enough genericity to warrant a generic driver in the first place.
>
> The commonality is in the functionality and defined by the standard
> registers. But there's little to no commonality in how that interface is
> glued into the SoC. Luckily the above subsystems implement the standard
> hardware programming in a library, so that non-generic drivers can still
> make use of most of the generic code.
To be fair, these additions are a couple of release old, and are quite
recent. You can't really make any judgement based on barely 2 releases
of history.
> > If you look at how almost all dt bindings work, then the dt node for
> > a device specifies the resources needed, I don't see why simplefb would
> > be so special that it should be different here. I agree that it is
> > important to get the abstractions right here, but to me it seems that
> > the right abstraction is to be consistent with how all other devices
> > are abstracted and to add needed resources to the dt node for the
> > simplefb.
>
> But simplefb is fundamentally different from other devices. It isn't a
> physical device at all. It's a virtual device that reuses resources as
> set up by some other piece of code (firmware). It implies that there's
> nothing that needs to be managed. It should only create a framebuffer
> with the given parameters and allow the kernel (and userspace) to render
> into it.
Nothing should be managed, but everything should stay as is. Again,
this is something that we all seem to agree on, yet differ completely
on how to implement that.
> The only way you can deal with such virtual, completely generic devices
> is by being very explicit about the requirements. For simplefb the
> assumptions are that firmware set everything up and passes information
> about what it set up to the kernel so that it can be reused. None of the
> resources need to be explicitly managed because they have all been
> properly configured. For that reason, again, I think the right way is
> for the kernel not to switch off any of the used resources.
So, you're saying that the firmware should inform the kernel about
what clocks it has set up?
I do agree on that too.
And it looks like we reached an agreement then, since it's exactly
what this patch is relying on.
> If you want to equate simplefb to other drivers then it is fundamentally
> broken anyway. Given only what's in the DTB the simplefb driver won't be
> able to do anything useful. Consider what would happen if the firmware
> didn't set up all the resources. Then the DT is missing resets, power
> supplies and all that to make it work.
That would mean that the framebuffer wasn't working in the first
place, which breaks the initial assumption, doesn't it?
> > > so I think we should keep the discussion going for a bit and
> > > see if we really can't come up with something that will fix this for a
> > > more general case rather than just your particular use-case. It's
> > > evidently a recurring problem that others suffer from, too.
> >
> > Yes needing resources despite the device being behind some generic
> > interface where the main driver code for that interface has no knowledge
> > about such resources sofar, because e.g. on PC-s this was not necessary
> > is a recurring problem, which has been solved already 3 times at least,
> > see the ohci-, ehci- and ahci-platform drivers, and for all 3 the conclusion
> > was that the best solution was to simple add the resources to the dt-node,
> > and add code to claim and enable them.
>
> And as I pointed out above in all three cases the generic platform
> driver is only marginally useful because many SoCs have specific
> requirements that make them incompatible with the generic driver.
s/marginally useful/marginally used/
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox