RE: [PATCH 88/90] staging: comedi: rtd520: store the pci_dev in thecomedi_device

[H Hartley Sweeten]

On Thursday, July 19, 2012 5:42 AM, Ian Abbott wrote:
> On 2012-07-19 03:05, H Hartley Sweeten wrote:
>> Use the hw_dev pointer in the comedi_device struct to hold the
>> pci_dev instead of carrying it in the private data.
>
> My reply to patch 86 has a knock-on effect for this patch...
>
>>   	/* Get the physical address from PCI config */
>> -	dev->iobase = pci_resource_start(devpriv->pci_dev, LAS0_PCIINDEX);
>> -	physLas1 = pci_resource_start(devpriv->pci_dev, LAS1_PCIINDEX);
>> -	physLcfg = pci_resource_start(devpriv->pci_dev, LCFG_PCIINDEX);
>> +	dev->iobase = pci_resource_start(pcidev, LAS0_PCIINDEX);
>> +	physLas1 = pci_resource_start(pcidev, LAS1_PCIINDEX);
>> +	physLcfg = pci_resource_start(pcidev, LCFG_PCIINDEX);
>
> ...in the use of dev->iobase to hold a PCI memory address.

90 patches, 12 issues... not bad... ;-)

I already posted a v2 of PATCH 01/90 to fix calling to_pci_dev() with
a NULL pointer.

Patches 15, 19, 22, and 23 all deal with the pci_is_enabled(). I think
that call basically keeps the existing functionality of the drivers.
If you feel they should be removed, would it be ok to do that in
a separate patchset?

Patches 30, 32, and 34 deal with the 'bustype'. You already mentioned
that you would submit a patch to fix this.

Patch 61 needs the subsystem_device check. I can fix this and repost
If you feel it's necessary.

Patches 67, 86, and 88 all deal with saving the pci base address in dev->iobase.
I think these are all 32-bit PCI cards so it shouldn't "break" anything. But,
the base address should probably be fetched into a resource_size_t for
passing into the ioremap() call. We can still use dev->iobase to allow the
detach to know that the pci device has been enabled. How about setting it
to something like -EINVAL?

Regards,
Hartley
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i