| Message ID | 20230111113018.459199-4-gregkh@linuxfoundation.org (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | None | expand |
Hi, On 1/11/23 12:30, Greg Kroah-Hartman wrote: > The driver core is changing to pass some pointers as const, so move > to_ssam_device() to use container_of_const() to handle this change. > to_ssam_device() now properly keeps the const-ness of the pointer passed > into it, while as before it could be lost. > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > Cc: platform-driver-x86@vger.kernel.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > include/linux/surface_aggregator/device.h | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h > index 46c45d1b6368..24151a0e2c96 100644 > --- a/include/linux/surface_aggregator/device.h > +++ b/include/linux/surface_aggregator/device.h > @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) > * Return: Returns a pointer to the &struct ssam_device wrapping the given > * device @d. > */ > -static inline struct ssam_device *to_ssam_device(struct device *d) > -{ > - return container_of(d, struct ssam_device, dev); > -} > +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) There is a kernel-doc comment for this helper, I'm not sure if sphinx is going to like changing this from a static inline to a #define ? At a minimum I guess the generated docs are going to loose some type info by switching to a define. Can we keep this as a static inline ? Regards, Hans > > /** > * to_ssam_device_driver() - Casts the given device driver to a SSAM client
On 1/11/23 12:30, Greg Kroah-Hartman wrote: > The driver core is changing to pass some pointers as const, so move > to_ssam_device() to use container_of_const() to handle this change. > to_ssam_device() now properly keeps the const-ness of the pointer passed > into it, while as before it could be lost. > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > Cc: platform-driver-x86@vger.kernel.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > include/linux/surface_aggregator/device.h | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h > index 46c45d1b6368..24151a0e2c96 100644 > --- a/include/linux/surface_aggregator/device.h > +++ b/include/linux/surface_aggregator/device.h > @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) > * Return: Returns a pointer to the &struct ssam_device wrapping the given > * device @d. > */ > -static inline struct ssam_device *to_ssam_device(struct device *d) > -{ > - return container_of(d, struct ssam_device, dev); > -} > +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) > > /** > * to_ssam_device_driver() - Casts the given device driver to a SSAM client A small note in addition to the comment by Hans: I think it might be a good idea to convert the to_ssam_device_driver() function as well. I could also send this in as a follow-up patch. Regards, Max
On Wed, Jan 11, 2023 at 12:54:54PM +0100, Hans de Goede wrote: > Hi, > > On 1/11/23 12:30, Greg Kroah-Hartman wrote: > > The driver core is changing to pass some pointers as const, so move > > to_ssam_device() to use container_of_const() to handle this change. > > to_ssam_device() now properly keeps the const-ness of the pointer passed > > into it, while as before it could be lost. > > > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > Cc: platform-driver-x86@vger.kernel.org > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > include/linux/surface_aggregator/device.h | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h > > index 46c45d1b6368..24151a0e2c96 100644 > > --- a/include/linux/surface_aggregator/device.h > > +++ b/include/linux/surface_aggregator/device.h > > @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) > > * Return: Returns a pointer to the &struct ssam_device wrapping the given > > * device @d. > > */ > > -static inline struct ssam_device *to_ssam_device(struct device *d) > > -{ > > - return container_of(d, struct ssam_device, dev); > > -} > > +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) > > There is a kernel-doc comment for this helper, I'm not sure if sphinx is going > to like changing this from a static inline to a #define ? > > At a minimum I guess the generated docs are going to loose some type info > by switching to a define. Can we keep this as a static inline ? No, container_of_const() will not work properly as an inline function as you don't know the return type until the preprocessor runs. I thought kerneldoc should still work just fine here as it does work for defines in other places. thanks, greg k-h
On Wed, Jan 11, 2023 at 01:02:01PM +0100, Maximilian Luz wrote: > On 1/11/23 12:30, Greg Kroah-Hartman wrote: > > The driver core is changing to pass some pointers as const, so move > > to_ssam_device() to use container_of_const() to handle this change. > > to_ssam_device() now properly keeps the const-ness of the pointer passed > > into it, while as before it could be lost. > > > > Cc: Maximilian Luz <luzmaximilian@gmail.com> > > Cc: platform-driver-x86@vger.kernel.org > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > include/linux/surface_aggregator/device.h | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h > > index 46c45d1b6368..24151a0e2c96 100644 > > --- a/include/linux/surface_aggregator/device.h > > +++ b/include/linux/surface_aggregator/device.h > > @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) > > * Return: Returns a pointer to the &struct ssam_device wrapping the given > > * device @d. > > */ > > -static inline struct ssam_device *to_ssam_device(struct device *d) > > -{ > > - return container_of(d, struct ssam_device, dev); > > -} > > +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) > > /** > > * to_ssam_device_driver() - Casts the given device driver to a SSAM client > > A small note in addition to the comment by Hans: I think it might be a > good idea to convert the to_ssam_device_driver() function as well. I > could also send this in as a follow-up patch. Sure, that can be a follow-on patch as it's not needed here. thanks, greg k-h
Hi, On 1/11/23 13:14, Greg Kroah-Hartman wrote: > On Wed, Jan 11, 2023 at 12:54:54PM +0100, Hans de Goede wrote: >> Hi, >> >> On 1/11/23 12:30, Greg Kroah-Hartman wrote: >>> The driver core is changing to pass some pointers as const, so move >>> to_ssam_device() to use container_of_const() to handle this change. >>> to_ssam_device() now properly keeps the const-ness of the pointer passed >>> into it, while as before it could be lost. >>> >>> Cc: Maximilian Luz <luzmaximilian@gmail.com> >>> Cc: platform-driver-x86@vger.kernel.org >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> --- >>> include/linux/surface_aggregator/device.h | 5 +---- >>> 1 file changed, 1 insertion(+), 4 deletions(-) >>> >>> diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h >>> index 46c45d1b6368..24151a0e2c96 100644 >>> --- a/include/linux/surface_aggregator/device.h >>> +++ b/include/linux/surface_aggregator/device.h >>> @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) >>> * Return: Returns a pointer to the &struct ssam_device wrapping the given >>> * device @d. >>> */ >>> -static inline struct ssam_device *to_ssam_device(struct device *d) >>> -{ >>> - return container_of(d, struct ssam_device, dev); >>> -} >>> +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) >> >> There is a kernel-doc comment for this helper, I'm not sure if sphinx is going >> to like changing this from a static inline to a #define ? >> >> At a minimum I guess the generated docs are going to loose some type info >> by switching to a define. Can we keep this as a static inline ? > > No, container_of_const() will not work properly as an inline function as > you don't know the return type until the preprocessor runs. I thought > kerneldoc should still work just fine here as it does work for defines > in other places. Ok then this is fine as is. Thanks for clarifying why the switch to a #define is necessary. Regards, Hans
On 1/11/23 13:14, Greg Kroah-Hartman wrote: > On Wed, Jan 11, 2023 at 01:02:01PM +0100, Maximilian Luz wrote: >> On 1/11/23 12:30, Greg Kroah-Hartman wrote: >>> The driver core is changing to pass some pointers as const, so move >>> to_ssam_device() to use container_of_const() to handle this change. >>> to_ssam_device() now properly keeps the const-ness of the pointer passed >>> into it, while as before it could be lost. >>> >>> Cc: Maximilian Luz <luzmaximilian@gmail.com> >>> Cc: platform-driver-x86@vger.kernel.org >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> --- >>> include/linux/surface_aggregator/device.h | 5 +---- >>> 1 file changed, 1 insertion(+), 4 deletions(-) >>> >>> diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h >>> index 46c45d1b6368..24151a0e2c96 100644 >>> --- a/include/linux/surface_aggregator/device.h >>> +++ b/include/linux/surface_aggregator/device.h >>> @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) >>> * Return: Returns a pointer to the &struct ssam_device wrapping the given >>> * device @d. >>> */ >>> -static inline struct ssam_device *to_ssam_device(struct device *d) >>> -{ >>> - return container_of(d, struct ssam_device, dev); >>> -} >>> +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) >>> /** >>> * to_ssam_device_driver() - Casts the given device driver to a SSAM client >> >> A small note in addition to the comment by Hans: I think it might be a >> good idea to convert the to_ssam_device_driver() function as well. I >> could also send this in as a follow-up patch. > > Sure, that can be a follow-on patch as it's not needed here. Perfect, I'll do that. As Hans' comment is now addressed, everything looks good to me. Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h index 46c45d1b6368..24151a0e2c96 100644 --- a/include/linux/surface_aggregator/device.h +++ b/include/linux/surface_aggregator/device.h @@ -229,10 +229,7 @@ static inline bool is_ssam_device(struct device *d) * Return: Returns a pointer to the &struct ssam_device wrapping the given * device @d. */ -static inline struct ssam_device *to_ssam_device(struct device *d) -{ - return container_of(d, struct ssam_device, dev); -} +#define to_ssam_device(d) container_of_const(d, struct ssam_device, dev) /** * to_ssam_device_driver() - Casts the given device driver to a SSAM client
The driver core is changing to pass some pointers as const, so move to_ssam_device() to use container_of_const() to handle this change. to_ssam_device() now properly keeps the const-ness of the pointer passed into it, while as before it could be lost. Cc: Maximilian Luz <luzmaximilian@gmail.com> Cc: platform-driver-x86@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- include/linux/surface_aggregator/device.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)