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 support for Actions Semi ATB1103 MCU and EVB #20984

Open
wants to merge 5 commits into
base: master
from

Conversation

@Mani-Sadhasivam
Copy link
Collaborator

Mani-Sadhasivam commented Nov 25, 2019

Hello,

This PR adds support for Actions Semi ATB1103 MCU and its evaluation board. ATB1103 is part of the Low power Bluetooth MCUs family (ATB110X) from Actions Semi. The target applications for this MCU are BLE Voice Remote, Smart wearable devices, Smart Home sensors, and BT Mesh device applications. The EVB (Golden board) is designed and manufactured by Actions Semi for their internal use but there are few commercial target boards available for this MCU as well.

This PR is based on the initial code support for the MCU and board developed by Actions Semi and hence the copyrights are shared based on the contributions.

Thanks,
Mani

@zephyrbot

This comment has been minimized.

Copy link
Collaborator

zephyrbot commented Nov 25, 2019

All checks are passing now.

checkpatch (informational only, not a failure)

-:111: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "actions,atb1103_golden" appears un-documented -- check ./dts/bindings/
#111: FILE: boards/arm/atb1103_golden/atb1103_golden.dts:12:
+	compatible = "actions,atb1103_golden";

-:1148: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "serial-flash" appears un-documented -- check ./dts/bindings/
#1148: FILE: dts/arm/actions/atb110x.dtsi:25:
+		compatible = "serial-flash";

-:1160: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string vendor "acts" appears un-documented -- check ./dts/bindings/vendor-prefixes.txt
#1160: FILE: dts/arm/actions/atb110x.dtsi:37:
+			compatible = "acts,cmu";

-:1168: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string vendor "acts" appears un-documented -- check ./dts/bindings/vendor-prefixes.txt
#1168: FILE: dts/arm/actions/atb110x.dtsi:45:
+			compatible = "acts,uart";

-:1177: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "actions,uart" appears un-documented -- check ./dts/bindings/
#1177: FILE: dts/arm/actions/atb110x.dtsi:54:
+			compatible = "actions,uart";

-:1186: WARNING:UNDOCUMENTED_DT_STRING: DT compatible string "actions,uart" appears un-documented -- check ./dts/bindings/
#1186: FILE: dts/arm/actions/atb110x.dtsi:63:
+			compatible = "actions,uart";

-:1516: WARNING:LONG_LINE: line over 80 characters
#1516: FILE: soc/arm/actions/atb110x/dts_fixup.h:9:
+#define DT_NUM_IRQ_PRIO_BITS		DT_ARM_V6M_NVIC_E000E100_ARM_NUM_IRQ_PRIORITY_BITS

-:1526: WARNING:LONG_LINE: line over 80 characters
#1526: FILE: soc/arm/actions/atb110x/dts_fixup.h:19:
+#define DT_UART_ACTS_UART_0_CLOCK_CONTROLLER	DT_ACTS_UART_4000D000_CLOCK_CONTROLLER

-:1534: WARNING:LONG_LINE: line over 80 characters
#1534: FILE: soc/arm/actions/atb110x/dts_fixup.h:27:
+#define DT_UART_ACTS_UART_1_CLOCK_CONTROLLER	DT_ACTS_UART_4000E000_CLOCK_CONTROLLER

-:1542: WARNING:LONG_LINE: line over 80 characters
#1542: FILE: soc/arm/actions/atb110x/dts_fixup.h:35:
+#define DT_UART_ACTS_UART_2_CLOCK_CONTROLLER	DT_ACTS_UART_4000F000_CLOCK_CONTROLLER

-:1811: WARNING:LONG_LINE: line over 80 characters
#1811: FILE: soc/arm/actions/atb110x/soc_gpio.h:55:
+#define PINMUX_MODE_MASK                  (GPIO_CTL_MFP_MASK | GPIO_CTL_PULL_MASK | GPIO_CTL_SMIT | GPIO_CTL_PADDRV_MASK)

- total: 0 errors, 11 warnings, 2003 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

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

@Mani-Sadhasivam Mani-Sadhasivam force-pushed the Mani-Sadhasivam:atb1103-upstream branch from a5ed0ee to 939c657 Nov 25, 2019
Copy link
Contributor

dbkinder left a comment

docs LGTM, thanks.

@Mani-Sadhasivam

This comment has been minimized.

Copy link
Collaborator Author

Mani-Sadhasivam commented Dec 15, 2019

@galak Any update on this PR?

@carlescufi carlescufi requested a review from jfischer-phytec-iot Dec 18, 2019
@jhedberg

This comment has been minimized.

Copy link
Member

jhedberg commented Jan 7, 2020

@Mani-Sadhasivam there seems to be a conflict with upstream (with include/arch/arm/cortex_m/scripts/linker.ld so could you please rebase? You might also want to take look at whether fixing the warnings from checkpatch makes sense (there are no errors though).

There are also essentially no reviews from codeowners, so could the assigned reviewers please take a look?

Add SoC support for Actions Semi ATB1103 MCU. ATB1103 MCU comes from
ATB110X family of Actions Semi and has below features:

* CPU - ARM Cortex-M0
* RAM - 40KB
* ROM - 160KB
* Integrated Flash - 4MB
* Bluetooth 4.2

More information about this MCU can be found in Actions product page:
http://www.actions-semi.com/en/productview.aspx?id=234

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add clock driver support for Actions CMU (Clock Management Unit). CMU
is responsible for managing clocks in ATB110X MCUs.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
@Mani-Sadhasivam Mani-Sadhasivam force-pushed the Mani-Sadhasivam:atb1103-upstream branch from 939c657 to 9b2fdb6 Jan 25, 2020
@galak

This comment has been minimized.

Copy link
Contributor

galak commented Jan 29, 2020

@Mani-Sadhasivam can you address the 'nit' CI check issue.

@galak galak force-pushed the Mani-Sadhasivam:atb1103-upstream branch from 9b2fdb6 to 6012cbd Jan 29, 2020
Add UART driver support for Actions Semi MCUs. The UART driver has
been used in ATB110X series MCUs.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add board support for ATB1103 Golden Board from Actions Semi. This
board is powered by Actions Semi ATB1103 MCU.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Add myself as the codeowner for Actions Semi stuffs.

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
@Mani-Sadhasivam Mani-Sadhasivam force-pushed the Mani-Sadhasivam:atb1103-upstream branch from 6012cbd to 035a0b6 Jan 29, 2020
@Mani-Sadhasivam

This comment has been minimized.

Copy link
Collaborator Author

Mani-Sadhasivam commented Jan 29, 2020

@galak Done

@@ -167,6 +170,8 @@
/drivers/sensor/lps*/ @avisconti
/drivers/sensor/lsm*/ @avisconti
/drivers/sensor/st*/ @avisconti
/drivers/serial/Kconfig.acts @Mani-Sadhasivam
/drivers/serial/uart_acts.c @Mani-Sadhasivam

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Can you make this just serial/*acts*

@@ -0,0 +1,7 @@
# Copyright (c) 2019 Actions (Zhuhai) Technology Co., Ltd

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Why copyright to Actions?

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Ah, see you commented in the PR on this. Is there a reference to their original code?

This comment has been minimized.

Copy link
@Mani-Sadhasivam

Mani-Sadhasivam Jan 29, 2020

Author Collaborator

I have inherited code from them

This comment has been minimized.

Copy link
@Mani-Sadhasivam

Mani-Sadhasivam Jan 29, 2020

Author Collaborator

Ah, see you commented in the PR on this. Is there a reference to their original code?

No. They just shared a zip file with me based on LTS version and I had to clean a lot for upstream.

@@ -0,0 +1,6 @@
# SPDX-License-Identifier: Apache-2.0

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Missing Copyright?

Copy link
Contributor

galak left a comment

Can you shrink the boards/arm/atb1103_golden/doc/img/atb1103_golden.jpg, 4M is kinda big.

Copy link
Contributor

galak left a comment

The compats for DTS, do you think the names cover other actions semi MCUs? Just wondering if there is some family name or something to use?

return err;

/* Enable receiver and transmitter */
uart->ctrl = ((0x1 << 31) | (0x1 << 30) | (0x1 << 23) | (0x1 << 22) |

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Can this magic number be replaced with defines?

};

static struct uart_acts_dev_data uart_acts_dev_data_0 = {
.baud_rate = 115200,

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

any reason baud_rate isn't coming from DT?

#ifdef CONFIG_UART_INTERRUPT_DRIVEN
static void uart_acts_irq_config_0(struct device *port)
{
IRQ_CONNECT(IRQ_ID_UART0,

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Any reason IRQ isn't coming from DT?

# must be >= the highest interrupt number used
default 32

config ISR_STACK_SIZE

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Why does this need setting here? probably a comment at min.

*/

. = 0xc0;
LONG(0x00425441);

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Does this magic number having meaning? If so a comment about it.

acts_request_rc_3M(false);

/* Remap Vector base address */
sys_write32(0x01000000, VECTOR_BASE);

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

What is magic number 0x01000000?

#ifndef _ACTIONS_SOC_IRQ_H_
#define _ACTIONS_SOC_IRQ_H_

#define IRQ_ID_TIMER0 0

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

I'd prefer we remove this file and get values from DT.


#define PMU_REG_BASE 0x40008000
#define ADC_REG_BASE 0x40008010

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Remove various REG_BASEs that should come from DT.

#ifndef _ACTIONS_SOC_RESET_H_
#define _ACTIONS_SOC_RESET_H_

#define RESET_ID_DMA 0

This comment has been minimized.

Copy link
@galak

galak Jan 29, 2020

Contributor

Should probably encode these in DTS.

Copy link
Contributor

galak left a comment

Mostly looks good, a number of cleanup comments.

@Mani-Sadhasivam

This comment has been minimized.

Copy link
Collaborator Author

Mani-Sadhasivam commented Jan 29, 2020

Mostly looks good, a number of cleanup comments.

Thanks. I'll address them soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.