updated jitterless kernel patch

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

updated jitterless kernel patch

Gennady Kupava
Hi, list.

As my jitterless proof of concept patch were inculed to all distribution
and seem working well, i decided to write proper version.

Problems fixed:
Rotation, mode switch, suspend issue
Code written in proper way, linked via device-dependent code instead of
direct calls from touchscreen to glamo.

Things left:
Better to place pixclock change to glamo-fb.c, but i do not want to
write two versions of patch for glamo-fb.c and kms.

Patch is not update over previous version, but contains all changes
http://www.bsdmn.com/openmoko/jitterless/jitterless_ts_v3full.patch

Regards,
Gennady.


Reply | Threaded
Open this post in threaded view
|

Re: updated jitterless kernel patch

Riccardo Magliocchetti
Il 31/10/2010 12:30, Gennady Kupava ha scritto:
> Hi, list.
>
> As my jitterless proof of concept patch were inculed to all distribution
> and seem working well, i decided to write proper version.

cool :)

> Problems fixed:
> Rotation, mode switch, suspend issue
> Code written in proper way, linked via device-dependent code instead of
> direct calls from touchscreen to glamo.
>
> Things left:
> Better to place pixclock change to glamo-fb.c, but i do not want to
> write two versions of patch for glamo-fb.c and kms.
>
> Patch is not update over previous version, but contains all changes
> http://www.bsdmn.com/openmoko/jitterless/jitterless_ts_v3full.patch

There are some style issues:

 >diff --git a/drivers/mfd/glamo-core.c b/drivers/mfd/glamo-core.c
 >index 1d51ac1..71d0278 100644
 >--- a/drivers/mfd/glamo-core.c
 >+++ b/drivers/mfd/glamo-core.c
 >@@ -104,6 +104,8 @@ static const struct reg_range reg_range[] = {
 > /* { 0x1b00, 0x900, "3D Unit", 0 }, */
 > };
 >
 >+struct glamo_core *default_glamo = 0;

This should be NULL.

 >@@ -169,6 +184,49 @@ static inline void __reg_set_bit(struct
 >glamo_core *glamo,
 > __reg_write(glamo, reg, tmp);
 > }
 >
 >+void glamo_pixclock_slow (struct glamo_core *glamo) {
 >+
 >+ int x,lastx=0;
 >+ int timeout=1000000;
 >+ int threshold=5;
 >+ int fa;
 >+
 >+ int evcnt=0;

opening brackets should go to new line and you can use a bit more
spacing in variable assignment.

 >+ //int phase=0; //wait for value changing, then for non-changing

This should be removed.

 >+ for (fa=0;fa<timeout;fa++) {
 >+ x = glamo_reg_read(glamo, 0x1100 + >GLAMO_REG_LCD_STATUS1) & 0x3ff;
 >+
 >+
 >+ if (x == lastx) {
 >+ evcnt++;
 >+ if (evcnt == threshold)
 >+ break;
 >+ } else
 >+ evcnt = 0;
 >+
 >+ lastx=x;
 >+ }

lastx = x can be moved inside the else branch, no?

thanks,
riccardo

Reply | Threaded
Open this post in threaded view
|

jitterless patch v4

Gennady Kupava
В Вск, 31/10/2010 в 16:08 +0100, Riccardo Magliocchetti пишет:
> Il 31/10/2010 12:30, Gennady Kupava ha scritto:
> > Hi, list.
> >
> > As my jitterless proof of concept patch were inculed to all distribution
> > and seem working well, i decided to write proper version.
>
> cool :)
Hi, Riccardo,

Thank you much for your review.

I agree and fixed all problems you found. I hope nothing is missed this
time.

New patch:
http://www.bsdmn.com/openmoko/jitterless/jitterless_ts_v4full.patch

Gennady.


Reply | Threaded
Open this post in threaded view
|

Re: updated jitterless kernel patch

NeilBrown
In reply to this post by Riccardo Magliocchetti
On Sun, 31 Oct 2010 16:08:55 +0100
Riccardo Magliocchetti <[hidden email]> wrote:

> > Patch is not update over previous version, but contains all changes
> > http://www.bsdmn.com/openmoko/jitterless/jitterless_ts_v3full.patch
>
> There are some style issues:
>
>  >diff --git a/drivers/mfd/glamo-core.c b/drivers/mfd/glamo-core.c
>  >index 1d51ac1..71d0278 100644
>  >--- a/drivers/mfd/glamo-core.c
>  >+++ b/drivers/mfd/glamo-core.c
>  >@@ -104,6 +104,8 @@ static const struct reg_range reg_range[] = {
>  > /* { 0x1b00, 0x900, "3D Unit", 0 }, */
>  > };
>  >
>  >+struct glamo_core *default_glamo = 0;
>
> This should be NULL.

Actually it should just be:

  static struct glamo_core *default_glamo;

And the 'extern' should be removed from glamo-core.h

All global and static vars are initialised to zero (I'm not sure if 'C'
guarantees this, but Linux Kernel does).  so "= 0" or "= NULL" is not
required and is generally discouraged.

If you './scripts/checkpatch.pl' on the patch it will tell you this and a few
other things.

NeilBrown


Reply | Threaded
Open this post in threaded view
|

Re: updated jitterless kernel patch

Gennady Kupava
Hi, Neil.

В Вск, 31/10/2010 в 13:54 -0400, Neil Brown пишет:

> On Sun, 31 Oct 2010 16:08:55 +0100
> Riccardo Magliocchetti <[hidden email]> wrote:
>
> > > Patch is not update over previous version, but contains all changes
> > > http://www.bsdmn.com/openmoko/jitterless/jitterless_ts_v3full.patch
> >
> > There are some style issues:
> >
> >  >diff --git a/drivers/mfd/glamo-core.c b/drivers/mfd/glamo-core.c
> >  >index 1d51ac1..71d0278 100644
> >  >--- a/drivers/mfd/glamo-core.c
> >  >+++ b/drivers/mfd/glamo-core.c
> >  >@@ -104,6 +104,8 @@ static const struct reg_range reg_range[] = {
> >  > /* { 0x1b00, 0x900, "3D Unit", 0 }, */
> >  > };
> >  >
> >  >+struct glamo_core *default_glamo = 0;
> >
> > This should be NULL.
>
> Actually it should just be:
>
>   static struct glamo_core *default_glamo;
>
> And the 'extern' should be removed from glamo-core.h

Thanks for comment, but I already removed all instances this struct (see
my latest patch in latest mail), it left only because I didn't look into
resulting patch with enought attention.

> All global and static vars are initialised to zero (I'm not sure if 'C'
> guarantees this, but Linux Kernel does).  

C guarantees this of course.

> so "= 0" or "= NULL" is not
> required and is generally discouraged.

Sure.

>
> If you './scripts/checkpatch.pl' on the patch it will tell you this and a few
> other things.

Thanks for hint, next time i'll check my patches with it.

Regards,
Gennady