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 | 22 ++++++++++++----------
drivers/fpga/zynq-fpga.c | 2 +-
include/linux/fpga/fpga-region.h | 7 ++++---
6 files changed, 30 insertions(+), 22 deletions(-)
--
2.25.1
On Thu, Apr 21, 2022 at 10:17:39AM +0530, Nava kishore Manne wrote:
> 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 | 22 ++++++++++++----------
> drivers/fpga/zynq-fpga.c | 2 +-
> include/linux/fpga/fpga-region.h | 7 ++++---
> 6 files changed, 30 insertions(+), 22 deletions(-)
>
> --
> 2.25.1
>
I've applied patches 1-4 to for-next.
Patch 5 seems to not apply.
- Moritz
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 <[email protected]>
Acked-by: Xu Yilun <[email protected]>
---
Changes for v2:
-Replaced s/@return:/Return:/
Changes for v3:
-Updated commit description.
Changes for v4:
-Updated commit description.
Changes for v5:
-None.
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 55209737075f..b5ae3e8de5ab 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(®ion->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(®ion->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
* 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
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()' APIs.
Signed-off-by: Nava kishore Manne <[email protected]>
Acked-by: Xu Yilun <[email protected]>
---
Changes for v2:
-Replaced s/@return:/Return:/
Changes for v3:
-Updated commit description.
Changes for v4:
-Updated commit description.
Changes for v5:
-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 d9a2aad7b35f..6bd018f20793 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
In FPGA Makefile has both space and tab indentation, to
make them align use tab instead of space indentation.
Signed-off-by: Nava kishore Manne <[email protected]>
Acked-by: Xu Yilun <[email protected]>
---
Changes for v2:
-None.
Changes for v3:
-Updated commit description.
Changes for v4:
-None.
Changes for v5:
-Updated commit description.
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
On Thu, Apr 21, 2022 at 07:25:54AM -0700, Moritz Fischer wrote:
> On Thu, Apr 21, 2022 at 10:17:39AM +0530, Nava kishore Manne wrote:
> > 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 | 22 ++++++++++++----------
> > drivers/fpga/zynq-fpga.c | 2 +-
> > include/linux/fpga/fpga-region.h | 7 ++++---
> > 6 files changed, 30 insertions(+), 22 deletions(-)
> >
> > --
> > 2.25.1
> >
> I've applied patches 1-4 to for-next.
>
> Patch 5 seems to not apply.
Hi Moritz:
Seems some patches still need rework, is it possible we drop them
and wait for next version?
Thanks,
Yilun
>
> - Moritz
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 <[email protected]>
Acked-by: Xu Yilun <[email protected]>
---
Changes for v2:
-None.
Changes for v3:
-Fixed similar issue exists in "drivers/fpga/*".
Changes for v4:
-None.
Changes for v5:
- Reduced the length of the 'fpga_mgr_write_init(...)' API
as suggested by Joe.
- To align Include declaration and definition of APIs updated
the FPGA-region.h file as suggested by joe.
drivers/fpga/fpga-mgr.c | 4 ++--
drivers/fpga/fpga-region.c | 7 ++++---
drivers/fpga/of-fpga-region.c | 6 +++---
include/linux/fpga/fpga-region.h | 7 ++++---
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index d49a9ce34568..d9a2aad7b35f 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -148,11 +148,11 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
int ret;
mgr->state = FPGA_MGR_STATE_WRITE_INIT;
+ count = min(mgr->mops->initial_header_size, count);
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, 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;
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index 50b83057c048..55209737075f 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -189,9 +189,9 @@ static int child_regions_with_firmware(struct device_node *overlay)
* fpga_image_info struct if there is an image to program.
* error code for invalid overlay.
*/
-static struct fpga_image_info *of_fpga_region_parse_ov(
- struct fpga_region *region,
- struct device_node *overlay)
+static struct fpga_image_info
+*of_fpga_region_parse_ov(struct fpga_region *region,
+ struct device_node *overlay)
{
struct device *dev = ®ion->dev;
struct fpga_image_info *info;
diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
index 3b87f232425c..7ebf743b8f8a 100644
--- a/include/linux/fpga/fpga-region.h
+++ b/include/linux/fpga/fpga-region.h
@@ -52,9 +52,10 @@ 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);
--
2.25.1
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 *"
Signed-off-by: Nava kishore Manne <[email protected]>
---
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.
Changes for v5:
- Dropped the irrelevant doc update changes.
drivers/fpga/zynq-fpga.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
index 426aa34c6a0d..6beaba9dfe97 100644
--- a/drivers/fpga/zynq-fpga.c
+++ b/drivers/fpga/zynq-fpga.c
@@ -239,7 +239,7 @@ static irqreturn_t zynq_fpga_isr(int irq, void *data)
* the correct byte order, and 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
On Thu, Apr 21, 2022 at 10:17:41AM +0530, Nava kishore Manne wrote:
> 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 <[email protected]>
> Acked-by: Xu Yilun <[email protected]>
Sorry, the checkpatch maintainer Joe gives comments after my Acked-by,
so maybe more changes should be made, see my comments inline.
And if you have more changes than expectation, it is better to remove the
Acked-by/Reviewed-by tags, otherwise people thought the reviewers are
good to new changes which are actually not checked at all.
> ---
> Changes for v2:
> -None.
> Changes for v3:
> -Fixed similar issue exists in "drivers/fpga/*".
> Changes for v4:
> -None.
> Changes for v5:
> - Reduced the length of the 'fpga_mgr_write_init(...)' API
> as suggested by Joe.
> - To align Include declaration and definition of APIs updated
> the FPGA-region.h file as suggested by joe.
The of_fpga_region_parse_ov() changes are also new to us.
>
> drivers/fpga/fpga-mgr.c | 4 ++--
> drivers/fpga/fpga-region.c | 7 ++++---
> drivers/fpga/of-fpga-region.c | 6 +++---
> include/linux/fpga/fpga-region.h | 7 ++++---
> 4 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index d49a9ce34568..d9a2aad7b35f 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -148,11 +148,11 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr,
> int ret;
>
> mgr->state = FPGA_MGR_STATE_WRITE_INIT;
> + count = min(mgr->mops->initial_header_size, count);
Move the line in 'else' block, cause the count is not used
if(!mgr->mops->initial_header_size).
> 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, 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 *))
According to Joe's suggestion, it had better be:
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/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index 50b83057c048..55209737075f 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -189,9 +189,9 @@ static int child_regions_with_firmware(struct device_node *overlay)
> * fpga_image_info struct if there is an image to program.
> * error code for invalid overlay.
> */
> -static struct fpga_image_info *of_fpga_region_parse_ov(
> - struct fpga_region *region,
> - struct device_node *overlay)
> +static struct fpga_image_info
> +*of_fpga_region_parse_ov(struct fpga_region *region,
> + struct device_node *overlay)
Don't put the '*' at the beginning, it should be:
static struct fpga_image_info *
of_fpga_region_parse_ov(struct fpga_region *region,
struct device_node *overlay)
> {
> struct device *dev = ®ion->dev;
> struct fpga_image_info *info;
> diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h
> index 3b87f232425c..7ebf743b8f8a 100644
> --- a/include/linux/fpga/fpga-region.h
> +++ b/include/linux/fpga/fpga-region.h
> @@ -52,9 +52,10 @@ 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 *));
Please also follow Joe's suggestion:
struct fpga_region *
fpga_region_class_find(struct device *start, const void *data,
int (*match)(struct device *, const void *));
Thanks,
Yilun
>
> int fpga_region_program_fpga(struct fpga_region *region);
>
> --
> 2.25.1