Re: [PATCH V8 1/1] i2c: i2c-qcom-geni: Add shutdown callback for i2c
On 1/8/2021 8:35 PM, Roja Rani Yarubandi wrote:
If the hardware is still accessing memory after SMMU translation
is disabled (as part of smmu shutdown callback), then the
IOVAs (I/O virtual address) which it was using will go on the bus
as the physical addresses which will result in unknown crashes
like NoC/interconnect errors.
So, implement shutdown callback to i2c driver to stop on-going transfer
and unmap DMA mappings during system "reboot" or "shutdown".
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C
controller")
Signed-off-by: Roja Rani Yarubandi
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH V7 1/2] i2c: i2c-qcom-geni: Store DMA mapping data in geni_i2c_dev struct
On 12/21/2020 6:08 PM, Roja Rani Yarubandi wrote: Store DMA mapping data in geni_i2c_dev struct to enhance DMA mapping data scope. For example during shutdown callback to unmap DMA mapping, this stored DMA mapping data can be used to call geni_se_tx_dma_unprep and geni_se_rx_dma_unprep functions. Add two helper functions geni_i2c_rx_msg_cleanup and geni_i2c_tx_msg_cleanup to unwrap the things after rx/tx FIFO/DMA transfers, so that the same can be used in geni_i2c_stop_xfer() function during shutdown callback. Signed-off-by: Roja Rani Yarubandi Reviewed-by: Akash Asthana -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH V7 2/2] i2c: i2c-qcom-geni: Add shutdown callback for i2c
On 12/21/2020 6:08 PM, Roja Rani Yarubandi wrote:
If the hardware is still accessing memory after SMMU translation
is disabled (as part of smmu shutdown callback), then the
IOVAs (I/O virtual address) which it was using will go on the bus
as the physical addresses which will result in unknown crashes
like NoC/interconnect errors.
So, implement shutdown callback to i2c driver to stop on-going transfer
and unmap DMA mappings during system "reboot" or "shutdown".
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C
controller")
Signed-off-by: Roja Rani Yarubandi
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [RESEND PATCH V6 1/2] i2c: i2c-qcom-geni: Store DMA mapping data in geni_i2c_dev struct
Hi Roja,
On 12/3/2020 4:01 PM, Roja Rani Yarubandi wrote:
Store DMA mapping data in geni_i2c_dev struct to enhance DMA mapping
data scope. For example during shutdown callback to unmap DMA mapping,
this stored DMA mapping data can be used to call geni_se_tx_dma_unprep
and geni_se_rx_dma_unprep functions.
Add two helper functions geni_i2c_rx_msg_cleanup and
geni_i2c_tx_msg_cleanup to unwrap the things after rx/tx FIFO/DMA
transfers, so that the same can be used in geni_i2c_stop_xfer()
function during shutdown callback.
Signed-off-by: Roja Rani Yarubandi
---
Changes in V5:
- As per Stephen's comments separated this patch from shutdown
callback patch, gi2c->cur = NULL is not removed from
geni_i2c_abort_xfer(), and made a copy of gi2c->cur and passed
to cleanup functions.
Changes in V6:
- Added spin_lock/unlock in geni_i2c_rx_msg_cleanup() and
geni_i2c_tx_msg_cleanup() functions.
drivers/i2c/busses/i2c-qcom-geni.c | 69 +++---
1 file changed, 53 insertions(+), 16 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index dce75b85253c..bfbc80f65006 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -86,6 +86,9 @@ struct geni_i2c_dev {
u32 clk_freq_out;
const struct geni_i2c_clk_fld *clk_fld;
int suspended;
+ void *dma_buf;
+ size_t xfer_len;
+ dma_addr_t dma_addr;
};
struct geni_i2c_err_log {
@@ -348,14 +351,49 @@ static void geni_i2c_tx_fsm_rst(struct geni_i2c_dev *gi2c)
dev_err(gi2c->se.dev, "Timeout resetting TX_FSM\n");
}
+static void geni_i2c_rx_msg_cleanup(struct geni_i2c_dev *gi2c,
+struct i2c_msg *cur)
+{
+ struct geni_se *se = >se;
+ unsigned long flags;
+
+ spin_lock_irqsave(>lock, flags);
+ gi2c->cur_rd = 0;
+ if (gi2c->dma_buf) {
+ if (gi2c->err)
+ geni_i2c_rx_fsm_rst(gi2c);
Which race we are trying to avoid here by holding spinlock?
We cannot call any sleeping API by holding spinlock, geni_i2c_rx_fsm_rst
calls *wait-for-completion*, which is a sleeping call.
+ geni_se_rx_dma_unprep(se, gi2c->dma_addr, gi2c->xfer_len);
+ i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err);
+ }
+ spin_unlock_irqrestore(>lock, flags);
+}
+
+static void geni_i2c_tx_msg_cleanup(struct geni_i2c_dev *gi2c,
+struct i2c_msg *cur)
+{
+ struct geni_se *se = >se;
+ unsigned long flags;
+
+ spin_lock_irqsave(>lock, flags);
+ gi2c->cur_wr = 0;
+ if (gi2c->dma_buf) {
+ if (gi2c->err)
+ geni_i2c_tx_fsm_rst(gi2c);
Same here
Regards,
Akash
+ geni_se_tx_dma_unprep(se, gi2c->dma_addr, gi2c->xfer_len);
+ i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err);
+ }
+ spin_unlock_irqrestore(>lock, flags);
+}
+
static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
u32 m_param)
{
- dma_addr_t rx_dma;
+ dma_addr_t rx_dma = 0;
unsigned long time_left;
void *dma_buf = NULL;
struct geni_se *se = >se;
size_t len = msg->len;
+ struct i2c_msg *cur;
if (!of_machine_is_compatible("lenovo,yoga-c630"))
dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
@@ -372,19 +410,18 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c,
struct i2c_msg *msg,
geni_se_select_mode(se, GENI_SE_FIFO);
i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
dma_buf = NULL;
+ } else {
+ gi2c->xfer_len = len;
+ gi2c->dma_addr = rx_dma;
+ gi2c->dma_buf = dma_buf;
}
+ cur = gi2c->cur;
time_left = wait_for_completion_timeout(>done, XFER_TIMEOUT);
if (!time_left)
geni_i2c_abort_xfer(gi2c);
- gi2c->cur_rd = 0;
- if (dma_buf) {
- if (gi2c->err)
- geni_i2c_rx_fsm_rst(gi2c);
- geni_se_rx_dma_unprep(se, rx_dma, len);
- i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err);
- }
+ geni_i2c_rx_msg_cleanup(gi2c, cur);
return gi2c->err;
}
@@ -392,11 +429,12 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c,
struct i2c_msg *msg,
static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
u32 m_param)
{
- dma_addr_t tx_dma;
+ dma_addr_t tx_dma = 0;
unsigned long time_left;
void *dma_buf = NULL;
struct geni_se *se = >se;
size_t len = msg->len;
+ struct i2c_msg *cur;
if (!of_machine_is_compatible("lenovo,yoga-c630"))
dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
@@ -413,22 +451,21 @@ static
Re: [PATCH 3/3] Serial: Separate out earlycon support
Hi Greg, On 12/7/2020 3:13 PM, Greg KH wrote: On Mon, Dec 07, 2020 at 02:17:27PM +0530, Akash Asthana wrote: Separate out earlycon support from serial driver and remove it's dependency on QUP wrapper driver. This enable us to manage earlycon independently and we can re-use the same earlycon driver for android project which currently uses downstream version of QUP drivers. Signed-off-by: Akash Asthana --- drivers/tty/serial/Kconfig | 9 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/qcom_geni_earlycon.c | 649 drivers/tty/serial/qcom_geni_serial.c | 97 - 4 files changed, 659 insertions(+), 97 deletions(-) create mode 100644 drivers/tty/serial/qcom_geni_earlycon.c Nit, your subject line shoudl say somewhere that this is the qcom earlycon driver/support, not "earlycon in general". Thanks for feedback, I will take care of it in next post. thanks, greg k-h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH 3/3] Serial: Separate out earlycon support
Hi Greg, On 12/7/2020 2:41 PM, Greg KH wrote: On Mon, Dec 07, 2020 at 02:17:27PM +0530, Akash Asthana wrote: Separate out earlycon support from serial driver and remove it's dependency on QUP wrapper driver. This enable us to manage earlycon independently and we can re-use the same earlycon driver for android project which currently uses downstream version of QUP drivers. What do you mean by "downstream" here? Signed-off-by: Akash Asthana --- drivers/tty/serial/Kconfig | 9 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/qcom_geni_earlycon.c | 649 drivers/tty/serial/qcom_geni_serial.c | 97 - So you are replacing 97 lines of code with 649 lines? How is this benefiting anyone? confused, We have 2 versions of QUP driver, upstream version(Present in linus tree, mostly used for chromium project) and downstream version(belong to vendor part of code in GKI design, used for all the other project). There is need to enable geni earlycon in Google provided boot image for GKI to facilitate the debug until real console(belong to vendor code) is up. Currently it won't be possible because geni earlycon cannot be enabled independently, it depends on upstream QUP wrapper driver (soc/qcom/qcom-geni-se.c) and upstream serial driver(serial/qcom_geni_serial.c). With this patch I am trying to break any dependency btw earlycon hook and QUP kernel drivers, so it can be managed independently. Regards, Akash -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH 3/3] Serial: Separate out earlycon support
Separate out earlycon support from serial driver and remove it's dependency on QUP wrapper driver. This enable us to manage earlycon independently and we can re-use the same earlycon driver for android project which currently uses downstream version of QUP drivers. Signed-off-by: Akash Asthana --- drivers/tty/serial/Kconfig | 9 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/qcom_geni_earlycon.c | 649 drivers/tty/serial/qcom_geni_serial.c | 97 - 4 files changed, 659 insertions(+), 97 deletions(-) create mode 100644 drivers/tty/serial/qcom_geni_earlycon.c diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 3409e0b..393a017 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -957,6 +957,15 @@ config SERIAL_QCOM_GENI depends on QCOM_GENI_SE select SERIAL_CORE +config SERIAL_QCOM_GENI_EARLYCON + bool "QCOM GENI Early Console support" + select SERIAL_CORE + select SERIAL_CORE_CONSOLE + select SERIAL_EARLYCON + help + Serial early console driver for Qualcomm Technologies Inc's GENI + based QUP hardware. + config SERIAL_QCOM_GENI_CONSOLE bool "QCOM GENI Serial Console support" depends on SERIAL_QCOM_GENI diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index b85d53f..4f9c318 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_SERIAL_VR41XX) += vr41xx_siu.o obj-$(CONFIG_SERIAL_ATMEL) += atmel_serial.o obj-$(CONFIG_SERIAL_UARTLITE) += uartlite.o obj-$(CONFIG_SERIAL_MSM) += msm_serial.o +obj-$(CONFIG_SERIAL_QCOM_GENI_EARLYCON) += qcom_geni_earlycon.o obj-$(CONFIG_SERIAL_QCOM_GENI) += qcom_geni_serial.o obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o diff --git a/drivers/tty/serial/qcom_geni_earlycon.c b/drivers/tty/serial/qcom_geni_earlycon.c new file mode 100644 index 000..847cc7b --- /dev/null +++ b/drivers/tty/serial/qcom_geni_earlycon.c @@ -0,0 +1,649 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2020, The Linux foundation. All rights reserved. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Common SE registers */ +#define GENI_FORCE_DEFAULT_REG 0x20 +#define GENI_FW_REVISION_RO0x68 +#define SE_GENI_DMA_MODE_EN0x258 +#define SE_GENI_M_CMD0 0x600 +#define SE_GENI_M_CMD_CTRL_REG 0x604 +#define SE_GENI_M_IRQ_STATUS 0x610 +#define SE_GENI_M_IRQ_EN 0x614 +#define SE_GENI_M_IRQ_CLEAR0x618 +#define SE_GENI_S_CMD0 0x630 +#define SE_GENI_S_CMD_CTRL_REG 0x634 +#define SE_GENI_S_IRQ_STATUS 0x640 +#define SE_GENI_S_IRQ_EN 0x644 +#define SE_GENI_S_IRQ_CLEAR0x648 +#define SE_GENI_TX_FIFOn 0x700 +#define SE_GENI_RX_FIFOn 0x780 +#define SE_GENI_TX_FIFO_STATUS 0x800 +#define SE_GENI_RX_FIFO_STATUS 0x804 +#define SE_GENI_TX_WATERMARK_REG 0x80c +#define SE_GENI_RX_WATERMARK_REG 0x810 +#define SE_GENI_RX_RFR_WATERMARK_REG 0x814 +#define SE_DMA_TX_IRQ_CLR 0xc44 +#define SE_DMA_RX_IRQ_CLR 0xd44 + +/* GENI_FORCE_DEFAULT_REG fields */ +#define FORCE_DEFAULT BIT(0) + +/* GENI_FW_REVISION_RO fields */ +#define FW_REV_PROTOCOL_MSKGENMASK(15, 8) +#define FW_REV_PROTOCOL_SHFT 8 + +/* SE_GENI_DMA_MODE_EN */ +#define GENI_DMA_MODE_EN BIT(0) + +/* GENI_M_CMD0 fields */ +#define M_OPCODE_MSK GENMASK(31, 27) +#define M_OPCODE_SHFT 27 +#define M_PARAMS_MSK GENMASK(26, 0) + +/* GENI_M_CMD_CTRL_REG */ +#define M_GENI_CMD_CANCEL BIT(2) +#define M_GENI_CMD_ABORT BIT(1) +#define M_GENI_DISABLE BIT(0) + +/* GENI_S_CMD0 fields */ +#define S_OPCODE_MSK GENMASK(31, 27) +#define S_OPCODE_SHFT 27 +#define S_PARAMS_MSK GENMASK(26, 0) + +/* GENI_S_CMD_CTRL_REG */ +#define S_GENI_CMD_CANCEL BIT(2) +#define S_GENI_CMD_ABORT BIT(1) +#define S_GENI_DISABLE BIT(0) + +/* GENI_M_IRQ_EN fields */ +#define M_CMD_DONE_EN BIT(0) +#define M_CMD_OVERRUN_EN BIT(1) +#define M_ILLEGAL_CMD_EN BIT(2) +#define M_CMD_FAILURE_EN BIT(3) +#define M_CMD_CANCEL_ENBIT(4) +#define M_CMD_ABORT_EN BIT(5) +#define M_TIMESTAMP_EN BIT(6) +#define M_RX_IRQ_ENBIT(7) +#define M_GP_SYNC_IRQ_0_EN BIT(8) +#define M_GP_IRQ_0_EN BIT(9) +#define M_GP_IRQ_1_EN BIT(10) +#define M_GP_IRQ_2_EN
[PATCH 1/3] soc: qcom-geni-se: Cleanup the code to remove proxy votes
ICC core and platforms drivers supports sync_state feature, which
ensures that the default ICC BW votes from the bootloader is not
removed until all it's consumers are probes.
The proxy votes were needed in case other QUP child drivers
I2C, SPI probes before UART, they can turn off the QUP-CORE clock
which is shared resources for all QUP driver, this causes unclocked
access to HW from earlycon.
Given above support from ICC there is no longer need to maintain
proxy votes on QUP-CORE ICC node from QUP wrapper driver for early
console usecase, the default votes won't be removed until real
console is probed.
Signed-off-by: Akash Asthana
---
drivers/soc/qcom/qcom-geni-se.c | 74 ---
drivers/tty/serial/qcom_geni_serial.c | 7
include/linux/qcom-geni-se.h | 2 -
3 files changed, 83 deletions(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index f42954e..1fd29f9 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -3,7 +3,6 @@
#include
#include
-#include
#include
#include
#include
@@ -92,14 +91,11 @@ struct geni_wrapper {
struct device *dev;
void __iomem *base;
struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
- struct geni_icc_path to_core;
};
static const char * const icc_path_names[] = {"qup-core", "qup-config",
"qup-memory"};
-static struct geni_wrapper *earlycon_wrapper;
-
#define QUP_HW_VER_REG 0x4
/* Common SE registers */
@@ -843,44 +839,11 @@ int geni_icc_disable(struct geni_se *se)
}
EXPORT_SYMBOL(geni_icc_disable);
-void geni_remove_earlycon_icc_vote(void)
-{
- struct platform_device *pdev;
- struct geni_wrapper *wrapper;
- struct device_node *parent;
- struct device_node *child;
-
- if (!earlycon_wrapper)
- return;
-
- wrapper = earlycon_wrapper;
- parent = of_get_next_parent(wrapper->dev->of_node);
- for_each_child_of_node(parent, child) {
- if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
- continue;
-
- pdev = of_find_device_by_node(child);
- if (!pdev)
- continue;
-
- wrapper = platform_get_drvdata(pdev);
- icc_put(wrapper->to_core.path);
- wrapper->to_core.path = NULL;
-
- }
- of_node_put(parent);
-
- earlycon_wrapper = NULL;
-}
-EXPORT_SYMBOL(geni_remove_earlycon_icc_vote);
-
static int geni_se_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct resource *res;
struct geni_wrapper *wrapper;
- struct console __maybe_unused *bcon;
- bool __maybe_unused has_earlycon = false;
int ret;
wrapper = devm_kzalloc(dev, sizeof(*wrapper), GFP_KERNEL);
@@ -903,43 +866,6 @@ static int geni_se_probe(struct platform_device *pdev)
}
}
-#ifdef CONFIG_SERIAL_EARLYCON
- for_each_console(bcon) {
- if (!strcmp(bcon->name, "qcom_geni")) {
- has_earlycon = true;
- break;
- }
- }
- if (!has_earlycon)
- goto exit;
-
- wrapper->to_core.path = devm_of_icc_get(dev, "qup-core");
- if (IS_ERR(wrapper->to_core.path))
- return PTR_ERR(wrapper->to_core.path);
- /*
-* Put minmal BW request on core clocks on behalf of early console.
-* The vote will be removed earlycon exit function.
-*
-* Note: We are putting vote on each QUP wrapper instead only to which
-* earlycon is connected because QUP core clock of different wrapper
-* share same voltage domain. If core1 is put to 0, then core2 will
-* also run at 0, if not voted. Default ICC vote will be removed ASA
-* we touch any of the core clock.
-* core1 = core2 = max(core1, core2)
-*/
- ret = icc_set_bw(wrapper->to_core.path, GENI_DEFAULT_BW,
- GENI_DEFAULT_BW);
- if (ret) {
- dev_err(>dev, "%s: ICC BW voting failed for core: %d\n",
- __func__, ret);
- return ret;
- }
-
- if (of_get_compatible_child(pdev->dev.of_node, "qcom,geni-debug-uart"))
- earlycon_wrapper = wrapper;
- of_node_put(pdev->dev.of_node);
-exit:
-#endif
dev_set_drvdata(dev, wrapper);
dev_dbg(dev, "GENI SE Driver probed\n");
return devm_of_platform_populate(dev);
diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
index 291649f..0d85b55 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drive
[PATCH 2/3] arm64: dts: qcom: sc7180: Remove QUP-CORE ICC path
We had introduced the QUP-CORE ICC path to put proxy votes from
QUP wrapper on behalf of earlycon, if other users of QUP-CORE turn
off this clock before the real console is probed, unclocked access
to HW was seen from earlycon.
With ICC sync state support proxy votes are no longer need as ICC
will ensure that the default bootloader votes are not removed until
all it's consumer are probed.
We can safely remove ICC path for QUP-CORE clock from QUP wrapper
device.
Signed-off-by: Akash Asthana
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 4
1 file changed, 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 22b832f..cd09b67 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -763,8 +763,6 @@
#size-cells = <2>;
ranges;
iommus = <_smmu 0x43 0x0>;
- interconnects = <_virt MASTER_QUP_CORE_0 0
_virt SLAVE_QUP_CORE_0 0>;
- interconnect-names = "qup-core";
status = "disabled";
i2c0: i2c@88 {
@@ -1054,8 +1052,6 @@
#size-cells = <2>;
ranges;
iommus = <_smmu 0x4c3 0x0>;
- interconnects = <_virt MASTER_QUP_CORE_1 0
_virt SLAVE_QUP_CORE_1 0>;
- interconnect-names = "qup-core";
status = "disabled";
i2c6: i2c@a8 {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH 0/3] Separate out earlycon
Patch 3/3 depends on patch 1/3, Greg would it be possible to ack patch 3/3. So it could land via QCOM tree. Akash Asthana (3): soc: qcom-geni-se: Cleanup the code to remove proxy votes arm64: dts: qcom: sc7180: Remove QUP-CORE ICC path Serial: Separate out earlycon support arch/arm64/boot/dts/qcom/sc7180.dtsi| 4 - drivers/soc/qcom/qcom-geni-se.c | 74 drivers/tty/serial/Kconfig | 9 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/qcom_geni_earlycon.c | 649 drivers/tty/serial/qcom_geni_serial.c | 104 - include/linux/qcom-geni-se.h| 2 - 7 files changed, 659 insertions(+), 184 deletions(-) create mode 100644 drivers/tty/serial/qcom_geni_earlycon.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] Revert "i2c: qcom-geni: Disable DMA processing on the Lenovo Yoga C630"
On 11/25/2020 12:27 AM, Bjorn Andersson wrote:
A combination of recent bug fixes by Doug Anderson and the proper
definition of iommu streams means that this hack is no longer needed.
Let's clean up the code by reverting '127068abe85b ("i2c: qcom-geni:
Disable DMA processing on the Lenovo Yoga C630")'.
Signed-off-by: Bjorn Andersson
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 5/5] i2c: geni: sdm845: dont perform DMA for OnePlus 6 devices
On 11/12/2020 9:52 PM, Caleb Connolly wrote: The OnePlus 6/T has the same issue as the Yoga c630 causing a crash when DMA is used for i2c, so disable it. https://patchwork.kernel.org/patch/11133827/ Signed-off-by: Caleb Connolly Reviewed-by : Akash Asthana -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] interconnect: qcom: Ensure that the floor bandwidth value is enforced
On 10/21/2020 9:29 PM, Georgi Djakov wrote:
Take into account the initial bandwidth from the framework and update
the internal sum and max values before committing if needed. This will
ensure that the floor bandwidth values are enforced until the providers
get into sync state.
Fixes: 7d3b0b0d8184 ("interconnect: qcom: Use icc_sync_state")
Signed-off-by: Georgi Djakov
Thanks Georgi,
I removed proxy ICC BW votes for earlycon driver "qcom_geni" introduced by
patch [1], trogdor chromium board booted up fine, which use to crash without this patch.
https://patchwork.kernel.org/project/linux-arm-msm/patch/1592908737-7068-3-git-send-email-akash...@codeaurora.org/
[1]
Tested-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH v2 3/3] soc: qcom: geni: Optimize/comment select fifo/dma mode
On 10/14/2020 2:55 AM, Douglas Anderson wrote: The functions geni_se_select_fifo_mode() and geni_se_select_fifo_mode() are a little funny. They read/write a bunch of memory mapped registers even if they don't change or aren't relevant for the current protocol. Let's make them a little more sane. We'll also add a comment explaining why we don't do some of the operations for UART. NOTE: there is no evidence at all that this makes any performance difference and it fixes no bugs. However, it seems (to me) like it makes the functions a little easier to understand. Decreasing the amount of times we read/write memory mapped registers is also nice, even if we are using "relaxed" variants. Signed-off-by: Douglas Anderson Reviewed-by: Akash Asthana The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH 1/3] soc: qcom: geni: More properly switch to DMA mode
Hi Stephen,
static void geni_se_select_dma_mode(struct geni_se *se)
{
+ u32 proto = geni_se_read_proto(se);
u32 val;
geni_se_irq_clear(se);
+ val = readl_relaxed(se->base + SE_GENI_M_IRQ_EN);
+ if (proto != GENI_SE_UART) {
Not a problem with this patch but it would be great if there was a
comment here (and probably in geni_se_select_fifo_mode() too) indicating
why GENI_SE_UART is special. Is it because GENI_SE_UART doesn't use the
main sequencer? I think that is the reason, but I forgot and reading
this code doesn't tell me that.
Splitting the driver in this way where the logic is in the geni wrapper
and in the engine driver leads to this confusion.
GENI_SE_UART uses main sequencer for TX and secondary for RX transfers
because it is asynchronous in nature.
That's why RX related bits (M_RX_FIFO_WATERMARK_EN |
M_RX_FIFO_LAST_EN) are not enable in main sequencer for UART.
(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN) bits are controlled from UART
driver, it's gets enabled and disabled multiple times from start_tx
,stop_tx respectively.
Regards,
Akash
+ val &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN);
+ val &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
+ }
+ writel_relaxed(val, se->base + SE_GENI_M_IRQ_EN);
+
+ val = readl_relaxed(se->base + SE_GENI_S_IRQ_EN);
+ if (proto != GENI_SE_UART)
+ val &= ~S_CMD_DONE_EN;
+ writel_relaxed(val, se->base + SE_GENI_S_IRQ_EN);
+
val = readl_relaxed(se->base + SE_GENI_DMA_MODE_EN);
val |= GENI_DMA_MODE_EN;
writel_relaxed(val, se->base + SE_GENI_DMA_MODE_EN);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 2/3] Revert "i2c: i2c-qcom-geni: Fix DMA transfer race"
On 10/9/2020 4:22 AM, Douglas Anderson wrote:
This reverts commit 02b9aec59243c6240fc42884acc958602146ddf6.
As talked about in the patch ("soc: qcom: geni: More properly switch
to DMA mode"), swapping the order of geni_se_setup_m_cmd() and
geni_se_xx_dma_prep() can sometimes cause corrupted transfers. Thus
we traded one problem for another. Now that we've debugged the
problem further and fixed the geni helper functions to more disable
FIFO interrupts when we move to DMA mode we can revert it and end up
with (hopefully) zero problems!
To be explicit, the patch ("soc: qcom: geni: More properly switch
to DMA mode") is a prerequisite for this one.
Fixes: 02b9aec59243 ("i2c: i2c-qcom-geni: Fix DMA transfer race")
Signed-off-by: Douglas Anderson
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 1/3] soc: qcom: geni: More properly switch to DMA mode
On 10/9/2020 4:22 AM, Douglas Anderson wrote:
On geni-i2c transfers using DMA, it was seen that if you program the
command (I2C_READ) before calling geni_se_rx_dma_prep() that it could
cause interrupts to fire. If we get unlucky, these interrupts can
just keep firing (and not be handled) blocking further progress and
hanging the system.
In commit 02b9aec59243 ("i2c: i2c-qcom-geni: Fix DMA transfer race")
we avoided that by making sure we didn't program the command until
after geni_se_rx_dma_prep() was called. While that avoided the
problems, it also turns out to be invalid. At least in the TX case we
started seeing sporadic corrupted transfers. This is easily seen by
adding an msleep() between the DMA prep and the writing of the
command, which makes the problem worse. That means we need to revert
that commit and find another way to fix the bogus IRQs.
Specifically, after reverting commit 02b9aec59243 ("i2c:
i2c-qcom-geni: Fix DMA transfer race"), I put some traces in. I found
that the when the interrupts were firing like crazy:
- "m_stat" had bits for M_RX_IRQ_EN, M_RX_FIFO_WATERMARK_EN set.
- "dma" was set.
Further debugging showed that I could make the problem happen more
reliably by adding an "msleep(1)" any time after geni_se_setup_m_cmd()
ran up until geni_se_rx_dma_prep() programmed the length.
A rather simple fix is to change geni_se_select_dma_mode() so it's a
true inverse of geni_se_select_fifo_mode() and disables all the FIFO
related interrupts. Now the problematic interrupts can't fire and we
can program things in the correct order without worrying.
As part of this, let's also change the writel_relaxed() in the prepare
function to a writel() so that our DMA is guaranteed to be prepared
now that we can't rely on geni_se_setup_m_cmd()'s writel().
NOTE: the only current user of GENI_SE_DMA in mainline is i2c.
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C
controller")
Fixes: 02b9aec59243 ("i2c: i2c-qcom-geni: Fix DMA transfer race")
Signed-off-by: Douglas Anderson
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 5/5] i2c: geni: sdm845: dont perform DMA for the oneplus6
On 10/7/2020 11:19 PM, Caleb Connolly wrote:
The OnePlus 6/T has the same issues as the c630 causing a crash when DMA
is used for i2c, so disable it.
https://patchwork.kernel.org/patch/11133827
Reviewed-by: Akash Asthana
Signed-off-by: Caleb Connolly
---
drivers/i2c/busses/i2c-qcom-geni.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index dead5db3315a..50a0674a6553 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -358,7 +358,8 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c,
struct i2c_msg *msg,
struct geni_se *se = >se;
size_t len = msg->len;
- if (!of_machine_is_compatible("lenovo,yoga-c630"))
+ if (!of_machine_is_compatible("lenovo,yoga-c630") &&
+ !of_machine_is_compatible("oneplus,oneplus6"))
dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
if (dma_buf)
@@ -400,7 +401,8 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c,
struct i2c_msg *msg,
struct geni_se *se = >se;
size_t len = msg->len;
- if (!of_machine_is_compatible("lenovo,yoga-c630"))
+ if (!of_machine_is_compatible("lenovo,yoga-c630") &&
+ !of_machine_is_compatible("oneplus,oneplus6"))
dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
if (dma_buf)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH V6] serial: qcom_geni_serial: To correct QUP Version detection logic
On 9/30/2020 11:35 AM, Paras Sharma wrote:
For QUP IP versions 2.5 and above the oversampling rate is
halved from 32 to 16.
Commit ce734600545f ("tty: serial: qcom_geni_serial: Update
the oversampling rate") is pushed to handle this scenario.
But the existing logic is failing to classify QUP Version 3.0
into the correct group ( 2.5 and above).
As result Serial Engine clocks are not configured properly for
baud rate and garbage data is sampled to FIFOs from the line.
So, fix the logic to detect QUP with versions 2.5 and above.
Fixes: ce734600545f ("tty: serial: qcom_geni_serial: Update the oversampling
rate")
Signed-off-by: Paras Sharma
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH v3 2/2] arm64: dts: qcom: Switch sc7180-trogdor to control SPI CS via GPIO
On 9/22/2020 2:57 AM, Douglas Anderson wrote:
As talked about in the patch ("arm64: dts: qcom: sc7180: Provide
pinconf for SPI to use GPIO for CS"), on some boards it makes much
more sense (and is much more efficient) to think of the SPI Chip
Select as a GPIO. Trogdor is one such board where the SPI parts don't
run in GSI mode and we do a lot of SPI traffic.
Signed-off-by: Douglas Anderson
Reviewed-by: Stephen Boyd
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH v3 1/2] arm64: dts: qcom: sc7180: Provide pinconf for SPI to use GPIO for CS
On 9/22/2020 2:57 AM, Douglas Anderson wrote: When the chip select line is controlled by the QUP, changing CS is a time consuming operation. We have to send a command over to the geni and wait for it to Ack us every time we want to change (both making it high and low). To send this command we have to make a choice in software when we want to control the chip select, we have to either: A) Wait for the Ack via interrupt which slows down all SPI transfers (and incurrs extra processing associated with interrupts). B) Sit in a loop and poll, waiting for the Ack. Neither A) nor B) is a great option. We can avoid all of this by realizing that, at least on some boards, there is no advantage of considering this line to be a geni line. While it's true that geni _can_ control the line, it's also true that the line can be a GPIO and there is no downside of viewing it that way. Setting a GPIO is a simple MMIO operation. This patch provides definitions so a board can easily select the GPIO mode. NOTE: apparently, it's possible to run the geni in "GSI" mode. In GSI the SPI port is allowed to be controlled by more than one user (like firmware and Linux) and also the port can operate sequences of operations in one go. In GSI mode it _would_ be invalid to look at the chip select as a GPIO because that would prevent other users from using it. In theory GSI mode would also avoid some overhead by allowing us to sequence the chip select better. However, I'll argue GSI is not relevant for all boards (and certainly not any boards supported by mainline today). Why? - Apparently to run a SPI chip in GSI mode you need to initialize it (in the bootloader) with a different firmware and then it will always run in GSI mode. Since there is no support for GSI mode in the current Linux driver, it must be that existing boards don't have firmware that's doing that. Note that the kernel device tree describes hardware but also firmware, so it is legitimate to make the assumption that we don't have GSI firmware in a given dts file. - Some boards with sc7180 have SPI connected to the Chrome OS EC or security chip (Cr50). The protocols for talking to cros_ec and cr50 are extremely complex. Both drivers in Linux fully lock the bus across several distinct SPI transfers. While I am not an expert on GSI mode it feels highly unlikely to me that we'd ever be able to enable GSI mode for these devices. From a testing perspective, running "flashrom -p ec -r /tmp/foo.bin" in a loop after this patch shows almost no reduction in time, but the number of interrupts per command goes from 32357 down to 30611 (about a 5% reduction). Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Reviewed-by: Akash Asthana -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH V4] serial: qcom_geni_serial: To correct QUP Version detection logic
Hi Paras,
On 9/14/2020 12:49 PM, Paras Sharma wrote:
The current implementation reduces the sampling rate by half
if qup HW version is greater is than 2.5 by checking if the
geni SE major version is greater than 2 and geni SE minor version
is greater than 5.This implementation fails when the version is
greater than or equal to 3.
Hence, a new macro QUP_SE_VERSION_2_5 is defined having value
for major number 2 and minor number 5 as 0x2005.Hence,if
ver is greater than this value,sampling rate is halved.
This logic would work for any future qup version.
Can we rewrite commit message something like below:
For QUP IP versions 2.5 and above the oversampling rate is halved from
32 to 16.
Commit ce734600545f ("tty: serial: qcom_geni_serial: Update the
oversampling rate") is pushed to handle this scenario.
But the existing logic is failing to classify QUP 3.0 to correct group (
2.5 and above).
As result SE clocks are not configured properly for baud rate and
garbage data is sampled to FIFOs from the line.
So, fix the logic to detect QUP with versions 2.5 and above.
Fixes: ce734600545f ("tty: serial: qcom_geni_serial: Update the oversampling
rate")
Signed-off-by: Paras Sharma
---
Changes in V4:
Created a new macro QUP_SE_VERSION_2_5 for Qup se version 2.5
You can mention changes in V3 and V2 here.
drivers/tty/serial/qcom_geni_serial.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
index f0b1b47..9b74b1e 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -106,6 +106,9 @@
/* We always configure 4 bytes per FIFO word */
#define BYTES_PER_FIFO_WORD 4
+/* QUP SE VERSION value for major number 2 and minor number 5 */
+#define QUP_SE_VERSION_2_5 0x2005
+
How about moving this Macro to common header, qcom-geni-se.h so that if
needed other QUP driver also can use it.
Regards,
Akash
struct qcom_geni_private_data {
/* NOTE: earlycon port will have NULL here */
struct uart_driver *drv;
@@ -1000,7 +1003,7 @@ static void qcom_geni_serial_set_termios(struct uart_port
*uport,
sampling_rate = UART_OVERSAMPLING;
/* Sampling rate is halved for IP versions >= 2.5 */
ver = geni_se_get_qup_hw_version(>se);
- if (GENI_SE_VERSION_MAJOR(ver) >= 2 && GENI_SE_VERSION_MINOR(ver) >= 5)
+ if (ver >= QUP_SE_VERSION_2_5)
sampling_rate /= 2;
clk_rate = get_clk_div_rate(baud, sampling_rate, _div);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 2/3] spi: spi-geni-qcom: Don't program CS_TOGGLE again and again
On 9/13/2020 2:38 AM, Douglas Anderson wrote:
We always toggle the chip select manually in spi-geni-qcom so that we
can properly implement the Linux API. There's no reason to program
this to the hardware on every transfer. Program it once at init and
be done with it.
This saves some part of a microsecond of overhead on each transfer.
While not really noticeable on any real world benchmarks, we might as
well save the time.
Yeah this is configuration part, can be moved to one time init function,
as per HPG CS_TOGGLE bit of SPI_TRANS_CFG register is used to instruct
FW to toggle CS line btw each words. We never came across any
usecase/slave who needs this.
Reviewed-by: Akash Asthana
Signed-off-by: Douglas Anderson
---
drivers/spi/spi-geni-qcom.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 7f0bf0dec466..92d88bf85a90 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -290,6 +290,7 @@ static int spi_geni_init(struct spi_geni_master *mas)
{
struct geni_se *se = >se;
unsigned int proto, major, minor, ver;
+ u32 spi_tx_cfg;
pm_runtime_get_sync(mas->dev);
@@ -322,6 +323,11 @@ static int spi_geni_init(struct spi_geni_master *mas)
geni_se_select_mode(se, GENI_SE_FIFO);
+ /* We always control CS manually */
+ spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG);
+ spi_tx_cfg &= ~CS_TOGGLE;
+ writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
+
pm_runtime_put(mas->dev);
return 0;
}
@@ -331,7 +337,7 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
u16 mode, struct spi_master *spi)
{
u32 m_cmd = 0;
- u32 spi_tx_cfg, len;
+ u32 len;
struct geni_se *se = >se;
int ret;
@@ -350,7 +356,6 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
spin_lock_irq(>lock);
spin_unlock_irq(>lock);
- spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG);
if (xfer->bits_per_word != mas->cur_bits_per_word) {
spi_setup_word_len(mas, mode, xfer->bits_per_word);
mas->cur_bits_per_word = xfer->bits_per_word;
@@ -364,8 +369,6 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
mas->tx_rem_bytes = 0;
mas->rx_rem_bytes = 0;
- spi_tx_cfg &= ~CS_TOGGLE;
-
if (!(mas->cur_bits_per_word % MIN_WORD_LEN))
len = xfer->len * BITS_PER_BYTE / mas->cur_bits_per_word;
else
@@ -384,7 +387,6 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
writel(len, se->base + SE_SPI_RX_TRANS_LEN);
mas->rx_rem_bytes = xfer->len;
}
- writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
/*
* Lock around right before we start the transfer since our
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 1/3] spi: spi-geni-qcom: Use the FIFO even more
On 9/13/2020 2:37 AM, Douglas Anderson wrote:
In commit 902481a78ee4 ("spi: spi-geni-qcom: Actually use our FIFO") I
explained that the maximum size we could program the FIFO was
"mas->tx_fifo_depth - 3" but that I chose "mas->tx_fifo_depth()"
because I was worried about decreased bandwidth.
Since that time:
* All the interconnect patches have landed, making things run at the
proper speed.
* I've done more measurements.
This lets me confirm that there's really no downside of using the FIFO
more. Specifically I did "flashrom -p ec -r /tmp/foo.bin" on a
Chromebook and averaged over several runs.
Before: It took 6.66 seconds and 59669 interrupts fired.
After: It took 6.66 seconds and 47992 interrupts fired.
Reviewed-by: Akash Asthana
Signed-off-by: Douglas Anderson
---
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH] spi: spi-geni-qcom: Don't wait to start 1st transfer if transmitting
On 9/12/2020 11:47 PM, Douglas Anderson wrote: If we're sending bytes over SPI, we know the FIFO is empty at the start of the transfer. There's no reason to wait for the interrupt telling us to start--we can just start right away. Then if we transmit everything in one swell foop we don't even need to bother listening for TX interrupts. In a test of "flashrom -p ec -r /tmp/foo.bin" interrupts were reduced from ~30560 to ~29730, about a 3% savings. This patch looks bigger than it is because I moved a few functions rather than adding a forward declaration. The only actual change to geni_spi_handle_tx() was to make it return a bool indicating if there is more to tx. Reviewed-by: Akash Asthana Signed-off-by: Douglas Anderson -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] tty: serial: qcom_geni_serial: 115.2 is a better console default than 9600
On 9/11/2020 8:30 PM, Douglas Anderson wrote:
Commit c5cbc78acf69 ("tty: serial: qcom_geni_serial: Initialize baud
in qcom_geni_console_setup") fixed a bug by initting a variable that
was used in some cases without initialization. However, the "default"
baud rate picked by that CL was probably not the best choice. The
chances that anyone out there is trying to run a system with kernel
messages piped out over a 9600 baud serial port is just about nil.
Console messages are printed in a blocking manner. At 9600 baud we
print about 1 character per millisecond which means that printing a
40-byte message to the console will take ~40 ms. While it would
probably work, it's going to make boot _very_ slow and probably cause
the occasional timeout here and there in drivers (heck, even at 115200
console delays can wreck havoc).
This has already bit at least two people that I'm aware of that tried
to enable serial console by just adding "console=ttyMSM0" (instead of
"console=ttyMSM0,115200n8") to the command line, so it seems like it'd
be nice to fix.
Let's switch the default to 115200.
Reviewed-by: Akash Asthana
Signed-off-by: Douglas Anderson
---
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH V2 2/2] i2c: i2c-qcom-geni: Add shutdown callback for i2c
Hi Roja,
On 8/20/2020 4:05 PM, Roja Rani Yarubandi wrote:
If the hardware is still accessing memory after SMMU translation
is disabled (as part of smmu shutdown callback), then the
IOVAs (I/O virtual address) which it was using will go on the bus
as the physical addresses which will result in unknown crashes
like NoC/interconnect errors.
So, implement shutdown callback to i2c driver to unmap DMA mappings
during system "reboot" or "shutdown".
Signed-off-by: Roja Rani Yarubandi
---
Changes in V2:
- As per Stephen's comments added seperate function for stop transfer,
fixed minor nitpicks.
drivers/i2c/busses/i2c-qcom-geni.c | 43 ++
include/linux/qcom-geni-se.h | 5
2 files changed, 48 insertions(+)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index 1fda5c7c2cfc..d07f2f33bb75 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -486,6 +486,28 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
return ret;
}
+static void geni_i2c_stop_xfer(struct geni_i2c_dev *gi2c)
+{
+ u32 val;
+ struct geni_se *se = >se;
+
+ val = readl_relaxed(gi2c->se.base + SE_DMA_DEBUG_REG0);
+ if (val & DMA_TX_ACTIVE) {
+ geni_i2c_abort_xfer(gi2c);
+ gi2c->cur_wr = 0;
+ if (gi2c->err)
+ geni_i2c_tx_fsm_rst(gi2c);
+ geni_se_tx_dma_unprep(se, gi2c->tx_dma, gi2c->xfer_len);
+ }
should be 'else if', because TX and RX can't happen parallel.
+ if (val & DMA_RX_ACTIVE) {
+ geni_i2c_abort_xfer(gi2c);
+ gi2c->cur_rd = 0;
+ if (gi2c->err)
+ geni_i2c_rx_fsm_rst(gi2c);
+ geni_se_rx_dma_unprep(se, gi2c->rx_dma, gi2c->xfer_len);
+ }
+}
+
static u32 geni_i2c_func(struct i2c_adapter *adap)
{
return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK);
@@ -617,6 +639,26 @@ static int geni_i2c_remove(struct platform_device *pdev)
return 0;
}
+static void geni_i2c_shutdown(struct platform_device *pdev)
+{
+ int ret;
+ u32 dma;
+ struct geni_i2c_dev *gi2c = platform_get_drvdata(pdev);
+ struct geni_se *se = >se;
+
+ ret = pm_runtime_get_sync(gi2c->se.dev);
+ if (ret < 0) {
+ dev_err(gi2c->se.dev, "Failed to resume device: %d\n", ret);
+ return;
+ }
+
+ dma = readl_relaxed(se->base + SE_GENI_DMA_MODE_EN);
Wrt to current issue context it may be suffice to stop just DMA mode
transfers but why not stop all mode of active transfer during shutdown
in a generic way.
Regards,
Akash
+ if (dma)
+ geni_i2c_stop_xfer(gi2c);
+
+ pm_runtime_put_sync_suspend(gi2c->se.dev);
+}
+
static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
{
int ret;
@@ -677,6 +719,7 @@ MODULE_DEVICE_TABLE(of, geni_i2c_dt_match);
static struct platform_driver geni_i2c_driver = {
.probe = geni_i2c_probe,
.remove = geni_i2c_remove,
+ .shutdown = geni_i2c_shutdown,
.driver = {
.name = "geni_i2c",
.pm = _i2c_pm_ops,
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index dd464943f717..c3c016496d98 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -77,6 +77,7 @@ struct geni_se {
#define SE_DMA_RX_FSM_RST 0xd58
#define SE_HW_PARAM_0 0xe24
#define SE_HW_PARAM_1 0xe28
+#define SE_DMA_DEBUG_REG0 0xe40
/* GENI_FORCE_DEFAULT_REG fields */
#define FORCE_DEFAULT BIT(0)
@@ -207,6 +208,10 @@ struct geni_se {
#define RX_GENI_CANCEL_IRQBIT(11)
#define RX_GENI_GP_IRQ_EXTGENMASK(13, 12)
+/* SE_DMA_DEBUG_REG0 Register fields */
+#define DMA_TX_ACTIVE BIT(0)
+#define DMA_RX_ACTIVE BIT(1)
+
/* SE_HW_PARAM_0 fields */
#define TX_FIFO_WIDTH_MSK GENMASK(29, 24)
#define TX_FIFO_WIDTH_SHFT24
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH V2 1/2] i2c: i2c-qcom-geni: Store DMA mapping data in geni_i2c_dev struct
Hi Roja,
On 8/20/2020 4:05 PM, Roja Rani Yarubandi wrote:
Store DMA mapping data in geni_i2c_dev struct to enhance DMA mapping
data scope. For example during shutdown callback to unmap DMA mapping,
this stored DMA mapping data can be used to call geni_se_tx_dma_unprep
and geni_se_rx_dma_unprep functions.
Signed-off-by: Roja Rani Yarubandi
---
Changes in V2:
- As per Stephen's comments, changed commit text, fixed minor nitpicks.
drivers/i2c/busses/i2c-qcom-geni.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index 7f130829bf01..1fda5c7c2cfc 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -86,6 +86,9 @@ struct geni_i2c_dev {
u32 clk_freq_out;
const struct geni_i2c_clk_fld *clk_fld;
int suspended;
+ dma_addr_t tx_dma;
+ dma_addr_t rx_dma;
+ size_t xfer_len;
};
struct geni_i2c_err_log {
@@ -358,6 +361,7 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c,
struct i2c_msg *msg,
struct geni_se *se = >se;
size_t len = msg->len;
+ gi2c->xfer_len = msg->len;
nit: gi2c->xfer = len, for tx_one_msg as well.
Regards,
Akash
if (!of_machine_is_compatible("lenovo,yoga-c630"))
dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
@@ -384,6 +388,7 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
if (dma_buf) {
if (gi2c->err)
geni_i2c_rx_fsm_rst(gi2c);
+ gi2c->rx_dma = rx_dma;
geni_se_rx_dma_unprep(se, rx_dma, len);
i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err);
}
@@ -400,6 +405,7 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c,
struct i2c_msg *msg,
struct geni_se *se = >se;
size_t len = msg->len;
+ gi2c->xfer_len = msg->len;
if (!of_machine_is_compatible("lenovo,yoga-c630"))
dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
@@ -429,6 +435,7 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
if (dma_buf) {
if (gi2c->err)
geni_i2c_tx_fsm_rst(gi2c);
+ gi2c->tx_dma = tx_dma;
geni_se_tx_dma_unprep(se, tx_dma, len);
i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH] serial: qcom_geni_serial: Fix recent kdb hang
On 8/11/2020 2:56 AM, Doug Anderson wrote:
Hi,
On Mon, Aug 10, 2020 at 5:32 AM Akash Asthana wrote:
Hi Doug,
On 8/7/2020 10:49 AM, Douglas Anderson wrote:
The commit e42d6c3ec0c7 ("serial: qcom_geni_serial: Make kgdb work
even if UART isn't console") worked pretty well and I've been doing a
lot of debugging with it. However, recently I typed "dmesg" in kdb
and then held the space key down to scroll through the pagination. My
device hung. This was repeatable and I found that it was introduced
with the aforementioned commit.
It turns out that there are some strange boundary cases in geni where
in some weird situations it will signal RX_LAST but then will put 0 in
RX_LAST_BYTE. This means that the entire last FIFO entry is valid.
IMO that means we received a word in RX_FIFO and it is the last word
hence RX_LAST bit is set.
What you say would make logical sense, but it's not how I have
observed geni to work. See below.
RX_LAST_BYTE is 0 means none of the bytes are valid in the last word.
This would imply that qcom_geni_serial_handle_rx() is also broken
though, wouldn't it? Specifically imagine that WORD_CNT is 1 and
RX_LAST is set and RX_LAST_BYTE_VALID is true. Here's the logic from
that function:
total_bytes = BYTES_PER_FIFO_WORD * (word_cnt - 1);
if (last_word_partial && last_word_byte_cnt)
total_bytes += last_word_byte_cnt;
else
total_bytes += BYTES_PER_FIFO_WORD;
port->handle_rx(uport, total_bytes, drop);
As you can see that logic will set "total_bytes" to 4 in the case I'm
talking about.
Yeah IMO as per theory this should also be corrected but since you have
already pulled out few experiment to prove garbage data issue(which I
was suspecting) is not seen.
It's already consistent with existing logic and it behaves well
practically . So the changes could be merge. Meanwhile I am checking
with HW team to get clarity.
In such scenario we should just read RX_FIFO buffer (to empty it),
discard the word and return NO_POLL_CHAR. Something like below.
-
else
private_data->poll_cached_bytes_cnt = 4;
private_data->poll_cached_bytes =
readl(uport->membase + SE_GENI_RX_FIFOn);
}
+if (!private_data->poll_cached_bytes_cnt)
+ return NO_POLL_CHAR;
private_data->poll_cached_bytes_cnt--;
ret = private_data->poll_cached_bytes & 0xff;
-
Please let me know whether above code helps.
Your code will avoid the hang. Yes. ...but it will drop bytes. I
devised a quick-n-dirty test. Here's a test of your code:
I assumed those as invalid bytes and don't wanted to read them so yeah
dropping of bytes was expected.
https://crrev.com/c/2346886
...and here's a test of my code:
https://crrev.com/c/2346884
I had to keep a buffer around since it's hard to debug the serial
driver. In both cases I put "DOUG" into the buffer when I detect this
case. If my theory about how geni worked was wrong then we should
expect to see some garbage in the buffer right after the DOUG, right?
...but my code gets the alphabet in nice sequence. Your code drops 4
bytes.
Yeah I was expecting garbage data.
NOTE: while poking around with the above two test patches I found it
was pretty easy to get geni to drop bytes / hit overflow cases and
also to insert bogus 0 bytes in the stream (I believe these are
related). I was able to reproduce this:
* With ${SUBJECT} patch in place.
* With your proposed patch.
* With the recent "geni" patches reverted (in other words back to 1
byte per FIFO entry).
It's not terribly surprising that we're overflowing since I believe
kgdb isn't too keen to read characters at the same time it's writing.
That doesn't explain the weird 0-bytes that geni seemed to be
inserting, but at least it would explain the overflows. However, even
after I fixed this I _still_ was getting problems. Specifically geni
seemed to be hiding bytes from me until it was too late. I put
logging in and would see this:
1 word in FIFO - wxyz
1 word in FIFO (last set, last FIFO has 1 byte) - \n
Check again, still 0 bytes in FIFO
Suddenly 16 bytes are in FIFO and S_RX_FIFO_WR_ERR_EN is set.
RX data first stored in RX_ASYNC_FIFO then it's transfered to RX_FIFO
When get_char is called and we observe 0 bytes in RX_FIFO, most probably
data is not transfered from RX_ASYNC_FIFO to RX_FIFO.
BITS 27:25 of SE_GENI_RX_FIFO_STATUS register shows RX_ASYNC_FIFO word
count.
I spent a whole bunch of time poking at this and couldn't find any
sort of workaround. P
Re: [PATCH] serial: qcom_geni_serial: Fix recent kdb hang
Hi Doug,
On 8/7/2020 10:49 AM, Douglas Anderson wrote:
The commit e42d6c3ec0c7 ("serial: qcom_geni_serial: Make kgdb work
even if UART isn't console") worked pretty well and I've been doing a
lot of debugging with it. However, recently I typed "dmesg" in kdb
and then held the space key down to scroll through the pagination. My
device hung. This was repeatable and I found that it was introduced
with the aforementioned commit.
It turns out that there are some strange boundary cases in geni where
in some weird situations it will signal RX_LAST but then will put 0 in
RX_LAST_BYTE. This means that the entire last FIFO entry is valid.
IMO that means we received a word in RX_FIFO and it is the last word
hence RX_LAST bit is set.
RX_LAST_BYTE is 0 means none of the bytes are valid in the last word.
In such scenario we should just read RX_FIFO buffer (to empty it),
discard the word and return NO_POLL_CHAR. Something like below.
-
else
private_data->poll_cached_bytes_cnt = 4;
private_data->poll_cached_bytes =
readl(uport->membase + SE_GENI_RX_FIFOn);
}
+ if (!private_data->poll_cached_bytes_cnt)
+ return NO_POLL_CHAR;
private_data->poll_cached_bytes_cnt--;
ret = private_data->poll_cached_bytes & 0xff;
-
Please let me know whether above code helps.
I am not sure about what all scenario can leads to this behavior from
hardware, I will try to get an answer from hardware team.
Any error bit was set for SE_GENI_S_IRQ_STATUS & SE_GENI_M_IRQ_STATUS
registers?
I guess the hang was seen because *poll_cached_bytes_cnt* is unsigned
int and it's value was 0, when it's decremented by 1 it's value become
'4294967295' (very large) and dummy RX (0x00) would happen that
many times before reading any actual RX transfers/bytes.
Regards,
Akash
This weird corner case is handled in qcom_geni_serial_handle_rx()
where you can see that we only honor RX_LAST_BYTE if RX_LAST is set
_and_ RX_LAST_BYTE is non-zero. If either of these is not true we use
BYTES_PER_FIFO_WORD (4) for the size of the last FIFO word.
Let's fix kgdb. While at it, also use the proper #define for 4.
Fixes: e42d6c3ec0c7 ("serial: qcom_geni_serial: Make kgdb work even if UART isn't
console")
Signed-off-by: Douglas Anderson
---
drivers/tty/serial/qcom_geni_serial.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
index 07b7b6b05b8b..e27077656939 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -361,11 +361,16 @@ static int qcom_geni_serial_get_char(struct uart_port
*uport)
return NO_POLL_CHAR;
if (word_cnt == 1 && (status & RX_LAST))
+ /*
+* NOTE: If RX_LAST_BYTE_VALID is 0 it needs to be
+* treated as if it was BYTES_PER_FIFO_WORD.
+*/
private_data->poll_cached_bytes_cnt =
(status & RX_LAST_BYTE_VALID_MSK) >>
RX_LAST_BYTE_VALID_SHFT;
- else
- private_data->poll_cached_bytes_cnt = 4;
+
+ if (private_data->poll_cached_bytes_cnt == 0)
+ private_data->poll_cached_bytes_cnt =
BYTES_PER_FIFO_WORD;
private_data->poll_cached_bytes =
readl(uport->membase + SE_GENI_RX_FIFOn);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH V2 3/3] tty: serial: qcom_geni_serial: Fix the UART wakeup issue
On 7/24/2020 9:28 AM, satya priya wrote: As a part of system suspend we call uart_port_suspend from the Serial driver, which calls set_mctrl passing mctrl as NULL. This makes RFR high(NOT_READY) during suspend. Due to this BT SoC is not able to send wakeup bytes to UART during suspend. Included if check for non-suspend case to keep RFR low during suspend. Reviewed-by: Akash Asthana Signed-off-by: satya priya --- Changes in V2: - This patch fixes the UART flow control issue during suspend. Newly added in V2. drivers/tty/serial/qcom_geni_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 07b7b6b..7108dfc 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -242,7 +242,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport, if (mctrl & TIOCM_LOOP) port->loopback = RX_TX_CTS_RTS_SORTED; - if (!(mctrl & TIOCM_RTS)) + if ((!(mctrl & TIOCM_RTS)) && (!(uport->suspended))) uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY; writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR); } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH V2 2/3] arm64: dts: qcom: sc7180: Add sleep pin ctrl for BT uart
On 7/24/2020 9:28 AM, satya priya wrote:
Add sleep pin ctrl for BT uart, and also change the bias
configuration to match Bluetooth module.
Reviewed-by: Akash Asthana
Signed-off-by: satya priya
---
Changes in V2:
- This patch adds sleep state for BT UART. Newly added in V2.
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 42 -
1 file changed, 36 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 26cc491..bc919f2 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -469,20 +469,50 @@
_uart3_default {
pinconf-cts {
- /*
-* Configure a pull-down on 38 (CTS) to match the pull of
-* the Bluetooth module.
-*/
+ /* Configure no pull on 38 (CTS) to match Bluetooth module */
pins = "gpio38";
+ bias-disable;
+ };
+
+ pinconf-rts {
+ /* We'll drive 39 (RTS), so configure pull-down */
+ pins = "gpio39";
+ drive-strength = <2>;
bias-pull-down;
+ };
+
+ pinconf-tx {
+ /* We'll drive 40 (TX), so no pull */
+ pins = "gpio40";
+ drive-strength = <2>;
+ bias-disable;
output-high;
};
+ pinconf-rx {
+ /*
+* Configure a pull-up on 41 (RX). This is needed to avoid
+* garbage data when the TX pin of the Bluetooth module is
+* in tri-state (module powered off or not driving the
+* signal yet).
+*/
+ pins = "gpio41";
+ bias-pull-up;
+ };
+};
+
+_uart3_sleep {
+ pinconf-cts {
+ /* Configure no-pull on 38 (CTS) to match Bluetooth module */
+ pins = "gpio38";
+ bias-disable;
+ };
+
pinconf-rts {
- /* We'll drive 39 (RTS), so no pull */
+ /* We'll drive 39 (RTS), so configure pull-down */
pins = "gpio39";
drive-strength = <2>;
- bias-disable;
+ bias-pull-down;
};
pinconf-tx {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH V2 1/3] arm64: dts: sc7180: Add wakeup support over UART RX
On 7/24/2020 9:28 AM, satya priya wrote: Add the necessary pinctrl and interrupts to make UART wakeup capable. Reviewed-by: Akash Asthana Signed-off-by: satya priya --- Changes in V2: - As per Matthias's comment added wakeup support for all the UARTs of SC7180. arch/arm64/boot/dts/qcom/sc7180.dtsi | 98 ++-- 1 file changed, 84 insertions(+), 14 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 16df08d..044a4d0 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -787,9 +787,11 @@ reg = <0 0x0088 0 0x4000>; clock-names = "se"; clocks = < GCC_QUPV3_WRAP0_S0_CLK>; - pinctrl-names = "default"; + pinctrl-names = "default", "sleep"; pinctrl-0 = <_uart0_default>; - interrupts = ; + pinctrl-1 = <_uart0_sleep>; + interrupts-extended = < GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>, + < 37 IRQ_TYPE_EDGE_FALLING>; power-domains = < SC7180_CX>; operating-points-v2 = <_opp_table>; interconnects = <_virt MASTER_QUP_CORE_0 _virt SLAVE_QUP_CORE_0>, @@ -839,9 +841,11 @@ reg = <0 0x00884000 0 0x4000>; clock-names = "se"; clocks = < GCC_QUPV3_WRAP0_S1_CLK>; - pinctrl-names = "default"; + pinctrl-names = "default", "sleep"; pinctrl-0 = <_uart1_default>; - interrupts = ; + pinctrl-1 = <_uart1_sleep>; + interrupts-extended = < GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>, + < 3 IRQ_TYPE_EDGE_FALLING>; power-domains = < SC7180_CX>; operating-points-v2 = <_opp_table>; interconnects = <_virt MASTER_QUP_CORE_0 _virt SLAVE_QUP_CORE_0>, @@ -925,9 +929,11 @@ reg = <0 0x0088c000 0 0x4000>; clock-names = "se"; clocks = < GCC_QUPV3_WRAP0_S3_CLK>; - pinctrl-names = "default"; + pinctrl-names = "default", "sleep"; pinctrl-0 = <_uart3_default>; - interrupts = ; + pinctrl-1 = <_uart3_sleep>; + interrupts-extended = < GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>, + < 41 IRQ_TYPE_EDGE_FALLING>; power-domains = < SC7180_CX>; operating-points-v2 = <_opp_table>; interconnects = <_virt MASTER_QUP_CORE_0 _virt SLAVE_QUP_CORE_0>, @@ -1011,9 +1017,11 @@ reg = <0 0x00894000 0 0x4000>; clock-names = "se"; clocks = < GCC_QUPV3_WRAP0_S5_CLK>; - pinctrl-names = "default"; + pinctrl-names = "default", "sleep"; pinctrl-0 = <_uart5_default>; - interrupts = ; + pinctrl-1 = <_uart5_sleep>; + interrupts-extended = < GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>, + < 28 IRQ_TYPE_EDGE_FALLING>; power-domains = < SC7180_CX>; operating-points-v2 = <_opp_table>; interconnects = <_virt MASTER_QUP_CORE_0 _virt SLAVE_QUP_CORE_0>, @@ -1078,9 +1086,11 @@ reg = <0 0x00a8 0 0x4000>; clock-names = "se"; clocks = < GCC_QUPV3_WRAP1_S0_CLK>; - pinctrl-names = "default"; + pinctrl-names = "default&quo
Re: [PATCH v2] i2c: i2c-qcom-geni: Fix DMA transfer race
On 7/26/2020 6:17 PM, Wolfram Sang wrote:
On Thu, Jul 23, 2020 at 09:56:34PM +0200, Wolfram Sang wrote:
On Wed, Jul 22, 2020 at 03:00:21PM -0700, Douglas Anderson wrote:
When I have KASAN enabled on my kernel and I start stressing the
touchscreen my system tends to hang. The touchscreen is one of the
only things that does a lot of big i2c transfers and ends up hitting
the DMA paths in the geni i2c driver. It appears that KASAN adds
enough delay in my system to tickle a race condition in the DMA setup
code.
When the system hangs, I found that it was running the geni_i2c_irq()
over and over again. It had these:
m_stat = 0x0480
rx_st= 0x3011
dm_tx_st = 0x
dm_rx_st = 0x
dma = 0x0001
Notably we're in DMA mode but are getting M_RX_IRQ_EN and
M_RX_FIFO_WATERMARK_EN over and over again.
Putting some traces in geni_i2c_rx_one_msg() showed that when we
failed we were getting to the start of geni_i2c_rx_one_msg() but were
never executing geni_se_rx_dma_prep().
I believe that the problem here is that we are starting the geni
command before we run geni_se_rx_dma_prep(). If a transfer makes it
far enough before we do that then we get into the state I have
observed. Let's change the order, which seems to work fine.
Although problems were seen on the RX path, code inspection suggests
that the TX should be changed too. Change it as well.
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C
controller")
Signed-off-by: Douglas Anderson
Tested-by: Sai Prakash Ranjan
Reviewed-by: Akash Asthana
Applied to for-current, thanks!
Glad we got this sorted. I just wondered that Alok wasn't part of the
discussion. Is he still interested in maintaining the driver? Also
because there is an unprocessed patch left for this driver:
http://patchwork.ozlabs.org/project/linux-i2c/patch/20191103212204.13606-1-colin.k...@canonical.com/
Alok has moved out of GENI team, he no longer supports GENI I2C drivers.
I have posted a patch to update maintainers list. Patch:
https://patchwork.kernel.org/patch/11686541/ [MAINTAINERS: Update Geni
I2C maintainers list]
Also, Girish Mahadevan, Sagar Dharia and Karthikeyan Ramasubramanian no
longer supports GENI drivers.
Regards,
Akash
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH] MAINTAINERS: Update GENI I2C maintainers list
Alok Chauhan has moved out of GENI team, he no longer supports GENI I2C driver, remove him from maintainer list. Add Akash Asthana & Mukesh Savaliya as maintainers for GENI I2C drivers. Signed-off-by: Akash Asthana --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 235ab38..d216344 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13845,7 +13845,8 @@ F: drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c F: Documentation/devicetree/bindings/net/qcom,ethqos.txt QUALCOMM GENERIC INTERFACE I2C DRIVER -M: Alok Chauhan +M: Akash Asthana +M: Mukesh Savaliya L: linux-...@vger.kernel.org L: linux-arm-...@vger.kernel.org S: Supported -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] i2c: qcom-geni: fix spelling mistake "unepxected" -> "unexpected"
On 7/27/2020 1:25 PM, Akash Asthana wrote:
On 11/4/2019 2:52 AM, Colin King wrote:
From: Colin Ian King
There is a spelling mistake in an error message string, fix it.
Signed-off-by: Colin Ian King
---
drivers/i2c/busses/i2c-qcom-geni.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index 17abf60c94ae..387fb5a83471 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -98,7 +98,7 @@ static const struct geni_i2c_err_log gi2c_log[] = {
[GP_IRQ0] = {-EIO, "Unknown I2C err GP_IRQ0"},
[NACK] = {-ENXIO, "NACK: slv unresponsive, check its
power/reset-ln"},
[GP_IRQ2] = {-EIO, "Unknown I2C err GP IRQ2"},
- [BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unepxected
start/stop"},
+ [BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unexpected
start/stop"},
[ARB_LOST] = {-EAGAIN, "Bus arbitration lost, clock line
undriveable"},
[GP_IRQ5] = {-EIO, "Unknown I2C err GP IRQ5"},
[GENI_OVERRUN] = {-EIO, "Cmd overrun, check GENI cmd-state
machine"},
The patch is still applying cleanly on tip.
Reviewed-by: Akash Asthana
Correct tag
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH] i2c: qcom-geni: fix spelling mistake "unepxected" -> "unexpected"
On 11/4/2019 2:52 AM, Colin King wrote:
From: Colin Ian King
There is a spelling mistake in an error message string, fix it.
Signed-off-by: Colin Ian King
---
drivers/i2c/busses/i2c-qcom-geni.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index 17abf60c94ae..387fb5a83471 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -98,7 +98,7 @@ static const struct geni_i2c_err_log gi2c_log[] = {
[GP_IRQ0] = {-EIO, "Unknown I2C err GP_IRQ0"},
[NACK] = {-ENXIO, "NACK: slv unresponsive, check its power/reset-ln"},
[GP_IRQ2] = {-EIO, "Unknown I2C err GP IRQ2"},
- [BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unepxected start/stop"},
+ [BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unexpected start/stop"},
[ARB_LOST] = {-EAGAIN, "Bus arbitration lost, clock line undriveable"},
[GP_IRQ5] = {-EIO, "Unknown I2C err GP IRQ5"},
[GENI_OVERRUN] = {-EIO, "Cmd overrun, check GENI cmd-state machine"},
The patch is still applying cleanly on tip.
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH] soc: qcom-geni-se: Don't use relaxed writes when writing commands
On 7/23/2020 3:31 AM, Douglas Anderson wrote: Writing the command is the final step in kicking off a transfer. Let's use writel() to ensure that any other memory accesses are done before the command kicks off. It's expected that this is mostly relevant if we're in DMA mode but since it doesn't appear to regress performance in a measurable way [1] even in PIO mode and it's easier to reason about then let's just always use it. NOTE: this patch came about due to code inspection. No actual problems were observed that this patch fixes. [1] Tested by timing "flashrom -p ec" on a Chromebook which stresses GENI SPI a lot. Suggested-by: Stephen Boyd Signed-off-by: Douglas Anderson --- Reviewed-by: Akash Asthana -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] i2c: i2c-qcom-geni: Fix DMA transfer race
On 7/21/2020 5:54 AM, Douglas Anderson wrote:
When I have KASAN enabled on my kernel and I start stressing the
touchscreen my system tends to hang. The touchscreen is one of the
only things that does a lot of big i2c transfers and ends up hitting
the DMA paths in the geni i2c driver. It appears that KASAN adds
enough delay in my system to tickle a race condition in the DMA setup
code.
When the system hangs, I found that it was running the geni_i2c_irq()
over and over again. It had these:
m_stat = 0x0480
rx_st= 0x3011
dm_tx_st = 0x
dm_rx_st = 0x
dma = 0x0001
Notably we're in DMA mode but are getting M_RX_IRQ_EN and
M_RX_FIFO_WATERMARK_EN over and over again.
Putting some traces in geni_i2c_rx_one_msg() showed that when we
failed we were getting to the start of geni_i2c_rx_one_msg() but were
never executing geni_se_rx_dma_prep().
I believe that the problem here is that we are writing the transfer
length and setting up the geni command before we run
geni_se_rx_dma_prep(). If a transfer makes it far enough before we do
that then we get into the state I have observed. Let's change the
order, which seems to work fine.
Fixes: 37692de5d523 ("i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C
controller")
Signed-off-by: Douglas Anderson
---
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH V2] soc: qcom: geni: Fix NULL pointer dereference
pdev struct doesn't exists for the devices whose status are disabled
from DT node, in such cases NULL is returned from 'of_find_device_by_node'
Later when we try to get drvdata from pdev struct NULL pointer dereference
is triggered.
Add a NULL check for return values to fix the issue.
We were hitting this issue when one of QUP is disabled.
Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix
earlycon crash")
Reported-by: Sai Prakash Ranjan
Reviewed-by: Matthias Kaehlcke
Signed-off-by: Akash Asthana
---
Changes in V2:
- Change variable name 'wrapper_pdev' to 'pdev.
drivers/soc/qcom/qcom-geni-se.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 355d503..996f20c 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
struct geni_wrapper *wrapper;
struct device_node *parent;
struct device_node *child;
+ struct platform_device *pdev;
if (!earlycon_wrapper)
return;
@@ -829,7 +830,12 @@ void geni_remove_earlycon_icc_vote(void)
for_each_child_of_node(parent, child) {
if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
continue;
- wrapper = platform_get_drvdata(of_find_device_by_node(child));
+
+ pdev = of_find_device_by_node(child);
+ if (!pdev)
+ continue;
+
+ wrapper = platform_get_drvdata(pdev);
icc_put(wrapper->to_core.path);
wrapper->to_core.path = NULL;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH] soc: qcom: geni: Fix NULL pointer dereference
Hi Matthias,
On 7/17/2020 8:18 PM, Matthias Kaehlcke wrote:
Please make sure to cc the linux-arm-msm@vger list for patches of
Qualcomm code.
Sure.
On Fri, Jul 17, 2020 at 08:02:22PM +0530, Akash Asthana wrote:
pdev struct doesn't exits for the devices whose status are disabled
s/exits/exist/
ok
from DT node, in such cases NULL is returned from 'of_find_device_by_node'
Later when we try to get drvdata from pdev struct NULL pointer dereference
is triggered.
Add a NULL check for return values to fix the issue.
We were hitting this issue when one of QUP is disabled.
Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix earlycon
crash")
Reported-by: Sai Prakash Ranjan
Signed-off-by: Akash Asthana
---
drivers/soc/qcom/qcom-geni-se.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 355d503..6e5fe65 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
struct geni_wrapper *wrapper;
struct device_node *parent;
struct device_node *child;
+ struct platform_device *wrapper_pdev;
nit: since there is no other 'pdev' in this function you could just name
it 'pdev', which is less clunky. The variable is only used immediately
after it is assigned, so it's clear from the context that it refers to
the 'wrapper'.
ok
Thankyou for review!.
regards,
Akash
Reviewed-by: Matthias Kaehlcke
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH] soc: qcom: geni: Fix NULL pointer dereference
pdev struct doesn't exits for the devices whose status are disabled
from DT node, in such cases NULL is returned from 'of_find_device_by_node'
Later when we try to get drvdata from pdev struct NULL pointer dereference
is triggered.
Add a NULL check for return values to fix the issue.
We were hitting this issue when one of QUP is disabled.
Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix
earlycon crash")
Reported-by: Sai Prakash Ranjan
Signed-off-by: Akash Asthana
---
drivers/soc/qcom/qcom-geni-se.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 355d503..6e5fe65 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -820,6 +820,7 @@ void geni_remove_earlycon_icc_vote(void)
struct geni_wrapper *wrapper;
struct device_node *parent;
struct device_node *child;
+ struct platform_device *wrapper_pdev;
if (!earlycon_wrapper)
return;
@@ -829,7 +830,12 @@ void geni_remove_earlycon_icc_vote(void)
for_each_child_of_node(parent, child) {
if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
continue;
- wrapper = platform_get_drvdata(of_find_device_by_node(child));
+
+ wrapper_pdev = of_find_device_by_node(child);
+ if (!wrapper_pdev)
+ continue;
+
+ wrapper = platform_get_drvdata(wrapper_pdev);
icc_put(wrapper->to_core.path);
wrapper->to_core.path = NULL;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH] of: Add stub for of_get_next_parent
Fixes compilation error reported on x86 platform:
drivers/soc/qcom/qcom-geni-se.c:819:11: error: implicit declaration of
function 'of_get_next_parent'.
drivers/soc/qcom/qcom-geni-se.c:819:9: warning: incompatible integer to
pointer conversion assigning to 'struct device_node *' from 'int'
Fixes: 048eb908a1f2 ("soc: qcom-geni-se: Add interconnect support to fix
earlycon crash")
Reported-by: kernel test robot
Signed-off-by: Akash Asthana
---
include/linux/of.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/linux/of.h b/include/linux/of.h
index c669c0a..72e7198 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -630,6 +630,12 @@ static inline struct device_node *of_get_parent(const
struct device_node *node)
return NULL;
}
+static inline struct device_node *of_get_next_parent(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
static inline struct device_node *of_get_next_child(
const struct device_node *node, struct device_node *prev)
{
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH v2 0/2] spi: spi-qcom-qspi: Avoid some per-transfer overhead
On 7/9/2020 8:21 PM, Douglas Anderson wrote:
Not to be confused with the similar series I posed for the _other_
Qualcomm SPI controller (spi-geni-qcom) [1], this one avoids the
overhead on the Quad SPI controller.
It's based atop the current Qualcomm tree including Rajendra's ("spi:
spi-qcom-qspi: Use OPP API to set clk/perf state"). As discussed in
individual patches, these could ideally land through the Qualcomm tree
with Mark's Ack.
Measuring:
* Before OPP / Interconnect patches reading all flash takes: ~3.4 seconds
* After OPP / Interconnect patches reading all flash takes: ~4.7 seconds
* After this patch reading all flash takes: ~3.3 seconds
[1] https://lore.kernel.org/r/20200702004509.2333554-1-diand...@chromium.org
[2]
https://lore.kernel.org/r/1593769293-6354-2-git-send-email-rna...@codeaurora.org
Changes in v2:
- Return error from runtime resume if dev_pm_opp_set_rate() fails.
Douglas Anderson (2):
spi: spi-qcom-qspi: Avoid clock setting if not needed
spi: spi-qcom-qspi: Set an autosuspend delay of 250 ms
drivers/spi/spi-qcom-qspi.c | 43 -
1 file changed, 33 insertions(+), 10 deletions(-)
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH v2] spi: spi-geni-qcom: Set the clock properly at runtime resume
On 7/9/2020 8:10 PM, Douglas Anderson wrote:
In the patch ("spi: spi-geni-qcom: Avoid clock setting if not needed")
we avoid a whole pile of clock code. As part of that, we should have
restored the clock at runtime resume. Do that.
It turns out that, at least with today's configurations, this doesn't
actually matter. That's because none of the current device trees have
an OPP table for geni SPI yet. That makes dev_pm_opp_set_rate(dev, 0)
a no-op. This is why it wasn't noticed in the testing of the original
patch. It's still a good idea to fix, though.
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH] tty: serial: qcom-geni-serial: Drop the icc bw votes in suspend for console
On 7/9/2020 3:07 PM, Rajendra Nayak wrote: When using the geni-serial as console, its important to be able to hit the lowest possible power state in suspend, even with no_console_suspend. The only thing that prevents it today on platforms like the sc7180 is the interconnect BW votes, which we certainly don't need when the system is in suspend. So in the suspend handler mark them as ACTIVE_ONLY (0x3) and on resume switch them back to the ALWAYS tag (0x7) Reviewed-by: Akash Asthana -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] spi: spi-geni-qcom: Set the clock properly at runtime resume
Hi Doug, @@ -670,7 +674,13 @@ static int __maybe_unused spi_geni_runtime_resume(struct device *dev) if (ret) return ret; - return geni_se_resources_on(>se); + ret = geni_se_resources_on(>se); + if (ret) + return ret; + + dev_pm_opp_set_rate(mas->dev, mas->cur_sclk_hz); + + return 0; } Should we fail to resume if error is returned from 'opp_set_rate'? 'spi_geni_prepare_message' use to fail for any error from 'opp_set_rate' before patch series "Avoid clock setting if not needed". But now it's possible that 'prepare_message' can return success even when opp are not at desired state(from previous resume call). Regards, Akash static int __maybe_unused spi_geni_suspend(struct device *dev) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH 3/3] spi: spi-geni-qcom: Get rid of most overhead in prepare_message()
On 7/2/2020 6:15 AM, Douglas Anderson wrote:
There's a bunch of overhead in spi-geni-qcom's prepare_message. Get
rid of it. Before this change spi_geni_prepare_message() took around
14.5 us. After this change, spi_geni_prepare_message() takes about
1.75 us (as measured by ftrace).
What's here:
* We're always in FIFO mode, so no need to call it for every transfer.
This avoids a whole ton of readl/writel calls.
* We don't need to write a whole pile of config registers if the mode
isn't changing. Cache the last mode and only do the work if needed.
* For several registers we were trying to do read/modify/write, but
there was no reason. The registers only have one thing in them, so
just write them.
Signed-off-by: Douglas Anderson
---
drivers/spi/spi-geni-qcom.c | 54 +
1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index f51279608fc7..97fac5ea6afd 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -77,6 +77,7 @@ struct spi_geni_master {
u32 tx_fifo_depth;
u32 fifo_width_bits;
u32 tx_wm;
+ u32 last_mode;
unsigned long cur_speed_hz;
unsigned int cur_bits_per_word;
unsigned int tx_rem_bytes;
@@ -177,8 +178,6 @@ static void spi_setup_word_len(struct spi_geni_master *mas,
u16 mode,
struct geni_se *se = >se;
u32 word_len;
- word_len = readl(se->base + SE_SPI_WORD_LEN);
-
/*
* If bits_per_word isn't a byte aligned value, set the packing to be
* 1 SPI word per FIFO word.
@@ -187,10 +186,9 @@ static void spi_setup_word_len(struct spi_geni_master
*mas, u16 mode,
pack_words = mas->fifo_width_bits / bits_per_word;
else
pack_words = 1;
- word_len &= ~WORD_LEN_MSK;
- word_len |= ((bits_per_word - MIN_WORD_LEN) & WORD_LEN_MSK);
geni_se_config_packing(>se, bits_per_word, pack_words, msb_first,
true, true);
+ word_len = (bits_per_word - MIN_WORD_LEN) & WORD_LEN_MSK;
writel(word_len, se->base + SE_SPI_WORD_LEN);
}
@@ -238,38 +236,34 @@ static int setup_fifo_params(struct spi_device *spi_slv,
{
struct spi_geni_master *mas = spi_master_get_devdata(spi);
struct geni_se *se = >se;
- u32 loopback_cfg, cpol, cpha, demux_output_inv;
+ u32 loopback_cfg = 0, cpol = 0, cpha = 0, demux_output_inv = 0;
u32 demux_sel;
- loopback_cfg = readl(se->base + SE_SPI_LOOPBACK);
- cpol = readl(se->base + SE_SPI_CPOL);
- cpha = readl(se->base + SE_SPI_CPHA);
- demux_output_inv = 0;
- loopback_cfg &= ~LOOPBACK_MSK;
- cpol &= ~CPOL;
- cpha &= ~CPHA;
+ if (mas->last_mode != spi_slv->mode) {
+ if (spi_slv->mode & SPI_LOOP)
+ loopback_cfg = LOOPBACK_ENABLE;
- if (spi_slv->mode & SPI_LOOP)
- loopback_cfg |= LOOPBACK_ENABLE;
+ if (spi_slv->mode & SPI_CPOL)
+ cpol = CPOL;
- if (spi_slv->mode & SPI_CPOL)
- cpol |= CPOL;
+ if (spi_slv->mode & SPI_CPHA)
+ cpha = CPHA;
- if (spi_slv->mode & SPI_CPHA)
- cpha |= CPHA;
+ if (spi_slv->mode & SPI_CS_HIGH)
+ demux_output_inv = BIT(spi_slv->chip_select);
- if (spi_slv->mode & SPI_CS_HIGH)
- demux_output_inv = BIT(spi_slv->chip_select);
+ demux_sel = spi_slv->chip_select;
+ mas->cur_bits_per_word = spi_slv->bits_per_word;
- demux_sel = spi_slv->chip_select;
- mas->cur_bits_per_word = spi_slv->bits_per_word;
+ spi_setup_word_len(mas, spi_slv->mode, spi_slv->bits_per_word);
+ writel(loopback_cfg, se->base + SE_SPI_LOOPBACK);
+ writel(demux_sel, se->base + SE_SPI_DEMUX_SEL);
+ writel(cpha, se->base + SE_SPI_CPHA);
+ writel(cpol, se->base + SE_SPI_CPOL);
+ writel(demux_output_inv, se->base + SE_SPI_DEMUX_OUTPUT_INV);
- spi_setup_word_len(mas, spi_slv->mode, spi_slv->bits_per_word);
- writel(loopback_cfg, se->base + SE_SPI_LOOPBACK);
- writel(demux_sel, se->base + SE_SPI_DEMUX_SEL);
- writel(cpha, se->base + SE_SPI_CPHA);
- writel(cpol, se->base + SE_SPI_CPOL);
- writel(demux_output_inv, se->base + SE_SPI_DEMUX_OUTPUT_INV);
+ mas->last_mode = spi_slv->mode;
+ }
return geni_spi_set_clock_and_bw(mas, spi_slv->max_speed_hz);
}
Yeah looks good to me, the default/reset value of these registers are 0
we don't have to preserve any bits here.
We can directly update the register with required value.
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 2/3] spi: spi-geni-qcom: Set an autosuspend delay of 250 ms
On 7/2/2020 6:15 AM, Douglas Anderson wrote:
In commit 0e3b8a81f5df ("spi: spi-geni-qcom: Add interconnect
support") the spi_geni_runtime_suspend() and spi_geni_runtime_resume()
became a bit slower. Measuring on my hardware I see numbers in the
hundreds of microseconds now.
Let's use autosuspend to help avoid some of the overhead. Now if
we're doing a bunch of transfers we won't need to be constantly
chruning.
The number 250 ms for the autosuspend delay was picked a bit
arbitrarily, so if someone has measurements showing a better value we
could easily change this.
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
Re: [PATCH 1/3] spi: spi-geni-qcom: Avoid clock setting if not needed
On 7/2/2020 6:15 AM, Douglas Anderson wrote:
Every SPI transfer could have a different clock rate. The
spi-geni-qcom controller code to deal with this was never very well
optimized and has always had a lot of code plus some calls into the
clk framework which, at the very least, would grab a mutex. However,
until recently, the overhead wasn't _too_ much. That changed with
commit 0e3b8a81f5df ("spi: spi-geni-qcom: Add interconnect support")
we're now calling geni_icc_set_bw(), which leads to a bunch of math
plus:
geni_icc_set_bw()
icc_set_bw()
apply_constraints()
qcom_icc_set()
qcom_icc_bcm_voter_commit()
rpmh_invalidate()
rpmh_write_batch()
...and those rpmh commands can be a bit beefy if you call them too
often.
Reviewed-by: Akash Asthana
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH V8 5/8] spi: spi-geni-qcom: Combine the clock setting code
From: Douglas Anderson
There is code for adjusting the clock both in setup_fifo_params()
(called from prepare_message()) and in setup_fifo_xfer() (called from
transfer_one()). The code is the same. Abstract it out to a shared
function.
This is a no-op cleanup patch. The only change is to the error string
if we fail to set the clock. Since the two paths has marginally
different error messages I picked the clean one.
Signed-off-by: Douglas Anderson
Signed-off-by: Akash Asthana
---
This patch is introduced in V8 of the series.
drivers/spi/spi-geni-qcom.c | 70 ++---
1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 0c534d1..f186906 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -192,14 +192,42 @@ static void spi_setup_word_len(struct spi_geni_master
*mas, u16 mode,
writel(word_len, se->base + SE_SPI_WORD_LEN);
}
+static int geni_spi_set_clock(struct spi_geni_master *mas, unsigned long
clk_hz)
+{
+ u32 clk_sel, m_clk_cfg, idx, div;
+ struct geni_se *se = >se;
+ int ret;
+
+ ret = get_spi_clk_cfg(clk_hz, mas, , );
+ if (ret) {
+ dev_err(mas->dev, "Err setting clk to %lu: %d\n", clk_hz, ret);
+ return ret;
+ }
+
+ /*
+* SPI core clock gets configured with the requested frequency
+* or the frequency closer to the requested frequency.
+* For that reason requested frequency is stored in the
+* cur_speed_hz and referred in the consecutive transfer instead
+* of calling clk_get_rate() API.
+*/
+ mas->cur_speed_hz = clk_hz;
+
+ clk_sel = idx & CLK_SEL_MSK;
+ m_clk_cfg = (div << CLK_DIV_SHFT) | SER_CLK_EN;
+ writel(clk_sel, se->base + SE_GENI_CLK_SEL);
+ writel(m_clk_cfg, se->base + GENI_SER_M_CLK_CFG);
+
+ return 0;
+}
+
static int setup_fifo_params(struct spi_device *spi_slv,
struct spi_master *spi)
{
struct spi_geni_master *mas = spi_master_get_devdata(spi);
struct geni_se *se = >se;
u32 loopback_cfg, cpol, cpha, demux_output_inv;
- u32 demux_sel, clk_sel, m_clk_cfg, idx, div;
- int ret;
+ u32 demux_sel;
loopback_cfg = readl(se->base + SE_SPI_LOOPBACK);
cpol = readl(se->base + SE_SPI_CPOL);
@@ -222,27 +250,16 @@ static int setup_fifo_params(struct spi_device *spi_slv,
demux_output_inv = BIT(spi_slv->chip_select);
demux_sel = spi_slv->chip_select;
- mas->cur_speed_hz = spi_slv->max_speed_hz;
mas->cur_bits_per_word = spi_slv->bits_per_word;
- ret = get_spi_clk_cfg(mas->cur_speed_hz, mas, , );
- if (ret) {
- dev_err(mas->dev, "Err setting clks ret(%d) for %ld\n",
- ret, mas->cur_speed_hz);
- return ret;
- }
-
- clk_sel = idx & CLK_SEL_MSK;
- m_clk_cfg = (div << CLK_DIV_SHFT) | SER_CLK_EN;
spi_setup_word_len(mas, spi_slv->mode, spi_slv->bits_per_word);
writel(loopback_cfg, se->base + SE_SPI_LOOPBACK);
writel(demux_sel, se->base + SE_SPI_DEMUX_SEL);
writel(cpha, se->base + SE_SPI_CPHA);
writel(cpol, se->base + SE_SPI_CPOL);
writel(demux_output_inv, se->base + SE_SPI_DEMUX_OUTPUT_INV);
- writel(clk_sel, se->base + SE_GENI_CLK_SEL);
- writel(m_clk_cfg, se->base + GENI_SER_M_CLK_CFG);
- return 0;
+
+ return geni_spi_set_clock(mas, spi_slv->max_speed_hz);
}
static int spi_geni_prepare_message(struct spi_master *spi,
@@ -304,6 +321,7 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
u32 m_cmd = 0;
u32 spi_tx_cfg, len;
struct geni_se *se = >se;
+ int ret;
/*
* Ensure that our interrupt handler isn't still running from some
@@ -328,27 +346,9 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
/* Speed and bits per word can be overridden per transfer */
if (xfer->speed_hz != mas->cur_speed_hz) {
- int ret;
- u32 clk_sel, m_clk_cfg;
- unsigned int idx, div;
-
- ret = get_spi_clk_cfg(xfer->speed_hz, mas, , );
- if (ret) {
- dev_err(mas->dev, "Err setting clks:%d\n", ret);
+ ret = geni_spi_set_clock(mas, xfer->speed_hz);
+ if (ret)
return;
- }
- /*
-* SPI core clock gets configured with the requested frequency
-* or the frequency closer to the requested frequency.
-* For that reason requested frequency is stored in the
-*
[PATCH V8 7/8] spi: spi-qcom-qspi: Add interconnect support
Get the interconnect paths for QSPI device and vote according to the
current bus speed of the driver.
Signed-off-by: Akash Asthana
Reviewed-by: Matthias Kaehlcke
---
Changes in V2:
- 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
Changes in V3:
- No Change.
Changes in V4:
- As per Mark's comment move peak_bw guess as twice of avg_bw if
nothing mentioned explicitly to ICC core.
Changes in V5:
- Add icc_enable/disable to power on/off call.
- Save some non-zero avg/peak value to ICC core by calling geni_icc_set_bw
from probe so that when resume/icc_enable is called NOC are running at
some non-zero value.
Changes in V6:
- As per Matthias's comment made print statement consistent across driver
Changes in V7:
- As per Matthias's comment removed usage of peak_bw variable because we don't
have explicit peak requirement, we were voting peak = avg and this can be
tracked using single variable for avg bw.
- As per Matthias's comment improved print log.
Changes in Resend V7:
- As per Matthias comment removed "unsigned int avg_bw_cpu" from
struct qcom_qspi as we are using that variable only once.
Changes in V8:
- No change
drivers/spi/spi-qcom-qspi.c | 56 -
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index 3c4f83b..b5b4cf6 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
+#include
#include
#include
#include
@@ -139,7 +140,8 @@ 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;
+ /* Lock to protect data accessed by IRQs */
spinlock_t lock;
};
@@ -229,6 +231,7 @@ static int qcom_qspi_transfer_one(struct spi_master *master,
int ret;
unsigned long speed_hz;
unsigned long flags;
+ unsigned int avg_bw_cpu;
speed_hz = slv->max_speed_hz;
if (xfer->speed_hz)
@@ -241,6 +244,18 @@ static int qcom_qspi_transfer_one(struct spi_master
*master,
return ret;
}
+ /*
+* Set BW quota for CPU as driver supports FIFO mode only.
+* We don't have explicit peak requirement so keep it equal to avg_bw.
+*/
+ avg_bw_cpu = Bps_to_icc(speed_hz);
+ ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, avg_bw_cpu, avg_bw_cpu);
+ if (ret) {
+ dev_err(ctrl->dev, "%s: ICC BW voting failed for cpu: %d\n",
+ __func__, ret);
+ return ret;
+ }
+
spin_lock_irqsave(>lock, flags);
/* We are half duplex, so either rx or tx will be set */
@@ -458,6 +473,29 @@ static int qcom_qspi_probe(struct platform_device *pdev)
if (ret)
goto exit_probe_master_put;
+ ctrl->icc_path_cpu_to_qspi = devm_of_icc_get(dev, "qspi-config");
+ if (IS_ERR(ctrl->icc_path_cpu_to_qspi)) {
+ ret = PTR_ERR(ctrl->icc_path_cpu_to_qspi);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get cpu path: %d\n", ret);
+ goto exit_probe_master_put;
+ }
+ /* Set BW vote for register access */
+ ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, Bps_to_icc(1000),
+ Bps_to_icc(1000));
+ if (ret) {
+ dev_err(ctrl->dev, "%s: ICC BW voting failed for cpu: %d\n",
+ __func__, ret);
+ goto exit_probe_master_put;
+ }
+
+ ret = icc_disable(ctrl->icc_path_cpu_to_qspi);
+ if (ret) {
+ dev_err(ctrl->dev, "%s: ICC disable failed for cpu: %d\n",
+ __func__, ret);
+ goto exit_probe_master_put;
+ }
+
ret = platform_get_irq(pdev, 0);
if (ret < 0)
goto exit_probe_master_put;
@@ -511,9 +549,17 @@ static int __maybe_unused qcom_qspi_runtime_suspend(struct
device *dev)
{
struct spi_master *master = dev_get_drvdata(dev);
struct qcom_qspi *ctrl = spi_master_get_devdata(master);
+ int ret;
clk_bulk_disable_unprepare(QSPI_NUM_CLKS, ctrl->clks);
+ ret = icc_disable(ctrl->icc_path_cpu_to_qspi);
+ if (ret) {
+ dev_err_ratelimited(ctrl->dev, "%s: ICC disable failed for cpu:
%d\n",
+ __func__, ret);
+ return ret;
+ }
+
return 0;
}
@@ -521,6 +567,14 @@ static int __maybe_unused qcom_qspi_runtime_r
[PATCH V8 8/8] arm64: dts: sc7180: Add interconnect for QUP and QSPI
Add interconnect ports for GENI QUPs and QSPI to set bus capabilities.
Signed-off-by: Akash Asthana
---
Changes in V2:
- As per Bjorn's comment, ignoring 80 char limit in defining interconnects
paths.
Changes in V3:
- No change.
Change in V4:
- No change.
Changes in V5:
- No change.
Chnages in V6:
- No change.
Changes in V7:
- No change.
Changes in V8:
- No change.
arch/arm64/boot/dts/qcom/sc7180.dtsi | 127 +++
1 file changed, 127 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 3a8076c..ad57df2 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -547,6 +547,8 @@
#size-cells = <2>;
ranges;
iommus = <_smmu 0x43 0x0>;
+ interconnects = <_virt MASTER_QUP_CORE_0 _virt
SLAVE_QUP_CORE_0>;
+ interconnect-names = "qup-core";
status = "disabled";
i2c0: i2c@88 {
@@ -559,6 +561,11 @@
interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
+ interconnects = <_virt MASTER_QUP_CORE_0
_virt SLAVE_QUP_CORE_0>,
+ <_noc MASTER_APPSS_PROC
_noc SLAVE_QUP_0>,
+ <_noc MASTER_QUP_0
_virt SLAVE_EBI1>;
+ interconnect-names = "qup-core", "qup-config",
+ "qup-memory";
status = "disabled";
};
@@ -572,6 +579,9 @@
interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
+ interconnects = <_virt MASTER_QUP_CORE_0
_virt SLAVE_QUP_CORE_0>,
+ <_noc MASTER_APPSS_PROC
_noc SLAVE_QUP_0>;
+ interconnect-names = "qup-core", "qup-config";
status = "disabled";
};
@@ -583,6 +593,9 @@
pinctrl-names = "default";
pinctrl-0 = <_uart0_default>;
interrupts = ;
+ interconnects = <_virt MASTER_QUP_CORE_0
_virt SLAVE_QUP_CORE_0>,
+ <_noc MASTER_APPSS_PROC
_noc SLAVE_QUP_0>;
+ interconnect-names = "qup-core", "qup-config";
status = "disabled";
};
@@ -596,6 +609,11 @@
interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
+ interconnects = <_virt MASTER_QUP_CORE_0
_virt SLAVE_QUP_CORE_0>,
+ <_noc MASTER_APPSS_PROC
_noc SLAVE_QUP_0>,
+ <_noc MASTER_QUP_0
_virt SLAVE_EBI1>;
+ interconnect-names = "qup-core", "qup-config",
+ "qup-memory";
status = "disabled";
};
@@ -609,6 +627,9 @@
interrupts = ;
#address-cells = <1>;
#size-cells = <0>;
+ interconnects = <_virt MASTER_QUP_CORE_0
_virt SLAVE_QUP_CORE_0>,
+ <_noc MASTER_APPSS_PROC
_noc SLAVE_QUP_0>;
+ interconnect-names = "qup-core", "qup-config";
status = "disabled";
};
@@ -620,6 +641,9 @@
pinctrl-names = "default";
pinctrl-0 = <_uart1_default>;
interrupts = ;
+ interconnects = <_virt MASTER_QUP_CORE_0
_virt SLAVE_QUP_CORE_0>,
+ <_noc MASTER_APPSS_PROC
_noc SLAVE_QUP_0>;
+ interconnect-names = "qup-core", "qup-config";
status = "disa
[PATCH V8 3/8] i2c: i2c-qcom-geni: Add interconnect support
Get the interconnect paths for I2C based Serial Engine device
and vote according to the bus speed of the driver.
Signed-off-by: Akash Asthana
Reviewed-by: Matthias Kaehlcke
Acked-by: Wolfram Sang
---
Changes in V2:
- As per Bjorn's comment, removed se == NULL check from geni_i2c_icc_get
- As per Bjorn's comment, removed code to set se->icc_path* to NULL in failure
- 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
Changes in V3:
- As per Matthias comment, use common library APIs defined in geni-se
driver for ICC functionality.
Changes in V4:
- Move peak_bw guess as twice of avg_bw if nothing mentioned explicitly
to ICC core.
Changes in V5:
- Use icc_enable/disable in power on/off call.
Changes in V6:
- No changes
Changes in V7:
- As per Matthias's comment removed usage of peak_bw variable because we don't
have explicit peak requirement, we were voting peak = avg and this can be
tracked using single variable for avg bw.
Changes in V8:
- No change.
drivers/i2c/busses/i2c-qcom-geni.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c
b/drivers/i2c/busses/i2c-qcom-geni.c
index 18d1e4f..32b2a99 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -557,6 +557,22 @@ static int geni_i2c_probe(struct platform_device *pdev)
gi2c->adap.dev.of_node = dev->of_node;
strlcpy(gi2c->adap.name, "Geni-I2C", sizeof(gi2c->adap.name));
+ ret = geni_icc_get(>se, "qup-memory");
+ if (ret)
+ return ret;
+ /*
+* Set the bus quota for core and cpu to a reasonable value for
+* register access.
+* Set quota for DDR based on bus speed.
+*/
+ gi2c->se.icc_paths[GENI_TO_CORE].avg_bw = GENI_DEFAULT_BW;
+ gi2c->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
+ gi2c->se.icc_paths[GENI_TO_DDR].avg_bw = Bps_to_icc(gi2c->clk_freq_out);
+
+ ret = geni_icc_set_bw(>se);
+ if (ret)
+ return ret;
+
ret = geni_se_resources_on(>se);
if (ret) {
dev_err(dev, "Error turning on resources %d\n", ret);
@@ -579,6 +595,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
return ret;
}
+ ret = geni_icc_disable(>se);
+ if (ret)
+ return ret;
+
dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
gi2c->suspended = 1;
@@ -623,7 +643,7 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct
device *dev)
gi2c->suspended = 1;
}
- return 0;
+ return geni_icc_disable(>se);
}
static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
@@ -631,6 +651,10 @@ static int __maybe_unused geni_i2c_runtime_resume(struct
device *dev)
int ret;
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
+ ret = geni_icc_enable(>se);
+ if (ret)
+ return ret;
+
ret = geni_se_resources_on(>se);
if (ret)
return ret;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH V8 0/8] Add interconnect support to QSPI and QUP drivers
This patch series is based on tag "next-20200622" of linux-next tree. High level design: - QUP wrapper/common driver. Vote for QUP core on behalf of earlycon from probe. Remove BW vote during earlycon exit call - SERIAL driver. Vote only for CPU/CORE path because driver is in FIFO mode only Vote/unvote from qcom_geni_serial_pm func. Bump up the CPU vote from set_termios call based on real time need - I2C driver. Vote for CORE/CPU/DDR path Vote/unvote from runtime resume/suspend callback As bus speed for I2C is fixed from probe itself no need for bump up. - SPI QUP driver. Vote only for CPU/CORE path because driver is in FIFO mode only Vote/unvote from runtime resume/suspend callback Bump up CPU vote based on real time need per transfer. - QSPI driver. Vote only for CPU path Vote/unvote from runtime resume/suspend callback Bump up CPU vote based on real time need per transfer. Changes in V2: - Add devm_of_icc_get() API interconnect core. - Add ICC support to common driver to fix earlyconsole crash. Changes in V3: - Define common ICC APIs in geni-se driver and use it across geni based I2C,SPI and UART driver. Changes in V4: - Add a patch to ICC core to scale peak requirement as twice of average if it is not mentioned explicilty. Changes in V5: - As per Georgi's suggestion removed patch from ICC core for assuming peak_bw as twice of average when it's not mentioned, instead assume it equall to avg_bw and keep this assumption in ICC client itself. - As per Matthias suggestion use enum for GENI QUP ICC paths. Changes in V6: - No Major change Changes in V7: - As per Matthias's comment removed usage of peak_bw variable because we don't have explicit peak requirement, we were voting peak = avg and this can be tracked using single variable for avg bw. - As per Matthias's comment improved print log. Changes in V8: - Add [PATCH V8 5/8] to factor out common code for clock setting. - Combine ICC voting and clock setting to single API. [PATCH V8 6/8] - Add ICC voting per transfer because in case of multi message, transfer frequency can vary for each message/transfer.[PATCH V8 6/8] Akash Asthana (7): soc: qcom: geni: Support for ICC voting soc: qcom-geni-se: Add interconnect support to fix earlycon crash i2c: i2c-qcom-geni: Add interconnect support tty: serial: qcom_geni_serial: Add interconnect support spi: spi-geni-qcom: Add interconnect support spi: spi-qcom-qspi: Add interconnect support arm64: dts: sc7180: Add interconnect for QUP and QSPI Douglas Anderson (1): spi: spi-geni-qcom: Combine the clock setting code arch/arm64/boot/dts/qcom/sc7180.dtsi | 127 drivers/i2c/busses/i2c-qcom-geni.c| 26 +- drivers/soc/qcom/qcom-geni-se.c | 150 ++ drivers/spi/spi-geni-qcom.c | 100 +++ drivers/spi/spi-qcom-qspi.c | 56 - drivers/tty/serial/qcom_geni_serial.c | 38 - include/linux/qcom-geni-se.h | 40 + 7 files changed, 496 insertions(+), 41 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V8 2/8] soc: qcom-geni-se: Add interconnect support to fix earlycon crash
QUP core clock is shared among all the SE drivers present on particular
QUP wrapper, the system will reset(unclocked access) if earlycon used after
QUP core clock is put to 0 from other SE drivers before real console comes
up.
As earlycon can't vote for it's QUP core need, to fix this add ICC
support to common/QUP wrapper driver and put vote for QUP core from
probe on behalf of earlycon and remove vote during earlycon exit call.
Signed-off-by: Akash Asthana
Reported-by: Matthias Kaehlcke
Reviewed-by: Matthias Kaehlcke
---
Change in V3:
- Add geni_remove_earlycon_icc_vote API that will be used by earlycon
exit function to remove ICC vote for earlyconsole.
- Remove suspend/resume hook for geni-se driver as we are no longer
removing earlyconsole ICC vote from system suspend, we are removing
from earlycon exit.
Change in V4:
- As per Matthias comment make 'earlycon_wrapper' as static structure.
Changes in V5:
- Vote for core path only after checking whether "qcom_geni" earlycon is
actually present or not by traversing over structure "console_drivers".
Changes in V6:
- As per Matthias's comment removed NULL check for console_drivers global
struct, added NULL check for earlycon_wrapper in _remove_earlycon_icc_vote
API
- Addressed nitpicks from Andy.
Changes in V7:
- As per Matthias's comment removed usage of peak_bw variable because we don't
have explicit peak requirement, we were voting peak = avg and this can be
tracked using single variable for avg bw.
- As per Matthias's comment improved print log.
Changes in V8:
- No change.
drivers/soc/qcom/qcom-geni-se.c | 68 +++
drivers/tty/serial/qcom_geni_serial.c | 7
include/linux/qcom-geni-se.h | 2 ++
3 files changed, 77 insertions(+)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 950e347..e2a0ba2 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include
#include
#include
@@ -90,11 +91,14 @@ struct geni_wrapper {
struct device *dev;
void __iomem *base;
struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
+ struct geni_icc_path to_core;
};
static const char * const icc_path_names[] = {"qup-core", "qup-config",
"qup-memory"};
+static struct geni_wrapper *earlycon_wrapper;
+
#define QUP_HW_VER_REG 0x4
/* Common SE registers */
@@ -802,11 +806,38 @@ int geni_icc_disable(struct geni_se *se)
}
EXPORT_SYMBOL(geni_icc_disable);
+void geni_remove_earlycon_icc_vote(void)
+{
+ struct geni_wrapper *wrapper;
+ struct device_node *parent;
+ struct device_node *child;
+
+ if (!earlycon_wrapper)
+ return;
+
+ wrapper = earlycon_wrapper;
+ parent = of_get_next_parent(wrapper->dev->of_node);
+ for_each_child_of_node(parent, child) {
+ if (!of_device_is_compatible(child, "qcom,geni-se-qup"))
+ continue;
+ wrapper = platform_get_drvdata(of_find_device_by_node(child));
+ icc_put(wrapper->to_core.path);
+ wrapper->to_core.path = NULL;
+
+ }
+ of_node_put(parent);
+
+ earlycon_wrapper = NULL;
+}
+EXPORT_SYMBOL(geni_remove_earlycon_icc_vote);
+
static int geni_se_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct resource *res;
struct geni_wrapper *wrapper;
+ struct console __maybe_unused *bcon;
+ bool __maybe_unused has_earlycon = false;
int ret;
wrapper = devm_kzalloc(dev, sizeof(*wrapper), GFP_KERNEL);
@@ -829,6 +860,43 @@ static int geni_se_probe(struct platform_device *pdev)
}
}
+#ifdef CONFIG_SERIAL_EARLYCON
+ for_each_console(bcon) {
+ if (!strcmp(bcon->name, "qcom_geni")) {
+ has_earlycon = true;
+ break;
+ }
+ }
+ if (!has_earlycon)
+ goto exit;
+
+ wrapper->to_core.path = devm_of_icc_get(dev, "qup-core");
+ if (IS_ERR(wrapper->to_core.path))
+ return PTR_ERR(wrapper->to_core.path);
+ /*
+* Put minmal BW request on core clocks on behalf of early console.
+* The vote will be removed earlycon exit function.
+*
+* Note: We are putting vote on each QUP wrapper instead only to which
+* earlycon is connected because QUP core clock of different wrapper
+* share same voltage domain. If core1 is put to 0, then core2 will
+* also run at 0, if not voted. Default ICC vote will be removed ASA
+* we touch any of the core clock.
+* core1 = core2 = max(core1, core2)
+*/
+ ret = icc_set_b
[PATCH V8 1/8] soc: qcom: geni: Support for ICC voting
Add necessary macros and structure variables to support ICC BW
voting from individual SE drivers.
Signed-off-by: Akash Asthana
Reviewed-by: Matthias Kaehlcke
---
Changes in V2:
- As per Bjorn's comment dropped enums for ICC paths, given the three
paths individual members
Changes in V3:
- Add geni_icc_get, geni_icc_vote_on and geni_icc_vote_off as helper API.
- Add geni_icc_path structure in common header
Changes in V4:
- As per Bjorn's comment print error message in geni_icc_get if return
value is not -EPROBE_DEFER.
- As per Bjorn's comment remove NULL on path before calling icc_set_bw
API.
- As per Bjorn's comment drop __func__ print.
- As per Matthias's comment, make ICC path a array instead of individual
member entry in geni_se struct.
Changes in V5:
- As per Matthias's comment defined enums for ICC paths.
- Integrate icc_enable/disable with power on/off call for driver.
- As per Matthias's comment added icc_path_names array to print icc path name
in failure case.
- As per Georgi's suggestion assume peak_bw = avg_bw if not mentioned.
Changes in V6:
- Addressed nitpicks from Matthias.
Changes in V7:
- As per Matthias's comment removed usage of peak_bw variable because we don't
have explicit peak requirement, we were voting peak = avg and this can be
tracked using single variable for avg bw.
- As per Matthias's comment improved print log.
Changes in V8:
- No change.
Note: I have ignored below check patch suggestion because it was throwing
compilation error as 'icc_ddr' is not compile time comstant.
WARNING: char * array declaration might be better as static const
- FILE: drivers/soc/qcom/qcom-geni-se.c:726:
- const char *icc_names[] = {"qup-core", "qup-config", icc_ddr};
drivers/soc/qcom/qcom-geni-se.c | 82 +
include/linux/qcom-geni-se.h| 38 +++
2 files changed, 120 insertions(+)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 7d622ea..950e347 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -92,6 +92,9 @@ struct geni_wrapper {
struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
};
+static const char * const icc_path_names[] = {"qup-core", "qup-config",
+ "qup-memory"};
+
#define QUP_HW_VER_REG 0x4
/* Common SE registers */
@@ -720,6 +723,85 @@ void geni_se_rx_dma_unprep(struct geni_se *se, dma_addr_t
iova, size_t len)
}
EXPORT_SYMBOL(geni_se_rx_dma_unprep);
+int geni_icc_get(struct geni_se *se, const char *icc_ddr)
+{
+ int i, err;
+ const char *icc_names[] = {"qup-core", "qup-config", icc_ddr};
+
+ for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++) {
+ if (!icc_names[i])
+ continue;
+
+ se->icc_paths[i].path = devm_of_icc_get(se->dev, icc_names[i]);
+ if (IS_ERR(se->icc_paths[i].path))
+ goto err;
+ }
+
+ return 0;
+
+err:
+ err = PTR_ERR(se->icc_paths[i].path);
+ if (err != -EPROBE_DEFER)
+ dev_err_ratelimited(se->dev, "Failed to get ICC path '%s':
%d\n",
+ icc_names[i], err);
+ return err;
+
+}
+EXPORT_SYMBOL(geni_icc_get);
+
+int geni_icc_set_bw(struct geni_se *se)
+{
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++) {
+ ret = icc_set_bw(se->icc_paths[i].path,
+ se->icc_paths[i].avg_bw, se->icc_paths[i].avg_bw);
+ if (ret) {
+ dev_err_ratelimited(se->dev, "ICC BW voting failed on
path '%s': %d\n",
+ icc_path_names[i], ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(geni_icc_set_bw);
+
+/* To do: Replace this by icc_bulk_enable once it's implemented in ICC core */
+int geni_icc_enable(struct geni_se *se)
+{
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++) {
+ ret = icc_enable(se->icc_paths[i].path);
+ if (ret) {
+ dev_err_ratelimited(se->dev, "ICC enable failed on path
'%s': %d\n",
+ icc_path_names[i], ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(geni_icc_enable);
+
+int geni_icc_disable(struct geni_se *se)
+{
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(se->icc_paths); i++) {
+ ret = icc_disable(se->icc_paths[i].path);
+ if (ret) {
+ dev_err_ratelimited(se->dev, "ICC disable failed on
path '%s': %d\n",
+ icc_pat
[PATCH V8 4/8] tty: serial: qcom_geni_serial: Add interconnect support
Get the interconnect paths for Uart based Serial Engine device
and vote according to the baud rate requirement of the driver.
Signed-off-by: Akash Asthana
Reviewed-by: Matthias Kaehlcke
Acked-by: Greg Kroah-Hartman
---
Changes in V2:
- As per Bjorn's comment, removed se == NULL check from geni_serial_icc_get
- As per Bjorn's comment, removed code to set se->icc_path* to NULL in failure
- 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
Changes in V3:
- As per Matthias comment, use common library APIs defined in geni-se
driver for ICC functionality.
Changes in V4:
- As per Mark's comment move peak_bw guess as twice of avg_bw if
nothing mentioned explicitly to ICC core.
- As per Matthias's comment select core clock BW based on baud rate.
If it's less than 115200 go for GENI_DEFAULT_BW else CORE_2X_50_MHZ
Changes in V5:
- Add icc_enable/disable to power on/off call.
- Save some non-zero avg/peak value to ICC core by calling geni_icc_set_bw
from probe so that when resume/icc_enable is called NOC are running at
some non-zero value. No need to call icc_disable after BW vote because
console devices are expected to be in active state from the probe itself
and qcom_geni_serial_pm(STATE_OFF) will be called for non-console ones.
Changes in V6:
- No change
Changes in V7:
- As per Matthias's comment removed usage of peak_bw variable because we don't
have explicit peak requirement, we were voting peak = avg and this can be
tracked using single variable for avg bw.
Changes in V8:
- No change.
drivers/tty/serial/qcom_geni_serial.c | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
index a4468db..f701c7e 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -945,6 +945,7 @@ static void qcom_geni_serial_set_termios(struct uart_port
*uport,
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
unsigned long clk_rate;
u32 ver, sampling_rate;
+ unsigned int avg_bw_core;
qcom_geni_serial_stop_rx(uport);
/* baud rate */
@@ -966,6 +967,16 @@ static void qcom_geni_serial_set_termios(struct uart_port
*uport,
ser_clk_cfg = SER_CLK_EN;
ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
+ /*
+* Bump up BW vote on CPU and CORE path as driver supports FIFO mode
+* only.
+*/
+ avg_bw_core = (baud > 115200) ? Bps_to_icc(CORE_2X_50_MHZ)
+ : GENI_DEFAULT_BW;
+ port->se.icc_paths[GENI_TO_CORE].avg_bw = avg_bw_core;
+ port->se.icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(baud);
+ geni_icc_set_bw(>se);
+
/* parity */
tx_trans_cfg = readl(uport->membase + SE_UART_TX_TRANS_CFG);
tx_parity_cfg = readl(uport->membase + SE_UART_TX_PARITY_CFG);
@@ -1235,11 +1246,14 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
if (old_state == UART_PM_STATE_UNDEFINED)
old_state = UART_PM_STATE_OFF;
- if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
+ if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF) {
+ geni_icc_enable(>se);
geni_se_resources_on(>se);
- else if (new_state == UART_PM_STATE_OFF &&
- old_state == UART_PM_STATE_ON)
+ } else if (new_state == UART_PM_STATE_OFF &&
+ old_state == UART_PM_STATE_ON) {
geni_se_resources_off(>se);
+ geni_icc_disable(>se);
+ }
}
static const struct uart_ops qcom_geni_console_pops = {
@@ -1337,6 +1351,17 @@ static int qcom_geni_serial_probe(struct platform_device
*pdev)
return -ENOMEM;
}
+ ret = geni_icc_get(>se, NULL);
+ if (ret)
+ return ret;
+ port->se.icc_paths[GENI_TO_CORE].avg_bw = GENI_DEFAULT_BW;
+ port->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
+
+ /* Set BW for register access */
+ ret = geni_icc_set_bw(>se);
+ if (ret)
+ return ret;
+
port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
"qcom_geni_serial_%s%d",
uart_console(uport) ? "console" : "uart", uport->line);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na
Linux Foundation Collaborative Project
[PATCH V8 6/8] spi: spi-geni-qcom: Add interconnect support
Get the interconnect paths for SPI based Serial Engine device
and vote according to the current bus speed of the driver.
Signed-off-by: Akash Asthana
---
Changes in V2:
- As per Bjorn's comment, removed se == NULL check from geni_spi_icc_get
- As per Bjorn's comment, removed code to set se->icc_path* to NULL in failure
- 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
Changes in V3:
- As per Matthias's comment, use helper ICC function from geni-se driver.
Changes in V4:
- Move peak_bw guess as twice of avg_bw if nothing mentioned explicitly
to ICC core.
Changes in V5:
- Use icc_enable/disable in power on/off call.
- Save some non-zero avg/peak value to ICC core by calling geni_icc_set_bw
from probe so that when resume/icc_enable is called NOC are running at
some non-zero value. No need to call icc_disable after BW vote because
device will resume and suspend before probe return and will leave ICC in
disabled state.
Changes in V6:
- No change
Changes in V7:
- As per Matthias's comment removed usage of peak_bw variable because we don't
have explicit peak requirement, we were voting peak = avg and this can be
tracked using single variable for avg bw.
Changes in V8:
- Adjust ICC vote per transfer, in multimessage transfer scenario.
- Move ICC voting to common API "geni_spi_set_clock_and_bw".
drivers/spi/spi-geni-qcom.c | 36
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index f186906..7a2e579 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -192,7 +192,8 @@ static void spi_setup_word_len(struct spi_geni_master *mas,
u16 mode,
writel(word_len, se->base + SE_SPI_WORD_LEN);
}
-static int geni_spi_set_clock(struct spi_geni_master *mas, unsigned long
clk_hz)
+static int geni_spi_set_clock_and_bw(struct spi_geni_master *mas,
+ unsigned long clk_hz)
{
u32 clk_sel, m_clk_cfg, idx, div;
struct geni_se *se = >se;
@@ -218,6 +219,12 @@ static int geni_spi_set_clock(struct spi_geni_master *mas,
unsigned long clk_hz)
writel(clk_sel, se->base + SE_GENI_CLK_SEL);
writel(m_clk_cfg, se->base + GENI_SER_M_CLK_CFG);
+ /* Set BW quota for CPU as driver supports FIFO mode only. */
+ se->icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(mas->cur_speed_hz);
+ ret = geni_icc_set_bw(se);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -259,7 +266,7 @@ static int setup_fifo_params(struct spi_device *spi_slv,
writel(cpol, se->base + SE_SPI_CPOL);
writel(demux_output_inv, se->base + SE_SPI_DEMUX_OUTPUT_INV);
- return geni_spi_set_clock(mas, spi_slv->max_speed_hz);
+ return geni_spi_set_clock_and_bw(mas, spi_slv->max_speed_hz);
}
static int spi_geni_prepare_message(struct spi_master *spi,
@@ -346,7 +353,7 @@ static void setup_fifo_xfer(struct spi_transfer *xfer,
/* Speed and bits per word can be overridden per transfer */
if (xfer->speed_hz != mas->cur_speed_hz) {
- ret = geni_spi_set_clock(mas, xfer->speed_hz);
+ ret = geni_spi_set_clock_and_bw(mas, xfer->speed_hz);
if (ret)
return;
}
@@ -620,6 +627,17 @@ static int spi_geni_probe(struct platform_device *pdev)
spin_lock_init(>lock);
pm_runtime_enable(dev);
+ ret = geni_icc_get(>se, NULL);
+ if (ret)
+ goto spi_geni_probe_runtime_disable;
+ /* Set the bus quota to a reasonable value for register access */
+ mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
+ mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
+
+ ret = geni_icc_set_bw(>se);
+ if (ret)
+ goto spi_geni_probe_runtime_disable;
+
ret = spi_geni_init(mas);
if (ret)
goto spi_geni_probe_runtime_disable;
@@ -658,14 +676,24 @@ 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;
+
+ ret = geni_se_resources_off(>se);
+ if (ret)
+ return ret;
- return geni_se_resources_off(>se);
+ return geni_icc_disable(>se);
}
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);
+ int ret;
+
+ ret = geni_icc_enable(>se);
+ if (ret)
+ return ret;
return geni_se_resources_on(>se);
}
--
The Qualco
[PATCH V6] serial: msm_geni_serial_console : Add Earlycon support
From: Mukesh Kumar Savaliya This change enables earlyconsole support as static driver for geni based UART. Kernel space UART console driver will be generic for console and other usecases of UART. Signed-off-by: Mukesh Kumar Savaliya Signed-off-by: Akash Asthana --- Changes In V2: - Fixed Makefile Typo issue. Changes In V3: - Removed mb() calls as *_relaxed() should take care. Changes In V4: - Minor change: space between offset and base addition. Changes In V5: - Removed unlikely() macro. - root_freq() array taken as static. - Removed extra readback of the register having no meaning. Changes in V6: - Drop SERIAL_MSM_GENI_HALF_SAMPLING config and support only QUP HW ver > 2.5. As of now no device with QUP ver < 2.5 is using this code for earlycon so it won't break them. I will post separate patch to resolve this limitation. Sending patch on behalf of Mukesh Savaliya. drivers/tty/serial/Kconfig | 7 + drivers/tty/serial/Makefile | 1 + drivers/tty/serial/msm_geni_serial_console.c | 480 +++ 3 files changed, 488 insertions(+) create mode 100644 drivers/tty/serial/msm_geni_serial_console.c diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 780908d..47caaa6 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -956,6 +956,13 @@ config SERIAL_MSM_CONSOLE select SERIAL_CORE_CONSOLE select SERIAL_EARLYCON +config SERIAL_MSM_GENI_EARLY_CONSOLE + bool "MSM on-chip GENI HW based early console support" + select SERIAL_MSM_GENI_HALF_SAMPLING + help + Serial early console driver for Qualcomm Technologies Inc's GENI + based QUP hardware. + config SERIAL_QCOM_GENI tristate "QCOM on-chip GENI based serial port support" depends on ARCH_QCOM || COMPILE_TEST diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index d056ee6..7b6422e 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_SERIAL_VR41XX) += vr41xx_siu.o obj-$(CONFIG_SERIAL_ATMEL) += atmel_serial.o obj-$(CONFIG_SERIAL_UARTLITE) += uartlite.o obj-$(CONFIG_SERIAL_MSM) += msm_serial.o +obj-$(CONFIG_SERIAL_MSM_GENI_EARLY_CONSOLE) += msm_geni_serial_console.o obj-$(CONFIG_SERIAL_QCOM_GENI) += qcom_geni_serial.o obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o diff --git a/drivers/tty/serial/msm_geni_serial_console.c b/drivers/tty/serial/msm_geni_serial_console.c new file mode 100644 index 000..80a7231 --- /dev/null +++ b/drivers/tty/serial/msm_geni_serial_console.c @@ -0,0 +1,480 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2020, The Linux Foundation. All rights reserved. + */ +#include +#include +#include +#include + +#define SE_UART_TX_TRANS_CFG (0x25C) +#define SE_UART_TX_WORD_LEN(0x268) +#define SE_UART_TX_STOP_BIT_LEN(0x26C) +#define SE_UART_TX_TRANS_LEN (0x270) +#define SE_UART_TX_PARITY_CFG (0x2A4) +/* SE_UART_TRANS_CFG */ +#define UART_CTS_MASK (BIT(1)) +/* UART M_CMD OP codes */ +#define UART_START_TX (0x1) + +#define UART_OVERSAMPLING (32) +#define DEF_FIFO_DEPTH_WORDS (16) +#define DEF_TX_WM (2) +#define DEF_FIFO_WIDTH_BITS(32) + +#define GENI_FORCE_DEFAULT_REG (0x20) +#define GENI_OUTPUT_CTRL (0x24) +#define GENI_CGC_CTRL (0x28) +#define GENI_SER_M_CLK_CFG (0x48) +#define GENI_FW_REVISION_RO(0x68) + +#define SE_GENI_TX_PACKING_CFG0(0x260) +#define SE_GENI_TX_PACKING_CFG1(0x264) +#define SE_GENI_M_CMD0 (0x600) +#define SE_GENI_M_CMD_CTRL_REG (0x604) +#define SE_GENI_M_IRQ_STATUS (0x610) +#define SE_GENI_M_IRQ_EN (0x614) +#define SE_GENI_M_IRQ_CLEAR(0x618) +#define SE_GENI_TX_FIFOn (0x700) +#define SE_GENI_TX_WATERMARK_REG (0x80C) + +#define SE_IRQ_EN (0xE1C) +#define SE_HW_PARAM_0 (0xE24) +#define SE_HW_PARAM_1 (0xE28) + +/* GENI_OUTPUT_CTRL fields */ +#define DEFAULT_IO_OUTPUT_CTRL_MSK (GENMASK(6, 0)) + +/* GENI_FORCE_DEFAULT_REG fields */ +#define FORCE_DEFAULT (BIT(0)) + +/* GENI_CGC_CTRL fields */ +#define CFG_AHB_CLK_CGC_ON (BIT(0)) +#define CFG_AHB_WR_ACLK_CGC_ON (BIT(1)) +#define DATA_AHB_CLK_CGC_ON(BIT(2)) +#define SCLK_CGC_ON(BIT(3)) +#define TX_CLK_CGC_ON (BIT(4)) +#define RX_CLK_CGC_ON (BIT(5)) +#define EXT_CLK_CGC_ON (BIT(6)) +#define PROG_RAM_HCLK_OFF (BIT(8)) +#define PROG_RAM_SCLK_OFF (BIT(9)) +#define DEFAULT_CGC_EN (GENMASK(6, 0)) + +/* GENI_STATUS fields */ +#define M_GENI_CMD_
[PATCH V7 1/3] dt-bindings: geni-se: Convert QUP geni-se bindings to YAML
Convert QUP geni-se bindings to DT schema format using json-schema. Signed-off-by: Akash Asthana Reviewed-by: Rob Herring Reviewed-by: Stephen Boyd --- Changes in V2: - As per Stephen's comment corrected defintion of interrupts for UART node. Any valid UART node must contain atleast 1 interrupts. Changes in V3: - As per Rob's comment, added number of reg entries for reg property. - As per Rob's comment, corrected unit address to hex. - As per Rob's comment, created a pattern which matches everything common to geni based I2C, SPI and UART controller and then one pattern for each. - As per Rob's comment, restored original example. Changes in V4: - Resolve below compilation error reported from bot. /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/ qcom,geni-se.yaml: properties:clocks:minItems: False schema does not allow 2 /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/ qcom,geni-se.yaml: properties:clocks:maxItems: False schema does not allow 2 Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/soc/qcom/ qcom,geni-se.example.dts] Error 1 Makefile:1263: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 Changes in V6: - Added reg entry for soc@0 example node to address below warning. Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dts:22.20-60.11 : Warning (unit_address_vs_reg): /example-0/soc@0: node has a unit name, but no reg or ranges property Changes in V7: - No change. .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 94 - .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 210 + 2 files changed, 210 insertions(+), 94 deletions(-) delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt deleted file mode 100644 index dab7ca9..000 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt +++ /dev/null @@ -1,94 +0,0 @@ -Qualcomm Technologies, Inc. GENI Serial Engine QUP Wrapper Controller - -Generic Interface (GENI) based Qualcomm Universal Peripheral (QUP) wrapper -is a programmable module for supporting a wide range of serial interfaces -like UART, SPI, I2C, I3C, etc. A single QUP module can provide upto 8 Serial -Interfaces, using its internal Serial Engines. The GENI Serial Engine QUP -Wrapper controller is modeled as a node with zero or more child nodes each -representing a serial engine. - -Required properties: -- compatible: Must be "qcom,geni-se-qup". -- reg: Must contain QUP register address and length. -- clock-names: Must contain "m-ahb" and "s-ahb". -- clocks: AHB clocks needed by the device. - -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 - -Properties for children: - -A GENI based QUP wrapper controller node can contain 0 or more child nodes -representing serial devices. These serial devices can be a QCOM UART, I2C -controller, SPI controller, or some combination of aforementioned devices. -Please refer below the child node definitions for the supported serial -interface protocols. - -Qualcomm Technologies Inc. GENI Serial Engine based I2C Controller - -Required properties: -- compatible: Must be "qcom,geni-i2c". -- reg: Must contain QUP register address and length. -- interrupts: Must contain I2C interrupt. -- clock-names: Must contain "se". -- clocks: Serial engine core clock needed by the device. -- #address-cells: Must be <1> for I2C device address. -- #size-cells: Must be <0> as I2C addresses have no size component. - -Optional property: -- clock-frequency: Desired I2C bus clock frequency in Hz. - When missing default to 10Hz. - -Child nodes should conform to I2C bus binding as described in i2c.txt. - -Qualcomm Technologies Inc. GENI Serial Engine based UART Controller - -Required properties: -- compatible: Must be "qcom,geni-debug-uart" or "qcom,geni-uart". -- reg: Must contain UART register location and length. -- interrupts: Must contain UART core interrupts. -- clock-names: Must contain "se". -- clocks: Serial engine core clock needed by the device. - -Qualcomm Technologies Inc. GENI Serial Engine based SPI Controller -node binding is
[PATCH V7 3/3] dt-bindings: serial: Add binding for UART pin swap
Add documentation to support RX-TX & CTS-RTS GPIO pin swap in HW. Signed-off-by: Akash Asthana Reviewed-by: Stephen Boyd --- Changes in V7: - As per Rob's comment, added type: boolean to properties. Documentation/devicetree/bindings/serial/serial.yaml | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml index 53204d9..8645d0e 100644 --- a/Documentation/devicetree/bindings/serial/serial.yaml +++ b/Documentation/devicetree/bindings/serial/serial.yaml @@ -67,6 +67,14 @@ properties: (wired and enabled by pinmux configuration). This depends on both the UART hardware and the board wiring. + rx-tx-swap: +type: boolean +description: RX and TX pins are swapped. + + cts-rts-swap: +type: boolean +description: CTS and RTS pins are swapped. + if: required: - uart-has-rtscts -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V7 2/3] dt-bindings: geni-se: Add interconnect binding for GENI QUP
Add documentation for the interconnect and interconnect-names properties for the GENI QUP. Signed-off-by: Akash Asthana Reviewed-by: Stephen Boyd --- Changes in V5: - Add interconnect property for QUP wrapper (parent node). - Add minItems = 2 for interconnect property in child nodes Changes in V6: - As per Rob's comment added minItems = 2 for interconnect-names. Changes in V7: - No change. .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml index 885966f..b19505b 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml @@ -46,6 +46,12 @@ properties: ranges: true + interconnects: +maxItems: 1 + + interconnect-names: +const: qup-core + required: - compatible - reg @@ -73,6 +79,17 @@ patternProperties: description: Serial engine core clock needed by the device. maxItems: 1 + interconnects: + minItems: 2 + maxItems: 3 + + interconnect-names: + minItems: 2 + items: + - const: qup-core + - const: qup-config + - const: qup-memory + required: - reg - clock-names -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V7 0/3] Convert QUP bindings to YAML and add ICC, pin swap doc
Changes in V6: - As per Rob's suggestion moved pin swap documentation from QUP to serial.yaml file[PATCH V6 3/3]. Changes in V4: - Add interconnect binding patch. - Add UART pin swap binding patch. Akash Asthana (3): dt-bindings: geni-se: Convert QUP geni-se bindings to YAML dt-bindings: geni-se: Add interconnect binding for GENI QUP dt-bindings: serial: Add binding for UART pin swap .../devicetree/bindings/serial/serial.yaml | 6 + .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 94 - .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 227 + 3 files changed, 233 insertions(+), 94 deletions(-) delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH V6 3/3] dt-bindings: serial: Add binding for UART pin swap
Hi Rob, On 5/15/2020 8:31 AM, Rob Herring wrote: On Thu, May 07, 2020 at 08:30:47PM +0530, Akash Asthana wrote: Add documentation to support RX-TX & CTS-RTS GPIO pin swap in HW. Signed-off-by: Akash Asthana --- Documentation/devicetree/bindings/serial/serial.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml index 53204d9..e657dd6 100644 --- a/Documentation/devicetree/bindings/serial/serial.yaml +++ b/Documentation/devicetree/bindings/serial/serial.yaml @@ -67,6 +67,12 @@ properties: (wired and enabled by pinmux configuration). This depends on both the UART hardware and the board wiring. + rx-tx-swap: +description: RX and TX pins are swapped. + + cts-rts-swap: +description: CTS and RTS pins are swapped. Need 'type: boolean' on both of these. okay, will correct in next version Regards, Akash + if: required: - uart-has-rtscts -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH V6 0/3] Convert QUP bindings to YAML and add ICC, pin swap doc
Hi Stephen, On 5/14/2020 7:33 AM, Stephen Boyd wrote: Quoting Akash Asthana (2020-05-07 08:00:44) Changes in V6: - As per Rob's suggestion moved pin swap documentation from QUP to serial.yaml file[PATCH V6 3/3]. Changes in V4: - Add interconnect binding patch. - Add UART pin swap binding patch. Akash Asthana (3): dt-bindings: geni-se: Convert QUP geni-se bindings to YAML dt-bindings: geni-se: Add interconnect binding for GENI QUP dt-bindings: serial: Add binding for UART pin swap Who do you intend to pick up these patches? Rob or Greg? I suppose if it's all in bindings then maybe Rob can pick them up. I intended Rob to pick these patches but Greg is maintainer of serial.yaml file so I send patches to him as well, I thought I would be needing ack from him. But from your reply it's clear that I should be sending to Rob only. Regards, Akash -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V6 2/3] dt-bindings: geni-se: Add interconnect binding for GENI QUP
Add documentation for the interconnect and interconnect-names properties for the GENI QUP. Signed-off-by: Akash Asthana --- Changes in V5: - Add interconnect property for QUP wrapper (parent node). - Add minItems = 2 for interconnect property in child nodes Changes in V6: - As per Rob's comment added minItems = 2 for interconnect-names. .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml index 885966f..b19505b 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml @@ -46,6 +46,12 @@ properties: ranges: true + interconnects: +maxItems: 1 + + interconnect-names: +const: qup-core + required: - compatible - reg @@ -73,6 +79,17 @@ patternProperties: description: Serial engine core clock needed by the device. maxItems: 1 + interconnects: + minItems: 2 + maxItems: 3 + + interconnect-names: + minItems: 2 + items: + - const: qup-core + - const: qup-config + - const: qup-memory + required: - reg - clock-names -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V6 3/3] dt-bindings: serial: Add binding for UART pin swap
Add documentation to support RX-TX & CTS-RTS GPIO pin swap in HW. Signed-off-by: Akash Asthana --- Documentation/devicetree/bindings/serial/serial.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml index 53204d9..e657dd6 100644 --- a/Documentation/devicetree/bindings/serial/serial.yaml +++ b/Documentation/devicetree/bindings/serial/serial.yaml @@ -67,6 +67,12 @@ properties: (wired and enabled by pinmux configuration). This depends on both the UART hardware and the board wiring. + rx-tx-swap: +description: RX and TX pins are swapped. + + cts-rts-swap: +description: CTS and RTS pins are swapped. + if: required: - uart-has-rtscts -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V6 1/3] dt-bindings: geni-se: Convert QUP geni-se bindings to YAML
Convert QUP geni-se bindings to DT schema format using json-schema. Signed-off-by: Akash Asthana Reviewed-by: Rob Herring Reviewed-by: Stephen Boyd --- Changes in V2: - As per Stephen's comment corrected defintion of interrupts for UART node. Any valid UART node must contain atleast 1 interrupts. Changes in V3: - As per Rob's comment, added number of reg entries for reg property. - As per Rob's comment, corrected unit address to hex. - As per Rob's comment, created a pattern which matches everything common to geni based I2C, SPI and UART controller and then one pattern for each. - As per Rob's comment, restored original example. Changes in V4: - Resolve below compilation error reported from bot. /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/ qcom,geni-se.yaml: properties:clocks:minItems: False schema does not allow 2 /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/ qcom,geni-se.yaml: properties:clocks:maxItems: False schema does not allow 2 Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/soc/qcom/ qcom,geni-se.example.dts] Error 1 Makefile:1263: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 Changes in V6: - Added reg entry for soc@0 example node to address below warning. Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dts:22.20-60.11 : Warning (unit_address_vs_reg): /example-0/soc@0: node has a unit name, but no reg or ranges property .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 94 - .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 210 + 2 files changed, 210 insertions(+), 94 deletions(-) delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt deleted file mode 100644 index dab7ca9..000 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt +++ /dev/null @@ -1,94 +0,0 @@ -Qualcomm Technologies, Inc. GENI Serial Engine QUP Wrapper Controller - -Generic Interface (GENI) based Qualcomm Universal Peripheral (QUP) wrapper -is a programmable module for supporting a wide range of serial interfaces -like UART, SPI, I2C, I3C, etc. A single QUP module can provide upto 8 Serial -Interfaces, using its internal Serial Engines. The GENI Serial Engine QUP -Wrapper controller is modeled as a node with zero or more child nodes each -representing a serial engine. - -Required properties: -- compatible: Must be "qcom,geni-se-qup". -- reg: Must contain QUP register address and length. -- clock-names: Must contain "m-ahb" and "s-ahb". -- clocks: AHB clocks needed by the device. - -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 - -Properties for children: - -A GENI based QUP wrapper controller node can contain 0 or more child nodes -representing serial devices. These serial devices can be a QCOM UART, I2C -controller, SPI controller, or some combination of aforementioned devices. -Please refer below the child node definitions for the supported serial -interface protocols. - -Qualcomm Technologies Inc. GENI Serial Engine based I2C Controller - -Required properties: -- compatible: Must be "qcom,geni-i2c". -- reg: Must contain QUP register address and length. -- interrupts: Must contain I2C interrupt. -- clock-names: Must contain "se". -- clocks: Serial engine core clock needed by the device. -- #address-cells: Must be <1> for I2C device address. -- #size-cells: Must be <0> as I2C addresses have no size component. - -Optional property: -- clock-frequency: Desired I2C bus clock frequency in Hz. - When missing default to 10Hz. - -Child nodes should conform to I2C bus binding as described in i2c.txt. - -Qualcomm Technologies Inc. GENI Serial Engine based UART Controller - -Required properties: -- compatible: Must be "qcom,geni-debug-uart" or "qcom,geni-uart". -- reg: Must contain UART register location and length. -- interrupts: Must contain UART core interrupts. -- clock-names: Must contain "se". -- clocks: Serial engine core clock needed by the device. - -Qualcomm Technologies Inc. GENI Serial Engine based SPI Controller -node binding is described in -Documentation/devicet
Re: [PATCH V5 2/3] dt-bindings: geni-se: Add interconnect binding for GENI QUP
Hi Rob, + interconnects: + minItems: 2 + maxItems: 3 + + interconnect-names: + items: + - const: qup-core + - const: qup-config + - const: qup-memory Don't you need 'minItems: 2' here? Yeah I need minItems: 2 here, thanks for reviewing. regards, Akash Rob -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
[PATCH V6 0/3] Convert QUP bindings to YAML and add ICC, pin swap doc
Changes in V6: - As per Rob's suggestion moved pin swap documentation from QUP to serial.yaml file[PATCH V6 3/3]. Changes in V4: - Add interconnect binding patch. - Add UART pin swap binding patch. Akash Asthana (3): dt-bindings: geni-se: Convert QUP geni-se bindings to YAML dt-bindings: geni-se: Add interconnect binding for GENI QUP dt-bindings: serial: Add binding for UART pin swap .../devicetree/bindings/serial/serial.yaml | 6 + .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 94 - .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 227 + 3 files changed, 233 insertions(+), 94 deletions(-) delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH] interconnect: Add helpers for enabling/disabling a path
Hi Georgi, On 4/28/2020 2:46 PM, Georgi Djakov wrote: There is a repeated pattern in multiple drivers where they want to switch the bandwidth between zero and some other value. This is happening often in the suspend/resume callbacks. Let's add helper functions to enable and disable the path, so that callers don't have to take care of remembering the bandwidth values and handle this in the framework instead. With this patch the users can call icc_disable() and icc_enable() to lower their bandwidth request to zero and then restore it back to it's previous value. Thanks for this patch. Are you planning to add bulk versions of icc_enable/disable APIs? Regards, Akash -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Re: [PATCH V5 3/3] dt-bindings: geni-se: Add binding for UART pin swap
Hi Matthias Thanks for notifying, I will make the changes. Regards, Akash On 4/25/2020 2:27 AM, Matthias Kaehlcke wrote: Hi Akash, On Tue, Mar 24, 2020 at 10:46:40AM +0530, Akash Asthana wrote: Hi Rob, On 3/20/2020 11:37 PM, Rob Herring wrote: On Fri, Mar 13, 2020 at 4:29 AM Akash Asthana wrote: Add documentation to support RX/TX/CTS/RTS pin swap in HW. Signed-off-by: Akash Asthana --- Changes in V5: - As per Matthias's comment, remove rx-tx-cts-rts-swap property from UART child node. Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 6 ++ 1 file changed, 6 insertions(+) STM32 folks need something similar. Can you move this to a common location. That's serial.txt, but that is being converted to DT schema. Rob Okay, once serial.txt is converted to DT schema, I will move it there. It has landed upstream: 175a7427bb72 dt-bindings: serial: Convert generic bindings to json-schema -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
