linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v14 0/8] Support AMD Pensando Elba SoC
@ 2023-05-15 18:15 Brad Larson
  2023-05-15 18:15 ` [PATCH v14 1/8] dt-bindings: arm: add AMD Pensando boards Brad Larson
                   ` (9 more replies)
  0 siblings, 10 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:15 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

This series enables support for AMD 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

== V14 changes ==
Updated email list using get_maintainer.pl

Rebase to 6.4.0-rc1 and drop these patches (merged into 6.4.0)
- v13-0002-dt-bindings-mmc-cdns-Add-AMD-Pensando-Elba-SoC.patch
- v13-0004-dt-bindings-spi-dw-Add-AMD-Pensando-Elba-SoC-SPI.patch
- v13-0010-spi-dw-Add-support-for-AMD-Pensando-Elba-SoC.patch
- v13-0011-mmc-sdhci-cadence-Enable-device-specific-overrid.patch
- v13-0012-mmc-sdhci-cadence-Support-device-specific-init-d.patch
- v13-0013-mmc-sdhci-cadence-Add-AMD-Pensando-Elba-SoC-supp.patch
- v13-0014-mmc-sdhci-cadence-Support-mmc-hardware-reset.patch

v14-0003-dt-bindings-soc-amd-amd-pensando-elba-ctrl-Add-P.patch
- Change GPL-2.0-only or BSD-2-Clause to GPL-2.0-only OR BSD-2-Clause

v14-0006-arm64-dts-Add-AMD-Pensando-Elba-SoC-support.patch
- Fix dtbs_check l2-cache* property issue by adding required
  cache-level and cache-unified properties
- Observed the issue after updating dtschema from 2023.1 to 2023.4
  and yamllint from 1.26.3 to 1.30.0

v14-0008-soc-amd-Add-support-for-AMD-Pensando-SoC-Control.patch
- Save 8 bytes of code size by swapping spi_device and reset_controller_dev
  in penctrl_device
- Code simplification and clarity from review inputs
- Set penctrl_spi_driver.driver.name to match compatible pensando-elba-ctrl
- Remove unused include in amd-pensando-ctrl.h
- Fix compile error, class_create() API changed

== V13 changes ==
v13-0013-mmc-sdhci-cadence-Add-AMD-Pensando-Elba-SoC-supp
- Use GENMASK(7, 3) in elba_priv_writel() to set all byte enables 
- Add a variable 'shift' with GENMASK(1, 0) in elba_write_w() and
  elba_write_b() to set the byte enable variable.

v13-0015-soc-amd-Add-support-for-AMD-Pensando-SoC-Control
- Update include list in pensando-ctrl.c
- Change variable spi_dev to spi throughout
- Removed unneeded variable initialization, simplification of
  error checks, remove extra castings, and use dev_err_probe()
- Sort the includes in amd-pensando-ctrl.h
- Updates to cleanup if there is an error in penctrl_spi_probe()

== V12 changes ==
v12-0004-dt-bindings-spi-dw-Add-AMD-Pensando-Elba-SoC-SPI
- Correct property amd,pensando-elba-syscon description

v12-0010-spi-dw-Add-support-for-AMD-Pensando-Elba-SoC
- Add a newline in function dw_spi_elba_init()

v12-0015-soc-amd-Add-support-for-AMD-Pensando-SoC-Control
- Fix gcc-12.1.0 warning:

== V11 changes ==
v11-0002-dt-bindings-mmc-cdns-Add-AMD-Pensando-Elba-SoC
- Remove resets description and reset-names
- Add descriptions for amd,pensando-elba-sd4hc reg items

v11-0003-dt-bindings-spi-cdns-Add-compatible-for-AMD-Pens
- Removed redundant if/then for amd,pensando-elba-qspi

v11-0005-dt-bindings-soc-amd-amd-pensando-elba-ctrl-Add-P
- Fixed the compatible which should have stayed as
  'amd,pensando-elba-ctrl', the commit message, and the filename.
- Reference spi-peripheral-props
- Delete spi-max-frequency
- Remove num-cs from example

v11-0008-arm64-dts-Add-AMD-Pensando-Elba-SoC-support
- Delete reset-names
- Fix spi0 compatible to be specific 'amd,pensando-elba-ctrl'

v11-0010-spi-dw-Add-support-for-AMD-Pensando-Elba-SoC
- Simplify dw_spi_elb_init by using syscon_regmap_lookup_by_phandle()

v11-0013-mmc-sdhci-cadence-Add-AMD-Pensando-Elba-SoC-supp
- Remove elba-drv_init() call to platform_get_resource() since that
  check is done inside devm_platform_ioremap_resource()
- Move spin_lock_init() before error check
- Remove extra parentheses

v11-0015-soc-amd-Add-support-for-AMD-Pensando-SoC-Control
- Fix the compatible to be specific 'amd,pensando-elba-ctrl'
- Cast arguments flagged with a gcc-12.1.0 warning:
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202303061526.I8VPcR1M-lkp@intel.com/

== V10 changes ==
Binding property amd,pensando-elba-syscon was merged in 6.2

v10-0002-dt-bindings-mmc-cdns-Add-AMD-Pensando-Elba-SoC
- Move reset-names property definition next to existing resets prop
- Move allOf to the bottom and set resets/reset-names required only for pensando
- Fix reg maxItems for existing, must be 1

v10-0003-dt-bindings-spi-cdns-Add-compatible-for-AMD-Pens
- Fix cdns,fifo-depth, only amd,pensando-elba-qspi is 1024 bytes

v10-0004-dt-bindings-spi-dw-Add-AMD-Pensando-Elba-SoC-SPI
- Move definition of amd,pensando-elba-syscon into properties with a better description
- Add amd,pensando-elba-syscon: false for non elba designs

v10-0005-dt-bindings-soc-amd-amd-pensando-elbasr-Add-AMD-
- Property renamed to amd,pensando-ctrl
- Driver is renamed and moved to soc/drivers/amd affecting binding
- Delete cs property, driver handles device node creation from parent num-cs
  fixing schema reg error in a different way

v10-0010-spi-dw-Add-support-for-AMD-Pensando-Elba-SoC
- Delete struct dw_spi_elba, use regmap directly in priv

v10-0011-mmc-sdhci-cadence-Enable-device-specific-overrid
- The 1st patch adding private writel() is unchanged.  The 2nd patch is split
  into two patches to provide for device specific init in one patch with no
  effect on existing designs.  Then add the pensando support into the next patch.
  Then the 4th patch is mmc hardware reset support which is unchanged.

v10-0012-mmc-sdhci-cadence-Support-device-specific-init-i
- New patch to provide for platform specific init() with no change
  to existing designs.

v10-0013-mmc-sdhci-cadence-Add-AMD-Pensando-Elba-SoC-supp
- Add Elba specific support into this 3rd patch.  This builds on the private
  writel() enabled in patch 1 followed by platform specific init() in patch 2.
- Specify when first used the reason for the spinlock use to order byte-enable
  prior to write data.

v10-0015-soc-amd-Add-support-for-AMD-Pensando-SoC-Control
- Different driver implementation specific to this Pensando controller device.
- Moved to soc/amd directory under new name based on guidance.  This driver is
  of no use to any design other than all Pensando SoC based cards.
- Removed use of builtin_driver, can be built as a module.

== V9 changes ==
v9-0002-dt-bindings-mmc-cdns-Add-AMD-Pensando-Elba-SoC
- Add reset-names and resets properties
- Add if/then on property amd,pensando-elba-sd4hc to set reg property
  values for minItems and maxItems

v9-0003-dt-bindings-spi-cdns-Add-compatible-for-AMD-Pensa
- Add 1024 to cdns,fifo-depth property to resolve dtbs_check error

v9-0004-dt-bindings-spi-dw-Add-AMD-Pensando-Elba-SoC-SPI-
- Define property amd,pensando-elba-syscon
- Move compatible amd,pensando-elba-spi ahead of baikal,bt1-ssi

v9-0006-dt-bindings-mfd-amd-pensando-elbasr-Add-AMD-Pensa
- Instead of four nodes, one per chip-select, a single
  node is used with reset-cells in the parent.
- No MFD API is used anymore in the driver so it made
  sense to move this to drivers/spi.
- This driver is common for all Pensando SoC based designs
  so changed the name to pensando-sr.c to not make it Elba
  SoC specific.
- Added property cs for the chip-select number which is used
  by the driver to create /dev/pensr0.<cs>

v9-0009-arm64-dts-Add-AMD-Pensando-Elba-SoC-support
- Single node for spi0 system-controller and squash
  the reset-controller child into parent

v9-0010-spi-cadence-quadspi-Add-compatible-for-AMD-Pensan
- Rebase to linux-next 6.2.0-rc1

v9-0011-spi-dw-Add-support-for-AMD-Pensando-Elba-SoC
- Add use of macros GENMASK() and BIT()
- Change ELBA_SPICS_SHIFT() to ELBA_SPICS_OFFSET()

v9-0012-mmc-sdhci-cadence-Enable-device-specific-override
- No change to this patch but as some patches are deleted and this is
  a respin the three successive patches to sdhci-cadence.c are
  patches 12, 13, and 14 which do the following:
  1. Add ability for Cadence specific design to have priv writel().
  2. Add Elba SoC support that requires its own priv writel() for
     byte-lane control .
  3. Add support for mmc hardware reset.

v9-0014-mmc-sdhci-cadence-Support-mmc-hardware-reset
- Previously patch 17/17
- Changed delay after reset_control_assert() from 9 to 3 usec
- Renamed sdhci_mmc_hw_reset() to sdhci_cdns_mmc_hw_reset()

v9-0015-spi-pensando-sr-Add-AMD-Pensando-SoC-System-Resou
- Previously patch 14/17
- After the change to the device tree node and squashing
  reset-cells into the parent simplified this to not use
  any MFD API and move it to drivers/spi/pensando-sr.c.
- Change the naming to remove elba since this driver is common
  for all Pensando SoC designs .
- Default yes SPI_PENSANDO_SR for ARCH_PENSANDO


== V6 changes ==
- Updated copyright and SPDX

v6-0001-dt-bindings-arm-add-AMD-Pensando-boards
- Delete 'Device Tree Bindings' in title

v6-0002-dt-bindings-mmc-cdns-Add-AMD-Pensando-Elba-SoC
- Change if/then for Elba which has a second reg for byte-lane control

v6-0003-dt-bindings-spi-cdns-Add-compatible-for-AMD-Pensa
- no change

v6-0004-dt-bindings-spi-dw-Add-AMD-Pensando-Elba-SoC-SPI-
- Add amd,pensando-elba-syscon

v6-0005-dt-bindings-mfd-syscon-Add-amd-pensando-elba-sysc
- no change

v6-0006-dt-bindings-mfd-amd-pensando-elbasr-Add-AMD-Pensa
- Expand description, rename nodes and change compatible usage

v6-0007-dt-bindings-reset-amd-pensando-elbasr-reset-Add-A
- Delete nodename pattern and changed spi0 to spi
- File amd,pensando-elba-reset.h is deleted as there is only
  one reset used.
- Update example

v6-0008-MAINTAINERS-Add-entry-for-AMD-PENSANDO
- no change

v6-0009-arm64-Add-config-for-AMD-Pensando-SoC-platforms
- no change

v6-0010-arm64-dts-Add-AMD-Pensando-Elba-SoC-support
- Update node names and add amd,pensando-elba-syscon
- Delete use of amd,pensando-elba-reset.h which had a single definition

v6-0011-spi-cadence-quadspi-Add-compatible-for-AMD-Pensan
- Remove (void) cast

v6-0012-spi-dw-Add-support-for-AMD-Pensando-Elba-SoC
- Update use of amd,pensando-elba-syscon

v6-0013-mmc-sdhci-cadence-Enable-device-specific-override
- Change this patch to add a priv_writel() callback where all
  existing designs use writel().  This separates the Elba
  support into three patches.  The second patch is added
  to the end of the sequence for Elba support.  The third
  patch enables mmc hardware reset.

v6-0014-mfd-pensando-elbasr-Add-AMD-Pensando-Elba-System-
- Updates from review comments
- Use spi_message_init_with_transfers instead of init/add_tail API

v6-0015-reset-elbasr-Add-AMD-Pensando-Elba-SR-Reset-Contr
- Remove use of amd,pensando-elba-reset.h and use BIT()

v6-0016-mmc-sdhci-cadence-Add-AMD-Pensando-Elba-SoC-suppo
- Elba sdhci-cadence.c support added in this patch to build on
  0013 which just adds a callback to override priv_writel()

v6-0017-mmc-sdhci-cadence-Support-mmc-hardware-reset
- New patch where Elba has a reset-controller for mmc hardware
  reset.  The reset is implemented by a register in the cpld.

== V5 changes ==
- Change to AMD Pensando instead of Pensando.
- No reference to spidev in the device tree.  Add multi-function driver
  pensando-elbasr and sub-device reset-elbasr which provides mfd and
  /dev interface to the cpld.
- Rebase to linux-next tag next-20220609 5.19.0-rc1
- Redo the email list after rebase and using scripts/get_maintainer.pl

== V4 changes ==
The version of dtschema used is 2022.3.2.

v4-0001-dt-bindings-arm-add-Pensando-boards.patch
- Add description and board compatible

v4-0003-dt-bindings-mmc-Add-Pensando-Elba-SoC-binding.patch
- Change from elba-emmc to elba-sd4hc to match file convention
- Use minItems: 1 and maxItems: 2 to pass schema check

v4-0005-dt-bindings-spi-dw-Add-Pensando-Elba-SoC-SPI-Control.patch
- Add required property pensando,syscon-spics to go with
  pensando,elba-spi

v4-0006-MAINTAINERS-Add-entry-for-PENSANDO.patch
- Change Maintained to Supported

v4-0007-arm64-Add-config-for-Pensando-SoC-platforms.patch
- Fix a typo on interface max speed

v4-0008-spi-cadence-quadspi-Add-compatible-for-Pensando-Elba.patch
- Update due to spi-cadence-quadspi.c changes

v4-0009-mmc-sdhci-cadence-Add-Pensando-Elba-SoC-support.patch
- Change from elba-emmc to elba-sd4hc to match file convention

v4-0010-spi-dw-Add-support-for-Pensando-Elba-SoC.patch
- Use more descriptive dt property pensando,syscon-spics
- Minor changes from review input

v4-0011-arm64-dts-Add-Pensando-Elba-SoC-support.patch
- Changed to dual copyright (GPL-2.0+ OR MIT)
- Minor changes from review input

== V3 changes ==
v3-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.

v3-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.

v3-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).

v3-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.

v3-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_

v3-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.

v3-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.

v3-0010-dt-bindings-spi-cadence-qspi-Add-support-for-Pensand.patch
- Updated since the latest documentation has been converted to yaml

v3-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.

== V2 changes ==
- 01    Fix typo, return code value and log message.
- 03    Remove else clause, intrinsic DW chip-select is never used.
- 08-11 Split out dts and bindings to sub-patches
- 10    Converted existing cadence-quadspi.txt to YAML schema
- 13    New driver should use <linux/gpio/driver.h>

Brad Larson (8):
  dt-bindings: arm: add AMD Pensando boards
  dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC
  dt-bindings: soc: amd: amd,pensando-elba-ctrl: Add Pensando SoC
    Controller
  MAINTAINERS: Add entry for AMD PENSANDO
  arm64: Add config for AMD Pensando SoC platforms
  arm64: dts: Add AMD Pensando Elba SoC support
  spi: cadence-quadspi: Add compatible for AMD Pensando Elba SoC
  soc: amd: Add support for AMD Pensando SoC Controller

 .../devicetree/bindings/arm/amd,pensando.yaml |  26 ++
 .../soc/amd/amd,pensando-elba-ctrl.yaml       |  58 +++
 .../bindings/spi/cdns,qspi-nor.yaml           |  18 +-
 MAINTAINERS                                   |   9 +
 arch/arm64/Kconfig.platforms                  |  12 +
 arch/arm64/boot/dts/amd/Makefile              |   1 +
 arch/arm64/boot/dts/amd/elba-16core.dtsi      | 197 ++++++++++
 arch/arm64/boot/dts/amd/elba-asic-common.dtsi |  80 ++++
 arch/arm64/boot/dts/amd/elba-asic.dts         |  28 ++
 arch/arm64/boot/dts/amd/elba-flash-parts.dtsi | 106 +++++
 arch/arm64/boot/dts/amd/elba.dtsi             | 191 +++++++++
 drivers/soc/Kconfig                           |   1 +
 drivers/soc/Makefile                          |   1 +
 drivers/soc/amd/Kconfig                       |  16 +
 drivers/soc/amd/Makefile                      |   2 +
 drivers/soc/amd/pensando-ctrl.c               | 368 ++++++++++++++++++
 drivers/spi/spi-cadence-quadspi.c             |  19 +
 include/uapi/linux/amd-pensando-ctrl.h        |  29 ++
 18 files changed, 1160 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amd,pensando.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/amd/amd,pensando-elba-ctrl.yaml
 create mode 100644 arch/arm64/boot/dts/amd/elba-16core.dtsi
 create mode 100644 arch/arm64/boot/dts/amd/elba-asic-common.dtsi
 create mode 100644 arch/arm64/boot/dts/amd/elba-asic.dts
 create mode 100644 arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
 create mode 100644 arch/arm64/boot/dts/amd/elba.dtsi
 create mode 100644 drivers/soc/amd/Kconfig
 create mode 100644 drivers/soc/amd/Makefile
 create mode 100644 drivers/soc/amd/pensando-ctrl.c
 create mode 100644 include/uapi/linux/amd-pensando-ctrl.h


base-commit: e922ba281a8d84f640d8c8e18a385d032c19e185
-- 
2.17.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v14 1/8] dt-bindings: arm: add AMD Pensando boards
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
@ 2023-05-15 18:15 ` Brad Larson
  2023-05-15 18:16 ` [PATCH v14 2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC Brad Larson
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:15 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

Document the compatible for AMD Pensando Elba SoC boards.

Signed-off-by: Brad Larson <blarson@amd.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/arm/amd,pensando.yaml | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/amd,pensando.yaml

diff --git a/Documentation/devicetree/bindings/arm/amd,pensando.yaml b/Documentation/devicetree/bindings/arm/amd,pensando.yaml
new file mode 100644
index 000000000000..e5c2591834a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/amd,pensando.yaml
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/amd,pensando.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando SoC Platforms
+
+maintainers:
+  - Brad Larson <blarson@amd.com>
+
+properties:
+  $nodename:
+    const: "/"
+  compatible:
+    oneOf:
+
+      - description: Boards with Pensando Elba SoC
+        items:
+          - enum:
+              - amd,pensando-elba-ortano
+          - const: amd,pensando-elba
+
+additionalProperties: true
+
+...
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
  2023-05-15 18:15 ` [PATCH v14 1/8] dt-bindings: arm: add AMD Pensando boards Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-16 15:18   ` Mark Brown
  2023-05-15 18:16 ` [PATCH v14 3/8] dt-bindings: soc: amd: amd,pensando-elba-ctrl: Add Pensando SoC Controller Brad Larson
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

Document the cadence qspi controller compatible for AMD Pensando
Elba SoC boards.  The Elba qspi fifo size is 1024.

Signed-off-by: Brad Larson <blarson@amd.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---

v14 changes:
- Rebase to 6.4.0-rc1 

v11 changes:
- Removed redundant if/then for amd,pensando-elba-qspi

v10 changes:
- Fix cdns,fifo-depth, only amd,pensando-elba-qspi is 1024 bytes

v9 changes:
- Add 1024 to cdns,fifo-depth property to resolve dtbs_check error

---
 .../devicetree/bindings/spi/cdns,qspi-nor.yaml | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index b310069762dd..4f15f9a0cc34 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -46,12 +46,28 @@ allOf:
           maxItems: 2
           items:
             enum: [ qspi, qspi-ocp ]
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: amd,pensando-elba-qspi
+    then:
+      properties:
+        cdns,fifo-depth:
+          enum: [ 128, 256, 1024 ]
+          default: 1024
+    else:
+      properties:
+        cdns,fifo-depth:
+          enum: [ 128, 256 ]
+          default: 128
 
 properties:
   compatible:
     oneOf:
       - items:
           - enum:
+              - amd,pensando-elba-qspi
               - ti,k2g-qspi
               - ti,am654-ospi
               - intel,lgm-qspi
@@ -76,8 +92,6 @@ properties:
     description:
       Size of the data FIFO in words.
     $ref: /schemas/types.yaml#/definitions/uint32
-    enum: [ 128, 256 ]
-    default: 128
 
   cdns,fifo-width:
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 3/8] dt-bindings: soc: amd: amd,pensando-elba-ctrl: Add Pensando SoC Controller
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
  2023-05-15 18:15 ` [PATCH v14 1/8] dt-bindings: arm: add AMD Pensando boards Brad Larson
  2023-05-15 18:16 ` [PATCH v14 2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-15 18:16 ` [PATCH v14 4/8] MAINTAINERS: Add entry for AMD PENSANDO Brad Larson
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

Support the AMD Pensando Elba SoC Controller which is a SPI connected
device providing a miscellaneous set of essential board control/status
registers.  This device is present in all Pensando SoC based designs.

Signed-off-by: Brad Larson <blarson@amd.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---

v14 changes:
- Change GPL-2.0-only or BSD-2-Clause to GPL-2.0-only OR BSD-2-Clause

v11 changes:
- Fixed the compatible which should have stayed as 'amd,pensando-elba-ctrl',
  the commit message, and the filename
- Reference spi-peripheral-props
- Delete spi-max-frequency
- Remove num-cs from example

v10 changes:
- Property renamed to amd,pensando-ctrl
- Driver is renamed and moved to soc/drivers/amd affecting binding
- Delete cs property, driver handles device node creation from parent num-cs
  fixing schema reg error in a different way

v9 changes:
- Instead of four nodes, one per chip-select, a single
  node is used with reset-cells in the parent.
- No MFD API is used anymore in the driver so it made
  sense to move this to drivers/spi.
- This driver is common for all Pensando SoC based designs
  so changed the name to pensando-sr.c to not make it Elba
  SoC specific.
- Added property cs for the chip-select number which is used
  by the driver to create /dev/pensr0.<cs>

---
 .../soc/amd/amd,pensando-elba-ctrl.yaml       | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/amd/amd,pensando-elba-ctrl.yaml

diff --git a/Documentation/devicetree/bindings/soc/amd/amd,pensando-elba-ctrl.yaml b/Documentation/devicetree/bindings/soc/amd/amd,pensando-elba-ctrl.yaml
new file mode 100644
index 000000000000..e96978ad1e37
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/amd/amd,pensando-elba-ctrl.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/amd/amd,pensando-elba-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD Pensando Elba SoC Controller
+
+description:
+  The AMD Pensando Elba SoC Controller is a SPI connected device with essential
+  control/status registers accessed on chip select 0.  This device is present
+  in all Pensando SoC based designs.
+
+maintainers:
+  - Brad Larson <blarson@amd.com>
+
+properties:
+  compatible:
+    enum:
+      - amd,pensando-elba-ctrl
+
+  reg:
+    maxItems: 1
+
+  '#reset-cells':
+    const: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - '#reset-cells'
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        system-controller@0 {
+            compatible = "amd,pensando-elba-ctrl";
+            reg = <0>;
+            spi-max-frequency = <12000000>;
+            interrupt-parent = <&porta>;
+            interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+            #reset-cells = <1>;
+        };
+    };
+
+...
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 4/8] MAINTAINERS: Add entry for AMD PENSANDO
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (2 preceding siblings ...)
  2023-05-15 18:16 ` [PATCH v14 3/8] dt-bindings: soc: amd: amd,pensando-elba-ctrl: Add Pensando SoC Controller Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-15 18:16 ` [PATCH v14 5/8] arm64: Add config for AMD Pensando SoC platforms Brad Larson
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

Add entry for AMD PENSANDO maintainer and files

Signed-off-by: Brad Larson <blarson@amd.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 638d83d5e135..28483bad27c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1883,6 +1883,15 @@ N:	allwinner
 N:	sun[x456789]i
 N:	sun[25]0i
 
+ARM/AMD PENSANDO ARM64 ARCHITECTURE
+M:	Brad Larson <blarson@amd.com>
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Supported
+F:	Documentation/devicetree/bindings/*/amd,pensando*
+F:	Documentation/devicetree/bindings/soc/amd/amd,pensando*
+F:	arch/arm64/boot/dts/amd/elba*
+F:	drivers/soc/amd/
+
 ARM/Amlogic Meson SoC CLOCK FRAMEWORK
 M:	Neil Armstrong <neil.armstrong@linaro.org>
 M:	Jerome Brunet <jbrunet@baylibre.com>
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 5/8] arm64: Add config for AMD Pensando SoC platforms
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (3 preceding siblings ...)
  2023-05-15 18:16 ` [PATCH v14 4/8] MAINTAINERS: Add entry for AMD PENSANDO Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-15 18:16 ` [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support Brad Larson
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

Add ARCH_PENSANDO configuration option for AMD Pensando SoC
based platforms.

Signed-off-by: Brad Larson <blarson@amd.com>
---
 arch/arm64/Kconfig.platforms | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 89a0b13b058d..3510daaabe27 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -236,6 +236,18 @@ config ARCH_NPCM
 	  General support for NPCM8xx BMC (Arbel).
 	  Nuvoton NPCM8xx BMC based on the Cortex A35.
 
+config ARCH_PENSANDO
+	bool "AMD Pensando Platforms"
+	help
+	  This enables support for the ARMv8 based AMD Pensando SoC
+	  family to include the Elba SoC.
+
+	  AMD Pensando SoCs support a range of Distributed Services
+	  Cards in PCIe format installed into servers.  The Elba
+	  SoC includes 16 A-72 CPU cores, 144 programmable P4
+	  cores for a minimal latency/jitter datapath, and network
+	  interfaces up to 200 Gb/s.
+
 config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select GPIOLIB
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (4 preceding siblings ...)
  2023-05-15 18:16 ` [PATCH v14 5/8] arm64: Add config for AMD Pensando SoC platforms Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-16  7:17   ` Krzysztof Kozlowski
  2023-05-16  7:54   ` Michal Simek
  2023-05-15 18:16 ` [PATCH v14 7/8] spi: cadence-quadspi: Add compatible for AMD Pensando Elba SoC Brad Larson
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

Add AMD Pensando common and Elba SoC specific device nodes

Signed-off-by: Brad Larson <blarson@amd.com>
---

v14 changes:
- Fix dtbs_check l2-cache* property issue by adding required
  cache-level and cache-unified properties
- Observed the issue after updating dtschema from 2023.1 to 2023.4
  and yamllint from 1.26.3 to 1.30.0

v11 changes:
- Delete reset-names
- Fix spi0 compatible to be specific 'amd,pensando-elba-ctrl'

v9 changes:
- Single node for spi0 system-controller and squash
  the reset-controller child into parent

---
 arch/arm64/boot/dts/amd/Makefile              |   1 +
 arch/arm64/boot/dts/amd/elba-16core.dtsi      | 197 ++++++++++++++++++
 arch/arm64/boot/dts/amd/elba-asic-common.dtsi |  80 +++++++
 arch/arm64/boot/dts/amd/elba-asic.dts         |  28 +++
 arch/arm64/boot/dts/amd/elba-flash-parts.dtsi | 106 ++++++++++
 arch/arm64/boot/dts/amd/elba.dtsi             | 191 +++++++++++++++++
 6 files changed, 603 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amd/elba-16core.dtsi
 create mode 100644 arch/arm64/boot/dts/amd/elba-asic-common.dtsi
 create mode 100644 arch/arm64/boot/dts/amd/elba-asic.dts
 create mode 100644 arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
 create mode 100644 arch/arm64/boot/dts/amd/elba.dtsi

diff --git a/arch/arm64/boot/dts/amd/Makefile b/arch/arm64/boot/dts/amd/Makefile
index 68103a8b0ef5..8502cc2afbc5 100644
--- a/arch/arm64/boot/dts/amd/Makefile
+++ b/arch/arm64/boot/dts/amd/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_ARCH_PENSANDO) += elba-asic.dtb
 dtb-$(CONFIG_ARCH_SEATTLE) += amd-overdrive-rev-b0.dtb amd-overdrive-rev-b1.dtb
diff --git a/arch/arm64/boot/dts/amd/elba-16core.dtsi b/arch/arm64/boot/dts/amd/elba-16core.dtsi
new file mode 100644
index 000000000000..f9f9f5fd5f69
--- /dev/null
+++ b/arch/arm64/boot/dts/amd/elba-16core.dtsi
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+/*
+ * Copyright 2020-2022 Advanced Micro Devices, Inc.
+ */
+
+/ {
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 { cpu = <&cpu0>; };
+				core1 { cpu = <&cpu1>; };
+				core2 { cpu = <&cpu2>; };
+				core3 { cpu = <&cpu3>; };
+			};
+
+			cluster1 {
+				core0 { cpu = <&cpu4>; };
+				core1 { cpu = <&cpu5>; };
+				core2 { cpu = <&cpu6>; };
+				core3 { cpu = <&cpu7>; };
+			};
+
+			cluster2 {
+				core0 { cpu = <&cpu8>; };
+				core1 { cpu = <&cpu9>; };
+				core2 { cpu = <&cpu10>; };
+				core3 { cpu = <&cpu11>; };
+			};
+
+			cluster3 {
+				core0 { cpu = <&cpu12>; };
+				core1 { cpu = <&cpu13>; };
+				core2 { cpu = <&cpu14>; };
+				core3 { cpu = <&cpu15>; };
+			};
+		};
+
+		/* CLUSTER 0 */
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x0>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x1>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x2>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x3>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		l2_0: l2-cache0 {
+			compatible = "cache";
+			cache-unified;
+			cache-level = <2>;
+		};
+
+		/* CLUSTER 1 */
+		cpu4: cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x100>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		cpu5: cpu@101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x101>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		cpu6: cpu@102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x102>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		cpu7: cpu@103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x103>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		l2_1: l2-cache1 {
+			compatible = "cache";
+			cache-unified;
+			cache-level = <2>;
+		};
+
+		/* CLUSTER 2 */
+		cpu8: cpu@200 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x200>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		cpu9: cpu@201 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x201>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		cpu10: cpu@202 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x202>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		cpu11: cpu@203 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x203>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		l2_2: l2-cache2 {
+			compatible = "cache";
+			cache-unified;
+			cache-level = <2>;
+		};
+
+		/* CLUSTER 3 */
+		cpu12: cpu@300 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x300>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		cpu13: cpu@301 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x301>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		cpu14: cpu@302 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x302>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		cpu15: cpu@303 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x303>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		l2_3: l2-cache3 {
+			compatible = "cache";
+			cache-unified;
+			cache-level = <2>;
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/amd/elba-asic-common.dtsi b/arch/arm64/boot/dts/amd/elba-asic-common.dtsi
new file mode 100644
index 000000000000..1a615788f54e
--- /dev/null
+++ b/arch/arm64/boot/dts/amd/elba-asic-common.dtsi
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+/*
+ * Copyright 2020-2022 Advanced Micro Devices, Inc.
+ */
+
+&ahb_clk {
+	clock-frequency = <400000000>;
+};
+
+&emmc_clk {
+	clock-frequency = <200000000>;
+};
+
+&flash_clk {
+	clock-frequency = <400000000>;
+};
+
+&ref_clk {
+	clock-frequency = <156250000>;
+};
+
+&qspi {
+	status = "okay";
+
+	flash0: flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <40000000>;
+		spi-rx-bus-width = <2>;
+		m25p,fast-read;
+		cdns,read-delay = <0>;
+		cdns,tshsl-ns = <0>;
+		cdns,tsd2d-ns = <0>;
+		cdns,tchsh-ns = <0>;
+		cdns,tslch-ns = <0>;
+	};
+};
+
+&gpio0 {
+	status = "okay";
+};
+
+&emmc {
+	bus-width = <8>;
+	cap-mmc-hw-reset;
+	resets = <&rstc 0>;
+	status = "okay";
+};
+
+&wdt0 {
+	status = "okay";
+};
+
+&i2c0 {
+	clock-frequency = <100000>;
+	status = "okay";
+
+	rtc@51 {
+		compatible = "nxp,pcf85263";
+		reg = <0x51>;
+	};
+};
+
+&spi0 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	num-cs = <4>;
+	cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
+		   <&porta 7 GPIO_ACTIVE_LOW>;
+	status = "okay";
+
+	rstc: system-controller@0 {
+		compatible = "amd,pensando-elba-ctrl";
+		reg = <0>;
+		spi-max-frequency = <12000000>;
+		interrupt-parent = <&porta>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		#reset-cells = <1>;
+	};
+};
diff --git a/arch/arm64/boot/dts/amd/elba-asic.dts b/arch/arm64/boot/dts/amd/elba-asic.dts
new file mode 100644
index 000000000000..c3f4da2f7449
--- /dev/null
+++ b/arch/arm64/boot/dts/amd/elba-asic.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+/*
+ * Device Tree file for AMD Pensando Elba Board.
+ *
+ * Copyright 2020-2022 Advanced Micro Devices, Inc.
+ */
+
+/dts-v1/;
+
+#include "elba.dtsi"
+#include "elba-16core.dtsi"
+#include "elba-asic-common.dtsi"
+#include "elba-flash-parts.dtsi"
+
+/ {
+	model = "AMD Pensando Elba Board";
+	compatible = "amd,pensando-elba-ortano", "amd,pensando-elba";
+
+	aliases {
+		serial0 = &uart0;
+		spi0 = &spi0;
+		spi1 = &qspi;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
diff --git a/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
new file mode 100644
index 000000000000..734893fef2c3
--- /dev/null
+++ b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+/*
+ * Copyright 2020-2022 Advanced Micro Devices, Inc.
+ */
+
+&flash0 {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		partition@0 {
+			label = "flash";
+			reg = <0x10000 0xfff0000>;
+		};
+
+		partition@f0000 {
+			label = "golduenv";
+			reg = <0xf0000 0x10000>;
+		};
+
+		partition@100000 {
+			label = "boot0";
+			reg = <0x100000 0x80000>;
+		};
+
+		partition@180000 {
+			label = "golduboot";
+			reg = <0x180000 0x200000>;
+		};
+
+		partition@380000 {
+			label = "brdcfg0";
+			reg = <0x380000 0x10000>;
+		};
+
+		partition@390000 {
+			label = "brdcfg1";
+			reg = <0x390000 0x10000>;
+		};
+
+		partition@400000 {
+			label = "goldfw";
+			reg = <0x400000 0x3c00000>;
+		};
+
+		partition@4010000 {
+			label = "fwmap";
+			reg = <0x4010000 0x20000>;
+		};
+
+		partition@4030000 {
+			label = "fwsel";
+			reg = <0x4030000 0x20000>;
+		};
+
+		partition@4090000 {
+			label = "bootlog";
+			reg = <0x4090000 0x20000>;
+		};
+
+		partition@40b0000 {
+			label = "panicbuf";
+			reg = <0x40b0000 0x20000>;
+		};
+
+		partition@40d0000 {
+			label = "uservars";
+			reg = <0x40d0000 0x20000>;
+		};
+
+		partition@4200000 {
+			label = "uboota";
+			reg = <0x4200000 0x400000>;
+		};
+
+		partition@4600000 {
+			label = "ubootb";
+			reg = <0x4600000 0x400000>;
+		};
+
+		partition@4a00000 {
+			label = "mainfwa";
+			reg = <0x4a00000 0x1000000>;
+		};
+
+		partition@5a00000 {
+			label = "mainfwb";
+			reg = <0x5a00000 0x1000000>;
+		};
+
+		partition@6a00000 {
+			label = "diaguboot";
+			reg = <0x6a00000 0x400000>;
+		};
+
+		partition@8000000 {
+			label = "diagfw";
+			reg = <0x8000000 0x7fe0000>;
+		};
+
+		partition@ffe0000 {
+			label = "ubootenv";
+			reg = <0xffe0000 0x10000>;
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/amd/elba.dtsi b/arch/arm64/boot/dts/amd/elba.dtsi
new file mode 100644
index 000000000000..674890cf2a34
--- /dev/null
+++ b/arch/arm64/boot/dts/amd/elba.dtsi
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+/*
+ * Copyright 2020-2022 Advanced Micro Devices, Inc.
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include "dt-bindings/interrupt-controller/arm-gic.h"
+
+/ {
+	model = "Elba ASIC Board";
+	compatible = "amd,pensando-elba";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	dma-coherent;
+
+	ahb_clk: oscillator0 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	emmc_clk: oscillator2 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	flash_clk: oscillator3 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	ref_clk: oscillator4 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	pmu {
+		compatible = "arm,cortex-a72-pmu";
+		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	soc: soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		i2c0: i2c@400 {
+			compatible = "snps,designware-i2c";
+			reg = <0x0 0x400 0x0 0x100>;
+			clocks = <&ahb_clk>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			i2c-sda-hold-time-ns = <480>;
+			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		wdt0: watchdog@1400 {
+			compatible = "snps,dw-wdt";
+			reg = <0x0 0x1400 0x0 0x100>;
+			clocks = <&ahb_clk>;
+			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		qspi: spi@2400 {
+			compatible = "amd,pensando-elba-qspi", "cdns,qspi-nor";
+			reg = <0x0 0x2400 0x0 0x400>,
+			      <0x0 0x7fff0000 0x0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&flash_clk>;
+			cdns,fifo-depth = <1024>;
+			cdns,fifo-width = <4>;
+			cdns,trigger-address = <0x7fff0000>;
+			status = "disabled";
+		};
+
+		spi0: spi@2800 {
+			compatible = "amd,pensando-elba-spi";
+			reg = <0x0 0x2800 0x0 0x100>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			amd,pensando-elba-syscon = <&syscon>;
+			clocks = <&ahb_clk>;
+			interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+			num-cs = <2>;
+			status = "disabled";
+		};
+
+		gpio0: gpio@4000 {
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x0 0x4000 0x0 0x78>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			porta: gpio-port@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				reg = <0>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				ngpios = <8>;
+				interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-controller;
+				interrupt-parent = <&gic>;
+				#interrupt-cells = <2>;
+			};
+
+			portb: gpio-port@1 {
+				compatible = "snps,dw-apb-gpio-port";
+				reg = <1>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				ngpios = <8>;
+			};
+		};
+
+		uart0: serial@4800 {
+			compatible = "ns16550a";
+			reg = <0x0 0x4800 0x0 0x100>;
+			clocks = <&ref_clk>;
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+		};
+
+		gic: interrupt-controller@800000 {
+			compatible = "arm,gic-v3";
+			reg = <0x0 0x800000 0x0 0x200000>,	/* GICD */
+			      <0x0 0xa00000 0x0 0x200000>,	/* GICR */
+			      <0x0 0x60000000 0x0 0x2000>,	/* GICC */
+			      <0x0 0x60010000 0x0 0x1000>,	/* GICH */
+			      <0x0 0x60020000 0x0 0x2000>;	/* GICV */
+			#address-cells = <2>;
+			#size-cells = <2>;
+			#interrupt-cells = <3>;
+			ranges;
+			interrupt-controller;
+			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+
+			/*
+			 * Elba specific pre-ITS is enabled using the
+			 * existing property socionext,synquacer-pre-its
+			 */
+			gic_its: msi-controller@820000 {
+				compatible = "arm,gic-v3-its";
+				reg = <0x0 0x820000 0x0 0x10000>;
+				msi-controller;
+				#msi-cells = <1>;
+				socionext,synquacer-pre-its =
+							<0xc00000 0x1000000>;
+			};
+		};
+
+		emmc: mmc@30440000 {
+			compatible = "amd,pensando-elba-sd4hc", "cdns,sd4hc";
+			reg = <0x0 0x30440000 0x0 0x10000>,
+			      <0x0 0x30480044 0x0 0x4>;	/* byte-lane ctrl */
+			clocks = <&emmc_clk>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+			cdns,phy-input-delay-sd-highspeed = <0x4>;
+			cdns,phy-input-delay-legacy = <0x4>;
+			cdns,phy-input-delay-sd-uhs-sdr50 = <0x6>;
+			cdns,phy-input-delay-sd-uhs-ddr50 = <0x16>;
+			mmc-ddr-1_8v;
+			status = "disabled";
+		};
+
+		syscon: syscon@307c0000 {
+			compatible = "amd,pensando-elba-syscon", "syscon";
+			reg = <0x0 0x307c0000 0x0 0x3000>;
+		};
+	};
+};
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 7/8] spi: cadence-quadspi: Add compatible for AMD Pensando Elba SoC
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (5 preceding siblings ...)
  2023-05-15 18:16 ` [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

The AMD 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 <blarson@amd.com>
---

v14 changes:
- Rebase to linux-next 6.4.0-rc1

v9 changes:
- Rebase to linux-next 6.2.0-rc1

---
 drivers/spi/spi-cadence-quadspi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 6ddb2dfc0f00..620316ab70ef 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -40,6 +40,7 @@
 #define CQSPI_SUPPORT_EXTERNAL_DMA	BIT(2)
 #define CQSPI_NO_SUPPORT_WR_COMPLETION	BIT(3)
 #define CQSPI_SLOW_SRAM		BIT(4)
+#define CQSPI_NEEDS_APB_AHB_HAZARD_WAR	BIT(5)
 
 /* Capabilities */
 #define CQSPI_SUPPORTS_OCTAL		BIT(0)
@@ -90,6 +91,7 @@ struct cqspi_st {
 	u32			pd_dev_id;
 	bool			wr_completion;
 	bool			slow_sram;
+	bool			apb_ahb_hazard;
 };
 
 struct cqspi_driver_platdata {
@@ -1027,6 +1029,13 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
 	if (cqspi->wr_delay)
 		ndelay(cqspi->wr_delay);
 
+	/*
+	 * If a hazard exists between the APB and AHB interfaces, perform a
+	 * dummy readback from the controller to ensure synchronization.
+	 */
+	if (cqspi->apb_ahb_hazard)
+		readl(reg_base + CQSPI_REG_INDIRECTWR);
+
 	while (remaining > 0) {
 		size_t write_words, mod_bytes;
 
@@ -1754,6 +1763,8 @@ static int cqspi_probe(struct platform_device *pdev)
 			cqspi->wr_completion = false;
 		if (ddata->quirks & CQSPI_SLOW_SRAM)
 			cqspi->slow_sram = true;
+		if (ddata->quirks & CQSPI_NEEDS_APB_AHB_HAZARD_WAR)
+			cqspi->apb_ahb_hazard = true;
 
 		if (of_device_is_compatible(pdev->dev.of_node,
 					    "xlnx,versal-ospi-1.0"))
@@ -1885,6 +1896,10 @@ static const struct cqspi_driver_platdata jh7110_qspi = {
 	.quirks = CQSPI_DISABLE_DAC_MODE,
 };
 
+static const struct cqspi_driver_platdata pensando_cdns_qspi = {
+	.quirks = CQSPI_NEEDS_APB_AHB_HAZARD_WAR | CQSPI_DISABLE_DAC_MODE,
+};
+
 static const struct of_device_id cqspi_dt_ids[] = {
 	{
 		.compatible = "cdns,qspi-nor",
@@ -1914,6 +1929,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
 		.compatible = "starfive,jh7110-qspi",
 		.data = &jh7110_qspi,
 	},
+	{
+		.compatible = "amd,pensando-elba-qspi",
+		.data = &pensando_cdns_qspi,
+	},
 	{ /* end of table */ }
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (6 preceding siblings ...)
  2023-05-15 18:16 ` [PATCH v14 7/8] spi: cadence-quadspi: Add compatible for AMD Pensando Elba SoC Brad Larson
@ 2023-05-15 18:16 ` Brad Larson
  2023-05-15 21:05   ` Andy Shevchenko
                     ` (3 more replies)
  2023-05-16  7:14 ` [PATCH v14 0/8] Support AMD Pensando Elba SoC Krzysztof Kozlowski
  2023-05-17 14:43 ` (subset) " Mark Brown
  9 siblings, 4 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-15 18:16 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, blarson, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

The Pensando SoC controller is a SPI connected companion device
that is present in all Pensando SoC board designs.  The essential
board management registers are accessed on chip select 0 with
board mgmt IO support accessed using additional chip selects.

Signed-off-by: Brad Larson <blarson@amd.com>
---

v14 changes:
- Save 8 bytes of code size by swapping spi_device and reset_controller_dev
  in penctrl_device
- Code simplification and clarity from review inputs
- Set penctrl_spi_driver.driver.name to match compatible pensando-elba-ctrl
- Remove unused include in amd-pensando-ctrl.h
- Rebase to linux-next 6.4.0-rc1 class_create() API change
 
v13 changes:
- Update include list in pensando-ctrl.c
- Change variable spi_dev to spi throughout
- Removed unneeded variable initialization, simplification of
  error checks, remove extra castings, and use dev_err_probe()
- Sort the includes in amd-pensando-ctrl.h
- Updates to cleanup if there is an error in penctrl_spi_probe()

v12 changes:
- Fix gcc-12.1.0 warning:
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202303120925.SxLjwOd2-lkp@intel.com/

v11 changes:
- Fix the compatible to be specific 'amd,pensando-elba-ctrl'

v10 changes:
- Different driver implementation specific to this Pensando controller device.
- Moved to soc/amd directory under new name based on guidance.  This driver is
  of no use to any design other than all Pensando SoC based cards.
- Removed use of builtin_driver, can be built as a module.

v9 changes:
- Previously patch 14/17
- After the change to the device tree node and squashing
  reset-cells into the parent simplified this to not use
  any MFD API and move it to drivers/spi/pensando-sr.c.
- Change the naming to remove elba since this driver is common
  for all Pensando SoC designs .
- Default yes SPI_PENSANDO_SR for ARCH_PENSANDO

---
 drivers/soc/Kconfig                    |   1 +
 drivers/soc/Makefile                   |   1 +
 drivers/soc/amd/Kconfig                |  16 ++
 drivers/soc/amd/Makefile               |   2 +
 drivers/soc/amd/pensando-ctrl.c        | 368 +++++++++++++++++++++++++
 include/uapi/linux/amd-pensando-ctrl.h |  29 ++
 6 files changed, 417 insertions(+)
 create mode 100644 drivers/soc/amd/Kconfig
 create mode 100644 drivers/soc/amd/Makefile
 create mode 100644 drivers/soc/amd/pensando-ctrl.c
 create mode 100644 include/uapi/linux/amd-pensando-ctrl.h

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 4e176280113a..9e023f74e47c 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -2,6 +2,7 @@
 menu "SOC (System On Chip) specific Drivers"
 
 source "drivers/soc/actions/Kconfig"
+source "drivers/soc/amd/Kconfig"
 source "drivers/soc/amlogic/Kconfig"
 source "drivers/soc/apple/Kconfig"
 source "drivers/soc/aspeed/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 3b0f9fb3b5c8..8914530f2721 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
+obj-y				+= amd/
 obj-y				+= apple/
 obj-y				+= aspeed/
 obj-$(CONFIG_ARCH_AT91)		+= atmel/
diff --git a/drivers/soc/amd/Kconfig b/drivers/soc/amd/Kconfig
new file mode 100644
index 000000000000..011d5339d14e
--- /dev/null
+++ b/drivers/soc/amd/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+menu "AMD Pensando SoC drivers"
+
+config AMD_PENSANDO_CTRL
+	tristate "AMD Pensando SoC Controller"
+	depends on SPI_MASTER=y
+	depends on (ARCH_PENSANDO && OF) || COMPILE_TEST
+	default ARCH_PENSANDO
+	select REGMAP_SPI
+	select MFD_SYSCON
+	help
+	  Enables AMD Pensando SoC controller device support.  This is a SPI
+	  attached companion device in all Pensando SoC board designs which
+	  provides essential board control/status registers and management IO
+	  support.
+endmenu
diff --git a/drivers/soc/amd/Makefile b/drivers/soc/amd/Makefile
new file mode 100644
index 000000000000..a2de0424f68d
--- /dev/null
+++ b/drivers/soc/amd/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_AMD_PENSANDO_CTRL)	+= pensando-ctrl.o
diff --git a/drivers/soc/amd/pensando-ctrl.c b/drivers/soc/amd/pensando-ctrl.c
new file mode 100644
index 000000000000..a7ddd181dfe8
--- /dev/null
+++ b/drivers/soc/amd/pensando-ctrl.c
@@ -0,0 +1,368 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD Pensando SoC Controller
+ *
+ * Userspace interface and reset driver support for SPI connected Pensando SoC
+ * controller device.  This device is present in all Pensando SoC designs and
+ * contains board control/status registers and management IO support.
+ *
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/reset-controller.h>
+#include <linux/spi/spi.h>
+
+#include <linux/amd-pensando-ctrl.h>
+
+struct penctrl_device {
+	struct reset_controller_dev rcdev;
+	struct spi_device *spi;
+};
+
+static DEFINE_MUTEX(spi_lock);
+static dev_t penctrl_devt;
+static struct penctrl_device *penctrl;
+static struct class *penctrl_class;
+
+static long
+penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	void __user *in_arg = (void __user *)arg;
+	struct penctrl_device *penctrl;
+	u8 tx_buf[PENCTRL_MAX_MSG_LEN];
+	u8 rx_buf[PENCTRL_MAX_MSG_LEN];
+	struct spi_transfer t[2] = {};
+	struct penctrl_spi_xfer *msg;
+	struct spi_device *spi;
+	unsigned int num_msgs;
+	struct spi_message m;
+	u32 size;
+	int ret;
+
+	/* Check for a valid command */
+	if (_IOC_TYPE(cmd) != PENCTRL_IOC_MAGIC)
+		return -ENOTTY;
+
+	if (_IOC_NR(cmd) > PENCTRL_IOC_MAXNR)
+		return -ENOTTY;
+
+	if (((_IOC_DIR(cmd) & _IOC_READ)) && !access_ok(in_arg, _IOC_SIZE(cmd)))
+		return -EFAULT;
+
+	if (((_IOC_DIR(cmd) & _IOC_WRITE)) && !access_ok(in_arg, _IOC_SIZE(cmd)))
+		return -EFAULT;
+
+	/* Get a reference to the SPI device */
+	penctrl = filp->private_data;
+	if (!penctrl)
+		return -ESHUTDOWN;
+
+	spi = spi_dev_get(penctrl->spi);
+	if (!spi)
+		return -ESHUTDOWN;
+
+	/* Verify and prepare SPI message */
+	size = _IOC_SIZE(cmd);
+	num_msgs = size / sizeof(struct penctrl_spi_xfer);
+	if (num_msgs > 2 || size == 0 || size % sizeof(struct penctrl_spi_xfer)) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	msg = memdup_user((struct penctrl_spi_xfer *)arg, size);
+	if (IS_ERR(msg)) {
+		ret = PTR_ERR(msg);
+		goto out_unlock;
+	}
+	if (msg->len > PENCTRL_MAX_MSG_LEN) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	t[0].tx_buf = tx_buf;
+	t[0].len = msg->len;
+	if (copy_from_user(tx_buf, (void __user *)msg->tx_buf, msg->len)) {
+		ret = -EFAULT;
+		goto out_unlock;
+	}
+	if (num_msgs > 1) {
+		msg++;
+		if (msg->len > PENCTRL_MAX_MSG_LEN) {
+			ret = -EINVAL;
+			goto out_unlock;
+		}
+		t[1].rx_buf = rx_buf;
+		t[1].len = msg->len;
+	}
+	spi_message_init_with_transfers(&m, t, num_msgs);
+
+	/* Perform the transfer */
+	mutex_lock(&spi_lock);
+	ret = spi_sync(spi, &m);
+	mutex_unlock(&spi_lock);
+
+	if (ret || (num_msgs == 1))
+		goto out_unlock;
+
+	if (copy_to_user((void __user *)msg->rx_buf, rx_buf, msg->len))
+		ret = -EFAULT;
+
+out_unlock:
+	spi_dev_put(spi);
+	return ret;
+}
+
+static int penctrl_open(struct inode *inode, struct file *filp)
+{
+	struct spi_device *spi;
+	u8 current_cs;
+
+	filp->private_data = penctrl;
+	current_cs = iminor(inode);
+	spi = penctrl->spi;
+	spi->chip_select = current_cs;
+	spi->cs_gpiod = spi->controller->cs_gpiods[current_cs];
+	spi_setup(spi);
+	return stream_open(inode, filp);
+}
+
+static int penctrl_release(struct inode *inode, struct file *filp)
+{
+	filp->private_data = NULL;
+	return 0;
+}
+
+static const struct file_operations penctrl_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl = penctrl_ioctl,
+	.open		= penctrl_open,
+	.release	= penctrl_release,
+	.llseek		= no_llseek,
+};
+
+static int penctrl_regs_read(struct penctrl_device *penctrl, u32 reg, u32 *val)
+{
+	struct spi_device *spi = penctrl->spi;
+	struct spi_transfer t[2] = {};
+	struct spi_message m;
+	u8 txbuf[3];
+	u8 rxbuf[1];
+	int ret;
+
+	txbuf[0] = PENCTRL_SPI_CMD_REGRD;
+	txbuf[1] = reg;
+	txbuf[2] = 0;
+	t[0].tx_buf = txbuf;
+	t[0].len = sizeof(txbuf);
+
+	rxbuf[0] = 0;
+	t[1].rx_buf = rxbuf;
+	t[1].len = sizeof(rxbuf);
+
+	spi_message_init_with_transfers(&m, t, ARRAY_SIZE(t));
+	ret = spi_sync(spi, &m);
+	if (ret == 0)
+		*val = rxbuf[0];
+
+	return ret;
+}
+
+static int penctrl_regs_write(struct penctrl_device *penctrl, u32 reg, u32 val)
+{
+	struct spi_device *spi = penctrl->spi;
+	struct spi_transfer t = {};
+	struct spi_message m;
+	u8 txbuf[4];
+
+	txbuf[0] = PENCTRL_SPI_CMD_REGWR;
+	txbuf[1] = reg;
+	txbuf[2] = val;
+	txbuf[3] = 0;
+
+	t.tx_buf = txbuf;
+	t.len = sizeof(txbuf);
+	spi_message_init_with_transfers(&m, &t, 1);
+	return spi_sync(spi, &m);
+}
+
+static int penctrl_reset_assert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct penctrl_device *penctrl =
+		container_of(rcdev, struct penctrl_device, rcdev);
+	struct spi_device *spi = penctrl->spi;
+	unsigned int val;
+	int ret;
+
+	mutex_lock(&spi_lock);
+	spi->chip_select = 0;
+	spi->cs_gpiod = spi->controller->cs_gpiods[0];
+	spi_setup(spi);
+	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
+	if (ret) {
+		dev_err(&spi->dev, "error reading ctrl0 reg\n");
+		goto out_unlock;
+	}
+
+	val |= BIT(6);
+	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
+	if (ret)
+		dev_err(&spi->dev, "error writing ctrl0 reg\n");
+
+out_unlock:
+	mutex_unlock(&spi_lock);
+	return ret;
+}
+
+static int penctrl_reset_deassert(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	struct penctrl_device *penctrl =
+		container_of(rcdev, struct penctrl_device, rcdev);
+	struct spi_device *spi = penctrl->spi;
+	unsigned int val;
+	int ret;
+
+	mutex_lock(&spi_lock);
+	spi->chip_select = 0;
+	spi->cs_gpiod = spi->controller->cs_gpiods[0];
+	spi_setup(spi);
+	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
+	if (ret) {
+		dev_err(&spi->dev, "error reading ctrl0 reg\n");
+		goto out_unlock;
+	}
+
+	val &= ~BIT(6);
+	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
+	if (ret)
+		dev_err(&spi->dev, "error writing ctrl0 reg\n");
+
+out_unlock:
+	mutex_unlock(&spi_lock);
+	return ret;
+}
+
+static const struct reset_control_ops penctrl_reset_ops = {
+	.assert = penctrl_reset_assert,
+	.deassert = penctrl_reset_deassert,
+};
+
+static int penctrl_spi_probe(struct spi_device *spi)
+{
+	struct device *dev;
+	struct cdev *cdev;
+	u32 num_cs;
+	int ret;
+	u32 cs;
+
+	ret = device_property_read_u32(spi->dev.parent, "num-cs", &num_cs);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "number of chip-selects not defined\n");
+
+	ret = alloc_chrdev_region(&penctrl_devt, 0, num_cs, "penctrl");
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "failed to alloc chrdev region\n");
+
+	penctrl_class = class_create("penctrl");
+	if (IS_ERR(penctrl_class)) {
+		ret = dev_err_probe(&spi->dev, PTR_ERR(penctrl_class),
+				    "failed to create class\n");
+		goto unregister_chrdev;
+	}
+
+	cdev = cdev_alloc();
+	if (!cdev) {
+		ret = dev_err_probe(&spi->dev, -ENOMEM,
+				    "allocation of cdev failed\n");
+		goto destroy_class;
+	}
+	cdev->owner = THIS_MODULE;
+	cdev_init(cdev, &penctrl_fops);
+
+	ret = cdev_add(cdev, penctrl_devt, num_cs);
+	if (ret) {
+		ret = dev_err_probe(&spi->dev, ret,
+				    "register of cdev failed\n");
+		goto free_cdev;
+	}
+
+	/* Allocate driver data */
+	penctrl = kzalloc(sizeof(*penctrl), GFP_KERNEL);
+	if (!penctrl) {
+		ret = -ENOMEM;
+		goto free_cdev;
+	}
+	penctrl->spi = spi;
+	mutex_init(&spi_lock);
+
+	/* Create a device for each chip select */
+	for (cs = 0; cs < num_cs; cs++) {
+		dev = device_create(penctrl_class,
+				    &spi->dev,
+				    MKDEV(MAJOR(penctrl_devt), cs),
+				    penctrl,
+				    "penctrl0.%d",
+				    cs);
+		if (IS_ERR(dev)) {
+			ret = dev_err_probe(&spi->dev, PTR_ERR(dev),
+					    "error creating device\n");
+			goto destroy_device;
+		}
+		dev_dbg(&spi->dev, "created device major %u, minor %d\n",
+			MAJOR(penctrl_devt), cs);
+	}
+
+	/* Register reset controller */
+	penctrl->rcdev.dev = &spi->dev;
+	penctrl->rcdev.ops = &penctrl_reset_ops;
+	penctrl->rcdev.owner = THIS_MODULE;
+	penctrl->rcdev.of_node = spi->dev.of_node;
+	penctrl->rcdev.nr_resets = 1;
+
+	ret = reset_controller_register(&penctrl->rcdev);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "failed to register reset controller\n");
+	return 0;
+
+destroy_device:
+	for (cs = 0; cs < num_cs; cs++)
+		device_destroy(penctrl_class, MKDEV(MAJOR(penctrl_devt), cs));
+	kfree(penctrl);
+free_cdev:
+	cdev_del(cdev);
+destroy_class:
+	class_destroy(penctrl_class);
+unregister_chrdev:
+	unregister_chrdev(MAJOR(penctrl_devt), "penctrl");
+
+	return ret;
+}
+
+static const struct of_device_id penctrl_dt_match[] = {
+	{ .compatible = "amd,pensando-elba-ctrl" },
+	{ /* sentinel */ }
+};
+
+static struct spi_driver penctrl_spi_driver = {
+	.probe = penctrl_spi_probe,
+	.driver = {
+		.name = "pensando-elba-ctrl",
+		.of_match_table = penctrl_dt_match,
+	},
+};
+module_spi_driver(penctrl_spi_driver);
+
+MODULE_AUTHOR("Brad Larson <blarson@amd.com>");
+MODULE_DESCRIPTION("AMD Pensando SoC Controller via SPI");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/amd-pensando-ctrl.h b/include/uapi/linux/amd-pensando-ctrl.h
new file mode 100644
index 000000000000..e5f9f0dfe146
--- /dev/null
+++ b/include/uapi/linux/amd-pensando-ctrl.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Userspace interface for /dev/penctrl
+ *
+ * This file can be used by applications that need to communicate
+ * with the AMD Pensando SoC controller device via the ioctl interface.
+ */
+#ifndef _UAPI_LINUX_AMD_PENSANDO_CTRL_H
+#define _UAPI_LINUX_AMD_PENSANDO_CTRL_H
+
+#include <linux/types.h>
+
+#define PENCTRL_SPI_CMD_REGRD	0x0b
+#define PENCTRL_SPI_CMD_REGWR	0x02
+#define PENCTRL_IOC_MAGIC	'k'
+#define PENCTRL_IOC_MAXNR	0
+#define PENCTRL_MAX_MSG_LEN	16
+#define PENCTRL_MAX_REG		0xff
+#define PENCTRL_REG_CTRL0	0x10
+
+struct penctrl_spi_xfer {
+	__u64 tx_buf;
+	__u64 rx_buf;
+	__u32 len;
+	__u32 speed_hz;
+	__u64 compat;
+};
+
+#endif /* _UAPI_LINUX_AMD_PENSANDO_CTRL_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
@ 2023-05-15 21:05   ` Andy Shevchenko
  2023-05-23  2:12     ` Brad Larson
  2023-05-16  5:19   ` Mahapatra, Amit Kumar
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2023-05-15 21:05 UTC (permalink / raw)
  To: Brad Larson
  Cc: linux-arm-kernel, linux-kernel, linux-mmc, linux-spi,
	adrian.hunter, alcooperx, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

On Mon, May 15, 2023 at 9:18 PM Brad Larson <blarson@amd.com> wrote:
>
> The Pensando SoC controller is a SPI connected companion device
> that is present in all Pensando SoC board designs.  The essential
> board management registers are accessed on chip select 0 with
> board mgmt IO support accessed using additional chip selects.

...

> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>

> +#include <linux/of.h>

Unneeded inclusion.

> +#include <linux/reset-controller.h>
> +#include <linux/spi/spi.h>

...


> +       u8 tx_buf[PENCTRL_MAX_MSG_LEN];
> +       u8 rx_buf[PENCTRL_MAX_MSG_LEN];

Does it need to be DMA-capable?

...

> +       spi->chip_select = current_cs;
> +       spi->cs_gpiod = spi->controller->cs_gpiods[current_cs];

Nowadays these require API calls instead of direct assignments.

...

> +static int penctrl_release(struct inode *inode, struct file *filp)
> +{
> +       filp->private_data = NULL;
> +       return 0;
> +}

Is it possible to unload the module without releasing the device node?

...

> +       u8 txbuf[3];
> +       u8 rxbuf[1];

Same question about DMA.

...

> +       ret = spi_sync(spi, &m);

> +       if (ret == 0)
> +               *val = rxbuf[0];
> +
> +       return ret;

Can also be written in more usual way:

  if (ret)
    return ret;
  ...
  return 0;

...

> +       u8 txbuf[4];

DMA?

...

> +       spi->chip_select = 0;
> +       spi->cs_gpiod = spi->controller->cs_gpiods[0];

Setter APIs.

...

> +       spi->chip_select = 0;
> +       spi->cs_gpiod = spi->controller->cs_gpiods[0];

Ditto.


> +       ret = device_property_read_u32(spi->dev.parent, "num-cs", &num_cs);
> +       if (ret)
> +               return dev_err_probe(&spi->dev, ret,
> +                                    "number of chip-selects not defined\n");

Hmm... Shouldn't SPI core take care of this in a generic way? Yes, I
understand that you need the number for the allocation, but I would
expect something like spi_fw_get_num_cs() to exist (seems not?).

...

> +       penctrl->rcdev.of_node = spi->dev.of_node;

Use device_set_node(). It helps to modify the data types beneath.

--
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 29+ messages in thread

* RE: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
  2023-05-15 21:05   ` Andy Shevchenko
@ 2023-05-16  5:19   ` Mahapatra, Amit Kumar
  2023-05-16  7:36     ` Michal Simek
  2023-05-16  8:45   ` kernel test robot
  2023-05-16 11:03   ` Arnd Bergmann
  3 siblings, 1 reply; 29+ messages in thread
From: Mahapatra, Amit Kumar @ 2023-05-16  5:19 UTC (permalink / raw)
  To: Larson, Bradley, linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, Larson, Bradley, brendan.higgins,
	briannorris, catalin.marinas, conor+dt, davidgow, gsomlo, gerg,
	hal.feng, hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan, Suthikulpanit,
	Suravee, Lendacky, Thomas, tonyhuang.sunplus, ulf.hansson,
	vaishnav.a, walker.chen, will, zhuyinbo, devicetree,
	git (AMD-Xilinx)



> -----Original Message-----
> From: Brad Larson <blarson@amd.com>
> Sent: Monday, May 15, 2023 11:46 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-
> spi@vger.kernel.org; adrian.hunter@intel.com; alcooperx@gmail.com;
> andy.shevchenko@gmail.com; arnd@arndb.de; Larson, Bradley
> <Bradley.Larson@amd.com>; brendan.higgins@linux.dev;
> briannorris@chromium.org; catalin.marinas@arm.com;
> conor+dt@kernel.org; davidgow@google.com; gsomlo@gmail.com;
> gerg@linux-m68k.org; hal.feng@starfivetech.com; hasegawa-
> hitomi@fujitsu.com; j.neuschaefer@gmx.net; joel@jms.id.au;
> kernel@esmil.dk; krzk@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> lee@kernel.org; lee.jones@linaro.org; broonie@kernel.org;
> p.zabel@pengutronix.de; rdunlap@infradead.org; robh+dt@kernel.org;
> samuel@sholland.org; fancer.lancer@gmail.com;
> skhan@linuxfoundation.org; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@amd.com>; Lendacky, Thomas
> <Thomas.Lendacky@amd.com>; tonyhuang.sunplus@gmail.com;
> ulf.hansson@linaro.org; vaishnav.a@ti.com; walker.chen@starfivetech.com;
> will@kernel.org; zhuyinbo@loongson.cn; devicetree@vger.kernel.org
> Subject: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC
> Controller
> 
> The Pensando SoC controller is a SPI connected companion device that is
> present in all Pensando SoC board designs.  The essential board management
> registers are accessed on chip select 0 with board mgmt IO support accessed
> using additional chip selects.
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
> 
> v14 changes:
> - Save 8 bytes of code size by swapping spi_device and reset_controller_dev
>   in penctrl_device
> - Code simplification and clarity from review inputs
> - Set penctrl_spi_driver.driver.name to match compatible pensando-elba-ctrl
> - Remove unused include in amd-pensando-ctrl.h
> - Rebase to linux-next 6.4.0-rc1 class_create() API change
> 
> v13 changes:
> - Update include list in pensando-ctrl.c
> - Change variable spi_dev to spi throughout
> - Removed unneeded variable initialization, simplification of
>   error checks, remove extra castings, and use dev_err_probe()
> - Sort the includes in amd-pensando-ctrl.h
> - Updates to cleanup if there is an error in penctrl_spi_probe()
> 
> v12 changes:
> - Fix gcc-12.1.0 warning:
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/oe-kbuild-all/202303120925.SxLjwOd2-
> lkp@intel.com/
> 
> v11 changes:
> - Fix the compatible to be specific 'amd,pensando-elba-ctrl'
> 
> v10 changes:
> - Different driver implementation specific to this Pensando controller device.
> - Moved to soc/amd directory under new name based on guidance.  This
> driver is
>   of no use to any design other than all Pensando SoC based cards.
> - Removed use of builtin_driver, can be built as a module.
> 
> v9 changes:
> - Previously patch 14/17
> - After the change to the device tree node and squashing
>   reset-cells into the parent simplified this to not use
>   any MFD API and move it to drivers/spi/pensando-sr.c.
> - Change the naming to remove elba since this driver is common
>   for all Pensando SoC designs .
> - Default yes SPI_PENSANDO_SR for ARCH_PENSANDO
> 
> ---
>  drivers/soc/Kconfig                    |   1 +
>  drivers/soc/Makefile                   |   1 +
>  drivers/soc/amd/Kconfig                |  16 ++
>  drivers/soc/amd/Makefile               |   2 +
>  drivers/soc/amd/pensando-ctrl.c        | 368 +++++++++++++++++++++++++
>  include/uapi/linux/amd-pensando-ctrl.h |  29 ++
>  6 files changed, 417 insertions(+)
>  create mode 100644 drivers/soc/amd/Kconfig  create mode 100644
> drivers/soc/amd/Makefile  create mode 100644 drivers/soc/amd/pensando-
> ctrl.c  create mode 100644 include/uapi/linux/amd-pensando-ctrl.h
> 
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index
> 4e176280113a..9e023f74e47c 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -2,6 +2,7 @@
>  menu "SOC (System On Chip) specific Drivers"
> 
>  source "drivers/soc/actions/Kconfig"
> +source "drivers/soc/amd/Kconfig"
>  source "drivers/soc/amlogic/Kconfig"
>  source "drivers/soc/apple/Kconfig"
>  source "drivers/soc/aspeed/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index
> 3b0f9fb3b5c8..8914530f2721 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -4,6 +4,7 @@
>  #
> 
>  obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
> +obj-y				+= amd/
>  obj-y				+= apple/
>  obj-y				+= aspeed/
>  obj-$(CONFIG_ARCH_AT91)		+= atmel/
> diff --git a/drivers/soc/amd/Kconfig b/drivers/soc/amd/Kconfig new file
> mode 100644 index 000000000000..011d5339d14e
> --- /dev/null
> +++ b/drivers/soc/amd/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0-only menu "AMD Pensando SoC drivers"
> +
> +config AMD_PENSANDO_CTRL
> +	tristate "AMD Pensando SoC Controller"
> +	depends on SPI_MASTER=y
> +	depends on (ARCH_PENSANDO && OF) || COMPILE_TEST
> +	default ARCH_PENSANDO
> +	select REGMAP_SPI
> +	select MFD_SYSCON
> +	help
> +	  Enables AMD Pensando SoC controller device support.  This is a SPI
> +	  attached companion device in all Pensando SoC board designs which
> +	  provides essential board control/status registers and management
> IO
> +	  support.
> +endmenu
> diff --git a/drivers/soc/amd/Makefile b/drivers/soc/amd/Makefile new file
> mode 100644 index 000000000000..a2de0424f68d
> --- /dev/null
> +++ b/drivers/soc/amd/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_AMD_PENSANDO_CTRL)	+= pensando-ctrl.o
> diff --git a/drivers/soc/amd/pensando-ctrl.c b/drivers/soc/amd/pensando-
> ctrl.c new file mode 100644 index 000000000000..a7ddd181dfe8
> --- /dev/null
> +++ b/drivers/soc/amd/pensando-ctrl.c
> @@ -0,0 +1,368 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Pensando SoC Controller
> + *
> + * Userspace interface and reset driver support for SPI connected
> +Pensando SoC
> + * controller device.  This device is present in all Pensando SoC
> +designs and
> + * contains board control/status registers and management IO support.
> + *
> + * Copyright 2023 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/amd-pensando-ctrl.h>
> +
> +struct penctrl_device {
> +	struct reset_controller_dev rcdev;
> +	struct spi_device *spi;
> +};
> +
> +static DEFINE_MUTEX(spi_lock);
> +static dev_t penctrl_devt;
> +static struct penctrl_device *penctrl;
> +static struct class *penctrl_class;
> +
> +static long
> +penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) {
> +	void __user *in_arg = (void __user *)arg;
> +	struct penctrl_device *penctrl;
> +	u8 tx_buf[PENCTRL_MAX_MSG_LEN];
> +	u8 rx_buf[PENCTRL_MAX_MSG_LEN];
> +	struct spi_transfer t[2] = {};
> +	struct penctrl_spi_xfer *msg;
> +	struct spi_device *spi;
> +	unsigned int num_msgs;
> +	struct spi_message m;
> +	u32 size;
> +	int ret;
> +
> +	/* Check for a valid command */
> +	if (_IOC_TYPE(cmd) != PENCTRL_IOC_MAGIC)
> +		return -ENOTTY;
> +
> +	if (_IOC_NR(cmd) > PENCTRL_IOC_MAXNR)
> +		return -ENOTTY;
> +
> +	if (((_IOC_DIR(cmd) & _IOC_READ)) && !access_ok(in_arg,
> _IOC_SIZE(cmd)))
> +		return -EFAULT;
> +
> +	if (((_IOC_DIR(cmd) & _IOC_WRITE)) && !access_ok(in_arg,
> _IOC_SIZE(cmd)))
> +		return -EFAULT;
> +
> +	/* Get a reference to the SPI device */
> +	penctrl = filp->private_data;
> +	if (!penctrl)
> +		return -ESHUTDOWN;
> +
> +	spi = spi_dev_get(penctrl->spi);
> +	if (!spi)
> +		return -ESHUTDOWN;
> +
> +	/* Verify and prepare SPI message */
> +	size = _IOC_SIZE(cmd);
> +	num_msgs = size / sizeof(struct penctrl_spi_xfer);
> +	if (num_msgs > 2 || size == 0 || size % sizeof(struct
> penctrl_spi_xfer)) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +	msg = memdup_user((struct penctrl_spi_xfer *)arg, size);
> +	if (IS_ERR(msg)) {
> +		ret = PTR_ERR(msg);
> +		goto out_unlock;
> +	}
> +	if (msg->len > PENCTRL_MAX_MSG_LEN) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	t[0].tx_buf = tx_buf;
> +	t[0].len = msg->len;
> +	if (copy_from_user(tx_buf, (void __user *)msg->tx_buf, msg->len))
> {
> +		ret = -EFAULT;
> +		goto out_unlock;
> +	}
> +	if (num_msgs > 1) {
> +		msg++;
> +		if (msg->len > PENCTRL_MAX_MSG_LEN) {
> +			ret = -EINVAL;
> +			goto out_unlock;
> +		}
> +		t[1].rx_buf = rx_buf;
> +		t[1].len = msg->len;
> +	}
> +	spi_message_init_with_transfers(&m, t, num_msgs);
> +
> +	/* Perform the transfer */
> +	mutex_lock(&spi_lock);
> +	ret = spi_sync(spi, &m);
> +	mutex_unlock(&spi_lock);
> +
> +	if (ret || (num_msgs == 1))
> +		goto out_unlock;
> +
> +	if (copy_to_user((void __user *)msg->rx_buf, rx_buf, msg->len))
> +		ret = -EFAULT;
> +
> +out_unlock:
> +	spi_dev_put(spi);
> +	return ret;
> +}
> +
> +static int penctrl_open(struct inode *inode, struct file *filp) {
> +	struct spi_device *spi;
> +	u8 current_cs;
> +
> +	filp->private_data = penctrl;
> +	current_cs = iminor(inode);
> +	spi = penctrl->spi;
> +	spi->chip_select = current_cs;

New set/get APIs for accessing spi->chip_select were introduced by 
https://github.com/torvalds/linux/commit/303feb3cc06ac0665d0ee9c1414941200e60e8a3
please use these APIs instead of accessing spi->chip_select dricetly. 

> +	spi->cs_gpiod = spi->controller->cs_gpiods[current_cs];
> +	spi_setup(spi);
> +	return stream_open(inode, filp);
> +}
> +
> +static int penctrl_release(struct inode *inode, struct file *filp) {
> +	filp->private_data = NULL;
> +	return 0;
> +}
> +
> +static const struct file_operations penctrl_fops = {
> +	.owner		= THIS_MODULE,
> +	.unlocked_ioctl = penctrl_ioctl,
> +	.open		= penctrl_open,
> +	.release	= penctrl_release,
> +	.llseek		= no_llseek,
> +};
> +
> +static int penctrl_regs_read(struct penctrl_device *penctrl, u32 reg,
> +u32 *val) {
> +	struct spi_device *spi = penctrl->spi;
> +	struct spi_transfer t[2] = {};
> +	struct spi_message m;
> +	u8 txbuf[3];
> +	u8 rxbuf[1];
> +	int ret;
> +
> +	txbuf[0] = PENCTRL_SPI_CMD_REGRD;
> +	txbuf[1] = reg;
> +	txbuf[2] = 0;
> +	t[0].tx_buf = txbuf;
> +	t[0].len = sizeof(txbuf);
> +
> +	rxbuf[0] = 0;
> +	t[1].rx_buf = rxbuf;
> +	t[1].len = sizeof(rxbuf);
> +
> +	spi_message_init_with_transfers(&m, t, ARRAY_SIZE(t));
> +	ret = spi_sync(spi, &m);
> +	if (ret == 0)
> +		*val = rxbuf[0];
> +
> +	return ret;
> +}
> +
> +static int penctrl_regs_write(struct penctrl_device *penctrl, u32 reg,
> +u32 val) {
> +	struct spi_device *spi = penctrl->spi;
> +	struct spi_transfer t = {};
> +	struct spi_message m;
> +	u8 txbuf[4];
> +
> +	txbuf[0] = PENCTRL_SPI_CMD_REGWR;
> +	txbuf[1] = reg;
> +	txbuf[2] = val;
> +	txbuf[3] = 0;
> +
> +	t.tx_buf = txbuf;
> +	t.len = sizeof(txbuf);
> +	spi_message_init_with_transfers(&m, &t, 1);
> +	return spi_sync(spi, &m);
> +}
> +
> +static int penctrl_reset_assert(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct penctrl_device *penctrl =
> +		container_of(rcdev, struct penctrl_device, rcdev);
> +	struct spi_device *spi = penctrl->spi;
> +	unsigned int val;
> +	int ret;
> +
> +	mutex_lock(&spi_lock);
> +	spi->chip_select = 0;

Same here.

> +	spi->cs_gpiod = spi->controller->cs_gpiods[0];
> +	spi_setup(spi);
> +	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "error reading ctrl0 reg\n");
> +		goto out_unlock;
> +	}
> +
> +	val |= BIT(6);
> +	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
> +	if (ret)
> +		dev_err(&spi->dev, "error writing ctrl0 reg\n");
> +
> +out_unlock:
> +	mutex_unlock(&spi_lock);
> +	return ret;
> +}
> +
> +static int penctrl_reset_deassert(struct reset_controller_dev *rcdev,
> +				  unsigned long id)
> +{
> +	struct penctrl_device *penctrl =
> +		container_of(rcdev, struct penctrl_device, rcdev);
> +	struct spi_device *spi = penctrl->spi;
> +	unsigned int val;
> +	int ret;
> +
> +	mutex_lock(&spi_lock);
> +	spi->chip_select = 0;

Same here.

Regards,
Amit

> +	spi->cs_gpiod = spi->controller->cs_gpiods[0];
> +	spi_setup(spi);
> +	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "error reading ctrl0 reg\n");
> +		goto out_unlock;
> +	}
> +
> +	val &= ~BIT(6);
> +	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
> +	if (ret)
> +		dev_err(&spi->dev, "error writing ctrl0 reg\n");
> +
> +out_unlock:
> +	mutex_unlock(&spi_lock);
> +	return ret;
> +}
> +
> +static const struct reset_control_ops penctrl_reset_ops = {
> +	.assert = penctrl_reset_assert,
> +	.deassert = penctrl_reset_deassert,
> +};
> +
> +static int penctrl_spi_probe(struct spi_device *spi) {
> +	struct device *dev;
> +	struct cdev *cdev;
> +	u32 num_cs;
> +	int ret;
> +	u32 cs;
> +
> +	ret = device_property_read_u32(spi->dev.parent, "num-cs",
> &num_cs);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "number of chip-selects not defined\n");
> +
> +	ret = alloc_chrdev_region(&penctrl_devt, 0, num_cs, "penctrl");
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "failed to alloc chrdev region\n");
> +
> +	penctrl_class = class_create("penctrl");
> +	if (IS_ERR(penctrl_class)) {
> +		ret = dev_err_probe(&spi->dev, PTR_ERR(penctrl_class),
> +				    "failed to create class\n");
> +		goto unregister_chrdev;
> +	}
> +
> +	cdev = cdev_alloc();
> +	if (!cdev) {
> +		ret = dev_err_probe(&spi->dev, -ENOMEM,
> +				    "allocation of cdev failed\n");
> +		goto destroy_class;
> +	}
> +	cdev->owner = THIS_MODULE;
> +	cdev_init(cdev, &penctrl_fops);
> +
> +	ret = cdev_add(cdev, penctrl_devt, num_cs);
> +	if (ret) {
> +		ret = dev_err_probe(&spi->dev, ret,
> +				    "register of cdev failed\n");
> +		goto free_cdev;
> +	}
> +
> +	/* Allocate driver data */
> +	penctrl = kzalloc(sizeof(*penctrl), GFP_KERNEL);
> +	if (!penctrl) {
> +		ret = -ENOMEM;
> +		goto free_cdev;
> +	}
> +	penctrl->spi = spi;
> +	mutex_init(&spi_lock);
> +
> +	/* Create a device for each chip select */
> +	for (cs = 0; cs < num_cs; cs++) {
> +		dev = device_create(penctrl_class,
> +				    &spi->dev,
> +				    MKDEV(MAJOR(penctrl_devt), cs),
> +				    penctrl,
> +				    "penctrl0.%d",
> +				    cs);
> +		if (IS_ERR(dev)) {
> +			ret = dev_err_probe(&spi->dev, PTR_ERR(dev),
> +					    "error creating device\n");
> +			goto destroy_device;
> +		}
> +		dev_dbg(&spi->dev, "created device major %u, minor
> %d\n",
> +			MAJOR(penctrl_devt), cs);
> +	}
> +
> +	/* Register reset controller */
> +	penctrl->rcdev.dev = &spi->dev;
> +	penctrl->rcdev.ops = &penctrl_reset_ops;
> +	penctrl->rcdev.owner = THIS_MODULE;
> +	penctrl->rcdev.of_node = spi->dev.of_node;
> +	penctrl->rcdev.nr_resets = 1;
> +
> +	ret = reset_controller_register(&penctrl->rcdev);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "failed to register reset controller\n");
> +	return 0;
> +
> +destroy_device:
> +	for (cs = 0; cs < num_cs; cs++)
> +		device_destroy(penctrl_class,
> MKDEV(MAJOR(penctrl_devt), cs));
> +	kfree(penctrl);
> +free_cdev:
> +	cdev_del(cdev);
> +destroy_class:
> +	class_destroy(penctrl_class);
> +unregister_chrdev:
> +	unregister_chrdev(MAJOR(penctrl_devt), "penctrl");
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id penctrl_dt_match[] = {
> +	{ .compatible = "amd,pensando-elba-ctrl" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct spi_driver penctrl_spi_driver = {
> +	.probe = penctrl_spi_probe,
> +	.driver = {
> +		.name = "pensando-elba-ctrl",
> +		.of_match_table = penctrl_dt_match,
> +	},
> +};
> +module_spi_driver(penctrl_spi_driver);
> +
> +MODULE_AUTHOR("Brad Larson <blarson@amd.com>");
> MODULE_DESCRIPTION("AMD
> +Pensando SoC Controller via SPI"); MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/amd-pensando-ctrl.h
> b/include/uapi/linux/amd-pensando-ctrl.h
> new file mode 100644
> index 000000000000..e5f9f0dfe146
> --- /dev/null
> +++ b/include/uapi/linux/amd-pensando-ctrl.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Userspace interface for /dev/penctrl
> + *
> + * This file can be used by applications that need to communicate
> + * with the AMD Pensando SoC controller device via the ioctl interface.
> + */
> +#ifndef _UAPI_LINUX_AMD_PENSANDO_CTRL_H #define
> +_UAPI_LINUX_AMD_PENSANDO_CTRL_H
> +
> +#include <linux/types.h>
> +
> +#define PENCTRL_SPI_CMD_REGRD	0x0b
> +#define PENCTRL_SPI_CMD_REGWR	0x02
> +#define PENCTRL_IOC_MAGIC	'k'
> +#define PENCTRL_IOC_MAXNR	0
> +#define PENCTRL_MAX_MSG_LEN	16
> +#define PENCTRL_MAX_REG		0xff
> +#define PENCTRL_REG_CTRL0	0x10
> +
> +struct penctrl_spi_xfer {
> +	__u64 tx_buf;
> +	__u64 rx_buf;
> +	__u32 len;
> +	__u32 speed_hz;
> +	__u64 compat;
> +};
> +
> +#endif /* _UAPI_LINUX_AMD_PENSANDO_CTRL_H */
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 0/8] Support AMD Pensando Elba SoC
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (7 preceding siblings ...)
  2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
@ 2023-05-16  7:14 ` Krzysztof Kozlowski
  2023-05-17 14:43 ` (subset) " Mark Brown
  9 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-16  7:14 UTC (permalink / raw)
  To: Brad Larson, linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

On 15/05/2023 20:15, Brad Larson wrote:
> This series enables support for AMD 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
> 
> == V14 changes ==
> Updated email list using get_maintainer.pl

If you split the patches per subsystem (e.g. SPI), you would get more of
them merged already.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-15 18:16 ` [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support Brad Larson
@ 2023-05-16  7:17   ` Krzysztof Kozlowski
  2023-05-16  7:54   ` Michal Simek
  1 sibling, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-16  7:17 UTC (permalink / raw)
  To: Brad Larson, linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree

On 15/05/2023 20:16, Brad Larson wrote:
> Add AMD Pensando common and Elba SoC specific device nodes
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-16  5:19   ` Mahapatra, Amit Kumar
@ 2023-05-16  7:36     ` Michal Simek
  2023-05-17 11:14       ` Geert Uytterhoeven
  0 siblings, 1 reply; 29+ messages in thread
From: Michal Simek @ 2023-05-16  7:36 UTC (permalink / raw)
  To: Mahapatra, Amit Kumar, Larson, Bradley, linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan, Suthikulpanit,
	Suravee, Lendacky, Thomas, tonyhuang.sunplus, ulf.hansson,
	vaishnav.a, walker.chen, will, zhuyinbo, devicetree,
	git (AMD-Xilinx)



On 5/16/23 07:19, Mahapatra, Amit Kumar wrote:
> 
> 
>> -----Original Message-----
>> From: Brad Larson <blarson@amd.com>
>> Sent: Monday, May 15, 2023 11:46 PM
>> To: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-
>> spi@vger.kernel.org; adrian.hunter@intel.com; alcooperx@gmail.com;
>> andy.shevchenko@gmail.com; arnd@arndb.de; Larson, Bradley
>> <Bradley.Larson@amd.com>; brendan.higgins@linux.dev;
>> briannorris@chromium.org; catalin.marinas@arm.com;
>> conor+dt@kernel.org; davidgow@google.com; gsomlo@gmail.com;
>> gerg@linux-m68k.org; hal.feng@starfivetech.com; hasegawa-
>> hitomi@fujitsu.com; j.neuschaefer@gmx.net; joel@jms.id.au;
>> kernel@esmil.dk; krzk@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> lee@kernel.org; lee.jones@linaro.org; broonie@kernel.org;
>> p.zabel@pengutronix.de; rdunlap@infradead.org; robh+dt@kernel.org;
>> samuel@sholland.org; fancer.lancer@gmail.com;
>> skhan@linuxfoundation.org; Suthikulpanit, Suravee
>> <Suravee.Suthikulpanit@amd.com>; Lendacky, Thomas
>> <Thomas.Lendacky@amd.com>; tonyhuang.sunplus@gmail.com;
>> ulf.hansson@linaro.org; vaishnav.a@ti.com; walker.chen@starfivetech.com;
>> will@kernel.org; zhuyinbo@loongson.cn; devicetree@vger.kernel.org
>> Subject: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC
>> Controller
>>
>> The Pensando SoC controller is a SPI connected companion device that is
>> present in all Pensando SoC board designs.  The essential board management
>> registers are accessed on chip select 0 with board mgmt IO support accessed
>> using additional chip selects.
>>
>> Signed-off-by: Brad Larson <blarson@amd.com>
>> ---
>>
>> v14 changes:
>> - Save 8 bytes of code size by swapping spi_device and reset_controller_dev
>>    in penctrl_device
>> - Code simplification and clarity from review inputs
>> - Set penctrl_spi_driver.driver.name to match compatible pensando-elba-ctrl
>> - Remove unused include in amd-pensando-ctrl.h
>> - Rebase to linux-next 6.4.0-rc1 class_create() API change
>>
>> v13 changes:
>> - Update include list in pensando-ctrl.c
>> - Change variable spi_dev to spi throughout
>> - Removed unneeded variable initialization, simplification of
>>    error checks, remove extra castings, and use dev_err_probe()
>> - Sort the includes in amd-pensando-ctrl.h
>> - Updates to cleanup if there is an error in penctrl_spi_probe()
>>
>> v12 changes:
>> - Fix gcc-12.1.0 warning:
>> Reported-by: kernel test robot <lkp@intel.com>
>> Link: https://lore.kernel.org/oe-kbuild-all/202303120925.SxLjwOd2-
>> lkp@intel.com/
>>
>> v11 changes:
>> - Fix the compatible to be specific 'amd,pensando-elba-ctrl'
>>
>> v10 changes:
>> - Different driver implementation specific to this Pensando controller device.
>> - Moved to soc/amd directory under new name based on guidance.  This
>> driver is
>>    of no use to any design other than all Pensando SoC based cards.
>> - Removed use of builtin_driver, can be built as a module.
>>
>> v9 changes:
>> - Previously patch 14/17
>> - After the change to the device tree node and squashing
>>    reset-cells into the parent simplified this to not use
>>    any MFD API and move it to drivers/spi/pensando-sr.c.
>> - Change the naming to remove elba since this driver is common
>>    for all Pensando SoC designs .
>> - Default yes SPI_PENSANDO_SR for ARCH_PENSANDO
>>
>> ---
>>   drivers/soc/Kconfig                    |   1 +
>>   drivers/soc/Makefile                   |   1 +
>>   drivers/soc/amd/Kconfig                |  16 ++
>>   drivers/soc/amd/Makefile               |   2 +
>>   drivers/soc/amd/pensando-ctrl.c        | 368 +++++++++++++++++++++++++
>>   include/uapi/linux/amd-pensando-ctrl.h |  29 ++
>>   6 files changed, 417 insertions(+)
>>   create mode 100644 drivers/soc/amd/Kconfig  create mode 100644
>> drivers/soc/amd/Makefile  create mode 100644 drivers/soc/amd/pensando-
>> ctrl.c  create mode 100644 include/uapi/linux/amd-pensando-ctrl.h
>>
>> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index
>> 4e176280113a..9e023f74e47c 100644
>> --- a/drivers/soc/Kconfig
>> +++ b/drivers/soc/Kconfig
>> @@ -2,6 +2,7 @@
>>   menu "SOC (System On Chip) specific Drivers"
>>
>>   source "drivers/soc/actions/Kconfig"
>> +source "drivers/soc/amd/Kconfig"
>>   source "drivers/soc/amlogic/Kconfig"
>>   source "drivers/soc/apple/Kconfig"
>>   source "drivers/soc/aspeed/Kconfig"
>> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index
>> 3b0f9fb3b5c8..8914530f2721 100644
>> --- a/drivers/soc/Makefile
>> +++ b/drivers/soc/Makefile
>> @@ -4,6 +4,7 @@
>>   #
>>
>>   obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
>> +obj-y				+= amd/
>>   obj-y				+= apple/
>>   obj-y				+= aspeed/
>>   obj-$(CONFIG_ARCH_AT91)		+= atmel/
>> diff --git a/drivers/soc/amd/Kconfig b/drivers/soc/amd/Kconfig new file
>> mode 100644 index 000000000000..011d5339d14e
>> --- /dev/null
>> +++ b/drivers/soc/amd/Kconfig
>> @@ -0,0 +1,16 @@
>> +# SPDX-License-Identifier: GPL-2.0-only menu "AMD Pensando SoC drivers"
>> +
>> +config AMD_PENSANDO_CTRL
>> +	tristate "AMD Pensando SoC Controller"
>> +	depends on SPI_MASTER=y
>> +	depends on (ARCH_PENSANDO && OF) || COMPILE_TEST
>> +	default ARCH_PENSANDO
>> +	select REGMAP_SPI
>> +	select MFD_SYSCON
>> +	help
>> +	  Enables AMD Pensando SoC controller device support.  This is a SPI
>> +	  attached companion device in all Pensando SoC board designs which
>> +	  provides essential board control/status registers and management
>> IO
>> +	  support.
>> +endmenu
>> diff --git a/drivers/soc/amd/Makefile b/drivers/soc/amd/Makefile new file
>> mode 100644 index 000000000000..a2de0424f68d
>> --- /dev/null
>> +++ b/drivers/soc/amd/Makefile
>> @@ -0,0 +1,2 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +obj-$(CONFIG_AMD_PENSANDO_CTRL)	+= pensando-ctrl.o
>> diff --git a/drivers/soc/amd/pensando-ctrl.c b/drivers/soc/amd/pensando-
>> ctrl.c new file mode 100644 index 000000000000..a7ddd181dfe8
>> --- /dev/null
>> +++ b/drivers/soc/amd/pensando-ctrl.c
>> @@ -0,0 +1,368 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * AMD Pensando SoC Controller
>> + *
>> + * Userspace interface and reset driver support for SPI connected
>> +Pensando SoC
>> + * controller device.  This device is present in all Pensando SoC
>> +designs and
>> + * contains board control/status registers and management IO support.
>> + *
>> + * Copyright 2023 Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include <linux/cdev.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include <linux/amd-pensando-ctrl.h>
>> +
>> +struct penctrl_device {
>> +	struct reset_controller_dev rcdev;
>> +	struct spi_device *spi;
>> +};
>> +
>> +static DEFINE_MUTEX(spi_lock);
>> +static dev_t penctrl_devt;
>> +static struct penctrl_device *penctrl;
>> +static struct class *penctrl_class;
>> +
>> +static long
>> +penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) {
>> +	void __user *in_arg = (void __user *)arg;
>> +	struct penctrl_device *penctrl;
>> +	u8 tx_buf[PENCTRL_MAX_MSG_LEN];
>> +	u8 rx_buf[PENCTRL_MAX_MSG_LEN];
>> +	struct spi_transfer t[2] = {};
>> +	struct penctrl_spi_xfer *msg;
>> +	struct spi_device *spi;
>> +	unsigned int num_msgs;
>> +	struct spi_message m;
>> +	u32 size;
>> +	int ret;
>> +
>> +	/* Check for a valid command */
>> +	if (_IOC_TYPE(cmd) != PENCTRL_IOC_MAGIC)
>> +		return -ENOTTY;
>> +
>> +	if (_IOC_NR(cmd) > PENCTRL_IOC_MAXNR)
>> +		return -ENOTTY;
>> +
>> +	if (((_IOC_DIR(cmd) & _IOC_READ)) && !access_ok(in_arg,
>> _IOC_SIZE(cmd)))
>> +		return -EFAULT;
>> +
>> +	if (((_IOC_DIR(cmd) & _IOC_WRITE)) && !access_ok(in_arg,
>> _IOC_SIZE(cmd)))
>> +		return -EFAULT;
>> +
>> +	/* Get a reference to the SPI device */
>> +	penctrl = filp->private_data;
>> +	if (!penctrl)
>> +		return -ESHUTDOWN;
>> +
>> +	spi = spi_dev_get(penctrl->spi);
>> +	if (!spi)
>> +		return -ESHUTDOWN;
>> +
>> +	/* Verify and prepare SPI message */
>> +	size = _IOC_SIZE(cmd);
>> +	num_msgs = size / sizeof(struct penctrl_spi_xfer);
>> +	if (num_msgs > 2 || size == 0 || size % sizeof(struct
>> penctrl_spi_xfer)) {
>> +		ret = -EINVAL;
>> +		goto out_unlock;
>> +	}
>> +	msg = memdup_user((struct penctrl_spi_xfer *)arg, size);
>> +	if (IS_ERR(msg)) {
>> +		ret = PTR_ERR(msg);
>> +		goto out_unlock;
>> +	}
>> +	if (msg->len > PENCTRL_MAX_MSG_LEN) {
>> +		ret = -EINVAL;
>> +		goto out_unlock;
>> +	}
>> +
>> +	t[0].tx_buf = tx_buf;
>> +	t[0].len = msg->len;
>> +	if (copy_from_user(tx_buf, (void __user *)msg->tx_buf, msg->len))
>> {
>> +		ret = -EFAULT;
>> +		goto out_unlock;
>> +	}
>> +	if (num_msgs > 1) {
>> +		msg++;
>> +		if (msg->len > PENCTRL_MAX_MSG_LEN) {
>> +			ret = -EINVAL;
>> +			goto out_unlock;
>> +		}
>> +		t[1].rx_buf = rx_buf;
>> +		t[1].len = msg->len;
>> +	}
>> +	spi_message_init_with_transfers(&m, t, num_msgs);
>> +
>> +	/* Perform the transfer */
>> +	mutex_lock(&spi_lock);
>> +	ret = spi_sync(spi, &m);
>> +	mutex_unlock(&spi_lock);
>> +
>> +	if (ret || (num_msgs == 1))
>> +		goto out_unlock;
>> +
>> +	if (copy_to_user((void __user *)msg->rx_buf, rx_buf, msg->len))
>> +		ret = -EFAULT;
>> +
>> +out_unlock:
>> +	spi_dev_put(spi);
>> +	return ret;
>> +}
>> +
>> +static int penctrl_open(struct inode *inode, struct file *filp) {
>> +	struct spi_device *spi;
>> +	u8 current_cs;
>> +
>> +	filp->private_data = penctrl;
>> +	current_cs = iminor(inode);
>> +	spi = penctrl->spi;
>> +	spi->chip_select = current_cs;
> 
> New set/get APIs for accessing spi->chip_select were introduced by
> https://github.com/torvalds/linux/commit/303feb3cc06ac0665d0ee9c1414941200e60e8a3
> please use these APIs instead of accessing spi->chip_select dricetly.
> 
>> +	spi->cs_gpiod = spi->controller->cs_gpiods[current_cs];
>> +	spi_setup(spi);
>> +	return stream_open(inode, filp);
>> +}
>> +
>> +static int penctrl_release(struct inode *inode, struct file *filp) {
>> +	filp->private_data = NULL;
>> +	return 0;
>> +}
>> +
>> +static const struct file_operations penctrl_fops = {
>> +	.owner		= THIS_MODULE,
>> +	.unlocked_ioctl = penctrl_ioctl,
>> +	.open		= penctrl_open,
>> +	.release	= penctrl_release,
>> +	.llseek		= no_llseek,
>> +};
>> +
>> +static int penctrl_regs_read(struct penctrl_device *penctrl, u32 reg,
>> +u32 *val) {
>> +	struct spi_device *spi = penctrl->spi;
>> +	struct spi_transfer t[2] = {};
>> +	struct spi_message m;
>> +	u8 txbuf[3];
>> +	u8 rxbuf[1];
>> +	int ret;
>> +
>> +	txbuf[0] = PENCTRL_SPI_CMD_REGRD;
>> +	txbuf[1] = reg;
>> +	txbuf[2] = 0;
>> +	t[0].tx_buf = txbuf;
>> +	t[0].len = sizeof(txbuf);
>> +
>> +	rxbuf[0] = 0;
>> +	t[1].rx_buf = rxbuf;
>> +	t[1].len = sizeof(rxbuf);
>> +
>> +	spi_message_init_with_transfers(&m, t, ARRAY_SIZE(t));
>> +	ret = spi_sync(spi, &m);
>> +	if (ret == 0)
>> +		*val = rxbuf[0];
>> +
>> +	return ret;
>> +}
>> +
>> +static int penctrl_regs_write(struct penctrl_device *penctrl, u32 reg,
>> +u32 val) {
>> +	struct spi_device *spi = penctrl->spi;
>> +	struct spi_transfer t = {};
>> +	struct spi_message m;
>> +	u8 txbuf[4];
>> +
>> +	txbuf[0] = PENCTRL_SPI_CMD_REGWR;
>> +	txbuf[1] = reg;
>> +	txbuf[2] = val;
>> +	txbuf[3] = 0;
>> +
>> +	t.tx_buf = txbuf;
>> +	t.len = sizeof(txbuf);
>> +	spi_message_init_with_transfers(&m, &t, 1);
>> +	return spi_sync(spi, &m);
>> +}
>> +
>> +static int penctrl_reset_assert(struct reset_controller_dev *rcdev,
>> +				unsigned long id)
>> +{
>> +	struct penctrl_device *penctrl =
>> +		container_of(rcdev, struct penctrl_device, rcdev);
>> +	struct spi_device *spi = penctrl->spi;
>> +	unsigned int val;
>> +	int ret;
>> +
>> +	mutex_lock(&spi_lock);
>> +	spi->chip_select = 0;
> 
> Same here.
> 
>> +	spi->cs_gpiod = spi->controller->cs_gpiods[0];
>> +	spi_setup(spi);
>> +	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "error reading ctrl0 reg\n");
>> +		goto out_unlock;
>> +	}
>> +
>> +	val |= BIT(6);
>> +	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
>> +	if (ret)
>> +		dev_err(&spi->dev, "error writing ctrl0 reg\n");
>> +
>> +out_unlock:
>> +	mutex_unlock(&spi_lock);
>> +	return ret;
>> +}
>> +
>> +static int penctrl_reset_deassert(struct reset_controller_dev *rcdev,
>> +				  unsigned long id)
>> +{
>> +	struct penctrl_device *penctrl =
>> +		container_of(rcdev, struct penctrl_device, rcdev);
>> +	struct spi_device *spi = penctrl->spi;
>> +	unsigned int val;
>> +	int ret;
>> +
>> +	mutex_lock(&spi_lock);
>> +	spi->chip_select = 0;
> 
> Same here.
> 
> Regards,
> Amit
> 
>> +	spi->cs_gpiod = spi->controller->cs_gpiods[0];
>> +	spi_setup(spi);
>> +	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "error reading ctrl0 reg\n");
>> +		goto out_unlock;
>> +	}
>> +
>> +	val &= ~BIT(6);
>> +	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
>> +	if (ret)
>> +		dev_err(&spi->dev, "error writing ctrl0 reg\n");
>> +
>> +out_unlock:
>> +	mutex_unlock(&spi_lock);
>> +	return ret;
>> +}
>> +
>> +static const struct reset_control_ops penctrl_reset_ops = {
>> +	.assert = penctrl_reset_assert,
>> +	.deassert = penctrl_reset_deassert,
>> +};
>> +
>> +static int penctrl_spi_probe(struct spi_device *spi) {
>> +	struct device *dev;
>> +	struct cdev *cdev;
>> +	u32 num_cs;
>> +	int ret;
>> +	u32 cs;
>> +
>> +	ret = device_property_read_u32(spi->dev.parent, "num-cs",
>> &num_cs);
>> +	if (ret)
>> +		return dev_err_probe(&spi->dev, ret,
>> +				     "number of chip-selects not defined\n");
>> +
>> +	ret = alloc_chrdev_region(&penctrl_devt, 0, num_cs, "penctrl");
>> +	if (ret)
>> +		return dev_err_probe(&spi->dev, ret,
>> +				     "failed to alloc chrdev region\n");
>> +
>> +	penctrl_class = class_create("penctrl");
>> +	if (IS_ERR(penctrl_class)) {
>> +		ret = dev_err_probe(&spi->dev, PTR_ERR(penctrl_class),
>> +				    "failed to create class\n");
>> +		goto unregister_chrdev;
>> +	}
>> +
>> +	cdev = cdev_alloc();
>> +	if (!cdev) {
>> +		ret = dev_err_probe(&spi->dev, -ENOMEM,
>> +				    "allocation of cdev failed\n");
>> +		goto destroy_class;
>> +	}
>> +	cdev->owner = THIS_MODULE;
>> +	cdev_init(cdev, &penctrl_fops);
>> +
>> +	ret = cdev_add(cdev, penctrl_devt, num_cs);
>> +	if (ret) {
>> +		ret = dev_err_probe(&spi->dev, ret,
>> +				    "register of cdev failed\n");
>> +		goto free_cdev;
>> +	}
>> +
>> +	/* Allocate driver data */
>> +	penctrl = kzalloc(sizeof(*penctrl), GFP_KERNEL);
>> +	if (!penctrl) {
>> +		ret = -ENOMEM;
>> +		goto free_cdev;
>> +	}
>> +	penctrl->spi = spi;
>> +	mutex_init(&spi_lock);
>> +
>> +	/* Create a device for each chip select */
>> +	for (cs = 0; cs < num_cs; cs++) {
>> +		dev = device_create(penctrl_class,
>> +				    &spi->dev,
>> +				    MKDEV(MAJOR(penctrl_devt), cs),
>> +				    penctrl,
>> +				    "penctrl0.%d",
>> +				    cs);
>> +		if (IS_ERR(dev)) {
>> +			ret = dev_err_probe(&spi->dev, PTR_ERR(dev),
>> +					    "error creating device\n");
>> +			goto destroy_device;
>> +		}
>> +		dev_dbg(&spi->dev, "created device major %u, minor
>> %d\n",
>> +			MAJOR(penctrl_devt), cs);
>> +	}
>> +
>> +	/* Register reset controller */
>> +	penctrl->rcdev.dev = &spi->dev;
>> +	penctrl->rcdev.ops = &penctrl_reset_ops;
>> +	penctrl->rcdev.owner = THIS_MODULE;
>> +	penctrl->rcdev.of_node = spi->dev.of_node;
>> +	penctrl->rcdev.nr_resets = 1;
>> +
>> +	ret = reset_controller_register(&penctrl->rcdev);
>> +	if (ret)
>> +		return dev_err_probe(&spi->dev, ret,
>> +				     "failed to register reset controller\n");
>> +	return 0;
>> +
>> +destroy_device:
>> +	for (cs = 0; cs < num_cs; cs++)
>> +		device_destroy(penctrl_class,
>> MKDEV(MAJOR(penctrl_devt), cs));
>> +	kfree(penctrl);
>> +free_cdev:
>> +	cdev_del(cdev);
>> +destroy_class:
>> +	class_destroy(penctrl_class);
>> +unregister_chrdev:
>> +	unregister_chrdev(MAJOR(penctrl_devt), "penctrl");
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct of_device_id penctrl_dt_match[] = {
>> +	{ .compatible = "amd,pensando-elba-ctrl" },
>> +	{ /* sentinel */ }
>> +};
>> +
>> +static struct spi_driver penctrl_spi_driver = {
>> +	.probe = penctrl_spi_probe,
>> +	.driver = {
>> +		.name = "pensando-elba-ctrl",
>> +		.of_match_table = penctrl_dt_match,
>> +	},
>> +};
>> +module_spi_driver(penctrl_spi_driver);
>> +
>> +MODULE_AUTHOR("Brad Larson <blarson@amd.com>");
>> MODULE_DESCRIPTION("AMD
>> +Pensando SoC Controller via SPI"); MODULE_LICENSE("GPL");
>> diff --git a/include/uapi/linux/amd-pensando-ctrl.h
>> b/include/uapi/linux/amd-pensando-ctrl.h
>> new file mode 100644
>> index 000000000000..e5f9f0dfe146
>> --- /dev/null
>> +++ b/include/uapi/linux/amd-pensando-ctrl.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Userspace interface for /dev/penctrl

You are missing AMD copyright here.
Also in DT patches I have seen that you didn't switch to 2023 year yet.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-15 18:16 ` [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support Brad Larson
  2023-05-16  7:17   ` Krzysztof Kozlowski
@ 2023-05-16  7:54   ` Michal Simek
  2023-05-23 19:28     ` Brad Larson
  1 sibling, 1 reply; 29+ messages in thread
From: Michal Simek @ 2023-05-16  7:54 UTC (permalink / raw)
  To: Brad Larson, linux-arm-kernel
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan,
	suravee.suthikulpanit, thomas.lendacky, tonyhuang.sunplus,
	ulf.hansson, vaishnav.a, walker.chen, will, zhuyinbo, devicetree



On 5/15/23 20:16, Brad Larson wrote:
> Add AMD Pensando common and Elba SoC specific device nodes
> 
> Signed-off-by: Brad Larson <blarson@amd.com>
> ---
> 
> v14 changes:
> - Fix dtbs_check l2-cache* property issue by adding required
>    cache-level and cache-unified properties
> - Observed the issue after updating dtschema from 2023.1 to 2023.4
>    and yamllint from 1.26.3 to 1.30.0
> 
> v11 changes:
> - Delete reset-names
> - Fix spi0 compatible to be specific 'amd,pensando-elba-ctrl'
> 
> v9 changes:
> - Single node for spi0 system-controller and squash
>    the reset-controller child into parent
> 
> ---
>   arch/arm64/boot/dts/amd/Makefile              |   1 +
>   arch/arm64/boot/dts/amd/elba-16core.dtsi      | 197 ++++++++++++++++++
>   arch/arm64/boot/dts/amd/elba-asic-common.dtsi |  80 +++++++
>   arch/arm64/boot/dts/amd/elba-asic.dts         |  28 +++
>   arch/arm64/boot/dts/amd/elba-flash-parts.dtsi | 106 ++++++++++
>   arch/arm64/boot/dts/amd/elba.dtsi             | 191 +++++++++++++++++
>   6 files changed, 603 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/amd/elba-16core.dtsi
>   create mode 100644 arch/arm64/boot/dts/amd/elba-asic-common.dtsi
>   create mode 100644 arch/arm64/boot/dts/amd/elba-asic.dts
>   create mode 100644 arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
>   create mode 100644 arch/arm64/boot/dts/amd/elba.dtsi
> 
> diff --git a/arch/arm64/boot/dts/amd/Makefile b/arch/arm64/boot/dts/amd/Makefile
> index 68103a8b0ef5..8502cc2afbc5 100644
> --- a/arch/arm64/boot/dts/amd/Makefile
> +++ b/arch/arm64/boot/dts/amd/Makefile
> @@ -1,2 +1,3 @@
>   # SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_PENSANDO) += elba-asic.dtb
>   dtb-$(CONFIG_ARCH_SEATTLE) += amd-overdrive-rev-b0.dtb amd-overdrive-rev-b1.dtb
> diff --git a/arch/arm64/boot/dts/amd/elba-16core.dtsi b/arch/arm64/boot/dts/amd/elba-16core.dtsi
> new file mode 100644
> index 000000000000..f9f9f5fd5f69
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd/elba-16core.dtsi
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +/*
> + * Copyright 2020-2022 Advanced Micro Devices, Inc.

2023 and the same below.

> + */
> +
> +/ {
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 { cpu = <&cpu0>; };
> +				core1 { cpu = <&cpu1>; };
> +				core2 { cpu = <&cpu2>; };
> +				core3 { cpu = <&cpu3>; };
> +			};
> +
> +			cluster1 {
> +				core0 { cpu = <&cpu4>; };
> +				core1 { cpu = <&cpu5>; };
> +				core2 { cpu = <&cpu6>; };
> +				core3 { cpu = <&cpu7>; };
> +			};
> +
> +			cluster2 {
> +				core0 { cpu = <&cpu8>; };
> +				core1 { cpu = <&cpu9>; };
> +				core2 { cpu = <&cpu10>; };
> +				core3 { cpu = <&cpu11>; };
> +			};
> +
> +			cluster3 {
> +				core0 { cpu = <&cpu12>; };
> +				core1 { cpu = <&cpu13>; };
> +				core2 { cpu = <&cpu14>; };
> +				core3 { cpu = <&cpu15>; };
> +			};
> +		};
> +
> +		/* CLUSTER 0 */
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x0>;

Do you really need 2/0 split here. The first cell is 0 anyway.


> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x1>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x2>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x3>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_0: l2-cache0 {
> +			compatible = "cache";
> +			cache-unified;
> +			cache-level = <2>;
> +		};
> +
> +		/* CLUSTER 1 */
> +		cpu4: cpu@100 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x100>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu5: cpu@101 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x101>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu6: cpu@102 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x102>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu7: cpu@103 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x103>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_1: l2-cache1 {
> +			compatible = "cache";
> +			cache-unified;
> +			cache-level = <2>;
> +		};
> +
> +		/* CLUSTER 2 */
> +		cpu8: cpu@200 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x200>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu9: cpu@201 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x201>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu10: cpu@202 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x202>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu11: cpu@203 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x203>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_2: l2-cache2 {
> +			compatible = "cache";
> +			cache-unified;
> +			cache-level = <2>;
> +		};
> +
> +		/* CLUSTER 3 */
> +		cpu12: cpu@300 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x300>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu13: cpu@301 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x301>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu14: cpu@302 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x302>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu15: cpu@303 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x303>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_3: l2-cache3 {
> +			compatible = "cache";
> +			cache-unified;
> +			cache-level = <2>;
> +		};
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/amd/elba-asic-common.dtsi b/arch/arm64/boot/dts/amd/elba-asic-common.dtsi
> new file mode 100644
> index 000000000000..1a615788f54e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd/elba-asic-common.dtsi
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +/*
> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
> + */
> +
> +&ahb_clk {
> +	clock-frequency = <400000000>;
> +};
> +
> +&emmc_clk {
> +	clock-frequency = <200000000>;
> +};
> +
> +&flash_clk {
> +	clock-frequency = <400000000>;
> +};
> +
> +&ref_clk {
> +	clock-frequency = <156250000>;
> +};
> +
> +&qspi {
> +	status = "okay";
> +
> +	flash0: flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <40000000>;
> +		spi-rx-bus-width = <2>;
> +		m25p,fast-read;
> +		cdns,read-delay = <0>;
> +		cdns,tshsl-ns = <0>;
> +		cdns,tsd2d-ns = <0>;
> +		cdns,tchsh-ns = <0>;
> +		cdns,tslch-ns = <0>;
> +	};
> +};
> +
> +&gpio0 {
> +	status = "okay";
> +};
> +
> +&emmc {
> +	bus-width = <8>;
> +	cap-mmc-hw-reset;
> +	resets = <&rstc 0>;
> +	status = "okay";
> +};
> +
> +&wdt0 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	clock-frequency = <100000>;
> +	status = "okay";
> +
> +	rtc@51 {
> +		compatible = "nxp,pcf85263";
> +		reg = <0x51>;
> +	};
> +};
> +
> +&spi0 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	num-cs = <4>;
> +	cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
> +		   <&porta 7 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +
> +	rstc: system-controller@0 {
> +		compatible = "amd,pensando-elba-ctrl";
> +		reg = <0>;
> +		spi-max-frequency = <12000000>;
> +		interrupt-parent = <&porta>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +		#reset-cells = <1>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/amd/elba-asic.dts b/arch/arm64/boot/dts/amd/elba-asic.dts
> new file mode 100644
> index 000000000000..c3f4da2f7449
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd/elba-asic.dts
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +/*
> + * Device Tree file for AMD Pensando Elba Board.
> + *
> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
> + */
> +
> +/dts-v1/;
> +
> +#include "elba.dtsi"
> +#include "elba-16core.dtsi"
> +#include "elba-asic-common.dtsi"
> +#include "elba-flash-parts.dtsi"
> +
> +/ {
> +	model = "AMD Pensando Elba Board";
> +	compatible = "amd,pensando-elba-ortano", "amd,pensando-elba";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		spi0 = &spi0;
> +		spi1 = &qspi;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
> new file mode 100644
> index 000000000000..734893fef2c3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +/*
> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
> + */
> +
> +&flash0 {
> +	partitions {
> +		compatible = "fixed-partitions";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		partition@0 {
> +			label = "flash";
> +			reg = <0x10000 0xfff0000>;

This doesn't fit with partition@0 above.
Also size is weird.


> +		};
> +
> +		partition@f0000 {
> +			label = "golduenv";
> +			reg = <0xf0000 0x10000>;
> +		};
> +
> +		partition@100000 {
> +			label = "boot0";
> +			reg = <0x100000 0x80000>;
> +		};
> +
> +		partition@180000 {
> +			label = "golduboot";
> +			reg = <0x180000 0x200000>;
> +		};
> +
> +		partition@380000 {
> +			label = "brdcfg0";
> +			reg = <0x380000 0x10000>;
> +		};
> +
> +		partition@390000 {
> +			label = "brdcfg1";
> +			reg = <0x390000 0x10000>;
> +		};
> +
> +		partition@400000 {
> +			label = "goldfw";
> +			reg = <0x400000 0x3c00000>;

This size looks weird.

> +		};
> +
> +		partition@4010000 {
> +			label = "fwmap";
> +			reg = <0x4010000 0x20000>;
> +		};
> +
> +		partition@4030000 {
> +			label = "fwsel";
> +			reg = <0x4030000 0x20000>;
> +		};
> +
> +		partition@4090000 {
> +			label = "bootlog";
> +			reg = <0x4090000 0x20000>;
> +		};
> +
> +		partition@40b0000 {
> +			label = "panicbuf";
> +			reg = <0x40b0000 0x20000>;
> +		};
> +
> +		partition@40d0000 {
> +			label = "uservars";
> +			reg = <0x40d0000 0x20000>;
> +		};
> +
> +		partition@4200000 {
> +			label = "uboota";
> +			reg = <0x4200000 0x400000>;
> +		};
> +
> +		partition@4600000 {
> +			label = "ubootb";
> +			reg = <0x4600000 0x400000>;
> +		};
> +
> +		partition@4a00000 {
> +			label = "mainfwa";
> +			reg = <0x4a00000 0x1000000>;
> +		};
> +
> +		partition@5a00000 {
> +			label = "mainfwb";
> +			reg = <0x5a00000 0x1000000>;
> +		};
> +
> +		partition@6a00000 {
> +			label = "diaguboot";
> +			reg = <0x6a00000 0x400000>;
> +		};
> +

here is gap

> +		partition@8000000 {
> +			label = "diagfw";
> +			reg = <0x8000000 0x7fe0000>;
> +		};
> +
> +		partition@ffe0000 {
> +			label = "ubootenv";
> +			reg = <0xffe0000 0x10000>;
> +		};

And this is missing space description.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
  2023-05-15 21:05   ` Andy Shevchenko
  2023-05-16  5:19   ` Mahapatra, Amit Kumar
@ 2023-05-16  8:45   ` kernel test robot
  2023-05-16 11:03   ` Arnd Bergmann
  3 siblings, 0 replies; 29+ messages in thread
From: kernel test robot @ 2023-05-16  8:45 UTC (permalink / raw)
  To: Brad Larson, linux-arm-kernel
  Cc: oe-kbuild-all, linux-kernel, linux-mmc, linux-spi, adrian.hunter,
	alcooperx, andy.shevchenko, arnd, blarson, brendan.higgins,
	briannorris, catalin.marinas, conor+dt, davidgow, gsomlo, gerg,
	hal.feng, hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel

Hi Brad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on e922ba281a8d84f640d8c8e18a385d032c19e185]

url:    https://github.com/intel-lab-lkp/linux/commits/Brad-Larson/dt-bindings-arm-add-AMD-Pensando-boards/20230516-032312
base:   e922ba281a8d84f640d8c8e18a385d032c19e185
patch link:    https://lore.kernel.org/r/20230515181606.65953-9-blarson%40amd.com
patch subject: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20230516/202305161642.PZQ4S8o2-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/48a90df35083d2f3788e171ff0af01ddc8cd871b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Brad-Larson/dt-bindings-arm-add-AMD-Pensando-boards/20230516-032312
        git checkout 48a90df35083d2f3788e171ff0af01ddc8cd871b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/soc/amd/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305161642.PZQ4S8o2-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/soc/amd/pensando-ctrl.c: In function 'penctrl_ioctl':
>> drivers/soc/amd/pensando-ctrl.c:91:36: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      91 |         if (copy_from_user(tx_buf, (void __user *)msg->tx_buf, msg->len)) {
         |                                    ^
   drivers/soc/amd/pensando-ctrl.c:114:26: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
     114 |         if (copy_to_user((void __user *)msg->rx_buf, rx_buf, msg->len))
         |                          ^


vim +91 drivers/soc/amd/pensando-ctrl.c

    34	
    35	static long
    36	penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
    37	{
    38		void __user *in_arg = (void __user *)arg;
    39		struct penctrl_device *penctrl;
    40		u8 tx_buf[PENCTRL_MAX_MSG_LEN];
    41		u8 rx_buf[PENCTRL_MAX_MSG_LEN];
    42		struct spi_transfer t[2] = {};
    43		struct penctrl_spi_xfer *msg;
    44		struct spi_device *spi;
    45		unsigned int num_msgs;
    46		struct spi_message m;
    47		u32 size;
    48		int ret;
    49	
    50		/* Check for a valid command */
    51		if (_IOC_TYPE(cmd) != PENCTRL_IOC_MAGIC)
    52			return -ENOTTY;
    53	
    54		if (_IOC_NR(cmd) > PENCTRL_IOC_MAXNR)
    55			return -ENOTTY;
    56	
    57		if (((_IOC_DIR(cmd) & _IOC_READ)) && !access_ok(in_arg, _IOC_SIZE(cmd)))
    58			return -EFAULT;
    59	
    60		if (((_IOC_DIR(cmd) & _IOC_WRITE)) && !access_ok(in_arg, _IOC_SIZE(cmd)))
    61			return -EFAULT;
    62	
    63		/* Get a reference to the SPI device */
    64		penctrl = filp->private_data;
    65		if (!penctrl)
    66			return -ESHUTDOWN;
    67	
    68		spi = spi_dev_get(penctrl->spi);
    69		if (!spi)
    70			return -ESHUTDOWN;
    71	
    72		/* Verify and prepare SPI message */
    73		size = _IOC_SIZE(cmd);
    74		num_msgs = size / sizeof(struct penctrl_spi_xfer);
    75		if (num_msgs > 2 || size == 0 || size % sizeof(struct penctrl_spi_xfer)) {
    76			ret = -EINVAL;
    77			goto out_unlock;
    78		}
    79		msg = memdup_user((struct penctrl_spi_xfer *)arg, size);
    80		if (IS_ERR(msg)) {
    81			ret = PTR_ERR(msg);
    82			goto out_unlock;
    83		}
    84		if (msg->len > PENCTRL_MAX_MSG_LEN) {
    85			ret = -EINVAL;
    86			goto out_unlock;
    87		}
    88	
    89		t[0].tx_buf = tx_buf;
    90		t[0].len = msg->len;
  > 91		if (copy_from_user(tx_buf, (void __user *)msg->tx_buf, msg->len)) {
    92			ret = -EFAULT;
    93			goto out_unlock;
    94		}
    95		if (num_msgs > 1) {
    96			msg++;
    97			if (msg->len > PENCTRL_MAX_MSG_LEN) {
    98				ret = -EINVAL;
    99				goto out_unlock;
   100			}
   101			t[1].rx_buf = rx_buf;
   102			t[1].len = msg->len;
   103		}
   104		spi_message_init_with_transfers(&m, t, num_msgs);
   105	
   106		/* Perform the transfer */
   107		mutex_lock(&spi_lock);
   108		ret = spi_sync(spi, &m);
   109		mutex_unlock(&spi_lock);
   110	
   111		if (ret || (num_msgs == 1))
   112			goto out_unlock;
   113	
   114		if (copy_to_user((void __user *)msg->rx_buf, rx_buf, msg->len))
   115			ret = -EFAULT;
   116	
   117	out_unlock:
   118		spi_dev_put(spi);
   119		return ret;
   120	}
   121	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
                     ` (2 preceding siblings ...)
  2023-05-16  8:45   ` kernel test robot
@ 2023-05-16 11:03   ` Arnd Bergmann
  2023-05-23 22:11     ` Brad Larson
  3 siblings, 1 reply; 29+ messages in thread
From: Arnd Bergmann @ 2023-05-16 11:03 UTC (permalink / raw)
  To: Brad Larson, linux-arm-kernel
  Cc: linux-kernel, linux-mmc @ vger . kernel . org, linux-spi,
	Adrian Hunter, alcooperx, Andy Shevchenko, brendan.higgins,
	Brian Norris, Catalin Marinas, Conor Dooley, David Gow, gsomlo,
	Greg Ungerer, Hal Feng, Hitomi Hasegawa,
	Jonathan Neuschäfer, Joel Stanley, Emil Renner Berthing,
	Krzysztof Kozlowski, krzysztof.kozlowski+dt, Lee Jones,
	Lee Jones, Mark Brown, Philipp Zabel, Randy Dunlap, Rob Herring,
	Samuel Holland, Serge Semin, skhan, suravee.suthikulpanit,
	Tom Lendacky, Tony Huang, Ulf Hansson, vaishnav.a, Walker Chen,
	Will Deacon, Yinbo Zhu, devicetree

On Mon, May 15, 2023, at 20:16, Brad Larson wrote:
> The Pensando SoC controller is a SPI connected companion device
> that is present in all Pensando SoC board designs.  The essential
> board management registers are accessed on chip select 0 with
> board mgmt IO support accessed using additional chip selects.
>
> Signed-off-by: Brad Larson <blarson@amd.com>

Hi Brad,

I'm sorry I wasn't paying enough attention to this driver as the
past 13 revisions went by.

> v10 changes:
> - Different driver implementation specific to this Pensando controller device.
> - Moved to soc/amd directory under new name based on guidance.  This driver is
>   of no use to any design other than all Pensando SoC based cards.
> - Removed use of builtin_driver, can be built as a module.

it looks like this was a fundamental change that I failed to see.

> +# SPDX-License-Identifier: GPL-2.0-only
> +menu "AMD Pensando SoC drivers"
> +
> +config AMD_PENSANDO_CTRL
> +	tristate "AMD Pensando SoC Controller"
> +	depends on SPI_MASTER=y
> +	depends on (ARCH_PENSANDO && OF) || COMPILE_TEST
> +	default ARCH_PENSANDO
> +	select REGMAP_SPI
> +	select MFD_SYSCON
> +	help
> +	  Enables AMD Pensando SoC controller device support.  This is a SPI
> +	  attached companion device in all Pensando SoC board designs which
> +	  provides essential board control/status registers and management IO
> +	  support.

So generally speaking, I don't want custom user interfaces in
drivers/soc. It looks like this one has internal interfaces for
a reset controller and the regmap, so those parts are fine, but
I'm confused about the purpose of the ioctl interface:

> +static long
> +penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{

> +	if (num_msgs > 1) {
> +		msg++;
> +		if (msg->len > PENCTRL_MAX_MSG_LEN) {
> +			ret = -EINVAL;
> +			goto out_unlock;
> +		}
> +		t[1].rx_buf = rx_buf;
> +		t[1].len = msg->len;
> +	}
> +	spi_message_init_with_transfers(&m, t, num_msgs);

This seems to be just a passthrough of user space messages, which
is what the spidev driver already provides, but using a different
ioctl interface. I don't really want any user-level interfaces
in drivers/soc as a rule, but having one that duplicates existing
functionality seems particularly wrong.

Can you explain what the purpose is? Is this about serializing
access between the in-kernel reset control and the user-side
access?

Also, can you explain why this needs a low-lever user interface
in the first place, rather than something that can be expressed
using high-level abstractions as you already do with the reset
control?

All of the above should be part of the changelog text to get a
driver like this merged. I don't think we can get a quick
solution here though, so maybe you can start by removing the
ioctl side and having the rest of the driver in drivers/reset?

     Arnd

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC
  2023-05-15 18:16 ` [PATCH v14 2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC Brad Larson
@ 2023-05-16 15:18   ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2023-05-16 15:18 UTC (permalink / raw)
  To: Brad Larson
  Cc: linux-arm-kernel, linux-kernel, linux-mmc, linux-spi,
	adrian.hunter, alcooperx, andy.shevchenko, arnd, brendan.higgins,
	briannorris, catalin.marinas, conor+dt, davidgow, gsomlo, gerg,
	hal.feng, hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, p.zabel, rdunlap,
	robh+dt, samuel, fancer.lancer, skhan, suravee.suthikulpanit,
	thomas.lendacky, tonyhuang.sunplus, ulf.hansson, vaishnav.a,
	walker.chen, will, zhuyinbo, devicetree

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

On Mon, May 15, 2023 at 11:16:00AM -0700, Brad Larson wrote:
> Document the cadence qspi controller compatible for AMD Pensando
> Elba SoC boards.  The Elba qspi fifo size is 1024.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-16  7:36     ` Michal Simek
@ 2023-05-17 11:14       ` Geert Uytterhoeven
  0 siblings, 0 replies; 29+ messages in thread
From: Geert Uytterhoeven @ 2023-05-17 11:14 UTC (permalink / raw)
  To: Michal Simek
  Cc: Mahapatra, Amit Kumar, Larson, Bradley, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, broonie, p.zabel,
	rdunlap, robh+dt, samuel, fancer.lancer, skhan, Suthikulpanit,
	Suravee, Lendacky, Thomas, tonyhuang.sunplus, ulf.hansson,
	vaishnav.a, walker.chen, will, zhuyinbo, devicetree,
	git (AMD-Xilinx)

Hi Michal,

On Tue, May 16, 2023 at 9:42 AM Michal Simek <michal.simek@amd.com> wrote:
> Also in DT patches I have seen that you didn't switch to 2023 year yet.

If no substantial changes were made in 2023, there is no need nor
desire to update the copyright year.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: (subset) [PATCH v14 0/8] Support AMD Pensando Elba SoC
  2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
                   ` (8 preceding siblings ...)
  2023-05-16  7:14 ` [PATCH v14 0/8] Support AMD Pensando Elba SoC Krzysztof Kozlowski
@ 2023-05-17 14:43 ` Mark Brown
  9 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2023-05-17 14:43 UTC (permalink / raw)
  To: linux-arm-kernel, Brad Larson
  Cc: linux-kernel, linux-mmc, linux-spi, adrian.hunter, alcooperx,
	andy.shevchenko, arnd, brendan.higgins, briannorris,
	catalin.marinas, conor+dt, davidgow, gsomlo, gerg, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee, lee.jones, p.zabel, rdunlap,
	robh+dt, samuel, fancer.lancer, skhan, suravee.suthikulpanit,
	thomas.lendacky, tonyhuang.sunplus, ulf.hansson, vaishnav.a,
	walker.chen, will, zhuyinbo, devicetree

On Mon, 15 May 2023 11:15:58 -0700, Brad Larson wrote:
> This series enables support for AMD 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
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC
      commit: f2156989bf3014c67707d73ccd202b2ada09080b
[7/8] spi: cadence-quadspi: Add compatible for AMD Pensando Elba SoC
      commit: f5c2f9f9584353bc816d76a65c97dd03dc61678c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-15 21:05   ` Andy Shevchenko
@ 2023-05-23  2:12     ` Brad Larson
  0 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-05-23  2:12 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: adrian.hunter, alcooperx, arnd, blarson, brendan.higgins,
	briannorris, broonie, catalin.marinas, conor+dt, davidgow,
	devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, p.zabel, rdunlap, robh+dt,
	samuel, skhan, suravee.suthikulpanit, thomas.lendacky,
	tonyhuang.sunplus, ulf.hansson, vaishnav.a, walker.chen, will,
	zhuyinbo

Hi Andy,

On Tue, May 16, 2023 at 00:05:32 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Mon, May 15, 2023 at 9:18 PM Brad Larson <blarson@amd.com> wrote:
>>
>> The Pensando SoC controller is a SPI connected companion device
>> that is present in all Pensando SoC board designs.  The essential
>> board management registers are accessed on chip select 0 with
>> board mgmt IO support accessed using additional chip selects.
>
> ...
>
>> +#include <linux/cdev.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>
>> +#include <linux/of.h>
>
> Unneeded inclusion.

Removed

>> +#include <linux/reset-controller.h>
>> +#include <linux/spi/spi.h>
>
> ...
>
>
>> +       u8 tx_buf[PENCTRL_MAX_MSG_LEN];
>> +       u8 rx_buf[PENCTRL_MAX_MSG_LEN];
>
> Does it need to be DMA-capable?

Doesn't need to be DMA-capable

> ...
>
>> +       spi->chip_select = current_cs;
>> +       spi->cs_gpiod = spi->controller->cs_gpiods[current_cs];
>
> Nowadays these require API calls instead of direct assignments.

Changed to:
	spi_set_csgpiod(spi, 0, spi->controller->cs_gpiods[current_cs]);

> ...
>
>> +static int penctrl_release(struct inode *inode, struct file *filp)
>> +{
>> +       filp->private_data = NULL;
>> +       return 0;
>> +}
>
> Is it possible to unload the module without releasing the device node?

If the refcount is not zero the kernel prevents the module from being unloaded.

> ...
>
>> +       u8 txbuf[3];
>> +       u8 rxbuf[1];
>
> Same question about DMA.

Not DMA-capable

> ...
>
>> +       ret = spi_sync(spi, &m);
>
>> +       if (ret == 0)
>> +               *val = rxbuf[0];
>> +
>> +       return ret;
>
> Can also be written in more usual way:
>
>  if (ret)
>    return ret;
>  ...
>  return 0;

Yes, changed to:
        ret = spi_sync(spi, &m);
        if (ret)
                return ret;

        *val = rxbuf[0];
        return 0;

> ...
>
>> +       u8 txbuf[4];
>
> DMA?

Not DMA-capable

> ...
>
>> +       spi->chip_select = 0;
>> +       spi->cs_gpiod = spi->controller->cs_gpiods[0];
>
> Setter APIs.

Changed to:
	spi_set_csgpiod(spi, 0, spi->controller->cs_gpiods[0]);

>
> ...
>
>> +       spi->chip_select = 0;
>> +       spi->cs_gpiod = spi->controller->cs_gpiods[0];
>
> Ditto.

Changed to:
	spi_set_csgpiod(spi, 0, spi->controller->cs_gpiods[0]);

>> +       ret = device_property_read_u32(spi->dev.parent, "num-cs", &num_cs);
>> +       if (ret)
>> +               return dev_err_probe(&spi->dev, ret,
>> +                                    "number of chip-selects not defined\n");
>
> Hmm... Shouldn't SPI core take care of this in a generic way? Yes, I
> understand that you need the number for the allocation, but I would
> expect something like spi_fw_get_num_cs() to exist (seems not?).
>

No need to look into the parent node, changed to this:

        num_cs = spi->controller->num_chipselect;

> ...
>
>> +       penctrl->rcdev.of_node = spi->dev.of_node;
>
> Use device_set_node(). It helps to modify the data types beneath.

Added:
	device_set_node(penctrl->rcdev.dev, dev_fwnode(&spi->dev));

Regards,
Brad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-16  7:54   ` Michal Simek
@ 2023-05-23 19:28     ` Brad Larson
  2023-05-24 11:52       ` Geert Uytterhoeven
  0 siblings, 1 reply; 29+ messages in thread
From: Brad Larson @ 2023-05-23 19:28 UTC (permalink / raw)
  To: michal.simek
  Cc: adrian.hunter, alcooperx, andy.shevchenko, arnd, blarson,
	brendan.higgins, briannorris, broonie, catalin.marinas, conor+dt,
	davidgow, devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, p.zabel, rdunlap, robh+dt,
	samuel, skhan, suravee.suthikulpanit, thomas.lendacky,
	tonyhuang.sunplus, ulf.hansson, vaishnav.a, walker.chen, will,
	zhuyinbo

Hi Michal,

Thanks for reviewing the patch.

On 5/16/23 09:54, Michal Simek wrote:
> On 5/15/23 20:16, Brad Larson wrote:
>> Add AMD Pensando common and Elba SoC specific device nodes
>> 
>> Signed-off-by: Brad Larson <blarson@amd.com>
>> ---
>> 
>> v14 changes:
>> - Fix dtbs_check l2-cache* property issue by adding required
>>    cache-level and cache-unified properties
>> - Observed the issue after updating dtschema from 2023.1 to 2023.4
>>    and yamllint from 1.26.3 to 1.30.0
>> 
>> v11 changes:
>> - Delete reset-names
>> - Fix spi0 compatible to be specific 'amd,pensando-elba-ctrl'
>> 
>> v9 changes:
>> - Single node for spi0 system-controller and squash
>>    the reset-controller child into parent
>> 
>> ---
>>   arch/arm64/boot/dts/amd/Makefile              |   1 +
>>   arch/arm64/boot/dts/amd/elba-16core.dtsi      | 197 ++++++++++++++++++
>>   arch/arm64/boot/dts/amd/elba-asic-common.dtsi |  80 +++++++
>>   arch/arm64/boot/dts/amd/elba-asic.dts         |  28 +++
>>   arch/arm64/boot/dts/amd/elba-flash-parts.dtsi | 106 ++++++++++
>>   arch/arm64/boot/dts/amd/elba.dtsi             | 191 +++++++++++++++++
>>   6 files changed, 603 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/amd/elba-16core.dtsi
>>   create mode 100644 arch/arm64/boot/dts/amd/elba-asic-common.dtsi
>>   create mode 100644 arch/arm64/boot/dts/amd/elba-asic.dts
>>   create mode 100644 arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
>>   create mode 100644 arch/arm64/boot/dts/amd/elba.dtsi
>> 
>> diff --git a/arch/arm64/boot/dts/amd/Makefile b/arch/arm64/boot/dts/amd/Makefile
>> index 68103a8b0ef5..8502cc2afbc5 100644
>> --- a/arch/arm64/boot/dts/amd/Makefile
>> +++ b/arch/arm64/boot/dts/amd/Makefile
>> @@ -1,2 +1,3 @@
>>   # SPDX-License-Identifier: GPL-2.0
>> +dtb-$(CONFIG_ARCH_PENSANDO) += elba-asic.dtb
>>   dtb-$(CONFIG_ARCH_SEATTLE) += amd-overdrive-rev-b0.dtb amd-overdrive-rev-b1.dtb
>> diff --git a/arch/arm64/boot/dts/amd/elba-16core.dtsi b/arch/arm64/boot/dts/amd/elba-16core.dtsi
>> new file mode 100644
>> index 000000000000..f9f9f5fd5f69
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amd/elba-16core.dtsi
>> @@ -0,0 +1,197 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> +/*
>> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
>
> 2023 and the same below.

I'll update the copyright in the next submit

>> + */
>> +
>> +/ {
>> +	cpus {
>> +		#address-cells = <2>;
>> +		#size-cells = <0>;
>> +
>> +		cpu-map {
>> +			cluster0 {
>> +				core0 { cpu = <&cpu0>; };
>> +				core1 { cpu = <&cpu1>; };
>> +				core2 { cpu = <&cpu2>; };
>> +				core3 { cpu = <&cpu3>; };
>> +			};
>> +
>> +			cluster1 {
>> +				core0 { cpu = <&cpu4>; };
>> +				core1 { cpu = <&cpu5>; };
>> +				core2 { cpu = <&cpu6>; };
>> +				core3 { cpu = <&cpu7>; };
>> +			};
>> +
>> +			cluster2 {
>> +				core0 { cpu = <&cpu8>; };
>> +				core1 { cpu = <&cpu9>; };
>> +				core2 { cpu = <&cpu10>; };
>> +				core3 { cpu = <&cpu11>; };
>> +			};
>> +
>> +			cluster3 {
>> +				core0 { cpu = <&cpu12>; };
>> +				core1 { cpu = <&cpu13>; };
>> +				core2 { cpu = <&cpu14>; };
>> +				core3 { cpu = <&cpu15>; };
>> +			};
>> +		};
>> +
>> +		/* CLUSTER 0 */
>> +		cpu0: cpu@0 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a72";
>> +			reg = <0 0x0>;
>
> Do you really need 2/0 split here. The first cell is 0 anyway.

Yes following 64-bit system definition

...

>> diff --git a/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
>> new file mode 100644
>> index 000000000000..734893fef2c3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
>> @@ -0,0 +1,106 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> +/*
>> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
>> + */
>> +
>> +&flash0 {
0xf0000>> +	partitions {
>> +		compatible = "fixed-partitions";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		partition@0 {
>> +			label = "flash";
>> +			reg = <0x10000 0xfff0000>;
>
> This doesn't fit with partition@0 above.
> Also size is weird.

This is intended to not expose sector 0.

>> +		};
>> +
>> +		partition@f0000 {
>> +			label = "golduenv";
>> +			reg = <0xf0000 0x10000>;
>> +		};
>> +
>> +		partition@100000 {
>> +			label = "boot0";
>> +			reg = <0x100000 0x80000>;
>> +		};
>> +
>> +		partition@180000 {
>> +			label = "golduboot";
>> +			reg = <0x180000 0x200000>;
>> +		};
>> +
>> +		partition@380000 {
>> +			label = "brdcfg0";
>> +			reg = <0x380000 0x10000>;
>> +		};
>> +
>> +		partition@390000 {
>> +			label = "brdcfg1";
>> +			reg = <0x390000 0x10000>;
>> +		};
>> +
>> +		partition@400000 {
>> +			label = "goldfw";
>> +			reg = <0x400000 0x3c00000>;
>
> This size looks weird.

It's the allocated size for this firmware component.

>> +		};
>> +
>> +		partition@4010000 {
>> +			label = "fwmap";
>> +			reg = <0x4010000 0x20000>;
>> +		};
>> +
>> +		partition@4030000 {
>> +			label = "fwsel";
>> +			reg = <0x4030000 0x20000>;
>> +		};
>> +
>> +		partition@4090000 {
>> +			label = "bootlog";
>> +			reg = <0x4090000 0x20000>;
>> +		};
>> +
>> +		partition@40b0000 {
>> +			label = "panicbuf";
>> +			reg = <0x40b0000 0x20000>;
>> +		};
>> +
>> +		partition@40d0000 {
>> +			label = "uservars";
>> +			reg = <0x40d0000 0x20000>;
>> +		};
>> +
>> +		partition@4200000 {
>> +			label = "uboota";
>> +			reg = <0x4200000 0x400000>;
>> +		};
>> +
>> +		partition@4600000 {
>> +			label = "ubootb";
>> +			reg = <0x4600000 0x400000>;
>> +		};
>> +
>> +		partition@4a00000 {
>> +			label = "mainfwa";
>> +			reg = <0x4a00000 0x1000000>;
>> +		};
>> +
>> +		partition@5a00000 {
>> +			label = "mainfwb";
>> +			reg = <0x5a00000 0x1000000>;
>> +		};
>> +
>> +		partition@6a00000 {
>> +			label = "diaguboot";
>> +			reg = <0x6a00000 0x400000>;
>> +		};
>> +
>
> here is gap

This is intentional for unallocated space.  I'll put in a 'spare' partition.

>> +		partition@8000000 {
>> +			label = "diagfw";
>> +			reg = <0x8000000 0x7fe0000>;
>> +		};
>> +
>> +		partition@ffe0000 {
>> +			label = "ubootenv";
>> +			reg = <0xffe0000 0x10000>;
>> +		};
>
> And this is missing space description.

space description?

Regards,
Brad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-16 11:03   ` Arnd Bergmann
@ 2023-05-23 22:11     ` Brad Larson
  2023-05-24 12:41       ` Arnd Bergmann
  0 siblings, 1 reply; 29+ messages in thread
From: Brad Larson @ 2023-05-23 22:11 UTC (permalink / raw)
  To: arnd
  Cc: adrian.hunter, alcooperx, andy.shevchenko, blarson,
	brendan.higgins, briannorris, broonie, catalin.marinas, conor+dt,
	davidgow, devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, p.zabel, rdunlap, robh+dt,
	samuel, skhan, suravee.suthikulpanit, thomas.lendacky,
	tonyhuang.sunplus, ulf.hansson, vaishnav.a, walker.chen, will,
	zhuyinbo

Hi Arnd,

> On Mon, May 15, 2023, at 20:16, Brad Larson wrote:
>> The Pensando SoC controller is a SPI connected companion device
>> that is present in all Pensando SoC board designs.  The essential
>> board management registers are accessed on chip select 0 with
>> board mgmt IO support accessed using additional chip selects.
>>
>> Signed-off-by: Brad Larson <blarson@amd.com>
>
> Hi Brad,
>
> I'm sorry I wasn't paying enough attention to this driver as the
> past 13 revisions went by.
>
No worries, bit of a saga.  See explanation below.

>> v10 changes:
>> - Different driver implementation specific to this Pensando controller device.
>> - Moved to soc/amd directory under new name based on guidance.  This driver is
>>   of no use to any design other than all Pensando SoC based cards.
>> - Removed use of builtin_driver, can be built as a module.
>
> it looks like this was a fundamental change that I failed to see.

See explanation below.

>> +# SPDX-License-Identifier: GPL-2.0-only
>> +menu "AMD Pensando SoC drivers"
>> +
>> +config AMD_PENSANDO_CTRL
>> +	tristate "AMD Pensando SoC Controller"
>> +	depends on SPI_MASTER=y
>> +	depends on (ARCH_PENSANDO && OF) || COMPILE_TEST
>> +	default ARCH_PENSANDO
>> +	select REGMAP_SPI
>> +	select MFD_SYSCON
>> +	help
>> +	  Enables AMD Pensando SoC controller device support.  This is a SPI
>> +	  attached companion device in all Pensando SoC board designs which
>> +	  provides essential board control/status registers and management IO
>> +	  support.
>
> So generally speaking, I don't want custom user interfaces in
> drivers/soc. It looks like this one has internal interfaces for
> a reset controller and the regmap, so those parts are fine, but
> I'm confused about the purpose of the ioctl interface:
>
>> +static long
>> +penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> +{
>
>> +	if (num_msgs > 1) {
>> +		msg++;
>> +		if (msg->len > PENCTRL_MAX_MSG_LEN) {
>> +			ret = -EINVAL;
>> +			goto out_unlock;
>> +		}
>> +		t[1].rx_buf = rx_buf;
>> +		t[1].len = msg->len;
>> +	}
>> +	spi_message_init_with_transfers(&m, t, num_msgs);
>
> This seems to be just a passthrough of user space messages, which
> is what the spidev driver already provides, but using a different
> ioctl interface. I don't really want any user-level interfaces
> in drivers/soc as a rule, but having one that duplicates existing
> functionality seems particularly wrong.
>
> Can you explain what the purpose is? Is this about serializing
> access between the in-kernel reset control and the user-side
> access?
>
> Also, can you explain why this needs a low-lever user interface
> in the first place, rather than something that can be expressed
> using high-level abstractions as you already do with the reset
> control?
>
> All of the above should be part of the changelog text to get a
> driver like this merged. I don't think we can get a quick
> solution here though, so maybe you can start by removing the
> ioctl side and having the rest of the driver in drivers/reset?

In the original patchset I added a pensando compatible to spidev and that
was nacked in review and reusing some random compatible that made it into 
spidev was just wrong.  Further it was recommended this should be a system 
specific driver and don't rely on a debug driver like spidev.  I changed 
over to a platform specific driver and at that time I also needed to include 
a reset controller (emmc reset only).  I put these in drivers/mfd and 
drivers/reset.  Review of the device tree for this approach went back and 
forth to _not_ have four child nodes on the spi device each with the same 
compatible. Decision was to squash the child nodes into the parent and put 
the reset-controller there also.  One driver and since its pensando
specific its currently in drivers/soc/amd.

There are five different user processes and some utilities that access the 
functionality in the cpld/fpga.  You're correct, its passing messages that 
are specific to the IP accessed via chip-select.  No Elba system will boot 
without this driver providing ioctl access.

Regards,
Brad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-23 19:28     ` Brad Larson
@ 2023-05-24 11:52       ` Geert Uytterhoeven
  2023-05-30 22:03         ` Brad Larson
  0 siblings, 1 reply; 29+ messages in thread
From: Geert Uytterhoeven @ 2023-05-24 11:52 UTC (permalink / raw)
  To: Brad Larson
  Cc: michal.simek, adrian.hunter, alcooperx, andy.shevchenko, arnd,
	brendan.higgins, briannorris, broonie, catalin.marinas, conor+dt,
	davidgow, devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, p.zabel, rdunlap, robh+dt,
	samuel, skhan, suravee.suthikulpanit, thomas.lendacky,
	tonyhuang.sunplus, ulf.hansson, vaishnav.a, walker.chen, will,
	zhuyinbo

Hi Brad,

On Tue, May 23, 2023 at 9:30 PM Brad Larson <blarson@amd.com> wrote:
> On 5/16/23 09:54, Michal Simek wrote:
> > On 5/15/23 20:16, Brad Larson wrote:
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/amd/elba-16core.dtsi
> >> @@ -0,0 +1,197 @@
> >> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> >> +/*
> >> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
> >
> > 2023 and the same below.
>
> I'll update the copyright in the next submit

Did you make any substantial changes in 2023?

> >> + */
> >> +
> >> +/ {
> >> +    cpus {
> >> +            #address-cells = <2>;
> >> +            #size-cells = <0>;
> >> +
> >> +            cpu-map {
> >> +                    cluster0 {
> >> +                            core0 { cpu = <&cpu0>; };
> >> +                            core1 { cpu = <&cpu1>; };
> >> +                            core2 { cpu = <&cpu2>; };
> >> +                            core3 { cpu = <&cpu3>; };
> >> +                    };
> >> +
> >> +                    cluster1 {
> >> +                            core0 { cpu = <&cpu4>; };
> >> +                            core1 { cpu = <&cpu5>; };
> >> +                            core2 { cpu = <&cpu6>; };
> >> +                            core3 { cpu = <&cpu7>; };
> >> +                    };
> >> +
> >> +                    cluster2 {
> >> +                            core0 { cpu = <&cpu8>; };
> >> +                            core1 { cpu = <&cpu9>; };
> >> +                            core2 { cpu = <&cpu10>; };
> >> +                            core3 { cpu = <&cpu11>; };
> >> +                    };
> >> +
> >> +                    cluster3 {
> >> +                            core0 { cpu = <&cpu12>; };
> >> +                            core1 { cpu = <&cpu13>; };
> >> +                            core2 { cpu = <&cpu14>; };
> >> +                            core3 { cpu = <&cpu15>; };
> >> +                    };
> >> +            };
> >> +
> >> +            /* CLUSTER 0 */
> >> +            cpu0: cpu@0 {
> >> +                    device_type = "cpu";
> >> +                    compatible = "arm,cortex-a72";
> >> +                    reg = <0 0x0>;
> >
> > Do you really need 2/0 split here. The first cell is 0 anyway.
>
> Yes following 64-bit system definition

You mean for the 64-bit main address space?
The CPU address space under /cpus is unrelated.

> >> +++ b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
> >> @@ -0,0 +1,106 @@
> >> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> >> +/*
> >> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
> >> + */
> >> +
> >> +&flash0 {
> 0xf0000>> +     partitions {
> >> +            compatible = "fixed-partitions";
> >> +            #address-cells = <1>;
> >> +            #size-cells = <1>;
> >> +            partition@0 {
> >> +                    label = "flash";
> >> +                    reg = <0x10000 0xfff0000>;
> >
> > This doesn't fit with partition@0 above.
> > Also size is weird.
>
> This is intended to not expose sector 0.

The unit address should still match the first reg entry
=> partition@10000.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-23 22:11     ` Brad Larson
@ 2023-05-24 12:41       ` Arnd Bergmann
  2023-08-07 22:17         ` Brad Larson
  0 siblings, 1 reply; 29+ messages in thread
From: Arnd Bergmann @ 2023-05-24 12:41 UTC (permalink / raw)
  To: Brad Larson
  Cc: Adrian Hunter, alcooperx, Andy Shevchenko, brendan.higgins,
	Brian Norris, Mark Brown, Catalin Marinas, Conor Dooley,
	David Gow, devicetree, Serge Semin, Greg Ungerer, gsomlo,
	Hal Feng, Hitomi Hasegawa, Jonathan Neuschäfer,
	Joel Stanley, Emil Renner Berthing, Krzysztof Kozlowski,
	krzysztof.kozlowski+dt, Lee Jones, Lee Jones, linux-arm-kernel,
	linux-kernel, linux-mmc @ vger . kernel . org, linux-spi,
	Philipp Zabel, Randy Dunlap, Rob Herring, Samuel Holland, skhan,
	suravee.suthikulpanit, Tom Lendacky, Tony Huang, Ulf Hansson,
	vaishnav.a, Walker Chen, Will Deacon, Yinbo Zhu

On Wed, May 24, 2023, at 00:11, Brad Larson wrote:
>> On Mon, May 15, 2023, at 20:16, Brad Larson wrote:
>
>> Also, can you explain why this needs a low-lever user interface
>> in the first place, rather than something that can be expressed
>> using high-level abstractions as you already do with the reset
>> control?
>>
>> All of the above should be part of the changelog text to get a
>> driver like this merged. I don't think we can get a quick
>> solution here though, so maybe you can start by removing the
>> ioctl side and having the rest of the driver in drivers/reset?
>
> In the original patchset I added a pensando compatible to spidev and that
> was nacked in review and reusing some random compatible that made it into 
> spidev was just wrong.  Further it was recommended this should be a system 
> specific driver and don't rely on a debug driver like spidev.  I changed 
> over to a platform specific driver and at that time I also needed to include 
> a reset controller (emmc reset only).  I put these in drivers/mfd and 
> drivers/reset.  Review of the device tree for this approach went back and 
> forth to _not_ have four child nodes on the spi device each with the same 
> compatible. Decision was to squash the child nodes into the parent and put 
> the reset-controller there also.  One driver and since its pensando
> specific its currently in drivers/soc/amd.
>
> There are five different user processes and some utilities that access the 
> functionality in the cpld/fpga.  You're correct, its passing messages that 
> are specific to the IP accessed via chip-select.  No Elba system will boot 
> without this driver providing ioctl access.

Thank you for the detailed summary. Moving away from spidev and
from mfd seems all reasonable here. I'm still a bit confused by
why you have multiple chipselects here that are for different
subdevices but ended with a single user interface for all of them,
but that's not a big deal.

The main bit that sticks out about this high-level design is how
it relies on user space utilities at all to understand the message
format. From what I understand about the actual functionality of
this device, it most closely resembles an embedded controller that
you might find in a laptop or server machine, and those usually
have kernel drivers in drivers/platform/ to interact with the
device.

Has anyone tried to do it like that? Maybe it would help
to see what the full protocol and the user space side looks
like, in order to move some or all of it into the kernel.

     Arnd

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-24 11:52       ` Geert Uytterhoeven
@ 2023-05-30 22:03         ` Brad Larson
  2023-05-31 13:09           ` Geert Uytterhoeven
  0 siblings, 1 reply; 29+ messages in thread
From: Brad Larson @ 2023-05-30 22:03 UTC (permalink / raw)
  To: geert
  Cc: adrian.hunter, alcooperx, andy.shevchenko, arnd, blarson,
	brendan.higgins, briannorris, broonie, catalin.marinas, conor+dt,
	davidgow, devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, michal.simek, p.zabel,
	rdunlap, robh+dt, samuel, skhan, suravee.suthikulpanit,
	thomas.lendacky, tonyhuang.sunplus, ulf.hansson, vaishnav.a,
	walker.chen, will, zhuyinbo

Hi Geert,

On Wed, May 24, 2023 at 13:52 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, May 23, 2023 at 9:30 PM Brad Larson <blarson@amd.com> wrote:
>> On 5/16/23 09:54, Michal Simek wrote:
>> > On 5/15/23 20:16, Brad Larson wrote:
>> >> --- /dev/null
>> >> +++ b/arch/arm64/boot/dts/amd/elba-16core.dtsi
>> >> @@ -0,0 +1,197 @@
>> >> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> >> +/*
>> >> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
>> >
>> > 2023 and the same below.
>>
>> I'll update the copyright in the next submit
>
> Did you make any substantial changes in 2023?

Yes, additional properties were added to l2-cache*

>> >> + */
>> >> +
>> >> +/ {
>> >> +    cpus {
>> >> +            #address-cells = <2>;
>> >> +            #size-cells = <0>;
>> >> +
>> >> +            cpu-map {
>> >> +                    cluster0 {
>> >> +                            core0 { cpu = <&cpu0>; };
>> >> +                            core1 { cpu = <&cpu1>; };
>> >> +                            core2 { cpu = <&cpu2>; };
>> >> +                            core3 { cpu = <&cpu3>; };
>> >> +                    };
>> >> +
>> >> +                    cluster1 {
>> >> +                            core0 { cpu = <&cpu4>; };
>> >> +                            core1 { cpu = <&cpu5>; };
>> >> +                            core2 { cpu = <&cpu6>; };
>> >> +                            core3 { cpu = <&cpu7>; };
>> >> +                    };
>> >> +
>> >> +                    cluster2 {
>> >> +                            core0 { cpu = <&cpu8>; };
>> >> +                            core1 { cpu = <&cpu9>; };
>> >> +                            core2 { cpu = <&cpu10>; };
>> >> +                            core3 { cpu = <&cpu11>; };
>> >> +                    };
>> >> +
>> >> +                    cluster3 {
>> >> +                            core0 { cpu = <&cpu12>; };
>> >> +                            core1 { cpu = <&cpu13>; };
>> >> +                            core2 { cpu = <&cpu14>; };
>> >> +                            core3 { cpu = <&cpu15>; };
>> >> +                    };
>> >> +            };
>> >> +
>> >> +            /* CLUSTER 0 */
>> >> +            cpu0: cpu@0 {
>> >> +                    device_type = "cpu";
>> >> +                    compatible = "arm,cortex-a72";
>> >> +                    reg = <0 0x0>;
>> >
>> > Do you really need 2/0 split here. The first cell is 0 anyway.
>>
>> Yes following 64-bit system definition
>
> You mean for the 64-bit main address space?
> The CPU address space under /cpus is unrelated.

Yes, the reg prop for this node is CPU/threads per dt spec.  Checked the history and
the Elba dt was derived from socionext for these nodes and this is how those device
trees are configured along with over a dozen other devices.  I changed to 
address-cells = <1> and dropped the leading zero from all cpu* reg<> and booting
the system I'm observing no change.  Looking in drivers/of I'm not seeing where
cpu*/reg is read and used, any recommendation?

>> >> +++ b/arch/arm64/boot/dts/amd/elba-flash-parts.dtsi
>> >> @@ -0,0 +1,106 @@
>> >> +// SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> >> +/*
>> >> + * Copyright 2020-2022 Advanced Micro Devices, Inc.
>> >> + */
>> >> +
>> >> +&flash0 {
>> 0xf0000>> +     partitions {
>> >> +            compatible = "fixed-partitions";
>> >> +            #address-cells = <1>;
>> >> +            #size-cells = <1>;
>> >> +            partition@0 {
>> >> +                    label = "flash";
>> >> +                    reg = <0x10000 0xfff0000>;
>> >
>> > This doesn't fit with partition@0 above.
>> > Also size is weird.
>>
>> This is intended to not expose sector 0.
>
> The unit address should still match the first reg entry
> => partition@10000.

Changed to this:

                partition@0 {
                        label = "rsvd";
                        reg = <0x0 0x10000>;
                        read-only;
                };

                partition@10000 {
                        label = "flash";
                        reg = <0x10000 0xfff0000>;
                };

Regards,
Brad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-30 22:03         ` Brad Larson
@ 2023-05-31 13:09           ` Geert Uytterhoeven
  2023-06-05 23:52             ` Brad Larson
  0 siblings, 1 reply; 29+ messages in thread
From: Geert Uytterhoeven @ 2023-05-31 13:09 UTC (permalink / raw)
  To: Brad Larson
  Cc: adrian.hunter, alcooperx, andy.shevchenko, arnd, brendan.higgins,
	briannorris, broonie, catalin.marinas, conor+dt, davidgow,
	devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, michal.simek, p.zabel,
	rdunlap, robh+dt, samuel, skhan, suravee.suthikulpanit,
	thomas.lendacky, tonyhuang.sunplus, ulf.hansson, vaishnav.a,
	walker.chen, will, zhuyinbo

Hi Brad,

On Wed, May 31, 2023 at 12:04 AM Brad Larson <blarson@amd.com> wrote:
> On Wed, May 24, 2023 at 13:52 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, May 23, 2023 at 9:30 PM Brad Larson <blarson@amd.com> wrote:
> >> On 5/16/23 09:54, Michal Simek wrote:
> >> > On 5/15/23 20:16, Brad Larson wrote:
> >> >> + */
> >> >> +
> >> >> +/ {
> >> >> +    cpus {
> >> >> +            #address-cells = <2>;
> >> >> +            #size-cells = <0>;
> >> >> +
> >> >> +            cpu-map {
> >> >> +                    cluster0 {
> >> >> +                            core0 { cpu = <&cpu0>; };
> >> >> +                            core1 { cpu = <&cpu1>; };
> >> >> +                            core2 { cpu = <&cpu2>; };
> >> >> +                            core3 { cpu = <&cpu3>; };
> >> >> +                    };
> >> >> +
> >> >> +                    cluster1 {
> >> >> +                            core0 { cpu = <&cpu4>; };
> >> >> +                            core1 { cpu = <&cpu5>; };
> >> >> +                            core2 { cpu = <&cpu6>; };
> >> >> +                            core3 { cpu = <&cpu7>; };
> >> >> +                    };
> >> >> +
> >> >> +                    cluster2 {
> >> >> +                            core0 { cpu = <&cpu8>; };
> >> >> +                            core1 { cpu = <&cpu9>; };
> >> >> +                            core2 { cpu = <&cpu10>; };
> >> >> +                            core3 { cpu = <&cpu11>; };
> >> >> +                    };
> >> >> +
> >> >> +                    cluster3 {
> >> >> +                            core0 { cpu = <&cpu12>; };
> >> >> +                            core1 { cpu = <&cpu13>; };
> >> >> +                            core2 { cpu = <&cpu14>; };
> >> >> +                            core3 { cpu = <&cpu15>; };
> >> >> +                    };
> >> >> +            };
> >> >> +
> >> >> +            /* CLUSTER 0 */
> >> >> +            cpu0: cpu@0 {
> >> >> +                    device_type = "cpu";
> >> >> +                    compatible = "arm,cortex-a72";
> >> >> +                    reg = <0 0x0>;
> >> >
> >> > Do you really need 2/0 split here. The first cell is 0 anyway.
> >>
> >> Yes following 64-bit system definition
> >
> > You mean for the 64-bit main address space?
> > The CPU address space under /cpus is unrelated.
>
> Yes, the reg prop for this node is CPU/threads per dt spec.  Checked the history and
> the Elba dt was derived from socionext for these nodes and this is how those device
> trees are configured along with over a dozen other devices.  I changed to
> address-cells = <1> and dropped the leading zero from all cpu* reg<> and booting
> the system I'm observing no change.  Looking in drivers/of I'm not seeing where
> cpu*/reg is read and used, any recommendation?

drivers/of/cpu.c

Looks like there are lots of DTS files that use #address-cells = <2> for
CPU nodes :-(

    git grep -w -A1 cpus -- "*dts*" | grep address-cells | grep "<2>"

I would use <1> is the first cell is always zero...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support
  2023-05-31 13:09           ` Geert Uytterhoeven
@ 2023-06-05 23:52             ` Brad Larson
  0 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-06-05 23:52 UTC (permalink / raw)
  To: geert
  Cc: adrian.hunter, alcooperx, andy.shevchenko, arnd, blarson,
	brendan.higgins, briannorris, broonie, catalin.marinas, conor+dt,
	davidgow, devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, michal.simek, p.zabel,
	rdunlap, robh+dt, samuel, skhan, suravee.suthikulpanit,
	thomas.lendacky, tonyhuang.sunplus, ulf.hansson, vaishnav.a,
	walker.chen, will, zhuyinbo

Hi Geert,

On Wed, May 31, 2023 at 15:09 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Wed, May 31, 2023 at 12:04 AM Brad Larson <blarson@amd.com> wrote:
>> On Wed, May 24, 2023 at 13:52 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> > On Tue, May 23, 2023 at 9:30 PM Brad Larson <blarson@amd.com> wrote:
>> >> On 5/16/23 09:54, Michal Simek wrote:
>> >> > On 5/15/23 20:16, Brad Larson wrote:
...
>> >> >> +            /* CLUSTER 0 */
>> >> >> +            cpu0: cpu@0 {
>> >> >> +                    device_type = "cpu";
>> >> >> +                    compatible = "arm,cortex-a72";
>> >> >> +                    reg = <0 0x0>;
>> >> >
>> >> > Do you really need 2/0 split here. The first cell is 0 anyway.
>> >>
>> >> Yes following 64-bit system definition
>> >
>> > You mean for the 64-bit main address space?
>> > The CPU address space under /cpus is unrelated.
>>
>> Yes, the reg prop for this node is CPU/threads per dt spec.  Checked the history and
>> the Elba dt was derived from socionext for these nodes and this is how those device
>> trees are configured along with over a dozen other devices.  I changed to
>> address-cells = <1> and dropped the leading zero from all cpu* reg<> and booting
>> the system I'm observing no change.  Looking in drivers/of I'm not seeing where
>> cpu*/reg is read and used, any recommendation?
>
> drivers/of/cpu.c
>
> Looks like there are lots of DTS files that use #address-cells = <2> for
> CPU nodes :-(
>
>     git grep -w -A1 cpus -- "*dts*" | grep address-cells | grep "<2>"
>
> I would use <1> is the first cell is always zero...

I'll do that.  Tha variation across DTS is likely coming from ~5.10 devicetree/bindings/arm/cpus.txt

        - #address-cells
...
                        # On ARM v8 64-bit systems value should be set to 2,
                          that corresponds to the MPIDR_EL1 register size.
                          If MPIDR_EL1[63:32] value is equal to 0 on all CPUs
                          in the system, #address-cells can be set to 1, since
                          MPIDR_EL1[63:32] bits are not used for CPUs
                          identification.

where the size of MPIDR_EL1 register is 2 for Elba cores.  However the shorthand is
allowed if MPIDR_EL1[63:32] bita are not used.

Latest version:

      On ARM v8 64-bit systems this property is required
        and matches the MPIDR_EL1 register affinity bits.

        * If cpus node's #address-cells property is set to 2

          The first reg cell bits [7:0] must be set to
          bits [39:32] of MPIDR_EL1.

          The second reg cell bits [23:0] must be set to
          bits [23:0] of MPIDR_EL1.

        * If cpus node's #address-cells property is set to 1

          The reg cell bits [23:0] must be set to bits [23:0]
          of MPIDR_EL1.

      All other bits in the reg cells must be set to 0.

Regards,
Brad

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller
  2023-05-24 12:41       ` Arnd Bergmann
@ 2023-08-07 22:17         ` Brad Larson
  0 siblings, 0 replies; 29+ messages in thread
From: Brad Larson @ 2023-08-07 22:17 UTC (permalink / raw)
  To: arnd
  Cc: adrian.hunter, alcooperx, andy.shevchenko, blarson,
	brendan.higgins, briannorris, broonie, catalin.marinas, conor+dt,
	davidgow, devicetree, fancer.lancer, gerg, gsomlo, hal.feng,
	hasegawa-hitomi, j.neuschaefer, joel, kernel, krzk,
	krzysztof.kozlowski+dt, lee.jones, lee, linux-arm-kernel,
	linux-kernel, linux-mmc, linux-spi, p.zabel, rdunlap, robh+dt,
	samuel, skhan, suravee.suthikulpanit, thomas.lendacky,
	tonyhuang.sunplus, ulf.hansson, vaishnav.a, walker.chen, will,
	zhuyinbo

Hi Arnd,

> On Wed, May 24, 2023, at 00:11, Brad Larson wrote:
>>> On Mon, May 15, 2023, at 20:16, Brad Larson wrote:
...
>>> Also, can you explain why this needs a low-lever user interface
>>> in the first place, rather than something that can be expressed
>>> using high-level abstractions as you already do with the reset
>>> control?
>>>
>>> All of the above should be part of the changelog text to get a
>>> driver like this merged. I don't think we can get a quick
>>> solution here though, so maybe you can start by removing the
>>> ioctl side and having the rest of the driver in drivers/reset?

Might be best to pull the whole thing for now until an acceptable 
solution is reached.  The reset function is a recovery mechanisim rarely
used where the byte access to the different IP at the 4 chip-selects
is needed for a system to boot.

>> In the original patchset I added a pensando compatible to spidev and that
>> was nacked in review and reusing some random compatible that made it into 
>> spidev was just wrong.  Further it was recommended this should be a system 
>> specific driver and don't rely on a debug driver like spidev.  I changed 
>> over to a platform specific driver and at that time I also needed to include 
>> a reset controller (emmc reset only).  I put these in drivers/mfd and 
>> drivers/reset.  Review of the device tree for this approach went back and 
>> forth to _not_ have four child nodes on the spi device each with the same 
>> compatible. Decision was to squash the child nodes into the parent and put 
>> the reset-controller there also.  One driver and since its pensando
>> specific its currently in drivers/soc/amd.
>>
>> There are five different user processes and some utilities that access the 
>> functionality in the cpld/fpga.  You're correct, its passing messages that 
>> are specific to the IP accessed via chip-select.  No Elba system will boot 
>> without this driver providing ioctl access.

> Thank you for the detailed summary. Moving away from spidev and
> from mfd seems all reasonable here. I'm still a bit confused by
> why you have multiple chipselects here that are for different
> subdevices but ended with a single user interface for all of them,
> but that's not a big deal.

The goal is to isolate the the kernel from device and platform specific
changes.  All the IO to the spi connected CPLD/FPGA (design/cost dependent)
is a byte at a time or up to 16 bytes for internal flash mgmt.  Performance
is not an issue and spidev was sufficient.

Maybe this paints the right picture to zero in on a correct approach.
Internal and external IP can be present at CS1/CS2 depending on the design
where the CS0 board controller registers get additions over time in a
backward compatible manner.

Design 1: FPGA 
CS0: Board controller registers
CS1: Designware SPI to I2C to board peripherals
CS2: Lattice dual I2C master
CS3: Internal storage

Design 2: CPLD
CS0: Board controller registers
CS1: Not used or some other board specific registers
CS2: Lattice dual I2C master
CS3: Internal storage

> The main bit that sticks out about this high-level design is how
> it relies on user space utilities at all to understand the message
> format. From what I understand about the actual functionality of
> this device, it most closely resembles an embedded controller that
> you might find in a laptop or server machine, and those usually
> have kernel drivers in drivers/platform/ to interact with the
> device.

The dozens of registers at CS0 for board management are defined in
userspace programs or script.  Only the regsiter offset/bit for
emmc reset is needed for the reset function in the patches.

> Has anyone tried to do it like that? Maybe it would help
> to see what the full protocol and the user space side looks
> like, in order to move some or all of it into the kernel.

Looking at drivers/platform its pretty sparse.  What do you 
recommend based on the design 1/2 variations?

Regards,
Brad

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2023-08-07 22:18 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-15 18:15 [PATCH v14 0/8] Support AMD Pensando Elba SoC Brad Larson
2023-05-15 18:15 ` [PATCH v14 1/8] dt-bindings: arm: add AMD Pensando boards Brad Larson
2023-05-15 18:16 ` [PATCH v14 2/8] dt-bindings: spi: cdns: Add compatible for AMD Pensando Elba SoC Brad Larson
2023-05-16 15:18   ` Mark Brown
2023-05-15 18:16 ` [PATCH v14 3/8] dt-bindings: soc: amd: amd,pensando-elba-ctrl: Add Pensando SoC Controller Brad Larson
2023-05-15 18:16 ` [PATCH v14 4/8] MAINTAINERS: Add entry for AMD PENSANDO Brad Larson
2023-05-15 18:16 ` [PATCH v14 5/8] arm64: Add config for AMD Pensando SoC platforms Brad Larson
2023-05-15 18:16 ` [PATCH v14 6/8] arm64: dts: Add AMD Pensando Elba SoC support Brad Larson
2023-05-16  7:17   ` Krzysztof Kozlowski
2023-05-16  7:54   ` Michal Simek
2023-05-23 19:28     ` Brad Larson
2023-05-24 11:52       ` Geert Uytterhoeven
2023-05-30 22:03         ` Brad Larson
2023-05-31 13:09           ` Geert Uytterhoeven
2023-06-05 23:52             ` Brad Larson
2023-05-15 18:16 ` [PATCH v14 7/8] spi: cadence-quadspi: Add compatible for AMD Pensando Elba SoC Brad Larson
2023-05-15 18:16 ` [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller Brad Larson
2023-05-15 21:05   ` Andy Shevchenko
2023-05-23  2:12     ` Brad Larson
2023-05-16  5:19   ` Mahapatra, Amit Kumar
2023-05-16  7:36     ` Michal Simek
2023-05-17 11:14       ` Geert Uytterhoeven
2023-05-16  8:45   ` kernel test robot
2023-05-16 11:03   ` Arnd Bergmann
2023-05-23 22:11     ` Brad Larson
2023-05-24 12:41       ` Arnd Bergmann
2023-08-07 22:17         ` Brad Larson
2023-05-16  7:14 ` [PATCH v14 0/8] Support AMD Pensando Elba SoC Krzysztof Kozlowski
2023-05-17 14:43 ` (subset) " Mark Brown

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).