Re: [PATCH 1/1] pci: Block config access during BIST (resend)

Greg KH
2005-02-01T18:59:36Z

On Tue, Feb 01, 2005 at 03:44:00PM +0000, Matthew Wilcox wrote:
> On Tue, Feb 01, 2005 at 09:12:56AM -0600, Brian King wrote:
> > Greg KH wrote:
> > >On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
> > >>+void pci_block_user_cfg_access(struct pci_dev *dev)
> > >>+{
> > >>+ unsigned long flags;
> > >>+
> > >>+ pci_save_state(dev);
> > >>+ spin_lock_irqsave(&pci_lock, flags);
> > >>+ dev->block_ucfg_access = 1;
> > >>+ spin_unlock_irqrestore(&pci_lock, flags);
> > >>+}
> > >>+EXPORT_SYMBOL(pci_block_user_cfg_access);
> > >
> > >
> > >EXPORT_SYMBOL_GPL() please?
> >
> > Ok.
>
> I'm not entirely convinced these should be GPL-only exports. Basically,
> this is saying that any driver for a device that has this problem must be
> GPLd. I think that's a firmer stance than Linus had in mind originally.

"originally"? These are new functions added to the PCI core. I think
that any driver that wants to use this functionality had better be
released under the GPL as what they are wanting to do is a "new" thing.

That's why I prefer all new exports to be marked _GPL.

thanks,

greg k-h
> > +void pci_unblock_user_cfg_access(struct pci_dev *dev)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&pci_lock, flags);
> > + dev->block_ucfg_access = 0;
> > + spin_unlock_irqrestore(&pci_lock, flags);
> > +}
>
> If we've done a write to config space while the adapter was blocked,
> shouldn't we replay those accesses at this point?

This has been discussed a lot already. I think we might as well let the
thing fail in random and odd ways, as it's some pretty broken hardware
anyway that wants this functionality :)

thanks,

greg k-h