Hi, Marco

Best Regards!
Anson Huang

> -----Original Message-----
> From: Marco Felsch [mailto:m.fel...@pengutronix.de]
> Sent: 2019年2月19日 22:48
> To: Anson Huang <anson.hu...@nxp.com>
> Cc: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org;
> s.ha...@pengutronix.de; ker...@pengutronix.de; feste...@gmail.com;
> ulf.hans...@linaro.org; Aisheng Dong <aisheng.d...@nxp.com>;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> 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:m.fel...@pengutronix.de]
> > >
> > > 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 <anson.hu...@nxp.com>
> > > > ---
> > > >  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: +49-5121-206917-0
> |
> > > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-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: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Reply via email to