From: Ira Weiny <[email protected]>
The documentation for creating an auxiliary device is a 3 step not a 2
step process. Specifically the requirements of setting the name, id,
dev.release, and dev.parent fields was not clear as a precursor to the '2
step' process documented.
Clarify by declaring this a 3 step process starting with setting the
fields of struct auxiliary_device correctly.
Also add some sample code and tie the change into the rest of the
documentation.
Reviewed-by: Dave Jiang <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Documentation/driver-api/auxiliary_bus.rst | 77 +++++++++++++++++-----
1 file changed, 59 insertions(+), 18 deletions(-)
diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
index ef902daf0d68..8b3e795f3691 100644
--- a/Documentation/driver-api/auxiliary_bus.rst
+++ b/Documentation/driver-api/auxiliary_bus.rst
@@ -79,18 +79,6 @@ 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.
-Registering an auxiliary_device is a two-step process. First call
-auxiliary_device_init(), which checks several aspects of the auxiliary_device
-struct and performs a device_initialize(). After this step completes, any
-error state must have a call to auxiliary_device_uninit() in its resolution path.
-The second step in registering an auxiliary_device is to perform a call to
-auxiliary_device_add(), which sets the name of the device and add the device to
-the bus.
-
-Unregistering an auxiliary_device is also a two-step process to mirror the
-register process. First call auxiliary_device_delete(), then call
-auxiliary_device_uninit().
-
.. code-block:: c
struct auxiliary_device {
@@ -99,15 +87,68 @@ auxiliary_device_uninit().
u32 id;
};
-If two auxiliary_devices both with a match_name "mod.foo" 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 and generates an error message.
+Registering an auxiliary_device is a three-step process.
+
+First, a 'struct auxiliary_device' needs to be defined or allocated for each
+sub-device desired. The name, id, dev.release, and dev.parent fields of this
+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
+and generates an error message.
The auxiliary_device.dev.type.release or auxiliary_device.dev.release must be
-populated with a non-NULL pointer to successfully register the auxiliary_device.
+populated with a non-NULL pointer to successfully register the
+auxiliary_device. This release call is where resources associated with the
+auxiliary device must be free'ed. Because once the device is placed on the bus
+the parent driver can not tell what other code may have a reference to this
+data.
+
+The auxiliary_device.dev.parent should be set. Typically to the registering
+drivers device.
+
+Second, call auxiliary_device_init(), which checks several aspects of the
+auxiliary_device struct and performs a device_initialize(). After this step
+completes, any error state must have a call to auxiliary_device_uninit() in its
+resolution path.
+
+The third and final step in registering an auxiliary_device is to perform a
+call to auxiliary_device_add(), which sets the name of the device and adds the
+device to the bus.
+
+.. code-block:: c
+
+ struct axiliary_device *my_aux_dev = my_aux_dev_alloc(xxx);
+
+ /* Step 1: */
+ my_aux_dev->name = MY_DEVICE_NAME;
+ my_aux_dev->id = my_unique_id_alloc(xxx);
+ my_aux_dev->dev.release = my_aux_dev_release;
+ my_aux_dev->dev.parent = my_dev;
+
+ /* Step 2: */
+ if (auxiliary_device_init(my_aux_dev))
+ goto fail;
+
+ /* Step 3: */
+ if (auxiliary_device_add(my_aux_dev)) {
+ auxiliary_device_uninit(my_aux_dev);
+ 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().
+
+
+.. code-block:: c
+
+ auxiliary_device_delete(my_dev->my_aux_dev);
+ auxiliary_device_uninit(my_dev->my_aux_dev);
-The auxiliary_device.dev.parent must also be populated.
Auxiliary Device Memory Model and Lifespan
------------------------------------------
--
2.28.0.rc0.12.gb6a658bd00c9
[email protected] writes:
> From: Ira Weiny <[email protected]>
>
> The documentation for creating an auxiliary device is a 3 step not a 2
> step process. Specifically the requirements of setting the name, id,
> dev.release, and dev.parent fields was not clear as a precursor to the '2
> step' process documented.
>
> Clarify by declaring this a 3 step process starting with setting the
> fields of struct auxiliary_device correctly.
>
> Also add some sample code and tie the change into the rest of the
> documentation.
>
> Reviewed-by: Dave Jiang <[email protected]>
> Signed-off-by: Ira Weiny <[email protected]>
Looks generally good but ...
> Documentation/driver-api/auxiliary_bus.rst | 77 +++++++++++++++++-----
> 1 file changed, 59 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
> index ef902daf0d68..8b3e795f3691 100644
> --- a/Documentation/driver-api/auxiliary_bus.rst
> +++ b/Documentation/driver-api/auxiliary_bus.rst
> @@ -79,18 +79,6 @@ 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.
>
> -Registering an auxiliary_device is a two-step process. First call
> -auxiliary_device_init(), which checks several aspects of the auxiliary_device
> -struct and performs a device_initialize(). After this step completes, any
> -error state must have a call to auxiliary_device_uninit() in its resolution path.
> -The second step in registering an auxiliary_device is to perform a call to
> -auxiliary_device_add(), which sets the name of the device and add the device to
> -the bus.
> -
> -Unregistering an auxiliary_device is also a two-step process to mirror the
> -register process. First call auxiliary_device_delete(), then call
> -auxiliary_device_uninit().
> -
> .. code-block:: c
>
> struct auxiliary_device {
> @@ -99,15 +87,68 @@ auxiliary_device_uninit().
> u32 id;
> };
>
> -If two auxiliary_devices both with a match_name "mod.foo" 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 and generates an error message.
> +Registering an auxiliary_device is a three-step process.
> +
> +First, a 'struct auxiliary_device' needs to be defined or allocated for each
> +sub-device desired. The name, id, dev.release, and dev.parent fields of this
> +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
> +and generates an error message.
>
> The auxiliary_device.dev.type.release or auxiliary_device.dev.release must be
> -populated with a non-NULL pointer to successfully register the auxiliary_device.
> +populated with a non-NULL pointer to successfully register the
> +auxiliary_device. This release call is where resources associated with the
> +auxiliary device must be free'ed. Because once the device is placed on the bus
> +the parent driver can not tell what other code may have a reference to this
> +data.
> +
> +The auxiliary_device.dev.parent should be set. Typically to the registering
> +drivers device.
> +
> +Second, call auxiliary_device_init(), which checks several aspects of the
> +auxiliary_device struct and performs a device_initialize(). After this step
> +completes, any error state must have a call to auxiliary_device_uninit() in its
> +resolution path.
> +
> +The third and final step in registering an auxiliary_device is to perform a
> +call to auxiliary_device_add(), which sets the name of the device and adds the
> +device to the bus.
> +
> +.. code-block:: c
> +
> + struct axiliary_device *my_aux_dev = my_aux_dev_alloc(xxx);
This ----------^ would appear to be a typo...
Thanks,
jon
From: Ira Weiny <[email protected]>
The documentation for creating an auxiliary device is a 3 step not a 2
step process. Specifically the requirements of setting the name, id,
dev.release, and dev.parent fields was not clear as a precursor to the '2
step' process documented.
Clarify by declaring this a 3 step process starting with setting the
fields of struct auxiliary_device correctly.
Also add some sample code and tie the change into the rest of the
documentation.
Reviewed-by: Dave Jiang <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from V1:
From Jonathan
Fix auxiliary spelling
---
Documentation/driver-api/auxiliary_bus.rst | 77 +++++++++++++++++-----
1 file changed, 59 insertions(+), 18 deletions(-)
diff --git a/Documentation/driver-api/auxiliary_bus.rst b/Documentation/driver-api/auxiliary_bus.rst
index ef902daf0d68..cecfa2db46e6 100644
--- a/Documentation/driver-api/auxiliary_bus.rst
+++ b/Documentation/driver-api/auxiliary_bus.rst
@@ -79,18 +79,6 @@ 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.
-Registering an auxiliary_device is a two-step process. First call
-auxiliary_device_init(), which checks several aspects of the auxiliary_device
-struct and performs a device_initialize(). After this step completes, any
-error state must have a call to auxiliary_device_uninit() in its resolution path.
-The second step in registering an auxiliary_device is to perform a call to
-auxiliary_device_add(), which sets the name of the device and add the device to
-the bus.
-
-Unregistering an auxiliary_device is also a two-step process to mirror the
-register process. First call auxiliary_device_delete(), then call
-auxiliary_device_uninit().
-
.. code-block:: c
struct auxiliary_device {
@@ -99,15 +87,68 @@ auxiliary_device_uninit().
u32 id;
};
-If two auxiliary_devices both with a match_name "mod.foo" 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 and generates an error message.
+Registering an auxiliary_device is a three-step process.
+
+First, a 'struct auxiliary_device' needs to be defined or allocated for each
+sub-device desired. The name, id, dev.release, and dev.parent fields of this
+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
+and generates an error message.
The auxiliary_device.dev.type.release or auxiliary_device.dev.release must be
-populated with a non-NULL pointer to successfully register the auxiliary_device.
+populated with a non-NULL pointer to successfully register the
+auxiliary_device. This release call is where resources associated with the
+auxiliary device must be free'ed. Because once the device is placed on the bus
+the parent driver can not tell what other code may have a reference to this
+data.
+
+The auxiliary_device.dev.parent should be set. Typically to the registering
+drivers device.
+
+Second, call auxiliary_device_init(), which checks several aspects of the
+auxiliary_device struct and performs a device_initialize(). After this step
+completes, any error state must have a call to auxiliary_device_uninit() in its
+resolution path.
+
+The third and final step in registering an auxiliary_device is to perform a
+call to auxiliary_device_add(), which sets the name of the device and adds the
+device to the bus.
+
+.. code-block:: c
+
+ struct auxiliary_device *my_aux_dev = my_aux_dev_alloc(xxx);
+
+ /* Step 1: */
+ my_aux_dev->name = MY_DEVICE_NAME;
+ my_aux_dev->id = my_unique_id_alloc(xxx);
+ my_aux_dev->dev.release = my_aux_dev_release;
+ my_aux_dev->dev.parent = my_dev;
+
+ /* Step 2: */
+ if (auxiliary_device_init(my_aux_dev))
+ goto fail;
+
+ /* Step 3: */
+ if (auxiliary_device_add(my_aux_dev)) {
+ auxiliary_device_uninit(my_aux_dev);
+ 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().
+
+
+.. code-block:: c
+
+ auxiliary_device_delete(my_dev->my_aux_dev);
+ auxiliary_device_uninit(my_dev->my_aux_dev);
-The auxiliary_device.dev.parent must also be populated.
Auxiliary Device Memory Model and Lifespan
------------------------------------------
--
2.28.0.rc0.12.gb6a658bd00c9
On Tue, Nov 02, 2021 at 03:53:10PM -0700, [email protected] wrote:
> From: Ira Weiny <[email protected]>
>
> The documentation for creating an auxiliary device is a 3 step not a 2
> step process. Specifically the requirements of setting the name, id,
> dev.release, and dev.parent fields was not clear as a precursor to the '2
> step' process documented.
>
> Clarify by declaring this a 3 step process starting with setting the
> fields of struct auxiliary_device correctly.
>
> Also add some sample code and tie the change into the rest of the
> documentation.
>
> Reviewed-by: Dave Jiang <[email protected]>
> Signed-off-by: Ira Weiny <[email protected]>
>
> ---
> Changes from V1:
> From Jonathan
> Fix auxiliary spelling
> ---
> Documentation/driver-api/auxiliary_bus.rst | 77 +++++++++++++++++-----
> 1 file changed, 59 insertions(+), 18 deletions(-)
Can you please resend the whole series, trying to just resend one patch
in the middle is horrible for our tools and to try to figure this out.
Would you like to have to unwind this? Please make it simple for
maintainers to review and if ok, apply your changes.
thanks,
greg k-h
On Fri, Nov 26, 2021 at 05:16:54PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Nov 02, 2021 at 03:53:10PM -0700, [email protected] wrote:
> > From: Ira Weiny <[email protected]>
> >
> > The documentation for creating an auxiliary device is a 3 step not a 2
> > step process. Specifically the requirements of setting the name, id,
> > dev.release, and dev.parent fields was not clear as a precursor to the '2
> > step' process documented.
> >
> > Clarify by declaring this a 3 step process starting with setting the
> > fields of struct auxiliary_device correctly.
> >
> > Also add some sample code and tie the change into the rest of the
> > documentation.
> >
> > Reviewed-by: Dave Jiang <[email protected]>
> > Signed-off-by: Ira Weiny <[email protected]>
> >
> > ---
> > Changes from V1:
> > From Jonathan
> > Fix auxiliary spelling
> > ---
> > Documentation/driver-api/auxiliary_bus.rst | 77 +++++++++++++++++-----
> > 1 file changed, 59 insertions(+), 18 deletions(-)
>
> Can you please resend the whole series, trying to just resend one patch
> in the middle is horrible for our tools and to try to figure this out.
Sorry I did not realize this was an issue. Other maintainers have been ok with
this because I think B4 works fine with this?
>
> Would you like to have to unwind this? Please make it simple for
> maintainers to review and if ok, apply your changes.
Regardless, I was planning on resending this as part of the c files as you
requested before. Did you still want me to make that conversion?
Or I can resend this and make the c conversion as a follow on patch?
Ira
>
> thanks,
>
> greg k-h
On Sat, Nov 27, 2021 at 09:32:24PM -0800, Ira Weiny wrote:
> On Fri, Nov 26, 2021 at 05:16:54PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Nov 02, 2021 at 03:53:10PM -0700, [email protected] wrote:
> > > From: Ira Weiny <[email protected]>
> > >
> > > The documentation for creating an auxiliary device is a 3 step not a 2
> > > step process. Specifically the requirements of setting the name, id,
> > > dev.release, and dev.parent fields was not clear as a precursor to the '2
> > > step' process documented.
> > >
> > > Clarify by declaring this a 3 step process starting with setting the
> > > fields of struct auxiliary_device correctly.
> > >
> > > Also add some sample code and tie the change into the rest of the
> > > documentation.
> > >
> > > Reviewed-by: Dave Jiang <[email protected]>
> > > Signed-off-by: Ira Weiny <[email protected]>
> > >
> > > ---
> > > Changes from V1:
> > > From Jonathan
> > > Fix auxiliary spelling
> > > ---
> > > Documentation/driver-api/auxiliary_bus.rst | 77 +++++++++++++++++-----
> > > 1 file changed, 59 insertions(+), 18 deletions(-)
> >
> > Can you please resend the whole series, trying to just resend one patch
> > in the middle is horrible for our tools and to try to figure this out.
>
> Sorry I did not realize this was an issue. Other maintainers have been ok with
> this because I think B4 works fine with this?
>
> >
> > Would you like to have to unwind this? Please make it simple for
> > maintainers to review and if ok, apply your changes.
>
> Regardless, I was planning on resending this as part of the c files as you
> requested before. Did you still want me to make that conversion?
Yes.
> Or I can resend this and make the c conversion as a follow on patch?
That is up to you.