Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-08-07 Thread Dhananjay Phadke
Ray Jui wrote: > > Any progress yet? > > I don't know if Dhananjay is actively working on this or not. > > Rayagonda, given that you have the I2C slave setup already, do you think > you can help to to test and above sequence from Wolfram (by using the > widened delay window as instructed)? >

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-08-07 Thread Ray Jui
Hi Rayagonda/Dhananjay, On 8/5/2020 2:17 AM, Wolfram Sang wrote: > On Mon, Jul 27, 2020 at 01:43:40PM -0700, Ray Jui wrote: >> >> >> On 7/27/2020 1:26 PM, Wolfram Sang wrote: >>> On Mon, Jul 27, 2020 at 08:13:46PM +0200, Wolfram Sang wrote: > Can you confirm that even if we have irq

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-08-05 Thread Wolfram Sang
On Mon, Jul 27, 2020 at 01:43:40PM -0700, Ray Jui wrote: > > > On 7/27/2020 1:26 PM, Wolfram Sang wrote: > > On Mon, Jul 27, 2020 at 08:13:46PM +0200, Wolfram Sang wrote: > >> > >>> Can you confirm that even if we have irq pending at the i2c IP core > >>> level, as long as we execute Step 2.

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-27 Thread Ray Jui
On 7/27/2020 1:26 PM, Wolfram Sang wrote: > On Mon, Jul 27, 2020 at 08:13:46PM +0200, Wolfram Sang wrote: >> >>> Can you confirm that even if we have irq pending at the i2c IP core >>> level, as long as we execute Step 2. below (to disable/mask all slave >>> interrupts), after 'enable_irq' is

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-27 Thread Wolfram Sang
On Mon, Jul 27, 2020 at 08:13:46PM +0200, Wolfram Sang wrote: > > > Can you confirm that even if we have irq pending at the i2c IP core > > level, as long as we execute Step 2. below (to disable/mask all slave > > interrupts), after 'enable_irq' is called, we still will not receive any > >

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-27 Thread Wolfram Sang
> Can you confirm that even if we have irq pending at the i2c IP core > level, as long as we execute Step 2. below (to disable/mask all slave > interrupts), after 'enable_irq' is called, we still will not receive any > further i2c slave interrupt? This is HW dependant. From my tests with Renesas

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-27 Thread Dhananjay Phadke
Ray Jui wrote: >>> I think the following sequence needs to be implemented to make this >>> safe, i.e., after 'synchronize_irq', no further slave interrupt will be >>> fired. >>> >>> In 'bcm_iproc_i2c_unreg_slave': >>> >>> 1. Set an atomic variable 'unreg_slave' (I'm bad in names so please come

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-27 Thread Ray Jui
Hi Wolfram, On 7/25/2020 3:18 AM, Wolfram Sang wrote: > >> I think the following sequence needs to be implemented to make this >> safe, i.e., after 'synchronize_irq', no further slave interrupt will be >> fired. >> >> In 'bcm_iproc_i2c_unreg_slave': >> >> 1. Set an atomic variable 'unreg_slave'

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-26 Thread Rayagonda Kokatanur
On Sat, Jul 25, 2020 at 3:48 PM Wolfram Sang wrote: > > > > I think the following sequence needs to be implemented to make this > > safe, i.e., after 'synchronize_irq', no further slave interrupt will be > > fired. > > > > In 'bcm_iproc_i2c_unreg_slave': > > > > 1. Set an atomic variable

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-25 Thread Wolfram Sang
> I think the following sequence needs to be implemented to make this > safe, i.e., after 'synchronize_irq', no further slave interrupt will be > fired. > > In 'bcm_iproc_i2c_unreg_slave': > > 1. Set an atomic variable 'unreg_slave' (I'm bad in names so please come > up with a better name than

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-22 Thread Dhananjay Phadke
Ray Jui wrote: > > On 7/22/2020 3:41 AM, Wolfram Sang wrote: > > > >>> + synchronize_irq(iproc_i2c->irq); > >> > >> If one takes a look at the I2C slave ISR routine, there are places where > >> IRQ can be re-enabled in the ISR itself. What happens after we mask all > >> slave interrupt and when

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-22 Thread Ray Jui
On 7/22/2020 8:51 AM, Ray Jui wrote: > > On 7/22/2020 3:41 AM, Wolfram Sang wrote: >> + synchronize_irq(iproc_i2c->irq); >>> >>> If one takes a look at the I2C slave ISR routine, there are places where >>> IRQ can be re-enabled in the ISR itself. What happens after we mask all >>> slave

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-22 Thread Ray Jui
On 7/22/2020 3:41 AM, Wolfram Sang wrote: > >>> + synchronize_irq(iproc_i2c->irq); >> >> If one takes a look at the I2C slave ISR routine, there are places where >> IRQ can be re-enabled in the ISR itself. What happens after we mask all >> slave interrupt and when 'synchronize_irq' is called,

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-22 Thread Wolfram Sang
> > + synchronize_irq(iproc_i2c->irq); > > If one takes a look at the I2C slave ISR routine, there are places where > IRQ can be re-enabled in the ISR itself. What happens after we mask all > slave interrupt and when 'synchronize_irq' is called, which I suppose is > meant to wait for inflight

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-20 Thread Ray Jui
On 7/18/2020 4:39 PM, Dhananjay Phadke wrote: > When i2c client unregisters, synchronize irq before setting > iproc_i2c->slave to NULL. > > Unable to handle kernel NULL pointer dereference at virtual address > 0318 > > [ 371.020421] pc : bcm_iproc_i2c_isr+0x530/0x11f0 > [

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-20 Thread Scott Branden
looks good. On 2020-07-19 12:59 a.m., Rayagonda Kokatanur wrote: On Sun, Jul 19, 2020 at 5:10 AM Dhananjay Phadke wrote: When i2c client unregisters, synchronize irq before setting iproc_i2c->slave to NULL. Unable to handle kernel NULL pointer dereference at virtual address 0318

Re: [PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-19 Thread Rayagonda Kokatanur
On Sun, Jul 19, 2020 at 5:10 AM Dhananjay Phadke wrote: > > When i2c client unregisters, synchronize irq before setting > iproc_i2c->slave to NULL. > > Unable to handle kernel NULL pointer dereference at virtual address > 0318 > > [ 371.020421] pc : bcm_iproc_i2c_isr+0x530/0x11f0 >

[PATCH] i2c: iproc: fix race between client unreg and isr

2020-07-18 Thread Dhananjay Phadke
When i2c client unregisters, synchronize irq before setting iproc_i2c->slave to NULL. Unable to handle kernel NULL pointer dereference at virtual address 0318 [ 371.020421] pc : bcm_iproc_i2c_isr+0x530/0x11f0 [ 371.025098] lr : __handle_irq_event_percpu+0x6c/0x170 [ 371.030309]