This is version 2 of the GPI dma series [1]
This series add the GPI DMA in qcom geni drivers. For this we
first need to move GENI_IF_DISABLE_RO and struct geni_wrapper to common
headers and then add support in the gpi dma in geni driver.
The SPI and I2C driver changes shall follow shortly
[1]: http://lore.kernel.org/r/[email protected]
Changes in v2:
- add r-b from Bjorn on patch 1
- add more details in log for patch 2
- Fix the comments from Doug and Bjorn for patch3, notable use read, modify
update for irq registers, use geni_se_irq_clear() for irq, dont update
single bit registers, add a bit more description for gpi dma etc
Vinod Koul (3):
soc: qcom: geni: move GENI_IF_DISABLE_RO to common header
soc: qcom: geni: move struct geni_wrapper to header
soc: qcom: geni: Add support for gpi dma
drivers/soc/qcom/qcom-geni-se.c | 47 ++++++++++++++++++++++-----------
include/linux/qcom-geni-se.h | 23 ++++++++++++++--
2 files changed, 52 insertions(+), 18 deletions(-)
--
2.31.1
SPI & I2C geni driver needs to access struct geni_wrapper, so move it to
header. The drivers needs this header to find the geni device and use it
in dma mapping.
Using this method works for both DT and ACPI systems
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/soc/qcom/qcom-geni-se.c | 14 --------------
include/linux/qcom-geni-se.h | 14 ++++++++++++++
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index fe666ea0c487..08d645b90ed3 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -78,20 +78,6 @@
*/
#define MAX_CLK_PERF_LEVEL 32
-#define NUM_AHB_CLKS 2
-
-/**
- * struct geni_wrapper - Data structure to represent the QUP Wrapper Core
- * @dev: Device pointer of the QUP wrapper core
- * @base: Base address of this instance of QUP wrapper core
- * @ahb_clks: Handle to the primary & secondary AHB clocks
- * @to_core: Core ICC path
- */
-struct geni_wrapper {
- struct device *dev;
- void __iomem *base;
- struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
-};
static const char * const icc_path_names[] = {"qup-core", "qup-config",
"qup-memory"};
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 5114e2144b17..5fda675c5cfe 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -38,6 +38,20 @@ struct geni_icc_path {
unsigned int avg_bw;
};
+#define NUM_AHB_CLKS 2
+
+/**
+ * @struct geni_wrapper - Data structure to represent the QUP Wrapper Core
+ * @dev: Device pointer of the QUP wrapper core
+ * @base: Base address of this instance of QUP wrapper core
+ * @ahb_clks: Handle to the primary & secondary AHB clocks
+ */
+struct geni_wrapper {
+ struct device *dev;
+ void __iomem *base;
+ struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
+};
+
/**
* struct geni_se - GENI Serial Engine
* @base: Base Address of the Serial Engine's register block
--
2.31.1
GENI_IF_DISABLE_RO is used by geni spi driver as well to check the
status if GENI, so move this to common header qcom-geni-se.h
Also, add FIFO_IF_DISABLE define.
Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/soc/qcom/qcom-geni-se.c | 1 -
include/linux/qcom-geni-se.h | 4 ++++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 5bdfb1565c14..fe666ea0c487 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -104,7 +104,6 @@ static const char * const icc_path_names[] = {"qup-core", "qup-config",
#define GENI_OUTPUT_CTRL 0x24
#define GENI_CGC_CTRL 0x28
#define GENI_CLK_CTRL_RO 0x60
-#define GENI_IF_DISABLE_RO 0x64
#define GENI_FW_S_REVISION_RO 0x6c
#define SE_GENI_BYTE_GRAN 0x254
#define SE_GENI_TX_PACKING_CFG0 0x260
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 7c811eebcaab..5114e2144b17 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -63,6 +63,7 @@ struct geni_se {
#define SE_GENI_STATUS 0x40
#define GENI_SER_M_CLK_CFG 0x48
#define GENI_SER_S_CLK_CFG 0x4c
+#define GENI_IF_DISABLE_RO 0x64
#define GENI_FW_REVISION_RO 0x68
#define SE_GENI_CLK_SEL 0x7c
#define SE_GENI_DMA_MODE_EN 0x258
@@ -105,6 +106,9 @@ struct geni_se {
#define CLK_DIV_MSK GENMASK(15, 4)
#define CLK_DIV_SHFT 4
+/* GENI_IF_DISABLE_RO fields */
+#define FIFO_IF_DISABLE (BIT(0))
+
/* GENI_FW_REVISION_RO fields */
#define FW_REV_PROTOCOL_MSK GENMASK(15, 8)
#define FW_REV_PROTOCOL_SHFT 8
--
2.31.1
On Fri 18 Jun 09:18 CDT 2021, Vinod Koul wrote:
> This is version 2 of the GPI dma series [1]
>
> This series add the GPI DMA in qcom geni drivers. For this we
> first need to move GENI_IF_DISABLE_RO and struct geni_wrapper to common
> headers and then add support in the gpi dma in geni driver.
>
> The SPI and I2C driver changes shall follow shortly
>
Given that the continuation of this series will have build time
dependencies on these patches I think it would be good to see them all
as one chunk (and practically I can create a immutable branch of the
soc/qcom pieces and share with Wolfram and Mark).
Regards,
Bjorn
> [1]: http://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - add r-b from Bjorn on patch 1
> - add more details in log for patch 2
> - Fix the comments from Doug and Bjorn for patch3, notable use read, modify
> update for irq registers, use geni_se_irq_clear() for irq, dont update
> single bit registers, add a bit more description for gpi dma etc
>
> Vinod Koul (3):
> soc: qcom: geni: move GENI_IF_DISABLE_RO to common header
> soc: qcom: geni: move struct geni_wrapper to header
> soc: qcom: geni: Add support for gpi dma
>
> drivers/soc/qcom/qcom-geni-se.c | 47 ++++++++++++++++++++++-----------
> include/linux/qcom-geni-se.h | 23 ++++++++++++++--
> 2 files changed, 52 insertions(+), 18 deletions(-)
>
> --
> 2.31.1
>
On Fri 18 Jun 09:18 CDT 2021, Vinod Koul wrote:
> GENI_IF_DISABLE_RO is used by geni spi driver as well to check the
> status if GENI, so move this to common header qcom-geni-se.h
>
> Also, add FIFO_IF_DISABLE define.
>
Afaict these registers relates to the hardware block that is primarily
owned by the individual engine-drivers, would it not make sense to move
them all to the shared header file?
(The patch itself still looks ok, but needs the consuming patch to be
merged)
Regards,
Bjorn
> Reviewed-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 1 -
> include/linux/qcom-geni-se.h | 4 ++++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index 5bdfb1565c14..fe666ea0c487 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -104,7 +104,6 @@ static const char * const icc_path_names[] = {"qup-core", "qup-config",
> #define GENI_OUTPUT_CTRL 0x24
> #define GENI_CGC_CTRL 0x28
> #define GENI_CLK_CTRL_RO 0x60
> -#define GENI_IF_DISABLE_RO 0x64
> #define GENI_FW_S_REVISION_RO 0x6c
> #define SE_GENI_BYTE_GRAN 0x254
> #define SE_GENI_TX_PACKING_CFG0 0x260
> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
> index 7c811eebcaab..5114e2144b17 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -63,6 +63,7 @@ struct geni_se {
> #define SE_GENI_STATUS 0x40
> #define GENI_SER_M_CLK_CFG 0x48
> #define GENI_SER_S_CLK_CFG 0x4c
> +#define GENI_IF_DISABLE_RO 0x64
> #define GENI_FW_REVISION_RO 0x68
> #define SE_GENI_CLK_SEL 0x7c
> #define SE_GENI_DMA_MODE_EN 0x258
> @@ -105,6 +106,9 @@ struct geni_se {
> #define CLK_DIV_MSK GENMASK(15, 4)
> #define CLK_DIV_SHFT 4
>
> +/* GENI_IF_DISABLE_RO fields */
> +#define FIFO_IF_DISABLE (BIT(0))
> +
> /* GENI_FW_REVISION_RO fields */
> #define FW_REV_PROTOCOL_MSK GENMASK(15, 8)
> #define FW_REV_PROTOCOL_SHFT 8
> --
> 2.31.1
>
On Fri 18 Jun 09:18 CDT 2021, Vinod Koul wrote:
> SPI & I2C geni driver needs to access struct geni_wrapper, so move it to
> header. The drivers needs this header to find the geni device and use it
> in dma mapping.
>
How does this differ from engine->dev->parent?
> Using this method works for both DT and ACPI systems
>
I was under the impression that the wrapper and engines are describe
completely independently in ACPI, so we don't have a link between them.
If that's not the case, then I guess that answers the above question
about ->parent.
Regards,
Bjorn
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> drivers/soc/qcom/qcom-geni-se.c | 14 --------------
> include/linux/qcom-geni-se.h | 14 ++++++++++++++
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
> index fe666ea0c487..08d645b90ed3 100644
> --- a/drivers/soc/qcom/qcom-geni-se.c
> +++ b/drivers/soc/qcom/qcom-geni-se.c
> @@ -78,20 +78,6 @@
> */
>
> #define MAX_CLK_PERF_LEVEL 32
> -#define NUM_AHB_CLKS 2
> -
> -/**
> - * struct geni_wrapper - Data structure to represent the QUP Wrapper Core
> - * @dev: Device pointer of the QUP wrapper core
> - * @base: Base address of this instance of QUP wrapper core
> - * @ahb_clks: Handle to the primary & secondary AHB clocks
> - * @to_core: Core ICC path
> - */
> -struct geni_wrapper {
> - struct device *dev;
> - void __iomem *base;
> - struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> -};
>
> static const char * const icc_path_names[] = {"qup-core", "qup-config",
> "qup-memory"};
> diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
> index 5114e2144b17..5fda675c5cfe 100644
> --- a/include/linux/qcom-geni-se.h
> +++ b/include/linux/qcom-geni-se.h
> @@ -38,6 +38,20 @@ struct geni_icc_path {
> unsigned int avg_bw;
> };
>
> +#define NUM_AHB_CLKS 2
> +
> +/**
> + * @struct geni_wrapper - Data structure to represent the QUP Wrapper Core
> + * @dev: Device pointer of the QUP wrapper core
> + * @base: Base address of this instance of QUP wrapper core
> + * @ahb_clks: Handle to the primary & secondary AHB clocks
> + */
> +struct geni_wrapper {
> + struct device *dev;
> + void __iomem *base;
> + struct clk_bulk_data ahb_clks[NUM_AHB_CLKS];
> +};
> +
> /**
> * struct geni_se - GENI Serial Engine
> * @base: Base Address of the Serial Engine's register block
> --
> 2.31.1
>
GPI DMA is one of the DMA modes supported on geni, this adds support to
enable that mode
Signed-off-by: Vinod Koul <[email protected]>
---
drivers/soc/qcom/qcom-geni-se.c | 32 +++++++++++++++++++++++++++++++-
include/linux/qcom-geni-se.h | 5 +++--
2 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 08d645b90ed3..40a0a1f88070 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -307,6 +307,32 @@ static void geni_se_select_dma_mode(struct geni_se *se)
writel_relaxed(val, se->base + SE_GENI_DMA_MODE_EN);
}
+static void geni_se_select_gpi_mode(struct geni_se *se)
+{
+ unsigned int gpi_event_en;
+ unsigned int m_irq_en;
+ unsigned int s_irq_en;
+
+ geni_se_irq_clear(se);
+ writel(0, se->base + SE_IRQ_EN);
+
+ s_irq_en = readl_relaxed(se->base + SE_GENI_S_IRQ_EN);
+ s_irq_en &= ~S_CMD_DONE_EN;
+ writel(s_irq_en, se->base + SE_GENI_S_IRQ_EN);
+
+ m_irq_en = readl(se->base + SE_GENI_M_IRQ_EN);
+ m_irq_en &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN |
+ M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
+ writel(m_irq_en, se->base + SE_GENI_M_IRQ_EN);
+
+ writel(GENI_DMA_MODE_EN, se->base + SE_GENI_DMA_MODE_EN);
+
+ gpi_event_en = readl(se->base + SE_GSI_EVENT_EN);
+ gpi_event_en |= (DMA_RX_EVENT_EN | DMA_TX_EVENT_EN |
+ GENI_M_EVENT_EN | GENI_S_EVENT_EN);
+ writel(gpi_event_en, se->base + SE_GSI_EVENT_EN);
+}
+
/**
* geni_se_select_mode() - Select the serial engine transfer mode
* @se: Pointer to the concerned serial engine.
@@ -314,7 +340,8 @@ static void geni_se_select_dma_mode(struct geni_se *se)
*/
void geni_se_select_mode(struct geni_se *se, enum geni_se_xfer_mode mode)
{
- WARN_ON(mode != GENI_SE_FIFO && mode != GENI_SE_DMA);
+ WARN_ON(mode != GENI_SE_FIFO && mode != GENI_SE_DMA &&
+ mode != GENI_GPI_DMA);
switch (mode) {
case GENI_SE_FIFO:
@@ -323,6 +350,9 @@ void geni_se_select_mode(struct geni_se *se, enum geni_se_xfer_mode mode)
case GENI_SE_DMA:
geni_se_select_dma_mode(se);
break;
+ case GENI_GPI_DMA:
+ geni_se_select_gpi_mode(se);
+ break;
case GENI_SE_INVALID:
default:
break;
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index 5fda675c5cfe..336b682392b1 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -11,8 +11,9 @@
/* Transfer mode supported by GENI Serial Engines */
enum geni_se_xfer_mode {
GENI_SE_INVALID,
- GENI_SE_FIFO,
- GENI_SE_DMA,
+ GENI_SE_FIFO, /* FIFO mode */
+ GENI_GPI_DMA, /* GSI aka GPI DMA mode */
+ GENI_SE_DMA, /* SE DMA mode */
};
/* Protocols supported by GENI Serial Engines */
--
2.31.1
On 18-06-21, 11:59, Bjorn Andersson wrote:
> On Fri 18 Jun 09:18 CDT 2021, Vinod Koul wrote:
>
> > This is version 2 of the GPI dma series [1]
> >
> > This series add the GPI DMA in qcom geni drivers. For this we
> > first need to move GENI_IF_DISABLE_RO and struct geni_wrapper to common
> > headers and then add support in the gpi dma in geni driver.
> >
> > The SPI and I2C driver changes shall follow shortly
> >
>
> Given that the continuation of this series will have build time
> dependencies on these patches I think it would be good to see them all
> as one chunk (and practically I can create a immutable branch of the
> soc/qcom pieces and share with Wolfram and Mark).
Okay, let me post full series in few days
>
> Regards,
> Bjorn
>
> > [1]: http://lore.kernel.org/r/[email protected]
> >
> > Changes in v2:
> > - add r-b from Bjorn on patch 1
> > - add more details in log for patch 2
> > - Fix the comments from Doug and Bjorn for patch3, notable use read, modify
> > update for irq registers, use geni_se_irq_clear() for irq, dont update
> > single bit registers, add a bit more description for gpi dma etc
> >
> > Vinod Koul (3):
> > soc: qcom: geni: move GENI_IF_DISABLE_RO to common header
> > soc: qcom: geni: move struct geni_wrapper to header
> > soc: qcom: geni: Add support for gpi dma
> >
> > drivers/soc/qcom/qcom-geni-se.c | 47 ++++++++++++++++++++++-----------
> > include/linux/qcom-geni-se.h | 23 ++++++++++++++--
> > 2 files changed, 52 insertions(+), 18 deletions(-)
> >
> > --
> > 2.31.1
> >
--
~Vinod
On 18-06-21, 12:03, Bjorn Andersson wrote:
> On Fri 18 Jun 09:18 CDT 2021, Vinod Koul wrote:
>
> > GENI_IF_DISABLE_RO is used by geni spi driver as well to check the
> > status if GENI, so move this to common header qcom-geni-se.h
> >
> > Also, add FIFO_IF_DISABLE define.
> >
>
> Afaict these registers relates to the hardware block that is primarily
> owned by the individual engine-drivers, would it not make sense to move
> them all to the shared header file?
the GENI_IF_DISABLE_RO is used by SPI and I2C drivers, so we would create two
copies. So better to be defined in geni header
--
~Vinod