* Re: [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged
2024-10-17 16:01 [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged Abel Vesa
@ 2024-10-21 12:43 ` Heikki Krogerus
2024-10-21 12:52 ` Neil Armstrong
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2024-10-21 12:43 UTC (permalink / raw)
To: Abel Vesa
Cc: Greg Kroah-Hartman, Bjorn Andersson, Konrad Dybcio,
Neil Armstrong, Dmitry Baryshkov, Johan Hovold, linux-usb,
linux-kernel, linux-arm-msm
On Thu, Oct 17, 2024 at 07:01:01PM +0300, Abel Vesa wrote:
> Currently, the ucsi glink client is only reporting orientation normal or
> reversed, based on the level of the gpio. On unplug, it defaults to
> orientation normal instead of none. This confuses some of the orientation
> switches drivers as they might rely on orientation none in order to
> configure the HW in some sort of safe mode. So propagate the orientation
> none instead when the connector status flags says cable is disconnected.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 3e4d88ab338e50d4265df15fc960907c36675282..b3bc02e4b0427a894c5b5df470af47433145243e 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -185,6 +185,11 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> int orientation;
>
> + if (!(con->status.flags & UCSI_CONSTAT_CONNECTED)) {
> + typec_set_orientation(con->port, TYPEC_ORIENTATION_NONE);
> + return;
> + }
> +
> if (con->num >= PMIC_GLINK_MAX_PORTS ||
> !ucsi->port_orientation[con->num - 1])
> return;
This looks reasonable to me. FWIW:
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
thanks,
--
heikki
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged
2024-10-17 16:01 [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged Abel Vesa
2024-10-21 12:43 ` Heikki Krogerus
@ 2024-10-21 12:52 ` Neil Armstrong
2024-10-22 16:33 ` Johan Hovold
2024-10-22 22:01 ` Bryan O'Donoghue
3 siblings, 0 replies; 6+ messages in thread
From: Neil Armstrong @ 2024-10-21 12:52 UTC (permalink / raw)
To: Abel Vesa, Heikki Krogerus, Greg Kroah-Hartman
Cc: Bjorn Andersson, Konrad Dybcio, Dmitry Baryshkov, Johan Hovold,
linux-usb, linux-kernel, linux-arm-msm
On 17/10/2024 18:01, Abel Vesa wrote:
> Currently, the ucsi glink client is only reporting orientation normal or
> reversed, based on the level of the gpio. On unplug, it defaults to
> orientation normal instead of none. This confuses some of the orientation
> switches drivers as they might rely on orientation none in order to
> configure the HW in some sort of safe mode. So propagate the orientation
> none instead when the connector status flags says cable is disconnected.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 3e4d88ab338e50d4265df15fc960907c36675282..b3bc02e4b0427a894c5b5df470af47433145243e 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -185,6 +185,11 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> int orientation;
>
> + if (!(con->status.flags & UCSI_CONSTAT_CONNECTED)) {
> + typec_set_orientation(con->port, TYPEC_ORIENTATION_NONE);
> + return;
> + }
> +
> if (con->num >= PMIC_GLINK_MAX_PORTS ||
> !ucsi->port_orientation[con->num - 1])
> return;
>
> ---
> base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
> change-id: 20241017-usb-typec-ucsi-glink-add-orientation-none-73f1f2522999
>
> Best regards,
Looks safe with phy-qcom-qmp-combo/wcd939x-usbss/fsa4480/nb7vpq904m
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged
2024-10-17 16:01 [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged Abel Vesa
2024-10-21 12:43 ` Heikki Krogerus
2024-10-21 12:52 ` Neil Armstrong
@ 2024-10-22 16:33 ` Johan Hovold
2024-10-23 9:29 ` Abel Vesa
2024-10-22 22:01 ` Bryan O'Donoghue
3 siblings, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2024-10-22 16:33 UTC (permalink / raw)
To: Abel Vesa
Cc: Heikki Krogerus, Greg Kroah-Hartman, Bjorn Andersson,
Konrad Dybcio, Neil Armstrong, Dmitry Baryshkov, linux-usb,
linux-kernel, linux-arm-msm
On Thu, Oct 17, 2024 at 07:01:01PM +0300, Abel Vesa wrote:
> Currently, the ucsi glink client is only reporting orientation normal or
> reversed, based on the level of the gpio. On unplug, it defaults to
> orientation normal instead of none. This confuses some of the orientation
> switches drivers as they might rely on orientation none in order to
> configure the HW in some sort of safe mode.
Can you be more specific here (e.g. so that reviewers and backporter can
determine whether this is a fix that should be backported to stable)?
Which driver is confused? How does this manifest itself?
Is this an issue today? Or something you need for future work, etc?
> So propagate the orientation
> none instead when the connector status flags says cable is disconnected.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 3e4d88ab338e50d4265df15fc960907c36675282..b3bc02e4b0427a894c5b5df470af47433145243e 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -185,6 +185,11 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> int orientation;
>
> + if (!(con->status.flags & UCSI_CONSTAT_CONNECTED)) {
> + typec_set_orientation(con->port, TYPEC_ORIENTATION_NONE);
> + return;
> + }
> +
> if (con->num >= PMIC_GLINK_MAX_PORTS ||
> !ucsi->port_orientation[con->num - 1])
> return;
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged
2024-10-22 16:33 ` Johan Hovold
@ 2024-10-23 9:29 ` Abel Vesa
0 siblings, 0 replies; 6+ messages in thread
From: Abel Vesa @ 2024-10-23 9:29 UTC (permalink / raw)
To: Johan Hovold
Cc: Heikki Krogerus, Greg Kroah-Hartman, Bjorn Andersson,
Konrad Dybcio, Neil Armstrong, Dmitry Baryshkov, linux-usb,
linux-kernel, linux-arm-msm
On 24-10-22 18:33:18, Johan Hovold wrote:
> On Thu, Oct 17, 2024 at 07:01:01PM +0300, Abel Vesa wrote:
> > Currently, the ucsi glink client is only reporting orientation normal or
> > reversed, based on the level of the gpio. On unplug, it defaults to
> > orientation normal instead of none. This confuses some of the orientation
> > switches drivers as they might rely on orientation none in order to
> > configure the HW in some sort of safe mode.
>
> Can you be more specific here (e.g. so that reviewers and backporter can
> determine whether this is a fix that should be backported to stable)?
I didn't add a fixes tag here on purpose as I see this as an
improvement. Nothing wrong with just setting the orientation to normal
when cable is unplugged. But makes more sense to set it to none.
>
> Which driver is confused? How does this manifest itself?
>
> Is this an issue today? Or something you need for future work, etc?
None of the upstream orientation switches drivers currently need
the orientation as none on unplug.
The new Parade PS8830 driver that is on the list for review is indeed helped by
this changed. But we can circumvent it there as well, if necessary.
My reason for this change was basically like: Since we can use the
connector status flags to figure out if the cable is plugged, we can
take this a step further and propagate orientation none on unplug.
Therefore, improvement rather than fix.
Anyway, if you still think that there should be a Fixes tag, please let
me know and I'll add it.
>
> > So propagate the orientation
> > none instead when the connector status flags says cable is disconnected.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> > drivers/usb/typec/ucsi/ucsi_glink.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> > index 3e4d88ab338e50d4265df15fc960907c36675282..b3bc02e4b0427a894c5b5df470af47433145243e 100644
> > --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> > +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> > @@ -185,6 +185,11 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> > struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> > int orientation;
> >
> > + if (!(con->status.flags & UCSI_CONSTAT_CONNECTED)) {
> > + typec_set_orientation(con->port, TYPEC_ORIENTATION_NONE);
> > + return;
> > + }
> > +
> > if (con->num >= PMIC_GLINK_MAX_PORTS ||
> > !ucsi->port_orientation[con->num - 1])
> > return;
>
> Johan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged
2024-10-17 16:01 [PATCH RFC] usb: typec: ucsi: Set orientation as none when connector is unplugged Abel Vesa
` (2 preceding siblings ...)
2024-10-22 16:33 ` Johan Hovold
@ 2024-10-22 22:01 ` Bryan O'Donoghue
3 siblings, 0 replies; 6+ messages in thread
From: Bryan O'Donoghue @ 2024-10-22 22:01 UTC (permalink / raw)
To: Abel Vesa, Heikki Krogerus, Greg Kroah-Hartman
Cc: Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Dmitry Baryshkov,
Johan Hovold, linux-usb, linux-kernel, linux-arm-msm
On 17/10/2024 17:01, Abel Vesa wrote:
> Currently, the ucsi glink client is only reporting orientation normal or
> reversed, based on the level of the gpio. On unplug, it defaults to
> orientation normal instead of none. This confuses some of the orientation
> switches drivers as they might rely on orientation none in order to
> configure the HW in some sort of safe mode. So propagate the orientation
> none instead when the connector status flags says cable is disconnected.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/usb/typec/ucsi/ucsi_glink.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 3e4d88ab338e50d4265df15fc960907c36675282..b3bc02e4b0427a894c5b5df470af47433145243e 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -185,6 +185,11 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> int orientation;
>
> + if (!(con->status.flags & UCSI_CONSTAT_CONNECTED)) {
> + typec_set_orientation(con->port, TYPEC_ORIENTATION_NONE);
> + return;
> + }
> +
> if (con->num >= PMIC_GLINK_MAX_PORTS ||
> !ucsi->port_orientation[con->num - 1])
> return;
>
> ---
> base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
> change-id: 20241017-usb-typec-ucsi-glink-add-orientation-none-73f1f2522999
>
> Best regards,
We discussed this on a meeting and the logic makes perfect sense i.e.
when the cable gets yanked the orientation shouldn't be assumed to be
anything => orientation is none.
Re Johan's comments needs a Fixes tag though, assuming that's applied.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread