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

[Andres Salomon]

On Sun, 23 Aug 2009 11:22:14 +0200
Alessandro Zummo <alessandro.zummo@xxxxxxxxxxxx> wrote:

> On Tue, 18 Aug 2009 17:53:14 -0400
> Andres Salomon <dilinger@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> 
>  fwiw,
> 
>  Reviewed-by: Alessandro Zummo <a.zummo@xxxxxxxxxxxx>
> 

Thanks!

> > +	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 ?
> 

I prefer to see it when the driver is loaded (as info).

> > 
> > +	/* 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.
> 

Hm, you're right.   Andrew, can you please add/merge the patch below?


> > +		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 .
> 
> 

Noted and changed (along w/ a few other printks).  Thanks!



From: Andres Salomon <dilinger@xxxxxxxxxxxxxxx>
Subject: [PATCH] cs5535-gpio: make printk usage consistent

Clean up places where we were using printk()s instead of dev_*.

Signed-off-by: Andres Salomon <dilinger@xxxxxxxxxxxxxxx>
---
 drivers/gpio/cs5535-gpio.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/cs5535-gpio.c b/drivers/gpio/cs5535-gpio.c
index ec1f0e8..0fdbe94 100644
--- a/drivers/gpio/cs5535-gpio.c
+++ b/drivers/gpio/cs5535-gpio.c
@@ -134,8 +134,8 @@ static int chip_gpio_request(struct gpio_chip *c, unsigned offset)
 
 	/* check if this pin is available */
 	if ((mask & (1 << offset)) == 0) {
-		printk(KERN_INFO DRV_NAME
-			": pin %u is not available (check mask)\n", offset);
+		dev_info(&chip->pdev->dev,
+			"pin %u is not available (check mask)\n", offset);
 		spin_unlock_irqrestore(&chip->lock, flags);
 		return -EINVAL;
 	}
@@ -265,18 +265,15 @@ static int __init cs5535_gpio_probe(struct pci_dev *pdev,
 	mask &= ~(1 << 28);
 
 	if (mask_orig != mask)
-		printk(KERN_INFO DRV_NAME
-				": mask changed from 0x%08lX to 0x%08lX\n",
+		dev_info(&pdev->dev, "mask changed from 0x%08lX to 0x%08lX\n",
 				mask_orig, mask);
 
 	/* 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");
+	if (err)
 		goto release_region;
-	}
 
-	printk(KERN_INFO DRV_NAME ": GPIO support successfully loaded.\n");
+	dev_info(&pdev->dev, DRV_NAME ": GPIO support successfully loaded.\n");
 	return 0;
 
 release_region:
-- 
1.5.6.5






--
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/