Attention is currently required from: Arthur Heymans, Christian Walter, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Held, Jeff Daly, Johnny Lin, Julius Werner, Kapil Porwal, Martin L Roth, Subrata Banik, Tarun, Tim Chu, Vanessa Eusebio.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80193?usp=email )
Change subject: soc/intel: Use write{64,32,16,8}p and read{64,32,16,8}p
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Basically, I think when you're trying to call write32() with an integer address then in 99% of the cases that means you already made a mistake beforehand, and should've been keeping that address in a pointer variable instead.
Totally disagree. We often use constant addresses, so 99% seems exaggerated,
maybe 1%? And technically using pointers for MMIO is 100% wrong in C.
I do like the idea of type safety, though. Right now we don't have much. The
*p() functions allow any integer. The others allow any pointer. So yeah, with
one set of functions we could accidentally put in a number that isn't an address.
But with the other set one could accidentally use a real pointer to a real object.
So the argument works both ways.
How about something like this:
```
typedef struct __packed {
uint8_t __dont_care;
} *mmio_addr;
```
That's a type that we can do byte-wise arithmetic with. And the compiler
should always warn if we pass another type. The fact that we are using
pointers for MMIO would be hidden.
What I don't know is if we'd get different (potentially bloated) code
generated for `void *`/`uintptr_t`/`mmio_addr`.
--
To view, visit
https://review.coreboot.org/c/coreboot/+/80193?usp=email
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4022e3bbfacf35d0529e46dc82cf303dae9438e4
Gerrit-Change-Number: 80193
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas
ehaouas@noos.fr
Gerrit-Reviewer: Arthur Heymans
arthur@aheymans.xyz
Gerrit-Reviewer: Christian Walter
christian.walter@9elements.com
Gerrit-Reviewer: Dinesh Gehlot
digehlot@google.com
Gerrit-Reviewer: Eran Mitrani
mitrani@google.com
Gerrit-Reviewer: Eric Lai
ericllai@google.com
Gerrit-Reviewer: Jakub Czapiga
czapiga@google.com
Gerrit-Reviewer: Jeff Daly
jeffd@silicom-usa.com
Gerrit-Reviewer: Johnny Lin
Johnny_Lin@wiwynn.com
Gerrit-Reviewer: Kapil Porwal
kapilporwal@google.com
Gerrit-Reviewer: Subrata Banik
subratabanik@google.com
Gerrit-Reviewer: Tarun
tstuli@gmail.com
Gerrit-Reviewer: Tim Chu
Tim.Chu@quantatw.com
Gerrit-Reviewer: Vanessa Eusebio
vanessa.f.eusebio@intel.com
Gerrit-Reviewer: build bot (Jenkins)
no-reply@coreboot.org
Gerrit-CC: Felix Held
felix-coreboot@felixheld.de
Gerrit-CC: Julius Werner
jwerner@chromium.org
Gerrit-CC: Martin L Roth
gaumless@gmail.com
Gerrit-CC: Nico Huber
nico.h@gmx.de
Gerrit-Attention: Jeff Daly
jeffd@silicom-usa.com
Gerrit-Attention: Eran Mitrani
mitrani@google.com
Gerrit-Attention: Dinesh Gehlot
digehlot@google.com
Gerrit-Attention: Julius Werner
jwerner@chromium.org
Gerrit-Attention: Arthur Heymans
arthur@aheymans.xyz
Gerrit-Attention: Tarun
tstuli@gmail.com
Gerrit-Attention: Martin L Roth
gaumless@gmail.com
Gerrit-Attention: Subrata Banik
subratabanik@google.com
Gerrit-Attention: Johnny Lin
Johnny_Lin@wiwynn.com
Gerrit-Attention: Kapil Porwal
kapilporwal@google.com
Gerrit-Attention: Christian Walter
christian.walter@9elements.com
Gerrit-Attention: Vanessa Eusebio
vanessa.f.eusebio@intel.com
Gerrit-Attention: Felix Held
felix-coreboot@felixheld.de
Gerrit-Attention: Elyes Haouas
ehaouas@noos.fr
Gerrit-Attention: Tim Chu
Tim.Chu@quantatw.com
Gerrit-Comment-Date: Wed, 07 Feb 2024 20:03:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin L Roth
gaumless@gmail.com
Comment-In-Reply-To: Julius Werner
jwerner@chromium.org
Comment-In-Reply-To: Felix Held
felix-coreboot@felixheld.de
Gerrit-MessageType: comment