Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 18 Apr 2020 14:47:25 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Scott Branden <scott.branden@broadcom.com>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        David Brown <david.brown@linaro.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Shuah Khan <shuah@kernel.org>, bjorn.andersson@linaro.org,
        Shuah Khan <skhan@linuxfoundation.org>,
        Arnd Bergmann <arnd@arndb.de>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-fsdevel@vger.kernel.org,
        BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,
        Olof Johansson <olof@lixom.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Colin Ian King <colin.king@canonical.com>,
        Kees Cook <keescook@chromium.org>,
        Takashi Iwai <tiwai@suse.de>, linux-kselftest@vger.kernel.org,
        Andy Gross <agross@kernel.org>,
        Desmond Yan <desmond.yan@broadcom.com>,
        James Hu <james.hu@broadcom.com>
Subject: Re: [PATCH v2 6/7] misc: bcm-vk: add Broadcom VK driver
Message-ID: <20200418114725.GF12862@kadam>
References: <20200220004825.23372-1-scott.branden@broadcom.com>
 <20200220004825.23372-7-scott.branden@broadcom.com>
 <20200220104321.GX7838@kadam>
 <63c9dcda-7a31-78a7-1d11-9d9af38add46@broadcom.com>
 <20200418114516.GE12862@kadam>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200418114516.GE12862@kadam>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Apr 18, 2020 at 02:45:16PM +0300, Dan Carpenter wrote:
> On Fri, Apr 17, 2020 at 02:49:11PM -0700, Scott Branden wrote:
> > > > +static int bcm_vk_dma_alloc(struct device *dev,
> > > > +			    struct bcm_vk_dma *dma,
> > > > +			    int direction,
> > > > +			    struct _vk_data *vkdata)
> > > > +{
> > > > +	dma_addr_t addr, sg_addr;
> > > > +	int err;
> > > > +	int i;
> > > > +	int offset;
> > > > +	uint32_t size;
> > > > +	uint32_t remaining_size;
> > > > +	uint32_t transfer_size;
> > > > +	uint64_t data;
> > > > +	unsigned long first, last;
> > > > +	struct _vk_data *sgdata;
> > > > +
> > > > +	/* Get 64-bit user address */
> > > > +	data = get_unaligned(&(vkdata->address));
> > > Extra parens.
> > removed
> > > 
> > > > +
> > > > +	/* offset into first page */
> > > > +	offset = offset_in_page(data);
> > > > +
> > > > +	/* Calculate number of pages */
> > > > +	first = (data & PAGE_MASK) >> PAGE_SHIFT;
> > > > +	last  = ((data + vkdata->size - 1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > +	dma->nr_pages = last - first + 1;
> > > > +
> > > > +	/* Allocate DMA pages */
> > > > +	dma->pages = kmalloc_array(dma->nr_pages,
> > > > +				   sizeof(struct page *),
> > > > +				   GFP_KERNEL);
> > > > +	if (dma->pages == NULL)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	dev_dbg(dev, "Alloc DMA Pages [0x%llx+0x%x => %d pages]\n",
> > > > +		data, vkdata->size, dma->nr_pages);
> > > > +
> > > > +	dma->direction = direction;
> > > > +
> > > > +	/* Get user pages into memory */
> > > > +	err = get_user_pages_fast(data & PAGE_MASK,
> > > > +				  dma->nr_pages,
> > > > +				  direction == DMA_FROM_DEVICE,
> > > > +				  dma->pages);
> > > > +	if (err != dma->nr_pages) {
> > > > +		dma->nr_pages = (err >= 0) ? err : 0;
> > > > +		dev_err(dev, "get_user_pages_fast, err=%d [%d]\n",
> > > > +			err, dma->nr_pages);
> > > > +		return err < 0 ? err : -EINVAL;
> > > > +	}
> > > > +
> > > > +	/* Max size of sg list is 1 per mapped page + fields at start */
> > > > +	dma->sglen = (dma->nr_pages * sizeof(*sgdata)) +
> > > > +		     (sizeof(uint32_t) * SGLIST_VKDATA_START);
> > > > +
> > > > +	/* Allocate sglist */
> > > > +	dma->sglist = dma_alloc_coherent(dev,
> > > > +					 dma->sglen,
> > > > +					 &dma->handle,
> > > > +					 GFP_KERNEL);
> > > 
> > > 	dma->sglist = dma_alloc_coherent(dev, dma->sglen, &dma->handle,
> > > 					 GFP_KERNEL);
> > done
> > > 
> > > 
> > > 
> > > > +	if (!dma->sglist)
> > > > +		return -ENOMEM;
> > > No cleanup?
> > what needs to be cleaned up?
> 
> dma->pages should be freed probably?  And a put_user_pages_fast()?

Sorry put_user_pages_fast() isn't a function.  My bad.

regards,
dan carpenter