2019-01-22 06:36:29

by Alok Chauhan

Subject: [PATCH 0/6] Add interconnect support for GENI QUPs

This patch series contains following:
* Add wrapper framework to support interconnect path from GENI QUPs.
This wrapper enabled and help individual SEs to put their BW request.
Adding this wrapper make sense because we don't want individual SEs
to request to interconnect driver separately and put individual bw
votes from QUP.

This wrapper framework does the following:
- Request for interconnect path handle
- Maintain record of individual SEs' avg/peak bw.
- Aggregated avg/peak bw based on how many SE's are active and put
single bw request from QUP

* Interconnect wrapper API calling from I2C, SPI & Uart driver
* dt binding in sdm845 soc for Interconnect path for GENI QUPs
* dt binding documentation


Alok Chauhan (6):
dt-bindings: soc: qcom: Add interconnect binding for GENI QUP
soc: qcom: Add wrapper to support for Interconnect path
i2c: i2c-qcom-geni: Add interconnect support
spi: spi-geni-qcom: Add interconnect support
tty: serial: qcom_geni_serial: Add interconnect support
arm64: dts: sdm845: Add interconnect for GENI QUP

.../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 14 +++
drivers/i2c/busses/i2c-qcom-geni.c | 13 +++
drivers/soc/qcom/qcom-geni-se.c | 129 +++++++++++++++++++++
drivers/spi/spi-geni-qcom.c | 20 +++-
drivers/tty/serial/qcom_geni_serial.c | 27 ++++-
include/linux/qcom-geni-se.h | 11 ++
7 files changed, 222 insertions(+), 2 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:35:59

by Alok Chauhan

Subject: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

Add documentation for the interconnect and interconnect-names bindings
for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.

Signed-off-by: Alok Chauhan <[email protected]>
---
Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
index dab7ca9..44d7e02 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
@@ -17,6 +17,12 @@ Required properties if child node exists:
- #address-cells: Must be <1> for Serial Engine Address
- #size-cells: Must be <1> for Serial Engine Address Size
- ranges: Must be present
+- interconnects: phandle to a interconnect provider. Please refer
+ ../interconnect/interconnect.txt for details.
+ Must be 2 paths corresponding to 2 AXI ports.
+- interconnect-names: Port names to differentiate between the
+ 2 interconnect paths defined with interconnect
+ specifier.

Properties for children:

@@ -67,6 +73,10 @@ Example:
#size-cells = <1>;
ranges;

+ interconnects = <&qnoc 11 &qnoc 512>,
+ <&qnoc 0 &qnoc 543>;
+ interconnect-names = "qup-memory", "qup-config";
+
i2c0: i2c@a94000 {
compatible = "qcom,geni-i2c";
reg = <0xa94000 0x4000>;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:36:07

by Alok Chauhan

Subject: [PATCH 3/6] i2c: i2c-qcom-geni: Add interconnect support

Get the interconnect paths for I2C based Serial Engine device
and vote accordingly based on maximum supported I2C frequency.

Signed-off-by: Alok Chauhan <[email protected]>
---
drivers/i2c/busses/i2c-qcom-geni.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index db075bc..e8fe63a 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -14,6 +14,7 @@
#include <linux/pm_runtime.h>
#include <linux/qcom-geni-se.h>
#include <linux/spinlock.h>
+#include <linux/interconnect.h>

#define SE_I2C_TX_TRANS_LEN 0x26c
#define SE_I2C_RX_TRANS_LEN 0x270
@@ -508,6 +509,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
return ret;
}

+ /* Set the bus quota to a reasonable value */
+ gi2c->se.avg_bw = Bps_to_icc(1000);
+ gi2c->se.peak_bw = Bps_to_icc(76800000);
+ ret = geni_interconnect_init(&gi2c->se);
+ if (ret) {
+ dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
+ return ret;
+ }
+
ret = device_property_read_u32(&pdev->dev, "clock-frequency",
&gi2c->clk_freq_out);
if (ret) {
@@ -611,6 +621,8 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
gi2c->suspended = 1;
}

+ geni_icc_update_bw(&gi2c->se, false);
+
return 0;
}

@@ -619,6 +631,7 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
int ret;
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);

+ geni_icc_update_bw(&gi2c->se, true);
ret = geni_se_resources_on(&gi2c->se);
if (ret)
return ret;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:36:11

by Alok Chauhan

Subject: [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support

Get the interconnect paths for SPI based Serial Engine device
and vote accordingly based on maximum supported SPI frequency.

Signed-off-by: Alok Chauhan <[email protected]>
---
drivers/spi/spi-geni-qcom.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index fdb7cb88..7bbbe9d 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -12,6 +12,7 @@
#include <linux/qcom-geni-se.h>
#include <linux/spi/spi.h>
#include <linux/spinlock.h>
+#include <linux/interconnect.h>

/* SPI SE specific registers and respective register fields */
#define SE_SPI_CPHA 0x224
@@ -589,6 +590,15 @@ static int spi_geni_probe(struct platform_device *pdev)
spin_lock_init(&mas->lock);
pm_runtime_enable(&pdev->dev);

+ /* Set the bus quota to a reasonable value */
+ mas->se.avg_bw = Bps_to_icc(2500);
+ mas->se.peak_bw = Bps_to_icc(200000000);
+ ret = geni_interconnect_init(&mas->se);
+ if (ret) {
+ dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
+ return ret;
+ }
+
ret = spi_geni_init(mas);
if (ret)
goto spi_geni_probe_runtime_disable;
@@ -628,8 +638,15 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
{
struct spi_master *spi = dev_get_drvdata(dev);
struct spi_geni_master *mas = spi_master_get_devdata(spi);
+ int ret;

- return geni_se_resources_off(&mas->se);
+ ret = geni_se_resources_off(&mas->se);
+ if (ret)
+ return ret;
+
+ geni_icc_update_bw(&mas->se, false);
+
+ return 0;
}

static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
@@ -637,6 +654,7 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev)
struct spi_master *spi = dev_get_drvdata(dev);
struct spi_geni_master *mas = spi_master_get_devdata(spi);

+ geni_icc_update_bw(&mas->se, true);
return geni_se_resources_on(&mas->se);
}

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:36:16

by Alok Chauhan

Subject: [PATCH 2/6] soc: qcom: Add wrapper to support for Interconnect path

Add the wrapper to support for interconnect path voting
from GENI QUP. This wrapper provides the functionalities
to individual Serial Engine device to get the interconnect
path and to vote for bandwidth based on their need.

This wrapper maintains bandwidth votes from each Serial Engine
and send the aggregated vote from GENI QUP to interconnect
framework.

Signed-off-by: Alok Chauhan <[email protected]>
---
drivers/soc/qcom/qcom-geni-se.c | 129 ++++++++++++++++++++++++++++++++++++++++
include/linux/qcom-geni-se.h | 11 ++++
2 files changed, 140 insertions(+)

diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 6b8ef01..1d8dcb1 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -11,6 +11,7 @@
#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
#include <linux/qcom-geni-se.h>
+#include <linux/interconnect.h>

/**
* DOC: Overview
@@ -84,11 +85,21 @@
* @dev: Device pointer of the QUP wrapper core
* @base: Base address of this instance of QUP wrapper core
* @ahb_clks: Handle to the primary & secondary AHB clocks
+ * @icc_path: Array of interconnect path handles
+ * @geni_wrapper_lock: Lock to protect the bus bandwidth request
+ * @cur_avg_bw: Current Bus Average BW request value from GENI QUP
+ * @cur_peak_bw: Current Bus Peak BW request value from GENI QUP
+ * @peak_bw_list_head: Sorted resource list based on peak bus BW
*/
struct geni_wrapper {
struct device *dev;
void __iomem *base;
struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
+ struct icc_path *icc_path[2];
+ struct mutex geni_wrapper_lock;
+ u32 cur_avg_bw;
+ u32 cur_peak_bw;
+ struct list_head peak_bw_list_head;
};

#define QUP_HW_VER_REG 0x4
@@ -440,6 +451,71 @@ static void geni_se_clks_off(struct geni_se *se)
}

/**
+ * geni_icc_update_bw() - Request to update bw vote on an interconnect path
+ * @se: Pointer to the concerned serial engine.
+ * @update: Flag to update bw vote.
+ *
+ * This function is used to request set bw vote on interconnect path handle.
+ */
+void geni_icc_update_bw(struct geni_se *se, bool update)
+{
+ struct geni_se *temp = NULL;
+ struct list_head *ins_list_head;
+ struct geni_wrapper *wrapper;
+
+ mutex_lock(&se->wrapper->geni_wrapper_lock);
+
+ wrapper = se->wrapper;
+
+ if (update) {
+ wrapper->cur_avg_bw += se->avg_bw;
+ ins_list_head = &wrapper->peak_bw_list_head;
+ list_for_each_entry(temp, &wrapper->peak_bw_list_head,
+ peak_bw_list) {
+ if (temp->peak_bw < se->peak_bw)
+ break;
+ ins_list_head = &temp->peak_bw_list;
+ }
+
+ list_add(&se->peak_bw_list, ins_list_head);
+
+ if (ins_list_head == &wrapper->peak_bw_list_head)
+ wrapper->cur_peak_bw = se->peak_bw;
+ } else {
+ if (unlikely(list_empty(&se->peak_bw_list))) {
+ mutex_unlock(&wrapper->geni_wrapper_lock);
+ return;
+ }
+
+ wrapper->cur_avg_bw -= se->avg_bw;
+
+ list_del_init(&se->peak_bw_list);
+ temp = list_first_entry_or_null(&wrapper->peak_bw_list_head,
+ struct geni_se, peak_bw_list);
+ if (temp && temp->peak_bw != wrapper->cur_peak_bw)
+ wrapper->cur_peak_bw = temp->peak_bw;
+ else if (!temp && wrapper->cur_peak_bw)
+ wrapper->cur_peak_bw = 0;
+ }
+
+ /*
+ * This bw vote is to enable internal QUP core clock as well as to
+ * enable path towards memory.
+ */
+ icc_set_bw(wrapper->icc_path[0], wrapper->cur_avg_bw,
+ wrapper->cur_peak_bw);
+
+ /*
+ * This is just register configuration path so doesn't need avg bw.
+ * Set only peak bw to enable this path.
+ */
+ icc_set_bw(wrapper->icc_path[1], 0, wrapper->cur_peak_bw);
+
+ mutex_unlock(&wrapper->geni_wrapper_lock);
+}
+EXPORT_SYMBOL(geni_icc_update_bw);
+
+/**
* geni_se_resources_off() - Turn off resources associated with the serial
* engine
* @se: Pointer to the concerned serial engine.
@@ -707,6 +783,47 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len)
}
EXPORT_SYMBOL(geni_se_rx_dma_unprep);

+/**
+ * geni_interconnect_init() - Request to get interconnect path handle
+ * @se: Pointer to the concerned serial engine.
+ *
+ * This function is used to get interconnect path handle.
+ */
+int geni_interconnect_init(struct geni_se *se)
+{
+ struct geni_wrapper *wrapper_rsc;
+
+ if (unlikely(!se || !se->wrapper))
+ return -EINVAL;
+
+ wrapper_rsc = se->wrapper;
+
+ if ((IS_ERR_OR_NULL(wrapper_rsc->icc_path[0]) ||
+ IS_ERR_OR_NULL(wrapper_rsc->icc_path[1]))) {
+
+ wrapper_rsc->icc_path[0] = of_icc_get(wrapper_rsc->dev,
+ "qup-memory");
+ if (IS_ERR(wrapper_rsc->icc_path[0]))
+ return PTR_ERR(wrapper_rsc->icc_path[0]);
+
+ wrapper_rsc->icc_path[1] = of_icc_get(wrapper_rsc->dev,
+ "qup-config");
+ if (IS_ERR(wrapper_rsc->icc_path[1])) {
+ icc_put(wrapper_rsc->icc_path[0]);
+ wrapper_rsc->icc_path[0] = NULL;
+ return PTR_ERR(wrapper_rsc->icc_path[1]);
+ }
+
+ INIT_LIST_HEAD(&wrapper_rsc->peak_bw_list_head);
+ mutex_init(&wrapper_rsc->geni_wrapper_lock);
+ }
+
+ INIT_LIST_HEAD(&se->peak_bw_list);
+
+ return 0;
+}
+EXPORT_SYMBOL(geni_interconnect_init);
+
static int geni_se_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -737,6 +854,17 @@ static int geni_se_probe(struct platform_device *pdev)
return devm_of_platform_populate(dev);
}

+static int geni_se_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct geni_wrapper *wrapper = dev_get_drvdata(dev);
+
+ icc_put(wrapper->icc_path[0]);
+ icc_put(wrapper->icc_path[1]);
+
+ return 0;
+}
+
static const struct of_device_id geni_se_dt_match[] = {
{ .compatible = "qcom,geni-se-qup", },
{}
@@ -749,6 +877,7 @@ static int geni_se_probe(struct platform_device *pdev)
.of_match_table = geni_se_dt_match,
},
.probe = geni_se_probe,
+ .remove = geni_se_remove,
};
module_platform_driver(geni_se_driver);

diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 3bcd67f..9bf03e9 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -33,6 +33,10 @@ enum geni_se_protocol_type {
* @clk: Handle to the core serial engine clock
* @num_clk_levels: Number of valid clock levels in clk_perf_tbl
* @clk_perf_tbl: Table of clock frequency input to serial engine clock
+ * @avg_bw: Average bus bandwidth value for Serial Engine device
+ * @peak_bw: Peak bus bandwidth value for Serial Engine device
+ * @peak_bw_list: List Head of peak bus banwidth list for Serial Engine
+ * device
*/
struct geni_se {
void __iomem *base;
@@ -41,6 +45,9 @@ struct geni_se {
struct clk *clk;
unsigned int num_clk_levels;
unsigned long *clk_perf_tbl;
+ u32 avg_bw;
+ u32 peak_bw;
+ struct list_head peak_bw_list;
};

/* Common SE registers */
@@ -416,5 +423,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void *buf, size_t len,
void geni_se_tx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len);

void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len);
+
+int geni_interconnect_init(struct geni_se *se);
+
+void geni_icc_update_bw(struct geni_se *se, bool update);
#endif
#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:36:46

by Alok Chauhan

Subject: [PATCH 6/6] arm64: dts: sdm845: Add interconnect for GENI QUP

Add interconnect ports for GENI QUPs to set bus
capabilities.

Signed-off-by: Alok Chauhan <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index c27cbd3..fb0a8a7 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -374,6 +374,13 @@
#address-cells = <1>;
#size-cells = <1>;
ranges;
+
+ interconnects = <&rsc_hlos MASTER_BLSP_1
+ &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC
+ &rsc_hlos SLAVE_BLSP_1>;
+ interconnect-names = "qup-memory", "qup-config";
+
status = "disabled";

