Bug 189772 - [NEW DRIVER] apuled(4): LED driver on PC Engines APU (1/2/3) boards
Summary: [NEW DRIVER] apuled(4): LED driver on PC Engines APU (1/2/3) boards
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Some People
Assignee: freebsd-bugs (Nobody)
URL: https://reviews.freebsd.org/D9876
Keywords: feature, needs-qa, patch
Depends on:
Blocks:
 
Reported: 2014-05-13 15:30 UTC by lab
Modified: 2020-09-03 14:37 UTC (History)
11 users (show)

See Also:


Attachments
apuled.tgz (4.51 KB, text/plain)
2014-05-13 15:30 UTC, lab
no flags Details
patch modified as diffs from current (11.0) (9.89 KB, patch)
2015-05-24 18:30 UTC, Keith White
no flags Details | Diff
Module to support LEDs on both APU1 and APU2 boards. (4.23 KB, application/octet-stream)
2017-01-17 15:47 UTC, lab
no flags Details
Updated APU led driver that support APU1, APU2, and APU3 (16.03 KB, text/plain)
2017-05-09 15:36 UTC, lab
no flags Details
APU2 DSDT, firmware v4.11.0.6 (228.08 KB, text/plain)
2020-08-18 09:29 UTC, Andriy Gapon
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description lab 2014-05-13 15:30:00 UTC
Driver for adding support for LEDs and mode switch on PC engines APU boards.
Can be built as a module or linked into kernel.
Upon loading the driver creates four devices.  Three led(4) devices:    
    /dev/led/led1                                                               
    /dev/led/led2                                                 
    /dev/led/led3                                       
                                                                                
One for the mode switch:
    /dev/modesw

Reading from /dev/modesw will return '1' or '0' depending upon if mode switch
is pressed or not.
Comment 1 olivier 2014-10-22 07:11:06 UTC
For compiling on latest -current, we just had to replace the getenv() by kern_getenv().
Comment 2 Keith White 2015-05-24 18:30:06 UTC
Created attachment 157102 [details]
patch modified as diffs from current (11.0)

Tested on APU.  Happy LEDs...
Comment 3 lab 2017-01-17 15:47:52 UTC
Created attachment 178995 [details]
Module to support LEDs on both APU1 and APU2 boards.

The updated patch has logic to detect if it is running on an APU1 or an APU2 board from PC Engines. It will then add support for all three LEDs and switch on the front of the box.
Comment 4 Olivier Cochard freebsd_committer 2017-03-03 12:43:03 UTC
I've created a phabricator review about this new drivers:
https://reviews.freebsd.org/D9876
Comment 5 lab 2017-05-09 15:36:18 UTC
Created attachment 182445 [details]
Updated APU led driver that support APU1, APU2, and APU3

Code in phabricator also updated.
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2018-05-20 23:50:29 UTC
For bugs matching the following conditions:
- Status == In Progress
- Assignee == "bugs@FreeBSD.org"
- Last Modified Year <= 2017

Do
- Set Status to "Open"
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-15 05:40:36 UTC
^Triage: It would be lovely to land this driver. The latest attachments needs:

- A test (validation) against CURRENT and a rebase if necessary
- Test confirmation against APU boards
Comment 8 Olivier Cochard freebsd_committer 2020-08-18 09:19:15 UTC
The code on the review was updated to be compliant on -head (https://reviews.freebsd.org/D9876).
Comment 9 Olivier Cochard freebsd_committer 2020-08-18 09:20:16 UTC
(In reply to Olivier Cochard from comment #8)

And it was tested on an APU2 (LEDs and reset switch).
Comment 10 Andriy Gapon freebsd_committer 2020-08-18 09:26:39 UTC
My personal opinion.

The gpio controller on APU2 is the standard AMD GPIO controller.
With the latest firmware it is advertised via ACPI, so amdgpu driver can attach perfectly well and gpioctl can be used to control the LEDs and query the switch.
Of course, that is not convenient at all.
But the latest firmware also describes the LEDs and the switch via ACPI.
So, we need some glue to parse the ACPI description and attach the standard gpioled and gpiokeys drivers under amdgpu.
That would be very similar to what is done on FDT based systems.

I think that such approach would be more extensible than hardcoding everything is a platform specific driver.
Comment 11 Andriy Gapon freebsd_committer 2020-08-18 09:29:48 UTC
Created attachment 217302 [details]
APU2 DSDT, firmware v4.11.0.6

See the attached DSDT for LEDS and BTNS elements.
Comment 12 Olivier Cochard freebsd_committer 2020-08-18 09:38:00 UTC
(In reply to Andriy Gapon from comment #10)

Yes your design idea seems cleaner than the existing code.

But:
- There is a working patch waiting to be committed since 2014;
- I don't know C & drivers coding: I've just adapted & tested the existing old patch, and it accidentally works.

So, who will be able to rewrite it (in less than other 6 years)?
Comment 13 Andriy Gapon freebsd_committer 2020-08-18 09:53:27 UTC
(In reply to Olivier Cochard from comment #12)
You are right, the perfect is the enemy of the good, of course.
I think that the current driver code can be a kmod port.
We have many useful kernel drivers in the ports. There is a much lower barrier to entry and more flexibility.  And for the end users it's almost as convenient.
Comment 14 Andriy Gapon freebsd_committer 2020-09-03 14:36:45 UTC
Created a review request for the suggested ACPI-based driver:
https://reviews.freebsd.org/D9876
Comment 15 Andriy Gapon freebsd_committer 2020-09-03 14:37:32 UTC
(In reply to Andriy Gapon from comment #14)
Pardon.  The correct link is: https://reviews.freebsd.org/D26311