* [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips
@ 2020-06-16 20:55 Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 01/12] PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB Jim Quinlan
` (10 more replies)
0 siblings, 11 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Alan Stern, Andy Shevchenko, Arnd Bergmann, Corey Minyard,
open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE,
open list:DRM DRIVERS FOR ALLWINNER A10, Florian Fainelli,
Greg Kroah-Hartman, Hans de Goede, H. Peter Anvin,
open list:IOMMU DRIVERS, Jens Axboe, Julien Grall,
open list:ACPI FOR ARM64 (ACPI/arm64),
moderated list:ARM PORT,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, open list:ALLWINNER A10 CSI DRIVER,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:SUPERH, open list:USB SUBSYSTEM, Oliver Neukum,
Rafael J. Wysocki, Rob Herring, Robin Murphy, Saravana Kannan,
Stefano Stabellini, Suzuki K Poulose, Ulf Hansson, Wolfram Sang
Patchset Summary:
Enhance a PCIe host controller driver. Because of its unusual design
we are foced to change dev->dma_pfn_offset into a more general role
allowing multiple offsets.
v5:
Commit "device core: Introduce multiple dma pfn offsets"
-- in of/address.c: "map_size = 0" => "*map_size = 0"
-- use kcalloc instead of kzalloc (AndyS)
-- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
Commit "PCI: brcmstb: Set internal memory viewport sizes"
-- now gives error on missing dma-ranges property.
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- removed "Allof:" from brcm,scb-sizes definition (RobH)
All Commits:
-- indentation style, use max chars 100 (AndyS)
-- rebased to torvalds master
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead of a single scalar offset it provides for multiple
offsets via a function which depends upon the "dma-ranges" property of
the PCIe host controller. This is required for proper functionality
of the BrcmSTB PCIe controller and possibly some other devices.
[1] https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101024@gmail.com/
Jim Quinlan (12):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
ata: ahci_brcm: Fix use of BCM7216 reset controller
dt-bindings: PCI: Add bindings for more Brcmstb chips
PCI: brcmstb: Add bcm7278 register info
PCI: brcmstb: Add suspend and resume pm_ops
PCI: brcmstb: Add bcm7278 PERST support
PCI: brcmstb: Add control of rescal reset
device core: Introduce multiple dma pfn offsets
PCI: brcmstb: Set internal memory viewport sizes
PCI: brcmstb: Accommodate MSI for older chips
PCI: brcmstb: Set bus max burst size by chip type
PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
.../bindings/pci/brcm,stb-pcie.yaml | 56 ++-
arch/arm/include/asm/dma-mapping.h | 9 +-
arch/arm/mach-keystone/keystone.c | 8 +-
arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
arch/sh/kernel/dma-coherent.c | 14 +-
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 4 +-
drivers/ata/ahci_brcm.c | 11 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
drivers/iommu/io-pgtable-arm.c | 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +-
drivers/of/address.c | 71 ++-
drivers/of/device.c | 19 +-
drivers/of/of_private.h | 11 +-
drivers/of/unittest.c | 8 +-
drivers/pci/controller/Kconfig | 3 +-
drivers/pci/controller/pcie-brcmstb.c | 403 +++++++++++++++---
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c | 7 +-
drivers/usb/core/message.c | 4 +-
drivers/usb/core/usb.c | 2 +-
include/linux/device.h | 4 +-
include/linux/dma-direct.h | 14 +-
include/linux/dma-mapping.h | 38 ++
kernel/dma/coherent.c | 11 +-
kernel/dma/mapping.c | 39 ++
27 files changed, 632 insertions(+), 132 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 01/12] PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 02/12] ata: ahci_brcm: Fix use of BCM7216 reset controller Jim Quinlan
` (9 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, open list
From: Jim Quinlan <jquinlan@broadcom.com>
Have PCIE_BRCMSTB depend on ARCH_BRCMSTB. Also set the default value to
ARCH_BRCMSTB.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
drivers/pci/controller/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index adddf21fa381..c165328d5fbb 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -262,9 +262,10 @@ config VMD
config PCIE_BRCMSTB
tristate "Broadcom Brcmstb PCIe host controller"
- depends on ARCH_BCM2835 || COMPILE_TEST
+ depends on ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST
depends on OF
depends on PCI_MSI_IRQ_DOMAIN
+ default ARCH_BRCMSTB
help
Say Y here to enable PCIe host controller support for
Broadcom STB based SoCs, like the Raspberry Pi 4.
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 02/12] ata: ahci_brcm: Fix use of BCM7216 reset controller
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 01/12] PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 22:04 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 03/12] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
` (8 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Jens Axboe, Philipp Zabel, Florian Fainelli,
Hans de Goede,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list
From: Jim Quinlan <jquinlan@broadcom.com>
A reset controller "rescal" is shared between the AHCI driver and the PCIe
driver for the BrcmSTB 7216 chip. The code is modified to allow this
sharing and to deassert() properly.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Fixes: 272ecd60a636 ("ata: ahci_brcm: BCM7216 reset is self de-asserting")
Fixes: c345ec6a50e9 ("ata: ahci_brcm: Support BCM7216 reset controller
name")
---
drivers/ata/ahci_brcm.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 6853dbb4131d..d6115bc04b09 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -428,7 +428,6 @@ static int brcm_ahci_probe(struct platform_device *pdev)
{
const struct of_device_id *of_id;
struct device *dev = &pdev->dev;
- const char *reset_name = NULL;
struct brcm_ahci_priv *priv;
struct ahci_host_priv *hpriv;
struct resource *res;
@@ -452,11 +451,10 @@ static int brcm_ahci_probe(struct platform_device *pdev)
/* Reset is optional depending on platform and named differently */
if (priv->version == BRCM_SATA_BCM7216)
- reset_name = "rescal";
+ priv->rcdev = devm_reset_control_get_optional_shared(&pdev->dev, "rescal");
else
- reset_name = "ahci";
+ priv->rcdev = devm_reset_control_get_optional(&pdev->dev, "ahci");
- priv->rcdev = devm_reset_control_get_optional(&pdev->dev, reset_name);
if (IS_ERR(priv->rcdev))
return PTR_ERR(priv->rcdev);
@@ -479,10 +477,7 @@ static int brcm_ahci_probe(struct platform_device *pdev)
break;
}
- if (priv->version == BRCM_SATA_BCM7216)
- ret = reset_control_reset(priv->rcdev);
- else
- ret = reset_control_deassert(priv->rcdev);
+ ret = reset_control_deassert(priv->rcdev);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 03/12] dt-bindings: PCI: Add bindings for more Brcmstb chips
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 01/12] PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 02/12] ata: ahci_brcm: Fix use of BCM7216 reset controller Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 04/12] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
` (7 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Florian Fainelli, Bjorn Helgaas, Rob Herring,
moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
- Add compatible strings for three more Broadcom STB chips: 7278, 7216,
7211 (STB version of RPi4).
- add new property 'brcm,scb-sizes'
- add new property 'resets'
- add new property 'reset-names' for 7216 only
- allow 'ranges' and 'dma-ranges' to have more than one item and update
the example to show this.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/pci/brcm,stb-pcie.yaml | 56 ++++++++++++++++---
1 file changed, 49 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 8680a0f86c5a..807694b4f41f 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -9,12 +9,15 @@ title: Brcmstb PCIe Host Controller Device Tree Bindings
maintainers:
- Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
-allOf:
- - $ref: /schemas/pci/pci-bus.yaml#
-
properties:
compatible:
- const: brcm,bcm2711-pcie # The Raspberry Pi 4
+ items:
+ - enum:
+ - brcm,bcm2711-pcie # The Raspberry Pi 4
+ - brcm,bcm7211-pcie # Broadcom STB version of RPi4
+ - brcm,bcm7278-pcie # Broadcom 7278 Arm
+ - brcm,bcm7216-pcie # Broadcom 7216 Arm
+ - brcm,bcm7445-pcie # Broadcom 7445 Arm
reg:
maxItems: 1
@@ -34,10 +37,12 @@ properties:
- const: msi
ranges:
- maxItems: 1
+ minItems: 1
+ maxItems: 4
dma-ranges:
- maxItems: 1
+ minItems: 1
+ maxItems: 6
clocks:
maxItems: 1
@@ -58,8 +63,31 @@ properties:
aspm-no-l0s: true
+ resets:
+ description: for "brcm,bcm7216-pcie", must be a valid reset
+ phandle pointing to the RESCAL reset controller provider node.
+ $ref: "/schemas/types.yaml#/definitions/phandle"
+
+ reset-names:
+ items:
+ - const: rescal
+
+ brcm,scb-sizes:
+ description: u64 giving the 64bit PCIe memory
+ viewport size of a memory controller. There may be up to
+ three controllers, and each size must be a power of two
+ with a size greater or equal to the amount of memory the
+ controller supports. Note that each memory controller
+ may have two component regions -- base and extended -- so
+ this information cannot be deduced from the dma-ranges.
+ $ref: /schemas/types.yaml#/definitions/uint64-array
+ items:
+ minItems: 1
+ maxItems: 3
+
required:
- reg
+ - ranges
- dma-ranges
- "#interrupt-cells"
- interrupts
@@ -68,6 +96,18 @@ required:
- interrupt-map
- msi-controller
+allOf:
+ - $ref: /schemas/pci/pci-bus.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: brcm,bcm7216-pcie
+ then:
+ required:
+ - resets
+ - reset-names
+
unevaluatedProperties: false
examples:
@@ -93,7 +133,9 @@ examples:
msi-parent = <&pcie0>;
msi-controller;
ranges = <0x02000000 0x0 0xf8000000 0x6 0x00000000 0x0 0x04000000>;
- dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>;
+ dma-ranges = <0x42000000 0x1 0x00000000 0x0 0x40000000 0x0 0x80000000>,
+ <0x42000000 0x1 0x80000000 0x3 0x00000000 0x0 0x80000000>;
brcm,enable-ssc;
+ brcm,scb-sizes = <0x0000000080000000 0x0000000080000000>;
};
};
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 04/12] PCI: brcmstb: Add bcm7278 register info
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (2 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 03/12] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 05/12] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
` (6 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
Add in compatibility strings and code for three Broadcom STB chips. Some
of the register locations, shifts, and masks are different for certain
chips, requiring the use of different constants based on of_id.
We would like to add the following at this time to the match list but we
need to wait until the end of this patchset so that everything works.
{ .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
{ .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg },
{ .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg },
{ .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg },
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 105 +++++++++++++++++++++++---
1 file changed, 93 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 7730ea845ff2..b7a222fde3c4 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -122,9 +122,8 @@
#define PCIE_EXT_SLOT_SHIFT 15
#define PCIE_EXT_FUNC_SHIFT 12
-#define PCIE_RGR1_SW_INIT_1 0x9210
#define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
-#define PCIE_RGR1_SW_INIT_1_INIT_MASK 0x2
+#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0
/* PCIe parameters */
#define BRCM_NUM_PCIE_OUT_WINS 0x4
@@ -154,6 +153,73 @@
#define SSC_STATUS_SSC_MASK 0x400
#define SSC_STATUS_PLL_LOCK_MASK 0x800
+#define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
+#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
+#define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
+
+enum {
+ RGR1_SW_INIT_1,
+ EXT_CFG_INDEX,
+ EXT_CFG_DATA,
+};
+
+enum {
+ RGR1_SW_INIT_1_INIT_MASK,
+ RGR1_SW_INIT_1_INIT_SHIFT,
+};
+
+enum pcie_type {
+ GENERIC,
+ BCM7278,
+ BCM2711,
+};
+
+struct pcie_cfg_data {
+ const int *reg_field_info;
+ const int *offsets;
+ const enum pcie_type type;
+};
+
+static const int pcie_reg_field_info[] = {
+ [RGR1_SW_INIT_1_INIT_MASK] = 0x2,
+ [RGR1_SW_INIT_1_INIT_SHIFT] = 0x1,
+};
+
+static const int pcie_reg_field_info_bcm7278[] = {
+ [RGR1_SW_INIT_1_INIT_MASK] = 0x1,
+ [RGR1_SW_INIT_1_INIT_SHIFT] = 0x0,
+};
+
+static const int pcie_offsets[] = {
+ [RGR1_SW_INIT_1] = 0x9210,
+ [EXT_CFG_INDEX] = 0x9000,
+ [EXT_CFG_DATA] = 0x9004,
+};
+
+static const struct pcie_cfg_data generic_cfg = {
+ .reg_field_info = pcie_reg_field_info,
+ .offsets = pcie_offsets,
+ .type = GENERIC,
+};
+
+static const int pcie_offset_bcm7278[] = {
+ [RGR1_SW_INIT_1] = 0xc010,
+ [EXT_CFG_INDEX] = 0x9000,
+ [EXT_CFG_DATA] = 0x9004,
+};
+
+static const struct pcie_cfg_data bcm7278_cfg = {
+ .reg_field_info = pcie_reg_field_info_bcm7278,
+ .offsets = pcie_offset_bcm7278,
+ .type = BCM7278,
+};
+
+static const struct pcie_cfg_data bcm2711_cfg = {
+ .reg_field_info = pcie_reg_field_info,
+ .offsets = pcie_offsets,
+ .type = BCM2711,
+};
+
struct brcm_msi {
struct device *dev;
void __iomem *base;
@@ -178,6 +244,9 @@ struct brcm_pcie {
int gen;
u64 msi_target_addr;
struct brcm_msi *msi;
+ const int *reg_offsets;
+ const int *reg_field_info;
+ enum pcie_type type;
};
/*
@@ -604,20 +673,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];
- tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
- u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK);
- writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
+ tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+ tmp = (tmp & ~mask) | ((val << shift) & mask);
+ writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
}
static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
{
u32 tmp;
- tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
+ tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
- writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
+ writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
}
static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
@@ -927,11 +997,17 @@ static int brcm_pcie_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id brcm_pcie_match[] = {
+ { .compatible = "brcm,bcm2711-pcie", .data = &bcm2711_cfg },
+ {},
+};
+
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;
+ const struct pcie_cfg_data *data;
struct brcm_pcie *pcie;
struct pci_bus *child;
struct resource *res;
@@ -955,9 +1031,18 @@ static int brcm_pcie_probe(struct platform_device *pdev)
if (!bridge)
return -ENOMEM;
+ data = of_device_get_match_data(&pdev->dev);
+ if (!data) {
+ pr_err("failed to look up compatible string\n");
+ return -EINVAL;
+ }
+
pcie = pci_host_bridge_priv(bridge);
pcie->dev = &pdev->dev;
pcie->np = np;
+ pcie->reg_offsets = data->offsets;
+ pcie->reg_field_info = data->reg_field_info;
+ pcie->type = data->type;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
pcie->base = devm_ioremap_resource(&pdev->dev, res);
@@ -1023,10 +1108,6 @@ static int brcm_pcie_probe(struct platform_device *pdev)
return ret;
}
-static const struct of_device_id brcm_pcie_match[] = {
- { .compatible = "brcm,bcm2711-pcie" },
- {},
-};
MODULE_DEVICE_TABLE(of, brcm_pcie_match);
static struct platform_driver brcm_pcie_driver = {
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 05/12] PCI: brcmstb: Add suspend and resume pm_ops
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (3 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 04/12] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 06/12] PCI: brcmstb: Add bcm7278 PERST support Jim Quinlan
` (5 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
Broadcom Set-top (BrcmSTB) boards typically support S2, S3, and S5 suspend
and resume. Now the PCIe driver may do so as well.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 47 +++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index b7a222fde3c4..7c148eb65170 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -979,6 +979,47 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
brcm_pcie_bridge_sw_init_set(pcie, 1);
}
+static int brcm_pcie_suspend(struct device *dev)
+{
+ struct brcm_pcie *pcie = dev_get_drvdata(dev);
+
+ brcm_pcie_turn_off(pcie);
+ clk_disable_unprepare(pcie->clk);
+
+ return 0;
+}
+
+static int brcm_pcie_resume(struct device *dev)
+{
+ struct brcm_pcie *pcie = dev_get_drvdata(dev);
+ void __iomem *base;
+ u32 tmp;
+ int ret;
+
+ base = pcie->base;
+ clk_prepare_enable(pcie->clk);
+
+ /* Take bridge out of reset so we can access the SERDES reg */
+ brcm_pcie_bridge_sw_init_set(pcie, 0);
+
+ /* SERDES_IDDQ = 0 */
+ tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
+ u32p_replace_bits(&tmp, 0, PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
+ writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
+
+ /* wait for serdes to be stable */
+ udelay(100);
+
+ ret = brcm_pcie_setup(pcie);
+ if (ret)
+ return ret;
+
+ if (pcie->msi)
+ brcm_msi_set_regs(pcie->msi);
+
+ return 0;
+}
+
static void __brcm_pcie_remove(struct brcm_pcie *pcie)
{
brcm_msi_remove(pcie);
@@ -1110,12 +1151,18 @@ static int brcm_pcie_probe(struct platform_device *pdev)
MODULE_DEVICE_TABLE(of, brcm_pcie_match);
+static const struct dev_pm_ops brcm_pcie_pm_ops = {
+ .suspend_noirq = brcm_pcie_suspend,
+ .resume_noirq = brcm_pcie_resume,
+};
+
static struct platform_driver brcm_pcie_driver = {
.probe = brcm_pcie_probe,
.remove = brcm_pcie_remove,
.driver = {
.name = "brcm-pcie",
.of_match_table = brcm_pcie_match,
+ .pm = &brcm_pcie_pm_ops,
},
};
module_platform_driver(brcm_pcie_driver);
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 06/12] PCI: brcmstb: Add bcm7278 PERST support
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (4 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 05/12] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset Jim Quinlan
` (4 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
The PERST bit was moved to a different register in 7278-type STB chips. In
addition, the polarity of the bit was also changed; for other chips writing
a 1 specified assert; for 7278-type chips, writing a 0 specifies assert.
Signal-wise, PERST is an asserted-low signal.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 7c148eb65170..d0e256d8578a 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -83,6 +83,7 @@
#define PCIE_MISC_PCIE_CTRL 0x4064
#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
+#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
#define PCIE_MISC_PCIE_STATUS 0x4068
#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
@@ -685,9 +686,16 @@ static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
{
u32 tmp;
- tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
- u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
- writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+ if (pcie->type == BCM7278) {
+ /* Perst bit has moved and assert value is 0 */
+ tmp = readl(pcie->base + PCIE_MISC_PCIE_CTRL);
+ u32p_replace_bits(&tmp, !val, PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK);
+ writel(tmp, pcie->base + PCIE_MISC_PCIE_CTRL);
+ } else {
+ tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+ u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
+ writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
+ }
}
static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (5 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 06/12] PCI: brcmstb: Add bcm7278 PERST support Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes Jim Quinlan
` (3 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
Florian Fainelli, Philipp Zabel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
Some STB chips have a special purpose reset controller named RESCAL (reset
calibration). The PCIe HW can now control RESCAL to start and stop its
operation.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 81 ++++++++++++++++++++++++++-
1 file changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index d0e256d8578a..9189406fd35c 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -23,6 +23,7 @@
#include <linux/of_platform.h>
#include <linux/pci.h>
#include <linux/printk.h>
+#include <linux/reset.h>
#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -158,6 +159,16 @@
#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
#define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
+/* Rescal registers */
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
+
enum {
RGR1_SW_INIT_1,
EXT_CFG_INDEX,
@@ -248,6 +259,7 @@ struct brcm_pcie {
const int *reg_offsets;
const int *reg_field_info;
enum pcie_type type;
+ struct reset_control *rescal;
};
/*
@@ -963,6 +975,47 @@ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie)
dev_err(pcie->dev, "failed to enter low-power link state\n");
}
+static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start)
+{
+ static const u32 shifts[PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS] = {
+ PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT,
+ PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT,
+ PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT,};
+ static const u32 masks[PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS] = {
+ PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK,
+ PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK,
+ PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK,};
+ const int beg = start ? 0 : PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS - 1;
+ const int end = start ? PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS : -1;
+ u32 tmp, combined_mask = 0;
+ u32 val = !!start;
+ void __iomem *base = pcie->base;
+ int i;
+
+ for (i = beg; i != end; start ? i++ : i--) {
+ tmp = readl(base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
+ tmp = (tmp & ~masks[i]) | ((val << shifts[i]) & masks[i]);
+ writel(tmp, base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
+ usleep_range(50, 200);
+ combined_mask |= masks[i];
+ }
+
+ tmp = readl(base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
+ val = start ? combined_mask : 0;
+
+ return (tmp & combined_mask) == val ? 0 : -EIO;
+}
+
+static inline int brcm_phy_start(struct brcm_pcie *pcie)
+{
+ return pcie->rescal ? brcm_phy_cntl(pcie, 1) : 0;
+}
+
+static inline int brcm_phy_stop(struct brcm_pcie *pcie)
+{
+ return pcie->rescal ? brcm_phy_cntl(pcie, 0) : 0;
+}
+
static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
{
void __iomem *base = pcie->base;
@@ -990,11 +1043,15 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
static int brcm_pcie_suspend(struct device *dev)
{
struct brcm_pcie *pcie = dev_get_drvdata(dev);
+ int ret;
brcm_pcie_turn_off(pcie);
+ ret = brcm_phy_stop(pcie);
+ if (ret)
+ dev_err(pcie->dev, "failed to stop phy\n");
clk_disable_unprepare(pcie->clk);
- return 0;
+ return ret;
}
static int brcm_pcie_resume(struct device *dev)
@@ -1007,6 +1064,12 @@ static int brcm_pcie_resume(struct device *dev)
base = pcie->base;
clk_prepare_enable(pcie->clk);
+ ret = brcm_phy_start(pcie);
+ if (ret) {
+ dev_err(pcie->dev, "failed to start phy\n");
+ return ret;
+ }
+
/* Take bridge out of reset so we can access the SERDES reg */
brcm_pcie_bridge_sw_init_set(pcie, 0);
@@ -1032,6 +1095,9 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie)
{
brcm_msi_remove(pcie);
brcm_pcie_turn_off(pcie);
+ if (brcm_phy_stop(pcie))
+ dev_err(pcie->dev, "failed to stop phy\n");
+ reset_control_assert(pcie->rescal);
clk_disable_unprepare(pcie->clk);
}
@@ -1117,6 +1183,19 @@ static int brcm_pcie_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "could not enable clock\n");
return ret;
}
+ pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev, "rescal");
+ if (IS_ERR(pcie->rescal))
+ return PTR_ERR(pcie->rescal);
+
+ ret = reset_control_deassert(pcie->rescal);
+ if (ret)
+ dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
+
+ ret = brcm_phy_start(pcie);
+ if (ret) {
+ dev_err(pcie->dev, "failed to start phy\n");
+ return ret;
+ }
ret = brcm_pcie_setup(pcie);
if (ret)
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (6 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 10/12] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
` (2 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
BrcmSTB PCIe controllers are intimately connected to the memory
controller(s) on the SOC. There is a "viewport" for each memory controller
that allows inbound accesses to CPU memory. Each viewport's size must be
set to a power of two, and that size must be equal to or larger than the
amount of memory each controller supports.
Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++-------
1 file changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 9189406fd35c..39f77709c6a2 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -57,6 +57,8 @@
#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0
#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
+#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
+#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
#define PCIE_MEM_WIN0_LO(win) \
@@ -154,6 +156,7 @@
#define SSC_STATUS_OFFSET 0x1
#define SSC_STATUS_SSC_MASK 0x400
#define SSC_STATUS_PLL_LOCK_MASK 0x800
+#define PCIE_BRCM_MAX_MEMC 3
#define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
@@ -260,6 +263,8 @@ struct brcm_pcie {
const int *reg_field_info;
enum pcie_type type;
struct reset_control *rescal;
+ int num_memc;
+ u64 memc_size[PCIE_BRCM_MAX_MEMC];
};
/*
@@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
u64 *rc_bar2_offset)
{
struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
- struct device *dev = pcie->dev;
struct resource_entry *entry;
+ struct device *dev = pcie->dev;
+ u64 lowest_pcie_addr = ~(u64)0;
+ int ret, i = 0;
+ u64 size = 0;
- entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM);
- if (!entry)
- return -ENODEV;
+ resource_list_for_each_entry(entry, &bridge->dma_ranges) {
+ u64 pcie_beg = entry->res->start - entry->offset;
+ size += entry->res->end - entry->res->start + 1;
+ if (pcie_beg < lowest_pcie_addr)
+ lowest_pcie_addr = pcie_beg;
+ }
- /*
- * The controller expects the inbound window offset to be calculated as
- * the difference between PCIe's address space and CPU's. The offset
- * provided by the firmware is calculated the opposite way, so we
- * negate it.
- */
- *rc_bar2_offset = -entry->offset;
- *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start);
+ if (lowest_pcie_addr == ~(u64)0) {
+ dev_err(dev, "DT node has no dma-ranges\n");
+ return -EINVAL;
+ }
+
+ ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
+ PCIE_BRCM_MAX_MEMC);
+
+ if (ret <= 0) {
+ /* Make an educated guess */
+ pcie->num_memc = 1;
+ pcie->memc_size[0] = 1 << fls64(size - 1);
+ } else {
+ pcie->num_memc = ret;
+ }
+
+ /* Each memc is viewed through a "port" that is a power of 2 */
+ for (i = 0, size = 0; i < pcie->num_memc; i++)
+ size += pcie->memc_size[i];
+
+ /* System memory starts at this address in PCIe-space */
+ *rc_bar2_offset = lowest_pcie_addr;
+ /* The sum of all memc views must also be a power of 2 */
+ *rc_bar2_size = 1ULL << fls64(size - 1);
/*
* We validate the inbound memory view even though we should trust
@@ -782,12 +809,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
void __iomem *base = pcie->base;
struct device *dev = pcie->dev;
struct resource_entry *entry;
- unsigned int scb_size_val;
bool ssc_good = false;
struct resource *res;
int num_out_wins = 0;
u16 nlw, cls, lnksta;
- int i, ret;
+ int i, ret, memc;
u32 tmp, aspm_support;
/* Reset the bridge */
@@ -824,11 +850,17 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
writel(upper_32_bits(rc_bar2_offset),
base + PCIE_MISC_RC_BAR2_CONFIG_HI);
- scb_size_val = rc_bar2_size ?
- ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */
tmp = readl(base + PCIE_MISC_MISC_CTRL);
- u32p_replace_bits(&tmp, scb_size_val,
- PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
+ for (memc = 0; memc < pcie->num_memc; memc++) {
+ u32 scb_size_val = ilog2(pcie->memc_size[memc]) - 15;
+
+ if (memc == 0)
+ u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
+ else if (memc == 1)
+ u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK);
+ else if (memc == 2)
+ u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK);
+ }
writel(tmp, base + PCIE_MISC_MISC_CTRL);
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 10/12] PCI: brcmstb: Accommodate MSI for older chips
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (7 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 11/12] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 12/12] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
10 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
Older BrcmSTB chips do not have a separate register for MSI interrupts; the
MSIs are in a register that also contains unrelated interrupts. In
addition, the interrupts lie in bits [31..24] for these legacy chips. This
commit provides common code for both legacy and non-legacy MSI interrupt
registers.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 71 +++++++++++++++++++--------
1 file changed, 50 insertions(+), 21 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 39f77709c6a2..29fad717f619 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -82,7 +82,8 @@
#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048
#define PCIE_MISC_MSI_DATA_CONFIG 0x404c
-#define PCIE_MISC_MSI_DATA_CONFIG_VAL 0xffe06540
+#define PCIE_MISC_MSI_DATA_CONFIG_VAL_32 0xffe06540
+#define PCIE_MISC_MSI_DATA_CONFIG_VAL_8 0xfff86540
#define PCIE_MISC_PCIE_CTRL 0x4064
#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
@@ -94,6 +95,9 @@
#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK 0x10
#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40
+#define PCIE_MISC_REVISION 0x406c
+#define BRCM_PCIE_HW_REV_33 0x0303
+
#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070
#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000
#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0
@@ -114,10 +118,14 @@
#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x08000000
-#define PCIE_MSI_INTR2_STATUS 0x4500
-#define PCIE_MSI_INTR2_CLR 0x4508
-#define PCIE_MSI_INTR2_MASK_SET 0x4510
-#define PCIE_MSI_INTR2_MASK_CLR 0x4514
+
+#define PCIE_INTR2_CPU_BASE 0x4300
+#define PCIE_MSI_INTR2_BASE 0x4500
+/* Offsets from PCIE_INTR2_CPU_BASE and PCIE_MSI_INTR2_BASE */
+#define MSI_INT_STATUS 0x0
+#define MSI_INT_CLR 0x8
+#define MSI_INT_MASK_SET 0x10
+#define MSI_INT_MASK_CLR 0x14
#define PCIE_EXT_CFG_DATA 0x8000
@@ -132,6 +140,8 @@
/* PCIe parameters */
#define BRCM_NUM_PCIE_OUT_WINS 0x4
#define BRCM_INT_PCI_MSI_NR 32
+#define BRCM_INT_PCI_MSI_LEGACY_NR 8
+#define BRCM_INT_PCI_MSI_SHIFT 0
/* MSI target adresses */
#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL
@@ -246,6 +256,12 @@ struct brcm_msi {
int irq;
/* used indicates which MSI interrupts have been alloc'd */
unsigned long used;
+ bool legacy;
+ /* Some chips have MSIs in bits [31..24] of a shared register. */
+ int legacy_shift;
+ int nr; /* No. of MSI available, depends on chip */
+ /* This is the base pointer for interrupt status/set/clr regs */
+ void __iomem *intr_base;
};
/* Internal PCIe Host Controller Information.*/
@@ -265,6 +281,7 @@ struct brcm_pcie {
struct reset_control *rescal;
int num_memc;
u64 memc_size[PCIE_BRCM_MAX_MEMC];
+ u32 hw_rev;
};
/*
@@ -455,8 +472,10 @@ static void brcm_pcie_msi_isr(struct irq_desc *desc)
msi = irq_desc_get_handler_data(desc);
dev = msi->dev;
- status = readl(msi->base + PCIE_MSI_INTR2_STATUS);
- for_each_set_bit(bit, &status, BRCM_INT_PCI_MSI_NR) {
+ status = readl(msi->intr_base + MSI_INT_STATUS);
+ status >>= msi->legacy_shift;
+
+ for_each_set_bit(bit, &status, msi->nr) {
virq = irq_find_mapping(msi->inner_domain, bit);
if (virq)
generic_handle_irq(virq);
@@ -473,7 +492,7 @@ static void brcm_msi_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
msg->address_lo = lower_32_bits(msi->target_addr);
msg->address_hi = upper_32_bits(msi->target_addr);
- msg->data = (0xffff & PCIE_MISC_MSI_DATA_CONFIG_VAL) | data->hwirq;
+ msg->data = (0xffff & PCIE_MISC_MSI_DATA_CONFIG_VAL_32) | data->hwirq;
}
static int brcm_msi_set_affinity(struct irq_data *irq_data,
@@ -485,8 +504,9 @@ static int brcm_msi_set_affinity(struct irq_data *irq_data,
static void brcm_msi_ack_irq(struct irq_data *data)
{
struct brcm_msi *msi = irq_data_get_irq_chip_data(data);
+ const int shift_amt = data->hwirq + msi->legacy_shift;
- writel(1 << data->hwirq, msi->base + PCIE_MSI_INTR2_CLR);
+ writel(1 << shift_amt, msi->intr_base + MSI_INT_CLR);
}
@@ -502,7 +522,7 @@ static int brcm_msi_alloc(struct brcm_msi *msi)
int hwirq;
mutex_lock(&msi->lock);
- hwirq = bitmap_find_free_region(&msi->used, BRCM_INT_PCI_MSI_NR, 0);
+ hwirq = bitmap_find_free_region(&msi->used, msi->nr, 0);
mutex_unlock(&msi->lock);
return hwirq;
@@ -551,8 +571,7 @@ static int brcm_allocate_domains(struct brcm_msi *msi)
struct fwnode_handle *fwnode = of_node_to_fwnode(msi->np);
struct device *dev = msi->dev;
- msi->inner_domain = irq_domain_add_linear(NULL, BRCM_INT_PCI_MSI_NR,
- &msi_domain_ops, msi);
+ msi->inner_domain = irq_domain_add_linear(NULL, msi->nr, &msi_domain_ops, msi);
if (!msi->inner_domain) {
dev_err(dev, "failed to create IRQ domain\n");
return -ENOMEM;
@@ -589,7 +608,10 @@ static void brcm_msi_remove(struct brcm_pcie *pcie)
static void brcm_msi_set_regs(struct brcm_msi *msi)
{
- writel(0xffffffff, msi->base + PCIE_MSI_INTR2_MASK_CLR);
+ u32 val = __GENMASK(31, msi->legacy_shift);
+
+ writel(val, msi->intr_base + MSI_INT_MASK_CLR);
+ writel(val, msi->intr_base + MSI_INT_CLR);
/*
* The 0 bit of PCIE_MISC_MSI_BAR_CONFIG_LO is repurposed to MSI
@@ -600,8 +622,8 @@ static void brcm_msi_set_regs(struct brcm_msi *msi)
writel(upper_32_bits(msi->target_addr),
msi->base + PCIE_MISC_MSI_BAR_CONFIG_HI);
- writel(PCIE_MISC_MSI_DATA_CONFIG_VAL,
- msi->base + PCIE_MISC_MSI_DATA_CONFIG);
+ val = msi->legacy ? PCIE_MISC_MSI_DATA_CONFIG_VAL_8 : PCIE_MISC_MSI_DATA_CONFIG_VAL_32;
+ writel(val, msi->base + PCIE_MISC_MSI_DATA_CONFIG);
}
static int brcm_pcie_enable_msi(struct brcm_pcie *pcie)
@@ -626,6 +648,17 @@ static int brcm_pcie_enable_msi(struct brcm_pcie *pcie)
msi->np = pcie->np;
msi->target_addr = pcie->msi_target_addr;
msi->irq = irq;
+ msi->legacy = pcie->hw_rev < BRCM_PCIE_HW_REV_33;
+
+ if (msi->legacy) {
+ msi->intr_base = msi->base + PCIE_INTR2_CPU_BASE;
+ msi->nr = BRCM_INT_PCI_MSI_LEGACY_NR;
+ msi->legacy_shift = 24;
+ } else {
+ msi->intr_base = msi->base + PCIE_MSI_INTR2_BASE;
+ msi->nr = BRCM_INT_PCI_MSI_NR;
+ msi->legacy_shift = 0;
+ }
ret = brcm_allocate_domains(msi);
if (ret)
@@ -885,12 +918,6 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
tmp &= ~PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK;
writel(tmp, base + PCIE_MISC_RC_BAR3_CONFIG_LO);
- /* Mask all interrupts since we are not handling any yet */
- writel(0xffffffff, pcie->base + PCIE_MSI_INTR2_MASK_SET);
-
- /* clear any interrupts we find on boot */
- writel(0xffffffff, pcie->base + PCIE_MSI_INTR2_CLR);
-
if (pcie->gen)
brcm_pcie_set_gen(pcie, pcie->gen);
@@ -1233,6 +1260,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
if (ret)
goto fail;
+ pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
+
msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);
if (pci_msi_enabled() && msi_np == pcie->np) {
ret = brcm_pcie_enable_msi(pcie);
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 11/12] PCI: brcmstb: Set bus max burst size by chip type
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (8 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 10/12] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 12/12] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
10 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Jim Quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
From: Jim Quinlan <jquinlan@broadcom.com>
The proper value of the parameter SCB_MAX_BURST_SIZE varies per chip. The
2711 family requires 128B whereas other devices can employ 512. The
assignment is complicated by the fact that the values for this two-bit
field have different meanings;
Value Type_Generic Type_7278
00 Reserved 128B
01 128B 256B
10 256B 512B
11 512B Reserved
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 29fad717f619..02b769534e53 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -55,7 +55,7 @@
#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK 0x1000
#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK 0x2000
#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
-#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0
+
#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
@@ -847,7 +847,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
int num_out_wins = 0;
u16 nlw, cls, lnksta;
int i, ret, memc;
- u32 tmp, aspm_support;
+ u32 tmp, burst, aspm_support;
/* Reset the bridge */
brcm_pcie_bridge_sw_init_set(pcie, 1);
@@ -864,11 +864,22 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
/* Wait for SerDes to be stable */
usleep_range(100, 200);
+ /*
+ * SCB_MAX_BURST_SIZE is a two bit field. For GENERIC chips it
+ * is encoded as 0=128, 1=256, 2=512, 3=Rsvd, for BCM7278 it
+ * is encoded as 0=Rsvd, 1=128, 2=256, 3=512.
+ */
+ if (pcie->type == BCM2711)
+ burst = 0x0; /* 128B */
+ else if (pcie->type == BCM7278)
+ burst = 0x3; /* 512 bytes */
+ else
+ burst = 0x2; /* 512 bytes */
+
/* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
- u32p_replace_bits(&tmp, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128,
- PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+ u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
writel(tmp, base + PCIE_MISC_MISC_CTRL);
ret = brcm_pcie_get_rc_bar2_size_and_offset(pcie, &rc_bar2_size,
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 12/12] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (9 preceding siblings ...)
2020-06-16 20:55 ` [PATCH v5 11/12] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
@ 2020-06-16 20:55 ` Jim Quinlan
10 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-16 20:55 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
Now that the support is in place with previous commits, we add several
chips that use the BrcmSTB driver.
Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 02b769534e53..1437ad1a26c8 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1184,6 +1184,10 @@ static int brcm_pcie_remove(struct platform_device *pdev)
static const struct of_device_id brcm_pcie_match[] = {
{ .compatible = "brcm,bcm2711-pcie", .data = &bcm2711_cfg },
+ { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
+ { .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg },
+ { .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg },
+ { .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg },
{},
};
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v5 02/12] ata: ahci_brcm: Fix use of BCM7216 reset controller
2020-06-16 20:55 ` [PATCH v5 02/12] ata: ahci_brcm: Fix use of BCM7216 reset controller Jim Quinlan
@ 2020-06-16 22:04 ` Bjorn Helgaas
0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Helgaas @ 2020-06-16 22:04 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, Jens Axboe, Philipp Zabel,
Florian Fainelli, Hans de Goede,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list
On Tue, Jun 16, 2020 at 04:55:09PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
>
> A reset controller "rescal" is shared between the AHCI driver and the PCIe
> driver for the BrcmSTB 7216 chip. The code is modified to allow this
> sharing and to deassert() properly.
Use imperative mood, e.g.,
A reset controller "rescal" is shared between the AHCI driver and
the PCIe driver for the BrcmSTB 7216 chip. Use
devm_reset_control_get_optional_shared control() to handle this
sharing.
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
>
> Fixes: 272ecd60a636 ("ata: ahci_brcm: BCM7216 reset is self de-asserting")
> Fixes: c345ec6a50e9 ("ata: ahci_brcm: Support BCM7216 reset controller
> name")
Don't wrap "Fixes:" lines so it's easier to search for them.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 03/12] dt-bindings: PCI: Add bindings for more Brcmstb chips
2020-06-16 20:55 ` [PATCH v5 03/12] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
@ 2020-06-16 22:05 ` Bjorn Helgaas
0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Helgaas @ 2020-06-16 22:05 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, Florian Fainelli, Bjorn Helgaas,
Rob Herring, moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Tue, Jun 16, 2020 at 04:55:10PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
>
> - Add compatible strings for three more Broadcom STB chips: 7278, 7216,
> 7211 (STB version of RPi4).
> - add new property 'brcm,scb-sizes'
> - add new property 'resets'
> - add new property 'reset-names' for 7216 only
> - allow 'ranges' and 'dma-ranges' to have more than one item and update
> the example to show this.
Capitalize and add periods to the list items consistently.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 06/12] PCI: brcmstb: Add bcm7278 PERST support
2020-06-16 20:55 ` [PATCH v5 06/12] PCI: brcmstb: Add bcm7278 PERST support Jim Quinlan
@ 2020-06-16 22:05 ` Bjorn Helgaas
0 siblings, 0 replies; 21+ messages in thread
From: Bjorn Helgaas @ 2020-06-16 22:05 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Tue, Jun 16, 2020 at 04:55:13PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
>
> The PERST bit was moved to a different register in 7278-type STB chips. In
> addition, the polarity of the bit was also changed; for other chips writing
> a 1 specified assert; for 7278-type chips, writing a 0 specifies assert.
>
> Signal-wise, PERST is an asserted-low signal.
s/PERST/PERST#/ to match usage of the signal name in spec.
The PERST bit above is the name of a register bit, so use whatever
matches the STB spec.
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 7c148eb65170..d0e256d8578a 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -83,6 +83,7 @@
>
> #define PCIE_MISC_PCIE_CTRL 0x4064
> #define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
> +#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
>
> #define PCIE_MISC_PCIE_STATUS 0x4068
> #define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
> @@ -685,9 +686,16 @@ static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
> {
> u32 tmp;
>
> - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
> - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> + if (pcie->type == BCM7278) {
> + /* Perst bit has moved and assert value is 0 */
s/Perst/PERST/ or PERST# so it doesn't look like an English word and
to match the STB spec.
> + tmp = readl(pcie->base + PCIE_MISC_PCIE_CTRL);
> + u32p_replace_bits(&tmp, !val, PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK);
> + writel(tmp, pcie->base + PCIE_MISC_PCIE_CTRL);
> + } else {
> + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> + u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
> + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> + }
> }
>
> static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset
2020-06-16 20:55 ` [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset Jim Quinlan
@ 2020-06-16 22:05 ` Bjorn Helgaas
2020-06-17 17:32 ` Jim Quinlan
0 siblings, 1 reply; 21+ messages in thread
From: Bjorn Helgaas @ 2020-06-16 22:05 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, Florian Fainelli, Philipp Zabel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Tue, Jun 16, 2020 at 04:55:14PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
>
> Some STB chips have a special purpose reset controller named RESCAL (reset
> calibration). The PCIe HW can now control RESCAL to start and stop its
> operation.
The HW *can* now control RESCAL, but what does this patch do?
I guess maybe this patch uses RESCAL to turn on the PHY in probe and
resume and turn it off in suspend and remove?
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 81 ++++++++++++++++++++++++++-
> 1 file changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index d0e256d8578a..9189406fd35c 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -23,6 +23,7 @@
> #include <linux/of_platform.h>
> #include <linux/pci.h>
> #include <linux/printk.h>
> +#include <linux/reset.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/string.h>
> @@ -158,6 +159,16 @@
> #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
> #define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
>
> +/* Rescal registers */
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
> +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
> +
> enum {
> RGR1_SW_INIT_1,
> EXT_CFG_INDEX,
> @@ -248,6 +259,7 @@ struct brcm_pcie {
> const int *reg_offsets;
> const int *reg_field_info;
> enum pcie_type type;
> + struct reset_control *rescal;
> };
>
> /*
> @@ -963,6 +975,47 @@ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie)
> dev_err(pcie->dev, "failed to enter low-power link state\n");
> }
>
> +static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start)
> +{
> + static const u32 shifts[PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS] = {
> + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT,
> + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT,
> + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT,};
> + static const u32 masks[PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS] = {
> + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK,
> + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK,
> + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK,};
> + const int beg = start ? 0 : PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS - 1;
> + const int end = start ? PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS : -1;
> + u32 tmp, combined_mask = 0;
> + u32 val = !!start;
> + void __iomem *base = pcie->base;
> + int i;
> +
> + for (i = beg; i != end; start ? i++ : i--) {
> + tmp = readl(base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
> + tmp = (tmp & ~masks[i]) | ((val << shifts[i]) & masks[i]);
> + writel(tmp, base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
> + usleep_range(50, 200);
> + combined_mask |= masks[i];
> + }
> +
> + tmp = readl(base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
> + val = start ? combined_mask : 0;
> +
> + return (tmp & combined_mask) == val ? 0 : -EIO;
> +}
> +
> +static inline int brcm_phy_start(struct brcm_pcie *pcie)
> +{
> + return pcie->rescal ? brcm_phy_cntl(pcie, 1) : 0;
> +}
> +
> +static inline int brcm_phy_stop(struct brcm_pcie *pcie)
> +{
> + return pcie->rescal ? brcm_phy_cntl(pcie, 0) : 0;
> +}
> +
> static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
> {
> void __iomem *base = pcie->base;
> @@ -990,11 +1043,15 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
> static int brcm_pcie_suspend(struct device *dev)
> {
> struct brcm_pcie *pcie = dev_get_drvdata(dev);
> + int ret;
>
> brcm_pcie_turn_off(pcie);
> + ret = brcm_phy_stop(pcie);
> + if (ret)
> + dev_err(pcie->dev, "failed to stop phy\n");
> clk_disable_unprepare(pcie->clk);
>
> - return 0;
> + return ret;
> }
>
> static int brcm_pcie_resume(struct device *dev)
> @@ -1007,6 +1064,12 @@ static int brcm_pcie_resume(struct device *dev)
> base = pcie->base;
> clk_prepare_enable(pcie->clk);
>
> + ret = brcm_phy_start(pcie);
> + if (ret) {
> + dev_err(pcie->dev, "failed to start phy\n");
> + return ret;
> + }
> +
> /* Take bridge out of reset so we can access the SERDES reg */
> brcm_pcie_bridge_sw_init_set(pcie, 0);
>
> @@ -1032,6 +1095,9 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie)
> {
> brcm_msi_remove(pcie);
> brcm_pcie_turn_off(pcie);
> + if (brcm_phy_stop(pcie))
> + dev_err(pcie->dev, "failed to stop phy\n");
> + reset_control_assert(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
> }
>
> @@ -1117,6 +1183,19 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "could not enable clock\n");
> return ret;
> }
> + pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev, "rescal");
> + if (IS_ERR(pcie->rescal))
> + return PTR_ERR(pcie->rescal);
> +
> + ret = reset_control_deassert(pcie->rescal);
> + if (ret)
> + dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
> +
> + ret = brcm_phy_start(pcie);
> + if (ret) {
> + dev_err(pcie->dev, "failed to start phy\n");
> + return ret;
> + }
>
> ret = brcm_pcie_setup(pcie);
> if (ret)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes
2020-06-16 20:55 ` [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes Jim Quinlan
@ 2020-06-16 22:05 ` Bjorn Helgaas
2020-06-17 17:28 ` Jim Quinlan
0 siblings, 1 reply; 21+ messages in thread
From: Bjorn Helgaas @ 2020-06-16 22:05 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Tue, Jun 16, 2020 at 04:55:16PM -0400, Jim Quinlan wrote:
> BrcmSTB PCIe controllers are intimately connected to the memory
> controller(s) on the SOC. There is a "viewport" for each memory controller
> that allows inbound accesses to CPU memory. Each viewport's size must be
> set to a power of two, and that size must be equal to or larger than the
> amount of memory each controller supports.
This describes some requirements, but doesn't actually say what this
patch *does*.
I *think* it reads the viewport sizes from the "brcm,scb-sizes" DT
property instead of computing something from "dma-ranges". Looks like
it also adds support for SCB1 and SCB2.
Those seem interesting, but don't really come through in the subject
or even the commit log.
If I understand correctly, this is all for DMA ("inbound accesses to
CPU memory"). I think it would be worth mentioning "DMA", since
that's the common term for this.
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++-------
> 1 file changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 9189406fd35c..39f77709c6a2 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -57,6 +57,8 @@
> #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
> #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0
> #define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
> +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
> +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
>
> #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
> #define PCIE_MEM_WIN0_LO(win) \
> @@ -154,6 +156,7 @@
> #define SSC_STATUS_OFFSET 0x1
> #define SSC_STATUS_SSC_MASK 0x400
> #define SSC_STATUS_PLL_LOCK_MASK 0x800
> +#define PCIE_BRCM_MAX_MEMC 3
>
> #define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
> #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
> @@ -260,6 +263,8 @@ struct brcm_pcie {
> const int *reg_field_info;
> enum pcie_type type;
> struct reset_control *rescal;
> + int num_memc;
> + u64 memc_size[PCIE_BRCM_MAX_MEMC];
> };
>
> /*
> @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> u64 *rc_bar2_offset)
> {
> struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> - struct device *dev = pcie->dev;
> struct resource_entry *entry;
> + struct device *dev = pcie->dev;
> + u64 lowest_pcie_addr = ~(u64)0;
> + int ret, i = 0;
> + u64 size = 0;
>
> - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM);
> - if (!entry)
> - return -ENODEV;
> + resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> + u64 pcie_beg = entry->res->start - entry->offset;
>
> + size += entry->res->end - entry->res->start + 1;
> + if (pcie_beg < lowest_pcie_addr)
> + lowest_pcie_addr = pcie_beg;
> + }
>
> - /*
> - * The controller expects the inbound window offset to be calculated as
> - * the difference between PCIe's address space and CPU's. The offset
> - * provided by the firmware is calculated the opposite way, so we
> - * negate it.
> - */
> - *rc_bar2_offset = -entry->offset;
> - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start);
> + if (lowest_pcie_addr == ~(u64)0) {
> + dev_err(dev, "DT node has no dma-ranges\n");
> + return -EINVAL;
> + }
> +
> + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> + PCIE_BRCM_MAX_MEMC);
> +
> + if (ret <= 0) {
> + /* Make an educated guess */
> + pcie->num_memc = 1;
> + pcie->memc_size[0] = 1 << fls64(size - 1);
> + } else {
> + pcie->num_memc = ret;
> + }
> +
> + /* Each memc is viewed through a "port" that is a power of 2 */
> + for (i = 0, size = 0; i < pcie->num_memc; i++)
> + size += pcie->memc_size[i];
> +
> + /* System memory starts at this address in PCIe-space */
> + *rc_bar2_offset = lowest_pcie_addr;
> + /* The sum of all memc views must also be a power of 2 */
> + *rc_bar2_size = 1ULL << fls64(size - 1);
>
> /*
> * We validate the inbound memory view even though we should trust
> @@ -782,12 +809,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> void __iomem *base = pcie->base;
> struct device *dev = pcie->dev;
> struct resource_entry *entry;
> - unsigned int scb_size_val;
> bool ssc_good = false;
> struct resource *res;
> int num_out_wins = 0;
> u16 nlw, cls, lnksta;
> - int i, ret;
> + int i, ret, memc;
> u32 tmp, aspm_support;
>
> /* Reset the bridge */
> @@ -824,11 +850,17 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> writel(upper_32_bits(rc_bar2_offset),
> base + PCIE_MISC_RC_BAR2_CONFIG_HI);
>
> - scb_size_val = rc_bar2_size ?
> - ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */
> tmp = readl(base + PCIE_MISC_MISC_CTRL);
> - u32p_replace_bits(&tmp, scb_size_val,
> - PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> + for (memc = 0; memc < pcie->num_memc; memc++) {
> + u32 scb_size_val = ilog2(pcie->memc_size[memc]) - 15;
> +
> + if (memc == 0)
> + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> + else if (memc == 1)
> + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK);
> + else if (memc == 2)
> + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK);
> + }
> writel(tmp, base + PCIE_MISC_MISC_CTRL);
>
> /*
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes
2020-06-16 22:05 ` Bjorn Helgaas
@ 2020-06-17 17:28 ` Jim Quinlan
2020-06-18 3:01 ` Bjorn Helgaas
0 siblings, 1 reply; 21+ messages in thread
From: Jim Quinlan @ 2020-06-17 17:28 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
Christoph Hellwig, Nicolas Saenz Julienne,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
Hello Bjorn,
On Tue, Jun 16, 2020 at 6:05 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Jun 16, 2020 at 04:55:16PM -0400, Jim Quinlan wrote:
> > BrcmSTB PCIe controllers are intimately connected to the memory
> > controller(s) on the SOC. There is a "viewport" for each memory controller
> > that allows inbound accesses to CPU memory. Each viewport's size must be
> > set to a power of two, and that size must be equal to or larger than the
> > amount of memory each controller supports.
>
> This describes some requirements, but doesn't actually say what this
> patch *does*.
>
> I *think* it reads the viewport sizes from the "brcm,scb-sizes" DT
> property instead of computing something from "dma-ranges". Looks like
> it also adds support for SCB1 and SCB2.
>
> Those seem interesting, but don't really come through in the subject
> or even the commit log.
>
> If I understand correctly, this is all for DMA ("inbound accesses to
> CPU memory"). I think it would be worth mentioning "DMA", since
> that's the common term for this.
I have changed the commit message to the text below. Please let me
know if it requires more work
Thanks, Jim
PCI: brcmstb: Set internal memory DMA viewport sizes
BrcmSTB PCIe controllers are intimately connected to the memory
controller(s) on the SOC. There is a "viewport" for each memory controller
that allows inbound DMA acceses to CPU memory. Each viewport's size must
be set to a power of two, and that size must be equal to or larger than the
amount of memory each controller supports. Unfortunately the viewport
sizes cannot be ascertained from the "dma-ranges" property so they have
their own property, "brcm,scb-sizes".
There may be one to three memory controllers; they are indicated by the
term SCBi. Each controller has a base region and an optional extension
region. In physical memory, the base and extension regions are not
adjacent, but in PCIe-space they are. Further, the 1-3 viewports are also
adjacent in PCIe-space.
The SCB settings work in conjunction with the "dma-ranges' offsets to
enable non-identity mappings between system memory and PCIe space.
>
>
> > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> > drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++-------
> > 1 file changed, 50 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index 9189406fd35c..39f77709c6a2 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -57,6 +57,8 @@
> > #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
> > #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0
> > #define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
> > +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
> > +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
> >
> > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
> > #define PCIE_MEM_WIN0_LO(win) \
> > @@ -154,6 +156,7 @@
> > #define SSC_STATUS_OFFSET 0x1
> > #define SSC_STATUS_SSC_MASK 0x400
> > #define SSC_STATUS_PLL_LOCK_MASK 0x800
> > +#define PCIE_BRCM_MAX_MEMC 3
> >
> > #define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
> > #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
> > @@ -260,6 +263,8 @@ struct brcm_pcie {
> > const int *reg_field_info;
> > enum pcie_type type;
> > struct reset_control *rescal;
> > + int num_memc;
> > + u64 memc_size[PCIE_BRCM_MAX_MEMC];
> > };
> >
> > /*
> > @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> > u64 *rc_bar2_offset)
> > {
> > struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> > - struct device *dev = pcie->dev;
> > struct resource_entry *entry;
> > + struct device *dev = pcie->dev;
> > + u64 lowest_pcie_addr = ~(u64)0;
> > + int ret, i = 0;
> > + u64 size = 0;
> >
> > - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM);
> > - if (!entry)
> > - return -ENODEV;
> > + resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> > + u64 pcie_beg = entry->res->start - entry->offset;
> >
> > + size += entry->res->end - entry->res->start + 1;
> > + if (pcie_beg < lowest_pcie_addr)
> > + lowest_pcie_addr = pcie_beg;
> > + }
> >
> > - /*
> > - * The controller expects the inbound window offset to be calculated as
> > - * the difference between PCIe's address space and CPU's. The offset
> > - * provided by the firmware is calculated the opposite way, so we
> > - * negate it.
> > - */
> > - *rc_bar2_offset = -entry->offset;
> > - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start);
> > + if (lowest_pcie_addr == ~(u64)0) {
> > + dev_err(dev, "DT node has no dma-ranges\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> > + PCIE_BRCM_MAX_MEMC);
> > +
> > + if (ret <= 0) {
> > + /* Make an educated guess */
> > + pcie->num_memc = 1;
> > + pcie->memc_size[0] = 1 << fls64(size - 1);
> > + } else {
> > + pcie->num_memc = ret;
> > + }
> > +
> > + /* Each memc is viewed through a "port" that is a power of 2 */
> > + for (i = 0, size = 0; i < pcie->num_memc; i++)
> > + size += pcie->memc_size[i];
> > +
> > + /* System memory starts at this address in PCIe-space */
> > + *rc_bar2_offset = lowest_pcie_addr;
> > + /* The sum of all memc views must also be a power of 2 */
> > + *rc_bar2_size = 1ULL << fls64(size - 1);
> >
> > /*
> > * We validate the inbound memory view even though we should trust
> > @@ -782,12 +809,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > void __iomem *base = pcie->base;
> > struct device *dev = pcie->dev;
> > struct resource_entry *entry;
> > - unsigned int scb_size_val;
> > bool ssc_good = false;
> > struct resource *res;
> > int num_out_wins = 0;
> > u16 nlw, cls, lnksta;
> > - int i, ret;
> > + int i, ret, memc;
> > u32 tmp, aspm_support;
> >
> > /* Reset the bridge */
> > @@ -824,11 +850,17 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > writel(upper_32_bits(rc_bar2_offset),
> > base + PCIE_MISC_RC_BAR2_CONFIG_HI);
> >
> > - scb_size_val = rc_bar2_size ?
> > - ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */
> > tmp = readl(base + PCIE_MISC_MISC_CTRL);
> > - u32p_replace_bits(&tmp, scb_size_val,
> > - PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> > + for (memc = 0; memc < pcie->num_memc; memc++) {
> > + u32 scb_size_val = ilog2(pcie->memc_size[memc]) - 15;
> > +
> > + if (memc == 0)
> > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> > + else if (memc == 1)
> > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK);
> > + else if (memc == 2)
> > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK);
> > + }
> > writel(tmp, base + PCIE_MISC_MISC_CTRL);
> >
> > /*
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset
2020-06-16 22:05 ` Bjorn Helgaas
@ 2020-06-17 17:32 ` Jim Quinlan
0 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-17 17:32 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
Christoph Hellwig, Nicolas Saenz Julienne,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, Florian Fainelli, Philipp Zabel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Tue, Jun 16, 2020 at 6:05 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Jun 16, 2020 at 04:55:14PM -0400, Jim Quinlan wrote:
> > From: Jim Quinlan <jquinlan@broadcom.com>
> >
> > Some STB chips have a special purpose reset controller named RESCAL (reset
> > calibration). The PCIe HW can now control RESCAL to start and stop its
> > operation.
>
> The HW *can* now control RESCAL, but what does this patch do?
>
> I guess maybe this patch uses RESCAL to turn on the PHY in probe and
> resume and turn it off in suspend and remove?
Yes, I will redo this text with a better description.
Thanks,
Jim
PS Will attend to your other responses as well in the V6.
>
> > Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> > Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> > drivers/pci/controller/pcie-brcmstb.c | 81 ++++++++++++++++++++++++++-
> > 1 file changed, 80 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index d0e256d8578a..9189406fd35c 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -23,6 +23,7 @@
> > #include <linux/of_platform.h>
> > #include <linux/pci.h>
> > #include <linux/printk.h>
> > +#include <linux/reset.h>
> > #include <linux/sizes.h>
> > #include <linux/slab.h>
> > #include <linux/string.h>
> > @@ -158,6 +159,16 @@
> > #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
> > #define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
> >
> > +/* Rescal registers */
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
> > +#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
> > +
> > enum {
> > RGR1_SW_INIT_1,
> > EXT_CFG_INDEX,
> > @@ -248,6 +259,7 @@ struct brcm_pcie {
> > const int *reg_offsets;
> > const int *reg_field_info;
> > enum pcie_type type;
> > + struct reset_control *rescal;
> > };
> >
> > /*
> > @@ -963,6 +975,47 @@ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie)
> > dev_err(pcie->dev, "failed to enter low-power link state\n");
> > }
> >
> > +static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start)
> > +{
> > + static const u32 shifts[PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS] = {
> > + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT,
> > + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT,
> > + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT,};
> > + static const u32 masks[PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS] = {
> > + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK,
> > + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK,
> > + PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK,};
> > + const int beg = start ? 0 : PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS - 1;
> > + const int end = start ? PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS : -1;
> > + u32 tmp, combined_mask = 0;
> > + u32 val = !!start;
> > + void __iomem *base = pcie->base;
> > + int i;
> > +
> > + for (i = beg; i != end; start ? i++ : i--) {
> > + tmp = readl(base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
> > + tmp = (tmp & ~masks[i]) | ((val << shifts[i]) & masks[i]);
> > + writel(tmp, base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
> > + usleep_range(50, 200);
> > + combined_mask |= masks[i];
> > + }
> > +
> > + tmp = readl(base + PCIE_DVT_PMU_PCIE_PHY_CTRL);
> > + val = start ? combined_mask : 0;
> > +
> > + return (tmp & combined_mask) == val ? 0 : -EIO;
> > +}
> > +
> > +static inline int brcm_phy_start(struct brcm_pcie *pcie)
> > +{
> > + return pcie->rescal ? brcm_phy_cntl(pcie, 1) : 0;
> > +}
> > +
> > +static inline int brcm_phy_stop(struct brcm_pcie *pcie)
> > +{
> > + return pcie->rescal ? brcm_phy_cntl(pcie, 0) : 0;
> > +}
> > +
> > static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
> > {
> > void __iomem *base = pcie->base;
> > @@ -990,11 +1043,15 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
> > static int brcm_pcie_suspend(struct device *dev)
> > {
> > struct brcm_pcie *pcie = dev_get_drvdata(dev);
> > + int ret;
> >
> > brcm_pcie_turn_off(pcie);
> > + ret = brcm_phy_stop(pcie);
> > + if (ret)
> > + dev_err(pcie->dev, "failed to stop phy\n");
> > clk_disable_unprepare(pcie->clk);
> >
> > - return 0;
> > + return ret;
> > }
> >
> > static int brcm_pcie_resume(struct device *dev)
> > @@ -1007,6 +1064,12 @@ static int brcm_pcie_resume(struct device *dev)
> > base = pcie->base;
> > clk_prepare_enable(pcie->clk);
> >
> > + ret = brcm_phy_start(pcie);
> > + if (ret) {
> > + dev_err(pcie->dev, "failed to start phy\n");
> > + return ret;
> > + }
> > +
> > /* Take bridge out of reset so we can access the SERDES reg */
> > brcm_pcie_bridge_sw_init_set(pcie, 0);
> >
> > @@ -1032,6 +1095,9 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie)
> > {
> > brcm_msi_remove(pcie);
> > brcm_pcie_turn_off(pcie);
> > + if (brcm_phy_stop(pcie))
> > + dev_err(pcie->dev, "failed to stop phy\n");
> > + reset_control_assert(pcie->rescal);
> > clk_disable_unprepare(pcie->clk);
> > }
> >
> > @@ -1117,6 +1183,19 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> > dev_err(&pdev->dev, "could not enable clock\n");
> > return ret;
> > }
> > + pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev, "rescal");
> > + if (IS_ERR(pcie->rescal))
> > + return PTR_ERR(pcie->rescal);
> > +
> > + ret = reset_control_deassert(pcie->rescal);
> > + if (ret)
> > + dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
> > +
> > + ret = brcm_phy_start(pcie);
> > + if (ret) {
> > + dev_err(pcie->dev, "failed to start phy\n");
> > + return ret;
> > + }
> >
> > ret = brcm_pcie_setup(pcie);
> > if (ret)
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes
2020-06-17 17:28 ` Jim Quinlan
@ 2020-06-18 3:01 ` Bjorn Helgaas
2020-06-18 14:56 ` Jim Quinlan
0 siblings, 1 reply; 21+ messages in thread
From: Bjorn Helgaas @ 2020-06-18 3:01 UTC (permalink / raw)
To: Jim Quinlan
Cc: open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
Christoph Hellwig, Nicolas Saenz Julienne,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
On Wed, Jun 17, 2020 at 01:28:12PM -0400, Jim Quinlan wrote:
> Hello Bjorn,
>
> On Tue, Jun 16, 2020 at 6:05 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Tue, Jun 16, 2020 at 04:55:16PM -0400, Jim Quinlan wrote:
> > > BrcmSTB PCIe controllers are intimately connected to the memory
> > > controller(s) on the SOC. There is a "viewport" for each memory controller
> > > that allows inbound accesses to CPU memory. Each viewport's size must be
> > > set to a power of two, and that size must be equal to or larger than the
> > > amount of memory each controller supports.
> >
> > This describes some requirements, but doesn't actually say what this
> > patch *does*.
> >
> > I *think* it reads the viewport sizes from the "brcm,scb-sizes" DT
> > property instead of computing something from "dma-ranges". Looks like
> > it also adds support for SCB1 and SCB2.
> >
> > Those seem interesting, but don't really come through in the subject
> > or even the commit log.
> >
> > If I understand correctly, this is all for DMA ("inbound accesses to
> > CPU memory"). I think it would be worth mentioning "DMA", since
> > that's the common term for this.
>
>
> I have changed the commit message to the text below. Please let me
> know if it requires more work
> Thanks, Jim
>
> PCI: brcmstb: Set internal memory DMA viewport sizes
Did you not set the viewport sizes before?
> BrcmSTB PCIe controllers are intimately connected to the memory
> controller(s) on the SOC. There is a "viewport" for each memory controller
> that allows inbound DMA acceses to CPU memory. Each viewport's size must
> be set to a power of two, and that size must be equal to or larger than the
> amount of memory each controller supports. Unfortunately the viewport
> sizes cannot be ascertained from the "dma-ranges" property so they have
> their own property, "brcm,scb-sizes".
s/inbound DMA acceses to CPU memory/DMA/
"Accesses" is redundant since the "A" in "DMA" stands for "access".
I'm not sure "inbound" adds anything and might confuse since DMA may
be either a read or write of CPU memory.
I assume *all* drivers need to know the address and size of regions in
"dma-ranges". Is there something special about this device that means
it needs something different?
I guess it's the base/extension split? That couldn't be described as
two separate DMA ranges?
Could/should the new property have a name somehow related to
"dma-ranges"?
Should "dma-ranges" be documented in
Documentation/devicetree/bindings/pci/pci.txt instead of the
individual device bindings?
> There may be one to three memory controllers; they are indicated by the
> term SCBi. Each controller has a base region and an optional extension
> region. In physical memory, the base and extension regions are not
> adjacent, but in PCIe-space they are. Further, the 1-3 viewports are also
> adjacent in PCIe-space.
>
> The SCB settings work in conjunction with the "dma-ranges' offsets to
> enable non-identity mappings between system memory and PCIe space.
s/ranges'/ranges"/ (mismatched quotes)
This describes the hardware, but still doesn't actually say what this
patch *does*.
If I'm a user, why do I want this patch? Does it fix something that
didn't work before? Does it increase the amount of DMA-able memory?
What does this mean in terms of backwards compatibility with old DTs?
Does this work with old DTs that don't have "brcm,scb-sizes"? Maybe
this is all related to specific devices that weren't supported before,
so there *are* no old DTs for them? I can't tell from the binding
update or the patch that this is related to specific devices.
> > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > > Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> > > ---
> > > drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++-------
> > > 1 file changed, 50 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > > index 9189406fd35c..39f77709c6a2 100644
> > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > @@ -57,6 +57,8 @@
> > > #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
> > > #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0
> > > #define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
> > > +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
> > > +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
> > >
> > > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
> > > #define PCIE_MEM_WIN0_LO(win) \
> > > @@ -154,6 +156,7 @@
> > > #define SSC_STATUS_OFFSET 0x1
> > > #define SSC_STATUS_SSC_MASK 0x400
> > > #define SSC_STATUS_PLL_LOCK_MASK 0x800
> > > +#define PCIE_BRCM_MAX_MEMC 3
> > >
> > > #define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
> > > #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
> > > @@ -260,6 +263,8 @@ struct brcm_pcie {
> > > const int *reg_field_info;
> > > enum pcie_type type;
> > > struct reset_control *rescal;
> > > + int num_memc;
> > > + u64 memc_size[PCIE_BRCM_MAX_MEMC];
> > > };
> > >
> > > /*
> > > @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> > > u64 *rc_bar2_offset)
> > > {
> > > struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> > > - struct device *dev = pcie->dev;
> > > struct resource_entry *entry;
> > > + struct device *dev = pcie->dev;
> > > + u64 lowest_pcie_addr = ~(u64)0;
> > > + int ret, i = 0;
> > > + u64 size = 0;
> > >
> > > - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM);
> > > - if (!entry)
> > > - return -ENODEV;
> > > + resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> > > + u64 pcie_beg = entry->res->start - entry->offset;
> > >
> > > + size += entry->res->end - entry->res->start + 1;
> > > + if (pcie_beg < lowest_pcie_addr)
> > > + lowest_pcie_addr = pcie_beg;
> > > + }
> > >
> > > - /*
> > > - * The controller expects the inbound window offset to be calculated as
> > > - * the difference between PCIe's address space and CPU's. The offset
> > > - * provided by the firmware is calculated the opposite way, so we
> > > - * negate it.
> > > - */
> > > - *rc_bar2_offset = -entry->offset;
> > > - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start);
> > > + if (lowest_pcie_addr == ~(u64)0) {
> > > + dev_err(dev, "DT node has no dma-ranges\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> > > + PCIE_BRCM_MAX_MEMC);
> > > +
> > > + if (ret <= 0) {
> > > + /* Make an educated guess */
> > > + pcie->num_memc = 1;
> > > + pcie->memc_size[0] = 1 << fls64(size - 1);
> > > + } else {
> > > + pcie->num_memc = ret;
> > > + }
> > > +
> > > + /* Each memc is viewed through a "port" that is a power of 2 */
> > > + for (i = 0, size = 0; i < pcie->num_memc; i++)
> > > + size += pcie->memc_size[i];
> > > +
> > > + /* System memory starts at this address in PCIe-space */
> > > + *rc_bar2_offset = lowest_pcie_addr;
> > > + /* The sum of all memc views must also be a power of 2 */
> > > + *rc_bar2_size = 1ULL << fls64(size - 1);
> > >
> > > /*
> > > * We validate the inbound memory view even though we should trust
> > > @@ -782,12 +809,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > > void __iomem *base = pcie->base;
> > > struct device *dev = pcie->dev;
> > > struct resource_entry *entry;
> > > - unsigned int scb_size_val;
> > > bool ssc_good = false;
> > > struct resource *res;
> > > int num_out_wins = 0;
> > > u16 nlw, cls, lnksta;
> > > - int i, ret;
> > > + int i, ret, memc;
> > > u32 tmp, aspm_support;
> > >
> > > /* Reset the bridge */
> > > @@ -824,11 +850,17 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > > writel(upper_32_bits(rc_bar2_offset),
> > > base + PCIE_MISC_RC_BAR2_CONFIG_HI);
> > >
> > > - scb_size_val = rc_bar2_size ?
> > > - ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */
> > > tmp = readl(base + PCIE_MISC_MISC_CTRL);
> > > - u32p_replace_bits(&tmp, scb_size_val,
> > > - PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> > > + for (memc = 0; memc < pcie->num_memc; memc++) {
> > > + u32 scb_size_val = ilog2(pcie->memc_size[memc]) - 15;
> > > +
> > > + if (memc == 0)
> > > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> > > + else if (memc == 1)
> > > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK);
> > > + else if (memc == 2)
> > > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK);
> > > + }
> > > writel(tmp, base + PCIE_MISC_MISC_CTRL);
> > >
> > > /*
> > > --
> > > 2.17.1
> > >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes
2020-06-18 3:01 ` Bjorn Helgaas
@ 2020-06-18 14:56 ` Jim Quinlan
0 siblings, 0 replies; 21+ messages in thread
From: Jim Quinlan @ 2020-06-18 14:56 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
Christoph Hellwig, Nicolas Saenz Julienne,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list
Hello Bjorn,
On Wed, Jun 17, 2020 at 11:01 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, Jun 17, 2020 at 01:28:12PM -0400, Jim Quinlan wrote:
> > Hello Bjorn,
> >
> > On Tue, Jun 16, 2020 at 6:05 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > On Tue, Jun 16, 2020 at 04:55:16PM -0400, Jim Quinlan wrote:
> > > > BrcmSTB PCIe controllers are intimately connected to the memory
> > > > controller(s) on the SOC. There is a "viewport" for each memory controller
> > > > that allows inbound accesses to CPU memory. Each viewport's size must be
> > > > set to a power of two, and that size must be equal to or larger than the
> > > > amount of memory each controller supports.
> > >
> > > This describes some requirements, but doesn't actually say what this
> > > patch *does*.
> > >
> > > I *think* it reads the viewport sizes from the "brcm,scb-sizes" DT
> > > property instead of computing something from "dma-ranges". Looks like
> > > it also adds support for SCB1 and SCB2.
> > >
> > > Those seem interesting, but don't really come through in the subject
> > > or even the commit log.
> > >
> > > If I understand correctly, this is all for DMA ("inbound accesses to
> > > CPU memory"). I think it would be worth mentioning "DMA", since
> > > that's the common term for this.
> >
> >
> > I have changed the commit message to the text below. Please let me
> > know if it requires more work
> > Thanks, Jim
> >
> > PCI: brcmstb: Set internal memory DMA viewport sizes
>
> Did you not set the viewport sizes before?
Only for SCB0, and that was set from the size of the first and only
dma-range region. This was enough for the Raspberry Pi to work but it
cannot handle BrcmSTB SOCs requiring more than one dma-range region.
>
> > BrcmSTB PCIe controllers are intimately connected to the memory
> > controller(s) on the SOC. There is a "viewport" for each memory controller
> > that allows inbound DMA acceses to CPU memory. Each viewport's size must
> > be set to a power of two, and that size must be equal to or larger than the
> > amount of memory each controller supports. Unfortunately the viewport
> > sizes cannot be ascertained from the "dma-ranges" property so they have
> > their own property, "brcm,scb-sizes".
>
> s/inbound DMA acceses to CPU memory/DMA/
>
> "Accesses" is redundant since the "A" in "DMA" stands for "access".
> I'm not sure "inbound" adds anything and might confuse since DMA may
> be either a read or write of CPU memory.
>
> I assume *all* drivers need to know the address and size of regions in
> "dma-ranges". Is there something special about this device that means
> it needs something different?
All previous Linux devices required at most one dma-range (to be
precise, there could be multiple dma-ranges but they had to have the
same offset). This device may have up to six dma-ranges, most of them
having unique offsets. This is explained in a reference given in my
cover letter under v1:
https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101024@gmail.com/
>
> I guess it's the base/extension split? That couldn't be described as
> two separate DMA ranges?
Using just dma-ranges one cannot tell the difference between
dma-range[0] -- from memc0 base, 2GB
dma-range[1] -- from memc0 extension, 1GB
/* Action: SCB0 is set to 4GB */
and
dma-range[0] -- from memc0 base 2GB
dma-range[1] -- from memc1 base 1GB
/* Action: SCB0 is set to 2GB, SCB1 is set to 1GB */
>
> Could/should the new property have a name somehow related to
> "dma-ranges"?
Even though they are related, I can't think of a different name which
would be helpful. The property name "brcm,scb-sizes" describes
exactly what it is. The mapping that is necessary for BrcmSTB PCIe
drivers is best described by a picture and I don't think a better
name is going to help anyone figure out its necessity. I will greatly
enhance my commit message for this patch to describe the details you
have asked about.
>
> Should "dma-ranges" be documented in
> Documentation/devicetree/bindings/pci/pci.txt instead of the
> individual device bindings?
That file references
http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, which
defines "ranges". So the "dma-ranges" should be described in the PDF
file but is not.
"dma-ranges" is certainly defined and enforced in the PCI YAML
description (aren't we moving towards YAML anyway?).
>
> > There may be one to three memory controllers; they are indicated by the
> > term SCBi. Each controller has a base region and an optional extension
> > region. In physical memory, the base and extension regions are not
> > adjacent, but in PCIe-space they are. Further, the 1-3 viewports are also
> > adjacent in PCIe-space.
> >
> > The SCB settings work in conjunction with the "dma-ranges' offsets to
> > enable non-identity mappings between system memory and PCIe space.
>
> s/ranges'/ranges"/ (mismatched quotes)
>
> This describes the hardware, but still doesn't actually say what this
> patch *does*.
>
> If I'm a user, why do I want this patch? Does it fix something that
> didn't work before? Does it increase the amount of DMA-able memory?
BrcmSTB SOCs, with the exception of the simple memory configuration
of the Raspberry Pi, will not work without this patch. The RPI sets
the SCB0 size which is enough for it to function, but this patch is
needed for other Broadcom STB SOCs.
>
> What does this mean in terms of backwards compatibility with old DTs?
Should be backwards compatible -- there is only the RPI to worry about
and I hope at some point Nicolas can send a "Tested-by" to confirm.
> Does this work with old DTs that don't have "brcm,scb-sizes"?
It should work for RPI.
> Maybe
> this is all related to specific devices that weren't supported before,
> so there *are* no old DTs for them?
There is only the RPI DT and they do not use "brcm,scb-sizes".. I've
been trying to upstream this driver for ~3 years and it always got
NACKed because of this DMA mapping issue. Now that the OF system
parses the dma-ranges all the way up from an EP -- with possibly no DT
node -- up to the PCIe host controller, the solution in this patchset
might have a chance.
> I can't tell from the binding
> update or the patch that this is related to specific devices.
Fair enough, I will describe this in the next rev.
Thanks,
Jim Quinlan
Broadcom STB
>
> > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > > > Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> > > > ---
> > > > drivers/pci/controller/pcie-brcmstb.c | 68 ++++++++++++++++++++-------
> > > > 1 file changed, 50 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > > > index 9189406fd35c..39f77709c6a2 100644
> > > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > > @@ -57,6 +57,8 @@
> > > > #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
> > > > #define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128 0x0
> > > > #define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
> > > > +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
> > > > +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
> > > >
> > > > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
> > > > #define PCIE_MEM_WIN0_LO(win) \
> > > > @@ -154,6 +156,7 @@
> > > > #define SSC_STATUS_OFFSET 0x1
> > > > #define SSC_STATUS_SSC_MASK 0x400
> > > > #define SSC_STATUS_PLL_LOCK_MASK 0x800
> > > > +#define PCIE_BRCM_MAX_MEMC 3
> > > >
> > > > #define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
> > > > #define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
> > > > @@ -260,6 +263,8 @@ struct brcm_pcie {
> > > > const int *reg_field_info;
> > > > enum pcie_type type;
> > > > struct reset_control *rescal;
> > > > + int num_memc;
> > > > + u64 memc_size[PCIE_BRCM_MAX_MEMC];
> > > > };
> > > >
> > > > /*
> > > > @@ -715,22 +720,44 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> > > > u64 *rc_bar2_offset)
> > > > {
> > > > struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> > > > - struct device *dev = pcie->dev;
> > > > struct resource_entry *entry;
> > > > + struct device *dev = pcie->dev;
> > > > + u64 lowest_pcie_addr = ~(u64)0;
> > > > + int ret, i = 0;
> > > > + u64 size = 0;
> > > >
> > > > - entry = resource_list_first_type(&bridge->dma_ranges, IORESOURCE_MEM);
> > > > - if (!entry)
> > > > - return -ENODEV;
> > > > + resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> > > > + u64 pcie_beg = entry->res->start - entry->offset;
> > > >
> > > > + size += entry->res->end - entry->res->start + 1;
> > > > + if (pcie_beg < lowest_pcie_addr)
> > > > + lowest_pcie_addr = pcie_beg;
> > > > + }
> > > >
> > > > - /*
> > > > - * The controller expects the inbound window offset to be calculated as
> > > > - * the difference between PCIe's address space and CPU's. The offset
> > > > - * provided by the firmware is calculated the opposite way, so we
> > > > - * negate it.
> > > > - */
> > > > - *rc_bar2_offset = -entry->offset;
> > > > - *rc_bar2_size = 1ULL << fls64(entry->res->end - entry->res->start);
> > > > + if (lowest_pcie_addr == ~(u64)0) {
> > > > + dev_err(dev, "DT node has no dma-ranges\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> > > > + PCIE_BRCM_MAX_MEMC);
> > > > +
> > > > + if (ret <= 0) {
> > > > + /* Make an educated guess */
> > > > + pcie->num_memc = 1;
> > > > + pcie->memc_size[0] = 1 << fls64(size - 1);
> > > > + } else {
> > > > + pcie->num_memc = ret;
> > > > + }
> > > > +
> > > > + /* Each memc is viewed through a "port" that is a power of 2 */
> > > > + for (i = 0, size = 0; i < pcie->num_memc; i++)
> > > > + size += pcie->memc_size[i];
> > > > +
> > > > + /* System memory starts at this address in PCIe-space */
> > > > + *rc_bar2_offset = lowest_pcie_addr;
> > > > + /* The sum of all memc views must also be a power of 2 */
> > > > + *rc_bar2_size = 1ULL << fls64(size - 1);
> > > >
> > > > /*
> > > > * We validate the inbound memory view even though we should trust
> > > > @@ -782,12 +809,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > > > void __iomem *base = pcie->base;
> > > > struct device *dev = pcie->dev;
> > > > struct resource_entry *entry;
> > > > - unsigned int scb_size_val;
> > > > bool ssc_good = false;
> > > > struct resource *res;
> > > > int num_out_wins = 0;
> > > > u16 nlw, cls, lnksta;
> > > > - int i, ret;
> > > > + int i, ret, memc;
> > > > u32 tmp, aspm_support;
> > > >
> > > > /* Reset the bridge */
> > > > @@ -824,11 +850,17 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > > > writel(upper_32_bits(rc_bar2_offset),
> > > > base + PCIE_MISC_RC_BAR2_CONFIG_HI);
> > > >
> > > > - scb_size_val = rc_bar2_size ?
> > > > - ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */
> > > > tmp = readl(base + PCIE_MISC_MISC_CTRL);
> > > > - u32p_replace_bits(&tmp, scb_size_val,
> > > > - PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> > > > + for (memc = 0; memc < pcie->num_memc; memc++) {
> > > > + u32 scb_size_val = ilog2(pcie->memc_size[memc]) - 15;
> > > > +
> > > > + if (memc == 0)
> > > > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> > > > + else if (memc == 1)
> > > > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK);
> > > > + else if (memc == 2)
> > > > + u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK);
> > > > + }
> > > > writel(tmp, base + PCIE_MISC_MISC_CTRL);
> > > >
> > > > /*
> > > > --
> > > > 2.17.1
> > > >
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-06-18 14:56 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 20:55 [PATCH v5 00/12] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 01/12] PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 02/12] ata: ahci_brcm: Fix use of BCM7216 reset controller Jim Quinlan
2020-06-16 22:04 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 03/12] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 04/12] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 05/12] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 06/12] PCI: brcmstb: Add bcm7278 PERST support Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-16 20:55 ` [PATCH v5 07/12] PCI: brcmstb: Add control of rescal reset Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-17 17:32 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 09/12] PCI: brcmstb: Set internal memory viewport sizes Jim Quinlan
2020-06-16 22:05 ` Bjorn Helgaas
2020-06-17 17:28 ` Jim Quinlan
2020-06-18 3:01 ` Bjorn Helgaas
2020-06-18 14:56 ` Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 10/12] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 11/12] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
2020-06-16 20:55 ` [PATCH v5 12/12] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).