Hi Evan,
On 3/18/2020 12:38 AM, Evan Green wrote:
On Tue, Mar 17, 2020 at 5:13 AM Akash Asthana <akashast@xxxxxxxxxxxxxx> wrote:
Hi Matthias,
On 3/14/2020 6:28 AM, Matthias Kaehlcke wrote:
Hi,
On Fri, Mar 13, 2020 at 06:42:13PM +0530, Akash Asthana wrote:
Get the interconnect paths for QSPI device and vote according to the
current bus speed of the driver.
Signed-off-by: Akash Asthana <akashast@xxxxxxxxxxxxxx>
---
- As per Bjorn's comment, introduced and using devm_of_icc_get API for getting
path handle
- As per Matthias comment, added error handling for icc_set_bw call
drivers/spi/spi-qcom-qspi.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index 3c4f83b..ad48f43 100644
--- a/drivers/spi/spi-qcom-qspi.c
+++ b/drivers/spi/spi-qcom-qspi.c
@@ -2,6 +2,7 @@
// Copyright (c) 2017-2018, The Linux foundation. All rights reserved.
#include <linux/clk.h>
+#include <linux/interconnect.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -139,7 +140,10 @@ struct qcom_qspi {
struct device *dev;
struct clk_bulk_data *clks;
struct qspi_xfer xfer;
- /* Lock to protect xfer and IRQ accessed registers */
+ struct icc_path *icc_path_cpu_to_qspi;
+ unsigned int avg_bw_cpu;
+ unsigned int peak_bw_cpu;
This triplet is a recurring pattern, and is probably not limited to geni SE/QSPI.
On https://patchwork.kernel.org/patch/11436889/#23221925 I suggested the creation
of a geni SE specific struct, however adding a generic convenience struct to
'linux/interconnect.h' might be the better solution:
struct icc_client {
struct icc_path *path;
unsigned int avg_bw;
unsigned int peak_bw;
};
I'm sure there are better names for it, but this would be the idea.
Yeah, I think introducing this to ICC header would be better solution.
+Georgi
I'm not as convinced this structure is generally useful and belongs in
the interconnect core. The thing that strikes me as weird with putting
it in the core is now we're saving these values both inside and
outside the interconnect core.
IIUC, you meant to say struct icc_req(inside icc_path) will be saving
avg_bw and peak_bw so no need to save it outside icc_path?
In the GENI case here, we only really
need them to undo the 0 votes we cast during suspend. If "vote for 0
in suspend and whatever it was before at resume" is a recurring theme,
maybe the core should give us path_disable() and path_enable() calls
instead. I'm thinking out loud, maybe Georgi has some thoughts.
Akash, for now if you want to avoid wading into a larger discussion
maybe just refactor to a common structure local to GENI.
Ok
Thanks,
Akash
-Evan
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
