Re: [PATCH 4/4] iommu/arm-smmu-v3: cleanup arm_smmu_dev_{enable,disable}_feature

2022-07-09 Thread Will Deacon 
On Fri, Jul 08, 2022 at 10:06:16AM +0200, Christoph Hellwig wrote:
> Fold the arm_smmu_dev_has_feature arm_smmu_dev_feature_enabled into
> the main methods.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 55 ++---
>  1 file changed, 14 insertions(+), 41 deletions(-)

Thanks for the cleanup:

Acked-by: Will Deacon 

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 4/4] iommu/arm-smmu-v3: cleanup arm_smmu_dev_{enable, disable}_feature

2022-07-09 Thread Christoph Hellwig 
Fold the arm_smmu_dev_has_feature arm_smmu_dev_feature_enabled into
the main methods.

Signed-off-by: Christoph Hellwig 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 55 ++---
 1 file changed, 14 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 4a5e435567f17..d32b02336411d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2765,58 +2765,27 @@ static void arm_smmu_get_resv_regions(struct device 
*dev,
iommu_dma_get_resv_regions(dev, head);
 }
 
-static bool arm_smmu_dev_has_feature(struct device *dev,
-enum iommu_dev_features feat)
-{
-   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
-   if (!master)
-   return false;
-
-   switch (feat) {
-   case IOMMU_DEV_FEAT_IOPF:
-   return arm_smmu_master_iopf_supported(master);
-   case IOMMU_DEV_FEAT_SVA:
-   return arm_smmu_master_sva_supported(master);
-   default:
-   return false;
-   }
-}
-
-static bool arm_smmu_dev_feature_enabled(struct device *dev,
-enum iommu_dev_features feat)
-{
-   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
-   if (!master)
-   return false;
-
-   switch (feat) {
-   case IOMMU_DEV_FEAT_IOPF:
-   return master->iopf_enabled;
-   case IOMMU_DEV_FEAT_SVA:
-   return arm_smmu_master_sva_enabled(master);
-   default:
-   return false;
-   }
-}
-
 static int arm_smmu_dev_enable_feature(struct device *dev,
   enum iommu_dev_features feat)
 {
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
 
-   if (!arm_smmu_dev_has_feature(dev, feat))
+   if (!master)
return -ENODEV;
 
-   if (arm_smmu_dev_feature_enabled(dev, feat))
-   return -EBUSY;
-
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
+   if (!arm_smmu_master_iopf_supported(master))
+   return -EINVAL;
+   if (master->iopf_enabled)
+   return -EBUSY;
master->iopf_enabled = true;
return 0;
case IOMMU_DEV_FEAT_SVA:
+   if (!arm_smmu_master_sva_supported(master))
+   return -EINVAL;
+   if (arm_smmu_master_sva_enabled(master))
+   return -EBUSY;
return arm_smmu_master_enable_sva(master);
default:
return -EINVAL;
@@ -2828,16 +2797,20 @@ static int arm_smmu_dev_disable_feature(struct device 
*dev,
 {
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
 
-   if (!arm_smmu_dev_feature_enabled(dev, feat))
+   if (!master)
return -EINVAL;
 
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
+   if (!master->iopf_enabled)
+   return -EINVAL;
if (master->sva_enabled)
return -EBUSY;
master->iopf_enabled = false;
return 0;
case IOMMU_DEV_FEAT_SVA:
+   if (!arm_smmu_master_sva_enabled(master))
+   return -EINVAL;
return arm_smmu_master_disable_sva(master);
default:
return -EINVAL;
-- 
2.30.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 4/4] iommu/arm-smmu-v3: cleanup arm_smmu_dev_{enable, disable}_feature

2022-04-07 Thread Tian, Kevin 
> From: Christoph Hellwig
> Sent: Thursday, April 7, 2022 2:26 PM
> 
> Fold the arm_smmu_dev_has_feature arm_smmu_dev_feature_enabled
> into
> the main methods.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 57 ++---
>  1 file changed, 15 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 1ea184bbf750a6..8e201c660139ae 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2760,58 +2760,27 @@ static void arm_smmu_get_resv_regions(struct
> device *dev,
>   iommu_dma_get_resv_regions(dev, head);
>  }
> 
> -static bool arm_smmu_dev_has_feature(struct device *dev,
> -  enum iommu_dev_features feat)
> -{
> - struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> -
> - if (!master)
> - return false;
> -
> - switch (feat) {
> - case IOMMU_DEV_FEAT_IOPF:
> - return arm_smmu_master_iopf_supported(master);
> - case IOMMU_DEV_FEAT_SVA:
> - return arm_smmu_master_sva_supported(master);
> - default:
> - return false;
> - }
> -}
> -
> -static bool arm_smmu_dev_feature_enabled(struct device *dev,
> -  enum iommu_dev_features feat)
> -{
> - struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> -
> - if (!master)
> - return false;
> -
> - switch (feat) {
> - case IOMMU_DEV_FEAT_IOPF:
> - return master->iopf_enabled;
> - case IOMMU_DEV_FEAT_SVA:
> - return arm_smmu_master_sva_enabled(master);
> - default:
> - return false;
> - }
> -}
> -
>  static int arm_smmu_dev_enable_feature(struct device *dev,
>  enum iommu_dev_features feat)
>  {
>   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> 
> - if (!arm_smmu_dev_has_feature(dev, feat))
> - return -ENODEV;
> -
> - if (arm_smmu_dev_feature_enabled(dev, feat))
> - return -EBUSY;
> + if (!master)
> + return -EINVAL;

Old logic returns -ENODEV but it's changed to -EINVAL here. Is it
intended? If yes, probably mention it in the patch description though
just a small semantics change.

> 
>   switch (feat) {
>   case IOMMU_DEV_FEAT_IOPF:
> + if (!arm_smmu_master_iopf_supported(master))
> + return -EINVAL;
> + if (master->iopf_enabled)
> + return -EBUSY;
>   master->iopf_enabled = true;
>   return 0;
>   case IOMMU_DEV_FEAT_SVA:
> + if (!arm_smmu_master_sva_supported(master))
> + return -EINVAL;
> + if (arm_smmu_master_sva_enabled(master))
> + return -EBUSY;
>   return arm_smmu_master_enable_sva(master);
>   default:
>   return -EINVAL;
> @@ -2823,16 +2792,20 @@ static int
> arm_smmu_dev_disable_feature(struct device *dev,
>  {
>   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> 
> - if (!arm_smmu_dev_feature_enabled(dev, feat))
> + if (!master)
>   return -EINVAL;
> 
>   switch (feat) {
>   case IOMMU_DEV_FEAT_IOPF:
> + if (!master->iopf_enabled)
> + return -EINVAL;
>   if (master->sva_enabled)
>   return -EBUSY;
>   master->iopf_enabled = false;
>   return 0;
>   case IOMMU_DEV_FEAT_SVA:
> + if (!arm_smmu_master_sva_enabled(master))
> + return -EINVAL;
>   return arm_smmu_master_disable_sva(master);
>   default:
>   return -EINVAL;
> --
> 2.30.2
> 
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 4/4] iommu/arm-smmu-v3: cleanup arm_smmu_dev_{enable, disable}_feature

2022-04-06 Thread Christoph Hellwig 
Fold the arm_smmu_dev_has_feature arm_smmu_dev_feature_enabled into
the main methods.

Signed-off-by: Christoph Hellwig 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 57 ++---
 1 file changed, 15 insertions(+), 42 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 1ea184bbf750a6..8e201c660139ae 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2760,58 +2760,27 @@ static void arm_smmu_get_resv_regions(struct device 
*dev,
iommu_dma_get_resv_regions(dev, head);
 }
 
-static bool arm_smmu_dev_has_feature(struct device *dev,
-enum iommu_dev_features feat)
-{
-   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
-   if (!master)
-   return false;
-
-   switch (feat) {
-   case IOMMU_DEV_FEAT_IOPF:
-   return arm_smmu_master_iopf_supported(master);
-   case IOMMU_DEV_FEAT_SVA:
-   return arm_smmu_master_sva_supported(master);
-   default:
-   return false;
-   }
-}
-
-static bool arm_smmu_dev_feature_enabled(struct device *dev,
-enum iommu_dev_features feat)
-{
-   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
-   if (!master)
-   return false;
-
-   switch (feat) {
-   case IOMMU_DEV_FEAT_IOPF:
-   return master->iopf_enabled;
-   case IOMMU_DEV_FEAT_SVA:
-   return arm_smmu_master_sva_enabled(master);
-   default:
-   return false;
-   }
-}
-
 static int arm_smmu_dev_enable_feature(struct device *dev,
   enum iommu_dev_features feat)
 {
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
 
-   if (!arm_smmu_dev_has_feature(dev, feat))
-   return -ENODEV;
-
-   if (arm_smmu_dev_feature_enabled(dev, feat))
-   return -EBUSY;
+   if (!master)
+   return -EINVAL;
 
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
+   if (!arm_smmu_master_iopf_supported(master))
+   return -EINVAL;
+   if (master->iopf_enabled)
+   return -EBUSY;
master->iopf_enabled = true;
return 0;
case IOMMU_DEV_FEAT_SVA:
+   if (!arm_smmu_master_sva_supported(master))
+   return -EINVAL;
+   if (arm_smmu_master_sva_enabled(master))
+   return -EBUSY;
return arm_smmu_master_enable_sva(master);
default:
return -EINVAL;
@@ -2823,16 +2792,20 @@ static int arm_smmu_dev_disable_feature(struct device 
*dev,
 {
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
 
-   if (!arm_smmu_dev_feature_enabled(dev, feat))
+   if (!master)
return -EINVAL;
 
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
+   if (!master->iopf_enabled)
+   return -EINVAL;
if (master->sva_enabled)
return -EBUSY;
master->iopf_enabled = false;
return 0;
case IOMMU_DEV_FEAT_SVA:
+   if (!arm_smmu_master_sva_enabled(master))
+   return -EINVAL;
return arm_smmu_master_disable_sva(master);
default:
return -EINVAL;
-- 
2.30.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu