Hi Robin, Thanks for your reply. On 02/28/2018 12:59 AM, Robin Murphy wrote: >>> the rockchip IOMMU is part of the master block in hardware, so it needs >>> to control the master's power domain and some of the master's clocks >>> when access it's registers. >>> >>> and the number of clocks needed here, might be different between each >>> IOMMUs(according to which master block it belongs), it's a little like >>> our power domain: >>> https://elixir.free-electrons.com/linux/latest/source/arch/arm64/boot/dts/rockchip/rk3399.dtsi#L935 >>> >>> >>> >>> i'm not sure how to describe this correctly, is it ok use something like >>> "the same as it's master block"? >> >> would it make sense to add a property to specify the master who owns >> the iommu, and we can get all clocks(only some of those clocks are >> actually needed) from it in the of_xlate()? and we can also reuse the >> clock-names of that master to build clk_bulk_data and log errors in >> clk_bulk_get. > > I'm inclined to agree with Rob here - if we're to add anything to the > binding, it should only be whatever clock inputs are defined for the > IOMMU IP block itself. If Linux doesn't properly handle the interconnect > clock hierarchy external to a particular integration, that's a separate > issue and it's not the binding's problem. > > I actually quite like the hack of "borrowing" the clocks from > dev->of_node in of_xlate() - you shouldn't need any DT changes for that, > because you already know that each IOMMU instance only has the one > master device anyway. Thanks:) but actually we are going to support sharing IOMMU between multiple masters(one of them is the main master i think) in the newer chips(not yet supported on upstream kernel)... So we might have to get all clocks from all masters, or find a way to specify the main master...and for the multiple masters case, do it in of_xlate() turns out to be a little racy...maybe we can add a property to specify main master, and get it's clocks in probe()? > > Robin.
