Mary! On Wed, Aug 14 2024 at 14:47, Mary Strodl wrote: So this caught the attention of my mail filter because the subsystem prefix is 'x86:' while it obviously should be 'mfd:' > The Congatec CGEB is a BIOS interface found on some Congatec x86 > modules. It provides access to on board peripherals like I2C busses > and watchdogs. This driver contains the basic support for accessing > the CGEB interface and registers the child devices. > > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > Signed-off-by: Christian Gmeiner <christian.gmeiner@xxxxxxxxx> > Signed-off-by: Mary Strodl <mstrodl@xxxxxxxxxxx> This Signed-off-by chain is invalid. See Documentation/process/ Aisde of that. How is any of this supposed to work. What is the user mode helper doing and why does this need this netlink indirection? And looking at that reference implementation on github makes me just shudder: board.mem_fd = open("/dev/mem", O_RDONLY); and then it allocates anon memory: board->code = mmap(NULL, msg->code.length, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); which it hands back to the kernel so the kernel can copy stuff into it followed by some other command: mprotect(board->code, msg->code.length, PROT_EXEC | PROT_READ | PROT_WRITE) Seriously? RWX mappings? So this copies a code blob out of the BIOS region into user space and lets user space use this blob to interact with the BIOS extension, right? So an i2c transfer does a full round trip: i2c::xfer -> cgeb -> netlink -> user helper -> execute random code -> user helper -> netlink -> cgeb -> ... right? Has anyone tried to analyze what this BIOS provided code blob is actually doing? All it does is to poke at a range of IOPORTS with in*(), out*() and that poking mechanism depends on the generation of that CGEB implementation. Congatec even provides the GPL2 licenced source for this pokery as a kernel driver. Three generations of interfaces and for each the poking is about 200 lines of unreadable, malformated gunk, which can be probably condensed to 100 lines of readable kernel code for each generation. Add a bunch of helpers which set up the various transfers for the subdevices and you can spare all this horrible nonsense with user mode helper, executing random BIOS provided gunk, netlink and completely ill defined data structures. Thanks, tglx