Hi Rami,


On 26.11.2016 16:48, Rami Rosen wrote:

quoted

@@ -0,0 +1,28 @@
+config NET_VENDOR_ALACRITECH
+        bool "Alacritech devices"
+        default y
+        ---help---
+          If you have a network (Ethernet) card belonging to this class, say Y.
+
+          Note that the answer to this question doesn't directly affect the
+          kernel: saying N will just cause the configurator to skip all

Shouldn't it be "Alacritech devices" here, as appears earlier ?

quoted

+          the questions about Renesas devices. If you say Y, you will be asked

Yes, it definitely should not be Renesas :). This is a stupid copy and paste error, I will fix it,
thank you! 

quoted

+          for your specific device in the following questions.
+

...
...
...

quoted

+struct slic_device {
+       struct pci_dev *pdev;

...

quoted

+       bool promisc;

Seems that the autoneg boolean is not used anywhere, apart from
setting it once to true in
the slic_set_link_autoneg() method. Apart from this member it is not
accessed anywhere, so it seems it should be removed.

quoted

+       bool autoneg;
+       int speed;

Agreed, this variable can be removed.

...

quoted

+static int slic_load_rcvseq_firmware(struct slic_device *sdev)
+{
+       const struct firmware *fw;
+       const char *file;
+       u32 codelen;
+       int idx = 0;
+       u32 instr;
+       u32 addr;
+       int err;
+

...

quoted

+       /* Do an initial sanity check concerning firmware size now. A further
+        * check follows below.
+        */
+       if (fw->size < SLIC_FIRMWARE_MIN_SIZE) {
+               dev_err(&sdev->pdev->dev,
+                       "invalid firmware size %zu (min %u expected)\n",
+                       fw->size, SLIC_FIRMWARE_MIN_SIZE);
+               err = -EINVAL;

in the release label, always 0 is returned:

quoted

+               goto release;
+       }
+
+       codelen = slic_read_dword_from_firmware(fw, &idx);
+
+       /* do another sanity check against firmware size */
+       if ((codelen + 4) > fw->size) {
+               dev_err(&sdev->pdev->dev,
+                       "invalid rcv-sequencer firmware size %zu\n", fw->size);
+               err = -EINVAL;

Again, in the release label, always 0 is returned:

quoted

+               goto release;
+       }
+

+release:
+       release_firmware(fw);
+
+       return 0;
+}

This should return "err", I will fix it.

Thanks a lot for the review!

Regards,
Lino