Message-ID: <CAGETcx-9VbdgNUUstbrGzoAA=4s+EPCznG9LrDoj+Gz-C5g__A@mail.gmail.com>
Date:   Mon, 20 Apr 2020 15:37:54 -0700
From:   Saravana Kannan <saravanak@...gle.com>
To:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
Cc:     Frank Rowand <frowand.list@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 0/2] of: property: fw_devlink misc fixes

On Mon, Apr 20, 2020 at 4:29 AM Nicolas Saenz Julienne
<nsaenzjulienne@...e.de> wrote:
>
> Hi Saravana,
>
> On Fri, 2020-04-17 at 13:55 -0700, Saravana Kannan wrote:
> > On Fri, Apr 17, 2020 at 11:06 AM Nicolas Saenz Julienne
> > <nsaenzjulienne@...e.de> wrote:
> > > Hi Saravana,
> > >
> > > On Fri, 2020-04-17 at 18:54 +0200, Nicolas Saenz Julienne wrote:
> > > > As I'm interested in using this feature to fine-tune Raspberry Pi 4's
> > > > device probe dependencies, I tried to get the board to boot with
> > > > fw_devlink=on. As of today's linux-next the board won't boot with that
> > > > option. I tried to address the underlying issues.
> > > >
> > >
> > > On a semi-related topic, have you looked at vendor specific properties? most
> > > of
> > > them create a consumer/supplier relationship, it'd be nice to be able to
> > > take
> > > those ones into account as well.
> >
> > I'm on the wall about that. If we take every vendor specific property,
> > this file will explode. Not sure I want to do that.
> >
> > Also, we haven't even finished all the generic bindings. I'm just
> > adding bindings as I get familiar with each of them and I test them on
> > hardware I have lying around before sending it out. So, there's where
> > my focus is right now wrt fw_devlink and DT.
> >
> > I wonder how many of the vendor specific properties do very similar
> > things and got in over time. Maybe they can be made generic? What one
> > did you have in mind?
>
> Well, long story short, we need to create a relationship between RPi4's PCI bus
> (which hangs from an interconnect in DT) and RPi4's co-processor, which has a
> highly unconventional firmware driver (raspberrypi.c in drivers/firmware). The
> PCI bus just needs the co-processor interface to be up before probing,

I'm guessing it still works fine today by doing a deferred probe and
you are just trying to avoid having to do a deferred probe? I haven't
kept track of RPi4's upstream support status.

> that's
> all (I'll spare you the details of why). Ideally we want to avoid adding
> platform specific code into an otherwise generic bus driver as it'll be used by
> a number of unrelated SoCs, and it's generally frowned upon.

Which PCI driver is that specifically (I'm sure I can dig around to
find RPi4's DT and figure it out, but it's easier to just ask :) ) ?
Also, can you point me to the DT and the nodes that we are talking
about here (the PCI and the firmware nodes)?

> There is no generic property to handle this case, and it's very unlikely there
> will ever be one, since these firmware drivers have very little in common. I
> guess this could make an argument for a generic _last resort only_
> 'supplied-by' property, but I bet this solution won't be very popular.

Ha, this was my initial idea for the whole fw_devlink feature. I
called it depends-on. Rob/Frank convinced me to instead just parse the
existing bindings -- which was definitely the right call. Otherwise DT
would have been a mess. Adding support for "depends-on" for one off
use cases might still be a touchy topic. I myself am on the wall. It's
useful for some rare cases, but it's also very easy to abuse.

> Another idea that comes to mind for vendor specific properties would be
> exporting a macro in the lines of "DEFINE_SIMPLE_PROP()" for supplier drivers
> to define custom properties. The parse_prop() callbacks could then be added
> into a special section for of/property.c to pickup and parse. The good thing is
> that the list length would be limited by the kernel configuration and the
> maintenance burden moved to the driver authors, at least to some extent.

I did think about this option too sometime back, but not too keen on
that -- at least at this point. Mostly because there are other issues
I want to resolve first and want to get their design right before
getting to hand off property parsing to other drivers/files.

> Anyway, if something comes to mind to solve RPi4's situation feel free to
> propose anything :).

I'll let you know if I do. I'm not sure I fully understand the issue
though. I don't think you are trying to avoid deferred probes because
you aren't trying to improve boot time. I'm guessing the PCI bus
driver in question has no way to know when to defer the probe, in this
case you are trying to fix?

-Saravana

Powered by blists - more mailing lists