[PATCH 0/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[PATCH 0/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[Kamal Dasu]

This patch adds support to claim host for TZOS RPMB access and synchronized
access to the controller hardware using hwspinlock framework.
To achieve this Linux OS and the secure TZOS make use of:
- shared hardware semaphore register
- a set of SDIO shared work registers and
- IPI interrupt registers 

The currently running OS that needs access to the controller puts itself in its
slot of work register and if its next in line it can try to grab the hardware
semaphore and complete its mmc requests. The shared work registers indicate next
in queue to access the controller, and current agent in the queue. Next agent
queue state is changed under the hwspinlock is owned by the current OS accessing
the controller hardware before release the semaphore, send and receive IPI
interrupts between linux and secure world are used to indicate completion of
transaction to the waiting OS. TZOS has its own RPMB driver which accesses
partition when it wants to read/write RPMB frames. Current implementation
assumes Linux and TZOS as the two work agents.

This patchset is an alternative method to initial RFC patch:
[PATCH RFC 0/3] mmc: sdhci-brcmstb: Add rpmb sharing support	
https://lkml.org/lkml/2025/2/6/1711

Kamal Dasu (4):
  dt-bindings: brcmstb-hwspinlock: support for hwspinlock
  hwspinlock: brcmstb hardware semaphore support
  dt-bindings: mmc: add brcmstb share register and hwlocks reference
  mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

 .../hwlock/brcm,brcmstb-hwspinlock.yaml       |  36 +++
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      |  29 ++-
 drivers/hwspinlock/Kconfig                    |   9 +
 drivers/hwspinlock/Makefile                   |   1 +
 drivers/hwspinlock/brcmstb_hwspinlock.c       |  98 +++++++
 drivers/mmc/host/sdhci-brcmstb.c              | 243 +++++++++++++++++-
 6 files changed, 413 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
 create mode 100644 drivers/hwspinlock/brcmstb_hwspinlock.c

-- 
2.34.1

[PATCH 0/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[Kamal Dasu]

This patch adds support to claim host for TZOS RPMB access and synchronized
access to the controller hardware using hwspinlock framework.
To achieve this Linux OS and the secure TZOS make use of:
- shared hardware semaphore register
- a set of SDIO shared work registers and
- IPI interrupt registers 

The currently running OS that needs access to the controller puts itself in its
slot of work register and if its next in line it can try to grab the hardware
semaphore and complete its mmc requests. The shared work registers indicate next
in queue to access the controller, and current agent in the queue. Next agent
queue state is changed under the hwspinlock is owned by the current OS accessing
the controller hardware before release the semaphore, send and receive IPI
interrupts between linux and secure world are used to indicate completion of
transaction to the waiting OS. TZOS has its own RPMB driver which accesses
partition when it wants to read/write RPMB frames. Current implementation
assumes Linux and TZOS as the two work agents.

Kamal Dasu (4):
  dt-bindings: brcmstb-hwspinlock: support for hwspinlock
  hwspinlock: brcmstb hardware semaphore support
  dt-bindings: mmc: add brcmstb share register and hwlocks reference
  mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

 .../hwlock/brcm,brcmstb-hwspinlock.yaml       |  36 +++
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      |  29 ++-
 drivers/hwspinlock/Kconfig                    |   9 +
 drivers/hwspinlock/Makefile                   |   1 +
 drivers/hwspinlock/brcmstb_hwspinlock.c       |  98 +++++++
 drivers/mmc/host/sdhci-brcmstb.c              | 243 +++++++++++++++++-
 6 files changed, 413 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
 create mode 100644 drivers/hwspinlock/brcmstb_hwspinlock.c

-- 
2.34.1

[PATCH 1/4] dt-bindings: brcmstb-hwspinlock: support for hwspinlock

[Kamal Dasu]

From: Kamal Dasu <kdasu@broadcom.com>

Adding brcmstb_hwspinlock bindings.

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
 .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml

diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
new file mode 100644
index 000000000000..b49ead166b1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom settop Hardware Spinlock
+
+maintainers:
+  - Kamal Dasu <kamal.dasu@broadcom.com>
+
+properties:
+  "#hwlock-cells":
+    const: 1
+
+  compatible:
+    const: brcm,brcmstb-hwspinlock
+
+  reg:
+    maxItems: 1
+
+required:
+  - "#hwlock-cells"
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    hwspinlock@8404038 {
+        compatible = "brcm,brcmstb-hwspinlock";
+        #hwlock-cells = <1>;
+        reg = <0x8404038 0x40>;
+    };
+
-- 
2.34.1

Re: [PATCH 1/4] dt-bindings: brcmstb-hwspinlock: support for hwspinlock

[Krzysztof Kozlowski]

On 11/07/2025 17:42, Kamal Dasu wrote:
> From: Kamal Dasu <kdasu@broadcom.com>
> 
> Adding brcmstb_hwspinlock bindings.
> 
> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

missing hwlock prefix

> ---
>  .../hwlock/brcm,brcmstb-hwspinlock.yaml       | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> new file mode 100644
> index 000000000000..b49ead166b1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwlock/brcm,brcmstb-hwspinlock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom settop Hardware Spinlock
> +
> +maintainers:
> +  - Kamal Dasu <kamal.dasu@broadcom.com>
> +
> +properties:
> +  "#hwlock-cells":
> +    const: 1
> +
> +  compatible:
> +    const: brcm,brcmstb-hwspinlock

Please use SoC specific compatibles. Then also filename will change.

Compatible always goes first. See 99% of other bindings and DTS coding
style.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - "#hwlock-cells"
> +  - compatible

Same here

> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hwspinlock@8404038 {

hwlock@

> +        compatible = "brcm,brcmstb-hwspinlock";

And this you got right...

> +        #hwlock-cells = <1>;
> +        reg = <0x8404038 0x40>;

But this not. Use order from DTS codinh style.

> +    };
> +


Best regards,
Krzysztof

[PATCH 2/4] hwspinlock: brcmstb hardware semaphore support

[Kamal Dasu]

From: Kamal Dasu <kdasu@broadcom.com>

Added support for brmstb_hwspinlock driver that makes use of
the hwspinlock framework. Driver uses SUN_TOP_CTRL_SEMAPHORE_[1:15]
registers to implement the hardware semaphore. With this change
other brcmstb drivers can use hwspin_trylock() and hwspin_unlock()
apis and make use of this hwspinlock framework. Other driver dt nodes
just need to use a reference to the &hwspinlock and the lock id
they want to use.
e.g. hwlocks = <&hwspinlock0 0>;

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
 drivers/hwspinlock/Kconfig              |  9 +++
 drivers/hwspinlock/Makefile             |  1 +
 drivers/hwspinlock/brcmstb_hwspinlock.c | 98 +++++++++++++++++++++++++
 3 files changed, 108 insertions(+)
 create mode 100644 drivers/hwspinlock/brcmstb_hwspinlock.c

diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 3874d15b0e9b..551afa8df2d0 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -63,4 +63,13 @@ config HSEM_U8500
 
 	  If unsure, say N.
 
+config HWSPINLOCK_BRCMSTB
+	tristate "Broadcom Setttop Hardware Semaphore functionality"
+	depends on ARCH_BRCMSTB || COMPILE_TEST
+	help
+	  Broadcom settop hwspinlock driver.
+	  Say y here to support the Broadcom Hardware Semaphore functionality, which
+	  provides a synchronisation mechanism on the SoC.
+
+	  If unsure, say N.
 endif # HWSPINLOCK
diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
index a0f16c9aaa82..4f5c05403209 100644
--- a/drivers/hwspinlock/Makefile
+++ b/drivers/hwspinlock/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_HWSPINLOCK_SPRD)		+= sprd_hwspinlock.o
 obj-$(CONFIG_HWSPINLOCK_STM32)		+= stm32_hwspinlock.o
 obj-$(CONFIG_HWSPINLOCK_SUN6I)		+= sun6i_hwspinlock.o
 obj-$(CONFIG_HSEM_U8500)		+= u8500_hsem.o
+obj-$(CONFIG_HWSPINLOCK_BRCMSTB)	+= brcmstb_hwspinlock.o
diff --git a/drivers/hwspinlock/brcmstb_hwspinlock.c b/drivers/hwspinlock/brcmstb_hwspinlock.c
new file mode 100644
index 000000000000..c27d53e06edf
--- /dev/null
+++ b/drivers/hwspinlock/brcmstb_hwspinlock.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * brcmstb HWSEM driver
+ *
+ * Copyright (C) 2025 Broadcom
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/hwspinlock.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+#include "hwspinlock_internal.h"
+
+#define BRCMSTB_MAX_SEMAPHORES		16
+#define RESET_SEMAPHORE			0
+
+#define HWSPINLOCK_VAL			'L'
+
+static int brcmstb_hwspinlock_trylock(struct hwspinlock *lock)
+{
+	void __iomem *lock_addr = lock->priv;
+
+	writel(HWSPINLOCK_VAL, lock_addr);
+
+	return (readl(lock_addr) == HWSPINLOCK_VAL);
+}
+
+static void brcmstb_hwspinlock_unlock(struct hwspinlock *lock)
+{
+	void __iomem *lock_addr = lock->priv;
+
+	/* release the lock by writing 0 to it */
+	writel(RESET_SEMAPHORE, lock_addr);
+}
+
+static void brcmstb_hwspinlock_relax(struct hwspinlock *lock)
+{
+	ndelay(50);
+}
+
+static const struct hwspinlock_ops brcmstb_hwspinlock_ops = {
+	.trylock	= brcmstb_hwspinlock_trylock,
+	.unlock		= brcmstb_hwspinlock_unlock,
+	.relax		= brcmstb_hwspinlock_relax,
+};
+
+static int brcmstb_hwspinlock_probe(struct platform_device *pdev)
+{
+	struct hwspinlock_device *bank;
+	struct hwspinlock *hwlock;
+	void __iomem *io_base;
+	int i, num_locks = BRCMSTB_MAX_SEMAPHORES;
+
+	io_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(io_base)) {
+		dev_err(&pdev->dev, "semaphore iobase mapping error\n");
+		return PTR_ERR(io_base);
+	}
+
+	bank = devm_kzalloc(&pdev->dev, struct_size(bank, lock, num_locks),
+			    GFP_KERNEL);
+	if (!bank)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, bank);
+
+	for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++)
+		hwlock->priv = io_base + sizeof(u32) * i;
+
+	return devm_hwspin_lock_register(&pdev->dev, bank,
+					 &brcmstb_hwspinlock_ops,
+					 0, num_locks);
+}
+
+static const struct of_device_id brcmstb_hwspinlock_ids[] = {
+	{ .compatible = "brcm,brcmstb-hwspinlock", },
+	{ /* end */ },
+};
+MODULE_DEVICE_TABLE(of, brcmstb_hwspinlock_ids);
+
+static struct platform_driver brcmstb_hwspinlock_driver = {
+	.probe		= brcmstb_hwspinlock_probe,
+	.driver		= {
+		.name	= "brcmstb_hwspinlock",
+		.of_match_table = brcmstb_hwspinlock_ids,
+	},
+};
+
+module_platform_driver(brcmstb_hwspinlock_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Hardware Spinlock driver for brcmstb");
+MODULE_AUTHOR("Kamal Dasu <kdasu@broadcom.com>");
-- 
2.34.1

Re: [PATCH 2/4] hwspinlock: brcmstb hardware semaphore support

[kernel test robot]

Hi Kamal,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.16-rc5 next-20250711]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kamal-Dasu/dt-bindings-brcmstb-hwspinlock-support-for-hwspinlock/20250712-034624
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250711154221.928164-4-kamal.dasu%40broadcom.com
patch subject: [PATCH 2/4] hwspinlock: brcmstb hardware semaphore support
config: arc-randconfig-r111-20250713 (https://download.01.org/0day-ci/archive/20250713/202507131422.6eIuyxUJ-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 12.4.0
reproduce: (https://download.01.org/0day-ci/archive/20250713/202507131422.6eIuyxUJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507131422.6eIuyxUJ-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/hwspinlock/brcmstb_hwspinlock.c:26:39: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void [noderef] __iomem *lock_addr @@     got void *priv @@
   drivers/hwspinlock/brcmstb_hwspinlock.c:26:39: sparse:     expected void [noderef] __iomem *lock_addr
   drivers/hwspinlock/brcmstb_hwspinlock.c:26:39: sparse:     got void *priv
   drivers/hwspinlock/brcmstb_hwspinlock.c:35:39: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void [noderef] __iomem *lock_addr @@     got void *priv @@
   drivers/hwspinlock/brcmstb_hwspinlock.c:35:39: sparse:     expected void [noderef] __iomem *lock_addr
   drivers/hwspinlock/brcmstb_hwspinlock.c:35:39: sparse:     got void *priv
>> drivers/hwspinlock/brcmstb_hwspinlock.c:73:30: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *priv @@     got void [noderef] __iomem * @@
   drivers/hwspinlock/brcmstb_hwspinlock.c:73:30: sparse:     expected void *priv
   drivers/hwspinlock/brcmstb_hwspinlock.c:73:30: sparse:     got void [noderef] __iomem *

vim +26 drivers/hwspinlock/brcmstb_hwspinlock.c

    23	
    24	static int brcmstb_hwspinlock_trylock(struct hwspinlock *lock)
    25	{
  > 26		void __iomem *lock_addr = lock->priv;
    27	
    28		writel(HWSPINLOCK_VAL, lock_addr);
    29	
    30		return (readl(lock_addr) == HWSPINLOCK_VAL);
    31	}
    32	
    33	static void brcmstb_hwspinlock_unlock(struct hwspinlock *lock)
    34	{
    35		void __iomem *lock_addr = lock->priv;
    36	
    37		/* release the lock by writing 0 to it */
    38		writel(RESET_SEMAPHORE, lock_addr);
    39	}
    40	
    41	static void brcmstb_hwspinlock_relax(struct hwspinlock *lock)
    42	{
    43		ndelay(50);
    44	}
    45	
    46	static const struct hwspinlock_ops brcmstb_hwspinlock_ops = {
    47		.trylock	= brcmstb_hwspinlock_trylock,
    48		.unlock		= brcmstb_hwspinlock_unlock,
    49		.relax		= brcmstb_hwspinlock_relax,
    50	};
    51	
    52	static int brcmstb_hwspinlock_probe(struct platform_device *pdev)
    53	{
    54		struct hwspinlock_device *bank;
    55		struct hwspinlock *hwlock;
    56		void __iomem *io_base;
    57		int i, num_locks = BRCMSTB_MAX_SEMAPHORES;
    58	
    59		io_base = devm_platform_ioremap_resource(pdev, 0);
    60		if (IS_ERR(io_base)) {
    61			dev_err(&pdev->dev, "semaphore iobase mapping error\n");
    62			return PTR_ERR(io_base);
    63		}
    64	
    65		bank = devm_kzalloc(&pdev->dev, struct_size(bank, lock, num_locks),
    66				    GFP_KERNEL);
    67		if (!bank)
    68			return -ENOMEM;
    69	
    70		platform_set_drvdata(pdev, bank);
    71	
    72		for (i = 0, hwlock = &bank->lock[0]; i < num_locks; i++, hwlock++)
  > 73			hwlock->priv = io_base + sizeof(u32) * i;
    74	
    75		return devm_hwspin_lock_register(&pdev->dev, bank,
    76						 &brcmstb_hwspinlock_ops,
    77						 0, num_locks);
    78	}
    79	

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

[PATCH 3/4] dt-bindings: mmc: add brcmstb share register and hwlocks reference

[Kamal Dasu]

From: Kamal Dasu <kdasu@broadcom.com>

Adding optional controller share registers and hwspinlock reference fields
to be used by sdhci-brcmstb driver.

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 29 +++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
index eee6be7a7867..fe9be7a7eca5 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
+++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
@@ -27,15 +27,20 @@ properties:
           - const: brcm,sdhci-brcmstb
 
   reg:
-    maxItems: 2
+    minItems: 2
+    maxItems: 4
 
   reg-names:
+    minItems: 2
     items:
       - const: host
       - const: cfg
+      - const: share       # Optional reg
+      - const: flshr_ipis0 # Optional reg
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   clocks:
     minItems: 1
@@ -60,6 +65,9 @@ properties:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+  hwlocks:
+    maxItems: 1
+
 allOf:
   - $ref: mmc-controller.yaml#
   - if:
@@ -115,3 +123,20 @@ examples:
       clocks = <&scmi_clk 245>;
       clock-names = "sw_sdio";
     };
+  - |
+    mmc@84b1000 {
+      mmc-ddr-1_8v;
+      mmc-hs200-1_8v;
+      mmc-hs400-1_8v;
+      no-sd;
+      no-sdio;
+      non-removable;
+      bus-width = <0x8>;
+      compatible = "brcm,bcm74165b0-sdhci", "brcm,sdhci-brcmstb";
+      reg = <0x84b1000 0x260>, <0x84b1300 0x200>,  <0x84b1600 0x10>, <0x84a5404 0x4>;
+      reg-names = "host", "cfg", "share", "flshr_ipis0";
+      hwlocks = <&hw_lock 0x0>;
+      interrupts = <0x1 0x0 0x1f 0x4 0x1b 0x11>;
+      clocks = <&scmi_clk 245>;
+      clock-names = "sw_sdio";
+    };
-- 
2.34.1

Re: [PATCH 3/4] dt-bindings: mmc: add brcmstb share register and hwlocks reference

[Krzysztof Kozlowski]

On 11/07/2025 17:42, Kamal Dasu wrote:
> From: Kamal Dasu <kdasu@broadcom.com>
> 
> Adding optional controller share registers and hwspinlock reference fields
> to be used by sdhci-brcmstb driver.
> 
> Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
> ---
>  .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 29 +++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> index eee6be7a7867..fe9be7a7eca5 100644
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> @@ -27,15 +27,20 @@ properties:
>            - const: brcm,sdhci-brcmstb
>  
>    reg:
> -    maxItems: 2
> +    minItems: 2
> +    maxItems: 4
>  
>    reg-names:
> +    minItems: 2
>      items:
>        - const: host
>        - const: cfg
> +      - const: share       # Optional reg
> +      - const: flshr_ipis0 # Optional reg

Drop comments. Schema says these are optional, so no need to point obvious.

>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2

You need to list items with minItem: 1.

Why all devices now got new reg and interrupts? What changed in EXISTING
hardware? All this has to be explained in the commit msg. If not all
devices got it, then you miss constraints.

>  
>    clocks:
>      minItems: 1
> @@ -60,6 +65,9 @@ properties:
>      type: boolean
>      description: Specifies that controller should use auto CMD12
>  
> +  hwlocks:
> +    maxItems: 1
> +
>  allOf:
>    - $ref: mmc-controller.yaml#
>    - if:
> @@ -115,3 +123,20 @@ examples:
>        clocks = <&scmi_clk 245>;
>        clock-names = "sw_sdio";
>      };
> +  - |
> +    mmc@84b1000 {
> +      mmc-ddr-1_8v;
> +      mmc-hs200-1_8v;
> +      mmc-hs400-1_8v;
> +      no-sd;
> +      no-sdio;
> +      non-removable;
> +      bus-width = <0x8>;
> +      compatible = "brcm,bcm74165b0-sdhci", "brcm,sdhci-brcmstb";
> +      reg = <0x84b1000 0x260>, <0x84b1300 0x200>,  <0x84b1600 0x10>, <0x84a5404 0x4>;
> +      reg-names = "host", "cfg", "share", "flshr_ipis0";
> +      hwlocks = <&hw_lock 0x0>;
> +      interrupts = <0x1 0x0 0x1f 0x4 0x1b 0x11>;

Undecipherable.

> +      clocks = <&scmi_clk 245>;
> +      clock-names = "sw_sdio";

Totally random order of properties. Please align it closely with DTS
coding style.

> +    };


