linux-kernel - Re: [PATCH v3 04/10] PCI: apple: Add initial hardware bring-up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9cf94456-d7ab-ac2b-fb12-019e40424793@marcan.st>
Date:   Fri, 17 Sep 2021 19:42:44 +0900
From:   Hector Martin <marcan@...can.st>
To:     Marc Zyngier <maz@...nel.org>, Sven Peter <sven@...npeter.dev>
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Stan Skowronek <stan@...ellium.com>,
        Mark Kettenis <kettenis@...nbsd.org>,
        Robin Murphy <Robin.Murphy@....com>, kernel-team@...roid.com
Subject: Re: [PATCH v3 04/10] PCI: apple: Add initial hardware bring-up

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@...can.st)
Public Key: https://mrcn.st/pub