[PATCH 0/2] Send acknowledgment to ec from cors_ec_typec

[PATCH 0/2] Send acknowledgment to ec from cors_ec_typec

[Utkarsh Patel]

This adds mechanism of sending mux configuration acknowledgment from kernel
to EC. It also modifies cros_typec_cmds_supported() to support multiple
feature flags.

Utkarsh Patel (2):
  platform/chrome: cros_ec_typec: Parameterize
    cros_typec_cmds_supported()
  platform/chrome: cros_ec_typec: Send mux configuration acknowledgment
    to EC

 drivers/platform/chrome/cros_ec_typec.c       | 28 +++++++++++++++----
 .../linux/platform_data/cros_ec_commands.h    | 17 +++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH 1/2] platform/chrome: cros_ec_typec: Parameterize cros_typec_cmds_supported()

[Utkarsh Patel]

cros_typec_cmds_supported() is currently being used to check only one
feature flag.
Add a new feature parameter to it so that it can be used to check
multiple feature flags supported in cros_ec.
Rename cros_typec_cmds_supported() to cros_typec_feature_supported().

Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
---
 drivers/platform/chrome/cros_ec_typec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index ce031a10eb1b..650aa5332055 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -829,8 +829,8 @@ static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
 	return 0;
 }
 
-/* Check the EC feature flags to see if TYPEC_* commands are supported. */
-static int cros_typec_cmds_supported(struct cros_typec_data *typec)
+/* Check the EC feature flags to see if TYPEC_* features are supported. */
+static int cros_typec_feature_supported(struct cros_typec_data *typec, enum ec_feature_code feature)
 {
 	struct ec_response_get_features resp = {};
 	int ret;
@@ -839,11 +839,12 @@ static int cros_typec_cmds_supported(struct cros_typec_data *typec)
 				    &resp, sizeof(resp));
 	if (ret < 0) {
 		dev_warn(typec->dev,
-			 "Failed to get features, assuming typec commands unsupported.\n");
+			 "Failed to get features, assuming typec feature=%d unsupported.\n",
+			 feature);
 		return 0;
 	}
 
-	return resp.flags[EC_FEATURE_TYPEC_CMD / 32] & EC_FEATURE_MASK_1(EC_FEATURE_TYPEC_CMD);
+	return resp.flags[feature / 32] & EC_FEATURE_MASK_1(feature);
 }
 
 static void cros_typec_port_work(struct work_struct *work)
@@ -905,7 +906,8 @@ static int cros_typec_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	typec->typec_cmd_supported = !!cros_typec_cmds_supported(typec);
+	typec->typec_cmd_supported = !!cros_typec_feature_supported(typec,
+					EC_FEATURE_TYPEC_CMD);
 
 	ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
 				    &resp, sizeof(resp));
-- 
2.25.1

[PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC

[Utkarsh Patel]

In some corner cases downgrade of the superspeed typec device(e.g. Dell
typec Dock, apple dongle) was seen because before the SOC mux configuration
finishes, EC starts configuring the next mux state.

With this change, once the SOC mux is configured, kernel will send an
acknowledgment to EC via Host command EC_CMD_USB_PD_MUX_ACK [1].
After sending the host event EC will wait for the acknowledgment from
kernel before starting the PD negotiation for the next mux state. This
helps to have a framework to build better error handling along with the
synchronization of timing sensitive mux states.

This change also brings in corresponding EC header updates from the EC code
base [1].

[1]:
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h

Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
---
 drivers/platform/chrome/cros_ec_typec.c        | 16 ++++++++++++++++
 include/linux/platform_data/cros_ec_commands.h | 17 +++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 650aa5332055..e6abe205890c 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -74,6 +74,7 @@ struct cros_typec_data {
 	struct notifier_block nb;
 	struct work_struct port_work;
 	bool typec_cmd_supported;
+	bool needs_mux_ack;
 };
 
 static int cros_typec_parse_port_props(struct typec_capability *cap,
@@ -503,6 +504,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 				struct ec_response_usb_pd_control_v2 *pd_ctrl)
 {
 	struct cros_typec_port *port = typec->ports[port_num];
+	struct ec_params_usb_pd_mux_ack mux_ack;
 	enum typec_orientation orientation;
 	int ret;
 
@@ -543,6 +545,18 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 		ret = -ENOTSUPP;
 	}
 
+	if (!typec->needs_mux_ack)
+		return ret;
+
+	/* Sending Acknowledgment to EC */
+	mux_ack.port = port_num;
+
+	if (cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_MUX_ACK, &mux_ack,
+				  sizeof(mux_ack), NULL, 0) < 0)
+		dev_warn(typec->dev,
+			 "Failed to send Mux ACK to EC for port: %d\n",
+			 port_num);
+
 	return ret;
 }
 
