Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 06/13] Hide ep_take_care_of_epollwakeup() behind #ifdef __KERNEL__
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd
  Cc: 3chas3, hpa, mingo, plagnioj, jikos, linux-api, linux-arch,
	linux-atm-general, linux-fbdev, linux-kernel, netdev, tglx,
	tomi.valkeinen, x86, Palmer Dabbelt
In-Reply-To: <1441832902-28993-1-git-send-email-palmer@dabbelt.com>

This doesn't make any sense to expose to userspace.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/eventpoll.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h
index bc81fb2e1f0e..290426bfb0aa 100644
--- a/include/uapi/linux/eventpoll.h
+++ b/include/uapi/linux/eventpoll.h
@@ -61,6 +61,7 @@ struct epoll_event {
 	__u64 data;
 } EPOLL_PACKED;
 
+#ifdef __KERNEL__
 #ifdef CONFIG_PM_SLEEP
 static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
 {
@@ -73,4 +74,6 @@ static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
 	epev->events &= ~EPOLLWAKEUP;
 }
 #endif
+#endif /*__KERNEL__*/
+
 #endif /* _UAPI_LINUX_EVENTPOLL_H */
-- 
2.4.6


^ permalink raw reply related

* [PATCH 05/13] Hide some of "struct elf_prstatus" behind #ifdef __KERNEL__
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd
  Cc: 3chas3, hpa, mingo, plagnioj, jikos, linux-api, linux-arch,
	linux-atm-general, linux-fbdev, linux-kernel, netdev, tglx,
	tomi.valkeinen, x86, Palmer Dabbelt
In-Reply-To: <1441832902-28993-1-git-send-email-palmer@dabbelt.com>

This one scares me: while I can't find any system calls that directly
take this as an argument, a comment in <linux/ptrace.h>

  "
   Generic ptrace interface that exports the architecture specific
   regsets using the corresponding NT_* types (which are also used in
   the core dump).  Please note that the NT_PRSTATUS note type in a
   core dump contains a full 'struct elf_prstatus'. But the
   user_regset for NT_PRSTATUS contains just the elf_gregset_t that is
   the pr_reg field of 'struct elf_prstatus'. For all the other
   user_regset flavors, the user_regset layout and the ELF core dump
   note payload are exactly the same layout.
  "

seems to indicate that it's possible to see this sometimes.  Since
this would only be visible to userspace in a somewhat convoluted
manner, I'm going to try and keep it as it was.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/elfcore.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/elfcore.h b/include/uapi/linux/elfcore.h
index 569737cfb557..f9320b588937 100644
--- a/include/uapi/linux/elfcore.h
+++ b/include/uapi/linux/elfcore.h
@@ -60,7 +60,7 @@ struct elf_prstatus
 	long	pr_instr;		/* Current instruction */
 #endif
 	elf_gregset_t pr_reg;	/* GP registers */
-#ifdef CONFIG_BINFMT_ELF_FDPIC
+#if defined(__KERNEL__) && defined(CONFIG_BINFMT_ELF_FDPIC)
 	/* When using FDPIC, the loadmap addresses need to be communicated
 	 * to GDB in order for GDB to do the necessary relocations.  The
 	 * fields (below) used to communicate this information are placed
-- 
2.4.6


^ permalink raw reply related

* [PATCH 04/13] Always expose MAP_UNINITIALIZED to userspace
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd
  Cc: 3chas3, hpa, mingo, plagnioj, jikos, linux-api, linux-arch,
	linux-atm-general, linux-fbdev, linux-kernel, netdev, tglx,
	tomi.valkeinen, x86, Palmer Dabbelt
In-Reply-To: <1441832902-28993-1-git-send-email-palmer@dabbelt.com>

This used to be hidden behind CONFIG_MMAP_ALLOW_UNINITIALIZED, so
userspace wouldn't actually ever see it.  While I could have kept
hiding it, the man pages seem to indicate that MAP_UNINITIALIZED
should be visible:

  mmap(2)
  MAP_UNINITIALIZED (since Linux 2.6.33)
    Don't clear anonymous pages.  This flag is intended to improve
    performance on embedded devices.  This flag is honored only if the
    kernel was configured with the CONFIG_MMAP_ALLOW_UNINITIALIZED
    option.  Because of the security implications, that option is
    normally enabled only on embedded devices (i.e., devices where one
    has complete control of the contents of user memory).

and since the only time it shows up in my /usr/include is in this
header I believe this should have been visible to userspace (as
non-zero, which wouldn't do anything when or'd into the flags) all
along.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/asm-generic/mman-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ddc3b36f1046..e58d1911ecc6 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -19,7 +19,7 @@
 #define MAP_TYPE	0x0f		/* Mask for type of mapping */
 #define MAP_FIXED	0x10		/* Interpret addr exactly */
 #define MAP_ANONYMOUS	0x20		/* don't use a file */
-#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
+#if !defined(__KERNEL__) || defined(CONFIG_MMAP_ALLOW_UNINITIALIZED)
 # define MAP_UNINITIALIZED 0x4000000	/* For anonymous mmap, memory could be uninitialized */
 #else
 # define MAP_UNINITIALIZED 0x0		/* Don't support this flag */
-- 
2.4.6


^ permalink raw reply related

* [PATCH 03/13] Hide COMPAT_ATM_ADDPARTY behind #ifdef __KERNEL__
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd
  Cc: 3chas3, hpa, mingo, plagnioj, jikos, linux-api, linux-arch,
	linux-atm-general, linux-fbdev, linux-kernel, netdev, tglx,
	tomi.valkeinen, x86, Palmer Dabbelt
In-Reply-To: <1441832902-28993-1-git-send-email-palmer@dabbelt.com>

This used to just be behind an #ifdef COMPAT_COMPAT, so most of
userspace wouldn't have seen the definition before.  This change just
makes the __KERNEL__ part explicit to quiet the header checker.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/linux/atmdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/atmdev.h b/include/uapi/linux/atmdev.h
index 93e0ec008ca8..f8b6223165da 100644
--- a/include/uapi/linux/atmdev.h
+++ b/include/uapi/linux/atmdev.h
@@ -100,7 +100,7 @@ struct atm_dev_stats {
 					/* use backend to make new if */
 #define ATM_ADDPARTY  	_IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf)
  					/* add party to p2mp call */
-#ifdef CONFIG_COMPAT
+#if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */
 #define COMPAT_ATM_ADDPARTY  	_IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf)
 #endif
-- 
2.4.6


^ permalink raw reply related

* [PATCH 02/13] Always expose __SYSCALL(... fork ...)
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd-r2nGTMty4D4
  Cc: 3chas3-Re5JQEeQqe8AvxtiuMwx3w, hpa-YMNOUZJC4hwAvxtiuMwx3w,
	mingo-H+wXaHxf7aLQT0dZR+AlfA, plagnioj-sclMFOaUSTBWk0Htik3J/w,
	jikos-DgEjT+Ai2ygdnm+yROfE0A, linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arch-u79uwXL29TY76Z2rM5mHXA,
	linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, tglx-hfZtesqFncYOwBW4kG4KsQ,
	tomi.valkeinen-l0cyMroinI0, x86-DgEjT+Ai2ygdnm+yROfE0A,
	Palmer Dabbelt
In-Reply-To: <1441832902-28993-1-git-send-email-palmer-96lFi9zoCfxBDgjK7y7TUQ@public.gmane.org>

I think this change actually doesn't do anything: __NR_fork was still
being defined either way, and on my machine fork() in <unistd.h> comes
from libc.

That said, I don't think there's any way to determine this
automatically, so this at least quiets the checker.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/asm-generic/unistd.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index e016bd9b1a04..e027ef7aa01f 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -865,11 +865,11 @@ __SYSCALL(__NR_uselib, sys_uselib)
 __SYSCALL(__NR__sysctl, sys_sysctl)
 
 #define __NR_fork 1079
-#ifdef CONFIG_MMU
+#if !defined(__KERNEL__) || defined(CONFIG_MMU)
 __SYSCALL(__NR_fork, sys_fork)
 #else
 __SYSCALL(__NR_fork, sys_ni_syscall)
-#endif /* CONFIG_MMU */
+#endif /* !__KERNEL__ || CONFIG_MMU */
 
 #undef __NR_syscalls
 #define __NR_syscalls (__NR_fork+1)
-- 
2.4.6


^ permalink raw reply related

* [PATCH 01/13] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd
  Cc: 3chas3, hpa, mingo, plagnioj, jikos, linux-api, linux-arch,
	linux-atm-general, linux-fbdev, linux-kernel, netdev, tglx,
	tomi.valkeinen, x86, Palmer Dabbelt
In-Reply-To: <1441832902-28993-1-git-send-email-palmer@dabbelt.com>

When working on the RISC-V port I noticed that F_SETLK64 was being
defined on our 64-bit platform, despite our port being so new that
we've only ever had the 64-bit file ops.  Since there's not compat
layer for these, this causes fcntl to bail out.

It turns out that one of the ways in with F_SETLK64 was being defined
(there's some more in glibc, but that's a whole different story... :))
is the result of CONFIG_64BIT showing up in this user-visible header.
<asm-generic/bitsperlong.h> confirms this isn't sane, so I replaced it
with a __BITS_PER_LONG check.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
 include/uapi/asm-generic/fcntl.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063effe0cc1..14a5c8237d84 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -1,6 +1,7 @@
 #ifndef _ASM_GENERIC_FCNTL_H
 #define _ASM_GENERIC_FCNTL_H
 
+#include <asm/bitsperlong.h>
 #include <linux/types.h>
 
 /*
@@ -115,7 +116,7 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
+#if (__BITS_PER_LONG = 32)
 #ifndef F_GETLK64
 #define F_GETLK64	12	/*  using 'struct flock64' */
 #define F_SETLK64	13
-- 
2.4.6


^ permalink raw reply related

* Re: [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
From: Palmer Dabbelt @ 2015-09-09 21:08 UTC (permalink / raw)
  To: arnd
  Cc: 3chas3, hpa, mingo, plagnioj, jikos, linux-api, linux-arch,
	linux-atm-general, linux-fbdev, linux-kernel, netdev, tglx,
	tomi.valkeinen, x86
In-Reply-To: <2644177.lVCYzIBfPW@wuerfel>

I cut the RISC-V stuff, but I intend to reply to it later.  As you
said, it's just a different topic.

>>> However, I did see a lot of similar bugs now that you point me to it:
>>>
>>> $  grep -r \\\<CONFIG obj-tmp/usr/include/
>>> obj-tmp/usr/include/asm-generic/fcntl.h:#ifndef CONFIG_64BIT
>>> obj-tmp/usr/include/asm-generic/mman-common.h:#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
>>> obj-tmp/usr/include/asm-generic/unistd.h:#ifdef CONFIG_MMU
>>> obj-tmp/usr/include/asm-generic/unistd.h:#endif /* CONFIG_MMU */
>>> obj-tmp/usr/include/linux/atmdev.h:#ifdef CONFIG_COMPAT
>>> obj-tmp/usr/include/linux/elfcore.h:#ifdef CONFIG_BINFMT_ELF_FDPIC
>>> obj-tmp/usr/include/linux/eventpoll.h:#ifdef CONFIG_PM_SLEEP
>>> obj-tmp/usr/include/linux/fb.h:#ifdef CONFIG_FB_BACKLIGHT
>>> obj-tmp/usr/include/linux/flat.h:#ifdef CONFIG_BINFMT_SHARED_FLAT
>>> obj-tmp/usr/include/linux/hw_breakpoint.h:#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
>>> obj-tmp/usr/include/linux/pktcdvd.h:#if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
>>> obj-tmp/usr/include/linux/raw.h:#define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS
>>> obj-tmp/usr/include/asm/ptrace.h:#ifdef CONFIG_CPU_ENDIAN_BE8
>>>
>>> These all have the same problem, and we should fix them, as well as
>>> (probably) adding an automated check to scripts/headers_install.sh.
>>
>> Well, I was going to go fix them all and ran a very similar grep, but
>> I think I got a lot of false-positives.  If I understand correctly,
>> it's allowed to have CONFIG_* when guarded by __KERNEL__ in
>> user-visible headers?
>
> That is right.

It turns out there was actually a header checking script
(scripts/headers_check.pl), and it already had a check for this.  The
check was just disabled because there was "too much noise".  Rather
than putting it in headers_install I've just fixed that script.  I'm
definately lacking in perl powers, so I have no idea if what I've done
is sane.  Specifically: there's a global variable and a line over 80
characters, but since there's a bunch of other violations I figure
it's fine.

>> Now that I've written that, I realize it'd be pretty easy to just use
>> cpp to drop everything inside __KERNEL__ and then look for CONFIG_*.
>
> The lines quoted above are from the output of 'make headers_install',
> which already drops everything inside of __KERNEL__. A lot of them
> probably just need to add that #ifdef, or move the portion of the
> header file to the normal (non-uabi) file.
>
>> If you want, I can try to do that, fix what triggers the check, and
>> re-submit everything together?
>
> That would be great, yes.

OK.  I think this has turned into more of a RFC than a PATCH,
though...  I've just #ifdef'd things for now to reduce the diff size,
though I think it might be cleaner to move some of them to the
non-user headers (ep_take_care_of_epollwakeup(), USE_WCACHING,
MAX_RAW_MINORS).

I'm pretty far out of my depth here, so these should all be carefully
considered, but there's a few that scare me more ("struct
elf_prstatus", "enum by_type_idx", AT_VECTOR_SIZE_ARCH).  I think
there's only one actual bug here (MAP_UNINITIALIZED), the rest just
quiet the checking script.  Each patch has my rationale for what I
did.

Since this touches a whole lot of stuff, I've added a whole bunch of
CCs.


^ permalink raw reply

* Re: [PATCHv2 1/3] leds: Add of_led_get() and led_put()
From: Jacek Anaszewski @ 2015-09-09 12:40 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jingoo Han, Lee Jones, linux-leds, linux-fbdev, Andrew Lunn
In-Reply-To: <55F0230C.9000309@ti.com>

On 09/09/2015 02:16 PM, Tomi Valkeinen wrote:
>
>
> On 08/09/15 17:04, Jacek Anaszewski wrote:
>
>>>> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */
>>>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>>>> index b122eeafb5dc..0fce71a06d68 100644
>>>> --- a/include/linux/leds.h
>>>> +++ b/include/linux/leds.h
>>>> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct
>>>> device *parent,
>>>>    extern void led_classdev_suspend(struct led_classdev *led_cdev);
>>>>    extern void led_classdev_resume(struct led_classdev *led_cdev);
>>>>
>>>> +extern void led_put(struct led_classdev *led_cdev);
>>>> +
>>
>> This also needs no-op version.
>
> Ok, but... I think other already existing functions need no-ops also. If
> there's a driver that uses of_led_get and led_put, it's sure to use some
> other led_* functions also.
>
> So if we want that driver to be compilable when LED support is disabled
> in the kernel, we need to provide no-ops for all those functions.
>
> Probably:
>
> led_set_brightness
> led_blink_set_oneshot
> led_blink_set

That's right. It needs to be addressed soon too. Potentially
this could show up by breaking randconfig build.

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply

* Re: [PATCHv2 1/3] leds: Add of_led_get() and led_put()
From: Tomi Valkeinen @ 2015-09-09 12:16 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Jingoo Han, Lee Jones, linux-leds, linux-fbdev, Andrew Lunn
In-Reply-To: <55EEEB04.3090207@samsung.com>

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



On 08/09/15 17:04, Jacek Anaszewski wrote:

>>> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */
>>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>>> index b122eeafb5dc..0fce71a06d68 100644
>>> --- a/include/linux/leds.h
>>> +++ b/include/linux/leds.h
>>> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct
>>> device *parent,
>>>   extern void led_classdev_suspend(struct led_classdev *led_cdev);
>>>   extern void led_classdev_resume(struct led_classdev *led_cdev);
>>>
>>> +extern void led_put(struct led_classdev *led_cdev);
>>> +
> 
> This also needs no-op version.

Ok, but... I think other already existing functions need no-ops also. If
there's a driver that uses of_led_get and led_put, it's sure to use some
other led_* functions also.

So if we want that driver to be compilable when LED support is disabled
in the kernel, we need to provide no-ops for all those functions.

Probably:

led_set_brightness
led_blink_set_oneshot
led_blink_set

 Tomi


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

^ permalink raw reply

* Re: [PATCHv2 1/3] leds: Add of_led_get() and led_put()
From: Tomi Valkeinen @ 2015-09-09 12:00 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Jingoo Han, Lee Jones, linux-leds, linux-fbdev, Andrew Lunn
In-Reply-To: <55EEE0A1.5070000@samsung.com>

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


On 08/09/15 16:20, Jacek Anaszewski wrote:
> Hi Tomi,
> 
> Thanks for the update.
> 
> On 09/08/2015 01:19 PM, Tomi Valkeinen wrote:
>> This patch adds basic support for a kernel driver to get a LED device.
>> This will be used by the led-backlight driver.
>>
>> Only OF version is implemented for now, and the behavior is similar to
>> PWM's of_pwm_get() and pwm_put().
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/leds/Makefile    |  6 +++-
>>   drivers/leds/led-class.c | 13 +++++++-
>>   drivers/leds/led-of.c    | 82
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>   drivers/leds/leds.h      |  1 +
>>   include/linux/leds-of.h  | 26 +++++++++++++++
> 
> According to existing naming convention this should be "of_leds.h".

Right. I was thinking it's "leds" first, and "of" second, but I see
of_*.h is the convention.

>> +#include <linux/leds.h>
>> +#include <linux/of.h>
>> +#include <linux/leds-of.h>
> 
> Please keep alphabetical order.

Yep.

>> +#include <linux/module.h>
>> +
>> +#include "leds.h"
>> +
>> +/* find OF node for the given led_cdev */
>> +static struct device_node *find_led_of_node(struct led_classdev
>> *led_cdev)
>> +{
>> +    struct device *led_dev = led_cdev->dev;
>> +    struct device_node *child;
>> +
>> +    for_each_child_of_node(led_dev->parent->of_node, child) {
>> +        if (of_property_match_string(child, "label", led_cdev->name)
>> == 0)
> 
> Line over 80 characters.

I don't like to split lines to exact 80 chars, when it makes the code
more difficult to read. In this case it's 3 chars over 80, and splitting
the function call above to two lines doesn't look nice to me.

I'll do the func call separately, then it stays under 80 chars.

>> +            return child;
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +static int led_match_led_node(struct device *led_dev, const void *data)
>> +{
>> +    struct led_classdev *led_cdev = dev_get_drvdata(led_dev);
>> +    const struct device_node *target_node = data;
>> +    struct device_node *led_node;
>> +
>> +    led_node = find_led_of_node(led_cdev);
>> +    if (!led_node)
>> +        return 0;
>> +
>> +    of_node_put(led_node);
>> +
>> +    return led_node == target_node ? 1 : 0;
> 
> return led_node == target_node;

Again a matter of taste, but to me, == returns a bool, whereas the match
function here returns an int.

But I'm fine with plain == here.

 Tomi


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

^ permalink raw reply

* Re: [PATCHv2 3/3] devicetree: Add led-backlight binding
From: Tomi Valkeinen @ 2015-09-09 11:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
In-Reply-To: <20150908134140.GH30736@lunn.ch>

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



On 08/09/15 16:41, Andrew Lunn wrote:
> On Tue, Sep 08, 2015 at 02:19:36PM +0300, Tomi Valkeinen wrote:
>> Add DT binding for led-backlight.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  .../bindings/video/backlight/led-backlight.txt     | 30 ++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/video/backlight/led-backlight.txt
>>
>> diff --git a/Documentation/devicetree/bindings/video/backlight/led-backlight.txt b/Documentation/devicetree/bindings/video/backlight/led-backlight.txt
>> new file mode 100644
>> index 000000000000..fb77051ac230
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/backlight/led-backlight.txt
>> @@ -0,0 +1,30 @@
>> +led-backlight bindings
>> +
>> +Required properties:
>> +  - compatible: "led-backlight"
>> +  - leds: phandle to a led OF node [0]
>> +  - brightness-levels: Array of distinct LED brightness levels. These
>> +      are in the range from 0 to 255, passed to the LED class driver.
>> +  - default-brightness-level: the default brightness level (index into the
>> +      array defined by the "brightness-levels" property)
>> +  - power-supply: regulator for supply voltage
> 
> Hi Tomi
> 
> Maybe this regulator should be optional? I could imagine blacklights
> without one, in none power sensitive cases, e.g. industrial PC with a
> touch screen display, train ticket machine, etc.

Yes, I think so. Especially in this case as this led-backlight device is
not exactly a specific HW device, but more of a virtual device.

 Tomi


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

^ permalink raw reply

* Re: [PATCH 2/2] fbdev: ssd1307fb: add ssd1309 support
From: Lad, Prabhakar @ 2015-09-09 10:19 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Olliver Schinagl, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Thomas Niederprüm, Maxime Ripard,
	devicetree@vger.kernel.org, LKML, LFBDEV
In-Reply-To: <1441739991-4676-3-git-send-email-o.schinagl@ultimaker.com>

On Tue, Sep 8, 2015 at 8:19 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> The ssd1307fb driver supports a lot of chips from the ssd130xfb series.
> This patch adds the ssd1309 chip, a 128x64 OLED driver chip. It is very
> similar to the other chips and only has some definitions added to
> support it.
>
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>

Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Cheers,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH v4 0/22] On-demand device probing
From: Tomeu Vizoso @ 2015-09-09  9:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Warren, Javier Martinez Canillas, Mark Brown,
	Thierry Reding, Rafael J. Wysocki,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Dmitry Torokhov,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linus Walleij,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux USB List, Felipe Balbi, Linux PWM List,
	Terje Bergström, Greg Kroah-Hartman
In-Reply-To: <55EF8C65.4030706-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On 9 September 2015 at 03:33, Rob Herring <robh@kernel.org> wrote:
> On 09/08/2015 02:30 AM, Tomeu Vizoso wrote:
>> On 7 September 2015 at 22:50, Rob Herring <robherring2@gmail.com> wrote:
>>> On Mon, Sep 7, 2015 at 7:23 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>>> Hello,
>>>>
>>>> I have a problem with the panel on my Tegra Chromebook taking longer
>>>> than expected to be ready during boot (Stéphane Marchesin reported what
>>>> is basically the same issue in [0]), and have looked into ordered
>>>> probing as a better way of solving this than moving nodes around in the
>>>> DT or playing with initcall levels and linking order.
>>>>
>>>> While reading the thread [1] that Alexander Holler started with his
>>>> series to make probing order deterministic, it occurred to me that it
>>>> should be possible to achieve the same by probing devices as they are
>>>> referenced by other devices.
>>>>
>>>> This basically reuses the information that is already implicit in the
>>>> probe() implementations, saving us from refactoring existing drivers or
>>>> adding information to DTBs.
>>>>
>>>> During review of v1 of this series Linus Walleij suggested that it
>>>> should be the device driver core to make sure that dependencies are
>>>> ready before probing a device. I gave this idea a try [2] but Mark Brown
>>>> pointed out to the logic duplication between the resource acquisition
>>>> and dependency discovery code paths (though I think it's fairly minor).
>>>>
>>>> To address that code duplication I experimented with Arnd's devm_probe
>>>> [3] concept of having drivers declare their dependencies instead of
>>>> acquiring them during probe, and while it worked [4], I don't think we
>>>> end up winning anything when compared to just probing devices on-demand
>>>> from resource getters.
>>>>
>>>> One remaining objection is to the "sprinkling" of calls to
>>>> of_device_probe() in the resource getters of each subsystem, but I think
>>>> it's the right thing to do given that the storage of resources is
>>>> currently subsystem-specific.
>>>>
>>>> We could avoid the above by moving resource storage into the core, but I
>>>> don't think there's a compelling case for that.
>>>>
>>>> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and
>>>> OMAP SoCs, and these patches were enough to eliminate all the deferred
>>>> probes (except one in PandaBoard because omap_dma_system doesn't have a
>>>> firmware node as of yet).
>>>>
>>>> Have submitted a branch [5] with only these patches on top of thursday's
>>>> linux-next to kernelci.org and I don't see any issues that could be
>>>> caused by them. For some reason it currently has more passes than the
>>>> version of -next it's based on!
>>>>
>>>> With this series I get the kernel to output to the panel in 0.5s,
>>>> instead of 2.8s.
>>>>
>>>> Regards,
>>>>
>>>> Tomeu
>>>>
>>>> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>>>>
>>>> [1] https://lkml.org/lkml/2014/5/12/452
>>>>
>>>> [2] https://lkml.org/lkml/2015/6/17/305
>>>>
>>>> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>>>>
>>>> [4] https://lkml.org/lkml/2015/7/21/441a
>>>>
>>>> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6
>>>>
>>>> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/
>>>>
>>>> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/
>>>>
>>>> Changes in v4:
>>>> - Added bus.pre_probe callback so the probes of Primecell devices can be
>>>>   deferred if their device IDs cannot be yet read because of the clock
>>>>   driver not having probed when they are registered. Maybe this goes
>>>>   overboard and the matching information should be in the DT if there is
>>>>   one.
>>>
>>> Seems overboard to me or at least a separate problem.
>>
>> It's a separate problem but this was preventing the series from
>> working on a few boards.
>
> What is the failure? Not booting? Fixing not working would certainly not
> be overboard.

On the device I was testing on (qemu's vexpress-a15 machine) the
machine booted and I was able to open a ssh session, but serial was
broken among other AMBA devices:

/memory-controller@2b0a0000
/memory-controller@7ffd0000
/dma@7ffb0000
/smb/motherboard/iofpga@3,00000000/sysctl@020000
/smb/motherboard/iofpga@3,00000000/aaci@040000
/smb/motherboard/iofpga@3,00000000/mmci@050000
/smb/motherboard/iofpga@3,00000000/kmi@060000
/smb/motherboard/iofpga@3,00000000/kmi@070000
/smb/motherboard/iofpga@3,00000000/uart@090000
/smb/motherboard/iofpga@3,00000000/uart@0a0000
/smb/motherboard/iofpga@3,00000000/uart@0b0000
/smb/motherboard/iofpga@3,00000000/uart@0c0000
/smb/motherboard/iofpga@3,00000000/wdt@0f0000
/smb/motherboard/iofpga@3,00000000/timer@110000
/smb/motherboard/iofpga@3,00000000/timer@120000
/smb/motherboard/iofpga@3,00000000/rtc@170000
/smb/motherboard/iofpga@3,00000000/clcd@1f0000

Another way of avoiding this particular problem would be not delaying
the probe of devices in the configuration bus, by doing something like
this:

diff --git a/drivers/bus/vexpress-config.c b/drivers/bus/vexpress-config.c
index 6575c0fe6a4e..eda293869cd3 100644
--- a/drivers/bus/vexpress-config.c
+++ b/drivers/bus/vexpress-config.c
@@ -181,7 +181,7 @@ static int vexpress_config_populate(struct
device_node *node)
        if (WARN_ON(!parent))
                return -ENODEV;

-       return of_platform_populate(node, NULL, NULL, parent);
+       return of_platform_populate_early(node, NULL, NULL, parent);
 }

 static int __init vexpress_config_init(void)

But I think this would be papering over the underlying issue and it
would be better to have proper explicit dependencies.

Regards,

Tomeu

>>> Most clocks have
>>> to be setup before the driver model simply because timers depend on
>>> clocks usually.
>>
>> Yes, but in this case the apb clocks for the primecell devices are
>> implemented in a normal platform driver (vexpress_osc_driver), instead
>> of using CLK_OF_DECLARE.
>
> Okay.
>
> Rob
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply related

* Re: [PATCH v4 0/22] On-demand device probing
From: Rob Herring @ 2015-09-09  1:33 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: linux-kernel@vger.kernel.org, Stephen Warren,
	Javier Martinez Canillas, Mark Brown, Thierry Reding,
	Rafael J. Wysocki, linux-arm-kernel@lists.infradead.org,
	Dmitry Torokhov, devicetree@vger.kernel.org, Linus Walleij,
	linux-acpi@vger.kernel.org, Arnd Bergmann,
	linux-fbdev@vger.kernel.org, Linux USB List, Felipe Balbi,
	Linux PWM List, Terje Bergström, Greg Kroah-Hartman
In-Reply-To: <CAAObsKDUzk-9H2_VrVmbc=ZeRaknGcjTMY3Mauh3JVZxLz4CiA@mail.gmail.com>

On 09/08/2015 02:30 AM, Tomeu Vizoso wrote:
> On 7 September 2015 at 22:50, Rob Herring <robherring2@gmail.com> wrote:
>> On Mon, Sep 7, 2015 at 7:23 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>> Hello,
>>>
>>> I have a problem with the panel on my Tegra Chromebook taking longer
>>> than expected to be ready during boot (Stéphane Marchesin reported what
>>> is basically the same issue in [0]), and have looked into ordered
>>> probing as a better way of solving this than moving nodes around in the
>>> DT or playing with initcall levels and linking order.
>>>
>>> While reading the thread [1] that Alexander Holler started with his
>>> series to make probing order deterministic, it occurred to me that it
>>> should be possible to achieve the same by probing devices as they are
>>> referenced by other devices.
>>>
>>> This basically reuses the information that is already implicit in the
>>> probe() implementations, saving us from refactoring existing drivers or
>>> adding information to DTBs.
>>>
>>> During review of v1 of this series Linus Walleij suggested that it
>>> should be the device driver core to make sure that dependencies are
>>> ready before probing a device. I gave this idea a try [2] but Mark Brown
>>> pointed out to the logic duplication between the resource acquisition
>>> and dependency discovery code paths (though I think it's fairly minor).
>>>
>>> To address that code duplication I experimented with Arnd's devm_probe
>>> [3] concept of having drivers declare their dependencies instead of
>>> acquiring them during probe, and while it worked [4], I don't think we
>>> end up winning anything when compared to just probing devices on-demand
>>> from resource getters.
>>>
>>> One remaining objection is to the "sprinkling" of calls to
>>> of_device_probe() in the resource getters of each subsystem, but I think
>>> it's the right thing to do given that the storage of resources is
>>> currently subsystem-specific.
>>>
>>> We could avoid the above by moving resource storage into the core, but I
>>> don't think there's a compelling case for that.
>>>
>>> I have tested this on boards with Tegra, iMX.6, Exynos, Rockchip and
>>> OMAP SoCs, and these patches were enough to eliminate all the deferred
>>> probes (except one in PandaBoard because omap_dma_system doesn't have a
>>> firmware node as of yet).
>>>
>>> Have submitted a branch [5] with only these patches on top of thursday's
>>> linux-next to kernelci.org and I don't see any issues that could be
>>> caused by them. For some reason it currently has more passes than the
>>> version of -next it's based on!
>>>
>>> With this series I get the kernel to output to the panel in 0.5s,
>>> instead of 2.8s.
>>>
>>> Regards,
>>>
>>> Tomeu
>>>
>>> [0] http://lists.freedesktop.org/archives/dri-devel/2014-August/066527.html
>>>
>>> [1] https://lkml.org/lkml/2014/5/12/452
>>>
>>> [2] https://lkml.org/lkml/2015/6/17/305
>>>
>>> [3] http://article.gmane.org/gmane.linux.ports.arm.kernel/277689
>>>
>>> [4] https://lkml.org/lkml/2015/7/21/441a
>>>
>>> [5] https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=on-demand-probes-v6
>>>
>>> [6] http://kernelci.org/boot/all/job/collabora/kernel/v4.2-11902-g25d80c927f8b/
>>>
>>> [7] http://kernelci.org/boot/all/job/next/kernel/next-20150903/
>>>
>>> Changes in v4:
>>> - Added bus.pre_probe callback so the probes of Primecell devices can be
>>>   deferred if their device IDs cannot be yet read because of the clock
>>>   driver not having probed when they are registered. Maybe this goes
>>>   overboard and the matching information should be in the DT if there is
>>>   one.
>>
>> Seems overboard to me or at least a separate problem.
> 
> It's a separate problem but this was preventing the series from
> working on a few boards.

What is the failure? Not booting? Fixing not working would certainly not
be overboard.

> 
>> Most clocks have
>> to be setup before the driver model simply because timers depend on
>> clocks usually.
> 
> Yes, but in this case the apb clocks for the primecell devices are
> implemented in a normal platform driver (vexpress_osc_driver), instead
> of using CLK_OF_DECLARE.

Okay.

Rob


^ permalink raw reply

* Re: [PATCH 2/2] fbdev: ssd1307fb: add ssd1309 support
From: Rob Herring @ 2015-09-08 23:02 UTC (permalink / raw)
  To: Olliver Schinagl, Olliver Schinagl, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	Jean-Christophe Plagniol-Villard
  Cc: Tomi Valkeinen, Thomas Niederprüm, Maxime Ripard,
	Prabhakar Lad, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1441739991-4676-3-git-send-email-o.schinagl-U3FVU11NWA554TAoqtyWWQ@public.gmane.org>

On 09/08/2015 02:19 PM, Olliver Schinagl wrote:
> The ssd1307fb driver supports a lot of chips from the ssd130xfb series.
> This patch adds the ssd1309 chip, a 128x64 OLED driver chip. It is very
> similar to the other chips and only has some definitions added to
> support it.
> 
> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  Documentation/devicetree/bindings/video/ssd1307fb.txt |  3 ++-
>  drivers/video/fbdev/ssd1307fb.c                       | 11 +++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
> index d1be78d..eb31ed4 100644
> --- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
> @@ -2,7 +2,8 @@
>  
>  Required properties:
>    - compatible: Should be "solomon,<chip>fb-<bus>". The only supported bus for
> -    now is i2c, and the supported chips are ssd1305, ssd1306 and ssd1307.
> +    now is i2c, and the supported chips are ssd1305, ssd1306, ssd1307 and
> +    ssd1309.
>    - reg: Should contain address of the controller on the I2C bus. Most likely
>           0x3c or 0x3d
>    - pwm: Should contain the pwm to use according to the OF device tree PWM
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 339615c..8fc7960 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -495,6 +495,12 @@ static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
>  	.need_pwm = 1,
>  };
>  
> +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1309_deviceinfo = {
> +	.default_vcomh = 0x34,
> +	.default_dclk_div = 1,
> +	.default_dclk_frq = 10,
> +};
> +
>  static const struct of_device_id ssd1307fb_of_match[] = {
>  	{
>  		.compatible = "solomon,ssd1305fb-i2c",
> @@ -508,6 +514,10 @@ static const struct of_device_id ssd1307fb_of_match[] = {
>  		.compatible = "solomon,ssd1307fb-i2c",
>  		.data = (void *)&ssd1307fb_ssd1307_deviceinfo,
>  	},
> +	{
> +		.compatible = "solomon,ssd1309fb-i2c",
> +		.data = (void *)&ssd1307fb_ssd1309_deviceinfo,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
> @@ -708,6 +718,7 @@ static const struct i2c_device_id ssd1307fb_i2c_id[] = {
>  	{ "ssd1305fb", 0 },
>  	{ "ssd1306fb", 0 },
>  	{ "ssd1307fb", 0 },
> +	{ "ssd1309fb", 0 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
> 


^ permalink raw reply

* [PATCH 2/2] fbdev: ssd1307fb: add ssd1309 support
From: Olliver Schinagl @ 2015-09-08 19:19 UTC (permalink / raw)
  To: Olliver Schinagl, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jean-Christophe Plagniol-Villard
  Cc: Tomi Valkeinen, Thomas Niederprüm, Maxime Ripard,
	Prabhakar Lad, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Olliver Schinagl
In-Reply-To: <1441739991-4676-1-git-send-email-o.schinagl-U3FVU11NWA554TAoqtyWWQ@public.gmane.org>

The ssd1307fb driver supports a lot of chips from the ssd130xfb series.
This patch adds the ssd1309 chip, a 128x64 OLED driver chip. It is very
similar to the other chips and only has some definitions added to
support it.

Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com>
---
 Documentation/devicetree/bindings/video/ssd1307fb.txt |  3 ++-
 drivers/video/fbdev/ssd1307fb.c                       | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt b/Documentation/devicetree/bindings/video/ssd1307fb.txt
index d1be78d..eb31ed4 100644
--- a/Documentation/devicetree/bindings/video/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/video/ssd1307fb.txt
@@ -2,7 +2,8 @@
 
 Required properties:
   - compatible: Should be "solomon,<chip>fb-<bus>". The only supported bus for
-    now is i2c, and the supported chips are ssd1305, ssd1306 and ssd1307.
+    now is i2c, and the supported chips are ssd1305, ssd1306, ssd1307 and
+    ssd1309.
   - reg: Should contain address of the controller on the I2C bus. Most likely
          0x3c or 0x3d
   - pwm: Should contain the pwm to use according to the OF device tree PWM
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 339615c..8fc7960 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -495,6 +495,12 @@ static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
 	.need_pwm = 1,
 };
 
+static struct ssd1307fb_deviceinfo ssd1307fb_ssd1309_deviceinfo = {
+	.default_vcomh = 0x34,
+	.default_dclk_div = 1,
+	.default_dclk_frq = 10,
+};
+
 static const struct of_device_id ssd1307fb_of_match[] = {
 	{
 		.compatible = "solomon,ssd1305fb-i2c",
@@ -508,6 +514,10 @@ static const struct of_device_id ssd1307fb_of_match[] = {
 		.compatible = "solomon,ssd1307fb-i2c",
 		.data = (void *)&ssd1307fb_ssd1307_deviceinfo,
 	},
+	{
+		.compatible = "solomon,ssd1309fb-i2c",
+		.data = (void *)&ssd1307fb_ssd1309_deviceinfo,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
@@ -708,6 +718,7 @@ static const struct i2c_device_id ssd1307fb_i2c_id[] = {
 	{ "ssd1305fb", 0 },
 	{ "ssd1306fb", 0 },
 	{ "ssd1307fb", 0 },
+	{ "ssd1309fb", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
-- 
2.1.4


^ permalink raw reply related

* [PATCH 1/2] fbdev: ssd1307fb: alphabetize headers
From: Olliver Schinagl @ 2015-09-08 19:19 UTC (permalink / raw)
  To: Olliver Schinagl, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jean-Christophe Plagniol-Villard
  Cc: Tomi Valkeinen, Thomas Niederprüm, Maxime Ripard,
	Prabhakar Lad, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1441739991-4676-1-git-send-email-o.schinagl-U3FVU11NWA554TAoqtyWWQ@public.gmane.org>

From: Olliver Schinagl <oliver@schinagl.nl>

This patch sorts the headers on ssd1307fb driver.

Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
---
 drivers/video/fbdev/ssd1307fb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3e153c0..339615c 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -6,16 +6,16 @@
  * Licensed under the GPLv2 or later.
  */
 
-#include <linux/module.h>
 #include <linux/backlight.h>
-#include <linux/kernel.h>
-#include <linux/i2c.h>
+#include <linux/delay.h>
 #include <linux/fb.h>
-#include <linux/uaccess.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/pwm.h>
-#include <linux/delay.h>
+#include <linux/uaccess.h>
 
 #define SSD1307FB_DATA			0x40
 #define SSD1307FB_COMMAND		0x80
-- 
2.1.4


^ permalink raw reply related

* [PATCH 0/2] SSD1307fb updates
From: Olliver Schinagl @ 2015-09-08 19:19 UTC (permalink / raw)
  To: Olliver Schinagl, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Jean-Christophe Plagniol-Villard
  Cc: Tomi Valkeinen, Thomas Niederprüm, Maxime Ripard,
	Prabhakar Lad, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Olliver Schinagl

Having a few ssd1309 128x64 OLED displays laying around, I added support for it to the existing 1307fb driver. While doing this I noticed the headers where out of order so I fixed those as well.

For this specific display, the following can be used in a i2c node for example.

	ssd1309: oled@3c {
		compatible = "solomon,ssd1309fb-i2c";
		pinctrl-names = "default";
		pinctrl-0 = <&oled_pins>;
		reg = <0x3c>;
		reset-gpios = <&pio 8 13 GPIO_ACTIVE_HIGH>;
		solomon,width = <128>;
		solomon,height = <64>;
		solomon,com-invdir;
		solomon,page-offset = <0>;
		solomon,prechargep1 = <2>;
		solomon,prechargep2 = <8>;
	};

Olliver Schinagl (2):
  fbdev: ssd1307fb: alphabetize headers
  fbdev: ssd1307fb: add ssd1309 support

 .../devicetree/bindings/video/ssd1307fb.txt         |  3 ++-
 drivers/video/fbdev/ssd1307fb.c                     | 21 ++++++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

-- 
2.1.4


^ permalink raw reply

* Re: [PATCHv2 1/3] leds: Add of_led_get() and led_put()
From: Jacek Anaszewski @ 2015-09-08 14:04 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jingoo Han, Lee Jones, linux-leds, linux-fbdev, Andrew Lunn
In-Reply-To: <55EEE0A1.5070000@samsung.com>

On 09/08/2015 01:19 PM, Tomi Valkeinen wrote:
>> This patch adds basic support for a kernel driver to get a LED device.
>> This will be used by the led-backlight driver.
>>
>> Only OF version is implemented for now, and the behavior is similar to
>> PWM's of_pwm_get() and pwm_put().
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/leds/Makefile    |  6 +++-
>>   drivers/leds/led-class.c | 13 +++++++-
>>   drivers/leds/led-of.c    | 82
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>   drivers/leds/leds.h      |  1 +
>>   include/linux/leds-of.h  | 26 +++++++++++++++

[...]

>> diff --git a/include/linux/leds-of.h b/include/linux/leds-of.h
>> new file mode 100644
>> index 000000000000..7e8e64bd9811
>> --- /dev/null
>> +++ b/include/linux/leds-of.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + * OF support for leds
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#ifndef __LINUX_LEDS_OF_H_INCLUDED
>> +#define __LINUX_LEDS_OF_H_INCLUDED
>> +
>> +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_LEDS_CLASS)
>> +
>> +extern struct led_classdev *of_led_get(struct device_node *np);
>> +
>> +#else
>> +
>> +static inline struct led_classdev *of_led_get(struct device_node *np)
>> +{
>> +    return -ENODEV;
>> +}
>> +
>> +#endif
>> +
>> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index b122eeafb5dc..0fce71a06d68 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct
>> device *parent,
>>   extern void led_classdev_suspend(struct led_classdev *led_cdev);
>>   extern void led_classdev_resume(struct led_classdev *led_cdev);
>>
>> +extern void led_put(struct led_classdev *led_cdev);
>> +

This also needs no-op version.

>>   /**
>>    * led_blink_set - set blinking with software fallback
>>    * @led_cdev: the LED to start blinking
>>
>
>


-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply

* Re: [PATCHv2 3/3] devicetree: Add led-backlight binding
From: Andrew Lunn @ 2015-09-08 13:41 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
In-Reply-To: <1441711176-4258-4-git-send-email-tomi.valkeinen@ti.com>

On Tue, Sep 08, 2015 at 02:19:36PM +0300, Tomi Valkeinen wrote:
> Add DT binding for led-backlight.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  .../bindings/video/backlight/led-backlight.txt     | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/backlight/led-backlight.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/backlight/led-backlight.txt b/Documentation/devicetree/bindings/video/backlight/led-backlight.txt
> new file mode 100644
> index 000000000000..fb77051ac230
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/backlight/led-backlight.txt
> @@ -0,0 +1,30 @@
> +led-backlight bindings
> +
> +Required properties:
> +  - compatible: "led-backlight"
> +  - leds: phandle to a led OF node [0]
> +  - brightness-levels: Array of distinct LED brightness levels. These
> +      are in the range from 0 to 255, passed to the LED class driver.
> +  - default-brightness-level: the default brightness level (index into the
> +      array defined by the "brightness-levels" property)
> +  - power-supply: regulator for supply voltage

Hi Tomi

Maybe this regulator should be optional? I could imagine blacklights
without one, in none power sensitive cases, e.g. industrial PC with a
touch screen display, train ticket machine, etc.

      Andrew

^ permalink raw reply

* Re: [PATCHv2 1/3] leds: Add of_led_get() and led_put()
From: Jacek Anaszewski @ 2015-09-08 13:20 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jingoo Han, Lee Jones, linux-leds, linux-fbdev, Andrew Lunn
In-Reply-To: <1441711176-4258-2-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

Thanks for the update.

On 09/08/2015 01:19 PM, Tomi Valkeinen wrote:
> This patch adds basic support for a kernel driver to get a LED device.
> This will be used by the led-backlight driver.
>
> Only OF version is implemented for now, and the behavior is similar to
> PWM's of_pwm_get() and pwm_put().
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   drivers/leds/Makefile    |  6 +++-
>   drivers/leds/led-class.c | 13 +++++++-
>   drivers/leds/led-of.c    | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/leds/leds.h      |  1 +
>   include/linux/leds-of.h  | 26 +++++++++++++++

According to existing naming convention this should be "of_leds.h".

>   include/linux/leds.h     |  2 ++
>   6 files changed, 128 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/leds/led-of.c
>   create mode 100644 include/linux/leds-of.h
>
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 8d6a24a2f513..6fd22e411810 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -1,7 +1,11 @@
>
>   # LED Core
>   obj-$(CONFIG_NEW_LEDS)			+= led-core.o
> -obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
> +
> +obj-$(CONFIG_LEDS_CLASS)		+= led-class-objs.o
> +led-class-objs-y			:= led-class.o
> +led-class-objs-$(CONFIG_OF)		+= led-of.o
> +
>   obj-$(CONFIG_LEDS_CLASS_FLASH)		+= led-class-flash.o
>   obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index beabfbc6f7cd..1234f9dc3537 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -22,7 +22,7 @@
>   #include <linux/timer.h>
>   #include "leds.h"
>
> -static struct class *leds_class;
> +struct class *leds_class;
>
>   static ssize_t brightness_show(struct device *dev,
>   		struct device_attribute *attr, char *buf)
> @@ -216,6 +216,17 @@ static int led_resume(struct device *dev)
>
>   static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume);
>
> +/**
> + * led_put() - release a LED device, reserved with led_get()
> + * @led_cdev: LED device
> + */
> +void led_put(struct led_classdev *led_cdev)
> +{
> +	put_device(led_cdev->dev);
> +	module_put(led_cdev->dev->parent->driver->owner);
> +}
> +EXPORT_SYMBOL_GPL(led_put);
> +
>   static int match_name(struct device *dev, const void *data)
>   {
>   	if (!dev_name(dev))
> diff --git a/drivers/leds/led-of.c b/drivers/leds/led-of.c
> new file mode 100644
> index 000000000000..32631682be07
> --- /dev/null
> +++ b/drivers/leds/led-of.c
> @@ -0,0 +1,82 @@
> +/*
> + * LED Class Core OF support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/leds.h>
> +#include <linux/of.h>
> +#include <linux/leds-of.h>

Please keep alphabetical order.

> +#include <linux/module.h>
> +
> +#include "leds.h"
> +
> +/* find OF node for the given led_cdev */
> +static struct device_node *find_led_of_node(struct led_classdev *led_cdev)
> +{
> +	struct device *led_dev = led_cdev->dev;
> +	struct device_node *child;
> +
> +	for_each_child_of_node(led_dev->parent->of_node, child) {
> +		if (of_property_match_string(child, "label", led_cdev->name) = 0)

Line over 80 characters.

> +			return child;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int led_match_led_node(struct device *led_dev, const void *data)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(led_dev);
> +	const struct device_node *target_node = data;
> +	struct device_node *led_node;
> +
> +	led_node = find_led_of_node(led_cdev);
> +	if (!led_node)
> +		return 0;
> +
> +	of_node_put(led_node);
> +
> +	return led_node = target_node ? 1 : 0;

return led_node = target_node;

> +}
> +
> +/**
> + * of_led_get() - request a LED device via the LED framework
> + * @np: device node to get the LED device from
> + *
> + * Returns the LED device parsed from the phandle specified in the "leds"
> + * property of a device tree node or a negative error-code on failure.
> + *
> + * The caller must use led_put() to release the device after use.
> + */
> +struct led_classdev *of_led_get(struct device_node *np)
> +{
> +	struct device *led_dev;
> +	struct led_classdev *led_cdev;
> +	struct device_node *led_node;
> +
> +	led_node = of_parse_phandle(np, "leds", 0);
> +	if (!led_node)
> +		return ERR_PTR(-ENODEV);
> +
> +	led_dev = class_find_device(leds_class, NULL, led_node,
> +		led_match_led_node);
> +
> +	of_node_put(led_node);
> +
> +	if (!led_dev) {
> +		pr_err("failed to find led device for node %s, deferring probe\n",
> +			of_node_full_name(led_node));
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	led_cdev = dev_get_drvdata(led_dev);
> +
> +	if (!try_module_get(led_cdev->dev->parent->driver->owner))
> +		return ERR_PTR(-ENODEV);
> +
> +	return led_cdev;
> +}
> +EXPORT_SYMBOL_GPL(of_led_get);
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index bc89d7ace2c4..ccc3abb417d4 100644
> --- a/drivers/leds/leds.h
> +++ b/drivers/leds/leds.h
> @@ -46,6 +46,7 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
>
>   void led_stop_software_blink(struct led_classdev *led_cdev);
>
> +extern struct class *leds_class;
>   extern struct rw_semaphore leds_list_lock;
>   extern struct list_head leds_list;
>
> diff --git a/include/linux/leds-of.h b/include/linux/leds-of.h
> new file mode 100644
> index 000000000000..7e8e64bd9811
> --- /dev/null
> +++ b/include/linux/leds-of.h
> @@ -0,0 +1,26 @@
> +/*
> + * OF support for leds
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#ifndef __LINUX_LEDS_OF_H_INCLUDED
> +#define __LINUX_LEDS_OF_H_INCLUDED
> +
> +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_LEDS_CLASS)
> +
> +extern struct led_classdev *of_led_get(struct device_node *np);
> +
> +#else
> +
> +static inline struct led_classdev *of_led_get(struct device_node *np)
> +{
> +	return -ENODEV;
> +}
> +
> +#endif
> +
> +#endif /* __LINUX_LEDS_OF_H_INCLUDED */
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b122eeafb5dc..0fce71a06d68 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct device *parent,
>   extern void led_classdev_suspend(struct led_classdev *led_cdev);
>   extern void led_classdev_resume(struct led_classdev *led_cdev);
>
> +extern void led_put(struct led_classdev *led_cdev);
> +
>   /**
>    * led_blink_set - set blinking with software fallback
>    * @led_cdev: the LED to start blinking
>


-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply

* Re: [PATCH 1/3] leds: Add of_led_get() and led_put()
From: Tomi Valkeinen @ 2015-09-08 11:23 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
In-Reply-To: <20150825121853.GB14871@lunn.ch>

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

Hi Andrew,

On 25/08/15 15:18, Andrew Lunn wrote:
> On Tue, Aug 25, 2015 at 02:34:00PM +0300, Tomi Valkeinen wrote:
>> This patch adds basic support for a kernel driver to get a LED device.
>> This will be used by the led-backlight driver.
>>
>> Only OF version is implemented for now, and the behavior is similar to
>> PWM's of_pwm_get() and pwm_put().
> 
> Hi Tomi
> 
> Is this the correct way to do it?  I would of expected an xlate
> function.

I just sent v2, but I don't use xlate there.

If I understand the purpose of xlate (in pwm, for example) correctly,
xlate is a function in the pwm chip to allow custom bindings for the pwm
outputs from that pwm chip.

The problem with LEDs is that there's no "LED chip". Each LED is
modelled as individual device, without a well defined parent. Thus
there's no place to add such an xlate function.

Do you have any thoughts on what the xlate for LEDs should look like?

 Tomi


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

^ permalink raw reply

* [PATCHv2 3/3] devicetree: Add led-backlight binding
From: Tomi Valkeinen @ 2015-09-08 11:19 UTC (permalink / raw)
  To: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
  Cc: Andrew Lunn, Tomi Valkeinen
In-Reply-To: <1441711176-4258-1-git-send-email-tomi.valkeinen@ti.com>

Add DT binding for led-backlight.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 .../bindings/video/backlight/led-backlight.txt     | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/backlight/led-backlight.txt

diff --git a/Documentation/devicetree/bindings/video/backlight/led-backlight.txt b/Documentation/devicetree/bindings/video/backlight/led-backlight.txt
new file mode 100644
index 000000000000..fb77051ac230
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/led-backlight.txt
@@ -0,0 +1,30 @@
+led-backlight bindings
+
+Required properties:
+  - compatible: "led-backlight"
+  - leds: phandle to a led OF node [0]
+  - brightness-levels: Array of distinct LED brightness levels. These
+      are in the range from 0 to 255, passed to the LED class driver.
+  - default-brightness-level: the default brightness level (index into the
+      array defined by the "brightness-levels" property)
+  - power-supply: regulator for supply voltage
+
+Optional properties:
+  - enable-gpios: contains a single GPIO specifier for the GPIO which enables
+                  and disables the backlight (see GPIO binding[1])
+
+[0]: Documentation/devicetree/bindings/leds/common.txt
+[1]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+
+	backlight {
+		compatible = "led-backlight";
+		leds = <&backlight_led>;
+
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <6>;
+
+		power-supply = <&vdd_bl_reg>;
+		enable-gpios = <&gpio 58 0>;
+	};
-- 
2.1.4


^ permalink raw reply related

* [PATCHv2 2/3] backlight: add led-backlight driver
From: Tomi Valkeinen @ 2015-09-08 11:19 UTC (permalink / raw)
  To: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
  Cc: Andrew Lunn, Tomi Valkeinen
In-Reply-To: <1441711176-4258-1-git-send-email-tomi.valkeinen@ti.com>

This patch adds a led-backlight driver (led_bl), which is similar to
pwm_bl except the driver uses a LED class driver to adjust the
brightness in the HW.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/backlight/Kconfig  |   7 ++
 drivers/video/backlight/Makefile |   1 +
 drivers/video/backlight/led_bl.c | 235 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/video/backlight/led_bl.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 0505b796d743..d1336196aba2 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -453,6 +453,13 @@ config BACKLIGHT_BD6107
 	help
 	  If you have a Rohm BD6107 say Y to enable the backlight driver.
 
+config BACKLIGHT_LED
+	tristate "Generic LED based Backlight Driver"
+	depends on LEDS_CLASS && OF
+	help
+	  If you have a LCD backlight adjustable by LED class driver, say Y
+	  to enable this driver.
+
 endif # BACKLIGHT_CLASS_DEVICE
 
 endif # BACKLIGHT_LCD_SUPPORT
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index d67073f9d421..ecd321daee21 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -54,3 +54,4 @@ 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
+obj-$(CONFIG_BACKLIGHT_LED)		+= led_bl.o
diff --git a/drivers/video/backlight/led_bl.c b/drivers/video/backlight/led_bl.c
new file mode 100644
index 000000000000..581d545ea36b
--- /dev/null
+++ b/drivers/video/backlight/led_bl.c
@@ -0,0 +1,235 @@
+/*
+ * Copyright 2015 Texas Instruments
+ *
+ * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
+ *
+ * Based on pwm_bl.c
+ *
+ * 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; version 2 of the License.
+ */
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/leds.h>
+#include <linux/leds-of.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+struct led_bl_data {
+	struct device		*dev;
+	struct backlight_device	*bl_dev;
+
+	unsigned int		*levels;
+	bool			enabled;
+	struct regulator	*power_supply;
+	struct gpio_desc	*enable_gpio;
+
+	struct led_classdev *led_cdev;
+
+	unsigned int max_brightness;
+	unsigned int default_brightness;
+};
+
+static void led_bl_set_brightness(struct led_bl_data *priv, int brightness)
+{
+	int err;
+
+	if (!priv->enabled) {
+		err = regulator_enable(priv->power_supply);
+		if (err < 0)
+			dev_err(priv->dev, "failed to enable power supply\n");
+
+		if (priv->enable_gpio)
+			gpiod_set_value_cansleep(priv->enable_gpio, 1);
+	}
+
+	led_set_brightness(priv->led_cdev, priv->levels[brightness]);
+
+	priv->enabled = true;
+}
+
+static void led_bl_power_off(struct led_bl_data *priv)
+{
+	if (!priv->enabled)
+		return;
+
+	led_set_brightness(priv->led_cdev, LED_OFF);
+
+	if (priv->enable_gpio)
+		gpiod_set_value_cansleep(priv->enable_gpio, 0);
+
+	regulator_disable(priv->power_supply);
+
+	priv->enabled = false;
+}
+
+static int led_bl_update_status(struct backlight_device *bl)
+{
+	struct led_bl_data *priv = bl_get_data(bl);
+	int brightness = bl->props.brightness;
+
+	if (bl->props.power != FB_BLANK_UNBLANK ||
+	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
+	    bl->props.state & BL_CORE_FBBLANK)
+		brightness = 0;
+
+	if (brightness > 0)
+		led_bl_set_brightness(priv, brightness);
+	else
+		led_bl_power_off(priv);
+
+	return 0;
+}
+
+static const struct backlight_ops led_bl_ops = {
+	.update_status	= led_bl_update_status,
+};
+
+static int led_bl_parse_dt(struct device *dev,
+			   struct led_bl_data *priv)
+{
+	struct device_node *node = dev->of_node;
+	int num_levels;
+	u32 *levels;
+	u32 value;
+	int ret;
+
+	if (!node)
+		return -ENODEV;
+
+	num_levels = of_property_count_u32_elems(node, "brightness-levels");
+	if (num_levels < 0) {
+		dev_err(dev, "failed to find 'brightness-levels'\n");
+		return num_levels;
+	}
+
+	levels = devm_kzalloc(dev, sizeof(u32) * num_levels, GFP_KERNEL);
+	if (!levels)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(node, "brightness-levels",
+					 levels,
+					 num_levels);
+	if (ret < 0) {
+		dev_err(dev, "failed to parse 'brightness-levels'\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(node, "default-brightness-level", &value);
+	if (ret < 0) {
+		dev_err(dev, "failed to parse 'default-brightness-level'\n");
+		return ret;
+	}
+
+	if (value >= num_levels) {
+		dev_err(dev, "invalid default-brightness-level\n");
+		return -EINVAL;
+	}
+
+	priv->levels = levels;
+	priv->max_brightness = num_levels - 1;
+	priv->default_brightness = value;
+
+	priv->led_cdev = of_led_get(node);
+	if (IS_ERR(priv->led_cdev)) {
+		dev_err(dev, "failed to get LED device\n");
+		return PTR_ERR(priv->led_cdev);
+	}
+
+	return 0;
+}
+
+static int led_bl_probe(struct platform_device *pdev)
+{
+	struct backlight_properties props;
+	struct led_bl_data *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->dev = &pdev->dev;
+
+	ret = led_bl_parse_dt(&pdev->dev, priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to parse DT data\n");
+		return ret;
+	}
+
+	priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+			    GPIOD_OUT_LOW);
+	if (IS_ERR(priv->enable_gpio)) {
+		ret = PTR_ERR(priv->enable_gpio);
+		goto err;
+	}
+
+	priv->power_supply = devm_regulator_get(&pdev->dev, "power");
+	if (IS_ERR(priv->power_supply)) {
+		ret = PTR_ERR(priv->power_supply);
+		goto err;
+	}
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.type = BACKLIGHT_RAW;
+	props.max_brightness = priv->max_brightness;
+	priv->bl_dev = backlight_device_register(dev_name(&pdev->dev),
+			&pdev->dev, priv, &led_bl_ops, &props);
+	if (IS_ERR(priv->bl_dev)) {
+		dev_err(&pdev->dev, "failed to register backlight\n");
+		ret = PTR_ERR(priv->bl_dev);
+		goto err;
+	}
+
+	priv->bl_dev->props.brightness = priv->default_brightness;
+	backlight_update_status(priv->bl_dev);
+
+	return 0;
+
+err:
+	if (priv->led_cdev)
+		led_put(priv->led_cdev);
+
+	return ret;
+}
+
+static int led_bl_remove(struct platform_device *pdev)
+{
+	struct led_bl_data *priv = platform_get_drvdata(pdev);
+	struct backlight_device *bl = priv->bl_dev;
+
+	backlight_device_unregister(bl);
+
+	led_bl_power_off(priv);
+
+	led_put(priv->led_cdev);
+
+	return 0;
+}
+
+static const struct of_device_id led_bl_of_match[] = {
+	{ .compatible = "led-backlight" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, led_bl_of_match);
+
+static struct platform_driver led_bl_driver = {
+	.driver		= {
+		.name		= "led-backlight",
+		.of_match_table	= of_match_ptr(led_bl_of_match),
+	},
+	.probe		= led_bl_probe,
+	.remove		= led_bl_remove,
+};
+
+module_platform_driver(led_bl_driver);
+
+MODULE_DESCRIPTION("LED based Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:led-backlight");
-- 
2.1.4


^ permalink raw reply related

* [PATCHv2 1/3] leds: Add of_led_get() and led_put()
From: Tomi Valkeinen @ 2015-09-08 11:19 UTC (permalink / raw)
  To: Jacek Anaszewski, Jingoo Han, Lee Jones, linux-leds, linux-fbdev
  Cc: Andrew Lunn, Tomi Valkeinen
In-Reply-To: <1441711176-4258-1-git-send-email-tomi.valkeinen@ti.com>

This patch adds basic support for a kernel driver to get a LED device.
This will be used by the led-backlight driver.

Only OF version is implemented for now, and the behavior is similar to
PWM's of_pwm_get() and pwm_put().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/leds/Makefile    |  6 +++-
 drivers/leds/led-class.c | 13 +++++++-
 drivers/leds/led-of.c    | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/leds/leds.h      |  1 +
 include/linux/leds-of.h  | 26 +++++++++++++++
 include/linux/leds.h     |  2 ++
 6 files changed, 128 insertions(+), 2 deletions(-)
 create mode 100644 drivers/leds/led-of.c
 create mode 100644 include/linux/leds-of.h

diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 8d6a24a2f513..6fd22e411810 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -1,7 +1,11 @@
 
 # LED Core
 obj-$(CONFIG_NEW_LEDS)			+= led-core.o
-obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
+
+obj-$(CONFIG_LEDS_CLASS)		+= led-class-objs.o
+led-class-objs-y			:= led-class.o
+led-class-objs-$(CONFIG_OF)		+= led-of.o
+
 obj-$(CONFIG_LEDS_CLASS_FLASH)		+= led-class-flash.o
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index beabfbc6f7cd..1234f9dc3537 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -22,7 +22,7 @@
 #include <linux/timer.h>
 #include "leds.h"
 
-static struct class *leds_class;
+struct class *leds_class;
 
 static ssize_t brightness_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
@@ -216,6 +216,17 @@ static int led_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(leds_class_dev_pm_ops, led_suspend, led_resume);
 
+/**
+ * led_put() - release a LED device, reserved with led_get()
+ * @led_cdev: LED device
+ */
+void led_put(struct led_classdev *led_cdev)
+{
+	put_device(led_cdev->dev);
+	module_put(led_cdev->dev->parent->driver->owner);
+}
+EXPORT_SYMBOL_GPL(led_put);
+
 static int match_name(struct device *dev, const void *data)
 {
 	if (!dev_name(dev))
diff --git a/drivers/leds/led-of.c b/drivers/leds/led-of.c
new file mode 100644
index 000000000000..32631682be07
--- /dev/null
+++ b/drivers/leds/led-of.c
@@ -0,0 +1,82 @@
+/*
+ * LED Class Core OF support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/leds.h>
+#include <linux/of.h>
+#include <linux/leds-of.h>
+#include <linux/module.h>
+
+#include "leds.h"
+
+/* find OF node for the given led_cdev */
+static struct device_node *find_led_of_node(struct led_classdev *led_cdev)
+{
+	struct device *led_dev = led_cdev->dev;
+	struct device_node *child;
+
+	for_each_child_of_node(led_dev->parent->of_node, child) {
+		if (of_property_match_string(child, "label", led_cdev->name) = 0)
+			return child;
+	}
+
+	return NULL;
+}
+
+static int led_match_led_node(struct device *led_dev, const void *data)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(led_dev);
+	const struct device_node *target_node = data;
+	struct device_node *led_node;
+
+	led_node = find_led_of_node(led_cdev);
+	if (!led_node)
+		return 0;
+
+	of_node_put(led_node);
+
+	return led_node = target_node ? 1 : 0;
+}
+
+/**
+ * of_led_get() - request a LED device via the LED framework
+ * @np: device node to get the LED device from
+ *
+ * Returns the LED device parsed from the phandle specified in the "leds"
+ * property of a device tree node or a negative error-code on failure.
+ *
+ * The caller must use led_put() to release the device after use.
+ */
+struct led_classdev *of_led_get(struct device_node *np)
+{
+	struct device *led_dev;
+	struct led_classdev *led_cdev;
+	struct device_node *led_node;
+
+	led_node = of_parse_phandle(np, "leds", 0);
+	if (!led_node)
+		return ERR_PTR(-ENODEV);
+
+	led_dev = class_find_device(leds_class, NULL, led_node,
+		led_match_led_node);
+
+	of_node_put(led_node);
+
+	if (!led_dev) {
+		pr_err("failed to find led device for node %s, deferring probe\n",
+			of_node_full_name(led_node));
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	led_cdev = dev_get_drvdata(led_dev);
+
+	if (!try_module_get(led_cdev->dev->parent->driver->owner))
+		return ERR_PTR(-ENODEV);
+
+	return led_cdev;
+}
+EXPORT_SYMBOL_GPL(of_led_get);
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index bc89d7ace2c4..ccc3abb417d4 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -46,6 +46,7 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
 
 void led_stop_software_blink(struct led_classdev *led_cdev);
 
+extern struct class *leds_class;
 extern struct rw_semaphore leds_list_lock;
 extern struct list_head leds_list;
 
diff --git a/include/linux/leds-of.h b/include/linux/leds-of.h
new file mode 100644
index 000000000000..7e8e64bd9811
--- /dev/null
+++ b/include/linux/leds-of.h
@@ -0,0 +1,26 @@
+/*
+ * OF support for leds
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __LINUX_LEDS_OF_H_INCLUDED
+#define __LINUX_LEDS_OF_H_INCLUDED
+
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_LEDS_CLASS)
+
+extern struct led_classdev *of_led_get(struct device_node *np);
+
+#else
+
+static inline struct led_classdev *of_led_get(struct device_node *np)
+{
+	return -ENODEV;
+}
+
+#endif
+
+#endif /* __LINUX_LEDS_OF_H_INCLUDED */
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b122eeafb5dc..0fce71a06d68 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -113,6 +113,8 @@ extern void devm_led_classdev_unregister(struct device *parent,
 extern void led_classdev_suspend(struct led_classdev *led_cdev);
 extern void led_classdev_resume(struct led_classdev *led_cdev);
 
+extern void led_put(struct led_classdev *led_cdev);
+
 /**
  * led_blink_set - set blinking with software fallback
  * @led_cdev: the LED to start blinking
-- 
2.1.4


^ permalink raw reply related


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