On Fri, Mar 02, 2007 at 01:53:48AM -0600, Al Borchers wrote:
> Oleg --
>
> Comments in line below.
>
> Quoting Oleg Verych <[EMAIL PROTECTED]>:
>
> > pp-by: Oleg Verych
> > ---
[]
> > -/* Firmware image header */
> > -struct ti_firmware_header {
> > - __le16 wLength;
> > - __u8 bCheckSum;
> > -} __attribute__((packed));
>
> These came from reading the TI bootloader firmware sources.
> The TI_DOWNLOAD_MAX_PACKET_SIZE is a constraint of the bootloader.
> The header structure also matches the TI bootloader firmware.
>
> I do not want to change these.
[]
> > +
> > +typedef union {
> > + __le32 a; /* all */
> > + struct {
> > + __le32 sz : 16, cs : 8;
> > + } d;
> > +} ti_firmware_header_t;
>
> The header is 24 bits, not 32. Why use a union here?
Yes. I use "int" as container for 24bits value of the firmware header. This
need no "packed" attribute for this structure, and it is just being sent
first, without recopying it in the firmware buffer.
> > +#define ti_fw_file_3410 "umpf3410.i51"
> > +#define ti_fw_file_5052 "umpf5052.i51"
>
> These are not the right names for the firmware files. Please see
> my patch at http://www.brimson.com/downloads/ti_usb_multitech-1.1.tgz
> for the firmware files names we are using. These names are specific
> to just one device.
OK. That means, in setup code one must choose file names based on Ids
> > -
> > - if (tdev->td_is_3410)
> > - status = ti_download_firmware(tdev, ti_fw_3410,
> > - sizeof(ti_fw_3410));
> > - else
> > - status = ti_download_firmware(tdev, ti_fw_5052,
> > - sizeof(ti_fw_5052));
here:
> > + status = ti_fw_change(tdev, tdev->td_is_3410 ?
> > + ti_fw_file_3410 : ti_fw_file_5052);
This also means, you will make more complex "td_is_3410" stuff (:.
> > +#include <linux/firmware.h>
> > #include "ti_usb_3410_5052.h"
> > -#include "ti_fw_3410.h" /* firmware image for 3410 */
> > -#include "ti_fw_5052.h" /* firmware image for 5052 */
>
> No. This will break the driver for any user who upgrades the
> kernel and does not have the firmware files. We talked about
> this before--I do not want to do that. I want to keep the
> firmware in the driver--we can load firmware if a firmware
> file is found, but if not, we need to fall back to the firmware
> image in the driver.
Maybe you are right. But as for me, i didn't see any Vendor Drivers
README with description of the setup and support on Linux, only driver
sources with outdated script.
If you yourself have one with all hotplug/udev stuff, then i don't see
problems to describe firmware files and point into *up to date* firmware
images, and to not bloat Linux kernel with it.
> Again see my patch at
> http://www.brimson.com/downloads/ti_usb_multitech-1.1.tgz
> to see how I want this done. I will submit this patch myself in a few days.
Maye i will, or cc me, please.
Thanks.
_____
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
