Messages in this thread | | | Date | Tue, 10 Oct 2023 12:02:56 +0100 | From | Sudeep Holla <> | Subject | Re: [PATCH] power: reset: vexpress: Use device_get_match_data() |
| |
On Mon, Oct 09, 2023 at 11:07:33PM +0100, Liviu Dudau wrote: > Hi Rob, > > On Mon, Oct 09, 2023 at 12:29:14PM -0500, Rob Herring wrote: > > Use preferred device_get_match_data() instead of of_match_device() to > > get the driver match data. With this, adjust the includes to explicitly > > include the correct headers. > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > drivers/power/reset/vexpress-poweroff.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c > > index 17064d7b19f6..bb22b2db5907 100644 > > --- a/drivers/power/reset/vexpress-poweroff.c > > +++ b/drivers/power/reset/vexpress-poweroff.c > > @@ -7,8 +7,8 @@ > > #include <linux/delay.h> > > #include <linux/notifier.h> > > #include <linux/of.h> > > -#include <linux/of_device.h> > > #include <linux/platform_device.h> > > +#include <linux/property.h> > > #include <linux/reboot.h> > > #include <linux/stat.h> > > #include <linux/vexpress.h> > > @@ -108,20 +108,17 @@ static int _vexpress_register_restart_handler(struct device *dev) > > > > static int vexpress_reset_probe(struct platform_device *pdev) > > { > > - const struct of_device_id *match = > > - of_match_device(vexpress_reset_of_match, &pdev->dev); > > + enum vexpress_reset_func func; > > struct regmap *regmap; > > int ret = 0; > > > > - if (!match) > > - return -EINVAL; > > - > > regmap = devm_regmap_init_vexpress_config(&pdev->dev); > > if (IS_ERR(regmap)) > > return PTR_ERR(regmap); > > dev_set_drvdata(&pdev->dev, regmap); > > > > - switch ((uintptr_t)match->data) { > > + func = (uintptr_t)device_get_match_data(&pdev->dev); > > + switch (func) { > > case FUNC_SHUTDOWN: > > vexpress_power_off_device = &pdev->dev; > > pm_power_off = vexpress_power_off; > > device_get_match_data() is badly un-documented but I think it can still > return NULL if no match. At the moment we're checking for a match earlier > and avoid calling devm_regmap_init_vexpress_config() needlessly, with your > patch not so. Can you not replace each line with the equivalent code and > keep the NULL check?
In contrast, I always questioned/wondered why we needed the NULL check for the match above as probe would have not got called if there was no match. While I agree with the less clear documentation of device_get_match_data(), I am fine with the change as such
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Also check similar response from Rob here[1]
Sebastian,
I assume you will pick this up.
-- Regards, Sudeep
[1] https://lore.kernel.org/all/CAL_JsqKi8PXVqHgVxqMN+mbX8U-ZGsCMUFqbxmjeFmj1xKTrjw@mail.gmail.com/
| |