From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Mon, 10 May 2021 19:28:57 -0400 Subject: [PATCH 16/17] clk: Detect failure to set defaults In-Reply-To: References: <20210508220021.1778080-1-sjg@chromium.org> <20210508220021.1778080-17-sjg@chromium.org> Message-ID: <12dbe672-75fe-c5d4-ca73-2ef72428733a@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 5/10/21 12:28 PM, Simon Glass wrote: > Hi Sean, > > On Sat, 8 May 2021 at 18:40, Sean Anderson wrote: >> >> On 5/8/21 6:00 PM, Simon Glass wrote: >>> When the default clocks cannot be set, the clock is silently probed and >>> the error is ignored. This is incorrect, since having the clocks at the >>> correct speed may be important for operation of the system. >>> >>> Fix it by checking the return code. >>> >>> Signed-off-by: Simon Glass >>> --- >>> >>> drivers/clk/clk-uclass.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c >>> index 4ab3c402ed8..2a2e1cfbd61 100644 >>> --- a/drivers/clk/clk-uclass.c >>> +++ b/drivers/clk/clk-uclass.c >>> @@ -796,13 +796,17 @@ void devm_clk_put(struct udevice *dev, struct clk *clk) >>> >>> int clk_uclass_post_probe(struct udevice *dev) >>> { >>> + int ret; >>> + >>> /* >>> * when a clock provider is probed. Call clk_set_defaults() >>> * also after the device is probed. This takes care of cases >>> * where the DT is used to setup default parents and rates >>> * using assigned-clocks >>> */ >>> - clk_set_defaults(dev, 1); >>> + ret = clk_set_defaults(dev, 1); >>> + if (ret) >>> + return log_ret(ret); >>> >>> return 0; >>> } >>> >> >> See also: https://patchwork.ozlabs.org/project/uboot/patch/20210409021313.433558-2-seanga2 at gmail.com/ > > So which should we do? My feeling is that a failure that is > programmatically silent could cause things to fail, but is there a > reason why this might be wrong but everything is still OK? I think both are fine. I think this is definitely a situation where we should complain loudly so that when things break there is some relevant output. --Sean > > Regards, > Simon > >> Reviewed-by: Sean Anderson