i2c0: i2c@880000 {
@@ -682,6 +689,13 @@
#address-cells = <1>;
#size-cells = <1>;
ranges;
+
+ interconnects = <&rsc_hlos MASTER_BLSP_2
+ &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC
+ &rsc_hlos SLAVE_BLSP_2>;
+ interconnect-names = "qup-memory", "qup-config";
+
status = "disabled";

i2c8: i2c@a80000 {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:37:05

by Alok Chauhan

Subject: [PATCH 5/6] tty: serial: qcom_geni_serial: Add interconnect support

Get the interconnect paths for Uart based Serial Engine device
and vote accordingly based on maximum supported Uart frequency.

Signed-off-by: Alok Chauhan <[email protected]>
---
drivers/tty/serial/qcom_geni_serial.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index a72d6d9..e2ea499 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
+#include <linux/interconnect.h>

/* UART specific GENI registers */
#define SE_UART_LOOPBACK_CFG 0x22c
@@ -1348,6 +1349,22 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
return ret;
}

+ /* Set the bus quota to a reasonable value */
+ port->se.avg_bw = console ? Bps_to_icc(1000) : Bps_to_icc(2500);
+ port->se.peak_bw = Bps_to_icc(76800000);
+ ret = geni_interconnect_init(&port->se);
+ if (ret) {
+ dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Vote for interconnect path early. This has to move as part of
+ * Runtime PM APIs when implemented for better control betwen
+ * console and non-console usecases
+ */
+ geni_icc_update_bw(&port->se, true);
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -EINVAL;
@@ -1385,8 +1402,15 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
{
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;
+ int ret;
+
+ ret = uart_suspend_port(uport->private_data, uport);
+ if (ret)
+ return ret;
+
+ geni_icc_update_bw(&port->se, false);

- return uart_suspend_port(uport->private_data, uport);
+ return 0;
}

static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
@@ -1394,6 +1418,7 @@ static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;

+ geni_icc_update_bw(&port->se, true);
return uart_resume_port(uport->private_data, uport);
}

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-01-22 06:47:07

by Alok Chauhan

Subject: Re: [PATCH 2/6] soc: qcom: Add wrapper to support for Interconnect path

Please don't review this patch. I've resend patches after adding all the
mailing list.
Sorry for inconvenience.

On 2019-01-21 16:51, Alok Chauhan wrote:
> Add the wrapper to support for interconnect path voting
> from GENI QUP. This wrapper provides the functionalities
> to individual Serial Engine device to get the interconnect
> path and to vote for bandwidth based on their need.
>
> This wrapper maintains bandwidth votes from each Serial Engine
> and send the aggregated vote from GENI QUP to interconnect
> framework.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 129
> ++++++++++++++++++++++++++++++++++++++++
> include/linux/qcom-geni-se.h | 11 ++++
> 2 files changed, 140 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c
> b/drivers/soc/qcom/qcom-geni-se.c
> index 6b8ef01..1d8dcb1 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -11,6 +11,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/qcom-geni-se.h>
> +#include <linux/interconnect.h>
>
> /**
> * DOC: Overview
> @@ -84,11 +85,21 @@
> * @dev: Device pointer of the QUP wrapper core
> * @base: Base address of this instance of QUP wrapper core
> * @ahb_clks: Handle to the primary & secondary AHB clocks
> + * @icc_path: Array of interconnect path handles
> + * @geni_wrapper_lock: Lock to protect the bus bandwidth request
> + * @cur_avg_bw: Current Bus Average BW request value from GENI QUP
> + * @cur_peak_bw: Current Bus Peak BW request value from GENI QUP
> + * @peak_bw_list_head: Sorted resource list based on peak bus BW
> */
> struct geni_wrapper {
> struct device *dev;
> void __iomem *base;
> struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> + struct icc_path *icc_path[2];
> + struct mutex geni_wrapper_lock;
> + u32 cur_avg_bw;
> + u32 cur_peak_bw;
> + struct list_head peak_bw_list_head;
> };
>
> #define QUP_HW_VER_REG 0x4
> @@ -440,6 +451,71 @@ static void geni_se_clks_off(struct geni_se *se)
> }
>
> /**
> + * geni_icc_update_bw() - Request to update bw vote on an interconnect
> path
> + * @se: Pointer to the concerned serial engine.
> + * @update: Flag to update bw vote.
> + *
> + * This function is used to request set bw vote on interconnect path
> handle.
> + */
> +void geni_icc_update_bw(struct geni_se *se, bool update)
> +{
> + struct geni_se *temp = NULL;
> + struct list_head *ins_list_head;
> + struct geni_wrapper *wrapper;
> +
> + mutex_lock(&se->wrapper->geni_wrapper_lock);
> +
> + wrapper = se->wrapper;
> +
> + if (update) {
> + wrapper->cur_avg_bw += se->avg_bw;
> + ins_list_head = &wrapper->peak_bw_list_head;
> + list_for_each_entry(temp, &wrapper->peak_bw_list_head,
> + peak_bw_list) {
> + if (temp->peak_bw < se->peak_bw)
> + break;
> + ins_list_head = &temp->peak_bw_list;
> + }
> +
> + list_add(&se->peak_bw_list, ins_list_head);
> +
> + if (ins_list_head == &wrapper->peak_bw_list_head)
> + wrapper->cur_peak_bw = se->peak_bw;
> + } else {
> + if (unlikely(list_empty(&se->peak_bw_list))) {
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> + return;
> + }
> +
> + wrapper->cur_avg_bw -= se->avg_bw;
> +
> + list_del_init(&se->peak_bw_list);
> + temp = list_first_entry_or_null(&wrapper->peak_bw_list_head,
> + struct geni_se, peak_bw_list);
> + if (temp && temp->peak_bw != wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = temp->peak_bw;
> + else if (!temp && wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = 0;
> + }
> +
> + /*
> + * This bw vote is to enable internal QUP core clock as well as to
> + * enable path towards memory.
> + */
> + icc_set_bw(wrapper->icc_path[0], wrapper->cur_avg_bw,
> + wrapper->cur_peak_bw);
> +
> + /*
> + * This is just register configuration path so doesn't need avg bw.
> + * Set only peak bw to enable this path.
> + */
> + icc_set_bw(wrapper->icc_path[1], 0, wrapper->cur_peak_bw);
> +
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> +}
> +EXPORT_SYMBOL(geni_icc_update_bw);
> +
> +/**
> * geni_se_resources_off() - Turn off resources associated with the
> serial
> * engine
> * @se: Pointer to the concerned serial engine.
> @@ -707,6 +783,47 @@ void geni_se_rx_dma_unprep(struct geni_se *se,
> dma_addr_t iova, size_t len)
> }
> EXPORT_SYMBOL(geni_se_rx_dma_unprep);
>
> +/**
> + * geni_interconnect_init() - Request to get interconnect path handle
> + * @se: Pointer to the concerned serial engine.
> + *
> + * This function is used to get interconnect path handle.
> + */
> +int geni_interconnect_init(struct geni_se *se)
> +{
> + struct geni_wrapper *wrapper_rsc;
> +
> + if (unlikely(!se || !se->wrapper))
> + return -EINVAL;
> +
> + wrapper_rsc = se->wrapper;
> +
> + if ((IS_ERR_OR_NULL(wrapper_rsc->icc_path[0]) ||
> + IS_ERR_OR_NULL(wrapper_rsc->icc_path[1]))) {
> +
> + wrapper_rsc->icc_path[0] = of_icc_get(wrapper_rsc->dev,
> + "qup-memory");
> + if (IS_ERR(wrapper_rsc->icc_path[0]))
> + return PTR_ERR(wrapper_rsc->icc_path[0]);
> +
> + wrapper_rsc->icc_path[1] = of_icc_get(wrapper_rsc->dev,
> + "qup-config");
> + if (IS_ERR(wrapper_rsc->icc_path[1])) {
> + icc_put(wrapper_rsc->icc_path[0]);
> + wrapper_rsc->icc_path[0] = NULL;
> + return PTR_ERR(wrapper_rsc->icc_path[1]);
> + }
> +
> + INIT_LIST_HEAD(&wrapper_rsc->peak_bw_list_head);
> + mutex_init(&wrapper_rsc->geni_wrapper_lock);
> + }
> +
> + INIT_LIST_HEAD(&se->peak_bw_list);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(geni_interconnect_init);
> +
> static int geni_se_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -737,6 +854,17 @@ static int geni_se_probe(struct platform_device
> *pdev)
> return devm_of_platform_populate(dev);
> }
>
> +static int geni_se_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct geni_wrapper *wrapper = dev_get_drvdata(dev);
> +
> + icc_put(wrapper->icc_path[0]);
> + icc_put(wrapper->icc_path[1]);
> +
> + return 0;
> +}
> +
> static const struct of_device_id geni_se_dt_match[] = {
> { .compatible = "qcom,geni-se-qup", },
> {}
> @@ -749,6 +877,7 @@ static int geni_se_probe(struct platform_device
> *pdev)
> .of_match_table = geni_se_dt_match,
> },
> .probe = geni_se_probe,
> + .remove = geni_se_remove,
> };
> module_platform_driver(geni_se_driver);
>
> diff --git a/include/linux/qcom-geni-se.h
> b/include/linux/qcom-geni-se.h
> index 3bcd67f..9bf03e9 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -33,6 +33,10 @@ enum geni_se_protocol_type {
> * @clk: Handle to the core serial engine clock
> * @num_clk_levels: Number of valid clock levels in clk_perf_tbl
> * @clk_perf_tbl: Table of clock frequency input to serial engine
> clock
> + * @avg_bw: Average bus bandwidth value for Serial Engine device
> + * @peak_bw: Peak bus bandwidth value for Serial Engine device
> + * @peak_bw_list: List Head of peak bus banwidth list for Serial
> Engine
> + * device
> */
> struct geni_se {
> void __iomem *base;
> @@ -41,6 +45,9 @@ struct geni_se {
> struct clk *clk;
> unsigned int num_clk_levels;
> unsigned long *clk_perf_tbl;
> + u32 avg_bw;
> + u32 peak_bw;
> + struct list_head peak_bw_list;
> };
>
> /* Common SE registers */
> @@ -416,5 +423,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void
> *buf, size_t len,
> void geni_se_tx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t
> len);
>
> void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t
> len);
> +
> +int geni_interconnect_init(struct geni_se *se);
> +
> +void geni_icc_update_bw(struct geni_se *se, bool update);
> #endif
> #endif

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-22 09:09:59

by Wolfram Sang

Subject: Re: [PATCH 3/6] i2c: i2c-qcom-geni: Add interconnect support

On Tue, Jan 22, 2019 at 12:03:33PM +0530, Alok Chauhan wrote:
> Get the interconnect paths for I2C based Serial Engine device
> and vote accordingly based on maximum supported I2C frequency.
>
> Signed-off-by: Alok Chauhan <[email protected]>

Acked-by: Wolfram Sang <[email protected]>

Attachments:

2019-01-22 09:14:54

by Peter Rosin

Subject: Re: [PATCH 3/6] i2c: i2c-qcom-geni: Add interconnect support

On 2019-01-22 07:33, Alok Chauhan wrote:
> Get the interconnect paths for I2C based Serial Engine device
> and vote accordingly based on maximum supported I2C frequency.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index db075bc..e8fe63a 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -14,6 +14,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/qcom-geni-se.h>
> #include <linux/spinlock.h>
> +#include <linux/interconnect.h>

Please keep the includes sorted. Probably applies for the other patches
in the series as well?

Cheers,
Peter

2019-01-22 20:31:59

by Mark Brown

Subject: Re: [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support

On Tue, Jan 22, 2019 at 12:03:34PM +0530, Alok Chauhan wrote:

> Get the interconnect paths for SPI based Serial Engine device
> and vote accordingly based on maximum supported SPI frequency.

Still not seeing anything except this patch here in my inbox - like I
said what's the story with dependencies?

Attachments:

2019-01-23 06:46:34

by Alok Chauhan

Subject: Re: [PATCH 0/6] Add interconnect support for GENI QUPs

+Mark Brown

On 2019-01-22 12:03, Alok Chauhan wrote:
> This patch series contains following:
> * Add wrapper framework to support interconnect path from GENI QUPs.
> This wrapper enabled and help individual SEs to put their BW request.
> Adding this wrapper make sense because we don't want individual SEs
> to request to interconnect driver separately and put individual bw
> votes from QUP.
>
> This wrapper framework does the following:
> - Request for interconnect path handle
> - Maintain record of individual SEs' avg/peak bw.
> - Aggregated avg/peak bw based on how many SE's are active and put
> single bw request from QUP
>
> * Interconnect wrapper API calling from I2C, SPI & Uart driver
> * dt binding in sdm845 soc for Interconnect path for GENI QUPs
> * dt binding documentation
>
>
> Alok Chauhan (6):
> dt-bindings: soc: qcom: Add interconnect binding for GENI QUP
> soc: qcom: Add wrapper to support for Interconnect path
> i2c: i2c-qcom-geni: Add interconnect support
> spi: spi-geni-qcom: Add interconnect support
> tty: serial: qcom_geni_serial: Add interconnect support
> arm64: dts: sdm845: Add interconnect for GENI QUP
>
> .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 14 +++
> drivers/i2c/busses/i2c-qcom-geni.c | 13 +++
> drivers/soc/qcom/qcom-geni-se.c | 129
> +++++++++++++++++++++
> drivers/spi/spi-geni-qcom.c | 20 +++-
> drivers/tty/serial/qcom_geni_serial.c | 27 ++++-
> include/linux/qcom-geni-se.h | 11 ++
> 7 files changed, 222 insertions(+), 2 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 06:49:32

by Alok Chauhan

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

+Mark Brown

On 2019-01-22 12:03, Alok Chauhan wrote:
> Add documentation for the interconnect and interconnect-names bindings
> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10
> ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> index dab7ca9..44d7e02 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> @@ -17,6 +17,12 @@ Required properties if child node exists:
> - #address-cells: Must be <1> for Serial Engine Address
> - #size-cells: Must be <1> for Serial Engine Address Size
> - ranges: Must be present
> +- interconnects: phandle to a interconnect provider. Please refer
> + ../interconnect/interconnect.txt for details.
> + Must be 2 paths corresponding to 2 AXI ports.
> +- interconnect-names: Port names to differentiate between the
> + 2 interconnect paths defined with interconnect
> + specifier.
>
> Properties for children:
>
> @@ -67,6 +73,10 @@ Example:
> #size-cells = <1>;
> ranges;
>
> + interconnects = <&qnoc 11 &qnoc 512>,
> + <&qnoc 0 &qnoc 543>;
> + interconnect-names = "qup-memory", "qup-config";
> +
> i2c0: i2c@a94000 {
> compatible = "qcom,geni-i2c";
> reg = <0xa94000 0x4000>;

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 06:51:16

by Alok Chauhan

Subject: Re: [PATCH 2/6] soc: qcom: Add wrapper to support for Interconnect path

+Mark Brown

On 2019-01-22 12:03, Alok Chauhan wrote:
> Add the wrapper to support for interconnect path voting
> from GENI QUP. This wrapper provides the functionalities
> to individual Serial Engine device to get the interconnect
> path and to vote for bandwidth based on their need.
>
> This wrapper maintains bandwidth votes from each Serial Engine
> and send the aggregated vote from GENI QUP to interconnect
> framework.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 129
> ++++++++++++++++++++++++++++++++++++++++
> include/linux/qcom-geni-se.h | 11 ++++
> 2 files changed, 140 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c
> b/drivers/soc/qcom/qcom-geni-se.c
> index 6b8ef01..1d8dcb1 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -11,6 +11,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/qcom-geni-se.h>
> +#include <linux/interconnect.h>
>
> /**
> * DOC: Overview
> @@ -84,11 +85,21 @@
> * @dev: Device pointer of the QUP wrapper core
> * @base: Base address of this instance of QUP wrapper core
> * @ahb_clks: Handle to the primary & secondary AHB clocks
> + * @icc_path: Array of interconnect path handles
> + * @geni_wrapper_lock: Lock to protect the bus bandwidth request
> + * @cur_avg_bw: Current Bus Average BW request value from GENI QUP
> + * @cur_peak_bw: Current Bus Peak BW request value from GENI QUP
> + * @peak_bw_list_head: Sorted resource list based on peak bus BW
> */
> struct geni_wrapper {
> struct device *dev;
> void __iomem *base;
> struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> + struct icc_path *icc_path[2];
> + struct mutex geni_wrapper_lock;
> + u32 cur_avg_bw;
> + u32 cur_peak_bw;
> + struct list_head peak_bw_list_head;
> };
>
> #define QUP_HW_VER_REG 0x4
> @@ -440,6 +451,71 @@ static void geni_se_clks_off(struct geni_se *se)
> }
>
> /**
> + * geni_icc_update_bw() - Request to update bw vote on an interconnect
> path
> + * @se: Pointer to the concerned serial engine.
> + * @update: Flag to update bw vote.
> + *
> + * This function is used to request set bw vote on interconnect path
> handle.
> + */
> +void geni_icc_update_bw(struct geni_se *se, bool update)
> +{
> + struct geni_se *temp = NULL;
> + struct list_head *ins_list_head;
> + struct geni_wrapper *wrapper;
> +
> + mutex_lock(&se->wrapper->geni_wrapper_lock);
> +
> + wrapper = se->wrapper;
> +
> + if (update) {
> + wrapper->cur_avg_bw += se->avg_bw;
> + ins_list_head = &wrapper->peak_bw_list_head;
> + list_for_each_entry(temp, &wrapper->peak_bw_list_head,
> + peak_bw_list) {
> + if (temp->peak_bw < se->peak_bw)
> + break;
> + ins_list_head = &temp->peak_bw_list;
> + }
> +
> + list_add(&se->peak_bw_list, ins_list_head);
> +
> + if (ins_list_head == &wrapper->peak_bw_list_head)
> + wrapper->cur_peak_bw = se->peak_bw;
> + } else {
> + if (unlikely(list_empty(&se->peak_bw_list))) {
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> + return;
> + }
> +
> + wrapper->cur_avg_bw -= se->avg_bw;
> +
> + list_del_init(&se->peak_bw_list);
> + temp = list_first_entry_or_null(&wrapper->peak_bw_list_head,
> + struct geni_se, peak_bw_list);
> + if (temp && temp->peak_bw != wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = temp->peak_bw;
> + else if (!temp && wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = 0;
> + }
> +
> + /*
> + * This bw vote is to enable internal QUP core clock as well as to
> + * enable path towards memory.
> + */
> + icc_set_bw(wrapper->icc_path[0], wrapper->cur_avg_bw,
> + wrapper->cur_peak_bw);
> +
> + /*
> + * This is just register configuration path so doesn't need avg bw.
> + * Set only peak bw to enable this path.
> + */
> + icc_set_bw(wrapper->icc_path[1], 0, wrapper->cur_peak_bw);
> +
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> +}
> +EXPORT_SYMBOL(geni_icc_update_bw);
> +
> +/**
> * geni_se_resources_off() - Turn off resources associated with the
> serial
> * engine
> * @se: Pointer to the concerned serial engine.
> @@ -707,6 +783,47 @@ void geni_se_rx_dma_unprep(struct geni_se *se,
> dma_addr_t iova, size_t len)
> }
> EXPORT_SYMBOL(geni_se_rx_dma_unprep);
>
> +/**
> + * geni_interconnect_init() - Request to get interconnect path handle
> + * @se: Pointer to the concerned serial engine.
> + *
> + * This function is used to get interconnect path handle.
> + */
> +int geni_interconnect_init(struct geni_se *se)
> +{
> + struct geni_wrapper *wrapper_rsc;
> +
> + if (unlikely(!se || !se->wrapper))
> + return -EINVAL;
> +
> + wrapper_rsc = se->wrapper;
> +
> + if ((IS_ERR_OR_NULL(wrapper_rsc->icc_path[0]) ||
> + IS_ERR_OR_NULL(wrapper_rsc->icc_path[1]))) {
> +
> + wrapper_rsc->icc_path[0] = of_icc_get(wrapper_rsc->dev,
> + "qup-memory");
> + if (IS_ERR(wrapper_rsc->icc_path[0]))
> + return PTR_ERR(wrapper_rsc->icc_path[0]);
> +
> + wrapper_rsc->icc_path[1] = of_icc_get(wrapper_rsc->dev,
> + "qup-config");
> + if (IS_ERR(wrapper_rsc->icc_path[1])) {
> + icc_put(wrapper_rsc->icc_path[0]);
> + wrapper_rsc->icc_path[0] = NULL;
> + return PTR_ERR(wrapper_rsc->icc_path[1]);
> + }
> +
> + INIT_LIST_HEAD(&wrapper_rsc->peak_bw_list_head);
> + mutex_init(&wrapper_rsc->geni_wrapper_lock);
> + }
> +
> + INIT_LIST_HEAD(&se->peak_bw_list);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(geni_interconnect_init);
> +
> static int geni_se_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -737,6 +854,17 @@ static int geni_se_probe(struct platform_device
> *pdev)
> return devm_of_platform_populate(dev);
> }
>
> +static int geni_se_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct geni_wrapper *wrapper = dev_get_drvdata(dev);
> +
> + icc_put(wrapper->icc_path[0]);
> + icc_put(wrapper->icc_path[1]);
> +
> + return 0;
> +}
> +
> static const struct of_device_id geni_se_dt_match[] = {
> { .compatible = "qcom,geni-se-qup", },
> {}
> @@ -749,6 +877,7 @@ static int geni_se_probe(struct platform_device
> *pdev)
> .of_match_table = geni_se_dt_match,
> },
> .probe = geni_se_probe,
> + .remove = geni_se_remove,
> };
> module_platform_driver(geni_se_driver);
>
> diff --git a/include/linux/qcom-geni-se.h
> b/include/linux/qcom-geni-se.h
> index 3bcd67f..9bf03e9 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -33,6 +33,10 @@ enum geni_se_protocol_type {
> * @clk: Handle to the core serial engine clock
> * @num_clk_levels: Number of valid clock levels in clk_perf_tbl
> * @clk_perf_tbl: Table of clock frequency input to serial engine
> clock
> + * @avg_bw: Average bus bandwidth value for Serial Engine device
> + * @peak_bw: Peak bus bandwidth value for Serial Engine device
> + * @peak_bw_list: List Head of peak bus banwidth list for Serial
> Engine
> + * device
> */
> struct geni_se {
> void __iomem *base;
> @@ -41,6 +45,9 @@ struct geni_se {
> struct clk *clk;
> unsigned int num_clk_levels;
> unsigned long *clk_perf_tbl;
> + u32 avg_bw;
> + u32 peak_bw;
> + struct list_head peak_bw_list;
> };
>
> /* Common SE registers */
> @@ -416,5 +423,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void
> *buf, size_t len,
> void geni_se_tx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t
> len);
>
> void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t
> len);
> +
> +int geni_interconnect_init(struct geni_se *se);
> +
> +void geni_icc_update_bw(struct geni_se *se, bool update);
> #endif
> #endif

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 06:53:40

by Alok Chauhan

Subject: Re: [PATCH 3/6] i2c: i2c-qcom-geni: Add interconnect support

On 2019-01-22 14:43, Peter Rosin wrote:
> On 2019-01-22 07:33, Alok Chauhan wrote:
>> Get the interconnect paths for I2C based Serial Engine device
>> and vote accordingly based on maximum supported I2C frequency.
>>
>> Signed-off-by: Alok Chauhan <[email protected]>
>> ---
>> drivers/i2c/busses/i2c-qcom-geni.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
>> b/drivers/i2c/busses/i2c-qcom-geni.c
>> index db075bc..e8fe63a 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -14,6 +14,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/qcom-geni-se.h>
>> #include <linux/spinlock.h>
>> +#include <linux/interconnect.h>
>
> Please keep the includes sorted. Probably applies for the other patches
> in the series as well?
>
> Cheers,
> Peter

Sure, will do. Thanks for your comment.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 06:55:38

by Alok Chauhan

Subject: Re: [PATCH 5/6] tty: serial: qcom_geni_serial: Add interconnect support

+Mark Brown

On 2019-01-22 12:03, Alok Chauhan wrote:
> Get the interconnect paths for Uart based Serial Engine device
> and vote accordingly based on maximum supported Uart frequency.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c
> b/drivers/tty/serial/qcom_geni_serial.c
> index a72d6d9..e2ea499 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -19,6 +19,7 @@
> #include <linux/slab.h>
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
> +#include <linux/interconnect.h>
>
> /* UART specific GENI registers */
> #define SE_UART_LOOPBACK_CFG 0x22c
> @@ -1348,6 +1349,22 @@ static int qcom_geni_serial_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> + /* Set the bus quota to a reasonable value */
> + port->se.avg_bw = console ? Bps_to_icc(1000) : Bps_to_icc(2500);
> + port->se.peak_bw = Bps_to_icc(76800000);
> + ret = geni_interconnect_init(&port->se);
> + if (ret) {
> + dev_err(&pdev->dev, "interconnect_init failed %d\n", ret);
> + return ret;
> + }
> +
> + /*
> + * Vote for interconnect path early. This has to move as part of
> + * Runtime PM APIs when implemented for better control betwen
> + * console and non-console usecases
> + */
> + geni_icc_update_bw(&port->se, true);
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res)
> return -EINVAL;
> @@ -1385,8 +1402,15 @@ static int __maybe_unused
> qcom_geni_serial_sys_suspend(struct device *dev)
> {
> struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> struct uart_port *uport = &port->uport;
> + int ret;
> +
> + ret = uart_suspend_port(uport->private_data, uport);
> + if (ret)
> + return ret;
> +
> + geni_icc_update_bw(&port->se, false);
>
> - return uart_suspend_port(uport->private_data, uport);
> + return 0;
> }
>
> static int __maybe_unused qcom_geni_serial_sys_resume(struct device
> *dev)
> @@ -1394,6 +1418,7 @@ static int __maybe_unused
> qcom_geni_serial_sys_resume(struct device *dev)
> struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> struct uart_port *uport = &port->uport;
>
> + geni_icc_update_bw(&port->se, true);
> return uart_resume_port(uport->private_data, uport);
> }

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 06:56:19

