Re: [PATCH 1/6] crypto: starfive - Add StarFive crypto engine support

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: JiaJie Ho <jiajie.ho@starfivetech.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S . Miller" <davem@davemloft.net>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>
Subject: Re: [PATCH 1/6] crypto: starfive - Add StarFive crypto engine support
Date: Thu, 1 Dec 2022 10:28:39 +0100	[thread overview]
Message-ID: <6028b265-bc8a-3a06-b17c-56aa772a4782@linaro.org> (raw)
In-Reply-To: <60ad0da0116044d3a1fe575e9904e22c@EXMBX068.cuchost.com>

On 01/12/2022 07:52, JiaJie Ho wrote:

>>> +
>>> +static inline u32 starfive_sec_read(struct starfive_sec_dev *sdev,
>>> +u32 offset) {
>>> +	return __raw_readl(sdev->io_base + offset);
>>
>> I don't think these read/write wrappers help anyhow...
>>
> 
> These wrappers are used by the crypto primitives in this patch series.
> I'll move these to subsequent patches when they are first used.
> 
> Thank you for spending time reviewing and providing helpful comments
> for this driver.
> 

Just drop the wrappers. I said they do not help and your answer "are
used" does not explain anything. If you insist on keeping them, please
explain what are the benefits except more code and more
indirections/layers making it more difficult to read?

Best regards,
Krzysztof