* [PATCH] sis900: add support for ethtool --eeprom-dump
@ 2019-07-25 16:18 Sergej Benilov
2019-07-25 16:25 ` Andrew Lunn
0 siblings, 1 reply; 5+ messages in thread
From: Sergej Benilov @ 2019-07-25 16:18 UTC (permalink / raw)
To: venza, netdev, andrew; +Cc: Sergej Benilov
Implement ethtool's EEPROM dump command (ethtool -e|--eeprom-dump).
Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
---
drivers/net/ethernet/sis/sis900.c | 68 +++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index 6e07f5ebacfc..cf9d75ffefc9 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -191,6 +191,8 @@ struct sis900_private {
unsigned int tx_full; /* The Tx queue is full. */
u8 host_bridge_rev;
u8 chipset_rev;
+ /* EEPROM data */
+ int eeprom_size;
};
MODULE_AUTHOR("Jim Huang <cmhuang@sis.com.tw>, Ollie Lho <ollie@sis.com.tw>");
@@ -475,6 +477,8 @@ static int sis900_probe(struct pci_dev *pci_dev,
sis_priv->pci_dev = pci_dev;
spin_lock_init(&sis_priv->lock);
+ sis_priv->eeprom_size = 24;
+
pci_set_drvdata(pci_dev, net_dev);
ring_space = pci_alloc_consistent(pci_dev, TX_TOTAL_SIZE, &ring_dma);
@@ -2122,6 +2126,68 @@ static void sis900_get_wol(struct net_device *net_dev, struct ethtool_wolinfo *w
wol->supported = (WAKE_PHY | WAKE_MAGIC);
}
+static int sis900_get_eeprom_len(struct net_device *dev)
+{
+ struct sis900_private *sis_priv = netdev_priv(dev);
+
+ return sis_priv->eeprom_size;
+}
+
+static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
+{
+ struct sis900_private *sis_priv = netdev_priv(net_dev);
+ void __iomem *ioaddr = sis_priv->ioaddr;
+ int wait, ret = -EAGAIN;
+ u16 signature;
+ u16 *ebuf = (u16 *)buf;
+ int i;
+
+ if (sis_priv->chipset_rev == SIS96x_900_REV) {
+ sw32(mear, EEREQ);
+ for (wait = 0; wait < 2000; wait++) {
+ if (sr32(mear) & EEGNT) {
+ /* read 16 bits, and index by 16 bits */
+ for (i = 0; i < sis_priv->eeprom_size / 2; i++)
+ ebuf[i] = (u16)read_eeprom(ioaddr, i);
+ ret = 0;
+ break;
+ }
+ udelay(1);
+ }
+ sw32(mear, EEDONE);
+ } else {
+ signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
+ if (signature != 0xffff && signature != 0x0000) {
+ /* read 16 bits, and index by 16 bits */
+ for (i = 0; i < sis_priv->eeprom_size / 2; i++)
+ ebuf[i] = (u16)read_eeprom(ioaddr, i);
+ ret = 0;
+ }
+ }
+ return ret;
+}
+
+#define SIS900_EEPROM_MAGIC 0xBABE
+static int sis900_get_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom, u8 *data)
+{
+ struct sis900_private *sis_priv = netdev_priv(dev);
+ u8 *eebuf;
+ int res;
+
+ eebuf = kmalloc(sis_priv->eeprom_size, GFP_KERNEL);
+ if (!eebuf)
+ return -ENOMEM;
+
+ eeprom->magic = SIS900_EEPROM_MAGIC;
+ spin_lock_irq(&sis_priv->lock);
+ res = sis900_read_eeprom(dev, eebuf);
+ spin_unlock_irq(&sis_priv->lock);
+ if (!res)
+ memcpy(data, eebuf + eeprom->offset, eeprom->len);
+ kfree(eebuf);
+ return res;
+}
+
static const struct ethtool_ops sis900_ethtool_ops = {
.get_drvinfo = sis900_get_drvinfo,
.get_msglevel = sis900_get_msglevel,
@@ -2132,6 +2198,8 @@ static const struct ethtool_ops sis900_ethtool_ops = {
.set_wol = sis900_set_wol,
.get_link_ksettings = sis900_get_link_ksettings,
.set_link_ksettings = sis900_set_link_ksettings,
+ .get_eeprom_len = sis900_get_eeprom_len,
+ .get_eeprom = sis900_get_eeprom,
};
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] sis900: add support for ethtool --eeprom-dump
2019-07-25 16:18 [PATCH] sis900: add support for ethtool --eeprom-dump Sergej Benilov
@ 2019-07-25 16:25 ` Andrew Lunn
2019-07-25 16:41 ` Sergej Benilov
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2019-07-25 16:25 UTC (permalink / raw)
To: Sergej Benilov; +Cc: venza, netdev
> +static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
> +{
> + struct sis900_private *sis_priv = netdev_priv(net_dev);
> + void __iomem *ioaddr = sis_priv->ioaddr;
> + int wait, ret = -EAGAIN;
> + u16 signature;
> + u16 *ebuf = (u16 *)buf;
> + int i;
> +
> + if (sis_priv->chipset_rev == SIS96x_900_REV) {
> + sw32(mear, EEREQ);
> + for (wait = 0; wait < 2000; wait++) {
> + if (sr32(mear) & EEGNT) {
> + /* read 16 bits, and index by 16 bits */
> + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> + ret = 0;
> + break;
> + }
> + udelay(1);
> + }
> + sw32(mear, EEDONE);
The indentation looks all messed up here.
> + } else {
> + signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
> + if (signature != 0xffff && signature != 0x0000) {
> + /* read 16 bits, and index by 16 bits */
> + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> + ret = 0;
> + }
> + }
> + return ret;
> +}
> +
> +#define SIS900_EEPROM_MAGIC 0xBABE
> +static int sis900_get_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom, u8 *data)
> +{
> + struct sis900_private *sis_priv = netdev_priv(dev);
> + u8 *eebuf;
> + int res;
> +
> + eebuf = kmalloc(sis_priv->eeprom_size, GFP_KERNEL);
> + if (!eebuf)
> + return -ENOMEM;
> +
> + eeprom->magic = SIS900_EEPROM_MAGIC;
> + spin_lock_irq(&sis_priv->lock);
> + res = sis900_read_eeprom(dev, eebuf);
> + spin_unlock_irq(&sis_priv->lock);
> + if (!res)
> + memcpy(data, eebuf + eeprom->offset, eeprom->len);
> + kfree(eebuf);
Why do you not put the data directly into data and avoid this memory
allocation, and memcpy?
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sis900: add support for ethtool --eeprom-dump
2019-07-25 16:25 ` Andrew Lunn
@ 2019-07-25 16:41 ` Sergej Benilov
2019-07-25 18:20 ` Andrew Lunn
0 siblings, 1 reply; 5+ messages in thread
From: Sergej Benilov @ 2019-07-25 16:41 UTC (permalink / raw)
To: Andrew Lunn; +Cc: venza, netdev
On Thu, 25 Jul 2019 at 18:25, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
> > +{
> > + struct sis900_private *sis_priv = netdev_priv(net_dev);
> > + void __iomem *ioaddr = sis_priv->ioaddr;
> > + int wait, ret = -EAGAIN;
> > + u16 signature;
> > + u16 *ebuf = (u16 *)buf;
> > + int i;
> > +
> > + if (sis_priv->chipset_rev == SIS96x_900_REV) {
> > + sw32(mear, EEREQ);
> > + for (wait = 0; wait < 2000; wait++) {
> > + if (sr32(mear) & EEGNT) {
> > + /* read 16 bits, and index by 16 bits */
> > + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> > + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> > + ret = 0;
> > + break;
> > + }
> > + udelay(1);
> > + }
> > + sw32(mear, EEDONE);
>
> The indentation looks all messed up here.
This has passed ./scripts/checkpatch.pl, as you had suggested for the
previous patch.
>
> > + } else {
> > + signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
> > + if (signature != 0xffff && signature != 0x0000) {
> > + /* read 16 bits, and index by 16 bits */
> > + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> > + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> > + ret = 0;
> > + }
> > + }
> > + return ret;
> > +}
> > +
> > +#define SIS900_EEPROM_MAGIC 0xBABE
> > +static int sis900_get_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom, u8 *data)
> > +{
> > + struct sis900_private *sis_priv = netdev_priv(dev);
> > + u8 *eebuf;
> > + int res;
> > +
> > + eebuf = kmalloc(sis_priv->eeprom_size, GFP_KERNEL);
> > + if (!eebuf)
> > + return -ENOMEM;
> > +
> > + eeprom->magic = SIS900_EEPROM_MAGIC;
> > + spin_lock_irq(&sis_priv->lock);
> > + res = sis900_read_eeprom(dev, eebuf);
> > + spin_unlock_irq(&sis_priv->lock);
> > + if (!res)
> > + memcpy(data, eebuf + eeprom->offset, eeprom->len);
> > + kfree(eebuf);
>
> Why do you not put the data directly into data and avoid this memory
> allocation, and memcpy?
Because EEPROM data from 'eeprom->offset' offset and of 'eeprom->len'
length only is expected to be returned in 'data'.
>
> Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sis900: add support for ethtool --eeprom-dump
2019-07-25 16:41 ` Sergej Benilov
@ 2019-07-25 18:20 ` Andrew Lunn
2019-07-25 19:52 ` Sergej Benilov
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2019-07-25 18:20 UTC (permalink / raw)
To: Sergej Benilov; +Cc: venza, netdev
On Thu, Jul 25, 2019 at 06:41:41PM +0200, Sergej Benilov wrote:
> On Thu, 25 Jul 2019 at 18:25, Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > +static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
> > > +{
> > > + struct sis900_private *sis_priv = netdev_priv(net_dev);
> > > + void __iomem *ioaddr = sis_priv->ioaddr;
> > > + int wait, ret = -EAGAIN;
> > > + u16 signature;
> > > + u16 *ebuf = (u16 *)buf;
> > > + int i;
> > > +
> > > + if (sis_priv->chipset_rev == SIS96x_900_REV) {
> > > + sw32(mear, EEREQ);
> > > + for (wait = 0; wait < 2000; wait++) {
> > > + if (sr32(mear) & EEGNT) {
> > > + /* read 16 bits, and index by 16 bits */
> > > + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> > > + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> > > + ret = 0;
> > > + break;
> > > + }
> > > + udelay(1);
> > > + }
> > > + sw32(mear, EEDONE);
> >
> > The indentation looks all messed up here.
>
> This has passed ./scripts/checkpatch.pl, as you had suggested for the
> previous patch.
checkpatch just checks for things like tabs vs space.
I would expect the indentation to be more like:
if (sis_priv->chipset_rev == SIS96x_900_REV) {
sw32(mear, EEREQ);
for (wait = 0; wait < 2000; wait++) {
if (sr32(mear) & EEGNT) {
/* read 16 bits, and index by 16 bits */
for (i = 0; i < sis_priv->eeprom_size / 2; i++)
ebuf[i] = (u16)read_eeprom(ioaddr, i);
ret = 0;
break;
}
udelay(1);
}
sw32(mear, EEDONE);
} else {
signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
if (signature != 0xffff && signature != 0x0000) {
/* read 16 bits, and index by 16 bits */
for (i = 0; i < sis_priv->eeprom_size / 2; i++)
ebuf[i] = (u16)read_eeprom(ioaddr, i);
ret = 0;
}
}
return ret;
> > Why do you not put the data directly into data and avoid this memory
> > allocation, and memcpy?
>
> Because EEPROM data from 'eeprom->offset' offset and of 'eeprom->len'
> length only is expected to be returned in 'data'.
O.K.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sis900: add support for ethtool --eeprom-dump
2019-07-25 18:20 ` Andrew Lunn
@ 2019-07-25 19:52 ` Sergej Benilov
0 siblings, 0 replies; 5+ messages in thread
From: Sergej Benilov @ 2019-07-25 19:52 UTC (permalink / raw)
To: Andrew Lunn; +Cc: venza, netdev
On Thu, 25 Jul 2019 at 20:20, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Jul 25, 2019 at 06:41:41PM +0200, Sergej Benilov wrote:
> > On Thu, 25 Jul 2019 at 18:25, Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > +static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
> > > > +{
> > > > + struct sis900_private *sis_priv = netdev_priv(net_dev);
> > > > + void __iomem *ioaddr = sis_priv->ioaddr;
> > > > + int wait, ret = -EAGAIN;
> > > > + u16 signature;
> > > > + u16 *ebuf = (u16 *)buf;
> > > > + int i;
> > > > +
> > > > + if (sis_priv->chipset_rev == SIS96x_900_REV) {
> > > > + sw32(mear, EEREQ);
> > > > + for (wait = 0; wait < 2000; wait++) {
> > > > + if (sr32(mear) & EEGNT) {
> > > > + /* read 16 bits, and index by 16 bits */
> > > > + for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> > > > + ebuf[i] = (u16)read_eeprom(ioaddr, i);
> > > > + ret = 0;
> > > > + break;
> > > > + }
> > > > + udelay(1);
> > > > + }
> > > > + sw32(mear, EEDONE);
> > >
> > > The indentation looks all messed up here.
> >
> > This has passed ./scripts/checkpatch.pl, as you had suggested for the
> > previous patch.
>
> checkpatch just checks for things like tabs vs space.
>
> I would expect the indentation to be more like:
>
>
> if (sis_priv->chipset_rev == SIS96x_900_REV) {
> sw32(mear, EEREQ);
> for (wait = 0; wait < 2000; wait++) {
> if (sr32(mear) & EEGNT) {
> /* read 16 bits, and index by 16 bits */
> for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> ebuf[i] = (u16)read_eeprom(ioaddr, i);
> ret = 0;
> break;
> }
> udelay(1);
> }
> sw32(mear, EEDONE);
> } else {
> signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
> if (signature != 0xffff && signature != 0x0000) {
> /* read 16 bits, and index by 16 bits */
> for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> ebuf[i] = (u16)read_eeprom(ioaddr, i);
> ret = 0;
> }
> }
> return ret;
>
Ok, I see now what you mean.
I fixed the alignment.
This patch is superseded.
> > > Why do you not put the data directly into data and avoid this memory
> > > allocation, and memcpy?
> >
> > Because EEPROM data from 'eeprom->offset' offset and of 'eeprom->len'
> > length only is expected to be returned in 'data'.
>
> O.K.
>
> Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-25 19:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-25 16:18 [PATCH] sis900: add support for ethtool --eeprom-dump Sergej Benilov
2019-07-25 16:25 ` Andrew Lunn
2019-07-25 16:41 ` Sergej Benilov
2019-07-25 18:20 ` Andrew Lunn
2019-07-25 19:52 ` Sergej Benilov
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).