linux-kernel - Re: [PATCH net-next v3 01/10] net: wwan: tmi: Add PCIe core

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: <d6f13d66a5ab0224f2ae424a0645d4cf29c2752b.camel@mediatek.com>
Date:   Fri, 24 Feb 2023 07:39:33 +0000
From:   Yanchao Yang (杨彦超) 
        <Yanchao.Yang@...iatek.com>
To:     "kuba@...nel.org" <kuba@...nel.org>
CC:     Chris Feng (冯保林) 
        <Chris.Feng@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Mingliang Xu (徐明亮) 
        <mingliang.xu@...iatek.com>,
        Min Dong (董敏) <min.dong@...iatek.com>,
        "linuxwwan@...el.com" <linuxwwan@...el.com>,
        "m.chetan.kumar@...el.com" <m.chetan.kumar@...el.com>,
        Liang Lu (吕亮) <liang.lu@...iatek.com>,
        Haijun Liu (刘海军) 
        <haijun.liu@...iatek.com>,
        Haozhe Chang (常浩哲) 
        <Haozhe.Chang@...iatek.com>,
        Hua Yang (杨华) <Hua.Yang@...iatek.com>,
        "ryazanov.s.a@...il.com" <ryazanov.s.a@...il.com>,
        Xiayu Zhang (张夏宇) 
        <Xiayu.Zhang@...iatek.com>,
        "loic.poulain@...aro.org" <loic.poulain@...aro.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        Ting Wang (王挺) <ting.wang@...iatek.com>,
        "johannes@...solutions.net" <johannes@...solutions.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Aiden Wang (王咏麒) 
        <Aiden.Wang@...iatek.com>,
        Felix Chen (陈非) <Felix.Chen@...iatek.com>,
        Lambert Wang (王伟) 
        <Lambert.Wang@...iatek.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Mingchuang Qiao (乔明闯) 
        <Mingchuang.Qiao@...iatek.com>,
        Guohao Zhang (张国豪) 
        <Guohao.Zhang@...iatek.com>
Subject: Re: [PATCH net-next v3 01/10] net: wwan: tmi: Add PCIe core

Hi Jakub,
On Thu, 2023-02-16 at 20:50 +0800, Yanchao Yang wrote:
> > 
> Hi Jakub,
> 
> On Tue, 2023-02-14 at 20:22 -0800, Jakub Kicinski wrote:
> > On Sat, 11 Feb 2023 16:37:23 +0800 Yanchao Yang wrote:
> > > +ccflags-y += -I$(srctree)/$(src)/
> > > +ccflags-y += -I$(srctree)/$(src)/pcie/
> > 
> > Do you really need these flags?
> 
> We will check and update if it's really redundant soon.
Update test result.
Both flags are deleted, then run the make command with 
"build in" and "build module" on a separate kernel tree. Both suffer
the same build error.
“drivers/net/wan/mediatek/pcie/mtk_pci.c:16:10: fatal error: mtk_fsm.h:
No such file or directory
 #include "mtk_fsm.h""
The reason is that all files are not placed in the same folder. The
driver named TMI needs a child folder, then needs these flags.

Any ideas or comments for this? Please help share it at your
convenience.

> > > +{
> > > +	struct pci_dev *pdev = to_pci_dev(mdev->dev);
> > > +	int irq_cnt;
> > > +
> > > +	irq_cnt = pci_alloc_irq_vectors(pdev, MTK_IRQ_CNT_MIN,
> > > max_irq_cnt, irq_type);
> > > +	mdev->msi_nvecs = irq_cnt;
> > > +
> > > +	if (irq_cnt < MTK_IRQ_CNT_MIN) {
> > > +		dev_err(mdev->dev,
> > > +			"Unable to alloc pci irq vectors. ret=%d
> > > maxirqcnt=%d irqtype=0x%x\n",
> > > +			irq_cnt, max_irq_cnt, irq_type);
> > > +		return -EFAULT;
> > > +	}
> > > +
> > > +	return mtk_pci_request_irq_msix(mdev, irq_cnt);
> > > +}
> > > +
> > > +static void mtk_pci_free_irq(struct mtk_md_dev *mdev)
> > > +{
> > > +	struct pci_dev *pdev = to_pci_dev(mdev->dev);
> > > +	struct mtk_pci_priv *priv = mdev->hw_priv;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < priv->irq_cnt; i++)
> > > +		pci_free_irq(pdev, i, &priv->irq_desc[i]);
> > > +
> > > +	pci_free_irq_vectors(pdev);
> > > +}
> > > +
> > > +static void mtk_pci_save_state(struct mtk_md_dev *mdev)
> > > +{
> > > +	struct pci_dev *pdev = to_pci_dev(mdev->dev);
> > > +	struct mtk_pci_priv *priv = mdev->hw_priv;
> > > +	int ltr, l1ss;
> > > +
> > > +	pci_save_state(pdev);
> > > +	/* save ltr */
> > > +	ltr = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
> > > +	if (ltr) {
> > > +		pci_read_config_word(pdev, ltr + PCI_LTR_MAX_SNOOP_LAT,
> > > +				     &priv->ltr_max_snoop_lat);
> > > +		pci_read_config_word(pdev, ltr +
> > > PCI_LTR_MAX_NOSNOOP_LAT,
> > > +				     &priv->ltr_max_nosnoop_lat);
> > > +	}
> > > +	/* save l1ss */
> > > +	l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
> > > +	if (l1ss) {
> > > +		pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1,
> > > &priv->l1ss_ctl1);
> > > +		pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL2,
> > > &priv->l1ss_ctl2);
> > > +	}
> > > +}
> > > +	dev_info(mdev->dev, "Probe done hw_ver=0x%x\n", mdev->hw_ver);
> > > +	return 0;
> > > +
> > > +err_save_state:
> > 
> > Labels should be named after action they perform, not where they
> > jump
> > from. Please fix this everywhere.
> 
> We can found similar samples in kernel codes, naming the label per
> where jump from…
> ex. pci-sysfs.c
> shall we apply this rule to our driver?
> I
> t's mandatory or nice to have.

Any ideas or comments for this? Please help share it at your
convenience.

Many thanks.
yanchao.yang