[PATCH 0/2] can: mcp251xfd: Enable support for PLL

[PATCH 0/2] can: mcp251xfd: Enable support for PLL

[Magnus Aagaard Sørensen]

This patch set enables support for the internal PLL in MCP251xFD.
Tested on a RPi 4 and a MCP2518FD with an external 4 MHz oscillator.

The first part has been tested with the PLL sanity check disabled.

This is my first attempt at contributing to Linux, so let me know of any errors in format and/or style.

Magnus Aagaard Sørensen (2):
  Preparation for support of internal PLL in mcp251xfd. Moves all calls
    which would alter the clock setup, apart from probe, into PM calls.
  Enable support for internal PLL in mcp251xfd. The PLL is enabled if
    the configured clock is less than or equal to 10 times the max clock
    frequency.

 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 174 +++++++++++++-----
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     |   3 +
 2 files changed, 126 insertions(+), 51 deletions(-)

-- 
2.20.1

[PATCH 1/2] Preparation for support of internal PLL in mcp251xfd.

[Magnus Aagaard Sørensen]

Moves all calls which would alter the clock setup, apart from probe, into PM calls.

---
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 91 ++++++++++---------
 1 file changed, 48 insertions(+), 43 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index c3f49543f..3c440f9c8 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -519,25 +519,22 @@ static inline bool mcp251xfd_osc_invalid(u32 reg)
 	return reg == 0x0 || reg == 0xffffffff;
 }
 
-static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv)
+static int mcp251xfd_chip_wake(const struct mcp251xfd_priv *priv)
 {
 	u32 osc, osc_reference, osc_mask;
 	int err;
 
-	/* Set Power On Defaults for "Clock Output Divisor" and remove
-	 * "Oscillator Disable" bit.
+	/* For normal sleep in MCP2517FD and MCP2518FD, clearing
+	 * "Oscillator Disable" will wake it. For low power mode in
+	 * MCP2518DF, asserting the chip will wake it. Writing to
+	 * the Oscillator register will wake it in both cases.
 	 */
+
 	osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK,
 			 MCP251XFD_REG_OSC_CLKODIV_10);
 	osc_reference = MCP251XFD_REG_OSC_OSCRDY;
-	osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY;
+	osc_mask = MCP251XFD_REG_OSC_OSCRDY;
 
-	/* Note:
-	 *
-	 * If the controller is in Sleep Mode the following write only
-	 * removes the "Oscillator Disable" bit and powers it up. All
-	 * other bits are unaffected.
-	 */
 	err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc);
 	if (err)
 		return err;
@@ -569,13 +566,6 @@ static int mcp251xfd_chip_softreset_do(const struct mcp251xfd_priv *priv)
 	const __be16 cmd = mcp251xfd_cmd_reset();
 	int err;
 
-	/* The Set Mode and SPI Reset command only seems to works if
-	 * the controller is not in Sleep Mode.
-	 */
-	err = mcp251xfd_chip_clock_enable(priv);
-	if (err)
-		return err;
-
 	err = mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_CONFIG);
 	if (err)
 		return err;
@@ -652,20 +642,6 @@ static int mcp251xfd_chip_softreset(const struct mcp251xfd_priv *priv)
 
 static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv)
 {
-	u32 osc;
-	int err;
-
-	/* Activate Low Power Mode on Oscillator Disable. This only
-	 * works on the MCP2518FD. The MCP2517FD will go into normal
-	 * Sleep Mode instead.
-	 */
-	osc = MCP251XFD_REG_OSC_LPMEN |
-		FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK,
-			   MCP251XFD_REG_OSC_CLKODIV_10);
-	err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc);
-	if (err)
-		return err;
-
 	/* Set Time Base Counter Prescaler to 1.
 	 *
 	 * This means an overflow of the 32 bit Time Base Counter
@@ -1020,17 +996,13 @@ static int mcp251xfd_chip_stop(struct mcp251xfd_priv *priv,
 
 	mcp251xfd_chip_interrupts_disable(priv);
 	mcp251xfd_chip_rx_int_disable(priv);
-	return mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP);
+	return mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_CONFIG);
 }
 
 static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv)
 {
 	int err;
 
-	err = mcp251xfd_chip_softreset(priv);
-	if (err)
-		goto out_chip_stop;
-
 	err = mcp251xfd_chip_clock_init(priv);
 	if (err)
 		goto out_chip_stop;
@@ -2660,12 +2632,9 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)
 
 	/* Put controller into sleep mode and let pm_runtime_put()
 	 * disable the clocks and vdd. If CONFIG_PM is not enabled,
-	 * the clocks and vdd will stay powered.
+	 * the clocks and vdd will stay powered, and the chip will
+	 * not enter sleep.
 	 */
-	err = mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP);
-	if (err)
-		goto out_unregister_candev;
-
 	pm_runtime_put(ndev->dev.parent);
 
 	return 0;
@@ -2893,16 +2862,52 @@ static int mcp251xfd_remove(struct spi_device *spi)
 
 static int __maybe_unused mcp251xfd_runtime_suspend(struct device *device)
 {
+	u32 osc;
+	int err;
 	const struct mcp251xfd_priv *priv = dev_get_drvdata(device);
 
-	return mcp251xfd_clks_and_vdd_disable(priv);
+	/* Activate Low Power Mode on Oscillator Disable. This only
+	 * works on the MCP2518FD. The MCP2517FD will go into normal
+	 * Sleep Mode instead.
+	 */
+	err = regmap_read(priv->map_reg, MCP251XFD_REG_OSC, &osc);
+	if (err)
+		return err;
+
+	osc |= MCP251XFD_REG_OSC_LPMEN;
+	err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc);
+	if (err)
+		return err;
+
+	err = mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP);
+	if (err)
+		return err;
+
+	err = mcp251xfd_clks_and_vdd_disable(priv);
+	if (err)
+		return err;
+
+	return err;
 }
 
 static int __maybe_unused mcp251xfd_runtime_resume(struct device *device)
 {
+	int err;
 	const struct mcp251xfd_priv *priv = dev_get_drvdata(device);
 
-	return mcp251xfd_clks_and_vdd_enable(priv);
+	err = mcp251xfd_clks_and_vdd_enable(priv);
+	if (err)
+		return err;
+
+	err = mcp251xfd_chip_wake(priv);
+	if (err)
+		return err;
+
+	err = mcp251xfd_chip_softreset(priv);
+	if (err)
+		return err;
+
+	return err;
 }
 
 static const struct dev_pm_ops mcp251xfd_pm_ops = {
-- 
2.20.1

[PATCH 2/2] Enable support for internal PLL in mcp251xfd.

[Magnus Aagaard Sørensen]

The PLL is enabled if the configured clock is less than or equal to 10 times the max clock frequency.

The device will operate with two different SPI speeds. A slow speed determined by the clock without the PLL enabled, and a fast speed derived from the frequency with the PLL enabled.
---
 .../net/can/spi/mcp251xfd/mcp251xfd-core.c    | 89 ++++++++++++++++---
 drivers/net/can/spi/mcp251xfd/mcp251xfd.h     |  3 +
 2 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 3c440f9c8..2436eaed2 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -561,6 +561,49 @@ static int mcp251xfd_chip_wake(const struct mcp251xfd_priv *priv)
 	return 0;
 }
 
