OFW: Add helper functions for parsing xref based lists.
ClosedPublic
Actions

Authored by mmel on Nov 30 2015, 12:41 PM.

Details

Reviewers

kib
nwhitehorn
andrew
imp
• ian

Commits

rS292156: OFW: Add helper functions for parsing xref based lists.

Summary

By using this functions, we can parse a list of tuples, each of them holds
xref and values.
For example:
clk1:{

		#clock-cells = <0>;

};
clk2:{

		#clock-cells = <1>;

};
clk3:{

		#clock-cells = <3>;

};

 	dev: {
		clocks = <&clk1>,
			 <&clk2 1>,
			 <&clk3 2 3>;
		clock-names = "clk_a", "clk_b", "clk_c";

};

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mmel updated this revision to Diff 10594.Nov 30 2015, 12:41 PM

mmel retitled this revision from to OFW: Add helper functions for parsing xref based lists..

mmel updated this object.

mmel edited the test plan for this revision. (Show Details)

mmel added reviewers: imp, andrew, • ian, kib.

mmel set the repository for this revision to rS FreeBSD src repository - subversion.

mmel added a project: ARM.

andrew added a reviewer: nwhitehorn.Nov 30 2015, 12:44 PM

What code do you intend to use this in? The utility function is reasonable, so far as it goes, but it seems like a relatively specialized use case and I'm not sure that much code is saved in the consumers by centralizing this.

This kind of list is widely used in DT for description of consumed resources (in name or index based variant) .
For example, its used by
clocks -> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124.dtsi?revision=279385&view=markup#l42
resets -> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124.dtsi?revision=279385&view=markup#l47
gpios -> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/at91sam9x5.dtsi?revision=279385&view=markup#l1197
phys -> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124.dtsi?revision=279385&view=markup#l53
dmas -> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124.dtsi?revision=279385&view=markup#l304
thermal sensors -> https://svnweb.freebsd.org/base/head/sys/gnu/dts/arm/tegra124.dtsi?revision=279385&view=markup#l919

And probably by many others...

That wasn't really my question: I was wondering where, in the tree, you plan to use it. Parsing these lists is not very complicated in general, and where it is (see inline comment), I think this code won't actually work. It seems like not much actual code will be saved.

What is the ofw_bus_find_string_index() code for?

sys/dev/ofw/ofw_bus_subr.c
627	The whole scheme of OF_searchprop() is faulty here. The property in question may not be on a direct parent node (for example, interrupt-parent tree crosslinks). It would be a better idea to pass the cells value directly. Of course, the number of cells could vary element by element...

• ian added inline comments.Dec 9 2015, 8:27 PM

sys/dev/ofw/ofw_bus_subr.c
627	It looks to me like this is getting the #clock-cells (or #whatever-cells) property from the cross-tree-linked reference. I assume it's using OF_searchencprop() (rather than get) because we used to have to deal with missing #whatever-cells properties. I wonder how much of that was because of our own badly-written dts, and moving to published dts files would let us treat required fields as required instead of this bogus assumption that a parent's value will be right if the child lacks the property. I think the need to do this (retrieve the #x-cells property from each cross-referenced node to know how to interpret the following cells in the array) is why this problem of parsing free-form lists of cross referenced properties isn't as simple as it seems at first glance.

I think the main function is good and should be in dev/ofw. What are the other ones for?

sys/dev/ofw/ofw_bus_subr.c
627	Ah, fair enough. I misread this. Given that, I think this is a good function to have.

In D4316#94205, @nwhitehorn wrote:

I think the main function is good and should be in dev/ofw. What are the other ones for?

linux dts source is always full of lists of names of things, and while I'm not clear on why that is, if there is some reason to access clocks, for example, by name then you'd want a routine that can find "bus_clk" in the array of clock names and tell you it's index 3, so that you can pass that value to ofw_bus_parse_xref_list_alloc() to extract the provider and property cells by index.

sys/dev/ofw/ofw_bus_subr.c
605	handle
610	list_name and cells_name probably should be const char *
659	const char * list_name, name
688	const char * list_name (but no way I'm going to try to figure out if/where any consts might go in char ***array).

The all functions will be used in upcoming clk/reset/regulator/... framework.
I only want to push this utility functions in separate commit.

I assume it's using OF_searchencprop() (rather than get) because we used to have to deal with missing #whatever-cells properties ...

It's hard to say which one (search/get) function is right one. I'm not able to anything about properties inheritance (other than interrupt parent) in device tree. Hmm, maybe it's safer to start with OF_getencprop(), we can switch to OF_searchencprop() at any time in case of troubles.

The name based list is newer variant of index based lists. The index based list cannot express situation where variable number of resources must be passed to driver.
For example - if given IP core(and his driver) supports 3 clocks (but only first is mandatory). Then one SoC may use first and second clock, and another SoC first and third clock.