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

[Jian Hu <jian.hu@amlogic.com>]

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@amlogic.com>
>>>> ---
>>>>    .../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