пт, 24 мая 2019 г. в 20:24, Heiner Kallweit <hkallwe...@gmail.com>:
>
> 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, ...)
The error shoud indicate that something is wrong with mdio bus. I.e.
write returned error. And it's more likely hardware bug which needs to
be fixed. Once I used to not print this error on each ifconfig
up/down.
Max.
>
> > + return ret;
> > + }
> > + }
> > +
> > /* Enable Interrupt output INT_OE in CFG3 register */
> > if (phy_interrupt_is_valid(phydev)) {
> > val = phy_read(phydev, DP83867_CFG3);
> >
>
--
Best regards,
Maxim Uvarov
