2012/7/25 Joe Perches <joe@xxxxxxxxxxx>: > On Wed, 2012-07-25 at 16:47 +0200, Rafał Miłecki wrote: >> use PHY names instead of magic numbers. > [] >> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c > [] >> @@ -4277,6 +4277,37 @@ out: > [] >> +static char *b43_phy_name(struct b43_wldev *dev, u8 phy_type) >> +{ >> + switch (phy_type) { >> + case B43_PHYTYPE_A: >> + return "A"; > [] >> + } >> + >> + b43err(dev->wl, "Unknown PHY Type: %d\n", phy_type); >> + return "UNKNOWN"; > > In general, it's a bad idea to emit error messages > in a generic call like this. This function is not supposed to be called anywhere else, it's only for a single user-friendly message. >> @@ -4337,13 +4368,12 @@ static int b43_phy_versioning(struct b43_wldev *dev) >> unsupported = 1; >> } >> if (unsupported) { >> - b43err(dev->wl, "FOUND UNSUPPORTED PHY " >> - "(Analog %u, Type %u, Revision %u)\n", >> - analog_type, phy_type, phy_rev); >> + b43err(dev->wl, "FOUND UNSUPPORTED PHY (Analog %u, Type %s, Revision %u)\n", >> + analog_type, b43_phy_name(dev, phy_type), phy_rev); > > The extra message doesn't add much useful info > and can also cause errors in continued messages. By "extra message" do you mean the one from b43_phy_name? I wanted to print PHY type as digit somewhere (in case it is not recognized). I can't use simple return ("UNKNOWN %d", phy_type); , such a solution would require allocating memory for string and freeing it. It would complicate the function. Do you have any suggestion for better handling this? -- 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