[RFC 0/2] clk: imx7d: Uart clock set-up conflict if clk needs gating on set rate or re-parent

[Adriana Reus]

On Lu, 2017-07-03 at 16:42 +0200, Lucas Stach wrote:
> Am Montag, den 03.07.2017, 17:28 +0300 schrieb Adriana Reus:
> > 
> > Hi all,
> > 
> > I am currently trying to fix an issue occurring when trying to set
> > the
> > CLK_SET_RATE_GATE flag for the uart root clks. This causes the imx
> > serial
> > driver to fail when setting up devicetree specified frequency or
> > parent because
> > the uart clk is already in use by the ccm driver that starts up all
> > uart clocks
> > for earlycon.
> > 
> > Some context:
> > 
> > There are three main types of clock slices on imx7d ccm: "Core
> > clock slice",
> > "Bus clock slice (AXI/AHB)", and "Peripheral clock slice". Vast
> > majority of
> > clock roots fall in the peripheral slice category:
> > 
> >     8:1 mux -> gate -> div -> div -> gate
> > 
> > The root clocks that derive from peripheral slices are expected to
> > be
> > gated when changing frequency [0].
> > 
> > The first patch of this series sets this up. (It is only a
> > reference point for
> > this conversation, not to be merged as it WIP and needs more
> > testing).
> > 
> > When setting this up, the imx serial driver
> > (platform_driver_register -> of_clk_set_defaults) will fail
> > to set the rate or parent specified in the devicetree:
> > 
> >     imx-uart 30a70000.serial: clk_set_rate() failed
> >     imx-uart: probe of 30a70000.serial failed with error -16
> > 
> > The reason for this is that the uart clks are already prepared and
> > enabled in
> > imx7d_clocks_init by the imx_register_uart_clocks function if
> > earlycon is
> > enabled. Functionality added in patches:
> > 'commit 55adc61c568af99419be1dc0412f8eae019c71f2
> > ("clk: imx: add common logic to detect early UART usage")'
> > 'commit 1b9af68f325cb91ac9fc691f52d69dfb0826afd7
> > ("clk: imx7d: retain early UART clocks during kernel init")'
> > 
> > One option is to remove the imx_register_uart_clocks from the ccm
> > driver and
> > leave the uart clock set up and enable entirely to the serial
> > driver and DT.
> > The second RFC patch does this for imx7d.
> 
> This is not an option. The CCM holds a reference to those clocks,
> specifically to prevent them from being disabled before a real serial
> driver is bound. If this isn't done you might end up in a situation
> where PM or driver probe deferal disables the UART clock/the parent
> of
> this clock, even if it was previously enabled by the bootloader. With
> earlycon this might lead to a full system hang at this point, when
> earlycon tries to access a clock gated UART.
> 
> Regards,
> Lucas
> 

Hi Lucas, Thank you for your reply.

Looking at clk_core_unprepare and clk_core_disable they seem to return
without hardware disabling the clock if enable_count or prepare_count
are 0. So, disabling a bootloader enabled only clock does not seem to
be a problem. Let me know if I misunderstand.