Hi Thomas, > -----Original Message----- > From: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Sent: Wednesday, March 13, 2024 3:40 PM > Subject: Re: [PATCH v2 5/5] irqchip/renesas-rzg2l: Use TIEN for enable/disable > > On Tue, Mar 05 2024 at 18:39, Biju Das wrote: > > Currently hardware settings for TINT detection is not in sync with > > TINT source as the enable/disable overrides source setting value > > leading to hardware inconsistent state. For eg: consider the case > > GPIOINT0 is used as TINT interrupt and configuring GPIOINT5 as > > edgetype. During disable the clearing of the entire bytes of TINT > > source selection for GPIOINT5 is same as GPIOINT0 with TIEN disabled. > > Other than this during enabling, the setting of GPIOINT5 with TIEN > > results in spurious IRQ as due to a HW race, it is possible that IP > > can use the TIEN with previous source value (GPIOINT0). > > > > So, it is better to just use TIEN for enable/disable and avoid > > modifying TINT source selection register.This will make the consistent > > hardware settings for detection method tied with TINT source and > > allows to simplify the code. > > I have no idea how the subject and change log is related to what the patch is doing. > > The patch just consolidates the almost identical functionality of > rzg2l_irqc_irq_disable() and rzg2l_irqc_irq_enable() into a helper function which is invoked from both > places. The existing code already uses TIEN for disable and enable, so what's the change? > > IOW, it's zero functional change and completely unrelated to the above blurb. There is functional change. During disable, TINT source and TIEN cleared together reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset)); During Enable, TINT source and TIEN set together reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset); This patch avoids modifying TINT source register which avoids hw race as mentioned by hardware team. According to them we should not set TINT source and TIEN together. I can update the change log. Cheers, Biju
