Moving gpio pin in device tree leads to error

[ErwinCes]

Hello,

in my device tree there are three nodes/items concerning battery Management:

1. One load enable pin
2. One measure enable pin
3. the required ADC configuration

The following configuration works:

	batloaden {
		compatible = "gpio-keys";
		bat_load_en: bat_load {
			gpios = <&gpio1 12 GPIO_PUSH_PULL>;
			label = "batLoadEnPin";
		};
	};
	
	btmeasen {
		compatible = "gpio-keys";
		bt_meas_en: bt_meas {
			gpios = <&gpio1 14 GPIO_PUSH_PULL>;
			label = "btMeasEn";
		};
	};
	batmeasure {
		bat_adc_conf: bat_adc_conf {
			compatible = "bat-adc-config";
			resolution = <12>;
			gain = <6>;                  //Numeric value of ADC_GAIN_1
			stabilize_time = <3>;
		};
    }

The pin configuration is taken in my Code via

static const struct gpio_dt_spec BatMeasENpinspec = GPIO_DT_SPEC_GET(BAT_MEAS_EN_PIN_NODE, gpios);

But when I try to Group the whole measurement stuff into one node as

	batmeasure {
		bat_adc_conf: bat_adc_conf {
			compatible = "bat-adc-config";
			resolution = <12>;
			gain = <6>;                  //Numeric value of ADC_GAIN_1
			reference = <4>;             //Numeric value of ADC_REF_INTERNAL
			aquisition_time = <0x4028>;  //Numeric value of ADC_ACQ_TIME(ADC_ACQ_TIME_MICROSECONDS, 40)
			ucal_channel_id = <0>;
			ubat_channel_id = <1>;
			ucal_plus_input = <1>;             //Numeric value of NRF_SAADC_INPUT_AIN0
			ucal_minus_ubat_plus_input = <2>;  //Numeric value of NRF_SAADC_INPUT_AIN1
			ubat_minus_input = <3>;            //Numeric value of NRF_SAADC_INPUT_AIN2
			stabilize_time = <3>;
		};
		bat_load_en: bat_load {
			compatible = "gpio-keys";
			gpios = <&gpio1 12 GPIO_PUSH_PULL>;
			label = "batLoadEnPin";
		};
		bt_meas_en: bt_meas {
			compatible = "gpio-keys";
			gpios = <&gpio1 14 GPIO_PUSH_PULL>;
			label = "btMeasEn";
		};
	};

I get the error identifier "__device_dts_ord_DT_N_S_batmeasure_S_bt_meas_P_gpios_IDX_0_PH_ORD" is undefined for the line static const struct gpio_dt_spec BatMeasENpinspec = GPIO_DT_SPEC_GET(BAT_MEAS_EN_PIN_NODE, gpios);

Practically I just moved the pin node from top into my matmeasure node.

The pin node is queried by

#define BAT_MEAS_EN_PIN_NODE    DT_NODELABEL(bt_meas_en)

which seems to work in both cases.

Is it required that all my gpio defines are at the top Level to get the Right __device_dts… define?

Regards

Erwin

[Simon]

I think the reason GPIO_DT_SPEC_GET is failing in the second approach is because you're adding the "gpio-keys" compatible to the same node as the gpio-keys properties. The correct approach is to add the properties under a child node of the "gpio-keys" node.

If you look at the yaml bindings file of "gpio-keys" you can see that "properties" is added under "child-binding". Read more about child-binding in the documentation: https://developer.nordicsemi.com/nRF_Connect_SDK/doc/2.1.0/zephyr/build/dts/bindings.html#child-binding 

	parent {
		compatible = "gpio-keys";
		//don't add properties here
		child {
			//add properties here
		};
	};

Best regards,

Simon

[ErwinCes]

Great! It works now.

Just one Question to improve my understanding. I can Change it in two ways:

even before my adc conf
batmeasure {
		compatible = "gpio-keys";
		bat_adc_conf: bat_adc_conf {
			compatible = "bat-adc-config";
			resolution = <12>;
		};
		bat_load_en: bat_load {
			gpios = <&gpio1 12 GPIO_PUSH_PULL>;
		};
		bt_meas_en: bt_meas {
			gpios = <&gpio1 14 GPIO_PUSH_PULL>;
		};
	};
	
or behind my adc_conf to cover only the gpios	
batmeasure {
		bat_adc_conf: bat_adc_conf {
			compatible = "bat-adc-config";
			resolution = <12>;
		};
		compatible = "gpio-keys";
		bat_load_en: bat_load {
			gpios = <&gpio1 12 GPIO_PUSH_PULL>;
		};
		bt_meas_en: bt_meas {
			gpios = <&gpio1 14 GPIO_PUSH_PULL>;
		};
	};	

Both ways seem to work; at least it compiles and does the stuff it should be.

In the documentation I haven't found anything About order. Would in both my ways bat:adc_conf also be compatible with gpio-keys? Or never as I have a dedicated compatible Statement in the bat_adc_conf node? Or not when I put the gpio-keys compatible line only after bat_adc_conf (but before the first pin=

[Simon]

ErwinCes said:

In the documentation I haven't found anything About order.

I don't have any documentation to back this up, but I don't think the order of where you put the gpio-keys compatible statement matters. 

ErwinCes said:

Would in both my ways bat:adc_conf also be compatible with gpio-keys?

That is a good question. I'm not sure what happens when a child node of a gpio-keys node have it's own compatible. Would you be able to test it? Do you get any complaints that the required gpios property is missing from bat_adc_conf? 

Wouldn't it be better to organize it like this?

batmeasure{    
    bat_adc_conf: bat_adc_conf {
		compatible = "bat-adc-config";
		resolution = <12>;
	};

    batmeasure_gpios {
    	compatible = "gpio-keys";
    	bat_load_en: bat_load {
    		gpios = <&gpio1 12 GPIO_PUSH_PULL>;
    	};
    	bt_meas_en: bt_meas {
    		gpios = <&gpio1 14 GPIO_PUSH_PULL>;
    	};
    };
};

Best regards,

Simon

[ErwinCes]

Hi Simon,

you wrote "I'm not sure what happens when a child node of a gpio-keys node have it's own compatible. Would you be able to test it?". As I wrote before, I used it and it worked. This is my first described way:

The gpio compatible statement was before my adc conf node and the gpio nodes after my conf node. I saw nothing, that doesn't work. 

So practically in that case I have a child compatible statement in my parent together with an own compatible statement in my node. Can it be that a compatible statement in my node kicks out all child compatible statements in my paremt? Or can a node have two compatible statements in practice: one own and one child statement from my parent? But in that case I could have even more compatible statements: if my parent still has a parent with a child statement, there might be three statements valid for me.

As written, this is somehow an academic question, as each solution works. But to complete my understanding of all this device tree stuff I would like to have an answer what is possible.

Your proposal, to move the gpios into a dedicated node below my batmeasurement is also a good idea and I guess it would work also.

Best regards

Erwin

[Simon]

I have forwarded your questions internally. I'll keep you updated.

Best regards,

Simon

[Simon]

I talked to a device tree expert and got some input. He was not able to look deply into your questions but made some quick comments.

"The battery adc config is likely served better by existing bindings, like the nordic ADC or SAADC bindings perhaps, or maybe some bindings specific to batteries."

Check out the sample \samples\boards\nrf\battery, it uses the vbatt node with the compatible voltage-divider

"I’ll have to check about how this works in terms of which binding is supposed to match in this case actually. It’s definitely not clean and it’s enough of an edge case that I will need to check the source code to be sure. I wouldn’t recommend it either way"

"It would be better to write a custom application specific binding describing the specific hardware in the customer application or reuse one if an existing general purpose one fits their needs"

[Simon]

I asked the devicetree expert internally these two questions:

• Will the gpio-keys compatible apply to bat_adc_conf?
• Is there any other consequences the customer can face doing it this way (placing a node with a separate compatible inside a gpio-keys node)?

I got the following answers:

Will the gpio-keys compatible apply to bat_adc_conf

"No. Nodes with explicit compatibles that have matching bindings always use those bindings. This check happens before falling back on a check for child bindings from a parent node."

Is there any other consequences the customer can face doing it this way?

"Probably not, but this is situation-dependent and it's impossible to guarantee no other errors could occur. For example if you used DT_FOREACH_CHILD() on the batmeasure node and got the GPIOs out of each child, this could lead to a build error on the bat_adc_conf node, which has no gpios property. Things like this are why it does not make sense to put that node there."

Best regards,

Simon

[ErwinCes]

Thank you very much. This answers all my questions.

I will mark the answer before as verify Answer to close this ticket. According my understanding this is the right way to close it, right?

Erwin