| Message ID | 1569854031-237636-2-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SMMUv3 PMCG IMP DEF event support | expand |
Hi John, On 2019/9/30 22:33, John Garry wrote: > In the IORT, a PMCG node includes a node reference to its associated > device. > > Set the PMCG platform device parent device for future referencing. > > For now, we only consider setting for when the associated component is an > SMMUv3. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > drivers/acpi/arm64/iort.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index 8569b79e8b58..0b687520c3e7 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -1455,7 +1455,7 @@ static __init const struct iort_dev_config *iort_get_dev_cfg( > * Returns: 0 on success, <0 failure ... > */ > static int __init iort_add_platform_device(struct acpi_iort_node *node, > - const struct iort_dev_config *ops) > + const struct iort_dev_config *ops, struct device *parent) Since you added a input for this function, could you please update the comments of this function as well? > { > struct fwnode_handle *fwnode; > struct platform_device *pdev; > @@ -1466,6 +1466,8 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node, > if (!pdev) > return -ENOMEM; > > + pdev->dev.parent = parent; > + > if (ops->dev_set_proximity) { > ret = ops->dev_set_proximity(&pdev->dev, node); > if (ret) > @@ -1573,6 +1575,11 @@ static void __init iort_enable_acs(struct acpi_iort_node *iort_node) > static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { } > #endif > > +static int iort_fwnode_match(struct device *dev, const void *fwnode) > +{ > + return dev->fwnode == fwnode; > +} > + > static void __init iort_init_platform_devices(void) > { > struct acpi_iort_node *iort_node, *iort_end; > @@ -1594,11 +1601,34 @@ static void __init iort_init_platform_devices(void) > iort_table->length); > > for (i = 0; i < iort->node_count; i++) { > + struct device *parent = NULL; > + > if (iort_node >= iort_end) { > pr_err("iort node pointer overflows, bad table\n"); > return; > } > > + /* Fixme: handle parent declared in IORT after PMCG */ > + if (iort_node->type == ACPI_IORT_NODE_PMCG) { > + struct acpi_iort_node *iort_assoc_node; > + struct acpi_iort_pmcg *pmcg; > + u32 node_reference; > + > + pmcg = (struct acpi_iort_pmcg *)iort_node->node_data; > + > + node_reference = pmcg->node_reference; > + iort_assoc_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, > + node_reference); > + > + if (iort_assoc_node->type == ACPI_IORT_NODE_SMMU_V3) { > + struct fwnode_handle *assoc_fwnode; > + > + assoc_fwnode = iort_get_fwnode(iort_assoc_node); > + > + parent = bus_find_device(&platform_bus_type, NULL, > + assoc_fwnode, iort_fwnode_match); > + } > + } How about using a function to include those new added code to make this function (iort_init_platform_devices()) a bit cleaner? > iort_enable_acs(iort_node); > > ops = iort_get_dev_cfg(iort_node); > @@ -1609,7 +1639,7 @@ static void __init iort_init_platform_devices(void) > > iort_set_fwnode(iort_node, fwnode); > > - ret = iort_add_platform_device(iort_node, ops); > + ret = iort_add_platform_device(iort_node, ops, parent); This function is called if ops is valid, so retrieve the parent can be done before this function I think. Thanks Hanjun
Hi Hanjun, Thanks for checking this. >> */ >> static int __init iort_add_platform_device(struct acpi_iort_node *node, >> - const struct iort_dev_config *ops) >> + const struct iort_dev_config *ops, struct device *parent) > > Since you added a input for this function, could you please update > the comments of this function as well? Right, that can be updated. Indeed, the current comment omit the @ops argument also. > >> { >> struct fwnode_handle *fwnode; >> struct platform_device *pdev; >> @@ -1466,6 +1466,8 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node, >> if (!pdev) >> return -ENOMEM; >> >> + pdev->dev.parent = parent; >> + >> if (ops->dev_set_proximity) { >> ret = ops->dev_set_proximity(&pdev->dev, node); >> if (ret) >> @@ -1573,6 +1575,11 @@ static void __init iort_enable_acs(struct acpi_iort_node *iort_node) >> static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { } >> #endif >> >> +static int iort_fwnode_match(struct device *dev, const void *fwnode) >> +{ >> + return dev->fwnode == fwnode; >> +} >> + >> static void __init iort_init_platform_devices(void) >> { >> struct acpi_iort_node *iort_node, *iort_end; >> @@ -1594,11 +1601,34 @@ static void __init iort_init_platform_devices(void) >> iort_table->length); >> >> for (i = 0; i < iort->node_count; i++) { >> + struct device *parent = NULL; >> + >> if (iort_node >= iort_end) { >> pr_err("iort node pointer overflows, bad table\n"); >> return; >> } >> >> + /* Fixme: handle parent declared in IORT after PMCG */ >> + if (iort_node->type == ACPI_IORT_NODE_PMCG) { >> + struct acpi_iort_node *iort_assoc_node; >> + struct acpi_iort_pmcg *pmcg; >> + u32 node_reference; >> + >> + pmcg = (struct acpi_iort_pmcg *)iort_node->node_data; >> + >> + node_reference = pmcg->node_reference; >> + iort_assoc_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, >> + node_reference); >> + >> + if (iort_assoc_node->type == ACPI_IORT_NODE_SMMU_V3) { >> + struct fwnode_handle *assoc_fwnode; >> + >> + assoc_fwnode = iort_get_fwnode(iort_assoc_node); >> + >> + parent = bus_find_device(&platform_bus_type, NULL, >> + assoc_fwnode, iort_fwnode_match); >> + } >> + } > > How about using a function to include those new added code to make this > function (iort_init_platform_devices()) a bit cleaner? > Can do. But I still need to add code to deal with the scenario of the parent PMCG not being an SMMU, which is supported in the spec as I recall. Note that I would not have FW to test that, so maybe can omit support for now as long as there's no regression. >> iort_enable_acs(iort_node); >> >> ops = iort_get_dev_cfg(iort_node); >> @@ -1609,7 +1639,7 @@ static void __init iort_init_platform_devices(void) >> >> iort_set_fwnode(iort_node, fwnode); >> >> - ret = iort_add_platform_device(iort_node, ops); >> + ret = iort_add_platform_device(iort_node, ops, parent); > > This function is called if ops is valid, so retrieve the parent > can be done before this function I think. Ah, yes Thanks, John > > Thanks > Hanjun > > > . >
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 8569b79e8b58..0b687520c3e7 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1455,7 +1455,7 @@ static __init const struct iort_dev_config *iort_get_dev_cfg( * Returns: 0 on success, <0 failure */ static int __init iort_add_platform_device(struct acpi_iort_node *node, - const struct iort_dev_config *ops) + const struct iort_dev_config *ops, struct device *parent) { struct fwnode_handle *fwnode; struct platform_device *pdev; @@ -1466,6 +1466,8 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node, if (!pdev) return -ENOMEM; + pdev->dev.parent = parent; + if (ops->dev_set_proximity) { ret = ops->dev_set_proximity(&pdev->dev, node); if (ret) @@ -1573,6 +1575,11 @@ static void __init iort_enable_acs(struct acpi_iort_node *iort_node) static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { } #endif +static int iort_fwnode_match(struct device *dev, const void *fwnode) +{ + return dev->fwnode == fwnode; +} + static void __init iort_init_platform_devices(void) { struct acpi_iort_node *iort_node, *iort_end; @@ -1594,11 +1601,34 @@ static void __init iort_init_platform_devices(void) iort_table->length); for (i = 0; i < iort->node_count; i++) { + struct device *parent = NULL; + if (iort_node >= iort_end) { pr_err("iort node pointer overflows, bad table\n"); return; } + /* Fixme: handle parent declared in IORT after PMCG */ + if (iort_node->type == ACPI_IORT_NODE_PMCG) { + struct acpi_iort_node *iort_assoc_node; + struct acpi_iort_pmcg *pmcg; + u32 node_reference; + + pmcg = (struct acpi_iort_pmcg *)iort_node->node_data; + + node_reference = pmcg->node_reference; + iort_assoc_node = ACPI_ADD_PTR(struct acpi_iort_node, iort, + node_reference); + + if (iort_assoc_node->type == ACPI_IORT_NODE_SMMU_V3) { + struct fwnode_handle *assoc_fwnode; + + assoc_fwnode = iort_get_fwnode(iort_assoc_node); + + parent = bus_find_device(&platform_bus_type, NULL, + assoc_fwnode, iort_fwnode_match); + } + } iort_enable_acs(iort_node); ops = iort_get_dev_cfg(iort_node); @@ -1609,7 +1639,7 @@ static void __init iort_init_platform_devices(void) iort_set_fwnode(iort_node, fwnode); - ret = iort_add_platform_device(iort_node, ops); + ret = iort_add_platform_device(iort_node, ops, parent); if (ret) { iort_delete_fwnode(iort_node); acpi_free_fwnode_static(fwnode);
In the IORT, a PMCG node includes a node reference to its associated device. Set the PMCG platform device parent device for future referencing. For now, we only consider setting for when the associated component is an SMMUv3. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/acpi/arm64/iort.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)