From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S940306AbdEXNpx (ORCPT <rfc822;w@1wt.eu>);
        Wed, 24 May 2017 09:45:53 -0400
Received: from mga11.intel.com ([192.55.52.93]:3969 "EHLO mga11.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S940211AbdEXNps (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 24 May 2017 09:45:48 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.38,386,1491289200"; 
   d="scan'208";a="1134271430"
Subject: Re: [PATCHv2 3/3] fpga_mgr: Add streaming support through the new
 driver_data API
To: Alan Tull <atull@kernel.org>
References: <1495262948-1106-1-git-send-email-yi1.li@linux.intel.com>
 <1495262948-1106-4-git-send-email-yi1.li@linux.intel.com>
 <CANk1AXRQU2CDDC4tR5Vn=Xw1sFF5BC9hY5ZRee84rrXhALh5pA@mail.gmail.com>
 <dc5207ce-c1a8-211b-9cfe-bfcff9cf775d@linux.intel.com>
 <CANk1AXQWgOMQdto85_oQfpumTnDXcWSHYzd81k8EpoSN=DSS-A@mail.gmail.com>
 <CANk1AXQ_nrXUqOdqUFkRYRz_arEg_7YAhZrp2z3-5SbmWOSmWQ@mail.gmail.com>
Cc: mcgrof@kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        wagi@monom.org, David Woodhouse <dwmw2@infradead.org>,
        rafal@milecki.pl, arend.vanspriel@broadcom.com, rjw@rjwysocki.net,
        Moritz Fischer <moritz.fischer@ettus.com>, pmladek@suse.com,
        johannes.berg@intel.com, emmanuel.grumbach@intel.com,
        Luca Coelho <luciano.coelho@intel.com>, kvalo@codeaurora.org,
        luto@kernel.org, Takahiro Akashi <takahiro.akashi@linaro.org>,
        dhowells@redhat.com, pjones@redhat.com,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-fpga@vger.kernel.org
From: "Li, Yi" <yi1.li@linux.intel.com>
Message-ID: <32f80c2a-e7e3-d822-9582-c8942e2a9d83@linux.intel.com>
Date: Wed, 24 May 2017 08:45:42 -0500
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <CANk1AXQ_nrXUqOdqUFkRYRz_arEg_7YAhZrp2z3-5SbmWOSmWQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org



On 5/23/2017 10:25 AM, Alan Tull wrote:
> On Tue, May 23, 2017 at 10:21 AM, Alan Tull <atull@kernel.org> wrote:
>
>>>>> +
>>>>> +       mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ;
>>>>> +       while (length > 0) {
>>>> This could be "do { ... } while ((params.fw_size >= FIRMWARE_CHUNK_SZ)
>>>> && (length > 0));" since that's what it's really doing.
>>> Yes, this is better, thanks.
>>>
>>>>> +               ret = driver_data_request_sync(image_name, &req_params,
>>>>> dev);
>>>>> +               if (ret) {
>>>>> +                       dev_err(dev, "Error reading firmware %d\n", ret);
>>>>> +                       mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ_ERR;
>> Also need:  kfree(buf);
>>
>>>>> +                       return ret;
> Or this could be a break instead of a return.

thanks Alan, will fix it.

>
>>>>> +               }
>>>>> +
>>>>> +               length -= params.fw_size;
>>>>> +               params.offset += params.fw_size;
>>>>> +               if (params.fw_size < SZ_4K)
>>>>> +                       break;
>>>>> +       }
>>>>> +
>>>>> +       kfree(buf);
>>>> Please skip a line before return.
>>>>
>>>>> +       return ret;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(fpga_mgr_firmware_stream);