All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Bjorn Helgaas <helgaas@kernel.org>,
	linux1394-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	edmund.raile@proton.me
Subject: Re: [PATCH v2] PCI: Mark LSI FW643 to avoid bus reset
Date: Fri, 29 Mar 2024 14:37:05 +0100	[thread overview]
Message-ID: <ZgbEAa91hYmmhj7h@wunner.de> (raw)
In-Reply-To: <20240329081219.GC231329@workstation.local>

On Fri, Mar 29, 2024 at 05:12:19PM +0900, Takashi Sakamoto wrote:
> On Fri, Mar 29, 2024 at 05:41:16AM +0100, Lukas Wunner wrote:
> > Just checked the ACPI tables and there's an FPEN method below the
> > FRWR device which toggles GPIO 48 on the PCH.  Checked the schematics
> > as well and GPIO 48 is marked FW_PWR_EN.  The GPIO controls load
> > switches which cut power to the FW643 chip when nothing is connected.
> > 
> > Also, FW_PWR_EN feeds into an SLG4AP016V chip where it seems to
> > internally gate FW_CLKREQ_L.
> > 
> > I'm guessing the driver may need to call the FPEN ACPI method after
> > issuing a SBR to force the chip on (or perhaps first off, then on)
> > and thereby re-enable Clock Request.
> > 
> > It's a pity the ohci.c driver doesn't seem to support runtime PM.
> > That would allow cutting power to the chip when nothing is connected
> > and thus increase battery life.  The ACPI tables indicate that the
> > platform sends a notification when something is plugged in, so all
> > the necessary ingredients are there but we're not taking advantage
> > of them.
> 
> Yup. In both PCI drivers and unit drivers belonging to Linux FireWire
> subsystem, any type of runtime PM is not supported. If I integrate 1394
> OHCI driver, I should implement all of the above in any callback of
> runtime PM, or the part of the above is already supported by any driver
> in parent PCI layer?

The power management method Apple uses to cut power to the FireWire
controller, Thunderbolt controller and discrete GPU is nonstandard.
It's *implemented* in ACPI, but doesn't *conform* to ACPI:  There are
no Power Resources described in the ACPI tables, just custom methods.

This can be made to work on Linux by assigning a dev_pm_domain to the
Root Port above the FireWire controller.  The dev_pm_domain callbacks
cut power to the FireWire controller on ->runtime_suspend() and
reinstate it on ->runtime_resume().  The reason this needs to be done
at the Root Port level is that the PCI core assumes the FireWire
controller is powered on when it calls pci_pm_runtime_resume() for it.
Normally that function would reinstate power through ACPI via
pci_power_up(), but that doesn't work due to the nonstandard nature
of Apple's ACPI tables.

I've implemented this 8 years ago for Thunderbolt but unfortunately
got sidetracked and thus haven't been able to finish upstreaming it yet:

https://github.com/l1k/linux/commit/a53d44439d42

I'm not as familiar with ohci.c as I am (or was) with thunderbolt.ko.
If you could amend ohci.c to call pm_runtime_get() when something is
attached and call pm_runtime_put() when something is detached, I could
look into bringing up the ACPI stuff.  You could acquire one runtime PM
ref for each attached device or just acquire a single ref if *anything*
is connected at all.  Doesn't matter.  But acquiring one ref per attached
device might be simpler.

I would also need a function to perform a bus scan upon runtime resume
which looks for new devices and acquires refs as necessary.  Once that
infrastructure exists, adding the Apple-specific ACPI stuff wouldn't
be too hard for me to do as I could just adapt what I did for Thunderbolt.

Thanks,

Lukas

  reply	other threads:[~2024-03-29 13:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-26 10:25 [PATCH] PCI: Mark LSI FW643 to avoid bus reset Edmund Raile
2024-02-26 21:02 ` Bjorn Helgaas
2024-02-27 13:14 ` [PATCH v2] " Edmund Raile
2024-02-29 23:00   ` Bjorn Helgaas
2024-03-25  1:21     ` Takashi Sakamoto
2024-03-25 14:41       ` Bjorn Helgaas
2024-03-26 13:18         ` Takashi Sakamoto
2024-03-27 15:01           ` Bjorn Helgaas
2024-03-28 20:42             ` Alex Williamson
2024-03-28 21:06               ` Bjorn Helgaas
2024-03-29  4:41               ` Lukas Wunner
2024-03-29  8:12                 ` Takashi Sakamoto
2024-03-29 13:37                   ` Lukas Wunner [this message]
2024-03-28 18:35           ` edmund.raile
2024-03-29  8:05             ` Takashi Sakamoto
  -- strict thread matches above, loose matches on Subject: below --
2024-03-30 10:14 edmund.raile

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=ZgbEAa91hYmmhj7h@wunner.de \
    --to=lukas@wunner.de \
    --cc=alex.williamson@redhat.com \
    --cc=edmund.raile@proton.me \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    --cc=o-takashi@sakamocchi.jp \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.