From: Jim Quinlan <jim2101024@gmail.com> To: Bjorn Helgaas <helgaas@kernel.org> Cc: linux-pci@vger.kernel.org, "Nicolas Saenz Julienne" <nsaenz@kernel.org>, "Bjorn Helgaas" <bhelgaas@google.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Cyril Brulebois" <kibi@debian.org>, "Phil Elwell" <phil@raspberrypi.com>, bcm-kernel-feedback-list@broadcom.com, james.quinlan@broadcom.com, "Florian Fainelli" <f.fainelli@gmail.com>, "Lorenzo Pieralisi" <lpieralisi@kernel.org>, "Krzysztof Wilczyński" <kw@linux.com>, "Rob Herring" <robh@kernel.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@lists.infradead.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, "open list" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 2/3] PCI: brcmstb: CLKREQ# accomodations of downstream device Date: Fri, 14 Apr 2023 19:14:02 -0400 [thread overview] Message-ID: <CANCKTBuBZ5j+MqD9_uudvESO5WGC5hqCK1Sd4vD-CBzHsv+0zA@mail.gmail.com> (raw) In-Reply-To: <20230414202720.GA215111@bhelgaas> On Fri, Apr 14, 2023 at 4:27 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > This subject line no verb. Can you add a leading verb to suggest what > this patch does? > > s/accomodations/accommodations/ > > On Tue, Apr 11, 2023 at 12:59:17PM -0400, Jim Quinlan wrote: > > The Broadcom STB/CM PCIe HW core, which is also used in RPi SOCs, must be > > deliberately set by the probe() into one of three mutually exclusive modes: > > > > (a) No CLKREQ# expected or required, refclk is always available. > > (b) CLKREQ# is expected to be driven by downstream device when needed. > > (c) Bidirectional CLKREQ# for L1SS capable devices. > > > > Previously, only (b) was supported by the driver, as almost all STB/CM > > boards operate in this mode. But now there is interest in activating L1SS > > power savings from STB/CM customers, and also interest in accomodating mode > > (a) for designs such as the RPi CM4 with IO board. > > accommodating > > > The HW+driver is able to tell us when mode (a) mode is needed. But there > > is no easy way to tell if L1SS mode should be configured. In certain > > situations, getting this wrong may cause a panic during boot time. So we > > rely on the DT prop "brcm,enable-l1ss" to tell us when mode (c) is desired. > > Using this mode only makes sense when the downstream device is L1SS-capable > > and the OS has been configured to activate L1SS > > (e.g. policy==powersupersave). > > I'm really concerned about the user experience here. I assume users > do not want to edit the DT based on what device they plug in. They > shouldn't need to (and probably won't) know whether the device > supports L1SS. > > I hate kernel/module parameters, but I think even that would be better > then having to edit the DT. > > There's obviously a period of time when L1SS is supported but not yet > enabled, so I'm *guessing* the "OS has been configured to activate > L1SS" is not actually a requirement, and choosing (c) really just > opens the possibility that L1SS can be used? Yes. Before this patch series we had two panic scenarios: (a) Endpoint devices with no CLKREQ# connection (b) Endpoints that are L1SS-capable Even without the "brcm,enable-l1ss" property present, both (a) and (b) should be eliminated. The reason (b) is eliminated is because the RC driver now unadvertises RC L1SS by default; subsequently, Linux does not turn it on. So the default setting should be fine for all devices. For those folks who have L1SS capable devices and desire L1SS power savings, they can add the brcm,enable-l1ss property. But everyone should have functionality w/o doing anything. As I am typing this I realize that my comments and dev_info()s are not aligned with what I am saying so I will change them in V3. Sorry about the confusion. > > Would be nice to have a hint (maybe a line or two of the panic > message) to help users find the fix for a problem they're seeing.SS > > Obviously the ideal would be if we could use (c) in all cases, so I > assume that's where a panic might happen. What situation would that > be? An endpoint that doesn't support L1SS? One that supports L1SS > but it's not enabled? Maybe if L1SS isn't configured correctly, e.g., > LTR values programmed wrong? Let me test everything on Monday and get back to you before I make any statements. Regards, Jim Quinilan Broadcom STB > > Bjorn
WARNING: multiple messages have this Message-ID (diff)
From: Jim Quinlan <jim2101024@gmail.com> To: Bjorn Helgaas <helgaas@kernel.org> Cc: linux-pci@vger.kernel.org, "Nicolas Saenz Julienne" <nsaenz@kernel.org>, "Bjorn Helgaas" <bhelgaas@google.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Cyril Brulebois" <kibi@debian.org>, "Phil Elwell" <phil@raspberrypi.com>, bcm-kernel-feedback-list@broadcom.com, james.quinlan@broadcom.com, "Florian Fainelli" <f.fainelli@gmail.com>, "Lorenzo Pieralisi" <lpieralisi@kernel.org>, "Krzysztof Wilczyński" <kw@linux.com>, "Rob Herring" <robh@kernel.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@lists.infradead.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, "open list" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 2/3] PCI: brcmstb: CLKREQ# accomodations of downstream device Date: Fri, 14 Apr 2023 19:14:02 -0400 [thread overview] Message-ID: <CANCKTBuBZ5j+MqD9_uudvESO5WGC5hqCK1Sd4vD-CBzHsv+0zA@mail.gmail.com> (raw) In-Reply-To: <20230414202720.GA215111@bhelgaas> On Fri, Apr 14, 2023 at 4:27 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > This subject line no verb. Can you add a leading verb to suggest what > this patch does? > > s/accomodations/accommodations/ > > On Tue, Apr 11, 2023 at 12:59:17PM -0400, Jim Quinlan wrote: > > The Broadcom STB/CM PCIe HW core, which is also used in RPi SOCs, must be > > deliberately set by the probe() into one of three mutually exclusive modes: > > > > (a) No CLKREQ# expected or required, refclk is always available. > > (b) CLKREQ# is expected to be driven by downstream device when needed. > > (c) Bidirectional CLKREQ# for L1SS capable devices. > > > > Previously, only (b) was supported by the driver, as almost all STB/CM > > boards operate in this mode. But now there is interest in activating L1SS > > power savings from STB/CM customers, and also interest in accomodating mode > > (a) for designs such as the RPi CM4 with IO board. > > accommodating > > > The HW+driver is able to tell us when mode (a) mode is needed. But there > > is no easy way to tell if L1SS mode should be configured. In certain > > situations, getting this wrong may cause a panic during boot time. So we > > rely on the DT prop "brcm,enable-l1ss" to tell us when mode (c) is desired. > > Using this mode only makes sense when the downstream device is L1SS-capable > > and the OS has been configured to activate L1SS > > (e.g. policy==powersupersave). > > I'm really concerned about the user experience here. I assume users > do not want to edit the DT based on what device they plug in. They > shouldn't need to (and probably won't) know whether the device > supports L1SS. > > I hate kernel/module parameters, but I think even that would be better > then having to edit the DT. > > There's obviously a period of time when L1SS is supported but not yet > enabled, so I'm *guessing* the "OS has been configured to activate > L1SS" is not actually a requirement, and choosing (c) really just > opens the possibility that L1SS can be used? Yes. Before this patch series we had two panic scenarios: (a) Endpoint devices with no CLKREQ# connection (b) Endpoints that are L1SS-capable Even without the "brcm,enable-l1ss" property present, both (a) and (b) should be eliminated. The reason (b) is eliminated is because the RC driver now unadvertises RC L1SS by default; subsequently, Linux does not turn it on. So the default setting should be fine for all devices. For those folks who have L1SS capable devices and desire L1SS power savings, they can add the brcm,enable-l1ss property. But everyone should have functionality w/o doing anything. As I am typing this I realize that my comments and dev_info()s are not aligned with what I am saying so I will change them in V3. Sorry about the confusion. > > Would be nice to have a hint (maybe a line or two of the panic > message) to help users find the fix for a problem they're seeing.SS > > Obviously the ideal would be if we could use (c) in all cases, so I > assume that's where a panic might happen. What situation would that > be? An endpoint that doesn't support L1SS? One that supports L1SS > but it's not enabled? Maybe if L1SS isn't configured correctly, e.g., > LTR values programmed wrong? Let me test everything on Monday and get back to you before I make any statements. Regards, Jim Quinilan Broadcom STB > > Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-04-14 23:14 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-04-11 16:59 [PATCH v2 0/3] PCI: brcmstb: CLKREQ# accomodations of downstream device Jim Quinlan 2023-04-11 16:59 ` Jim Quinlan 2023-04-11 16:59 ` [PATCH v2 1/3] dt-bindings: PCI: brcmstb: Add two optional props Jim Quinlan 2023-04-11 16:59 ` Jim Quinlan 2023-04-12 8:09 ` Krzysztof Kozlowski 2023-04-12 8:09 ` Krzysztof Kozlowski 2023-04-12 11:49 ` Florian Fainelli 2023-04-12 11:49 ` Florian Fainelli 2023-04-12 11:56 ` Krzysztof Kozlowski 2023-04-12 11:56 ` Krzysztof Kozlowski 2023-04-12 14:14 ` Jim Quinlan 2023-04-12 14:14 ` Jim Quinlan 2023-04-12 15:37 ` Rob Herring 2023-04-12 15:37 ` Rob Herring 2023-04-12 16:12 ` Florian Fainelli 2023-04-12 16:12 ` Florian Fainelli 2023-04-18 18:35 ` Rob Herring 2023-04-18 18:35 ` Rob Herring 2023-04-21 19:07 ` Konstantin Ryabitsev 2023-04-21 19:07 ` Konstantin Ryabitsev 2023-04-14 20:14 ` Bjorn Helgaas 2023-04-14 20:14 ` Bjorn Helgaas 2023-04-11 16:59 ` [PATCH v2 2/3] PCI: brcmstb: CLKREQ# accomodations of downstream device Jim Quinlan 2023-04-11 16:59 ` Jim Quinlan 2023-04-13 14:39 ` Cyril Brulebois 2023-04-13 14:39 ` Cyril Brulebois 2023-04-13 14:57 ` Jim Quinlan 2023-04-13 14:57 ` Jim Quinlan 2023-04-13 20:06 ` Cyril Brulebois 2023-04-14 12:14 ` Jim Quinlan 2023-04-14 12:14 ` Jim Quinlan 2023-04-14 12:27 ` Florian Fainelli 2023-04-14 12:27 ` Florian Fainelli 2023-04-14 13:31 ` Jim Quinlan 2023-04-14 13:31 ` Jim Quinlan 2023-04-14 16:19 ` Cyril Brulebois 2023-04-14 16:19 ` Cyril Brulebois 2023-04-19 14:23 ` Jim Quinlan 2023-04-19 14:23 ` Jim Quinlan 2023-04-19 15:57 ` Cyril Brulebois 2023-04-19 15:57 ` Cyril Brulebois 2023-04-13 14:58 ` Florian Fainelli 2023-04-13 14:58 ` Florian Fainelli 2023-04-14 20:27 ` Bjorn Helgaas 2023-04-14 20:27 ` Bjorn Helgaas 2023-04-14 20:33 ` Florian Fainelli 2023-04-14 20:33 ` Florian Fainelli 2023-04-17 21:41 ` Bjorn Helgaas 2023-04-17 21:41 ` Bjorn Helgaas 2023-04-14 23:14 ` Jim Quinlan [this message] 2023-04-14 23:14 ` Jim Quinlan 2023-04-11 16:59 ` [PATCH v2 3/3] PCI: brcmstb: Set PCIe transaction completion timeout Jim Quinlan 2023-04-11 16:59 ` Jim Quinlan 2023-04-12 0:26 ` Cyril Brulebois 2023-04-12 0:26 ` Cyril Brulebois 2023-04-13 18:40 ` [PATCH v2 0/3] PCI: brcmstb: CLKREQ# accomodations of downstream device Florian Fainelli 2023-04-13 18:40 ` Florian Fainelli
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=CANCKTBuBZ5j+MqD9_uudvESO5WGC5hqCK1Sd4vD-CBzHsv+0zA@mail.gmail.com \ --to=jim2101024@gmail.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bhelgaas@google.com \ --cc=f.fainelli@gmail.com \ --cc=helgaas@kernel.org \ --cc=james.quinlan@broadcom.com \ --cc=kibi@debian.org \ --cc=kw@linux.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=lpieralisi@kernel.org \ --cc=nsaenz@kernel.org \ --cc=phil@raspberrypi.com \ --cc=robh@kernel.org \ /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: linkBe 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.