Re: [riot-devel] Board stability

2020-10-01 Thread Marian Buschsieweke
Hi,

I like the idea of alphanumeric identifiers. How about using `#define` or
a `enum` that map a alphanumeric token to the RIOT internal numeric identifier?
That way the strings come for free in terms of ROM usage. (But as a downside,
those strings would be hard to e.g. reuse from within the shell. However, the
name of enum constants are visible to GDB.)

E.g. let's say a board configures only one I2C bus which is labeled I2C3 in the
datasheet of the MCU. How about using the following:

#define I2C_ID_3 I2C_DEV(0)

or

typdef enum {
I2C_ID_3 = I2C_DEV(0),
} i2c_dev_stable_id_t;

Kind regards,
Marian

On Thu, 01 Oct 2020 06:18:01 -0400
Michael Richardson  wrote:

> chrysn  wrote:
> > Do we have any existing text on these kinds of stability? Do we want to?
> > If so, is that RDM material or really just a note to the board porting
> > documentation?  
> 
> Perhaps not everyone is aware of, has lived through with great frustration,
> the multiple times the "indices" for ethernet devices have changed on Linux
> (and BSD and Solaris).
> 
> From order of linking in the kernel for ISA devices, to cmdline= order, to
> PCI reverse index order (a goof), to PCI index order, to moving from ethX to
> ensXXX order due to PCI vs USB ordering, to udev for consistent ordering
> despite hotplug, to...
> 
> Fortunately, most devices in the SoC which we typically run on are not
> hotpluggable.   Integer indices are really very convenient, but not stable.
> But, that doesn't mean we should use names at runtime, but perhaps it means
> that we should have a name->indice resolution step that is part of the
> linking step.
> 
> > # Long-term stability  
> 
> > Board identities are generally stable: While they may be renamed if it 
> turns
> > out that the old name was misleading, a physical board that worked with 
> a given
> > name in one release can be expected to work with the next release.  
> 
> Yes, but refactoring.
> We tend to start with some specific vendorSOC-CPU-devel-board name, and then
> refactor out CPU or SOC, for other devel-board.
> 
> --
> ]   Never tell me the odds! | ipv6 mesh networks [
> ]   Michael Richardson, Sandelman Software Works|IoT architect   [
> ] m...@sandelman.ca  http://www.sandelman.ca/|   ruby on rails
> [
> 



pgpKxtIr4Kk3P.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] Does the servo driver work on atmegas?

2020-09-03 Thread Marian Buschsieweke
Hi,

care to open an issue on Github for this? This sounds very much like a bug in
RIOT.

Could you also provide the board details. If you're using an out of tree board,
especially the clock configuration would be of interest.

Kind regards,
Marian

On Tue, 1 Sep 2020 21:58:32 +0200
Wouter Symons  wrote:

> Thanks for the quick response Marian!
> 
> > So, double check that you are using PWM_LEFT.  
> The servo driver uses PWM_LEFT hardcoded.
> 
> > Do you have an logic analyzer at hand to confirm the PWM output is correct? 
> >  
> Great idea! I had already tried checking the output with an oscilloscope 
> I had laying on my desk yesterday, and also tried to hook up a small 
> buzzer, and both more or less indicated that the output was fine. 
> However... I checked with a logic analyzer now, as per your suggestion, 
> and indeed... something is off: if I apply a PWM decoder to the output, 
> it gives me a duty cycle of 0.390221%. It varies slightly, but not by 
> much, and it is not sweeping like it should.
> 
> > Does the serve has any
> > requirements on the PWM frequency?  
> I tried an SG90 and an MG995 servo. Both require a 50Hz frequency 
> according to the datasheet. I adjusted this in the servo driver.
> 
> > this is because currently only the 8 bit timers are used for PWM. There are
> > also 16 bit timers that can be used for PWM, but those are not yet 
> > implemented.
> > I heard that @benpicco's avr-rss2 boards have an on-board LED connected to 
> > the
> > 16 bit PWM output - so he might jump on this and implement the missing 16 
> > bit
> > support ;-)  
> I might have a look myself if I can find some time to spare. However, 
> the project I'm working on also has its deadlines, so I might also just 
> cheat by ordering a I2C PWM controller breakout board instead.
> 
> 
> But I still find it a bit strange though. I quote @smlng on #10498: 
> "/Please note, that this finally brings in the missing PWM periph for 
> the atmega-based Arduinos/". It sounds like PWM should work just fine on 
> this board (unless RIOT handles arduino boards differently than 
> non-arduino boards with the same cpu, but that sounds unlikely). Why 
> would it suddenly not work anymore?
> 
> 
> Kind regards,
> 
> Wouter
> 
> 
> On 9/1/20 8:39 PM, Marian Buschsieweke wrote:
> > Hi,
> >  
> >> I was wondering if it is even possible at all. The current version of
> >> the PWM peripheral has its resolution limited to 256.  
> > this is because currently only the 8 bit timers are used for PWM. There are
> > also 16 bit timers that can be used for PWM, but those are not yet 
> > implemented.
> > I heard that @benpicco's avr-rss2 boards have an on-board LED connected to 
> > the
> > 16 bit PWM output - so he might jump on this and implement the missing 16 
> > bit
> > support ;-)
> >
> > Not that for ATmegas currently only PWM_LEFT is implemented, PWM_RIGHT and
> > PWM_CENTER won't work. So, double check that you are using PWM_LEFT.
> >
> > Do you have an logic analyzer at hand to confirm the PWM output is correct?
> > Otherwise, the brightness of an LED connected to the PWM output should also 
> > be
> > good indicator. (Don't forget to take into account that most ATmegas 
> > operate at
> > 5 V when choosing the resistor for the LED ;-)) Does the serve has any
> > requirements on the PWM frequency?
> >
> > Kind regards,
> > Marian
> >
> > On Tue, 1 Sep 2020 20:00:09 +0200
> > Wouter Symons  wrote:
> >  
> >> Hello all,
> >>
> >> I spent quite some time yesterday trying to get a servo working on an
> >> atmega1284p, unfortunately to no avail.
> >>
> >> I was wondering if it is even possible at all. The current version of
> >> the PWM peripheral has its resolution limited to 256. I think this
> >> limitation is probably the cause for it not working.
> >>
> >> I tried removing the asserts that enforce this limitation, and this
> >> caused the servo to rotate into one direction, but then stop moving
> >> again (I could not get it to rotate in the other direction).
> >>
> >> I tried the same servo on a nucleo-board and this worked just fine.
> >> However... only with a resolution way higher than 256. (Because STM32
> >> does not have this limitation)
> >>
> >> So I wonder... should atmega be blacklisted for this test, since it
> >> cannot work? And would there maybe be a workaround to this problem? I'm
> >> not that experienced on this matter, so I'm not really sure why the
&g

Re: [riot-devel] Does the servo driver work on atmegas?

2020-09-01 Thread Marian Buschsieweke
Hi,

> I was wondering if it is even possible at all. The current version of 
> the PWM peripheral has its resolution limited to 256.

this is because currently only the 8 bit timers are used for PWM. There are
also 16 bit timers that can be used for PWM, but those are not yet implemented.
I heard that @benpicco's avr-rss2 boards have an on-board LED connected to the
16 bit PWM output - so he might jump on this and implement the missing 16 bit
support ;-)

Not that for ATmegas currently only PWM_LEFT is implemented, PWM_RIGHT and
PWM_CENTER won't work. So, double check that you are using PWM_LEFT.

Do you have an logic analyzer at hand to confirm the PWM output is correct?
Otherwise, the brightness of an LED connected to the PWM output should also be
good indicator. (Don't forget to take into account that most ATmegas operate at
5 V when choosing the resistor for the LED ;-)) Does the serve has any
requirements on the PWM frequency?

Kind regards,
Marian

On Tue, 1 Sep 2020 20:00:09 +0200
Wouter Symons  wrote:

> Hello all,
> 
> I spent quite some time yesterday trying to get a servo working on an 
> atmega1284p, unfortunately to no avail.
> 
> I was wondering if it is even possible at all. The current version of 
> the PWM peripheral has its resolution limited to 256. I think this 
> limitation is probably the cause for it not working.
> 
> I tried removing the asserts that enforce this limitation, and this 
> caused the servo to rotate into one direction, but then stop moving 
> again (I could not get it to rotate in the other direction).
> 
> I tried the same servo on a nucleo-board and this worked just fine. 
> However... only with a resolution way higher than 256. (Because STM32 
> does not have this limitation)
> 
> So I wonder... should atmega be blacklisted for this test, since it 
> cannot work? And would there maybe be a workaround to this problem? I'm 
> not that experienced on this matter, so I'm not really sure why the 
> limitation on this resolution was put in place, but is it possible to 
> make changes to the peripheral driver so we can increase it?
> 
> It's also possible that I'm simply doing something very wrong, and that 
> it should work without any changes. (hence the reason I asked on this 
> mailing list instead of opening an issue straight away)
> 
> Kind regards,
> 
> Wouter
> 
> ___
> devel mailing list
> devel@riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel



pgpZKB1fOC16x.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] Which tests are expected to succeed for my board

2020-05-30 Thread Marian Buschsieweke
Hi,

every failing test indicates a bug. (From time to time the bug is found in the
test rather than in the code to test.) Feel free to open an issue for the
failing tests. I personally favor a tracking issue that lists each failing test
with a checkbox. It is motivating to tick them of one by one; and with the
tiring task of bug hunting some motivation is always welcome :-)

Kind regards,
Marian

On Fri, 29 May 2020 23:14:19 +0200
Kees Bakker  wrote:

> Hi,
> 
> Now that I'm happily running automated tests for my SAMD21 board(s) I am
> wondering which tests should succeed. There are several that fail, but I
> don't know if that "normal" or not.
> 
> Some examples of test that fail
> 
> xtimer_periodic_wakeup: hangs at the end, last couple of printf don't
> come out
> tests/thread_flood: [ERROR] expected 30, created 29
> 
> Is there maybe a wiki with the status of the tests (per board?).



pgpjm62I2o1MM.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] ADC API resolution

2020-01-08 Thread Marian Buschsieweke
Hi,

> I was thinking of a function such as `adc_res_t adc_res_max(adc_line_t line)`
> to be able to support different ADC peripherals on a single image.

good point! This could be provided with no run-time overhead in
drivers/include/periph/adc.h e.g. via:

#if !defined(HAVE_ADC_RES_MAX)
static inline adc_res_t adc_res-max(adc_line_t line) {
(void)line;
return ADC_RES_NUMOF - 1;
}
#endif /* HAVE_ADC_RES_MAX */

And overwritten as needed. This would also make retro-fitting external ADC
drivers to that API easier.

Kind regards,
Marian

On Wed, 8 Jan 2020 14:59:18 +0100
Koen Zandberg  wrote:

> Hi all,
> 
> A month or so back I added ADC peripherals bindings for the MicroPython
> fork. As this had to be generic across boards I also stumbled over a
> missing run-time indicator for which resolutions are supported. At that
> moment I worked around it by iterating over all possible ADC resolutions
> until a supported resolution was tried. Of course this is a bit suboptimal.
> 
> I'm very much in favor of some way to determine the available
> resolutions and min/max resolutions of an ADC. I was thinking of a
> function such as `adc_res_t adc_res_max(adc_line_t line)` to be able to
> support different ADC peripherals on a single image.
> 
> Just my 2 cents on this issue. I'm already glad that this is tackled.
> 
> Cheers,
> Koen
> 
> On 08/01/2020 14.30, Hauke Petersen wrote:
> > Hi Marian,
> >
> > I agree that users are unlikely to produce that garbage when passing
> > value to the `adc_sample` function directly. More likely those kind of
> > values are produced in cases where values are deducted
> > programmatically via (potentially broken) code, or from broken
> > indirect configuration (nested defines...). The main point is simply
> > that the compiler does not catch this and we need to handle it somehow  
> > -> which we agree upon as it seems :-)  
> >
> >  So +1 in documenting the preconditions correctly, checking the `line`
> > and `res` values using assertions and not returning any specific code.
> > How about we change the API doc to something `@return <0 for any
> > internal error` or similar, to still allow certain peripherals to
> > signal an error and mark the result invalid.
> >
> > While touching this API, would it make sense to also change the return
> > type from `int` to `int32_t`? This would allow for 16-bit ADCs on 16-
> > and 8-bit platforms... Just thinking loud here :-)
> >
> > Cheers (and thanks for tackling this!),
> > Hauke
> >
> > On 1/7/20 10:09 AM, Marian Buschsieweke wrote:  
> >> Hi,
> >>
> >> thanks for your reply.
> >>
> >> On Tue, 7 Jan 2020 09:00:54 +0100
> >> Hauke Petersen  wrote:
> >>  
> >>> Hi,
> >>>
> >>> keep in mind that just because an enum value is not defined, it does not 
> >>> prevent code like
> >>> ```
> >>> adc_res_t res = 77;
> >>> adc_init(.., res);
> >>> ```
> >>> Also, calling `adc_init(..., 1234)` is completely fine for the
> >>> compiler...  
> >> To me, this is a text book example of garbage in, garbage out. I personally
> >> don't expect someone to use random numbers out of the head of their mind
> >> instead of the enum constants and expecting things to just somehow
> >> magically work.
> >>
> >> How about we add a precondition statement to the API documentation that
> >> only values provided via enum constants are valid input for the resolution?
> >>
> >> But if we cannot assume a minimum level of common sense being applied to
> >> the users source code, why not at least use assert()? This way at least
> >> production code doesn't have to pay the overhead of checking for
> >> completely insane bugs.
> >>
> >> (Don't get me wrong: I personally very much in favor of doing proper error
> >> handling even at the expense of some overhead. But adding overhead to
> >> check for completely crazy stuff seams not to be a good trade off to me.)
> >>  
> >>> Hence the two fold approach in the current implementations: each CPU 
> >>> should define only the ADC_RES_X values it actually supports and on top 
> >>> adc_sample() *should* return -1 on unsupported values to catch mishaps 
> >>> as stated above.  
> >> Sadly, neither is it (consistently) implemented like this nor documented
> >> this way. But if there is an agreement that only the enum constants
> >> actually supported should be defined, I can open a PR t

Re: [riot-devel] ADC API resolution

2020-01-07 Thread Marian Buschsieweke
Hi,

thanks for your reply.

On Tue, 7 Jan 2020 09:00:54 +0100
Hauke Petersen  wrote:

> Hi,
> 
> keep in mind that just because an enum value is not defined, it does not 
> prevent code like
> ```
> adc_res_t res = 77;
> adc_init(.., res);
> ```
> Also, calling `adc_init(..., 1234)` is completely fine for the compiler...

To me, this is a text book example of garbage in, garbage out. I personally
don't expect someone to use random numbers out of the head of their mind
instead of the enum constants and expecting things to just somehow magically
work.

How about we add a precondition statement to the API documentation that only
values provided via enum constants are valid input for the resolution?

But if we cannot assume a minimum level of common sense being applied to the
users source code, why not at least use assert()? This way at least production
code doesn't have to pay the overhead of checking for completely insane bugs.

(Don't get me wrong: I personally very much in favor of doing proper error
handling even at the expense of some overhead. But adding overhead to check for
completely crazy stuff seams not to be a good trade off to me.)

> Hence the two fold approach in the current implementations: each CPU 
> should define only the ADC_RES_X values it actually supports and on top 
> adc_sample() *should* return -1 on unsupported values to catch mishaps 
> as stated above.

Sadly, neither is it (consistently) implemented like this nor documented this
way. But if there is an agreement that only the enum constants actually
supported should be defined, I can open a PR to document this. But let's keep
the discussion on how to handle it if users call adc_sample() with a resolution
not provided by the enum going for while.

While I'm at it: How should adc_sample() behave if the line parameter is out of
range? This is something that can easily happen, e.g. when compiling code
written for one board for a different board. Again: I would say a precondition
added to the doc and an assert() would be this best solution here.

I'd also like to add that the API is not safe to be called from IRQ context, as
(at least some) implementations use a mutex to serialize access to the ADC.

(Btw: If we would replace the `ADC_LINE()` macro by a `static inline adc_t
adc_line(unsinged num)` function, we could use `_Static_assert()` to generate
compile time errors there as well. For backward compatibility an macro
ADC_LINE() calling that function could be provided.)

> 
> But I do like the idea of adding defines like `HAVE_ADC_RES_X` and 
> `_MAX` (and `_MIN`).
> 

I'll open a PR for that. (I will also add the minimum resolution in that.)

Kind regards,
Marian

> Cheers,
> Hauke
> 
> On 12/26/19 10:01 AM, Marian Buschsieweke wrote:
> > Hi,
> >
> > I just noticed that most of the ADC implementations providing their own
> > adc_res_t do not cover all values. The API documentation states that
> > adc_sample() should return -1 on unsupported resolutions. This indicates 
> > that
> > all possible resolutions have to be defined in any case, so that a user 
> > could
> > check at run time which resolutions are provided.
> >
> > However: Wouldn't it make more sense to only provide the enum values 
> > actually
> > supported? This would have two advantages:
> >
> > 1. Currently all places where adc_res_t is provided need to be updated when 
> > new
> > resolutions are added, resulting in some maintenance effort
> > 2. Only having the resolutions defined that are actually supported would 
> > result
> > in compile time errors, which are much easier to spot and debug than 
> > run time
> > errors
> >
> > Additionally, use cases where users needed to determine available 
> > resolutions
> > could be address by e.g. defining HAVE_ADC_RES_10BIT when ADC_RES_10BIT is
> > supported. And ADC_RES_MAX could be provided for the highest resolution enum
> > and ADC_RES_MAX_BITS for the number of bits this has. This would allow to
> > determine the resolution at compile time, resulting in less overhead in 
> > terms
> > of both runtime and memory.
> >
> > But: As currently the approach to detect available resolutions would result 
> > in
> > compile time errors (when testing for resolutions not covered in the enum),
> > maybe nobody actually needs this?
> >
> > Kind regards,
> > Marian
> >
> > ___
> > devel mailing list
> > devel@riot-os.org
> > https://lists.riot-os.org/mailman/listinfo/devel  
> 
> ___
> devel mailing list
> devel@riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel



pgpsxrMuHhtVc.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


[riot-devel] ADC API resolution

2019-12-26 Thread Marian Buschsieweke
Hi,

I just noticed that most of the ADC implementations providing their own
adc_res_t do not cover all values. The API documentation states that
adc_sample() should return -1 on unsupported resolutions. This indicates that
all possible resolutions have to be defined in any case, so that a user could
check at run time which resolutions are provided.

However: Wouldn't it make more sense to only provide the enum values actually
supported? This would have two advantages:

1. Currently all places where adc_res_t is provided need to be updated when new
   resolutions are added, resulting in some maintenance effort
2. Only having the resolutions defined that are actually supported would result
   in compile time errors, which are much easier to spot and debug than run time
   errors

Additionally, use cases where users needed to determine available resolutions
could be address by e.g. defining HAVE_ADC_RES_10BIT when ADC_RES_10BIT is
supported. And ADC_RES_MAX could be provided for the highest resolution enum
and ADC_RES_MAX_BITS for the number of bits this has. This would allow to
determine the resolution at compile time, resulting in less overhead in terms
of both runtime and memory.

But: As currently the approach to detect available resolutions would result in
compile time errors (when testing for resolutions not covered in the enum),
maybe nobody actually needs this?

Kind regards,
Marian


pgpl6bbrRXU73.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] ztimer - a new high-level timer for RIOT

2019-12-16 Thread Marian Buschsieweke
Hi,

in order to push this forward I just opened a PR for an RDM at
https://github.com/RIOT-OS/RIOT/pull/12970. This PR is in an early state and
both feedback and help will be greatly appreciated. 

> IMO, users should just not use these timer values as timestamps.
> That maybe needs to be stated at a prominent place (or multiple) in the
> documentation.
> For that, we can (should?) re-introduce a 64bit API in ztimer.

I don't think that 64 bit is needed for every feature of a high-level timer
API; especially for delaying the execution of the calling thread or for
software timers. A 64 bit system clock would however be quite nice in some use
cases.

> > E.g. adding support for dedicated overflow interrupts together with an
> > API to read
> > the corresponding IRQ status bit.
> > The high level timer would benefit from that on many platforms.
> > E.g. Ztimer wouldn't require the code for the time partitioning
> > mechanism then.
> > But thats yet another part of the story...  
>
> Yes. Also, do all platforms support that overflow interrupt?
> I don't think so, in which case this cannot be relied upon to be available.

This would not strictly be needed. In case either an overflow interrupt or a
alarm interrupt is supported, the other could be implemented by the timer
driver. (If only an overflow is supported, the driver could set the current
timer value so that the overflow interrupt happens exactly when the alarm
should happen. And if only an alarm is supported, the driver could multiplex
two alarms with one being the overflow.) It needs however to be determined if
this would indeed be better.

> Marian: would you also be interested in attending a timer meeting?

If I can find the time, sure! Some weekend in the first half of January might
work for me. How about Kaspar or Michel can organize a room and use one of the
many online scheduling services to find out a date most interested developers
could join?

Kind regards,
Marian

On Mon, 16 Dec 2019 11:31:34 +0100
Kaspar Schleiser  wrote:

> Hi Ralf,
> 
> On 12/13/19 6:41 PM, Ralf Schlatterbeck wrote:
> > As far as I understand, the new timer implementation would not use 64
> > bit for the timer and the user is responsible for not overrunning the
> > timer? Note that I haven't looked a the implementation yet, so forgive
> > my ignorance.  
> 
> I think you got it right. In ztimer, the clocks' now() value will
> overflow after 32bits.
> 
> E.g., `t1 = now(); t2 = now(); assert(t1 < t2)` might fail.
> 
> Setting timer values is always relative (target = now() + X), and X can
> be any 32bit value, the user does not need to handle any overflow there.
> 
> > Over the years my experience is that it's no good idea to burden the
> > user with the knowledge of timer overflow. The latest example was a
> > bunch of HPE SSDs that stop working after 32.768 hours (a little less
> > than 4 years). Bad when you have several of them in a RAID for
> > redundancy :-)  
> 
> Failing after 32768h strongly points to using a signed 16bit variable
> for the hours count. That's an odd choice to begin with.
> 
> > https://www.techradar.com/news/hpe-ssd-drives-could-fail-at-this-critical-moment
> > So I'd vote for the (small) additional overhead, even on 8-bit µCs due
> > to safety reasons. Unless the implementation can produce the correct
> > timer representation with, say, C-preprocessor magic at compile-time.  
> 
> You mean, you'd vote for all timer values to be always 64bit? I don't
> think the overhead is small enough to justify that, but that needs to be
> evaluated.
> The increased CPU usage needs benchmarks, in RAM we can estimate easily.
> 
> IMO, users should just not use these timer values as timestamps.
> That maybe needs to be stated at a prominent place (or multiple) in the
> documentation.
> For that, we can (should?) re-introduce a 64bit API in ztimer.
> 
> It might make sense to add a (very large or possibly randomized)
> constant offset to all 32bit timers, maybe limited to development
> builds, that causes the timer value to overflow early(er), so
> applications would break early in tests.
> 
> Kaspar
> 
> ___
> devel mailing list
> devel@riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel



pgpxImlMJaxBj.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] ztimer - a new high-level timer for RIOT

2019-12-12 Thread Marian Buschsieweke
Hi,

here are my thoughts on the discussion.

# Not Getting Lost in Requirement Analysis and Problem Specifications

A good requirement analysis is a valuable tool for both development and
evaluation of a software component. But once a solid understanding of the
problem to solve is reached, additional effort put into a requirement analysis
doesn't yield enough additional benefit to justify the work. And every
requirement analysis is flawed: Assembling a complete list of all current
requirements in RIOT's code base is hard. Predicting how future developments
influence the requirements we have is impossible. There has to be a point when
we just stop on collecting more requirements and consider the current list as
good enough; a perfect, complete and definite result cannot be reached.

High level timer API are no new concept and the basic goals and requirements
are well understood. On top of these basic requirements, benchmarks could be a
good tool to quantify how well specific timer implementations perform. To me,
writing a set of benchmarks would be more useful than additional requirements
collection. Not only would it allow to see how good ztimer/xtimer are
performing. They will also be useful for development and reviews of future PRs
targeting RIOTs timer framework.

In the end, RIOT will be judged upon the features it provides. Not on the
features on RIOT's to do lists. Not on how tough and rigorous the requirements
are we have formulated on some yet-to-be-implemented feature. And not on how
much documents and emails have been written about an yet-to-be-implemented
feature.

# Complete Rewrites can be the Best Option

@Michel: You seem to dismiss the development approach of a complete rewrite
fundamentally, apparently for ideological reasons. While most of the time a
complete rewrite is not the best option, there are good reasons for doing so in
some cases: When fundamental architectural changes are needed, the effort of an
rewrite can be less compared to iterative transform the architecture. In such
cases, a rewrite is the better option.

I don't think that there is a reason to see a complete rewrite as some kind
of failure that we should try very hard to prevent for the future. And
ultimately, any attempt to prevent rewrites can fail. Any design decision is
based on the current experience, the available tools, and the current
requirements. But with time, more experience is gained, better tools become
available, and completely new use cases with potentially fully distinct
requirements pop up. Perfectly solid design decisions can be rendered obsolete
by this. And sometimes, fundamental architecture changes are needed in
response. In that cases, a rewrite can be the best option. And this could
happen very well without any mistakes being made in the original implementation.

# Suggestion of a Path Forward

There seems to be an agreement, that an additional parameter is needed in the
API. How about we specify an implementation independent high level timer API
based on xtimer with an additional parameter. The type of the first argument
could just by some `foo_t` that is provided by the implementation of that API.
The implementation could be freely chosen as module, and xtimer could be the
default backend when the user has not specified a backend.

I bet it is trivial to extend xtimer in a way to comply with that API by just
adding an ignored parameter. Everyone with the motivation, knowledge and
dedication to fix the discussed issue in xtimer has still the opportunity to do
so. And providing ztimer as alternative implementation of the same API would
not hurt anyone.

Not defining the semantics and contents of the first parameter sounds a bit
crazy. But I bet that 90% of all use cases will only use two different values
there: One low power low resolution setting for long sleeps and one high power
high precision setting e.g. in drivers. Just providing these two settings as
global variables (or preprocessor macros) under defined names would be enough
to cater 90% of the use cases in an implementation agnostic way. (And the other
use cases could provide their own global variables for the additional settings,
so that only the assignment of those variable depend on the used
implementation.) And if there is indeed enough interest in fixing xtimer, this
would allow them to freely decide if they want this parameter to be a pointer
to a clock struct as in ztimer, or maybe a bitmask holding a set of flags, or
something completely different.

It might be very well possible that the future will bring use cases will
mutually exclusive requirements. A implementation independent API would allow
to just let the users choose what they need.

Kind regards,
Marian


On Tue, 10 Dec 2019 18:23:53 +0100
Michel Rottleuthner  wrote:

> Hi,
> 
> Thanks for starting this! It's very much appreciated.
> Discussing these things, reaching common ground and documenting 
> decisions and findings
> during this process is IMO one of the most 

Re: [riot-devel] ztimer - a new high-level timer for RIOT

2019-12-09 Thread Marian Buschsieweke
Hi,

I was just literally about to send an email with pretty much the same arguments
Kaspar wrote right now. So I skip them and throw in a +1 instead.

> Below that, context switching takes the bulk of the time, so spinning (not
> using a callback) is probably preferable.

I think that the ws281x driver is currently one of the most timing-critical
driver in RIOT. And in the worst case scenario it currently is useable, it needs
a delay of exactly 2-3 CPU cycles. On the same hardware, it takes longer from
an interrupt request coming in to the ISR being run than those 2-3 CPU cycles.

To me, it is safe to say that there are simply use cases that just cannot be
addressed with any high level timer API. So in my opinion focusing on the use
cases that can reasonably be addressed with a high level API makes most sense.
And based on this, follow up PRs can try to squeeze as much efficiency out of
the implementation as possible without sacrificing something on the other
metrics (e.g. ROM/RAM consumption, maintainability, flexibility, etc.).

Kind regards,
Marian

On Mon, 9 Dec 2019 23:28:19 +0100
Kaspar Schleiser  wrote:

> Hey,
> 
> On 12/9/19 10:32 PM, Oleg Hahm wrote:
> > Hm, to be honest, I'm not so sure of what kind of efficiency we're speaking
> > here. CPU time or memory? Probably both, right? Regarding the CPU
> > efficiency, I would assume that this also dictates the maximum precision,
> > right?  
> 
> I don't think so. The hardware dictates the maximum precision.
> 
> I wrote a test (its in tests/ztimer_overhead in the ztimer PR), that
> sets a timer, which in its callback gets the current time, and then
> calculates the differenve.
> 
> Uncorrected (with XTIMER_OVERHEAD=0 or the ztimer equivalent), both add
> a pretty constant ~7us, on an nrf52dk using a periph timer clocked at
> 1MHz. There's not much happening in this simplest case. I don't think we
> can carve off much in any high level timer implementation, compared to
> using periph_timer or even skipping that and using plain register writes.
> 
> Callback based (high-level) timers are only suitable for timeouts down
> to a certain level (somewhere below 10us on our hardware scope). Below
> that, context switching takes the bulk of the time, so spinning (not
> using a callback) is probably preferable.
> I mean, if a device can do 100k context switches per second, each takes
> 10us. Setting a timer to anything below that might work, but doesn't
> make much sense.
> 
> Even ztimer using only 32bit arithmetic vs. xtimer sometimes needing
> 64bit comparisons might show measurable differences when there are many
> timers active, but I honestly don't know if this difference can become a
> deal breaker, or even just relevant.
> 
> Cycle wise, xtimer is not terrible to begin with.
> 
> That said, if we get an alternative that matches or improves on most
> metrics and is still more flexible, and allows proper sleeping, then I
> take any percent improvement with a happy face.
> 
> > Regarding
> > memory we have probably different requirements: ROM for the whole thing, RAM
> > per instance and for the module itself.
> >   
> 
> Here, the requirement should be "needs to fulfill all other
> requirements", and from that point on, less is better.
> xtimer and ztimer are comparable in ROM and RAM usage. Again, I don't
> think that any high level timer implementation can bring disruptive
> reduction here.
> 
> I'm not looking forward to coming up with requirements for accuracy.
> Simple tests (like the overhead test from above) are easy to optimize so
> a timer hits bang on the target time, by subtracting a measured overhead
> from any interval. Figuring out the impact of having N timers in the
> list is difficult, and more difficult to correct.
> 
> Here we could come up with a test setting as many timers as fit in RAM
> to random values, then measuring each timer's punctuality. All making
> sure that the target times don't overlap.
> 
> But all the requirements don't help over the fact that xtimer currently
> just doesn't work for many identified use cases. Fixing that would
> require substantial code changes, probably including API changes.
> Knowning the complexity of xtimer, I decided to give a rewrite from
> scratch a try, and IMO, the result is quite nice.
> 
> I'd maybe roll up the discussion from a different side: If noone can
> identify a conceptual flaw, something where a different implementation
> or design might yield benefits compared to ztimer, and if ztimer does
> not show regressions compared to xtimer, but improves on some relevant
> aspects, why don't we consider going with better in the short term and
> pursue provably best as a longer term goal?
> 
> >>> Besides I'm missing a requirement regarding the maximum granularity and
> >>> the maximum duration of a timer.  
> >>
> >> You mean minimum granularity?  
> > 
> > In my understanding of the term maximum granularity translates to the finest
> > granularity.  
> 
> Yup, got it.
> 
> Kaspar
> 

Re: [riot-devel] ztimer - a new high-level timer for RIOT

2019-12-09 Thread Marian Buschsieweke
Dear Thomas,

I'd like to point out that the research community has largely dismissed Karl
Poppers contribution to the demarcation problem, as largely accepted fields of
research are not falsifiable. E.g. the evolution theory cannot be falsified.

Maybe it is time for you to move on as well?

Kind regards,
Marian

On Mon, 9 Dec 2019 20:35:24 +0100
"Thomas C. Schmidt"  wrote:

> Hi Marian,
> 
> On 09/12/2019 20:06, Marian Buschsieweke wrote:
> 
> >> Also, a clear and falsifiable problem statement should be given.  
> > 
> > could you elaborate on what you mean by a problem statement being 
> > falsifiable?  
> 
> "falsifiable" is a standard principle in science: It is sometimes 
> difficult to verify a statement, if application contexts cannot be 
> exhaustively enumerated ("It never rains in California"). Falsification 
> is often simpler, since only a counter-example is needed. A statement 
> that is neither verifiable nor falsifiable is useless for rigorous 
> argumentation.
> "RIOT needs an easy to use, efficient and flexible timer system" is such 
> a poster child of a non-arguable statement and may move to the introduction.
> 
> Regarding xtimer:
> 
> If the current 1 us resolution in the API is the key problem, then this 
> should be stated clearly in the problem statement so that it can be 
> questioned and discussed.
> 
> Best,
>   Thomas
> 
> > Do you want to be able to check that a given problem cannot be solved by
> > existing features?
> >   
> >> This should IMO address the question, why timer problems cannot be fixed by
> >> simply repairing the xtimer (+ underlying HW abstractions).  
> > 
> > The mayor issue is that the API uses a fixed 1 µs resolution. As an 
> > uint32_t in
> > 1 µs resolution would overflow after ~72 minutes, an uint64_t is needed as a
> > direct consequence. This in turn results in the use of 64 bit arithmetic, 
> > which
> > is very ill suited on IoT devices, especially on 8 bit and 16 bit platforms.
> > 
> > Additionally, an API using 1µs resolution can be best implemented with fast
> > timer hardware. But those usually prevent any power saving modes. This is 
> > very
> > ill suited for the huge majority of IoT scenarios.
> > 
> > Simply changing xtimer to use an RTT instead would solve the power saving
> > issue. But RTTs usually operate at frequencies in the range of 1kHz -
> > 32.678kHz. A base unit of 1µs makes very little sense in that case. And I'm
> > aware of places in RIOT that depend on xtimer having a resolution in the 
> > order
> > of microseconds; those would just break.
> > 
> > All those issues are direct consequences of the use of a fixed 1 µs 
> > resolution.
> > Allowing callers to specify the timer resolution would fix these. But that
> > requires an API change.
> > 
> > (All that reasoning is part of the wiki page already.)
> > 
> > Kind regards,
> > Marian
> > 
> > On Mon, 9 Dec 2019 18:48:39 +0100
> > "Thomas C. Schmidt"  wrote:
> >   
> >> Hi,
> >>
> >> if this is a "problem statement and design document", then concise and
> >> measurable requirements on power management should go into the
> >> corresponding section.
> >>
> >> Also, a clear and falsifiable problem statement should be given. This
> >> should IMO address the question, why timer problems cannot be fixed by
> >> simply repairing the xtimer (+ underlying HW abstractions).
> >>
> >> A long list of ztimer promises appears rather unessential and confusing.
> >>
> >> Thomas
> >>
> >> On 09/12/2019 17:45, Kaspar Schleiser wrote:  
> >>> Hi,
> >>>
> >>> On 12/9/19 4:52 PM, Kaspar Schleiser wrote:  
> >>>> Hi Robert,
> >>>>
> >>>> On 12/9/19 4:25 PM, Robert Hartung wrote:  
> >>>>> Do we need to put any thoughts in power management / low_power /
> >>>>> integration with pm_layered? Or are the possible issues addreses /
> >>>>> already talked about?  
> >>>>
> >>>> Yes and yes. ;)
> >>>>
> >>>> I'll my thoughts so far.  
> >>>
> >>> I've added this to the wiki page:
> >>>
> >>> # Power management considerations
> >>> - currently, ztimer is pm_layered agnostic. If a timer is set on a
> >>> periph_timer, this would probably not prevent sleep (timer would not
> >>> trigger), whereas if a ztimer i

Re: [riot-devel] ztimer - a new high-level timer for RIOT

2019-12-09 Thread Marian Buschsieweke
Hi Thomas,

> Also, a clear and falsifiable problem statement should be given.

could you elaborate on what you mean by a problem statement being falsifiable?
Do you want to be able to check that a given problem cannot be solved by
existing features?

> This should IMO address the question, why timer problems cannot be fixed by 
> simply repairing the xtimer (+ underlying HW abstractions).

The mayor issue is that the API uses a fixed 1 µs resolution. As an uint32_t in
1 µs resolution would overflow after ~72 minutes, an uint64_t is needed as a
direct consequence. This in turn results in the use of 64 bit arithmetic, which
is very ill suited on IoT devices, especially on 8 bit and 16 bit platforms.

Additionally, an API using 1µs resolution can be best implemented with fast
timer hardware. But those usually prevent any power saving modes. This is very
ill suited for the huge majority of IoT scenarios.

Simply changing xtimer to use an RTT instead would solve the power saving
issue. But RTTs usually operate at frequencies in the range of 1kHz -
32.678kHz. A base unit of 1µs makes very little sense in that case. And I'm
aware of places in RIOT that depend on xtimer having a resolution in the order
of microseconds; those would just break.

All those issues are direct consequences of the use of a fixed 1 µs resolution.
Allowing callers to specify the timer resolution would fix these. But that
requires an API change.

(All that reasoning is part of the wiki page already.)

Kind regards,
Marian

On Mon, 9 Dec 2019 18:48:39 +0100
"Thomas C. Schmidt"  wrote:

