diff mbox series

[PATCHv1,1/4] fpga: fpga-mgr: add FPGA_MGR_BITSTREM_AUTHENTICATION flag

Message ID 1605204403-6663-2-git-send-email-richard.gong@linux.intel.com (mailing list archive)
State Superseded, archived
Headers show
Series Extend FPGA manager and region drivers for | expand

Commit Message

Richard Gong Nov. 12, 2020, 6:06 p.m. UTC 
From: Richard Gong <richard.gong@intel.com>

Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
authentication.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
 include/linux/fpga/fpga-mgr.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Tom Rix Nov. 13, 2020, 8:24 p.m. UTC | #1 
On 11/12/20 10:06 AM, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
>
> Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
> authentication.

Should improve this commit so explain what you mean authentication.

it could mean 'it wrote correctly' or 'it was signed correctly' or something else.

>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
>  include/linux/fpga/fpga-mgr.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 2bc3030..1d65814 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -67,12 +67,15 @@ enum fpga_mgr_states {
>   * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
>   *
>   * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
> + *
> + * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication
>   */
>  #define FPGA_MGR_PARTIAL_RECONFIG	BIT(0)
>  #define FPGA_MGR_EXTERNAL_CONFIG	BIT(1)
>  #define FPGA_MGR_ENCRYPTED_BITSTREAM	BIT(2)
>  #define FPGA_MGR_BITSTREAM_LSB_FIRST	BIT(3)
>  #define FPGA_MGR_COMPRESSED_BITSTREAM	BIT(4)
> +#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)

A whitespace issue, the new BIT(5) should align with the others, so add two spaces to the others.

Tom

>  
>  /**
>   * struct fpga_image_info - information specific to a FPGA image
Richard Gong Nov. 14, 2020, 2:30 p.m. UTC | #2 
Hi Tom,

Thanks for review!

On 11/13/20 2:24 PM, Tom Rix wrote:
> 
> On 11/12/20 10:06 AM, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> Add FPGA_MGR_BITSTREM_AUTHENTICATION flag for FPGA bitstream
>> authentication.
> 
> Should improve this commit so explain what you mean authentication.
> 
> it could mean 'it wrote correctly' or 'it was signed correctly' or something else.
> 

Authentication = make sure a signed bitstream has valid signatures 
before committing it to QSPI memory. I will update the commit messages 
in version 2.

>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>>   include/linux/fpga/fpga-mgr.h | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
>> index 2bc3030..1d65814 100644
>> --- a/include/linux/fpga/fpga-mgr.h
>> +++ b/include/linux/fpga/fpga-mgr.h
>> @@ -67,12 +67,15 @@ enum fpga_mgr_states {
>>    * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
>>    *
>>    * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
>> + *
>> + * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication
>>    */
>>   #define FPGA_MGR_PARTIAL_RECONFIG	BIT(0)
>>   #define FPGA_MGR_EXTERNAL_CONFIG	BIT(1)
>>   #define FPGA_MGR_ENCRYPTED_BITSTREAM	BIT(2)
>>   #define FPGA_MGR_BITSTREAM_LSB_FIRST	BIT(3)
>>   #define FPGA_MGR_COMPRESSED_BITSTREAM	BIT(4)
>> +#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
> 
> A whitespace issue, the new BIT(5) should align with the others, so add two spaces to the others.
> 

There is only one space, also I ran checkpatch with strict option and 
didn't see any whitespace issue.

In the original patch, BIT(0) to BIT(4) align themselves. I am not sure 
why we see differently in email.

  #define FPGA_MGR_PARTIAL_RECONFIG      BIT(0)
  #define FPGA_MGR_EXTERNAL_CONFIG       BIT(1)
  #define FPGA_MGR_ENCRYPTED_BITSTREAM   BIT(2)
  #define FPGA_MGR_BITSTREAM_LSB_FIRST   BIT(3)
  #define FPGA_MGR_COMPRESSED_BITSTREAM  BIT(4)
+#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)

To align BIT(5) with others, I have to use additional tab to BIT(0) to 
BIT(4). But I don't think I should make such change on them, agree?

Regards,
Richard

> Tom
> 
>>   
>>   /**
>>    * struct fpga_image_info - information specific to a FPGA image
>
Tom Rix Nov. 14, 2020, 3:53 p.m. UTC | #3 
On 11/14/20 6:30 AM, Richard Gong wrote:
>
>> A whitespace issue, the new BIT(5) should align with the others, so add two spaces to the others.
>>
>
> There is only one space, also I ran checkpatch with strict option and didn't see any whitespace issue.
>
> In the original patch, BIT(0) to BIT(4) align themselves. I am not sure why we see differently in email.
>
>  #define FPGA_MGR_PARTIAL_RECONFIG      BIT(0)
>  #define FPGA_MGR_EXTERNAL_CONFIG       BIT(1)
>  #define FPGA_MGR_ENCRYPTED_BITSTREAM   BIT(2)
>  #define FPGA_MGR_BITSTREAM_LSB_FIRST   BIT(3)
>  #define FPGA_MGR_COMPRESSED_BITSTREAM  BIT(4)
> +#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
>
> To align BIT(5) with others, I have to use additional tab to BIT(0) to BIT(4). But I don't think I should make such change on them, agree?

The existing table of #defines has aligned values for BIT(0) to BIT(4)

Your addition of BIT(5) value has an inconsistent alignment with the others BIT(0) to BIT(4)

The alignment of all the values should be consistent.

Tom

>
> Regards,
> Richard
>
>> Tom
>>
>>>     /**
>>>    * struct fpga_image_info - information specific to a FPGA image
>>
>
Richard Gong Nov. 16, 2020, 1:39 p.m. UTC | #4 
Hi Tom,

On 11/14/20 9:53 AM, Tom Rix wrote:
> 
> On 11/14/20 6:30 AM, Richard Gong wrote:
>>
>>> A whitespace issue, the new BIT(5) should align with the others, so add two spaces to the others.
>>>
>>
>> There is only one space, also I ran checkpatch with strict option and didn't see any whitespace issue.
>>
>> In the original patch, BIT(0) to BIT(4) align themselves. I am not sure why we see differently in email.
>>
>>   #define FPGA_MGR_PARTIAL_RECONFIG      BIT(0)
>>   #define FPGA_MGR_EXTERNAL_CONFIG       BIT(1)
>>   #define FPGA_MGR_ENCRYPTED_BITSTREAM   BIT(2)
>>   #define FPGA_MGR_BITSTREAM_LSB_FIRST   BIT(3)
>>   #define FPGA_MGR_COMPRESSED_BITSTREAM  BIT(4)
>> +#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
>>
>> To align BIT(5) with others, I have to use additional tab to BIT(0) to BIT(4). But I don't think I should make such change on them, agree?
> 
> The existing table of #defines has aligned values for BIT(0) to BIT(4)
> 
> Your addition of BIT(5) value has an inconsistent alignment with the others BIT(0) to BIT(4)
> 
> The alignment of all the values should be consistent.
> 

OK, I will make them all aligned.

Regards,
Richard

> Tom
> 
>>
>> Regards,
>> Richard
>>
>>> Tom
>>>
>>>>      /**
>>>>     * struct fpga_image_info - information specific to a FPGA image
>>>
>>
>
diff mbox series

Patch

diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index 2bc3030..1d65814 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -67,12 +67,15 @@  enum fpga_mgr_states {
  * %FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
  *
  * %FPGA_MGR_COMPRESSED_BITSTREAM: FPGA bitstream is compressed
+ *
+ * %FPGA_MGR_BITSTREM_AUTHENTICATION: do FPGA bitstream authentication
  */
 #define FPGA_MGR_PARTIAL_RECONFIG	BIT(0)
 #define FPGA_MGR_EXTERNAL_CONFIG	BIT(1)
 #define FPGA_MGR_ENCRYPTED_BITSTREAM	BIT(2)
 #define FPGA_MGR_BITSTREAM_LSB_FIRST	BIT(3)
 #define FPGA_MGR_COMPRESSED_BITSTREAM	BIT(4)
+#define FPGA_MGR_BITSTREM_AUTHENTICATION BIT(5)
 
 /**
  * struct fpga_image_info - information specific to a FPGA image