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

boards: MikroE Clicker 2: Added board #27345

Merged
merged 2 commits into from Nov 30, 2020

Conversation

pbeTrifork
Copy link
Contributor

Added MikroE Clicker 2 board.

Discussed with @henrikbrixandersen the best way to cope with the two mikrobus ports on the board, didn't get anything conclusive so added as aliases for now

Signed-off-by: Paul M. Bendixen pbe@trifork.com

Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, a few minor corrections below.

boards/arm/mikroe_clicker_2/CMakeLists.txt Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/Kconfig.board Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/Kconfig.defconfig Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/board.cmake Show resolved Hide resolved
boards/arm/mikroe_clicker_2/doc/mikroe_clicker_2.rst Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/doc/mikroe_clicker_2.rst Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/doc/mikroe_clicker_2.rst Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/doc/mikroe_clicker_2.rst Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/mikroe_clicker_2.dts Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/pinmux.c Outdated Show resolved Hide resolved
};

&adc1 {
status ="okay";
Copy link
Contributor Author

@pbeTrifork pbeTrifork Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to have labels for the channels here ? I couldn't find (or understand) the documentation of it.
e.g.:

mikrobus_1_adc: adc1_ch2 {
    label = "mikroBUS 1 ADC";
    channel = <2>;
};
mikrobus_2_adc: adc1_ch3 {
    label = "mikroBUS 2 ADC";
    channel = <3>;
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not currently possible. We would need something along the line of the gpio-nexus for ADCs in order to support this...

boards/arm/mikroe_clicker_2/Kconfig.board Outdated Show resolved Hide resolved
boards/arm/mikroe_clicker_2/Kconfig.defconfig Outdated Show resolved Hide resolved
};

&adc1 {
status ="okay";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not currently possible. We would need something along the line of the gpio-nexus for ADCs in order to support this...

@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Aug 5, 2020
@pbeTrifork
Copy link
Contributor Author

Ok so it seems the problem in the tests is that I use defconfig to get a uart console on the usb interface, but when running the non-multithreaded kernel test POSIX is not defined and then the USB stack cannot be defined.
Should I move UART_CONFIG_ON_DEV_NAME to Kconfig.defconfig and make it depend on USB or something?
Any ideas on fixing this would be appreciated.

@erwango
Copy link
Member

erwango commented Oct 13, 2020

@pbeSmartDevice Sorry, it seems I missed your PR.
If you're still interesting in getting it merged (which would be nice), can you rebase on top of master and move UART configuration to device tree ?

@pbeTrifork
Copy link
Contributor Author

@pbeSmartDevice Sorry, it seems I missed your PR.
If you're still interesting in getting it merged (which would be nice), can you rebase on top of master and move UART configuration to device tree ?

That's fine
I've rebased, but what exactly do you mean by moving the UART configuration to device tree, apart from the pinmux it is already defined in the .dts?

@erwango
Copy link
Member

erwango commented Oct 13, 2020

I've rebased, but what exactly do you mean by moving the UART configuration to device tree, apart from the pinmux it is already defined in the .dts?

I meant the pinmux actually. I2C and ADC are on their way too. Cf #28999 for more info.
Actually, it might be worthy waiting one or 2 more weeks so the transition is completed.

@pbeTrifork
Copy link
Contributor Author

I tried to update it, but I can't seem to find the right include and I can't figure out how to include something that is not a file in my .dts

@pbeTrifork pbeTrifork force-pushed the Mikroe_clicker_2 branch 2 times, most recently from 001ca45 to 904421b Compare October 14, 2020 10:01
@erwango
Copy link
Member

erwango commented Oct 14, 2020

I tried to update it, but I can't seem to find the right include and I can't figure out how to include something that is not a file in my .dts

Do you mean this #include <st/f4/stm32f407v(e-g)tx-pinctrl.dtsi> ?
If not working, please try to run west update and check that the latest hal_stm32 module version is correctly pulled.

@pbeTrifork
Copy link
Contributor Author

I tried to update it, but I can't seem to find the right include and I can't figure out how to include something that is not a file in my .dts

Do you mean this #include <st/f4/stm32f407v(e-g)tx-pinctrl.dtsi> ?
If not working, please try to run west update and check that the latest hal_stm32 module version is correctly pulled.

Yes that was exactly it, everything seems to be working now.

@pbeTrifork
Copy link
Contributor Author

@erwango So is this sufficient?

@erwango
Copy link
Member

erwango commented Oct 19, 2020

@pbeSmartDevice It seems fine aside from the pinmux part that should be moved to device tree.
I'd advise to wait a few days that SPI and USB are available

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash 2 first commit.
And it would be good to confirm this is working, otherwise there is not point ...

@pbeTrifork
Copy link
Contributor Author

I've Added ADC and SPI as well, should I hold out for USB (I can see it doesn't have a PR yet, at least according to #28999) or should I just squash the three board related ones and have it upstreamed?

Also could I get feedback about the ADC if this is the correct way to go?

@erwango erwango added the platform: STM32 ST Micro STM32 label Oct 21, 2020
@erwango
Copy link
Member

erwango commented Oct 21, 2020

Also could I get feedback about the ADC if this is the correct way to go?

Looks good indeed.

I've Added ADC and SPI as well, should I hold out for USB (I can see it doesn't have a PR yet, at least according to #28999) or should I just squash the three board related ones and have it upstreamed?

USB is on its way, but it might take longer to merge. So let's do the squash and we'll try to merge.

@pbeTrifork pbeTrifork force-pushed the Mikroe_clicker_2 branch 4 times, most recently from f78ba7c to 339e020 Compare October 23, 2020 14:47
@pbeTrifork
Copy link
Contributor Author

So @erwango I could guess that you would want me to update to get me to use the dts for the USB as well, however I haven't been able to get serial port over USB to work with the new patch and I have no idea why.
The device doesn't come up in dmesg when I attach it and the hello-world example therefore can't really work.
blinky works fine, so it's not the setup.

@erwango
Copy link
Member

erwango commented Oct 23, 2020

@pbeTrifork

You mean that this is working fine if you configure uart4 pins using pinmux.c ?

You would be able to compare GPIO (A in your case) register settings after pins have been configured (let's say end of uart_stm32_init) ?
And if that's equivalent (which I expect to be, since I've verified that several times on various targets), would you be able to look at generated signals in both cases?
I'm thinking this can be a timing issue or a glitch generated on the line. Which would be strange as uart is generally quite error resilient. But I'm not familiar with the probe you're using.

Last point, is there a working branch you can share?

@pbeTrifork
Copy link
Contributor Author

No, what I mean is that no matter what setting I try out I can't get the hello world sample to work.
When setting the config to

CONFIG_UART_INTERRUPT_DRIVEN=y
CONFIG_UART_CONSOLE_ON_DEV_NAME="CDC_ACM_0"
CONFIG_CONSOLE=y
CONFIG_USB_UART_CONSOLE=y
CONFIG_SERIAL=y
CONFIG_USB=y
CONFIG_USB_DEVICE_STACK=y
CONFIG_USB_DEVICE_OS_DESC=y

Then the /dev/ttyACM0 doesn't show up, when I omit fdb6c46 ( changing pinmux settings to dts settings ) it still doesn't show up and dmesg also doesn't show any new connected devices.

@erwango
Copy link
Member

erwango commented Nov 4, 2020

@pbeTrifork, do you know if there's any life? Is blinky working ?

@pbeTrifork
Copy link
Contributor Author

Yes, blinky is working, my main project for this is also working, I can read the sensor outputs from my program using gdb, but it still doesn't show up on dmesg or as a serial port, so the output doesn't come out.

@pbeTrifork
Copy link
Contributor Author

Merged with master and tried the cdc usb sample, which worked fine.
Seems like I have a problem in my application configuration

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor changes and we're good to go, if you're ready.

Comment on lines 2 to 20
* Copyright (c) 2020 Trifork
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <kernel.h>
#include <device.h>
#include <init.h>
#include <drivers/pinmux.h>
#include <sys/sys_io.h>

#include <pinmux/stm32/pinmux_stm32.h>

/* Pin assignments for MikroE Clicker 2 board for STM32 */
static const struct pin_config pinconf[] = {
};

static int pinmux_mikroe_clicker_2_init(const struct device *port)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file could be removed now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I thought I read that the idea was to keep it in, but ok.

Comment on lines 1 to 6
# SPDX-License-Identifier: Apache-2.0

if(CONFIG_PINMUX)
zephyr_library()
zephyr_library_sources(pinmux.c)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this file

Added support for the Clicker 2 for STM32 board from MikroElektronika

Signed-off-by: Paul M. Bendixen <pbe@trifork.com>
Modified the drivers.adc test to include the MikroE Clicker 2 board

Signed-off-by: Paul M. Bendixen <pbe@trifork.com>
Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@carlescufi carlescufi merged commit e5d36db into zephyrproject-rtos:master Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards area: Devicetree area: Documentation area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants