Message ID | 20151001015832.GA11317@sophia |
---|---|
State | New |
Headers | show |
Hi William,
[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
config: microblaze-allmodconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=microblaze
All error/warnings (new ones prefixed by >>):
ERROR: "isa_io_base" [drivers/net/wan/lmc/lmc.ko] undefined!
ERROR: "isa_io_base" [drivers/net/wan/farsync.ko] undefined!
ERROR: "isa_io_base" [drivers/net/irda/vlsi_ir.ko] undefined!
ERROR: "isa_io_base" [drivers/net/irda/donauboe.ko] undefined!
ERROR: "isa_io_base" [drivers/net/hamradio/yam.ko] undefined!
ERROR: "isa_io_base" [drivers/net/hamradio/baycom_ser_hdx.ko] undefined!
ERROR: "isa_io_base" [drivers/net/hamradio/baycom_ser_fdx.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/via/via-rhine.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/ti/tlan.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/sis/sis900.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/sis/sis190.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/intel/e1000/e1000.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/hp/hp100.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/dec/tulip/de4x5.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/amd/pcnet32.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/8390/ne2k-pci.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/8390/8390.ko] undefined!
ERROR: "isa_io_base" [drivers/net/ethernet/3com/3c59x.ko] undefined!
ERROR: "isa_io_base" [drivers/net/can/sja1000/sja1000_isa.ko] undefined!
ERROR: "isa_io_base" [drivers/net/can/cc770/cc770_isa.ko] undefined!
ERROR: "isa_io_base" [drivers/net/arcnet/com90xx.ko] undefined!
ERROR: "isa_io_base" [drivers/net/arcnet/com90io.ko] undefined!
ERROR: "isa_io_base" [drivers/net/arcnet/com20020.ko] undefined!
ERROR: "isa_io_base" [drivers/net/arcnet/com20020-pci.ko] undefined!
ERROR: "isa_io_base" [drivers/misc/altera-stapl/altera-stapl.ko] undefined!
ERROR: "isa_io_base" [drivers/message/fusion/mptbase.ko] undefined!
ERROR: "isa_io_base" [drivers/media/radio/radio-maxiradio.ko] undefined!
ERROR: "isa_io_base" [drivers/media/pci/dm1105/dm1105.ko] undefined!
ERROR: "isa_io_base" [drivers/leds/leds-ot200.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hysdn/hysdn.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hisax/hisax_fcpcipnp.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hisax/hisax.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hisax/hfc4s8s_l1.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/mISDN/w6692.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/mISDN/speedfax.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/mISDN/netjet.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/mISDN/mISDNinfineon.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/mISDN/hfcmulti.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/mISDN/avmfritz.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/eicon/divas.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/avm/b1pci.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/avm/b1dma.ko] undefined!
ERROR: "isa_io_base" [drivers/isdn/hardware/avm/b1.ko] undefined!
ERROR: "isa_io_base" [drivers/input/touchscreen/mk712.ko] undefined!
ERROR: "isa_io_base" [drivers/input/serio/pcips2.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/tmdc.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/sidewinder.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/joydump.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/interact.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/guillemot.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/grip_mp.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/grip.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/gf2k.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/cobra.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/analog.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/adi.ko] undefined!
ERROR: "isa_io_base" [drivers/input/joystick/a3d.ko] undefined!
ERROR: "isa_io_base" [drivers/input/gameport/ns558.ko] undefined!
ERROR: "isa_io_base" [drivers/input/gameport/lightning.ko] undefined!
ERROR: "isa_io_base" [drivers/input/gameport/gameport.ko] undefined!
ERROR: "isa_io_base" [drivers/input/gameport/fm801-gp.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-viapro.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-via.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-sis96x.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-sis630.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-sis5595.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-piix4.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-parport-light.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-nforce2.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-isch.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-i801.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-amd8111.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-amd756.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-ali15x3.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-ali1563.ko] undefined!
ERROR: "isa_io_base" [drivers/i2c/busses/i2c-ali1535.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/w83627hf.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/w83627ehf.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/vt8231.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/vt1211.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/via686a.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/smsc47m1.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/smsc47b397.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/sis5595.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/sch56xx-common.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/pc87427.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/pc87360.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/nct6775.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/nct6683.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/it87.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/f71882fg.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/f71805f.ko] undefined!
ERROR: "isa_io_base" [drivers/hwmon/dme1737.ko] undefined!
ERROR: "isa_io_base" [drivers/gpu/drm/qxl/qxl.ko] undefined!
ERROR: "isa_io_base" [drivers/gpu/drm/cirrus/cirrus.ko] undefined!
ERROR: "isa_io_base" [drivers/gpu/drm/bochs/bochs-drm.ko] undefined!
ERROR: "isa_io_base" [drivers/gpio/gpio-vx855.ko] undefined!
ERROR: "isa_io_base" [drivers/gpio/gpio-ts5500.ko] undefined!
ERROR: "isa_io_base" [drivers/gpio/gpio-sch311x.ko] undefined!
ERROR: "isa_io_base" [drivers/gpio/gpio-amd8111.ko] undefined!
>> ERROR: "isa_io_base" [drivers/gpio/gpio-104-idio-16.ko] undefined!
ERROR: "isa_io_base" [drivers/char/tpm/tpm_atmel.ko] undefined!
ERROR: "isa_io_base" [drivers/char/ipmi/ipmi_si.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/on26.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/on20.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/ktti.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/kbic.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/frpw.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/friq.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/fit3.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/fit2.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/epia.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/epat.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/dstr.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/comm.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/bpck6.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/bpck.ko] undefined!
ERROR: "isa_io_base" [drivers/block/paride/aten.ko] undefined!
ERROR: "isa_io_base" [drivers/atm/zatm.ko] undefined!
ERROR: "isa_io_base" [drivers/atm/horizon.ko] undefined!
ERROR: "isa_io_base" [drivers/atm/ambassador.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_sc1200.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_pdc202xx_old.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_optidma.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_legacy.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_hpt3x2n.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_hpt37x.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_cypress.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_cmd64x.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/pata_artop.ko] undefined!
ERROR: "isa_io_base" [drivers/ata/libata.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Oct 1, 2015 at 3:58 AM, William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > The ACCES 104-IDIO-16 family of PC/104 utility boards Sounds like a PC104 keyboard :D > feature 16 > optically isolated inputs and 16 optically isolated FET solid state > outputs. This driver provides GPIO support for these 32 channels of > digital I/O. Change-of-State detection interrupts are not supported. So it has IRQ support but it's not supported yet I take it. > GPIO 0-15 correspond to digital outputs 0-15, while GPIO 16-31 > correspond to digital inputs 0-15. > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > +menu "ISA GPIO expanders" Interesting submenu. I guess it is proper to have so OK. Add: depends on PCI As they all will need that, I guess? > +menuconfig GPIO_104_IDIO_16 > + tristate "ACCES 104-IDIO-16 GPIO support" > + help > + Enables GPIO support for the ACCES 104-IDIO-16 family. > + > +config 104_IDIO_16_BASE > + hex "ACCES 104-IDIO-16 base address" > + depends on GPIO_104_IDIO_16 > + default 0x000 This can't be right. PCI devices have their config space for a reason I'm told. On other platforms we use device tree or ACPI to set this up but PCI is either hotplug or wrong I think. The driver is full of ISA style inb/outb stuff, I get all confused. Why is this not using the PCI infrastructure? Involving Björn Helgås for a comment on this. > +#include <linux/gpio.h> Only #include <linux/gpio/driver.h> > +#include <linux/io.h> > +#include <linux/ioport.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/printk.h> > +#include <linux/spinlock.h> > + > +struct a_104_idio_16_gpio { > + struct gpio_chip chip; > + spinlock_t lock; > + unsigned base; Isn't this void __iomem *base? > + unsigned data; > +}; kerneldoc this. > +static void __exit a_104_idio_16_exit(void); > +static int a_104_idio_16_gpio_direction_input(struct gpio_chip *chip, > + unsigned offset); > +static int a_104_idio_16_gpio_direction_output(struct gpio_chip *chip, > + unsigned offset, int value); > +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset); > +static void a_104_idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, > + int value); > +static int __init a_104_idio_16_init(void); Re-arrange code to avoid forward declarations please. > +static const unsigned A_104_IDIO_16_EXTENT = 8; Looks like it could be a #define A_104_IDIO_16_EXTENT 8 > +static struct a_104_idio_16_gpio gp = { > + .chip = { > + .label = "104-IDIO-16 GPIO", > + .owner = THIS_MODULE, > + .base = -1, > + .ngpio = 32, > + .direction_input = a_104_idio_16_gpio_direction_input, > + .direction_output = a_104_idio_16_gpio_direction_output, > + .get = a_104_idio_16_gpio_get, > + .set = a_104_idio_16_gpio_set > + }, > + .base = CONFIG_104_IDIO_16_BASE > +}; So if you put this *below* the functions you need not forward-declare them. > +static void __exit a_104_idio_16_exit(void) > +{ > + pr_info("104-idio-16: Exiting 104-idio-16 module\n"); > + > + gpiochip_remove(&gp.chip); Where is that &gp.chip? Not in this file. Nor should you use any globals. Does this driver even compile? > +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset) > +{ > + struct a_104_idio_16_gpio *a104i16gp = to_a104i16gp(chip); > + const unsigned BIT_MASK = 1U << (offset-16); > + > + if (offset < 16) > + return 0; Always return 0, why? Is that really correct? > +static int __init a_104_idio_16_init(void) > + spin_lock_init(&gp.lock); > + err = gpiochip_add(&gp.chip); This gp global again. Read Documentation/driver-model/design-patterns.txt Please. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi William and Linus, On Mon, Oct 5, 2015 at 3:29 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Oct 1, 2015 at 3:58 AM, William Breathitt Gray > <vilhelm.gray@gmail.com> wrote: > >> The ACCES 104-IDIO-16 family of PC/104 utility boards > > Sounds like a PC104 keyboard :D > >> feature 16 >> optically isolated inputs and 16 optically isolated FET solid state >> outputs. This driver provides GPIO support for these 32 channels of >> digital I/O. Change-of-State detection interrupts are not supported. > > So it has IRQ support but it's not supported yet I take it. > >> GPIO 0-15 correspond to digital outputs 0-15, while GPIO 16-31 >> correspond to digital inputs 0-15. >> >> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > >> +menu "ISA GPIO expanders" > > Interesting submenu. I guess it is proper to have so OK. > > Add: > depends on PCI > > As they all will need that, I guess? Why do they need PCI? I don't see any PCI interfaces used here. >> +menuconfig GPIO_104_IDIO_16 >> + tristate "ACCES 104-IDIO-16 GPIO support" >> + help >> + Enables GPIO support for the ACCES 104-IDIO-16 family. >> + >> +config 104_IDIO_16_BASE >> + hex "ACCES 104-IDIO-16 base address" >> + depends on GPIO_104_IDIO_16 >> + default 0x000 > > This can't be right. PCI devices have their config space for a reason > I'm told. On other platforms we use device tree or ACPI to set this > up but PCI is either hotplug or wrong I think. > > The driver is full of ISA style inb/outb stuff, I get all confused. Why > is this not using the PCI infrastructure? I'm really not familiar with PC/104, but wikipedia claims it only uses ISA. Apparently PCI/104-Plus, PCI-104, PCI/104-Express, and PCIe/104 *do* have PCI and/or PCIe, but it looks like the base PC/104 does not. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/05/2015 06:47 PM, Bjorn Helgaas wrote: > I'm really not familiar with PC/104, but wikipedia claims it only uses > ISA. Apparently PCI/104-Plus, PCI-104, PCI/104-Express, and PCIe/104 > *do* have PCI and/or PCIe, but it looks like the base PC/104 does not. > > Bjorn > This is correct: PC/104 is only ISA. The ACCES 104-IDIO-16 board is a PC/104 stackable GPIO board, whose port address is configured via physical jumper switches on the board; it is not "hotpluggable." For this reason, the CONFIG_104_IDIO_16_BASE option should be set to the same port address physically configured on the board. - William -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/05/2015 04:29 AM, Linus Walleij wrote: >> +struct a_104_idio_16_gpio { >> + struct gpio_chip chip; >> + spinlock_t lock; >> + unsigned base; > > Isn't this void __iomem *base? The 'base' member is used to hold the I/O port base address passed to the inb/outb functions for port-mapped I/O operations. Since the addresses are not dereferenced, I don't believe an __iomem pointer would be correct. >> +static const unsigned A_104_IDIO_16_EXTENT = 8; > > Looks like it could be a #define A_104_IDIO_16_EXTENT 8 I used a const variable for the benefit of type-safety; I assumed the compiler would optimize it. What is the advantage of a #define constant? >> +static void __exit a_104_idio_16_exit(void) >> +{ >> + pr_info("104-idio-16: Exiting 104-idio-16 module\n"); >> + >> + gpiochip_remove(&gp.chip); > > Where is that &gp.chip? Not in this file. Nor should you use any globals. > I agree that using a global data structure isn't good practice, but I'm not sure how else to expose the gpio_chip structure in the respective module _init and _exit functions since they have void parameter lists. Would it be more appropriate to use the platform device API in this situation to avoid the global data structure? >> +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset) >> +{ >> + struct a_104_idio_16_gpio *a104i16gp = to_a104i16gp(chip); >> + const unsigned BIT_MASK = 1U << (offset-16); >> + >> + if (offset < 16) >> + return 0; > > Always return 0, why? Is that really correct? GPIO 0-15 are output-only. The kerneldoc for 'struct gpio_chip' states that for output signals the get function should return the value actually sensed, or zero. Since I cannot sense the output signals, I return zero in these cases. Is this behavior correct? - William -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 6, 2015 at 1:11 AM, William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > On 10/05/2015 06:47 PM, Bjorn Helgaas wrote: >> I'm really not familiar with PC/104, but wikipedia claims it only uses >> ISA. Apparently PCI/104-Plus, PCI-104, PCI/104-Express, and PCIe/104 >> *do* have PCI and/or PCIe, but it looks like the base PC/104 does not. > > This is correct: PC/104 is only ISA. OK then let's drop that confusing "PCI GPIO expanders" menu. Maybe "ISA GPIO cards" or similar is more apropriate. > The ACCES 104-IDIO-16 board is a > PC/104 stackable GPIO board, whose port address is configured via > physical jumper switches on the board; it is not "hotpluggable." For > this reason, the CONFIG_104_IDIO_16_BASE option should be set to the > same port address physically configured on the board. I think the pattern in the kernel is to support such configuration for ISA is with module parameters, not with compile-time constants. So switch that base to use a module parameter. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 6, 2015 at 1:40 AM, William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > On 10/05/2015 04:29 AM, Linus Walleij wrote: >>> +struct a_104_idio_16_gpio { >>> + struct gpio_chip chip; >>> + spinlock_t lock; >>> + unsigned base; >> >> Isn't this void __iomem *base? > > The 'base' member is used to hold the I/O port base address passed to the > inb/outb functions for port-mapped I/O operations. Since the addresses are > not dereferenced, I don't believe an __iomem pointer would be correct. You're right, sorry I was confused. >>> +static const unsigned A_104_IDIO_16_EXTENT = 8; >> >> Looks like it could be a #define A_104_IDIO_16_EXTENT 8 > > I used a const variable for the benefit of type-safety; I assumed the > compiler would optimize it. What is the advantage of a #define constant? Usually we use #define's for compile-time constants. >>> +static void __exit a_104_idio_16_exit(void) >>> +{ >>> + pr_info("104-idio-16: Exiting 104-idio-16 module\n"); >>> + >>> + gpiochip_remove(&gp.chip); >> >> Where is that &gp.chip? Not in this file. Nor should you use any globals. > > I agree that using a global data structure isn't good practice, but I'm not > sure how else to expose the gpio_chip structure in the respective module > _init and _exit functions since they have void parameter lists. Would it be > more appropriate to use the platform device API in this situation to avoid > the global data structure? It depends where your device is spawn. If there is a natural place to instantiate the platform device like from a MFD or PCI driver or something then yes. >>> +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset) >>> +{ >>> + struct a_104_idio_16_gpio *a104i16gp = to_a104i16gp(chip); >>> + const unsigned BIT_MASK = 1U << (offset-16); >>> + >>> + if (offset < 16) >>> + return 0; >> >> Always return 0, why? Is that really correct? > > GPIO 0-15 are output-only. The kerneldoc for 'struct gpio_chip' states that > for output signals the get function should return the value actually sensed, > or zero. Since I cannot sense the output signals, I return zero in these cases. > Is this behavior correct? I guess, we recently augmented the gpiolib core so you can actually return an error code as -ENODEV here. No strong preference though. Maybe I should have... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 8949b3f..6309071 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -684,6 +684,20 @@ config GPIO_SX150X endmenu +menu "ISA GPIO expanders" + +menuconfig GPIO_104_IDIO_16 + tristate "ACCES 104-IDIO-16 GPIO support" + help + Enables GPIO support for the ACCES 104-IDIO-16 family. + +config 104_IDIO_16_BASE + hex "ACCES 104-IDIO-16 base address" + depends on GPIO_104_IDIO_16 + default 0x000 + +endmenu + menu "MFD GPIO expanders" config GPIO_ADP5520 diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index f79a7c4..6f2fea5 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_GPIO_ACPI) += gpiolib-acpi.o # Device drivers. Generally keep list sorted alphabetically obj-$(CONFIG_GPIO_GENERIC) += gpio-generic.o +obj-$(CONFIG_GPIO_104_IDIO_16) += gpio-104-idio-16.o obj-$(CONFIG_GPIO_74X164) += gpio-74x164.o obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o obj-$(CONFIG_GPIO_ADNP) += gpio-adnp.o diff --git a/drivers/gpio/gpio-104-idio-16.c b/drivers/gpio/gpio-104-idio-16.c new file mode 100644 index 0000000..5e3e89d --- /dev/null +++ b/drivers/gpio/gpio-104-idio-16.c @@ -0,0 +1,154 @@ +/* + * GPIO driver for the ACCES 104-IDIO-16 family + * Copyright (C) 2015 William Breathitt Gray + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ +#include <linux/gpio.h> +#include <linux/io.h> +#include <linux/ioport.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/printk.h> +#include <linux/spinlock.h> + +struct a_104_idio_16_gpio { + struct gpio_chip chip; + spinlock_t lock; + unsigned base; + unsigned data; +}; + +#define to_a104i16gp(chip) container_of(chip, struct a_104_idio_16_gpio, chip) + +static void __exit a_104_idio_16_exit(void); +static int a_104_idio_16_gpio_direction_input(struct gpio_chip *chip, + unsigned offset); +static int a_104_idio_16_gpio_direction_output(struct gpio_chip *chip, + unsigned offset, int value); +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset); +static void a_104_idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, + int value); +static int __init a_104_idio_16_init(void); + +static const unsigned A_104_IDIO_16_EXTENT = 8; + +static struct a_104_idio_16_gpio gp = { + .chip = { + .label = "104-IDIO-16 GPIO", + .owner = THIS_MODULE, + .base = -1, + .ngpio = 32, + .direction_input = a_104_idio_16_gpio_direction_input, + .direction_output = a_104_idio_16_gpio_direction_output, + .get = a_104_idio_16_gpio_get, + .set = a_104_idio_16_gpio_set + }, + .base = CONFIG_104_IDIO_16_BASE +}; + +static void __exit a_104_idio_16_exit(void) +{ + pr_info("104-idio-16: Exiting 104-idio-16 module\n"); + + gpiochip_remove(&gp.chip); + release_region(CONFIG_104_IDIO_16_BASE, A_104_IDIO_16_EXTENT); +} + +static int a_104_idio_16_gpio_direction_input(struct gpio_chip *chip, + unsigned offset) +{ + return 0; +} + +static int a_104_idio_16_gpio_direction_output(struct gpio_chip *chip, + unsigned offset, int value) +{ + chip->set(chip, offset, value); + return 0; +} + +static int a_104_idio_16_gpio_get(struct gpio_chip *chip, unsigned offset) +{ + struct a_104_idio_16_gpio *a104i16gp = to_a104i16gp(chip); + const unsigned BIT_MASK = 1U << (offset-16); + + if (offset < 16) + return 0; + + if (offset < 24) + return !!(inb(a104i16gp->base + 1) & BIT_MASK); + + return !!(inb(a104i16gp->base + 5) & (BIT_MASK>>8)); +} + +static void a_104_idio_16_gpio_set(struct gpio_chip *chip, unsigned offset, + int value) +{ + struct a_104_idio_16_gpio *a104i16gp = to_a104i16gp(chip); + const unsigned BIT_MASK = 1U << offset; + unsigned long flags; + + if (offset > 15) + return; + + spin_lock_irqsave(&a104i16gp->lock, flags); + + if (value) + a104i16gp->data |= BIT_MASK; + else + a104i16gp->data &= ~BIT_MASK; + + if (offset > 7) + outb(~a104i16gp->data >> 8, a104i16gp->base + 4); + else + outb(~a104i16gp->data, a104i16gp->base); + + spin_unlock_irqrestore(&a104i16gp->lock, flags); +} + +static int __init a_104_idio_16_init(void) +{ + int err = 0; + + pr_info("104-idio-16: Initializing 104-idio-16 module\n"); + + if (!request_region(CONFIG_104_IDIO_16_BASE, A_104_IDIO_16_EXTENT, + "104-idio-16")) { + pr_err("104-idio-16: Unable to lock 104-idio-16 port addresses (0x%X-0x%X)\n", + CONFIG_104_IDIO_16_BASE, + CONFIG_104_IDIO_16_BASE + A_104_IDIO_16_EXTENT); + err = -EBUSY; + goto out_lock_a_104_idio_16_port; + } + + spin_lock_init(&gp.lock); + + pr_info("104-idio-16: 104-IDIO-16 GPIO detected\n"); + err = gpiochip_add(&gp.chip); + if (err) { + pr_err("104-idio-16: GPIO registering failed (%d)\n", err); + goto out_gpio_register; + } + + return 0; + +out_gpio_register: + release_region(CONFIG_104_IDIO_16_BASE, A_104_IDIO_16_EXTENT); +out_lock_a_104_idio_16_port: + return err; +} + +module_init(a_104_idio_16_init); +module_exit(a_104_idio_16_exit); + +MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>"); +MODULE_DESCRIPTION("ACCES 104-IDIO-16 GPIO driver"); +MODULE_LICENSE("GPL");
The ACCES 104-IDIO-16 family of PC/104 utility boards feature 16 optically isolated inputs and 16 optically isolated FET solid state outputs. This driver provides GPIO support for these 32 channels of digital I/O. Change-of-State detection interrupts are not supported. GPIO 0-15 correspond to digital outputs 0-15, while GPIO 16-31 correspond to digital inputs 0-15. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/gpio/Kconfig | 14 ++++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-104-idio-16.c | 154 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 drivers/gpio/gpio-104-idio-16.c