> Hi,
> 
> if this is a "problem statement and design document", then concise and 
> measurable requirements on power management should go into the 
> corresponding section.
> 
> Also, a clear and falsifiable problem statement should be given. This 
> should IMO address the question, why timer problems cannot be fixed by 
> simply repairing the xtimer (+ underlying HW abstractions).
> 
> A long list of ztimer promises appears rather unessential and confusing.
> 
> Thomas
> 
> On 09/12/2019 17:45, Kaspar Schleiser wrote:
> > Hi,
> > 
> > On 12/9/19 4:52 PM, Kaspar Schleiser wrote:  
> >> Hi Robert,
> >>
> >> On 12/9/19 4:25 PM, Robert Hartung wrote:  
> >>> Do we need to put any thoughts in power management / low_power /
> >>> integration with pm_layered? Or are the possible issues addreses /
> >>> already talked about?  
> >>
> >> Yes and yes. ;)
> >>
> >> I'll my thoughts so far.  
> > 
> > I've added this to the wiki page:
> > 
> > # Power management considerations
> > - currently, ztimer is pm_layered agnostic. If a timer is set on a
> > periph_timer, this would probably not prevent sleep (timer would not
> > trigger), whereas if a ztimer is set on a rtt, it would behave as
> > expected (timer hardware keeps running in sleep, timer isr wakes up MCU).
> > 
> > - (TODO) if a timeout has been set (e.g., `ztimer_set(clock, timeout)`),
> > the backend device blocks sleeping if necessary. IMO this is the minimum
> > requirement, but still needs to be implemented.
> > 
> > - Idea: we specify that by convention, ZTIMER_MSEC (and ZTIMER_SEC)
> > *keep running in sleep mode*, whereas ZTIMER_USEC stops when the MCU
> > enters sleep (unless a timeout is scheduled). This is current behaviour
> > *if* ZTIMER_USEC is using periph_timer as backend and ZTIMER_MSEC is
> > using RTT/RTC.
> > 
> >This would mean that `before = ztimer_now(clock); do something; diff =
> > ztimer_now(clock) - before;` only works if either `do_something` does
> > not schedule away the thread causing sleep *or* a clock is used that
> > runs in sleep mode.
> > 
> > - the behaviour could be accessible either through defines
> > (ZTIMER_USEC_LPM_MODE or ZTIMER_USEC_DEEPSLEEP ...), *or* be made part
> > of the ztimer API
> > 
> > - in addition, we could add functions to explicitly tell the clocks to
> > stay available until released, e.g., `ztimer_acquire(clock); before =
> > ztimer_now(clock); do something; diff = ztimer_now(clock) - before;
> > ztimer_release(clock);`.
> >Once the "if timer is scheduled, don't sleep" is implemented, this
> > could also be worked around by:
> >  `ztimer_set(clock, dummy, 0x); ...; ztimer_cancel(clock,
> > dummy);`
> > 
> > 
> > Feedback appreciated!
> > 
> > Kaspar
> > ___
> > devel mailing list
> > devel@riot-os.org
> > https://lists.riot-os.org/mailman/listinfo/devel
> >   
> 



pgp9YsMqnufSk.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] Driver for the at86rf215

2019-10-25 Thread Marian Buschsieweke
Hi,

see https://github.com/RIOT-OS/RIOT/pull/12537

Kind regards,
Marian


On Fri, 25 Oct 2019 16:06:02 +0200
Robert Olsson  wrote:

> Hi,
> 
> More questions on this topic... are there any PR pending for the
> radio integrated in the AtMega128rfa1 and AtMega256rfr2? Very much
> the same but not over SPI.
> 
> Cheers
>   --ro
> 
> 
> José Alamos writes:
>  > Hi Robert,
>  > 
>  > No Problem!
>  > Good that you asked before so there weren't duplicated attempts :P
>  > 
>  > Cheers,
>  > José
>  > 
>  > On Fri, 2019-10-25 at 08:22 +0200, Robert Hartung wrote:  
>  > > Hi José,
>  > > hrmpf. Should've looked again into the list of PRs, was a while ago
>  > > ;)
>  > > Will contribute with reviewing and testing.
>  > > Regards
>  > > Robert
>  > > 
>  > > On 24.10.19 16:26, José Alamos wrote:  
>  > > > Hi Robert,
>  > > > 
>  > > > Note there's already a PR for this radio [1].
>  > > > It's using 2 `netdev_ieee802154` for handling each radio.
>  > > > 
>  > > > Maybe you can sync there and contribute with feedback and testing.
>  > > > 
>  > > > Best,
>  > > > José
>  > > > 
>  > > > [1]: https://github.com/RIOT-OS/RIOT/pull/12128
>  > > > On Thu, 2019-10-24 at 16:20 +0200, Robert Hartung wrote:  
>  > > > > Hello fellow developers,
>  > > > > 
>  > > > > I am targeting this question to developers here first before
>  > > > > opening
>  > > > > any
>  > > > > issues. We are looking to implement a driver for the at86rf215.
>  > > > > The
>  > > > > challenge here is, that we cannot fit it into the existing
>  > > > > at86rf2xx
>  > > > > driver for a simple reason: It has two interfaces for Sub-Ghz and
>  > > > > 2.4Ghz
>  > > > > communication, but shares a single interrupt pin. My idea was to
>  > > > > have
>  > > > > a
>  > > > > single device, which has multiple communication interfaces. From
>  > > > > what
>  > > > > I've seen and researched about netdev, this is not possible, as a
>  > > > > netdev
>  > > > > has a driver, which consists of the callbacks, including the isr
>  > > > > handler.
>  > > > > 
>  > > > > The Idea would then be to have a meta device for the interrupt
>  > > > > handling
>  > > > > and then calling the actual isr for the radio that the interrupt
>  > > > > was
>  > > > > ment for. Any one having a similar problem or any ideas how to
>  > > > > solve
>  > > > > this issue?
>  > > > > 
>  > > > > Best Regards
>  > > > > Robert
>  > > > >   
>  > > > 
>  > > > ___
>  > > > devel mailing list
>  > > > devel@riot-os.org
>  > > > https://lists.riot-os.org/mailman/listinfo/devel
>  > > >   
>  > 
>  > ___
>  > devel mailing list
>  > devel@riot-os.org
>  > https://lists.riot-os.org/mailman/listinfo/devel  
> 



pgpQJiYmoGzhV.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] Shell Commands

2019-09-23 Thread Marian Buschsieweke
Hi,

If I remember correctly stdio in RIOT is not buffering at all, but pyterm is 
buffering until \n. You might solve the issue by passing an additional flag to 
pyterm or using a different terminal.

Kind regards,
Marian

Am 23. September 2019 11:55:51 MESZ schrieb smlng :
>Welcome Rik!
>
>without seeing you actual code it's hard to debug, could you share the
>code snippet for
>you shell-command and loop? From what you write: remember to close
>everything you want
>to printf with '\n', bc the shell is looking for this before it prints
>anything.
>
>It should look like this:
>
>while (run) {
> printf("in the loop\n");
> xtimer_sleep(1);
>}
>
>If you forget the '\n' the shell will buffer the string.
>
>Best,
>  Sebastian
>
>> On 22. Sep 2019, at 22:26, Rik Gene  wrote:
>> 
>> Dear All,
>> 
>> I'm writing a shell command that loops until a button press
>terminates it through an interrupt callback. The loop delay is set by
>xtimer.
>> 
>> My printf outputs inside the loop seem to be buffered in the
>background and only ends up on screen when the command returns.
>> 
>> How can I get it to print on every loop iteration while the shell
>command is still executing?
>> 
>> Using RTT / Linux / pyterm / NRF52.
>> 
>> Cheers,
>> Rik
>> ___
>> devel mailing list
>> devel@riot-os.org
>> https://lists.riot-os.org/mailman/listinfo/devel
>
>___
>devel mailing list
>devel@riot-os.org
>https://lists.riot-os.org/mailman/listinfo/devel

-- 
Via phone___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


[riot-devel] Are there users of avsextrem?

2019-09-16 Thread Marian Buschsieweke
Hi,

are there still users of the board avsextrem? I'd like to create a PR to drop
support for it. I believe this is again a board with no users left.

Kind regards,
Marian


pgpaeW6SpGQ1y.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] tests compiler policy

2019-09-10 Thread Marian Buschsieweke
Hi,

I'd personally go the vanilla version, but not for philosophical reasons. My
reasons would be:

- More consistent to other toolchains (no TI specific stuff)
- Does not rely on TI for future updates (in case TI looses interest in
  maintaining their on version)

So +1 for vanilla GCC.

Kind regards,
Marian

On Tue, 10 Sep 2019 09:12:01 +0200
Gero Müller  wrote:

> Hi!
> 
> I am about to update the Dockerfile for testing to include a new version
> of msp430-gcc.
> We have the choice between a binary version (gcc 8) provded by TI:
> 
>  
> http://software-dl.ti.com/msp430/msp430_public_sw/mcu/msp430/MSPGCC/latest/index_FDS.html
> 
> and the vanilla upstream gcc 9.2 release.
> I have no idea about the performance / feature differences, but I use
> the vanilla version and it works fine.
> 
> Which one would better match the RIOT philosophy?
> 
> Cheers
> Gero
> 



pgpIDKrLzgfr5.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


[riot-devel] dropping board support for jiminy-mega256rfr2

2019-09-09 Thread Marian Buschsieweke
Hi,

I just opened a pull request to drop the jiminy-mega256rfr2 [1]. If you want
this board to remain supported, please drop a line there.

# Reasoning

## Inclusion policy

During the maintainer assembly in Helsinki I understood that our inclusion
policy is that a board needs to fulfill at least one of the following
conditions to get merged:

1. It is actively maintained
2. It has a significant user base
3. It is an off-the-shelf product, so it might be easily obtained for
   testing

The Jiminy is clearly not an off the shelf product and it has not been
maintained for many month now. And I personally doubt that there are more than
two active users of the board. (I think it is safe to say that two users do not
count as significant user base.)

## Trade-off

Support for the board has real costs. It has delayed quite a number of PRs, as
no one stepped up to testing it. Without knowing how many active users the
board has (if any), it is unclear how big the benefit of having it upstream is.
But if my guess of two or less active users is correct, I'd say the costs of
maintenance outweighs the benefits

Kind regards,
Marian

PS: I marked the PR as RFC and as "needs more than one ACK", so the board will
not be deleted right away. But please make yourself heard within the next two
weeks if you want to speak out for keeping it.

[1]: https://github.com/RIOT-OS/RIOT/pull/12182


pgp4XRyLkxkfG.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] Issue with implementation of multiple boards in one directory

2019-08-26 Thread Marian Buschsieweke
Hi,

I think the question is what mapping you want to have between a board (the
physical hardware) and a RIOT board. If the mapping should be 1:1, than one
physical board could have different CPUs and configurations.

If you however agree with a 1:n mapping, than having different RIOT boards for
each configuration of a board is fine.

With the issues you presented in mind, I'd say a 1:n mapping might be the
solution with the least maintenance overhead, provided that `boards/common` is
heavily used to avoid duplication.

Kind regards,
Marian

On Mon, 26 Aug 2019 15:34:30 +0200
Bas Stottelaar  wrote:

> Hi Gaëten,
> 
> BOARD_MODULE is used by the Silicon Labs SLWSTK6000b because it is one board
> with many supported CPU daughter boards (different CPU, other radio configs).
> You would buy that board, but also need to buy this daughter board separately.
> 
> A similar approach is used by the EFM32 CPU family. The (incomplete) list of
> daughter boards is here:
> https://github.com/RIOT-OS/RIOT/blob/master/boards/slwstk6000b/modules.txt
> 
> Kind regards,
> Bas
> 
> > Op 26 aug. 2019 om 14:41 heeft Gaëtan Harter 
> > het volgende geschreven:
> > 
> > Dear RIOT developers,
> > 
> > as part of migrating CPU and CPU_MODEL definition to `Makefile.features` I
> > am facing custom handling of multiple boards in the same directory with
> > sometime creative handling but not a proper build system integration.
> > 
> > There are multiple issues with this:
> > * none of the alternative boards are ever be compiled in the CI
> > * when testing, giving multiple variables to specify a board is not really
> > handled
> > * building the separate boards is not handled with `BUILD_IN_DOCKER`
> > * custom non standard configuration variables that are not properly handled
> > in the whole build system
> > 
> > The current implementation are currently done through:
> > 
> > * Using a default `CPU_MODEL ?= default_model` however the `CPU_MODEL`
> > variable is not passed to docker
> >  * mulle (2 possibilities)
> >  * pba-d-01-kw2x (3 possibilities)
> >  * cc2538dk (no documentation but I think this one is just a mis-write and
> > not a wanted thing)
> > * Using a `STM32F103C8_FLASH_HACK` variable to just set a different
> > `CPU_MODEL`. It applies to `bluepill` and `blackpill` but is just
> > documented for the `bluepill`
> > * An undocumented `BOARD_MODULE` variable that triggers a wildcard then a
> > grep to find `CPU_MODEL` (yes it queries your filesystem for a static
> > mapping). Its generic with many possible values but only implemented for 2
> > "board modules"
> > 
> > I would like to know who really uses these boards modifications, and what
> > we should do with them. Why were they implemented in the first place
> > instead of having separate boards? Would have prevented having them merged?
> > 
> > Should we just split all these boards, it would currently result in 5 or 6
> > boards (depending on the blackpill).
> > 
> > Should we keep support for multiple boards together? if yes, can we limit
> > to only changing `CPU_MODEL` through this variable and remove the other
> > custom handling? Still knowing they would not be automatically tested by CI.
> > 
> > I am willing to take care of the required changes when a direction is
> > decided.
> > 
> > Regards,
> > Gaëtan - cladmi
> > ___
> > devel mailing list
> > devel@riot-os.org
> > https://lists.riot-os.org/mailman/listinfo/devel  



pgpBuYZdn2LCd.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] sched_active_thread is a volatile pointer, but volatile is ignored

2019-02-26 Thread Marian Buschsieweke
Hi everyone,

> I seem to remember that we tried making sched_active_thread non-volatile
> at some point, breaking things, but that has also been a long time ago.

I think the reason might be that the compiler reordered access to the variables.

See C99 standard section 5.1.3 §5:

> The least requirements on a conforming implementation are:
> —  At sequence points, volatile objects are stable in the sense that previous
>accesses are complete and subsequent accesses have not yet occurred.
> [...]

See also section 6.8 §4:

> [...] The end of a full expression is a sequence point.

So e.g. a call to `thread_yield_higher()` would be a sequence point, so all
memory accesses to variable declared `volatile` written before that call would
be actually executed before, and all accesses after would have to be executed
afterwards.

So regarding to the `volatile` variable the call to `thread_yield_higher()`
would be a memory barrier (but not regarding to every other variable). And this
is exactly why `volatile` solves a problem here.

I think getting rid of `volatile` and adding "proper" memory barriers to places
where context switches are supposed to happen will make the code more robust and
might allow the compiler to optimize the code better: More robust, because
some memory accesses near context switches might have just by luck not yet been
moved by the compiler to the wrong side of a context switch - there might be
non-volatile variables that are vulnerable to being moved. And better optimized
because `volatile` will blindly turn off all optimization while accessing the
affected variables, but memory barriers are more fine-grained.

`void __sync_synchronize(void)` placed just before and just after a context
change seems to be the reasonable thing to me. (Also `core/atomic_c11.c`
conveniently provides a reasonable fallback for compilers not having it - at
least as long as the CPU does not reorder commands. But I bet this is true for
most/all IoT class CPUs as of now.)

@Kees: Still interested in this conversion? Mind to take the lead? I'm happy to
help in any way I can.

Kind regards,
Marian

On Tue, 8 Jan 2019 16:10:27 +0100
Kaspar Schleiser  wrote:

> Hi Kees,
> 
> On 1/7/19 9:19 PM, Kees Bakker wrote:
> > My main concern is: who made it volatile in the first place?  
> 
> I did, almost a decade ago.
> 
> > And what was the reasoning behind it? Volatile is one of the least
> > understood properties of the C language (my personal opinion). I'm
> > hoping that the volatile was not just thrown in because it feels good
> > when doing threads. And in other places the volatile is ignored,
> > hopefully for a good reason (optimisation is _not_ a good reason).  
> IIRC the intention was so the IPC code would create read / write
> accesses whenever accessing fields of thread_t.
> 
> E.g.:
> 
> void msg_recv(...)
> {
>if (!sched_active_thread->waiters) {
> // platform specific macro to suspend thread
>}
> 
>thread_t *waiter = sched_active_thread->waiters;
> 
>// ...
> }
> 
> (or similar)
> 
> My understanding of volatile back then was that the compiler could,
> without volatile, assume that sched_active_thread->waiters equals NULL.
> 
> This was certainly a case of only (at most) half-understanding volatile,
> which then turned into "if it is not broken, don't fix it".
> 
> Nowadays such code is always guarded with disabled IRQs.
> 
> I seem to remember that we tried making sched_active_thread non-volatile
> at some point, breaking things, but that has also been a long time ago.
> 
> I'm all for removing the qualifier. But we do have to make sure to
> thoroughly test core/ on all platforms.
> 
> Kaspar
> ___
> devel mailing list
> devel@riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel



pgp9Ip5Xc1mdx.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


[riot-devel] Someone having access to a Thingy:52 or a RuuviTag?

2019-02-15 Thread Marian Buschsieweke
Hi everyone,

this PR #9407 [1] is waiting for someone to test it on a Thingy:52 or a
RuuviTag. You'll need a current development version of OpenOCD in $PATH. Testing
will be as simple as running

make BOARD=thingy52 PROGRAMMER=openocd flash
make BOARD=thingy52 term

in `examples/hello_world`.

It would be nice to have this PR tested on the last two remaining boards before
merging it into master.

Thank you very much!

Kind regards,
Marian

[1]: https://github.com/RIOT-OS/RIOT/pull/9407


pgpCVh1fj0a6U.pgp
Description: OpenPGP digital signature
___
devel mailing list
devel@riot-os.org
https://lists.riot-os.org/mailman/listinfo/devel


Re: [riot-devel] SAM Flash Read-While-Write

2019-01-27 Thread Marian Buschsieweke
Hi,

do you mind to create a pull request for it? This imho would make discussion of 
the code easier. You can add [WIP] in the title to highlight it's work in 
progress.

Cheers,
Marian

Am 27. Januar 2019 11:44:21 MEZ schrieb Federico Pellegrin :
>Hi,
>I did a mostly working (please see below for why mostly!) scratch of
>the implementation and put it here for now:
>https://github.com/fedepell/RIOT/commit/3b9de66ce0a9d722460c3b236ef67835df91cd90
>
>I tested it and it is working on my saml21-xpro board. I can write on
>the 8KBs of RWWEE memory and read it and see that it is still there
>after rebooting and stuff. I took care to make it compile also on
>saml11 (define names are different) although I cannot test this. As
>you will see the code is mostly a copy & paste of the standard
>flashpage, with a bunch of different defines used (for base adresses
>and registers).
>As I presumed in previous emails, it looks kinda ugly to me, but I'm
>not sure we could reuse totally the code without a major refactoring
>or by making many if-s in the common code (even asserts would depend
>on if we are using RWWEE or not) to keep them united (and therefore
>less performant). Please give it a look and let me know what do you
>think!
>
>
>As for "mostly working" what is still puzzling me is that the code
>seems to have some kind of alignment issue or so (although I take care
>of using aligned memory structures are required) since if I add/remove
>some statement it may then just hang or generate a hardfault. It is
>quite puzzling me since I tried to rule out all possibilities but
>cannot get it working. But maybe it's just that I'm stubbornly doing
>this from some time and I need to detach from it mentally for a while
>;) But since I have to go out in a while I wanted to commit it so if
>someone wants to give it a look in the meantime, mostly refering to
>the structural question of how to implement it, it is there on github.
>And maybe someone has a general suggestion about this maybe-alignment
>issue from previous experience!
>
>Thanks!
>
>Cheers,
>Federico
>
>
>
>
>
>
>Il giorno sab 26 gen 2019 alle ore 05:46 Federico Pellegrin
> ha scritto:
>>
>> Hi Dylan and Oleg,
>> Thanks for the feedback!
>>
>> Sorry I meant kilobyte indeed, but always write it wrong, both the
>k-s
>> (should be K I see now) and b-s ;)
>>
>> Regarding implementation:
>>
>> To make it interrupt driven I guess it would be quite more a radical
>> change, so I would look at least for now for a blocking one, which
>> basically makes it just an additional flash area no different than
>the
>> main one.
>>
>> About doing it similar to eeprom what I see, API wise, is that the
>> eeprom one you link (and eeprom in general) is not page based, while
>> the flash one (and also the RWWEE since the interface is the same) is
>> page based. So you need always to work with pages (erase especially).
>> So I would see it more indeed like the flashpage itself indeed.
>That's
>> why my doubts on having it separate or not, since mostly what changes
>> in the code would be the start offset, various limits now #defines
>> (such as FLASHPAGE_NUMOF) and some flags when accessing the CPU (ie
>> using NVMCTRL_CTRLA_CMD_RWWEEER instead of NVMCTRL_CTRLA_CMD_ER)
>>
>>
>> Cheers,
>> Federico
>>
>> Il giorno ven 25 gen 2019 alle ore 09:21 Dylan Laduranty
>>  ha scritto:
>> >
>> > Hello Federico,
>> >
>> > IIRC, this flash can be use as a virtual EEPROM. Maybe It would be
>better to write a eeprom driver (like STM32 [1]) rather than add it to
>the flashpage driver ?
>> > I also think this memory is 8KB for SAML21J18 (not 8Kb), which is a
>lot :)
>> >
>> > As a side note: SAML21 also have 8KB of Low Power SRAM in addition
>to its 32 KB of SRAM
>> >
>> > I've never played with these additional memories but I'll be happy
>to help.
>> >
>> > Cheers,
>> > Dylan
>> >
>> > [1]
>https://github.com/RIOT-OS/RIOT/blob/master/cpu/stm32_common/periph/eeprom.c
>> >
>> > Le mer. 23 janv. 2019 à 14:27, Federico Pellegrin
> a écrit :
>> >>
>> >> Hi,
>> >> Working on SAML21 based project it came to my attention also the
>> >> presence of an 8Kb additional flash that is called
>"Read-While-Write"
>> >> (or RWWEE in the datasheet) that, as far as I understood, can be
>> >> programmed and read in a very similar way to the normal flash.
>This is
>> >> currently not supported anyhow in RIOT as far as I see. This is
>also
>> >> present in other SAM chips (a grep on the RIOT sam0 common
>includes
>> >> tells me saml21, samd21 and samr30).
>> >>
>> >> Well it's just 8Kb (in saml21 case) but a customer is asking about
>it ;)
>> >>
>> >> Before starting to fiddle with code I was wondering if any of you
>> >> already saw this or had intention to work on or had problems with
>this
>> >> or any information would be great :)
>> >>
>> >> On the other side I would also be interested in an opinion if the
>> >> access to this facility should eventually be kept totally separate
>> >> from the current (with some