linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Manikanta Maddireddy <mmaddireddy@nvidia.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	robh+dt@kernel.org, mark.rutland@arm.com, jonathanh@nvidia.com,
	vidyas@nvidia.com, linux-tegra@vger.kernel.org,
	linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH V4 22/28] PCI: tegra: Access endpoint config only if PCIe link is up
Date: Wed, 19 Jun 2019 08:38:17 -0500	[thread overview]
Message-ID: <20190619133817.GA143205@google.com> (raw)
In-Reply-To: <9c0fb01f0dc6a193265297eaa100a35ff25413e7.camel@sipsolutions.net>

On Tue, Jun 18, 2019 at 02:32:59PM +0200, Johannes Berg wrote:
> On Tue, 2019-06-18 at 12:49 +0200, Thierry Reding wrote:
> 
> > > > > > > > > > 1. WiFi devices provides power-off feature for power saving
> > > > > > > > > > in mobiles.  When WiFi is turned off we shouldn't power on
> > > > > > > > > > the HW back without user turning it back on.
> 
> But why would you disconnect the PCIe device just to power it down?!
> 
> > > > > > > The problem that Manikanta is trying to solve here occurs in
> > > > > > > this situation (Manikanta, correct me if I've got this wrong):
> > > > > > > on some setups, a WiFi module connected over PCI will toggle a
> > > > > > > power GPIO as part of runtime suspend. This effectively causes
> > > > > > > the module to disappear from the PCI bus (i.e. it can no longer
> > > > > > > be accessed until the power GPIO is toggled again).
> > > > > > 
> > > > > > GPIO is toggled as part of WiFi on/off, can be triggered from
> > > > > > network manager UI.
> 
> That's kinda icky, IMHO.
> 
> > > > > > Correct, rfkill switch should handle the GPIO.
> > > > > > Sequence will be,
> > > > > >  - WiFi ON
> > > > > >    - rfkill switch enables the WiFi GPIO
> > > > > >    - Tegra PCIe receives hot plug event
> > > > > >    - Tegra PCIe hot plug driver rescans PCI bus and enumerates the device
> > > > > >    - PCI client driver is probed, which will create network interface
> > > > > >  - WiFi OFF
> > > > > >    - rfkill switch disables the WiFi GPIO
> > > > > >    - Tegra PCIe receives hot unplug event
> > > > > >    - Tegra PCIe hot plug driver removes PCI devices under the bus
> > > > > >    - PCI client driver remove is executed, which will remove
> > > > > >      network interface
> > > > > > We don't need current patch in this case because PCI device is not
> > > > > > present in the PCI hierarchy, so there cannot be EP config access
> > > > > > with link down.  However Tegra doesn't support hot plug and unplug
> > > > > > events. I am not sure if we have any software based hot plug event
> > > > > > trigger.
> 
> Looks reasonable to me.
> 
> I guess if you absolutely know in software when the device is
> present or not, you don't need "real" PCIe hotplug, just need to
> tickle the software right?
> 
> > > > How does rfkill work?  It sounds like it completely removes
> > > > power from the wifi device, putting it in D3cold.  Is there
> > > > any software notification other than the "Slot present pin
> > > > change" (which looks like a Tegra-specific thing)?
> 
> Well, they said above it's a GPIO that controls it, so the software
> already knows and doesn't really need an event?

Forgive my ignorance about rfkill.  At least in this Tegra case, it
sounds like rfkill basically controls a power switch for the entire
device, i.e., it doesn't merely turn off the radio portion of the
device; it puts the entire PCI device in D3cold.

Is rfkill integrated with the power management subsystem?  E.g., when
lspci or X tries to read config space via pci_read_config(), does the
pci_config_pm_runtime_get() in that path wake up the device?

IMO, if the struct pci_dev exists, we should be able to rely on the
device actually being accessible (possibly after bringing it back to
D0).  If rfkill only turns off the radio, leaving the PCI interface
active, that would be fine -- in that case generic PCI things like
lspci would work normally and it would be up to the driver to manage
network-related things.

But if rfkill turns off PCI interface and the power management
subsystem can't wake it up, I think we should unbind the driver and
remove the pci_dev, so it wouldn't appear in lspci at all.

Bjorn

  parent reply	other threads:[~2019-06-19 13:38 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16  5:52 [PATCH V4 00/28] Enable Tegra PCIe root port features Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 01/28] soc/tegra: pmc: Export tegra_powergate_power_on() Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 02/28] PCI: tegra: Handle failure cases in tegra_pcie_power_on() Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 03/28] PCI: tegra: Rearrange Tegra PCIe driver functions Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 04/28] PCI: tegra: Mask AFI_INTR in runtime suspend Manikanta Maddireddy
