Added support for FreeBSD

[0xc0decafe]

As developed and designed for use on RBPi, all read and write ops are
implemented using the I2CRDWR syscall, it beeing the only one implemented
in the bcm2835_bsc driver.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[kplindegaard]

Hi @0xc0decafe and thanks for your PR :)

Unfortuantely I don't have time to review it right now, but I will come back with comments later. Doing some minor maintenance on the CI setup and stuff first. The plan is to get 0.4.2 out so that everything is as up to date as possible first.

[brd]

@kplindegaard ping? :)

[sanpei3]

Thank you for your effort.
I cannot use this change on Raspberry PI 3(armv7).
I got the below message[NG].
File "/tmp/smbus2.py", line 731, in i2c_rdwr
ioctl(self.fd, I2CRDWR, ioctl_data)
OSError: [Errno 25] Inappropriate ioctl for device

On armv7(Raspberry PI-PI3), it should be below.
I2CRDWR = 0x80086906

We should change this value on the CPU arch/hardware platform.

[0xc0decafe]

Well, that's odd. I have a couple of systems here, and running the following code:

#include <stdio.h>
#include <stdint.h>
#include <sys/types.h>
#include <dev/iicbus/iic.h>

int main() {
        printf("%lx\n", I2CRDWR);
        return 0;
}

gives me the following results:

# uname -a
FreeBSD amd64-system 13.2-RELEASE-p2 FreeBSD 13.2-RELEASE-p2 GENERIC amd64
# ./test
80106906

# uname -a
FreeBSD arm64-system 13.2-RELEASE-p4 FreeBSD 13.2-RELEASE-p4 GENERIC arm64
# ./test 
80106906

unfortunately i don't have an armv7 system here to test on, but your raspi3 will run aarch64 (;
Are you sure you are using the correct device?

best

[sanpei3]

I have three types of Raspberry PI(RPI-zero, RPI3, RPI4).

I tested some devices, including amd64 and cloud.

64bit CPU's I2CRDWR is 80106906

• 13.2-RELEASE-p4 GENERIC amd64(Intel CPU)
• 12.4-RELEASE-p6 amd64(Goolgle Cloud Platform)
• 14.0-CURRENT arm64(Pine64)
• 13.2-RELEASE-p4 GENERIC arm64 (Oracle Cloud)
  (This time, I did not test with RPI4)

32bit CPU's I2CRDWR is 80086906

• 14.0-CURRENT arm (PRI-3)
• 13.2-RELEASE arm (PRI ZERO)

[0xc0decafe]

So smth. like this could work:

import platform
(bits, _) = platform.architecture()
I2CRDWR = { '64bit': 0x80106906, '32bit': 0x80086906 }[bits]

[0xc0decafe]

now i have one question left: whats about i386 systems? they also use 0x80086906 ? I don't have any 32bit systems run anymore. Can anyone here test this?

[sanpei3]

I checked I2CRDWR on i386 systems.
All system's I2CRDWR is 0x80086906[OK]

0x80086906
FreeBSD freebsd-i386 13.2-RELEASE FreeBSD 13.2-RELEASE releng/13.2-n254617-525ecfdad597 GENERIC i386
0x80086906
FreeBSD FreeBSD-15-i386 15.0-CURRENT FreeBSD 15.0-CURRENT #0 main-n265904-bb679b0c4909: Thu Oct 12 02:53:29 UTC 2023 [email protected]:/usr/obj/usr/src/i386.i386/sys/GENERIC i386

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[kplindegaard]

Hi there and thanks for your interest in this library and extending it to support FreeBSD. I have no experience with FreeBSD at all myself.

What I would like to request is a more clear separating between "standard" behavior and required tweaks for FreeBSD. What I have in mind more specifically is to eliminate all the if system() == 'FreeBSD' statements injected everywhere. Imho that makes the code less maintainable and messy.

One approach to achieve this with relatively little effort involves two things:

1. Make a subclass of SMBus with all the FreeBSD changes. Let's call that new subclass SMBusFreeBSD.
2. In the __enter__() method of the now parent class SMBus you implement the logic checking for architecture once and for all. If it's FreeBSD, you return a new instance of SMBusFreeBSD and if not return self.

Something like this:

class SMBus:
    def __init__(self, bus):
        self.bus = bus

    def __enter__(self):
        if system() == 'FreeBSD:
            return SMBusFreeBSD(self.bus)
        else:
            return self

class SMBusFreeBSD(SMBus):
    def __init__(self, bus):
        # You may have to invoke the parent constructor, don't remember.
        SMBus.__init__(self, bus)
        # Do other required stuff here

    # Overload the methods you need to
    def write_byte(self, i2c_addr, value, force=None):
        msg = i2c_msg.write(i2c_addr, value)
        self.i2c_rdwr(msg)

    # ....

There are probably other factory-like patterns that are more elegant, but at the end of the day we have to ensure backwards compatibility and, my agenda here today, better maintainability through separation.

[0xc0decafe]

Hi @kplindegaard, thanks for your suggestions.
How about you put the generic changes into the code base, the design then can be to your pleasing.
After that I'll come back and add the FreeBSD specific implementation.
After all I'm doing this in my free time to the best of the community, and I really don't want a lengthy back and forth over design decisions.
Best