Messages in this thread | | | Subject | Re: [PATCH V2 2/4] gpio: gpio-f81504: Add Fintek F81504/508/512 PCIE-to-UART/GPIO GPIOLIB support | From | Peter Hung <> | Date | Fri, 29 Jan 2016 16:15:49 +0800 |
| |
Hi Andy,
Andy Shevchenko 於 2016/1/28 下午 08:03 寫道: > On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote: >> + /* 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.
I had only tested on x86 environment currently. We'll try to get an ARM platform with PCIE to test it. We'll remain it until getting new board.
>> +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?
Sorry for the big mistake, I'd confused for dev_get_platdata() & platform_set_drvdata(). I'll rewrite this and check 8250_f81504.c too.
>> + 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(); ?
Just return gpiochip_add() seems good.
Thanks your advices -- With Best Regards, Peter Hung
| |