| Message ID | 20240710-tcpc-cleanup-v1-0-0ec1f41f4263@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) | expand |
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@linaro.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'
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@linaro.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(-) --- base-commit: 82d01fe6ee52086035b201cfa1410a3b04384257 change-id: 20240710-tcpc-cleanup-61e29124a87d Best regards,