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

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

On 4/28/26 11:26 PM, Alexey Klimov wrote:
> 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

Indeed, I forgot, sorry.

> add switch/case for different SoCs or I misunderstood something I don't see
> how this is gonna fly.

other option is to have a .ring_doorbell callback defined in the SoC data,
and then you can use FIELD_PREP. But it increases boilerplate code just to
ring a doorbell. I wouldn't go via switch/case. I think we'll have to live
with the mask and shift.

Cheers,
ta