[PATCH 1/3 V2] bus: hisi_lpc: remove unused head file in hisi_lpc.c

[PATCH 1/3 V2] bus: hisi_lpc: remove unused head file in hisi_lpc.c

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

linux/console.h,linux/pci.h,linux/slab.h
are not used, removes it.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v2: fix build error in i386
---
 drivers/bus/hisi_lpc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 53dd1573e323..f0ef9680ad66 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -7,17 +7,14 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/console.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/logic_pio.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
-#include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/serial_8250.h>
-#include <linux/slab.h>
 
 #define DRV_NAME "hisi-lpc"
 
-- 
2.25.1

Re: [PATCH 1/3 V2] bus: hisi_lpc: remove unused head file in hisi_lpc.c

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 07:31:22PM +0800, xiaopeitux@foxmail.com wrote:
> From: Pei Xiao <xiaopei01@kylinos.cn>
> 
> linux/console.h,linux/pci.h,linux/slab.h
> are not used, removes it.

There are more unused and many are missed.
My diff looks like this:

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 53dd1573e323..4911afdac316 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -7,17 +7,22 @@
  */
 
 #include <linux/acpi.h>
-#include <linux/console.h>
+#include <linux/bits.h>
 #include <linux/delay.h>
+#include <linux/err.h>
 #include <linux/io.h>
+#include <linux/ioport.h>
 #include <linux/logic_pio.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/of_platform.h>
-#include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/serial_8250.h>
-#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include <asm/byteorder.h>
 
 #define DRV_NAME "hisi-lpc"
 

...

>  #include <linux/acpi.h>
> -#include <linux/console.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/logic_pio.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> -#include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/serial_8250.h>
> -#include <linux/slab.h>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3 V2] bus: hisi_lpc: remove unused head file in hisi_lpc.c

[Pei Xiao]

在 2025/4/22 23:47, Andy Shevchenko 写道:
> On Tue, Apr 22, 2025 at 07:31:22PM +0800, xiaopeitux@foxmail.com wrote:
>> From: Pei Xiao <xiaopei01@kylinos.cn>
>>
>> linux/console.h,linux/pci.h,linux/slab.h
>> are not used, removes it.
> There are more unused and many are missed.
> My diff looks like this:
>
> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
> index 53dd1573e323..4911afdac316 100644
> --- a/drivers/bus/hisi_lpc.c
> +++ b/drivers/bus/hisi_lpc.c
> @@ -7,17 +7,22 @@
>   */
>  
>  #include <linux/acpi.h>
> -#include <linux/console.h>
> +#include <linux/bits.h>
>  #include <linux/delay.h>
> +#include <linux/err.h>
>  #include <linux/io.h>
> +#include <linux/ioport.h>
>  #include <linux/logic_pio.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
>  #include <linux/of_platform.h>
> -#include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/serial_8250.h>
> -#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +#include <asm/byteorder.h>

Thanks for you help.

I have send v3 : https://lore.kernel.org/lkml/cover.1745377493.git.xiaopei01@kylinos.cn/

You are such a helpful and technically skilled expert!

Thanks again!

Pei.

>  
>  #define DRV_NAME "hisi-lpc"
>  
>
> ...
>
>>  #include <linux/acpi.h>
>> -#include <linux/console.h>
>>  #include <linux/delay.h>
>>  #include <linux/io.h>
>>  #include <linux/logic_pio.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_platform.h>
>> -#include <linux/pci.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/serial_8250.h>
>> -#include <linux/slab.h>

[PATCH 2/3 V1] lib: logic_pio: export function symbol in logic_pio

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

export some function via EXPORT_SYMBOL, convenient to use in
kernel modules in the future if somebody went to use this lib.
This is one reason for this library exists.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 lib/logic_pio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/logic_pio.c b/lib/logic_pio.c
index e29496a38d06..db3ad65ccc48 100644
--- a/lib/logic_pio.c
+++ b/lib/logic_pio.c
@@ -99,6 +99,7 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
 	mutex_unlock(&io_range_mutex);
 	return ret;
 }
+EXPORT_SYMBOL(logic_pio_register_range);
 
 /**
  * logic_pio_unregister_range - unregister a logical PIO range for a host
@@ -113,6 +114,7 @@ void logic_pio_unregister_range(struct logic_pio_hwaddr *range)
 	mutex_unlock(&io_range_mutex);
 	synchronize_rcu();
 }
+EXPORT_SYMBOL(logic_pio_unregister_range);
 
 /**
  * find_io_range_by_fwnode - find logical PIO range for given FW node
@@ -137,6 +139,7 @@ struct logic_pio_hwaddr *find_io_range_by_fwnode(const struct fwnode_handle *fwn
 
 	return found_range;
 }
+EXPORT_SYMBOL(find_io_range_by_fwnode);
 
 /* Return a registered range given an input PIO token */
 static struct logic_pio_hwaddr *find_io_range(unsigned long pio)
@@ -177,6 +180,7 @@ resource_size_t logic_pio_to_hwaddr(unsigned long pio)
 
 	return (resource_size_t)~0;
 }
+EXPORT_SYMBOL(logic_pio_to_hwaddr);
 
 /**
  * logic_pio_trans_hwaddr - translate HW address to logical PIO
@@ -203,6 +207,7 @@ unsigned long logic_pio_trans_hwaddr(const struct fwnode_handle *fwnode,
 	}
 	return addr - range->hw_start + range->io_start;
 }
+EXPORT_SYMBOL(logic_pio_trans_hwaddr);
 
 unsigned long logic_pio_trans_cpuaddr(resource_size_t addr)
 {
@@ -227,6 +232,7 @@ unsigned long logic_pio_trans_cpuaddr(resource_size_t addr)
 
 	return ~0UL;
 }
+EXPORT_SYMBOL(logic_pio_trans_cpuaddr);
 
 #if defined(CONFIG_INDIRECT_PIO) && defined(PCI_IOBASE)
 #define BUILD_LOGIC_IO(bwl, type)					\
-- 
2.25.1

Re: [PATCH 2/3 V1] lib: logic_pio: export function symbol in logic_pio

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 07:31:59PM +0800, xiaopeitux@foxmail.com wrote:
> From: Pei Xiao <xiaopei01@kylinos.cn>
> 
> export some function via EXPORT_SYMBOL, convenient to use in

Export

> kernel modules in the future if somebody went to use this lib.
> This is one reason for this library exists.

Can we use namespace from day 1, please?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3 V1] lib: logic_pio: export function symbol in logic_pio

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 07:31:59PM +0800, xiaopeitux@foxmail.com wrote:
> From: Pei Xiao <xiaopei01@kylinos.cn>
> 
> export some function via EXPORT_SYMBOL, convenient to use in
> kernel modules in the future if somebody went to use this lib.
> This is one reason for this library exists.

Btw, why V1?!

Really, stop sending new patches until you assure that the format is correct.

With standard tools this can be simply achieved by running:

`git format-patch --cover-letter -v3 --thread ...`

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3 V1] lib: logic_pio: export function symbol in logic_pio

[John Garry]

On 22/04/2025 12:31, xiaopeitux@foxmail.com wrote:
> From: Pei Xiao<xiaopei01@kylinos.cn>
> 
> export some function via EXPORT_SYMBOL, convenient to use in
> kernel modules in the future if somebody went to use this lib.
> This is one reason for this library exists.
> 
> Signed-off-by: Pei Xiao<xiaopei01@kylinos.cn>

No, we don't export just because someone might need this is future.

And, FWIW, EXPORT_SYMBOL_GPL is preferred.

Re: [PATCH 2/3 V1] lib: logic_pio: export function symbol in logic_pio

[Pei Xiao]

在 2025/4/23 00:05, John Garry 写道:
> On 22/04/2025 12:31, xiaopeitux@foxmail.com wrote:
>> From: Pei Xiao<xiaopei01@kylinos.cn>
>>
>> export some function via EXPORT_SYMBOL, convenient to use in
>> kernel modules in the future if somebody went to use this lib.
>> This is one reason for this library exists.
>>
>> Signed-off-by: Pei Xiao<xiaopei01@kylinos.cn>
>
> No, we don't export just because someone might need this is future.
>
Now modify it，Might it be convenient for future people?

People in the future might be able to save some effort here.

> And, FWIW, EXPORT_SYMBOL_GPL is preferred.

Re: [PATCH 2/3 V1] lib: logic_pio: export function symbol in logic_pio

[John Garry]

On 23/04/2025 04:36, Pei Xiao wrote:
> 
> 在 2025/4/23 00:05, John Garry 写道:
>> On 22/04/2025 12:31, xiaopeitux@foxmail.com wrote:
>>> From: Pei Xiao<xiaopei01@kylinos.cn>
>>>
>>> export some function via EXPORT_SYMBOL, convenient to use in
>>> kernel modules in the future if somebody went to use this lib.
>>> This is one reason for this library exists.
>>>
>>> Signed-off-by: Pei Xiao<xiaopei01@kylinos.cn>
>>
>> No, we don't export just because someone might need this is future.
>>
> Now modify it，Might it be convenient for future people?
> 
> People in the future might be able to save some effort here.

No, absolutely not. The reason we don't is because it enables more 
out-of-tree rubbish.

> 
>> And, FWIW, EXPORT_SYMBOL_GPL is preferred.

[PATCH 3/3 V2] lib: logic_pio: Add detailed comments for find_io_range() and logic_pio_trans_cpuaddr()

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

Add comments that we were previously overlooked for find_io_range()
and logic_pio_trans_cpuaddr().

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v2: fix returns differ
---
 lib/logic_pio.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/logic_pio.c b/lib/logic_pio.c
index db3ad65ccc48..44ba50386714 100644
--- a/lib/logic_pio.c
+++ b/lib/logic_pio.c
@@ -141,7 +141,15 @@ struct logic_pio_hwaddr *find_io_range_by_fwnode(const struct fwnode_handle *fwn
 }
 EXPORT_SYMBOL(find_io_range_by_fwnode);
 
-/* Return a registered range given an input PIO token */
+/**
+ * find_io_range - find a registered range by PIO
+ * @pio: logical PIO value
+ *
+ * Traverse the io_range_list to find the registered node for @pio.
+ * The input PIO should be unique in the whole logical PIO space.
+ *
+ * Returns a registered range, NULL otherwise.
+ */
 static struct logic_pio_hwaddr *find_io_range(unsigned long pio)
 {
 	struct logic_pio_hwaddr *range, *found_range = NULL;
@@ -209,6 +217,14 @@ unsigned long logic_pio_trans_hwaddr(const struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL(logic_pio_trans_hwaddr);
 
+/**
+ * logic_pio_trans_cpuaddr - translate CPU address to logical PIO
+ * @addr: Host-relative CPU address
+ *
+ * translate CPU address to logical PIO
+ *
+ * Returns Logical PIO value if successful, ~0UL otherwise
+ */
 unsigned long logic_pio_trans_cpuaddr(resource_size_t addr)
 {
 	struct logic_pio_hwaddr *range;
-- 
2.25.1

Re: [PATCH 3/3 V2] lib: logic_pio: Add detailed comments for find_io_range() and logic_pio_trans_cpuaddr()

[Andy Shevchenko]

On Tue, Apr 22, 2025 at 07:32:19PM +0800, xiaopeitux@foxmail.com wrote:
> From: Pei Xiao <xiaopei01@kylinos.cn>
> 
> Add comments that we were previously overlooked for find_io_range()
> and logic_pio_trans_cpuaddr().

You effectively ignored part of my comment. Please, run kernel-doc validator
and see the result. There are missing "Return:" sections. That's what it will
tell you.

-- 
With Best Regards,
Andy Shevchenko