| Message ID | 20230719113542.2293295-3-apatel@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Linux RISC-V AIA Support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 44 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Wed, Jul 19, 2023 at 4:36 AM Anup Patel <apatel@ventanamicro.com> wrote: > > This allows fw_devlink to create device links between consumers of > a MSI and the supplier of the MSI. > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index ddc75cd50825..e4096b79a872 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np, > return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; > } > > +static struct device_node *parse_msi_parent(struct device_node *np, > + const char *prop_name, int index) > +{ > + struct of_phandle_args sup_args; > + struct device_node *msi_np; > + > + if (!IS_ENABLED(CONFIG_OF_IRQ)) > + return NULL; > + > + if (strcmp(prop_name, "msi-parent")) > + return NULL; > + > + msi_np = of_parse_phandle(np, prop_name, 0); > + if (msi_np) { > + if (!of_property_read_bool(msi_np, "#msi-cells")) { > + if (index) { > + of_node_put(msi_np); > + return NULL; > + } > + return msi_np; > + } > + of_node_put(msi_np); > + } > + > + if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index, > + &sup_args)) > + return NULL; > + > + return sup_args.np; > +} > + I'm amazed by the different ways you choose to waste people's time. Did you even scroll up to see how the other properties are handled? Why can't this be handled using DEFINE_SIMPLE_PROP macro? -Saravana > static const struct supplier_bindings of_supplier_bindings[] = { > { .parse_prop = parse_clocks, }, > { .parse_prop = parse_interconnects, }, > @@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { > { .parse_prop = parse_regulators, }, > { .parse_prop = parse_gpio, }, > { .parse_prop = parse_gpios, }, > + { .parse_prop = parse_msi_parent, }, > {} > }; > > -- > 2.34.1 >
On Wed, Jul 19, 2023 at 05:05:30PM +0530, Anup Patel wrote: > This allows fw_devlink to create device links between consumers of > a MSI and the supplier of the MSI. > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/of/property.c b/drivers/of/property.c > index ddc75cd50825..e4096b79a872 100644 > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np, > return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; > } > > +static struct device_node *parse_msi_parent(struct device_node *np, > + const char *prop_name, int index) > +{ > + struct of_phandle_args sup_args; > + struct device_node *msi_np; > + > + if (!IS_ENABLED(CONFIG_OF_IRQ)) > + return NULL; Why do we need this? Sparc is not going to have MSI properties to begin with. I guess it saves a little bit of code. Though Sparc doesn't need any of this file. Or maybe there's a better kconfig symbol to use here if MSIs are not supported? > + > + if (strcmp(prop_name, "msi-parent")) > + return NULL; > + > + msi_np = of_parse_phandle(np, prop_name, 0); > + if (msi_np) { > + if (!of_property_read_bool(msi_np, "#msi-cells")) { > + if (index) { > + of_node_put(msi_np); > + return NULL; > + } > + return msi_np; > + } > + of_node_put(msi_np); > + } > + > + if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index, > + &sup_args)) > + return NULL; > + > + return sup_args.np; > +} > + > static const struct supplier_bindings of_supplier_bindings[] = { > { .parse_prop = parse_clocks, }, > { .parse_prop = parse_interconnects, }, > @@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { > { .parse_prop = parse_regulators, }, > { .parse_prop = parse_gpio, }, > { .parse_prop = parse_gpios, }, > + { .parse_prop = parse_msi_parent, }, > {} > }; > > -- > 2.34.1 >
On Thu, Jul 20, 2023 at 3:55 AM Saravana Kannan <saravanak@google.com> wrote: > > On Wed, Jul 19, 2023 at 4:36 AM Anup Patel <apatel@ventanamicro.com> wrote: > > > > This allows fw_devlink to create device links between consumers of > > a MSI and the supplier of the MSI. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > index ddc75cd50825..e4096b79a872 100644 > > --- a/drivers/of/property.c > > +++ b/drivers/of/property.c > > @@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np, > > return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; > > } > > > > +static struct device_node *parse_msi_parent(struct device_node *np, > > + const char *prop_name, int index) > > +{ > > + struct of_phandle_args sup_args; > > + struct device_node *msi_np; > > + > > + if (!IS_ENABLED(CONFIG_OF_IRQ)) > > + return NULL; > > + > > + if (strcmp(prop_name, "msi-parent")) > > + return NULL; > > + > > + msi_np = of_parse_phandle(np, prop_name, 0); > > + if (msi_np) { > > + if (!of_property_read_bool(msi_np, "#msi-cells")) { > > + if (index) { > > + of_node_put(msi_np); > > + return NULL; > > + } > > + return msi_np; > > + } > > + of_node_put(msi_np); > > + } > > + > > + if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index, > > + &sup_args)) > > + return NULL; > > + > > + return sup_args.np; > > +} > > + > > I'm amazed by the different ways you choose to waste people's time. > Did you even scroll up to see how the other properties are handled? > > Why can't this be handled using DEFINE_SIMPLE_PROP macro? DEFINE_SIMPLE_PROP() is not suitable for msi-parent because we have a special case where for a single MSI parent the "#msi-cells" property won't be present in the supplier DT node. The of_msi_get_domain() function also handles this special case separately. Regards, Anup > > -Saravana > > > static const struct supplier_bindings of_supplier_bindings[] = { > > { .parse_prop = parse_clocks, }, > > { .parse_prop = parse_interconnects, }, > > @@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { > > { .parse_prop = parse_regulators, }, > > { .parse_prop = parse_gpio, }, > > { .parse_prop = parse_gpios, }, > > + { .parse_prop = parse_msi_parent, }, > > {} > > }; > > > > -- > > 2.34.1 > >
On Thu, Jul 20, 2023 at 4:08 AM Rob Herring <robh@kernel.org> wrote: > > On Wed, Jul 19, 2023 at 05:05:30PM +0530, Anup Patel wrote: > > This allows fw_devlink to create device links between consumers of > > a MSI and the supplier of the MSI. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > > > diff --git a/drivers/of/property.c b/drivers/of/property.c > > index ddc75cd50825..e4096b79a872 100644 > > --- a/drivers/of/property.c > > +++ b/drivers/of/property.c > > @@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np, > > return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; > > } > > > > +static struct device_node *parse_msi_parent(struct device_node *np, > > + const char *prop_name, int index) > > +{ > > + struct of_phandle_args sup_args; > > + struct device_node *msi_np; > > + > > + if (!IS_ENABLED(CONFIG_OF_IRQ)) > > + return NULL; > > Why do we need this? Sparc is not going to have MSI properties to begin > with. I guess it saves a little bit of code. Though Sparc doesn't need > any of this file. Or maybe there's a better kconfig symbol to use here > if MSIs are not supported? I can't think of a better kconfig symbol over here but since Sparc does not have MSI properties, I think following is better: if (IS_ENABLED(CONFIG_SPARC)) return NULL; Any other suggestions ? Regards, Anup > > > + > > + if (strcmp(prop_name, "msi-parent")) > > + return NULL; > > + > > + msi_np = of_parse_phandle(np, prop_name, 0); > > + if (msi_np) { > > + if (!of_property_read_bool(msi_np, "#msi-cells")) { > > + if (index) { > > + of_node_put(msi_np); > > + return NULL; > > + } > > + return msi_np; > > + } > > + of_node_put(msi_np); > > + } > > + > > + if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index, > > + &sup_args)) > > + return NULL; > > + > > + return sup_args.np; > > +} > > + > > static const struct supplier_bindings of_supplier_bindings[] = { > > { .parse_prop = parse_clocks, }, > > { .parse_prop = parse_interconnects, }, > > @@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { > > { .parse_prop = parse_regulators, }, > > { .parse_prop = parse_gpio, }, > > { .parse_prop = parse_gpios, }, > > + { .parse_prop = parse_msi_parent, }, > > {} > > }; > > > > -- > > 2.34.1 > >
diff --git a/drivers/of/property.c b/drivers/of/property.c index ddc75cd50825..e4096b79a872 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1325,6 +1325,37 @@ static struct device_node *parse_interrupts(struct device_node *np, return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; } +static struct device_node *parse_msi_parent(struct device_node *np, + const char *prop_name, int index) +{ + struct of_phandle_args sup_args; + struct device_node *msi_np; + + if (!IS_ENABLED(CONFIG_OF_IRQ)) + return NULL; + + if (strcmp(prop_name, "msi-parent")) + return NULL; + + msi_np = of_parse_phandle(np, prop_name, 0); + if (msi_np) { + if (!of_property_read_bool(msi_np, "#msi-cells")) { + if (index) { + of_node_put(msi_np); + return NULL; + } + return msi_np; + } + of_node_put(msi_np); + } + + if (of_parse_phandle_with_args(np, prop_name, "#msi-cells", index, + &sup_args)) + return NULL; + + return sup_args.np; +} + static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_clocks, }, { .parse_prop = parse_interconnects, }, @@ -1359,6 +1390,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_regulators, }, { .parse_prop = parse_gpio, }, { .parse_prop = parse_gpios, }, + { .parse_prop = parse_msi_parent, }, {} };
This allows fw_devlink to create device links between consumers of a MSI and the supplier of the MSI. Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- drivers/of/property.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)