On 7/18/19 10:28 AM, Andrew Lunn wrote:
On Thu, Jul 18, 2019 at 10:05:23AM -0700, Shannon Nelson wrote:
On 7/17/19 8:21 PM, Andrew Lunn wrote:
On Tue, Jul 09, 2019 at 03:42:39PM -0700, Shannon Nelson wrote:
On 7/8/19 7:27 PM, Andrew Lunn wrote:
+static int ionic_get_module_eeprom(struct net_device *netdev,
+                                  struct ethtool_eeprom *ee,
+                                  u8 *data)
+{
+       struct lif *lif = netdev_priv(netdev);
+       struct ionic_dev *idev = &lif->ionic->idev;
+       struct xcvr_status *xcvr;
+       u32 len;
+
+       /* copy the module bytes into data */
+       xcvr = &idev->port_info->status.xcvr;
+       len = min_t(u32, sizeof(xcvr->sprom), ee->len);
+       memcpy(data, xcvr->sprom, len);
Hi Shannon

This also looks odd. Where is the call into the firmware to get the
eeprom contents? Even though it is called 'eeprom', the data is not
static. It contains real time diagnostic values, temperature, transmit
power, receiver power, voltages etc.
idev->port_info is a memory mapped space that the device keeps up-to-date.
Hi Shannon

It at least needs a comment. How frequently does the device update
this chunk of memory? It would be good to comment about that as
well. Or do MMIO reads block while i2c operations occur to update the
memory?
The device keeps this updated when changes happen internally so that there
is no need to block on MMIO read.
Hi Shannon

I'm thinking about the diagnostic page. RX and TX power, temperature,
alarms etc. These are real time values, so you should read them on
demand, or at last only cache them for a short time.



They *are* read on demand.  The port_info and lif_info structs are dma mapped spaces that the device keeps up-to-date with PCI writes in the background so that the driver can do a quick memory read for current data.

sln

Reply via email to