Re: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
Hey Dong Aisheng, > Besides above comments: > > Reviewed-by: Dong Aisheng Are you maybe interested in maintaining this driver? We are seriously needing one with experience. Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
On Wed, Apr 17, 2019 at 01:59:34AM +, Anson Huang wrote: > The way of getting private imx_i2c_struct in i2c_imx_clk_notifier_call() > is incorrect, should use clk_change_nb element to get correct address > and avoid below kernel dump during POST_RATE_CHANGE notify by clk > framework: > > Unable to handle kernel paging request at virtual address 03ef1488 > pgd = (ptrval) > [03ef1488] *pgd= > Internal error: Oops: 5 [#1] PREEMPT SMP ARM > Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > Workqueue: events reduce_bus_freq_handler > PC is at i2c_imx_set_clk+0x10/0xb8 > LR is at i2c_imx_clk_notifier_call+0x20/0x28 > pc : [<806a893c>]lr : [<806a8a04>]psr: a0080013 > sp : bf399dd8 ip : bf3432ac fp : bf7c1dc0 > r10: 0002 r9 : r8 : > r7 : 03ef1480 r6 : bf399e50 r5 : r4 : > r3 : bf025300 r2 : bf399e50 r1 : 00b71b00 r0 : bf399be8 > Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 4e03004a DAC: 0051 > Process kworker/2:1 (pid: 38, stack limit = 0x(ptrval)) > Stack: (0xbf399dd8 to 0xbf39a000) > 9dc0: 806a89e4 > 9de0: bf399e50 0002 806a8a04 806a89e4 80142900 > 9e00: bf34ef18 bf34ef04 bf399e50 80142d84 bf399e6c > 9e20: bf34ef00 80f214c4 bf025300 0002 80f08d08 bf017480 80142df0 > 9e40: 80166ed8 80c27638 8045de58 bf352340 03ef1480 00b71b00 0f82e242 > 9e60: bf025300 0002 03ef1480 80f60e5c 0001 8045edf0 0002 8045eb08 > 9e80: bf025300 0002 03ef1480 8045ee10 03ef1480 8045eb08 bf01be40 0002 > 9ea0: 03ef1480 8045ee10 07de2900 8045eb08 bf01b780 0002 07de2900 8045ee10 > 9ec0: 80c27898 bf399ee4 bf020a80 0002 1f78a400 8045ee10 80f60e5c 80460514 > 9ee0: 80f60e5c bf01b600 bf01b480 80460460 0f82e242 bf383a80 bf383a00 80f60e5c > 9f00: bf7c1dc0 80f60e70 80460564 80f60df0 80f60d24 80f60df0 8011e72c > 9f20: 80f60df0 80f60e6c bf7c4f00 8011e7ac bf274000 8013bd84 > 9f40: bf7c1dd8 80f03d00 bf274000 bf7c1dc0 bf274014 bf7c1dd8 80f03d00 bf398000 > 9f60: 0008 8013bfb4 bf25d100 bf25d0c0 bf274000 8013bf88 > 9f80: bf25d11c bf0cfebc 8014140c bf25d0c0 801412ec > 9fa0: 801010e8 > 9fc0: > 9fe0: 0013 > [<806a893c>] (i2c_imx_set_clk) from [<806a8a04>] > (i2c_imx_clk_notifier_call+0x20/0x28) > [<806a8a04>] (i2c_imx_clk_notifier_call) from [<80142900>] > (notifier_call_chain+0x44/0x84) > [<80142900>] (notifier_call_chain) from [<80142d84>] > (__srcu_notifier_call_chain+0x44/0x98) > [<80142d84>] (__srcu_notifier_call_chain) from [<80142df0>] > (srcu_notifier_call_chain+0x18/0x20) > [<80142df0>] (srcu_notifier_call_chain) from [<8045de58>] > (__clk_notify+0x78/0xa4) > [<8045de58>] (__clk_notify) from [<8045edf0>] (__clk_recalc_rates+0x60/0xb4) > [<8045edf0>] (__clk_recalc_rates) from [<8045ee10>] > (__clk_recalc_rates+0x80/0xb4) > Code: e92d40f8 e5903298 e59072a0 e1530001 (e5975008) Applied to for-current, thanks! signature.asc Description: PGP signature
RE: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
Hi, Aisheng Best Regards! Anson Huang > -Original Message- > From: Aisheng Dong > Sent: Wednesday, April 17, 2019 11:13 AM > To: Anson Huang ; shawn...@kernel.org; > s.ha...@pengutronix.de; ker...@pengutronix.de; feste...@gmail.com; > wsa+rene...@sang-engineering.com; u.kleine-koe...@pengutronix.de; > e...@deif.com; li...@rempel-privat.de; mo...@codeaurora.org; Laurentiu > Tudor ; p...@axentia.se; linux- > i...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- > ker...@vger.kernel.org > Cc: dl-linux-imx > Subject: RE: [PATCH] i2c: imx: correct the method of getting private data in > notifier_call > > > From: Anson Huang > > Sent: Wednesday, April 17, 2019 10:00 AM > > > > The way of getting private imx_i2c_struct in > > i2c_imx_clk_notifier_call() is incorrect, should use clk_change_nb > > element to get correct address and avoid below kernel dump during > > POST_RATE_CHANGE notify by clk > > framework: > > > > Unable to handle kernel paging request at virtual address 03ef1488 pgd > > = > > (ptrval) [03ef1488] *pgd= Internal error: Oops: 5 [#1] PREEMPT > > SMP ARM Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > > Workqueue: events reduce_bus_freq_handler PC is at > > i2c_imx_set_clk+0x10/0xb8 LR is at i2c_imx_clk_notifier_call+0x20/0x28 > > pc : [<806a893c>]lr : [<806a8a04>]psr: a0080013 > > sp : bf399dd8 ip : bf3432ac fp : bf7c1dc0 > > r10: 0002 r9 : r8 : > > r7 : 03ef1480 r6 : bf399e50 r5 : r4 : > > r3 : bf025300 r2 : bf399e50 r1 : 00b71b00 r0 : bf399be8 > > Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > > Control: 10c5387d Table: 4e03004a DAC: 0051 Process kworker/2:1 > > (pid: 38, stack limit = 0x(ptrval)) > > Stack: (0xbf399dd8 to 0xbf39a000) > > 9dc0: > > 806a89e4 > > 9de0: bf399e50 0002 806a8a04 806a89e4 80142900 > > > > 9e00: bf34ef18 bf34ef04 bf399e50 80142d84 > > bf399e6c > > 9e20: bf34ef00 80f214c4 bf025300 0002 80f08d08 bf017480 > > 80142df0 > > 9e40: 80166ed8 80c27638 8045de58 bf352340 03ef1480 > > 00b71b00 0f82e242 > > 9e60: bf025300 0002 03ef1480 80f60e5c 0001 8045edf0 > > 0002 8045eb08 > > 9e80: bf025300 0002 03ef1480 8045ee10 03ef1480 8045eb08 > > bf01be40 0002 > > 9ea0: 03ef1480 8045ee10 07de2900 8045eb08 bf01b780 0002 > > 07de2900 8045ee10 > > 9ec0: 80c27898 bf399ee4 bf020a80 0002 1f78a400 8045ee10 80f60e5c > > 80460514 > > 9ee0: 80f60e5c bf01b600 bf01b480 80460460 0f82e242 bf383a80 bf383a00 > > 80f60e5c > > 9f00: bf7c1dc0 80f60e70 80460564 80f60df0 80f60d24 80f60df0 > > 8011e72c > > 9f20: 80f60df0 80f60e6c bf7c4f00 8011e7ac bf274000 > > 8013bd84 > > 9f40: bf7c1dd8 80f03d00 bf274000 bf7c1dc0 bf274014 bf7c1dd8 80f03d00 > > bf398000 > > 9f60: 0008 8013bfb4 bf25d100 bf25d0c0 > > bf274000 8013bf88 > > 9f80: bf25d11c bf0cfebc 8014140c bf25d0c0 801412ec > > > > 9fa0: 801010e8 > > > > 9fc0: > > > > 9fe0: 0013 > > [<806a893c>] (i2c_imx_set_clk) from [<806a8a04>] > > (i2c_imx_clk_notifier_call+0x20/0x28) > > [<806a8a04>] (i2c_imx_clk_notifier_call) from [<80142900>] > > (notifier_call_chain+0x44/0x84) [<80142900>] (notifier_call_chain) > > from [<80142d84>] (__srcu_notifier_call_chain+0x44/0x98) > > [<80142d84>] (__srcu_notifier_call_chain) from [<80142df0>] > > (srcu_notifier_call_chain+0x18/0x20) > > [<80142df0>] (srcu_notifier_call_chain) from [<8045de58>] > > (__clk_notify+0x78/0xa4) [<8045de58>] (__clk_notify) from [<8045edf0>] > > (__clk_recalc_rates+0x60/0xb4) [<8045edf0>] (__clk_recalc_rates) from > > [<8045ee10>] (__clk_recalc_rates+0x80/0xb4) > > Code: e92d40f8 e5903298 e59072a0 e1530001 (e5975008) ---[ end trace > > fc7f5514b97b6cbb ]--- > > > > Fixes: 90ad2cbe88c2("i2c: imx: use clk notifier for rate changes") > > Signed-off-by: Anson Huang > > Please also provide how to reproduce it. > And it seems not a new issue, should we CC stable? This issue is NOT easy to reproduce unless creating a dedicated test case, upstream kernel normally does NOT go into this path, I met this issue during internal bus-freq test, and it should be a real issue. Code review also can find this issue I think. Yes, if it is indeed a real issue, we should CC stable. Anson. > > Regards > Dong Aisheng > > > --- > > drivers/i2c/busses/i2c-imx.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-imx.c > > b/drivers/i2c/busses/i2c-imx.c index > > c0c3043..fd70b11 100644 > > --- a/drivers/i2c/busses/i2c-imx.c > > +++ b/drivers/i2c/busses/i2c-imx.c > > @@ -515,9 +515,9 @@ static int i2c_imx_clk_notifier_call(struct > > notifier_block
RE: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
[...] > > > > Fixes: 90ad2cbe88c2("i2c: imx: use clk notifier for rate changes") > > Signed-off-by: Anson Huang > > Please also provide how to reproduce it. > And it seems not a new issue, should we CC stable? Besides above comments: Reviewed-by: Dong Aisheng Regards Dong Aisheng > > Regards > Dong Aisheng > > > --- > > drivers/i2c/busses/i2c-imx.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-imx.c > > b/drivers/i2c/busses/i2c-imx.c index > > c0c3043..fd70b11 100644 > > --- a/drivers/i2c/busses/i2c-imx.c > > +++ b/drivers/i2c/busses/i2c-imx.c > > @@ -515,9 +515,9 @@ static int i2c_imx_clk_notifier_call(struct > > notifier_block *nb, > > unsigned long action, void *data) { > > struct clk_notifier_data *ndata = data; > > - struct imx_i2c_struct *i2c_imx = container_of(>clk, > > + struct imx_i2c_struct *i2c_imx = container_of(nb, > > struct imx_i2c_struct, > > - clk); > > + clk_change_nb); > > > > if (action & POST_RATE_CHANGE) > > i2c_imx_set_clk(i2c_imx, ndata->new_rate); > > -- > > 2.7.4
RE: [PATCH] i2c: imx: correct the method of getting private data in notifier_call
> From: Anson Huang > Sent: Wednesday, April 17, 2019 10:00 AM > > The way of getting private imx_i2c_struct in i2c_imx_clk_notifier_call() is > incorrect, should use clk_change_nb element to get correct address and avoid > below kernel dump during POST_RATE_CHANGE notify by clk > framework: > > Unable to handle kernel paging request at virtual address 03ef1488 pgd = > (ptrval) [03ef1488] *pgd= Internal error: Oops: 5 [#1] PREEMPT SMP > ARM Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > Workqueue: events reduce_bus_freq_handler PC is at > i2c_imx_set_clk+0x10/0xb8 LR is at i2c_imx_clk_notifier_call+0x20/0x28 > pc : [<806a893c>]lr : [<806a8a04>]psr: a0080013 > sp : bf399dd8 ip : bf3432ac fp : bf7c1dc0 > r10: 0002 r9 : r8 : > r7 : 03ef1480 r6 : bf399e50 r5 : r4 : > r3 : bf025300 r2 : bf399e50 r1 : 00b71b00 r0 : bf399be8 > Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 4e03004a DAC: 0051 Process kworker/2:1 > (pid: 38, stack limit = 0x(ptrval)) > Stack: (0xbf399dd8 to 0xbf39a000) > 9dc0: > 806a89e4 > 9de0: bf399e50 0002 806a8a04 806a89e4 80142900 > > 9e00: bf34ef18 bf34ef04 bf399e50 80142d84 > bf399e6c > 9e20: bf34ef00 80f214c4 bf025300 0002 80f08d08 bf017480 > 80142df0 > 9e40: 80166ed8 80c27638 8045de58 bf352340 03ef1480 > 00b71b00 0f82e242 > 9e60: bf025300 0002 03ef1480 80f60e5c 0001 8045edf0 > 0002 8045eb08 > 9e80: bf025300 0002 03ef1480 8045ee10 03ef1480 8045eb08 > bf01be40 0002 > 9ea0: 03ef1480 8045ee10 07de2900 8045eb08 bf01b780 0002 > 07de2900 8045ee10 > 9ec0: 80c27898 bf399ee4 bf020a80 0002 1f78a400 8045ee10 80f60e5c > 80460514 > 9ee0: 80f60e5c bf01b600 bf01b480 80460460 0f82e242 bf383a80 bf383a00 > 80f60e5c > 9f00: bf7c1dc0 80f60e70 80460564 80f60df0 80f60d24 80f60df0 > 8011e72c > 9f20: 80f60df0 80f60e6c bf7c4f00 8011e7ac bf274000 > 8013bd84 > 9f40: bf7c1dd8 80f03d00 bf274000 bf7c1dc0 bf274014 bf7c1dd8 80f03d00 > bf398000 > 9f60: 0008 8013bfb4 bf25d100 bf25d0c0 > bf274000 8013bf88 > 9f80: bf25d11c bf0cfebc 8014140c bf25d0c0 801412ec > > 9fa0: 801010e8 > > 9fc0: > > 9fe0: 0013 > [<806a893c>] (i2c_imx_set_clk) from [<806a8a04>] > (i2c_imx_clk_notifier_call+0x20/0x28) > [<806a8a04>] (i2c_imx_clk_notifier_call) from [<80142900>] > (notifier_call_chain+0x44/0x84) [<80142900>] (notifier_call_chain) from > [<80142d84>] (__srcu_notifier_call_chain+0x44/0x98) > [<80142d84>] (__srcu_notifier_call_chain) from [<80142df0>] > (srcu_notifier_call_chain+0x18/0x20) > [<80142df0>] (srcu_notifier_call_chain) from [<8045de58>] > (__clk_notify+0x78/0xa4) [<8045de58>] (__clk_notify) from [<8045edf0>] > (__clk_recalc_rates+0x60/0xb4) [<8045edf0>] (__clk_recalc_rates) from > [<8045ee10>] (__clk_recalc_rates+0x80/0xb4) > Code: e92d40f8 e5903298 e59072a0 e1530001 (e5975008) ---[ end trace > fc7f5514b97b6cbb ]--- > > Fixes: 90ad2cbe88c2("i2c: imx: use clk notifier for rate changes") > Signed-off-by: Anson Huang Please also provide how to reproduce it. And it seems not a new issue, should we CC stable? Regards Dong Aisheng > --- > drivers/i2c/busses/i2c-imx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index > c0c3043..fd70b11 100644 > --- a/drivers/i2c/busses/i2c-imx.c > +++ b/drivers/i2c/busses/i2c-imx.c > @@ -515,9 +515,9 @@ static int i2c_imx_clk_notifier_call(struct > notifier_block *nb, >unsigned long action, void *data) { > struct clk_notifier_data *ndata = data; > - struct imx_i2c_struct *i2c_imx = container_of(>clk, > + struct imx_i2c_struct *i2c_imx = container_of(nb, > struct imx_i2c_struct, > - clk); > + clk_change_nb); > > if (action & POST_RATE_CHANGE) > i2c_imx_set_clk(i2c_imx, ndata->new_rate); > -- > 2.7.4