| Message ID | 20200417165442.1856-1-nsaenzjulienne@suse.de |
|---|---|
| Headers | show |
| Series | of: property: fw_devlink misc fixes | expand |
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. Regards, Nicolas
On Fri, Apr 17, 2020 at 11:06 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.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? -Saravana
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@suse.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, 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. 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. 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. Anyway, if something comes to mind to solve RPi4's situation feel free to propose anything :). Regards, Nicolas
On Mon, Apr 20, 2020 at 4:29 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.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@suse.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
On Mon, 2020-04-20 at 15:37 -0700, Saravana Kannan wrote: [...] > On Mon, Apr 20, 2020 at 4:29 AM Nicolas Saenz Julienne > > 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. Yes that's the idea, and here's the patch I'm trying to avoid: https://lkml.org/lkml/2020/3/24/1508 > > 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)? So the PCI driver is pcie-brcmstb.c, its DT node is available in bcm2711.dtsi (search for pcie0), the firmware interface is defined in bcm2835-rpi.dtsi (search for firmware). > > 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. Yes, I agree. Regards, Nicolas
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. --- Changes since v1: - Address Saravana's comments - Drop patch #3 & #4 Nicolas Saenz Julienne (2): of: property: Fix create device links for all child-supplier dependencies of: property: Do not link to disabled devices drivers/of/property.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)