Date   

Re: A possible case of TCP server no-response, was: Re: bufs lost in TCP connection establishment

Flavio Santes <flavio.santes@...>
 

Hello,

Hello Rohit, et al,

Thanks for the information below. I didn't yet have chance to test
multiple TCP/IPv4 connections in row, because I have issue with
reliably handling one. Just as before, my setup is a scripting language
(MicroPython), using combination of Zephyr and uIP APIs to emulate BSD
sockets. I however proceeded to play with mbedTLS on top of sockets
implemented in such way. That means multiple bidirectional packet
exchanges which exposes quite a number of edge conditions. There're
packets can be dropped anytime.

One particularly naughty conditions I experienced is that while my
usual testcase usually proceeded beyond connect() call, sometimes I got
time "strips" when connect() call just hanged (I don't use timeouts,
like BSD sockets by default) - with consecutive dozen or so of QEMU
restarts (I'm using QEMU virtual networking still so far). Then it
suddenly started working again, to repeat soon again. Looking with
Wireshark, Zephyr app just was sending SYN packets, with zero reply
from a Linux side. At this time, it's easy to start wonder what gets
wrong - Linux handling of TUN interface, tunslip's setting it up, socat
which is involved in connecting tunslip and QEMU, Wireshark, or
something else.

I set to debug just that weird case, with initial hypothesis that Linux
SYN flood protection may be involved. With some googling and netstat
grepping, I found that when I get those can't-connect time strips,
there's actually ESTABLISHED connection on the host. That led me to
http://stackoverflow.com/questions/6825036/what-will-happen-if-i-send-a-s...
,
which finished the picture.

So, uIP/Zephyr don't use source port randomization, and always open
connection from port 1025. While code snippets I ran included closing
a socket, if something went wrong, like a packet was lost during
initial connection or TLS exchange, the connection wasn't closed
explicitly, but left open on server. On QEMU restart, it started to
send SYN to establish new connection from the same source port, which,
as explained in the link above, gets zero response from a Linux host.
Eventually Apache on host side times out that client connection, then
the situation recovers.

The solution is thus to check whether there's ESTABLISHED connection on
host side with netstat and restart a server process which owns it
(apache in my case).

The same situation may surely happen with a real device too, so I hope
this mail may save debugging time to some readers.
This is a hardware-agnostic issue, see: https://jira.zephyrproject.org/browse/ZEP-428.
The issue description is not really useful, although comments will help.
BTW, we apply the same workaround: restart the server :)


Thanks,
Paul


On Wed, 7 Sep 2016 15:26:23 +0000
Rohit Grover <Rohit.Grover(a)arm.com&gt; wrote:
Regards,
Flavio


[RFC]PWM API Update

Liu, Baohong
 

Hi everyone,

