| Message ID | 20201112163630.17177-3-nsaenzjulienne@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Raspberry Pi PoE HAT fan support | expand |
On Thu, Nov 12, 2020 at 5:44 PM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Itroduce devm_rpi_firmware_get(), it'll simplify the firmware handling > for most consumers. > > Suggested-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > --- > > Changes since v2: > - Introduce devm_rpi_firmware_get() > > drivers/firmware/raspberrypi.c | 31 +++++++++++++++++++++- > include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++ > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c > index 438e17074a97..4ab2dfdc82ad 100644 > --- a/drivers/firmware/raspberrypi.c > +++ b/drivers/firmware/raspberrypi.c > @@ -237,10 +237,17 @@ static void rpi_firmware_delete(struct kref *kref) > kfree(fw); > } > > -void rpi_firmware_put(struct rpi_firmware *fw) > +static void __rpi_firmware_put(void *data) > { The '__' prefix is very vague and usually used for unlocked variants of functions. The casting to void * in rpi_firmware_put() is also unneeded. I would much prefer that the devres release callback be called devm_rpi_firmware_put() and that it call rpi_firmware_put() which would then call kref_put(). Bartosz > + struct rpi_firmware *fw = data; > + > kref_put(&fw->consumers, rpi_firmware_delete); > } > + > +void rpi_firmware_put(struct rpi_firmware *fw) > +{ > + __rpi_firmware_put(fw); > +} > EXPORT_SYMBOL_GPL(rpi_firmware_put); > > static int rpi_firmware_probe(struct platform_device *pdev) > @@ -326,6 +333,28 @@ struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node) > } > EXPORT_SYMBOL_GPL(rpi_firmware_get); > > +/** > + * devm_rpi_firmware_get - Get pointer to rpi_firmware structure. > + * @firmware_node: Pointer to the firmware Device Tree node. > + * > + * Returns NULL is the firmware device is not ready. > + */ > +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, > + struct device_node *firmware_node) > +{ > + struct rpi_firmware *fw; > + > + fw = rpi_firmware_get(firmware_node); > + if (!fw) > + return NULL; > + > + if (devm_add_action_or_reset(dev, __rpi_firmware_put, fw)) > + return NULL; > + > + return fw; > +} > +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get); > + > static const struct of_device_id rpi_firmware_of_match[] = { > { .compatible = "raspberrypi,bcm2835-firmware", }, > {}, > diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h > index fdfef7fe40df..73ad784fca96 100644 > --- a/include/soc/bcm2835/raspberrypi-firmware.h > +++ b/include/soc/bcm2835/raspberrypi-firmware.h > @@ -142,6 +142,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw, > void *data, size_t tag_size); > void rpi_firmware_put(struct rpi_firmware *fw); > struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node); > +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, > + struct device_node *firmware_node); > #else > static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag, > void *data, size_t len) > @@ -160,6 +162,12 @@ static inline struct rpi_firmware *rpi_firmware_get(struct device_node *firmware > { > return NULL; > } > + > +static inline struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, > + struct device_node *firmware_node) > +{ > + return NULL; > +} > #endif > > #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */ > -- > 2.29.2 >
On Thu, 2020-11-12 at 18:25 +0100, Bartosz Golaszewski wrote: > On Thu, Nov 12, 2020 at 5:44 PM Nicolas Saenz Julienne > <nsaenzjulienne@suse.de> wrote: > > Itroduce devm_rpi_firmware_get(), it'll simplify the firmware handling > > for most consumers. > > > > Suggested-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > --- > > > > Changes since v2: > > - Introduce devm_rpi_firmware_get() > > > > drivers/firmware/raspberrypi.c | 31 +++++++++++++++++++++- > > include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++ > > 2 files changed, 38 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c > > index 438e17074a97..4ab2dfdc82ad 100644 > > --- a/drivers/firmware/raspberrypi.c > > +++ b/drivers/firmware/raspberrypi.c > > @@ -237,10 +237,17 @@ static void rpi_firmware_delete(struct kref *kref) > > kfree(fw); > > } > > > > -void rpi_firmware_put(struct rpi_firmware *fw) > > +static void __rpi_firmware_put(void *data) > > { > > The '__' prefix is very vague and usually used for unlocked variants > of functions. The casting to void * in rpi_firmware_put() is also > unneeded. I would much prefer that the devres release callback be > called devm_rpi_firmware_put() and that it call rpi_firmware_put() > which would then call kref_put(). Yes, that's better. I'll change it. Regards, Nicolas
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c index 438e17074a97..4ab2dfdc82ad 100644 --- a/drivers/firmware/raspberrypi.c +++ b/drivers/firmware/raspberrypi.c @@ -237,10 +237,17 @@ static void rpi_firmware_delete(struct kref *kref) kfree(fw); } -void rpi_firmware_put(struct rpi_firmware *fw) +static void __rpi_firmware_put(void *data) { + struct rpi_firmware *fw = data; + kref_put(&fw->consumers, rpi_firmware_delete); } + +void rpi_firmware_put(struct rpi_firmware *fw) +{ + __rpi_firmware_put(fw); +} EXPORT_SYMBOL_GPL(rpi_firmware_put); static int rpi_firmware_probe(struct platform_device *pdev) @@ -326,6 +333,28 @@ struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node) } EXPORT_SYMBOL_GPL(rpi_firmware_get); +/** + * devm_rpi_firmware_get - Get pointer to rpi_firmware structure. + * @firmware_node: Pointer to the firmware Device Tree node. + * + * Returns NULL is the firmware device is not ready. + */ +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, + struct device_node *firmware_node) +{ + struct rpi_firmware *fw; + + fw = rpi_firmware_get(firmware_node); + if (!fw) + return NULL; + + if (devm_add_action_or_reset(dev, __rpi_firmware_put, fw)) + return NULL; + + return fw; +} +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get); + static const struct of_device_id rpi_firmware_of_match[] = { { .compatible = "raspberrypi,bcm2835-firmware", }, {}, diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h index fdfef7fe40df..73ad784fca96 100644 --- a/include/soc/bcm2835/raspberrypi-firmware.h +++ b/include/soc/bcm2835/raspberrypi-firmware.h @@ -142,6 +142,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw, void *data, size_t tag_size); void rpi_firmware_put(struct rpi_firmware *fw); struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node); +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, + struct device_node *firmware_node); #else static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag, void *data, size_t len) @@ -160,6 +162,12 @@ static inline struct rpi_firmware *rpi_firmware_get(struct device_node *firmware { return NULL; } + +static inline struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, + struct device_node *firmware_node) +{ + return NULL; +} #endif #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
Itroduce devm_rpi_firmware_get(), it'll simplify the firmware handling for most consumers. Suggested-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> --- Changes since v2: - Introduce devm_rpi_firmware_get() drivers/firmware/raspberrypi.c | 31 +++++++++++++++++++++- include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++++ 2 files changed, 38 insertions(+), 1 deletion(-)