linux-kernel - Re: [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask())

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dac4a84c72bba076ea0d3a51a78b6d350732a7a6.camel@linaro.org>
Date: Fri, 26 Jul 2024 06:59:01 +0100
From: André Draszik <andre.draszik@...aro.org>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Greg Kroah-Hartman
	 <gregkh@...uxfoundation.org>
Cc: Peter Griffin <peter.griffin@...aro.org>, Tudor Ambarus
 <tudor.ambarus@...aro.org>, Will McVicker <willmcvicker@...gle.com>, Badhri
 Jagan Sridharan <badhri@...gle.com>, kernel-team@...roid.com,
 linux-usb@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim
 cleanups (mostly genmask())

Hi,

On Wed, 2024-07-10 at 11:36 +0100, André Draszik wrote:
> While looking through the TCPCi drivers, it occurred to me that all of the
> open-coded register bit manipulations can be simplified and made more
> legible by using the standard GENMASK(), FIELD_GET(), and FIELD_PREP()
> macros, which also is less prone to errors: e.g.
> 
> 	if (((reg & (TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT)) >>
> 	     TCPC_ROLE_CTRL_CC2_SHIFT) !=
> 	    ((reg & (TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT)) >>
> 	     TCPC_ROLE_CTRL_CC1_SHIFT))
> 
> (arguably) is much harder to read and reason about than:
> 
> 	if (FIELD_GET(TCPC_ROLE_CTRL_CC2, reg) != FIELD_GET(TCPC_ROLE_CTRL_CC1, reg))
> 
> and so on.
> 
> These patches do that. In addition, there are a few comment fixes and I
> added in a conversion to using dev_err_probe() and
> devm_add_action_or_reset() in the Maxim TCPCi driver.
> 
> I kept the patches separated by register or register field as appropriate to
> simplify reviewing, allowing to focus on one field/register at a time.
> 
> Signed-off-by: André Draszik <andre.draszik@...aro.org>
> ---
> André Draszik (15):
>       usb: typec: tcpci: fix a comment typo
>       usb: typec: tcpm/tcpci_maxim: clarify a comment
>       usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12]
>       usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12]
>       usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL
>       usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV
>       usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields
>       usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit
>       usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF
>       usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK()
>       usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register
>       usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register
>       usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register
>       usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe()
>       usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration
> 
>  drivers/usb/typec/anx7411.c                |   5 +-
>  drivers/usb/typec/tcpm/maxim_contaminant.c |  46 +++++++------
>  drivers/usb/typec/tcpm/tcpci.c             | 104 ++++++++++++++---------------
>  drivers/usb/typec/tcpm/tcpci_maxim.h       |  18 ++---
>  drivers/usb/typec/tcpm/tcpci_maxim_core.c  |  71 ++++++++++----------
>  drivers/usb/typec/tcpm/tcpci_rt1711h.c     |  27 ++++----
>  include/linux/usb/tcpci.h                  |  31 +++------
>  7 files changed, 144 insertions(+), 158 deletions(-)

Any comments on this series?

Cheers,
Andre'