* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.