[PATCH 1/2] char: xilinx_hwicap: Fix kernel doc warnings

[PATCH 1/2] char: xilinx_hwicap: Fix kernel doc warnings

[Michal Simek]

From: Nava kishore Manne <nava.manne@xilinx.com>

This patch fixes the kernel doc warnings in the driver.

Signed-off-by: Nava kishore Manne <navam@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/char/xilinx_hwicap/xilinx_hwicap.c | 4 ++++
 drivers/char/xilinx_hwicap/xilinx_hwicap.h | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
index 3e6b23c3453c..5e5c8ee23a5b 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
@@ -222,6 +222,8 @@
  * hwicap_command_desync - Send a DESYNC command to the ICAP port.
  * @drvdata: a pointer to the drvdata.
  *
+ * Returns: '0' on success and failure value on error
+ *
  * This command desynchronizes the ICAP After this command, a
  * bitstream containing a NULL packet, followed by a SYNCH packet is
  * required before the ICAP will recognize commands.
@@ -255,6 +257,8 @@ static int hwicap_command_desync(struct hwicap_drvdata *drvdata)
  * 		Examples:  XHI_IDCODE, XHI_FLR.
  * @reg_data: returns the value of the register.
  *
+ * Returns: '0' on success and failure value on error
+ *
  * Sends a query packet to the ICAP and then receives the response.
  * The icap is left in Synched state.
  */
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
index 38b145eaf24d..1f687a71e36e 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
@@ -193,11 +193,12 @@ struct config_registers {
  * hwicap_type_1_read - Generates a Type 1 read packet header.
  * @reg: is the address of the register to be read back.
  *
+ * Return:
  * Generates a Type 1 read packet header, which is used to indirectly
  * read registers in the configuration logic.  This packet must then
  * be sent through the icap device, and a return packet received with
  * the information.
- **/
+ */
 static inline u32 hwicap_type_1_read(u32 reg)
 {
 	return (XHI_TYPE_1 << XHI_TYPE_SHIFT) |
@@ -208,7 +209,9 @@ static inline u32 hwicap_type_1_read(u32 reg)
 /**
  * hwicap_type_1_write - Generates a Type 1 write packet header
  * @reg: is the address of the register to be read back.
- **/
+ *
+ * Return: Type 1 write packet header
+ */
 static inline u32 hwicap_type_1_write(u32 reg)
 {
 	return (XHI_TYPE_1 << XHI_TYPE_SHIFT) |
-- 
1.9.1

[PATCH 2/2] char: xilinx_hwicap: Fix warnings in the driver

[Michal Simek]

From: Nava kishore Manne <nava.manne@xilinx.com>

This patch fixes the below warning
        --> Use #include <linux/io.h> instead of <asm/io.h>
        --> Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
        --> please, no space before tabs
        --> Block comments use a trailing */ on a separate line
        --> Possible unnecessary 'out of memory' message
        --> Block comments use * on subsequent lines
        --> Block comments use a trailing */ on a separate line
        --> braces {} are not necessary for any arm of this statement
        --> DT compatible string "xlnx,opb-hwicap-1.00.b"
	    appears un-documented
        --> DT compatible string "xlnx,xps-hwicap-1.00.a"
            appears un-documented

Signed-off-by: Nava kishore Manne <navam@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 Documentation/devicetree/bindings/xilinx.txt |  2 ++
 drivers/char/xilinx_hwicap/xilinx_hwicap.c   | 35 ++++++++++++++--------------
 drivers/char/xilinx_hwicap/xilinx_hwicap.h   |  6 +++--
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/xilinx.txt b/Documentation/devicetree/bindings/xilinx.txt
index 299d0923537b..1d11b9002ef8 100644
--- a/Documentation/devicetree/bindings/xilinx.txt
+++ b/Documentation/devicetree/bindings/xilinx.txt
@@ -281,6 +281,8 @@
                       capabilities of the underlying ICAP hardware
                       differ between different families.  May be
                       'virtex2p', 'virtex4', or 'virtex5'.
+		- compatible : should contain "xlnx,xps-hwicap-1.00.a" or
+				"xlnx,opb-hwicap-1.00.b".
 
       vi) Xilinx Uart 16550
 
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
index 5e5c8ee23a5b..067396bedf22 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c
@@ -86,8 +86,7 @@
 #include <linux/cdev.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-
-#include <asm/io.h>
+#include <linux/io.h>
 #include <linux/uaccess.h>
 
 #ifdef CONFIG_OF
@@ -253,8 +252,8 @@ static int hwicap_command_desync(struct hwicap_drvdata *drvdata)
  * hwicap_get_configuration_register - Query a configuration register.
  * @drvdata: a pointer to the drvdata.
  * @reg: a constant which represents the configuration
- *		register value to be returned.
- * 		Examples:  XHI_IDCODE, XHI_FLR.
+ * register value to be returned.
+ * Examples: XHI_IDCODE, XHI_FLR.
  * @reg_data: returns the value of the register.
  *
  * Returns: '0' on success and failure value on error
@@ -324,7 +323,8 @@ static int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata)
 	dev_dbg(drvdata->dev, "initializing\n");
 
 	/* Abort any current transaction, to make sure we have the
-	 * ICAP in a good state. */
+	 * ICAP in a good state.
+	 */
 	dev_dbg(drvdata->dev, "Reset...\n");
 	drvdata->config->reset(drvdata);
 
@@ -636,7 +636,6 @@ static int hwicap_setup(struct device *dev, int id,
 
 	drvdata = kzalloc(sizeof(struct hwicap_drvdata), GFP_KERNEL);
 	if (!drvdata) {
-		dev_err(dev, "Couldn't allocate device private record\n");
 		retval = -ENOMEM;
 		goto failed0;
 	}
@@ -763,20 +762,20 @@ static int hwicap_of_probe(struct platform_device *op,
 	id = of_get_property(op->dev.of_node, "port-number", NULL);
 
 	/* It's most likely that we're using V4, if the family is not
-	   specified */
+	 * specified
+	 */
 	regs = &v4_config_registers;
 	family = of_get_property(op->dev.of_node, "xlnx,family", NULL);
 
 	if (family) {
-		if (!strcmp(family, "virtex2p")) {
+		if (!strcmp(family, "virtex2p"))
 			regs = &v2_config_registers;
-		} else if (!strcmp(family, "virtex4")) {
+		else if (!strcmp(family, "virtex4"))
 			regs = &v4_config_registers;
-		} else if (!strcmp(family, "virtex5")) {
+		else if (!strcmp(family, "virtex5"))
 			regs = &v5_config_registers;
-		} else if (!strcmp(family, "virtex6")) {
+		else if (!strcmp(family, "virtex6"))
 			regs = &v6_config_registers;
-		}
 	}
 	return hwicap_setup(&op->dev, id ? *id : -1, &res, config,
 			regs);
@@ -806,20 +805,20 @@ static int hwicap_drv_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	/* It's most likely that we're using V4, if the family is not
-	   specified */
+	 * specified
+	 */
 	regs = &v4_config_registers;
 	family = pdev->dev.platform_data;
 
 	if (family) {
-		if (!strcmp(family, "virtex2p")) {
+		if (!strcmp(family, "virtex2p"))
 			regs = &v2_config_registers;
-		} else if (!strcmp(family, "virtex4")) {
+		else if (!strcmp(family, "virtex4"))
 			regs = &v4_config_registers;
-		} else if (!strcmp(family, "virtex5")) {
+		else if (!strcmp(family, "virtex5"))
 			regs = &v5_config_registers;
-		} else if (!strcmp(family, "virtex6")) {
+		else if (!strcmp(family, "virtex6"))
 			regs = &v6_config_registers;
-		}
 	}
 
 	return hwicap_setup(&pdev->dev, pdev->id, res,
diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
index 1f687a71e36e..6b963d1c8ba3 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
@@ -62,11 +62,13 @@ struct hwicap_drvdata {
 
 struct hwicap_driver_config {
 	/* Read configuration data given by size into the data buffer.
-	   Return 0 if successful. */
+	 * Return 0 if successful.
+	 */
 	int (*get_configuration)(struct hwicap_drvdata *drvdata, u32 *data,
 			u32 size);
 	/* Write configuration data given by size from the data buffer.
-	   Return 0 if successful. */
+	 * Return 0 if successful.
+	 */
 	int (*set_configuration)(struct hwicap_drvdata *drvdata, u32 *data,
 			u32 size);
 	/* Get the status register, bit pattern given by:
-- 
1.9.1

Re: [PATCH 2/2] char: xilinx_hwicap: Fix warnings in the driver

[Rob Herring]

On Fri, Jul 28, 2017 at 03:17:26PM +0200, Michal Simek wrote:
> From: Nava kishore Manne <nava.manne@xilinx.com>
> 
> This patch fixes the below warning
>         --> Use #include <linux/io.h> instead of <asm/io.h>
>         --> Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
>         --> please, no space before tabs
>         --> Block comments use a trailing */ on a separate line
>         --> Possible unnecessary 'out of memory' message
>         --> Block comments use * on subsequent lines
>         --> Block comments use a trailing */ on a separate line
>         --> braces {} are not necessary for any arm of this statement
>         --> DT compatible string "xlnx,opb-hwicap-1.00.b"
> 	    appears un-documented
>         --> DT compatible string "xlnx,xps-hwicap-1.00.a"
>             appears un-documented
> 
> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  Documentation/devicetree/bindings/xilinx.txt |  2 ++

It's preferred to split bindings to separate patches. No need unless you 
respin:

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/char/xilinx_hwicap/xilinx_hwicap.c   | 35 ++++++++++++++--------------
>  drivers/char/xilinx_hwicap/xilinx_hwicap.h   |  6 +++--
>  3 files changed, 23 insertions(+), 20 deletions(-)

Re: [PATCH 2/2] char: xilinx_hwicap: Fix warnings in the driver

[Michal Simek]

On 4.8.2017 00:32, Rob Herring wrote:
> On Fri, Jul 28, 2017 at 03:17:26PM +0200, Michal Simek wrote:
>> From: Nava kishore Manne <nava.manne@xilinx.com>
>>
>> This patch fixes the below warning
>>         --> Use #include <linux/io.h> instead of <asm/io.h>
>>         --> Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
>>         --> please, no space before tabs
>>         --> Block comments use a trailing */ on a separate line
>>         --> Possible unnecessary 'out of memory' message
>>         --> Block comments use * on subsequent lines
>>         --> Block comments use a trailing */ on a separate line
>>         --> braces {} are not necessary for any arm of this statement
>>         --> DT compatible string "xlnx,opb-hwicap-1.00.b"
>> 	    appears un-documented
>>         --> DT compatible string "xlnx,xps-hwicap-1.00.a"
>>             appears un-documented
>>
>> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  Documentation/devicetree/bindings/xilinx.txt |  2 ++
> 
> It's preferred to split bindings to separate patches. No need unless you 
> respin:
> 
> Acked-by: Rob Herring <robh@kernel.org>

thanks. Was there any outcome from that discussion to extract binding
out of kernel source code?

Thanks,
Michal

Re: [PATCH 2/2] char: xilinx_hwicap: Fix warnings in the driver

[Rob Herring]

On Fri, Aug 4, 2017 at 2:49 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 4.8.2017 00:32, Rob Herring wrote:
>> On Fri, Jul 28, 2017 at 03:17:26PM +0200, Michal Simek wrote:
>>> From: Nava kishore Manne <nava.manne@xilinx.com>
>>>
>>> This patch fixes the below warning
>>>         --> Use #include <linux/io.h> instead of <asm/io.h>
>>>         --> Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
>>>         --> please, no space before tabs
>>>         --> Block comments use a trailing */ on a separate line
>>>         --> Possible unnecessary 'out of memory' message
>>>         --> Block comments use * on subsequent lines
>>>         --> Block comments use a trailing */ on a separate line
>>>         --> braces {} are not necessary for any arm of this statement
>>>         --> DT compatible string "xlnx,opb-hwicap-1.00.b"
>>>          appears un-documented
>>>         --> DT compatible string "xlnx,xps-hwicap-1.00.a"
>>>             appears un-documented
>>>
>>> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>  Documentation/devicetree/bindings/xilinx.txt |  2 ++
>>
>> It's preferred to split bindings to separate patches. No need unless you
>> respin:
>>
>> Acked-by: Rob Herring <robh@kernel.org>
>
> thanks. Was there any outcome from that discussion to extract binding
> out of kernel source code?

Well, the reason to split the patches is so the history of the
filtered DT repository we generate from the kernel tree[1] looks
saner.

As far as actually moving things out, no there's not been any
movement. My biggest concern with moving things out would be the loss
of kernel subsystem maintainers' review. Once it's a separate tree and
not merged thru their tree, we'd loose their reviews and I don't have
confidence that we'd be gaining reviews from other places. Though some
subsystem maintainers merge bindings and don't look at them already.

Rob

[1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/

Re: [PATCH 2/2] char: xilinx_hwicap: Fix warnings in the driver

[Michal Simek]

On 10.8.2017 20:16, Rob Herring wrote:
> On Fri, Aug 4, 2017 at 2:49 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 4.8.2017 00:32, Rob Herring wrote:
>>> On Fri, Jul 28, 2017 at 03:17:26PM +0200, Michal Simek wrote:
>>>> From: Nava kishore Manne <nava.manne@xilinx.com>
>>>>
>>>> This patch fixes the below warning
>>>>         --> Use #include <linux/io.h> instead of <asm/io.h>
>>>>         --> Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
>>>>         --> please, no space before tabs
>>>>         --> Block comments use a trailing */ on a separate line
>>>>         --> Possible unnecessary 'out of memory' message
>>>>         --> Block comments use * on subsequent lines
>>>>         --> Block comments use a trailing */ on a separate line
>>>>         --> braces {} are not necessary for any arm of this statement
>>>>         --> DT compatible string "xlnx,opb-hwicap-1.00.b"
>>>>          appears un-documented
>>>>         --> DT compatible string "xlnx,xps-hwicap-1.00.a"
>>>>             appears un-documented
>>>>
>>>> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>>  Documentation/devicetree/bindings/xilinx.txt |  2 ++
>>>
>>> It's preferred to split bindings to separate patches. No need unless you
>>> respin:
>>>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>
>> thanks. Was there any outcome from that discussion to extract binding
>> out of kernel source code?
> 
> Well, the reason to split the patches is so the history of the
> filtered DT repository we generate from the kernel tree[1] looks
> saner.
> 

So many merge commits. It looks weird.

https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/log/

> As far as actually moving things out, no there's not been any
> movement. My biggest concern with moving things out would be the loss
> of kernel subsystem maintainers' review. Once it's a separate tree and
> not merged thru their tree, we'd loose their reviews and I don't have
> confidence that we'd be gaining reviews from other places. Though some
> subsystem maintainers merge bindings and don't look at them already.

ok.

Thanks,
Michal