Hello Angelo, On Tue, 2021-02-02 at 15:42 +0100, AngeloGioacchino Del Regno wrote: > Il 02/02/21 08:36, Matti Vaittinen ha scritto: > > If a spurious OCP IRQ occurs the isr schedules delayed work > > but does not disable the IRQ. The delayed work assumes IRQ was > > disabled in handler and attempts enabling it again causing > > unbalanced enable. > > > > You break the logic like this. Though, I also see the problem. > It is critical for the recovery worker to be executed whenever we > enter > the OCP interrupt routine, as we get in there only something wrong > happened. Then the comment just above this check should be adjusted. It states: /* * If we (unlikely) can't read this register, to prevent hardware * damage at all costs, we assume that the overcurrent event was * real; Moreover, if the status register is not signaling OCP, * it was a spurious event, so it's all ok. */ The " if the status register is not signaling OCP, it was a spurious event, so it's all ok." is incredibly misleading. That comment combined with comment above qcom_labibb_check_ocp_status() * This function checks the STATUS1 register for the VREG_OK bit: if it is * set, then there is no Over-Current event. * * Returns: Zero if there is no over-current, 1 if in over-current or * negative number for error made me to _assume_ that when qcom_labibb_check_ocp_status returns zero we have spurious event - for which just returning the IRQ_NONE should be perfectly sane thing to do. > Please fix this patch. > P.S.: You can't disable irq before qcom_labibb_check_ocp_status; > perhaps just after it, or in the if branch before goto? As I said, I don't have the HW or specifications or expertise with this IC. If this was not spurious event then I don't know what is the right thing to do. I am just shooting this blindly. Feel free to take over this fix and also adjust the comments so that they match the HW behaviour :) Best Regards --Matti
