On Tue, Jan 12, 2021 at 9:56 PM Heikki Krogerus
[off-list ref] wrote:

On Wed, Jan 06, 2021 at 12:39:26AM +0800, Kyle Tso wrote:

quoted

PD3.0 Spec 6.8.1 describes how to handle Protocol Error. There are
general rules defined in Table 6-61 which regulate incoming Message
handling. If the incoming Message is unexpected, unsupported, or
unrecognized, Protocol Error occurs. Follow the rules to handle these
situations. Also consider PD2.0 connection (PD2.0 Spec Table 6-36) for
backward compatibilities.

To know the types of AMS in all the recipient's states, identify those
AMS who are initiated by the port partner but not yet recorded in the
current code.

Besides, introduce a new state CHUNK_NOT_SUPP to delay the NOT_SUPPORTED
message after receiving a chunked message.

Looks good to me. I put a few style related nitpicks below, but
nothing major.

quoted

+             if (port->pwr_role == TYPEC_SOURCE) {
+                     if (port->ams == GET_SOURCE_CAPABILITIES)
+                             tcpm_pd_handle_state(port, SRC_READY, NONE_AMS,
+                                                  0);
+                     /* Unexpected Source Capabilities */
+                     else
+                             tcpm_pd_handle_msg(port,
+                                        port->negotiated_rev < PD_REV30 ?
+                                        PD_MSG_CTRL_REJECT :
+                                        PD_MSG_CTRL_NOT_SUPP,
+                                        NONE_AMS);

You can align that properly:

                                tcpm_pd_handle_msg(port,
                                                   port->negotiated_rev < PD_REV30 ?
                                                   PD_MSG_CTRL_REJECT :
                                                   PD_MSG_CTRL_NOT_SUPP,
                                                   NONE_AMS);

Yes it looks better. will fix it.

quoted

              case PD_MSG_DATA_SINK_CAP:
-                     tcpm_pd_send_sink_caps(port);
+                     ret = tcpm_pd_send_sink_caps(port);
+                     if (ret < 0) {
+                             tcpm_log(port,
+                                      "Unable to send snk caps, ret=%d",
+                                      ret);

One line is enough:

                                tcpm_log(port, "Unable to send snk caps, ret=%d", ret);

will fix it in the next version.

thanks,
Kyle