linux-kernel - Re: [PATCH 2/2] driver: core: add probe control driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZvUyObXxkU+LKjJD@NAB-HP-ProDesk-600sony.com>
Date: Thu, 26 Sep 2024 15:36:49 +0530
From: Nayeemahmed Badebade <nayeemahmed.badebade@...y.com>
To: Rob Herring <robh@...nel.org>
Cc: krzk+dt@...nel.org, conor+dt@...nel.org, gregkh@...uxfoundation.org,
        rafael@...nel.org, yoshihiro.toyama@...y.com,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] driver: core: add probe control driver

Hi Rob,

Sorry for the delay in our response. 
Please find our reply to your comments.

On Wed, Sep 18, 2024 at 09:55:16AM -0500, Rob Herring wrote:
> > On Thu, Sep 12, 2024 at 03:46:34PM -0500, Rob Herring wrote:
> > > On Wed, Sep 11, 2024 at 07:53:19PM +0530, Nayeemahmed Badebade wrote:
> > > > Probe control driver framework allows deferring the probes of a group of
> > > > devices to an arbitrary time, giving the user control to trigger the probes
> > > > after boot. This is useful for deferring probes from builtin drivers that
> > > > are not required during boot and probe when user wants after boot.
> > > 
> > > This seems like the wrong way around to me. Why not define what you want 
> > > to probe first or some priority order? I could see use for kernel to 
> > > probe whatever is the console device first. Or the rootfs device... You 
> > > don't need anything added to DT for those.
> > > 
> > > Of course, there's the issue that Linux probes are triggered bottom-up 
> > > rather than top-down.
> > > 
> > 
> > Our intention is to only postpone some driver probes not required during
> > boot, similar to https://elinux.org/Deferred_Initcalls. But instead of
> > delaying initcall execution(which requires initmem to be kept and not
> > freed during boot) we are trying to delay driver probes as this is much
> > simpler.
> > 
> > > 
> > > > This is achieved by adding a dummy device aka probe control device node
> > > > as provider to a group of devices(consumer nodes) in platform's device
> > > > tree. Consumers are the devices we want to probe after boot.
> > > 
> > > There's the obvious question of then why not make those devices modules 
> > > instead of built-in?
> > > 
> > 
> > Yes we can use modules for this, but there are drivers that cannot be
> > built as modules and this framework is specifically for such scenario.
> > Example: drivers/pci/controller/dwc/pci-imx6.c
> 
> Then fix the driver to work as a module. Or to use async probe which is 
> not the default and is opt-in per driver.
> 

Sure, we will try to fix the driver and also explore how async probe can be
used for this kind of scenario.

> > 
> > > > 
> > > > To establish control over consumer device probes, each consumer device node
> > > > need to refer the probe control provider node by the phandle.
> > > > 'probe-control-supply' property is used for this.
> > > > 
> > > > Example:
> > > >     // The node below defines a probe control device/provider node
> > > >     prb_ctrl_dev_0: prb_ctrl_dev_0 {
> > > >         compatible = "linux,probe-control";
> > > >     };
> > > > 
> > > >     // The node below is the consumer device node that refers to provider
> > > >     // node by its phandle and a result will not be probed until provider
> > > >     // node is probed.
> > > >     pcie@...c000 {
> > > >         reg = <0x01ffc000 0x04000>, <0x01f00000 0x80000>;
> > > >         #address-cells = <3>;
> > > >         #size-cells = <2>;
> > > >         device_type = "pci";
> > > >         ranges = <0x81000000 0 0          0x01f80000 0 0x00010000>,
> > > >                  <0x82000000 0 0x01000000 0x01000000 0 0x00f00000>;
> > > > 
> > > >         probe-control-supply = <&prb_ctrl_dev_0>;
> > > >     };
> > > 
> > > Sorry, but this isn't going to happen in DT.
> > > 
> > 
> > You mean we cannot add custom properties like this to an existing
> > device node in DT?
> 
> Sure, you can add properties. It happens all the time. This is too tied 
> to some OS implementation/behavior and therefore is not appropriate for 
> DT.
> 
> Rob

We understand now that this approach is not appropriate for DT.
Thank you for your feedback.

Regards,
Nayeem