[PATCH v2 1/2] staging: panel: fix lcd type

[PATCH v2 1/2] staging: panel: fix lcd type

[Sudip Mukherjee]

the lcd type as defined in the Kconfig is not matching in the code.
as a result the rs, rw and en pins were getting interchanged.
Kconfig defines the value of PANEL_LCD to be 1 if we select custom
configuration but in the code LCD_TYPE_CUSTOM is defined as 5.

my hardware is LCD_TYPE_CUSTOM, but the pins were assigned to it
as pins of LCD_TYPE_OLD, and it was not working.
Now values are corrected with referenece to the values defined in
Kconfig and it is working.
checked on JHD204A lcd with LCD_TYPE_CUSTOM configuration.

Cc: <stable@vger.kernel.org> # 2.6.32+
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v2: added stable and the second patch of the series.

 drivers/staging/panel/panel.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index b7ffdfb..4724bef 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -335,11 +335,11 @@ static unsigned char lcd_bits[LCD_PORTS][LCD_BITS][BIT_STATES];
  * LCD types
  */
 #define LCD_TYPE_NONE		0
-#define LCD_TYPE_OLD		1
-#define LCD_TYPE_KS0074		2
-#define LCD_TYPE_HANTRONIX	3
-#define LCD_TYPE_NEXCOM		4
-#define LCD_TYPE_CUSTOM		5
+#define LCD_TYPE_CUSTOM		1
+#define LCD_TYPE_OLD		2
+#define LCD_TYPE_KS0074		3
+#define LCD_TYPE_HANTRONIX	4
+#define LCD_TYPE_NEXCOM		5
 
 /*
  * keypad types
-- 
1.8.1.2

[PATCH v2 2/2] staging: panel: fix lcd type in module parameters

[Sudip Mukherjee]

with reference to the previous patch of the series, fixed the
lcd type in module parameters.

Cc: <stable@vger.kernel.org> # 2.6.32+
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v2: it was not in v1.
might not apply properly to old versions, some reordering was done
in commit <98e0e762e>

 drivers/staging/panel/panel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index 4724bef..ea54fb4 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -500,7 +500,7 @@ MODULE_PARM_DESC(keypad_type,
 static int lcd_type = NOT_SET;
 module_param(lcd_type, int, 0000);
 MODULE_PARM_DESC(lcd_type,
-		 "LCD type: 0=none, 1=old //, 2=serial ks0074, 3=hantronix //, 4=nexcom //, 5=compiled-in");
+		 "LCD type: 0=none, 1=compiled-in, 2=old, 3=serial ks0074, 4=hantronix, 5=nexcom");
 
 static int lcd_height = NOT_SET;
 module_param(lcd_height, int, 0000);
-- 
1.8.1.2

Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

[Dan Carpenter]

On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> with reference to the previous patch of the series, fixed the
> lcd type in module parameters.
> 
> Cc: <stable@vger.kernel.org> # 2.6.32+
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---

This should have been folded into the other patch because now if we
just apply 1/2 it doesn't make sense.

regards,
dan carpenter

Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

[Sudip Mukherjee]

On Tue, Mar 24, 2015 at 01:05:20PM +0300, Dan Carpenter wrote:
> On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> > with reference to the previous patch of the series, fixed the
> > lcd type in module parameters.
> > 
> > Cc: <stable@vger.kernel.org> # 2.6.32+
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> 
> This should have been folded into the other patch because now if we
> just apply 1/2 it doesn't make sense.
ohh, ok. but i thought of separating them as 1/2 can be aplied cleanly
on the stable versions, but this one, it might need some modifications.
will resend a single patch as v3.

regards
sudip
> 
> regards,
> dan carpenter
> 

Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

[Willy Tarreau]

On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> with reference to the previous patch of the series, fixed the
> lcd type in module parameters.

Sudip, it's better to avoid fragmenting patches like you did, because
it will result in a kernel state where there is an inconsistency between
the parameters actually used by the kernel and those reported by modinfo.
This can happen for example if someone does a bisect and ends up on patch
1/2 applied only. Obviously in this case there is very little harm, but
you get the idea : each patch should be a functional change, address one
thing and do it consistently. So if you change the #defines or enums or
whatever, all the locations where their old values were referenced must
be changed in the same patch, simply because in fact they are duplicate
entries. Another point is that someone who notices your patch v1 and
does not notice patch v2 could pick v1 for his kernel and end up with
something inconsistent. For this reason it would be better to merge your
patches into a single one here.

> might not apply properly to old versions, some reordering was done
> in commit <98e0e762e>

Appreciated, thanks for checking this!

Best regards,
Willy

Re: [PATCH v2 2/2] staging: panel: fix lcd type in module parameters

[Sudip Mukherjee]

On Wed, Mar 25, 2015 at 12:25:59AM +0100, Willy Tarreau wrote:
> On Tue, Mar 24, 2015 at 02:56:33PM +0530, Sudip Mukherjee wrote:
> > with reference to the previous patch of the series, fixed the
> > lcd type in module parameters.
> 
> Sudip, it's better to avoid fragmenting patches like you did, because
> it will result in a kernel state where there is an inconsistency between
> the parameters actually used by the kernel and those reported by modinfo.
> This can happen for example if someone does a bisect and ends up on patch
> 1/2 applied only. Obviously in this case there is very little harm, but
> you get the idea : each patch should be a functional change, address one
> thing and do it consistently. So if you change the #defines or enums or
> whatever, all the locations where their old values were referenced must
> be changed in the same patch, simply because in fact they are duplicate
> entries. Another point is that someone who notices your patch v1 and
> does not notice patch v2 could pick v1 for his kernel and end up with
> something inconsistent. For this reason it would be better to merge your
> patches into a single one here.
explained very well. thanks. i was wondering why Dan asked me to merge
them into one.

regards
sudip

> 
> > might not apply properly to old versions, some reordering was done
> > in commit <98e0e762e>
> 
> Appreciated, thanks for checking this!
> 
> Best regards,
> Willy
>