* [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask())
@ 2024-07-10 10:36 André Draszik
2024-07-10 10:36 ` [PATCH 01/15] usb: typec: tcpci: fix a comment typo André Draszik
` (15 more replies)
0 siblings, 16 replies; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
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,
--
André Draszik <andre.draszik@linaro.org>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 01/15] usb: typec: tcpci: fix a comment typo
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 9:47 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 02/15] usb: typec: tcpm/tcpci_maxim: clarify a comment André Draszik
` (14 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
autonously -> autonomously
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
include/linux/usb/tcpci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 0ab39b6ea205..d27fe0c22a8b 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -190,7 +190,7 @@ struct tcpci;
* Optional; Callback to perform chip specific operations when FRS
* is sourcing vbus.
* @auto_discharge_disconnect:
- * Optional; Enables TCPC to autonously discharge vbus on disconnect.
+ * Optional; Enables TCPC to autonomously discharge vbus on disconnect.
* @vbus_vsafe0v:
* optional; Set when TCPC can detect whether vbus is at VSAFE0V.
* @set_partner_usb_comm_capable:
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 02/15] usb: typec: tcpm/tcpci_maxim: clarify a comment
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
2024-07-10 10:36 ` [PATCH 01/15] usb: typec: tcpci: fix a comment typo André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 10:10 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 03/15] usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12] André Draszik
` (13 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
We loop while the status is != 0, so rephrase the comment slightly for
clarity.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci_maxim_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
index eec3bcec119c..87102b4d060d 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
@@ -397,7 +397,7 @@ static irqreturn_t max_tcpci_irq(int irq, void *dev_id)
}
while (status) {
irq_return = _max_tcpci_irq(chip, status);
- /* Do not return if the ALERT is already set. */
+ /* Do not return if a (new) ALERT is set (again). */
ret = max_tcpci_read16(chip, TCPC_ALERT, &status);
if (ret < 0)
break;
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 03/15] usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12]
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
2024-07-10 10:36 ` [PATCH 01/15] usb: typec: tcpci: fix a comment typo André Draszik
2024-07-10 10:36 ` [PATCH 02/15] usb: typec: tcpm/tcpci_maxim: clarify a comment André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 10:26 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 04/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12] André Draszik
` (12 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
The existing code here, particularly in maxim_contaminant.c, is
arguably quite hard to read due to the open-coded masking and shifting
spanning multiple lines.
Use GENMASK() and FIELD_GET() instead, which arguably make the code
much easier to follow.
While at it, use the symbolic name TCPC_CC_STATE_SRC_OPEN for one
instance of open-coded 0x0.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/maxim_contaminant.c | 8 +++-----
drivers/usb/typec/tcpm/tcpci.c | 7 +++----
drivers/usb/typec/tcpm/tcpci_rt1711h.c | 7 +++----
include/linux/usb/tcpci.h | 8 +++-----
4 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
index f8504a90da26..e7687aeb69c0 100644
--- a/drivers/usb/typec/tcpm/maxim_contaminant.c
+++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
@@ -5,6 +5,7 @@
* USB-C module to reduce wakeups due to contaminants.
*/
+#include <linux/bitfield.h>
#include <linux/device.h>
#include <linux/irqreturn.h>
#include <linux/module.h>
@@ -48,11 +49,8 @@ enum fladc_select {
#define STATUS_CHECK(reg, mask, val) (((reg) & (mask)) == (val))
#define IS_CC_OPEN(cc_status) \
- (STATUS_CHECK((cc_status), TCPC_CC_STATUS_CC1_MASK << TCPC_CC_STATUS_CC1_SHIFT, \
- TCPC_CC_STATE_SRC_OPEN) && STATUS_CHECK((cc_status), \
- TCPC_CC_STATUS_CC2_MASK << \
- TCPC_CC_STATUS_CC2_SHIFT, \
- TCPC_CC_STATE_SRC_OPEN))
+ (FIELD_GET(TCPC_CC_STATUS_CC1, cc_status) == TCPC_CC_STATE_SRC_OPEN \
+ && FIELD_GET(TCPC_CC_STATUS_CC2, cc_status) == TCPC_CC_STATE_SRC_OPEN)
static int max_contaminant_adc_to_mv(struct max_tcpci_chip *chip, enum fladc_select channel,
bool ua_src, u8 fladc)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 8a18d561b063..ce11a154c7dc 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -5,6 +5,7 @@
* USB Type-C Port Controller Interface.
*/
+#include <linux/bitfield.h>
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -241,12 +242,10 @@ static int tcpci_get_cc(struct tcpc_dev *tcpc,
if (ret < 0)
return ret;
- *cc1 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC1_SHIFT) &
- TCPC_CC_STATUS_CC1_MASK,
+ *cc1 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC1, reg),
reg & TCPC_CC_STATUS_TERM ||
tcpc_presenting_rd(role_control, CC1));
- *cc2 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC2_SHIFT) &
- TCPC_CC_STATUS_CC2_MASK,
+ *cc2 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC2, reg),
reg & TCPC_CC_STATUS_TERM ||
tcpc_presenting_rd(role_control, CC2));
diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
index 67422d45eb54..c6dbccf6b17a 100644
--- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
+++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
@@ -5,6 +5,7 @@
* Richtek RT1711H Type-C Chip Driver
*/
+#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/kernel.h>
#include <linux/mod_devicetable.h>
@@ -195,12 +196,10 @@ static inline int rt1711h_init_cc_params(struct rt1711h_chip *chip, u8 status)
if (ret < 0)
return ret;
- cc1 = tcpci_to_typec_cc((status >> TCPC_CC_STATUS_CC1_SHIFT) &
- TCPC_CC_STATUS_CC1_MASK,
+ cc1 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC1, status),
status & TCPC_CC_STATUS_TERM ||
tcpc_presenting_rd(role, CC1));
- cc2 = tcpci_to_typec_cc((status >> TCPC_CC_STATUS_CC2_SHIFT) &
- TCPC_CC_STATUS_CC2_MASK,
+ cc2 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC2, status),
status & TCPC_CC_STATUS_TERM ||
tcpc_presenting_rd(role, CC2));
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index d27fe0c22a8b..31d21ccf662b 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -92,11 +92,9 @@
#define TCPC_CC_STATUS_TERM BIT(4)
#define TCPC_CC_STATUS_TERM_RP 0
#define TCPC_CC_STATUS_TERM_RD 1
+#define TCPC_CC_STATUS_CC2 GENMASK(3, 2)
+#define TCPC_CC_STATUS_CC1 GENMASK(1, 0)
#define TCPC_CC_STATE_SRC_OPEN 0
-#define TCPC_CC_STATUS_CC2_SHIFT 2
-#define TCPC_CC_STATUS_CC2_MASK 0x3
-#define TCPC_CC_STATUS_CC1_SHIFT 0
-#define TCPC_CC_STATUS_CC1_MASK 0x3
#define TCPC_POWER_STATUS 0x1e
#define TCPC_POWER_STATUS_DBG_ACC_CON BIT(7)
@@ -256,7 +254,7 @@ static inline enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
if (sink)
return TYPEC_CC_RP_3_0;
fallthrough;
- case 0x0:
+ case TCPC_CC_STATE_SRC_OPEN:
default:
return TYPEC_CC_OPEN;
}
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 04/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12]
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (2 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 03/15] usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12] André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 10:28 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 05/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL André Draszik
` (11 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
All this open-coded shifting and masking is quite hard to read, in
particular the if-statement in tcpci_apply_rc().
Declare TCPC_ROLE_CTRL_CC[12] using GENMASK() which allows using
FIELD_GET() and FIELD_PREP() to arguably make the code more legible.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/anx7411.c | 5 ++-
drivers/usb/typec/tcpm/tcpci.c | 73 +++++++++++++++-------------------
drivers/usb/typec/tcpm/tcpci_rt1711h.c | 8 ++--
include/linux/usb/tcpci.h | 9 ++---
4 files changed, 43 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/typec/anx7411.c b/drivers/usb/typec/anx7411.c
index b12a07edc71b..78b0d856cfc1 100644
--- a/drivers/usb/typec/anx7411.c
+++ b/drivers/usb/typec/anx7411.c
@@ -6,6 +6,7 @@
* Copyright(c) 2022, Analogix Semiconductor. All rights reserved.
*
*/
+#include <linux/bitfield.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
@@ -884,8 +885,8 @@ static void anx7411_chip_standby(struct anx7411_data *ctx)
OCM_RESET);
ret |= anx7411_reg_write(ctx->tcpc_client, ANALOG_CTRL_10, 0x80);
/* Set TCPC to RD and DRP enable */
- cc1 = TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT;
- cc2 = TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT;
+ cc1 = FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD);
+ cc2 = FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD);
ret |= anx7411_reg_write(ctx->tcpc_client, TCPC_ROLE_CTRL,
TCPC_ROLE_CTRL_DRP | cc1 | cc2);
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index ce11a154c7dc..cd71ece7b956 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -104,45 +104,42 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
switch (cc) {
case TYPEC_CC_RA:
- reg = (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RA)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RA));
break;
case TYPEC_CC_RD:
- reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD));
break;
case TYPEC_CC_RP_DEF:
- reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
- (TCPC_ROLE_CTRL_RP_VAL_DEF <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
+ | (TCPC_ROLE_CTRL_RP_VAL_DEF << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
break;
case TYPEC_CC_RP_1_5:
- reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
- (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
+ | (TCPC_ROLE_CTRL_RP_VAL_1_5 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
break;
case TYPEC_CC_RP_3_0:
- reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
- (TCPC_ROLE_CTRL_RP_VAL_3_0 <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
+ | (TCPC_ROLE_CTRL_RP_VAL_3_0 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
break;
case TYPEC_CC_OPEN:
default:
- reg = (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_OPEN)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_OPEN));
break;
}
if (vconn_pres) {
if (polarity == TYPEC_POLARITY_CC2) {
- reg &= ~(TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT);
- reg |= (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT);
+ reg &= ~TCPC_ROLE_CTRL_CC1;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_OPEN);
} else {
- reg &= ~(TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT);
- reg |= (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg &= ~TCPC_ROLE_CTRL_CC2;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_OPEN);
}
}
@@ -168,15 +165,11 @@ static int tcpci_apply_rc(struct tcpc_dev *tcpc, enum typec_cc_status cc,
* APPLY_RC state is when ROLE_CONTROL.CC1 != ROLE_CONTROL.CC2 and vbus autodischarge on
* disconnect is disabled. Bail out when ROLE_CONTROL.CC1 != ROLE_CONTROL.CC2.
*/
- 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))
+ if (FIELD_GET(TCPC_ROLE_CTRL_CC2, reg) != FIELD_GET(TCPC_ROLE_CTRL_CC1, reg))
return 0;
return regmap_update_bits(tcpci->regmap, TCPC_ROLE_CTRL, polarity == TYPEC_POLARITY_CC1 ?
- TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT :
- TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT,
+ TCPC_ROLE_CTRL_CC2 : TCPC_ROLE_CTRL_CC1,
TCPC_ROLE_CTRL_CC_OPEN);
}
@@ -215,11 +208,11 @@ static int tcpci_start_toggling(struct tcpc_dev *tcpc,
}
if (cc == TYPEC_CC_RD)
- reg |= (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD));
else
- reg |= (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP));
ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
if (ret < 0)
return ret;
@@ -281,28 +274,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
reg = reg & ~TCPC_ROLE_CTRL_DRP;
if (polarity == TYPEC_POLARITY_CC2) {
- reg &= ~(TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg &= ~TCPC_ROLE_CTRL_CC2;
/* Local port is source */
if (cc2 == TYPEC_CC_RD)
/* Role control would have the Rp setting when DRP was enabled */
- reg |= TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP);
else
- reg |= TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD);
} else {
- reg &= ~(TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT);
+ reg &= ~TCPC_ROLE_CTRL_CC1;
/* Local port is source */
if (cc1 == TYPEC_CC_RD)
/* Role control would have the Rp setting when DRP was enabled */
- reg |= TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP);
else
- reg |= TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD);
}
}
if (polarity == TYPEC_POLARITY_CC2)
- reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_OPEN);
else
- reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_OPEN);
ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
if (ret < 0)
return ret;
diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
index c6dbccf6b17a..bdb78d08b5b5 100644
--- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
+++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
@@ -246,11 +246,11 @@ static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
}
if (cc == TYPEC_CC_RD)
- reg |= (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD));
else
- reg |= (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
- (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT);
+ reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
+ | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP));
ret = rt1711h_write8(chip, TCPC_ROLE_CTRL, reg);
if (ret < 0)
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 31d21ccf662b..552d074429f0 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -68,10 +68,8 @@
#define TCPC_ROLE_CTRL_RP_VAL_DEF 0x0
#define TCPC_ROLE_CTRL_RP_VAL_1_5 0x1
#define TCPC_ROLE_CTRL_RP_VAL_3_0 0x2
-#define TCPC_ROLE_CTRL_CC2_SHIFT 2
-#define TCPC_ROLE_CTRL_CC2_MASK 0x3
-#define TCPC_ROLE_CTRL_CC1_SHIFT 0
-#define TCPC_ROLE_CTRL_CC1_MASK 0x3
+#define TCPC_ROLE_CTRL_CC2 GENMASK(3, 2)
+#define TCPC_ROLE_CTRL_CC1 GENMASK(1, 0)
#define TCPC_ROLE_CTRL_CC_RA 0x0
#define TCPC_ROLE_CTRL_CC_RP 0x1
#define TCPC_ROLE_CTRL_CC_RD 0x2
@@ -176,8 +174,7 @@
#define tcpc_presenting_rd(reg, cc) \
(!(TCPC_ROLE_CTRL_DRP & (reg)) && \
- (((reg) & (TCPC_ROLE_CTRL_## cc ##_MASK << TCPC_ROLE_CTRL_## cc ##_SHIFT)) == \
- (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_## cc ##_SHIFT)))
+ FIELD_GET(TCPC_ROLE_CTRL_## cc, reg) == TCPC_ROLE_CTRL_CC_RD)
struct tcpci;
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 05/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (3 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 04/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12] André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 10:37 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 06/15] usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV André Draszik
` (10 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Align the last remaining field TCPC_ROLE_CTRL_RP_VAL with the other
fields in the TCPC_ROLE_CTRL register by using GENMASK() and
FIELD_PREP().
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci.c | 21 ++++++++++++---------
drivers/usb/typec/tcpm/tcpci_rt1711h.c | 12 ++++++------
include/linux/usb/tcpci.h | 3 +--
3 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index cd71ece7b956..5ad05a5bbbd1 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -114,17 +114,20 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
case TYPEC_CC_RP_DEF:
reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
| FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
- | (TCPC_ROLE_CTRL_RP_VAL_DEF << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
+ | FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_DEF));
break;
case TYPEC_CC_RP_1_5:
reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
| FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
- | (TCPC_ROLE_CTRL_RP_VAL_1_5 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
+ | FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_1_5));
break;
case TYPEC_CC_RP_3_0:
reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
| FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
- | (TCPC_ROLE_CTRL_RP_VAL_3_0 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
+ | FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_3_0));
break;
case TYPEC_CC_OPEN:
default:
@@ -194,16 +197,16 @@ static int tcpci_start_toggling(struct tcpc_dev *tcpc,
switch (cc) {
default:
case TYPEC_CC_RP_DEF:
- reg |= (TCPC_ROLE_CTRL_RP_VAL_DEF <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_DEF);
break;
case TYPEC_CC_RP_1_5:
- reg |= (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_1_5);
break;
case TYPEC_CC_RP_3_0:
- reg |= (TCPC_ROLE_CTRL_RP_VAL_3_0 <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_3_0);
break;
}
diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
index bdb78d08b5b5..64f6dd0dc660 100644
--- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
+++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
@@ -232,16 +232,16 @@ static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
switch (cc) {
default:
case TYPEC_CC_RP_DEF:
- reg |= (TCPC_ROLE_CTRL_RP_VAL_DEF <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_DEF);
break;
case TYPEC_CC_RP_1_5:
- reg |= (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_1_5);
break;
case TYPEC_CC_RP_3_0:
- reg |= (TCPC_ROLE_CTRL_RP_VAL_3_0 <<
- TCPC_ROLE_CTRL_RP_VAL_SHIFT);
+ reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
+ TCPC_ROLE_CTRL_RP_VAL_3_0);
break;
}
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 552d074429f0..80652d4f722e 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -63,8 +63,7 @@
#define TCPC_ROLE_CTRL 0x1a
#define TCPC_ROLE_CTRL_DRP BIT(6)
-#define TCPC_ROLE_CTRL_RP_VAL_SHIFT 4
-#define TCPC_ROLE_CTRL_RP_VAL_MASK 0x3
+#define TCPC_ROLE_CTRL_RP_VAL GENMASK(5, 4)
#define TCPC_ROLE_CTRL_RP_VAL_DEF 0x0
#define TCPC_ROLE_CTRL_RP_VAL_1_5 0x1
#define TCPC_ROLE_CTRL_RP_VAL_3_0 0x2
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 06/15] usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (4 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 05/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 10:39 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 07/15] usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields André Draszik
` (9 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Convert field TCPC_MSG_HDR_INFO_REV from register TCPC_MSG_HDR_INFO to
using GENMASK() and FIELD_PREP() so as to keep using a similar approach
for all fields.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci.c | 2 +-
include/linux/usb/tcpci.h | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 5ad05a5bbbd1..ad5c9d5bf6a9 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -456,7 +456,7 @@ static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
unsigned int reg;
int ret;
- reg = PD_REV20 << TCPC_MSG_HDR_INFO_REV_SHIFT;
+ reg = FIELD_PREP(TCPC_MSG_HDR_INFO_REV, PD_REV20);
if (role == TYPEC_SOURCE)
reg |= TCPC_MSG_HDR_INFO_PWR_ROLE;
if (data == TYPEC_HOST)
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 80652d4f722e..3cd61e9f73b3 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -129,9 +129,8 @@
#define TCPC_MSG_HDR_INFO 0x2e
#define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
+#define TCPC_MSG_HDR_INFO_REV GENMASK(2, 1)
#define TCPC_MSG_HDR_INFO_PWR_ROLE BIT(0)
-#define TCPC_MSG_HDR_INFO_REV_SHIFT 1
-#define TCPC_MSG_HDR_INFO_REV_MASK 0x3
#define TCPC_RX_DETECT 0x2f
#define TCPC_RX_DETECT_HARD_RESET BIT(5)
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 07/15] usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (5 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 06/15] usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 11:01 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 08/15] usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit André Draszik
` (8 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Convert all fields from register TCPC_TRANSMIT to using GENMASK() and
FIELD_PREP() so as to keep using a similar approach throughout the code
base and make it arguably easier to read.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci.c | 7 +++++--
include/linux/usb/tcpci.h | 6 ++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index ad5c9d5bf6a9..b9ee9ccff99b 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -607,8 +607,11 @@ static int tcpci_pd_transmit(struct tcpc_dev *tcpc, enum tcpm_transmit_type type
}
/* nRetryCount is 3 in PD2.0 spec where 2 in PD3.0 spec */
- reg = ((negotiated_rev > PD_REV20 ? PD_RETRY_COUNT_3_0_OR_HIGHER : PD_RETRY_COUNT_DEFAULT)
- << TCPC_TRANSMIT_RETRY_SHIFT) | (type << TCPC_TRANSMIT_TYPE_SHIFT);
+ reg = FIELD_PREP(TCPC_TRANSMIT_RETRY,
+ (negotiated_rev > PD_REV20
+ ? PD_RETRY_COUNT_3_0_OR_HIGHER
+ : PD_RETRY_COUNT_DEFAULT));
+ reg |= FIELD_PREP(TCPC_TRANSMIT_TYPE, type);
ret = regmap_write(tcpci->regmap, TCPC_TRANSMIT, reg);
if (ret < 0)
return ret;
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 3cd61e9f73b3..f7f5cfbdef12 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -148,10 +148,8 @@
#define TCPC_RX_DATA 0x34 /* through 0x4f */
#define TCPC_TRANSMIT 0x50
-#define TCPC_TRANSMIT_RETRY_SHIFT 4
-#define TCPC_TRANSMIT_RETRY_MASK 0x3
-#define TCPC_TRANSMIT_TYPE_SHIFT 0
-#define TCPC_TRANSMIT_TYPE_MASK 0x7
+#define TCPC_TRANSMIT_RETRY GENMASK(5, 4)
+#define TCPC_TRANSMIT_TYPE GENMASK(2, 0)
#define TCPC_TX_BYTE_CNT 0x51
#define TCPC_TX_HDR 0x52
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 08/15] usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (6 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 07/15] usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 11:09 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 09/15] usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF André Draszik
` (7 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
This makes it easier to see what's missing and what's being programmed.
While at it, add brackets to help with formatting and remove a comment
that doesn't add much value.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci_maxim_core.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
index 87102b4d060d..ad9bb61fd9e0 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
@@ -97,11 +97,13 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
if (ret < 0)
return;
- alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
- TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
- TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
- /* Enable Extended alert for detecting Fast Role Swap Signal */
- TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS | TCPC_ALERT_FAULT;
+ alert_mask = (TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED |
+ TCPC_ALERT_TX_FAILED | TCPC_ALERT_RX_HARD_RST |
+ TCPC_ALERT_RX_STATUS | TCPC_ALERT_POWER_STATUS |
+ TCPC_ALERT_CC_STATUS |
+ TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS |
+ TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF |
+ TCPC_ALERT_FAULT);
ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
if (ret < 0) {
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 09/15] usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (7 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 08/15] usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 11:12 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 10/15] usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK() André Draszik
` (6 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
There is no need for using the ternary if/else here, simply mask
TCPC_ALERT_RX_BUF_OVF as necessary, which arguably makes the code
easier to read.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci_maxim_core.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
index ad9bb61fd9e0..5b5441db7047 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
@@ -193,9 +193,8 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
* Read complete, clear RX status alert bit.
* Clear overflow as well if set.
*/
- ret = max_tcpci_write16(chip, TCPC_ALERT, status & TCPC_ALERT_RX_BUF_OVF ?
- TCPC_ALERT_RX_STATUS | TCPC_ALERT_RX_BUF_OVF :
- TCPC_ALERT_RX_STATUS);
+ ret = max_tcpci_write16(chip, TCPC_ALERT,
+ TCPC_ALERT_RX_STATUS | (status & TCPC_ALERT_RX_BUF_OVF));
if (ret < 0)
return;
@@ -297,9 +296,8 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
* be cleared until we have successfully retrieved message.
*/
if (status & ~TCPC_ALERT_RX_STATUS) {
- mask = status & TCPC_ALERT_RX_BUF_OVF ?
- status & ~(TCPC_ALERT_RX_STATUS | TCPC_ALERT_RX_BUF_OVF) :
- status & ~TCPC_ALERT_RX_STATUS;
+ mask = status & ~(TCPC_ALERT_RX_STATUS
+ | (status & TCPC_ALERT_RX_BUF_OVF));
ret = max_tcpci_write16(chip, TCPC_ALERT, mask);
if (ret < 0) {
dev_err(chip->dev, "ALERT clear failed\n");
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 10/15] usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK()
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (8 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 09/15] usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 12:29 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 11/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register André Draszik
` (5 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Only one user of STATUS_CHECK() remains, and the code can actually be
made more legible by simply avoiding the use of that wrapper macro,
allowing to also drop the macro altogether.
Do so.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/maxim_contaminant.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
index e7687aeb69c0..8ac8eeade277 100644
--- a/drivers/usb/typec/tcpm/maxim_contaminant.c
+++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
@@ -46,8 +46,6 @@ enum fladc_select {
#define READ1_SLEEP_MS 10
#define READ2_SLEEP_MS 5
-#define STATUS_CHECK(reg, mask, val) (((reg) & (mask)) == (val))
-
#define IS_CC_OPEN(cc_status) \
(FIELD_GET(TCPC_CC_STATUS_CC1, cc_status) == TCPC_CC_STATE_SRC_OPEN \
&& FIELD_GET(TCPC_CC_STATUS_CC2, cc_status) == TCPC_CC_STATE_SRC_OPEN)
@@ -368,7 +366,7 @@ bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect
}
return false;
} else if (chip->contaminant_state == DETECTED) {
- if (STATUS_CHECK(cc_status, TCPC_CC_STATUS_TOGGLING, 0)) {
+ if (!(cc_status & TCPC_CC_STATUS_TOGGLING)) {
chip->contaminant_state = max_contaminant_detect_contaminant(chip);
if (chip->contaminant_state == DETECTED) {
max_contaminant_enable_dry_detection(chip);
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 11/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (9 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 10/15] usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK() André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 12:36 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 12/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register André Draszik
` (4 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Convert register TCPC_VENDOR_CC_CTRL2 to using GENMASK() and
FIELD_PREP() so as to keep using a similar approach throughout the code
base and make it arguably easier to read.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/maxim_contaminant.c | 18 +++++++++++-------
drivers/usb/typec/tcpm/tcpci_maxim.h | 6 +++---
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
index 8ac8eeade277..f7acaa42329f 100644
--- a/drivers/usb/typec/tcpm/maxim_contaminant.c
+++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
@@ -116,13 +116,14 @@ static int max_contaminant_read_resistance_kohm(struct max_tcpci_chip *chip,
if (channel == CC1_SCALE1 || channel == CC2_SCALE1 || channel == CC1_SCALE2 ||
channel == CC2_SCALE2) {
/* Enable 1uA current source */
- ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL_MASK,
- ULTRA_LOW_POWER_MODE);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL,
+ FIELD_PREP(CCLPMODESEL, ULTRA_LOW_POWER_MODE));
if (ret < 0)
return ret;
/* Enable 1uA current source */
- ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL_MASK, UA_1_SRC);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL,
+ FIELD_PREP(CCRPCTRL, UA_1_SRC));
if (ret < 0)
return ret;
@@ -176,7 +177,8 @@ static int max_contaminant_read_comparators(struct max_tcpci_chip *chip, u8 *ven
int ret;
/* Enable 80uA source */
- ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL_MASK, UA_80_SRC);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL,
+ FIELD_PREP(CCRPCTRL, UA_80_SRC));
if (ret < 0)
return ret;
@@ -209,7 +211,8 @@ static int max_contaminant_read_comparators(struct max_tcpci_chip *chip, u8 *ven
if (ret < 0)
return ret;
- ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL_MASK, 0);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL,
+ FIELD_PREP(CCRPCTRL, 0));
if (ret < 0)
return ret;
@@ -298,8 +301,9 @@ static int max_contaminant_enable_dry_detection(struct max_tcpci_chip *chip)
if (ret < 0)
return ret;
- ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL_MASK,
- ULTRA_LOW_POWER_MODE);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL,
+ FIELD_PREP(CCLPMODESEL,
+ ULTRA_LOW_POWER_MODE));
if (ret < 0)
return ret;
ret = max_tcpci_read8(chip, TCPC_VENDOR_CC_CTRL2, &temp);
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
index 78ff3b73ee7e..92c9a628ebe1 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim.h
+++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
@@ -20,9 +20,9 @@
#define SBUOVPDIS BIT(7)
#define CCOVPDIS BIT(6)
#define SBURPCTRL BIT(5)
-#define CCLPMODESEL_MASK GENMASK(4, 3)
-#define ULTRA_LOW_POWER_MODE BIT(3)
-#define CCRPCTRL_MASK GENMASK(2, 0)
+#define CCLPMODESEL GENMASK(4, 3)
+#define ULTRA_LOW_POWER_MODE 1
+#define CCRPCTRL GENMASK(2, 0)
#define UA_1_SRC 1
#define UA_80_SRC 3
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 12/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (10 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 11/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 12:38 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 13/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register André Draszik
` (3 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Convert register TCPC_VENDOR_CC_CTRL3 to using GENMASK() so as to keep
using a similar approach throughout the code base and make it arguably
easier to read.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/maxim_contaminant.c | 9 +++++----
drivers/usb/typec/tcpm/tcpci_maxim.h | 9 +++------
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
index f7acaa42329f..cf9887de96c9 100644
--- a/drivers/usb/typec/tcpm/maxim_contaminant.c
+++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
@@ -283,10 +283,11 @@ static int max_contaminant_enable_dry_detection(struct max_tcpci_chip *chip)
u8 temp;
int ret;
- ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL3, CCWTRDEB_MASK | CCWTRSEL_MASK
- | WTRCYCLE_MASK, CCWTRDEB_1MS << CCWTRDEB_SHIFT |
- CCWTRSEL_1V << CCWTRSEL_SHIFT | WTRCYCLE_4_8_S <<
- WTRCYCLE_SHIFT);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL3,
+ CCWTRDEB | CCWTRSEL | WTRCYCLE,
+ FIELD_PREP(CCWTRDEB, CCWTRDEB_1MS)
+ | FIELD_PREP(CCWTRSEL, CCWTRSEL_1V)
+ | FIELD_PREP(WTRCYCLE, WTRCYCLE_4_8_S));
if (ret < 0)
return ret;
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
index 92c9a628ebe1..34076069444f 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim.h
+++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
@@ -27,15 +27,12 @@
#define UA_80_SRC 3
#define TCPC_VENDOR_CC_CTRL3 0x8e
-#define CCWTRDEB_MASK GENMASK(7, 6)
-#define CCWTRDEB_SHIFT 6
+#define CCWTRDEB GENMASK(7, 6)
#define CCWTRDEB_1MS 1
-#define CCWTRSEL_MASK GENMASK(5, 3)
-#define CCWTRSEL_SHIFT 3
+#define CCWTRSEL GENMASK(5, 3)
#define CCWTRSEL_1V 0x4
#define CCLADDERDIS BIT(2)
-#define WTRCYCLE_MASK BIT(0)
-#define WTRCYCLE_SHIFT 0
+#define WTRCYCLE GENMASK(0, 0)
#define WTRCYCLE_2_4_S 0
#define WTRCYCLE_4_8_S 1
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 13/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (11 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 12/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 12:40 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 14/15] usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe() André Draszik
` (2 subsequent siblings)
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Convert register TCPC_VENDOR_ADC_CTRL1 to using GENMASK() and
FIELD_PREP() so as to keep using a similar approach throughout the code
base and make it arguably easier to read.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/maxim_contaminant.c | 7 ++++---
drivers/usb/typec/tcpm/tcpci_maxim.h | 3 +--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
index cf9887de96c9..7bfec45e8f4f 100644
--- a/drivers/usb/typec/tcpm/maxim_contaminant.c
+++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
@@ -76,8 +76,8 @@ static int max_contaminant_read_adc_mv(struct max_tcpci_chip *chip, enum fladc_s
int ret;
/* Channel & scale select */
- ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL_MASK,
- channel << ADC_CHANNEL_OFFSET);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL,
+ FIELD_PREP(ADCINSEL, channel));
if (ret < 0)
return ret;
@@ -96,7 +96,8 @@ static int max_contaminant_read_adc_mv(struct max_tcpci_chip *chip, enum fladc_s
if (ret < 0)
return ret;
- ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL_MASK, 0);
+ ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL,
+ FIELD_PREP(ADCINSEL, 0));
if (ret < 0)
return ret;
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
index 34076069444f..a41ca9e7ad08 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim.h
+++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
@@ -37,8 +37,7 @@
#define WTRCYCLE_4_8_S 1
#define TCPC_VENDOR_ADC_CTRL1 0x91
-#define ADCINSEL_MASK GENMASK(7, 5)
-#define ADC_CHANNEL_OFFSET 5
+#define ADCINSEL GENMASK(7, 5)
#define ADCEN BIT(0)
enum contamiant_state {
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 14/15] usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe()
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (12 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 13/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 12:44 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 15/15] usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration André Draszik
2024-07-26 5:59 ` [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
dev_err_probe() exists as a useful helper ensuring standardized
error messages during .probe() and using it also helps to make
the code more legible.
Use it.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci_maxim_core.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
index 5b5441db7047..ee3e86797f17 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
@@ -484,17 +484,17 @@ static int max_tcpci_probe(struct i2c_client *client)
chip->client = client;
chip->data.regmap = devm_regmap_init_i2c(client, &max_tcpci_regmap_config);
- if (IS_ERR(chip->data.regmap)) {
- dev_err(&client->dev, "Regmap init failed\n");
- return PTR_ERR(chip->data.regmap);
- }
+ if (IS_ERR(chip->data.regmap))
+ return dev_err_probe(&client->dev, PTR_ERR(chip->data.regmap),
+ "Regmap init failed\n");
chip->dev = &client->dev;
i2c_set_clientdata(client, chip);
ret = max_tcpci_read8(chip, TCPC_POWER_STATUS, &power_status);
if (ret < 0)
- return ret;
+ return dev_err_probe(&client->dev, ret,
+ "Failed to read TCPC_POWER_STATUS\n");
/* Chip level tcpci callbacks */
chip->data.set_vbus = max_tcpci_set_vbus;
@@ -511,10 +511,10 @@ static int max_tcpci_probe(struct i2c_client *client)
max_tcpci_init_regs(chip);
chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
- if (IS_ERR(chip->tcpci)) {
- dev_err(&client->dev, "TCPCI port registration failed\n");
- return PTR_ERR(chip->tcpci);
- }
+ if (IS_ERR(chip->tcpci))
+ return dev_err_probe(&client->dev, PTR_ERR(chip->tcpci),
+ "TCPCI port registration failed\n");
+
chip->port = tcpci_get_tcpm_port(chip->tcpci);
ret = max_tcpci_init_alert(chip, client);
if (ret < 0)
@@ -526,7 +526,8 @@ static int max_tcpci_probe(struct i2c_client *client)
unreg_port:
tcpci_unregister_port(chip->tcpci);
- return ret;
+ return dev_err_probe(&client->dev, ret,
+ "Maxim TCPCI driver initialization failed\n");
}
static void max_tcpci_remove(struct i2c_client *client)
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 15/15] usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (13 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 14/15] usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe() André Draszik
@ 2024-07-10 10:36 ` André Draszik
2024-07-31 12:45 ` Heikki Krogerus
2024-07-26 5:59 ` [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
15 siblings, 1 reply; 32+ messages in thread
From: André Draszik @ 2024-07-10 10:36 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel,
André Draszik
Instead of open-coding the call to tcpci_unregister_port() in various
places, let's just register a hook using devm_add_action_or_reset() so
that it gets called automatically as and when necessary by the device
management APIs.
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
drivers/usb/typec/tcpm/tcpci_maxim_core.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
index ee3e86797f17..7abfd29b4b01 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
@@ -472,6 +472,11 @@ static bool max_tcpci_attempt_vconn_swap_discovery(struct tcpci *tcpci, struct t
return true;
}
+static void max_tcpci_unregister_tcpci_port(void *tcpci)
+{
+ tcpci_unregister_port(tcpci);
+}
+
static int max_tcpci_probe(struct i2c_client *client)
{
int ret;
@@ -515,27 +520,21 @@ static int max_tcpci_probe(struct i2c_client *client)
return dev_err_probe(&client->dev, PTR_ERR(chip->tcpci),
"TCPCI port registration failed\n");
+ ret = devm_add_action_or_reset(&client->dev,
+ max_tcpci_unregister_tcpci_port,
+ chip->tcpci);
+ if (ret)
+ return ret;
+
chip->port = tcpci_get_tcpm_port(chip->tcpci);
+
ret = max_tcpci_init_alert(chip, client);
if (ret < 0)
- goto unreg_port;
+ return dev_err_probe(&client->dev, ret,
+ "IRQ initialization failed\n");
device_init_wakeup(chip->dev, true);
return 0;
-
-unreg_port:
- tcpci_unregister_port(chip->tcpci);
-
- return dev_err_probe(&client->dev, ret,
- "Maxim TCPCI driver initialization failed\n");
-}
-
-static void max_tcpci_remove(struct i2c_client *client)
-{
- struct max_tcpci_chip *chip = i2c_get_clientdata(client);
-
- if (!IS_ERR_OR_NULL(chip->tcpci))
- tcpci_unregister_port(chip->tcpci);
}
static const struct i2c_device_id max_tcpci_id[] = {
@@ -558,7 +557,6 @@ static struct i2c_driver max_tcpci_i2c_driver = {
.of_match_table = of_match_ptr(max_tcpci_of_match),
},
.probe = max_tcpci_probe,
- .remove = max_tcpci_remove,
.id_table = max_tcpci_id,
};
module_i2c_driver(max_tcpci_i2c_driver);
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask())
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
` (14 preceding siblings ...)
2024-07-10 10:36 ` [PATCH 15/15] usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration André Draszik
@ 2024-07-26 5:59 ` André Draszik
15 siblings, 0 replies; 32+ messages in thread
From: André Draszik @ 2024-07-26 5:59 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman
Cc: Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
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'
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 01/15] usb: typec: tcpci: fix a comment typo
2024-07-10 10:36 ` [PATCH 01/15] usb: typec: tcpci: fix a comment typo André Draszik
@ 2024-07-31 9:47 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 9:47 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:08AM +0100, André Draszik wrote:
> autonously -> autonomously
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> include/linux/usb/tcpci.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 0ab39b6ea205..d27fe0c22a8b 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -190,7 +190,7 @@ struct tcpci;
> * Optional; Callback to perform chip specific operations when FRS
> * is sourcing vbus.
> * @auto_discharge_disconnect:
> - * Optional; Enables TCPC to autonously discharge vbus on disconnect.
> + * Optional; Enables TCPC to autonomously discharge vbus on disconnect.
> * @vbus_vsafe0v:
> * optional; Set when TCPC can detect whether vbus is at VSAFE0V.
> * @set_partner_usb_comm_capable:
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 02/15] usb: typec: tcpm/tcpci_maxim: clarify a comment
2024-07-10 10:36 ` [PATCH 02/15] usb: typec: tcpm/tcpci_maxim: clarify a comment André Draszik
@ 2024-07-31 10:10 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 10:10 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:09AM +0100, André Draszik wrote:
> We loop while the status is != 0, so rephrase the comment slightly for
> clarity.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index eec3bcec119c..87102b4d060d 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -397,7 +397,7 @@ static irqreturn_t max_tcpci_irq(int irq, void *dev_id)
> }
> while (status) {
> irq_return = _max_tcpci_irq(chip, status);
> - /* Do not return if the ALERT is already set. */
> + /* Do not return if a (new) ALERT is set (again). */
> ret = max_tcpci_read16(chip, TCPC_ALERT, &status);
> if (ret < 0)
> break;
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 03/15] usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12]
2024-07-10 10:36 ` [PATCH 03/15] usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12] André Draszik
@ 2024-07-31 10:26 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 10:26 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:10AM +0100, André Draszik wrote:
> The existing code here, particularly in maxim_contaminant.c, is
> arguably quite hard to read due to the open-coded masking and shifting
> spanning multiple lines.
>
> Use GENMASK() and FIELD_GET() instead, which arguably make the code
> much easier to follow.
>
> While at it, use the symbolic name TCPC_CC_STATE_SRC_OPEN for one
> instance of open-coded 0x0.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/maxim_contaminant.c | 8 +++-----
> drivers/usb/typec/tcpm/tcpci.c | 7 +++----
> drivers/usb/typec/tcpm/tcpci_rt1711h.c | 7 +++----
> include/linux/usb/tcpci.h | 8 +++-----
> 4 files changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
> index f8504a90da26..e7687aeb69c0 100644
> --- a/drivers/usb/typec/tcpm/maxim_contaminant.c
> +++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
> @@ -5,6 +5,7 @@
> * USB-C module to reduce wakeups due to contaminants.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/device.h>
> #include <linux/irqreturn.h>
> #include <linux/module.h>
> @@ -48,11 +49,8 @@ enum fladc_select {
> #define STATUS_CHECK(reg, mask, val) (((reg) & (mask)) == (val))
>
> #define IS_CC_OPEN(cc_status) \
> - (STATUS_CHECK((cc_status), TCPC_CC_STATUS_CC1_MASK << TCPC_CC_STATUS_CC1_SHIFT, \
> - TCPC_CC_STATE_SRC_OPEN) && STATUS_CHECK((cc_status), \
> - TCPC_CC_STATUS_CC2_MASK << \
> - TCPC_CC_STATUS_CC2_SHIFT, \
> - TCPC_CC_STATE_SRC_OPEN))
> + (FIELD_GET(TCPC_CC_STATUS_CC1, cc_status) == TCPC_CC_STATE_SRC_OPEN \
> + && FIELD_GET(TCPC_CC_STATUS_CC2, cc_status) == TCPC_CC_STATE_SRC_OPEN)
>
> static int max_contaminant_adc_to_mv(struct max_tcpci_chip *chip, enum fladc_select channel,
> bool ua_src, u8 fladc)
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 8a18d561b063..ce11a154c7dc 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -5,6 +5,7 @@
> * USB Type-C Port Controller Interface.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/delay.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> @@ -241,12 +242,10 @@ static int tcpci_get_cc(struct tcpc_dev *tcpc,
> if (ret < 0)
> return ret;
>
> - *cc1 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC1_SHIFT) &
> - TCPC_CC_STATUS_CC1_MASK,
> + *cc1 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC1, reg),
> reg & TCPC_CC_STATUS_TERM ||
> tcpc_presenting_rd(role_control, CC1));
> - *cc2 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC2_SHIFT) &
> - TCPC_CC_STATUS_CC2_MASK,
> + *cc2 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC2, reg),
> reg & TCPC_CC_STATUS_TERM ||
> tcpc_presenting_rd(role_control, CC2));
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> index 67422d45eb54..c6dbccf6b17a 100644
> --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> @@ -5,6 +5,7 @@
> * Richtek RT1711H Type-C Chip Driver
> */
>
> +#include <linux/bitfield.h>
> #include <linux/bits.h>
> #include <linux/kernel.h>
> #include <linux/mod_devicetable.h>
> @@ -195,12 +196,10 @@ static inline int rt1711h_init_cc_params(struct rt1711h_chip *chip, u8 status)
> if (ret < 0)
> return ret;
>
> - cc1 = tcpci_to_typec_cc((status >> TCPC_CC_STATUS_CC1_SHIFT) &
> - TCPC_CC_STATUS_CC1_MASK,
> + cc1 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC1, status),
> status & TCPC_CC_STATUS_TERM ||
> tcpc_presenting_rd(role, CC1));
> - cc2 = tcpci_to_typec_cc((status >> TCPC_CC_STATUS_CC2_SHIFT) &
> - TCPC_CC_STATUS_CC2_MASK,
> + cc2 = tcpci_to_typec_cc(FIELD_GET(TCPC_CC_STATUS_CC2, status),
> status & TCPC_CC_STATUS_TERM ||
> tcpc_presenting_rd(role, CC2));
>
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index d27fe0c22a8b..31d21ccf662b 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -92,11 +92,9 @@
> #define TCPC_CC_STATUS_TERM BIT(4)
> #define TCPC_CC_STATUS_TERM_RP 0
> #define TCPC_CC_STATUS_TERM_RD 1
> +#define TCPC_CC_STATUS_CC2 GENMASK(3, 2)
> +#define TCPC_CC_STATUS_CC1 GENMASK(1, 0)
> #define TCPC_CC_STATE_SRC_OPEN 0
> -#define TCPC_CC_STATUS_CC2_SHIFT 2
> -#define TCPC_CC_STATUS_CC2_MASK 0x3
> -#define TCPC_CC_STATUS_CC1_SHIFT 0
> -#define TCPC_CC_STATUS_CC1_MASK 0x3
>
> #define TCPC_POWER_STATUS 0x1e
> #define TCPC_POWER_STATUS_DBG_ACC_CON BIT(7)
> @@ -256,7 +254,7 @@ static inline enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
> if (sink)
> return TYPEC_CC_RP_3_0;
> fallthrough;
> - case 0x0:
> + case TCPC_CC_STATE_SRC_OPEN:
> default:
> return TYPEC_CC_OPEN;
> }
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 04/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12]
2024-07-10 10:36 ` [PATCH 04/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12] André Draszik
@ 2024-07-31 10:28 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 10:28 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:11AM +0100, André Draszik wrote:
> All this open-coded shifting and masking is quite hard to read, in
> particular the if-statement in tcpci_apply_rc().
>
> Declare TCPC_ROLE_CTRL_CC[12] using GENMASK() which allows using
> FIELD_GET() and FIELD_PREP() to arguably make the code more legible.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/anx7411.c | 5 ++-
> drivers/usb/typec/tcpm/tcpci.c | 73 +++++++++++++++-------------------
> drivers/usb/typec/tcpm/tcpci_rt1711h.c | 8 ++--
> include/linux/usb/tcpci.h | 9 ++---
> 4 files changed, 43 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/usb/typec/anx7411.c b/drivers/usb/typec/anx7411.c
> index b12a07edc71b..78b0d856cfc1 100644
> --- a/drivers/usb/typec/anx7411.c
> +++ b/drivers/usb/typec/anx7411.c
> @@ -6,6 +6,7 @@
> * Copyright(c) 2022, Analogix Semiconductor. All rights reserved.
> *
> */
> +#include <linux/bitfield.h>
> #include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> @@ -884,8 +885,8 @@ static void anx7411_chip_standby(struct anx7411_data *ctx)
> OCM_RESET);
> ret |= anx7411_reg_write(ctx->tcpc_client, ANALOG_CTRL_10, 0x80);
> /* Set TCPC to RD and DRP enable */
> - cc1 = TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT;
> - cc2 = TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT;
> + cc1 = FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD);
> + cc2 = FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD);
> ret |= anx7411_reg_write(ctx->tcpc_client, TCPC_ROLE_CTRL,
> TCPC_ROLE_CTRL_DRP | cc1 | cc2);
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ce11a154c7dc..cd71ece7b956 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -104,45 +104,42 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>
> switch (cc) {
> case TYPEC_CC_RA:
> - reg = (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RA)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RA));
> break;
> case TYPEC_CC_RD:
> - reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD));
> break;
> case TYPEC_CC_RP_DEF:
> - reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
> - (TCPC_ROLE_CTRL_RP_VAL_DEF <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
> + | (TCPC_ROLE_CTRL_RP_VAL_DEF << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
> break;
> case TYPEC_CC_RP_1_5:
> - reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
> - (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
> + | (TCPC_ROLE_CTRL_RP_VAL_1_5 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
> break;
> case TYPEC_CC_RP_3_0:
> - reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
> - (TCPC_ROLE_CTRL_RP_VAL_3_0 <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
> + | (TCPC_ROLE_CTRL_RP_VAL_3_0 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
> break;
> case TYPEC_CC_OPEN:
> default:
> - reg = (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_OPEN)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_OPEN));
> break;
> }
>
> if (vconn_pres) {
> if (polarity == TYPEC_POLARITY_CC2) {
> - reg &= ~(TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT);
> - reg |= (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT);
> + reg &= ~TCPC_ROLE_CTRL_CC1;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_OPEN);
> } else {
> - reg &= ~(TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT);
> - reg |= (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg &= ~TCPC_ROLE_CTRL_CC2;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_OPEN);
> }
> }
>
> @@ -168,15 +165,11 @@ static int tcpci_apply_rc(struct tcpc_dev *tcpc, enum typec_cc_status cc,
> * APPLY_RC state is when ROLE_CONTROL.CC1 != ROLE_CONTROL.CC2 and vbus autodischarge on
> * disconnect is disabled. Bail out when ROLE_CONTROL.CC1 != ROLE_CONTROL.CC2.
> */
> - 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))
> + if (FIELD_GET(TCPC_ROLE_CTRL_CC2, reg) != FIELD_GET(TCPC_ROLE_CTRL_CC1, reg))
> return 0;
>
> return regmap_update_bits(tcpci->regmap, TCPC_ROLE_CTRL, polarity == TYPEC_POLARITY_CC1 ?
> - TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT :
> - TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT,
> + TCPC_ROLE_CTRL_CC2 : TCPC_ROLE_CTRL_CC1,
> TCPC_ROLE_CTRL_CC_OPEN);
> }
>
> @@ -215,11 +208,11 @@ static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> }
>
> if (cc == TYPEC_CC_RD)
> - reg |= (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD));
> else
> - reg |= (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP));
> ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> if (ret < 0)
> return ret;
> @@ -281,28 +274,28 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> reg = reg & ~TCPC_ROLE_CTRL_DRP;
>
> if (polarity == TYPEC_POLARITY_CC2) {
> - reg &= ~(TCPC_ROLE_CTRL_CC2_MASK << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg &= ~TCPC_ROLE_CTRL_CC2;
> /* Local port is source */
> if (cc2 == TYPEC_CC_RD)
> /* Role control would have the Rp setting when DRP was enabled */
> - reg |= TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP);
> else
> - reg |= TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD);
> } else {
> - reg &= ~(TCPC_ROLE_CTRL_CC1_MASK << TCPC_ROLE_CTRL_CC1_SHIFT);
> + reg &= ~TCPC_ROLE_CTRL_CC1;
> /* Local port is source */
> if (cc1 == TYPEC_CC_RD)
> /* Role control would have the Rp setting when DRP was enabled */
> - reg |= TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP);
> else
> - reg |= TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD);
> }
> }
>
> if (polarity == TYPEC_POLARITY_CC2)
> - reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_OPEN);
> else
> - reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_OPEN);
> ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> if (ret < 0)
> return ret;
> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> index c6dbccf6b17a..bdb78d08b5b5 100644
> --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> @@ -246,11 +246,11 @@ static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
> }
>
> if (cc == TYPEC_CC_RD)
> - reg |= (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RD)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RD));
> else
> - reg |= (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> - (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT);
> + reg |= (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> + | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP));
>
> ret = rt1711h_write8(chip, TCPC_ROLE_CTRL, reg);
> if (ret < 0)
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 31d21ccf662b..552d074429f0 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -68,10 +68,8 @@
> #define TCPC_ROLE_CTRL_RP_VAL_DEF 0x0
> #define TCPC_ROLE_CTRL_RP_VAL_1_5 0x1
> #define TCPC_ROLE_CTRL_RP_VAL_3_0 0x2
> -#define TCPC_ROLE_CTRL_CC2_SHIFT 2
> -#define TCPC_ROLE_CTRL_CC2_MASK 0x3
> -#define TCPC_ROLE_CTRL_CC1_SHIFT 0
> -#define TCPC_ROLE_CTRL_CC1_MASK 0x3
> +#define TCPC_ROLE_CTRL_CC2 GENMASK(3, 2)
> +#define TCPC_ROLE_CTRL_CC1 GENMASK(1, 0)
> #define TCPC_ROLE_CTRL_CC_RA 0x0
> #define TCPC_ROLE_CTRL_CC_RP 0x1
> #define TCPC_ROLE_CTRL_CC_RD 0x2
> @@ -176,8 +174,7 @@
>
> #define tcpc_presenting_rd(reg, cc) \
> (!(TCPC_ROLE_CTRL_DRP & (reg)) && \
> - (((reg) & (TCPC_ROLE_CTRL_## cc ##_MASK << TCPC_ROLE_CTRL_## cc ##_SHIFT)) == \
> - (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_## cc ##_SHIFT)))
> + FIELD_GET(TCPC_ROLE_CTRL_## cc, reg) == TCPC_ROLE_CTRL_CC_RD)
>
> struct tcpci;
>
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 05/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL
2024-07-10 10:36 ` [PATCH 05/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL André Draszik
@ 2024-07-31 10:37 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 10:37 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:12AM +0100, André Draszik wrote:
> Align the last remaining field TCPC_ROLE_CTRL_RP_VAL with the other
> fields in the TCPC_ROLE_CTRL register by using GENMASK() and
> FIELD_PREP().
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 21 ++++++++++++---------
> drivers/usb/typec/tcpm/tcpci_rt1711h.c | 12 ++++++------
> include/linux/usb/tcpci.h | 3 +--
> 3 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index cd71ece7b956..5ad05a5bbbd1 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -114,17 +114,20 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> case TYPEC_CC_RP_DEF:
> reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
> - | (TCPC_ROLE_CTRL_RP_VAL_DEF << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
> + | FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_DEF));
> break;
> case TYPEC_CC_RP_1_5:
> reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
> - | (TCPC_ROLE_CTRL_RP_VAL_1_5 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
> + | FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_1_5));
> break;
> case TYPEC_CC_RP_3_0:
> reg = (FIELD_PREP(TCPC_ROLE_CTRL_CC1, TCPC_ROLE_CTRL_CC_RP)
> | FIELD_PREP(TCPC_ROLE_CTRL_CC2, TCPC_ROLE_CTRL_CC_RP)
> - | (TCPC_ROLE_CTRL_RP_VAL_3_0 << TCPC_ROLE_CTRL_RP_VAL_SHIFT));
> + | FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_3_0));
> break;
> case TYPEC_CC_OPEN:
> default:
> @@ -194,16 +197,16 @@ static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> switch (cc) {
> default:
> case TYPEC_CC_RP_DEF:
> - reg |= (TCPC_ROLE_CTRL_RP_VAL_DEF <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_DEF);
> break;
> case TYPEC_CC_RP_1_5:
> - reg |= (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_1_5);
> break;
> case TYPEC_CC_RP_3_0:
> - reg |= (TCPC_ROLE_CTRL_RP_VAL_3_0 <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_3_0);
> break;
> }
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> index bdb78d08b5b5..64f6dd0dc660 100644
> --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> @@ -232,16 +232,16 @@ static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
> switch (cc) {
> default:
> case TYPEC_CC_RP_DEF:
> - reg |= (TCPC_ROLE_CTRL_RP_VAL_DEF <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_DEF);
> break;
> case TYPEC_CC_RP_1_5:
> - reg |= (TCPC_ROLE_CTRL_RP_VAL_1_5 <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_1_5);
> break;
> case TYPEC_CC_RP_3_0:
> - reg |= (TCPC_ROLE_CTRL_RP_VAL_3_0 <<
> - TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> + reg |= FIELD_PREP(TCPC_ROLE_CTRL_RP_VAL,
> + TCPC_ROLE_CTRL_RP_VAL_3_0);
> break;
> }
>
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 552d074429f0..80652d4f722e 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -63,8 +63,7 @@
>
> #define TCPC_ROLE_CTRL 0x1a
> #define TCPC_ROLE_CTRL_DRP BIT(6)
> -#define TCPC_ROLE_CTRL_RP_VAL_SHIFT 4
> -#define TCPC_ROLE_CTRL_RP_VAL_MASK 0x3
> +#define TCPC_ROLE_CTRL_RP_VAL GENMASK(5, 4)
> #define TCPC_ROLE_CTRL_RP_VAL_DEF 0x0
> #define TCPC_ROLE_CTRL_RP_VAL_1_5 0x1
> #define TCPC_ROLE_CTRL_RP_VAL_3_0 0x2
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 06/15] usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV
2024-07-10 10:36 ` [PATCH 06/15] usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV André Draszik
@ 2024-07-31 10:39 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 10:39 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:13AM +0100, André Draszik wrote:
> Convert field TCPC_MSG_HDR_INFO_REV from register TCPC_MSG_HDR_INFO to
> using GENMASK() and FIELD_PREP() so as to keep using a similar approach
> for all fields.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 2 +-
> include/linux/usb/tcpci.h | 3 +--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 5ad05a5bbbd1..ad5c9d5bf6a9 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -456,7 +456,7 @@ static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
> unsigned int reg;
> int ret;
>
> - reg = PD_REV20 << TCPC_MSG_HDR_INFO_REV_SHIFT;
> + reg = FIELD_PREP(TCPC_MSG_HDR_INFO_REV, PD_REV20);
> if (role == TYPEC_SOURCE)
> reg |= TCPC_MSG_HDR_INFO_PWR_ROLE;
> if (data == TYPEC_HOST)
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 80652d4f722e..3cd61e9f73b3 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -129,9 +129,8 @@
>
> #define TCPC_MSG_HDR_INFO 0x2e
> #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> +#define TCPC_MSG_HDR_INFO_REV GENMASK(2, 1)
> #define TCPC_MSG_HDR_INFO_PWR_ROLE BIT(0)
> -#define TCPC_MSG_HDR_INFO_REV_SHIFT 1
> -#define TCPC_MSG_HDR_INFO_REV_MASK 0x3
>
> #define TCPC_RX_DETECT 0x2f
> #define TCPC_RX_DETECT_HARD_RESET BIT(5)
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 07/15] usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields
2024-07-10 10:36 ` [PATCH 07/15] usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields André Draszik
@ 2024-07-31 11:01 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 11:01 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:14AM +0100, André Draszik wrote:
> Convert all fields from register TCPC_TRANSMIT to using GENMASK() and
> FIELD_PREP() so as to keep using a similar approach throughout the code
> base and make it arguably easier to read.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 7 +++++--
> include/linux/usb/tcpci.h | 6 ++----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ad5c9d5bf6a9..b9ee9ccff99b 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -607,8 +607,11 @@ static int tcpci_pd_transmit(struct tcpc_dev *tcpc, enum tcpm_transmit_type type
> }
>
> /* nRetryCount is 3 in PD2.0 spec where 2 in PD3.0 spec */
> - reg = ((negotiated_rev > PD_REV20 ? PD_RETRY_COUNT_3_0_OR_HIGHER : PD_RETRY_COUNT_DEFAULT)
> - << TCPC_TRANSMIT_RETRY_SHIFT) | (type << TCPC_TRANSMIT_TYPE_SHIFT);
> + reg = FIELD_PREP(TCPC_TRANSMIT_RETRY,
> + (negotiated_rev > PD_REV20
> + ? PD_RETRY_COUNT_3_0_OR_HIGHER
> + : PD_RETRY_COUNT_DEFAULT));
> + reg |= FIELD_PREP(TCPC_TRANSMIT_TYPE, type);
> ret = regmap_write(tcpci->regmap, TCPC_TRANSMIT, reg);
> if (ret < 0)
> return ret;
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 3cd61e9f73b3..f7f5cfbdef12 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -148,10 +148,8 @@
> #define TCPC_RX_DATA 0x34 /* through 0x4f */
>
> #define TCPC_TRANSMIT 0x50
> -#define TCPC_TRANSMIT_RETRY_SHIFT 4
> -#define TCPC_TRANSMIT_RETRY_MASK 0x3
> -#define TCPC_TRANSMIT_TYPE_SHIFT 0
> -#define TCPC_TRANSMIT_TYPE_MASK 0x7
> +#define TCPC_TRANSMIT_RETRY GENMASK(5, 4)
> +#define TCPC_TRANSMIT_TYPE GENMASK(2, 0)
>
> #define TCPC_TX_BYTE_CNT 0x51
> #define TCPC_TX_HDR 0x52
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 08/15] usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit
2024-07-10 10:36 ` [PATCH 08/15] usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit André Draszik
@ 2024-07-31 11:09 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 11:09 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:15AM +0100, André Draszik wrote:
> This makes it easier to see what's missing and what's being programmed.
>
> While at it, add brackets to help with formatting and remove a comment
> that doesn't add much value.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index 87102b4d060d..ad9bb61fd9e0 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -97,11 +97,13 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
> if (ret < 0)
> return;
>
> - alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
> - TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
> - TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
> - /* Enable Extended alert for detecting Fast Role Swap Signal */
> - TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS | TCPC_ALERT_FAULT;
> + alert_mask = (TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED |
> + TCPC_ALERT_TX_FAILED | TCPC_ALERT_RX_HARD_RST |
> + TCPC_ALERT_RX_STATUS | TCPC_ALERT_POWER_STATUS |
> + TCPC_ALERT_CC_STATUS |
> + TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS |
> + TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF |
> + TCPC_ALERT_FAULT);
>
> ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
> if (ret < 0) {
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 09/15] usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF
2024-07-10 10:36 ` [PATCH 09/15] usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF André Draszik
@ 2024-07-31 11:12 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 11:12 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:16AM +0100, André Draszik wrote:
> There is no need for using the ternary if/else here, simply mask
> TCPC_ALERT_RX_BUF_OVF as necessary, which arguably makes the code
> easier to read.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index ad9bb61fd9e0..5b5441db7047 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -193,9 +193,8 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
> * Read complete, clear RX status alert bit.
> * Clear overflow as well if set.
> */
> - ret = max_tcpci_write16(chip, TCPC_ALERT, status & TCPC_ALERT_RX_BUF_OVF ?
> - TCPC_ALERT_RX_STATUS | TCPC_ALERT_RX_BUF_OVF :
> - TCPC_ALERT_RX_STATUS);
> + ret = max_tcpci_write16(chip, TCPC_ALERT,
> + TCPC_ALERT_RX_STATUS | (status & TCPC_ALERT_RX_BUF_OVF));
> if (ret < 0)
> return;
>
> @@ -297,9 +296,8 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
> * be cleared until we have successfully retrieved message.
> */
> if (status & ~TCPC_ALERT_RX_STATUS) {
> - mask = status & TCPC_ALERT_RX_BUF_OVF ?
> - status & ~(TCPC_ALERT_RX_STATUS | TCPC_ALERT_RX_BUF_OVF) :
> - status & ~TCPC_ALERT_RX_STATUS;
> + mask = status & ~(TCPC_ALERT_RX_STATUS
> + | (status & TCPC_ALERT_RX_BUF_OVF));
> ret = max_tcpci_write16(chip, TCPC_ALERT, mask);
> if (ret < 0) {
> dev_err(chip->dev, "ALERT clear failed\n");
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 10/15] usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK()
2024-07-10 10:36 ` [PATCH 10/15] usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK() André Draszik
@ 2024-07-31 12:29 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 12:29 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:17AM +0100, André Draszik wrote:
> Only one user of STATUS_CHECK() remains, and the code can actually be
> made more legible by simply avoiding the use of that wrapper macro,
> allowing to also drop the macro altogether.
>
> Do so.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/maxim_contaminant.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
> index e7687aeb69c0..8ac8eeade277 100644
> --- a/drivers/usb/typec/tcpm/maxim_contaminant.c
> +++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
> @@ -46,8 +46,6 @@ enum fladc_select {
> #define READ1_SLEEP_MS 10
> #define READ2_SLEEP_MS 5
>
> -#define STATUS_CHECK(reg, mask, val) (((reg) & (mask)) == (val))
> -
> #define IS_CC_OPEN(cc_status) \
> (FIELD_GET(TCPC_CC_STATUS_CC1, cc_status) == TCPC_CC_STATE_SRC_OPEN \
> && FIELD_GET(TCPC_CC_STATUS_CC2, cc_status) == TCPC_CC_STATE_SRC_OPEN)
> @@ -368,7 +366,7 @@ bool max_contaminant_is_contaminant(struct max_tcpci_chip *chip, bool disconnect
> }
> return false;
> } else if (chip->contaminant_state == DETECTED) {
> - if (STATUS_CHECK(cc_status, TCPC_CC_STATUS_TOGGLING, 0)) {
> + if (!(cc_status & TCPC_CC_STATUS_TOGGLING)) {
> chip->contaminant_state = max_contaminant_detect_contaminant(chip);
> if (chip->contaminant_state == DETECTED) {
> max_contaminant_enable_dry_detection(chip);
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 11/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register
2024-07-10 10:36 ` [PATCH 11/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register André Draszik
@ 2024-07-31 12:36 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 12:36 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:18AM +0100, André Draszik wrote:
> Convert register TCPC_VENDOR_CC_CTRL2 to using GENMASK() and
> FIELD_PREP() so as to keep using a similar approach throughout the code
> base and make it arguably easier to read.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/maxim_contaminant.c | 18 +++++++++++-------
> drivers/usb/typec/tcpm/tcpci_maxim.h | 6 +++---
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
> index 8ac8eeade277..f7acaa42329f 100644
> --- a/drivers/usb/typec/tcpm/maxim_contaminant.c
> +++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
> @@ -116,13 +116,14 @@ static int max_contaminant_read_resistance_kohm(struct max_tcpci_chip *chip,
> if (channel == CC1_SCALE1 || channel == CC2_SCALE1 || channel == CC1_SCALE2 ||
> channel == CC2_SCALE2) {
> /* Enable 1uA current source */
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL_MASK,
> - ULTRA_LOW_POWER_MODE);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL,
> + FIELD_PREP(CCLPMODESEL, ULTRA_LOW_POWER_MODE));
> if (ret < 0)
> return ret;
>
> /* Enable 1uA current source */
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL_MASK, UA_1_SRC);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL,
> + FIELD_PREP(CCRPCTRL, UA_1_SRC));
> if (ret < 0)
> return ret;
>
> @@ -176,7 +177,8 @@ static int max_contaminant_read_comparators(struct max_tcpci_chip *chip, u8 *ven
> int ret;
>
> /* Enable 80uA source */
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL_MASK, UA_80_SRC);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL,
> + FIELD_PREP(CCRPCTRL, UA_80_SRC));
> if (ret < 0)
> return ret;
>
> @@ -209,7 +211,8 @@ static int max_contaminant_read_comparators(struct max_tcpci_chip *chip, u8 *ven
> if (ret < 0)
> return ret;
>
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL_MASK, 0);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCRPCTRL,
> + FIELD_PREP(CCRPCTRL, 0));
> if (ret < 0)
> return ret;
>
> @@ -298,8 +301,9 @@ static int max_contaminant_enable_dry_detection(struct max_tcpci_chip *chip)
> if (ret < 0)
> return ret;
>
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL_MASK,
> - ULTRA_LOW_POWER_MODE);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL2, CCLPMODESEL,
> + FIELD_PREP(CCLPMODESEL,
> + ULTRA_LOW_POWER_MODE));
> if (ret < 0)
> return ret;
> ret = max_tcpci_read8(chip, TCPC_VENDOR_CC_CTRL2, &temp);
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
> index 78ff3b73ee7e..92c9a628ebe1 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim.h
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
> @@ -20,9 +20,9 @@
> #define SBUOVPDIS BIT(7)
> #define CCOVPDIS BIT(6)
> #define SBURPCTRL BIT(5)
> -#define CCLPMODESEL_MASK GENMASK(4, 3)
> -#define ULTRA_LOW_POWER_MODE BIT(3)
> -#define CCRPCTRL_MASK GENMASK(2, 0)
> +#define CCLPMODESEL GENMASK(4, 3)
> +#define ULTRA_LOW_POWER_MODE 1
> +#define CCRPCTRL GENMASK(2, 0)
> #define UA_1_SRC 1
> #define UA_80_SRC 3
>
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 12/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register
2024-07-10 10:36 ` [PATCH 12/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register André Draszik
@ 2024-07-31 12:38 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 12:38 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:19AM +0100, André Draszik wrote:
> Convert register TCPC_VENDOR_CC_CTRL3 to using GENMASK() so as to keep
> using a similar approach throughout the code base and make it arguably
> easier to read.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/maxim_contaminant.c | 9 +++++----
> drivers/usb/typec/tcpm/tcpci_maxim.h | 9 +++------
> 2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
> index f7acaa42329f..cf9887de96c9 100644
> --- a/drivers/usb/typec/tcpm/maxim_contaminant.c
> +++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
> @@ -283,10 +283,11 @@ static int max_contaminant_enable_dry_detection(struct max_tcpci_chip *chip)
> u8 temp;
> int ret;
>
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL3, CCWTRDEB_MASK | CCWTRSEL_MASK
> - | WTRCYCLE_MASK, CCWTRDEB_1MS << CCWTRDEB_SHIFT |
> - CCWTRSEL_1V << CCWTRSEL_SHIFT | WTRCYCLE_4_8_S <<
> - WTRCYCLE_SHIFT);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_CC_CTRL3,
> + CCWTRDEB | CCWTRSEL | WTRCYCLE,
> + FIELD_PREP(CCWTRDEB, CCWTRDEB_1MS)
> + | FIELD_PREP(CCWTRSEL, CCWTRSEL_1V)
> + | FIELD_PREP(WTRCYCLE, WTRCYCLE_4_8_S));
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
> index 92c9a628ebe1..34076069444f 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim.h
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
> @@ -27,15 +27,12 @@
> #define UA_80_SRC 3
>
> #define TCPC_VENDOR_CC_CTRL3 0x8e
> -#define CCWTRDEB_MASK GENMASK(7, 6)
> -#define CCWTRDEB_SHIFT 6
> +#define CCWTRDEB GENMASK(7, 6)
> #define CCWTRDEB_1MS 1
> -#define CCWTRSEL_MASK GENMASK(5, 3)
> -#define CCWTRSEL_SHIFT 3
> +#define CCWTRSEL GENMASK(5, 3)
> #define CCWTRSEL_1V 0x4
> #define CCLADDERDIS BIT(2)
> -#define WTRCYCLE_MASK BIT(0)
> -#define WTRCYCLE_SHIFT 0
> +#define WTRCYCLE GENMASK(0, 0)
> #define WTRCYCLE_2_4_S 0
> #define WTRCYCLE_4_8_S 1
>
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 13/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register
2024-07-10 10:36 ` [PATCH 13/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register André Draszik
@ 2024-07-31 12:40 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 12:40 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:20AM +0100, André Draszik wrote:
> Convert register TCPC_VENDOR_ADC_CTRL1 to using GENMASK() and
> FIELD_PREP() so as to keep using a similar approach throughout the code
> base and make it arguably easier to read.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/maxim_contaminant.c | 7 ++++---
> drivers/usb/typec/tcpm/tcpci_maxim.h | 3 +--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/maxim_contaminant.c b/drivers/usb/typec/tcpm/maxim_contaminant.c
> index cf9887de96c9..7bfec45e8f4f 100644
> --- a/drivers/usb/typec/tcpm/maxim_contaminant.c
> +++ b/drivers/usb/typec/tcpm/maxim_contaminant.c
> @@ -76,8 +76,8 @@ static int max_contaminant_read_adc_mv(struct max_tcpci_chip *chip, enum fladc_s
> int ret;
>
> /* Channel & scale select */
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL_MASK,
> - channel << ADC_CHANNEL_OFFSET);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL,
> + FIELD_PREP(ADCINSEL, channel));
> if (ret < 0)
> return ret;
>
> @@ -96,7 +96,8 @@ static int max_contaminant_read_adc_mv(struct max_tcpci_chip *chip, enum fladc_s
> if (ret < 0)
> return ret;
>
> - ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL_MASK, 0);
> + ret = regmap_update_bits(regmap, TCPC_VENDOR_ADC_CTRL1, ADCINSEL,
> + FIELD_PREP(ADCINSEL, 0));
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
> index 34076069444f..a41ca9e7ad08 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim.h
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
> @@ -37,8 +37,7 @@
> #define WTRCYCLE_4_8_S 1
>
> #define TCPC_VENDOR_ADC_CTRL1 0x91
> -#define ADCINSEL_MASK GENMASK(7, 5)
> -#define ADC_CHANNEL_OFFSET 5
> +#define ADCINSEL GENMASK(7, 5)
> #define ADCEN BIT(0)
>
> enum contamiant_state {
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 14/15] usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe()
2024-07-10 10:36 ` [PATCH 14/15] usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe() André Draszik
@ 2024-07-31 12:44 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 12:44 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:21AM +0100, André Draszik wrote:
> dev_err_probe() exists as a useful helper ensuring standardized
> error messages during .probe() and using it also helps to make
> the code more legible.
>
> Use it.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index 5b5441db7047..ee3e86797f17 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -484,17 +484,17 @@ static int max_tcpci_probe(struct i2c_client *client)
>
> chip->client = client;
> chip->data.regmap = devm_regmap_init_i2c(client, &max_tcpci_regmap_config);
> - if (IS_ERR(chip->data.regmap)) {
> - dev_err(&client->dev, "Regmap init failed\n");
> - return PTR_ERR(chip->data.regmap);
> - }
> + if (IS_ERR(chip->data.regmap))
> + return dev_err_probe(&client->dev, PTR_ERR(chip->data.regmap),
> + "Regmap init failed\n");
>
> chip->dev = &client->dev;
> i2c_set_clientdata(client, chip);
>
> ret = max_tcpci_read8(chip, TCPC_POWER_STATUS, &power_status);
> if (ret < 0)
> - return ret;
> + return dev_err_probe(&client->dev, ret,
> + "Failed to read TCPC_POWER_STATUS\n");
>
> /* Chip level tcpci callbacks */
> chip->data.set_vbus = max_tcpci_set_vbus;
> @@ -511,10 +511,10 @@ static int max_tcpci_probe(struct i2c_client *client)
>
> max_tcpci_init_regs(chip);
> chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
> - if (IS_ERR(chip->tcpci)) {
> - dev_err(&client->dev, "TCPCI port registration failed\n");
> - return PTR_ERR(chip->tcpci);
> - }
> + if (IS_ERR(chip->tcpci))
> + return dev_err_probe(&client->dev, PTR_ERR(chip->tcpci),
> + "TCPCI port registration failed\n");
> +
> chip->port = tcpci_get_tcpm_port(chip->tcpci);
> ret = max_tcpci_init_alert(chip, client);
> if (ret < 0)
> @@ -526,7 +526,8 @@ static int max_tcpci_probe(struct i2c_client *client)
> unreg_port:
> tcpci_unregister_port(chip->tcpci);
>
> - return ret;
> + return dev_err_probe(&client->dev, ret,
> + "Maxim TCPCI driver initialization failed\n");
> }
>
> static void max_tcpci_remove(struct i2c_client *client)
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 15/15] usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration
2024-07-10 10:36 ` [PATCH 15/15] usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration André Draszik
@ 2024-07-31 12:45 ` Heikki Krogerus
0 siblings, 0 replies; 32+ messages in thread
From: Heikki Krogerus @ 2024-07-31 12:45 UTC (permalink / raw)
To: André Draszik
Cc: Greg Kroah-Hartman, Peter Griffin, Tudor Ambarus, Will McVicker,
Badhri Jagan Sridharan, kernel-team, linux-usb, linux-kernel
On Wed, Jul 10, 2024 at 11:36:22AM +0100, André Draszik wrote:
> Instead of open-coding the call to tcpci_unregister_port() in various
> places, let's just register a hook using devm_add_action_or_reset() so
> that it gets called automatically as and when necessary by the device
> management APIs.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index ee3e86797f17..7abfd29b4b01 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -472,6 +472,11 @@ static bool max_tcpci_attempt_vconn_swap_discovery(struct tcpci *tcpci, struct t
> return true;
> }
>
> +static void max_tcpci_unregister_tcpci_port(void *tcpci)
> +{
> + tcpci_unregister_port(tcpci);
> +}
> +
> static int max_tcpci_probe(struct i2c_client *client)
> {
> int ret;
> @@ -515,27 +520,21 @@ static int max_tcpci_probe(struct i2c_client *client)
> return dev_err_probe(&client->dev, PTR_ERR(chip->tcpci),
> "TCPCI port registration failed\n");
>
> + ret = devm_add_action_or_reset(&client->dev,
> + max_tcpci_unregister_tcpci_port,
> + chip->tcpci);
> + if (ret)
> + return ret;
> +
> chip->port = tcpci_get_tcpm_port(chip->tcpci);
> +
> ret = max_tcpci_init_alert(chip, client);
> if (ret < 0)
> - goto unreg_port;
> + return dev_err_probe(&client->dev, ret,
> + "IRQ initialization failed\n");
>
> device_init_wakeup(chip->dev, true);
> return 0;
> -
> -unreg_port:
> - tcpci_unregister_port(chip->tcpci);
> -
> - return dev_err_probe(&client->dev, ret,
> - "Maxim TCPCI driver initialization failed\n");
> -}
> -
> -static void max_tcpci_remove(struct i2c_client *client)
> -{
> - struct max_tcpci_chip *chip = i2c_get_clientdata(client);
> -
> - if (!IS_ERR_OR_NULL(chip->tcpci))
> - tcpci_unregister_port(chip->tcpci);
> }
>
> static const struct i2c_device_id max_tcpci_id[] = {
> @@ -558,7 +557,6 @@ static struct i2c_driver max_tcpci_i2c_driver = {
> .of_match_table = of_match_ptr(max_tcpci_of_match),
> },
> .probe = max_tcpci_probe,
> - .remove = max_tcpci_remove,
> .id_table = max_tcpci_id,
> };
> module_i2c_driver(max_tcpci_i2c_driver);
>
> --
> 2.45.2.803.g4e1b14247a-goog
--
heikki
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2024-07-31 12:45 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-10 10:36 [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
2024-07-10 10:36 ` [PATCH 01/15] usb: typec: tcpci: fix a comment typo André Draszik
2024-07-31 9:47 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 02/15] usb: typec: tcpm/tcpci_maxim: clarify a comment André Draszik
2024-07-31 10:10 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 03/15] usb: typec: tcpci: use GENMASK() for TCPC_CC_STATUS_CC[12] André Draszik
2024-07-31 10:26 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 04/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_CC[12] André Draszik
2024-07-31 10:28 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 05/15] usb: typec: tcpci: use GENMASK() for TCPC_ROLE_CTRL_RP_VAL André Draszik
2024-07-31 10:37 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 06/15] usb: typec: tcpci: use GENMASK() for TCPC_MSG_HDR_INFO_REV André Draszik
2024-07-31 10:39 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 07/15] usb: typec: tcpci: use GENMASK() for TCPC_TRANSMIT register fields André Draszik
2024-07-31 11:01 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 08/15] usb: typec: tcpm/tcpci_maxim: sort TCPC_ALERT_MASK values by bit André Draszik
2024-07-31 11:09 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 09/15] usb: typec: tcpm/tcpci_maxim: simplify clearing of TCPC_ALERT_RX_BUF_OVF André Draszik
2024-07-31 11:12 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 10/15] usb: typec: tcpm/tcpci_maxim: drop STATUS_CHECK() André Draszik
2024-07-31 12:29 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 11/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL2 register André Draszik
2024-07-31 12:36 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 12/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_CC_CTRL3 register André Draszik
2024-07-31 12:38 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 13/15] usb: typec: tcpm/tcpci_maxim: use GENMASK() for TCPC_VENDOR_ADC_CTRL1 register André Draszik
2024-07-31 12:40 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 14/15] usb: typec: tcpm/tcpci_maxim: convert to dev_err_probe() André Draszik
2024-07-31 12:44 ` Heikki Krogerus
2024-07-10 10:36 ` [PATCH 15/15] usb: typec: tcpm/tcpci_maxim: use device managed TCPCI port deregistration André Draszik
2024-07-31 12:45 ` Heikki Krogerus
2024-07-26 5:59 ` [PATCH 00/15] usb: typec: tcpci: few TCPCi & TCPCi-Maxim cleanups (mostly genmask()) André Draszik
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).