-
Notifications
You must be signed in to change notification settings - Fork 6.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
devicetree: add phandles support for chosen properties #84890
base: main
Are you sure you want to change the base?
Conversation
aa390e4
to
646a504
Compare
646a504
to
82a8e79
Compare
This commit adds the necessary Python code to parse chosen properties that have multiple phandles as a value. Also adds devicetree macros to get nodes from chosen properties. Other chosen props are treated as if they are Phandles with 1 element, and as such, a '0' index is appended to them in generated devicetree header CMake DTS data. Signed-off-by: Abderrahmane JARMOUNI <[email protected]>
82a8e79
to
423fe0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! I understand the goal is to support multiple displays, and the current zephyr,display
chosen node is not enough.
I'm not sure about the approach in this PR, though. Let's discuss that first before we get into the specifics of the code changes here.
First, I can't find any precedent in linux for multiple-phandle-valued properties in chosen, and I'm reluctant to make zephyr's devicetree dialect even more different than what is in linux. See https://docs.zephyrproject.org/latest/build/dts/design.html#source-compatibility-with-other-operating-systems.
Second, I'm not sure this extension is necessary. It seems like zephyr,display
could instead be generalized to a new zephyr,displays
chosen node, pointing to a node that contains the relevant phandles
property instead, without having to extend the magic features in our chosen node support:
{
chosen { zephyr,displays = &displays; };
displays: my-displays { displays = <d1 d2 d3>; };
};
I get that it's a little clunkier syntax-wise, but I'm not sure that avoiding the extra typing alone is worth the extra "magic", yet.
Did you consider that? If so, why was it rejected? Did you consider any other implementations?
Putting a -1 on here for now until we can resolve the approach for enabling the multiple displays feature.
@mbolivar Thank you for the quick and detailed feedback!
Yeah I thought about this too to be honest, seems like a valid thing to consider...
No I actually haven't, thanks for the suggestion, will try it and see if it can be used easily for multi-display support. |
This PR adds the necessary Python code to parse chosen properties that have multiple phandles as a value.
Also adds DeviceTree macros to get node at an index from a chosen property.
Other chosen properties are treated as if they are of type Phandles with 1 element, and as such, a '0' index is appended to them in generated DeviceTree header and CMake DTS data.
This will be usefull for adding LVGL multi-display support (#83567) and being able to get the display devices and their number from DT directly to create the buffers statically for all displays.