@@ -908,6 +922,8 @@ static int cros_typec_probe(struct platform_device *pdev)
 
 	typec->typec_cmd_supported = !!cros_typec_feature_supported(typec,
 					EC_FEATURE_TYPEC_CMD);
+	typec->needs_mux_ack = !!cros_typec_feature_supported(typec,
+					EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
 
 	ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
 				    &resp, sizeof(resp));
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 7f54fdcdd8cb..3b53e45cb5a0 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -1286,6 +1286,16 @@ enum ec_feature_code {
 	EC_FEATURE_ISH = 40,
 	/* New TCPMv2 TYPEC_ prefaced commands supported */
 	EC_FEATURE_TYPEC_CMD = 41,
+	/*
+	 * The EC will wait for direction from the AP to enter Type-C alternate
+	 * modes or USB4.
+	 */
+	EC_FEATURE_TYPEC_REQUIRE_AP_MODE_ENTRY = 42,
+	/*
+	 * The EC will wait for an acknowledge from the AP after setting the
+	 * mux.
+	 */
+	EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK = 43,
 };
 
 #define EC_FEATURE_MASK_0(event_code) BIT(event_code % 32)
@@ -6054,6 +6064,13 @@ struct ec_params_charger_control {
 	uint8_t allow_charging;
 } __ec_align_size1;
 
