On Fri, Aug 16, 2013 at 6:34 PM, Fabian Vogt <fabian@xxxxxxxxxxxxxx> wrote: > On Fri, Aug 16, 2013 at 4:46 PM, Linus Walleij <linus.walleij@xxxxxxxxxx> > wrote: >> I want an ACK from one of the DT bindings maintainers for this >> portion of the driver ideally. (It looks all right to me.) > > Any ideas which mail addresses I should add to CC:? No. devicetree@xxxxxxxxxxxxxxx is the place. But if nothing happens you might want to ask Stephen Warren. >>> +config GPIO_ZEVIO >>> + bool "LSI ZEVIO SoC memory mapped GPIOs" >>> + depends on ARCH_NSPIRE >> >> >> Can't this appear in some other SoC? > > The problem is, no documents about this SoC are available at all. > Everything we know about this chip has been found out by disassembling the > nspire software (nucleus PLUS), so I have no idea, but probably not. > Also, it can't be tested during bootup, as the only platform we can test it > on > boots from an internal read-only flash, which loads boot2, > which itself is exploitable to start linux. > The entire hardware has already been initialized before booting linux. The thing is that if we don't put in the dependency, we can get some nice compile coverage on a few different compilers and platforms. >>> + * 0x00-0x3F: Section 0 >>> + * +0x00: Masked interrupt status (read-only) >>> + * +0x04: R: Interrupt status W: Reset interrupt status >>> + * +0x08: R: Interrupt mask W: Mask interrupt >>> + * +0x0C: W: Unmask interrupt (write-only) >>> + * +0x10: Direction: I/O=1/0 >>> + * +0x14: Output >>> + * +0x18: Input (read-only) >>> + * +0x20: R: Sticky interrupts W: Set sticky interrupt >> >> >> What is a sticky interrupt? Do you mean it is a level IRQ? >> Then it's edge triggered if zero and level triggered if "sticky" >> is set to 1, right? > > It's how the GPIO controller signals the VIC. > On sticky it keeps the IRQ high until it has been handled (W to 0x4), This is what is called a level interrupt. So please update the terminology to match the common one. > if not sticky, it sets the IRQ line at the same state as the GPIO pin is. > If GPIO high => IRQ high, if GPIO gets low again => IRQ low, which is not > VERY useful.. Hm doesn't seem like an interrupt at all, rather some test operation to test the IRQ line. Well whatever... >> Use just u16 please. uint16_t is some portable C type. >> >> Please replace uint16_t with u16 everywhere. > > It's used VERY often and I couldn't find any coding style document which > prefers u16.. There is no consensus around this. It is up to the subsystem maintainer to decide this I think. >>> +static int zevio_gpio_to_irq(struct gpio_chip *chip, unsigned pin) >>> +{ >>> + /* Not implemented due to weird lockups */ >>> + return -ENXIO; >> >> >> Hm. I guess this should be marked TODO: or something. >> >> So when you figure this out you also add an irqchip. >> >> The way this looks I was thinking it could use the >> drivers/gpio/gpio-generic.c driver, but maybe not? > > Would be great, but it doesn't support multiple registers (4*8 GPIOs), > so I would have to register 4 of them. Then, they had to share one > interrupt, > which would have to be implemented seperately (or not at all) and then it > has > to be working with device tree without any extra (struct platform_)data. > I prefer two bitops (pin&7 and pin>>3), which are present anyways (for IRQs) > over an array of four gpio_chips. OK I buy this, but you know you can reuse only part of the generic MMIO driver, and do not have to use all of it? > I'll resubmit the improved version as V4 after you told me which devicetree > mail addresses I should add. OK thanks. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html