Linux-LEDs Archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
	Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Cc: Stuart Hayes <stuart.w.hayes@gmail.com>,
	Dan Williams <dan.j.williams@intel.com>,
	<linux-pci@vger.kernel.org>, <linux-leds@vger.kernel.org>,
	Lukas Wunner <lukas@wunner.de>
Subject: Re: [PATCH 2/2] PCI/NPEM: Add Native PCIe Enclosure Management support
Date: Fri, 22 Mar 2024 14:30:03 -0700	[thread overview]
Message-ID: <65fdf85bba0e0_4a98a294b2@dwillia2-mobl3.amr.corp.intel.com.notmuch> (raw)
In-Reply-To: <20240322195612.GA1372991@bhelgaas>

Bjorn Helgaas wrote:
> On Tue, Mar 12, 2024 at 10:08:16AM +0100, Mariusz Tkaczyk wrote:
> > On Mon, 11 Mar 2024 17:30:06 -0500
> > Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
> > 
> > > > > No, Linux doesn't support _DSM. It was proposed in previous
> > > > > iterations by Stuart but I dropped it. We decided that it need to be
> > > > > strongly rebuild because "pci/pcie" is not right place for ACPI code
> > > > > so we cannot register _DSM driver instead of NPEM as it was proposed
> > > > > and I don't have _DSM capable hardware to test it.  
> > > 
> > > I'm not sure I understand why pci/pcie isn't the right place for ACPI code--
> > > there are other _DSMs used in PCI code already, and this _DSM is defined
> > > in a PCI ECN.
> > 
> > I looked into internal review history and I found out that I dropped it after
> > discussion with Dan Williams:
> > 
> > > After review and discussion with Dan _DSM extension is dropped.
> > 
> > Unfortunately, I don't remember what exactly he suggested, I just remembered
> > conclusion that it needs to be reworked and I decided to drop it.
> > Maybe, I didn't understand him correctly.
> > 
> > Dan, could you take a look? Do you remember something?
> 
> Straw man proposal:
> 
>   - Update this patch so we use NPEM if the device advertises it.
> 
>   - If/when Linux support for the _DSM is added, we use the _DSM when
>     present.  If a device advertises NPEM but no _DSM applies to it,
>     we use native NPEM for it.

The current patch matches my last recollection of the discussion, at a
minimum do not use the NPEM interface when the _DSM is present. That was
the compromise to meet the spirit of the _DSM definition and leave it to
folks that care about the _DSM and have hardware to implement and test
that support.

However, I think the strawman is workable if only because base NPEM
already has a problem of ambiguity of which NPEM instance in a topology
should be used.

For example an NVME or CXL endpoint could have an NPEM implementation
that is superseded by an NPEM instance in its parent downstream port, or
another ancestor downstream port / root port.

The fact that the native NPEM may not be the right interface to use in
the presence of the _DSM has no specified way to resolve conflicts is at
least matched by downstream-port vs endpoint conflict resolution not
being specified.

So the spec left a bit of a mess and it is reasonable for Linux to say
"just turn on all the NPEMs and hope that userspace knows what it is
supposed to do".

  reply	other threads:[~2024-03-22 21:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-15 14:23 [PATCH 0/2] Native PCIe Enclosure Management Mariusz Tkaczyk
2024-02-15 14:23 ` [PATCH 1/2] leds: Init leds class earlier Mariusz Tkaczyk
2024-02-15 14:23 ` [PATCH 2/2] PCI/NPEM: Add Native PCIe Enclosure Management support Mariusz Tkaczyk
2024-03-06 22:40   ` Bjorn Helgaas
2024-03-07 12:25     ` Lukas Wunner
2024-03-11  9:47     ` Mariusz Tkaczyk
2024-03-11 22:13       ` Bjorn Helgaas
     [not found]         ` <CAL5oW00nSZV=oAjWPbYTwVGZ9OS1hW9hyZ5C0yzWbMjAstAA2g@mail.gmail.com>
2024-03-12  9:08           ` Mariusz Tkaczyk
2024-03-22 19:56             ` Bjorn Helgaas
2024-03-22 21:30               ` Dan Williams [this message]
2024-03-22 21:42                 ` Bjorn Helgaas
2024-03-22 21:51                   ` Dan Williams
2024-03-23  5:09               ` Lukas Wunner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=65fdf85bba0e0_4a98a294b2@dwillia2-mobl3.amr.corp.intel.com.notmuch \
    --to=dan.j.williams@intel.com \
    --cc=helgaas@kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mariusz.tkaczyk@linux.intel.com \
    --cc=stuart.w.hayes@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).