Re: [PATCH V3 02/20] OPP: Make dev_pm_opp_set_regulators() accept NULL terminated list
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: 2022-07-05 04:34:50
Also in:
dri-devel, linux-pm, linux-samsung-soc, linux-tegra, lkml
Subsystem:
arm mali panfrost drm driver, drm drivers, drm drivers and misc gpu patches, the rest · Maintainers:
Boris Brezillon, Rob Herring, Steven Price, Adrián Larumbe, David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Linus Torvalds
On 04-07-22, 15:35, Steven Price wrote:
I have to say the 'new improved' list ending with NULL approach doesn't
work out so well for Panfrost. We already have to have a separate
'num_supplies' variable for devm_regulator_bulk_get() /
regulator_bulk_{en,dis}able(), so the keeping everything in sync
argument is lost here.
I would suggest added the NULL on the end of the lists in panfrost_drv.c
but then it would break the use of ARRAY_SIZE() to automagically keep
the length correct...Actually we can still make it work.
For now the approach isn't too bad because Panfrost doesn't yet support enabling devfreq with more than one supply. But that array isn't going to work so nicely when that restriction is removed. The only sane way I can see of handling this in Panfrost would be replicating the loop to count the supplies in the Panfrost code which would allow dropping num_supplies from struct panfrost_compatible and then supply_names in the same struct could be NULL terminated ready for devm_pm_opp_set_regulators().
Or doing this, which will simplify both the cases.
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 7fcbc2a5b6cd..b3b55565b8ef 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c@@ -625,24 +625,29 @@ static int panfrost_remove(struct platform_device *pdev) return 0; } -static const char * const default_supplies[] = { "mali" }; +/* + * The OPP core wants the supply names to be NULL terminated, but we need the + * correct num_supplies value for regulator core. Hence, we NULL terminate here + * and then initialize num_supplies with ARRAY_SIZE - 1. + */ +static const char * const default_supplies[] = { "mali", NULL }; static const struct panfrost_compatible default_data = { - .num_supplies = ARRAY_SIZE(default_supplies), + .num_supplies = ARRAY_SIZE(default_supplies) - 1, .supply_names = default_supplies, .num_pm_domains = 1, /* optional */ .pm_domain_names = NULL, }; static const struct panfrost_compatible amlogic_data = { - .num_supplies = ARRAY_SIZE(default_supplies), + .num_supplies = ARRAY_SIZE(default_supplies) - 1, .supply_names = default_supplies, .vendor_quirk = panfrost_gpu_amlogic_quirk, }; -static const char * const mediatek_mt8183_supplies[] = { "mali", "sram" }; +static const char * const mediatek_mt8183_supplies[] = { "mali", "sram", NULL }; static const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" }; static const struct panfrost_compatible mediatek_mt8183_data = { - .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies), + .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies) - 1, .supply_names = mediatek_mt8183_supplies, .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains), .pm_domain_names = mediatek_mt8183_pm_domains,
--
viresh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel