[PATCH v1 08/25] dt-bindings: clock: mt8173: Add dummy clock ID

AngeloGioacchino Del Regno posted 25 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v1 08/25] dt-bindings: clock: mt8173: Add dummy clock ID
Posted by AngeloGioacchino Del Regno 1 year, 9 months ago
Some old MediaTek clock drivers are starting the clock count (so, the
clock ID) from one instead of zero and this is logically incorrect,
as we should start from 0.
During a cleanup an issue emerged due to that and the cleanest and
shortest way to keep devicetree backwards compatibility while still
performing the well deserved cleanup is to add a dummy clock where
needed, with ID 0.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 include/dt-bindings/clock/mt8173-clk.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 3d00c98b9654..86c298e8eb89 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -7,6 +7,9 @@
 #ifndef _DT_BINDINGS_CLK_MT8173_H
 #define _DT_BINDINGS_CLK_MT8173_H
 
+/* Dummy clock for backwards compatibility */
+#define CLK_DUMMY			0
+
 /* TOPCKGEN */
 
 #define CLK_TOP_CLKPH_MCK_O		1
-- 
2.39.0
Re: [PATCH v1 08/25] dt-bindings: clock: mt8173: Add dummy clock ID
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 22/12/2022 12:48, AngeloGioacchino Del Regno wrote:
> Some old MediaTek clock drivers are starting the clock count (so, the
> clock ID) from one instead of zero and this is logically incorrect,
> as we should start from 0.
> During a cleanup an issue emerged due to that and the cleanest and
> shortest way to keep devicetree backwards compatibility while still
> performing the well deserved cleanup is to add a dummy clock where
> needed, with ID 0.

Unfortunately I do not understand at all why adding dummy (fake) ID
cleans anything here. Unifying IDs to start from 0 is not an argument on
DT bindings header IDs.

Best regards,
Krzysztof
Re: [PATCH v1 08/25] dt-bindings: clock: mt8173: Add dummy clock ID
Posted by AngeloGioacchino Del Regno 1 year, 9 months ago
Il 23/12/22 09:52, Krzysztof Kozlowski ha scritto:
> On 22/12/2022 12:48, AngeloGioacchino Del Regno wrote:
>> Some old MediaTek clock drivers are starting the clock count (so, the
>> clock ID) from one instead of zero and this is logically incorrect,
>> as we should start from 0.
>> During a cleanup an issue emerged due to that and the cleanest and
>> shortest way to keep devicetree backwards compatibility while still
>> performing the well deserved cleanup is to add a dummy clock where
>> needed, with ID 0.
> 
> Unfortunately I do not understand at all why adding dummy (fake) ID
> cleans anything here. Unifying IDs to start from 0 is not an argument on
> DT bindings header IDs.
> 
> Best regards,
> Krzysztof
> 
> 

All clocks are in one or multiple arrays, and if we don't register ID 0,
devicetrees will reference the wrong clock, as the IDs will shift back by
one during registration.

This was done for a commonization of probe() and remove() callbacks for
MediaTek clock drivers... since we have 3 affected SoCs (MT8173, MT2701
and MT6779) out of *19* (soon 20), to me, it didn't make sense to write
commonized code to address this just because of 3 out of 20 SoCs (note
that each SoC has around 4 clock drivers).

Any suggestion to keep this one short, while not touching dt-bindings?

Regards,
Angelo
Re: [PATCH v1 08/25] dt-bindings: clock: mt8173: Add dummy clock ID
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 23/12/2022 10:21, AngeloGioacchino Del Regno wrote:
> Il 23/12/22 09:52, Krzysztof Kozlowski ha scritto:
>> On 22/12/2022 12:48, AngeloGioacchino Del Regno wrote:
>>> Some old MediaTek clock drivers are starting the clock count (so, the
>>> clock ID) from one instead of zero and this is logically incorrect,
>>> as we should start from 0.
>>> During a cleanup an issue emerged due to that and the cleanest and
>>> shortest way to keep devicetree backwards compatibility while still
>>> performing the well deserved cleanup is to add a dummy clock where
>>> needed, with ID 0.
>>
>> Unfortunately I do not understand at all why adding dummy (fake) ID
>> cleans anything here. Unifying IDs to start from 0 is not an argument on
>> DT bindings header IDs.
>>
>> Best regards,
>> Krzysztof
>>
>>
> 
> All clocks are in one or multiple arrays, and if we don't register ID 0,
> devicetrees will reference the wrong clock, as the IDs will shift back by
> one during registration.

So what stops you to register some 0-dummy clock? Why do you need a
binding for it?

> This was done for a commonization of probe() and remove() callbacks for
> MediaTek clock drivers... since we have 3 affected SoCs (MT8173, MT2701
> and MT6779) out of *19* (soon 20), to me, it didn't make sense to write
> commonized code to address this just because of 3 out of 20 SoCs (note
> that each SoC has around 4 clock drivers).
> 
> Any suggestion to keep this one short, while not touching dt-bindings?

Just add a clock or better empty entry in your table, without touching
bindings.


Best regards,
Krzysztof
Re: [PATCH v1 08/25] dt-bindings: clock: mt8173: Add dummy clock ID
Posted by AngeloGioacchino Del Regno 1 year, 9 months ago
Il 23/12/22 10:26, Krzysztof Kozlowski ha scritto:
> On 23/12/2022 10:21, AngeloGioacchino Del Regno wrote:
>> Il 23/12/22 09:52, Krzysztof Kozlowski ha scritto:
>>> On 22/12/2022 12:48, AngeloGioacchino Del Regno wrote:
>>>> Some old MediaTek clock drivers are starting the clock count (so, the
>>>> clock ID) from one instead of zero and this is logically incorrect,
>>>> as we should start from 0.
>>>> During a cleanup an issue emerged due to that and the cleanest and
>>>> shortest way to keep devicetree backwards compatibility while still
>>>> performing the well deserved cleanup is to add a dummy clock where
>>>> needed, with ID 0.
>>>
>>> Unfortunately I do not understand at all why adding dummy (fake) ID
>>> cleans anything here. Unifying IDs to start from 0 is not an argument on
>>> DT bindings header IDs.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>>
>>
>> All clocks are in one or multiple arrays, and if we don't register ID 0,
>> devicetrees will reference the wrong clock, as the IDs will shift back by
>> one during registration.
> 
> So what stops you to register some 0-dummy clock? Why do you need a
> binding for it?
> 
>> This was done for a commonization of probe() and remove() callbacks for
>> MediaTek clock drivers... since we have 3 affected SoCs (MT8173, MT2701
>> and MT6779) out of *19* (soon 20), to me, it didn't make sense to write
>> commonized code to address this just because of 3 out of 20 SoCs (note
>> that each SoC has around 4 clock drivers).
>>
>> Any suggestion to keep this one short, while not touching dt-bindings?
> 
> Just add a clock or better empty entry in your table, without touching
> bindings.
> 


Okay, now that's embarassing - that's a simpler and obvious solution I
should've thought of before sending this series. Heh.

Thanks, by the way!

Regards,
Angelo