[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bd6c90ff8c5bb176567cd07b761a51e691dfe0b4.camel@microchip.com>
Date: Thu, 9 Feb 2023 08:06:13 +0000
From: <Arun.Ramadoss@...rochip.com>
To: <o.rempel@...gutronix.de>
CC: <olteanv@...il.com>, <UNGLinuxDriver@...rochip.com>,
<linux-kernel@...r.kernel.org>, <vivien.didelot@...il.com>,
<andrew@...n.ch>, <f.fainelli@...il.com>, <kuba@...nel.org>,
<wei.fang@....com>, <edumazet@...gle.com>, <pabeni@...hat.com>,
<netdev@...r.kernel.org>, <Woojung.Huh@...rochip.com>,
<davem@...emloft.net>, <hkallweit1@...il.com>,
<kernel@...gutronix.de>
Subject: Re: [PATCH net-next v6 1/9] net: dsa: microchip: enable EEE support
Hi Oleksij,
On Thu, 2023-02-09 at 06:48 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Hi Arun,
>
> On Thu, Feb 09, 2023 at 04:07:11AM +0000, Arun.Ramadoss@...rochip.com
> wrote:
> > Hi Oleksij,
> > On Wed, 2023-02-08 at 11:32 +0100, Oleksij Rempel wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> > >
> > > Some of KSZ9477 family switches provides EEE support.
> >
> > nit: If you can elaborate what are the chip supports will be good.
>
> Do you mean list of supported chips or link speeds with EEE support?
Yes, since you mentioned some of KSZ9477, I thought it will be better
to mention the supported chips in commit description.
>
> > > To enable it, we
> > > just need to register set_mac_eee/set_mac_eee handlers and
> > > validate
> > > supported chip version and port.
> > >
> > > Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> > > Reviewed-by: Andrew Lunn <andrew@...n.ch>
> > > ---
> > > drivers/net/dsa/microchip/ksz_common.c | 65
> > > ++++++++++++++++++++++++++
> > > 1 file changed, 65 insertions(+)
> > >
> > > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > > b/drivers/net/dsa/microchip/ksz_common.c
> > > index 46becc0382d6..0a2d78253d17 100644
> > > --- a/drivers/net/dsa/microchip/ksz_common.c
> > > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > > @@ -2673,6 +2673,69 @@ static int ksz_max_mtu(struct dsa_switch
> > > *ds,
> > > int port)
> > > return -EOPNOTSUPP;
> > > }
> > >
> > > +static int ksz_get_mac_eee(struct dsa_switch *ds, int port,
> > > + struct ethtool_eee *e)
> > > +{
> > > + int ret;
> > > +
> > > + ret = ksz_validate_eee(ds, port);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* There is no documented control of Tx LPI
> > > configuration. */
> > > + e->tx_lpi_enabled = true;
> >
> > Blank line before comment will increase readability.
> >
> > > + /* There is no documented control of Tx LPI timer.
> > > According
> > > to tests
> > > + * Tx LPI timer seems to be set by default to minimal
> > > value.
> > > + */
> > > + e->tx_lpi_timer = 0;
> >
> > for lpi_enabled, you have used true and for lpi_timer you have used
> > 0.
> > It can be consistent either true/false or 1/0.
>
> tx_lpi_enabled has only on/off states. This is why i use bool values.
>
> tx_lpi_timer is a range in microseconds to re-enter LPI mode.
Got it. I overlooked the variable data type.
>
> Beside, tx_lpi_timer can be used to optimize EEE for some
> applications.
> For example do not start Low Power Idle for some usecs so latency
> will
> be reduced. Are there some secret register to configure this value?
I am not aware of it.
>
> Regards,
> Oleksij
> --
> Pengutronix
> e.K. | |
> Steuerwalder Str. 21 |
> http://www.pengutronix.de/e/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-
> 0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-
> 5555 |
Powered by blists - more mailing lists