* [PATCH RFC 0/3] ARM64: bcm2837-rpi: Fix random crashes caused by i2c @ 2017-02-16 21:20 Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference Stefan Wahren ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Stefan Wahren @ 2017-02-16 21:20 UTC (permalink / raw) To: Eric Anholt, Wolfram Sang, Peter Robinson Cc: Martin Sperl, Noralf Trønnes, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel, devicetree, linux-i2c, Stefan Wahren This patch series tries to fix the random crashes [1] which occur on the RPi 3. The first patch fix the possible NULL pointer dereference and the rest tries to fix the root cause of the unexpected interrupts. Btw this should also fix the random i2c timeouts: i2c-bcm2835 3f805000.i2c: i2c transfer timed out In absence of a RPi 3 this is only compile tested. [1] - https://github.com/anholt/linux/issues/89 Stefan Wahren (3): i2c: bcm2835: Avoid possible NULL ptr dereference ARM64: bcm2837: remove link to bcm2835 ARM64: bcm2837-rpi: Fix pwm and i2c pin control arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi | 1 - arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts | 2 +- arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi | 89 ++++++++++++++++++++++ drivers/i2c/busses/i2c-bcm2835.c | 4 +- 4 files changed, 93 insertions(+), 3 deletions(-) delete mode 120000 arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi create mode 100644 arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi -- 1.7.9.5 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-16 21:20 [PATCH RFC 0/3] ARM64: bcm2837-rpi: Fix random crashes caused by i2c Stefan Wahren @ 2017-02-16 21:20 ` Stefan Wahren [not found] ` <1487280047-29608-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org> 2017-02-20 18:22 ` Wolfram Sang 2017-02-16 21:20 ` [PATCH RFC 2/3] ARM64: bcm2837-rpi: remove link to bcm2835-rpi.dtsi Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control Stefan Wahren 2 siblings, 2 replies; 20+ messages in thread From: Stefan Wahren @ 2017-02-16 21:20 UTC (permalink / raw) To: Eric Anholt, Wolfram Sang, Peter Robinson Cc: Martin Sperl, Noralf Trønnes, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel, devicetree, linux-i2c, Stefan Wahren Since commit e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") the interrupt handler is prone to a possible NULL pointer dereference. This could happen if an interrupt fires before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs on the RPi 3. Even this is an unexpected behavior the driver must handle that with an error instead of a crash. CC: Noralf Trønnes <noralf@tronnes.org> CC: Martin Sperl <kernel@martin.sperl.org> Reported-by: Peter Robinson <pbrobinson@gmail.com> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- drivers/i2c/busses/i2c-bcm2835.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c index c3436f6..10e39c8 100644 --- a/drivers/i2c/busses/i2c-bcm2835.c +++ b/drivers/i2c/busses/i2c-bcm2835.c @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data) } if (val & BCM2835_I2C_S_DONE) { - if (i2c_dev->curr_msg->flags & I2C_M_RD) { + if (!i2c_dev->curr_msg) { + dev_err(i2c_dev->dev, "Got unexpected interrupt (from firmware?)\n"); + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { bcm2835_drain_rxfifo(i2c_dev); val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <1487280047-29608-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>]
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <1487280047-29608-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org> @ 2017-02-18 18:34 ` Noralf Trønnes [not found] ` <48907a31-eaa6-27e2-633f-d36de521e868-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> 0 siblings, 1 reply; 20+ messages in thread From: Noralf Trønnes @ 2017-02-18 18:34 UTC (permalink / raw) To: Stefan Wahren, Eric Anholt, Wolfram Sang, Peter Robinson Cc: Martin Sperl, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Den 16.02.2017 22.20, skrev Stefan Wahren: > Since commit e2474541032d ("bcm2835: Fix hang for writing messages > larger than 16 bytes") the interrupt handler is prone to a possible > NULL pointer dereference. This could happen if an interrupt fires > before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs > on the RPi 3. Even this is an unexpected behavior the driver must > handle that with an error instead of a crash. > > CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> > Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > --- > drivers/i2c/busses/i2c-bcm2835.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c > index c3436f6..10e39c8 100644 > --- a/drivers/i2c/busses/i2c-bcm2835.c > +++ b/drivers/i2c/busses/i2c-bcm2835.c > @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data) > } > > if (val & BCM2835_I2C_S_DONE) { > - if (i2c_dev->curr_msg->flags & I2C_M_RD) { > + if (!i2c_dev->curr_msg) { > + dev_err(i2c_dev->dev, "Got unexpected interrupt (from firmware?)\n"); > + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { > bcm2835_drain_rxfifo(i2c_dev); > val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); > } Thanks, Acked-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <48907a31-eaa6-27e2-633f-d36de521e868-L59+Z2yzLopAfugRpC6u6w@public.gmane.org>]
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <48907a31-eaa6-27e2-633f-d36de521e868-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> @ 2017-02-20 19:40 ` Stefan Wahren [not found] ` <1983281189.110224.1487619653742-7tX72C7vayboQLBSYMtkGA@public.gmane.org> 0 siblings, 1 reply; 20+ messages in thread From: Stefan Wahren @ 2017-02-20 19:40 UTC (permalink / raw) To: Peter Robinson, Michael Zoran, Eric Anholt, Noralf Trønnes Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang Hi, > Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> hat am 18. Februar 2017 um 19:34 geschrieben: > > > > Den 16.02.2017 22.20, skrev Stefan Wahren: > > Since commit e2474541032d ("bcm2835: Fix hang for writing messages > > larger than 16 bytes") the interrupt handler is prone to a possible > > NULL pointer dereference. This could happen if an interrupt fires > > before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs > > on the RPi 3. Even this is an unexpected behavior the driver must > > handle that with an error instead of a crash. > > > > CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > > CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> > > Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") > > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > > --- > > drivers/i2c/busses/i2c-bcm2835.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c > > index c3436f6..10e39c8 100644 > > --- a/drivers/i2c/busses/i2c-bcm2835.c > > +++ b/drivers/i2c/busses/i2c-bcm2835.c > > @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data) > > } > > > > if (val & BCM2835_I2C_S_DONE) { > > - if (i2c_dev->curr_msg->flags & I2C_M_RD) { > > + if (!i2c_dev->curr_msg) { > > + dev_err(i2c_dev->dev, "Got unexpected interrupt (from firmware?)\n"); > > + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { > > bcm2835_drain_rxfifo(i2c_dev); > > val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); > > } > > Thanks, > > Acked-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > thanks, but i would be more happier to receive feedback for patches 2 and 3. Stefan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <1983281189.110224.1487619653742-7tX72C7vayboQLBSYMtkGA@public.gmane.org>]
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <1983281189.110224.1487619653742-7tX72C7vayboQLBSYMtkGA@public.gmane.org> @ 2017-02-20 21:22 ` Noralf Trønnes [not found] ` <5b14ed6d-4db8-abf7-ceba-ef46534b6023-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> 0 siblings, 1 reply; 20+ messages in thread From: Noralf Trønnes @ 2017-02-20 21:22 UTC (permalink / raw) To: Stefan Wahren, Peter Robinson, Michael Zoran, Eric Anholt Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang Den 20.02.2017 20.40, skrev Stefan Wahren: > Hi, > >> Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> hat am 18. Februar 2017 um 19:34 geschrieben: >> >> >> >> Den 16.02.2017 22.20, skrev Stefan Wahren: >>> Since commit e2474541032d ("bcm2835: Fix hang for writing messages >>> larger than 16 bytes") the interrupt handler is prone to a possible >>> NULL pointer dereference. This could happen if an interrupt fires >>> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs >>> on the RPi 3. Even this is an unexpected behavior the driver must >>> handle that with an error instead of a crash. >>> >>> CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >>> CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> >>> Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >>> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") >>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> >>> --- >>> drivers/i2c/busses/i2c-bcm2835.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c >>> index c3436f6..10e39c8 100644 >>> --- a/drivers/i2c/busses/i2c-bcm2835.c >>> +++ b/drivers/i2c/busses/i2c-bcm2835.c >>> @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data) >>> } >>> >>> if (val & BCM2835_I2C_S_DONE) { >>> - if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>> + if (!i2c_dev->curr_msg) { >>> + dev_err(i2c_dev->dev, "Got unexpected interrupt (from firmware?)\n"); >>> + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>> bcm2835_drain_rxfifo(i2c_dev); >>> val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); >>> } >> Thanks, >> >> Acked-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >> > thanks, but i would be more happier to receive feedback for patches 2 and 3. I have only worked on dts files downstream and never done any arm64 stuff, so I'm not up to speed on this. Noralf. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <5b14ed6d-4db8-abf7-ceba-ef46534b6023-L59+Z2yzLopAfugRpC6u6w@public.gmane.org>]
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <5b14ed6d-4db8-abf7-ceba-ef46534b6023-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> @ 2017-02-21 8:58 ` Stefan Wahren [not found] ` <31b96f29-e369-546a-6270-266daea71062-eS4NqCHxEME@public.gmane.org> 2017-02-21 15:07 ` Michael Zoran 1 sibling, 1 reply; 20+ messages in thread From: Stefan Wahren @ 2017-02-21 8:58 UTC (permalink / raw) To: Noralf Trønnes, Peter Robinson, Michael Zoran, Eric Anholt Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang Am 20.02.2017 um 22:22 schrieb Noralf Trønnes: > > Den 20.02.2017 20.40, skrev Stefan Wahren: >> Hi, >> >>> Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> hat am 18. Februar 2017 um 19:34 >>> geschrieben: >>> >>> >>> >>> Den 16.02.2017 22.20, skrev Stefan Wahren: >>>> Since commit e2474541032d ("bcm2835: Fix hang for writing messages >>>> larger than 16 bytes") the interrupt handler is prone to a possible >>>> NULL pointer dereference. This could happen if an interrupt fires >>>> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs >>>> on the RPi 3. Even this is an unexpected behavior the driver must >>>> handle that with an error instead of a crash. >>>> >>>> CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >>>> CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> >>>> Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >>>> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger >>>> than 16 bytes") >>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> >>>> --- >>>> drivers/i2c/busses/i2c-bcm2835.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c >>>> b/drivers/i2c/busses/i2c-bcm2835.c >>>> index c3436f6..10e39c8 100644 >>>> --- a/drivers/i2c/busses/i2c-bcm2835.c >>>> +++ b/drivers/i2c/busses/i2c-bcm2835.c >>>> @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int >>>> this_irq, void *data) >>>> } >>>> if (val & BCM2835_I2C_S_DONE) { >>>> - if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>> + if (!i2c_dev->curr_msg) { >>>> + dev_err(i2c_dev->dev, "Got unexpected interrupt (from >>>> firmware?)\n"); >>>> + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>> bcm2835_drain_rxfifo(i2c_dev); >>>> val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); >>>> } >>> Thanks, >>> >>> Acked-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >>> >> thanks, but i would be more happier to receive feedback for patches 2 >> and 3. > > I have only worked on dts files downstream and never done any arm64 > stuff, so I'm not up to speed on this. > > Noralf. > Okay, at the end i only want to know 2 things: Does new message "Got unexpected interrupt" still appear after applying all 3 patches? Does the annoying message "i2c-bcm2835 3f805000.i2c: i2c transfer timed out" still appear after applying all 3 patches? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <31b96f29-e369-546a-6270-266daea71062-eS4NqCHxEME@public.gmane.org>]
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <31b96f29-e369-546a-6270-266daea71062-eS4NqCHxEME@public.gmane.org> @ 2017-02-21 11:54 ` kernel-TqfNSX0MhmxHKSADF0wUEw 0 siblings, 0 replies; 20+ messages in thread From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2017-02-21 11:54 UTC (permalink / raw) To: Stefan Wahren Cc: Noralf Trønnes, Peter Robinson, Michael Zoran, Eric Anholt, Will Deacon, Frank Rowand, Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang > On 21.02.2017, at 09:58, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote: > > Am 20.02.2017 um 22:22 schrieb Noralf Trønnes: >> >> Den 20.02.2017 20.40, skrev Stefan Wahren: >>> Hi, >>> >>>> Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> hat am 18. Februar 2017 um 19:34 geschrieben: >>>> >>>> >>>> >>>> Den 16.02.2017 22.20, skrev Stefan Wahren: >>>>> Since commit e2474541032d ("bcm2835: Fix hang for writing messages >>>>> larger than 16 bytes") the interrupt handler is prone to a possible >>>>> NULL pointer dereference. This could happen if an interrupt fires >>>>> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs >>>>> on the RPi 3. Even this is an unexpected behavior the driver must >>>>> handle that with an error instead of a crash. >>>>> >>>>> CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >>>>> CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> >>>>> Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >>>>> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") >>>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> >>>>> --- >>>>> drivers/i2c/busses/i2c-bcm2835.c | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c >>>>> index c3436f6..10e39c8 100644 >>>>> --- a/drivers/i2c/busses/i2c-bcm2835.c >>>>> +++ b/drivers/i2c/busses/i2c-bcm2835.c >>>>> @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int this_irq, void *data) >>>>> } >>>>> if (val & BCM2835_I2C_S_DONE) { >>>>> - if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>>> + if (!i2c_dev->curr_msg) { >>>>> + dev_err(i2c_dev->dev, "Got unexpected interrupt (from firmware?)\n"); >>>>> + } else if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>>> bcm2835_drain_rxfifo(i2c_dev); >>>>> val = bcm2835_i2c_readl(i2c_dev, BCM2835_I2C_S); >>>>> } >>>> Thanks, >>>> >>>> Acked-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >>>> >>> thanks, but i would be more happier to receive feedback for patches 2 and 3. >> >> I have only worked on dts files downstream and never done any arm64 >> stuff, so I'm not up to speed on this. >> >> Noralf. >> > > Okay, at the end i only want to know 2 things: > > Does new message "Got unexpected interrupt" still appear after applying all 3 patches? > > Does the annoying message "i2c-bcm2835 3f805000.i2c: i2c transfer timed out" still appear after applying all 3 patches? I believe the issue is primarily that i2c0@3f805000 is “owned” by the firmware. This is less of a problem on the rpi1 and rpi2, as there is little to no activity on those on i2c0 by the firmware (besides when using the camera). But on the rpi3 some Voltage sensors as well as GPIO extenders are attached to this bus, which are frequently read/modified by the firmware. So in principle this i2c bus should be disabled. But if it is then there will be an interrupt triggered on the VC4 core as well as on the ARM core - both will see the interrupt. but typically the RT-firmware will respond much faster and disable the irq sources already before the arm gets to “detect” the active interrupt in the irq handler. But sometimes the firmware is not fast enough (or the irq path is currently active in linux at the right time) so the interrupt handler will trigger even if there was no running i2c transfer on that bus. The patch is basically defensive programming to avoid such cases. Martin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <5b14ed6d-4db8-abf7-ceba-ef46534b6023-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> 2017-02-21 8:58 ` Stefan Wahren @ 2017-02-21 15:07 ` Michael Zoran 2017-02-21 15:37 ` Stefan Wahren 1 sibling, 1 reply; 20+ messages in thread From: Michael Zoran @ 2017-02-21 15:07 UTC (permalink / raw) To: Noralf Trønnes, Stefan Wahren, Peter Robinson, Eric Anholt Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang On Mon, 2017-02-20 at 22:22 +0100, Noralf Trønnes wrote: > Den 20.02.2017 20.40, skrev Stefan Wahren: > > Hi, > > > > > Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> hat am 18. Februar 2017 um > > > 19:34 geschrieben: > > > > > > > > > > > > Den 16.02.2017 22.20, skrev Stefan Wahren: > > > > Since commit e2474541032d ("bcm2835: Fix hang for writing > > > > messages > > > > larger than 16 bytes") the interrupt handler is prone to a > > > > possible > > > > NULL pointer dereference. This could happen if an interrupt > > > > fires > > > > before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly > > > > occurs > > > > on the RPi 3. Even this is an unexpected behavior the driver > > > > must > > > > handle that with an error instead of a crash. > > > > > > > > CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > > > > CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> > > > > Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > > Fixes: e2474541032d ("bcm2835: Fix hang for writing messages > > > > larger than 16 bytes") > > > > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > > > > --- > > > > drivers/i2c/busses/i2c-bcm2835.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/i2c/busses/i2c-bcm2835.c > > > > b/drivers/i2c/busses/i2c-bcm2835.c > > > > index c3436f6..10e39c8 100644 > > > > --- a/drivers/i2c/busses/i2c-bcm2835.c > > > > +++ b/drivers/i2c/busses/i2c-bcm2835.c > > > > @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int > > > > this_irq, void *data) > > > > } > > > > > > > > if (val & BCM2835_I2C_S_DONE) { > > > > - if (i2c_dev->curr_msg->flags & I2C_M_RD) { > > > > + if (!i2c_dev->curr_msg) { > > > > + dev_err(i2c_dev->dev, "Got unexpected > > > > interrupt (from firmware?)\n"); > > > > + } else if (i2c_dev->curr_msg->flags & > > > > I2C_M_RD) { > > > > bcm2835_drain_rxfifo(i2c_dev); > > > > val = bcm2835_i2c_readl(i2c_dev, > > > > BCM2835_I2C_S); > > > > } > > > > > > Thanks, > > > > > > Acked-by: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > > > > > > > thanks, but i would be more happier to receive feedback for patches > > 2 and 3. > > I have only worked on dts files downstream and never done any arm64 > stuff, so I'm not up to speed on this. > > Noralf. > Just a note, the original downstream driver returns IRQ_NONE in this case. Does that make any difference? Also, the the original driver has the check at the start of the IRQ and it doesn't log an error message. I'm wondering if logging at err level is the best since it might make people think a serious error has happened. I think by default this would show up on the console, correct? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-21 15:07 ` Michael Zoran @ 2017-02-21 15:37 ` Stefan Wahren 0 siblings, 0 replies; 20+ messages in thread From: Stefan Wahren @ 2017-02-21 15:37 UTC (permalink / raw) To: Michael Zoran, Noralf Trønnes, Peter Robinson, Eric Anholt Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c, Catalin Marinas, Florian Fainelli, linux-rpi-kernel, devicetree, Wolfram Sang Am 21.02.2017 um 16:07 schrieb Michael Zoran: > On Mon, 2017-02-20 at 22:22 +0100, Noralf Trønnes wrote: >> Den 20.02.2017 20.40, skrev Stefan Wahren: >>> Hi, >>> >>>> Noralf Trønnes <noralf@tronnes.org> hat am 18. Februar 2017 um >>>> 19:34 geschrieben: >>>> >>>> >>>> >>>> Den 16.02.2017 22.20, skrev Stefan Wahren: >>>>> Since commit e2474541032d ("bcm2835: Fix hang for writing >>>>> messages >>>>> larger than 16 bytes") the interrupt handler is prone to a >>>>> possible >>>>> NULL pointer dereference. This could happen if an interrupt >>>>> fires >>>>> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly >>>>> occurs >>>>> on the RPi 3. Even this is an unexpected behavior the driver >>>>> must >>>>> handle that with an error instead of a crash. >>>>> >>>>> CC: Noralf Trønnes <noralf@tronnes.org> >>>>> CC: Martin Sperl <kernel@martin.sperl.org> >>>>> Reported-by: Peter Robinson <pbrobinson@gmail.com> >>>>> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages >>>>> larger than 16 bytes") >>>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >>>>> --- >>>>> drivers/i2c/busses/i2c-bcm2835.c | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c >>>>> b/drivers/i2c/busses/i2c-bcm2835.c >>>>> index c3436f6..10e39c8 100644 >>>>> --- a/drivers/i2c/busses/i2c-bcm2835.c >>>>> +++ b/drivers/i2c/busses/i2c-bcm2835.c >>>>> @@ -195,7 +195,9 @@ static irqreturn_t bcm2835_i2c_isr(int >>>>> this_irq, void *data) >>>>> } >>>>> >>>>> if (val & BCM2835_I2C_S_DONE) { >>>>> - if (i2c_dev->curr_msg->flags & I2C_M_RD) { >>>>> + if (!i2c_dev->curr_msg) { >>>>> + dev_err(i2c_dev->dev, "Got unexpected >>>>> interrupt (from firmware?)\n"); >>>>> + } else if (i2c_dev->curr_msg->flags & >>>>> I2C_M_RD) { >>>>> bcm2835_drain_rxfifo(i2c_dev); >>>>> val = bcm2835_i2c_readl(i2c_dev, >>>>> BCM2835_I2C_S); >>>>> } >>>> Thanks, >>>> >>>> Acked-by: Noralf Trønnes <noralf@tronnes.org> >>>> >>> thanks, but i would be more happier to receive feedback for patches >>> 2 and 3. >> I have only worked on dts files downstream and never done any arm64 >> stuff, so I'm not up to speed on this. >> >> Noralf. >> > Just a note, the original downstream driver returns IRQ_NONE in this > case. Does that make any difference? Yes, i decided to handle the IRQ in order to avoid a possible interrupt storm. Please look at the github discussion [1] for more details. > > Also, the the original driver has the check at the start of the IRQ and > it doesn't log an error message. > > I'm wondering if logging at err level is the best since it might make > people think a serious error has happened. It is a serious error and should be fixed by patch 2, 3 and a possible patch 4 later to disable i2c0. But before i disable i2c0 we need the test results of patch 2 and 3. The reason why i don't disable i2c at first is that Gerd [2] reported that disabling the PWM fixed the i2c timeout issue, which doesn't fit to Martins theory. [1] - https://github.com/anholt/linux/issues/89 [2] - http://lists.infradead.org/pipermail/linux-rpi-kernel/2016-June/003969.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-16 21:20 ` [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference Stefan Wahren [not found] ` <1487280047-29608-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org> @ 2017-02-20 18:22 ` Wolfram Sang 2017-02-21 16:31 ` Stefan Wahren 1 sibling, 1 reply; 20+ messages in thread From: Wolfram Sang @ 2017-02-20 18:22 UTC (permalink / raw) To: Stefan Wahren Cc: Eric Anholt, Peter Robinson, Martin Sperl, Noralf Trønnes, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel, devicetree, linux-i2c [-- Attachment #1: Type: text/plain, Size: 939 bytes --] On Thu, Feb 16, 2017 at 09:20:45PM +0000, Stefan Wahren wrote: > Since commit e2474541032d ("bcm2835: Fix hang for writing messages > larger than 16 bytes") the interrupt handler is prone to a possible > NULL pointer dereference. This could happen if an interrupt fires > before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs > on the RPi 3. Even this is an unexpected behavior the driver must > handle that with an error instead of a crash. > > CC: Noralf Trønnes <noralf@tronnes.org> > CC: Martin Sperl <kernel@martin.sperl.org> > Reported-by: Peter Robinson <pbrobinson@gmail.com> > Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> Applied to for-next, thanks (will be in 4.11)! Note for patches 2+3: I usually don't take DTS changes via I2C, so this likely needs to go via arm-soc or some other bcm tree. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-20 18:22 ` Wolfram Sang @ 2017-02-21 16:31 ` Stefan Wahren 2017-02-21 20:14 ` Wolfram Sang 0 siblings, 1 reply; 20+ messages in thread From: Stefan Wahren @ 2017-02-21 16:31 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Eric Anholt, Peter Robinson, Martin Sperl, Wolfram Sang, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, Noralf Trønnes, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hi Greg, hi Wolfram, Am 20.02.2017 um 19:22 schrieb Wolfram Sang: > On Thu, Feb 16, 2017 at 09:20:45PM +0000, Stefan Wahren wrote: >> Since commit e2474541032d ("bcm2835: Fix hang for writing messages >> larger than 16 bytes") the interrupt handler is prone to a possible >> NULL pointer dereference. This could happen if an interrupt fires >> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs >> on the RPi 3. Even this is an unexpected behavior the driver must >> handle that with an error instead of a crash. >> >> CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> >> CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> >> Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") >> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > Applied to for-next, thanks (will be in 4.11)! > since this patch is too late for 4.10, should i resent with CC to stable in order to get it into the next 4.10 release? Stefan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-21 16:31 ` Stefan Wahren @ 2017-02-21 20:14 ` Wolfram Sang 2017-02-22 7:20 ` Greg Kroah-Hartman 0 siblings, 1 reply; 20+ messages in thread From: Wolfram Sang @ 2017-02-21 20:14 UTC (permalink / raw) To: Stefan Wahren Cc: Greg Kroah-Hartman, Eric Anholt, Peter Robinson, Martin Sperl, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, Noralf Trønnes, devicetree, linux-i2c, linux-rpi-kernel [-- Attachment #1: Type: text/plain, Size: 1013 bytes --] > >On Thu, Feb 16, 2017 at 09:20:45PM +0000, Stefan Wahren wrote: > >>Since commit e2474541032d ("bcm2835: Fix hang for writing messages > >>larger than 16 bytes") the interrupt handler is prone to a possible > >>NULL pointer dereference. This could happen if an interrupt fires > >>before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs > >>on the RPi 3. Even this is an unexpected behavior the driver must > >>handle that with an error instead of a crash. > >> > >>CC: Noralf Trønnes <noralf@tronnes.org> > >>CC: Martin Sperl <kernel@martin.sperl.org> > >>Reported-by: Peter Robinson <pbrobinson@gmail.com> > >>Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") > >>Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > >Applied to for-next, thanks (will be in 4.11)! > > > > since this patch is too late for 4.10, should i resent with CC to stable in > order to get it into the next 4.10 release? It has the Fixes: tag, that will do. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-21 20:14 ` Wolfram Sang @ 2017-02-22 7:20 ` Greg Kroah-Hartman 2017-02-28 12:42 ` Stefan Wahren 0 siblings, 1 reply; 20+ messages in thread From: Greg Kroah-Hartman @ 2017-02-22 7:20 UTC (permalink / raw) To: Wolfram Sang Cc: Stefan Wahren, Eric Anholt, Peter Robinson, Martin Sperl, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, Noralf Trønnes, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Tue, Feb 21, 2017 at 09:14:03PM +0100, Wolfram Sang wrote: > > > >On Thu, Feb 16, 2017 at 09:20:45PM +0000, Stefan Wahren wrote: > > >>Since commit e2474541032d ("bcm2835: Fix hang for writing messages > > >>larger than 16 bytes") the interrupt handler is prone to a possible > > >>NULL pointer dereference. This could happen if an interrupt fires > > >>before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs > > >>on the RPi 3. Even this is an unexpected behavior the driver must > > >>handle that with an error instead of a crash. > > >> > > >>CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > > >>CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> > > >>Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > >>Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") > > >>Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > > >Applied to for-next, thanks (will be in 4.11)! > > > > > > > since this patch is too late for 4.10, should i resent with CC to stable in > > order to get it into the next 4.10 release? > > It has the Fixes: tag, that will do. But it moves it much lower on my "this needs to get into stable now!" priority list. I'll try to remember this one when it goes by... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference 2017-02-22 7:20 ` Greg Kroah-Hartman @ 2017-02-28 12:42 ` Stefan Wahren [not found] ` <77c2af87-6f0e-4de0-18e9-0aa798f282d0-eS4NqCHxEME@public.gmane.org> 0 siblings, 1 reply; 20+ messages in thread From: Stefan Wahren @ 2017-02-28 12:42 UTC (permalink / raw) To: Greg Kroah-Hartman, Wolfram Sang Cc: Eric Anholt, Peter Robinson, Martin Sperl, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, Noralf Trønnes, devicetree, linux-i2c, linux-rpi-kernel Hi Greg, Am 22.02.2017 um 08:20 schrieb Greg Kroah-Hartman: > On Tue, Feb 21, 2017 at 09:14:03PM +0100, Wolfram Sang wrote: >>>> On Thu, Feb 16, 2017 at 09:20:45PM +0000, Stefan Wahren wrote: >>>>> Since commit e2474541032d ("bcm2835: Fix hang for writing messages >>>>> larger than 16 bytes") the interrupt handler is prone to a possible >>>>> NULL pointer dereference. This could happen if an interrupt fires >>>>> before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs >>>>> on the RPi 3. Even this is an unexpected behavior the driver must >>>>> handle that with an error instead of a crash. >>>>> >>>>> CC: Noralf Trønnes <noralf@tronnes.org> >>>>> CC: Martin Sperl <kernel@martin.sperl.org> >>>>> Reported-by: Peter Robinson <pbrobinson@gmail.com> >>>>> Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") >>>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >>>> Applied to for-next, thanks (will be in 4.11)! >>>> >>> since this patch is too late for 4.10, should i resent with CC to stable in >>> order to get it into the next 4.10 release? >> It has the Fixes: tag, that will do. > But it moves it much lower on my "this needs to get into stable now!" > priority list. I'll try to remember this one when it goes by... > > thanks, > > greg k-h should i resend, since i didn't send you the initial patch? ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <77c2af87-6f0e-4de0-18e9-0aa798f282d0-eS4NqCHxEME@public.gmane.org>]
* Re: [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference [not found] ` <77c2af87-6f0e-4de0-18e9-0aa798f282d0-eS4NqCHxEME@public.gmane.org> @ 2017-02-28 12:47 ` Greg Kroah-Hartman 0 siblings, 0 replies; 20+ messages in thread From: Greg Kroah-Hartman @ 2017-02-28 12:47 UTC (permalink / raw) To: Stefan Wahren Cc: Wolfram Sang, Eric Anholt, Peter Robinson, Martin Sperl, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, Noralf Trønnes, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Tue, Feb 28, 2017 at 01:42:42PM +0100, Stefan Wahren wrote: > Hi Greg, > > Am 22.02.2017 um 08:20 schrieb Greg Kroah-Hartman: > > On Tue, Feb 21, 2017 at 09:14:03PM +0100, Wolfram Sang wrote: > > > > > On Thu, Feb 16, 2017 at 09:20:45PM +0000, Stefan Wahren wrote: > > > > > > Since commit e2474541032d ("bcm2835: Fix hang for writing messages > > > > > > larger than 16 bytes") the interrupt handler is prone to a possible > > > > > > NULL pointer dereference. This could happen if an interrupt fires > > > > > > before curr_msg is set by bcm2835_i2c_xfer_msg() and randomly occurs > > > > > > on the RPi 3. Even this is an unexpected behavior the driver must > > > > > > handle that with an error instead of a crash. > > > > > > > > > > > > CC: Noralf Trønnes <noralf-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> > > > > > > CC: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> > > > > > > Reported-by: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > > > > Fixes: e2474541032d ("bcm2835: Fix hang for writing messages larger than 16 bytes") > > > > > > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > > > > > Applied to for-next, thanks (will be in 4.11)! > > > > > > > > > since this patch is too late for 4.10, should i resent with CC to stable in > > > > order to get it into the next 4.10 release? > > > It has the Fixes: tag, that will do. > > But it moves it much lower on my "this needs to get into stable now!" > > priority list. I'll try to remember this one when it goes by... > > > > thanks, > > > > greg k-h > > should i resend, since i didn't send you the initial patch? Sure! -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 2/3] ARM64: bcm2837-rpi: remove link to bcm2835-rpi.dtsi 2017-02-16 21:20 [PATCH RFC 0/3] ARM64: bcm2837-rpi: Fix random crashes caused by i2c Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference Stefan Wahren @ 2017-02-16 21:20 ` Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control Stefan Wahren 2 siblings, 0 replies; 20+ messages in thread From: Stefan Wahren @ 2017-02-16 21:20 UTC (permalink / raw) To: Eric Anholt, Wolfram Sang, Peter Robinson Cc: Martin Sperl, Noralf Trønnes, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel, devicetree, linux-i2c, Stefan Wahren Before we can make any Raspberry Pi specific changes we need a local copy of bcm2835-rpi.dtsi instead of a link. After that we include this file and remove the unnecessary link. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi | 1 - arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts | 2 +- arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi | 89 ++++++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) delete mode 120000 arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi create mode 100644 arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi diff --git a/arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi b/arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi deleted file mode 120000 index 3937b77..0000000 --- a/arch/arm64/boot/dts/broadcom/bcm2835-rpi.dtsi +++ /dev/null @@ -1 +0,0 @@ -../../../../arm/boot/dts/bcm2835-rpi.dtsi \ No newline at end of file diff --git a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts index c309633..e80db7e 100644 --- a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts +++ b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts @@ -1,6 +1,6 @@ /dts-v1/; #include "bcm2837.dtsi" -#include "bcm2835-rpi.dtsi" +#include "bcm2837-rpi.dtsi" #include "bcm283x-rpi-smsc9514.dtsi" #include "bcm283x-rpi-usb-host.dtsi" diff --git a/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi b/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi new file mode 100644 index 0000000..6ddf7df --- /dev/null +++ b/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi @@ -0,0 +1,89 @@ +#include <dt-bindings/power/raspberrypi-power.h> + +/ { + memory { + device_type = "memory"; + reg = <0 0x10000000>; + }; + + leds { + compatible = "gpio-leds"; + + act { + label = "ACT"; + default-state = "keep"; + linux,default-trigger = "heartbeat"; + }; + }; + + soc { + firmware: firmware { + compatible = "raspberrypi,bcm2835-firmware"; + mboxes = <&mailbox>; + }; + + power: power { + compatible = "raspberrypi,bcm2835-power"; + firmware = <&firmware>; + #power-domain-cells = <1>; + }; + }; +}; + +&gpio { + pinctrl-names = "default"; + + gpioout: gpioout { + brcm,pins = <6>; + brcm,function = <BCM2835_FSEL_GPIO_OUT>; + }; + + alt0: alt0 { + brcm,pins = <4 5 7 8 9 10 11 14 15>; + brcm,function = <BCM2835_FSEL_ALT0>; + }; +}; + +&i2c0 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c0_gpio0>; + status = "okay"; + clock-frequency = <100000>; +}; + +&i2c1 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c1_gpio2>; + status = "okay"; + clock-frequency = <100000>; +}; + +&i2c2 { + status = "okay"; +}; + +&sdhci { + pinctrl-names = "default"; + pinctrl-0 = <&emmc_gpio48>; + status = "okay"; + bus-width = <4>; +}; + +&pwm { + pinctrl-names = "default"; + pinctrl-0 = <&pwm0_gpio40 &pwm1_gpio45>; + status = "okay"; +}; + +&usb { + power-domains = <&power RPI_POWER_DOMAIN_USB>; +}; + +&v3d { + power-domains = <&power RPI_POWER_DOMAIN_V3D>; +}; + +&hdmi { + power-domains = <&power RPI_POWER_DOMAIN_HDMI>; + status = "okay"; +}; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control 2017-02-16 21:20 [PATCH RFC 0/3] ARM64: bcm2837-rpi: Fix random crashes caused by i2c Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 2/3] ARM64: bcm2837-rpi: remove link to bcm2835-rpi.dtsi Stefan Wahren @ 2017-02-16 21:20 ` Stefan Wahren 2017-02-27 20:56 ` Eric Anholt 2 siblings, 1 reply; 20+ messages in thread From: Stefan Wahren @ 2017-02-16 21:20 UTC (permalink / raw) To: Eric Anholt, Wolfram Sang, Peter Robinson Cc: Martin Sperl, Noralf Trønnes, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel, devicetree, linux-i2c, Stefan Wahren According to the firmware DT blob [1] the RPi 3 uses different pins for i2c0 and pwm. [1] - https://github.com/raspberrypi/firmware/blob/master/extra/dt-blob.dts Fixes: 9d56c22a7861 ("ARM: bcm2835: Add devicetree for the Raspberry Pi 3.") Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi b/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi index 6ddf7df..9f3f3ed 100644 --- a/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi +++ b/arch/arm64/boot/dts/broadcom/bcm2837-rpi.dtsi @@ -46,7 +46,7 @@ &i2c0 { pinctrl-names = "default"; - pinctrl-0 = <&i2c0_gpio0>; + pinctrl-0 = <&i2c0_gpio44>; status = "okay"; clock-frequency = <100000>; }; @@ -71,7 +71,7 @@ &pwm { pinctrl-names = "default"; - pinctrl-0 = <&pwm0_gpio40 &pwm1_gpio45>; + pinctrl-0 = <&pwm0_gpio40 &pwm1_gpio41>; status = "okay"; }; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control 2017-02-16 21:20 ` [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control Stefan Wahren @ 2017-02-27 20:56 ` Eric Anholt [not found] ` <87r32j8ial.fsf-omZaPlIz5HhaEpDpdNBo/KxOck334EZe@public.gmane.org> 0 siblings, 1 reply; 20+ messages in thread From: Eric Anholt @ 2017-02-27 20:56 UTC (permalink / raw) To: Wolfram Sang, Peter Robinson Cc: Martin Sperl, Noralf Trønnes, Catalin Marinas, Will Deacon, Rob Herring, Frank Rowand, Florian Fainelli, linux-rpi-kernel, devicetree, linux-i2c, Stefan Wahren [-- Attachment #1: Type: text/plain, Size: 1251 bytes --] Stefan Wahren <stefan.wahren@i2se.com> writes: > According to the firmware DT blob [1] the RPi 3 uses different > pins for i2c0 and pwm. > > [1] - https://github.com/raspberrypi/firmware/blob/master/extra/dt-blob.dts > > Fixes: 9d56c22a7861 ("ARM: bcm2835: Add devicetree for the Raspberry Pi 3.") > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> Interesting. The dt-blob does, I think, basically reflect the state that the pins will be in when the firmware hands off to us. Note that the firmware also, at boot, sets ID_SDA/SCL to function 0 (i2c0), probes for a HAT ROM and saves it, then restores those pins to whatever mux they had before (presumably the dt-blob pin_config values). Linux doesn't have any HAT probing code itself, so leaving pins 0/1 as GPIO input (most boards) or i2c0 (rev2) should be fine. I think this patch should effectively be a no-op, unless you've enabled the pwm node. My only suggestion for change is: These pin configurations are board-specific, so could we just move them into the specific board files, instead? Basically leave bcm2835-rpi.dtsi as the "generic configuration of bcm283x for the rpi firmware" rather than "things common to bcm2835/6 with rpi firmware but not bcm2837 with rpi firmware". [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <87r32j8ial.fsf-omZaPlIz5HhaEpDpdNBo/KxOck334EZe@public.gmane.org>]
* Re: [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control [not found] ` <87r32j8ial.fsf-omZaPlIz5HhaEpDpdNBo/KxOck334EZe@public.gmane.org> @ 2017-02-27 21:13 ` Stefan Wahren 2017-02-27 22:36 ` Eric Anholt 0 siblings, 1 reply; 20+ messages in thread From: Stefan Wahren @ 2017-02-27 21:13 UTC (permalink / raw) To: Peter Robinson, Eric Anholt, Wolfram Sang Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Florian Fainelli, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Noralf Trønnes, devicetree-u79uwXL29TY76Z2rM5mHXA > Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 27. Februar 2017 um 21:56 geschrieben: > > > Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes: > > > According to the firmware DT blob [1] the RPi 3 uses different > > pins for i2c0 and pwm. > > > > [1] - https://github.com/raspberrypi/firmware/blob/master/extra/dt-blob.dts > > > > Fixes: 9d56c22a7861 ("ARM: bcm2835: Add devicetree for the Raspberry Pi 3.") > > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> > > Interesting. The dt-blob does, I think, basically reflect the state > that the pins will be in when the firmware hands off to us. Note that > the firmware also, at boot, sets ID_SDA/SCL to function 0 (i2c0), probes > for a HAT ROM and saves it, then restores those pins to whatever mux > they had before (presumably the dt-blob pin_config values). > > Linux doesn't have any HAT probing code itself, so leaving pins 0/1 as > GPIO input (most boards) or i2c0 (rev2) should be fine. I think this > patch should effectively be a no-op, unless you've enabled the pwm node. > > My only suggestion for change is: These pin configurations are > board-specific, so could we just move them into the specific board > files, instead? Yes, that would be better. In case of pin configurations do you mean only those affected by this patch or all pin configurations? > Basically leave bcm2835-rpi.dtsi as the "generic > configuration of bcm283x for the rpi firmware" rather than "things > common to bcm2835/6 with rpi firmware but not bcm2837 with rpi > firmware". If this our intension, shouldn't we rename it to bcm283x-rpi.dtsi? Ah, now i feel the pain about having symlinks from arm64/dts to arm/dts. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control 2017-02-27 21:13 ` Stefan Wahren @ 2017-02-27 22:36 ` Eric Anholt 0 siblings, 0 replies; 20+ messages in thread From: Eric Anholt @ 2017-02-27 22:36 UTC (permalink / raw) To: Stefan Wahren, Peter Robinson, Wolfram Sang Cc: Will Deacon, Frank Rowand, Martin Sperl, Rob Herring, linux-i2c, Catalin Marinas, Florian Fainelli, linux-rpi-kernel, Noralf Trønnes, devicetree [-- Attachment #1: Type: text/plain, Size: 1809 bytes --] Stefan Wahren <stefan.wahren@i2se.com> writes: >> Eric Anholt <eric@anholt.net> hat am 27. Februar 2017 um 21:56 geschrieben: >> >> >> Stefan Wahren <stefan.wahren@i2se.com> writes: >> >> > According to the firmware DT blob [1] the RPi 3 uses different >> > pins for i2c0 and pwm. >> > >> > [1] - https://github.com/raspberrypi/firmware/blob/master/extra/dt-blob.dts >> > >> > Fixes: 9d56c22a7861 ("ARM: bcm2835: Add devicetree for the Raspberry Pi 3.") >> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >> >> Interesting. The dt-blob does, I think, basically reflect the state >> that the pins will be in when the firmware hands off to us. Note that >> the firmware also, at boot, sets ID_SDA/SCL to function 0 (i2c0), probes >> for a HAT ROM and saves it, then restores those pins to whatever mux >> they had before (presumably the dt-blob pin_config values). >> >> Linux doesn't have any HAT probing code itself, so leaving pins 0/1 as >> GPIO input (most boards) or i2c0 (rev2) should be fine. I think this >> patch should effectively be a no-op, unless you've enabled the pwm node. >> >> My only suggestion for change is: These pin configurations are >> board-specific, so could we just move them into the specific board >> files, instead? > > Yes, that would be better. In case of pin configurations do you mean > only those affected by this patch or all pin configurations? I was just thinking about the ones you're changing in this patch. >> Basically leave bcm2835-rpi.dtsi as the "generic >> configuration of bcm283x for the rpi firmware" rather than "things >> common to bcm2835/6 with rpi firmware but not bcm2837 with rpi >> firmware". > > If this our intension, shouldn't we rename it to bcm283x-rpi.dtsi? That sounds good to me. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-02-28 12:47 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-16 21:20 [PATCH RFC 0/3] ARM64: bcm2837-rpi: Fix random crashes caused by i2c Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 1/3] i2c: bcm2835: Avoid possible NULL ptr dereference Stefan Wahren [not found] ` <1487280047-29608-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org> 2017-02-18 18:34 ` Noralf Trønnes [not found] ` <48907a31-eaa6-27e2-633f-d36de521e868-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> 2017-02-20 19:40 ` Stefan Wahren [not found] ` <1983281189.110224.1487619653742-7tX72C7vayboQLBSYMtkGA@public.gmane.org> 2017-02-20 21:22 ` Noralf Trønnes [not found] ` <5b14ed6d-4db8-abf7-ceba-ef46534b6023-L59+Z2yzLopAfugRpC6u6w@public.gmane.org> 2017-02-21 8:58 ` Stefan Wahren [not found] ` <31b96f29-e369-546a-6270-266daea71062-eS4NqCHxEME@public.gmane.org> 2017-02-21 11:54 ` kernel-TqfNSX0MhmxHKSADF0wUEw 2017-02-21 15:07 ` Michael Zoran 2017-02-21 15:37 ` Stefan Wahren 2017-02-20 18:22 ` Wolfram Sang 2017-02-21 16:31 ` Stefan Wahren 2017-02-21 20:14 ` Wolfram Sang 2017-02-22 7:20 ` Greg Kroah-Hartman 2017-02-28 12:42 ` Stefan Wahren [not found] ` <77c2af87-6f0e-4de0-18e9-0aa798f282d0-eS4NqCHxEME@public.gmane.org> 2017-02-28 12:47 ` Greg Kroah-Hartman 2017-02-16 21:20 ` [PATCH RFC 2/3] ARM64: bcm2837-rpi: remove link to bcm2835-rpi.dtsi Stefan Wahren 2017-02-16 21:20 ` [PATCH RFC 3/3] ARM64: bcm2837-rpi: Fix pwm and i2c pin control Stefan Wahren 2017-02-27 20:56 ` Eric Anholt [not found] ` <87r32j8ial.fsf-omZaPlIz5HhaEpDpdNBo/KxOck334EZe@public.gmane.org> 2017-02-27 21:13 ` Stefan Wahren 2017-02-27 22:36 ` Eric Anholt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.