On Thu, Jun 19, 2025 at 01:42:05PM +0800, Hans Zhang wrote:
>
>
> On 2025/6/19 12:29, Frank Li wrote:
> > EXTERNAL EMAIL
> >
> > On Wed, Jun 18, 2025 at 11:21:00PM +0800, Hans Zhang wrote:
> > > DesignWare PCIe controller drivers implement register bit manipulation
> > > through explicit read-modify-write sequences. These patterns appear
> > > repeatedly across multiple drivers with minor variations, creating
> > > code duplication and maintenance overhead.
> > >
> > > Implement dw_pcie_clear_and_set_dword() helper to encapsulate atomic
> > > register modification. The function reads the current register value,
> > > clears specified bits, sets new bits, and writes back the result in
> > > a single operation. This abstraction hides bitwise manipulation details
> > > while ensuring consistent behavior across all usage sites.
> > >
> > > Centralizing this logic reduces future maintenance effort when modifying
> > > register access patterns and minimizes the risk of implementation
> > > divergence between drivers.
> > >
> > > Signed-off-by: Hans Zhang <18255117159@xxxxxxx>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware.h | 11 +++++++++++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index ce9e18554e42..f401c144df0f 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -707,6 +707,17 @@ static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> > > }
> > >
> > > +static inline void dw_pcie_clear_and_set_dword(struct dw_pcie *pci, int pos,
> > > + u32 clear, u32 set)
> >
> > Can align with regmap_update_bits() argumnet?
> >
> > dw_pcie_update_dbi_bits()?
> >
>
> Dear Frank,
>
> Thank you for your reply.
>
>
> Personally, I think it should be the same as the following API. In this way,
> we can easily know the corresponding parameters and it also conforms to the
> usage habits of PCIe. What do you think?
You are right!
Frank
>
>
> drivers/pci/access.c
>
> int pcie_capability_clear_and_set_dword(struct pci_dev *dev, int pos,
> u32 clear, u32 set)
> {
> int ret;
> u32 val;
>
> ret = pcie_capability_read_dword(dev, pos, &val);
> if (ret)
> return ret;
>
> val &= ~clear;
> val |= set;
> return pcie_capability_write_dword(dev, pos, val);
> }
> EXPORT_SYMBOL(pcie_capability_clear_and_set_dword);
>
>
> void pci_clear_and_set_config_dword(const struct pci_dev *dev, int pos,
> u32 clear, u32 set)
> {
> u32 val;
>
> pci_read_config_dword(dev, pos, &val);
> val &= ~clear;
> val |= set;
> pci_write_config_dword(dev, pos, val);
> }
> EXPORT_SYMBOL(pci_clear_and_set_config_dword);
>
>
> Best regards,
> Hans
>
> > Frank
> >
> > > +{
> > > + u32 val;
> > > +
> > > + val = dw_pcie_readl_dbi(pci, pos);
> > > + val &= ~clear;
> > > + val |= set;
> > > + dw_pcie_writel_dbi(pci, pos, val);
> > > +}
> > > +
> > > static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> > > {
> > > u32 reg;
> > > --
> > > 2.25.1
> > >
> >
