Messages in this thread |  | | | Date | Thu, 11 Jun 2009 23:35:55 +0200 | | Subject | Re: [PATCH 1/2] cs5535-gpio: add AMD CS5535/CS5536 GPIO driver support | | From | Tobias Müller <> |
| |
>> /** >> * Some GPIO pins >> * 31-29,23 : reserved (always mask out) >> * 28 : Power Button >> * 26 : PME# >> * 22-16 : LPC >> * 14,15 : SMBus >> * 9,8 : UART1 >> * 7 : PCI INTB >> * 3,4 : UART2/DDC >> * 2 : IDE_IRQ0 >> * 1 : AC_BEEP >> * 0 : PCI INTA >> * >> * If a mask was not specified, be conservative and only allow: >> * 1,2,5,6,10-13,24,25,27 >> */ >> >> I'll add this in my patch to clear it out. >> > > But why are you being conservative in the first place? If something's > using GPIOs, unless they're unmapped, you should allow it to use them > without requiring a boot arg. > > For example, OLPC uses GPIO 7 for its DCON IRQ. With the masking > scheme, OLPC will need to set that mask from the default. I don't see > the point of having the mask at all if other drivers in the kernel are > going to be requesting GPIOs (presumably they know what they're doing). Hmm... OK, this makes sense. So default mask allow everything exept reserved pins and pin 28 (power button).
I think the mask is quite useful if you've critical things on GPIO pins and they should be changeable (especially from userspace and when non-root users are allowed to use userspace gpio).
>> >> + /* disable output aux 1 & 2 on this pin */ >> >> + __cs5535_gpio_clear(chip, offset, GPIO_OUTPUT_AUX1); >> >> + __cs5535_gpio_clear(chip, offset, GPIO_OUTPUT_AUX2); >> >> + >> >> + /* disable input aux 1 on this pin */ >> >> + __cs5535_gpio_clear(chip, offset, GPIO_INPUT_AUX1); >> >> + >> >> + /* disable output */ >> >> + __cs5535_gpio_clear(chip, offset, GPIO_OUTPUT_ENABLE); >> >> + >> >> + /* enable input */ >> >> + __cs5535_gpio_set(chip, offset, GPIO_INPUT_ENABLE); >> > >> > I don't think this is the right place for all of this. Your earlier >> > email mentioned disabling OUT_AUX{1,2} for outputs, and IN_AUX for >> > inputs. I'm fine with doing that here, but I don't see why you're >> > also disabling output and enabling input by default. >> >> I mentioned this in an ealier mail too. When I request the GPIO from >> userspace the direction file always contains "in", so I thought >> this is the standard direction after resetting as I should be in a >> defined state after requesting. But I didn't found anything >> about this in GPIO lib documentation, so I would be fine to change >> this if there is any common default behavoir. > > To be honest, I'd have to play around with it a bit before I > knew whether it actually breaks anything or not. I'm not sure if > it would break anything on OLPC, and I don't have any other geode > machines that do anything interesting w/ GPIOs. > > Maybe David can clear up whether this behavior is correct from the > userspace GPIO usage standpoint.. This would be nice. But I wouldn't have a problem when those two statements are removed. I just thought it's better to put them in a defined state. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |