CM4 SPI speed issue Bookworm v6.6

[robotdoly]

Describe the bug

I have a custom SPI driver which drives 2 LCD via SPI0, CS0 and CS1. It was working perfectly until the 6.6.31 update. I figured out that the issue related to SPI speed.

I am not able control SPI speed with DT or manually. It won't response to ;

spi-max-frequency = or spi->max_speed_hz = xxxx;

If I set speed via Device Tree; spi->max_speed_hz returns abnormal values like 1077680128, 1090041856

If I set it manually with spi->max_speed_hz = xxxx than I can read the correct value but it won't change the speed.

The device can handle 125mhz without an issue with kernel 5.15.74.

Did I miss something or is there an issue on SPI bus?

Steps to reproduce the behaviour

Probably the best approach will be to check speed via oscilloscope or a logic analyzer. My equipment does not allow me to check speeds over 100mhz. It looks between 150 - 200 Mhz based on my frame calculation!

Device (s)

Raspberry Pi CM4 Lite

System

Raspberry Pi reference 2024-03-15
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, f19ee211ddafcae300827f953d143de92a5c6624, stage2

May 24 2024 15:30:04
Copyright (c) 2012 Broadcom
version 4942b7633c0ff1af1ee95a51a33b56a9dae47529 (clean) (release) (start)

Linux doly 6.6.31+rpt-rpi-v8 #1 SMP PREEMPT Debian 1:6.6.31-1+rpt1 (2024-05-29) aarch64 GNU/Linux

Logs

No response

Additional context

No response

[pelwell]

It was working perfectly until the 6.6.31 update

What was the last-known-good version?

Rather than have us start an open-ended investigation into SPI speed handling on CM4, please post some example code/DTS that demonstrates the issue. If there is a problem in the SPI driver/framework then you should be able to reproduce it using spidev, removing the need to use your driver.

[robotdoly]

Hi @pelwell, like I mentioned before it is working with 5.15.74. (5.15.74 -> 6.6.31)

Here is my DT,

/dts-v1/;
/plugin/;

/ {
     compatible = "brcm,bcm2835", "bcrm,bcm2708", "bcrm,bcm2711";

    fragment@0 {
        target = <&spidev0>;

        __overlay__ {
            status = "disabled";
        };
    };

     fragment@1 {
        target = <&spidev1>;

        __overlay__ {
            status = "disabled";
        };
    };

    fragment@2 {
		target = <&spi0>;
		__overlay__ {
            status = "okay";
			#address-cells = <1>;
			#size-cells = <0>;
            //spi-max-frequency = <1000000>;

           lcd_left: lcd-left@0 {
                compatible = "doly,lcd-left";
                reg = <0>;
                #address-cells = <1>;
                #size-cells = <0>;
                spi-max-frequency = <1000000>;
            };

            lcd_right: lcd-right@1 {
                compatible = "doly,lcd-right";
                reg = <1>;
                #address-cells = <1>;
                #size-cells = <0>;
                spi-max-frequency = <1000000>;
            };
		};
	};
};

And this is trimmed driver test code:

// for 1 byte enum -fshort-enums dont forget ccflag otherwise 4 byte

#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/kdev_t.h>
#include <linux/fs.h>
#include <linux/cdev.h>
#include <linux/device.h>
#include <linux/slab.h>	   //kmalloc()
#include <linux/uaccess.h> //copy_to/from_user()
#include <linux/ioctl.h>
#include <linux/spi/spi.h> // for spi interface
#include <linux/gpio.h>	   // Required for the GPIO functions
#include <linux/delay.h>   //for msleep
#include <asm/uaccess.h>   // Needed by segment descriptors
#include <linux/of.h>	   // Include for Device Tree support
#include <linux/of_device.h>
#include <linux/mutex.h> // Include for mutex

#define LCD_LEFT 0
#define LCD_RIGHT 1

static struct spi_device *spi_left;
static struct spi_device *spi_right;

static int __init lcd_driver_init(void);
static void __exit lcd_driver_exit(void);

uint8_t default_tx[] = {
	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,
	0x40,	0x00,	0x00,	0x00,	0x00,	0x95,
	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,
	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,
	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,	0xFF,
	0xF0,	0x0D,
};

static const struct of_device_id lcd_of_match[] = {
	{.compatible = "doly,lcd-left"},
	{.compatible = "doly,lcd-right"},
	{},
};
MODULE_DEVICE_TABLE(of, lcd_of_match);

static const struct spi_device_id lcd_spi_id[] = {
	{.name = "lcd-left"},
	{.name = "lcd-right"},
	{}};
MODULE_DEVICE_TABLE(spi, lcd_spi_id);

