On Sun, Feb 13, 2022 at 11:01:16PM -0600, Samuel Holland wrote:
> Basic programmable non-PD Type-C port controllers do not need the full
> TCPM library, but they share the same devicetree binding and the same
> typec_capability structure. Factor out a helper for parsing those
> properties which map to fields in struct typec_capability, so the code
> can be shared between TCPM and basic non-TCPM drivers.
>
> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
>
> Changes in v3:
> - Leave the call to fw_devlink_purge_absent_suppliers in its original
> place in the TCPM code. It is not needed by the new driver.
> - Remove unused variable from TCPM function.
>
> Changes in v2:
> - Always put the return values from typec_find_* in a signed variable
> for error checking.
>
> drivers/usb/typec/class.c | 43 +++++++++++++++++++++++++++++++++++
> drivers/usb/typec/tcpm/tcpm.c | 24 +------------------
> include/linux/usb/typec.h | 3 +++
> 3 files changed, 47 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 45a6f0c807cb..ee0e520707dd 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1894,6 +1894,49 @@ void *typec_get_drvdata(struct typec_port *port)
> }
> EXPORT_SYMBOL_GPL(typec_get_drvdata);
>
> +int typec_get_fw_cap(struct typec_capability *cap,
> + struct fwnode_handle *fwnode)
> +{
> + const char *cap_str;
> + int ret;
> +
> + cap->fwnode = fwnode;
> +
> + ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
> + if (ret < 0)
> + return ret;
> +
> + ret = typec_find_port_power_role(cap_str);
> + if (ret < 0)
> + return ret;
> + cap->type = ret;
> +
> + /* USB data support is optional */
> + ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
> + if (ret == 0) {
> + ret = typec_find_port_data_role(cap_str);
> + if (ret < 0)
> + return ret;
> + cap->data = ret;
> + }
> +
> + /* Get the preferred power role for a DRP */
> + if (cap->type == TYPEC_PORT_DRP) {
> + cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
> +
> + ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
> + if (ret == 0) {
> + ret = typec_find_power_role(cap_str);
> + if (ret < 0)
> + return ret;
> + cap->prefer_role = ret;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(typec_get_fw_cap);
> +
> /**
> * typec_port_register_altmode - Register USB Type-C Port Alternate Mode
> * @port: USB Type-C Port that supports the alternate mode
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 5fce795b69c7..3bc2f4ebd1fe 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -5928,7 +5928,6 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> struct fwnode_handle *fwnode)
> {
> const char *opmode_str;
> - const char *cap_str;
> int ret;
> u32 mw, frs_current;
>
> @@ -5944,23 +5943,10 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> */
> fw_devlink_purge_absent_suppliers(fwnode);
>
> - /* USB data support is optional */
> - ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
> - if (ret == 0) {
> - ret = typec_find_port_data_role(cap_str);
> - if (ret < 0)
> - return ret;
> - port->typec_caps.data = ret;
> - }
> -
> - ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
> + ret = typec_get_fw_cap(&port->typec_caps, fwnode);
> if (ret < 0)
> return ret;
>
> - ret = typec_find_port_power_role(cap_str);
> - if (ret < 0)
> - return ret;
> - port->typec_caps.type = ret;
> port->port_type = port->typec_caps.type;
> port->pd_supported = !fwnode_property_read_bool(fwnode, "pd-disable");
>
> @@ -5997,14 +5983,6 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> if (port->port_type == TYPEC_PORT_SRC)
> return 0;
>
> - /* Get the preferred power role for DRP */
> - ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
> - if (ret < 0)
> - return ret;
> -
> - port->typec_caps.prefer_role = typec_find_power_role(cap_str);
> - if (port->typec_caps.prefer_role < 0)
> - return -EINVAL;
> sink:
> port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
>
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 7ba45a97eeae..fdf737d48b3b 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -295,6 +295,9 @@ int typec_set_mode(struct typec_port *port, int mode);
>
> void *typec_get_drvdata(struct typec_port *port);
>
> +int typec_get_fw_cap(struct typec_capability *cap,
> + struct fwnode_handle *fwnode);
> +
> int typec_find_pwr_opmode(const char *name);
> int typec_find_orientation(const char *name);
> int typec_find_port_power_role(const char *name);
> --
> 2.33.1
thanks,
--
heikki