Best regards,
Krzysztof

[PATCH 4/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[Kamal Dasu]

From: Kamal Dasu <kdasu@broadcom.com>

Adding sdio rpmb partition sharing support in brcmstb host driver.
The sdhci-brcmstb controller driver uses SDIO_SHARE work registers
along with use of brcmstb hwspinlock framework to synchronize access
between linux and trusted zone firmware. The sdhci-brcmstb driver
claims and releases host for TZOS only when it is requested to do
via ipi0 interrupt.

Signed-off-by: Kamal Dasu <kamal.dasu@broadcom.com>
---
 drivers/mmc/host/sdhci-brcmstb.c | 243 ++++++++++++++++++++++++++++++-
 1 file changed, 242 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 48cdcba0f39c..dd68fd512459 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -12,6 +12,10 @@
 #include <linux/of.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>
+#include <linux/hwspinlock.h>
+#include <linux/workqueue.h>
+#include <linux/kthread.h>
+#include "../core/core.h"
 
 #include "sdhci-cqhci.h"
 #include "sdhci-pltfm.h"
@@ -34,6 +38,11 @@
 #define SDIO_CFG_CQ_CAPABILITY			0x4c
 #define SDIO_CFG_CQ_CAPABILITY_FMUL		GENMASK(13, 12)
 
+#define SDHCI_BRCMSTB_AGENT_LINUX		'L'
+#define SDHCI_BRCMSTB_AGENT_TZOS		'A'
+#define FLSHARE_IPIS0_INT_SEND_MASK		BIT(17)
+#define HWSPINLOCK_TIMEOUT_MS			100
+
 #define SDIO_CFG_CTRL				0x0
 #define SDIO_CFG_CTRL_SDCD_N_TEST_EN		BIT(31)
 #define SDIO_CFG_CTRL_SDCD_N_TEST_LEV		BIT(30)
@@ -46,9 +55,26 @@
 /* Select all SD UHS type I SDR speed above 50MB/s */
 #define MMC_CAP_UHS_I_SDR_MASK	(MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104)
 
+#define BRCMSTB_SD_SHARE_REG_NEXT		0x0 /* Next Agent Register */
+#define BRCMSTB_SD_SHARE_REG_PMC		0x4 /* Work Agent1 Register */
+#define BRCMSTB_SD_SHARE_REG_TZOS		0x8 /* Work Agent2 Register */
+#define BRCMSTB_SD_SHARE_REG_LINUX		0xc /* Work Agent3 Register */
+
+struct brcmstb_sdio_share_info {
+	void __iomem *share_reg;
+	void __iomem *ipis0_reg;
+	struct hwspinlock *hwlock;
+	struct sdhci_host *host;
+	int irq_recv;
+	int host_claimed;
+	wait_queue_head_t wq;
+	struct task_struct  *claim_thread;
+};
+
 struct sdhci_brcmstb_priv {
 	void __iomem *cfg_regs;
 	unsigned int flags;
+	struct brcmstb_sdio_share_info *si;
 	struct clk *base_clk;
 	u32 base_freq_hz;
 };
@@ -303,6 +329,218 @@ static const struct of_device_id __maybe_unused sdhci_brcm_of_match[] = {
 	{},
 };
 
+static void sdhci_brcmstb_dump_shr_regs(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	struct brcmstb_sdio_share_info *si = priv->si;
+
+	dev_dbg(mmc_dev(host->mmc), "wn:0x%x wa:0x%x wl:0x%x\n",
+		readl(si->share_reg + BRCMSTB_SD_SHARE_REG_NEXT),
+		readl(si->share_reg + BRCMSTB_SD_SHARE_REG_TZOS),
+		readl(si->share_reg + BRCMSTB_SD_SHARE_REG_LINUX));
+}
+
+static bool sdhci_brcmstb_linux_host_is_next(struct brcmstb_sdio_share_info *si)
+{
+	u32 wn;
+
+	/* check if linux is next */
+	wn = readl(si->share_reg + BRCMSTB_SD_SHARE_REG_NEXT);
+	return ((wn == SDHCI_BRCMSTB_AGENT_LINUX) ? true : false);
+}
+
+static void sdhci_brcmstb_wait_for_linux_host(struct brcmstb_sdio_share_info *si)
+{
+	int ret;
+
+	while (1) {
+		/* put self in wait queue when host not available  */
+		ret = wait_event_interruptible(si->wq,
+				       sdhci_brcmstb_linux_host_is_next(si));
+		if (ret != -ERESTARTSYS)
+			break;
+	};
+}
+
+static bool sdhci_brcmstb_tzos_is_waiting(struct brcmstb_sdio_share_info *si)
+{
+	u32 wt;
+
+	/* check if TZOS has put itself in the work queue  */
+	wt = readl(si->share_reg + BRCMSTB_SD_SHARE_REG_TZOS);
+	return ((wt == SDHCI_BRCMSTB_AGENT_TZOS) ? true : false);
+}
+
+static void sdhci_brcmstb_wait_for_tzos(struct brcmstb_sdio_share_info *si)
+{
+	int ret;
+
+	while (1) {
+		/* wait in queue when tzos cannot use controller */
+		ret = wait_event_interruptible(si->wq,
+				   sdhci_brcmstb_tzos_is_waiting(si));
+		if (ret != -ERESTARTSYS)
+			break;
+	}
+}
+
+static void sdhci_brcmstb_aquire_hwsem(struct brcmstb_sdio_share_info *si)
+{
+	u32 wl = SDHCI_BRCMSTB_AGENT_LINUX;
+	struct mmc_host *mmc = si->host->mmc;
+	int ret;
+
+	/*
+	 * aquire hw sem :
+	 * 1. write linux agent id to work register WL
+	 * 2. Aquire hw semaphore
+	 * 2. clear next work register WN
+	 */
+	writel(wl, si->share_reg + BRCMSTB_SD_SHARE_REG_LINUX);
+	/*  try hw semaphore lock, we should never have to wait here */
+	ret =  hwspin_lock_timeout(si->hwlock, HWSPINLOCK_TIMEOUT_MS);
+	WARN_ON(ret != 0);
+	/* clear next register when holding the semaphore */
+	writel(0, si->share_reg + BRCMSTB_SD_SHARE_REG_NEXT);
+	dev_dbg(mmc_dev(mmc), "hwsem aquire\n");
+	sdhci_brcmstb_dump_shr_regs(si->host);
+}
+
+static void sdhci_brcmstb_release_hwsem(struct brcmstb_sdio_share_info *si)
+{
+	u32 wt;
+	struct mmc_host *mmc = si->host->mmc;
+	int ret;
+
+	/*
+	 * release hw semphore
+	 * 1. set the next work agent register WN before releasing hw sem
+	 * 2. Release hw semaphore
+	 * 3. send ipi to TZOS
+	 */
+	wt = readl(si->share_reg + BRCMSTB_SD_SHARE_REG_TZOS);
+	writel(wt, si->share_reg + BRCMSTB_SD_SHARE_REG_NEXT);
+
+	/* release hw semaphore if we hold it and send IPI */
+	ret =  hwspin_trylock_raw(si->hwlock);
+	WARN_ON(ret != 0);
+	hwspin_unlock(si->hwlock);
+
+	if (wt == SDHCI_BRCMSTB_AGENT_TZOS)
+		writel(FLSHARE_IPIS0_INT_SEND_MASK, si->ipis0_reg);
+
+	dev_dbg(mmc_dev(mmc), "hwsem release\n");
+	sdhci_brcmstb_dump_shr_regs(si->host);
+}
+
+static irqreturn_t sdhci_brcmstb_recv_ipi0_irq(int irq, void *dev_id)
+{
+	struct sdhci_host *host = dev_id;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	struct brcmstb_sdio_share_info *si = priv->si;
+
+	dev_dbg(mmc_dev(host->mmc), "ipi irq %d next L:%d T:%d ch:%d\n",
+		 irq, sdhci_brcmstb_linux_host_is_next(si),
+		 sdhci_brcmstb_tzos_is_waiting(si),
+		 si->host_claimed);
+	wake_up_interruptible(&si->wq);
+	return IRQ_HANDLED;
+}
+
+static int sdhci_brcmstb_host_claim_thread(void *data)
+{
+	struct brcmstb_sdio_share_info *si = data;
+	struct mmc_host *mmc = si->host->mmc;
+
+	do {
+		sdhci_brcmstb_wait_for_tzos(si);
+		/* claim host for TZOS */
+		mmc_claim_host(mmc);
+		si->host_claimed += 1;
+		sdhci_brcmstb_release_hwsem(si);
+		dev_dbg(mmc_dev(mmc), "host claimed %d\n", si->host_claimed);
+		sdhci_brcmstb_wait_for_linux_host(si);
+		sdhci_brcmstb_aquire_hwsem(si);
+		/* release host */
+		mmc_release_host(mmc);
+		si->host_claimed -= 1;
+		dev_dbg(mmc_dev(mmc), "host released %d\n", si->host_claimed);
+	} while (!kthread_should_stop());
+
+	return 0;
+}
+
+static int sdhci_brcmstb_sdio_share_init(struct platform_device *pdev)
+{
+	struct sdhci_host *host = dev_get_drvdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
+	struct brcmstb_sdio_share_info *si;
+	void __iomem *sdio_sh_regs;
+	int ret;
+
+	/* sdio_share block */
+	sdio_sh_regs = devm_platform_ioremap_resource_byname(pdev, "share");
+	if (IS_ERR(sdio_sh_regs))
+		return 0;
+
+	si = devm_kcalloc(&pdev->dev, 1, sizeof(struct brcmstb_sdio_share_info),
+			  GFP_KERNEL);
+	if (!si)
+		return -ENOMEM;
+
+	si->share_reg = sdio_sh_regs;
+	ret = of_hwspin_lock_get_id(np, 0);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get hwspinlock id %d\n", ret);
+		return ret;
+	}
+
+	si->hwlock = devm_hwspin_lock_request_specific(&pdev->dev, ret);
+	if (!si->hwlock) {
+		dev_err(&pdev->dev, "failed to request hwspinlock\n");
+		return -ENXIO;
+	}
+
+	si->irq_recv = platform_get_irq_byname_optional(pdev, "recv_ipi0");
+	if (si->irq_recv < 0) {
+		ret = si->irq_recv;
+		dev_err(&pdev->dev, "recv_ipi0 IRQ not found\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(&pdev->dev, si->irq_recv,
+			       sdhci_brcmstb_recv_ipi0_irq,
+			       0, "mmc_recv_ipi0", host);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "mmc_recv_ipi0 IRQ request_irq failed\n");
+		return ret;
+	}
+
+	si->ipis0_reg = devm_platform_ioremap_resource_byname(pdev, "flshr_ipis0");
+	if (IS_ERR(si->ipis0_reg))
+		return -ENXIO;
+
+	priv->si = si;
+	si->host = host;
+	init_waitqueue_head(&si->wq);
+	/* acquire hwsem */
+	sdhci_brcmstb_aquire_hwsem(si);
+	si->claim_thread =
+		kthread_run(sdhci_brcmstb_host_claim_thread, si,
+			    "ksdshrthread/%s", mmc_hostname(host->mmc));
+	if (IS_ERR(si->claim_thread)) {
+		ret = PTR_ERR(si->claim_thread);
+		dev_err(&pdev->dev, "failed to run claim thread\n");
+		return -ENOEXEC;
+	}
+
+	return 0;
+}
+
 static u32 sdhci_brcmstb_cqhci_irq(struct sdhci_host *host, u32 intmask)
 {
 	int cmd_error = 0;
@@ -482,8 +720,11 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 		goto err;
 
 	pltfm_host->clk = clk;
-	return res;
+	res  = sdhci_brcmstb_sdio_share_init(pdev);
+	if (res)
+		dev_warn(&pdev->dev, "sdio share unavailable\n");
 
+	return 0;
 err:
 	sdhci_pltfm_free(pdev);
 	clk_disable_unprepare(base_clk);
-- 
2.34.1

Re: [PATCH 4/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[Krzysztof Kozlowski]

On 11/07/2025 17:42, Kamal Dasu wrote:
> +
> +static int sdhci_brcmstb_sdio_share_init(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = dev_get_drvdata(&pdev->dev);
> +	struct device_node *np = pdev->dev.of_node;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +	struct brcmstb_sdio_share_info *si;
> +	void __iomem *sdio_sh_regs;
> +	int ret;
> +
> +	/* sdio_share block */
> +	sdio_sh_regs = devm_platform_ioremap_resource_byname(pdev, "share");
> +	if (IS_ERR(sdio_sh_regs))
> +		return 0;
> +
> +	si = devm_kcalloc(&pdev->dev, 1, sizeof(struct brcmstb_sdio_share_info),

sizeof(*)

> +			  GFP_KERNEL);
> +	if (!si)
> +		return -ENOMEM;
> +
> +	si->share_reg = sdio_sh_regs;
> +	ret = of_hwspin_lock_get_id(np, 0);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get hwspinlock id %d\n", ret);

Uh? So you changed the ABI in the driver but not in the bindings? No,
this must be always synced. Look what your binding is saying. Is it
optional? Yes.

And why would you print errors anyway on deferred probe? Twice! One here
and warning in your probe.

> +		return ret;
> +	}
> +
> +	si->hwlock = devm_hwspin_lock_request_specific(&pdev->dev, ret);
> +	if (!si->hwlock) {
> +		dev_err(&pdev->dev, "failed to request hwspinlock\n");

Syntax is: return dev_err_probe

> +		return -ENXIO;
> +	}
> +
> +	si->irq_recv = platform_get_irq_byname_optional(pdev, "recv_ipi0");
> +	if (si->irq_recv < 0) {
> +		ret = si->irq_recv;
> +		dev_err(&pdev->dev, "recv_ipi0 IRQ not found\n");

Syntax is: return dev_err_probe

> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, si->irq_recv,
> +			       sdhci_brcmstb_recv_ipi0_irq,
> +			       0, "mmc_recv_ipi0", host);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "mmc_recv_ipi0 IRQ request_irq failed\n");
> +		return ret;

return dev_err_probe

> +	}
> +
> +	si->ipis0_reg = devm_platform_ioremap_resource_byname(pdev, "flshr_ipis0");
> +	if (IS_ERR(si->ipis0_reg))
> +		return -ENXIO;
> +
> +	priv->si = si;
> +	si->host = host;
> +	init_waitqueue_head(&si->wq);
> +	/* acquire hwsem */
> +	sdhci_brcmstb_aquire_hwsem(si);
> +	si->claim_thread =
> +		kthread_run(sdhci_brcmstb_host_claim_thread, si,
> +			    "ksdshrthread/%s", mmc_hostname(host->mmc));
> +	if (IS_ERR(si->claim_thread)) {
> +		ret = PTR_ERR(si->claim_thread);
> +		dev_err(&pdev->dev, "failed to run claim thread\n");
> +		return -ENOEXEC;
> +	}
> +
> +	return 0;
> +}
> +
>  static u32 sdhci_brcmstb_cqhci_irq(struct sdhci_host *host, u32 intmask)
>  {
>  	int cmd_error = 0;
> @@ -482,8 +720,11 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>  		goto err;
>  
>  	pltfm_host->clk = clk;
> -	return res;
> +	res  = sdhci_brcmstb_sdio_share_init(pdev);

Only single space before '='

> +	if (res)
> +		dev_warn(&pdev->dev, "sdio share unavailable\n");

Why do you warn on completely optional hwlock? Drop, this should be
silent. You already print errors earlier and there is no point to print
error twice.

>  
> +	return 0;
>  err:
>  	sdhci_pltfm_free(pdev);
>  	clk_disable_unprepare(base_clk);


Best regards,
Krzysztof

Re: [PATCH 0/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[Adrian Hunter]

On 11/07/2025 18:42, Kamal Dasu wrote:
> This patch adds support to claim host for TZOS RPMB access and synchronized
> access to the controller hardware using hwspinlock framework.
> To achieve this Linux OS and the secure TZOS make use of:
> - shared hardware semaphore register
> - a set of SDIO shared work registers and
> - IPI interrupt registers 
> 
> The currently running OS that needs access to the controller puts itself in its
> slot of work register and if its next in line it can try to grab the hardware
> semaphore and complete its mmc requests. The shared work registers indicate next
> in queue to access the controller, and current agent in the queue. Next agent
> queue state is changed under the hwspinlock is owned by the current OS accessing
> the controller hardware before release the semaphore, send and receive IPI
> interrupts between linux and secure world are used to indicate completion of
> transaction to the waiting OS. TZOS has its own RPMB driver which accesses
> partition when it wants to read/write RPMB frames. Current implementation
> assumes Linux and TZOS as the two work agents.
> 
> This patchset is an alternative method to initial RFC patch:
> [PATCH RFC 0/3] mmc: sdhci-brcmstb: Add rpmb sharing support	
> https://lkml.org/lkml/2025/2/6/1711

Does it address Ulf's comments? i.e.

	https://lore.kernel.org/all/CAPDyKFrCjo8gGnxmXWP6V39N+b1o62VQH9zwMUNb2_+D3-qrdw@mail.gmail.com/

> 
> Kamal Dasu (4):
>   dt-bindings: brcmstb-hwspinlock: support for hwspinlock
>   hwspinlock: brcmstb hardware semaphore support
>   dt-bindings: mmc: add brcmstb share register and hwlocks reference
>   mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS
> 
>  .../hwlock/brcm,brcmstb-hwspinlock.yaml       |  36 +++
>  .../bindings/mmc/brcm,sdhci-brcmstb.yaml      |  29 ++-
>  drivers/hwspinlock/Kconfig                    |   9 +
>  drivers/hwspinlock/Makefile                   |   1 +
>  drivers/hwspinlock/brcmstb_hwspinlock.c       |  98 +++++++
>  drivers/mmc/host/sdhci-brcmstb.c              | 243 +++++++++++++++++-
>  6 files changed, 413 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwlock/brcm,brcmstb-hwspinlock.yaml
>  create mode 100644 drivers/hwspinlock/brcmstb_hwspinlock.c
> 

Re: [PATCH 0/4] mmc: sdhci-brcmstb: rpmb sharing by claiming host for TZOS

[Florian Fainelli]

On 7/28/25 02:11, Adrian Hunter wrote:
> On 11/07/2025 18:42, Kamal Dasu wrote:
>> This patch adds support to claim host for TZOS RPMB access and synchronized
>> access to the controller hardware using hwspinlock framework.
>> To achieve this Linux OS and the secure TZOS make use of:
>> - shared hardware semaphore register
>> - a set of SDIO shared work registers and
>> - IPI interrupt registers
>>
>> The currently running OS that needs access to the controller puts itself in its
>> slot of work register and if its next in line it can try to grab the hardware
>> semaphore and complete its mmc requests. The shared work registers indicate next
>> in queue to access the controller, and current agent in the queue. Next agent
>> queue state is changed under the hwspinlock is owned by the current OS accessing
>> the controller hardware before release the semaphore, send and receive IPI
>> interrupts between linux and secure world are used to indicate completion of
>> transaction to the waiting OS. TZOS has its own RPMB driver which accesses
>> partition when it wants to read/write RPMB frames. Current implementation
>> assumes Linux and TZOS as the two work agents.
>>
>> This patchset is an alternative method to initial RFC patch:
>> [PATCH RFC 0/3] mmc: sdhci-brcmstb: Add rpmb sharing support	
>> https://lkml.org/lkml/2025/2/6/1711
> 
> Does it address Ulf's comments? i.e.
> 
> 	https://lore.kernel.org/all/CAPDyKFrCjo8gGnxmXWP6V39N+b1o62VQH9zwMUNb2_+D3-qrdw@mail.gmail.com/

If you are referring to the fact that Linux is the exclusive owner of 
the eMMC controller, then no, that is still not what is being done here, 
but we did respond as to why this is done that way, and there was no 
subsequent question or comments on my respond to Ulf:

https://lore.kernel.org/all/6328fe8d-c4ea-4945-b6ba-d994403121b5@broadcom.com/
-- 
Florian