Re: [PATCH v3 1/3] dt-bindings: mailbox: add bindings for samsung,exynos

On 09/12/2024 09:11, Tudor Ambarus wrote:

Thanks for the review, Krzysztof!

On 12/9/24 7:52 AM, Krzysztof Kozlowski wrote:

quoted

On Thu, Dec 05, 2024 at 05:41:35PM +0000, Tudor Ambarus wrote:

quoted

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