dt-bindings: imx: update scu resource id headfile

Commit Message

Anson Huang Feb. 19, 2019, 9:01 a.m. UTC

Update i.MX SCU resource ID table according to latest
system controller firmware.

Signed-off-by: Anson Huang <[e-mail]>
---
 include/dt-bindings/firmware/imx/rsrc.h | 39 +++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 17 deletions(-)

Comments

Marco Felsch Feb. 19, 2019, 12:52 p.m. UTC | #1

Hi Anson,

On 19-02-19 09:01, Anson Huang wrote:
> Update i.MX SCU resource ID table according to latest
> system controller firmware.

will this happen every time the scu firmware gets a update?

Regards,
Marco

> Signed-off-by: Anson Huang <[e-mail]>
> ---
>  include/dt-bindings/firmware/imx/rsrc.h | 39 +++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h
> index 4481f2d..ad747a8 100644
> --- a/include/dt-bindings/firmware/imx/rsrc.h
> +++ b/include/dt-bindings/firmware/imx/rsrc.h
> @@ -36,15 +36,15 @@
>  #define IMX_SC_R_DC_0_BLIT1		20
>  #define IMX_SC_R_DC_0_BLIT2		21
>  #define IMX_SC_R_DC_0_BLIT_OUT		22
> -#define IMX_SC_R_DC_0_CAPTURE0		23
> -#define IMX_SC_R_DC_0_CAPTURE1		24
> +#define IMX_SC_R_PERF			23
> +#define IMX_SC_R_UNUSED5		24
>  #define IMX_SC_R_DC_0_WARP		25
> -#define IMX_SC_R_DC_0_INTEGRAL0		26
> -#define IMX_SC_R_DC_0_INTEGRAL1		27
> +#define IMX_SC_R_UNUSED7		26
> +#define IMX_SC_R_UNUSED8		27
>  #define IMX_SC_R_DC_0_VIDEO0		28
>  #define IMX_SC_R_DC_0_VIDEO1		29
>  #define IMX_SC_R_DC_0_FRAC0		30
> -#define IMX_SC_R_DC_0_FRAC1		31
> +#define IMX_SC_R_UNUSED6		31
>  #define IMX_SC_R_DC_0			32
>  #define IMX_SC_R_GPU_2_PID0		33
>  #define IMX_SC_R_DC_0_PLL_0		34
> @@ -53,17 +53,17 @@
>  #define IMX_SC_R_DC_1_BLIT1		37
>  #define IMX_SC_R_DC_1_BLIT2		38
>  #define IMX_SC_R_DC_1_BLIT_OUT		39
> -#define IMX_SC_R_DC_1_CAPTURE0		40
> -#define IMX_SC_R_DC_1_CAPTURE1		41
> +#define IMX_SC_R_UNUSED9		40
> +#define IMX_SC_R_UNUSED10		41
>  #define IMX_SC_R_DC_1_WARP		42
> -#define IMX_SC_R_DC_1_INTEGRAL0		43
> -#define IMX_SC_R_DC_1_INTEGRAL1		44
> +#define IMX_SC_R_UNUSED11		43
> +#define IMX_SC_R_UNUSED12		44
>  #define IMX_SC_R_DC_1_VIDEO0		45
>  #define IMX_SC_R_DC_1_VIDEO1		46
>  #define IMX_SC_R_DC_1_FRAC0		47
> -#define IMX_SC_R_DC_1_FRAC1		48
> +#define IMX_SC_R_UNUSED13		48
>  #define IMX_SC_R_DC_1			49
> -#define IMX_SC_R_GPU_3_PID0		50
> +#define IMX_SC_R_UNUSED14		50
>  #define IMX_SC_R_DC_1_PLL_0		51
>  #define IMX_SC_R_DC_1_PLL_1		52
>  #define IMX_SC_R_SPI_0			53
> @@ -303,8 +303,8 @@
>  #define IMX_SC_R_M4_0_UART		287
>  #define IMX_SC_R_M4_0_I2C		288
>  #define IMX_SC_R_M4_0_INTMUX		289
> -#define IMX_SC_R_M4_0_SIM		290
> -#define IMX_SC_R_M4_0_WDOG		291
> +#define IMX_SC_R_UNUSED15		290
> +#define IMX_SC_R_UNUSED16		291
>  #define IMX_SC_R_M4_0_MU_0B		292
>  #define IMX_SC_R_M4_0_MU_0A0		293
>  #define IMX_SC_R_M4_0_MU_0A1		294
> @@ -323,8 +323,8 @@
>  #define IMX_SC_R_M4_1_UART		307
>  #define IMX_SC_R_M4_1_I2C		308
>  #define IMX_SC_R_M4_1_INTMUX		309
> -#define IMX_SC_R_M4_1_SIM		310
> -#define IMX_SC_R_M4_1_WDOG		311
> +#define IMX_SC_R_UNUSED17		310
> +#define IMX_SC_R_UNUSED18		311
>  #define IMX_SC_R_M4_1_MU_0B		312
>  #define IMX_SC_R_M4_1_MU_0A0		313
>  #define IMX_SC_R_M4_1_MU_0A1		314
> @@ -337,7 +337,7 @@
>  #define IMX_SC_R_IRQSTR_SCU2		321
>  #define IMX_SC_R_IRQSTR_DSP		322
>  #define IMX_SC_R_ELCDIF_PLL		323
> -#define IMX_SC_R_UNUSED6		324
> +#define IMX_SC_R_OCRAM			324
>  #define IMX_SC_R_AUDIO_PLL_0		325
>  #define IMX_SC_R_PI_0			326
>  #define IMX_SC_R_PI_0_PWM_0		327
> @@ -554,6 +554,11 @@
>  #define IMX_SC_R_VPU_MU_3		538
>  #define IMX_SC_R_VPU_ENC_1		539
>  #define IMX_SC_R_VPU			540
> -#define IMX_SC_R_LAST			541
> +#define IMX_SC_R_DMA_5_CH0		541
> +#define IMX_SC_R_DMA_5_CH1		542
> +#define IMX_SC_R_DMA_5_CH2		543
> +#define IMX_SC_R_DMA_5_CH3		544
> +#define IMX_SC_R_ATTESTATION		545
> +#define IMX_SC_R_LAST			546
>  
>  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> -- 
> 2.7.4
> 
> 
>

Anson Huang Feb. 19, 2019, 1:21 p.m. UTC | #2

Hi, Marco

Best Regards!
Anson Huang

> -----Original Message-----
> From: Marco Felsch [mailto:[e-mail]]
> Sent: 2019年2月19日 20:52
> To: Anson Huang <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Aisheng Dong <[e-mail]>;
> [e-mail]; [e-mail]; linux-
> [e-mail]; dl-linux-imx <[e-mail]>
> Subject: Re: [PATCH] dt-bindings: imx: update scu resource id headfile
> 
> Hi Anson,
> 
> On 19-02-19 09:01, Anson Huang wrote:
> > Update i.MX SCU resource ID table according to latest system
> > controller firmware.
> 
> will this happen every time the scu firmware gets a update?

If SCU firmware updates this table, then yes, it MUST keep same as SCU firmware,
but such ID table is NOT updated very often, I checked the history in our internal tree, there
are ONLY 3 updates since 2016.

Anson.

> 
> Regards,
> Marco
> 
> > Signed-off-by: Anson Huang <[e-mail]>
> > ---
> >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > +++++++++++++++++++--------------
> >  1 file changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > b/include/dt-bindings/firmware/imx/rsrc.h
> > index 4481f2d..ad747a8 100644
> > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > @@ -36,15 +36,15 @@
> >  #define IMX_SC_R_DC_0_BLIT1		20
> >  #define IMX_SC_R_DC_0_BLIT2		21
> >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > +#define IMX_SC_R_PERF			23
> > +#define IMX_SC_R_UNUSED5		24
> >  #define IMX_SC_R_DC_0_WARP		25
> > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > +#define IMX_SC_R_UNUSED7		26
> > +#define IMX_SC_R_UNUSED8		27
> >  #define IMX_SC_R_DC_0_VIDEO0		28
> >  #define IMX_SC_R_DC_0_VIDEO1		29
> >  #define IMX_SC_R_DC_0_FRAC0		30
> > -#define IMX_SC_R_DC_0_FRAC1		31
> > +#define IMX_SC_R_UNUSED6		31
> >  #define IMX_SC_R_DC_0			32
> >  #define IMX_SC_R_GPU_2_PID0		33
> >  #define IMX_SC_R_DC_0_PLL_0		34
> > @@ -53,17 +53,17 @@
> >  #define IMX_SC_R_DC_1_BLIT1		37
> >  #define IMX_SC_R_DC_1_BLIT2		38
> >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > +#define IMX_SC_R_UNUSED9		40
> > +#define IMX_SC_R_UNUSED10		41
> >  #define IMX_SC_R_DC_1_WARP		42
> > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > +#define IMX_SC_R_UNUSED11		43
> > +#define IMX_SC_R_UNUSED12		44
> >  #define IMX_SC_R_DC_1_VIDEO0		45
> >  #define IMX_SC_R_DC_1_VIDEO1		46
> >  #define IMX_SC_R_DC_1_FRAC0		47
> > -#define IMX_SC_R_DC_1_FRAC1		48
> > +#define IMX_SC_R_UNUSED13		48
> >  #define IMX_SC_R_DC_1			49
> > -#define IMX_SC_R_GPU_3_PID0		50
> > +#define IMX_SC_R_UNUSED14		50
> >  #define IMX_SC_R_DC_1_PLL_0		51
> >  #define IMX_SC_R_DC_1_PLL_1		52
> >  #define IMX_SC_R_SPI_0			53
> > @@ -303,8 +303,8 @@
> >  #define IMX_SC_R_M4_0_UART		287
> >  #define IMX_SC_R_M4_0_I2C		288
> >  #define IMX_SC_R_M4_0_INTMUX		289
> > -#define IMX_SC_R_M4_0_SIM		290
> > -#define IMX_SC_R_M4_0_WDOG		291
> > +#define IMX_SC_R_UNUSED15		290
> > +#define IMX_SC_R_UNUSED16		291
> >  #define IMX_SC_R_M4_0_MU_0B		292
> >  #define IMX_SC_R_M4_0_MU_0A0		293
> >  #define IMX_SC_R_M4_0_MU_0A1		294
> > @@ -323,8 +323,8 @@
> >  #define IMX_SC_R_M4_1_UART		307
> >  #define IMX_SC_R_M4_1_I2C		308
> >  #define IMX_SC_R_M4_1_INTMUX		309
> > -#define IMX_SC_R_M4_1_SIM		310
> > -#define IMX_SC_R_M4_1_WDOG		311
> > +#define IMX_SC_R_UNUSED17		310
> > +#define IMX_SC_R_UNUSED18		311
> >  #define IMX_SC_R_M4_1_MU_0B		312
> >  #define IMX_SC_R_M4_1_MU_0A0		313
> >  #define IMX_SC_R_M4_1_MU_0A1		314
> > @@ -337,7 +337,7 @@
> >  #define IMX_SC_R_IRQSTR_SCU2		321
> >  #define IMX_SC_R_IRQSTR_DSP		322
> >  #define IMX_SC_R_ELCDIF_PLL		323
> > -#define IMX_SC_R_UNUSED6		324
> > +#define IMX_SC_R_OCRAM			324
> >  #define IMX_SC_R_AUDIO_PLL_0		325
> >  #define IMX_SC_R_PI_0			326
> >  #define IMX_SC_R_PI_0_PWM_0		327
> > @@ -554,6 +554,11 @@
> >  #define IMX_SC_R_VPU_MU_3		538
> >  #define IMX_SC_R_VPU_ENC_1		539
> >  #define IMX_SC_R_VPU			540
> > -#define IMX_SC_R_LAST			541
> > +#define IMX_SC_R_DMA_5_CH0		541
> > +#define IMX_SC_R_DMA_5_CH1		542
> > +#define IMX_SC_R_DMA_5_CH2		543
> > +#define IMX_SC_R_DMA_5_CH3		544
> > +#define IMX_SC_R_ATTESTATION		545
> > +#define IMX_SC_R_LAST			546
> >
> >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > --
> > 2.7.4
> >
> >
> >
> 
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> Cfe709ac17a164d82c7d508d6966915fb%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C636861775412076730&amp;sdata=05ZzPf2%2BQF10JXLLs
> OPqDdqTi00BWXNHxmMOsQ1z0yI%3D&amp;reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Marco Felsch Feb. 19, 2019, 2:48 p.m. UTC | #3

Hi Anson,

On 19-02-19 13:21, Anson Huang wrote:
> Hi, Marco
> 
> Best Regards!
> Anson Huang
> 
> > -----Original Message-----
> > From: Marco Felsch [mailto:[e-mail]]
> > 
> > Hi Anson,
> > 
> > On 19-02-19 09:01, Anson Huang wrote:
> > > Update i.MX SCU resource ID table according to latest system
> > > controller firmware.
> > 
> > will this happen every time the scu firmware gets a update?
> 
> If SCU firmware updates this table, then yes, it MUST keep same as SCU firmware,
> but such ID table is NOT updated very often, I checked the history in our internal tree, there
> are ONLY 3 updates since 2016.

Please don't get me wrong, but 3 times since 2016 == every year (since
now) == every 3rd kernel. This seems wrong to me. Can you explain me if
the following use-case will work:

Starting point:
 - firmware (incl. bootloader, dtb, scu and other nxp closed source fw)
   from 2017
 - kernel from 2017

Now the project update the kernel to a version from 2019, but the fw
won't be updated.

There are a lot of projects using such a approach to retrieve security
updates.

