netdev - Re: [RFC] NET: PHY: adds driver for Lantiq PHY11G

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]
Date:	Sun, 22 May 2016 22:36:38 +0200
From:	Hauke Mehrtens <hauke@...ke-m.de>
To:	Mathias Kresin <m@...sin.me>, f.fainelli@...il.com
Cc:	alexander.stein@...tec-electronic.com, netdev@...r.kernel.org,
	andrew@...n.ch, john@...ozen.org, openwrt@...sin.me,
	hauke.mehrtens@...el.com
Subject: Re: [RFC] NET: PHY: adds driver for Lantiq PHY11G

On 05/22/2016 10:30 PM, Mathias Kresin wrote:
> Hi Hauke,
> 
> find my comments in-line.
> 
> Am 22.05.2016 um 21:33 schrieb Hauke Mehrtens:
>> Supports the Lantiq / Intel CHD 11G and 22E PHYs.
>> These PHYs are also named PEF 7061, PEF 7071, PEF 7072
>>
>> Signed-off-by: John Crispin <john@...ozen.org>
>> Signed-off-by: Hauke Mehrtens <hauke@...ke-m.de>
>> ---
>>
>> This is based on a driver from OpenWrt / LEDE. This is send as a RFC
>> because the merge window is open now and it adds a new driver. This
>> patch was cleaned up on request of Alexander.
>>
>>
>>   .../devicetree/bindings/phy/phy-lanitq.txt         | 216
>> +++++++++++++++++
> 
> Looks like a typo in the filename. lantiq != lanitq

Thanks for spotting this, I just copied it from OpenWrt. ;-)

> 
>>   drivers/net/phy/Kconfig                            |   6 +
>>   drivers/net/phy/Makefile                           |   1 +
>>   drivers/net/phy/lantiq.c                           | 269
>> +++++++++++++++++++++
>>   4 files changed, 492 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/phy/phy-lanitq.txt
>>   create mode 100644 drivers/net/phy/lantiq.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-lanitq.txt
>> b/Documentation/devicetree/bindings/phy/phy-lanitq.txt
>> new file mode 100644
>> index 0000000..d9746e8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-lanitq.txt
>> @@ -0,0 +1,216 @@
>> +Lanitq PHY binding
> 
> Same typo as mentioned above.
> 

Will change this too.

.....

>> +
>> +static void lantiq_gphy_of_reg_init(struct phy_device *phydev)
>> +{
>> +    struct device_node *node = phydev->mdio.dev.of_node;
>> +    u32 tmp;
>> +
>> +    if (!IS_ENABLED(CONFIG_OF_MDIO))
>> +        return;
>> +
>> +    /* store the led values if one was passed by the device tree */
>> +    if (!of_property_read_u32(node, "lantiq,ledch", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e0, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,ledcl", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e1, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led0h", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e2, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led0l", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e3, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led1h", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e4, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led1l", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e5, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led2h", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e6, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led2l", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e7, MDIO_MMD_VEND2, tmp);
>> +
>> +    /* The LED3 is only available in PEF 7072 package. */
>> +    if (!of_property_read_u32(node, "lantiq,led3h", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e8, MDIO_MMD_VEND2, tmp);
>> +
>> +    if (!of_property_read_u32(node, "lantiq,led3l", &tmp))
>> +        phy_write_mmd_indirect(phydev, 0x1e9, MDIO_MMD_VEND2, tmp);
>> +}
>> +
>> +static int lantiq_gphy_config_init(struct phy_device *phydev)
>> +{
>> +    int err;
>> +
>> +    /* Mask all interrupts */
>> +    err = phy_write(phydev, MII_VR9_11G_IMASK, 0);
>> +    if (err)
>> +        return err;
>> +
>> +    /* Clear all pending interrupts */
>> +    phy_read(phydev, MII_VR9_11G_ISTAT);
>> +
>> +    phy_write_mmd_indirect(phydev, 0x1e0, MDIO_MMD_VEND2, 0xc5);
>> +    phy_write_mmd_indirect(phydev, 0x1e1, MDIO_MMD_VEND2, 0x67);
> 
> Any specific reason why the complex functions are enabled by default?

This is the same configuration the vendor SDK uses.

>> +    phy_write_mmd_indirect(phydev, 0x1e2, MDIO_MMD_VEND2, 0x42);
>> +    phy_write_mmd_indirect(phydev, 0x1e3, MDIO_MMD_VEND2, 0x10);
>> +    phy_write_mmd_indirect(phydev, 0x1e4, MDIO_MMD_VEND2, 0x70);
>> +    phy_write_mmd_indirect(phydev, 0x1e5, MDIO_MMD_VEND2, 0x03);
>> +    phy_write_mmd_indirect(phydev, 0x1e6, MDIO_MMD_VEND2, 0x20);
>> +    phy_write_mmd_indirect(phydev, 0x1e7, MDIO_MMD_VEND2, 0x00);
>> +    phy_write_mmd_indirect(phydev, 0x1e8, MDIO_MMD_VEND2, 0x40);
>> +    phy_write_mmd_indirect(phydev, 0x1e9, MDIO_MMD_VEND2, 0x20);
> 
> I would suggest to use the same blink/permanent on configuration for all
> led pins (as it is in LEDE/OpenWrt):
> 
>     Constant On: 10/100/1000MBit
>     Blink Fast: None
>     Blink Slow: None
>     Pulse: TX/RX
> 
> I'm aware of only one CPE that uses more than one led for status
> indication. All other have a single led attached to any of the pins.
> 
> This way it's only required to change the default configuration via the
> device tree bindings for the minority of the devices.

Ok, I am not aware on how all the boards are looking like. If most of
the boards only use on led it makes sense to make that the default, I
will change that.

.....

Hauke


Download attachment "smime.p7s" of type "application/pkcs7-signature" (4233 bytes)