| Message ID | 20231206090641.18849-1-jensenhuang@friendlyarm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i2c: rk3x: fix potential spinlock recursion on poll | expand |
Am Mittwoch, 6. Dezember 2023, 10:06:40 CET schrieb Jensen Huang: > Possible deadlock scenario (on reboot): > rk3x_i2c_xfer_common(polling) > -> rk3x_i2c_wait_xfer_poll() > -> rk3x_i2c_irq(0, i2c); > --> spin_lock(&i2c->lock); > ... > <rk3x i2c interrupt> > -> rk3x_i2c_irq(0, i2c); > --> spin_lock(&i2c->lock); (deadlock here) > > Store the IRQ number and disable/enable it around the polling transfer. > This patch has been tested on NanoPC-T4. > > Signed-off-by: Jensen Huang <jensenhuang@friendlyarm.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> > --- > drivers/i2c/busses/i2c-rk3x.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c > index a044ca0c35a1..94514637c3bd 100644 > --- a/drivers/i2c/busses/i2c-rk3x.c > +++ b/drivers/i2c/busses/i2c-rk3x.c > @@ -200,6 +200,7 @@ struct rk3x_i2c { > struct clk *clk; > struct clk *pclk; > struct notifier_block clk_rate_nb; > + int irq; > > /* Settings */ > struct i2c_timings t; > @@ -1087,13 +1088,18 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap, > > spin_unlock_irqrestore(&i2c->lock, flags); > > - rk3x_i2c_start(i2c); > - > if (!polling) { > + rk3x_i2c_start(i2c); > + > timeout = wait_event_timeout(i2c->wait, !i2c->busy, > msecs_to_jiffies(WAIT_TIMEOUT)); > } else { > + disable_irq(i2c->irq); > + rk3x_i2c_start(i2c); > + > timeout = rk3x_i2c_wait_xfer_poll(i2c); > + > + enable_irq(i2c->irq); > } > > spin_lock_irqsave(&i2c->lock, flags); > @@ -1310,6 +1316,8 @@ static int rk3x_i2c_probe(struct platform_device *pdev) > return ret; > } > > + i2c->irq = irq; > + > platform_set_drvdata(pdev, i2c); > > if (i2c->soc_data->calc_timings == rk3x_i2c_v0_calc_timings) { >
Hi Jensen, On Wed, Dec 06, 2023 at 05:06:40PM +0800, Jensen Huang wrote: > Possible deadlock scenario (on reboot): > rk3x_i2c_xfer_common(polling) > -> rk3x_i2c_wait_xfer_poll() > -> rk3x_i2c_irq(0, i2c); > --> spin_lock(&i2c->lock); > ... > <rk3x i2c interrupt> > -> rk3x_i2c_irq(0, i2c); > --> spin_lock(&i2c->lock); (deadlock here) > > Store the IRQ number and disable/enable it around the polling transfer. > This patch has been tested on NanoPC-T4. > > Signed-off-by: Jensen Huang <jensenhuang@friendlyarm.com> Looks good, Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
Hi Jensen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on rockchip/for-next]
[also build test WARNING on linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jensen-Huang/i2c-rk3x-fix-potential-spinlock-recursion-on-poll/20231206-170807
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
patch link: https://lore.kernel.org/r/20231206090641.18849-1-jensenhuang%40friendlyarm.com
patch subject: [PATCH] i2c: rk3x: fix potential spinlock recursion on poll
config: arc-randconfig-r081-20231206 (https://download.01.org/0day-ci/archive/20231207/202312070421.Lccd0PSI-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/202312070421.Lccd0PSI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312070421.Lccd0PSI-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-rk3x.c:223: warning: Function parameter or member 'irq' not described in 'rk3x_i2c'
vim +223 drivers/i2c/busses/i2c-rk3x.c
c41aa3ce938b68 Max Schwarz 2014-06-11 171
0a6ad2f95f36bf David Wu 2016-05-16 172 /**
0a6ad2f95f36bf David Wu 2016-05-16 173 * struct rk3x_i2c - private data of the controller
0a6ad2f95f36bf David Wu 2016-05-16 174 * @adap: corresponding I2C adapter
0a6ad2f95f36bf David Wu 2016-05-16 175 * @dev: device for this controller
0a6ad2f95f36bf David Wu 2016-05-16 176 * @soc_data: related soc data struct
0a6ad2f95f36bf David Wu 2016-05-16 177 * @regs: virtual memory area
7e086c3fc2df09 David Wu 2016-05-16 178 * @clk: function clk for rk3399 or function & Bus clks for others
7e086c3fc2df09 David Wu 2016-05-16 179 * @pclk: Bus clk for rk3399
0a6ad2f95f36bf David Wu 2016-05-16 180 * @clk_rate_nb: i2c clk rate change notify
0a6ad2f95f36bf David Wu 2016-05-16 181 * @t: I2C known timing information
0a6ad2f95f36bf David Wu 2016-05-16 182 * @lock: spinlock for the i2c bus
0a6ad2f95f36bf David Wu 2016-05-16 183 * @wait: the waitqueue to wait for i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 184 * @busy: the condition for the event to wait for
0a6ad2f95f36bf David Wu 2016-05-16 185 * @msg: current i2c message
0a6ad2f95f36bf David Wu 2016-05-16 186 * @addr: addr of i2c slave device
0a6ad2f95f36bf David Wu 2016-05-16 187 * @mode: mode of i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 188 * @is_last_msg: flag determines whether it is the last msg in this transfer
0a6ad2f95f36bf David Wu 2016-05-16 189 * @state: state of i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 190 * @processed: byte length which has been send or received
0a6ad2f95f36bf David Wu 2016-05-16 191 * @error: error code for i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 192 */
c41aa3ce938b68 Max Schwarz 2014-06-11 193 struct rk3x_i2c {
c41aa3ce938b68 Max Schwarz 2014-06-11 194 struct i2c_adapter adap;
c41aa3ce938b68 Max Schwarz 2014-06-11 195 struct device *dev;
d032a2eb2e3b9f Julia Lawall 2018-01-02 196 const struct rk3x_i2c_soc_data *soc_data;
c41aa3ce938b68 Max Schwarz 2014-06-11 197
c41aa3ce938b68 Max Schwarz 2014-06-11 198 /* Hardware resources */
c41aa3ce938b68 Max Schwarz 2014-06-11 199 void __iomem *regs;
c41aa3ce938b68 Max Schwarz 2014-06-11 200 struct clk *clk;
7e086c3fc2df09 David Wu 2016-05-16 201 struct clk *pclk;
249051f49907e7 Max Schwarz 2014-11-20 202 struct notifier_block clk_rate_nb;
58bb392dd4c7fc Jensen Huang 2023-12-06 203 int irq;
c41aa3ce938b68 Max Schwarz 2014-06-11 204
c41aa3ce938b68 Max Schwarz 2014-06-11 205 /* Settings */
1ab92956d4aac5 David Wu 2016-03-17 206 struct i2c_timings t;
c41aa3ce938b68 Max Schwarz 2014-06-11 207
c41aa3ce938b68 Max Schwarz 2014-06-11 208 /* Synchronization & notification */
c41aa3ce938b68 Max Schwarz 2014-06-11 209 spinlock_t lock;
c41aa3ce938b68 Max Schwarz 2014-06-11 210 wait_queue_head_t wait;
c41aa3ce938b68 Max Schwarz 2014-06-11 211 bool busy;
c41aa3ce938b68 Max Schwarz 2014-06-11 212
c41aa3ce938b68 Max Schwarz 2014-06-11 213 /* Current message */
c41aa3ce938b68 Max Schwarz 2014-06-11 214 struct i2c_msg *msg;
c41aa3ce938b68 Max Schwarz 2014-06-11 215 u8 addr;
c41aa3ce938b68 Max Schwarz 2014-06-11 216 unsigned int mode;
c41aa3ce938b68 Max Schwarz 2014-06-11 217 bool is_last_msg;
c41aa3ce938b68 Max Schwarz 2014-06-11 218
c41aa3ce938b68 Max Schwarz 2014-06-11 219 /* I2C state machine */
c41aa3ce938b68 Max Schwarz 2014-06-11 220 enum rk3x_i2c_state state;
0a6ad2f95f36bf David Wu 2016-05-16 221 unsigned int processed;
c41aa3ce938b68 Max Schwarz 2014-06-11 222 int error;
c41aa3ce938b68 Max Schwarz 2014-06-11 @223 };
c41aa3ce938b68 Max Schwarz 2014-06-11 224
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index a044ca0c35a1..94514637c3bd 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -200,6 +200,7 @@ struct rk3x_i2c { struct clk *clk; struct clk *pclk; struct notifier_block clk_rate_nb; + int irq; /* Settings */ struct i2c_timings t; @@ -1087,13 +1088,18 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap, spin_unlock_irqrestore(&i2c->lock, flags); - rk3x_i2c_start(i2c); - if (!polling) { + rk3x_i2c_start(i2c); + timeout = wait_event_timeout(i2c->wait, !i2c->busy, msecs_to_jiffies(WAIT_TIMEOUT)); } else { + disable_irq(i2c->irq); + rk3x_i2c_start(i2c); + timeout = rk3x_i2c_wait_xfer_poll(i2c); + + enable_irq(i2c->irq); } spin_lock_irqsave(&i2c->lock, flags); @@ -1310,6 +1316,8 @@ static int rk3x_i2c_probe(struct platform_device *pdev) return ret; } + i2c->irq = irq; + platform_set_drvdata(pdev, i2c); if (i2c->soc_data->calc_timings == rk3x_i2c_v0_calc_timings) {
Possible deadlock scenario (on reboot): rk3x_i2c_xfer_common(polling) -> rk3x_i2c_wait_xfer_poll() -> rk3x_i2c_irq(0, i2c); --> spin_lock(&i2c->lock); ... <rk3x i2c interrupt> -> rk3x_i2c_irq(0, i2c); --> spin_lock(&i2c->lock); (deadlock here) Store the IRQ number and disable/enable it around the polling transfer. This patch has been tested on NanoPC-T4. Signed-off-by: Jensen Huang <jensenhuang@friendlyarm.com> --- drivers/i2c/busses/i2c-rk3x.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)