boards: arm: add adc node and fixes for Nucleo L412RB-P

[gudnimg]

• Fix wrong SCL pin assignment on Nucleo L412RB-P
• Add SPI support to Nucleo L412RB-P documentation
• SPI pin assignment on Nucleo L412RB-P is now using Arduino pins.
• Add ADC1 node to Nucleo L412RB-P
• Add sensor apds9960 node to two boards (both tested):
  • Nucleo L412RB-P
  • Nucleo F401RE

[gudnimg]

One problem I need to fix... Arduino header uses SPI2 and not SPI1. The L4 device tree does not support SPI2 so I will need to add it myself.

[gudnimg]

Added a SPI2 node to L412 device tree (to use Arduino pins) and fixed trailing white spaces.

[gudnimg]

Added a quick change: arduino_spi: &spi1 {}; to arduino_spi: &spi2 {};

[gudnimg]

Added defined(CONFIG_BOARD_NUCLEO_L412RB_P) directive to adc test (build was failing with 'unsupported board').

[erwango]

Don't add arbitrary devices to the board configuration.
Board configuration should stick to available "out-of-the-box" hardware.

[gudnimg]

Don't add arbitrary devices to the board configuration.
Board configuration should stick to available "out-of-the-box" hardware.

Is .overlay file in the sensor sample folder a better way? I've not used it before... will look into it 😄

[gudnimg]

@erwango please take a look again. I created overlay files in the sample which works.

[erwango]

I wonder if there would be ways to set this much more generic using arduino_i2c port reference, and avoid introducing board specific files.
Could you have a check to samples/sensor/amg88xx/app.overlay and see if same approach could be used here ?

[gudnimg]

That would be nice. I will take a look.

[jfischer-no]

app.overlay is not right way here because we have a board in the tree equipped with this sensor. It is better to add a generic shield support for APDS9960 breakout-boards.

[jfischer-no]

Something like shield support for SSD1306 displays

[jfischer-no]

app.overlay is not right way here because we have a board in the tree equipped with this sensor. It is better to add a generic shield support for APDS9960 breakout-boards.

[erwango]

app.overlay is not right way here because we have a board in the tree equipped with this sensor. It is better to add a generic shield support for APDS9960 breakout-boards.

So it means that each time a sensor is available on a supported board, we can't use this generic approach ?
That would be unfortunate.
Is that a pure cmake constraint?

[gudnimg]

I'm now facing a problem with the Shield approach.

When I create a shield for the sensor like this (by following https://docs.zephyrproject.org/latest/guides/porting/shields.html):

And then add the shield to the sensor sample's CMakeLists.txt:

cmake_minimum_required(VERSION 3.13.1)

set(SHIELD adafruit_apds9960) # <-- add SHIELD to preprocessor 

find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
project(apds9960)

FILE(GLOB app_sources src/*.c)
target_sources(app PRIVATE ${app_sources})

The sample will work for all boards that support the arduino header. Now the problem I see is how do we determine when to set SHIELD?

set(SHIELD adafruit_apds9960)

Two cases I see where we don't want this:

1. Board in use already has the sensor onboard (example: Arduino Nano 33 BLE Sense)
2. Board is not compatible with "arduino-header-r3"

Any ideas on how to check these conditions in CMake? What are your thoughts on this?

Edit:

For reference, this is how my generic overlay looks like for the shield at the moment:

&arduino_i2c {
	status = "okay";

        apds9960: apds9960@39 {
		/* Device Address is 0x39 */
		compatible = "avago,apds9960";
		reg = <0x39>;
		label = "APDS9960";
		int-gpios = <&arduino_header 19 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>; /* D13 */
	};
};

[erwango]

Any ideas on how to check these conditions in CMake? What are your thoughts on this?

