On Mon, Nov 11, 2019 at 11:28:14AM +0100, Marc Kleine-Budde wrote: > On 11/11/19 12:09 AM, Drew Fustini wrote: > > Hello, I'm looking at mcp251x driver and it does not seem to read the > > device tree property > > mcp251x,oscillator-frequency. > > > > I have a problem where I have MCP2515 with a 16MHz oscillator. However, > > it is getting configured with a 8MHz clock despite > > having mcp251x,oscillator-frequency defined as 16MHz: > > https://github.com/pdp7/bb.org-overlays/blob/patch-1/src/arm/PB-MCP2515-SPI1.dts > > I assume you get the 8MHz clock rate from the "ip" output, right? > I added printk to output the value of: freq = clk_get_rate(clk); which is from line 1041: https://elixir.bootlin.com/linux/latest/source/drivers/net/can/spi/mcp251x.c#L1041 which shows: mcp251x_can_probe: clk_get_rate(clk)=8000000 I'm confused where that value is coming from. The line that divedes freq / 2 comes later. > Like here? (example output from a different CAN controller) > > It's due to this line in the code: > > https://elixir.bootlin.com/linux/latest/source/drivers/net/can/spi/mcp251x.c#L1065 > > where only the half of the external osc frequency is stored. That value > is displayed in the user space by the "ip" command. Here is the output which shows 8MHz when freq is hardcoded to 16MHz instead of the 8MHz that clk_get_rate(clk) returns: # ip -details -statistics link show can2 6: can2: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10 link/can promiscuity 0 can state ERROR-ACTIVE restart-ms 0 bitrate 33057 sample-point 0.818 tq 2750 prop-seg 4 phase-seg1 4 phase-seg2 2 sjw 1 mcp251x: tseg1 3..16 tseg2 2..8 sjw 1..4 brp 1..64 brp-inc 1 clock 8000000 re-started bus-errors arbit-lost error-warn error-pass bus-off 0 0 0 0 0 0 numtxqueues 1 gso_max_size 65536 gso_max_segs 65535 RX: bytes packets errors dropped overrun mcast 4 1 0 0 0 0 TX: bytes packets errors dropped carrier collsns 0 0 0 0 0 0 debian@beaglebone:~$ dmesg |grep -i mcp [ 27.295958] mcp251x_can_probe: enter [ 27.363720] mcp251x_can_probe: clk_get_rate(clk)=8000000 [ 27.363731] mcp251x_can_probe: OVERRIDE freq=clk_get_rate(clk)=16000000 [ 28.144501] mcp251x spi1.1 can2: MCP2515 successfully initialized. [ 579.547200] mcp251x spi1.1 can2: bitrate error 0.1% Here is the diff where I hardcode 16MHz: https://gist.github.com/pdp7/8e1765110178f4f65b04d19ac7cb1c4d#gistcomment-3079280 > > I would appreciate any advice on whether a patch for mcp251x.c to > > read mcp251x,oscillator-frequency would be a good way to solve this issue. > > Looking at the datasheet[1] of the mcp2515 page 40, equation 5-2, I > think this is correct. As the timequanta is calculated by: > > Tq = (2 * Brp) / fosc > > On other words: > > Tq = Brp / (fosc / 2) > > We have no means of expressing the additional "/2" during the bit timing > calculation, but to store the fosc/2 as the "effective" oscillator > frequency. > > I successfully got a mcp25625 (which is basically a mcp2515 with > internal phy) running on a rapsi using the frequency printed on the > oscillator in the DT-overlay. Thanks for the explanation. I'm still trying to figure out why clk_get_rate(clk) is returning 8MHz when I have 16MHz in the device tree. Would you be able to the DT overlay you are using? Maybe I am not using the correct properties to define the clock? This is the DT Overlay: https://gist.github.com/pdp7/56174646bb9d075b041f24de2bb01973 Thank you, Drew
