lkml.org 
[lkml]   [2019]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/3] fpga: altera-cvp: Preparation for V2 parts.
From
Date
On 7/14/19 1:46 PM, Moritz Fischer wrote:
> Hi Thor,
>
> On Thu, Jul 11, 2019 at 03:32:49PM -0500, thor.thayer@linux.intel.com wrote:
>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>
>> In preparation for adding newer V2 parts that use a FIFO,
>> reorganize altera_cvp_chk_error() and change the write
>> function to block based.
>> V2 parts have a block size matching the FIFO while older
>> V1 parts write a 32 bit word at a time.
>>
>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
>> ---
>> drivers/fpga/altera-cvp.c | 72 ++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 46 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c
>> index 04f2b2a072a7..59835f6f9b2d 100644
>> --- a/drivers/fpga/altera-cvp.c
>> +++ b/drivers/fpga/altera-cvp.c
>> @@ -140,6 +140,41 @@ static int altera_cvp_wait_status(struct altera_cvp_conf *conf, u32 status_mask,
>> return -ETIMEDOUT;
>> }
>>
>> +static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
>
> Please drop the inline here.

OK. Thanks.

>> +{
>> + struct altera_cvp_conf *conf = mgr->priv;
>> + u32 val;
>> +
>> + /* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
>> + altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
>> + if (val & VSE_CVP_STATUS_CFG_ERR) {
>> + dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
>> + bytes);
>> + return -EPROTO;
>> + }
>> + return 0;
>> +}
>> +
>> +static int altera_cvp_send_block(struct altera_cvp_conf *conf,
>> + const u32 *data, size_t len)
>> +{
>> + int i, remainder;
>> + u32 mask, words = len / sizeof(u32);
> Reverse xmas-tree, please.

OK. I'm new to the reverse xmas-tree but if I read [0] correctly, it
isn't the size of the variable that is important but the length of the
declaration.

>> +
>> + for (i = 0; i < words; i++)
>> + conf->write_data(conf, *data++);
>> +
>> + /* write up to 3 trailing bytes, if any */
>> + remainder = len % sizeof(u32);
>> + if (remainder) {
>> + mask = BIT(remainder * 8) - 1;
>> + if (mask)
>> + conf->write_data(conf, *data & mask);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int altera_cvp_teardown(struct fpga_manager *mgr,
>> struct fpga_image_info *info)
>> {
>> @@ -262,39 +297,29 @@ static int altera_cvp_write_init(struct fpga_manager *mgr,
>> return 0;
>> }
>>
>> -static inline int altera_cvp_chk_error(struct fpga_manager *mgr, size_t bytes)
>> -{
>> - struct altera_cvp_conf *conf = mgr->priv;
>> - u32 val;
>> -
>> - /* STEP 10 (optional) - check CVP_CONFIG_ERROR flag */
>> - altera_read_config_dword(conf, VSE_CVP_STATUS, &val);
>> - if (val & VSE_CVP_STATUS_CFG_ERR) {
>> - dev_err(&mgr->dev, "CVP_CONFIG_ERROR after %zu bytes!\n",
>> - bytes);
>> - return -EPROTO;
>> - }
>> - return 0;
>> -}
>> -
>> static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
>> size_t count)
>> {
>> struct altera_cvp_conf *conf = mgr->priv;
>> const u32 *data;
>> - size_t done, remaining;
>> + size_t done, remaining, len;
>
> Reverse xmas-tree please.

OK.

>> int status = 0;
>> - u32 mask;
>>
>> /* STEP 9 - write 32-bit data from RBF file to CVP data register */
>> data = (u32 *)buf;
>
> Are there endianness concerns with this?

No, this maintains the previous big endian byte handling for
Cyclone5/Arria10. The Stratix10 bitstream is generated in 4KB chunks.

>> remaining = count;
>> done = 0;
>>
>> - while (remaining >= 4) {
>> - conf->write_data(conf, *data++);
>> - done += 4;
>> - remaining -= 4;
>> + while (remaining) {
>> + if (remaining >= sizeof(u32))
>> + len = sizeof(u32);
>> + else
>> + len = remaining;
>> +
>> + altera_cvp_send_block(conf, data, len);
>> + data++;
>> + done += len;
>> + remaining -= len;
>>
>> /*
>> * STEP 10 (optional) and STEP 11
>> @@ -312,11 +337,6 @@ static int altera_cvp_write(struct fpga_manager *mgr, const char *buf,
>> }
>> }
>>
>> - /* write up to 3 trailing bytes, if any */
>> - mask = BIT(remaining * 8) - 1;
>> - if (mask)
>> - conf->write_data(conf, *data & mask);
>> -
>> if (altera_cvp_chkcfg)
>> status = altera_cvp_chk_error(mgr, count);
>>
>> --
>> 2.7.4
>>
>
> Thanks,
> Moritz
>

I wasn't sure if you preferred this as a separate patch or squashed into
patch #3. Having a separate patch makes the migration a little clearer.

Thanks for reviewing!

[0] https://www.spinics.net/lists/netdev/msg402833.html

\
 
 \ /
  Last update: 2019-07-15 19:39    [W:0.069 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site