+/* Get ACK from the USB-C SS muxes */
+#define EC_CMD_USB_PD_MUX_ACK 0x0603
+
+struct ec_params_usb_pd_mux_ack {
+	uint8_t port; /* USB-C port number */
+} __ec_align1;
+
 /*****************************************************************************/
 /*
  * Reserve a range of host commands for board-specific, experimental, or
-- 
2.25.1

Re: [PATCH 1/2] platform/chrome: cros_ec_typec: Parameterize cros_typec_cmds_supported()

[Prashant Malani]

Hi Utkarsh,

On Wed, Dec 09, 2020 at 10:09:02PM -0800, Utkarsh Patel wrote:
> cros_typec_cmds_supported() is currently being used to check only one
> feature flag.
> Add a new feature parameter to it so that it can be used to check
> multiple feature flags supported in cros_ec.
> Rename cros_typec_cmds_supported() to cros_typec_feature_supported().
> 
> Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
Reviewed-by: Prashant Malani <pmalani@chromium.org>

Thanks,

-Prashant

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC

[Prashant Malani]

Hi Utkarsh,

On Wed, Dec 09, 2020 at 10:09:03PM -0800, Utkarsh Patel wrote:
> In some corner cases downgrade of the superspeed typec device(e.g. Dell
> typec Dock, apple dongle) was seen because before the SOC mux configuration
> finishes, EC starts configuring the next mux state.
> 
> With this change, once the SOC mux is configured, kernel will send an
> acknowledgment to EC via Host command EC_CMD_USB_PD_MUX_ACK [1].
> After sending the host event EC will wait for the acknowledgment from
> kernel before starting the PD negotiation for the next mux state. This
> helps to have a framework to build better error handling along with the
> synchronization of timing sensitive mux states.
> 
> This change also brings in corresponding EC header updates from the EC code
> base [1].
> 
> [1]:
> https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h
> 
> Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>

I'm not sure what the maintainers' preference is for the header (same
patch or separate patch). FWIW:

Reviewed-by: Prashant Malani <pmalani@chromium.org>

Thanks,

-Prashant

Re: [PATCH 0/2] Send acknowledgment to ec from cors_ec_typec

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

Hi Utkarsh,

On Wed, 9 Dec 2020 22:09:01 -0800, Utkarsh Patel wrote:
> This adds mechanism of sending mux configuration acknowledgment from kernel
> to EC. It also modifies cros_typec_cmds_supported() to support multiple
> feature flags.
> 
> Utkarsh Patel (2):
>   platform/chrome: cros_ec_typec: Parameterize
>     cros_typec_cmds_supported()
>   platform/chrome: cros_ec_typec: Send mux configuration acknowledgment
>     to EC
> 
> [...]

Applied, thanks!

[1/2] platform/chrome: cros_ec_typec: Parameterize cros_typec_cmds_supported()
      commit: ba8ce515454e1fc5e73ff8989c18c596a3449fef
[2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC
      commit: 8553a979fcd03448a4096c7d431b7ee1a52bfca3

Best regards,
-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

Hi Prashant and Utkarash,

On Mon, Dec 21, 2020 at 04:45:38PM -0800, Prashant Malani wrote:
> Hi Utkarsh,
> 
> On Wed, Dec 09, 2020 at 10:09:03PM -0800, Utkarsh Patel wrote:
> > In some corner cases downgrade of the superspeed typec device(e.g. Dell
> > typec Dock, apple dongle) was seen because before the SOC mux configuration
> > finishes, EC starts configuring the next mux state.
> > 
> > With this change, once the SOC mux is configured, kernel will send an
> > acknowledgment to EC via Host command EC_CMD_USB_PD_MUX_ACK [1].
> > After sending the host event EC will wait for the acknowledgment from
> > kernel before starting the PD negotiation for the next mux state. This
> > helps to have a framework to build better error handling along with the
> > synchronization of timing sensitive mux states.
> > 
> > This change also brings in corresponding EC header updates from the EC code
> > base [1].
> > 
> > [1]:
> > https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h
> > 
> > Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
> 
> I'm not sure what the maintainers' preference is for the header (same
> patch or separate patch). FWIW:
> 
> Reviewed-by: Prashant Malani <pmalani@chromium.org>

I asked Gwendal to look over this change. It looks good to him, so I've gone
ahead and merged it.

At some point we have to completely sync ec_commands.h, but the change here
is well contained, so it minimally adds to the delta between the EC codebase's
header and the kernel's.

Thanks,
Benson
-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 5457 bytes --]

Hi Utkarsh,

On Wed, Dec 09, 2020 at 10:09:03PM -0800, Utkarsh Patel wrote:
> In some corner cases downgrade of the superspeed typec device(e.g. Dell
> typec Dock, apple dongle) was seen because before the SOC mux configuration
> finishes, EC starts configuring the next mux state.
> 
> With this change, once the SOC mux is configured, kernel will send an
> acknowledgment to EC via Host command EC_CMD_USB_PD_MUX_ACK [1].
> After sending the host event EC will wait for the acknowledgment from
> kernel before starting the PD negotiation for the next mux state. This
> helps to have a framework to build better error handling along with the
> synchronization of timing sensitive mux states.
> 
> This change also brings in corresponding EC header updates from the EC code
> base [1].
> 
> [1]:
> https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/include/ec_commands.h
> 
> Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
> ---
>  drivers/platform/chrome/cros_ec_typec.c        | 16 ++++++++++++++++
>  include/linux/platform_data/cros_ec_commands.h | 17 +++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 650aa5332055..e6abe205890c 100644

I went ahead and staged this commit, but just for your information, applying
this patch was problematic because the sha1 used here didn't match anything
in the upstream kernel.

Here's the error when I try to apply this using git am:
Applying: platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC
error: sha1 information is lacking or useless (drivers/platform/chrome/cros_ec_typec.c).
error: could not build fake ancestor
Patch failed at 0001 platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC


Please double check when you run git-format-patch that the branch you are
working on comes from publicly available git repos, ideally, based on linus's
tagged releases. This will ensure things apply cleanly on my side.

I've gone ahead and staged this, but could you please sanity check that the
result is as intended?

https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/commit/?h=cros-ec-typec-for-5.12&id=8553a979fcd03448a4096c7d431b7ee1a52bfca3

Thank you,
Benson

> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -74,6 +74,7 @@ struct cros_typec_data {
>  	struct notifier_block nb;
>  	struct work_struct port_work;
>  	bool typec_cmd_supported;
> +	bool needs_mux_ack;
>  };
>  
>  static int cros_typec_parse_port_props(struct typec_capability *cap,
> @@ -503,6 +504,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
>  				struct ec_response_usb_pd_control_v2 *pd_ctrl)
>  {
>  	struct cros_typec_port *port = typec->ports[port_num];
> +	struct ec_params_usb_pd_mux_ack mux_ack;
>  	enum typec_orientation orientation;
>  	int ret;
>  
> @@ -543,6 +545,18 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
>  		ret = -ENOTSUPP;
>  	}
>  
> +	if (!typec->needs_mux_ack)
> +		return ret;
> +
> +	/* Sending Acknowledgment to EC */
> +	mux_ack.port = port_num;
> +
> +	if (cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_MUX_ACK, &mux_ack,
> +				  sizeof(mux_ack), NULL, 0) < 0)
> +		dev_warn(typec->dev,
> +			 "Failed to send Mux ACK to EC for port: %d\n",
> +			 port_num);
> +
>  	return ret;
>  }
>  
> @@ -908,6 +922,8 @@ static int cros_typec_probe(struct platform_device *pdev)
>  
>  	typec->typec_cmd_supported = !!cros_typec_feature_supported(typec,
>  					EC_FEATURE_TYPEC_CMD);
> +	typec->needs_mux_ack = !!cros_typec_feature_supported(typec,
> +					EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
>  
>  	ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
>  				    &resp, sizeof(resp));
> diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> index 7f54fdcdd8cb..3b53e45cb5a0 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -1286,6 +1286,16 @@ enum ec_feature_code {
>  	EC_FEATURE_ISH = 40,
>  	/* New TCPMv2 TYPEC_ prefaced commands supported */
>  	EC_FEATURE_TYPEC_CMD = 41,
> +	/*
> +	 * The EC will wait for direction from the AP to enter Type-C alternate
> +	 * modes or USB4.
> +	 */
> +	EC_FEATURE_TYPEC_REQUIRE_AP_MODE_ENTRY = 42,
> +	/*
> +	 * The EC will wait for an acknowledge from the AP after setting the
> +	 * mux.
> +	 */
> +	EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK = 43,
>  };
>  
>  #define EC_FEATURE_MASK_0(event_code) BIT(event_code % 32)
> @@ -6054,6 +6064,13 @@ struct ec_params_charger_control {
>  	uint8_t allow_charging;
>  } __ec_align_size1;
>  
> +/* Get ACK from the USB-C SS muxes */
> +#define EC_CMD_USB_PD_MUX_ACK 0x0603
> +
> +struct ec_params_usb_pd_mux_ack {
> +	uint8_t port; /* USB-C port number */
> +} __ec_align1;
> +
>  /*****************************************************************************/
>  /*
>   * Reserve a range of host commands for board-specific, experimental, or
> -- 
> 2.25.1
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux configuration acknowledgment to EC

[Patel, Utkarsh H]

Hi Benson,

Thank you for applying the patches.

> -----Original Message-----
> From: Benson Leung <bleung@google.com>
> Sent: Tuesday, January 05, 2021 2:51 PM
> To: Patel, Utkarsh H <utkarsh.h.patel@intel.com>
> Cc: linux-kernel@vger.kernel.org; enric.balletbo@collabora.com;
> pmalani@chromium.org; bleung@chromium.org;
> heikki.krogerus@linux.intel.com; Mani, Rajmohan
> <rajmohan.mani@intel.com>
> Subject: Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Send mux
> configuration acknowledgment to EC
> 
> Hi Utkarsh,
> 
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 650aa5332055..e6abe205890c 100644
> 
> I went ahead and staged this commit, but just for your information, applying
> this patch was problematic because the sha1 used here didn't match anything
> in the upstream kernel.
> 
> Here's the error when I try to apply this using git am:
> Applying: platform/chrome: cros_ec_typec: Send mux configuration
> acknowledgment to EC
> error: sha1 information is lacking or useless
> (drivers/platform/chrome/cros_ec_typec.c).
> error: could not build fake ancestor
> Patch failed at 0001 platform/chrome: cros_ec_typec: Send mux configuration
> acknowledgment to EC
> 
> 
> Please double check when you run git-format-patch that the branch you are
> working on comes from publicly available git repos, ideally, based on linus's
> tagged releases. This will ensure things apply cleanly on my side.

Sorry about this. I think I might have used for-next branch mistakenly when running git-format-patch.
I will make sure to use correct one next time. 

> 
> I've gone ahead and staged this, but could you please sanity check that the
> result is as intended?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/chrome-
> platform/linux.git/commit/?h=cros-ec-typec-for-
> 5.12&id=8553a979fcd03448a4096c7d431b7ee1a52bfca3

I have checked it on myside and works as intended.

> 
> Thank you,
> Benson
> 

Sincerely,
Utkarsh Patel.