I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0) by mark
them as unused or even worse give them a other meaning. IMHO the scu-api
should be stable since day 1 and the ID's should only be extended.
Marking ID's as deprecated is much better than moving them around.

Regards,
Marco

> 
> Anson.
> 
> > 
> > Regards,
> > Marco
> > 
> > > Signed-off-by: Anson Huang <[e-mail]>
> > > ---
> > >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > > +++++++++++++++++++--------------
> > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > index 4481f2d..ad747a8 100644
> > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > @@ -36,15 +36,15 @@
> > >  #define IMX_SC_R_DC_0_BLIT1		20
> > >  #define IMX_SC_R_DC_0_BLIT2		21
> > >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > > +#define IMX_SC_R_PERF			23
> > > +#define IMX_SC_R_UNUSED5		24
> > >  #define IMX_SC_R_DC_0_WARP		25
> > > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > > +#define IMX_SC_R_UNUSED7		26
> > > +#define IMX_SC_R_UNUSED8		27
> > >  #define IMX_SC_R_DC_0_VIDEO0		28
> > >  #define IMX_SC_R_DC_0_VIDEO1		29
> > >  #define IMX_SC_R_DC_0_FRAC0		30
> > > -#define IMX_SC_R_DC_0_FRAC1		31
> > > +#define IMX_SC_R_UNUSED6		31
> > >  #define IMX_SC_R_DC_0			32
> > >  #define IMX_SC_R_GPU_2_PID0		33
> > >  #define IMX_SC_R_DC_0_PLL_0		34
> > > @@ -53,17 +53,17 @@
> > >  #define IMX_SC_R_DC_1_BLIT1		37
> > >  #define IMX_SC_R_DC_1_BLIT2		38
> > >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > > +#define IMX_SC_R_UNUSED9		40
> > > +#define IMX_SC_R_UNUSED10		41
> > >  #define IMX_SC_R_DC_1_WARP		42
> > > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > > +#define IMX_SC_R_UNUSED11		43
> > > +#define IMX_SC_R_UNUSED12		44
> > >  #define IMX_SC_R_DC_1_VIDEO0		45
> > >  #define IMX_SC_R_DC_1_VIDEO1		46
> > >  #define IMX_SC_R_DC_1_FRAC0		47
> > > -#define IMX_SC_R_DC_1_FRAC1		48
> > > +#define IMX_SC_R_UNUSED13		48
> > >  #define IMX_SC_R_DC_1			49
> > > -#define IMX_SC_R_GPU_3_PID0		50
> > > +#define IMX_SC_R_UNUSED14		50
> > >  #define IMX_SC_R_DC_1_PLL_0		51
> > >  #define IMX_SC_R_DC_1_PLL_1		52
> > >  #define IMX_SC_R_SPI_0			53
> > > @@ -303,8 +303,8 @@
> > >  #define IMX_SC_R_M4_0_UART		287
> > >  #define IMX_SC_R_M4_0_I2C		288
> > >  #define IMX_SC_R_M4_0_INTMUX		289
> > > -#define IMX_SC_R_M4_0_SIM		290
> > > -#define IMX_SC_R_M4_0_WDOG		291
> > > +#define IMX_SC_R_UNUSED15		290
> > > +#define IMX_SC_R_UNUSED16		291
> > >  #define IMX_SC_R_M4_0_MU_0B		292
> > >  #define IMX_SC_R_M4_0_MU_0A0		293
> > >  #define IMX_SC_R_M4_0_MU_0A1		294
> > > @@ -323,8 +323,8 @@
> > >  #define IMX_SC_R_M4_1_UART		307
> > >  #define IMX_SC_R_M4_1_I2C		308
> > >  #define IMX_SC_R_M4_1_INTMUX		309
> > > -#define IMX_SC_R_M4_1_SIM		310
> > > -#define IMX_SC_R_M4_1_WDOG		311
> > > +#define IMX_SC_R_UNUSED17		310
> > > +#define IMX_SC_R_UNUSED18		311
> > >  #define IMX_SC_R_M4_1_MU_0B		312
> > >  #define IMX_SC_R_M4_1_MU_0A0		313
> > >  #define IMX_SC_R_M4_1_MU_0A1		314
> > > @@ -337,7 +337,7 @@
> > >  #define IMX_SC_R_IRQSTR_SCU2		321
> > >  #define IMX_SC_R_IRQSTR_DSP		322
> > >  #define IMX_SC_R_ELCDIF_PLL		323
> > > -#define IMX_SC_R_UNUSED6		324
> > > +#define IMX_SC_R_OCRAM			324
> > >  #define IMX_SC_R_AUDIO_PLL_0		325
> > >  #define IMX_SC_R_PI_0			326
> > >  #define IMX_SC_R_PI_0_PWM_0		327
> > > @@ -554,6 +554,11 @@
> > >  #define IMX_SC_R_VPU_MU_3		538
> > >  #define IMX_SC_R_VPU_ENC_1		539
> > >  #define IMX_SC_R_VPU			540
> > > -#define IMX_SC_R_LAST			541
> > > +#define IMX_SC_R_DMA_5_CH0		541
> > > +#define IMX_SC_R_DMA_5_CH1		542
> > > +#define IMX_SC_R_DMA_5_CH2		543
> > > +#define IMX_SC_R_DMA_5_CH3		544
> > > +#define IMX_SC_R_ATTESTATION		545
> > > +#define IMX_SC_R_LAST			546
> > >
> > >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > > --
> > > 2.7.4
> > >
> > >
> > >
> > 
> > --
> > Pengutronix e.K.                           |                             |
> > Industrial Linux Solutions                 |
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> > Cfe709ac17a164d82c7d508d6966915fb%7C686ea1d3bc2b4c6fa92cd99c5c301
> > 635%7C0%7C0%7C636861775412076730&amp;sdata=05ZzPf2%2BQF10JXLLs
> > OPqDdqTi00BWXNHxmMOsQ1z0yI%3D&amp;reserved=0  |
> > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Anson Huang Feb. 19, 2019, 3:28 p.m. UTC | #4

Hi, Marco

Best Regards!
Anson Huang

> -----Original Message-----
> From: Marco Felsch [mailto:[e-mail]]
> Sent: 2019年2月19日 22:48
> To: Anson Huang <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; Aisheng Dong <[e-mail]>;
> [e-mail]; [e-mail]; linux-
> [e-mail]; dl-linux-imx <[e-mail]>
> Subject: Re: [PATCH] dt-bindings: imx: update scu resource id headfile
> 
> Hi Anson,
> 
> On 19-02-19 13:21, Anson Huang wrote:
> > Hi, Marco
> >
> > Best Regards!
> > Anson Huang
> >
> > > -----Original Message-----
> > > From: Marco Felsch [mailto:[e-mail]]
> > >
> > > Hi Anson,
> > >
> > > On 19-02-19 09:01, Anson Huang wrote:
> > > > Update i.MX SCU resource ID table according to latest system
> > > > controller firmware.
> > >
> > > will this happen every time the scu firmware gets a update?
> >
> > If SCU firmware updates this table, then yes, it MUST keep same as SCU
> > firmware, but such ID table is NOT updated very often, I checked the
> > history in our internal tree, there are ONLY 3 updates since 2016.
> 
> Please don't get me wrong, but 3 times since 2016 == every year (since
> now) == every 3rd kernel. This seems wrong to me. Can you explain me if the
> following use-case will work:

3 times since 2016 does NOT means it will be updated every year, it is just
because this is a SoC with new architecture, so there could be some changes
before the firmware version got stable. 

> 
> Starting point:
>  - firmware (incl. bootloader, dtb, scu and other nxp closed source fw)
>    from 2017
>  - kernel from 2017
> 
> Now the project update the kernel to a version from 2019, but the fw won't
> be updated.

I think for now, if using 2019 kernel version and 2017 firmware version, it should
be still working because the features depends on SCU firmware in current kernel
are NOT changed in recent SCU firmware update, the SCU firmware update normally
target to support new features or fixing bugs, stable/basic features will NOT be
changed I think. 

> 
> There are a lot of projects using such a approach to retrieve security updates.
> 
> I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0) by mark
> them as unused or even worse give them a other meaning. IMHO the scu-api
> should be stable since day 1 and the ID's should only be extended.
> Marking ID's as deprecated is much better than moving them around.

I agree the SCU APIs should be stable since day 1 and the ID should ONLY be extended,
but that is the best cases, the reality is, there are different SoCs/Revision, some revisions
may remove the resources ID defined in pre-coded SCU firmware, like the IMX_SC_R_DC_0_CAPTURE0 etc.,
so SCU APIs removes them after real silicon arrived, now latest SCU firmware marks them as UNUSED,
they could be replaced by some other new resources in later new SoC, I am NOT sure, but if it happens,
this resource ID table should be updated anyway, leaving the out-of-date resource ID table there will have issues,
since it is NOT sync with SCU firmware.

So how to resolve such issue? We hope the SCU firmware should be stable since day 1, but
the truth is NOT, could be still some updates but NOT very often. And I believe the updates will
NOT break old kernel version.

Anson.

> 
> Regards,
> Marco
> 
> >
> > Anson.
> >
> > >
> > > Regards,
> > > Marco
> > >
> > > > Signed-off-by: Anson Huang <[e-mail]>
> > > > ---
> > > >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > > > +++++++++++++++++++--------------
> > > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > > index 4481f2d..ad747a8 100644
> > > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > > @@ -36,15 +36,15 @@
> > > >  #define IMX_SC_R_DC_0_BLIT1		20
> > > >  #define IMX_SC_R_DC_0_BLIT2		21
> > > >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > > > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > > > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > > > +#define IMX_SC_R_PERF			23
> > > > +#define IMX_SC_R_UNUSED5		24
> > > >  #define IMX_SC_R_DC_0_WARP		25
> > > > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > > > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > > > +#define IMX_SC_R_UNUSED7		26
> > > > +#define IMX_SC_R_UNUSED8		27
> > > >  #define IMX_SC_R_DC_0_VIDEO0		28
> > > >  #define IMX_SC_R_DC_0_VIDEO1		29
> > > >  #define IMX_SC_R_DC_0_FRAC0		30
> > > > -#define IMX_SC_R_DC_0_FRAC1		31
> > > > +#define IMX_SC_R_UNUSED6		31
> > > >  #define IMX_SC_R_DC_0			32
> > > >  #define IMX_SC_R_GPU_2_PID0		33
> > > >  #define IMX_SC_R_DC_0_PLL_0		34
> > > > @@ -53,17 +53,17 @@
> > > >  #define IMX_SC_R_DC_1_BLIT1		37
> > > >  #define IMX_SC_R_DC_1_BLIT2		38
> > > >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > > > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > > > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > > > +#define IMX_SC_R_UNUSED9		40
> > > > +#define IMX_SC_R_UNUSED10		41
> > > >  #define IMX_SC_R_DC_1_WARP		42
> > > > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > > > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > > > +#define IMX_SC_R_UNUSED11		43
> > > > +#define IMX_SC_R_UNUSED12		44
> > > >  #define IMX_SC_R_DC_1_VIDEO0		45
> > > >  #define IMX_SC_R_DC_1_VIDEO1		46
> > > >  #define IMX_SC_R_DC_1_FRAC0		47
> > > > -#define IMX_SC_R_DC_1_FRAC1		48
> > > > +#define IMX_SC_R_UNUSED13		48
> > > >  #define IMX_SC_R_DC_1			49
> > > > -#define IMX_SC_R_GPU_3_PID0		50
> > > > +#define IMX_SC_R_UNUSED14		50
> > > >  #define IMX_SC_R_DC_1_PLL_0		51
> > > >  #define IMX_SC_R_DC_1_PLL_1		52
> > > >  #define IMX_SC_R_SPI_0			53
> > > > @@ -303,8 +303,8 @@
> > > >  #define IMX_SC_R_M4_0_UART		287
> > > >  #define IMX_SC_R_M4_0_I2C		288
> > > >  #define IMX_SC_R_M4_0_INTMUX		289
> > > > -#define IMX_SC_R_M4_0_SIM		290
> > > > -#define IMX_SC_R_M4_0_WDOG		291
> > > > +#define IMX_SC_R_UNUSED15		290
> > > > +#define IMX_SC_R_UNUSED16		291
> > > >  #define IMX_SC_R_M4_0_MU_0B		292
> > > >  #define IMX_SC_R_M4_0_MU_0A0		293
> > > >  #define IMX_SC_R_M4_0_MU_0A1		294
> > > > @@ -323,8 +323,8 @@
> > > >  #define IMX_SC_R_M4_1_UART		307
> > > >  #define IMX_SC_R_M4_1_I2C		308
> > > >  #define IMX_SC_R_M4_1_INTMUX		309
> > > > -#define IMX_SC_R_M4_1_SIM		310
> > > > -#define IMX_SC_R_M4_1_WDOG		311
> > > > +#define IMX_SC_R_UNUSED17		310
> > > > +#define IMX_SC_R_UNUSED18		311
> > > >  #define IMX_SC_R_M4_1_MU_0B		312
> > > >  #define IMX_SC_R_M4_1_MU_0A0		313
> > > >  #define IMX_SC_R_M4_1_MU_0A1		314
> > > > @@ -337,7 +337,7 @@
> > > >  #define IMX_SC_R_IRQSTR_SCU2		321
> > > >  #define IMX_SC_R_IRQSTR_DSP		322
> > > >  #define IMX_SC_R_ELCDIF_PLL		323
> > > > -#define IMX_SC_R_UNUSED6		324
> > > > +#define IMX_SC_R_OCRAM			324
> > > >  #define IMX_SC_R_AUDIO_PLL_0		325
> > > >  #define IMX_SC_R_PI_0			326
> > > >  #define IMX_SC_R_PI_0_PWM_0		327
> > > > @@ -554,6 +554,11 @@
> > > >  #define IMX_SC_R_VPU_MU_3		538
> > > >  #define IMX_SC_R_VPU_ENC_1		539
> > > >  #define IMX_SC_R_VPU			540
> > > > -#define IMX_SC_R_LAST			541
> > > > +#define IMX_SC_R_DMA_5_CH0		541
> > > > +#define IMX_SC_R_DMA_5_CH1		542
> > > > +#define IMX_SC_R_DMA_5_CH2		543
> > > > +#define IMX_SC_R_DMA_5_CH3		544
> > > > +#define IMX_SC_R_ATTESTATION		545
> > > > +#define IMX_SC_R_LAST			546
> > > >
> > > >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > > > --
> > > > 2.7.4
> > > >
> > > >
> > > >
> > >
> > > --
> > > Pengutronix e.K.                           |                             |
> > > Industrial Linux Solutions                 |
> > >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > >
> w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> > >
> Cfe709ac17a164d82c7d508d6966915fb%7C686ea1d3bc2b4c6fa92cd99c5c301
> > >
> 635%7C0%7C0%7C636861775412076730&amp;sdata=05ZzPf2%2BQF10JXLLs
> > > OPqDdqTi00BWXNHxmMOsQ1z0yI%3D&amp;reserved=0  |
> > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]
> |
> > > Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |
> 
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> Cd5d39730435e4b06549c08d696794904%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C636861845015130502&amp;sdata=tQYrNl5lzIRRNVBCji6A
> sPREOnIfDgdPWgAnsWyCErg%3D&amp;reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Dong Aisheng Feb. 20, 2019, 3:29 a.m. UTC | #5

> From: Anson Huang
> Sent: Tuesday, February 19, 2019 5:01 PM
> Subject: [PATCH] dt-bindings: imx: update scu resource id headfile
> 
> Update i.MX SCU resource ID table according to latest system controller
> firmware.
> 

You need at least explain what changes made like
what new features added? What removed? Side affect if any?

Regards
Dong Aisheng

> Signed-off-by: Anson Huang <[e-mail]>
> ---
>  include/dt-bindings/firmware/imx/rsrc.h | 39
> +++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> b/include/dt-bindings/firmware/imx/rsrc.h
> index 4481f2d..ad747a8 100644
> --- a/include/dt-bindings/firmware/imx/rsrc.h
> +++ b/include/dt-bindings/firmware/imx/rsrc.h
> @@ -36,15 +36,15 @@
>  #define IMX_SC_R_DC_0_BLIT1		20
>  #define IMX_SC_R_DC_0_BLIT2		21
>  #define IMX_SC_R_DC_0_BLIT_OUT		22
> -#define IMX_SC_R_DC_0_CAPTURE0		23
> -#define IMX_SC_R_DC_0_CAPTURE1		24
> +#define IMX_SC_R_PERF			23
> +#define IMX_SC_R_UNUSED5		24
>  #define IMX_SC_R_DC_0_WARP		25
> -#define IMX_SC_R_DC_0_INTEGRAL0		26
> -#define IMX_SC_R_DC_0_INTEGRAL1		27
> +#define IMX_SC_R_UNUSED7		26
> +#define IMX_SC_R_UNUSED8		27
>  #define IMX_SC_R_DC_0_VIDEO0		28
>  #define IMX_SC_R_DC_0_VIDEO1		29
>  #define IMX_SC_R_DC_0_FRAC0		30
> -#define IMX_SC_R_DC_0_FRAC1		31
> +#define IMX_SC_R_UNUSED6		31
>  #define IMX_SC_R_DC_0			32
>  #define IMX_SC_R_GPU_2_PID0		33
>  #define IMX_SC_R_DC_0_PLL_0		34
> @@ -53,17 +53,17 @@
>  #define IMX_SC_R_DC_1_BLIT1		37
>  #define IMX_SC_R_DC_1_BLIT2		38
>  #define IMX_SC_R_DC_1_BLIT_OUT		39
> -#define IMX_SC_R_DC_1_CAPTURE0		40
> -#define IMX_SC_R_DC_1_CAPTURE1		41
> +#define IMX_SC_R_UNUSED9		40
> +#define IMX_SC_R_UNUSED10		41
>  #define IMX_SC_R_DC_1_WARP		42
> -#define IMX_SC_R_DC_1_INTEGRAL0		43
> -#define IMX_SC_R_DC_1_INTEGRAL1		44
> +#define IMX_SC_R_UNUSED11		43
> +#define IMX_SC_R_UNUSED12		44
>  #define IMX_SC_R_DC_1_VIDEO0		45
>  #define IMX_SC_R_DC_1_VIDEO1		46
>  #define IMX_SC_R_DC_1_FRAC0		47
> -#define IMX_SC_R_DC_1_FRAC1		48
> +#define IMX_SC_R_UNUSED13		48
>  #define IMX_SC_R_DC_1			49
> -#define IMX_SC_R_GPU_3_PID0		50
> +#define IMX_SC_R_UNUSED14		50
>  #define IMX_SC_R_DC_1_PLL_0		51
>  #define IMX_SC_R_DC_1_PLL_1		52
>  #define IMX_SC_R_SPI_0			53
> @@ -303,8 +303,8 @@
>  #define IMX_SC_R_M4_0_UART		287
>  #define IMX_SC_R_M4_0_I2C		288
>  #define IMX_SC_R_M4_0_INTMUX		289
> -#define IMX_SC_R_M4_0_SIM		290
> -#define IMX_SC_R_M4_0_WDOG		291
> +#define IMX_SC_R_UNUSED15		290
> +#define IMX_SC_R_UNUSED16		291
>  #define IMX_SC_R_M4_0_MU_0B		292
>  #define IMX_SC_R_M4_0_MU_0A0		293
>  #define IMX_SC_R_M4_0_MU_0A1		294
> @@ -323,8 +323,8 @@
>  #define IMX_SC_R_M4_1_UART		307
>  #define IMX_SC_R_M4_1_I2C		308
>  #define IMX_SC_R_M4_1_INTMUX		309
> -#define IMX_SC_R_M4_1_SIM		310
> -#define IMX_SC_R_M4_1_WDOG		311
> +#define IMX_SC_R_UNUSED17		310
> +#define IMX_SC_R_UNUSED18		311
>  #define IMX_SC_R_M4_1_MU_0B		312
>  #define IMX_SC_R_M4_1_MU_0A0		313
>  #define IMX_SC_R_M4_1_MU_0A1		314
> @@ -337,7 +337,7 @@
>  #define IMX_SC_R_IRQSTR_SCU2		321
>  #define IMX_SC_R_IRQSTR_DSP		322
>  #define IMX_SC_R_ELCDIF_PLL		323
> -#define IMX_SC_R_UNUSED6		324
> +#define IMX_SC_R_OCRAM			324
>  #define IMX_SC_R_AUDIO_PLL_0		325
>  #define IMX_SC_R_PI_0			326
>  #define IMX_SC_R_PI_0_PWM_0		327
> @@ -554,6 +554,11 @@
>  #define IMX_SC_R_VPU_MU_3		538
>  #define IMX_SC_R_VPU_ENC_1		539
>  #define IMX_SC_R_VPU			540
> -#define IMX_SC_R_LAST			541
> +#define IMX_SC_R_DMA_5_CH0		541
> +#define IMX_SC_R_DMA_5_CH1		542
> +#define IMX_SC_R_DMA_5_CH2		543
> +#define IMX_SC_R_DMA_5_CH3		544
> +#define IMX_SC_R_ATTESTATION		545
> +#define IMX_SC_R_LAST			546
> 
>  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> --
> 2.7.4

Anson Huang Feb. 20, 2019, 3:36 a.m. UTC | #6

Best Regards!
Anson Huang

> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019年2月20日 11:29
> To: Anson Huang <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; linux-
> [e-mail]
> Cc: dl-linux-imx <[e-mail]>
> Subject: RE: [PATCH] dt-bindings: imx: update scu resource id headfile
> 
> > From: Anson Huang
> > Sent: Tuesday, February 19, 2019 5:01 PM
> > Subject: [PATCH] dt-bindings: imx: update scu resource id headfile
> >
> > Update i.MX SCU resource ID table according to latest system
> > controller firmware.
> >
> 
> You need at least explain what changes made like what new features added?
> What removed? Side affect if any?

No new features added, looks like SCFW just remove some unused resources.
No side-effect, as they are NOT used by anyone.

Anson.

> 
> Regards
> Dong Aisheng
> 
> > Signed-off-by: Anson Huang <[e-mail]>
> > ---
> >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > +++++++++++++++++++--------------
> >  1 file changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > b/include/dt-bindings/firmware/imx/rsrc.h
> > index 4481f2d..ad747a8 100644
> > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > @@ -36,15 +36,15 @@
> >  #define IMX_SC_R_DC_0_BLIT1		20
> >  #define IMX_SC_R_DC_0_BLIT2		21
> >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > +#define IMX_SC_R_PERF			23
> > +#define IMX_SC_R_UNUSED5		24
> >  #define IMX_SC_R_DC_0_WARP		25
> > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > +#define IMX_SC_R_UNUSED7		26
> > +#define IMX_SC_R_UNUSED8		27
> >  #define IMX_SC_R_DC_0_VIDEO0		28
> >  #define IMX_SC_R_DC_0_VIDEO1		29
> >  #define IMX_SC_R_DC_0_FRAC0		30
> > -#define IMX_SC_R_DC_0_FRAC1		31
> > +#define IMX_SC_R_UNUSED6		31
> >  #define IMX_SC_R_DC_0			32
> >  #define IMX_SC_R_GPU_2_PID0		33
> >  #define IMX_SC_R_DC_0_PLL_0		34
> > @@ -53,17 +53,17 @@
> >  #define IMX_SC_R_DC_1_BLIT1		37
> >  #define IMX_SC_R_DC_1_BLIT2		38
> >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > +#define IMX_SC_R_UNUSED9		40
> > +#define IMX_SC_R_UNUSED10		41
> >  #define IMX_SC_R_DC_1_WARP		42
> > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > +#define IMX_SC_R_UNUSED11		43
> > +#define IMX_SC_R_UNUSED12		44
> >  #define IMX_SC_R_DC_1_VIDEO0		45
> >  #define IMX_SC_R_DC_1_VIDEO1		46
> >  #define IMX_SC_R_DC_1_FRAC0		47
> > -#define IMX_SC_R_DC_1_FRAC1		48
> > +#define IMX_SC_R_UNUSED13		48
> >  #define IMX_SC_R_DC_1			49
> > -#define IMX_SC_R_GPU_3_PID0		50
> > +#define IMX_SC_R_UNUSED14		50
> >  #define IMX_SC_R_DC_1_PLL_0		51
> >  #define IMX_SC_R_DC_1_PLL_1		52
> >  #define IMX_SC_R_SPI_0			53
> > @@ -303,8 +303,8 @@
> >  #define IMX_SC_R_M4_0_UART		287
> >  #define IMX_SC_R_M4_0_I2C		288
> >  #define IMX_SC_R_M4_0_INTMUX		289
> > -#define IMX_SC_R_M4_0_SIM		290
> > -#define IMX_SC_R_M4_0_WDOG		291
> > +#define IMX_SC_R_UNUSED15		290
> > +#define IMX_SC_R_UNUSED16		291
> >  #define IMX_SC_R_M4_0_MU_0B		292
> >  #define IMX_SC_R_M4_0_MU_0A0		293
> >  #define IMX_SC_R_M4_0_MU_0A1		294
> > @@ -323,8 +323,8 @@
> >  #define IMX_SC_R_M4_1_UART		307
> >  #define IMX_SC_R_M4_1_I2C		308
> >  #define IMX_SC_R_M4_1_INTMUX		309
> > -#define IMX_SC_R_M4_1_SIM		310
> > -#define IMX_SC_R_M4_1_WDOG		311
> > +#define IMX_SC_R_UNUSED17		310
> > +#define IMX_SC_R_UNUSED18		311
> >  #define IMX_SC_R_M4_1_MU_0B		312
> >  #define IMX_SC_R_M4_1_MU_0A0		313
> >  #define IMX_SC_R_M4_1_MU_0A1		314
> > @@ -337,7 +337,7 @@
> >  #define IMX_SC_R_IRQSTR_SCU2		321
> >  #define IMX_SC_R_IRQSTR_DSP		322
> >  #define IMX_SC_R_ELCDIF_PLL		323
> > -#define IMX_SC_R_UNUSED6		324
> > +#define IMX_SC_R_OCRAM			324
> >  #define IMX_SC_R_AUDIO_PLL_0		325
> >  #define IMX_SC_R_PI_0			326
> >  #define IMX_SC_R_PI_0_PWM_0		327
> > @@ -554,6 +554,11 @@
> >  #define IMX_SC_R_VPU_MU_3		538
> >  #define IMX_SC_R_VPU_ENC_1		539
> >  #define IMX_SC_R_VPU			540
> > -#define IMX_SC_R_LAST			541
> > +#define IMX_SC_R_DMA_5_CH0		541
> > +#define IMX_SC_R_DMA_5_CH1		542
> > +#define IMX_SC_R_DMA_5_CH2		543
> > +#define IMX_SC_R_DMA_5_CH3		544
> > +#define IMX_SC_R_ATTESTATION		545
> > +#define IMX_SC_R_LAST			546
> >
> >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > --
> > 2.7.4

Dong Aisheng Feb. 20, 2019, 3:38 a.m. UTC | #7

[...]

> > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0) by mark
> > them as unused or even worse give them a other meaning. IMHO the
> > scu-api should be stable since day 1 and the ID's should only be extended.
> > Marking ID's as deprecated is much better than moving them around.
> 
> I agree the SCU APIs should be stable since day 1 and the ID should ONLY be
> extended, but that is the best cases, the reality is, there are different
> SoCs/Revision, some revisions may remove the resources ID defined in
> pre-coded SCU firmware, like the IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs
> removes them after real silicon arrived, now latest SCU firmware marks them
> as UNUSED, they could be replaced by some other new resources in later new
> SoC, I am NOT sure, but if it happens, this resource ID table should be updated
> anyway, leaving the out-of-date resource ID table there will have issues, since it
> is NOT sync with SCU firmware.
> 
> So how to resolve such issue? We hope the SCU firmware should be stable
> since day 1, but the truth is NOT, could be still some updates but NOT very
> often. And I believe the updates will NOT break old kernel version.
> 

Please double check with SCU firmware owner what the removed ID are used before?
Any side effect if removing them.

And please also check if the combability can be maintained via IMX_SC_RPC_VERSION?

Regards
Dong Aisheng

> Anson.
> 
> >
> > Regards,
> > Marco
> >
> > >
> > > Anson.
> > >
> > > >
> > > > Regards,
> > > > Marco
> > > >
> > > > > Signed-off-by: Anson Huang <[e-mail]>
> > > > > ---
> > > > >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > > > > +++++++++++++++++++--------------
> > > > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > > > >
> > > > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > > > index 4481f2d..ad747a8 100644
> > > > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > > > @@ -36,15 +36,15 @@
> > > > >  #define IMX_SC_R_DC_0_BLIT1		20
> > > > >  #define IMX_SC_R_DC_0_BLIT2		21
> > > > >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > > > > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > > > > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > > > > +#define IMX_SC_R_PERF			23
> > > > > +#define IMX_SC_R_UNUSED5		24
> > > > >  #define IMX_SC_R_DC_0_WARP		25
> > > > > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > > > > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > > > > +#define IMX_SC_R_UNUSED7		26
> > > > > +#define IMX_SC_R_UNUSED8		27
> > > > >  #define IMX_SC_R_DC_0_VIDEO0		28
> > > > >  #define IMX_SC_R_DC_0_VIDEO1		29
> > > > >  #define IMX_SC_R_DC_0_FRAC0		30
> > > > > -#define IMX_SC_R_DC_0_FRAC1		31
> > > > > +#define IMX_SC_R_UNUSED6		31
> > > > >  #define IMX_SC_R_DC_0			32
> > > > >  #define IMX_SC_R_GPU_2_PID0		33
> > > > >  #define IMX_SC_R_DC_0_PLL_0		34
> > > > > @@ -53,17 +53,17 @@
> > > > >  #define IMX_SC_R_DC_1_BLIT1		37
> > > > >  #define IMX_SC_R_DC_1_BLIT2		38
> > > > >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > > > > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > > > > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > > > > +#define IMX_SC_R_UNUSED9		40
> > > > > +#define IMX_SC_R_UNUSED10		41
> > > > >  #define IMX_SC_R_DC_1_WARP		42
> > > > > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > > > > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > > > > +#define IMX_SC_R_UNUSED11		43
> > > > > +#define IMX_SC_R_UNUSED12		44
> > > > >  #define IMX_SC_R_DC_1_VIDEO0		45
> > > > >  #define IMX_SC_R_DC_1_VIDEO1		46
> > > > >  #define IMX_SC_R_DC_1_FRAC0		47
> > > > > -#define IMX_SC_R_DC_1_FRAC1		48
> > > > > +#define IMX_SC_R_UNUSED13		48
> > > > >  #define IMX_SC_R_DC_1			49
> > > > > -#define IMX_SC_R_GPU_3_PID0		50
> > > > > +#define IMX_SC_R_UNUSED14		50
> > > > >  #define IMX_SC_R_DC_1_PLL_0		51
> > > > >  #define IMX_SC_R_DC_1_PLL_1		52
> > > > >  #define IMX_SC_R_SPI_0			53
> > > > > @@ -303,8 +303,8 @@
> > > > >  #define IMX_SC_R_M4_0_UART		287
> > > > >  #define IMX_SC_R_M4_0_I2C		288
> > > > >  #define IMX_SC_R_M4_0_INTMUX		289
> > > > > -#define IMX_SC_R_M4_0_SIM		290
> > > > > -#define IMX_SC_R_M4_0_WDOG		291
> > > > > +#define IMX_SC_R_UNUSED15		290
> > > > > +#define IMX_SC_R_UNUSED16		291
> > > > >  #define IMX_SC_R_M4_0_MU_0B		292
> > > > >  #define IMX_SC_R_M4_0_MU_0A0		293
> > > > >  #define IMX_SC_R_M4_0_MU_0A1		294
> > > > > @@ -323,8 +323,8 @@
> > > > >  #define IMX_SC_R_M4_1_UART		307
> > > > >  #define IMX_SC_R_M4_1_I2C		308
> > > > >  #define IMX_SC_R_M4_1_INTMUX		309
> > > > > -#define IMX_SC_R_M4_1_SIM		310
> > > > > -#define IMX_SC_R_M4_1_WDOG		311
> > > > > +#define IMX_SC_R_UNUSED17		310
> > > > > +#define IMX_SC_R_UNUSED18		311
> > > > >  #define IMX_SC_R_M4_1_MU_0B		312
> > > > >  #define IMX_SC_R_M4_1_MU_0A0		313
> > > > >  #define IMX_SC_R_M4_1_MU_0A1		314
> > > > > @@ -337,7 +337,7 @@
> > > > >  #define IMX_SC_R_IRQSTR_SCU2		321
> > > > >  #define IMX_SC_R_IRQSTR_DSP		322
> > > > >  #define IMX_SC_R_ELCDIF_PLL		323
> > > > > -#define IMX_SC_R_UNUSED6		324
> > > > > +#define IMX_SC_R_OCRAM			324
> > > > >  #define IMX_SC_R_AUDIO_PLL_0		325
> > > > >  #define IMX_SC_R_PI_0			326
> > > > >  #define IMX_SC_R_PI_0_PWM_0		327
> > > > > @@ -554,6 +554,11 @@
> > > > >  #define IMX_SC_R_VPU_MU_3		538
> > > > >  #define IMX_SC_R_VPU_ENC_1		539
> > > > >  #define IMX_SC_R_VPU			540
> > > > > -#define IMX_SC_R_LAST			541
> > > > > +#define IMX_SC_R_DMA_5_CH0		541
> > > > > +#define IMX_SC_R_DMA_5_CH1		542
> > > > > +#define IMX_SC_R_DMA_5_CH2		543
> > > > > +#define IMX_SC_R_DMA_5_CH3		544
> > > > > +#define IMX_SC_R_ATTESTATION		545
> > > > > +#define IMX_SC_R_LAST			546
> > > > >
> > > > >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > > > > --
> > > > > 2.7.4
> > > > >
> > > > >
> > > > >
> > > >
> > > > --
> > > > Pengutronix e.K.                           |
> |
> > > > Industrial Linux Solutions                 |
> > > >
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > > >
> > w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> > > >
> >
> Cfe709ac17a164d82c7d508d6966915fb%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1
> > > >
> >
> 635%7C0%7C0%7C636861775412076730&amp;sdata=05ZzPf2%2BQF10JXLLs
> > > > OPqDdqTi00BWXNHxmMOsQ1z0yI%3D&amp;reserved=0  | Peiner Str.
> 6-8,
> > > > 31137 Hildesheim, Germany | Phone: [phone]
> > |
> > > > Amtsgericht Hildesheim, HRA 2686           | Fax:
> [phone]-5555 |
> >
> > --
> > Pengutronix e.K.                           |
> |
> > Industrial Linux Solutions                 |
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> >
> Cd5d39730435e4b06549c08d696794904%7C686ea1d3bc2b4c6fa92cd99c5c
> 30
> >
> 1635%7C0%7C0%7C636861845015130502&amp;sdata=tQYrNl5lzIRRNVBCji6
> A
> > sPREOnIfDgdPWgAnsWyCErg%3D&amp;reserved=0  |
> > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]
> |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:
> [phone]-5555 |

Dong Aisheng Feb. 20, 2019, 3:43 a.m. UTC | #8

> >
> > You need at least explain what changes made like what new features added?
> > What removed? Side affect if any?
> 
> No new features added, looks like SCFW just remove some unused resources.
> No side-effect, as they are NOT used by anyone.
> 

That seems not true.
I see some new IDs added.
e.g. IMX_SC_R_PERF, IMX_SC_R_OCRAM

Regards
Dong Aisheng

> Anson.
> 
> >
> > Regards
> > Dong Aisheng
> >
> > > Signed-off-by: Anson Huang <[e-mail]>
> > > ---
> > >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > > +++++++++++++++++++--------------
> > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > index 4481f2d..ad747a8 100644
> > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > @@ -36,15 +36,15 @@
> > >  #define IMX_SC_R_DC_0_BLIT1		20
> > >  #define IMX_SC_R_DC_0_BLIT2		21
> > >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > > +#define IMX_SC_R_PERF			23
> > > +#define IMX_SC_R_UNUSED5		24
> > >  #define IMX_SC_R_DC_0_WARP		25
> > > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > > +#define IMX_SC_R_UNUSED7		26
> > > +#define IMX_SC_R_UNUSED8		27
> > >  #define IMX_SC_R_DC_0_VIDEO0		28
> > >  #define IMX_SC_R_DC_0_VIDEO1		29
> > >  #define IMX_SC_R_DC_0_FRAC0		30
> > > -#define IMX_SC_R_DC_0_FRAC1		31
> > > +#define IMX_SC_R_UNUSED6		31
> > >  #define IMX_SC_R_DC_0			32
> > >  #define IMX_SC_R_GPU_2_PID0		33
> > >  #define IMX_SC_R_DC_0_PLL_0		34
> > > @@ -53,17 +53,17 @@
> > >  #define IMX_SC_R_DC_1_BLIT1		37
> > >  #define IMX_SC_R_DC_1_BLIT2		38
> > >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > > +#define IMX_SC_R_UNUSED9		40
> > > +#define IMX_SC_R_UNUSED10		41
> > >  #define IMX_SC_R_DC_1_WARP		42
> > > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > > +#define IMX_SC_R_UNUSED11		43
> > > +#define IMX_SC_R_UNUSED12		44
> > >  #define IMX_SC_R_DC_1_VIDEO0		45
> > >  #define IMX_SC_R_DC_1_VIDEO1		46
> > >  #define IMX_SC_R_DC_1_FRAC0		47
> > > -#define IMX_SC_R_DC_1_FRAC1		48
> > > +#define IMX_SC_R_UNUSED13		48
> > >  #define IMX_SC_R_DC_1			49
> > > -#define IMX_SC_R_GPU_3_PID0		50
> > > +#define IMX_SC_R_UNUSED14		50
> > >  #define IMX_SC_R_DC_1_PLL_0		51
> > >  #define IMX_SC_R_DC_1_PLL_1		52
> > >  #define IMX_SC_R_SPI_0			53
> > > @@ -303,8 +303,8 @@
> > >  #define IMX_SC_R_M4_0_UART		287
> > >  #define IMX_SC_R_M4_0_I2C		288
> > >  #define IMX_SC_R_M4_0_INTMUX		289
> > > -#define IMX_SC_R_M4_0_SIM		290
> > > -#define IMX_SC_R_M4_0_WDOG		291
> > > +#define IMX_SC_R_UNUSED15		290
> > > +#define IMX_SC_R_UNUSED16		291
> > >  #define IMX_SC_R_M4_0_MU_0B		292
> > >  #define IMX_SC_R_M4_0_MU_0A0		293
> > >  #define IMX_SC_R_M4_0_MU_0A1		294
> > > @@ -323,8 +323,8 @@
> > >  #define IMX_SC_R_M4_1_UART		307
> > >  #define IMX_SC_R_M4_1_I2C		308
> > >  #define IMX_SC_R_M4_1_INTMUX		309
> > > -#define IMX_SC_R_M4_1_SIM		310
> > > -#define IMX_SC_R_M4_1_WDOG		311
> > > +#define IMX_SC_R_UNUSED17		310
> > > +#define IMX_SC_R_UNUSED18		311
> > >  #define IMX_SC_R_M4_1_MU_0B		312
> > >  #define IMX_SC_R_M4_1_MU_0A0		313
> > >  #define IMX_SC_R_M4_1_MU_0A1		314
> > > @@ -337,7 +337,7 @@
> > >  #define IMX_SC_R_IRQSTR_SCU2		321
> > >  #define IMX_SC_R_IRQSTR_DSP		322
> > >  #define IMX_SC_R_ELCDIF_PLL		323
> > > -#define IMX_SC_R_UNUSED6		324
> > > +#define IMX_SC_R_OCRAM			324
> > >  #define IMX_SC_R_AUDIO_PLL_0		325
> > >  #define IMX_SC_R_PI_0			326
> > >  #define IMX_SC_R_PI_0_PWM_0		327
> > > @@ -554,6 +554,11 @@
> > >  #define IMX_SC_R_VPU_MU_3		538
> > >  #define IMX_SC_R_VPU_ENC_1		539
> > >  #define IMX_SC_R_VPU			540
> > > -#define IMX_SC_R_LAST			541
> > > +#define IMX_SC_R_DMA_5_CH0		541
> > > +#define IMX_SC_R_DMA_5_CH1		542
> > > +#define IMX_SC_R_DMA_5_CH2		543
> > > +#define IMX_SC_R_DMA_5_CH3		544
> > > +#define IMX_SC_R_ATTESTATION		545
> > > +#define IMX_SC_R_LAST			546
> > >
> > >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > > --
> > > 2.7.4

Anson Huang Feb. 20, 2019, 3:46 a.m. UTC | #9

Best Regards!
Anson Huang

> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019年2月20日 11:43
> To: Anson Huang <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; linux-
> [e-mail]
> Cc: dl-linux-imx <[e-mail]>
> Subject: RE: [PATCH] dt-bindings: imx: update scu resource id headfile
> 
> > >
> > > You need at least explain what changes made like what new features
> added?
> > > What removed? Side affect if any?
> >
> > No new features added, looks like SCFW just remove some unused
> resources.
> > No side-effect, as they are NOT used by anyone.
> >
> 
> That seems not true.
> I see some new IDs added.
> e.g. IMX_SC_R_PERF, IMX_SC_R_OCRAM

When I said no new features added, I mean for kernel side, SCFW may add new resource/feature
always, but that does NOT impact our kernel if we do NOT enable these features so far.

Anson.

> 
> Regards
> Dong Aisheng
> 
> > Anson.
> >
> > >
> > > Regards
> > > Dong Aisheng
> > >
> > > > Signed-off-by: Anson Huang <[e-mail]>
> > > > ---
> > > >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > > > +++++++++++++++++++--------------
> > > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > > index 4481f2d..ad747a8 100644
> > > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > > @@ -36,15 +36,15 @@
> > > >  #define IMX_SC_R_DC_0_BLIT1		20
> > > >  #define IMX_SC_R_DC_0_BLIT2		21
> > > >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > > > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > > > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > > > +#define IMX_SC_R_PERF			23
> > > > +#define IMX_SC_R_UNUSED5		24
> > > >  #define IMX_SC_R_DC_0_WARP		25
> > > > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > > > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > > > +#define IMX_SC_R_UNUSED7		26
> > > > +#define IMX_SC_R_UNUSED8		27
> > > >  #define IMX_SC_R_DC_0_VIDEO0		28
> > > >  #define IMX_SC_R_DC_0_VIDEO1		29
> > > >  #define IMX_SC_R_DC_0_FRAC0		30
> > > > -#define IMX_SC_R_DC_0_FRAC1		31
> > > > +#define IMX_SC_R_UNUSED6		31
> > > >  #define IMX_SC_R_DC_0			32
> > > >  #define IMX_SC_R_GPU_2_PID0		33
> > > >  #define IMX_SC_R_DC_0_PLL_0		34
> > > > @@ -53,17 +53,17 @@
> > > >  #define IMX_SC_R_DC_1_BLIT1		37
> > > >  #define IMX_SC_R_DC_1_BLIT2		38
> > > >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > > > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > > > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > > > +#define IMX_SC_R_UNUSED9		40
> > > > +#define IMX_SC_R_UNUSED10		41
> > > >  #define IMX_SC_R_DC_1_WARP		42
> > > > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > > > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > > > +#define IMX_SC_R_UNUSED11		43
> > > > +#define IMX_SC_R_UNUSED12		44
> > > >  #define IMX_SC_R_DC_1_VIDEO0		45
> > > >  #define IMX_SC_R_DC_1_VIDEO1		46
> > > >  #define IMX_SC_R_DC_1_FRAC0		47
> > > > -#define IMX_SC_R_DC_1_FRAC1		48
> > > > +#define IMX_SC_R_UNUSED13		48
> > > >  #define IMX_SC_R_DC_1			49
> > > > -#define IMX_SC_R_GPU_3_PID0		50
> > > > +#define IMX_SC_R_UNUSED14		50
> > > >  #define IMX_SC_R_DC_1_PLL_0		51
> > > >  #define IMX_SC_R_DC_1_PLL_1		52
> > > >  #define IMX_SC_R_SPI_0			53
> > > > @@ -303,8 +303,8 @@
> > > >  #define IMX_SC_R_M4_0_UART		287
> > > >  #define IMX_SC_R_M4_0_I2C		288
> > > >  #define IMX_SC_R_M4_0_INTMUX		289
> > > > -#define IMX_SC_R_M4_0_SIM		290
> > > > -#define IMX_SC_R_M4_0_WDOG		291
> > > > +#define IMX_SC_R_UNUSED15		290
> > > > +#define IMX_SC_R_UNUSED16		291
> > > >  #define IMX_SC_R_M4_0_MU_0B		292
> > > >  #define IMX_SC_R_M4_0_MU_0A0		293
> > > >  #define IMX_SC_R_M4_0_MU_0A1		294
> > > > @@ -323,8 +323,8 @@
> > > >  #define IMX_SC_R_M4_1_UART		307
> > > >  #define IMX_SC_R_M4_1_I2C		308
> > > >  #define IMX_SC_R_M4_1_INTMUX		309
> > > > -#define IMX_SC_R_M4_1_SIM		310
> > > > -#define IMX_SC_R_M4_1_WDOG		311
> > > > +#define IMX_SC_R_UNUSED17		310
> > > > +#define IMX_SC_R_UNUSED18		311
> > > >  #define IMX_SC_R_M4_1_MU_0B		312
> > > >  #define IMX_SC_R_M4_1_MU_0A0		313
> > > >  #define IMX_SC_R_M4_1_MU_0A1		314
> > > > @@ -337,7 +337,7 @@
> > > >  #define IMX_SC_R_IRQSTR_SCU2		321
> > > >  #define IMX_SC_R_IRQSTR_DSP		322
> > > >  #define IMX_SC_R_ELCDIF_PLL		323
> > > > -#define IMX_SC_R_UNUSED6		324
> > > > +#define IMX_SC_R_OCRAM			324
> > > >  #define IMX_SC_R_AUDIO_PLL_0		325
> > > >  #define IMX_SC_R_PI_0			326
> > > >  #define IMX_SC_R_PI_0_PWM_0		327
> > > > @@ -554,6 +554,11 @@
> > > >  #define IMX_SC_R_VPU_MU_3		538
> > > >  #define IMX_SC_R_VPU_ENC_1		539
> > > >  #define IMX_SC_R_VPU			540
> > > > -#define IMX_SC_R_LAST			541
> > > > +#define IMX_SC_R_DMA_5_CH0		541
> > > > +#define IMX_SC_R_DMA_5_CH1		542
> > > > +#define IMX_SC_R_DMA_5_CH2		543
> > > > +#define IMX_SC_R_DMA_5_CH3		544
> > > > +#define IMX_SC_R_ATTESTATION		545
> > > > +#define IMX_SC_R_LAST			546
> > > >
> > > >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > > > --
> > > > 2.7.4

Marco Felsch Feb. 20, 2019, 8:16 a.m. UTC | #10

On 19-02-20 03:38, Aisheng Dong wrote:
> [...]
> 
> > > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0) by mark
> > > them as unused or even worse give them a other meaning. IMHO the
> > > scu-api should be stable since day 1 and the ID's should only be extended.
> > > Marking ID's as deprecated is much better than moving them around.
> > 
> > I agree the SCU APIs should be stable since day 1 and the ID should ONLY be
> > extended, but that is the best cases, the reality is, there are different
> > SoCs/Revision, some revisions may remove the resources ID defined in
> > pre-coded SCU firmware, like the IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs
> > removes them after real silicon arrived, now latest SCU firmware marks them
> > as UNUSED, they could be replaced by some other new resources in later new
> > SoC, I am NOT sure, but if it happens, this resource ID table should be updated
> > anyway, leaving the out-of-date resource ID table there will have issues, since it
> > is NOT sync with SCU firmware.
> > 
> > So how to resolve such issue? We hope the SCU firmware should be stable
> > since day 1, but the truth is NOT, could be still some updates but NOT very
> > often. And I believe the updates will NOT break old kernel version.

Hi Anson,

Please don't mix the dt-bindings and the kernel related stuff.
Unfortunately the bindings are within the kernel repo which in fact is
great for us kernel developer but the bindings are also used in other
projects such as barebox or other kernels (don't know the BSD guys). So
you can't ensure that your change will break something. Please keep that
in mind.

IMHO solving that issue should be done by the scu firmware. I tought the
scu is a cortex-m4 with a bunch of embedded flash and ram (I don't know
that much about the scu since it is closed/black-boxed). Why do you
don't use a translation table within the scu? As I said earlier I don't
like the redefinition of ID's since they are now part of the dt-bindings.
The bindings can store up to 32bit values which is a large number ;)
IMHO wasting some ID's in favour of stability is a better solution.

Regards,
Marco

> > 
> 
> Please double check with SCU firmware owner what the removed ID are used before?
> Any side effect if removing them.
> 
> And please also check if the combability can be maintained via IMX_SC_RPC_VERSION?
> 
> Regards
> Dong Aisheng
> 
> > Anson.
> > 
> > >
> > > Regards,
> > > Marco
> > >
> > > >
> > > > Anson.
> > > >
> > > > >
> > > > > Regards,
> > > > > Marco
> > > > >
> > > > > > Signed-off-by: Anson Huang <[e-mail]>
> > > > > > ---
> > > > > >  include/dt-bindings/firmware/imx/rsrc.h | 39
> > > > > > +++++++++++++++++++--------------
> > > > > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > > > > >
> > > > > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > > > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > > > > index 4481f2d..ad747a8 100644
> > > > > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > > > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > > > > @@ -36,15 +36,15 @@
> > > > > >  #define IMX_SC_R_DC_0_BLIT1		20
> > > > > >  #define IMX_SC_R_DC_0_BLIT2		21
> > > > > >  #define IMX_SC_R_DC_0_BLIT_OUT		22
> > > > > > -#define IMX_SC_R_DC_0_CAPTURE0		23
> > > > > > -#define IMX_SC_R_DC_0_CAPTURE1		24
> > > > > > +#define IMX_SC_R_PERF			23
> > > > > > +#define IMX_SC_R_UNUSED5		24
> > > > > >  #define IMX_SC_R_DC_0_WARP		25
> > > > > > -#define IMX_SC_R_DC_0_INTEGRAL0		26
> > > > > > -#define IMX_SC_R_DC_0_INTEGRAL1		27
> > > > > > +#define IMX_SC_R_UNUSED7		26
> > > > > > +#define IMX_SC_R_UNUSED8		27
> > > > > >  #define IMX_SC_R_DC_0_VIDEO0		28
> > > > > >  #define IMX_SC_R_DC_0_VIDEO1		29
> > > > > >  #define IMX_SC_R_DC_0_FRAC0		30
> > > > > > -#define IMX_SC_R_DC_0_FRAC1		31
> > > > > > +#define IMX_SC_R_UNUSED6		31
> > > > > >  #define IMX_SC_R_DC_0			32
> > > > > >  #define IMX_SC_R_GPU_2_PID0		33
> > > > > >  #define IMX_SC_R_DC_0_PLL_0		34
> > > > > > @@ -53,17 +53,17 @@
> > > > > >  #define IMX_SC_R_DC_1_BLIT1		37
> > > > > >  #define IMX_SC_R_DC_1_BLIT2		38
> > > > > >  #define IMX_SC_R_DC_1_BLIT_OUT		39
> > > > > > -#define IMX_SC_R_DC_1_CAPTURE0		40
> > > > > > -#define IMX_SC_R_DC_1_CAPTURE1		41
> > > > > > +#define IMX_SC_R_UNUSED9		40
> > > > > > +#define IMX_SC_R_UNUSED10		41
> > > > > >  #define IMX_SC_R_DC_1_WARP		42
> > > > > > -#define IMX_SC_R_DC_1_INTEGRAL0		43
> > > > > > -#define IMX_SC_R_DC_1_INTEGRAL1		44
> > > > > > +#define IMX_SC_R_UNUSED11		43
> > > > > > +#define IMX_SC_R_UNUSED12		44
> > > > > >  #define IMX_SC_R_DC_1_VIDEO0		45
> > > > > >  #define IMX_SC_R_DC_1_VIDEO1		46
> > > > > >  #define IMX_SC_R_DC_1_FRAC0		47
> > > > > > -#define IMX_SC_R_DC_1_FRAC1		48
> > > > > > +#define IMX_SC_R_UNUSED13		48
> > > > > >  #define IMX_SC_R_DC_1			49
> > > > > > -#define IMX_SC_R_GPU_3_PID0		50
> > > > > > +#define IMX_SC_R_UNUSED14		50
> > > > > >  #define IMX_SC_R_DC_1_PLL_0		51
> > > > > >  #define IMX_SC_R_DC_1_PLL_1		52
> > > > > >  #define IMX_SC_R_SPI_0			53
> > > > > > @@ -303,8 +303,8 @@
> > > > > >  #define IMX_SC_R_M4_0_UART		287
> > > > > >  #define IMX_SC_R_M4_0_I2C		288
> > > > > >  #define IMX_SC_R_M4_0_INTMUX		289
> > > > > > -#define IMX_SC_R_M4_0_SIM		290
> > > > > > -#define IMX_SC_R_M4_0_WDOG		291
> > > > > > +#define IMX_SC_R_UNUSED15		290
> > > > > > +#define IMX_SC_R_UNUSED16		291
> > > > > >  #define IMX_SC_R_M4_0_MU_0B		292
> > > > > >  #define IMX_SC_R_M4_0_MU_0A0		293
> > > > > >  #define IMX_SC_R_M4_0_MU_0A1		294
> > > > > > @@ -323,8 +323,8 @@
> > > > > >  #define IMX_SC_R_M4_1_UART		307
> > > > > >  #define IMX_SC_R_M4_1_I2C		308
> > > > > >  #define IMX_SC_R_M4_1_INTMUX		309
> > > > > > -#define IMX_SC_R_M4_1_SIM		310
> > > > > > -#define IMX_SC_R_M4_1_WDOG		311
> > > > > > +#define IMX_SC_R_UNUSED17		310
> > > > > > +#define IMX_SC_R_UNUSED18		311
> > > > > >  #define IMX_SC_R_M4_1_MU_0B		312
> > > > > >  #define IMX_SC_R_M4_1_MU_0A0		313
> > > > > >  #define IMX_SC_R_M4_1_MU_0A1		314
> > > > > > @@ -337,7 +337,7 @@
> > > > > >  #define IMX_SC_R_IRQSTR_SCU2		321
> > > > > >  #define IMX_SC_R_IRQSTR_DSP		322
> > > > > >  #define IMX_SC_R_ELCDIF_PLL		323
> > > > > > -#define IMX_SC_R_UNUSED6		324
> > > > > > +#define IMX_SC_R_OCRAM			324
> > > > > >  #define IMX_SC_R_AUDIO_PLL_0		325
> > > > > >  #define IMX_SC_R_PI_0			326
> > > > > >  #define IMX_SC_R_PI_0_PWM_0		327
> > > > > > @@ -554,6 +554,11 @@
> > > > > >  #define IMX_SC_R_VPU_MU_3		538
> > > > > >  #define IMX_SC_R_VPU_ENC_1		539
> > > > > >  #define IMX_SC_R_VPU			540
> > > > > > -#define IMX_SC_R_LAST			541
> > > > > > +#define IMX_SC_R_DMA_5_CH0		541
> > > > > > +#define IMX_SC_R_DMA_5_CH1		542
> > > > > > +#define IMX_SC_R_DMA_5_CH2		543
> > > > > > +#define IMX_SC_R_DMA_5_CH3		544
> > > > > > +#define IMX_SC_R_ATTESTATION		545
> > > > > > +#define IMX_SC_R_LAST			546
> > > > > >
> > > > > >  #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> > > > > > --
> > > > > > 2.7.4
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > Pengutronix e.K.                           |
> > |
> > > > > Industrial Linux Solutions                 |
> > > > >
> > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > > > >
> > > w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> > > > >
> > >
> > Cfe709ac17a164d82c7d508d6966915fb%7C686ea1d3bc2b4c6fa92cd99c5c30
> > 1
> > > > >
> > >
> > 635%7C0%7C0%7C636861775412076730&amp;sdata=05ZzPf2%2BQF10JXLLs
> > > > > OPqDdqTi00BWXNHxmMOsQ1z0yI%3D&amp;reserved=0  | Peiner Str.
> > 6-8,
> > > > > 31137 Hildesheim, Germany | Phone: [phone]
> > > |
> > > > > Amtsgericht Hildesheim, HRA 2686           | Fax:
> > [phone]-5555 |
> > >
> > > --
> > > Pengutronix e.K.                           |
> > |
> > > Industrial Linux Solutions                 |
> > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> > > w.pengutronix.de%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7
> > >
> > Cd5d39730435e4b06549c08d696794904%7C686ea1d3bc2b4c6fa92cd99c5c
> > 30
> > >
> > 1635%7C0%7C0%7C636861845015130502&amp;sdata=tQYrNl5lzIRRNVBCji6
> > A
> > > sPREOnIfDgdPWgAnsWyCErg%3D&amp;reserved=0  |
> > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]
> > |
> > > Amtsgericht Hildesheim, HRA 2686           | Fax:
> > [phone]-5555 |
>

Dong Aisheng Feb. 20, 2019, 9:49 a.m. UTC | #11

> From: Marco Felsch [mailto:[e-mail]]
> Sent: Wednesday, February 20, 2019 4:17 PM
> On 19-02-20 03:38, Aisheng Dong wrote:
> > [...]
> >
> > > > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0) by
> > > > mark them as unused or even worse give them a other meaning. IMHO
> > > > the scu-api should be stable since day 1 and the ID's should only be
> extended.
> > > > Marking ID's as deprecated is much better than moving them around.
> > >
> > > I agree the SCU APIs should be stable since day 1 and the ID should
> > > ONLY be extended, but that is the best cases, the reality is, there
> > > are different SoCs/Revision, some revisions may remove the resources
> > > ID defined in pre-coded SCU firmware, like the
> > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them after real
> > > silicon arrived, now latest SCU firmware marks them as UNUSED, they
> > > could be replaced by some other new resources in later new SoC, I am
> > > NOT sure, but if it happens, this resource ID table should be
> > > updated anyway, leaving the out-of-date resource ID table there will have
> issues, since it is NOT sync with SCU firmware.
> > >
> > > So how to resolve such issue? We hope the SCU firmware should be
> > > stable since day 1, but the truth is NOT, could be still some
> > > updates but NOT very often. And I believe the updates will NOT break old
> kernel version.
> 
> Hi Anson,
> 
> Please don't mix the dt-bindings and the kernel related stuff.
> Unfortunately the bindings are within the kernel repo which in fact is great for
> us kernel developer but the bindings are also used in other projects such as
> barebox or other kernels (don't know the BSD guys). So you can't ensure that
> your change will break something. Please keep that in mind.
> 
> IMHO solving that issue should be done by the scu firmware. I tought the scu is
> a cortex-m4 with a bunch of embedded flash and ram (I don't know that much
> about the scu since it is closed/black-boxed). Why do you don't use a
> translation table within the scu? As I said earlier I don't like the redefinition of
> ID's since they are now part of the dt-bindings.
> The bindings can store up to 32bit values which is a large number ;) IMHO
> wasting some ID's in favour of stability is a better solution.
> 

As far as I know, those remove resource IDs are pre-defined and has never been used
and won't be used anymore by SC firmware. (Anson can double check it)
So I think it's safe to remove them or mark them as deprecated.

Personally I may prefer to remove them as they never worked to avoid confusing,
especially at this early stage for mx8.

Regards
Dong Aisheng

Marco Felsch Feb. 20, 2019, 10:52 a.m. UTC | #12

Hi Aisheng,

On 19-02-20 09:49, Aisheng Dong wrote:
> > From: Marco Felsch [mailto:[e-mail]]
> > Sent: Wednesday, February 20, 2019 4:17 PM
> > On 19-02-20 03:38, Aisheng Dong wrote:
> > > [...]
> > >
> > > > > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0) by
> > > > > mark them as unused or even worse give them a other meaning. IMHO
> > > > > the scu-api should be stable since day 1 and the ID's should only be
> > extended.
> > > > > Marking ID's as deprecated is much better than moving them around.
> > > >
> > > > I agree the SCU APIs should be stable since day 1 and the ID should
> > > > ONLY be extended, but that is the best cases, the reality is, there
> > > > are different SoCs/Revision, some revisions may remove the resources
> > > > ID defined in pre-coded SCU firmware, like the
> > > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them after real
> > > > silicon arrived, now latest SCU firmware marks them as UNUSED, they
> > > > could be replaced by some other new resources in later new SoC, I am
> > > > NOT sure, but if it happens, this resource ID table should be
> > > > updated anyway, leaving the out-of-date resource ID table there will have
> > issues, since it is NOT sync with SCU firmware.
> > > >
> > > > So how to resolve such issue? We hope the SCU firmware should be
> > > > stable since day 1, but the truth is NOT, could be still some
> > > > updates but NOT very often. And I believe the updates will NOT break old
> > kernel version.
> > 
> > Hi Anson,
> > 
> > Please don't mix the dt-bindings and the kernel related stuff.
> > Unfortunately the bindings are within the kernel repo which in fact is great for
> > us kernel developer but the bindings are also used in other projects such as
> > barebox or other kernels (don't know the BSD guys). So you can't ensure that
> > your change will break something. Please keep that in mind.
> > 
> > IMHO solving that issue should be done by the scu firmware. I tought the scu is
> > a cortex-m4 with a bunch of embedded flash and ram (I don't know that much
> > about the scu since it is closed/black-boxed). Why do you don't use a
> > translation table within the scu? As I said earlier I don't like the redefinition of
> > ID's since they are now part of the dt-bindings.
> > The bindings can store up to 32bit values which is a large number ;) IMHO
> > wasting some ID's in favour of stability is a better solution.
> > 
> 
> As far as I know, those remove resource IDs are pre-defined and has never been used
> and won't be used anymore by SC firmware. (Anson can double check it)
> So I think it's safe to remove them or mark them as deprecated.

I think marking them as deprecated by a commentar is better than
redefining bits to be unused. As I said the bindings not only linux
related they are used in other projects too.

> 
> Personally I may prefer to remove them as they never worked to avoid confusing,
> especially at this early stage for mx8.

So why they are there if they never worked? Wouldn't it a better
approach to start with a basic and working ID file and extend this
rather than adding id's because maybe the will work.. Sorry but this
seems wrong to me too.
I know the approach from driver development, adding a driver specific
*_reg.h file and later figuring out that those bits won't work as
aspected. As I said this will be driver and only linux related so we can
change them as many times as we want. But in your case we are talking
about dt-bindings and this approach won't work.

Regards,
Marco

> 
> Regards
> Dong Aisheng
>

Dong Aisheng Feb. 20, 2019, 11:21 a.m. UTC | #13

Hi Marco,

> From: Marco Felsch [mailto:[e-mail]]
> Sent: Wednesday, February 20, 2019 6:53 PM
> 
> Hi Aisheng,
> 
> On 19-02-20 09:49, Aisheng Dong wrote:
> > > From: Marco Felsch [mailto:[e-mail]]
> > > Sent: Wednesday, February 20, 2019 4:17 PM On 19-02-20 03:38,
> > > Aisheng Dong wrote:
> > > > [...]
> > > >
> > > > > > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0)
> > > > > > by mark them as unused or even worse give them a other
> > > > > > meaning. IMHO the scu-api should be stable since day 1 and the
> > > > > > ID's should only be
> > > extended.
> > > > > > Marking ID's as deprecated is much better than moving them around.
> > > > >
> > > > > I agree the SCU APIs should be stable since day 1 and the ID
> > > > > should ONLY be extended, but that is the best cases, the reality
> > > > > is, there are different SoCs/Revision, some revisions may remove
> > > > > the resources ID defined in pre-coded SCU firmware, like the
> > > > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them after real
> > > > > silicon arrived, now latest SCU firmware marks them as UNUSED,
> > > > > they could be replaced by some other new resources in later new
> > > > > SoC, I am NOT sure, but if it happens, this resource ID table
> > > > > should be updated anyway, leaving the out-of-date resource ID
> > > > > table there will have
> > > issues, since it is NOT sync with SCU firmware.
> > > > >
> > > > > So how to resolve such issue? We hope the SCU firmware should be
> > > > > stable since day 1, but the truth is NOT, could be still some
> > > > > updates but NOT very often. And I believe the updates will NOT
> > > > > break old
> > > kernel version.
> > >
> > > Hi Anson,
> > >
> > > Please don't mix the dt-bindings and the kernel related stuff.
> > > Unfortunately the bindings are within the kernel repo which in fact
> > > is great for us kernel developer but the bindings are also used in
> > > other projects such as barebox or other kernels (don't know the BSD
> > > guys). So you can't ensure that your change will break something. Please
> keep that in mind.
> > >
> > > IMHO solving that issue should be done by the scu firmware. I tought
> > > the scu is a cortex-m4 with a bunch of embedded flash and ram (I
> > > don't know that much about the scu since it is closed/black-boxed).
> > > Why do you don't use a translation table within the scu? As I said
> > > earlier I don't like the redefinition of ID's since they are now part of the
> dt-bindings.
> > > The bindings can store up to 32bit values which is a large number ;)
> > > IMHO wasting some ID's in favour of stability is a better solution.
> > >
> >
> > As far as I know, those remove resource IDs are pre-defined and has
> > never been used and won't be used anymore by SC firmware. (Anson can
> > double check it) So I think it's safe to remove them or mark them as
> deprecated.
> 
> I think marking them as deprecated by a commentar is better than redefining
> bits to be unused. As I said the bindings not only linux related they are used in
> other projects too.
> 

For other projects, the result is same because SC firmware does not support those
Resource IDs.

> >
> > Personally I may prefer to remove them as they never worked to avoid
> > confusing, especially at this early stage for mx8.
> 
> So why they are there if they never worked? Wouldn't it a better approach to
> start with a basic and working ID file and extend this rather than adding id's
> because maybe the will work.. 

Ideally yes, but may be hard in reality.

SC firmware code usually is developed quite early before silicon comes.
But the real chip may changes, e.g. removed or added something.

AFAIK those resource IDs removed have never worked in SC firmware,
and they're likely to come out of pre-silicon definition. But chip changes
later.

> Sorry but this seems wrong to me too.
> I know the approach from driver development, adding a driver specific *_reg.h
> file and later figuring out that those bits won't work as aspected. As I said this
> will be driver and only linux related so we can change them as many times as
> we want. But in your case we are talking about dt-bindings and this approach
> won't work.

I would agree with you if those resource IDs have worked before.
But they never worked.
Should we keep a thing never worked in device tree just because
It makes us look like keeping compatibility?

If that, I'd rather removing them to avoid confusing in the future. Life is long, right?
I don't want people to bother with those unmeaning things anymore when they
look at it.

Regards
Dong Aisheng

> 
> Regards,
> Marco
> 
> >
> > Regards
> > Dong Aisheng
> >
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&data=02%7C01%7Caisheng.dong%40nxp.com%7C7e
> 39985c9866476290b608d697219367%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C636862567803869154&sdata=2kFu%2BC7Eh%2BYYrT
> azzrKSHalHfZpdoiEw4RrxcSAMCQg%3D&reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: [phone]
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> [phone]-5555 |

Lucas Stach Feb. 20, 2019, 12:11 p.m. UTC | #14

Am Mittwoch, den 20.02.2019, 11:21 +0000 schrieb Aisheng Dong:
> Hi Marco,
> 
> > From: Marco Felsch [mailto:[e-mail]]
> > Sent: Wednesday, February 20, 2019 6:53 PM
> > 
> > Hi Aisheng,
> > 
> > On 19-02-20 09:49, Aisheng Dong wrote:
> > > > From: Marco Felsch [mailto:[e-mail]]
> > > > Sent: Wednesday, February 20, 2019 4:17 PM On 19-02-20 03:38,
> > > > Aisheng Dong wrote:
> > > > > [...]
> > > > > 
> > > > > > > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0)
> > > > > > > by mark them as unused or even worse give them a other
> > > > > > > meaning. IMHO the scu-api should be stable since day 1 and the
> > > > > > > ID's should only be
> > > > extended.
> > > > > > > Marking ID's as deprecated is much better than moving them around.
> > > > > > 
> > > > > > I agree the SCU APIs should be stable since day 1 and the ID
> > > > > > should ONLY be extended, but that is the best cases, the reality
> > > > > > is, there are different SoCs/Revision, some revisions may remove
> > > > > > the resources ID defined in pre-coded SCU firmware, like the
> > > > > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them after real
> > > > > > silicon arrived, now latest SCU firmware marks them as UNUSED,
> > > > > > they could be replaced by some other new resources in later new
> > > > > > SoC, I am NOT sure, but if it happens, this resource ID table
> > > > > > should be updated anyway, leaving the out-of-date resource ID
> > > > > > table there will have
> > > > issues, since it is NOT sync with SCU firmware.
> > > > > > So how to resolve such issue? We hope the SCU firmware should be
> > > > > > stable since day 1, but the truth is NOT, could be still some
> > > > > > updates but NOT very often. And I believe the updates will NOT
> > > > > > break old
> > > > kernel version.
> > > > 
> > > > Hi Anson,
> > > > 
> > > > Please don't mix the dt-bindings and the kernel related stuff.
> > > > Unfortunately the bindings are within the kernel repo which in fact
> > > > is great for us kernel developer but the bindings are also used in
> > > > other projects such as barebox or other kernels (don't know the BSD
> > > > guys). So you can't ensure that your change will break something. Please
> > keep that in mind.
> > > > IMHO solving that issue should be done by the scu firmware. I tought
> > > > the scu is a cortex-m4 with a bunch of embedded flash and ram (I
> > > > don't know that much about the scu since it is closed/black-boxed).
> > > > Why do you don't use a translation table within the scu? As I said
> > > > earlier I don't like the redefinition of ID's since they are now part of the
> > dt-bindings.
> > > > The bindings can store up to 32bit values which is a large number ;)
> > > > IMHO wasting some ID's in favour of stability is a better solution.
> > > > 
> > > 
> > > As far as I know, those remove resource IDs are pre-defined and has
> > > never been used and won't be used anymore by SC firmware. (Anson can
> > > double check it) So I think it's safe to remove them or mark them as
> > deprecated.
> > 
> > I think marking them as deprecated by a commentar is better than redefining
> > bits to be unused. As I said the bindings not only linux related they are used in
> > other projects too.
> > 
> 
> For other projects, the result is same because SC firmware does not support those
> Resource IDs.
> 
> > > Personally I may prefer to remove them as they never worked to avoid
> > > confusing, especially at this early stage for mx8.
> > 
> > So why they are there if they never worked? Wouldn't it a better approach to
> > start with a basic and working ID file and extend this rather than adding id's
> > because maybe the will work.. 
> 
> Ideally yes, but may be hard in reality.
> 
> SC firmware code usually is developed quite early before silicon comes.
> But the real chip may changes, e.g. removed or added something.
> 
> AFAIK those resource IDs removed have never worked in SC firmware,
> and they're likely to come out of pre-silicon definition. But chip changes
> later.
> 
> > Sorry but this seems wrong to me too.
> > I know the approach from driver development, adding a driver specific *_reg.h
> > file and later figuring out that those bits won't work as aspected. As I said this
> > will be driver and only linux related so we can change them as many times as
> > we want. But in your case we are talking about dt-bindings and this approach
> > won't work.
> 
> I would agree with you if those resource IDs have worked before.
> But they never worked.
> Should we keep a thing never worked in device tree just because
> It makes us look like keeping compatibility?
> 
> If that, I'd rather removing them to avoid confusing in the future. Life is long, right?
> I don't want people to bother with those unmeaning things anymore when they
> look at it.

Removing things is totally fine if they have never worked. Re-using the
now "free" IDs is what is problematic. Only ever use previously unused
IDs when introducing new stuff into the SCU firmware. This is something
you need to communicate quite clearly with the SCU firmware developers.

Regards,
Lucas

Dong Aisheng Feb. 20, 2019, 1:44 p.m. UTC | #15

> From: Lucas Stach [mailto:[e-mail]]
> Sent: Wednesday, February 20, 2019 8:11 PM
> 
> Am Mittwoch, den 20.02.2019, 11:21 +0000 schrieb Aisheng Dong:
> > Hi Marco,
> >
> > > From: Marco Felsch [mailto:[e-mail]]
> > > Sent: Wednesday, February 20, 2019 6:53 PM
> > >
> > > Hi Aisheng,
> > >
> > > On 19-02-20 09:49, Aisheng Dong wrote:
> > > > > From: Marco Felsch [mailto:[e-mail]]
> > > > > Sent: Wednesday, February 20, 2019 4:17 PM On 19-02-20 03:38,
> > > > > Aisheng Dong wrote:
> > > > > > [...]
> > > > > >
> > > > > > > > I don't like droping some ID's (e.g.
> > > > > > > > IMX_SC_R_DC_0_CAPTURE0) by mark them as unused or even
> > > > > > > > worse give them a other meaning. IMHO the scu-api should
> > > > > > > > be stable since day 1 and the ID's should only be
> > > > > extended.
> > > > > > > > Marking ID's as deprecated is much better than moving them
> around.
> > > > > > >
> > > > > > > I agree the SCU APIs should be stable since day 1 and the ID
> > > > > > > should ONLY be extended, but that is the best cases, the
> > > > > > > reality is, there are different SoCs/Revision, some
> > > > > > > revisions may remove the resources ID defined in pre-coded
> > > > > > > SCU firmware, like the
> > > > > > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them after
> > > > > > > real silicon arrived, now latest SCU firmware marks them as
> > > > > > > UNUSED, they could be replaced by some other new resources
> > > > > > > in later new SoC, I am NOT sure, but if it happens, this
> > > > > > > resource ID table should be updated anyway, leaving the
> > > > > > > out-of-date resource ID table there will have
> > > > > issues, since it is NOT sync with SCU firmware.
> > > > > > > So how to resolve such issue? We hope the SCU firmware
> > > > > > > should be stable since day 1, but the truth is NOT, could be
> > > > > > > still some updates but NOT very often. And I believe the
> > > > > > > updates will NOT break old
> > > > > kernel version.
> > > > >
> > > > > Hi Anson,
> > > > >
> > > > > Please don't mix the dt-bindings and the kernel related stuff.
> > > > > Unfortunately the bindings are within the kernel repo which in
> > > > > fact is great for us kernel developer but the bindings are also
> > > > > used in other projects such as barebox or other kernels (don't
> > > > > know the BSD guys). So you can't ensure that your change will
> > > > > break something. Please
> > > keep that in mind.
> > > > > IMHO solving that issue should be done by the scu firmware. I
> > > > > tought the scu is a cortex-m4 with a bunch of embedded flash and
> > > > > ram (I don't know that much about the scu since it is
> closed/black-boxed).
> > > > > Why do you don't use a translation table within the scu? As I
> > > > > said earlier I don't like the redefinition of ID's since they
> > > > > are now part of the
> > > dt-bindings.
> > > > > The bindings can store up to 32bit values which is a large
> > > > > number ;) IMHO wasting some ID's in favour of stability is a better
> solution.
> > > > >
> > > >
> > > > As far as I know, those remove resource IDs are pre-defined and
> > > > has never been used and won't be used anymore by SC firmware.
> > > > (Anson can double check it) So I think it's safe to remove them or
> > > > mark them as
> > > deprecated.
> > >
> > > I think marking them as deprecated by a commentar is better than
> > > redefining bits to be unused. As I said the bindings not only linux
> > > related they are used in other projects too.
> > >
> >
> > For other projects, the result is same because SC firmware does not
> > support those Resource IDs.
> >
> > > > Personally I may prefer to remove them as they never worked to
> > > > avoid confusing, especially at this early stage for mx8.
> > >
> > > So why they are there if they never worked? Wouldn't it a better
> > > approach to start with a basic and working ID file and extend this
> > > rather than adding id's because maybe the will work..
> >
> > Ideally yes, but may be hard in reality.
> >
> > SC firmware code usually is developed quite early before silicon comes.
> > But the real chip may changes, e.g. removed or added something.
> >
> > AFAIK those resource IDs removed have never worked in SC firmware, and
> > they're likely to come out of pre-silicon definition. But chip changes
> > later.
> >
> > > Sorry but this seems wrong to me too.
> > > I know the approach from driver development, adding a driver
> > > specific *_reg.h file and later figuring out that those bits won't
> > > work as aspected. As I said this will be driver and only linux
> > > related so we can change them as many times as we want. But in your
> > > case we are talking about dt-bindings and this approach won't work.
> >
> > I would agree with you if those resource IDs have worked before.
> > But they never worked.
> > Should we keep a thing never worked in device tree just because It
> > makes us look like keeping compatibility?
> >
> > If that, I'd rather removing them to avoid confusing in the future. Life is long,
> right?
> > I don't want people to bother with those unmeaning things anymore when
> > they look at it.
> 
> Removing things is totally fine if they have never worked. Re-using the now
> "free" IDs is what is problematic. Only ever use previously unused IDs when
> introducing new stuff into the SCU firmware. This is something you need to
> communicate quite clearly with the SCU firmware developers.
> 

Yes, that's right.
I will forward this request to them.
Thanks for the info.

Regards
Dong Aisheng

> Regards,
> Lucas

Marco Felsch Feb. 20, 2019, 1:52 p.m. UTC | #16

On 19-02-20 13:11, Lucas Stach wrote:
> Am Mittwoch, den 20.02.2019, 11:21 +0000 schrieb Aisheng Dong:
> > Hi Marco,
> > 
> > > From: Marco Felsch [mailto:[e-mail]]
> > > Sent: Wednesday, February 20, 2019 6:53 PM
> > > 
> > > Hi Aisheng,
> > > 
> > > On 19-02-20 09:49, Aisheng Dong wrote:
> > > > > From: Marco Felsch [mailto:[e-mail]]
> > > > > Sent: Wednesday, February 20, 2019 4:17 PM On 19-02-20 03:38,
> > > > > Aisheng Dong wrote:
> > > > > > [...]
> > > > > > 
> > > > > > > > I don't like droping some ID's (e.g. IMX_SC_R_DC_0_CAPTURE0)
> > > > > > > > by mark them as unused or even worse give them a other
> > > > > > > > meaning. IMHO the scu-api should be stable since day 1 and the
> > > > > > > > ID's should only be
> > > > > extended.
> > > > > > > > Marking ID's as deprecated is much better than moving them around.
> > > > > > > 
> > > > > > > I agree the SCU APIs should be stable since day 1 and the ID
> > > > > > > should ONLY be extended, but that is the best cases, the reality
> > > > > > > is, there are different SoCs/Revision, some revisions may remove
> > > > > > > the resources ID defined in pre-coded SCU firmware, like the
> > > > > > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them after real
> > > > > > > silicon arrived, now latest SCU firmware marks them as UNUSED,
> > > > > > > they could be replaced by some other new resources in later new
> > > > > > > SoC, I am NOT sure, but if it happens, this resource ID table
> > > > > > > should be updated anyway, leaving the out-of-date resource ID
> > > > > > > table there will have
> > > > > issues, since it is NOT sync with SCU firmware.
> > > > > > > So how to resolve such issue? We hope the SCU firmware should be
> > > > > > > stable since day 1, but the truth is NOT, could be still some
> > > > > > > updates but NOT very often. And I believe the updates will NOT
> > > > > > > break old
> > > > > kernel version.
> > > > > 
> > > > > Hi Anson,
> > > > > 
> > > > > Please don't mix the dt-bindings and the kernel related stuff.
> > > > > Unfortunately the bindings are within the kernel repo which in fact
> > > > > is great for us kernel developer but the bindings are also used in
> > > > > other projects such as barebox or other kernels (don't know the BSD
> > > > > guys). So you can't ensure that your change will break something. Please
> > > keep that in mind.
> > > > > IMHO solving that issue should be done by the scu firmware. I tought
> > > > > the scu is a cortex-m4 with a bunch of embedded flash and ram (I
> > > > > don't know that much about the scu since it is closed/black-boxed).
> > > > > Why do you don't use a translation table within the scu? As I said
> > > > > earlier I don't like the redefinition of ID's since they are now part of the
> > > dt-bindings.
> > > > > The bindings can store up to 32bit values which is a large number ;)
> > > > > IMHO wasting some ID's in favour of stability is a better solution.
> > > > > 
> > > > 
> > > > As far as I know, those remove resource IDs are pre-defined and has
> > > > never been used and won't be used anymore by SC firmware. (Anson can
> > > > double check it) So I think it's safe to remove them or mark them as
> > > deprecated.
> > > 
> > > I think marking them as deprecated by a commentar is better than redefining
> > > bits to be unused. As I said the bindings not only linux related they are used in
> > > other projects too.
> > > 
> > 
> > For other projects, the result is same because SC firmware does not support those
> > Resource IDs.
> > 
> > > > Personally I may prefer to remove them as they never worked to avoid
> > > > confusing, especially at this early stage for mx8.
> > > 
> > > So why they are there if they never worked? Wouldn't it a better approach to
> > > start with a basic and working ID file and extend this rather than adding id's
> > > because maybe the will work.. 
> > 
> > Ideally yes, but may be hard in reality.
> > 
> > SC firmware code usually is developed quite early before silicon comes.
> > But the real chip may changes, e.g. removed or added something.
> > 
> > AFAIK those resource IDs removed have never worked in SC firmware,
> > and they're likely to come out of pre-silicon definition. But chip changes
> > later.
> > 
> > > Sorry but this seems wrong to me too.
> > > I know the approach from driver development, adding a driver specific *_reg.h
> > > file and later figuring out that those bits won't work as aspected. As I said this
> > > will be driver and only linux related so we can change them as many times as
> > > we want. But in your case we are talking about dt-bindings and this approach
> > > won't work.
> > 
> > I would agree with you if those resource IDs have worked before.
> > But they never worked.
> > Should we keep a thing never worked in device tree just because
> > It makes us look like keeping compatibility?
> > 
> > If that, I'd rather removing them to avoid confusing in the future. Life is long, right?
> > I don't want people to bother with those unmeaning things anymore when they
> > look at it.
> 
> Removing things is totally fine if they have never worked. Re-using the
> now "free" IDs is what is problematic. Only ever use previously unused
> IDs when introducing new stuff into the SCU firmware. This is something
> you need to communicate quite clearly with the SCU firmware developers.

