Messages in this thread | | | Date | Tue, 30 Sep 2014 11:02:23 +0300 | From | Tanya Brokhman <> | Subject | Re: [PATCH] mtd: ubi: Extend UBI layer debug/messaging capabilities |
| |
On 9/28/2014 3:01 PM, Artem Bityutskiy wrote: > On Sun, 2014-09-28 at 09:36 +0300, Tanya Brokhman wrote: >> If there is more then one UBI device mounted, there is no way to >> distinguish between messages from different UBI devices. >> Add device number to all ubi layer message types. > > Hi, the goal looks legit to me, but the patch is so large that I do not > think that I can really review it in this form. > > a) A patch which changes the macros (ubi_err(), etc)
If I divide the patches like this, patch (a) wont compile
> b) A set of patches which do not change messages at all, but add the > 'ubi' parameter to the places where it is missing. > c) A patch which changes the messages.
I think patches (b)+(C) can be combined into one patch. Don't you agree? I changed ~2 or 3 messages that were printing ubi number by themselves. No need for a separate patch for this. Don't you agree?
> > So a) will be the most important patch for the reviewer. b) - more or > less mechanical changes of a similar kind. c) - the same. > > Also, if you add a parameter to 'ubi_err()' and the other printing > wrappers, add 'ubi' there, not 'ubi_num'. This will allow to prefix > messages with vary different things, not just the device number in the > future. So the calls would look like > > ubi_err(ubi, "inconsistent used_ebs"); > > Once this is done, the series should be more reviewable. The next thing > I'd check is whether we really need to change all the messages, or most > of them, or we actually need to change only a small part of them. In the > former case, it is OK to do what you do, I guess. In the latter case we > probably better off with introducing a separate set of printing macros > and leave the existing ones as they are.
Large portion of the messages needs updating. I think perhaps I'll overcome the messages during init (when I don't' have the ubi yet) in a different manner and add "ubi" not "ubi_num" parameter to the macros, as you suggested
> > Thanks! >
-- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
| |