+static int mcp251xfd_chip_setup_clock(const struct mcp251xfd_priv *priv)
+{
+	u32 osc, osc_reference, osc_mask;
+	int err;
+
+	if (priv->pll_enabled == false) {
+		return 0;
+	}
+
+	err = regmap_read(priv->map_reg, MCP251XFD_REG_OSC, &osc);
+	if (err)
+		return err;
+
+	osc |= MCP251XFD_REG_OSC_PLLEN;
+	osc_reference = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY;
+	osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY;
+
+	err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc);
+	if (err)
+		return err;
+
+	/* Wait for "Oscillator Ready" and "PLL Ready" bit */
+	err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_OSC, osc,
+				       (osc & osc_mask) == osc_reference,
+				       MCP251XFD_OSC_STAB_SLEEP_US,
+				       MCP251XFD_OSC_STAB_TIMEOUT_US);
+	if (mcp251xfd_osc_invalid(osc)) {
+		netdev_err(priv->ndev,
+			   "Failed to detect %s (osc=0x%08x).\n",
+			   mcp251xfd_get_model_str(priv), osc);
+		return -ENODEV;
+	} else if (err == -ETIMEDOUT) {
+		netdev_err(priv->ndev,
+			   "Timeout waiting for Oscillator Ready (osc=0x%08x, osc_reference=0x%08x)\n",
+			   osc, osc_reference);
+		return -ETIMEDOUT;
+	} else if (err) {
+		return err;
+	}
+
+	return 0;
+}
+
 static int mcp251xfd_chip_softreset_do(const struct mcp251xfd_priv *priv)
 {
 	const __be16 cmd = mcp251xfd_cmd_reset();
@@ -2568,11 +2611,12 @@ mcp251xfd_register_done(const struct mcp251xfd_priv *priv)
 		return err;
 
 	netdev_info(priv->ndev,
-		    "%s rev%lu.%lu (%cRX_INT %cMAB_NO_WARN %cCRC_REG %cCRC_RX %cCRC_TX %cECC %cHD c:%u.%02uMHz m:%u.%02uMHz r:%u.%02uMHz e:%u.%02uMHz) successfully initialized.\n",
+		    "%s rev%lu.%lu (%cRX_INT %cPLL %cMAB_NO_WARN %cCRC_REG %cCRC_RX %cCRC_TX %cECC %cHD c:%u.%02uMHz m:%u.%02uMHz rs:%u.%02uMHz rf:%u.%02uMHz e:%u.%02uMHz) successfully initialized.\n",
 		    mcp251xfd_get_model_str(priv),
 		    FIELD_GET(MCP251XFD_REG_DEVID_ID_MASK, dev_id),
 		    FIELD_GET(MCP251XFD_REG_DEVID_REV_MASK, dev_id),
 		    priv->rx_int ? '+' : '-',
+		    priv->pll_enabled ? '+' : '-',
 		    MCP251XFD_QUIRK_ACTIVE(MAB_NO_WARN),
 		    MCP251XFD_QUIRK_ACTIVE(CRC_REG),
 		    MCP251XFD_QUIRK_ACTIVE(CRC_RX),
@@ -2583,8 +2627,10 @@ mcp251xfd_register_done(const struct mcp251xfd_priv *priv)
 		    priv->can.clock.freq % 1000000 / 1000 / 10,
 		    priv->spi_max_speed_hz_orig / 1000000,
 		    priv->spi_max_speed_hz_orig % 1000000 / 1000 / 10,
-		    priv->spi->max_speed_hz / 1000000,
-		    priv->spi->max_speed_hz % 1000000 / 1000 / 10,
+		    priv->spi_max_speed_hz_slow / 1000000,
+		    priv->spi_max_speed_hz_slow % 1000000 / 1000 / 10,
+		    priv->spi_max_speed_hz_fast / 1000000,
+		    priv->spi_max_speed_hz_fast % 1000000 / 1000 / 10,
 		    effective_speed_hz / 1000000,
 		    effective_speed_hz % 1000000 / 1000 / 10);
 
@@ -2614,6 +2660,12 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv)
 	if (err)
 		goto out_chip_set_mode_sleep;
 
+	err = mcp251xfd_chip_setup_clock(priv);
+	if (err)
+		return err;
+
+	priv->spi->max_speed_hz = priv->spi_max_speed_hz_fast;
+
 	err = mcp251xfd_register_chip_detect(priv);
 	if (err)
 		goto out_chip_set_mode_sleep;
@@ -2706,6 +2758,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	struct clk *clk;
 	u32 freq;
 	int err;
+	bool pll_enabled = false;
 
 	rx_int = devm_gpiod_get_optional(&spi->dev, "microchip,rx-int",
 					 GPIOD_IN);
@@ -2747,10 +2800,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	}
 
 	if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) {
-		dev_err(&spi->dev,
-			"Oscillator frequency (%u Hz) is too low and PLL is not supported.\n",
-			freq);
-		return -ERANGE;
+		pll_enabled = true;
 	}
 
 	ndev = alloc_candev(sizeof(struct mcp251xfd_priv),
@@ -2766,7 +2816,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
 
 	priv = netdev_priv(ndev);
 	spi_set_drvdata(spi, priv);
-	priv->can.clock.freq = freq;
+	priv->can.clock.freq = pll_enabled ? freq * MCP251XFD_OSC_PLL_MULTIPLIER : freq;
 	priv->can.do_set_mode = mcp251xfd_set_mode;
 	priv->can.do_get_berr_counter = mcp251xfd_get_berr_counter;
 	priv->can.bittiming_const = &mcp251xfd_bittiming_const;
@@ -2778,6 +2828,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	priv->spi = spi;
 	priv->rx_int = rx_int;
 	priv->clk = clk;
+	priv->pll_enabled = pll_enabled;
 	priv->reg_vdd = reg_vdd;
 	priv->reg_xceiver = reg_xceiver;
 
@@ -2817,7 +2868,15 @@ static int mcp251xfd_probe(struct spi_device *spi)
 	 *
 	 */
 	priv->spi_max_speed_hz_orig = spi->max_speed_hz;
-	spi->max_speed_hz = min(spi->max_speed_hz, freq / 2 / 1000 * 850);
+
+	priv->spi_max_speed_hz_slow = min(spi->max_speed_hz, freq / 2 / 1000 * 850);
+	if (priv->pll_enabled == true) {
+		priv->spi_max_speed_hz_fast = min(spi->max_speed_hz, freq * MCP251XFD_OSC_PLL_MULTIPLIER / 2 / 1000 * 850);
+	} else {
+		priv->spi_max_speed_hz_fast = priv->spi_max_speed_hz_slow;
+	}
+
+	spi->max_speed_hz = priv->spi_max_speed_hz_slow;
 	spi->bits_per_word = 8;
 	spi->rt = true;
 	err = spi_setup(spi);
@@ -2866,6 +2925,8 @@ static int __maybe_unused mcp251xfd_runtime_suspend(struct device *device)
 	int err;
 	const struct mcp251xfd_priv *priv = dev_get_drvdata(device);
 
+	priv->spi->max_speed_hz = priv->spi_max_speed_hz_slow;
+
 	/* Activate Low Power Mode on Oscillator Disable. This only
 	 * works on the MCP2518FD. The MCP2517FD will go into normal
 	 * Sleep Mode instead.
@@ -2887,7 +2948,7 @@ static int __maybe_unused mcp251xfd_runtime_suspend(struct device *device)
 	if (err)
 		return err;
 
-	return err;
+	return 0;
 }
 
 static int __maybe_unused mcp251xfd_runtime_resume(struct device *device)
@@ -2907,7 +2968,13 @@ static int __maybe_unused mcp251xfd_runtime_resume(struct device *device)
 	if (err)
 		return err;
 
-	return err;
+	err = mcp251xfd_chip_setup_clock(priv);
+	if (err)
+		return err;
+
+	priv->spi->max_speed_hz = priv->spi_max_speed_hz_fast;
+
+	return 0;
 }
 
 static const struct dev_pm_ops mcp251xfd_pm_ops = {
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
index fa1246e39..fc1a3ba5a 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h
@@ -579,6 +579,8 @@ struct mcp251xfd_priv {
 
 	struct spi_device *spi;
 	u32 spi_max_speed_hz_orig;
+	u32 spi_max_speed_hz_fast;
+	u32 spi_max_speed_hz_slow;
 
 	struct mcp251xfd_tef_ring tef;
 	struct mcp251xfd_tx_ring tx[1];
@@ -591,6 +593,7 @@ struct mcp251xfd_priv {
 
 	struct gpio_desc *rx_int;
 	struct clk *clk;
+	bool pll_enabled;
 	struct regulator *reg_vdd;
 	struct regulator *reg_xceiver;
 
-- 
2.20.1