Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756966AbcCXJvG (ORCPT ); Thu, 24 Mar 2016 05:51:06 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:44848 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755204AbcCXJuy (ORCPT ); Thu, 24 Mar 2016 05:50:54 -0400 Subject: Re: [PATCH v2 1/8] i2c-mux: add common core data for every mux instance To: Peter Rosin References: <1452009438-27347-1-git-send-email-peda@lysator.liu.se> <1452009438-27347-2-git-send-email-peda@lysator.liu.se> CC: Wolfram Sang , Peter Rosin , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Peter Korsgaard , Guenter Roeck , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Antti Palosaari , Mauro Carvalho Chehab , Frank Rowand , Grant Likely , Adriana Reus , Srinivas Pandruvada , Krzysztof Kozlowski , Hans Verkuil , Nicholas Mc Guire , Olli Salonen , , , , , From: Vladimir Zapolskiy Message-ID: <56F3B86E.4050002@mentor.com> Date: Thu, 24 Mar 2016 11:50:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Icedove/38.1.0 MIME-Version: 1.0 In-Reply-To: <1452009438-27347-2-git-send-email-peda@lysator.liu.se> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [137.202.0.76] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1736 Lines: 59 Hi Peter, On 05.01.2016 17:57, Peter Rosin wrote: > From: Peter Rosin > > The initial core mux structure starts off small with only the parent > adapter pointer, which all muxes have, and a priv pointer for mux > driver private data. > > Add i2c_mux_alloc function to unify the creation of a mux. > > Where appropriate, pass around the mux core structure instead of the > parent adapter or the driver private data. > > Remove the parent adapter pointer from the driver private data for all > mux drivers. > > Signed-off-by: Peter Rosin is it still under review? If yes, please find one question from me below :) [snip] > @@ -196,21 +195,21 @@ static int i2c_arbitrator_probe(struct platform_device *pdev) > dev_err(dev, "Cannot parse i2c-parent\n"); > return -EINVAL; > } > - arb->parent = of_get_i2c_adapter_by_node(parent_np); > + muxc->parent = of_find_i2c_adapter_by_node(parent_np); why do you prefer here to use "unlocked" version of API? Foe example would it be safe/possible to unload an I2C bus device driver module or unbind I2C device itself in runtime? > of_node_put(parent_np); > - if (!arb->parent) { > + if (!muxc->parent) { > dev_err(dev, "Cannot find parent bus\n"); > return -EPROBE_DEFER; > } > > /* Actually add the mux adapter */ > - arb->child = i2c_add_mux_adapter(arb->parent, dev, arb, 0, 0, 0, > + arb->child = i2c_add_mux_adapter(muxc, dev, arb, 0, 0, 0, > i2c_arbitrator_select, > i2c_arbitrator_deselect); > if (!arb->child) { > dev_err(dev, "Failed to add adapter\n"); > ret = -ENODEV; > - i2c_put_adapter(arb->parent); > + i2c_put_adapter(muxc->parent); > } > > return ret; -- With best wishes, Vladimir