On 6/6/19 8:12 AM, Ezequiel Garcia wrote: > On Thu, 2019-06-06 at 16:51 +0200, Greg Kroah-Hartman wrote: >> On Thu, Jun 06, 2019 at 11:01:17AM -0300, Ezequiel Garcia wrote: >>> On Tue, 2019-06-04 at 20:59 +0200, Greg Kroah-Hartman wrote: >>>> On Tue, Jun 04, 2019 at 11:39:21AM -0700, Guenter Roeck wrote: >>>>> On Tue, Jun 4, 2019 at 11:35 AM Greg Kroah-Hartman >>>>> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >>>>>> On Tue, Jun 04, 2019 at 01:58:38PM -0300, Ezequiel Garcia wrote: >>>>>>> Hey Greg, >>>>>>> >>>>>>>>> + dev_info(&pdev->dev, "Created misc device /dev/%s\n", >>>>>>>>> + data->misc.name); >>>>>>>> >>>>>>>> No need to be noisy, if all goes well, your code should be quiet. >>>>>>>> >>>>>>> >>>>>>> I sometimes wonder about this being noise or not, so I will slightly >>>>>>> hijack this thread for this discussion. >>>>>>> >>>>>>>> From a kernel developer point-of-view, or even from a platform >>>>>>> developer or user with a debugging hat point-of-view, having >>>>>>> a "device created" or "device registered" message is often very useful. >>>>>> >>>>>> For you, yes. For someone with 30000 devices attached to their system, >>>>>> it is not, and causes booting to take longer than it should be. >>>>>> >>>>>>> In fact, I wish people would do this more often, so I don't have to >>>>>>> deal with dynamic debug, or hack my way: >>>>>>> >>>>>>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c >>>>>>> index 4589631798c9..473549b26bb2 100644 >>>>>>> --- a/drivers/media/i2c/ov5647.c >>>>>>> +++ b/drivers/media/i2c/ov5647.c >>>>>>> @@ -603,7 +603,7 @@ static int ov5647_probe(struct i2c_client *client, >>>>>>> if (ret < 0) >>>>>>> goto error; >>>>>>> >>>>>>> - dev_dbg(dev, "OmniVision OV5647 camera driver probed\n"); >>>>>>> + dev_info(dev, "OmniVision OV5647 camera driver probed\n"); >>>>>>> return 0; >>>>>>> error: >>>>>>> media_entity_cleanup(&sd->entity); >>>>>>> >>>>>>> In some subsystems, it's even a behavior I'm more or less relying on: >>>>>>> >>>>>>> $ git grep v4l2_info.*registered drivers/media/ | wc -l >>>>>>> 26 >>>>>>> >>>>>>> And on the downsides, I can't find much. It's just one little line, >>>>>>> that is not even noticed unless you have logging turned on. >>>>>> >>>>>> Its better to be quiet, which is why the "default driver registration" >>>>>> macros do not have any printk messages in them. When converting drivers >>>>>> over to it, we made the boot process much more sane, don't try to go and >>>>>> add messages for no good reason back in please. >>>>>> >>>>>> dynamic debugging can be enabled on a module and line-by-line basis, >>>>>> even from the boot command line. So if you need debugging, you can >>>>>> always ask someone to just reboot or unload/load the module and get the >>>>>> message that way. >>>>>> >>>>> >>>>> Can we by any chance make this an official policy ? I am kind of tired >>>>> having to argue about this over and over again. >>>> >>>> Sure, but how does anyone make any "official policy" in the kernel? :) >>>> >>>> I could just go through and delete all "look ma, a new driver/device!" >>>> messages, but that might be annoying... >>>> >>> >>> Well, I really need to task. >> >> ??? >> > > Oops, typo: s/task/ask :-) > >>> If it's not an official policy (and won't be anytime soon?), >> >> The ":)" there was that we really have very few "official" policies, >> only things that we all strongly encourage to happen. And get grumpy if >> we see them in code reviews. Like I did here. >> > > Well, not everyone gets grumpy. As I pointed out, we use this "registered" > messages (messages or noise, seems this lie in the eye of the beholder), > consistently across entire subsystems. :( >>> then what's preventing Enric from pushing this print on this driver, >>> given he is the one maintaining the code? >> >> Given that he wants people to review his code, why would you tell him to >> ignore what people are trying to tell him? >> > > I'm not suggesting to ignore anyone, rather to consider all voices > involved in each review comment. > >> Again, don't be noisy, it's not hard, and is how things have been >> trending for many years now. Ack that. -- ~Randy
