linux-kernel - Re: [PATCH v2 01/13] PCI: dwc: Add dw_pcie_clear_and_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <02c3d0b2-e97e-47ce-b472-4525e8eff68a@cixtech.com>
Date: Thu, 19 Jun 2025 13:42:05 +0800
From: Hans Zhang <hans.zhang@...tech.com>
To: Frank Li <Frank.li@....com>, Hans Zhang <18255117159@....com>
Cc: lpieralisi@...nel.org, bhelgaas@...gle.com, mani@...nel.org,
 kwilczynski@...nel.org, robh@...nel.org, jingoohan1@...il.com,
 linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 01/13] PCI: dwc: Add dw_pcie_clear_and_set_dword() for
 register bit manipulation



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@....com>
>> ---
>>   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?


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
>>
>