diff mbox series

[v2,2/3] iommu/mediatek: add support for 6-bit encoded port IDs

Message ID 20221001-iommu-support-v2-2-dbfef2eeebc9@baylibre.com (mailing list archive)
State New, archived
Headers show
Series iommu/mediatek: Add mt8365 iommu support | expand

Commit Message

Alexandre Mergnat Oct. 4, 2022, 10:01 a.m. UTC 
From: Fabien Parent <fparent@baylibre.com>

Until now the port ID was always encoded as a 5-bit data. On MT8365,
the port ID is encoded as a 6-bit data. This requires to rework the
macros F_MMU_INT_ID_LARB_ID, and F_MMU_INT_ID_PORT_ID in order
to support 5-bit and 6-bit encoded port IDs.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 drivers/iommu/mtk_iommu.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

AngeloGioacchino Del Regno Oct. 4, 2022, 11:59 a.m. UTC | #1 
Il 04/10/22 12:01, Alexandre Mergnat ha scritto:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Until now the port ID was always encoded as a 5-bit data. On MT8365,
> the port ID is encoded as a 6-bit data. This requires to rework the
> macros F_MMU_INT_ID_LARB_ID, and F_MMU_INT_ID_PORT_ID in order
> to support 5-bit and 6-bit encoded port IDs.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>   drivers/iommu/mtk_iommu.c | 24 ++++++++++++++++++++----
>   1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 5a4e00e4bbbc..a57ce509c8b5 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -108,8 +108,10 @@
>   #define F_MMU_INT_ID_SUB_COMM_ID(a)		(((a) >> 7) & 0x3)
>   #define F_MMU_INT_ID_COMM_ID_EXT(a)		(((a) >> 10) & 0x7)
>   #define F_MMU_INT_ID_SUB_COMM_ID_EXT(a)		(((a) >> 7) & 0x7)
> -#define F_MMU_INT_ID_LARB_ID(a)			(((a) >> 7) & 0x7)
> -#define F_MMU_INT_ID_PORT_ID(a)			(((a) >> 2) & 0x1f)
> +#define F_MMU_INT_ID_LARB_ID(a, int_id_port_width)	\
> +				((a) >> (((int_id_port_width) + 2) & 0x7))
> +#define F_MMU_INT_ID_PORT_ID(a, int_id_port_width)	\
> +				(((a) >> 2) & GENMASK((int_id_port_width) - 1, 0))

I can't think about any cleaner way than this one, but that's decreasing human
readability by "quite a bit".

The only way you can keep it readable is by adding a comment before these macros
that explains the sub-fields of FAULT_ID, located in the INT_ID register: please
add that.

Regards,
Angelo
Robin Murphy Oct. 6, 2022, 10:46 a.m. UTC | #2 
On 2022-10-04 12:59, AngeloGioacchino Del Regno wrote:
> Il 04/10/22 12:01, Alexandre Mergnat ha scritto:
>> From: Fabien Parent <fparent@baylibre.com>
>>
>> Until now the port ID was always encoded as a 5-bit data. On MT8365,
>> the port ID is encoded as a 6-bit data. This requires to rework the
>> macros F_MMU_INT_ID_LARB_ID, and F_MMU_INT_ID_PORT_ID in order
>> to support 5-bit and 6-bit encoded port IDs.
>>
>> Signed-off-by: Fabien Parent <fparent@baylibre.com>
>> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
>> ---
>>   drivers/iommu/mtk_iommu.c | 24 ++++++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>> index 5a4e00e4bbbc..a57ce509c8b5 100644
>> --- a/drivers/iommu/mtk_iommu.c
>> +++ b/drivers/iommu/mtk_iommu.c
>> @@ -108,8 +108,10 @@
>>   #define F_MMU_INT_ID_SUB_COMM_ID(a)        (((a) >> 7) & 0x3)
>>   #define F_MMU_INT_ID_COMM_ID_EXT(a)        (((a) >> 10) & 0x7)
>>   #define F_MMU_INT_ID_SUB_COMM_ID_EXT(a)        (((a) >> 7) & 0x7)
>> -#define F_MMU_INT_ID_LARB_ID(a)            (((a) >> 7) & 0x7)
>> -#define F_MMU_INT_ID_PORT_ID(a)            (((a) >> 2) & 0x1f)
>> +#define F_MMU_INT_ID_LARB_ID(a, int_id_port_width)    \
>> +                ((a) >> (((int_id_port_width) + 2) & 0x7))
>> +#define F_MMU_INT_ID_PORT_ID(a, int_id_port_width)    \
>> +                (((a) >> 2) & GENMASK((int_id_port_width) - 1, 0))
> 
> I can't think about any cleaner way than this one, but that's decreasing 
> human
> readability by "quite a bit".

In terms of readability, the best thing to do would be define separate 
macros for each register format and make the choice at the (single) 
callsite rather than hiding it in the macro. In fact we're already doing 
exactly that with the HAS_SUB_COMM_2BITS and HAS_SUB_COMM_3BITS flags 
right at the same point, so please follow that same pattern for consistency.

Thanks,
Robin.
diff mbox series

Patch

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 5a4e00e4bbbc..a57ce509c8b5 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -108,8 +108,10 @@ 
 #define F_MMU_INT_ID_SUB_COMM_ID(a)		(((a) >> 7) & 0x3)
 #define F_MMU_INT_ID_COMM_ID_EXT(a)		(((a) >> 10) & 0x7)
 #define F_MMU_INT_ID_SUB_COMM_ID_EXT(a)		(((a) >> 7) & 0x7)
-#define F_MMU_INT_ID_LARB_ID(a)			(((a) >> 7) & 0x7)
-#define F_MMU_INT_ID_PORT_ID(a)			(((a) >> 2) & 0x1f)
+#define F_MMU_INT_ID_LARB_ID(a, int_id_port_width)	\
+				((a) >> (((int_id_port_width) + 2) & 0x7))
+#define F_MMU_INT_ID_PORT_ID(a, int_id_port_width)	\
+				(((a) >> 2) & GENMASK((int_id_port_width) - 1, 0))
 
 #define MTK_PROTECT_PA_ALIGN			256
 #define MTK_IOMMU_BANK_SZ			0x1000
@@ -188,6 +190,7 @@  struct mtk_iommu_plat_data {
 	enum mtk_iommu_plat	m4u_plat;
 	u32			flags;
 	u32			inv_sel_reg;
+	u8			int_id_port_width;
 
 	char			*pericfg_comp_str;
 	struct list_head	*hw_list;
@@ -441,7 +444,8 @@  static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
 	fault_pa |= (u64)pa34_32 << 32;
 
 	if (MTK_IOMMU_IS_TYPE(plat_data, MTK_IOMMU_TYPE_MM)) {
-		fault_port = F_MMU_INT_ID_PORT_ID(regval);
+		fault_port = F_MMU_INT_ID_PORT_ID(regval,
+						  data->plat_data->int_id_port_width);
 		if (MTK_IOMMU_HAS_FLAG(plat_data, HAS_SUB_COMM_2BITS)) {
 			fault_larb = F_MMU_INT_ID_COMM_ID(regval);
 			sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
@@ -449,7 +453,8 @@  static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
 			fault_larb = F_MMU_INT_ID_COMM_ID_EXT(regval);
 			sub_comm = F_MMU_INT_ID_SUB_COMM_ID_EXT(regval);
 		} else {
-			fault_larb = F_MMU_INT_ID_LARB_ID(regval);
+			fault_larb = F_MMU_INT_ID_LARB_ID(
+					regval, data->plat_data->int_id_port_width);
 		}
 		fault_larb = data->plat_data->larbid_remap[fault_larb][sub_comm];
 	}
@@ -1379,6 +1384,7 @@  static const struct mtk_iommu_plat_data mt2712_data = {
 	.banks_enable = {true},
 	.iova_region_nr = ARRAY_SIZE(single_domain),
 	.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt6779_data = {
@@ -1391,6 +1397,7 @@  static const struct mtk_iommu_plat_data mt6779_data = {
 	.iova_region   = single_domain,
 	.iova_region_nr = ARRAY_SIZE(single_domain),
 	.larbid_remap  = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt6795_data = {
@@ -1404,6 +1411,7 @@  static const struct mtk_iommu_plat_data mt6795_data = {
 	.iova_region  = single_domain,
 	.iova_region_nr = ARRAY_SIZE(single_domain),
 	.larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. */
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8167_data = {
@@ -1415,6 +1423,7 @@  static const struct mtk_iommu_plat_data mt8167_data = {
 	.iova_region  = single_domain,
 	.iova_region_nr = ARRAY_SIZE(single_domain),
 	.larbid_remap = {{0}, {1}, {2}}, /* Linear mapping. */
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8173_data = {
@@ -1428,6 +1437,7 @@  static const struct mtk_iommu_plat_data mt8173_data = {
 	.iova_region  = single_domain,
 	.iova_region_nr = ARRAY_SIZE(single_domain),
 	.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8183_data = {
@@ -1439,6 +1449,7 @@  static const struct mtk_iommu_plat_data mt8183_data = {
 	.iova_region  = single_domain,
 	.iova_region_nr = ARRAY_SIZE(single_domain),
 	.larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8186_data_mm = {
@@ -1453,6 +1464,7 @@  static const struct mtk_iommu_plat_data mt8186_data_mm = {
 	.banks_enable   = {true},
 	.iova_region    = mt8192_multi_dom,
 	.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8192_data = {
@@ -1466,6 +1478,7 @@  static const struct mtk_iommu_plat_data mt8192_data = {
 	.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
 	.larbid_remap   = {{0}, {1}, {4, 5}, {7}, {2}, {9, 11, 19, 20},
 			   {0, 14, 16}, {0, 13, 18, 17}},
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8195_data_infra = {
@@ -1481,6 +1494,7 @@  static const struct mtk_iommu_plat_data mt8195_data_infra = {
 			    },
 	.iova_region      = single_domain,
 	.iova_region_nr   = ARRAY_SIZE(single_domain),
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8195_data_vdo = {
@@ -1495,6 +1509,7 @@  static const struct mtk_iommu_plat_data mt8195_data_vdo = {
 	.iova_region_nr	= ARRAY_SIZE(mt8192_multi_dom),
 	.larbid_remap   = {{2, 0}, {21}, {24}, {7}, {19}, {9, 10, 11},
 			   {13, 17, 15/* 17b */, 25}, {5}},
+	.int_id_port_width = 5,
 };
 
 static const struct mtk_iommu_plat_data mt8195_data_vpp = {
@@ -1513,6 +1528,7 @@  static const struct mtk_iommu_plat_data mt8195_data_vpp = {
 			   /* 16: 16a; 29: 16b; 30: CCUtop0; 31: CCUtop1 */
 			   {14, 16, 29, 26, 30, 31, 18},
 			   {4, MTK_INVALID_LARBID, MTK_INVALID_LARBID, MTK_INVALID_LARBID, 6}},
+	.int_id_port_width = 5,
 };
 
 static const struct of_device_id mtk_iommu_of_ids[] = {