Re: [PATCH net-next] net: phy: meson-gxl: cleanup by defining the control registers

From: Andrew Lunn
Date: Tue Dec 05 2017 - 13:01:50 EST


On Tue, Dec 05, 2017 at 10:33:34AM +0100, Jerome Brunet wrote:
> From: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>
> Define registers and bits in meson-gxl PHY driver to make a bit
> more human friendly. No functional change


> static int meson_gxl_config_init(struct phy_device *phydev)
> {
> - /* Enable Analog and DSP register Bank access by */
> - phy_write(phydev, 0x14, 0x0000);
> - phy_write(phydev, 0x14, 0x0400);
> - phy_write(phydev, 0x14, 0x0000);
> - phy_write(phydev, 0x14, 0x0400);
> + int ret;
>
> - /* Write Analog register 23 */
> - phy_write(phydev, 0x17, 0x8E0D);
> - phy_write(phydev, 0x14, 0x4417);
> + /* Write PLL Configuration 1 */
> + ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
> + A6_CONFIG_PLLMULX4ICH |
> + FIELD_PREP(A6_CONFIG_PLLBUFITRIM, 7) |
> + A6_CONFIG_PLLRSTVCOPD |
> + A6_CONFIG_PLLCPOFF |
> + A6_CONFIG_PLL_SRC);
> + if (ret)
> + return ret;

This does not look like "No functional Change".

Please can you break this up. First make use of #defines.
That should be a clear "No functional Change".

Then a second patch adding a helper for banked registers?

Thanks
Andrew