Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
9.4k stars 5.3k forks source link

ae89a65 breaks serial (not usb) communication for lpc1768 #4194

Closed pulsar256 closed 3 years ago

pulsar256 commented 3 years ago

Serial Communication with default settings (250000 baud) is not working starting with commit-id ae89a65956f95c2bee652396286d97b47ff804b6

verified by bisecting twice resulting in the same identified change:

➜  klipper git:(6cab7bcf) ✗ git bisect good
ae89a65956f95c2bee652396286d97b47ff804b6 is the first bad commit
commit ae89a65956f95c2bee652396286d97b47ff804b6
Author: Kevin O'Connor <kevin@koconnor.net>
Date:   Wed Mar 24 19:49:08 2021 -0400

    lpc176x: Do not modify PCLKSELx at runtime

    The lpc176x has an errata that could cause updates to PCLKSELx to not
    take effect.  Rework the code to use the default peripheral clock
    speed (25Mhz or 30Mhz) so that this register does not need to be
    updated at runtime.

    Signed-off-by: Kevin O'Connor <kevin@koconnor.net>

:040000 040000 4fb3f0682de8801a8e69d228e23a8de3d21e80c0 44aba2445b47ca4338c21d5c420dc1b5011f6fd4 M      src

Testing-Environment:

reverting the ae89a65956f95c2bee652396286d97b47ff804b6 against current master (8f9e497d6950bf47274cb084a6d3d8ca2b854976) restores the functionality.

KevinOConnor commented 3 years ago

I can confirm that commit is not correct for serial support. After the above change, a baud of 250000 would require a serial clock divisor of 6.25, and the code does not work correctly with fractions like that. (Previously, the peripheral clock was 4 times faster, and a divisor of 25 was fine.)

I'll need to think on how to fix that.

-Kevin

dianlight commented 3 years ago

As a temporary workaround just use a baud rate of 256000 where the divisor is 6.1035....

@KevinOConnor To keep the 250000 I found no other solution than to modify the clock as before. In any case, looking at the specifications of the lpc176x processor/libraries I noticed that the get_pclock_frequency should be modified to be more generic.

The code would look something like this:

// Return the frequency of the given peripheral clock
uint32_t
get_pclock_frequency(uint32_t pclk)
{
    uint32_t var_UartPclk_u32,var_Pclk_u32;
    var_UartPclk_u32 = (LPC_SC->PCLKSEL0 >> 6) & 0x03;
    switch( var_UartPclk_u32 )
    {
          case 0x00:
            var_Pclk_u32 = SystemCoreClock/4;
            break;
          case 0x01:
            var_Pclk_u32 = SystemCoreClock;
            break; 
          case 0x02:
            var_Pclk_u32 = SystemCoreClock/2;
            break; 
          case 0x03:
            var_Pclk_u32 = SystemCoreClock/8;
            break;
    }      
    return var_Pclk_u32;
}

If you agree I send a PR.

L.

pulsar256 commented 3 years ago

I had a laymans look at the errata which suggest, that one should not change the PCLKSELx registers after the PLL0 is connected. I take it, the initialization order is not easy to rework / implement?

Gfermoto commented 3 years ago

256000 did not help (lpc1769)

om2kw commented 3 years ago

baudrate 230400 is working with lpc1769 (skr1.4Turbo)

jakep82 commented 3 years ago

Is their any reason the original commit can't be reverted? A recent youtube video showed how to connect a Pi Zero to the SKR boards via UART, and now we're getting a wave of support requests because it doesn't work at the default 250000 baud.

Sineos commented 3 years ago

https://github.com/KevinOConnor/klipper/pull/4248#issuecomment-831406504

KevinOConnor commented 3 years ago

FYI, I have put together a possible fix on the work-lpcuart-20210504 branch (commit 191c358a). Unfortunately, I can't test this right now.

cd ~/klipper ; git fetch ; git checkout origin/work-lpcuart-20210504 ; sudo service klipper stop ; make ; make flash ; sudo service klipper start

-Kevin

pulsar256 commented 3 years ago

Works for me:

➜  klipper git:(work-lpcuart-20210504) ✗ make clean
➜  klipper git:(work-lpcuart-20210504) ✗ make
➜  klipper git:(work-lpcuart-20210504) ✗ flash.sh
lpc21isp version 1.97
File /home/pi/klipper/out/klipper.bin:
        loaded...
        image size : 19152
Image size : 19152
Synchronizing (ESC to abort). OK
Read bootcode version: 2
4
Read part ID: LPC1768, 512 kiB FLASH / 64 kiB SRAM (0x26013F37)
Will start programming at Sector 1 if possible, and conclude with Sector 0 to ensure that checksum is written last.
Wiping Device. OK
Sector 1: ..............................................................................................
Sector 2: ..............................................................................................
Sector 3: ..............................................................................................
Sector 4: ..................................................................
Sector 0: ..............................................................................................
Download Finished... taking 1 seconds
Now launching the brand new code
➜  klipper git:(work-lpcuart-20210504) ✗ sudo systemctl start klipper.service moonraker.service
Tue, 04 May 2021 14:22:15 GMT > status
Tue, 04 May 2021 14:22:15 GMT < ok
Tue, 04 May 2021 14:22:15 GMT < // Klipper state: Ready
pulsar256 commented 3 years ago

P.S. might come handy for testing / flashing over the serial port (requires reset pin and boot pin to be connected to the RPI)

flash.sh

#!/bin/bash

pin_reset=22
pin_boot=26
firmware=$HOME/klipper/out/klipper.bin

sudo systemctl stop klipper

# enter boot bootloader sequence
gpio mode $pin_reset OUT
gpio mode $pin_boot  OUT

# hold reset and bootloader down
gpio write $pin_reset 0
gpio write $pin_boot 0

# go back to high-z and let skr pullups set the lines to high
gpio mode $pin_reset IN
gpio mode $pin_boot IN

lpc21isp -wipe -bin $firmware /dev/ttyS0 230400 12000

sudo systemctl start klipper
harjoat commented 3 years ago

FYI, I have put together a possible fix on the work-lpcuart-20210504 branch (commit 191c358). Unfortunately, I can't test this right now.

cd ~/klipper ; git fetch ; git checkout origin/work-lpcuart-20210504 ; sudo service klipper stop ; make ; make flash ; sudo service klipper start

-Kevin

Tried it, worked for me with an skr 1.4 turbo.

dw-0 commented 3 years ago

FYI, I have put together a possible fix on the work-lpcuart-20210504 branch (commit 191c358). Unfortunately, I can't test this right now.

cd ~/klipper ; git fetch ; git checkout origin/work-lpcuart-20210504 ; sudo service klipper stop ; make ; make flash ; sudo service klipper start

-Kevin

Tested with a SKR 1.4 Turbo connected to a Pi Zero W via UART -> i can confirm it is working.

totolook commented 3 years ago

FYI, I have put together a possible fix on the work-lpcuart-20210504 branch (commit 191c358). Unfortunately, I can't test this right now.

cd ~/klipper ; git fetch ; git checkout origin/work-lpcuart-20210504 ; sudo service klipper stop ; make ; make flash ; sudo service klipper start

-Kevin

Tested with skr 1.4T and it's working well

KevinOConnor commented 3 years ago

Thanks. I committed that change to the master branch (commit 45cd3543).

-Kevin

pmlody commented 3 years ago

I can also confirm working set SKR 1.4 turbo with RPi Zero W

github-actions[bot] commented 3 years ago

This ticket is being closed because the underlying issue is now thought to be resolved.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.