Re: [PATCH v5 3/4] dt-bindings: arm: amlogic: amlogic,meson-gx-ao-secure: add secure-monitor property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for review.

13/06/2024 19.42, Rob Herring wrote:
On Tue, Jun 11, 2024 at 07:07:28PM +0100, Conor Dooley wrote:
On Tue, Jun 11, 2024 at 01:25:11PM +0300, Viacheslav wrote:
Hi!

10/06/2024 19.08, Conor Dooley wrote:
On Mon, Jun 10, 2024 at 11:39:49AM +0300, Viacheslav Bocharov wrote:
Add secure-monitor property to schema for meson-gx-socinfo-sm driver.

"bindings are for hardware, not drivers". Why purpose does the "secure
monitor" serve that the secure firmware needs a reference to it?

This driver is an extension to the meson-gx-socinfo driver: it supplements
information obtained from the register with information from the
SM_GET_CHIP_ID secure monitor call. Due to the specifics of the module
loading order, we cannot do away with meson-gx-socinfo, as it is used for
platform identification in some drivers. Therefore, the extended information
is formatted as a separate driver, which is loaded after the secure-monitor
driver.

Please stop talking about drivers, this is a binding which is about
hardware. Please provide, in your next version, a commit message that
justifies adding this property without talking about driver probing
order etc, and instead focuses on what service the "secure monitor"
provides etc.

To put it another way, how many secure monitors does 1 system have?

One per system in current device tree.



What do you do if the property is not present? You didn't make it
required which is good because that would be an ABI break.

We need an indication of the ability to use the secure-monitor to obtain additional information within the soc driver. It seemed to me that using an explicit reference to the secure-monitor is the best choice.


You only need a link in DT if there are different possible providers or
some per consumer information to describe (e.g. an interrupt number or
clock ID). You don't have the latter and likely there is only 1 possible
provider.

Would replacing the reference to sm with an option, for example, use-secure-monitor = <1>; look more appropriate in this case?



Rob


_______________________________________________
linux-amlogic mailing list
linux-amlogic@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-amlogic

--
Viacheslav




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux