[KJ][Patch] fix kbuild warning in sisfb.o

[KJ][Patch] fix kbuild warning in sisfb.o

[Darren Jenkins\]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

G'day list

This one was a lot harder to find, but much easier to fix.

WARNING: drivers/video/sis/sisfb.o - Section mismatch: reference
to .init.data: from .data between 'sisfb_driver' (at offset 0xf8) and
'sisfb_ops'


This is caused by the 'pci_driver struct' in sis_main.c having a pointer
to a 'pci_device_id struct' in 'sis_main.h' that is marked as
__devinitdata.

The patch below just removes the __devinitdata annotation from the
'pci_device_id struct', which seems like the best solution here.



Note: This driver also contains a lot of 

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)

which is in the TODO list
- Code that depends on LINUX_VERSION_CODE & KERNEL_VERSION < 2.6 can be
  deleted in most cases. (Mostly/all? done)

So I might follow with a patch that removes it.


Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>

--- linux-2.6.16-git13/drivers/video/sis/sis_main.h.orig	2006-03-31 23:33:50.000000000 +1100
+++ linux-2.6.16-git13/drivers/video/sis/sis_main.h	2006-03-31 23:34:05.000000000 +1100
@@ -125,7 +125,7 @@ static struct sisfb_chip_info {
 	{ XGI_40,     SIS_315_VGA, 1, HW_CURSOR_AREA_SIZE_315 * 4, SIS_CRT2_WENABLE_315, "XGI V3XT/V5/V8" },
 };
 
-static struct pci_device_id __devinitdata sisfb_pci_table[] = {
+static struct pci_device_id sisfb_pci_table[] = {
 #ifdef CONFIG_FB_SIS_300
 	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_300,     PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_540_VGA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 1},



[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch] fix kbuild warning in sisfb.o

[Adrian Bunk]

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

On Sat, Apr 01, 2006 at 12:11:18AM +1100, Darren Jenkins" wrote:
> G'day list
> 
> This one was a lot harder to find, but much easier to fix.
> 
> WARNING: drivers/video/sis/sisfb.o - Section mismatch: reference
> to .init.data: from .data between 'sisfb_driver' (at offset 0xf8) and
> 'sisfb_ops'
> 
> 
> This is caused by the 'pci_driver struct' in sis_main.c having a pointer
> to a 'pci_device_id struct' in 'sis_main.h' that is marked as
> __devinitdata.
> 
> The patch below just removes the __devinitdata annotation from the
> 'pci_device_id struct', which seems like the best solution here.

ACK, this is a bug that should be fixed.

> Note: This driver also contains a lot of 
> 
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)
> 
> which is in the TODO list
> - Code that depends on LINUX_VERSION_CODE & KERNEL_VERSION < 2.6 can be
>   deleted in most cases. (Mostly/all? done)
> 
> So I might follow with a patch that removes it.

AFAIR, Thomas (Cc'ed) does still share this code between kernel 2.4 and 
2.6, and the #ifdef's should therefore stay.

> Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
> 
> --- linux-2.6.16-git13/drivers/video/sis/sis_main.h.orig	2006-03-31 23:33:50.000000000 +1100
> +++ linux-2.6.16-git13/drivers/video/sis/sis_main.h	2006-03-31 23:34:05.000000000 +1100
> @@ -125,7 +125,7 @@ static struct sisfb_chip_info {
>  	{ XGI_40,     SIS_315_VGA, 1, HW_CURSOR_AREA_SIZE_315 * 4, SIS_CRT2_WENABLE_315, "XGI V3XT/V5/V8" },
>  };
>  
> -static struct pci_device_id __devinitdata sisfb_pci_table[] = {
> +static struct pci_device_id sisfb_pci_table[] = {
>  #ifdef CONFIG_FB_SIS_300
>  	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_300,     PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
>  	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_540_VGA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 1},
> 
> 

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch] fix kbuild warning in sisfb.o

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

On Fri, Mar 31, 2006 at 03:29:29PM +0200, Adrian Bunk wrote:
> > This is caused by the 'pci_driver struct' in sis_main.c having a pointer
> > to a 'pci_device_id struct' in 'sis_main.h' that is marked as
> > __devinitdata.
> > 
> > The patch below just removes the __devinitdata annotation from the
> > 'pci_device_id struct', which seems like the best solution here.
> 
> ACK, this is a bug that should be fixed.

If it is, then Documentation/pci.txt needs to be updated:

        The struct pci_driver shouldn't be marked with any of these tags.
        The ID table array should be marked __devinitdata.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch] fix kbuild warning in sisfb.o

[Matthew Wilcox]

