Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
On Sat, 2016-09-24 at 18:32 +0200, SF Markus Elfring wrote:
> > > @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
> > > struct gameport_driver *dr
> > > unsigned long flags;
> > > unsigned char u;
> >
> >
> > > - printk(KERN_INFO "joydump: ,-- START
> > > .\n");
> > > - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
> > > - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
> > > + pr_info(",-- START .\n"
> > > + "| Dumping: %30s |\n"
> > > + "| Speed: %28d kHz |\n",
> > > + gameport->phys,
> > > + gameport->speed);
>
>
> > Not the same output.
>
>
> Should the desired output be the same when the relevant data are passed by a
> single function call
> (instead of three as before)?
Adding a singleton for a pr_fmt #define constant string and
updating the printk subsystem to prepend that constant string
to each use of a pr_ at runtime would be an improvement
as it could reduce constant data used by the format strings.
That would be a _real_ improvement.
Please try to implement something like that before submitting
more of these incorrect patches.
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
On Sat, 2016-09-24 at 18:32 +0200, SF Markus Elfring wrote:
> > > @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
> > > struct gameport_driver *dr
> > > unsigned long flags;
> > > unsigned char u;
> >
> >
> > > - printk(KERN_INFO "joydump: ,-- START
> > > .\n");
> > > - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
> > > - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
> > > + pr_info(",-- START .\n"
> > > + "| Dumping: %30s |\n"
> > > + "| Speed: %28d kHz |\n",
> > > + gameport->phys,
> > > + gameport->speed);
>
>
> > Not the same output.
>
>
> Should the desired output be the same when the relevant data are passed by a
> single function call
> (instead of three as before)?
Adding a singleton for a pr_fmt #define constant string and
updating the printk subsystem to prepend that constant string
to each use of a pr_ at runtime would be an improvement
as it could reduce constant data used by the format strings.
That would be a _real_ improvement.
Please try to implement something like that before submitting
more of these incorrect patches.
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
>> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
>> struct gameport_driver *dr
>>> unsigned long flags;
>>> unsigned char u;
>>
>>> - printk(KERN_INFO "joydump: ,-- START
>>> .\n");
>>> - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
>>> - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
>>> + pr_info(",-- START .\n"
>>> + "| Dumping: %30s |\n"
>>> + "| Speed: %28d kHz |\n",
>>> + gameport->phys,
>>> + gameport->speed);
>
> Not the same output.
Do you insist that each line from a multi-line text that is passed by such a
single logging call
contains the same module prefix?
Regards,
Markus
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
>> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
>> struct gameport_driver *dr
>>> unsigned long flags;
>>> unsigned char u;
>>
>>> - printk(KERN_INFO "joydump: ,-- START
>>> .\n");
>>> - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
>>> - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
>>> + pr_info(",-- START .\n"
>>> + "| Dumping: %30s |\n"
>>> + "| Speed: %28d kHz |\n",
>>> + gameport->phys,
>>> + gameport->speed);
>
> Not the same output.
Do you insist that each line from a multi-line text that is passed by such a
single logging call
contains the same module prefix?
Regards,
Markus
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
On Sat, Sep 24, 2016 at 06:32:29PM +0200, SF Markus Elfring wrote:
> >> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
> >> struct gameport_driver *dr
> >>> unsigned long flags;
> >>> unsigned char u;
> >>
> >>> - printk(KERN_INFO "joydump: ,-- START
> >>> .\n");
> >>> - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
> >>> - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
> >>> + pr_info(",-- START .\n"
> >>> + "| Dumping: %30s |\n"
> >>> + "| Speed: %28d kHz |\n",
> >>> + gameport->phys,
> >>> + gameport->speed);
> >
> > Not the same output.
>
> Should the desired output be the same when the relevant data are passed by a
> single function call
> (instead of three as before)?
The desired output should not be broken in conversion, which you did. Do
you know how syslog works and why the transformation is not correct.
I am also curious as to why you are patching joydump? Are you working on
extending it?
Thanks.
--
Dmitry
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
On Sat, Sep 24, 2016 at 06:32:29PM +0200, SF Markus Elfring wrote:
> >> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
> >> struct gameport_driver *dr
> >>> unsigned long flags;
> >>> unsigned char u;
> >>
> >>> - printk(KERN_INFO "joydump: ,-- START
> >>> .\n");
> >>> - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
> >>> - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
> >>> + pr_info(",-- START .\n"
> >>> + "| Dumping: %30s |\n"
> >>> + "| Speed: %28d kHz |\n",
> >>> + gameport->phys,
> >>> + gameport->speed);
> >
> > Not the same output.
>
> Should the desired output be the same when the relevant data are passed by a
> single function call
> (instead of three as before)?
The desired output should not be broken in conversion, which you did. Do
you know how syslog works and why the transformation is not correct.
I am also curious as to why you are patching joydump? Are you working on
extending it?
Thanks.
--
Dmitry
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
>> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
>> struct gameport_driver *dr
>>> unsigned long flags;
>>> unsigned char u;
>>
>>> - printk(KERN_INFO "joydump: ,-- START
>>> .\n");
>>> - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
>>> - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
>>> + pr_info(",-- START .\n"
>>> + "| Dumping: %30s |\n"
>>> + "| Speed: %28d kHz |\n",
>>> + gameport->phys,
>>> + gameport->speed);
>
> Not the same output.
Should the desired output be the same when the relevant data are passed by a
single function call
(instead of three as before)?
Regards,
Markus
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
>> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
>> struct gameport_driver *dr
>>> unsigned long flags;
>>> unsigned char u;
>>
>>> - printk(KERN_INFO "joydump: ,-- START
>>> .\n");
>>> - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
>>> - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
>>> + pr_info(",-- START .\n"
>>> + "| Dumping: %30s |\n"
>>> + "| Speed: %28d kHz |\n",
>>> + gameport->phys,
>>> + gameport->speed);
>
> Not the same output.
Should the desired output be the same when the relevant data are passed by a
single function call
(instead of three as before)?
Regards,
Markus
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
On Sat, 2016-09-24 at 15:50 +0200, SF Markus Elfring wrote:
> * Prefer usage of the macro "pr_info" over the interface "printk"
> in this function.
> * Reduce number of output function calls.
Did you test this? I doubt it.
> diff --git a/drivers/input/joystick/joydump.c
> b/drivers/input/joystick/joydump.c
[]
> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
> struct gameport_driver *dr
> > unsigned long flags;
> > unsigned char u;
>
> > - printk(KERN_INFO "joydump: ,-- START
> > .\n");
> > - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
> > - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
> > + pr_info(",-- START .\n"
> > + "| Dumping: %30s |\n"
> > + "| Speed: %28d kHz |\n",
> > + gameport->phys,
> > + gameport->speed);
Not the same output.
Re: [PATCH 4/4] Input-gameport: Replace some printk() calls by pr_info() in joydump_connect()
On Sat, 2016-09-24 at 15:50 +0200, SF Markus Elfring wrote:
> * Prefer usage of the macro "pr_info" over the interface "printk"
> in this function.
> * Reduce number of output function calls.
Did you test this? I doubt it.
> diff --git a/drivers/input/joystick/joydump.c
> b/drivers/input/joystick/joydump.c
[]
> @@ -57,27 +57,29 @@ static int joydump_connect(struct gameport *gameport,
> struct gameport_driver *dr
> > unsigned long flags;
> > unsigned char u;
>
> > - printk(KERN_INFO "joydump: ,-- START
> > .\n");
> > - printk(KERN_INFO "joydump: | Dumping: %30s |\n", gameport->phys);
> > - printk(KERN_INFO "joydump: | Speed: %28d kHz |\n", gameport->speed);
> > + pr_info(",-- START .\n"
> > + "| Dumping: %30s |\n"
> > + "| Speed: %28d kHz |\n",
> > + gameport->phys,
> > + gameport->speed);
Not the same output.
