linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hector Martin <marcan@marcan.st>
To: Rob Herring <robh@kernel.org>
Cc: "Marc Zyngier" <maz@kernel.org>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Alyssa Rosenzweig" <alyssa@rosenzweig.io>,
	"Sven Peter" <sven@svenpeter.dev>,
	PCI <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] PCI: apple: Add support for optional PWREN GPIO
Date: Tue, 3 May 2022 00:32:19 +0900	[thread overview]
Message-ID: <0ccc44cd-21aa-3670-24b3-4ee051dd3c12@marcan.st> (raw)
In-Reply-To: <CAL_Jsq+_cWZUXtJVXC_cwhmADj0NQc95v1sqgFioMsfEX6OqGg@mail.gmail.com>

On 03/05/2022 00.14, Rob Herring wrote:
> On Mon, May 2, 2022 at 4:39 AM Hector Martin <marcan@marcan.st> wrote:
>>
>> WiFi and SD card devices on M1 Macs have a separate power enable GPIO.
>> Add support for this to the PCIe controller. This is modeled after how
>> pcie-fu740 does it.
> 
> It did, but it's not ideal really. The problem is the GPIO is really
> associated with the device (WiFi/SD) rather than the PCI host and
> therefore should be part of a WiFi or SD node. You probably don't have
> one (yet), but I would suspect that SD will need one for all the
> standard MMC/SD DT properties. The secondary issue is we'll end up
> adding more power sequencing properties to control ordering and timing
> for different devices. The exception here is standard PCI slot
> properties like perst#, clkreq, and standard voltage rails can go in
> the host bridge (and for new bindings, those should really be in the
> root port node). For a complicated example, see Hikey960 or 970.
> 
> Of course with power control related properties there's a chicken or
> egg issue that the PCI device is not discoverable until the device is
> powered on. This issue comes up over and over with various hacky
> solutions in the bindings. The PCI subsystem needs to solve this. My
> suggestion is that if the firmware says there is a device on the bus
> and it wasn't probed, then we should force probing (or add a pre-probe
> hook for drivers). That is what MDIO bus does for example.
> 

I agree with the premise. Right now macOS does not actually power down
these devices as far as I know (except maybe sleep mode? not sure what
goes on then yet), but I think the hardware actually has an SD card
detect GPIO hookup that would allow us to entirely power down the SD
controller when no card is inserted. That would obviously be ideal.

FWIW, we do have the device nodes downstream [1]. I did in fact have to
add the SD one for the CD/WP inversion flags (and had to add driver
support for that too).

That said, as for how to make this happen in the PCI subsystem
properly... I think I'll defer to the maintainers' opinion there before
trying to hack something up ;)

Meanwhile, I guess I better get PCIe hotplug working, since doing it in
the driver isn't going to work without that first...

[1]
https://github.com/AsahiLinux/linux/blob/bits/000-devicetree/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi#L222

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

  reply	other threads:[~2022-05-02 15:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-02  9:38 [PATCH 0/3] PCI: apple: PWREN GPIO support & related fixes Hector Martin
2022-05-02  9:38 ` [PATCH 1/3] PCI: apple: GPIO handling nitfixes Hector Martin
2022-05-02 10:20   ` Marc Zyngier
2022-05-02 12:16     ` Hector Martin
2022-05-02 11:39   ` Greg KH
2022-05-02  9:38 ` [PATCH 2/3] PCI: apple: Probe all GPIOs for availability first Hector Martin
2022-05-02 10:23   ` Marc Zyngier
2022-05-02  9:38 ` [PATCH 3/3] PCI: apple: Add support for optional PWREN GPIO Hector Martin
2022-05-02 10:31   ` Marc Zyngier
2022-05-02 12:15     ` Hector Martin
2022-05-02 15:14   ` Rob Herring
2022-05-02 15:32     ` Hector Martin [this message]
2022-05-03  3:20       ` Hector Martin
2022-05-04  0:33         ` Rob Herring
2022-05-04  3:36           ` Hector Martin
2022-05-05 15:38             ` Marc Zyngier
2022-05-02 20:59 Mark Kettenis

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=0ccc44cd-21aa-3670-24b3-4ee051dd3c12@marcan.st \
    --to=marcan@marcan.st \
    --cc=alyssa@rosenzweig.io \
    --cc=bhelgaas@google.com \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=maz@kernel.org \
    --cc=robh@kernel.org \
    --cc=sven@svenpeter.dev \
    /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 NNTP newsgroup(s).