I would like to propose the re-design of PWM API interfaces.
(https://jira.zephyrproject.org/browse/ZEP-745).

PWM APIs had been through a few times of updates from different aspects.
The following are the known problems.
--------

- Argument "flags" in PWM config API is not used and its meaning is not clear.

- Usage of PWM set phase API is not clear since only zero phase is supported.
The definition of phase is not meaningful for most of the PWM devices.

- PWM set values API seems redundant since PWM set period API and PWM
set duty cycle API together can accomplish the same thing.

Proposed Solution
--------

Period, pulse width are the only parameters needed for PWM.
So, the following API will be enough to set up PWM.

pwm_set_pin(uint8_t pin, uint32_t period, uint32_t pulse_width)

As for the unit for period and pulse width, I understand that both time
(micro-second) and clock cycles are popularly used. To cater for this,
the above-mentioned API will be expanded into two.

pwm_set_pin_usec(uint8_t pin, uint32_t period_usec, uint32_t pulse_usec)
pwm_set_pin_cycles(uint8_t pin, uint32_t period_cycles, uint32_t pulse_cycles)

These two APIs are equivalent. User only needs to use one of them based on
the preference for unit.

Final API Format
--------

pwm_set_pin_usec(struct device *dev, uint8_t pin, uint32_t period_usec,
uint32_t pulse_usec)
pwm_set_pin_cycles(struct device *dev, uint8_t pin, uint32_t period_cycles,
uint32_t pulse_cycles)

Suggestions and comments are welcome.


Re: [RFC] SPI API update

Liu, Baohong
 

-----Original Message-----
From: Tomasz Bursztyka [mailto:tomasz.bursztyka(a)linux.intel.com]
Sent: Wednesday, September 14, 2016 12:31 AM
To: Liu, Baohong <baohong.liu(a)intel.com>; devel(a)lists.zephyrproject.org
Subject: Re: [devel] [RFC] SPI API update

Hi Baohong,

inline int spi_transceive(struct spi_config *conf, uint32_t slave,
struct spi_buffer *tx, uint32_t nb_tx_buf,
struct spi_buffer *rx, uint32_t
nb_rx_buf);

GPIO is and will be used for virtually any cases since the maximum
duration of the CS from the controller is so short. So, slave select
is just giving a non-zero value to make the controller happy and the
GPIO pin is selecting the slave device. But, in the current code, only
one GPIO pin is defined for each controller by Kconfig. In other words, one
controller is only able to control one slave. This needs to be addressed.

This GPIO used as CS is a hack for - currently - DW controllers. When hw
integration makes it available (it's possible on QuarkSE X86 core, not on
QuarkSE ARC core for instance).

So indeed, it's very much limited for now, but I don't see any reason to
expose anything about it in the public API. Hack should stay hidden, deep, in
driver's specific code.
User or app writer needs to be aware of this. User needs the info to connect
the external or on-board devices. This will be part of user-configurable setting.
It can't be hidden.


For dw, we could create some array of gpio pins: index would be the slave. It
could be provided by the relevant board.h file. We could provide "generic"
spi_config_cs()/spi_control_cs() but only for the drivers, like in some
drivers/spi/spi_utils.h file. Just a quick idea.

Tomasz


A possible case of TCP server no-response, was: Re: bufs lost in TCP connection establishment

Paul Sokolovsky
 

Hello Rohit, et al,

Thanks for the information below. I didn't yet have chance to test
multiple TCP/IPv4 connections in row, because I have issue with
reliably handling one. Just as before, my setup is a scripting language
(MicroPython), using combination of Zephyr and uIP APIs to emulate BSD
sockets. I however proceeded to play with mbedTLS on top of sockets
implemented in such way. That means multiple bidirectional packet
exchanges which exposes quite a number of edge conditions. There're
packets can be dropped anytime.

One particularly naughty conditions I experienced is that while my
usual testcase usually proceeded beyond connect() call, sometimes I got
time "strips" when connect() call just hanged (I don't use timeouts,
like BSD sockets by default) - with consecutive dozen or so of QEMU
restarts (I'm using QEMU virtual networking still so far). Then it
suddenly started working again, to repeat soon again. Looking with
Wireshark, Zephyr app just was sending SYN packets, with zero reply
from a Linux side. At this time, it's easy to start wonder what gets
wrong - Linux handling of TUN interface, tunslip's setting it up, socat
which is involved in connecting tunslip and QEMU, Wireshark, or
something else.

I set to debug just that weird case, with initial hypothesis that Linux
SYN flood protection may be involved. With some googling and netstat
grepping, I found that when I get those can't-connect time strips,
there's actually ESTABLISHED connection on the host. That led me to
http://stackoverflow.com/questions/6825036/what-will-happen-if-i-send-a-syn-packet-to-the-server-when-there-has-already-bee ,
which finished the picture.

So, uIP/Zephyr don't use source port randomization, and always open
connection from port 1025. While code snippets I ran included closing
a socket, if something went wrong, like a packet was lost during
initial connection or TLS exchange, the connection wasn't closed
explicitly, but left open on server. On QEMU restart, it started to
send SYN to establish new connection from the same source port, which,
as explained in the link above, gets zero response from a Linux host.
Eventually Apache on host side times out that client connection, then
the situation recovers.

The solution is thus to check whether there's ESTABLISHED connection on
host side with netstat and restart a server process which owns it
(apache in my case).

The same situation may surely happen with a real device too, so I hope
this mail may save debugging time to some readers.


Thanks,
Paul


On Wed, 7 Sep 2016 15:26:23 +0000
Rohit Grover <Rohit.Grover(a)arm.com> wrote:

Jukka, Paul,

I find that uIP leaks one TX buf with every TCP connection due to
incorrectly managed ref-counts. I'm able to setup and teardown the
same number of connections as the value of CONFIG_IP_BUF_TX_SIZE.

The initial buf for the SYN packet gets its ref-count bumped to 2 (by
tcpip_poll_tcp()), but then this count never goes down to 0. It seems
to me that when the tcpip_event is posted to process_thread_tcp()
upon the sending of the SYN buf, the following code fragment

if (buf && uip_connected(buf)) {
struct net_context *context = user_data;
NET_DBG("Connection established context %p\n",
user_data);
context->connection_status = -EALREADY;
data = INT_TO_POINTER(TCP_WRITE_EVENT);
goto try_send;
}

is able is able to discover the transition to connected state, and
cycles back to call handle_tcp_connection(); but the ref-count of SYN
buf isn't decremented.

Can you help?

Thanks,
Rohit.

IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose
the contents to any other person, use it for any purpose, or store or
copy the information in any medium. Thank you.


--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog


Daily JIRA Digest

donotreply@...
 

NEW JIRA items within last 24 hours: 3
[ZEP-865] convert filesystem sample to a runnable test
https://jira.zephyrproject.org/browse/ZEP-865

[ZEP-869] TCP connection fails if communication slows down.
https://jira.zephyrproject.org/browse/ZEP-869

[ZEP-868] TCP Connections are broken on Galileo
https://jira.zephyrproject.org/browse/ZEP-868


UPDATED JIRA items within last 24 hours: 10
[ZEP-540] add APIs for asynchronous transfer callbacks
https://jira.zephyrproject.org/browse/ZEP-540

[ZEP-750] Arduino 101 board should support one configuration using original bootloader
https://jira.zephyrproject.org/browse/ZEP-750

[ZEP-817] Neighbor Discovery Optimization for IPv6 over 6LowPAN
https://jira.zephyrproject.org/browse/ZEP-817

[ZEP-245] Restructure Documentation content
https://jira.zephyrproject.org/browse/ZEP-245

[ZEP-852] SPI API Update
https://jira.zephyrproject.org/browse/ZEP-852

[ZEP-737] Update host tools from upstream: fixdep.c
https://jira.zephyrproject.org/browse/ZEP-737

[ZEP-240] printk/printf usage in samples
https://jira.zephyrproject.org/browse/ZEP-240

[ZEP-584] warn user if SDK is out of date
https://jira.zephyrproject.org/browse/ZEP-584

[ZEP-748] Enable mbedtls_sslclient sample to run on quark se board
https://jira.zephyrproject.org/browse/ZEP-748

[ZEP-637] Building and linking static libraries
https://jira.zephyrproject.org/browse/ZEP-637


CLOSED JIRA items within last 24 hours: 3
[ZEP-863] (Cannot Reproduce) minimal libc 'size_t' conflicting with ISSM toolchain
https://jira.zephyrproject.org/browse/ZEP-863

[ZEP-756] (Duplicate) Remove Sandboxes from documentation
https://jira.zephyrproject.org/browse/ZEP-756

[ZEP-763] (Fixed) Samples:README: samples/drivers/I2c_stts751 describes error
https://jira.zephyrproject.org/browse/ZEP-763


RESOLVED JIRA items within last 24 hours: 1
[ZEP-640] (Fixed) Remove dynamic IRQs/exceptions from Zephyr
https://jira.zephyrproject.org/browse/ZEP-640


Re: [RFC] SPI API update

Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2016-09-14 at 16:29 +0200, Tomasz Bursztyka wrote:
Hi Jon,

On Wed, 2016-09-14 at 13:42 +0200, Tomasz Bursztyka wrote:
Tixy wrote
With my proposal SPI_ENABLE_CS_BEFORE would acquire the lock and
SPI_DISABLE_CS_AFTER release it. All that's different is that one
function call is replaced by N Function calls.
For this to work, you would need to relate the lock to the caller, to
block any other caller
to mess up. It's possible (fiber/task id), but a bit more bloated than
doing all at once in one function.
I was assuming 'lock' == mutex/semaphore and would be the same method
you would use in a single function. E.g.

Single function...
lock
do some work
unlock

Separate function1
lock
do some work
Separate function2
do some work
Separate function3
do some work
unlock

The SPI bus is a shared resource, and users claim use of if for using
it.
As I said your example does not work unless you keep track of who owns
the lock.
What about your lock that you'll use in the RFC's proposed of
spi_tranceive? I think I've must have misunderstood something
fundamental, because I see so difference between

A();
B();
C();

and

void F()
{
A();
B();
C();
}

F();

and the compiler would also agree.

[...]

You _cannot_ know in your code, if another part of the system has
touched the device configuration.
So the code always need to make sure the dev is running the right
configuration.

Either we always ask the user to run spi_configure(), as ugly as it is,
before each spi_ io calls.
Or we ask him to provide a statically allocated configuration structure,
with a content not meant
to change, and give it for each spi_ io calls.

Btw, if you run such code in a task, you can be pre-empted after your
spi_configure(),
then another task/fiber kicks in and also runs spi_configure(): when the
kernel is
back at your task/fiber, you are not anymore running on your
configuration...
And your alternative of a single function call that checks a config arg
and updates the config before then transferring data is different? What
if it gets preempted between updating the config in hardware and
transferring the data? It's my same confusion as to why it should be
that "A() B() C()" != F()"

I'm not trying to press my API suggestions on people, I'll code to
whatever Zephyr specifies. I was carrying on these emails to try and
clear up my own misunderstanding of this as much as anything. I'll stop
now, and review any code that is forthcoming. It's a lot easier to talk
about real code and say things like 'what if we are prempted at this
line here...' to which the answer may well be 'we can't be preempted
here because of X'.

--
Tixy


Daily Gerrit Digest

donotreply@...
 

NEW within last 24 hours:
- https://gerrit.zephyrproject.org/r/4773 : Bluetooth: Controller: Make HCI endianness-independent
- https://gerrit.zephyrproject.org/r/4774 : intel_quark: move X86_IAMCU to defconfig
- https://gerrit.zephyrproject.org/r/4757 : sanity: test_context build only for qemu arm
- https://gerrit.zephyrproject.org/r/4782 : samples: zoap server: exclude quark d2000 not enough ram
- https://gerrit.zephyrproject.org/r/4780 : unified: Fix semaphore group tests
- https://gerrit.zephyrproject.org/r/4781 : unified: Enable semaphore group use in test_mail
- https://gerrit.zephyrproject.org/r/4779 : unified: Add support for semaphore groups
- https://gerrit.zephyrproject.org/r/4778 : unified: Add SEMAPHORE_GROUPS Kconfig option
- https://gerrit.zephyrproject.org/r/4777 : unified: Include _timeout structure in tcs_base
- https://gerrit.zephyrproject.org/r/4776 : unified: Conditionally define __printf_like() macro
- https://gerrit.zephyrproject.org/r/4770 : tinycrypt: Add test case for the CTR PRNG algorithm
- https://gerrit.zephyrproject.org/r/4771 : net: tests: Remove broadcast and multicast tests from ARP
- https://gerrit.zephyrproject.org/r/4768 : known issues: update SKIP regex
- https://gerrit.zephyrproject.org/r/4755 : sdk: zephyr: check for minimum required version of SDK

UPDATED within last 24 hours:
- https://gerrit.zephyrproject.org/r/4687 : arduino 101: make factory bootloader config the default
- https://gerrit.zephyrproject.org/r/4671 : soc: intel_quark: move quark d2000 to intel_quark family
- https://gerrit.zephyrproject.org/r/4670 : intel_quark: Group Quark SoCs under intel_quark/
- https://gerrit.zephyrproject.org/r/4684 : quark_x1000: move the X1000 into the intel_quark family
- https://gerrit.zephyrproject.org/r/4422 : boards: rename Quark SE Devboard to Quark SE C1000
- https://gerrit.zephyrproject.org/r/4420 : boards: rename Quark SE Devboard to Quark SE C1000 (Sensor Subsystem)
- https://gerrit.zephyrproject.org/r/4423 : MAINTAINERS: add maintainer for some of the boards
- https://gerrit.zephyrproject.org/r/4689 : parse board defconfig at the very end
- https://gerrit.zephyrproject.org/r/4688 : boards: rename arduino_101_sss to arduino_101_ss
- https://gerrit.zephyrproject.org/r/4715 : net: yaip: Add DHCPv4 client support
- https://gerrit.zephyrproject.org/r/4623 : eth: Adjust ENC28J60 transmission/reception return codes.
- https://gerrit.zephyrproject.org/r/4717 : net: apps: Add DHCPv4 client sample application
- https://gerrit.zephyrproject.org/r/4716 : net: tests: Add DHCPv4 client unit tests
- https://gerrit.zephyrproject.org/r/4585 : samples: use printf/printk directly instead of macros
- https://gerrit.zephyrproject.org/r/4551 : tests: move test code from samples to tests
- https://gerrit.zephyrproject.org/r/3922 : lib: Add HTTP support for Zephyr
- https://gerrit.zephyrproject.org/r/3924 : lib/http: Add test-case for HTTP header fields
- https://gerrit.zephyrproject.org/r/4711 : net: tests: Add include dir only when specific options enabled
- https://gerrit.zephyrproject.org/r/4713 : net: yaip: Fix net address state
- https://gerrit.zephyrproject.org/r/4745 : tinycrypt: Rename current tests to avoid confusions with new algorithms
- https://gerrit.zephyrproject.org/r/4747 : tinycrypt: Solve style issues
- https://gerrit.zephyrproject.org/r/4555 : Bluetooth: HFP HF: SLC connection-Send/Parse BRSF
- https://gerrit.zephyrproject.org/r/4489 : Bluetooth: SDP: Server: Support ServiceAttrReq and ServiceSearchAttrReq
- https://gerrit.zephyrproject.org/r/4488 : Bluetooth: SDP: Server: Support ServiceSearchRequest
- https://gerrit.zephyrproject.org/r/4486 : Bluetooth: SDP: Server: Initialize and accept incoming connections
- https://gerrit.zephyrproject.org/r/4487 : Bluetooth: SDP: Server: Support service record registration
- https://gerrit.zephyrproject.org/r/4708 : net: yaip: Fix arp/ethernet broadcast and multcast addr scenario
- https://gerrit.zephyrproject.org/r/4707 : net: yaip: Fix slip multipackets reception
- https://gerrit.zephyrproject.org/r/4452 : Bluetooth: A2DP: Initialization of A2DP.
- https://gerrit.zephyrproject.org/r/4447 : Bluetooth: AVDTP: Module Initialization
- https://gerrit.zephyrproject.org/r/4540 : Bluetooth: AVDTP: Connect and Disconnect
- https://gerrit.zephyrproject.org/r/4710 : net: yaip: Fix IPv4 packet reception
- https://gerrit.zephyrproject.org/r/4562 : Bluetooth: Sample: handsfree sample application
- https://gerrit.zephyrproject.org/r/4321 : Bluetooth: L2CAP: Cleanup hanging channel when initiator
- https://gerrit.zephyrproject.org/r/4750 : Bluetooth: HCI: Fix updating RPA too early
- https://gerrit.zephyrproject.org/r/4749 : nano_work: Add nano_work_pending
- https://gerrit.zephyrproject.org/r/4699 : nano_work: Add nano_delayed_work_pending
- https://gerrit.zephyrproject.org/r/4600 : samples: heartrate-monitor: Fixes for following issues.
- https://gerrit.zephyrproject.org/r/4625 : eth: ENC28J60 sample application
- https://gerrit.zephyrproject.org/r/4683 : ARM: irq: Add _arch_irq_is_priority_equal external interrupt API
- https://gerrit.zephyrproject.org/r/4676 : irq: Add irq_enable_keep external interrupt API
- https://gerrit.zephyrproject.org/r/3895 : tests/kernel/test_multilib: Test for proper multilib selection.
- https://gerrit.zephyrproject.org/r/4704 : power_mgmt: Update sample and drivers according to new pm device API
- https://gerrit.zephyrproject.org/r/4463 : power_mgmt: Update Power Management device driver API
- https://gerrit.zephyrproject.org/r/4552 : build: support pre-built host tools (DO NOT MERGE)
- https://gerrit.zephyrproject.org/r/3363 : Commit: validate accent in name.
- https://gerrit.zephyrproject.org/r/4622 : eth: Add full-duplex configuration to ENC28J60
- https://gerrit.zephyrproject.org/r/4624 : eth: Add ENC28J60 support into Paho's MQTT publisher example
- https://gerrit.zephyrproject.org/r/4511 : unified/doc: Kernel primer for unified kernel
- https://gerrit.zephyrproject.org/r/4621 : eth: Initial release to tx semaphore for the ENC28J60 driver.
- https://gerrit.zephyrproject.org/r/4620 : eth: Adjust ENC28J60 MAC configuration.
- https://gerrit.zephyrproject.org/r/4681 : ARM: irq: Add _arch_irq_pending_clear external interrupt API
- https://gerrit.zephyrproject.org/r/4680 : irq: Add irq_pending_clear external interrupt API
- https://gerrit.zephyrproject.org/r/4679 : ARM: irq: Add _arch_irq_is_enabled external interrupt API
- https://gerrit.zephyrproject.org/r/4678 : irq: Add irq_is_enabled external interrupt API
- https://gerrit.zephyrproject.org/r/4662 : irq: Add irq_pending_set external interrupt API
- https://gerrit.zephyrproject.org/r/4635 : serial: make nrf5 driver more generic, prepare for nrf51

MERGED within last 24 hours:
- https://gerrit.zephyrproject.org/r/4775 : gpio: stm32: Fix bug introduced by removing API 1.0 support
- https://gerrit.zephyrproject.org/r/4756 : known-issues: fix regex to catch summary messages to ignore
- https://gerrit.zephyrproject.org/r/4769 : samples/task_profile: fix testcase.ini's long lines
- https://gerrit.zephyrproject.org/r/4772 : Bluetooth: SMP: Factor out BR/EDR encryption check to helper
- https://gerrit.zephyrproject.org/r/4754 : Bluetooth: SMP: Fix encryption key size check in BR/EDR pairing req
- https://gerrit.zephyrproject.org/r/4753 : Bluetooth: Add support for reading encryption key size for BR/EDR
- https://gerrit.zephyrproject.org/r/4360 : ksdk: Add KSDK RNGA driver.
- https://gerrit.zephyrproject.org/r/4697 : gpio: Remove obsolete API 1.0 callback mechanism
- https://gerrit.zephyrproject.org/r/4729 : Bluetooth: SMP: Add helper for reporting BR/EDR pairing complete
- https://gerrit.zephyrproject.org/r/4728 : Bluetooth: SMP: Add support for Signing Information over BR/EDR
- https://gerrit.zephyrproject.org/r/4727 : Bluetooth: SMP: Add support for Identity Information over BR/EDR
- https://gerrit.zephyrproject.org/r/4730 : Bluetooth: SMP: Add support for sending Pairing Request over BR/EDR
- https://gerrit.zephyrproject.org/r/4726 : Bluetooth: SMP: Add support for LTK derivation from LinkKey
- https://gerrit.zephyrproject.org/r/4725 : Bluetooth: SMP: Allow to force BR/EDR without SC support
- https://gerrit.zephyrproject.org/r/4724 : Bluetooth: SMP: Support Pairing Response over BR/EDR
- https://gerrit.zephyrproject.org/r/4752 : Bluetooth: Controller: Implement LE_RAND command
- https://gerrit.zephyrproject.org/r/4516 : workqueue: use kernel.h for workqueue header file
- https://gerrit.zephyrproject.org/r/4674 : x86: load _nanokernel in %edi in _Swap()
- https://gerrit.zephyrproject.org/r/4515 : atomic: fix bug in ATOMIC_INIT()
- https://gerrit.zephyrproject.org/r/4521 : zperf_shell: add unified kernel string for unified kernel case
- https://gerrit.zephyrproject.org/r/4530 : unified: initial unified kernel implementation
- https://gerrit.zephyrproject.org/r/4513 : unified/arm: add unified kernel support for ARM arch
- https://gerrit.zephyrproject.org/r/4508 : build: only generate the SSE group for x86
- https://gerrit.zephyrproject.org/r/4525 : unified/test_pipe: adapt to not use sem groups
- https://gerrit.zephyrproject.org/r/4510 : arm: only compile gdb stubs when CONFIG_GDB_INFO=y
- https://gerrit.zephyrproject.org/r/4528 : unified/test_sema: fix isr wrapper names
- https://gerrit.zephyrproject.org/r/4526 : unified/test_timer: adapt for unified kernel
- https://gerrit.zephyrproject.org/r/4514 : unified/x86: add unified kernel support for x86 arch
- https://gerrit.zephyrproject.org/r/4506 : build: make sysgen take optional command line arguments
- https://gerrit.zephyrproject.org/r/4675 : unified/test_mutex: adapt to run on unified kernel
- https://gerrit.zephyrproject.org/r/4522 : unified/tests: tag working some tests kernel as 'unified_capable'
- https://gerrit.zephyrproject.org/r/4529 : unified/test_fp: mark test so that it runs the nanokernel version
- https://gerrit.zephyrproject.org/r/4527 : unified: Fix test_sema/microkernel
- https://gerrit.zephyrproject.org/r/4571 : checkpatch: do not check for min_t/max_t
- https://gerrit.zephyrproject.org/r/4503 : slist: add sys_slist_get() to fetch and remove the head
- https://gerrit.zephyrproject.org/r/4509 : arm: add __ASSERT() for stack alignment
- https://gerrit.zephyrproject.org/r/4523 : unified/test_context: adapt test to run on unified kernel
- https://gerrit.zephyrproject.org/r/4531 : unified/build: allow building the unified kernel
- https://gerrit.zephyrproject.org/r/4505 : kernel: add CONFIG_MDEF
- https://gerrit.zephyrproject.org/r/4512 : unified/build: adapt Kbuild for unified kernel
- https://gerrit.zephyrproject.org/r/4519 : unified/sys_timer: guard microkernel announce with !KERNEL_V2
- https://gerrit.zephyrproject.org/r/4504 : slist: add sys_slist_append_list and sys_slist_merge_slist()
- https://gerrit.zephyrproject.org/r/4507 : sysgen: add --kernel_type argument
- https://gerrit.zephyrproject.org/r/4524 : unified/test_mail: adapt test to not use sem groups and mem pools
- https://gerrit.zephyrproject.org/r/4518 : unified/drivers: adapt timer drivers to unified kernel
- https://gerrit.zephyrproject.org/r/4502 : dlist: add static initialization macro
- https://gerrit.zephyrproject.org/r/4520 : unified/object_tracing: disable object tracing in unified kernel
- https://gerrit.zephyrproject.org/r/4570 : checkpatch: add --ignore DATE_TIME
- https://gerrit.zephyrproject.org/r/4517 : unified: include kernel.h via major top-level header files
- https://gerrit.zephyrproject.org/r/4501 : dlist: add SYS_DLIST_FOR_EACH_NODE/_SAFE
- https://gerrit.zephyrproject.org/r/4741 : doc: workaround for __deprecated functions
- https://gerrit.zephyrproject.org/r/4593 : Clean dev and latest docs sites before deploy
- https://gerrit.zephyrproject.org/r/4659 : task profiler: Adds the task profiler samples to the sanity check
- https://gerrit.zephyrproject.org/r/4657 : task profiler: project configuration files clean up
- https://gerrit.zephyrproject.org/r/4658 : task profiler: README file update
- https://gerrit.zephyrproject.org/r/4751 : Bluetooth: Controller: Clean up HCI macros
- https://gerrit.zephyrproject.org/r/4748 : Bluetooth: Controller: Use hci.h for ACL data
- https://gerrit.zephyrproject.org/r/4746 : Bluetooth: Fix giving back pkts semaphore when disconnecting
- https://gerrit.zephyrproject.org/r/4744 : libc: replace null.h and size_t.h with stddef.h


Re: [RFC] SPI API update

Tomasz Bursztyka
 

Hi Jon,

On Wed, 2016-09-14 at 13:42 +0200, Tomasz Bursztyka wrote:
Tixy wrote
With my proposal SPI_ENABLE_CS_BEFORE would acquire the lock and
SPI_DISABLE_CS_AFTER release it. All that's different is that one
function call is replaced by N Function calls.
For this to work, you would need to relate the lock to the caller, to
block any other caller
to mess up. It's possible (fiber/task id), but a bit more bloated than
doing all at once in one function.
I was assuming 'lock' == mutex/semaphore and would be the same method
you would use in a single function. E.g.

Single function...
lock
do some work
unlock

Separate function1
lock
do some work
Separate function2
do some work
Separate function3
do some work
unlock

The SPI bus is a shared resource, and users claim use of if for using
it.
As I said your example does not work unless you keep track of who owns
the lock.

And anyway, it looks ugly to handle low level SPI concept (CS) in user API.

If we wanted, we could also crowbar the slave select value into those
flags. Then if we created a config function that operated per slave:

spi_configure(dev, config, slave_id);

and that saved the config in an array somewhere, then the driver or
framework could reconfigure the hardware if the slave changes.
Same problem with the existing API: you will need to call
spi_configure() before
every spi transaction call, because you won't know if another part of
the system did access
or not the same device, and changed the config/slave etc...
The SPI driver can remember the last-slave-used and check if it's
changed for the new transaction and if so, do a re-configure. That's
what you proposed doing and would work here too.

And if the user called spi_configure to change the config used for a
slave, and that was the current last-slave-used then the last-slave-used
value could be invalidated to force a re-configure before next
transaction.

All that would go wrong is if two parts of the system were tring to
access the same slave with different configs. But in that case, they are
likely to be interfering with each other in more ways that just changing
the config. The slave device itself is likely to have state that would
get trashed as well.
The driver will remember the config and the slave anyway (in same
struct, or separate).

I really don't see why we should keep spi_configure() around, that's all.
It lets the driver know the config has changed.

If you pass a pointer to a struct containing the config, then do you
assume same pointer == same config or copy the config data and do a
memcmp to see if it was changed between calls? If you just compare
pointers, then anyone wanting to change config has to use another struct
at a different address, and know this implementation detail of the API.
We'd also get some weird bugs if anyone used a config struct allocated
on the stack, where the address may or may not match the current stashed
pointer depending on program flow and compilation options. So a config
may or may not actually be used.
You _cannot_ know in your code, if another part of the system has
touched the device configuration.
So the code always need to make sure the dev is running the right
configuration.

Either we always ask the user to run spi_configure(), as ugly as it is,
before each spi_ io calls.
Or we ask him to provide a statically allocated configuration structure,
with a content not meant
to change, and give it for each spi_ io calls.

Btw, if you run such code in a task, you can be pre-empted after your
spi_configure(),
then another task/fiber kicks in and also runs spi_configure(): when the
kernel is
back at your task/fiber, you are not anymore running on your
configuration...

Second option is much simpler and safer, spi_config can be seen as a
"handle" in a way.
We can document things to make sure the user does not mess up too much
(not allocation of the struct on stack, etc...). It's not super perfect,
but we don't have the luxury
of being able to use as much memory we want to, in order to properly
keep track of config/owners
etc and transparently switch to one or the other when relevant.

And no, it has nothing to do with personal preference.

Tomasz


Re: [RFC] SPI API update

Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2016-09-14 at 13:42 +0200, Tomasz Bursztyka wrote:
Tixy wrote
With my proposal SPI_ENABLE_CS_BEFORE would acquire the lock and
SPI_DISABLE_CS_AFTER release it. All that's different is that one
function call is replaced by N Function calls.
For this to work, you would need to relate the lock to the caller, to
block any other caller
to mess up. It's possible (fiber/task id), but a bit more bloated than
doing all at once in one function.
I was assuming 'lock' == mutex/semaphore and would be the same method
you would use in a single function. E.g.

Single function...
lock
do some work
unlock

Separate function1
lock
do some work
Separate function2
do some work
Separate function3
do some work
unlock

The SPI bus is a shared resource, and users claim use of if for using
it.

And it still lets the possibility for a code to introduce a bug by never
releasing the lock anyway.
There's endless ways for code to be buggy.

If we wanted, we could also crowbar the slave select value into those
flags. Then if we created a config function that operated per slave:

spi_configure(dev, config, slave_id);

and that saved the config in an array somewhere, then the driver or
framework could reconfigure the hardware if the slave changes.
Same problem with the existing API: you will need to call
spi_configure() before
every spi transaction call, because you won't know if another part of
the system did access
or not the same device, and changed the config/slave etc...
The SPI driver can remember the last-slave-used and check if it's
changed for the new transaction and if so, do a re-configure. That's
what you proposed doing and would work here too.

And if the user called spi_configure to change the config used for a
slave, and that was the current last-slave-used then the last-slave-used
value could be invalidated to force a re-configure before next
transaction.

All that would go wrong is if two parts of the system were tring to
access the same slave with different configs. But in that case, they are
likely to be interfering with each other in more ways that just changing
the config. The slave device itself is likely to have state that would
get trashed as well.
The driver will remember the config and the slave anyway (in same
struct, or separate).

I really don't see why we should keep spi_configure() around, that's all.
It lets the driver know the config has changed.

If you pass a pointer to a struct containing the config, then do you
assume same pointer == same config or copy the config data and do a
memcmp to see if it was changed between calls? If you just compare
pointers, then anyone wanting to change config has to use another struct
at a different address, and know this implementation detail of the API.
We'd also get some weird bugs if anyone used a config struct allocated
on the stack, where the address may or may not match the current stashed
pointer depending on program flow and compilation options. So a config
may or may not actually be used.

I guess it boils down to a personal preference for functions with simple
explicit actions that you can use as building bricks to achieve the end
you want, rather than more monolithic functions that mandate a
particular way of working, even if it doesn't fit your use case.

Perhaps I should create a Jira to rewrite Zephyr in Forth :-)

--
Tixy


Re: [RFC] SPI API update

Tomasz Bursztyka
 

Hi Jon,
On Wed, 2016-09-14 at 11:36 +0200, Tomasz Bursztyka wrote:
Hi Jon,

1. What about DMA integration? I haven't spotted much in the way of DMA
support in driver APIs and the DMA API itself doesn't support scatter
gather so that's looking like a whole area that's yet to be sorted out?
DMA is not meant to support such scatter-gather buffer, but it would be
easy to wrap dma primitives to do so.

(However, I don't know why it separates transfer cb and error cb, these
could be the same,
it would just require a status code in cb parameter. That's off subject
however).

That said, I don't see any requirement for exposing anything about DMA
in public SPI API.
I'm was just wondering how to tell a generic SPI driver, which DMA to
use. Or how it requests one itself. Or if these things we're going to
have to be hard coded at build time in a device specific manner, which
would negate writing a generic driver for some given IP.
I guess we would have to think about something driver generic,
configured through board.h also.
There isn't much other way. Or it can be done through Kconfig as well,
though that could end bloated.

But I haven't really looked into DMA properly yet, so I'm all fuzzy on
the possibilities and requirements :-)
It's a good thing you raised this DMA issue, because DMA API itself
would need some work it seems.

