Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 04/10] PCI: apple: Add initial hardware bring-up
To:     Marc Zyngier <maz@kernel.org>, Sven Peter <sven@svenpeter.dev>
Cc:     devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        Stan Skowronek <stan@corellium.com>,
        Mark Kettenis <kettenis@openbsd.org>,
        Robin Murphy <Robin.Murphy@arm.com>, kernel-team@android.com
References: <20210913182550.264165-1-maz@kernel.org>
 <20210913182550.264165-5-maz@kernel.org>
 <6eb53661-e11e-4634-9fa5-5e7e62d57a15@www.fastmail.com>
 <87lf3vblt6.wl-maz@kernel.org>
From:   Hector Martin <marcan@marcan.st>
Message-ID: <9cf94456-d7ab-ac2b-fb12-019e40424793@marcan.st>
Date:   Fri, 17 Sep 2021 19:42:44 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.12.0
MIME-Version: 1.0
In-Reply-To: <87lf3vblt6.wl-maz@kernel.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: es-ES
Content-Transfer-Encoding: 7bit
Precedence: bulk

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