On Fri, May 09, 2025 at 01:29:40PM +0300, Claudiu Beznea wrote:
> On 05.05.2025 14:26, Claudiu Beznea wrote:
> > On 01.05.2025 23:12, Bjorn Helgaas wrote:
> >> On Wed, Apr 30, 2025 at 01:32:33PM +0300, Claudiu wrote:
> >>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>
> >>> The Renesas RZ/G3S features a PCIe IP that complies with the PCI Express
> >>> Base Specification 4.0 and supports speeds of up to 5 GT/s. It functions
> >>> only as a root complex, with a single-lane (x1) configuration. The
> >>> controller includes Type 1 configuration registers, as well as IP
> >>> specific registers (called AXI registers) required for various adjustments.
> >>>
> >>> Other Renesas RZ SoCs (e.g., RZ/G3E, RZ/V2H) share the same AXI registers
> >>> but have both Root Complex and Endpoint capabilities. As a result, the PCIe
> >>> host driver can be reused for these variants with minimal adjustments.
> ...
> >>> +static void rzg3s_pcie_update_bits(void __iomem *base, u32 offset, u32 mask, u32 val)
> >>> +{
> >>> + u32 tmp;
> >>> +
> >>> + tmp = readl(base + offset);
> >>> + tmp &= ~mask;
> >>> + tmp |= val & mask;
> >>> + writel(tmp, base + offset);
> >>> +}
> >>
> >> Nothing rzg3s-specific here.
> >>
> >> I think u32p_replace_bits() (include/linux/bitfield.h) is basically this.
> >
> > I wasn't aware of it. I'll use it in the next version. Thank for pointing it.
>
> I look into changing to u32p_replace_bits() but this one needs a mask that
> can be verified at build time. It cannot be used directly in this function.
> Would you prefer me to replace all the calls to rzg3s_pcie_update_bits() with:
>
> tmp = readl();
> u32p_replace_bits(&tmp, ...)
> writel(tmp);
It seems like this is the prevailing way it's used.
You have ~20 calls, so it seems like it might be excessive to replace
each with readl/u32p_replace_bits/writel.
But maybe you could use u32p_replace_bits() inside
rzg3s_pcie_update_bits().
Bjorn
