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

[Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>]

On 27/11/14 05:11, Jassi Brar wrote:
> On 26 November 2014 at 22:08, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> wrote:
>>
>>
>> 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.
>>
> Could you please suggest some way of identifying that from DT if we
> adding the support for secure channel in the driver?
>

Sorry, I don't have any idea yet on that as the general rule is not to
touch anything secure in kernel assuming Linux always runs in non-secure
mode.

Now if you need an exception, you need to propose the binding and take
up the discussion on the devicetree list.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html