Re: [PATCH 1/2] leds: bcm6328: improve write and read functions

From: Pavel Machek
Date: Wed Feb 24 2021 - 16:46:18 EST


Hi!

> > Less code duplication? It is immediately clear that driver including
> > this is specific for brcm SoCs and would not try to work somewhere else?
>
> Yes maybe, there still does not feel like this deserves a shared header,
> but as long as the generated code is the same, why not.

Ok, it seems patch is not needed at all, after all?

> >
> >> As far as using _relaxed() this is absolutely correct because the bus
> >> logic that connects the CPU to its on-chip registers is non re-ordering
> >> non posted. That is true on the MIPS BE/LE and ARM when configured in LE
> >> or BE.
> >
> > If that's right on particular SoC, then _relaxed and normal versions
> > should be same; drivers still need to use normal versions, because
> > they may be running on different SoC...?
>
> readl() includes barriers and read_relaxed() does not, hence the
> difference in the name. There is no need to pay the price of a barrier
> when a) the bus architecture guarantees non re-ordering and posting and
> that statement is true on all the SoCs where these peripherals are used,
> and b) you have worked on fine tuning your drivers to get the most
> performance out of them.

Exactly. When bus architecture guarantees ... readl and read_relaxed
can be the same. That knowledge should be in architecture code, not in
drivers.

(But it does not matter much when the drivers are
architecture-specific).

> Given these peripherals can only be used on CPUs/SoCs made by Broadcom,
> any argument about portability to other SoCs is moot.

Ok.
Pavel

--
http://www.livejournal.com/~pavelmachek

Attachment: signature.asc
Description: PGP signature