This series enables support for Pensando Elba SoC based platforms.
The Elba SoC has the following features:
- Sixteen ARM64 A72 cores
- Dual DDR 4/5 memory controllers
- 32 lanes of PCIe Gen3/4 to the Host
- Network interfaces: Dual 200GE, Quad 100GE, 50GE, 25GE, 10GE and
also a single 1GE management port.
- Storage/crypto offloads and 144 programmable P4 cores.
- QSPI and EMMC for SoC storage
- Two SPI interfaces for peripheral management
- I2C bus for platform management
This is a respin based on review input. Summary of the changes are:
0001-gpio-Add-Elba-SoC-gpio-driver-for-spi-cs-control.patch
- This patch is deleted. Elba SOC specific gpio spics control is
integrated into spi-dw-mmio.c.
0002-spi-cadence-quadspi-Add-QSPI-support-for-Pensando-El.patch
- Changed compatible to "pensando,elba-qspi" to be more descriptive
in spi-cadence-quadspi.c.
- Arnd wondered if moving to DT properties for quirks may be the
way to go. Feedback I've received on other patches was don't
mix two efforts in one patch so I'm currently just adding the
Elba support to the current design.
0003-spi-dw-Add-support-for-Pensando-Elba-SoC-SPI.patch
- Changed the implementation to use existing dw_spi_set_cs() and
integrated Elba specific CS control into spi-dw-mmio.c. The
native designware support is for two chip-selects while Elba
provides 4 chip-selects. Instead of adding a new file for
this support in gpio-elba-spics.c the support is in one
file (spi-dw-mmio.c).
0004-spidev-Add-Pensando-CPLD-compatible.patch
- This patch is deleted. The addition of compatible "pensando,cpld"
to spidev.c is not added and an existing compatible is used
in the device tree to enable.
0005-mmc-sdhci-cadence-Add-Pensando-Elba-SoC-support.patch
- Ulf and Yamada-san agreed the amount of code for this support
is not enough to need a new file. The support is added into
sdhci-cadence.c and new files sdhci-cadence-elba.c and
sdhci-cadence.h are deleted.
- Redundant defines are removed (e.g. use SDHCI_CDNS_HRS04 and
remove SDIO_REG_HRS4).
- Removed phy init function sd4_set_dlyvr() and used existing
sdhci_cdns_phy_init(). Init values are from DT properties.
- Replace devm_ioremap_resource(&pdev->dev, iomem)
with devm_platform_ioremap_resource(pdev, 1)
- Refactored the elba priv_writ_l() and elba_write_l() to
remove a little redundant code.
- The config option CONFIG_MMC_SDHCI_CADENCE_ELBA goes away.
- Only C syntax and Elba functions are prefixed with elba_
0006-arm64-Add-config-for-Pensando-SoC-platforms.patch
- Added a little more info to the platform help text to assist
users to decide on including platform support or not.
0007-arm64-dts-Add-Pensando-Elba-SoC-support.patch
- Node names changed to DT generic names
- Changed from using 'spi@' which is reserved
- The elba-flash-parts.dtsi is kept separate as
it is included in multiple dts files.
- SPDX license tags at the top of each file
- The compatible = "pensando,elba" and 'model' are
now together in the board file.
- UIO nodes removed
- Ordered nodes by increasing unit address
- Removed an unreferenced container node.
- Dropped deprecated 'device_type' for uart0 node.
0010-dt-bindings-spi-cadence-qspi-Add-support-for-Pensand.patch
- Updated since the latest documentation has been converted to yaml
0011-dt-bindings-gpio-Add-Pensando-Elba-SoC-support.patch
- This patch is deleted since the Elba gpio spics is added to
the spi dw driver and documented there.
Because of the deletion of patches and merging of code
the new patchset is not similar. A changelog is added into
the patches for merged code to be helpful on the history.
Brad Larson (11):
dt-bindings: arm: pensando: add Pensando boards
dt-bindings: Add vendor prefix for Pensando Systems
dt-bindings: mmc: Add Pensando Elba SoC binding
dt-bindings: spi: Add compatible for Pensando Elba SoC
spi: dw: Add Pensando Elba SoC SPI Controller bindings
MAINTAINERS: Add entry for PENSANDO
arm64: Add config for Pensando SoC platforms
spi: cadence-quadspi: Add compatible for Pensando Elba SoC
mmc: sdhci-cadence: Add Pensando Elba SoC support
spi: dw: Add support for Pensando Elba SoC
arm64: dts: Add Pensando Elba SoC support
.../bindings/arm/pensando,elba.yaml | 20 ++
.../devicetree/bindings/mmc/cdns,sdhci.yaml | 13 +-
.../bindings/spi/cdns,qspi-nor.yaml | 3 +-
.../bindings/spi/snps,dw-apb-ssi.yaml | 2 +
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 7 +
arch/arm64/Kconfig.platforms | 12 ++
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/pensando/Makefile | 6 +
arch/arm64/boot/dts/pensando/elba-16core.dtsi | 192 ++++++++++++++++++
.../boot/dts/pensando/elba-asic-common.dtsi | 96 +++++++++
arch/arm64/boot/dts/pensando/elba-asic.dts | 23 +++
.../boot/dts/pensando/elba-flash-parts.dtsi | 103 ++++++++++
arch/arm64/boot/dts/pensando/elba.dtsi | 181 +++++++++++++++++
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-cadence.c | 148 ++++++++++++--
drivers/spi/spi-cadence-quadspi.c | 19 ++
drivers/spi/spi-dw-mmio.c | 85 ++++++++
18 files changed, 894 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/pensando,elba.yaml
create mode 100644 arch/arm64/boot/dts/pensando/Makefile
create mode 100644 arch/arm64/boot/dts/pensando/elba-16core.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba-asic.dts
create mode 100644 arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba.dtsi
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables resulting in an additional
reg property resource.
Signed-off-by: Brad Larson <redacted>
---
.../devicetree/bindings/mmc/cdns,sdhci.yaml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
@@ -38,7 +39,7 @@ properties:description:Size of the data FIFO in words.$ref:"/schemas/types.yaml#/definitions/uint32"-enum:[128,256]+enum:[128,256,1024]default:128cdns,fifo-width:
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The Pensando Elba SoC has the Cadence QSPI controller integrated.
The quirk CQSPI_NEEDS_APB_AHB_HAZARD_WAR is added and if enabled
a dummy readback from the controller is performed to ensure
synchronization.
Signed-off-by: Brad Larson <redacted>
---
drivers/spi/spi-cadence-quadspi.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
Add support for Pensando Elba SoC which explicitly controls
byte-lane enables on writes. Add priv_write_l() which is
used on Elba platforms for byte-lane control.
Select MMC_SDHCI_IO_ACCESSORS for MMC_SDHCI_CADENCE which
allows Elba SoC sdhci_elba_ops to overwrite the SDHCI
IO memory accessors.
Signed-off-by: Brad Larson <redacted>
---
Changelog:
- Ulf and Yamada-san agreed the amount of code for this support
is not enough to need a new file. The support is added into
sdhci-cadence.c and new files sdhci-cadence-elba.c and
sdhci-cadence.h are deleted.
- Redundant defines are removed (e.g. use SDHCI_CDNS_HRS04 and
remove SDIO_REG_HRS4).
- Removed phy init function sd4_set_dlyvr() and used existing
sdhci_cdns_phy_init(). Init values are from DT properties.
- Replace devm_ioremap_resource(&pdev->dev, iomem)
with devm_platform_ioremap_resource(pdev, 1)
- Refactored the elba priv_writ_l() and elba_write_l() to
remove a little redundant code.
- The config option CONFIG_MMC_SDHCI_CADENCE_ELBA goes away.
- Only C syntax and Elba functions are prefixed with elba_
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-cadence.c | 148 ++++++++++++++++++++++++++++---
2 files changed, 135 insertions(+), 14 deletions(-)
@@ -243,6 +243,7 @@ config MMC_SDHCI_CADENCEtristate"SDHCI support for the Cadence SD/SDIO/eMMC controller"depends onMMC_SDHCI_PLTFMdepends onOF+selectMMC_SDHCI_IO_ACCESSORShelpThisselectstheCadenceSD/SDIO/eMMCdriver.
The Pensando Elba SoC includes a DW apb_ssi v4 controller
with device specific chip-select control. The Elba SoC
provides four chip-selects where the native DW IP supports
two chip-selects.
Signed-off-by: Brad Larson <redacted>
---
Changelog:
- Changed the implementation to use existing dw_spi_set_cs() and
integrated Elba specific CS control into spi-dw-mmio.c. The
native designware support is for two chip-selects while Elba
provides 4 chip-selects. Instead of adding a new file for
this support in gpio-elba-spics.c the support is in one
file (spi-dw-mmio.c).
drivers/spi/spi-dw-mmio.c | 85 +++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
Add Pensando common and Elba SoC specific device nodes
Signed-off-by: Brad Larson <redacted>
---
Changelog:
- Node names changed to DT generic names
- Changed from using 'spi@' which is reserved
- The elba-flash-parts.dtsi is kept separate as
it is included in multiple dts files.
- SPDX license tags at the top of each file
- The compatible = "pensando,elba" and 'model' are
now together in the board file.
- UIO nodes removed
- Ordered nodes by increasing unit address
- Removed an unreferenced container node.
- Dropped deprecated 'device_type' for uart0 node.
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/pensando/Makefile | 6 +
arch/arm64/boot/dts/pensando/elba-16core.dtsi | 192 ++++++++++++++++++
.../boot/dts/pensando/elba-asic-common.dtsi | 96 +++++++++
arch/arm64/boot/dts/pensando/elba-asic.dts | 23 +++
.../boot/dts/pensando/elba-flash-parts.dtsi | 103 ++++++++++
arch/arm64/boot/dts/pensando/elba.dtsi | 181 +++++++++++++++++
7 files changed, 602 insertions(+)
create mode 100644 arch/arm64/boot/dts/pensando/Makefile
create mode 100644 arch/arm64/boot/dts/pensando/elba-16core.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba-asic.dts
create mode 100644 arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba.dtsi
The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
have GICv3, where this is not valid, so this should go.
Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
doesn't mak sense for the 16 CPUs you have.
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
have GICv3, where this is not valid, so this should go.
Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
doesn't mak sense for the 16 CPUs you have.
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
Funny, it looks like there is a sudden outburst of stupid copy/paste
among HW designers. TI did the exact same thing recently.
This totally negates all the advantages of having an ITS and makes
sure that you have all the overhead. Facepalm...
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Rob Herring <robh@kernel.org> Date: 2021-10-25 12:54:22
On Sun, 24 Oct 2021 18:51:48 -0700, Brad Larson wrote:
Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables resulting in an additional
reg property resource.
Signed-off-by: Brad Larson <redacted>
---
.../devicetree/bindings/mmc/cdns,sdhci.yaml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml:20:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1545481
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Rob Herring <robh@kernel.org> Date: 2021-10-26 18:11:01
On Sun, Oct 24, 2021 at 06:51:48PM -0700, Brad Larson wrote:
quoted hunk
Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables resulting in an additional
reg property resource.
Signed-off-by: Brad Larson <redacted>
---
.../devicetree/bindings/mmc/cdns,sdhci.yaml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
From: Rob Herring <robh@kernel.org> Date: 2021-10-27 21:37:30
On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
quoted hunk
Add Pensando common and Elba SoC specific device nodes
Signed-off-by: Brad Larson <redacted>
---
Changelog:
- Node names changed to DT generic names
- Changed from using 'spi@' which is reserved
- The elba-flash-parts.dtsi is kept separate as
it is included in multiple dts files.
- SPDX license tags at the top of each file
- The compatible = "pensando,elba" and 'model' are
now together in the board file.
- UIO nodes removed
- Ordered nodes by increasing unit address
- Removed an unreferenced container node.
- Dropped deprecated 'device_type' for uart0 node.
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/pensando/Makefile | 6 +
arch/arm64/boot/dts/pensando/elba-16core.dtsi | 192 ++++++++++++++++++
.../boot/dts/pensando/elba-asic-common.dtsi | 96 +++++++++
arch/arm64/boot/dts/pensando/elba-asic.dts | 23 +++
.../boot/dts/pensando/elba-flash-parts.dtsi | 103 ++++++++++
arch/arm64/boot/dts/pensando/elba.dtsi | 181 +++++++++++++++++
7 files changed, 602 insertions(+)
create mode 100644 arch/arm64/boot/dts/pensando/Makefile
create mode 100644 arch/arm64/boot/dts/pensando/elba-16core.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba-asic.dts
create mode 100644 arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
create mode 100644 arch/arm64/boot/dts/pensando/elba.dtsi
Hello Brad
The patch name "dt-bindings: spi: Add compatible for Pensando Elba
SoC" doesn't mention to what bindings it is referring to. For the sake
of having a more representative git log I'd suggest at least to add
"cdns" vendor name in the title, like: "dt-bindings: spi: cdns: Add ..."
Otherwise it's impossible to understand to what bindings you're adding
a new compatibility especially seeing you are doing the similar thing
for the DW SPI in the next patch.
-Sergey
On Sun, Oct 24, 2021 at 06:51:49PM -0700, Brad Larson wrote:
quoted hunk
Document the cadence qspi controller compatible for Pensando Elba SoC
boards. The Elba qspi fifo size is 1024.
Signed-off-by: Brad Larson <redacted>
---
Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
@@ -38,7 +39,7 @@ properties:description:Size of the data FIFO in words.$ref:"/schemas/types.yaml#/definitions/uint32"-enum:[128,256]+enum:[128,256,1024]default:128cdns,fifo-width:
On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
The Pensando Elba SoC has integrated the DW APB SPI Controller
Please add the "dt-bindings: " prefix to the patch name and discard
the word "bindings" from the title as the submitting DT-patches
requires:
Documentation/devicetree/bindings/submitting-patches.rst
AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit, but for some
reason discarded from this series (btw why?). If you did you would
have got an error of an unevaluated property detection.
-Sergey
On Thu, Oct 28, 2021 at 10:49:48AM +0300, Serge Semin wrote:
On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
quoted
The Pensando Elba SoC has integrated the DW APB SPI Controller
Please add the "dt-bindings: " prefix to the patch name and discard
the word "bindings" from the title as the submitting DT-patches
requires:
Documentation/devicetree/bindings/submitting-patches.rst
AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit,
but for some reason discarded from this series (btw why?).
Oops. Found it. The question regarding "dtbs_check" is still actual.
-Sergey
If you did you would
have got an error of an unevaluated property detection.
-Sergey
On Sun, Oct 24, 2021 at 06:51:55PM -0700, Brad Larson wrote:
quoted hunk
The Pensando Elba SoC includes a DW apb_ssi v4 controller
with device specific chip-select control. The Elba SoC
provides four chip-selects where the native DW IP supports
two chip-selects.
Signed-off-by: Brad Larson <redacted>
---
Changelog:
- Changed the implementation to use existing dw_spi_set_cs() and
integrated Elba specific CS control into spi-dw-mmio.c. The
native designware support is for two chip-selects while Elba
provides 4 chip-selects. Instead of adding a new file for
this support in gpio-elba-spics.c the support is in one
file (spi-dw-mmio.c).
drivers/spi/spi-dw-mmio.c | 85 +++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
@@ -53,6 +53,24 @@ struct dw_spi_mscc {void__iomem*spi_mst;/* Not sparx5 */};+structdw_spi_elba{+structregmap*regmap;+unsignedintreg;+};++/*
+ * Elba SoC does not use ssi, pin override is used for cs 0,1 and
+ * gpios for cs 2,3 as defined in the device tree.
I believe GPIO-based CS is the platform-property rather than the SoC
one. It's up to the board designers which GPIOs to use as a custom
chip-select signal. Thus it would be better to discard the comment
regarding the GPIOs here.
quoted hunk
+ *
+ * cs: | 1 0
+ * bit: |---3-------2-------1-------0
+ * | cs1 cs1_ovr cs0 cs0_ovr
+ */
+#define ELBA_SPICS_SHIFT(cs) (2 * (cs))
+#define ELBA_SPICS_MASK(cs) (0x3 << ELBA_SPICS_SHIFT(cs))
+#define ELBA_SPICS_SET(cs, val) \
+ ((((val) << 1) | 0x1) << ELBA_SPICS_SHIFT(cs))
+
/*
* The Designware SPI controller (referred to as master in the documentation)
* automatically deasserts chip select when the tx fifo is empty. The chip
Is it correct to think that the DW SSI output CS signals are
multiplexed between the native DW SSI CS logic and the logic
implemented in the ELBA SPICS syscon? Thus by setting "csX_ovr" in the
ELBA_SPICS CSR do you get to switch between the DW SSI SER logic and
the signal level selected by the "csX" field of that register?
* Most likely I already asked this question in v2 but it was long time
ago, so it's better to clarify things over.
There is a good wrapper for this: syscon_regmap_lookup_by_phandle_args() .
The property name isn't well descriptive in the syscon-related
part. Could you add something like:
"pensando,elba-syscon-spics"/"pensando,syscon-spics"?
-Sergey
From: Andy Shevchenko <hidden> Date: 2021-10-31 13:20:29
On Mon, Oct 25, 2021 at 4:54 AM Brad Larson [off-list ref] wrote:
The Pensando Elba SoC includes a DW apb_ssi v4 controller
with device specific chip-select control. The Elba SoC
provides four chip-selects where the native DW IP supports
two chip-selects.
+ regmap = syscon_node_to_regmap(args.np);
+ if (IS_ERR(regmap)) {
+ dev_err(&pdev->dev, "could not map pensando,spics\n");
+ return PTR_ERR(regmap);
+ }
Why not return dev_err_probe()?
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
have GICv3, where this is not valid, so this should go.
Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
doesn't mak sense for the 16 CPUs you have.
Thanks for pointing this out. Elba SoC is a GICv3 implementation and looking
at other device tree files we should be using this:
timer {
compatible = "arm,armv8-timer";
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>;
};
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
There is no shared lineage with Synqacer. We are solving the same issue
with the same mechanism. I'll add a comment to this DTS node.
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
This is missing the GICv2 compat regions that the CPUs implement.
Is this what is described as optional in the GIC architecture specification
where a GICv3 system can run restricted GICv2 code? Can you point
me in the right direction in the spec and example dts node if needed.
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
Funny, it looks like there is a sudden outburst of stupid copy/paste
among HW designers. TI did the exact same thing recently.
This totally negates all the advantages of having an ITS and makes
sure that you have all the overhead. Facepalm...
Some background may help explain. To generate an LPI a peripheral must
write to the GITS_TRANSLATER (a specific address). For the ITS to know
which translations apply to the generated interrupts, it must know which
peripheral performed the write. The ID of the peripheral is known as its
DeviceID, which is often carried along with the write as an AXI sideband
signal.
The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
between the peripheral and the ITS. Instead of telling a peripheral to target
the GITS_TRANSLATER directly, we instead direct it to a specific offset
within a pre-ITS address range (our own IP block). For writes that land in
that memory range, we derive the DeviceID from (offset >> 2). The pre-ITS
block then sends (DeviceID, data) to the GITS_TRANSLATER.
The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
When we looked at the upstream kernel later (we developed on 4.14)
we found that not only did it support something similar, it supported the
exact scheme we are using.
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Rob,
On Mon, Oct 25, 2021 at 5:54 AM Rob Herring [off-list ref] wrote:
On Sun, 24 Oct 2021 18:51:48 -0700, Brad Larson wrote:
quoted
Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables resulting in an additional
reg property resource.
Signed-off-by: Brad Larson <redacted>
---
.../devicetree/bindings/mmc/cdns,sdhci.yaml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml:20:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1545481
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
yamllint was not installed, it is now and dtschema is updated to run again
before re-submit.
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
ack, will order this way for re-spin of the patch set.
Thanks
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
This is missing the GICv2 compat regions that the CPUs implement.
Is this what is described as optional in the GIC architecture specification
where a GICv3 system can run restricted GICv2 code?
Yup, that. It is actually implemented by the CPU.
Can you point me in the right direction in the spec and example dts
node if needed.
The Cortex-A72 TRM has everything you need [1]. And since you used the
Synquacer as the model for this, you will see that it has the missing
regions. Alternatively, rk3399.dtsi is a good example.
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
Funny, it looks like there is a sudden outburst of stupid copy/paste
among HW designers. TI did the exact same thing recently.
This totally negates all the advantages of having an ITS and makes
sure that you have all the overhead. Facepalm...
Some background may help explain. To generate an LPI a peripheral must
write to the GITS_TRANSLATER (a specific address). For the ITS to know
which translations apply to the generated interrupts, it must know which
peripheral performed the write. The ID of the peripheral is known as its
DeviceID, which is often carried along with the write as an AXI sideband
signal.
Yes, I happen to be vaguely familiar with the GIC architecture.
The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
between the peripheral and the ITS. Instead of telling a peripheral to target
the GITS_TRANSLATER directly, we instead direct it to a specific offset
within a pre-ITS address range (our own IP block). For writes that land in
that memory range, we derive the DeviceID from (offset >> 2). The pre-ITS
block then sends (DeviceID, data) to the GITS_TRANSLATER.
The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
When we looked at the upstream kernel later (we developed on 4.14)
we found that not only did it support something similar, it supported the
exact scheme we are using.
And this scheme is totally wrong. It breaks interrupt isolation.
Instead of having a single doorbell and getting the ITS to segregate
between devices itself, you end-up with multiple ones, allowing a
rogue device to impersonate another one by targeting another doorbell.
You can't even use an SMMU to preserve some isolation, because all the
doorbells are in the *same page*. Unmitigated disaster.
At this stage, why did you bother having an ITS at all? You get none
of the security features. Only the excess area, memory allocation,
additional latency and complexity. All you get is a larger INTID
space.
This only shows that the hardware designer didn't understand the ITS
at all. Which seems a common pattern, unfortunately.
M.
[1] https://developer.arm.com/documentation/100095/0003/Generic-Interrupt-Controller-CPU-Interface/GIC-functional-description/GIC-memory-map?lang=en#way1382452674438__CHDEBJAJ
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
have GICv3, where this is not valid, so this should go.
Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
doesn't mak sense for the 16 CPUs you have.
Thanks for pointing this out. Elba SoC is a GICv3 implementation and looking
at other device tree files we should be using this:
timer {
compatible = "arm,armv8-timer";
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(16) |
IRQ_TYPE_LEVEL_LOW)>;
};
No; as above, you should *not* use GIC_CPU_MASK_SIMPLE() at all for GICv3. i.e.
Please see the GICv3 binding documentation:
Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
... and note that it does not have the cpumask field as use by the binding for
prior generations of GIC:
Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
If you've seen other dts files using GIC_CPU_MASK_SIMPLE() with GICv3, those
are incorrect, and need to be fixed.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Please see the GICv3 binding documentation:
Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
... and note that it does not have the cpumask field as use by the binding for
prior generations of GIC:
Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
If you've seen other dts files using GIC_CPU_MASK_SIMPLE() with GICv3, those
are incorrect, and need to be fixed.
Thanks,
Mark.
I'll use the bindings documentation as the primary reference. The use of
GIC_CPU_MASK_SIMPLE() is removed and tests ok. These arm64 dts files in
linux-next are gic-v3 and use GIC_CPU_MASK_SIMPLE(1, 2, 4, 8)
./nvidia/tegra234.dtsi
./renesas/r9a07g044.dtsi
./renesas/r8a779a0.dtsi
./qcom/sm8350.dtsi
./qcom/sm8250.dtsi
./freescale/fsl-ls1028a.dtsi
./freescale/imx8mp.dtsi
./freescale/imx8mn.dtsi
./freescale/imx8mm.dtsi
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
Funny, it looks like there is a sudden outburst of stupid copy/paste
among HW designers. TI did the exact same thing recently.
This totally negates all the advantages of having an ITS and makes
sure that you have all the overhead. Facepalm...
Some background may help explain. To generate an LPI a peripheral must
write to the GITS_TRANSLATER (a specific address). For the ITS to know
which translations apply to the generated interrupts, it must know which
peripheral performed the write. The ID of the peripheral is known as its
DeviceID, which is often carried along with the write as an AXI sideband
signal.
Yes, I happen to be vaguely familiar with the GIC architecture.
quoted
The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
between the peripheral and the ITS. Instead of telling a peripheral to target
the GITS_TRANSLATER directly, we instead direct it to a specific offset
within a pre-ITS address range (our own IP block). For writes that land in
that memory range, we derive the DeviceID from (offset >> 2). The pre-ITS
block then sends (DeviceID, data) to the GITS_TRANSLATER.
The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
When we looked at the upstream kernel later (we developed on 4.14)
we found that not only did it support something similar, it supported the
exact scheme we are using.
And this scheme is totally wrong. It breaks interrupt isolation.
Instead of having a single doorbell and getting the ITS to segregate
between devices itself, you end-up with multiple ones, allowing a
rogue device to impersonate another one by targeting another doorbell.
You can't even use an SMMU to preserve some isolation, because all the
doorbells are in the *same page*. Unmitigated disaster.
At this stage, why did you bother having an ITS at all? You get none
of the security features. Only the excess area, memory allocation,
additional latency and complexity. All you get is a larger INTID
space.
This only shows that the hardware designer didn't understand the ITS
at all. Which seems a common pattern, unfortunately.
The Elba SoC is an embedded chip and not intended as a SBSA-compliant
general platform. In this implementation the ITS is used to provide
message-based interrupts for our (potentially large set) of hardware
based platform device instances. Virtualization is not a consideration.
We don't have a SMMU. Interrupt isolation isn't a practical consideration
for this product. Propose adding a comment to the dts.
+ /*
+ * Elba SoC implemented a pre-ITS that happened to
+ * be the same implementation as synquacer.
+ */
its: interrupt-controller@820000 {
compatible = "arm,gic-v3-its";
msi-controller;
Thanks
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.
Funny, it looks like there is a sudden outburst of stupid copy/paste
among HW designers. TI did the exact same thing recently.
This totally negates all the advantages of having an ITS and makes
sure that you have all the overhead. Facepalm...
Some background may help explain. To generate an LPI a peripheral must
write to the GITS_TRANSLATER (a specific address). For the ITS to know
which translations apply to the generated interrupts, it must know which
peripheral performed the write. The ID of the peripheral is known as its
DeviceID, which is often carried along with the write as an AXI sideband
signal.
Yes, I happen to be vaguely familiar with the GIC architecture.
quoted
The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
between the peripheral and the ITS. Instead of telling a peripheral to target
the GITS_TRANSLATER directly, we instead direct it to a specific offset
within a pre-ITS address range (our own IP block). For writes that land in
that memory range, we derive the DeviceID from (offset >> 2). The pre-ITS
block then sends (DeviceID, data) to the GITS_TRANSLATER.
The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
When we looked at the upstream kernel later (we developed on 4.14)
we found that not only did it support something similar, it supported the
exact scheme we are using.
And this scheme is totally wrong. It breaks interrupt isolation.
Instead of having a single doorbell and getting the ITS to segregate
between devices itself, you end-up with multiple ones, allowing a
rogue device to impersonate another one by targeting another doorbell.
You can't even use an SMMU to preserve some isolation, because all the
doorbells are in the *same page*. Unmitigated disaster.
At this stage, why did you bother having an ITS at all? You get none
of the security features. Only the excess area, memory allocation,
additional latency and complexity. All you get is a larger INTID
space.
This only shows that the hardware designer didn't understand the ITS
at all. Which seems a common pattern, unfortunately.
The Elba SoC is an embedded chip and not intended as a SBSA-compliant
general platform.
This has nothing to do with following a standard. It has to do with
following the intended use of the architecture. What you have here is
the system architecture equivalent of trusting userspace to build the
kernel page tables. It can work in limited cases. But would you want
to deploy such construct at scale? Probably not.
In this implementation the ITS is used to provide message-based
interrupts for our (potentially large set) of hardware based
platform device instances. Virtualization is not a consideration.
We don't have a SMMU. Interrupt isolation isn't a practical
consideration for this product.
Because you have foreseen all use cases for this HW ahead of time, and
can already tell how SW is going to make use of it? Oh well...
Propose adding a comment to the dts.
+ /*
+ * Elba SoC implemented a pre-ITS that happened to
+ * be the same implementation as synquacer.
+ */
Which contains zero information. What you really want is: "We have
decided to ignore the system architecture, good luck".
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Nov 8, 2021 at 11:54 AM Marc Zyngier [off-list ref] wrote:
quoted
The Elba SoC is an embedded chip and not intended as a SBSA-compliant
general platform.
This has nothing to do with following a standard. It has to do with
following the intended use of the architecture. What you have here is
the system architecture equivalent of trusting userspace to build the
kernel page tables. It can work in limited cases. But would you want
to deploy such construct at scale? Probably not.
quoted
In this implementation the ITS is used to provide message-based
interrupts for our (potentially large set) of hardware based
platform device instances. Virtualization is not a consideration.
We don't have a SMMU. Interrupt isolation isn't a practical
consideration for this product.
Because you have foreseen all use cases for this HW ahead of time, and
can already tell how SW is going to make use of it? Oh well...
quoted
Propose adding a comment to the dts.
+ /*
+ * Elba SoC implemented a pre-ITS that happened to
+ * be the same implementation as synquacer.
+ */
Which contains zero information. What you really want is: "We have
decided to ignore the system architecture, good luck".
M.
--
Without deviation from the norm, progress is not possible.
On the contrary, the confusion of using the existing driver match
"socionext,synquacer-pre-its" is answered, why add new code.
Looks like we are deviating from the norm ;-). I'm not seeing how
this conversation is a productive use of time for a platform in
production.
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Node names should reflect the class of device and use standard name
defined in the DT spec. This probably doesn't have one. 'lora' perhaps?
Right, I didn't see a standard name and found many approaches in other files
so I likely based off of one of these below. I searched the dts
files for 'lora' and
didn't find it. Is that an acronym? I can change it to what the preference is.
./microchip/sparx5_pcb134_board.dtsi:
&spi0 {
status = "okay";
spi@0 {
compatible = "spi-mux";
...
};
./rockchip/rk3399.dtsi:
spi5 {
spi5_clk: spi5-clk {
rockchip,pins =
<2 RK_PC6 2 &pcfg_pull_up>;
};
spi5_cs0: spi5-cs0 {
rockchip,pins =
<2 RK_PC7 2 &pcfg_pull_up>;
};
spi5_rx: spi5-rx {
rockchip,pins =
<2 RK_PC4 2 &pcfg_pull_up>;
};
spi5_tx: spi5-tx {
rockchip,pins =
<2 RK_PC5 2 &pcfg_pull_up>;
};
};
It's module drivers/spi/spidev.c which won't populate /dev/spidevB.C unless
there is a match which we need for the system to boot. An earlier patch added
to the compatible list below and the feedback on that was to remove it. Later I
noticed the compatible list expanded...
static const struct of_device_id spidev_dt_ids[] = {
{ .compatible = "rohm,dh2228fv" },
{ .compatible = "lineartechnology,ltc2488" },
{ .compatible = "semtech,sx1301" },
{ .compatible = "lwn,bk4" },
{ .compatible = "dh,dhcom-board" },
{ .compatible = "menlo,m53cpld" },
{ .compatible = "cisco,spi-petra" },
{ .compatible = "micron,spi-authenta" },
{},
};
Normally we have a compatible for the board plus the soc compatible.
In this case there are currently five different boards/products that have no
variations needing a board level description.
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Sergey,
On Thu, Oct 28, 2021 at 12:26 AM Serge Semin [off-list ref] wrote:
Hello Brad
The patch name "dt-bindings: spi: Add compatible for Pensando Elba
SoC" doesn't mention to what bindings it is referring to. For the sake
of having a more representative git log I'd suggest at least to add
"cdns" vendor name in the title, like: "dt-bindings: spi: cdns: Add ..."
Otherwise it's impossible to understand to what bindings you're adding
a new compatibility especially seeing you are doing the similar thing
for the DW SPI in the next patch.
Hi Sergey,
On Thu, Oct 28, 2021 at 12:49 AM Serge Semin [off-list ref] wrote:
On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
quoted
The Pensando Elba SoC has integrated the DW APB SPI Controller
Please add the "dt-bindings: " prefix to the patch name and discard
the word "bindings" from the title as the submitting DT-patches
requires:
Documentation/devicetree/bindings/submitting-patches.rst
I'll add that. I recall looking at the recent git log for similar
changes to the file as the current recommended approach.
AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit, but for some
reason discarded from this series (btw why?). If you did you would
have got an error of an unevaluated property detection.
I ran the checks below and didn't get errors. Rob provided some info
and I found the server did not have yamllint installed (not flagged by
tool). Also dt-schema was not the latest. I'm re-doing this and
including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
Thanks
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Nov 15, 2021 at 02:24:40PM -0800, Brad Larson wrote:
Hi Sergey,
On Thu, Oct 28, 2021 at 12:49 AM Serge Semin [off-list ref] wrote:
quoted
On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
quoted
The Pensando Elba SoC has integrated the DW APB SPI Controller
Please add the "dt-bindings: " prefix to the patch name and discard
the word "bindings" from the title as the submitting DT-patches
requires:
Documentation/devicetree/bindings/submitting-patches.rst
I'll add that. I recall looking at the recent git log for similar
changes to the file as the current recommended approach.
AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit, but for some
reason discarded from this series (btw why?). If you did you would
have got an error of an unevaluated property detection.
I ran the checks below and didn't get errors. Rob provided some info
and I found the server did not have yamllint installed (not flagged by
tool). Also dt-schema was not the latest. I'm re-doing this and
including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
Hmm, that's weird. Rob, does dtschema tool have the
"unevaluatedProperties" property support?
Brad, anyway you still need to add the syscon-property (pensando,*spics)
requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there
for instance for "baikal,bt1-sys-ssi" when it comes to the
vendor-specific properties definition in the allOf composition block.
You'll need to define a custom phandle property there in case if a
DT-node is compatible with you SPI controller.
-Sergey
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
Thanks
Brad
Hi Sergey,
On Tue, Nov 16, 2021 at 3:29 AM Serge Semin [off-list ref] wrote:
quoted
quoted
AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit, but for some
reason discarded from this series (btw why?). If you did you would
have got an error of an unevaluated property detection.
I ran the checks below and didn't get errors. Rob provided some info
and I found the server did not have yamllint installed (not flagged by
tool). Also dt-schema was not the latest. I'm re-doing this and
including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
quoted
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
Hmm, that's weird. Rob, does dtschema tool have the
"unevaluatedProperties" property support?
Brad, anyway you still need to add the syscon-property (pensando,*spics)
requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there
for instance for "baikal,bt1-sys-ssi" when it comes to the
vendor-specific properties definition in the allOf composition block.
You'll need to define a custom phandle property there in case if a
DT-node is compatible with you SPI controller.
Updating and adding only this bindings update to file
snps,dw-apb-ssi.yaml in 5.16.0-rc1 (next-20211116):
@@ -37,6 +37,21 @@ allOf:else:required:-interrupts+-if:+properties:+compatible:+contains:+enum:+-pensando,elba-spics+then:+properties:+pensando,spics:+$ref:/schemas/types.yaml#/definitions/phandle+description:+Phandle to the system control device node which provides access to+the spics control register+required:+-pensando,spicsproperties:compatible:
$ make ARCH=arm64 defconfig
...
$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
...
DTEX Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dts
DTC Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
CHECK Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml:0:0:
/example-0/spi@fff00000/flash@1: failed to match any schema with
compatible: ['spi-nand']
The spi-nand schema match failure happens before I make any change.
The tool also throws errors for these files which are unrelated
Documentation/devicetree/bindings/net/qcom,ipa.yaml: ignoring, error
in schema: properties: qcom,smem-state-names
Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml:
ignoring, error in schema: patternProperties: ^filter@[0-9]+$:
properties: st,adc-channel-names
Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml:
ignoring, error in schema: properties: qcom,bcm-voter-names
Thanks,
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Rob,
On Tue, Oct 26, 2021 at 11:10 AM Rob Herring [off-list ref] wrote:
On Sun, Oct 24, 2021 at 06:51:48PM -0700, Brad Larson wrote:
quoted
Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables resulting in an additional
reg property resource.
Signed-off-by: Brad Larson <redacted>
---
.../devicetree/bindings/mmc/cdns,sdhci.yaml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
If there is more than 1, then you need to describe what each entry is.
The dtschema update and yamllint install shows the errors your bot
reported. With the updated cdns,sdhci.yaml to add the description for
Elba's two reg items this is what I'm getting with in 5.16.0-rc1
(next-20211116):
"microchip,mpfs-sd4hc" was inadvertently removed in moving to
5.16.0-rc1 and won't be in the re-spin of the patchset, its recently
added.
Also, as you mentioned the patchset should be sent against rc1
Thanks
Brad
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Nov 16, 2021 at 03:11:53PM -0800, Brad Larson wrote:
quoted hunk
Hi Sergey,
On Tue, Nov 16, 2021 at 3:29 AM Serge Semin [off-list ref] wrote:
quoted
quoted
quoted
AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit, but for some
reason discarded from this series (btw why?). If you did you would
have got an error of an unevaluated property detection.
I ran the checks below and didn't get errors. Rob provided some info
and I found the server did not have yamllint installed (not flagged by
tool). Also dt-schema was not the latest. I'm re-doing this and
including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
quoted
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
Hmm, that's weird. Rob, does dtschema tool have the
"unevaluatedProperties" property support?
Brad, anyway you still need to add the syscon-property (pensando,*spics)
requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there
for instance for "baikal,bt1-sys-ssi" when it comes to the
vendor-specific properties definition in the allOf composition block.
You'll need to define a custom phandle property there in case if a
DT-node is compatible with you SPI controller.
Updating and adding only this bindings update to file
snps,dw-apb-ssi.yaml in 5.16.0-rc1 (next-20211116):
I was wrong using that construction here (fixup patch would be very
welcome) seeing the "snps,dw-apb-ssi" doesn't permit having a generic
"snps,dw*" compatible string. So just const-compatible property should
be enough:
+ compatible:
+ const: pensando,elba-spics
+ then:
+ properties:
+ pensando,spics:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the system control device node which provides access to
+ the spics control register
+ required:
+ - pensando,spics
Please note, I've asked to be more specific in this property naming.
Something like this should be fine
"pensando,elba-syscon-spics"/"pensando,syscon-spics".
$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
I am kind of surprised that this command is still evaluating all
the schemas. Compiling only the depended DT-schemas would much
better... Anyway that's why you are getting unrelated to the
snps,dw-apb-ssi.yaml errors.
...
DTEX Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dts
DTC Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
CHECK Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml:0:0:
/example-0/spi@fff00000/flash@1: failed to match any schema with
compatible: ['spi-nand']
That means there is no bindings defined for the "spi-nand"-compatible
node. AFAICS it doesn't make the tool to fail though. Anyway fixing this
part would be a subject of additional patches (which would be very
appreciated). That would concern converting the
Documentation/devicetree/bindings/mtd/spi-nand.txt legacy bindings to
the DT-schema. It's not enough though. Seeing that file lacks of
the NAND Flash specific DT-property description, you'd need to detach
ones (described by the "^nand@[a-f0-9]$"-pattern property) from the
Documentation/devicetree/bindings/mtd/nand-controller.yaml schema and
place them into a separate DT-schema file for generic nand-flashes
Documentation/devicetree/bindings/mtd/nand-flash.yaml in a framework
of a pre-requisite patch. Than in the legacy bindings conversion patch
you'd need to use it to correctly evaluate a generic NAND flash node.
Adding some example properties to the DT-schema would be also
required.
-Sergey
The spi-nand schema match failure happens before I make any change.
The tool also throws errors for these files which are unrelated
Documentation/devicetree/bindings/net/qcom,ipa.yaml: ignoring, error
in schema: properties: qcom,smem-state-names
Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml:
ignoring, error in schema: patternProperties: ^filter@[0-9]+$:
properties: st,adc-channel-names
Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml:
ignoring, error in schema: properties: qcom,bcm-voter-names
Thanks,
Brad
Hi Sergey,
On Wed, Nov 17, 2021 at 12:19 AM Serge Semin [off-list ref] wrote:
I was wrong using that construction here (fixup patch would be very
welcome) seeing the "snps,dw-apb-ssi" doesn't permit having a generic
"snps,dw*" compatible string. So just const-compatible property should
be enough:
+ compatible:
+ const: pensando,elba-spics
quoted
+ then:
+ properties:
+ pensando,spics:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the system control device node which provides access to
+ the spics control register
+ required:
quoted
+ - pensando,spics
Please note, I've asked to be more specific in this property naming.
Something like this should be fine
"pensando,elba-syscon-spics"/"pensando,syscon-spics".
I would have avoided a typo in the last reply if the spics property
was more specific. Based on needed construction like this?
DT:
spi0: spi@2800 {
compatible = "pensando,elba-spi";
reg = <0x0 0x2800 0x0 0x100>;
pensando,elba-syscon-spics = <&mssoc 0x2468>;
clocks = <&ahb_clk>;
interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
#address-cells = <1>;
#size-cells = <0>;
num-cs = <2>;
status = "disabled";
};
Binding: