2024-02-22 21:10:10

by Marco Felsch

[permalink] [raw]
Subject: [PATCH v3 4/4] usb: typec: tcpci: add support to set connector orientation

This add the support to set the optional connector orientation bit which
is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
allows system designers to connect the tcpc orientation pin directly to
the 2:1 ss-mux.

[1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf

Signed-off-by: Marco Felsch <[email protected]>
---
v3:
- no changes
v2:
- Make use of fallthrough

drivers/usb/typec/tcpm/tcpci.c | 44 ++++++++++++++++++++++++++++++++++
include/linux/usb/tcpci.h | 8 +++++++
2 files changed, 52 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 7118551827f6..73a52e7f95c2 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
}

+static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
+{
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, &reg);
+ if (ret < 0)
+ return ret;
+
+ return (reg & mask) == mask;
+}
+
static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
{
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -301,6 +313,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
TCPC_TCPC_CTRL_ORIENTATION : 0);
}

+static int tcpci_set_orientation(struct tcpc_dev *tcpc,
+ enum typec_orientation orientation)
+{
+ struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+ unsigned int reg;
+
+ switch (orientation) {
+ case TYPEC_ORIENTATION_NONE:
+ /* We can't put a single output into high impedance */
+ fallthrough;
+ case TYPEC_ORIENTATION_NORMAL:
+ reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
+ break;
+ case TYPEC_ORIENTATION_REVERSE:
+ reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
+ break;
+ }
+
+ return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
+ TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
+}
+
static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
{
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -808,6 +842,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
if (tcpci->data->vbus_vsafe0v)
tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;

+ if (tcpci->data->set_orientation)
+ tcpci->tcpc.set_orientation = tcpci_set_orientation;
+
err = tcpci_parse_config(tcpci);
if (err < 0)
return ERR_PTR(err);
@@ -851,6 +888,13 @@ static int tcpci_probe(struct i2c_client *client)
if (err < 0)
return err;

+ err = tcpci_check_std_output_cap(chip->data.regmap,
+ TCPC_STD_OUTPUT_CAP_ORIENTATION);
+ if (err < 0)
+ return err;
+
+ chip->data.set_orientation = err;
+
chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
if (IS_ERR(chip->tcpci))
return PTR_ERR(chip->tcpci);
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 467e8045e9f8..f2bfb4250366 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -47,6 +47,9 @@
#define TCPC_SINK_FAST_ROLE_SWAP BIT(0)

#define TCPC_CONFIG_STD_OUTPUT 0x18
+#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
+#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
+#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1

#define TCPC_TCPC_CTRL 0x19
#define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
@@ -127,6 +130,7 @@
#define TCPC_DEV_CAP_2 0x26
#define TCPC_STD_INPUT_CAP 0x28
#define TCPC_STD_OUTPUT_CAP 0x29
+#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)

#define TCPC_MSG_HDR_INFO 0x2e
#define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
@@ -198,12 +202,16 @@ struct tcpci;
* Chip level drivers are expected to check for contaminant and call
* tcpm_clean_port when the port is clean to put the port back into
* toggling state.
+ * @set_orientation:
+ * Optional; Enable setting the connector orientation
+ * CONFIG_STANDARD_OUTPUT (0x18) bit0.
*/
struct tcpci_data {
struct regmap *regmap;
unsigned char TX_BUF_BYTE_x_hidden:1;
unsigned char auto_discharge_disconnect:1;
unsigned char vbus_vsafe0v:1;
+ unsigned char set_orientation:1;

int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
--
2.39.2



2024-02-26 08:13:44

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] usb: typec: tcpci: add support to set connector orientation

On Thu, Feb 22, 2024 at 10:09:03PM +0100, Marco Felsch wrote:
> This add the support to set the optional connector orientation bit which
> is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> allows system designers to connect the tcpc orientation pin directly to
> the 2:1 ss-mux.
>
> [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
>
> Signed-off-by: Marco Felsch <[email protected]>
> ---
> v3:
> - no changes
> v2:
> - Make use of fallthrough
>
> drivers/usb/typec/tcpm/tcpci.c | 44 ++++++++++++++++++++++++++++++++++
> include/linux/usb/tcpci.h | 8 +++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 7118551827f6..73a52e7f95c2 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> }
>
> +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> +{
> + unsigned int reg;
> + int ret;
> +
> + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, &reg);
> + if (ret < 0)
> + return ret;
> +
> + return (reg & mask) == mask;
> +}
> +
> static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> {
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> @@ -301,6 +313,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> TCPC_TCPC_CTRL_ORIENTATION : 0);
> }
>
> +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> + enum typec_orientation orientation)
> +{
> + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> + unsigned int reg;
> +
> + switch (orientation) {
> + case TYPEC_ORIENTATION_NONE:
> + /* We can't put a single output into high impedance */
> + fallthrough;
> + case TYPEC_ORIENTATION_NORMAL:
> + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> + break;
> + case TYPEC_ORIENTATION_REVERSE:
> + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> + break;
> + }
> +
> + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> +}
> +
> static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> {
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> @@ -808,6 +842,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> if (tcpci->data->vbus_vsafe0v)
> tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
>
> + if (tcpci->data->set_orientation)
> + tcpci->tcpc.set_orientation = tcpci_set_orientation;

I don't think that flag is needed - not yet at least. Please just call
tcpci_check_std_output_cap() directly from here.

> err = tcpci_parse_config(tcpci);
> if (err < 0)
> return ERR_PTR(err);
> @@ -851,6 +888,13 @@ static int tcpci_probe(struct i2c_client *client)
> if (err < 0)
> return err;
>
> + err = tcpci_check_std_output_cap(chip->data.regmap,
> + TCPC_STD_OUTPUT_CAP_ORIENTATION);
> + if (err < 0)
> + return err;
> +
> + chip->data.set_orientation = err;
> +
> chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> if (IS_ERR(chip->tcpci))
> return PTR_ERR(chip->tcpci);
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 467e8045e9f8..f2bfb4250366 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -47,6 +47,9 @@
> #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
>
> #define TCPC_CONFIG_STD_OUTPUT 0x18
> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
>
> #define TCPC_TCPC_CTRL 0x19
> #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> @@ -127,6 +130,7 @@
> #define TCPC_DEV_CAP_2 0x26
> #define TCPC_STD_INPUT_CAP 0x28
> #define TCPC_STD_OUTPUT_CAP 0x29
> +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
>
> #define TCPC_MSG_HDR_INFO 0x2e
> #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> @@ -198,12 +202,16 @@ struct tcpci;
> * Chip level drivers are expected to check for contaminant and call
> * tcpm_clean_port when the port is clean to put the port back into
> * toggling state.
> + * @set_orientation:
> + * Optional; Enable setting the connector orientation
> + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> */
> struct tcpci_data {
> struct regmap *regmap;
> unsigned char TX_BUF_BYTE_x_hidden:1;
> unsigned char auto_discharge_disconnect:1;
> unsigned char vbus_vsafe0v:1;
> + unsigned char set_orientation:1;
>
> int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> --
> 2.39.2

--
heikki

2024-02-26 12:28:03

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] usb: typec: tcpci: add support to set connector orientation

Hi,

On 24-02-26, Heikki Krogerus wrote:
> On Thu, Feb 22, 2024 at 10:09:03PM +0100, Marco Felsch wrote:
> > This add the support to set the optional connector orientation bit which
> > is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> > allows system designers to connect the tcpc orientation pin directly to
> > the 2:1 ss-mux.
> >
> > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
> >
> > Signed-off-by: Marco Felsch <[email protected]>
> > ---
> > v3:
> > - no changes
> > v2:
> > - Make use of fallthrough
> >
> > drivers/usb/typec/tcpm/tcpci.c | 44 ++++++++++++++++++++++++++++++++++
> > include/linux/usb/tcpci.h | 8 +++++++
> > 2 files changed, 52 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index 7118551827f6..73a52e7f95c2 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> > return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> > }
> >
> > +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> > +{
> > + unsigned int reg;
> > + int ret;
> > +
> > + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, &reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return (reg & mask) == mask;
> > +}
> > +
> > static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> > {
> > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > @@ -301,6 +313,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> > TCPC_TCPC_CTRL_ORIENTATION : 0);
> > }
> >
> > +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> > + enum typec_orientation orientation)
> > +{
> > + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > + unsigned int reg;
> > +
> > + switch (orientation) {
> > + case TYPEC_ORIENTATION_NONE:
> > + /* We can't put a single output into high impedance */
> > + fallthrough;
> > + case TYPEC_ORIENTATION_NORMAL:
> > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> > + break;
> > + case TYPEC_ORIENTATION_REVERSE:
> > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> > + break;
> > + }
> > +
> > + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> > + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> > +}
> > +
> > static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> > {
> > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > @@ -808,6 +842,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> > if (tcpci->data->vbus_vsafe0v)
> > tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
> >
> > + if (tcpci->data->set_orientation)
> > + tcpci->tcpc.set_orientation = tcpci_set_orientation;
>
> I don't think that flag is needed - not yet at least. Please just call
> tcpci_check_std_output_cap() directly from here.

The reason for having it this way was to not break exsisting user like:
tcpci_rt1711h, tcpci_mt6370, tcpci_maxim which may or may not implement
the TCPC_STD_OUTPUT_CAP_ORIENTATION. This way the users of
tcpci_register_port() can decide by on its own if they do have this
feature or not and how this is checked. I'm fine with your proposal if
you still think that we can check this unconditional.

Regards,
Marco

> > err = tcpci_parse_config(tcpci);
> > if (err < 0)
> > return ERR_PTR(err);
> > @@ -851,6 +888,13 @@ static int tcpci_probe(struct i2c_client *client)
> > if (err < 0)
> > return err;
> >
> > + err = tcpci_check_std_output_cap(chip->data.regmap,
> > + TCPC_STD_OUTPUT_CAP_ORIENTATION);
> > + if (err < 0)
> > + return err;
> > +
> > + chip->data.set_orientation = err;
> > +
> > chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > if (IS_ERR(chip->tcpci))
> > return PTR_ERR(chip->tcpci);
> > diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> > index 467e8045e9f8..f2bfb4250366 100644
> > --- a/include/linux/usb/tcpci.h
> > +++ b/include/linux/usb/tcpci.h
> > @@ -47,6 +47,9 @@
> > #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
> >
> > #define TCPC_CONFIG_STD_OUTPUT 0x18
> > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
> >
> > #define TCPC_TCPC_CTRL 0x19
> > #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> > @@ -127,6 +130,7 @@
> > #define TCPC_DEV_CAP_2 0x26
> > #define TCPC_STD_INPUT_CAP 0x28
> > #define TCPC_STD_OUTPUT_CAP 0x29
> > +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
> >
> > #define TCPC_MSG_HDR_INFO 0x2e
> > #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> > @@ -198,12 +202,16 @@ struct tcpci;
> > * Chip level drivers are expected to check for contaminant and call
> > * tcpm_clean_port when the port is clean to put the port back into
> > * toggling state.
> > + * @set_orientation:
> > + * Optional; Enable setting the connector orientation
> > + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> > */
> > struct tcpci_data {
> > struct regmap *regmap;
> > unsigned char TX_BUF_BYTE_x_hidden:1;
> > unsigned char auto_discharge_disconnect:1;
> > unsigned char vbus_vsafe0v:1;
> > + unsigned char set_orientation:1;
> >
> > int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> > int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> > --
> > 2.39.2
>
> --
> heikki
>

2024-03-07 09:46:43

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] usb: typec: tcpci: add support to set connector orientation

Hi,

gentle ping, since Greg started to pick the other patches.

On 24-02-26, Marco Felsch wrote:
> Hi,
>
> On 24-02-26, Heikki Krogerus wrote:
> > On Thu, Feb 22, 2024 at 10:09:03PM +0100, Marco Felsch wrote:
> > > This add the support to set the optional connector orientation bit which
> > > is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> > > allows system designers to connect the tcpc orientation pin directly to
> > > the 2:1 ss-mux.
> > >
> > > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
> > >
> > > Signed-off-by: Marco Felsch <[email protected]>
> > > ---
> > > v3:
> > > - no changes
> > > v2:
> > > - Make use of fallthrough
> > >
> > > drivers/usb/typec/tcpm/tcpci.c | 44 ++++++++++++++++++++++++++++++++++
> > > include/linux/usb/tcpci.h | 8 +++++++
> > > 2 files changed, 52 insertions(+)
> > >
> > > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > > index 7118551827f6..73a52e7f95c2 100644
> > > --- a/drivers/usb/typec/tcpm/tcpci.c
> > > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > > @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> > > return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> > > }
> > >
> > > +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> > > +{
> > > + unsigned int reg;
> > > + int ret;
> > > +
> > > + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, &reg);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return (reg & mask) == mask;
> > > +}
> > > +
> > > static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> > > {
> > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > @@ -301,6 +313,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> > > TCPC_TCPC_CTRL_ORIENTATION : 0);
> > > }
> > >
> > > +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> > > + enum typec_orientation orientation)
> > > +{
> > > + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > + unsigned int reg;
> > > +
> > > + switch (orientation) {
> > > + case TYPEC_ORIENTATION_NONE:
> > > + /* We can't put a single output into high impedance */
> > > + fallthrough;
> > > + case TYPEC_ORIENTATION_NORMAL:
> > > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> > > + break;
> > > + case TYPEC_ORIENTATION_REVERSE:
> > > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> > > + break;
> > > + }
> > > +
> > > + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> > > + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> > > +}
> > > +
> > > static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> > > {
> > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > @@ -808,6 +842,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> > > if (tcpci->data->vbus_vsafe0v)
> > > tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
> > >
> > > + if (tcpci->data->set_orientation)
> > > + tcpci->tcpc.set_orientation = tcpci_set_orientation;
> >
> > I don't think that flag is needed - not yet at least. Please just call
> > tcpci_check_std_output_cap() directly from here.
>
> The reason for having it this way was to not break exsisting user like:
> tcpci_rt1711h, tcpci_mt6370, tcpci_maxim which may or may not implement
> the TCPC_STD_OUTPUT_CAP_ORIENTATION. This way the users of
> tcpci_register_port() can decide by on its own if they do have this
> feature or not and how this is checked. I'm fine with your proposal if
> you still think that we can check this unconditional.
^
?

Regards,
Marco

> Regards,
> Marco
>
> > > err = tcpci_parse_config(tcpci);
> > > if (err < 0)
> > > return ERR_PTR(err);
> > > @@ -851,6 +888,13 @@ static int tcpci_probe(struct i2c_client *client)
> > > if (err < 0)
> > > return err;
> > >
> > > + err = tcpci_check_std_output_cap(chip->data.regmap,
> > > + TCPC_STD_OUTPUT_CAP_ORIENTATION);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + chip->data.set_orientation = err;
> > > +
> > > chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > > if (IS_ERR(chip->tcpci))
> > > return PTR_ERR(chip->tcpci);
> > > diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> > > index 467e8045e9f8..f2bfb4250366 100644
> > > --- a/include/linux/usb/tcpci.h
> > > +++ b/include/linux/usb/tcpci.h
> > > @@ -47,6 +47,9 @@
> > > #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
> > >
> > > #define TCPC_CONFIG_STD_OUTPUT 0x18
> > > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> > > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> > > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
> > >
> > > #define TCPC_TCPC_CTRL 0x19
> > > #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> > > @@ -127,6 +130,7 @@
> > > #define TCPC_DEV_CAP_2 0x26
> > > #define TCPC_STD_INPUT_CAP 0x28
> > > #define TCPC_STD_OUTPUT_CAP 0x29
> > > +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
> > >
> > > #define TCPC_MSG_HDR_INFO 0x2e
> > > #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> > > @@ -198,12 +202,16 @@ struct tcpci;
> > > * Chip level drivers are expected to check for contaminant and call
> > > * tcpm_clean_port when the port is clean to put the port back into
> > > * toggling state.
> > > + * @set_orientation:
> > > + * Optional; Enable setting the connector orientation
> > > + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> > > */
> > > struct tcpci_data {
> > > struct regmap *regmap;
> > > unsigned char TX_BUF_BYTE_x_hidden:1;
> > > unsigned char auto_discharge_disconnect:1;
> > > unsigned char vbus_vsafe0v:1;
> > > + unsigned char set_orientation:1;
> > >
> > > int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> > > int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> > > --
> > > 2.39.2
> >
> > --
> > heikki
> >
>
>

2024-03-12 11:28:13

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] usb: typec: tcpci: add support to set connector orientation

Hi,

I'm sorry to keep you waiting.

On Mon, Feb 26, 2024 at 01:27:01PM +0100, Marco Felsch wrote:
> Hi,
>
> On 24-02-26, Heikki Krogerus wrote:
> > On Thu, Feb 22, 2024 at 10:09:03PM +0100, Marco Felsch wrote:
> > > This add the support to set the optional connector orientation bit which
> > > is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> > > allows system designers to connect the tcpc orientation pin directly to
> > > the 2:1 ss-mux.
> > >
> > > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
> > >
> > > Signed-off-by: Marco Felsch <[email protected]>
> > > ---
> > > v3:
> > > - no changes
> > > v2:
> > > - Make use of fallthrough
> > >
> > > drivers/usb/typec/tcpm/tcpci.c | 44 ++++++++++++++++++++++++++++++++++
> > > include/linux/usb/tcpci.h | 8 +++++++
> > > 2 files changed, 52 insertions(+)
> > >
> > > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > > index 7118551827f6..73a52e7f95c2 100644
> > > --- a/drivers/usb/typec/tcpm/tcpci.c
> > > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > > @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> > > return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> > > }
> > >
> > > +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> > > +{
> > > + unsigned int reg;
> > > + int ret;
> > > +
> > > + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, &reg);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return (reg & mask) == mask;
> > > +}
> > > +
> > > static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> > > {
> > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > @@ -301,6 +313,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> > > TCPC_TCPC_CTRL_ORIENTATION : 0);
> > > }
> > >
> > > +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> > > + enum typec_orientation orientation)
> > > +{
> > > + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > + unsigned int reg;
> > > +
> > > + switch (orientation) {
> > > + case TYPEC_ORIENTATION_NONE:
> > > + /* We can't put a single output into high impedance */
> > > + fallthrough;
> > > + case TYPEC_ORIENTATION_NORMAL:
> > > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> > > + break;
> > > + case TYPEC_ORIENTATION_REVERSE:
> > > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> > > + break;
> > > + }
> > > +
> > > + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> > > + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> > > +}
> > > +
> > > static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> > > {
> > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > @@ -808,6 +842,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> > > if (tcpci->data->vbus_vsafe0v)
> > > tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
> > >
> > > + if (tcpci->data->set_orientation)
> > > + tcpci->tcpc.set_orientation = tcpci_set_orientation;
> >
> > I don't think that flag is needed - not yet at least. Please just call
> > tcpci_check_std_output_cap() directly from here.
>
> The reason for having it this way was to not break exsisting user like:
> tcpci_rt1711h, tcpci_mt6370, tcpci_maxim which may or may not implement
> the TCPC_STD_OUTPUT_CAP_ORIENTATION. This way the users of
> tcpci_register_port() can decide by on its own if they do have this
> feature or not and how this is checked. I'm fine with your proposal if
> you still think that we can check this unconditional.

Ah, I failed to consider those other glue drivers. This is fine as it
is. I'm really sorry about the delay.

Br,

--
heikki

2024-03-12 15:06:50

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] usb: typec: tcpci: add support to set connector orientation

On Thu, Feb 22, 2024 at 10:09:03PM +0100, Marco Felsch wrote:
> This add the support to set the optional connector orientation bit which
> is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> allows system designers to connect the tcpc orientation pin directly to
> the 2:1 ss-mux.
>
> [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
>
> Signed-off-by: Marco Felsch <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>


--
heikki