| Message ID | 20240325081957.10946-1-johan+linaro@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: qcom: gdsc: treat optional supplies as optional | expand |
On Mon, Mar 25, 2024 at 09:19:57AM +0100, Johan Hovold wrote: > Since commit deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external > supply for GX gdsc") the GDSC supply must be treated as optional to > avoid warnings like: > > gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator > > on SC8280XP. Can this device actually run with the supply physically disconnected?
On Mon, 25 Mar 2024 at 16:01, Mark Brown <broonie@kernel.org> wrote: > > On Mon, Mar 25, 2024 at 09:19:57AM +0100, Johan Hovold wrote: > > Since commit deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external > > supply for GX gdsc") the GDSC supply must be treated as optional to > > avoid warnings like: > > > > gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator > > > > on SC8280XP. > > Can this device actually run with the supply physically disconnected? On SC8280XP this is supplied via power-domain instead of the supply.
On 25.03.2024 3:10 PM, Dmitry Baryshkov wrote: > On Mon, 25 Mar 2024 at 16:01, Mark Brown <broonie@kernel.org> wrote: >> >> On Mon, Mar 25, 2024 at 09:19:57AM +0100, Johan Hovold wrote: >>> Since commit deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external >>> supply for GX gdsc") the GDSC supply must be treated as optional to >>> avoid warnings like: >>> >>> gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator >>> >>> on SC8280XP. >> >> Can this device actually run with the supply physically disconnected? > > On SC8280XP this is supplied via power-domain instead of the supply. I think Dmitry is asking about this bit: if (ret != -ENODEV) return ret; which is basically repeating the difference that _optional makes Konrad
On Mon, Mar 25, 2024 at 02:01:16PM +0000, Mark Brown wrote: > On Mon, Mar 25, 2024 at 09:19:57AM +0100, Johan Hovold wrote: > > Since commit deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external > > supply for GX gdsc") the GDSC supply must be treated as optional to > > avoid warnings like: > > > > gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator > > > > on SC8280XP. > > Can this device actually run with the supply physically disconnected? The gpucc-sc8280xp driver is used for both sc8280xp and a couple of derivative platforms. AFAIU only the latter use this supply, but the driver unfortunately currently cannot tell which platform it runs on and requests the vdd-gfx unconditionally. An alternative would have been to add new compatible strings for the derivate platforms and only request the regulator for those as I mentioned here: https://lore.kernel.org/all/ZgFGCGgbY-4Xd_2k@hovoldconsulting.com/ Johan
On Mon, Mar 25, 2024 at 08:21:24PM +0100, Konrad Dybcio wrote: > On 25.03.2024 3:10 PM, Dmitry Baryshkov wrote: > > On Mon, 25 Mar 2024 at 16:01, Mark Brown <broonie@kernel.org> wrote: > >> > >> On Mon, Mar 25, 2024 at 09:19:57AM +0100, Johan Hovold wrote: > >>> Since commit deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external > >>> supply for GX gdsc") the GDSC supply must be treated as optional to > >>> avoid warnings like: > >>> > >>> gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator > >>> > >>> on SC8280XP. > >> > >> Can this device actually run with the supply physically disconnected? > > > > On SC8280XP this is supplied via power-domain instead of the supply. > > I think Dmitry is asking about this bit: AFAICT, Dmitry did not ask anything. > if (ret != -ENODEV) > return ret; > > which is basically repeating the difference that _optional makes Not sure what you meant by this either. This is how the regulator subsystem works. Johan
On Tue, Mar 26, 2024 at 08:16:16AM +0100, Johan Hovold wrote: > An alternative would have been to add new compatible strings for the > derivate platforms and only request the regulator for those as I > mentioned here: > https://lore.kernel.org/all/ZgFGCGgbY-4Xd_2k@hovoldconsulting.com/ Ah, yes - that would be much better.
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index e7a4068b9f39..df9618ab7eea 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -487,9 +487,14 @@ int gdsc_register(struct gdsc_desc *desc, if (!scs[i] || !scs[i]->supply) continue; - scs[i]->rsupply = devm_regulator_get(dev, scs[i]->supply); - if (IS_ERR(scs[i]->rsupply)) - return PTR_ERR(scs[i]->rsupply); + scs[i]->rsupply = devm_regulator_get_optional(dev, scs[i]->supply); + if (IS_ERR(scs[i]->rsupply)) { + ret = PTR_ERR(scs[i]->rsupply); + if (ret != -ENODEV) + return ret; + + scs[i]->rsupply = NULL; + } } data->num_domains = num;
Since commit deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external supply for GX gdsc") the GDSC supply must be treated as optional to avoid warnings like: gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator on SC8280XP. Fortunately, the driver is already prepared to handle this by checking that the regulator pointer is non-NULL before use. This also avoids triggering a potential deadlock on SC8280XP even if the underlying issue still remains for the derivative platforms like SA8295P that actually use the supply. Fixes: deebc79b28d6 ("clk: qcom: gpucc-sc8280xp: Add external supply for GX gdsc") Link: https://lore.kernel.org/lkml/Zf25Sv2x9WaCFuIH@hovoldconsulting.com/ Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/clk/qcom/gdsc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)