cpu/sam0_common: avoid bitfield usage #20747

dylad · 2024-06-11T20:19:51Z

Contribution description

This PR removes all bitfields usage from cpu/sam0_common/ folder.
I decided to go with a commit per file to allow an easy revert in case something got terribly wrong. Nevertheless, CI should catch most of the issues early. However, I might get sleepy here and there so please, take a careful eyes at this.

/!\ Breakages are highly probable here /!\

Testing procedure

Triple careful review and CI should save us hopefully.

Issues/PRs references

#20457

riot-ci · 2024-06-11T20:22:29Z

Murdock results

✔️ PASSED

ccc155e cpu/sam0: remove bitfield usage in sdhc driver

Success	Failures	Total	Runtime
10178	0	10178	18m:06s

Artifacts

Documentation preview

cpu/sam0_common/periph/adc.c

cpu/sam0_common/periph/dma.c

kfessel

there are a lot of 1 rmw to 2 rmw cases.

some might need a temporary, can probably be some thing like

REG = (REG & ~Msk) | ((val << Pos) &Msk);

there might be a macro for this

cpu/sam0_common/include/periph_cpu_common.h

cpu/sam0_common/periph/adc.c

cpu/sam0_common/periph/dma.c

cpu/sam0_common/periph/rtc_rtt.c

cpu/sam0_common/periph/usbdev.c

cpu/sam0_common/periph/wdt.c

kfessel

Please do not merge until the doubling of the rmws is fixed

dylad · 2024-06-12T19:49:15Z

@kfessel I've pushed my fixup commits.

kfessel

👍

mostly good

cpu/sam0_common/periph/dma.c

cpu/sam0_common/periph/usbdev.c

cpu/sam0_common/periph/wdt.c

cpu/sam0_common/periph/usbdev.c

cpu/sam0_common/periph/uart.c

kfessel · 2024-06-13T14:02:42Z

cpu/sam0_common/periph/timer.c

+    do {
+        cmd = ((dev(tim)->CTRLBSET.reg & TC_CTRLBSET_CMD_Msk) >> TC_CTRLBSET_CMD_Pos);
+    } while(cmd == TC_CTRLBSET_CMD_READSYNC_Val);
+


might be faster but please ignore unless someone objdumped

Suggested change

do {

cmd = ((dev(tim)->CTRLBSET.reg & TC_CTRLBSET_CMD_Msk) >> TC_CTRLBSET_CMD_Pos);

} while(cmd == TC_CTRLBSET_CMD_READSYNC_Val);

do {

cmd = ((dev(tim)->CTRLBSET.reg & TC_CTRLBSET_CMD_Msk));

} while(cmd == TC_CTRLBSET_CMD_READSYNC_Val << TC_CTRLBSET_CMD_Pos);

cpu/sam0_common/periph/adc.c

dylad · 2024-06-14T07:44:46Z

@kfessel A new round of fixups has been pushed.

kfessel

it getting fewer

btw i'd like to ask: if you are doing tests?

cpu/sam0_common/periph/adc.c

cpu/sam0_common/periph/uart.c

kfessel · 2024-06-14T09:01:56Z

cpu/sam0_common/periph/adc.c

+    uint32_t ctrlc = dev->CTRLC.reg;
+    dev->CTRLC.reg = ((ctrlc & ~ADC_CTRLC_RESSEL_Msk) | ADC_CTRLC_RESSEL(res));
 #else
    /* Reset resolution bits in CTRLB */
-    dev->CTRLB.bit.RESSEL = res & 0x3;
+    uint32_t ctrlb = dev->CTRLB.reg;
+    dev->CTRLB.reg = ((ctrlb & ~ADC_CTRLB_RESSEL_Msk) | ADC_CTRLB_RESSEL(res));


the function is called configure but does not configure all the bits (and nothing else in this file does) this seem to me like bad code translated well.

you might ignore this comment your translation mimics the old behavior

cpu/sam0_common/include/periph_cpu_common.h

cpu/sam0_common/periph/adc.c

cpu/sam0_common/periph/uart.c

cpu/sam0_common/periph/usbdev.c

cpu/sam0_common/periph/wdt.c

cpu/sam0_common/sam0_sdhc/sdhc.c

dylad · 2024-06-14T10:21:42Z

@benpicco @kfessel Third round is done.

kfessel

looks fine testing?

obvious issues are resolved -- looks fine

dylad · 2024-06-14T10:48:04Z

looks fine testing?

I don't have hardware with me right now. I'll re-run tests on hardware this weekend if I manage to get some time.

dylad · 2024-06-17T19:39:07Z

I've run basics tests (timers, RTC, I2C, SPI, UART, USB, PWM) on saml11-xpro, saml21-xpro, samr21-xpro. I didn't spot any regression so far.

dylad · 2024-06-20T14:42:15Z

@benpicco @kfessel May I squash this one ?

kfessel · 2024-06-20T22:42:23Z

thanks for testing; please squash

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

dylad · 2024-06-21T07:46:31Z

Squashed !

dylad · 2024-06-24T11:57:59Z

Fixed a typo in the UART driver spotted by Murdock.
Let's see if it's happy now.

dylad · 2024-06-24T12:37:49Z

All green !

kfessel

translation looks correct,

For the test i trust @dylad.

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

dylad · 2024-06-25T14:14:26Z

Murdock spotted an inconsistency in PWM driver for SAMD20. That should be good now.

dylad · 2024-06-26T05:24:58Z

Thanks !

dylad requested review from benpicco and keestux as code owners June 11, 2024 20:19

github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Jun 11, 2024

dylad added CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 11, 2024

dylad removed the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Jun 12, 2024

dylad commented Jun 12, 2024

View reviewed changes

cpu/sam0_common/periph/adc.c Outdated Show resolved Hide resolved

cpu/sam0_common/periph/dma.c Outdated Show resolved Hide resolved

cpu/sam0_common/periph/dma.c Outdated Show resolved Hide resolved

kfessel reviewed Jun 12, 2024

View reviewed changes

kfessel previously requested changes Jun 12, 2024

View reviewed changes

kfessel reviewed Jun 13, 2024

View reviewed changes

kfessel reviewed Jun 14, 2024

View reviewed changes

benpicco reviewed Jun 14, 2024

View reviewed changes

kfessel reviewed Jun 14, 2024

View reviewed changes

dylad added 7 commits June 21, 2024 09:29

cpu/sam0/periph: remove bitfield usage in ADC driver

3c64901

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in DAC driver

292111a

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in DMAC driver

9d29813

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in GMAC driver

1140241

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in flashpage driver

ae7ed46

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in gpio driver

1e7167d

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in gpio_ll_irq driver

b2bbc4c

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

dylad force-pushed the pr/cpu/sam0/avoid_bitfield_usage branch from fa40dc1 to f32f08a Compare June 21, 2024 07:46

dylad force-pushed the pr/cpu/sam0/avoid_bitfield_usage branch from f32f08a to f1116f2 Compare June 24, 2024 11:57

kfessel approved these changes Jun 25, 2024

View reviewed changes

dylad added this pull request to the merge queue Jun 25, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 25, 2024

dylad added 11 commits June 25, 2024 16:13

cpu/sam0/periph: remove bitfield usage in PWM driver

38c65b9

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in trng driver

37b03d8

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in I2C driver

a66eebb

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in RTC/RTT driver

7c1e66b

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in SPI driver

925e98b

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in timer driver

6ccaca1

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in UART driver

f31ac8a

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in usbdev driver

707fecb

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0/periph: remove bitfield usage in watchdog driver

25a04e6

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0: remove bitfield usage in headers

04e4770

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

cpu/sam0: remove bitfield usage in sdhc driver

ccc155e

Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>

dylad force-pushed the pr/cpu/sam0/avoid_bitfield_usage branch from f1116f2 to ccc155e Compare June 25, 2024 14:13

dylad enabled auto-merge June 25, 2024 14:17

dylad added this pull request to the merge queue Jun 25, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 25, 2024

dylad added this pull request to the merge queue Jun 25, 2024

Merged via the queue into RIOT-OS:master with commit 8f645a9 Jun 26, 2024
25 checks passed

dylad deleted the pr/cpu/sam0/avoid_bitfield_usage branch June 26, 2024 05:24

mguetschow added this to the Release 2024.07 milestone Jul 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cpu/sam0_common: avoid bitfield usage #20747

cpu/sam0_common: avoid bitfield usage #20747

dylad commented Jun 11, 2024 •

edited

Loading

riot-ci commented Jun 11, 2024 •

edited

Loading

kfessel left a comment

kfessel left a comment

dylad commented Jun 12, 2024

kfessel left a comment

kfessel Jun 13, 2024

dylad commented Jun 14, 2024

kfessel left a comment

kfessel Jun 14, 2024

dylad commented Jun 14, 2024

kfessel left a comment

dylad commented Jun 14, 2024

dylad commented Jun 17, 2024

dylad commented Jun 20, 2024

kfessel commented Jun 20, 2024 •

edited

Loading

dylad commented Jun 21, 2024

dylad commented Jun 24, 2024

dylad commented Jun 24, 2024

kfessel left a comment

dylad commented Jun 25, 2024

dylad commented Jun 26, 2024

cpu/sam0_common: avoid bitfield usage #20747

cpu/sam0_common: avoid bitfield usage #20747

Conversation

dylad commented Jun 11, 2024 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

riot-ci commented Jun 11, 2024 • edited Loading

Murdock results

Artifacts

kfessel left a comment

Choose a reason for hiding this comment

kfessel left a comment

Choose a reason for hiding this comment

dylad commented Jun 12, 2024

kfessel left a comment

Choose a reason for hiding this comment

kfessel Jun 13, 2024

Choose a reason for hiding this comment

dylad commented Jun 14, 2024

kfessel left a comment

Choose a reason for hiding this comment

kfessel Jun 14, 2024

Choose a reason for hiding this comment

dylad commented Jun 14, 2024

kfessel left a comment

Choose a reason for hiding this comment

dylad commented Jun 14, 2024

dylad commented Jun 17, 2024

dylad commented Jun 20, 2024

kfessel commented Jun 20, 2024 • edited Loading

dylad commented Jun 21, 2024

dylad commented Jun 24, 2024

dylad commented Jun 24, 2024

kfessel left a comment

Choose a reason for hiding this comment

dylad commented Jun 25, 2024

dylad commented Jun 26, 2024

dylad commented Jun 11, 2024 •

edited

Loading

riot-ci commented Jun 11, 2024 •

edited

Loading

kfessel commented Jun 20, 2024 •

edited

Loading