| Message ID | c9cf3995f45c363e432b3ae8eb1275e54f009fc8.1636967198.git.matthias.schiffer@ew.tq-group.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix bit timings for m_can_pci (Elkhart Lake) | expand |
Hi On 11/15/21 11:18 AM, Matthias Schiffer wrote: > When testing the CAN controller on our Ekhart Lake hardware, we > determined that all communication was running with twice the configured > bitrate. Changing the reference clock rate from 100MHz to 200MHz fixed > this. Intel's support has confirmed to us that 200MHz is indeed the > correct clock rate. > > Fixes: cab7ffc0324f ("can: m_can: add PCI glue driver for Intel Elkhart Lake") > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > --- > drivers/net/can/m_can/m_can_pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c > index 89cc3d41e952..d3c030a13cbe 100644 > --- a/drivers/net/can/m_can/m_can_pci.c > +++ b/drivers/net/can/m_can/m_can_pci.c > @@ -18,7 +18,7 @@ > > #define M_CAN_PCI_MMIO_BAR 0 > > -#define M_CAN_CLOCK_FREQ_EHL 100000000 > +#define M_CAN_CLOCK_FREQ_EHL 200000000 > #define CTL_CSR_INT_CTL_OFFSET 0x508 > I'll double check this from HW people but at quick test on an HW I have the signals on an oscilloscope were having 1 us shortest cycle (~500 ns low, ~500 ns high) when testing like below: ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd on ip link set can0 up ip link set can1 type can bitrate 1000000 dbitrate 2000000 fd on ip link set can1 up candump can0 & cansend can1 01a#11223344AABBCCDD Caveat: I'm not an CAN signaling expert at all. Jarkko
Hi On 11/15/21 4:48 PM, Jarkko Nikula wrote: > Hi > > On 11/15/21 11:18 AM, Matthias Schiffer wrote: >> When testing the CAN controller on our Ekhart Lake hardware, we >> determined that all communication was running with twice the configured >> bitrate. Changing the reference clock rate from 100MHz to 200MHz fixed >> this. Intel's support has confirmed to us that 200MHz is indeed the >> correct clock rate. >> >> Fixes: cab7ffc0324f ("can: m_can: add PCI glue driver for Intel >> Elkhart Lake") >> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> >> --- >> drivers/net/can/m_can/m_can_pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/can/m_can/m_can_pci.c >> b/drivers/net/can/m_can/m_can_pci.c >> index 89cc3d41e952..d3c030a13cbe 100644 >> --- a/drivers/net/can/m_can/m_can_pci.c >> +++ b/drivers/net/can/m_can/m_can_pci.c >> @@ -18,7 +18,7 @@ >> #define M_CAN_PCI_MMIO_BAR 0 >> -#define M_CAN_CLOCK_FREQ_EHL 100000000 >> +#define M_CAN_CLOCK_FREQ_EHL 200000000 >> #define CTL_CSR_INT_CTL_OFFSET 0x508 > I'll double check this from HW people but at quick test on an HW I have > the signals on an oscilloscope were having 1 us shortest cycle (~500 ns > low, ~500 ns high) when testing like below: > > ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd on I got confirmation the clock to CAN controller is indeed changed from 100 MHz to 200 MHz in release HW & firmware. I haven't upgraded the FW in a while on our HW so that perhaps explain why I was seeing expected rate :-) So which one is more appropriate: Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> or Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
On 16.11.2021 09:11:40, Jarkko Nikula wrote: > > ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd on > > I got confirmation the clock to CAN controller is indeed changed from 100 > MHz to 200 MHz in release HW & firmware. > > I haven't upgraded the FW in a while on our HW so that perhaps explain > why I was seeing expected rate :-) Can we query the FW version in the driver and set the clock rate accordingly? > So which one is more appropriate: > > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> > or > Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> regards, Marc
On 11/16/21 9:15 AM, Marc Kleine-Budde wrote: > On 16.11.2021 09:11:40, Jarkko Nikula wrote: >>> ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd on >> >> I got confirmation the clock to CAN controller is indeed changed from 100 >> MHz to 200 MHz in release HW & firmware. >> >> I haven't upgraded the FW in a while on our HW so that perhaps explain >> why I was seeing expected rate :-) > > Can we query the FW version in the driver and set the clock rate > accordingly? > Perhaps or check some clock register. I guess for now it can be considered fixed clock since I understood rate was changed before released to customers. I.e. customers shouldn't have firmware with different rates. At least I hope so :-) Jarkko
diff --git a/drivers/net/can/m_can/m_can_pci.c b/drivers/net/can/m_can/m_can_pci.c index 89cc3d41e952..d3c030a13cbe 100644 --- a/drivers/net/can/m_can/m_can_pci.c +++ b/drivers/net/can/m_can/m_can_pci.c @@ -18,7 +18,7 @@ #define M_CAN_PCI_MMIO_BAR 0 -#define M_CAN_CLOCK_FREQ_EHL 100000000 +#define M_CAN_CLOCK_FREQ_EHL 200000000 #define CTL_CSR_INT_CTL_OFFSET 0x508 struct m_can_pci_priv {
When testing the CAN controller on our Ekhart Lake hardware, we determined that all communication was running with twice the configured bitrate. Changing the reference clock rate from 100MHz to 200MHz fixed this. Intel's support has confirmed to us that 200MHz is indeed the correct clock rate. Fixes: cab7ffc0324f ("can: m_can: add PCI glue driver for Intel Elkhart Lake") Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> --- drivers/net/can/m_can/m_can_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)