Re: [irda-users] [PATCH] [IRDA] STIR421X updates

[Nick F. <nfe...@at...>]

=C2 =F1=EE=EE=E1=F9=E5=ED=E8=E8 =EE=F2 Monday 05 June 2006 10:26 Samuel Ort=
iz =ED=E0=EF=E8=F1=E0=EB(a):
> This patch looks good to me, I like the fact that we are now using fixed
> names for the firmware files. The code is also simpler, which is good.
> I very slightly modifed it for cosmetic reasons, see below.
> Nick, could you please quickly re-test it before I send it upstream ?
Patch re-tested (upload image file from mobile phone)
No problems found.

So just a few notes, IMHO:
1) Fix the function name in the comment below:=20
+/*
+ * Function stir421x_patching(struct irda_usb_cb *self)
+ *
+ * Get a firmware code from userspase using hotplug request_firmware() call
+  */
 static int stir421x_patch_device(struct irda_usb_cb *self)

2) Probably useless vars
+                /* Let's check if the product version is dotted */
+                if (fw_version_ptr[3] =3D=3D '.' &&
+                   fw_version_ptr[7] =3D=3D '.') {
+                       unsigned long major, minor, build;
=2E..
+                       fw_version =3D (major << 12)
+                               + (minor << 8)
+                               + ((build / 10) << 4)
+                               + (build % 10);
=2E..
+        if (self->usbdev->descriptor.bcdDevice =3D=3D fw_version) {
The vars unsigned long major, minor, build such as fw_version nowhere user =
outside of the=20
code above, so for my case i tried to avoid excessive variables.
/* Agree - no limits for code optimization  :) */
So it on Your decision.

3) Remove the space ") ;"
=2D       self->needspatch =3D ((self->capability & IUC_STIR_4210) !=3D 0) ;
+       self->needspatch =3D ((self->capability & IUC_STIR421X) !=3D 0) ;


4) Indentation - bring to one line up stir421x_fw_upload()?
+                               ret =3D
+                                       stir421x_fw_upload(self, &fw->data[=
i],
+                                                          fw->size - i);

In some places where You made a changes, tabs changed to spaces, I don't su=
re=20
what should be in kernel patch (spaces or tabs), so take care.

=2D-=20
Best Regards,=20
Nick Fedchik