Re: register_device: was: Re: [PATCH v6 06/11] printk: console: Introduce sysfs interface for per-console loglevels

[Petr Mladek]

On Wed 2024-11-13 16:59:17, Petr Mladek wrote:
> > A sysfs interface under /sys/class/console/ is created that permits
> > viewing and configuring per-console attributes. This is the main
> > interface with which we expect users to interact with and configure
> > per-console loglevels.
> 
> > diff --git a/kernel/printk/sysfs.c b/kernel/printk/sysfs.c
> > new file mode 100644
> > index 000000000000..e24590074861
> > --- /dev/null
> > +++ b/kernel/printk/sysfs.c
> > +ATTRIBUTE_GROUPS(console_sysfs);
> > +
> > +static void console_classdev_release(struct device *dev)
> > +{
> > +	kfree(dev);
> > +}
> > +
> > +void console_register_device(struct console *con)
> > +{
> > +	/*
> > +	 * We might be called from register_console() before the class is
> > +	 * registered. If that happens, we'll take care of it in
> > +	 * printk_late_init.
> > +	 */
> > +	if (IS_ERR_OR_NULL(console_class))
> > +		return;
> > +
> > +	if (WARN_ON(con->classdev))
> > +		return;
> > +
> > +	con->classdev = kzalloc(sizeof(struct device), GFP_KERNEL);
> > +	if (!con->classdev)
> > +		return;
> > +
> > +	device_initialize(con->classdev);
> > +	dev_set_name(con->classdev, "%s%d", con->name, con->index);
> > +	dev_set_drvdata(con->classdev, con);
> > +	con->classdev->release = console_classdev_release;
> > +	con->classdev->class = console_class;
> > +	if (device_add(con->classdev))
> > +		put_device(con->classdev);
> 
> Honestly, I am not sure how to review this. I am not familiar with
> these APIs. I have spent few hours trying to investigate various
> drivers but I did not find any similar use case. I tried to look
> for documentation but I did not find any good HOWTO.

BTW: When investigating other users of these APIs, I saw
a use of pm_runtime_no_callbacks() in

static int i2c_register_adapter(struct i2c_adapter *adap)
{
	dev_set_name(&adap->dev, "i2c-%d", adap->nr);
[...]
	device_initialize(&adap->dev);
[...]
	/*
	 * This adapter can be used as a parent immediately after device_add(),
	 * setup runtime-pm (especially ignore-children) before hand.
	 */
	device_enable_async_suspend(&adap->dev);
	pm_runtime_no_callbacks(&adap->dev);

It removed part of the sysfs interface in the power subdirectory.

It might make sense to use this for the console devices as well.
If I get it correctly then the newly added struct device in
struct console can't affect the power management of the underlying
HW device.

Best Regards,
Petr