[PATCH] phy: phy-can-transceiver: Add suspend operation for tcan1043

[PATCH] phy: phy-can-transceiver: Add suspend operation for tcan1043

[Thomas Richard (TI)]

Add suspend operation for tcan1043. It switches the PHY in Sleep mode, the
lowest power mode of the device. If a bus wake-up pattern or a local
wake-up event occurs, the PHY transitions to Standby mode, set its internal
WAKERQ flag and set the INH output high. In Sleep mode INH is floating.

The WAKERQ flag prevents transition to Go-to-Sleep mode. The only way to
clear it is to switch to Normal mode. So to reach Sleep mode, we firstly
switch to Normal mode, then to Go-to-Sleep mode.

Suspend sequence (PHY is off):

    Standby -> Normal -> Go-to-Sleep -> Sleep

Suspend sequence (PHY is on):

    Normal -> Go-to-Sleep -> Sleep

Signed-off-by: Thomas Richard (TI) <thomas.richard@bootlin.com>
---
 drivers/phy/phy-can-transceiver.c | 56 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
index 75dc49e75ca0..2bca1a173fcc 100644
--- a/drivers/phy/phy-can-transceiver.c
+++ b/drivers/phy/phy-can-transceiver.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com
  *
  */
+#include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -12,25 +13,28 @@
 #include <linux/module.h>
 #include <linux/mux/consumer.h>
 
+struct can_transceiver_phy {
+	struct phy *generic_phy;
+	struct gpio_desc *silent_gpio;
+	struct gpio_desc *standby_gpio;
+	struct gpio_desc *enable_gpio;
+	struct can_transceiver_priv *priv;
+};
+
 struct can_transceiver_data {
 	u32 flags;
 #define CAN_TRANSCEIVER_STB_PRESENT	BIT(0)
 #define CAN_TRANSCEIVER_EN_PRESENT	BIT(1)
 #define CAN_TRANSCEIVER_DUAL_CH		BIT(2)
 #define CAN_TRANSCEIVER_SILENT_PRESENT	BIT(3)
+	int	(*suspend)(struct can_transceiver_phy *phy);
 };
 
-struct can_transceiver_phy {
-	struct phy *generic_phy;
-	struct gpio_desc *silent_gpio;
-	struct gpio_desc *standby_gpio;
-	struct gpio_desc *enable_gpio;
-	struct can_transceiver_priv *priv;
-};
 
 struct can_transceiver_priv {
 	struct mux_state *mux_state;
 	int num_ch;
+	const struct can_transceiver_data *data;
 	struct can_transceiver_phy can_transceiver_phy[] __counted_by(num_ch);
 };
 
@@ -76,12 +80,28 @@ static const struct phy_ops can_transceiver_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static int tcan1043_suspend(struct can_transceiver_phy *phy)
+{
+	/* Switch to Normal mode, it clears WAKERQ */
+	gpiod_set_value_cansleep(phy->standby_gpio, 0);
+	gpiod_set_value_cansleep(phy->enable_gpio, 1);
+
+	/* Switch to Go-to-Sleep mode */
+	gpiod_set_value_cansleep(phy->standby_gpio, 1);
+
+	/* Wait transition to Sleep mode */
+	fsleep(5);
+
+	return 0;
+}
+
 static const struct can_transceiver_data tcan1042_drvdata = {
 	.flags = CAN_TRANSCEIVER_STB_PRESENT,
 };
 
 static const struct can_transceiver_data tcan1043_drvdata = {
 	.flags = CAN_TRANSCEIVER_STB_PRESENT | CAN_TRANSCEIVER_EN_PRESENT,
+	.suspend = tcan1043_suspend,
 };
 
 static const struct can_transceiver_data tja1048_drvdata = {
@@ -115,6 +135,26 @@ static struct phy *can_transceiver_phy_xlate(struct device *dev,
 	return priv->can_transceiver_phy[idx].generic_phy;
 }
 
+static int can_transceiver_phy_suspend(struct device *dev)
+{
+	struct can_transceiver_priv *priv = dev_get_drvdata(dev);
+	const struct can_transceiver_data *data = priv->data;
+	int ret, i;
+
+	for (i = 0; i < priv->num_ch; i++) {
+		if (data->suspend) {
+			ret = data->suspend(&priv->can_transceiver_phy[i]);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(can_transceiver_phy_pm_ops,
+				can_transceiver_phy_suspend, NULL);
+
 static int can_transceiver_phy_probe(struct platform_device *pdev)
 {
 	struct phy_provider *phy_provider;
@@ -145,6 +185,7 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv->num_ch = num_ch;
+	priv->data = drvdata;
 	platform_set_drvdata(pdev, priv);
 
 	mux_state = devm_mux_state_get_optional(dev, NULL);
@@ -247,6 +288,7 @@ static struct platform_driver can_transceiver_phy_driver = {
 	.driver = {
 		.name = "can-transceiver-phy",
 		.of_match_table = can_transceiver_phy_ids,
+		.pm = pm_sleep_ptr(&can_transceiver_phy_pm_ops),
 	},
 };
 

---
base-commit: a25d586f7f9e7f22e7e0350053c15924d28ed6ee
change-id: 20260618-phy-can-transceiver-suspend-resume-support-de9f32852fb8

Best regards,
-- 
Thomas Richard (TI) <thomas.richard@bootlin.com>

Re: [PATCH] phy: phy-can-transceiver: Add suspend operation for tcan1043

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `.resume` callback in PM operations leaves the CAN PHY permanently in Sleep mode after system resume.
- [Medium] Missing delay when transitioning through Normal mode may result in the transition being ignored by the transceiver hardware.
--

commit 2d2e1a772ce8aac2faea27b0135abc2a7ed70072
Author: Thomas Richard (TI) <thomas.richard@bootlin.com>

phy: phy-can-transceiver: Add suspend operation for tcan1043

This commit adds a suspend operation for the tcan1043 PHY, switching it
to Sleep mode during system suspend. It transitions the PHY through Normal
mode to clear the WAKERQ flag before entering Go-to-Sleep and finally
Sleep mode.

> diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
> index 75dc49e75ca0e..2bca1a173fcc3 100644
> --- a/drivers/phy/phy-can-transceiver.c
> +++ b/drivers/phy/phy-can-transceiver.c
[ ... ]
> @@ -76,12 +80,28 @@ static const struct phy_ops can_transceiver_phy_ops = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +static int tcan1043_suspend(struct can_transceiver_phy *phy)
> +{
> +	/* Switch to Normal mode, it clears WAKERQ */
> +	gpiod_set_value_cansleep(phy->standby_gpio, 0);
> +	gpiod_set_value_cansleep(phy->enable_gpio, 1);
> +
> +	/* Switch to Go-to-Sleep mode */
> +	gpiod_set_value_cansleep(phy->standby_gpio, 1);

[Severity: Medium]
Will this back-to-back GPIO toggle execute too quickly on platforms with fast
memory-mapped GPIOs?

Without an intermediate delay, the sub-microsecond state change might not
give the transceiver's internal state machine enough time to register the
Normal mode transition. Can this cause the transceiver to ignore the
transition, fail to clear the WAKERQ flag, and thus fail to enter Sleep
mode?

> +
> +	/* Wait transition to Sleep mode */
> +	fsleep(5);
> +
> +	return 0;
> +}
[ ... ]
> @@ -115,6 +135,26 @@ static struct phy *can_transceiver_phy_xlate(struct device *dev,
>  	return priv->can_transceiver_phy[idx].generic_phy;
>  }
>  
> +static int can_transceiver_phy_suspend(struct device *dev)
> +{
> +	struct can_transceiver_priv *priv = dev_get_drvdata(dev);
> +	const struct can_transceiver_data *data = priv->data;
> +	int ret, i;
> +
> +	for (i = 0; i < priv->num_ch; i++) {
> +		if (data->suspend) {
> +			ret = data->suspend(&priv->can_transceiver_phy[i]);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(can_transceiver_phy_pm_ops,
> +				can_transceiver_phy_suspend, NULL);

[Severity: High]
Does leaving the resume callback as NULL permanently leave the CAN PHY in
Sleep mode after a system resume?

Since the parent CAN MAC driver typically relies on the PHY framework for
power management and may not manually wake the PHY during its own resume
flow, could this prevent the CAN interface from becoming responsive again
until it is manually brought down and back up?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-phy-can-transceiver-suspend-resume-support-v1-1-6842c2d697e4@bootlin.com?part=1