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