Re: [PATCH] [MTD] MAPS: Support for BIOS flash chips on the nvidia ck804 southbridge

[Artem Bityutskiy <dedekind@infradead.org>]

Hello Rayan,

I have few questions (may be silly) and comments.

On Tue, 2006-10-31 at 16:56 -0700, Ryan Jackson wrote:
> +static int __devinit ck804xrom_init_one (struct pci_dev *pdev,
> +	const struct pci_device_id *ent)

I wonder why do you use __devinit here, not __init? AFAIK, __devinit is
used for hotplug-able devices, is this the case for a video card?

> +{
> +	static char *rom_probe_types[] = { "cfi_probe", "jedec_probe", NULL };

So the interface may really be JEDEC and CFI?
> phys;
> +		map->map.virt = (void __iomem *)
> +			(((unsigned long)(window->virt)) + offset);
map->map.virt is 'void *', so 

map->map.virt = window->virt + offset * sizeof(long);

could be used instead and it looks neater. In gcc sizeof(void) is 1 and
is is widely used in the kernel.

> +static int __init init_ck804xrom(void)
> +{
> +	struct pci_dev *pdev;
> +	struct pci_device_id *id;
> +	int retVal;
> +	pdev = NULL;
> +
> +	for(id = ck804xrom_pci_tbl; id->vendor; id++) {
> +		pdev = pci_find_device(id->vendor, id->device, NULL);
> +		if (pdev)
> +			break;
> +	}
> +	if (pdev) {
> +		retVal = ck804xrom_init_one(pdev, &ck804xrom_pci_tbl[0]);
> +		pci_dev_put(pdev);
> +		return retVal;
> +	}
> +	return -ENXIO;
> +#if 0
> +	return pci_module_init(&ck804xrom_driver);
> +#endif

Why you commented out pci_module_init() and the related stuff? Why you
do not use this standard mechanism?

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)