All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Don Bollinger" <don@thebollingers.org>
To: "'Andrew Lunn'" <andrew@lunn.ch>
Cc: "'Moshe Shemesh'" <moshe@nvidia.com>,
	"'David S. Miller'" <davem@davemloft.net>,
	"'Jakub Kicinski'" <kuba@kernel.org>,
	"'Adrian Pop'" <pop.adrian61@gmail.com>,
	"'Michal Kubecek'" <mkubecek@suse.cz>, <netdev@vger.kernel.org>,
	"'Vladyslav Tarasiuk'" <vladyslavt@nvidia.com>,
	<don@thebollingers.org>
Subject: RE: [RFC PATCH V4 net-next 1/5] ethtool: Allow network drivers to dump arbitrary EEPROM data
Date: Tue, 23 Mar 2021 10:47:05 -0700	[thread overview]
Message-ID: <008901d7200c$8a59db40$9f0d91c0$@thebollingers.org> (raw)
In-Reply-To: <YFlMjO4ZMBCcJqQ7@lunn.ch>

> > > I don't even see a need for this. The offset should be within one
> > > 1/2
> > page, of
> > > one bank. So offset >= 0 and <= 127. Length is also > 0 and
> > > <- 127. And offset+length is <= 127.
> >
> > I like the clean approach, but...   How do you request low memory?
> 
> Duh!
> 
> I got my conditions wrong. Too focused on 1/2 pages to think that two of
> them makes one page!
> 
> Lets try again:
> 
> offset < 256
> 0 < len < 128

Actually 0 < len <= 128.  Length of 128 is not only legal, but very common.
"Read the whole 1/2 page block".

> 
> if (offset < 128)
>    offset + len < 128

Again, offset + len <= 128

> else
>    offset + len < 256

offset + len <= 256

> 
> Does that look better?
> 
> Reading bytes from the lower 1/2 of page 0 should give the same data as
> reading data from the lower 1/2 of page 42. So we can allow that, but
don't
> be too surprised when an SFP gets it wrong and gives you rubbish. I would

The spec is clear that the lower half is the same for all pages.  If the SFP
gives you rubbish you should throw the device in the rubbish.

> suggest ethtool(1) never actually does read from the lower 1/2 of any page
> other than 0.

I agree, despite my previous comment.  While the spec is clear that should
work, I believe virtually all such instances are bugs not yet discovered.

And, note that the legacy API provides no way to access lower memory from
any page but 0.  There's just no syntax for it.  Not that we care about
legacy :-).

> 
> And i agree about documentation. I would suggest a comment in
> ethtool_netlink.h, and the RST documentation.
> 
> 		   Andrew

Don



  reply	other threads:[~2021-03-23 17:48 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-22 17:11 [RFC PATCH V4 net-next 0/5] ethtool: Extend module EEPROM dump API Moshe Shemesh
2021-03-22 17:11 ` [RFC PATCH V4 net-next 1/5] ethtool: Allow network drivers to dump arbitrary EEPROM data Moshe Shemesh
2021-03-22 18:17   ` Don Bollinger
2021-03-23  0:27     ` Andrew Lunn
2021-03-23  1:23       ` Don Bollinger
2021-03-23  2:03         ` Andrew Lunn
2021-03-23 17:47           ` Don Bollinger [this message]
2021-03-23 22:17             ` Andrew Lunn
2021-03-24 10:14             ` Moshe Shemesh
2021-03-24 10:03       ` Moshe Shemesh
2021-03-24 12:15         ` Andrew Lunn
2021-03-23  0:54   ` Andrew Lunn
2021-03-24 10:05     ` Moshe Shemesh
2021-03-22 17:11 ` [RFC PATCH V4 net-next 2/5] net/mlx5: Refactor module EEPROM query Moshe Shemesh
2021-03-22 17:11 ` [RFC PATCH V4 net-next 3/5] net/mlx5: Implement get_module_eeprom_by_page() Moshe Shemesh
2021-03-22 17:11 ` [RFC PATCH V4 net-next 4/5] net/mlx5: Add support for DSFP module EEPROM dumps Moshe Shemesh
2021-03-22 17:11 ` [RFC PATCH V4 net-next 5/5] ethtool: Add fallback to get_module_eeprom from netlink command Moshe Shemesh

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='008901d7200c$8a59db40$9f0d91c0$@thebollingers.org' \
    --to=don@thebollingers.org \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=mkubecek@suse.cz \
    --cc=moshe@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pop.adrian61@gmail.com \
    --cc=vladyslavt@nvidia.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.