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

DT_NODE_HAS_COMPAT does not consider parents/path #51607

Closed
richesonk opened this issue Oct 25, 2022 · 10 comments
Closed

DT_NODE_HAS_COMPAT does not consider parents/path #51607

richesonk opened this issue Oct 25, 2022 · 10 comments

Comments

@richesonk
Copy link

Describe the bug
When a devicetree node inherits it's compatible from a parent, DT_NODE_HAS_COMPAT() does not return "true"

To Reproduce
Consider the case of buttons or LEDs on a dev kit. These are usually defined in a group where the compatible is on the parent node then each button is actually a child of that node. For example from zephyr/boards/arm/nrf5340dk_nrf5340/nrf5340_cpuapp_common.dts

	buttons {
		compatible = "gpio-keys";
		button0: button_0 {
			gpios = <&gpio0 23 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>;
			label = "Push button 1";
		};
		button1: button_1 {
			gpios = <&gpio0 24 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>;
			label = "Push button 2";
		};
		button2: button_2 {
			gpios = <&gpio0 8 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>;
			label = "Push button 3";
		};
		button3: button_3 {
			gpios = <&gpio0 9 (GPIO_PULL_UP | GPIO_ACTIVE_LOW)>;
			label = "Push button 4";
		};
	};

Invoking DT_NODE_HAS_COMPAT(button_3, gpio-keys) returns 0 because the compatible is not directly on that given child node.

Expected behavior
I was expecting DT_NODE_HAS_COMPAT() to return 1 if the node is indeed using the given compatible whether that compatible was assigned directly or inherited from a parent (or further up the tree) node.

Impact
This is mostly an annoyance, but it does add an additional challenge when dealing with different board configurations and forces us to know more about the devicetree structure than seems necessary (i.e. is it always the parent node that defines the "gpio-keys" compatible? Can "gpio-keys" groups be nested such that it's the grandparent node instead? etc.)

Environment (please complete the following information):
This is not significantly affected by the environment, but does affect both the devicetree macros and the derived Kconfig functions for accessing device tree information from Kconfig (dt_nodelable_has_compat() etc).

@richesonk richesonk added the bug The issue is a bug, or the PR is fixing a bug label Oct 25, 2022
@henrikbrixandersen
Copy link
Member

I believe the current behavior is correct. If the node in question does not itself have the compatible set, the API should not look further up the tree.

@henrikbrixandersen
Copy link
Member

CC: @mbolivar-nordic @galak

@galak
Copy link
Collaborator

galak commented Oct 25, 2022

I believe the current behavior is correct. If the node in question does not itself have the compatible set, the API should not look further up the tree.

Correct, as @henrikbrixandersen said, this is expected behavior. This is not a bug.

@galak galak added area: Devicetree and removed bug The issue is a bug, or the PR is fixing a bug labels Oct 25, 2022
@galak
Copy link
Collaborator

galak commented Oct 25, 2022

In the example you give of gpio-keys and its children are considered a singular unit. A driver should work / exist at the gpio-keys level and thus it doesn't make sense to utilize DT_NODE_HAS_COMPAT on the button node.

@richesonk
Copy link
Author

So, how about a different, unfortunately slightly more abstract, example. Let's say I have a temperature sensor that is capable of communicating via both SPI and I2C. Further, let's say I don't have a great relationship with my HW team so on some boards that sensor is connected over SPI and on other boards it's connected over I2C. In both cases that device will exist in the device tree as a child node of the appropriate bus.

In this case, I'll either need to write two separate interface modules and then be able to choose between them based on information from the devicetree, or, more likely, I'll have one interface module with two different sets of low level interface functions that, ideally, will be switched out with DT_ macros. Something like the following:

#if DT_NODE_HAS_COMPAT(my_sensor, nordic_nrf_twim)
int read_sensor_register(int reg_addr)
{
  // Do I2C things....
}
#elif DT_NODE_HAS_COMPAT(my_sensor, nordic_nrf_spim)
int read_sensor_register(int reg_addr)
{
  // Do SPI things....
}
#endif

int read_sensor(void)
{
  read_sensor_register(0x0a);
}

@galak
Copy link
Collaborator

galak commented Oct 25, 2022

We have examples of this already -- see something like drivers/sensor/bme280/. The compatible of the sensor is the same regardless of the type of bus it is on. We have provided means to look at the type of parent bus.

@richesonk
Copy link
Author

We have examples of this already -- see something like drivers/sensor/bme280/. The compatible of the sensor is the same regardless of the type of bus it is on. We have provided means to look at the type of parent bus.

And you're comfortable with the fact that this requires the node in question to be only one level below the bus, so there would not be an opportunity to group child nodes below the bus level. Something like:

bus {
  compatible = "spi"
  sensors {
    temp_sensor {
      ...
    };
    accelerometer {
      ...
    };
  };
  storage
  {
    nonvol_eeprom {
      ...
    };
  };
};

@henrikbrixandersen
Copy link
Member

Perfectly comfortable, yes. What you posted there is not how I2C or SPI bus devices are represented in DTS.

@richesonk
Copy link
Author

Perfectly comfortable, yes. What you posted there is not how I2C or SPI bus devices are represented in DTS.

Please forgive my ignorance. Are the rules around representing I2C and SPI devices in DTS documented somewhere that I could look to learn more. Are there similar rules for all classes of devices that might be represented in DTS?

@mbolivar-nordic
Copy link
Contributor

somewhere that I could look to learn more

Sure, you can watch this video: https://www.youtube.com/watch?v=sWaxQyIgEBY

Both I2C and SPI devices in DT are covered there in the second half of the presentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants