RE: [PATCH V2 1/3] iommu/arm-smmu: Add pm_runtime/sleep ops

["Sricharan" <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

Hi Robin,

>>>>> On Thu, Feb 02, 2017 at 10:40:18PM +0530, Sricharan R wrote:
>>>>>> +- clock-names:    Should be a pair of "smmu_iface_clk" and "smmu_bus_clk"
>>>>>> +                  required for smmu's register group access and interface
>>>>>> +                  clk for the smmu's underlying bus access.
>>>>>> +
>>>>>> +- clocks:         Phandles for respective clocks described by clock-names.
>>>>>
>>>>> Which SMMU implementations are those clock-names valid for?
>>>>>
>>>>> The SMMU architecture specifications do not architect the clocks, which
>>>>> are implemementation-specific.
>>>>>
>>>>> AFAICT, this doesn't match MMU-400 or MMU-500.
>>>>
>>>> Ok, should be more specific. Infact QCOM has MMU-500 and also
>>>> a smmu v2 implementation which is fully compatible with
>>>> "arm,smmu-v2", with the clocks being controlled by the soc's
>>>> clock controller. i was trying to define these clock bindings
>>>> so that its works across socs.
>>>
>>> I don't think we can do that, if we don't know precisely what those
>>> clocks are used for.
>>>
>>> i.e. we'd need a compatible string for the QCOM SMMUv2 variant, which
>>> would imply the set of clocks.
>>>
>>
>> Ok, this was what i was trying to do for V3 and will actually put it
>> this way.
>
>Clocks are not architectural, so it only makes sense to associate them
>with an implementation-specific compatible string. There's also no

ok, it for this the QCOM specific implementation binding is tried(going to).

>guarantee that different microarchitectures have equivalent internal
>clock domains - I'm not sure if "the SMMU's underlying bus access" is
>meant to refer to accesses *by* the SMMU, i.e. page table walks,
>accesses *through* the SMMU by upstream masters, or both

In the above QCOM case, it is actually both. Its the same path for both the
page table walker and upstream masters.

>differences are rather significant. I'd also note that an MMU-500
>configuration may have up to *33* clocks.
>
>Either way, the QCOM implementation deserves its own compatible if only
>for the sake of the imp-def gaps in the architecture (e.g. FSR.SS
>behaviour WRT to IRQs as touched upon in the other thread).
>

Ok, slightly unclear, so you mean then *clocks* are not good enough reason
to have a new compatible ?

Regards,
 Sricharan





>Robin.
>
>>>> But just thinking if it would scale well for any other soc that is
>>>> compatible with arm,smmu-v2 driver and wants to handle clocks in the
>>>> future ?
>>>
>>> I don't think we can have our cake and eat it here. Either we handle the
>>> clock management for each variant, or we don't do it at all. We have no
>>> idea what requirements a future variant might have w.r.t. the management
>>> of clocks we don't know about yet.
>>>
>>
>> Right, at this point, this is first soc which adds the clocks in to the driver.
>> Feels if the clocks are initialized and enabled/disabled as a part of some
>> implementation specific callbacks, that would help always because that is
>> the part which is going to different for each implementation and patches 2,3
>> would remain common. Finally, as you have suggested will introduce new
>> SMMU binding in the case of QCOM and will try to handle clocks specifically for that
>> implementation and see how it looks.
>>
>> Regards,
>>  Sricharan