Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10GEthernet PHY

From: Grant Likely
Date: Wed Oct 12 2011 - 22:25:45 EST


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 <david.daney@xxxxxxxxxx>
> ---
> .../devicetree/bindings/net/broadcom-bcm8706.txt | 28 +++
> drivers/net/phy/Kconfig | 5 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/bcm8706.c | 212 ++++++++++++++++++++
> include/linux/brcmphy.h | 1 +
> 5 files changed, 247 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> create mode 100644 drivers/net/phy/bcm8706.c
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> new file mode 100644
> index 0000000..d58bea9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> @@ -0,0 +1,28 @@
> +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.

What have we got so far in this regard for other phys and devices? I
don't think it necessary to put 'c45' in the property name. reg-init
should be sufficient. I'd like to hear from others if it would be
valuable to have a 'reg-init-sequence' property of the above format.

What does the device type cell indicate? Wouldn't the driver
naturally have the device id from the address of the cell?

> +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);

Otherwise the driver code seems to be fine.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/