[PATCH v4 0/5]fpga: fix for coding style and kernel-doc issues

[PATCH v4 0/5]fpga: fix for coding style and kernel-doc issues

[Nava kishore Manne]

This patch series fixes the coding style and kernel-doc issues
exists in the fpga framework, zynq and ZynqMP drivers.

Nava kishore Manne (5):
  fpga: zynq: Fix incorrect variable type
  fpga: fix for coding style issues
  fpga: fpga-mgr: fix kernel-doc warnings
  fpga: Use tab instead of space indentation
  fpga: fpga-region: fix kernel-doc formatting issues

 drivers/fpga/Makefile         |  6 +++---
 drivers/fpga/fpga-mgr.c       |  8 ++++++--
 drivers/fpga/fpga-region.c    |  7 ++++---
 drivers/fpga/of-fpga-region.c | 16 +++++++++-------
 drivers/fpga/zynq-fpga.c      |  8 ++++----
 5 files changed, 26 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH v4 1/5] fpga: zynq: Fix incorrect variable type

[Nava kishore Manne]

zynq_fpga_has_sync () API is expecting "u8 *" but the
formal parameter that was passed is of type "const char *".
fixes this issue by changing the buf type to "const char *"

This patch will also update zynq_fpga_has_sync () API description
to align with API functionality.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v2:
                -None.
Changes for v3:
               - Changed arg buf type to "const char *" as suggested by Tom.
               - update zynq_fpga_has_sync () API description to align with API
                 functionality.
Changes for v4:
               - None.

 drivers/fpga/zynq-fpga.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 426aa34c6a0d..ada07eea64bc 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -235,11 +235,11 @@ static irqreturn_t zynq_fpga_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-/* Sanity check the proposed bitstream. It must start with the sync word in
- * the correct byte order, and be dword aligned. The input is a Xilinx .bin
- * file with every 32 bit quantity swapped.
+/* Sanity check the proposed bitstream. The sync word must be found in the
+ * correct byte order and it should be dword aligned. The input is a
+ * Xilinx.bin file with every 32 bit quantity swapped.
  */
-static bool zynq_fpga_has_sync(const u8 *buf, size_t count)
+static bool zynq_fpga_has_sync(const char *buf, size_t count)
 {
 	for (; count >= 4; buf += 4, count -= 4)
 		if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
-- 
2.25.1

[PATCH v4 2/5] fpga: fix for coding style issues

[Nava kishore Manne]

fixes the below checks reported by checkpatch.pl
Lines should not end with a '('
Alignment should match open parenthesis

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v2:
                -None.
Changes for v3:
               -Fixed similar issue exists in "drivers/fpga/*".
Changes for v4:
               -None.

 drivers/fpga/fpga-mgr.c    | 4 ++--
 drivers/fpga/fpga-region.c | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index d49a9ce34568..a699cc8e2fa6 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -151,8 +151,8 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
 	if (!mgr->mops->initial_header_size)
 		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
 	else
-		ret = fpga_mgr_write_init(
-		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
+		ret = fpga_mgr_write_init(mgr, info, buf,
+					  min(mgr->mops->initial_header_size, count));
 
 	if (ret) {
 		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index b0ac18de4885..3864bf4f8920 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -18,9 +18,10 @@
 static DEFINE_IDA(fpga_region_ida);
 static struct class *fpga_region_class;
 
-struct fpga_region *fpga_region_class_find(
-	struct device *start, const void *data,
-	int (*match)(struct device *, const void *))
+struct fpga_region *fpga_region_class_find(struct device *start,
+					   const void *data,
+					   int (*match)(struct device *,
+							const void *))
 {
 	struct device *dev;
 
-- 
2.25.1

[PATCH v4 3/5] fpga: fpga-mgr: fix kernel-doc warnings

[Nava kishore Manne]

warnings: No description found for return value of 'xxx'

In-order to fix the above kernel-doc warnings added the
'Return' description for 'devm_fpga_mgr_register_full()'
and 'devm_fpga_mgr_register()' API's.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v2:
                -Replaced s/@return:/Return:/
Changes for v3:
               -Updated commit description.
Changes for v4:
               -Updated commit description.

 drivers/fpga/fpga-mgr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index a699cc8e2fa6..0f2b28538f17 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -730,6 +730,8 @@ static void devm_fpga_mgr_unregister(struct device *dev, void *res)
  * @parent:	fpga manager device from pdev
  * @info:	parameters for fpga manager
  *
+ * Return:  fpga manager pointer on success, negative error code otherwise.
+ *
  * This is the devres variant of fpga_mgr_register_full() for which the unregister
  * function will be called automatically when the managing device is detached.
  */
@@ -763,6 +765,8 @@ EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_full);
  * @mops:	pointer to structure of fpga manager ops
  * @priv:	fpga manager private data
  *
+ * Return:  fpga manager pointer on success, negative error code otherwise.
+ *
  * This is the devres variant of fpga_mgr_register() for which the
  * unregister function will be called automatically when the managing
  * device is detached.
-- 
2.25.1

[PATCH v4 4/5] fpga: Use tab instead of space indentation

[Nava kishore Manne]

In FPGA Make file has both space and tab indentation to
make them align use tab instead of space indentation.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v2:
                -None.
Changes for v3:
                -Updated commit description.
Changes for v4:
                -None.

 drivers/fpga/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 0bff783d1b61..5935b3d0abd5 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -18,9 +18,9 @@ obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
 obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
 obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
-obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
-obj-$(CONFIG_ALTERA_PR_IP_CORE)         += altera-pr-ip-core.o
-obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)    += altera-pr-ip-core-plat.o
+obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)	+= versal-fpga.o
+obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
+obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
 
 # FPGA Bridge Drivers
 obj-$(CONFIG_FPGA_BRIDGE)		+= fpga-bridge.o
-- 
2.25.1

[PATCH v4 5/5] fpga: fpga-region: fix kernel-doc formatting issues

[Nava kishore Manne]

To fix below kernel-doc warnings this patch does the following
->Replaced Return\Returns with 'Return:' keyword.
->Added 'Return' description For __init of_fpga_region_init()' API.
->Added description for 'child_regions_with_firmware()' API.

warning: No description found for return value of
'of_fpga_region_find'.
warning: No description found for return value of
'of_fpga_region_get_bridges'.
warning: missing initial short description on line:
* child_regions_with_firmware
warning: No description found for return value of
'child_regions_with_firmware'.
warning: No description found for return value of
'of_fpga_region_notify_pre_apply'.
warning: No description found for return value of
'of_fpga_region_notify'.
warning: No description found for return value of
'of_fpga_region_init'.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
---
Changes for v2:
                -Replaced s/@return:/Return:/
Changes for v3:
               -Updated commit description.
Changes for v4:
               -Updated commit description.

 drivers/fpga/of-fpga-region.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 50b83057c048..9e330a2c0a1b 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -28,7 +28,7 @@ MODULE_DEVICE_TABLE(of, fpga_region_of_match);
  *
  * Caller will need to put_device(&region->dev) when done.
  *
- * Returns FPGA Region struct or NULL
+ * Return: FPGA Region struct or NULL
  */
 static struct fpga_region *of_fpga_region_find(struct device_node *np)
 {
@@ -80,7 +80,7 @@ static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np)
  * Caller should call fpga_bridges_put(&region->bridge_list) when
  * done with the bridges.
  *
- * Return 0 for success (even if there are no bridges specified)
+ * Return: 0 for success (even if there are no bridges specified)
  * or -EBUSY if any of the bridges are in use.
  */
 static int of_fpga_region_get_bridges(struct fpga_region *region)
@@ -139,13 +139,13 @@ static int of_fpga_region_get_bridges(struct fpga_region *region)
 }
 
 /**
- * child_regions_with_firmware
+ * child_regions_with_firmware - Used to check the child region info.
  * @overlay: device node of the overlay
  *
  * If the overlay adds child FPGA regions, they are not allowed to have
  * firmware-name property.
  *
- * Return 0 for OK or -EINVAL if child FPGA region adds firmware-name.
+ * Return: 0 for OK or -EINVAL if child FPGA region adds firmware-name.
  */
 static int child_regions_with_firmware(struct device_node *overlay)
 {
@@ -184,7 +184,7 @@ static int child_regions_with_firmware(struct device_node *overlay)
  * Given an overlay applied to an FPGA region, parse the FPGA image specific
  * info in the overlay and do some checking.
  *
- * Returns:
+ * Return:
  *   NULL if overlay doesn't direct us to program the FPGA.
  *   fpga_image_info struct if there is an image to program.
  *   error code for invalid overlay.
@@ -279,7 +279,7 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
  * If the checks fail, overlay is rejected and does not get added to the
  * live tree.
  *
- * Returns 0 for success or negative error code for failure.
+ * Return: 0 for success or negative error code for failure.
  */
 static int of_fpga_region_notify_pre_apply(struct fpga_region *region,
 					   struct of_overlay_notify_data *nd)
@@ -339,7 +339,7 @@ static void of_fpga_region_notify_post_remove(struct fpga_region *region,
  * This notifier handles programming an FPGA when a "firmware-name" property is
  * added to an fpga-region.
  *
- * Returns NOTIFY_OK or error if FPGA programming fails.
+ * Return: NOTIFY_OK or error if FPGA programming fails.
  */
 static int of_fpga_region_notify(struct notifier_block *nb,
 				 unsigned long action, void *arg)
@@ -446,6 +446,8 @@ static struct platform_driver of_fpga_region_driver = {
 /**
  * of_fpga_region_init - init function for fpga_region class
  * Creates the fpga_region class and registers a reconfig notifier.
+ *
+ * Return: 0 on success, negative error code otherwise.
  */
 static int __init of_fpga_region_init(void)
 {
-- 
2.25.1

Re: [PATCH v4 1/5] fpga: zynq: Fix incorrect variable type

[Xu Yilun]

On Sat, Apr 16, 2022 at 07:07:15PM +0530, Nava kishore Manne wrote:
> zynq_fpga_has_sync () API is expecting "u8 *" but the
> formal parameter that was passed is of type "const char *".
> fixes this issue by changing the buf type to "const char *"
> 
> This patch will also update zynq_fpga_has_sync () API description
> to align with API functionality.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> ---
> Changes for v2:
>                 -None.
> Changes for v3:
>                - Changed arg buf type to "const char *" as suggested by Tom.
>                - update zynq_fpga_has_sync () API description to align with API
>                  functionality.
> Changes for v4:
>                - None.

There are some minor comments in v3, please check it.

Thanks,
Yilun

> 
>  drivers/fpga/zynq-fpga.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 426aa34c6a0d..ada07eea64bc 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -235,11 +235,11 @@ static irqreturn_t zynq_fpga_isr(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -/* Sanity check the proposed bitstream. It must start with the sync word in
> - * the correct byte order, and be dword aligned. The input is a Xilinx .bin
> - * file with every 32 bit quantity swapped.
> +/* Sanity check the proposed bitstream. The sync word must be found in the
> + * correct byte order and it should be dword aligned. The input is a
> + * Xilinx.bin file with every 32 bit quantity swapped.
>   */
> -static bool zynq_fpga_has_sync(const u8 *buf, size_t count)
> +static bool zynq_fpga_has_sync(const char *buf, size_t count)
>  {
>  	for (; count >= 4; buf += 4, count -= 4)
>  		if (buf[0] == 0x66 && buf[1] == 0x55 && buf[2] == 0x99 &&
> -- 
> 2.25.1

Re: [PATCH v4 5/5] fpga: fpga-region: fix kernel-doc formatting issues

[Xu Yilun]

On Sat, Apr 16, 2022 at 07:07:19PM +0530, Nava kishore Manne wrote:
> To fix below kernel-doc warnings this patch does the following
> ->Replaced Return\Returns with 'Return:' keyword.
> ->Added 'Return' description For __init of_fpga_region_init()' API.
> ->Added description for 'child_regions_with_firmware()' API.
> 
> warning: No description found for return value of
> 'of_fpga_region_find'.
> warning: No description found for return value of
> 'of_fpga_region_get_bridges'.
> warning: missing initial short description on line:
> * child_regions_with_firmware
> warning: No description found for return value of
> 'child_regions_with_firmware'.
> warning: No description found for return value of
> 'of_fpga_region_notify_pre_apply'.
> warning: No description found for return value of
> 'of_fpga_region_notify'.
> warning: No description found for return value of
> 'of_fpga_region_init'.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
> Changes for v2:
>                 -Replaced s/@return:/Return:/
> Changes for v3:
>                -Updated commit description.
> Changes for v4:
>                -Updated commit description.
> 
>  drivers/fpga/of-fpga-region.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index 50b83057c048..9e330a2c0a1b 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -28,7 +28,7 @@ MODULE_DEVICE_TABLE(of, fpga_region_of_match);
>   *
>   * Caller will need to put_device(&region->dev) when done.
>   *
> - * Returns FPGA Region struct or NULL
> + * Return: FPGA Region struct or NULL
>   */
>  static struct fpga_region *of_fpga_region_find(struct device_node *np)
>  {
> @@ -80,7 +80,7 @@ static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np)
>   * Caller should call fpga_bridges_put(&region->bridge_list) when
>   * done with the bridges.
>   *
> - * Return 0 for success (even if there are no bridges specified)
> + * Return: 0 for success (even if there are no bridges specified)
>   * or -EBUSY if any of the bridges are in use.
>   */
>  static int of_fpga_region_get_bridges(struct fpga_region *region)
> @@ -139,13 +139,13 @@ static int of_fpga_region_get_bridges(struct fpga_region *region)
>  }
>  
>  /**
> - * child_regions_with_firmware
> + * child_regions_with_firmware - Used to check the child region info.
>   * @overlay: device node of the overlay
>   *
>   * If the overlay adds child FPGA regions, they are not allowed to have
>   * firmware-name property.
>   *
> - * Return 0 for OK or -EINVAL if child FPGA region adds firmware-name.
> + * Return: 0 for OK or -EINVAL if child FPGA region adds firmware-name.
>   */
>  static int child_regions_with_firmware(struct device_node *overlay)
>  {
> @@ -184,7 +184,7 @@ static int child_regions_with_firmware(struct device_node *overlay)
>   * Given an overlay applied to an FPGA region, parse the FPGA image specific
>   * info in the overlay and do some checking.
>   *
> - * Returns:
> + * Return:
>   *   NULL if overlay doesn't direct us to program the FPGA.
>   *   fpga_image_info struct if there is an image to program.
>   *   error code for invalid overlay.
> @@ -279,7 +279,7 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
>   * If the checks fail, overlay is rejected and does not get added to the
>   * live tree.
>   *
> - * Returns 0 for success or negative error code for failure.
> + * Return: 0 for success or negative error code for failure.
>   */
>  static int of_fpga_region_notify_pre_apply(struct fpga_region *region,
>  					   struct of_overlay_notify_data *nd)
> @@ -339,7 +339,7 @@ static void of_fpga_region_notify_post_remove(struct fpga_region *region,
>   * This notifier handles programming an FPGA when a "firmware-name" property is
>   * added to an fpga-region.
>   *
> - * Returns NOTIFY_OK or error if FPGA programming fails.
> + * Return: NOTIFY_OK or error if FPGA programming fails.
>   */
>  static int of_fpga_region_notify(struct notifier_block *nb,
>  				 unsigned long action, void *arg)
> @@ -446,6 +446,8 @@ static struct platform_driver of_fpga_region_driver = {
>  /**
>   * of_fpga_region_init - init function for fpga_region class
>   * Creates the fpga_region class and registers a reconfig notifier.
> + *
> + * Return: 0 on success, negative error code otherwise.
>   */
>  static int __init of_fpga_region_init(void)
>  {
> -- 
> 2.25.1

Re: [PATCH v4 4/5] fpga: Use tab instead of space indentation

[Xu Yilun]

On Sat, Apr 16, 2022 at 07:07:18PM +0530, Nava kishore Manne wrote:
> In FPGA Make file has both space and tab indentation to

          Makefile                         indentation, to

> make them align use tab instead of space indentation.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>

With the minor fixes, please add my Acked-by

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
> Changes for v2:
>                 -None.
> Changes for v3:
>                 -Updated commit description.
> Changes for v4:
>                 -None.
> 
>  drivers/fpga/Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 0bff783d1b61..5935b3d0abd5 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -18,9 +18,9 @@ obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
> -obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
> -obj-$(CONFIG_ALTERA_PR_IP_CORE)         += altera-pr-ip-core.o
> -obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)    += altera-pr-ip-core-plat.o
> +obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)	+= versal-fpga.o
> +obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
> +obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
>  
>  # FPGA Bridge Drivers
>  obj-$(CONFIG_FPGA_BRIDGE)		+= fpga-bridge.o
> -- 
> 2.25.1

Re: [PATCH v4 3/5] fpga: fpga-mgr: fix kernel-doc warnings

[Xu Yilun]

On Sat, Apr 16, 2022 at 07:07:17PM +0530, Nava kishore Manne wrote:
> warnings: No description found for return value of 'xxx'
> 
> In-order to fix the above kernel-doc warnings added the
> 'Return' description for 'devm_fpga_mgr_register_full()'
> and 'devm_fpga_mgr_register()' API's.

                                 APIs.

> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>

With the minor fix, please add my Acked-by.

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
> Changes for v2:
>                 -Replaced s/@return:/Return:/
> Changes for v3:
>                -Updated commit description.
> Changes for v4:
>                -Updated commit description.
> 
>  drivers/fpga/fpga-mgr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index a699cc8e2fa6..0f2b28538f17 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -730,6 +730,8 @@ static void devm_fpga_mgr_unregister(struct device *dev, void *res)
>   * @parent:	fpga manager device from pdev
>   * @info:	parameters for fpga manager
>   *
> + * Return:  fpga manager pointer on success, negative error code otherwise.
> + *
>   * This is the devres variant of fpga_mgr_register_full() for which the unregister
>   * function will be called automatically when the managing device is detached.
>   */
> @@ -763,6 +765,8 @@ EXPORT_SYMBOL_GPL(devm_fpga_mgr_register_full);
>   * @mops:	pointer to structure of fpga manager ops
>   * @priv:	fpga manager private data
>   *
> + * Return:  fpga manager pointer on success, negative error code otherwise.
> + *
>   * This is the devres variant of fpga_mgr_register() for which the
>   * unregister function will be called automatically when the managing
>   * device is detached.
> -- 
> 2.25.1

Re: [PATCH v4 2/5] fpga: fix for coding style issues

[Joe Perches]

On Sat, 2022-04-16 at 19:07 +0530, Nava kishore Manne wrote:
> fixes the below checks reported by checkpatch.pl
> Lines should not end with a '('
> Alignment should match open parenthesis

in fpga-mgr:
	Another possibillty would be to change the function arguments

and 

in fpga-region:
	Ideally keep the include declaration and definition styles synced

Perhaps:
---
 drivers/fpga/fpga-mgr.c          | 13 ++++++++-----
 drivers/fpga/fpga-region.c       |  6 +++---
 include/linux/fpga/fpga-region.h |  6 +++---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index d49a9ce34568..c65b97450a67 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -148,12 +148,15 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
 	int ret;
 
 	mgr->state = FPGA_MGR_STATE_WRITE_INIT;
-	if (!mgr->mops->initial_header_size)
-		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
-	else
-		ret = fpga_mgr_write_init(
-		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
 
+	if (mgr->mops->initial_header_size) {
+		count = min(mgr->mops->initial_header_size, count);
+	} else {
+		buf = NULL;
+		count = 0;
+	}
+
+	ret = fpga_mgr_write_init(mgr, info, buf, count);
 	if (ret) {
 		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
 		mgr->state = FPGA_MGR_STATE_WRITE_INIT_ERR;
diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index b0ac18de4885..485948e3c0db 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -18,9 +18,9 @@
 static DEFINE_IDA(fpga_region_ida);
 static struct class *fpga_region_class;
 
-struct fpga_region *fpga_region_class_find(
-	struct device *start, const void *data,
-	int (*match)(struct device *, const void *))
+struct fpga_region *
+fpga_region_class_find(struct device *start, const void *data,
+		       int (*match)(struct device *, const void *))
 {
 	struct device *dev;
 
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 3b87f232425c..9d4d32909340 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -52,9 +52,9 @@ struct fpga_region {
 
 #define to_fpga_region(d) container_of(d, struct fpga_region, dev)
 
-struct fpga_region *fpga_region_class_find(
-	struct device *start, const void *data,
-	int (*match)(struct device *, const void *));
+struct fpga_region *
+fpga_region_class_find(struct device *start, const void *data,
+		       int (*match)(struct device *, const void *));
 
 int fpga_region_program_fpga(struct fpga_region *region);
 

Re: [PATCH v4 2/5] fpga: fix for coding style issues

[Xu Yilun]

On Sat, Apr 16, 2022 at 07:07:16PM +0530, Nava kishore Manne wrote:
> fixes the below checks reported by checkpatch.pl
> Lines should not end with a '('
> Alignment should match open parenthesis

Please help format the commit log, like:

fixes the below checks reported by checkpatch.pl:

- Lines should not end with a '('
- Alignment should match open parenthesis

> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>

With the minor fixes, please add my Acked-by.

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
> Changes for v2:
>                 -None.
> Changes for v3:
>                -Fixed similar issue exists in "drivers/fpga/*".
> Changes for v4:
>                -None.
> 
>  drivers/fpga/fpga-mgr.c    | 4 ++--
>  drivers/fpga/fpga-region.c | 7 ++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index d49a9ce34568..a699cc8e2fa6 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -151,8 +151,8 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
>  	if (!mgr->mops->initial_header_size)
>  		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
>  	else
> -		ret = fpga_mgr_write_init(
> -		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
> +		ret = fpga_mgr_write_init(mgr, info, buf,
> +					  min(mgr->mops->initial_header_size, count));
>  
>  	if (ret) {
>  		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index b0ac18de4885..3864bf4f8920 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -18,9 +18,10 @@
>  static DEFINE_IDA(fpga_region_ida);
>  static struct class *fpga_region_class;
>  
> -struct fpga_region *fpga_region_class_find(
> -	struct device *start, const void *data,
> -	int (*match)(struct device *, const void *))
> +struct fpga_region *fpga_region_class_find(struct device *start,
> +					   const void *data,
> +					   int (*match)(struct device *,
> +							const void *))
>  {
>  	struct device *dev;
>  
> -- 
> 2.25.1

Re: [PATCH v4 2/5] fpga: fix for coding style issues

[Xu Yilun]

On Sun, Apr 17, 2022 at 01:02:57AM +0800, Xu Yilun wrote:
> On Sat, Apr 16, 2022 at 07:07:16PM +0530, Nava kishore Manne wrote:
> > fixes the below checks reported by checkpatch.pl
> > Lines should not end with a '('
> > Alignment should match open parenthesis
> 
> Please help format the commit log, like:
> 
> fixes the below checks reported by checkpatch.pl:
> 
> - Lines should not end with a '('
> - Alignment should match open parenthesis
> 
> > 
> > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> 
> With the minor fixes, please add my Acked-by.
> 
> Acked-by: Xu Yilun <yilun.xu@intel.com>

Sorry, please also check Joe's comments

Thanks,
Yilun

> 
> > ---
> > Changes for v2:
> >                 -None.
> > Changes for v3:
> >                -Fixed similar issue exists in "drivers/fpga/*".
> > Changes for v4:
> >                -None.
> > 
> >  drivers/fpga/fpga-mgr.c    | 4 ++--
> >  drivers/fpga/fpga-region.c | 7 ++++---
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> > index d49a9ce34568..a699cc8e2fa6 100644
> > --- a/drivers/fpga/fpga-mgr.c
> > +++ b/drivers/fpga/fpga-mgr.c
> > @@ -151,8 +151,8 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
> >  	if (!mgr->mops->initial_header_size)
> >  		ret = fpga_mgr_write_init(mgr, info, NULL, 0);
> >  	else
> > -		ret = fpga_mgr_write_init(
> > -		    mgr, info, buf, min(mgr->mops->initial_header_size, count));
> > +		ret = fpga_mgr_write_init(mgr, info, buf,
> > +					  min(mgr->mops->initial_header_size, count));
> >  
> >  	if (ret) {
> >  		dev_err(&mgr->dev, "Error preparing FPGA for writing\n");
> > diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> > index b0ac18de4885..3864bf4f8920 100644
> > --- a/drivers/fpga/fpga-region.c
> > +++ b/drivers/fpga/fpga-region.c
> > @@ -18,9 +18,10 @@
> >  static DEFINE_IDA(fpga_region_ida);
> >  static struct class *fpga_region_class;
> >  
> > -struct fpga_region *fpga_region_class_find(
> > -	struct device *start, const void *data,
> > -	int (*match)(struct device *, const void *))
> > +struct fpga_region *fpga_region_class_find(struct device *start,
> > +					   const void *data,
> > +					   int (*match)(struct device *,
> > +							const void *))
> >  {
> >  	struct device *dev;
> >  
> > -- 
> > 2.25.1

RE: [PATCH v4 2/5] fpga: fix for coding style issues

[Nava kishore Manne]

Hi Joe,

	Please find my response inline.

> -----Original Message-----
> From: Joe Perches <joe@perches.com>
> Sent: Saturday, April 16, 2022 10:29 PM
> To: Nava kishore Manne <navam@xilinx.com>; mdf@kernel.org;
> hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com; Michal Simek
> <michals@xilinx.com>; linux-fpga@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; git
> <git@xilinx.com>
> Subject: Re: [PATCH v4 2/5] fpga: fix for coding style issues
> 
> On Sat, 2022-04-16 at 19:07 +0530, Nava kishore Manne wrote:
> > fixes the below checks reported by checkpatch.pl Lines should not end
> > with a '('
> > Alignment should match open parenthesis
> 
> in fpga-mgr:
> 	Another possibillty would be to change the function arguments
> 
> and
> 
> in fpga-region:
> 	Ideally keep the include declaration and definition styles synced
> 

These changes are targeted to fix the checks reported by checkpatch.pl without touching the actual functionality.

Regards,
Navakishore.

Re: [PATCH v4 2/5] fpga: fix for coding style issues

[Joe Perches]

On Mon, 2022-04-18 at 09:54 +0000, Nava kishore Manne wrote:
> Hi Joe,
> 
> 	Please find my response inline.
> 
> > -----Original Message-----
> > From: Joe Perches <joe@perches.com>
> > Sent: Saturday, April 16, 2022 10:29 PM

> > On Sat, 2022-04-16 at 19:07 +0530, Nava kishore Manne wrote:
> > > fixes the below checks reported by checkpatch.pl Lines should not end
> > > with a '('
> > > Alignment should match open parenthesis
> > 
> > in fpga-mgr:
> > 	Another possibillty would be to change the function arguments
> > 
> > and
> > 
> > in fpga-region:
> > 	Ideally keep the include declaration and definition styles synced
> 
> These changes are targeted to fix the checks reported by checkpatch.pl without touching the actual functionality.

Hello.

I am the checkpatch maintainer.

The goal of the checkpatch script is simply to mark things that
generally don't conform to the typical kernel coding style not
to mandate that all of the messages it emits is dicta.

checkpatch is a stupid little script.  It has no taste.

Please take what I wrote with a little consideration rather than
follow the advise of a brainless script.

RE: [PATCH v4 2/5] fpga: fix for coding style issues

[Nava kishore Manne]

Hi Joe,

	Please find my response inline.

> -----Original Message-----
> From: Joe Perches <joe@perches.com>
> Sent: Monday, April 18, 2022 6:04 PM
> To: Nava kishore Manne <navam@xilinx.com>; mdf@kernel.org;
> hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com; Michal Simek
> <michals@xilinx.com>; linux-fpga@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; git
> <git@xilinx.com>
> Subject: Re: [PATCH v4 2/5] fpga: fix for coding style issues
> 
> On Mon, 2022-04-18 at 09:54 +0000, Nava kishore Manne wrote:
> > Hi Joe,
> >
> > 	Please find my response inline.
> >
> > > -----Original Message-----
> > > From: Joe Perches <joe@perches.com>
> > > Sent: Saturday, April 16, 2022 10:29 PM
> 
> > > On Sat, 2022-04-16 at 19:07 +0530, Nava kishore Manne wrote:
> > > > fixes the below checks reported by checkpatch.pl Lines should not
> > > > end with a '('
> > > > Alignment should match open parenthesis
> > >
> > > in fpga-mgr:
> > > 	Another possibillty would be to change the function arguments
> > >

This API is there for a long back. Not sure changing the function arguments is fine or Not.
@yilun: Is it ok to change the function arguments?

> > > and
> > >
> > > in fpga-region:
> > > 	Ideally keep the include declaration and definition styles synced
> >

Will fix it in next version.

Regards,
Navakishore.

Re: [PATCH v4 2/5] fpga: fix for coding style issues

[Xu Yilun]

On Tue, Apr 19, 2022 at 08:15:57AM +0000, Nava kishore Manne wrote:
> Hi Joe,
> 
> 	Please find my response inline.
> 
> > -----Original Message-----
> > From: Joe Perches <joe@perches.com>
> > Sent: Monday, April 18, 2022 6:04 PM
> > To: Nava kishore Manne <navam@xilinx.com>; mdf@kernel.org;
> > hao.wu@intel.com; yilun.xu@intel.com; trix@redhat.com; Michal Simek
> > <michals@xilinx.com>; linux-fpga@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; git
> > <git@xilinx.com>
> > Subject: Re: [PATCH v4 2/5] fpga: fix for coding style issues
> > 
> > On Mon, 2022-04-18 at 09:54 +0000, Nava kishore Manne wrote:
> > > Hi Joe,
> > >
> > > 	Please find my response inline.
> > >
> > > > -----Original Message-----
> > > > From: Joe Perches <joe@perches.com>
> > > > Sent: Saturday, April 16, 2022 10:29 PM
> > 
> > > > On Sat, 2022-04-16 at 19:07 +0530, Nava kishore Manne wrote:
> > > > > fixes the below checks reported by checkpatch.pl Lines should not
> > > > > end with a '('
> > > > > Alignment should match open parenthesis
> > > >
> > > > in fpga-mgr:
> > > > 	Another possibillty would be to change the function arguments
> > > >
> 
> This API is there for a long back. Not sure changing the function arguments is fine or Not.
> @yilun: Is it ok to change the function arguments?

Joe's example code below doesn't actually change any function definition.
It just tries to store the value of 'min(mgr->mops->initial_header_size, count)'
in 'count', thus to reduce the length of the 'fpga_mgr_write_init(...)'
expression.

So I think it is OK.

Thanks,
Yilun

> 
> > > > and
> > > >
> > > > in fpga-region:
> > > > 	Ideally keep the include declaration and definition styles synced
> > >
> 
> Will fix it in next version.
> 
> Regards,
> Navakishore.