Re: [PATCH v3 0/5] Introduce "non-pixel" sub node within iris video node

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 7/10/25 8:18 PM, Prakash Gupta wrote:
> 
> 
> On 7/5/2025 4:14 AM, Bryan O'Donoghue wrote:
>> On 04/07/2025 17:45, Dmitry Baryshkov wrote:
>>> What about instead:
>>>
>>> - keep IOMMU entries as is
>> ack
>>
>>> - Add iommu-maps, mapping the non-pixel SID
>>> - In future expand iommu-maps, describing the secure contexts?
>>
>> Interesting, we are _adding_ so that's not an ABI break and if I'm
>> reading the documentation right, there's no hard rule on what iommu-map
>> defines i.e. nothing to preclude us from encoding the information we
>> want here.
>>
>> This might work.
>>
>> drivers/pci/controller/dwc/pcie-qcom.c::qcom_pcie_config_sid_1_9_0()
>>
>> You can register your platform device to the SID map you parse.
> 
> I see few limitations with using iommu-map here, some of these are
> listed in [1]
> 
> 1. We can't specify SMR mask with iommu-map.
> 2. We can't specify different iommu-addresses range for specific contexts.
> 3. The secure CB support [2] would require vmid information associated
> with per context. I think this can't be provided with iommu-map.

FWIW iommu-map should probably be evolved to support passing more
than one cell of iommu_spec in general.. For us specifically, it's
only a matter of time before some platform's PCIe controller
requires* we pass a non-zero SMR mask (unless we should be doing
that already somewhere..)

(* we can obviously do the masking manually and put the effective
values in dt, but "eeh")

Perhaps that can even be done without messing up backwards
compatiblity (treat it as pseudocode, def incomplete):

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7043acd971a0..bca54035f90e 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2068,9 +2068,12 @@ int of_map_id(const struct device_node *np, u32 id,
               const char *map_name, const char *map_mask_name,
               struct device_node **target, u32 *id_out)
 {
+       const char *cells_prop_name __free(kfree);
        u32 map_mask, masked_id;
+       u32 cell_count;
        int map_len;
        const __be32 *map = NULL;
+       int ret;
 
        if (!np || !map_name || (!target && !id_out))
                return -EINVAL;
@@ -2084,7 +2087,15 @@ int of_map_id(const struct device_node *np, u32 id,
                return 0;
        }
 
-       if (!map_len || map_len % (4 * sizeof(*map))) {
+       cells_prop_name = kasprintf(GFP_KERNEL, "#%s-cells", map_name);
+       if (!cells_prop_name)
+               return -EINVAL;
+
+       ret = of_property_read_u32(np, cells_prop_name, &cell_count);
+       if (ret)
+               return ret;
+
+       if (!map_len || map_len % ((2 + cell_count + 1) * sizeof(*map))) {
                pr_err("%pOF: Error: Bad %s length: %d\n", np,
                        map_name, map_len);
                return -EINVAL;
@@ -2106,7 +2117,7 @@ int of_map_id(const struct device_node *np, u32 id,
                u32 id_base = be32_to_cpup(map + 0);
                u32 phandle = be32_to_cpup(map + 1);
                u32 out_base = be32_to_cpup(map + 2);
-               u32 id_len = be32_to_cpup(map + 3);
+               u32 id_len = be32_to_cpup(map + cell_count - 1);
 
                if (id_base & ~map_mask) {
                        pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",

Konrad