You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Kevin Townsend <ke...@adafruit.com> on 2016/11/17 01:19:09 UTC
nRF52DK BSP SPI Conflict?
I'm just starting to look at the SPI HAL to implement a test driver, but
looking through the hal_spi code I see what might be a conflict.
In the master branch of apache-mynewt-core the nRF52SK BSP has both SPI0
and SPIS0 enabled:
* https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/include/bsp/nrf_drv_config.h#L211
* https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/include/bsp/nrf_drv_config.h#L252
Won't this lead to a situation where the hal_bsp_init function will
always initialize in slave mode, when end users might be expecting
master mode if they don't read the code carefully:
https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/src/hal_bsp.c#L217
Perhaps these should be exclusive and an #error is raised if both master
and slave are enabled on the same bus (which would also apply for I2C)?
The two options don't seem to be possible at the same time since there
are separate init functions in the MCU HAL SPI code:
https://github.com/apache/incubator-mynewt-core/blob/master/hw/mcu/nordic/nrf52xxx/src/hal_spi.c#L620
... and the pins will have to be configured in different directions (SCK
will be input in one mode and output in the other, etc.).
Kevin
Re: nRF52DK BSP SPI Conflict?
Posted by will sanfilippo <wi...@runtime.io>.
They are going to be committed shortly so you should see the commit soon.
> On Nov 18, 2016, at 11:54 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>
> Oh good to know since I started working on an SPI driver today. I'll hold
> off until the HAL changes are live then.
>
> Le ven. 18 nov. 2016 à 20:49, will sanfilippo <wi...@runtime.io> a écrit :
>
>> No, no need to do that as I am currently modifying the spi hal so that we
>> dont have license issues with the nordic SDK. I will commit changes to the
>> syscfg.yml files when I do this.
>>
>> Will
>>> On Nov 18, 2016, at 4:09 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>>>
>>> On 17/11/16 05:15, will sanfilippo wrote:
>>>> There is an easy way to prevent this as well which should be added to
>> syscfg.yml. The following works:
>>>>
>>>> SPI_0_MASTER:
>>>> description: 'SPI 0 master'
>>>> value: 1
>>>> restrictions:
>>>> - "!SPI_0_SLAVE"
>>>> SPI_0_SLAVE:
>>>> description: 'SPI 0 slave'
>>>> value: 1
>>>> restrictions:
>>>> - "!SPI_0_MASTER"
>>>> The newt tool generates a nice error message:
>>>>
>>>> Error: Syscfg restriction violations detected:
>>>> SPI_0_MASTER=1 requires SPI_0_SLAVE not be set, but SPI_0_SLAVE=1
>>>> SPI_0_SLAVE=1 requires SPI_0_MASTER not be set, but SPI_0_MASTER=1
>>>>
>>>> Setting history (newest -> oldest):
>>>> SPI_0_MASTER: [hw/bsp/nrf52dk:1]
>>>> SPI_0_SLAVE: [hw/bsp/nrf52dk:1]
>>>
>>> I wasn't aware of the restrictions flag, that's nice to know about.
>>>
>>> Should I submit a pull request to resolve the conflict in master, though
>> (for at least the nRF52DK, I haven' t checked the other BSP packages)?
>>>
>>> I'd propose only enabling SPI_0_MASTER by default, but I'm perhaps
>> missing some specific intentions with this as well (having both MASTER and
>> SLAVE enabled in the BSP).
>>>
>>>
>>
>>
Re: nRF52DK BSP SPI Conflict?
Posted by Kevin Townsend <ke...@adafruit.com>.
Oh good to know since I started working on an SPI driver today. I'll hold
off until the HAL changes are live then.
Le ven. 18 nov. 2016 à 20:49, will sanfilippo <wi...@runtime.io> a écrit :
> No, no need to do that as I am currently modifying the spi hal so that we
> dont have license issues with the nordic SDK. I will commit changes to the
> syscfg.yml files when I do this.
>
> Will
> > On Nov 18, 2016, at 4:09 AM, Kevin Townsend <ke...@adafruit.com> wrote:
> >
> > On 17/11/16 05:15, will sanfilippo wrote:
> >> There is an easy way to prevent this as well which should be added to
> syscfg.yml. The following works:
> >>
> >> SPI_0_MASTER:
> >> description: 'SPI 0 master'
> >> value: 1
> >> restrictions:
> >> - "!SPI_0_SLAVE"
> >> SPI_0_SLAVE:
> >> description: 'SPI 0 slave'
> >> value: 1
> >> restrictions:
> >> - "!SPI_0_MASTER"
> >> The newt tool generates a nice error message:
> >>
> >> Error: Syscfg restriction violations detected:
> >> SPI_0_MASTER=1 requires SPI_0_SLAVE not be set, but SPI_0_SLAVE=1
> >> SPI_0_SLAVE=1 requires SPI_0_MASTER not be set, but SPI_0_MASTER=1
> >>
> >> Setting history (newest -> oldest):
> >> SPI_0_MASTER: [hw/bsp/nrf52dk:1]
> >> SPI_0_SLAVE: [hw/bsp/nrf52dk:1]
> >
> > I wasn't aware of the restrictions flag, that's nice to know about.
> >
> > Should I submit a pull request to resolve the conflict in master, though
> (for at least the nRF52DK, I haven' t checked the other BSP packages)?
> >
> > I'd propose only enabling SPI_0_MASTER by default, but I'm perhaps
> missing some specific intentions with this as well (having both MASTER and
> SLAVE enabled in the BSP).
> >
> >
>
>
Re: nRF52DK BSP SPI Conflict?
Posted by will sanfilippo <wi...@runtime.io>.
No, no need to do that as I am currently modifying the spi hal so that we dont have license issues with the nordic SDK. I will commit changes to the syscfg.yml files when I do this.
Will
> On Nov 18, 2016, at 4:09 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>
> On 17/11/16 05:15, will sanfilippo wrote:
>> There is an easy way to prevent this as well which should be added to syscfg.yml. The following works:
>>
>> SPI_0_MASTER:
>> description: 'SPI 0 master'
>> value: 1
>> restrictions:
>> - "!SPI_0_SLAVE"
>> SPI_0_SLAVE:
>> description: 'SPI 0 slave'
>> value: 1
>> restrictions:
>> - "!SPI_0_MASTER"
>> The newt tool generates a nice error message:
>>
>> Error: Syscfg restriction violations detected:
>> SPI_0_MASTER=1 requires SPI_0_SLAVE not be set, but SPI_0_SLAVE=1
>> SPI_0_SLAVE=1 requires SPI_0_MASTER not be set, but SPI_0_MASTER=1
>>
>> Setting history (newest -> oldest):
>> SPI_0_MASTER: [hw/bsp/nrf52dk:1]
>> SPI_0_SLAVE: [hw/bsp/nrf52dk:1]
>
> I wasn't aware of the restrictions flag, that's nice to know about.
>
> Should I submit a pull request to resolve the conflict in master, though (for at least the nRF52DK, I haven' t checked the other BSP packages)?
>
> I'd propose only enabling SPI_0_MASTER by default, but I'm perhaps missing some specific intentions with this as well (having both MASTER and SLAVE enabled in the BSP).
>
>
Re: nRF52DK BSP SPI Conflict?
Posted by Kevin Townsend <ke...@adafruit.com>.
On 17/11/16 05:15, will sanfilippo wrote:
> There is an easy way to prevent this as well which should be added to syscfg.yml. The following works:
>
> SPI_0_MASTER:
> description: 'SPI 0 master'
> value: 1
> restrictions:
> - "!SPI_0_SLAVE"
> SPI_0_SLAVE:
> description: 'SPI 0 slave'
> value: 1
> restrictions:
> - "!SPI_0_MASTER"
> The newt tool generates a nice error message:
>
> Error: Syscfg restriction violations detected:
> SPI_0_MASTER=1 requires SPI_0_SLAVE not be set, but SPI_0_SLAVE=1
> SPI_0_SLAVE=1 requires SPI_0_MASTER not be set, but SPI_0_MASTER=1
>
> Setting history (newest -> oldest):
> SPI_0_MASTER: [hw/bsp/nrf52dk:1]
> SPI_0_SLAVE: [hw/bsp/nrf52dk:1]
I wasn't aware of the restrictions flag, that's nice to know about.
Should I submit a pull request to resolve the conflict in master, though
(for at least the nRF52DK, I haven' t checked the other BSP packages)?
I'd propose only enabling SPI_0_MASTER by default, but I'm perhaps
missing some specific intentions with this as well (having both MASTER
and SLAVE enabled in the BSP).
Re: nRF52DK BSP SPI Conflict?
Posted by will sanfilippo <wi...@runtime.io>.
There is an easy way to prevent this as well which should be added to syscfg.yml. The following works:
SPI_0_MASTER:
description: 'SPI 0 master'
value: 1
restrictions:
- "!SPI_0_SLAVE"
SPI_0_SLAVE:
description: 'SPI 0 slave'
value: 1
restrictions:
- "!SPI_0_MASTER"
The newt tool generates a nice error message:
Error: Syscfg restriction violations detected:
SPI_0_MASTER=1 requires SPI_0_SLAVE not be set, but SPI_0_SLAVE=1
SPI_0_SLAVE=1 requires SPI_0_MASTER not be set, but SPI_0_MASTER=1
Setting history (newest -> oldest):
SPI_0_MASTER: [hw/bsp/nrf52dk:1]
SPI_0_SLAVE: [hw/bsp/nrf52dk:1]
> On Nov 16, 2016, at 5:19 PM, Kevin Townsend <ke...@adafruit.com> wrote:
>
> I'm just starting to look at the SPI HAL to implement a test driver, but looking through the hal_spi code I see what might be a conflict.
>
> In the master branch of apache-mynewt-core the nRF52SK BSP has both SPI0 and SPIS0 enabled:
>
> * https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/include/bsp/nrf_drv_config.h#L211
> * https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/include/bsp/nrf_drv_config.h#L252
>
> Won't this lead to a situation where the hal_bsp_init function will always initialize in slave mode, when end users might be expecting master mode if they don't read the code carefully: https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/src/hal_bsp.c#L217
>
> Perhaps these should be exclusive and an #error is raised if both master and slave are enabled on the same bus (which would also apply for I2C)?
>
> The two options don't seem to be possible at the same time since there are separate init functions in the MCU HAL SPI code: https://github.com/apache/incubator-mynewt-core/blob/master/hw/mcu/nordic/nrf52xxx/src/hal_spi.c#L620 ... and the pins will have to be configured in different directions (SCK will be input in one mode and output in the other, etc.).
>
> Kevin
>
Re: nRF52DK BSP SPI Conflict?
Posted by will sanfilippo <wi...@runtime.io>.
Well, I certainly think more could be done here to make sure if someone did that things would just #error out. However, if both SPI_0_SLAVE and SPI_0_MASTER were set to #1, the second init should fail as that resource has already been acquired. Still, probably better to have a #error instead of an assert.
> On Nov 16, 2016, at 5:19 PM, Kevin Townsend <ke...@adafruit.com> wrote:
>
> I'm just starting to look at the SPI HAL to implement a test driver, but looking through the hal_spi code I see what might be a conflict.
>
> In the master branch of apache-mynewt-core the nRF52SK BSP has both SPI0 and SPIS0 enabled:
>
> * https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/include/bsp/nrf_drv_config.h#L211
> * https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/include/bsp/nrf_drv_config.h#L252
>
> Won't this lead to a situation where the hal_bsp_init function will always initialize in slave mode, when end users might be expecting master mode if they don't read the code carefully: https://github.com/apache/incubator-mynewt-core/blob/master/hw/bsp/nrf52dk/src/hal_bsp.c#L217
>
> Perhaps these should be exclusive and an #error is raised if both master and slave are enabled on the same bus (which would also apply for I2C)?
>
> The two options don't seem to be possible at the same time since there are separate init functions in the MCU HAL SPI code: https://github.com/apache/incubator-mynewt-core/blob/master/hw/mcu/nordic/nrf52xxx/src/hal_spi.c#L620 ... and the pins will have to be configured in different directions (SCK will be input in one mode and output in the other, etc.).
>
> Kevin
>