[PATCH 1/2] video/sis: Use SiS_DRAMType from init.h and annotate it __devinitconst

This patch removes the duplicated SiS_DRAMType from sis_main.c since it
is already defined exactly the same in init.h.
Since SiS_DRAMType is const and only used by sisfb_post_300_rwtest which is
marked __devinit we can also annotate SiS_DRAMType with __devinitconst.

And since hardcoded values are bad we use ARRAY_SIZE for determining the
size of SiS_DRAMType ;)

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/video/sis/init.h     |    2 +-
 drivers/video/sis/sis_main.c |   21 +--------------------
 2 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/video/sis/init.h b/drivers/video/sis/init.h
index aff7384..26ffb3e 100644
--- a/drivers/video/sis/init.h
+++ b/drivers/video/sis/init.h

@@ -105,7 +105,7 @@ static const unsigned short ModeIndex_1920x1440[]    = {0x68, 0x69, 0x00, 0x6b};
 static const unsigned short ModeIndex_300_2048x1536[]= {0x6c, 0x6d, 0x00, 0x00};
 static const unsigned short ModeIndex_310_2048x1536[]= {0x6c, 0x6d, 0x00, 0x6e};
 
-static const unsigned short SiS_DRAMType[17][5]={
+static const unsigned short __devinitconst SiS_DRAMType[17][5] = {
 	{0x0C,0x0A,0x02,0x40,0x39},
 	{0x0D,0x0A,0x01,0x40,0x48},
 	{0x0C,0x09,0x02,0x20,0x35},

diff --git a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
index 078ca21..8abd42b 100644
--- a/drivers/video/sis/sis_main.c
+++ b/drivers/video/sis/sis_main.c

@@ -4231,27 +4231,8 @@ sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration, int buswidth
 	unsigned short sr14;
 	unsigned int k, RankCapacity, PageCapacity, BankNumHigh, BankNumMid;
 	unsigned int PhysicalAdrOtherPage, PhysicalAdrHigh, PhysicalAdrHalfPage;
-	static const unsigned short SiS_DRAMType[17][5] = {
-		{0x0C,0x0A,0x02,0x40,0x39},
-		{0x0D,0x0A,0x01,0x40,0x48},
-		{0x0C,0x09,0x02,0x20,0x35},
-		{0x0D,0x09,0x01,0x20,0x44},
-		{0x0C,0x08,0x02,0x10,0x31},
-		{0x0D,0x08,0x01,0x10,0x40},
-		{0x0C,0x0A,0x01,0x20,0x34},
-		{0x0C,0x09,0x01,0x08,0x32},
-		{0x0B,0x08,0x02,0x08,0x21},
-		{0x0C,0x08,0x01,0x08,0x30},
-		{0x0A,0x08,0x02,0x04,0x11},
-		{0x0B,0x0A,0x01,0x10,0x28},
-		{0x09,0x08,0x02,0x02,0x01},
-		{0x0B,0x09,0x01,0x08,0x24},
-		{0x0B,0x08,0x01,0x04,0x20},
-		{0x0A,0x08,0x01,0x02,0x10},
-		{0x09,0x08,0x01,0x01,0x00}
-	};
 
-	 for(k = 0; k <= 16; k++) {
+	 for (k = 0; k < ARRAY_SIZE(SiS_DRAMType); k++) {
 
 		RankCapacity = buswidth * SiS_DRAMType[k][3];
 

-- 
1.7.3.4

This patch removes the unused structs SiS_SDRDRAM_TYPE and SiS_DDRDRAM_TYPE
from init.h

These are not used anywhere so we can delete them.

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/video/sis/init.h |   25 -------------------------
 1 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/drivers/video/sis/init.h b/drivers/video/sis/init.h
index 26ffb3e..5c5a96b 100644
--- a/drivers/video/sis/init.h
+++ b/drivers/video/sis/init.h

@@ -125,31 +125,6 @@ static const unsigned short __devinitconst SiS_DRAMType[17][5] = {
 	{0x09,0x08,0x01,0x01,0x00}
 };
 
-static const unsigned short SiS_SDRDRAM_TYPE[13][5] -{
-	{ 2,12, 9,64,0x35},
-	{ 1,13, 9,64,0x44},
-	{ 2,12, 8,32,0x31},
-	{ 2,11, 9,32,0x25},
-	{ 1,12, 9,32,0x34},
-	{ 1,13, 8,32,0x40},
-	{ 2,11, 8,16,0x21},
-	{ 1,12, 8,16,0x30},
-	{ 1,11, 9,16,0x24},
-	{ 1,11, 8, 8,0x20},
-	{ 2, 9, 8, 4,0x01},
-	{ 1,10, 8, 4,0x10},
-	{ 1, 9, 8, 2,0x00}
-};
-
-static const unsigned short SiS_DDRDRAM_TYPE[4][5] -{
-	{ 2,12, 9,64,0x35},
-	{ 2,12, 8,32,0x31},
-	{ 2,11, 8,16,0x21},
-	{ 2, 9, 8, 4,0x01}
-};
-
 static const unsigned char SiS_MDA_DAC[]  {
 	0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,

-- 
1.7.3.4

This patch removes the duplicated SiS_DRAMType from sis_main.c since it
is already defined exactly the same in init.h.
Since we don't want to include init.h (for size reasons) we move the
struct to sis_main.h.
Since SiS_DRAMType is const and only used by sisfb_post_300_rwtest which is
marked __devinit we can also annotate SiS_DRAMType with __devinitconst.

And since hardcoded values are bad we use ARRAY_SIZE for determining the
size of SiS_DRAMType ;)

v2:
Move struct to sis_main.h to decrease module size

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/video/sis/init.h     |   20 --------------------
 drivers/video/sis/sis_main.c |   21 +--------------------
 drivers/video/sis/sis_main.h |   20 ++++++++++++++++++++
 3 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/video/sis/init.h b/drivers/video/sis/init.h
index a22e892..85d6738 100644
--- a/drivers/video/sis/init.h
+++ b/drivers/video/sis/init.h

@@ -105,26 +105,6 @@ static const unsigned short ModeIndex_1920x1440[]    = {0x68, 0x69, 0x00, 0x6b};
 static const unsigned short ModeIndex_300_2048x1536[]= {0x6c, 0x6d, 0x00, 0x00};
 static const unsigned short ModeIndex_310_2048x1536[]= {0x6c, 0x6d, 0x00, 0x6e};
 
-static const unsigned short SiS_DRAMType[17][5]={
-	{0x0C,0x0A,0x02,0x40,0x39},
-	{0x0D,0x0A,0x01,0x40,0x48},
-	{0x0C,0x09,0x02,0x20,0x35},
-	{0x0D,0x09,0x01,0x20,0x44},
-	{0x0C,0x08,0x02,0x10,0x31},
-	{0x0D,0x08,0x01,0x10,0x40},
-	{0x0C,0x0A,0x01,0x20,0x34},
-	{0x0C,0x09,0x01,0x08,0x32},
-	{0x0B,0x08,0x02,0x08,0x21},
-	{0x0C,0x08,0x01,0x08,0x30},
-	{0x0A,0x08,0x02,0x04,0x11},
-	{0x0B,0x0A,0x01,0x10,0x28},
-	{0x09,0x08,0x02,0x02,0x01},
-	{0x0B,0x09,0x01,0x08,0x24},
-	{0x0B,0x08,0x01,0x04,0x20},
-	{0x0A,0x08,0x01,0x02,0x10},
-	{0x09,0x08,0x01,0x01,0x00}
-};
-
 static const unsigned char SiS_MDA_DAC[]  {
 	0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,

diff --git a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
index 078ca21..8abd42b 100644
--- a/drivers/video/sis/sis_main.c
+++ b/drivers/video/sis/sis_main.c

@@ -4231,27 +4231,8 @@ sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration, int buswidth
 	unsigned short sr14;
 	unsigned int k, RankCapacity, PageCapacity, BankNumHigh, BankNumMid;
 	unsigned int PhysicalAdrOtherPage, PhysicalAdrHigh, PhysicalAdrHalfPage;
-	static const unsigned short SiS_DRAMType[17][5] = {
-		{0x0C,0x0A,0x02,0x40,0x39},
-		{0x0D,0x0A,0x01,0x40,0x48},
-		{0x0C,0x09,0x02,0x20,0x35},
-		{0x0D,0x09,0x01,0x20,0x44},
-		{0x0C,0x08,0x02,0x10,0x31},
-		{0x0D,0x08,0x01,0x10,0x40},
-		{0x0C,0x0A,0x01,0x20,0x34},
-		{0x0C,0x09,0x01,0x08,0x32},
-		{0x0B,0x08,0x02,0x08,0x21},
-		{0x0C,0x08,0x01,0x08,0x30},
-		{0x0A,0x08,0x02,0x04,0x11},
-		{0x0B,0x0A,0x01,0x10,0x28},
-		{0x09,0x08,0x02,0x02,0x01},
-		{0x0B,0x09,0x01,0x08,0x24},
-		{0x0B,0x08,0x01,0x04,0x20},
-		{0x0A,0x08,0x01,0x02,0x10},
-		{0x09,0x08,0x01,0x01,0x00}
-	};
 
-	 for(k = 0; k <= 16; k++) {
+	 for (k = 0; k < ARRAY_SIZE(SiS_DRAMType); k++) {
 
 		RankCapacity = buswidth * SiS_DRAMType[k][3];
 

diff --git a/drivers/video/sis/sis_main.h b/drivers/video/sis/sis_main.h
index 9540e97..242c1c3 100644
--- a/drivers/video/sis/sis_main.h
+++ b/drivers/video/sis/sis_main.h

@@ -776,6 +776,26 @@ extern void		SiS_Chrontel701xBLOff(struct SiS_Private *SiS_Pr);
 #endif
 extern void		SiS_SiS30xBLOn(struct SiS_Private *SiS_Pr);
 extern void		SiS_SiS30xBLOff(struct SiS_Private *SiS_Pr);
+
+static const unsigned short __devinitconst SiS_DRAMType[17][5] = {
+	{0x0C,0x0A,0x02,0x40,0x39},
+	{0x0D,0x0A,0x01,0x40,0x48},
+	{0x0C,0x09,0x02,0x20,0x35},
+	{0x0D,0x09,0x01,0x20,0x44},
+	{0x0C,0x08,0x02,0x10,0x31},
+	{0x0D,0x08,0x01,0x10,0x40},
+	{0x0C,0x0A,0x01,0x20,0x34},
+	{0x0C,0x09,0x01,0x08,0x32},
+	{0x0B,0x08,0x02,0x08,0x21},
+	{0x0C,0x08,0x01,0x08,0x30},
+	{0x0A,0x08,0x02,0x04,0x11},
+	{0x0B,0x0A,0x01,0x10,0x28},
+	{0x09,0x08,0x02,0x02,0x01},
+	{0x0B,0x09,0x01,0x08,0x24},
+	{0x0B,0x08,0x01,0x04,0x20},
+	{0x0A,0x08,0x01,0x02,0x10},
+	{0x09,0x08,0x01,0x01,0x00}
+};
 #endif
 
 

-- 
1.7.3.4

Hi Peter,

On 04/29/2012 08:26 PM, Peter Huewe wrote:

This patch removes the duplicated SiS_DRAMType from sis_main.c since it
is already defined exactly the same in init.h.
Since we don't want to include init.h (for size reasons) we move the
struct to sis_main.h.
Since SiS_DRAMType is const and only used by sisfb_post_300_rwtest which is
marked __devinit we can also annotate SiS_DRAMType with __devinitconst.

And since hardcoded values are bad we use ARRAY_SIZE for determining the
size of SiS_DRAMType ;)

v2:
Move struct to sis_main.h to decrease module size

As far as I can see it should be possible to keep the array in
sis_main.c and just delete it in the header files, shouldn't it? I'd
prefer to do it this way.


Best regards,

Florian Tobias Schandinat

Hi Florian,

As far as I can see it should be possible to keep the array in
sis_main.c and just delete it in the header files, shouldn't it? I'd
prefer to do it this way.

Yes it is absolutely possible to leave it in sis_main.c and remove it from the 
header files. I already thought about this when creating the patch, but 
decided against it, as the 17*5 = 85 bytes are allocated on the stack, while 
they nicely can be put in the .devinit.rodata section ;)

With the patch:
344072  sis_main.o
1200950 sisfb.o
1217491 sisfb.ko

vs without the patch and removing it only from the header:
 344176  sis_main.o
1201056  sisfb.o
1217597  sisfb.ko -> ~100bytes more in the final module.

However I'm fine with this  and will remove it from the header and squash this 
together with the 
"video/sis: Remove unused structs SiS_SDRDRAM_TYPE/SiS_DDRDRAM_TYPE"
I also sent to you.


Thanks,
Peterq

Hi Peter,

On 05/03/2012 09:40 PM, Peter Hüwe wrote:

Hi Florian,

quoted

As far as I can see it should be possible to keep the array in
sis_main.c and just delete it in the header files, shouldn't it? I'd
prefer to do it this way.

Yes it is absolutely possible to leave it in sis_main.c and remove it from the 
header files. I already thought about this when creating the patch, but 
decided against it, as the 17*5 = 85 bytes are allocated on the stack, while 
they nicely can be put in the .devinit.rodata section ;)

You can still mark it as __devinitconst, I just wanted to have the array
inside a C file, not a header. If you agree with this, I'll change your
new patch that way.

With the patch:
344072  sis_main.o
1200950 sisfb.o
1217491 sisfb.ko

vs without the patch and removing it only from the header:
 344176  sis_main.o
1201056  sisfb.o
1217597  sisfb.ko -> ~100bytes more in the final module.

However I'm fine with this  and will remove it from the header and squash this 
together with the 
"video/sis: Remove unused structs SiS_SDRDRAM_TYPE/SiS_DDRDRAM_TYPE"
I also sent to you.

Thanks,

Florian Tobias Schandinat

Am Donnerstag 03 Mai 2012, 23:48:00 schrieb Florian Tobias Schandinat:

You can still mark it as __devinitconst, I just wanted to have the array
inside a C file, not a header. If you agree with this, I'll change your
new patch that way.

I'm totally fine with this - Thanks,
Peter

SiS_DRAMType is const and only used by sisfb_post_300_rwtest which is
marked __devinit we can annotate SiS_DRAMType with __devinitconst and
move it into the file scope in order to not have it created on the
stack.
This patch decreases the compiled module size by about 100bytes.

And since hardcoded values are bad we use ARRAY_SIZE for determining
the size of SiS_DRAMType ;)

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/video/sis/sis_main.c |   41 +++++++++++++++++++++--------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
index 078ca21..a7a48db 100644
--- a/drivers/video/sis/sis_main.c
+++ b/drivers/video/sis/sis_main.c

@@ -4222,6 +4222,26 @@ sisfb_post_300_buswidth(struct sis_video_info *ivideo)
 	return 1;			/* 32bit */
 }
 
+static const unsigned short __devinitconst SiS_DRAMType[17][5] = {
+	{0x0C,0x0A,0x02,0x40,0x39},
+	{0x0D,0x0A,0x01,0x40,0x48},
+	{0x0C,0x09,0x02,0x20,0x35},
+	{0x0D,0x09,0x01,0x20,0x44},
+	{0x0C,0x08,0x02,0x10,0x31},
+	{0x0D,0x08,0x01,0x10,0x40},
+	{0x0C,0x0A,0x01,0x20,0x34},
+	{0x0C,0x09,0x01,0x08,0x32},
+	{0x0B,0x08,0x02,0x08,0x21},
+	{0x0C,0x08,0x01,0x08,0x30},
+	{0x0A,0x08,0x02,0x04,0x11},
+	{0x0B,0x0A,0x01,0x10,0x28},
+	{0x09,0x08,0x02,0x02,0x01},
+	{0x0B,0x09,0x01,0x08,0x24},
+	{0x0B,0x08,0x01,0x04,0x20},
+	{0x0A,0x08,0x01,0x02,0x10},
+	{0x09,0x08,0x01,0x01,0x00}
+};
+
 static int __devinit
 sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration, int buswidth,
 			int PseudoRankCapacity, int PseudoAdrPinCount,

@@ -4231,27 +4251,8 @@ sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration, int buswidth
 	unsigned short sr14;
 	unsigned int k, RankCapacity, PageCapacity, BankNumHigh, BankNumMid;
 	unsigned int PhysicalAdrOtherPage, PhysicalAdrHigh, PhysicalAdrHalfPage;
-	static const unsigned short SiS_DRAMType[17][5] = {
-		{0x0C,0x0A,0x02,0x40,0x39},
-		{0x0D,0x0A,0x01,0x40,0x48},
-		{0x0C,0x09,0x02,0x20,0x35},
-		{0x0D,0x09,0x01,0x20,0x44},
-		{0x0C,0x08,0x02,0x10,0x31},
-		{0x0D,0x08,0x01,0x10,0x40},
-		{0x0C,0x0A,0x01,0x20,0x34},
-		{0x0C,0x09,0x01,0x08,0x32},
-		{0x0B,0x08,0x02,0x08,0x21},
-		{0x0C,0x08,0x01,0x08,0x30},
-		{0x0A,0x08,0x02,0x04,0x11},
-		{0x0B,0x0A,0x01,0x10,0x28},
-		{0x09,0x08,0x02,0x02,0x01},
-		{0x0B,0x09,0x01,0x08,0x24},
-		{0x0B,0x08,0x01,0x04,0x20},
-		{0x0A,0x08,0x01,0x02,0x10},
-		{0x09,0x08,0x01,0x01,0x00}
-	};
 
-	 for(k = 0; k <= 16; k++) {
+	 for(k = 0; k < ARRAY_SIZE(SiS_DRAMType); k++) {
 
 		RankCapacity = buswidth * SiS_DRAMType[k][3];
 

-- 
1.7.3.4

On 05/03/2012 10:14 PM, Peter Huewe wrote:

SiS_DRAMType is const and only used by sisfb_post_300_rwtest which is
marked __devinit we can annotate SiS_DRAMType with __devinitconst and
move it into the file scope in order to not have it created on the
stack.
This patch decreases the compiled module size by about 100bytes.

And since hardcoded values are bad we use ARRAY_SIZE for determining
the size of SiS_DRAMType ;)

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>

Good that you did this one, it was more to be done than I expected. I
ignored the checkpatch errors as you didn't introduce them but maybe it
wouldn't be a bad idea to fix things up if you touch them. Applied.


Thanks,

Florian Tobias Schandinat

Am Donnerstag 10 Mai 2012, 02:19:18 schrieb Florian Tobias Schandinat:

Good that you did this one, it was more to be done than I expected. I
ignored the checkpatch errors as you didn't introduce them but maybe it
wouldn't be a bad idea to fix things up if you touch them. Applied.

Oh sorry, usually I do checkpatch, but I missed it on this one somehow.
If you want, I can do a checkpatch cleanup for the whole sis driver as 
compensation ;)

Thanks,
Peter

Hi Peter,

On 05/10/2012 09:39 PM, Peter Hüwe wrote:

Am Donnerstag 10 Mai 2012, 02:19:18 schrieb Florian Tobias Schandinat:

quoted

Good that you did this one, it was more to be done than I expected. I
ignored the checkpatch errors as you didn't introduce them but maybe it
wouldn't be a bad idea to fix things up if you touch them. Applied.

Oh sorry, usually I do checkpatch, but I missed it on this one somehow.
If you want, I can do a checkpatch cleanup for the whole sis driver as 
compensation ;)

That's not necessary.
I just wanted to highlight that I prefer it if people fix the style
on-the-fly while working on things rather than sending me individual
patches for each line and not even fixing the complete line but only one
special aspect of it so that checkpatch complains about their style fixes.

btw. Special characters are allowed in sign-offs.


Best regards,

Florian Tobias Schandinat