Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

[Saravana Kannan <saravanak@xxxxxxxxxx>]

On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren <tony@xxxxxxxxxxx> wrote:
>
> * Saravana Kannan <saravanak@xxxxxxxxxx> [220623 08:17]:
> > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > >
> > > * Saravana Kannan <saravanak@xxxxxxxxxx> [220622 19:05]:
> > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > > > > This issue is no directly related fw_devlink. It is a side effect of
> > > > > removing driver_deferred_probe_check_state(). We no longer return
> > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > >
> > > > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > > > was deleted because fw_devlink=on should have short circuited the
> > > > probe attempt with an  -EPROBE_DEFER before reaching the bus/driver
> > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > asking the other questions.
> > >
> > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > driver_deferred_probe_check_state() then?
> >
> > device_links_check_suppliers() call inside really_probe() would short
> > circuit and return an -EPROBE_DEFER if the device links are created as
> > expected.
>
> OK
>
> > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > the booting case without your patches. I see the suppliers for the
> > > ocp child device instances only.
> >
> > Hmmm... this is strange (that the device link isn't there), but this
> > is what I suspected.
>
> Yup, maybe it's because of the supplier being a device in the child
> interconnect for the ocp.

Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
isn't being created.

So the aggregated view is something like (I had to set tabs = 4 space
to fit it within 80 cols):

    ocp: ocp {         <========================= Consumer
        compatible = "simple-pm-bus";
        power-domains = <&prm_per>; <=========== Supplier ref

                l4_wkup: interconnect@44c00000 {
            compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";

            segment@200000 {  /* 0x44e00000 */
                compatible = "simple-pm-bus";

                target-module@0 { /* 0x44e00000, ap 8 58.0 */
                    compatible = "ti,sysc-omap4", "ti,sysc";

                    prcm: prcm@0 {
                        compatible = "ti,am3-prcm", "simple-bus";

                        prm_per: prm@c00 { <========= Actual Supplier
                            compatible = "ti,am3-prm-inst", "ti,omap-prm-inst";
                        };
                    };
                };
            };
        };
    };

The power-domain supplier is the great-great-great-grand-child of the
consumer. It's not clear to me how this is valid. What does it even
mean?

Rob, is this considered a valid DT?

Geert, thoughts on whether this is a correct use of simple-pm-bus device?

Also, how is the power domain attach/get working in this case? As far
as I can tell, at least for "simple-pm-bus" devices, the pm domain
attachment is happening under:
really_probe() -> call_driver_probe -> platform_probe() ->
dev_pm_domain_attach()

So, how is the pm domain attach succeeding in the first place without
my changes?

> > Now we need to figure out why it's missing. There are only a few
> > things that could cause this and I don't see any of those. I already
> > checked to make sure the power domain in this instance had a proper
> > driver with a probe() function -- if it didn't, then that's one thing
> > that'd could have caused the missing device link. The device does seem
> > to have a proper driver, so looks like I can rule that out.
> >
> > Can you point me to the dts file that corresponds to the specific
> > board you are testing this one? I probably won't find anything, but I
> > want to rule out some of the possibilities.
>
> You can use the beaglebone black dts for example, that's
> arch/arm/boot/dts/am335x-boneblack.dts and uses am33xx.dtsi for
> ocp interconnect with simple-pm-bus.
>
> > All the device link creation logic is inside drivers/base/core.c. So
> > if you can look at the existing messages or add other stuff to figure
> > out why the device link isn't getting created, that'd be handy. In
> > either case, I'll continue staring at the DT and code to see what
> > might be happening here.
>
> In device_links_check_suppliers() I see these ocp suppliers:
>
> platform ocp: device_links_check_suppliers: 1024: supplier 44e00d00.prm: link->status: 0 link->flags: 000001c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e01000.prm: link->status: 0 link->flags: 000001c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e00c00.prm: link->status: 0 link->flags: 000001c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e00e00.prm: link->status: 0 link->flags: 000001c0
> platform ocp: device_links_check_suppliers: 1024: supplier 44e01100.prm: link->status: 0 link->flags: 000001c0
> platform ocp: device_links_check_suppliers: 1024: supplier fixedregulator0: link->status: 1 link->flags: 000001c0
>
> No -EPROBE_DEFER is returned in device_links_check_suppliers() for
> 44e00c00.prm supplier for beaglebone black for example, 0 gets
> returned.

Yeah, the "1c0" flags are SYNC_STATE_ONLY device links and aren't
relevant to the issue we are seeing. Those links are being created as
a proxy for other descendant devices of ocp that haven't been added
yet, but are consumers of these *.prm devices. They are mainly meant
for correctness of sync_state() callbacks of the supplier and don't
affect probe order. For example: target-module@56000000 is a consumer
of prm_gfx 44e01100.prm.

-Saravana