On Mon, Mar 6, 2017 at 9:41 PM, David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Mar 06, 2017 at 04:48:18AM -0600, Rob Herring wrote:
>> On Thu, Mar 2, 2017 at 8:12 PM, David Gibson
>> <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> > On Tue, Feb 28, 2017 at 04:43:09PM -0600, Rob Herring wrote:
>> >> Add checks to identify simple-bus bus types and checks for child
>> >> devices. Simple-bus type is generally identified by "simple-bus"
>> >> compatible string. We also treat the root as a simple-bus, but only for
>> >> child nodes with reg property.
>> >>
>> >> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>> >> ---
>> >> v2:
>> >> - new patch
>> >>
>> >> checks.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> 1 file changed, 69 insertions(+)
>> >>
>> >> diff --git a/checks.c b/checks.c
>> >> index 5ed91ac50a10..c4865b4c8da0 100644
>> >> --- a/checks.c
>> >> +++ b/checks.c
>> >> @@ -817,6 +817,72 @@ static void check_pci_device_reg(struct check *c, struct dt_info *dti, struct no
>> >> }
>> >> WARNING(pci_device_reg, check_pci_device_reg, NULL, ®_format);
>> >>
>> >> +static const struct bus_type simple_bus = {
>> >> + .name = "simple-bus",
>> >> +};
>> >> +
>> >> +static bool node_is_compatible(struct node *node, const char *compat)
>> >> +{
>> >> + struct property *prop;
>> >> + const char *str;
>> >> +
>> >> + prop = get_property(node, "compatible");
>> >> + if (!prop)
>> >> + return false;
>> >> +
>> >> + for (str = prop->val.val; str < prop->val.val + prop->val.len; str += strlen(str) + 1) {
>> >
>> > This isn't safe if the compatible property is filled with garbage (not
>> > '\0' terminated) - the strlen() could access beyond the end of the
>> > property value.
>>
>> Okay, I guess I can check that prop->val.val[prop->val.len - 1] == 0 up front.
>
> Sure. Or use strnlen.
Duh...
>> >> + if (streq(str, compat))
>> >> + return true;
>> >> + }
>> >> + return false;
>> >> +}
>> >> +
>> >> +static void check_simple_bus_bridge(struct check *c, struct dt_info *dti, struct node *node)
>> >> +{
>> >> + if (node_is_compatible(node, "simple-bus") || !node->parent)
>> >> + node->bus = &simple_bus;
>> >
>> > I don't think it's correct to assume the root bus is always a
>> > simple-bus. If it is, it really should be listed explicitly in the
>> > root node's compatible property.
>>
>> It is in the sense that Linux treats the root the same and creates
>> devices for top level children and then descends for nodes with
>> "simple-bus".
>
> Hmm.. where in Linux is that? I think that's a bug, technically
> speaking, traversing the root node's children without regard to the
> type of the root node.
drivers/of/platform.c:of_platform_populate() which is called on the
root node with "simple-bus" in the match table.
Plus I know we have some DT's like Tegra that didn't put all their
devices under a bus (but should have). Maybe I should warn on that
(i.e. warn on having unit-addresses without a bus type set on root).
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
