Re: [ALSA] add __devinitdata to all pci_device_id

Re: [ALSA] add __devinitdata to all pci_device_id

[Arjan van de Ven]

On Mon, 2006-05-01 at 15:11 +0000, Linux Kernel Mailing List wrote:
> commit 396c9b928d5c24775846a161a8191dcc1ea4971f
> tree 447f4b28c2dd8e0026b96025fb94dbc654d6cade
> parent 71b2ccc3a2fd6c27e3cd9b4239670005978e94ce
> author Henrik Kretzschmar <henne@nachtwindheim.de> Mon, 24 Apr 2006 15:59:04 +0200
> committer Jaroslav Kysela <perex@suse.cz> Thu, 27 Apr 2006 21:10:34 +0200
> 
> [ALSA] add __devinitdata to all pci_device_id


are you really really sure you want to do this?
These structures are exported via sysfs for example, I would think this
is quite the wrong thing to make go away silently...

Re: [ALSA] add __devinitdata to all pci_device_id

[Greg KH]

On Mon, May 01, 2006 at 06:49:24PM +0200, Arjan van de Ven wrote:
> On Mon, 2006-05-01 at 15:11 +0000, Linux Kernel Mailing List wrote:
> > commit 396c9b928d5c24775846a161a8191dcc1ea4971f
> > tree 447f4b28c2dd8e0026b96025fb94dbc654d6cade
> > parent 71b2ccc3a2fd6c27e3cd9b4239670005978e94ce
> > author Henrik Kretzschmar <henne@nachtwindheim.de> Mon, 24 Apr 2006 15:59:04 +0200
> > committer Jaroslav Kysela <perex@suse.cz> Thu, 27 Apr 2006 21:10:34 +0200
> > 
> > [ALSA] add __devinitdata to all pci_device_id
> 
> 
> are you really really sure you want to do this?
> These structures are exported via sysfs for example, I would think this
> is quite the wrong thing to make go away silently...

I asked Henrik to not do this, but oh well...

No, if they are marked __devinit, and CONFIG_HOTPLUG is enabled, then
the sysfs stuff is enabled.  And since CONFIG_HOTPLUG is pretty much
always enabled these days, the savings of this kind of patch is
non-existant...

greg k-h

Re: [ALSA] add __devinitdata to all pci_device_id

[Henne]

On Mon, May 01, 2006 at 06:49:24PM +0200, Arjan van de Ven wrote:

 > are you really really sure you want to do this?
 > These structures are exported via sysfs for example, I would think this
 > is quite the wrong thing to make go away silently...

Hi there!

Sorry if I didn't mention everything I was .
A couple of devices had __devinitdata on their tables like written in Documentation/pci.txt,
so I thought this is something that was forgotten.
How can we get clearness in that topic?

Greets,
Henrik

Re: [ALSA] add __devinitdata to all pci_device_id

[Takashi Iwai]

At Mon, 1 May 2006 09:54:43 -0700,
Greg KH wrote:
> 
> On Mon, May 01, 2006 at 06:49:24PM +0200, Arjan van de Ven wrote:
> > On Mon, 2006-05-01 at 15:11 +0000, Linux Kernel Mailing List wrote:
> > > commit 396c9b928d5c24775846a161a8191dcc1ea4971f
> > > tree 447f4b28c2dd8e0026b96025fb94dbc654d6cade
> > > parent 71b2ccc3a2fd6c27e3cd9b4239670005978e94ce
> > > author Henrik Kretzschmar <henne@nachtwindheim.de> Mon, 24 Apr 2006 15:59:04 +0200
> > > committer Jaroslav Kysela <perex@suse.cz> Thu, 27 Apr 2006 21:10:34 +0200
> > > 
> > > [ALSA] add __devinitdata to all pci_device_id
> > 
> > 
> > are you really really sure you want to do this?
> > These structures are exported via sysfs for example, I would think this
> > is quite the wrong thing to make go away silently...
> 
> I asked Henrik to not do this, but oh well...
> 
> No, if they are marked __devinit, and CONFIG_HOTPLUG is enabled, then
> the sysfs stuff is enabled.  And since CONFIG_HOTPLUG is pretty much
> always enabled these days, the savings of this kind of patch is
> non-existant...

Then actually what could be a pitfall by adding __devinitdata to
pci_device_id table?  If there is a potential danger, we should remove
these modifiers from all places, especially from
Documentation/pci.txt.


thanks,

Takashi

Re: [ALSA] add __devinitdata to all pci_device_id

[Henne]

Henne wrote:
> On Mon, May 01, 2006 at 06:49:24PM +0200, Arjan van de Ven wrote:
> 
>  > are you really really sure you want to do this?
>  > These structures are exported via sysfs for example, I would think this
>  > is quite the wrong thing to make go away silently...


I tested that on my system.

make oldconfig
<comment out CONFIG_HOTPLUG>
<install and booting that kernel>
<reading vendor device, etc. of built-in devices in sysfs>
<adding new ID's via new_id>
no errors, no crash
But that mustn't mean something.
I'll take a look at the pci-driver, but some who is more familiar with that should take a look, too.

Greets,
Henne

Re: [ALSA] add __devinitdata to all pci_device_id

[Greg KH]

On Tue, May 02, 2006 at 11:48:31AM +0200, Takashi Iwai wrote:
> At Mon, 1 May 2006 09:54:43 -0700,
> Greg KH wrote:
> > 
> > On Mon, May 01, 2006 at 06:49:24PM +0200, Arjan van de Ven wrote:
> > > On Mon, 2006-05-01 at 15:11 +0000, Linux Kernel Mailing List wrote:
> > > > commit 396c9b928d5c24775846a161a8191dcc1ea4971f
> > > > tree 447f4b28c2dd8e0026b96025fb94dbc654d6cade
> > > > parent 71b2ccc3a2fd6c27e3cd9b4239670005978e94ce
> > > > author Henrik Kretzschmar <henne@nachtwindheim.de> Mon, 24 Apr 2006 15:59:04 +0200
> > > > committer Jaroslav Kysela <perex@suse.cz> Thu, 27 Apr 2006 21:10:34 +0200
> > > > 
> > > > [ALSA] add __devinitdata to all pci_device_id
> > > 
> > > 
> > > are you really really sure you want to do this?
> > > These structures are exported via sysfs for example, I would think this
> > > is quite the wrong thing to make go away silently...
> > 
> > I asked Henrik to not do this, but oh well...
> > 
> > No, if they are marked __devinit, and CONFIG_HOTPLUG is enabled, then
> > the sysfs stuff is enabled.  And since CONFIG_HOTPLUG is pretty much
> > always enabled these days, the savings of this kind of patch is
> > non-existant...
> 
> Then actually what could be a pitfall by adding __devinitdata to
> pci_device_id table?  If there is a potential danger, we should remove
> these modifiers from all places, especially from
> Documentation/pci.txt.

There's no real pitfall, only the chance for people to get it wrong
(using __initdata instead, or using __devinitdata for hotplug-only
drivers.)

thanks,

greg k-h

Re: [ALSA] add __devinitdata to all pci_device_id

[Greg KH]

On Tue, May 02, 2006 at 05:36:21PM +0200, Henne wrote:
> Henne wrote:
> >On Mon, May 01, 2006 at 06:49:24PM +0200, Arjan van de Ven wrote:
> >
> > > are you really really sure you want to do this?
> > > These structures are exported via sysfs for example, I would think this
> > > is quite the wrong thing to make go away silently...
> 
> 
> I tested that on my system.
> 
> make oldconfig
> <comment out CONFIG_HOTPLUG>
> <install and booting that kernel>
> <reading vendor device, etc. of built-in devices in sysfs>
> <adding new ID's via new_id>

Hm, I don't think you really disabled CONFIG_HOTPLUG, as that file will
not be present if you disable that option.

So I think your testing was invalid.

thanks,

greg k-h