On Sun, 2009-02-22 at 11:37 +0100, Bastian Blank wrote:
> On Sun, Feb 22, 2009 at 03:53:22AM +0000, Ben Hutchings wrote:
> > --- debian/patches/series/orig-0    (revision 12898)
> > +++ debian/patches/series/orig-0    (working copy)
> > @@ -1,10 +1,16 @@
> > -X debian/dfsg/files-1
> 
> There is a reason why this was at the beginning. See the old tg3 patches
> for details.

It needs to be placed after any patches that insert markers for unifdef.

> > ++static const struct firmware *typhoon_fw;
> 
> const is already static.

What are you talking about?  There is no connection whatsoever between
the two.

> Why is that a global variable?

This was written by Jaswinder Singh; ask him. ;-)

> > ++  pdev = platform_device_register_simple("r128_cce", 0, NULL, 0);
> > ++  if (IS_ERR(pdev)) {
> > ++          printk(KERN_ERR "r128_cce: Failed to register firmware\n");
> > ++          return PTR_ERR(pdev);
> > ++  }
> > ++  rc = request_firmware(&fw, FIRMWARE_NAME, &pdev->dev);
> > ++  platform_device_unregister(pdev);
> 
> The drm modules don't register proper devices already?

Apparently not, though there are PCI devices associated with this.  I
based this on what Jaswinder did for radeon.

> > ++/* Firmware section */
> > ++#define FIRMWARE_BDX      "tehuti/bdx.bin"
> > ++static const struct firmware *bdx_fw;
> 
> Again, why global?
> 
> > +index efaf84d..dec67e0 100644
> > +--- a/drivers/net/tehuti.h
> > ++++ b/drivers/net/tehuti.h
> > +@@ -29,6 +29,7 @@
> > + #include <linux/if_vlan.h>
> > + #include <linux/interrupt.h>
> > + #include <linux/vmalloc.h>
> > ++#include <linux/firmware.h>
> > + #include <asm/byteorder.h>
> 
> This looks wrong. Nothing from firmware.h is used in this header.

Right, this seems to be a holdover from an earlier version of the patch
which kept referenes to the firmware in struct bdx_priv.

> > +  [ Ben Hutchings ]
> > +  * Remove firmware from drivers and make them use request_firmware():
> > +    - mga (closes: #502666)
> > +    - qla1280 (closes: #502667)
> > +    - r128 (closes: #494007)
> > +    - radeon (closes: #494009)
> > +    - tehuti (closes: #501153)
> > +    - typhoon (closes: #502669)
> 
> The patches still needs to be accepted upstream.

The patch for qla1280 has been.  The others, I'll try to push again.
But this hasn't been a requirement for e.g. the tg3 patch so I don't see
why it should be for others.

Ben.

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to