On Sat, 11 Feb 2023 13:45:37 -0600 Samuel Holland <samuel@xxxxxxxxxxxx> wrote: Hi Samuel, > On 2/9/23 14:29, Andre Przywara wrote: > > On Wed, 8 Feb 2023 13:50:04 +0100 > > Andreas Feldner <andreas@xxxxxxxxxxxxx> wrote: > > > > Hi Andreas, > > > > CC:ing Maxime, who wrote the debouncing code back then. > > > >> Am 07.02.23 um 02:16 schrieb Andre Przywara: > >>> On Mon, 6 Feb 2023 20:51:50 +0100 > >>> Andreas Feldner <pelzi@xxxxxxxxxxxxxxx> wrote: > >>> > >>> Hi Andreas, > >>> > >>> thanks for taking care about this board and sending patches! > >> Thank YOU for maintaining it! > >>>> The SoC features debounce logic for external interrupts. Per default, > >>>> this is based on a 32kHz oscillator, in effect filtering away multiple > >>>> interrupts separated by less than roughly 100�s. > >>>> > >>>> This patch sets different defaults for this filter for this board: > >>>> PG is connected to non-mechanical components, without any risk for > >>>> showing bounces. PA is mostly exposed to GPIO pins, however the > >>>> existence of a debounce filter is undesirable as well if electronic > >>>> components are connected. > >>> So how do you know if that's the case? It seems to be quite normal to > >>> just connect mechanical switches to GPIO pins. > >>> > >>> If you are trying to fix a particular issue you encountered, please > >>> describe that here, and say how (or at least that) the patch fixes it. > >>> > >>> And I would suggest to treat port G and port A differently. If you > >>> need a lower debounce threshold for port A, you can apply a DT overlay > >>> in U-Boot, just for your board. > >> > >> Fair enough. You run into problems when you connect (electronic) > >> devices to bank A (typically by the 40pin CON2 connector), where > >> the driver requires fast IRQs to work. In my case this has been a > >> DHT22 sensor, and the default debounce breaking the dht11.ko > >> driver. > > > > Sure, what I meant is that this is a property of your particular > > setup (because you attach something to the *headers*) , so it shouldn't > > be in the generic DT, but just in your copy. Which ideally means using > > a DT overlay. > > > >> Now, what kind of problem is this - I'm no way sure: > >> > >> a) is it an unlucky default, because whoever connects a mechanical > >> switch will know about the problem of bouncing and be taking > >> care to deal with it (whereas at least I was complete unsuspecting > >> when connecting an electronic device that a debounce function > >> might be in place), or > > > > The Linux default is basically the reset default: just leave the > > register at 0x0. It seems like you cannot really turn that off at all > > in hardware, and the reset setting is indeed 32KHz/1. So far there > > haven't been any complaints, though I don't know if people just > > don't use it in anger, or cannot be bothered to send a report to the > > list. > > > >> b) is it a bug in the devicetree for (at least) the BananaPi M2 Zero, > >> because the IRQ bank G is hard wired to electronic devices that > >> should not be fenced by a debouncing function, or > > > > Well, we could try to turn that "off" as much as possible, but on the > > other hand the debounce only affects *GPIO* *interrupts*, so not sure > > that gives us anything. The PortG pins are used for the SDIO Wifi, BT > > UART, and the wakeup pins for the Wifi chip. Only the wakeup pins would > > be affected, and I doubt that we wake up that often that it matters. If > > you've made other observations, please let me know. > > > > Certainly no board with an in-tree DT sets the debounce property, which > > means everyone uses 32KHz/1, and also did so before the functionality > > was introduced. > > One side note relevant to wakeup pins: if the debounce clock source is > set to HOSC, and the 24 MHz oscillator is disabled, then IRQs for those > pins will never fire. That's a good point. > Currently, Crust does not check the debounce configuration when deciding > if it can turn off the 24 MHz crystal during system suspend (or fake-off > on boards without PMICs), so any wakeup-capable GPIOs need to use LOSC > as their debounce clock. > > Do you have any thoughts about if/how we should handle this > automatically? Should Linux (or Crust) override the debounce > configuration when entering suspend? My feeling is since it's Crust's decision to disable the 24MHz oscillator, it should make sure that's a workable configuration. So it would be Crust's responsibility to avoid using 24 MHz as the debounce clock, I'd say. There could be an argument about Linux re-initialising the GPIO during resume, but that wouldn't help you anyway. > I imagine no wakeup source will > require a particularly short debounce time. Since it all seems to work fine with the current 32KHz/1 setup, I wouldn't expect any issues due to too short pulses being eaten by the debouncing logic. And a too short debounce period shouldn't matter for wakeup either, since it's the first edge that counts. Cheers, Andre