2019-06-04 13:08   ` Thierry Reding
2019-05-16  5:52 ` [PATCH V4 05/28] PCI: tegra: Fix PCIe host power up sequence Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 06/28] PCI: tegra: Add PCIe Gen2 link speed support Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 07/28] PCI: tegra: Advertise PCIe Advanced Error Reporting (AER) capability Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 08/28] PCI: tegra: Program UPHY electrical settings for Tegra210 Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 09/28] PCI: tegra: Enable opportunistic UpdateFC and ACK Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 10/28] PCI: tegra: Disable AFI dynamic clock gating Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 11/28] PCI: tegra: Process pending DLL transactions before entering L1 or L2 Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 12/28] PCI: tegra: Enable PCIe xclk clock clamping Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 13/28] PCI: tegra: Increase the deskew retry time Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 14/28] PCI: tegra: Add SW fixup for RAW violations Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 15/28] PCI: tegra: Update flow control timer frequency in Tegra210 Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 16/28] PCI: tegra: Set target speed as Gen1 before starting LTSSM Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 17/28] PCI: tegra: Fix PLLE power down issue due to CLKREQ# signal Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 18/28] PCI: tegra: Program AFI_CACHE* registers only for Tegra20 Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 19/28] PCI: tegra: Change PRSNT_SENSE IRQ log to debug Manikanta Maddireddy
2019-05-16  5:52 ` [PATCH V4 20/28] PCI: tegra: Use legacy IRQ for port service drivers Manikanta Maddireddy
2019-05-20 20:37   ` Bjorn Helgaas
2019-05-21  9:07     ` Manikanta Maddireddy
2019-05-16  5:53 ` [PATCH V4 21/28] PCI: tegra: Add AFI_PEX2_CTRL reg offset as part of soc struct Manikanta Maddireddy
2019-05-16  5:53 ` [PATCH V4 22/28] PCI: tegra: Access endpoint config only if PCIe link is up Manikanta Maddireddy
2019-06-04 13:14   ` Thierry Reding
2019-06-04 14:10     ` Manikanta Maddireddy
2019-06-10  4:38       ` Manikanta Maddireddy
2019-06-13 14:39         ` Lorenzo Pieralisi
2019-06-13 15:42           ` Thierry Reding
2019-06-17 10:01             ` Manikanta Maddireddy
2019-06-17 11:47               ` Thierry Reding
2019-06-17 19:30                 ` Bjorn Helgaas
2019-06-18  5:36                   ` Manikanta Maddireddy
2019-06-18 10:49                     ` Thierry Reding
2019-06-18 12:32                       ` Johannes Berg
2019-06-18 13:40                         ` Thierry Reding
2019-06-18 14:48                           ` Johannes Berg
2019-06-19 13:38                         ` Bjorn Helgaas [this message]
2019-06-19 13:40                           ` Johannes Berg
2019-05-16  5:53 ` [PATCH V4 23/28] dt-bindings: pci: tegra: Document PCIe DPD pinctrl optional prop Manikanta Maddireddy
2019-05-16  5:53 ` [PATCH V4 24/28] arm64: tegra: Add PEX DPD states as pinctrl properties Manikanta Maddireddy
2019-05-16  5:53 ` [PATCH V4 25/28] PCI: tegra: Put PEX CLK & BIAS pads in DPD mode Manikanta Maddireddy
2019-05-16  5:53 ` [PATCH V4 26/28] PCI: Add DT binding for "reset-gpios" property Manikanta Maddireddy
2019-06-17 11:30   ` Thierry Reding
2019-06-17 11:38     ` Manikanta Maddireddy
2019-06-17 11:48       ` Thierry Reding
2019-05-16  5:53 ` [PATCH V4 27/28] PCI: tegra: Add support for GPIO based PERST# Manikanta Maddireddy
2019-06-04 13:22   ` Thierry Reding
2019-06-13 15:24     ` Lorenzo Pieralisi
2019-06-14 10:37       ` Manikanta Maddireddy
2019-06-14 14:32         ` Lorenzo Pieralisi
2019-06-14 14:38           ` Manikanta Maddireddy
2019-06-14 14:50             ` Lorenzo Pieralisi
2019-06-14 14:56               ` Manikanta Maddireddy
2019-06-14 15:23               ` Thierry Reding
2019-06-14 15:59                 ` Lorenzo Pieralisi
2019-06-14 16:30                   ` Manikanta Maddireddy
2019-06-14 16:53                     ` Lorenzo Pieralisi
2019-06-14 17:23                       ` Manikanta Maddireddy
2019-06-17  9:48                         ` Lorenzo Pieralisi
2019-06-17 10:27                           ` Manikanta Maddireddy
2019-06-17 10:39                             ` Lorenzo Pieralisi
2019-06-17 11:29                         ` Thierry Reding
2019-06-17 11:26                       ` Thierry Reding
2019-05-16  5:53 ` [PATCH V4 28/28] PCI: tegra: Change link retry log level to debug Manikanta Maddireddy
2019-06-04 13:22   ` Thierry Reding
2019-05-16 13:12 ` [PATCH V4 00/28] Enable Tegra PCIe root port features Bjorn Helgaas
2019-05-17  8:38   ` Manikanta Maddireddy
2019-06-10  4:45 ` Manikanta Maddireddy
2019-06-10 17:33   ` Lorenzo Pieralisi

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=20190619133817.GA143205@google.com \
    --to=helgaas@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=johannes@sipsolutions.net \
    --cc=jonathanh@nvidia.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=mmaddireddy@nvidia.com \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=vidyas@nvidia.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 NNTP newsgroup(s).