Re: [PATCH 13/15] iommu/qcom: Use accessor functions for iommu private data

Jean-Philippe Brucker
2020-03-16T15:52:23Z

On Tue, Mar 10, 2020 at 10:12:27AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Make use of dev_iommu_priv_set/get() functions.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> drivers/iommu/qcom_iommu.c | 61 ++++++++++++++++++++++----------------
> 1 file changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index 4328da0b0a9f..80147f82d427 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -74,16 +74,19 @@ static struct qcom_iommu_domain *to_qcom_iommu_domain(struct iommu_domain *dom)
>
> static const struct iommu_ops qcom_iommu_ops;
>
> -static struct qcom_iommu_dev * to_iommu(struct iommu_fwspec *fwspec)
> +static struct qcom_iommu_dev * to_iommu(struct device *dev)
> {
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> if (!fwspec || fwspec->ops != &qcom_iommu_ops)
> return NULL;

Following other drivers, I think this test could move to add_device() (and
remove_device()?), but this is probably good enough for now.

> - return fwspec->iommu_priv;
> +
> + return dev_iommu_priv_get(dev);
> }
[...]
> static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
> {
> - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> struct qcom_iommu_dev *qcom_iommu;
> struct platform_device *iommu_pdev;
> unsigned asid = args->args[0];
> @@ -583,14 +594,14 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
> WARN_ON(asid > qcom_iommu->num_ctxs))
> return -EINVAL;
>
> - if (!fwspec->iommu_priv) {
> - fwspec->iommu_priv = qcom_iommu;
> + if (dev_iommu_priv_get(dev)) {

Should be:

if (!dev_iommu_priv_set(dev))

Thanks,
Jean

> + dev_iommu_priv_set(dev, qcom_iommu);
> } else {
> /* make sure devices iommus dt node isn't referring to
> * multiple different iommu devices. Multiple context
> * banks are ok, but multiple devices are not:
> */
> - if (WARN_ON(qcom_iommu != fwspec->iommu_priv))
> + if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev)))
> return -EINVAL;
> }
>
> --
> 2.17.1
>