Hi

-----Original Message-----
From: Adam Thomson [mailto:Adam.Thomson.Opensource@diasemi.com]
Sent: 2018年6月25日 18:46
To: Jun Li <redacted>; robh+dt@kernel.org; gregkh@linuxfoundation.org;
heikki.krogerus@linux.intel.com; linux@roeck-us.net
Cc: a.hajda@samsung.com; cw00.choi@samsung.com; shufan_lee@richtek.com;
Peter Chen [off-list ref]; devicetree@vger.kernel.org;
linux-usb@vger.kernel.org; dl-linux-imx [off-list ref]
Subject: RE: [PATCH v7 06/14] usb: typec: tcpm: support get typec and pd config
from device properties

On 25 June 2018 09:09, Li Jun wrote:

quoted

This patch adds support of get typec and power delivery config from
firmware description.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Li Jun <redacted>
---
 drivers/usb/typec/tcpm.c | 132
+++++++++++++++++++++++++++++++++++++++-----
---
 1 file changed, 110 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index

d22b37b..0454654 100644

--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c

@@ -4236,6 +4236,81 @@ static int tcpm_copy_vdos(u32 *dest_vdo, const

u32 *src_vdo,
 	return nr_vdo;
 }

...

quoted

 struct tcpm_port *tcpm_register_port(struct device *dev, struct
tcpc_dev *tcpc)  {
 	struct tcpm_port *port;
 	int i, err;

-	if (!dev || !tcpc || !tcpc->config ||
+	if (!dev || !tcpc ||
 	    !tcpc->get_vbus || !tcpc->set_cc || !tcpc->get_cc ||
 	    !tcpc->set_polarity || !tcpc->set_vconn || !tcpc->set_vbus ||
 	    !tcpc->set_pd_rx || !tcpc->set_roles || !tcpc->pd_transmit) @@
-4556,30 +4655,19 @@ struct tcpm_port *tcpm_register_port(struct
device *dev, struct tcpc_dev *tcpc)
 	init_completion(&port->pps_complete);
 	tcpm_debugfs_init(port);

-	if (tcpm_validate_caps(port, tcpc->config->src_pdo,
-			       tcpc->config->nr_src_pdo) ||
-	    tcpm_validate_caps(port, tcpc->config->snk_pdo,
-			       tcpc->config->nr_snk_pdo)) {
-		err = -EINVAL;
+	if (tcpc->config)
+		err = tcpm_copy_caps(port, tcpc->config);
+	else
+		err = tcpm_fw_get_caps(port, tcpc->fwnode);
+	if (err < 0)

Should we not be favouring the generic FW based capabilities over values
defined in a Type C port controller driver, if the FW config is present? For
example
fusb302 driver has a hard coded config so that would always be selected rather
than the FW based config, unless I'm missing something obvious?

You are right, I will change it to be below:
err = tcpm_fw_get_caps(port, tcpc->fwnode);
if ((err < 0) && tcpc->config)
	err = tcpm_copy_caps(port, tcpc->config);

thanks
Jun