On Fri, Mar 31, 2006 at 03:29:29PM +0200, Adrian Bunk wrote:
> > This is caused by the 'pci_driver struct' in sis_main.c having a pointer
> > to a 'pci_device_id struct' in 'sis_main.h' that is marked as
> > __devinitdata.
> > 
> > The patch below just removes the __devinitdata annotation from the
> > 'pci_device_id struct', which seems like the best solution here.
> 
> ACK, this is a bug that should be fixed.

If it is, then Documentation/pci.txt needs to be updated:

        The struct pci_driver shouldn't be marked with any of these tags.
        The ID table array should be marked __devinitdata.

Re: [KJ][Patch] fix kbuild warning in sisfb.o

[Greg KH]

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

On Sat, Apr 01, 2006 at 12:11:18AM +1100, Darren Jenkins" wrote:
> G'day list
> 
> This one was a lot harder to find, but much easier to fix.
> 
> WARNING: drivers/video/sis/sisfb.o - Section mismatch: reference
> to .init.data: from .data between 'sisfb_driver' (at offset 0xf8) and
> 'sisfb_ops'
> 
> 
> This is caused by the 'pci_driver struct' in sis_main.c having a pointer
> to a 'pci_device_id struct' in 'sis_main.h' that is marked as
> __devinitdata.
> 
> The patch below just removes the __devinitdata annotation from the
> 'pci_device_id struct', which seems like the best solution here.

No, this should be just fine.  devinitdata should be just fine for PCI
device ids.

thanks,

greg k-h

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch] fix kbuild warning in sisfb.o

[Darren Jenkins\]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Sam

I don't know weather you are aware of this one yet, but we have just
found an issue where 'struct pci_device_id' should be marked
__devinitdata (see Documentation/pci.txt) as the pci code apparently
only uses it in the driver initialisation, but the 'struct pci_driver'
will still hold a pointer to it, giving a kbuild warning. See below.
This definitely looks like a false positive that should be removed.

Darren Jenkins


On Fri, 2006-03-31 at 09:00 -0800, Greg KH wrote:
> On Sat, Apr 01, 2006 at 12:11:18AM +1100, Darren Jenkins" wrote:
> > G'day list
> > 
> > This one was a lot harder to find, but much easier to fix.
> > 
> > WARNING: drivers/video/sis/sisfb.o - Section mismatch: reference
> > to .init.data: from .data between 'sisfb_driver' (at offset 0xf8) and
> > 'sisfb_ops'
> > 
> > 
> > This is caused by the 'pci_driver struct' in sis_main.c having a pointer
> > to a 'pci_device_id struct' in 'sis_main.h' that is marked as
> > __devinitdata.
> > 
> > The patch below just removes the __devinitdata annotation from the
> > 'pci_device_id struct', which seems like the best solution here.
> 
> No, this should be just fine.  devinitdata should be just fine for PCI
> device ids.
> 
> thanks,
> 
> greg k-h


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ][Patch] fix kbuild warning in sisfb.o

[Sam Ravnborg]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On Sat, Apr 01, 2006 at 01:12:13PM +1100, Darren Jenkins" wrote:
> Sam
> 
> I don't know weather you are aware of this one yet, but we have just
> found an issue where 'struct pci_device_id' should be marked
> __devinitdata (see Documentation/pci.txt) as the pci code apparently
> only uses it in the driver initialisation, but the 'struct pci_driver'
> will still hold a pointer to it, giving a kbuild warning. See below.
> This definitely looks like a false positive that should be removed.

Hi Darren.
Following patch should fix it.
It recognize that the variable is named *_driver, and in that case
it will not warn about references to .init.data (+.init.text and
.exit.text).

This is only compiletine tested since I'm a bit busy atm.
Let me know if this solves it as expected and especially if it seems
to silence warnings that is not false positives.

	Sam

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0b92ddf..81448e5 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -492,7 +492,7 @@ static int strrcmp(const char *s, const 
  *   These functions may often be marked __init and we do not want to
  *   warn here.
  *   the pattern is identified by:
- *   tosec   = .init.text | .exit.text
+ *   tosec   = .init.text | .exit.text | .init.data
  *   fromsec = .data
  *   atsym = *_driver, *_ops, *_probe, *probe_one
  **/
@@ -522,7 +522,8 @@ static int secref_whitelist(const char *
 
 	/* Check for pattern 2 */
 	if ((strcmp(tosec, ".init.text") != 0) &&
-	    (strcmp(tosec, ".exit.text") != 0))
+	    (strcmp(tosec, ".exit.text") != 0) &&
+	    (strcmp(tosec, ".init.data") != 0))
 		f2 = 0;
 	if (strcmp(fromsec, ".data") != 0)
 		f2 = 0;


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors