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
| ||
|
Message-ID: <20170524142543.GD26577@lunn.ch> Date: Wed, 24 May 2017 16:25:43 +0200 From: Andrew Lunn <andrew@...n.ch> To: Bogdan Purcareata <bogdan.purcareata@....com> Cc: "f.fainelli@...il.com" <f.fainelli@...il.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v2] drivers: phy: Add Cortina CS4340 driver > Yes, I can do that. However, I don't see a "phy" folder under Documentation/devicetree/bindings/net/. > Should I change the location to Documentation/devicetree/bindings/net/cortina.txt instead? Ah, sorry, my error. Yes, Documentation/devicetree/bindings/net/cortina.txt. > > I think there needs to be some explanation here. What exactly are you > > using to indicate link up? What does CORTINA_GPIO_GPIO_INTS mean? > > I can only assume it's the location of an interrupt status register. The Cortina PHYs go mostly undocumented, I've found an implementation in an old thread [1], that also did the job of programming the Cortina PHY microcode. I understand that microcode programming is now part of the bootloader. > > The register seems to provide the link status properly, and based on that the phydev is initialized with the only (currently) supported configuration, 10Gbps full duplex. > > I can change the register name to something more meaningful, though - e.g. CORTINA_LINK_STATUS. No, leave the name as is. The MIPS driver you gave a reference to seems to be using sensible names. My guess is, the boot loader is configuring the PHY to generate an interrupt on link up via one of its GPIO lines. And this code is reading that GPIO status. This is all very fragile, you are making a lot of assumptions. Have you tested pulling the cable out? And plugging it back in again? There is a chance this interrupt is "link change", not "link up". What i do like in that mips code is the probe function verifies the product ID. + /* + * CS4312 keeps its ID values in non-standard registers, make + * sure we are talking to what we think we are. + */ + id_lsb = cs4321_phy_read(phydev, CS4321_GLOBAL_CHIP_ID_LSB); + if (id_lsb < 0) { + ret = id_lsb; + goto err; + } + + id_msb = cs4321_phy_read(phydev, CS4321_GLOBAL_CHIP_ID_MSB); + if (id_msb < 0) { + ret = id_msb; + goto err; + } + + if (id_lsb != 0x23E5 || id_msb != 0x1002) { + ret = -ENODEV; + goto err; + } You should do that as well. Andrew
Powered by blists - more mailing lists