Re: [PATCH v7 05/10] hw/dma/xlnx_csu_dma: Support starting a read transfer through a class method

[Luc Michel]

On 16:11 Fri 21 Jan     , Francisco Iglesias wrote:
> An option on real hardware when embedding a DMA engine into a peripheral
> is to make the peripheral control the engine through a custom DMA control
> (hardware) interface between the two. Software drivers in this scenario
> configure and trigger DMA operations through the controlling peripheral's
> register API (for example, writing a specific bit in a register could
> propagate down to a transfer start signal on the DMA control interface).
> At the same time the status, results and interrupts for the transfer might
> still be intended to be read and caught through the DMA engine's register
> API (and signals).
> 
> This patch adds a class 'read' method for allowing to start read transfers
> from peripherals embedding and controlling the Xilinx CSU DMA engine as in
> above scenario.
> 
> Signed-off-by: Francisco Iglesias <francisco.iglesias@xilinx.com>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
>  include/hw/dma/xlnx_csu_dma.h | 19 +++++++++++++++++--
>  hw/dma/xlnx_csu_dma.c         | 17 +++++++++++++++++
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/dma/xlnx_csu_dma.h b/include/hw/dma/xlnx_csu_dma.h
> index 28806628b1..922ab80eb6 100644
> --- a/include/hw/dma/xlnx_csu_dma.h
> +++ b/include/hw/dma/xlnx_csu_dma.h
> @@ -51,7 +51,22 @@ typedef struct XlnxCSUDMA {
>      RegisterInfo regs_info[XLNX_CSU_DMA_R_MAX];
>  } XlnxCSUDMA;
>  
> -#define XLNX_CSU_DMA(obj) \
> -    OBJECT_CHECK(XlnxCSUDMA, (obj), TYPE_XLNX_CSU_DMA)
> +OBJECT_DECLARE_TYPE(XlnxCSUDMA, XlnxCSUDMAClass, XLNX_CSU_DMA)
> +
> +struct XlnxCSUDMAClass {
> +    SysBusDeviceClass parent_class;
> +
> +    /*
> +     * read: Start a read transfer on a Xilinx CSU DMA engine
> +     *
> +     * @s: the Xilinx CSU DMA engine to start the transfer on
> +     * @addr: the address to read
> +     * @len: the number of bytes to read at 'addr'
> +     *
> +     * @return a MemTxResult indicating whether the operation succeeded 
> ('len'
> +     * bytes were read) or failed.
> +     */
> +    MemTxResult (*read)(XlnxCSUDMA *s, hwaddr addr, uint32_t len);
> +};
>  
>  #endif
> diff --git a/hw/dma/xlnx_csu_dma.c b/hw/dma/xlnx_csu_dma.c
> index 896bb3574d..095f954476 100644
> --- a/hw/dma/xlnx_csu_dma.c
> +++ b/hw/dma/xlnx_csu_dma.c
> @@ -472,6 +472,20 @@ static uint64_t addr_msb_pre_write(RegisterInfo *reg, 
> uint64_t val)
>      return val & R_ADDR_MSB_ADDR_MSB_MASK;
>  }
>  
> +static MemTxResult xlnx_csu_dma_class_read(XlnxCSUDMA *s, hwaddr addr,
> +                                           uint32_t len)
> +{
> +    RegisterInfo *reg = &s->regs_info[R_SIZE];
> +    uint64_t we = MAKE_64BIT_MASK(0, 4 * 8);
> +
> +    s->regs[R_ADDR] = addr;
> +    s->regs[R_ADDR_MSB] = (uint64_t)addr >> 32;
> +
> +    register_write(reg, len, we, object_get_typename(OBJECT(s)), false);
> +
> +    return (s->regs[R_SIZE] == 0) ? MEMTX_OK : MEMTX_ERROR;
> +}
> +
>  static const RegisterAccessInfo *xlnx_csu_dma_regs_info[] = {
>  #define DMACH_REGINFO(NAME, snd)                                             
>  \
>      (const RegisterAccessInfo []) {                                          
>  \
> @@ -696,6 +710,7 @@ static void xlnx_csu_dma_class_init(ObjectClass *klass, 
> void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      StreamSinkClass *ssc = STREAM_SINK_CLASS(klass);
> +    XlnxCSUDMAClass *xcdc = XLNX_CSU_DMA_CLASS(klass);
>  
>      dc->reset = xlnx_csu_dma_reset;
>      dc->realize = xlnx_csu_dma_realize;
> @@ -704,6 +719,8 @@ static void xlnx_csu_dma_class_init(ObjectClass *klass, 
> void *data)
>  
>      ssc->push = xlnx_csu_dma_stream_push;
>      ssc->can_push = xlnx_csu_dma_stream_can_push;
> +
> +    xcdc->read = xlnx_csu_dma_class_read;
>  }
>  
>  static void xlnx_csu_dma_init(Object *obj)
> -- 
> 2.11.0
> 

--