gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips
PATCH v2 1/1 gpio: add a driver for GPIO pins found on AMD-8111 south bridge chips
Add a driver to use GPIO pins available on several AMD south bridges (currently only AMD 8111 is supported).
---
Changes since V1:
* Replaced magic numbers in register access with named values
drivers/gpio/Kconfig | 12 +++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-amd8111.c | 235 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 248 insertions(+) create mode 100644 drivers/gpio/gpio-amd8111.c
drivers/gpio/Kconfig +12 -0
@@ -396,6 +396,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
drivers/gpio/Makefile +1 -0
@@ -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
drivers/gpio/gpio-amd8111.c new file +235 -0
235 lines not on this page · raw message
Signature · 1 line
1.7.10
Quoted 8 lines · Dmitry Eremin-Solenikov
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 <[email protected]>---
Changes since V1:
* Replaced magic numbers in register access with named values
This is looking a lot better, some nitpicking:
Quoted 1 line
+#define
AMD_GPIO_MODE_ALTFN0x08 /* Or0x09*/
Hm Hm I wonder what this register does... looks a lot like a mux. (Just philisophizing...)
drivers/gpio/gpio-amd8111.c line 185
Quoted 10 lines
drivers/gpio/gpio-amd8111.c new file +10
-0+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;
It's not like I know how PCI abstractions really work, but what happens here if there would be two instances of the device? It looks like you will only find the first?
Quoted 3 lines
+ }
+ /* Device not found. */
+ goto out;
Can't you just return -ENODEV here? Why jump around...
Now as I said I'm not a PCI expert, but isn't the proper way to do this to use this stuff:
static struct pci_driver amd_gpio_driver = {
.name = "amd-gpio",
.id_table = pci_ids,
.probe = amd_gpio_probe,
.remove = __devexit_p(amd_gpio_remove),};
static int __init amd_gpio_init(void)
{
return pci_register_driver(&amd_gpio_driver);
}static void __exit amd_gpio_exit(void)
{
pci_unregister_driver(&amd_gpio_driver);
}module_init(amd_gpio_init);module_exit(amd_gpio_exit);
Then start working from the probe function insteaf of go and iterate over the PCI bus yourself? I was thinking there was a reason for but couldn't find any.
Yours,
Linus Walleij
Hello,
Quoted 6 lines · Linus Walleij
Quoted 3 lines · Dmitry Eremin-Solenikov
Add a driver to use GPIO pins available on several AMD south bridges (currently only AMD 8111 is supported).
Quoted 7 lines
Quoted 5 lines
Signed-off-by Dmitry Eremin-Solenikov <[email protected]>---
Changes since V1:
* Replaced magic numbers in register access with named valuesThis is looking a lot better, some nitpicking:
Oh, my, I forgot to change module_init/module_exit function names. So anyway I'll have to send V3
Quoted 5 lines
Quoted 2 lines
+#define
AMD_GPIO_MODE_ALTFN0x08 /* Or0x09*/Hm Hm I wonder what this register does... looks a lot like a mux. (Just philisophizing...)
As the name says, it is an "alternate function". Some (most) of these pins can have alternative functions like "clock output", IRQ, SMBus, etc. Setting this ALTFN mode enables special logic for that pin.
Quoted 15 lines
drivers/gpio/gpio-amd8111.c line 185
Quoted 11 lines
drivers/gpio/gpio-amd8111.c new file +10
-0+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;It's not like I know how PCI abstractions really work, but what happens here if there would be two instances of the device? It looks like you will only find the first?
Hmm. I don't know a system with two amd 8111 south bridges in it. This part is mostly a c&p from amd hw_random driver.
Quoted 6 lines
Quoted 4 lines
+ }
+ /* Device not found. */
+ goto out;Can't you just return
-ENODEVhere? Why jump around...
OK.
Quoted 21 lines
Now as I said I'm not a PCI expert, but isn't the proper way to do this to use this stuff:
static struct
pci_driveramd_gpio_driver= { .name = "amd-gpio",
.id_table =pci_ids,
.probe = amd_gpio_probe, .remove =__devexit_p(amd_gpio_remove), };static int
__initamd_gpio_init(void)
{
returnpci_register_driver(&amd_gpio_driver); }static void
__exitamd_gpio_exit(void)
{
pci_unregister_driver(&amd_gpio_driver);
}
Quoted 6 lines
module_init(amd_gpio_init);module_exit(amd_gpio_exit);Then start working from the probe function insteaf of go and iterate over the PCI bus yourself? I was thinking there was a reason for but couldn't find any.
It would be the usual way. There is however a problem with that way. The System Management devices (that the driver looks for):
1) Is a kind of placeholder. If you check, the driver get's the IO address not via usual pci_resource_start()/_end(), which end up with the data from BARs from pci device, but does special register access magic
2) Is a multifunction device. The IO space that is provided by the device is used for HW random generator, I2C (well, SMBus) controller, now the GPIO pins driver. Also it covers some power management/SMI/SCI magic, which usually nobody should touch. So, in the ideal world, we should register an MFD driver over that PCI device and then register all other child sub-devices. However this looks like a
huger overkill
to me. In the end, it's just a GPIO part of south bridge, which should not be touched by normal people :)
Signature · 2 lines
With best wishes Dmitry