With the serial core controller related changes we can now start
addressing serial ports with dev_name:0.0 naming. The names are something
like 00:04.0:0.0 on qemu, and 2800000.serial.0:0.0 on ARM for example.
The dev_name is unique serial port hardware controller device name, also
known as port->dev, and 0.0 are the serial core controller id and port id.
Typically 0.0 are used for each controller and port instance unless the
serial port hardware controller has multiple controllers or ports.
Signed-off-by: Tony Lindgren <[email protected]>
---
Note that this depends on fix for serial core port ids patch
"[PATCH] serial: core: Fix serial core port id to not use port->line"
---
drivers/tty/serial/serial_core.c | 47 ++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3322,6 +3322,49 @@ static int serial_core_port_device_add(struct serial_ctrl_device *ctrl_dev,
return 0;
}
+/*
+ * Add preferred console if configured on kernel command line with naming
+ * "console=dev_name:0.0".
+ */
+static int serial_core_add_preferred_console(struct uart_driver *drv,
+ struct uart_port *port)
+{
+ char *port_match, *opt, *name;
+ int len, ret = 0;
+
+ port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
+ dev_name(port->dev), port->ctrl_id,
+ port->port_id);
+ if (!port_match)
+ return -ENOMEM;
+
+ opt = strstr(saved_command_line, port_match);
+ if (!opt)
+ goto free_port_match;
+
+ len = strlen(port_match);
+
+ if (strlen(opt) > len + 1 && opt[len] == ',')
+ opt += len + 1;
+ else
+ opt = NULL;
+
+ name = kstrdup(drv->dev_name, GFP_KERNEL);
+ if (!name) {
+ ret = -ENOMEM;
+ goto free_port_match;
+ }
+
+ add_preferred_console(name, port->line, opt);
+
+ kfree(name);
+
+free_port_match:
+ kfree(port_match);
+
+ return ret;
+}
+
/*
* Initialize a serial core port device, and a controller device if needed.
*/
@@ -3358,6 +3401,10 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
if (ret)
goto err_unregister_ctrl_dev;
+ ret = serial_core_add_preferred_console(drv, port);
+ if (ret)
+ goto err_unregister_port_dev;
+
ret = serial_core_add_one_port(drv, port);
if (ret)
goto err_unregister_port_dev;
--
2.41.0
* Jiri Slaby <[email protected]> [230719 05:25]:
> On 19. 07. 23, 7:15, Tony Lindgren wrote:
> > +/*
> > + * Add preferred console if configured on kernel command line with naming
> > + * "console=dev_name:0.0".
> > + */
> > +static int serial_core_add_preferred_console(struct uart_driver *drv,
> > + struct uart_port *port)
> > +{
> > + char *port_match, *opt, *name;
> > + int len, ret = 0;
> > +
> > + port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
> > + dev_name(port->dev), port->ctrl_id,
> > + port->port_id);
> > + if (!port_match)
> > + return -ENOMEM;
> > +
> > + opt = strstr(saved_command_line, port_match);
> > + if (!opt)
> > + goto free_port_match;
> > +
> > + len = strlen(port_match);
> > +
> > + if (strlen(opt) > len + 1 && opt[len] == ',')
> > + opt += len + 1;
> > + else
> > + opt = NULL;
> > +
> > + name = kstrdup(drv->dev_name, GFP_KERNEL);
>
> Why do you dup the name here?
I was getting ignoring const warning, but maybe the right solution is
to just use const char *name here.. Let me check.
Regards,
Tony
On 19. 07. 23, 7:15, Tony Lindgren wrote:
> With the serial core controller related changes we can now start
> addressing serial ports with dev_name:0.0 naming. The names are something
> like 00:04.0:0.0 on qemu, and 2800000.serial.0:0.0 on ARM for example.
>
> The dev_name is unique serial port hardware controller device name, also
> known as port->dev, and 0.0 are the serial core controller id and port id.
>
> Typically 0.0 are used for each controller and port instance unless the
> serial port hardware controller has multiple controllers or ports.
>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
>
> Note that this depends on fix for serial core port ids patch
> "[PATCH] serial: core: Fix serial core port id to not use port->line"
>
> ---
> drivers/tty/serial/serial_core.c | 47 ++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3322,6 +3322,49 @@ static int serial_core_port_device_add(struct serial_ctrl_device *ctrl_dev,
> return 0;
> }
>
> +/*
> + * Add preferred console if configured on kernel command line with naming
> + * "console=dev_name:0.0".
> + */
> +static int serial_core_add_preferred_console(struct uart_driver *drv,
> + struct uart_port *port)
> +{
> + char *port_match, *opt, *name;
> + int len, ret = 0;
> +
> + port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
> + dev_name(port->dev), port->ctrl_id,
> + port->port_id);
> + if (!port_match)
> + return -ENOMEM;
> +
> + opt = strstr(saved_command_line, port_match);
> + if (!opt)
> + goto free_port_match;
> +
> + len = strlen(port_match);
> +
> + if (strlen(opt) > len + 1 && opt[len] == ',')
> + opt += len + 1;
> + else
> + opt = NULL;
> +
> + name = kstrdup(drv->dev_name, GFP_KERNEL);
Why do you dup the name here?
> + if (!name) {
> + ret = -ENOMEM;
> + goto free_port_match;
> + }
> +
> + add_preferred_console(name, port->line, opt);
> +
> + kfree(name);
thanks,
--
js
suse labs
On 19. 07. 23, 7:28, Tony Lindgren wrote:
> * Jiri Slaby <[email protected]> [230719 05:25]:
>> On 19. 07. 23, 7:15, Tony Lindgren wrote:
>>> +/*
>>> + * Add preferred console if configured on kernel command line with naming
>>> + * "console=dev_name:0.0".
>>> + */
>>> +static int serial_core_add_preferred_console(struct uart_driver *drv,
>>> + struct uart_port *port)
>>> +{
>>> + char *port_match, *opt, *name;
>>> + int len, ret = 0;
>>> +
>>> + port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
>>> + dev_name(port->dev), port->ctrl_id,
>>> + port->port_id);
>>> + if (!port_match)
>>> + return -ENOMEM;
>>> +
>>> + opt = strstr(saved_command_line, port_match);
>>> + if (!opt)
>>> + goto free_port_match;
>>> +
>>> + len = strlen(port_match);
>>> +
>>> + if (strlen(opt) > len + 1 && opt[len] == ',')
>>> + opt += len + 1;
>>> + else
>>> + opt = NULL;
>>> +
>>> + name = kstrdup(drv->dev_name, GFP_KERNEL);
>>
>> Why do you dup the name here?
>
> I was getting ignoring const warning, but maybe the right solution is
> to just use const char *name here.. Let me check.
So fix add_preferred_console() instead ;).
thanks,
--
js
suse labs
On 19. 07. 23, 7:32, Tony Lindgren wrote:
> * Jiri Slaby <[email protected]> [230719 05:29]:
>> On 19. 07. 23, 7:28, Tony Lindgren wrote:
>>> * Jiri Slaby <[email protected]> [230719 05:25]:
>>>> On 19. 07. 23, 7:15, Tony Lindgren wrote:
>>>>> +/*
>>>>> + * Add preferred console if configured on kernel command line with naming
>>>>> + * "console=dev_name:0.0".
>>>>> + */
>>>>> +static int serial_core_add_preferred_console(struct uart_driver *drv,
>>>>> + struct uart_port *port)
>>>>> +{
>>>>> + char *port_match, *opt, *name;
>>>>> + int len, ret = 0;
>>>>> +
>>>>> + port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
>>>>> + dev_name(port->dev), port->ctrl_id,
>>>>> + port->port_id);
>>>>> + if (!port_match)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + opt = strstr(saved_command_line, port_match);
>>>>> + if (!opt)
>>>>> + goto free_port_match;
>>>>> +
>>>>> + len = strlen(port_match);
>>>>> +
>>>>> + if (strlen(opt) > len + 1 && opt[len] == ',')
>>>>> + opt += len + 1;
>>>>> + else
>>>>> + opt = NULL;
>>>>> +
>>>>> + name = kstrdup(drv->dev_name, GFP_KERNEL);
>>>>
>>>> Why do you dup the name here?
>>>
>>> I was getting ignoring const warning, but maybe the right solution is
>>> to just use const char *name here.. Let me check.
>>
>> So fix add_preferred_console() instead ;).
>
> Let's see what kind of trouble changing it to use const char *name
> might be.
I don't see any, the string is copied internally. So it should be
straightforward. Actually all three parameters of
__add_preferred_console() should be const, IMO. But that involves
changing struct console_cmdline. But that should be straightforward too.
Aside from that, why do you parse saved_command_line on your own?
Instead of using setup() or other commonly used mechanisms for command
line handling?
thanks,
--
js
suse labs
On Wed, Jul 19, 2023 at 08:15:23AM +0300, Tony Lindgren wrote:
> With the serial core controller related changes we can now start
> addressing serial ports with dev_name:0.0 naming. The names are something
> like 00:04.0:0.0 on qemu, and 2800000.serial.0:0.0 on ARM for example.
>
> The dev_name is unique serial port hardware controller device name, also
Maybe for the sake of consistency you may use DEVNAME here and everywhere else
to link this to the DEVNAME uevent environment variable?
> known as port->dev, and 0.0 are the serial core controller id and port id.
>
> Typically 0.0 are used for each controller and port instance unless the
> serial port hardware controller has multiple controllers or ports.
--
With Best Regards,
Andy Shevchenko
* Jiri Slaby <[email protected]> [230719 05:29]:
> On 19. 07. 23, 7:28, Tony Lindgren wrote:
> > * Jiri Slaby <[email protected]> [230719 05:25]:
> > > On 19. 07. 23, 7:15, Tony Lindgren wrote:
> > > > +/*
> > > > + * Add preferred console if configured on kernel command line with naming
> > > > + * "console=dev_name:0.0".
> > > > + */
> > > > +static int serial_core_add_preferred_console(struct uart_driver *drv,
> > > > + struct uart_port *port)
> > > > +{
> > > > + char *port_match, *opt, *name;
> > > > + int len, ret = 0;
> > > > +
> > > > + port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
> > > > + dev_name(port->dev), port->ctrl_id,
> > > > + port->port_id);
> > > > + if (!port_match)
> > > > + return -ENOMEM;
> > > > +
> > > > + opt = strstr(saved_command_line, port_match);
> > > > + if (!opt)
> > > > + goto free_port_match;
> > > > +
> > > > + len = strlen(port_match);
> > > > +
> > > > + if (strlen(opt) > len + 1 && opt[len] == ',')
> > > > + opt += len + 1;
> > > > + else
> > > > + opt = NULL;
> > > > +
> > > > + name = kstrdup(drv->dev_name, GFP_KERNEL);
> > >
> > > Why do you dup the name here?
> >
> > I was getting ignoring const warning, but maybe the right solution is
> > to just use const char *name here.. Let me check.
>
> So fix add_preferred_console() instead ;).
Let's see what kind of trouble changing it to use const char *name
might be.
Tony
* Jiri Slaby <[email protected]> [230719 05:36]:
> On 19. 07. 23, 7:32, Tony Lindgren wrote:
> > * Jiri Slaby <[email protected]> [230719 05:29]:
> > > On 19. 07. 23, 7:28, Tony Lindgren wrote:
> > > > * Jiri Slaby <[email protected]> [230719 05:25]:
> > > > > On 19. 07. 23, 7:15, Tony Lindgren wrote:
> > > > > > +/*
> > > > > > + * Add preferred console if configured on kernel command line with naming
> > > > > > + * "console=dev_name:0.0".
> > > > > > + */
> > > > > > +static int serial_core_add_preferred_console(struct uart_driver *drv,
> > > > > > + struct uart_port *port)
> > > > > > +{
> > > > > > + char *port_match, *opt, *name;
> > > > > > + int len, ret = 0;
> > > > > > +
> > > > > > + port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
> > > > > > + dev_name(port->dev), port->ctrl_id,
> > > > > > + port->port_id);
> > > > > > + if (!port_match)
> > > > > > + return -ENOMEM;
> > > > > > +
> > > > > > + opt = strstr(saved_command_line, port_match);
> > > > > > + if (!opt)
> > > > > > + goto free_port_match;
> > > > > > +
> > > > > > + len = strlen(port_match);
> > > > > > +
> > > > > > + if (strlen(opt) > len + 1 && opt[len] == ',')
> > > > > > + opt += len + 1;
> > > > > > + else
> > > > > > + opt = NULL;
> > > > > > +
> > > > > > + name = kstrdup(drv->dev_name, GFP_KERNEL);
> > > > >
> > > > > Why do you dup the name here?
> > > >
> > > > I was getting ignoring const warning, but maybe the right solution is
> > > > to just use const char *name here.. Let me check.
> > >
> > > So fix add_preferred_console() instead ;).
> >
> > Let's see what kind of trouble changing it to use const char *name
> > might be.
>
> I don't see any, the string is copied internally. So it should be
> straightforward. Actually all three parameters of __add_preferred_console()
> should be const, IMO. But that involves changing struct console_cmdline. But
> that should be straightforward too.
Yeah I'll send a patch for that.
> Aside from that, why do you parse saved_command_line on your own? Instead of
> using setup() or other commonly used mechanisms for command line handling?
Hmm that might be easier yeah :)
Regards,
Tony
Hi Tony,
kernel test robot noticed the following build errors:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next linus/master v6.5-rc2 next-20230719]
[cannot apply to tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tony-Lindgren/serial-core-Add-support-for-dev_name-0-0-naming-for-kernel-console/20230719-131657
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20230719051525.46494-1-tony%40atomide.com
patch subject: [PATCH] serial: core: Add support for dev_name:0.0 naming for kernel console
config: x86_64-randconfig-r013-20230718 (https://download.01.org/0day-ci/archive/20230719/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230719/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> drivers/tty/serial/serial_core.c:3337:17: error: no member named 'port_id' in 'struct uart_port'
port->port_id);
~~~~ ^
1 error generated.
vim +3337 drivers/tty/serial/serial_core.c
3324
3325 /*
3326 * Add preferred console if configured on kernel command line with naming
3327 * "console=dev_name:0.0".
3328 */
3329 static int serial_core_add_preferred_console(struct uart_driver *drv,
3330 struct uart_port *port)
3331 {
3332 char *port_match, *opt, *name;
3333 int len, ret = 0;
3334
3335 port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
3336 dev_name(port->dev), port->ctrl_id,
> 3337 port->port_id);
3338 if (!port_match)
3339 return -ENOMEM;
3340
3341 opt = strstr(saved_command_line, port_match);
3342 if (!opt)
3343 goto free_port_match;
3344
3345 len = strlen(port_match);
3346
3347 if (strlen(opt) > len + 1 && opt[len] == ',')
3348 opt += len + 1;
3349 else
3350 opt = NULL;
3351
3352 name = kstrdup(drv->dev_name, GFP_KERNEL);
3353 if (!name) {
3354 ret = -ENOMEM;
3355 goto free_port_match;
3356 }
3357
3358 add_preferred_console(name, port->line, opt);
3359
3360 kfree(name);
3361
3362 free_port_match:
3363 kfree(port_match);
3364
3365 return ret;
3366 }
3367
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Tony,
kernel test robot noticed the following build errors:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.5-rc2 next-20230719]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tony-Lindgren/serial-core-Add-support-for-dev_name-0-0-naming-for-kernel-console/20230719-131657
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20230719051525.46494-1-tony%40atomide.com
patch subject: [PATCH] serial: core: Add support for dev_name:0.0 naming for kernel console
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230720/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230720/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/tty/serial/serial_core.c: In function 'serial_core_add_preferred_console':
>> drivers/tty/serial/serial_core.c:3337:36: error: 'struct uart_port' has no member named 'port_id'
3337 | port->port_id);
| ^~
vim +3337 drivers/tty/serial/serial_core.c
3324
3325 /*
3326 * Add preferred console if configured on kernel command line with naming
3327 * "console=dev_name:0.0".
3328 */
3329 static int serial_core_add_preferred_console(struct uart_driver *drv,
3330 struct uart_port *port)
3331 {
3332 char *port_match, *opt, *name;
3333 int len, ret = 0;
3334
3335 port_match = kasprintf(GFP_KERNEL, "console=%s:%i.%i",
3336 dev_name(port->dev), port->ctrl_id,
> 3337 port->port_id);
3338 if (!port_match)
3339 return -ENOMEM;
3340
3341 opt = strstr(saved_command_line, port_match);
3342 if (!opt)
3343 goto free_port_match;
3344
3345 len = strlen(port_match);
3346
3347 if (strlen(opt) > len + 1 && opt[len] == ',')
3348 opt += len + 1;
3349 else
3350 opt = NULL;
3351
3352 name = kstrdup(drv->dev_name, GFP_KERNEL);
3353 if (!name) {
3354 ret = -ENOMEM;
3355 goto free_port_match;
3356 }
3357
3358 add_preferred_console(name, port->line, opt);
3359
3360 kfree(name);
3361
3362 free_port_match:
3363 kfree(port_match);
3364
3365 return ret;
3366 }
3367
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
* Andy Shevchenko <[email protected]> [230719 05:37]:
> On Wed, Jul 19, 2023 at 08:15:23AM +0300, Tony Lindgren wrote:
> > With the serial core controller related changes we can now start
> > addressing serial ports with dev_name:0.0 naming. The names are something
> > like 00:04.0:0.0 on qemu, and 2800000.serial.0:0.0 on ARM for example.
> >
> > The dev_name is unique serial port hardware controller device name, also
>
> Maybe for the sake of consistency you may use DEVNAME here and everywhere else
> to link this to the DEVNAME uevent environment variable?
Yes good idea will do.
Regards,
Tony