Hi Heikki,
Thanks for the patches.
On Thu, May 07, 2020 at 06:08:58PM +0300, Heikki Krogerus wrote:
> The SBU and HSL orientation may be fixed/static from the mux
> PoW. Apparently the retimer may take care of the orientation
> of these lines. Handling the static SBU (AUX) and HSL
> orientation with device properties.
>
> If the SBU orientation is static, a device property
> "sbu-orintation" can be used. When the property exists, the
> driver always sets the SBU orientation according to the
> property value, and when it's not set, the driver uses the
> cable plug orientation with SBU.
>
> And with static HSL orientation, "hsl-orientation" device
> property can be used in the same way.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/typec/mux/intel_pmc_mux.c | 42 +++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
> index f5c5e0aef66f..1aac218099f3 100644
> --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> @@ -91,6 +91,9 @@ struct pmc_usb_port {
>
> u8 usb2_port;
> u8 usb3_port;
> +
> + enum typec_orientation sbu_orientation;
> + enum typec_orientation hsl_orientation;
> };
>
> struct pmc_usb {
> @@ -99,6 +102,22 @@ struct pmc_usb {
> struct pmc_usb_port *port;
> };
>
> +static int sbu_orientation(struct pmc_usb_port *port)
> +{
> + if (port->sbu_orientation)
> + return port->sbu_orientation - 1;
> +
> + return port->orientation - 1;
> +}
> +
> +static int hsl_orientation(struct pmc_usb_port *port)
> +{
> + if (port->hsl_orientation)
> + return port->hsl_orientation - 1;
> +
> + return port->orientation - 1;
> +}
> +
> static int pmc_usb_command(struct pmc_usb_port *port, u8 *msg, u32 len)
> {
> u8 response[4];
> @@ -151,8 +170,9 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct typec_mux_state *state)
>
> req.mode_data = (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
> req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
> - req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_AUX_SHIFT;
> - req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_HSL_SHIFT;
> +
> + req.mode_data |= sbu_orientation(port) << PMC_USB_ALTMODE_ORI_AUX_SHIFT;
I'm curious to know what would happen when sbu-orientation == "normal".
That means |port->sbu_orientation| == 1.
It sounds like what should happen is the AUX_SHIFT orientation
setting should follow what |port->orientation| is, but here it
looks like it will always be set to |port->sbu_orientation - 1|, i.e 0,
even if port->orientation == TYPEC_ORIENTATION_REVERSE, i.e 2, meaning
it should be set to 1 ?
Apologies if I misunderstood the code...
Best regards,
> + req.mode_data |= hsl_orientation(port) << PMC_USB_ALTMODE_ORI_HSL_SHIFT;
>
> req.mode_data |= (state->mode - TYPEC_STATE_MODAL) <<
> PMC_USB_ALTMODE_DP_MODE_SHIFT;
> @@ -173,8 +193,9 @@ pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
>
> req.mode_data = (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
> req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
> - req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_AUX_SHIFT;
> - req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_HSL_SHIFT;
> +
> + req.mode_data |= sbu_orientation(port) << PMC_USB_ALTMODE_ORI_AUX_SHIFT;
> + req.mode_data |= hsl_orientation(port) << PMC_USB_ALTMODE_ORI_HSL_SHIFT;
>
> if (TBT_ADAPTER(data->device_mode) == TBT_ADAPTER_TBT3)
> req.mode_data |= PMC_USB_ALTMODE_TBT_TYPE;
> @@ -211,8 +232,8 @@ static int pmc_usb_connect(struct pmc_usb_port *port)
> msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
>
> msg[1] = port->usb2_port << PMC_USB_MSG_USB2_PORT_SHIFT;
> - msg[1] |= (port->orientation - 1) << PMC_USB_MSG_ORI_HSL_SHIFT;
> - msg[1] |= (port->orientation - 1) << PMC_USB_MSG_ORI_AUX_SHIFT;
> + msg[1] |= hsl_orientation(port) << PMC_USB_MSG_ORI_HSL_SHIFT;
> + msg[1] |= sbu_orientation(port) << PMC_USB_MSG_ORI_AUX_SHIFT;
>
> return pmc_usb_command(port, msg, sizeof(msg));
> }
> @@ -296,6 +317,7 @@ static int pmc_usb_register_port(struct pmc_usb *pmc, int index,
> struct usb_role_switch_desc desc = { };
> struct typec_switch_desc sw_desc = { };
> struct typec_mux_desc mux_desc = { };
> + const char *str;
> int ret;
>
> ret = fwnode_property_read_u8(fwnode, "usb2-port", &port->usb2_port);
> @@ -306,6 +328,14 @@ static int pmc_usb_register_port(struct pmc_usb *pmc, int index,
> if (ret)
> return ret;
>
> + ret = fwnode_property_read_string(fwnode, "sbu-orientation", &str);
> + if (!ret)
> + port->sbu_orientation = typec_find_orientation(str);
> +
> + ret = fwnode_property_read_string(fwnode, "hsl-orientation", &str);
> + if (!ret)
> + port->hsl_orientation = typec_find_orientation(str);
> +
> port->num = index;
> port->pmc = pmc;
>
> --
> 2.26.2
>
