On Tue, Jul 2, 2019 at 5:59 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote: > > On Tue, Jul 2, 2019 at 5:03 PM David Collins <collinsd@xxxxxxxxxxxxxx> wrote: > > > > Hello Saravana, > > > > On 7/1/19 5:48 PM, Saravana Kannan wrote: > > ... > > > TODO: > > > - For the case of consumer child sub-nodes being added by a parent > > > device after late_initcall_sync we might be able to address that by > > > recursively parsing all child nodes and adding all their suppliers as > > > suppliers of the parent node too. The parent probe will add the > > > children before its probe is completed and that will prevent the > > > supplier's sync_state from being executed before the children are > > > probed. > > > > > > But I'll write that part once I see how this series is received. > > > > I don't think that this scheme will work in all cases. It can also lead > > to probing deadlock. > > > > Here is an example: > > > > Three DT devices (top level A with subnodes B and C): > > /A > > /A/B > > /A/C > > C is a consumer of B. > > > > When device A is created, a search of its subnodes will find the link from > > C to B. Since device B hasn't been created yet, of_link_to_suppliers() > > will fail and add A to the wait_for_suppliers list. This will cause the > > probe of A to fail with -EPROBE_DEFER (thanks to the check in > > device_links_check_suppliers()). As a result device B will not be created > > and device A will never probe. > > > > You could try to resolve this situation by detecting the cycle and *not* > > adding A to the wait_for_suppliers list. However, that would get us back > > to the problem we had before. A would be allowed to probe which would > > then result in devices being added for B and C. If the device for B is > > added before C, then it would be allowed to immediately probe and > > (assuming this all takes place after late_initcall_sync thanks to modules) > > its sync_state() callback would be called since no consumer devices are > > linked to B. > > > > Please note that to change this example from theoretical to practical, > > replace "A" with apps_rsc, "B" with pmi8998-rpmh-regulators, and "C" with > > pm8998-rpmh-regulators in [1]. > > Interesting use case. > > First, to clarify my TODO: I was initially thinking of the recursive > "up-heritance" of suppliers from child to parent to handle cases where > the supplier is a device from some other top level device (or its > child). My thinking has evolved a bit on that. I think the parent > needs to inherit only from it's immediate children and not its > grandchildren (the child is responsible for handling grandchildren > suppliers). I'll also have to make sure I don't try to create a link > from a parent device to one of its child device nodes (should be easy > to check). > > Anyway, going back to your case, for dependencies between child nodes > of a parent, can't the parent just populate them in the right order? > You can loop through the children and add them in multiple stages. > > I'll continue to think if I can come up with anything nicer on the > drivers, but even if we can't come up with anything better, we can > still make sync_state() work. There's actually a much better way to handle this case where you won't have to handle ordering on the driver side. I just need to add one or two patches to my patch series. I'll send that out sometime next week. -Saravana
