Re: [PATCH 6/6] bmips: dma: drop redundant boot_cpu_type in arch_dma_sync

[Florian Fainelli <florian.fainelli@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

On 5/3/24 12:39, Christian Marangi wrote:
> On Fri, May 03, 2024 at 12:07:45PM -0700, Florian Fainelli wrote:
>> On 5/3/24 06:54, Christian Marangi wrote:
>>> Drop redundant boot_cpu_type in arch_sync_dma_for_cpu_all. These needs
>>> to be parsed only once and we can make use of bmips_rac_flush_disable to
>>> disable RAC flush on unsupported CPU.
>>>
>>> Set this value in bmips_cpu_setup for unsupported CPU to skip this
>>> redundant check every time DMA needs to be synced.
>>>
>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>
>> You are taking a shortcut that is reasonable in premise, but keying off the
>> bmips_rac_flush_disable is IMHO misleading. The RAC is enabled in the
>> BMIPS5000 and BMIPS5200 cores, just it does not need SW management unlike
>> earlier cores.
>>
>> If you renamed it to bmips_rac_flush_needed that might be more compelling.
>> Also, the other reason is that on a kernel that was configured for
>> supporting only BMIPS5000 and BMIPS5200 CPUs, I think we could get some
>> decent dead code elimination of the boot_cpu_type() check, which would not
>> be the case.
> 
> I was a bit confused by the last part, should I drop this or just rename
> the variable? Cause I think for kernel that support ONLY those CPU I
> guess the DMA function will be optimized anyway since the bool will
> always be false I guess?

I don't think it can be optimized, I would drop that patch. This is a 
hot-path and so any optimization is welcome.
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]