linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Sam Ravnborg <sam@ravnborg.org>, Wolfram Sang <wsa@kernel.org>
Cc: Lyude Paul <lyude@redhat.com>,
	Steev Klimaszewski <steev@kali.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	linux-arm-msm@vger.kernel.org,
	Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>,
	Linus W <linus.walleij@linaro.org>,
	robdclark@chromium.org,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thierry Reding <treding@nvidia.com>,
	linux-i2c@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Douglas Anderson <dianders@chromium.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v6 1/5] i2c: i2c-core-of: Fix corner case of finding adapter by node
Date: Mon,  3 May 2021 14:58:40 -0700	[thread overview]
Message-ID: <20210503145750.v6.1.Ib7e3a4af2f3e2cb3bd8e4adbac3bcfc966f27791@changeid> (raw)
In-Reply-To: <20210503215844.2996320-1-dianders@chromium.org>

The of_find_i2c_adapter_by_node() could end up failing to find an
adapter in certain conditions. Specifically it's possible that
of_dev_or_parent_node_match() could end up finding an I2C client in
the list and cause bus_find_device() to stop early even though an I2C
adapter was present later in the list.

Let's move the i2c_verify_adapter() into the predicate function to
prevent this. Now we'll properly skip over the I2C client and be able
to find the I2C adapter.

This issue has always been a potential problem if a single device tree
node could represent both an I2C client and an adapter. I believe this
is a sane thing to do if, for instance, an I2C-connected DP bridge
chip is present. The bridge chip is an I2C client but it can also
provide an I2C adapter (DDC tunneled over AUX channel). We don't want
to have to create a sub-node just so a panel can link to it with the
"ddc-i2c-bus" property.

I believe that this problem got worse, however, with commit
e814e688413a ("i2c: of: Try to find an I2C adapter matching the
parent"). Starting at that commit it would be even easier to
accidentally miss finding the adapter.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Later patches in this series won't work right without this one, but
they won't crash. If we can't find the i2c bus we'll just fall back to
the hardcoded panel modes which, at least today, all panels have.

I'll also note that part of me wonders if we should actually fix this
further to run two passes through everything: first look to see if we
find an exact match and only look at the parent pointer if there is no
match. I don't currently have a need for that and it's a slightly
bigger change, but it seems conceivable that it could affect someone?

(no changes since v1)

 drivers/i2c/i2c-core-of.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 3ed74aa4b44b..de0bf5fce3a2 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -124,6 +124,14 @@ static int of_dev_or_parent_node_match(struct device *dev, const void *data)
 	return 0;
 }
 
+static int of_i2c_adapter_match(struct device *dev, const void *data)
+{
+	if (!of_dev_or_parent_node_match(dev, data))
+		return 0;
+
+	return !!i2c_verify_adapter(dev);
+}
+
 /* must call put_device() when done with returned i2c_client device */
 struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
 {
@@ -146,18 +154,13 @@ EXPORT_SYMBOL(of_find_i2c_device_by_node);
 struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
 {
 	struct device *dev;
-	struct i2c_adapter *adapter;
 
 	dev = bus_find_device(&i2c_bus_type, NULL, node,
-			      of_dev_or_parent_node_match);
+			      of_i2c_adapter_match);
 	if (!dev)
 		return NULL;
 
-	adapter = i2c_verify_adapter(dev);
-	if (!adapter)
-		put_device(dev);
-
-	return adapter;
+	return to_i2c_adapter(dev);
 }
 EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
 
-- 
2.31.1.527.g47e6f16901-goog


  reply	other threads:[~2021-05-03 21:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-03 21:58 [PATCH v6 0/5] drm: Fix EDID reading on ti-sn65dsi86; solve some chicken-and-egg problems Douglas Anderson
2021-05-03 21:58 ` Douglas Anderson [this message]
2021-05-17 20:16   ` [PATCH v6 1/5] i2c: i2c-core-of: Fix corner case of finding adapter by node Doug Anderson
2021-05-03 21:58 ` [PATCH v6 2/5] drm/dp: Allow an early call to register DDC i2c bus Douglas Anderson
2021-05-07 21:18   ` Lyude Paul
2021-05-07 22:00     ` Bjorn Andersson
2021-05-07 22:09       ` Lyude Paul
2021-05-14 11:16         ` Jani Nikula
2021-05-17 20:17           ` Doug Anderson
2021-05-03 21:58 ` [PATCH v6 3/5] drm/bridge: ti-sn65dsi86: Promote the AUX channel to its own sub-dev Douglas Anderson
2021-05-03 21:58 ` [PATCH v6 4/5] drm/bridge: ti-sn65dsi86: Don't read EDID blob over DDC Douglas Anderson
2021-05-03 21:58 ` [PATCH v6 5/5] arm64: dts: qcom: Link the panel to the bridge's DDC bus Douglas Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210503145750.v6.1.Ib7e3a4af2f3e2cb3bd8e4adbac3bcfc966f27791@changeid \
    --to=dianders@chromium.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=a.hajda@samsung.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jernej.skrabec@siol.net \
    --cc=jonas@kwiboo.se \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=narmstrong@baylibre.com \
    --cc=robdclark@chromium.org \
    --cc=sam@ravnborg.org \
    --cc=stanislav.lisovskiy@intel.com \
    --cc=steev@kali.org \
    --cc=swboyd@chromium.org \
    --cc=treding@nvidia.com \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).