Re: [MeeGo-Dev][PATCH v2] Topcliff: Update PCH_GPIO driver to 2.6.35

[Mike Frysinger]

On Tue, Sep 28, 2010 at 05:37, Masayuki Ohtak wrote:
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
>
> Âcomment "I2C GPIO expanders:"
>
> +config PCH_GPIO
> + Â Â Â tristate "PCH GPIO"
> + Â Â Â depends on PCI

this is connected to a PCI bus, not I2C, so it shouldnt be under the
I2C comment.  move it to a diff place in the file.

> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -35,3 +35,4 @@ obj-$(CONFIG_GPIO_WM8994) Â Â += wm8994-gpio.o
> Âobj-$(CONFIG_GPIO_SCH) Â Â Â Â += sch_gpio.o
> Âobj-$(CONFIG_GPIO_RDC321X) Â Â += rdc321x-gpio.o
> Âobj-$(CONFIG_GPIO_JANZ_TTL) Â Â+= janz-ttl.o
> +obj-$(CONFIG_PCH_GPIO) += pch_gpio.o

there is a clear standard of naming things CONFIG_GPIO_XXX, so i'd
change your CONFIG_PCH_GPIO to CONFIG_GPIO_PCH

> +#include <linux/cdev.h>

you dont actually use cdev anywhere.  punt the header.

> +#define MODULE_NAME "pch_gpio"

KBUILD_MODNAME already exists

> +#define PCI_DEVICE_ID_PCH_GPIO 0x8803

considering you use this define in one place, i dont see why you need
a define at all.  just put the id in the one structure.

> +#ifdef CONFIG_PM
> +static s32 pch_gpio_suspend(struct pci_dev *pdev, pm_message_t state)
> +static s32 pch_gpio_resume(struct pci_dev *pdev)

suspend/resume funcs return "int", not "s32"

> +static struct pci_device_id pch_gpio_pcidev_id[] = {

should be const

> +static s32 __devinit pch_gpio_probe(struct pci_dev *pdev,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â const struct pci_device_id *id)

return value is "int", not "s32"

> + Â Â Â chip = kzalloc(sizeof(struct pch_gpio_chip), GFP_KERNEL);

sizeof(*chip)


> + Â Â Â Â Â Â Â printk(KERN_ERR "PCH gpio: Failed to register GPIO\n");

use dev_err()

> +static s32 __init pch_gpio_pci_init(void)
> +{
> + Â Â Â return pci_register_driver(&pch_gpio_driver);
> +}
> +module_init(pch_gpio_pci_init);

module_init funcs return "int", not "s32"
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/