From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752539Ab1JMEpu (ORCPT ); Thu, 13 Oct 2011 00:45:50 -0400 Received: from mail-iy0-f174.google.com ([209.85.210.174]:45327 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750789Ab1JMEps (ORCPT ); Thu, 13 Oct 2011 00:45:48 -0400 Message-ID: <4E966CF7.7060901@gmail.com> Date: Wed, 12 Oct 2011 21:45:43 -0700 From: David Daney User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110428 Fedora/3.1.10-1.fc13 Thunderbird/3.1.10 MIME-Version: 1.0 To: Grant Likely CC: David Daney , devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, davem@davemloft.net, afleming@gmail.com Subject: Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY References: <1318442783-29058-1-git-send-email-david.daney@cavium.com> <1318442783-29058-4-git-send-email-david.daney@cavium.com> <20111013003129.GC14042@ponder.secretlab.ca> In-Reply-To: <20111013003129.GC14042@ponder.secretlab.ca> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/2011 05:31 PM, Grant Likely wrote: > On Wed, Oct 12, 2011 at 11:06:23AM -0700, David Daney wrote: >> Add a driver and PHY_ID number for said device. This is a 10Gig PHY >> which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so >> there is no autonegotiation. All we do is report link state and send >> interrupts when it changes. >> >> If the PHY has a device tree of_node associated with it, the >> "broadcom,c45-reg-init" property is used to supply register >> initialization values when config_init() is called. >> >> Signed-off-by: David Daney >> --- >> [...] >> +The Broadcom BCM8706 is a 10G Ethernet PHY. It has these bindings in >> +addition to the standard PHY bindings. >> + >> +Compatible: Should contain "broadcom,bcm8706" and >> + "ethernet-phy-ieee802.3-c45" >> + >> +Optional Properties: >> + >> +- broadcom,c45-reg-init : one of more sets of 4 cells. The first cell >> + is the device type, the second a register address, the third cell >> + contains a mask to be ANDed with the existing register value, and >> + the fourth cell is ORed with he result to yield the new register >> + value. > ... a mask value of '0' should also guarantee that the driver does not do a read before the write. The implementation does that, I will update the binding text to reflect this. > What have we got so far in this regard for other phys and devices? http://devicetree.org/Compatible%3Dmarvell,88e1149r This is basically the same thing adapted for the page select register specific to Marvell PHYs. > I > don't think it necessary to put 'c45' in the property name. reg-init > should be sufficient. 10M/100M/1G PHYs from different manufacturers and even within a single manufacturer have a wide variety of ways to multiplex many registers into the 5 bit addressing scheme allowed by clause 22. The Marvell scheme already implemented doesn't work for Broadcom. For clause 45, there are more address bits... > I'd like to hear from others if it would be > valuable to have a 'reg-init-sequence' property of the above format. A clause 45 specific property might work for *all* 10G PHYs, the same cannot be said for clause 22, hence my idea to put 'c45' in the name > What does the device type cell indicate? Wouldn't the driver > naturally have the device id from the address of the cell? > There are three portions to a clause 45 address: phy_id: Denoted by the "reg" property is a 5-bit value that identifies a particular PHY on the MDIO bus. device id: Really a sub-device within a given PHY, another 5-bit value contained in the first cell of the proposed register init sequence. Clause 45 defines several different standard device ids. register id: a 16-bit address that identifies a particular 16-bit register within the 'device' (or sub-device if you will. Does that answer your question? In theory we could compose the 5-bit device id and 16-bit register address into a single 32-bit cell in the init sequence property, but I chose to have them separate. >> +static int __init bcm8706_init(void) >> +{ >> + int ret; >> + >> + ret = phy_driver_register(&bcm8706_driver); >> + >> + return ret; >> +} >> +module_init(bcm8706_init); > or simply: > static int __init bcm8706_init(void) > { > return phy_driver_register(&bcm8706_driver); > } > module_init(bcm8706_init); > Yes, I will make that change. David Daney