Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andrew Morton <akpm@linux-foundation.org>]

On Sat, 30 May 2009 20:05:52 +0200
Tobias M__ller <Tobias_Mueller@twam.info> wrote:

> From: Tobias Mueller <Tobias_Mueller@twam.info>
> 
> A GPIO driver for AMD Geode Companion Device CS5535/CS5536
> using the GPIO framework as a replacement for old cs5535-gpio driver.
>
> ...
>
> +/* base address of gpio I/O*/
> +static u32 iobase;

Formally this should have type resource_size_t, I think.  Or `void
__iomem *'.  But given the limited applicability of this driver
(geode-only), I guess u32 is OK.

> +static int gpiobase;
> +module_param_named(gpiobase, gpiobase, int, 0444);
> +MODULE_PARM_DESC(gpiobase, "The GPIO number base. -1 means dynamic");
> +
> +static ulong mask;

hm, ulong isn't a particularly popular "type" in the kernel.  But I
guess it's stable enough.

> +module_param_named(mask, mask, ulong, 0444);
> +MODULE_PARM_DESC(mask, "GPIO channel mask.");
> +
> +/* gets the regiter offset for the GPIO bank.
> + *      low (0-15) stats at 0x00, high (16-31) starts at 0x80 */
>
> ...
>
> +#define gpio_cs5535_suspend NULL
> +#define gpio_cs5535_resume NULL
> +
> +static struct pci_driver gpio_cs5535_driver = {
> +	.name           = "gpio_cs5535",
> +	.id_table       = gpio_cs5535_pci_tbl,
> +	.probe          = gpio_cs5535_probe,
> +	.remove         = gpio_cs5535_remove,
> +	.suspend        = gpio_cs5535_suspend,
> +	.resume         = gpio_cs5535_resume,
> +};

What are the implications of not having suspend/resume support?

> +static int __init gpio_cs5535_init(void)
> +{
> +	return pci_register_driver(&gpio_cs5535_driver);
> +}
> +module_init(gpio_cs5535_init)
> +
> +static void __exit gpio_cs5535_exit(void)
> +{
> +	pci_unregister_driver(&gpio_cs5535_driver);
> +}
> +module_exit(gpio_cs5535_exit);