Re: [PATCH v2 2/5] clk: imx6: Make the LDB_DI0 and LDB_DI1 clocks read-only

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Andrey,

On 27/5/19 09:01, Andrey Smirnov wrote:
> On Tue, May 21, 2019 at 8:56 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> From: Philipp Zabel <p.zabel@pengutronix.de>
>>
>> Due to incorrect placement of the clock gate cell in the ldb_di[x]_clk
>> tree, the glitchy parent mux of ldb_di[x]_clk can cause a glitch to
>> enter the ldb_di_ipu_div divider. If the divider gets locked up, no
>> ldb_di[x]_clk is generated, and the LVDS display will hang when the
>> ipu_di_clk is sourced from ldb_di_clk.
>>
>> To fix the problem, both the new and current parent of the ldb_di_clk
>> should be disabled before the switch. As this can not be guaranteed by
>> the clock framework during runtime, make the ldb_di[x]_sel muxes read-only.
>> A workaround to set the muxes once during boot could be added to the
>> kernel or bootloader.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
>> Signed-off-by: Shawn Guo <shawnguo@kernel.org>
>> [afa: ported from Linux kernel commit 03d576f202]
>> [afa: added exception for i.MX6QP, see kernel commit f4a0a6c309]
>> [afa: added cpu_has_err009219 helper function]
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/clk/imx/clk-imx6.c | 17 +++++++++++++++--
>>  drivers/clk/imx/clk.h      |  8 ++++++++
>>  2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-imx6.c b/drivers/clk/imx/clk-imx6.c
>> index 319485f8521c..01b649ebbd26 100644
>> --- a/drivers/clk/imx/clk-imx6.c
>> +++ b/drivers/clk/imx/clk-imx6.c
>> @@ -64,6 +64,12 @@ static inline int cpu_is_plus(void)
>>         return cpu_is_mx6qp() || cpu_is_mx6dp();
>>  }
>>
>> +/* i.MX6 Quad/Dual/DualLite/Solo are all affected */
>> +static inline int cpu_has_err009219(void) {
> 
> That { should really be on a separate line.
> 
>> +       return cpu_is_mx6d() || cpu_is_mx6q() ||
>> +               cpu_is_mx6dl() || cpu_is_mx6s();
>> +}
> 
> Hmm, imx_init() is executed at "postcore" and this function will be
> called at "core" init-level. All of the cpu_is_mx6*() functions will
> call cpu_is_mx6() which won't work right until imx_init() is called.
> Are you sure this is working as intended? I think cpu_mx6_is_imx6*()
> functions should be used instead. That's what I had to do on my board
> at least. It looks that there's other unrelated code in this file that
> might have this problem as well.

Thanks for spotting that. I will send out a v3 with this fixed.

> 
> Thanks,
> Andrey Smirnov
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox