SensorTag basic board support #68

ghost · 2017-05-03T13:00:01Z

Basic board support for the TI SensorTag, with minimal drivers.

Switch from Gerrit to Github for this changeset
Split linker script modification and minimal SoC support into 2 commits
Move uart.h, gpio.h and trng.h to corresponding drivers

Basic board support for the TI SensorTag, with minimal drivers.
Questions remain to be answered, like Stellaris / CC3200 UART driver reuse
and how to accomodate HW-specific flash header / footer needs by modifying
the Cortex generic linker script.

galak

So the patch series should be re-ordered slightly, the linker script should come first, than SoC (merge the cc2650: Add basic support for Device Tree. patch with SoC patch).

Let's also see about the UART driver.

ghost · 2017-05-03T15:05:40Z

Sorry about that, commit dates mixed up the order when pushing.
Squashed SoC + Device Tree support accordingly.

ghost · 2017-05-05T14:09:42Z

Had to move gpio.h to arch/ again, because it is also needed by pinmux driver
(hardware awkardness).
Tried to use both the stellaris and the CC32xx UART drivers.
- With CC32xx UART: TI provides a HAL for CC26xx SoC, "cc26xxware". It is
  different than the HAL used for CC3200 port ("cc32xxware"). Not surprisingly, the CC32xx
  UART driver does not work for the CC2650 (probably power management issues, but see below).
- With Stellaris UART: Polling interface works, interrupts do not. Pushed code is a quick hack
  to show what's needed to make it work; most importantly, powering on the UART HW module
  used to be done in CC2650 UART driver and was copy-pasted here.

I have the following questions:

What's the policy regarding HALs for Zephyr? I could not find any. If that's preferred,
I could import TI's cc26xxware HAL along cc32xxware and use it everywhere.
Regarding SoC power management, should I power on everything that may be needed
in SoC's init() (in arch/) and not in each individual driver?
There is still a problem when using Device Tree generated macros: the address is
baked in their names. Here, a quick hack in the board's .fixup file does the job; but
why not using generic names? We could generate names using port numbers, for example.

GAnthony · 2017-05-05T17:25:38Z

Regarding policy on HALs, my understanding is that they are initially being encouraged as a way to expand board support in Zephyr, though I've heard some comments against HALs if they bring in too much extra code. In the case of CC32xx, I was able to save ~2K in code space by using the ROM versions of the cc32xx driverlib functions: MAP_*.

I wonder if there is a SimpleLink SDK for cc26xx? The following link seems to suggest that is the case:
http://dev.ti.com/tirex/content/simplelink_cc13x0_sdk_1_30_00_06/docs/simplelink_mcu_sdk/Users_Guide.html#directory-structure
In which case, cc26xx device files could go under the new ext/ti/hal/simplelink/ directory structure, if you choose to use them.

GAnthony · 2017-05-05T17:28:09Z

Regarding power management, the cc32xx UART driver does not currently support power management, so is not a good example. I would think it's best for the device driver to provide the power management hooks, and initialization.

galak · 2017-05-05T19:50:42Z

drivers/serial/uart_stellaris.c

could this be done in soc.c for cc2650?

galak · 2017-05-05T19:53:20Z

With regards to DT generated, we intend to generate the code bits so the #defines go away, the problem with names is its hard to come up with a generic solution that works for everyone. Some people start at 0, some want it to be the name the board uses, and not the SoC, etc.

So for now the fixup file exists until we can generate the bits instead that are in the driver code.

ghost · 2017-05-09T09:29:01Z

Move inline register manipulations functions to soc.c.
Move power/clock setup of HW modules to soc.c, and remove such code from device drivers, where it was previously located.
Document the Stellaris UART driver to explain use for CC2650 SoC.

galak · 2017-05-23T12:56:26Z

Sorry for taking so long on this, headed on vacation for a few days, but will focus on getting this reviewed and merged next week. Thanks.

ghost · 2017-05-23T14:37:43Z

No problem, I tried to keep conflicts minimal to avoid headaches upon merging.

galak

