From: Bjorn Andersson <andersson@kernel.org> To: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>, Prashant Malani <pmalani@chromium.org>, Doug Anderson <dianders@chromium.org>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, Abhinav Kumar <abhinavk@codeaurora.org>, Stephen Boyd <swboyd@chromium.org>, Kuogee Hsieh <khsieh@codeaurora.org>, dri-devel <dri-devel@lists.freedesktop.org>, Vara Reddy <varar@codeaurora.org>, freedreno <freedreno@lists.freedesktop.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Benson Leung <bleung@chromium.org> Subject: Re: [RFC] drm/msm/dp: Allow attaching a drm_panel Date: Mon, 22 May 2023 15:51:01 -0500 [thread overview] Message-ID: <do5veo5axxbvmcddpqf7u5rfer6soxzy5selfnjv5sn6n57h47@q3hfznslndba> (raw) In-Reply-To: <YWA7vXp+4QbKWU1S@kuha.fi.intel.com> On Fri, Oct 08, 2021 at 03:38:21PM +0300, Heikki Krogerus wrote: > Hi, > > On Thu, Oct 07, 2021 at 09:15:12AM -0700, Bjorn Andersson wrote: > > The one thing that I still don't understand though is, if the typec_mux > > is used by the typec controller to inform _the_ mux about the function > > to be used, what's up with the complexity in typec_mux_match()? This is > > what lead me to believe that typec_mux was enabling/disabling individual > > altmodes, rather just flipping the physical switch at the bottom. > > Ah, typec_mux_match() is a mess. I'm sorry about that. I think most of > the code in that function is not used by anybody. If I remember > correctly, all that complexity is attempting to solve some > hypothetical corner case(s). Probable a case where we have multiple > muxes per port to deal with. > > I think it would probable be best to clean the function to the bare > minimum by keeping only the parts that are actually used today > (attached). > Sorry for not replying to this in a timely manner Heikki. I've been ignoring this issue for a long time now, just adding "svid" to our dts files. But, this obviously shows up in DT validation - and I'd prefer not defining these properties as valid. The attached patch works as expected. Could you please spin this as a proper patch, so we can get it merged? Regards, Bjorn > thanks, > > -- > heikki > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c > index c8340de0ed495..44f168c9bd9bf 100644 > --- a/drivers/usb/typec/mux.c > +++ b/drivers/usb/typec/mux.c > @@ -193,56 +193,15 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode) > static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id, > void *data) > { > - const struct typec_altmode_desc *desc = data; > struct device *dev; > - bool match; > - int nval; > - u16 *val; > - int ret; > - int i; > > /* > - * Check has the identifier already been "consumed". If it > - * has, no need to do any extra connection identification. > + * The connection identifier will be needed with device graph (OF graph). > + * Device graph is not supported by this code yet, so bailing out. > */ > - match = !id; > - if (match) > - goto find_mux; > - > - /* Accessory Mode muxes */ > - if (!desc) { > - match = fwnode_property_present(fwnode, "accessory"); > - if (match) > - goto find_mux; > - return NULL; > - } > - > - /* Alternate Mode muxes */ > - nval = fwnode_property_count_u16(fwnode, "svid"); > - if (nval <= 0) > - return NULL; > - > - val = kcalloc(nval, sizeof(*val), GFP_KERNEL); > - if (!val) > - return ERR_PTR(-ENOMEM); > - > - ret = fwnode_property_read_u16_array(fwnode, "svid", val, nval); > - if (ret < 0) { > - kfree(val); > - return ERR_PTR(ret); > - } > - > - for (i = 0; i < nval; i++) { > - match = val[i] == desc->svid; > - if (match) { > - kfree(val); > - goto find_mux; > - } > - } > - kfree(val); > - return NULL; > + if (id) > + return ERR_PTR(-ENOTSUPP); > > -find_mux: > dev = class_find_device(&typec_mux_class, NULL, fwnode, > mux_fwnode_match); >
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Andersson <andersson@kernel.org> To: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: freedreno <freedreno@lists.freedesktop.org>, David Airlie <airlied@linux.ie>, linux-arm-msm <linux-arm-msm@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Doug Anderson <dianders@chromium.org>, Abhinav Kumar <abhinavk@codeaurora.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Kuogee Hsieh <khsieh@codeaurora.org>, Prashant Malani <pmalani@chromium.org>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Vara Reddy <varar@codeaurora.org>, Stephen Boyd <swboyd@chromium.org>, Sean Paul <sean@poorly.run>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [RFC] drm/msm/dp: Allow attaching a drm_panel Date: Mon, 22 May 2023 15:51:01 -0500 [thread overview] Message-ID: <do5veo5axxbvmcddpqf7u5rfer6soxzy5selfnjv5sn6n57h47@q3hfznslndba> (raw) In-Reply-To: <YWA7vXp+4QbKWU1S@kuha.fi.intel.com> On Fri, Oct 08, 2021 at 03:38:21PM +0300, Heikki Krogerus wrote: > Hi, > > On Thu, Oct 07, 2021 at 09:15:12AM -0700, Bjorn Andersson wrote: > > The one thing that I still don't understand though is, if the typec_mux > > is used by the typec controller to inform _the_ mux about the function > > to be used, what's up with the complexity in typec_mux_match()? This is > > what lead me to believe that typec_mux was enabling/disabling individual > > altmodes, rather just flipping the physical switch at the bottom. > > Ah, typec_mux_match() is a mess. I'm sorry about that. I think most of > the code in that function is not used by anybody. If I remember > correctly, all that complexity is attempting to solve some > hypothetical corner case(s). Probable a case where we have multiple > muxes per port to deal with. > > I think it would probable be best to clean the function to the bare > minimum by keeping only the parts that are actually used today > (attached). > Sorry for not replying to this in a timely manner Heikki. I've been ignoring this issue for a long time now, just adding "svid" to our dts files. But, this obviously shows up in DT validation - and I'd prefer not defining these properties as valid. The attached patch works as expected. Could you please spin this as a proper patch, so we can get it merged? Regards, Bjorn > thanks, > > -- > heikki > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c > index c8340de0ed495..44f168c9bd9bf 100644 > --- a/drivers/usb/typec/mux.c > +++ b/drivers/usb/typec/mux.c > @@ -193,56 +193,15 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode) > static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id, > void *data) > { > - const struct typec_altmode_desc *desc = data; > struct device *dev; > - bool match; > - int nval; > - u16 *val; > - int ret; > - int i; > > /* > - * Check has the identifier already been "consumed". If it > - * has, no need to do any extra connection identification. > + * The connection identifier will be needed with device graph (OF graph). > + * Device graph is not supported by this code yet, so bailing out. > */ > - match = !id; > - if (match) > - goto find_mux; > - > - /* Accessory Mode muxes */ > - if (!desc) { > - match = fwnode_property_present(fwnode, "accessory"); > - if (match) > - goto find_mux; > - return NULL; > - } > - > - /* Alternate Mode muxes */ > - nval = fwnode_property_count_u16(fwnode, "svid"); > - if (nval <= 0) > - return NULL; > - > - val = kcalloc(nval, sizeof(*val), GFP_KERNEL); > - if (!val) > - return ERR_PTR(-ENOMEM); > - > - ret = fwnode_property_read_u16_array(fwnode, "svid", val, nval); > - if (ret < 0) { > - kfree(val); > - return ERR_PTR(ret); > - } > - > - for (i = 0; i < nval; i++) { > - match = val[i] == desc->svid; > - if (match) { > - kfree(val); > - goto find_mux; > - } > - } > - kfree(val); > - return NULL; > + if (id) > + return ERR_PTR(-ENOTSUPP); > > -find_mux: > dev = class_find_device(&typec_mux_class, NULL, fwnode, > mux_fwnode_match); >
next prev parent reply other threads:[~2023-05-22 20:51 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-26 23:13 [RFC] drm/msm/dp: Allow attaching a drm_panel Bjorn Andersson 2021-07-26 23:13 ` Bjorn Andersson 2021-07-29 9:59 ` Dmitry Baryshkov 2021-07-29 9:59 ` Dmitry Baryshkov 2021-08-25 23:28 ` Bjorn Andersson 2021-08-26 1:31 ` Stephen Boyd 2021-08-26 1:31 ` Stephen Boyd 2021-08-26 20:36 ` Doug Anderson 2021-08-26 20:36 ` Doug Anderson 2021-08-26 20:29 ` Doug Anderson 2021-08-26 20:29 ` Doug Anderson 2021-08-27 20:52 ` Doug Anderson 2021-08-27 20:52 ` Doug Anderson 2021-08-28 14:40 ` Bjorn Andersson 2021-08-30 16:01 ` Doug Anderson 2021-08-30 16:01 ` Doug Anderson 2021-10-01 21:02 ` Bjorn Andersson 2021-10-05 0:36 ` Doug Anderson 2021-10-05 1:11 ` Bjorn Andersson 2021-10-05 1:50 ` Stephen Boyd 2021-10-05 2:11 ` Bjorn Andersson 2021-10-05 15:39 ` Doug Anderson 2021-10-05 17:34 ` Bjorn Andersson 2021-10-05 23:09 ` Doug Anderson 2021-10-06 2:29 ` Bjorn Andersson 2021-10-06 15:12 ` Doug Anderson 2021-10-06 20:26 ` Prashant Malani 2021-10-07 10:17 ` Heikki Krogerus 2021-10-07 16:15 ` Bjorn Andersson 2021-10-08 12:38 ` Heikki Krogerus 2023-05-22 20:51 ` Bjorn Andersson [this message] 2023-05-22 20:51 ` Bjorn Andersson 2023-05-22 21:53 ` Bjorn Andersson 2023-05-22 21:53 ` Bjorn Andersson 2023-05-24 9:50 ` Heikki Krogerus 2023-05-24 9:50 ` Heikki Krogerus 2021-12-06 22:31 ` Bjorn Andersson 2021-12-06 22:31 ` Bjorn Andersson 2021-12-07 12:26 ` Heikki Krogerus 2021-12-07 12:26 ` Heikki Krogerus 2021-12-07 16:56 ` Hans de Goede 2021-12-07 16:56 ` Hans de Goede 2021-12-07 17:29 ` Bjorn Andersson 2021-12-07 17:29 ` Bjorn Andersson 2021-12-07 17:54 ` Imre Deak 2021-12-07 17:54 ` Imre Deak
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=do5veo5axxbvmcddpqf7u5rfer6soxzy5selfnjv5sn6n57h47@q3hfznslndba \ --to=andersson@kernel.org \ --cc=abhinavk@codeaurora.org \ --cc=airlied@linux.ie \ --cc=bjorn.andersson@linaro.org \ --cc=bleung@chromium.org \ --cc=daniel@ffwll.ch \ --cc=dianders@chromium.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=enric.balletbo@collabora.com \ --cc=freedreno@lists.freedesktop.org \ --cc=heikki.krogerus@linux.intel.com \ --cc=khsieh@codeaurora.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=pmalani@chromium.org \ --cc=robdclark@gmail.com \ --cc=sean@poorly.run \ --cc=swboyd@chromium.org \ --cc=varar@codeaurora.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.