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

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

Hi Mark,

>On Wed, Feb 08, 2017 at 04:23:17PM +0530, Sricharan wrote:
>> >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.

>> So there are one or more interface clocks which are required for the
>> smmu's interface or the configuration access and one or more clocks
>> required for smmu's downstream bus access. That was the reason i was
>> trying to iterate over the list of clocks down below.  But agree that
>> the bindings should define each of the clocks required separately.
>
>As above, I don't think the code should do this. It should only touch
>the clocks it knows about.
>

ok, after defining QCOM specific SMMU bindings, this would be become 
handling clocks specific to QCOM implementation as per its clock
bindings, which as i understand is what you suggest.

>> So one way here is, define a separate compatible for QCOM's SMMU
>> implementation and define all the clock bindings as a part of it
>> and handle it in the same way in the driver.
>
>That would be my preference.
>

ok.

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