All of lore.kernel.org
 help / color / mirror / Atom feed
* mcp25xxfd driver questions
@ 2020-09-29 11:07 Magnus Aagaard Sørensen
  2020-09-29 13:46 ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Magnus Aagaard Sørensen @ 2020-09-29 11:07 UTC (permalink / raw)
  To: linux-can

This is my first post to the list, so please inform me of any errors in 
etiquette.

I'm evaluating the MCP2518FD, and have two questions to the driver.

1. I could not find any references to the GPIOs of the chip. Is it 
correct that these are not exposed to the host system?

2. When setting the oscillator frequency outside the 
MCP25XXFD_SYSCLOCK_HZ_MIN and MCP25XXFD_SYSCLOCK_HZ_MAX range, the 
frequency is compared to the max value scaled by the max PLL value. Is 
the intention to compare with the min value? Currently, an external 
oscillator of 4 MHz and a PLL value of 10, resulting in 40 MHz, is 
treated as being too low.

diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c 
b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
index bd2ba981ae36..9e0246c4e49f 100644
--- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
+++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
@@ -2770,7 +2770,7 @@ static int mcp25xxfd_probe(struct spi_device *spi)
          return -ERANGE;
      }

-    if (freq <= MCP25XXFD_SYSCLOCK_HZ_MAX / MCP25XXFD_OSC_PLL_MULTIPLIER) {
+    if (freq <= MCP25XXFD_SYSCLOCK_HZ_MIN / MCP25XXFD_OSC_PLL_MULTIPLIER) {
          dev_err(&spi->dev,
              "Oscillator frequency (%u Hz) is too low and PLL is not 
supported.\n",
              freq);

Regards, Magnus.




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: mcp25xxfd driver questions
  2020-09-29 11:07 mcp25xxfd driver questions Magnus Aagaard Sørensen
@ 2020-09-29 13:46 ` Marc Kleine-Budde
  2020-09-30  5:32   ` Magnus Aagaard Sørensen
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2020-09-29 13:46 UTC (permalink / raw)
  To: Magnus Aagaard Sørensen, linux-can


[-- Attachment #1.1: Type: text/plain, Size: 2179 bytes --]

On 9/29/20 1:07 PM, Magnus Aagaard Sørensen wrote:
> This is my first post to the list, so please inform me of any errors in 
> etiquette.

:D

> I'm evaluating the MCP2518FD, and have two questions to the driver.
> 
> 1. I could not find any references to the GPIOs of the chip. Is it 
> correct that these are not exposed to the host system?

ACK, gpio support is not implemented yet. Drop me a note, if you need it.

> 2. When setting the oscillator frequency outside the 
> MCP25XXFD_SYSCLOCK_HZ_MIN and MCP25XXFD_SYSCLOCK_HZ_MAX range, the 
> frequency is compared to the max value scaled by the max PLL value. Is 
> the intention to compare with the min value? Currently, an external 
> oscillator of 4 MHz and a PLL value of 10, resulting in 40 MHz, is 
> treated as being too low.

This is intended. I have no hardware with a 4MHz osc to test, so I decided to
not support the 4MHz osc for now. If you design new hardware I suggest to use a
40MHz osc, as probably no one has tested the hardware thoroughly in the PLL
mode. If you need 4MHz support, it can be added, given there is hardware.

> diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c 
> b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
> index bd2ba981ae36..9e0246c4e49f 100644
> --- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
> +++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd-core.c
> @@ -2770,7 +2770,7 @@ static int mcp25xxfd_probe(struct spi_device *spi)
>           return -ERANGE;
>       }
> 
> -    if (freq <= MCP25XXFD_SYSCLOCK_HZ_MAX / MCP25XXFD_OSC_PLL_MULTIPLIER) {
> +    if (freq <= MCP25XXFD_SYSCLOCK_HZ_MIN / MCP25XXFD_OSC_PLL_MULTIPLIER) {
>           dev_err(&spi->dev,
>               "Oscillator frequency (%u Hz) is too low and PLL is not 
> supported.\n",
>               freq);

Hope that helps.

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] 6+ messages in thread

* Re: mcp25xxfd driver questions
  2020-09-29 13:46 ` Marc Kleine-Budde
@ 2020-09-30  5:32   ` Magnus Aagaard Sørensen
  2020-09-30  7:27     ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Magnus Aagaard Sørensen @ 2020-09-30  5:32 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can

On 29-09-2020 15:46, Marc Kleine-Budde wrote:
> On 9/29/20 1:07 PM, Magnus Aagaard Sørensen wrote:
>> I'm evaluating the MCP2518FD, and have two questions to the driver.
I should clarify that we already have an older internal test board with 
the chip working, using the driver by Martin Sperl. I'm evaluating the 
possibilities to migrate to this driver in the future, since I can see 
it is being mainline, but for now I mainly wish to use it on a new 
internal test board.
>> 1. I could not find any references to the GPIOs of the chip. Is it
>> correct that these are not exposed to the host system?
> ACK, gpio support is not implemented yet. Drop me a note, if you need it.

On the board I'm currently working on getting up and running, we have no 
need for GPIOs or any of the other advanced features of the MCP2518FD. 
On our older board, we do utilize the GPIO and oscillator output 
functionality of the chip. It works for now with the old driver, so it 
is not a priority. It would be nice to have this functionality in the 
future for us.

>> 2. When setting the oscillator frequency outside the
>> MCP25XXFD_SYSCLOCK_HZ_MIN and MCP25XXFD_SYSCLOCK_HZ_MAX range, the
>> frequency is compared to the max value scaled by the max PLL value. Is
>> the intention to compare with the min value? Currently, an external
>> oscillator of 4 MHz and a PLL value of 10, resulting in 40 MHz, is
>> treated as being too low.
> This is intended. I have no hardware with a 4MHz osc to test, so I decided to
> not support the 4MHz osc for now. If you design new hardware I suggest to use a
> 40MHz osc, as probably no one has tested the hardware thoroughly in the PLL
> mode. If you need 4MHz support, it can be added, given there is hardware.

I can provide some test information if needed, as I have internal 
testing hardware with a 4 MHz oscillator already. Are there any specific 
message sequences that needs to be tested?

The chip will most likely not reach high loads with the setup we have in 
mind, so I'll manually change the frequency check in the probe function 
for now.

> Hope that helps.
>
> regards,
> Marc

It's great to hear the reasoning. Thanks for the hard work you and 
others have put into this driver and the whole CAN system in linux.

Regards, Magnus.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: mcp25xxfd driver questions
  2020-09-30  5:32   ` Magnus Aagaard Sørensen
@ 2020-09-30  7:27     ` Marc Kleine-Budde
  2020-09-30 10:11       ` Magnus Aagaard Sørensen
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2020-09-30  7:27 UTC (permalink / raw)
  To: Magnus Aagaard Sørensen; +Cc: linux-can, Bas Vermeulen, Thomas Kopp


[-- Attachment #1.1: Type: text/plain, Size: 4020 bytes --]

On 9/30/20 7:32 AM, Magnus Aagaard Sørensen wrote:
> On 29-09-2020 15:46, Marc Kleine-Budde wrote:
>> On 9/29/20 1:07 PM, Magnus Aagaard Sørensen wrote:
>>> I'm evaluating the MCP2518FD, and have two questions to the driver.

> I should clarify that we already have an older internal test board with 
> the chip working, using the driver by Martin Sperl. I'm evaluating the 
> possibilities to migrate to this driver in the future, since I can see 
> it is being mainline, but for now I mainly wish to use it on a new 
> internal test board.

Ah, ok. More users are always welcome. Which SoC are you using?

>>> 1. I could not find any references to the GPIOs of the chip. Is it
>>> correct that these are not exposed to the host system?
>> ACK, gpio support is not implemented yet. Drop me a note, if you need it.
> 
> On the board I'm currently working on getting up and running, we have no 
> need for GPIOs or any of the other advanced features of the MCP2518FD. 
> On our older board, we do utilize the GPIO and oscillator output 
> functionality of the chip. It works for now with the old driver, so it 
> is not a priority. It would be nice to have this functionality in the 
> future for us.

Bas Vermeulen (Cc'ed) is using the mainline driver (or an older version of it)
and send a pull request for oscillator output
(https://github.com/marckleinebudde/linux/pull/5). I did an initial review
there. This is a good starting point if you want to contribute (or drop me a
note for commercial support).

>>> 2. When setting the oscillator frequency outside the
>>> MCP25XXFD_SYSCLOCK_HZ_MIN and MCP25XXFD_SYSCLOCK_HZ_MAX range, the
>>> frequency is compared to the max value scaled by the max PLL value. Is
>>> the intention to compare with the min value? Currently, an external
>>> oscillator of 4 MHz and a PLL value of 10, resulting in 40 MHz, is
>>> treated as being too low.
>
>> This is intended. I have no hardware with a 4MHz osc to test, so I decided to
>> not support the 4MHz osc for now. If you design new hardware I suggest to use a
>> 40MHz osc, as probably no one has tested the hardware thoroughly in the PLL
>> mode. If you need 4MHz support, it can be added, given there is hardware.
> 
> I can provide some test information if needed, as I have internal 
> testing hardware with a 4 MHz oscillator already. Are there any specific 
> message sequences that needs to be tested?

You have to take care of the PLL in the functions mcp25xxfd_chip_clock_enable(),
mcp25xxfd_chip_softreset_check(), mcp25xxfd_chip_clock_init().

The MCP25XXFD_REG_OSC_PLLEN bit has to be set and the MCP25XXFD_REG_OSC_PLLRDY
bit has to be polled.

I'm not sure what SPI speed you can use, when communicating with the mcp, if the
PLL is not enabled. Maybe Thomas (Cc'ed) can answer this. I suspect you can only
use 2MHZ (or rather 85% of it) if you have a 4 MHz OSC with PLL still disabled.
> The chip will most likely not reach high loads with the setup we have in 
> mind, so I'll manually change the frequency check in the probe function 
> for now.

This will probably not work, as the driver has to enable the PLL and the SPI
clock has to be really slow.

> It's great to hear the reasoning. Thanks for the hard work you and 
> others have put into this driver and the whole CAN system in linux.

Thanks. I'm just standing on the shoulder of giants. This driver was and is
great community work. First of all there was Martin's driver, which shows how a
Linux driver for the chip can look. Then several tireless testers here and on
github, direct and open communication with Microchip, and there are several
$CUSTOMERs paying to get the driver mainline.

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] 6+ messages in thread

* Re: mcp25xxfd driver questions
  2020-09-30  7:27     ` Marc Kleine-Budde
@ 2020-09-30 10:11       ` Magnus Aagaard Sørensen
  2020-09-30 10:34         ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Magnus Aagaard Sørensen @ 2020-09-30 10:11 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can, Bas Vermeulen, Thomas Kopp

On 30-09-2020 09:27, Marc Kleine-Budde wrote:
> On 9/30/20 7:32 AM, Magnus Aagaard Sørensen wrote:
>> On 29-09-2020 15:46, Marc Kleine-Budde wrote:
>>> On 9/29/20 1:07 PM, Magnus Aagaard Sørensen wrote:
>>>> I'm evaluating the MCP2518FD, and have two questions to the driver.
>> I should clarify that we already have an older internal test board with
>> the chip working, using the driver by Martin Sperl. I'm evaluating the
>> possibilities to migrate to this driver in the future, since I can see
>> it is being mainline, but for now I mainly wish to use it on a new
>> internal test board.
> Ah, ok. More users are always welcome. Which SoC are you using?
We are controlling the test boards using a Raspberry Pi 4, so I assume 
it is a Broadcom BCM2711.
>>>> 1. I could not find any references to the GPIOs of the chip. Is it
>>>> correct that these are not exposed to the host system?
>>> ACK, gpio support is not implemented yet. Drop me a note, if you need it.
>> On the board I'm currently working on getting up and running, we have no
>> need for GPIOs or any of the other advanced features of the MCP2518FD.
>> On our older board, we do utilize the GPIO and oscillator output
>> functionality of the chip. It works for now with the old driver, so it
>> is not a priority. It would be nice to have this functionality in the
>> future for us.
> Bas Vermeulen (Cc'ed) is using the mainline driver (or an older version of it)
> and send a pull request for oscillator output
> (https://github.com/marckleinebudde/linux/pull/5). I did an initial review
> there. This is a good starting point if you want to contribute (or drop me a
> note for commercial support).
>
>>>> 2. When setting the oscillator frequency outside the
>>>> MCP25XXFD_SYSCLOCK_HZ_MIN and MCP25XXFD_SYSCLOCK_HZ_MAX range, the
>>>> frequency is compared to the max value scaled by the max PLL value. Is
>>>> the intention to compare with the min value? Currently, an external
>>>> oscillator of 4 MHz and a PLL value of 10, resulting in 40 MHz, is
>>>> treated as being too low.
>>> This is intended. I have no hardware with a 4MHz osc to test, so I decided to
>>> not support the 4MHz osc for now. If you design new hardware I suggest to use a
>>> 40MHz osc, as probably no one has tested the hardware thoroughly in the PLL
>>> mode. If you need 4MHz support, it can be added, given there is hardware.
>> I can provide some test information if needed, as I have internal
>> testing hardware with a 4 MHz oscillator already. Are there any specific
>> message sequences that needs to be tested?
> You have to take care of the PLL in the functions mcp25xxfd_chip_clock_enable(),
> mcp25xxfd_chip_softreset_check(), mcp25xxfd_chip_clock_init().
>
> The MCP25XXFD_REG_OSC_PLLEN bit has to be set and the MCP25XXFD_REG_OSC_PLLRDY
> bit has to be polled.
>
> I'm not sure what SPI speed you can use, when communicating with the mcp, if the
> PLL is not enabled. Maybe Thomas (Cc'ed) can answer this. I suspect you can only
> use 2MHZ (or rather 85% of it) if you have a 4 MHz OSC with PLL still disabled.
Thanks for the hints, I'll see if I can adapt it to our use case.
>> The chip will most likely not reach high loads with the setup we have in
>> mind, so I'll manually change the frequency check in the probe function
>> for now.
> This will probably not work, as the driver has to enable the PLL and the SPI
> clock has to be really slow.
>
>> It's great to hear the reasoning. Thanks for the hard work you and
>> others have put into this driver and the whole CAN system in linux.
> Thanks. I'm just standing on the shoulder of giants. This driver was and is
> great community work. First of all there was Martin's driver, which shows how a
> Linux driver for the chip can look. Then several tireless testers here and on
> github, direct and open communication with Microchip, and there are several
> $CUSTOMERs paying to get the driver mainline.
>
> regards,
> Marc

Regards, Magnus.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: mcp25xxfd driver questions
  2020-09-30 10:11       ` Magnus Aagaard Sørensen
@ 2020-09-30 10:34         ` Marc Kleine-Budde
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2020-09-30 10:34 UTC (permalink / raw)
  To: Magnus Aagaard Sørensen; +Cc: linux-can, Bas Vermeulen, Thomas Kopp


[-- Attachment #1.1: Type: text/plain, Size: 3097 bytes --]

On 9/30/20 12:11 PM, Magnus Aagaard Sørensen wrote:
>> Ah, ok. More users are always welcome. Which SoC are you using?
> We are controlling the test boards using a Raspberry Pi 4, so I assume 
> it is a Broadcom BCM2711.

Ok....The SPI clock situation on the rpi4 is not optimal. But's a a different
story :)

>>>>> 1. I could not find any references to the GPIOs of the chip. Is it
>>>>> correct that these are not exposed to the host system?
>>>> ACK, gpio support is not implemented yet. Drop me a note, if you need it.
>>> On the board I'm currently working on getting up and running, we have no
>>> need for GPIOs or any of the other advanced features of the MCP2518FD.
>>> On our older board, we do utilize the GPIO and oscillator output
>>> functionality of the chip. It works for now with the old driver, so it
>>> is not a priority. It would be nice to have this functionality in the
>>> future for us.
>> Bas Vermeulen (Cc'ed) is using the mainline driver (or an older version of it)
>> and send a pull request for oscillator output
>> (https://github.com/marckleinebudde/linux/pull/5). I did an initial review
>> there. This is a good starting point if you want to contribute (or drop me a
>> note for commercial support).
>>
>>>>> 2. When setting the oscillator frequency outside the
>>>>> MCP25XXFD_SYSCLOCK_HZ_MIN and MCP25XXFD_SYSCLOCK_HZ_MAX range, the
>>>>> frequency is compared to the max value scaled by the max PLL value. Is
>>>>> the intention to compare with the min value? Currently, an external
>>>>> oscillator of 4 MHz and a PLL value of 10, resulting in 40 MHz, is
>>>>> treated as being too low.
>>>> This is intended. I have no hardware with a 4MHz osc to test, so I decided to
>>>> not support the 4MHz osc for now. If you design new hardware I suggest to use a
>>>> 40MHz osc, as probably no one has tested the hardware thoroughly in the PLL
>>>> mode. If you need 4MHz support, it can be added, given there is hardware.
>>> I can provide some test information if needed, as I have internal
>>> testing hardware with a 4 MHz oscillator already. Are there any specific
>>> message sequences that needs to be tested?
>> You have to take care of the PLL in the functions mcp25xxfd_chip_clock_enable(),
>> mcp25xxfd_chip_softreset_check(), mcp25xxfd_chip_clock_init().
>>
>> The MCP25XXFD_REG_OSC_PLLEN bit has to be set and the MCP25XXFD_REG_OSC_PLLRDY
>> bit has to be polled.
>>
>> I'm not sure what SPI speed you can use, when communicating with the mcp, if the
>> PLL is not enabled. Maybe Thomas (Cc'ed) can answer this. I suspect you can only
>> use 2MHZ (or rather 85% of it) if you have a 4 MHz OSC with PLL still disabled.

> Thanks for the hints, I'll see if I can adapt it to our use case.

I'm looking forward for patches :)

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] 6+ messages in thread

end of thread, other threads:[~2020-09-30 10:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 11:07 mcp25xxfd driver questions Magnus Aagaard Sørensen
2020-09-29 13:46 ` Marc Kleine-Budde
2020-09-30  5:32   ` Magnus Aagaard Sørensen
2020-09-30  7:27     ` Marc Kleine-Budde
2020-09-30 10:11       ` Magnus Aagaard Sørensen
2020-09-30 10:34         ` 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.