| Message ID | 1586353607-32222-3-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DVFS for IO devices on sdm845 and sc7180 | expand |
Hi Rajendra, On Wed, Apr 08, 2020 at 07:16:28PM +0530, 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 <rnayak@codeaurora.org> > Cc: Akash Asthana <akashast@codeaurora.org> > Cc: linux-serial@vger.kernel.org > --- > drivers/tty/serial/qcom_geni_serial.c | 20 +++++++++++++++----- > include/linux/qcom-geni-se.h | 2 ++ > 2 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index 6119090..754eaf6 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -9,6 +9,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/pm_opp.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/pm_wakeirq.h> > @@ -961,7 +962,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(uport->dev, clk_rate); > ser_clk_cfg = SER_CLK_EN; > ser_clk_cfg |= clk_div << CLK_DIV_SHFT; > > @@ -1198,8 +1199,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(&port->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(uport->dev, 0); > geni_se_resources_off(&port->se); > + } > } > > static const struct uart_ops qcom_geni_console_pops = { > @@ -1318,13 +1321,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) > port->cts_rts_swap = true; > > + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); dev_pm_opp_set_clkname() can fail for multiple reasons, it seems an error check would be warranted. Is it actually necessary to save the OPP table in 'struct geni_se'? Both the serial and the SPI driver save the table, but don't use it later (nor does the SE driver).
Hi Rajendra, On 4/8/2020 7:16 PM, Rajendra Nayak wrote: > geni serial needs to express a perforamnce state requirement on CX *performance > 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 <rnayak@codeaurora.org> > Cc: Akash Asthana <akashast@codeaurora.org> > Cc: linux-serial@vger.kernel.org > --- > drivers/tty/serial/qcom_geni_serial.c | 20 +++++++++++++++----- > include/linux/qcom-geni-se.h | 2 ++ > 2 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index 6119090..754eaf6 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -9,6 +9,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/pm_opp.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/pm_wakeirq.h> > @@ -961,7 +962,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(uport->dev, clk_rate); Is this change not intended for backward compatibility? If I don't pick DT change for Geni drivers, dev_pm_opp_set_rate is failing and causing functionality issues. > ser_clk_cfg = SER_CLK_EN; > ser_clk_cfg |= clk_div << CLK_DIV_SHFT; > > @@ -1198,8 +1199,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(&port->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(uport->dev, 0); > geni_se_resources_off(&port->se); > + } > } > > static const struct uart_ops qcom_geni_console_pops = { > @@ -1318,13 +1321,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) > port->cts_rts_swap = true; > > + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); > + dev_pm_opp_of_add_table(&pdev->dev); > + > uport->private_data = drv; > platform_set_drvdata(pdev, port); > port->handle_rx = console ? handle_rx_console : handle_rx_uart; > > ret = uart_add_one_port(drv, uport); > if (ret) > - return ret; > + goto err; > > irq_set_status_flags(uport->irq, IRQ_NOAUTOEN); > ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr, > @@ -1332,7 +1338,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (ret) { > dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret); > uart_remove_one_port(drv, uport); > - return ret; > + goto err; > } > > /* > @@ -1349,11 +1355,14 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) > if (ret) { > device_init_wakeup(&pdev->dev, false); > uart_remove_one_port(drv, uport); > - return ret; > + goto err; > } > } > > return 0; > +err: > + dev_pm_opp_of_remove_table(&pdev->dev); do we need to call "dev_pm_opp_put_clkname" here and in remove to release clk resource grabbed by dev_pm_opp_set_clkname(&pdev->dev, "se");? Regards, Akash > + return ret; > } > > static int qcom_geni_serial_remove(struct platform_device *pdev) > @@ -1361,6 +1370,7 @@ static int qcom_geni_serial_remove(struct platform_device *pdev) > struct qcom_geni_serial_port *port = platform_get_drvdata(pdev); > struct uart_driver *drv = port->uport.private_data; > > + dev_pm_opp_of_remove_table(&pdev->dev); > dev_pm_clear_wake_irq(&pdev->dev); > device_init_wakeup(&pdev->dev, false); > uart_remove_one_port(drv, &port->uport); > diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h > index dd46494..737e713 100644 > --- a/include/linux/qcom-geni-se.h > +++ b/include/linux/qcom-geni-se.h > @@ -24,6 +24,7 @@ enum geni_se_protocol_type { > > struct geni_wrapper; > struct clk; > +struct opp_table; > > /** > * struct geni_se - GENI Serial Engine > @@ -39,6 +40,7 @@ struct geni_se { > struct device *dev; > struct geni_wrapper *wrapper; > struct clk *clk; > + struct opp_table *opp; > unsigned int num_clk_levels; > unsigned long *clk_perf_tbl; > };
> > @@ -961,7 +962,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(uport->dev, clk_rate); Hi Rajendra, I see lowest rpmhpd_opp_low_svs opp is for 75MHz. It is a bit higher for a serial. I am just curious about this. I also want to confirm that the rpmhpd_opp_low_svs voltage restriction is for serial controller, not for clock controller? Because I see there is similar restriction to clock controller on another platform, the restriction is for branch clock, not leaf clock that consumer device will get. Jun
Hi Rajendra, On 4/10/2020 12:26 PM, Akash Asthana wrote: > Hi Rajendra, > > On 4/8/2020 7:16 PM, Rajendra Nayak wrote: >> geni serial needs to express a perforamnce state requirement on CX > *performance >> 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 <rnayak@codeaurora.org> >> Cc: Akash Asthana <akashast@codeaurora.org> >> Cc: linux-serial@vger.kernel.org >> --- >> drivers/tty/serial/qcom_geni_serial.c | 20 +++++++++++++++----- >> include/linux/qcom-geni-se.h | 2 ++ >> 2 files changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/tty/serial/qcom_geni_serial.c >> b/drivers/tty/serial/qcom_geni_serial.c >> index 6119090..754eaf6 100644 >> --- a/drivers/tty/serial/qcom_geni_serial.c >> +++ b/drivers/tty/serial/qcom_geni_serial.c >> @@ -9,6 +9,7 @@ >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/of_device.h> >> +#include <linux/pm_opp.h> >> #include <linux/platform_device.h> >> #include <linux/pm_runtime.h> >> #include <linux/pm_wakeirq.h> >> @@ -961,7 +962,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(uport->dev, clk_rate); > > Is this change not intended for backward compatibility? If I don't > pick DT change for Geni drivers, dev_pm_opp_set_rate is failing and > causing functionality issues. oops Sorry, 1st patch is intended for backward compatibility. Which I missed earlier. Regards, Akash > >> ser_clk_cfg = SER_CLK_EN; >> ser_clk_cfg |= clk_div << CLK_DIV_SHFT; >> @@ -1198,8 +1199,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(&port->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(uport->dev, 0); >> geni_se_resources_off(&port->se); >> + } >> } >> static const struct uart_ops qcom_geni_console_pops = { >> @@ -1318,13 +1321,16 @@ static int qcom_geni_serial_probe(struct >> platform_device *pdev) >> if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) >> port->cts_rts_swap = true; >> + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); >> + dev_pm_opp_of_add_table(&pdev->dev); >> + >> uport->private_data = drv; >> platform_set_drvdata(pdev, port); >> port->handle_rx = console ? handle_rx_console : handle_rx_uart; >> ret = uart_add_one_port(drv, uport); >> if (ret) >> - return ret; >> + goto err; >> irq_set_status_flags(uport->irq, IRQ_NOAUTOEN); >> ret = devm_request_irq(uport->dev, uport->irq, >> qcom_geni_serial_isr, >> @@ -1332,7 +1338,7 @@ static int qcom_geni_serial_probe(struct >> platform_device *pdev) >> if (ret) { >> dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret); >> uart_remove_one_port(drv, uport); >> - return ret; >> + goto err; >> } >> /* >> @@ -1349,11 +1355,14 @@ static int qcom_geni_serial_probe(struct >> platform_device *pdev) >> if (ret) { >> device_init_wakeup(&pdev->dev, false); >> uart_remove_one_port(drv, uport); >> - return ret; >> + goto err; >> } >> } >> return 0; >> +err: >> + dev_pm_opp_of_remove_table(&pdev->dev); > do we need to call "dev_pm_opp_put_clkname" here and in remove to > release clk resource grabbed by > > dev_pm_opp_set_clkname(&pdev->dev, "se");? > > Regards, > Akash > >> + return ret; >> } >> static int qcom_geni_serial_remove(struct platform_device *pdev) >> @@ -1361,6 +1370,7 @@ static int qcom_geni_serial_remove(struct >> platform_device *pdev) >> struct qcom_geni_serial_port *port = platform_get_drvdata(pdev); >> struct uart_driver *drv = port->uport.private_data; >> + dev_pm_opp_of_remove_table(&pdev->dev); >> dev_pm_clear_wake_irq(&pdev->dev); >> device_init_wakeup(&pdev->dev, false); >> uart_remove_one_port(drv, &port->uport); >> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h >> index dd46494..737e713 100644 >> --- a/include/linux/qcom-geni-se.h >> +++ b/include/linux/qcom-geni-se.h >> @@ -24,6 +24,7 @@ enum geni_se_protocol_type { >> struct geni_wrapper; >> struct clk; >> +struct opp_table; >> /** >> * struct geni_se - GENI Serial Engine >> @@ -39,6 +40,7 @@ struct geni_se { >> struct device *dev; >> struct geni_wrapper *wrapper; >> struct clk *clk; >> + struct opp_table *opp; >> unsigned int num_clk_levels; >> unsigned long *clk_perf_tbl; >> }; >
Hi Matthias, On 4/9/2020 11:15 PM, Matthias Kaehlcke wrote: > Hi Rajendra, > > On Wed, Apr 08, 2020 at 07:16:28PM +0530, 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 <rnayak@codeaurora.org> >> Cc: Akash Asthana <akashast@codeaurora.org> >> Cc: linux-serial@vger.kernel.org >> --- >> drivers/tty/serial/qcom_geni_serial.c | 20 +++++++++++++++----- >> include/linux/qcom-geni-se.h | 2 ++ >> 2 files changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c >> index 6119090..754eaf6 100644 >> --- a/drivers/tty/serial/qcom_geni_serial.c >> +++ b/drivers/tty/serial/qcom_geni_serial.c >> @@ -9,6 +9,7 @@ >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/of_device.h> >> +#include <linux/pm_opp.h> >> #include <linux/platform_device.h> >> #include <linux/pm_runtime.h> >> #include <linux/pm_wakeirq.h> >> @@ -961,7 +962,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(uport->dev, clk_rate); >> ser_clk_cfg = SER_CLK_EN; >> ser_clk_cfg |= clk_div << CLK_DIV_SHFT; >> >> @@ -1198,8 +1199,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(&port->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(uport->dev, 0); >> geni_se_resources_off(&port->se); >> + } >> } >> >> static const struct uart_ops qcom_geni_console_pops = { >> @@ -1318,13 +1321,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) >> if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) >> port->cts_rts_swap = true; >> >> + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); > > dev_pm_opp_set_clkname() can fail for multiple reasons, it seems an error > check would be warranted. right, looks like I should put some error check there > Is it actually necessary to save the OPP table in 'struct geni_se'? Both > the serial and the SPI driver save the table, but don't use it later (nor > does the SE driver). I think I did that initially because I wanted to use that to call into dev_pm_opp_put_clkname during cleanup. That however never worked since the way the clk_put is done in dev_pm_opp_put_clkname() and _opp_table_kref_release() seems buggy. I kind of forgot about fixing it up, I will figure our whats the right way to do it, and either not call dev_pm_opp_put_clkname() or not store the opp table returned by it. thanks for taking time to review. - Rajendra
[].. >> @@ -1318,13 +1321,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) >> if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) >> port->cts_rts_swap = true; >> + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); >> + dev_pm_opp_of_add_table(&pdev->dev); >> + >> uport->private_data = drv; >> platform_set_drvdata(pdev, port); >> port->handle_rx = console ? handle_rx_console : handle_rx_uart; >> ret = uart_add_one_port(drv, uport); >> if (ret) >> - return ret; >> + goto err; >> irq_set_status_flags(uport->irq, IRQ_NOAUTOEN); >> ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr, >> @@ -1332,7 +1338,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) >> if (ret) { >> dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret); >> uart_remove_one_port(drv, uport); >> - return ret; >> + goto err; >> } >> /* >> @@ -1349,11 +1355,14 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) >> if (ret) { >> device_init_wakeup(&pdev->dev, false); >> uart_remove_one_port(drv, uport); >> - return ret; >> + goto err; >> } >> } >> return 0; >> +err: >> + dev_pm_opp_of_remove_table(&pdev->dev); > do we need to call "dev_pm_opp_put_clkname" here and in remove to release clk resource grabbed by > > dev_pm_opp_set_clkname(&pdev->dev, "se");? Thanks for catching this, I did indeed try to call dev_pm_opp_put_clkname() but the way clk_put is handled in it seems buggy. I need to go back and fix it. Besides I realized dev_pm_opp_of_remove_table() does go ahead and do a clk_put on the clock. Viresh, whats the right way to clean up >> + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); >> + dev_pm_opp_of_add_table(&pdev->dev); is it 1. dev_pm_opp_of_remove_table() dev_pm_opp_put_clkname() or 2. dev_pm_opp_put_clkname() dev_pm_opp_of_remove_table() or, what this patch is currently doing, which is just calling dev_pm_opp_of_remove_table()? Note that both 1. and 2. today result in a crash, since they don't handle clk_put very well. I can send in a fix if you think dev_pm_opp_put_clkname is needed and in a certain order.
On 4/10/2020 2:06 PM, Jun Nie wrote: >>> @@ -961,7 +962,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(uport->dev, clk_rate); > > Hi Rajendra, Hi Jun, > I see lowest rpmhpd_opp_low_svs opp is for 75MHz. It is a bit higher > for a serial. > I am just curious about this. Well these OPP tables are technically what we call as fmax tables, which means you can get the clock to a max of 75MHz at that perf level. You need to go to the next perf level if you want to go higher. That however does not mean that serial cannot run at clocks lower than 75Mhz. > I also want to confirm that the rpmhpd_opp_low_svs voltage restriction > is for serial > controller, not for clock controller? Because I see there is similar > restriction to clock > controller on another platform, the restriction is for branch clock, > not leaf clock that > consumer device will get. yes, its a serial controller restriction and not of the clock provider. On your note on the branch clock vs leaf clock I am not sure I understand the point you are making.
Rajendra Nayak <rnayak@codeaurora.org> 于2020年4月13日周一 下午10:22写道： > > > > On 4/10/2020 2:06 PM, Jun Nie wrote: > >>> @@ -961,7 +962,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(uport->dev, clk_rate); > > > > Hi Rajendra, > > Hi Jun, > > > I see lowest rpmhpd_opp_low_svs opp is for 75MHz. It is a bit higher > > for a serial. > > I am just curious about this. > > Well these OPP tables are technically what we call as fmax tables, which means > you can get the clock to a max of 75MHz at that perf level. You need to go > to the next perf level if you want to go higher. > That however does not mean that serial cannot run at clocks lower than 75Mhz. > > > I also want to confirm that the rpmhpd_opp_low_svs voltage restriction > > is for serial > > controller, not for clock controller? Because I see there is similar > > restriction to clock > > controller on another platform, the restriction is for branch clock, > > not leaf clock that > > consumer device will get. > > yes, its a serial controller restriction and not of the clock provider. > On your note on the branch clock vs leaf clock I am not sure I understand > the point you are making. For the leaf clock, I mean the clock that consumer get with devm_clk_get(). The branch clock means it is not for consumer directly, and its child clock or grandchild clock is for consumer. In that case, the restriction has to be done in clock driver, not in clock consumer driver. Sorry for confusing you. I just want to know more about what function this patch set provide. Because I am working on the clock controller restriction of fmax/voltage. Thanks! Jun > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 6119090..754eaf6 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -9,6 +9,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/pm_opp.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/pm_wakeirq.h> @@ -961,7 +962,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(uport->dev, clk_rate); ser_clk_cfg = SER_CLK_EN; ser_clk_cfg |= clk_div << CLK_DIV_SHFT; @@ -1198,8 +1199,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(&port->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(uport->dev, 0); geni_se_resources_off(&port->se); + } } static const struct uart_ops qcom_geni_console_pops = { @@ -1318,13 +1321,16 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) if (of_property_read_bool(pdev->dev.of_node, "cts-rts-swap")) port->cts_rts_swap = true; + port->se.opp = dev_pm_opp_set_clkname(&pdev->dev, "se"); + dev_pm_opp_of_add_table(&pdev->dev); + uport->private_data = drv; platform_set_drvdata(pdev, port); port->handle_rx = console ? handle_rx_console : handle_rx_uart; ret = uart_add_one_port(drv, uport); if (ret) - return ret; + goto err; irq_set_status_flags(uport->irq, IRQ_NOAUTOEN); ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr, @@ -1332,7 +1338,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) if (ret) { dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret); uart_remove_one_port(drv, uport); - return ret; + goto err; } /* @@ -1349,11 +1355,14 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) if (ret) { device_init_wakeup(&pdev->dev, false); uart_remove_one_port(drv, uport); - return ret; + goto err; } } return 0; +err: + dev_pm_opp_of_remove_table(&pdev->dev); + return ret; } static int qcom_geni_serial_remove(struct platform_device *pdev) @@ -1361,6 +1370,7 @@ static int qcom_geni_serial_remove(struct platform_device *pdev) struct qcom_geni_serial_port *port = platform_get_drvdata(pdev); struct uart_driver *drv = port->uport.private_data; + dev_pm_opp_of_remove_table(&pdev->dev); dev_pm_clear_wake_irq(&pdev->dev); device_init_wakeup(&pdev->dev, false); uart_remove_one_port(drv, &port->uport); diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h index dd46494..737e713 100644 --- a/include/linux/qcom-geni-se.h +++ b/include/linux/qcom-geni-se.h @@ -24,6 +24,7 @@ enum geni_se_protocol_type { struct geni_wrapper; struct clk; +struct opp_table; /** * struct geni_se - GENI Serial Engine @@ -39,6 +40,7 @@ struct geni_se { struct device *dev; struct geni_wrapper *wrapper; struct clk *clk; + struct opp_table *opp; unsigned int num_clk_levels; unsigned long *clk_perf_tbl; };
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 <rnayak@codeaurora.org> Cc: Akash Asthana <akashast@codeaurora.org> Cc: linux-serial@vger.kernel.org --- drivers/tty/serial/qcom_geni_serial.c | 20 +++++++++++++++----- include/linux/qcom-geni-se.h | 2 ++ 2 files changed, 17 insertions(+), 5 deletions(-)