netdev - Re: Microsemi VSC 8531/41 PHY 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160804091714.GA3786@microsemi.com>
Date:	Thu, 4 Aug 2016 14:47:15 +0530
From:	Nagaraju Lakkaraju <Raju.Lakkaraju@...rosemi.com>
To:	Andrew Lunn <andrew@...n.ch>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"f.fainelli@...il.com" <f.fainelli@...il.com>,
	Allan Nielsen <Allan.Nielsen@...rosemi.com>
Subject: Re: Microsemi VSC 8531/41 PHY Driver

On Fri, Jul 29, 2016 at 10:17:36AM +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> > > +/* RGMII Rx Clock delay value change with board lay-out */ static u8
> > > +rgmii_rx_clk_delay = RGMII_RX_CLK_DELAY_1_1_NS;
> >
> > Doesn't this stop you from having a board with two PHYs with different layouts? You should be getting this value from the device tree.
> >
> > Raju: As of now, RGMII Rx clock delay value should be 1.1 nsec as optimized/recommended value.
> > We tested on Beaglebone Black with VSC 8531 PHY.
> > We would like to provide new function to configure correct/require value based on PHY layouts
> > alone with other RGMII configuration parameters as part of our next implementation.
> 
> Please either do it properly now or hard code it as the default, and
> then later replace it with device tree, etc. We don't like to see half
> finished features.
> 

I accepted your review comment. I do hard code it as the default values.

> > What are you locking against?
> >
> > Raju: VSC 8531 has different PAGEs. Whenever MDC/MDIO access the PHY control registers,
> > first set the page number then read/write the register address. Default page should be Page 0.
> > When I want to access not default page register, I have to lock phy device access and change
> > the page number and register access as atomic operation.
> 
> I understand all that, which is why i asked, "what are you locking
> against?", not "why are you locking?" What are the other call paths? I
> don't see you taking this lock anywhere else? Should you be? I would
> just like to see a comment which suggests you understand when this
> lock is needed, and when not.
> 

This is Read Modify Write (RMW) operation on register MSCC_PHY_RGMII_CNTL. This register in different page i.e. EXTENDED-2. I would like to execute all these operations in atomic.
I also use the mutex in different functions. But not in this patch.

>      Andrew