linux-kernel - Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY

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: <20111013003129.GC14042@ponder.secretlab.ca>
Date:	Wed, 12 Oct 2011 18:31:29 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	David Daney <david.daney@...ium.com>
Cc:	devicetree-discuss@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, davem@...emloft.net, afleming@...il.com
Subject: Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G
 Ethernet PHY

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@...ium.com>
> ---
>  .../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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/