I am bit skeptical on some part, like the handshake_interface, this
really looks like something qmsi centric,
and that's a bit bad because Zephyr device driver APIs should not be
tight to any HAL, it should be the contrary: HAL
should do what's necessary to fit into Zephyr's API. Unless I
misunderstood the point of that attribute.

Anyway, I'll probably send another mail about DMA later.

Tomasz


Re: [RFC] SPI API update

Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2016-09-14 at 13:12 +0200, Tomasz Bursztyka wrote:
I call it a hack in DW driver, because this controller has a CS...
which
is broken.

So, rather than upstreaming this driver along with other board
support, which is what my brief is, I'll just park this work in a
fork
somewhere until Zephyr's APIs settle down to something more
usable. :-(

Legacy or new API: your driver will have to deal with CS internally,
either controlling the controller's capability or
a GPIO line. That has nothing to do with zero-copy btw.
Except as I want zero copy now, with current APIs, I have to handle CS
manually, outside the SPI driver; that's what I meant. I didn't want to
write code to handle GPIO chip select in the driver which I would then
have to '#if 0' out again :-)

--
Tixy


Re: [RFC] SPI API update

Tomasz Bursztyka
 

Hi Jon,

Orthogonal to my comment above. How about separating out config into a
separate 'begin' function call. If that also enables chip select, then
we can build arbitrary complex transactions (with zero-copy) using the
current API's

spi_begin(dev, conf, slave) /* Enables chip select for slave */
...
spi_write(dev, ...)
spi_tranceive(dev, ...)
spi_read(dev, ...)
...
spi_end(dev) /* Disables chip select */
See my comment below, this idea is affected by the same flaw.

An alternative to that for zero copy support would be to add a 'flags'
parameter to spi_{read,write,transceive} with 2 flags bits:

SPI_ENABLE_CS_BEFORE
SPI_DISABLE_CS_AFTER

That would gain the arbitrary complex and zero copy transaction support,
without forcing a begin/end call. E.g. I could send my bitmap to the LCD
controller without copying it by

spi_write(dev, &id_byte, sizeof(id_byte), SPI_ENABLE_CS_BEFORE);
spi_write(dev, &bitmap, sizeof(bitmap), SPI_DISABLE_CS_AFTER);

And the current implementation of spi_write would be equivalent to

spi_write(dev, data, size, SPI_ENABLE_CS_BEFORE|SPI_DISABLE_CS_AFTER)
This would open quite a nasty can of worms for concurrent access.

One fiber/task won't know the CS status, and might ask to disable it,
while the original
fiber/task enabling CS will still think it's enabled when it's not, etc...
I don't see that concurrent access is any more of a problem with this
than without it. What if a task calls the single transceive() function
you propose, then that task gets preempted and another one calls
tranceive() for a different slave? The current implementation don't have
any locking to prevent things going badly wrong.

With my proposal SPI_ENABLE_CS_BEFORE would acquire the lock and
SPI_DISABLE_CS_AFTER release it. All that's different is that one
function call is replaced by N Function calls.
For this to work, you would need to relate the lock to the caller, to
block any other caller
to mess up. It's possible (fiber/task id), but a bit more bloated than
doing all at once in one function.

And it still lets the possibility for a code to introduce a bug by never
releasing the lock anyway.

If we wanted, we could also crowbar the slave select value into those
flags. Then if we created a config function that operated per slave:

spi_configure(dev, config, slave_id);

and that saved the config in an array somewhere, then the driver or
framework could reconfigure the hardware if the slave changes.
Same problem with the existing API: you will need to call
spi_configure() before
every spi transaction call, because you won't know if another part of
the system did access
or not the same device, and changed the config/slave etc...
The SPI driver can remember the last-slave-used and check if it's
changed for the new transaction and if so, do a re-configure. That's
what you proposed doing and would work here too.

And if the user called spi_configure to change the config used for a
slave, and that was the current last-slave-used then the last-slave-used
value could be invalidated to force a re-configure before next
transaction.

All that would go wrong is if two parts of the system were tring to
access the same slave with different configs. But in that case, they are
likely to be interfering with each other in more ways that just changing
the config. The slave device itself is likely to have state that would
get trashed as well.
The driver will remember the config and the slave anyway (in same
struct, or separate).

I really don't see why we should keep spi_configure() around, that's all.

Tomasz


Re: [RFC] SPI API update

Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2016-09-14 at 11:36 +0200, Tomasz Bursztyka wrote:
Hi Jon,

1. What about DMA integration? I haven't spotted much in the way of DMA
support in driver APIs and the DMA API itself doesn't support scatter
gather so that's looking like a whole area that's yet to be sorted out?
DMA is not meant to support such scatter-gather buffer, but it would be
easy to wrap dma primitives to do so.

(However, I don't know why it separates transfer cb and error cb, these
could be the same,
it would just require a status code in cb parameter. That's off subject
however).

That said, I don't see any requirement for exposing anything about DMA
in public SPI API.
I'm was just wondering how to tell a generic SPI driver, which DMA to
use. Or how it requests one itself. Or if these things we're going to
have to be hard coded at build time in a device specific manner, which
would negate writing a generic driver for some given IP.

But I haven't really looked into DMA properly yet, so I'm all fuzzy on
the possibilities and requirements :-)

--
Tixy


Re: [RFC] SPI API update

Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2016-09-14 at 10:16 +0200, Tomasz Bursztyka wrote:
On 13/09/2016 17:18, Jon Medhurst (Tixy) wrote:
[...]

Orthogonal to my comment above. How about separating out config into a
separate 'begin' function call. If that also enables chip select, then
we can build arbitrary complex transactions (with zero-copy) using the
current API's

spi_begin(dev, conf, slave) /* Enables chip select for slave */
...
spi_write(dev, ...)
spi_tranceive(dev, ...)
spi_read(dev, ...)
...
spi_end(dev) /* Disables chip select */
See my comment below, this idea is affected by the same flaw.

An alternative to that for zero copy support would be to add a 'flags'
parameter to spi_{read,write,transceive} with 2 flags bits:

SPI_ENABLE_CS_BEFORE
SPI_DISABLE_CS_AFTER

That would gain the arbitrary complex and zero copy transaction support,
without forcing a begin/end call. E.g. I could send my bitmap to the LCD
controller without copying it by

spi_write(dev, &id_byte, sizeof(id_byte), SPI_ENABLE_CS_BEFORE);
spi_write(dev, &bitmap, sizeof(bitmap), SPI_DISABLE_CS_AFTER);

And the current implementation of spi_write would be equivalent to

spi_write(dev, data, size, SPI_ENABLE_CS_BEFORE|SPI_DISABLE_CS_AFTER)
This would open quite a nasty can of worms for concurrent access.

One fiber/task won't know the CS status, and might ask to disable it,
while the original
fiber/task enabling CS will still think it's enabled when it's not, etc...
I don't see that concurrent access is any more of a problem with this
than without it. What if a task calls the single transceive() function
you propose, then that task gets preempted and another one calls
tranceive() for a different slave? The current implementation don't have
any locking to prevent things going badly wrong.

With my proposal SPI_ENABLE_CS_BEFORE would acquire the lock and
SPI_DISABLE_CS_AFTER release it. All that's different is that one
function call is replaced by N Function calls.



If we wanted, we could also crowbar the slave select value into those
flags. Then if we created a config function that operated per slave:

spi_configure(dev, config, slave_id);

and that saved the config in an array somewhere, then the driver or
framework could reconfigure the hardware if the slave changes.
Same problem with the existing API: you will need to call
spi_configure() before
every spi transaction call, because you won't know if another part of
the system did access
or not the same device, and changed the config/slave etc...
The SPI driver can remember the last-slave-used and check if it's
changed for the new transaction and if so, do a re-configure. That's
what you proposed doing and would work here too.

And if the user called spi_configure to change the config used for a
slave, and that was the current last-slave-used then the last-slave-used
value could be invalidated to force a re-configure before next
transaction.

All that would go wrong is if two parts of the system were tring to
access the same slave with different configs. But in that case, they are
likely to be interfering with each other in more ways that just changing
the config. The slave device itself is likely to have state that would
get trashed as well.

--
Tixy


Re: [RFC] SPI API update

Tomasz Bursztyka
 

Hi Jon,

On Wed, 2016-09-14 at 09:31 +0200, Tomasz Bursztyka wrote:
This GPIO used as CS is a hack for - currently - DW controllers. When
hw
integration makes it
available (it's possible on QuarkSE X86 core, not on QuarkSE ARC core
for instance).

So indeed, it's very much limited for now, but I don't see any reason
to
expose anything about it
in the public API. Hack should stay hidden, deep, in driver's specific
code.
Not sure why you call this a 'hack'. Anyway, the serial device IP I'm
writing an SPI driver for (ARM's PL022) doesn't have a concept of a chip
select, so I'm going to have to implement this same 'hack'. Except I
won't, because making the current SPI APIs gate CS on every call to
tranceive makes them silly to use (the zero copy problem as mentioned in
the RFC).
I call it a hack in DW driver, because this controller has a CS... which
is broken.

So, rather than upstreaming this driver along with other board
support, which is what my brief is, I'll just park this work in a fork
somewhere until Zephyr's APIs settle down to something more usable. :-(
Legacy or new API: your driver will have to deal with CS internally,
either controlling the controller's capability or
a GPIO line. That has nothing to do with zero-copy btw.

As I proposed, we could make such "GPIO as CS" generic into an
spi_utils.h into drivers/spi/
It's really a driver centric piece, nothing that needs to be public. Up
to the boards.h file to provide the proper mapping.

We will have to sort this out anyway

Tomasz


Re: [RFC] Add new SYS_LOG_BACKEND_FN to sys_log

Nashif, Anas
 

Eitan,,

In principle I agree we need to support additional backends, but I do not think this fits in how it is being done in sys_log.h.
I propose to extend syslog to support custom backends without having to change the header, i.e. using hooks for example.


On 8 Sep 2016, at 08:52, Perry, Eitan <eitan.perry(a)intel.com> wrote:

Add new SYS_LOG_BACKEND_FN to sys_log.h
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
========
Overview
========
Current system log implementation:
• When using system log API (SYS_LOG_ERR(…), SYS_LOG_WRN(…),
SYS_LOG_INF(…), SYS_LOG_DBG(…) ), each of the four SYS_LOG_X macros is
calling the same macro - SYS_LOG_BACKEND_FN that should output the log
message.
• SYS_LOG_BACKEND_FN is mapped to printk or printf according to kconfig.

=====
Goals
=====
• Have the ability to use various back ends to output the messages,
i.e. different UART than the console, SPI, flash, FS, etc..
• The users should use the log in asynchronous way (i.e. not wait to IO
drivers).

==========
Suggestion
==========
• Add the option to map SYS_LOG_BACKEND_FN to a new function, that will
Put the log message in a cyclic_buffer, depends on kconfig.
• Keep the same system log api.
• The log messages will be written to a cyclic buffer (instead of
printk / printf).
• Application developers may add various back ends, that will read the
messages from the cyclic buffer and output the data to the required
driver.

======================
Code change suggestion
======================
At: /include/misc/sys_log.h

/* decide print func */
#if defined(CONFIG_LOGGER_CYCLIC_BUF)
#include <bsp_log_cyclic_buffer.h>
#define SYS_LOG_BACKEND_FN log_cyclic_buf_put
#elif defined(CONFIG_STDOUT_CONSOLE)
#include <stdio.h>
#define SYS_LOG_BACKEND_FN printf
#else
#include <misc/printk.h>
#define SYS_LOG_BACKEND_FN printk
#endif /* CONFIG_STDOUT_CONSOLE */

I do not think the above is the right approach, instead, define a hook mechanism and an interface to enable the desired backend.


Anas


Re: [RFC] SPI API update

Jon Medhurst (Tixy) <tixy@...>
 

On Wed, 2016-09-14 at 09:31 +0200, Tomasz Bursztyka wrote:
This GPIO used as CS is a hack for - currently - DW controllers. When
hw
integration makes it
available (it's possible on QuarkSE X86 core, not on QuarkSE ARC core
for instance).

So indeed, it's very much limited for now, but I don't see any reason
to
expose anything about it
in the public API. Hack should stay hidden, deep, in driver's specific
code.
Not sure why you call this a 'hack'. Anyway, the serial device IP I'm
writing an SPI driver for (ARM's PL022) doesn't have a concept of a chip
select, so I'm going to have to implement this same 'hack'. Except I
won't, because making the current SPI APIs gate CS on every call to
tranceive makes them silly to use (the zero copy problem as mentioned in
the RFC). So, rather than upstreaming this driver along with other board
support, which is what my brief is, I'll just park this work in a fork
somewhere until Zephyr's APIs settle down to something more usable. :-(

--
Tixy


Re: [RFC] SPI API update

Tomasz Bursztyka
 

Hi Jon,

1. What about DMA integration? I haven't spotted much in the way of DMA
support in driver APIs and the DMA API itself doesn't support scatter
gather so that's looking like a whole area that's yet to be sorted out?
DMA is not meant to support such scatter-gather buffer, but it would be
easy to wrap dma primitives to do so.
(However, I don't know why it separates transfer cb and error cb, these
could be the same,
it would just require a status code in cb parameter. That's off subject
however).

That said, I don't see any requirement for exposing anything about DMA
in public SPI API.

Tomasz


Re: [RFC] SPI API update

Tomasz Bursztyka
 

On 13/09/2016 17:18, Jon Medhurst (Tixy) wrote:
On Tue, 2016-09-13 at 13:19 +0200, Tomasz Bursztyka wrote:
Hello everyone,

I would like to introduce a major update on SPI API
That's good timing I was about to ask about the flaws in the current
API, specifically that it seems to force everything into a single
transaction on a single buffer, meaning copying of large data buffers
just to prepend a tiny (in my case one byte) header.

[...]
Solutions
---------

1) The SPI configuration pointer should be provided to every API
calls. For instance:

int spi_write(struct device *dev, struct spi_config *conf,
const void *buf, uint32_t len)

From driver's implementation point of view, it will be then just a
matter of storing a struct spi_config pointer in its private data
holder, and then at every call:
if (conf != private->conf) {
... call configure ...

private->conf = conf;
}

Public function spi_configure() would thus disappear.

2) Instead of calling the slave every-time, we could give it as a
parameter to every call as well. The logic behind would be more or
less the same as for the configuration.

