* [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops
2023-03-03 12:46 [PATCH v5 0/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
@ 2023-03-03 12:46 ` Cai Huoqing
2023-03-03 16:51 ` Serge Semin
2023-03-03 12:46 ` [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation Cai Huoqing
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Cai Huoqing @ 2023-03-03 12:46 UTC (permalink / raw)
To: fancer.lancer
Cc: Cai huoqing, Gustavo Pimentel, Vinod Koul, Jingoo Han,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, linux-kernel, dmaengine, linux-pci
From: Cai huoqing <cai.huoqing@linux.dev>
Rename dw_edma_core_ops structure to dw_edma_plat_ops, the ops is platform
specific operations: the DMA device environment configs like IRQs,
address translation, etc.
The dw_edma_plat_ops name was supposed to refer to the platform which
the DW eDMA engine is embedded to, like PCIe end-point (accessible via
the PCIe bus) or a PCIe root port (directly accessible by CPU).
Needless to say that for them the IRQ-vector and PCI-addresses are
differently determined. The suggested name has a connection with the
kernel platform device only as a private case of the eDMA/hDMA embedded
into the DW PCI Root ports, though basically it was supposed to refer to
any platform in which the DMA hardware lives.
Anyway the renaming was necessary to distinguish two types of
the implementation callbacks:
1. DW eDMA/hDMA IP-core specific operations: device-specific CSR
setups in one or another aspect of the DMA-engine initialization.
2. DW eDMA/hDMA platform specific operations: the DMA device
environment configs like IRQs, address translation, etc.
dw_edma_core_ops is supposed to be used for the case 1, and
dw_edma_plat_ops - for the case 2.
Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
---
v4->v5:
1.Revert the instance dw_edma_pcie_core_ops
2.Move the change EDMA_MF_HDMA_NATIVE to patch[3/4]
v4 link:
https://lore.kernel.org/lkml/20230221034656.14476-2-cai.huoqing@linux.dev/
drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
include/linux/dma/edma.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 2b40f2b44f5e..190b32d8016d 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
return region.start;
}
-static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
+static const struct dw_edma_plat_ops dw_edma_pcie_core_ops = {
.irq_vector = dw_edma_pcie_irq_vector,
.pci_address = dw_edma_pcie_address,
};
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 53a16b8b6ac2..44e90b71d429 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -828,7 +828,7 @@ static int dw_pcie_edma_irq_vector(struct device *dev, unsigned int nr)
return platform_get_irq_byname_optional(pdev, name);
}
-static struct dw_edma_core_ops dw_pcie_edma_ops = {
+static struct dw_edma_plat_ops dw_pcie_edma_ops = {
.irq_vector = dw_pcie_edma_irq_vector,
};
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index d2638d9259dc..ed401c965a87 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -40,7 +40,7 @@ struct dw_edma_region {
* iATU windows. That will be done by the controller
* automatically.
*/
-struct dw_edma_core_ops {
+struct dw_edma_plat_ops {
int (*irq_vector)(struct device *dev, unsigned int nr);
u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
};
@@ -80,7 +80,7 @@ enum dw_edma_chip_flags {
struct dw_edma_chip {
struct device *dev;
int nr_irqs;
- const struct dw_edma_core_ops *ops;
+ const struct dw_edma_plat_ops *ops;
u32 flags;
void __iomem *reg_base;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops
2023-03-03 12:46 ` [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops Cai Huoqing
@ 2023-03-03 16:51 ` Serge Semin
2023-03-06 13:31 ` Cai Huoqing
0 siblings, 1 reply; 16+ messages in thread
From: Serge Semin @ 2023-03-03 16:51 UTC (permalink / raw)
To: Cai Huoqing
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Fri, Mar 03, 2023 at 08:46:31PM +0800, Cai Huoqing wrote:
> From: Cai huoqing <cai.huoqing@linux.dev>
>
> Rename dw_edma_core_ops structure to dw_edma_plat_ops, the ops is platform
> specific operations: the DMA device environment configs like IRQs,
> address translation, etc.
>
> The dw_edma_plat_ops name was supposed to refer to the platform which
> the DW eDMA engine is embedded to, like PCIe end-point (accessible via
> the PCIe bus) or a PCIe root port (directly accessible by CPU).
> Needless to say that for them the IRQ-vector and PCI-addresses are
> differently determined. The suggested name has a connection with the
> kernel platform device only as a private case of the eDMA/hDMA embedded
> into the DW PCI Root ports, though basically it was supposed to refer to
> any platform in which the DMA hardware lives.
>
> Anyway the renaming was necessary to distinguish two types of
> the implementation callbacks:
> 1. DW eDMA/hDMA IP-core specific operations: device-specific CSR
> setups in one or another aspect of the DMA-engine initialization.
> 2. DW eDMA/hDMA platform specific operations: the DMA device
> environment configs like IRQs, address translation, etc.
>
> dw_edma_core_ops is supposed to be used for the case 1, and
> dw_edma_plat_ops - for the case 2.
This text was my explanation to Bjorn of why the renaming was
necessary. The patch log has a bit different context so I would
change it to something like this:
"The dw_edma_core_ops structure contains a set of the operations:
device IRQ numbers getter, CPU/PCI address translation. Based on the
functions semantics the structure name "dw_edma_plat_ops" looks more
descriptive since indeed the operations are platform-specific. The
"dw_edma_core_ops" name shall be used for a structure with the IP-core
specific set of callbacks in order to abstract out DW eDMA and DW HDMA
setups. Such structure will be added in one of the next commit in the
framework of the set of changes adding the DW HDMA device support."
>
> Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> ---
> v4->v5:
> 1.Revert the instance dw_edma_pcie_core_ops
> 2.Move the change EDMA_MF_HDMA_NATIVE to patch[3/4]
>
> v4 link:
> https://lore.kernel.org/lkml/20230221034656.14476-2-cai.huoqing@linux.dev/
>
> drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> include/linux/dma/edma.h | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 2b40f2b44f5e..190b32d8016d 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
> return region.start;
> }
>
> -static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> +static const struct dw_edma_plat_ops dw_edma_pcie_core_ops = {
Please carefully note my comment to v4. I asked to add the prefix
specific to the local naming convention:
-static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
+static const struct dw_edma_plat_ops dw_edma_pcie_plat_ops = {
But besides of adding the correct prefix you changed the suffix to the
improper one. Please get it back so the instance name would be
"dw_edma_pcie_plat_ops".
-Serge(y)
> .irq_vector = dw_edma_pcie_irq_vector,
> .pci_address = dw_edma_pcie_address,
> };
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..44e90b71d429 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -828,7 +828,7 @@ static int dw_pcie_edma_irq_vector(struct device *dev, unsigned int nr)
> return platform_get_irq_byname_optional(pdev, name);
> }
>
> -static struct dw_edma_core_ops dw_pcie_edma_ops = {
> +static struct dw_edma_plat_ops dw_pcie_edma_ops = {
> .irq_vector = dw_pcie_edma_irq_vector,
> };
>
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index d2638d9259dc..ed401c965a87 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -40,7 +40,7 @@ struct dw_edma_region {
> * iATU windows. That will be done by the controller
> * automatically.
> */
> -struct dw_edma_core_ops {
> +struct dw_edma_plat_ops {
> int (*irq_vector)(struct device *dev, unsigned int nr);
> u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
> };
> @@ -80,7 +80,7 @@ enum dw_edma_chip_flags {
> struct dw_edma_chip {
> struct device *dev;
> int nr_irqs;
> - const struct dw_edma_core_ops *ops;
> + const struct dw_edma_plat_ops *ops;
> u32 flags;
>
> void __iomem *reg_base;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops
2023-03-03 16:51 ` Serge Semin
@ 2023-03-06 13:31 ` Cai Huoqing
2023-03-06 14:51 ` Serge Semin
0 siblings, 1 reply; 16+ messages in thread
From: Cai Huoqing @ 2023-03-06 13:31 UTC (permalink / raw)
To: Serge Semin
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On 03 3月 23 19:51:25, Serge Semin wrote:
> On Fri, Mar 03, 2023 at 08:46:31PM +0800, Cai Huoqing wrote:
> > From: Cai huoqing <cai.huoqing@linux.dev>
> >
>
> > Rename dw_edma_core_ops structure to dw_edma_plat_ops, the ops is platform
> > specific operations: the DMA device environment configs like IRQs,
> > address translation, etc.
> >
> > The dw_edma_plat_ops name was supposed to refer to the platform which
> > the DW eDMA engine is embedded to, like PCIe end-point (accessible via
> > the PCIe bus) or a PCIe root port (directly accessible by CPU).
> > Needless to say that for them the IRQ-vector and PCI-addresses are
> > differently determined. The suggested name has a connection with the
> > kernel platform device only as a private case of the eDMA/hDMA embedded
> > into the DW PCI Root ports, though basically it was supposed to refer to
> > any platform in which the DMA hardware lives.
> >
> > Anyway the renaming was necessary to distinguish two types of
> > the implementation callbacks:
> > 1. DW eDMA/hDMA IP-core specific operations: device-specific CSR
> > setups in one or another aspect of the DMA-engine initialization.
> > 2. DW eDMA/hDMA platform specific operations: the DMA device
> > environment configs like IRQs, address translation, etc.
> >
> > dw_edma_core_ops is supposed to be used for the case 1, and
> > dw_edma_plat_ops - for the case 2.
>
> This text was my explanation to Bjorn of why the renaming was
> necessary. The patch log has a bit different context so I would
> change it to something like this:
>
> "The dw_edma_core_ops structure contains a set of the operations:
> device IRQ numbers getter, CPU/PCI address translation. Based on the
> functions semantics the structure name "dw_edma_plat_ops" looks more
> descriptive since indeed the operations are platform-specific. The
> "dw_edma_core_ops" name shall be used for a structure with the IP-core
> specific set of callbacks in order to abstract out DW eDMA and DW HDMA
> setups. Such structure will be added in one of the next commit in the
> framework of the set of changes adding the DW HDMA device support."
OK
>
> >
> > Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> > ---
> > v4->v5:
> > 1.Revert the instance dw_edma_pcie_core_ops
> > 2.Move the change EDMA_MF_HDMA_NATIVE to patch[3/4]
> >
> > v4 link:
> > https://lore.kernel.org/lkml/20230221034656.14476-2-cai.huoqing@linux.dev/
> >
> > drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
> > drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> > include/linux/dma/edma.h | 4 ++--
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 2b40f2b44f5e..190b32d8016d 100644
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
> > return region.start;
> > }
> >
>
> > -static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> > +static const struct dw_edma_plat_ops dw_edma_pcie_core_ops = {
>
> Please carefully note my comment to v4. I asked to add the prefix
> specific to the local naming convention:
>
> -static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> +static const struct dw_edma_plat_ops dw_edma_pcie_plat_ops = {
>
> But besides of adding the correct prefix you changed the suffix to the
> improper one. Please get it back so the instance name would be
> "dw_edma_pcie_plat_ops".
Do you mean ditto
- chip->ops = &dw_edma_pcie_core_ops;
+ chip->ops = &dw_edma_pcie_plat_ops;
>
> -Serge(y)
>
> > .irq_vector = dw_edma_pcie_irq_vector,
> > .pci_address = dw_edma_pcie_address,
> > };
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 53a16b8b6ac2..44e90b71d429 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -828,7 +828,7 @@ static int dw_pcie_edma_irq_vector(struct device *dev, unsigned int nr)
> > return platform_get_irq_byname_optional(pdev, name);
> > }
> >
> > -static struct dw_edma_core_ops dw_pcie_edma_ops = {
> > +static struct dw_edma_plat_ops dw_pcie_edma_ops = {
> > .irq_vector = dw_pcie_edma_irq_vector,
> > };
> >
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > index d2638d9259dc..ed401c965a87 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -40,7 +40,7 @@ struct dw_edma_region {
> > * iATU windows. That will be done by the controller
> > * automatically.
> > */
> > -struct dw_edma_core_ops {
> > +struct dw_edma_plat_ops {
> > int (*irq_vector)(struct device *dev, unsigned int nr);
> > u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
> > };
> > @@ -80,7 +80,7 @@ enum dw_edma_chip_flags {
> > struct dw_edma_chip {
> > struct device *dev;
> > int nr_irqs;
> > - const struct dw_edma_core_ops *ops;
> > + const struct dw_edma_plat_ops *ops;
> > u32 flags;
> >
> > void __iomem *reg_base;
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops
2023-03-06 13:31 ` Cai Huoqing
@ 2023-03-06 14:51 ` Serge Semin
0 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2023-03-06 14:51 UTC (permalink / raw)
To: Cai Huoqing
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Mon, Mar 06, 2023 at 09:31:54PM +0800, Cai Huoqing wrote:
> On 03 3月 23 19:51:25, Serge Semin wrote:
> > On Fri, Mar 03, 2023 at 08:46:31PM +0800, Cai Huoqing wrote:
> > > From: Cai huoqing <cai.huoqing@linux.dev>
> > >
> >
> > > Rename dw_edma_core_ops structure to dw_edma_plat_ops, the ops is platform
> > > specific operations: the DMA device environment configs like IRQs,
> > > address translation, etc.
> > >
> > > The dw_edma_plat_ops name was supposed to refer to the platform which
> > > the DW eDMA engine is embedded to, like PCIe end-point (accessible via
> > > the PCIe bus) or a PCIe root port (directly accessible by CPU).
> > > Needless to say that for them the IRQ-vector and PCI-addresses are
> > > differently determined. The suggested name has a connection with the
> > > kernel platform device only as a private case of the eDMA/hDMA embedded
> > > into the DW PCI Root ports, though basically it was supposed to refer to
> > > any platform in which the DMA hardware lives.
> > >
> > > Anyway the renaming was necessary to distinguish two types of
> > > the implementation callbacks:
> > > 1. DW eDMA/hDMA IP-core specific operations: device-specific CSR
> > > setups in one or another aspect of the DMA-engine initialization.
> > > 2. DW eDMA/hDMA platform specific operations: the DMA device
> > > environment configs like IRQs, address translation, etc.
> > >
> > > dw_edma_core_ops is supposed to be used for the case 1, and
> > > dw_edma_plat_ops - for the case 2.
> >
> > This text was my explanation to Bjorn of why the renaming was
> > necessary. The patch log has a bit different context so I would
> > change it to something like this:
> >
> > "The dw_edma_core_ops structure contains a set of the operations:
> > device IRQ numbers getter, CPU/PCI address translation. Based on the
> > functions semantics the structure name "dw_edma_plat_ops" looks more
> > descriptive since indeed the operations are platform-specific. The
> > "dw_edma_core_ops" name shall be used for a structure with the IP-core
> > specific set of callbacks in order to abstract out DW eDMA and DW HDMA
> > setups. Such structure will be added in one of the next commit in the
> > framework of the set of changes adding the DW HDMA device support."
> OK
>
> >
> > >
> > > Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> > > ---
> > > v4->v5:
> > > 1.Revert the instance dw_edma_pcie_core_ops
> > > 2.Move the change EDMA_MF_HDMA_NATIVE to patch[3/4]
> > >
> > > v4 link:
> > > https://lore.kernel.org/lkml/20230221034656.14476-2-cai.huoqing@linux.dev/
> > >
> > > drivers/dma/dw-edma/dw-edma-pcie.c | 2 +-
> > > drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> > > include/linux/dma/edma.h | 4 ++--
> > > 3 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > index 2b40f2b44f5e..190b32d8016d 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > @@ -109,7 +109,7 @@ static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
> > > return region.start;
> > > }
> > >
> >
> > > -static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> > > +static const struct dw_edma_plat_ops dw_edma_pcie_core_ops = {
> >
> > Please carefully note my comment to v4. I asked to add the prefix
> > specific to the local naming convention:
> >
> > -static const struct dw_edma_core_ops dw_edma_pcie_core_ops = {
> > +static const struct dw_edma_plat_ops dw_edma_pcie_plat_ops = {
> >
> > But besides of adding the correct prefix you changed the suffix to the
> > improper one. Please get it back so the instance name would be
> > "dw_edma_pcie_plat_ops".
> Do you mean ditto
> - chip->ops = &dw_edma_pcie_core_ops;
> + chip->ops = &dw_edma_pcie_plat_ops;
Yes, please.
-Serge(y)
>
> >
> > -Serge(y)
> >
> > > .irq_vector = dw_edma_pcie_irq_vector,
> > > .pci_address = dw_edma_pcie_address,
> > > };
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 53a16b8b6ac2..44e90b71d429 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -828,7 +828,7 @@ static int dw_pcie_edma_irq_vector(struct device *dev, unsigned int nr)
> > > return platform_get_irq_byname_optional(pdev, name);
> > > }
> > >
> > > -static struct dw_edma_core_ops dw_pcie_edma_ops = {
> > > +static struct dw_edma_plat_ops dw_pcie_edma_ops = {
> > > .irq_vector = dw_pcie_edma_irq_vector,
> > > };
> > >
> > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > index d2638d9259dc..ed401c965a87 100644
> > > --- a/include/linux/dma/edma.h
> > > +++ b/include/linux/dma/edma.h
> > > @@ -40,7 +40,7 @@ struct dw_edma_region {
> > > * iATU windows. That will be done by the controller
> > > * automatically.
> > > */
> > > -struct dw_edma_core_ops {
> > > +struct dw_edma_plat_ops {
> > > int (*irq_vector)(struct device *dev, unsigned int nr);
> > > u64 (*pci_address)(struct device *dev, phys_addr_t cpu_addr);
> > > };
> > > @@ -80,7 +80,7 @@ enum dw_edma_chip_flags {
> > > struct dw_edma_chip {
> > > struct device *dev;
> > > int nr_irqs;
> > > - const struct dw_edma_core_ops *ops;
> > > + const struct dw_edma_plat_ops *ops;
> > > u32 flags;
> > >
> > > void __iomem *reg_base;
> > > --
> > > 2.34.1
> > >
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation
2023-03-03 12:46 [PATCH v5 0/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
2023-03-03 12:46 ` [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops Cai Huoqing
@ 2023-03-03 12:46 ` Cai Huoqing
2023-03-03 17:09 ` Serge Semin
2023-03-03 12:46 ` [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
2023-03-03 12:46 ` [PATCH v5 4/4] dmaengine: dw-edma: Add HDMA DebugFS support Cai Huoqing
3 siblings, 1 reply; 16+ messages in thread
From: Cai Huoqing @ 2023-03-03 12:46 UTC (permalink / raw)
To: fancer.lancer
Cc: Cai huoqing, Gustavo Pimentel, Vinod Koul, Jingoo Han,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, linux-kernel, dmaengine, linux-pci
From: Cai huoqing <cai.huoqing@linux.dev>
The structure dw_edma_core_ops has a set of the pointers
abstracting out the DW eDMA vX and DW HDMA Native controllers.
And use dw_edma_v0_core_register to set up operation.
Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
---
v4->v5:
1.Refactor add return irqreturn_t to dw_edma_core_handle_int
2.Define dw_edma_core_handle_int as inline fuction and move to
dw-edma-core.h.
v4 link:
https://lore.kernel.org/lkml/20230221034656.14476-3-cai.huoqing@linux.dev/
drivers/dma/dw-edma/dw-edma-core.c | 83 ++++++++-------------------
drivers/dma/dw-edma/dw-edma-core.h | 64 +++++++++++++++++++++
drivers/dma/dw-edma/dw-edma-v0-core.c | 75 ++++++++++++++++++++----
drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
4 files changed, 153 insertions(+), 83 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 1906a836f0aa..5cfba5730695 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -183,6 +183,7 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
static void dw_edma_start_transfer(struct dw_edma_chan *chan)
{
+ struct dw_edma *dw = chan->dw;
struct dw_edma_chunk *child;
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
@@ -200,7 +201,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
if (!child)
return;
- dw_edma_v0_core_start(child, !desc->xfer_sz);
+ dw_edma_core_start(dw, child, !desc->xfer_sz);
desc->xfer_sz += child->ll_region.sz;
dw_edma_free_burst(child);
list_del(&child->list);
@@ -285,7 +286,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
chan->configured = false;
} else if (chan->status == EDMA_ST_IDLE) {
chan->configured = false;
- } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
+ } else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
/*
* The channel is in a false BUSY state, probably didn't
* receive or lost an interrupt
@@ -588,14 +589,12 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
return dw_edma_device_transfer(&xfer);
}
-static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
+void dw_edma_done_interrupt(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
unsigned long flags;
- dw_edma_v0_core_clear_done_int(chan);
-
spin_lock_irqsave(&chan->vc.lock, flags);
vd = vchan_next_desc(&chan->vc);
if (vd) {
@@ -631,13 +630,11 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
spin_unlock_irqrestore(&chan->vc.lock, flags);
}
-static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
+void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
{
struct virt_dma_desc *vd;
unsigned long flags;
- dw_edma_v0_core_clear_abort_int(chan);
-
spin_lock_irqsave(&chan->vc.lock, flags);
vd = vchan_next_desc(&chan->vc);
if (vd) {
@@ -649,63 +646,29 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
chan->status = EDMA_ST_IDLE;
}
-static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
+static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
{
struct dw_edma_irq *dw_irq = data;
- struct dw_edma *dw = dw_irq->dw;
- unsigned long total, pos, val;
- unsigned long off;
- u32 mask;
-
- if (write) {
- total = dw->wr_ch_cnt;
- off = 0;
- mask = dw_irq->wr_mask;
- } else {
- total = dw->rd_ch_cnt;
- off = dw->wr_ch_cnt;
- mask = dw_irq->rd_mask;
- }
-
- val = dw_edma_v0_core_status_done_int(dw, write ?
- EDMA_DIR_WRITE :
- EDMA_DIR_READ);
- val &= mask;
- for_each_set_bit(pos, &val, total) {
- struct dw_edma_chan *chan = &dw->chan[pos + off];
-
- dw_edma_done_interrupt(chan);
- }
-
- val = dw_edma_v0_core_status_abort_int(dw, write ?
- EDMA_DIR_WRITE :
- EDMA_DIR_READ);
- val &= mask;
- for_each_set_bit(pos, &val, total) {
- struct dw_edma_chan *chan = &dw->chan[pos + off];
-
- dw_edma_abort_interrupt(chan);
- }
- return IRQ_HANDLED;
-}
-
-static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
-{
- return dw_edma_interrupt(irq, data, true);
+ return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
}
static inline irqreturn_t dw_edma_interrupt_read(int irq, void *data)
{
- return dw_edma_interrupt(irq, data, false);
+ struct dw_edma_irq *dw_irq = data;
+
+ return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
}
static irqreturn_t dw_edma_interrupt_common(int irq, void *data)
{
- dw_edma_interrupt(irq, data, true);
- dw_edma_interrupt(irq, data, false);
+ struct dw_edma_irq *dw_irq = data;
+ irqreturn_t ret = IRQ_NONE;
+
+ ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
+ ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
- return IRQ_HANDLED;
+ return ret;
}
static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
@@ -806,7 +769,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
vchan_init(&chan->vc, dma);
- dw_edma_v0_core_device_config(chan);
+ dw_edma_core_ch_config(chan);
}
/* Set DMA channel capabilities */
@@ -951,14 +914,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
dw->chip = chip;
+ dw_edma_v0_core_register(dw);
+
raw_spin_lock_init(&dw->lock);
dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
- dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
+ dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
- dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
+ dw_edma_core_ch_count(dw, EDMA_DIR_READ));
dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
@@ -977,7 +942,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
dev_name(chip->dev));
/* Disable eDMA, only to establish the ideal initial conditions */
- dw_edma_v0_core_off(dw);
+ dw_edma_core_off(dw);
/* Request IRQs */
err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
@@ -990,7 +955,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
goto err_irq_free;
/* Turn debugfs on */
- dw_edma_v0_core_debugfs_on(dw);
+ dw_edma_core_debugfs_on(dw);
chip->dw = dw;
@@ -1016,7 +981,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
return -ENODEV;
/* Disable eDMA */
- dw_edma_v0_core_off(dw);
+ dw_edma_core_off(dw);
/* Free irqs */
for (i = (dw->nr_irqs - 1); i >= 0; i--)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 0ab2b6dba880..b0c4648cd30c 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -111,6 +111,19 @@ struct dw_edma {
raw_spinlock_t lock; /* Only for legacy */
struct dw_edma_chip *chip;
+
+ const struct dw_edma_core_ops *core;
+};
+
+struct dw_edma_core_ops {
+ void (*off)(struct dw_edma *dw);
+ u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
+ enum dma_status (*ch_status)(struct dw_edma_chan *chan);
+ irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq,
+ enum dw_edma_dir dir);
+ void (*start)(struct dw_edma_chunk *chunk, bool first);
+ void (*ch_config)(struct dw_edma_chan *chan);
+ void (*debugfs_on)(struct dw_edma *dw);
};
struct dw_edma_sg {
@@ -136,6 +149,9 @@ struct dw_edma_transfer {
enum dw_edma_xfer_type type;
};
+void dw_edma_done_interrupt(struct dw_edma_chan *chan);
+void dw_edma_abort_interrupt(struct dw_edma_chan *chan);
+
static inline
struct dw_edma_chan *vc2dw_edma_chan(struct virt_dma_chan *vc)
{
@@ -148,4 +164,52 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
return vc2dw_edma_chan(to_virt_chan(dchan));
}
+static inline
+void dw_edma_core_off(struct dw_edma *dw)
+{
+ dw->core->off(dw);
+}
+
+static inline
+u16 dw_edma_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+{
+ return dw->core->ch_count(dw, dir);
+}
+
+static inline
+enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ return dw->core->ch_status(chan);
+}
+
+static inline irqreturn_t
+dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+
+ return dw->core->handle_int(dw_irq, dir);
+}
+
+static inline
+void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
+{
+ dw->core->start(chunk, first);
+}
+
+static inline
+void dw_edma_core_ch_config(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ dw->core->ch_config(chan);
+}
+
+static inline
+void dw_edma_core_debugfs_on(struct dw_edma *dw)
+{
+ dw->core->debugfs_on(dw);
+}
+
#endif /* _DW_EDMA_CORE_H */
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 72e79a0c0a4e..2ebae48531f9 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -216,7 +216,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
readq_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
/* eDMA management callbacks */
-void dw_edma_v0_core_off(struct dw_edma *dw)
+static void dw_edma_v0_core_off(struct dw_edma *dw)
{
SET_BOTH_32(dw, int_mask,
EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
@@ -225,7 +225,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
SET_BOTH_32(dw, engine_en, 0);
}
-u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
{
u32 num_ch;
@@ -242,7 +242,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
return (u16)num_ch;
}
-enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
+static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
u32 tmp;
@@ -258,7 +258,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
return DMA_ERROR;
}
-void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
+static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -266,7 +266,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
}
-void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
+static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -274,18 +274,56 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
}
-u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
+static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
{
return FIELD_GET(EDMA_V0_DONE_INT_MASK,
GET_RW_32(dw, dir, int_status));
}
-u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
+static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
{
return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
GET_RW_32(dw, dir, int_status));
}
+static
+irqreturn_t dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ struct dw_edma_chan *chan;
+ unsigned long off;
+ u32 mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ val = dw_edma_v0_core_status_done_int(dw, dir);
+ val &= mask;
+ for_each_set_bit(pos, &val, total) {
+ chan = &dw->chan[pos + off];
+ dw_edma_v0_core_clear_done_int(chan);
+ dw_edma_done_interrupt(chan);
+ }
+
+ val = dw_edma_v0_core_status_abort_int(dw, dir);
+ val &= mask;
+ for_each_set_bit(pos, &val, total) {
+ chan = &dw->chan[pos + off];
+ dw_edma_v0_core_clear_abort_int(chan);
+ dw_edma_abort_interrupt(chan);
+ }
+
+ return IRQ_HANDLED;
+}
+
static void dw_edma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
u32 control, u32 size, u64 sar, u64 dar)
{
@@ -356,7 +394,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
dw_edma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
}
-void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
{
struct dw_edma_chan *chan = chunk->chan;
struct dw_edma *dw = chan->dw;
@@ -427,7 +465,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
}
-int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
+static void dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
u32 tmp = 0;
@@ -494,12 +532,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
SET_RW_32(dw, chan->dir, ch67_imwr_data, tmp);
break;
}
-
- return 0;
}
/* eDMA debugfs callbacks */
-void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
+static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
{
dw_edma_v0_debugfs_on(dw);
}
+
+static const struct dw_edma_core_ops dw_edma_v0_core = {
+ .off = dw_edma_v0_core_off,
+ .ch_count = dw_edma_v0_core_ch_count,
+ .ch_status = dw_edma_v0_core_ch_status,
+ .handle_int = dw_edma_v0_core_handle_int,
+ .start = dw_edma_v0_core_start,
+ .ch_config = dw_edma_v0_core_ch_config,
+ .debugfs_on = dw_edma_v0_core_debugfs_on,
+};
+
+void dw_edma_v0_core_register(struct dw_edma *dw)
+{
+ dw->core = &dw_edma_v0_core;
+}
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
index ab96a1f48080..04a882222f99 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.h
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
@@ -11,17 +11,7 @@
#include <linux/dma/edma.h>
-/* eDMA management callbacks */
-void dw_edma_v0_core_off(struct dw_edma *chan);
-u16 dw_edma_v0_core_ch_count(struct dw_edma *chan, enum dw_edma_dir dir);
-enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan);
-void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan);
-void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan);
-u32 dw_edma_v0_core_status_done_int(struct dw_edma *chan, enum dw_edma_dir dir);
-u32 dw_edma_v0_core_status_abort_int(struct dw_edma *chan, enum dw_edma_dir dir);
-void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first);
-int dw_edma_v0_core_device_config(struct dw_edma_chan *chan);
-/* eDMA debug fs callbacks */
-void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
+/* eDMA core register */
+void dw_edma_v0_core_register(struct dw_edma *dw);
#endif /* _DW_EDMA_V0_CORE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation
2023-03-03 12:46 ` [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation Cai Huoqing
@ 2023-03-03 17:09 ` Serge Semin
2023-03-03 17:14 ` Serge Semin
2023-03-06 13:18 ` Cai Huoqing
0 siblings, 2 replies; 16+ messages in thread
From: Serge Semin @ 2023-03-03 17:09 UTC (permalink / raw)
To: Cai Huoqing
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Fri, Mar 03, 2023 at 08:46:32PM +0800, Cai Huoqing wrote:
> From: Cai huoqing <cai.huoqing@linux.dev>
>
> The structure dw_edma_core_ops has a set of the pointers
> abstracting out the DW eDMA vX and DW HDMA Native controllers.
> And use dw_edma_v0_core_register to set up operation.
>
> Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> ---
> v4->v5:
> 1.Refactor add return irqreturn_t to dw_edma_core_handle_int
> 2.Define dw_edma_core_handle_int as inline fuction and move to
> dw-edma-core.h.
>
> v4 link:
> https://lore.kernel.org/lkml/20230221034656.14476-3-cai.huoqing@linux.dev/
>
> drivers/dma/dw-edma/dw-edma-core.c | 83 ++++++++-------------------
> drivers/dma/dw-edma/dw-edma-core.h | 64 +++++++++++++++++++++
> drivers/dma/dw-edma/dw-edma-v0-core.c | 75 ++++++++++++++++++++----
> drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> 4 files changed, 153 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 1906a836f0aa..5cfba5730695 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -183,6 +183,7 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
>
> static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> {
> + struct dw_edma *dw = chan->dw;
> struct dw_edma_chunk *child;
> struct dw_edma_desc *desc;
> struct virt_dma_desc *vd;
> @@ -200,7 +201,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> if (!child)
> return;
>
> - dw_edma_v0_core_start(child, !desc->xfer_sz);
> + dw_edma_core_start(dw, child, !desc->xfer_sz);
> desc->xfer_sz += child->ll_region.sz;
> dw_edma_free_burst(child);
> list_del(&child->list);
> @@ -285,7 +286,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> chan->configured = false;
> } else if (chan->status == EDMA_ST_IDLE) {
> chan->configured = false;
> - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> + } else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
> /*
> * The channel is in a false BUSY state, probably didn't
> * receive or lost an interrupt
> @@ -588,14 +589,12 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
> return dw_edma_device_transfer(&xfer);
> }
>
> -static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> +void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> {
> struct dw_edma_desc *desc;
> struct virt_dma_desc *vd;
> unsigned long flags;
>
> - dw_edma_v0_core_clear_done_int(chan);
> -
> spin_lock_irqsave(&chan->vc.lock, flags);
> vd = vchan_next_desc(&chan->vc);
> if (vd) {
> @@ -631,13 +630,11 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> spin_unlock_irqrestore(&chan->vc.lock, flags);
> }
>
> -static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> +void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> {
> struct virt_dma_desc *vd;
> unsigned long flags;
>
> - dw_edma_v0_core_clear_abort_int(chan);
> -
> spin_lock_irqsave(&chan->vc.lock, flags);
> vd = vchan_next_desc(&chan->vc);
> if (vd) {
> @@ -649,63 +646,29 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> chan->status = EDMA_ST_IDLE;
> }
>
> -static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
> +static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> {
> struct dw_edma_irq *dw_irq = data;
> - struct dw_edma *dw = dw_irq->dw;
> - unsigned long total, pos, val;
> - unsigned long off;
> - u32 mask;
> -
> - if (write) {
> - total = dw->wr_ch_cnt;
> - off = 0;
> - mask = dw_irq->wr_mask;
> - } else {
> - total = dw->rd_ch_cnt;
> - off = dw->wr_ch_cnt;
> - mask = dw_irq->rd_mask;
> - }
> -
> - val = dw_edma_v0_core_status_done_int(dw, write ?
> - EDMA_DIR_WRITE :
> - EDMA_DIR_READ);
> - val &= mask;
> - for_each_set_bit(pos, &val, total) {
> - struct dw_edma_chan *chan = &dw->chan[pos + off];
> -
> - dw_edma_done_interrupt(chan);
> - }
> -
> - val = dw_edma_v0_core_status_abort_int(dw, write ?
> - EDMA_DIR_WRITE :
> - EDMA_DIR_READ);
> - val &= mask;
> - for_each_set_bit(pos, &val, total) {
> - struct dw_edma_chan *chan = &dw->chan[pos + off];
> -
> - dw_edma_abort_interrupt(chan);
> - }
>
> - return IRQ_HANDLED;
> -}
> -
> -static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> -{
> - return dw_edma_interrupt(irq, data, true);
> + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> }
>
> static inline irqreturn_t dw_edma_interrupt_read(int irq, void *data)
> {
> - return dw_edma_interrupt(irq, data, false);
> + struct dw_edma_irq *dw_irq = data;
> +
> + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> }
>
> static irqreturn_t dw_edma_interrupt_common(int irq, void *data)
> {
> - dw_edma_interrupt(irq, data, true);
> - dw_edma_interrupt(irq, data, false);
> + struct dw_edma_irq *dw_irq = data;
> + irqreturn_t ret = IRQ_NONE;
> +
> + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
>
> - return IRQ_HANDLED;
> + return ret;
> }
>
> static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
> @@ -806,7 +769,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>
> vchan_init(&chan->vc, dma);
>
> - dw_edma_v0_core_device_config(chan);
> + dw_edma_core_ch_config(chan);
> }
>
> /* Set DMA channel capabilities */
> @@ -951,14 +914,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> + dw_edma_v0_core_register(dw);
> +
> raw_spin_lock_init(&dw->lock);
>
> dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> + dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
>
> dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> + dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
>
> if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> @@ -977,7 +942,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> dev_name(chip->dev));
>
> /* Disable eDMA, only to establish the ideal initial conditions */
> - dw_edma_v0_core_off(dw);
> + dw_edma_core_off(dw);
>
> /* Request IRQs */
> err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> @@ -990,7 +955,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> goto err_irq_free;
>
> /* Turn debugfs on */
> - dw_edma_v0_core_debugfs_on(dw);
> + dw_edma_core_debugfs_on(dw);
>
> chip->dw = dw;
>
> @@ -1016,7 +981,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> return -ENODEV;
>
> /* Disable eDMA */
> - dw_edma_v0_core_off(dw);
> + dw_edma_core_off(dw);
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 0ab2b6dba880..b0c4648cd30c 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -111,6 +111,19 @@ struct dw_edma {
> raw_spinlock_t lock; /* Only for legacy */
>
> struct dw_edma_chip *chip;
> +
> + const struct dw_edma_core_ops *core;
> +};
> +
> +struct dw_edma_core_ops {
> + void (*off)(struct dw_edma *dw);
> + u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> + enum dma_status (*ch_status)(struct dw_edma_chan *chan);
> + irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq,
> + enum dw_edma_dir dir);
> + void (*start)(struct dw_edma_chunk *chunk, bool first);
> + void (*ch_config)(struct dw_edma_chan *chan);
> + void (*debugfs_on)(struct dw_edma *dw);
> };
>
> struct dw_edma_sg {
> @@ -136,6 +149,9 @@ struct dw_edma_transfer {
> enum dw_edma_xfer_type type;
> };
>
> +void dw_edma_done_interrupt(struct dw_edma_chan *chan);
> +void dw_edma_abort_interrupt(struct dw_edma_chan *chan);
I'll ask one more time. So you've decided to go with the global
dw_edma_done_interrupt()/dw_edma_abort_interrupt() methods instead of
passing them as the function pointers to the handle_int callback.
Are you sure it looks better (simpler, more readable) that way?
> +
> static inline
> struct dw_edma_chan *vc2dw_edma_chan(struct virt_dma_chan *vc)
> {
> @@ -148,4 +164,52 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
> return vc2dw_edma_chan(to_virt_chan(dchan));
> }
>
> +static inline
> +void dw_edma_core_off(struct dw_edma *dw)
> +{
> + dw->core->off(dw);
> +}
> +
> +static inline
> +u16 dw_edma_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> + return dw->core->ch_count(dw, dir);
> +}
> +
> +static inline
> +enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + return dw->core->ch_status(chan);
> +}
> +
> +static inline irqreturn_t
> +dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> +
> + return dw->core->handle_int(dw_irq, dir);
> +}
> +
> +static inline
> +void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
> +{
> + dw->core->start(chunk, first);
> +}
> +
> +static inline
> +void dw_edma_core_ch_config(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + dw->core->ch_config(chan);
> +}
> +
> +static inline
> +void dw_edma_core_debugfs_on(struct dw_edma *dw)
> +{
> + dw->core->debugfs_on(dw);
> +}
> +
> #endif /* _DW_EDMA_CORE_H */
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 72e79a0c0a4e..2ebae48531f9 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -216,7 +216,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> readq_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
>
> /* eDMA management callbacks */
> -void dw_edma_v0_core_off(struct dw_edma *dw)
> +static void dw_edma_v0_core_off(struct dw_edma *dw)
> {
> SET_BOTH_32(dw, int_mask,
> EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> @@ -225,7 +225,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> SET_BOTH_32(dw, engine_en, 0);
> }
>
> -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> u32 num_ch;
>
> @@ -242,7 +242,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> return (u16)num_ch;
> }
>
> -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp;
> @@ -258,7 +258,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> return DMA_ERROR;
> }
>
> -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
>
> @@ -266,7 +266,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> }
>
> -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
>
> @@ -274,18 +274,56 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
> }
>
> -u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> GET_RW_32(dw, dir, int_status));
> }
>
> -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> {
> return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> GET_RW_32(dw, dir, int_status));
> }
>
> +static
> +irqreturn_t dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + struct dw_edma_chan *chan;
+ int ret = IRQ_NONE;
> + unsigned long off;
> + u32 mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + val = dw_edma_v0_core_status_done_int(dw, dir);
> + val &= mask;
> + for_each_set_bit(pos, &val, total) {
> + chan = &dw->chan[pos + off];
+ newline
> + dw_edma_v0_core_clear_done_int(chan);
> + dw_edma_done_interrupt(chan);
+ newline
+ ret = IRQ_HANDLED;
> + }
> +
> + val = dw_edma_v0_core_status_abort_int(dw, dir);
> + val &= mask;
> + for_each_set_bit(pos, &val, total) {
> + chan = &dw->chan[pos + off];
+ newline
> + dw_edma_v0_core_clear_abort_int(chan);
> + dw_edma_abort_interrupt(chan);
+ newline
+ ret = IRQ_HANDLED;
> + }
> +
> - return IRQ_HANDLED;
+ return ret;
> +}
Your version of the handler doesn't indicate whether the IRQ was
actually handled. Instead it misleadingly returns always-handled
status. What if no IRQ flag was actually set and no for-each
loop was taken? It's possible for the shared IRQ lines.
Besides of adding the actual IRQ-handling status return please note
1. newlines
2. include "linux/irqreturn.h" to the head of the file.
-Serge(y)
> +
> static void dw_edma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
> u32 control, u32 size, u64 sar, u64 dar)
> {
> @@ -356,7 +394,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> dw_edma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
> }
>
> -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> {
> struct dw_edma_chan *chan = chunk->chan;
> struct dw_edma *dw = chan->dw;
> @@ -427,7 +465,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> }
>
> -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> +static void dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp = 0;
> @@ -494,12 +532,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> SET_RW_32(dw, chan->dir, ch67_imwr_data, tmp);
> break;
> }
> -
> - return 0;
> }
>
> /* eDMA debugfs callbacks */
> -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> {
> dw_edma_v0_debugfs_on(dw);
> }
> +
> +static const struct dw_edma_core_ops dw_edma_v0_core = {
> + .off = dw_edma_v0_core_off,
> + .ch_count = dw_edma_v0_core_ch_count,
> + .ch_status = dw_edma_v0_core_ch_status,
> + .handle_int = dw_edma_v0_core_handle_int,
> + .start = dw_edma_v0_core_start,
> + .ch_config = dw_edma_v0_core_ch_config,
> + .debugfs_on = dw_edma_v0_core_debugfs_on,
> +};
> +
> +void dw_edma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &dw_edma_v0_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> index ab96a1f48080..04a882222f99 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> @@ -11,17 +11,7 @@
>
> #include <linux/dma/edma.h>
>
> -/* eDMA management callbacks */
> -void dw_edma_v0_core_off(struct dw_edma *chan);
> -u16 dw_edma_v0_core_ch_count(struct dw_edma *chan, enum dw_edma_dir dir);
> -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan);
> -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan);
> -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan);
> -u32 dw_edma_v0_core_status_done_int(struct dw_edma *chan, enum dw_edma_dir dir);
> -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *chan, enum dw_edma_dir dir);
> -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first);
> -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan);
> -/* eDMA debug fs callbacks */
> -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> +/* eDMA core register */
> +void dw_edma_v0_core_register(struct dw_edma *dw);
>
> #endif /* _DW_EDMA_V0_CORE_H */
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation
2023-03-03 17:09 ` Serge Semin
@ 2023-03-03 17:14 ` Serge Semin
2023-03-06 13:18 ` Cai Huoqing
1 sibling, 0 replies; 16+ messages in thread
From: Serge Semin @ 2023-03-03 17:14 UTC (permalink / raw)
To: Cai Huoqing
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Fri, Mar 03, 2023 at 08:09:38PM +0300, Serge Semin wrote:
> On Fri, Mar 03, 2023 at 08:46:32PM +0800, Cai Huoqing wrote:
> > From: Cai huoqing <cai.huoqing@linux.dev>
> >
> > The structure dw_edma_core_ops has a set of the pointers
> > abstracting out the DW eDMA vX and DW HDMA Native controllers.
> > And use dw_edma_v0_core_register to set up operation.
> >
> > Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> > ---
> > v4->v5:
> > 1.Refactor add return irqreturn_t to dw_edma_core_handle_int
> > 2.Define dw_edma_core_handle_int as inline fuction and move to
> > dw-edma-core.h.
> >
> > v4 link:
> > https://lore.kernel.org/lkml/20230221034656.14476-3-cai.huoqing@linux.dev/
> >
> > drivers/dma/dw-edma/dw-edma-core.c | 83 ++++++++-------------------
> > drivers/dma/dw-edma/dw-edma-core.h | 64 +++++++++++++++++++++
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 75 ++++++++++++++++++++----
> > drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> > 4 files changed, 153 insertions(+), 83 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 1906a836f0aa..5cfba5730695 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -183,6 +183,7 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
> >
> > static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > {
> > + struct dw_edma *dw = chan->dw;
> > struct dw_edma_chunk *child;
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > @@ -200,7 +201,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > if (!child)
> > return;
> >
> > - dw_edma_v0_core_start(child, !desc->xfer_sz);
> > + dw_edma_core_start(dw, child, !desc->xfer_sz);
> > desc->xfer_sz += child->ll_region.sz;
> > dw_edma_free_burst(child);
> > list_del(&child->list);
> > @@ -285,7 +286,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > chan->configured = false;
> > } else if (chan->status == EDMA_ST_IDLE) {
> > chan->configured = false;
> > - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> > + } else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
> > /*
> > * The channel is in a false BUSY state, probably didn't
> > * receive or lost an interrupt
> > @@ -588,14 +589,12 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
> > return dw_edma_device_transfer(&xfer);
> > }
> >
> > -static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > +void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > {
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_done_int(chan);
> > -
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > if (vd) {
> > @@ -631,13 +630,11 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > spin_unlock_irqrestore(&chan->vc.lock, flags);
> > }
> >
> > -static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > +void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > {
> > struct virt_dma_desc *vd;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_abort_int(chan);
> > -
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > if (vd) {
> > @@ -649,63 +646,29 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > chan->status = EDMA_ST_IDLE;
> > }
> >
> > -static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
> > +static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> > {
> > struct dw_edma_irq *dw_irq = data;
> > - struct dw_edma *dw = dw_irq->dw;
> > - unsigned long total, pos, val;
> > - unsigned long off;
> > - u32 mask;
> > -
> > - if (write) {
> > - total = dw->wr_ch_cnt;
> > - off = 0;
> > - mask = dw_irq->wr_mask;
> > - } else {
> > - total = dw->rd_ch_cnt;
> > - off = dw->wr_ch_cnt;
> > - mask = dw_irq->rd_mask;
> > - }
> > -
> > - val = dw_edma_v0_core_status_done_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > -
> > - dw_edma_done_interrupt(chan);
> > - }
> > -
> > - val = dw_edma_v0_core_status_abort_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > -
> > - dw_edma_abort_interrupt(chan);
> > - }
> >
> > - return IRQ_HANDLED;
> > -}
> > -
> > -static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> > -{
> > - return dw_edma_interrupt(irq, data, true);
> > + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> > }
> >
> > static inline irqreturn_t dw_edma_interrupt_read(int irq, void *data)
> > {
> > - return dw_edma_interrupt(irq, data, false);
> > + struct dw_edma_irq *dw_irq = data;
> > +
> > + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> > }
> >
> > static irqreturn_t dw_edma_interrupt_common(int irq, void *data)
> > {
> > - dw_edma_interrupt(irq, data, true);
> > - dw_edma_interrupt(irq, data, false);
> > + struct dw_edma_irq *dw_irq = data;
> > + irqreturn_t ret = IRQ_NONE;
> > +
> > + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> > + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> >
> > - return IRQ_HANDLED;
> > + return ret;
> > }
> >
> > static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
> > @@ -806,7 +769,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> >
> > vchan_init(&chan->vc, dma);
> >
> > - dw_edma_v0_core_device_config(chan);
> > + dw_edma_core_ch_config(chan);
> > }
> >
> > /* Set DMA channel capabilities */
> > @@ -951,14 +914,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> >
> > dw->chip = chip;
> >
> > + dw_edma_v0_core_register(dw);
> > +
> > raw_spin_lock_init(&dw->lock);
> >
> > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> > + dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> > dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> >
> > dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> > + dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> > dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> >
> > if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> > @@ -977,7 +942,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > dev_name(chip->dev));
> >
> > /* Disable eDMA, only to establish the ideal initial conditions */
> > - dw_edma_v0_core_off(dw);
> > + dw_edma_core_off(dw);
> >
> > /* Request IRQs */
> > err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> > @@ -990,7 +955,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > goto err_irq_free;
> >
> > /* Turn debugfs on */
> > - dw_edma_v0_core_debugfs_on(dw);
> > + dw_edma_core_debugfs_on(dw);
> >
> > chip->dw = dw;
> >
> > @@ -1016,7 +981,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > return -ENODEV;
> >
> > /* Disable eDMA */
> > - dw_edma_v0_core_off(dw);
> > + dw_edma_core_off(dw);
> >
> > /* Free irqs */
> > for (i = (dw->nr_irqs - 1); i >= 0; i--)
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > index 0ab2b6dba880..b0c4648cd30c 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -111,6 +111,19 @@ struct dw_edma {
> > raw_spinlock_t lock; /* Only for legacy */
> >
> > struct dw_edma_chip *chip;
> > +
> > + const struct dw_edma_core_ops *core;
> > +};
> > +
> > +struct dw_edma_core_ops {
> > + void (*off)(struct dw_edma *dw);
> > + u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> > + enum dma_status (*ch_status)(struct dw_edma_chan *chan);
> > + irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq,
> > + enum dw_edma_dir dir);
> > + void (*start)(struct dw_edma_chunk *chunk, bool first);
> > + void (*ch_config)(struct dw_edma_chan *chan);
> > + void (*debugfs_on)(struct dw_edma *dw);
> > };
> >
> > struct dw_edma_sg {
> > @@ -136,6 +149,9 @@ struct dw_edma_transfer {
> > enum dw_edma_xfer_type type;
> > };
> >
>
> > +void dw_edma_done_interrupt(struct dw_edma_chan *chan);
> > +void dw_edma_abort_interrupt(struct dw_edma_chan *chan);
>
> I'll ask one more time. So you've decided to go with the global
> dw_edma_done_interrupt()/dw_edma_abort_interrupt() methods instead of
> passing them as the function pointers to the handle_int callback.
>
> Are you sure it looks better (simpler, more readable) that way?
>
> > +
> > static inline
> > struct dw_edma_chan *vc2dw_edma_chan(struct virt_dma_chan *vc)
> > {
> > @@ -148,4 +164,52 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
> > return vc2dw_edma_chan(to_virt_chan(dchan));
> > }
> >
> > +static inline
> > +void dw_edma_core_off(struct dw_edma *dw)
> > +{
> > + dw->core->off(dw);
> > +}
> > +
> > +static inline
> > +u16 dw_edma_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +{
> > + return dw->core->ch_count(dw, dir);
> > +}
> > +
> > +static inline
> > +enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
> > +{
> > + struct dw_edma *dw = chan->dw;
> > +
> > + return dw->core->ch_status(chan);
> > +}
> > +
> > +static inline irqreturn_t
> > +dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > +
> > + return dw->core->handle_int(dw_irq, dir);
> > +}
> > +
> > +static inline
> > +void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
> > +{
> > + dw->core->start(chunk, first);
> > +}
> > +
> > +static inline
> > +void dw_edma_core_ch_config(struct dw_edma_chan *chan)
> > +{
> > + struct dw_edma *dw = chan->dw;
> > +
> > + dw->core->ch_config(chan);
> > +}
> > +
> > +static inline
> > +void dw_edma_core_debugfs_on(struct dw_edma *dw)
> > +{
> > + dw->core->debugfs_on(dw);
> > +}
> > +
> > #endif /* _DW_EDMA_CORE_H */
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 72e79a0c0a4e..2ebae48531f9 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -216,7 +216,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > readq_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
> >
> > /* eDMA management callbacks */
> > -void dw_edma_v0_core_off(struct dw_edma *dw)
> > +static void dw_edma_v0_core_off(struct dw_edma *dw)
> > {
> > SET_BOTH_32(dw, int_mask,
> > EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> > @@ -225,7 +225,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> > SET_BOTH_32(dw, engine_en, 0);
> > }
> >
> > -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > u32 num_ch;
> >
> > @@ -242,7 +242,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > return (u16)num_ch;
> > }
> >
> > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp;
> > @@ -258,7 +258,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > return DMA_ERROR;
> > }
> >
> > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -266,7 +266,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> > }
> >
> > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -274,18 +274,56 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
> > }
> >
> > -u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > +static
> > +irqreturn_t dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > + unsigned long total, pos, val;
> > + struct dw_edma_chan *chan;
> + int ret = IRQ_NONE;
I meant "irqreturn_t" type of course.
-Serge(y)
> > + unsigned long off;
> > + u32 mask;
> > +
> > + if (dir == EDMA_DIR_WRITE) {
> > + total = dw->wr_ch_cnt;
> > + off = 0;
> > + mask = dw_irq->wr_mask;
> > + } else {
> > + total = dw->rd_ch_cnt;
> > + off = dw->wr_ch_cnt;
> > + mask = dw_irq->rd_mask;
> > + }
> > +
> > + val = dw_edma_v0_core_status_done_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total) {
> > + chan = &dw->chan[pos + off];
> + newline
> > + dw_edma_v0_core_clear_done_int(chan);
> > + dw_edma_done_interrupt(chan);
> + newline
> + ret = IRQ_HANDLED;
> > + }
> > +
> > + val = dw_edma_v0_core_status_abort_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total) {
> > + chan = &dw->chan[pos + off];
> + newline
> > + dw_edma_v0_core_clear_abort_int(chan);
> > + dw_edma_abort_interrupt(chan);
> + newline
> + ret = IRQ_HANDLED;
> > + }
> > +
> > - return IRQ_HANDLED;
> + return ret;
> > +}
>
> Your version of the handler doesn't indicate whether the IRQ was
> actually handled. Instead it misleadingly returns always-handled
> status. What if no IRQ flag was actually set and no for-each
> loop was taken? It's possible for the shared IRQ lines.
>
> Besides of adding the actual IRQ-handling status return please note
> 1. newlines
> 2. include "linux/irqreturn.h" to the head of the file.
>
> -Serge(y)
>
> > +
> > static void dw_edma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
> > u32 control, u32 size, u64 sar, u64 dar)
> > {
> > @@ -356,7 +394,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > dw_edma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
> > }
> >
> > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > +static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > {
> > struct dw_edma_chan *chan = chunk->chan;
> > struct dw_edma *dw = chan->dw;
> > @@ -427,7 +465,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> > }
> >
> > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp = 0;
> > @@ -494,12 +532,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > SET_RW_32(dw, chan->dir, ch67_imwr_data, tmp);
> > break;
> > }
> > -
> > - return 0;
> > }
> >
> > /* eDMA debugfs callbacks */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > {
> > dw_edma_v0_debugfs_on(dw);
> > }
> > +
> > +static const struct dw_edma_core_ops dw_edma_v0_core = {
> > + .off = dw_edma_v0_core_off,
> > + .ch_count = dw_edma_v0_core_ch_count,
> > + .ch_status = dw_edma_v0_core_ch_status,
> > + .handle_int = dw_edma_v0_core_handle_int,
> > + .start = dw_edma_v0_core_start,
> > + .ch_config = dw_edma_v0_core_ch_config,
> > + .debugfs_on = dw_edma_v0_core_debugfs_on,
> > +};
> > +
> > +void dw_edma_v0_core_register(struct dw_edma *dw)
> > +{
> > + dw->core = &dw_edma_v0_core;
> > +}
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > index ab96a1f48080..04a882222f99 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > @@ -11,17 +11,7 @@
> >
> > #include <linux/dma/edma.h>
> >
> > -/* eDMA management callbacks */
> > -void dw_edma_v0_core_off(struct dw_edma *chan);
> > -u16 dw_edma_v0_core_ch_count(struct dw_edma *chan, enum dw_edma_dir dir);
> > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan);
> > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan);
> > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan);
> > -u32 dw_edma_v0_core_status_done_int(struct dw_edma *chan, enum dw_edma_dir dir);
> > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *chan, enum dw_edma_dir dir);
> > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first);
> > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan);
> > -/* eDMA debug fs callbacks */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> > +/* eDMA core register */
> > +void dw_edma_v0_core_register(struct dw_edma *dw);
> >
> > #endif /* _DW_EDMA_V0_CORE_H */
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation
2023-03-03 17:09 ` Serge Semin
2023-03-03 17:14 ` Serge Semin
@ 2023-03-06 13:18 ` Cai Huoqing
2023-03-06 15:08 ` Serge Semin
1 sibling, 1 reply; 16+ messages in thread
From: Cai Huoqing @ 2023-03-06 13:18 UTC (permalink / raw)
To: Serge Semin
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On 03 3月 23 20:09:31, Serge Semin wrote:
> On Fri, Mar 03, 2023 at 08:46:32PM +0800, Cai Huoqing wrote:
> > From: Cai huoqing <cai.huoqing@linux.dev>
> >
> > The structure dw_edma_core_ops has a set of the pointers
> > abstracting out the DW eDMA vX and DW HDMA Native controllers.
> > And use dw_edma_v0_core_register to set up operation.
> >
> > Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> > ---
> > v4->v5:
> > 1.Refactor add return irqreturn_t to dw_edma_core_handle_int
> > 2.Define dw_edma_core_handle_int as inline fuction and move to
> > dw-edma-core.h.
> >
> > v4 link:
> > https://lore.kernel.org/lkml/20230221034656.14476-3-cai.huoqing@linux.dev/
> >
> > drivers/dma/dw-edma/dw-edma-core.c | 83 ++++++++-------------------
> > drivers/dma/dw-edma/dw-edma-core.h | 64 +++++++++++++++++++++
> > drivers/dma/dw-edma/dw-edma-v0-core.c | 75 ++++++++++++++++++++----
> > drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> > 4 files changed, 153 insertions(+), 83 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 1906a836f0aa..5cfba5730695 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -183,6 +183,7 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
> >
> > static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > {
> > + struct dw_edma *dw = chan->dw;
> > struct dw_edma_chunk *child;
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > @@ -200,7 +201,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > if (!child)
> > return;
> >
> > - dw_edma_v0_core_start(child, !desc->xfer_sz);
> > + dw_edma_core_start(dw, child, !desc->xfer_sz);
> > desc->xfer_sz += child->ll_region.sz;
> > dw_edma_free_burst(child);
> > list_del(&child->list);
> > @@ -285,7 +286,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > chan->configured = false;
> > } else if (chan->status == EDMA_ST_IDLE) {
> > chan->configured = false;
> > - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> > + } else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
> > /*
> > * The channel is in a false BUSY state, probably didn't
> > * receive or lost an interrupt
> > @@ -588,14 +589,12 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
> > return dw_edma_device_transfer(&xfer);
> > }
> >
> > -static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > +void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > {
> > struct dw_edma_desc *desc;
> > struct virt_dma_desc *vd;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_done_int(chan);
> > -
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > if (vd) {
> > @@ -631,13 +630,11 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > spin_unlock_irqrestore(&chan->vc.lock, flags);
> > }
> >
> > -static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > +void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > {
> > struct virt_dma_desc *vd;
> > unsigned long flags;
> >
> > - dw_edma_v0_core_clear_abort_int(chan);
> > -
> > spin_lock_irqsave(&chan->vc.lock, flags);
> > vd = vchan_next_desc(&chan->vc);
> > if (vd) {
> > @@ -649,63 +646,29 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > chan->status = EDMA_ST_IDLE;
> > }
> >
> > -static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
> > +static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> > {
> > struct dw_edma_irq *dw_irq = data;
> > - struct dw_edma *dw = dw_irq->dw;
> > - unsigned long total, pos, val;
> > - unsigned long off;
> > - u32 mask;
> > -
> > - if (write) {
> > - total = dw->wr_ch_cnt;
> > - off = 0;
> > - mask = dw_irq->wr_mask;
> > - } else {
> > - total = dw->rd_ch_cnt;
> > - off = dw->wr_ch_cnt;
> > - mask = dw_irq->rd_mask;
> > - }
> > -
> > - val = dw_edma_v0_core_status_done_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > -
> > - dw_edma_done_interrupt(chan);
> > - }
> > -
> > - val = dw_edma_v0_core_status_abort_int(dw, write ?
> > - EDMA_DIR_WRITE :
> > - EDMA_DIR_READ);
> > - val &= mask;
> > - for_each_set_bit(pos, &val, total) {
> > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > -
> > - dw_edma_abort_interrupt(chan);
> > - }
> >
> > - return IRQ_HANDLED;
> > -}
> > -
> > -static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> > -{
> > - return dw_edma_interrupt(irq, data, true);
> > + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> > }
> >
> > static inline irqreturn_t dw_edma_interrupt_read(int irq, void *data)
> > {
> > - return dw_edma_interrupt(irq, data, false);
> > + struct dw_edma_irq *dw_irq = data;
> > +
> > + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> > }
> >
> > static irqreturn_t dw_edma_interrupt_common(int irq, void *data)
> > {
> > - dw_edma_interrupt(irq, data, true);
> > - dw_edma_interrupt(irq, data, false);
> > + struct dw_edma_irq *dw_irq = data;
> > + irqreturn_t ret = IRQ_NONE;
> > +
> > + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> > + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> >
> > - return IRQ_HANDLED;
> > + return ret;
> > }
> >
> > static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
> > @@ -806,7 +769,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> >
> > vchan_init(&chan->vc, dma);
> >
> > - dw_edma_v0_core_device_config(chan);
> > + dw_edma_core_ch_config(chan);
> > }
> >
> > /* Set DMA channel capabilities */
> > @@ -951,14 +914,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> >
> > dw->chip = chip;
> >
> > + dw_edma_v0_core_register(dw);
> > +
> > raw_spin_lock_init(&dw->lock);
> >
> > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> > + dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> > dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> >
> > dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> > + dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> > dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> >
> > if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> > @@ -977,7 +942,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > dev_name(chip->dev));
> >
> > /* Disable eDMA, only to establish the ideal initial conditions */
> > - dw_edma_v0_core_off(dw);
> > + dw_edma_core_off(dw);
> >
> > /* Request IRQs */
> > err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> > @@ -990,7 +955,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > goto err_irq_free;
> >
> > /* Turn debugfs on */
> > - dw_edma_v0_core_debugfs_on(dw);
> > + dw_edma_core_debugfs_on(dw);
> >
> > chip->dw = dw;
> >
> > @@ -1016,7 +981,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > return -ENODEV;
> >
> > /* Disable eDMA */
> > - dw_edma_v0_core_off(dw);
> > + dw_edma_core_off(dw);
> >
> > /* Free irqs */
> > for (i = (dw->nr_irqs - 1); i >= 0; i--)
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > index 0ab2b6dba880..b0c4648cd30c 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -111,6 +111,19 @@ struct dw_edma {
> > raw_spinlock_t lock; /* Only for legacy */
> >
> > struct dw_edma_chip *chip;
> > +
> > + const struct dw_edma_core_ops *core;
> > +};
> > +
> > +struct dw_edma_core_ops {
> > + void (*off)(struct dw_edma *dw);
> > + u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> > + enum dma_status (*ch_status)(struct dw_edma_chan *chan);
> > + irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq,
> > + enum dw_edma_dir dir);
> > + void (*start)(struct dw_edma_chunk *chunk, bool first);
> > + void (*ch_config)(struct dw_edma_chan *chan);
> > + void (*debugfs_on)(struct dw_edma *dw);
> > };
> >
> > struct dw_edma_sg {
> > @@ -136,6 +149,9 @@ struct dw_edma_transfer {
> > enum dw_edma_xfer_type type;
> > };
> >
>
> > +void dw_edma_done_interrupt(struct dw_edma_chan *chan);
> > +void dw_edma_abort_interrupt(struct dw_edma_chan *chan);
>
> I'll ask one more time. So you've decided to go with the global
> dw_edma_done_interrupt()/dw_edma_abort_interrupt() methods instead of
> passing them as the function pointers to the handle_int callback.
>
> Are you sure it looks better (simpler, more readable) that way?
Do you pefer the two method as handle_int callback?
>
> > +
> > static inline
> > struct dw_edma_chan *vc2dw_edma_chan(struct virt_dma_chan *vc)
> > {
> > @@ -148,4 +164,52 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
> > return vc2dw_edma_chan(to_virt_chan(dchan));
> > }
> >
> > +static inline
> > +void dw_edma_core_off(struct dw_edma *dw)
> > +{
> > + dw->core->off(dw);
> > +}
> > +
> > +static inline
> > +u16 dw_edma_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +{
> > + return dw->core->ch_count(dw, dir);
> > +}
> > +
> > +static inline
> > +enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
> > +{
> > + struct dw_edma *dw = chan->dw;
> > +
> > + return dw->core->ch_status(chan);
> > +}
> > +
> > +static inline irqreturn_t
> > +dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > +
> > + return dw->core->handle_int(dw_irq, dir);
> > +}
> > +
> > +static inline
> > +void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
> > +{
> > + dw->core->start(chunk, first);
> > +}
> > +
> > +static inline
> > +void dw_edma_core_ch_config(struct dw_edma_chan *chan)
> > +{
> > + struct dw_edma *dw = chan->dw;
> > +
> > + dw->core->ch_config(chan);
> > +}
> > +
> > +static inline
> > +void dw_edma_core_debugfs_on(struct dw_edma *dw)
> > +{
> > + dw->core->debugfs_on(dw);
> > +}
> > +
> > #endif /* _DW_EDMA_CORE_H */
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 72e79a0c0a4e..2ebae48531f9 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > @@ -216,7 +216,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > readq_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
> >
> > /* eDMA management callbacks */
> > -void dw_edma_v0_core_off(struct dw_edma *dw)
> > +static void dw_edma_v0_core_off(struct dw_edma *dw)
> > {
> > SET_BOTH_32(dw, int_mask,
> > EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> > @@ -225,7 +225,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> > SET_BOTH_32(dw, engine_en, 0);
> > }
> >
> > -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > u32 num_ch;
> >
> > @@ -242,7 +242,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > return (u16)num_ch;
> > }
> >
> > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp;
> > @@ -258,7 +258,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > return DMA_ERROR;
> > }
> >
> > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -266,7 +266,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> > }
> >
> > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> >
> > @@ -274,18 +274,56 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
> > }
> >
> > -u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > {
> > return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> > GET_RW_32(dw, dir, int_status));
> > }
> >
> > +static
> > +irqreturn_t dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> > +{
> > + struct dw_edma *dw = dw_irq->dw;
> > + unsigned long total, pos, val;
> > + struct dw_edma_chan *chan;
> + int ret = IRQ_NONE;
> > + unsigned long off;
> > + u32 mask;
> > +
> > + if (dir == EDMA_DIR_WRITE) {
> > + total = dw->wr_ch_cnt;
> > + off = 0;
> > + mask = dw_irq->wr_mask;
> > + } else {
> > + total = dw->rd_ch_cnt;
> > + off = dw->wr_ch_cnt;
> > + mask = dw_irq->rd_mask;
> > + }
> > +
> > + val = dw_edma_v0_core_status_done_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total) {
> > + chan = &dw->chan[pos + off];
> + newline
> > + dw_edma_v0_core_clear_done_int(chan);
> > + dw_edma_done_interrupt(chan);
> + newline
> + ret = IRQ_HANDLED;
> > + }
> > +
> > + val = dw_edma_v0_core_status_abort_int(dw, dir);
> > + val &= mask;
> > + for_each_set_bit(pos, &val, total) {
> > + chan = &dw->chan[pos + off];
> + newline
> > + dw_edma_v0_core_clear_abort_int(chan);
> > + dw_edma_abort_interrupt(chan);
> + newline
> + ret = IRQ_HANDLED;
> > + }
> > +
> > - return IRQ_HANDLED;
> + return ret;
> > +}
>
> Your version of the handler doesn't indicate whether the IRQ was
> actually handled. Instead it misleadingly returns always-handled
> status. What if no IRQ flag was actually set and no for-each
> loop was taken? It's possible for the shared IRQ lines.
>
> Besides of adding the actual IRQ-handling status return please note
> 1. newlines
> 2. include "linux/irqreturn.h" to the head of the file.
>
> -Serge(y)
>
> > +
> > static void dw_edma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
> > u32 control, u32 size, u64 sar, u64 dar)
> > {
> > @@ -356,7 +394,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > dw_edma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
> > }
> >
> > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > +static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > {
> > struct dw_edma_chan *chan = chunk->chan;
> > struct dw_edma *dw = chan->dw;
> > @@ -427,7 +465,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> > }
> >
> > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > +static void dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> > {
> > struct dw_edma *dw = chan->dw;
> > u32 tmp = 0;
> > @@ -494,12 +532,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > SET_RW_32(dw, chan->dir, ch67_imwr_data, tmp);
> > break;
> > }
> > -
> > - return 0;
> > }
> >
> > /* eDMA debugfs callbacks */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > {
> > dw_edma_v0_debugfs_on(dw);
> > }
> > +
> > +static const struct dw_edma_core_ops dw_edma_v0_core = {
> > + .off = dw_edma_v0_core_off,
> > + .ch_count = dw_edma_v0_core_ch_count,
> > + .ch_status = dw_edma_v0_core_ch_status,
> > + .handle_int = dw_edma_v0_core_handle_int,
> > + .start = dw_edma_v0_core_start,
> > + .ch_config = dw_edma_v0_core_ch_config,
> > + .debugfs_on = dw_edma_v0_core_debugfs_on,
> > +};
> > +
> > +void dw_edma_v0_core_register(struct dw_edma *dw)
> > +{
> > + dw->core = &dw_edma_v0_core;
> > +}
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > index ab96a1f48080..04a882222f99 100644
> > --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > @@ -11,17 +11,7 @@
> >
> > #include <linux/dma/edma.h>
> >
> > -/* eDMA management callbacks */
> > -void dw_edma_v0_core_off(struct dw_edma *chan);
> > -u16 dw_edma_v0_core_ch_count(struct dw_edma *chan, enum dw_edma_dir dir);
> > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan);
> > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan);
> > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan);
> > -u32 dw_edma_v0_core_status_done_int(struct dw_edma *chan, enum dw_edma_dir dir);
> > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *chan, enum dw_edma_dir dir);
> > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first);
> > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan);
> > -/* eDMA debug fs callbacks */
> > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> > +/* eDMA core register */
> > +void dw_edma_v0_core_register(struct dw_edma *dw);
> >
> > #endif /* _DW_EDMA_V0_CORE_H */
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation
2023-03-06 13:18 ` Cai Huoqing
@ 2023-03-06 15:08 ` Serge Semin
0 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2023-03-06 15:08 UTC (permalink / raw)
To: Cai Huoqing
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Mon, Mar 06, 2023 at 09:18:26PM +0800, Cai Huoqing wrote:
> On 03 3月 23 20:09:31, Serge Semin wrote:
> > On Fri, Mar 03, 2023 at 08:46:32PM +0800, Cai Huoqing wrote:
> > > From: Cai huoqing <cai.huoqing@linux.dev>
> > >
> > > The structure dw_edma_core_ops has a set of the pointers
> > > abstracting out the DW eDMA vX and DW HDMA Native controllers.
> > > And use dw_edma_v0_core_register to set up operation.
> > >
> > > Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> > > ---
> > > v4->v5:
> > > 1.Refactor add return irqreturn_t to dw_edma_core_handle_int
> > > 2.Define dw_edma_core_handle_int as inline fuction and move to
> > > dw-edma-core.h.
> > >
> > > v4 link:
> > > https://lore.kernel.org/lkml/20230221034656.14476-3-cai.huoqing@linux.dev/
> > >
> > > drivers/dma/dw-edma/dw-edma-core.c | 83 ++++++++-------------------
> > > drivers/dma/dw-edma/dw-edma-core.h | 64 +++++++++++++++++++++
> > > drivers/dma/dw-edma/dw-edma-v0-core.c | 75 ++++++++++++++++++++----
> > > drivers/dma/dw-edma/dw-edma-v0-core.h | 14 +----
> > > 4 files changed, 153 insertions(+), 83 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > index 1906a836f0aa..5cfba5730695 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -183,6 +183,7 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
> > >
> > > static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > > {
> > > + struct dw_edma *dw = chan->dw;
> > > struct dw_edma_chunk *child;
> > > struct dw_edma_desc *desc;
> > > struct virt_dma_desc *vd;
> > > @@ -200,7 +201,7 @@ static void dw_edma_start_transfer(struct dw_edma_chan *chan)
> > > if (!child)
> > > return;
> > >
> > > - dw_edma_v0_core_start(child, !desc->xfer_sz);
> > > + dw_edma_core_start(dw, child, !desc->xfer_sz);
> > > desc->xfer_sz += child->ll_region.sz;
> > > dw_edma_free_burst(child);
> > > list_del(&child->list);
> > > @@ -285,7 +286,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> > > chan->configured = false;
> > > } else if (chan->status == EDMA_ST_IDLE) {
> > > chan->configured = false;
> > > - } else if (dw_edma_v0_core_ch_status(chan) == DMA_COMPLETE) {
> > > + } else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
> > > /*
> > > * The channel is in a false BUSY state, probably didn't
> > > * receive or lost an interrupt
> > > @@ -588,14 +589,12 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
> > > return dw_edma_device_transfer(&xfer);
> > > }
> > >
> > > -static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > > +void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > > {
> > > struct dw_edma_desc *desc;
> > > struct virt_dma_desc *vd;
> > > unsigned long flags;
> > >
> > > - dw_edma_v0_core_clear_done_int(chan);
> > > -
> > > spin_lock_irqsave(&chan->vc.lock, flags);
> > > vd = vchan_next_desc(&chan->vc);
> > > if (vd) {
> > > @@ -631,13 +630,11 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > > spin_unlock_irqrestore(&chan->vc.lock, flags);
> > > }
> > >
> > > -static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > > +void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > > {
> > > struct virt_dma_desc *vd;
> > > unsigned long flags;
> > >
> > > - dw_edma_v0_core_clear_abort_int(chan);
> > > -
> > > spin_lock_irqsave(&chan->vc.lock, flags);
> > > vd = vchan_next_desc(&chan->vc);
> > > if (vd) {
> > > @@ -649,63 +646,29 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> > > chan->status = EDMA_ST_IDLE;
> > > }
> > >
> > > -static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
> > > +static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> > > {
> > > struct dw_edma_irq *dw_irq = data;
> > > - struct dw_edma *dw = dw_irq->dw;
> > > - unsigned long total, pos, val;
> > > - unsigned long off;
> > > - u32 mask;
> > > -
> > > - if (write) {
> > > - total = dw->wr_ch_cnt;
> > > - off = 0;
> > > - mask = dw_irq->wr_mask;
> > > - } else {
> > > - total = dw->rd_ch_cnt;
> > > - off = dw->wr_ch_cnt;
> > > - mask = dw_irq->rd_mask;
> > > - }
> > > -
> > > - val = dw_edma_v0_core_status_done_int(dw, write ?
> > > - EDMA_DIR_WRITE :
> > > - EDMA_DIR_READ);
> > > - val &= mask;
> > > - for_each_set_bit(pos, &val, total) {
> > > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > > -
> > > - dw_edma_done_interrupt(chan);
> > > - }
> > > -
> > > - val = dw_edma_v0_core_status_abort_int(dw, write ?
> > > - EDMA_DIR_WRITE :
> > > - EDMA_DIR_READ);
> > > - val &= mask;
> > > - for_each_set_bit(pos, &val, total) {
> > > - struct dw_edma_chan *chan = &dw->chan[pos + off];
> > > -
> > > - dw_edma_abort_interrupt(chan);
> > > - }
> > >
> > > - return IRQ_HANDLED;
> > > -}
> > > -
> > > -static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> > > -{
> > > - return dw_edma_interrupt(irq, data, true);
> > > + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> > > }
> > >
> > > static inline irqreturn_t dw_edma_interrupt_read(int irq, void *data)
> > > {
> > > - return dw_edma_interrupt(irq, data, false);
> > > + struct dw_edma_irq *dw_irq = data;
> > > +
> > > + return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> > > }
> > >
> > > static irqreturn_t dw_edma_interrupt_common(int irq, void *data)
> > > {
> > > - dw_edma_interrupt(irq, data, true);
> > > - dw_edma_interrupt(irq, data, false);
> > > + struct dw_edma_irq *dw_irq = data;
> > > + irqreturn_t ret = IRQ_NONE;
> > > +
> > > + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE);
> > > + ret |= dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ);
> > >
> > > - return IRQ_HANDLED;
> > > + return ret;
> > > }
> > >
> > > static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
> > > @@ -806,7 +769,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> > >
> > > vchan_init(&chan->vc, dma);
> > >
> > > - dw_edma_v0_core_device_config(chan);
> > > + dw_edma_core_ch_config(chan);
> > > }
> > >
> > > /* Set DMA channel capabilities */
> > > @@ -951,14 +914,16 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > >
> > > dw->chip = chip;
> > >
> > > + dw_edma_v0_core_register(dw);
> > > +
> > > raw_spin_lock_init(&dw->lock);
> > >
> > > dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE));
> > > + dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> > > dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> > >
> > > dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> > > - dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ));
> > > + dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> > > dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> > >
> > > if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> > > @@ -977,7 +942,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > > dev_name(chip->dev));
> > >
> > > /* Disable eDMA, only to establish the ideal initial conditions */
> > > - dw_edma_v0_core_off(dw);
> > > + dw_edma_core_off(dw);
> > >
> > > /* Request IRQs */
> > > err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> > > @@ -990,7 +955,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> > > goto err_irq_free;
> > >
> > > /* Turn debugfs on */
> > > - dw_edma_v0_core_debugfs_on(dw);
> > > + dw_edma_core_debugfs_on(dw);
> > >
> > > chip->dw = dw;
> > >
> > > @@ -1016,7 +981,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > > return -ENODEV;
> > >
> > > /* Disable eDMA */
> > > - dw_edma_v0_core_off(dw);
> > > + dw_edma_core_off(dw);
> > >
> > > /* Free irqs */
> > > for (i = (dw->nr_irqs - 1); i >= 0; i--)
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> > > index 0ab2b6dba880..b0c4648cd30c 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > > @@ -111,6 +111,19 @@ struct dw_edma {
> > > raw_spinlock_t lock; /* Only for legacy */
> > >
> > > struct dw_edma_chip *chip;
> > > +
> > > + const struct dw_edma_core_ops *core;
> > > +};
> > > +
> > > +struct dw_edma_core_ops {
> > > + void (*off)(struct dw_edma *dw);
> > > + u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> > > + enum dma_status (*ch_status)(struct dw_edma_chan *chan);
> > > + irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq,
> > > + enum dw_edma_dir dir);
> > > + void (*start)(struct dw_edma_chunk *chunk, bool first);
> > > + void (*ch_config)(struct dw_edma_chan *chan);
> > > + void (*debugfs_on)(struct dw_edma *dw);
> > > };
> > >
> > > struct dw_edma_sg {
> > > @@ -136,6 +149,9 @@ struct dw_edma_transfer {
> > > enum dw_edma_xfer_type type;
> > > };
> > >
> >
> > > +void dw_edma_done_interrupt(struct dw_edma_chan *chan);
> > > +void dw_edma_abort_interrupt(struct dw_edma_chan *chan);
> >
> > I'll ask one more time. So you've decided to go with the global
> > dw_edma_done_interrupt()/dw_edma_abort_interrupt() methods instead of
> > passing them as the function pointers to the handle_int callback.
> >
> > Are you sure it looks better (simpler, more readable) that way?
> Do you pefer the two method as handle_int callback?
Personally I think that passing the "done" and "abort" callbacks to
the handle_int() function would be more descriptive. Thus the
interface will be more-or-less complete with no implicit dependency
from the external objects. So If I were you I would have defined the
handle_int() prototype like this
struct dw_edma_core_ops {
irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
void (*done)(struct dw_edma_chan *),
void (*abort)(struct dw_edma_chan *))
}
Alternatively if the construction above looks a bit bulky the handlers
type could be also defined:
typedef void (*dw_edma_handler_t)(struct dw_edma_chan *);
struct dw_edma_core_ops {
irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
dw_edma_handler_t done, dw_edma_handler_t abort),
}
-Serge(y)
>
> >
> > > +
> > > static inline
> > > struct dw_edma_chan *vc2dw_edma_chan(struct virt_dma_chan *vc)
> > > {
> > > @@ -148,4 +164,52 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
> > > return vc2dw_edma_chan(to_virt_chan(dchan));
> > > }
> > >
> > > +static inline
> > > +void dw_edma_core_off(struct dw_edma *dw)
> > > +{
> > > + dw->core->off(dw);
> > > +}
> > > +
> > > +static inline
> > > +u16 dw_edma_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > > +{
> > > + return dw->core->ch_count(dw, dir);
> > > +}
> > > +
> > > +static inline
> > > +enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
> > > +{
> > > + struct dw_edma *dw = chan->dw;
> > > +
> > > + return dw->core->ch_status(chan);
> > > +}
> > > +
> > > +static inline irqreturn_t
> > > +dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> > > +{
> > > + struct dw_edma *dw = dw_irq->dw;
> > > +
> > > + return dw->core->handle_int(dw_irq, dir);
> > > +}
> > > +
> > > +static inline
> > > +void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
> > > +{
> > > + dw->core->start(chunk, first);
> > > +}
> > > +
> > > +static inline
> > > +void dw_edma_core_ch_config(struct dw_edma_chan *chan)
> > > +{
> > > + struct dw_edma *dw = chan->dw;
> > > +
> > > + dw->core->ch_config(chan);
> > > +}
> > > +
> > > +static inline
> > > +void dw_edma_core_debugfs_on(struct dw_edma *dw)
> > > +{
> > > + dw->core->debugfs_on(dw);
> > > +}
> > > +
> > > #endif /* _DW_EDMA_CORE_H */
> > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > index 72e79a0c0a4e..2ebae48531f9 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > > @@ -216,7 +216,7 @@ static inline u64 readq_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
> > > readq_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
> > >
> > > /* eDMA management callbacks */
> > > -void dw_edma_v0_core_off(struct dw_edma *dw)
> > > +static void dw_edma_v0_core_off(struct dw_edma *dw)
> > > {
> > > SET_BOTH_32(dw, int_mask,
> > > EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> > > @@ -225,7 +225,7 @@ void dw_edma_v0_core_off(struct dw_edma *dw)
> > > SET_BOTH_32(dw, engine_en, 0);
> > > }
> > >
> > > -u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > > +static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > > {
> > > u32 num_ch;
> > >
> > > @@ -242,7 +242,7 @@ u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> > > return (u16)num_ch;
> > > }
> > >
> > > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > > +static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > > {
> > > struct dw_edma *dw = chan->dw;
> > > u32 tmp;
> > > @@ -258,7 +258,7 @@ enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> > > return DMA_ERROR;
> > > }
> > >
> > > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > > +static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > > {
> > > struct dw_edma *dw = chan->dw;
> > >
> > > @@ -266,7 +266,7 @@ void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> > > FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id)));
> > > }
> > >
> > > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > > +static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > > {
> > > struct dw_edma *dw = chan->dw;
> > >
> > > @@ -274,18 +274,56 @@ void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> > > FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
> > > }
> > >
> > > -u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > > +static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > > {
> > > return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> > > GET_RW_32(dw, dir, int_status));
> > > }
> > >
> > > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > > +static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> > > {
> > > return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> > > GET_RW_32(dw, dir, int_status));
> > > }
> > >
> > > +static
> > > +irqreturn_t dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> > > +{
> > > + struct dw_edma *dw = dw_irq->dw;
> > > + unsigned long total, pos, val;
> > > + struct dw_edma_chan *chan;
> > + int ret = IRQ_NONE;
> > > + unsigned long off;
> > > + u32 mask;
> > > +
> > > + if (dir == EDMA_DIR_WRITE) {
> > > + total = dw->wr_ch_cnt;
> > > + off = 0;
> > > + mask = dw_irq->wr_mask;
> > > + } else {
> > > + total = dw->rd_ch_cnt;
> > > + off = dw->wr_ch_cnt;
> > > + mask = dw_irq->rd_mask;
> > > + }
> > > +
> > > + val = dw_edma_v0_core_status_done_int(dw, dir);
> > > + val &= mask;
> > > + for_each_set_bit(pos, &val, total) {
> > > + chan = &dw->chan[pos + off];
> > + newline
> > > + dw_edma_v0_core_clear_done_int(chan);
> > > + dw_edma_done_interrupt(chan);
> > + newline
> > + ret = IRQ_HANDLED;
> > > + }
> > > +
> > > + val = dw_edma_v0_core_status_abort_int(dw, dir);
> > > + val &= mask;
> > > + for_each_set_bit(pos, &val, total) {
> > > + chan = &dw->chan[pos + off];
> > + newline
> > > + dw_edma_v0_core_clear_abort_int(chan);
> > > + dw_edma_abort_interrupt(chan);
> > + newline
> > + ret = IRQ_HANDLED;
> > > + }
> > > +
> > > - return IRQ_HANDLED;
> > + return ret;
> > > +}
> >
> > Your version of the handler doesn't indicate whether the IRQ was
> > actually handled. Instead it misleadingly returns always-handled
> > status. What if no IRQ flag was actually set and no for-each
> > loop was taken? It's possible for the shared IRQ lines.
> >
> > Besides of adding the actual IRQ-handling status return please note
> > 1. newlines
> > 2. include "linux/irqreturn.h" to the head of the file.
> >
> > -Serge(y)
> >
> > > +
> > > static void dw_edma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
> > > u32 control, u32 size, u64 sar, u64 dar)
> > > {
> > > @@ -356,7 +394,7 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> > > dw_edma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
> > > }
> > >
> > > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > > +static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > > {
> > > struct dw_edma_chan *chan = chunk->chan;
> > > struct dw_edma *dw = chan->dw;
> > > @@ -427,7 +465,7 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > > FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> > > }
> > >
> > > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > > +static void dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
> > > {
> > > struct dw_edma *dw = chan->dw;
> > > u32 tmp = 0;
> > > @@ -494,12 +532,25 @@ int dw_edma_v0_core_device_config(struct dw_edma_chan *chan)
> > > SET_RW_32(dw, chan->dir, ch67_imwr_data, tmp);
> > > break;
> > > }
> > > -
> > > - return 0;
> > > }
> > >
> > > /* eDMA debugfs callbacks */
> > > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > > +static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> > > {
> > > dw_edma_v0_debugfs_on(dw);
> > > }
> > > +
> > > +static const struct dw_edma_core_ops dw_edma_v0_core = {
> > > + .off = dw_edma_v0_core_off,
> > > + .ch_count = dw_edma_v0_core_ch_count,
> > > + .ch_status = dw_edma_v0_core_ch_status,
> > > + .handle_int = dw_edma_v0_core_handle_int,
> > > + .start = dw_edma_v0_core_start,
> > > + .ch_config = dw_edma_v0_core_ch_config,
> > > + .debugfs_on = dw_edma_v0_core_debugfs_on,
> > > +};
> > > +
> > > +void dw_edma_v0_core_register(struct dw_edma *dw)
> > > +{
> > > + dw->core = &dw_edma_v0_core;
> > > +}
> > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.h b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > > index ab96a1f48080..04a882222f99 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.h
> > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.h
> > > @@ -11,17 +11,7 @@
> > >
> > > #include <linux/dma/edma.h>
> > >
> > > -/* eDMA management callbacks */
> > > -void dw_edma_v0_core_off(struct dw_edma *chan);
> > > -u16 dw_edma_v0_core_ch_count(struct dw_edma *chan, enum dw_edma_dir dir);
> > > -enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan);
> > > -void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan);
> > > -void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan);
> > > -u32 dw_edma_v0_core_status_done_int(struct dw_edma *chan, enum dw_edma_dir dir);
> > > -u32 dw_edma_v0_core_status_abort_int(struct dw_edma *chan, enum dw_edma_dir dir);
> > > -void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first);
> > > -int dw_edma_v0_core_device_config(struct dw_edma_chan *chan);
> > > -/* eDMA debug fs callbacks */
> > > -void dw_edma_v0_core_debugfs_on(struct dw_edma *dw);
> > > +/* eDMA core register */
> > > +void dw_edma_v0_core_register(struct dw_edma *dw);
> > >
> > > #endif /* _DW_EDMA_V0_CORE_H */
> > > --
> > > 2.34.1
> > >
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA
2023-03-03 12:46 [PATCH v5 0/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
2023-03-03 12:46 ` [PATCH v5 1/4] dmaengine: dw-edma: Rename dw_edma_core_ops structure to dw_edma_plat_ops Cai Huoqing
2023-03-03 12:46 ` [PATCH v5 2/4] dmaengine: dw-edma: Create a new dw_edma_core_ops structure to abstract controller operation Cai Huoqing
@ 2023-03-03 12:46 ` Cai Huoqing
2023-03-03 13:04 ` Cai Huoqing
` (2 more replies)
2023-03-03 12:46 ` [PATCH v5 4/4] dmaengine: dw-edma: Add HDMA DebugFS support Cai Huoqing
3 siblings, 3 replies; 16+ messages in thread
From: Cai Huoqing @ 2023-03-03 12:46 UTC (permalink / raw)
To: fancer.lancer
Cc: Cai huoqing, Gustavo Pimentel, Vinod Koul, Jingoo Han,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, linux-kernel, dmaengine, linux-pci
From: Cai huoqing <cai.huoqing@linux.dev>
Add support for HDMA NATIVE, as long the IP design has set
the compatible register map parameter-HDMA_NATIVE,
which allows compatibility for native HDMA register configuration.
The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
And the native HDMA registers are different from eDMA, so this patch
add support for HDMA NATIVE mode.
HDMA write and read channels operate independently to maximize
the performance of the HDMA read and write data transfer over
the link When you configure the HDMA with multiple read channels,
then it uses a round robin (RR) arbitration scheme to select
the next read channel to be serviced.The same applies when you
have multiple write channels.
The native HDMA driver also supports a maximum of 16 independent
channels (8 write + 8 read), which can run simultaneously.
Both SAR (Source Address Register) and DAR (Destination Address Register)
are aligned to byte.
Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
---
v4->v5:
1.Add missing the function call -dw_hdma_v0_core_register.
v4 link:
https://lore.kernel.org/lkml/20230221034656.14476-4-cai.huoqing@linux.dev/
drivers/dma/dw-edma/Makefile | 5 +-
drivers/dma/dw-edma/dw-edma-core.c | 6 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 302 ++++++++++++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 129 +++++++++++
include/linux/dma/edma.h | 3 +-
6 files changed, 458 insertions(+), 4 deletions(-)
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
index 8d45c0d5689d..b1c91ef2c63d 100644
--- a/drivers/dma/dw-edma/Makefile
+++ b/drivers/dma/dw-edma/Makefile
@@ -2,6 +2,7 @@
obj-$(CONFIG_DW_EDMA) += dw-edma.o
dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
-dw-edma-objs := dw-edma-core.o \
- dw-edma-v0-core.o $(dw-edma-y)
+dw-edma-objs := dw-edma-core.o \
+ dw-edma-v0-core.o \
+ dw-hdma-v0-core.o $(dw-edma-y)
obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 5cfba5730695..4710009549b6 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -18,6 +18,7 @@
#include "dw-edma-core.h"
#include "dw-edma-v0-core.h"
+#include "dw-hdma-v0-core.h"
#include "../dmaengine.h"
#include "../virt-dma.h"
@@ -914,7 +915,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
dw->chip = chip;
- dw_edma_v0_core_register(dw);
+ if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
+ dw_hdma_v0_core_register(dw);
+ else
+ dw_edma_v0_core_register(dw);
raw_spin_lock_init(&dw->lock);
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
new file mode 100644
index 000000000000..e14a3907241d
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 core
+ */
+
+#include <linux/bitfield.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+
+#include "dw-edma-core.h"
+#include "dw-hdma-v0-core.h"
+#include "dw-hdma-v0-regs.h"
+
+enum dw_hdma_control {
+ DW_HDMA_V0_CB = BIT(0),
+ DW_HDMA_V0_TCB = BIT(1),
+ DW_HDMA_V0_LLP = BIT(2),
+ DW_HDMA_V0_LIE = BIT(3),
+ DW_HDMA_V0_RIE = BIT(4),
+ DW_HDMA_V0_CCS = BIT(8),
+ DW_HDMA_V0_LLE = BIT(9),
+};
+
+static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
+{
+ return dw->chip->reg_base;
+}
+
+static inline struct dw_hdma_v0_ch_regs __iomem *
+__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
+{
+ if (dir == EDMA_DIR_WRITE)
+ return &(__dw_regs(dw)->ch[ch].wr);
+ else
+ return &(__dw_regs(dw)->ch[ch].rd);
+}
+
+#define SET_CH_32(dw, dir, ch, name, value) \
+ writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
+
+#define GET_CH_32(dw, dir, ch, name) \
+ readl(&(__dw_ch_regs(dw, dir, ch)->name))
+
+#define SET_BOTH_CH_32(dw, ch, name, value) \
+ do { \
+ writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
+ writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
+ } while (0)
+
+/* HDMA management callbacks */
+static void dw_hdma_v0_core_off(struct dw_edma *dw)
+{
+ int id;
+
+ for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
+ SET_BOTH_CH_32(dw, id, int_setup,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ SET_BOTH_CH_32(dw, id, int_clear,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ SET_BOTH_CH_32(dw, id, ch_en, 0);
+ }
+}
+
+static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
+{
+ u32 num_ch = 0;
+ int id;
+
+ for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
+ if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
+ num_ch++;
+ }
+
+ if (num_ch > HDMA_V0_MAX_NR_CH)
+ num_ch = HDMA_V0_MAX_NR_CH;
+
+ return (u16)num_ch;
+}
+
+static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+ u32 tmp;
+
+ tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
+ GET_CH_32(dw, chan->id, chan->dir, ch_stat));
+
+ if (tmp == 1)
+ return DMA_IN_PROGRESS;
+ else if (tmp == 3)
+ return DMA_COMPLETE;
+ else
+ return DMA_ERROR;
+}
+
+static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
+}
+
+static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
+}
+
+static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ return GET_CH_32(dw, chan->dir, chan->id, int_stat);
+}
+
+static u32 dw_hdma_v0_core_check_done_int(u32 val)
+{
+ return FIELD_GET(HDMA_V0_STOP_INT_MASK, val);
+}
+
+static u32 dw_hdma_v0_core_check_abort_int(u32 val)
+{
+ return FIELD_GET(HDMA_V0_ABORT_INT_MASK, val);
+}
+
+static
+irqreturn_t dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
+{
+ struct dw_edma *dw = dw_irq->dw;
+ unsigned long total, pos, val;
+ struct dw_edma_chan *chan;
+ unsigned long off, mask;
+
+ if (dir == EDMA_DIR_WRITE) {
+ total = dw->wr_ch_cnt;
+ off = 0;
+ mask = dw_irq->wr_mask;
+ } else {
+ total = dw->rd_ch_cnt;
+ off = dw->wr_ch_cnt;
+ mask = dw_irq->rd_mask;
+ }
+
+ for_each_set_bit(pos, &mask, total) {
+ chan = &dw->chan[pos + off];
+
+ val = dw_hdma_v0_core_status_int(chan);
+ if (dw_hdma_v0_core_check_done_int(val)) {
+ dw_hdma_v0_core_clear_done_int(chan);
+ dw_edma_done_interrupt(chan);
+ }
+ }
+
+ for_each_set_bit(pos, &mask, total) {
+ chan = &dw->chan[pos + off];
+
+ val = dw_hdma_v0_core_status_int(chan);
+ if (dw_hdma_v0_core_check_abort_int(val)) {
+ dw_hdma_v0_core_clear_abort_int(chan);
+ dw_edma_abort_interrupt(&dw->chan[pos + off]);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void dw_hdma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
+ u32 control, u32 size, u64 sar, u64 dar)
+{
+ ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
+
+ if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ struct dw_hdma_v0_lli *lli = chunk->ll_region.vaddr.mem + ofs;
+
+ lli->control = control;
+ lli->transfer_size = size;
+ lli->sar.reg = sar;
+ lli->dar.reg = dar;
+ } else {
+ struct dw_hdma_v0_lli __iomem *lli = chunk->ll_region.vaddr.io + ofs;
+
+ writel(control, &lli->control);
+ writel(size, &lli->transfer_size);
+ writeq(sar, &lli->sar.reg);
+ writeq(dar, &lli->dar.reg);
+ }
+}
+
+static void dw_hdma_v0_write_ll_link(struct dw_edma_chunk *chunk,
+ int i, u32 control, u64 pointer)
+{
+ ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
+
+ if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ struct dw_hdma_v0_llp *llp = chunk->ll_region.vaddr.mem + ofs;
+
+ llp->control = control;
+ llp->llp.reg = pointer;
+ } else {
+ struct dw_hdma_v0_llp __iomem *llp = chunk->ll_region.vaddr.io + ofs;
+
+ writel(control, &llp->control);
+ writeq(pointer, &llp->llp.reg);
+ }
+}
+
+static void dw_hdma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
+{
+ struct dw_edma_burst *child;
+ struct dw_edma_chan *chan = chunk->chan;
+ u32 control = 0, i = 0;
+ int j;
+
+ if (chunk->cb)
+ control = DW_HDMA_V0_CB;
+
+ j = chunk->bursts_alloc;
+ list_for_each_entry(child, &chunk->burst->list, list) {
+ j--;
+ if (!j) {
+ control |= DW_HDMA_V0_LIE;
+ if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+ control |= DW_HDMA_V0_RIE;
+ }
+
+ dw_hdma_v0_write_ll_data(chunk, i++, control, child->sz,
+ child->sar, child->dar);
+ }
+
+ control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
+ if (!chunk->cb)
+ control |= DW_HDMA_V0_CB;
+
+ dw_hdma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
+}
+
+static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+{
+ struct dw_edma_chan *chan = chunk->chan;
+ struct dw_edma *dw = chan->dw;
+ u32 tmp;
+
+ dw_hdma_v0_core_write_chunk(chunk);
+
+ if (first) {
+ /* Enable engine */
+ SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
+ /* Interrupt enable&unmask - done, abort */
+ tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
+ HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
+ SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
+ /* Channel control */
+ SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
+ /* Linked list */
+ /* llp is not aligned on 64bit -> keep 32bit accesses */
+ SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
+ lower_32_bits(chunk->ll_region.paddr));
+ SET_CH_32(dw, chan->dir, chan->id, llp.msb,
+ upper_32_bits(chunk->ll_region.paddr));
+ }
+ /* Set consumer cycle */
+ SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
+ HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
+ /* Doorbell */
+ SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
+}
+
+static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ /* MSI done addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
+ /* MSI abort addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
+ /* config MSI data */
+ SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
+}
+
+/* HDMA debugfs callbacks */
+static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
+{
+}
+
+static const struct dw_edma_core_ops dw_hdma_v0_core = {
+ .off = dw_hdma_v0_core_off,
+ .ch_count = dw_hdma_v0_core_ch_count,
+ .ch_status = dw_hdma_v0_core_ch_status,
+ .handle_int = dw_hdma_v0_core_handle_int,
+ .start = dw_hdma_v0_core_start,
+ .ch_config = dw_hdma_v0_core_ch_config,
+ .debugfs_on = dw_hdma_v0_core_debugfs_on,
+};
+
+void dw_hdma_v0_core_register(struct dw_edma *dw)
+{
+ dw->core = &dw_hdma_v0_core;
+}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
new file mode 100644
index 000000000000..c373b4f0bd8a
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 core
+ *
+ * Author: Cai Huoqing <cai.huoqing@linux.dev>
+ */
+
+#ifndef _DW_HDMA_V0_CORE_H
+#define _DW_HDMA_V0_CORE_H
+
+#include <linux/dma/edma.h>
+
+/* HDMA core register */
+void dw_hdma_v0_core_register(struct dw_edma *dw);
+
+#endif /* _DW_HDMA_V0_CORE_H */
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
new file mode 100644
index 000000000000..61359d50e9f4
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
@@ -0,0 +1,129 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 reg
+ *
+ * Author: Cai Huoqing <cai.huoqing@linux.dev>
+ */
+
+#ifndef _DW_HDMA_V0_REGS_H
+#define _DW_HDMA_V0_REGS_H
+
+#include <linux/dmaengine.h>
+
+#define HDMA_V0_MAX_NR_CH 8
+#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
+#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
+#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
+#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
+#define HDMA_V0_ABORT_INT_MASK BIT(2)
+#define HDMA_V0_STOP_INT_MASK BIT(0)
+#define HDMA_V0_LINKLIST_EN BIT(0)
+#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
+#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
+#define HDMA_V0_DOORBELL_START BIT(0)
+#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
+
+struct dw_hdma_v0_ch_regs {
+ u32 ch_en; /* 0x0000 */
+ u32 doorbell; /* 0x0004 */
+ u32 prefetch; /* 0x0008 */
+ u32 handshake; /* 0x000c */
+ union {
+ u64 reg; /* 0x0010..0x0014 */
+ struct {
+ u32 lsb; /* 0x0010 */
+ u32 msb; /* 0x0014 */
+ };
+ } llp;
+ u32 cycle_sync; /* 0x0018 */
+ u32 transfer_size; /* 0x001c */
+ union {
+ u64 reg; /* 0x0020..0x0024 */
+ struct {
+ u32 lsb; /* 0x0020 */
+ u32 msb; /* 0x0024 */
+ };
+ } sar;
+ union {
+ u64 reg; /* 0x0028..0x002c */
+ struct {
+ u32 lsb; /* 0x0028 */
+ u32 msb; /* 0x002c */
+ };
+ } dar;
+
+ u32 watermark_en; /* 0x0030 */
+ u32 control1; /* 0x0034 */
+ u32 func_num; /* 0x0038 */
+ u32 qos; /* 0x003c */
+ u32 reserved; /* 0x0040..0x007c */
+ u32 ch_stat; /* 0x0080 */
+ u32 int_stat; /* 0x0084 */
+ u32 int_setup; /* 0x0088 */
+ u32 int_clear; /* 0x008c */
+ union {
+ u64 reg; /* 0x0090..0x0094 */
+ struct {
+ u32 lsb; /* 0x0090 */
+ u32 msb; /* 0x0094 */
+ };
+ } msi_stop;
+ union {
+ u64 reg; /* 0x0098..0x009c */
+ struct {
+ u32 lsb; /* 0x0098 */
+ u32 msb; /* 0x009c */
+ };
+ } msi_watermark;
+ union {
+ u64 reg; /* 0x00a0..0x00a4 */
+ struct {
+ u32 lsb; /* 0x00a0 */
+ u32 msb; /* 0x00a4 */
+ };
+ } msi_abort;
+ u32 msi_msgdata; /* 0x00a8 */
+} __packed;
+
+struct dw_hdma_v0_ch {
+ struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
+ struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
+} __packed;
+
+struct dw_hdma_v0_regs {
+ struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
+} __packed;
+
+struct dw_hdma_v0_lli {
+ u32 control;
+ u32 transfer_size;
+ union {
+ u64 reg;
+ struct {
+ u32 lsb;
+ u32 msb;
+ };
+ } sar;
+ union {
+ u64 reg;
+ struct {
+ u32 lsb;
+ u32 msb;
+ };
+ } dar;
+} __packed;
+
+struct dw_hdma_v0_llp {
+ u32 control;
+ u32 reserved;
+ union {
+ u64 reg;
+ struct {
+ u32 lsb;
+ u32 msb;
+ };
+ } llp;
+} __packed;
+
+#endif /* _DW_HDMA_V0_REGS_H */
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index ed401c965a87..3080747689f6 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -48,7 +48,8 @@ struct dw_edma_plat_ops {
enum dw_edma_map_format {
EDMA_MF_EDMA_LEGACY = 0x0,
EDMA_MF_EDMA_UNROLL = 0x1,
- EDMA_MF_HDMA_COMPAT = 0x5
+ EDMA_MF_HDMA_COMPAT = 0x5,
+ EDMA_MF_HDMA_NATIVE = 0x7,
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA
2023-03-03 12:46 ` [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
@ 2023-03-03 13:04 ` Cai Huoqing
2023-03-03 18:12 ` Serge Semin
2023-03-04 6:51 ` kernel test robot
2 siblings, 0 replies; 16+ messages in thread
From: Cai Huoqing @ 2023-03-03 13:04 UTC (permalink / raw)
To: fancer.lancer, lars
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On 03 3月 23 20:46:33, Cai Huoqing wrote:
> From: Cai huoqing <cai.huoqing@linux.dev>
+ Lars-Peter
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA, so this patch
> add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.The same applies when you
> have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are aligned to byte.
>
> Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> ---
> v4->v5:
> 1.Add missing the function call -dw_hdma_v0_core_register.
>
> v4 link:
> https://lore.kernel.org/lkml/20230221034656.14476-4-cai.huoqing@linux.dev/
>
> drivers/dma/dw-edma/Makefile | 5 +-
> drivers/dma/dw-edma/dw-edma-core.c | 6 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 302 ++++++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 129 +++++++++++
> include/linux/dma/edma.h | 3 +-
> 6 files changed, 458 insertions(+), 4 deletions(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
> index 8d45c0d5689d..b1c91ef2c63d 100644
> --- a/drivers/dma/dw-edma/Makefile
> +++ b/drivers/dma/dw-edma/Makefile
> @@ -2,6 +2,7 @@
>
> obj-$(CONFIG_DW_EDMA) += dw-edma.o
> dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
> -dw-edma-objs := dw-edma-core.o \
> - dw-edma-v0-core.o $(dw-edma-y)
> +dw-edma-objs := dw-edma-core.o \
> + dw-edma-v0-core.o \
> + dw-hdma-v0-core.o $(dw-edma-y)
> obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 5cfba5730695..4710009549b6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -18,6 +18,7 @@
>
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> +#include "dw-hdma-v0-core.h"
> #include "../dmaengine.h"
> #include "../virt-dma.h"
>
> @@ -914,7 +915,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> - dw_edma_v0_core_register(dw);
> + if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
> + dw_hdma_v0_core_register(dw);
> + else
> + dw_edma_v0_core_register(dw);
>
> raw_spin_lock_init(&dw->lock);
>
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> new file mode 100644
> index 000000000000..e14a3907241d
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +
> +#include "dw-edma-core.h"
> +#include "dw-hdma-v0-core.h"
> +#include "dw-hdma-v0-regs.h"
> +
> +enum dw_hdma_control {
> + DW_HDMA_V0_CB = BIT(0),
> + DW_HDMA_V0_TCB = BIT(1),
> + DW_HDMA_V0_LLP = BIT(2),
> + DW_HDMA_V0_LIE = BIT(3),
> + DW_HDMA_V0_RIE = BIT(4),
> + DW_HDMA_V0_CCS = BIT(8),
> + DW_HDMA_V0_LLE = BIT(9),
> +};
> +
> +static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> +{
> + return dw->chip->reg_base;
> +}
> +
> +static inline struct dw_hdma_v0_ch_regs __iomem *
> +__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
> +{
> + if (dir == EDMA_DIR_WRITE)
> + return &(__dw_regs(dw)->ch[ch].wr);
> + else
> + return &(__dw_regs(dw)->ch[ch].rd);
> +}
> +
> +#define SET_CH_32(dw, dir, ch, name, value) \
> + writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define GET_CH_32(dw, dir, ch, name) \
> + readl(&(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define SET_BOTH_CH_32(dw, ch, name, value) \
> + do { \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
> + } while (0)
> +
> +/* HDMA management callbacks */
> +static void dw_hdma_v0_core_off(struct dw_edma *dw)
> +{
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + SET_BOTH_CH_32(dw, id, int_setup,
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, int_clear,
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, ch_en, 0);
> + }
> +}
> +
> +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> + u32 num_ch = 0;
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> + num_ch++;
> + }
> +
> + if (num_ch > HDMA_V0_MAX_NR_CH)
> + num_ch = HDMA_V0_MAX_NR_CH;
> +
> + return (u16)num_ch;
> +}
> +
> +static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> + GET_CH_32(dw, chan->id, chan->dir, ch_stat));
> +
> + if (tmp == 1)
> + return DMA_IN_PROGRESS;
> + else if (tmp == 3)
> + return DMA_COMPLETE;
> + else
> + return DMA_ERROR;
> +}
> +
> +static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
> +}
> +
> +static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
> +}
> +
> +static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + return GET_CH_32(dw, chan->dir, chan->id, int_stat);
> +}
> +
> +static u32 dw_hdma_v0_core_check_done_int(u32 val)
> +{
> + return FIELD_GET(HDMA_V0_STOP_INT_MASK, val);
> +}
> +
> +static u32 dw_hdma_v0_core_check_abort_int(u32 val)
> +{
> + return FIELD_GET(HDMA_V0_ABORT_INT_MASK, val);
> +}
> +
> +static
> +irqreturn_t dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
> + struct dw_edma_chan *chan;
> + unsigned long off, mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_done_int(val)) {
> + dw_hdma_v0_core_clear_done_int(chan);
> + dw_edma_done_interrupt(chan);
> + }
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_abort_int(val)) {
> + dw_hdma_v0_core_clear_abort_int(chan);
> + dw_edma_abort_interrupt(&dw->chan[pos + off]);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void dw_hdma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
> + u32 control, u32 size, u64 sar, u64 dar)
> +{
> + ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
> +
> + if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
> + struct dw_hdma_v0_lli *lli = chunk->ll_region.vaddr.mem + ofs;
> +
> + lli->control = control;
> + lli->transfer_size = size;
> + lli->sar.reg = sar;
> + lli->dar.reg = dar;
> + } else {
> + struct dw_hdma_v0_lli __iomem *lli = chunk->ll_region.vaddr.io + ofs;
> +
> + writel(control, &lli->control);
> + writel(size, &lli->transfer_size);
> + writeq(sar, &lli->sar.reg);
> + writeq(dar, &lli->dar.reg);
> + }
> +}
> +
> +static void dw_hdma_v0_write_ll_link(struct dw_edma_chunk *chunk,
> + int i, u32 control, u64 pointer)
> +{
> + ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
> +
> + if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
> + struct dw_hdma_v0_llp *llp = chunk->ll_region.vaddr.mem + ofs;
> +
> + llp->control = control;
> + llp->llp.reg = pointer;
> + } else {
> + struct dw_hdma_v0_llp __iomem *llp = chunk->ll_region.vaddr.io + ofs;
> +
> + writel(control, &llp->control);
> + writeq(pointer, &llp->llp.reg);
> + }
> +}
> +
> +static void dw_hdma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> +{
> + struct dw_edma_burst *child;
> + struct dw_edma_chan *chan = chunk->chan;
> + u32 control = 0, i = 0;
> + int j;
> +
> + if (chunk->cb)
> + control = DW_HDMA_V0_CB;
> +
> + j = chunk->bursts_alloc;
> + list_for_each_entry(child, &chunk->burst->list, list) {
> + j--;
> + if (!j) {
> + control |= DW_HDMA_V0_LIE;
> + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> + control |= DW_HDMA_V0_RIE;
> + }
> +
> + dw_hdma_v0_write_ll_data(chunk, i++, control, child->sz,
> + child->sar, child->dar);
> + }
> +
> + control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
> + if (!chunk->cb)
> + control |= DW_HDMA_V0_CB;
> +
> + dw_hdma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
> +}
> +
> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> + struct dw_edma_chan *chan = chunk->chan;
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + dw_hdma_v0_core_write_chunk(chunk);
> +
> + if (first) {
> + /* Enable engine */
> + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> + /* Interrupt enable&unmask - done, abort */
> + tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
> + HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
> + SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
> + /* Channel control */
> + SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
> + /* Linked list */
> + /* llp is not aligned on 64bit -> keep 32bit accesses */
> + SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
> + lower_32_bits(chunk->ll_region.paddr));
> + SET_CH_32(dw, chan->dir, chan->id, llp.msb,
> + upper_32_bits(chunk->ll_region.paddr));
> + }
> + /* Set consumer cycle */
> + SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
> + HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
> + /* Doorbell */
> + SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> +}
> +
> +static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + /* MSI done addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
> + /* MSI abort addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
> + /* config MSI data */
> + SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
> +}
> +
> +/* HDMA debugfs callbacks */
> +static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> +{
> +}
> +
> +static const struct dw_edma_core_ops dw_hdma_v0_core = {
> + .off = dw_hdma_v0_core_off,
> + .ch_count = dw_hdma_v0_core_ch_count,
> + .ch_status = dw_hdma_v0_core_ch_status,
> + .handle_int = dw_hdma_v0_core_handle_int,
> + .start = dw_hdma_v0_core_start,
> + .ch_config = dw_hdma_v0_core_ch_config,
> + .debugfs_on = dw_hdma_v0_core_debugfs_on,
> +};
> +
> +void dw_hdma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &dw_hdma_v0_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> new file mode 100644
> index 000000000000..c373b4f0bd8a
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + *
> + * Author: Cai Huoqing <cai.huoqing@linux.dev>
> + */
> +
> +#ifndef _DW_HDMA_V0_CORE_H
> +#define _DW_HDMA_V0_CORE_H
> +
> +#include <linux/dma/edma.h>
> +
> +/* HDMA core register */
> +void dw_hdma_v0_core_register(struct dw_edma *dw);
> +
> +#endif /* _DW_HDMA_V0_CORE_H */
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> new file mode 100644
> index 000000000000..61359d50e9f4
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> @@ -0,0 +1,129 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 reg
> + *
> + * Author: Cai Huoqing <cai.huoqing@linux.dev>
> + */
> +
> +#ifndef _DW_HDMA_V0_REGS_H
> +#define _DW_HDMA_V0_REGS_H
> +
> +#include <linux/dmaengine.h>
> +
> +#define HDMA_V0_MAX_NR_CH 8
> +#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
> +#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
> +#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
> +#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
> +#define HDMA_V0_ABORT_INT_MASK BIT(2)
> +#define HDMA_V0_STOP_INT_MASK BIT(0)
> +#define HDMA_V0_LINKLIST_EN BIT(0)
> +#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
> +#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
> +#define HDMA_V0_DOORBELL_START BIT(0)
> +#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
> +
> +struct dw_hdma_v0_ch_regs {
> + u32 ch_en; /* 0x0000 */
> + u32 doorbell; /* 0x0004 */
> + u32 prefetch; /* 0x0008 */
> + u32 handshake; /* 0x000c */
> + union {
> + u64 reg; /* 0x0010..0x0014 */
> + struct {
> + u32 lsb; /* 0x0010 */
> + u32 msb; /* 0x0014 */
> + };
> + } llp;
> + u32 cycle_sync; /* 0x0018 */
> + u32 transfer_size; /* 0x001c */
> + union {
> + u64 reg; /* 0x0020..0x0024 */
> + struct {
> + u32 lsb; /* 0x0020 */
> + u32 msb; /* 0x0024 */
> + };
> + } sar;
> + union {
> + u64 reg; /* 0x0028..0x002c */
> + struct {
> + u32 lsb; /* 0x0028 */
> + u32 msb; /* 0x002c */
> + };
> + } dar;
> +
> + u32 watermark_en; /* 0x0030 */
> + u32 control1; /* 0x0034 */
> + u32 func_num; /* 0x0038 */
> + u32 qos; /* 0x003c */
> + u32 reserved; /* 0x0040..0x007c */
> + u32 ch_stat; /* 0x0080 */
> + u32 int_stat; /* 0x0084 */
> + u32 int_setup; /* 0x0088 */
> + u32 int_clear; /* 0x008c */
> + union {
> + u64 reg; /* 0x0090..0x0094 */
> + struct {
> + u32 lsb; /* 0x0090 */
> + u32 msb; /* 0x0094 */
> + };
> + } msi_stop;
> + union {
> + u64 reg; /* 0x0098..0x009c */
> + struct {
> + u32 lsb; /* 0x0098 */
> + u32 msb; /* 0x009c */
> + };
> + } msi_watermark;
> + union {
> + u64 reg; /* 0x00a0..0x00a4 */
> + struct {
> + u32 lsb; /* 0x00a0 */
> + u32 msb; /* 0x00a4 */
> + };
> + } msi_abort;
> + u32 msi_msgdata; /* 0x00a8 */
> +} __packed;
> +
> +struct dw_hdma_v0_ch {
> + struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
> + struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
> +} __packed;
> +
> +struct dw_hdma_v0_regs {
> + struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
> +} __packed;
> +
> +struct dw_hdma_v0_lli {
> + u32 control;
> + u32 transfer_size;
> + union {
> + u64 reg;
> + struct {
> + u32 lsb;
> + u32 msb;
> + };
> + } sar;
> + union {
> + u64 reg;
> + struct {
> + u32 lsb;
> + u32 msb;
> + };
> + } dar;
> +} __packed;
> +
> +struct dw_hdma_v0_llp {
> + u32 control;
> + u32 reserved;
> + union {
> + u64 reg;
> + struct {
> + u32 lsb;
> + u32 msb;
> + };
> + } llp;
> +} __packed;
> +
> +#endif /* _DW_HDMA_V0_REGS_H */
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index ed401c965a87..3080747689f6 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -48,7 +48,8 @@ struct dw_edma_plat_ops {
> enum dw_edma_map_format {
> EDMA_MF_EDMA_LEGACY = 0x0,
> EDMA_MF_EDMA_UNROLL = 0x1,
> - EDMA_MF_HDMA_COMPAT = 0x5
> + EDMA_MF_HDMA_COMPAT = 0x5,
> + EDMA_MF_HDMA_NATIVE = 0x7,
> };
>
> /**
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA
2023-03-03 12:46 ` [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
2023-03-03 13:04 ` Cai Huoqing
@ 2023-03-03 18:12 ` Serge Semin
2023-03-04 6:51 ` kernel test robot
2 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2023-03-03 18:12 UTC (permalink / raw)
To: Cai Huoqing
Cc: lars, Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Fri, Mar 03, 2023 at 08:46:33PM +0800, Cai Huoqing wrote:
> From: Cai huoqing <cai.huoqing@linux.dev>
>
> Add support for HDMA NATIVE, as long the IP design has set
> the compatible register map parameter-HDMA_NATIVE,
> which allows compatibility for native HDMA register configuration.
>
> The HDMA Hyper-DMA IP is an enhancement of the eDMA embedded-DMA IP.
> And the native HDMA registers are different from eDMA, so this patch
> add support for HDMA NATIVE mode.
>
> HDMA write and read channels operate independently to maximize
> the performance of the HDMA read and write data transfer over
> the link When you configure the HDMA with multiple read channels,
> then it uses a round robin (RR) arbitration scheme to select
> the next read channel to be serviced.The same applies when you
> have multiple write channels.
>
> The native HDMA driver also supports a maximum of 16 independent
> channels (8 write + 8 read), which can run simultaneously.
> Both SAR (Source Address Register) and DAR (Destination Address Register)
> are aligned to byte.
>
> Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> ---
> v4->v5:
> 1.Add missing the function call -dw_hdma_v0_core_register.
>
> v4 link:
> https://lore.kernel.org/lkml/20230221034656.14476-4-cai.huoqing@linux.dev/
>
> drivers/dma/dw-edma/Makefile | 5 +-
> drivers/dma/dw-edma/dw-edma-core.c | 6 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 302 ++++++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.h | 17 ++
> drivers/dma/dw-edma/dw-hdma-v0-regs.h | 129 +++++++++++
> include/linux/dma/edma.h | 3 +-
> 6 files changed, 458 insertions(+), 4 deletions(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-core.h
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-regs.h
>
> diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
> index 8d45c0d5689d..b1c91ef2c63d 100644
> --- a/drivers/dma/dw-edma/Makefile
> +++ b/drivers/dma/dw-edma/Makefile
> @@ -2,6 +2,7 @@
>
> obj-$(CONFIG_DW_EDMA) += dw-edma.o
> dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
> -dw-edma-objs := dw-edma-core.o \
> - dw-edma-v0-core.o $(dw-edma-y)
> +dw-edma-objs := dw-edma-core.o \
> + dw-edma-v0-core.o \
> + dw-hdma-v0-core.o $(dw-edma-y)
> obj-$(CONFIG_DW_EDMA_PCIE) += dw-edma-pcie.o
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 5cfba5730695..4710009549b6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -18,6 +18,7 @@
>
> #include "dw-edma-core.h"
> #include "dw-edma-v0-core.h"
> +#include "dw-hdma-v0-core.h"
> #include "../dmaengine.h"
> #include "../virt-dma.h"
>
> @@ -914,7 +915,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> dw->chip = chip;
>
> - dw_edma_v0_core_register(dw);
> + if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
> + dw_hdma_v0_core_register(dw);
> + else
> + dw_edma_v0_core_register(dw);
>
> raw_spin_lock_init(&dw->lock);
>
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> new file mode 100644
> index 000000000000..e14a3907241d
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +
> +#include "dw-edma-core.h"
> +#include "dw-hdma-v0-core.h"
> +#include "dw-hdma-v0-regs.h"
> +
> +enum dw_hdma_control {
> + DW_HDMA_V0_CB = BIT(0),
> + DW_HDMA_V0_TCB = BIT(1),
> + DW_HDMA_V0_LLP = BIT(2),
> + DW_HDMA_V0_LIE = BIT(3),
> + DW_HDMA_V0_RIE = BIT(4),
> + DW_HDMA_V0_CCS = BIT(8),
> + DW_HDMA_V0_LLE = BIT(9),
> +};
> +
> +static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
> +{
> + return dw->chip->reg_base;
> +}
> +
> +static inline struct dw_hdma_v0_ch_regs __iomem *
> +__dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
> +{
> + if (dir == EDMA_DIR_WRITE)
> + return &(__dw_regs(dw)->ch[ch].wr);
> + else
> + return &(__dw_regs(dw)->ch[ch].rd);
> +}
> +
> +#define SET_CH_32(dw, dir, ch, name, value) \
> + writel(value, &(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define GET_CH_32(dw, dir, ch, name) \
> + readl(&(__dw_ch_regs(dw, dir, ch)->name))
> +
> +#define SET_BOTH_CH_32(dw, ch, name, value) \
> + do { \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_WRITE, ch)->name)); \
> + writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
> + } while (0)
> +
> +/* HDMA management callbacks */
> +static void dw_hdma_v0_core_off(struct dw_edma *dw)
> +{
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + SET_BOTH_CH_32(dw, id, int_setup,
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, int_clear,
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> + SET_BOTH_CH_32(dw, id, ch_en, 0);
> + }
> +}
> +
> +static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> + u32 num_ch = 0;
> + int id;
> +
> + for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> + if (GET_CH_32(dw, id, dir, ch_en) & BIT(0))
> + num_ch++;
> + }
> +
> + if (num_ch > HDMA_V0_MAX_NR_CH)
> + num_ch = HDMA_V0_MAX_NR_CH;
> +
> + return (u16)num_ch;
> +}
> +
> +static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> + GET_CH_32(dw, chan->id, chan->dir, ch_stat));
> +
> + if (tmp == 1)
> + return DMA_IN_PROGRESS;
> + else if (tmp == 3)
> + return DMA_COMPLETE;
> + else
> + return DMA_ERROR;
> +}
> +
> +static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
> +}
> +
> +static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_ABORT_INT_MASK);
> +}
> +
> +static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + return GET_CH_32(dw, chan->dir, chan->id, int_stat);
> +}
> +
> +static u32 dw_hdma_v0_core_check_done_int(u32 val)
> +{
> + return FIELD_GET(HDMA_V0_STOP_INT_MASK, val);
> +}
> +
> +static u32 dw_hdma_v0_core_check_abort_int(u32 val)
> +{
> + return FIELD_GET(HDMA_V0_ABORT_INT_MASK, val);
> +}
I don't see a good reason to have these methods seeing the
register value is passed as an argument.
> +
> +static
> +irqreturn_t dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir)
> +{
> + struct dw_edma *dw = dw_irq->dw;
> + unsigned long total, pos, val;
+ irqreturn_t ret = IRQ_NONE;
> + struct dw_edma_chan *chan;
> + unsigned long off, mask;
> +
> + if (dir == EDMA_DIR_WRITE) {
> + total = dw->wr_ch_cnt;
> + off = 0;
> + mask = dw_irq->wr_mask;
> + } else {
> + total = dw->rd_ch_cnt;
> + off = dw->wr_ch_cnt;
> + mask = dw_irq->rd_mask;
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_done_int(val)) {
> + dw_hdma_v0_core_clear_done_int(chan);
> + dw_edma_done_interrupt(chan);
> + }
> + }
> +
> + for_each_set_bit(pos, &mask, total) {
> + chan = &dw->chan[pos + off];
> +
> + val = dw_hdma_v0_core_status_int(chan);
> + if (dw_hdma_v0_core_check_abort_int(val)) {
> + dw_hdma_v0_core_clear_abort_int(chan);
> + dw_edma_abort_interrupt(&dw->chan[pos + off]);
> + }
> + }
Just noticed that implementing two loops is pointless in your case.
Having one loop is enough:
< for_each_set_bit(pos, &mask, total) {
< chan = &dw->chan[pos + off];
<
< val = dw_hdma_v0_core_status_int(chan);
<
< if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
< dw_hdma_v0_core_clear_abort_int(chan);
< dw_edma_abort_interrupt(&dw->chan[pos + off]);
<
< ret = IRQ_HANDLED;
< }
<
< if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
< dw_hdma_v0_core_clear_abort_int(chan);
< dw_edma_abort_interrupt(&dw->chan[pos + off]);
<
< ret = IRQ_HANDLED;
< }
< }
* BTW The same optimization could be done in the DW eDMA v0 core, but
* it should be done in a separate patch after your [PATCH 2/4] change
* is applied.
> +
> - return IRQ_HANDLED;
+ return ret;
Please fix the method to returning the actual IRQ-handling status.
-Serge(y)
> +}
> +
> +static void dw_hdma_v0_write_ll_data(struct dw_edma_chunk *chunk, int i,
> + u32 control, u32 size, u64 sar, u64 dar)
> +{
> + ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
> +
> + if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
> + struct dw_hdma_v0_lli *lli = chunk->ll_region.vaddr.mem + ofs;
> +
> + lli->control = control;
> + lli->transfer_size = size;
> + lli->sar.reg = sar;
> + lli->dar.reg = dar;
> + } else {
> + struct dw_hdma_v0_lli __iomem *lli = chunk->ll_region.vaddr.io + ofs;
> +
> + writel(control, &lli->control);
> + writel(size, &lli->transfer_size);
> + writeq(sar, &lli->sar.reg);
> + writeq(dar, &lli->dar.reg);
> + }
> +}
> +
> +static void dw_hdma_v0_write_ll_link(struct dw_edma_chunk *chunk,
> + int i, u32 control, u64 pointer)
> +{
> + ptrdiff_t ofs = i * sizeof(struct dw_hdma_v0_lli);
> +
> + if (chunk->chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
> + struct dw_hdma_v0_llp *llp = chunk->ll_region.vaddr.mem + ofs;
> +
> + llp->control = control;
> + llp->llp.reg = pointer;
> + } else {
> + struct dw_hdma_v0_llp __iomem *llp = chunk->ll_region.vaddr.io + ofs;
> +
> + writel(control, &llp->control);
> + writeq(pointer, &llp->llp.reg);
> + }
> +}
> +
> +static void dw_hdma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
> +{
> + struct dw_edma_burst *child;
> + struct dw_edma_chan *chan = chunk->chan;
> + u32 control = 0, i = 0;
> + int j;
> +
> + if (chunk->cb)
> + control = DW_HDMA_V0_CB;
> +
> + j = chunk->bursts_alloc;
> + list_for_each_entry(child, &chunk->burst->list, list) {
> + j--;
> + if (!j) {
> + control |= DW_HDMA_V0_LIE;
> + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> + control |= DW_HDMA_V0_RIE;
> + }
> +
> + dw_hdma_v0_write_ll_data(chunk, i++, control, child->sz,
> + child->sar, child->dar);
> + }
> +
> + control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
> + if (!chunk->cb)
> + control |= DW_HDMA_V0_CB;
> +
> + dw_hdma_v0_write_ll_link(chunk, i, control, chunk->ll_region.paddr);
> +}
> +
> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> + struct dw_edma_chan *chan = chunk->chan;
> + struct dw_edma *dw = chan->dw;
> + u32 tmp;
> +
> + dw_hdma_v0_core_write_chunk(chunk);
> +
> + if (first) {
> + /* Enable engine */
> + SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> + /* Interrupt enable&unmask - done, abort */
> + tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> + HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
> + HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
> + SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
> + /* Channel control */
> + SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
> + /* Linked list */
> + /* llp is not aligned on 64bit -> keep 32bit accesses */
> + SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
> + lower_32_bits(chunk->ll_region.paddr));
> + SET_CH_32(dw, chan->dir, chan->id, llp.msb,
> + upper_32_bits(chunk->ll_region.paddr));
> + }
> + /* Set consumer cycle */
> + SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
> + HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
> + /* Doorbell */
> + SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> +}
> +
> +static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> +{
> + struct dw_edma *dw = chan->dw;
> +
> + /* MSI done addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
> + /* MSI abort addr - low, high */
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
> + SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
> + /* config MSI data */
> + SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
> +}
> +
> +/* HDMA debugfs callbacks */
> +static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> +{
> +}
> +
> +static const struct dw_edma_core_ops dw_hdma_v0_core = {
> + .off = dw_hdma_v0_core_off,
> + .ch_count = dw_hdma_v0_core_ch_count,
> + .ch_status = dw_hdma_v0_core_ch_status,
> + .handle_int = dw_hdma_v0_core_handle_int,
> + .start = dw_hdma_v0_core_start,
> + .ch_config = dw_hdma_v0_core_ch_config,
> + .debugfs_on = dw_hdma_v0_core_debugfs_on,
> +};
> +
> +void dw_hdma_v0_core_register(struct dw_edma *dw)
> +{
> + dw->core = &dw_hdma_v0_core;
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.h b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> new file mode 100644
> index 000000000000..c373b4f0bd8a
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 core
> + *
> + * Author: Cai Huoqing <cai.huoqing@linux.dev>
> + */
> +
> +#ifndef _DW_HDMA_V0_CORE_H
> +#define _DW_HDMA_V0_CORE_H
> +
> +#include <linux/dma/edma.h>
> +
> +/* HDMA core register */
> +void dw_hdma_v0_core_register(struct dw_edma *dw);
> +
> +#endif /* _DW_HDMA_V0_CORE_H */
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> new file mode 100644
> index 000000000000..61359d50e9f4
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> @@ -0,0 +1,129 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 reg
> + *
> + * Author: Cai Huoqing <cai.huoqing@linux.dev>
> + */
> +
> +#ifndef _DW_HDMA_V0_REGS_H
> +#define _DW_HDMA_V0_REGS_H
> +
> +#include <linux/dmaengine.h>
> +
> +#define HDMA_V0_MAX_NR_CH 8
> +#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
> +#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
> +#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
> +#define HDMA_V0_REMOTEL_STOP_INT_EN BIT(3)
> +#define HDMA_V0_ABORT_INT_MASK BIT(2)
> +#define HDMA_V0_STOP_INT_MASK BIT(0)
> +#define HDMA_V0_LINKLIST_EN BIT(0)
> +#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
> +#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
> +#define HDMA_V0_DOORBELL_START BIT(0)
> +#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
> +
> +struct dw_hdma_v0_ch_regs {
> + u32 ch_en; /* 0x0000 */
> + u32 doorbell; /* 0x0004 */
> + u32 prefetch; /* 0x0008 */
> + u32 handshake; /* 0x000c */
> + union {
> + u64 reg; /* 0x0010..0x0014 */
> + struct {
> + u32 lsb; /* 0x0010 */
> + u32 msb; /* 0x0014 */
> + };
> + } llp;
> + u32 cycle_sync; /* 0x0018 */
> + u32 transfer_size; /* 0x001c */
> + union {
> + u64 reg; /* 0x0020..0x0024 */
> + struct {
> + u32 lsb; /* 0x0020 */
> + u32 msb; /* 0x0024 */
> + };
> + } sar;
> + union {
> + u64 reg; /* 0x0028..0x002c */
> + struct {
> + u32 lsb; /* 0x0028 */
> + u32 msb; /* 0x002c */
> + };
> + } dar;
> +
> + u32 watermark_en; /* 0x0030 */
> + u32 control1; /* 0x0034 */
> + u32 func_num; /* 0x0038 */
> + u32 qos; /* 0x003c */
> + u32 reserved; /* 0x0040..0x007c */
> + u32 ch_stat; /* 0x0080 */
> + u32 int_stat; /* 0x0084 */
> + u32 int_setup; /* 0x0088 */
> + u32 int_clear; /* 0x008c */
> + union {
> + u64 reg; /* 0x0090..0x0094 */
> + struct {
> + u32 lsb; /* 0x0090 */
> + u32 msb; /* 0x0094 */
> + };
> + } msi_stop;
> + union {
> + u64 reg; /* 0x0098..0x009c */
> + struct {
> + u32 lsb; /* 0x0098 */
> + u32 msb; /* 0x009c */
> + };
> + } msi_watermark;
> + union {
> + u64 reg; /* 0x00a0..0x00a4 */
> + struct {
> + u32 lsb; /* 0x00a0 */
> + u32 msb; /* 0x00a4 */
> + };
> + } msi_abort;
> + u32 msi_msgdata; /* 0x00a8 */
> +} __packed;
> +
> +struct dw_hdma_v0_ch {
> + struct dw_hdma_v0_ch_regs wr; /* 0x0000 */
> + struct dw_hdma_v0_ch_regs rd; /* 0x0100 */
> +} __packed;
> +
> +struct dw_hdma_v0_regs {
> + struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH]; /* 0x0000..0x0fa8 */
> +} __packed;
> +
> +struct dw_hdma_v0_lli {
> + u32 control;
> + u32 transfer_size;
> + union {
> + u64 reg;
> + struct {
> + u32 lsb;
> + u32 msb;
> + };
> + } sar;
> + union {
> + u64 reg;
> + struct {
> + u32 lsb;
> + u32 msb;
> + };
> + } dar;
> +} __packed;
> +
> +struct dw_hdma_v0_llp {
> + u32 control;
> + u32 reserved;
> + union {
> + u64 reg;
> + struct {
> + u32 lsb;
> + u32 msb;
> + };
> + } llp;
> +} __packed;
> +
> +#endif /* _DW_HDMA_V0_REGS_H */
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index ed401c965a87..3080747689f6 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -48,7 +48,8 @@ struct dw_edma_plat_ops {
> enum dw_edma_map_format {
> EDMA_MF_EDMA_LEGACY = 0x0,
> EDMA_MF_EDMA_UNROLL = 0x1,
> - EDMA_MF_HDMA_COMPAT = 0x5
> + EDMA_MF_HDMA_COMPAT = 0x5,
> + EDMA_MF_HDMA_NATIVE = 0x7,
> };
>
> /**
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA
2023-03-03 12:46 ` [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
2023-03-03 13:04 ` Cai Huoqing
2023-03-03 18:12 ` Serge Semin
@ 2023-03-04 6:51 ` kernel test robot
2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-03-04 6:51 UTC (permalink / raw)
To: Cai Huoqing, fancer.lancer
Cc: llvm, oe-kbuild-all, Cai huoqing, Gustavo Pimentel, Vinod Koul,
Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, linux-kernel, dmaengine, linux-pci
Hi Cai,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on linus/master next-20230303]
[cannot apply to vkoul-dmaengine/next pci/for-linus v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Cai-Huoqing/dmaengine-dw-edma-Rename-dw_edma_core_ops-structure-to-dw_edma_plat_ops/20230303-204905
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20230303124642.5519-4-cai.huoqing%40linux.dev
patch subject: [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA
config: arm-randconfig-r016-20230303 (https://download.01.org/0day-ci/archive/20230304/202303041444.2M7BzjRU-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/061ed1e9b9bf70de12d7885880ee6302865a72d0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Cai-Huoqing/dmaengine-dw-edma-Rename-dw_edma_core_ops-structure-to-dw_edma_plat_ops/20230303-204905
git checkout 061ed1e9b9bf70de12d7885880ee6302865a72d0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/dma/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303041444.2M7BzjRU-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/dma/dw-edma/dw-hdma-v0-core.c:12:
>> drivers/dma/dw-edma/dw-hdma-v0-regs.h:71:4: warning: field msi_stop within 'struct dw_hdma_v0_ch_regs' is less aligned than 'union (unnamed union at drivers/dma/dw-edma/dw-hdma-v0-regs.h:65:2)' and is usually due to 'struct dw_hdma_v0_ch_regs' being packed, which can lead to unaligned accesses [-Wunaligned-access]
} msi_stop;
^
>> drivers/dma/dw-edma/dw-hdma-v0-regs.h:78:4: warning: field msi_watermark within 'struct dw_hdma_v0_ch_regs' is less aligned than 'union (unnamed union at drivers/dma/dw-edma/dw-hdma-v0-regs.h:72:2)' and is usually due to 'struct dw_hdma_v0_ch_regs' being packed, which can lead to unaligned accesses [-Wunaligned-access]
} msi_watermark;
^
>> drivers/dma/dw-edma/dw-hdma-v0-regs.h:85:4: warning: field msi_abort within 'struct dw_hdma_v0_ch_regs' is less aligned than 'union (unnamed union at drivers/dma/dw-edma/dw-hdma-v0-regs.h:79:2)' and is usually due to 'struct dw_hdma_v0_ch_regs' being packed, which can lead to unaligned accesses [-Wunaligned-access]
} msi_abort;
^
3 warnings generated.
vim +71 drivers/dma/dw-edma/dw-hdma-v0-regs.h
26
27 struct dw_hdma_v0_ch_regs {
28 u32 ch_en; /* 0x0000 */
29 u32 doorbell; /* 0x0004 */
30 u32 prefetch; /* 0x0008 */
31 u32 handshake; /* 0x000c */
32 union {
33 u64 reg; /* 0x0010..0x0014 */
34 struct {
35 u32 lsb; /* 0x0010 */
36 u32 msb; /* 0x0014 */
37 };
38 } llp;
39 u32 cycle_sync; /* 0x0018 */
40 u32 transfer_size; /* 0x001c */
41 union {
42 u64 reg; /* 0x0020..0x0024 */
43 struct {
44 u32 lsb; /* 0x0020 */
45 u32 msb; /* 0x0024 */
46 };
47 } sar;
48 union {
49 u64 reg; /* 0x0028..0x002c */
50 struct {
51 u32 lsb; /* 0x0028 */
52 u32 msb; /* 0x002c */
53 };
54 } dar;
55
56 u32 watermark_en; /* 0x0030 */
57 u32 control1; /* 0x0034 */
58 u32 func_num; /* 0x0038 */
59 u32 qos; /* 0x003c */
60 u32 reserved; /* 0x0040..0x007c */
61 u32 ch_stat; /* 0x0080 */
62 u32 int_stat; /* 0x0084 */
63 u32 int_setup; /* 0x0088 */
64 u32 int_clear; /* 0x008c */
65 union {
66 u64 reg; /* 0x0090..0x0094 */
67 struct {
68 u32 lsb; /* 0x0090 */
69 u32 msb; /* 0x0094 */
70 };
> 71 } msi_stop;
72 union {
73 u64 reg; /* 0x0098..0x009c */
74 struct {
75 u32 lsb; /* 0x0098 */
76 u32 msb; /* 0x009c */
77 };
> 78 } msi_watermark;
79 union {
80 u64 reg; /* 0x00a0..0x00a4 */
81 struct {
82 u32 lsb; /* 0x00a0 */
83 u32 msb; /* 0x00a4 */
84 };
> 85 } msi_abort;
86 u32 msi_msgdata; /* 0x00a8 */
87 } __packed;
88
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 4/4] dmaengine: dw-edma: Add HDMA DebugFS support
2023-03-03 12:46 [PATCH v5 0/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
` (2 preceding siblings ...)
2023-03-03 12:46 ` [PATCH v5 3/4] dmaengine: dw-edma: Add support for native HDMA Cai Huoqing
@ 2023-03-03 12:46 ` Cai Huoqing
2023-03-03 18:27 ` Serge Semin
3 siblings, 1 reply; 16+ messages in thread
From: Cai Huoqing @ 2023-03-03 12:46 UTC (permalink / raw)
To: fancer.lancer
Cc: Cai huoqing, Gustavo Pimentel, Vinod Koul, Jingoo Han,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, linux-kernel, dmaengine, linux-pci
From: Cai huoqing <cai.huoqing@linux.dev>
Add HDMA DebugFS support to show register information
Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
---
v4->v5:
1.Remove the check of *regs_dent *ch_dent.
v4 link:
https://lore.kernel.org/lkml/20230221034656.14476-5-cai.huoqing@linux.dev/
drivers/dma/dw-edma/Makefile | 3 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 2 +
drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 175 +++++++++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 +++
4 files changed, 201 insertions(+), 1 deletion(-)
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
index b1c91ef2c63d..83ab58f87760 100644
--- a/drivers/dma/dw-edma/Makefile
+++ b/drivers/dma/dw-edma/Makefile
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DW_EDMA) += dw-edma.o
-dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
+dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
+ dw-hdma-v0-debugfs.o
dw-edma-objs := dw-edma-core.o \
dw-edma-v0-core.o \
dw-hdma-v0-core.o $(dw-edma-y)
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index e14a3907241d..d7abdf154594 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -10,6 +10,7 @@
#include "dw-edma-core.h"
#include "dw-hdma-v0-core.h"
#include "dw-hdma-v0-regs.h"
+#include "dw-hdma-v0-debugfs.h"
enum dw_hdma_control {
DW_HDMA_V0_CB = BIT(0),
@@ -284,6 +285,7 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
/* HDMA debugfs callbacks */
static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
{
+ dw_hdma_v0_debugfs_on(dw);
}
static const struct dw_edma_core_ops dw_hdma_v0_core = {
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
new file mode 100644
index 000000000000..9516a6c3af73
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 debugfs
+ *
+ * Author: Cai Huoqing <cai.huoqing@linux.dev>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/bitfield.h>
+
+#include "dw-hdma-v0-debugfs.h"
+#include "dw-hdma-v0-regs.h"
+#include "dw-edma-core.h"
+
+#define REGS_ADDR(dw, name) \
+ ({ \
+ struct dw_hdma_v0_regs __iomem *__regs = (dw)->chip->reg_base; \
+ \
+ (void __iomem *)&__regs->name; \
+ })
+
+#define REGS_CH_ADDR(dw, name, _dir, _ch) \
+ ({ \
+ struct dw_hdma_v0_ch_regs __iomem *__ch_regs; \
+ \
+ if (_dir == EDMA_DIR_READ) \
+ __ch_regs = REGS_ADDR(dw, ch[_ch].rd); \
+ else \
+ __ch_regs = REGS_ADDR(dw, ch[_ch].wr); \
+ \
+ (void __iomem *)&__ch_regs->name; \
+ })
+
+#define CTX_REGISTER(dw, name, dir, ch) \
+ { dw, #name, REGS_CH_ADDR(dw, name, dir, ch), dir, ch }
+
+#define REGISTER(dw, name) \
+ { dw, #name, REGS_ADDR(dw, name) }
+
+#define WRITE_STR "write"
+#define READ_STR "read"
+#define CHANNEL_STR "channel"
+#define REGISTERS_STR "registers"
+
+struct dw_hdma_debugfs_entry {
+ struct dw_edma *dw;
+ const char *name;
+ void __iomem *reg;
+ enum dw_edma_dir dir;
+ u16 ch;
+};
+
+static int dw_hdma_debugfs_u32_get(void *data, u64 *val)
+{
+ void __iomem *reg = (void __force __iomem *)data;
+
+ *val = readl(reg);
+
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_hdma_debugfs_u32_get, NULL, "0x%08llx\n");
+
+static void dw_hdma_debugfs_create_x32(struct dw_edma *dw,
+ const struct dw_hdma_debugfs_entry ini[],
+ int nr_entries, struct dentry *dent)
+{
+ struct dw_hdma_debugfs_entry *entries;
+ int i;
+
+ entries = devm_kcalloc(dw->chip->dev, nr_entries, sizeof(*entries),
+ GFP_KERNEL);
+ if (!entries)
+ return;
+
+ for (i = 0; i < nr_entries; i++) {
+ entries[i] = ini[i];
+
+ debugfs_create_file_unsafe(entries[i].name, 0444, dent,
+ &entries[i], &fops_x32);
+ }
+}
+
+static void dw_hdma_debugfs_regs_ch(struct dw_edma *dw, enum dw_edma_dir dir,
+ u16 ch, struct dentry *dent)
+{
+ const struct dw_hdma_debugfs_entry debugfs_regs[] = {
+ CTX_REGISTER(dw, ch_en, dir, ch),
+ CTX_REGISTER(dw, doorbell, dir, ch),
+ CTX_REGISTER(dw, prefetch, dir, ch),
+ CTX_REGISTER(dw, handshake, dir, ch),
+ CTX_REGISTER(dw, llp.lsb, dir, ch),
+ CTX_REGISTER(dw, llp.msb, dir, ch),
+ CTX_REGISTER(dw, cycle_sync, dir, ch),
+ CTX_REGISTER(dw, transfer_size, dir, ch),
+ CTX_REGISTER(dw, sar.lsb, dir, ch),
+ CTX_REGISTER(dw, sar.msb, dir, ch),
+ CTX_REGISTER(dw, dar.lsb, dir, ch),
+ CTX_REGISTER(dw, dar.msb, dir, ch),
+ CTX_REGISTER(dw, watermark_en, dir, ch),
+ CTX_REGISTER(dw, control1, dir, ch),
+ CTX_REGISTER(dw, func_num, dir, ch),
+ CTX_REGISTER(dw, qos, dir, ch),
+ CTX_REGISTER(dw, ch_stat, dir, ch),
+ CTX_REGISTER(dw, int_stat, dir, ch),
+ CTX_REGISTER(dw, int_setup, dir, ch),
+ CTX_REGISTER(dw, int_clear, dir, ch),
+ CTX_REGISTER(dw, msi_stop.lsb, dir, ch),
+ CTX_REGISTER(dw, msi_stop.msb, dir, ch),
+ CTX_REGISTER(dw, msi_watermark.lsb, dir, ch),
+ CTX_REGISTER(dw, msi_watermark.msb, dir, ch),
+ CTX_REGISTER(dw, msi_abort.lsb, dir, ch),
+ CTX_REGISTER(dw, msi_abort.msb, dir, ch),
+ CTX_REGISTER(dw, msi_msgdata, dir, ch),
+ };
+ int nr_entries = ARRAY_SIZE(debugfs_regs);
+
+ dw_hdma_debugfs_create_x32(dw, debugfs_regs, nr_entries, dent);
+}
+
+static void dw_hdma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent)
+{
+ struct dentry *regs_dent, *ch_dent;
+ char name[16];
+ int i;
+
+ regs_dent = debugfs_create_dir(WRITE_STR, dent);
+
+ for (i = 0; i < dw->wr_ch_cnt; i++) {
+ snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
+
+ ch_dent = debugfs_create_dir(name, regs_dent);
+
+ dw_hdma_debugfs_regs_ch(dw, EDMA_DIR_WRITE, i, ch_dent);
+ }
+}
+
+static void dw_hdma_debugfs_regs_rd(struct dw_edma *dw, struct dentry *dent)
+{
+ struct dentry *regs_dent, *ch_dent;
+ char name[16];
+ int i;
+
+ regs_dent = debugfs_create_dir(READ_STR, dent);
+
+ for (i = 0; i < dw->rd_ch_cnt; i++) {
+ snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
+
+ ch_dent = debugfs_create_dir(name, regs_dent);
+
+ dw_hdma_debugfs_regs_ch(dw, EDMA_DIR_READ, i, ch_dent);
+ }
+}
+
+static void dw_hdma_debugfs_regs(struct dw_edma *dw)
+{
+ struct dentry *regs_dent;
+
+ regs_dent = debugfs_create_dir(REGISTERS_STR, dw->dma.dbg_dev_root);
+
+ dw_hdma_debugfs_regs_wr(dw, regs_dent);
+ dw_hdma_debugfs_regs_rd(dw, regs_dent);
+}
+
+void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
+{
+ if (!debugfs_initialized())
+ return;
+
+ debugfs_create_u32("mf", 0444, dw->dma.dbg_dev_root, &dw->chip->mf);
+ debugfs_create_u16("wr_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->wr_ch_cnt);
+ debugfs_create_u16("rd_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->rd_ch_cnt);
+
+ dw_hdma_debugfs_regs(dw);
+}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
new file mode 100644
index 000000000000..e6842c83777d
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Cai Huoqing
+ * Synopsys DesignWare HDMA v0 debugfs
+ *
+ * Author: Cai Huoqing <cai.huoqing@linux.dev>
+ */
+
+#ifndef _DW_HDMA_V0_DEBUG_FS_H
+#define _DW_HDMA_V0_DEBUG_FS_H
+
+#include <linux/dma/edma.h>
+
+#ifdef CONFIG_DEBUG_FS
+void dw_hdma_v0_debugfs_on(struct dw_edma *dw);
+#else
+static inline void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+
+#endif /* _DW_HDMA_V0_DEBUG_FS_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v5 4/4] dmaengine: dw-edma: Add HDMA DebugFS support
2023-03-03 12:46 ` [PATCH v5 4/4] dmaengine: dw-edma: Add HDMA DebugFS support Cai Huoqing
@ 2023-03-03 18:27 ` Serge Semin
0 siblings, 0 replies; 16+ messages in thread
From: Serge Semin @ 2023-03-03 18:27 UTC (permalink / raw)
To: Cai Huoqing
Cc: Gustavo Pimentel, Vinod Koul, Jingoo Han, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
linux-kernel, dmaengine, linux-pci
On Fri, Mar 03, 2023 at 08:46:34PM +0800, Cai Huoqing wrote:
> From: Cai huoqing <cai.huoqing@linux.dev>
>
> Add HDMA DebugFS support to show register information
>
> Signed-off-by: Cai huoqing <cai.huoqing@linux.dev>
> ---
> v4->v5:
> 1.Remove the check of *regs_dent *ch_dent.
>
> v4 link:
> https://lore.kernel.org/lkml/20230221034656.14476-5-cai.huoqing@linux.dev/
>
> drivers/dma/dw-edma/Makefile | 3 +-
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 2 +
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 175 +++++++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-debugfs.h | 22 +++
> 4 files changed, 201 insertions(+), 1 deletion(-)
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> create mode 100644 drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
>
> diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
> index b1c91ef2c63d..83ab58f87760 100644
> --- a/drivers/dma/dw-edma/Makefile
> +++ b/drivers/dma/dw-edma/Makefile
> @@ -1,7 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
>
> obj-$(CONFIG_DW_EDMA) += dw-edma.o
> -dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o
> +dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
> + dw-hdma-v0-debugfs.o
> dw-edma-objs := dw-edma-core.o \
> dw-edma-v0-core.o \
> dw-hdma-v0-core.o $(dw-edma-y)
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index e14a3907241d..d7abdf154594 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -10,6 +10,7 @@
> #include "dw-edma-core.h"
> #include "dw-hdma-v0-core.h"
> #include "dw-hdma-v0-regs.h"
> +#include "dw-hdma-v0-debugfs.h"
>
> enum dw_hdma_control {
> DW_HDMA_V0_CB = BIT(0),
> @@ -284,6 +285,7 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> /* HDMA debugfs callbacks */
> static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> {
> + dw_hdma_v0_debugfs_on(dw);
> }
>
> static const struct dw_edma_core_ops dw_hdma_v0_core = {
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> new file mode 100644
> index 000000000000..9516a6c3af73
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 debugfs
> + *
> + * Author: Cai Huoqing <cai.huoqing@linux.dev>
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/bitfield.h>
> +
> +#include "dw-hdma-v0-debugfs.h"
> +#include "dw-hdma-v0-regs.h"
> +#include "dw-edma-core.h"
> +
> +#define REGS_ADDR(dw, name) \
> + ({ \
> + struct dw_hdma_v0_regs __iomem *__regs = (dw)->chip->reg_base; \
> + \
> + (void __iomem *)&__regs->name; \
> + })
> +
> +#define REGS_CH_ADDR(dw, name, _dir, _ch) \
> + ({ \
> + struct dw_hdma_v0_ch_regs __iomem *__ch_regs; \
> + \
> + if (_dir == EDMA_DIR_READ) \
> + __ch_regs = REGS_ADDR(dw, ch[_ch].rd); \
> + else \
> + __ch_regs = REGS_ADDR(dw, ch[_ch].wr); \
> + \
> + (void __iomem *)&__ch_regs->name; \
> + })
> +
> +#define CTX_REGISTER(dw, name, dir, ch) \
> + { dw, #name, REGS_CH_ADDR(dw, name, dir, ch), dir, ch }
> +
> +#define REGISTER(dw, name) \
> + { dw, #name, REGS_ADDR(dw, name) }
> +
> +#define WRITE_STR "write"
> +#define READ_STR "read"
> +#define CHANNEL_STR "channel"
> +#define REGISTERS_STR "registers"
> +
> +struct dw_hdma_debugfs_entry {
> + struct dw_edma *dw;
> + const char *name;
> + void __iomem *reg;
> + enum dw_edma_dir dir;
> + u16 ch;
> +};
> +
> +static int dw_hdma_debugfs_u32_get(void *data, u64 *val)
> +{
> + void __iomem *reg = (void __force __iomem *)data;
^
!!! ------------------+ Brr, did you test it out?
v
> +
> + *val = readl(reg);
I am sure you'll get a mess returned because the
debugfs_create_file_unsafe() method is called with the pointer to the
dw_hdma_debugfs_entry instance passed.
-Serge(y)
> +
> + return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_x32, dw_hdma_debugfs_u32_get, NULL, "0x%08llx\n");
> +
> +static void dw_hdma_debugfs_create_x32(struct dw_edma *dw,
> + const struct dw_hdma_debugfs_entry ini[],
> + int nr_entries, struct dentry *dent)
> +{
> + struct dw_hdma_debugfs_entry *entries;
> + int i;
> +
> + entries = devm_kcalloc(dw->chip->dev, nr_entries, sizeof(*entries),
> + GFP_KERNEL);
> + if (!entries)
> + return;
> +
> + for (i = 0; i < nr_entries; i++) {
> + entries[i] = ini[i];
> +
> + debugfs_create_file_unsafe(entries[i].name, 0444, dent,
> + &entries[i], &fops_x32);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs_ch(struct dw_edma *dw, enum dw_edma_dir dir,
> + u16 ch, struct dentry *dent)
> +{
> + const struct dw_hdma_debugfs_entry debugfs_regs[] = {
> + CTX_REGISTER(dw, ch_en, dir, ch),
> + CTX_REGISTER(dw, doorbell, dir, ch),
> + CTX_REGISTER(dw, prefetch, dir, ch),
> + CTX_REGISTER(dw, handshake, dir, ch),
> + CTX_REGISTER(dw, llp.lsb, dir, ch),
> + CTX_REGISTER(dw, llp.msb, dir, ch),
> + CTX_REGISTER(dw, cycle_sync, dir, ch),
> + CTX_REGISTER(dw, transfer_size, dir, ch),
> + CTX_REGISTER(dw, sar.lsb, dir, ch),
> + CTX_REGISTER(dw, sar.msb, dir, ch),
> + CTX_REGISTER(dw, dar.lsb, dir, ch),
> + CTX_REGISTER(dw, dar.msb, dir, ch),
> + CTX_REGISTER(dw, watermark_en, dir, ch),
> + CTX_REGISTER(dw, control1, dir, ch),
> + CTX_REGISTER(dw, func_num, dir, ch),
> + CTX_REGISTER(dw, qos, dir, ch),
> + CTX_REGISTER(dw, ch_stat, dir, ch),
> + CTX_REGISTER(dw, int_stat, dir, ch),
> + CTX_REGISTER(dw, int_setup, dir, ch),
> + CTX_REGISTER(dw, int_clear, dir, ch),
> + CTX_REGISTER(dw, msi_stop.lsb, dir, ch),
> + CTX_REGISTER(dw, msi_stop.msb, dir, ch),
> + CTX_REGISTER(dw, msi_watermark.lsb, dir, ch),
> + CTX_REGISTER(dw, msi_watermark.msb, dir, ch),
> + CTX_REGISTER(dw, msi_abort.lsb, dir, ch),
> + CTX_REGISTER(dw, msi_abort.msb, dir, ch),
> + CTX_REGISTER(dw, msi_msgdata, dir, ch),
> + };
> + int nr_entries = ARRAY_SIZE(debugfs_regs);
> +
> + dw_hdma_debugfs_create_x32(dw, debugfs_regs, nr_entries, dent);
> +}
> +
> +static void dw_hdma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent)
> +{
> + struct dentry *regs_dent, *ch_dent;
> + char name[16];
> + int i;
> +
> + regs_dent = debugfs_create_dir(WRITE_STR, dent);
> +
> + for (i = 0; i < dw->wr_ch_cnt; i++) {
> + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
> +
> + ch_dent = debugfs_create_dir(name, regs_dent);
> +
> + dw_hdma_debugfs_regs_ch(dw, EDMA_DIR_WRITE, i, ch_dent);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs_rd(struct dw_edma *dw, struct dentry *dent)
> +{
> + struct dentry *regs_dent, *ch_dent;
> + char name[16];
> + int i;
> +
> + regs_dent = debugfs_create_dir(READ_STR, dent);
> +
> + for (i = 0; i < dw->rd_ch_cnt; i++) {
> + snprintf(name, sizeof(name), "%s:%d", CHANNEL_STR, i);
> +
> + ch_dent = debugfs_create_dir(name, regs_dent);
> +
> + dw_hdma_debugfs_regs_ch(dw, EDMA_DIR_READ, i, ch_dent);
> + }
> +}
> +
> +static void dw_hdma_debugfs_regs(struct dw_edma *dw)
> +{
> + struct dentry *regs_dent;
> +
> + regs_dent = debugfs_create_dir(REGISTERS_STR, dw->dma.dbg_dev_root);
> +
> + dw_hdma_debugfs_regs_wr(dw, regs_dent);
> + dw_hdma_debugfs_regs_rd(dw, regs_dent);
> +}
> +
> +void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
> +{
> + if (!debugfs_initialized())
> + return;
> +
> + debugfs_create_u32("mf", 0444, dw->dma.dbg_dev_root, &dw->chip->mf);
> + debugfs_create_u16("wr_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->wr_ch_cnt);
> + debugfs_create_u16("rd_ch_cnt", 0444, dw->dma.dbg_dev_root, &dw->rd_ch_cnt);
> +
> + dw_hdma_debugfs_regs(dw);
> +}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> new file mode 100644
> index 000000000000..e6842c83777d
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023 Cai Huoqing
> + * Synopsys DesignWare HDMA v0 debugfs
> + *
> + * Author: Cai Huoqing <cai.huoqing@linux.dev>
> + */
> +
> +#ifndef _DW_HDMA_V0_DEBUG_FS_H
> +#define _DW_HDMA_V0_DEBUG_FS_H
> +
> +#include <linux/dma/edma.h>
> +
> +#ifdef CONFIG_DEBUG_FS
> +void dw_hdma_v0_debugfs_on(struct dw_edma *dw);
> +#else
> +static inline void dw_hdma_v0_debugfs_on(struct dw_edma *dw)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
> +#endif /* _DW_HDMA_V0_DEBUG_FS_H */
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread