public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] mailbox: add samsung exynos driver
@ 2024-12-05 17:41 Tudor Ambarus
  2024-12-05 17:41 ` [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos Tudor Ambarus
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tudor Ambarus @ 2024-12-05 17:41 UTC (permalink / raw)
  To: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar
  Cc: linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin,
	Tudor Ambarus

Hi,

The samsung exynos mailbox controller has 16 flag bits for hardware
interrupt generation and a shared register for passing mailbox messages.
When the controller is used by the ACPM protocol the shared register is
ignored and the mailbox controller acts as a doorbell. The controller
just raises the interrupt to APM after the ACPM protocol has written
the message to SRAM.

I mark this as v3 because it is a continuation of:
https://lore.kernel.org/linux-arm-kernel/20241017163649.3007062-1-tudor.ambarus@linaro.org/

Changes in v3:
- decouple the mailbox controller driver from the ACPM protocol driver
- address Krzysztof's eview comments 

Thanks,
ta

Tudor Ambarus (3):
  dt-bindings: mailbox: add bindings for samsung,exynos
  mailbox: add samsung exynos driver
  MAINTAINERS: add entry for samsung exynos mailbox driver

 .../bindings/mailbox/samsung,exynos.yaml      |  70 +++++++++
 MAINTAINERS                                   |   9 ++
 drivers/mailbox/Kconfig                       |  11 ++
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/exynos-mailbox.c              | 143 ++++++++++++++++++
 5 files changed, 235 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml
 create mode 100644 drivers/mailbox/exynos-mailbox.c

-- 
2.47.0.338.g60cca15819-goog


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

* [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos
  2024-12-05 17:41 [PATCH v3 0/3] mailbox: add samsung exynos driver Tudor Ambarus
@ 2024-12-05 17:41 ` Tudor Ambarus
  2024-12-09  7:52   ` Krzysztof Kozlowski
  2024-12-05 17:41 ` [PATCH v3 2/3] mailbox: add samsung exynos driver Tudor Ambarus
  2024-12-05 17:41 ` [PATCH v3 3/3] MAINTAINERS: add entry for samsung exynos mailbox driver Tudor Ambarus
  2 siblings, 1 reply; 10+ messages in thread
From: Tudor Ambarus @ 2024-12-05 17:41 UTC (permalink / raw)
  To: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar
  Cc: linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin,
	Tudor Ambarus

Add bindings for the Samsung Exynos Mailbox Controller.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 .../bindings/mailbox/samsung,exynos.yaml      | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml b/Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml
new file mode 100644
index 000000000000..1fddec1fc64c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/samsung,exynos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos Mailbox Controller
+
+maintainers:
+  - Tudor Ambarus <tudor.ambarus@linaro.org>
+
+description: |
+  The samsung exynos mailbox controller has 16 flag bits for hardware interrupt
+  generation and a shared register for passing mailbox messages. When the
+  controller is used by the ACPM protocol the shared register is ignored and
+  the mailbox controller acts as a doorbell. The controller just raises the
+  interrupt to the firmware after the ACPM protocol has written the message to
+  SRAM.
+
+properties:
+  compatible:
+    const: google,gs101-acpm-mbox
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: pclk
+
+  interrupts:
+    description: IRQ line for the RX mailbox.
+    maxItems: 1
+
+  '#mbox-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - '#mbox-cells'
+
+additionalProperties: false
+
+examples:
+  # Doorbell mode.
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/google,gs101.h>
+
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        ap2apm_mailbox: mailbox@17610000 {
+            compatible = "google,gs101-acpm-mbox";
+            reg = <0x17610000 0x1000>;
+            clocks = <&cmu_apm CLK_GOUT_APM_MAILBOX_APM_AP_PCLK>;
+            clock-names = "pclk";
+            interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH 0>;
+            #mbox-cells = <1>;
+        };
+    };
-- 
2.47.0.338.g60cca15819-goog


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

* [PATCH v3 2/3] mailbox: add samsung exynos driver
  2024-12-05 17:41 [PATCH v3 0/3] mailbox: add samsung exynos driver Tudor Ambarus
  2024-12-05 17:41 ` [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos Tudor Ambarus
@ 2024-12-05 17:41 ` Tudor Ambarus
  2024-12-09  7:55   ` Krzysztof Kozlowski
  2024-12-05 17:41 ` [PATCH v3 3/3] MAINTAINERS: add entry for samsung exynos mailbox driver Tudor Ambarus
  2 siblings, 1 reply; 10+ messages in thread
From: Tudor Ambarus @ 2024-12-05 17:41 UTC (permalink / raw)
  To: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar
  Cc: linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin,
	Tudor Ambarus

The samsung exynos mailbox controller has 16 flag bits for hardware
interrupt generation and a shared register for passing mailbox messages.
When the controller is used by the ACPM protocol the shared register is
ignored and the mailbox controller acts as a doorbell. The controller
just raises the interrupt to APM after the ACPM protocol has written
the message to SRAM.

Add support for the samsung exynos mailbox controller.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mailbox/Kconfig          |  11 +++
 drivers/mailbox/Makefile         |   2 +
 drivers/mailbox/exynos-mailbox.c | 143 +++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/mailbox/exynos-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 8ecba7fb999e..44b808c4d97f 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -36,6 +36,17 @@ config ARM_MHU_V3
 	  that provides different means of transports: supported extensions
 	  will be discovered and possibly managed at probe-time.
 
+config EXYNOS_MBOX
+	tristate "Exynos Mailbox"
+	depends on ARCH_EXYNOS || COMPILE_TEST
+	help
+	  Say Y here if you want to build the Samsung Exynos Mailbox controller
+	  driver. The controller has 16 flag bits for hardware interrupt
+	  generation and a shared register for passing mailbox messages.
+	  When the controller is used by the ACPM protocol the shared register
+	  is ignored and the mailbox controller acts as a doorbell that raises
+	  the interrupt to the ACPM firmware.
+
 config IMX_MBOX
 	tristate "i.MX Mailbox"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 5f4f5b0ce2cc..86192b5c7c32 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -11,6 +11,8 @@ obj-$(CONFIG_ARM_MHU_V2)	+= arm_mhuv2.o
 
 obj-$(CONFIG_ARM_MHU_V3)	+= arm_mhuv3.o
 
+obj-$(CONFIG_EXYNOS_MBOX)	+= exynos-mailbox.o
+
 obj-$(CONFIG_IMX_MBOX)	+= imx-mailbox.o
 
 obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX)	+= armada-37xx-rwtm-mailbox.o
diff --git a/drivers/mailbox/exynos-mailbox.c b/drivers/mailbox/exynos-mailbox.c
new file mode 100644
index 000000000000..6d4e9b3106b2
--- /dev/null
+++ b/drivers/mailbox/exynos-mailbox.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Samsung Electronics Co., Ltd.
+ * Copyright 2020 Google LLC.
+ * Copyright 2024 Linaro Ltd.
+ */
+
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define EXYNOS_MBOX_MCUCTRL		0x0	/* Mailbox Control Register */
+#define EXYNOS_MBOX_INTCR0		0x24	/* Interrupt Clear Register 0 */
+#define EXYNOS_MBOX_INTMR0		0x28	/* Interrupt Mask Register 0 */
+#define EXYNOS_MBOX_INTSR0		0x2c	/* Interrupt Status Register 0 */
+#define EXYNOS_MBOX_INTMSR0		0x30	/* Interrupt Mask Status Register 0 */
+#define EXYNOS_MBOX_INTGR1		0x40	/* Interrupt Generation Register 1 */
+#define EXYNOS_MBOX_INTMR1		0x48	/* Interrupt Mask Register 1 */
+#define EXYNOS_MBOX_INTSR1		0x4c	/* Interrupt Status Register 1 */
+#define EXYNOS_MBOX_INTMSR1		0x50	/* Interrupt Mask Status Register 1 */
+
+#define EXYNOS_MBOX_INTMR0_MASK		GENMASK(15, 0)
+#define EXYNOS_MBOX_INTGR1_MASK		GENMASK(15, 0)
+
+#define EXYNOS_MBOX_CHAN_COUNT		HWEIGHT32(EXYNOS_MBOX_INTGR1_MASK)
+
+/**
+ * struct exynos_mbox - driver's private data.
+ * @regs:	mailbox registers base address.
+ * @mbox:	pointer to the mailbox controller.
+ * @dev:	pointer to the mailbox device.
+ * @pclk:	pointer to the mailbox peripheral clock.
+ */
+struct exynos_mbox {
+	void __iomem *regs;
+	struct mbox_controller *mbox;
+	struct device *dev;
+	struct clk *pclk;
+};
+
+static int exynos_mbox_chan_index(struct mbox_chan *chan)
+{
+	struct mbox_controller *mbox = chan->mbox;
+	int i;
+
+	for (i = 0; i < mbox->num_chans; i++)
+		if (chan == &mbox->chans[i])
+			return i;
+	return -EINVAL;
+}
+
+static int exynos_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct exynos_mbox *exynos_mbox = dev_get_drvdata(chan->mbox->dev);
+	int index;
+
+	index = exynos_mbox_chan_index(chan);
+	if (index < 0)
+		return index;
+
+	writel_relaxed(BIT(index), exynos_mbox->regs + EXYNOS_MBOX_INTGR1);
+
+	return 0;
+}
+
+static const struct mbox_chan_ops exynos_mbox_chan_ops = {
+	.send_data = exynos_mbox_send_data,
+};
+
+static const struct of_device_id exynos_mbox_match[] = {
+	{ .compatible = "google,gs101-acpm-mbox" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_mbox_match);
+
+static int exynos_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct exynos_mbox *exynos_mbox;
+	struct mbox_controller *mbox;
+	struct mbox_chan *chans;
+	int i;
+
+	exynos_mbox = devm_kzalloc(dev, sizeof(*exynos_mbox), GFP_KERNEL);
+	if (!exynos_mbox)
+		return -ENOMEM;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	chans = devm_kcalloc(dev, EXYNOS_MBOX_CHAN_COUNT, sizeof(*chans),
+			     GFP_KERNEL);
+	if (!chans)
+		return -ENOMEM;
+
+	exynos_mbox->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(exynos_mbox->regs))
+		return PTR_ERR(exynos_mbox->regs);
+
+	exynos_mbox->pclk = devm_clk_get_enabled(dev, "pclk");
+	if (IS_ERR(exynos_mbox->pclk))
+		return dev_err_probe(dev, PTR_ERR(exynos_mbox->pclk),
+				     "Failed to enable clock.\n");
+
+	mbox->num_chans = EXYNOS_MBOX_CHAN_COUNT;
+	mbox->chans = chans;
+	mbox->dev = dev;
+	mbox->ops = &exynos_mbox_chan_ops;
+
+	for (i = 0; i < EXYNOS_MBOX_CHAN_COUNT; i++)
+		chans[i].mbox = mbox;
+
+	exynos_mbox->dev = dev;
+	exynos_mbox->mbox = mbox;
+
+	platform_set_drvdata(pdev, exynos_mbox);
+
+	/* Mask out all interrupts. We support just polling channels for now. */
+	writel_relaxed(EXYNOS_MBOX_INTMR0_MASK,
+		       exynos_mbox->regs + EXYNOS_MBOX_INTMR0);
+
+	return devm_mbox_controller_register(dev, mbox);
+}
+
+static struct platform_driver exynos_mbox_driver = {
+	.probe	= exynos_mbox_probe,
+	.driver	= {
+		.name = "exynos-acpm-mbox",
+		.of_match_table	= of_match_ptr(exynos_mbox_match),
+	},
+};
+module_platform_driver(exynos_mbox_driver);
+
+MODULE_AUTHOR("Tudor Ambarus <tudor.ambarus@linaro.org>");
+MODULE_DESCRIPTION("Exynos mailbox driver");
+MODULE_LICENSE("GPL");
-- 
2.47.0.338.g60cca15819-goog


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

* [PATCH v3 3/3] MAINTAINERS: add entry for samsung exynos mailbox driver
  2024-12-05 17:41 [PATCH v3 0/3] mailbox: add samsung exynos driver Tudor Ambarus
  2024-12-05 17:41 ` [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos Tudor Ambarus
  2024-12-05 17:41 ` [PATCH v3 2/3] mailbox: add samsung exynos driver Tudor Ambarus
@ 2024-12-05 17:41 ` Tudor Ambarus
  2 siblings, 0 replies; 10+ messages in thread
From: Tudor Ambarus @ 2024-12-05 17:41 UTC (permalink / raw)
  To: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar
  Cc: linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin,
	Tudor Ambarus

Add entry for the samsung exynos mailbox driver.

Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1e930c7a58b1..41a29d1d6e4d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3023,6 +3023,7 @@ F:	drivers/*/*s3c24*
 F:	drivers/*/*s3c64xx*
 F:	drivers/*/*s5pv210*
 F:	drivers/clocksource/samsung_pwm_timer.c
+F:	drivers/mailbox/exynos-mailbox.c
 F:	drivers/memory/samsung/
 F:	drivers/pwm/pwm-samsung.c
 F:	drivers/soc/samsung/
@@ -20712,6 +20713,14 @@ F:	arch/arm64/boot/dts/exynos/exynos850*
 F:	drivers/clk/samsung/clk-exynos850.c
 F:	include/dt-bindings/clock/exynos850.h
 
+SAMSUNG EXYNOS MAILBOX DRIVER
+M:	Tudor Ambarus <tudor.ambarus@linaro.org>
+L:	linux-kernel@vger.kernel.org
+L:	linux-samsung-soc@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml
+F:	drivers/mailbox/exynos-mailbox.c
+
 SAMSUNG EXYNOS PSEUDO RANDOM NUMBER GENERATOR (RNG) DRIVER
 M:	Krzysztof Kozlowski <krzk@kernel.org>
 L:	linux-crypto@vger.kernel.org
-- 
2.47.0.338.g60cca15819-goog


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

* Re: [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos
  2024-12-05 17:41 ` [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos Tudor Ambarus
@ 2024-12-09  7:52   ` Krzysztof Kozlowski
  2024-12-09  8:11     ` Tudor Ambarus
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-09  7:52 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar,
	linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin

On Thu, Dec 05, 2024 at 05:41:35PM +0000, Tudor Ambarus wrote:
> Add bindings for the Samsung Exynos Mailbox Controller.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  .../bindings/mailbox/samsung,exynos.yaml      | 70 +++++++++++++++++++

Filename based on compatible, so:
google,gs101-acpm-mbox

but then entire binding seems for different device, so you most likely
miss here actual Exynos devices.

>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/samsung,exynos.yaml

Best regards,
Krzysztof


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

* Re: [PATCH v3 2/3] mailbox: add samsung exynos driver
  2024-12-05 17:41 ` [PATCH v3 2/3] mailbox: add samsung exynos driver Tudor Ambarus
@ 2024-12-09  7:55   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-09  7:55 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar,
	linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin

On Thu, Dec 05, 2024 at 05:41:36PM +0000, Tudor Ambarus wrote:
> The samsung exynos mailbox controller has 16 flag bits for hardware


Here, subject and other text/descriptions:
s/samsung/Samsung/
s/exynos/Exynos/


> interrupt generation and a shared register for passing mailbox messages.
> When the controller is used by the ACPM protocol the shared register is
> ignored and the mailbox controller acts as a doorbell. The controller
> just raises the interrupt to APM after the ACPM protocol has written
> the message to SRAM.

...

> +static int exynos_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> +	struct exynos_mbox *exynos_mbox = dev_get_drvdata(chan->mbox->dev);
> +	int index;
> +
> +	index = exynos_mbox_chan_index(chan);
> +	if (index < 0)
> +		return index;
> +
> +	writel_relaxed(BIT(index), exynos_mbox->regs + EXYNOS_MBOX_INTGR1);

writel()

> +
> +	return 0;
> +}
> +
> +static const struct mbox_chan_ops exynos_mbox_chan_ops = {
> +	.send_data = exynos_mbox_send_data,
> +};
> +
> +static const struct of_device_id exynos_mbox_match[] = {
> +	{ .compatible = "google,gs101-acpm-mbox" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_mbox_match);
> +
> +static int exynos_mbox_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_mbox *exynos_mbox;
> +	struct mbox_controller *mbox;
> +	struct mbox_chan *chans;
> +	int i;
> +
> +	exynos_mbox = devm_kzalloc(dev, sizeof(*exynos_mbox), GFP_KERNEL);
> +	if (!exynos_mbox)
> +		return -ENOMEM;
> +
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +
> +	chans = devm_kcalloc(dev, EXYNOS_MBOX_CHAN_COUNT, sizeof(*chans),
> +			     GFP_KERNEL);
> +	if (!chans)
> +		return -ENOMEM;
> +
> +	exynos_mbox->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(exynos_mbox->regs))
> +		return PTR_ERR(exynos_mbox->regs);
> +
> +	exynos_mbox->pclk = devm_clk_get_enabled(dev, "pclk");
> +	if (IS_ERR(exynos_mbox->pclk))
> +		return dev_err_probe(dev, PTR_ERR(exynos_mbox->pclk),
> +				     "Failed to enable clock.\n");
> +
> +	mbox->num_chans = EXYNOS_MBOX_CHAN_COUNT;
> +	mbox->chans = chans;
> +	mbox->dev = dev;
> +	mbox->ops = &exynos_mbox_chan_ops;
> +
> +	for (i = 0; i < EXYNOS_MBOX_CHAN_COUNT; i++)
> +		chans[i].mbox = mbox;
> +
> +	exynos_mbox->dev = dev;
> +	exynos_mbox->mbox = mbox;
> +
> +	platform_set_drvdata(pdev, exynos_mbox);
> +
> +	/* Mask out all interrupts. We support just polling channels for now. */
> +	writel_relaxed(EXYNOS_MBOX_INTMR0_MASK,

writel()

> +		       exynos_mbox->regs + EXYNOS_MBOX_INTMR0);
> +
> +	return devm_mbox_controller_register(dev, mbox);
> +}
> +
> +static struct platform_driver exynos_mbox_driver = {
> +	.probe	= exynos_mbox_probe,
> +	.driver	= {
> +		.name = "exynos-acpm-mbox",
> +		.of_match_table	= of_match_ptr(exynos_mbox_match),

Drop of_match_ptr() - unused symbol warnings.

Best regards,
Krzysztof


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

* Re: [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos
  2024-12-09  7:52   ` Krzysztof Kozlowski
@ 2024-12-09  8:11     ` Tudor Ambarus
  2024-12-09  8:33       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Tudor Ambarus @ 2024-12-09  8:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar,
	linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin

Thanks for the review, Krzysztof!

On 12/9/24 7:52 AM, Krzysztof Kozlowski wrote:
> On Thu, Dec 05, 2024 at 05:41:35PM +0000, Tudor Ambarus wrote:
>> Add bindings for the Samsung Exynos Mailbox Controller.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>> ---
>>  .../bindings/mailbox/samsung,exynos.yaml      | 70 +++++++++++++++++++
> 
> Filename based on compatible, so:
> google,gs101-acpm-mbox
> 
> but then entire binding seems for different device, so you most likely
> miss here actual Exynos devices.
> 

I need some guidance here, please. The mailbox controller can pass the
mailbox messages either via its own data registers, or via SRAM (like it
is used by the ACPM protocol).

I'm thinking of using the same driver for both cases, and differentiate
between the two by compatible and `of_device_id.data`. Thus I propose to
have a "google,gs101-acpm-mbox" compatible for the ACPM SRAM case and in
the future we may add a "google,gs101-mbox" compatible for the messages
passed via the controller's data register case.

Given this, I shall use the more generic name for the bindings, thus
maybe "google,gs101-mbox.yaml"? But then exynos850 has the same
controller, shouldn't we just use "samsung,exynos.yaml"?

Thanks!
ta

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

* Re: [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos
  2024-12-09  8:11     ` Tudor Ambarus
@ 2024-12-09  8:33       ` Krzysztof Kozlowski
  2024-12-09 14:19         ` Tudor Ambarus
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-09  8:33 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar,
	linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin

On 09/12/2024 09:11, Tudor Ambarus wrote:
> Thanks for the review, Krzysztof!
> 
> On 12/9/24 7:52 AM, Krzysztof Kozlowski wrote:
>> On Thu, Dec 05, 2024 at 05:41:35PM +0000, Tudor Ambarus wrote:
>>> Add bindings for the Samsung Exynos Mailbox Controller.
>>>
>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>> ---
>>>  .../bindings/mailbox/samsung,exynos.yaml      | 70 +++++++++++++++++++
>>
>> Filename based on compatible, so:
>> google,gs101-acpm-mbox
>>
>> but then entire binding seems for different device, so you most likely
>> miss here actual Exynos devices.
>>
> 
> I need some guidance here, please. The mailbox controller can pass the
> mailbox messages either via its own data registers, or via SRAM (like it
> is used by the ACPM protocol).
> 
> I'm thinking of using the same driver for both cases, and differentiate
> between the two by compatible and `of_device_id.data`. Thus I propose to
> have a "google,gs101-acpm-mbox" compatible for the ACPM SRAM case and in
> the future we may add a "google,gs101-mbox" compatible for the messages
> passed via the controller's data register case.

Good that you pointed it out, I was indeed wondering why this is
"acpm-mbox", not "mbox in compatible.

This needs to be fixed - you cannot have two compatibles for the same
device.

> 
> Given this, I shall use the more generic name for the bindings, thus
> maybe "google,gs101-mbox.yaml"? But then exynos850 has the same
> controller, shouldn't we just use "samsung,exynos.yaml"?

If exynos850 has the same controller, then add it to the binding. Anyway
then use samsung,exynos850-mbox, because samsung,exynos is way too generic.



Best regards,
Krzysztof

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

* Re: [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos
  2024-12-09  8:33       ` Krzysztof Kozlowski
@ 2024-12-09 14:19         ` Tudor Ambarus
  2024-12-09 14:32           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Tudor Ambarus @ 2024-12-09 14:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar,
	linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin



On 12/9/24 8:33 AM, Krzysztof Kozlowski wrote:
>> I'm thinking of using the same driver for both cases, and differentiate
>> between the two by compatible and `of_device_id.data`. Thus I propose to
>> have a "google,gs101-acpm-mbox" compatible for the ACPM SRAM case and in
>> the future we may add a "google,gs101-mbox" compatible for the messages
>> passed via the controller's data register case.
> Good that you pointed it out, I was indeed wondering why this is
> "acpm-mbox", not "mbox in compatible.
> 
> This needs to be fixed - you cannot have two compatibles for the same
> device.

Will fix. I followed arm,mhu, which differentiates the transfer mode,
data or doorbell, via compatible.

For the fix I'll use "#mbox-cells" as <&phandle type channel>, where
type specifies doorbel or data type. Clients will use:
	mboxes = <&ap2apm_mailbox DOORBELL 2>;
or
	mboxes = <&ap2apm_mailbox DATA 3>;

arm,mhu3 and fsl,mu pass the transfer mode in a similar way.


>> Given this, I shall use the more generic name for the bindings, thus
>> maybe "google,gs101-mbox.yaml"? But then exynos850 has the same
>> controller, shouldn't we just use "samsung,exynos.yaml"?
> If exynos850 has the same controller, then add it to the binding. Anyway
> then use samsung,exynos850-mbox, because samsung,exynos is way too generic.

Looks the same, yes, it differs by the number of how many data registers
each has. But I'll stick to "google,gs101-mbox.yaml", as I can't test
exynos850 and I assume we can rename the file when we'll need it.

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

* Re: [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos
  2024-12-09 14:19         ` Tudor Ambarus
@ 2024-12-09 14:32           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-09 14:32 UTC (permalink / raw)
  To: Tudor Ambarus
  Cc: jassisinghbrar, robh, krzk+dt, conor+dt, alim.akhtar,
	linux-kernel, linux-samsung-soc, devicetree, linux-arm-kernel,
	andre.draszik, kernel-team, willmcvicker, peter.griffin

On 09/12/2024 15:19, Tudor Ambarus wrote:
> 
> 
> On 12/9/24 8:33 AM, Krzysztof Kozlowski wrote:
>>> I'm thinking of using the same driver for both cases, and differentiate
>>> between the two by compatible and `of_device_id.data`. Thus I propose to
>>> have a "google,gs101-acpm-mbox" compatible for the ACPM SRAM case and in
>>> the future we may add a "google,gs101-mbox" compatible for the messages
>>> passed via the controller's data register case.
>> Good that you pointed it out, I was indeed wondering why this is
>> "acpm-mbox", not "mbox in compatible.
>>
>> This needs to be fixed - you cannot have two compatibles for the same
>> device.
> 
> Will fix. I followed arm,mhu, which differentiates the transfer mode,
> data or doorbell, via compatible.

There was a reasoning behind: different firmware.

Best regards,
Krzysztof

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

end of thread, other threads:[~2024-12-09 14:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-05 17:41 [PATCH v3 0/3] mailbox: add samsung exynos driver Tudor Ambarus
2024-12-05 17:41 ` [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos Tudor Ambarus
2024-12-09  7:52   ` Krzysztof Kozlowski
2024-12-09  8:11     ` Tudor Ambarus
2024-12-09  8:33       ` Krzysztof Kozlowski
2024-12-09 14:19         ` Tudor Ambarus
2024-12-09 14:32           ` Krzysztof Kozlowski
2024-12-05 17:41 ` [PATCH v3 2/3] mailbox: add samsung exynos driver Tudor Ambarus
2024-12-09  7:55   ` Krzysztof Kozlowski
2024-12-05 17:41 ` [PATCH v3 3/3] MAINTAINERS: add entry for samsung exynos mailbox driver Tudor Ambarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox