| Message ID | 20241205174137.190545-2-tudor.ambarus@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mailbox: add samsung exynos driver | expand |
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
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
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
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.
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
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>; + }; + };
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