Kernel driver code which honored FDT variable

A

Alexander Mishin

I wrote a couple of kernel drivers for GPIO devices which code uses a conditional inclusion based on an FDT flag.
I use it for a device definition using either FDT overlays or a device.hints file.
C: 
#ifdef FDT
    if ((err = rcrecv_fdt_setup_pin(sc)) == 0)
      rcrecv_fdt_get_params(sc);
#else
    err = ENOENT;
#endif
    if (err != 0) {

Just now I place a string "#define FDT" right to the top of a main C-file (or comment it).
Is it a correct way to set that flag?
Maybe there is a way to inherit somehow the FDT value actual for my system, at compile time?

Thanks
 
I think that looking at the source code helps. OneWire contains some good code to study.
/usr/src/sys/dev/ow/owc_gpiobus.c
So first question. Are you running this code on i386/amd64 or Arm. Platform agnostic?

The OneWire driver has a good example.
Some background: Warner created OW drivers for Arm. Ian updated it for wider use.
FreeBSD 11 contains the older Arm only code.
FreeBSD 12 contained platform agnostic coding.

⚙ D22710 Update owc_gpiobus (one-wire over gpio) to the modern gpio_pin interface.

reviews.freebsd.org reviews.freebsd.org
 
My system is arm, specifically Orange PI PC. OS is FreeBSD 13.0-STABLE #11 stable/13-n246687-18ea5bc166c1: Sat Aug 7 09:35:14 +04 2021.

owc_gpiobus.c is one of a source I based my modules code on.

The code itself, depending on a manually defined (or undefined) FDT, works fine (Though, I am the only tester).
I think also it will be better when an FDT capability of the system was automatically determined. But I can't see
how and where the owc_gpiobus.c did so.

It is not a onewire device, if that matters. It is a 433 MHz RC receiver (one pin with interrupts)
 
It seems that I found the answer to my question but this does not solved my problem.
An FDT variable is declared in a file /usr/obj/usr/src/arm.armv7/GENERIC/opt_platform.h, i.e in the ${KERNBUILDDIR} folder.
The file contents is honored in the building kernel process.
But the file opt_platform.h automatically generated in the project folder is stably empty and I don't see how to easy use the original file here.
 
Alexander Mishin said:
Maybe there is a way to inherit somehow the FDT value actual for my system, at compile time?
Click to expand...
I am not a coder but I can read code. So please take my questions lightly.
I am unsure what FDT value or capability you are referring to in these comments.
Can you give an example for an idiot like me..
I use Device Tree Overlays for accessories using sysutils/rpi-firmware
So I am aware of dtc and fragments and phandles.
Alexander Mishin said:
I think also it will be better when an FDT capability of the system was automatically determined. But I can't see
how and where the owc_gpiobus.c did so.
Click to expand...
 
Sorry, I do not exactly understand what an example You ask me about...

I have a FreeBSD 13 running on an "Orange PI PC" (architecture: arm, cpu: Allwinner h3).
In the source codes FreeBSD I often see a construction I mentioned at my first message.
It uses to determine if the architecture can be configured by DTS (i.e. architecture is FDT capable, has a "simplebus") or by /boot/device.hints file
(aka "hinted configuration", "gpiobus" must be used by the driver).

So...
When the construction is used by kernel build process all is ok, as in that case a string "#include <opt_platform.h>" uses a file /usr/obj/usr/src/arm.armv7/GENERIC/opt_platform.h,
it contains platform depended definitions (and "#define FDT" in my case).
When I build my own project the same inclusion string uses ./opt_platform.h from the project folder but that file is automatically created empty, w/o anything at all.
I.e. a programmer to build my project must made changes to main C-file, specifically either to write "#define FDT" at the top of a main C-file or on the contrary delete it.
My question was if this can be avoided.

...or You ask me for link to my source files as examples?
 
What build tool do you use, if any? It's easy with cmake or automake.
Regardless why can't it be specified in CFLAGS environment variable which is used by {gc}make or just add -DFDT to the compile line if you're doing it manually?
Or am I missing something?
 
mark_j said:
Or am I missing something?
Click to expand...
No, It seems It is me who was "missing something", If I am repeatedly misunderstood on the forum.
It is me who can't correctly ask a question.

I use clang and Makefile.
All builds ok. All even runs ok, compilled with and without FDT flag (I praise myself so much :)).

I believe that FreeBS knows perfectly well whether it runs on FDT capable platform or not.
I'd prefer to make such changes to Makefile, which ones would be automatically create opt_platform.h in the project's folder, with or without #define FDT string, accordingly to this knowledge, instead of a creating an empty one.

That is my Makefile:
Code: 
# $FreeBSD$

KMOD=rcrecv
SRCS=rcrecv.c
SUBDIR=include man

SRCS+= \
        bus_if.h device_if.h gpio_if.h gpiobus_if.h \
        ofw_bus_if.h opt_platform.h fdt_pinctrl_if.h \

CFLAGS+=  -I.

.include <bsd.kmod.mk>
 
I found a solution elegant enough for me.

/sbin/sysctl hw.fdt.dtb returns 0 if DTS detected and 1 otherwise.

Part of Makefile:
Code: 
...

CFLAGS+=  -I.

beforedepend:
        @-/sbin/sysctl hw.fdt.dtb && echo "#define FDT 1" > opt_platform.h

.include <bsd.kmod.mk>
After make is done:
Code: 
$ cat opt_platform.h 
#define FDT 1
What leads to a correct compilation result at the end

Thank you all!
 
Yea I deleted that post. That flag is used for arm image building and crosscompiling.
Enabled by default on Arm so un-needed when compiling there.
 
No, the solution found by me is not as elegant as I thought before, not good at all.
Especially bad for cross compiling.
I am looking on WITH_FDT, /etc/src.conf but it seems that WITH_x and WITHOUT_x variables is ignored outside of the kernel (or world) build process.
Now I am looking for how to turn them honored on for build kernel modules outside the source tree.
 
mark_j said:
Well yes and no. I was addressing OP's need to see FDT in the compilation to enable something.
Click to expand...
I do so.
I just want that my Makefile was smart enough for enable FDT for FDT capable systems but give the opportunity to change it for cross compiling.
I see several entry points for define FDT for project:
  • Makefile itself (if it is the "smart enough" one);
  • opt_platform.h in the project's directory (created empty every time);
  • main C-file, worse of all (as it is now).
Or somehow take it from kernel sources, to be more precise, from /usr/obj if it is system's opt_platform.h.
On arm PC I often mount /usr/src for a kmod build (by nfs from big brother's zfs-snapshot), but to mount additionally /usr/obj for an only variable and, even more, to await this from users... this is a bit thick.
 
So, as I understand you, your problem now is cross-compiling where you don't have access to the kernel to see if your sysctl hack works?

If you're cross-compiling, you don't see that kernel (with the FDT) and therefore your test will fail? Correct?

So you would either have to settle for manually inputting the FDT define into your code/makefile (using CFLAGS I suppose) or only compile on the system you're focusing on and use your hack of using sysctl.

The only thing I can think of, is at runtime to call sysctl(3) (or probably better to use sysctlbyname(3)) with the MIB for hw.fdt.dtb and set an FDT flag accordingly.

Obviously, I don't know how this affects your code, having only seen a snippet, but could it be a solution that works regardless?
 
mark_j said:
So, as I understand you, your problem now is cross-compiling where you don't have access to the kernel to see if your sysctl hack works?
Click to expand...
Right now after hack, yes
If you're cross-compiling, you don't see that kernel (with the FDT) and therefore your test will fail? Correct?
Click to expand...
Not exactly, will not fail. Test just will be wrong as it will set (or will not set) FDT flag accordingly with FDT capablity of the host platform (not the target one)
So you would either have to settle for manually inputting the FDT define into your code/makefile (using CFLAGS I suppose) or only compile on the system you're focusing on and use your hack of using sysctl.
Click to expand...
Yes. Do not see other variants.
Obviously, I don't know how this affects your code, having only seen a snippet, but could it be a solution that works regardless?
Click to expand...
If I would not use sysctlbyname(3) no affects will be needed.

Right now I think that I remove a definition of FDT from code/Makefile and add a description "how to install" to README.md. Something about:
Code: 
make depend
# If Your platform haven't DTS enabled skip next line (keep opt_platform.h empty)
echo "#define FDT 1" > opt_platform.h
make
sudo make install

To keep Makefile clear (Someday I'll be ripe to propose to add my drivers in the base :). Not before I find a tester for the arm64 platform)

Thank you for help
 
I know you've resolved this, but I was thinking a little about this problem.
There are probably a couple of ways around this:
1. Place the code in contrib in the source tree and edit the contrib tree makefile. That will use the WITH_FDT.
2. Use the ifunc feature of clang. I think FreeBSD has full support for it (though perhaps not on all platforms). This will get around the #define by performing it dynamically at runtime. If you've never used ifunc, I can provIde some links for usage.
Usage of ifunc feature does depend on your code, because I've only seen a snippet of your code.
 
mark_j said:
1. Place the code in contrib in the source tree and edit the contrib tree makefile. That will use the WITH_FDT.
2. Use the ifunc feature of clang. I think FreeBSD has full support for it (though perhaps not on all platforms). This will get around the #define by performing it dynamically at runtime. If you've never used ifunc, I can provIde some links for usage
Click to expand...
I would be grateful for links, even if they are not suitable for solving this problem. This is the first time I hear about these options and would like to understand better what it is about.
mark_j said:
...because I've only seen a snippet of your code.
Click to expand...
tm1637 (gpio), bt1750 (i2c), MX-RM-5V rc receiver (gpio) - kernel drivers with daemons for them, written for my home projects (since I did not find ready-made solutions), all code under the BSD 2-Clause License on gitlab. If you are interested, I can send links.
 
No problems Alexander. Give me a few hours (it's night here) and I will dig up the links for it tomorrow morning.
If I haven't done it in a few days, reply back here so I get an email and that will jolt my memory.

I'm busy and sometimes forget stuff when I get to work... and it's just my home office at the moment. :rolleyes: Just in case you think I'm ignoring you. ;)
 
Ok, here's what I could gather up.
This has the first few paragraphs that sum up ifunc nicely, I think: http://maskray.me/blog/2021-01-18-gnu-indirect-function
Also here.

The clang implementation of ifunc is documented here.
The GNU implementation (the first to do it back in 2004? or thereabouts) is here.

FreeBSD apparently has ifunc implemented for aarch64, ppc64 and amd64 and possibly others. I definitely found the changes for arm64 here and here. A detailed usage of ifunc is in the pfil(9) set of kernel functions. If it interests you, they're a good place to scan the source code.

This is a good example from this book, which is actually a really good read and reference book. It's called "Power and Performance: Software Analysis and Optimization" by Jim Kukunas. My work copy is dated 2015. There may be newer, dealing more with clang.

C: 
# include <stddef.h>

extern void foo(unsigned *data, size_t len);

void foo_c (unsigned *data, size_t len)
{
    /* ... */
}

void foo_sse42 (unsigned *data, size_t len)
{
    /* ... */
}

void foo_avx2 (unsigned *data, size_t len)
{
    /* ... */
}

extern int cpu_has_sse42(void);
extern int cpu_has_avx2(void);

void foo(unsigned *data, size_t len)
            __attribute__((ifunc ("resolve_foo")));

static void *resolve_foo(void)
{
        if (cpu_has_avx2())
                return foo_avx2;
        else if (cpu_has_sse42());
                return foo_sse42;
        else
                return foo_c;
}
 
Thank you very much for links! Have read some...

But I think that it is much simpler to operate with #ifdef FDT for this is only two variants in there, a simplebus (fdt) and a gpiobus (device.hints).
Just now I meditate on /usr/src/sys/conf/kern.opts.mk, its very last lines... it looks like I am completely confused with these variables: FDT, OPT_FDT, WITHOUT_FDT, trying to figure out which one is after which.
 
That's cool, whatever works for you. You can never have too many ways to solve 1 problem. ;)

Regarding kern.opts.mk, are you referring to this?
Code: 
# Some modules only compile successfully if option FDT is set, due to #ifdef FDT
# wrapped around declarations.  Module makefiles can optionally compile such
# things using .if !empty(OPT_FDT)
.if !defined(OPT_FDT) && defined(KERNBUILDDIR)
OPT_FDT!= sed -n '/FDT/p' ${KERNBUILDDIR}/opt_platform.h
.export OPT_FDT
.endif

If you look in /usr/src/sys/conf/config.mk and /usr/src/sys/conf/kmod.mk they explain the rationale (somewhat). This allows you to set FDT per makefile.

For example, look in /usr/src/sys/modules/i2c/iicbus/Makefile:

Code: 
.if !empty(OPT_FDT)
SRCS+=  ofw_iicbus.c ofw_bus_if.h
.endif

Your issue really is getting the code into the tree. Perhaps look at this:
docs.freebsd.org

Chapter 5. Source Tree Guidelines and Policies

Source Tree Guidelines and Policies
docs.freebsd.org docs.freebsd.org
 
I placed my rcrecv-kmod/ to /usr/src/sys/contrib/, a slightly modified Makefile to /usr/src/sys/modules/ and try to compile and cross-compile it by different ways. By now I have no success with it - when I build the kmod, the FDT variable is undefined, nether on a arm platform nor on a amd64 one with TARGET_ARCH=armv7. I am still interested in this question, but only in terms of self education - as for the installation procedure, I think, it will be much easier for people if all steps are accurately described in the README.md, w/o all that.

Thank you all.
 
You must log in or register to reply here.
Back
Top