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

[Vinod Koul]

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