Hello Russell,
> > Doh, right. Do you use a different checkpatch? I mean it doesn't react on
> > that for me.
>
> No, all of the comments I've given were spotted by merely reading the patch.
May I ask how you spot tab vs. 8 space problems? Do you apply the patch
and check in an editor?
Best
I made a mistake on the latest patch I sent,
if (IS_ERR(bllcd->lcd_device)) {
+ ret = PTR_ERR(bllcd->bl_device);
+ backlight_device_unregister(bllcd->bl_device);
+ printk(KERN_ERR "lcd :failed to register device\n");
+ kfree(bllcd);
+
On Wed, Feb 06, 2008 at 08:43:40PM +0100, Kristoffer Ericson wrote:
> ATTEMPT #4.
> ---
> Fixed issues as suggested by Russell below.
Great, I'm happy now.
> Doh, right. Do you use a different checkpatch? I mean it doesn't react on
> that for me.
No, all of the comments I've given were
ATTEMPT #4.
---
Fixed issues as suggested by Russell below.
diff --git a/drivers/video/backlight/jornada720_bllcd.c
b/drivers/video/backlight/jornada720_bllcd.c
new file mode 100644
index 000..f3c3f26
--- /dev/null
+++ b/drivers/video/backlight/jornada720_bllcd.c
@@ -0,0 +1,290 @@
On Wed, Feb 06, 2008 at 08:21:28PM +0100, Kristoffer Ericson wrote:
> Oh, and thanks for all the feedback people! (and for doing it nicely)
I'm afraid you missed one - and there's a better fix for one of the other
points I made... 8)
> +static int jornada_bl_probe(struct platform_device *pdev)
>
Attempt #3 :
Changes :
-
* removed JORN.. prefixes except from functions.
* removed file location in head
* fixed (hope) indent issues
* do not exceed 80 chars per line
* added extra checks if driver allocation fails
* return -1 instead of (MAX + 1)
...and probably alot more.
Patch:
*
On Wed, 2008-02-06 at 17:33 +0100, Kristoffer Ericson wrote:
> Oki, here is attempt #2 (btw, new mail or just keep thread?)
Apart from the issues Russell mentioned,
--- /dev/null
+++ b/drivers/video/backlight/jornada720_bllcd.c
@@ -0,0 +1,286 @@
+/*
+ * drivers/video/backlight/jornada720_bllcd.c
On Wed, Feb 06, 2008 at 05:33:41PM +0100, Kristoffer Ericson wrote:
>
> Oki, here is attempt #2 (btw, new mail or just keep thread?)
keep thread to keep comments coming.
But always when posting the updated patch include the full
changelog. Document whats done since last posting
below your s-o-b
Kristoffer Ericson wrote:
> Oki, here is attempt #2 (btw, new mail or just keep thread?)
>
> Tested to build and checkpatch.
>
> diff --git a/drivers/video/backlight/jornada720_bllcd.c
> b/drivers/video/backlight/jornada720_bllcd.c
> +static int jornada_bl_update_status(struct backlight_device
On Wed, Feb 06, 2008 at 05:33:41PM +0100, Kristoffer Ericson wrote:
> Oki, here is attempt #2 (btw, new mail or just keep thread?)
Probably better to keep the thread. More comments 8)
> +struct jornada_bllcd_device {
> + struct backlight_device *jorn_backlight_device;
> + struct
Oki, here is attempt #2 (btw, new mail or just keep thread?)
Tested to build and checkpatch.
diff --git a/drivers/video/backlight/jornada720_bllcd.c
b/drivers/video/backlight/jornada720_bllcd.c
new file mode 100644
index 000..e911a5e
--- /dev/null
+++
On Wed, 06 Feb 2008 13:34:12 +0100
Roel Kluin <[EMAIL PROTECTED]> wrote:
> Kristoffer Ericson wrote:
> > Greetings,
> >
> > Richard I've cleaned up the driver by checking with checkpatch.pl as
> > Russell suggested. I would appreciate it if you could
> > look through the patch again and give
Roel Kluin wrote:
> Kristoffer Ericson wrote:
>> Greetings,
>>
>> Richard I've cleaned up the driver by checking with checkpatch.pl as Russell
>> suggested. I would appreciate it if you could
>> look through the patch again and give comments since the patch looks
>> somewhat different.
>>
>> I
Kristoffer Ericson wrote:
> Greetings,
>
> Richard I've cleaned up the driver by checking with checkpatch.pl as Russell
> suggested. I would appreciate it if you could
> look through the patch again and give comments since the patch looks somewhat
> different.
>
> I can add patch for
Roel Kluin wrote:
Kristoffer Ericson wrote:
Greetings,
Richard I've cleaned up the driver by checking with checkpatch.pl as Russell
suggested. I would appreciate it if you could
look through the patch again and give comments since the patch looks
somewhat different.
I can add patch for
Kristoffer Ericson wrote:
Greetings,
Richard I've cleaned up the driver by checking with checkpatch.pl as Russell
suggested. I would appreciate it if you could
look through the patch again and give comments since the patch looks somewhat
different.
I can add patch for Kconfig/Makefile
On Wed, 06 Feb 2008 13:34:12 +0100
Roel Kluin [EMAIL PROTECTED] wrote:
Kristoffer Ericson wrote:
Greetings,
Richard I've cleaned up the driver by checking with checkpatch.pl as
Russell suggested. I would appreciate it if you could
look through the patch again and give comments since
On Wed, Feb 06, 2008 at 05:33:41PM +0100, Kristoffer Ericson wrote:
Oki, here is attempt #2 (btw, new mail or just keep thread?)
Probably better to keep the thread. More comments 8)
+struct jornada_bllcd_device {
+ struct backlight_device *jorn_backlight_device;
+ struct lcd_device
Oki, here is attempt #2 (btw, new mail or just keep thread?)
Tested to build and checkpatch.
diff --git a/drivers/video/backlight/jornada720_bllcd.c
b/drivers/video/backlight/jornada720_bllcd.c
new file mode 100644
index 000..e911a5e
--- /dev/null
+++
Kristoffer Ericson wrote:
Oki, here is attempt #2 (btw, new mail or just keep thread?)
Tested to build and checkpatch.
diff --git a/drivers/video/backlight/jornada720_bllcd.c
b/drivers/video/backlight/jornada720_bllcd.c
+static int jornada_bl_update_status(struct backlight_device *dev)
On Wed, Feb 06, 2008 at 05:33:41PM +0100, Kristoffer Ericson wrote:
Oki, here is attempt #2 (btw, new mail or just keep thread?)
keep thread to keep comments coming.
But always when posting the updated patch include the full
changelog. Document whats done since last posting
below your s-o-b
On Wed, 2008-02-06 at 17:33 +0100, Kristoffer Ericson wrote:
Oki, here is attempt #2 (btw, new mail or just keep thread?)
Apart from the issues Russell mentioned,
--- /dev/null
+++ b/drivers/video/backlight/jornada720_bllcd.c
@@ -0,0 +1,286 @@
+/*
+ * drivers/video/backlight/jornada720_bllcd.c
Attempt #3 :
Changes :
-
* removed JORN.. prefixes except from functions.
* removed file location in head
* fixed (hope) indent issues
* do not exceed 80 chars per line
* added extra checks if driver allocation fails
* return -1 instead of (MAX + 1)
...and probably alot more.
Patch:
*
On Wed, Feb 06, 2008 at 08:21:28PM +0100, Kristoffer Ericson wrote:
Oh, and thanks for all the feedback people! (and for doing it nicely)
I'm afraid you missed one - and there's a better fix for one of the other
points I made... 8)
+static int jornada_bl_probe(struct platform_device *pdev)
+{
ATTEMPT #4.
---
Fixed issues as suggested by Russell below.
diff --git a/drivers/video/backlight/jornada720_bllcd.c
b/drivers/video/backlight/jornada720_bllcd.c
new file mode 100644
index 000..f3c3f26
--- /dev/null
+++ b/drivers/video/backlight/jornada720_bllcd.c
@@ -0,0 +1,290 @@
On Wed, Feb 06, 2008 at 08:43:40PM +0100, Kristoffer Ericson wrote:
ATTEMPT #4.
---
Fixed issues as suggested by Russell below.
Great, I'm happy now.
Doh, right. Do you use a different checkpatch? I mean it doesn't react on
that for me.
No, all of the comments I've given were
I made a mistake on the latest patch I sent,
if (IS_ERR(bllcd-lcd_device)) {
+ ret = PTR_ERR(bllcd-bl_device);
+ backlight_device_unregister(bllcd-bl_device);
+ printk(KERN_ERR lcd :failed to register device\n);
+ kfree(bllcd);
+
Hello Russell,
Doh, right. Do you use a different checkpatch? I mean it doesn't react on
that for me.
No, all of the comments I've given were spotted by merely reading the patch.
May I ask how you spot tab vs. 8 space problems? Do you apply the patch
and check in an editor?
Best regards
Greetings,
Richard I've cleaned up the driver by checking with checkpatch.pl as Russell
suggested. I would appreciate it if you could
look through the patch again and give comments since the patch looks somewhat
different.
I can add patch for Kconfig/Makefile later on but suspect there will be
Greetings,
Richard I've cleaned up the driver by checking with checkpatch.pl as Russell
suggested. I would appreciate it if you could
look through the patch again and give comments since the patch looks somewhat
different.
I can add patch for Kconfig/Makefile later on but suspect there will be
30 matches
Mail list logo