static int lcd_driver_probe(struct spi_device *spi)
{
	const struct of_device_id *match;

	match = of_match_device(of_match_ptr(lcd_of_match), &spi->dev);

	if (!match)
	{
		printk(KERN_ERR "LCD: No matching device found\n");
		return -ENODEV;
	}

	if (strcmp(match->compatible, "doly,lcd-left") == 0)
	{
		// spi->chip_select = 0; // DT not set the chip select value force it to set 0
		spi_left = spi;
	}
	else if (strcmp(match->compatible, "doly,lcd-right") == 0)
	{
		// spi->chip_select = 1; // DT not set the chip select value force it to set 0
		spi_right = spi;
	}
	else
	{
		return -ENODEV;
	}

	spi->bits_per_word = 8;
	spi->max_speed_hz = 1000000;
	spi->mode = SPI_MODE_0;

	if (spi_setup(spi))
	{
		printk(KERN_ERR "LCD: Failed to setup SPI device %s\n", match->compatible);
		return -ENODEV;
	}

	printk(KERN_INFO "LCD: Device: %s CS: %i Speed: %i\n", match->compatible, spi->chip_select, spi->max_speed_hz);

	return 0;
}

static void lcd_driver_remove(struct spi_device *spi)
{
	if (spi == spi_left)
	{
		spi_left = NULL;
	}
	else if (spi == spi_right)
	{
		spi_right = NULL;
	}
}

static struct spi_driver lcd_driver = {
	.driver = {
		.name = "doly_lcd",
		.of_match_table = lcd_of_match,
		.owner = THIS_MODULE,
	},
	.id_table = lcd_spi_id,
	.probe = lcd_driver_probe,
	.remove = lcd_driver_remove,
};


static int __init lcd_driver_init(void)
{
	if (spi_register_driver(&lcd_driver) < 0)
	{
		printk(KERN_ERR "LCD: Failed to register SPI driver\n");
		return -1;
	}

	spi_write(spi_left, &default_tx, 32);
	msleep(10); // wait
	spi_write(spi_right, &default_tx, 32);

	printk(KERN_INFO "LCD: Device Driver Insert Success !\n");
	return 0;
}

static void __exit lcd_driver_exit(void)
{
	if (spi_left)
		spi_unregister_device(spi_left); // free spi device

	if (spi_right)
		spi_unregister_device(spi_right); // free spi device

	printk(KERN_INFO "LCD: Device Driver Remove...Done!!!\n");
}

module_init(lcd_driver_init);
module_exit(lcd_driver_exit);

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Levent");
MODULE_DESCRIPTION("LCD Test module");
MODULE_VERSION("0.1");

And this is what I get with kernel 5.15.74. This is correct speed and data!

The following image is taken kernel 6.6.31. Probably the data is true (logic analyzer can't handle) but the problem is speed!

Simple math 717 us / 5.9 us =~121 So, it looks like the SPI speed is around 125 MHZ instead of 1MHZ

I can't reproduce the same issue with spidev driver. I spent 2 days but I can't figure out what is wrong, I am not able to set speed with current kernel 6.6.31. Thanks for your help.

[pelwell]

Strange - it's working for me on a CM4 with a (self-built, but with the standard configuration) 6.6.34 kernel:

[pelwell]

And viewing the whole sequence with timing markers:

I don't think anything has changed in the kernel since 6.6.31, so I can't explain what you are seeing.

[robotdoly]

Thanks for your time @pelwell, today I tried all combinations that I have;
Different custom boards and CM4 IO boards with

CM4 lite 1Gb ( 2 different)
CM4 lite 2 Gb
CM4 2 Gb 8gm eMMC

6.6.20-1+rpt1 (2024-03-07) aarch64 (fresh installed)
6.6.31-1+rpt1 (2024-05-29) aarch64 (fresh installed)
6.1.21-v8+ #1642 SMP aarch64 (fresh installed)

All ended with the same result. I can't control SPI speed! The only working kernel is 5.15.74. I am going crazy, I am probably missing something.

Could you let me know your device and setup? And how did you load the test driver? Thank you.

[pelwell]

I'm testing on a CM4+CM4IO, loading your driver using your overlay. I chose to load it at runtime (sudo dtoverlay is6225, which is what I called it) for convenience, but it seems unlikely that loading it from config.txt would have a different outcome.

[robotdoly]

Thanks for your reply. Are you able to share your config.txt?

[pelwell]

This is what I have in config.txt (comments and blank lines removed):

dtparam=audio=on
camera_auto_detect=1
display_auto_detect=1
auto_initramfs=1
dtoverlay=vc4-kms-v3d
max_framebuffers=2
disable_fw_kms_setup=1
arm_64bit=1
disable_overscan=1
arm_boost=1
[cm4]
otg_mode=1
[all]
enable_uart=1
uart_2ndstage=1
dtoverlay=mcp23017,addr=0x10,gpiopin=12
kernel=kernel8.img

kernel=kernel8.img is there because the card is often in a Pi 5 and it is convenient to run the same kernel. The mcp23017 overlay was for another test, and is almost certainly irrelevant.

Ah - it might be interesting for you to add enable_uart=1. It will slow down booting slightly because it takes time for the UART to output the debug messageds, but it will also have the side effect of prevent the core clock from changing, resulting in an unvarying SPI clock.

[pelwell]

Under other circumstance I would now be running the same test without enable_uart=1, but I'm not in a position to drive the logic analyser remotely.

[robotdoly]

@pelwell, Finally I figured out the issue. All this happens due to "short-enums" flag. I have a few int8_t enum definitions in the original driver. I made a mistake and used the same make file while compiling it. And that "short-enums" flag cause the problem. I still don't know what has changed since 5.15.74, this flag has an issue, but now I am able to control SPI speed with the current kernel. I appreciate your support and time.