Re: [PATCH 2/3] dt-bindings: clock: amlogic: t7: Add missing mpll3 parent clock

[Jian Hu]

On 3/10/2026 3:08 PM, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
>
> On 10/03/2026 07:51, Jian Hu wrote:
> > On 3/6/2026 4:12 PM, Krzysztof Kozlowski wrote:
> > > [ EXTERNAL EMAIL ]
> > >
> > > On Thu, Mar 05, 2026 at 03:43:26PM +0800, Jian Hu wrote:
> > > > The mpll3 clock is a valid parent clock for sd_emmc and mipi_isp on
> > > > the Amlogic T7 SoC, but was missing from t7-peripherals-clkc.yaml.
> > > > Add it to enable proper clock parent configuration for these peripherals.
> > > >
> > > > Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
> > > > ---
> > > >    .../bindings/clock/amlogic,t7-peripherals-clkc.yaml       | 8 ++++++--
> > > >    1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/clock/amlogic,t7-peripherals-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,t7-peripherals-clkc.yaml
> > > > index 55bb73707d58..27cc1f331587 100644
> > > > --- a/Documentation/devicetree/bindings/clock/amlogic,t7-peripherals-clkc.yaml
> > > > +++ b/Documentation/devicetree/bindings/clock/amlogic,t7-peripherals-clkc.yaml
> > > > @@ -24,7 +24,7 @@ properties:
> > > >        const: 1
> > > >
> > > >      clocks:
> > > > -    minItems: 14
> > > > +    minItems: 15
> > > >        items:
> > > >          - description: input oscillator
> > > >          - description: input sys clk
> > > > @@ -40,12 +40,13 @@ properties:
> > > >          - description: input gp1 pll
> > > >          - description: input mpll1
> > > >          - description: input mpll2
> > > > +      - description: input mpll3
> > > Nah, ABI break. You add it to the end of the list or provide arguments
> > > on ABI impact.
> > The third patch in this series enables the DT for the Amlogic T7 clock
> > controller.
> >
> > The clock controller node for amlogic,t7-peripherals-clkc has not been
> > merged upstream yet.
> > This change modifies the clock index order, but it will not break any
> > existing device tree since the
> > amlogic,t7-peripherals-clkc bindings are not used by any upstream or
> > downstream DT at this time.
> >
> > Therefore, it does NOT break the ABI.
> It does. Clearly visible from the diff above, because the order is the ABI.


Got it, I understand your point that changing the order break the ABI 
(since index order is part of the ABI).

> > The last clock entry is an external pad input for RTC and it is optional.
> > For logical consistency, it is better to place the required mpll3 entry
> > before the optional entry.
> >
> > If this change does not break the ABI, could I keep it in its original
> > logical order right after mpll2?
> Change breaks the ABI and commit must explain why and the impact.

It does break the ABI.

The mpll3 clock is one parent clock of sd_emmc and mipi_isp clock on
the Amlogic T7 SoC, but was missing from t7-peripherals-clkc.yaml.
Add the mpll3 clock source for the T7 peripherals clock controller.
so that sd_emmc and mipi_isp can use mpll3. That's why this patch is added.

While the amlogic,t7-peripherals-clkc bindings have been merged upstream,
the corresponding device tree (DT) that uses these bindings has not been 
merged yet.
As a result, there are no real-world users or systems
that would be broken by this change.

So could I update the commit message like:

The mpll3 clock is one parent clock of sd_emmc and mipi_isp clock on the 
Amlogic T7 SoC,
but was missing from t7-peripherals-clkc.yaml.
Add the mpll3 clock source for the T7 peripherals clock controller.
so that sd_emmc and mipi_isp can use it.

For logical consistency, place the required mpll3 entry before the 
optional entry.

This change will break the ABI, but while the 
amlogic,t7-peripherals-clkc bindings
have been merged upstream, the corresponding DT has not been merged yet.
Thus, no real users or systems are broken by this change.
> Best regards,
> Krzysztof