Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Robin,

Thanks for your reply.

On 02/28/2018 12:59 AM, Robin Murphy wrote:

quoted

quoted

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.