RE: [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Sent: Friday, 16 October 2020 22:52
> To: linux-can@xxxxxxxxxxxxxxx
> Cc: Magnus Aagaard Sørensen <mas@xxxxxxxxxxxxxxxxxx>; Marc Kleine-
> Budde <mkl@xxxxxxxxxxxxxx>
> Subject: [PATCH v2 1/3] can: mcp251xfd: mcp251xfd_chip_wake():
> renamed from mcp251xfd_chip_clock_enable()
> 
> Co-developed-by: Magnus Aagaard Sørensen <mas@xxxxxxxxxxxxxxxxxx>
> Not-Singed-off-by: Magnus Aagaard Sørensen <mas@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx>

Best Regards,
Thomas





[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux