diff mbox series

[2/8] usb: typec: altmode: add low level altmode configuration helper

Message ID 20240416-ucsi-glink-altmode-v1-2-890db00877ac@linaro.org (mailing list archive)
State Superseded
Headers show
Series usb: typec: ucsi: glink: merge in altmode support | expand

Commit Message

Dmitry Baryshkov April 16, 2024, 2:20 a.m. UTC
In some obscure cases (Qualcomm PMIC Glink) altmode is completely
handled by the firmware. Linux does not get proper partner altmode info.
Instead we get the notification once the altmode is negotiated and
entered (or left). However even in such a case the driver has to switch
board components (muxes, switches and retimers) according to the altmode
selected by the hardware.

We can not use existing typec_altmode_enter() / typec_altmode_exit() /
typec_altmode_notify() functions in such a case, since there is no
corresponding partner's altmode instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/bus.c           | 34 ++++++++++++++++++++++++++++++++++
 include/linux/usb/typec_altmode.h |  3 +++
 2 files changed, 37 insertions(+)

Comments

Konrad Dybcio April 16, 2024, 2:32 p.m. UTC | #1
On 4/16/24 04:20, Dmitry Baryshkov wrote:
> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> handled by the firmware. Linux does not get proper partner altmode info.
> Instead we get the notification once the altmode is negotiated and
> entered (or left). However even in such a case the driver has to switch
> board components (muxes, switches and retimers) according to the altmode
> selected by the hardware.
> 
> We can not use existing typec_altmode_enter() / typec_altmode_exit() /
> typec_altmode_notify() functions in such a case, since there is no
> corresponding partner's altmode instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Should this now be called from e.g. typec_almode_notify to limit
duplication?

Konrad
Dmitry Baryshkov April 16, 2024, 2:48 p.m. UTC | #2
On Tue, 16 Apr 2024 at 17:32, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 4/16/24 04:20, Dmitry Baryshkov wrote:
> > In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> > handled by the firmware. Linux does not get proper partner altmode info.
> > Instead we get the notification once the altmode is negotiated and
> > entered (or left). However even in such a case the driver has to switch
> > board components (muxes, switches and retimers) according to the altmode
> > selected by the hardware.
> >
> > We can not use existing typec_altmode_enter() / typec_altmode_exit() /
> > typec_altmode_notify() functions in such a case, since there is no
> > corresponding partner's altmode instance.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
>
> Should this now be called from e.g. typec_almode_notify to limit
> duplication?

typec_altmode_notify works only if there is an altmode->partner (which
we don't have with pmic-glink).
Konrad Dybcio April 16, 2024, 2:57 p.m. UTC | #3
On 4/16/24 16:48, Dmitry Baryshkov wrote:
> On Tue, 16 Apr 2024 at 17:32, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 4/16/24 04:20, Dmitry Baryshkov wrote:
>>> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
>>> handled by the firmware. Linux does not get proper partner altmode info.
>>> Instead we get the notification once the altmode is negotiated and
>>> entered (or left). However even in such a case the driver has to switch
>>> board components (muxes, switches and retimers) according to the altmode
>>> selected by the hardware.
>>>
>>> We can not use existing typec_altmode_enter() / typec_altmode_exit() /
>>> typec_altmode_notify() functions in such a case, since there is no
>>> corresponding partner's altmode instance.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>
>> Should this now be called from e.g. typec_almode_notify to limit
>> duplication?
> 
> typec_altmode_notify works only if there is an altmode->partner (which
> we don't have with pmic-glink).

Yes and this seems to be an excerpt from these functions, should they
now drop that code and call this function instead, so as not to have
it in the tree twice?

Konrad
Dmitry Baryshkov April 16, 2024, 3:20 p.m. UTC | #4
On Tue, 16 Apr 2024 at 17:57, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 4/16/24 16:48, Dmitry Baryshkov wrote:
> > On Tue, 16 Apr 2024 at 17:32, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >>
> >>
> >> On 4/16/24 04:20, Dmitry Baryshkov wrote:
> >>> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> >>> handled by the firmware. Linux does not get proper partner altmode info.
> >>> Instead we get the notification once the altmode is negotiated and
> >>> entered (or left). However even in such a case the driver has to switch
> >>> board components (muxes, switches and retimers) according to the altmode
> >>> selected by the hardware.
> >>>
> >>> We can not use existing typec_altmode_enter() / typec_altmode_exit() /
> >>> typec_altmode_notify() functions in such a case, since there is no
> >>> corresponding partner's altmode instance.
> >>>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>
> >> Should this now be called from e.g. typec_almode_notify to limit
> >> duplication?
> >
> > typec_altmode_notify works only if there is an altmode->partner (which
> > we don't have with pmic-glink).
>
> Yes and this seems to be an excerpt from these functions, should they
> now drop that code and call this function instead, so as not to have
> it in the tree twice?

I thought about it, but then I abandoned this idea. The
typec_altmode_notify() has its own use case, normal altmode drivers.
It is an error to call it if there is no partner registered, etc. So I
wanted to preserve error checks in that function instead of dropping
them. The significant part of the code is shared anyway thanks to
typec_altmode_set_switches().
diff mbox series

Patch

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index 6ea103e1abae..68f3908401c6 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -67,6 +67,40 @@  static int typec_altmode_set_state(struct typec_altmode *adev,
 	return typec_altmode_set_switches(port_altmode, conf, data);
 }
 
+/**
+ * typec_altmode_set_port - set the altmode configuration
+ * @conf: Alternate mode specific configuration value
+ * @dVata: Alternate mode specific data
+ *
+ * This function allows configuring muxes and retimer for the selected altmode.
+ * This function may only be used by the special case drivers, that handle
+ * the altmode negotiation by the alternative means and thus have no
+ * corresponding typec_altmode instance for the parnter.
+ */
+int typec_altmode_set_port(struct typec_altmode *adev,
+			   unsigned long conf, void *data)
+{
+	bool is_port;
+	struct altmode *altmode;
+	int ret;
+
+	if (!adev)
+		return 0;
+
+	altmode = to_altmode(adev);
+	is_port = is_typec_port(adev->dev.parent);
+
+	if (altmode->partner || !is_port)
+		return -EINVAL;
+
+	ret = typec_altmode_set_switches(altmode, conf, data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(typec_altmode_set_port);
+
 /* -------------------------------------------------------------------------- */
 /* Common API */
 
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..d78a9618bedf 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -77,6 +77,9 @@  int typec_altmode_notify(struct typec_altmode *altmode, unsigned long conf,
 const struct typec_altmode *
 typec_altmode_get_partner(struct typec_altmode *altmode);
 
+int typec_altmode_set_port(struct typec_altmode *altmode, unsigned long conf,
+			   void *data);
+
 /**
  * struct typec_cable_ops - Cable alternate mode operations vector
  * @enter: Operations to be executed with Enter Mode Command