Allow more revisions in epson s1d13... driver

Allow more revisions in epson s1d13... driver

[Kristoffer Ericson]

Andrew,

I've been trying to get someones attention to this patch. Currently the
file in question is without maintainer. 
Ive however talked to Author Thiabaut Varene and adapted according
to his suggestions (he's cc'ed in this mail).

Is there anyone special that I should email?

/Kristoffer

Patch included below.

-----------------

The Epson s1d13xxx hardware is common in many handhelds, but our
driver is currently locked to a single chip revision. This patch
adds an array of known to work revisions (which can be extended).

Signed-off-by: Kristoffer Ericson <Kristoffer.Ericson@gmail.com>

diff --git a/drivers/video/s1d13xxxfb.c b/drivers/video/s1d13xxxfb.c
index b829dc7..c674949 100644
--- a/drivers/video/s1d13xxxfb.c
+++ b/drivers/video/s1d13xxxfb.c
@@ -50,6 +50,11 @@
 #define dbg(fmt, args...) do { } while (0)
 #endif
 
+static const int s1d13xxxfb_revisions[] = {
+	S1D13506_CHIP_REV,/* Rev.4 on HP Jornada 7xx S1D13506 */
+	S1D13806_CHIP_REV /* Rev.7 on .. */
+};
+
 /*
  * Here we define the default struct fb_fix_screeninfo
  */
@@ -538,6 +543,7 @@ s1d13xxxfb_probe(struct platform_device *pdev)
 	struct fb_info *info;
 	struct s1d13xxxfb_pdata *pdata = NULL;
 	int ret = 0;
+	int i;
 	u8 revision;
 
 	dbg("probe called: device is %p\n", pdev);
@@ -607,10 +613,19 @@ s1d13xxxfb_probe(struct platform_device *pdev)
 		goto bail;
 	}
 
-	revision = s1d13xxxfb_readreg(default_par, S1DREG_REV_CODE);
-	if ((revision >> 2) != S1D_CHIP_REV) {
-		printk(KERN_INFO PFX "chip not found: %i\n", (revision >> 2));
-		ret = -ENODEV;
+	revision = s1d13xxxfb_readreg(default_par, S1DREG_REV_CODE) >> 2;
+
+	ret = -ENODEV;
+
+	for (i = 0; i < ARRAY_SIZE(s1d13xxxfb_revisions); i++) {
+		if (revision == s1d13xxxfb_revisions[i])
+			ret = 0;
+	}
+
+	if (!ret)
+		printk(KERN_INFO PFX "chip revision %i\n", (revision));
+	else {
+		printk(KERN_INFO PFX "unknown chip revision %i\n", (revision));
 		goto bail;
 	}
 
diff --git a/include/video/s1d13xxxfb.h b/include/video/s1d13xxxfb.h
index c99d261..fe41b84 100644
--- a/include/video/s1d13xxxfb.h
+++ b/include/video/s1d13xxxfb.h
@@ -14,7 +14,8 @@
 #define	S1D13XXXFB_H
 
 #define S1D_PALETTE_SIZE		256
-#define S1D_CHIP_REV			7	/* expected chip revision number for s1d13806 */
+#define S1D13506_CHIP_REV		4	/* expected chip revision number for s1d13506 */
+#define S1D13806_CHIP_REV		7	/* expected chip revision number for s1d13806 */
 #define S1D_FBID			"S1D13806"
 #define S1D_DEVICENAME			"s1d13806fb"
 




-- 
Kristoffer Ericson <kristoffer.ericson@gmail.com>

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: Allow more revisions in epson s1d13... driver

[Thibaut VARENE]

On Mon, Sep 29, 2008 at 3:47 PM, Kristoffer Ericson
<kristoffer.ericson@gmail.com> wrote:
> Andrew,
>
> I've been trying to get someones attention to this patch. Currently the
> file in question is without maintainer.
> Ive however talked to Author Thiabaut Varene and adapted according
> to his suggestions (he's cc'ed in this mail).

This patch is a Ack from my side, save maybe for a tiny nitpick on the
fact that I think s1d13xxxfb_revisions should be __devinitdata, but
doing so seems to trigger compile issues. Haven't had time yet to dig
that out.

> Signed-off-by: Kristoffer Ericson <Kristoffer.Ericson@gmail.com>

Acked-by: Thibaut Varène <varenet@parisc-linux.org>

> diff --git a/drivers/video/s1d13xxxfb.c  b/drivers/video/s1d13xxxfb.c
> index b829dc7..c674949 100644
> --- a/drivers/video/s1d13xxxfb.c
> +++ b/drivers/video/s1d13xxxfb.c
> @@ -50,6 +50,11 @@
>  #define dbg(fmt, args...) do { } while (0)
>  #endif
>
> +static const int s1d13xxxfb_revisions[] = {
> +       S1D13506_CHIP_REV,/* Rev.4 on HP Jornada 7xx S1D13506 */
> +       S1D13806_CHIP_REV /* Rev.7 on .. */
> +};
> +
>  /*
>  * Here we define the default struct fb_fix_screeninfo
>  */
> @@ -538,6 +543,7 @@ s1d13xxxfb_probe(struct platform_device *pdev)
>        struct fb_info *info;
>        struct s1d13xxxfb_pdata *pdata = NULL;
>        int ret = 0;
> +       int i;
>        u8 revision;
>
>        dbg("probe called: device is %p\n", pdev);
> @@ -607,10 +613,19 @@ s1d13xxxfb_probe(struct platform_device *pdev)
>                goto bail;
>        }
>
> -       revision = s1d13xxxfb_readreg(default_par, S1DREG_REV_CODE);
> -       if ((revision >> 2) != S1D_CHIP_REV) {
> -               printk(KERN_INFO PFX "chip not found: %i\n", (revision >> 2));
> -               ret = -ENODEV;
> +       revision = s1d13xxxfb_readreg(default_par, S1DREG_REV_CODE) >> 2;
> +
> +       ret = -ENODEV;
> +
> +       for (i = 0; i < ARRAY_SIZE(s1d13xxxfb_revisions); i++) {
> +               if (revision == s1d13xxxfb_revisions[i])
> +                       ret = 0;
> +       }
> +
> +       if (!ret)
> +               printk(KERN_INFO PFX "chip revision %i\n", (revision));
> +       else {
> +               printk(KERN_INFO PFX "unknown chip revision %i\n", (revision));
>                goto bail;
>        }
>
> diff --git a/include/video/s1d13xxxfb.h b/include/video/s1d13xxxfb.h
> index c99d261..fe41b84 100644
> --- a/include/video/s1d13xxxfb.h
> +++ b/include/video/s1d13xxxfb.h
> @@ -14,7 +14,8 @@
>  #define        S1D13XXXFB_H
>
>  #define S1D_PALETTE_SIZE               256
> -#define S1D_CHIP_REV                   7       /* expected chip revision number for s1d13806 */
> +#define S1D13506_CHIP_REV              4       /* expected chip revision number for s1d13506 */
> +#define S1D13806_CHIP_REV              7       /* expected chip revision number for s1d13806 */
>  #define S1D_FBID                       "S1D13806"
>  #define S1D_DEVICENAME                 "s1d13806fb"
>
>
>
>
>
> --
> Kristoffer Ericson <kristoffer.ericson@gmail.com>
>



-- 
Thibaut VARENE
http://www.parisc-linux.org/~varenet/
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Re: Allow more revisions in epson s1d13... driver

[Geert Uytterhoeven]

On Mon, 29 Sep 2008, Thibaut VARENE wrote:
> On Mon, Sep 29, 2008 at 3:47 PM, Kristoffer Ericson
> <kristoffer.ericson@gmail.com> wrote:
> > I've been trying to get someones attention to this patch. Currently the
> > file in question is without maintainer.
> > Ive however talked to Author Thiabaut Varene and adapted according
> > to his suggestions (he's cc'ed in this mail).
> 
> This patch is a Ack from my side, save maybe for a tiny nitpick on the
> fact that I think s1d13xxxfb_revisions should be __devinitdata, but
> doing so seems to trigger compile issues. Haven't had time yet to dig
> that out.

> > +static const int s1d13xxxfb_revisions[] = {
> > +       S1D13506_CHIP_REV,/* Rev.4 on HP Jornada 7xx S1D13506 */
> > +       S1D13806_CHIP_REV /* Rev.7 on .. */
> > +};

Does the compile issue go away if you use __devinitconst instead of
__devinitdata?

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

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: Allow more revisions in epson s1d13... driver

[Kristoffer Ericson]

On Mon, 29 Sep 2008 17:18:26 +0200 (CEST)
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On Mon, 29 Sep 2008, Thibaut VARENE wrote:
> > On Mon, Sep 29, 2008 at 3:47 PM, Kristoffer Ericson
> > <kristoffer.ericson@gmail.com> wrote:
> > > I've been trying to get someones attention to this patch. Currently the
> > > file in question is without maintainer.
> > > Ive however talked to Author Thiabaut Varene and adapted according
> > > to his suggestions (he's cc'ed in this mail).
> > 
> > This patch is a Ack from my side, save maybe for a tiny nitpick on the
> > fact that I think s1d13xxxfb_revisions should be __devinitdata, but
> > doing so seems to trigger compile issues. Haven't had time yet to dig
> > that out.
> 
> > > +static const int s1d13xxxfb_revisions[] = {
> > > +       S1D13506_CHIP_REV,/* Rev.4 on HP Jornada 7xx S1D13506 */
> > > +       S1D13806_CHIP_REV /* Rev.7 on .. */
> > > +};
> 
> Does the compile issue go away if you use __devinitconst instead of
> __devinitdata?
> 

Yes, thanks for clearing that up. Going to resend with fixed also adding the ack.


> 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


-- 
Kristoffer Ericson <kristoffer.ericson@gmail.com>

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/