Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org BB0AC607DC
MIME-Version: 1.0
In-Reply-To: <5179668.PHK6S3sxLu@aspire.rjw.lan>
References: <20180708173413.1965-1-vivek.gautam@codeaurora.org>
 <20180708173413.1965-4-vivek.gautam@codeaurora.org> <5179668.PHK6S3sxLu@aspire.rjw.lan>
From:   Vivek Gautam <vivek.gautam@codeaurora.org>
Date:   Wed, 18 Jul 2018 15:00:39 +0530
Message-ID: <CAFp+6iGcwdK=wyPf++u3B+ORghuB1YhYmnJLSwvt1efG9H4YeA@mail.gmail.com>
Subject: Re: [PATCH v12 3/4] iommu/arm-smmu: Add the device_link between
 masters and smmu
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Robin Murphy <robin.murphy@arm.com>
Cc:     "list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg
        Roedel <joro@8bytes.org>," <joro@8bytes.org>,
        "robh+dt" <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        "list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg
        Roedel <joro@8bytes.org>," <iommu@lists.linux-foundation.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        alex.williamson@redhat.com, Rob Clark <robdclark@gmail.com>,
        Linux PM <linux-pm@vger.kernel.org>,
        freedreno <freedreno@lists.freedesktop.org>, sboyd@kernel.org,
        Tomasz Figa <tfiga@chromium.org>,
        Sricharan R <sricharan@codeaurora.org>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Archit Taneja <architt@codeaurora.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Jordan Crouse <jcrouse@codeaurora.org>,
        Lukas Wunner <lukas@wunner.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jul 11, 2018 at 3:23 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Sunday, July 8, 2018 7:34:12 PM CEST Vivek Gautam wrote:
>> From: Sricharan R <sricharan@codeaurora.org>
>>
>> Finally add the device link between the master device and
>> smmu, so that the smmu gets runtime enabled/disabled only when the
>> master needs it. This is done from add_device callback which gets
>> called once when the master is added to the smmu.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Lukas Wunner <lukas@wunner.de>
>> ---
>>
>>  - Change since v11
>>    * Replaced DL_FLAG_AUTOREMOVE flag with DL_FLAG_AUTOREMOVE_SUPPLIER.
>>
>>  drivers/iommu/arm-smmu.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 09265e206e2d..916cde4954d2 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -1461,8 +1461,20 @@ static int arm_smmu_add_device(struct device *dev)
>>
>>       iommu_device_link(&smmu->iommu, dev);
>>
>> +     if (pm_runtime_enabled(smmu->dev) &&
>
> Why does the creation of the link depend on whether or not runtime PM
> is enabled for the MMU device?
>
> What about system-wide PM and system shutdown?  Are they always guaranteed
> to happen in the right order without the link?

Hi Robin,

As Rafael pointed, we should the device link creation should not depend on
runtime PM being enabled or not, as we would also want to guarantee
that system wide PM callbacks are called in the right order for smmu
and clients.

Does this change of removing the check for pm_runtime_enabled() from here
looks okay to you?

FYI, as discussed in the first patch [1] of this series, I will add a
system wide
suspend callback - arm_smmu_pm_suspend, that would do clock disable, and will
add corresponding clock enable calls in arm_smmu_pm_resume().

[1] https://lore.kernel.org/patchwork/patch/960460/


Best regards
Vivek

>
>> +         !device_link_add(dev, smmu->dev,
>> +                     DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER)) {
>> +             dev_err(smmu->dev, "Unable to add link to the consumer %s\n",
>> +                     dev_name(dev));
>> +             ret = -ENODEV;
>> +             goto out_unlink;
>> +     }
>> +
>>       return 0;
>>
>> +out_unlink:
>> +     iommu_device_unlink(&smmu->iommu, dev);
>> +     arm_smmu_master_free_smes(fwspec);
>>  out_cfg_free:
>>       kfree(cfg);
>>  out_free:
>>
>
>


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation