From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C420C43603 for ; Wed, 11 Dec 2019 10:52:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0ACE921556 for ; Wed, 11 Dec 2019 10:52:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1576061561; bh=KnWDWMENquDzWNp+R8f9eK+p6TF4rRyM36s80K7DgtE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=hjXE6yLwj6fUUG+pqWHeJ+OZMpp2MfMzRkKuD+8A4pl1o60fBNTWaGQAtxk2HG75b VYV5xrFWkW8bY6tjVP9Lm9u4E6v1+yWUv9fj6dB0kxdeDhCtujggNB/KQPOIWFV1of /54dYpmE8ezJf36uRxgTNLhI3xJGSkmULVj86BBc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727469AbfLKKwk (ORCPT ); Wed, 11 Dec 2019 05:52:40 -0500 Received: from mail.kernel.org ([198.145.29.99]:56050 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726119AbfLKKwk (ORCPT ); Wed, 11 Dec 2019 05:52:40 -0500 Received: from localhost (unknown [171.76.100.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BE0A62073B; Wed, 11 Dec 2019 10:52:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1576061559; bh=KnWDWMENquDzWNp+R8f9eK+p6TF4rRyM36s80K7DgtE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jiSoLd1mYP1cuSrNC3eC1Qb5/deelIw4rYPa2BO+Y6krCXXWkqQTjkWbUTQmcX14o i43+WgvslGbPVnYLAFzspf2OysuATbK73p4U/AZYXRLI2ae/u09OGuT2H+1kHovTeB Pm6qfOUvMg7wBENceN3cSB1+s6y6TX75OUK0euaQ= Date: Wed, 11 Dec 2019 16:22:34 +0530 From: Vinod Koul To: Zhou Wang Cc: Dan Williams , dmaengine@vger.kernel.org, linuxarm@huawei.com, Zhenfa Qiu Subject: Re: [PATCH v2] dmaengine: hisilicon: Add Kunpeng DMA engine support Message-ID: <20191211105234.GG2536@vkoul-mobl> References: <1575943997-164744-1-git-send-email-wangzhou1@hisilicon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1575943997-164744-1-git-send-email-wangzhou1@hisilicon.com> Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org On 10-12-19, 10:13, Zhou Wang wrote: > This patch adds a driver for HiSilicon Kunpeng DMA engine. Can you please describe this controller here, how many channels, controller capabilities etc > +static void hisi_dma_enable_dma(struct hisi_dma_dev *hdma_dev, u32 index, > + bool enable) Coding Style expects the second line to be aligned to preceding line brace opne, --strict option with checkpatch should warn you! > +{ > + void __iomem *addr = hdma_dev->base + HISI_DMA_CTRL0(index); > + u32 tmp; > + > + tmp = readl_relaxed(addr); > + tmp = enable ? tmp | HISI_DMA_CTRL0_QUEUE_EN : > + tmp & ~HISI_DMA_CTRL0_QUEUE_EN; > + writel_relaxed(tmp, addr); > +} why not create a modifyl() macro and then use that here and other places rather than doiun read, modify, write sequence > +static void hisi_dma_reset_hw_chan(struct hisi_dma_chan *chan) > +{ > + struct hisi_dma_dev *hdma_dev = chan->hdma_dev; > + u32 index = chan->qp_num, tmp; > + int ret; > + > + hisi_dma_pause_dma(hdma_dev, index, true); > + hisi_dma_enable_dma(hdma_dev, index, false); > + hisi_dma_mask_irq(hdma_dev, index); > + > + ret = readl_relaxed_poll_timeout(hdma_dev->base + > + HISI_DMA_Q_FSM_STS(index), tmp, > + FIELD_GET(HISI_DMA_FSM_STS_MASK, tmp) != RUN, 10, 1000); > + if (ret) > + dev_err(&hdma_dev->pdev->dev, "disable channel timeout!\n"); you want to continue on this timeout? > +static int hisi_dma_terminate_all(struct dma_chan *c) > +{ > + struct hisi_dma_chan *chan = to_hisi_dma_chan(c); > + unsigned long flags; > + LIST_HEAD(head); > + > + spin_lock_irqsave(&chan->vc.lock, flags); > + > + hisi_dma_pause_dma(chan->hdma_dev, chan->qp_num, true); > + if (chan->desc) { > + vchan_terminate_vdesc(&chan->desc->vd); > + chan->desc = NULL; > + } > + > + vchan_get_all_descriptors(&chan->vc, &head); > + > + spin_unlock_irqrestore(&chan->vc.lock, flags); > + > + vchan_dma_desc_free_list(&chan->vc, &head); > + hisi_dma_pause_dma(chan->hdma_dev, chan->qp_num, false); pause on terminate? Not DISABLE? > +static struct pci_driver hisi_dma_pci_driver = { > + .name = "hisi_dma", > + .id_table = hisi_dma_pci_tbl, > + .probe = hisi_dma_probe, no .remove and kconfig has a tristate option! > +MODULE_AUTHOR("Zhou Wang "); > +MODULE_AUTHOR("Zhenfa Qiu "); > +MODULE_DESCRIPTION("HiSilicon Kunpeng DMA controller driver"); > +MODULE_LICENSE("GPL"); GPL v2.. -- ~Vinod