Re: [PATCH 2/4] iommu/exynos: Check if SysMMU v7 has VM registers

2022-07-09 Thread Sam Protsenko
On Sun, 3 Jul 2022 at 22:10, Krzysztof Kozlowski
 wrote:
>
> On 02/07/2022 23:37, Sam Protsenko wrote:
> > SysMMU v7 can have Virtual Machine registers, which implement multiple
> > translation domains. The driver should know if it's true or not, as VM
> > registers shouldn't be accessed if not present. Read corresponding
> > capabilities register to obtain that info, and store it in driver data.
> >
> > Signed-off-by: Sam Protsenko 
> > ---
> >  drivers/iommu/exynos-iommu.c | 42 ++--
> >  1 file changed, 36 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 28f8c8d93aa3..df6ddbebbe2b 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> >  #define CFG_SYSSEL   (1 << 22) /* System MMU 3.2 only */
> >  #define CFG_FLPDCACHE(1 << 20) /* System MMU 3.2+ only */
> >
> > +#define CAPA0_CAPA1_EXISTBIT(11)
> > +#define CAPA1_VCR_ENABLEDBIT(14)
> > +
> >  /* common registers */
> >  #define REG_MMU_CTRL 0x000
> >  #define REG_MMU_CFG  0x004
> > @@ -171,6 +174,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
> >  #define REG_V5_FAULT_AR_VA   0x070
> >  #define REG_V5_FAULT_AW_VA   0x080
> >
> > +/* v7.x registers */
> > +#define REG_V7_CAPA0 0x870
> > +#define REG_V7_CAPA1 0x874
> > +
> >  #define has_sysmmu(dev)  (dev_iommu_priv_get(dev) != NULL)
> >
> >  static struct device *dma_dev;
> > @@ -274,6 +281,9 @@ struct sysmmu_drvdata {
> >   unsigned int version;   /* our version */
> >
> >   struct iommu_device iommu;  /* IOMMU core handle */
> > +
> > + /* v7 fields */
> > + bool has_vcr;   /* virtual machine control register */
> >  };
> >
> >  static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain 
> > *dom)
> > @@ -364,11 +374,7 @@ static void __sysmmu_disable_clocks(struct 
> > sysmmu_drvdata *data)
> >
> >  static void __sysmmu_get_version(struct sysmmu_drvdata *data)
> >  {
> > - u32 ver;
> > -
> > - __sysmmu_enable_clocks(data);
> > -
> > - ver = readl(data->sfrbase + REG_MMU_VERSION);
> > + const u32 ver = readl(data->sfrbase + REG_MMU_VERSION);
>
>
> No need for const for local, non-pointer variables. There is no benefit
> in preventing the modification and it is not a constant.
>

I'd say it's more a matter of taste, having "const" kinda disciplines
one. But I don't mind removing those bits, will do in v2.

> >
> >   /* controllers on some SoCs don't report proper version */
> >   if (ver == 0x8001u)
> > @@ -378,6 +384,29 @@ static void __sysmmu_get_version(struct sysmmu_drvdata 
> > *data)
> >
> >   dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
> >   MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> > +}
> > +
> > +static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
> > +{
> > + const u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);
>
> Same here and further.
>
>
> Best regards,
> Krzysztof
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/4] iommu/exynos: Check if SysMMU v7 has VM registers

2022-07-03 Thread Krzysztof Kozlowski
On 02/07/2022 23:37, Sam Protsenko wrote:
> SysMMU v7 can have Virtual Machine registers, which implement multiple
> translation domains. The driver should know if it's true or not, as VM
> registers shouldn't be accessed if not present. Read corresponding
> capabilities register to obtain that info, and store it in driver data.
> 
> Signed-off-by: Sam Protsenko 
> ---
>  drivers/iommu/exynos-iommu.c | 42 ++--
>  1 file changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 28f8c8d93aa3..df6ddbebbe2b 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -135,6 +135,9 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>  #define CFG_SYSSEL   (1 << 22) /* System MMU 3.2 only */
>  #define CFG_FLPDCACHE(1 << 20) /* System MMU 3.2+ only */
>  
> +#define CAPA0_CAPA1_EXISTBIT(11)
> +#define CAPA1_VCR_ENABLEDBIT(14)
> +
>  /* common registers */
>  #define REG_MMU_CTRL 0x000
>  #define REG_MMU_CFG  0x004
> @@ -171,6 +174,10 @@ static u32 lv2ent_offset(sysmmu_iova_t iova)
>  #define REG_V5_FAULT_AR_VA   0x070
>  #define REG_V5_FAULT_AW_VA   0x080
>  
> +/* v7.x registers */
> +#define REG_V7_CAPA0 0x870
> +#define REG_V7_CAPA1 0x874
> +
>  #define has_sysmmu(dev)  (dev_iommu_priv_get(dev) != NULL)
>  
>  static struct device *dma_dev;
> @@ -274,6 +281,9 @@ struct sysmmu_drvdata {
>   unsigned int version;   /* our version */
>  
>   struct iommu_device iommu;  /* IOMMU core handle */
> +
> + /* v7 fields */
> + bool has_vcr;   /* virtual machine control register */
>  };
>  
>  static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
> @@ -364,11 +374,7 @@ static void __sysmmu_disable_clocks(struct 
> sysmmu_drvdata *data)
>  
>  static void __sysmmu_get_version(struct sysmmu_drvdata *data)
>  {
> - u32 ver;
> -
> - __sysmmu_enable_clocks(data);
> -
> - ver = readl(data->sfrbase + REG_MMU_VERSION);
> + const u32 ver = readl(data->sfrbase + REG_MMU_VERSION);


No need for const for local, non-pointer variables. There is no benefit
in preventing the modification and it is not a constant.

>  
>   /* controllers on some SoCs don't report proper version */
>   if (ver == 0x8001u)
> @@ -378,6 +384,29 @@ static void __sysmmu_get_version(struct sysmmu_drvdata 
> *data)
>  
>   dev_dbg(data->sysmmu, "hardware version: %d.%d\n",
>   MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version));
> +}
> +
> +static bool __sysmmu_has_capa1(struct sysmmu_drvdata *data)
> +{
> + const u32 capa0 = readl(data->sfrbase + REG_V7_CAPA0);

Same here and further.


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