Re: [PATCH 6/7] iommu: dart: Support different variants with different registers

From: Sven Peter
Date: Wed Jan 04 2023 - 08:44:23 EST

Next message: Geert Uytterhoeven: "Re: [PATCH] clk: microchip: mpfs-ccc: Use devm_kasprintf() for allocating formatted strings"
Previous message: Gatien CHEVALLIER: "Re: [RFC PATCH 3/7] dt-bindings: bus: add STM32MP15 ETZPC firewall bus bindings"
In reply to: Hector Martin: "[PATCH 6/7] iommu: dart: Support different variants with different registers"
Next in thread: Hector Martin: "Re: [PATCH 6/7] iommu: dart: Support different variants with different registers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Jan 4, 2023, at 12:00, Hector Martin wrote:
> T8110 has a new register layout. To accommodate this, first move all the
> register offsets to the hw structure, and rename all the existing
> registers to DART_T8020_*.
>
> Signed-off-by: Hector Martin <marcan@xxxxxxxxx>
> ---

Reviewed-by: Sven Peter <sven@xxxxxxxxxxxxx>

> drivers/iommu/apple-dart.c | 188 ++++++++++++++++++++++++-------------
> 1 file changed, 125 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
> index 03a3cb5638ba..396da83f2f9e 100644
> --- a/drivers/iommu/apple-dart.c
> +++ b/drivers/iommu/apple-dart.c
> @@ -38,6 +38,7 @@
> #define DART_MAX_TTBR 4
> #define MAX_DARTS_PER_DEVICE 2
>
> +/* Common registers */
>
> #define DART_PARAMS1 0x00
> #define DART_PARAMS1_PAGE_SHIFT GENMASK(27, 24)
> @@ -45,52 +46,79 @@
> #define DART_PARAMS2 0x04
> #define DART_PARAMS2_BYPASS_SUPPORT BIT(0)
>
> -#define DART_STREAM_COMMAND 0x20
> -#define DART_STREAM_COMMAND_BUSY BIT(2)
> -#define DART_STREAM_COMMAND_INVALIDATE BIT(20)
> +/* T8020/T6000 registers */
>
> -#define DART_STREAM_SELECT 0x34
> +#define DART_T8020_STREAM_COMMAND 0x20
> +#define DART_T8020_STREAM_COMMAND_BUSY BIT(2)
> +#define DART_T8020_STREAM_COMMAND_INVALIDATE BIT(20)
>
> -#define DART_ERROR 0x40
> -#define DART_ERROR_STREAM GENMASK(27, 24)
> -#define DART_ERROR_CODE GENMASK(11, 0)
> -#define DART_ERROR_FLAG BIT(31)
> +#define DART_T8020_STREAM_SELECT 0x34
>
> -#define DART_ERROR_READ_FAULT BIT(4)
> -#define DART_ERROR_WRITE_FAULT BIT(3)
> -#define DART_ERROR_NO_PTE BIT(2)
> -#define DART_ERROR_NO_PMD BIT(1)
> -#define DART_ERROR_NO_TTBR BIT(0)
> +#define DART_T8020_ERROR 0x40
> +#define DART_T8020_ERROR_STREAM GENMASK(27, 24)
> +#define DART_T8020_ERROR_CODE GENMASK(11, 0)
> +#define DART_T8020_ERROR_FLAG BIT(31)
>
> -#define DART_CONFIG 0x60
> -#define DART_CONFIG_LOCK BIT(15)
> +#define DART_T8020_ERROR_READ_FAULT BIT(4)
> +#define DART_T8020_ERROR_WRITE_FAULT BIT(3)
> +#define DART_T8020_ERROR_NO_PTE BIT(2)
> +#define DART_T8020_ERROR_NO_PMD BIT(1)
> +#define DART_T8020_ERROR_NO_TTBR BIT(0)
> +
> +#define DART_T8020_CONFIG 0x60
> +#define DART_T8020_CONFIG_LOCK BIT(15)
>
> #define DART_STREAM_COMMAND_BUSY_TIMEOUT 100
>
> -#define DART_ERROR_ADDR_HI 0x54
> -#define DART_ERROR_ADDR_LO 0x50
> +#define DART_T8020_ERROR_ADDR_HI 0x54
> +#define DART_T8020_ERROR_ADDR_LO 0x50
> +
> +#define DART_T8020_STREAMS_ENABLE 0xfc
>
> -#define DART_STREAMS_ENABLE 0xfc
> +#define DART_T8020_TCR 0x100
> +#define DART_T8020_TCR_TRANSLATE_ENABLE BIT(7)
> +#define DART_T8020_TCR_BYPASS_DART BIT(8)
> +#define DART_T8020_TCR_BYPASS_DAPF BIT(12)
>
> -#define DART_TCR(sid) (0x100 + 4 * (sid))
> -#define DART_TCR_TRANSLATE_ENABLE BIT(7)
> -#define DART_TCR_BYPASS0_ENABLE BIT(8)
> -#define DART_TCR_BYPASS1_ENABLE BIT(12)
> +#define DART_T8020_TTBR 0x200
> +#define DART_T8020_TTBR_VALID BIT(31)
> +#define DART_T8020_TTBR_ADDR_OFF 0
> +#define DART_T8020_TTBR_SHIFT 12
>
> -#define DART_TTBR_VALID BIT(31)
> -#define DART_TTBR_SHIFT 12
> +#define DART_TCR(dart, sid) ((dart)->hw->tcr + ((sid) << 2))
>
> -#define DART_TTBR(dart, sid, idx) (0x200 + \
> +#define DART_TTBR(dart, sid, idx) ((dart)->hw->ttbr + \
> (((dart)->hw->ttbr_count * (sid)) << 2) + \
> ((idx) << 2))
>
> +struct apple_dart_stream_map;
>
> struct apple_dart_hw {
> + irqreturn_t (*irq_handler)(int irq, void *dev);
> + int (*invalidate_tlb)(struct apple_dart_stream_map *stream_map);
> +
> u32 oas;
> enum io_pgtable_fmt fmt;
>
> int max_sid_count;
>
> + u64 lock;
> + u64 lock_bit;
> +
> + u64 error;
> +
> + u64 enable_streams;
> + u64 disable_streams;
I don't think disable_streams is used anywhere. I assume you just left it in
here to document it?

> +
> + u64 tcr;
> + u64 tcr_enabled;
> + u64 tcr_disabled;
> + u64 tcr_bypass;
> +
> + u64 ttbr;
> + u64 ttbr_valid;
> + u64 ttbr_addr_off;

This name confused me a bit since off sounds like offset to me while
this is actually another shift. Can't really think of a better name
right now though. I'd at least a comment here to describe it.

> + u64 ttbr_shift;
> int ttbr_count;
> };
>
> @@ -217,8 +245,7 @@ apple_dart_hw_enable_translation(struct
> apple_dart_stream_map *stream_map)
> int sid;
>
> for_each_set_bit(sid, stream_map->sidmap, dart->num_streams)
> - writel(DART_TCR_TRANSLATE_ENABLE,
> - dart->regs + DART_TCR(sid));
> + writel(dart->hw->tcr_enabled, dart->regs + DART_TCR(dart, sid));
> }
>
> static void apple_dart_hw_disable_dma(struct apple_dart_stream_map
> *stream_map)
> @@ -227,7 +254,7 @@ static void apple_dart_hw_disable_dma(struct
> apple_dart_stream_map *stream_map)
> int sid;
>
> for_each_set_bit(sid, stream_map->sidmap, dart->num_streams)
> - writel(0, dart->regs + DART_TCR(sid));
> + writel(dart->hw->tcr_disabled, dart->regs + DART_TCR(dart, sid));
> }
>
> static void
> @@ -238,8 +265,8 @@ apple_dart_hw_enable_bypass(struct
> apple_dart_stream_map *stream_map)
>
> WARN_ON(!stream_map->dart->supports_bypass);
> for_each_set_bit(sid, stream_map->sidmap, dart->num_streams)
> - writel(DART_TCR_BYPASS0_ENABLE | DART_TCR_BYPASS1_ENABLE,
> - dart->regs + DART_TCR(sid));
> + writel(dart->hw->tcr_bypass,
> + dart->regs + DART_TCR(dart, sid));
> }
>
> static void apple_dart_hw_set_ttbr(struct apple_dart_stream_map
> *stream_map,
> @@ -248,9 +275,10 @@ static void apple_dart_hw_set_ttbr(struct
> apple_dart_stream_map *stream_map,
> struct apple_dart *dart = stream_map->dart;
> int sid;
>
> - WARN_ON(paddr & ((1 << DART_TTBR_SHIFT) - 1));
> + WARN_ON(paddr & ((1 << dart->hw->ttbr_shift) - 1));
> for_each_set_bit(sid, stream_map->sidmap, dart->num_streams)
> - writel(DART_TTBR_VALID | (paddr >> DART_TTBR_SHIFT),
> + writel(dart->hw->ttbr_valid |
> + (paddr >> dart->hw->ttbr_shift) << dart->hw->ttbr_addr_off,
> dart->regs + DART_TTBR(dart, sid, idx));
> }
>
> @@ -274,7 +302,7 @@ apple_dart_hw_clear_all_ttbrs(struct
> apple_dart_stream_map *stream_map)
> }
>
> static int
> -apple_dart_hw_stream_command(struct apple_dart_stream_map *stream_map,
> +apple_dart_t8020_hw_stream_command(struct apple_dart_stream_map
> *stream_map,
> u32 command)
> {
> unsigned long flags;
> @@ -283,12 +311,12 @@ apple_dart_hw_stream_command(struct
> apple_dart_stream_map *stream_map,
>
> spin_lock_irqsave(&stream_map->dart->lock, flags);
>
> - writel(stream_map->sidmap[0], stream_map->dart->regs +
> DART_STREAM_SELECT);
> - writel(command, stream_map->dart->regs + DART_STREAM_COMMAND);
> + writel(stream_map->sidmap[0], stream_map->dart->regs +
> DART_T8020_STREAM_SELECT);
> + writel(command, stream_map->dart->regs + DART_T8020_STREAM_COMMAND);
>
> ret = readl_poll_timeout_atomic(
> - stream_map->dart->regs + DART_STREAM_COMMAND, command_reg,
> - !(command_reg & DART_STREAM_COMMAND_BUSY), 1,
> + stream_map->dart->regs + DART_T8020_STREAM_COMMAND, command_reg,
> + !(command_reg & DART_T8020_STREAM_COMMAND_BUSY), 1,
> DART_STREAM_COMMAND_BUSY_TIMEOUT);
>
> spin_unlock_irqrestore(&stream_map->dart->lock, flags);
> @@ -304,10 +332,10 @@ apple_dart_hw_stream_command(struct
> apple_dart_stream_map *stream_map,
> }
>
> static int
> -apple_dart_hw_invalidate_tlb(struct apple_dart_stream_map *stream_map)
> +apple_dart_t8020_hw_invalidate_tlb(struct apple_dart_stream_map *stream_map)
> {
> - return apple_dart_hw_stream_command(stream_map,
> - DART_STREAM_COMMAND_INVALIDATE);
> + return apple_dart_t8020_hw_stream_command(
> + stream_map, DART_T8020_STREAM_COMMAND_INVALIDATE);
> }
>
> static int apple_dart_hw_reset(struct apple_dart *dart)
> @@ -316,8 +344,8 @@ static int apple_dart_hw_reset(struct apple_dart *dart)
> struct apple_dart_stream_map stream_map;
> int i;
>
> - config = readl(dart->regs + DART_CONFIG);
> - if (config & DART_CONFIG_LOCK) {
> + config = readl(dart->regs + dart->hw->lock);
> + if (config & dart->hw->lock_bit) {
> dev_err(dart->dev, "DART is locked down until reboot: %08x\n",
> config);
> return -EINVAL;
> @@ -331,12 +359,12 @@ static int apple_dart_hw_reset(struct apple_dart *dart)
>
> /* enable all streams globally since TCR is used to control isolation */
> for (i = 0; i < BITS_TO_U32(dart->num_streams); i++)
> - writel(U32_MAX, dart->regs + DART_STREAMS_ENABLE);
> + writel(U32_MAX, dart->regs + dart->hw->enable_streams);
>
> /* clear any pending errors before the interrupt is unmasked */
> - writel(readl(dart->regs + DART_ERROR), dart->regs + DART_ERROR);
> + writel(readl(dart->regs + dart->hw->error), dart->regs + dart->hw->error);
>
> - return apple_dart_hw_invalidate_tlb(&stream_map);
> + return dart->hw->invalidate_tlb(&stream_map);
> }
>
> static void apple_dart_domain_flush_tlb(struct apple_dart_domain
> *domain)
> @@ -351,7 +379,7 @@ static void apple_dart_domain_flush_tlb(struct
> apple_dart_domain *domain)
> for (j = 0; j < BITS_TO_LONGS(stream_map.dart->num_streams); j++)
> stream_map.sidmap[j] =
> atomic_long_read(&domain_stream_map->sidmap[j]);
>
> - apple_dart_hw_invalidate_tlb(&stream_map);
> + stream_map.dart->hw->invalidate_tlb(&stream_map);
> }
> }
>
> @@ -425,7 +453,7 @@ apple_dart_setup_translation(struct
> apple_dart_domain *domain,
> apple_dart_hw_clear_ttbr(stream_map, i);
>
> apple_dart_hw_enable_translation(stream_map);
> - apple_dart_hw_invalidate_tlb(stream_map);
> + stream_map->dart->hw->invalidate_tlb(stream_map);
> }
>
> static int apple_dart_finalize_domain(struct iommu_domain *domain,
> @@ -816,30 +844,30 @@ static const struct iommu_ops apple_dart_iommu_ops = {
> }
> };
>
> -static irqreturn_t apple_dart_irq(int irq, void *dev)
> +static irqreturn_t apple_dart_t8020_irq(int irq, void *dev)
> {
> struct apple_dart *dart = dev;
> const char *fault_name = NULL;
> - u32 error = readl(dart->regs + DART_ERROR);
> - u32 error_code = FIELD_GET(DART_ERROR_CODE, error);
> - u32 addr_lo = readl(dart->regs + DART_ERROR_ADDR_LO);
> - u32 addr_hi = readl(dart->regs + DART_ERROR_ADDR_HI);
> + u32 error = readl(dart->regs + DART_T8020_ERROR);
> + u32 error_code = FIELD_GET(DART_T8020_ERROR_CODE, error);
> + u32 addr_lo = readl(dart->regs + DART_T8020_ERROR_ADDR_LO);
> + u32 addr_hi = readl(dart->regs + DART_T8020_ERROR_ADDR_HI);
> u64 addr = addr_lo | (((u64)addr_hi) << 32);
> - u8 stream_idx = FIELD_GET(DART_ERROR_STREAM, error);
> + u8 stream_idx = FIELD_GET(DART_T8020_ERROR_STREAM, error);
>
> - if (!(error & DART_ERROR_FLAG))
> + if (!(error & DART_T8020_ERROR_FLAG))
> return IRQ_NONE;
>
> /* there should only be a single bit set but let's use == to be sure */
> - if (error_code == DART_ERROR_READ_FAULT)
> + if (error_code == DART_T8020_ERROR_READ_FAULT)
> fault_name = "READ FAULT";
> - else if (error_code == DART_ERROR_WRITE_FAULT)
> + else if (error_code == DART_T8020_ERROR_WRITE_FAULT)
> fault_name = "WRITE FAULT";
> - else if (error_code == DART_ERROR_NO_PTE)
> + else if (error_code == DART_T8020_ERROR_NO_PTE)
> fault_name = "NO PTE FOR IOVA";
> - else if (error_code == DART_ERROR_NO_PMD)
> + else if (error_code == DART_T8020_ERROR_NO_PMD)
> fault_name = "NO PMD FOR IOVA";
> - else if (error_code == DART_ERROR_NO_TTBR)
> + else if (error_code == DART_T8020_ERROR_NO_TTBR)
> fault_name = "NO TTBR FOR IOVA";
> else
> fault_name = "unknown";
> @@ -849,7 +877,7 @@ static irqreturn_t apple_dart_irq(int irq, void *dev)
> "translation fault: status:0x%x stream:%d code:0x%x (%s) at 0x%llx",
> error, stream_idx, error_code, fault_name, addr);
>
> - writel(error, dart->regs + DART_ERROR);
> + writel(error, dart->regs + DART_T8020_ERROR);
> return IRQ_HANDLED;
> }
>
> @@ -911,7 +939,7 @@ static int apple_dart_probe(struct platform_device *pdev)
> if (ret)
> goto err_clk_disable;
>
> - ret = request_irq(dart->irq, apple_dart_irq, IRQF_SHARED,
> + ret = request_irq(dart->irq, dart->hw->irq_handler, IRQF_SHARED,
> "apple-dart fault handler", dart);
> if (ret)
> goto err_clk_disable;
> @@ -959,17 +987,51 @@ static int apple_dart_remove(struct platform_device *pdev)
> }
>
> static const struct apple_dart_hw apple_dart_hw_t8103 = {
> + .irq_handler = apple_dart_t8020_irq,
> + .invalidate_tlb = apple_dart_t8020_hw_invalidate_tlb,
> .oas = 36,
> .fmt = APPLE_DART,
> .max_sid_count = 16,
>
> + .enable_streams = DART_T8020_STREAMS_ENABLE,
> + .lock = DART_T8020_CONFIG,
> + .lock_bit = DART_T8020_CONFIG_LOCK,
> +
> + .error = DART_T8020_ERROR,
> +
> + .tcr = DART_T8020_TCR,
> + .tcr_enabled = DART_T8020_TCR_TRANSLATE_ENABLE,
> + .tcr_disabled = 0,
> + .tcr_bypass = DART_T8020_TCR_BYPASS_DAPF | DART_T8020_TCR_BYPASS_DART,
> +
> + .ttbr = DART_T8020_TTBR,
> + .ttbr_valid = DART_T8020_TTBR_VALID,
> + .ttbr_addr_off = DART_T8020_TTBR_ADDR_OFF,
> + .ttbr_shift = DART_T8020_TTBR_SHIFT,
> .ttbr_count = 4,
> };
> static const struct apple_dart_hw apple_dart_hw_t6000 = {
> + .irq_handler = apple_dart_t8020_irq,
> + .invalidate_tlb = apple_dart_t8020_hw_invalidate_tlb,
> .oas = 42,
> .fmt = APPLE_DART2,
> .max_sid_count = 16,
>
> + .enable_streams = DART_T8020_STREAMS_ENABLE,
> + .lock = DART_T8020_CONFIG,
> + .lock_bit = DART_T8020_CONFIG_LOCK,
> +
> + .error = DART_T8020_ERROR,
> +
> + .tcr = DART_T8020_TCR,
> + .tcr_enabled = DART_T8020_TCR_TRANSLATE_ENABLE,
> + .tcr_disabled = 0,
> + .tcr_bypass = DART_T8020_TCR_BYPASS_DAPF | DART_T8020_TCR_BYPASS_DART,
> +
> + .ttbr = DART_T8020_TTBR,
> + .ttbr_valid = DART_T8020_TTBR_VALID,
> + .ttbr_addr_off = DART_T8020_TTBR_ADDR_OFF,
> + .ttbr_shift = DART_T8020_TTBR_SHIFT,
> .ttbr_count = 4,
> };
>
> @@ -979,7 +1041,7 @@ static __maybe_unused int
> apple_dart_suspend(struct device *dev)
> unsigned int sid, idx;
>
> for (sid = 0; sid < dart->num_streams; sid++) {
> - dart->save_tcr[sid] = readl_relaxed(dart->regs + DART_TCR(sid));
> + dart->save_tcr[sid] = readl_relaxed(dart->regs + DART_TCR(dart,
> sid));
> for (idx = 0; idx < dart->hw->ttbr_count; idx++)
> dart->save_ttbr[sid][idx] =
> readl(dart->regs + DART_TTBR(dart, sid, idx));
> @@ -1004,7 +1066,7 @@ static __maybe_unused int
> apple_dart_resume(struct device *dev)
> for (idx = 0; idx < DART_MAX_TTBR; idx++)
> writel(dart->save_ttbr[sid][idx],
> dart->regs + DART_TTBR(dart, sid, idx));
> - writel(dart->save_tcr[sid], dart->regs + DART_TCR(sid));
> + writel(dart->save_tcr[sid], dart->regs + DART_TCR(dart, sid));
> }
>
> return 0;
> --
> 2.35.1

Next message: Geert Uytterhoeven: "Re: [PATCH] clk: microchip: mpfs-ccc: Use devm_kasprintf() for allocating formatted strings"
Previous message: Gatien CHEVALLIER: "Re: [RFC PATCH 3/7] dt-bindings: bus: add STM32MP15 ETZPC firewall bus bindings"
In reply to: Hector Martin: "[PATCH 6/7] iommu: dart: Support different variants with different registers"
Next in thread: Hector Martin: "Re: [PATCH 6/7] iommu: dart: Support different variants with different registers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]