Re: [PATCHv2 2/3] fpga: altera-cvp: Preparation for V2 parts.

[Thor Thayer]

Hi Moritz,

On 7/21/19 7:59 PM, Moritz Fischer wrote:
Thor,
On Tue, Jul 16, 2019 at 05:48:06PM -0500, thor.tha...@linux.intel.com wrote:
From: Thor Thayer <thor.tha...@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.tha...@linux.intel.com>
---
v2 Remove inline function declaration
    Reverse Christmas Tree format for local variables
---
  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 b78c90580071..37419d6b9915 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 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);
Same as in the other email, why can we ignore return values here. I
think the original code probably did that already.

Yes, I actually didn't make any changes to this function. You can see I 
moved it from below since it is used in the following function.

I'm not checking the return code from any of the read/write functions 
since the original driver didn't. Would you prefer I check and issue a 
warning?

Thanks for reviewing!

+       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)
+{
+       u32 mask, words = len / sizeof(u32);
+       int i, remainder;
+
+       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;
+       size_t done, remaining, len;
        const u32 *data;
-       size_t done, remaining;
        int status = 0;
-       u32 mask;
  
  	/* STEP 9 - write 32-bit data from RBF file to CVP data register */
        data = (u32 *)buf;
        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

Cheers,
Moritz