Re: [PATCH V4] misc: pci_endpoint_test: simplify endpoint test read and write operations

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jan 20, 2022 at 08:47:50PM +0800, Li Chen wrote:
> From: Li Chen <lchen@ambarella.com>
> 
> Introduce pci_endpoint_epf_transfer_data to simplify
> read and write operations.
> 
> Signed-off-by: Li Chen <lchen@ambarella.com>
> ---
> Changes in V2:
> fix WARNING: line length of 108 exceeds 100 columns
> #128: FILE: drivers/misc/pci_endpoint_test.c:243:
> Changes in V3:
> This patch context doesn't change but resend with my Zoho mail account in that previous
> company mail will contain un-removeable proprietary messages.
> Changes in V4:
> Add "From:" to the first line of the message body.
> 
> drivers/misc/pci_endpoint_test.c | 289 ++++++++++++-------------------
> 1 file changed, 109 insertions(+), 180 deletions(-)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 2ed7e3aaff3a8..b6b0b19b251b3 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -103,6 +103,11 @@ enum pci_barno {
>     BAR_5,
> };
> 
> +enum operation {
> +    EPF_READ,
> +    EPF_WRITE,
> +};
> +
> struct pci_endpoint_test {
>     struct pci_dev    *pdev;
>     void __iomem    *base;
> @@ -142,6 +147,108 @@ static inline u32 pci_endpoint_test_bar_readl(struct pci_endpoint_test *test,
> {
>     return readl(test->bar[bar] + offset);
> }
> +static bool pci_endpoint_test_transfer_data(struct pci_endpoint_test *test,
> +                unsigned long arg, const enum operation operation)
> +{
> +    struct pci_endpoint_test_xfer_param param;
> +    bool ret = false;
> +    u32 flags = 0;
> +    bool use_dma;
> +    void *addr;
> +    dma_addr_t phys_addr;
> +    struct pci_dev *pdev = test->pdev;
> +    struct device *dev = &pdev->dev;
> +    void *orig_addr;
> +    dma_addr_t orig_phys_addr;
> +    size_t offset;
> +    size_t alignment = test->alignment;
> +    int irq_type = test->irq_type;
> +    size_t size;
> +    int err;
> +
> +    err = copy_from_user(&param, (void __user *)arg, sizeof(param));
> +    if (err != 0) {
> +        dev_err(dev, "Failed to get transfer param\n");
> +        return false;
> +    }
> +
> +    size = param.size;
> +    if (size > SIZE_MAX - alignment)
> +        goto err;
> +
> +    use_dma = !!(param.flags & PCITEST_FLAGS_USE_DMA);
> +    if (use_dma)
> +        flags |= FLAG_USE_DMA;
> +
> +    if (irq_type < IRQ_TYPE_LEGACY || irq_type > IRQ_TYPE_MSIX) {
> +        dev_err(dev, "Invalid IRQ type option\n");
> +        goto err;
> +    }
> +
> +    orig_addr = kzalloc(size + alignment, GFP_KERNEL);
> +    if (!orig_addr)
> +        goto err;
> +
> +    get_random_bytes(orig_addr, size + alignment);
> +
> +    orig_phys_addr = dma_map_single(dev, orig_addr, size + alignment,
> +                    operation == EPF_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> +    if (dma_mapping_error(dev, orig_phys_addr)) {
> +        dev_err(dev, "failed to map source buffer address\n");
> +        goto err_phys_addr;
> +    }
> +
> +    if (alignment && !IS_ALIGNED(orig_phys_addr, alignment)) {
> +        phys_addr = PTR_ALIGN(orig_phys_addr, alignment);
> +        offset = phys_addr - orig_phys_addr;
> +        addr = orig_addr + offset;
> +    } else {
> +        phys_addr = orig_phys_addr;
> +        addr = orig_addr;
> +    }
> +
> +    if (operation == EPF_WRITE) {
> +
> +        pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_CHECKSUM,
> +                 crc32_le(~0, addr, size));
> +
> +        pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_LOWER_SRC_ADDR,
> +                                lower_32_bits(phys_addr));
> +        pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_UPPER_SRC_ADDR,
> +                                upper_32_bits(phys_addr));
> +    } else {
> +        pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_LOWER_DST_ADDR,
> +                                lower_32_bits(phys_addr));
> +        pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_UPPER_DST_ADDR,
> +                                upper_32_bits(phys_addr));
> +    }
> +
> +    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_SIZE, size);
> +    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_FLAGS, flags);
> +    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> +    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> +
> +    // if we ask rc to write to ep, then ep should do read operation, and vice versa.
> +    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> +                 operation == EPF_WRITE ? COMMAND_READ : COMMAND_WRITE);
> +
> +    wait_for_completion(&test->irq_raised);
> +
> +    dma_unmap_single(dev, orig_phys_addr, size + alignment,
> +                     operation == EPF_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> +
> +    if (operation == WRITE)
> +        ret = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS) & STATUS_READ_SUCCESS;
> +    else
> +        ret = crc32_le(~0, addr, size) ==
> +            pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CHECKSUM);
> +
> +err_phys_addr:
> +    kfree(orig_addr);
> +
> +err:
> +    return ret;
> +}
> 
> static inline void pci_endpoint_test_bar_writel(struct pci_endpoint_test *test,
>                         int bar, u32 offset, u32 value)
> @@ -473,191 +580,13 @@ static bool pci_endpoint_test_copy(struct pci_endpoint_test *test,
> static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
>                  unsigned long arg)
> {
> -    struct pci_endpoint_test_xfer_param param;
> -    bool ret = false;
> -    u32 flags = 0;
> -    bool use_dma;
> -    u32 reg;
> -    void *addr;
> -    dma_addr_t phys_addr;
> -    struct pci_dev *pdev = test->pdev;
> -    struct device *dev = &pdev->dev;
> -    void *orig_addr;
> -    dma_addr_t orig_phys_addr;
> -    size_t offset;
> -    size_t alignment = test->alignment;
> -    int irq_type = test->irq_type;
> -    size_t size;
> -    u32 crc32;
> -    int err;
> -
> -    err = copy_from_user(&param, (void __user *)arg, sizeof(param));
> -    if (err != 0) {
> -        dev_err(dev, "Failed to get transfer param\n");
> -        return false;
> -    }
> -
> -    size = param.size;
> -    if (size > SIZE_MAX - alignment)
> -        goto err;
> -
> -    use_dma = !!(param.flags & PCITEST_FLAGS_USE_DMA);
> -    if (use_dma)
> -        flags |= FLAG_USE_DMA;
> -
> -    if (irq_type < IRQ_TYPE_LEGACY || irq_type > IRQ_TYPE_MSIX) {
> -        dev_err(dev, "Invalid IRQ type option\n");
> -        goto err;
> -    }
> -
> -    orig_addr = kzalloc(size + alignment, GFP_KERNEL);
> -    if (!orig_addr) {
> -        dev_err(dev, "Failed to allocate address\n");
> -        ret = false;
> -        goto err;
> -    }
> -
> -    get_random_bytes(orig_addr, size + alignment);
> -
> -    orig_phys_addr = dma_map_single(dev, orig_addr, size + alignment,
> -                    DMA_TO_DEVICE);
> -    if (dma_mapping_error(dev, orig_phys_addr)) {
> -        dev_err(dev, "failed to map source buffer address\n");
> -        ret = false;
> -        goto err_phys_addr;
> -    }
> -
> -    if (alignment && !IS_ALIGNED(orig_phys_addr, alignment)) {
> -        phys_addr = PTR_ALIGN(orig_phys_addr, alignment);
> -        offset = phys_addr - orig_phys_addr;
> -        addr = orig_addr + offset;
> -    } else {
> -        phys_addr = orig_phys_addr;
> -        addr = orig_addr;
> -    }
> -
> -    crc32 = crc32_le(~0, addr, size);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_CHECKSUM,
> -                 crc32);
> -
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_LOWER_SRC_ADDR,
> -                 lower_32_bits(phys_addr));
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_UPPER_SRC_ADDR,
> -                 upper_32_bits(phys_addr));
> -
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_SIZE, size);
> -
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_FLAGS, flags);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -                 COMMAND_READ);
> -
> -    wait_for_completion(&test->irq_raised);
> -
> -    reg = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
> -    if (reg & STATUS_READ_SUCCESS)
> -        ret = true;
> -
> -    dma_unmap_single(dev, orig_phys_addr, size + alignment,
> -             DMA_TO_DEVICE);
> -
> -err_phys_addr:
> -    kfree(orig_addr);
> -
> -err:
> -    return ret;
> +    return pci_endpoint_test_transfer_data(test, arg, EPF_WRITE);
> }
> 
> static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
>                  unsigned long arg)
> {
> -    struct pci_endpoint_test_xfer_param param;
> -    bool ret = false;
> -    u32 flags = 0;
> -    bool use_dma;
> -    size_t size;
> -    void *addr;
> -    dma_addr_t phys_addr;
> -    struct pci_dev *pdev = test->pdev;
> -    struct device *dev = &pdev->dev;
> -    void *orig_addr;
> -    dma_addr_t orig_phys_addr;
> -    size_t offset;
> -    size_t alignment = test->alignment;
> -    int irq_type = test->irq_type;
> -    u32 crc32;
> -    int err;
> -
> -    err = copy_from_user(&param, (void __user *)arg, sizeof(param));
> -    if (err) {
> -        dev_err(dev, "Failed to get transfer param\n");
> -        return false;
> -    }
> -
> -    size = param.size;
> -    if (size > SIZE_MAX - alignment)
> -        goto err;
> -
> -    use_dma = !!(param.flags & PCITEST_FLAGS_USE_DMA);
> -    if (use_dma)
> -        flags |= FLAG_USE_DMA;
> -
> -    if (irq_type < IRQ_TYPE_LEGACY || irq_type > IRQ_TYPE_MSIX) {
> -        dev_err(dev, "Invalid IRQ type option\n");
> -        goto err;
> -    }
> -
> -    orig_addr = kzalloc(size + alignment, GFP_KERNEL);
> -    if (!orig_addr) {
> -        dev_err(dev, "Failed to allocate destination address\n");
> -        ret = false;
> -        goto err;
> -    }
> -
> -    orig_phys_addr = dma_map_single(dev, orig_addr, size + alignment,
> -                    DMA_FROM_DEVICE);
> -    if (dma_mapping_error(dev, orig_phys_addr)) {
> -        dev_err(dev, "failed to map source buffer address\n");
> -        ret = false;
> -        goto err_phys_addr;
> -    }
> -
> -    if (alignment && !IS_ALIGNED(orig_phys_addr, alignment)) {
> -        phys_addr = PTR_ALIGN(orig_phys_addr, alignment);
> -        offset = phys_addr - orig_phys_addr;
> -        addr = orig_addr + offset;
> -    } else {
> -        phys_addr = orig_phys_addr;
> -        addr = orig_addr;
> -    }
> -
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_LOWER_DST_ADDR,
> -                 lower_32_bits(phys_addr));
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_UPPER_DST_ADDR,
> -                 upper_32_bits(phys_addr));
> -
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_SIZE, size);
> -
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_FLAGS, flags);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> -    pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -                 COMMAND_WRITE);
> -
> -    wait_for_completion(&test->irq_raised);
> -
> -    dma_unmap_single(dev, orig_phys_addr, size + alignment,
> -             DMA_FROM_DEVICE);
> -
> -    crc32 = crc32_le(~0, addr, size);
> -    if (crc32 == pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CHECKSUM))
> -        ret = true;
> -
> -err_phys_addr:
> -    kfree(orig_addr);
> -err:
> -    return ret;
> +    return pci_endpoint_test_transfer_data(test, arg, EPF_READ);
> }
> 
> static bool pci_endpoint_test_clear_irq(struct pci_endpoint_test *test)
> --
> 2.34.1

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch is malformed (tabs converted to spaces, linewrapped, etc.)
  and can not be applied.  Please read the file,
  Documentation/email-clients.txt in order to fix this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot