Skip to content
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: bus: does not work in child-binding #32071

Closed
kurddt opened this issue Feb 8, 2021 · 7 comments · Fixed by #32105
Closed

devicetree: bus: does not work in child-binding #32071

kurddt opened this issue Feb 8, 2021 · 7 comments · Fixed by #32105
Assignees
Labels
area: Devicetree Tooling PR modifies or adds a Device Tree tooling bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@kurddt
Copy link
Contributor

kurddt commented Feb 8, 2021

Describe the bug
I'm trying to have a child binding declare a new bus controller. That does not seems to work as expected, the childs of this new controller does not refers to it in the generated defines.

It also seems that even when using a separate binding file for the controller, it must be on a bus (binding must contains on-bus) otherwise the childs also don't use it as the controller

To Reproduce
dts_debug.zip
Steps to reproduce the behavior:
Run gen_defines with provided dts and .yaml files
The joined archive contains:

  • zephyr.dts: small description containing a soc, a root i2c controller, an i2c mux, i2c devices
  • bindings: binding of the i2c mux using child-bindings
  • bindings_2: bindings of the i2c mux using two separate files
  • debug_child.h: generated header using bindings folder
  • debuf_sep.h: generated header using bindings_2 folder

Expected behavior
Childs of the controller should refers to it in their *_BUS define

Environment (please complete the following information):

  • OS: Linux
  • Toolchain Arm gnu
  • Zephyr v2.4
@kurddt kurddt added the bug The issue is a bug, or the PR is fixing a bug label Feb 8, 2021
@kurddt
Copy link
Contributor Author

kurddt commented Feb 8, 2021

@mbolivar-nordic

@carlescufi carlescufi added the area: Devicetree Tooling PR modifies or adds a Device Tree tooling label Feb 8, 2021
@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Feb 8, 2021

@kurddt Thank you for the detailed report. This is an interesting case; I've never run into the TCA9546A.

Let me confirm the problem.

In debug_sep.h, I see the following bus macros generated for /soc/i2c@40004000/tca9546a@3B/mux_i2c@0/tmp116@49:

/* Bus info (controller: '/soc/i2c@40004000/tca9546a@3B/mux_i2c@0', type: 'i2c') */
#define DT_N_S_soc_S_i2c_40004000_S_tca9546a_3b_S_mux_i2c_0_S_tmp116_49_BUS_i2c 1
#define DT_N_S_soc_S_i2c_40004000_S_tca9546a_3b_S_mux_i2c_0_S_tmp116_49_BUS DT_N_S_soc_S_i2c_40004000_S_tca9546a_3b_S_mux_i2c_0

This is what you want. The tmp1 device is an I2C device on the mux1 bus.

In debug_child.h, I see these instead:

/* Bus info (controller: '/soc/i2c@40004000', type: 'i2c') */
#define DT_N_S_soc_S_i2c_40004000_S_tca9546a_3b_S_mux_i2c_0_S_tmp116_49_BUS_i2c 1
#define DT_N_S_soc_S_i2c_40004000_S_tca9546a_3b_S_mux_i2c_0_S_tmp116_49_BUS DT_N_S_soc_S_i2c_40004000

This is not what you want. This binding results in the bus pointing at the twim device, which is the wrong bus.

I don't see anything wrong with bindings_2/i2c-tca9546a.yaml and I agree this looks like a bug at this point. I'm investigating.

@mbolivar-nordic
Copy link
Contributor

@kurddt I believe #32105 is a fix. Please retest and confirm on your end if you can. Thanks.

@kurddt
Copy link
Contributor Author

kurddt commented Feb 9, 2021

@mbolivar-nordic I looked and tested your fix. Would it be possible to have the same behavior when the child node does not have any compatible?

In my use case this binding will not be used anywhere in the driver code, the parent device will create the child devices.
The binding file will look like the following:

compatible: "ti,tca9546a"

include: "i2c-device.yaml"

properties:
    label:
        required: true
        type: string
        description: tca9546


child-binding:
    include: "i2c-controller.yaml"
    description: Downlink I2C bus tca9546
    properties:
      label:
        required: true
        type: string

@nashif nashif added the priority: low Low impact/importance bug label Feb 9, 2021
@mbolivar-nordic
Copy link
Contributor

Thanks for testing!

Would it be possible to have the same behavior when the child node does not have any compatible?

Not easily with the current tooling, I'm afraid.

@kurddt
Copy link
Contributor Author

kurddt commented Feb 11, 2021

Ok, I think we can close this ticket when your fix is merged then, thank you for working on it. I will create a new enhancement request for the child compatible unless @mbolivar-nordic you prefer to do so yourself ?

@mbolivar-nordic
Copy link
Contributor

I will create a new enhancement request for the child compatible unless @mbolivar-nordic you prefer to do so yourself ?

Please go ahead, thanks.

mbolivar-nordic added a commit to mbolivar-nordic/zephyr that referenced this issue Mar 15, 2021
Whenever a child-binding: dict has a compatible, we ought to make
that available in EDT._compat2binding. If we don't, we can't look it
up later.

This has adverse effects like missing child bindings which describe
buses.

Fixes: zephyrproject-rtos#32071

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
galak pushed a commit that referenced this issue Mar 25, 2021
Whenever a child-binding: dict has a compatible, we ought to make
that available in EDT._compat2binding. If we don't, we can't look it
up later.

This has adverse effects like missing child bindings which describe
buses.

Fixes: #32071

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Tooling PR modifies or adds a Device Tree tooling bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants