| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZrW7rPfbf68W5ZHk@kuha.fi.intel.com>
Date: Fri, 9 Aug 2024 09:48:12 +0300
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: André Draszik <andre.draszik@...aro.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Peter Griffin <peter.griffin@...aro.org>,
Tudor Ambarus <tudor.ambarus@...aro.org>,
Will McVicker <willmcvicker@...gle.com>, kernel-team@...roid.com,
RD Babiera <rdbabiera@...gle.com>,
Badhri Jagan Sridharan <badhri@...gle.com>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] usb: typec: tcpm/tcpci_maxim: fix non-contaminant CC
handling
On Tue, Aug 06, 2024 at 05:25:24PM +0100, André Draszik wrote:
> tcpci_maxim currently never triggers the TCPM state machine when CC
> status has not changed due to a contaminant but due to a real
> connection event, i.e. a genuine plug event, meaning the system will
> stay idle and not notify any subscribers.
>
> The reason is that the initial state of the port is 'toggling', which
> causes _max_tcpci_irq() to only drive the contamination part of the
> TCPM state machine (via tcpm_port_clean()).
>
> What should happen instead is that if no contamination was detected,
> the TCPM should be notified of the CC change in this case.
>
> To fix this, we update ...is_contaminant() to also allow its caller to
> determine if more CC processing is required and then call into the TCPM
> as required.
>
> While at it, add a kernel-doc for max_contaminant_is_contaminant().
>
> Note: the code has an issue where I2C errors during contaminant
> detection also cause the TCPM state machine to not be updated. This
> commit doesn't change this behaviour and should be addressed by
> follow-up commit(s).
>
> Signed-off-by: André Draszik <andre.draszik@...aro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> ---
> Changes in v2:
> - longer line length in tcpci_maxim_core.c (Heikki)
> - update kernel-doc (Badhri)
> - drop now-obsolete comment for cc_handled in _max_tcpci_irq() (we have
> kernel-doc now)
> - Link to v1: https://lore.kernel.org/r/20240710-max33359-toggling-v1-1-f6dc123f3a0a@linaro.org
> ---
> drivers/usb/typec/tcpm/maxim_contaminant.c | 7 +++++--
> drivers/usb/typec/tcpm/tcpci_maxim.h | 16 +++++++++++++++-
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 11 +++++++----
> 3 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
> index f8504a90da26..e7fa3e36f8ae 100644
> --- a/drivers/usb/typec/tcpm/maxim_contaminant.c
> +++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
> @@ -322,11 +322,14 @@ static int max_contaminant_enable_dry_detection(struct max_tcpci_chip *chip)
> return 0;
> }
>
> -bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect_while_debounce)
> +bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect_while_debounce,
> + bool *cc_handled)
> {
> u8 cc_status, pwr_cntl;
> int ret;
>
> + *cc_handled = true;
> +
> ret = max_tcpci_read8(chip, TCPC_CC_STATUS, &cc_status);
> if (ret < 0)
> return false;
> @@ -368,7 +371,6 @@ bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect
> return true;
> }
> }
> - return false;
> } else if (chip->contaminant_state == DETECTED) {
> if (STATUS_CHECK(cc_status, TCPC_CC_STATUS_TOGGLING, 0)) {
> chip->contaminant_state = max_contaminant_detect_contaminant(chip);
> @@ -379,6 +381,7 @@ bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect
> }
> }
>
> + *cc_handled = false;
> return false;
> }
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
> index 78ff3b73ee7e..7719e291132a 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim.h
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
> @@ -85,6 +85,20 @@ static inline int max_tcpci_write8(struct max_tcpci_chip *chip, unsigned int reg
> return regmap_raw_write(chip->data.regmap, reg, &val, sizeof(u8));
> }
>
> -bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect_while_debounce);
> +/**
> + * max_contaminant_is_contaminant - Test if CC was toggled due to contaminant
> + *
> + * @chip: Handle to a struct max_tcpci_chip
> + * @disconnect_while_debounce: Whether the disconnect was detected when CC
> + * pins were debouncing
> + * @cc_handled: Returns whether or not update to CC status was handled here
> + *
> + * Determine if a contaminant was detected.
> + *
> + * Returns: true if a contaminant was detected, false otherwise. cc_handled
> + * is updated to reflect whether or not further CC handling is required.
> + */
> +bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect_while_debounce,
> + bool *cc_handled);
>
> #endif // TCPCI_MAXIM_H_
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index 760e2f92b958..22bc05064b3e 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -357,12 +357,14 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
> tcpm_vbus_change(chip->port);
>
> if (status & TCPC_ALERT_CC_STATUS) {
> + bool cc_handled = false;
> +
> if (chip->contaminant_state == DETECTED || tcpm_port_is_toggling(chip->port)) {
> - if (!max_contaminant_is_contaminant(chip, false))
> + if (!max_contaminant_is_contaminant(chip, false, &cc_handled))
> tcpm_port_clean(chip->port);
> - } else {
> - tcpm_cc_change(chip->port);
> }
> + if (!cc_handled)
> + tcpm_cc_change(chip->port);
> }
>
> if (status & TCPC_ALERT_POWER_STATUS)
> @@ -455,8 +457,9 @@ static int tcpci_init(struct tcpci *tcpci, struct tcpci_data *data)
> static void max_tcpci_check_contaminant(struct tcpci *tcpci, struct tcpci_data *tdata)
> {
> struct max_tcpci_chip *chip = tdata_to_max_tcpci(tdata);
> + bool cc_handled;
>
> - if (!max_contaminant_is_contaminant(chip, true))
> + if (!max_contaminant_is_contaminant(chip, true, &cc_handled))
> tcpm_port_clean(chip->port);
> }
>
>
> ---
> base-commit: 1e391b34f6aa043c7afa40a2103163a0ef06d179
> change-id: 20240710-max33359-toggling-cf7f7e5e1443
>
> Best regards,
> --
> André Draszik <andre.draszik@...aro.org>
--
heikki
Powered by blists - more mailing lists