2021-11-02 21:54:40

by Ira Weiny

[permalink] [raw]
Subject: [PATCH 0/3] Improve Auxiliary Device documentation

From: Ira Weiny <[email protected]>

The auxiliary device documentation was not wrong but it was a bit difficult to
follow. Add clarifications to ensure that details are not missed.


Ira Weiny (3):
Documentation/auxiliary_bus: Clarify auxiliary_device creation
Documentation/auxiliary_bus: Clarify match_name
Documentation/auxiliary_bus: Update Auxiliary device lifespan

Documentation/driver-api/auxiliary_bus.rst | 136 ++++++++++++++++-----
1 file changed, 107 insertions(+), 29 deletions(-)

--
2.28.0.rc0.12.gb6a658bd00c9


2021-11-02 21:55:10

by Ira Weiny

[permalink] [raw]
Subject: [PATCH 3/3] Documentation/auxiliary_bus: Update Auxiliary device lifespan

From: Ira Weiny <[email protected]>

It was unclear when the auxiliary device objects were to be free'ed by
the parent (registering) driver.

Also there are some patterns like using devm_add_action_or_reset() which
are helpful to mention to those using the interface to ensure they don't
double free or miss freeing the auxiliary devices.

Reviewed-by: Dave Jiang <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Documentation/driver-api/auxiliary_bus.rst | 32 ++++++++++++++--------
1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
index 41ec9aed059f..77c1a39e38d5 100644
--- a/Documentation/driver-api/auxiliary_bus.rst
+++ b/Documentation/driver-api/auxiliary_bus.rst
@@ -164,9 +164,15 @@ Auxiliary Device Memory Model and Lifespan
------------------------------------------

The registering driver is the entity that allocates memory for the
-auxiliary_device and register it on the auxiliary bus. It is important to note
+auxiliary_device and registers it on the auxiliary bus. It is important to note
that, as opposed to the platform bus, the registering driver is wholly
-responsible for the management for the memory used for the driver object.
+responsible for the management of the memory used for the device object.
+
+To be clear the memory for the auxiliary_device is freed in the release()
+callback defined by the registering driver. The registering driver should only
+call auxiliary_device_delete() and then auxiliary_device_uninit() when it is
+done with the device. The release() function is then automatically called if
+and when other drivers release their reference to the devices.

A parent object, defined in the shared header file, contains the
auxiliary_device. It also contains a pointer to the shared object(s), which
@@ -177,18 +183,22 @@ from the pointer to the auxiliary_device, that is passed during the call to the
auxiliary_driver's probe function, up to the parent object, and then have
access to the shared object(s).

-The memory for the auxiliary_device is freed only in its release() callback
-flow as defined by its registering driver.
-
The memory for the shared object(s) must have a lifespan equal to, or greater
-than, the lifespan of the memory for the auxiliary_device. The auxiliary_driver
-should only consider that this shared object is valid as long as the
-auxiliary_device is still registered on the auxiliary bus. It is up to the
-registering driver to manage (e.g. free or keep available) the memory for the
-shared object beyond the life of the auxiliary_device.
+than, the lifespan of the memory for the auxiliary_device. The
+auxiliary_driver should only consider that the shared object is valid as long
+as the auxiliary_device is still registered on the auxiliary bus. It is up to
+the registering driver to manage (e.g. free or keep available) the memory for
+the shared object beyond the life of the auxiliary_device.

The registering driver must unregister all auxiliary devices before its own
-driver.remove() is completed.
+driver.remove() is completed. An easy way to ensure this is to use the
+devm_add_action_or_reset() call to register a function against the parent device
+which unregisters the auxiliary device object(s).
+
+Finally, any operations which operate on the auxiliary devices must continue to
+function (if only to return an error) after the registering driver unregisters
+the auxiliary device.
+

Auxiliary Drivers
=================
--
2.28.0.rc0.12.gb6a658bd00c9

2021-11-02 21:55:21

by Ira Weiny

