Re: [PATCH v2 2/3] mailbox: exynos: Add support for Exynos850 mailbox

Alexey Klimov
2026-04-28T20:26:38Z

On Wed Apr 8, 2026 at 2:08 PM BST, Alexey Klimov wrote:

[...]

> On Thu Apr 2, 2026 at 9:42 AM BST, Tudor Ambarus wrote:
>>> static int exynos_mbox_send_data(struct mbox_chan *chan, void *data)
>>> @@ -57,7 +104,8 @@ static int exynos_mbox_send_data(struct mbox_chan *chan, void *data)
>>> return -EINVAL;
>>> }
>>>
>>> - writel(BIT(msg->chan_id), exynos_mbox->regs + EXYNOS_MBOX_INTGR1);
>>> + writel(BIT(msg->chan_id) << exynos_mbox->data->irq_doorbell_shift,
>>> + exynos_mbox->regs + exynos_mbox->data->irq_doorbell_offset);
>>
>> Use FIELD_PREP from <linux/bitfield.h> please. You will use a mask instead of
>> a shift.
>>
>> I would rename irq_doorbell_offset to intgr. It aligns with the register name
>> from the datasheet. You won't need to prepend _offset to the name, we already
>> see it's an offset when doing the writel().
>
> Sure. Thanks. Let's use FIELD_PREP.

+ /* Ring the doorbell */
+ writel(BIT(msg->chan_id) << exynos_mbox->data->intgr_shift,
+ exynos_mbox->regs + exynos_mbox->data->intgr);

FIELD_PREP() wants a mask as a compile-time constant. Unless you want me to
add switch/case for different SoCs or I misunderstood something I don't see
how this is gonna fly.

Best regards,
Alexey