* Re: Question on s3fb DDC support
From: Ondrej Zary @ 2014-08-03 16:13 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20140803144236.0494b47a@endymion.delvare>
On Sunday 03 August 2014 18:03:31 Jean Delvare wrote:
> Hi Geert,
>
> On Sun, 3 Aug 2014 16:47:17 +0200, Geert Uytterhoeven wrote:
> > On Sun, Aug 3, 2014 at 2:42 PM, Jean Delvare <jdelvare@suse.de> wrote:
> > > I am looking at the DDC / I2C implementation in that driver and I'm not
> > > sure I understand how it works. Specifically I am wondering about bit
> > > DDC_DRIVE_EN. It is set unconditionally in s3fb_ddc_setscl() and
> > > s3fb_ddc_setsda() and never cleared explicitly. If this bit is a
> > > regular bit then I don't understand why it is not just set at driver
> > > initialization time. Or is this bit self-clearing and/or not-sticking?
> > >
> > > The reason why I am asking is that I don't think the code in these
> > > functions is completely correct. For the I2C protocol, 1 is the natural
> > > state of both lines and the devices on the bus should only ever pull
> > > the lines low to force a 0 state. They should never force a 1 state, as
> > > this breaks some protocol features (specifically arbitration and clock
> > > stretching.) My interpretation of the current code is that the lines
> > > are forced to both 0 and 1 by the master, which is not correct. But as
> > > I don't know how DDC_DRIVE_EN works, I'm not sure, and if it's indeed
> > > broken [1], I'm also not sure how to fix it.
> > >
> > > So if you could clarify how bit DDC_DRIVE_EN works, that would be
> > > great.
> >
> > http://www.vgamuseum.info/images/stories/doc/s3/virge.pdf, p. 24-11:
> >
> > Bit 0 SCW - Serial Clock Write
> > 0 = Pin 205 is driven low
> > 1 = Pin 205 is tri-stated
> > Pin 205 carries the DDC/I 2 C clock, depending on the operational
> > mode. When pin 205
> > is tri-stated, other devices may drive this line. The actual state of
> > the pin is read via
> > bit 2 of this register.
> > Bit 1 SDW - Serial Data Write
> > 0 = Pin 206 is driven low
> > 1 = Pin 206 is tri-stated
> > Pin 206 carries the DDC/I 2 C data, depending on the operational mode.
> > When pin 206
> > is tri-stated, other devices may drive this line. The actual state of
> > the pin is read via
> > bit 3 of this register.
> > Bit 2 SCR - Serial Clock Read (Read Only)
> > 0 = Pin 205 is low
> > 1 = Pin 205 is tri-stated (no device is driving this line)
> > Bit 3 SDR - Serial Data Read (Read Only)
> > 0 = Pin 206 is low
> > 1 = Pin 206 is tri-stated (no device is driving this line)
> > Bit 4 SPE - Serial Port Enable
> > 0 = Use of bits 1-0 of this register disabled
> > 1 = Use of bits 1-0 of this register enabled
> >
> > Still doesn't explain well the real meaning of bit 4, but it does confirm
> > that writing a 1 to bit 0 or bit 1 doesn't drive the line high.
>
> Very nice, thanks for the documentation. So the driver code looks
> correct and I can move on to the remaining two suspect drivers (intelfb
> and i740fb.)
The i740fb code is also my work - and done the same way without any docs.
--
Ondrej Zary
^ permalink raw reply
* Re: Question on s3fb DDC support
From: Jean Delvare @ 2014-08-03 16:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20140803144236.0494b47a@endymion.delvare>
On Sun, 3 Aug 2014 18:13:10 +0200, Ondrej Zary wrote:
> The i740fb code is also my work - and done the same way without any docs.
http://www.vgamuseum.info/images/stories/doc/intel/i740.pdf
:-)
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: Question on s3fb DDC support
From: Ondrej Zary @ 2014-08-03 16:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20140803144236.0494b47a@endymion.delvare>
On Sunday 03 August 2014 18:20:32 Jean Delvare wrote:
> On Sun, 3 Aug 2014 18:13:10 +0200, Ondrej Zary wrote:
> > The i740fb code is also my work - and done the same way without any docs.
>
> http://www.vgamuseum.info/images/stories/doc/intel/i740.pdf
>
> :-)
The problem is that there's no I2C register description in that document :(
--
Ondrej Zary
^ permalink raw reply
* Re: Question on s3fb DDC support
From: Jean Delvare @ 2014-08-03 20:56 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20140803144236.0494b47a@endymion.delvare>
On Sun, 3 Aug 2014 18:33:04 +0200, Ondrej Zary wrote:
> On Sunday 03 August 2014 18:20:32 Jean Delvare wrote:
> > On Sun, 3 Aug 2014 18:13:10 +0200, Ondrej Zary wrote:
> > > The i740fb code is also my work - and done the same way without any docs.
> >
> > http://www.vgamuseum.info/images/stories/doc/intel/i740.pdf
> >
> > :-)
>
> The problem is that there's no I2C register description in that document :(
Yep, seen that after sending the link, sorry :(
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH v4] video: hyperv: hyperv_fb: refresh the VM screen by force on VM panic
From: Tomi Valkeinen @ 2014-08-04 6:36 UTC (permalink / raw)
To: Dexuan Cui
Cc: linux-fbdev, gregkh, jasowang, driverdev-devel, linux-kernel,
olaf, apw, plagnioj, haiyangz, dan.carpenter
In-Reply-To: <1406892474-6548-1-git-send-email-decui@microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]
On 01/08/14 14:27, Dexuan Cui wrote:
> Currently the VSC has no chance to notify the VSP of the dirty rectangle on VM
> panic because the notification work is done in a workqueue, and in panic() the
> kernel typically ends up in an infinite loop, and a typical kernel config has
> CONFIG_PREEMPT_VOLUNTARY=y and CONFIG_PREEMPT is not set, so a context switch
> can't happen in panic() and the workqueue won't have a chance to run. As a
> result, the VM Connection window can't refresh until it's closed and we
> re-connect to the VM.
>
> We can register a handler on panic_notifier_list: the handler can notify
> the VSC and switch the framebuffer driver to a "synchronous mode", meaning
> the VSC flushes any future framebuffer change to the VSP immediately.
>
> v2: removed the MS-TFS line in the commit message
> v3: remove some 'unlikely' markings
> v4: avoid global variables as Tomi Valkeinen suggested
>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/video/fbdev/hyperv_fb.c | 62 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 59 insertions(+), 3 deletions(-)
Thanks, looks fine to me. Queuing for 3.17.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: ARM CLCD: Fix DT-related build problems
From: Tomi Valkeinen @ 2014-08-04 6:38 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1406904214-7360-1-git-send-email-pawel.moll@arm.com>
[-- Attachment #1: Type: text/plain, Size: 863 bytes --]
On 01/08/14 17:43, Pawel Moll wrote:
> This patch fixes the following error when !CONFIG_OF:
>
> drivers/video/fbdev/amba-clcd.c:800:54: warning: ‘struct amba_dev’ declared inside parameter list [enabled by default]
> static struct clcd_board *clcdfb_of_get_board(struct amba_dev *dev)
> ^
> and adds a missing Kconfig select causing this
> when CONFIG_OF && !CONFIG_FB_MODE_HELPERS:
>
> drivers/video/fbdev/amba-clcd.c:567: undefined reference to `fb_videomode_from_videomode'
>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> ---
>
> Hi Tomi,
>
> This fixes two small problems detected in next by the kbuild test robot
> (thanks, Fengguang!) Hope you can apply it before the merge window.
Thanks, applied.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: amba-clcd: Select FB_MODE_HELPERS
From: Tomi Valkeinen @ 2014-08-04 6:48 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1406908430-18487-1-git-send-email-broonie@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 889 bytes --]
Hi,
On 01/08/14 18:53, Mark Brown wrote:
> From: Mark Brown <broonie@linaro.org>
>
> The AMBA CLCD driver uses fb_videomode_from_videomode() so it needs to
> select DB_MODE_HELPERS as well as VIDEOMODE_HELPERS.
>
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
> drivers/video/fbdev/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index 37d473f..e222048 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -280,6 +280,7 @@ config FB_ARMCLCD
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> + select FB_MODE_HELPERS
> select VIDEOMODE_HELPERS if OF
> help
> This framebuffer device driver is for the ARM PrimeCell PL110
Pawel sent a patch fixing the same issue. I've applied that one.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] msm: msm_fb: Add remove function for platform driver for clean unloading
From: Pramod Gurav @ 2014-08-04 14:55 UTC (permalink / raw)
To: linux-fbdev, linux-kernel
Cc: Pramod Gurav, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Stephen Boyd, Jingoo Han, Rob Clark
This adds a remove function to platform driver structure so that
resources are released when driver is unloaded.
Moved kzalloc to use managed resource and adds a return check if failed.
CC: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
CC: Stephen Boyd <sboyd@codeaurora.org>
CC: Jingoo Han <jg1.han@samsung.com>
CC: Rob Clark <robdclark@gmail.com>
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
drivers/video/fbdev/msm/msm_fb.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/msm/msm_fb.c b/drivers/video/fbdev/msm/msm_fb.c
index 1374803..ac277e2 100644
--- a/drivers/video/fbdev/msm/msm_fb.c
+++ b/drivers/video/fbdev/msm/msm_fb.c
@@ -553,6 +553,7 @@ static int msmfb_probe(struct platform_device *pdev)
fb = framebuffer_alloc(sizeof(struct msmfb_info), &pdev->dev);
if (!fb)
return -ENOMEM;
+
msmfb = fb->par;
msmfb->fb = fb;
msmfb->panel = panel;
@@ -569,8 +570,13 @@ static int msmfb_probe(struct platform_device *pdev)
mutex_init(&msmfb->panel_init_lock);
init_waitqueue_head(&msmfb->frame_wq);
INIT_WORK(&msmfb->resume_work, power_on_panel);
- msmfb->black = kzalloc(msmfb->fb->var.bits_per_pixel*msmfb->xres,
- GFP_KERNEL);
+ msmfb->black = devm_kzalloc(&pdev->dev,
+ msmfb->fb->var.bits_per_pixel*msmfb->xres,
+ GFP_KERNEL);
+ if (!msmfb->black) {
+ ret = -ENOMEM;
+ goto error_register_framebuffer;
+ }
printk(KERN_INFO "msmfb_probe() installing %d x %d panel\n",
msmfb->xres, msmfb->yres);
@@ -589,6 +595,8 @@ static int msmfb_probe(struct platform_device *pdev)
msmfb->sleeping = WAKING;
+ platform_set_drvdata(pdev, msmfb);
+
return 0;
error_register_framebuffer:
@@ -598,13 +606,27 @@ error_setup_fbmem:
return ret;
}
+static int msmfb_remove(struct platform_device *pdev)
+{
+ struct msmfb_info *msmfb = NULL;
+
+ msmfb = platform_get_drvdata(pdev);
+ if (msmfb) {
+ unregister_framebuffer(msmfb->fb);
+ iounmap(msmfb->fb->screen_base);
+ framebuffer_release(msmfb->fb);
+ }
+
+ return 0;
+}
+
static struct platform_driver msm_panel_driver = {
/* need to write remove */
.probe = msmfb_probe,
+ .remove = msmfb_remove,
.driver = {.name = "msm_panel"},
};
-
static int msmfb_add_mdp_device(struct device *dev,
struct class_interface *class_intf)
{
--
1.7.9.5
^ permalink raw reply related
* [PATCH] video/fbdev: Always built-in video= cmdline parsing
From: Daniel Vetter @ 2014-08-06 9:43 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, Tomi Valkeinen,
Plagniol-Villard, linux-fbdev
In-Reply-To: <1407312512-6533-1-git-send-email-daniel.vetter@ffwll.ch>
In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.
But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.
Whiel at it fix the checkpatch fail in this code.
Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
drivers/video/fbdev/core/Makefile | 2 +-
drivers/video/fbdev/core/fb_cmdline.c | 103 ++++++++++++++++++++++++++++++++++
drivers/video/fbdev/core/fbmem.c | 92 ------------------------------
3 files changed, 104 insertions(+), 93 deletions(-)
create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y += fb_notify.o
+obj-y += fb_notify.o fb_cmdline.o
obj-$(CONFIG_FB) += fb.o
fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..91503a43213e
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,103 @@
+/*
+ * linux/drivers/video/fb_cmdline.c
+ *
+ * Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ * Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name: framebuffer name as it would appear in
+ * the boot parameter line
+ * (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+ char *opt, *options = NULL;
+ int retval = 0;
+ int name_len = strlen(name), i;
+
+ if (name_len && ofonly && strncmp(name, "offb", 4))
+ retval = 1;
+
+ if (name_len && !retval) {
+ for (i = 0; i < FB_MAX; i++) {
+ if (video_options[i] = NULL)
+ continue;
+ if (!video_options[i][0])
+ continue;
+ opt = video_options[i];
+ if (!strncmp(name, opt, name_len) &&
+ opt[name_len] = ':')
+ options = opt + name_len + 1;
+ }
+ }
+ /* No match, pass global option */
+ if (!options && option && fb_mode_option)
+ options = kstrdup(fb_mode_option, GFP_KERNEL);
+ if (options && !strncmp(options, "off", 3))
+ retval = 1;
+
+ if (option)
+ *option = options;
+
+ return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ * video_setup - process command line options
+ * @options: string of options
+ *
+ * Process command line options for frame buffer subsystem.
+ *
+ * NOTE: This function is a __setup and __init function.
+ * It only stores the options. Drivers have to call
+ * fb_get_options() as necessary.
+ *
+ * Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+ int i, global = 0;
+
+ if (!options || !*options)
+ global = 1;
+
+ if (!global && !strncmp(options, "ofonly", 6)) {
+ ofonly = 1;
+ global = 1;
+ }
+
+ if (!global && !strchr(options, ':')) {
+ fb_mode_option = options;
+ global = 1;
+ }
+
+ if (!global) {
+ for (i = 0; i < FB_MAX; i++) {
+ if (video_options[i] = NULL) {
+ video_options[i] = options;
+ break;
+ }
+ }
+ }
+
+ return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
return err;
}
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name: framebuffer name as it would appear in
- * the boot parameter line
- * (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
- char *opt, *options = NULL;
- int retval = 0;
- int name_len = strlen(name), i;
-
- if (name_len && ofonly && strncmp(name, "offb", 4))
- retval = 1;
-
- if (name_len && !retval) {
- for (i = 0; i < FB_MAX; i++) {
- if (video_options[i] = NULL)
- continue;
- if (!video_options[i][0])
- continue;
- opt = video_options[i];
- if (!strncmp(name, opt, name_len) &&
- opt[name_len] = ':')
- options = opt + name_len + 1;
- }
- }
- /* No match, pass global option */
- if (!options && option && fb_mode_option)
- options = kstrdup(fb_mode_option, GFP_KERNEL);
- if (options && !strncmp(options, "off", 3))
- retval = 1;
-
- if (option)
- *option = options;
-
- return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- * video_setup - process command line options
- * @options: string of options
- *
- * Process command line options for frame buffer subsystem.
- *
- * NOTE: This function is a __setup and __init function.
- * It only stores the options. Drivers have to call
- * fb_get_options() as necessary.
- *
- * Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
- int i, global = 0;
-
- if (!options || !*options)
- global = 1;
-
- if (!global && !strncmp(options, "ofonly", 6)) {
- ofonly = 1;
- global = 1;
- }
-
- if (!global && !strchr(options, ':')) {
- fb_mode_option = options;
- global = 1;
- }
-
- if (!global) {
- for (i = 0; i < FB_MAX; i++) {
- if (video_options[i] = NULL) {
- video_options[i] = options;
- break;
- }
-
- }
- }
-
- return 1;
-}
-__setup("video=", video_setup);
-#endif
-
MODULE_LICENSE("GPL");
--
2.0.1
^ permalink raw reply related
* Re: [PATCH] video/fbdev: Always built-in video= cmdline parsing
From: Geert Uytterhoeven @ 2014-08-06 10:27 UTC (permalink / raw)
To: Daniel Vetter
Cc: Intel Graphics Development, Tomi Valkeinen, Plagniol-Villard,
Linux Fbdev development list, DRI Development
In-Reply-To: <1407318226-23169-1-git-send-email-daniel.vetter@ffwll.ch>
On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
>
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.
How much is "so little"? Think memory-constrained systems.
You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index fa306538dac2..891c1f890e03 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,4 +1,4 @@
> -obj-y += fb_notify.o
Oh, this is already unconditional. Who are its users?
> +obj-y += fb_notify.o fb_cmdline.o
> obj-$(CONFIG_FB) += fb.o
> fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> modedb.o fbcvt.o
> diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> new file mode 100644
> index 000000000000..91503a43213e
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_cmdline.c
> @@ -0,0 +1,103 @@
> +/*
> + * linux/drivers/video/fb_cmdline.c
> + *
> + * Copyright (C) 2014 Intel Corp
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file COPYING in the main directory of this archive
> + * for more details.
> + *
> + * Authors:
> + * Vetter <danie.vetter@ffwll.ch>
> + */
The above chunk doesn't sound appropriate for extracting existing code...
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
* [PATCH] video/fbdev: Always built-in video= cmdline parsing
From: Daniel Vetter @ 2014-08-06 12:05 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Intel Graphics Development, Tomi Valkeinen,
Plagniol-Villard, linux-fbdev
In-Reply-To: <1407312512-6533-1-git-send-email-daniel.vetter@ffwll.ch>
In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.
But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.
Whiel at it fix the checkpatch fail in this code.
v2: Also move fb_mode_option. Spotted by the kbuild.
Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
drivers/video/fbdev/core/Makefile | 2 +-
drivers/video/fbdev/core/fb_cmdline.c | 106 ++++++++++++++++++++++++++++++++++
drivers/video/fbdev/core/fbmem.c | 92 -----------------------------
drivers/video/fbdev/core/modedb.c | 3 -
4 files changed, 107 insertions(+), 96 deletions(-)
create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y += fb_notify.o
+obj-y += fb_notify.o fb_cmdline.o
obj-$(CONFIG_FB) += fb.o
fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..e8ce614bdd03
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,106 @@
+/*
+ * linux/drivers/video/fb_cmdline.c
+ *
+ * Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ * Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+const char *fb_mode_option;
+EXPORT_SYMBOL_GPL(fb_mode_option);
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name: framebuffer name as it would appear in
+ * the boot parameter line
+ * (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+ char *opt, *options = NULL;
+ int retval = 0;
+ int name_len = strlen(name), i;
+
+ if (name_len && ofonly && strncmp(name, "offb", 4))
+ retval = 1;
+
+ if (name_len && !retval) {
+ for (i = 0; i < FB_MAX; i++) {
+ if (video_options[i] = NULL)
+ continue;
+ if (!video_options[i][0])
+ continue;
+ opt = video_options[i];
+ if (!strncmp(name, opt, name_len) &&
+ opt[name_len] = ':')
+ options = opt + name_len + 1;
+ }
+ }
+ /* No match, pass global option */
+ if (!options && option && fb_mode_option)
+ options = kstrdup(fb_mode_option, GFP_KERNEL);
+ if (options && !strncmp(options, "off", 3))
+ retval = 1;
+
+ if (option)
+ *option = options;
+
+ return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ * video_setup - process command line options
+ * @options: string of options
+ *
+ * Process command line options for frame buffer subsystem.
+ *
+ * NOTE: This function is a __setup and __init function.
+ * It only stores the options. Drivers have to call
+ * fb_get_options() as necessary.
+ *
+ * Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+ int i, global = 0;
+
+ if (!options || !*options)
+ global = 1;
+
+ if (!global && !strncmp(options, "ofonly", 6)) {
+ ofonly = 1;
+ global = 1;
+ }
+
+ if (!global && !strchr(options, ':')) {
+ fb_mode_option = options;
+ global = 1;
+ }
+
+ if (!global) {
+ for (i = 0; i < FB_MAX; i++) {
+ if (video_options[i] = NULL) {
+ video_options[i] = options;
+ break;
+ }
+ }
+ }
+
+ return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
return err;
}
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name: framebuffer name as it would appear in
- * the boot parameter line
- * (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
- char *opt, *options = NULL;
- int retval = 0;
- int name_len = strlen(name), i;
-
- if (name_len && ofonly && strncmp(name, "offb", 4))
- retval = 1;
-
- if (name_len && !retval) {
- for (i = 0; i < FB_MAX; i++) {
- if (video_options[i] = NULL)
- continue;
- if (!video_options[i][0])
- continue;
- opt = video_options[i];
- if (!strncmp(name, opt, name_len) &&
- opt[name_len] = ':')
- options = opt + name_len + 1;
- }
- }
- /* No match, pass global option */
- if (!options && option && fb_mode_option)
- options = kstrdup(fb_mode_option, GFP_KERNEL);
- if (options && !strncmp(options, "off", 3))
- retval = 1;
-
- if (option)
- *option = options;
-
- return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- * video_setup - process command line options
- * @options: string of options
- *
- * Process command line options for frame buffer subsystem.
- *
- * NOTE: This function is a __setup and __init function.
- * It only stores the options. Drivers have to call
- * fb_get_options() as necessary.
- *
- * Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
- int i, global = 0;
-
- if (!options || !*options)
- global = 1;
-
- if (!global && !strncmp(options, "ofonly", 6)) {
- ofonly = 1;
- global = 1;
- }
-
- if (!global && !strchr(options, ':')) {
- fb_mode_option = options;
- global = 1;
- }
-
- if (!global) {
- for (i = 0; i < FB_MAX; i++) {
- if (video_options[i] = NULL) {
- video_options[i] = options;
- break;
- }
-
- }
- }
-
- return 1;
-}
-__setup("video=", video_setup);
-#endif
-
MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index a9a907c440d7..388f7971494b 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -29,9 +29,6 @@
#define DPRINTK(fmt, args...)
#endif
-const char *fb_mode_option;
-EXPORT_SYMBOL_GPL(fb_mode_option);
-
/*
* Standard video mode definitions (taken from XFree86)
*/
--
2.0.1
^ permalink raw reply related
* Re: [PATCH] video/fbdev: Always built-in video= cmdline parsing
From: Daniel Vetter @ 2014-08-06 12:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Fbdev development list, Daniel Vetter,
Intel Graphics Development, DRI Development, Tomi Valkeinen,
Plagniol-Villard
In-Reply-To: <CAMuHMdUmy=PLh_z=D9S=oj2dxG74sXC-3_-7V8PCQgNMLz8ngQ@mail.gmail.com>
On Wed, Aug 06, 2014 at 12:27:32PM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > In drm/i915 we want to get at the video= cmdline modes even when we
> > don't have fbdev support enabled, so that users can always override
> > the kernel's initial mode selection.
> >
> > But that gives us a direct depency upon the parsing code in the fbdev
> > subsystem. Since it's so little code just extract these 2 functions
> > and always build them in.
>
> How much is "so little"? Think memory-constrained systems.
>
> You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.
I'll do it as an option selected by FB and DRM then.
>
> > diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> > index fa306538dac2..891c1f890e03 100644
> > --- a/drivers/video/fbdev/core/Makefile
> > +++ b/drivers/video/fbdev/core/Makefile
> > @@ -1,4 +1,4 @@
> > -obj-y += fb_notify.o
>
> Oh, this is already unconditional. Who are its users?
Welcome in fbdev-land. Not going to fix this, since my dragon-slaying
sword is already broken.
>
> > +obj-y += fb_notify.o fb_cmdline.o
> > obj-$(CONFIG_FB) += fb.o
> > fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> > modedb.o fbcvt.o
> > diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> > new file mode 100644
> > index 000000000000..91503a43213e
> > --- /dev/null
> > +++ b/drivers/video/fbdev/core/fb_cmdline.c
> > @@ -0,0 +1,103 @@
> > +/*
> > + * linux/drivers/video/fb_cmdline.c
> > + *
> > + * Copyright (C) 2014 Intel Corp
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License. See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + * Authors:
> > + * Vetter <danie.vetter@ffwll.ch>
> > + */
>
> The above chunk doesn't sound appropriate for extracting existing code...
Well it all horribly predates git history, so no idea who actually wrote
this. I'll copy over the old header on top.
-Daniel
>
> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
From: Daniel Vetter @ 2014-08-06 12:52 UTC (permalink / raw)
To: DRI Development
Cc: linux-fbdev, Daniel Vetter, Intel Graphics Development,
Tomi Valkeinen, Geert Uytterhoeven, Plagniol-Villard
In-Reply-To: <1407312512-6533-1-git-send-email-daniel.vetter@ffwll.ch>
In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.
But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.
Whiel at it fix the checkpatch fail in this code.
v2: Also move fb_mode_option. Spotted by the kbuild.
v3: Review from Geert:
- Keep the old copyright notice from fb_mem.c, although I have no
idea what exactly applies.
- Only compile this when needed.
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
drivers/video/fbdev/Kconfig | 4 ++
drivers/video/fbdev/core/Makefile | 1 +
drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
drivers/video/fbdev/core/fbmem.c | 92 ----------------------------
drivers/video/fbdev/core/modedb.c | 3 -
5 files changed, 115 insertions(+), 95 deletions(-)
create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 59c98bfd5a8a..f1458c95a688 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -4,6 +4,7 @@
menuconfig FB
tristate "Support for frame buffer devices"
+ select FB_CMDLINE
---help---
The frame buffer device provides an abstraction for the graphics
hardware. It represents the frame buffer of some video hardware and
@@ -52,6 +53,9 @@ config FIRMWARE_EDID
combination with certain motherboards and monitors are known to
suffer from this problem.
+config FB_CMDLINE
+ bool
+
config FB_DDC
tristate
depends on FB
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..67f28e20a892 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,5 @@
obj-y += fb_notify.o
+obj-$(CONFIG_FB_CMDLINE) += fb_cmdline.o
obj-$(CONFIG_FB) += fb.o
fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..39509ccd92f1
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,110 @@
+/*
+ * linux/drivers/video/fb_cmdline.c
+ *
+ * Copyright (C) 2014 Intel Corp
+ * Copyright (C) 1994 Martin Schaller
+ *
+ * 2001 - Documented with DocBook
+ * - Brad Douglas <brad@neruo.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ * Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+const char *fb_mode_option;
+EXPORT_SYMBOL_GPL(fb_mode_option);
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name: framebuffer name as it would appear in
+ * the boot parameter line
+ * (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+ char *opt, *options = NULL;
+ int retval = 0;
+ int name_len = strlen(name), i;
+
+ if (name_len && ofonly && strncmp(name, "offb", 4))
+ retval = 1;
+
+ if (name_len && !retval) {
+ for (i = 0; i < FB_MAX; i++) {
+ if (video_options[i] = NULL)
+ continue;
+ if (!video_options[i][0])
+ continue;
+ opt = video_options[i];
+ if (!strncmp(name, opt, name_len) &&
+ opt[name_len] = ':')
+ options = opt + name_len + 1;
+ }
+ }
+ /* No match, pass global option */
+ if (!options && option && fb_mode_option)
+ options = kstrdup(fb_mode_option, GFP_KERNEL);
+ if (options && !strncmp(options, "off", 3))
+ retval = 1;
+
+ if (option)
+ *option = options;
+
+ return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ * video_setup - process command line options
+ * @options: string of options
+ *
+ * Process command line options for frame buffer subsystem.
+ *
+ * NOTE: This function is a __setup and __init function.
+ * It only stores the options. Drivers have to call
+ * fb_get_options() as necessary.
+ *
+ * Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+ int i, global = 0;
+
+ if (!options || !*options)
+ global = 1;
+
+ if (!global && !strncmp(options, "ofonly", 6)) {
+ ofonly = 1;
+ global = 1;
+ }
+
+ if (!global && !strchr(options, ':')) {
+ fb_mode_option = options;
+ global = 1;
+ }
+
+ if (!global) {
+ for (i = 0; i < FB_MAX; i++) {
+ if (video_options[i] = NULL) {
+ video_options[i] = options;
+ break;
+ }
+ }
+ }
+
+ return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
return err;
}
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name: framebuffer name as it would appear in
- * the boot parameter line
- * (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
- char *opt, *options = NULL;
- int retval = 0;
- int name_len = strlen(name), i;
-
- if (name_len && ofonly && strncmp(name, "offb", 4))
- retval = 1;
-
- if (name_len && !retval) {
- for (i = 0; i < FB_MAX; i++) {
- if (video_options[i] = NULL)
- continue;
- if (!video_options[i][0])
- continue;
- opt = video_options[i];
- if (!strncmp(name, opt, name_len) &&
- opt[name_len] = ':')
- options = opt + name_len + 1;
- }
- }
- /* No match, pass global option */
- if (!options && option && fb_mode_option)
- options = kstrdup(fb_mode_option, GFP_KERNEL);
- if (options && !strncmp(options, "off", 3))
- retval = 1;
-
- if (option)
- *option = options;
-
- return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- * video_setup - process command line options
- * @options: string of options
- *
- * Process command line options for frame buffer subsystem.
- *
- * NOTE: This function is a __setup and __init function.
- * It only stores the options. Drivers have to call
- * fb_get_options() as necessary.
- *
- * Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
- int i, global = 0;
-
- if (!options || !*options)
- global = 1;
-
- if (!global && !strncmp(options, "ofonly", 6)) {
- ofonly = 1;
- global = 1;
- }
-
- if (!global && !strchr(options, ':')) {
- fb_mode_option = options;
- global = 1;
- }
-
- if (!global) {
- for (i = 0; i < FB_MAX; i++) {
- if (video_options[i] = NULL) {
- video_options[i] = options;
- break;
- }
-
- }
- }
-
- return 1;
-}
-__setup("video=", video_setup);
-#endif
-
MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index a9a907c440d7..388f7971494b 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -29,9 +29,6 @@
#define DPRINTK(fmt, args...)
#endif
-const char *fb_mode_option;
-EXPORT_SYMBOL_GPL(fb_mode_option);
-
/*
* Standard video mode definitions (taken from XFree86)
*/
--
2.0.1
^ permalink raw reply related
* [PATCH 4/4] drivers: video: fbdev: atmel_lcdfb.c: fix error return code
From: Julia Lawall @ 2014-08-06 20:12 UTC (permalink / raw)
To: Nicolas Ferre
Cc: kernel-janitors, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev, linux-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
Convert a zero return value on error to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The error codes chosen are the ones that are commonly used elsewhere or
these functions. Perhaps something else is wanted.
drivers/video/fbdev/atmel_lcdfb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
index 92640d4..1d8bdb9 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1102,12 +1102,14 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
timings = of_get_display_timings(display_np);
if (!timings) {
dev_err(dev, "failed to get display timings\n");
+ ret = -EINVAL;
goto put_display_node;
}
timings_np = of_find_node_by_name(display_np, "display-timings");
if (!timings_np) {
dev_err(dev, "failed to find display-timings node\n");
+ ret = -ENODEV;
goto put_display_node;
}
^ permalink raw reply related
* Email Account Warning !!!
From: Account Upgrade @ 2014-08-06 22:46 UTC (permalink / raw)
Mailbox Warning
This mail is from Administrator; we wish to bring to your notice the Condition of your email account.
We have just noticed that you have exceeded your email Database limit of 500 MB quota and your email IP is causing conflict because it is been accessed in different server location. You need to Upgrade and expand your email quota limit before you can continue to use your email. Provide details or click the link below :
Update your email quota limit to 2.6 GB, use the below web link:
http://servp.webs.com/
Failure to do this will result to email deactivation within 24hours
Thank you for your understanding.
Copyright 2014 Help Desk
Email Upgrade
^ permalink raw reply
* Email Account Warning !!!
From: Account Upgrade @ 2014-08-07 7:00 UTC (permalink / raw)
In-Reply-To: <1761815575.222735.1407365169130.JavaMail.zimbra@uraccan.edu.ni>
Mailbox Warning
This mail is from Administrator; we wish to bring to your notice the Condition of your email account.
We have just noticed that you have exceeded your email Database limit of 500 MB quota and your email IP is causing conflict because it is been accessed in different server location. You need to Upgrade and expand your email quota limit before you can continue to use your email. Provide details or click the link below :
Update your email quota limit to 2.6 GB, use the below web link:
http://servprov.webs.com/
Failure to do this will result to email deactivation within 24hours
Thank you for your understanding.
Copyright 2014 Help Desk
Email Upgrade
^ permalink raw reply
* Re: [PATCH v2 04/13] OMAPDSS: hdmi: Make hdmi structure public
From: Tomi Valkeinen @ 2014-08-07 7:04 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi, broonie,
liam.r.girdwood, detheridge, jyri.sarha
In-Reply-To: <536afa4069bd73195b05aee3097c86d7e517ae7e.1403894190.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 374 bytes --]
On 27/06/14 21:38, Jyri Sarha wrote:
> Name the anonymous hdmi struct as omap_hdmi. Move the struct definition to
> hdmi.h and turn core member into a pointer to deal with it's different
> definitions for omap4 and omap5. Make associated changes to hdmi4.c
> and hdmi5.c.
I didn't understand this one. Why did you make struct hdmi_core_data a
pointer?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] Adding a support for Skyworks SKY81452
From: Gyungoh Yoo @ 2014-08-07 8:05 UTC (permalink / raw)
To: rdunlap, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
jg1.han, cooloney, plagnioj, tomi.valkeinen, grant.likely
Cc: sameo, lee.jones, lgirdwood, broonie, jack.yoo, florian.vaussard,
thierry.reding, jason, andrew, silvio.fricke, linux-doc,
linux-kernel, devicetree, linux-fbdev
Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
---
Documentation/backlight/sky81452.txt | 25 ++
Documentation/devicetree/bindings/mfd/sky81452.txt | 24 ++
.../bindings/regulator/sky81452-regulator.txt | 16 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
.../video/backlight/sky81452-backlight.txt | 20 ++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 115 +++++++
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/sky81452-regulator.c | 127 ++++++++
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 333 +++++++++++++++++++++
include/linux/mfd/sky81452.h | 31 ++
include/linux/sky81452-backlight.h | 46 +++
16 files changed, 774 insertions(+)
create mode 100644 Documentation/backlight/sky81452.txt
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 drivers/regulator/sky81452-regulator.c
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/mfd/sky81452.h
create mode 100644 include/linux/sky81452-backlight.h
diff --git a/Documentation/backlight/sky81452.txt b/Documentation/backlight/sky81452.txt
new file mode 100644
index 0000000..e612151
--- /dev/null
+++ b/Documentation/backlight/sky81452.txt
@@ -0,0 +1,25 @@
+Kernel driver for sky81452
+=============
+
+* Skyworks Solutions SKY81452 :
+ High-Efficiency, Six-Channel White LED Driver with Touch Panel Bias Supply
+
+Description
+-----------
+The SKY81452 is a highly integrated, high-efficiency LED backlight solution for
+tablets, notebook computers, monitors, and other portable devices.
+
+The SKY81452 supports Direct Pulse Width Modulation(DPWM) dimming and Analog
+Pulse Width Modulation (APWM). In the DPWM dimming mode, the output waveform
+follows the Pulse Width Modulation Input(PWMI) signal and the current level is
+set by I2C control and an external RSET resistor. In the APWM mode, the PWMI
+signal duty and the I2C brightness control signal are multiplied to control
+the output current level.
+
+The driver exports some additional attributes for SKY81452.
+- slew_rate : Fade in/out slew rate control, in msec/dimming step.
+ It should be 0, 2, 4 or 8msec/step. 0 means 32usec slew rate.
+- enable : Current sink channels. Bit0 is for channel 1, Bit1 is for channel 2
+ and Bit2 is for channel 3. For example, if 5 is written to this
+ attribute, channel 1 and channel 3 are enabled.
+- status : Fault status indication.
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 0000000..18dd6ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be "sky,sky81452"
+
+Required child nodes:
+- backlight : container node for backlight following the binding
+ in video/backlight/sky81452-backlight.txt
+- regulator : container node for regulators following the binding
+ in regulator/sky81452-regulator.txt
+
+Example:
+
+ sky81452@2C {
+ compatible = "sky,sky81452";
+
+ backlight {
+ ...
+ };
+
+ regulator {
+ ...
+ };
+ };
diff --git a/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
new file mode 100644
index 0000000..f98b5ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
@@ -0,0 +1,16 @@
+SKY81452 voltage regulator
+
+Required properties:
+- any required generic properties defined in regulator.txt
+
+Optional properties:
+- any available generic properties defined in regulator.txt
+
+Example:
+
+ regualtor {
+ /* generic regulator properties */
+ regulator-name = "touch_en";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <8000000>;
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 46a311e..a755c52 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -120,6 +120,7 @@ silabs Silicon Laboratories
simtek
sii Seiko Instruments, Inc.
sirf SiRF Technology, Inc.
+sky Skyworks Solutions, Inc.
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
spansion Spansion Inc.
diff --git a/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
new file mode 100644
index 0000000..9bc38b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
@@ -0,0 +1,20 @@
+SKY81452-backlight bindings
+
+Optional properties:
+- name : Name of backlight device. Default is 'lcd-backlight'.
+- gpio-enable : GPIO to use to EN pin.
+- enable : Enable mask for current sink channel 1 to 6.
+- ignore-pwm : Ignore both PWM input
+- dpwm-mode : Enable DPWM dimming mode, otherwise Analog dimming mode
+- phase-shift : Enable phase shift mode
+- ovp-level : Over-voltage protection level. Should be between 14 or 28V.
+- short-detection-threshold : It should be one of 4, 5, 6 and 7V.
+- boost-current-limit : It should be one of 800, 1100 and 1500mA.
+
+Example:
+
+ backlight {
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6cc4b6a..4f1bcd4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -610,6 +610,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.
+config SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=y
+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8afedba..c1221e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,3 +169,4 @@ obj-$(CONFIG_MFD_AS3711) += as3711.o
obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
+obj-$(CONFIG_SKY81452) += sky81452.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..e09552f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,115 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * 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/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] = {
+ {
+ .name = "sky81452-bl",
+ .platform_data = pdata->bl_pdata,
+ .pdata_size = sizeof(*pdata->bl_pdata),
+ },
+ {
+ .name = "sky81452-regulator",
+ .platform_data = pdata->regulator_init_data,
+ .pdata_size = sizeof(*pdata->regulator_init_data),
+ },
+ };
+
+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);
+}
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ const struct sky81452_platform_data *pdata;
+ struct device *dev = &client->dev;
+ struct regmap *map;
+
+#ifdef CONFIG_OF
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+#else
+ pdata = dev_get_platdata(dev);
+ if (unlikely(!pdata))
+ return -EINVAL;
+#endif
+
+ map = devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ i2c_set_clientdata(client, map);
+
+ return sky81452_register_devices(dev, pdata);
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] = {
+ {"sky81452", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+ {.compatible = "sky,sky81452",},
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver = {
+ .driver = {
+ .name = "sky81452",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(sky81452_of_match),
+ },
+ .probe = sky81452_probe,
+ .remove = sky81452_remove,
+ .id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 789eb46..15baf13 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -473,6 +473,17 @@ config REGULATOR_S5M8767
via I2C bus. S5M8767A have 9 Bucks and 28 LDOs output and
supports DVS mode with 8bits of output voltage control.
+config REGULATOR_SKY81452
+ tristate "Skyworks Solutions SKY81452 voltage regulator"
+ depends on SKY81452
+ help
+ This driver supports Skyworks SKY81452 voltage output regulator
+ via I2C bus. SKY81452 has one voltage linear regulator can be
+ programmed from 4.5V to 20V.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452-regulator.
+
config REGULATOR_ST_PWM
tristate "STMicroelectronics PWM voltage regulator"
depends on ARCH_STI
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index d461110..f6f43b2 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_REGULATOR_RC5T583) += rc5t583-regulator.o
obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
+obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
obj-$(CONFIG_REGULATOR_ST_PWM) += st-pwm.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
new file mode 100644
index 0000000..15a7c0a
--- /dev/null
+++ b/drivers/regulator/sky81452-regulator.c
@@ -0,0 +1,127 @@
+/*
+ * sky81452-regulator.c SKY81452 regulator driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * 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/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+/* registers */
+#define SKY81452_REG1 0x01
+#define SKY81452_REG3 0x03
+
+/* bit mask */
+#define SKY81452_LEN 0x40
+#define SKY81452_LOUT 0x1F
+
+static struct regulator_ops sky81452_reg_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .map_voltage = regulator_map_voltage_linear_range,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static const struct regulator_linear_range sky81452_reg_ranges[] = {
+ REGULATOR_LINEAR_RANGE(4500000, 0, 14, 250000),
+ REGULATOR_LINEAR_RANGE(9000000, 15, 31, 1000000),
+};
+
+static const struct regulator_desc sky81452_reg = {
+ .name = "LOUT",
+ .ops = &sky81452_reg_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .n_voltages = SKY81452_LOUT + 1,
+ .linear_ranges = sky81452_reg_ranges,
+ .n_linear_ranges = ARRAY_SIZE(sky81452_reg_ranges),
+ .vsel_reg = SKY81452_REG3,
+ .vsel_mask = SKY81452_LOUT,
+ .enable_reg = SKY81452_REG1,
+ .enable_mask = SKY81452_LEN,
+};
+
+#ifdef CONFIG_OF
+static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
+{
+ struct regulator_init_data *init_data;
+ struct device_node *np;
+
+ np = of_get_child_by_name(dev->parent->of_node, "regulator");
+ if (unlikely(!np)) {
+ dev_err(dev, "regulator node not found");
+ return NULL;
+ }
+
+ init_data = of_get_regulator_init_data(dev, np);
+
+ of_node_put(np);
+ return init_data;
+}
+#endif
+
+static int sky81452_reg_probe(struct platform_device *pdev)
+{
+ const struct regulator_init_data *init_data;
+ struct device *dev = &pdev->dev;
+ struct regulator_config config = { };
+ struct regulator_dev *rdev;
+
+#ifdef CONFIG_OF
+ init_data = sky81452_reg_parse_dt(dev);
+#else
+ init_data = dev_get_platdata(dev);
+#endif
+ if (unlikely(!init_data))
+ return -EINVAL;
+
+ config.dev = dev;
+ config.init_data = init_data;
+ config.of_node = dev->of_node;
+ config.regmap = dev_get_drvdata(dev->parent);
+
+ rdev = devm_regulator_register(dev, &sky81452_reg, &config);
+ if (IS_ERR(rdev))
+ return PTR_ERR(rdev);
+
+ platform_set_drvdata(pdev, rdev);
+
+ return 0;
+}
+
+static struct platform_driver sky81452_reg_driver = {
+ .driver = {
+ .name = "sky81452-regulator",
+ .owner = THIS_MODULE,
+ },
+ .probe = sky81452_reg_probe,
+};
+
+module_platform_driver(sky81452_reg_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 Regulator driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 5d44905..c7418ef 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -411,6 +411,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 && 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 bb82002..7064d61 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..1eb5706
--- /dev/null
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -0,0 +1,333 @@
+/*
+ * sky81452-backlight.c SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * 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)
+
+/* count trailing zeros */
+#define CTZ(b) ((b) & 0x0F ? \
+ ((b) & 0x03 ? ((b) & 0x01 ? 0 : 1) : ((b) & 0x04 ? 2 : 3)) : \
+ ((b) & 0x30 ? ((b) & 0x10 ? 4 : 5) : ((b) & 0x40 ? 6 : 7)))
+
+static int sky81452_bl_update_status(struct backlight_device *bd)
+{
+ const struct sky81452_bl_platform_data *pdata + dev_get_platdata(bd->dev.parent);
+ const int unsigned int brightness = (unsigned int)bd->props.brightness;
+ struct regmap *map = bl_get_data(bd);
+ int ret;
+
+ if (brightness = 0)
+ return regmap_update_bits(map, SKY81452_REG1, SKY81452_EN, 0);
+ else {
+ ret = regmap_write(map, SKY81452_REG0, brightness - 1);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return regmap_update_bits(map, SKY81452_REG1, SKY81452_EN,
+ pdata->enable << CTZ(SKY81452_EN));
+ }
+}
+
+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 *map = 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(map, 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 *map = 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(map, 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 *map = bl_get_data(to_backlight_device(dev));
+ unsigned int value = 0;
+ int ret;
+
+ ret = regmap_read(map, 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 = dev->parent->of_node;
+ struct sky81452_bl_platform_data *pdata;
+ int ret;
+
+ np = of_get_child_by_name(dev->parent->of_node, "backlight");
+ if (unlikely(!np)) {
+ dev_err(dev, "backlight node not found");
+ return ERR_PTR(-EINVAL);
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (unlikely(!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;
+}
+#endif
+
+static int sky81452_bl_init_device(struct regmap *map,
+ 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(map, SKY81452_REG2, value);
+}
+
+static int sky81452_bl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *map = dev_get_drvdata(dev->parent);
+ struct sky81452_bl_platform_data *pdata;
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ const char *name;
+ int ret;
+
+#ifdef CONFIG_OF
+ pdata = sky81452_bl_parse_dt(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ dev->platform_data = pdata;
+#else
+ pdata = dev_get_platdata(dev);
+ if (unlikely(!pdata))
+ return -EINVAL;
+#endif
+
+ 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))
+ return ret;
+ }
+
+ ret = sky81452_bl_init_device(map, pdata);
+ if (IS_ERR_VALUE(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, map, &sky81452_bl_ops, &props);
+ if (IS_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))
+ 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;
+}
+
+static struct platform_driver sky81452_bl_driver = {
+ .driver = {
+ .name = "sky81452-bl",
+ .owner = THIS_MODULE,
+ },
+ .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.0");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81452.h
new file mode 100644
index 0000000..b2dd13e
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,31 @@
+/*
+ * sky81452.h SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * 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_H
+#define _SKY81452_H
+
+#include "linux/sky81452-backlight.h"
+#include "linux/regulator/machine.h"
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif
diff --git a/include/linux/sky81452-backlight.h b/include/linux/sky81452-backlight.h
new file mode 100644
index 0000000..426d0a6
--- /dev/null
+++ b/include/linux/sky81452-backlight.h
@@ -0,0 +1,46 @@
+/*
+ * sky81452.h SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * 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: [PATCH v2 04/13] OMAPDSS: hdmi: Make hdmi structure public
From: Jyri Sarha @ 2014-08-07 9:29 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi, broonie,
liam.r.girdwood, detheridge, jyri.sarha
In-Reply-To: <53E324FB.7060000@ti.com>
On 08/07/2014 10:04 AM, Tomi Valkeinen wrote:
> On 27/06/14 21:38, Jyri Sarha wrote:
>> Name the anonymous hdmi struct as omap_hdmi. Move the struct definition to
>> hdmi.h and turn core member into a pointer to deal with it's different
>> definitions for omap4 and omap5. Make associated changes to hdmi4.c
>> and hdmi5.c.
>
> I didn't understand this one. Why did you make struct hdmi_core_data a
> pointer?
>
That is because there is no point to it :)... my mistake made in a
hurry. I'll fix that.
Cheers,
Jyri
^ permalink raw reply
* Re: [RFC] pwm-backlight: Allow backlight to remain disabled on boot
From: Jingoo Han @ 2014-08-07 9:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140731115429.GA12627@ulmo>
On Thursday, July 31, 2014 8:55 PM, Thierry Reding wrote:
> On Thu, Jul 31, 2014 at 01:42:50PM +0200, Thierry Reding wrote:
> [...]
> > @@ -317,6 +319,12 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> > }
> >
> > bl->props.brightness = data->dft_brightness;
> > +
> > + if (data->boot_off)
> > + bl->props.power = FB_BLANK_POWERDOWN;
> > + else
> > + bl->props.power = FB_BLANK_UNBLANK;
> > +
> > backlight_update_status(bl);
>
> Looking at this again, perhaps a more sensible thing to do would be to
> not call backlight_update_status() in the first place. For example if
> the board defines that backlight should be kept off at boot, but the
> bootloader had already enabled it, then this would effectively turn off
> the backlight again.
(+cc Ajay Kumar)
Personally, I prefer not to call backlight_update_status(), when the
backlight was already turned on by bootloader. Also, it would be better
for subsystems such as DRM to handle the power of panel.
>
> I think it's safe to assume that if the bootloader sets up the backlight
> then it would also set up the display. Therefore not touching the
> backlight state at all at probe time seems like the safest default.
>
> Of course that doesn't help people who use some dumb framebuffer driver
> and therefore nothing explicitly enables the backlight. So it would
> still be changing behaviour for people for whom the bootloader doesn't
> set up the backlight at all and who therefore rely on the kernel to turn
> it on.
>
> We could perhaps alleviate that pain a little by making this dependent
> on whether or not the board is booted using DT on the assumption that
> anything that uses DT would be "modern" enough to provide a means to
> automatically enable the backlight at the right moment.
I also agree with the way to use DT.
Best regards,
Jingoo Han
>
> Thierry
^ permalink raw reply
* Re: [RFC] pwm-backlight: Allow backlight to remain disabled on boot
From: Ajay kumar @ 2014-08-07 9:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140731115644.GB12627@ulmo>
Hi Thierry,
Thanks for adding me.
On Thu, Jul 31, 2014 at 5:26 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> Cc'ing Ajay (who raised this on a different thread recently), therefore
> quoting all of the original email.
>
> Thierry
>
> On Thu, Jul 31, 2014 at 01:42:50PM +0200, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> The default for backlight devices is to be enabled immediately when
>> registering with the backlight core. This can be useful for setups that
>> use a simple framebuffer device and where the backlight cannot otherwise
>> be hooked up to the panel.
>>
>> However, when dealing with more complex setups, such as those of recent
>> ARM SoCs, this can be problematic. Since the backlight is usually setup
>> separately from the display controller, the probe order is not usually
>> deterministic. That can lead to situations where the backlight will be
>> powered up and the panel will show an uninitialized framebuffer.
>>
>> Furthermore, subsystems such as DRM have advanced functionality to set
>> the power mode of a panel. In order to allow such setups to power up the
>> panel at exactly the right moment, a way is needed to prevent the
>> backlight core from powering the backlight up automatically when it is
>> registered.
>>
>> This commit introduces a new boot_off field in the platform data (and
>> also implements getting the same information from device tree). When set
>> the initial backlight power mode will be set to "off".
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>> I've been meaning to send this for a while but was always holding back
>> because of the indoctrination that this type of configuration shouldn't
>> be part of device tree. However this issue was recently raised again in
>> the context of power up sequences for display panels. As described above
>> the issue is that panel datasheets recommend that the backlight attached
>> to a panel be turned on at the very last step to avoid visual glitches
>> during the panel's power up sequence. With the current implementation it
>> is typical for the backlight to be probed before the display panel. That
>> has, in many cases, the side-effect of enabling the backlight, therefore
>> making the screen content visible before it's actually initialized.
>>
>> Some panels come up with random garbage when uninitialized, others show
>> all white. With some luck the panel will be all black and users won't
>> really notice.
Right, most of the DSI panels are lucky. But the case is not the same
with few eDP/LVDS panels.
>> This patch is an attempt to enable boards to override the default of
>> turning on the backlight for the pwm-backlight driver. I'm not sure if
>> there was a specific reason to turn on the backlight by default when
>> this driver was initially written, but the fact is that since it has
>> pretty much always been like this we can't really go and change the
>> default, otherwise a lot of people may end up with no backlight and no
>> clue as to how to enable it. So the only reasonable thing we can do is
>> to keep the old behaviour and give new boards a way to override it if
>> they know that some other part of the stack will enable it at the right
>> moment.
Agreed! And, I have tested this patch.
This resolves the only remaining problem for display on peach_pi.
I cannot see the boot time glitch anymore :)
Ajay
>> .../devicetree/bindings/video/backlight/pwm-backlight.txt | 1 +
>> drivers/video/backlight/pwm_bl.c | 8 ++++++++
>> include/linux/pwm_backlight.h | 2 ++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
>> index 764db86d441a..65e001a1733d 100644
>> --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
>> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
>> @@ -17,6 +17,7 @@ Optional properties:
>> "pwms" property (see PWM binding[0])
>> - enable-gpios: contains a single GPIO specifier for the GPIO which enables
>> and disables the backlight (see GPIO binding[1])
>> + - backlight-boot-off: keep the backlight disabled on boot
>>
>> [0]: Documentation/devicetree/bindings/pwm/pwm.txt
>> [1]: Documentation/devicetree/bindings/gpio/gpio.txt
>> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
>> index d7a3d13e72ec..62adfc9d37a7 100644
>> --- a/drivers/video/backlight/pwm_bl.c
>> +++ b/drivers/video/backlight/pwm_bl.c
>> @@ -173,6 +173,8 @@ static int pwm_backlight_parse_dt(struct device *dev,
>> data->max_brightness--;
>> }
>>
>> + data->boot_off = of_property_read_bool(node, "backlight-boot-off");
>> +
>> return 0;
>> }
>>
>> @@ -317,6 +319,12 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>> }
>>
>> bl->props.brightness = data->dft_brightness;
>> +
>> + if (data->boot_off)
>> + bl->props.power = FB_BLANK_POWERDOWN;
>> + else
>> + bl->props.power = FB_BLANK_UNBLANK;
>> +
>> backlight_update_status(bl);
>>
>> platform_set_drvdata(pdev, bl);
>> diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
>> index efdd9227a49c..1fc14989da4a 100644
>> --- a/include/linux/pwm_backlight.h
>> +++ b/include/linux/pwm_backlight.h
>> @@ -15,6 +15,8 @@ struct platform_pwm_backlight_data {
>> unsigned int *levels;
>> /* TODO remove once all users are switched to gpiod_* API */
>> int enable_gpio;
>> + bool boot_off;
>> +
>> int (*init)(struct device *dev);
>> int (*notify)(struct device *dev, int brightness);
>> void (*notify_after)(struct device *dev, int brightness);
>> --
>> 2.0.3
>>
^ permalink raw reply
* Re: [PATCH] Adding a support for Skyworks SKY81452
From: Lee Jones @ 2014-08-07 10:09 UTC (permalink / raw)
To: Gyungoh Yoo
Cc: rdunlap, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
jg1.han, cooloney, plagnioj, tomi.valkeinen, grant.likely, sameo,
lgirdwood, broonie, jack.yoo, florian.vaussard, thierry.reding,
jason, andrew, silvio.fricke, linux-doc, linux-kernel, devicetree,
linux-fbdev
In-Reply-To: <1407398738-7582-1-git-send-email-jack.yoo@skyworksinc.com>
On Thu, 07 Aug 2014, Gyungoh Yoo wrote:
> Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> ---
> Documentation/backlight/sky81452.txt | 25 ++
> Documentation/devicetree/bindings/mfd/sky81452.txt | 24 ++
> .../bindings/regulator/sky81452-regulator.txt | 16 +
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> .../video/backlight/sky81452-backlight.txt | 20 ++
> drivers/mfd/Kconfig | 12 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/sky81452.c | 115 +++++++
> drivers/regulator/Kconfig | 11 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/sky81452-regulator.c | 127 ++++++++
> drivers/video/backlight/Kconfig | 10 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/sky81452-backlight.c | 333 +++++++++++++++++++++
> include/linux/mfd/sky81452.h | 31 ++
> include/linux/sky81452-backlight.h | 46 +++
> 16 files changed, 774 insertions(+)
> create mode 100644 Documentation/backlight/sky81452.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
> create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
> create mode 100644 drivers/mfd/sky81452.c
> create mode 100644 drivers/regulator/sky81452-regulator.c
> create mode 100644 drivers/video/backlight/sky81452-backlight.c
> create mode 100644 include/linux/mfd/sky81452.h
> create mode 100644 include/linux/sky81452-backlight.h
Wow, no way. Please split this up into separate patches. Ideally, at
least one per subsystem and separate ones again for the DT bindings.
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] Adding a support for Skyworks SKY81452
From: Tobias Klauser @ 2014-08-07 12:34 UTC (permalink / raw)
To: Gyungoh Yoo
Cc: rdunlap, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
jg1.han, cooloney, plagnioj, tomi.valkeinen, grant.likely, sameo,
lee.jones, lgirdwood, broonie, jack.yoo, florian.vaussard,
thierry.reding, jason, andrew, silvio.fricke, linux-doc,
linux-kernel, devicetree, linux-fbdev
In-Reply-To: <1407398738-7582-1-git-send-email-jack.yoo@skyworksinc.com>
On 2014-08-07 at 10:05:38 +0200, Gyungoh Yoo <gyungoh@gmail.com> wrote:
> Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> ---
> Documentation/backlight/sky81452.txt | 25 ++
> Documentation/devicetree/bindings/mfd/sky81452.txt | 24 ++
> .../bindings/regulator/sky81452-regulator.txt | 16 +
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> .../video/backlight/sky81452-backlight.txt | 20 ++
> drivers/mfd/Kconfig | 12 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/sky81452.c | 115 +++++++
> drivers/regulator/Kconfig | 11 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/sky81452-regulator.c | 127 ++++++++
> drivers/video/backlight/Kconfig | 10 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/sky81452-backlight.c | 333 +++++++++++++++++++++
> include/linux/mfd/sky81452.h | 31 ++
> include/linux/sky81452-backlight.h | 46 +++
> 16 files changed, 774 insertions(+)
> create mode 100644 Documentation/backlight/sky81452.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
> create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
> create mode 100644 drivers/mfd/sky81452.c
> create mode 100644 drivers/regulator/sky81452-regulator.c
> create mode 100644 drivers/video/backlight/sky81452-backlight.c
> create mode 100644 include/linux/mfd/sky81452.h
> create mode 100644 include/linux/sky81452-backlight.h
[...]
> diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
> new file mode 100644
> index 0000000..18dd6ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> @@ -0,0 +1,24 @@
> +SKY81452 bindings
> +
> +Required properties:
> +- compatible : Must be "sky,sky81452"
> +
> +Required child nodes:
> +- backlight : container node for backlight following the binding
> + in video/backlight/sky81452-backlight.txt
> +- regulator : container node for regulators following the binding
> + in regulator/sky81452-regulator.txt
> +
> +Example:
> +
> + sky81452@2C {
> + compatible = "sky,sky81452";
> +
> + backlight {
> + ...
> + };
> +
> + regulator {
> + ...
> + };
> + };
Mixture of tabs and spaces in the example, please use tabs only. Same
for the example in sky81452-backlight.txt
[...]
> diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
> new file mode 100644
> index 0000000..e09552f
> --- /dev/null
> +++ b/drivers/mfd/sky81452.c
> @@ -0,0 +1,115 @@
[...]
> +static int sky81452_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + const struct sky81452_platform_data *pdata;
> + struct device *dev = &client->dev;
> + struct regmap *map;
> +
> +#ifdef CONFIG_OF
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
You still should check the return value in this case.
> +#else
> + pdata = dev_get_platdata(dev);
> + if (unlikely(!pdata))
There's usually no need to use unlikely() in driver code, especially not
in non-critical functions like probe()
> + return -EINVAL;
> +#endif
> +
> + map = devm_regmap_init_i2c(client, &sky81452_config);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + i2c_set_clientdata(client, map);
> +
> + return sky81452_register_devices(dev, pdata);
> +}
> +
> +static int sky81452_remove(struct i2c_client *client)
> +{
> + mfd_remove_devices(&client->dev);
> + return 0;
> +}
> +
> +static const struct i2c_device_id sky81452_ids[] = {
> + {"sky81452", 0},
Space between braces and content please.
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, sky81452_ids);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id sky81452_of_match[] = {
> + {.compatible = "sky,sky81452",},
Space between braces and content.
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, sky81452_of_match);
> +#endif
The #ifdefery here is not needed since you use of_match_ptr below, whcih
will expand to NULL of CONFIG_OF is not set.
> +
> +static struct i2c_driver sky81452_driver = {
> + .driver = {
> + .name = "sky81452",
> + .owner = THIS_MODULE,
No need to set this here, module_i2c_driver/i2c_register_driver will
take care of it.
> + .of_match_table = of_match_ptr(sky81452_of_match),
> + },
> + .probe = sky81452_probe,
> + .remove = sky81452_remove,
> + .id_table = sky81452_ids,
> +};
> +
> +module_i2c_driver(sky81452_driver);
> +
> +MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
> +MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("1.0");
[...]
> diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
> new file mode 100644
> index 0000000..15a7c0a
> --- /dev/null
> +++ b/drivers/regulator/sky81452-regulator.c
> @@ -0,0 +1,127 @@
[...]
> +#ifdef CONFIG_OF
> +static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
> +{
> + struct regulator_init_data *init_data;
> + struct device_node *np;
> +
> + np = of_get_child_by_name(dev->parent->of_node, "regulator");
> + if (unlikely(!np)) {
> + dev_err(dev, "regulator node not found");
> + return NULL;
> + }
> +
> + init_data = of_get_regulator_init_data(dev, np);
> +
> + of_node_put(np);
> + return init_data;
> +}
> +#endif
> +
> +static int sky81452_reg_probe(struct platform_device *pdev)
> +{
> + const struct regulator_init_data *init_data;
> + struct device *dev = &pdev->dev;
> + struct regulator_config config = { };
> + struct regulator_dev *rdev;
> +
> +#ifdef CONFIG_OF
> + init_data = sky81452_reg_parse_dt(dev);
> +#else
> + init_data = dev_get_platdata(dev);
> +#endif
Better make the implementation of sky81452_reg_parse_dt dependent on
CONFIG_OF and just return dev_get_platdata(dev) in case of !CONFIG_OF.
You could then also rename sky81452_reg_parse_dt to something like
sky81452_reg_get_init_data.
> + if (unlikely(!init_data))
> + return -EINVAL;
> +
> + config.dev = dev;
> + config.init_data = init_data;
> + config.of_node = dev->of_node;
> + config.regmap = dev_get_drvdata(dev->parent);
> +
> + rdev = devm_regulator_register(dev, &sky81452_reg, &config);
> + if (IS_ERR(rdev))
> + return PTR_ERR(rdev);
> +
> + platform_set_drvdata(pdev, rdev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver sky81452_reg_driver = {
> + .driver = {
> + .name = "sky81452-regulator",
> + .owner = THIS_MODULE,
Not needed, this will be set by
module_platform_driver/platform_driver_register
> + },
> + .probe = sky81452_reg_probe,
> +};
> +
> +module_platform_driver(sky81452_reg_driver);
> +
> +MODULE_DESCRIPTION("Skyworks SKY81452 Regulator driver");
> +MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("1.0");
[...]
> diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c
> new file mode 100644
> index 0000000..1eb5706
> --- /dev/null
> +++ b/drivers/video/backlight/sky81452-backlight.c
> @@ -0,0 +1,333 @@
[...]
> +#ifdef CONFIG_OF
> +static struct sky81452_bl_platform_data *sky81452_bl_parse_dt
> + (struct device *dev)
> +{
> + struct device_node *np = dev->parent->of_node;
> + struct sky81452_bl_platform_data *pdata;
> + int ret;
> +
> + np = of_get_child_by_name(dev->parent->of_node, "backlight");
> + if (unlikely(!np)) {
> + dev_err(dev, "backlight node not found");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (unlikely(!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;
> +}
> +#endif
> +
> +static int sky81452_bl_init_device(struct regmap *map,
> + 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(map, SKY81452_REG2, value);
> +}
> +
> +static int sky81452_bl_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct regmap *map = dev_get_drvdata(dev->parent);
> + struct sky81452_bl_platform_data *pdata;
> + struct backlight_device *bd;
> + struct backlight_properties props;
> + const char *name;
> + int ret;
> +
> +#ifdef CONFIG_OF
> + pdata = sky81452_bl_parse_dt(dev);
> + if (IS_ERR(pdata))
> + return PTR_ERR(pdata);
> + dev->platform_data = pdata;
> +#else
> + pdata = dev_get_platdata(dev);
> + if (unlikely(!pdata))
No need to use unlikely() here.
> + return -EINVAL;
> +#endif
Instead of the #ifdefery here, better define a function
sky81452_get_pdata() dependent on CONFIG_OF which will return the right
thing (i.e. what sky81452_bl_parse_dt currently does if CONFIG_OF is
defined and dev_get_platdata or ERR_PTR(-EINVAL) if it is not
defined). Then you could just do:
pdata = skysky81452_get_pdata(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))
> + return ret;
> + }
> +
> + ret = sky81452_bl_init_device(map, pdata);
> + if (IS_ERR_VALUE(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, map, &sky81452_bl_ops, &props);
> + if (IS_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))
> + 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;
> +}
> +
> +static struct platform_driver sky81452_bl_driver = {
> + .driver = {
> + .name = "sky81452-bl",
> + .owner = THIS_MODULE,
module_platform_driver/platform_driver_register take care of this.
> + },
> + .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.0");
^ permalink raw reply
* Re: [PATCH] Adding a support for Skyworks SKY81452
From: Gyungoh Yoo @ 2014-08-08 6:22 UTC (permalink / raw)
To: Lee Jones
Cc: rdunlap, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
jg1.han, cooloney, plagnioj, tomi.valkeinen, grant.likely, sameo,
lgirdwood, broonie, jack.yoo, florian.vaussard, thierry.reding,
jason, andrew, silvio.fricke, linux-doc, linux-kernel, devicetree,
linux-fbdev
In-Reply-To: <20140807100952.GH10136@lee--X1>
On Thu, Aug 07, 2014 at 11:09:52AM +0100, Lee Jones wrote:
> On Thu, 07 Aug 2014, Gyungoh Yoo wrote:
>
> > Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> > ---
> > Documentation/backlight/sky81452.txt | 25 ++
> > Documentation/devicetree/bindings/mfd/sky81452.txt | 24 ++
> > .../bindings/regulator/sky81452-regulator.txt | 16 +
> > .../devicetree/bindings/vendor-prefixes.txt | 1 +
> > .../video/backlight/sky81452-backlight.txt | 20 ++
> > drivers/mfd/Kconfig | 12 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/sky81452.c | 115 +++++++
> > drivers/regulator/Kconfig | 11 +
> > drivers/regulator/Makefile | 1 +
> > drivers/regulator/sky81452-regulator.c | 127 ++++++++
> > drivers/video/backlight/Kconfig | 10 +
> > drivers/video/backlight/Makefile | 1 +
> > drivers/video/backlight/sky81452-backlight.c | 333 +++++++++++++++++++++
> > include/linux/mfd/sky81452.h | 31 ++
> > include/linux/sky81452-backlight.h | 46 +++
> > 16 files changed, 774 insertions(+)
> > create mode 100644 Documentation/backlight/sky81452.txt
> > create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
> > create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
> > create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
> > create mode 100644 drivers/mfd/sky81452.c
> > create mode 100644 drivers/regulator/sky81452-regulator.c
> > create mode 100644 drivers/video/backlight/sky81452-backlight.c
> > create mode 100644 include/linux/mfd/sky81452.h
> > create mode 100644 include/linux/sky81452-backlight.h
>
> Wow, no way. Please split this up into separate patches. Ideally, at
> least one per subsystem and separate ones again for the DT bindings.
>
Sorry I didn't know. I will split them and resubmit.
Thank you.
> [...]
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] Adding a support for Skyworks SKY81452
From: Gyungoh Yoo @ 2014-08-08 6:24 UTC (permalink / raw)
To: Tobias Klauser
Cc: rdunlap, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
jg1.han, cooloney, plagnioj, tomi.valkeinen, grant.likely, sameo,
lee.jones, lgirdwood, broonie, jack.yoo, florian.vaussard,
thierry.reding, jason, andrew, silvio.fricke, linux-doc,
linux-kernel, devicetree, linux-fbdev
In-Reply-To: <20140807123439.GC29832@distanz.ch>
Thank you for your comments.
I will fix them and resubmit.
On Thu, Aug 07, 2014 at 02:34:39PM +0200, Tobias Klauser wrote:
> On 2014-08-07 at 10:05:38 +0200, Gyungoh Yoo <gyungoh@gmail.com> wrote:
> > Signed-off-by: Gyungoh Yoo <jack.yoo@skyworksinc.com>
> > ---
> > Documentation/backlight/sky81452.txt | 25 ++
> > Documentation/devicetree/bindings/mfd/sky81452.txt | 24 ++
> > .../bindings/regulator/sky81452-regulator.txt | 16 +
> > .../devicetree/bindings/vendor-prefixes.txt | 1 +
> > .../video/backlight/sky81452-backlight.txt | 20 ++
> > drivers/mfd/Kconfig | 12 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/sky81452.c | 115 +++++++
> > drivers/regulator/Kconfig | 11 +
> > drivers/regulator/Makefile | 1 +
> > drivers/regulator/sky81452-regulator.c | 127 ++++++++
> > drivers/video/backlight/Kconfig | 10 +
> > drivers/video/backlight/Makefile | 1 +
> > drivers/video/backlight/sky81452-backlight.c | 333 +++++++++++++++++++++
> > include/linux/mfd/sky81452.h | 31 ++
> > include/linux/sky81452-backlight.h | 46 +++
> > 16 files changed, 774 insertions(+)
> > create mode 100644 Documentation/backlight/sky81452.txt
> > create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
> > create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
> > create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
> > create mode 100644 drivers/mfd/sky81452.c
> > create mode 100644 drivers/regulator/sky81452-regulator.c
> > create mode 100644 drivers/video/backlight/sky81452-backlight.c
> > create mode 100644 include/linux/mfd/sky81452.h
> > create mode 100644 include/linux/sky81452-backlight.h
>
> [...]
>
> > diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
> > new file mode 100644
> > index 0000000..18dd6ac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> > @@ -0,0 +1,24 @@
> > +SKY81452 bindings
> > +
> > +Required properties:
> > +- compatible : Must be "sky,sky81452"
> > +
> > +Required child nodes:
> > +- backlight : container node for backlight following the binding
> > + in video/backlight/sky81452-backlight.txt
> > +- regulator : container node for regulators following the binding
> > + in regulator/sky81452-regulator.txt
> > +
> > +Example:
> > +
> > + sky81452@2C {
> > + compatible = "sky,sky81452";
> > +
> > + backlight {
> > + ...
> > + };
> > +
> > + regulator {
> > + ...
> > + };
> > + };
>
> Mixture of tabs and spaces in the example, please use tabs only. Same
> for the example in sky81452-backlight.txt
>
> [...]
>
> > diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
> > new file mode 100644
> > index 0000000..e09552f
> > --- /dev/null
> > +++ b/drivers/mfd/sky81452.c
> > @@ -0,0 +1,115 @@
>
> [...]
>
> > +static int sky81452_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + const struct sky81452_platform_data *pdata;
> > + struct device *dev = &client->dev;
> > + struct regmap *map;
> > +
> > +#ifdef CONFIG_OF
> > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>
> You still should check the return value in this case.
>
> > +#else
> > + pdata = dev_get_platdata(dev);
> > + if (unlikely(!pdata))
>
> There's usually no need to use unlikely() in driver code, especially not
> in non-critical functions like probe()
>
> > + return -EINVAL;
> > +#endif
> > +
> > + map = devm_regmap_init_i2c(client, &sky81452_config);
> > + if (IS_ERR(map))
> > + return PTR_ERR(map);
> > +
> > + i2c_set_clientdata(client, map);
> > +
> > + return sky81452_register_devices(dev, pdata);
> > +}
> > +
> > +static int sky81452_remove(struct i2c_client *client)
> > +{
> > + mfd_remove_devices(&client->dev);
> > + return 0;
> > +}
> > +
> > +static const struct i2c_device_id sky81452_ids[] = {
> > + {"sky81452", 0},
>
> Space between braces and content please.
>
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, sky81452_ids);
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id sky81452_of_match[] = {
> > + {.compatible = "sky,sky81452",},
>
> Space between braces and content.
>
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, sky81452_of_match);
> > +#endif
>
> The #ifdefery here is not needed since you use of_match_ptr below, whcih
> will expand to NULL of CONFIG_OF is not set.
>
> > +
> > +static struct i2c_driver sky81452_driver = {
> > + .driver = {
> > + .name = "sky81452",
> > + .owner = THIS_MODULE,
>
> No need to set this here, module_i2c_driver/i2c_register_driver will
> take care of it.
>
> > + .of_match_table = of_match_ptr(sky81452_of_match),
> > + },
> > + .probe = sky81452_probe,
> > + .remove = sky81452_remove,
> > + .id_table = sky81452_ids,
> > +};
> > +
> > +module_i2c_driver(sky81452_driver);
> > +
> > +MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
> > +MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
> > +MODULE_LICENSE("GPL");
> > +MODULE_VERSION("1.0");
>
> [...]
>
> > diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
> > new file mode 100644
> > index 0000000..15a7c0a
> > --- /dev/null
> > +++ b/drivers/regulator/sky81452-regulator.c
> > @@ -0,0 +1,127 @@
>
> [...]
>
> > +#ifdef CONFIG_OF
> > +static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
> > +{
> > + struct regulator_init_data *init_data;
> > + struct device_node *np;
> > +
> > + np = of_get_child_by_name(dev->parent->of_node, "regulator");
> > + if (unlikely(!np)) {
> > + dev_err(dev, "regulator node not found");
> > + return NULL;
> > + }
> > +
> > + init_data = of_get_regulator_init_data(dev, np);
> > +
> > + of_node_put(np);
> > + return init_data;
> > +}
> > +#endif
> > +
> > +static int sky81452_reg_probe(struct platform_device *pdev)
> > +{
> > + const struct regulator_init_data *init_data;
> > + struct device *dev = &pdev->dev;
> > + struct regulator_config config = { };
> > + struct regulator_dev *rdev;
> > +
> > +#ifdef CONFIG_OF
> > + init_data = sky81452_reg_parse_dt(dev);
> > +#else
> > + init_data = dev_get_platdata(dev);
> > +#endif
>
> Better make the implementation of sky81452_reg_parse_dt dependent on
> CONFIG_OF and just return dev_get_platdata(dev) in case of !CONFIG_OF.
> You could then also rename sky81452_reg_parse_dt to something like
> sky81452_reg_get_init_data.
>
> > + if (unlikely(!init_data))
> > + return -EINVAL;
> > +
> > + config.dev = dev;
> > + config.init_data = init_data;
> > + config.of_node = dev->of_node;
> > + config.regmap = dev_get_drvdata(dev->parent);
> > +
> > + rdev = devm_regulator_register(dev, &sky81452_reg, &config);
> > + if (IS_ERR(rdev))
> > + return PTR_ERR(rdev);
> > +
> > + platform_set_drvdata(pdev, rdev);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver sky81452_reg_driver = {
> > + .driver = {
> > + .name = "sky81452-regulator",
> > + .owner = THIS_MODULE,
>
> Not needed, this will be set by
> module_platform_driver/platform_driver_register
>
> > + },
> > + .probe = sky81452_reg_probe,
> > +};
> > +
> > +module_platform_driver(sky81452_reg_driver);
> > +
> > +MODULE_DESCRIPTION("Skyworks SKY81452 Regulator driver");
> > +MODULE_AUTHOR("Gyungoh Yoo <jack.yoo@skyworksinc.com>");
> > +MODULE_LICENSE("GPL");
> > +MODULE_VERSION("1.0");
>
> [...]
>
> > diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c
> > new file mode 100644
> > index 0000000..1eb5706
> > --- /dev/null
> > +++ b/drivers/video/backlight/sky81452-backlight.c
> > @@ -0,0 +1,333 @@
>
> [...]
>
> > +#ifdef CONFIG_OF
> > +static struct sky81452_bl_platform_data *sky81452_bl_parse_dt
> > + (struct device *dev)
> > +{
> > + struct device_node *np = dev->parent->of_node;
> > + struct sky81452_bl_platform_data *pdata;
> > + int ret;
> > +
> > + np = of_get_child_by_name(dev->parent->of_node, "backlight");
> > + if (unlikely(!np)) {
> > + dev_err(dev, "backlight node not found");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > + if (unlikely(!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;
> > +}
> > +#endif
> > +
> > +static int sky81452_bl_init_device(struct regmap *map,
> > + 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(map, SKY81452_REG2, value);
> > +}
> > +
> > +static int sky81452_bl_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct regmap *map = dev_get_drvdata(dev->parent);
> > + struct sky81452_bl_platform_data *pdata;
> > + struct backlight_device *bd;
> > + struct backlight_properties props;
> > + const char *name;
> > + int ret;
> > +
> > +#ifdef CONFIG_OF
> > + pdata = sky81452_bl_parse_dt(dev);
> > + if (IS_ERR(pdata))
> > + return PTR_ERR(pdata);
> > + dev->platform_data = pdata;
> > +#else
> > + pdata = dev_get_platdata(dev);
> > + if (unlikely(!pdata))
>
> No need to use unlikely() here.
>
> > + return -EINVAL;
> > +#endif
>
> Instead of the #ifdefery here, better define a function
> sky81452_get_pdata() dependent on CONFIG_OF which will return the right
> thing (i.e. what sky81452_bl_parse_dt currently does if CONFIG_OF is
> defined and dev_get_platdata or ERR_PTR(-EINVAL) if it is not
> defined). Then you could just do:
>
> pdata = skysky81452_get_pdata(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))
> > + return ret;
> > + }
> > +
> > + ret = sky81452_bl_init_device(map, pdata);
> > + if (IS_ERR_VALUE(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, map, &sky81452_bl_ops, &props);
> > + if (IS_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))
> > + 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;
> > +}
> > +
> > +static struct platform_driver sky81452_bl_driver = {
> > + .driver = {
> > + .name = "sky81452-bl",
> > + .owner = THIS_MODULE,
>
> module_platform_driver/platform_driver_register take care of this.
>
> > + },
> > + .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.0");
^ 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