2011/4/5 Joe Perches <joe@xxxxxxxxxxx>:
> On Tue, 2011-04-05 at 21:57 +0200, RafaÅ MiÅecki wrote:
>> Signed-off-by: RafaÅ MiÅecki <zajec5@xxxxxxxxx>
>
> Just some trivia.
>
>> diff --git a/drivers/bcmai/b43_pci_ai_bridge.c b/drivers/bcmai/b43_pci_ai_bridge.c
> []
>> +#include <linux/bcmai/bcmai.h>
>> +#include <linux/bcmai/bcmai_regs.h>
>> +#include <linux/pci.h>
>> +
>> +static const struct pci_device_id b43_pci_ai_bridge_tbl[] = {
>
> DEFINE_PCI_DEVICE_TABLE
Thanks, I'll convert.
>> diff --git a/drivers/bcmai/core.c b/drivers/bcmai/core.c
> []
>> + Â Â Â Â Â Â bcmai_err("Power control not implemented!\n");
> []
>> diff --git a/include/linux/bcmai/bcmai.h b/include/linux/bcmai/bcmai.h
> []
>> +#define bcmai_info(fmt, args...) Â Â printk(KERN_INFO "bcmai: " fmt, ##args)
>> +#ifdef CONFIG_BCMAI_DEBUG
>> +#define bcmai_dbg(fmt, args...) Â Â Â Â Â Â Âprintk(KERN_DEBUG "bcmai debug: " fmt, ##args)
>> +#else
>> +#define bcmai_dbg(fmt, args...) Â Â Â Â Â Â Âdo { } while (0)
>> +#endif
>> +#define bcmai_err(fmt, args...) Â Â Â Â Â Â Âprintk(KERN_ERR "bcmai error: " fmt, ##args)
>
> I think there's very little value in prefixing
> "error" and "debug" in front of equivalent
> KERN_<level> message levels.
>
> I think you might as well just use pr_<level>
> and pr_fmt.
Hm, I've taken this idea from ssb and b43:
[b43err] printk(KERN_ERR "b43-%s ERROR: %pV", ...);
[_pll_init] ssb_printk(KERN_ERR PFX "ERROR: PLL init unknown for...
Some more reviews, please? Should I drop that prefix, is Joe right? Or
is there some official coding-style for such a situations?
>> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> []
>> +/* AI core, see drivers/bcmai/ */
>> +struct bcmai_device_id {
>> + Â Â __u16 Â manuf;
>> + Â Â __u16 Â id;
>> + Â Â __u8 Â Ârev;
>> +};
>
> Do some of these structs need __packed declarations?
I was reading about __packed long time ago and it was a little tricky
for me. However I don't see anything in mod_devicetable.h using that
__packed. Why should we?
--
RafaÅ
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
