| Message ID | 1486055420-19671-2-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, 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. > +static int arm_smmu_init_clocks(struct arm_smmu_device *smmu) > +{ > + const char *cname; > + struct property *prop; > + int i = 0; > + struct device *dev = smmu->dev; > + > + smmu->num_clocks = > + of_property_count_strings(dev->of_node, "clock-names"); > + > + if (smmu->num_clocks < 1) > + return 0; > + > + smmu->clocks = devm_kzalloc(dev, > + sizeof(*smmu->clocks) * smmu->num_clocks, > + GFP_KERNEL); > + > + if (!smmu->clocks) { > + dev_err(dev, "Failed to allocate memory for clocks\n"); > + return -ENODEV; > + } > + > + of_property_for_each_string(dev->of_node, "clock-names", prop, cname) { > + struct clk *c = devm_clk_get(dev, cname); > + > + if (IS_ERR(c)) { > + dev_err(dev, "Couldn't get clock: %s", cname); > + return -EPROBE_DEFER; > + } > + > + smmu->clocks[i] = c; > + ++i; > + } > + > + return 0; > +} I am very much not a fan of grabbing hold of resources that don't necessarily match the binding, and we likely don't understand the use of. Either we know the names, and can manage them, or we don't, and cannot. Thanks, Mark.
Hi Mark, > >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. 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. 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. 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 ? Regards, Sricharan > >> +static int arm_smmu_init_clocks(struct arm_smmu_device *smmu) >> +{ >> + const char *cname; >> + struct property *prop; >> + int i = 0; >> + struct device *dev = smmu->dev; >> + >> + smmu->num_clocks = >> + of_property_count_strings(dev->of_node, "clock-names"); >> + >> + if (smmu->num_clocks < 1) >> + return 0; >> + >> + smmu->clocks = devm_kzalloc(dev, >> + sizeof(*smmu->clocks) * smmu->num_clocks, >> + GFP_KERNEL); >> + >> + if (!smmu->clocks) { >> + dev_err(dev, "Failed to allocate memory for clocks\n"); >> + return -ENODEV; >> + } >> + >> + of_property_for_each_string(dev->of_node, "clock-names", prop, cname) { >> + struct clk *c = devm_clk_get(dev, cname); >> + >> + if (IS_ERR(c)) { >> + dev_err(dev, "Couldn't get clock: %s", cname); >> + return -EPROBE_DEFER; >> + } >> + >> + smmu->clocks[i] = c; >> + ++i; >> + } >> + >> + return 0; >> +} > >I am very much not a fan of grabbing hold of resources that don't >necessarily match the binding, and we likely don't understand the use >of. > >Either we know the names, and can manage them, or we don't, and cannot.
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. > 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. > 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. > 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. Thanks, Mark.
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
On 08/02/17 12:30, Sricharan wrote: > 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. Clocks are not architectural, so it only makes sense to associate them with an implementation-specific compatible string. There's also no 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, and the differences are rather significant. I'd also note that an MMU-500 configuration may have up to *33* clocks. >>> 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. 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). 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 >
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
On Wed, Feb 08, 2017 at 07:15:37PM +0530, Sricharan wrote: > >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 ? I beleive Robin's point was even if the clocks didn't matter, there are other reasons we should have the QCOM-specific compatible string. So we should have one regardless. Thanks, Mark.
On 08/02/17 13:52, Mark Rutland wrote: > On Wed, Feb 08, 2017 at 07:15:37PM +0530, Sricharan wrote: >>> 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. Right, that's what I feared. As far as I can make out the current ARM implementations, transactions passing through will require at least TBUn_BCLK for the appropriate TBU, but would also need the page table walker clocked with CCLK to resolve TLB misses. But then the programming interface is also in the CCLK domain (not counting the incoming APB or AXI clock for the actual slave port itself). Thus this 'generic' clock binding already isn't compatible with MMU-40x/500. >>> 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 ? > > I beleive Robin's point was even if the clocks didn't matter, there are > other reasons we should have the QCOM-specific compatible string. > > So we should have one regardless. Exactly. Robin. > > Thanks, > Mark. >
Hi Robin, >-----Original Message----- >From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy >Sent: Wednesday, February 08, 2017 8:01 PM >To: Mark Rutland <mark.rutland@arm.com>; Sricharan <sricharan@codeaurora.org> >Cc: devicetree@vger.kernel.org; mathieu.poirier@linaro.org; linux-arm-msm@vger.kernel.org; joro@8bytes.org; >will.deacon@arm.com; iommu@lists.linux-foundation.org; robh+dt@kernel.org; sboyd@codeaurora.org; linux-arm- >kernel@lists.infradead.org; m.szyprowski@samsung.com >Subject: Re: [PATCH V2 1/3] iommu/arm-smmu: Add pm_runtime/sleep ops > >On 08/02/17 13:52, Mark Rutland wrote: >> On Wed, Feb 08, 2017 at 07:15:37PM +0530, Sricharan wrote: >>>> 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. > >Right, that's what I feared. As far as I can make out the current ARM >implementations, transactions passing through will require at least >TBUn_BCLK for the appropriate TBU, but would also need the page table >walker clocked with CCLK to resolve TLB misses. But then the programming >interface is also in the CCLK domain (not counting the incoming APB or >AXI clock for the actual slave port itself). Thus this 'generic' clock >binding already isn't compatible with MMU-40x/500. > Right, this implementation's clock bindings are not going to compatible with MMU-500. There is also another soc which integrates MMU-500. So will have to add the clock bindings for MMU-500 as well separately. Also in MMU-500 i saw that there is a possibility where the clock-domain can be shared between the TCU logic, programming interface and PTW read channel. Does this mean that the TCU-clock has to be 'ON' for both register access and PTW, similar to above ?. So for MMU-500 clock bindings there can be a CFG_CLK (optional and not required in shared case), TBUn_CLK and TCU_CLK Regards, Sricharan >>>> 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 ? >> >> I beleive Robin's point was even if the clocks didn't matter, there are >> other reasons we should have the QCOM-specific compatible string. >> >> So we should have one regardless. > >Exactly. > >Robin. > >> >> Thanks, >> Mark. >> > > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt index e862d148..2376828 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt @@ -60,6 +60,18 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- 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. + +- power-domains: Phandles to SMMU's power domain specifier. This is + required even if SMMU belongs to the master's power + domain, as the SMMU will have to be enabled and + accessed before master gets enabled and linked to its + SMMU. + ** Deprecated properties: - mmu-masters (deprecated in favour of the generic "iommus" binding) : @@ -84,6 +96,10 @@ conditions. <0 36 4>, <0 37 4>; #iommu-cells = <1>; + clocks = <&mmcc SMMU_MDP_AHB_CLK>, + <&mmcc SMMU_MDP_AXI_CLK>; + clock-names = "smmu_iface_clk", + "smmu_bus_clk"; }; /* device with two stream IDs, 0 and 7 */ diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 091d4a6..cd2e3db 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -48,6 +48,7 @@ #include <linux/of_iommu.h> #include <linux/pci.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -387,6 +388,8 @@ struct arm_smmu_device { u32 num_global_irqs; u32 num_context_irqs; unsigned int *irqs; + int num_clocks; + struct clk **clocks; u32 cavium_id_base; /* Specific to Cavium */ }; @@ -444,6 +447,31 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) return container_of(dom, struct arm_smmu_domain, domain); } +static int arm_smmu_enable_clocks(struct arm_smmu_device *smmu) +{ + int i, ret = 0; + + for (i = 0; i < smmu->num_clocks; ++i) { + ret = clk_prepare_enable(smmu->clocks[i]); + if (ret) { + dev_err(smmu->dev, "Couldn't enable clock #%d\n", i); + while (i--) + clk_disable_unprepare(smmu->clocks[i]); + break; + } + } + + return ret; +} + +static void arm_smmu_disable_clocks(struct arm_smmu_device *smmu) +{ + int i = smmu->num_clocks; + + while (i--) + clk_disable_unprepare(smmu->clocks[i]); +} + static void parse_driver_options(struct arm_smmu_device *smmu) { int i = 0; @@ -1740,6 +1768,43 @@ static int arm_smmu_id_size_to_bits(int size) } } +static int arm_smmu_init_clocks(struct arm_smmu_device *smmu) +{ + const char *cname; + struct property *prop; + int i = 0; + struct device *dev = smmu->dev; + + smmu->num_clocks = + of_property_count_strings(dev->of_node, "clock-names"); + + if (smmu->num_clocks < 1) + return 0; + + smmu->clocks = devm_kzalloc(dev, + sizeof(*smmu->clocks) * smmu->num_clocks, + GFP_KERNEL); + + if (!smmu->clocks) { + dev_err(dev, "Failed to allocate memory for clocks\n"); + return -ENODEV; + } + + of_property_for_each_string(dev->of_node, "clock-names", prop, cname) { + struct clk *c = devm_clk_get(dev, cname); + + if (IS_ERR(c)) { + dev_err(dev, "Couldn't get clock: %s", cname); + return -EPROBE_DEFER; + } + + smmu->clocks[i] = c; + ++i; + } + + return 0; +} + static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) { unsigned long size; @@ -2121,6 +2186,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev) smmu->irqs[i] = irq; } + err = arm_smmu_init_clocks(smmu); + if (err) + return err; + + pm_runtime_enable(dev); err = arm_smmu_device_cfg_probe(smmu); if (err) return err; @@ -2182,10 +2252,37 @@ static int arm_smmu_device_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM +static int arm_smmu_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct arm_smmu_device *smmu = platform_get_drvdata(pdev); + + return arm_smmu_enable_clocks(smmu); +} + +static int arm_smmu_suspend(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct arm_smmu_device *smmu = platform_get_drvdata(pdev); + + arm_smmu_disable_clocks(smmu); + + return 0; +} +#endif + +static const struct dev_pm_ops arm_smmu_pm_ops = { + SET_RUNTIME_PM_OPS(arm_smmu_suspend, arm_smmu_resume, NULL) + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) +}; + static struct platform_driver arm_smmu_driver = { .driver = { .name = "arm-smmu", .of_match_table = of_match_ptr(arm_smmu_of_match), + .pm = &arm_smmu_pm_ops, }, .probe = arm_smmu_device_probe, .remove = arm_smmu_device_remove,
The smmu needs to be functional only when the respective master's using it are active. The device_link feature helps to track such functional dependencies, so that the iommu gets powered when the master device enables itself using pm_runtime. So by adapting the smmu driver for runtime pm, above said dependency can be addressed. This patch adds the pm runtime/sleep callbacks to the driver and also the functions to parse the smmu clocks from DT and enable them in resume/suspend. Signed-off-by: Sricharan R <sricharan@codeaurora.org> --- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++ drivers/iommu/arm-smmu.c | 97 ++++++++++++++++++++++ 2 files changed, 113 insertions(+)