| Message ID | 20201111123838.15682-10-yong.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MT8192 IOMMU support | expand |
On 2020-11-11 12:38, Yong Wu wrote: > The current _ARM_V7S_LVL_BITS/ARM_V7S_LVL_SHIFT use a formula to calculate > the corresponding value for level1 and level2 to pretend the code sane. > Actually their level1 and level2 values are different from each other. > This patch only clear the two macro. No functional change. Grammar nit: to "clear" the macro sounds like you're making it empty or removing it entirely; I think you mean to say "clarify" here. English is the worst language sometimes... :) Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Suggested-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/io-pgtable-arm-v7s.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c > index 4d0aa079470f..58cc201c10a3 100644 > --- a/drivers/iommu/io-pgtable-arm-v7s.c > +++ b/drivers/iommu/io-pgtable-arm-v7s.c > @@ -44,13 +44,11 @@ > > /* > * We have 32 bits total; 12 bits resolved at level 1, 8 bits at level 2, > - * and 12 bits in a page. With some carefully-chosen coefficients we can > - * hide the ugly inconsistencies behind these macros and at least let the > - * rest of the code pretend to be somewhat sane. > + * and 12 bits in a page. > */ > #define ARM_V7S_ADDR_BITS 32 > -#define _ARM_V7S_LVL_BITS(lvl) (16 - (lvl) * 4) > -#define ARM_V7S_LVL_SHIFT(lvl) (ARM_V7S_ADDR_BITS - (4 + 8 * (lvl))) > +#define _ARM_V7S_LVL_BITS(lvl) ((lvl) == 1 ? 12 : 8) > +#define ARM_V7S_LVL_SHIFT(lvl) ((lvl) == 1 ? 20 : 12) > #define ARM_V7S_TABLE_SHIFT 10 > > #define ARM_V7S_PTES_PER_LVL(lvl) (1 << _ARM_V7S_LVL_BITS(lvl)) >
On Thu, 2020-11-26 at 16:03 +0000, Robin Murphy wrote: > On 2020-11-11 12:38, Yong Wu wrote: > > The current _ARM_V7S_LVL_BITS/ARM_V7S_LVL_SHIFT use a formula to calculate > > the corresponding value for level1 and level2 to pretend the code sane. > > Actually their level1 and level2 values are different from each other. > > This patch only clear the two macro. No functional change. > > Grammar nit: to "clear" the macro sounds like you're making it empty or > removing it entirely; I think you mean to say "clarify" here. English is > the worst language sometimes... :) Thanks for the review. Feel free to tell me if some words is not fit:) I will use "clarify" in the title. > > Reviewed-by: Robin Murphy <robin.murphy@arm.com> > > > Suggested-by: Robin Murphy <robin.murphy@arm.com> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/iommu/io-pgtable-arm-v7s.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c > > index 4d0aa079470f..58cc201c10a3 100644 > > --- a/drivers/iommu/io-pgtable-arm-v7s.c > > +++ b/drivers/iommu/io-pgtable-arm-v7s.c > > @@ -44,13 +44,11 @@ > > > > /* > > * We have 32 bits total; 12 bits resolved at level 1, 8 bits at level 2, > > - * and 12 bits in a page. With some carefully-chosen coefficients we can > > - * hide the ugly inconsistencies behind these macros and at least let the > > - * rest of the code pretend to be somewhat sane. > > + * and 12 bits in a page. > > */ > > #define ARM_V7S_ADDR_BITS 32 > > -#define _ARM_V7S_LVL_BITS(lvl) (16 - (lvl) * 4) > > -#define ARM_V7S_LVL_SHIFT(lvl) (ARM_V7S_ADDR_BITS - (4 + 8 * (lvl))) > > +#define _ARM_V7S_LVL_BITS(lvl) ((lvl) == 1 ? 12 : 8) > > +#define ARM_V7S_LVL_SHIFT(lvl) ((lvl) == 1 ? 20 : 12) > > #define ARM_V7S_TABLE_SHIFT 10 > > > > #define ARM_V7S_PTES_PER_LVL(lvl) (1 << _ARM_V7S_LVL_BITS(lvl)) > >
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c index 4d0aa079470f..58cc201c10a3 100644 --- a/drivers/iommu/io-pgtable-arm-v7s.c +++ b/drivers/iommu/io-pgtable-arm-v7s.c @@ -44,13 +44,11 @@ /* * We have 32 bits total; 12 bits resolved at level 1, 8 bits at level 2, - * and 12 bits in a page. With some carefully-chosen coefficients we can - * hide the ugly inconsistencies behind these macros and at least let the - * rest of the code pretend to be somewhat sane. + * and 12 bits in a page. */ #define ARM_V7S_ADDR_BITS 32 -#define _ARM_V7S_LVL_BITS(lvl) (16 - (lvl) * 4) -#define ARM_V7S_LVL_SHIFT(lvl) (ARM_V7S_ADDR_BITS - (4 + 8 * (lvl))) +#define _ARM_V7S_LVL_BITS(lvl) ((lvl) == 1 ? 12 : 8) +#define ARM_V7S_LVL_SHIFT(lvl) ((lvl) == 1 ? 20 : 12) #define ARM_V7S_TABLE_SHIFT 10 #define ARM_V7S_PTES_PER_LVL(lvl) (1 << _ARM_V7S_LVL_BITS(lvl))
The current _ARM_V7S_LVL_BITS/ARM_V7S_LVL_SHIFT use a formula to calculate the corresponding value for level1 and level2 to pretend the code sane. Actually their level1 and level2 values are different from each other. This patch only clear the two macro. No functional change. Suggested-by: Robin Murphy <robin.murphy@arm.com> Signed-off-by: Yong Wu <yong.wu@mediatek.com> --- drivers/iommu/io-pgtable-arm-v7s.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)