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

Add SPI and RTC support for 96Boards Wistrio #19861

Merged
merged 5 commits into from
Oct 18, 2019

Conversation

Mani-Sadhasivam
Copy link
Member

This PR adds SPI1 and RTC support for 96Boards Wistrio board. It has been forked out of #18998 to ease merging the STM32 specifics independent of the core LoRa code.

Signed-off-by: Manivannan Sadhasivam mani@kernel.org

@zephyrbot
Copy link
Collaborator

zephyrbot commented Oct 16, 2019

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@erwango erwango added the platform: STM32 ST Micro STM32 label Oct 16, 2019
@erwango
Copy link
Member

erwango commented Oct 16, 2019

@Mani-Sadhasivam, for SPI there's an on-going PR to add it on L1 which is ready for merge (#19729)
So maybe you can pick up the commits you need in this PR for the time being.

@@ -9,7 +9,7 @@ if SOC_STM32L151XB

config SOC
string
default "stm32l151xb"
default "stm32l151xba"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

This can't be right, there are both xxxxB and xxxxBA parts in the wild.

I didn't say that stm32l151xb is deprecated. I assumed that in zephyr mainline, there is only one L1X SoC present and it is based on the Wistrio board. So the exact part number of the SoC is found to be stm32l151xba, hence I just modified it.

@erwango What is the criteria for default SoC name?

Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't the default soc, the file you're editing here is "Kconfig.defconfig.stm32l151xb" you're changing it to add an "a" suffix. That can't be right. If the wistrio board has a different part, that's fine, change what wistrio refers too, add the -a suffix parts, but don't try and make the xb file appear as xb-A.

Copy link
Member Author

Choose a reason for hiding this comment

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

this isn't the default soc, the file you're editing here is "Kconfig.defconfig.stm32l151xb" you're changing it to add an "a" suffix.

I think I misunderstood the actual difference between those 2 SoCs. Anyway, I'v asked the board vendor to clarify the SoC part number as there seems to be different versions of RAK811 modules. If they confirm it is STM32L151XB-A then I'll do the change accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

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

The stm32flash utility confirms that it is STM32L151XB-A.

Copy link
Member

Choose a reason for hiding this comment

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

@erwango What is the criteria for default SoC name?

So, first it should match your SoC. Then, it should match (in lower case), the definition in stm32cube/stm32l1xx/soc/stm32l1xx.h.
It will be used in stm32cube/CMakeLists.txt

Copy link
Member Author

Choose a reason for hiding this comment

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

@erwango So for the STM32L151xBA SoC, I should create soc/arm/st_stm32/stm32l1/Kconfig.defconfig.stm32l151xba, right? It will be an exact duplicate of stm32l151xb though.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is needed, but if I'm correct, @karlp did it already

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually added the x8-a, because that's the part I wanted for something else. I raised it on the slack channel, what the future plans were for collecting all the variants, because you're goign to explode in boilerplate. response there was "just add them as you need them, we'll sort it out later"

The defconfig might be, but it will refer to a different dtsi file, as they have different flash/ram to the non-a parts. (and less eeprom writes too, but that's getting off topic...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is needed, but if I'm correct, @karlp did it already

@erwango He added x8-a but for me, xb-a is needed. So I'll create a new one!

@@ -351,7 +351,7 @@ static int spi_stm32_configure(struct device *dev,
ll_func_set_fifo_threshold_8bit(spi);
#endif

#ifndef CONFIG_SOC_SERIES_STM32F1X
#if !defined(CONFIG_SOC_SERIES_STM32F1X) && !defined(CONFIG_SOC_SERIES_STM32L1X)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the wrong fix. L1 has the FRF bit, you're just missing this fix in the HAL: zephyrproject-rtos/hal_stm32#22 (enabled inside my other PR: 5e5afa0)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Since the definition was not present in CMSIS, I thought it is not supported. My bad for not looking into deeply. Anyway, I'll just rebase once your PR gets in.

@Mani-Sadhasivam
Copy link
Member Author

@erwango Rebased!

@erwango erwango requested a review from MaureenHelm October 17, 2019 07:58
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.

@Mani-Sadhasivam can you update the board doc with your change please ? Otherwise this is fine.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

+1 for doc changes

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.

@Mani-Sadhasivam, request to update board doc with additional supported feature (SPI, RTC) still stands.

@Mani-Sadhasivam
Copy link
Member Author

@erwango Sure, will update.

Mani-Sadhasivam and others added 5 commits October 18, 2019 19:22
Enable SPI1 available on the expansion header of 96Boards
Wistrio board.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add RTC/Counter support for STM32L1 SoCs.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Enable on-chip RTC driver for 96Boards Wistrio board.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
STM32L151XB-A SoC is almost similar to the STM32L151XB SoC except that
it has more RAM (32KiB). Hence add devicetree and Kconfig support.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
The RAK811 module used on this board incorporates STM32L151CB-A SoC,
which has more RAM (32 KiB) compared to its companion STM32L151CB.
Hence, fix the doc, dts and Kconfig to include correct part number.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
@Mani-Sadhasivam
Copy link
Member Author

@erwango Done!

@erwango erwango requested review from erwango and galak October 18, 2019 15:00
@MaureenHelm MaureenHelm merged commit 56ed28e into zephyrproject-rtos:master Oct 18, 2019
@Mani-Sadhasivam Mani-Sadhasivam deleted the wistrio-lora branch October 21, 2019 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants