Skip to content

Conversation

@nvlsianpu
Copy link
Owner

settings line is encoding directly by the settings module.

Signed-off-by: Andrzej Puzdrowski [email protected]

@Laczen
Copy link

Laczen commented May 15, 2018

@nvlsianpu, this is a big change to the settings module. I am uncertain if I completely understand the change. From the code I would conclude that the settings module would keep track of: name, value and also the address offset in flash. This way it can access the values quickly. Is this correct ?

@nvlsianpu
Copy link
Owner Author

Almost correct. 'Address offset' in an abstraction storage representation - this represent can be an offset in the flash_area as for FCB backend - it depends what is implemented under write_cb.

@Laczen
Copy link

Laczen commented May 15, 2018

Agreed, for file this will probably be an offset in the file. However what happens when you are rotating, then fcb has to update the settings ?

@Laczen
Copy link

Laczen commented May 16, 2018

@nvlsianpu, this change would make integrating NVS as a backend very easy. Address offset would become a item id for NVS, the settings_nvs part should just keep track of the next "address offset" to "distribute". The start of address offset could be set to 0x8000 and each new setting would get a new offset like 0x8001, 0x8002, ... I just need to think about how to handle deleted items (gaps in the list of address offset).

@Laczen
Copy link

Laczen commented May 16, 2018

@nvlsianpu, would it not be better to move the encoding to the back-end: fcb does not need encoding, file needs encoding, nvs does not need encoding, the communication layer to newtmgr needs encoding.

@nvlsianpu
Copy link
Owner Author

nvlsianpu commented May 16, 2018

@jhedberg FYI - it is very early to look into - but line encoding sub-method is implemented.

@jhedberg
Copy link

jhedberg commented May 17, 2018

I'm curious what kind of implications this has on the public API, since right now it seems you haven't touched it at all. The decoding part will also be tricky, I suppose, since you need a target buffer from the app, whereas with the current design set() is only called when a full value has been decoded.

@nvlsianpu
Copy link
Owner Author

@jhedberg For export handlers the int (*cb)(const char *name, char *value) callback will be changed to int (*cb)(const char *name, char *value, size_t value_length) - this callback will encode value so a value converting prior will be unnecessary.

For set handler I expect that 'char* val' will be replaced by a value_context + callback for fetching the value data ( something int read_cb(void* value_context, void *buf, size_t len). example:

u32_t val32;
char byte_array[32];
size_t array_cnt;

int c_handle_set(int argc, char **argv, void *value_context)
{
    int rc;

    if (argc == 1 && !strcmp(argv[0], "v")) {
        rc = read_cb(value_context, &val32, sizeof(val32));
        if  (rc == sizeof(val32)) {
            return 0;
        }
        
        return -EIO;
    }
    
    if (argc == 1 && !strcmp(argv[0], "s")) {
        rc = read_cb(value_context, byte_array, sizeof(byte_array));
        if  (rc > 0) {
            /* positive rc is equal to the result read size */
            array_cnt = rc;
            return 0;
        }
        
        return -EIO;
    }
    
    return -ENOENT;
}


@jhedberg
Copy link

@jhedberg For export handlers the int (*cb)(const char *name, char *value) callback will be changed to int (*cb)(const char *name, char *value, size_t value_length) - this callback will encode value so a value converting prior will be unnecessary.

Ok, good, however I suppose the "friendly" type (i.e. not requiring explicit type casts) would then be const void *value instead of char *value, right?

@nvlsianpu
Copy link
Owner Author

Right - I just typed this quickly.

@nvlsianpu nvlsianpu force-pushed the settings_stream_codec branch from 1d8ccb0 to 5930ccc Compare May 18, 2018 07:56
@nvlsianpu nvlsianpu changed the base branch from master to add_fcb May 18, 2018 08:07
@nvlsianpu nvlsianpu changed the base branch from add_fcb to master May 18, 2018 08:07
@nvlsianpu nvlsianpu force-pushed the settings_stream_codec branch from 6f81984 to fb74553 Compare May 29, 2018 09:23
@nvlsianpu nvlsianpu force-pushed the settings_stream_codec branch 7 times, most recently from dd84b67 to aebee6e Compare August 19, 2018 14:24
galak and others added 7 commits September 27, 2018 08:09
To move forward and remove use of Kconfig in dts files lets just create
SoC specific dtsi files that the boards can include.  We also seperate
out the F0 dtsi files into their own dir.

Signed-off-by: Kumar Gala <[email protected]>
This commit groups together the MPU region types
that are related to the User-space feature, so that
a single #ifdef USERSPACE is present in the enum.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Some minor style fixes and rewording of the documentation
for ARM MPU region types.

Signed-off-by: Ioannis Glaropoulos <[email protected]>
Some applications might want to check whether flash_areas binds to
any flash drive in the system. It might be better to do that while
sanity check at application start-up then while regular run process.
Example of such application is the mcuboot.

This patch introduce such API for checking whether device bindings
were resolved properly during system startup.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
The posix arch does not compile either of Zephyr's libc,
so _prf() is not avaliable

Signed-off-by: Alberto Escolar Piedras <[email protected]>
The shell subsystem, as it is today, depends on having a UART,
therefore let's add the dependency explicitly in its Kconfig

Fixes zephyrproject-rtos#10190

Signed-off-by: Alberto Escolar Piedras <[email protected]>
Added a new UART driver for posix arch boards.
The driver can be configured to either attach to a new
pseudo-terminal, or to connect to the invoking shell
stdin-out.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
galak and others added 16 commits October 4, 2018 07:48
Drop a few paths from when we preprocess the dts.  This is to reduce
creep of where #defines might get resolved from.

Signed-off-by: Kumar Gala <[email protected]>
Add simple device tree support for the Pulpino SoC and Zedboard-Pulpino
board port.  This gets the UART info from device tree instead of soc.h

Signed-off-by: Kumar Gala <[email protected]>
Add the needed bits to get device tree support for the GPIO controller
on the Zedboard-Pulpino.  This will allow us to move LED & button info
into the board.dts.

Signed-off-by: Kumar Gala <[email protected]>
Move describing of LED & Buttons on the Zedboard-Pulpino into the device
tree.

Signed-off-by: Kumar Gala <[email protected]>
File was named atmel.sam-gpio.yaml, rather than our convention of
atmel,sam-gpio.yaml.

Signed-off-by: Kumar Gala <[email protected]>
The gpio controllers on SAM4S, SAME70, and SAMD were missing properties
related to GPIO pin generation.  Add the missing details into the yaml
and dts files to allow boards to specific gpio pins.

Signed-off-by: Kumar Gala <[email protected]>
Move the details about LEDs and Buttons from board.h into device tree.

Signed-off-by: Kumar Gala <[email protected]>
Move the details about LEDs and Buttons from board.h into device tree.

Signed-off-by: Kumar Gala <[email protected]>
In case of having more than one reg, aliases would not generate
properly. Number of register at the end of define was missing.

Signed-off-by: Mieszko Mierunski <[email protected]>
Declares the array with constant data const.

Tested with tests/unit/lib/crc.

Signed-off-by: Alexander Polleti <[email protected]>
Some of the headers referenced in the pinmux.c file don't exist.  Match
the includes that the lpcxpresso54114_m4 pinmux.c file uses.

Signed-off-by: Kumar Gala <[email protected]>
Tweak the wildcard matching for any build or sanity-out dir

Signed-off-by: Kumar Gala <[email protected]>
Refactor and club the POSIX API tests into one test suite.

Signed-off-by: Spoorthi K <[email protected]>
Enhance semaphore test to validate sem_timedwait()
    and sem_trywait(). This is intended to improve
    code coverage.

Also modify ztest check with zassert_equal with
return value instead of zassert_false for better
understandability.

Signed-off-by: Spoorthi K <[email protected]>
Added functionality to enable active shell backends via Kconfig
file. When there will be more backends implemented user will
have an option to select only required ones.

It is no longer needed to select SERIAL in prj.conf.

Fixes zephyrproject-rtos#10190

Signed-off-by: Jakub Rzeszutko <[email protected]>
In commit d583619 ("Bluetooth: controller: Increase advertising
random delay resolution"), the resolution of random_delay was
increased from 8-bit to 16-bit.  Due to this switch the result
of HAL_TICKER_US_TO_TICKS() can now be a 0, which causes the following
crash:
***** Kernel OOPS! *****
Current thread ID = 0x200043f0
Faulting instruction address = 0x17914
Fatal fault in ISR! Spinning...

Let's make sure we don't pass a 0 to ticker_update() by increasing
the result of HAL_TICKER_US_TO_TICKS() by 1.

Signed-off-by: Michael Scott <[email protected]>
@nvlsianpu nvlsianpu force-pushed the settings_stream_codec branch from aebee6e to 753f97a Compare October 5, 2018 10:05
@nvlsianpu nvlsianpu force-pushed the settings_stream_codec branch 2 times, most recently from 92cb5dc to 6670e64 Compare November 14, 2018 10:59
This patch reworks routines used to store and read the settings data.
Provide stream-style encoding and decoding to/from flash, so the the
API only requires a pointer to binary data, and the settings
implementation takes care of encoding/decoding to/from base64 and
writing/reading to/from flash on the fly. This would eliminate the
need of a separate base64 value buffer on the application-side, thereby
further contributing to the stack footprint reduction.

Above changes allows to remove:
  256-byte value length limitation.
  removing enum settings_type usage so all settings data are treated
  now as a byte array (i.e. what's previously SETTINGS_BYTES)

Introduced routine settings_val_read_cb for read and decode the
settings data from storage inside h_set handler implementations.
h_set settings handler now provide persistent value's context
used along with read routine instead of immediately value.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
Added tests for recently introduced sterem-style encoding.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
Introduction of stream-style encoding required tests reworks.
Test for checking especially byte-string encoding is not required
anymore as any value is kept as byte-string.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
Introduction of stream-style encoding required rework
of tests.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
Introduction of stream-style encoding required rework
of tests.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
@nvlsianpu nvlsianpu force-pushed the settings_stream_codec branch from 7e4d791 to 9a8f0a0 Compare November 16, 2018 16:07
@nvlsianpu
Copy link
Owner Author

upsteram PR zephyrproject-rtos#9521

@nvlsianpu nvlsianpu closed this Nov 20, 2018
nvlsianpu pushed a commit that referenced this pull request Jun 23, 2020
This makes the gatt metrics also available for
gatt write-without-rsp-cb so it now prints the rate of each write:

uart:~$ gatt write-without-response-cb 1e ff 10 10
Write #1: 16 bytes (0 bps)
Write #2: 32 bytes (3445948416 bps)
Write #3: 48 bytes (2596929536 bps)
Write zephyrproject-rtos#4: 64 bytes (6400 bps)
Write zephyrproject-rtos#5: 80 bytes (8533 bps)
Write zephyrproject-rtos#6: 96 bytes (10666 bps)
Write zephyrproject-rtos#7: 112 bytes (8533 bps)
Write zephyrproject-rtos#8: 128 bytes (9955 bps)
Write zephyrproject-rtos#9: 144 bytes (11377 bps)
Write zephyrproject-rtos#10: 160 bytes (7680 bps)
Write zephyrproject-rtos#11: 176 bytes (8533 bps)
Write zephyrproject-rtos#12: 192 bytes (9386 bps)
Write Complete (err 0)
Write zephyrproject-rtos#13: 208 bytes (8533 bps)
Write zephyrproject-rtos#14: 224 bytes (9244 bps)
Write zephyrproject-rtos#15: 240 bytes (9955 bps)
Write zephyrproject-rtos#16: 256 bytes (8000 bps)

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
nvlsianpu pushed a commit that referenced this pull request Nov 10, 2020
The _ldiv5() is an optimized divide-by-5 function that is smaller and
faster than the generic libgcc implementation.

Yet it can be made even smaller and faster with this replacement
implementation based on a reciprocal multiplication plus some tricks.

For example, here's the assembly from the original code on ARM:

_ldiv5:
        ldr     r3, [r0]
        movw    ip, zephyrproject-rtos#52429
        ldr     r1, [r0, zephyrproject-rtos#4]
        movt    ip, 52428
        adds    r3, r3, #2
        push    {r4, r5, r6, r7, lr}
        mov     lr, #0
        adc     r1, r1, lr
        adds    r2, lr, lr
        umull   r7, r6, ip, r1
        lsr     r6, r6, #2
        adc     r7, r6, r6
        adds    r2, r2, r2
        adc     r7, r7, r7
        adds    r2, r2, lr
        adc     r7, r7, r6
        subs    r3, r3, r2
        sbc     r7, r1, r7
        lsr     r2, r3, #3
        orr     r2, r2, r7, lsl zephyrproject-rtos#29
        umull   r2, r1, ip, r2
        lsr     r2, r1, #2
        lsr     r7, r1, zephyrproject-rtos#31
        lsl     r1, r2, #3
        adds    r4, lr, r1
        adc     r5, r6, r7
        adds    r2, r1, r1
        adds    r2, r2, r2
        adds    r2, r2, r1
        subs    r2, r3, r2
        umull   r3, r2, ip, r2
        lsr     r2, r2, #2
        adds    r4, r4, r2
        adc     r5, r5, #0
        strd    r4, [r0]
        pop     {r4, r5, r6, r7, pc}

And here's the resulting assembly with this commit applied:

_ldiv5:
        push    {r4, r5, r6, r7}
        movw    r4, zephyrproject-rtos#13107
        ldr     r6, [r0]
        movt    r4, 13107
        ldr     r1, [r0, zephyrproject-rtos#4]
        mov     r3, #0
        umull   r6, r7, r6, r4
        add     r2, r4, r4, lsl #1
        umull   r4, r5, r1, r4
        adds    r1, r6, r2
        adc     r2, r7, r2
        adds    ip, r6, r4
        adc     r1, r7, r5
        adds    r2, ip, r2
        adc     r2, r1, r3
        adds    r2, r4, r2
        adc     r3, r5, r3
        strd    r2, [r0]
        pop     {r4, r5, r6, r7}
        bx      lr

So we're down to 20 instructions from 36 initially, with only 2 umull
instructions instead of 3, and slightly smaller stack footprint.

Signed-off-by: Nicolas Pitre <[email protected]>
nvlsianpu pushed a commit that referenced this pull request Dec 8, 2022
This patch reworks how fragments are handled in the net_buf
infrastructure.

In particular, it removes the union around the node and frags members in
the main net_buf structure. This is done so that both can be used at the
same time, at a cost of 4 bytes per net_buf instance.
This implies that the layout of net_buf instances changes whenever being
inserted into a queue (fifo or lifo) or a linked list (slist).

Until now, this is what happened when enqueueing a net_buf with frags in
a queue or linked list:

1.1 Before enqueueing:

 +--------+      +--------+      +--------+
 |#1  node|\     |#2  node|\     |#3  node|\
 |        | \    |        | \    |        | \
 | frags  |------| frags  |------| frags  |------NULL
 +--------+      +--------+      +--------+

net_buf #1 has 2 fragments, net_bufs #2 and #3. Both the node and frags
pointers (they are the same, since they are unioned) point to the next
fragment.

1.2 After enqueueing:

 +--------+      +--------+      +--------+      +--------+      +--------+
 |q/slist |------|#1  node|------|#2  node|------|#3  node|------|q/slist |
 |node    |      | *flag  | /    | *flag  | /    |        | /    |node    |
 |        |      | frags  |/     | frags  |/     | frags  |/     |        |
 +--------+      +--------+      +--------+      +--------+      +--------+

When enqueing a net_buf (in this case #1) that contains fragments, the
current net_buf implementation actually enqueues all the fragments (in
this case #2 and #3) as actual queue/slist items, since node and frags
are one and the same in memory. This makes the enqueuing operation
expensive and it makes it impossible to atomically dequeue. The `*flag`
notation here means that the `flags` member has been set to
`NET_BUF_FRAGS` in order to be able to reconstruct the frags pointers
when dequeuing.

After this patch, the layout changes considerably:

2.1 Before enqueueing:

 +--------+       +--------+       +--------+
 |#1  node|--NULL |#2  node|--NULL |#3  node|--NULL
 |        |       |        |       |        |
 | frags  |-------| frags  |-------| frags  |------NULL
 +--------+       +--------+       +--------+

This is very similar to 1.1, except that now node and frags are
different pointers, so node is just set to NULL.

2.2 After enqueueing:

 +--------+       +--------+       +--------+
 |q/slist |-------|#1  node|-------|q/slist |
 |node    |       |        |       |node    |
 |        |       | frags  |       |        |
 +--------+       +--------+       +--------+
                      |            +--------+       +--------+
                      |            |#2  node|--NULL |#3  node|--NULL
                      |            |        |       |        |
                      +------------| frags  |-------| frags  |------NULL
                                   +--------+       +--------+

When enqueuing net_buf #1, now we only enqueue that very item, instead
of enqueing the frags as well, since now node and frags are separate
pointers. This simplifies the operation and makes it atomic.

Resolves zephyrproject-rtos#52718.

Signed-off-by: Carles Cufi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.