netdev - Re: [PATCH net 1/4] can: m_can: pci: fix incorrect reference clock rate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0400022a-0515-db87-03cc-30b83c2aede2@linux.intel.com>
Date:   Tue, 16 Nov 2021 09:11:40 +0200
From:   Jarkko Nikula <jarkko.nikula@...ux.intel.com>
To:     Matthias Schiffer <matthias.schiffer@...tq-group.com>,
        Chandrasekar Ramakrishnan <rcsekar@...sung.com>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "Felipe Balbi (Intel)" <balbi@...nel.org>,
        linux-can@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 1/4] can: m_can: pci: fix incorrect reference clock
 rate

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@...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@...ux.intel.com>
or
Reviewed-by: Jarkko Nikula <jarkko.nikula@...ux.intel.com>