2024-01-16 19:56:19

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.7 044/108] usb: typec: ucsi: fix UCSI on buggy Qualcomm devices

From: Dmitry Baryshkov <[email protected]>

[ Upstream commit 1d103d6af241dbfc7e11eb9a46dff65db257a37f ]

On sevral Qualcomm platforms (SC8180X, SM8350, SC8280XP) a call to
UCSI_GET_PDOS for non-PD partners will cause a firmware crash with no
easy way to recover from it. Since we have no easy way to determine
whether the partner really has PD support, shortcut UCSI_GET_PDOS on
such platforms. This allows us to enable UCSI support on such devices.

Signed-off-by: Dmitry Baryshkov <[email protected]>
Acked-by: Heikki Krogerus <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/usb/typec/ucsi/ucsi.c | 3 +++
drivers/usb/typec/ucsi/ucsi.h | 3 +++
drivers/usb/typec/ucsi/ucsi_glink.c | 13 +++++++++++++
3 files changed, 19 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 61b64558f96c..5392ec698959 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -578,6 +578,9 @@ static int ucsi_read_pdos(struct ucsi_connector *con,
u64 command;
int ret;

+ if (ucsi->quirks & UCSI_NO_PARTNER_PDOS)
+ return 0;
+
command = UCSI_COMMAND(UCSI_GET_PDOS) | UCSI_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_PDOS_PARTNER_PDO(is_partner);
command |= UCSI_GET_PDOS_PDO_OFFSET(offset);
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 474315a72c77..6478016d5cb8 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -317,6 +317,9 @@ struct ucsi {
#define EVENT_PENDING 0
#define COMMAND_PENDING 1
#define ACK_PENDING 2
+
+ unsigned long quirks;
+#define UCSI_NO_PARTNER_PDOS BIT(0) /* Don't read partner's PDOs */
};

#define UCSI_MAX_SVID 5
diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index 4853141cd10c..53a7ede8556d 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -6,6 +6,7 @@
#include <linux/auxiliary_bus.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/of_device.h>
#include <linux/property.h>
#include <linux/soc/qcom/pdr.h>
#include <linux/usb/typec_mux.h>
@@ -296,11 +297,19 @@ static void pmic_glink_ucsi_destroy(void *data)
mutex_unlock(&ucsi->lock);
}

+static const struct of_device_id pmic_glink_ucsi_of_quirks[] = {
+ { .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)UCSI_NO_PARTNER_PDOS, },
+ { .compatible = "qcom,sc8280xp-pmic-glink", .data = (void *)UCSI_NO_PARTNER_PDOS, },
+ { .compatible = "qcom,sm8350-pmic-glink", .data = (void *)UCSI_NO_PARTNER_PDOS, },
+ {}
+};
+
static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
const struct auxiliary_device_id *id)
{
struct pmic_glink_ucsi *ucsi;
struct device *dev = &adev->dev;
+ const struct of_device_id *match;
struct fwnode_handle *fwnode;
int ret;

@@ -327,6 +336,10 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
if (ret)
return ret;

+ match = of_match_device(pmic_glink_ucsi_of_quirks, dev->parent);
+ if (match)
+ ucsi->ucsi->quirks = (unsigned long)match->data;
+
ucsi_set_drvdata(ucsi->ucsi, ucsi);

device_for_each_child_node(dev, fwnode) {
--
2.43.0



2024-01-17 08:04:11

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH AUTOSEL 6.7 044/108] usb: typec: ucsi: fix UCSI on buggy Qualcomm devices

On Tue, Jan 16, 2024 at 02:39:10PM -0500, Sasha Levin wrote:
> From: Dmitry Baryshkov <[email protected]>
>
> [ Upstream commit 1d103d6af241dbfc7e11eb9a46dff65db257a37f ]
>
> On sevral Qualcomm platforms (SC8180X, SM8350, SC8280XP) a call to
> UCSI_GET_PDOS for non-PD partners will cause a firmware crash with no
> easy way to recover from it. Since we have no easy way to determine
> whether the partner really has PD support, shortcut UCSI_GET_PDOS on
> such platforms. This allows us to enable UCSI support on such devices.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> Acked-by: Heikki Krogerus <[email protected]>
> Reviewed-by: Neil Armstrong <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>

Correct me if I'm wrong Dmitry, but while the commit message makes this
sound like a fix, it is not needed unless you backport follow-on patches
that enable UCSI on these platforms.

So this one can be dropped from all stable queues (unless you're
backporting patches that enable new features and that depend on this
one).

Johan

2024-01-17 12:25:41

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH AUTOSEL 6.7 044/108] usb: typec: ucsi: fix UCSI on buggy Qualcomm devices

On Wed, 17 Jan 2024 at 10:03, Johan Hovold <[email protected]> wrote:
>
> On Tue, Jan 16, 2024 at 02:39:10PM -0500, Sasha Levin wrote:
> > From: Dmitry Baryshkov <[email protected]>
> >
> > [ Upstream commit 1d103d6af241dbfc7e11eb9a46dff65db257a37f ]
> >
> > On sevral Qualcomm platforms (SC8180X, SM8350, SC8280XP) a call to
> > UCSI_GET_PDOS for non-PD partners will cause a firmware crash with no
> > easy way to recover from it. Since we have no easy way to determine
> > whether the partner really has PD support, shortcut UCSI_GET_PDOS on
> > such platforms. This allows us to enable UCSI support on such devices.
> >
> > Signed-off-by: Dmitry Baryshkov <[email protected]>
> > Acked-by: Heikki Krogerus <[email protected]>
> > Reviewed-by: Neil Armstrong <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > Signed-off-by: Sasha Levin <[email protected]>
>
> Correct me if I'm wrong Dmitry, but while the commit message makes this
> sound like a fix, it is not needed unless you backport follow-on patches
> that enable UCSI on these platforms.
>
> So this one can be dropped from all stable queues (unless you're
> backporting patches that enable new features and that depend on this
> one).

Exactly. It didn't have the Fixes: tag. So I'm completely unsure why
it ended up in the autosel queue at all.

--
With best wishes
Dmitry

2024-01-17 12:51:27

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH AUTOSEL 6.7 044/108] usb: typec: ucsi: fix UCSI on buggy Qualcomm devices

On Wed, Jan 17, 2024 at 02:17:40PM +0200, Dmitry Baryshkov wrote:
> On Wed, 17 Jan 2024 at 10:03, Johan Hovold <[email protected]> wrote:
> >
> > On Tue, Jan 16, 2024 at 02:39:10PM -0500, Sasha Levin wrote:
> > > From: Dmitry Baryshkov <[email protected]>
> > >
> > > [ Upstream commit 1d103d6af241dbfc7e11eb9a46dff65db257a37f ]
> > >
> > > On sevral Qualcomm platforms (SC8180X, SM8350, SC8280XP) a call to
> > > UCSI_GET_PDOS for non-PD partners will cause a firmware crash with no
> > > easy way to recover from it. Since we have no easy way to determine
> > > whether the partner really has PD support, shortcut UCSI_GET_PDOS on
> > > such platforms. This allows us to enable UCSI support on such devices.

> > Correct me if I'm wrong Dmitry, but while the commit message makes this
> > sound like a fix, it is not needed unless you backport follow-on patches
> > that enable UCSI on these platforms.
> >
> > So this one can be dropped from all stable queues (unless you're
> > backporting patches that enable new features and that depend on this
> > one).
>
> Exactly. It didn't have the Fixes: tag. So I'm completely unsure why
> it ended up in the autosel queue at all.

AUTOSEL is Sasha's machine-learning based tool for picking potential
fixes also among commits that lack a Fixes tag.

Johan

2024-01-17 16:04:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH AUTOSEL 6.7 044/108] usb: typec: ucsi: fix UCSI on buggy Qualcomm devices

On Wed, Jan 17, 2024 at 02:17:40PM +0200, Dmitry Baryshkov wrote:
> On Wed, 17 Jan 2024 at 10:03, Johan Hovold <[email protected]> wrote:
> >
> > On Tue, Jan 16, 2024 at 02:39:10PM -0500, Sasha Levin wrote:
> > > From: Dmitry Baryshkov <[email protected]>
> > >
> > > [ Upstream commit 1d103d6af241dbfc7e11eb9a46dff65db257a37f ]
> > >
> > > On sevral Qualcomm platforms (SC8180X, SM8350, SC8280XP) a call to
> > > UCSI_GET_PDOS for non-PD partners will cause a firmware crash with no
> > > easy way to recover from it. Since we have no easy way to determine
> > > whether the partner really has PD support, shortcut UCSI_GET_PDOS on
> > > such platforms. This allows us to enable UCSI support on such devices.
> > >
> > > Signed-off-by: Dmitry Baryshkov <[email protected]>
> > > Acked-by: Heikki Krogerus <[email protected]>
> > > Reviewed-by: Neil Armstrong <[email protected]>
> > > Link: https://lore.kernel.org/r/[email protected]
> > > Signed-off-by: Bjorn Andersson <[email protected]>
> > > Signed-off-by: Sasha Levin <[email protected]>
> >
> > Correct me if I'm wrong Dmitry, but while the commit message makes this
> > sound like a fix, it is not needed unless you backport follow-on patches
> > that enable UCSI on these platforms.
> >
> > So this one can be dropped from all stable queues (unless you're
> > backporting patches that enable new features and that depend on this
> > one).
>
> Exactly. It didn't have the Fixes: tag. So I'm completely unsure why
> it ended up in the autosel queue at all.

Based on the text in the subject and in the changelog, it sure looks
like a bugfix to me! Perhaps don't write changelogs that say "fix
SOMETHING on SOMETHING" next time if they really are not a fix :)

thanks,

greg k-h