[PATCH v3 2/3] net: phy: ca_phy: Add driver for CAxxxx SoCs
Alex Nemirovsky
Alex.Nemirovsky at cortina-access.com
Tue Sep 29 20:17:26 CEST 2020
Tom,
Yes the intent is to allow overwrite the functionality elsewhere. I agree that the name is too generic and should be more unique to avoid name space conflicts.
Abbie,
Please Review and update per Tom’s request.
> On Sep 29, 2020, at 10:57 AM, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Sep 23, 2020 at 05:59:13PM -0700, Alex Nemirovsky wrote:
>> From: Abbie Chang <abbie.chang at cortina-access.com>
>>
>> Add phy driver support for MACs embedded inside Cortina Access SoCs
>>
>> Signed-off-by: Abbie Chang <abbie.chang at cortina-access.com>
>> Signed-off-by: Alex Nemirovsky <alex.nemirovsky at cortina-access.com>
>>
>> CC: Joe Hershberger <joe.hershberger at ni.com>
>> CC: Tom Rini <trini at konsulko.com>
>> CC: Aaron Tseng <aaron.tseng at cortina-access.com>
>>
>> Moved out PHY specific code out of Cortina NI Ethernet driver
>> and into a Cortina Access PHY interface driver
> [snip]
>> +__weak void __internal_phy_init(struct phy_device *phydev, int reset_phy)
>> +{
>> + u8 phy_addr;
>> + u16 data;
>> +
>> + /* should initialize 4 GPHYs at once */
>> + for (phy_addr = 4; phy_addr > 0; phy_addr--) {
>> + phydev->addr = phy_addr;
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0x0BC6);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x0053);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 18, 0x4003);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x7e01);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0x0A42);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0x0A40);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 0, 0x1140);
>> + }
>> +
>> + /* workaround to fix GPHY fail */
>> + for (phy_addr = 1; phy_addr < 5; phy_addr++) {
>> + /* Clear clock fail interrupt */
>> + phydev->addr = phy_addr;
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0xB90);
>> + data = phy_read(phydev, MDIO_DEVAD_NONE, 19);
>> + if (data == 0x10) {
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0xB90);
>> + data = phy_read(phydev, MDIO_DEVAD_NONE, 19);
>> + printf("%s: read again.\n", __func__);
>> + }
>> +
>> + printf("%s: phy_addr=%d, read register 19, value=0x%x\n",
>> + __func__, phy_addr, data);
>> + }
>> +}
>> +
>> +__weak void __external_phy_init(struct phy_device *phydev, int reset_phy)
>> +{
>> + u16 val;
>> +
>> + /* Disable response PHYAD=0 function of RTL8211 series PHY */
>> + /* REG31 write 0x0007, set to extension page */
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0x0007);
>> +
>> + /* REG30 write 0x002C, set to extension page 44 */
>> + phy_write(phydev, MDIO_DEVAD_NONE, 30, 0x002C);
>> +
>> + /*
>> + * REG27 write bit[2] = 0 disable response PHYAD = 0 function.
>> + * we should read REG27 and clear bit[2], and write back
>> + */
>> + val = phy_read(phydev, MDIO_DEVAD_NONE, 27);
>> + val &= ~(1 << 2);
>> + phy_write(phydev, MDIO_DEVAD_NONE, 27, val);
>> +
>> + /* REG31 write 0X0000, back to page0 */
>> + phy_write(phydev, MDIO_DEVAD_NONE, 31, 0x0000);
>> +}
>> +
>> +static int rtl8211_external_config(struct phy_device *phydev)
>> +{
>> + __external_phy_init(phydev, 0);
>> + printf("%s: initialize RTL8211 external done.\n", __func__);
>> + return 0;
>> +}
>> +
>> +static int rtl8211_internal_config(struct phy_device *phydev)
>> +{
>> + struct phy_device phydev_init;
>> +
>> + memcpy(&phydev_init, phydev, sizeof(struct phy_device));
>> + /* should initialize 4 GPHYs at once */
>> + __internal_phy_init(&phydev_init, 0);
>> + printf("%s: initialize RTL8211 internal done.\n", __func__);
>> + return 0;
>> +}
>
> Why do we have two weak functions (which have very generic names and
> could get overwritten by accident) and not just inlining them to the
> functions?
>
> --
> Tom
More information about the U-Boot
mailing list