linux-kernel - Re: [PATCH] PCI: hotplug: Add extension driver for Ampere Altra hotplug LED control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230928212627.GA502803@bhelgaas>
Date:   Thu, 28 Sep 2023 16:26:27 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     D Scott Phillips <scott@...amperecomputing.com>
Cc:     linux-acpi@...r.kernel.org, linux-pci@...r.kernel.org,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Len Brown <lenb@...nel.org>, linux-kernel@...r.kernel.org,
        Darren Hart <darren@...amperecomputing.com>,
        patches@...erecomputing.com
Subject: Re: [PATCH] PCI: hotplug: Add extension driver for Ampere Altra
 hotplug LED control

On Thu, Sep 28, 2023 at 02:02:07PM -0700, D Scott Phillips wrote:
> Bjorn Helgaas <helgaas@...nel.org> writes:
> > On Wed, Sep 27, 2023 at 01:23:47PM -0700, D Scott Phillips wrote:
> >> On Ampere Altra, PCIe hotplug is handled through ACPI. A side interface is
> >> also present to request system firmware control of attention LEDs. Add an
> >> ACPI PCI Hotplug companion driver to support attention LED control.

> >> +config HOTPLUG_PCI_ACPI_AMPERE_ALTRA
> >> +	tristate "ACPI PCI Hotplug driver Ampere Altra extensions"
> >> +	depends on HOTPLUG_PCI_ACPI
> >> +	depends on HAVE_ARM_SMCCC_DISCOVERY
> >> +	depends on m
> >
> > Why is this restricted to being a module?  It's not unprecedented, but
> > unless this only works as a module for some reason, I would leave that
> > choice up to the user.
> 
> I did that because acpiphp_register_attention() wouldn't register the
> handler unless it was built as a module. Maybe better would be this
> change first:
> 
> --- a/drivers/pci/hotplug/acpiphp_core.c
> +++ b/drivers/pci/hotplug/acpiphp_core.c
> @@ -78,8 +78,7 @@ int acpiphp_register_attention(struct acpiphp_attention_info *info)
>  {
>  	int retval = -EINVAL;
>  
> -	if (info && info->owner && info->set_attn &&
> -			info->get_attn && !attention_info) {
> +	if (info && info->set_attn && info->get_attn && !attention_info) {
>  		retval = 0;
>  		attention_info = info;
>  	}

I would investigate the history of that "info->owner" check to see if
it's required somewhere.  If not, it seems like we could drop it.

At one time, we did support building hotplug drivers, including
acpiphp, as modules, but we no longer do.  Maybe that test dates from
that time.

Bjorn