[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-08 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467493171



##
File path: hw/mcu/nordic/nrf91xx/include/mcu/mcu.h
##
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __MCU_MCU_H_
+#define __MCU_MCU_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+ * Defines for naming GPIOs. NOTE: the nordic chip docs use numeric labels for
+ * ports. Port A corresponds to Port 0, B to 1, etc. The nrf9160 has only one
+ * port and thus uses pins 0 - 31.
+ */
+#define MCU_GPIO_PORTA(pin) ((0 * 31) + (pin))
+
+#if NRF9160_XXAA
+
+#define MCU_SYSVIEW_INTERRUPTS \
+
"I#1=Reset,I#2=MNI,I#3=HardFault,I#4=MemoryMgmt,I#5=BusFault,I#6=UsageFault," \
+"I#11=SecureFault,I#12=SVCall,I#13=DebugMonitor,I#14=PendSV,I#15=SysTick," 
\
+
"I#16=SPU,I#16=POWER_CLOCK,I#17=UARTE0_SPIM0_SPIS0_TWIM0_TWIS0,I#18=UARTE1_SPIM1_SPIS1_TWIM1_TWIS1,"
 \

Review comment:
   Good catch, I went ahead and made the numbers match the isr_vector so 
that there is a defined way of specifying these going forward.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-08 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467493121



##
File path: hw/mcu/nordic/nrf91xx/syscfg.yml
##
@@ -0,0 +1,455 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+syscfg.defs:
+MCU_TARGET:
+description: >
+Specifies target MCU, shall be set by BSP.
+value:
+restrictions:
+- $notnull
+choices:
+- nRF9160
+
+MCU_FLASH_MIN_WRITE_SIZE:
+description: >
+Specifies the required alignment for internal flash writes.
+Used internally by the newt tool.
+value: 1
+
+MCU_DCDC_ENABLED:
+description: >
+Specifies whether or not to enable DC/DC regulator. This requires
+external circuitry so is defined to be zero by default and
+expected to be overridden by the BSP.
+value: 0
+
+MCU_LFCLK_SOURCE:
+description: >
+Selected source for low frequency clock (LFCLK).
+value:
+choices:
+- LFRC  # 32.768 kHz RC oscillator
+- LFXO  # 32.768 kHz crystal oscillator
+- LFSYNTH   # 32.768 kHz synthesized from HFCLK
+
+MCU_GPIO_USE_PORT_EVENT:
+description: >
+When enabled, hal_gpio will use GPIOTE PORT event instead of PIN
+events for interrupts. This mode may be less accurate (i.e. pulse
+length needs to be longer in order to be detected) but it reduces
+power consumption since it does not require HFCLK to be running.
+Refer to nRF91xxx Product Specification document for more details.
+value: 0
+
+MCU_DEBUG_IGNORE_BKPT:
+   description: >
+When enabled, asm(bkpt) will be ignored. If not set, it will hit
+the breakpoint wherever it gets called, For example, reset and 
crash
+   value: 0
+
+# MCU peripherals definitions
+I2C_0:
+description: 'Enable nRF91xxx I2C 0'

Review comment:
   thanks, done

##
File path: hw/mcu/nordic/nrf91xx/src/hal_timer.c
##
@@ -0,0 +1,926 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "os/mynewt.h"
+#include "mcu/cmsis_nvic.h"
+#include "hal/hal_timer.h"
+#include "hal/nrf_clock.h"
+#include "nrf.h"
+#include "mcu/nrf91_hal.h"
+#include "nrfx_config_nrf9160.h"
+
+/* IRQ prototype */
+typedef void (*hal_timer_irq_handler_t)(void);
+
+/* User CC 2 for reading counter, CC 3 for timer isr */
+#define NRF_TIMER_CC_READ   (2)
+#define NRF_TIMER_CC_INT(3)
+
+/* Output compare 2 used for RTC timers */
+#define NRF_RTC_TIMER_CC_INT(2)
+
+/* Maximum number of hal timers used */
+#define NRF91_HAL_TIMER_MAX (5)
+
+/* Maximum timer frequency */
+#define NRF91_MAX_TIMER_FREQ(1600)
+
+struct nrf91_hal_timer {
+uint8_t tmr_enabled;
+uint8_t tmr_irq_num;
+uint8_t tmr_rtc;
+uint8_t tmr_pad;
+uint32_t tmr_cntr;
+uint32_t timer_isrs;
+uint32_t tmr_freq;
+void *tmr_reg;
+TAILQ_HEAD(hal_timer_qhead, hal_timer) hal_timer_q;
+};
+
+#if MYNEWT_VAL(TIMER_0)
+struct nrf91_hal_timer nrf91_hal_timer0;
+#endif
+#if MYNEWT_VAL(TIMER_1)
+struct nrf91_hal_timer nrf91_hal_timer1;
+#endif
+#if MYNEWT_VAL(TIMER_2)
+struct nrf91_hal_timer nrf91_hal_timer2;
+#endif
+#if MYNEWT_VAL(TIMER_3)
+struct nrf91_hal_timer 

[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467334451



##
File path: hw/mcu/nordic/nrf91xxx/src/hal_timer.c
##
@@ -0,0 +1,926 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "os/mynewt.h"
+#include "mcu/cmsis_nvic.h"
+#include "hal/hal_timer.h"
+#include "hal/nrf_clock.h"
+#include "nrf.h"
+#include "mcu/nrf91_hal.h"
+#include "nrfx_config_nrf9160.h"
+
+/* IRQ prototype */
+typedef void (*hal_timer_irq_handler_t)(void);
+
+/* User CC 2 for reading counter, CC 3 for timer isr */
+#define NRF_TIMER_CC_READ   (2)
+#define NRF_TIMER_CC_INT(3)
+
+/* Output compare 2 used for RTC timers */
+#define NRF_RTC_TIMER_CC_INT(2)
+
+/* Maximum number of hal timers used */
+#define NRF91_HAL_TIMER_MAX (5)
+
+/* Maximum timer frequency */
+#define NRF91_MAX_TIMER_FREQ(1600)
+
+struct nrf91_hal_timer {
+uint8_t tmr_enabled;
+uint8_t tmr_irq_num;
+uint8_t tmr_rtc;
+uint8_t tmr_pad;
+uint32_t tmr_cntr;
+uint32_t timer_isrs;
+uint32_t tmr_freq;
+void *tmr_reg;
+TAILQ_HEAD(hal_timer_qhead, hal_timer) hal_timer_q;
+};
+
+#if MYNEWT_VAL(TIMER_0)
+struct nrf91_hal_timer nrf91_hal_timer0;
+#endif
+#if MYNEWT_VAL(TIMER_1)
+struct nrf91_hal_timer nrf91_hal_timer1;
+#endif
+#if MYNEWT_VAL(TIMER_2)
+struct nrf91_hal_timer nrf91_hal_timer2;
+#endif
+#if MYNEWT_VAL(TIMER_3)
+struct nrf91_hal_timer nrf91_hal_timer3;
+#endif
+#if MYNEWT_VAL(TIMER_4)

Review comment:
   Why won't it work ? It is not being used for the OS Tick, it is just 
used as a normal timer here.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467335632



##
File path: hw/mcu/nordic/nrf91xxx/src/hal_spi.c
##
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include 
+#include 
+#include "os/mynewt.h"
+#include 
+#include 
+#include "mcu/nrf91_hal.h"
+#include "nrf.h"
+#include "nrfx_config_nrf9160.h"
+
+#ifndef min
+#define min(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
+#define SPIM_TXD_MAXCNT_MAX 65535
+
+/* IRQ handler type */
+typedef void (*nrf91_spi_irq_handler_t)(void);
+
+/* XXX:
+ * 1) what about stats?
+ * 2) Dealing with errors (OVERFLOW, OVERREAD)
+ * 3) Dont think I need dummy_rx as I can set master RX maxcnt to zero.
+ */
+
+/* The maximum number of SPI interfaces we will allow */
+#define NRF91_HAL_SPI_MAX (4)
+
+/* Used to disable all interrupts */
+#define NRF_SPI_IRQ_DISABLE_ALL 0x
+
+/*
+ *  Slave states
+ *
+ *  IDLE: Slave not ready to be used. If master attempts to access
+ *slave it will receive the default character
+ *  ACQ_SEM: Slave is attempting to acquire semaphore.
+ *  READY: Slave is ready for master to send it data
+ *
+ */
+#define HAL_SPI_SLAVE_STATE_IDLE(0)
+#define HAL_SPI_SLAVE_STATE_ACQ_SEM (1)
+#define HAL_SPI_SLAVE_STATE_READY   (2)
+
+struct nrf91_hal_spi {
+uint8_t spi_type;
+uint8_t spi_xfr_flag;   /* Master only */
+uint8_t dummy_rx;   /* Master only */
+uint8_t slave_state;/* Slave only */
+uint16_t nhs_buflen;

Review comment:
   but you are right for this one I am just going to remove the prefix, not 
needed. 





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467334451



##
File path: hw/mcu/nordic/nrf91xxx/src/hal_timer.c
##
@@ -0,0 +1,926 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "os/mynewt.h"
+#include "mcu/cmsis_nvic.h"
+#include "hal/hal_timer.h"
+#include "hal/nrf_clock.h"
+#include "nrf.h"
+#include "mcu/nrf91_hal.h"
+#include "nrfx_config_nrf9160.h"
+
+/* IRQ prototype */
+typedef void (*hal_timer_irq_handler_t)(void);
+
+/* User CC 2 for reading counter, CC 3 for timer isr */
+#define NRF_TIMER_CC_READ   (2)
+#define NRF_TIMER_CC_INT(3)
+
+/* Output compare 2 used for RTC timers */
+#define NRF_RTC_TIMER_CC_INT(2)
+
+/* Maximum number of hal timers used */
+#define NRF91_HAL_TIMER_MAX (5)
+
+/* Maximum timer frequency */
+#define NRF91_MAX_TIMER_FREQ(1600)
+
+struct nrf91_hal_timer {
+uint8_t tmr_enabled;
+uint8_t tmr_irq_num;
+uint8_t tmr_rtc;
+uint8_t tmr_pad;
+uint32_t tmr_cntr;
+uint32_t timer_isrs;
+uint32_t tmr_freq;
+void *tmr_reg;
+TAILQ_HEAD(hal_timer_qhead, hal_timer) hal_timer_q;
+};
+
+#if MYNEWT_VAL(TIMER_0)
+struct nrf91_hal_timer nrf91_hal_timer0;
+#endif
+#if MYNEWT_VAL(TIMER_1)
+struct nrf91_hal_timer nrf91_hal_timer1;
+#endif
+#if MYNEWT_VAL(TIMER_2)
+struct nrf91_hal_timer nrf91_hal_timer2;
+#endif
+#if MYNEWT_VAL(TIMER_3)
+struct nrf91_hal_timer nrf91_hal_timer3;
+#endif
+#if MYNEWT_VAL(TIMER_4)

Review comment:
   Why won't it work ? It is not being used as for the OS Tick, it is just 
used as a normal timer here.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467334083



##
File path: 
hw/bsp/nordic_pca10090/src/arch/cortex_m33/gcc_startup_nrf9160_split.s
##
@@ -0,0 +1,166 @@
+/*
+Copyright (c) 2015, Nordic Semiconductor ASA
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are met:
+
+* Redistributions of source code must retain the above copyright notice, this
+  list of conditions and the following disclaimer.
+
+* Redistributions in binary form must reproduce the above copyright notice,
+  this list of conditions and the following disclaimer in the documentation
+  and/or other materials provided with the distribution.
+
+* Neither the name of Nordic Semiconductor ASA nor the names of its
+  contributors may be used to endorse or promote products derived from
+  this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+*/
+
+/*
+NOTE: Template files (including this one) are application specific and 
therefore
+expected to be copied into the application project folder prior to its use!
+*/
+
+.syntax unified
+.arch armv8-m.main
+.section .stack
+.align 3
+.equStack_Size, 432
+.globl__StackTop
+.globl__StackLimit
+__StackLimit:
+.spaceStack_Size
+.size __StackLimit, . - __StackLimit
+__StackTop:
+.size __StackTop, . - __StackTop
+
+.section .heap
+.align 3
+#ifdef __HEAP_SIZE
+.equHeap_Size, __HEAP_SIZE
+#else
+.equHeap_Size, 0
+#endif
+.globl__HeapBase
+.globl__HeapLimit
+__HeapBase:
+.ifHeap_Size
+.spaceHeap_Size
+.endif
+.size __HeapBase, . - __HeapBase
+__HeapLimit:
+.size __HeapLimit, . - __HeapLimit
+
+.section .isr_vector_split
+.align 2
+.globl __isr_vector_split
+__isr_vector_split:
+.long__StackTop/* Top of Stack */
+.long   Reset_Handler_split   /* Reset Handler */
+
+.size__isr_vector_split, . - __isr_vector_split
+
+/* Reset Handler */
+
+.text
+.thumb
+.thumb_func
+.align 1
+.globlReset_Handler_split
+.typeReset_Handler_split, %function
+Reset_Handler_split:
+.fnstart
+
+/* Clear CPU state before proceeding */
+mov r0, #0
+msr control, r0
+msr primask, r0
+/* Clear BSS */
+ldr r2, =__bss_start__
+ldr r3, =__bss_end__
+.bss_zero_loop:
+cmp r2, r3
+itt lt
+strlt   r0, [r2], #4
+blt.bss_zero_loop
+
+
+/* Loop to copy data from read only memory to RAM. The ranges
+ *  of copy from/to are specified by following symbols evaluated in
+ *  linker script.
+ *  __etext: End of code section, i.e., begin of data sections to copy 
from.
+ *  __data_start__/__data_end__: RAM address range that data should be
+ *  copied to. Both must be aligned to 4 bytes boundary.  */
+
+ldrr1, =__etext
+ldrr2, =__data_start__
+ldrr3, =__data_end__
+
+subsr3, r2
+ble .LC0
+
+.LC1:
+subsr3, 4
+ldrr0, [r1,r3]
+strr0, [r2,r3]
+bgt.LC1
+
+.LC0:
+ldrr1, =__etext_loader
+ldrr2, =__data_start___loader
+ldrr3, =__data_end___loader
+
+subsr3, r2
+ble .LC2
+
+.LC3:
+subsr3, 4
+ldrr0, [r1,r3]
+strr0, [r2,r3]
+bgt.LC3
+.LC2:
+
+subsr0, r0
+ldrr2, =__bss_start___loader
+ldrr3, =__bss_end___loader
+
+subsr3, r2
+ble .LC4
+
+.LC5:
+subsr3, 4
+strr0, [r2,r3]
+bgt.LC5
+.LC4:
+
+LDR R0, =__HeapBase

Review comment:
   Copy paste from nrf52, but I will fix it, thanks.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467333871



##
File path: hw/bus/drivers/i2c_nrf91_twim/src/i2c_nrf91_twim.c
##
@@ -0,0 +1,664 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include "defs/error.h"
+#include "hal/hal_gpio.h"
+#include "bus/bus.h"
+#include "bus/bus_debug.h"
+#include "bus/bus_driver.h"
+#include "bus/drivers/i2c_common.h"
+#include "bus/drivers/i2c_nrf91_twim.h"
+#include "mcu/nrf91_hal.h"
+#include "nrfx.h"
+#if MYNEWT_VAL(I2C_NRF91_TWIM_STAT)
+#include "stats/stats.h"
+#endif
+
+#define TWIM_GPIO_PIN_CNF \
+((GPIO_PIN_CNF_SENSE_Disabled << GPIO_PIN_CNF_SENSE_Pos) |  \
+ (GPIO_PIN_CNF_DRIVE_S0D1 << GPIO_PIN_CNF_DRIVE_Pos) |  \
+ (GPIO_PIN_CNF_PULL_Pullup << GPIO_PIN_CNF_PULL_Pos) |  \
+ (GPIO_PIN_CNF_INPUT_Connect << GPIO_PIN_CNF_INPUT_Pos) |   \
+ (GPIO_PIN_CNF_DIR_Input << GPIO_PIN_CNF_DIR_Pos))
+
+#if MYNEWT_VAL(I2C_NRF91_TWIM_STAT)
+STATS_SECT_START(twim_stats_section)
+STATS_SECT_ENTRY(sda_lo_err)/* SDA pulled low on r/w */

Review comment:
   Yeah, I figured a lot of the rules are not very consistent in that 
check, probably needs some tweaking another issue to be filed.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467333691



##
File path: hw/mcu/nordic/nrf91xxx/include/mcu/mcu.h
##
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef __MCU_MCU_H_
+#define __MCU_MCU_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/*
+ * Defines for naming GPIOs. NOTE: the nordic chip docs use numeric labels for
+ * ports. Port A corresponds to Port 0, B to 1, etc. The nrf9160 has only one
+ * port and thus uses pins 0 - 31.
+ */
+#define MCU_GPIO_PORTA(pin) ((0 * 31) + (pin))
+
+#if NRF9160_XXAA
+
+#define MCU_SYSVIEW_INTERRUPTS \
+
"I#1=Reset,I#2=MNI,I#3=HardFault,I#4=MemoryMgmt,I#5=BusFault,I#6=UsageFault," \

Review comment:
   What wrong values ? Spaces not needed I agree but it doesn't matter, in 
fact in the first place spaces should have been there since in the coding style 
we need spaces before and after an '='. To keep it consistent I am going to 
remove the spaces.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467331054



##
File path: hw/mcu/nordic/nrf91xxx/src/hal_spi.c
##
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include 
+#include 
+#include "os/mynewt.h"
+#include 
+#include 
+#include "mcu/nrf91_hal.h"
+#include "nrf.h"
+#include "nrfx_config_nrf9160.h"
+
+#ifndef min
+#define min(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
+#define SPIM_TXD_MAXCNT_MAX 65535
+
+/* IRQ handler type */
+typedef void (*nrf91_spi_irq_handler_t)(void);
+
+/* XXX:
+ * 1) what about stats?
+ * 2) Dealing with errors (OVERFLOW, OVERREAD)
+ * 3) Dont think I need dummy_rx as I can set master RX maxcnt to zero.
+ */
+
+/* The maximum number of SPI interfaces we will allow */
+#define NRF91_HAL_SPI_MAX (4)
+
+/* Used to disable all interrupts */
+#define NRF_SPI_IRQ_DISABLE_ALL 0x
+
+/*
+ *  Slave states
+ *
+ *  IDLE: Slave not ready to be used. If master attempts to access
+ *slave it will receive the default character
+ *  ACQ_SEM: Slave is attempting to acquire semaphore.
+ *  READY: Slave is ready for master to send it data
+ *
+ */
+#define HAL_SPI_SLAVE_STATE_IDLE(0)
+#define HAL_SPI_SLAVE_STATE_ACQ_SEM (1)
+#define HAL_SPI_SLAVE_STATE_READY   (2)
+
+struct nrf91_hal_spi {
+uint8_t spi_type;
+uint8_t spi_xfr_flag;   /* Master only */
+uint8_t dummy_rx;   /* Master only */
+uint8_t slave_state;/* Slave only */
+uint16_t nhs_buflen;

Review comment:
   Good point, since this was a copy of nrf52 for the most part, it got 
ported over. I guess we should file an issue for the prefixes for nrf52 as well.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [mynewt-core] vrahane commented on a change in pull request #2348: Add NRF9160 BSP with I2C, SPI, UART and GPIO support

2020-08-07 Thread GitBox


vrahane commented on a change in pull request #2348:
URL: https://github.com/apache/mynewt-core/pull/2348#discussion_r467330046



##
File path: hw/mcu/nordic/nrf91xxx/src/nrf91_periph.c
##
@@ -0,0 +1,443 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include 
+#include 
+#include "syscfg/syscfg.h"
+#include "mcu/nrf91_hal.h"
+#include "hal/hal_i2c.h"
+#include "hal/hal_spi.h"
+#include "bsp/bsp.h"
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include "bus/bus.h"
+#if MYNEWT_VAL(I2C_0) || MYNEWT_VAL(I2C_1) || MYNEWT_VAL(I2C_2) || 
MYNEWT_VAL(I2C_3)
+#if MYNEWT_VAL(MCU_BUS_DRIVER_I2C_USE_TWIM)
+#include "bus/drivers/i2c_nrf91_twim.h"
+#else
+#include "bus/drivers/i2c_hal.h"
+#endif
+#endif
+#if MYNEWT_VAL(SPI_0_MASTER) || MYNEWT_VAL(SPI_1_MASTER) || 
MYNEWT_VAL(SPI_2_MASTER) || MYNEWT_VAL(SPI_3_MASTER)
+#include "bus/drivers/spi_hal.h"
+#endif
+#endif
+#include "nrfx.h"
+#if MYNEWT_VAL(UART_0) || MYNEWT_VAL(UART_1) || MYNEWT_VAL(UART_3) || 
MYNEWT_VAL(UART_4)
+#include "uart/uart.h"
+#include "uart_hal/uart_hal.h"
+#endif
+
+#if MYNEWT_VAL(UART_0)
+static struct uart_dev os_bsp_uart0;
+static const struct nrf91_uart_cfg os_bsp_uart0_cfg = {
+.suc_pin_tx = MYNEWT_VAL(UART_0_PIN_TX),
+.suc_pin_rx = MYNEWT_VAL(UART_0_PIN_RX),
+.suc_pin_rts = MYNEWT_VAL(UART_0_PIN_RTS),
+.suc_pin_cts = MYNEWT_VAL(UART_0_PIN_CTS),
+};
+#endif
+#if MYNEWT_VAL(UART_1)
+static struct uart_dev os_bsp_uart1;
+static const struct nrf91_uart_cfg os_bsp_uart1_cfg = {
+.suc_pin_tx = MYNEWT_VAL(UART_1_PIN_TX),
+.suc_pin_rx = MYNEWT_VAL(UART_1_PIN_RX),
+.suc_pin_rts = MYNEWT_VAL(UART_1_PIN_RTS),
+.suc_pin_cts = MYNEWT_VAL(UART_1_PIN_CTS),
+};
+#endif
+#if MYNEWT_VAL(UART_2)
+static struct uart_dev os_bsp_uart2;
+static const struct nrf91_uart_cfg os_bsp_uart2_cfg = {
+.suc_pin_tx = MYNEWT_VAL(UART_2_PIN_TX),
+.suc_pin_rx = MYNEWT_VAL(UART_2_PIN_RX),
+.suc_pin_rts = MYNEWT_VAL(UART_2_PIN_RTS),
+.suc_pin_cts = MYNEWT_VAL(UART_2_PIN_CTS),
+};
+#endif
+#if MYNEWT_VAL(UART_3)
+static struct uart_dev os_bsp_uart3;
+static const struct nrf91_uart_cfg os_bsp_uart3_cfg = {
+.suc_pin_tx = MYNEWT_VAL(UART_3_PIN_TX),
+.suc_pin_rx = MYNEWT_VAL(UART_3_PIN_RX),
+.suc_pin_rts = MYNEWT_VAL(UART_3_PIN_RTS),
+.suc_pin_cts = MYNEWT_VAL(UART_3_PIN_CTS),
+};
+#endif
+
+#if MYNEWT_VAL(I2C_0)
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+static const struct bus_i2c_dev_cfg i2c0_cfg = {
+.i2c_num = 0,
+.pin_sda = MYNEWT_VAL(I2C_0_PIN_SDA),
+.pin_scl = MYNEWT_VAL(I2C_0_PIN_SCL),
+};
+static struct bus_i2c_dev i2c0_bus;
+#else
+static const struct nrf91_hal_i2c_cfg hal_i2c0_cfg = {
+.scl_pin = MYNEWT_VAL(I2C_0_PIN_SCL),
+.sda_pin = MYNEWT_VAL(I2C_0_PIN_SDA),
+.i2c_frequency = MYNEWT_VAL(I2C_0_FREQ_KHZ),
+};
+#endif
+#endif
+#if MYNEWT_VAL(I2C_1)
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+static const struct bus_i2c_dev_cfg i2c1_cfg = {
+.i2c_num = 1,
+.pin_sda = MYNEWT_VAL(I2C_1_PIN_SDA),
+.pin_scl = MYNEWT_VAL(I2C_1_PIN_SCL),
+};
+static struct bus_i2c_dev i2c1_bus;
+#else
+static const struct nrf91_hal_i2c_cfg hal_i2c1_cfg = {
+.scl_pin = MYNEWT_VAL(I2C_1_PIN_SCL),
+.sda_pin = MYNEWT_VAL(I2C_1_PIN_SDA),
+.i2c_frequency = MYNEWT_VAL(I2C_1_FREQ_KHZ),
+};
+#endif
+#endif
+#if MYNEWT_VAL(I2C_2)
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+static const struct bus_i2c_dev_cfg i2c2_cfg = {
+.i2c_num = 0,

Review comment:
   good catch, thanks.

##
File path: hw/mcu/nordic/nrf91xxx/src/nrf91_periph.c
##
@@ -0,0 +1,443 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF