diff mbox series

dt-bindings: clock: imx8mq: Fix numbering overlaps and gaps

Message ID 1551779342-2640-1-git-send-email-abel.vesa@nxp.com (mailing list archive)
State Mainlined, archived
Commit 010d5166bbe95523e8584f3caca9f1bbeac9ea6e
Headers show
Series dt-bindings: clock: imx8mq: Fix numbering overlaps and gaps | expand

Commit Message

Abel Vesa March 5, 2019, 9:49 a.m. UTC
IMX8MQ_CLK_USB_PHY_REF changes from 163 to 153, this way removing the gap.
All the following clock ids are now decreased by 10 to keep the numbering
right. Doing this, the IMX8MQ_CLK_CSI2_CORE is not overlapped with
IMX8MQ_CLK_GPT1 anymore. IMX8MQ_CLK_GPT1_ROOT changes from 193 to 183 and
all the following ids are updated accordingly.

Reported-by: Patrick Wildt <patrick@blueri.se>
Fixes: 1cf3817b ("dt-bindings: Add binding for i.MX8MQ CCM")
Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 include/dt-bindings/clock/imx8mq-clock.h | 220 +++++++++++++++----------------
 1 file changed, 110 insertions(+), 110 deletions(-)

Comments

Stephen Boyd March 5, 2019, 6:38 p.m. UTC | #1
Quoting Abel Vesa (2019-03-05 01:49:16)
> IMX8MQ_CLK_USB_PHY_REF changes from 163 to 153, this way removing the gap.
> All the following clock ids are now decreased by 10 to keep the numbering
> right. Doing this, the IMX8MQ_CLK_CSI2_CORE is not overlapped with
> IMX8MQ_CLK_GPT1 anymore. IMX8MQ_CLK_GPT1_ROOT changes from 193 to 183 and
> all the following ids are updated accordingly.

Why do the numbers need to be consecutive? This looks difficult to merge
given that the commit that's being "fixed" is in v5.0 and thus the
integer value of these defines is pretty much an ABI. So if there are
holes in the space, I suggest we leave them there unless something is
really wrong or unworkable with that solution.

BTW, it would be great if the binding header was generated once and then
never changed again so that we don't have to spend time on these sorts
of patches in the future. Please try to fully describe each possible clk
that might be used with a particular binding instead of adding new clk
ids over time, especially if you have access to the silicon manufacturer
documentation and can easily figure out all the clks that are there
beforehand.
Abel Vesa March 6, 2019, 1:09 p.m. UTC | #2
On 19-03-05 10:38:29, Stephen Boyd wrote:
> Quoting Abel Vesa (2019-03-05 01:49:16)
> > IMX8MQ_CLK_USB_PHY_REF changes from 163 to 153, this way removing the gap.
> > All the following clock ids are now decreased by 10 to keep the numbering
> > right. Doing this, the IMX8MQ_CLK_CSI2_CORE is not overlapped with
> > IMX8MQ_CLK_GPT1 anymore. IMX8MQ_CLK_GPT1_ROOT changes from 193 to 183 and
> > all the following ids are updated accordingly.
> 
> Why do the numbers need to be consecutive? This looks difficult to merge
> given that the commit that's being "fixed" is in v5.0 and thus the
> integer value of these defines is pretty much an ABI. So if there are
> holes in the space, I suggest we leave them there unless something is
> really wrong or unworkable with that solution.
> 

I would've ignored it but there are a few overlaps.

#define IMX8MQ_CLK_GPT1                        170
overlaps with:
#define IMX8MQ_CLK_CSI2_CORE           170

#define IMX8MQ_CLK_CSI2_PHY_REF                181
overlaps with:
#define IMX8MQ_CLK_ECSPI3_ROOT                 181

#define IMX8MQ_CLK_CSI2_ESC            182
overlaps with:
#define IMX8MQ_CLK_ENET1_ROOT                  182

By removing the gaps, some of the overlaps are also removed.

I can just get rid of the overlaps and keep the gaps if that makes it more ok
for the stability of the ABI.

> BTW, it would be great if the binding header was generated once and then
> never changed again so that we don't have to spend time on these sorts
> of patches in the future. Please try to fully describe each possible clk
> that might be used with a particular binding instead of adding new clk
> ids over time, especially if you have access to the silicon manufacturer
> documentation and can easily figure out all the clks that are there
> beforehand.
> 

Here is an example of why this is not really doable: clk-sccg-pll.c.
The original design was adding the intermediary clocks like:

IMX8MQ_SYS1_PLL1_OUT        
IMX8MQ_SYS1_PLL1_OUT_DIV    
IMX8MQ_SYS1_PLL1_REF_DIV    
IMX8MQ_SYS1_PLL2
IMX8MQ_SYS1_PLL2_DIV  
IMX8MQ_SYS1_PLL2_OUT  

And these were just for SYS1_PLL. There are 2 more SYSx_PLL.
Plus the DRAM_PLL, the VIDEO2_PLL and the AUDIO_PLL.

Since the refactoring of clk-sccg-pll.c, these are not used anymore (or should
not be used). So I would have to remove them, but as you said, it would break
the ABI. And this example goes even further with the fact that the PHY_27M
and the mux between the PHY_27M and the OSC_27 are to be controlled by the
display controller driver itself (at this point the PHY_27M is not yet upstream
and I can't say for sure it will ever be). Basically, what this means is that 
the PHY_27M will have to be exposed as a standalone clock even if it is hidden
behind a mux which has another clock that can provide the same rate. That
is only because there is some difference in phase (AFAIU) between the OSC_27M
and the PHY_27M, at the same rate. And this is just one example.

Point being, there is no way of knowing beforehand what intermediary clocks
are needed, even with the silicon manufacturer documentation, until the
driver that makes use of a specific clock actually works.
Stephen Boyd March 8, 2019, 3:29 p.m. UTC | #3
Quoting Abel Vesa (2019-03-06 05:09:42)
> On 19-03-05 10:38:29, Stephen Boyd wrote:
> > Quoting Abel Vesa (2019-03-05 01:49:16)
> > > IMX8MQ_CLK_USB_PHY_REF changes from 163 to 153, this way removing the gap.
> > > All the following clock ids are now decreased by 10 to keep the numbering
> > > right. Doing this, the IMX8MQ_CLK_CSI2_CORE is not overlapped with
> > > IMX8MQ_CLK_GPT1 anymore. IMX8MQ_CLK_GPT1_ROOT changes from 193 to 183 and
> > > all the following ids are updated accordingly.
> > 
> > Why do the numbers need to be consecutive? This looks difficult to merge
> > given that the commit that's being "fixed" is in v5.0 and thus the
> > integer value of these defines is pretty much an ABI. So if there are
> > holes in the space, I suggest we leave them there unless something is
> > really wrong or unworkable with that solution.
> > 
> 
> I would've ignored it but there are a few overlaps.
> 
> #define IMX8MQ_CLK_GPT1                        170
> overlaps with:
> #define IMX8MQ_CLK_CSI2_CORE           170
> 
> #define IMX8MQ_CLK_CSI2_PHY_REF                181
> overlaps with:
> #define IMX8MQ_CLK_ECSPI3_ROOT                 181
> 
> #define IMX8MQ_CLK_CSI2_ESC            182
> overlaps with:
> #define IMX8MQ_CLK_ENET1_ROOT                  182

Ok. Are any of the overlaps in use by dtbs right now?

> 
> By removing the gaps, some of the overlaps are also removed.
> 
> I can just get rid of the overlaps and keep the gaps if that makes it more ok
> for the stability of the ABI.

It's mostly about making sure that any existing dtbs don't have their
numbers shifted around. So hopefully any overlapping identifiers aren't
in use yet and then those ids can be changed while leaving the ones that
are in use how they are.

> 
> > BTW, it would be great if the binding header was generated once and then
> > never changed again so that we don't have to spend time on these sorts
> > of patches in the future. Please try to fully describe each possible clk
> > that might be used with a particular binding instead of adding new clk
> > ids over time, especially if you have access to the silicon manufacturer
> > documentation and can easily figure out all the clks that are there
> > beforehand.
> > 
> 
> Here is an example of why this is not really doable: clk-sccg-pll.c.
> The original design was adding the intermediary clocks like:
> 
> IMX8MQ_SYS1_PLL1_OUT        
> IMX8MQ_SYS1_PLL1_OUT_DIV    
> IMX8MQ_SYS1_PLL1_REF_DIV    
> IMX8MQ_SYS1_PLL2
> IMX8MQ_SYS1_PLL2_DIV  
> IMX8MQ_SYS1_PLL2_OUT  
> 
> And these were just for SYS1_PLL. There are 2 more SYSx_PLL.
> Plus the DRAM_PLL, the VIDEO2_PLL and the AUDIO_PLL.
> 
> Since the refactoring of clk-sccg-pll.c, these are not used anymore (or should
> not be used). So I would have to remove them, but as you said, it would break
> the ABI.

Why do you need to remove them? Why can't they just return an error if
some driver tries to get those clks after they shouldn't be use anymore?

> And this example goes even further with the fact that the PHY_27M
> and the mux between the PHY_27M and the OSC_27 are to be controlled by the
> display controller driver itself (at this point the PHY_27M is not yet upstream
> and I can't say for sure it will ever be). Basically, what this means is that 
> the PHY_27M will have to be exposed as a standalone clock even if it is hidden
> behind a mux which has another clock that can provide the same rate. That
> is only because there is some difference in phase (AFAIU) between the OSC_27M
> and the PHY_27M, at the same rate. And this is just one example.

It sounds like these are two independent clks that could or couldn't be
exposed into the DT numberspace?

> 
> Point being, there is no way of knowing beforehand what intermediary clocks
> are needed, even with the silicon manufacturer documentation, until the
> driver that makes use of a specific clock actually works.

I agree that we don't know what clks are needed beforehand, predicting
the future is hard, but that doesn't mean they can't be exposed into the
header file if they exist. It doesn't really matter if there are numbers
in there or not for clks that shouldn't be used. The provider and the
driver implementation need to make the decision to provide them or not
to the consumer drivers when they ask for them. It's perfectly valid for
new code to start failing those requests because "they shouldn't be
used". The header file (really the DT bindings) is the place that
decides what the numbers are and what they correspond to.
Patrick Wildt March 12, 2019, 7:36 a.m. UTC | #4
On Fri, Mar 08, 2019 at 07:29:05AM -0800, Stephen Boyd wrote:
> It's mostly about making sure that any existing dtbs don't have their
> numbers shifted around. So hopefully any overlapping identifiers aren't
> in use yet and then those ids can be changed while leaving the ones that
> are in use how they are.

In practice I bet no one uses Linux 5.0's i.MX8M device trees since they
lack too much support.  It's so basic it's not useful.  You'd still run
your existing non-mainline bindings until it is.  Thus I would argue
changing the ABI right now would be the only chance there is.

If you think that chance is gone, then I guess the reasonable thing is
to keep the numbers and only move those (to the end) which overlap.  Or
put them into that erreneous number gap.

Patrick
Stephen Boyd March 12, 2019, 8:39 p.m. UTC | #5
Quoting Patrick Wildt (2019-03-12 00:36:54)
> On Fri, Mar 08, 2019 at 07:29:05AM -0800, Stephen Boyd wrote:
> > It's mostly about making sure that any existing dtbs don't have their
> > numbers shifted around. So hopefully any overlapping identifiers aren't
> > in use yet and then those ids can be changed while leaving the ones that
> > are in use how they are.
> 
> In practice I bet no one uses Linux 5.0's i.MX8M device trees since they
> lack too much support.  It's so basic it's not useful.  You'd still run
> your existing non-mainline bindings until it is.  Thus I would argue
> changing the ABI right now would be the only chance there is.
> 
> If you think that chance is gone, then I guess the reasonable thing is
> to keep the numbers and only move those (to the end) which overlap.  Or
> put them into that erreneous number gap.
> 

The chance is quickly slipping away because we're going to be at -rc1
soon. I'm not the one to decide what is and isn't being used by people
out there, so I'm happy to apply this patch now before the next -rc1
comes out as long as it doesn't break anything in arm-soc area. The
confidence I'm getting isn't high though. Has anyone from NXP reviewed
this change? Maybe I can get an ack from someone else that normally
looks after the arm-soc/dts side of things here indicating that nothing
should go wrong? That would increase my confidence levels.
Patrick Wildt March 12, 2019, 8:59 p.m. UTC | #6
On Tue, Mar 12, 2019 at 01:39:50PM -0700, Stephen Boyd wrote:
> Quoting Patrick Wildt (2019-03-12 00:36:54)
> > On Fri, Mar 08, 2019 at 07:29:05AM -0800, Stephen Boyd wrote:
> > > It's mostly about making sure that any existing dtbs don't have their
> > > numbers shifted around. So hopefully any overlapping identifiers aren't
> > > in use yet and then those ids can be changed while leaving the ones that
> > > are in use how they are.
> > 
> > In practice I bet no one uses Linux 5.0's i.MX8M device trees since they
> > lack too much support.  It's so basic it's not useful.  You'd still run
> > your existing non-mainline bindings until it is.  Thus I would argue
> > changing the ABI right now would be the only chance there is.
> > 
> > If you think that chance is gone, then I guess the reasonable thing is
> > to keep the numbers and only move those (to the end) which overlap.  Or
> > put them into that erreneous number gap.
> > 
> 
> The chance is quickly slipping away because we're going to be at -rc1
> soon. I'm not the one to decide what is and isn't being used by people
> out there, so I'm happy to apply this patch now before the next -rc1
> comes out as long as it doesn't break anything in arm-soc area. The
> confidence I'm getting isn't high though. Has anyone from NXP reviewed
> this change? Maybe I can get an ack from someone else that normally
> looks after the arm-soc/dts side of things here indicating that nothing
> should go wrong? That would increase my confidence levels.

The person that supplied the diff apparently is from NXP, which should
be enough to say that NXP reviewed it?

It's a bit of a shame that the ones that are CC'd keep quiet.  I would
take this chance and go ahead with it.  After 5.1/rc1 there will be no
chance to rectify this in a sane way.
Stephen Boyd March 12, 2019, 10:02 p.m. UTC | #7
Quoting Patrick Wildt (2019-03-12 13:59:22)
> On Tue, Mar 12, 2019 at 01:39:50PM -0700, Stephen Boyd wrote:
> > Quoting Patrick Wildt (2019-03-12 00:36:54)
> > > On Fri, Mar 08, 2019 at 07:29:05AM -0800, Stephen Boyd wrote:
> > > > It's mostly about making sure that any existing dtbs don't have their
> > > > numbers shifted around. So hopefully any overlapping identifiers aren't
> > > > in use yet and then those ids can be changed while leaving the ones that
> > > > are in use how they are.
> > > 
> > > In practice I bet no one uses Linux 5.0's i.MX8M device trees since they
> > > lack too much support.  It's so basic it's not useful.  You'd still run
> > > your existing non-mainline bindings until it is.  Thus I would argue
> > > changing the ABI right now would be the only chance there is.
> > > 
> > > If you think that chance is gone, then I guess the reasonable thing is
> > > to keep the numbers and only move those (to the end) which overlap.  Or
> > > put them into that erreneous number gap.
> > > 
> > 
> > The chance is quickly slipping away because we're going to be at -rc1
> > soon. I'm not the one to decide what is and isn't being used by people
> > out there, so I'm happy to apply this patch now before the next -rc1
> > comes out as long as it doesn't break anything in arm-soc area. The
> > confidence I'm getting isn't high though. Has anyone from NXP reviewed
> > this change? Maybe I can get an ack from someone else that normally
> > looks after the arm-soc/dts side of things here indicating that nothing
> > should go wrong? That would increase my confidence levels.
> 
> The person that supplied the diff apparently is from NXP, which should
> be enough to say that NXP reviewed it?
> 
> It's a bit of a shame that the ones that are CC'd keep quiet.  I would
> take this chance and go ahead with it.  After 5.1/rc1 there will be no
> chance to rectify this in a sane way.

Ok. I'm just going to merge it and see if anyone complains. I'll send
the PR tomorrow.
Dong Aisheng March 15, 2019, 1:33 p.m. UTC | #8
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> 
> Quoting Patrick Wildt (2019-03-12 00:36:54)
> > On Fri, Mar 08, 2019 at 07:29:05AM -0800, Stephen Boyd wrote:
> > > It's mostly about making sure that any existing dtbs don't have
> > > their numbers shifted around. So hopefully any overlapping
> > > identifiers aren't in use yet and then those ids can be changed
> > > while leaving the ones that are in use how they are.
> >
> > In practice I bet no one uses Linux 5.0's i.MX8M device trees since
> > they lack too much support.  It's so basic it's not useful.  You'd
> > still run your existing non-mainline bindings until it is.  Thus I
> > would argue changing the ABI right now would be the only chance there is.
> >
> > If you think that chance is gone, then I guess the reasonable thing is
> > to keep the numbers and only move those (to the end) which overlap.
> > Or put them into that erreneous number gap.
> >
> 
> The chance is quickly slipping away because we're going to be at -rc1 soon. I'm
> not the one to decide what is and isn't being used by people out there, so I'm
> happy to apply this patch now before the next -rc1 comes out as long as it
> doesn't break anything in arm-soc area. The confidence I'm getting isn't high
> though. Has anyone from NXP reviewed this change? Maybe I can get an ack
> from someone else that normally looks after the arm-soc/dts side of things
> here indicating that nothing should go wrong? That would increase my
> confidence levels.

AFAIK no one out there using it for product without being able to update accordingly,
as it still has a very preliminary support.
So I agree we need to fix it at this early time

Tested-and-Acked-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng
diff mbox series

Patch

diff --git a/include/dt-bindings/clock/imx8mq-clock.h b/include/dt-bindings/clock/imx8mq-clock.h
index b58cc64..6677e92 100644
--- a/include/dt-bindings/clock/imx8mq-clock.h
+++ b/include/dt-bindings/clock/imx8mq-clock.h
@@ -245,160 +245,160 @@ 
 /* USB_CORE_REF */
 #define IMX8MQ_CLK_USB_CORE_REF		152
 /* USB_PHY_REF */
-#define IMX8MQ_CLK_USB_PHY_REF		163
+#define IMX8MQ_CLK_USB_PHY_REF		153
 /* ECSPI1 */
-#define IMX8MQ_CLK_ECSPI1		164
+#define IMX8MQ_CLK_ECSPI1		154
 /* ECSPI2 */
-#define IMX8MQ_CLK_ECSPI2		165
+#define IMX8MQ_CLK_ECSPI2		155
 /* PWM1 */
-#define IMX8MQ_CLK_PWM1			166
+#define IMX8MQ_CLK_PWM1			156
 /* PWM2 */
-#define IMX8MQ_CLK_PWM2			167
+#define IMX8MQ_CLK_PWM2			157
 /* PWM3 */
-#define IMX8MQ_CLK_PWM3			168
+#define IMX8MQ_CLK_PWM3			158
 /* PWM4 */
-#define IMX8MQ_CLK_PWM4			169
+#define IMX8MQ_CLK_PWM4			159
 /* GPT1 */
-#define IMX8MQ_CLK_GPT1			170
+#define IMX8MQ_CLK_GPT1			160
 /* WDOG */
-#define IMX8MQ_CLK_WDOG			171
+#define IMX8MQ_CLK_WDOG			161
 /* WRCLK */
-#define IMX8MQ_CLK_WRCLK		172
+#define IMX8MQ_CLK_WRCLK		162
 /* DSI_CORE */
-#define IMX8MQ_CLK_DSI_CORE		173
+#define IMX8MQ_CLK_DSI_CORE		163
 /* DSI_PHY */
-#define IMX8MQ_CLK_DSI_PHY_REF		174
+#define IMX8MQ_CLK_DSI_PHY_REF		164
 /* DSI_DBI */
-#define IMX8MQ_CLK_DSI_DBI		175
+#define IMX8MQ_CLK_DSI_DBI		165
 /*DSI_ESC */
-#define IMX8MQ_CLK_DSI_ESC		176
+#define IMX8MQ_CLK_DSI_ESC		166
 /* CSI1_CORE */
-#define IMX8MQ_CLK_CSI1_CORE		177
+#define IMX8MQ_CLK_CSI1_CORE		167
 /* CSI1_PHY */
-#define IMX8MQ_CLK_CSI1_PHY_REF		178
+#define IMX8MQ_CLK_CSI1_PHY_REF		168
 /* CSI_ESC */
-#define IMX8MQ_CLK_CSI1_ESC		179
+#define IMX8MQ_CLK_CSI1_ESC		169
 /* CSI2_CORE */
 #define IMX8MQ_CLK_CSI2_CORE		170
 /* CSI2_PHY */
-#define IMX8MQ_CLK_CSI2_PHY_REF		181
+#define IMX8MQ_CLK_CSI2_PHY_REF		171
 /* CSI2_ESC */
-#define IMX8MQ_CLK_CSI2_ESC		182
+#define IMX8MQ_CLK_CSI2_ESC		172
 /* PCIE2_CTRL */
-#define IMX8MQ_CLK_PCIE2_CTRL		183
+#define IMX8MQ_CLK_PCIE2_CTRL		173
 /* PCIE2_PHY */
-#define IMX8MQ_CLK_PCIE2_PHY		184
+#define IMX8MQ_CLK_PCIE2_PHY		174
 /* PCIE2_AUX */
-#define IMX8MQ_CLK_PCIE2_AUX		185
+#define IMX8MQ_CLK_PCIE2_AUX		175
 /* ECSPI3 */
-#define IMX8MQ_CLK_ECSPI3		186
+#define IMX8MQ_CLK_ECSPI3		176
 
 /* CCGR clocks */
-#define IMX8MQ_CLK_A53_ROOT			187
-#define IMX8MQ_CLK_DRAM_ROOT			188
-#define IMX8MQ_CLK_ECSPI1_ROOT			189
+#define IMX8MQ_CLK_A53_ROOT			177
+#define IMX8MQ_CLK_DRAM_ROOT			178
+#define IMX8MQ_CLK_ECSPI1_ROOT			179
 #define IMX8MQ_CLK_ECSPI2_ROOT			180
 #define IMX8MQ_CLK_ECSPI3_ROOT			181
 #define IMX8MQ_CLK_ENET1_ROOT			182
-#define IMX8MQ_CLK_GPT1_ROOT			193
-#define IMX8MQ_CLK_I2C1_ROOT			194
-#define IMX8MQ_CLK_I2C2_ROOT			195
-#define IMX8MQ_CLK_I2C3_ROOT			196
-#define IMX8MQ_CLK_I2C4_ROOT			197
-#define IMX8MQ_CLK_M4_ROOT			198
-#define IMX8MQ_CLK_PCIE1_ROOT			199
-#define IMX8MQ_CLK_PCIE2_ROOT			200
-#define IMX8MQ_CLK_PWM1_ROOT			201
-#define IMX8MQ_CLK_PWM2_ROOT			202
-#define IMX8MQ_CLK_PWM3_ROOT			203
-#define IMX8MQ_CLK_PWM4_ROOT			204
-#define IMX8MQ_CLK_QSPI_ROOT			205
-#define IMX8MQ_CLK_SAI1_ROOT			206
-#define IMX8MQ_CLK_SAI2_ROOT			207
-#define IMX8MQ_CLK_SAI3_ROOT			208
-#define IMX8MQ_CLK_SAI4_ROOT			209
-#define IMX8MQ_CLK_SAI5_ROOT			210
-#define IMX8MQ_CLK_SAI6_ROOT			212
-#define IMX8MQ_CLK_UART1_ROOT			213
-#define IMX8MQ_CLK_UART2_ROOT			214
-#define IMX8MQ_CLK_UART3_ROOT			215
-#define IMX8MQ_CLK_UART4_ROOT			216
-#define IMX8MQ_CLK_USB1_CTRL_ROOT		217
-#define IMX8MQ_CLK_USB2_CTRL_ROOT		218
-#define IMX8MQ_CLK_USB1_PHY_ROOT		219
-#define IMX8MQ_CLK_USB2_PHY_ROOT		220
-#define IMX8MQ_CLK_USDHC1_ROOT			221
-#define IMX8MQ_CLK_USDHC2_ROOT			222
-#define IMX8MQ_CLK_WDOG1_ROOT			223
-#define IMX8MQ_CLK_WDOG2_ROOT			224
-#define IMX8MQ_CLK_WDOG3_ROOT			225
-#define IMX8MQ_CLK_GPU_ROOT			226
-#define IMX8MQ_CLK_HEVC_ROOT			227
-#define IMX8MQ_CLK_AVC_ROOT			228
-#define IMX8MQ_CLK_VP9_ROOT			229
-#define IMX8MQ_CLK_HEVC_INTER_ROOT		230
-#define IMX8MQ_CLK_DISP_ROOT			231
-#define IMX8MQ_CLK_HDMI_ROOT			232
-#define IMX8MQ_CLK_HDMI_PHY_ROOT		233
-#define IMX8MQ_CLK_VPU_DEC_ROOT			234
-#define IMX8MQ_CLK_CSI1_ROOT			235
-#define IMX8MQ_CLK_CSI2_ROOT			236
-#define IMX8MQ_CLK_RAWNAND_ROOT			237
-#define IMX8MQ_CLK_SDMA1_ROOT			238
-#define IMX8MQ_CLK_SDMA2_ROOT			239
-#define IMX8MQ_CLK_VPU_G1_ROOT			240
-#define IMX8MQ_CLK_VPU_G2_ROOT			241
+#define IMX8MQ_CLK_GPT1_ROOT			183
+#define IMX8MQ_CLK_I2C1_ROOT			184
+#define IMX8MQ_CLK_I2C2_ROOT			185
+#define IMX8MQ_CLK_I2C3_ROOT			186
+#define IMX8MQ_CLK_I2C4_ROOT			187
+#define IMX8MQ_CLK_M4_ROOT			188
+#define IMX8MQ_CLK_PCIE1_ROOT			189
+#define IMX8MQ_CLK_PCIE2_ROOT			190
+#define IMX8MQ_CLK_PWM1_ROOT			191
+#define IMX8MQ_CLK_PWM2_ROOT			192
+#define IMX8MQ_CLK_PWM3_ROOT			193
+#define IMX8MQ_CLK_PWM4_ROOT			194
+#define IMX8MQ_CLK_QSPI_ROOT			195
+#define IMX8MQ_CLK_SAI1_ROOT			196
+#define IMX8MQ_CLK_SAI2_ROOT			197
+#define IMX8MQ_CLK_SAI3_ROOT			198
+#define IMX8MQ_CLK_SAI4_ROOT			199
+#define IMX8MQ_CLK_SAI5_ROOT			200
+#define IMX8MQ_CLK_SAI6_ROOT			201
+#define IMX8MQ_CLK_UART1_ROOT			202
+#define IMX8MQ_CLK_UART2_ROOT			203
+#define IMX8MQ_CLK_UART3_ROOT			204
+#define IMX8MQ_CLK_UART4_ROOT			205
+#define IMX8MQ_CLK_USB1_CTRL_ROOT		206
+#define IMX8MQ_CLK_USB2_CTRL_ROOT		207
+#define IMX8MQ_CLK_USB1_PHY_ROOT		208
+#define IMX8MQ_CLK_USB2_PHY_ROOT		209
+#define IMX8MQ_CLK_USDHC1_ROOT			210
+#define IMX8MQ_CLK_USDHC2_ROOT			211
+#define IMX8MQ_CLK_WDOG1_ROOT			212
+#define IMX8MQ_CLK_WDOG2_ROOT			213
+#define IMX8MQ_CLK_WDOG3_ROOT			214
+#define IMX8MQ_CLK_GPU_ROOT			215
+#define IMX8MQ_CLK_HEVC_ROOT			216
+#define IMX8MQ_CLK_AVC_ROOT			217
+#define IMX8MQ_CLK_VP9_ROOT			218
+#define IMX8MQ_CLK_HEVC_INTER_ROOT		219
+#define IMX8MQ_CLK_DISP_ROOT			220
+#define IMX8MQ_CLK_HDMI_ROOT			221
+#define IMX8MQ_CLK_HDMI_PHY_ROOT		222
+#define IMX8MQ_CLK_VPU_DEC_ROOT			223
+#define IMX8MQ_CLK_CSI1_ROOT			224
+#define IMX8MQ_CLK_CSI2_ROOT			225
+#define IMX8MQ_CLK_RAWNAND_ROOT			226
+#define IMX8MQ_CLK_SDMA1_ROOT			227
+#define IMX8MQ_CLK_SDMA2_ROOT			228
+#define IMX8MQ_CLK_VPU_G1_ROOT			229
+#define IMX8MQ_CLK_VPU_G2_ROOT			230
 
 /* SCCG PLL GATE */
-#define IMX8MQ_SYS1_PLL_OUT			242
-#define IMX8MQ_SYS2_PLL_OUT			243
-#define IMX8MQ_SYS3_PLL_OUT			244
-#define IMX8MQ_DRAM_PLL_OUT			245
-
-#define IMX8MQ_GPT_3M_CLK			246
-
-#define IMX8MQ_CLK_IPG_ROOT			247
-#define IMX8MQ_CLK_IPG_AUDIO_ROOT		248
-#define IMX8MQ_CLK_SAI1_IPG			249
-#define IMX8MQ_CLK_SAI2_IPG			250
-#define IMX8MQ_CLK_SAI3_IPG			251
-#define IMX8MQ_CLK_SAI4_IPG			252
-#define IMX8MQ_CLK_SAI5_IPG			253
-#define IMX8MQ_CLK_SAI6_IPG			254
+#define IMX8MQ_SYS1_PLL_OUT			231
+#define IMX8MQ_SYS2_PLL_OUT			232
+#define IMX8MQ_SYS3_PLL_OUT			233
+#define IMX8MQ_DRAM_PLL_OUT			234
+
+#define IMX8MQ_GPT_3M_CLK			235
+
+#define IMX8MQ_CLK_IPG_ROOT			236
+#define IMX8MQ_CLK_IPG_AUDIO_ROOT		237
+#define IMX8MQ_CLK_SAI1_IPG			238
+#define IMX8MQ_CLK_SAI2_IPG			239
+#define IMX8MQ_CLK_SAI3_IPG			240
+#define IMX8MQ_CLK_SAI4_IPG			241
+#define IMX8MQ_CLK_SAI5_IPG			242
+#define IMX8MQ_CLK_SAI6_IPG			243
 
 /* DSI AHB/IPG clocks */
 /* rxesc clock */
-#define IMX8MQ_CLK_DSI_AHB			255
+#define IMX8MQ_CLK_DSI_AHB			244
 /* txesc clock */
-#define IMX8MQ_CLK_DSI_IPG_DIV                  256
+#define IMX8MQ_CLK_DSI_IPG_DIV                  245
 
-#define IMX8MQ_CLK_TMU_ROOT			257
+#define IMX8MQ_CLK_TMU_ROOT			246
 
 /* Display root clocks */
-#define IMX8MQ_CLK_DISP_AXI_ROOT		258
-#define IMX8MQ_CLK_DISP_APB_ROOT		259
-#define IMX8MQ_CLK_DISP_RTRM_ROOT		260
+#define IMX8MQ_CLK_DISP_AXI_ROOT		247
+#define IMX8MQ_CLK_DISP_APB_ROOT		248
+#define IMX8MQ_CLK_DISP_RTRM_ROOT		249
 
-#define IMX8MQ_CLK_OCOTP_ROOT			261
+#define IMX8MQ_CLK_OCOTP_ROOT			250
 
-#define IMX8MQ_CLK_DRAM_ALT_ROOT		262
-#define IMX8MQ_CLK_DRAM_CORE			263
+#define IMX8MQ_CLK_DRAM_ALT_ROOT		251
+#define IMX8MQ_CLK_DRAM_CORE			252
 
-#define IMX8MQ_CLK_MU_ROOT			264
-#define IMX8MQ_VIDEO2_PLL_OUT			265
+#define IMX8MQ_CLK_MU_ROOT			253
+#define IMX8MQ_VIDEO2_PLL_OUT			254
 
-#define IMX8MQ_CLK_CLKO2			266
+#define IMX8MQ_CLK_CLKO2			255
 
-#define IMX8MQ_CLK_NAND_USDHC_BUS_RAWNAND_CLK	267
+#define IMX8MQ_CLK_NAND_USDHC_BUS_RAWNAND_CLK	256
 
-#define IMX8MQ_CLK_CLKO1			268
-#define IMX8MQ_CLK_ARM				269
+#define IMX8MQ_CLK_CLKO1			257
+#define IMX8MQ_CLK_ARM				258
 
-#define IMX8MQ_CLK_GPIO1_ROOT			270
-#define IMX8MQ_CLK_GPIO2_ROOT			271
-#define IMX8MQ_CLK_GPIO3_ROOT			272
-#define IMX8MQ_CLK_GPIO4_ROOT			273
-#define IMX8MQ_CLK_GPIO5_ROOT			274
+#define IMX8MQ_CLK_GPIO1_ROOT			259
+#define IMX8MQ_CLK_GPIO2_ROOT			260
+#define IMX8MQ_CLK_GPIO3_ROOT			261
+#define IMX8MQ_CLK_GPIO4_ROOT			262
+#define IMX8MQ_CLK_GPIO5_ROOT			263
 
-#define IMX8MQ_CLK_END				275
+#define IMX8MQ_CLK_END				264
 #endif /* __DT_BINDINGS_CLOCK_IMX8MQ_H */