int spi_write(struct device *dev, struct spi_config *conf,
uint32_t slave, const void *buf, uint32_t len)

Public function spi_slave_select() would thus disappear as well.
What about the case (like I have) where there is only a single device
attached to an SPI bus and the config never needs to change once set?
For this there is no need for conf and slave parameters on every
transaction and it's an unnecessary overhead to supply those parameters
and for them to be checked.
From memory/complexity point of view, is it better to provide a
spi_config for each slave
device (thus the slave id into it), or is it better to have one
possibility to share the same spi_config?

From user perspective, the latter "might" be interesting but: in slave
drivers like cc2520 or enc28j60,
which do not share their spi_config to external world, this possibility
is useless.

Maybe we can gather all in spi_config. I am open on that.


Orthogonal to my comment above. How about separating out config into a
separate 'begin' function call. If that also enables chip select, then
we can build arbitrary complex transactions (with zero-copy) using the
current API's

spi_begin(dev, conf, slave) /* Enables chip select for slave */
...
spi_write(dev, ...)
spi_tranceive(dev, ...)
spi_read(dev, ...)
...
spi_end(dev) /* Disables chip select */
See my comment below, this idea is affected by the same flaw.

An alternative to that for zero copy support would be to add a 'flags'
parameter to spi_{read,write,transceive} with 2 flags bits:

SPI_ENABLE_CS_BEFORE
SPI_DISABLE_CS_AFTER

That would gain the arbitrary complex and zero copy transaction support,
without forcing a begin/end call. E.g. I could send my bitmap to the LCD
controller without copying it by

spi_write(dev, &id_byte, sizeof(id_byte), SPI_ENABLE_CS_BEFORE);
spi_write(dev, &bitmap, sizeof(bitmap), SPI_DISABLE_CS_AFTER);

And the current implementation of spi_write would be equivalent to

spi_write(dev, data, size, SPI_ENABLE_CS_BEFORE|SPI_DISABLE_CS_AFTER)
This would open quite a nasty can of worms for concurrent access.

One fiber/task won't know the CS status, and might ask to disable it,
while the original
fiber/task enabling CS will still think it's enabled when it's not, etc...


If we wanted, we could also crowbar the slave select value into those
flags. Then if we created a config function that operated per slave:

spi_configure(dev, config, slave_id);

and that saved the config in an array somewhere, then the driver or
framework could reconfigure the hardware if the slave changes.
Same problem with the existing API: you will need to call
spi_configure() before
every spi transaction call, because you won't know if another part of
the system did access
or not the same device, and changed the config/slave etc...

Thus why I want to put that as parameter config and slave.


[...]
4) About zero-copy, we would provide a "scatter-gather" type of buffer.

struct spi_buf {
void *buf;
uint32_t len;
};

And then spi_transceive would be:

int spi_transceive(struct spi_config *conf, uint32_t slave,
const struct spi_buf *tx, uint32_t tx_nb_buf,
struct spi_buf *rx, uint32_t rx_nb_buf);

tx and rx would be array of struct spi_buf, and the function would
get the number of items in each array.

Let's use again cc2520 as an example, on function
write_txfifo_content():

{
uint8_t ins = CC2520_INS_TXBUF;
struct spi_buf cmd[2] = {
{ .buf = &ins, .len = 1 },
{ .buf = net_nbuf_ll(buf),
.len = net_nbuf_ll_reserve(buf) +
net_buf_frags_len(buf)
},
};

return (spi_write(spi_conf, spi_slave, cmd, 2) == 0);
}

Now, when reading, on function cc2520's read_rxfifo_content():

{
uint8_t ins = CC2520_INS_RXBUF;
struct spi_buf cmd = { .buf = &ins, .len = 1 };
struct spi_buf data[2] = {
{ .buf = NULL, .len = 0 },
So the above pairs with the single 'cmd' buffer and means 'ignore the
read data'? And in general, each entry in the tx list pairs with the
equivalent in the rx list.
You can have more tx entries than rx, or the invert. In the example above,
I am interested to read the data, after the first tx, so indeed I have
to indicate that for the
first tx buffer, we drop the received data, but we definitely want the
next bytes.

And if one list runs out it means transmit
zeros or discard received data as appropriate? I.e. the code
implementing the new transceive is a for loop passing pairs of
parameters to the existing transceive?
You could see it like that, but I believe it's way more efficient to
code the logic directly into the driver,
since it's the one who can transparently handle dummy bytes.

Seems simple to implement but a pig to use and understand ;-)
I like my separate function calls with CS flags better :-)
See my comment on you solution.


Some other questions about things not mentioned in the RFC...

1. What about DMA integration? I haven't spotted much in the way of DMA
support in driver APIs and the DMA API itself doesn't support scatter
gather so that's looking like a whole area that's yet to be sorted out?
You are right, it's totally out of the picture right now. So I am fully
open on suggestions/ideas here.


2. Like most Zephyr code, there seems to be an absence of any kind of
locking in drivers or APIs that prevent users interfering with each
other. So, what prevents two drivers/apps trying to talk to two
different devices on the same SPI bus at the same time?
I believe there is a task on-going about that, something about
re-entrance ?


Tomasz


Re: Porting to ARM M0, M0+

Ricardo Salveti <ricardo.salveti@...>
 

On Tue, Sep 13, 2016 at 10:34 AM, Phil Keating <pkeating(a)intrinsix.com> wrote:
Good morning,
I'm new to the Zephyr project, as well as open source in general, and noticed another thread regarding a port to the ARM M0+ processor. I didn't want to piggy back on that thread as my question(s) are more general regarding porting Zephyr from one ARM core (M3/M4) to another (M0/M0+).
I'm looking for a set of guidelines as I've tried mucking up the 1.4.0 tree to get an M0 SOC/board building and keep tripping over the GNU tools claiming my "processor" either doesn't support THUMB instructions at all, or specific THUMB instructions (depending on whether I define CONFIG_ISA_THUMB or CONFIG_ISA_THUMB2).
I noticed a JIRA entry requesting documentation for new SOC/Board porting.
Any pointers, help, etc would be appreciated. For example, since someone is already working on an M0+ port, would that become part of the Zephyr code base?
Take a look at https://jira.zephyrproject.org/browse/ZEP-783. My idea
is to start sending those patches over the next few days, once I'm
done with some other unrelated critical work.

Also check https://github.com/rsalveti/zephyr/commits/nrf51-rebase,
where there is a working port for nRF51 (Cortex-M0). The M0/M0+
related patches should help you understanding some of the build issues
specific to M0/M0+.

Cheers,
--
Ricardo Salveti

6141 - 6160 of 7600