On Tue, Oct 07, 2003 at 03:27:56PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 07 of October 2003 10:27, Jeremy Higdon wrote:
> > lspci gives the version number.
> > /proc/ide/sgiioc4 gives you:
> >
> >         SGI IOC4 Chipset rev 79.
> >         Chipset has 1 IDE channel and supports 2 devices on that channel.
> >         Chipset supports DMA in MultiMode-2 data transfer protocol.
> >
> > Is the # of IDE channels/devices and the DMA mode also available elsewhere?
> 
> Channels/devices is only available indirectly by /proc/ide/ device symlinks
> and mate entry in (ie.) ide0 proc directory.  Controller DMA mode info is not
> available.  However /proc/ide/sgiioc4 is useless for programs (parsing it to
> obtain needed info is a really bad idea) and info for users should be provided
> by drivers/ide/Kconfig (and optionally by init time message).

Okay, I've removed this /proc entry (which was the only /proc entry in the
driver).

> > How important is this to you?  It seems more a style issue.  I agree with
> > you, by the way.  When I write code, I try to minimize forward
> > declarations. If I can get rid of some easily, will that be good enough?
> 
> Okay, I can clear the rest.
> 
> It is good for simplicity (people reading this code) and maintainability
> (you don't have to update declaration if you change corresponding function).

Once I got started, I found that I was able to get rid of all of them.


> > What are they used for (i.e. what are you looking for in the comment)?
> 
> It is mainly x86 thing (if channel is disabled in BIOS we don't probe it).
> You can just put "/* SGI IOC4 doesn't have enablebits. */ somewhere.

done

> > > sgiioc4_init_one():
> > > +	pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
> > > +	class_rev &= 0xff;
> > >
> > > Access to PCI devices before pci_enable_device()
> > > (it is called later in pci_init_sgiioc4).
> >
> > Is pci_enable_device required before a config space access?
> 
> Yes, please read Documentation/pci.txt chapter 3.

Fixed.

Attached is the latest patch, which also includes your patch sent previously.
Thanks for your review and assistance.

I will be on a vacation starting tomorrow, so I won't be able to reply until
Oct 19th or 20th, in case there are any more issues.  Hopefully this one will
be okay  :-)

thanks

jeremy