On Fri, Dec 31, 2021 at 12:24 PM Yu Tu <yu.tu@xxxxxxxxxxx> wrote: [...] > >> static int meson_uart_request_port(struct uart_port *port) > >> { > >> + struct meson_uart_data *private_data = port->private_data; > >> + int ret; > >> + > >> + ret = clk_prepare_enable(private_data->pclk); > >> + if (ret) > >> + return ret; > >> + > >> + ret = clk_prepare_enable(private_data->baud_clk); > >> + if (ret) { > >> + clk_disable_unprepare(private_data->pclk); > >> + return ret; > >> + } > > This code is from my original suggestion - and I had a doubt there > > which I forgot to add as a comment originally: > > Can you confirm that accessing the UART controller registers works > > even when "pclk" is turned off? > > I am asking this because the common clock framework can access the > > clocks at any time. > > And I have seen SoCs which would hang when trying to access a module's > > registers while the module's pclk is turned off. > On all meson platforms, the default pclk for all UART is turned on > during the u-boot phase. When registering uart pclk in the kernel phase, > the CLK_IGNORE_UNUSED flag is added. So the real shutdown is when the > standby goes down, the parent clk shuts down. Interesting, thanks for sharing that u-boot turns these clocks on. Let's say someone wanted to make u-boot save power and turn off all UART clocks except the one for uart_AO (where we typically connect the serial console). In that case the pclk of uart_C (just to choose an example here) is turned off. Would there be a problem then accessing the registers of uart_C before clk_prepare_enable is called? [...] > >> port->fifosize = 64; > > commit 27d44e05d7b85d ("tty: serial: meson: retrieve port FIFO size > > from DT") [0] from May 2021 has changed this line to: > > port->fifosize = fifosize; > > So your patch currently does not apply to linux-next (or even Linus' > > mainline tree). > > > So do I need to wait for [0] patch merged before I can continue to make > changes ? These changes are already merged. > What can I do before? You should base your changes on top of the tty.git/tty-next branch [1] where Greg (the maintainer of this tree) will pick up the patches once they are good (got enough Acked-by/Reviewed-by, etc.). I suspect that you based your changes on an older or stable kernel version (let's say 5.10). New functionality should always get into the -next tree where various auto-build robots will compile-test the changes and we even have Kernel CI where changes are tested on real hardware (BayLibre even maintains Amlogic boards in their Kernel CI labs). Let's say Amlogic updates to Linux 5.17 next year then the patches are already included in that kernel version - instead of being only available in Linux 5.10. Best regards, Martin [1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/log/?h=tty-next