serial: qcom-geni: fix opp vote on shutdown
cover serial: qcom-geni: fix opp vote on shutdown
This series fixes the missing opp vote removal when closing a serial port, which, for example, can lead to wasted power when a UART connected Bluetooth controller is not in use.
Included is also a clean up of a related debug printk.
Johan
Johan Hovold (2):
serial: qcom-geni: fix opp vote on shutdown
serial: qcom-geni: clean up clock-rate debug printk
drivers/tty/serial/qcom_geni_serial.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Signature · 1 line
2.41.0
PATCH 1/2 serial: qcom-geni: fix opp vote on shutdown
The operating-performance-point vote needs to be dropped when shutting down the port to avoid wasting power by keeping resources like power domains in an unnecessarily high performance state (e.g. when a UART connected Bluetooth controller is not in use).
--- drivers/tty/serial/qcom_geni_serial.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/tty/serial/qcom_geni_serial.c +5 -0
@@ -126,6 +126,7 @@ struct qcom_geni_serial_port {dma_addr_t rx_dma_addr;bool setup;unsigned int baud; +unsigned long clk_rate;void *rx_buf;u32 loopback;bool brk;
@@ -1249,6 +1250,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, baud * sampling_rate, clk_rate, clk_div); uport->uartclk = clk_rate; + port->clk_rate = clk_rate;dev_pm_opp_set_rate (uport->dev, clk_rate); ser_clk_cfg =SER_CLK_EN ; ser_clk_cfg |= clk_div <<CLK_DIV_SHFT ;
@@ -1513,10 +1515,13 @@ static void qcom_geni_serial_pm(struct uart_port *uport,if (new_state ==UART_PM_STATE_ON && old_state ==UART_PM_STATE_OFF ) {geni_icc_enable (&port->se); +if (port->clk_rate) +dev_pm_opp_set_rate (uport->dev, port->clk_rate);geni_se_resources_on (&port->se); }else if (new_state ==UART_PM_STATE_OFF && old_state ==UART_PM_STATE_ON ) {geni_se_resources_off (&port->se); +dev_pm_opp_set_rate (uport->dev,0 );geni_icc_disable (&port->se); } }
Signature · 1 line
2.41.0
Quoted 11 lines · Johan Hovold
The operating-performance-point vote needs to be dropped when shutting down the port to avoid wasting power by keeping resources like power domains in an unnecessarily high performance state (e.g. when a UART connected Bluetooth controller is not in use).
Fixes a5819b548af0 ("tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state")Cc [email protected] # 5.9Cc Rajendra Nayak <[email protected]>Cc Matthias Kaehlcke <[email protected]>Signed-off-by Johan Hovold <[email protected]>---
I don't know a whole lot about this subsystem, but the PM call has a pointer to uport which already contains this clock rate.. Is it zeroed out by the core before we reach it, which would prevent us from reusing it?
Konrad
Quoted 37 lines
drivers/tty/serial/qcom_geni_serial.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/tty/serial/qcom_geni_serial.c +5
-0@@ -126,6 +126,7 @@ struct qcom_geni_serial_port {dma_addr_t rx_dma_addr;bool setup;unsigned int baud; +unsigned long clk_rate;void *rx_buf;u32 loopback;bool brk;@@ -1249,6 +1250,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, baud * sampling_rate, clk_rate, clk_div); uport->uartclk = clk_rate; + port->clk_rate = clk_rate;dev_pm_opp_set_rate (uport->dev, clk_rate); ser_clk_cfg =SER_CLK_EN ; ser_clk_cfg |= clk_div <<CLK_DIV_SHFT ;@@ -1513,10 +1515,13 @@ static void qcom_geni_serial_pm(struct uart_port *uport,if (new_state ==UART_PM_STATE_ON && old_state ==UART_PM_STATE_OFF ) {geni_icc_enable (&port->se); +if (port->clk_rate) +dev_pm_opp_set_rate (uport->dev, port->clk_rate);geni_se_resources_on (&port->se); }else if (new_state ==UART_PM_STATE_OFF && old_state ==UART_PM_STATE_ON ) {geni_se_resources_off (&port->se); +dev_pm_opp_set_rate (uport->dev,0 );geni_icc_disable (&port->se); } }
Quoted 12 lines · Konrad Dybcio
Quoted 11 lines · Johan Hovold
The operating-performance-point vote needs to be dropped when shutting down the port to avoid wasting power by keeping resources like power domains in an unnecessarily high performance state (e.g. when a UART connected Bluetooth controller is not in use).
Fixes a5819b548af0 ("tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state")Cc [email protected] # 5.9Cc Rajendra Nayak <[email protected]>Cc Matthias Kaehlcke <[email protected]>Signed-off-by Johan Hovold <[email protected]>---
Quoted 4 lines
I don't know a whole lot about this subsystem, but the PM call has a pointer to uport which already contains this clock rate.. Is it zeroed out by the core before we reach it, which would prevent us from reusing it?
No, but this driver has other issues and I couldn't be arsed fixing them before addressing this bug.
Specifically that uartclk variable can currently be set by userspace...
I'll fix that up next week.
Johan
Konrad Dybcio
Acked-byQuoted 18 lines · Johan Hovold
Quoted 12 lines · Konrad Dybcio
Quoted 11 lines · Johan Hovold
The operating-performance-point vote needs to be dropped when shutting down the port to avoid wasting power by keeping resources like power domains in an unnecessarily high performance state (e.g. when a UART connected Bluetooth controller is not in use).
Fixes a5819b548af0 ("tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state")Cc [email protected] # 5.9Cc Rajendra Nayak <[email protected]>Cc Matthias Kaehlcke <[email protected]>Signed-off-by Johan Hovold <[email protected]>---
Quoted 4 lines
I don't know a whole lot about this subsystem, but the PM call has a pointer to uport which already contains this clock rate.. Is it zeroed out by the core before we reach it, which would prevent us from reusing it?
Quoted 6 lines
No, but this driver has other issues and I couldn't be arsed fixing them before addressing this bug.
Specifically that uartclk variable can currently be set by userspace...
I'll fix that up next week.
OK sounds good
Konrad
Quoted 2 lines
Johan
PATCH 2/2 serial: qcom-geni: clean up clock-rate debug printk
In reply to Johan HovoldMake the clock-rate debug printk more readable by using an equal sign instead of a dash as separator between names and values and adding some spaces:
qcom_geni_serial 988000.serial: desired_rate = 1843200, clk_rate = 7372800, clk_div = 4
--- drivers/tty/serial/qcom_geni_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/qcom_geni_serial.c +1 -1
@@ -1246,7 +1246,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; } -dev_dbg (port->se.dev,"desired_rate-%u, clk_rate-%lu, clk_div-%u\n" , +dev_dbg (port->se.dev,"desired_rate = %u, clk_rate = %lu, clk_div = %u\n" , baud * sampling_rate, clk_rate, clk_div); uport->uartclk = clk_rate;
Signature · 1 line
2.41.0
Re: [PATCH 2/2] serial: qcom-geni: clean up clock-rate debug printk
Quoted 8 lines · Johan Hovold
Make the clock-rate debug printk more readable by using an equal sign instead of a dash as separator between names and values and adding some spaces:
qcom_geni_serial 988000.serial: desired_rate = 1843200, clk_rate = 7372800, clk_div = 4Signed-off-by Johan Hovold <[email protected]>---
Konrad
Quoted 16 lines
drivers/tty/serial/qcom_geni_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/tty/serial/qcom_geni_serial.c +1
-1@@ -1246,7 +1246,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; } -dev_dbg (port->se.dev,"desired_rate-%u, clk_rate-%lu, clk_div-%u\n" , +dev_dbg (port->se.dev,"desired_rate = %u, clk_rate = %lu, clk_div = %u\n" , baud * sampling_rate, clk_rate, clk_div); uport->uartclk = clk_rate;