This allows fw_devlink to create device links between consumers of
a MSI and the supplier of the MSI.
Signed-off-by: Anup Patel <[email protected]>
---
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;
+}
+
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 4:36 AM Anup Patel <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>
> ---
> 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 <[email protected]> wrote:
>
> On Wed, Jul 19, 2023 at 4:36 AM Anup Patel <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >