On 24.05.2019 12:25, Max Uvarov wrote:
> For support 10Mps sped in SGMII mode DP83867_10M_SGMII_RATE_ADAPT bit
> of DP83867_10M_SGMII_CFG register has to be cleared by software.
> That does not affect speeds 100 and 1000 so can be done on init.
>
> Signed-off-by: Max Uvarov <muva...@gmail.com>
> ---
> drivers/net/phy/dp83867.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index fd35131a0c39..afd31c516cc7 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -30,6 +30,7 @@
> #define DP83867_STRAP_STS1 0x006E
> #define DP83867_RGMIIDCTL 0x0086
> #define DP83867_IO_MUX_CFG 0x0170
> +#define DP83867_10M_SGMII_CFG 0x016F
>
> #define DP83867_SW_RESET BIT(15)
> #define DP83867_SW_RESTART BIT(14)
> @@ -74,6 +75,9 @@
> /* CFG4 bits */
> #define DP83867_CFG4_PORT_MIRROR_EN BIT(0)
>
> +/* 10M_SGMII_CFG bits */
> +#define DP83867_10M_SGMII_RATE_ADAPT BIT(7)
> +
> enum {
> DP83867_PORT_MIRROING_KEEP,
> DP83867_PORT_MIRROING_EN,
> @@ -277,6 +281,24 @@ static int dp83867_config_init(struct phy_device *phydev)
> DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL);
> }
>
> + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> + /* For support SPEED_10 in SGMII mode
> + * DP83867_10M_SGMII_RATE_ADAPT bit
> + * has to be cleared by software. That
> + * does not affect SPEED_100 and
> + * SPEED_1000.
> + */
> + val = phy_read_mmd(phydev, DP83867_DEVADDR,
> + DP83867_10M_SGMII_CFG);
> + val &= ~DP83867_10M_SGMII_RATE_ADAPT;
> + ret = phy_write_mmd(phydev, DP83867_DEVADDR,
> + DP83867_10M_SGMII_CFG, val);
This could be simplified by using phy_modify_mmd().
> + if (ret) {
> + WARN_ONCE(1, "dp83867: err DP83867_10M_SGMII_CFG\n");
This error message says more or less nothing. The context is visible in the
stack trace, so you can remove the message w/o losing anything.
As we're in the config_init callback, I don't think the "ONCE" version is
needed. So you could simply use WARN_ON(1). Typically just the errno is
returned w/o additional message, so you could also simply do:
return phy_modify_mmd(phydev, ...)
> + return ret;
> + }
> + }
> +
> /* Enable Interrupt output INT_OE in CFG3 register */
> if (phy_interrupt_is_valid(phydev)) {
> val = phy_read(phydev, DP83867_CFG3);
>
