linux-kernel - Re: [PATCH v4 09/16] phy: qcom-qmp-pcie: add register init helper

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: <Y2ZljEGAdg9zehE7@matsya>
Date:   Sat, 5 Nov 2022 19:00:52 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Johan Hovold <johan@...nel.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 09/16] phy: qcom-qmp-pcie: add register init helper

On 05-11-22, 14:17, Johan Hovold wrote:
> On Sat, Nov 05, 2022 at 05:38:54PM +0530, Vinod Koul wrote:
> > On 28-10-22, 15:35, Johan Hovold wrote:

> > > +	qmp_pcie_configure(serdes, tbls->serdes, tbls->serdes_num);
> > 
> > We are tbls
> 
> Yeah, it's a separate function.
> 
> Note that qmp_pcie_configure_lane() above use 'tbl' too.
> 
> > > +
> > >  	qmp_pcie_configure_lane(tx, tbls->tx, tbls->tx_num, 1);
> > >  	qmp_pcie_configure_lane(rx, tbls->rx, tbls->rx_num, 1);
> > >  
> > > @@ -1848,15 +1843,6 @@ static void qmp_pcie_lanes_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_t
> > >  		qmp_pcie_configure_lane(tx2, tbls->tx, tbls->tx_num, 2);
> > >  		qmp_pcie_configure_lane(rx2, tbls->rx, tbls->rx_num, 2);
> > >  	}
> > > -}
> > > -
> > > -static void qmp_pcie_pcs_init(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tables *tbls)
> > > -{
> > > -	void __iomem *pcs = qmp->pcs;
> > > -	void __iomem *pcs_misc = qmp->pcs_misc;
> > > -
> > > -	if (!tbls)
> > > -		return;
> > >  
> > >  	qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num);
> > >  	qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num);
> > > @@ -1932,8 +1918,8 @@ static int qmp_pcie_power_on(struct phy *phy)
> > >  	else
> > >  		mode_tables = cfg->tables_ep;
> > >  
> > > -	qmp_pcie_serdes_init(qmp, &cfg->tables);
> > > -	qmp_pcie_serdes_init(qmp, mode_tables);
> > > +	qmp_pcie_init_registers(qmp, &cfg->tables);
> > > +	qmp_pcie_init_registers(qmp, mode_tables);
> > 
> > but here tables :(
> > 
> > Lets stick with either please, or if we have differentiation lets make
> > it real obvious
> 
> It's not uncommon to use shorter local identifiers and a more descriptive
> name in structures, but since the driver already used 'tbl' consistently
> before the recent addition of the aggregate tables structure, I can
> rename also those pointers so that we use 'tbl' and 'tbls' consistently
> throughout the driver.

Thanks that would be great. Lets stick to one convention throughout the
driver

-- 
~Vinod