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

On 27/11/14 05:11, Jassi Brar wrote:

On 26 November 2014 at 22:08, Sudeep Holla [off-list ref] wrote:

quoted

On 26/11/14 16:20, Jassi Brar wrote:

quoted

On 26 November 2014 at 19:30, Sudeep Holla [off-list ref] wrote:

quoted

On 26/11/14 05:37, Jassi Brar wrote:

quoted

quoted

quoted

quoted

quoted

     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 at 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