I'm fine with removing too then there would be a gap. Don't know if it
is better to have a gap or a comment that those ID isn't supported since
fw-revision XYZ / silicon version ZYX.

Just some toughts:
The gap will then suggest that this ID is free and was never assigned to
some functionality. In real it was assigned previously and now gets
redefined. Also as far as I can see there are no gaps currently. Let's
wait for Rob's feedback.

As Lucas mentioned using IMX_SC_R_UNUSED* ID's for new functionality is
totaly fine but redefine ID's seems wrong to me. 

Regards,
Marco

> 
> Regards,
> Lucas
> 
>

Dong Aisheng Feb. 20, 2019, 2:09 p.m. UTC | #17

> From: Marco Felsch [mailto:[e-mail]]
> Sent: Wednesday, February 20, 2019 9:53 PM
> 
> On 19-02-20 13:11, Lucas Stach wrote:
> > Am Mittwoch, den 20.02.2019, 11:21 +0000 schrieb Aisheng Dong:
> > > Hi Marco,
> > >
> > > > From: Marco Felsch [mailto:[e-mail]]
> > > > Sent: Wednesday, February 20, 2019 6:53 PM
> > > >
> > > > Hi Aisheng,
> > > >
> > > > On 19-02-20 09:49, Aisheng Dong wrote:
> > > > > > From: Marco Felsch [mailto:[e-mail]]
> > > > > > Sent: Wednesday, February 20, 2019 4:17 PM On 19-02-20 03:38,
> > > > > > Aisheng Dong wrote:
> > > > > > > [...]
> > > > > > >
> > > > > > > > > I don't like droping some ID's (e.g.
> > > > > > > > > IMX_SC_R_DC_0_CAPTURE0) by mark them as unused or even
> > > > > > > > > worse give them a other meaning. IMHO the scu-api should
> > > > > > > > > be stable since day 1 and the ID's should only be
> > > > > > extended.
> > > > > > > > > Marking ID's as deprecated is much better than moving them
> around.
> > > > > > > >
> > > > > > > > I agree the SCU APIs should be stable since day 1 and the
> > > > > > > > ID should ONLY be extended, but that is the best cases,
> > > > > > > > the reality is, there are different SoCs/Revision, some
> > > > > > > > revisions may remove the resources ID defined in pre-coded
> > > > > > > > SCU firmware, like the
> > > > > > > > IMX_SC_R_DC_0_CAPTURE0 etc., so SCU APIs removes them
> > > > > > > > after real silicon arrived, now latest SCU firmware marks
> > > > > > > > them as UNUSED, they could be replaced by some other new
> > > > > > > > resources in later new SoC, I am NOT sure, but if it
> > > > > > > > happens, this resource ID table should be updated anyway,
> > > > > > > > leaving the out-of-date resource ID table there will have
> > > > > > issues, since it is NOT sync with SCU firmware.
> > > > > > > > So how to resolve such issue? We hope the SCU firmware
> > > > > > > > should be stable since day 1, but the truth is NOT, could
> > > > > > > > be still some updates but NOT very often. And I believe
> > > > > > > > the updates will NOT break old
> > > > > > kernel version.
> > > > > >
> > > > > > Hi Anson,
> > > > > >
> > > > > > Please don't mix the dt-bindings and the kernel related stuff.
> > > > > > Unfortunately the bindings are within the kernel repo which in
> > > > > > fact is great for us kernel developer but the bindings are
> > > > > > also used in other projects such as barebox or other kernels
> > > > > > (don't know the BSD guys). So you can't ensure that your
> > > > > > change will break something. Please
> > > > keep that in mind.
> > > > > > IMHO solving that issue should be done by the scu firmware. I
> > > > > > tought the scu is a cortex-m4 with a bunch of embedded flash
> > > > > > and ram (I don't know that much about the scu since it is
> closed/black-boxed).
> > > > > > Why do you don't use a translation table within the scu? As I
> > > > > > said earlier I don't like the redefinition of ID's since they
> > > > > > are now part of the
> > > > dt-bindings.
> > > > > > The bindings can store up to 32bit values which is a large
> > > > > > number ;) IMHO wasting some ID's in favour of stability is a better
> solution.
> > > > > >
> > > > >
> > > > > As far as I know, those remove resource IDs are pre-defined and
> > > > > has never been used and won't be used anymore by SC firmware.
> > > > > (Anson can double check it) So I think it's safe to remove them
> > > > > or mark them as
> > > > deprecated.
> > > >
> > > > I think marking them as deprecated by a commentar is better than
> > > > redefining bits to be unused. As I said the bindings not only
> > > > linux related they are used in other projects too.
> > > >
> > >
> > > For other projects, the result is same because SC firmware does not
> > > support those Resource IDs.
> > >
> > > > > Personally I may prefer to remove them as they never worked to
> > > > > avoid confusing, especially at this early stage for mx8.
> > > >
> > > > So why they are there if they never worked? Wouldn't it a better
> > > > approach to start with a basic and working ID file and extend this
> > > > rather than adding id's because maybe the will work..
> > >
> > > Ideally yes, but may be hard in reality.
> > >
> > > SC firmware code usually is developed quite early before silicon comes.
> > > But the real chip may changes, e.g. removed or added something.
> > >
> > > AFAIK those resource IDs removed have never worked in SC firmware,
> > > and they're likely to come out of pre-silicon definition. But chip
> > > changes later.
> > >
> > > > Sorry but this seems wrong to me too.
> > > > I know the approach from driver development, adding a driver
> > > > specific *_reg.h file and later figuring out that those bits won't
> > > > work as aspected. As I said this will be driver and only linux
> > > > related so we can change them as many times as we want. But in
> > > > your case we are talking about dt-bindings and this approach won't work.
> > >
> > > I would agree with you if those resource IDs have worked before.
> > > But they never worked.
> > > Should we keep a thing never worked in device tree just because It
> > > makes us look like keeping compatibility?
> > >
> > > If that, I'd rather removing them to avoid confusing in the future. Life is
> long, right?
> > > I don't want people to bother with those unmeaning things anymore
> > > when they look at it.
> >
> > Removing things is totally fine if they have never worked. Re-using
> > the now "free" IDs is what is problematic. Only ever use previously
> > unused IDs when introducing new stuff into the SCU firmware. This is
> > something you need to communicate quite clearly with the SCU firmware
> developers.
> 
> I'm fine with removing too then there would be a gap. Don't know if it is better
> to have a gap or a comment that those ID isn't supported since fw-revision XYZ
> / silicon version ZYX.
> 
> Just some toughts:
> The gap will then suggest that this ID is free and was never assigned to some
> functionality. In real it was assigned previously and now gets redefined. Also as
> far as I can see there are no gaps currently. Let's wait for Rob's feedback.
> 
> As Lucas mentioned using IMX_SC_R_UNUSED* ID's for new functionality is
> totaly fine but redefine ID's seems wrong to me.
> 

Probably we still keep those removed IDs as IMX_SC_R_UNUSED*?

BTW, I guess Lucas's suggestion is:
" Re-using the now "free" IDs is what is problematic, Only ever use previously
unused IDs when introducing new stuff into the SCU firmware".
Because it's true that reused IDs are new stuff into SCU firmware.
So it's okay to change IMX_SC_R_UNUSED into a new ID, right?

Regards
Dong Aisheng

> Regards,
> Marco
> 
> >
> > Regards,
> > Lucas
> >
> >

Patch

diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h
index 4481f2d..ad747a8 100644
--- a/include/dt-bindings/firmware/imx/rsrc.h
+++ b/include/dt-bindings/firmware/imx/rsrc.h
@@ -36,15 +36,15 @@ 
 #define IMX_SC_R_DC_0_BLIT1		20
 #define IMX_SC_R_DC_0_BLIT2		21
 #define IMX_SC_R_DC_0_BLIT_OUT		22
-#define IMX_SC_R_DC_0_CAPTURE0		23
-#define IMX_SC_R_DC_0_CAPTURE1		24
+#define IMX_SC_R_PERF			23
+#define IMX_SC_R_UNUSED5		24
 #define IMX_SC_R_DC_0_WARP		25
-#define IMX_SC_R_DC_0_INTEGRAL0		26
-#define IMX_SC_R_DC_0_INTEGRAL1		27
+#define IMX_SC_R_UNUSED7		26
+#define IMX_SC_R_UNUSED8		27
 #define IMX_SC_R_DC_0_VIDEO0		28
 #define IMX_SC_R_DC_0_VIDEO1		29
 #define IMX_SC_R_DC_0_FRAC0		30
-#define IMX_SC_R_DC_0_FRAC1		31
+#define IMX_SC_R_UNUSED6		31
 #define IMX_SC_R_DC_0			32
 #define IMX_SC_R_GPU_2_PID0		33
 #define IMX_SC_R_DC_0_PLL_0		34
@@ -53,17 +53,17 @@ 
 #define IMX_SC_R_DC_1_BLIT1		37
 #define IMX_SC_R_DC_1_BLIT2		38
 #define IMX_SC_R_DC_1_BLIT_OUT		39
-#define IMX_SC_R_DC_1_CAPTURE0		40
-#define IMX_SC_R_DC_1_CAPTURE1		41
+#define IMX_SC_R_UNUSED9		40
+#define IMX_SC_R_UNUSED10		41
 #define IMX_SC_R_DC_1_WARP		42
-#define IMX_SC_R_DC_1_INTEGRAL0		43
-#define IMX_SC_R_DC_1_INTEGRAL1		44
+#define IMX_SC_R_UNUSED11		43
+#define IMX_SC_R_UNUSED12		44
 #define IMX_SC_R_DC_1_VIDEO0		45
 #define IMX_SC_R_DC_1_VIDEO1		46
 #define IMX_SC_R_DC_1_FRAC0		47
-#define IMX_SC_R_DC_1_FRAC1		48
+#define IMX_SC_R_UNUSED13		48
 #define IMX_SC_R_DC_1			49
-#define IMX_SC_R_GPU_3_PID0		50
+#define IMX_SC_R_UNUSED14		50
 #define IMX_SC_R_DC_1_PLL_0		51
 #define IMX_SC_R_DC_1_PLL_1		52
 #define IMX_SC_R_SPI_0			53
@@ -303,8 +303,8 @@ 
 #define IMX_SC_R_M4_0_UART		287
 #define IMX_SC_R_M4_0_I2C		288
 #define IMX_SC_R_M4_0_INTMUX		289
-#define IMX_SC_R_M4_0_SIM		290
-#define IMX_SC_R_M4_0_WDOG		291
+#define IMX_SC_R_UNUSED15		290
+#define IMX_SC_R_UNUSED16		291
 #define IMX_SC_R_M4_0_MU_0B		292
 #define IMX_SC_R_M4_0_MU_0A0		293
 #define IMX_SC_R_M4_0_MU_0A1		294
@@ -323,8 +323,8 @@ 
 #define IMX_SC_R_M4_1_UART		307
 #define IMX_SC_R_M4_1_I2C		308
 #define IMX_SC_R_M4_1_INTMUX		309
-#define IMX_SC_R_M4_1_SIM		310
-#define IMX_SC_R_M4_1_WDOG		311
+#define IMX_SC_R_UNUSED17		310
+#define IMX_SC_R_UNUSED18		311
 #define IMX_SC_R_M4_1_MU_0B		312
 #define IMX_SC_R_M4_1_MU_0A0		313
 #define IMX_SC_R_M4_1_MU_0A1		314
@@ -337,7 +337,7 @@ 
 #define IMX_SC_R_IRQSTR_SCU2		321
 #define IMX_SC_R_IRQSTR_DSP		322
 #define IMX_SC_R_ELCDIF_PLL		323
-#define IMX_SC_R_UNUSED6		324
+#define IMX_SC_R_OCRAM			324
 #define IMX_SC_R_AUDIO_PLL_0		325
 #define IMX_SC_R_PI_0			326
 #define IMX_SC_R_PI_0_PWM_0		327
@@ -554,6 +554,11 @@ 
 #define IMX_SC_R_VPU_MU_3		538
 #define IMX_SC_R_VPU_ENC_1		539
 #define IMX_SC_R_VPU			540
-#define IMX_SC_R_LAST			541
+#define IMX_SC_R_DMA_5_CH0		541
+#define IMX_SC_R_DMA_5_CH1		542
+#define IMX_SC_R_DMA_5_CH2		543
+#define IMX_SC_R_DMA_5_CH3		544
+#define IMX_SC_R_ATTESTATION		545
+#define IMX_SC_R_LAST			546
 
 #endif /* __DT_BINDINGS_RSCRC_IMX_H */