Re: [PATCH] gpio/ml_ioh_gpio: ML7213 GPIO driver

Andrew Morton
2011-01-03T21:43:06Z

On Thu, 23 Dec 2010 20:58:44 +0900
Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com> wrote:

> ML7213 is companion chip for Intel Atom E6xx series.
> This driver can be used for OKI SEMICONDUCTOR ML7213 IOH(Input/Output Hub) which
> is for IVI(In-Vehicle Infotainment) use.
> This driver can access the IOH's GPIO device.
>
>
> ...
>
> +static int __devinit ioh_gpio_probe(struct pci_dev *pdev,
> + const struct pci_device_id *id)
> +{
> + int ret;
> + int i;
> + struct ioh_gpio *chip;
> + void __iomem *base;
> + void __iomem *chip_save;
> +
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "%s : pci_enable_device failed", __func__);
> + goto err_pci_enable;
> + }
> +
> + ret = pci_request_regions(pdev, KBUILD_MODNAME);
> + if (ret) {
> + dev_err(&pdev->dev, "pci_request_regions failed-%d", ret);
> + goto err_request_regions;
> + }
> +
> + base = pci_iomap(pdev, 1, 0);
> + if (base == 0) {
> + dev_err(&pdev->dev, "%s : pci_iomap failed", __func__);
> + ret = -ENOMEM;
> + goto err_iomap;
> + }
> +
> + chip_save = kzalloc(sizeof(*chip) * 8, GFP_KERNEL);
> + if (chip_save == NULL) {
> + dev_err(&pdev->dev, "%s : kzalloc failed", __func__);
> + ret = -ENOMEM;
> + goto err_kzalloc;
> + }
> +
> + for (i = 0, chip = chip_save; i < 8; i++, chip++) {

chip = chip_save;
for (i = 0; i < 8; i++, chip++) {

is simpler, and simpler is better.

> + chip->dev = &pdev->dev;
> + chip->base = base;
> + chip->reg = chip->base;
> + chip->ch = i;
> + mutex_init(&chip->lock);
> + ioh_gpio_setup(chip, num_ports[i]);
> + ret = gpiochip_add(&chip->gpio);
> + if (ret) {
> + dev_err(&pdev->dev, "IOH gpio: Failed to register GPIO\n");
> + goto err_gpiochip_add;

Here we should run gpiochip_remove() against chips 0 .. i-1, no?

> + }
> + }
> +
> + chip = chip_save;
> + pci_set_drvdata(pdev, chip);
> +
> + return 0;
> +
> +err_gpiochip_add:
> + kfree(chip_save);
> +
> +err_kzalloc:
> + pci_iounmap(pdev, base);
> +
> +err_iomap:
> + pci_release_regions(pdev);
> +
> +err_request_regions:
> + pci_disable_device(pdev);
> +
> +err_pci_enable:
> +
> + dev_err(&pdev->dev, "%s Failed returns %d\n", __func__, ret);
> + return ret;
> +}