All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Tristram.Ha@microchip.com, olteanv@gmail.com,
	linux@rempel-privat.de, Arun.Ramadoss@microchip.com,
	f.fainelli@gmail.com, andrew@lunn.ch, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, Woojung.Huh@microchip.com,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com
Subject: Re: [PATCH 2/2] net: dsa: microchip: Provide Module 4 KSZ9477 errata (DS80000754C)
Date: Wed, 30 Aug 2023 10:13:47 +0200	[thread overview]
Message-ID: <20230830101347.0f2aad73@wsk> (raw)
In-Reply-To: <20230830061626.GF31399@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 4313 bytes --]

Hi Oleksij,

> On Tue, Aug 29, 2023 at 10:23:26PM +0000, Tristram.Ha@microchip.com
> wrote:
> > > Yes, removing linkmod_and() will not should not help. I said, "The
> > > phydev->supported_eee should be cleared."
> > > 
> > > For example like this:
> > > --- a/drivers/net/phy/micrel.c
> > > +++ b/drivers/net/phy/micrel.c
> > > @@ -1400,6 +1400,7 @@ static int ksz9131_config_aneg(struct
> > > phy_device *phydev)
> > > 
> > >  static int ksz9477_get_features(struct phy_device *phydev)
> > >  {
> > > +       __ETHTOOL_DECLARE_LINK_MODE_MASK(zero) = { 0, };
> > >         int ret;
> > > 
> > >         ret = genphy_read_abilities(phydev);
> > > @@ -1413,7 +1414,7 @@ static int ksz9477_get_features(struct
> > > phy_device *phydev)
> > >          * KSZ8563R should have 100BaseTX/Full only.
> > >          */
> > >         linkmode_and(phydev->supported_eee, phydev->supported,
> > > -                    PHY_EEE_CAP1_FEATURES);
> > > +                    zero);
> > > 
> > >         return 0;
> > >  }
> > > 
> > > You will need to clear it only on KSZ9477 variants with this bug.
> > > This change is tested and it works on my KSZ9477-EVB.  
> >  
> > I think this is best for disabling EEE support.
> > I think before some customers asked for Ethtool EEE support not
> > because they want to use it but to disable it because of link
> > instability. KSZ9893/KSZ9563 switches should have the same problem.
> >  The EEE problem depends on the link partner.  For example my
> > laptop does not have problem even though EEE is enabled, although I
> > am not sure if EEE is really active.  The problem here is just
> > using two KSZ9477 switches and programming those PHY setup values
> > and enabling EEE will make the link unstable.  Management decided
> > to disable EEE feature to avoid customer support issues.
> > Another issue is EEE should be disabled when using 1588 PTP.
> >   
> 
> I'd like to share my thoughts on the concerns raised:
> 
> - KSZ9477 & EEE: Disabling EEE for the KSZ9477 makes sense,
> especially since the datasheet doesn't list it as supported.
> 

+1

I will prepare proper patch

> - EEE Support for KSZ9893 & KSZ9563: The datasheets for the KSZ9893
> indicate support for EEE. The erratum recommends making adjustments
> to the "transmit Refresh Time for Waketx to meet the IEEE Refresh
> Time specification" instead of turning it off completely. The same
> applies to KSZ9563. We should consider these adjustments.
> 
> - General Experience with KSZ Chips*: From my experience with these
> chips, irrespective of the EEE functionality, only the 1000/full and
> 100/full link modes tend to be stable enough.
> 
> - Responsibility to Customers and Certifications: As a part of the
> product supply chain, I believe in our commitment to honesty with our
> customers. When we select components for end products, we trust their
> listed features. For instance, designing for ENERGY STAR
> certification requires that all copper-based physical network ports
> in an LNE product must be compliant with IEEE 802.3 Clause 78, which
> mandates Energy Efficient Ethernet. If Microchip promotes a KSZ* chip
> with EEE as a feature, it becomes a cornerstone of the end product.
> Negating a key functionality, which was sold and then incorporated
> into the product, could risk non-compliance with certification
> criteria.
> 
> - Consistency in Product Representation: If the overarching company
> decision is to disable the EEE feature for all chips to preempt
> potential customer support issues, our product information should
> mirror this change. It's crucial that we neither misrepresent nor
> over-promise on features. Your deeper insights, given your
> involvement with Microchip's strategy, would be most enlightening.
> 

For the last two - at least the KSZ9477 Errata was clear about the
issue. Other (switch IC) vendors have even problems to admit that
something is wrong with their silicon design...

> Regards,
> Oleksij




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2023-08-30 19:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-24 15:48 [PATCH 1/2] net: dsa: microchip: KSZ9477: Provide functions to access MMD registers Lukasz Majewski
2023-08-24 15:48 ` [PATCH 2/2] net: dsa: microchip: Provide Module 4 KSZ9477 errata (DS80000754C) Lukasz Majewski
2023-08-24 15:54   ` Florian Fainelli
2023-08-25  7:42     ` Lukasz Majewski
2023-08-25  1:12   ` Tristram.Ha
2023-08-25  8:39     ` Lukasz Majewski
2023-08-25 15:26       ` Florian Fainelli
2023-08-25 18:48         ` Tristram.Ha
2023-08-26 10:49           ` Vladimir Oltean
2023-08-29  8:35             ` Lukasz Majewski
2023-08-29 10:18               ` Vladimir Oltean
2023-08-29 11:24                 ` Lukasz Majewski
2023-08-29 11:47                   ` Oleksij Rempel
2023-08-29 12:38                     ` Lukasz Majewski
2023-08-29 14:42                       ` Oleksij Rempel
2023-08-29 15:29                         ` Lukasz Majewski
2023-08-29 17:12                           ` Oleksij Rempel
2023-08-29 22:23                             ` Tristram.Ha
2023-08-30  6:16                               ` Oleksij Rempel
2023-08-30  8:13                                 ` Lukasz Majewski [this message]
2023-08-29 21:57             ` Tristram.Ha
2023-08-29 22:00               ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230830101347.0f2aad73@wsk \
    --to=lukma@denx.de \
    --cc=Arun.Ramadoss@microchip.com \
    --cc=Tristram.Ha@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=Woojung.Huh@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rempel-privat.de \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.