* [PATCH v2 0/4] can: fsl,flexcan: add fsl,s32v234-flexcan and imx95 wakeup
@ 2024-07-15 21:27 Frank Li
2024-07-15 21:27 ` [PATCH v2 1/4] dt-bindings: can: fsl,flexcan: add compatible string fsl,s32v234-flexcan Frank Li
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Frank Li @ 2024-07-15 21:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu, Frank Li, Chircu-Mare Bogdan-Petru, Dan Nica,
Stefan-Gabriel Mirea, Li Yang, Joakim Zhang, Leonard Crestez
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
To: David S. Miller <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: haibo.chen@nxp.com
Cc: imx@lists.linux.dev
Cc: han.xu@nxp.com
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v2:
- fixed typo an rework commit message for patch 4
- Add rob's review tag
- Add Vincent's review tag
- Add const for fsl_imx95_devtype_data
- Link to v1: https://lore.kernel.org/r/20240711-flexcan-v1-0-d5210ec0a34b@nxp.com
---
Chircu-Mare Bogdan-Petru (1):
can: flexcan: Add S32V234 support to FlexCAN driver
Frank Li (1):
dt-bindings: can: fsl,flexcan: add compatible string fsl,s32v234-flexcan
Haibo Chen (2):
bingdings: can: flexcan: move fsl,imx95-flexcan standalone
can: flexcan: add wakeup support for imx95
.../devicetree/bindings/net/can/fsl,flexcan.yaml | 5 +-
drivers/net/can/flexcan/flexcan-core.c | 54 ++++++++++++++++++++--
drivers/net/can/flexcan/flexcan.h | 2 +
3 files changed, 53 insertions(+), 8 deletions(-)
---
base-commit: a4cf44c1ad6471737cf687b1f1644cf33641e738
change-id: 20240709-flexcan-c75cfef2acb9
Best regards,
---
Frank Li <Frank.Li@nxp.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/4] dt-bindings: can: fsl,flexcan: add compatible string fsl,s32v234-flexcan
2024-07-15 21:27 [PATCH v2 0/4] can: fsl,flexcan: add fsl,s32v234-flexcan and imx95 wakeup Frank Li
@ 2024-07-15 21:27 ` Frank Li
2024-07-15 21:27 ` [PATCH v2 2/4] can: flexcan: Add S32V234 support to FlexCAN driver Frank Li
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2024-07-15 21:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu, Frank Li
Add compatible string fsl,s32v234-flexcan for s32 chips.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
index f197d9b516bb2..b6c92684c5e29 100644
--- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
+++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
@@ -27,6 +27,7 @@ properties:
- fsl,vf610-flexcan
- fsl,ls1021ar2-flexcan
- fsl,lx2160ar1-flexcan
+ - fsl,s32v234-flexcan
- items:
- enum:
- fsl,imx53-flexcan
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] can: flexcan: Add S32V234 support to FlexCAN driver
2024-07-15 21:27 [PATCH v2 0/4] can: fsl,flexcan: add fsl,s32v234-flexcan and imx95 wakeup Frank Li
2024-07-15 21:27 ` [PATCH v2 1/4] dt-bindings: can: fsl,flexcan: add compatible string fsl,s32v234-flexcan Frank Li
@ 2024-07-15 21:27 ` Frank Li
2024-07-16 6:56 ` Marc Kleine-Budde
2024-07-15 21:27 ` [PATCH v2 3/4] bingdings: can: flexcan: move fsl,imx95-flexcan standalone Frank Li
2024-07-15 21:27 ` [PATCH v2 4/4] can: flexcan: add wakeup support for imx95 Frank Li
3 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2024-07-15 21:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu, Frank Li, Chircu-Mare Bogdan-Petru, Dan Nica,
Stefan-Gabriel Mirea, Li Yang, Joakim Zhang, Leonard Crestez
From: Chircu-Mare Bogdan-Petru <Bogdan.Chircu@freescale.com>
Add flexcan support for S32V234.
Signed-off-by: Chircu-Mare Bogdan-Petru <Bogdan.Chircu@freescale.com>
Signed-off-by: Dan Nica <dan.nica@nxp.com>
Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
Reviewed-by: Li Yang <leoyang.li@nxp.com>
Reviewed-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/net/can/flexcan/flexcan-core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index 8ea7f2795551b..f6e609c388d55 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -378,6 +378,10 @@ static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
};
+static struct flexcan_devtype_data fsl_s32v234_devtype_data = {
+ .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_DISABLE_MECR,
+};
+
static const struct can_bittiming_const flexcan_bittiming_const = {
.name = DRV_NAME,
.tseg1_min = 4,
@@ -2018,6 +2022,7 @@ static const struct of_device_id flexcan_of_match[] = {
{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, },
{ .compatible = "fsl,ls1021ar2-flexcan", .data = &fsl_ls1021a_r2_devtype_data, },
{ .compatible = "fsl,lx2160ar1-flexcan", .data = &fsl_lx2160a_r1_devtype_data, },
+ { .compatible = "fsl,s32v234-flexcan", .data = &fsl_s32v234_devtype_data, },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, flexcan_of_match);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] bingdings: can: flexcan: move fsl,imx95-flexcan standalone
2024-07-15 21:27 [PATCH v2 0/4] can: fsl,flexcan: add fsl,s32v234-flexcan and imx95 wakeup Frank Li
2024-07-15 21:27 ` [PATCH v2 1/4] dt-bindings: can: fsl,flexcan: add compatible string fsl,s32v234-flexcan Frank Li
2024-07-15 21:27 ` [PATCH v2 2/4] can: flexcan: Add S32V234 support to FlexCAN driver Frank Li
@ 2024-07-15 21:27 ` Frank Li
2024-07-15 21:27 ` [PATCH v2 4/4] can: flexcan: add wakeup support for imx95 Frank Li
3 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2024-07-15 21:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu, Frank Li
From: Haibo Chen <haibo.chen@nxp.com>
The flexcan in iMX95 is not compatible with imx93 because wakeup method is
difference. Make fsl,imx95-flexcan not fallback to fsl,imx93-flexcan.
Reviewed-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
index b6c92684c5e29..c08bd78e3367e 100644
--- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
+++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
@@ -17,6 +17,7 @@ properties:
compatible:
oneOf:
- enum:
+ - fsl,imx95-flexcan
- fsl,imx93-flexcan
- fsl,imx8qm-flexcan
- fsl,imx8mp-flexcan
@@ -39,9 +40,6 @@ properties:
- fsl,imx6ul-flexcan
- fsl,imx6sx-flexcan
- const: fsl,imx6q-flexcan
- - items:
- - const: fsl,imx95-flexcan
- - const: fsl,imx93-flexcan
- items:
- enum:
- fsl,ls1028ar1-flexcan
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
2024-07-15 21:27 [PATCH v2 0/4] can: fsl,flexcan: add fsl,s32v234-flexcan and imx95 wakeup Frank Li
` (2 preceding siblings ...)
2024-07-15 21:27 ` [PATCH v2 3/4] bingdings: can: flexcan: move fsl,imx95-flexcan standalone Frank Li
@ 2024-07-15 21:27 ` Frank Li
2024-07-16 7:06 ` Marc Kleine-Budde
3 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2024-07-15 21:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu, Frank Li
From: Haibo Chen <haibo.chen@nxp.com>
iMX95 defines a bit in GPR that sets/unsets the IPG_STOP signal to the
FlexCAN module, controlling its entry into STOP mode. Wakeup should work
even if FlexCAN is in STOP mode.
Due to iMX95 architecture design, the A-Core cannot access GPR; only the
system manager (SM) can configure GPR. To support the wakeup feature,
follow these steps:
- For suspend:
1) During Linux suspend, when CAN suspends, do nothing for GPR and keep
CAN-related clocks on.
2) In ATF, check whether CAN needs to support wakeup; if yes, send a
request to SM through the SCMI protocol.
3) In SM, configure the GPR and unset IPG_STOP.
4) A-Core suspends.
- For wakeup and resume:
1) A-Core wakeup event arrives.
2) In SM, deassert IPG_STOP.
3) Linux resumes.
Add a new fsl_imx95_devtype_data and FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI to
reflect this.
Reviewed-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
drivers/net/can/flexcan/flexcan-core.c | 49 ++++++++++++++++++++++++++++++----
drivers/net/can/flexcan/flexcan.h | 2 ++
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index f6e609c388d55..fe972d5b8fbe0 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -354,6 +354,14 @@ static struct flexcan_devtype_data fsl_imx93_devtype_data = {
FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
};
+static const struct flexcan_devtype_data fsl_imx95_devtype_data = {
+ .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+ FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI |
+ FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC |
+ FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
+ FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
+};
static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
@@ -548,6 +556,13 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
} else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+ } else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI) {
+ /* For the SCMI mode, driver do nothing, ATF will send request to
+ * SM(system manager, M33 core) through SCMI protocol after linux
+ * suspend. Once SM get this request, it will send IPG_STOP signal
+ * to Flex_CAN, let CAN in STOP mode.
+ */
+ return 0;
}
return flexcan_low_power_enter_ack(priv);
@@ -559,7 +574,11 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
u32 reg_mcr;
int ret;
- /* remove stop request */
+ /* Remove stop request, for FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI,
+ * do nothing here, because ATF already send request to SM before
+ * linux resume. Once SM get this request, it will deassert the
+ * IPG_STOP signal to Flex_CAN.
+ */
if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
ret = flexcan_stop_mode_enable_scfw(priv, false);
if (ret < 0)
@@ -1987,6 +2006,9 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
ret = flexcan_setup_stop_mode_scfw(pdev);
else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR)
ret = flexcan_setup_stop_mode_gpr(pdev);
+ else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)
+ /* ATF will handle all STOP_IPG related work */
+ ret = 0;
else
/* return 0 directly if doesn't support stop mode feature */
return 0;
@@ -2013,6 +2035,7 @@ static const struct of_device_id flexcan_of_match[] = {
{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
{ .compatible = "fsl,imx93-flexcan", .data = &fsl_imx93_devtype_data, },
+ { .compatible = "fsl,imx95-flexcan", .data = &fsl_imx95_devtype_data, },
{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, },
@@ -2311,9 +2334,22 @@ static int __maybe_unused flexcan_noirq_suspend(struct device *device)
if (netif_running(dev)) {
int err;
- if (device_may_wakeup(device))
+ if (device_may_wakeup(device)) {
flexcan_enable_wakeup_irq(priv, true);
+ /* For FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI, it need
+ * ATF to send request to SM through SCMI protocol,
+ * SM will assert the IPG_STOP signal. But all this
+ * works need the CAN clocks keep on.
+ * After the CAN module get the IPG_STOP mode, and
+ * switch to STOP mode, whether still keep the CAN
+ * clocks on or gate them off depend on the Hardware
+ * design.
+ */
+ if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)
+ return 0;
+ }
+
err = pm_runtime_force_suspend(device);
if (err)
return err;
@@ -2330,9 +2366,12 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
if (netif_running(dev)) {
int err;
- err = pm_runtime_force_resume(device);
- if (err)
- return err;
+ if (!(device_may_wakeup(device) &&
+ priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)) {
+ err = pm_runtime_force_resume(device);
+ if (err)
+ return err;
+ }
if (device_may_wakeup(device))
flexcan_enable_wakeup_irq(priv, false);
diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
index 025c3417031f4..4933d8c7439e6 100644
--- a/drivers/net/can/flexcan/flexcan.h
+++ b/drivers/net/can/flexcan/flexcan.h
@@ -68,6 +68,8 @@
#define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
/* Device supports RX via FIFO */
#define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
+/* Setup stop mode with ATF SCMI protocol to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(17)
struct flexcan_devtype_data {
u32 quirks; /* quirks needed for different IP cores */
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] can: flexcan: Add S32V234 support to FlexCAN driver
2024-07-15 21:27 ` [PATCH v2 2/4] can: flexcan: Add S32V234 support to FlexCAN driver Frank Li
@ 2024-07-16 6:56 ` Marc Kleine-Budde
0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2024-07-16 6:56 UTC (permalink / raw)
To: Frank Li
Cc: Vincent Mailhol, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu, Chircu-Mare Bogdan-Petru, Dan Nica, Stefan-Gabriel Mirea,
Li Yang, Joakim Zhang, Leonard Crestez
[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]
On 15.07.2024 17:27:21, Frank Li wrote:
> From: Chircu-Mare Bogdan-Petru <Bogdan.Chircu@freescale.com>
>
> Add flexcan support for S32V234.
>
> Signed-off-by: Chircu-Mare Bogdan-Petru <Bogdan.Chircu@freescale.com>
> Signed-off-by: Dan Nica <dan.nica@nxp.com>
> Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
> Reviewed-by: Li Yang <leoyang.li@nxp.com>
> Reviewed-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/net/can/flexcan/flexcan-core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 8ea7f2795551b..f6e609c388d55 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -378,6 +378,10 @@ static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
> FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
> };
>
> +static struct flexcan_devtype_data fsl_s32v234_devtype_data = {
> + .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_DISABLE_MECR,
> +};
- Does the core support CAN-FD?
- Does the core generate an error interrupt when going from error warning
to error passive?
- Are you sure, you don't need to enable FLEXCAN_QUIRK_ENABLE_EACEN_RRS?
- You probably want to use the mailboxes instead of the FIFO for the
RX-path.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
2024-07-15 21:27 ` [PATCH v2 4/4] can: flexcan: add wakeup support for imx95 Frank Li
@ 2024-07-16 7:06 ` Marc Kleine-Budde
2024-07-16 14:40 ` Frank Li
0 siblings, 1 reply; 11+ messages in thread
From: Marc Kleine-Budde @ 2024-07-16 7:06 UTC (permalink / raw)
To: Frank Li
Cc: Vincent Mailhol, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu
[-- Attachment #1: Type: text/plain, Size: 7524 bytes --]
On 15.07.2024 17:27:23, Frank Li wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
>
> iMX95 defines a bit in GPR that sets/unsets the IPG_STOP signal to the
> FlexCAN module, controlling its entry into STOP mode. Wakeup should work
> even if FlexCAN is in STOP mode.
>
> Due to iMX95 architecture design, the A-Core cannot access GPR; only the
> system manager (SM) can configure GPR. To support the wakeup feature,
> follow these steps:
>
> - For suspend:
> 1) During Linux suspend, when CAN suspends, do nothing for GPR and keep
> CAN-related clocks on.
> 2) In ATF, check whether CAN needs to support wakeup; if yes, send a
> request to SM through the SCMI protocol.
> 3) In SM, configure the GPR and unset IPG_STOP.
> 4) A-Core suspends.
>
> - For wakeup and resume:
> 1) A-Core wakeup event arrives.
> 2) In SM, deassert IPG_STOP.
> 3) Linux resumes.
>
> Add a new fsl_imx95_devtype_data and FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI to
> reflect this.
>
> Reviewed-by: Han Xu <han.xu@nxp.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/net/can/flexcan/flexcan-core.c | 49 ++++++++++++++++++++++++++++++----
> drivers/net/can/flexcan/flexcan.h | 2 ++
> 2 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f6e609c388d55..fe972d5b8fbe0 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -354,6 +354,14 @@ static struct flexcan_devtype_data fsl_imx93_devtype_data = {
> FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
> };
>
> +static const struct flexcan_devtype_data fsl_imx95_devtype_data = {
> + .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> + FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
> + FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI |
> + FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC |
> + FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
> + FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
Please keep the flags sorted by their value.
> +};
Please add a newline here.
> static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
> .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
> @@ -548,6 +556,13 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
> } else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
> regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
> + } else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI) {
> + /* For the SCMI mode, driver do nothing, ATF will send request to
> + * SM(system manager, M33 core) through SCMI protocol after linux
> + * suspend. Once SM get this request, it will send IPG_STOP signal
> + * to Flex_CAN, let CAN in STOP mode.
> + */
> + return 0;
> }
>
> return flexcan_low_power_enter_ack(priv);
> @@ -559,7 +574,11 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
> u32 reg_mcr;
> int ret;
>
> - /* remove stop request */
> + /* Remove stop request, for FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI,
> + * do nothing here, because ATF already send request to SM before
> + * linux resume. Once SM get this request, it will deassert the
> + * IPG_STOP signal to Flex_CAN.
> + */
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
> ret = flexcan_stop_mode_enable_scfw(priv, false);
> if (ret < 0)
> @@ -1987,6 +2006,9 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
> ret = flexcan_setup_stop_mode_scfw(pdev);
> else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR)
> ret = flexcan_setup_stop_mode_gpr(pdev);
> + else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)
> + /* ATF will handle all STOP_IPG related work */
> + ret = 0;
> else
> /* return 0 directly if doesn't support stop mode feature */
> return 0;
> @@ -2013,6 +2035,7 @@ static const struct of_device_id flexcan_of_match[] = {
> { .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
> { .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
> { .compatible = "fsl,imx93-flexcan", .data = &fsl_imx93_devtype_data, },
> + { .compatible = "fsl,imx95-flexcan", .data = &fsl_imx95_devtype_data, },
> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
> { .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, },
> @@ -2311,9 +2334,22 @@ static int __maybe_unused flexcan_noirq_suspend(struct device *device)
> if (netif_running(dev)) {
> int err;
>
> - if (device_may_wakeup(device))
> + if (device_may_wakeup(device)) {
> flexcan_enable_wakeup_irq(priv, true);
>
> + /* For FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI, it need
needs
> + * ATF to send request to SM through SCMI protocol,
> + * SM will assert the IPG_STOP signal. But all this
> + * works need the CAN clocks keep on.
> + * After the CAN module get the IPG_STOP mode, and
gets
> + * switch to STOP mode, whether still keep the CAN
switches
> + * clocks on or gate them off depend on the Hardware
> + * design.
> + */
> + if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)
> + return 0;
> + }
> +
> err = pm_runtime_force_suspend(device);
> if (err)
> return err;
> @@ -2330,9 +2366,12 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
> if (netif_running(dev)) {
> int err;
>
> - err = pm_runtime_force_resume(device);
> - if (err)
> - return err;
> + if (!(device_may_wakeup(device) &&
^^^^^^^^^^^^^^^^^^^^^^^^
Where does this come from?
> + priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)) {
> + err = pm_runtime_force_resume(device);
> + if (err)
> + return err;
> + }
>
> if (device_may_wakeup(device))
> flexcan_enable_wakeup_irq(priv, false);
> diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
> index 025c3417031f4..4933d8c7439e6 100644
> --- a/drivers/net/can/flexcan/flexcan.h
> +++ b/drivers/net/can/flexcan/flexcan.h
> @@ -68,6 +68,8 @@
> #define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
> /* Device supports RX via FIFO */
> #define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
> +/* Setup stop mode with ATF SCMI protocol to support wakeup */
> +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(17)
>
> struct flexcan_devtype_data {
> u32 quirks; /* quirks needed for different IP cores */
>
> --
> 2.34.1
>
>
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
2024-07-16 7:06 ` Marc Kleine-Budde
@ 2024-07-16 14:40 ` Frank Li
2024-07-16 14:45 ` Marc Kleine-Budde
0 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2024-07-16 14:40 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Vincent Mailhol, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu
On Tue, Jul 16, 2024 at 09:06:14AM +0200, Marc Kleine-Budde wrote:
> On 15.07.2024 17:27:23, Frank Li wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > iMX95 defines a bit in GPR that sets/unsets the IPG_STOP signal to the
> > FlexCAN module, controlling its entry into STOP mode. Wakeup should work
> > even if FlexCAN is in STOP mode.
> >
> > Due to iMX95 architecture design, the A-Core cannot access GPR; only the
> > system manager (SM) can configure GPR. To support the wakeup feature,
> > follow these steps:
> >
> > - For suspend:
> > 1) During Linux suspend, when CAN suspends, do nothing for GPR and keep
> > CAN-related clocks on.
> > 2) In ATF, check whether CAN needs to support wakeup; if yes, send a
> > request to SM through the SCMI protocol.
> > 3) In SM, configure the GPR and unset IPG_STOP.
> > 4) A-Core suspends.
> >
> > - For wakeup and resume:
> > 1) A-Core wakeup event arrives.
> > 2) In SM, deassert IPG_STOP.
> > 3) Linux resumes.
> >
> > Add a new fsl_imx95_devtype_data and FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI to
> > reflect this.
> >
> > Reviewed-by: Han Xu <han.xu@nxp.com>
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > drivers/net/can/flexcan/flexcan-core.c | 49 ++++++++++++++++++++++++++++++----
> > drivers/net/can/flexcan/flexcan.h | 2 ++
> > 2 files changed, 46 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> > index f6e609c388d55..fe972d5b8fbe0 100644
> > --- a/drivers/net/can/flexcan/flexcan-core.c
> > +++ b/drivers/net/can/flexcan/flexcan-core.c
> > @@ -354,6 +354,14 @@ static struct flexcan_devtype_data fsl_imx93_devtype_data = {
> > FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
> > };
> >
> > +static const struct flexcan_devtype_data fsl_imx95_devtype_data = {
> > + .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > + FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
> > + FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI |
> > + FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC |
> > + FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
> > + FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
>
> Please keep the flags sorted by their value.
>
> > +};
>
> Please add a newline here.
>
> > static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
> > .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
> > @@ -548,6 +556,13 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
> > } else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
> > regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> > 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
> > + } else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI) {
> > + /* For the SCMI mode, driver do nothing, ATF will send request to
> > + * SM(system manager, M33 core) through SCMI protocol after linux
> > + * suspend. Once SM get this request, it will send IPG_STOP signal
> > + * to Flex_CAN, let CAN in STOP mode.
> > + */
> > + return 0;
> > }
> >
> > return flexcan_low_power_enter_ack(priv);
> > @@ -559,7 +574,11 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
> > u32 reg_mcr;
> > int ret;
> >
> > - /* remove stop request */
> > + /* Remove stop request, for FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI,
> > + * do nothing here, because ATF already send request to SM before
> > + * linux resume. Once SM get this request, it will deassert the
> > + * IPG_STOP signal to Flex_CAN.
> > + */
> > if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
> > ret = flexcan_stop_mode_enable_scfw(priv, false);
> > if (ret < 0)
> > @@ -1987,6 +2006,9 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
> > ret = flexcan_setup_stop_mode_scfw(pdev);
> > else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR)
> > ret = flexcan_setup_stop_mode_gpr(pdev);
> > + else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)
> > + /* ATF will handle all STOP_IPG related work */
> > + ret = 0;
> > else
> > /* return 0 directly if doesn't support stop mode feature */
> > return 0;
> > @@ -2013,6 +2035,7 @@ static const struct of_device_id flexcan_of_match[] = {
> > { .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
> > { .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
> > { .compatible = "fsl,imx93-flexcan", .data = &fsl_imx93_devtype_data, },
> > + { .compatible = "fsl,imx95-flexcan", .data = &fsl_imx95_devtype_data, },
> > { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
> > { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
> > { .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, },
> > @@ -2311,9 +2334,22 @@ static int __maybe_unused flexcan_noirq_suspend(struct device *device)
> > if (netif_running(dev)) {
> > int err;
> >
> > - if (device_may_wakeup(device))
> > + if (device_may_wakeup(device)) {
> > flexcan_enable_wakeup_irq(priv, true);
> >
> > + /* For FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI, it need
> needs
> > + * ATF to send request to SM through SCMI protocol,
> > + * SM will assert the IPG_STOP signal. But all this
> > + * works need the CAN clocks keep on.
> > + * After the CAN module get the IPG_STOP mode, and
> gets
> > + * switch to STOP mode, whether still keep the CAN
> switches
> > + * clocks on or gate them off depend on the Hardware
> > + * design.
> > + */
> > + if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)
> > + return 0;
> > + }
> > +
> > err = pm_runtime_force_suspend(device);
> > if (err)
> > return err;
> > @@ -2330,9 +2366,12 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
> > if (netif_running(dev)) {
> > int err;
> >
> > - err = pm_runtime_force_resume(device);
> > - if (err)
> > - return err;
> > + if (!(device_may_wakeup(device) &&
> ^^^^^^^^^^^^^^^^^^^^^^^^
>
> Where does this come from?
include/linux/pm_wakeup.h
static inline bool device_may_wakeup(struct device *dev)
{
return dev->power.can_wakeup && !!dev->power.wakeup;
}
Frank
>
> > + priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)) {
> > + err = pm_runtime_force_resume(device);
> > + if (err)
> > + return err;
> > + }
> >
> > if (device_may_wakeup(device))
> > flexcan_enable_wakeup_irq(priv, false);
> > diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
> > index 025c3417031f4..4933d8c7439e6 100644
> > --- a/drivers/net/can/flexcan/flexcan.h
> > +++ b/drivers/net/can/flexcan/flexcan.h
> > @@ -68,6 +68,8 @@
> > #define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
> > /* Device supports RX via FIFO */
> > #define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
> > +/* Setup stop mode with ATF SCMI protocol to support wakeup */
> > +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(17)
> >
> > struct flexcan_devtype_data {
> > u32 quirks; /* quirks needed for different IP cores */
> >
> > --
> > 2.34.1
> >
> >
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung Nürnberg | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
2024-07-16 14:40 ` Frank Li
@ 2024-07-16 14:45 ` Marc Kleine-Budde
2024-07-17 2:03 ` Bough Chen
0 siblings, 1 reply; 11+ messages in thread
From: Marc Kleine-Budde @ 2024-07-16 14:45 UTC (permalink / raw)
To: Frank Li
Cc: Vincent Mailhol, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-can, netdev, devicetree, linux-kernel, haibo.chen, imx,
han.xu
[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]
On 16.07.2024 10:40:31, Frank Li wrote:
> > > @@ -2330,9 +2366,12 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
> > > if (netif_running(dev)) {
> > > int err;
> > >
> > > - err = pm_runtime_force_resume(device);
> > > - if (err)
> > > - return err;
> > > + if (!(device_may_wakeup(device) &&
> > ^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > Where does this come from?
>
> include/linux/pm_wakeup.h
>
> static inline bool device_may_wakeup(struct device *dev)
> {
> return dev->power.can_wakeup && !!dev->power.wakeup;
> }
Sorry for the confusion. I wanted to point out, that the original driver
doesn't have the check to device_may_wakeup(). Why was this added?
> >
> > > + priv->devtype_data.quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)) {
> > > + err = pm_runtime_force_resume(device);
> > > + if (err)
> > > + return err;
> > > + }
> > >
> > > if (device_may_wakeup(device))
> > > flexcan_enable_wakeup_irq(priv, false);
> > > diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
> > > index 025c3417031f4..4933d8c7439e6 100644
> > > --- a/drivers/net/can/flexcan/flexcan.h
> > > +++ b/drivers/net/can/flexcan/flexcan.h
> > > @@ -68,6 +68,8 @@
> > > #define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
> > > /* Device supports RX via FIFO */
> > > #define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
> > > +/* Setup stop mode with ATF SCMI protocol to support wakeup */
> > > +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(17)
> > >
> > > struct flexcan_devtype_data {
> > > u32 quirks; /* quirks needed for different IP cores */
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
2024-07-16 14:45 ` Marc Kleine-Budde
@ 2024-07-17 2:03 ` Bough Chen
2024-07-17 7:20 ` Marc Kleine-Budde
0 siblings, 1 reply; 11+ messages in thread
From: Bough Chen @ 2024-07-17 2:03 UTC (permalink / raw)
To: Marc Kleine-Budde, Frank Li
Cc: Vincent Mailhol, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev, Han Xu
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2024年7月16日 22:46
> To: Frank Li <frank.li@nxp.com>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; linux-can@vger.kernel.org; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Bough Chen
> <haibo.chen@nxp.com>; imx@lists.linux.dev; Han Xu <han.xu@nxp.com>
> Subject: Re: [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
>
> On 16.07.2024 10:40:31, Frank Li wrote:
> > > > @@ -2330,9 +2366,12 @@ static int __maybe_unused
> flexcan_noirq_resume(struct device *device)
> > > > if (netif_running(dev)) {
> > > > int err;
> > > >
> > > > - err = pm_runtime_force_resume(device);
> > > > - if (err)
> > > > - return err;
> > > > + if (!(device_may_wakeup(device) &&
> > > ^^^^^^^^^^^^^^^^^^^^^^^^
> > >
> > > Where does this come from?
> >
> > include/linux/pm_wakeup.h
> >
> > static inline bool device_may_wakeup(struct device *dev)
> > {
> > return dev->power.can_wakeup && !!dev->power.wakeup;
> > }
>
> Sorry for the confusion. I wanted to point out, that the original driver doesn't
> have the check to device_may_wakeup(). Why was this added?
Hi Marc,
Here add this to make sure for CAN with flag FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI and really used as wakeup source, do not need to call pm_runtime_force_resume(), keep it align with
what we do in flexcan_noirq_suspend.
As the comment in flexcan_noirq_suspend, CAN with flag FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI, when used as wakeup source, need to keep CAN clock on when system suspend, let ATF part logic works, detail steps please refer to this patch commit log. Whether gate off the CAN clock or not depends on the Hardware design. So for this case, in flexcan_noirq_suspend, directly return0, do not call the pm_runtime_force_suspend(), then in flexcan_noirq_resume(), use the same logic to skip the pm_runtime_force_resume().
Best Regards
Haibo Chen
>
> > >
> > > > + priv->devtype_data.quirks &
> FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI)) {
> > > > + err = pm_runtime_force_resume(device);
> > > > + if (err)
> > > > + return err;
> > > > + }
> > > >
> > > > if (device_may_wakeup(device))
> > > > flexcan_enable_wakeup_irq(priv, false); diff --git
> > > > a/drivers/net/can/flexcan/flexcan.h
> > > > b/drivers/net/can/flexcan/flexcan.h
> > > > index 025c3417031f4..4933d8c7439e6 100644
> > > > --- a/drivers/net/can/flexcan/flexcan.h
> > > > +++ b/drivers/net/can/flexcan/flexcan.h
> > > > @@ -68,6 +68,8 @@
> > > > #define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
> > > > /* Device supports RX via FIFO */ #define
> > > > FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
> > > > +/* Setup stop mode with ATF SCMI protocol to support wakeup */
> > > > +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(17)
> > > >
> > > > struct flexcan_devtype_data {
> > > > u32 quirks; /* quirks needed for different IP cores */
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung Nürnberg | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RE: [PATCH v2 4/4] can: flexcan: add wakeup support for imx95
2024-07-17 2:03 ` Bough Chen
@ 2024-07-17 7:20 ` Marc Kleine-Budde
0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2024-07-17 7:20 UTC (permalink / raw)
To: Bough Chen
Cc: Frank Li, Vincent Mailhol, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev, Han Xu
[-- Attachment #1: Type: text/plain, Size: 2044 bytes --]
On 17.07.2024 02:03:35, Bough Chen wrote:
> > On 16.07.2024 10:40:31, Frank Li wrote:
> > > > > @@ -2330,9 +2366,12 @@ static int __maybe_unused
> > flexcan_noirq_resume(struct device *device)
> > > > > if (netif_running(dev)) {
> > > > > int err;
> > > > >
> > > > > - err = pm_runtime_force_resume(device);
> > > > > - if (err)
> > > > > - return err;
> > > > > + if (!(device_may_wakeup(device) &&
> > > > ^^^^^^^^^^^^^^^^^^^^^^^^
> > > >
> > > > Where does this come from?
> > >
> > > include/linux/pm_wakeup.h
> > >
> > > static inline bool device_may_wakeup(struct device *dev)
> > > {
> > > return dev->power.can_wakeup && !!dev->power.wakeup;
> > > }
> >
> > Sorry for the confusion. I wanted to point out, that the original driver doesn't
> > have the check to device_may_wakeup(). Why was this added?
>
> Here add this to make sure for CAN with flag
> FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI and really used as wakeup source,
> do not need to call pm_runtime_force_resume(), keep it align with what
> we do in flexcan_noirq_suspend.
> As the comment in flexcan_noirq_suspend, CAN with flag
> FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI, when used as wakeup source, need
> to keep CAN clock on when system suspend, let ATF part logic works,
> detail steps please refer to this patch commit log. Whether gate off
> the CAN clock or not depends on the Hardware design. So for this case,
> in flexcan_noirq_suspend, directly return0, do not call the
> pm_runtime_force_suspend(), then in flexcan_noirq_resume(), use the
> same logic to skip the pm_runtime_force_resume().
Please change the control flow, so that flexcan_noirq_suspend() and
flexcan_noirq_resume() look symmetrical.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-07-17 7:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-15 21:27 [PATCH v2 0/4] can: fsl,flexcan: add fsl,s32v234-flexcan and imx95 wakeup Frank Li
2024-07-15 21:27 ` [PATCH v2 1/4] dt-bindings: can: fsl,flexcan: add compatible string fsl,s32v234-flexcan Frank Li
2024-07-15 21:27 ` [PATCH v2 2/4] can: flexcan: Add S32V234 support to FlexCAN driver Frank Li
2024-07-16 6:56 ` Marc Kleine-Budde
2024-07-15 21:27 ` [PATCH v2 3/4] bingdings: can: flexcan: move fsl,imx95-flexcan standalone Frank Li
2024-07-15 21:27 ` [PATCH v2 4/4] can: flexcan: add wakeup support for imx95 Frank Li
2024-07-16 7:06 ` Marc Kleine-Budde
2024-07-16 14:40 ` Frank Li
2024-07-16 14:45 ` Marc Kleine-Budde
2024-07-17 2:03 ` Bough Chen
2024-07-17 7:20 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).