* [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write()
@ 2023-09-14 12:09 Xu Yang
2023-09-14 12:09 ` [PATCH 2/2] usb: typec: tcpci: enable vSafe0v Detection and Auto Discharge Disconnect for PTN5110 Xu Yang
0 siblings, 1 reply; 4+ messages in thread
From: Xu Yang @ 2023-09-14 12:09 UTC (permalink / raw)
To: linux, heikki.krogerus; +Cc: linux-usb, gregkh, jun.li
The return value from tcpci/regmap_read/write() must be checked to get
rid of the bad influence of other modules. This will add check code for
all of the rest read/write() callbacks and will show error when failed
to get ALERT register.
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- remove printing code
---
drivers/usb/typec/tcpm/tcpci.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 0ee3e6e29bb1..8ccc2d1a8ffc 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -657,21 +657,28 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
int ret;
unsigned int raw;
- tcpci_read16(tcpci, TCPC_ALERT, &status);
+ ret = tcpci_read16(tcpci, TCPC_ALERT, &status);
+ if (ret < 0)
+ return ret;
/*
* Clear alert status for everything except RX_STATUS, which shouldn't
* be cleared until we have successfully retrieved message.
*/
- if (status & ~TCPC_ALERT_RX_STATUS)
- tcpci_write16(tcpci, TCPC_ALERT,
+ if (status & ~TCPC_ALERT_RX_STATUS) {
+ ret = tcpci_write16(tcpci, TCPC_ALERT,
status & ~TCPC_ALERT_RX_STATUS);
+ if (ret < 0)
+ return ret;
+ }
if (status & TCPC_ALERT_CC_STATUS)
tcpm_cc_change(tcpci->port);
if (status & TCPC_ALERT_POWER_STATUS) {
- regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ if (ret < 0)
+ return ret;
/*
* If power status mask has been reset, then the TCPC
* has reset.
@@ -687,7 +694,9 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
unsigned int cnt, payload_cnt;
u16 header;
- regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
+ ret = regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
+ if (ret < 0)
+ return ret;
/*
* 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
* of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
@@ -699,18 +708,25 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
else
payload_cnt = 0;
- tcpci_read16(tcpci, TCPC_RX_HDR, &header);
+ ret = tcpci_read16(tcpci, TCPC_RX_HDR, &header);
+ if (ret < 0)
+ return ret;
msg.header = cpu_to_le16(header);
if (WARN_ON(payload_cnt > sizeof(msg.payload)))
payload_cnt = sizeof(msg.payload);
- if (payload_cnt > 0)
- regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
+ if (payload_cnt > 0) {
+ ret = regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
&msg.payload, payload_cnt);
+ if (ret < 0)
+ return ret;
+ }
/* Read complete, clear RX status alert bit */
- tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
+ ret = tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
+ if (ret < 0)
+ return ret;
tcpm_pd_receive(tcpci->port, &msg);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] usb: typec: tcpci: enable vSafe0v Detection and Auto Discharge Disconnect for PTN5110
2023-09-14 12:09 [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write() Xu Yang
@ 2023-09-14 12:09 ` Xu Yang
0 siblings, 0 replies; 4+ messages in thread
From: Xu Yang @ 2023-09-14 12:09 UTC (permalink / raw)
To: linux, heikki.krogerus; +Cc: linux-usb, gregkh, jun.li
PTN5110 itself supports vSafe0V detection and auto discharge disconnect
capabilities. This will enable these feature.
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- no changes
---
drivers/usb/typec/tcpm/tcpci.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 8ccc2d1a8ffc..141d313b9a55 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -861,6 +861,9 @@ static int tcpci_probe(struct i2c_client *client)
i2c_set_clientdata(client, chip);
+ chip->data.vbus_vsafe0v = 1;
+ chip->data.auto_discharge_disconnect = 1;
+
/* Disable chip interrupts before requesting irq */
err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
sizeof(u16));
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write()
@ 2023-08-28 9:30 Xu Yang
2023-08-28 9:49 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Xu Yang @ 2023-08-28 9:30 UTC (permalink / raw)
To: linux, heikki.krogerus; +Cc: linux-usb, gregkh, jun.li
The return value from tcpci/regmap_read/write() must be checked to get
rid of the bad influence of other modules. This will add check code for
all of the rest read/write() callbacks and will show error when failed
to get ALERT register.
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
drivers/usb/typec/tcpm/tcpci.c | 36 +++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 0ee3e6e29bb1..698d00b7fce9 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -657,21 +657,30 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
int ret;
unsigned int raw;
- tcpci_read16(tcpci, TCPC_ALERT, &status);
+ ret = tcpci_read16(tcpci, TCPC_ALERT, &status);
+ if (ret < 0) {
+ dev_err(tcpci->dev, "ALERT read failed\n");
+ return ret;
+ }
/*
* Clear alert status for everything except RX_STATUS, which shouldn't
* be cleared until we have successfully retrieved message.
*/
- if (status & ~TCPC_ALERT_RX_STATUS)
- tcpci_write16(tcpci, TCPC_ALERT,
+ if (status & ~TCPC_ALERT_RX_STATUS) {
+ ret = tcpci_write16(tcpci, TCPC_ALERT,
status & ~TCPC_ALERT_RX_STATUS);
+ if (ret < 0)
+ return ret;
+ }
if (status & TCPC_ALERT_CC_STATUS)
tcpm_cc_change(tcpci->port);
if (status & TCPC_ALERT_POWER_STATUS) {
- regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+ if (ret < 0)
+ return ret;
/*
* If power status mask has been reset, then the TCPC
* has reset.
@@ -687,7 +696,9 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
unsigned int cnt, payload_cnt;
u16 header;
- regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
+ ret = regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
+ if (ret < 0)
+ return ret;
/*
* 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
* of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
@@ -699,18 +710,25 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
else
payload_cnt = 0;
- tcpci_read16(tcpci, TCPC_RX_HDR, &header);
+ ret = tcpci_read16(tcpci, TCPC_RX_HDR, &header);
+ if (ret < 0)
+ return ret;
msg.header = cpu_to_le16(header);
if (WARN_ON(payload_cnt > sizeof(msg.payload)))
payload_cnt = sizeof(msg.payload);
- if (payload_cnt > 0)
- regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
+ if (payload_cnt > 0) {
+ ret = regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
&msg.payload, payload_cnt);
+ if (ret < 0)
+ return ret;
+ }
/* Read complete, clear RX status alert bit */
- tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
+ ret = tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
+ if (ret < 0)
+ return ret;
tcpm_pd_receive(tcpci->port, &msg);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write()
2023-08-28 9:30 [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write() Xu Yang
@ 2023-08-28 9:49 ` Greg KH
0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2023-08-28 9:49 UTC (permalink / raw)
To: Xu Yang; +Cc: linux, heikki.krogerus, linux-usb, jun.li
On Mon, Aug 28, 2023 at 05:30:34PM +0800, Xu Yang wrote:
> The return value from tcpci/regmap_read/write() must be checked to get
> rid of the bad influence of other modules.
What do you mean by "other modules" here?
> This will add check code for
> all of the rest read/write() callbacks and will show error when failed
> to get ALERT register.
>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 36 +++++++++++++++++++++++++---------
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 0ee3e6e29bb1..698d00b7fce9 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -657,21 +657,30 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> int ret;
> unsigned int raw;
>
> - tcpci_read16(tcpci, TCPC_ALERT, &status);
> + ret = tcpci_read16(tcpci, TCPC_ALERT, &status);
> + if (ret < 0) {
> + dev_err(tcpci->dev, "ALERT read failed\n");
You are printing something in an irq handler, are you sure you want to
do that? What happens if this is very noisy?
> + return ret;
> + }
>
> /*
> * Clear alert status for everything except RX_STATUS, which shouldn't
> * be cleared until we have successfully retrieved message.
> */
> - if (status & ~TCPC_ALERT_RX_STATUS)
> - tcpci_write16(tcpci, TCPC_ALERT,
> + if (status & ~TCPC_ALERT_RX_STATUS) {
> + ret = tcpci_write16(tcpci, TCPC_ALERT,
> status & ~TCPC_ALERT_RX_STATUS);
> + if (ret < 0)
> + return ret;
> + }
>
> if (status & TCPC_ALERT_CC_STATUS)
> tcpm_cc_change(tcpci->port);
>
> if (status & TCPC_ALERT_POWER_STATUS) {
> - regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
> + ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
> + if (ret < 0)
> + return ret;
> /*
> * If power status mask has been reset, then the TCPC
> * has reset.
> @@ -687,7 +696,9 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> unsigned int cnt, payload_cnt;
> u16 header;
>
> - regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> + ret = regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> + if (ret < 0)
> + return ret;
> /*
> * 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
> * of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
> @@ -699,18 +710,25 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> else
> payload_cnt = 0;
>
> - tcpci_read16(tcpci, TCPC_RX_HDR, &header);
> + ret = tcpci_read16(tcpci, TCPC_RX_HDR, &header);
> + if (ret < 0)
> + return ret;
> msg.header = cpu_to_le16(header);
>
> if (WARN_ON(payload_cnt > sizeof(msg.payload)))
> payload_cnt = sizeof(msg.payload);
>
> - if (payload_cnt > 0)
> - regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
> + if (payload_cnt > 0) {
> + ret = regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
> &msg.payload, payload_cnt);
> + if (ret < 0)
> + return ret;
> + }
>
> /* Read complete, clear RX status alert bit */
> - tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
> + ret = tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
> + if (ret < 0)
> + return ret;
For all of these, if an error happens, you are not able to unwind from
the previous actions that the irq handler took.
Are you sure this is ok? How was this tested?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-09-14 12:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-14 12:09 [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write() Xu Yang
2023-09-14 12:09 ` [PATCH 2/2] usb: typec: tcpci: enable vSafe0v Detection and Auto Discharge Disconnect for PTN5110 Xu Yang
-- strict thread matches above, loose matches on Subject: below --
2023-08-28 9:30 [PATCH 1/2] usb: typec: tcpci: add check code for tcpci/regmap_read/write() Xu Yang
2023-08-28 9:49 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox