* [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
@ 2020-08-24 19:30 Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 02/11] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
` (9 more replies)
0 siblings, 10 replies; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: open list:SUPERH,
open list:REMOTE PROCESSOR REMOTEPROC SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, open list:STAGING SUBSYSTEM, Rob Herring,
Stefano Stabellini, Saravana Kannan, Bartosz Golaszewski,
Rafael J. Wysocki, open list:ACPI FOR ARM64 ACPI/arm64,
Alan Stern, open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS
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. See the 'v1' notes below for more info.
v11:
Commit: "device-mapping: Introduce DMA range map, supplanting ..."
-- Rebased to latest torvalds, Aug 20, 2020.
-- Minor change in of_dma_get_range() to satisfy the kernel's
robot tester.
-- Use of PFN_DOWN(), PFN_PHYS() instead of explicit shifts (Andy)
-- Eliminate extra return in dma_offset_from_xxx_addr() (Andy)
-- Change dma_set_offset_range() to correctly handle the case
of pre-existing DMA map and zero offset.
v10:
Commit: "device-mapping: Introduce DMA range map, supplanting ..."
-- change title of commit; "bus core:" => "device-mapping:"
-- instead of allocating the DMA map with devm, use kcalloc
and call kfree() during device_release(). (RobH) Also,
for three cases that want to use the same DMA map, copy
the dma_range_map using a helper function.
-- added a missing 'return = 0;' to of_dma_get_range(). (Nicolas)
-- removed dma_range_overlaps(); instead return error if there
is an existing DMA map. (Christoph).
Commit: "PCI: brcmstb: Set additional internal memory DMA ..."
-- Changed constant 1 to 1ULL. (Nicolas)
Commit: "ata: ahci_brcm: Fix use of BCM7216 reset controller"
This commit has been removed from this patchset and will be
submitted on its own.
v9:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- A number of code improvements were implemented as suggested by
ChristophH. Unfortunately, some of these changes reversed the
implemented suggestions of other reviewers; for example, the new
macros PFN_DMA_ADDR(), DMA_ADDR_PFN() have been pulled.
v8:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- To satisfy a specific m68 compile configuration, I moved the 'struct
bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
three inline functions for !CONFIG_HAS_DMA (kernel test robot).
-- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
a pfn_offset outside of_dma_configure() but the code offers no
insight on the size of the translation window. V7 had me using
SIZE_MAX as the size. I have since contacted the sunXi maintainer and
he said that using a size of SZ_4G would cover sunXi configurations.
v7:
Commit: "device core: Introduce DMA range map, supplanting ..."
-- remove second kcalloc/copy in device.c (AndyS)
-- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
-- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
-- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
-- Fixed compile error in "sun6i_csi.c" (kernel test robot)
Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
-- correct name of function in the commit msg (SergeiS)
v6:
Commit "device core: Introduce DMA range map":
-- of_dma_get_range() now takes a single argument and returns either
NULL, a valid map, or an ERR_PTR. (Robin)
-- offsets are no longer a PFN value but an actual address. (Robin)
-- the bus_dma_region struct stores the range size instead of
the cpu_end and pci_end values. (Robin)
-- devices that were setting a single offset with no boundaries
have been modified to have boundaries; in a few places
where this information was unavilable a /* FIXME: ... */
comment was added. (Robin)
-- dma_attach_offset_range() can be called when an offset
map already exists; if it's range is already present
nothing is done and success is returned. (Robin)
All commits:
-- Man name/style/corrections/etc changed (Bjorn)
-- rebase to Torvalds master
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 (11):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
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-mapping: Introduce DMA range map, supplanting dma_pfn_offset
PCI: brcmstb: Set additional internal memory DMA 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 | 10 +-
arch/arm/mach-keystone/keystone.c | 17 +-
arch/sh/drivers/pci/pcie-sh7786.c | 9 +-
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/base/core.c | 2 +
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 | 72 ++-
drivers/of/device.c | 43 +-
drivers/of/of_private.h | 10 +-
drivers/of/unittest.c | 34 +-
drivers/pci/controller/Kconfig | 3 +-
drivers/pci/controller/pcie-brcmstb.c | 409 +++++++++++++++---
drivers/remoteproc/remoteproc_core.c | 8 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c | 7 +-
drivers/usb/core/message.c | 9 +-
drivers/usb/core/usb.c | 7 +-
include/linux/device.h | 4 +-
include/linux/dma-direct.h | 8 +-
include/linux/dma-mapping.h | 36 ++
kernel/dma/coherent.c | 10 +-
kernel/dma/mapping.c | 66 +++
26 files changed, 668 insertions(+), 180 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v11 02/11] dt-bindings: PCI: Add bindings for more Brcmstb chips
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 03/11] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
` (8 subsequent siblings)
9 siblings, 0 replies; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Florian Fainelli, open list, Rob Herring,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Jim Quinlan, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 03/11] PCI: brcmstb: Add bcm7278 register info
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 02/11] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 15:44 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
` (7 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Jim Quinlan, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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 85fa7d54f11f..c2b3d2946a36 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;
@@ -177,6 +243,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;
};
/*
@@ -603,20 +672,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;
int ret;
@@ -953,9 +1029,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;
pcie->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pcie->base))
@@ -1000,10 +1085,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 02/11] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 03/11] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 15:56 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 05/11] PCI: brcmstb: Add bcm7278 PERST# support Jim Quinlan
` (6 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Jim Quinlan, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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 c2b3d2946a36..3d588ab7a6dd 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -978,6 +978,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);
@@ -1087,12 +1128,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 05/11] PCI: brcmstb: Add bcm7278 PERST# support
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (2 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 16:04 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset Jim Quinlan
` (5 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Jim Quinlan, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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.
Of course, PERST# is a PCIe 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 | 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 3d588ab7a6dd..acf2239b0251 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
@@ -684,9 +685,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,
@@ -771,7 +779,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
/* Reset the bridge */
brcm_pcie_bridge_sw_init_set(pcie, 1);
- brcm_pcie_perst_set(pcie, 1);
+
+ /* BCM7278 fails when PERST# is set here */
+ if (pcie->type != BCM7278)
+ brcm_pcie_perst_set(pcie, 1);
usleep_range(100, 200);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (3 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 05/11] PCI: brcmstb: Add bcm7278 PERST# support Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-08 13:32 ` Lorenzo Pieralisi
2020-09-10 16:09 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes Jim Quinlan
` (4 subsequent siblings)
9 siblings, 2 replies; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, Jim Quinlan, open list,
Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Philipp Zabel, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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. On probe(), the RESCAL is deasserted and the driver goes
through the sequence of setting registers and reading status in order to
start the internal PHY that is required for the PCIe.
Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/pci/controller/pcie-brcmstb.c | 82 ++++++++++++++++++++++++++-
1 file changed, 81 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index acf2239b0251..041b8d109563 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,
@@ -247,6 +258,7 @@ struct brcm_pcie {
const int *reg_offsets;
const int *reg_field_info;
enum pcie_type type;
+ struct reset_control *rescal;
};
/*
@@ -965,6 +977,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;
@@ -992,11 +1045,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)
@@ -1009,6 +1066,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);
@@ -1034,6 +1097,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);
}
@@ -1112,6 +1178,20 @@ 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");
+ reset_control_assert(pcie->rescal);
+ return ret;
+ }
ret = brcm_pcie_setup(pcie);
if (ret)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (4 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 16:17 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 09/11] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
` (3 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
The Raspberry Pi (RPI) is currently the only chip using this driver
(pcie-brcmstb.c). There, only one memory controller is used, without an
extension region, and the SCB0 viewport size is set to the size of the
first and only dma-range region. Other BrcmSTB SOCs have more complicated
memory configurations that require setting additional viewport sizes.
BrcmSTB PCIe controllers are intimately connected to the memory
controller(s) on the SOC. The SOC may have 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 of a controller are not adjacent, but in PCIe-space
they are.
There is a "viewport" for each memory controller that allows DMA from
endpoint devices. 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 which is the sum of base region and its optional
extension. Further, the 1-3 viewports are also adjacent in PCIe-space.
Unfortunately the viewport sizes cannot be ascertained from the
"dma-ranges" property so they have their own property, "brcm,scb-sizes".
This is because dma-range information does not indicate what memory
controller it is associated. For example, consider the following case
where the size of one dma-range is 2GB and the second dma-range is 1GB:
/* Case 1: SCB0 size set to 4GB */
dma-range0: 2GB (from memc0-base)
dma-range1: 1GB (from memc0-extension)
/* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */
dma-range0: 2GB (from memc0-base)
dma-range1: 1GB (from memc0-extension)
By just looking at the dma-ranges information, one cannot tell which
situation applies. That is why an additional property is needed. Its
length indicates the number of memory controllers being used and each value
indicates the viewport size.
Note that the RPI DT does not have a "brcm,scb-sizes" property value,
as it is assumed that it only requires one memory controller and no
extension. So the optional use of "brcm,scb-sizes" will be backwards
compatible.
One last layer of complexity exists: all of the viewports sizes must be
added and rounded up to a power of two to determine what the "BAR" size is.
Further, an offset must be given that indicates the base PCIe address of
this "BAR". The use of the term BAR is typically associated with endpoint
devices, and the term is used here because the PCIe HW may be used as an RC
or an EP. In the former case, all of the system memory appears in a single
"BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely
used in the EP role and its system of mapping memory is an artifact that
requires multiple dma-ranges regions.
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 041b8d109563..7150eaa803c2 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])
@@ -259,6 +262,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];
};
/*
@@ -714,22 +719,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] = 1ULL << 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
@@ -781,12 +808,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 */
@@ -826,11 +852,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 09/11] PCI: brcmstb: Accommodate MSI for older chips
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (5 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 16:20 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 10/11] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
` (2 subsequent siblings)
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Jim Quinlan, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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 7150eaa803c2..61c1be766522 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.*/
@@ -264,6 +280,7 @@ struct brcm_pcie {
struct reset_control *rescal;
int num_memc;
u64 memc_size[PCIE_BRCM_MAX_MEMC];
+ u32 hw_rev;
};
/*
@@ -454,8 +471,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);
@@ -472,7 +491,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,
@@ -484,8 +503,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);
}
@@ -501,7 +521,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;
@@ -550,8 +570,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;
@@ -588,7 +607,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
@@ -599,8 +621,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)
@@ -625,6 +647,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)
@@ -887,12 +920,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);
@@ -1229,6 +1256,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 10/11] PCI: brcmstb: Set bus max burst size by chip type
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (6 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 09/11] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 16:22 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 11/11] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
2020-08-25 17:40 ` [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Florian Fainelli
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Jim Quinlan, Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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 61c1be766522..0d8234da519c 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
@@ -846,7 +846,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);
@@ -866,11 +866,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v11 11/11] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (7 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 10/11] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
@ 2020-08-24 19:30 ` Jim Quinlan
2020-09-10 16:23 ` Rob Herring
2020-08-25 17:40 ` [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Florian Fainelli
9 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-24 19:30 UTC (permalink / raw)
To: linux-pci, Nicolas Saenz Julienne, Christoph Hellwig,
Robin Murphy, bcm-kernel-feedback-list, james.quinlan
Cc: Rob Herring, Lorenzo Pieralisi, open list, Florian Fainelli,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
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 0d8234da519c..ca4023423acb 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1187,6 +1187,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
` (8 preceding siblings ...)
2020-08-24 19:30 ` [PATCH v11 11/11] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
@ 2020-08-25 17:40 ` Florian Fainelli
2020-08-27 6:35 ` Christoph Hellwig
9 siblings, 1 reply; 36+ messages in thread
From: Florian Fainelli @ 2020-08-25 17:40 UTC (permalink / raw)
To: Jim Quinlan, linux-pci, Nicolas Saenz Julienne,
Christoph Hellwig, Robin Murphy, bcm-kernel-feedback-list
Cc: open list:SUPERH,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, open list:STAGING SUBSYSTEM, Rob Herring,
Stefano Stabellini, Saravana Kannan, Bartosz Golaszewski,
Rafael J. Wysocki, open list:ACPI FOR ARM64 (ACPI/arm64),
Alan Stern, open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS
Hi,
On 8/24/2020 12:30 PM, Jim Quinlan wrote:
>
> 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. See the 'v1' notes below for more info.
We are version 11 and counting, and it is not clear to me whether there
is any chance of getting these patches reviewed and hopefully merged for
the 5.10 merge window.
There are a lot of different files being touched, so what would be the
ideal way of routing those changes towards inclusion?
Thanks!
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-08-25 17:40 ` [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Florian Fainelli
@ 2020-08-27 6:35 ` Christoph Hellwig
2020-08-27 13:29 ` Jim Quinlan
0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2020-08-27 6:35 UTC (permalink / raw)
To: Florian Fainelli
Cc: open list:SUPERH, linux-pci,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, Christoph Hellwig, open list:STAGING SUBSYSTEM,
Rob Herring, Stefano Stabellini, Saravana Kannan,
Bartosz Golaszewski, Rafael J. Wysocki,
open list:ACPI FOR ARM64 (ACPI/arm64),
bcm-kernel-feedback-list, Alan Stern,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Jim Quinlan, Robin Murphy,
Nicolas Saenz Julienne
On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:
> Hi,
>
> On 8/24/2020 12:30 PM, Jim Quinlan wrote:
>>
>> 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. See the 'v1' notes below for more info.
>
> We are version 11 and counting, and it is not clear to me whether there is
> any chance of getting these patches reviewed and hopefully merged for the
> 5.10 merge window.
>
> There are a lot of different files being touched, so what would be the
> ideal way of routing those changes towards inclusion?
FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
I have a bit of a backlog, but plan to review and if Jim is ok with that
apply the current version.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-08-27 6:35 ` Christoph Hellwig
@ 2020-08-27 13:29 ` Jim Quinlan
2020-09-07 9:16 ` Lorenzo Pieralisi
0 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-08-27 13:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: open list:SUPERH,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, open list:STAGING SUBSYSTEM, Rob Herring,
Florian Fainelli, Saravana Kannan, Bartosz Golaszewski,
Rafael J. Wysocki, open list:ACPI FOR ARM64 (ACPI/arm64),
Alan Stern, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
Stefano Stabellini,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Robin Murphy,
Nicolas Saenz Julienne
On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:
> > Hi,
> >
> > On 8/24/2020 12:30 PM, Jim Quinlan wrote:
> >>
> >> 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. See the 'v1' notes below for more info.
> >
> > We are version 11 and counting, and it is not clear to me whether there is
> > any chance of getting these patches reviewed and hopefully merged for the
> > 5.10 merge window.
> >
> > There are a lot of different files being touched, so what would be the
> > ideal way of routing those changes towards inclusion?
>
> FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
> I have a bit of a backlog, but plan to review and if Jim is ok with that
> apply the current version.
Sounds good to me.
Thanks, Jim
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-08-27 13:29 ` Jim Quinlan
@ 2020-09-07 9:16 ` Lorenzo Pieralisi
2020-09-07 17:43 ` Jim Quinlan
0 siblings, 1 reply; 36+ messages in thread
From: Lorenzo Pieralisi @ 2020-09-07 9:16 UTC (permalink / raw)
To: Jim Quinlan
Cc: open list:SUPERH,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, Christoph Hellwig, open list:STAGING SUBSYSTEM,
Rob Herring, Florian Fainelli, Saravana Kannan,
Bartosz Golaszewski, Rafael J. Wysocki,
open list:ACPI FOR ARM64 (ACPI/arm64),
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Alan Stern,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
Stefano Stabellini,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Robin Murphy,
Nicolas Saenz Julienne
On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote:
> On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:
> > > Hi,
> > >
> > > On 8/24/2020 12:30 PM, Jim Quinlan wrote:
> > >>
> > >> 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. See the 'v1' notes below for more info.
> > >
> > > We are version 11 and counting, and it is not clear to me whether there is
> > > any chance of getting these patches reviewed and hopefully merged for the
> > > 5.10 merge window.
> > >
> > > There are a lot of different files being touched, so what would be the
> > > ideal way of routing those changes towards inclusion?
> >
> > FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
> > I have a bit of a backlog, but plan to review and if Jim is ok with that
> > apply the current version.
> Sounds good to me.
Hi Jim,
is the dependency now solved ? Should we review/take this series as
is for v5.10 through the PCI tree ?
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-09-07 9:16 ` Lorenzo Pieralisi
@ 2020-09-07 17:43 ` Jim Quinlan
2020-09-07 18:29 ` Florian Fainelli
0 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-09-07 17:43 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: open list:SUPERH,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, Christoph Hellwig, open list:STAGING SUBSYSTEM,
Rob Herring, Florian Fainelli, Saravana Kannan,
Bartosz Golaszewski, Rafael J. Wysocki,
open list:ACPI FOR ARM64 (ACPI/arm64),
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Alan Stern,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
Stefano Stabellini,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Robin Murphy,
Nicolas Saenz Julienne
On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote:
> > On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:
> > > > Hi,
> > > >
> > > > On 8/24/2020 12:30 PM, Jim Quinlan wrote:
> > > >>
> > > >> 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. See the 'v1' notes below for more info.
> > > >
> > > > We are version 11 and counting, and it is not clear to me whether there is
> > > > any chance of getting these patches reviewed and hopefully merged for the
> > > > 5.10 merge window.
> > > >
> > > > There are a lot of different files being touched, so what would be the
> > > > ideal way of routing those changes towards inclusion?
> > >
> > > FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
> > > I have a bit of a backlog, but plan to review and if Jim is ok with that
> > > apply the current version.
> > Sounds good to me.
>
> Hi Jim,
>
> is the dependency now solved ? Should we review/take this series as
> is for v5.10 through the PCI tree ?
Hello Lorenzo,
We are still working out a regression with the DMA offset commit on
the RaspberryPi. Nicolas has found the root cause and we are now
devising a solution.
Thanks,
Jim Quinlan
Broadcom STB
>
> Thanks,
> Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-09-07 17:43 ` Jim Quinlan
@ 2020-09-07 18:29 ` Florian Fainelli
2020-09-08 10:42 ` Lorenzo Pieralisi
0 siblings, 1 reply; 36+ messages in thread
From: Florian Fainelli @ 2020-09-07 18:29 UTC (permalink / raw)
To: Jim Quinlan, Lorenzo Pieralisi
Cc: open list:SUPERH,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, Christoph Hellwig, open list:STAGING SUBSYSTEM,
Rob Herring, Florian Fainelli, Saravana Kannan,
Bartosz Golaszewski, Rafael J. Wysocki,
open list:ACPI FOR ARM64 (ACPI/arm64),
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Alan Stern,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
Stefano Stabellini,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Robin Murphy,
Nicolas Saenz Julienne
On 9/7/2020 10:43 AM, Jim Quinlan wrote:
> On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
>>
>> On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote:
>>> On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig <hch@lst.de> wrote:
>>>>
>>>> On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:
>>>>> Hi,
>>>>>
>>>>> On 8/24/2020 12:30 PM, Jim Quinlan wrote:
>>>>>>
>>>>>> 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. See the 'v1' notes below for more info.
>>>>>
>>>>> We are version 11 and counting, and it is not clear to me whether there is
>>>>> any chance of getting these patches reviewed and hopefully merged for the
>>>>> 5.10 merge window.
>>>>>
>>>>> There are a lot of different files being touched, so what would be the
>>>>> ideal way of routing those changes towards inclusion?
>>>>
>>>> FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
>>>> I have a bit of a backlog, but plan to review and if Jim is ok with that
>>>> apply the current version.
>>> Sounds good to me.
>>
>> Hi Jim,
>>
>> is the dependency now solved ? Should we review/take this series as
>> is for v5.10 through the PCI tree ?
> Hello Lorenzo,
>
> We are still working out a regression with the DMA offset commit on
> the RaspberryPi. Nicolas has found the root cause and we are now
> devising a solution.
Maybe we can parallelize the PCIe driver review while the DMA changes
are being worked on in Christoph's branch. Lorenzo, are you fine with
the PCIe changes proper?
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-09-07 18:29 ` Florian Fainelli
@ 2020-09-08 10:42 ` Lorenzo Pieralisi
2020-09-08 12:20 ` Christoph Hellwig
0 siblings, 1 reply; 36+ messages in thread
From: Lorenzo Pieralisi @ 2020-09-08 10:42 UTC (permalink / raw)
To: Florian Fainelli
Cc: open list:SUPERH,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, Christoph Hellwig, open list:STAGING SUBSYSTEM,
Rob Herring, Stefano Stabellini, Saravana Kannan,
Bartosz Golaszewski, Rafael J. Wysocki,
open list:ACPI FOR ARM64 (ACPI/arm64),
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Alan Stern,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Jim Quinlan, Robin Murphy,
Nicolas Saenz Julienne
On Mon, Sep 07, 2020 at 11:29:06AM -0700, Florian Fainelli wrote:
>
>
> On 9/7/2020 10:43 AM, Jim Quinlan wrote:
> > On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > >
> > > On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote:
> > > > On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig <hch@lst.de> wrote:
> > > > >
> > > > > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 8/24/2020 12:30 PM, Jim Quinlan wrote:
> > > > > > >
> > > > > > > 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. See the 'v1' notes below for more info.
> > > > > >
> > > > > > We are version 11 and counting, and it is not clear to me whether there is
> > > > > > any chance of getting these patches reviewed and hopefully merged for the
> > > > > > 5.10 merge window.
> > > > > >
> > > > > > There are a lot of different files being touched, so what would be the
> > > > > > ideal way of routing those changes towards inclusion?
> > > > >
> > > > > FYI, I offered to take the dma-mapping bits through the dma-mapping tree.
> > > > > I have a bit of a backlog, but plan to review and if Jim is ok with that
> > > > > apply the current version.
> > > > Sounds good to me.
> > >
> > > Hi Jim,
> > >
> > > is the dependency now solved ? Should we review/take this series as
> > > is for v5.10 through the PCI tree ?
> > Hello Lorenzo,
> >
> > We are still working out a regression with the DMA offset commit on
> > the RaspberryPi. Nicolas has found the root cause and we are now
> > devising a solution.
>
> Maybe we can parallelize the PCIe driver review while the DMA changes
> are being worked on in Christoph's branch. Lorenzo, are you fine with
> the PCIe changes proper?
I will have a look - the main contentious point was about the DMA
changes - if Christoph is happy with them I am OK with them
too - I hope there is not anything controversial in the host
bridge driver itself but I will look into it.
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
2020-09-08 10:42 ` Lorenzo Pieralisi
@ 2020-09-08 12:20 ` Christoph Hellwig
0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2020-09-08 12:20 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: open list:SUPERH,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:DRM DRIVERS FOR ALLWINNER A10, Julien Grall,
H. Peter Anvin, Christoph Hellwig, open list:STAGING SUBSYSTEM,
Rob Herring, Florian Fainelli, Saravana Kannan,
Bartosz Golaszewski, Rafael J. Wysocki,
open list:ACPI FOR ARM64 (ACPI/arm64),
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Alan Stern,
open list:ALLWINNER A10 CSI DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, Joerg Roedel,
Stefano Stabellini,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Dan Williams, Andy Shevchenko, moderated list:ARM PORT,
Felipe Balbi, Greg Kroah-Hartman, open list:USB SUBSYSTEM,
open list, open list:IOMMU DRIVERS, Jim Quinlan, Robin Murphy,
Nicolas Saenz Julienne
On Tue, Sep 08, 2020 at 11:42:26AM +0100, Lorenzo Pieralisi wrote:
> > Maybe we can parallelize the PCIe driver review while the DMA changes
> > are being worked on in Christoph's branch. Lorenzo, are you fine with
> > the PCIe changes proper?
>
> I will have a look - the main contentious point was about the DMA
> changes - if Christoph is happy with them I am OK with them
> too - I hope there is not anything controversial in the host
> bridge driver itself but I will look into it.
I'm pretty happy with the overall shape. Now we just need to squeeze
out the regressions..
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset
2020-08-24 19:30 ` [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset Jim Quinlan
@ 2020-09-08 13:32 ` Lorenzo Pieralisi
2020-09-10 16:09 ` Rob Herring
1 sibling, 0 replies; 36+ messages in thread
From: Lorenzo Pieralisi @ 2020-09-08 13:32 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Rob Herring, Florian Fainelli, linux-pci, open list,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Philipp Zabel, Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, Aug 24, 2020 at 03:30:19PM -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. On probe(), the RESCAL is deasserted and the driver goes
> through the sequence of setting registers and reading status in order to
> start the internal PHY that is required for the PCIe.
>
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 82 ++++++++++++++++++++++++++-
> 1 file changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index acf2239b0251..041b8d109563 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,
> @@ -247,6 +258,7 @@ struct brcm_pcie {
> const int *reg_offsets;
> const int *reg_field_info;
> enum pcie_type type;
> + struct reset_control *rescal;
> };
>
> /*
> @@ -965,6 +977,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;
Quick note: I think it would be nicer to init val within the loop
below (IIUC how this works):
val = start ? BIT_MASK(shifts[i]) : 0;
and then use it in the tmp computation but I leave it up to you.
Lorenzo
> + 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;
> @@ -992,11 +1045,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)
> @@ -1009,6 +1066,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);
>
> @@ -1034,6 +1097,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);
> }
>
> @@ -1112,6 +1178,20 @@ 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");
> + reset_control_assert(pcie->rescal);
> + return ret;
> + }
>
> ret = brcm_pcie_setup(pcie);
> if (ret)
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 03/11] PCI: brcmstb: Add bcm7278 register info
2020-08-24 19:30 ` [PATCH v11 03/11] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
@ 2020-09-10 15:44 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 15:44 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi, linux-pci, open list, Florian Fainelli,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, Aug 24, 2020 at 03:30:16PM -0400, Jim Quinlan wrote:
> 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(-)
I think a better abstraction would be to have 2 versions of the 2
functions that need to be different. But as-is is fine.
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-08-24 19:30 ` [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
@ 2020-09-10 15:56 ` Rob Herring
2020-09-10 16:42 ` Jim Quinlan
2020-09-10 18:47 ` Florian Fainelli
0 siblings, 2 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 15:56 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi, linux-pci, open list, Florian Fainelli,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
> 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 c2b3d2946a36..3d588ab7a6dd 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -978,6 +978,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);
Really needs to be a spinloop?
> +
> + 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);
> @@ -1087,12 +1128,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,
Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
and 1352 of .suspend in the tree.
Is doing a clk unprepare even safe in .suspend_noirq? IIRC,
prepare/unprepare can sleep.
> +};
> +
> 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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 05/11] PCI: brcmstb: Add bcm7278 PERST# support
2020-08-24 19:30 ` [PATCH v11 05/11] PCI: brcmstb: Add bcm7278 PERST# support Jim Quinlan
@ 2020-09-10 16:04 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 16:04 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi, linux-pci, open list, Florian Fainelli,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, Aug 24, 2020 at 03:30:18PM -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.
>
> Of course, PERST# is a PCIe 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 | 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 3d588ab7a6dd..acf2239b0251 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
> @@ -684,9 +685,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));
Humm, now we have a mixture of a code path based on the chip and
variables to abstract the register details. Just do a function per chip.
I have some notion to abstract out the PERST# handling from the host
bridges. We have several cases of GPIO based handling and random
assertion times. So having an ops function here will move in that
direction.
> + }
> }
>
> static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> @@ -771,7 +779,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>
> /* Reset the bridge */
> brcm_pcie_bridge_sw_init_set(pcie, 1);
> - brcm_pcie_perst_set(pcie, 1);
If these 2 functions are always called together, then you just need 1
per chip function.
> +
> + /* BCM7278 fails when PERST# is set here */
> + if (pcie->type != BCM7278)
> + brcm_pcie_perst_set(pcie, 1);
>
> usleep_range(100, 200);
>
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset
2020-08-24 19:30 ` [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset Jim Quinlan
2020-09-08 13:32 ` Lorenzo Pieralisi
@ 2020-09-10 16:09 ` Rob Herring
1 sibling, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 16:09 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi, linux-pci, open list, Florian Fainelli,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Philipp Zabel, Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, Aug 24, 2020 at 03:30:19PM -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. On probe(), the RESCAL is deasserted and the driver goes
> through the sequence of setting registers and reading status in order to
> start the internal PHY that is required for the PCIe.
>
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 82 ++++++++++++++++++++++++++-
> 1 file changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index acf2239b0251..041b8d109563 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,
> @@ -247,6 +258,7 @@ struct brcm_pcie {
> const int *reg_offsets;
> const int *reg_field_info;
> enum pcie_type type;
> + struct reset_control *rescal;
> };
>
> /*
> @@ -965,6 +977,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;
> @@ -992,11 +1045,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)
> @@ -1009,6 +1066,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);
>
> @@ -1034,6 +1097,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);
> }
>
> @@ -1112,6 +1178,20 @@ 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");
4 calls to brcm_phy_cntl() and 4 error prints. Move the error print to
brcm_phy_cntl.
> + reset_control_assert(pcie->rescal);
> + return ret;
> + }
>
> ret = brcm_pcie_setup(pcie);
> if (ret)
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes
2020-08-24 19:30 ` [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes Jim Quinlan
@ 2020-09-10 16:17 ` Rob Herring
2020-09-11 15:28 ` Jim Quinlan
0 siblings, 1 reply; 36+ messages in thread
From: Rob Herring @ 2020-09-10 16:17 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi, linux-pci, open list, Florian Fainelli,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, Aug 24, 2020 at 03:30:21PM -0400, Jim Quinlan wrote:
> The Raspberry Pi (RPI) is currently the only chip using this driver
> (pcie-brcmstb.c). There, only one memory controller is used, without an
> extension region, and the SCB0 viewport size is set to the size of the
> first and only dma-range region. Other BrcmSTB SOCs have more complicated
> memory configurations that require setting additional viewport sizes.
>
> BrcmSTB PCIe controllers are intimately connected to the memory
> controller(s) on the SOC. The SOC may have 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 of a controller are not adjacent, but in PCIe-space
> they are.
>
> There is a "viewport" for each memory controller that allows DMA from
> endpoint devices. 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 which is the sum of base region and its optional
> extension. Further, the 1-3 viewports are also adjacent in PCIe-space.
>
> Unfortunately the viewport sizes cannot be ascertained from the
> "dma-ranges" property so they have their own property, "brcm,scb-sizes".
> This is because dma-range information does not indicate what memory
> controller it is associated. For example, consider the following case
> where the size of one dma-range is 2GB and the second dma-range is 1GB:
>
> /* Case 1: SCB0 size set to 4GB */
> dma-range0: 2GB (from memc0-base)
> dma-range1: 1GB (from memc0-extension)
>
> /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */
> dma-range0: 2GB (from memc0-base)
> dma-range1: 1GB (from memc0-extension)
>
> By just looking at the dma-ranges information, one cannot tell which
> situation applies. That is why an additional property is needed. Its
> length indicates the number of memory controllers being used and each value
> indicates the viewport size.
>
> Note that the RPI DT does not have a "brcm,scb-sizes" property value,
> as it is assumed that it only requires one memory controller and no
> extension. So the optional use of "brcm,scb-sizes" will be backwards
> compatible.
>
> One last layer of complexity exists: all of the viewports sizes must be
> added and rounded up to a power of two to determine what the "BAR" size is.
> Further, an offset must be given that indicates the base PCIe address of
> this "BAR". The use of the term BAR is typically associated with endpoint
> devices, and the term is used here because the PCIe HW may be used as an RC
> or an EP. In the former case, all of the system memory appears in a single
> "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely
> used in the EP role and its system of mapping memory is an artifact that
> requires multiple dma-ranges regions.
>
> 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 041b8d109563..7150eaa803c2 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
Perhaps make 0-2 an arg and then you can just do:
u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB_SIZE_MASK(memc))
>
> #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])
> @@ -259,6 +262,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];
> };
>
> /*
> @@ -714,22 +719,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] = 1ULL << fls64(size - 1);
Use roundup_pow_of_two()
> + } 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);
Use roundup_pow_of_two()
>
> /*
> * We validate the inbound memory view even though we should trust
> @@ -781,12 +808,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 */
> @@ -826,11 +852,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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 09/11] PCI: brcmstb: Accommodate MSI for older chips
2020-08-24 19:30 ` [PATCH v11 09/11] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
@ 2020-09-10 16:20 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 16:20 UTC (permalink / raw)
To: Jim Quinlan
Cc: Florian Fainelli, linux-pci, open list, Lorenzo Pieralisi,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
On Mon, 24 Aug 2020 15:30:22 -0400, Jim Quinlan wrote:
> 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(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 10/11] PCI: brcmstb: Set bus max burst size by chip type
2020-08-24 19:30 ` [PATCH v11 10/11] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
@ 2020-09-10 16:22 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 16:22 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Florian Fainelli, linux-pci, open list, Lorenzo Pieralisi,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Mon, 24 Aug 2020 15:30:23 -0400, Jim Quinlan wrote:
> 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(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 11/11] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
2020-08-24 19:30 ` [PATCH v11 11/11] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
@ 2020-09-10 16:23 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 16:23 UTC (permalink / raw)
To: Jim Quinlan
Cc: Lorenzo Pieralisi, linux-pci, open list, Nicolas Saenz Julienne,
Florian Fainelli, bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
On Mon, 24 Aug 2020 15:30:24 -0400, Jim Quinlan wrote:
> 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(+)
>
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 15:56 ` Rob Herring
@ 2020-09-10 16:42 ` Jim Quinlan
2020-09-10 18:50 ` Rob Herring
2020-09-10 18:47 ` Florian Fainelli
1 sibling, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-09-10 16:42 UTC (permalink / raw)
To: Rob Herring
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
[-- Attachment #1.1: Type: text/plain, Size: 3218 bytes --]
On Thu, Sep 10, 2020 at 11:56 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
> > 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 c2b3d2946a36..3d588ab7a6dd 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -978,6 +978,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);
>
> Really needs to be a spinloop?
>
> > +
> > + 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);
> > @@ -1087,12 +1128,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,
>
> Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
> and 1352 of .suspend in the tree.
I will test switching this to suspend_late/resume_early.
Thanks,
Jim Quinlan
Broadcom STB
>
> Is doing a clk unprepare even safe in .suspend_noirq? IIRC,
> prepare/unprepare can sleep.
>
> > +};
> > +
> > 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
> >
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 15:56 ` Rob Herring
2020-09-10 16:42 ` Jim Quinlan
@ 2020-09-10 18:47 ` Florian Fainelli
1 sibling, 0 replies; 36+ messages in thread
From: Florian Fainelli @ 2020-09-10 18:47 UTC (permalink / raw)
To: Rob Herring, Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi, linux-pci, open list,
bcm-kernel-feedback-list,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On 9/10/2020 8:56 AM, Rob Herring wrote:
> On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
>> 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 c2b3d2946a36..3d588ab7a6dd 100644
>> --- a/drivers/pci/controller/pcie-brcmstb.c
>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>> @@ -978,6 +978,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);
>
> Really needs to be a spinloop?
>
>> +
>> + 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);
>> @@ -1087,12 +1128,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,
>
> Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
> and 1352 of .suspend in the tree.
>
> Is doing a clk unprepare even safe in .suspend_noirq? IIRC,
> prepare/unprepare can sleep.
Yes, it is safe, provided that your clock provider (clk-scmi.c in our
case) supports it, too. In our case the underlying mailbox driver has
its interrupts flagged with IRQF_NOSUSPEND such that they can still be
processed at _noirq time.
I think the rationale was to ensure that this would be done much later
after other subsystem have been made quiescent, but given the Linux
device driver model, the PCI bridge should be suspended after all
pci_device child device, so it should be safe not to use _noirq.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 16:42 ` Jim Quinlan
@ 2020-09-10 18:50 ` Rob Herring
2020-09-10 18:54 ` Florian Fainelli
2020-09-10 19:05 ` Jim Quinlan
0 siblings, 2 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-10 18:50 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Thu, Sep 10, 2020 at 10:42 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
>
> On Thu, Sep 10, 2020 at 11:56 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
> > > 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 c2b3d2946a36..3d588ab7a6dd 100644
> > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > @@ -978,6 +978,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);
> >
> > Really needs to be a spinloop?
> >
> > > +
> > > + 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);
> > > @@ -1087,12 +1128,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,
> >
> > Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
> > and 1352 of .suspend in the tree.
>
> I will test switching this to suspend_late/resume_early.
Why not just the 'regular' flavor suspend/resume?
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 18:50 ` Rob Herring
@ 2020-09-10 18:54 ` Florian Fainelli
2020-09-10 19:05 ` Jim Quinlan
1 sibling, 0 replies; 36+ messages in thread
From: Florian Fainelli @ 2020-09-10 18:54 UTC (permalink / raw)
To: Rob Herring, Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On 9/10/2020 11:50 AM, Rob Herring wrote:
> On Thu, Sep 10, 2020 at 10:42 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
>>
>> On Thu, Sep 10, 2020 at 11:56 AM Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
>>>> 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 c2b3d2946a36..3d588ab7a6dd 100644
>>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>>> @@ -978,6 +978,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);
>>>
>>> Really needs to be a spinloop?
>>>
>>>> +
>>>> + 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);
>>>> @@ -1087,12 +1128,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,
>>>
>>> Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
>>> and 1352 of .suspend in the tree.
>>
>> I will test switching this to suspend_late/resume_early.
>
> Why not just the 'regular' flavor suspend/resume?
We must have inherited this from when the driver was not a
platform_device back in our 3.14 downstream kernel and we used
syscore_ops to do the system suspend/resume.
Later on, we sort of mechanically made those _noirq() to preserve the
semantics of syscore_ops, but in hindsight it should not be necessary,
the regular suspsend/resume should work and the device driver model
ordering between parent/child should take care of the bridge being
suspended last within the PCI bus type that is.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 18:50 ` Rob Herring
2020-09-10 18:54 ` Florian Fainelli
@ 2020-09-10 19:05 ` Jim Quinlan
2020-09-10 19:07 ` Florian Fainelli
1 sibling, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-09-10 19:05 UTC (permalink / raw)
To: Rob Herring
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
[-- Attachment #1.1: Type: text/plain, Size: 3467 bytes --]
On Thu, Sep 10, 2020 at 2:50 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Sep 10, 2020 at 10:42 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
> >
> > On Thu, Sep 10, 2020 at 11:56 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
> > > > 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 c2b3d2946a36..3d588ab7a6dd 100644
> > > > --- a/drivers/pci/controller/pcie-brcmstb.c
> > > > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > > > @@ -978,6 +978,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);
> > >
> > > Really needs to be a spinloop?
> > >
> > > > +
> > > > + 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);
> > > > @@ -1087,12 +1128,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,
> > >
> > > Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
> > > and 1352 of .suspend in the tree.
> >
> > I will test switching this to suspend_late/resume_early.
>
> Why not just the 'regular' flavor suspend/resume?
>
> Rob
We must have our PCIe driver suspend last and resume first because our
current driver turns off/on the power for the EPs. Note that this
code isn't in the driver as we are still figuring out a way to make it
upstreamable.
Jim
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 19:05 ` Jim Quinlan
@ 2020-09-10 19:07 ` Florian Fainelli
2020-09-10 19:09 ` Jim Quinlan
0 siblings, 1 reply; 36+ messages in thread
From: Florian Fainelli @ 2020-09-10 19:07 UTC (permalink / raw)
To: Jim Quinlan, Rob Herring
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On 9/10/2020 12:05 PM, Jim Quinlan wrote:
> On Thu, Sep 10, 2020 at 2:50 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Thu, Sep 10, 2020 at 10:42 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
>>>
>>> On Thu, Sep 10, 2020 at 11:56 AM Rob Herring <robh@kernel.org> wrote:
>>>>
>>>> On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
>>>>> 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 c2b3d2946a36..3d588ab7a6dd 100644
>>>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>>>> @@ -978,6 +978,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);
>>>>
>>>> Really needs to be a spinloop?
>>>>
>>>>> +
>>>>> + 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);
>>>>> @@ -1087,12 +1128,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,
>>>>
>>>> Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
>>>> and 1352 of .suspend in the tree.
>>>
>>> I will test switching this to suspend_late/resume_early.
>>
>> Why not just the 'regular' flavor suspend/resume?
>>
>> Rob
> We must have our PCIe driver suspend last and resume first because our
> current driver turns off/on the power for the EPs. Note that this
> code isn't in the driver as we are still figuring out a way to make it
> upstreamable.
The suspend/resume ordering should be guaranteed by the Linux device
driver model though if not, this is a bug that ought to be fixed. The
PCI bridge sits at the top of the pci_device list and all EPs should be
child devices, so the suspend order should be from EPs down to the
bridge, and the resume the converse.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops
2020-09-10 19:07 ` Florian Fainelli
@ 2020-09-10 19:09 ` Jim Quinlan
0 siblings, 0 replies; 36+ messages in thread
From: Jim Quinlan @ 2020-09-10 19:09 UTC (permalink / raw)
To: Florian Fainelli
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Rob Herring, Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
[-- Attachment #1.1: Type: text/plain, Size: 4035 bytes --]
On Thu, Sep 10, 2020 at 3:08 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 9/10/2020 12:05 PM, Jim Quinlan wrote:
> > On Thu, Sep 10, 2020 at 2:50 PM Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Thu, Sep 10, 2020 at 10:42 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
> >>>
> >>> On Thu, Sep 10, 2020 at 11:56 AM Rob Herring <robh@kernel.org> wrote:
> >>>>
> >>>> On Mon, Aug 24, 2020 at 03:30:17PM -0400, Jim Quinlan wrote:
> >>>>> 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 c2b3d2946a36..3d588ab7a6dd 100644
> >>>>> --- a/drivers/pci/controller/pcie-brcmstb.c
> >>>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
> >>>>> @@ -978,6 +978,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);
> >>>>
> >>>> Really needs to be a spinloop?
> >>>>
> >>>>> +
> >>>>> + 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);
> >>>>> @@ -1087,12 +1128,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,
> >>>>
> >>>> Why do you need interrupts disabled? There's 39 cases of .suspend_noirq
> >>>> and 1352 of .suspend in the tree.
> >>>
> >>> I will test switching this to suspend_late/resume_early.
> >>
> >> Why not just the 'regular' flavor suspend/resume?
> >>
> >> Rob
> > We must have our PCIe driver suspend last and resume first because our
> > current driver turns off/on the power for the EPs. Note that this
> > code isn't in the driver as we are still figuring out a way to make it
> > upstreamable.
>
> The suspend/resume ordering should be guaranteed by the Linux device
> driver model though if not, this is a bug that ought to be fixed. The
> PCI bridge sits at the top of the pci_device list and all EPs should be
> child devices, so the suspend order should be from EPs down to the
> bridge, and the resume the converse.
I remembered that after I hit send.
Jim
> --
> Florian
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes
2020-09-10 16:17 ` Rob Herring
@ 2020-09-11 15:28 ` Jim Quinlan
2020-09-11 16:13 ` Rob Herring
0 siblings, 1 reply; 36+ messages in thread
From: Jim Quinlan @ 2020-09-11 15:28 UTC (permalink / raw)
To: Rob Herring
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
[-- Attachment #1.1: Type: text/plain, Size: 9370 bytes --]
On Thu, Sep 10, 2020 at 12:17 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Aug 24, 2020 at 03:30:21PM -0400, Jim Quinlan wrote:
> > The Raspberry Pi (RPI) is currently the only chip using this driver
> > (pcie-brcmstb.c). There, only one memory controller is used, without an
> > extension region, and the SCB0 viewport size is set to the size of the
> > first and only dma-range region. Other BrcmSTB SOCs have more complicated
> > memory configurations that require setting additional viewport sizes.
> >
> > BrcmSTB PCIe controllers are intimately connected to the memory
> > controller(s) on the SOC. The SOC may have 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 of a controller are not adjacent, but in PCIe-space
> > they are.
> >
> > There is a "viewport" for each memory controller that allows DMA from
> > endpoint devices. 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 which is the sum of base region and its optional
> > extension. Further, the 1-3 viewports are also adjacent in PCIe-space.
> >
> > Unfortunately the viewport sizes cannot be ascertained from the
> > "dma-ranges" property so they have their own property, "brcm,scb-sizes".
> > This is because dma-range information does not indicate what memory
> > controller it is associated. For example, consider the following case
> > where the size of one dma-range is 2GB and the second dma-range is 1GB:
> >
> > /* Case 1: SCB0 size set to 4GB */
> > dma-range0: 2GB (from memc0-base)
> > dma-range1: 1GB (from memc0-extension)
> >
> > /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */
> > dma-range0: 2GB (from memc0-base)
> > dma-range1: 1GB (from memc0-extension)
> >
> > By just looking at the dma-ranges information, one cannot tell which
> > situation applies. That is why an additional property is needed. Its
> > length indicates the number of memory controllers being used and each value
> > indicates the viewport size.
> >
> > Note that the RPI DT does not have a "brcm,scb-sizes" property value,
> > as it is assumed that it only requires one memory controller and no
> > extension. So the optional use of "brcm,scb-sizes" will be backwards
> > compatible.
> >
> > One last layer of complexity exists: all of the viewports sizes must be
> > added and rounded up to a power of two to determine what the "BAR" size is.
> > Further, an offset must be given that indicates the base PCIe address of
> > this "BAR". The use of the term BAR is typically associated with endpoint
> > devices, and the term is used here because the PCIe HW may be used as an RC
> > or an EP. In the former case, all of the system memory appears in a single
> > "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely
> > used in the EP role and its system of mapping memory is an artifact that
> > requires multiple dma-ranges regions.
> >
> > 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 041b8d109563..7150eaa803c2 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
>
> Perhaps make 0-2 an arg and then you can just do:
>
> u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB_SIZE_MASK(memc))
I cannot get this to work. In this case u32p_replace_bits requires
that the mask is a compile-time constant; when "memc" is a variable I
don't see how to do this.
>
> >
> > #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])
> > @@ -259,6 +262,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];
> > };
> >
> > /*
> > @@ -714,22 +719,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] = 1ULL << fls64(size - 1);
>
> Use roundup_pow_of_two()
The reason I didn't use roundup_pow_of_two() is that it returns a
ulong which on ARM is 32bits and cannot represent 4GB.
>
> > + } 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);
>
> Use roundup_pow_of_two()
Ditto
Jim Quinlan
Broadcom STB
>
> >
> > /*
> > * We validate the inbound memory view even though we should trust
> > @@ -781,12 +808,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 */
> > @@ -826,11 +852,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
> >
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes
2020-09-11 15:28 ` Jim Quinlan
@ 2020-09-11 16:13 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-09-11 16:13 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Robin Murphy, Christoph Hellwig,
Nicolas Saenz Julienne
On Fri, Sep 11, 2020 at 9:28 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
>
> On Thu, Sep 10, 2020 at 12:17 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Aug 24, 2020 at 03:30:21PM -0400, Jim Quinlan wrote:
> > > The Raspberry Pi (RPI) is currently the only chip using this driver
> > > (pcie-brcmstb.c). There, only one memory controller is used, without an
> > > extension region, and the SCB0 viewport size is set to the size of the
> > > first and only dma-range region. Other BrcmSTB SOCs have more complicated
> > > memory configurations that require setting additional viewport sizes.
> > >
> > > BrcmSTB PCIe controllers are intimately connected to the memory
> > > controller(s) on the SOC. The SOC may have 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 of a controller are not adjacent, but in PCIe-space
> > > they are.
> > >
> > > There is a "viewport" for each memory controller that allows DMA from
> > > endpoint devices. 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 which is the sum of base region and its optional
> > > extension. Further, the 1-3 viewports are also adjacent in PCIe-space.
> > >
> > > Unfortunately the viewport sizes cannot be ascertained from the
> > > "dma-ranges" property so they have their own property, "brcm,scb-sizes".
> > > This is because dma-range information does not indicate what memory
> > > controller it is associated. For example, consider the following case
> > > where the size of one dma-range is 2GB and the second dma-range is 1GB:
> > >
> > > /* Case 1: SCB0 size set to 4GB */
> > > dma-range0: 2GB (from memc0-base)
> > > dma-range1: 1GB (from memc0-extension)
> > >
> > > /* Case 2: SCB0 size set to 2GB, SCB1 size set to 1GB */
> > > dma-range0: 2GB (from memc0-base)
> > > dma-range1: 1GB (from memc0-extension)
> > >
> > > By just looking at the dma-ranges information, one cannot tell which
> > > situation applies. That is why an additional property is needed. Its
> > > length indicates the number of memory controllers being used and each value
> > > indicates the viewport size.
> > >
> > > Note that the RPI DT does not have a "brcm,scb-sizes" property value,
> > > as it is assumed that it only requires one memory controller and no
> > > extension. So the optional use of "brcm,scb-sizes" will be backwards
> > > compatible.
> > >
> > > One last layer of complexity exists: all of the viewports sizes must be
> > > added and rounded up to a power of two to determine what the "BAR" size is.
> > > Further, an offset must be given that indicates the base PCIe address of
> > > this "BAR". The use of the term BAR is typically associated with endpoint
> > > devices, and the term is used here because the PCIe HW may be used as an RC
> > > or an EP. In the former case, all of the system memory appears in a single
> > > "BAR" region in PCIe memory. As it turns out, BrcmSTB PCIe HW is rarely
> > > used in the EP role and its system of mapping memory is an artifact that
> > > requires multiple dma-ranges regions.
> > >
> > > 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 041b8d109563..7150eaa803c2 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
> >
> > Perhaps make 0-2 an arg and then you can just do:
> >
> > u32p_replace_bits(&tmp, scb_size_val, PCIE_MISC_MISC_CTRL_SCB_SIZE_MASK(memc))
>
> I cannot get this to work. In this case u32p_replace_bits requires
> that the mask is a compile-time constant; when "memc" is a variable I
> don't see how to do this.
> >
> > >
> > > #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])
> > > @@ -259,6 +262,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];
> > > };
> > >
> > > /*
> > > @@ -714,22 +719,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] = 1ULL << fls64(size - 1);
> >
> > Use roundup_pow_of_two()
> The reason I didn't use roundup_pow_of_two() is that it returns a
> ulong which on ARM is 32bits and cannot represent 4GB.
Guess time for a roundup_pow_of_two_64... Anyways, save that for another day.
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2020-09-11 16:15 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24 19:30 [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 02/11] dt-bindings: PCI: Add bindings for more Brcmstb chips Jim Quinlan
2020-08-24 19:30 ` [PATCH v11 03/11] PCI: brcmstb: Add bcm7278 register info Jim Quinlan
2020-09-10 15:44 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 04/11] PCI: brcmstb: Add suspend and resume pm_ops Jim Quinlan
2020-09-10 15:56 ` Rob Herring
2020-09-10 16:42 ` Jim Quinlan
2020-09-10 18:50 ` Rob Herring
2020-09-10 18:54 ` Florian Fainelli
2020-09-10 19:05 ` Jim Quinlan
2020-09-10 19:07 ` Florian Fainelli
2020-09-10 19:09 ` Jim Quinlan
2020-09-10 18:47 ` Florian Fainelli
2020-08-24 19:30 ` [PATCH v11 05/11] PCI: brcmstb: Add bcm7278 PERST# support Jim Quinlan
2020-09-10 16:04 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 06/11] PCI: brcmstb: Add control of rescal reset Jim Quinlan
2020-09-08 13:32 ` Lorenzo Pieralisi
2020-09-10 16:09 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 08/11] PCI: brcmstb: Set additional internal memory DMA viewport sizes Jim Quinlan
2020-09-10 16:17 ` Rob Herring
2020-09-11 15:28 ` Jim Quinlan
2020-09-11 16:13 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 09/11] PCI: brcmstb: Accommodate MSI for older chips Jim Quinlan
2020-09-10 16:20 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 10/11] PCI: brcmstb: Set bus max burst size by chip type Jim Quinlan
2020-09-10 16:22 ` Rob Herring
2020-08-24 19:30 ` [PATCH v11 11/11] PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list Jim Quinlan
2020-09-10 16:23 ` Rob Herring
2020-08-25 17:40 ` [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips Florian Fainelli
2020-08-27 6:35 ` Christoph Hellwig
2020-08-27 13:29 ` Jim Quinlan
2020-09-07 9:16 ` Lorenzo Pieralisi
2020-09-07 17:43 ` Jim Quinlan
2020-09-07 18:29 ` Florian Fainelli
2020-09-08 10:42 ` Lorenzo Pieralisi
2020-09-08 12:20 ` Christoph Hellwig
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).