Skip to content

Why is SpiChipSelect limited to 0, 1, 2? #457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
douglm opened this issue Mar 28, 2025 · 14 comments
Closed

Why is SpiChipSelect limited to 0, 1, 2? #457

douglm opened this issue Mar 28, 2025 · 14 comments

Comments

@douglm
Copy link

douglm commented Mar 28, 2025

Fairly new to this and trying to get widgetlords digital input boards working with a Raspberry Pi 5 and pi4j.

I hope I'm reading this right but cat /sys/kernel/debug/gpioio gives me
...
gpio-575 (GPIO4 |spi0 CS2 ) out hi ACTIVE LOW
...
gpio-578 (GPIO7 |spi0 CS1 ) out hi ACTIVE LOW
gpio-579 (GPIO8 |spi0 CS0 ) out hi ACTIVE LOW
...
gpio-588 (GPIO17 |spi0 CS3 ) out hi ACTIVE LOW
...
gpio-593 (GPIO22 |spi0 CS4 ) out hi ACTIVE LOW

and I see
crw-rw---- 1 root spi 153, 3 Mar 21 11:17 /dev/spidev0.0
crw-rw---- 1 root spi 153, 4 Mar 21 11:17 /dev/spidev0.1
crw-rw---- 1 root spi 153, 2 Mar 21 11:17 /dev/spidev0.2
crw-rw---- 1 root spi 153, 1 Mar 21 11:17 /dev/spidev0.3
crw-rw---- 1 root spi 153, 0 Mar 21 11:17 /dev/spidev0.4
crw-rw---- 1 root spi 153, 5 Mar 21 11:17 /dev/spidev10.0

The board uses GPIO17 or GPIO22 which I believe correspond to address 3 and 4. However - setting address 3 gives me an NPE as there is no SpiChipselect value for that address.

Is there any reason why this isn't allowed?

@FDelporte
Copy link
Member

Historically I guess...
Could it be that 3 and up was not available pre-RPI5?
In that case, we should change the code to allow a higher value.

@douglm
Copy link
Author

douglm commented Mar 28, 2025

I rebuilt with up to CS_6 defined and my code now sees the state changes on the digital board.

From my perspective (50 years programming - java since the beginning - RPI since a couple of weeks ago) the SpiChipSelect thing adds a layer of confusion. Shouldn't this - in LinuxFsSpi:

String spiDev = SPI_DEVICE_BASE + config().bus().getBus() + "." + config().getChipSelect().getChipSelect();

be

String spiDev = SPI_DEVICE_BASE + config().bus().getBus() + "." + config().address();

SpiChipSelect introduces a limit on the addresses - my config was actually:

final var config = Spi.newConfigBuilder(pi4j)

// .address(address)
.channel(address)
.build();

because at first I thought channel and address were different things. Setting the address to 3 blew it up.
Having said that referring to chip select seems a better name - it's just the limits. This article:

https://www.analog.com/en/resources/analog-dialogue/articles/introduction-to-spi-interface.html

seems to suggest there's no limit (other than the number of pins)

@eitch
Copy link
Member

eitch commented Mar 29, 2025

From what i can see, there really isn't a reason to hardcode the possible SPI buses and addresses. Perhaps we can simply remove it, and allow to set the bus and address on the config, then use those two values. If the device doesn't exist, we'll get an error anyhow and future platforms with more buses will simply work.

@taartspi
Copy link
Collaborator

The pigpio code base does limit spi bus numbers as the underlying code base we use has that limit. I didn't think i limited the possible spi bus numbers in the linuxfs provider i did. When i am home i will look at my code to validate that comment

@taartspi
Copy link
Collaborator

taartspi commented Apr 4, 2025

So i was part of the linuxfs spi provider. I did the config object and decided to maintain the old parm names although yes what was the gpio number with pigpio is now the channel/chipselect. In the next year it is possible the providers will again be redone to abandon jni and jna. At that time we can consider changes you suggested as that work Will be a major release.
Apart from that using the limuxfs spi should allow you to use any existing spi number (bus) and address/channel. If you find that is not the case please document your config and error you encountered Tom

@douglm
Copy link
Author

douglm commented Apr 4, 2025

The problem is with this - in LinuxFsSpi:

String spiDev = SPI_DEVICE_BASE + config().bus().getBus() + "." + config().getChipSelect().getChipSelect();

which I think should be

String spiDev = SPI_DEVICE_BASE + config().bus().getBus() + "." + config().address();

The code as it stands now limits the addresses to those provided in SpiChipSelect - 0, 1 and 2. I extended SpiChipSelect to handle up to 6 but the change above would remove any limit.

My raspberry pi 5 - running an unmodified raspberry pi OS - has 0-5 as valid addresses - and I needed to address
/dev/spidev0.3

@taartspi
Copy link
Collaborator

taartspi commented Apr 5, 2025

Ah. Understood. That chipselect code has the old pigpio limitation in the code. I mistakenly thought the old Limits were only in the provider we replaced. I will do a fix for this and let you know when a snapshot is available and ask you to verify.

@douglm
Copy link
Author

douglm commented Apr 5, 2025

Great - by the way this is the change I made to SpiChipSelect:

public enum SpiChipSelect {
    CS_0(0), CS_1(1), CS_2(2), CS_3(3), CS_4(4), CS_5(5), CS_6(6);

I'd be inclined to add more just in case...

@eitch
Copy link
Member

eitch commented Apr 7, 2025

Perhaps the best idea is to completely remove the SpiChipSelect enum. We should just add a check before accessing the actual file and if it doesn't exist, then we can throw an appropriate exception. This way we stay open for other devices.

I do think we should limit our selves to what Linux offers, not the Raspberry Pi. The less such Raspberry Pi specific checks we have, the greater the chance that the code Just Works on other devices.

@taartspi
Copy link
Collaborator

taartspi commented Apr 7, 2025

Was looking at this last night. Pigpio has some limit checks in the provider and one check in chipselect. To keep chipselect generic it needs to change to allow many values. But there is a config value that takes the chipselect so we can't remove it without possibly breaking someones existing code. Was thinking make chipselect generic or say max of cs_10 and don't use it in pigpio but add this one additional check into its provider. Thoughts

@taartspi
Copy link
Collaborator

taartspi commented Apr 7, 2025

And linux Spi code just takes the bus and channel and tries an open. No enforcement. Other hand pigpio spi does a great deal of checks. Need to remember i did not use my spi code for pi5 but what was done by another so i will make sure the error message when the open fails contains enough detail

@douglm
Copy link
Author

douglm commented Apr 7, 2025

My inclination would be to make the change I outlined above:

String spiDev = SPI_DEVICE_BASE + config().bus().getBus() + "." + config().getChipSelect().getChipSelect();

which I think should be

String spiDev = SPI_DEVICE_BASE + config().bus().getBus() + "." + config().address();

Which removes the constraint, expand SpiChipSelect to cs_10 as you suggest and also deprecate SpiChipSelect. That way we lose the constraints Pi4j is making and don't break earlier code.

@eitch
Copy link
Member

eitch commented Apr 8, 2025

sounds good

@FDelporte
Copy link
Member

Fixed in version 3.0.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants