Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1469476 > unrolled thread
| Started by | Alban Bedel <alban.bedel@avionic-design.de> |
|---|---|
| First post | 2016-08-24 16:00 +0200 |
| Last post | 2016-08-25 12:20 +0200 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM Alban Bedel <alban.bedel@avionic-design.de> - 2016-08-24 16:00 +0200
Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM Oliver Neukum <oneukum@suse.com> - 2016-08-24 16:40 +0200
Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM Alban Bedel <alban.bedel@avionic-design.de> - 2016-08-24 16:50 +0200
Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM Oliver Neukum <oneukum@suse.com> - 2016-08-25 11:30 +0200
Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM Alban Bedel <alban.bedel@avionic-design.de> - 2016-08-25 12:10 +0200
Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM Oliver Neukum <oneukum@suse.com> - 2016-08-25 12:20 +0200
| From | Alban Bedel <alban.bedel@avionic-design.de> |
|---|---|
| Date | 2016-08-24 16:00 +0200 |
| Subject | [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM |
| Message-ID | <s9GSK-33R-49@gated-at.bofh.it> |
Implement the .set_eeprom callback to allow setting the MAC address
as well as a few other parameters. Note that the EEPROM must have a
correct PID/VID checksum set otherwise the SROM is used and reads
return the SROM content.
Signed-off-by: Alban Bedel <alban.bedel@avionic-design.de>
---
drivers/net/usb/ax88179_178a.c | 57 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index e6338c16081a..e6a986303dad 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -28,6 +28,7 @@
#define AX88179_PHY_ID 0x03
#define AX_EEPROM_LEN 0x100
+#define AX_EEPROM_BLOCK 0x40u
#define AX88179_EEPROM_MAGIC 0x17900b95
#define AX_MCAST_FLTSIZE 8
#define AX_MAX_MCAST 64
@@ -43,6 +44,7 @@
#define AX_ACCESS_PHY 0x02
#define AX_ACCESS_EEPROM 0x04
#define AX_ACCESS_EFUS 0x05
+#define AX_RELOAD_EEPROM 0x06
#define AX_PAUSE_WATERLVL_HIGH 0x54
#define AX_PAUSE_WATERLVL_LOW 0x55
@@ -620,6 +622,60 @@ ax88179_get_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom,
return 0;
}
+static int
+ax88179_set_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom,
+ u8 *data)
+{
+ struct usbnet *dev = netdev_priv(net);
+ unsigned int offset = eeprom->offset;
+ unsigned int len = eeprom->len;
+ int i, err = 0;
+ u8 *block;
+
+ /* The EEPROM data must be aligned on blocks of 64 bytes */
+ if ((offset % AX_EEPROM_BLOCK) || (len % AX_EEPROM_BLOCK)) {
+ offset = eeprom->offset / AX_EEPROM_BLOCK * AX_EEPROM_BLOCK;
+ len = eeprom->len + eeprom->offset - offset;
+ len = DIV_ROUND_UP(len, AX_EEPROM_BLOCK) * AX_EEPROM_BLOCK;
+
+ block = kmalloc(len, GFP_KERNEL);
+ if (!block)
+ return -ENOMEM;
+
+ /* Copy the current data, we could skip some but KISS */
+ for (i = 0; i < len; i += AX_EEPROM_BLOCK) {
+ err = __ax88179_read_cmd(dev, AX_ACCESS_EEPROM,
+ (offset + i) >> 1,
+ AX_EEPROM_BLOCK >> 1,
+ AX_EEPROM_BLOCK,
+ &block[i], 0);
+ if (err < 0) {
+ kfree(block);
+ return err;
+ }
+ }
+ memcpy(block + eeprom->offset - offset, data, eeprom->len);
+ } else {
+ block = data;
+ }
+
+ for (i = 0; err >= 0 && i < len; i += AX_EEPROM_BLOCK) {
+ err = ax88179_write_cmd(dev, AX_ACCESS_EEPROM,
+ (offset + i) >> 1,
+ AX_EEPROM_BLOCK >> 1,
+ AX_EEPROM_BLOCK, &block[i]);
+ }
+
+ if (block != data)
+ kfree(block);
+
+ /* Reload the EEPROM */
+ if (err >= 0)
+ err = ax88179_write_cmd(dev, AX_RELOAD_EEPROM, 0, 0, 0, NULL);
+
+ return err < 0 ? err : 0;
+}
+
static int ax88179_get_settings(struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -826,6 +882,7 @@ static const struct ethtool_ops ax88179_ethtool_ops = {
.set_wol = ax88179_set_wol,
.get_eeprom_len = ax88179_get_eeprom_len,
.get_eeprom = ax88179_get_eeprom,
+ .set_eeprom = ax88179_set_eeprom,
.get_settings = ax88179_get_settings,
.set_settings = ax88179_set_settings,
.get_eee = ax88179_get_eee,
--
2.9.3
[toc] | [next] | [standalone]
| From | Oliver Neukum <oneukum@suse.com> |
|---|---|
| Date | 2016-08-24 16:40 +0200 |
| Message-ID | <s9Hvs-3yl-15@gated-at.bofh.it> |
| In reply to | #1469476 |
On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote:
> Implement the .set_eeprom callback to allow setting the MAC address
> as well as a few other parameters. Note that the EEPROM must have a
> correct PID/VID checksum set otherwise the SROM is used and reads
> return the SROM content.
>
> Signed-off-by: Alban Bedel <alban.bedel@avionic-design.de>
> ---
> drivers/net/usb/ax88179_178a.c | 57
> ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/drivers/net/usb/ax88179_178a.c
> b/drivers/net/usb/ax88179_178a.c
> index e6338c16081a..e6a986303dad 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -28,6 +28,7 @@
>
> #define AX88179_PHY_ID 0x03
> #define AX_EEPROM_LEN 0x100
> +#define AX_EEPROM_BLOCK 0x40u
> #define AX88179_EEPROM_MAGIC 0x17900b95
> #define AX_MCAST_FLTSIZE 8
> #define AX_MAX_MCAST 64
> @@ -43,6 +44,7 @@
> #define AX_ACCESS_PHY 0x02
> #define AX_ACCESS_EEPROM 0x04
> #define AX_ACCESS_EFUS 0x05
> +#define AX_RELOAD_EEPROM 0x06
> #define AX_PAUSE_WATERLVL_HIGH 0x54
> #define AX_PAUSE_WATERLVL_LOW 0x55
>
> @@ -620,6 +622,60 @@ ax88179_get_eeprom(struct net_device *net, struct
> ethtool_eeprom *eeprom,
> return 0;
> }
>
> +static int
> +ax88179_set_eeprom(struct net_device *net, struct ethtool_eeprom
> *eeprom,
> + u8 *data)
> +{
> + struct usbnet *dev = netdev_priv(net);
> + unsigned int offset = eeprom->offset;
> + unsigned int len = eeprom->len;
> + int i, err = 0;
> + u8 *block;
> +
> + /* The EEPROM data must be aligned on blocks of 64 bytes */
> + if ((offset % AX_EEPROM_BLOCK) || (len % AX_EEPROM_BLOCK)) {
> + offset = eeprom->offset / AX_EEPROM_BLOCK *
> AX_EEPROM_BLOCK;
> + len = eeprom->len + eeprom->offset - offset;
> + len = DIV_ROUND_UP(len, AX_EEPROM_BLOCK) *
> AX_EEPROM_BLOCK;
> +
> + block = kmalloc(len, GFP_KERNEL);
> + if (!block)
> + return -ENOMEM;
> +
> + /* Copy the current data, we could skip some but KISS
> */
> + for (i = 0; i < len; i += AX_EEPROM_BLOCK) {
> + err = __ax88179_read_cmd(dev,
> AX_ACCESS_EEPROM,
> + (offset + i) >> 1,
> + AX_EEPROM_BLOCK >> 1,
> + AX_EEPROM_BLOCK,
> + &block[i], 0);
> + if (err < 0) {
> + kfree(block);
> + return err;
> + }
> + }
> + memcpy(block + eeprom->offset - offset, data,
> eeprom->len);
> + } else {
> + block = data;
> + }
> +
> + for (i = 0; err >= 0 && i < len; i += AX_EEPROM_BLOCK) {
> + err = ax88179_write_cmd(dev, AX_ACCESS_EEPROM,
> + (offset + i) >> 1,
> + AX_EEPROM_BLOCK >> 1,
> + AX_EEPROM_BLOCK, &block[i]);
> + }
> +
> + if (block != data)
> + kfree(block);
And if block == dta, what frees the memory?
Regards
Oliver
[toc] | [prev] | [next] | [standalone]
| From | Alban Bedel <alban.bedel@avionic-design.de> |
|---|---|
| Date | 2016-08-24 16:50 +0200 |
| Subject | Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM |
| Message-ID | <s9HF7-3BB-19@gated-at.bofh.it> |
| In reply to | #1469497 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, 24 Aug 2016 16:30:39 +0200
Oliver Neukum <oneukum@suse.com> wrote:
> On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote:
> > Implement the .set_eeprom callback to allow setting the MAC address
> > as well as a few other parameters. Note that the EEPROM must have a
> > correct PID/VID checksum set otherwise the SROM is used and reads
> > return the SROM content.
> >
> > Signed-off-by: Alban Bedel <alban.bedel@avionic-design.de>
> > ---
> > drivers/net/usb/ax88179_178a.c | 57
> > ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 57 insertions(+)
> >
> > diff --git a/drivers/net/usb/ax88179_178a.c
> > b/drivers/net/usb/ax88179_178a.c
> > index e6338c16081a..e6a986303dad 100644
> > --- a/drivers/net/usb/ax88179_178a.c
> > +++ b/drivers/net/usb/ax88179_178a.c
> > @@ -28,6 +28,7 @@
> >
> > #define AX88179_PHY_ID 0x03
> > #define AX_EEPROM_LEN 0x100
> > +#define AX_EEPROM_BLOCK 0x40u
> > #define AX88179_EEPROM_MAGIC 0x17900b95
> > #define AX_MCAST_FLTSIZE 8
> > #define AX_MAX_MCAST 64
> > @@ -43,6 +44,7 @@
> > #define AX_ACCESS_PHY 0x02
> > #define AX_ACCESS_EEPROM 0x04
> > #define AX_ACCESS_EFUS 0x05
> > +#define AX_RELOAD_EEPROM 0x06
> > #define AX_PAUSE_WATERLVL_HIGH 0x54
> > #define AX_PAUSE_WATERLVL_LOW 0x55
> >
> > @@ -620,6 +622,60 @@ ax88179_get_eeprom(struct net_device *net, struct
> > ethtool_eeprom *eeprom,
> > return 0;
> > }
> >
> > +static int
> > +ax88179_set_eeprom(struct net_device *net, struct ethtool_eeprom
> > *eeprom,
> > + u8 *data)
> > +{
> > + struct usbnet *dev = netdev_priv(net);
> > + unsigned int offset = eeprom->offset;
> > + unsigned int len = eeprom->len;
> > + int i, err = 0;
> > + u8 *block;
> > +
> > + /* The EEPROM data must be aligned on blocks of 64 bytes */
> > + if ((offset % AX_EEPROM_BLOCK) || (len % AX_EEPROM_BLOCK)) {
> > + offset = eeprom->offset / AX_EEPROM_BLOCK *
> > AX_EEPROM_BLOCK;
> > + len = eeprom->len + eeprom->offset - offset;
> > + len = DIV_ROUND_UP(len, AX_EEPROM_BLOCK) *
> > AX_EEPROM_BLOCK;
> > +
> > + block = kmalloc(len, GFP_KERNEL);
> > + if (!block)
> > + return -ENOMEM;
> > +
> > + /* Copy the current data, we could skip some but KISS
> > */
> > + for (i = 0; i < len; i += AX_EEPROM_BLOCK) {
> > + err = __ax88179_read_cmd(dev,
> > AX_ACCESS_EEPROM,
> > + (offset + i) >> 1,
> > + AX_EEPROM_BLOCK >> 1,
> > + AX_EEPROM_BLOCK,
> > + &block[i], 0);
> > + if (err < 0) {
> > + kfree(block);
> > + return err;
> > + }
> > + }
> > + memcpy(block + eeprom->offset - offset, data,
> > eeprom->len);
> > + } else {
> > + block = data;
> > + }
> > +
> > + for (i = 0; err >= 0 && i < len; i += AX_EEPROM_BLOCK) {
> > + err = ax88179_write_cmd(dev, AX_ACCESS_EEPROM,
> > + (offset + i) >> 1,
> > + AX_EEPROM_BLOCK >> 1,
> > + AX_EEPROM_BLOCK, &block[i]);
> > + }
> > +
> > + if (block != data)
> > + kfree(block);
>
> And if block == dta, what frees the memory?
In this case this function didn't allocate any memory, so there is
nothing to free.
Alban
[toc] | [prev] | [next] | [standalone]
| From | Oliver Neukum <oneukum@suse.com> |
|---|---|
| Date | 2016-08-25 11:30 +0200 |
| Message-ID | <s9Z90-7za-19@gated-at.bofh.it> |
| In reply to | #1469503 |
On Wed, 2016-08-24 at 16:40 +0200, Alban Bedel wrote: > On Wed, 24 Aug 2016 16:30:39 +0200 > Oliver Neukum <oneukum@suse.com> wrote: > > > On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote: > > > + if (block != data) > > > + kfree(block); > > > > And if block == dta, what frees the memory? > > In this case this function didn't allocate any memory, so there is > nothing to free. Hi, I see. kfree() has a check for NULL, so you could drop the test, but it doesn't matter much either way. Regards Oliver
[toc] | [prev] | [next] | [standalone]
| From | Alban Bedel <alban.bedel@avionic-design.de> |
|---|---|
| Date | 2016-08-25 12:10 +0200 |
| Subject | Re: [PATCH] usbnet: ax88179_178a: Add support for writing the EEPROM |
| Message-ID | <s9ZLI-82P-19@gated-at.bofh.it> |
| In reply to | #1470002 |
[Multipart message — attachments visible in raw view] — view raw
On Thu, 25 Aug 2016 11:16:36 +0200 Oliver Neukum <oneukum@suse.com> wrote: > On Wed, 2016-08-24 at 16:40 +0200, Alban Bedel wrote: > > On Wed, 24 Aug 2016 16:30:39 +0200 > > Oliver Neukum <oneukum@suse.com> wrote: > > > > > On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote: > > > > > + if (block != data) > > > > + kfree(block); > > > > > > And if block == dta, what frees the memory? > > > > In this case this function didn't allocate any memory, so there is > > nothing to free. > > Hi, > > I see. kfree() has a check for NULL, so you could drop the > test, but it doesn't matter much either way. I think you misunderstand something here. data is the buffer passed by the caller and block is a local variable. There is two cases: 1) The data to write is block aligned, then we use the caller buffer as is and set block = data. 2) The requested data is not block aligned, then we kalloc block. In both case the writing loop then use the block pointer. Afterwards we only need to kfree block in case 2, that is when block != data. Alban
[toc] | [prev] | [next] | [standalone]
| From | Oliver Neukum <oneukum@suse.com> |
|---|---|
| Date | 2016-08-25 12:20 +0200 |
| Message-ID | <s9ZVn-86m-3@gated-at.bofh.it> |
| In reply to | #1470034 |
On Thu, 2016-08-25 at 12:07 +0200, Alban Bedel wrote: > On Thu, 25 Aug 2016 11:16:36 +0200 > Oliver Neukum <oneukum@suse.com> wrote: > > > On Wed, 2016-08-24 at 16:40 +0200, Alban Bedel wrote: > > > On Wed, 24 Aug 2016 16:30:39 +0200 > > > Oliver Neukum <oneukum@suse.com> wrote: > > > > > > > On Wed, 2016-08-24 at 15:52 +0200, Alban Bedel wrote: > > > > > > > + if (block != data) > > > > > + kfree(block); > > > > > > > > And if block == dta, what frees the memory? > > > > > > In this case this function didn't allocate any memory, so there is > > > nothing to free. > > > > Hi, > > > > I see. kfree() has a check for NULL, so you could drop the > > test, but it doesn't matter much either way. > > I think you misunderstand something here. data is the buffer passed > by the caller and block is a local variable. There is two cases: > > 1) The data to write is block aligned, then we use the caller buffer > as is and set block = data. > 2) The requested data is not block aligned, then we kalloc block. > > In both case the writing loop then use the block pointer. Afterwards > we only need to kfree block in case 2, that is when block != data. Thanks for the clarification. Maybe worth a comment in the code? Regards Oliver
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web