netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Adrian Pop <popadrian1996@gmail.com>
Cc: Ido Schimmel <idosch@idosch.org>,
	netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
	jiri@mellanox.com, vadimp@mellanox.com, mlxsw@mellanox.com,
	Ido Schimmel <idosch@mellanox.com>
Subject: Re: [PATCH net-next 1/2] mlxsw: core: Add ethtool support for QSFP-DD transceivers
Date: Fri, 26 Jun 2020 21:07:16 +0200	[thread overview]
Message-ID: <20200626190716.GG535869@lunn.ch> (raw)
In-Reply-To: <CAL_jBfT93picGGoCNWQDY21pWmo3jffanhBzqVwm1kVbyEb4ow@mail.gmail.com>

On Fri, Jun 26, 2020 at 06:33:55PM +0100, Adrian Pop wrote:
> >
> > Is page 03h valid for a QSFP DD? Do we add pages 10h and 11h after
> > page 03h, or instead of? How do we indicate to user space what pages
> > of data have been passed to it?
> >
> >    Andrew
> 
> >From QSFP-DD CMIS Rev 4.0: "In particular, support of the Lower Memory
> and of Page 00h is required for all modules, including passive copper
> cables. These pages are therefore always implemented. Additional
> support for Pages 01h, 02h and bank 0 of Pages 10h and 11h is required
> for all paged memory modules."
> 
> According to the same document, page 0x03 contains "User EEPROM
> (NVRs)". Byte 142, bit 2, page 0x01 indicates if the user page 0x03
> was implemented. I did not find anything about page 0x02 (where the
> user EEPROM is stored) in the documentation for QSFP. I suppose it is
> always implemented? If we really want to have it so it is similar to
> QSFP, one could send 896 bytes (instead of 768) and just fill that
> portion with 0 in case it's not implemented. Note that this is just an
> idea, I'm not aware of best practices in cases like this.

It does seem common to only return a subset of the pages. This patch
for example. We need some clear rules to known what the kernel has
passed to user space, so we can both correctly interpret when a subset
has been passed, and also ensure all drivers are doing the same thing.

Currently we have:

 *       ----------   ----------   ---------    ------------
 *      | Upper    | | Upper    | | Upper    | | Upper      |
 *      | Page 00h | | Page 01h | | Page 02h | | Page 03h   |
 *      |          | |(Optional)| |(Optional)| | (Optional) |
 *      |          | |          | |          | |            |
 *      |          | |          | |          | |            |
 *      |    ID    | |   AST    | |  User    | |  For       |
 *      |  Fields  | |  Table   | | EEPROM   | |  Cable     |
 *      |          | |          | | Data     | | Assemblies |
 *      |          | |          | |          | |            |
 *      |          | |          | |          | |            |
 *      -----------  -----------   ----------  --------------

You are saying pages 00h, 01h and 02h are mandatory for QSPF-DD.  Page
03h is optional, but when present, it seems to contain what is page
02h above. Since the QSPF KAPI has it, QSPF-DD KAPI should also have
it. So i would suggest that pages 10h and 11h come after that.

If a driver wants to pass a subset, it can, but it must always trim
from the right, it cannot remove pages from the middle.

     Andrew

  reply	other threads:[~2020-06-26 19:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-26 14:47 [PATCH net-next 0/2] mlxsw: Add support for QSFP-DD transceiver type Ido Schimmel
2020-06-26 14:47 ` [PATCH net-next 1/2] mlxsw: core: Add ethtool support for QSFP-DD transceivers Ido Schimmel
2020-06-26 15:19   ` Andrew Lunn
2020-06-26 17:33     ` Adrian Pop
2020-06-26 19:07       ` Andrew Lunn [this message]
2020-06-26 22:13         ` Adrian Pop
2020-06-27 19:16           ` Ido Schimmel
2020-06-27 20:42             ` Adrian Pop
2020-06-28 11:55               ` Ido Schimmel
2020-06-28 12:27                 ` Adrian Pop
2020-06-30  0:21                 ` Andrew Lunn
2020-06-30  5:59                   ` Ido Schimmel
2020-06-30  9:37                     ` Vadim Pasternak
2020-06-30 13:00                     ` Andrew Lunn
2020-06-26 14:47 ` [PATCH net-next 2/2] mlxsw: core: Add support for temperature thresholds reading " Ido Schimmel
2020-06-26 14:53 ` [PATCH net-next 0/2] mlxsw: Add support for QSFP-DD transceiver type Ido Schimmel
2020-06-26 15:06   ` Andrew Lunn
2020-06-26 16:45     ` Adrian Pop

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=20200626190716.GG535869@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=idosch@idosch.org \
    --cc=idosch@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=kuba@kernel.org \
    --cc=mlxsw@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=popadrian1996@gmail.com \
    --cc=vadimp@mellanox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).