A number of changes needed. Lets have the 'uart' patch be the first patch, since its just cleaning / commenting on support for CC2650. Also, lets pull the random # patch from this PR so we can get SoC/Board support in.

galak · 2017-06-09T18:44:49Z

include/arch/arm/cortex_m/scripts/linker.ld

Why is CCFG_SIZE not just hard coded value here? Does the size change device to device?

No, I thought initially that other TI devices shared a similar CCFG concept with different options, but I only find this for the CC13xx / CC26xx SoCs. I can remove CONFIG_TI_CCFG_SIZE and hardcode it here, if that's okay.

galak · 2017-06-09T18:45:23Z

include/arch/arm/cortex_m/scripts/linker.ld

ROM_ADDR is defined on line 43, 45, and 52. Lets remove lines 45 & 52.

galak · 2017-06-09T18:46:59Z

include/arch/arm/cortex_m/scripts/linker.ld

Lets add a comment here about:

/* Some TI SoCs have a cfg footer at the end of flash for device configuration */

galak · 2017-06-09T18:47:46Z

arch/arm/soc/ti_simplelink/cc2650/Kconfig.soc

When would this value ever be something other than 88?

galak · 2017-06-09T18:50:25Z

arch/arm/soc/ti_simplelink/cc2650/soc.c

__ti_ccfg_section instead of _GENERIC_SECTION()

galak · 2017-06-09T19:03:30Z

drivers/serial/uart_stellaris.c

What do we need from soc.h?

galak · 2017-06-09T19:03:49Z

drivers/serial/uart_stellaris.c

Lets just remove usage of UART_IRQ_FLAGS, just change the function to pass a 0.

galak · 2017-06-09T19:04:41Z

drivers/pinmux/Kconfig.cc2650

since pin mux depends on GPIO driver, the GPIO driver should be first in the sequence of commits.

galak · 2017-06-09T19:05:54Z

drivers/pinmux/pinmux_cc2650.c

you shouldn't need these prototypes, re-order the code in the file if needed so they can be removed.

galak · 2017-06-09T19:06:44Z

drivers/gpio/gpio_cc2650.c

remove this, just call function with 0.

galak · 2017-06-09T19:09:13Z

drivers/random/random_cc2650.c

do we really need toolchain/gcc.h?, trim headres down to what's needed.

I'm just using the ARG_UNUSED() macro.

galak · 2017-06-09T19:09:50Z

drivers/random/random_cc2650.c

should look like:

SPDX-License-Identifier: Apache-2.0

galak

some minor comments on random driver.

ghost · 2017-06-15T13:56:29Z

Reorganized commits so that GPIO driver comes before pinmux driver,
Removed random number generator driver, for another pull request,
Removed CCFG_SIZE config variable, now hard-coded in linker script,
Various small fixes,
Fixed license to "SPDX-License-Identifier: Apache-2.0"

galak · 2017-06-15T14:58:40Z

Can you rebase the patch set on latest 'arm' branch. Thanks.

galak · 2017-06-15T15:01:37Z

arch/arm/soc/ti_simplelink/cc2650/soc.c

nit picking, but we should probably just have bit_is_set as a static inline in the header.

Points of view seem to differ on this. I was asked exactly the opposite (implement functions in soc.c separately from soc.h) some weeks ago.

galak · 2017-06-15T15:14:47Z

dts/arm/ti/cc2650.dtsi

should be: compatible = "ti,stellaris-uart";

galak · 2017-06-15T15:15:39Z

dts/arm/ti/cc2650.dtsi

This doesn't look right, the number of bits should probably be 3? Guessing 8 priority levels.

Good catch, it looks like I mixed up the Cortex M3's user guide number of prio levels (256) with the ones available for the CC2650 SoC (8 indeed).

galak

Some dts related changes need to be made.

galak · 2017-06-15T15:44:32Z

I'm going to pull in the linker script and modify the uart driver patch into the arm branch so we can close out on those bits.

ghost · 2017-06-15T16:13:57Z

