| Message ID | 20220201032440.5196-3-samuel@sholland.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/4] dt-bindings: vendor-prefixes: Add willsemi | expand |
Hi Samuel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on robh/for-next v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Samuel-Holland/dt-bindings-vendor-prefixes-Add-willsemi/20220201-112541
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: openrisc-randconfig-m031-20220201 (https://download.01.org/0day-ci/archive/20220202/202202021458.xcH4F4SQ-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/usb/typec/class.c:1919 typec_get_fw_cap() warn: unsigned 'cap->type' is never less than zero.
drivers/usb/typec/class.c:1926 typec_get_fw_cap() warn: unsigned 'cap->data' is never less than zero.
vim +1919 drivers/usb/typec/class.c
1896
1897 int typec_get_fw_cap(struct typec_capability *cap,
1898 struct fwnode_handle *fwnode)
1899 {
1900 const char *cap_str;
1901 int ret;
1902
1903 /*
1904 * This fwnode has a "compatible" property, but is never populated as a
1905 * struct device. Instead we simply parse it to read the properties.
1906 * This it breaks fw_devlink=on. To maintain backward compatibility
1907 * with existing DT files, we work around this by deleting any
1908 * fwnode_links to/from this fwnode.
1909 */
1910 fw_devlink_purge_absent_suppliers(fwnode);
1911
1912 cap->fwnode = fwnode;
1913
1914 ret = fwnode_property_read_string(fwnode, "power-role", &cap_str);
1915 if (ret < 0)
1916 return ret;
1917
1918 cap->type = typec_find_port_power_role(cap_str);
> 1919 if (cap->type < 0)
1920 return cap->type;
1921
1922 /* USB data support is optional */
1923 ret = fwnode_property_read_string(fwnode, "data-role", &cap_str);
1924 if (ret == 0) {
1925 cap->data = typec_find_port_data_role(cap_str);
> 1926 if (cap->data < 0)
1927 return cap->data;
1928 }
1929
1930 /* Get the preferred power role for a DRP */
1931 if (cap->type == TYPEC_PORT_DRP) {
1932 cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
1933
1934 ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
1935 if (ret == 0) {
1936 cap->prefer_role = typec_find_power_role(cap_str);
1937 if (cap->prefer_role < 0)
1938 return cap->prefer_role;
1939 }
1940 }
1941
1942 return 0;
1943 }
1944 EXPORT_SYMBOL_GPL(typec_get_fw_cap);
1945
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, Feb 02, 2022 at 02:22:27PM +0800, kernel test robot wrote: > Hi Samuel, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on usb/usb-testing] > [also build test WARNING on robh/for-next v5.17-rc2 next-20220202] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Samuel-Holland/dt-bindings-vendor-prefixes-Add-willsemi/20220201-112541 > base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing > config: openrisc-randconfig-m031-20220201 (https://download.01.org/0day-ci/archive/20220202/202202021458.xcH4F4SQ-lkp@intel.com/config) > compiler: or1k-linux-gcc (GCC) 11.2.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > smatch warnings: > drivers/usb/typec/class.c:1919 typec_get_fw_cap() warn: unsigned 'cap->type' is never less than zero. > drivers/usb/typec/class.c:1926 typec_get_fw_cap() warn: unsigned 'cap->data' is never less than zero. > Ah yes, there was a reason to assign the return values to 'ret' first and only afterwards to the actual capabilities. Please fix and resubmit. Guenter
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index 45a6f0c807cb..8863d2e93d50 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -1894,6 +1894,55 @@ 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; + + /* + * This fwnode has a "compatible" property, but is never populated as a + * struct device. Instead we simply parse it to read the properties. + * This it breaks fw_devlink=on. To maintain backward compatibility + * with existing DT files, we work around this by deleting any + * fwnode_links to/from this fwnode. + */ + fw_devlink_purge_absent_suppliers(fwnode); + + cap->fwnode = fwnode; + + ret = fwnode_property_read_string(fwnode, "power-role", &cap_str); + if (ret < 0) + return ret; + + cap->type = typec_find_port_power_role(cap_str); + if (cap->type < 0) + return cap->type; + + /* USB data support is optional */ + ret = fwnode_property_read_string(fwnode, "data-role", &cap_str); + if (ret == 0) { + cap->data = typec_find_port_data_role(cap_str); + if (cap->data < 0) + return cap->data; + } + + /* 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) { + cap->prefer_role = typec_find_power_role(cap_str); + if (cap->prefer_role < 0) + return cap->prefer_role; + } + } + + 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..8b58aa6e3509 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -5935,32 +5935,10 @@ static int tcpm_fw_get_caps(struct tcpm_port *port, if (!fwnode) return -EINVAL; - /* - * This fwnode has a "compatible" property, but is never populated as a - * struct device. Instead we simply parse it to read the properties. - * This it breaks fw_devlink=on. To maintain backward compatibility - * with existing DT files, we work around this by deleting any - * fwnode_links to/from this fwnode. - */ - 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 +5975,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);
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@sholland.org> --- drivers/usb/typec/class.c | 49 +++++++++++++++++++++++++++++++++++ drivers/usb/typec/tcpm/tcpm.c | 32 +---------------------- include/linux/usb/typec.h | 3 +++ 3 files changed, 53 insertions(+), 31 deletions(-)