Messages in this thread | | | Subject | Re: [PATCH v3 04/10] PCI: apple: Add initial hardware bring-up | From | Hector Martin <> | Date | Fri, 17 Sep 2021 19:42:44 +0900 |
| |
On 17/09/2021 18.20, Marc Zyngier wrote: > On Mon, 13 Sep 2021 21:48:47 +0100, >> On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: >>> +static inline void rmwl(u32 clr, u32 set, void __iomem *addr) >>> +{ >>> + writel_relaxed((readl_relaxed(addr) & ~clr) | set, addr); >>> +} >> >> This helper is a bit strange, especially since it's always only used >> with either clr != 0 or set != 0 but never (clr = 0 and set = 0) afaict. >> Maybe create two instead for setting and clearing bits? > > That's indeed nicer, and it makes the code more readable.
This seems to be a pattern in Corellium code; the cpufreq code is the same and I honestly found it quite hard to read when used for simple set or clear operations. I find set32/clear32 style helpers much more readable, so it's probably a good idea to make this change any time we upstream stuff derived from their tree.
-- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
| |