Re: [PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic
On Thu, May 21, 2020 at 04:47:19PM -0600, Rob Herring wrote: > On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi > wrote: > > > > There is nothing PCI specific (other than the RID - requester ID) > > in the of_map_rid() implementation, so the same function can be > > reused for input/output IDs mapping for other busses just as well. > > > > Rename the RID instances/names to a generic "id" tag and provide > > an of_map_rid() wrapper function so that we can leave the existing > > (and legitimate) callers unchanged. > > It's not all that clear to a casual observer that RID is a PCI thing, > so I don't know that keeping it buys much. And there's only 3 callers. Yes I agree - I think we can remove the _rid interface. > > No functionality change intended. > > > > Signed-off-by: Lorenzo Pieralisi > > Cc: Rob Herring > > Cc: Joerg Roedel > > Cc: Robin Murphy > > Cc: Marc Zyngier > > --- > > drivers/iommu/of_iommu.c | 2 +- > > drivers/of/base.c| 42 > > include/linux/of.h | 17 +++- > > 3 files changed, 38 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > > index 20738aacac89..ad96b87137d6 100644 > > --- a/drivers/iommu/of_iommu.c > > +++ b/drivers/iommu/of_iommu.c > > @@ -145,7 +145,7 @@ static int of_fsl_mc_iommu_init(struct fsl_mc_device > > *mc_dev, > > struct of_phandle_args iommu_spec = { .args_count = 1 }; > > int err; > > > > - err = of_map_rid(master_np, mc_dev->icid, "iommu-map", > > + err = of_map_id(master_np, mc_dev->icid, "iommu-map", > > I'm not sure this is an improvement because I'd refactor this function > and of_pci_iommu_init() into a single function: > > of_bus_iommu_init(struct device *dev, struct device_node *np, u32 id) > > Then of_pci_iommu_init() becomes: > > of_pci_iommu_init() > { > return of_bus_iommu_init(info->dev, info->np, alias); > } > > And replace of_fsl_mc_iommu_init call with: > err = of_bus_iommu_init(dev, master_np, to_fsl_mc_device(dev)->icid); I will follow up on this on patch 7. > > "iommu-map-mask", _spec.np, > > iommu_spec.args); > > if (err) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > index ae03b1218b06..e000e17bd602 100644 > > --- a/drivers/of/base.c > > +++ b/drivers/of/base.c > > @@ -2201,15 +2201,15 @@ int of_find_last_cache_level(unsigned int cpu) > > } > > > > /** > > - * of_map_rid - Translate a requester ID through a downstream mapping. > > + * of_map_id - Translate a requester ID through a downstream mapping. > > Still a requester ID? Fixed, thanks. Lorenzo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic
On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi wrote: > > There is nothing PCI specific (other than the RID - requester ID) > in the of_map_rid() implementation, so the same function can be > reused for input/output IDs mapping for other busses just as well. > > Rename the RID instances/names to a generic "id" tag and provide > an of_map_rid() wrapper function so that we can leave the existing > (and legitimate) callers unchanged. It's not all that clear to a casual observer that RID is a PCI thing, so I don't know that keeping it buys much. And there's only 3 callers. > No functionality change intended. > > Signed-off-by: Lorenzo Pieralisi > Cc: Rob Herring > Cc: Joerg Roedel > Cc: Robin Murphy > Cc: Marc Zyngier > --- > drivers/iommu/of_iommu.c | 2 +- > drivers/of/base.c| 42 > include/linux/of.h | 17 +++- > 3 files changed, 38 insertions(+), 23 deletions(-) > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index 20738aacac89..ad96b87137d6 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -145,7 +145,7 @@ static int of_fsl_mc_iommu_init(struct fsl_mc_device > *mc_dev, > struct of_phandle_args iommu_spec = { .args_count = 1 }; > int err; > > - err = of_map_rid(master_np, mc_dev->icid, "iommu-map", > + err = of_map_id(master_np, mc_dev->icid, "iommu-map", I'm not sure this is an improvement because I'd refactor this function and of_pci_iommu_init() into a single function: of_bus_iommu_init(struct device *dev, struct device_node *np, u32 id) Then of_pci_iommu_init() becomes: of_pci_iommu_init() { return of_bus_iommu_init(info->dev, info->np, alias); } And replace of_fsl_mc_iommu_init call with: err = of_bus_iommu_init(dev, master_np, to_fsl_mc_device(dev)->icid); > "iommu-map-mask", _spec.np, > iommu_spec.args); > if (err) > diff --git a/drivers/of/base.c b/drivers/of/base.c > index ae03b1218b06..e000e17bd602 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -2201,15 +2201,15 @@ int of_find_last_cache_level(unsigned int cpu) > } > > /** > - * of_map_rid - Translate a requester ID through a downstream mapping. > + * of_map_id - Translate a requester ID through a downstream mapping. Still a requester ID? > * @np: root complex device node. > - * @rid: device requester ID to map. > + * @id: device ID to map. > * @map_name: property name of the map to use. > * @map_mask_name: optional property name of the mask to use. > * @target: optional pointer to a target device node. > * @id_out: optional pointer to receive the translated ID. > * > - * Given a device requester ID, look up the appropriate > implementation-defined > + * Given a device ID, look up the appropriate implementation-defined > * platform ID and/or the target device which receives transactions on that > * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or > * @id_out may be NULL if only the other is required. If @target points to > @@ -2219,11 +2219,11 @@ int of_find_last_cache_level(unsigned int cpu) > * > * Return: 0 on success or a standard error code on failure. > */ > -int of_map_rid(struct device_node *np, u32 rid, > +int of_map_id(struct device_node *np, u32 id, >const char *map_name, const char *map_mask_name, >struct device_node **target, u32 *id_out) > { > - u32 map_mask, masked_rid; > + u32 map_mask, masked_id; > int map_len; > const __be32 *map = NULL; > > @@ -2235,7 +2235,7 @@ int of_map_rid(struct device_node *np, u32 rid, > if (target) > return -ENODEV; > /* Otherwise, no map implies no translation */ > - *id_out = rid; > + *id_out = id; > return 0; > } > > @@ -2255,22 +2255,22 @@ int of_map_rid(struct device_node *np, u32 rid, > if (map_mask_name) > of_property_read_u32(np, map_mask_name, _mask); > > - masked_rid = map_mask & rid; > + masked_id = map_mask & id; > for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) { > struct device_node *phandle_node; > - u32 rid_base = be32_to_cpup(map + 0); > + u32 id_base = be32_to_cpup(map + 0); > u32 phandle = be32_to_cpup(map + 1); > u32 out_base = be32_to_cpup(map + 2); > - u32 rid_len = be32_to_cpup(map + 3); > + u32 id_len = be32_to_cpup(map + 3); > > - if (rid_base & ~map_mask) { > - pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) > ignores rid-base (0x%x)\n", > + if (id_base & ~map_mask) { > + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) > ignores id-base (0x%x)\n", >
[PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic
There is nothing PCI specific (other than the RID - requester ID) in the of_map_rid() implementation, so the same function can be reused for input/output IDs mapping for other busses just as well. Rename the RID instances/names to a generic "id" tag and provide an of_map_rid() wrapper function so that we can leave the existing (and legitimate) callers unchanged. No functionality change intended. Signed-off-by: Lorenzo Pieralisi Cc: Rob Herring Cc: Joerg Roedel Cc: Robin Murphy Cc: Marc Zyngier --- drivers/iommu/of_iommu.c | 2 +- drivers/of/base.c| 42 include/linux/of.h | 17 +++- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 20738aacac89..ad96b87137d6 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -145,7 +145,7 @@ static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev, struct of_phandle_args iommu_spec = { .args_count = 1 }; int err; - err = of_map_rid(master_np, mc_dev->icid, "iommu-map", + err = of_map_id(master_np, mc_dev->icid, "iommu-map", "iommu-map-mask", _spec.np, iommu_spec.args); if (err) diff --git a/drivers/of/base.c b/drivers/of/base.c index ae03b1218b06..e000e17bd602 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -2201,15 +2201,15 @@ int of_find_last_cache_level(unsigned int cpu) } /** - * of_map_rid - Translate a requester ID through a downstream mapping. + * of_map_id - Translate a requester ID through a downstream mapping. * @np: root complex device node. - * @rid: device requester ID to map. + * @id: device ID to map. * @map_name: property name of the map to use. * @map_mask_name: optional property name of the mask to use. * @target: optional pointer to a target device node. * @id_out: optional pointer to receive the translated ID. * - * Given a device requester ID, look up the appropriate implementation-defined + * Given a device ID, look up the appropriate implementation-defined * platform ID and/or the target device which receives transactions on that * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or * @id_out may be NULL if only the other is required. If @target points to @@ -2219,11 +2219,11 @@ int of_find_last_cache_level(unsigned int cpu) * * Return: 0 on success or a standard error code on failure. */ -int of_map_rid(struct device_node *np, u32 rid, +int of_map_id(struct device_node *np, u32 id, const char *map_name, const char *map_mask_name, struct device_node **target, u32 *id_out) { - u32 map_mask, masked_rid; + u32 map_mask, masked_id; int map_len; const __be32 *map = NULL; @@ -2235,7 +2235,7 @@ int of_map_rid(struct device_node *np, u32 rid, if (target) return -ENODEV; /* Otherwise, no map implies no translation */ - *id_out = rid; + *id_out = id; return 0; } @@ -2255,22 +2255,22 @@ int of_map_rid(struct device_node *np, u32 rid, if (map_mask_name) of_property_read_u32(np, map_mask_name, _mask); - masked_rid = map_mask & rid; + masked_id = map_mask & id; for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) { struct device_node *phandle_node; - u32 rid_base = be32_to_cpup(map + 0); + u32 id_base = be32_to_cpup(map + 0); u32 phandle = be32_to_cpup(map + 1); u32 out_base = be32_to_cpup(map + 2); - u32 rid_len = be32_to_cpup(map + 3); + u32 id_len = be32_to_cpup(map + 3); - if (rid_base & ~map_mask) { - pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n", + if (id_base & ~map_mask) { + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n", np, map_name, map_name, - map_mask, rid_base); + map_mask, id_base); return -EFAULT; } - if (masked_rid < rid_base || masked_rid >= rid_base + rid_len) + if (masked_id < id_base || masked_id >= id_base + id_len) continue; phandle_node = of_find_node_by_phandle(phandle); @@ -2288,20 +2288,20 @@ int of_map_rid(struct device_node *np, u32 rid, } if (id_out) - *id_out = masked_rid - rid_base + out_base; + *id_out = masked_id - id_base + out_base; - pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n", -