From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935418AbcA1MFP (ORCPT ); Thu, 28 Jan 2016 07:05:15 -0500 Received: from mga01.intel.com ([192.55.52.88]:53548 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935372AbcA1MFJ (ORCPT ); Thu, 28 Jan 2016 07:05:09 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,358,1449561600"; d="scan'208";a="642798197" Message-ID: <1453982584.2521.285.camel@linux.intel.com> Subject: Re: [PATCH V2 2/4] gpio: gpio-f81504: Add Fintek F81504/508/512 PCIE-to-UART/GPIO GPIOLIB support From: Andy Shevchenko To: Peter Hung , linus.walleij@linaro.org, gnurou@gmail.com, gregkh@linuxfoundation.org, paul.gortmaker@windriver.com, lee.jones@linaro.org, jslaby@suse.com, peter_hong@fintek.com.tw Cc: heikki.krogerus@linux.intel.com, peter@hurleysoftware.com, soeren.grunewald@desy.de, udknight@gmail.com, adam.lee@canonical.com, arnd@arndb.de, manabian@gmail.com, scottwood@freescale.com, yamada.masahiro@socionext.com, paul.burton@imgtec.com, mans@mansr.com, matthias.bgg@gmail.com, ralf@linux-mips.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, linux-serial@vger.kernel.org, tom_tsai@fintek.com.tw, Peter Hung Date: Thu, 28 Jan 2016 14:03:04 +0200 In-Reply-To: <1453972838-30268-3-git-send-email-hpeter+linux_kernel@gmail.com> References: <1453972838-30268-1-git-send-email-hpeter+linux_kernel@gmail.com> <1453972838-30268-3-git-send-email-hpeter+linux_kernel@gmail.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.3-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote: > This driver is GPIOLIB driver for F81504/508/512, it'll handle the > GPIOLIB operation of this device. This module will depend on > MFD_FINTEK_F81504_CORE. > > + /* > +  * The GPIO default driven mode for this device is open- > drain. The > +  * GPIOLIB had no change GPIO mode API currently. So we > leave the > +  * Push-Pull code below. > +  * > +  * pci_read_config_byte(dev, GPIO_START_ADDR + idx * > GPIO_SET_OFFSET + > +  * GPIO_DRIVE_EN_OFFSET, &tmp); > +  * pci_write_config_byte(dev, GPIO_START_ADDR + idx * > GPIO_SET_OFFSET + > +  * GPIO_DRIVE_EN_OFFSET, tmp | > BIT(gpio_num)); > +  */ > + > + /* set output data */ > + tmp = inb(priv->gpio_ioaddr + gc->idx); ioread8 is a bit better since it automatically works with IO space and MMIO. But if you are certain you will always have the address in IO space, you can disregard this comment. > +static int f81504_gpio_probe(struct platform_device *pdev) > +{ > + int status; > + struct f81504_gpio_chip *gc; > + void *data = dev_get_platdata(&pdev->dev); > + u8 gpio_idx = *(u8 *)data; > + char *name; > + > + if (gpio_idx >= ARRAY_SIZE(fintek_gpio_mapping)) { > + dev_err(&pdev->dev, "%s: gpio_idx:%d out of > range.\n", > + __func__, gpio_idx); > + return -ENODEV; > + } > + > + gc = devm_kzalloc(&pdev->dev, sizeof(*gc), GFP_KERNEL); > + if (!gc) > + return -ENOMEM; > + > > + kfree(data); What the heck? > + mutex_init(&gc->locker); > + platform_set_drvdata(pdev, gc); > + > + name = devm_kzalloc(&pdev->dev, FINTEK_GPIO_NAME_LEN, > GFP_KERNEL); > + if (!name) > + return -ENOMEM; > + > + /* This will display like as GPIO-1x */ > + sprintf(name, "%s-%dx", FINTEK_GPIO_DISPLAY, gpio_idx); > + > + gc->chip.owner = THIS_MODULE; > + gc->chip.label = name; > + gc->chip.ngpio = 8; > + gc->chip.dev = &pdev->dev; > + gc->chip.get = f81504_gpio_get; > + gc->chip.set = f81504_gpio_set; > + gc->chip.direction_input = f81504_gpio_direction_in; > + gc->chip.direction_output = f81504_gpio_direction_out; > + gc->chip.get_direction = f81504_gpio_get_direction; > + gc->chip.can_sleep = 1; > + gc->chip.base = -1; > + gc->idx = gpio_idx; > + > > + status = gpiochip_add(&gc->chip); > + if (status) { > + dev_err(&pdev->dev, "%s: gpiochip_add failed: %d\n", > __func__, > + status); > + return -ENOMEM; You ignored the status. > + } > + > + return 0; Perhaps just return gpiochip_add(); ? > +} > + > +static int f81504_gpio_remove(struct platform_device *pdev) > +{ > + struct f81504_gpio_chip *gc = platform_get_drvdata(pdev); > + > + gpiochip_remove(&gc->chip); > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(f81504_gpio_pm_ops, f81504_gpio_suspend, > + f81504_gpio_resume); > + > +static struct platform_driver f81504_gpio_driver = { > + .driver = { > + .name = F81504_GPIO_NAME, > + .owner = THIS_MODULE, > + .pm     = &f81504_gpio_pm_ops, > + }, > + .probe = f81504_gpio_probe, > + .remove = f81504_gpio_remove, > +}; > + > +module_platform_driver(f81504_gpio_driver); > + > +MODULE_AUTHOR("Peter Hong "); > +MODULE_DESCRIPTION("Fintek F81504/508/512 PCIE GPIOLIB driver"); > +MODULE_LICENSE("GPL"); -- Andy Shevchenko Intel Finland Oy