Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
Hi Rajendra, On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak wrote: > > > On 8/12/2020 7:03 AM, John Stultz wrote: > > On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: > >> > >> On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak > >> wrote: > >>> > >>> geni serial needs to express a perforamnce state requirement on CX > >>> depending on the frequency of the clock rates. Use OPP table from > >>> DT to register with OPP framework and use dev_pm_opp_set_rate() to > >>> set the clk/perf state. > >>> > >>> Signed-off-by: Rajendra Nayak > >>> Signed-off-by: Stephen Boyd > >>> --- > >>> drivers/tty/serial/qcom_geni_serial.c | 15 +-- > >>> 1 file changed, 13 insertions(+), 2 deletions(-) > >>> > >> > >> Hey, > >>I just wanted to follow up on this patch, as I've bisected it > >> (a5819b548af0) down as having broken qca bluetooth on the Dragonboard > >> 845c. > >> > >> I haven't yet had time to debug it yet, but wanted to raise the issue > >> in case anyone else has seen similar trouble. > > > > So I dug in a bit further, and this chunk seems to be causing the issue: > >> @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct > >> uart_port *uport, > >> goto out_restart_rx; > >> > >> uport->uartclk = clk_rate; > >> - clk_set_rate(port->se.clk, clk_rate); > >> + dev_pm_opp_set_rate(port->dev, clk_rate); > >> ser_clk_cfg = SER_CLK_EN; > >> ser_clk_cfg |= clk_div << CLK_DIV_SHFT; > >> > > > > > > With that applied, I see the following errors in dmesg and bluetooth > > fails to function: > > [4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: > > failed to find OPP for freq 10240 (-34) > > [4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: > > failed to find OPP for freq 10240 (-34) > > > > With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. > > This seems like the same issue that was also reported on venus [1] because the > clock frequency tables apparently don;t exactly match the achievable clock > frequencies (which we also used to construct the OPP tables) > > Can you try updating the OPP table for QUP to have 10240 instead of the > current 1 and see if that fixes it? That worked. Thanks. Should this change be common to base sdm845.dtsi or platform specific dts? For what it's worth, we see this BT breakage on PocoF1 phone too. Regards, Amit Pundir > > [1] https://lkml.org/lkml/2020/7/27/507 > > > > > thanks > > -john > > > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On 8/12/2020 1:09 PM, Rajendra Nayak wrote: On 8/12/2020 1:05 PM, Amit Pundir wrote: Hi Rajendra, On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak wrote: On 8/12/2020 7:03 AM, John Stultz wrote: On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; With that applied, I see the following errors in dmesg and bluetooth fails to function: [ 4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [ 4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. This seems like the same issue that was also reported on venus [1] because the clock frequency tables apparently don;t exactly match the achievable clock frequencies (which we also used to construct the OPP tables) Can you try updating the OPP table for QUP to have 10240 instead of the current 1 and see if that fixes it? That worked. Thanks. Should this change be common to base sdm845.dtsi or platform specific dts? For what it's worth, we see this BT breakage on PocoF1 phone too. Thanks for confirming, it will have to be part of the SoC dtsi, and I am guessing a similar change is perhaps also needed on sc7180. I will send a patch out to fix the OPP tables for both. I spent some more time looking at this and it does not look like this is the rounding issues with clock FMAX tables. I had these tables picked from downstream clock code and it turns out these tables were reworked at clock init based on the silicon rev, so I need to fix up the OPP tables accordingly which will add a new OPP entry for 102.4Mhz. I'll post a patch shortly. Regards, Amit Pundir [1] https://lkml.org/lkml/2020/7/27/507 thanks -john -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On 8/12/2020 1:05 PM, Amit Pundir wrote: Hi Rajendra, On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak wrote: On 8/12/2020 7:03 AM, John Stultz wrote: On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; With that applied, I see the following errors in dmesg and bluetooth fails to function: [4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. This seems like the same issue that was also reported on venus [1] because the clock frequency tables apparently don;t exactly match the achievable clock frequencies (which we also used to construct the OPP tables) Can you try updating the OPP table for QUP to have 10240 instead of the current 1 and see if that fixes it? That worked. Thanks. Should this change be common to base sdm845.dtsi or platform specific dts? For what it's worth, we see this BT breakage on PocoF1 phone too. Thanks for confirming, it will have to be part of the SoC dtsi, and I am guessing a similar change is perhaps also needed on sc7180. I will send a patch out to fix the OPP tables for both. Regards, Amit Pundir [1] https://lkml.org/lkml/2020/7/27/507 thanks -john -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On 8/12/2020 7:03 AM, John Stultz wrote: On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: geni serial needs to express a perforamnce state requirement on CX depending on the frequency of the clock rates. Use OPP table from DT to register with OPP framework and use dev_pm_opp_set_rate() to set the clk/perf state. Signed-off-by: Rajendra Nayak Signed-off-by: Stephen Boyd --- drivers/tty/serial/qcom_geni_serial.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, goto out_restart_rx; uport->uartclk = clk_rate; - clk_set_rate(port->se.clk, clk_rate); + dev_pm_opp_set_rate(port->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; With that applied, I see the following errors in dmesg and bluetooth fails to function: [4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. This seems like the same issue that was also reported on venus [1] because the clock frequency tables apparently don;t exactly match the achievable clock frequencies (which we also used to construct the OPP tables) Can you try updating the OPP table for QUP to have 10240 instead of the current 1 and see if that fixes it? [1] https://lkml.org/lkml/2020/7/27/507 thanks -john -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On Tue, Aug 11, 2020 at 4:11 PM John Stultz wrote: > > On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: > > > > geni serial needs to express a perforamnce state requirement on CX > > depending on the frequency of the clock rates. Use OPP table from > > DT to register with OPP framework and use dev_pm_opp_set_rate() to > > set the clk/perf state. > > > > Signed-off-by: Rajendra Nayak > > Signed-off-by: Stephen Boyd > > --- > > drivers/tty/serial/qcom_geni_serial.c | 15 +-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > Hey, > I just wanted to follow up on this patch, as I've bisected it > (a5819b548af0) down as having broken qca bluetooth on the Dragonboard > 845c. > > I haven't yet had time to debug it yet, but wanted to raise the issue > in case anyone else has seen similar trouble. So I dug in a bit further, and this chunk seems to be causing the issue: > @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port > *uport, > goto out_restart_rx; > > uport->uartclk = clk_rate; > - clk_set_rate(port->se.clk, clk_rate); > + dev_pm_opp_set_rate(port->dev, clk_rate); > ser_clk_cfg = SER_CLK_EN; > ser_clk_cfg |= clk_div << CLK_DIV_SHFT; > With that applied, I see the following errors in dmesg and bluetooth fails to function: [4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) [4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate: failed to find OPP for freq 10240 (-34) With just that chunk reverted on linus/HEAD, bluetooth seems to work ok. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak wrote: > > geni serial needs to express a perforamnce state requirement on CX > depending on the frequency of the clock rates. Use OPP table from > DT to register with OPP framework and use dev_pm_opp_set_rate() to > set the clk/perf state. > > Signed-off-by: Rajendra Nayak > Signed-off-by: Stephen Boyd > --- > drivers/tty/serial/qcom_geni_serial.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > Hey, I just wanted to follow up on this patch, as I've bisected it (a5819b548af0) down as having broken qca bluetooth on the Dragonboard 845c. I haven't yet had time to debug it yet, but wanted to raise the issue in case anyone else has seen similar trouble. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
geni serial needs to express a perforamnce state requirement on CX
depending on the frequency of the clock rates. Use OPP table from
DT to register with OPP framework and use dev_pm_opp_set_rate() to
set the clk/perf state.
Signed-off-by: Rajendra Nayak
Signed-off-by: Stephen Boyd
---
drivers/tty/serial/qcom_geni_serial.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
index 3bcec1c20219..422852911141 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -115,6 +116,7 @@ struct qcom_geni_serial_port {
bool brk;
unsigned int tx_remaining;
+ struct device *dev;
};
static const struct uart_ops qcom_geni_console_pops;
@@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port
*uport,
goto out_restart_rx;
uport->uartclk = clk_rate;
- clk_set_rate(port->se.clk, clk_rate);
+ dev_pm_opp_set_rate(port->dev, clk_rate);
ser_clk_cfg = SER_CLK_EN;
ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
@@ -1198,8 +1200,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
geni_se_resources_on(>se);
else if (new_state == UART_PM_STATE_OFF &&
- old_state == UART_PM_STATE_ON)
+ old_state == UART_PM_STATE_ON) {
+ dev_pm_opp_set_rate(port->dev, 0);
geni_se_resources_off(>se);
+ }
}
static const struct uart_ops qcom_geni_console_pops = {
@@ -1265,6 +1269,7 @@ static int qcom_geni_serial_probe(struct platform_device
*pdev)
dev_err(>dev, "Invalid line %d\n", line);
return PTR_ERR(port);
}
+ port->dev = >dev;
uport = >uport;
/* Don't allow 2 drivers to access the same port */
@@ -1286,6 +1291,12 @@ static int qcom_geni_serial_probe(struct platform_device
*pdev)
return -EINVAL;
uport->mapbase = res->start;
+ ret = dev_pm_opp_of_add_table(>dev);
+ if (ret < 0) {
+ dev_err(>dev, "failed to init OPP table: %d\n", ret);
+ return ret;
+ }
+
port->tx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