by Alok Chauhan

Subject: Re: [PATCH 6/6] arm64: dts: sdm845: Add interconnect for GENI QUP

+Mark Brown

On 2019-01-22 12:03, Alok Chauhan wrote:
> Add interconnect ports for GENI QUPs to set bus
> capabilities.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index c27cbd3..fb0a8a7 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -374,6 +374,13 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> +
> + interconnects = <&rsc_hlos MASTER_BLSP_1
> + &rsc_hlos SLAVE_EBI1>,
> + <&rsc_hlos MASTER_APPSS_PROC
> + &rsc_hlos SLAVE_BLSP_1>;
> + interconnect-names = "qup-memory", "qup-config";
> +
> status = "disabled";
>
> i2c0: i2c@880000 {
> @@ -682,6 +689,13 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> +
> + interconnects = <&rsc_hlos MASTER_BLSP_2
> + &rsc_hlos SLAVE_EBI1>,
> + <&rsc_hlos MASTER_APPSS_PROC
> + &rsc_hlos SLAVE_BLSP_2>;
> + interconnect-names = "qup-memory", "qup-config";
> +
> status = "disabled";
>
> i2c8: i2c@a80000 {

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 07:04:19

by Alok Chauhan

Subject: Re: [PATCH 2/6] soc: qcom: Add wrapper to support for Interconnect path

+Mark Brown

On 2019-01-22 12:03, Alok Chauhan wrote:
> Add the wrapper to support for interconnect path voting
> from GENI QUP. This wrapper provides the functionalities
> to individual Serial Engine device to get the interconnect
> path and to vote for bandwidth based on their need.
>
> This wrapper maintains bandwidth votes from each Serial Engine
> and send the aggregated vote from GENI QUP to interconnect
> framework.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 129
> ++++++++++++++++++++++++++++++++++++++++
> include/linux/qcom-geni-se.h | 11 ++++
> 2 files changed, 140 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c
> b/drivers/soc/qcom/qcom-geni-se.c
> index 6b8ef01..1d8dcb1 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -11,6 +11,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/qcom-geni-se.h>
> +#include <linux/interconnect.h>
>
> /**
> * DOC: Overview
> @@ -84,11 +85,21 @@
> * @dev: Device pointer of the QUP wrapper core
> * @base: Base address of this instance of QUP wrapper core
> * @ahb_clks: Handle to the primary & secondary AHB clocks
> + * @icc_path: Array of interconnect path handles
> + * @geni_wrapper_lock: Lock to protect the bus bandwidth request
> + * @cur_avg_bw: Current Bus Average BW request value from GENI QUP
> + * @cur_peak_bw: Current Bus Peak BW request value from GENI QUP
> + * @peak_bw_list_head: Sorted resource list based on peak bus BW
> */
> struct geni_wrapper {
> struct device *dev;
> void __iomem *base;
> struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> + struct icc_path *icc_path[2];
> + struct mutex geni_wrapper_lock;
> + u32 cur_avg_bw;
> + u32 cur_peak_bw;
> + struct list_head peak_bw_list_head;
> };
>
> #define QUP_HW_VER_REG 0x4
> @@ -440,6 +451,71 @@ static void geni_se_clks_off(struct geni_se *se)
> }
>
> /**
> + * geni_icc_update_bw() - Request to update bw vote on an interconnect
> path
> + * @se: Pointer to the concerned serial engine.
> + * @update: Flag to update bw vote.
> + *
> + * This function is used to request set bw vote on interconnect path
> handle.
> + */
> +void geni_icc_update_bw(struct geni_se *se, bool update)
> +{
> + struct geni_se *temp = NULL;
> + struct list_head *ins_list_head;
> + struct geni_wrapper *wrapper;
> +
> + mutex_lock(&se->wrapper->geni_wrapper_lock);
> +
> + wrapper = se->wrapper;
> +
> + if (update) {
> + wrapper->cur_avg_bw += se->avg_bw;
> + ins_list_head = &wrapper->peak_bw_list_head;
> + list_for_each_entry(temp, &wrapper->peak_bw_list_head,
> + peak_bw_list) {
> + if (temp->peak_bw < se->peak_bw)
> + break;
> + ins_list_head = &temp->peak_bw_list;
> + }
> +
> + list_add(&se->peak_bw_list, ins_list_head);
> +
> + if (ins_list_head == &wrapper->peak_bw_list_head)
> + wrapper->cur_peak_bw = se->peak_bw;
> + } else {
> + if (unlikely(list_empty(&se->peak_bw_list))) {
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> + return;
> + }
> +
> + wrapper->cur_avg_bw -= se->avg_bw;
> +
> + list_del_init(&se->peak_bw_list);
> + temp = list_first_entry_or_null(&wrapper->peak_bw_list_head,
> + struct geni_se, peak_bw_list);
> + if (temp && temp->peak_bw != wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = temp->peak_bw;
> + else if (!temp && wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = 0;
> + }
> +
> + /*
> + * This bw vote is to enable internal QUP core clock as well as to
> + * enable path towards memory.
> + */
> + icc_set_bw(wrapper->icc_path[0], wrapper->cur_avg_bw,
> + wrapper->cur_peak_bw);
> +
> + /*
> + * This is just register configuration path so doesn't need avg bw.
> + * Set only peak bw to enable this path.
> + */
> + icc_set_bw(wrapper->icc_path[1], 0, wrapper->cur_peak_bw);
> +
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> +}
> +EXPORT_SYMBOL(geni_icc_update_bw);
> +
> +/**
> * geni_se_resources_off() - Turn off resources associated with the
> serial
> * engine
> * @se: Pointer to the concerned serial engine.
> @@ -707,6 +783,47 @@ void geni_se_rx_dma_unprep(struct geni_se *se,
> dma_addr_t iova, size_t len)
> }
> EXPORT_SYMBOL(geni_se_rx_dma_unprep);
>
> +/**
> + * geni_interconnect_init() - Request to get interconnect path handle
> + * @se: Pointer to the concerned serial engine.
> + *
> + * This function is used to get interconnect path handle.
> + */
> +int geni_interconnect_init(struct geni_se *se)
> +{
> + struct geni_wrapper *wrapper_rsc;
> +
> + if (unlikely(!se || !se->wrapper))
> + return -EINVAL;
> +
> + wrapper_rsc = se->wrapper;
> +
> + if ((IS_ERR_OR_NULL(wrapper_rsc->icc_path[0]) ||
> + IS_ERR_OR_NULL(wrapper_rsc->icc_path[1]))) {
> +
> + wrapper_rsc->icc_path[0] = of_icc_get(wrapper_rsc->dev,
> + "qup-memory");
> + if (IS_ERR(wrapper_rsc->icc_path[0]))
> + return PTR_ERR(wrapper_rsc->icc_path[0]);
> +
> + wrapper_rsc->icc_path[1] = of_icc_get(wrapper_rsc->dev,
> + "qup-config");
> + if (IS_ERR(wrapper_rsc->icc_path[1])) {
> + icc_put(wrapper_rsc->icc_path[0]);
> + wrapper_rsc->icc_path[0] = NULL;
> + return PTR_ERR(wrapper_rsc->icc_path[1]);
> + }
> +
> + INIT_LIST_HEAD(&wrapper_rsc->peak_bw_list_head);
> + mutex_init(&wrapper_rsc->geni_wrapper_lock);
> + }
> +
> + INIT_LIST_HEAD(&se->peak_bw_list);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(geni_interconnect_init);
> +
> static int geni_se_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -737,6 +854,17 @@ static int geni_se_probe(struct platform_device
> *pdev)
> return devm_of_platform_populate(dev);
> }
>
> +static int geni_se_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct geni_wrapper *wrapper = dev_get_drvdata(dev);
> +
> + icc_put(wrapper->icc_path[0]);
> + icc_put(wrapper->icc_path[1]);
> +
> + return 0;
> +}
> +
> static const struct of_device_id geni_se_dt_match[] = {
> { .compatible = "qcom,geni-se-qup", },
> {}
> @@ -749,6 +877,7 @@ static int geni_se_probe(struct platform_device
> *pdev)
> .of_match_table = geni_se_dt_match,
> },
> .probe = geni_se_probe,
> + .remove = geni_se_remove,
> };
> module_platform_driver(geni_se_driver);
>
> diff --git a/include/linux/qcom-geni-se.h
> b/include/linux/qcom-geni-se.h
> index 3bcd67f..9bf03e9 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -33,6 +33,10 @@ enum geni_se_protocol_type {
> * @clk: Handle to the core serial engine clock
> * @num_clk_levels: Number of valid clock levels in clk_perf_tbl
> * @clk_perf_tbl: Table of clock frequency input to serial engine
> clock
> + * @avg_bw: Average bus bandwidth value for Serial Engine device
> + * @peak_bw: Peak bus bandwidth value for Serial Engine device
> + * @peak_bw_list: List Head of peak bus banwidth list for Serial
> Engine
> + * device
> */
> struct geni_se {
> void __iomem *base;
> @@ -41,6 +45,9 @@ struct geni_se {
> struct clk *clk;
> unsigned int num_clk_levels;
> unsigned long *clk_perf_tbl;
> + u32 avg_bw;
> + u32 peak_bw;
> + struct list_head peak_bw_list;
> };
>
> /* Common SE registers */
> @@ -416,5 +423,9 @@ int geni_se_rx_dma_prep(struct geni_se *se, void
> *buf, size_t len,
> void geni_se_tx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t
> len);
>
> void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t
> len);
> +
> +int geni_interconnect_init(struct geni_se *se);
> +
> +void geni_icc_update_bw(struct geni_se *se, bool update);
> #endif
> #endif

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 07:16:58

by Alok Chauhan

Subject: Re: [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support

On 2019-01-23 01:59, Mark Brown wrote:
> On Tue, Jan 22, 2019 at 12:03:34PM +0530, Alok Chauhan wrote:
>
>> Get the interconnect paths for SPI based Serial Engine device
>> and vote accordingly based on maximum supported SPI frequency.
>
> Still not seeing anything except this patch here in my inbox - like I
> said what's the story with dependencies?

I've added linux-spi mailing list to all the patches sent as part of
this series. Not sure why you didn't get other patches. Today I've
explicitly added you in all the patches.
This change series basically enabled interconnect (as per
bindings/interconnect/interconnect.txt) support in GENI QUPs based
drivers so appropriate BW request can be put based on SE usages.
here is my complete change series:
https://lore.kernel.org/lkml/[email protected]/
I hope I answer to your question properly.


--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-23 11:01:47

by Mark Brown

Subject: Re: [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support

On Wed, Jan 23, 2019 at 12:45:30PM +0530, [email protected] wrote:
> On 2019-01-23 01:59, Mark Brown wrote:

> > Still not seeing anything except this patch here in my inbox - like I
> > said what's the story with dependencies?

> I've added linux-spi mailing list to all the patches sent as part of this
> series. Not sure why you didn't get other patches. Today I've explicitly
> added you in all the patches.

You really need to ensure that things end up in people's inboxes -
sending them to a mailing list usually means they'll go to a completely
different folder.

> This change series basically enabled interconnect (as per
> bindings/interconnect/interconnect.txt) support in GENI QUPs based drivers
> so appropriate BW request can be put based on SE usages.
> here is my complete change series: https://lore.kernel.org/lkml/[email protected]/
> I hope I answer to your question properly.

So it looks like there is a dependency on earlier patches in the series,
that's what I needed to know.

Attachments:

2019-01-23 16:06:16

by Mark Brown

Subject: Re: [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support

On Tue, Jan 22, 2019 at 12:03:34PM +0530, Alok Chauhan wrote:
> Get the interconnect paths for SPI based Serial Engine device
> and vote accordingly based on maximum supported SPI frequency.

Acked-by: Mark Brown <[email protected]>

Attachments:

2019-01-23 17:10:42

by Bjorn Andersson

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:

> Add documentation for the interconnect and interconnect-names bindings
> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> index dab7ca9..44d7e02 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> @@ -17,6 +17,12 @@ Required properties if child node exists:
> - #address-cells: Must be <1> for Serial Engine Address
> - #size-cells: Must be <1> for Serial Engine Address Size
> - ranges: Must be present
> +- interconnects: phandle to a interconnect provider. Please refer
> + ../interconnect/interconnect.txt for details.
> + Must be 2 paths corresponding to 2 AXI ports.
> +- interconnect-names: Port names to differentiate between the

s/Port names/Path names/

> + 2 interconnect paths defined with interconnect
> + specifier.

These two names are significant in that they must match what the driver
expects, hence you must actually specify them here.

And as the scope of these strings are local to the QUP node you can omit
"qup" from them, so make them "memory" and "config" (or perhaps iface,
to match the clock naming?).

Regards,
Bjorn

>
> Properties for children:
>
> @@ -67,6 +73,10 @@ Example:
> #size-cells = <1>;
> ranges;
>
> + interconnects = <&qnoc 11 &qnoc 512>,
> + <&qnoc 0 &qnoc 543>;

I presu

> + interconnect-names = "qup-memory", "qup-config";
> +
> i2c0: i2c@a94000 {
> compatible = "qcom,geni-i2c";
> reg = <0xa94000 0x4000>;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

2019-01-23 18:15:14

by Bjorn Andersson

Subject: Re: [PATCH 2/6] soc: qcom: Add wrapper to support for Interconnect path

On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:

> Add the wrapper to support for interconnect path voting
> from GENI QUP. This wrapper provides the functionalities
> to individual Serial Engine device to get the interconnect
> path and to vote for bandwidth based on their need.
>
> This wrapper maintains bandwidth votes from each Serial Engine
> and send the aggregated vote from GENI QUP to interconnect
> framework.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 129 ++++++++++++++++++++++++++++++++++++++++
> include/linux/qcom-geni-se.h | 11 ++++
> 2 files changed, 140 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 6b8ef01..1d8dcb1 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -11,6 +11,7 @@
> #include <linux/pinctrl/consumer.h>
> #include <linux/platform_device.h>
> #include <linux/qcom-geni-se.h>
> +#include <linux/interconnect.h>
>
> /**
> * DOC: Overview
> @@ -84,11 +85,21 @@
> * @dev: Device pointer of the QUP wrapper core
> * @base: Base address of this instance of QUP wrapper core
> * @ahb_clks: Handle to the primary & secondary AHB clocks
> + * @icc_path: Array of interconnect path handles
> + * @geni_wrapper_lock: Lock to protect the bus bandwidth request
> + * @cur_avg_bw: Current Bus Average BW request value from GENI QUP
> + * @cur_peak_bw: Current Bus Peak BW request value from GENI QUP
> + * @peak_bw_list_head: Sorted resource list based on peak bus BW
> */
> struct geni_wrapper {
> struct device *dev;
> void __iomem *base;
> struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> + struct icc_path *icc_path[2];
> + struct mutex geni_wrapper_lock;
> + u32 cur_avg_bw;
> + u32 cur_peak_bw;
> + struct list_head peak_bw_list_head;
> };
>
> #define QUP_HW_VER_REG 0x4
> @@ -440,6 +451,71 @@ static void geni_se_clks_off(struct geni_se *se)
> }
>
> /**
> + * geni_icc_update_bw() - Request to update bw vote on an interconnect path
> + * @se: Pointer to the concerned serial engine.
> + * @update: Flag to update bw vote.

So update = false means "yes, do update, but subtract from the global
sum".


You track the bandwidth request for each "se", so I would recommend that
you turn this parameter into a "enabled" and track that as well for each
se. Then in this function you loop over all the wrapper's se and compute
the sum of the enabled ses.

> + *
> + * This function is used to request set bw vote on interconnect path handle.
> + */
> +void geni_icc_update_bw(struct geni_se *se, bool update)
> +{
> + struct geni_se *temp = NULL;
> + struct list_head *ins_list_head;
> + struct geni_wrapper *wrapper;
> +
> + mutex_lock(&se->wrapper->geni_wrapper_lock);
> +
> + wrapper = se->wrapper;
> +
> + if (update) {
> + wrapper->cur_avg_bw += se->avg_bw;

I find this fragile, as you're relying on the se drivers always doing
the right thing.


That said, all you're doing here is sum up all the SEs bandwidth
requests and send them to the interconnect framework, which will sum
them up. Why don't you just call the interconnect framework from the SE
drivers directly?

You can still define the interconnects property in the wrapper DT node
and have the SEs do of_icc_get() from the parent DT node.

> + ins_list_head = &wrapper->peak_bw_list_head;
> + list_for_each_entry(temp, &wrapper->peak_bw_list_head,
> + peak_bw_list) {
> + if (temp->peak_bw < se->peak_bw)
> + break;
> + ins_list_head = &temp->peak_bw_list;
> + }
> +
> + list_add(&se->peak_bw_list, ins_list_head);
> +
> + if (ins_list_head == &wrapper->peak_bw_list_head)
> + wrapper->cur_peak_bw = se->peak_bw;
> + } else {
> + if (unlikely(list_empty(&se->peak_bw_list))) {
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> + return;
> + }
> +
> + wrapper->cur_avg_bw -= se->avg_bw;
> +
> + list_del_init(&se->peak_bw_list);
> + temp = list_first_entry_or_null(&wrapper->peak_bw_list_head,
> + struct geni_se, peak_bw_list);
> + if (temp && temp->peak_bw != wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = temp->peak_bw;
> + else if (!temp && wrapper->cur_peak_bw)
> + wrapper->cur_peak_bw = 0;
> + }
> +
> + /*
> + * This bw vote is to enable internal QUP core clock as well as to
> + * enable path towards memory.
> + */
> + icc_set_bw(wrapper->icc_path[0], wrapper->cur_avg_bw,
> + wrapper->cur_peak_bw);
> +
> + /*
> + * This is just register configuration path so doesn't need avg bw.
> + * Set only peak bw to enable this path.
> + */
> + icc_set_bw(wrapper->icc_path[1], 0, wrapper->cur_peak_bw);
> +
> + mutex_unlock(&wrapper->geni_wrapper_lock);
> +}
> +EXPORT_SYMBOL(geni_icc_update_bw);
> +
> +/**
> * geni_se_resources_off() - Turn off resources associated with the serial
> * engine
> * @se: Pointer to the concerned serial engine.
> @@ -707,6 +783,47 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t iova, size_t len)
> }
> EXPORT_SYMBOL(geni_se_rx_dma_unprep);
>
> +/**
> + * geni_interconnect_init() - Request to get interconnect path handle
> + * @se: Pointer to the concerned serial engine.
> + *
> + * This function is used to get interconnect path handle.
> + */
> +int geni_interconnect_init(struct geni_se *se)
> +{
> + struct geni_wrapper *wrapper_rsc;
> +
> + if (unlikely(!se || !se->wrapper))
> + return -EINVAL;
> +
> + wrapper_rsc = se->wrapper;
> +
> + if ((IS_ERR_OR_NULL(wrapper_rsc->icc_path[0]) ||
> + IS_ERR_OR_NULL(wrapper_rsc->icc_path[1]))) {

You have a probe function for the wrapper, initialize the interconnects
from there instead. That way you don't have to do this.

> +
> + wrapper_rsc->icc_path[0] = of_icc_get(wrapper_rsc->dev,
> + "qup-memory");
> + if (IS_ERR(wrapper_rsc->icc_path[0]))
> + return PTR_ERR(wrapper_rsc->icc_path[0]);
> +
> + wrapper_rsc->icc_path[1] = of_icc_get(wrapper_rsc->dev,
> + "qup-config");
> + if (IS_ERR(wrapper_rsc->icc_path[1])) {
> + icc_put(wrapper_rsc->icc_path[0]);
> + wrapper_rsc->icc_path[0] = NULL;
> + return PTR_ERR(wrapper_rsc->icc_path[1]);
> + }
> +
> + INIT_LIST_HEAD(&wrapper_rsc->peak_bw_list_head);
> + mutex_init(&wrapper_rsc->geni_wrapper_lock);
> + }
> +
> + INIT_LIST_HEAD(&se->peak_bw_list);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(geni_interconnect_init);
> +

Regards,
Bjorn

2019-01-23 18:36:42

by Georgi Djakov

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

Hi Alok,

Thanks for the patches!

On 1/22/19 08:33, Alok Chauhan wrote:
> Add documentation for the interconnect and interconnect-names bindings

s/interconnect /interconnects /

> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> index dab7ca9..44d7e02 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> @@ -17,6 +17,12 @@ Required properties if child node exists:
> - #address-cells: Must be <1> for Serial Engine Address
> - #size-cells: Must be <1> for Serial Engine Address Size
> - ranges: Must be present
> +- interconnects: phandle to a interconnect provider. Please refer

s/a interconnect/an interconnect/

> + ../interconnect/interconnect.txt for details.
> + Must be 2 paths corresponding to 2 AXI ports.
> +- interconnect-names: Port names to differentiate between the
> + 2 interconnect paths defined with interconnect
> + specifier.
>
> Properties for children:
>
> @@ -67,6 +73,10 @@ Example:
> #size-cells = <1>;
> ranges;
>
> + interconnects = <&qnoc 11 &qnoc 512>,
> + <&qnoc 0 &qnoc 543>;

Please take a snippet from your patch 6/6 and put it here instead of the
hard-coded integers above.

> + interconnect-names = "qup-memory", "qup-config";
> +
> i2c0: i2c@a94000 {
> compatible = "qcom,geni-i2c";
> reg = <0xa94000 0x4000>;
>

When you post a new version, please change the subject of the patch
series to PATCH v2, PATCH v3 etc, in order to be able to distinguish
between different versions.

Thanks,
Georgi

2019-01-23 18:41:49

by Georgi Djakov

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

Hi,

On 1/23/19 19:07, Bjorn Andersson wrote:
> On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:
>
>> Add documentation for the interconnect and interconnect-names bindings
>> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>>
>> Signed-off-by: Alok Chauhan <[email protected]>
>> ---
>> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> index dab7ca9..44d7e02 100644
>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> @@ -17,6 +17,12 @@ Required properties if child node exists:
>> - #address-cells: Must be <1> for Serial Engine Address
>> - #size-cells: Must be <1> for Serial Engine Address Size
>> - ranges: Must be present
>> +- interconnects: phandle to a interconnect provider. Please refer
>> + ../interconnect/interconnect.txt for details.
>> + Must be 2 paths corresponding to 2 AXI ports.
>> +- interconnect-names: Port names to differentiate between the
>
> s/Port names/Path names/
>
>> + 2 interconnect paths defined with interconnect
>> + specifier.
>
> These two names are significant in that they must match what the driver
> expects, hence you must actually specify them here.
>
> And as the scope of these strings are local to the QUP node you can omit
> "qup" from them, so make them "memory" and "config" (or perhaps iface,
> to match the clock naming?).

Actually there was a discussion in the past where we decided include
both the src and dst endpoint names in this property so that there is
some symmetry with the "interconnects" property. It would be nice to be
consistent across different drivers at least for now.
If we want to denote the master and slave ports here, my two cents would
be for "qup-mem" and "cpu-qup" or something similar?

Thanks,
Georgi

2019-01-24 01:19:38

by Evan Green

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

On Mon, Jan 21, 2019 at 10:34 PM Alok Chauhan <[email protected]> wrote:
>
> Add documentation for the interconnect and interconnect-names bindings
> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> index dab7ca9..44d7e02 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> @@ -17,6 +17,12 @@ Required properties if child node exists:
> - #address-cells: Must be <1> for Serial Engine Address
> - #size-cells: Must be <1> for Serial Engine Address Size
> - ranges: Must be present
> +- interconnects: phandle to a interconnect provider. Please refer
> + ../interconnect/interconnect.txt for details.

This path to the interconnect documentation is not correct.

2019-01-24 01:20:57

by Evan Green

Subject: Re: [PATCH 3/6] i2c: i2c-qcom-geni: Add interconnect support

On Mon, Jan 21, 2019 at 10:34 PM Alok Chauhan <[email protected]> wrote:
>
> Get the interconnect paths for I2C based Serial Engine device
> and vote accordingly based on maximum supported I2C frequency.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index db075bc..e8fe63a 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -14,6 +14,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/qcom-geni-se.h>
> #include <linux/spinlock.h>
> +#include <linux/interconnect.h>
>
> #define SE_I2C_TX_TRANS_LEN 0x26c
> #define SE_I2C_RX_TRANS_LEN 0x270
> @@ -508,6 +509,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
> return ret;
> }
>
> + /* Set the bus quota to a reasonable value */
> + gi2c->se.avg_bw = Bps_to_icc(1000);
> + gi2c->se.peak_bw = Bps_to_icc(76800000);

Can you compute a real bandwidth based on the i2c bus speed, rather
than some max vote?

2019-01-24 01:23:28

by Evan Green

Subject: Re: [PATCH 4/6] spi: spi-geni-qcom: Add interconnect support

On Mon, Jan 21, 2019 at 10:34 PM Alok Chauhan <[email protected]> wrote:
>
> Get the interconnect paths for SPI based Serial Engine device
> and vote accordingly based on maximum supported SPI frequency.
>
> Signed-off-by: Alok Chauhan <[email protected]>
> ---
> drivers/spi/spi-geni-qcom.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index fdb7cb88..7bbbe9d 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -12,6 +12,7 @@
> #include <linux/qcom-geni-se.h>
> #include <linux/spi/spi.h>
> #include <linux/spinlock.h>
> +#include <linux/interconnect.h>

Sorted includes please, as mentioned by others.

>
> /* SPI SE specific registers and respective register fields */
> #define SE_SPI_CPHA 0x224
> @@ -589,6 +590,15 @@ static int spi_geni_probe(struct platform_device *pdev)
> spin_lock_init(&mas->lock);
> pm_runtime_enable(&pdev->dev);
>
> + /* Set the bus quota to a reasonable value */
> + mas->se.avg_bw = Bps_to_icc(2500);
> + mas->se.peak_bw = Bps_to_icc(200000000);

Like the i2c case, can we compute real value based on clock speed?

2019-01-30 23:08:26

by Alok Chauhan

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

On 2019-01-23 10:35, Georgi Djakov wrote:
> Hi Alok,
>
> Thanks for the patches!
>
> On 1/22/19 08:33, Alok Chauhan wrote:
>> Add documentation for the interconnect and interconnect-names bindings
>
> s/interconnect /interconnects /
>
>> for the GENI QUP as detailed by
>> bindings/interconnect/interconnect.txt.
>>
>> Signed-off-by: Alok Chauhan <[email protected]>
>> ---
>> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10
>> ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> index dab7ca9..44d7e02 100644
>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> @@ -17,6 +17,12 @@ Required properties if child node exists:
>> - #address-cells: Must be <1> for Serial Engine Address
>> - #size-cells: Must be <1> for Serial Engine Address Size
>> - ranges: Must be present
>> +- interconnects: phandle to a interconnect provider. Please refer
>
> s/a interconnect/an interconnect/
sure, will do.
>
>> + ../interconnect/interconnect.txt for details.
>> + Must be 2 paths corresponding to 2 AXI ports.
>> +- interconnect-names: Port names to differentiate between the
>> + 2 interconnect paths defined with interconnect
>> + specifier.
>>
>> Properties for children:
>>
>> @@ -67,6 +73,10 @@ Example:
>> #size-cells = <1>;
>> ranges;
>>
>> + interconnects = <&qnoc 11 &qnoc 512>,
>> + <&qnoc 0 &qnoc 543>;
>
> Please take a snippet from your patch 6/6 and put it here instead of
> the
> hard-coded integers above.
sure
>
>> + interconnect-names = "qup-memory", "qup-config";
>> +
>> i2c0: i2c@a94000 {
>> compatible = "qcom,geni-i2c";
>> reg = <0xa94000 0x4000>;
>>
>
> When you post a new version, please change the subject of the patch
> series to PATCH v2, PATCH v3 etc, in order to be able to distinguish
> between different versions.
sure, will do this.
>
> Thanks,
> Georgi

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-01-30 23:09:40

by Alok Chauhan

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

On 2019-01-23 17:10, Evan Green wrote:
> On Mon, Jan 21, 2019 at 10:34 PM Alok Chauhan <[email protected]>
> wrote:
>>
>> Add documentation for the interconnect and interconnect-names bindings
>> for the GENI QUP as detailed by
>> bindings/interconnect/interconnect.txt.
>>
>> Signed-off-by: Alok Chauhan <[email protected]>
>> ---
>> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10
>> ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> index dab7ca9..44d7e02 100644
>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>> @@ -17,6 +17,12 @@ Required properties if child node exists:
>> - #address-cells: Must be <1> for Serial Engine Address
>> - #size-cells: Must be <1> for Serial Engine Address
>> Size
>> - ranges: Must be present
>> +- interconnects: phandle to a interconnect provider. Please
>> refer
>> + ../interconnect/interconnect.txt for details.
>
> This path to the interconnect documentation is not correct.
sorry, i will correct this in next patch.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,a Linux Foundation Collaborative Project

2019-02-23 00:27:37

by Rob Herring (Arm)

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

On Wed, Jan 23, 2019 at 08:41:20PM +0200, Georgi Djakov wrote:
> Hi,
>
> On 1/23/19 19:07, Bjorn Andersson wrote:
> > On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:
> >
> >> Add documentation for the interconnect and interconnect-names bindings
> >> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
> >>
> >> Signed-off-by: Alok Chauhan <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >> index dab7ca9..44d7e02 100644
> >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >> @@ -17,6 +17,12 @@ Required properties if child node exists:
> >> - #address-cells: Must be <1> for Serial Engine Address
> >> - #size-cells: Must be <1> for Serial Engine Address Size
> >> - ranges: Must be present
> >> +- interconnects: phandle to a interconnect provider. Please refer
> >> + ../interconnect/interconnect.txt for details.
> >> + Must be 2 paths corresponding to 2 AXI ports.
> >> +- interconnect-names: Port names to differentiate between the
> >
> > s/Port names/Path names/
> >
> >> + 2 interconnect paths defined with interconnect
> >> + specifier.
> >
> > These two names are significant in that they must match what the driver
> > expects, hence you must actually specify them here.
> >
> > And as the scope of these strings are local to the QUP node you can omit
> > "qup" from them, so make them "memory" and "config" (or perhaps iface,
> > to match the clock naming?).
>
> Actually there was a discussion in the past where we decided include
> both the src and dst endpoint names in this property so that there is
> some symmetry with the "interconnects" property. It would be nice to be
> consistent across different drivers at least for now.
> If we want to denote the master and slave ports here, my two cents would
> be for "qup-mem" and "cpu-qup" or something similar?

Well, there's a proposal from Maxime to add 'dma-memory' or something.
You all need to sort this out.

I assume config or cpu-qup is for register access? Why is this needed?
That should get described thru the DT tree. The interconnect stuff was
supposed to be for the non-cpu centric view (i.e. DMA masters). Maybe
it's fine, but that's not my initial reaction.

Rob

2019-02-25 17:40:27

by Georgi Djakov

Subject: Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP

On 2/23/19 02:26, Rob Herring wrote:
> On Wed, Jan 23, 2019 at 08:41:20PM +0200, Georgi Djakov wrote:
>> Hi,
>>
>> On 1/23/19 19:07, Bjorn Andersson wrote:
>>> On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:
>>>
>>>> Add documentation for the interconnect and interconnect-names bindings
>>>> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>>>>
>>>> Signed-off-by: Alok Chauhan <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>>>> index dab7ca9..44d7e02 100644
>>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>>>> @@ -17,6 +17,12 @@ Required properties if child node exists:
>>>> - #address-cells: Must be <1> for Serial Engine Address
>>>> - #size-cells: Must be <1> for Serial Engine Address Size
>>>> - ranges: Must be present
>>>> +- interconnects: phandle to a interconnect provider. Please refer
>>>> + ../interconnect/interconnect.txt for details.
>>>> + Must be 2 paths corresponding to 2 AXI ports.
>>>> +- interconnect-names: Port names to differentiate between the
>>>
>>> s/Port names/Path names/
>>>
>>>> + 2 interconnect paths defined with interconnect
>>>> + specifier.
>>>
>>> These two names are significant in that they must match what the driver
>>> expects, hence you must actually specify them here.
>>>
>>> And as the scope of these strings are local to the QUP node you can omit
>>> "qup" from them, so make them "memory" and "config" (or perhaps iface,
>>> to match the clock naming?).
>>
>> Actually there was a discussion in the past where we decided include
>> both the src and dst endpoint names in this property so that there is
>> some symmetry with the "interconnects" property. It would be nice to be
>> consistent across different drivers at least for now.
>> If we want to denote the master and slave ports here, my two cents would
>> be for "qup-mem" and "cpu-qup" or something similar?
>
> Well, there's a proposal from Maxime to add 'dma-memory' or something.
> You all need to sort this out.

Agree. I haven't commented on the latest MBUS patches yet. Meanwhile i
am trying to get a better understanding of the hardware. In general if
there are no better suggestions, i am fine with adding a specific name
to handle this kind of dma transfers.

> I assume config or cpu-qup is for register access? Why is this needed?
> That should get described thru the DT tree. The interconnect stuff was
> supposed to be for the non-cpu centric view (i.e. DMA masters). Maybe
> it's fine, but that's not my initial reaction.

Yes, cpu-qup should be for register access. Alok, please correct me if i
am wrong, but I believe this is a separate bus used only for
configuration access to some HW blocks. This bus might be disabled by
default and we may need to request some bandwidth in order to enable it.
It's described as a separate path in DT and we are trying to give it a
meaningful name.

Thanks,
Georgi