linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: sricharan@codeaurora.org (Sricharan)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2 1/3] iommu/arm-smmu: Add pm_runtime/sleep ops
Date: Wed, 8 Feb 2017 19:15:37 +0530	[thread overview]
Message-ID: <000301d28211$a5233a90$ef69afb0$@codeaurora.org> (raw)
In-Reply-To: <b55359d8-2665-aef0-3215-53a0e8f21bcd@arm.com>

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

  reply	other threads:[~2017-02-08 13:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-02 17:10 [PATCH V2 0/3] iommu/arm-smmu: Add runtime pm/sleep support Sricharan R
2017-02-02 17:10 ` [PATCH V2 1/3] iommu/arm-smmu: Add pm_runtime/sleep ops Sricharan R
2017-02-02 17:42   ` Mark Rutland
2017-02-08 10:53     ` Sricharan
2017-02-08 11:40       ` Mark Rutland
2017-02-08 12:30         ` Sricharan
2017-02-08 12:54           ` Robin Murphy
2017-02-08 13:45             ` Sricharan [this message]
2017-02-08 13:52               ` Mark Rutland
2017-02-08 14:30                 ` Robin Murphy
2017-02-09 13:35                   ` Sricharan
2017-02-02 17:10 ` [PATCH V2 2/3] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device Sricharan R
2017-02-02 17:10 ` [PATCH V2 3/3] iommu/arm-smmu: Add the device_link between masters and smmu Sricharan R

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='000301d28211$a5233a90$ef69afb0$@codeaurora.org' \
    --to=sricharan@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).