On Tue, Aug 29, 2023 at 09:18:53AM +0200, Michal Simek wrote:
>
>
> On 8/28/23 17:50, Tom Rini wrote:
> > On Mon, Aug 28, 2023 at 12:25:30PM +0200, Michal Simek wrote:
> > >
> > >
> > > On 8/25/23 16:39, Tom Rini wrote:
> > > > On Fri, Aug 25, 2023 at 09:15:09AM +0200, Michal Simek wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On 7/11/23 11:51, Ashok Reddy Soma wrote:
> > > > > > There is a chance that assigned-clock-rates is given and
> > > > > > assigned-clocks
> > > > > > could be empty. Dont return error in that case, because the probe
> > > > > > of the
> > > > > > corresponding driver will not be called at all if this fails.
> > > > > > Better to continue to look for it and return 0.
> > > > > >
> > > > > > Signed-off-by: Ashok Reddy Soma <ashok.reddy.s...@amd.com>
> > > > > > ---
> > > > > >
> > > > > > drivers/clk/clk-uclass.c | 8 +++++++-
> > > > > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > > > > > index dc3e9d6a26..f186fcbcdb 100644
> > > > > > --- a/drivers/clk/clk-uclass.c
> > > > > > +++ b/drivers/clk/clk-uclass.c
> > > > > > @@ -329,7 +329,13 @@ static int clk_set_default_rates(struct
> > > > > > udevice *dev,
> > > > > > dev_dbg(dev,
> > > > > > "could not get assigned clock
> > > > > > %d (err = %d)\n",
> > > > > > index, ret);
> > > > > > - continue;
> > > > > > + /* Skip if it is empty */
> > > > > > + if (ret == -ENOENT) {
> > > > > > + ret = 0;
> > > > > > + continue;
> > > > > > + }
> > > > > > +
> > > > > > + return ret;
> > > > > > }
> > > > > > /* This is clk provider device trying to
> > > > > > program itself
> > > > >
> > > > > What's your take on this one? I didn't get reply from Sean.
> > > >
> > > > I guess, what's the validated upstream dts where this is the case?
> > > >
> > >
> > > It was found by incorrect DT. It means I don't think there is any DT which
> > > contains this issue.
> > > But that being said we can extend current clock tests to cover this case.
> > > Please look below.
> >
> > Well, if the DT is invalid (and yes, we can't easily run the validation
> > suite in U-Boot today), I'd rather go with a debug we can optimize out
> > so that the next person with an invalid DT can more quickly find their
> > problem and we don't work-around it instead. Or am I missing something?
>
> There is already dev_dbg print above. It means when user enable DEBUG it
> gets information about it.
>
> The thing is that it just sync behavior with the linux kernel.
> You can look at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-conf.c?h=v6.5#n90
>
> There is also one more example like this.
> clk-test4 {
> compatible = "sandbox,clk-test";
> assigned-clock-rates = <654>, <321>;
> assigned-clocks = <&clk_sandbox 1>;
> };
>
> But if you prefer to fail in all these cases I am also fine with it.Ah, OK. Yes, I guess matching the kernel here is a good idea then. And yes, updating the tests next. Reviewed-by: Tom Rini <tr...@konsulko.com> -- Tom
signature.asc
Description: PGP signature
