On Wed, Jan 05, 2022 at 08:56:50PM +0100, Marek Vasut wrote: > On 1/5/22 20:37, Tom Rini wrote: > > On Wed, Jan 05, 2022 at 08:35:19PM +0100, Marek Vasut wrote: > > > On 1/1/22 22:41, Sean Anderson wrote: > > > > Hi Marek, > > > > > > Hi, > > > > > > > Please CC clock maintainers for future patches. > > > > > > btw. I'm surprised the commit 92f1e9a4b31c0bf0f4f61ab823a6a88657323646 has > > > zero reviews/acks from clock maintainers. > > > > > > > On 1/1/22 1:51 PM, Marek Vasut wrote: > > > > > This reverts commit 92f1e9a4b31c0bf0f4f61ab823a6a88657323646. > > > > > The aforementioned patch causes massive breakage on all platforms > > > > > which > > > > > have 'assigned-clock' DT property in their DT which references any > > > > > clock > > > > > that are not supported by the platform clock driver. That can easily > > > > > happen either in SPL, or because the clock driver is reduced. > > > > > Currently > > > > > it seems all iMX8M are affected and fail to boot altogether. > > > > > > > > > > Signed-off-by: Marek Vasut <ma...@denx.de> > > > > > Cc: Peng Fan <peng....@oss.nxp.com> > > > > > Cc: Simon Glass <s...@chromium.org> > > > > > --- > > > > > drivers/clk/clk-uclass.c | 6 +----- > > > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > > > > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c > > > > > index f2d26427543..094b1abf13c 100644 > > > > > --- a/drivers/clk/clk-uclass.c > > > > > +++ b/drivers/clk/clk-uclass.c > > > > > @@ -846,17 +846,13 @@ 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 > > > > > */ > > > > > - ret = clk_set_defaults(dev, CLK_DEFAULTS_POST); > > > > > - if (ret) > > > > > - return log_ret(ret); > > > > > + clk_set_defaults(dev, CLK_DEFAULTS_POST); > > > > > return 0; > > > > > } > > > > > > > > > > > > > See [1] for previous discussion. For more background, > > > > > > > > - Device trees for i.MX are sync'd with Linux. > > > > - General clock assignments may live in the clock-controller node, > > > > > > clock assignments can be anywhere, even in non-clock-controller nodes. > > > > > > > including those which U-Boot does not implement, but which Linux > > > > does. > > > > It's OK to not set up these clocks, but U-Boot doesn't know that and > > > > fails. > > > > > > > > We don't necessarily need to revert this commit, but we do need a way to > > > > say "it's OK not to set the defaults, since we can function without > > > > them". Tom suggested doing this in the clock driver last time. I think a > > > > Kconfig or a device tree property would work, perhaps something like > > > > 'u-boot,clock-defaults-optional'. > > > > > > We didn't need custom DT properties before, Linux doesn't need them > > > either, > > > so that approach seems wrong. > > > > > > If the clock driver could say "skip unimplemented clock, because I don't > > > implement them and that is OK", that sounds like the right approach. > > > > > > Unless the 2022.01 release should be completely broken for a lot of > > > platforms, I would propose we revert the clock uclass patch now and re-add > > > it right after the release, so we would not roll out a completely broken > > > release and would have more time to fix this properly. > > > > It'll be no more broken than v2021.10 was for whatever platforms have > > problems here, yes? Since that's what has the problematic commit. > > So it seems. Does that mean we are OK with releasing such a broken release, > even though there is an easy way to improve the situation ?
I will defer to the clock maintainer who I see agrees with your patch. I was just noting that we'd already shipped one release so a last minute revert is not something I'm happy about either. -- Tom
signature.asc
Description: PGP signature