[permalink] [raw]
Subject: [PATCH 2/3] Documentation/auxiliary_bus: Clarify match_name

From: Ira Weiny <[email protected]>

Provide example code for how the match name is formed and where it is
supposed to be set.

Reviewed-by: Dave Jiang <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Documentation/driver-api/auxiliary_bus.rst | 33 ++++++++++++++++++++--
1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
index 8b3e795f3691..41ec9aed059f 100644
--- a/Documentation/driver-api/auxiliary_bus.rst
+++ b/Documentation/driver-api/auxiliary_bus.rst
@@ -78,6 +78,9 @@ An auxiliary_device represents a part of its parent device's functionality. It
is given a name that, combined with the registering drivers KBUILD_MODNAME,
creates a match_name that is used for driver binding, and an id that combined
with the match_name provide a unique name to register with the bus subsystem.
+For example, a driver registering an auxiliary device is named 'foo_mod.ko' and
+the subdevice is named 'foo_dev'. The match name is therefore
+'foo_mod.foo_dev'.

.. code-block:: c

@@ -95,9 +98,9 @@ structure must be filled in as follows.

The 'name' field is to be given a name that is recognized by the auxiliary
driver. If two auxiliary_devices with the same match_name, eg
-"mod.MY_DEVICE_NAME", are registered onto the bus, they must have unique id
-values (e.g. "x" and "y") so that the registered devices names are "mod.foo.x"
-and "mod.foo.y". If match_name + id are not unique, then the device_add fails
+"foo_mod.foo_dev", are registered onto the bus, they must have unique id
+values (e.g. "x" and "y") so that the registered devices names are "foo_mod.foo_dev.x"
+and "foo_mod.foo_dev.y". If match_name + id are not unique, then the device_add fails
and generates an error message.

The auxiliary_device.dev.type.release or auxiliary_device.dev.release must be
@@ -121,6 +124,10 @@ device to the bus.

.. code-block:: c

+ #define MY_DEVICE_NAME "foo_dev"
+
+ ...
+
struct axiliary_device *my_aux_dev = my_aux_dev_alloc(xxx);

/* Step 1: */
@@ -139,6 +146,9 @@ device to the bus.
goto fail;
}

+ ...
+
+
Unregistering an auxiliary_device is a two-step process to mirror the register
process. First call auxiliary_device_delete(), then call
auxiliary_device_uninit().
@@ -205,6 +215,23 @@ Auxiliary drivers register themselves with the bus by calling
auxiliary_driver_register(). The id_table contains the match_names of auxiliary
devices that a driver can bind with.

+.. code-block:: c
+
+ static const struct auxiliary_device_id my_auxiliary_id_table[] = {
+ { .name = "foo_mod.foo_dev" },
+ {},
+ };
+
+ MODULE_DEVICE_TABLE(auxiliary, my_auxiliary_id_table);
+
+ struct auxiliary_driver my_drv = {
+ .name = "myauxiliarydrv",
+ .id_table = my_auxiliary_id_table,
+ .probe = my_drv_probe,
+ .remove = my_drv_remove
+ };
+
+
Example Usage
=============

--
2.28.0.rc0.12.gb6a658bd00c9

2021-11-03 07:59:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/3] Improve Auxiliary Device documentation

On Tue, Nov 02, 2021 at 02:53:14PM -0700, [email protected] wrote:
> From: Ira Weiny <[email protected]>
>
> The auxiliary device documentation was not wrong but it was a bit difficult to
> follow. Add clarifications to ensure that details are not missed.
>
>
> Ira Weiny (3):
> Documentation/auxiliary_bus: Clarify auxiliary_device creation
> Documentation/auxiliary_bus: Clarify match_name
> Documentation/auxiliary_bus: Update Auxiliary device lifespan
>
> Documentation/driver-api/auxiliary_bus.rst | 136 ++++++++++++++++-----

Any chance that we can move this documentation into the .c file itself
to make it easier to maintain and keep up to date over time? I think
the v4l2 and drm subsystems do this pretty well, right?

thanks,

greg k-h