Re: [PATCH 1/3] cs5535-gpio: add AMD CS5535/CS5536 GPIO driver support

[Alessandro Zummo <alessandro.zummo@towertech.it>]

On Tue, 18 Aug 2009 17:53:14 -0400
Andres Salomon <dilinger@collabora.co.uk> wrote:

> 
> This creates a CS5535/CS5536 GPIO driver which uses a gpio_chip backend
> (allowing GPIO users to use the generic GPIO API if desired) while also
> allowing architecture-specific users directly (via the cs5535_gpio_*
> functions).
> 
> Tested on an OLPC machine.  Some Leemotes also use CS5536 (with a mips
> cpu), which is why this is in drivers/gpio rather than arch/x86.
> Currently, it conflicts with older geode GPIO support; once MFGPT support
> is reworked to also be more generic, the older geode code will be removed.
> 
> Signed-off-by: Andres Salomon <dilinger@collabora.co.uk>

 fwiw,

 Reviewed-by: Alessandro Zummo <a.zummo@towertech.it>

> +	dev_info(&pdev->dev, "allocated PCI BAR #%d: base 0x%llx\n", GPIO_BAR,
> +			(unsigned long long) cs5535_gpio_chip.base);

 shouldn't this be dev_dbg ?

> 
> +	/* finally, register with the generic GPIO API */
> +	err = gpiochip_add(&cs5535_gpio_chip.chip);
> +	if (err) {
> +		dev_err(&pdev->dev, "failed to register gpio chip\n");

 gpiochip_add already uses pr_err for error conditions, there's no need
 to report twice.

> +		goto release_region;
> +	}
> +
> +	printk(KERN_INFO DRV_NAME ": GPIO support successfully loaded.\n");

 I'd use dev_xxx here, maybe it's worth to have a generic one
 in gpiochip_add .


-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it