Re: [PATCH 2/2] clk: amlogic: c3: Limit the rate boundaries of clk_hw

[Chuan Liu]

On 1/13/2025 11:49 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Mon 13 Jan 2025 at 15:46, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
>
> > > > I think that the clock configuration exceeding the timing constraints
> > > > is a hidden danger that all chips have and face, but this hidden danger
> > > > is not easy to be exposed?
> > > >
> > > > For instance, if the routing of a clock network is close to the clock
> > > > or data bus of other modules, and this clock network is wrongly
> > > > configured to a frequency beyond the constraints, causing crosstalk
> > > > that affects the normal operation of other modules. If such a situation
> > > > occurs, it will be very difficult to troubleshoot. How should this
> > > > situation be handled more reasonably?
> > > Fix your consumers drivers if you need to. Set range if you must.


I don't think it's reliable to have consumers drivers self-regulate.
They are very likely to overlook this constraint. Moreover, when the
clock configuration exceeds the constraint, if their own module can
handle it but it affects other modules, this situation can easily
mislead people to look for the problem in the wrong direction.

Setting the range offers relatively higher fault tolerance, but it
requires adding members to each "clk_regmap_**_data" and implementing
callback functions *init() in the ops of each type of clock (*init()
calls clk_hw_set_rate_range to set the range of the provider). This
seems to complicate the originally simple logic.


> > > Those are not clock provider constraints. Those are use-case ones. It
> > > does belong here and CCF already provides the necessary infra to deal
> > > with ranges.
> > I kind of disagree here, if the vendor has the data and is willing to share
> > the range for each clock path of the system, I think it should be welcome!
> >
> > Usually those ranges are not disclosed, so we don't set them, but CCF will
> > certainly use all those range to make an even better decision on the lock
> > routing.
> I did not say you should not use them, I say that a platform
> use-case, which is what this is, should not be hard coded within the
> clock provider driver.
>
> This is no different than an assigned-rate, and like any other platform
> description, it belong in DT.
>
> We've already seen how these ranges may depend on what else you choose
> to do on the system or even what package a particular SoC variant is
> using.


That makes sense. The information I have doesn't make a distinction,
I'm not sure if other manufacturers do.


I think it's more controllable to converge this clock constraint issue
within our clock driver. Should we implement this constraint in
Amlogic's clock driver?


> > Neil