Page MenuHomeFreeBSD

Automatic dependency tracking for FDT/OFW.
AcceptedPublic

Authored by ray on Aug 30 2019, 10:47 PM.

Details

Summary

There is a lot of tricks on how to handle case if required device come later than device which need it.
That patch make it automatically by checking standard props:

  • interrupts
  • clocks
  • power-domains
  • resets
  • phys
  • dmas
  • gpios

Plus it track dependency via special prop "freebsd,phandle". My board has broken generic_timer, which require to enable first 64bits timer of "Generic Purpose Timer" device. So I add such code to platform_attach sub:

phandle_t gpt, gt;

gt = OF_finddevice("/timer");
gpt = OF_finddevice("/timer@10008000");
if (gt && gpt && gt != gpt) {
        gpt = OF_xref_from_node(gpt);
        gpt = cpu_to_fdt32(gpt);
        OF_setprop(gt, "freebsd,phandle", &gpt, sizeof(gpt));
}

Now ofwbus attach mtk_gpt first, mtk_gpt enables 64bits counter and then, ofwbus attach generic_timer.
Subsystem can be enabled by adding option OFWBUS_AUTODEP to kernel config file.
If you need reoder generic_timer, you also need to do something with its EARLY_DRIVER_MODULE(...., BUS_PASS_TIMER ...). For that I prepare simple helper which redefine BUS_PASS_TIMER as BUS_PASS_DEFAULT. It can be enabled with kernel config option OFWBUS_AUTODEP_BUS_PASS_OVERRIDE.
Since many vendors use own props and some of them to define required module, like our syscon(extres), I did linker list which can be added to platform_attach that way:

static struct ofwbus_static_req_props mtk_gpio_req_props = {
        "mediatek,pctl-regmap",
        NULL,
        OFWBUS_REQ_FIND_PROP,
};

OFWBUS_STATIC_REQ_PROP(mtk_gpio_req_props);

This way I make gpio controller driver to use syscon defined lower in DTS.

It currently limited to track dependency on single bus.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26203
Build 24708: arc lint + arc unit

Event Timeline

ray created this revision.Aug 30 2019, 10:47 PM
ray edited the summary of this revision. (Show Details)Aug 30 2019, 10:54 PM
ray added a reviewer: manu.Aug 30 2019, 11:01 PM
ray updated this revision to Diff 61497.Aug 30 2019, 11:09 PM

s/Firware/Firmware/

mizhka accepted this revision.Sep 4 2019, 7:54 AM
This revision is now accepted and ready to land.Sep 4 2019, 7:54 AM

I have pretty mixed feelings about this approach. It only works for ofwbus children, not simplebus children, and canonicalizes a bunch of behaviors that I don't believe are standards (I'm in an airport departure lounge and don't have the spec handy). I'm also not really sure how it interacts with multipass etc. Could you elaborate a little more on the mechanism?

An alternative that I have been discussing with Warner would be to allow DEVICE_ATTACH() to return EAGAIN. Then you could implement this with a helper function (ofwbus_check_deps() or something) called from the children's DEVICE_ATTACH(). I think it's a wash in terms of code length and makes the mechanism both more general and would waive my concern about standardization of non-standards-based behaviors since the operation moves from the parent to the child. Would that work for you?

ray added a comment.Sep 7 2019, 8:41 AM

I have pretty mixed feelings about this approach. It only works for ofwbus children, not simplebus children, and canonicalizes a bunch of behaviors that I don't believe are standards (I'm in an airport departure lounge and don't have the spec handy). I'm also not really sure how it interacts with multipass etc. Could you elaborate a little more on the mechanism?

Hehe, all my drivers declared as children of simplebus. I even more worried about real ofwbus children.
Indeed, things like "dmas" are not standard. But not sure if it can broke something.
Did not check yet why, but very early drivers attached first, that why I did second config option.
Mechanism not so complex:

  1. Every driver instance check if added devs require it;
  2. Then find topmost entry of all requiring and add self before.

An alternative that I have been discussing with Warner would be to allow DEVICE_ATTACH() to return EAGAIN. Then you could implement this with a helper function (ofwbus_check_deps() or something) called from the children's DEVICE_ATTACH(). I think it's a wash in terms of code length and makes the mechanism both more general and would waive my concern about standardization of non-standards-based behaviors since the operation moves from the parent to the child. Would that work for you?

I read you "delayed attach" patch and had some objections, but after two weeks I forget them :)
So, I agree, I will recheck your patch and come back with results.

Thanks for review Nathan!