* [PATCH 0/2] Add a interface for passing supported PD rev from TCPC @ 2022-02-08 8:20 Potin Lai 2022-02-08 8:20 ` [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Potin Lai @ 2022-02-08 8:20 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams, Potin Lai This series add an interface for TCPC passing supported PD revision. supported_pd_rev is optional, TCPM will get PD rev for negotiation if lower level driver has implementaion, otherwise use PD_MAX_REV for negotiation Potin Lai (2): usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev usb: typec: fusb302: add support of supported_pd_rev drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++-- include/linux/usb/tcpm.h | 4 ++++ 3 files changed, 36 insertions(+), 2 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev 2022-02-08 8:20 [PATCH 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai @ 2022-02-08 8:20 ` Potin Lai 2022-02-08 8:41 ` Greg Kroah-Hartman 2022-02-08 8:20 ` [PATCH 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai 2022-02-08 11:22 ` [PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2 siblings, 1 reply; 14+ messages in thread From: Potin Lai @ 2022-02-08 8:20 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams, Potin Lai Current TCPM allways assume using PD_MAX_REV for negotiation, but for some USB controller only support PD 2.0, adding an interface for passing supported_pd_rev from tcpc_dev. Signed-off-by: Potin Lai <potin.lai@quantatw.com> --- drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++-- include/linux/usb/tcpm.h | 4 ++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 59d4fa2443f2..31770fa8643d 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port) port->cc2 == TYPEC_CC_OPEN))); } +static u32 tcpm_pd_supported_rev(struct tcpm_port *port) +{ + u32 rev = PD_MAX_REV; + + if (port->tcpc->supported_pd_rev) + rev = port->tcpc->supported_pd_rev(port->tcpc); + + return (rev > PD_MAX_REV) ? PD_MAX_REV : rev; +} + /* * Logging */ @@ -3932,7 +3942,7 @@ static void run_state_machine(struct tcpm_port *port) typec_set_pwr_opmode(port->typec_port, opmode); port->pwr_opmode = TYPEC_PWR_MODE_USB; port->caps_count = 0; - port->negotiated_rev = PD_MAX_REV; + port->negotiated_rev = tcpm_pd_supported_rev(port); port->message_id = 0; port->rx_msgid = -1; port->explicit_contract = false; @@ -4167,7 +4177,7 @@ static void run_state_machine(struct tcpm_port *port) port->cc2 : port->cc1); typec_set_pwr_opmode(port->typec_port, opmode); port->pwr_opmode = TYPEC_PWR_MODE_USB; - port->negotiated_rev = PD_MAX_REV; + port->negotiated_rev = tcpm_pd_supported_rev(port); port->message_id = 0; port->rx_msgid = -1; port->explicit_contract = false; diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h index bffc8d3e14ad..36282b2a9d9c 100644 --- a/include/linux/usb/tcpm.h +++ b/include/linux/usb/tcpm.h @@ -114,6 +114,9 @@ enum tcpm_transmit_type { * Optional; The USB Communications Capable bit indicates if port * partner is capable of communication over the USB data lines * (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit. + * @supported_pd_rev: + * Optional; TCPM call this function to get supported PD revesion + * from lower level driver. */ struct tcpc_dev { struct fwnode_handle *fwnode; @@ -148,6 +151,7 @@ struct tcpc_dev { bool pps_active, u32 requested_vbus_voltage); bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev); void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable); + u32 (*supported_pd_rev)(struct tcpc_dev *dev); }; struct tcpm_port; -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev 2022-02-08 8:20 ` [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai @ 2022-02-08 8:41 ` Greg Kroah-Hartman 0 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2022-02-08 8:41 UTC (permalink / raw) To: Potin Lai Cc: Guenter Roeck, Heikki Krogerus, linux-usb, linux-kernel, Patrick Williams On Tue, Feb 08, 2022 at 04:20:25PM +0800, Potin Lai wrote: > Current TCPM allways assume using PD_MAX_REV for negotiation, > but for some USB controller only support PD 2.0, adding an interface > for passing supported_pd_rev from tcpc_dev. > > Signed-off-by: Potin Lai <potin.lai@quantatw.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++-- > include/linux/usb/tcpm.h | 4 ++++ > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 59d4fa2443f2..31770fa8643d 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port) > port->cc2 == TYPEC_CC_OPEN))); > } > > +static u32 tcpm_pd_supported_rev(struct tcpm_port *port) > +{ > + u32 rev = PD_MAX_REV; > + > + if (port->tcpc->supported_pd_rev) > + rev = port->tcpc->supported_pd_rev(port->tcpc); > + > + return (rev > PD_MAX_REV) ? PD_MAX_REV : rev; Please spell this out in a real if statement to make it obvious: if (rev > PD_MAX_REV) return PD_MAX_REV return rev Or better yet: return min(PD_MAX_REV, rev); right? thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] usb: typec: fusb302: add support of supported_pd_rev 2022-02-08 8:20 [PATCH 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2022-02-08 8:20 ` [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai @ 2022-02-08 8:20 ` Potin Lai 2022-02-08 11:22 ` [PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2 siblings, 0 replies; 14+ messages in thread From: Potin Lai @ 2022-02-08 8:20 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams, Potin Lai Add support for passing supported PD rev to TCPM. If "supported-pd-rev" property exist, then return supported_pd_rev as defined value in DTS, otherwise return PD_MAX_REV Example of DTS: fusb302: typec-portc@22 { compatible = "fcs,fusb302"; reg = <0x22>; ... supported-pd-rev=<1>; // PD_REV20 ... }; Signed-off-by: Potin Lai <potin.lai@quantatw.com> --- drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c index 72f9001b0792..8cff92d58b96 100644 --- a/drivers/usb/typec/tcpm/fusb302.c +++ b/drivers/usb/typec/tcpm/fusb302.c @@ -109,6 +109,9 @@ struct fusb302_chip { enum typec_cc_status cc2; u32 snk_pdo[PDO_MAX_OBJECTS]; + /* supported pd rev */ + u32 supported_pd_rev; + #ifdef CONFIG_DEBUG_FS struct dentry *dentry; /* lock for log buffer access */ @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type, return ret; } +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev) +{ + struct fusb302_chip *chip = container_of(dev, struct fusb302_chip, + tcpc_dev); + return chip->supported_pd_rev; +} + static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl) { if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX) @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev) fusb302_tcpc_dev->set_roles = tcpm_set_roles; fusb302_tcpc_dev->start_toggling = tcpm_start_toggling; fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit; + fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev; } static const char * const cc_polarity_name[] = { @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client, struct fusb302_chip *chip; struct i2c_adapter *adapter = client->adapter; struct device *dev = &client->dev; + struct device_node *np = dev->of_node; const char *name; int ret = 0; @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client, dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); goto tcpm_unregister_port; } + + if (of_property_read_u32(np, "supported-pd-rev", + &chip->supported_pd_rev) < 0) { + chip->supported_pd_rev = PD_MAX_REV; + } else if (chip->supported_pd_rev > PD_MAX_REV) { + chip->supported_pd_rev = PD_MAX_REV; + } + enable_irq_wake(chip->gpio_int_n_irq); i2c_set_clientdata(client, chip); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC 2022-02-08 8:20 [PATCH 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2022-02-08 8:20 ` [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai 2022-02-08 8:20 ` [PATCH 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai @ 2022-02-08 11:22 ` Potin Lai 2022-02-08 11:22 ` [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai 2022-02-08 11:22 ` [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai 2 siblings, 2 replies; 14+ messages in thread From: Potin Lai @ 2022-02-08 11:22 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams, Potin Lai This series add an interface for TCPC passing supported PD revision. supported_pd_rev is optional, TCPM will get PD rev for negotiation if lower level driver has implementaion, otherwise use PD_MAX_REV for negotiation changes v1 -> v2: - tcpm.c * use min() to simplify the statement Potin Lai (2): usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev usb: typec: fusb302: add support of supported_pd_rev drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++-- include/linux/usb/tcpm.h | 4 ++++ 3 files changed, 36 insertions(+), 2 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev 2022-02-08 11:22 ` [PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai @ 2022-02-08 11:22 ` Potin Lai 2022-02-08 15:25 ` Guenter Roeck ` (2 more replies) 2022-02-08 11:22 ` [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai 1 sibling, 3 replies; 14+ messages in thread From: Potin Lai @ 2022-02-08 11:22 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams, Potin Lai Current TCPM allways assume using PD_MAX_REV for negotiation, but for some USB controller only support PD 2.0, adding an interface for passing supported_pd_rev from tcpc_dev. Signed-off-by: Potin Lai <potin.lai@quantatw.com> --- drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++-- include/linux/usb/tcpm.h | 4 ++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 59d4fa2443f2..22e7d226826e 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port) port->cc2 == TYPEC_CC_OPEN))); } +static u32 tcpm_pd_supported_rev(struct tcpm_port *port) +{ + u32 rev = PD_MAX_REV; + + if (port->tcpc->supported_pd_rev) + rev = port->tcpc->supported_pd_rev(port->tcpc); + + return min(rev, PD_MAX_REV); +} + /* * Logging */ @@ -3932,7 +3942,7 @@ static void run_state_machine(struct tcpm_port *port) typec_set_pwr_opmode(port->typec_port, opmode); port->pwr_opmode = TYPEC_PWR_MODE_USB; port->caps_count = 0; - port->negotiated_rev = PD_MAX_REV; + port->negotiated_rev = tcpm_pd_supported_rev(port); port->message_id = 0; port->rx_msgid = -1; port->explicit_contract = false; @@ -4167,7 +4177,7 @@ static void run_state_machine(struct tcpm_port *port) port->cc2 : port->cc1); typec_set_pwr_opmode(port->typec_port, opmode); port->pwr_opmode = TYPEC_PWR_MODE_USB; - port->negotiated_rev = PD_MAX_REV; + port->negotiated_rev = tcpm_pd_supported_rev(port); port->message_id = 0; port->rx_msgid = -1; port->explicit_contract = false; diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h index bffc8d3e14ad..36282b2a9d9c 100644 --- a/include/linux/usb/tcpm.h +++ b/include/linux/usb/tcpm.h @@ -114,6 +114,9 @@ enum tcpm_transmit_type { * Optional; The USB Communications Capable bit indicates if port * partner is capable of communication over the USB data lines * (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit. + * @supported_pd_rev: + * Optional; TCPM call this function to get supported PD revesion + * from lower level driver. */ struct tcpc_dev { struct fwnode_handle *fwnode; @@ -148,6 +151,7 @@ struct tcpc_dev { bool pps_active, u32 requested_vbus_voltage); bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev); void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable); + u32 (*supported_pd_rev)(struct tcpc_dev *dev); }; struct tcpm_port; -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev 2022-02-08 11:22 ` [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai @ 2022-02-08 15:25 ` Guenter Roeck 2022-02-08 16:41 ` kernel test robot 2022-02-09 6:53 ` kernel test robot 2 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2022-02-08 15:25 UTC (permalink / raw) To: Potin Lai, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams On 2/8/22 03:22, Potin Lai wrote: > Current TCPM allways assume using PD_MAX_REV for negotiation, > but for some USB controller only support PD 2.0, adding an interface > for passing supported_pd_rev from tcpc_dev. > The PD revision supported by the usb controller is a constant. I don't see why this would need a callback function. Other capabilitied are passed to tcpm using the fwnode pointer. I don't see why this capability would have to be handled differently. Guenter > Signed-off-by: Potin Lai <potin.lai@quantatw.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++-- > include/linux/usb/tcpm.h | 4 ++++ > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 59d4fa2443f2..22e7d226826e 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -571,6 +571,16 @@ static bool tcpm_port_is_disconnected(struct tcpm_port *port) > port->cc2 == TYPEC_CC_OPEN))); > } > > +static u32 tcpm_pd_supported_rev(struct tcpm_port *port) > +{ > + u32 rev = PD_MAX_REV; > + > + if (port->tcpc->supported_pd_rev) > + rev = port->tcpc->supported_pd_rev(port->tcpc); > + > + return min(rev, PD_MAX_REV); > +} > + > /* > * Logging > */ > @@ -3932,7 +3942,7 @@ static void run_state_machine(struct tcpm_port *port) > typec_set_pwr_opmode(port->typec_port, opmode); > port->pwr_opmode = TYPEC_PWR_MODE_USB; > port->caps_count = 0; > - port->negotiated_rev = PD_MAX_REV; > + port->negotiated_rev = tcpm_pd_supported_rev(port); > port->message_id = 0; > port->rx_msgid = -1; > port->explicit_contract = false; > @@ -4167,7 +4177,7 @@ static void run_state_machine(struct tcpm_port *port) > port->cc2 : port->cc1); > typec_set_pwr_opmode(port->typec_port, opmode); > port->pwr_opmode = TYPEC_PWR_MODE_USB; > - port->negotiated_rev = PD_MAX_REV; > + port->negotiated_rev = tcpm_pd_supported_rev(port); > port->message_id = 0; > port->rx_msgid = -1; > port->explicit_contract = false; > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h > index bffc8d3e14ad..36282b2a9d9c 100644 > --- a/include/linux/usb/tcpm.h > +++ b/include/linux/usb/tcpm.h > @@ -114,6 +114,9 @@ enum tcpm_transmit_type { > * Optional; The USB Communications Capable bit indicates if port > * partner is capable of communication over the USB data lines > * (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit. > + * @supported_pd_rev: > + * Optional; TCPM call this function to get supported PD revesion > + * from lower level driver. > */ > struct tcpc_dev { > struct fwnode_handle *fwnode; > @@ -148,6 +151,7 @@ struct tcpc_dev { > bool pps_active, u32 requested_vbus_voltage); > bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev); > void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable); > + u32 (*supported_pd_rev)(struct tcpc_dev *dev); > }; > > struct tcpm_port; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev 2022-02-08 11:22 ` [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai 2022-02-08 15:25 ` Guenter Roeck @ 2022-02-08 16:41 ` kernel test robot 2022-02-09 6:53 ` kernel test robot 2 siblings, 0 replies; 14+ messages in thread From: kernel test robot @ 2022-02-08 16:41 UTC (permalink / raw) To: Potin Lai, Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: llvm, kbuild-all, linux-usb, linux-kernel, Patrick Williams, Potin Lai Hi Potin, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on v5.17-rc3 next-20220208] [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/Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: hexagon-randconfig-r034-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090006.YLbevIuT-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project e8bff9ae54a55b4dbfeb6ba55f723abbd81bf494) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/322696594704fa918e63d1c80fa6d346a02e9a28 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246 git checkout 322696594704fa918e63d1c80fa6d346a02e9a28 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/usb/typec/tcpm/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/usb/typec/tcpm/tcpm.c:581:9: warning: comparison of distinct pointer types ('typeof (rev) *' (aka 'unsigned int *') and 'typeof (2) *' (aka 'int *')) [-Wcompare-distinct-pointer-types] return min(rev, PD_MAX_REV); ^~~~~~~~~~~~~~~~~~~~ include/linux/minmax.h:45:19: note: expanded from macro 'min' #define min(x, y) __careful_cmp(x, y, <) ^~~~~~~~~~~~~~~~~~~~~~ include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp' __builtin_choose_expr(__safe_cmp(x, y), \ ^~~~~~~~~~~~~~~~ include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp' (__typecheck(x, y) && __no_side_effects(x, y)) ^~~~~~~~~~~~~~~~~ include/linux/minmax.h:20:28: note: expanded from macro '__typecheck' (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~ 1 warning generated. vim +581 drivers/usb/typec/tcpm/tcpm.c 573 574 static u32 tcpm_pd_supported_rev(struct tcpm_port *port) 575 { 576 u32 rev = PD_MAX_REV; 577 578 if (port->tcpc->supported_pd_rev) 579 rev = port->tcpc->supported_pd_rev(port->tcpc); 580 > 581 return min(rev, PD_MAX_REV); 582 } 583 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev 2022-02-08 11:22 ` [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai 2022-02-08 15:25 ` Guenter Roeck 2022-02-08 16:41 ` kernel test robot @ 2022-02-09 6:53 ` kernel test robot 2 siblings, 0 replies; 14+ messages in thread From: kernel test robot @ 2022-02-09 6:53 UTC (permalink / raw) To: Potin Lai, Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: kbuild-all, linux-usb, linux-kernel, Patrick Williams, Potin Lai Hi Potin, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on v5.17-rc3 next-20220208] [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/Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: arm64-randconfig-s031-20220208 (https://download.01.org/0day-ci/archive/20220209/202202091453.L5BVOjcx-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 11.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/322696594704fa918e63d1c80fa6d346a02e9a28 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Potin-Lai/usb-typec-tcpm-add-interface-for-passing-supported_pd_rev-from-tcpc_dev/20220208-202246 git checkout 322696594704fa918e63d1c80fa6d346a02e9a28 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/usb/typec/tcpm/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/usb/typec/tcpm/tcpm.c:581:16: sparse: sparse: incompatible types in comparison expression (different signedness): >> drivers/usb/typec/tcpm/tcpm.c:581:16: sparse: unsigned int * >> drivers/usb/typec/tcpm/tcpm.c:581:16: sparse: int * vim +581 drivers/usb/typec/tcpm/tcpm.c 573 574 static u32 tcpm_pd_supported_rev(struct tcpm_port *port) 575 { 576 u32 rev = PD_MAX_REV; 577 578 if (port->tcpc->supported_pd_rev) 579 rev = port->tcpc->supported_pd_rev(port->tcpc); 580 > 581 return min(rev, PD_MAX_REV); 582 } 583 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev 2022-02-08 11:22 ` [PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2022-02-08 11:22 ` [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai @ 2022-02-08 11:22 ` Potin Lai 2022-02-08 15:22 ` Guenter Roeck 1 sibling, 1 reply; 14+ messages in thread From: Potin Lai @ 2022-02-08 11:22 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams, Potin Lai Add support for passing supported PD rev to TCPM. If "supported-pd-rev" property exist, then return supported_pd_rev as defined value in DTS, otherwise return PD_MAX_REV Example of DTS: fusb302: typec-portc@22 { compatible = "fcs,fusb302"; reg = <0x22>; ... supported-pd-rev=<1>; // PD_REV20 ... }; Signed-off-by: Potin Lai <potin.lai@quantatw.com> --- drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c index 72f9001b0792..8cff92d58b96 100644 --- a/drivers/usb/typec/tcpm/fusb302.c +++ b/drivers/usb/typec/tcpm/fusb302.c @@ -109,6 +109,9 @@ struct fusb302_chip { enum typec_cc_status cc2; u32 snk_pdo[PDO_MAX_OBJECTS]; + /* supported pd rev */ + u32 supported_pd_rev; + #ifdef CONFIG_DEBUG_FS struct dentry *dentry; /* lock for log buffer access */ @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type, return ret; } +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev) +{ + struct fusb302_chip *chip = container_of(dev, struct fusb302_chip, + tcpc_dev); + return chip->supported_pd_rev; +} + static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl) { if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX) @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev) fusb302_tcpc_dev->set_roles = tcpm_set_roles; fusb302_tcpc_dev->start_toggling = tcpm_start_toggling; fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit; + fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev; } static const char * const cc_polarity_name[] = { @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client, struct fusb302_chip *chip; struct i2c_adapter *adapter = client->adapter; struct device *dev = &client->dev; + struct device_node *np = dev->of_node; const char *name; int ret = 0; @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client, dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); goto tcpm_unregister_port; } + + if (of_property_read_u32(np, "supported-pd-rev", + &chip->supported_pd_rev) < 0) { + chip->supported_pd_rev = PD_MAX_REV; + } else if (chip->supported_pd_rev > PD_MAX_REV) { + chip->supported_pd_rev = PD_MAX_REV; + } + enable_irq_wake(chip->gpio_int_n_irq); i2c_set_clientdata(client, chip); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev 2022-02-08 11:22 ` [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai @ 2022-02-08 15:22 ` Guenter Roeck 2022-02-09 3:34 ` Potin Lai 0 siblings, 1 reply; 14+ messages in thread From: Guenter Roeck @ 2022-02-08 15:22 UTC (permalink / raw) To: Potin Lai, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams On 2/8/22 03:22, Potin Lai wrote: > Add support for passing supported PD rev to TCPM. > If "supported-pd-rev" property exist, then return supported_pd_rev as > defined value in DTS, otherwise return PD_MAX_REV > > Example of DTS: > > fusb302: typec-portc@22 { > compatible = "fcs,fusb302"; > reg = <0x22>; > ... > supported-pd-rev=<1>; // PD_REV20 > ... > }; > The new property needs to be documented. However, I am not entirely sure I understand why it is needed. Wouldn't the supported PD revision be a chip (fusb302) specific constant ? If so, why does it need a devicetree property ? I think that needs some additional explanation. Thanks, Guenter > Signed-off-by: Potin Lai <potin.lai@quantatw.com> > --- > drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c > index 72f9001b0792..8cff92d58b96 100644 > --- a/drivers/usb/typec/tcpm/fusb302.c > +++ b/drivers/usb/typec/tcpm/fusb302.c > @@ -109,6 +109,9 @@ struct fusb302_chip { > enum typec_cc_status cc2; > u32 snk_pdo[PDO_MAX_OBJECTS]; > > + /* supported pd rev */ > + u32 supported_pd_rev; > + > #ifdef CONFIG_DEBUG_FS > struct dentry *dentry; > /* lock for log buffer access */ > @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type, > return ret; > } > > +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev) > +{ > + struct fusb302_chip *chip = container_of(dev, struct fusb302_chip, > + tcpc_dev); > + return chip->supported_pd_rev; > +} > + > static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl) > { > if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX) > @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev) > fusb302_tcpc_dev->set_roles = tcpm_set_roles; > fusb302_tcpc_dev->start_toggling = tcpm_start_toggling; > fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit; > + fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev; > } > > static const char * const cc_polarity_name[] = { > @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client, > struct fusb302_chip *chip; > struct i2c_adapter *adapter = client->adapter; > struct device *dev = &client->dev; > + struct device_node *np = dev->of_node; > const char *name; > int ret = 0; > > @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client, > dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); > goto tcpm_unregister_port; > } > + > + if (of_property_read_u32(np, "supported-pd-rev", > + &chip->supported_pd_rev) < 0) { > + chip->supported_pd_rev = PD_MAX_REV; > + } else if (chip->supported_pd_rev > PD_MAX_REV) { > + chip->supported_pd_rev = PD_MAX_REV; > + } The else part is also checked in the tcpm code and thus seems to be redundant. > + > enable_irq_wake(chip->gpio_int_n_irq); > i2c_set_clientdata(client, chip); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev 2022-02-08 15:22 ` Guenter Roeck @ 2022-02-09 3:34 ` Potin Lai 2022-02-09 17:50 ` Guenter Roeck 0 siblings, 1 reply; 14+ messages in thread From: Potin Lai @ 2022-02-09 3:34 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams Guenter Roeck 於 2022/2/8 下午 11:22 寫道: > On 2/8/22 03:22, Potin Lai wrote: >> Add support for passing supported PD rev to TCPM. >> If "supported-pd-rev" property exist, then return supported_pd_rev as >> defined value in DTS, otherwise return PD_MAX_REV >> >> Example of DTS: >> >> fusb302: typec-portc@22 { >> compatible = "fcs,fusb302"; >> reg = <0x22>; >> ... >> supported-pd-rev=<1>; // PD_REV20 >> ... >> }; >> > > The new property needs to be documented. However, I am not entirely sure > I understand why it is needed. Wouldn't the supported PD revision > be a chip (fusb302) specific constant ? If so, why does it need a > devicetree property ? I think that needs some additional explanation. > > Thanks, > Guenter > My initially intend was adding flexibility for developer to decided which PD revision they want to use for negotiation. I saw your reply in another patch, I agree with you, it will be simple and consistent if move "supported-pd-rev" to tcpm fwnode as other capabilities. Just want to double confirm, is "usb-connector.yaml" right place to put documentation if adding "supported-pd-rev" in fwnode? Thanks, Potin >> Signed-off-by: Potin Lai <potin.lai@quantatw.com> >> --- >> drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/usb/typec/tcpm/fusb302.c >> b/drivers/usb/typec/tcpm/fusb302.c >> index 72f9001b0792..8cff92d58b96 100644 >> --- a/drivers/usb/typec/tcpm/fusb302.c >> +++ b/drivers/usb/typec/tcpm/fusb302.c >> @@ -109,6 +109,9 @@ struct fusb302_chip { >> enum typec_cc_status cc2; >> u32 snk_pdo[PDO_MAX_OBJECTS]; >> + /* supported pd rev */ >> + u32 supported_pd_rev; >> + >> #ifdef CONFIG_DEBUG_FS >> struct dentry *dentry; >> /* lock for log buffer access */ >> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev >> *dev, enum tcpm_transmit_type type, >> return ret; >> } >> +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev) >> +{ >> + struct fusb302_chip *chip = container_of(dev, struct fusb302_chip, >> + tcpc_dev); >> + return chip->supported_pd_rev; >> +} >> + >> static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl) >> { >> if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX) >> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev >> *fusb302_tcpc_dev) >> fusb302_tcpc_dev->set_roles = tcpm_set_roles; >> fusb302_tcpc_dev->start_toggling = tcpm_start_toggling; >> fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit; >> + fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev; >> } >> static const char * const cc_polarity_name[] = { >> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client >> *client, >> struct fusb302_chip *chip; >> struct i2c_adapter *adapter = client->adapter; >> struct device *dev = &client->dev; >> + struct device_node *np = dev->of_node; >> const char *name; >> int ret = 0; >> @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client >> *client, >> dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", >> ret); >> goto tcpm_unregister_port; >> } >> + >> + if (of_property_read_u32(np, "supported-pd-rev", >> + &chip->supported_pd_rev) < 0) { >> + chip->supported_pd_rev = PD_MAX_REV; >> + } else if (chip->supported_pd_rev > PD_MAX_REV) { >> + chip->supported_pd_rev = PD_MAX_REV; >> + } > > The else part is also checked in the tcpm code and thus seems > to be redundant. > >> + >> enable_irq_wake(chip->gpio_int_n_irq); >> i2c_set_clientdata(client, chip); > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev 2022-02-09 3:34 ` Potin Lai @ 2022-02-09 17:50 ` Guenter Roeck 2022-02-11 8:35 ` Potin Lai (賴柏廷) 0 siblings, 1 reply; 14+ messages in thread From: Guenter Roeck @ 2022-02-09 17:50 UTC (permalink / raw) To: Potin Lai, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams On 2/8/22 19:34, Potin Lai wrote: > > Guenter Roeck 於 2022/2/8 下午 11:22 寫道: >> On 2/8/22 03:22, Potin Lai wrote: >>> Add support for passing supported PD rev to TCPM. >>> If "supported-pd-rev" property exist, then return supported_pd_rev as >>> defined value in DTS, otherwise return PD_MAX_REV >>> >>> Example of DTS: >>> >>> fusb302: typec-portc@22 { >>> compatible = "fcs,fusb302"; >>> reg = <0x22>; >>> ... >>> supported-pd-rev=<1>; // PD_REV20 >>> ... >>> }; >>> >> >> The new property needs to be documented. However, I am not entirely sure >> I understand why it is needed. Wouldn't the supported PD revision >> be a chip (fusb302) specific constant ? If so, why does it need a >> devicetree property ? I think that needs some additional explanation. >> >> Thanks, >> Guenter >> > > My initially intend was adding flexibility for developer to decided which PD revision > they want to use for negotiation. > I may be missing something, but I don't think that "flexibility for developer to decide which PD revision they want to use for negotiation" is entirely appropriate. This should be a chip property, not something a developer should decide. As written, the code does accept PC version 3 for FUSB302 by default, which seems odd and unusual. If the chip supports PD version 3, why artificially limit it to earlier PD revisions ? I think this requires a detailed description of the use case. Is fusb302 indeed not able to support a specific version of the power delivery specification ? If yes, what is the limitation, and why would it need to be configurable with a devicetree property ? Thanks, Guenter > I saw your reply in another patch, I agree with you, it will be simple and consistent if > move "supported-pd-rev" to tcpm fwnode as other capabilities. > > Just want to double confirm, is "usb-connector.yaml" right place to put documentation > if adding "supported-pd-rev" in fwnode? > > Thanks, > Potin > >>> Signed-off-by: Potin Lai <potin.lai@quantatw.com> >>> --- >>> drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c >>> index 72f9001b0792..8cff92d58b96 100644 >>> --- a/drivers/usb/typec/tcpm/fusb302.c >>> +++ b/drivers/usb/typec/tcpm/fusb302.c >>> @@ -109,6 +109,9 @@ struct fusb302_chip { >>> enum typec_cc_status cc2; >>> u32 snk_pdo[PDO_MAX_OBJECTS]; >>> + /* supported pd rev */ >>> + u32 supported_pd_rev; >>> + >>> #ifdef CONFIG_DEBUG_FS >>> struct dentry *dentry; >>> /* lock for log buffer access */ >>> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type, >>> return ret; >>> } >>> +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev) >>> +{ >>> + struct fusb302_chip *chip = container_of(dev, struct fusb302_chip, >>> + tcpc_dev); >>> + return chip->supported_pd_rev; >>> +} >>> + >>> static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl) >>> { >>> if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX) >>> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev) >>> fusb302_tcpc_dev->set_roles = tcpm_set_roles; >>> fusb302_tcpc_dev->start_toggling = tcpm_start_toggling; >>> fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit; >>> + fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev; >>> } >>> static const char * const cc_polarity_name[] = { >>> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client, >>> struct fusb302_chip *chip; >>> struct i2c_adapter *adapter = client->adapter; >>> struct device *dev = &client->dev; >>> + struct device_node *np = dev->of_node; >>> const char *name; >>> int ret = 0; >>> @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client, >>> dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); >>> goto tcpm_unregister_port; >>> } >>> + >>> + if (of_property_read_u32(np, "supported-pd-rev", >>> + &chip->supported_pd_rev) < 0) { >>> + chip->supported_pd_rev = PD_MAX_REV; >>> + } else if (chip->supported_pd_rev > PD_MAX_REV) { >>> + chip->supported_pd_rev = PD_MAX_REV; >>> + } >> >> The else part is also checked in the tcpm code and thus seems >> to be redundant. >> >>> + >>> enable_irq_wake(chip->gpio_int_n_irq); >>> i2c_set_clientdata(client, chip); >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev 2022-02-09 17:50 ` Guenter Roeck @ 2022-02-11 8:35 ` Potin Lai (賴柏廷) 0 siblings, 0 replies; 14+ messages in thread From: Potin Lai (賴柏廷) @ 2022-02-11 8:35 UTC (permalink / raw) To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman Cc: linux-usb, linux-kernel, Patrick Williams Guenter Roeck 於 2022/2/10 上午 01:50 寫道: > On 2/8/22 19:34, Potin Lai wrote: >> >> Guenter Roeck 於 2022/2/8 下午 11:22 寫道: >>> On 2/8/22 03:22, Potin Lai wrote: >>>> Add support for passing supported PD rev to TCPM. >>>> If "supported-pd-rev" property exist, then return supported_pd_rev as >>>> defined value in DTS, otherwise return PD_MAX_REV >>>> >>>> Example of DTS: >>>> >>>> fusb302: typec-portc@22 { >>>> compatible = "fcs,fusb302"; >>>> reg = <0x22>; >>>> ... >>>> supported-pd-rev=<1>; // PD_REV20 >>>> ... >>>> }; >>>> >>> >>> The new property needs to be documented. However, I am not entirely sure >>> I understand why it is needed. Wouldn't the supported PD revision >>> be a chip (fusb302) specific constant ? If so, why does it need a >>> devicetree property ? I think that needs some additional explanation. >>> >>> Thanks, >>> Guenter >>> >> >> My initially intend was adding flexibility for developer to decided which PD revision >> they want to use for negotiation. >> > > I may be missing something, but I don't think that "flexibility for developer > to decide which PD revision they want to use for negotiation" is entirely appropriate. > This should be a chip property, not something a developer should decide. As written, > the code does accept PC version 3 for FUSB302 by default, which seems odd and unusual. > If the chip supports PD version 3, why artificially limit it to earlier PD revisions ? > > I think this requires a detailed description of the use case. Is fusb302 indeed not able > to support a specific version of the power delivery specification ? If yes, > what is the limitation, and why would it need to be configurable with a devicetree > property ? > > Thanks, > Guenter > [ 414.375215] AMS DISCOVER_IDENTITY start [ 414.375228] cc:=4 [ 414.380834] PD RX, header: 0x291 [1] [ 414.380846] AMS DISCOVER_IDENTITY finished [ 414.380850] cc:=5 [ 414.388203] PD RX, header: 0x291 [1] [ 414.388211] PD RX, header: 0x291 [1] [ 414.388220] PD TX, header: 0x7b0 [ 414.499594] Received hard reset [ 414.499615] state change SRC_READY -> HARD_RESET_START [rev3 HARD_RESET] In my case, if I keep it as PD_MAX_REV (3.0), I always receive hard reset after "PD RX, header: 0x291" (Get_Source_Cap_Extended), then entire state machine will start over again and again. If I force PD rev at 2.0, then I won't receive Get_Source_Cap_Extended message, and state machine become stable. Not quite sure the reason of receiving hard reset, my guessing, fus302 not recognize PD 3.0 message, so it doesn't reply GoodCRC automatically, and then it causing timeout or events to trigger hard reset from the other side. Thanks, Potin >> I saw your reply in another patch, I agree with you, it will be simple and consistent if >> move "supported-pd-rev" to tcpm fwnode as other capabilities. >> >> Just want to double confirm, is "usb-connector.yaml" right place to put documentation >> if adding "supported-pd-rev" in fwnode? >> >> Thanks, >> Potin >> >>>> Signed-off-by: Potin Lai <potin.lai@quantatw.com> >>>> --- >>>> drivers/usb/typec/tcpm/fusb302.c | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c >>>> index 72f9001b0792..8cff92d58b96 100644 >>>> --- a/drivers/usb/typec/tcpm/fusb302.c >>>> +++ b/drivers/usb/typec/tcpm/fusb302.c >>>> @@ -109,6 +109,9 @@ struct fusb302_chip { >>>> enum typec_cc_status cc2; >>>> u32 snk_pdo[PDO_MAX_OBJECTS]; >>>> + /* supported pd rev */ >>>> + u32 supported_pd_rev; >>>> + >>>> #ifdef CONFIG_DEBUG_FS >>>> struct dentry *dentry; >>>> /* lock for log buffer access */ >>>> @@ -1056,6 +1059,13 @@ static int tcpm_pd_transmit(struct tcpc_dev *dev, enum tcpm_transmit_type type, >>>> return ret; >>>> } >>>> +static u32 tcpm_supported_pd_rev(struct tcpc_dev *dev) >>>> +{ >>>> + struct fusb302_chip *chip = container_of(dev, struct fusb302_chip, >>>> + tcpc_dev); >>>> + return chip->supported_pd_rev; >>>> +} >>>> + >>>> static enum typec_cc_status fusb302_bc_lvl_to_cc(u8 bc_lvl) >>>> { >>>> if (bc_lvl == FUSB_REG_STATUS0_BC_LVL_1230_MAX) >>>> @@ -1129,6 +1139,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev) >>>> fusb302_tcpc_dev->set_roles = tcpm_set_roles; >>>> fusb302_tcpc_dev->start_toggling = tcpm_start_toggling; >>>> fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit; >>>> + fusb302_tcpc_dev->supported_pd_rev = tcpm_supported_pd_rev; >>>> } >>>> static const char * const cc_polarity_name[] = { >>>> @@ -1683,6 +1694,7 @@ static int fusb302_probe(struct i2c_client *client, >>>> struct fusb302_chip *chip; >>>> struct i2c_adapter *adapter = client->adapter; >>>> struct device *dev = &client->dev; >>>> + struct device_node *np = dev->of_node; >>>> const char *name; >>>> int ret = 0; >>>> @@ -1756,6 +1768,14 @@ static int fusb302_probe(struct i2c_client *client, >>>> dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); >>>> goto tcpm_unregister_port; >>>> } >>>> + >>>> + if (of_property_read_u32(np, "supported-pd-rev", >>>> + &chip->supported_pd_rev) < 0) { >>>> + chip->supported_pd_rev = PD_MAX_REV; >>>> + } else if (chip->supported_pd_rev > PD_MAX_REV) { >>>> + chip->supported_pd_rev = PD_MAX_REV; >>>> + } >>> >>> The else part is also checked in the tcpm code and thus seems >>> to be redundant. >>> >>>> + >>>> enable_irq_wake(chip->gpio_int_n_irq); >>>> i2c_set_clientdata(client, chip); >>> > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-02-11 8:35 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-08 8:20 [PATCH 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2022-02-08 8:20 ` [PATCH 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai 2022-02-08 8:41 ` Greg Kroah-Hartman 2022-02-08 8:20 ` [PATCH 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai 2022-02-08 11:22 ` [PATCH v2 0/2] Add a interface for passing supported PD rev from TCPC Potin Lai 2022-02-08 11:22 ` [PATCH v2 1/2] usb: typec: tcpm: add interface for passing supported_pd_rev from tcpc_dev Potin Lai 2022-02-08 15:25 ` Guenter Roeck 2022-02-08 16:41 ` kernel test robot 2022-02-09 6:53 ` kernel test robot 2022-02-08 11:22 ` [PATCH v2 2/2] usb: typec: fusb302: add support of supported_pd_rev Potin Lai 2022-02-08 15:22 ` Guenter Roeck 2022-02-09 3:34 ` Potin Lai 2022-02-09 17:50 ` Guenter Roeck 2022-02-11 8:35 ` Potin Lai (賴柏廷)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).