Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754895AbcDSQiH (ORCPT ); Tue, 19 Apr 2016 12:38:07 -0400 Received: from mail.lysator.liu.se ([130.236.254.3]:44003 "EHLO mail.lysator.liu.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753377AbcDSQiE (ORCPT ); Tue, 19 Apr 2016 12:38:04 -0400 X-Spam-Score: -1.0 User-Agent: K-9 Mail for Android In-Reply-To: <57165593.4040700@intel.com> References: <1459673574-11440-1-git-send-email-peda@lysator.liu.se> <1459673574-11440-9-git-send-email-peda@lysator.liu.se> <57165593.4040700@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PATCH v6 08/24] iio: imu: inv_mpu6050: convert to use an explicit i2c mux core From: Peter Rosin Date: Tue, 19 Apr 2016 18:37:41 +0200 To: Crestez Dan Leonard , linux-kernel@vger.kernel.org CC: Peter Rosin , Wolfram Sang , Jonathan Corbet , Peter Korsgaard , Guenter Roeck , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Antti Palosaari , Mauro Carvalho Chehab , Rob Herring , Frank Rowand , Grant Likely , Andrew Morton , Greg Kroah-Hartman , "David S. Miller" , Kalle Valo , Joe Perches , Jiri Slaby , Daniel Baluta , Adriana Reus , Lucas De Marchi , Matt Ranostay , Krzysztof Kozlowski , Terry Heo , Hans@lysator.liu.se, Verkuil@lysator.liu.se Message-Id: <20160419163748.9129940005@mail.lysator.liu.se> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3584 Lines: 92 ,Arnd Bergmann ,Tommi Rantala ,linux-i2c@vger.kernel.org,linux-doc@vger.kernel.org,linux-iio@vger.kernel.org,linux-media@vger.kernel.org,devicetree@vger.kernel.org Message-ID: On April 19, 2016 5:58:11 PM CEST, Crestez Dan Leonard wrote: > On 04/03/2016 11:52 AM, Peter Rosin wrote: > > From: Peter Rosin > > > > Allocate an explicit i2c mux core to handle parent and child > adapters > > etc. Update the select/deselect ops to be in terms of the i2c mux > core > > instead of the child adapter. > > > > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c > > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c > > @@ -136,16 +133,15 @@ static int inv_mpu_probe(struct i2c_client > *client, > > return result; > > > > st = iio_priv(dev_get_drvdata(&client->dev)); > > - st->mux_adapter = i2c_add_mux_adapter(client->adapter, > > - &client->dev, > > - client, > > - 0, 0, 0, > > - inv_mpu6050_select_bypass, > > - inv_mpu6050_deselect_bypass); > > - if (!st->mux_adapter) { > > - result = -ENODEV; > > + st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, > 0, > > + 0, 0, 0, > > + inv_mpu6050_select_bypass, > > + inv_mpu6050_deselect_bypass); > > + if (IS_ERR(st->muxc)) { > > + result = PTR_ERR(st->muxc); > > goto out_unreg_device; > > } > > + st->muxc->priv = client; > > I just tested this patch on mpu9150 (combo mpu6050+ak8975) and I got a > crash on probe which looks sort of like this: > > [ 5.565374] RIP: 0010:[] [] > mutex_lock+0xd/0x30 > [ 5.565374] Call Trace: > [ 5.565374] [] > inv_mpu6050_select_bypass+0x1c/0xa0 > ... > [ 5.565374] [] i2c_transfer+0x51/0x90 > ... > [ 5.565374] [] > i2c_smbus_read_i2c_block_data+0x45/0xd0 > [ 5.565374] [] ak8975_probe+0x14a/0x5c0 > ... > [ 5.565374] [] i2c_new_device+0x178/0x1e0 > [ 5.565374] [] of_i2c_register_device+0xdd/0x1a0 > [ 5.565374] [] of_i2c_register_devices+0x3b/0x60 > [ 5.565374] [] i2c_register_adapter+0x178/0x410 > [ 5.565374] [] i2c_add_adapter+0x73/0x90 > [ 5.565374] [] i2c_mux_add_adapter+0x2ed/0x400 > [ 5.565374] [] i2c_mux_one_adapter+0x41/0x70 > [ 5.565374] [] inv_mpu_probe+0xba/0x1a0 > > This happens because muxc->priv is not initialized when probing > devices > behind the mux as described by devicetree. This can be easily fixed by > using muxc->dev instead of muxc->priv, or perhaps by calling > i2c_mux_alloc, initializing priv and later doing i2c_mux_add_adapter. > > These fixes are driver-specific and other drivers might experience > similar issues. Perhaps i2c_mux_one_adapter should take void *priv > just > like old i2c_add_mux_adapter? > > After I fix this locally the deadlock when reading from both devices > no > longer happens. > > -- > Regards, > Leonard Duh. Obvious now that you point it out. Thanks for catching this! I will just remove the i2c_mux_one_adapter interface for v7 and have the affected drivers do i2c_mux_alloc as a separate step (which was one of your suggested paths forward...) Thanks again, and sorry for the inconvenience, Peter (Written on my phone, sorry for crappy formatting)