[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160729081736.GB13798@lunn.ch>
Date: Fri, 29 Jul 2016 10:17:36 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Raju Lakkaraju <Raju.Lakkaraju@...rosemi.com>
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
> > +/* 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.
> 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.
Andrew
Powered by blists - more mailing lists