linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips
@ 2012-04-27 10:08 Dmitry Eremin-Solenikov
  2012-04-27 10:52 ` Linus Walleij
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Eremin-Solenikov @ 2012-04-27 10:08 UTC (permalink / raw)
  To: Grant Likely, Linus Walleij; +Cc: linux-kernel

Add a driver to use GPIO pins available on several AMD south bridges
(currently only AMD 8111 is supported).

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/gpio/Kconfig        |   12 +++
 drivers/gpio/Makefile       |    1 +
 drivers/gpio/gpio-amd8111.c |  218 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 drivers/gpio/gpio-amd8111.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d0c4118..b05902f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -379,6 +379,18 @@ config GPIO_BT8XX
 
 	  If unsure, say N.
 
+config GPIO_AMD8111
+	tristate "AMD 8111 GPIO driver"
+	depends on PCI
+	help
+	  The AMD 8111 south bridge contains 32 GPIO pins which can be used.
+
+	  Note, that usually system firmware/ACPI handles GPIO pins on their
+	  own and users might easily break their systems with uncarefull usage
+	  of this driver!
+
+	  If unsure, say N
+
 config GPIO_LANGWELL
 	bool "Intel Langwell/Penwell GPIO support"
 	depends on PCI && X86
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index fa10df6..a6d2ac7 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_GPIO_74X164)	+= gpio-74x164.o
 obj-$(CONFIG_GPIO_AB8500)	+= gpio-ab8500.o
 obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
+obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
diff --git a/drivers/gpio/gpio-amd8111.c b/drivers/gpio/gpio-amd8111.c
new file mode 100644
index 0000000..fc4a09e
--- /dev/null
+++ b/drivers/gpio/gpio-amd8111.c
@@ -0,0 +1,218 @@
+/*
+ * GPIO driver for AMD 8111 south bridges
+ *
+ * Copyright (c) 2012 Dmitry Eremin-Solenikov
+ *
+ * Based on the AMD RNG driver:
+ * Copyright 2005 (c) MontaVista Software, Inc.
+ * with the majority of the code coming from:
+ *
+ * Hardware driver for the Intel/AMD/VIA Random Number Generators (RNG)
+ * (c) Copyright 2003 Red Hat Inc <jgarzik@redhat.com>
+ *
+ * derived from
+ *
+ * Hardware driver for the AMD 768 Random Number Generator (RNG)
+ * (c) Copyright 2001 Red Hat Inc
+ *
+ * derived from
+ *
+ * Hardware driver for Intel i810 Random Number Generator (RNG)
+ * Copyright 2000,2001 Jeff Garzik <jgarzik@pobox.com>
+ * Copyright 2000,2001 Philipp Rumpf <prumpf@mandrakesoft.com>
+ *
+ * This file is licensed under  the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/gpio.h>
+#include <linux/pci.h>
+#include <linux/spinlock.h>
+
+#define PMBASE_OFFSET 0xb0
+#define PMBASE_SIZE   0x30
+#define PMBASE_GPIO(i) (0xc0 + (i))
+
+/*
+ * Data for PCI driver interface
+ *
+ * This data only exists for exporting the supported
+ * PCI ids via MODULE_DEVICE_TABLE.  We do not actually
+ * register a pci_driver, because someone else might one day
+ * want to register another driver on the same PCI id.
+ */
+static DEFINE_PCI_DEVICE_TABLE(pci_tbl) = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8111_SMBUS), 0 },
+	{ 0, },	/* terminate list */
+};
+MODULE_DEVICE_TABLE(pci, pci_tbl);
+
+struct amd_gpio {
+	struct gpio_chip	chip;
+	u32			pmbase;
+	void __iomem		*pm;
+	struct pci_dev		*pdev;
+	spinlock_t		lock; /* guards hw registers and orig table */
+	u8			orig[32];
+};
+
+#define to_agp(chip)	container_of(chip, struct amd_gpio, chip)
+
+static int amd_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	struct amd_gpio *agp = to_agp(chip);
+
+	agp->orig[offset] = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+
+	dev_dbg(&agp->pdev->dev, "Requested gpio %d, data %x\n", offset, agp->orig[offset]);
+
+	return 0;
+}
+
+static void amd_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	struct amd_gpio *agp = to_agp(chip);
+
+	dev_dbg(&agp->pdev->dev, "Freed gpio %d, data %x\n", offset, agp->orig[offset]);
+
+	iowrite8(agp->orig[offset], agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+}
+
+static void amd_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct amd_gpio *agp = to_agp(chip);
+	u8 temp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&agp->lock, flags);
+	temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+	temp = (temp & 0x1c) | (!!value);
+	iowrite8(temp, agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+	spin_unlock_irqrestore(&agp->lock, flags);
+
+	dev_dbg(&agp->pdev->dev, "Setting gpio %d, value %d, reg=%02x\n", offset, !!value, temp);
+}
+
+static int amd_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct amd_gpio *agp = to_agp(chip);
+	u8 temp;
+
+	temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+
+	dev_dbg(&agp->pdev->dev, "Getting gpio %d, reg=%02x\n", offset, temp);
+
+	return !!(temp & 0x20);
+}
+
+static int amd_gpio_dirout(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct amd_gpio *agp = to_agp(chip);
+	u8 temp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&agp->lock, flags);
+	temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+	temp = (temp & 0x10) | 0x4 | (!!value);
+	iowrite8(temp, agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+	spin_unlock_irqrestore(&agp->lock, flags);
+
+	dev_dbg(&agp->pdev->dev, "Dirout gpio %d, value %d, reg=%02x\n", offset, !!value, temp);
+
+	return 0;
+}
+
+static int amd_gpio_dirin(struct gpio_chip *chip, unsigned offset)
+{
+	struct amd_gpio *agp = to_agp(chip);
+	u8 temp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&agp->lock, flags);
+	temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+	temp = (temp & 0x10);
+	iowrite8(temp, agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
+	spin_unlock_irqrestore(&agp->lock, flags);
+
+	dev_dbg(&agp->pdev->dev, "Dirin gpio %d, reg=%02x\n", offset, temp);
+
+	return 0;
+}
+
+static struct amd_gpio gp = {
+	.chip = {
+		.label		= "AMD GPIO",
+		.owner		= THIS_MODULE,
+		.base		= -1,
+		.ngpio		= 32,
+		.request	= amd_gpio_request,
+		.free		= amd_gpio_free,
+		.set		= amd_gpio_set,
+		.get		= amd_gpio_get,
+		.direction_output = amd_gpio_dirout,
+		.direction_input = amd_gpio_dirin,
+	},
+};
+
+static int __init mod_init(void)
+{
+	int err = -ENODEV;
+	struct pci_dev *pdev = NULL;
+	const struct pci_device_id *ent;
+
+	for_each_pci_dev(pdev) {
+		ent = pci_match_id(pci_tbl, pdev);
+		if (ent)
+			goto found;
+	}
+	/* Device not found. */
+	goto out;
+
+found:
+	err = pci_read_config_dword(pdev, 0x58, &gp.pmbase);
+	if (err)
+		goto out;
+	err = -EIO;
+	gp.pmbase &= 0x0000FF00;
+	if (gp.pmbase == 0)
+		goto out;
+	if (!request_region(gp.pmbase + PMBASE_OFFSET, PMBASE_SIZE, "AMD GPIO")) {
+		dev_err(&pdev->dev, "AMD GPIO region 0x%x already in use!\n",
+			gp.pmbase + PMBASE_OFFSET);
+		err = -EBUSY;
+		goto out;
+	}
+	gp.pm = ioport_map(gp.pmbase + PMBASE_OFFSET, PMBASE_SIZE);
+	gp.pdev = pdev;
+	gp.chip.dev = &pdev->dev;
+
+	spin_lock_init(&gp.lock);
+
+	printk(KERN_INFO "AMD-8111 GPIO detected\n");
+	err = gpiochip_add(&gp.chip);
+	if (err) {
+		printk(KERN_ERR "GPIO registering failed (%d)\n",
+		       err);
+		release_region(gp.pmbase + PMBASE_OFFSET, PMBASE_SIZE);
+		goto out;
+	}
+out:
+	return err;
+}
+
+static void __exit mod_exit(void)
+{
+	int err = gpiochip_remove(&gp.chip);
+	WARN_ON(err);
+	ioport_unmap(gp.pm);
+	release_region(gp.pmbase + PMBASE_OFFSET, PMBASE_SIZE);
+}
+
+module_init(mod_init);
+module_exit(mod_exit);
+
+MODULE_AUTHOR("The Linux Kernel team");
+MODULE_DESCRIPTION("GPIO driver for AMD chipsets");
+MODULE_LICENSE("GPL");
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips
  2012-04-27 10:08 [PATCH] gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips Dmitry Eremin-Solenikov
@ 2012-04-27 10:52 ` Linus Walleij
  2012-05-18 22:35   ` Grant Likely
  0 siblings, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2012-04-27 10:52 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov; +Cc: Grant Likely, Linus Walleij, linux-kernel

On Fri, Apr 27, 2012 at 12:08 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> Add a driver to use GPIO pins available on several AMD south bridges
> (currently only AMD 8111 is supported).
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Mainly nitpicks, it's looking good overall...

> +       spin_lock_irqsave(&agp->lock, flags);
> +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> +       temp = (temp & 0x1c) | (!!value);

This is a bit idomatic but I think I understand what's going on. However this
0x1C magic could be better of with some #define AMD_8111_GPIO_FOO don't
you think?

> +static int amd_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct amd_gpio *agp = to_agp(chip);
> +       u8 temp;
> +
> +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> +
> +       dev_dbg(&agp->pdev->dev, "Getting gpio %d, reg=%02x\n", offset, temp);
> +
> +       return !!(temp & 0x20);

And this could be #define AMD_8111_GPIO_INVAL or so (I bet this bit has a
name in the datasheet, right?)

> +static int amd_gpio_dirout(struct gpio_chip *chip, unsigned offset, int value)
> +{
> +       struct amd_gpio *agp = to_agp(chip);
> +       u8 temp;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&agp->lock, flags);
> +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> +       temp = (temp & 0x10) | 0x4 | (!!value);

And so on...

> +       iowrite8(temp, agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> +       spin_unlock_irqrestore(&agp->lock, flags);
> +
> +       dev_dbg(&agp->pdev->dev, "Dirout gpio %d, value %d, reg=%02x\n", offset, !!value, temp);
> +
> +       return 0;
> +}

> +static int __init mod_init(void)

amd_gpio_init() maybe? These names are nice for e.g. ftrace.

> +       printk(KERN_INFO "AMD-8111 GPIO detected\n");

pr_info() & friends are nice shorthands for this.

> +static void __exit mod_exit(void)

amd_gpio_mod_exit()?

> +MODULE_AUTHOR("The Linux Kernel team");

Don't be shy, put in your own name :-)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips
  2012-04-27 10:52 ` Linus Walleij
@ 2012-05-18 22:35   ` Grant Likely
  2012-05-18 22:43     ` Dmitry Eremin-Solenikov
  0 siblings, 1 reply; 4+ messages in thread
From: Grant Likely @ 2012-05-18 22:35 UTC (permalink / raw)
  To: Linus Walleij, Dmitry Eremin-Solenikov; +Cc: Linus Walleij, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]

On Fri, 27 Apr 2012 12:52:19 +0200, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Apr 27, 2012 at 12:08 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
> 
> > Add a driver to use GPIO pins available on several AMD south bridges
> > (currently only AMD 8111 is supported).
> >
> > Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> 
> Mainly nitpicks, it's looking good overall...

Hi Dimtry,

Do you have an updated version of this patch to address Linus' comments?

g.

> 
> > +       spin_lock_irqsave(&agp->lock, flags);
> > +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> > +       temp = (temp & 0x1c) | (!!value);
> 
> This is a bit idomatic but I think I understand what's going on. However this
> 0x1C magic could be better of with some #define AMD_8111_GPIO_FOO don't
> you think?
> 
> > +static int amd_gpio_get(struct gpio_chip *chip, unsigned offset)
> > +{
> > +       struct amd_gpio *agp = to_agp(chip);
> > +       u8 temp;
> > +
> > +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> > +
> > +       dev_dbg(&agp->pdev->dev, "Getting gpio %d, reg=%02x\n", offset, temp);
> > +
> > +       return !!(temp & 0x20);
> 
> And this could be #define AMD_8111_GPIO_INVAL or so (I bet this bit has a
> name in the datasheet, right?)
> 
> > +static int amd_gpio_dirout(struct gpio_chip *chip, unsigned offset, int value)
> > +{
> > +       struct amd_gpio *agp = to_agp(chip);
> > +       u8 temp;
> > +       unsigned long flags;
> > +
> > +       spin_lock_irqsave(&agp->lock, flags);
> > +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> > +       temp = (temp & 0x10) | 0x4 | (!!value);
> 
> And so on...
> 
> > +       iowrite8(temp, agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
> > +       spin_unlock_irqrestore(&agp->lock, flags);
> > +
> > +       dev_dbg(&agp->pdev->dev, "Dirout gpio %d, value %d, reg=%02x\n", offset, !!value, temp);
> > +
> > +       return 0;
> > +}
> 
> > +static int __init mod_init(void)
> 
> amd_gpio_init() maybe? These names are nice for e.g. ftrace.
> 
> > +       printk(KERN_INFO "AMD-8111 GPIO detected\n");
> 
> pr_info() & friends are nice shorthands for this.
> 
> > +static void __exit mod_exit(void)
> 
> amd_gpio_mod_exit()?
> 
> > +MODULE_AUTHOR("The Linux Kernel team");
> 
> Don't be shy, put in your own name :-)
> 
> Yours,
> Linus Walleij

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips
  2012-05-18 22:35   ` Grant Likely
@ 2012-05-18 22:43     ` Dmitry Eremin-Solenikov
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Eremin-Solenikov @ 2012-05-18 22:43 UTC (permalink / raw)
  To: Grant Likely; +Cc: Linus Walleij, Linus Walleij, linux-kernel

On Sat, May 19, 2012 at 2:35 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, 27 Apr 2012 12:52:19 +0200, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Fri, Apr 27, 2012 at 12:08 PM, Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com> wrote:
>>
>> > Add a driver to use GPIO pins available on several AMD south bridges
>> > (currently only AMD 8111 is supported).
>> >
>> > Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>
>> Mainly nitpicks, it's looking good overall...
>
> Hi Dimtry,
>
> Do you have an updated version of this patch to address Linus' comments?

Colleagues,

I was a little bit busy with job & private topics. I plan to post
updated version
either during the weekend, or on Mon-Wed.

>
> g.
>
>>
>> > +       spin_lock_irqsave(&agp->lock, flags);
>> > +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
>> > +       temp = (temp & 0x1c) | (!!value);
>>
>> This is a bit idomatic but I think I understand what's going on. However this
>> 0x1C magic could be better of with some #define AMD_8111_GPIO_FOO don't
>> you think?
>>
>> > +static int amd_gpio_get(struct gpio_chip *chip, unsigned offset)
>> > +{
>> > +       struct amd_gpio *agp = to_agp(chip);
>> > +       u8 temp;
>> > +
>> > +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
>> > +
>> > +       dev_dbg(&agp->pdev->dev, "Getting gpio %d, reg=%02x\n", offset, temp);
>> > +
>> > +       return !!(temp & 0x20);
>>
>> And this could be #define AMD_8111_GPIO_INVAL or so (I bet this bit has a
>> name in the datasheet, right?)
>>
>> > +static int amd_gpio_dirout(struct gpio_chip *chip, unsigned offset, int value)
>> > +{
>> > +       struct amd_gpio *agp = to_agp(chip);
>> > +       u8 temp;
>> > +       unsigned long flags;
>> > +
>> > +       spin_lock_irqsave(&agp->lock, flags);
>> > +       temp = ioread8(agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
>> > +       temp = (temp & 0x10) | 0x4 | (!!value);
>>
>> And so on...
>>
>> > +       iowrite8(temp, agp->pm - PMBASE_OFFSET + PMBASE_GPIO(offset));
>> > +       spin_unlock_irqrestore(&agp->lock, flags);
>> > +
>> > +       dev_dbg(&agp->pdev->dev, "Dirout gpio %d, value %d, reg=%02x\n", offset, !!value, temp);
>> > +
>> > +       return 0;
>> > +}
>>
>> > +static int __init mod_init(void)
>>
>> amd_gpio_init() maybe? These names are nice for e.g. ftrace.
>>
>> > +       printk(KERN_INFO "AMD-8111 GPIO detected\n");
>>
>> pr_info() & friends are nice shorthands for this.
>>
>> > +static void __exit mod_exit(void)
>>
>> amd_gpio_mod_exit()?
>>
>> > +MODULE_AUTHOR("The Linux Kernel team");
>>
>> Don't be shy, put in your own name :-)
>>
>> Yours,
>> Linus Walleij
>
> --
> Grant Likely, B.Sc, P.Eng.
> Secret Lab Technologies, Ltd.



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-05-18 22:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-27 10:08 [PATCH] gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips Dmitry Eremin-Solenikov
2012-04-27 10:52 ` Linus Walleij
2012-05-18 22:35   ` Grant Likely
2012-05-18 22:43     ` Dmitry Eremin-Solenikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).