Re: [PATCH 1/3] dt-bindings: mmc: iproc-sdhci: Convert to json-schema
On Mon, 2021-04-12 at 09:52 +0200, Ulf Hansson wrote: > On Tue, 6 Apr 2021 at 12:48, Nicolas Saenz Julienne wrote: > > > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema > > > > Signed-off-by: Nicolas Saenz Julienne > > Applied for next, thanks! (I guess patch2 and patch3 needs a re-spin?) Yes, that's right. Thanks! Regards, Nicolas
[PATCH] mailmap: Update email address for Nicolas Saenz
Add my kernel.org address for old email address. Signed-off-by: Nicolas Saenz Julienne --- .mailmap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.mailmap b/.mailmap index 78835e80214a..b407e1192fb3 100644 --- a/.mailmap +++ b/.mailmap @@ -260,6 +260,8 @@ Nguyen Anh Quynh Nicolas Ferre Nicolas Pitre Nicolas Pitre +Nicolas Saenz Julienne +Nicolas Saenz Julienne Oleksij Rempel Oleksij Rempel Oleksij Rempel -- 2.31.1
Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
Hi again, On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote: > Nicolas, > > I got a better description of the failure and it looks like the bus > clock needs to be limited to 300KHz for a 500MHz core clock. > What's happening is that an internal reset sequence is needed after a > command timeout and the reset signal needs to be asserted for at least > 2 ticks of the bus clock. This is done using a 12 bit counter clocked > by the core clock. That means a 500MHz core clock produces a 122KHz > reset signal which is too fast for 2 ticks of the 200KHz bus clock > (100KHz) but is okay for the 300KHz (150Khz) bus clock. Is there any value in implementing this in a generic way? That is, will a SoC other than BCM2711 ever need this? Otherwise I can simply limit BCM2711's sdhci-iproc min clk frequency to 300KHz and call it a day. The alternative is something the likes of: min_clk = clk_get_rate(core_bus) >> 11; But it involves updating the bindings and DT. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH] MAINTAINERS: Update BCM2711/BCM2335 maintainer's mail
The @kernel.org e-mail address is likely to last longer than the current one, so use it. Signed-off-by: Nicolas Saenz Julienne --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index d269763af39d..c8f32b7c1e06 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3414,7 +3414,7 @@ F:include/linux/dsa/brcm.h F: include/linux/platform_data/b53.h BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE -M: Nicolas Saenz Julienne +M: Nicolas Saenz Julienne L: bcm-kernel-feedback-l...@broadcom.com L: linux-rpi-ker...@lists.infradead.org (moderated for non-subscribers) L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) -- 2.31.1
Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
Hi Al, On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote: > Nicolas, > > I got a better description of the failure and it looks like the bus > clock needs to be limited to 300KHz for a 500MHz core clock. > What's happening is that an internal reset sequence is needed after a > command timeout and the reset signal needs to be asserted for at least > 2 ticks of the bus clock. This is done using a 12 bit counter clocked > by the core clock. That means a 500MHz core clock produces a 122KHz > reset signal which is too fast for 2 ticks of the 200KHz bus clock > (100KHz) but is okay for the 300KHz (150Khz) bus clock. Thanks for the info. I'll work something out. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH] pwm: raspberrypi-poe: Fix mailbox message initialization
For testing purposes this driver might be built on big-endian
architectures. So make sure we take that into account when populating
structures that are to be passed to RPi4's mailbox.
Reported-by: kernel test robot
Fixes: 79caa362eab6 ("pwm: Add Raspberry Pi Firmware based PWM bus")
Signed-off-by: Nicolas Saenz Julienne
---
@arndb: This was just meged into the arm-soc tree some days ago. Should I
prepare a second PR once it's been reviewed?
drivers/pwm/pwm-raspberrypi-poe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-raspberrypi-poe.c
b/drivers/pwm/pwm-raspberrypi-poe.c
index 043fc32e8be8..78423e66f4fc 100644
--- a/drivers/pwm/pwm-raspberrypi-poe.c
+++ b/drivers/pwm/pwm-raspberrypi-poe.c
@@ -66,7 +66,7 @@ static int raspberrypi_pwm_get_property(struct rpi_firmware
*firmware,
u32 reg, u32 *val)
{
struct raspberrypi_pwm_prop msg = {
- .reg = reg
+ .reg = cpu_to_le32(reg),
};
int ret;
--
2.30.2
Re: [PATCH 2/3] mmc: sdhci-iproc: Cap min clock frequency on BCM2711
On Tue, 2021-04-06 at 19:59 +0200, Stefan Wahren wrote:
> Hi Nicolas,
>
> Am 06.04.21 um 12:48 schrieb Nicolas Saenz Julienne:
> > There is a known bug on BCM2711's SDHCI core integration where the
> > controller will hang when the difference between the core clock and the
> > bus clock is too great. Specifically this can be reproduced under the
> > following conditions:
> >
> > - No SD card plugged in, polling thread is running, probing cards at
> > 100KHz.
> > - BCM2711's core clock configured at 500MHz or more.
> >
> > So set 200MHz as the minimum clock frequency available for that board.
> i think it should be 200 kHz?
Of course... Sorry for that.
Regards,
Nicolas
> >
> > For more information on the issue see this:
> > https://lore.kernel.org/linux-mmc/20210322185816.27582-1-nsa...@kernel.org/T/#m11f2783a09b581da6b8a15f302625b43a6ecdeca
> >
> > Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the
> > BCM2711")
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/mmc/host/sdhci-iproc.c | 18 ++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> > index ddeaf8e1f72f..1ef888e91f73 100644
> > --- a/drivers/mmc/host/sdhci-iproc.c
> > +++ b/drivers/mmc/host/sdhci-iproc.c
> > @@ -173,6 +173,23 @@ static unsigned int sdhci_iproc_get_max_clock(struct
> > sdhci_host *host)
> > return pltfm_host->clock;
> > }
> >
> >
> > +/*
> > + * There is a known bug on BCM2711's SDHCI core integration where the
> > + * controller will hang when the difference between the core clock and the
> > bus
> > + * clock is too great. Specifically this can be reproduced under the
> > following
> > + * conditions:
> > + *
> > + * - No SD card plugged in, polling thread is running, probing cards at
> > + *100KHz.
> > + * - BCM2711's core clock configured at 500MHz or more
> > + *
> > + * So we set 200MHz as the minimum clock frequency available for that SoC.
> > + */
> > +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host
> > *host)
> > +{
> > + return 20;
> > +}
> > +
> > static const struct sdhci_ops sdhci_iproc_ops = {
> > .set_clock = sdhci_set_clock,
> > .get_max_clock = sdhci_iproc_get_max_clock,
> > @@ -271,6 +288,7 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops =
> > {
> > .set_clock = sdhci_set_clock,
> > .set_power = sdhci_set_power_and_bus_voltage,
> > .get_max_clock = sdhci_iproc_get_max_clock,
> > + .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
> > .set_bus_width = sdhci_set_bus_width,
> > .reset = sdhci_reset,
> > .set_uhs_signaling = sdhci_set_uhs_signaling,
signature.asc
Description: This is a digitally signed message part
[PATCH 3/3] mmc: sdhci-iproc: Set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN on BCM2711
The controller doesn't seem to pick-up on clock changes, so set the
SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN flag to query the clock frequency
directly from the clock.
Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")
Signed-off-by: Nicolas Saenz Julienne
---
drivers/mmc/host/sdhci-iproc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 1ef888e91f73..31f61ea474f0 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -295,7 +295,8 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
};
static const struct sdhci_pltfm_data sdhci_bcm2711_pltfm_data = {
- .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
+ .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = _iproc_bcm2711_ops,
};
--
2.30.2
[PATCH 2/3] mmc: sdhci-iproc: Cap min clock frequency on BCM2711
There is a known bug on BCM2711's SDHCI core integration where the
controller will hang when the difference between the core clock and the
bus clock is too great. Specifically this can be reproduced under the
following conditions:
- No SD card plugged in, polling thread is running, probing cards at
100KHz.
- BCM2711's core clock configured at 500MHz or more.
So set 200MHz as the minimum clock frequency available for that board.
For more information on the issue see this:
https://lore.kernel.org/linux-mmc/20210322185816.27582-1-nsa...@kernel.org/T/#m11f2783a09b581da6b8a15f302625b43a6ecdeca
Fixes: f84e411c85be ("mmc: sdhci-iproc: Add support for emmc2 of the BCM2711")
Signed-off-by: Nicolas Saenz Julienne
---
drivers/mmc/host/sdhci-iproc.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index ddeaf8e1f72f..1ef888e91f73 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -173,6 +173,23 @@ static unsigned int sdhci_iproc_get_max_clock(struct
sdhci_host *host)
return pltfm_host->clock;
}
+/*
+ * There is a known bug on BCM2711's SDHCI core integration where the
+ * controller will hang when the difference between the core clock and the bus
+ * clock is too great. Specifically this can be reproduced under the following
+ * conditions:
+ *
+ * - No SD card plugged in, polling thread is running, probing cards at
+ *100KHz.
+ * - BCM2711's core clock configured at 500MHz or more
+ *
+ * So we set 200MHz as the minimum clock frequency available for that SoC.
+ */
+static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
+{
+ return 20;
+}
+
static const struct sdhci_ops sdhci_iproc_ops = {
.set_clock = sdhci_set_clock,
.get_max_clock = sdhci_iproc_get_max_clock,
@@ -271,6 +288,7 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
.set_clock = sdhci_set_clock,
.set_power = sdhci_set_power_and_bus_voltage,
.get_max_clock = sdhci_iproc_get_max_clock,
+ .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
.set_bus_width = sdhci_set_bus_width,
.reset = sdhci_reset,
.set_uhs_signaling = sdhci_set_uhs_signaling,
--
2.30.2
[PATCH 1/3] dt-bindings: mmc: iproc-sdhci: Convert to json-schema
Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
Signed-off-by: Nicolas Saenz Julienne
---
.../bindings/mmc/brcm,iproc-sdhci.yaml| 63 +++
.../bindings/mmc/brcm,sdhci-iproc.txt | 37 ---
2 files changed, 63 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
new file mode 100644
index ..6f569fbfa134
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom IPROC SDHCI controller
+
+maintainers:
+ - Ray Jui
+ - Scott Branden
+ - Nicolas Saenz Julienne
+
+allOf:
+ - $ref: mmc-controller.yaml#
+
+properties:
+ compatible:
+enum:
+ - brcm,bcm2835-sdhci
+ - brcm,bcm2711-emmc2
+ - brcm,sdhci-iproc-cygnus
+ - brcm,sdhci-iproc
+
+ reg:
+minItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+description:
+ Handle to core clock for the sdhci controller.
+
+ sdhci,auto-cmd12:
+type: boolean
+description: Specifies that controller should use auto CMD12
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+#include
+#include
+
+mmc@18041000 {
+ compatible = "brcm,sdhci-iproc-cygnus";
+ reg = <0x18041000 0x100>;
+ interrupts = ;
+ clocks = <_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
+ bus-width = <4>;
+ sdhci,auto-cmd12;
+ no-1-8-v;
+};
+...
diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
deleted file mode 100644
index 09d87cc1182a..
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Broadcom IPROC SDHCI controller
-
-This file documents differences between the core properties described
-by mmc.txt and the properties that represent the IPROC SDHCI controller.
-
-Required properties:
-- compatible : Should be one of the following
- "brcm,bcm2835-sdhci"
- "brcm,bcm2711-emmc2"
- "brcm,sdhci-iproc-cygnus"
- "brcm,sdhci-iproc"
-
-Use brcm2835-sdhci for the eMMC controller on the BCM2835 (Raspberry Pi) and
-bcm2711-emmc2 for the additional eMMC2 controller on BCM2711.
-
-Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
-restricted to 32bit host accesses to SDHCI registers.
-
-Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
-8, 16, 32-bit host access to SDHCI register.
-
-- clocks : The clock feeding the SDHCI controller.
-
-Optional properties:
- - sdhci,auto-cmd12: specifies that controller should use auto CMD12.
-
-Example:
-
-sdhci0: sdhci@18041000 {
- compatible = "brcm,sdhci-iproc-cygnus";
- reg = <0x18041000 0x100>;
- interrupts = ;
- clocks = <_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
- bus-width = <4>;
- sdhci,auto-cmd12;
- no-1-8-v;
-};
--
2.30.2
[PATCH 0/3] BCM2711 sdhci-iproc improvements
This is a follow-up series to "BCM2711's sdhci-iproc CMD timeouts"[1], but since much most of the patches changed I decided to start counting patch revisions from scratch. This series tries to address rather odd behavior from BCM2711's integration of sdhci-iproc (Raspberry Pi 4's SoC). The controller will timeout on SDHCI CMDs under the following conditions: - No SD card plugged in (the card polling thread is running, CD irq disabled). - BCM2711's VPU clock configured at 500MHz or more, lower clocks are OK. There is no specific command that will time out, it seems random. As an extra to this I also include a small fix, and convert the controller's bindings to yaml. Regards, Nicolas [1] https://lore.kernel.org/linux-mmc/20210322185816.27582-1-nsa...@kernel.org/ --- Nicolas Saenz Julienne (3): dt-bindings: mmc: iproc-sdhci: Convert to json-schema mmc: sdhci-iproc: Cap min clock frequency on BCM2711 mmc: sdhci-iproc: Set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN on BCM2711 .../bindings/mmc/brcm,iproc-sdhci.yaml| 63 +++ .../bindings/mmc/brcm,sdhci-iproc.txt | 37 --- drivers/mmc/host/sdhci-iproc.c| 21 ++- 3 files changed, 83 insertions(+), 38 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt -- 2.30.2
Re: [PATCH] clk: Mark fwnodes when their clock provider is added
On Wed, 2021-03-31 at 12:25 -0700, Stephen Boyd wrote: > Quoting Geert Uytterhoeven (2021-03-31 00:05:00) > > On Wed, Mar 31, 2021 at 4:22 AM Stephen Boyd wrote: > > > > > Does it have any use? > > > > > > > > of_clk_del_provider() removes the first provider found with node == > > > > NULL. > > > > If there are two drivers calling of_clk_add_hw_provider(), and one of > > > > hem calls of_clk_del_provider() later, the wrong provider may be > > > > removed from the list. > > > > > > > > > > So you're saying we shouldn't add a NULL device node pointer to the list > > > so that this can't happen? That doesn't mean returning an error from > > > of_clk_add_hw_provider() would be useful though. > > > of_clk_add_hw_provider() can return 0 if np == NULL and > > > of_clk_del_provider() can return early if np == NULL too. > > > > I don't know if I grasp all meanings of the above. > > > > The main question is if it is valid for a driver to call > > of_clk_add_hw_provider() > > with np == NULL. > > - If yes, should that register the provider? > > No it should not register the provider. That would be bad as you pointed > out. > > > - If yes, how to handle two drivers calling of_clk_add_hw_provider() > > with np = NULL, as their unregistration order is not guaranteed to > > be correct. > > > > If no, is that something to ignore (0), or a bug (error)? > > This is my question above. Is there a use to having > of_clk_add_hw_provider() return an error value when np == NULL? I doubt > it. > > Returning 0 would reduce the if conditions in driver code in this case > and be consistent with the CONFIG_OF=n inline stub that returns 0 when > CONFIG_OF is disabled. The only case an error would be returned is if we > couldn't allocate memory or if the assigned clocks code failed. Seems > sane to me. The downside is that drivers would maybe register clkdev > lookups when they don't need to and waste some memory. I'm fine with > that until we have some sort of non-DT based clk provider lookup > mechanism that could unify the two methods. What about devm_of_clk_add_hw_provider() users, do we care that a seemingly empty managed resource will be created? Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
Hi Alan, On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote: > Nicolas, > > Sorry, I just noticed this thread. > This is a known bug in some newer Arasan cores. > The problem happens when the difference between the core clock and the bus > clock is too great. > Limiting the clock to 200KHz minimum should be a good fix. Great, that's what I was hoping to hear :). Out of curiosity, can you share more details on how the failure occurs? > In my experience, it's only eMMC that needs the clock to be retried > below 400KHz and not SD or SDIO. That's because the CMD signal for > eMMC starts out as open-drain during identification and the size of > the pull-up on the CMD signal can require the <400KHz clock. Once eMMC > is out of identification mode the CMD signal is switched to push-pull > and can run at much higher clock rates. Fair enough, I need to do some tests, some of the compute modules use an eMMC. > I don't think that SD and SDIO have any open-drain signals, so they > shouldn't need to retry at slower clock speeds. Noted. > I'm trying to get more detail on the bug, like the exact ratio of core > clock to bus clock that causes the problem. When I first found this > bug I was told that the failure would not happen at 200KHz, but we > were using a 405MHz core clock. That would be nice to have. > One other question. Why are you using polling for the SD card, this > newer controller supports the interrupt driven "Card Inserted" signal > and avoids wasting time polling? I believe the line isn't routed on RPi4. > Al signature.asc Description: This is a digitally signed message part
Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
> Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> > Hi Stefan,
> >
> > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> > > Hi Nicolas,
> > >
> > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > > > From: Nicolas Saenz Julienne
> > > >
> > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > > > seems to interfere with the VPU clock when setup at frequencies bigger
> > > > than 500MHz (a pretty common case). This ends up causing unwarranted
> > > > SDHCI CMD hangs when no SD card is present.
> > > >
> > > > Signed-off-by: Nicolas Saenz Julienne
> > > > ---
> > > > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > index 3b4ab947492a..9aa8408d9960 100644
> > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > @@ -257,6 +257,12 @@ {
> > > > vqmmc-supply = <_io_1v8_reg>;
> > > > vmmc-supply = <_vcc_reg>;
> > > > broken-cd;
> > > > + /*
> > > > +* Force the frequency to 150MHz as the default 100MHz seems to
> > > > +* interfere with the VPU clock when setup at frequencies
> > > > bigger than
> > > > +* 500MHz, causing unwarranted CMD hangs.
> > > > +*/
> > > > + clock-frequency = <15000>;
> > > i don't want to bike-shed here, but is there any chance to solve this in
> > > clk-bcm2835 in a less hacky way?
> > What do you have in mind?
> Sorry, nothing specific.
> >
> > All I can think of is adding some kind of heuristic to the clock's prepare()
> > callback. That said, I don't feel it would be a better solution than this.
>
> Based on my limited knowledge and an old SD card specification, all
> possibly connected devices could have different frequencies. So my
> concern here is, that in case we limit the frequency to a specific value
> we could break things just to suppress a warning.
SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
available, it depends on the implementation but the worst kind provide /2^n.
Not perfect, but good enough for things to work.
Now, I've been having a deeper look into how clocks are handled, and found two
new clues:
- First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
that is, the controller isn't properly identifying the clock frequency fed
into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
expert, so it's possible changing frequencies also needs a special operation
to recalculate this variable. But this was making all internal calculations
wrong when paired with this series.
- With this flag set SDHCI's core now properly calculates divisor values based
on whatever clock frequency I set in DT. And guess what, the issue reappears
even when running on 150MHz. It turns out, as I had some debugging enabled,
the issue only happens when the controller is configured at 100KHz (that
only happens while running the card detect thread).
So, I can now do this (note that for card detection try to communicate with the
card starting at 400KHz down to 100KHz in 100KHz steps):
->8-
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 536c382e2486..e5a5de63f347 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct
sdhci_host *host)
return pltfm_host->clock;
}
+static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
+{
+ return 20;
+}
+
static const struct sdhci_ops sdhci_iproc_ops = {
.set_clock = sdhci_set_clock,
.get_max_clock = sdhci_iproc_get_max_clock,
@@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
.set_clock = sdhci_set_clock,
.set_power = sdhci_set_power_and_bus_voltage,
.get_max_clock = sdhci_iproc_get_max_clock,
+ .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
.set_bus_width = sdhci_set_bus_width,
.reset = sdhci_reset,
.set_uhs_signaling = sdhci_set_uhs_signaling,
};
->8-
Which is rather nicer than what this series introduces. But I can't still
explain why configuring the controller at 100KHz is causing the hangs (while
having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
from the polling frequency list is going to break support for older SD cards.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[PATCH] clk: bcm: rpi: Don't register as OF provider if !dev->np
There are two ways clk-raspberrypi might be registered: through
device-tree or through an explicit platform device registration. The
latter happens after firmware/raspberrypi's probe, and it's limited to
RPi3s, which solely use the ARM clock to scale CPU's frequency. That
clock is matched with cpu0's device thanks to the ARM clock being
registered as a clkdev.
In that scenario, don't register the device as an OF clock provider, as
it makes no sense and will cause trouble.
Fixes: d4b4f1b6b97e ("clk: bcm: rpi: Add DT provider for the clocks")
Reported-by: Marek Szyprowski
Signed-off-by: Nicolas Saenz Julienne
---
drivers/clk/bcm/clk-raspberrypi.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/bcm/clk-raspberrypi.c
b/drivers/clk/bcm/clk-raspberrypi.c
index f89b9cfc4309..27e85687326f 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -337,10 +337,12 @@ static int raspberrypi_clk_probe(struct platform_device
*pdev)
if (ret)
return ret;
- ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
- clk_data);
- if (ret)
- return ret;
+ if (dev->of_node) {
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+ clk_data);
+ if (ret)
+ return ret;
+ }
rpi->cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq",
-1, NULL, 0);
--
2.30.2
Re: [PATCH] clk: Mark fwnodes when their clock provider is added
On Thu, 2021-03-25 at 14:31 +0100, Marek Szyprowski wrote:
> Hi
>
> On 10.02.2021 12:44, Tudor Ambarus wrote:
> > This is a follow-up for:
> > commit 3c9ea42802a1 ("clk: Mark fwnodes when their clock provider is
> > added/removed")
> >
> > The above commit updated the deprecated of_clk_add_provider(),
> > but missed to update the preferred of_clk_add_hw_provider().
> > Update it now.
> >
> > Signed-off-by: Tudor Ambarus
>
> This patch, which landed in linux-next as commit 6579c8d97ad7 ("clk:
> Mark fwnodes when their clock provider is added") causes the following
> NULL pointer dereference on Raspberry Pi 3b+ boards:
>
> --->8---
>
> raspberrypi-firmware soc:firmware: Attached to firmware from
> 2020-01-06T13:05:25
> Unable to handle kernel NULL pointer dereference at virtual address
> 0050
> Mem abort info:
> ESR = 0x9604
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x0004
> CM = 0, WnR = 0
> [0050] user address but active_mm is swapper
> Internal error: Oops: 9604 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 10 Comm: kworker/0:1 Not tainted 5.12.0-rc4+ #2764
> Hardware name: Raspberry Pi 3 Model B (DT)
> Workqueue: events deferred_probe_work_func
> pstate: 0005 (nzcv daif -PAN -UAO -TCO BTYPE=--)
> pc : of_clk_add_hw_provider+0xac/0xe8
> lr : of_clk_add_hw_provider+0x94/0xe8
> sp : 8000130936b0
> x29: 8000130936b0 x28: 800012494e04
> x27: 3b18cb05 x26: 3aa5c010
> x25: x24:
> x23: 3aa1e380 x22: 8000106830d0
> x21: 80001233f180 x20: 0018
> x19: x18: 8000124d38b0
> x17: 0013 x16: 0014
> x15: 8000125758b0 x14: 000184e0
> x13: 292e x12: 80001258dd98
> x11: 0001 x10: 0101010101010101
> x9 : 80001233f288 x8 : 7f7f7f7f7f7f7f7f
> x7 : fefefefeff6c626f x6 : 5d636d8080808080
> x5 : 006d635d x4 :
> x3 : x2 : 540eb5edae191600
> x1 : x0 :
> Call trace:
> of_clk_add_hw_provider+0xac/0xe8
> devm_of_clk_add_hw_provider+0x5c/0xb8
> raspberrypi_clk_probe+0x110/0x210
> platform_probe+0x90/0xd8
> really_probe+0x108/0x3c0
> driver_probe_device+0x60/0xc0
> __device_attach_driver+0x9c/0xd0
> bus_for_each_drv+0x70/0xc8
> __device_attach+0xec/0x150
> device_initial_probe+0x10/0x18
> bus_probe_device+0x94/0xa0
> device_add+0x47c/0x780
> platform_device_add+0x110/0x248
> platform_device_register_full+0x120/0x150
> rpi_firmware_probe+0x158/0x1f8
> platform_probe+0x90/0xd8
> really_probe+0x108/0x3c0
> driver_probe_device+0x60/0xc0
> __device_attach_driver+0x9c/0xd0
> bus_for_each_drv+0x70/0xc8
> __device_attach+0xec/0x150
> device_initial_probe+0x10/0x18
> bus_probe_device+0x94/0xa0
> deferred_probe_work_func+0x70/0xa8
> process_one_work+0x2a8/0x718
> worker_thread+0x48/0x460
> kthread+0x134/0x160
> ret_from_fork+0x10/0x18
> Code: b1006294 54c0 b140069f 5488 (3940e280)
> ---[ end trace 7ead5ec2f0c51cfe ]---
>
> This patch mainly revealed that clk/bcm/clk-raspberrypi.c driver calls
> devm_of_clk_add_hw_provider(), with a device pointer, which has a NULL
> dev->of_node. I'm not sure if adding a check for a NULL np in
> of_clk_add_hw_provider() is a right fix, though.
I believe the right fix is not to call 'devm_of_clk_add_hw_provider()' if
'pdev->dev.of_node == NULL'. In such case, which is RPi3's, only the CPU clock
is used, and it's defined and queried later through
devm_clk_hw_register_clkdev().
@Marek, I don't mind taking care of it if it's OK with you.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema
On Wed, 2021-03-24 at 09:27 -0700, Scott Branden wrote: > On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote: > > On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote: > > > On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote: > > > > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote: > > > > > Convert the brcm,iproc-sdhci binding to DT schema format using > > > > > json-schema > > > > > > > > > > Signed-off-by: Nicolas Saenz Julienne > > > > > --- > > > > > .../bindings/mmc/brcm,iproc-sdhci.yaml| 58 > > > > > +++ > > > > > .../bindings/mmc/brcm,sdhci-iproc.txt | 37 > > > > > 2 files changed, 58 insertions(+), 37 deletions(-) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > > > delete mode 100644 > > > > > Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > > > b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > > > new file mode 100644 > > > > > index ..19d84f3ef9e6 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > > > @@ -0,0 +1,58 @@ > > > > > +# SPDX-License-Identifier: GPL-2.0 > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Broadcom IPROC SDHCI controller > > > > > + > > > > > +maintainers: > > > > > + - Nicolas Saenz Julienne > > > > This is already covered in the MAINTAINERS section via "N: iproc". > > > > M: Ray Jui > > > > > > > > M: Scott Branden > > > > > > > > M: bcm-kernel-feedback-l...@broadcom.com > > > > > > Maybe so, but still required here. The problem is there is no > > > MAINTAINERS file in the DT only tree[1]. > > > > Well in that case, if Scott and Ray are OK with it I'll add them. > I do not know what the "maintainers" section in the yaml file is used to > indicate. > If it is maintainer for the driver then please add the duplicate of what is > already in the MAINTAINERS file. If it is for maintainer of devicetrees that > use this driver then no need to add us. From the dt bindings documentation: maintainers A DT specific property. Contains a list of email address(es) for maintainers of this binding. That's the maintainers for the bindings, not the devicetrees consuming them. I belive it makes sense for you guys to maintain it as it has a strong relationship to driver changes. But if you're not interested I'll do it myself. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
Hi Stefan,
On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> Hi Nicolas,
>
> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > From: Nicolas Saenz Julienne
> >
> > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > seems to interfere with the VPU clock when setup at frequencies bigger
> > than 500MHz (a pretty common case). This ends up causing unwarranted
> > SDHCI CMD hangs when no SD card is present.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > index 3b4ab947492a..9aa8408d9960 100644
> > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > @@ -257,6 +257,12 @@ {
> > vqmmc-supply = <_io_1v8_reg>;
> > vmmc-supply = <_vcc_reg>;
> > broken-cd;
> > + /*
> > +* Force the frequency to 150MHz as the default 100MHz seems to
> > +* interfere with the VPU clock when setup at frequencies bigger than
> > +* 500MHz, causing unwarranted CMD hangs.
> > +*/
> > + clock-frequency = <15000>;
>
> i don't want to bike-shed here, but is there any chance to solve this in
> clk-bcm2835 in a less hacky way?
What do you have in mind?
All I can think of is adding some kind of heuristic to the clock's prepare()
callback. That said, I don't feel it would be a better solution than this.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema
On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote: > On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote: > > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote: > > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema > > > > > > Signed-off-by: Nicolas Saenz Julienne > > > --- > > > .../bindings/mmc/brcm,iproc-sdhci.yaml| 58 +++ > > > .../bindings/mmc/brcm,sdhci-iproc.txt | 37 > > > 2 files changed, 58 insertions(+), 37 deletions(-) > > > create mode 100644 > > > Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > delete mode 100644 > > > Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > new file mode 100644 > > > index ..19d84f3ef9e6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > > @@ -0,0 +1,58 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Broadcom IPROC SDHCI controller > > > + > > > +maintainers: > > > + - Nicolas Saenz Julienne > > This is already covered in the MAINTAINERS section via "N: iproc". > > M: Ray Jui > > > > M: Scott Branden > > > > M: bcm-kernel-feedback-l...@broadcom.com > > Maybe so, but still required here. The problem is there is no > MAINTAINERS file in the DT only tree[1]. Well in that case, if Scott and Ray are OK with it I'll add them. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
On Mon, 2021-03-22 at 12:06 -0700, Scott Branden wrote: > 1st line of commit should be ARM: dts: bcm2711 Ouch, of course... Sorry for that. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema
On Mon, 2021-03-22 at 12:11 -0700, Scott Branden wrote: > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote: > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema > > > > Signed-off-by: Nicolas Saenz Julienne > > --- > > .../bindings/mmc/brcm,iproc-sdhci.yaml| 58 +++ > > .../bindings/mmc/brcm,sdhci-iproc.txt | 37 > > 2 files changed, 58 insertions(+), 37 deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > delete mode 100644 > > Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt > > > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > new file mode 100644 > > index ..19d84f3ef9e6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml > > @@ -0,0 +1,58 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Broadcom IPROC SDHCI controller > > + > > +maintainers: > > + - Nicolas Saenz Julienne > This is already covered in the MAINTAINERS section via "N:iproc". > M:Ray Jui > > M:Scott Branden > > M:bcm-kernel-feedback-l...@broadcom.com Fair enough, I'll drop it. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency
From: Nicolas Saenz Julienne
Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
seems to interfere with the VPU clock when setup at frequencies bigger
than 500MHz (a pretty common case). This ends up causing unwarranted
SDHCI CMD hangs when no SD card is present.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 3b4ab947492a..9aa8408d9960 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -257,6 +257,12 @@ {
vqmmc-supply = <_io_1v8_reg>;
vmmc-supply = <_vcc_reg>;
broken-cd;
+ /*
+* Force the frequency to 150MHz as the default 100MHz seems to
+* interfere with the VPU clock when setup at frequencies bigger than
+* 500MHz, causing unwarranted CMD hangs.
+*/
+ clock-frequency = <15000>;
status = "okay";
};
--
2.30.2
[PATCH 2/4] dt-bindings: mmc: iproc-sdhci: Add clock-frequency support
Users might want to choose a different clock frequency than whatever the bootloader provided at probe time. Add 'clock-frequency' bindings. Signed-off-by: Nicolas Saenz Julienne --- Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml index 19d84f3ef9e6..9d0d97b2c0a7 100644 --- a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml @@ -35,6 +35,12 @@ properties: type: boolean description: Specifies that controller should use auto CMD12 + clock-frequency: +description: + Should be the frequency (in Hz) of the core clock. If this is specified + and the core clock is specified then we'll try to set the core clock to + this at probe time. + required: - compatible - reg -- 2.30.2
[PATCH 3/4] mmc: sdhci-iproc: Set clock frequency as per DT
From: Nicolas Saenz Julienne
devicetree might request a clock frequency different from whatever is
set-up by the bootloader. Make sure to setup the new rate.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/mmc/host/sdhci-iproc.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index ddeaf8e1f72f..536c382e2486 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -358,6 +358,16 @@ static int sdhci_iproc_probe(struct platform_device *pdev)
ret = PTR_ERR(pltfm_host->clk);
goto err;
}
+
+ if (pltfm_host->clock) {
+ ret = clk_set_rate(pltfm_host->clk, pltfm_host->clock);
+ if (ret) {
+ dev_err(dev, "failed to set host clk at %u
Hz\n",
+ pltfm_host->clock);
+ goto err;
+ }
+ }
+
ret = clk_prepare_enable(pltfm_host->clk);
if (ret) {
dev_err(dev, "failed to enable host clk\n");
--
2.30.2
[PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema
Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
Signed-off-by: Nicolas Saenz Julienne
---
.../bindings/mmc/brcm,iproc-sdhci.yaml| 58 +++
.../bindings/mmc/brcm,sdhci-iproc.txt | 37
2 files changed, 58 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
new file mode 100644
index ..19d84f3ef9e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom IPROC SDHCI controller
+
+maintainers:
+ - Nicolas Saenz Julienne
+
+allOf:
+ - $ref: mmc-controller.yaml#
+
+properties:
+ compatible:
+enum:
+ - brcm,bcm2835-sdhci
+ - brcm,bcm2711-emmc2
+ - brcm,sdhci-iproc-cygnus
+ - brcm,sdhci-iproc
+
+ reg:
+minItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+description:
+ Handle to core clock for the sdhci controller.
+
+ sdhci,auto-cmd12:
+type: boolean
+description: Specifies that controller should use auto CMD12
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+sdhci0: sdhci@18041000 {
+ compatible = "brcm,sdhci-iproc-cygnus";
+ reg = <0x18041000 0x100>;
+ interrupts = ;
+ clocks = <_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
+ bus-width = <4>;
+ sdhci,auto-cmd12;
+ no-1-8-v;
+};
+
+...
diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
deleted file mode 100644
index 09d87cc1182a..
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Broadcom IPROC SDHCI controller
-
-This file documents differences between the core properties described
-by mmc.txt and the properties that represent the IPROC SDHCI controller.
-
-Required properties:
-- compatible : Should be one of the following
- "brcm,bcm2835-sdhci"
- "brcm,bcm2711-emmc2"
- "brcm,sdhci-iproc-cygnus"
- "brcm,sdhci-iproc"
-
-Use brcm2835-sdhci for the eMMC controller on the BCM2835 (Raspberry Pi) and
-bcm2711-emmc2 for the additional eMMC2 controller on BCM2711.
-
-Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
-restricted to 32bit host accesses to SDHCI registers.
-
-Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
-8, 16, 32-bit host access to SDHCI register.
-
-- clocks : The clock feeding the SDHCI controller.
-
-Optional properties:
- - sdhci,auto-cmd12: specifies that controller should use auto CMD12.
-
-Example:
-
-sdhci0: sdhci@18041000 {
- compatible = "brcm,sdhci-iproc-cygnus";
- reg = <0x18041000 0x100>;
- interrupts = ;
- clocks = <_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
- bus-width = <4>;
- sdhci,auto-cmd12;
- no-1-8-v;
-};
--
2.30.2
[PATCH 0/4] BCM2711's sdhci-iproc CMD timeouts
This series tries to address rather odd behavior from BCM2711's integration of sdhci-iproc (Raspberry Pi 4's SoC). The controller will timeout on SDHCI CMDs under the following conditions: - No SD card plugged in (the card polling thread is running, CD irq disabled). - BCM2711's VPU clock configured at 500MHz or more, lower clocks are OK. There is no specific command that will time out, it seems random. I found out that by bumping the controller's frequency to 150MHz the issue disapears. So let's do that. Regards, Nicolas --- Nicolas Saenz Julienne (4): dt-bindings: mmc: iproc-sdhci: Convert to json-schema dt-bindings: mmc: iproc-sdhci: Add clock-frequency support mmc: sdhci-iproc: Set clock frequency as per DT ARM: dts: Fix-up EMMC2 controller's frequency .../bindings/mmc/brcm,iproc-sdhci.yaml| 64 +++ .../bindings/mmc/brcm,sdhci-iproc.txt | 37 --- arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++ drivers/mmc/host/sdhci-iproc.c| 10 +++ 4 files changed, 80 insertions(+), 37 deletions(-) create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt -- 2.30.2
Re: [PATCH v3 2/2] arm64: mm: reserve CMA and crashkernel in ZONE_DMA32
On Mon, 2021-03-22 at 14:40 -0400, Jon Masters wrote:
> On 3/22/21 2:34 PM, Jon Masters wrote:
> > Hi Nicolas,
> >
> > On 11/7/19 4:56 AM, Nicolas Saenz Julienne wrote:
> > > With the introduction of ZONE_DMA in arm64 we moved the default CMA and
> > > crashkernel reservation into that area. This caused a regression on big
> > > machines that need big CMA and crashkernel reservations. Note that
> > > ZONE_DMA is only 1GB big.
> > >
> > > Restore the previous behavior as the wide majority of devices are OK
> > > with reserving these in ZONE_DMA32. The ones that need them in ZONE_DMA
> > > will configure it explicitly.
> > >
> > > Reported-by: Qian Cai
> > > Signed-off-by: Nicolas Saenz Julienne
> > > ---
> > > arch/arm64/mm/init.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > index 580d1052ac34..8385d3c0733f 100644
> > > --- a/arch/arm64/mm/init.c
> > > +++ b/arch/arm64/mm/init.c
> > > @@ -88,7 +88,7 @@ static void __init reserve_crashkernel(void)
> > > if (crash_base == 0) {
> > > /* Current arm64 boot protocol requires 2MB alignment */
> > > - crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
> > > + crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
> > > crash_size, SZ_2M);
> > > if (crash_base == 0) {
> > > pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
> > > @@ -454,7 +454,7 @@ void __init arm64_memblock_init(void)
> > > high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
> > > - dma_contiguous_reserve(arm64_dma_phys_limit ? :
> > > arm64_dma32_phys_limit);
> > > + dma_contiguous_reserve(arm64_dma32_phys_limit);
> > > }
> > > void __init bootmem_init(void)
> >
> > Can we get a bit more of a backstory about what the regression was on
> > larger machines? If the 32-bit DMA region is too small, but the machine
> > otherwise has plenty of memory, the crashkernel reservation will fail.
> > Most e.g. enterprise users aren't going to respond to that situation by
> > determining the placement manually, they'll just not have a crashkernel.
>
> Nevermind, looks like Catalin already changed this logic in Jan 2021 by
> removing arm64_dma32_phys_limit and I'm out of date.
Also see this series (already merged):
https://lore.kernel.org/linux-arm-kernel/20201119175400.9995-1-nsaenzjulie...@suse.de/
Regads,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH] dt-bindings: Drop type references on common properties
On Tue, 2021-03-16 at 13:48 -0600, Rob Herring wrote: > Users of common properties shouldn't have a type definition as the > common schemas already have one. Drop all the unnecessary type > references in the tree. > > A meta-schema update to catch these is pending. > > Cc: Nicolas Saenz Julienne > Cc: Maxime Ripard > Cc: Linus Walleij > Cc: Bartosz Golaszewski > Cc: Bjorn Andersson > Cc: Krzysztof Kozlowski > Cc: Marc Kleine-Budde > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: Srinivas Kandagatla > Cc: Ohad Ben-Cohen > Cc: Mark Brown > Cc: Cheng-Yi Chiang > Cc: Benson Leung > Cc: Zhang Rui > Cc: Daniel Lezcano > Cc: Greg Kroah-Hartman > Cc: Stefan Wahren > Cc: Masahiro Yamada > Cc: Odelu Kukatla > Cc: Alex Elder > Cc: Suman Anna > Cc: Kuninori Morimoto > Cc: Dmitry Baryshkov > Cc: linux-g...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: net...@vger.kernel.org > Cc: linux-remotep...@vger.kernel.org > Cc: alsa-de...@alsa-project.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Rob Herring > --- > .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml | 5 + ^ | Reviewed-by: Nicolas Saenz Julienne Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v8 11/11] pwm: Add Raspberry Pi Firmware based PWM bus
Hi Uwe, On Fri, 2021-03-12 at 21:12 +0100, Uwe Kleine-König wrote: > Hello Nicolas, > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright 2020 Nicolas Saenz Julienne > > 2021? Yes. > > + * For more information on Raspberry Pi's PoE hat see: > > + * https://www.raspberrypi.org/products/poe-hat/ > > Out of personal interest: Is this hat also able to power a RPi CM4? I haven't tested it, and can't at the moment (no PoE injector available). But the physical pin layout, and routing in CM4's IO board fits the hat. So I'd say yes. > > + * Limitations: > > + * - No disable bit, so a disabled PWM is simulated by duty_cycle 0 > > + * - Only normal polarity > > + * - Fixed 12.5 kHz period > > + * > > + * The current period is completed when HW is reconfigured. > > + */ > > Other than that as mentioned in the previous round: This looks good, > > Reviewed-by: Uwe Kleine-König Thanks! > What is your thought about how to get this series merged? > At least input, staging, armsoc, clk, reset anf firmware are touched. Do you > prepare a branch for merging in the relevant trees (once you have all the > necessary Acks)? As per Linusw suggestion I'll send a pull request myself into the SoC tree and hope for the best. :) Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH] nvmem: rmem: fix undefined reference to memremap
On Mon, 2021-03-15 at 10:44 +, Srinivas Kandagatla wrote: > Fix below error reporte by kernel test robot > rmem.c:(.text+0x14e): undefined reference to memremap > s390x-linux-gnu-ld: rmem.c:(.text+0x1b6): undefined reference to memunmap > > Reported-by: kernel test robot > Signed-off-by: Srinivas Kandagatla > --- Reviewed-by: Nicolas Saenz Julienne Thanks! > drivers/nvmem/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > index 642ddc699fd1..dd2019006838 100644 > --- a/drivers/nvmem/Kconfig > +++ b/drivers/nvmem/Kconfig > @@ -272,6 +272,7 @@ config SPRD_EFUSE > > > config NVMEM_RMEM > tristate "Reserved Memory Based Driver Support" > + depends on HAS_IOMEM > help > This driver maps reserved memory into an nvmem device. It might be > useful to expose information left by firmware in memory. signature.asc Description: This is a digitally signed message part
[PATCH v8 11/11] pwm: Add Raspberry Pi Firmware based PWM bus
Adds support to control the PWM bus available in official Raspberry Pi
PoE HAT. Only RPi's co-processor has access to it, so commands have to
be sent through RPi's firmware mailbox interface.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v7:
- Remove unwarranted RPI_PWM_DEF_DUTY_REG usage
Changes since v6:
- Use %pe
- Round divisions properly
- Use dev_err_probe()
- Pass check_patch
Changes since v3:
- Rename compatible string to be more explicit WRT to bus's limitations
Changes since v2:
- Use devm_rpi_firmware_get()
- Rename driver
- Small cleanups
Changes since v1:
- Use default pwm bindings and get rid of xlate() function
- Correct spelling errors
- Correct apply() function
- Round values
- Fix divisions in arm32 mode
- Small cleanups
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-raspberrypi-poe.c | 206 ++
3 files changed, 216 insertions(+)
create mode 100644 drivers/pwm/pwm-raspberrypi-poe.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index a7a7a9f26aef..d3371ac7b871 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -431,6 +431,15 @@ config PWM_PXA
To compile this driver as a module, choose M here: the module
will be called pwm-pxa.
+config PWM_RASPBERRYPI_POE
+ tristate "Raspberry Pi Firwmware PoE Hat PWM support"
+ # Make sure not 'y' when RASPBERRYPI_FIRMWARE is 'm'. This can only
+ # happen when COMPILE_TEST=y, hence the added !RASPBERRYPI_FIRMWARE.
+ depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
!RASPBERRYPI_FIRMWARE)
+ help
+ Enable Raspberry Pi firmware controller PWM bus used to control the
+ official RPI PoE hat
+
config PWM_RCAR
tristate "Renesas R-Car PWM support"
depends on ARCH_RENESAS || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 4e35a55fa7b6..d3879619bd76 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_PWM_NTXEC) += pwm-ntxec.o
obj-$(CONFIG_PWM_OMAP_DMTIMER) += pwm-omap-dmtimer.o
obj-$(CONFIG_PWM_PCA9685) += pwm-pca9685.o
obj-$(CONFIG_PWM_PXA) += pwm-pxa.o
+obj-$(CONFIG_PWM_RASPBERRYPI_POE) += pwm-raspberrypi-poe.o
obj-$(CONFIG_PWM_RCAR) += pwm-rcar.o
obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o
obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o
diff --git a/drivers/pwm/pwm-raspberrypi-poe.c
b/drivers/pwm/pwm-raspberrypi-poe.c
new file mode 100644
index ..71ade5e55069
--- /dev/null
+++ b/drivers/pwm/pwm-raspberrypi-poe.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020 Nicolas Saenz Julienne
+ * For more information on Raspberry Pi's PoE hat see:
+ * https://www.raspberrypi.org/products/poe-hat/
+ *
+ * Limitations:
+ * - No disable bit, so a disabled PWM is simulated by duty_cycle 0
+ * - Only normal polarity
+ * - Fixed 12.5 kHz period
+ *
+ * The current period is completed when HW is reconfigured.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#define RPI_PWM_MAX_DUTY 255
+#define RPI_PWM_PERIOD_NS 8 /* 12.5 kHz */
+
+#define RPI_PWM_CUR_DUTY_REG 0x0
+
+struct raspberrypi_pwm {
+ struct rpi_firmware *firmware;
+ struct pwm_chip chip;
+ unsigned int duty_cycle;
+};
+
+struct raspberrypi_pwm_prop {
+ __le32 reg;
+ __le32 val;
+ __le32 ret;
+} __packed;
+
+static inline
+struct raspberrypi_pwm *raspberrypi_pwm_from_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct raspberrypi_pwm, chip);
+}
+
+static int raspberrypi_pwm_set_property(struct rpi_firmware *firmware,
+ u32 reg, u32 val)
+{
+ struct raspberrypi_pwm_prop msg = {
+ .reg = cpu_to_le32(reg),
+ .val = cpu_to_le32(val),
+ };
+ int ret;
+
+ ret = rpi_firmware_property(firmware, RPI_FIRMWARE_SET_POE_HAT_VAL,
+ , sizeof(msg));
+ if (ret)
+ return ret;
+ if (msg.ret)
+ return -EIO;
+
+ return 0;
+}
+
+static int raspberrypi_pwm_get_property(struct rpi_firmware *firmware,
+ u32 reg, u32 *val)
+{
+ struct raspberrypi_pwm_prop msg = {
+ .reg = reg
+ };
+ int ret;
+
+ ret = rpi_firmware_property(firmware, RPI_FIRMWARE_GET_POE_HAT_VAL,
+ , sizeof(msg));
+ if (ret)
+ return ret;
+ if (msg.ret)
+ return -EIO;
+
+ *val = le32_to_cpu(msg.val);
+
+ return 0;
+}
+
+static void raspberrypi_pwm_get_state(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ st
[PATCH v8 10/11] DO NOT MERGE: ARM: dts: Add RPi's official PoE hat support
This is an example on how to enable the fan on top of RPi's official PoE
hat.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 54 +++
1 file changed, 54 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 3b4ab947492a..6897c7831d09 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -5,6 +5,7 @@
#include "bcm283x-rpi-usb-peripheral.dtsi"
#include
+#include
/ {
compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
@@ -69,6 +70,54 @@ sd_vcc_reg: sd_vcc_reg {
enable-active-high;
gpio = < 6 GPIO_ACTIVE_HIGH>;
};
+
+ fan: pwm-fan {
+ compatible = "pwm-fan";
+ cooling-levels = <0 50 150 255>;
+ #cooling-cells = <2>;
+ pwms = < RASPBERRYPI_FIRMWARE_PWM_POE 8>;
+ };
+
+ thermal-zones {
+ cpu_thermal: cpu-thermal {
+ trips {
+ threshold: trip-point@0 {
+ temperature = <45000>;
+ hysteresis = <5000>;
+ type = "active";
+ };
+
+ target: trip-point@1 {
+ temperature = <5>;
+ hysteresis = <2000>;
+ type = "active";
+ };
+
+ cpu_hot: cpu_hot@0 {
+ temperature = <55000>;
+ hysteresis = <2000>;
+ type = "active";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <>;
+ cooling-device = < 0 1>;
+ };
+
+ map1 {
+ trip = <>;
+ cooling-device = < 1 2>;
+ };
+
+ map2 {
+ trip = <_hot>;
+ cooling-device = < 2 3>;
+ };
+ };
+ };
+ };
};
{
@@ -104,6 +153,11 @@ reset: reset {
compatible = "raspberrypi,firmware-reset";
#reset-cells = <1>;
};
+
+ fwpwm: pwm {
+ compatible = "raspberrypi,firmware-poe-pwm";
+ #pwm-cells = <2>;
+ };
};
{
--
2.30.1
[PATCH v8 09/11] dt-bindings: pwm: Add binding for RPi firmware PWM bus
The PWM bus controlling the fan in RPi's official PoE hat can only be
controlled by the board's co-processor.
Signed-off-by: Nicolas Saenz Julienne
Reviewed-by: Rob Herring
---
Changes since v4:
- Rename compatible string to be more explicit with the bus' limitations
Changes since v3:
- Fix example
Changes since v1:
- Update bindings to use 2 #pwm-cells
.../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 20 +++
.../pwm/raspberrypi,firmware-poe-pwm.h| 13
2 files changed, 33 insertions(+)
create mode 100644 include/dt-bindings/pwm/raspberrypi,firmware-poe-pwm.h
diff --git
a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index a2c63c8b1d10..e3664eab0f6a 100644
---
a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++
b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -64,6 +64,21 @@ properties:
- compatible
- "#reset-cells"
+ pwm:
+type: object
+
+properties:
+ compatible:
+const: raspberrypi,firmware-poe-pwm
+
+ "#pwm-cells":
+# See pwm.yaml in this directory for a description of the cells format.
+const: 2
+
+required:
+ - compatible
+ - "#pwm-cells"
+
additionalProperties: false
required:
@@ -87,5 +102,10 @@ examples:
compatible = "raspberrypi,firmware-reset";
#reset-cells = <1>;
};
+
+pwm: pwm {
+compatible = "raspberrypi,firmware-poe-pwm";
+#pwm-cells = <2>;
+};
};
...
diff --git a/include/dt-bindings/pwm/raspberrypi,firmware-poe-pwm.h
b/include/dt-bindings/pwm/raspberrypi,firmware-poe-pwm.h
new file mode 100644
index ..27c5ce68847b
--- /dev/null
+++ b/include/dt-bindings/pwm/raspberrypi,firmware-poe-pwm.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020 Nicolas Saenz Julienne
+ * Author: Nicolas Saenz Julienne
+ */
+
+#ifndef _DT_BINDINGS_RASPBERRYPI_FIRMWARE_PWM_H
+#define _DT_BINDINGS_RASPBERRYPI_FIRMWARE_PWM_H
+
+#define RASPBERRYPI_FIRMWARE_PWM_POE 0
+#define RASPBERRYPI_FIRMWARE_PWM_NUM 1
+
+#endif
--
2.30.1
[PATCH v8 05/11] reset: raspberrypi: Release firmware handle on unbind
Use devm_rpi_firmware_get() so as to make sure we release RPi's firmware interface when unbinding the device. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Florian Fainelli --- drivers/reset/reset-raspberrypi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/reset/reset-raspberrypi.c b/drivers/reset/reset-raspberrypi.c index 02f59c06f69b..fa23db554bcf 100644 --- a/drivers/reset/reset-raspberrypi.c +++ b/drivers/reset/reset-raspberrypi.c @@ -82,7 +82,7 @@ static int rpi_reset_probe(struct platform_device *pdev) return -ENOENT; } - fw = rpi_firmware_get(np); + fw = devm_rpi_firmware_get(>dev, np); of_node_put(np); if (!fw) return -EPROBE_DEFER; -- 2.30.1
[PATCH v8 04/11] gpio: raspberrypi-exp: Release firmware handle on unbind
Use devm_rpi_firmware_get() so as to make sure we release RPi's firmware interface when unbinding the device. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Florian Fainelli Acked-by: Bartosz Golaszewski --- drivers/gpio/gpio-raspberrypi-exp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-raspberrypi-exp.c b/drivers/gpio/gpio-raspberrypi-exp.c index bb100e0124e6..64a552ecc2ad 100644 --- a/drivers/gpio/gpio-raspberrypi-exp.c +++ b/drivers/gpio/gpio-raspberrypi-exp.c @@ -208,7 +208,7 @@ static int rpi_exp_gpio_probe(struct platform_device *pdev) return -ENOENT; } - fw = rpi_firmware_get(fw_node); + fw = devm_rpi_firmware_get(>dev, fw_node); of_node_put(fw_node); if (!fw) return -EPROBE_DEFER; -- 2.30.1
[PATCH v8 06/11] soc: bcm: raspberrypi-power: Release firmware handle on unbind
Use devm_rpi_firmware_get() so as to make sure we release RPi's firmware interface when unbinding the device. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Florian Fainelli --- drivers/soc/bcm/raspberrypi-power.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/bcm/raspberrypi-power.c b/drivers/soc/bcm/raspberrypi-power.c index 5d1aacdd84ef..068715d6e66d 100644 --- a/drivers/soc/bcm/raspberrypi-power.c +++ b/drivers/soc/bcm/raspberrypi-power.c @@ -177,7 +177,7 @@ static int rpi_power_probe(struct platform_device *pdev) return -ENODEV; } - rpi_domains->fw = rpi_firmware_get(fw_np); + rpi_domains->fw = devm_rpi_firmware_get(>dev, fw_np); of_node_put(fw_np); if (!rpi_domains->fw) return -EPROBE_DEFER; -- 2.30.1
[PATCH v8 07/11] staging: vchiq: Release firmware handle on unbind
Use devm_rpi_firmware_get() so as to make sure we release RPi's firmware interface when unbinding the device. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Florian Fainelli Reviewed-by: Greg Kroah-Hartman --- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 8b2b4771f420..e39897c38e6a 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -2738,7 +2738,7 @@ static int vchiq_probe(struct platform_device *pdev) return -ENOENT; } - drvdata->fw = rpi_firmware_get(fw_node); + drvdata->fw = devm_rpi_firmware_get(>dev, fw_node); of_node_put(fw_node); if (!drvdata->fw) return -EPROBE_DEFER; -- 2.30.1
[PATCH v8 03/11] clk: bcm: rpi: Release firmware handle on unbind
Use devm_rpi_firmware_get() so as to make sure we release RPi's firmware interface when unbinding the device. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Florian Fainelli Acked-by: Stephen Boyd drivers/clk/bcm/clk-raspberrypi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- drivers/clk/bcm/clk-raspberrypi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index f89b9cfc4309..dd3b71eafabf 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -314,7 +314,7 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) return -ENOENT; } - firmware = rpi_firmware_get(firmware_node); + firmware = devm_rpi_firmware_get(>dev, firmware_node); of_node_put(firmware_node); if (!firmware) return -EPROBE_DEFER; -- 2.30.1
[PATCH v8 02/11] firmware: raspberrypi: Introduce devm_rpi_firmware_get()
It'll simplify the firmware handling for most consumers.
Suggested-by: Bartosz Golaszewski
Signed-off-by: Nicolas Saenz Julienne
Reviewed-by: Florian Fainelli
Reviewed-by: Bartosz Golaszewski
---
Changes since v4:
- Rearrange function calls for clarity, same functionality
Changes since v2:
- Create devm_rpi_firmware_get()
drivers/firmware/raspberrypi.c | 29 ++
include/soc/bcm2835/raspberrypi-firmware.h | 8 ++
2 files changed, 37 insertions(+)
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index b65e4c495772..250e01680742 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -243,6 +243,13 @@ void rpi_firmware_put(struct rpi_firmware *fw)
}
EXPORT_SYMBOL_GPL(rpi_firmware_put);
+static void devm_rpi_firmware_put(void *data)
+{
+ struct rpi_firmware *fw = data;
+
+ rpi_firmware_put(fw);
+}
+
static int rpi_firmware_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
@@ -331,6 +338,28 @@ struct rpi_firmware *rpi_firmware_get(struct device_node
*firmware_node)
}
EXPORT_SYMBOL_GPL(rpi_firmware_get);
+/**
+ * devm_rpi_firmware_get - Get pointer to rpi_firmware structure.
+ * @firmware_node:Pointer to the firmware Device Tree node.
+ *
+ * Returns NULL is the firmware device is not ready.
+ */
+struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
+ struct device_node *firmware_node)
+{
+ struct rpi_firmware *fw;
+
+ fw = rpi_firmware_get(firmware_node);
+ if (!fw)
+ return NULL;
+
+ if (devm_add_action_or_reset(dev, devm_rpi_firmware_put, fw))
+ return NULL;
+
+ return fw;
+}
+EXPORT_SYMBOL_GPL(devm_rpi_firmware_get);
+
static const struct of_device_id rpi_firmware_of_match[] = {
{ .compatible = "raspberrypi,bcm2835-firmware", },
{},
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h
b/include/soc/bcm2835/raspberrypi-firmware.h
index fdfef7fe40df..73ad784fca96 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -142,6 +142,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
void *data, size_t tag_size);
void rpi_firmware_put(struct rpi_firmware *fw);
struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
+struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
+ struct device_node *firmware_node);
#else
static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
void *data, size_t len)
@@ -160,6 +162,12 @@ static inline struct rpi_firmware *rpi_firmware_get(struct
device_node *firmware
{
return NULL;
}
+
+static inline struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
+ struct device_node *firmware_node)
+{
+ return NULL;
+}
#endif
#endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
--
2.30.1
[PATCH v8 08/11] input: raspberrypi-ts: Release firmware handle when not needed
There is no use for the firmware interface after getting the touch
buffer address, so release it.
Signed-off-by: Nicolas Saenz Julienne
Acked-by: Dmitry Torokhov
Reviewed-by: Florian Fainelli
---
Changes since v5:
- Correct commit message
Changes since v3:
- Release firmware handle in probe function
Changes since v2:
- Use devm_rpi_firmware_get(), instead of remove function
drivers/input/touchscreen/raspberrypi-ts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/raspberrypi-ts.c
b/drivers/input/touchscreen/raspberrypi-ts.c
index ef6aaed217cf..5000f5fd9ec3 100644
--- a/drivers/input/touchscreen/raspberrypi-ts.c
+++ b/drivers/input/touchscreen/raspberrypi-ts.c
@@ -160,7 +160,7 @@ static int rpi_ts_probe(struct platform_device *pdev)
touchbuf = (u32)ts->fw_regs_phys;
error = rpi_firmware_property(fw, RPI_FIRMWARE_FRAMEBUFFER_SET_TOUCHBUF,
, sizeof(touchbuf));
-
+ rpi_firmware_put(fw);
if (error || touchbuf != 0) {
dev_warn(dev, "Failed to set touchbuf, %d\n", error);
return error;
--
2.30.1
[PATCH v8 01/11] firmware: raspberrypi: Keep count of all consumers
When unbinding the firmware device we need to make sure it has no
consumers left. Otherwise we'd leave them with a firmware handle
pointing at freed memory.
Keep a reference count of all consumers and introduce rpi_firmware_put()
which will permit automatically decrease the reference count upon
unbinding consumer drivers.
Suggested-by: Uwe Kleine-König
Signed-off-by: Nicolas Saenz Julienne
Reviewed-by: Florian Fainelli
Reviewed-by: Stephen Boyd
Reviewed-by: Bartosz Golaszewski
---
Changes since v5:
- Add comment to avoid people blindly switching the memory allocation
to the devm variant.
- Fix function documentation as per Florian's comment.
Changes since v3:
- Use kref instead of waiting on refcount
drivers/firmware/raspberrypi.c | 40 --
include/soc/bcm2835/raspberrypi-firmware.h | 2 ++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 30259dc9b805..b65e4c495772 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -27,6 +28,8 @@ struct rpi_firmware {
struct mbox_chan *chan; /* The property channel. */
struct completion c;
u32 enabled;
+
+ struct kref consumers;
};
static DEFINE_MUTEX(transaction_lock);
@@ -225,12 +228,31 @@ static void rpi_register_clk_driver(struct device *dev)
-1, NULL, 0);
}
+static void rpi_firmware_delete(struct kref *kref)
+{
+ struct rpi_firmware *fw = container_of(kref, struct rpi_firmware,
+ consumers);
+
+ mbox_free_channel(fw->chan);
+ kfree(fw);
+}
+
+void rpi_firmware_put(struct rpi_firmware *fw)
+{
+ kref_put(>consumers, rpi_firmware_delete);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_put);
+
static int rpi_firmware_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct rpi_firmware *fw;
- fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
+ /*
+* Memory will be freed by rpi_firmware_delete() once all users have
+* released their firmware handles. Don't use devm_kzalloc() here.
+*/
+ fw = kzalloc(sizeof(*fw), GFP_KERNEL);
if (!fw)
return -ENOMEM;
@@ -247,6 +269,7 @@ static int rpi_firmware_probe(struct platform_device *pdev)
}
init_completion(>c);
+ kref_init(>consumers);
platform_set_drvdata(pdev, fw);
@@ -275,7 +298,8 @@ static int rpi_firmware_remove(struct platform_device *pdev)
rpi_hwmon = NULL;
platform_device_unregister(rpi_clk);
rpi_clk = NULL;
- mbox_free_channel(fw->chan);
+
+ rpi_firmware_put(fw);
return 0;
}
@@ -284,16 +308,26 @@ static int rpi_firmware_remove(struct platform_device
*pdev)
* rpi_firmware_get - Get pointer to rpi_firmware structure.
* @firmware_node:Pointer to the firmware Device Tree node.
*
+ * The reference to rpi_firmware has to be released with rpi_firmware_put().
+ *
* Returns NULL is the firmware device is not ready.
*/
struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)
{
struct platform_device *pdev = of_find_device_by_node(firmware_node);
+ struct rpi_firmware *fw;
if (!pdev)
return NULL;
- return platform_get_drvdata(pdev);
+ fw = platform_get_drvdata(pdev);
+ if (!fw)
+ return NULL;
+
+ if (!kref_get_unless_zero(>consumers))
+ return NULL;
+
+ return fw;
}
EXPORT_SYMBOL_GPL(rpi_firmware_get);
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h
b/include/soc/bcm2835/raspberrypi-firmware.h
index cc9cdbc66403..fdfef7fe40df 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -140,6 +140,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
u32 tag, void *data, size_t len);
int rpi_firmware_property_list(struct rpi_firmware *fw,
void *data, size_t tag_size);
+void rpi_firmware_put(struct rpi_firmware *fw);
struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
#else
static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
@@ -154,6 +155,7 @@ static inline int rpi_firmware_property_list(struct
rpi_firmware *fw,
return -ENOSYS;
}
+static inline void rpi_firmware_put(struct rpi_firmware *fw) { }
static inline struct rpi_firmware *rpi_firmware_get(struct device_node
*firmware_node)
{
return NULL;
--
2.30.1
[PATCH v8 00/11] Raspberry Pi PoE HAT fan support
The aim of this series is to add support to the fan found on RPi's PoE HAT. Some commentary on the design can be found below. But the important part to the people CC'd here not involved with PWM is that, in order to achieve this properly, we also have to fix the firmware interface the driver uses to communicate with the PWM bus (and many other low level functions). Specifically, we have to make sure the firmware interface isn't unbound while consumers are still up. So, patch #1 & #2 introduce reference counting in the firmware interface driver and patches #3 to #8 update all firmware users. Patches #9 to #11 introduce the new PWM driver. I sent everything as a single series as the final version of the PWM drivers depends on the firmware fixes, but I'll be happy to split this into two separate series if you think it's better. --- Original cover letter below --- This series aims at adding support to RPi's official PoE HAT fan[1]. The HW setup is the following: | Raspberry Pi | PoE HAT| arm core -> Mailbox -> RPi co-processor -> I2C -> Atmel MCU -> PWM -> FAN The arm cores have only access to the mailbox interface, as i2c0, even if physically accessible, is to be used solely by the co-processor (VideoCore 4/6). This series implements a PWM bus, and has pwm-fan sitting on top of it as per this discussion: https://lkml.org/lkml/2018/9/2/486. Although this design has a series of shortcomings: - It depends on a DT binding: it's not flexible if a new hat shows up with new functionality, we're not 100% sure we'll be able to expand it without breaking backwards compatibility. But without it we can't make use of DT thermal-zones, which IMO is overkill. - We're using pwm-fan, writing a hwmon driver would, again, give us more flexibility, but it's not really needed at the moment. I personally think that it's not worth the effort, it's unlikely we'll get things right in advance. And ultimately, if the RPi people come up with something new, we can always write a new driver/bindings from scratch (as in not reusing previous code). That said, I'm more than happy to change things if there is a consensus that another design will do the trick. [1] https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ --- Changes since v7: - Remove unwarranted RPI_PWM_DEF_DUTY_REG usage Changes since v6: - Address PWM driver comments Changes since v5: - Small cleanups - Add extra code comments Changes since v4: - Cleanup devm calls - Rename compatible string so it's unique to the PoE HAT Changes since v3: - Split first patch, #1 introduces refcount, then #2 the devm function - Fix touchscreen function - Use kref Changes since v2: - Introduce devm_rpi_firmware_get() - Small cleanups in PWM driver Changes since v1: - Address PWM driver changes - Fix binding, now with 2 cells Nicolas Saenz Julienne (11): firmware: raspberrypi: Keep count of all consumers firmware: raspberrypi: Introduce devm_rpi_firmware_get() clk: bcm: rpi: Release firmware handle on unbind gpio: raspberrypi-exp: Release firmware handle on unbind reset: raspberrypi: Release firmware handle on unbind soc: bcm: raspberrypi-power: Release firmware handle on unbind staging: vchiq: Release firmware handle on unbind input: raspberrypi-ts: Release firmware handle when not needed dt-bindings: pwm: Add binding for RPi firmware PWM bus DO NOT MERGE: ARM: dts: Add RPi's official PoE hat support pwm: Add Raspberry Pi Firmware based PWM bus .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 20 ++ arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 54 + drivers/clk/bcm/clk-raspberrypi.c | 2 +- drivers/firmware/raspberrypi.c| 69 +- drivers/gpio/gpio-raspberrypi-exp.c | 2 +- drivers/input/touchscreen/raspberrypi-ts.c| 2 +- drivers/pwm/Kconfig | 9 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-raspberrypi-poe.c | 206 ++ drivers/reset/reset-raspberrypi.c | 2 +- drivers/soc/bcm/raspberrypi-power.c | 2 +- .../interface/vchiq_arm/vchiq_arm.c | 2 +- .../pwm/raspberrypi,firmware-poe-pwm.h| 13 ++ include/soc/bcm2835/raspberrypi-firmware.h| 10 + 14 files changed, 385 insertions(+), 9 deletions(-) create mode 100644 drivers/pwm/pwm-raspberrypi-poe.c create mode 100644 include/dt-bindings/pwm/raspberrypi,firmware-poe-pwm.h -- 2.30.1
Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus
On Thu, 2021-03-11 at 14:18 +0100, Uwe Kleine-König wrote:
> Hello Nicolas,
>
> On Thu, Mar 11, 2021 at 02:01:00PM +0100, Nicolas Saenz Julienne wrote:
> > On Wed, 2021-03-10 at 12:50 +0100, Uwe Kleine-König wrote:
> > > On Mon, Jan 18, 2021 at 01:32:44PM +0100, Nicolas Saenz Julienne wrote:
> >
> > [...]
> >
> > > > + /*
> > > > +* This sets the default duty cycle after resetting the board,
> > > > we
> > > > +* updated it every time to mimic Raspberry Pi's downstream's
> > > > driver
> > > > +* behaviour.
> > > > +*/
> > > > + ret = raspberrypi_pwm_set_property(rpipwm->firmware,
> > > > RPI_PWM_DEF_DUTY_REG,
> > > > + duty_cycle);
> > > > + if (ret) {
> > > > + dev_err(chip->dev, "Failed to set default duty cycle:
> > > > %pe\n",
> > > > + ERR_PTR(ret));
> > > > + return ret;
> > >
> > > This only has an effect for the next reboot, right?
> >
> > It effects all reboots until it's further changed.
> >
> > > If so I wonder if it is a good idea in general. (Think: The current PWM
> > > setting enables a motor that makes a self-driving car move at 100 km/h.
> > > Consider the rpi crashes, do I want to car to pick up driving 100 km/h at
> > > power up even before Linux is up again?)
> >
> > I get your point. But this isn't used as a general purpose PWM. For now the
> > interface is solely there to drive a PWM fan that's arguably harmless. This
> > doesn't mean that the RPi foundation will not reuse the firmware interface
> > for
> > other means in the future. In such case we can always use a new DT
> > compatible
> > and bypass this feature (the current DT string is
> > 'raspberrypi,firmware-poe-pwm', which is specific to this use-case).
> >
> > My aim here is to be on par feature wise with RPi's downstream
> > implementation.
>
> Just because the downstream kernel does it should not be the (single)
> reason to do that. My gut feeling is: For a motor restoring the PWM
> config on reboot is bad and for a fan it doesn't really hurt if it
> doesn't restart automatically. So I'd prefer to to drop this feature.
Fair enough, I'll remove it then.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus
Hi Uwe,
thanks for taking the time to look into this. :)
On Wed, 2021-03-10 at 12:50 +0100, Uwe Kleine-König wrote:
> Hello Nicolas,
>
> On Mon, Jan 18, 2021 at 01:32:44PM +0100, Nicolas Saenz Julienne wrote:
[...]
> > + /*
> > +* This sets the default duty cycle after resetting the board, we
> > +* updated it every time to mimic Raspberry Pi's downstream's driver
> > +* behaviour.
> > +*/
> > + ret = raspberrypi_pwm_set_property(rpipwm->firmware,
> > RPI_PWM_DEF_DUTY_REG,
> > + duty_cycle);
> > + if (ret) {
> > + dev_err(chip->dev, "Failed to set default duty cycle: %pe\n",
> > + ERR_PTR(ret));
> > + return ret;
>
> This only has an effect for the next reboot, right?
It effects all reboots until it's further changed.
> If so I wonder if it is a good idea in general. (Think: The current PWM
> setting enables a motor that makes a self-driving car move at 100 km/h.
> Consider the rpi crashes, do I want to car to pick up driving 100 km/h at
> power up even before Linux is up again?)
I get your point. But this isn't used as a general purpose PWM. For now the
interface is solely there to drive a PWM fan that's arguably harmless. This
doesn't mean that the RPi foundation will not reuse the firmware interface for
other means in the future. In such case we can always use a new DT compatible
and bypass this feature (the current DT string is
'raspberrypi,firmware-poe-pwm', which is specific to this use-case).
My aim here is to be on par feature wise with RPi's downstream implementation.
So as for them to be able to use it as is and avoid duplication. Now, if this
is blocking the driver from being merged, I'd rather remove it. It'll be a
patch for the downstream kernel to maintain, but better than nothing.
> And if we agree it's a good idea: Should raspberrypi_pwm_apply return 0 if
> setting the duty cycle succeeded and only setting the default didn't?
Good point. I don't think so. We'd be also missing on the following by
returning early:
rpipwm->duty_cycle = duty_cycle;
I propose to change it to a 'best effort' approach, if it fails, log it and
continue successfully.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v7 11/11] pwm: Add Raspberry Pi Firmware based PWM bus
On Mon, 2021-01-18 at 13:32 +0100, Nicolas Saenz Julienne wrote:
> Adds support to control the PWM bus available in official Raspberry Pi
> PoE HAT. Only RPi's co-processor has access to it, so commands have to
> be sent through RPi's firmware mailbox interface.
>
> Signed-off-by: Nicolas Saenz Julienne
>
> ---
ping :)
> Changes since v6:
> - Use %pe
> - Round divisions properly
> - Use dev_err_probe()
> - Pass check_patch
>
> Changes since v3:
> - Rename compatible string to be more explicit WRT to bus's limitations
>
> Changes since v2:
> - Use devm_rpi_firmware_get()
> - Rename driver
> - Small cleanups
>
> Changes since v1:
> - Use default pwm bindings and get rid of xlate() function
> - Correct spelling errors
> - Correct apply() function
> - Round values
> - Fix divisions in arm32 mode
> - Small cleanups
>
> drivers/pwm/Kconfig | 9 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-raspberrypi-poe.c | 220 ++
> 3 files changed, 230 insertions(+)
> create mode 100644 drivers/pwm/pwm-raspberrypi-poe.c
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 0937e1c047ac..75e2344703b3 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -423,6 +423,15 @@ config PWM_PXA
> To compile this driver as a module, choose M here: the module
> will be called pwm-pxa.
>
>
> +config PWM_RASPBERRYPI_POE
> + tristate "Raspberry Pi Firwmware PoE Hat PWM support"
> + # Make sure not 'y' when RASPBERRYPI_FIRMWARE is 'm'. This can only
> + # happen when COMPILE_TEST=y, hence the added !RASPBERRYPI_FIRMWARE.
> + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
> !RASPBERRYPI_FIRMWARE)
> + help
> + Enable Raspberry Pi firmware controller PWM bus used to control the
> + official RPI PoE hat
> +
> config PWM_RCAR
> tristate "Renesas R-Car PWM support"
> depends on ARCH_RENESAS || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 18b89d7fd092..ed28d7bd4c64 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_PWM_MXS) += pwm-mxs.o
> obj-$(CONFIG_PWM_OMAP_DMTIMER) += pwm-omap-dmtimer.o
> obj-$(CONFIG_PWM_PCA9685)+= pwm-pca9685.o
> obj-$(CONFIG_PWM_PXA)+= pwm-pxa.o
> +obj-$(CONFIG_PWM_RASPBERRYPI_POE)+= pwm-raspberrypi-poe.o
> obj-$(CONFIG_PWM_RCAR) += pwm-rcar.o
> obj-$(CONFIG_PWM_RENESAS_TPU)+= pwm-renesas-tpu.o
> obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o
> diff --git a/drivers/pwm/pwm-raspberrypi-poe.c
> b/drivers/pwm/pwm-raspberrypi-poe.c
> new file mode 100644
> index ..ca845e8fabe6
> --- /dev/null
> +++ b/drivers/pwm/pwm-raspberrypi-poe.c
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 Nicolas Saenz Julienne
> + * For more information on Raspberry Pi's PoE hat see:
> + * https://www.raspberrypi.org/products/poe-hat/
> + *
> + * Limitations:
> + * - No disable bit, so a disabled PWM is simulated by duty_cycle 0
> + * - Only normal polarity
> + * - Fixed 12.5 kHz period
> + *
> + * The current period is completed when HW is reconfigured.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#define RPI_PWM_MAX_DUTY 255
> +#define RPI_PWM_PERIOD_NS8 /* 12.5 kHz */
> +
> +#define RPI_PWM_CUR_DUTY_REG 0x0
> +#define RPI_PWM_DEF_DUTY_REG 0x1
> +
> +struct raspberrypi_pwm {
> + struct rpi_firmware *firmware;
> + struct pwm_chip chip;
> + unsigned int duty_cycle;
> +};
> +
> +struct raspberrypi_pwm_prop {
> + __le32 reg;
> + __le32 val;
> + __le32 ret;
> +} __packed;
> +
> +static inline
> +struct raspberrypi_pwm *raspberrypi_pwm_from_chip(struct pwm_chip *chip)
> +{
> + return container_of(chip, struct raspberrypi_pwm, chip);
> +}
> +
> +static int raspberrypi_pwm_set_property(struct rpi_firmware *firmware,
> + u32 reg, u32 val)
> +{
> + struct raspberrypi_pwm_prop msg = {
> + .reg = cpu_to_le32(reg),
> + .val = cpu_to_le32(val),
> + };
> + int ret;
> +
> + ret = rpi_firmware_property(firmware, RPI_FIRMWARE_SET_POE_HAT_VAL,
> + , sizeof(msg));
> + if (ret)
> + return ret;
> + if (msg.ret)
> + return -EIO;
> +
> + return 0;
> +}
> +
> +stati
Re: [PATCH] hwrng: bcm2835: set quality to 1000
On Wed, 2021-03-03 at 10:29 +0100, Álvaro Fernández Rojas wrote: > Hi Herbert, > > > El 3 mar 2021, a las 10:20, Herbert Xu > > escribió: > > > > On Sat, Feb 20, 2021 at 08:12:45PM +0100, Álvaro Fernández Rojas wrote: > > > > > > I ran rngtest and this is what I got: > > > > This is meaningless except for sources that have not been whitened. > > > > Your justification needs to be based on what the hardware does or > > is documented to do. > > Ok, so I guess that we’re never setting that value to anything since there’s > no public documentation about that ¯\_(ツ)_/¯. @Florian, is there a way you might be able to get the official value? Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH stable v5.10 0/7] arm64: Default to 32-bit wide ZONE_DMA
On Thu, 2021-03-04 at 15:17 +0100, Greg KH wrote: > On Thu, Mar 04, 2021 at 03:05:32PM +0100, Nicolas Saenz Julienne wrote: > > Hi Greg. > > > > On Thu, 2021-03-04 at 14:46 +0100, Greg KH wrote: > > > On Wed, Mar 03, 2021 at 03:33:12PM +0800, Jing Xiangfeng wrote: > > > > Using two distinct DMA zones turned out to be problematic. Here's an > > > > attempt go back to a saner default. > > > > > > What problem does this solve? How does this fit into the stable kernel > > > rules? > > > > We changed the way we setup memory zones in arm64 in order to cater for > > Raspberry Pi 4's weird DMA constraints: ZONE_DMA spans the lower 1GB of > > memory > > and ZONE_DMA32 the rest of the 32bit address space. Since you can't allocate > > memory that crosses zone boundaries, this broke crashkernel allocations on > > big > > machines. This series fixes all this by parsing the HW description and > > checking > > for DMA constrained buses. When not found, the unnecessary zone creation is > > skipped. > > What kernel/commit caused this "breakage"? 1a8e1cef7603 arm64: use both ZONE_DMA and ZONE_DMA32 Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH stable v5.10 0/7] arm64: Default to 32-bit wide ZONE_DMA
Hi Greg. On Thu, 2021-03-04 at 14:46 +0100, Greg KH wrote: > On Wed, Mar 03, 2021 at 03:33:12PM +0800, Jing Xiangfeng wrote: > > Using two distinct DMA zones turned out to be problematic. Here's an > > attempt go back to a saner default. > > What problem does this solve? How does this fit into the stable kernel > rules? We changed the way we setup memory zones in arm64 in order to cater for Raspberry Pi 4's weird DMA constraints: ZONE_DMA spans the lower 1GB of memory and ZONE_DMA32 the rest of the 32bit address space. Since you can't allocate memory that crosses zone boundaries, this broke crashkernel allocations on big machines. This series fixes all this by parsing the HW description and checking for DMA constrained buses. When not found, the unnecessary zone creation is skipped. That said, I have no clue whether this falls or not into the stable kernel rules. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
On Thu, 2021-03-04 at 13:18 +0100, Álvaro Fernández Rojas wrote:
>
> > El 4 mar 2021, a las 13:07, Nicolas Saenz Julienne
> > escribió:
> >
> > Hi Alvaro,
> >
> > On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote:
> > > Some devices may need to perform a reset before using the RNG, such as the
> > > BCM6368.
> > >
> > > Signed-off-by: Álvaro Fernández Rojas
> > > ---
> > > v3: make resets required if brcm,bcm6368-rng.
> > > v2: document reset support.
> > >
> > > .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > index c147900f9041..11c23e1f6988 100644
> > > --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > @@ -37,6 +37,21 @@ required:
> > >
> > >
> > >
> > > additionalProperties: false
> >
> > I can't claim I fully understand all the meta stuff in shemas, so I
> > generally
> > just follow the patterns already available out there.
>
> Well, that makes two of us :).
>
> > That said, shoudln't this be at the end, just before the examples?
>
> I don’t know but I can move it there ¯\_(ツ)_/¯
Yes please do. See commit 7f464532b05 ("dt-bindings: Add missing
'additionalProperties: false'") which expands on why it is needed, and why it
should be at the end.
> > Maybe the cause of that odd warning
> > you got there?
>
> Which odd warning?
The one pointed out by Rob Herring's script, which I can reproduce too BTW. On
the other hand I can't really tell what's wrong right away.
> I don’t get any warnings when running (or at least warnings related to rig,
> because I get warnings related to other yamls):
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>
> >
> > > +if:
> > > + properties:
> > > +compatible:
> > > + enum:
> > > +- brcm,bcm6368-rng
> > > +then:
> > > + properties:
> > > +resets:
> > > + maxItems: 1
> > > + required:
> > > +- resets
> >
> > I belive you can't really make a property required when the bindings for
> > 'brcm,bcm6368-rng' were already defined. This will break the schema for
> > those
> > otherwise correct devicetrees.
>
> Why not?
> Wouldn’t just be required for brcm,bcm6368-rng?
Well, do all 'brcm,bcm6368-rng' users absolutely need the reset controller? If
so, the original binding is wrong, which should be mentioned and a 'Fixes:' tag
should be added to the commit message. Otherwise, the requirement is optional.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v5 1/2] dt-bindings: rng: bcm2835: document reset support
Sorry Alvaro I missed you sent a v5 already. See my comments in v3.
Regards,
Nicolas
On Thu, 2021-03-04 at 08:33 +0100, Álvaro Fernández Rojas wrote:
> Some devices may need to perform a reset before using the RNG, such as the
> BCM6368.
>
> Signed-off-by: Álvaro Fernández Rojas
> ---
> v5: no changes.
> v4: pass dt_binding_check.
> v3: make resets required if brcm,bcm6368-rng.
> v2: document reset support.
>
> .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> index c147900f9041..11c23e1f6988 100644
> --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> @@ -37,6 +37,21 @@ required:
>
>
> additionalProperties: false
>
>
> +if:
> + properties:
> +compatible:
> + enum:
> +- brcm,bcm6368-rng
> +then:
> + properties:
> +resets:
> + maxItems: 1
> + required:
> +- resets
> +else:
> + properties:
> +resets: false
> +
> examples:
> - |
> rng@7e104000 {
> @@ -58,4 +73,6 @@ examples:
>
>
> clocks = <_clk 18>;
> clock-names = "ipsec";
> +
> +resets = <_rst 4>;
> };
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
Hi Alvaro,
On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote:
> Some devices may need to perform a reset before using the RNG, such as the
> BCM6368.
>
> Signed-off-by: Álvaro Fernández Rojas
> ---
> v3: make resets required if brcm,bcm6368-rng.
> v2: document reset support.
>
> .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> index c147900f9041..11c23e1f6988 100644
> --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> @@ -37,6 +37,21 @@ required:
>
>
> additionalProperties: false
I can't claim I fully understand all the meta stuff in shemas, so I generally
just follow the patterns already available out there. That said, shoudln't this
be at the end, just before the examples? Maybe the cause of that odd warning
you got there?
> +if:
> + properties:
> +compatible:
> + enum:
> +- brcm,bcm6368-rng
> +then:
> + properties:
> +resets:
> + maxItems: 1
> + required:
> +- resets
I belive you can't really make a property required when the bindings for
'brcm,bcm6368-rng' were already defined. This will break the schema for those
otherwise correct devicetrees.
> +else:
> + properties:
> +resets: false
> +
> examples:
> - |
> rng@7e104000 {
> @@ -58,4 +73,6 @@ examples:
>
>
> clocks = <_clk 18>;
> clock-names = "ipsec";
> +
> +resets = <_rst 4>;
> };
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [RFC 02/13] driver core: Introduce MMIO configuration
Hi Robin, On Tue, 2021-03-02 at 11:29 +, Robin Murphy wrote: > On 2021-02-26 14:02, Nicolas Saenz Julienne wrote: > > Some devices might inadvertently sit on buses that don't support 64bit > > MMIO access, and need a mechanism to query these limitations without > > prejudice to other buses in the system (i.e. defaulting to 32bit access > > system wide isn't an option). > > > > Introduce a new bus callback, 'mmio_configure(),' which will take care > > of populating the relevant device properties based on the bus' > > limitations. > > Devil's advocate: there already exist workarounds for 8-bit and/or > 16-bit accesses not working in various places, does it make sense for a > 64-bit workaround to be so wildly different and disjoint? Can you point out an example of the workarounds? > > Signed-off-by: Nicolas Saenz Julienne > > --- > > arch/Kconfig | 8 > > drivers/base/dd.c | 6 ++ > > include/linux/device.h | 3 +++ > > include/linux/device/bus.h | 3 +++ > > 4 files changed, 20 insertions(+) > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 2bb30673d8e6..ba7f246b6b9d 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -1191,6 +1191,14 @@ config ARCH_SPLIT_ARG64 > > config ARCH_HAS_ELFCORE_COMPAT > > bool > > > > > > +config ARCH_HAS_64BIT_MMIO_BROKEN > > + bool > > + depends on 64BIT > > As mentioned previously, 32-bit systems may not need the overrides for > kernel I/O accessors, but they could still need the same workarounds for > the memory-mapping implications (if this is to be a proper generic > mechanism). I'll keep it in mind. > > + default n > > Tip: it is always redundant to state that. Noted! Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC 10/13] iommu/arm-smmu-impl: Get rid of Marvell's implementation details
On Tue, 2021-03-02 at 11:40 +, Robin Murphy wrote: > On 2021-02-26 14:03, Nicolas Saenz Julienne wrote: > > arm-smmu can now deal with integrations on buses that don't support > > 64bit MMIO accesses. No need to create a special case for that on > > Marvell's integration. > > This breaks compatibility with existing DTs. Yes. On top of that, I had a brief word with robh on the topic of DT properties. I'm going to explore alternatives that don't depend on it. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC 09/13] iommu/arm-smmu: Make use of dev_64bit_mmio_supported()
Hi Robin, thanks for taking the time to look at this.
On Tue, 2021-03-02 at 11:07 +, Robin Murphy wrote:
> On 2021-02-26 14:03, Nicolas Saenz Julienne wrote:
> > Some arm SMMU implementations might sit on a bus that doesn't support
> > 64bit memory accesses. In that case default to using hi_lo_{readq,
> > writeq}() and BUG if such platform tries to use AArch64 formats as they
> > rely on writeq()'s atomicity.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/iommu/arm/arm-smmu/arm-smmu.c | 9 +
> > drivers/iommu/arm/arm-smmu/arm-smmu.h | 9 +++--
> > 2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index d8c6bfde6a61..239ff42b20c3 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -1889,6 +1889,15 @@ static int arm_smmu_device_cfg_probe(struct
> > arm_smmu_device *smmu)
> > smmu->features |= ARM_SMMU_FEAT_FMT_AARCH64_64K;
> > }
> >
> >
> > + /*
> > +* 64bit accesses not possible through the interconnect, AArch64
> > +* formats depend on it.
> > +*/
> > + BUG_ON(!dev_64bit_mmio_supported(smmu->dev) &&
> > + smmu->features & (ARM_SMMU_FEAT_FMT_AARCH64_4K |
> > + ARM_SMMU_FEAT_FMT_AARCH64_16K |
> > + ARM_SMMU_FEAT_FMT_AARCH64_64K));
>
> No. Crashing the kernel in a probe routine which is free to fail is
> unacceptable either way, but guaranteeing failure in the case that the
> workaround *would* be required is doubly so.
>
> Basically, this logic is backwards - if you really wanted to handle it
> generically, this would be the point at which you'd need to actively
> suppress all the detected hardware features which depend on 64-bit
> atomicity, not complain about them.
Understood.
> > +
> > if (smmu->impl && smmu->impl->cfg_probe) {
> > ret = smmu->impl->cfg_probe(smmu);
> > if (ret)
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > index d2a2d1bc58ba..997d13a21717 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> > @@ -477,15 +477,20 @@ static inline void arm_smmu_writel(struct
> > arm_smmu_device *smmu, int page,
> > {
> > if (smmu->impl && unlikely(smmu->impl->write_reg))
> > smmu->impl->write_reg(smmu, page, offset, val);
> > - else
> > + else if (dev_64bit_mmio_supported(smmu->dev))
> > writel_relaxed(val, arm_smmu_page(smmu, page) + offset);
> > + else
> > + hi_lo_writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
>
> As Arnd pointed out, this is in completely the wrong place. Also, in
Yes, sorry for that, not too proud of it.
> general it doesn't work if the implementation already needs a hook to
> filter or override register accesses for any other reason. TBH I'm not
I'm not sure I get your point here, 'smmu->impl' has precedence over the MMIO
capability check. Custom implementations would still get their callbacks.
> convinced that this isn't *more* of a mess than handling it on a
> SoC-specific basis...
I see your point.
Just to explain why I went to these lengths: my understanding is that the
specifics of how to perform 32bit accesses to SMMU's 64bit registers is defined
in spec. So it made sense to move it into the non implementation dependent side
of the driver.
All in all, I'll think of something simpler.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [RFC 09/13] iommu/arm-smmu: Make use of dev_64bit_mmio_supported()
Hi Arnd, thanks for the reviews!
On Tue, 2021-03-02 at 10:32 +0100, Arnd Bergmann wrote:
> On Fri, Feb 26, 2021 at 3:03 PM Nicolas Saenz Julienne
> wrote:
>
> > if (smmu->impl && unlikely(smmu->impl->write_reg))
> > smmu->impl->write_reg(smmu, page, offset, val);
> > - else
> > + else if (dev_64bit_mmio_supported(smmu->dev))
> > writel_relaxed(val, arm_smmu_page(smmu, page) + offset);
> > + else
> > + hi_lo_writeq_relaxed(val, arm_smmu_page(smmu, page) +
> > offset);
> > }
>
> This is a writel_relaxed(), not a writeq_relaxed(), so I suppose you don't
> have to change it at all.
Yes, that was silly of me. I was worrying about the semantics of the whole
thing, and missed basic stuff like this.
> > + else if (dev_64bit_mmio_supported(smmu->dev))
> > + return readq_relaxed(arm_smmu_page(smmu, page) + offset);
> > + else
> > + return hi_lo_readq_relaxed(arm_smmu_page(smmu, page) +
> > offset);
> > }
>
>
> I see this pattern repeat across multiple drivers. I think Christoph
> had originally
> suggested folding the if/else logic into the writel_relaxed() that is defined
> in
> include/linux/io-64-nonatomic-hi-lo.h, but of course that doesn't work if you
> need to pass a device pointer.
>
> It might still make sense to have another wrapper in that same file though,
> something like
>
> static inline hi_lo_writeq_relaxed_if_possible(struct device *dev, __u64 val,
> volatile void __iomem *addr)
> {
> if (dev_64bit_mmio_supported(smmu->dev)) {
> readq_relaxed(arm_smmu_page(smmu, page) + offset);
> } else {
> writel_relaxed(val >> 32, addr + 4);
> writel_relaxed(val, addr);
> }
> }
I like the idea. I'll try to integrate it into the next revision.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[RFC 11/13] arm64: Mark ARCH_BCM2835 as needing broken 64bit MMIO support
The PCIe bus present in BCM2711 can't handle 64bit accesses on device memory. So select 'ARCH_HAS_64BIT_MMIO_BROKEN.' Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/Kconfig.platforms | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index 04a97cf486c5..0178a46cc10a 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -38,6 +38,7 @@ config ARCH_ALPINE config ARCH_BCM2835 bool "Broadcom BCM2835 family" + select ARCH_HAS_64BIT_MMIO_BROKEN select TIMER_OF select GPIOLIB select MFD_CORE -- 2.30.1
[RFC 13/13] scsi: megaraid: Make use of dev_64bit_mmio_supported()
Instead of relying on defines use dev_64bit_mmio_supported(), which
provides the same functionality. On top of that convert the
implementation to lo_hi_writeq(), for a cleaner end result.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/scsi/megaraid/megaraid_sas_fusion.c | 23 ++---
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 38fc9467c625..d4933a591330 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
#include
@@ -259,19 +260,17 @@ static void
megasas_write_64bit_req_desc(struct megasas_instance *instance,
union MEGASAS_REQUEST_DESCRIPTOR_UNION *req_desc)
{
-#if defined(writeq) && defined(CONFIG_64BIT)
- u64 req_data = (((u64)le32_to_cpu(req_desc->u.high) << 32) |
- le32_to_cpu(req_desc->u.low));
- writeq(req_data, >reg_set->inbound_low_queue_port);
-#else
+ u64 req_data = ((u64)le32_to_cpu(req_desc->u.high) << 32) |
+ le32_to_cpu(req_desc->u.low);
unsigned long flags;
- spin_lock_irqsave(>hba_lock, flags);
- writel(le32_to_cpu(req_desc->u.low),
- >reg_set->inbound_low_queue_port);
- writel(le32_to_cpu(req_desc->u.high),
- >reg_set->inbound_high_queue_port);
- spin_unlock_irqrestore(>hba_lock, flags);
-#endif
+
+ if (dev_64bit_mmio_supported(>pdev->dev)) {
+ writeq(req_data, >reg_set->inbound_low_queue_port);
+ } else {
+ spin_lock_irqsave(>hba_lock, flags);
+ lo_hi_writeq(req_data,
>reg_set->inbound_low_queue_port);
+ spin_unlock_irqrestore(>hba_lock, flags);
+ }
}
/**
--
2.30.1
[RFC 07/13] arm64: Mark ARCH_MVEBU as needing broken 64bit MMIO support
The bus AP806's IOMMU sits on can't handle 64bit MMIO accesses[1]. So select 'ARCH_HAS_64BIT_MMIO_BROKEN' for the platform. Signed-off-by: Nicolas Saenz Julienne [1] See Armada-AP806 erratum #582743 --- arch/arm64/Kconfig.platforms | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index cdfd5fed457f..04a97cf486c5 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -175,6 +175,7 @@ config ARCH_MESON config ARCH_MVEBU bool "Marvell EBU SoC Family" + select ARCH_HAS_64BIT_MMIO_BROKEN select ARMADA_AP806_SYSCON select ARMADA_CP110_SYSCON select ARMADA_37XX_CLK -- 2.30.1
[RFC 12/13] ARM: dts: bcm2711: Mark PCIe bus as 64bit-mmio-broken
The bus implementation can't handle 64bit MMIO accesses.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 462b1dfb0385..825abdbc0d76 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -529,6 +529,7 @@ pcie0: pcie@7d50 {
dma-ranges = <0x0200 0x0 0x 0x0 0x
0x0 0xc000>;
brcm,enable-ssc;
+ 64bit-mmio-broken;
};
genet: ethernet@7d58 {
--
2.30.1
[RFC 10/13] iommu/arm-smmu-impl: Get rid of Marvell's implementation details
arm-smmu can now deal with integrations on buses that don't support
64bit MMIO accesses. No need to create a special case for that on
Marvell's integration.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 21 -
1 file changed, 21 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index 136872e77195..55d40e37e144 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -145,25 +145,6 @@ static const struct arm_smmu_impl arm_mmu500_impl = {
.reset = arm_mmu500_reset,
};
-static u64 mrvl_mmu500_readq(struct arm_smmu_device *smmu, int page, int off)
-{
- /*
-* Marvell Armada-AP806 erratum #582743.
-* Split all the readq to double readl
-*/
- return hi_lo_readq_relaxed(arm_smmu_page(smmu, page) + off);
-}
-
-static void mrvl_mmu500_writeq(struct arm_smmu_device *smmu, int page, int off,
- u64 val)
-{
- /*
-* Marvell Armada-AP806 erratum #582743.
-* Split all the writeq to double writel
-*/
- hi_lo_writeq_relaxed(val, arm_smmu_page(smmu, page) + off);
-}
-
static int mrvl_mmu500_cfg_probe(struct arm_smmu_device *smmu)
{
@@ -181,8 +162,6 @@ static int mrvl_mmu500_cfg_probe(struct arm_smmu_device
*smmu)
}
static const struct arm_smmu_impl mrvl_mmu500_impl = {
- .read_reg64 = mrvl_mmu500_readq,
- .write_reg64 = mrvl_mmu500_writeq,
.cfg_probe = mrvl_mmu500_cfg_probe,
.reset = arm_mmu500_reset,
};
--
2.30.1
[RFC 09/13] iommu/arm-smmu: Make use of dev_64bit_mmio_supported()
Some arm SMMU implementations might sit on a bus that doesn't support
64bit memory accesses. In that case default to using hi_lo_{readq,
writeq}() and BUG if such platform tries to use AArch64 formats as they
rely on writeq()'s atomicity.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 9 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 9 +++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index d8c6bfde6a61..239ff42b20c3 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1889,6 +1889,15 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
smmu->features |= ARM_SMMU_FEAT_FMT_AARCH64_64K;
}
+ /*
+* 64bit accesses not possible through the interconnect, AArch64
+* formats depend on it.
+*/
+ BUG_ON(!dev_64bit_mmio_supported(smmu->dev) &&
+ smmu->features & (ARM_SMMU_FEAT_FMT_AARCH64_4K |
+ ARM_SMMU_FEAT_FMT_AARCH64_16K |
+ ARM_SMMU_FEAT_FMT_AARCH64_64K));
+
if (smmu->impl && smmu->impl->cfg_probe) {
ret = smmu->impl->cfg_probe(smmu);
if (ret)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index d2a2d1bc58ba..997d13a21717 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -477,15 +477,20 @@ static inline void arm_smmu_writel(struct arm_smmu_device
*smmu, int page,
{
if (smmu->impl && unlikely(smmu->impl->write_reg))
smmu->impl->write_reg(smmu, page, offset, val);
- else
+ else if (dev_64bit_mmio_supported(smmu->dev))
writel_relaxed(val, arm_smmu_page(smmu, page) + offset);
+ else
+ hi_lo_writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
}
static inline u64 arm_smmu_readq(struct arm_smmu_device *smmu, int page, int
offset)
{
if (smmu->impl && unlikely(smmu->impl->read_reg64))
return smmu->impl->read_reg64(smmu, page, offset);
- return readq_relaxed(arm_smmu_page(smmu, page) + offset);
+ else if (dev_64bit_mmio_supported(smmu->dev))
+ return readq_relaxed(arm_smmu_page(smmu, page) + offset);
+ else
+ return hi_lo_readq_relaxed(arm_smmu_page(smmu, page) + offset);
}
static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
--
2.30.1
[RFC 05/13] pci: Introduce pci_mmio_configure()
The function will traverse the pci device's bus hierarchy and set
the relevant MMIO access flags.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/pci/pci-driver.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index ec44a79e951a..554d91e7ec52 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1596,6 +1596,31 @@ static int pci_dma_configure(struct device *dev)
return ret;
}
+/**
+ * pci_mmio_configure - Setup MMIO configuration
+ * @dev: ptr to dev structure
+ *
+ * Function to update PCI devices's MMIO configuration using the same
+ * info from the OF node of host bridge's parent (if any).
+ */
+static int pci_mmio_configure(struct device *dev)
+{
+ struct device *bridge;
+ int ret = 0;
+
+ bridge = pci_get_host_bridge_device(to_pci_dev(dev));
+
+ dev_info(dev, "MMIO CONFIGURATION\n");
+ if (IS_ENABLED(CONFIG_OF) && bridge->parent &&
+ bridge->parent->of_node) {
+ dev_info(dev, "MMIO CONFIGURATION, %pOF\n",
bridge->parent->of_node);
+ ret = of_mmio_configure(dev, bridge->parent->of_node);
+ }
+
+ pci_put_host_bridge_device(bridge);
+ return ret;
+}
+
struct bus_type pci_bus_type = {
.name = "pci",
.match = pci_bus_match,
@@ -1609,6 +1634,7 @@ struct bus_type pci_bus_type = {
.pm = PCI_PM_OPS_PTR,
.num_vf = pci_bus_num_vf,
.dma_configure = pci_dma_configure,
+ .mmio_configure = pci_mmio_configure,
};
EXPORT_SYMBOL(pci_bus_type);
--
2.30.1
[RFC 06/13] device core: Introduce dev_64bit_mmio_supported()
This helper function will be help drivers ascertain whether they can use
64-bit memory accesses.
Signed-off-by: Nicolas Saenz Julienne
---
include/linux/device.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index bd94aa0cbd72..e9b4b2f99a44 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -826,6 +826,23 @@ static inline int dev_num_vf(struct device *dev)
return 0;
}
+#if defined(CONFIG_ARCH_HAS_64BIT_MMIO_BROKEN)
+static inline bool dev_64bit_mmio_supported(struct device *dev)
+{
+ return !dev->mmio_64bit_broken;
+}
+#elif defined(CONFIG_64BIT)
+static inline bool dev_64bit_mmio_supported(struct device *dev)
+{
+ return true;
+}
+#else
+static inline bool dev_64bit_mmio_supported(struct device *dev)
+{
+ return false;
+}
+#endif
+
/*
* Root device objects for grouping under /sys/devices
*/
--
2.30.1
[RFC 08/13] arm64: dts: marvell: armada-ap80x: Mark config-space bus as 64bit-mmio-broken
As per Marvell's Armada-AP806 erratum #582743 the bus can't handle 64bit
MMIO accesses.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
index 6614472100c2..a009458edf24 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
@@ -54,6 +54,7 @@ config-space@f000 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "simple-bus";
+ 64bit-mmio-broken;
ranges = <0x0 0x0 0xf000 0x100>;
smmu: iommu@500 {
--
2.30.1
[RFC 04/13] driver core: plafrom: Introduce platform_mmio_configure()
The function will traverse the platform device's bus hierarchy and set
the relevant MMIO access flags.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/base/platform.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 6e1f8e0b661c..31772fd4ca1d 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1504,6 +1504,14 @@ int platform_dma_configure(struct device *dev)
return ret;
}
+static int platform_mmio_configure(struct device *dev)
+{
+ if (dev->parent && dev->parent->of_node)
+ return of_mmio_configure(dev, dev->parent->of_node);
+
+ return 0;
+}
+
static const struct dev_pm_ops platform_dev_pm_ops = {
.runtime_suspend = pm_generic_runtime_suspend,
.runtime_resume = pm_generic_runtime_resume,
@@ -1519,6 +1527,7 @@ struct bus_type platform_bus_type = {
.remove = platform_remove,
.shutdown = platform_shutdown,
.dma_configure = platform_dma_configure,
+ .mmio_configure = platform_mmio_configure,
.pm = _dev_pm_ops,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
--
2.30.1
[RFC 03/13] of: device: Introduce of_mmio_configure()
The function will traverse a device's bus hierarchy looking for MMIO
limited buses. If found it'll populate the relevant struct device
quirks.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/of/device.c | 19 +++
include/linux/of_device.h | 8
2 files changed, 27 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 6cb86de404f1..b80367a2764b 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -169,6 +169,25 @@ int of_dma_configure_id(struct device *dev, struct
device_node *np,
}
EXPORT_SYMBOL_GPL(of_dma_configure_id);
+int of_mmio_configure(struct device *dev, struct device_node *np)
+{
+#if defined(CONFIG_ARCH_HAS_64BIT_MMIO_BROKEN)
+ struct device_node *node = of_node_get(np);
+
+ do {
+ if (of_property_read_bool(node, "64bit-mmio-broken")) {
+ dev->mmio_64bit_broken = true;
+ dev_dbg(dev, "device behind 64bit mmio broken bus\n");
+ break;
+ }
+ } while ((node = of_get_next_parent(node)));
+
+ of_node_put(node);
+#endif
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_mmio_configure);
+
int of_device_register(struct platform_device *pdev)
{
device_initialize(>dev);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 1d7992a02e36..c465edd509c7 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -56,6 +56,9 @@ static inline int of_dma_configure(struct device *dev,
{
return of_dma_configure_id(dev, np, force_dma, NULL);
}
+
+int of_mmio_configure(struct device *dev, struct device_node *np);
+
#else /* CONFIG_OF */
static inline int of_driver_match_device(struct device *dev,
@@ -112,6 +115,11 @@ static inline int of_dma_configure(struct device *dev,
{
return 0;
}
+
+static inline int of_mmio_configure(struct device *dev, struct device_node
*np);
+{
+ return 0;
+}
#endif /* CONFIG_OF */
#endif /* _LINUX_OF_DEVICE_H */
--
2.30.1
[RFC 00/13] Generic way of dealing with broken 64-bit buses
BCM2711, Raspberry Pi 4's arm64 system on chip, contains a PCIe bus that can't handle 64-bit accesses to its MMIO address space. The issue has already been discussed here[1], and it turns out BCM2711 isn't the only broken device in the wild. In most cases, the solution to this issue is to convert writeq/readq() to into their lo_hi/hi_lo variants and the eventual introduction of some amount of locking. But that's not good enough for every device. For example, on some arm's SMMU configurations atomic 64-bit accesses are mandatory. This series tries to introduce a mechanism for drivers to be able to ascertain whether or not they are allowed to perform 64-bit accesses. The big question is the amount of granularity needed to deal with this (think here of distro images): - Build-time: if a broken platform included in the image, disallow any 64-bit access. Drivers that need 64-bit accesses could simply bypass the check and hope for the best. Imposes a performance penalty on otherwise well behaving platforms, and features that depend on 64bit access might be disabled unnecessarily. It's simple to implement, yet not very generic/future proof. - Run-time: allow/disallow 64-bit accesses based on boot time checks (i.e. check which platform the kernel is running on). Gets rid of all the negative aspects imposed to well-behaving platforms. Well-behaving buses can't coexist with broken ones while using all features. - Per-device: each device has its MMIO access properties and can take decisions based on its local bus. That said, I'm not aware of a system that absolutely needs this ATM. This series implements the third option mainly as a proof of concept. It's my personal preference on how to deal with this. That said, my main aim ATM is to settle on a general approach. Regards, Nicolas [1] https://lore.kernel.org/linux-arm-kernel/c188698ca0de3ed6c56a0cf7880e1578aa753077.ca...@suse.de/ --- Nicolas Saenz Julienne (13): dt-bindings: Introduce 64bit-mmio-broken driver core: Introduce MMIO configuration of: device: Introduce of_mmio_configure() driver core: plafrom: Introduce platform_mmio_configure() pci: Introduce pci_mmio_configure() device core: Introduce dev_64bit_mmio_supported() arm64: Mark ARCH_MVEBU as needing broken 64bit MMIO support arm64: dts: marvell: armada-ap80x: Mark config-space bus as 64bit-mmio-broken iommu/arm-smmu: Make use of dev_64bit_mmio_supported() iommu/arm-smmu-impl: Get rid of Marvell's implementation details arm64: Mark ARCH_BCM2835 as needing broken 64bit MMIO support ARM: dts: bcm2711: Mark PCIe bus as 64bit-mmio-broken scsi: megaraid: Make use of dev_64bit_mmio_supported() .../devicetree/bindings/common-properties.txt | 15 +++ arch/Kconfig | 8 ++ arch/arm/boot/dts/bcm2711.dtsi| 1 + arch/arm64/Kconfig.platforms | 2 ++ arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 1 + drivers/base/dd.c | 6 + drivers/base/platform.c | 9 +++ drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 21 --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 9 +++ drivers/iommu/arm/arm-smmu/arm-smmu.h | 9 +-- drivers/of/device.c | 19 ++ drivers/pci/pci-driver.c | 26 +++ drivers/scsi/megaraid/megaraid_sas_fusion.c | 23 include/linux/device.h| 20 ++ include/linux/device/bus.h| 3 +++ include/linux/of_device.h | 8 ++ 16 files changed, 145 insertions(+), 35 deletions(-) -- 2.30.1
[RFC 02/13] driver core: Introduce MMIO configuration
Some devices might inadvertently sit on buses that don't support 64bit
MMIO access, and need a mechanism to query these limitations without
prejudice to other buses in the system (i.e. defaulting to 32bit access
system wide isn't an option).
Introduce a new bus callback, 'mmio_configure(),' which will take care
of populating the relevant device properties based on the bus'
limitations.
Signed-off-by: Nicolas Saenz Julienne
---
arch/Kconfig | 8
drivers/base/dd.c | 6 ++
include/linux/device.h | 3 +++
include/linux/device/bus.h | 3 +++
4 files changed, 20 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index 2bb30673d8e6..ba7f246b6b9d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1191,6 +1191,14 @@ config ARCH_SPLIT_ARG64
config ARCH_HAS_ELFCORE_COMPAT
bool
+config ARCH_HAS_64BIT_MMIO_BROKEN
+ bool
+ depends on 64BIT
+ default n
+ help
+ Arch might contain busses unable to perform 64bit mmio accessses on
+ an otherwise 64bit system.
+
source "kernel/gcov/Kconfig"
source "scripts/gcc-plugins/Kconfig"
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 9179825ff646..8086ce8f17a6 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -538,6 +538,12 @@ static int really_probe(struct device *dev, struct
device_driver *drv)
goto probe_failed;
}
+ if (dev->bus->mmio_configure) {
+ ret = dev->bus->mmio_configure(dev);
+ if (ret)
+ goto probe_failed;
+ }
+
if (driver_sysfs_add(dev)) {
pr_err("%s: driver_sysfs_add(%s) failed\n",
__func__, dev_name(dev));
diff --git a/include/linux/device.h b/include/linux/device.h
index ba660731bd25..bd94aa0cbd72 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -553,6 +553,9 @@ struct device {
#ifdef CONFIG_DMA_OPS_BYPASS
booldma_ops_bypass : 1;
#endif
+#if defined(CONFIG_ARCH_HAS_64BIT_MMIO_BROKEN)
+ boolmmio_64bit_broken:1;
+#endif
};
/**
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index 1ea5e1d1545b..680dfc3b4744 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -59,6 +59,8 @@ struct fwnode_handle;
* bus supports.
* @dma_configure: Called to setup DMA configuration on a device on
* this bus.
+ * @mmio_configure:Called to setup MMIO configuration on a device on
+ * this bus.
* @pm:Power management operations of this bus, callback the
specific
* device driver's pm-ops.
* @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU
@@ -103,6 +105,7 @@ struct bus_type {
int (*num_vf)(struct device *dev);
int (*dma_configure)(struct device *dev);
+ int (*mmio_configure)(struct device *dev);
const struct dev_pm_ops *pm;
--
2.30.1
[RFC 01/13] dt-bindings: Introduce 64bit-mmio-broken
Some buses might not be able to handle 64-bit sized MMIO accesses, on
otherwise 64-bit systems. Introduce a boolean property to cater for this
limitation.
Signed-off-by: Nicolas Saenz Julienne
---
.../devicetree/bindings/common-properties.txt | 15 +++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/common-properties.txt
b/Documentation/devicetree/bindings/common-properties.txt
index 98a28130e100..3783510102c3 100644
--- a/Documentation/devicetree/bindings/common-properties.txt
+++ b/Documentation/devicetree/bindings/common-properties.txt
@@ -83,3 +83,18 @@ gpio@0 {
#gpio-cells = <2>;
#daisy-chained-devices = <3>;
};
+
+Broken 64-bit buses
+---
+
+Some buses might not be able to handle 64-bit sized MMIO accesses, on otherwise
+64-bit systems. This property is only relevant to MMIO bus nodes.
+
+Optional properties:
+ - 64bit-mmio-broken: Boolean
+
+Example:
+pcie@0 {
+ compatible = "name";
+ 64bit-mmio-broken;
+};
--
2.30.1
[RFC 2/2] mmc: sdhci-iproc: Set clock frequency as per DT
devicetree might request a clock frequency different from whatever is
set-up by the bootloader. Make sure to setup the new rate.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/mmc/host/sdhci-iproc.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index ddeaf8e1f72f..536c382e2486 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -358,6 +358,16 @@ static int sdhci_iproc_probe(struct platform_device *pdev)
ret = PTR_ERR(pltfm_host->clk);
goto err;
}
+
+ if (pltfm_host->clock) {
+ ret = clk_set_rate(pltfm_host->clk, pltfm_host->clock);
+ if (ret) {
+ dev_err(dev, "failed to set host clk at %u
Hz\n",
+ pltfm_host->clock);
+ goto err;
+ }
+ }
+
ret = clk_prepare_enable(pltfm_host->clk);
if (ret) {
dev_err(dev, "failed to enable host clk\n");
--
2.30.1
[RFC 1/2] ARM: dts: Fix-up EMMC2 controller's frequency
Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
seems to interfere with the VPU clock when setup at frequencies bigger
than 500MHz, causing unwarranted SDHCI CMD hangs when no SD card is
present.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 3b4ab947492a..9aa8408d9960 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -257,6 +257,12 @@ {
vqmmc-supply = <_io_1v8_reg>;
vmmc-supply = <_vcc_reg>;
broken-cd;
+ /*
+* Force the frequency to 150MHz as the default 100MHz seems to
+* interfere with the VPU clock when setup at frequencies bigger than
+* 500MHz, causing unwarranted CMD hangs.
+*/
+ clock-frequency = <15000>;
status = "okay";
};
--
2.30.1
[RFC 0/2] sdhci-iproc CMD timeouts
I'm seeing a rather odd behavior from sdhci-iproc's integration in BCM2711 (Raspberry Pi 4's SoC), and would appreciate some opinions. The controller will timeout on SDHCI CMDs under the following conditions: - No SD card plugged in (the card polling thread is running, CD irq disabled). - BCM2711's VPU clock[1] configured at 500MHz or more, lower clocks are OK. There is no specific command that will time out, it seems random. Here's what I found out. The SDHCI controller runs at 100MHz, by bumping the frequency to 150MHz the issue disapears. Might be pure luck, or maybe I hit the nail and it's proper interference. Can't say. Regards, Nicolas --- [1] For those who are not aware, here's the relationship between VPU's clock and emmc2's: osc5400 plld 300091 plld_per75023 emmc2149882908 pllc 299988 pllc_core0 6 vpu 49998 Nicolas Saenz Julienne (2): ARM: dts: Fix-up EMMC2 controller's frequency mmc: sdhci-iproc: Set clock frequency as per DT arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++ drivers/mmc/host/sdhci-iproc.c| 10 ++ 2 files changed, 16 insertions(+) -- 2.30.1
Re: [PATCH -next] nvmem: Fix return value check in rmem_read()
On Tue, 2021-02-23 at 14:22 +, Wei Yongjun wrote:
> In case of error, the function memremap() returns NULL pointer
> not ERR_PTR(). The IS_ERR() test in the return value check
> should be replaced with NULL test.
>
> Fixes: 5a3fa75a4d9c ("nvmem: Add driver to expose reserved memory as nvmem")
> Reported-by: Hulk Robot
> Signed-off-by: Wei Yongjun
> ---
Acked-by: Nicolas Saenz Julienne
Thanks!
signature.asc
Description: This is a digitally signed message part
[PATCH v3 15/15] arm64: config: Enable DRM_V3D
BCM2711, the SoC used on the Raspberry Pi 4 has a different GPU than its predecessors. Enable it. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index d612f633b771..a13e631b7c50 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -699,6 +699,7 @@ CONFIG_DRM_I2C_ADV7511_AUDIO=y CONFIG_DRM_DW_HDMI_AHB_AUDIO=m CONFIG_DRM_DW_HDMI_CEC=m CONFIG_DRM_IMX_DCSS=m +CONFIG_DRM_V3D=m CONFIG_DRM_VC4=m CONFIG_DRM_ETNAVIV=m CONFIG_DRM_HISI_HIBMC=m -- 2.30.0
[PATCH v3 14/15] ARM: configs: Enable DRM_V3D
BCM2711, the SoC used on the Raspberry Pi 4 has a different GPU than its predecessors. Enable it. Signed-off-by: Nicolas Saenz Julienne --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 3823da605430..0ad4ffe4e6f6 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -713,6 +713,7 @@ CONFIG_DRM_TOSHIBA_TC358764=m CONFIG_DRM_I2C_ADV7511=m CONFIG_DRM_I2C_ADV7511_AUDIO=y CONFIG_DRM_STI=m +CONFIG_DRM_V3D=m CONFIG_DRM_VC4=m CONFIG_DRM_ETNAVIV=m CONFIG_DRM_MXSFB=m -- 2.30.0
[PATCH v3 13/15] ARM: dts: bcm2711: Enable V3D
This enables V3D for bcm2711 (used in the Raspberry Pi 4).
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v1:
- Correct node's name address
arch/arm/boot/dts/bcm2711.dtsi | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index e864974b10c9..d2bbfc7c4daa 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -549,6 +549,18 @@ genet_mdio: mdio@e14 {
#size-cells = <0x1>;
};
};
+
+ v3d: gpu@7ec0 {
+ compatible = "brcm,bcm2711-v3d";
+ reg = <0x0 0x7ec0 0x4000>,
+ <0x0 0x7ec04000 0x4000>;
+ reg-names = "hub", "core0";
+
+ power-domains = < BCM2835_POWER_DOMAIN_GRAFX_V3D>;
+ resets = < BCM2835_RESET_V3D>;
+ clocks = <_clocks 5>;
+ interrupts = ;
+ };
};
};
--
2.30.0
[PATCH v3 12/15] drm/v3d: Add support for bcm2711
Add compatible string and Kconfig options for bcm2711.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/gpu/drm/v3d/Kconfig | 2 +-
drivers/gpu/drm/v3d/v3d_drv.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/Kconfig b/drivers/gpu/drm/v3d/Kconfig
index 9a5c44606337..b0e048697964 100644
--- a/drivers/gpu/drm/v3d/Kconfig
+++ b/drivers/gpu/drm/v3d/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config DRM_V3D
tristate "Broadcom V3D 3.x and newer"
- depends on ARCH_BCM || ARCH_BCMSTB || COMPILE_TEST
+ depends on ARCH_BCM || ARCH_BCMSTB || ARCH_BCM2835 || COMPILE_TEST
depends on DRM
depends on COMMON_CLK
depends on MMU
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 7a3336443a12..1504b6f84441 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -184,6 +184,7 @@ static const struct drm_driver v3d_drm_driver = {
static const struct of_device_id v3d_of_match[] = {
{ .compatible = "brcm,7268-v3d" },
{ .compatible = "brcm,7278-v3d" },
+ { .compatible = "brcm,bcm2711-v3d" },
{},
};
MODULE_DEVICE_TABLE(of, v3d_of_match);
--
2.30.0
[PATCH v3 11/15] drm/v3d: Get rid of pm code
Runtime PM doesn't seem to work correctly on this driver. On top of
that, commit 8b6864e3e138 ("drm/v3d/v3d_drv: Remove unused static
variable 'v3d_v3d_pm_ops'") hints that it most likely never did as the
driver's PM ops were not hooked-up.
So, in order to support regular operation with V3D on BCM2711 (Raspberry
Pi 4), get rid of the PM code. PM will be reinstated once we figure out
the underlying issues.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 18 +-
drivers/gpu/drm/v3d/v3d_drv.c | 11 ---
drivers/gpu/drm/v3d/v3d_gem.c | 9 -
3 files changed, 1 insertion(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index e76b24bb8828..e1d5f3423059 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -4,7 +4,6 @@
#include
#include
#include
-#include
#include
#include
@@ -130,11 +129,7 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void
*unused)
struct drm_device *dev = node->minor->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
u32 ident0, ident1, ident2, ident3, cores;
- int ret, core;
-
- ret = pm_runtime_get_sync(v3d->drm.dev);
- if (ret < 0)
- return ret;
+ int core;
ident0 = V3D_READ(V3D_HUB_IDENT0);
ident1 = V3D_READ(V3D_HUB_IDENT1);
@@ -187,9 +182,6 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void
*unused)
(misccfg & V3D_MISCCFG_OVRTMUOUT) != 0);
}
- pm_runtime_mark_last_busy(v3d->drm.dev);
- pm_runtime_put_autosuspend(v3d->drm.dev);
-
return 0;
}
@@ -217,11 +209,6 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
uint32_t cycles;
int core = 0;
int measure_ms = 1000;
- int ret;
-
- ret = pm_runtime_get_sync(v3d->drm.dev);
- if (ret < 0)
- return ret;
if (v3d->ver >= 40) {
V3D_CORE_WRITE(core, V3D_V4_PCTR_0_SRC_0_3,
@@ -245,9 +232,6 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
cycles / (measure_ms * 1000),
(cycles / (measure_ms * 100)) % 10);
- pm_runtime_mark_last_busy(v3d->drm.dev);
- pm_runtime_put_autosuspend(v3d->drm.dev);
-
return 0;
}
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 99e22beea90b..7a3336443a12 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -19,7 +19,6 @@
#include
#include
#include
-#include
#include
#include
@@ -43,7 +42,6 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void
*data,
{
struct v3d_dev *v3d = to_v3d_dev(dev);
struct drm_v3d_get_param *args = data;
- int ret;
static const u32 reg_map[] = {
[DRM_V3D_PARAM_V3D_UIFCFG] = V3D_HUB_UIFCFG,
[DRM_V3D_PARAM_V3D_HUB_IDENT1] = V3D_HUB_IDENT1,
@@ -69,17 +67,12 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void
*data,
if (args->value != 0)
return -EINVAL;
- ret = pm_runtime_get_sync(v3d->drm.dev);
- if (ret < 0)
- return ret;
if (args->param >= DRM_V3D_PARAM_V3D_CORE0_IDENT0 &&
args->param <= DRM_V3D_PARAM_V3D_CORE0_IDENT2) {
args->value = V3D_CORE_READ(0, offset);
} else {
args->value = V3D_READ(offset);
}
- pm_runtime_mark_last_busy(v3d->drm.dev);
- pm_runtime_put_autosuspend(v3d->drm.dev);
return 0;
}
@@ -271,10 +264,6 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
return -ENOMEM;
}
- pm_runtime_use_autosuspend(dev);
- pm_runtime_set_autosuspend_delay(dev, 50);
- pm_runtime_enable(dev);
-
ret = v3d_gem_init(drm);
if (ret)
goto dma_free;
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 4eb354226972..75582b03470b 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -6,7 +6,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -372,9 +371,6 @@ v3d_job_free(struct kref *ref)
dma_fence_put(job->irq_fence);
dma_fence_put(job->done_fence);
- pm_runtime_mark_last_busy(job->v3d->drm.dev);
- pm_runtime_put_autosuspend(job->v3d->drm.dev);
-
kfree(job);
}
@@ -441,10 +437,6 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file
*file_priv,
job->v3d = v3d;
job->free = free;
- ret = pm_runtime_get_sync(v3d->drm.dev);
- if (ret < 0)
-
[PATCH v3 06/15] mfd: bcm2835-pm: Use 'reg-names' to get resources
If available in firmware, find resources by their 'reg-names' position
instead of relying on hardcoded offsets. Care is taken to support old
firmware nonetheless.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v2:
- correct names
drivers/mfd/bcm2835-pm.c | 55 ++--
1 file changed, 36 insertions(+), 19 deletions(-)
diff --git a/drivers/mfd/bcm2835-pm.c b/drivers/mfd/bcm2835-pm.c
index 42fe67f1538e..36fede92775c 100644
--- a/drivers/mfd/bcm2835-pm.c
+++ b/drivers/mfd/bcm2835-pm.c
@@ -25,9 +25,37 @@ static const struct mfd_cell bcm2835_power_devs[] = {
{ .name = "bcm2835-power" },
};
+static int bcm2835_pm_get_pdata(struct platform_device *pdev,
+ struct bcm2835_pm *pm)
+{
+ /* If no 'reg-names' property is found we can assume we're using old
+* firmware.
+*/
+ if (!of_find_property(pm->dev->of_node, "reg-names", NULL)) {
+ dev_warn(pm->dev, "Old devicetree detected, please update your
firmware.\n");
+
+ pm->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(pm->base))
+ return PTR_ERR(pm->base);
+
+ pm->asb = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(pm->asb))
+ pm->asb = NULL;
+ } else {
+ pm->base = devm_platform_ioremap_resource_byname(pdev, "pm");
+ if (IS_ERR(pm->base))
+ return PTR_ERR(pm->base);
+
+ pm->asb = devm_platform_ioremap_resource_byname(pdev, "asb");
+ if (IS_ERR(pm->base))
+ pm->asb = NULL;
+ }
+
+ return 0;
+}
+
static int bcm2835_pm_probe(struct platform_device *pdev)
{
- struct resource *res;
struct device *dev = >dev;
struct bcm2835_pm *pm;
int ret;
@@ -39,10 +67,9 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
pm->dev = dev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- pm->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(pm->base))
- return PTR_ERR(pm->base);
+ ret = bcm2835_pm_get_pdata(pdev, pm);
+ if (ret)
+ return ret;
ret = devm_mfd_add_devices(dev, -1,
bcm2835_pm_devs, ARRAY_SIZE(bcm2835_pm_devs),
@@ -54,20 +81,10 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
* bcm2835-pm binding as the key for whether we can reference
* the full PM register range and support power domains.
*/
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (res) {
- pm->asb = devm_ioremap_resource(dev, res);
- if (IS_ERR(pm->asb))
- return PTR_ERR(pm->asb);
-
- ret = devm_mfd_add_devices(dev, -1,
- bcm2835_power_devs,
- ARRAY_SIZE(bcm2835_power_devs),
- NULL, 0, NULL);
- if (ret)
- return ret;
- }
-
+ if (pm->asb)
+ return devm_mfd_add_devices(dev, -1, bcm2835_power_devs,
+ ARRAY_SIZE(bcm2835_power_devs),
+ NULL, 0, NULL);
return 0;
}
--
2.30.0
[PATCH v3 10/15] dt-bindings: gpu: v3d: Add BCM2711's compatible
BCM2711, Raspberry Pi 4's SoC, contains a V3D core. So add its specific compatible to the bindings. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml index 9d72264fa90a..8401385bb33c 100644 --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml @@ -18,6 +18,7 @@ properties: enum: - brcm,7268-v3d - brcm,7278-v3d + - brcm,bcm2711-v3d reg: items: -- 2.30.0
[PATCH v3 05/15] ARM: dts: bcm2711: Use proper compatible in PM/Watchdog node
A new compatible string was introduced specifically for BCM2711, so make
use of it.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index e4bbc1a83452..e864974b10c9 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -107,7 +107,7 @@ dma: dma@7e007000 {
};
pm: watchdog@7e10 {
- compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
+ compatible = "brcm,bcm2711-pm", "brcm,bcm2835-pm-wdt";
#power-domain-cells = <1>;
#reset-cells = <1>;
reg = <0x7e10 0x114>,
--
2.30.0
[PATCH v3 07/15] mfd: bcm2835-pm: Add support for BCM2711
In BCM2711 the new RPiVid ASB took over V3D. The old ASB is still present
with the ISP and H264 bits, and V3D is in the same place in the new ASB
as the old one.
As per the devicetree bindings, BCM2711 will provide both the old and
new ASB resources, so get both of them and pass them into
'bcm2835-power,' which will take care of selecting which one to use
accordingly.
Since the RPiVid ASB's resources were being provided prior to formalizing
the bindings[1], also support the old firmwares that didn't use
'reg-names.'
Signed-off-by: Nicolas Saenz Julienne
[1] See: 7dbe8c62ceeb ("ARM: dts: Add minimal Raspberry Pi 4 support")
---
Changes since v2:
- Correct names again!
Changes since v1:
- Use reg-names
- Correct ASB names
drivers/mfd/bcm2835-pm.c | 25 +++--
include/linux/mfd/bcm2835-pm.h | 1 +
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/bcm2835-pm.c b/drivers/mfd/bcm2835-pm.c
index 36fede92775c..a06e9cf19b64 100644
--- a/drivers/mfd/bcm2835-pm.c
+++ b/drivers/mfd/bcm2835-pm.c
@@ -6,6 +6,7 @@
* the WDT and power drivers.
*/
+#include
#include
#include
#include
@@ -17,6 +18,9 @@
#include
#include
+#define BCM2835BIT(1)
+#define BCM2711BIT(2)
+
static const struct mfd_cell bcm2835_pm_devs[] = {
{ .name = "bcm2835-wdt" },
};
@@ -28,6 +32,8 @@ static const struct mfd_cell bcm2835_power_devs[] = {
static int bcm2835_pm_get_pdata(struct platform_device *pdev,
struct bcm2835_pm *pm)
{
+ bool is_bcm2711 = (uintptr_t)device_get_match_data(pm->dev) & BCM2711;
+
/* If no 'reg-names' property is found we can assume we're using old
* firmware.
*/
@@ -41,6 +47,10 @@ static int bcm2835_pm_get_pdata(struct platform_device *pdev,
pm->asb = devm_platform_ioremap_resource(pdev, 1);
if (IS_ERR(pm->asb))
pm->asb = NULL;
+
+ pm->rpivid_asb = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(pm->rpivid_asb))
+ pm->rpivid_asb = NULL;
} else {
pm->base = devm_platform_ioremap_resource_byname(pdev, "pm");
if (IS_ERR(pm->base))
@@ -49,6 +59,16 @@ static int bcm2835_pm_get_pdata(struct platform_device *pdev,
pm->asb = devm_platform_ioremap_resource_byname(pdev, "asb");
if (IS_ERR(pm->base))
pm->asb = NULL;
+
+ pm->rpivid_asb = devm_platform_ioremap_resource_byname(pdev,
+
"rpivid_asb");
+ if (IS_ERR(pm->base))
+ pm->rpivid_asb = NULL;
+
+ if (pm->rpivid_asb && !is_bcm2711) {
+ dev_err(pm->dev, "RPiVid ASB support only present in
BCM2711\n");
+ return -EINVAL;
+ }
}
return 0;
@@ -89,8 +109,9 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
}
static const struct of_device_id bcm2835_pm_of_match[] = {
- { .compatible = "brcm,bcm2835-pm-wdt", },
- { .compatible = "brcm,bcm2835-pm", },
+ { .compatible = "brcm,bcm2835-pm-wdt", .data = (void *)BCM2835},
+ { .compatible = "brcm,bcm2835-pm", .data = (void *)BCM2835},
+ { .compatible = "brcm,bcm2711-pm", .data = (void *)BCM2711},
{},
};
MODULE_DEVICE_TABLE(of, bcm2835_pm_of_match);
diff --git a/include/linux/mfd/bcm2835-pm.h b/include/linux/mfd/bcm2835-pm.h
index ed37dc40e82a..f70a810c55f7 100644
--- a/include/linux/mfd/bcm2835-pm.h
+++ b/include/linux/mfd/bcm2835-pm.h
@@ -9,6 +9,7 @@ struct bcm2835_pm {
struct device *dev;
void __iomem *base;
void __iomem *asb;
+ void __iomem *rpivid_asb;
};
#endif /* BCM2835_MFD_PM_H */
--
2.30.0
[PATCH v3 09/15] soc: bcm: bcm2835-power: Bypass power_on/off() calls
Bypass power_on/power_off() when running on BCM2711 as they are not
needed.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v2:
- Correct names
drivers/soc/bcm/bcm2835-power.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
index eea31f75dc64..b65a39cc2788 100644
--- a/drivers/soc/bcm/bcm2835-power.c
+++ b/drivers/soc/bcm/bcm2835-power.c
@@ -197,6 +197,10 @@ static int bcm2835_power_power_off(struct
bcm2835_power_domain *pd, u32 pm_reg)
{
struct bcm2835_power *power = pd->power;
+ /* We don't run this on BCM2711 */
+ if (power->rpivid_asb)
+ return 0;
+
/* Enable functional isolation */
PM_WRITE(pm_reg, PM_READ(pm_reg) & ~PM_ISFUNC);
@@ -218,6 +222,10 @@ static int bcm2835_power_power_on(struct
bcm2835_power_domain *pd, u32 pm_reg)
int inrush;
bool powok;
+ /* We don't run this on BCM2711 */
+ if (power->rpivid_asb)
+ return 0;
+
/* If it was already powered on by the fw, leave it that way. */
if (PM_READ(pm_reg) & PM_POWUP)
return 0;
--
2.30.0
[PATCH v3 08/15] soc: bcm: bcm2835-power: Add support for BCM2711's RPiVid ASB
In BCM2711 the new RPiVid ASB took over V3D. The old ASB is still present
with the ISP and H264 bits, and V3D is in the same place in the new ASB
as the old one.
Use the fact that 'pm->rpivid_asb' is populated as a hint that we're on
BCM2711. On top of that introduce the macro ASB_BASE() which will select
the correct ASB register base, based on whether we're trying to access
V3D and which platform we're on.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v2:
- Correct names again
Changes since v1:
- Correct names
drivers/soc/bcm/bcm2835-power.c | 66 -
1 file changed, 41 insertions(+), 25 deletions(-)
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
index 1e0041ec8132..eea31f75dc64 100644
--- a/drivers/soc/bcm/bcm2835-power.c
+++ b/drivers/soc/bcm/bcm2835-power.c
@@ -126,8 +126,9 @@
#define ASB_AXI_BRDG_ID0x20
-#define ASB_READ(reg) readl(power->asb + (reg))
-#define ASB_WRITE(reg, val) writel(PM_PASSWORD | (val), power->asb + (reg))
+#define ASB_BASE(is_v3d) (is_v3d && power->rpivid_asb ? power->rpivid_asb :
power->asb)
+#define ASB_READ(reg, is_v3d) readl(ASB_BASE(is_v3d) + (reg))
+#define ASB_WRITE(reg, val, is_v3d) writel(PM_PASSWORD | (val),
ASB_BASE(is_v3d) + (reg))
struct bcm2835_power_domain {
struct generic_pm_domain base;
@@ -142,13 +143,16 @@ struct bcm2835_power {
void __iomem*base;
/* AXI Async bridge registers. */
void __iomem*asb;
+ /* RPiVid bridge registers. */
+ void __iomem*rpivid_asb;
struct genpd_onecell_data pd_xlate;
struct bcm2835_power_domain domains[BCM2835_POWER_DOMAIN_COUNT];
struct reset_controller_dev reset;
};
-static int bcm2835_asb_enable(struct bcm2835_power *power, u32 reg)
+static int bcm2835_asb_enable(struct bcm2835_power *power, u32 reg,
+ bool is_v3d)
{
u64 start;
@@ -158,8 +162,8 @@ static int bcm2835_asb_enable(struct bcm2835_power *power,
u32 reg)
start = ktime_get_ns();
/* Enable the module's async AXI bridges. */
- ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP);
- while (ASB_READ(reg) & ASB_ACK) {
+ ASB_WRITE(reg, ASB_READ(reg, is_v3d) & ~ASB_REQ_STOP, is_v3d);
+ while (ASB_READ(reg, is_v3d) & ASB_ACK) {
cpu_relax();
if (ktime_get_ns() - start >= 1000)
return -ETIMEDOUT;
@@ -168,7 +172,8 @@ static int bcm2835_asb_enable(struct bcm2835_power *power,
u32 reg)
return 0;
}
-static int bcm2835_asb_disable(struct bcm2835_power *power, u32 reg)
+static int bcm2835_asb_disable(struct bcm2835_power *power, u32 reg,
+ bool is_v3d)
{
u64 start;
@@ -178,8 +183,8 @@ static int bcm2835_asb_disable(struct bcm2835_power *power,
u32 reg)
start = ktime_get_ns();
/* Enable the module's async AXI bridges. */
- ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP);
- while (!(ASB_READ(reg) & ASB_ACK)) {
+ ASB_WRITE(reg, ASB_READ(reg, is_v3d) | ASB_REQ_STOP, is_v3d);
+ while (!(ASB_READ(reg, is_v3d) & ASB_ACK)) {
cpu_relax();
if (ktime_get_ns() - start >= 1000)
return -ETIMEDOUT;
@@ -274,7 +279,8 @@ static int bcm2835_asb_power_on(struct bcm2835_power_domain
*pd,
u32 pm_reg,
u32 asb_m_reg,
u32 asb_s_reg,
- u32 reset_flags)
+ u32 reset_flags,
+ bool is_v3d)
{
struct bcm2835_power *power = pd->power;
int ret;
@@ -301,13 +307,13 @@ static int bcm2835_asb_power_on(struct
bcm2835_power_domain *pd,
goto err_enable_resets;
}
- ret = bcm2835_asb_enable(power, asb_m_reg);
+ ret = bcm2835_asb_enable(power, asb_m_reg, is_v3d);
if (ret) {
dev_err(power->dev, "Failed to enable ASB master for %s\n",
pd->base.name);
goto err_disable_clk;
}
- ret = bcm2835_asb_enable(power, asb_s_reg);
+ ret = bcm2835_asb_enable(power, asb_s_reg, is_v3d);
if (ret) {
dev_err(power->dev, "Failed to enable ASB slave for %s\n",
pd->base.name);
@@ -317,7 +323,7 @@ static int bcm2835_asb_power_on(struct bcm2835_power_domain
*pd,
return 0;
err_disable_asb_master:
- bcm2835_asb_disable(power, asb_m_reg);
+ bcm2835_asb_disable(power, asb_m_reg, is_v3d);
err_disable_clk:
clk_disable_unprepare(pd->clk);
err_enable_resets:
@@ -329,22 +335,23 @@ static int bcm2835_asb_power_off(struct
bcm2835_power_domain *p
[PATCH v3 04/15] ARM: dts: bcm2835/bcm2711: Introduce reg-names in watchdog node
bcm2835-pm's bindings now support explicitly setting 'reg-names,' so use
them.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711.dtsi| 1 +
arch/arm/boot/dts/bcm2835-common.dtsi | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 462b1dfb0385..e4bbc1a83452 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -113,6 +113,7 @@ pm: watchdog@7e10 {
reg = <0x7e10 0x114>,
<0x7e00a000 0x24>,
<0x7ec11000 0x20>;
+ reg-names = "pm", "asb", "rpivid_asb";
clocks = < BCM2835_CLOCK_V3D>,
< BCM2835_CLOCK_PERI_IMAGE>,
< BCM2835_CLOCK_H264>,
diff --git a/arch/arm/boot/dts/bcm2835-common.dtsi
b/arch/arm/boot/dts/bcm2835-common.dtsi
index 4119271c979d..db0ed88edec2 100644
--- a/arch/arm/boot/dts/bcm2835-common.dtsi
+++ b/arch/arm/boot/dts/bcm2835-common.dtsi
@@ -62,6 +62,7 @@ pm: watchdog@7e10 {
#reset-cells = <1>;
reg = <0x7e10 0x114>,
<0x7e00a000 0x24>;
+ reg-names = "pm", "asb";
clocks = < BCM2835_CLOCK_V3D>,
< BCM2835_CLOCK_PERI_IMAGE>,
< BCM2835_CLOCK_H264>,
--
2.30.0
[PATCH v3 03/15] dt-bindings: soc: bcm: bcm2835-pm: Add support for bcm2711
Add a new compatible string for BCM2711 and the option to provide a third reg property for the board's new RPiVid ASB. In BCM2711 the new RPiVid ASB took over V3D, which is our only consumer of this driver so far. The old ASB is still be present with ISP and H264 bits but no V3D. Signed-off-by: Nicolas Saenz Julienne Reviewed-by: Rob Herring --- Changes since v2: - Correct names s/argon/rpivid/ .../devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml index 837963e9f219..0dd264f80fb3 100644 --- a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml +++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml @@ -19,19 +19,22 @@ allOf: properties: compatible: items: - - const: brcm,bcm2835-pm + - enum: + - brcm,bcm2835-pm + - brcm,bcm2711-pm - const: brcm,bcm2835-pm-wdt reg: minItems: 1 -maxItems: 2 +maxItems: 3 reg-names: minItems: 1 -maxItems: 2 +maxItems: 3 items: - const: pm - const: asb + - const: rpivid_asb "#power-domain-cells": const: 1 @@ -74,7 +77,7 @@ examples: #reset-cells = <1>; reg = <0x7e10 0x114>, <0x7e00a000 0x24>; -reg-names = "pm", "asb"; +reg-names = "pm", "asb", "rpivid_asb"; clocks = < BCM2835_CLOCK_V3D>, < BCM2835_CLOCK_PERI_IMAGE>, < BCM2835_CLOCK_H264>, -- 2.30.0
[PATCH v3 02/15] dt-bindings: soc: bcm: bcm2835-pm: Introduce reg-names
Anticipating the introduction of BCM2711, of which we'll need to support its new RPiVid ASB, introduce reg-names into bcm2835-pm's binding. This will help to have a consistent mapping between resources and their meaning. Signed-off-by: Nicolas Saenz Julienne --- .../devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml index 47b9c266ddd1..837963e9f219 100644 --- a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml +++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml @@ -25,9 +25,13 @@ properties: reg: minItems: 1 maxItems: 2 + + reg-names: +minItems: 1 +maxItems: 2 items: - - description: PM registers - - description: ASB registers + - const: pm + - const: asb "#power-domain-cells": const: 1 @@ -70,6 +74,7 @@ examples: #reset-cells = <1>; reg = <0x7e10 0x114>, <0x7e00a000 0x24>; +reg-names = "pm", "asb"; clocks = < BCM2835_CLOCK_V3D>, < BCM2835_CLOCK_PERI_IMAGE>, < BCM2835_CLOCK_H264>, -- 2.30.0
[PATCH v3 00/15] Raspberry PI 4 V3D enablement
This series attempts to enable V3D on BCM2711, the SoC available on the
Raspberry Pi 4 family of boards.
Due to the lack of documentation some things are taken as it from
testing/downstream implementation[1], which I'm hilighting here:
- It's not clear that the following is 100% true, maybe someone can confirm:
"In BCM2711 the new ARGON ASB took over V3D. The old ASB is still
present with the ISP and H264 bits, and V3D is in the same place in the
new ASB as the old one."
- Patch #9 ("soc: bcm: bcm2835-power: Bypass power_on/off() calls"), I
took as is from the downstream implementation, I can't really provide
an exact explanation on what changed HW wise.
- Ultimately, I need confirmation from the Broadcom folks that they are alright
with patch #11 ("drm/v3d: Get rid of pm code")
With all this, I get a more or less stable experience using mesa 20.3.4 and
X11/Gnome.
Regards,
Nicolas
---
Changes since v2:
- Correct ASB names
- Address dt-binding comments
Changes since v1:
- Use 'reg-names'
- Correct ASB names
- Add missing binding patch for V3D
- Address Stefan's comments
Nicolas Saenz Julienne (15):
dt-bindings: soc: bcm: bcm2835-pm: Convert bindings to DT schema
dt-bindings: soc: bcm: bcm2835-pm: Introduce reg-names
dt-bindings: soc: bcm: bcm2835-pm: Add support for bcm2711
ARM: dts: bcm2835/bcm2711: Introduce reg-names in watchdog node
ARM: dts: bcm2711: Use proper compatible in PM/Watchdog node
mfd: bcm2835-pm: Use 'reg-names' to get resources
mfd: bcm2835-pm: Add support for BCM2711
soc: bcm: bcm2835-power: Add support for BCM2711's RPiVid ASB
soc: bcm: bcm2835-power: Bypass power_on/off() calls
dt-bindings: gpu: v3d: Add BCM2711's compatible
drm/v3d: Get rid of pm code
drm/v3d: Add support for bcm2711
ARM: dts: bcm2711: Enable V3D
ARM: configs: Enable DRM_V3D
arm64: config: Enable DRM_V3D
.../devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 1 +
.../bindings/soc/bcm/brcm,bcm2835-pm.txt | 46 --
.../bindings/soc/bcm/brcm,bcm2835-pm.yaml | 87 +++
arch/arm/boot/dts/bcm2711.dtsi| 15 +++-
arch/arm/boot/dts/bcm2835-common.dtsi | 1 +
arch/arm/configs/multi_v7_defconfig | 1 +
arch/arm64/configs/defconfig | 1 +
drivers/gpu/drm/v3d/Kconfig | 2 +-
drivers/gpu/drm/v3d/v3d_debugfs.c | 18 +---
drivers/gpu/drm/v3d/v3d_drv.c | 12 +--
drivers/gpu/drm/v3d/v3d_gem.c | 9 --
drivers/mfd/bcm2835-pm.c | 80 -
drivers/soc/bcm/bcm2835-power.c | 74 ++--
include/linux/mfd/bcm2835-pm.h| 1 +
14 files changed, 217 insertions(+), 131 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
create mode 100644
Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml
--
2.30.0
[PATCH v3 01/15] dt-bindings: soc: bcm: bcm2835-pm: Convert bindings to DT schema
This converts the brcm,bcm2835-pm bindings from text to proper schema.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v2:
- Slightly change things to make it less dependent on next commits
.../bindings/soc/bcm/brcm,bcm2835-pm.txt | 46 ---
.../bindings/soc/bcm/brcm,bcm2835-pm.yaml | 79 +++
2 files changed, 79 insertions(+), 46 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
create mode 100644
Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml
diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
deleted file mode 100644
index 72ff033565e5..
--- a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.txt
+++ /dev/null
@@ -1,46 +0,0 @@
-BCM2835 PM (Power domains, watchdog)
-
-The PM block controls power domains and some reset lines, and includes
-a watchdog timer. This binding supersedes the brcm,bcm2835-pm-wdt
-binding which covered some of PM's register range and functionality.
-
-Required properties:
-
-- compatible: Should be "brcm,bcm2835-pm"
-- reg: Specifies base physical address and size of the two
- register ranges ("PM" and "ASYNC_BRIDGE" in that
- order)
-- clocks: a) v3d: The V3D clock from CPRMAN
- b) peri_image: The PERI_IMAGE clock from CPRMAN
- c) h264: The H264 clock from CPRMAN
- d) isp: The ISP clock from CPRMAN
-- #reset-cells:Should be 1. This property follows the reset controller
- bindings[1].
-- #power-domain-cells: Should be 1. This property follows the power domain
- bindings[2].
-
-Optional properties:
-
-- timeout-sec: Contains the watchdog timeout in seconds
-- system-power-controller: Whether the watchdog is controlling the
-system power. This node follows the power controller bindings[3].
-
-[1] Documentation/devicetree/bindings/reset/reset.txt
-[2] Documentation/devicetree/bindings/power/power-domain.yaml
-[3] Documentation/devicetree/bindings/power/power-controller.txt
-
-Example:
-
-pm {
- compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
- #power-domain-cells = <1>;
- #reset-cells = <1>;
- reg = <0x7e10 0x114>,
- <0x7e00a000 0x24>;
- clocks = < BCM2835_CLOCK_V3D>,
-< BCM2835_CLOCK_PERI_IMAGE>,
-< BCM2835_CLOCK_H264>,
-< BCM2835_CLOCK_ISP>;
- clock-names = "v3d", "peri_image", "h264", "isp";
- system-power-controller;
-};
diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml
b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml
new file mode 100644
index ..47b9c266ddd1
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-pm.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/bcm/brcm,bcm2835-pm.yaml#;
+$schema: "http://devicetree.org/meta-schemas/core.yaml#;
+
+title: BCM2835 PM (Power domains, watchdog)
+
+description: |
+ The PM block controls power domains and some reset lines, and includes a
+ watchdog timer.
+
+maintainers:
+ - Nicolas Saenz Julienne
+
+allOf:
+ - $ref: ../../watchdog/watchdog.yaml#
+
+properties:
+ compatible:
+items:
+ - const: brcm,bcm2835-pm
+ - const: brcm,bcm2835-pm-wdt
+
+ reg:
+minItems: 1
+maxItems: 2
+items:
+ - description: PM registers
+ - description: ASB registers
+
+ "#power-domain-cells":
+const: 1
+
+ "#reset-cells":
+const: 1
+
+ clocks:
+minItems: 4
+maxItems: 4
+
+ clock-names:
+items:
+ - const: v3d
+ - const: peri_image
+ - const: h264
+ - const: isp
+
+ system-power-controller:
+type: boolean
+
+ timeout-sec: true
+
+required:
+ - compatible
+ - reg
+ - "#power-domain-cells"
+ - "#reset-cells"
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+watchdog@7e10 {
+compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
+#power-domain-cells = <1>;
+#reset-cells = <1>;
+reg = <0x7e10 0x114>,
+ <0x7e00a000 0x24>;
+clocks = < BCM2835_CLOCK_V3D>,
+< BCM2835_CLOCK_PERI_IMAGE>,
+< BCM2835_CLOCK_H264>,
+< BCM2835_CLOCK_ISP>;
+clock-names = "v3d", "peri_image", "h264", "isp";
+system-power-controller;
+};
--
2.30.0
Re: [PATCH] Revert "ARM: dts: bcm2711: Add the BSC interrupt controller"
On Fri, 2021-02-12 at 11:11 -0800, Florian Fainelli wrote: > As Dave reported: > > This seems to have unintended side effects. GIC interrupt 117 is shared > between the standard I2C controllers (i2c-bcm2835) and the l2-intc block > handling the HDMI I2C interrupts. > > There is not a great way to share an interrupt between an interrupt > controller using the chained IRQ handler which is an interrupt flow and > another driver like i2c-bcm2835 which uses an interrupt handler > (although it specifies IRQF_SHARED). > > Simply revert this change for now which will mean that HDMI I2C will be > polled, like it was before. > > Reported-by: Dave Stevenson > Signed-off-by: Florian Fainelli Acked-by: Nicolas Saenz Julienne Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH] spi: Skip zero-length transfers in spi_transfer_one_message()
On Fri, 2021-02-12 at 12:31 +, Mark Brown wrote:
> On Thu, Feb 11, 2021 at 07:08:20PM +0100, Nicolas Saenz Julienne wrote:
>
> > - if (xfer->tx_buf || xfer->rx_buf) {
> > + if ((xfer->tx_buf || xfer->rx_buf) && xfer->len) {
>
> I think the issue here is more that some users were passing in buffers
> with zero length transfers, the above check was already intended to
> catch this case but was working on the assumption that if there was
> nothing to transfer then no buffer would be provided.
Fair enough, maybe it makes sense to move the check into __spi_validate() and
propagate an error upwards?
Regads,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller
On Wed, 2021-02-10 at 10:49 -0800, Florian Fainelli wrote: > On 2/10/21 7:49 AM, Dave Stevenson wrote: > > Hi Marc. > > > > On Wed, 10 Feb 2021 at 15:30, Marc Zyngier wrote: > > > > > > Hi Maxime, > > > > > > On 2021-02-10 14:40, Maxime Ripard wrote: > > > > Hi Dave, > > > > > > > > On Tue, Feb 09, 2021 at 09:49:05AM +, Dave Stevenson wrote: > > > > > On Mon, 11 Jan 2021 at 14:23, Maxime Ripard wrote: > > > > > > > > > > > > The BSC controllers used for the HDMI DDC have an interrupt > > > > > > controller > > > > > > shared between both instances. Let's add it to avoid polling. > > > > > > > > > > This seems to have unintended side effects. > > > > > GIC interrupt 117 is shared between the standard I2C controllers > > > > > (i2c-bcm2835) and the l2-intc block handling the HDMI I2C interrupts. > > > > > > > > > > Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that > > > > > doesn't appear to be an option for l2-intc registering as an interrupt > > > > > controller. i2c-bcm2835 therefore loses out and fails to register for > > > > > the interrupt. > > > > > > > > > > Is there an equivalent flag that an interrupt controller can add to > > > > > say that the parent interrupt is shared? Is that even supported? > > > > > > > > Indeed, it looks like setting an equivalent to IRQF_SHARED would be the > > > > solution, but I couldn't find anything that would allow us to in the > > > > irqchip code. > > > > > > > > Marc, Thomas, is it something that is allowed? > > > > > > No, not really. That's because the chained handler is actually an > > > interrupt flow, and not a normal handler. IRQF_SHARED acts at the wrong > > > level for that. > > > > > > I can see two possibilities: > > > > > > - the l2-intc gets turned into a normal handler, and does the demux > > > from there. Horrible stuff. > > > > > > - the i2c controller gets parented to the l2c-int as a fake interrupt, > > > and gets called from there. Horrible stuff. > > > > > > Pick your poison... :-/ > > > > Thanks for the info. > > > > Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb > > blocks (which the driver supports anyway). > > HDMI I2C generally isn't heavily used once displays are connected, so > > I'd be OK with that. > > > > (We can keep the l2-intc that handles CEC and HPD as that is on a > > unique GIC interrupt). > > Agreed, Maxime or Nicolas do you want me to send a revert of this patch? Reverting seems the safe move, but I'll defer to whatever Maxime says. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH] spi: Skip zero-length transfers in spi_transfer_one_message()
With the introduction of 26751de25d25 ("spi: bcm2835: Micro-optimise
FIFO loops") it has become apparent that some users might initiate
zero-length SPI transfers. A fact the micro-optimization omitted, and
which turned out to cause crashes[1].
Instead of changing the micro-optimization itself, use a bigger hammer
and skip zero-length transfers altogether for drivers using the default
transfer_one_message() implementation.
Reported-by: Phil Elwell
Fixes: 26751de25d25 ("spi: bcm2835: Micro-optimise FIFO loops")
Signed-off-by: Nicolas Saenz Julienne
[1] https://github.com/raspberrypi/linux/issues/4100
---
NOTE: I've reviewed a bunch of drivers and couldn't find a compelling
reason why zero-length transfers should be passed into them. But I
might be missing something.
drivers/spi/spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 7745eec994fd..b08efe88ccd6 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1269,7 +1269,7 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
ptp_read_system_prets(xfer->ptp_sts);
}
- if (xfer->tx_buf || xfer->rx_buf) {
+ if ((xfer->tx_buf || xfer->rx_buf) && xfer->len) {
reinit_completion(>xfer_completion);
fallback_pio:
--
2.30.0
Re: bcm2711_thermal: Kernel panic - not syncing: Asynchronous SError Interrupt
Hi Robin,
On Wed, 2021-02-10 at 16:25 +, Robin Murphy wrote:
> On 2021-02-10 13:15, Nicolas Saenz Julienne wrote:
> > [ Add Robin, Catalin and Florian in case they want to chime in ]
> >
> > Hi Juerg, thanks for the report!
> >
> > On Wed, 2021-02-10 at 11:48 +0100, Juerg Haefliger wrote:
> > > Trying to dump the BCM2711 registers kills the kernel:
> > >
> > > # cat /sys/kernel/debug/regmap/dummy-avs-monitor\@fd5d2000/range
> > > 0-efc
> > > # cat /sys/kernel/debug/regmap/dummy-avs-monitor\@fd5d2000/registers
> > >
> > > [ 62.857661] SError Interrupt on CPU1, code 0xbf02 -- SError
> >
> > So ESR's IDS (bit 24) is set, which means it's an 'Implementation Defined
> > SError,' hence IIUC the rest of the error code is meaningless to anyone
> > outside
> > of Broadcom/RPi.
>
> It's imp-def from the architecture's PoV, but the implementation in this
> case is Cortex-A72, where 0x02 means an attributable, containable
> Slave Error:
>
> https://developer.arm.com/documentation/100095/0003/system-control/aarch64-register-descriptions/exception-syndrome-register--el1-and-el3?lang=en
>
> In other words, the thing at the other end of an interconnect
> transaction said "no" :)
>
> (The fact that Cortex-A72 gets too far ahead of itself to take it as a
> synchronous external abort is a mild annoyance, but hey...)
Thanks for both your clarifications! Reading arm documentation is a skill on
its own.
> > The regmap is created through the following syscon device:
> >
> > avs_monitor: avs-monitor@7d5d2000 {
> > compatible = "brcm,bcm2711-avs-monitor",
> > "syscon", "simple-mfd";
> > reg = <0x7d5d2000 0xf00>;
> >
> > thermal: thermal {
> > compatible = "brcm,bcm2711-thermal";
> > #thermal-sensor-cells = <0>;
> > };
> > };
> >
> > I've done some tests with devmem, and the whole <0x7d5d2000 0xf00> range is
> > full of addresses that trigger this same error. Also note that as per
> > Florian's
> > comments[1]: "AVS_RO_REGISTERS_0: 0x7d5d2200 - 0x7d5d22e3." But from what I
> > can
> > tell, at least 0x7d5d22b0 seems to be faulty too.
> >
> > Any ideas/comments? My guess is that those addresses are marked somehow as
> > secure, and only for VC4 to access (VC4 is RPi4's co-processor). Ultimately,
> > the solution is to narrow the register range exposed by avs-monitor to
> > whatever
> > bcm2711-thermal needs (which is ATM a single 32bit register).
>
> When a peripheral decodes a region of address space, nobody says it has
> to accept accesses to *every* address in that space; registers may be
> sparsely populated, and although some devices might be "nice" and make
> unused areas behave as RAZ/WI, others may throw slave errors if you poke
> at the wrong places. As you note, in a TrustZone-aware device some
> registers may only exist in one or other of the Secure/Non-Secure
> address spaces.
>
> Even when there is a defined register at a given address, it still
> doesn't necessarily accept all possible types of access; it wouldn't be
> particularly friendly, but a device *could* have, say, some registers
> that support 32-bit accesses and others that only support 16-bit
> accesses, and thus throw slave errors if you do the wrong thing in the
> wrong place.
>
> It really all depends on the device itself.
All in all, assuming there is no special device quirk to apply, the feeling I'm
getting is to just let the error be. As you hint, firmware has no blame here,
and debugfs is a 'best effort, zero guarantees' interface after all.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: bcm2711_thermal: Kernel panic - not syncing: Asynchronous SError Interrupt
[ Add Robin, Catalin and Florian in case they want to chime in ]
Hi Juerg, thanks for the report!
On Wed, 2021-02-10 at 11:48 +0100, Juerg Haefliger wrote:
> Trying to dump the BCM2711 registers kills the kernel:
>
> # cat /sys/kernel/debug/regmap/dummy-avs-monitor\@fd5d2000/range
> 0-efc
> # cat /sys/kernel/debug/regmap/dummy-avs-monitor\@fd5d2000/registers
>
> [ 62.857661] SError Interrupt on CPU1, code 0xbf02 -- SError
So ESR's IDS (bit 24) is set, which means it's an 'Implementation Defined
SError,' hence IIUC the rest of the error code is meaningless to anyone outside
of Broadcom/RPi.
The regmap is created through the following syscon device:
avs_monitor: avs-monitor@7d5d2000 {
compatible = "brcm,bcm2711-avs-monitor",
"syscon", "simple-mfd";
reg = <0x7d5d2000 0xf00>;
thermal: thermal {
compatible = "brcm,bcm2711-thermal";
#thermal-sensor-cells = <0>;
};
};
I've done some tests with devmem, and the whole <0x7d5d2000 0xf00> range is
full of addresses that trigger this same error. Also note that as per Florian's
comments[1]: "AVS_RO_REGISTERS_0: 0x7d5d2200 - 0x7d5d22e3." But from what I can
tell, at least 0x7d5d22b0 seems to be faulty too.
Any ideas/comments? My guess is that those addresses are marked somehow as
secure, and only for VC4 to access (VC4 is RPi4's co-processor). Ultimately,
the solution is to narrow the register range exposed by avs-monitor to whatever
bcm2711-thermal needs (which is ATM a single 32bit register).
Regards,
Nicolas
[1]
https://lore.kernel.org/linux-pm/82125042-684a-b4e2-fbaa-45a393b2c...@gmx.net/
> [ 62.857671] CPU: 1 PID: 478 Comm: cat Not tainted 5.11.0-rc7 #4
> [ 62.857674] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
> [ 62.857676] pstate: 2085 (nzCv daIf -PAN -UAO -TCO BTYPE=--)
> [ 62.857679] pc : regmap_mmio_read32le+0x1c/0x34
> [ 62.857681] lr : regmap_mmio_read+0x50/0x80
> [ 62.857682] sp : 8000105c3c00
> [ 62.857685] x29: 8000105c3c00 x28: 0014
> [ 62.857694] x27: 0014 x26: d2ea1c2060b0
> [ 62.857699] x25: 4e34408ecc00 x24: 0efc
> [ 62.857704] x23: 8000105c3e20 x22: 8000105c3d3c
> [ 62.857710] x21: 8000105c3d3c x20: 0014
> [ 62.857715] x19: 4e344037a900 x18: 0020
> [ 62.857720] x17: x16:
> [ 62.857725] x15: 4e3447ac40f0 x14: 0003
> [ 62.857730] x13: 4e34422c x12: 4e34422a0046
> [ 62.857735] x11: d2ea1c8765e0 x10:
> [ 62.857741] x9 : d2ea1b9495a0 x8 : 4e34429ef980
> [ 62.857746] x7 : 000f x6 : 4e34422a004b
> [ 62.857751] x5 : fff9 x4 :
> [ 62.857757] x3 : d2ea1b949550 x2 : d2ea1b949330
> [ 62.857761] x1 : 0014 x0 :
> [ 62.857767] Kernel panic - not syncing: Asynchronous SError Interrupt
> [ 62.857770] CPU: 1 PID: 478 Comm: cat Not tainted 5.11.0-rc7 #4
> [ 62.857773] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
> [ 62.857775] Call trace:
> [ 62.85] dump_backtrace+0x0/0x1e0
> [ 62.857778] show_stack+0x24/0x70
> [ 62.857780] dump_stack+0xd0/0x12c
> [ 62.857782] panic+0x168/0x370
> [ 62.857783] nmi_panic+0x98/0xa0
> [ 62.857786] arm64_serror_panic+0x8c/0x98
> [ 62.857787] do_serror+0x3c/0x6c
> [ 62.857789] el1_error+0x78/0xf0
> [ 62.857791] regmap_mmio_read32le+0x1c/0x34
> [ 62.857793] _regmap_bus_reg_read+0x24/0x30
> [ 62.857795] _regmap_read+0x6c/0x17c
> [ 62.857797] regmap_read+0x58/0x84
> [ 62.857799] regmap_read_debugfs+0x138/0x3f4
> [ 62.857801] regmap_map_read_file+0x34/0x40
> [ 62.857803] full_proxy_read+0x6c/0xc0
> [ 62.857805] vfs_read+0xb8/0x1e4
> [ 62.857807] ksys_read+0x78/0x10c
> [ 62.857809] __arm64_sys_read+0x28/0x34
> [ 62.857811] el0_svc_common.constprop.0+0x7c/0x194
> [ 62.857813] do_el0_svc+0x30/0x9c
> [ 62.857814] el0_svc+0x20/0x30
> [ 62.857816] el0_sync_handler+0x1a4/0x1b0
> [ 62.857818] el0_sync+0x174/0x180
> [ 62.857842] SMP: stopping secondary CPUs
> [ 62.857845] Kernel Offset: 0x52ea0b08 from 0x80001000
> [ 62.857847] PHYS_OFFSET: 0xb1cc
> [ 62.857849] CPU features: 0x00240022,61806000
> [ 62.857851] Memory Limit: none
>
> Sprinkling printks around regmap_read [1] shows that reading from 0x14 (20)
> seems to cause the issue:
>
>
> [ 40.456230] map=020a069c9c00, from=0, to=3836, count=131072
> [ 40.462520] map=020a069c9c00, i=0
> [ 40.466319] ret=0, val=0
> [ 40.468922] map=020a069c9c00, i=4
> [ 40.472684] ret=0, val=0
> [ 40.475292] map=020a069c9c00, i=8
> [ 40.479048] ret=0, val=0
> [ 40.481649] map=020a069c9c00, i=12
> [ 40.485492] ret=0, val=0
> [ 40.488080]
Re: [PATCH] staging: bcm2835-audio: Replace unsafe strcpy() with strscpy()
On Fri, 2021-02-05 at 08:25 +0100, Juerg Haefliger wrote: > Replace strcpy() with strscpy() in bcm2835-audio/bcm2835.c to prevent the > following when loading snd-bcm2835: > > [ 58.480634] [ cut here ] > [ 58.485321] kernel BUG at lib/string.c:1149! > [ 58.489650] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > [ 58.495214] Modules linked in: snd_bcm2835(COE+) snd_pcm snd_timer snd > dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua btsdio bluetooth > ecdh_generic ecc bcm2835_v4l2(CE) bcm2835_codec(CE) brcmfmac bcm2835_isp(CE) > bcm2835_mmal_vchiq(CE) brcmutil cfg80211 v4l2_mem2mem videobuf2_vmalloc > videobuf2_dma_contig videobuf2_memops raspberrypi_hwmon videobuf2_v4l2 > videobuf2_common videodev bcm2835_gpiomem mc vc_sm_cma(CE) rpivid_mem > uio_pdrv_genirq uio sch_fq_codel drm ip_tables x_tables autofs4 btrfs > blake2b_generic raid10 raid456 async_raid6_recov async_memcpy async_pq > async_xor async_tx xor xor_neon raid6_pq libcrc32c raid1 raid0 multipath > linear dwc2 roles spidev udc_core crct10dif_ce xhci_pci xhci_pci_renesas > phy_generic aes_neon_bs aes_neon_blk crypto_simd cryptd > [ 58.563787] CPU: 3 PID: 1959 Comm: insmod Tainted: G C OE > 5.11.0-1001-raspi #1 > [ 58.572172] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT) > [ 58.578086] pstate: 6045 (nZCv daif +PAN -UAO -TCO BTYPE=--) > [ 58.584178] pc : fortify_panic+0x20/0x24 > [ 58.588161] lr : fortify_panic+0x20/0x24 > [ 58.592136] sp : 800010a83990 > [ 58.595491] x29: 800010a83990 x28: 0002 > [ 58.600879] x27: b0b07cb72928 x26: > [ 58.606268] x25: 39e884973838 x24: b0b07cb74190 > [ 58.611655] x23: b0b07cb72030 x22: > [ 58.617042] x21: 39e884973014 x20: 39e88b793010 > [ 58.622428] x19: b0b07cb72670 x18: 0030 > [ 58.627814] x17: x16: b0b092ce2c1c > [ 58.633200] x15: 39e88b901500 x14: 0720072007200720 > [ 58.638588] x13: 0720072007200720 x12: 0720072007200720 > [ 58.643979] x11: b0b0936cbdf0 x10: f000 > [ 58.649366] x9 : b0b09220cfa8 x8 : > [ 58.654752] x7 : b0b093673df0 x6 : b0b09364e000 > [ 58.660140] x5 : x4 : 39e93b7db948 > [ 58.665526] x3 : 39e93b7ebcf0 x2 : > [ 58.670913] x1 : x0 : 0022 > [ 58.676299] Call trace: > [ 58.678775] fortify_panic+0x20/0x24 > [ 58.682402] snd_bcm2835_alsa_probe+0x5b8/0x7d8 [snd_bcm2835] > [ 58.688247] platform_probe+0x74/0xe4 > [ 58.691963] really_probe+0xf0/0x510 > [ 58.695585] driver_probe_device+0xe0/0x100 > [ 58.699826] device_driver_attach+0xcc/0xd4 > [ 58.704068] __driver_attach+0xb0/0x17c > [ 58.707956] bus_for_each_dev+0x7c/0xd4 > [ 58.711843] driver_attach+0x30/0x40 > [ 58.715467] bus_add_driver+0x154/0x250 > [ 58.719354] driver_register+0x84/0x140 > [ 58.723242] __platform_driver_register+0x34/0x40 > [ 58.728013] bcm2835_alsa_driver_init+0x30/0x1000 [snd_bcm2835] > [ 58.734024] do_one_initcall+0x54/0x300 > [ 58.737914] do_init_module+0x60/0x280 > [ 58.741719] load_module+0x680/0x770 > [ 58.745344] __do_sys_finit_module+0xbc/0x130 > [ 58.749761] __arm64_sys_finit_module+0x2c/0x40 > [ 58.754356] el0_svc_common.constprop.0+0x88/0x220 > [ 58.759216] do_el0_svc+0x30/0xa0 > [ 58.762575] el0_svc+0x28/0x70 > [ 58.765669] el0_sync_handler+0x1a4/0x1b0 > [ 58.769732] el0_sync+0x178/0x180 > [ 58.773095] Code: aa0003e1 91366040 910003fd 97ffee21 (d421) > [ 58.779275] ---[ end trace 29be5b17497bd898 ]--- > [ 58.783955] note: insmod[1959] exited with preempt_count 1 > [ 58.791921] --------[ cut here ] > > For the sake of it, replace all the other occurences of strcpy() under > bcm2835-audio/ as well. > > Signed-off-by: Juerg Haefliger > --- Reviewed-by: Nicolas Saenz Julienne Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC/PATCH v2 09/16] soc: bcm: bcm2835-power: Add support for BCM2711's Argon ASB
Hi Phil, On Tue, 2021-02-09 at 16:38 +, Phil Elwell wrote: > Nicolas, > > > > Please don't refer to this block as ARGON - it is the IP of Raspberry > > > Pi Trading and it's name is RPiVid. > > > > OK, sorry for that. I, again, mixed both ASB names. I'll rename the Argon > > ASB > > to RPiVid. How should I call the one present in older RPis? > > It can keep its unqualified name of ASB_*. Fair enough. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC/PATCH v2 09/16] soc: bcm: bcm2835-power: Add support for BCM2711's Argon ASB
On Tue, 2021-02-09 at 13:19 +, Phil Elwell wrote:
> Hi Nicolas,
>
> On Tue, 9 Feb 2021 at 13:00, Nicolas Saenz Julienne
> wrote:
> >
> > In BCM2711 the new ARGON ASB took over V3D. The old ASB is still present
> > with the ISP and H264 bits, and V3D is in the same place in the new ASB
> > as the old one.
> >
> > Use the fact that 'pm->argon_asb' is populated as a hint that we're on
> > BCM2711. On top of that introduce the macro ASB_BASE() which will select
> > the correct ASB register base, based on whether we're trying to access
> > V3D and which platform we're on.
>
> Please don't refer to this block as ARGON - it is the IP of Raspberry
> Pi Trading and it's name is RPiVid.
OK, sorry for that. I, again, mixed both ASB names. I'll rename the Argon ASB
to RPiVid. How should I call the one present in older RPis?
> > Signed-off-by: Nicolas Saenz Julienne
> >
> > ---
> >
> > Changes since v1:
> > - Correct names
> >
> > drivers/soc/bcm/bcm2835-power.c | 68 -
> > 1 file changed, 42 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/soc/bcm/bcm2835-power.c
> > b/drivers/soc/bcm/bcm2835-power.c
> > index 59b8abfc5617..42e105758b47 100644
> > --- a/drivers/soc/bcm/bcm2835-power.c
[...]
> > case BCM2835_POWER_DOMAIN_USB:
> > PM_WRITE(PM_USB, 0);
> > @@ -626,13 +633,22 @@ static int bcm2835_power_probe(struct platform_device
> > *pdev)
> > power->dev = dev;
> > power->base = pm->base;
> > power->rpivid_asb = pm->rpivid_asb;
> > + power->argon_asb = pm->argon_asb;
> >
> > - id = ASB_READ(ASB_AXI_BRDG_ID);
> > + id = ASB_READ(ASB_AXI_BRDG_ID, false);
> > if (id != 0x62726467 /* "BRDG" */) {
> > - dev_err(dev, "ASB register ID returned 0x%08x\n", id);
> > + dev_err(dev, "RPiVid ASB register ID returned 0x%08x\n",
> > id);
> > return -ENODEV;
> > }
> >
> > + if (pm->argon_asb) {
> > + id = ASB_READ(ASB_AXI_BRDG_ID, true);
> > + if (id != 0x62726467 /* "BRDG" */) {
> > + dev_err(dev, "Argon ASB register ID returned
> > 0x%08x\n", id);
> > + return -ENODEV;
> > + }
> > + }
> > +
>
> Surely these are the same register. Is this the result of a bad merge?
AFAIU There are two ASBs the old one at 0x7e00a000 and the new RPiVid one at
0x7ec11000. They both can be checked for valid IDs. Note the new argument in
ASB_READ().
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[RFC/PATCH v2 15/16] ARM: configs: Enable DRM_V3D
BCM2711, the SoC used on the Raspberry Pi 4 has a different GPU than its predecessors. Enable it. Signed-off-by: Nicolas Saenz Julienne --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 3823da605430..0ad4ffe4e6f6 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -713,6 +713,7 @@ CONFIG_DRM_TOSHIBA_TC358764=m CONFIG_DRM_I2C_ADV7511=m CONFIG_DRM_I2C_ADV7511_AUDIO=y CONFIG_DRM_STI=m +CONFIG_DRM_V3D=m CONFIG_DRM_VC4=m CONFIG_DRM_ETNAVIV=m CONFIG_DRM_MXSFB=m -- 2.30.0
[RFC/PATCH v2 16/16] arm64: config: Enable DRM_V3D
BCM2711, the SoC used on the Raspberry Pi 4 has a different GPU than its predecessors. Enable it. Signed-off-by: Nicolas Saenz Julienne --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 38c9e921f50e..cbcbe72fbd56 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -699,6 +699,7 @@ CONFIG_DRM_I2C_ADV7511_AUDIO=y CONFIG_DRM_DW_HDMI_AHB_AUDIO=m CONFIG_DRM_DW_HDMI_CEC=m CONFIG_DRM_IMX_DCSS=m +CONFIG_DRM_V3D=m CONFIG_DRM_VC4=m CONFIG_DRM_ETNAVIV=m CONFIG_DRM_HISI_HIBMC=m -- 2.30.0
