linux-kernel - Re: [PATCH v2 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host controller driver

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] [day] [month] [year] [list]

Message-ID: <ktpiiszfmtnvyh3yxchfqnpkfv43uxbke47vptexeg4tli2hmh@keifchvj44yj>
Date: Mon, 8 Sep 2025 12:55:34 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: zhangsenchuan <zhangsenchuan@...incomputing.com>
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kwilczynski@...nel.org, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	p.zabel@...gutronix.de, johan+linaro@...nel.org, quic_schintav@...cinc.com, 
	shradha.t@...sung.com, cassel@...nel.org, thippeswamy.havalige@....com, 
	mayank.rana@....qualcomm.com, inochiama@...il.com, ningyu@...incomputing.com, 
	linmin@...incomputing.com, pinkesh.vaghela@...fochips.com
Subject: Re: [PATCH v2 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host
 controller driver

On Thu, Sep 04, 2025 at 04:57:17PM GMT, zhangsenchuan wrote:
> Dear Manivannan
> 
> Thank you for your thorough review.Here are some of my clarifications and questions.
> Looking forward to your answer, Thank you very much.
> 
> > -----Original Messages-----
> > From: "Manivannan Sadhasivam" <mani@...nel.org>
> > Send time:Monday, 01/09/2025 14:40:41
> > To: zhangsenchuan@...incomputing.com
> > Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, linux-pci@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, p.zabel@...gutronix.de, johan+linaro@...nel.org, quic_schintav@...cinc.com, shradha.t@...sung.com, cassel@...nel.org, thippeswamy.havalige@....com, mayank.rana@....qualcomm.com, inochiama@...il.com, ningyu@...incomputing.com, linmin@...incomputing.com, pinkesh.vaghela@...fochips.com
> > Subject: Re: [PATCH v2 2/2] PCI: eic7700: Add Eswin eic7700 PCIe host controller driver
> 
> 
> > > +	/* config eswin vendor id and eic7700 device id */
> > > +	dw_pcie_writel_dbi(pci, PCIE_TYPE_DEV_VEND_ID, 0x20301fe1);
> > 
> > Does it need to be configured all the time?
> 
> Clarification：
> Our hardware initialization did not configure the device Id and vendor Id.
> Now, we can only rewrite the device Id and vendor Id in the code.
> 

Ok. Then mention it in the comment itself. Like,

	/*
	 * Configure ESWIN VID:DID for Root Port as the default values are
	 * invalid.
	 */

> > 
> > > +
> > > +	/* lane fix config, real driver NOT need, default x4 */
> > 
> > What do you mean by 'readl driver NOT need'?
> > 
> 
> Clarification：
> Sorry, this was added during the compatibility platform test. It is not needed for real devices. 
> I will remove it later.
> 
> > > +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
> > > +	val &= 0xffffff80;
> > > +	val |= 0x44;
> > > +	dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
> > > +
> > > +	val = dw_pcie_readl_dbi(pci, DEVICE_CONTROL_DEVICE_STATUS);
> > > +	val &= ~(0x7 << 5);
> > > +	val |= (0x2 << 5);
> > > +	dw_pcie_writel_dbi(pci, DEVICE_CONTROL_DEVICE_STATUS, val);
> > > +
> > > +	/*  config support 32 msi vectors */
> > > +	val = dw_pcie_readl_dbi(pci, PCIE_DSP_PF0_MSI_CAP);
> > > +	val &= ~PCIE_MSI_MULTIPLE_MSG_MASK;
> > > +	val |= PCIE_MSI_MULTIPLE_MSG_32;
> > > +	dw_pcie_writel_dbi(pci, PCIE_DSP_PF0_MSI_CAP, val);
> > > +
> > > +	/* disable msix cap */
> > 
> > Why? Hw doesn't support MSI-X but it advertises MSI-X capability?
> > 
> 
> I'm not quite sure what this comment means? Indeed, our hardware doesn't support MSI-X.

So it advertises MSI-X in capability by mistake then. If so, do you think it is
going to be applicable for future revisions of the controller also? I believe
this is a kind of hw bug.

Usually, these kind of issues are fixed in future revisions of the SoC. So I was
checking if you intend to clear it for all SoCs in the future or not. Otherwise,
you may set a flag and clear it conditionally.

> We can't disable the MSI-X capability using the PCIE_NEXT_CAP_PTR register? Then which 
> register is needed to disable the MSI-X capability?
> 

No, my question was not about *how to clear MSI cap*.

- Mani

-- 
மணிவண்ணன் சதாசிவம்