Please have a look to samples/shields/*/sample.yaml:

• provides depends_on: arduino_i2c arduino_gpio filter that will indicate the required arduino connector dependency
• platform_exclude: disco_l475_iot1 is used to inform that we don't want the sample to be run on some boards (the ones with the same sensor
  But I think platform_exclude should be replaced by a filter:
  filter: !dt_compat_enabled("avago,apds9960")

[jfischer-no]

app.overlay is not right way here because we have a board in the tree equipped with this sensor. It is better to add a generic shield support for APDS9960 breakout-boards.

So it means that each time a sensor is available on a supported board, we can't use this generic approach ?
That would be unfortunate.
Is that a pure cmake constraint?

Node definition in app.overlay would win and overwrite the one in boards/arm/reel_board/dts/reel_board.dtsi for example. But even if it would not, approach with a shield has significantly higher benefit for all.

[jfischer-no]

The sample will work for all boards that support the arduino header. Now the problem I see is how do we determine when to set SHIELD?
set(SHIELD adafruit_apds9960)

@GunZi200
No, the sample should work for all boards equipped with APDS9960 sensor or APDS9960 breakout-board connected.
When APDS9960 breakout-board is connected, the user has to build it with -DSHIELD=apds9960. Please do not name it adafruit_apds9960 or at least do not place it in boards/shields/adafruit_apds9960. There are several manufacturers for these breakout-boards, we do not need separate folders for each.

[erwango]

Node definition in app.overlay would win and overwrite the one in boards/arm/reel_board/dts/reel_board.dtsi for example. But even if it would not, approach with a shield has significantly higher benefit for all.

In the long run we need to enable a generic way to run add-on sensors on boards w/o too much code addition (such as having a board.overlay for each board in each sensor with obviously doesn't scale).
Adding a "generic" shield for each sensor/hw piece is somewhat a better proposal, but I consider this is still too much additional code, we need something more efficient.
The app.overlay sounds a good approach but I recognize the fact that it does not work with boards that embed the hw makes it broken for now. We should find a way to fix it.

[jfischer-no]

@GunZi200 can you please drop the last commit related to APDS9960 sensor so that this PR can be unblocked? You can then open a new PR for APDS9960 shield.

[gudnimg]

@GunZi200 can you please drop the last commit related to APDS9960 sensor so that this PR can be unblocked? You can then open a new PR for APDS9960 shield.

Sure I’ll do it later this evening and also fix the SPI issue @FRASTM found. :)

[gudnimg]

I've removed the sensor node changes.

Regarding the SPI CS pin, there is no chip select (NSS) assigned to PA11 in the pinmux or in the datasheet. See:
https://github.com/zephyrproject-rtos/hal_stm32/blob/master/dts/st/l4/stm32l412rbtx-pinctrl.dtsi

			/* SPI_NSS */

			spi1_nss_pa4: spi1_nss_pa4 {
				pinmux = <STM32_PINMUX('A', 4, AF5)>;
				bias-pull-up;
			};

			spi1_nss_pa15: spi1_nss_pa15 {
				pinmux = <STM32_PINMUX('A', 15, AF5)>;
				bias-pull-up;
			};

			spi1_nss_pb0: spi1_nss_pb0 {
				pinmux = <STM32_PINMUX('B', 0, AF5)>;
				bias-pull-up;
			};

			spi2_nss_pb9: spi2_nss_pb9 {
				pinmux = <STM32_PINMUX('B', 9, AF5)>;
				bias-pull-up;
			};

			spi2_nss_pb12: spi2_nss_pb12 {
				pinmux = <STM32_PINMUX('B', 12, AF5)>;
				bias-pull-up;
			};

None of SPI2 NSS are on the arduino connector. I think this is a fault with the board design. But can we not assign any GPIO pin as CS in the device tree? The I/O pin choice should not matter. I'm not sure how to select PA11 specifically.

For now I did not change the pin assignment.

[gudnimg]

@erwango I've added two notes to the documentation. One about the revision number and one about the SPI2 CS pin not being located on the arduino connector.

I'm assuming this is what you meant by:

If there's a conflict on revision, please at least add revision information in the board description.

addressed

[gudnimg]

Hi, just a friendly reminder :)

[MaureenHelm]

@erwango please revisit

[gudnimg]

I don’t see how the build failures are related to my changes. Is it possible to re-run it?

[erwango]

@GunZi200 Indeed. This should be fixed by rebasing on latest master.