mbox series

[v4,0/2] Add support for Congatec CGEB BIOS interface

Message ID 20240814184731.1310988-1-mstrodl@csh.rit.edu
Headers show
Series Add support for Congatec CGEB BIOS interface | expand

Message

Mary Strodl Aug. 14, 2024, 6:47 p.m. UTC
The following series adds support for the Congatec CGEB interface
found on some Congatec x86 boards. The CGEB interface is a BIOS
interface which provides access to onboard peripherals like I2C
busses and watchdogs. It works by mapping BIOS code and searching
for magic values which specify the entry points to the CGEB call.
The CGEB call is an API provided by the BIOS which provides access
to the functions in an ioctl like fashion.

At the request of some folks the first time I sent this series out,
CGEB has a userspace component which runs the x86 blob (rather than
running it directly in the kernel), which sends requests back and
forth using the cn_netlink API.

You can find a reference implementation of the userspace helper here:
https://github.com/Mstrodl/cgeb-helper

I didn't get an answer when I asked where the userspace component
should live, so I didn't put a ton of work into getting the helper
up to snuff since similar userspace helpers (like v86d) are not
in-tree. If folks would like the helper in-tree, that's fine too.

Changelog:
v2:
  * Moved CGEB code snippet execution into userspace
v3:
* `checkpatch` pass
  * Should I add the driver files to MAINTAINERS? I'm not sure what
    the norm is there...
* `sparse` pass
  * I'm not sure there's a good way to keep the __iomem marker
    around while not making the struct fields really inconvenient
    to access, so I just cast them away which causes a sparse
    warning. I figure it's probably okay since this driver
    is x86-specific anyways? Let me know if this is an issue and
    what the preferred approach is. 
v4:
  * Sort includes
  * Remove GPL blurb from top of file
  * Use memremap instead of ioremap_cache, avoids sparse warning
  * Minor styling stuff
  * Relocated some static declarations into function bodies

This series is based on the excellent work of Sascha Hauer and
Christian Gmeiner. You can find their original work here:

http://patchwork.ozlabs.org/patch/219756/
http://patchwork.ozlabs.org/patch/219755/
http://patchwork.ozlabs.org/patch/219757/

http://patchwork.ozlabs.org/patch/483262/
http://patchwork.ozlabs.org/patch/483264/
http://patchwork.ozlabs.org/patch/483261/
http://patchwork.ozlabs.org/patch/483263/

Mary Strodl (1):
  x86: Add basic support for the Congatec CGEB BIOS interface

Sascha Hauer (1):
  i2c: Add Congatec CGEB I2C driver

 drivers/i2c/busses/Kconfig             |   10 +
 drivers/i2c/busses/Makefile            |    1 +
 drivers/i2c/busses/i2c-congatec-cgeb.c |  190 ++++
 drivers/mfd/Kconfig                    |   10 +
 drivers/mfd/Makefile                   |    1 +
 drivers/mfd/congatec-cgeb.c            | 1125 ++++++++++++++++++++++++
 include/linux/mfd/congatec-cgeb.h      |  112 +++
 include/uapi/linux/connector.h         |    4 +-
 8 files changed, 1452 insertions(+), 1 deletion(-)
 create mode 100644 drivers/i2c/busses/i2c-congatec-cgeb.c
 create mode 100644 drivers/mfd/congatec-cgeb.c
 create mode 100644 include/linux/mfd/congatec-cgeb.h

Comments

Thomas Richard Aug. 20, 2024, 8:39 a.m. UTC | #1
On 8/14/24 20:47, Mary Strodl wrote:
> The following series adds support for the Congatec CGEB interface
> found on some Congatec x86 boards. The CGEB interface is a BIOS
> interface which provides access to onboard peripherals like I2C
> busses and watchdogs. It works by mapping BIOS code and searching
> for magic values which specify the entry points to the CGEB call.
> The CGEB call is an API provided by the BIOS which provides access
> to the functions in an ioctl like fashion.
> 
> At the request of some folks the first time I sent this series out,
> CGEB has a userspace component which runs the x86 blob (rather than
> running it directly in the kernel), which sends requests back and
> forth using the cn_netlink API.
> 
> You can find a reference implementation of the userspace helper here:
> https://github.com/Mstrodl/cgeb-helper
> 
> I didn't get an answer when I asked where the userspace component
> should live, so I didn't put a ton of work into getting the helper
> up to snuff since similar userspace helpers (like v86d) are not
> in-tree. If folks would like the helper in-tree, that's fine too.

Hello Mary !!

It was by pure luck that I found your series.

It seems we are working on the same thing, the Congatec Board Controller.

I sent a first version of my series few weeks ago [1].
My implementation is very different.
There is an MFD which maps the needed IO regions and declares cells
(gpio, watchdog, i2c). It also contains all the code to communicate with
the Board Controller (using ioread and iowrite).
The DMI table is used to detect if the board is supported, so the driver
can be probed (or not).
Other drivers (gpio, i2c and watchdog for now) use the API provided by
the MFD to communicate with the Board Controller.
With this approach, I don't need a userspace component.

For this work I used the Congatec driver (that Thomas Gleixner pointed
you) as reference. But as mentioned by Thomas the driver is not well
written at all.
By the way their latest version is available in their yocto metalayer
(please find the link in my series).

For now I did the job for the conga-SA7 board (which has a 5th
generation Board Controller).
So if you have hardware which has the same generation of the Board
Controller, you can easily test the series. You only need to add the DMI
entry of your board in the MFD driver.

For other generations, I had a quick look. The sequences seem very
similar, with minor differences. It could be easily implemented in the
future, and only the MFD will be modified.

Please feel free to test/comment my series [1].

[1]
https://lore.kernel.org/all/20240503-congatec-board-controller-v1-0-fec5236270e7@bootlin.com/

Best Regards,

Thomas
Christian Gmeiner Aug. 20, 2024, 8:48 a.m. UTC | #2
Hi Thomas!

>
> On 8/14/24 20:47, Mary Strodl wrote:
> > The following series adds support for the Congatec CGEB interface
> > found on some Congatec x86 boards. The CGEB interface is a BIOS
> > interface which provides access to onboard peripherals like I2C
> > busses and watchdogs. It works by mapping BIOS code and searching
> > for magic values which specify the entry points to the CGEB call.
> > The CGEB call is an API provided by the BIOS which provides access
> > to the functions in an ioctl like fashion.
> >
> > At the request of some folks the first time I sent this series out,
> > CGEB has a userspace component which runs the x86 blob (rather than
> > running it directly in the kernel), which sends requests back and
> > forth using the cn_netlink API.
> >
> > You can find a reference implementation of the userspace helper here:
> > https://github.com/Mstrodl/cgeb-helper
> >
> > I didn't get an answer when I asked where the userspace component
> > should live, so I didn't put a ton of work into getting the helper
> > up to snuff since similar userspace helpers (like v86d) are not
> > in-tree. If folks would like the helper in-tree, that's fine too.
>
> Hello Mary !!
>
> It was by pure luck that I found your series.
>
> It seems we are working on the same thing, the Congatec Board Controller.
>
> I sent a first version of my series few weeks ago [1].
> My implementation is very different.
> There is an MFD which maps the needed IO regions and declares cells
> (gpio, watchdog, i2c). It also contains all the code to communicate with
> the Board Controller (using ioread and iowrite).
> The DMI table is used to detect if the board is supported, so the driver
> can be probed (or not).
> Other drivers (gpio, i2c and watchdog for now) use the API provided by
> the MFD to communicate with the Board Controller.
> With this approach, I don't need a userspace component.
>
> For this work I used the Congatec driver (that Thomas Gleixner pointed
> you) as reference. But as mentioned by Thomas the driver is not well
> written at all.
> By the way their latest version is available in their yocto metalayer
> (please find the link in my series).
>

I'm glad to see that Congatec has taken a different approach with
their driver and
is no longer relying on the BIOS blob method. Years ago, I shared my
frustrations
with them over this outdated solution through numerous phone calls,
but the company
I worked for was simply too small to have any real influence.

> For now I did the job for the conga-SA7 board (which has a 5th
> generation Board Controller).
> So if you have hardware which has the same generation of the Board
> Controller, you can easily test the series. You only need to add the DMI
> entry of your board in the MFD driver.
>
> For other generations, I had a quick look. The sequences seem very
> similar, with minor differences. It could be easily implemented in the
> future, and only the MFD will be modified.
>

Fantastic news - looks like all the pain with this board controller
under Linux is gone soon.