On 02/05/2024 12:41, André Draszik wrote: > On Thu, 2024-05-02 at 09:46 +0200, Krzysztof Kozlowski wrote: >> On 02/05/2024 09:41, Tudor Ambarus wrote: >>>> >>>> @@ -223,6 +268,13 @@ static void exynos_irq_release_resources(struct irq_data *irqd) >>>> shift = irqd->hwirq * bank_type->fld_width[PINCFG_TYPE_FUNC]; >>>> mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1; >>>> >>>> + if (clk_enable(bank->drvdata->pclk)) { >>>> + dev_err(bank->gpio_chip.parent, >>>> + "unable to enable clock for deconfiguring pin %s-%lu\n", >>>> + bank->name, irqd->hwirq); >>>> + return; >>> >>> but here we just print an error. I guess that for consistency reasons it >>> would be good to follow up with a patch and change the return types of >>> these methods and return the error too when the clock enable fails. >> >> That's a release, so usually void callback. The true issue is that we >> expect release to always succeed, I think. >> >> This points to issue with this patchset: looks like some patchwork all >> around the places having register accesses. But how do you even expect >> interrupts and pins to work if entire pinctrl block is clock gated? > > I was initially thinking the same, but the clock seems to be required for > register access only, interrupts are still being received and triggered > with pclk turned off as per my testing. Probably we could simplify this all and keep the clock enabled always, when device is not suspended. Toggling clock on/off for every pin change is also an overhead. Anyway, I merged the patches for now, because it addresses real problem and seems like one of reasonable solutions. Best regards, Krzysztof