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 <j...@phrozen.org> >> Signed-off-by: Hauke Mehrtens <ha...@hauke-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
smime.p7s
Description: S/MIME Cryptographic Signature
