* [PATCH v2 0/3] can: mcp251xfd: PLL support
@ 2020-10-16 20:52 Marc Kleine-Budde
2020-10-16 20:52 ` [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() Marc Kleine-Budde
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Marc Kleine-Budde @ 2020-10-16 20:52 UTC (permalink / raw)
To: linux-can; +Cc: Magnus Aagaard Sørensen
Hello,
I've looked at Magnus Aagaard Sørensen's patches and decided the PLL idea looks
quite good, but the runtime pm change should better made after adding PLL support.
This series focuses on adding PLL support only. Consider this as an RFC, the
patches need proper description.
I've tested the setup here with two mcp2518fd, where one supplies the 4MHz for
the other and it works for me.
Testing welcome.
regards,
Marc
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() 2020-10-16 20:52 [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde @ 2020-10-16 20:52 ` Marc Kleine-Budde 2020-10-19 5:23 ` Magnus Aagaard Sørensen 2020-10-26 9:50 ` Thomas.Kopp 2020-10-16 20:52 ` [PATCH v2 2/3] can: mcp251xfd: mcp251xfd_chip_timestamp_init(): Marc Kleine-Budde ` (2 subsequent siblings) 3 siblings, 2 replies; 9+ messages in thread From: Marc Kleine-Budde @ 2020-10-16 20:52 UTC (permalink / raw) To: linux-can; +Cc: Magnus Aagaard Sørensen, Marc Kleine-Budde Co-developed-by: Magnus Aagaard Sørensen <mas@csselectronics.com> Not-Singed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index c3f49543ff26..c36f5f14d50c 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -519,13 +519,16 @@ 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 on MCP2517FD and MCP2518FD, clearing + * "Oscillator Disable" will wake the chip. For low power mode + * on MCP2518FD, asserting the chip select will wake the + * chip. Writing to the Oscillator register will wake it in + * both cases. */ osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, MCP251XFD_REG_OSC_CLKODIV_10); @@ -569,10 +572,10 @@ 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. + /* The Set Mode and SPI Reset command only works if the + * controller is not in Sleep Mode. */ - err = mcp251xfd_chip_clock_enable(priv); + err = mcp251xfd_chip_wake(priv); if (err) return err; -- 2.28.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() 2020-10-16 20:52 ` [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() Marc Kleine-Budde @ 2020-10-19 5:23 ` Magnus Aagaard Sørensen 2020-10-26 9:50 ` Thomas.Kopp 1 sibling, 0 replies; 9+ messages in thread From: Magnus Aagaard Sørensen @ 2020-10-19 5:23 UTC (permalink / raw) To: linux-can; +Cc: Marc Kleine-Budde Signed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> On 16-10-2020 22:52, Marc Kleine-Budde wrote: > Co-developed-by: Magnus Aagaard Sørensen <mas@csselectronics.com> > Not-Singed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > index c3f49543ff26..c36f5f14d50c 100644 > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > @@ -519,13 +519,16 @@ 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 on MCP2517FD and MCP2518FD, clearing > + * "Oscillator Disable" will wake the chip. For low power mode > + * on MCP2518FD, asserting the chip select will wake the > + * chip. Writing to the Oscillator register will wake it in > + * both cases. > */ > osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, > MCP251XFD_REG_OSC_CLKODIV_10); > @@ -569,10 +572,10 @@ 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. > + /* The Set Mode and SPI Reset command only works if the > + * controller is not in Sleep Mode. > */ > - err = mcp251xfd_chip_clock_enable(priv); > + err = mcp251xfd_chip_wake(priv); > if (err) > return err; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() 2020-10-16 20:52 ` [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() Marc Kleine-Budde 2020-10-19 5:23 ` Magnus Aagaard Sørensen @ 2020-10-26 9:50 ` Thomas.Kopp 2020-10-26 10:31 ` Marc Kleine-Budde 1 sibling, 1 reply; 9+ messages in thread From: Thomas.Kopp @ 2020-10-26 9:50 UTC (permalink / raw) To: mkl, linux-can; +Cc: mas > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: Friday, 16 October 2020 22:52 > To: linux-can@vger.kernel.org > Cc: Magnus Aagaard Sørensen <mas@csselectronics.com>; Marc Kleine- > Budde <mkl@pengutronix.de> > Subject: [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): > renamed from mcp251xfd_chip_clock_enable() > > Co-developed-by: Magnus Aagaard Sørensen <mas@csselectronics.com> > Not-Singed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > index c3f49543ff26..c36f5f14d50c 100644 > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > @@ -519,13 +519,16 @@ 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 on MCP2517FD and MCP2518FD, clearing > + * "Oscillator Disable" will wake the chip. For low power mode > + * on MCP2518FD, asserting the chip select will wake the > + * chip. Writing to the Oscillator register will wake it in > + * both cases. > */ > osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, > MCP251XFD_REG_OSC_CLKODIV_10); > @@ -569,10 +572,10 @@ 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. > + /* The Set Mode and SPI Reset command only works if the > + * controller is not in Sleep Mode. > */ > - err = mcp251xfd_chip_clock_enable(priv); > + err = mcp251xfd_chip_wake(priv); > if (err) > return err; > > -- > 2.28.0 This patch series works fine on my setup (RPI4, MCP2518FD, external 4 MHz crystal). Going through the code I noticed that the Min specified OSC frequency is specified from 1 MHz to 40 MHz. Technically the DS only specifies 4,20 and 40MHz as crystal/resonator options and 2MHz input as the minimum external clock. 4 with PLL and 40 direct are the preferred options for CAN-FD. I think the code is fine, given that the default is 40 MHz and it's the user's responsibility to design in the part according to the DS. We could narrow down the pll clockcheck to only allow 4 MHz though. All 3 patches in this series: Tested-by: Thomas Kopp <thomas.kopp@microchip.com> Best Regards, Thomas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() 2020-10-26 9:50 ` Thomas.Kopp @ 2020-10-26 10:31 ` Marc Kleine-Budde 0 siblings, 0 replies; 9+ messages in thread From: Marc Kleine-Budde @ 2020-10-26 10:31 UTC (permalink / raw) To: Thomas.Kopp, linux-can; +Cc: mas [-- Attachment #1.1: Type: text/plain, Size: 1110 bytes --] On 10/26/20 10:50 AM, Thomas.Kopp@microchip.com wrote: > This patch series works fine on my setup (RPI4, MCP2518FD, external 4 MHz > crystal). \o/ Thanks \o/ > Going through the code I noticed that the Min specified OSC frequency is > specified from 1 MHz to 40 MHz. Technically the DS only specifies 4,20 and > 40MHz as crystal/resonator options and 2MHz input as the minimum external > clock. 4 with PLL and 40 direct are the preferred options for CAN-FD. I think > the code is fine, given that the default is 40 MHz and it's the user's > responsibility to design in the part according to the DS. We could narrow > down the pll clockcheck to only allow 4 MHz though. I'll address this in my next patch series. > All 3 patches in this series: > Tested-by: Thomas Kopp <thomas.kopp@microchip.com> regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] can: mcp251xfd: mcp251xfd_chip_timestamp_init(): 2020-10-16 20:52 [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() Marc Kleine-Budde @ 2020-10-16 20:52 ` Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 3/3] can: mcp251xfd: add support for internal PLL Marc Kleine-Budde 2020-10-16 20:55 ` [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde 3 siblings, 0 replies; 9+ messages in thread From: Marc Kleine-Budde @ 2020-10-16 20:52 UTC (permalink / raw) To: linux-can; +Cc: Magnus Aagaard Sørensen, Marc Kleine-Budde Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index c36f5f14d50c..b8304b7096d1 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -665,10 +665,11 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) 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; + return regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); +} +static int mcp251xfd_chip_timestamp_init(const struct mcp251xfd_priv *priv) +{ /* Set Time Base Counter Prescaler to 1. * * This means an overflow of the 32 bit Time Base Counter @@ -1038,6 +1039,10 @@ static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv) if (err) goto out_chip_stop; + err = mcp251xfd_chip_timestamp_init(priv); + if (err) + goto out_chip_stop; + err = mcp251xfd_set_bittiming(priv); if (err) goto out_chip_stop; -- 2.28.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] can: mcp251xfd: add support for internal PLL 2020-10-16 20:52 [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 2/3] can: mcp251xfd: mcp251xfd_chip_timestamp_init(): Marc Kleine-Budde @ 2020-10-16 20:52 ` Marc Kleine-Budde 2020-10-19 5:23 ` Magnus Aagaard Sørensen 2020-10-16 20:55 ` [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde 3 siblings, 1 reply; 9+ messages in thread From: Marc Kleine-Budde @ 2020-10-16 20:52 UTC (permalink / raw) To: linux-can; +Cc: Magnus Aagaard Sørensen, Marc Kleine-Budde 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. Link: https://lore.kernel.org/r/20201015124401.2766-3-mas@csselectronics.com Co-developed-by: Magnus Aagaard Sørensen <mas@csselectronics.com> Not-Singed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 73 +++++++++++++++---- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 3 + 2 files changed, 63 insertions(+), 13 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index b8304b7096d1..2be643c6e243 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -479,8 +479,12 @@ __mcp251xfd_chip_set_mode(const struct mcp251xfd_priv *priv, if (err) return err; - if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP || nowait) + if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP || nowait) { + if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP) + priv->spi->max_speed_hz = priv->spi_max_speed_hz_slow; + return 0; + } err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_CON, con, FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK, @@ -655,7 +659,7 @@ static int mcp251xfd_chip_softreset(const struct mcp251xfd_priv *priv) static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) { - u32 osc; + u32 osc, osc_reference, osc_mask; int err; /* Activate Low Power Mode on Oscillator Disable. This only @@ -665,7 +669,34 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) osc = MCP251XFD_REG_OSC_LPMEN | FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, MCP251XFD_REG_OSC_CLKODIV_10); - return regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); + osc_reference = MCP251XFD_REG_OSC_OSCRDY; + osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY; + + if (priv->pll_enable) { + osc |= MCP251XFD_REG_OSC_PLLEN; + osc_reference |= MCP251XFD_REG_OSC_PLLRDY; + } + + err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); + if (err) + return err; + + /* Wait for "Oscillator + 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 (err == -ETIMEDOUT) { + netdev_err(priv->ndev, + "Timeout waiting for Oscillator + PLL Ready (osc=0x%08x, osc_reference=0x%08x)\n", + osc, osc_reference); + return -ETIMEDOUT; + } + + priv->spi->max_speed_hz = priv->spi_max_speed_hz_fast; + + return 0; } static int mcp251xfd_chip_timestamp_init(const struct mcp251xfd_priv *priv) @@ -2604,11 +2635,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 ef:%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_enable ? '+' : '-', MCP251XFD_QUIRK_ACTIVE(MAB_NO_WARN), MCP251XFD_QUIRK_ACTIVE(CRC_REG), MCP251XFD_QUIRK_ACTIVE(CRC_RX), @@ -2619,8 +2651,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); @@ -2650,6 +2684,10 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv) if (err) goto out_chip_set_mode_sleep; + err = mcp251xfd_chip_clock_init(priv); + if (err) + goto out_chip_set_mode_sleep; + err = mcp251xfd_register_chip_detect(priv); if (err) goto out_chip_set_mode_sleep; @@ -2743,6 +2781,7 @@ static int mcp251xfd_probe(struct spi_device *spi) struct gpio_desc *rx_int; struct regulator *reg_vdd, *reg_xceiver; struct clk *clk; + bool pll_enable = false; u32 freq; int err; @@ -2785,12 +2824,8 @@ static int mcp251xfd_probe(struct spi_device *spi) return -ERANGE; } - 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; - } + if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) + pll_enable = true; ndev = alloc_candev(sizeof(struct mcp251xfd_priv), MCP251XFD_TX_OBJ_NUM_MAX); @@ -2806,6 +2841,8 @@ static int mcp251xfd_probe(struct spi_device *spi) priv = netdev_priv(ndev); spi_set_drvdata(spi, priv); priv->can.clock.freq = freq; + if (pll_enable) + priv->can.clock.freq *= MCP251XFD_OSC_PLL_MULTIPLIER; 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; @@ -2817,6 +2854,7 @@ static int mcp251xfd_probe(struct spi_device *spi) priv->spi = spi; priv->rx_int = rx_int; priv->clk = clk; + priv->pll_enable = pll_enable; priv->reg_vdd = reg_vdd; priv->reg_xceiver = reg_xceiver; @@ -2856,7 +2894,16 @@ 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_enable) + 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); diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index fa1246e39980..813a791234de 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_enable; struct regulator *reg_vdd; struct regulator *reg_xceiver; -- 2.28.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] can: mcp251xfd: add support for internal PLL 2020-10-16 20:52 ` [PATCH v2 3/3] can: mcp251xfd: add support for internal PLL Marc Kleine-Budde @ 2020-10-19 5:23 ` Magnus Aagaard Sørensen 0 siblings, 0 replies; 9+ messages in thread From: Magnus Aagaard Sørensen @ 2020-10-19 5:23 UTC (permalink / raw) To: linux-can; +Cc: Marc Kleine-Budde Signed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> On 16-10-2020 22:52, Marc Kleine-Budde wrote: > 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. > > Link: https://lore.kernel.org/r/20201015124401.2766-3-mas@csselectronics.com > Co-developed-by: Magnus Aagaard Sørensen <mas@csselectronics.com> > Not-Singed-off-by: Magnus Aagaard Sørensen <mas@csselectronics.com> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 73 +++++++++++++++---- > drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 3 + > 2 files changed, 63 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > index b8304b7096d1..2be643c6e243 100644 > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c > @@ -479,8 +479,12 @@ __mcp251xfd_chip_set_mode(const struct mcp251xfd_priv *priv, > if (err) > return err; > > - if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP || nowait) > + if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP || nowait) { > + if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP) > + priv->spi->max_speed_hz = priv->spi_max_speed_hz_slow; > + > return 0; > + } > > err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_CON, con, > FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK, > @@ -655,7 +659,7 @@ static int mcp251xfd_chip_softreset(const struct mcp251xfd_priv *priv) > > static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) > { > - u32 osc; > + u32 osc, osc_reference, osc_mask; > int err; > > /* Activate Low Power Mode on Oscillator Disable. This only > @@ -665,7 +669,34 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) > osc = MCP251XFD_REG_OSC_LPMEN | > FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, > MCP251XFD_REG_OSC_CLKODIV_10); > - return regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); > + osc_reference = MCP251XFD_REG_OSC_OSCRDY; > + osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY; > + > + if (priv->pll_enable) { > + osc |= MCP251XFD_REG_OSC_PLLEN; > + osc_reference |= MCP251XFD_REG_OSC_PLLRDY; > + } > + > + err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); > + if (err) > + return err; > + > + /* Wait for "Oscillator + 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 (err == -ETIMEDOUT) { > + netdev_err(priv->ndev, > + "Timeout waiting for Oscillator + PLL Ready (osc=0x%08x, osc_reference=0x%08x)\n", > + osc, osc_reference); > + return -ETIMEDOUT; > + } > + > + priv->spi->max_speed_hz = priv->spi_max_speed_hz_fast; > + > + return 0; > } > > static int mcp251xfd_chip_timestamp_init(const struct mcp251xfd_priv *priv) > @@ -2604,11 +2635,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 ef:%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_enable ? '+' : '-', > MCP251XFD_QUIRK_ACTIVE(MAB_NO_WARN), > MCP251XFD_QUIRK_ACTIVE(CRC_REG), > MCP251XFD_QUIRK_ACTIVE(CRC_RX), > @@ -2619,8 +2651,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); > > @@ -2650,6 +2684,10 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv) > if (err) > goto out_chip_set_mode_sleep; > > + err = mcp251xfd_chip_clock_init(priv); > + if (err) > + goto out_chip_set_mode_sleep; > + > err = mcp251xfd_register_chip_detect(priv); > if (err) > goto out_chip_set_mode_sleep; > @@ -2743,6 +2781,7 @@ static int mcp251xfd_probe(struct spi_device *spi) > struct gpio_desc *rx_int; > struct regulator *reg_vdd, *reg_xceiver; > struct clk *clk; > + bool pll_enable = false; > u32 freq; > int err; > > @@ -2785,12 +2824,8 @@ static int mcp251xfd_probe(struct spi_device *spi) > return -ERANGE; > } > > - 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; > - } > + if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) > + pll_enable = true; > > ndev = alloc_candev(sizeof(struct mcp251xfd_priv), > MCP251XFD_TX_OBJ_NUM_MAX); > @@ -2806,6 +2841,8 @@ static int mcp251xfd_probe(struct spi_device *spi) > priv = netdev_priv(ndev); > spi_set_drvdata(spi, priv); > priv->can.clock.freq = freq; > + if (pll_enable) > + priv->can.clock.freq *= MCP251XFD_OSC_PLL_MULTIPLIER; > 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; > @@ -2817,6 +2854,7 @@ static int mcp251xfd_probe(struct spi_device *spi) > priv->spi = spi; > priv->rx_int = rx_int; > priv->clk = clk; > + priv->pll_enable = pll_enable; > priv->reg_vdd = reg_vdd; > priv->reg_xceiver = reg_xceiver; > > @@ -2856,7 +2894,16 @@ 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_enable) > + 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); > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > index fa1246e39980..813a791234de 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_enable; > struct regulator *reg_vdd; > struct regulator *reg_xceiver; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] can: mcp251xfd: PLL support 2020-10-16 20:52 [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde ` (2 preceding siblings ...) 2020-10-16 20:52 ` [PATCH v2 3/3] can: mcp251xfd: add support for internal PLL Marc Kleine-Budde @ 2020-10-16 20:55 ` Marc Kleine-Budde 3 siblings, 0 replies; 9+ messages in thread From: Marc Kleine-Budde @ 2020-10-16 20:55 UTC (permalink / raw) To: linux-can; +Cc: Magnus Aagaard Sørensen [-- Attachment #1.1: Type: text/plain, Size: 1001 bytes --] On 10/16/20 10:52 PM, Marc Kleine-Budde wrote: > Hello, > > I've looked at Magnus Aagaard Sørensen's patches and decided the PLL idea looks > quite good, but the runtime pm change should better made after adding PLL support. > > This series focuses on adding PLL support only. Consider this as an RFC, the > patches need proper description. > > I've tested the setup here with two mcp2518fd, where one supplies the 4MHz for > the other and it works for me. > > Testing welcome. Magnus, I've credited you as the Co-devloper of the patches 1 and 3, can I have your Signed-off-by? See https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L423 for details. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-26 10:40 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-16 20:52 [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() Marc Kleine-Budde 2020-10-19 5:23 ` Magnus Aagaard Sørensen 2020-10-26 9:50 ` Thomas.Kopp 2020-10-26 10:31 ` Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 2/3] can: mcp251xfd: mcp251xfd_chip_timestamp_init(): Marc Kleine-Budde 2020-10-16 20:52 ` [PATCH v2 3/3] can: mcp251xfd: add support for internal PLL Marc Kleine-Budde 2020-10-19 5:23 ` Magnus Aagaard Sørensen 2020-10-16 20:55 ` [PATCH v2 0/3] can: mcp251xfd: PLL support Marc Kleine-Budde
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.