linux-kernel - RE: [PATCH 2/9] atl1c: refine phy-register read/write function

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: <157393863283F442885425D2C454285623D5A948@nasanexd02a.na.qualcomm.com>
Date:	Fri, 20 Apr 2012 08:54:54 +0000
From:	"Huang, Xiong" <xiong@....qualcomm.com>
To:	Florian Fainelli <florian@...nwrt.org>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	qca-linux-team <qca-linux-team@...lcomm.com>,
	nic-devel <nic-devel@...lcomm.com>
Subject: RE: [PATCH 2/9] atl1c: refine phy-register read/write function



> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
> On Behalf Of Florian Fainelli
> Sent: Friday, April 20, 2012 15:41
> To: Huang, Xiong
> Cc: davem@...emloft.net; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; qca-linux-team; nic-devel
> Subject: Re: [PATCH 2/9] atl1c: refine phy-register read/write function
> 
> Hi Xiong,
> 
> Le 04/20/12 08:16, xiong a écrit :
> > phy register is read/write via MDIO control module --- that module
> > will be affected by the hibernate status, to access phy regs in hib
> > stutus, slow frequency clk must be selected.
> > To access phy extension register, the MDIO related registers are
> > refined/updated, a _core function is re-wroted for both regular PHY
> > regs and extension regs.
> > existing PHY r/w function is revised based on the _core.
> > PHY extension registers will be used for the comming patches.
> >
> > Signed-off-by: xiong<xiong@....qualcomm.com>
> > Tested-by: Liu David<dwliu@....qualcomm.com>
> > ---
> >   drivers/net/ethernet/atheros/atl1c/atl1c_hw.c   |  178
> +++++++++++++++++++----
> >   drivers/net/ethernet/atheros/atl1c/atl1c_hw.h   |   64 +++++---
> >   drivers/net/ethernet/atheros/atl1c/atl1c_main.c |    4 +-
> >   3 files changed, 189 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_hw.c
> > b/drivers/net/ethernet/atheros/atl1c/atl1c_hw.c
> > index bd1667c..6cbe78a 100644
> > --- a/drivers/net/ethernet/atheros/atl1c/atl1c_hw.c
> > +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_hw.c
> > @@ -277,65 +277,181 @@ void atl1c_hash_set(struct atl1c_hw *hw, u32
> hash_value)
> >   	AT_WRITE_REG_ARRAY(hw, REG_RX_HASH_TABLE, hash_reg, mta);
> >   }
> >
> > +
> > +void atl1c_stop_phy_polling(struct atl1c_hw *hw) {
> > +	u32 val;
> > +	int i;
> > +
> > +	if (hw->ctrl_flags&  ATL1C_FPGA_VERSION) {
> > +		AT_WRITE_REG(hw, REG_MDIO_CTRL, 0);
> > +		for (i = 0; i<  MDIO_MAX_AC_TO; i++) {
> > +			AT_READ_REG(hw, REG_MDIO_CTRL,&val);
> > +			if (0 == (val&  MDIO_CTRL_BUSY))
> > +				break;
> > +			udelay(10);
> > +		}
> > +	}
> > +}
> 
> Please reduce the identation by doing this:
> 
> if (!(hw->ctrl_flags & ALT1C_FPAG_VERSION))
> 	return;
> 
> that makes it much more readable.


No problem


> > +
> > +void atl1c_start_phy_polling(struct atl1c_hw *hw, u16 clk_sel) {
> > +	u32 val;
> > +	int i;
> > +
> > +	if (hw->ctrl_flags&  ATL1C_FPGA_VERSION) {
> > +		val = MDIO_CTRL_SPRES_PRMBL |
> > +			FIELDX(MDIO_CTRL_CLK_SEL, clk_sel) |
> > +			FIELDX(MDIO_CTRL_REG, 1) |
> > +			MDIO_CTRL_START |
> > +			MDIO_CTRL_OP_READ;
> > +		AT_WRITE_REG(hw, REG_MDIO_CTRL, val);
> > +		for (i = 0; i<  MDIO_MAX_AC_TO; i++) {
> > +			AT_READ_REG(hw, REG_MDIO_CTRL,&val);
> > +			if (0 == (val&  MDIO_CTRL_BUSY))
> > +				break;
> > +			udelay(10);
> > +		}
> > +		val |= MDIO_CTRL_AP_EN;
> > +		val&= ~MDIO_CTRL_START;
> > +		AT_WRITE_REG(hw, REG_MDIO_CTRL, val);
> > +		udelay(30);
> > +	}
> > +}
> 
> Seems like the for() busy-checking of the register could use their own function
> since it is used both in : atl1c_stop_phy_polling() and atl1c_start_phy_polling().
> 

Good suggestion. I will update.