Hi Bartosz, thanks for the review.
On Thu, 2020-11-05 at 10:13 +0100, Bartosz Golaszewski wrote:
> > +/**
> > + * 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 platform_device *pdev = of_find_device_by_node(firmware_node);
> > + struct rpi_firmware *fw;
> > +
> > + if (!pdev)
> > + return NULL;
> > +
> > + fw = platform_get_drvdata(pdev);
> > + if (!fw)
> > + return NULL;
> > +
> > + if (!refcount_inc_not_zero(&fw->consumers))
> > + return NULL;
> > +
> > + if (devm_add_action_or_reset(dev, rpi_firmware_put, fw))
> > + return NULL;
> > +
> > + return fw;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get);
>
> Usually I'd expect the devres variant to simply call
> rpi_firmware_get() and then schedule a release callback which would
> call whatever function is the release counterpart for it currently.
> Devres actions are for drivers which want to schedule some more
> unusual tasks at driver detach. Any reason for designing it this way?
Yes, see patch #8 where I get rid of rpi_firmware_get() altogether after
converting all users to devres. Since there is no use for the vanilla version
of the function anymore, I figured it'd be better to merge everything into
devm_rpi_firmware_get(). That said it's not something I have strong feelings
about.
Regards,
Nicolas
Attachment:
signature.asc
Description: This is a digitally signed message part
