Re: [PATCH v2 2/2] i2c: mux: pca954x: Support multiple devices on a single reset line
From: Rob Herring <robh@kernel.org>
Date: 2021-08-02 22:35:58
Also in:
linux-i2c, lkml
On Mon, Aug 2, 2021 at 3:51 PM Eddie James [off-list ref] wrote:
On Mon, 2021-08-02 at 14:46 -0600, Rob Herring wrote:quoted
On Tue, Jul 27, 2021 at 11:03:15AM -0500, Eddie James wrote:quoted
Some systems connect several PCA954x devices to a single reset GPIO. For these devices to get out of reset and probe successfully, each device must defer the probe until the GPIO has been hogged. Accomplish this by attempting to grab a new "reset-shared-hogged" devicetree property, but expect it to fail with EPROBE_DEFER or EBUSY. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/i2c/muxes/i2c-mux-pca954x.c | 46 +++++++++++++++++++++++ ------ 1 file changed, 37 insertions(+), 9 deletions(-)diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.cb/drivers/i2c/muxes/i2c-mux-pca954x.c index 4ad665757dd8..376b54ffb590 100644--- a/drivers/i2c/muxes/i2c-mux-pca954x.c +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c@@ -434,15 +434,43 @@ static int pca954x_probe(struct i2c_client*client, i2c_set_clientdata(client, muxc); data->client = client; - /* Reset the mux if a reset GPIO is specified. */ - gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(gpio)) - return PTR_ERR(gpio); - if (gpio) { - udelay(1); - gpiod_set_value_cansleep(gpio, 0); - /* Give the chip some time to recover. */ - udelay(1); + /* + * Grab the shared, hogged gpio that controls the mux reset. We expect + * this to fail with either EPROBE_DEFER or EBUSY. The only purpose of + * trying to get it is to make sure the gpio controller has probed up + * and hogged the line to take the mux out of reset, meaning that the + * mux is ready to be probed up. Don't try and set the line any way; in + * the event we actually successfully get the line (if it wasn't + * hogged) then we immediately release it, since there is no way to + * sync up the line between muxes. + */ + gpio = gpiod_get_optional(dev, "reset-shared-hogged", 0); + if (IS_ERR(gpio)) { + ret = PTR_ERR(gpio); + if (ret != -EBUSY) + return ret;Why can't you just do this with the existing 'reset-gpios' property? What's the usecase where you'd want to fail probe because EBUSY other than an error in your DT.Hi, thanks for the reply. Are you suggesting I use "reset-gpios" and change the driver to ignore EBUSY? I don't know any other usecase, I just didn't think it would be acceptable to ignore EBUSY on that, but perhaps it is a better solution.
Yes. I'm assuming that's the only way EBUSY is returned? A DT already contains everything needed to know multiple 'reset-gpios' are sharing a GPIO line. It's a kernel problem to figure that out. (Really, I'd like to make 'reset-gpios' be handled by the reset subsystem which handles shared resets already. Then 'reset-gpios' or 'resets' could be used in nodes and drivers don't know the difference.) Rob