diff mbox series

[v5,003/111] pwm: Provide a macro to get the parent device of a given chip

Message ID 1cae6f73264ab313205eaa9483251f7aaf259cb4.1706182805.git.u.kleine-koenig@pengutronix.de (mailing list archive)
State New
Headers show
Series pwm: Improve lifetime tracking for pwm_chips | expand

Commit Message

Uwe Kleine-König Jan. 25, 2024, 12:08 p.m. UTC
Currently a pwm_chip stores in its struct device *dev member a pointer
to the parent device. Preparing a change that embeds a full struct
device in struct pwm_chip, this accessor macro should be used in all
drivers directly accessing chip->dev now. This way struct pwm_chip and
this macro can be changed without having to touch all drivers in the
same change set.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 include/linux/pwm.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

AngeloGioacchino Del Regno Jan. 25, 2024, 2:14 p.m. UTC | #1
Il 25/01/24 13:08, Uwe Kleine-König ha scritto:
> Currently a pwm_chip stores in its struct device *dev member a pointer
> to the parent device. Preparing a change that embeds a full struct
> device in struct pwm_chip, this accessor macro should be used in all
> drivers directly accessing chip->dev now. This way struct pwm_chip and
> this macro can be changed without having to touch all drivers in the
> same change set.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   include/linux/pwm.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 8ffe9ae7a23a..d7966918f301 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -289,6 +289,11 @@ struct pwm_chip {
>   	struct pwm_device *pwms;
>   };
>   
> +static inline struct device *pwmchip_parent(struct pwm_chip *chip)
> +{
> +	return chip->dev;
> +}
> +
>   #if IS_ENABLED(CONFIG_PWM)
>   /* PWM user APIs */
>   int pwm_apply_might_sleep(struct pwm_device *pwm, const struct pwm_state *state);
Florian Fainelli Jan. 25, 2024, 7:32 p.m. UTC | #2
On 1/25/24 04:08, Uwe Kleine-König wrote:
> Currently a pwm_chip stores in its struct device *dev member a pointer
> to the parent device. Preparing a change that embeds a full struct
> device in struct pwm_chip, this accessor macro should be used in all
> drivers directly accessing chip->dev now. This way struct pwm_chip and
> this macro can be changed without having to touch all drivers in the
> same change set.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Nit: this is not a macro but an inline function.
Uwe Kleine-König Jan. 25, 2024, 8:29 p.m. UTC | #3
On Thu, Jan 25, 2024 at 11:32:47AM -0800, Florian Fainelli wrote:
> On 1/25/24 04:08, Uwe Kleine-König wrote:
> > Currently a pwm_chip stores in its struct device *dev member a pointer
> > to the parent device. Preparing a change that embeds a full struct
> > device in struct pwm_chip, this accessor macro should be used in all
> > drivers directly accessing chip->dev now. This way struct pwm_chip and
> > this macro can be changed without having to touch all drivers in the
> > same change set.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Nit: this is not a macro but an inline function.

Oh right, it used to be a macro, but I changed that. I made the commit
log read:

    pwm: Provide an inline function to get the parent device of a given chip

    Currently a pwm_chip stores in its struct device *dev member a pointer
    to the parent device. Preparing a change that embeds a full struct
    device in struct pwm_chip, this accessor function should be used in all
    drivers directly accessing chip->dev now. This way struct pwm_chip and
    this new function can be changed without having to touch all drivers in
    the same change set.

Thanks,
Uwe
Uwe Kleine-König Jan. 25, 2024, 8:54 p.m. UTC | #4
On Thu, Jan 25, 2024 at 09:29:37PM +0100, Uwe Kleine-König wrote:
> On Thu, Jan 25, 2024 at 11:32:47AM -0800, Florian Fainelli wrote:
> > On 1/25/24 04:08, Uwe Kleine-König wrote:
> > > Currently a pwm_chip stores in its struct device *dev member a pointer
> > > to the parent device. Preparing a change that embeds a full struct
> > > device in struct pwm_chip, this accessor macro should be used in all
> > > drivers directly accessing chip->dev now. This way struct pwm_chip and
> > > this macro can be changed without having to touch all drivers in the
> > > same change set.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > Nit: this is not a macro but an inline function.
> 
> Oh right, it used to be a macro, but I changed that. I made the commit
> log read:
> 
>     pwm: Provide an inline function to get the parent device of a given chip
> 
>     Currently a pwm_chip stores in its struct device *dev member a pointer
>     to the parent device. Preparing a change that embeds a full struct
>     device in struct pwm_chip, this accessor function should be used in all
>     drivers directly accessing chip->dev now. This way struct pwm_chip and
>     this new function can be changed without having to touch all drivers in
>     the same change set.

While looking into further feedback, I noticed I did the same mistake in
all the patches that convert the drivers to use this function. I did

	git filter-branch --msg-filter 'sed "s/Make use of pwmchip_parent() macro/Make use of pwmchip_parent() accessor/; s/commit as struct pwm_chip::dev, use the macro/commit as struct pwm_chip::dev, use the accessor/; s/provided for exactly this purpose./function provided for exactly this purpose./"' linus/master..

on my branch to make the typical commit log read:

	pwm: atmel: Make use of pwmchip_parent() accessor
	
	struct pwm_chip::dev is about to change. To not have to touch this
	driver in the same commit as struct pwm_chip::dev, use the accessor
	function provided for exactly this purpose.

I wont resend the whole series if this is the only change to it.

Best regards
Uwe
diff mbox series

Patch

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 8ffe9ae7a23a..d7966918f301 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -289,6 +289,11 @@  struct pwm_chip {
 	struct pwm_device *pwms;
 };
 
+static inline struct device *pwmchip_parent(struct pwm_chip *chip)
+{
+	return chip->dev;
+}
+
 #if IS_ENABLED(CONFIG_PWM)
 /* PWM user APIs */
 int pwm_apply_might_sleep(struct pwm_device *pwm, const struct pwm_state *state);