Re: [PATCH v5 1/3] dt-bindings: mailbox: add google,gs101-mbox

[Tudor Ambarus <tudor.ambarus@linaro.org>]

On 12/18/24 11:23 AM, Peter Griffin wrote:
> Hi,
> 
> On Wed, 18 Dec 2024 at 09:29, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Tue, Dec 17, 2024 at 09:40:20AM +0000, Tudor Ambarus wrote:
>>> +description: |
>>> +  The samsung exynos mailbox controller has 16 flag bits for hardware interrupt
>>
>> If there is going to be any new posting:
>>
>> The Samsung Exynos mailbox controller, used on Google GS101 SoC, ....
>>
>>> +  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-mbox
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: pclk
>>> +
>>> +  interrupts:
>>> +    description: IRQ line for the RX mailbox.
>>> +    maxItems: 1
>>> +
>>> +  '#mbox-cells':
>>> +    description: |
>>> +      <&phandle type channel>
>>> +      phandle : label name of controller.
>>> +      type    : channel type, doorbell or data-transfer.
>>> +      channel : channel number.
>>> +
>>> +      Here is how a client can reference them:
>>> +      mboxes = <&ap2apm_mailbox DOORBELL 2>;
>>> +      mboxes = <&ap2apm_mailbox DATA 3>;
>>
>> This ordering assumes there is channel "2" in doorbel and in data, so
>> two times "2" and of course the same for all others. If this is
>> configuration of one channel, so "2" is either doorbell or data, then
>> type should be the last.

ha, nice observation!

> My understanding was each channel is either configured for doorbell or
> data, but Tudor can confirm. With Krzysztof's feedback addressed:

For the ACPM interface use case, mailbox is always used as a doorbell
indeed. Regardless if the ACPM interface writes data or not to SRAM, it
will use the mailbox controller just to flip the interrupt bit without
touching the mbox controller data registers.

For the other cases where the mailbox controller is used to (also?) pass
data via its data registers, I can't tell whether the passing of data is
mandatory or not. At least not by reading the gs101 mailbox datasheet
that I have, it doesn't describe the SR registers. However, Exynos850 -
which has the same registers, says that:
```CPU cores can use these registers for sharing data```.
"Can" implies that writing to SR is optional.

So I assume a channel can work in both modes and clients use mboxes to
specify which mode to use. Shall I still switch the order?

Thanks,
ta