gpioled_acpi: driver for GPIO controlled LEDs declared using ACPI _DSD
AcceptedPublic
Actions

Authored by avg on Sep 3 2020, 2:25 PM.

Details

Reviewers

gonzo
• ian
jhb
jkim
loos
manu
markj
mmel
olivier
wulf
cem

Summary

Some code in this driver should really be shared and belongs to acpi module.
But I want to wait for at least one other driver to appear that uses either
_DSD or GpioIo / GpioInt resources before sharing the corresponding code.

With just one driver it is hard to guess useful abstractions and KPI.

Test Plan

Tested in PC Engines APU2, firmware v4.11.0.6.

Verbose dmesg:

gpioleds0: <ACPI GPIO LEDs> on acpi0
gpioleds0: using pin 68 of gpio0 [\_SB_.PCI0.GPIO] (active low)
gpioleds0: added LED apu2:green:led1
gpioleds0: using pin 69 of gpio0 [\_SB_.PCI0.GPIO] (active low)
gpioleds0: added LED apu2:green:led2
gpioleds0: using pin 70 of gpio0 [\_SB_.PCI0.GPIO] (active low)
gpioleds0: ignored unknown property linux,default-trigger
gpioleds0: added LED apu2:green:led3

LED character devices:

$ ls -1 /dev/led
apu2:green:led1
apu2:green:led2
apu2:green:led3

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33360
Build 30669: arc lint + arc unit

Event Timeline

avg created this revision.Sep 3 2020, 2:25 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 3 2020, 2:25 PM

avg requested review of this revision.Sep 3 2020, 2:25 PM

Harbormaster completed remote builds in B33360: Diff 76609.Sep 3 2020, 2:25 PM

Related (to a degree) review request: D9876

Snippets from DSDT ASL:

    Scope (PCI0)
    {
        Device (GPIO)
        {
            Name (_HID, "AMD0030")  // _HID: Hardware ID
            Name (_CID, "AMD0030")  // _CID: Compatible ID
            Name (_UID, Zero)  // _UID: Unique ID
            Name (_DDN, "GPIO Controller")  // _DDN: DOS Device Name
            Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
            {
                Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, )
                {
                    0x00000007,
                }
                Memory32Fixed (ReadWrite,
                    0xFED81500,         // Address Base
                    0x00000300,         // Address Length
                    )
            })
            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0F)
            }
        }
    }


Scope (_SB.PCI0)
{
    Device (LEDS)
    {
        Name (_HID, "PRP0001")  // _HID: Hardware ID
        Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
        {
            GpioIo (Exclusive, PullUp, 0x0000, 0x0000, IoRestrictionOutputOnly,
                "\\_SB.PCI0.GPIO", 0x00, ResourceConsumer, ,
                )
                {   // Pin list
                    0x0044,
                    0x0045,
                    0x0046
                }
        })
        Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
        {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, 
            Package (0x01)
            {
                Package (0x02)
                {
                    "compatible", 
                    "gpio-leds"
                }
            }
        })
        Device (LED1)
        {
            Name (_HID, "PRP0001")  // _HID: Hardware ID
            Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
            {
                ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, 
                Package (0x03)
                {
                    Package (0x02)
                    {
                        "label", 
                        "apu2:green:led1"
                    }, 

                    Package (0x02)
                    {
                        "gpios", 
                        Package (0x04)
                        {
                            LEDS, 
                            Zero, 
                            Zero, 
                            One
                        }
                    }, 

                    Package (0x02)
                    {
                        "default-state", 
                        "on"
                    }
                }
            })
        }

        Device (LED2)
        {
            Name (_HID, "PRP0001")  // _HID: Hardware ID
            Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
            {
                ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, 
                Package (0x03)
                {
                    Package (0x02)
                    {
                        "label", 
                        "apu2:green:led2"
                    }, 

                    Package (0x02)
                    {
                        "gpios", 
                        Package (0x04)
                        {
                            LEDS, 
                            Zero, 
                            One, 
                            One
                        }
                    }, 

                    Package (0x02)
                    {
                        "default-state", 
                        "off"
                    }
                }
            })
        }

        Device (LED3)
        {
            Name (_HID, "PRP0001")  // _HID: Hardware ID
            Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
            {
                ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, 
                Package (0x03)
                {
                    Package (0x02)
                    {
                        "label", 
                        "apu2:green:led3"
                    }, 

                    Package (0x02)
                    {
                        "gpios", 
                        Package (0x04)
                        {
                            LEDS, 
                            Zero, 
                            0x02, 
                            One
                        }
                    }, 

                    Package (0x02)
                    {
                        "linux,default-trigger", 
                        "heartbeat"
                    }
                }
            })
        }
    }
}

I don’t have a lot of availability right now, sorry.

Overall it looks OK to me.

This revision is now accepted and ready to land.Sep 7 2020, 8:34 PM

I get currently the following reply on stable/13 with an APU2 bios 4.13.0.4:
gpioleds0: <ACPI GPIO LEDs> on acpi0
gpioleds0: GPIO controller device \134_SB_.PCI0.GPIO is not attached
gpioleds0: GPIO controller device \134_SB_.PCI0.GPIO is not attached
gpioleds0: GPIO controller device \134_SB_.PCI0.GPIO is not attached
gpioleds0: detached

@mm , do you have amdgpio driver? Has it attached?

In D26311#676085, @avg wrote:

@mm , do you have amdgpio driver? Has it attached?

Thanks, I have missed that. Are there any specific reasons why this wasn't merged? Should we offer this as a port instead?

The only reason is that I thought that I could do some things better but never got around to actually working on them.
Specifically, I don't like having so many walk+callback patterns. Especially for _DSD.
I hoped that I would be able to represent it as an nvlist or something similar.

avg mentioned this in D47253: GPIO: Add ACPI _AEI support.Fri, Nov 1, 3:03 PM