Re: [PATCH 2/9] mailbox: arm_mhu: add driver for ARM MHU controller

Sudeep Holla
2014-11-26T16:38:07Z

On 26/11/14 16:20, Jassi Brar wrote:
> On 26 November 2014 at 19:30, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 26/11/14 05:37, Jassi Brar wrote:
>>>
>
>>>>> It seems you still don't get my point that the driver should manage
>>>>> all channels - S & NS. If Linux is running in NS mode on a platform,
>>>>> the DT will specify only some NS channel to be used. The controller
>>>>> driver shouldn't be crippled just because you think Linux will never
>>>>> be run in Secure mode.
>>>>>
>>>>
>>>> Ok how do you handle that, I don't see that in the DT binding. As it
>>>> stands, you can unconditionally try to access the secure channel and
>>>> cause aborts if the platform is running in non-secure mode.
>>>>
>>> No. Please look at the dtsi again ....
>>>
>>> mhu: mailbox@2b1f0000 {
>>> #mbox-cells = <1>;
>>> compatible = "arm,mbox-mhu";
>>> reg = <0 0x2b1f0000 0x1000>;
>>> interrupts = <0 36 4>, /* LP Non-Sec */
>>> <0 35 4>, /* HP Non-Sec */
>>> <0 37 4>; /* Secure */
>>
>>
>> One possible issue I can think of(though current driver design requests
>> irq only on channel startup, it could be moved to probe for optimization
>> in which case you need a way to make sure secure channel or irq is not
>> accessed)
>>
> As you see it is fine as such.

Agreed, but assuming some driver logic. I would like to see some way of
identifying that from DT if we adding the support for secure channel in
the driver else I prefer not to add it unless there is a real user of
it(which is not the case with your current patch set). That will be
handy if there's any issue in future due to some firmware that can't be
changed or upgraded.

Regards,
Sudeep