Den 20.10.2021 00.07, skrev Andy Shevchenko: > > > On Monday, October 18, 2021, Noralf Trønnes <noralf@xxxxxxxxxxx > <mailto:noralf@xxxxxxxxxxx>> wrote: > > When reading the voltage: > > $ cat /sys/bus/iio/devices/iio\:device0/in_voltage0_raw > > Lockdep complains: > > [ 153.910616] ====================================================== > [ 153.916918] WARNING: possible circular locking dependency detected > [ 153.923221] 5.14.0+ #5 Not tainted > [ 153.926692] ------------------------------------------------------ > [ 153.932992] cat/717 is trying to acquire lock: > [ 153.937525] c2585358 (&indio_dev->mlock){+.+.}-{3:3}, at: > iio_device_claim_direct_mode+0x28/0x44 > [ 153.946541] > but task is already holding lock: > [ 153.952487] c2585860 (&dln2->mutex){+.+.}-{3:3}, at: > dln2_adc_read_raw+0x94/0x2bc [dln2_adc] > [ 153.961152] > which lock already depends on the new lock. > > Fix this by not calling into the iio core underneath the dln2->mutex > lock. > > > Side question: have you noticed any race condition when you connect and > disconnect DLN2 module many times in a row? (okay, I have a board where > uB and type A connectors are for the same DR port, and it has a switch > for ID pin, that user may toggle, when I do this, sometimes race happens > and usb doesn’t really switches the role and dln2 drivers crashes with > rcu splat) > No I haven't seen that, but I haven't got an actual DLN2 adapter if that matters, I've implemented the protocol on a Raspberry Pi Pico: https://github.com/notro/pico-usb-io-board Noralf. > > Fixes: 7c0299e879dd ("iio: adc: Add support for DLN2 ADC") > Cc: Jack Andersen <jackoalan@xxxxxxxxx <mailto:jackoalan@xxxxxxxxx>> > Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx > <mailto:noralf@xxxxxxxxxxx>> > --- > > Note that this patch is needed for the driver to be usable: > > mfd: dln2: Add cell for initializing DLN2 ADC > https://lore.kernel.org/lkml/20211018112541.25466-1-noralf@xxxxxxxxxxx/T/#u > <https://lore.kernel.org/lkml/20211018112541.25466-1-noralf@xxxxxxxxxxx/T/#u> > > > drivers/iio/adc/dln2-adc.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c > index 16407664182c..6c67192946aa 100644 > --- a/drivers/iio/adc/dln2-adc.c > +++ b/drivers/iio/adc/dln2-adc.c > @@ -248,7 +248,6 @@ static int dln2_adc_set_chan_period(struct > dln2_adc *dln2, > static int dln2_adc_read(struct dln2_adc *dln2, unsigned int channel) > { > int ret, i; > - struct iio_dev *indio_dev = platform_get_drvdata(dln2->pdev); > u16 conflict; > __le16 value; > int olen = sizeof(value); > @@ -257,13 +256,9 @@ static int dln2_adc_read(struct dln2_adc *dln2, > unsigned int channel) > .chan = channel, > }; > > - ret = iio_device_claim_direct_mode(indio_dev); > - if (ret < 0) > - return ret; > - > ret = dln2_adc_set_chan_enabled(dln2, channel, true); > if (ret < 0) > - goto release_direct; > + return ret; > > ret = dln2_adc_set_port_enabled(dln2, true, &conflict); > if (ret < 0) { > @@ -300,8 +295,6 @@ static int dln2_adc_read(struct dln2_adc *dln2, > unsigned int channel) > dln2_adc_set_port_enabled(dln2, false, NULL); > disable_chan: > dln2_adc_set_chan_enabled(dln2, channel, false); > -release_direct: > - iio_device_release_direct_mode(indio_dev); > > return ret; > } > @@ -337,10 +330,16 @@ static int dln2_adc_read_raw(struct iio_dev > *indio_dev, > > switch (mask) { > case IIO_CHAN_INFO_RAW: > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret < 0) > + return ret; > + > mutex_lock(&dln2->mutex); > ret = dln2_adc_read(dln2, chan->channel); > mutex_unlock(&dln2->mutex); > > + iio_device_release_direct_mode(indio_dev); > + > if (ret < 0) > return ret; > > -- > 2.33.0 > > > > -- > With Best Regards, > Andy Shevchenko > >
