Re: [PATCH v8 0/4] USB: pci-quirks: Add Raspberry Pi 4 quirk
On Wed, 2020-05-13 at 12:17 +0100, Lorenzo Pieralisi wrote: > On Tue, May 05, 2020 at 06:13:13PM +0200, Nicolas Saenz Julienne wrote: > > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be > > loaded directly from an EEPROM or, if not present, by the SoC's > > co-processor, VideoCore. This series adds support for the later. > > > > Note that there are a set of constraints we have to consider: > > - We need to make sure the VideoCore firmware interface is up and > >running before running the VL805 firmware load call. > > > > - There is no way to discern RPi4's VL805 chip from other platforms', > >so we need the firmware load to happen *before* running > >quirk_usb_handoff_xhci(). Failure to do so results in an unwarranted > >5 second wait while the fixup code polls xHC's non-existing state. > > > > By Florian's suggestion I've been spending some time exploring the device > > link[1] API in order to see if that could save us from explicitly creating > > probe dependencies between pcie-brcmstb and firmware/raspberrypi (patch #3). > > Technically these dependencies could be inferred from DT. It turns out > > Saravana > > Kannan has been looking at this already. A new boot mechanism, activated > > with > > fw_devlink=on takes care of the device probe ordering on devices with > > consumer/supplier relationships. For now this relationship is created based > > on > > the usage of generic DT properties, but has no support for vendor-specifc DT > > properties, which we'd be forced to use in order to create a relationship > > between our two devices since our setup is highly non generic. There will > > probably be at some point support for such properties, and we will then be > > able > > to revisit some of this code. > > > > All this is based on the work by Tim Gover in RPi's downstream > > kernel[2]. > > > > [1] https://www.kernel.org/doc/html/v4.13/driver-api/device_link.html > > [2] > > https://github.com/raspberrypi/linux/commit/9935b4c7e360b4494b4cb6e3ce797238a1ab78bd > > > > --- > > > > Changes since v7: > > - Address Stefan's comments > > > > Changes since v6: > > - Make rpi_firmware_init_vl805() more robust > > - Rewrite comments and patch descriptions to be more accessible to non RPi > >fluent people > > - Removed Florian's Reviewed-by in patch #2 as function changed > >substantially > > - Tested with/witout u-boot > > > > Changes since v5: > > - Fix issues reported by Kbuild test robot > > > > Changes since v4: > > - Addressed Sergei's comments > > - Fix potential warning in patch #2 > > > > Changes since v3: > > - Addressed Greg's comments > > > > There was no v2, my bad. > > > > Changes since v1: > > - Addressed Floarians comments > > > > Nicolas Saenz Julienne (4): > > soc: bcm2835: Add notify xHCI reset property > > firmware: raspberrypi: Introduce vl805 init routine > > PCI: brcmstb: Wait for Raspberry Pi's firmware when present > > USB: pci-quirks: Add Raspberry Pi 4 quirk > > > > drivers/firmware/Kconfig | 3 +- > > drivers/firmware/raspberrypi.c | 61 ++ > > drivers/pci/controller/pcie-brcmstb.c | 17 ++ > > drivers/usb/host/pci-quirks.c | 16 ++ > > include/soc/bcm2835/raspberrypi-firmware.h | 9 +++- > > 5 files changed, 104 insertions(+), 2 deletions(-) > > Hi Nicolas, > > should I queue this series via the PCI tree ? Just let me know, most of > the changes are not in the PCI tree, asking in order to > minimize/simplify conflicts handling if possible. Yes, I agree, it's better if you take the whole thing. Thanks, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 0/2] usb: xhci: Load Raspberry Pi 4 VL805's firmware
On Tue, 2020-05-05 at 18:26 +0200, Nicolas Saenz Julienne wrote: > Newer revisions of the RPi4 need their xHCI chip, VL805, firmware to be > loaded explicitly. Earlier versions didn't need that as they where using > an EEPROM for that purpose. This series takes care of setting up the > relevant infrastructure and run the firmware loading routine at the > right moment. > > Note that this builds on top of Sylwester Nawrocki's "USB host support > for Raspberry Pi 4 board" series. > > --- Just for the record, here's the version of this in the Linux Kernel: https://patchwork.kernel.org/cover/11529585/ Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 04/15] PCI: brcmstb: Add compatibily of other chips
On Thu, 2020-05-21 at 15:35 -0400, Jim Quinlan wrote:
> On Wed, May 20, 2020 at 7:51 AM Nicolas Saenz Julienne
[...]
> > > /*
> > > @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = {
> > >
> > > static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie,
> > > u32
> > > val)
> > > {
> > > - u32 tmp;
> > > + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK];
> > > + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT];
> >
> > I don't think you need shift here, IIUC u32p_replace_bits() will take care
> > of
> > all the masking and shifting internally, moreover, you'd be able to drop the
> > shift entry from reg_field_info.
> I believe that u32p_replace_bits requires at least one of the value or
> mask to be compile time constants to work and we don't have that here.
Of course, sorry for the noise then.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[PATCH 0/9] Raspberry Pi 4 USB firmware initialization rework
On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be loaded directly from an EEPROM or, if not present, by the SoC's co-processor, VideoCore. This series reworks how we handle this. The previous solution makes use of PCI quirks and exporting platform specific functions. Albeit functional it feels pretty shoehorned. This proposes an alternative way of handling the triggering of the xHCI chip initialization trough means of a reset controller. The benefits are pretty evident: less platform churn in core xHCI code, and no explicit device dependency management in pcie-brcmstb. Note that patch #1 depend on another series[1]. The series is based on next-20200605. [1] https://lwn.net/ml/linux-kernel/cover.662a8d401787ef33780d91252a352de91dc4be10.1590594293.git-series.max...@cerno.tech/ --- Nicolas Saenz Julienne (9): dt-bindings: reset: Add a binding for the RPi Firmware USB reset reset: Add Raspberry Pi 4 firmware USB reset controller ARM: dts: bcm2711: Add firmware usb reset node ARM: dts: bcm2711: Add reset controller to xHCI node usb: xhci-pci: Add support for reset controllers Revert "USB: pci-quirks: Add Raspberry Pi 4 quirk" usb: host: pci-quirks: Bypass xHCI quirks for Raspberry Pi 4 Revert "firmware: raspberrypi: Introduce vl805 init routine" Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when present" .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++ arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 12 ++ drivers/firmware/Kconfig | 3 +- drivers/firmware/raspberrypi.c| 61 - drivers/pci/controller/pcie-brcmstb.c | 17 --- drivers/reset/Kconfig | 9 ++ drivers/reset/Makefile| 1 + drivers/reset/reset-raspberrypi-usb.c | 122 ++ drivers/usb/host/pci-quirks.c | 22 ++-- drivers/usb/host/xhci-pci.c | 9 ++ include/soc/bcm2835/raspberrypi-firmware.h| 7 - 11 files changed, 184 insertions(+), 100 deletions(-) create mode 100644 drivers/reset/reset-raspberrypi-usb.c -- 2.26.2
[PATCH 2/9] reset: Add Raspberry Pi 4 firmware USB reset controller
The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip
that implements the xHCI. After a PCI fundamental reset, VL805's
firmware may either be loaded directly from an EEPROM or, if not
present, by the SoC's co-processor, VideoCore. RPi4's VideoCore OS
contains both the non public firmware load logic and the VL805 firmware
blob.
We control this trough a reset controller device that's able to trigger
the aforementioned process when relevant.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/reset/Kconfig | 9 ++
drivers/reset/Makefile| 1 +
drivers/reset/reset-raspberrypi-usb.c | 122 ++
3 files changed, 132 insertions(+)
create mode 100644 drivers/reset/reset-raspberrypi-usb.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index d9efbfd29646..80e07190cd04 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -140,6 +140,15 @@ config RESET_QCOM_PDC
to control reset signals provided by PDC for Modem, Compute,
Display, GPU, Debug, AOP, Sensors, Audio, SP and APPS.
+config RESET_RASPBERRYPI_USB
+ tristate "Raspberry Pi 4 USB Reset Driver"
+ depends on RASPBERRYPI_FIRMWARE || (RASPBERRYPI_FIRMWARE=n &&
COMPILE_TEST)
+ default USB_XHCI_PCI
+ help
+ This driver provides support for resetting the USB HW available in
+ the Raspberry Pi 4. This reset process is controlled by firmware
+ through a custom interface (see drivers/firmware/raspberrypi.c).
+
config RESET_SCMI
tristate "Reset driver controlled via ARM SCMI interface"
depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 249ed357c997..49cd2868c7ab 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_QCOM_AOSS) += reset-qcom-aoss.o
obj-$(CONFIG_RESET_QCOM_PDC) += reset-qcom-pdc.o
+obj-$(CONFIG_RESET_RASPBERRYPI_USB) += reset-raspberrypi-usb.o
obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
diff --git a/drivers/reset/reset-raspberrypi-usb.c
b/drivers/reset/reset-raspberrypi-usb.c
new file mode 100644
index ..e9a6e7018c6d
--- /dev/null
+++ b/drivers/reset/reset-raspberrypi-usb.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Raspberry Pi 4 USB reset driver
+ *
+ * The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip that
+ * implements xHCI. After a PCI reset, VL805's firmware may either be loaded
+ * directly from an EEPROM or, if not present, by the SoC's co-processor,
+ * VideoCore. rpi's VideoCore OS contains both the non public firmware load
+ * logic and the VL805 firmware blob. This driver triggers the aforementioned
+ * process.
+ *
+ * Copyright (C) 2020 Nicolas Saenz Julienne
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct rpi_usb_reset {
+ struct reset_controller_dev rcdev;
+ struct rpi_firmware *fw;
+};
+
+static inline
+struct rpi_usb_reset *to_rpi_usb(struct reset_controller_dev *rcdev)
+{
+ return container_of(rcdev, struct rpi_usb_reset, rcdev);
+}
+
+static int rpi_usb_reset_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct rpi_usb_reset *priv = to_rpi_usb(rcdev);
+ u32 dev_addr;
+ int ret;
+
+ /*
+* The pci device address is expected like this:
+*
+* PCI_BUS << 20 | PCI_SLOT << 15 | PCI_FUNC << 12
+*
+* But since rpi's PCIe setup is hardwired, we know the address in
+* advance.
+*/
+ dev_addr = 0x10;
+ ret = rpi_firmware_property(priv->fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
+ &dev_addr, sizeof(dev_addr));
+ if (ret)
+ return ret;
+
+ /* Wait for vl805 to startup */
+ usleep_range(200, 1000);
+
+ return 0;
+}
+
+static const struct reset_control_ops rpi_usb_reset_ops = {
+ .reset = rpi_usb_reset_reset,
+};
+
+static int rpi_usb_reset_xlate(struct reset_controller_dev *rcdev,
+ const struct of_phandle_args *reset_spec)
+{
+ /* This is needed if #reset-cells == 0. */
+ return 0;
+}
+
+static int rpi_usb_reset_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *fw_node;
+ struct rpi_usb_reset *priv;
+ struct rpi_firmware *fw;
+
+ fw_node = of_get_parent(dev->of_node);
+ if (!fw_node) {
+ dev_err(dev, "Missing firmware node\n");
+ return -ENOENT;
+ }
+
+ fw
[PATCH 7/9] usb: host: pci-quirks: Bypass xHCI quirks for Raspberry Pi 4
The board doesn't need the quirks to be run, and takes case of its own initialization trough a reset controller device. So let's bypass it quirk. Signed-off-by: Nicolas Saenz Julienne --- drivers/usb/host/pci-quirks.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 92150ecdb036..4b3be05d1290 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -16,6 +16,8 @@ #include #include #include +#include + #include "pci-quirks.h" #include "xhci-ext-caps.h" @@ -1248,6 +1250,16 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev) */ if (pdev->vendor == 0x184e) /* vendor Netlogic */ return; + + /* +* Bypass the Raspberry Pi 4 controller xHCI controller, things are +* taken care by the board's co-processor. +*/ + if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483 && + of_device_is_compatible(of_get_parent(pdev->bus->dev.of_node), + "brcm,bcm2711-pcie")) + return; + if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI && pdev->class != PCI_CLASS_SERIAL_USB_OHCI && pdev->class != PCI_CLASS_SERIAL_USB_EHCI && -- 2.26.2
[PATCH 3/9] ARM: dts: bcm2711: Add firmware usb reset node
Now that the reset driver exposing Raspberry Pi 4's firmware based USB
reset routine is available, let's add the device tree node exposing it.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index c7f1d97e69bb..47e7c9c14ddf 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -83,6 +83,11 @@ expgpio: gpio {
"";
status = "okay";
};
+
+ usb_reset: usb-reset {
+ compatible = "raspberrypi,firmware-usb-reset";
+ #reset-cells = <0>;
+ };
};
&gpio {
--
2.26.2
[PATCH 8/9] Revert "firmware: raspberrypi: Introduce vl805 init routine"
This reverts commit fbbc5ff3f7f9f4cad562e530ae2cf5d8964fe6d3.
The vl805 routine has moved to drivers/reset/reset-raspberrypi-usb.c
Signed-off-by: Nicolas Saenz Julienne
---
drivers/firmware/raspberrypi.c | 61 --
include/soc/bcm2835/raspberrypi-firmware.h | 7 ---
2 files changed, 68 deletions(-)
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index ef8098856a47..a3e85186f8e6 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -12,8 +12,6 @@
#include
#include
#include
-#include
-#include
#include
#define MBOX_MSG(chan, data28) (((data28) & ~0xf) | ((chan) & 0xf))
@@ -21,8 +19,6 @@
#define MBOX_DATA28(msg) ((msg) & ~0xf)
#define MBOX_CHAN_PROPERTY 8
-#define VL805_PCI_CONFIG_VERSION_OFFSET0x50
-
static struct platform_device *rpi_hwmon;
static struct platform_device *rpi_clk;
@@ -284,63 +280,6 @@ struct rpi_firmware *rpi_firmware_get(struct device_node
*firmware_node)
}
EXPORT_SYMBOL_GPL(rpi_firmware_get);
-/*
- * The Raspberry Pi 4 gets its USB functionality from VL805, a PCIe chip that
- * implements xHCI. After a PCI reset, VL805's firmware may either be loaded
- * directly from an EEPROM or, if not present, by the SoC's co-processor,
- * VideoCore. RPi4's VideoCore OS contains both the non public firmware load
- * logic and the VL805 firmware blob. This function triggers the aforementioned
- * process.
- */
-int rpi_firmware_init_vl805(struct pci_dev *pdev)
-{
- struct device_node *fw_np;
- struct rpi_firmware *fw;
- u32 dev_addr, version;
- int ret;
-
- fw_np = of_find_compatible_node(NULL, NULL,
- "raspberrypi,bcm2835-firmware");
- if (!fw_np)
- return 0;
-
- fw = rpi_firmware_get(fw_np);
- of_node_put(fw_np);
- if (!fw)
- return -ENODEV;
-
- /*
-* Make sure we don't trigger a firmware load unnecessarily.
-*
-* If something went wrong with PCI, this whole exercise would be
-* futile as VideoCore expects from us a configured PCI bus. Just take
-* the faulty version (likely ~0) and let xHCI's registration fail
-* further down the line.
-*/
- pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET, &version);
- if (version)
- goto exit;
-
- dev_addr = pdev->bus->number << 20 | PCI_SLOT(pdev->devfn) << 15 |
- PCI_FUNC(pdev->devfn) << 12;
-
- ret = rpi_firmware_property(fw, RPI_FIRMWARE_NOTIFY_XHCI_RESET,
- &dev_addr, sizeof(dev_addr));
- if (ret)
- return ret;
-
- /* Wait for vl805 to startup */
- usleep_range(200, 1000);
-
- pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET,
- &version);
-exit:
- pci_info(pdev, "VL805 firmware version %08x\n", version);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(rpi_firmware_init_vl805);
-
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 3025aca3c358..cc9cdbc66403 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -10,7 +10,6 @@
#include
struct rpi_firmware;
-struct pci_dev;
enum rpi_firmware_property_status {
RPI_FIRMWARE_STATUS_REQUEST = 0,
@@ -142,7 +141,6 @@ int rpi_firmware_property(struct rpi_firmware *fw,
int rpi_firmware_property_list(struct rpi_firmware *fw,
void *data, size_t tag_size);
struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
-int rpi_firmware_init_vl805(struct pci_dev *pdev);
#else
static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
void *data, size_t len)
@@ -160,11 +158,6 @@ static inline struct rpi_firmware *rpi_firmware_get(struct
device_node *firmware
{
return NULL;
}
-
-static inline int rpi_firmware_init_vl805(struct pci_dev *pdev)
-{
- return 0;
-}
#endif
#endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
--
2.26.2
[PATCH 6/9] Revert "USB: pci-quirks: Add Raspberry Pi 4 quirk"
This reverts commit c65822fef4adc0ba40c37a47337376ce75f7a7bc.
The initialization of Raspberry Pi 4's USB chip is now handled through a
reset controller. No need to directly call the firmware routine trough a
pci quirk.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/firmware/Kconfig | 3 +--
drivers/usb/host/pci-quirks.c | 16
2 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index fbd785dd0513..4843e94713a4 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -178,9 +178,8 @@ config ISCSI_IBFT
Otherwise, say N.
config RASPBERRYPI_FIRMWARE
- bool "Raspberry Pi Firmware Driver"
+ tristate "Raspberry Pi Firmware Driver"
depends on BCM2835_MBOX
- default USB_PCI
help
This option enables support for communicating with the firmware on the
Raspberry Pi.
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 0b949acfa258..92150ecdb036 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,9 +16,6 @@
#include
#include
#include
-
-#include
-
#include "pci-quirks.h"
#include "xhci-ext-caps.h"
@@ -1246,24 +1243,11 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
static void quirk_usb_early_handoff(struct pci_dev *pdev)
{
- int ret;
-
/* Skip Netlogic mips SoC's internal PCI USB controller.
* This device does not need/support EHCI/OHCI handoff
*/
if (pdev->vendor == 0x184e) /* vendor Netlogic */
return;
-
- if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483) {
- ret = rpi_firmware_init_vl805(pdev);
- if (ret) {
- /* Firmware might be outdated, or something failed */
- dev_warn(&pdev->dev,
-"Failed to load VL805's firmware: %d. Will
continue to attempt to work, but bad things might happen. You should fix
this...\n",
-ret);
- }
- }
-
if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
--
2.26.2
[PATCH 1/9] dt-bindings: reset: Add a binding for the RPi Firmware USB reset
The firmware running on the RPi VideoCore can be used to reset and
initialize the board's xHCI controller. The reset controller is passed
to the PCI device through the DT, hence this binding.
Signed-off-by: Nicolas Saenz Julienne
---
.../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++
1 file changed, 21 insertions(+)
diff --git
a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index b48ed875eb8e..8f9d0986c28f 100644
---
a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++
b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -39,6 +39,22 @@ properties:
- compatible
- "#clock-cells"
+ usb-reset:
+type: object
+
+properties:
+ compatible:
+const: raspberrypi,firmware-usb-reset
+
+ "#clock-cells":
+const: 0
+description: >
+ There is only one reset line available, so no need for cell decoding.
+
+required:
+ - compatible
+ - "#reset-cells"
+
additionalProperties: false
required:
@@ -55,5 +71,10 @@ examples:
compatible = "raspberrypi,firmware-clocks";
#clock-cells = <1>;
};
+
+usb_reset: usb-reset {
+compatible = "raspberrypi,firmware-usb-reset";
+#reset-cells = <0>;
+};
};
...
--
2.26.2
[PATCH 9/9] Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when present"
This reverts commit 44331189f9082c7e659697bbac1747db3def73e7.
Now that the VL805 init routine is run through a reset controller driver
the dependencies are being taken care of by the device core. No need to
do it manually here.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/pci/controller/pcie-brcmstb.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c
b/drivers/pci/controller/pcie-brcmstb.c
index 7730ea845ff2..752f5b331579 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -28,8 +28,6 @@
#include
#include
-#include
-
#include "../pci.h"
/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
@@ -931,26 +929,11 @@ static int brcm_pcie_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node, *msi_np;
struct pci_host_bridge *bridge;
- struct device_node *fw_np;
struct brcm_pcie *pcie;
struct pci_bus *child;
struct resource *res;
int ret;
- /*
-* We have to wait for Raspberry Pi's firmware interface to be up as a
-* PCI fixup, rpi_firmware_init_vl805(), depends on it. This driver's
-* probe can race with the firmware interface's (see
-* drivers/firmware/raspberrypi.c) and potentially break the PCI fixup.
-*/
- fw_np = of_find_compatible_node(NULL, NULL,
- "raspberrypi,bcm2835-firmware");
- if (fw_np && !rpi_firmware_get(fw_np)) {
- of_node_put(fw_np);
- return -EPROBE_DEFER;
- }
- of_node_put(fw_np);
-
bridge = devm_pci_alloc_host_bridge(&pdev->dev, sizeof(*pcie));
if (!bridge)
return -ENOMEM;
--
2.26.2
[PATCH 5/9] usb: xhci-pci: Add support for reset controllers
Some atypical users of xhci-pci might need to manually reset their xHCI
controller before starting the HCD setup. Check if a reset controller
device is available to the PCI bus and trigger a reset.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/usb/host/xhci-pci.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index ef513c2fb843..45f70facdfcd 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "xhci.h"
#include "xhci-trace.h"
@@ -339,6 +340,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct
pci_device_id *id)
struct xhci_hcd *xhci;
struct usb_hcd *hcd;
struct xhci_driver_data *driver_data;
+ struct reset_control *reset;
driver_data = (struct xhci_driver_data *)id->driver_data;
if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
@@ -347,6 +349,13 @@ static int xhci_pci_probe(struct pci_dev *dev, const
struct pci_device_id *id)
return retval;
}
+ reset = devm_reset_control_get(&dev->bus->dev, NULL);
+ if (IS_ERR(reset)) {
+ retval = PTR_ERR(reset);
+ return retval;
+ }
+ reset_control_reset(reset);
+
/* Prevent runtime suspending between USB-2 and USB-3 initialization */
pm_runtime_get_noresume(&dev->dev);
--
2.26.2
[PATCH 4/9] ARM: dts: bcm2711: Add reset controller to xHCI node
The chip is hardwired to the board's PCIe bus and needs to be properly
setup trough a firmware routine after a PCI fundamental reset. Pass the
reset controller phandle that takes care of triggering the
initialization to the relevant PCI device.
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 47e7c9c14ddf..2646c858449f 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -207,6 +207,13 @@ phy1: ethernet-phy@1 {
};
};
+&pcie0 {
+ usb@1,0 {
+ reg = <0 0 0 0 0>;
+ resets = <&usb_reset>;
+ };
+};
+
/* uart0 communicates with the BT module */
&uart0 {
pinctrl-names = "default";
--
2.26.2
Re: [PATCH v2 28/47] staging: vchi: Get rid of vchiq_shim's message callback
Hi Jacopo, sorry if I'm a little late with my replies but I'm on vacation. I'll be back Sept 7th, but wanted to reply since I don't want to stop your work. On Fri, 2020-08-28 at 16:31 +0200, Jacopo Mondi wrote: > Hi Nicolas, > >I'm working on a v2 of the bcm2835-isp support which was sent along > with UNICAM v4l2 driver and some misc changes you have collected in > this series. Reference to v1: > https://lore.kernel.org/linux-media/20200504092611.9798-1-laurent.pinch...@ideasonboard.com/ > > On Mon, Jun 29, 2020 at 05:09:26PM +0200, Nicolas Saenz Julienne wrote: > > As vchiq_shim's callback does nothing aside from pushing messages into > > the service's queue, let's bypass it and jump directly to the service's > > callbacks, letting them choose whether to use the message queue. > > I admit this patch caused me some pain, as after a few days chasing > why the ISP got stuck in importing buffers into the VPU through the vc-sm-cma > driver I realized that this patch removed a significant part of the > process.. Sorry for the pain, I made my best to keep the downstream code in mind, and also to keep the amount of functional changes needed in the services minimal. That said, getting rid of VCHI is, IMO, a necessary step towards making VCHIQ upstreamable. > > It turns out most services don't need to use the message queue, which > > makes for simpler code in the end. > > > > - > > - if (reason == VCHIQ_MESSAGE_AVAILABLE) > > - vchiq_msg_queue_push(service->handle, header); > > This one '-.- > > I wonder if this was intentional and it is expected all services now > handle the message queue (it seems so according to your commit > message). Indeed, it was intentional. Upstream services (mmal & audio) don't need it and IIRC vchiq's ioctl interface has it's own queue implementation. So I figured it would be best to keep its usage optional. > Fair enough, I could add in the vc-sma-cma callback a call to > vchiq_msg_queue_push() but I wonder if it wouldn't be better to do so > in vchiq_core.c:parse_rx_slots(), just before calling the service's > callback, so that this has not to be re-implemented in all services. > > What would you suggest ? Actually, in hindsight my suggestion would be to get rid of the vchiq message queue altogether[1], keeping VCHIQ as simple as possible is a must if we want to get it upstream, and since vc-sma-cma is the only queue user there is little benefit to having a generic implementation. Let the service do it's own custom queueing and just force all services to release the messages when they see fit. It'll make for a simpler VCHIQ usage. > And by the way I see mmal-vchiq.c:service_callback() releasing > messages but never pushing them to the queue. Is this intended as well ? Yes, sorry, it's pretty confusing. That call, vchiq_release_message(), has nothing to do with the queue. It's the call that really gets the core to release the message from its slot and the only mandatory thing to do after receiving a message. All in all VCHIQ isn't documented an pretty cryptic in its design. I don't claim to be an expert. So feel free to contradict me anytime, I'll be happy to work out something that fits all of us. Regards, Nicolas [1] Here I mean vchiq_msg_queue_push() & vchiq_msg_hold()
Re: [PATCH v2 2/2] dma-pool: Only allocate from CMA when in same memory zone
On Tue, 2020-08-04 at 08:06 +0200, Christoph Hellwig wrote: > On Mon, Aug 03, 2020 at 06:09:56PM +0200, Nicolas Saenz Julienne wrote: > > + if (IS_ENABLED(CONFIG_ZONE_DMA) && (gfp & GFP_DMA)) > > + return end <= DMA_BIT_MASK(zone_dma_bits); > > + if (IS_ENABLED(CONFIG_ZONE_DMA32) && (gfp & GFP_DMA32)) > > + return end <= DMA_BIT_MASK(32); > > + if (gfp & GFP_KERNEL) > > + return end > DMA_BIT_MASK(32); > > So the GFP_KERNEL one here looks weird. For one I don't think the if > line is needed at all, and it just confuses things. Yes, sorry, shoud've seen that. > Second I don't see the need (and actually some harm) in preventing GFP_KERNEL > allocations from dipping into lower CMA areas - something that we did support > before 5.8 with the single pool. My thinking is the least we pressure CMA the better, it's generally scarse, and it'll not grow as the atomic pools grow. As far as harm is concerned, we now check addresses for correctness, so we shouldn't run into problems. There is a potential case for architectures defining a default CMA but not defining DMA zones where this could be problematic. But isn't that just plain abusing CMA? If you need low memory allocations, you should be defining DMA zones. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC] arm64: mm: Do not use both DMA zones when 30-bit address space unavailable
Hi Catalin, sorry for the late reply, I was on vacation.
On Fri, 2020-08-28 at 18:43 +0100, Catalin Marinas wrote:
> Hi Nicolas,
>
> On Wed, Aug 19, 2020 at 08:24:33PM +0200, Nicolas Saenz Julienne wrote:
> > There is no benefit in splitting the 32-bit address space into two
> > distinct DMA zones when the 30-bit address space isn't even available on
> > a device. If that is the case, default to one big ZONE_DMA spanning the
> > whole 32-bit address space.
> >
> > This will help reduce some of the issues we've seen with big crash
> > kernel allocations.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> >
> > Whith this patch, on a 8GB RPi4 the setup looks like this:
> >
> > DMA [mem 0x-0x3fff]
> > DMA32[mem 0x4000-0x]
> > Normal [mem 0x0001-0x0001]
> >
> > And stock 8GB virtme/qemu:
> >
> > DMA [mem 0x4000-0x]
> > DMA32empty
> > Normal [mem 0x0001-0x00023fff]
> >
> > arch/arm64/mm/init.c | 29 +
> > 1 file changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index b6881d61b818..857a62611d7a 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -183,13 +183,20 @@ static void __init reserve_elfcorehdr(void)
> >
> > /*
> > * Return the maximum physical address for a zone with a given address size
> > - * limit. It currently assumes that for memory starting above 4G, 32-bit
> > - * devices will use a DMA offset.
> > + * limit or zero if memory starts from an address higher than the zone
> > targeted.
> > + * It currently assumes that for memory starting above 4G, 32-bit devices
> > will
> > + * use a DMA offset.
> > */
> > static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
> > {
> > - phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63,
> > zone_bits);
> > - return min(offset + (1ULL << zone_bits), memblock_end_of_DRAM());
> > + phys_addr_t base = memblock_start_of_DRAM();
> > + phys_addr_t offset = base & GENMASK_ULL(63, 32);
> > + s64 zone_size = (1ULL << zone_bits) - (base & DMA_BIT_MASK(32));
> > +
> > + if (zone_size <= 0)
> > + return 0;
> > +
> > + return min(base + zone_size + offset, memblock_end_of_DRAM());
> > }
>
> OK, so we can still get some ZONE_DMA if DRAM starts in the first GB.
>
> I don't think it entirely solves the problem.
Agree. Didn't mean to imply it.
> It just happens that the
> other affected SoCs don't have memory in the first GB. With this patch,
> we go by the assumption that ZONE_DMA/DMA32 split is only needed if
> there is memory in the low 1GB and such <32-bit devices don't have a DMA
> offset.
The way I understand it is: "we may have 30 bit DMA limited devices, the rest
can deal with 32 bit."
On top of that, I believe it makes little sense to use an offset in the
physical address space below the 32bit mark. You'd be limiting the amount of
memory available to the system. So, if you're going support DMA limited devices
on your otherwise RAM hungry SoC, you'll have to have that physical address
space directly available, or at least part of it.
All in all, I believe that assuming no 30 bit DMA limited devices exist in the
system if the physical addresses don't exist is a fairly safe.
Also note the usage of 'zone_dma_bits' in the DMA code, which assumes that
ZONE_DMA's physical address space is always smaller than (1 << zone_dma_bits) -
1.
> Adding Rob H (it's easier to ask him than grep'ing the DT files ;)), we
> may be ok with this assumption on current SoCs.
From what I've personally grep'd there is no new devices with odd ranges in
sight.
> An alternative (and I think we had a patch at some point) is to make it
> generic and parse the dma-range in the DT to identify the minimum mask
> and set ZONE_DMA accordingly. But this doesn't solve ACPI, so if Linux
> can boot with ACPI on RPi4 it would still be broken.
ACPI is being worked on by, among others, Jeremy Linton (one of your colleagues
I believe).
We could always use sane defaults for ACPI and be smarter with DT. Yet,
implementing this entails translating nested dma-ranges with the only help of
libfdt, which isn't trivial (see devices/of/address.c). IIRC RobH said that it
wasn't worth the effort just for a board.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v5 00/80] drm/vc4: Support BCM2711 Display Pipeline
Hi Maxime, On Mon, 2020-09-07 at 18:22 +0200, Maxime Ripard wrote: > Hi, > > On Thu, Sep 03, 2020 at 10:00:32AM +0200, Maxime Ripard wrote: > > Hi everyone, > > > > Here's a (pretty long) series to introduce support in the VC4 DRM driver > > for the display pipeline found in the BCM2711 (and thus the RaspberryPi 4). > > > > The main differences are that there's two HDMI controllers and that there's > > more pixelvalve now. Those pixelvalve come with a mux in the HVS that still > > have only 3 FIFOs. Both of those differences are breaking a bunch of > > expectations in the driver, so we first need a good bunch of cleanup and > > reworks to introduce support for the new controllers. > > > > Similarly, the HDMI controller has all its registers shuffled and split in > > multiple controllers now, so we need a bunch of changes to support this as > > well. > > > > Only the HDMI support is enabled for now (even though the DPI and DSI > > outputs have been tested too). > > I've applied the patches 1-79 to drm-misc. I guess the final DT patch > should go through the arm-soc tree? I'll take care of it tomorrow. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/4] clk: bcm: rpi: Add register to control pixel bvb clk
On Tue, 2020-09-01 at 13:07 +0900, Hoegeun Kwon wrote: > To use QHD or higher, we need to modify the pixel_bvb_clk value. So > add register to control this clock. > > Signed-off-by: Hoegeun Kwon > --- > drivers/clk/bcm/clk-raspberrypi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c > b/drivers/clk/bcm/clk-raspberrypi.c > index 5cc82954e1ce..f89b9cfc4309 100644 > --- a/drivers/clk/bcm/clk-raspberrypi.c > +++ b/drivers/clk/bcm/clk-raspberrypi.c > @@ -271,6 +271,7 @@ static int raspberrypi_discover_clocks(struct > raspberrypi_clk *rpi, > case RPI_FIRMWARE_CORE_CLK_ID: > case RPI_FIRMWARE_M2MC_CLK_ID: > case RPI_FIRMWARE_V3D_CLK_ID: > + case RPI_FIRMWARE_PIXEL_BVB_CLK_ID: > hw = raspberrypi_clk_register(rpi, clks->parent, > clks->id); > if (IS_ERR(hw)) Acked-by: Nicolas Saenz Julienne Tested-by: Nicolas Saenz Julienne Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [RFC] arm64: mm: Do not use both DMA zones when 30-bit address space unavailable
Hi Catalin, thanks for taking the time.
On Tue, 2020-09-08 at 12:14 +0100, Catalin Marinas wrote:
> > Also note the usage of 'zone_dma_bits' in the DMA code, which assumes that
> > ZONE_DMA's physical address space is always smaller than (1 <<
> > zone_dma_bits) -
> > 1.
>
> I think part of those uses are broken. dma_direct_supported() does the
> right thing and uses the DMA address instead of the physical one. Here
> __phys_to_dma() subtracts the dma_pfn_offset, which in my above example
> would be (0b10 << (30 - PAGE_SHIFT)).
>
> dma_direct_optimal_gfp_mask(), OTOH, seems to start ok with a
> __dma_to_phys() on the dma_limit but it ends up comparing the physical
> address with the DMA mask. This gives the wrong result on arm64
> platforms where RAM starts above 4GB and still expect a ZONE_DMA32. It
> should compare *phys_limit with __dma_to_phys(DMA_BIT_MASK(...)). I
> guess it ends up bouncing via swiotlb more often.
I'll look into this.
> We assumed such offsets on arm64 since commit d50314a6b070 ("arm64:
> Create non-empty ZONE_DMA when DRAM starts above 4GB").
>
> > > An alternative (and I think we had a patch at some point) is to make it
> > > generic and parse the dma-range in the DT to identify the minimum mask
> > > and set ZONE_DMA accordingly. But this doesn't solve ACPI, so if Linux
> > > can boot with ACPI on RPi4 it would still be broken.
> >
> > ACPI is being worked on by, among others, Jeremy Linton (one of your
> > colleagues
> > I believe).
> >
> > We could always use sane defaults for ACPI and be smarter with DT. Yet,
> > implementing this entails translating nested dma-ranges with the only help
> > of
> > libfdt, which isn't trivial (see devices/of/address.c). IIRC RobH said that
> > it
> > wasn't worth the effort just for a board.
>
> That would have been the ideal, more generic solution. But I agree that
> it's not worth the effort if the only SoC affected is RPi4.
>
> To summarise, I'd like ZONE_DMA to overlap with ZONE_DMA32 (i.e. expand
> zone_dma_bits to 32 and drop ZONE_DMA32) for all SoCs other than RPi4.
> The solutions so far:
>
> 1. Assume that, if RAM starts at 0, we need a zone_dma_bits == 30. This
>also assumes that it's only RPi4 in this category or that any such
>future SoC has a need for 30-bit DMA.
>
> 2. Adjust zone_dma_bits at boot-time only if the SoC is RPi4.
>
> 3. Use the more complex dma-ranges approach to calculate the correct
>zone_dma_bits as a minimum of all dma masks in the DT.
>
> We can discount (3) as not worth the effort. I'd go with (1) (this
> patch) if we can guarantee that no current or future SoC has RAM
> starting at 0 while not needing 30-bit DMA masks. If not, we can go with
> (2) unless others have a better suggestion.
After a quick check at the devices we have for testing at suse it's clear that
(1) is impossible. So I'll push for solution (2).
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
On Thu, 2020-09-03 at 10:01 +0200, Maxime Ripard wrote: > Now that all the drivers have been adjusted for it, let's bring in the > necessary device tree changes. > > The VEC and PV3 are left out for now, since it will require a more specific > clock setup. > > Reviewed-by: Dave Stevenson > Tested-by: Chanwoo Choi > Tested-by: Hoegeun Kwon > Tested-by: Stefan Wahren > Signed-off-by: Maxime Ripard > --- Applied for-next. Thanks! Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 0/4] drm/vc4: Support HDMI QHD or higher output
On Tue, 2020-09-01 at 13:07 +0900, Hoegeun Kwon wrote: > Hi everyone, > > There is a problem that the output does not work at a resolution > exceeding FHD. To solve this, we need to adjust the bvb clock at a > resolution exceeding FHD. > > Rebased on top of next-20200708 and [1]. > > [1] : [PATCH v4 00/78] drm/vc4: Support BCM2711 Display Pipeline (Maxime's > patchset) > > Changes from v1: > - Added dt-bindings documents > - Change patch order, first fix driver and then device tree > > Hoegeun Kwon (4): > clk: bcm: rpi: Add register to control pixel bvb clk > drm/vc4: hdmi: Add pixel bvb clock control > dt-bindings: display: vc4: hdmi: Add bvb clock-names property > ARM: dts: bcm2711: Add bvb clock for hdmi-pixel > > .../bindings/display/brcm,bcm2711-hdmi.yaml | 12 ++--- > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 +++-- > drivers/clk/bcm/clk-raspberrypi.c | 1 + > drivers/gpu/drm/vc4/vc4_hdmi.c| 25 +++ > drivers/gpu/drm/vc4/vc4_hdmi.h| 1 + > 5 files changed, 39 insertions(+), 6 deletions(-) Small note to anyone reviewing this, patches 3 & 4 where squashed into this series: https://lkml.org/lkml/2020/9/3/219 Regards, Nicolas signature.asc Description: This is a digitally signed message part
[GIT PULL 1/1] bcm2835-dt-next-2020-09-08
Hi Florian, The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5: Linux 5.9-rc1 (2020-08-16 13:04:57 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/nsaenz/linux-rpi.git tags/bcm2835-dt-next-2020-09-08 for you to fetch changes up to 4564363351e2680e55edc23c7953aebd2acb4ab7: ARM: dts: bcm2711: Enable the display pipeline (2020-09-08 18:28:23 +0200) Maxime Ripard enables vc4 on BCM2711 (RPi4), which among other things adds HDMI functionality (no 4K yet). Maxime Ripard (1): ARM: dts: bcm2711: Enable the display pipeline arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 48 + arch/arm/boot/dts/bcm2711.dtsi| 122 +- 2 files changed, 169 insertions(+), 1 deletion(-)
Re: [PATCH v5 0/9] Raspberry Pi 4 USB firmware initialization rework
Hi everyone. On Mon, 2020-06-29 at 18:18 +0200, Nicolas Saenz Julienne wrote: > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be > loaded directly from an EEPROM or, if not present, by the SoC's > co-processor, VideoCore. This series reworks how we handle this. > > The previous solution makes use of PCI quirks and exporting platform > specific functions. Albeit functional it feels pretty shoehorned. This > proposes an alternative way of handling the triggering of the xHCI chip > initialization trough means of a reset controller. > > The benefits are pretty evident: less platform churn in core xHCI code, > and no explicit device dependency management in pcie-brcmstb. > > Note that patch #1 depends on another series[1], that was just applied > into the clk maintainer's tree. > > The series is based on v5.8-rc3 > > v3: https://www.spinics.net/lists/arm-kernel/msg813612.html > v2: https://lkml.org/lkml/2020/6/9/875 > v1: > https://lore.kernel.org/linux-usb/20200608192701.18355-1-nsaenzjulie...@suse.de/T/#t > > [1] > https://lore.kernel.org/linux-clk/159304773261.62212.983376627029743...@swboyd.mtv.corp.google.com/T/#t > > --- We were waiting on a dependency to be merged upstream to get this. They are now in, so could we move things forward? I can take the device tree patches, I guess philipp can take the reset controller code. But I'm not so sure who should be taking the PCI/USB counterparts. Regards, Nicolas > > Changes since v4: > - Adress Andy's comments > > Changes since v3: > - Rework dt patch to include root bridge as a separate node > - Update xhci-pci patch now that the xhci dev has a dt node (it was >getting it in the past from its bus) > > Changes since v2: > - Add reset to resume routine in xhci-pci > - Correct of refcount in pci-quirks > - Correct typos > - Use include file to define firmware reset IDs > > Changes since v1: > - Rework reset controller so it's less USB centric > - Use correct reset controller API in xhci-pci > - Correct typos > > Nicolas Saenz Julienne (9): > dt-bindings: reset: Add a binding for the RPi Firmware reset > controller > reset: Add Raspberry Pi 4 firmware reset controller > ARM: dts: bcm2711: Add firmware usb reset node > ARM: dts: bcm2711: Add reset controller to xHCI node > usb: xhci-pci: Add support for reset controllers > Revert "USB: pci-quirks: Add Raspberry Pi 4 quirk" > usb: host: pci-quirks: Bypass xHCI quirks for Raspberry Pi 4 > Revert "firmware: raspberrypi: Introduce vl805 init routine" > Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when present" > > .../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 21 +++ > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 22 > drivers/firmware/Kconfig | 3 +- > drivers/firmware/raspberrypi.c| 61 - > drivers/pci/controller/pcie-brcmstb.c | 17 --- > drivers/reset/Kconfig | 11 ++ > drivers/reset/Makefile| 1 + > drivers/reset/reset-raspberrypi.c | 122 ++ > drivers/usb/host/pci-quirks.c | 22 ++-- > drivers/usb/host/xhci-pci.c | 10 ++ > drivers/usb/host/xhci.h | 2 + > .../reset/raspberrypi,firmware-reset.h| 13 ++ > include/soc/bcm2835/raspberrypi-firmware.h| 7 - > 13 files changed, 215 insertions(+), 97 deletions(-) > create mode 100644 drivers/reset/reset-raspberrypi.c > create mode 100644 include/dt-bindings/reset/raspberrypi,firmware-reset.h >
Re: [PATCH v3 2/2] dma-pool: Only allocate from CMA when in same memory zone
On Fri, 2020-08-14 at 08:06 +0200, Christoph Hellwig wrote: > On Sat, Aug 08, 2020 at 08:33:54AM +0200, Christoph Hellwig wrote: > > On Fri, Aug 07, 2020 at 10:50:19AM +0200, Nicolas Saenz Julienne wrote: > > > On Fri, 2020-08-07 at 07:21 +0200, Christoph Hellwig wrote: > > > > On Thu, Aug 06, 2020 at 08:47:55PM +0200, Nicolas Saenz Julienne wrote: > > > > > There is no guarantee to CMA's placement, so allocating a zone > > > > > specific > > > > > atomic pool from CMA might return memory from a completely different > > > > > memory zone. To get around this double check CMA's placement before > > > > > allocating from it. > > > > > > > > As the builtbot pointed out, memblock_start_of_DRAM can't be used from > > > > non-__init code. But lookig at it I think throwing that in > > > > is bogus anyway, as cma_get_base returns a proper physical address > > > > already. > > > > > > It does indeed, but I'm comparing CMA's base with bitmasks that don't > > > take into > > > account where the memory starts. Say memory starts at 0x8000, and CMA > > > falls > > > into ZONE_DMA [0x8000 0xC000], if you want to compare it with > > > DMA_BIT_MASK(zone_dma_bits) you're forced to unify the memory bases. > > > > > > That said, I now realize that this doesn't work for ZONE_DMA32 which has > > > a hard > > > limit on 32bit addresses reglardless of the memory base. > > > > > > That said I still need to call memblock_start_of_DRAM() any suggestions > > > WRT > > > that? I could save the value in dma_atomic_pool_init(), which is __init > > > code. > > > > The pool must be about a 32-bit physical address. The offsets can be > > different for every device.. I now see what you mean. I was trying to blindly fit CMA with arm64's DMA zone setup, which, as it turns out, doesn't really honor its purpose. arm64 introduced ZONE_DMA to provide a 30-bit address space, but we're creating it regardless of whether it exists or not. This creates a mismatch between zone_dma_bits and ZONE_DMA's real placement. I'll try to look at fixing that in arm64. > Do you plan to resend this one without the memblock_start_of_DRAM > thingy? Yes, sorry for the wait, I've been on vacation and short on time, I'll send it during the day. Regards, Nicolas
Re: [PATCH v5 0/9] Raspberry Pi 4 USB firmware initialization rework
On Fri, 2020-08-14 at 08:11 +0200, Greg KH wrote: > On Thu, Aug 13, 2020 at 12:17:49PM -0700, Florian Fainelli wrote: > > > > On 8/13/2020 3:01 AM, Nicolas Saenz Julienne wrote: > > > Hi everyone. > > > > > > On Mon, 2020-06-29 at 18:18 +0200, Nicolas Saenz Julienne wrote: > > > > On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be > > > > loaded directly from an EEPROM or, if not present, by the SoC's > > > > co-processor, VideoCore. This series reworks how we handle this. > > > > > > > > The previous solution makes use of PCI quirks and exporting platform > > > > specific functions. Albeit functional it feels pretty shoehorned. This > > > > proposes an alternative way of handling the triggering of the xHCI chip > > > > initialization trough means of a reset controller. > > > > > > > > The benefits are pretty evident: less platform churn in core xHCI code, > > > > and no explicit device dependency management in pcie-brcmstb. > > > > > > > > Note that patch #1 depends on another series[1], that was just applied > > > > into the clk maintainer's tree. > > > > > > > > The series is based on v5.8-rc3 > > > > > > > > v3: https://www.spinics.net/lists/arm-kernel/msg813612.html > > > > v2: https://lkml.org/lkml/2020/6/9/875 > > > > v1: > > > > https://lore.kernel.org/linux-usb/20200608192701.18355-1-nsaenzjulie...@suse.de/T/#t > > > > > > > > [1] > > > > https://lore.kernel.org/linux-clk/159304773261.62212.983376627029743...@swboyd.mtv.corp.google.com/T/#t > > > > > > > > --- > > > > > > We were waiting on a dependency to be merged upstream to get this. They > > > are now > > > in, so could we move things forward? > > > > > > I can take the device tree patches, I guess philipp can take the reset > > > controller code. But I'm not so sure who should be taking the PCI/USB > > > counterparts. > > > > Should we route everything through the USB tree since that is where the > > changes that do require synchronization with other subsystems and DTS is > > needed the most? > > -- > > Florian > > That's fine with me, if everyone else is ok with it :) Sounds good to me.
[PATCH v4 1/2] dma-pool: Only allocate from CMA when in same memory zone
There is no guarantee to CMA's placement, so allocating a zone specific
atomic pool from CMA might return memory from a completely different
memory zone. To get around this double check CMA's placement before
allocating from it.
Signed-off-by: Nicolas Saenz Julienne
[hch: rebased, added a fallback to the page allocator, allow dipping into
lower CMA pools]
Signed-off-by: Christoph Hellwig
---
Changes since v3:
- Do not use memblock_start_of_DRAM()
Changes since v2:
- Go back to v1 behavior
kernel/dma/pool.c | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 6bc74a2d5127..57f4a0f32a92 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -3,7 +3,9 @@
* Copyright (C) 2012 ARM Ltd.
* Copyright (C) 2020 Google LLC
*/
+#include
#include
+#include
#include
#include
#include
@@ -55,6 +57,29 @@ static void dma_atomic_pool_size_add(gfp_t gfp, size_t size)
pool_size_kernel += size;
}
+static bool cma_in_zone(gfp_t gfp)
+{
+ unsigned long size;
+ phys_addr_t end;
+ struct cma *cma;
+
+ cma = dev_get_cma_area(NULL);
+ if (!cma)
+ return false;
+
+ size = cma_get_size(cma);
+ if (!size)
+ return false;
+
+ /* CMA can't cross zone boundaries, see cma_activate_area() */
+ end = cma_get_base(cma) + size - 1;
+ if (IS_ENABLED(CONFIG_ZONE_DMA) && (gfp & GFP_DMA))
+ return end <= DMA_BIT_MASK(zone_dma_bits);
+ if (IS_ENABLED(CONFIG_ZONE_DMA32) && (gfp & GFP_DMA32))
+ return end <= DMA_BIT_MASK(32);
+ return true;
+}
+
static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size,
gfp_t gfp)
{
@@ -68,7 +93,11 @@ static int atomic_pool_expand(struct gen_pool *pool, size_t
pool_size,
do {
pool_size = 1 << (PAGE_SHIFT + order);
- page = alloc_pages(gfp, order);
+ if (cma_in_zone(gfp))
+ page = dma_alloc_from_contiguous(NULL, 1 << order,
+order, false);
+ if (!page)
+ page = alloc_pages(gfp, order);
} while (!page && order-- > 0);
if (!page)
goto out;
--
2.28.0
[PATCH v4 0/2] dma-pool fixes
Now that we have an explanation to Amir's issue, we can re-spin this series. --- Changes since v3: - Do not use memblock_start_of_DRAM() Changes since v2: - Go back to v1's behavior for patch #2 Changes since v1: - Make cma_in_zone() more strict, GFP_KERNEL doesn't default to true now - Check if phys_addr_ok() exists prior calling it Christoph Hellwig (1): dma-pool: fix coherent pool allocations for IOMMU mappings Nicolas Saenz Julienne (1): dma-pool: Only allocate from CMA when in same memory zone drivers/iommu/dma-iommu.c | 4 +- include/linux/dma-direct.h | 3 - include/linux/dma-mapping.h | 5 +- kernel/dma/direct.c | 13 +++- kernel/dma/pool.c | 145 +++- 5 files changed, 92 insertions(+), 78 deletions(-) -- 2.28.0
[PATCH v4 2/2] dma-pool: fix coherent pool allocations for IOMMU mappings
From: Christoph Hellwig
When allocating coherent pool memory for an IOMMU mapping we don't care
about the DMA mask. Move the guess for the initial GFP mask into the
dma_direct_alloc_pages and pass dma_coherent_ok as a function pointer
argument so that it doesn't get applied to the IOMMU case.
Signed-off-by: Christoph Hellwig
---
Changes since v1:
- Check if phys_addr_ok() exists prior calling it
drivers/iommu/dma-iommu.c | 4 +-
include/linux/dma-direct.h | 3 -
include/linux/dma-mapping.h | 5 +-
kernel/dma/direct.c | 13 ++--
kernel/dma/pool.c | 114 +++-
5 files changed, 62 insertions(+), 77 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 4959f5df21bd..5141d49a046b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1035,8 +1035,8 @@ static void *iommu_dma_alloc(struct device *dev, size_t
size,
if (IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
!gfpflags_allow_blocking(gfp) && !coherent)
- cpu_addr = dma_alloc_from_pool(dev, PAGE_ALIGN(size), &page,
- gfp);
+ page = dma_alloc_from_pool(dev, PAGE_ALIGN(size), &cpu_addr,
+ gfp, NULL);
else
cpu_addr = iommu_dma_alloc_pages(dev, size, &page, gfp, attrs);
if (!cpu_addr)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 5a3ce2a24794..6e87225600ae 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -73,9 +73,6 @@ static inline bool dma_capable(struct device *dev, dma_addr_t
addr, size_t size,
}
u64 dma_direct_get_required_mask(struct device *dev);
-gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
- u64 *phys_mask);
-bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size);
void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t gfp, unsigned long attrs);
void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 016b96b384bd..52635e91143b 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -522,8 +522,9 @@ void *dma_common_pages_remap(struct page **pages, size_t
size,
pgprot_t prot, const void *caller);
void dma_common_free_remap(void *cpu_addr, size_t size);
-void *dma_alloc_from_pool(struct device *dev, size_t size,
- struct page **ret_page, gfp_t flags);
+struct page *dma_alloc_from_pool(struct device *dev, size_t size,
+ void **cpu_addr, gfp_t flags,
+ bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t));
bool dma_free_from_pool(struct device *dev, void *start, size_t size);
int
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index bb0041e99659..db6ef07aec3b 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -43,7 +43,7 @@ u64 dma_direct_get_required_mask(struct device *dev)
return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
}
-gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
+static gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
u64 *phys_limit)
{
u64 dma_limit = min_not_zero(dma_mask, dev->bus_dma_limit);
@@ -68,7 +68,7 @@ gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64
dma_mask,
return 0;
}
-bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
+static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
{
return phys_to_dma_direct(dev, phys) + size - 1 <=
min_not_zero(dev->coherent_dma_mask,
dev->bus_dma_limit);
@@ -161,8 +161,13 @@ void *dma_direct_alloc_pages(struct device *dev, size_t
size,
size = PAGE_ALIGN(size);
if (dma_should_alloc_from_pool(dev, gfp, attrs)) {
- ret = dma_alloc_from_pool(dev, size, &page, gfp);
- if (!ret)
+ u64 phys_mask;
+
+ gfp |= dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
+ &phys_mask);
+ page = dma_alloc_from_pool(dev, size, &ret, gfp,
+ dma_coherent_ok);
+ if (!page)
return NULL;
goto done;
}
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 57f4a0f32a92..b0aaba4197ae 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -225,93 +225,75 @@ static int __init dma_atomic_pool_init(void)
}
postcore_initcall(dma_atomic_pool_init);
-static inline struct gen_pool *dma_guess_pool_from_device(struct device *dev)
+static inline struct gen_pool *dma_guess_pool(struct gen_pool *prev, gfp_t gfp)
{
- u64 phys_mask;
- gfp_t gfp;
-
-
Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
On Sun, 2020-10-11 at 09:47 +0200, Ard Biesheuvel wrote: > Hi Nicolas, > > $SUBJECT is out of sync with the patch below. Also, for legibility, it > helps if the commit log is intelligible by itself, rather than relying > on $SUBJECT being the first line of the first paragraph. Noted, I'll update all commit logs. > On Sat, 10 Oct 2020 at 17:12, Nicolas Saenz Julienne > wrote: > > The function provides the CPU physical address addressable by the most > > constrained bus in the system. It might be useful in order to > > dynamically set up memory zones during boot. > > > > Signed-off-by: Nicolas Saenz Julienne > > --- > > drivers/of/address.c | 34 ++ > > include/linux/of.h | 7 +++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > index eb9ab4f1e80b..755e97b65096 100644 > > --- a/drivers/of/address.c > > +++ b/drivers/of/address.c > > @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const > > struct bus_dma_region **map) > > } > > #endif /* CONFIG_HAS_DMA */ > > > > +/** > > + * of_dma_safe_phys_limit - Get system wide DMA safe address space > > + * > > + * Gets the CPU physical address limit for safe DMA addressing system wide > > by > > + * searching for the most constraining dma-range. Otherwise it returns > > ~0ULL. > > + */ > > +u64 __init of_dma_safe_phys_limit(void) > > I don't think 'safe' strikes the right tone here. You are looking for > the highest CPU address that is addressable by all DMA masters in the > system. > > Something like > > of_dma_get_max_cpu_address(void) > > perhaps? Also, since this is generic code, phys_addr_t is probably a > better type to return. Sonds good to me, I dindn't like the name I used either. Will use with phys_addr_t. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
Hi Rob,
On Mon, 2020-10-12 at 10:25 -0500, Rob Herring wrote:
> On Sat, Oct 10, 2020 at 10:12 AM Nicolas Saenz Julienne
> wrote:
> > The function provides the CPU physical address addressable by the most
> > constrained bus in the system. It might be useful in order to
> > dynamically set up memory zones during boot.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/of/address.c | 34 ++
> > include/linux/of.h | 7 +++
> > 2 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index eb9ab4f1e80b..755e97b65096 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const
> > struct bus_dma_region **map)
> > }
> > #endif /* CONFIG_HAS_DMA */
> >
> > +/**
> > + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> > + *
> > + * Gets the CPU physical address limit for safe DMA addressing system wide
> > by
> > + * searching for the most constraining dma-range. Otherwise it returns
> > ~0ULL.
> > + */
> > +u64 __init of_dma_safe_phys_limit(void)
> > +{
> > + struct device_node *np = NULL;
> > + struct of_range_parser parser;
> > + const __be32 *ranges = NULL;
> > + u64 phys_dma_limit = ~0ULL;
> > + struct of_range range;
> > + int len;
> > +
> > + for_each_of_allnodes(np) {
> > + dma_addr_t cpu_end = 0;
> > +
> > + ranges = of_get_property(np, "dma-ranges", &len);
> > + if (!ranges || !len)
> > + continue;
> > +
> > + of_dma_range_parser_init(&parser, np);
> > + for_each_of_range(&parser, &range)
> > + if (range.cpu_addr + range.size > cpu_end)
> > + cpu_end = range.cpu_addr + range.size;
>
> This doesn't work if you have more than one level of dma-ranges. The
> address has to be translated first. It should be okay to do that on
> the start or end address (if not, your DT is broken).
for_each_of_range() calls of_pci_range_parser_one() which utimately populates
range.cpu_addr with of_translate_dma_address() results. Isn't that good enough?
> Please add/extend a unittest for this.
Will do.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[PATCH v3 4/8] of: unittest: Add test for of_dma_get_max_cpu_address()
Introduce a test for of_dma_get_max_cup_address(), it uses the same DT
data as the rest of dma-ranges unit tests.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/of/unittest.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 06cc988faf78..2cbf2a585c9f 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -869,6 +869,25 @@ static void __init of_unittest_changeset(void)
#endif
}
+static void __init of_unittest_dma_get_max_cpu_address(void)
+{
+#ifdef CONFIG_HAS_DMA
+ struct device_node *np;
+ phys_addr_t cpu_addr;
+
+ np = of_find_node_by_path("/testcase-data/address-tests");
+ if (!np) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ cpu_addr = of_dma_get_max_cpu_address(np);
+ unittest(cpu_addr == 0x5000ULL,
+"of_dma_get_max_cpu_address: wrong CPU addr %pad (expecting
%llx)\n",
+&cpu_addr, 0x5000ULL);
+#endif
+}
+
static void __init of_unittest_dma_ranges_one(const char *path,
u64 expect_dma_addr, u64 expect_paddr)
{
@@ -3266,6 +3285,7 @@ static int __init of_unittest(void)
of_unittest_changeset();
of_unittest_parse_interrupts();
of_unittest_parse_interrupts_extended();
+ of_unittest_dma_get_max_cpu_address();
of_unittest_parse_dma_ranges();
of_unittest_pci_dma_ranges();
of_unittest_match_node();
--
2.28.0
[PATCH v3 8/8] mm: Update DMA zones description
The default behavior for arm64 changed, so reflect that.
Signed-off-by: Nicolas Saenz Julienne
Acked-by: Catalin Marinas
---
include/linux/mmzone.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..4ee2306351b9 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -363,8 +363,9 @@ enum zone_type {
* - arm only uses ZONE_DMA, the size, up to 4G, may vary depending on
*the specific device.
*
-* - arm64 has a fixed 1G ZONE_DMA and ZONE_DMA32 for the rest of the
-*lower 4G.
+* - arm64 uses a single 4GB ZONE_DMA, except on the Raspberry Pi 4,
+*in which ZONE_DMA covers the first GB and ZONE_DMA32 the rest of
+*the lower 4GB.
*
* - powerpc only uses ZONE_DMA, the size, up to 2G, may vary
*depending on the specific device.
--
2.28.0
[PATCH v3 5/8] dma-direct: Turn zone_dma_bits default value into a define
Set zone_dma_bits default value through a define so as for architectures to be able to override it with their default value. Signed-off-by: Nicolas Saenz Julienne --- include/linux/dma-direct.h | 3 +++ kernel/dma/direct.c| 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h index 18aade195884..e433d90cbacf 100644 --- a/include/linux/dma-direct.h +++ b/include/linux/dma-direct.h @@ -12,6 +12,9 @@ #include #include +#ifndef ZONE_DMA_BITS_DEFAULT +#define ZONE_DMA_BITS_DEFAULT 24 +#endif extern unsigned int zone_dma_bits; /* diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 06c111544f61..c0d97f536e93 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -20,7 +20,7 @@ * it for entirely different regions. In that case the arch code needs to * override the variable below for dma-direct to work properly. */ -unsigned int zone_dma_bits __ro_after_init = 24; +unsigned int zone_dma_bits __ro_after_init = ZONE_DMA_BITS_DEFAULT; static inline dma_addr_t phys_to_dma_direct(struct device *dev, phys_addr_t phys) -- 2.28.0
[PATCH v3 7/8] arm64: mm: Set ZONE_DMA size based on early IORT scan
From: Ard Biesheuvel
We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)
Instructing the DMA layer about these limitations is straight-forward,
even though we had to fix some issues regarding memory limits set in
the IORT for named components, and regarding the handling of ACPI _DMA
methods. However, the DMA layer also needs to be able to allocate
memory that is guaranteed to meet those DMA constraints, for bounce
buffering as well as allocating the backing for consistent mappings.
This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
problems with kdump, and potentially in other places where allocations
cannot cross zone boundaries. Therefore, we should avoid having two
separate DMA zones when possible.
So let's do an early scan of the IORT, and only create the ZONE_DMA
if we encounter any devices that need it. This puts the burden on
the firmware to describe such limitations in the IORT, which may be
redundant (and less precise) if _DMA methods are also being provided.
However, it should be noted that this situation is highly unusual for
arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
the _DMA method if implemented, and so we will not lose the ability to
perform streaming DMA outside the ZONE_DMA if the _DMA method permits
it.
Cc: Jeremy Linton
Cc: Lorenzo Pieralisi
Cc: Nicolas Saenz Julienne
Cc: Rob Herring
Cc: Christoph Hellwig
Cc: Robin Murphy
Cc: Hanjun Guo
Cc: Sudeep Holla
Cc: Anshuman Khandual
Signed-off-by: Ard Biesheuvel
[nsaenz: Rebased, removed documentation change, warnings and add
declaration in acpi_iort.h]
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm64/mm/init.c | 6 +
drivers/acpi/arm64/iort.c | 51 +++
include/linux/acpi_iort.h | 4 +++
3 files changed, 61 insertions(+)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 97b0d2768349..f321761eedb2 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
@@ -196,6 +197,11 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
#ifdef CONFIG_ZONE_DMA
zone_dma_bits = min(zone_dma_bits,
(unsigned
int)ilog2(of_dma_get_max_cpu_address(NULL)));
+
+ if (IS_ENABLED(CONFIG_ACPI))
+ zone_dma_bits = min(zone_dma_bits,
+ acpi_iort_get_zone_dma_size());
+
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 9929ff50c0c0..8f530bf3c03b 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1718,3 +1718,54 @@ void __init acpi_iort_init(void)
iort_init_platform_devices();
}
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+ struct acpi_table_iort *iort;
+ struct acpi_iort_node *node, *end;
+ acpi_status status;
+ u8 limit = 32;
+ int i;
+
+ if (acpi_disabled)
+ return limit;
+
+ status = acpi_get_table(ACPI_SIG_IORT, 0,
+ (struct acpi_table_header **)&iort);
+ if (ACPI_FAILURE(status))
+ return limit;
+
+ node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+ end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+ for (i = 0; i < iort->node_count; i++) {
+ if (node >= end)
+ break;
+
+ switch (node->type) {
+ struct acpi_iort_named_component *ncomp;
+ struct acpi_iort_root_complex *rc;
+
+ case ACPI_IORT_NODE_NAMED_COMPONENT:
+ ncomp = (struct acpi_iort_named_component
*)node->node_data;
+ if (ncomp->memory_address_limit)
+ limit = min(limit, ncomp->memory_address_limit);
+ break;
+
+ case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+ rc = (struct acpi_iort_root_complex *)node->node_data;
+ if (rc->memory_address_limit)
+ limit = min(limit, rc->memory_address_limit);
+ break;
+ }
+ node = ACPI_A
[PATCH v3 1/8] arm64: mm: Move reserve_crashkernel() into mem_init()
crashkernel might reserve memory located in ZONE_DMA. We plan to delay ZONE_DMA's initialization after unflattening the devicetree and ACPI's boot table initialization, so move it later in the boot process. Specifically into mem_init(), this is the last place crashkernel will be able to reserve the memory before the page allocator kicks in and there is no need to do it earlier. 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 a53c1e0fb017..1d29f2ca81c7 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -396,8 +396,6 @@ void __init arm64_memblock_init(void) else arm64_dma32_phys_limit = PHYS_MASK + 1; - reserve_crashkernel(); - reserve_elfcorehdr(); high_memory = __va(memblock_end_of_DRAM() - 1) + 1; @@ -518,6 +516,8 @@ void __init mem_init(void) else swiotlb_force = SWIOTLB_NO_FORCE; + reserve_crashkernel(); + set_max_mapnr(max_pfn - PHYS_PFN_OFFSET); #ifndef CONFIG_SPARSEMEM_VMEMMAP -- 2.28.0
[PATCH v3 0/8] arm64: Default to 32-bit wide ZONE_DMA
Using two distinct DMA zones turned out to be problematic. Here's an attempt go back to a saner default. I tested this on both a RPi4 and QEMU. --- Changes since v2: - Introduce Ard's patch - Improve OF dma-ranges parsing function - Add unit test for OF function - Address small changes - Move crashkernel reservation later in boot process Changes since v1: - Parse dma-ranges instead of using machine compatible string Ard Biesheuvel (1): arm64: mm: Set ZONE_DMA size based on early IORT scan Nicolas Saenz Julienne (7): arm64: mm: Move reserve_crashkernel() into mem_init() arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() of/address: Introduce of_dma_get_max_cpu_address() of: unittest: Add test for of_dma_get_max_cpu_address() dma-direct: Turn zone_dma_bits default value into a define arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges mm: Update DMA zones description arch/arm64/include/asm/processor.h | 1 + arch/arm64/mm/init.c | 20 ++-- drivers/acpi/arm64/iort.c | 51 ++ drivers/of/address.c | 42 drivers/of/unittest.c | 20 include/linux/acpi_iort.h | 4 +++ include/linux/dma-direct.h | 3 ++ include/linux/mmzone.h | 5 +-- include/linux/of.h | 7 kernel/dma/direct.c| 2 +- 10 files changed, 143 insertions(+), 12 deletions(-) -- 2.28.0
[PATCH v3 2/8] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
zone_dma_bits's initialization happens earlier that it's actually
needed, in arm64_memblock_init(). So move it into the more suitable
zone_sizes_init().
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm64/mm/init.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 1d29f2ca81c7..ef0ef0087e2c 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -196,6 +196,8 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
#ifdef CONFIG_ZONE_DMA
+ zone_dma_bits = ARM64_ZONE_DMA_BITS;
+ arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
#ifdef CONFIG_ZONE_DMA32
@@ -386,11 +388,6 @@ void __init arm64_memblock_init(void)
early_init_fdt_scan_reserved_mem();
- if (IS_ENABLED(CONFIG_ZONE_DMA)) {
- zone_dma_bits = ARM64_ZONE_DMA_BITS;
- arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
- }
-
if (IS_ENABLED(CONFIG_ZONE_DMA32))
arm64_dma32_phys_limit = max_zone_phys(32);
else
--
2.28.0
[PATCH v3 6/8] arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges
We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)
The DMA layer also needs to be able to allocate memory that is
guaranteed to meet those DMA constraints, for bounce buffering as well
as allocating the backing for consistent mappings. This is why the 1 GB
ZONE_DMA was introduced recently. Unfortunately, it turns out the having
a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and
potentially in other places where allocations cannot cross zone
boundaries. Therefore, we should avoid having two separate DMA zones
when possible.
So, with the help of of_dma_get_max_cpu_address() get the topmost
physical address accessible to all DMA masters in system and use that
information to fine-tune ZONE_DMA's size. In the absence of addressing
limited masters ZONE_DMA will span the whole 32-bit address space,
otherwise, in the case of the Raspberry Pi 4 it'll only span the 30-bit
address space, and have ZONE_DMA32 cover the rest of the 32-bit address
space.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v2:
- Updated commit log by shamelessly copying Ard's ACPI counterpart commit log
arch/arm64/include/asm/processor.h | 1 +
arch/arm64/mm/init.c | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/processor.h
b/arch/arm64/include/asm/processor.h
index fce8cbecd6bc..c09d3f1a9a6b 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -97,6 +97,7 @@
extern phys_addr_t arm64_dma_phys_limit;
#define ARCH_LOW_ADDRESS_LIMIT (arm64_dma_phys_limit - 1)
+#define ZONE_DMA_BITS_DEFAULT 32
struct debug_info {
#ifdef CONFIG_HAVE_HW_BREAKPOINT
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index ef0ef0087e2c..97b0d2768349 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -42,8 +42,6 @@
#include
#include
-#define ARM64_ZONE_DMA_BITS30
-
/*
* We need to be able to catch inadvertent references to memstart_addr
* that occur (potentially in generic code) before arm64_memblock_init()
@@ -196,7 +194,8 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
#ifdef CONFIG_ZONE_DMA
- zone_dma_bits = ARM64_ZONE_DMA_BITS;
+ zone_dma_bits = min(zone_dma_bits,
+ (unsigned
int)ilog2(of_dma_get_max_cpu_address(NULL)));
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
--
2.28.0
[PATCH v3 3/8] of/address: Introduce of_dma_get_max_cpu_address()
Introduce of_dma_get_max_cpu_address(), which provides the highest CPU
physical address addressable by all DMA masters in the system. It's
specially useful for setting memory zones sizes at early boot time.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v2:
- Use PHYS_ADDR_MAX
- return phys_dma_t
- Rename function
- Correct subject
- Add support to start parsing from an arbitrary device node in order
for the function to work with unit tests
drivers/of/address.c | 42 ++
include/linux/of.h | 7 +++
2 files changed, 49 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index eb9ab4f1e80b..b5a9695aaf82 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1024,6 +1024,48 @@ int of_dma_get_range(struct device_node *np, const
struct bus_dma_region **map)
}
#endif /* CONFIG_HAS_DMA */
+/**
+ * of_dma_get_max_cpu_address - Gets highest CPU address suitable for DMA
+ * @np: The node to start searching from or NULL to start from the root
+ *
+ * Gets the highest CPU physical address that is addressable by all DMA masters
+ * in the system (or subtree when np is non-NULL). If no DMA constrained device
+ * is found, it returns PHYS_ADDR_MAX.
+ */
+phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
+{
+ phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
+ struct of_range_parser parser;
+ phys_addr_t subtree_max_addr;
+ struct device_node *child;
+ phys_addr_t cpu_end = 0;
+ struct of_range range;
+ const __be32 *ranges;
+ int len;
+
+ if (!np)
+ np = of_root;
+
+ ranges = of_get_property(np, "dma-ranges", &len);
+ if (ranges && len) {
+ of_dma_range_parser_init(&parser, np);
+ for_each_of_range(&parser, &range)
+ if (range.cpu_addr + range.size > cpu_end)
+ cpu_end = range.cpu_addr + range.size;
+
+ if (max_cpu_addr > cpu_end)
+ max_cpu_addr = cpu_end;
+ }
+
+ for_each_available_child_of_node(np, child) {
+ subtree_max_addr = of_dma_get_max_cpu_address(child);
+ if (max_cpu_addr > subtree_max_addr)
+ max_cpu_addr = subtree_max_addr;
+ }
+
+ return max_cpu_addr;
+}
+
/**
* of_dma_is_coherent - Check if device is coherent
* @np:device node
diff --git a/include/linux/of.h b/include/linux/of.h
index 481ec0467285..db8db8f2c967 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out);
+phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
+
#else /* CONFIG_OF */
static inline void of_core_init(void)
@@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
return -EINVAL;
}
+static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
+{
+ return PHYS_ADDR_MAX;
+}
+
#define of_match_ptr(_ptr) NULL
#define of_match_node(_matches, _node) NULL
#endif /* CONFIG_OF */
--
2.28.0
Re: [PATCH v3 3/8] of/address: Introduce of_dma_get_max_cpu_address()
On Wed, 2020-10-14 at 17:02 -0500, Rob Herring wrote: > On Wed, Oct 14, 2020 at 2:12 PM Nicolas Saenz Julienne > wrote: > > Introduce of_dma_get_max_cpu_address(), which provides the highest CPU > > physical address addressable by all DMA masters in the system. It's > > specially useful for setting memory zones sizes at early boot time. > > > > Signed-off-by: Nicolas Saenz Julienne > > > > --- [...] > > + struct of_range_parser parser; > > + phys_addr_t subtree_max_addr; > > + struct device_node *child; > > + phys_addr_t cpu_end = 0; > > + struct of_range range; > > + const __be32 *ranges; > > + int len; > > + > > + if (!np) > > + np = of_root; > > + > > + ranges = of_get_property(np, "dma-ranges", &len); > > I'm not really following why you changed the algorithm here. You're > skipping disabled nodes which is good. Was there some other reason? Yes, it's a little more complex. But I had to change it in order to be able to start parsing down from an arbitrary device node, which is needed for the unit tests. for_each_of_allnodes() and friends will traverse the whole tree, regardless of the starting point. I couldn't find a similar function that would just iterate over a subsection of the tree, so I went with this recursive approach. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 1/8] arm64: mm: Move reserve_crashkernel() into mem_init()
On Thu, 2020-10-15 at 09:40 +0100, Will Deacon wrote: > On Wed, Oct 14, 2020 at 09:12:03PM +0200, Nicolas Saenz Julienne wrote: > > crashkernel might reserve memory located in ZONE_DMA. We plan to delay > > ZONE_DMA's initialization after unflattening the devicetree and ACPI's > > boot table initialization, so move it later in the boot process. > > Specifically into mem_init(), this is the last place crashkernel will be > > able to reserve the memory before the page allocator kicks in and there is > > no need to do it earlier. > > > > Signed-off-by: Nicolas Saenz Julienne > > --- > > arch/arm64/mm/init.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Please can you cc me on the whole series next time? I know different > maintainers have different preferences here, but I find it much easier to > figure out what's happening when I can see all of the changes together. Will do. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 3/8] of/address: Introduce of_dma_get_max_cpu_address()
On Thu, 2020-10-15 at 08:56 +0200, Ard Biesheuvel wrote:
> On Thu, 15 Oct 2020 at 00:03, Rob Herring wrote:
> > On Wed, Oct 14, 2020 at 2:12 PM Nicolas Saenz Julienne
> > wrote:
> > > Introduce of_dma_get_max_cpu_address(), which provides the highest CPU
> > > physical address addressable by all DMA masters in the system. It's
> > > specially useful for setting memory zones sizes at early boot time.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne
> > >
> > > ---
> > >
> > > Changes since v2:
> > > - Use PHYS_ADDR_MAX
> > > - return phys_dma_t
> > > - Rename function
> > > - Correct subject
> > > - Add support to start parsing from an arbitrary device node in order
> > >for the function to work with unit tests
> > >
> > > drivers/of/address.c | 42 ++
> > > include/linux/of.h | 7 +++
> > > 2 files changed, 49 insertions(+)
> > >
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > index eb9ab4f1e80b..b5a9695aaf82 100644
> > > --- a/drivers/of/address.c
> > > +++ b/drivers/of/address.c
> > > @@ -1024,6 +1024,48 @@ int of_dma_get_range(struct device_node *np, const
> > > struct bus_dma_region **map)
> > > }
> > > #endif /* CONFIG_HAS_DMA */
> > >
> > > +/**
> > > + * of_dma_get_max_cpu_address - Gets highest CPU address suitable for DMA
> > > + * @np: The node to start searching from or NULL to start from the root
> > > + *
> > > + * Gets the highest CPU physical address that is addressable by all DMA
> > > masters
> > > + * in the system (or subtree when np is non-NULL). If no DMA constrained
> > > device
> > > + * is found, it returns PHYS_ADDR_MAX.
> > > + */
> > > +phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> > > +{
> > > + phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> >
> > One issue with using phys_addr_t is it may be 32-bit even though the
> > DT is 64-bit addresses. LPAE capable system with LPAE disabled. Maybe
> > the truncation is fine here? Maybe not.
> >
>
> PHYS_ADDR_MAX is the max addressable CPU address on the system, and so
> it makes sense to use it for the return type, and for the preliminary
> return value: this is actually what /prevents/ truncation, because we
> will only overwrite max_cpu_addr if the new u64 value is lower.
>
Actually I now see how things might go south.
> > > + if (ranges && len) {
> > > + of_dma_range_parser_init(&parser, np);
> > > + for_each_of_range(&parser, &range)
> > > + if (range.cpu_addr + range.size > cpu_end)
> > > + cpu_end = range.cpu_addr + range.size;
If cpu_end hits 0x1_, it'll overflow to 0. This is possible on 32-bit
systems (LPAE or not). And something similar might happen on LPAE disabled
systems.
I could add some extra logic, something like:
/* We overflowed */
if (cpu_end < range.cpu_addr)
cpu_end = PHYS_ADDR_MAX;
Which is not perfect but will cover most sensible cases.
Or simply deal internally in u64s, and upon returning, check if "max_cpu_addr"
falls higher than PHYS_ADDR_MAX.
> > > +
> > > + if (max_cpu_addr > cpu_end)
> > > + max_cpu_addr = cpu_end;
> > > + }
> > > +
> > > + for_each_available_child_of_node(np, child) {
> > > + subtree_max_addr = of_dma_get_max_cpu_address(child);
> > > + if (max_cpu_addr > subtree_max_addr)
> > > + max_cpu_addr = subtree_max_addr;
> > > + }
> > > +
> > > + return max_cpu_addr;
> > > +}
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 4/8] of: unittest: Add test for of_dma_get_max_cpu_address()
On Wed, 2020-10-14 at 17:04 -0500, Rob Herring wrote:
> On Wed, Oct 14, 2020 at 2:12 PM Nicolas Saenz Julienne
> wrote:
> > Introduce a test for of_dma_get_max_cup_address(), it uses the same DT
> > data as the rest of dma-ranges unit tests.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/of/unittest.c | 20
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> > index 06cc988faf78..2cbf2a585c9f 100644
> > --- a/drivers/of/unittest.c
> > +++ b/drivers/of/unittest.c
> > @@ -869,6 +869,25 @@ static void __init of_unittest_changeset(void)
> > #endif
> > }
> >
> > +static void __init of_unittest_dma_get_max_cpu_address(void)
> > +{
> > +#ifdef CONFIG_HAS_DMA
>
> Can't the unittest run without this? I run the unittests under UML.
It was cargo culted from its sibling of_unittest_dma_ranges_one(), now that you
mention it, I can't seem to find the reason why it's here in the first place,
nor for other similar usages in OF code.
I ran the test in UML with all HAS_DMA conditionals removed from OF code and
things went well. I'll prepare a fix for that.
> > + struct device_node *np;
> > + phys_addr_t cpu_addr;
> > +
> > + np = of_find_node_by_path("/testcase-data/address-tests");
> > + if (!np) {
> > + pr_err("missing testcase data\n");
> > + return;
> > + }
> > +
> > + cpu_addr = of_dma_get_max_cpu_address(np);
> > + unittest(cpu_addr == 0x5000ULL,
> > +"of_dma_get_max_cpu_address: wrong CPU addr %pad
> > (expecting %llx)\n",
> > +&cpu_addr, 0x5000ULL);
> > +#endif
> > +}
> > +
> > static void __init of_unittest_dma_ranges_one(const char *path,
> > u64 expect_dma_addr, u64 expect_paddr)
> > {
> > @@ -3266,6 +3285,7 @@ static int __init of_unittest(void)
> > of_unittest_changeset();
> > of_unittest_parse_interrupts();
> > of_unittest_parse_interrupts_extended();
> > + of_unittest_dma_get_max_cpu_address();
> > of_unittest_parse_dma_ranges();
> > of_unittest_pci_dma_ranges();
> > of_unittest_match_node();
> > --
> > 2.28.0
> >
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 3/8] of/address: Introduce of_dma_get_max_cpu_address()
On Thu, 2020-10-15 at 07:42 +0200, Christoph Hellwig wrote:
> > +phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> > +{
> > + phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> > + struct of_range_parser parser;
> > + phys_addr_t subtree_max_addr;
> > + struct device_node *child;
> > + phys_addr_t cpu_end = 0;
> > + struct of_range range;
> > + const __be32 *ranges;
> > + int len;
> > +
> > + if (!np)
> > + np = of_root;
>
> Requiring of_root to be passed explicitly would seem more natural
> to me than the magic NULL argument. There doesn't seem to be any
> precedent for that kind of calling convention either.
I inspired that behavior from __of_find_all_nodes(). I'll change it.
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 5/8] dma-direct: Turn zone_dma_bits default value into a define
On Thu, 2020-10-15 at 07:38 +0200, Christoph Hellwig wrote: > On Wed, Oct 14, 2020 at 09:12:07PM +0200, Nicolas Saenz Julienne wrote: > > Set zone_dma_bits default value through a define so as for architectures > > to be able to override it with their default value. > > Architectures can do that already by assigning a value to zone_dma_bits > at runtime. I really do not want to add the extra clutter. I'll remove it then. signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 6/8] arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges
On Thu, 2020-10-15 at 07:39 +0200, Christoph Hellwig wrote: > On Wed, Oct 14, 2020 at 09:12:08PM +0200, Nicolas Saenz Julienne wrote: > > + zone_dma_bits = min(zone_dma_bits, > > + (unsigned > > int)ilog2(of_dma_get_max_cpu_address(NULL))); > > Plase avoid pointlessly long lines. Especially if it is completely trivial > by using either min_t or not overindenting like here. Noted signature.asc Description: This is a digitally signed message part
Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2
Hi Richard, your series is very welcome, upstream support for audio codecs on the RPi4 has always been lackluster. Could you provide more information on the actual products? Are there custom made hats for the RPi4 or this wired into a generic development board. On Wed, 2020-10-14 at 15:54 +0100, Richard Fitzgerald wrote: > This is based on the default bcm2711-rpi-4-b.dts. Note that you could've included bcm2711-rpi-4.dts (as if it was a .dtsi). > Configurations are provided for Cirrus Logic codecs CS42L92, CS47L15, > CS47L24, CS47L35, CS47L90 and WM8998. > > For each codec there is a sound node and a codec device node and both > default to disabled. Enable the pair for the codec in use. > > Signed-off-by: Richard Fitzgerald > --- Sadly I don't think creating a new device tree is a good solution here. If we were to do so for every RPi hat/usage it'd become unmanageable very fast. There is a way to maintain this in the open nonetheless. I suggest you build a DT overlay and submit it to https://github.com/raspberrypi/linux, see 'arch/arm/boot/dts/overlays.' The Raspberry Pi engineers have a kernel branch that tracks of the latest kernel release, so once you get the rest of patches sorted out and they are included in a release it'll make sense to do so. I can't tell for other distros, but opensuse packages overlays, so the effort will ultimately be useful to users. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[RFC] of/platform: Create device link between simple-mfd and its children
'simple-mfd' usage implies there might be some kind of resource sharing
between the parent device and its children. By creating a device link
with DL_FLAG_AUTOREMOVE_CONSUMER we make sure that at no point in time
the parent device is unbound while leaving its children unaware that
some of their resources disappeared.
Signed-off-by: Nicolas Saenz Julienne
---
Some questions:
- To what extent do we care about cleanly unbinding platform devices at
runtime? My rationale here is: "It's a platform device, for all you
know you might be unbinding someting essential to the system. So if
you're doing it, you better know what you're doing."
- Would this be an abuse of device links?
- If applying this to all simple-mfd devices is a bit too much, would
this be acceptable for a specific device setup. For example RPi4's
firmware interface (simple-mfd user) is passed to consumer drivers
trough a custom API (see rpi_firmware_get()). So, when unbound,
consumers are left with a firmware handle that points to nothing.
drivers/of/platform.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b557a0fcd4ba..8d5b55b81764 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -390,8 +390,14 @@ static int of_platform_bus_create(struct device_node *bus,
}
dev = of_platform_device_create_pdata(bus, bus_id, platform_data,
parent);
- if (!dev || !of_match_node(matches, bus))
- return 0;
+ if (!dev)
+ return 0;
+
+ if (parent && of_device_is_compatible(parent->of_node, "simple-mfd"))
+ device_link_add(&dev->dev, parent, DL_FLAG_AUTOREMOVE_CONSUMER);
+
+ if (!of_match_node(matches, bus))
+ return 0;
for_each_child_of_node(bus, child) {
pr_debug(" create child: %pOF\n", child);
--
2.28.0
Re: [PATCH 6/7] ARM: dts: Add dts for Raspberry Pi 4 + Cirrus Logic Lochnagar2
On Thu, 2020-10-15 at 12:14 +0100, Richard Fitzgerald wrote: > > Sadly I don't think creating a new device tree is a good solution here. If > > we > > were to do so for every RPi hat/usage it'd become unmanageable very fast. > > There > > is a way to maintain this in the open nonetheless. I suggest you build a DT > > overlay and submit it to https://github.com/raspberrypi/linux, see > > 'arch/arm/boot/dts/overlays.' The Raspberry Pi engineers have a kernel > > branch > > We want something in mainline so that it can be used by people > developing on mainline and taken as a starting point for configuring > the codecs for other host platforms. The RPi is a convenient platform to > use as the base because it is widely available and low-cost. If what you want to convey is the proper way of configuring your specific device the way to go is writing a devicetree binding. See Documentation/devicetree. It's even possible to validate a given devicetree against the bindings (given they are written in yaml format). Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 7/8] arm64: mm: Set ZONE_DMA size based on early IORT scan
On Thu, 2020-10-15 at 22:26 +0800, Hanjun Guo wrote: > On 2020/10/15 3:12, Nicolas Saenz Julienne wrote: > > From: Ard Biesheuvel > > > > We recently introduced a 1 GB sized ZONE_DMA to cater for platforms > > incorporating masters that can address less than 32 bits of DMA, in > > particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has > > peripherals that can only address up to 1 GB (and its PCIe host > > bridge can only access the bottom 3 GB) > > > > Instructing the DMA layer about these limitations is straight-forward, > > even though we had to fix some issues regarding memory limits set in > > the IORT for named components, and regarding the handling of ACPI _DMA > > methods. However, the DMA layer also needs to be able to allocate > > memory that is guaranteed to meet those DMA constraints, for bounce > > buffering as well as allocating the backing for consistent mappings. > > > > This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately, > > it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes > > problems with kdump, and potentially in other places where allocations > > cannot cross zone boundaries. Therefore, we should avoid having two > > separate DMA zones when possible. > > > > So let's do an early scan of the IORT, and only create the ZONE_DMA > > if we encounter any devices that need it. This puts the burden on > > the firmware to describe such limitations in the IORT, which may be > > redundant (and less precise) if _DMA methods are also being provided. > > However, it should be noted that this situation is highly unusual for > > arm64 ACPI machines. Also, the DMA subsystem still gives precedence to > > the _DMA method if implemented, and so we will not lose the ability to > > perform streaming DMA outside the ZONE_DMA if the _DMA method permits > > it. > > Sorry, I'm still a little bit confused. With this patch, if we have > a device which set the right _DMA method (DMA size >= 32), but with the > wrong DMA size in IORT, we still have the ZONE_DMA created which > is actually not needed? Yes, that would be the case. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v4 0/7] arm64: Default to 32-bit wide ZONE_DMA
On Fri, 2020-10-23 at 14:05 -0500, Jeremy Linton wrote: > Hi, > > On 10/21/20 7:34 AM, Nicolas Saenz Julienne wrote: > > Using two distinct DMA zones turned out to be problematic. Here's an > > attempt go back to a saner default. > > > > I tested this on both a RPi4 and QEMU. > > I've tested this in ACPI mode on the rpi4 (4+8G with/without the 3G > limiter) as well, with Ard's IORT patch. Nothing seems to have regressed. > > Thanks, > > Tested-by: Jeremy Linton Thanks! signature.asc Description: This is a digitally signed message part
Re: [RFC] of/platform: Create device link between simple-mfd and its children
Hi Uwe,
Sorry for the late reply, got distracted with other stuff.
On Mon, 2020-10-19 at 08:52 +0200, Uwe Kleine-König wrote:
> On Fri, Oct 16, 2020 at 05:26:56PM +0200, Nicolas Saenz Julienne wrote:
> > On Fri, 2020-10-16 at 09:38 -0500, Rob Herring wrote:
> > > On Thu, Oct 15, 2020 at 6:43 AM Nicolas Saenz Julienne
> > > wrote:
> > > > 'simple-mfd' usage implies there might be some kind of resource sharing
> > > > between the parent device and its children.
> > >
> > > It does? No! The reason behind simple-mfd was specifically because
> > > there was no parent driver or dependency on the parent. No doubt
> > > simple-mfd has been abused.
> >
> > Fair enough, so we're doing things wrong. Just for the record, I'm looking
> > at
> > RPi´s firmware interface:
> >
> > firmware: firmware {
> > compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
> > #address-cells = <1>;
> > #size-cells = <1>;
> > mboxes = <&mailbox>;
> >
> > firmware_clocks: clocks {
> > compatible = "raspberrypi,firmware-clocks";
> > #clock-cells = <1>;
> > };
> >
> > reset: reset {
> > compatible = "raspberrypi,firmware-reset";
> > #reset-cells = <1>;
> > };
> > [...]
> > };
> >
> > Note that "raspberrypi,bcm2835-firmware" has a driver, it's not just a
> > placeholder. Consumer drivers get a handle to RPi's firmware interface
> > through
> > the supplier's API, rpi_firmware_get(). The handle to firmware becomes
> > meaningless if it is unbinded, which I want to protect myself against.
> >
> > A simpler solution would be to manually create a device link between both
> > devices ("raspberrypi,bcm2835-firmware" and "raspberrypi,firmware-clocks"
> > for
> > example) upon calling rpi_firmware_get(). But I wanted to try addressing the
> > problem in a generic way first.
>
> IMHO rpi_firmware_get() should get a reference on the firmware device
> (and call try_module_get()) which prevents unbinding it.
Yes, it seems the way to go. Just one last question WRT this, since
'drv->remove(dev)' can't fail should I just block until the reference count
hits zero?
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[PATCH v4 1/7] arm64: mm: Move reserve_crashkernel() into mem_init()
crashkernel might reserve memory located in ZONE_DMA. We plan to delay ZONE_DMA's initialization after unflattening the devicetree and ACPI's boot table initialization, so move it later in the boot process. Specifically into mem_init(), this is the last place crashkernel will be able to reserve the memory before the page allocator kicks in. There isn't any apparent reason for doing this earlier. 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 095540667f0f..fc4ab0d6d5d2 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -386,8 +386,6 @@ void __init arm64_memblock_init(void) else arm64_dma32_phys_limit = PHYS_MASK + 1; - reserve_crashkernel(); - reserve_elfcorehdr(); high_memory = __va(memblock_end_of_DRAM() - 1) + 1; @@ -508,6 +506,8 @@ void __init mem_init(void) else swiotlb_force = SWIOTLB_NO_FORCE; + reserve_crashkernel(); + set_max_mapnr(max_pfn - PHYS_PFN_OFFSET); #ifndef CONFIG_SPARSEMEM_VMEMMAP -- 2.28.0
[PATCH v4 0/7] arm64: Default to 32-bit wide ZONE_DMA
Using two distinct DMA zones turned out to be problematic. Here's an attempt go back to a saner default. I tested this on both a RPi4 and QEMU. --- Changes since v3: - Drop patch adding define in dma-mapping - Address small review changes - Update Ard's patch - Add new patch removing examples from mmzone.h Changes since v2: - Introduce Ard's patch - Improve OF dma-ranges parsing function - Add unit test for OF function - Address small changes - Move crashkernel reservation later in boot process Changes since v1: - Parse dma-ranges instead of using machine compatible string Ard Biesheuvel (1): arm64: mm: Set ZONE_DMA size based on early IORT scan Nicolas Saenz Julienne (6): arm64: mm: Move reserve_crashkernel() into mem_init() arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() of/address: Introduce of_dma_get_max_cpu_address() of: unittest: Add test for of_dma_get_max_cpu_address() arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges mm: Remove examples from enum zone_type comment arch/arm64/mm/init.c | 16 ++-- drivers/acpi/arm64/iort.c | 52 +++ drivers/of/address.c | 42 +++ drivers/of/unittest.c | 18 ++ include/linux/acpi_iort.h | 4 +++ include/linux/mmzone.h| 20 --- include/linux/of.h| 7 ++ 7 files changed, 130 insertions(+), 29 deletions(-) -- 2.28.0
[PATCH v4 6/7] arm64: mm: Set ZONE_DMA size based on early IORT scan
From: Ard Biesheuvel
We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)
Instructing the DMA layer about these limitations is straight-forward,
even though we had to fix some issues regarding memory limits set in
the IORT for named components, and regarding the handling of ACPI _DMA
methods. However, the DMA layer also needs to be able to allocate
memory that is guaranteed to meet those DMA constraints, for bounce
buffering as well as allocating the backing for consistent mappings.
This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
problems with kdump, and potentially in other places where allocations
cannot cross zone boundaries. Therefore, we should avoid having two
separate DMA zones when possible.
So let's do an early scan of the IORT, and only create the ZONE_DMA
if we encounter any devices that need it. This puts the burden on
the firmware to describe such limitations in the IORT, which may be
redundant (and less precise) if _DMA methods are also being provided.
However, it should be noted that this situation is highly unusual for
arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
the _DMA method if implemented, and so we will not lose the ability to
perform streaming DMA outside the ZONE_DMA if the _DMA method permits
it.
Cc: Jeremy Linton
Cc: Lorenzo Pieralisi
Cc: Nicolas Saenz Julienne
Cc: Rob Herring
Cc: Christoph Hellwig
Cc: Robin Murphy
Cc: Hanjun Guo
Cc: Sudeep Holla
Cc: Anshuman Khandual
Signed-off-by: Ard Biesheuvel
[nsaenz: Rebased, removed documentation change and add declaration in
acpi_iort.h]
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v3:
- Use min_not_zero()
- Check ACPI revision
- Remove unnecessary #ifdef in zone_sizes_init()
arch/arm64/mm/init.c | 3 ++-
drivers/acpi/arm64/iort.c | 52 +++
include/linux/acpi_iort.h | 4 +++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 94e38f99748b..f5d4f85506e4 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
@@ -190,7 +191,7 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
#ifdef CONFIG_ZONE_DMA
dt_zone_dma_bits = ilog2(of_dma_get_max_cpu_address(NULL));
- zone_dma_bits = min(32U, dt_zone_dma_bits);
+ zone_dma_bits = min3(32U, dt_zone_dma_bits,
acpi_iort_get_zone_dma_size());
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 9929ff50c0c0..05fe4a076bab 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1718,3 +1718,55 @@ void __init acpi_iort_init(void)
iort_init_platform_devices();
}
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+ struct acpi_table_iort *iort;
+ struct acpi_iort_node *node, *end;
+ acpi_status status;
+ u8 limit = 32;
+ int i;
+
+ if (acpi_disabled)
+ return limit;
+
+ status = acpi_get_table(ACPI_SIG_IORT, 0,
+ (struct acpi_table_header **)&iort);
+ if (ACPI_FAILURE(status))
+ return limit;
+
+ node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+ end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+ for (i = 0; i < iort->node_count; i++) {
+ if (node >= end)
+ break;
+
+ switch (node->type) {
+ struct acpi_iort_named_component *ncomp;
+ struct acpi_iort_root_complex *rc;
+
+ case ACPI_IORT_NODE_NAMED_COMPONENT:
+ ncomp = (struct acpi_iort_named_component
*)node->node_data;
+ limit = min_not_zero(limit,
ncomp->memory_address_limit);
+ break;
+
+ case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+ if (node->revision < 1)
+ break;
+
+ rc = (struct acpi_iort_root_complex *)node->node_data;
+ limit = min_not_zero(limit, rc->memory_address_limit);
+ break;
+ }
+ node = ACPI_A
[PATCH v4 4/7] of: unittest: Add test for of_dma_get_max_cpu_address()
Introduce a test for of_dma_get_max_cup_address(), it uses the same DT
data as the rest of dma-ranges unit tests.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v3:
- Remove HAS_DMA guards
drivers/of/unittest.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 06cc988faf78..b9a4d047a95e 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -869,6 +869,23 @@ static void __init of_unittest_changeset(void)
#endif
}
+static void __init of_unittest_dma_get_max_cpu_address(void)
+{
+ struct device_node *np;
+ phys_addr_t cpu_addr;
+
+ np = of_find_node_by_path("/testcase-data/address-tests");
+ if (!np) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ cpu_addr = of_dma_get_max_cpu_address(np);
+ unittest(cpu_addr == 0x5000,
+"of_dma_get_max_cpu_address: wrong CPU addr %pad (expecting
%x)\n",
+&cpu_addr, 0x5000);
+}
+
static void __init of_unittest_dma_ranges_one(const char *path,
u64 expect_dma_addr, u64 expect_paddr)
{
@@ -3266,6 +3283,7 @@ static int __init of_unittest(void)
of_unittest_changeset();
of_unittest_parse_interrupts();
of_unittest_parse_interrupts_extended();
+ of_unittest_dma_get_max_cpu_address();
of_unittest_parse_dma_ranges();
of_unittest_pci_dma_ranges();
of_unittest_match_node();
--
2.28.0
[PATCH v4 7/7] mm: Remove examples from enum zone_type comment
We can't really list every setup in common code. On top of that they are
unlikely to stay true for long as things change in the arch trees
independently of this comment.
Suggested-by: Christoph Hellwig
Signed-off-by: Nicolas Saenz Julienne
---
include/linux/mmzone.h | 20
1 file changed, 20 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..9d0c454d23cd 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -354,26 +354,6 @@ enum zone_type {
* DMA mask is assumed when ZONE_DMA32 is defined. Some 64-bit
* platforms may need both zones as they support peripherals with
* different DMA addressing limitations.
-*
-* Some examples:
-*
-* - i386 and x86_64 have a fixed 16M ZONE_DMA and ZONE_DMA32 for the
-*rest of the lower 4G.
-*
-* - arm only uses ZONE_DMA, the size, up to 4G, may vary depending on
-*the specific device.
-*
-* - arm64 has a fixed 1G ZONE_DMA and ZONE_DMA32 for the rest of the
-*lower 4G.
-*
-* - powerpc only uses ZONE_DMA, the size, up to 2G, may vary
-*depending on the specific device.
-*
-* - s390 uses ZONE_DMA fixed to the lower 2G.
-*
-* - ia64 and riscv only use ZONE_DMA32.
-*
-* - parisc uses neither.
*/
#ifdef CONFIG_ZONE_DMA
ZONE_DMA,
--
2.28.0
[PATCH v4 2/7] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
zone_dma_bits's initialization happens earlier that it's actually
needed, in arm64_memblock_init(). So move it into the more suitable
zone_sizes_init().
Signed-off-by: Nicolas Saenz Julienne
---
arch/arm64/mm/init.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index fc4ab0d6d5d2..410721fc4fc0 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -190,6 +190,8 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
#ifdef CONFIG_ZONE_DMA
+ zone_dma_bits = ARM64_ZONE_DMA_BITS;
+ arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
#ifdef CONFIG_ZONE_DMA32
@@ -376,11 +378,6 @@ void __init arm64_memblock_init(void)
early_init_fdt_scan_reserved_mem();
- if (IS_ENABLED(CONFIG_ZONE_DMA)) {
- zone_dma_bits = ARM64_ZONE_DMA_BITS;
- arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
- }
-
if (IS_ENABLED(CONFIG_ZONE_DMA32))
arm64_dma32_phys_limit = max_zone_phys(32);
else
--
2.28.0
[PATCH v4 3/7] of/address: Introduce of_dma_get_max_cpu_address()
Introduce of_dma_get_max_cpu_address(), which provides the highest CPU
physical address addressable by all DMA masters in the system. It's
specially useful for setting memory zones sizes at early boot time.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v3:
- use u64 with cpu_end
Changes since v2:
- Use PHYS_ADDR_MAX
- return phys_dma_t
- Rename function
- Correct subject
- Add support to start parsing from an arbitrary device node in order
for the function to work with unit tests
drivers/of/address.c | 42 ++
include/linux/of.h | 7 +++
2 files changed, 49 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index eb9ab4f1e80b..47dfe5881e18 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1024,6 +1024,48 @@ int of_dma_get_range(struct device_node *np, const
struct bus_dma_region **map)
}
#endif /* CONFIG_HAS_DMA */
+/**
+ * of_dma_get_max_cpu_address - Gets highest CPU address suitable for DMA
+ * @np: The node to start searching from or NULL to start from the root
+ *
+ * Gets the highest CPU physical address that is addressable by all DMA masters
+ * in the sub-tree pointed by np, or the whole tree if NULL is passed. If no
+ * DMA constrained device is found, it returns PHYS_ADDR_MAX.
+ */
+phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
+{
+ phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
+ struct of_range_parser parser;
+ phys_addr_t subtree_max_addr;
+ struct device_node *child;
+ struct of_range range;
+ const __be32 *ranges;
+ u64 cpu_end = 0;
+ int len;
+
+ if (!np)
+ np = of_root;
+
+ ranges = of_get_property(np, "dma-ranges", &len);
+ if (ranges && len) {
+ of_dma_range_parser_init(&parser, np);
+ for_each_of_range(&parser, &range)
+ if (range.cpu_addr + range.size > cpu_end)
+ cpu_end = range.cpu_addr + range.size;
+
+ if (max_cpu_addr > cpu_end)
+ max_cpu_addr = cpu_end;
+ }
+
+ for_each_available_child_of_node(np, child) {
+ subtree_max_addr = of_dma_get_max_cpu_address(child);
+ if (max_cpu_addr > subtree_max_addr)
+ max_cpu_addr = subtree_max_addr;
+ }
+
+ return max_cpu_addr;
+}
+
/**
* of_dma_is_coherent - Check if device is coherent
* @np:device node
diff --git a/include/linux/of.h b/include/linux/of.h
index 481ec0467285..db8db8f2c967 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out);
+phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
+
#else /* CONFIG_OF */
static inline void of_core_init(void)
@@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
return -EINVAL;
}
+static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
+{
+ return PHYS_ADDR_MAX;
+}
+
#define of_match_ptr(_ptr) NULL
#define of_match_node(_matches, _node) NULL
#endif /* CONFIG_OF */
--
2.28.0
[PATCH v4 5/7] arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges
We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)
The DMA layer also needs to be able to allocate memory that is
guaranteed to meet those DMA constraints, for bounce buffering as well
as allocating the backing for consistent mappings. This is why the 1 GB
ZONE_DMA was introduced recently. Unfortunately, it turns out the having
a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and
potentially in other places where allocations cannot cross zone
boundaries. Therefore, we should avoid having two separate DMA zones
when possible.
So, with the help of of_dma_get_max_cpu_address() get the topmost
physical address accessible to all DMA masters in system and use that
information to fine-tune ZONE_DMA's size. In the absence of addressing
limited masters ZONE_DMA will span the whole 32-bit address space,
otherwise, in the case of the Raspberry Pi 4 it'll only span the 30-bit
address space, and have ZONE_DMA32 cover the rest of the 32-bit address
space.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v3:
- Simplify code for readability.
Changes since v2:
- Updated commit log by shamelessly copying Ard's ACPI commit log
arch/arm64/mm/init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 410721fc4fc0..94e38f99748b 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -42,8 +42,6 @@
#include
#include
-#define ARM64_ZONE_DMA_BITS30
-
/*
* We need to be able to catch inadvertent references to memstart_addr
* that occur (potentially in generic code) before arm64_memblock_init()
@@ -188,9 +186,11 @@ static phys_addr_t __init max_zone_phys(unsigned int
zone_bits)
static void __init zone_sizes_init(unsigned long min, unsigned long max)
{
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
+ unsigned int __maybe_unused dt_zone_dma_bits;
#ifdef CONFIG_ZONE_DMA
- zone_dma_bits = ARM64_ZONE_DMA_BITS;
+ dt_zone_dma_bits = ilog2(of_dma_get_max_cpu_address(NULL));
+ zone_dma_bits = min(32U, dt_zone_dma_bits);
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
--
2.28.0
Re: [PATCH v4 3/7] of/address: Introduce of_dma_get_max_cpu_address()
On Thu, 2020-10-22 at 14:23 +0200, Ard Biesheuvel wrote:
> > +/**
> > + * of_dma_get_max_cpu_address - Gets highest CPU address suitable for DMA
> > + * @np: The node to start searching from or NULL to start from the root
> > + *
> > + * Gets the highest CPU physical address that is addressable by all DMA
> > masters
> > + * in the sub-tree pointed by np, or the whole tree if NULL is passed. If
> > no
> > + * DMA constrained device is found, it returns PHYS_ADDR_MAX.
> > + */
> > +phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
> > +{
> > + phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
> > + struct of_range_parser parser;
> > + phys_addr_t subtree_max_addr;
> > + struct device_node *child;
> > + struct of_range range;
> > + const __be32 *ranges;
> > + u64 cpu_end = 0;
> > + int len;
> > +
> > + if (!np)
> > + np = of_root;
> > +
> > + ranges = of_get_property(np, "dma-ranges", &len);
> > + if (ranges && len) {
> > + of_dma_range_parser_init(&parser, np);
> > + for_each_of_range(&parser, &range)
> > + if (range.cpu_addr + range.size > cpu_end)
> > + cpu_end = range.cpu_addr + range.size;
>
>
> Shouldn't this be 'range.cpu_addr + range.size - 1' ?
Yes, I agree. In that case arm64's counterpart should be:
zone_dma_bits = max(32U, fls64(of_dma_get_max_cpu_address(NULL)));
I'll update it.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
[PATCH v2 02/10] clk: bcm: rpi: Release firmware handle on unbind
Upon unbinding the clock device make sure we release RPi's firmware interface. Signed-off-by: Nicolas Saenz Julienne --- drivers/clk/bcm/clk-raspberrypi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index f89b9cfc4309..845510ff7514 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -353,6 +353,7 @@ static int raspberrypi_clk_remove(struct platform_device *pdev) struct raspberrypi_clk *rpi = platform_get_drvdata(pdev); platform_device_unregister(rpi->cpufreq); + rpi_firmware_put(rpi->firmware); return 0; } -- 2.28.0
[PATCH v2 03/10] gpio: raspberrypi-exp: Release firmware handle on unbind
Upon unbinding the device make sure we release RPi's firmware interface.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/gpio/gpio-raspberrypi-exp.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-raspberrypi-exp.c
b/drivers/gpio/gpio-raspberrypi-exp.c
index bb100e0124e6..c008336e1131 100644
--- a/drivers/gpio/gpio-raspberrypi-exp.c
+++ b/drivers/gpio/gpio-raspberrypi-exp.c
@@ -231,8 +231,19 @@ static int rpi_exp_gpio_probe(struct platform_device *pdev)
rpi_gpio->gc.get = rpi_exp_gpio_get;
rpi_gpio->gc.set = rpi_exp_gpio_set;
rpi_gpio->gc.can_sleep = true;
+ platform_set_drvdata(pdev, rpi_gpio);
- return devm_gpiochip_add_data(dev, &rpi_gpio->gc, rpi_gpio);
+ return gpiochip_add_data(&rpi_gpio->gc, rpi_gpio);
+}
+
+static int rpi_exp_gpio_remove(struct platform_device *pdev)
+{
+ struct rpi_exp_gpio *rpi_gpio = platform_get_drvdata(pdev);
+
+ gpiochip_remove(&rpi_gpio->gc);
+ rpi_firmware_put(rpi_gpio->fw);
+
+ return 0;
}
static const struct of_device_id rpi_exp_gpio_ids[] = {
@@ -247,6 +258,7 @@ static struct platform_driver rpi_exp_gpio_driver = {
.of_match_table = of_match_ptr(rpi_exp_gpio_ids),
},
.probe = rpi_exp_gpio_probe,
+ .remove = rpi_exp_gpio_remove,
};
module_platform_driver(rpi_exp_gpio_driver);
--
2.28.0
[PATCH v2 00/10] 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 imporant 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 introduces reference counting in the firwmware interface driver and patches #2 to #7 update all firmware users. Patches #8 to #10 introduce the new PWM driver. I sent everything as a single series as the final version of the PWM drivers depends on the firwmare 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 v1: - Address PWM driver changes - Fix binding, now with 2 cells - Add reference count to rpi_firmware_get() Nicolas Saenz Julienne (10): firmware: raspberrypi: Introduce rpi_firmware_put() 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 | 1 + drivers/firmware/raspberrypi.c| 30 ++- drivers/gpio/gpio-raspberrypi-exp.c | 14 +- drivers/input/touchscreen/raspberrypi-ts.c| 1 + drivers/pwm/Kconfig | 9 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-raspberrypi.c | 221 ++ drivers/reset/reset-raspberrypi.c | 13 +- drivers/soc/bcm/raspberrypi-power.c | 15 ++ .../interface/vchiq_arm/vchiq_arm.c | 3 + .../pwm/raspberrypi,firmware-pwm.h| 13 ++ include/soc/bcm2835/raspberrypi-firmware.h| 3 + 14 files changed, 395 insertions(+), 3 deletions(-) create mode 100644 drivers/pwm/pwm-raspberrypi.c create mode 100644 include/dt-bindings/pwm/raspberrypi,firmware-pwm.h -- 2.28.0
[PATCH v2 10/10] 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 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
- Add comment explaining weird Kconfig construct
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-raspberrypi.c | 221 ++
3 files changed, 231 insertions(+)
create mode 100644 drivers/pwm/pwm-raspberrypi.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 63be5362fd3a..b0ffb1e690c0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -379,6 +379,15 @@ config PWM_PXA
To compile this driver as a module, choose M here: the module
will be called pwm-pxa.
+config PWM_RASPBERRYPI
+ tristate "Raspberry Pi Firwmware 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 cbdcd55d69ee..b557b549d9f3 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -35,6 +35,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) += pwm-raspberrypi.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.c b/drivers/pwm/pwm-raspberrypi.c
new file mode 100644
index ..72dc0fc5a206
--- /dev/null
+++ b/drivers/pwm/pwm-raspberrypi.c
@@ -0,0 +1,221 @@
+// 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
+#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 *to_raspberrypi_pwm(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,
+ &msg, sizeof(msg));
+ if (ret)
+ return ret;
+ else 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,
+ &msg, sizeof(msg));
+ if (ret)
+ return ret;
+ else 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)
+{
+ struct raspberrypi_pwm *rpipwm = to_raspberrypi_pwm(chip);
+
+ state->period = RPI_PWM_PERIOD_NS;
+ state->duty_cycle = DIV_ROUND_CLOSEST(rpipwm->duty_cycle *
RPI_PWM_PERIOD_NS,
+ RPI_PWM_MAX
[PATCH v2 08/10] 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
---
Changes since v1:
- Update bindings to use 2 #pwm-cells
.../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 20 +++
.../pwm/raspberrypi,firmware-pwm.h| 13
2 files changed, 33 insertions(+)
create mode 100644 include/dt-bindings/pwm/raspberrypi,firmware-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..8029ce958c48 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-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-pwm";
+#pwm-cells = <1>;
+};
};
...
diff --git a/include/dt-bindings/pwm/raspberrypi,firmware-pwm.h
b/include/dt-bindings/pwm/raspberrypi,firmware-pwm.h
new file mode 100644
index ..27c5ce68847b
--- /dev/null
+++ b/include/dt-bindings/pwm/raspberrypi,firmware-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.28.0
[PATCH v2 04/10] reset: raspberrypi: Release firmware handle on unbind
Upon unbinding the device make sure we release RPi's firmware interface.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/reset/reset-raspberrypi.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/reset/reset-raspberrypi.c
b/drivers/reset/reset-raspberrypi.c
index 02f59c06f69b..29311192e2c9 100644
--- a/drivers/reset/reset-raspberrypi.c
+++ b/drivers/reset/reset-raspberrypi.c
@@ -99,7 +99,17 @@ static int rpi_reset_probe(struct platform_device *pdev)
priv->rcdev.ops = &rpi_reset_ops;
priv->rcdev.of_node = dev->of_node;
- return devm_reset_controller_register(dev, &priv->rcdev);
+ return reset_controller_register(&priv->rcdev);
+}
+
+static int rpi_reset_remove(struct platform_device *pdev)
+{
+ struct rpi_reset *priv = platform_get_drvdata(pdev);
+
+ reset_controller_unregister(&priv->rcdev);
+ rpi_firmware_put(priv->fw);
+
+ return 0;
}
static const struct of_device_id rpi_reset_of_match[] = {
@@ -110,6 +120,7 @@ MODULE_DEVICE_TABLE(of, rpi_reset_of_match);
static struct platform_driver rpi_reset_driver = {
.probe = rpi_reset_probe,
+ .remove = rpi_reset_remove,
.driver = {
.name = "raspberrypi-reset",
.of_match_table = rpi_reset_of_match,
--
2.28.0
[PATCH v2 05/10] soc: bcm: raspberrypi-power: Release firmware handle on unbind
Upon unbinding the device make sure we release RPi's firmware interface.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/soc/bcm/raspberrypi-power.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/soc/bcm/raspberrypi-power.c
b/drivers/soc/bcm/raspberrypi-power.c
index 5d1aacdd84ef..a0b38db5886c 100644
--- a/drivers/soc/bcm/raspberrypi-power.c
+++ b/drivers/soc/bcm/raspberrypi-power.c
@@ -225,6 +225,20 @@ static int rpi_power_probe(struct platform_device *pdev)
return 0;
}
+static int rpi_power_remove(struct platform_device *pdev)
+{
+ struct rpi_power_domains *rpi_domains = platform_get_drvdata(pdev);
+
+ of_genpd_del_provider(dev->of_node);
+
+ for (i = 0; i < RPI_POWER_DOMAIN_COUNT; i++)
+ pm_genpd_remove(&rpi_domains->domains[i].base);
+
+ rpi_firmware_put(rpi_domaina->fw);
+
+ return 0;
+}
+
static const struct of_device_id rpi_power_of_match[] = {
{ .compatible = "raspberrypi,bcm2835-power", },
{},
@@ -237,6 +251,7 @@ static struct platform_driver rpi_power_driver = {
.of_match_table = rpi_power_of_match,
},
.probe = rpi_power_probe,
+ .remove = rpi_power_remove,
};
builtin_platform_driver(rpi_power_driver);
--
2.28.0
[PATCH v2 07/10] input: raspberrypi-ts: Release firmware handle when not needed
After passing the DMA buffer address through the firmware interface,
release the firmware handle, we won't need it anymore.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/input/touchscreen/raspberrypi-ts.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/touchscreen/raspberrypi-ts.c
b/drivers/input/touchscreen/raspberrypi-ts.c
index ef6aaed217cf..29c878a00018 100644
--- a/drivers/input/touchscreen/raspberrypi-ts.c
+++ b/drivers/input/touchscreen/raspberrypi-ts.c
@@ -165,6 +165,7 @@ static int rpi_ts_probe(struct platform_device *pdev)
dev_warn(dev, "Failed to set touchbuf, %d\n", error);
return error;
}
+ rpi_firmware_put(fw);
input = devm_input_allocate_device(dev);
if (!input) {
--
2.28.0
[PATCH v2 01/10] firmware: raspberrypi: Introduce rpi_firmware_put()
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 make sure they all finished
unbinding before we do.
Suggested-by: Uwe Kleine-König
Signed-off-by: Nicolas Saenz Julienne
---
@Uwe: I didn't found it necessary to call 'try_module_get()' as the rest
of modules depend on the 'rpi_firmware_get/put()' symbols which already
block users from unloading the module prematurely.
drivers/firmware/raspberrypi.c | 30 +-
include/soc/bcm2835/raspberrypi-firmware.h | 3 +++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 2371d08bdd17..e5ba609e3985 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -11,7 +11,9 @@
#include
#include
#include
+#include
#include
+#include
#include
#define MBOX_MSG(chan, data28) (((data28) & ~0xf) | ((chan) & 0xf))
@@ -27,6 +29,9 @@ struct rpi_firmware {
struct mbox_chan *chan; /* The property channel. */
struct completion c;
u32 enabled;
+
+ refcount_t consumers;
+ wait_queue_head_t wait;
};
static DEFINE_MUTEX(transaction_lock);
@@ -247,6 +252,8 @@ static int rpi_firmware_probe(struct platform_device *pdev)
}
init_completion(&fw->c);
+ refcount_set(&fw->consumers, 1);
+ init_waitqueue_head(&fw->wait);
platform_set_drvdata(pdev, fw);
@@ -275,6 +282,8 @@ static int rpi_firmware_remove(struct platform_device *pdev)
rpi_hwmon = NULL;
platform_device_unregister(rpi_clk);
rpi_clk = NULL;
+
+ wait_event(fw->wait, refcount_dec_if_one(&fw->consumers));
mbox_free_channel(fw->chan);
return 0;
@@ -289,14 +298,33 @@ static int rpi_firmware_remove(struct platform_device
*pdev)
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 (!refcount_inc_not_zero(&fw->consumers))
+ return NULL;
+
+ return fw;
}
EXPORT_SYMBOL_GPL(rpi_firmware_get);
+/**
+ * rpi_firmware_put - Put pointer to rpi_firmware structure.
+ * @rpi_firmware:Pointer to struct rpi_firmware
+ */
+void rpi_firmware_put(struct rpi_firmware *fw)
+{
+ refcount_dec(&fw->consumers);
+ wake_up(&fw->wait);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_put);
+
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 cc9cdbc66403..7836ea51fbdf 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -141,6 +141,7 @@ int rpi_firmware_property(struct rpi_firmware *fw,
int rpi_firmware_property_list(struct rpi_firmware *fw,
void *data, size_t tag_size);
struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
+void rpi_firmware_put(struct rpi_firmware *fw);
#else
static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
void *data, size_t len)
@@ -158,6 +159,8 @@ static inline struct rpi_firmware *rpi_firmware_get(struct
device_node *firmware
{
return NULL;
}
+
+void rpi_firmware_put(struct rpi_firmware *fw) { }
#endif
#endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
--
2.28.0
[PATCH v2 06/10] staging: vchiq: Release firmware handle on unbind
Upon unbinding the device make sure we release RPi's firmware interface.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +++
1 file changed, 3 insertions(+)
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 01125d9f991b..dfa4d144faa8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -2771,11 +2771,14 @@ static int vchiq_probe(struct platform_device *pdev)
static int vchiq_remove(struct platform_device *pdev)
{
+ struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
+
platform_device_unregister(bcm2835_audio);
platform_device_unregister(bcm2835_camera);
vchiq_debugfs_deinit();
device_destroy(vchiq_class, vchiq_devid);
cdev_del(&vchiq_cdev);
+ rpi_firmware_put(drvdata->fw);
return 0;
}
--
2.28.0
[PATCH v2 09/10] 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
---
Changes since v1:
- Update patch to use 2 pwm cells
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 09a1182c2936..361db82619a4 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";
@@ -68,6 +69,54 @@ sd_vcc_reg: sd_vcc_reg {
enable-active-high;
gpio = <&expgpio 6 GPIO_ACTIVE_HIGH>;
};
+
+ fan: pwm-fan {
+ compatible = "pwm-fan";
+ cooling-levels = <0 50 150 255>;
+ #cooling-cells = <2>;
+ pwms = <&fwpwm 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 = <&threshold>;
+ cooling-device = <&fan 0 1>;
+ };
+
+ map1 {
+ trip = <&target>;
+ cooling-device = <&fan 1 2>;
+ };
+
+ map2 {
+ trip = <&cpu_hot>;
+ cooling-device = <&fan 2 3>;
+ };
+ };
+ };
+ };
};
&ddc0 {
@@ -103,6 +152,11 @@ reset: reset {
compatible = "raspberrypi,firmware-reset";
#reset-cells = <1>;
};
+
+ fwpwm: pwm {
+ compatible = "raspberrypi,firmware-pwm";
+ #pwm-cells = <2>;
+ };
};
&gpio {
--
2.28.0
Re: [PATCH v4 5/7] arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges
Hi Catalin,
On Thu, 2020-10-22 at 19:06 +0100, Catalin Marinas wrote:
> On Wed, Oct 21, 2020 at 02:34:35PM +0200, Nicolas Saenz Julienne wrote:
> > @@ -188,9 +186,11 @@ static phys_addr_t __init max_zone_phys(unsigned int
> > zone_bits)
> > static void __init zone_sizes_init(unsigned long min, unsigned long max)
> > {
> > unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
> > + unsigned int __maybe_unused dt_zone_dma_bits;
> >
> > #ifdef CONFIG_ZONE_DMA
> > - zone_dma_bits = ARM64_ZONE_DMA_BITS;
> > + dt_zone_dma_bits = ilog2(of_dma_get_max_cpu_address(NULL));
> > + zone_dma_bits = min(32U, dt_zone_dma_bits);
>
> A thought: can we remove the min here and expand ZONE_DMA to whatever
> dt_zone_dma_bits says? More on this below.
On most platforms we'd get PHYS_ADDR_MAX, or something bigger than the actual
amount of RAM. Which would ultimately create a system wide ZONE_DMA. At first
sight, I don't see it breaking dma-direct in any way.
On the other hand, there is a big amount of MMIO devices out there that can
only handle 32-bit addressing. Be it PCI cards or actual IP cores. To make
things worse, this limitation is often expressed in the driver, not FW (with
dma_set_mask() and friends). If those devices aren't behind an IOMMU we have be
able to provide at least 32-bit addressable memory. See this comment from
dma_direct_supported():
/*
* Because 32-bit DMA masks are so common we expect every architecture
* to be able to satisfy them - either by not supporting more physical
* memory, or by providing a ZONE_DMA32. If neither is the case, the
* architecture needs to use an IOMMU instead of the direct mapping.
*/
I think, for the common case, we're stuck with at least one zone spanning the
32-bit address space.
> > arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
> > max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> > #endif
>
> I was talking earlier to Ard and Robin on the ZONE_DMA32 history and the
> need for max_zone_phys(). This was rather theoretical, the Seattle
> platform has all RAM starting above 4GB and that led to an empty
> ZONE_DMA32 originally. The max_zone_phys() hack was meant to lift
> ZONE_DMA32 into the bottom of the RAM, on the assumption that such
> 32-bit devices would have a DMA offset hardwired. We are not aware of
> any such case on arm64 systems and even on Seattle, IIUC 32-bit devices
> only work if they are behind an SMMU (so no hardwired offset).
>
> In hindsight, it would have made more sense on platforms with RAM above
> 4GB to expand ZONE_DMA32 to cover the whole memory (so empty
> ZONE_NORMAL). Something like:
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index a53c1e0fb017..7d5e3dd85617 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -187,8 +187,12 @@ static void __init reserve_elfcorehdr(void)
> */
> static phys_addr_t __init max_zone_phys(unsigned int zone_bits)
> {
> - phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63,
> zone_bits);
> - return min(offset + (1ULL << zone_bits), memblock_end_of_DRAM());
> + phys_addr_t zone_mask = 1ULL << zone_bits;
> +
> + if (!(memblock_start_of_DRAM() & zone_mask))
> + zone_mask = PHYS_ADDR_MAX;
> +
> + return min(zone_mask, memblock_end_of_DRAM());
> }
>
> static void __init zone_sizes_init(unsigned long min, unsigned long max)
>
> I don't think this makes any difference for ZONE_DMA unless a
> broken DT or IORT reports the max CPU address below the start of DRAM.
>
> There's a minor issue if of_dma_get_max_cpu_address() matches
> memblock_end_of_DRAM() but they are not a power of 2. We'd be left with
> a bit of RAM at the end in ZONE_NORMAL due to ilog2 truncation.
I agree it makes no sense to create more than one zone when the beginning of
RAM is located above the 32-bit address space. I'm all for disregarding the
possibility of hardwired offsets. As a bonus, as we already discussed some time
ago, this is something that never played well with current dma-direct code[1].
Regards,
Nicolas
[1] https://lkml.org/lkml/2020/9/8/377
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 10/10] pwm: Add Raspberry Pi Firmware based PWM bus
Hi Andy, thanks for the review!
On Thu, 2020-10-22 at 21:53 +0300, Andy Shevchenko wrote:
> On Thu, Oct 22, 2020 at 9:05 PM 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.
> > drivers/pwm/pwm-raspberrypi.c | 221 ++
>
> Name is completely confusing.
> Please, make it unique enough to understand that this is exactly the
> device it serves for.
>
> For example, pwm-rpi-poe is better.
Sounds reasonable, I'll change that.
>
> ...
>
> > + * - Only normal polarity
>
> Can't it be emulated? Isn't it 100% - duty cycle % ?
I guess it can, OTOH given the rather specific use case, I doubt it'll be
worth the effort.
> > +#include
> > +#include
> > +#include
> > +#include
>
> ...
>
> > + ret = rpi_firmware_property(firmware, RPI_FIRMWARE_SET_POE_HAT_VAL,
> > + &msg, sizeof(msg));
> > + if (ret)
> > + return ret;
> > + else if (msg.ret)
>
> Redundant 'else'
Noted.
> > + firmware_node = of_get_parent(dev->of_node);
> > + if (!firmware_node) {
> > + dev_err(dev, "Missing firmware node\n");
> > + return -ENOENT;
> > + }
> > +
> > + firmware = rpi_firmware_get(firmware_node);
> > + of_node_put(firmware_node);
> > + if (!firmware)
> > + return -EPROBE_DEFER;
>
> Looks like a hack.
This is the pattern we've been using on all firmware dependent devices so far.
Feel free to suggest a better way, I'll be happy to look into it.
>
> ...
>
> > + ret = pwmchip_remove(&rpipwm->chip);
> > + if (!ret)
> > + rpi_firmware_put(rpipwm->firmware);
> > +
> > + return ret;
>
> Can't you use the usual pattern?
Yes of course. Don't know why I went this way.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 05/10] soc: bcm: raspberrypi-power: Release firmware handle on unbind
On Thu, 2020-10-22 at 17:58 +0200, Nicolas Saenz Julienne wrote:
> Upon unbinding the device make sure we release RPi's firmware interface.
>
> Signed-off-by: Nicolas Saenz Julienne
> ---
> drivers/soc/bcm/raspberrypi-power.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/soc/bcm/raspberrypi-power.c
> b/drivers/soc/bcm/raspberrypi-power.c
> index 5d1aacdd84ef..a0b38db5886c 100644
> --- a/drivers/soc/bcm/raspberrypi-power.c
> +++ b/drivers/soc/bcm/raspberrypi-power.c
> @@ -225,6 +225,20 @@ static int rpi_power_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int rpi_power_remove(struct platform_device *pdev)
> +{
> + struct rpi_power_domains *rpi_domains = platform_get_drvdata(pdev);
> +
> + of_genpd_del_provider(dev->of_node);
> +
> + for (i = 0; i < RPI_POWER_DOMAIN_COUNT; i++)
> + pm_genpd_remove(&rpi_domains->domains[i].base);
> +
> + rpi_firmware_put(rpi_domaina->fw);
I Just realised I failed to squash a fix for this patch, so this will not
build. Sorry for that.
Regards,
Nicolas
> +
> + return 0;
> +}
> +
> static const struct of_device_id rpi_power_of_match[] = {
> { .compatible = "raspberrypi,bcm2835-power", },
> {},
> @@ -237,6 +251,7 @@ static struct platform_driver rpi_power_driver = {
> .of_match_table = rpi_power_of_match,
> },
> .probe = rpi_power_probe,
> + .remove = rpi_power_remove,
> };
> builtin_platform_driver(rpi_power_driver);
>
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 03/10] gpio: raspberrypi-exp: Release firmware handle on unbind
On Mon, 2020-10-26 at 15:40 +0100, Bartosz Golaszewski wrote:
> On Thu, Oct 22, 2020 at 5:59 PM Nicolas Saenz Julienne
> wrote:
> > Upon unbinding the device make sure we release RPi's firmware interface.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/gpio/gpio-raspberrypi-exp.c | 14 +-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-raspberrypi-exp.c
> > b/drivers/gpio/gpio-raspberrypi-exp.c
> > index bb100e0124e6..c008336e1131 100644
> > --- a/drivers/gpio/gpio-raspberrypi-exp.c
> > +++ b/drivers/gpio/gpio-raspberrypi-exp.c
> > @@ -231,8 +231,19 @@ static int rpi_exp_gpio_probe(struct platform_device
> > *pdev)
> > rpi_gpio->gc.get = rpi_exp_gpio_get;
> > rpi_gpio->gc.set = rpi_exp_gpio_set;
> > rpi_gpio->gc.can_sleep = true;
> > + platform_set_drvdata(pdev, rpi_gpio);
> >
> > - return devm_gpiochip_add_data(dev, &rpi_gpio->gc, rpi_gpio);
> > + return gpiochip_add_data(&rpi_gpio->gc, rpi_gpio);
> > +}
> > +
> > +static int rpi_exp_gpio_remove(struct platform_device *pdev)
> > +{
> > + struct rpi_exp_gpio *rpi_gpio = platform_get_drvdata(pdev);
> > +
> > + gpiochip_remove(&rpi_gpio->gc);
> > + rpi_firmware_put(rpi_gpio->fw);
> > +
> > + return 0;
> > }
> >
> > static const struct of_device_id rpi_exp_gpio_ids[] = {
> > @@ -247,6 +258,7 @@ static struct platform_driver rpi_exp_gpio_driver = {
> > .of_match_table = of_match_ptr(rpi_exp_gpio_ids),
> > },
> > .probe = rpi_exp_gpio_probe,
> > + .remove = rpi_exp_gpio_remove,
> > };
> > module_platform_driver(rpi_exp_gpio_driver);
> >
> > --
> > 2.28.0
> >
>
> Why not introduce devm_rpi_firmware_get()? That would allow you to
> keep the driver elegant without re-adding remove().
I like the idea, I'll look into it.
Thanks,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 01/10] firmware: raspberrypi: Introduce rpi_firmware_put()
On Thu, 2020-10-22 at 21:46 +0300, Andy Shevchenko wrote: > On Thu, Oct 22, 2020 at 9:06 PM Nicolas Saenz Julienne > wrote: > > 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 make sure they all finished > > unbinding before we do. > > Wait, if it's a device, why do we need all these? > get_device() / put_device() along with module_get() / module_put() > should be sufficient, no? Could you expand here a little, I do see how I could use get_device()'s reference count. But it seems to me I'd be digging way too deep into kobj in order to get the functionality I need. If you meant to say that it should be automatically taken care by the platform bus, just FYI we're using 'simple-mfd' in DT. Where firmware supplier is the parent and all consumers are children. Regards, Nicolas signature.asc Description: This is a digitally signed message part
[PATCH v5 0/7] arm64: Default to 32-bit wide ZONE_DMA
Using two distinct DMA zones turned out to be problematic. Here's an attempt go back to a saner default. I tested this on both a RPi4 and QEMU. --- Changes since v4: - Fix of_dma_get_max_cpu_address() so it returns the last addressable addres, not the limit Changes since v3: - Drop patch adding define in dma-mapping - Address small review changes - Update Ard's patch - Add new patch removing examples from mmzone.h Changes since v2: - Introduce Ard's patch - Improve OF dma-ranges parsing function - Add unit test for OF function - Address small changes - Move crashkernel reservation later in boot process Changes since v1: - Parse dma-ranges instead of using machine compatible string Ard Biesheuvel (1): arm64: mm: Set ZONE_DMA size based on early IORT scan Nicolas Saenz Julienne (6): arm64: mm: Move reserve_crashkernel() into mem_init() arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() of/address: Introduce of_dma_get_max_cpu_address() of: unittest: Add test for of_dma_get_max_cpu_address() arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges mm: Remove examples from enum zone_type comment arch/arm64/mm/init.c | 16 ++-- drivers/acpi/arm64/iort.c | 52 +++ drivers/of/address.c | 42 +++ drivers/of/unittest.c | 18 ++ include/linux/acpi_iort.h | 4 +++ include/linux/mmzone.h| 20 --- include/linux/of.h| 7 ++ 7 files changed, 130 insertions(+), 29 deletions(-) -- 2.29.0
[PATCH v5 3/7] of/address: Introduce of_dma_get_max_cpu_address()
Introduce of_dma_get_max_cpu_address(), which provides the highest CPU
physical address addressable by all DMA masters in the system. It's
specially useful for setting memory zones sizes at early boot time.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v4:
- Return max address, not address limit (one-off difference)
Changes since v3:
- use u64 with cpu_end
Changes since v2:
- Use PHYS_ADDR_MAX
- return phys_dma_t
- Rename function
- Correct subject
- Add support to start parsing from an arbitrary device node in order
for the function to work with unit tests
drivers/of/address.c | 42 ++
include/linux/of.h | 7 +++
2 files changed, 49 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index eb9ab4f1e80b..09c0af7fd1c4 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1024,6 +1024,48 @@ int of_dma_get_range(struct device_node *np, const
struct bus_dma_region **map)
}
#endif /* CONFIG_HAS_DMA */
+/**
+ * of_dma_get_max_cpu_address - Gets highest CPU address suitable for DMA
+ * @np: The node to start searching from or NULL to start from the root
+ *
+ * Gets the highest CPU physical address that is addressable by all DMA masters
+ * in the sub-tree pointed by np, or the whole tree if NULL is passed. If no
+ * DMA constrained device is found, it returns PHYS_ADDR_MAX.
+ */
+phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
+{
+ phys_addr_t max_cpu_addr = PHYS_ADDR_MAX;
+ struct of_range_parser parser;
+ phys_addr_t subtree_max_addr;
+ struct device_node *child;
+ struct of_range range;
+ const __be32 *ranges;
+ u64 cpu_end = 0;
+ int len;
+
+ if (!np)
+ np = of_root;
+
+ ranges = of_get_property(np, "dma-ranges", &len);
+ if (ranges && len) {
+ of_dma_range_parser_init(&parser, np);
+ for_each_of_range(&parser, &range)
+ if (range.cpu_addr + range.size > cpu_end)
+ cpu_end = range.cpu_addr + range.size - 1;
+
+ if (max_cpu_addr > cpu_end)
+ max_cpu_addr = cpu_end;
+ }
+
+ for_each_available_child_of_node(np, child) {
+ subtree_max_addr = of_dma_get_max_cpu_address(child);
+ if (max_cpu_addr > subtree_max_addr)
+ max_cpu_addr = subtree_max_addr;
+ }
+
+ return max_cpu_addr;
+}
+
/**
* of_dma_is_coherent - Check if device is coherent
* @np:device node
diff --git a/include/linux/of.h b/include/linux/of.h
index 5d51891cbf1a..9ed5b8532c30 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out);
+phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
+
#else /* CONFIG_OF */
static inline void of_core_init(void)
@@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
return -EINVAL;
}
+static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
+{
+ return PHYS_ADDR_MAX;
+}
+
#define of_match_ptr(_ptr) NULL
#define of_match_node(_matches, _node) NULL
#endif /* CONFIG_OF */
--
2.29.0
[PATCH v5 4/7] of: unittest: Add test for of_dma_get_max_cpu_address()
Introduce a test for of_dma_get_max_cup_address(), it uses the same DT
data as the rest of dma-ranges unit tests.
Signed-off-by: Nicolas Saenz Julienne
Reviewed-by: Rob Herring
---
Changes since v3:
- Remove HAS_DMA guards
drivers/of/unittest.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 06cc988faf78..b9a4d047a95e 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -869,6 +869,23 @@ static void __init of_unittest_changeset(void)
#endif
}
+static void __init of_unittest_dma_get_max_cpu_address(void)
+{
+ struct device_node *np;
+ phys_addr_t cpu_addr;
+
+ np = of_find_node_by_path("/testcase-data/address-tests");
+ if (!np) {
+ pr_err("missing testcase data\n");
+ return;
+ }
+
+ cpu_addr = of_dma_get_max_cpu_address(np);
+ unittest(cpu_addr == 0x5000,
+"of_dma_get_max_cpu_address: wrong CPU addr %pad (expecting
%x)\n",
+&cpu_addr, 0x5000);
+}
+
static void __init of_unittest_dma_ranges_one(const char *path,
u64 expect_dma_addr, u64 expect_paddr)
{
@@ -3266,6 +3283,7 @@ static int __init of_unittest(void)
of_unittest_changeset();
of_unittest_parse_interrupts();
of_unittest_parse_interrupts_extended();
+ of_unittest_dma_get_max_cpu_address();
of_unittest_parse_dma_ranges();
of_unittest_pci_dma_ranges();
of_unittest_match_node();
--
2.29.0
[PATCH v5 7/7] mm: Remove examples from enum zone_type comment
We can't really list every setup in common code. On top of that they are
unlikely to stay true for long as things change in the arch trees
independently of this comment.
Suggested-by: Christoph Hellwig
Signed-off-by: Nicolas Saenz Julienne
Reviewed-by: Christoph Hellwig
---
include/linux/mmzone.h | 20
1 file changed, 20 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..9d0c454d23cd 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -354,26 +354,6 @@ enum zone_type {
* DMA mask is assumed when ZONE_DMA32 is defined. Some 64-bit
* platforms may need both zones as they support peripherals with
* different DMA addressing limitations.
-*
-* Some examples:
-*
-* - i386 and x86_64 have a fixed 16M ZONE_DMA and ZONE_DMA32 for the
-*rest of the lower 4G.
-*
-* - arm only uses ZONE_DMA, the size, up to 4G, may vary depending on
-*the specific device.
-*
-* - arm64 has a fixed 1G ZONE_DMA and ZONE_DMA32 for the rest of the
-*lower 4G.
-*
-* - powerpc only uses ZONE_DMA, the size, up to 2G, may vary
-*depending on the specific device.
-*
-* - s390 uses ZONE_DMA fixed to the lower 2G.
-*
-* - ia64 and riscv only use ZONE_DMA32.
-*
-* - parisc uses neither.
*/
#ifdef CONFIG_ZONE_DMA
ZONE_DMA,
--
2.29.0
[PATCH v5 5/7] arm64: mm: Set ZONE_DMA size based on devicetree's dma-ranges
We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)
The DMA layer also needs to be able to allocate memory that is
guaranteed to meet those DMA constraints, for bounce buffering as well
as allocating the backing for consistent mappings. This is why the 1 GB
ZONE_DMA was introduced recently. Unfortunately, it turns out the having
a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and
potentially in other places where allocations cannot cross zone
boundaries. Therefore, we should avoid having two separate DMA zones
when possible.
So, with the help of of_dma_get_max_cpu_address() get the topmost
physical address accessible to all DMA masters in system and use that
information to fine-tune ZONE_DMA's size. In the absence of addressing
limited masters ZONE_DMA will span the whole 32-bit address space,
otherwise, in the case of the Raspberry Pi 4 it'll only span the 30-bit
address space, and have ZONE_DMA32 cover the rest of the 32-bit address
space.
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v4:
- Use fls64 as we're now using the max address (as opposed to the
limit)
Changes since v3:
- Simplify code for readability.
Changes since v2:
- Updated commit log by shamelessly copying Ard's ACPI commit log
arch/arm64/mm/init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 410721fc4fc0..a2ce8a9a71a6 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -42,8 +42,6 @@
#include
#include
-#define ARM64_ZONE_DMA_BITS30
-
/*
* We need to be able to catch inadvertent references to memstart_addr
* that occur (potentially in generic code) before arm64_memblock_init()
@@ -188,9 +186,11 @@ static phys_addr_t __init max_zone_phys(unsigned int
zone_bits)
static void __init zone_sizes_init(unsigned long min, unsigned long max)
{
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
+ unsigned int __maybe_unused dt_zone_dma_bits;
#ifdef CONFIG_ZONE_DMA
- zone_dma_bits = ARM64_ZONE_DMA_BITS;
+ dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
+ zone_dma_bits = min(32U, dt_zone_dma_bits);
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
--
2.29.0
[PATCH v5 2/7] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
zone_dma_bits's initialization happens earlier that it's actually
needed, in arm64_memblock_init(). So move it into the more suitable
zone_sizes_init().
Signed-off-by: Nicolas Saenz Julienne
Tested-by: Jeremy Linton
---
arch/arm64/mm/init.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index fc4ab0d6d5d2..410721fc4fc0 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -190,6 +190,8 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
#ifdef CONFIG_ZONE_DMA
+ zone_dma_bits = ARM64_ZONE_DMA_BITS;
+ arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
#ifdef CONFIG_ZONE_DMA32
@@ -376,11 +378,6 @@ void __init arm64_memblock_init(void)
early_init_fdt_scan_reserved_mem();
- if (IS_ENABLED(CONFIG_ZONE_DMA)) {
- zone_dma_bits = ARM64_ZONE_DMA_BITS;
- arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
- }
-
if (IS_ENABLED(CONFIG_ZONE_DMA32))
arm64_dma32_phys_limit = max_zone_phys(32);
else
--
2.29.0
[PATCH v5 1/7] arm64: mm: Move reserve_crashkernel() into mem_init()
crashkernel might reserve memory located in ZONE_DMA. We plan to delay ZONE_DMA's initialization after unflattening the devicetree and ACPI's boot table initialization, so move it later in the boot process. Specifically into mem_init(), this is the last place crashkernel will be able to reserve the memory before the page allocator kicks in. There isn't any apparent reason for doing this earlier. Signed-off-by: Nicolas Saenz Julienne Tested-by: Jeremy Linton --- 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 095540667f0f..fc4ab0d6d5d2 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -386,8 +386,6 @@ void __init arm64_memblock_init(void) else arm64_dma32_phys_limit = PHYS_MASK + 1; - reserve_crashkernel(); - reserve_elfcorehdr(); high_memory = __va(memblock_end_of_DRAM() - 1) + 1; @@ -508,6 +506,8 @@ void __init mem_init(void) else swiotlb_force = SWIOTLB_NO_FORCE; + reserve_crashkernel(); + set_max_mapnr(max_pfn - PHYS_PFN_OFFSET); #ifndef CONFIG_SPARSEMEM_VMEMMAP -- 2.29.0
[PATCH v5 6/7] arm64: mm: Set ZONE_DMA size based on early IORT scan
From: Ard Biesheuvel
We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)
Instructing the DMA layer about these limitations is straight-forward,
even though we had to fix some issues regarding memory limits set in
the IORT for named components, and regarding the handling of ACPI _DMA
methods. However, the DMA layer also needs to be able to allocate
memory that is guaranteed to meet those DMA constraints, for bounce
buffering as well as allocating the backing for consistent mappings.
This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
problems with kdump, and potentially in other places where allocations
cannot cross zone boundaries. Therefore, we should avoid having two
separate DMA zones when possible.
So let's do an early scan of the IORT, and only create the ZONE_DMA
if we encounter any devices that need it. This puts the burden on
the firmware to describe such limitations in the IORT, which may be
redundant (and less precise) if _DMA methods are also being provided.
However, it should be noted that this situation is highly unusual for
arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
the _DMA method if implemented, and so we will not lose the ability to
perform streaming DMA outside the ZONE_DMA if the _DMA method permits
it.
Cc: Jeremy Linton
Cc: Lorenzo Pieralisi
Cc: Nicolas Saenz Julienne
Cc: Rob Herring
Cc: Christoph Hellwig
Cc: Robin Murphy
Cc: Hanjun Guo
Cc: Sudeep Holla
Cc: Anshuman Khandual
Signed-off-by: Ard Biesheuvel
[nsaenz: Rebased, removed documentation change and add declaration in
acpi_iort.h]
Signed-off-by: Nicolas Saenz Julienne
Tested-by: Jeremy Linton
Acked-by: Lorenzo Pieralisi
Acked-by: Hanjun Guo
---
Changes since v3:
- Use min_not_zero()
- Check revision
- Remove unnecessary #ifdef in zone_sizes_init()
arch/arm64/mm/init.c | 3 ++-
drivers/acpi/arm64/iort.c | 52 +++
include/linux/acpi_iort.h | 4 +++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index a2ce8a9a71a6..b9dc3831dd6b 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
@@ -190,7 +191,7 @@ static void __init zone_sizes_init(unsigned long min,
unsigned long max)
#ifdef CONFIG_ZONE_DMA
dt_zone_dma_bits = fls64(of_dma_get_max_cpu_address(NULL));
- zone_dma_bits = min(32U, dt_zone_dma_bits);
+ zone_dma_bits = min3(32U, dt_zone_dma_bits,
acpi_iort_get_zone_dma_size());
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 9929ff50c0c0..05fe4a076bab 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1718,3 +1718,55 @@ void __init acpi_iort_init(void)
iort_init_platform_devices();
}
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+ struct acpi_table_iort *iort;
+ struct acpi_iort_node *node, *end;
+ acpi_status status;
+ u8 limit = 32;
+ int i;
+
+ if (acpi_disabled)
+ return limit;
+
+ status = acpi_get_table(ACPI_SIG_IORT, 0,
+ (struct acpi_table_header **)&iort);
+ if (ACPI_FAILURE(status))
+ return limit;
+
+ node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+ end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+ for (i = 0; i < iort->node_count; i++) {
+ if (node >= end)
+ break;
+
+ switch (node->type) {
+ struct acpi_iort_named_component *ncomp;
+ struct acpi_iort_root_complex *rc;
+
+ case ACPI_IORT_NODE_NAMED_COMPONENT:
+ ncomp = (struct acpi_iort_named_component
*)node->node_data;
+ limit = min_not_zero(limit,
ncomp->memory_address_limit);
+ break;
+
+ case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+ if (node->revision < 1)
+ break;
+
+ rc = (struct acpi_iort_root_complex *)node->node_data;
+ limit = min_not_zero(limit, rc->memo
[PATCH 3/4] dma-pool: Introduce dma_guess_pool()
dma-pool's dev_to_pool() creates the false impression that there is a
way to grantee a mapping between a device's DMA constraints and an
atomic pool. It tuns out it's just a guess, and the device might need to
use an atomic pool containing memory from a 'safer' (or lower) memory
zone.
To help mitigate this, introduce dma_guess_pool() which can be fed a
device's DMA constraints or an atomic pool already known to be faulty in
order for it to provide a better guess at which pool to use.
Signed-off-by: Nicolas Saenz Julienne
---
kernel/dma/pool.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 7363640fc91c..3d518de07617 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -203,7 +203,7 @@ static int __init dma_atomic_pool_init(void)
}
postcore_initcall(dma_atomic_pool_init);
-static inline struct gen_pool *dev_to_pool(struct device *dev)
+static inline struct gen_pool *dma_guess_pool_from_device(struct device *dev)
{
u64 phys_mask;
gfp_t gfp;
@@ -217,10 +217,30 @@ static inline struct gen_pool *dev_to_pool(struct device
*dev)
return atomic_pool_kernel;
}
+static inline struct gen_pool *dma_get_safer_pool(struct gen_pool *bad_pool)
+{
+ if (bad_pool == atomic_pool_kernel)
+ return atomic_pool_dma32 ? : atomic_pool_dma;
+
+ if (bad_pool == atomic_pool_dma32)
+ return atomic_pool_dma;
+
+ return NULL;
+}
+
+static inline struct gen_pool *dma_guess_pool(struct device *dev,
+ struct gen_pool *bad_pool)
+{
+ if (bad_pool)
+ return dma_get_safer_pool(bad_pool);
+
+ return dma_guess_pool_from_device(dev);
+}
+
void *dma_alloc_from_pool(struct device *dev, size_t size,
struct page **ret_page, gfp_t flags)
{
- struct gen_pool *pool = dev_to_pool(dev);
+ struct gen_pool *pool = dma_guess_pool(dev, NULL);
unsigned long val;
void *ptr = NULL;
@@ -245,7 +265,7 @@ void *dma_alloc_from_pool(struct device *dev, size_t size,
bool dma_free_from_pool(struct device *dev, void *start, size_t size)
{
- struct gen_pool *pool = dev_to_pool(dev);
+ struct gen_pool *pool = dma_guess_pool(dev, NULL);
if (!pool || !gen_pool_has_addr(pool, (unsigned long)start, size))
return false;
--
2.27.0
[PATCH 1/4] dma-direct: Provide function to check physical memory area validity
dma_coherent_ok() checks if a physical memory area fits a device's DMA
constraints.
Signed-off-by: Nicolas Saenz Julienne
---
include/linux/dma-direct.h | 1 +
kernel/dma/direct.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index cdfa400f89b3..cb23a8305132 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -69,6 +69,7 @@ static inline bool dma_capable(struct device *dev, dma_addr_t
addr, size_t size,
u64 dma_direct_get_required_mask(struct device *dev);
gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
u64 *phys_mask);
+bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size);
void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t gfp, unsigned long attrs);
void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 93f578a8e613..4de864cacd22 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -70,7 +70,7 @@ gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64
dma_mask,
return 0;
}
-static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
+bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
{
return phys_to_dma_direct(dev, phys) + size - 1 <=
min_not_zero(dev->coherent_dma_mask,
dev->bus_dma_limit);
--
2.27.0
[PATCH 0/4] dma-pool: Fix atomic pool selection
This is my attempt at fixing one of the regressions we've seen[1] after the introduction of per-zone atomic pools. This combined with "dma-pool: Do not allocate pool memory from CMA"[2] should fix the boot issues on Jeremy's RPi4 setup. [1] https://lkml.org/lkml/2020/7/2/974 [2] https://lkml.org/lkml/2020/7/8/1108 --- Nicolas Saenz Julienne (4): dma-direct: Provide function to check physical memory area validity dma-pool: Get rid of dma_in_atomic_pool() dma-pool: Introduce dma_guess_pool() dma-pool: Make sure atomic pool suits device include/linux/dma-direct.h | 1 + kernel/dma/direct.c| 2 +- kernel/dma/pool.c | 76 +++--- 3 files changed, 56 insertions(+), 23 deletions(-) -- 2.27.0
[PATCH 4/4] dma-pool: Make sure atomic pool suits device
When allocating DMA memory from a pool, the core can only guess which
atomic pool will fit a device's constraints. If it doesn't, get a safer
atomic pool and try again.
Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to map to gfp
mask")
Reported-by: Jeremy Linton
Suggested-by: Robin Murphy
Signed-off-by: Nicolas Saenz Julienne
---
kernel/dma/pool.c | 53 +--
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 3d518de07617..d48d9acb585f 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -240,35 +240,56 @@ static inline struct gen_pool *dma_guess_pool(struct
device *dev,
void *dma_alloc_from_pool(struct device *dev, size_t size,
struct page **ret_page, gfp_t flags)
{
- struct gen_pool *pool = dma_guess_pool(dev, NULL);
- unsigned long val;
+ struct gen_pool *pool = NULL;
+ unsigned long val = 0;
void *ptr = NULL;
-
- if (!pool) {
- WARN(1, "%pGg atomic pool not initialised!\n", &flags);
- return NULL;
+ phys_addr_t phys;
+
+ while (1) {
+ pool = dma_guess_pool(dev, pool);
+ if (!pool) {
+ WARN(1, "Failed to get suitable pool for %s\n",
+dev_name(dev));
+ break;
+ }
+
+ val = gen_pool_alloc(pool, size);
+ if (!val)
+ continue;
+
+ phys = gen_pool_virt_to_phys(pool, val);
+ if (dma_coherent_ok(dev, phys, size))
+ break;
+
+ gen_pool_free(pool, val, size);
+ val = 0;
}
- val = gen_pool_alloc(pool, size);
- if (val) {
- phys_addr_t phys = gen_pool_virt_to_phys(pool, val);
+ if (val) {
*ret_page = pfn_to_page(__phys_to_pfn(phys));
ptr = (void *)val;
memset(ptr, 0, size);
+
+ if (gen_pool_avail(pool) < atomic_pool_size)
+ schedule_work(&atomic_pool_work);
}
- if (gen_pool_avail(pool) < atomic_pool_size)
- schedule_work(&atomic_pool_work);
return ptr;
}
bool dma_free_from_pool(struct device *dev, void *start, size_t size)
{
- struct gen_pool *pool = dma_guess_pool(dev, NULL);
+ struct gen_pool *pool = NULL;
+
+ while (1) {
+ pool = dma_guess_pool(dev, pool);
+ if (!pool)
+ return false;
- if (!pool || !gen_pool_has_addr(pool, (unsigned long)start, size))
- return false;
- gen_pool_free(pool, (unsigned long)start, size);
- return true;
+ if (gen_pool_has_addr(pool, (unsigned long)start, size)) {
+ gen_pool_free(pool, (unsigned long)start, size);
+ return true;
+ }
+ }
}
--
2.27.0
[PATCH 2/4] dma-pool: Get rid of dma_in_atomic_pool()
The function is only used once and can be simplified to a one-liner.
Signed-off-by: Nicolas Saenz Julienne
---
kernel/dma/pool.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 8cfa01243ed2..7363640fc91c 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -217,15 +217,6 @@ static inline struct gen_pool *dev_to_pool(struct device
*dev)
return atomic_pool_kernel;
}
-static bool dma_in_atomic_pool(struct device *dev, void *start, size_t size)
-{
- struct gen_pool *pool = dev_to_pool(dev);
-
- if (unlikely(!pool))
- return false;
- return gen_pool_has_addr(pool, (unsigned long)start, size);
-}
-
void *dma_alloc_from_pool(struct device *dev, size_t size,
struct page **ret_page, gfp_t flags)
{
@@ -256,7 +247,7 @@ bool dma_free_from_pool(struct device *dev, void *start,
size_t size)
{
struct gen_pool *pool = dev_to_pool(dev);
- if (!dma_in_atomic_pool(dev, start, size))
+ if (!pool || !gen_pool_has_addr(pool, (unsigned long)start, size))
return false;
gen_pool_free(pool, (unsigned long)start, size);
return true;
--
2.27.0
Re: [PATCH] dma-pool: use single atomic pool for both DMA zones
On Thu, 2020-07-09 at 14:49 -0700, David Rientjes wrote:
> On Wed, 8 Jul 2020, Christoph Hellwig wrote:
>
> > On Wed, Jul 08, 2020 at 06:00:35PM +0200, Nicolas Saenz Julienne wrote:
> > > On Wed, 2020-07-08 at 17:35 +0200, Christoph Hellwig wrote:
> > > > On Tue, Jul 07, 2020 at 02:28:04PM +0200, Nicolas Saenz Julienne wrote:
> > > > > When allocating atomic DMA memory for a device, the dma-pool core
> > > > > queries __dma_direct_optimal_gfp_mask() to check which atomic pool to
> > > > > use. It turns out the GFP flag returned is only an optimistic guess.
> > > > > The pool selected might sometimes live in a zone higher than the
> > > > > device's view of memory.
> > > > >
> > > > > As there isn't a way to grantee a mapping between a device's DMA
> > > > > constraints and correct GFP flags this unifies both DMA atomic pools.
> > > > > The resulting pool is allocated in the lower DMA zone available, if
> > > > > any,
> > > > > so as for devices to always get accessible memory while having the
> > > > > flexibility of using dma_pool_kernel for the non constrained ones.
> > > > >
> > > > > Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to map
> > > > > to gfp
> > > > > mask")
> > > > > Reported-by: Jeremy Linton
> > > > > Suggested-by: Robin Murphy
> > > > > Signed-off-by: Nicolas Saenz Julienne
> > > >
> > > > Hmm, this is not what I expected from the previous thread. I thought
> > > > we'd just use one dma pool based on runtime available of the zones..
> > >
> > > I may be misunderstanding you, but isn't that going back to how things
> > > used to
> > > be before pulling in David Rientjes' work? The benefit of having a
> > > GFP_KERNEL
> > > pool is that non-address-constrained devices can get their atomic memory
> > > there,
> > > instead of consuming somewhat scarcer low memory.
> >
> > Yes, I think we are misunderstanding each other. I don't want to remove
> > any pool, just make better runtime decisions when to use them.
> >
>
> Just to be extra explicit for the record and for my own understanding:
> Nicolas, your patch series "dma-pool: Fix atomic pool selection" obsoletes
> this patch, right? :)
Yes, that's right.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [RFC] of/platform: Create device link between simple-mfd and its children
On Fri, 2020-10-16 at 09:38 -0500, Rob Herring wrote:
> On Thu, Oct 15, 2020 at 6:43 AM Nicolas Saenz Julienne
> wrote:
> > 'simple-mfd' usage implies there might be some kind of resource sharing
> > between the parent device and its children.
>
> It does? No! The reason behind simple-mfd was specifically because
> there was no parent driver or dependency on the parent. No doubt
> simple-mfd has been abused.
Fair enough, so we're doing things wrong. Just for the record, I'm looking at
RPi´s firmware interface:
firmware: firmware {
compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
#address-cells = <1>;
#size-cells = <1>;
mboxes = <&mailbox>;
firmware_clocks: clocks {
compatible = "raspberrypi,firmware-clocks";
#clock-cells = <1>;
};
reset: reset {
compatible = "raspberrypi,firmware-reset";
#reset-cells = <1>;
};
[...]
};
Note that "raspberrypi,bcm2835-firmware" has a driver, it's not just a
placeholder. Consumer drivers get a handle to RPi's firmware interface through
the supplier's API, rpi_firmware_get(). The handle to firmware becomes
meaningless if it is unbinded, which I want to protect myself against.
A simpler solution would be to manually create a device link between both
devices ("raspberrypi,bcm2835-firmware" and "raspberrypi,firmware-clocks" for
example) upon calling rpi_firmware_get(). But I wanted to try addressing the
problem in a generic way first.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [RFC] of/platform: Create device link between simple-mfd and its children
Hi Saravana, thanks for your comments. On Thu, 2020-10-15 at 09:52 -0700, Saravana Kannan wrote: > On Thu, Oct 15, 2020 at 4:43 AM Nicolas Saenz Julienne > wrote: > > 'simple-mfd' usage implies there might be some kind of resource sharing > > between the parent device and its children. By creating a device link > > with DL_FLAG_AUTOREMOVE_CONSUMER we make sure that at no point in time > > the parent device is unbound while leaving its children unaware that > > some of their resources disappeared. > > Doesn't the parent child relationship already ensure that? If not, > maybe that's what needs fixing? Well as Rob puts it, we're not using simple-mfd as it was intended. So that pretty much settles it for generic solutions. > > > Signed-off-by: Nicolas Saenz Julienne [...] > > > - If applying this to all simple-mfd devices is a bit too much, would > > this be acceptable for a specific device setup. For example RPi4's > > firmware interface (simple-mfd user) is passed to consumer drivers > > trough a custom API (see rpi_firmware_get()). So, when unbound, > > consumers are left with a firmware handle that points to nothing. > > You can always create device link between the real suppliers and consumers. RPi's firmware consumers use a custom API to get a handle to the firmware interface itself, rpi_firmware_get(). So no trace of the relationship is expressed in DT. If the firmware interface device, the supplier, is unbinded, that firmware handle now points nowhere and consumers will end up triggering kernel panics. Would it make sense to make a device link between the two in that case? Or I'd be, again, abusing the concept? Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v3 01/11] firmware: raspberrypi: Introduce devm_rpi_firmware_get()
Hi Bartosz, thanks for the feedback.
On Thu, 2020-11-05 at 10:42 +0100, Bartosz Golaszewski wrote:
> On Thu, Nov 5, 2020 at 10:28 AM Nicolas Saenz Julienne
> wrote:
> > Hi Bartosz, thanks for the review.
> >
> > On Thu, 2020-11-05 at 10:13 +0100, Bartosz Golaszewski wrote:
> > > > +/**
> > > > + * 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 platform_device *pdev =
> > > > of_find_device_by_node(firmware_node);
> > > > + struct rpi_firmware *fw;
> > > > +
> > > > + if (!pdev)
> > > > + return NULL;
> > > > +
> > > > + fw = platform_get_drvdata(pdev);
> > > > + if (!fw)
> > > > + return NULL;
> > > > +
> > > > + if (!refcount_inc_not_zero(&fw->consumers))
> > > > + return NULL;
> > > > +
> > > > + if (devm_add_action_or_reset(dev, rpi_firmware_put, fw))
> > > > + return NULL;
> > > > +
> > > > + return fw;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get);
> > >
> > > Usually I'd expect the devres variant to simply call
> > > rpi_firmware_get() and then schedule a release callback which would
> > > call whatever function is the release counterpart for it currently.
> > > Devres actions are for drivers which want to schedule some more
> > > unusual tasks at driver detach. Any reason for designing it this way?
> >
> > Yes, see patch #8 where I get rid of rpi_firmware_get() altogether after
> > converting all users to devres. Since there is no use for the vanilla
> > version
> > of the function anymore, I figured it'd be better to merge everything into
> > devm_rpi_firmware_get(). That said it's not something I have strong feelings
> > about.
> >
>
> I see. So the previous version didn't really have any reference
> counting and it leaked the reference returned by
> of_find_device_by_node(), got it. Could you just clarify for me the
> logic behind the wait_queue in rpi_firmware_remove()? If the firmware
> driver gets detached and remove() stops on the wait_queue - it will be
> stuck until the last user releases the firmware. I'm not sure this is
> correct.
Yes, that's what I meant to implement.
> I'd prefer to see a kref with a release callback and remove
> would simply decrease the kref and return. Each user would do the same
> and then after the last user is detached the firmware would be
> destroyed.
Sounds good to me. I'll update it.
> Don't we really have some centralized firmware subsystem that would handle
> this?
Sadly no, this is an RPi specific thing, it doesn't live behind a standard like
other firmware based protocols (for ex. SCMI), and evolves as the needs arise.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] PCI: brcmstb: Fix race in removing chained IRQ handler
On Wed, 2020-11-11 at 22:53 +0100, Martin Kaiser wrote:
> Call irq_set_chained_handler_and_data() to clear the chained handler
> and the handler's data under irq_desc->lock.
>
> See also 2cf5a03cb29d ("PCI/keystone: Fix race in installing chained
> IRQ handler").
>
> Signed-off-by: Martin Kaiser
> Acked-by: Florian Fainelli
> ---
Acked-by: Nicolas Saenz Julienne
Thanks!
> v2:
> - rewrite the commit message to clarify that this is a bugfix
>
> drivers/pci/controller/pcie-brcmstb.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c
> b/drivers/pci/controller/pcie-brcmstb.c
> index bea86899bd5d..7c666f4deb47 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -606,8 +606,7 @@ static void brcm_msi_remove(struct brcm_pcie *pcie)
>
> if (!msi)
> return;
> - irq_set_chained_handler(msi->irq, NULL);
> - irq_set_handler_data(msi->irq, NULL);
> + irq_set_chained_handler_and_data(msi->irq, NULL, NULL);
> brcm_free_domains(msi);
> }
>
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 07/11] input: raspberrypi-ts: Release firmware handle when not needed
On Wed, 2020-11-11 at 17:45 -0800, Dmitry Torokhov wrote: > Hi Nicolas, > > On Wed, Nov 04, 2020 at 11:39:33AM +0100, Nicolas Saenz Julienne wrote: > > Use devm_rpi_firmware_get() so as to make sure we release RPi's firmware > > interface when unbinding the device. > > Unless I am mistaken this driver does not really need the firmware > structure past rpi_ts_probe(), and will be fine if it disappears earlier > than unbind time. Yes, I missed that. Will update it. Regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v6 1/7] arm64: mm: Move reserve_crashkernel() into mem_init()
Hi Catalin,
On Tue, 2020-11-10 at 18:17 +, Catalin Marinas wrote:
> On Fri, Nov 06, 2020 at 07:46:29PM +0100, Nicolas Saenz Julienne wrote:
> > On Thu, 2020-11-05 at 16:11 +, James Morse wrote:
> > > On 03/11/2020 17:31, Nicolas Saenz Julienne wrote:
> > > > crashkernel might reserve memory located in ZONE_DMA. We plan to delay
> > > > ZONE_DMA's initialization after unflattening the devicetree and ACPI's
> > > > boot table initialization, so move it later in the boot process.
> > > > Specifically into mem_init(), this is the last place crashkernel will be
> > > > able to reserve the memory before the page allocator kicks in.
> > > > There
> > > > isn't any apparent reason for doing this earlier.
> > >
> > > It's so that map_mem() can carve it out of the linear/direct map.
> > > This is so that stray writes from a crashing kernel can't accidentally
> > > corrupt the kdump
> > > kernel. We depend on this if we continue with kdump, but failed to
> > > offline all the other
> > > CPUs.
> >
> > I presume here you refer to arch_kexec_protect_crashkres(), IIUC this will
> > only
> > happen further down the line, after having loaded the kdump kernel image.
> > But
> > it also depends on the mappings to be PAGE sized (flags ==
> > NO_BLOCK_MAPPINGS |
> > NO_CONT_MAPPINGS).
>
> IIUC, arch_kexec_protect_crashkres() is only for the crashkernel image,
> not the whole reserved memory that the crashkernel will use. For the
> latter, we avoid the linear map by marking it as nomap in map_mem().
I'm not sure we're on the same page here, so sorry if this was already implied.
The crashkernel memory mapping is bypassed while preparing the linear mappings
but it is then mapped right away, with page granularity and !MTE.
See paging_init()->map_mem():
/*
* Use page-level mappings here so that we can shrink the region
* in page granularity and put back unused memory to buddy system
* through /sys/kernel/kexec_crash_size interface.
*/
if (crashk_res.end) {
__map_memblock(pgdp, crashk_res.start, crashk_res.end + 1,
PAGE_KERNEL,
NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
memblock_clear_nomap(crashk_res.start,
resource_size(&crashk_res));
}
IIUC the inconvenience here is that we need special mapping options for
crashkernel and updating those after having mapped that memory as regular
memory isn't possible/easy to do.
> > > We also depend on this when skipping the checksum code in purgatory,
> > > which can be
> > > exceedingly slow.
> >
> > This one I don't fully understand, so I'll lazily assume the prerequisite is
> > the same WRT how memory is mapped. :)
> >
> > Ultimately there's also /sys/kernel/kexec_crash_size's handling. Same
> > prerequisite.
> >
> > Keeping in mind acpi_table_upgrade() and unflatten_device_tree() depend on
> > having the linear mappings available.
>
> So it looks like reserve_crashkernel() wants to reserve memory before
> setting up the linear map with the information about the DMA zones in
> place but that comes later when we can parse the firmware tables.
>
> I wonder, instead of not mapping the crashkernel reservation, can we not
> do an arch_kexec_protect_crashkres() for the whole reservation after we
> created the linear map?
arch_kexec_protect_crashkres() depends on __change_memory_common() which
ultimately depends on the memory to be mapped with PAGE_SIZE pages. As I
comment above, the trick would work as long as there is as way to update the
linear mappings with whatever crashkernel needs later in the boot process.
> > Let me stress that knowing the DMA constraints in the system before
> > reserving
> > crashkernel's regions is necessary if we ever want it to work seamlessly on
> > all
> > platforms. Be it small stuff like the Raspberry Pi or huge servers with TB
> > of
> > memory.
>
> Indeed. So we have 3 options (so far):
>
> 1. Allow the crashkernel reservation to go into the linear map but set
>it to invalid once allocated.
>
> 2. Parse the flattened DT (not sure what we do with ACPI) before
>creating the linear map. We may have to rely on some SoC ID here
>instead of actual DMA ranges.
>
> 3. Assume the smallest ZONE_DMA possible on arm64 (1GB) for crashkernel
>reservations and not rely on arm64_dma_phys_limit in
>reserve_crashkernel().
>
> I think (2) we tried hard to avoi
[PATCH v4 08/11] input: raspberrypi-ts: Release firmware handle when not needed
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
---
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,
&touchbuf, sizeof(touchbuf));
-
+ rpi_firmware_put(fw);
if (error || touchbuf != 0) {
dev_warn(dev, "Failed to set touchbuf, %d\n", error);
return error;
--
2.29.2
[PATCH v4 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 imporant 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 firwmware 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 firwmare 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 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| 66 +- 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 | 216 ++ drivers/reset/reset-raspberrypi.c | 2 +- drivers/soc/bcm/raspberrypi-power.c | 2 +- .../interface/vchiq_arm/vchiq_arm.c | 2 +- .../pwm/raspberrypi,firmware-pwm.h| 13 ++ include/soc/bcm2835/raspberrypi-firmware.h| 10 + 14 files changed, 391 insertions(+), 10 deletions(-) create mode 100644 drivers/pwm/pwm-raspberrypi-poe.c create mode 100644 include/dt-bindings/pwm/raspberrypi,firmware-pwm.h -- 2.29.2
[PATCH v4 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
---
Changes since v3:
- Use kref instead of waiting on refcount
drivers/firmware/raspberrypi.c | 37 +++---
include/soc/bcm2835/raspberrypi-firmware.h | 2 ++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 2371d08bdd17..438e17074a97 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,27 @@ 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(&fw->consumers, rpi_firmware_delete);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_put);
+
static int rpi_firmware_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct rpi_firmware *fw;
- fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
+ fw = kzalloc(sizeof(*fw), GFP_KERNEL);
if (!fw)
return -ENOMEM;
@@ -247,6 +265,7 @@ static int rpi_firmware_probe(struct platform_device *pdev)
}
init_completion(&fw->c);
+ kref_init(&fw->consumers);
platform_set_drvdata(pdev, fw);
@@ -275,25 +294,35 @@ 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;
}
/**
- * 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(&fw->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.29.2