Fixed number of priority bits for interrupts (DTSI file)
Fixed UART driver used (my fault, don't know where I changed the configuration again)

galak · 2017-06-15T16:22:00Z

drivers/serial/uart_stellaris.c

Why this change, we should keep things as they are and use label in device tree like other uarts are using.

galak

uart patch still needs work, in theory we should be able to remove it.

galak · 2017-06-15T16:22:22Z

dts/arm/cc2650_sensortag.fixup

as mentioned before, merge this change into the board patch.

ghost · 2017-06-16T07:51:58Z

Changed Stellaris driver to use DTS-provided labels. Moved this patch upfront so it can be moved out quickly.
CC2650 related work: Moved .fixup changes to board patch.

galak

the uart: Use DTS labels... commit, should be first, and should also remove the defines from dts/arm/qemu_cortex_m3.fixup.

#define CONFIG_UART_STELLARIS_PORT_0_NAME TI_STELLARIS_UART_4000C000_LABEL
#define CONFIG_UART_STELLARIS_PORT_1_NAME TI_STELLARIS_UART_4000D000_LABEL
#define CONFIG_UART_STELLARIS_PORT_2_NAME TI_STELLARIS_UART_4000E000_LABEL

galak · 2017-06-16T12:46:39Z

dts/arm/ti/cc2650.dtsi

I'd expect this in the SOC commit.

galak

Remove GPIO_INT_PIN_EXTRAFLAGS, doesn't seem to add anything.

galak · 2017-06-16T12:48:57Z

samples/drivers/gpio/src/main.c

is there really a need to introduce GPIO_INT_PIN_EXTRAFLAGS since its only used in the CC2650 ifdef case?

galak · 2017-06-16T12:49:24Z

samples/drivers/gpio/src/main.c

Why not just replace GPIO_INT_PIN_EXTRAFLAGS with GPIO_PUD_PULL_UP, not sure what the #define is getting us.

For the Sensortag, I needed an extra GPIO_PUD_PULL_UP flag.
As this is probably not the only board needing to pass some extra flags for now, I added a little bit of flexibility, so other boards in the future can define (or not) more flags as needed while keeping code readable.
So yes, for now it's only used for the CC2650 but it may prove useful as the test code becomes used for more boards.

I tend to do such things once we see the need for them.

Alright, I'll just use GPIO_PUD_PULL_UP then.

Update driver to use DTS-generated #defines for port names, and not obsolete Kconfig variables. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

Add support in arch/arm/soc/ti_simplelink, along with support for CC32xx SoC. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

Add support for TI's SensorTag board, which uses a CC2650 SoC. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

Add extra #defines in samples/drivers/gpio to test TI SensorTag board. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

ghost · 2017-06-16T15:43:36Z

Moved Stellaris UART driver update commit as first commit.
Simplify GPIO driver sample code.

ghost · 2017-06-16T18:30:31Z

Thanks a lot Kumar for your work and patience! I'm happy to make my first steps in the Zephyr project. :)

galak · 2017-06-16T18:54:01Z

Thanks for your patience as well. Hope to see some more development from you in the future!

…-sample [samples] Fix two bugs in TwoButtons.js sample

galak self-assigned this May 3, 2017

galak self-requested a review May 3, 2017 14:40

galak requested changes May 3, 2017

View reviewed changes

galak force-pushed the arm branch 2 times, most recently from 78c0221 to a1c74b7 Compare May 4, 2017 14:02

galak reviewed May 5, 2017

View reviewed changes

drivers/serial/uart_stellaris.c Outdated

Copy link

Contributor

galak May 5, 2017

Choose a reason for hiding this comment

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

could this be done in soc.c for cc2650?

galak force-pushed the arm branch from 2187c1c to 5802867 Compare May 8, 2017 12:58

galak force-pushed the arm branch from 34a8649 to 255f198 Compare May 9, 2017 15:42

galak force-pushed the arm branch from 42dad4a to ed187eb Compare May 19, 2017 17:40

galak force-pushed the arm branch 3 times, most recently from 1d365cc to 0a8ad73 Compare June 9, 2017 14:38

galak requested changes Jun 9, 2017

View reviewed changes

galak reviewed Jun 9, 2017

View reviewed changes

drivers/random/random_cc2650.c Outdated

Copy link

Contributor

galak Jun 9, 2017

Choose a reason for hiding this comment

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

should look like:

SPDX-License-Identifier: Apache-2.0

galak reviewed Jun 9, 2017

View reviewed changes

galak force-pushed the arm branch from adce86e to 8b5c982 Compare June 12, 2017 16:45

galak reviewed Jun 15, 2017

View reviewed changes

dts/arm/ti/cc2650.dtsi Outdated

Copy link

Contributor

galak Jun 15, 2017

Choose a reason for hiding this comment

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

should be: compatible = "ti,stellaris-uart";

galak reviewed Jun 15, 2017

View reviewed changes

galak requested changes Jun 15, 2017

View reviewed changes

galak reviewed Jun 15, 2017

View reviewed changes

galak requested changes Jun 15, 2017

View reviewed changes

dts/arm/cc2650_sensortag.fixup Outdated

Copy link

Contributor

galak Jun 15, 2017

Choose a reason for hiding this comment

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

as mentioned before, merge this change into the board patch.

galak force-pushed the arm branch 2 times, most recently from 8176617 to 49326ad Compare June 15, 2017 21:15

galak requested changes Jun 16, 2017

View reviewed changes

galak reviewed Jun 16, 2017

View reviewed changes

dts/arm/ti/cc2650.dtsi Outdated

Copy link

Contributor

galak Jun 16, 2017

Choose a reason for hiding this comment

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

I'd expect this in the SOC commit.

galak requested changes Jun 16, 2017

View reviewed changes

Geoffrey Le Gourriérec added 6 commits June 16, 2017 17:42

uart: Use DTS labels for Stellaris driver.

5bc764c

Update driver to use DTS-generated #defines for port names, and not obsolete Kconfig variables. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

arm: Add support for TI's CC2650 SoC.

c9bedc5

Add support in arch/arm/soc/ti_simplelink, along with support for CC32xx SoC. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

sensortag: Add TI's SensorTag board.

fee9aaa

Add support for TI's SensorTag board, which uses a CC2650 SoC. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

cc2650: Add GPIO driver.

ab6127d

Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

cc2650: Add pinmux driver.

8837876

Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

samples: gpio: Add support for SensorTag board.

d927332

Add extra #defines in samples/drivers/gpio to test TI SensorTag board. Signed-off-by: Geoffrey Le Gourriérec <[email protected]>

galak approved these changes Jun 16, 2017

View reviewed changes

galak merged this pull request into zephyrproject-rtos:arm Jun 16, 2017

zephyrbot mentioned this pull request Sep 23, 2017

nano_timer will never expired @ARC @ARM when started and tested from ISR #1724

Closed

nagineni pushed a commit to nagineni/zephyr that referenced this pull request Nov 20, 2017

Merge pull request zephyrproject-rtos#68 from grgustaf/fix-twobuttons…

794e5b5

…-sample [samples] Fix two bugs in TwoButtons.js sample

MariuszSkamra mentioned this pull request Jul 15, 2019

L2CAP: On insufficient authentication error received, Zephyr does unauthenticated pairing #17525

Closed

piotrParf mentioned this pull request Feb 9, 2022

BLE Testing functions do not work properly #42534

Closed

SensorTag basic board support #68

SensorTag basic board support #68

Uh oh!

Conversation

ghost commented May 3, 2017

Uh oh!

galak left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented May 3, 2017

Uh oh!

ghost commented May 5, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GAnthony commented May 5, 2017

Uh oh!

GAnthony commented May 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

galak commented May 5, 2017

Uh oh!

ghost commented May 9, 2017

Uh oh!

galak commented May 23, 2017

Uh oh!

ghost commented May 23, 2017

Uh oh!

galak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

galak left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Jun 15, 2017

Uh oh!

galak commented Jun 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost Jun 15, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ghost commented May 5, 2017 •

edited by ghost

Loading

ghost Jun 15, 2017 •

edited by ghost

Loading

ghost commented Jun 16, 2017 •

edited by ghost

Loading

ghost Jun 16, 2017 •

edited by ghost

Loading