hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write

[PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write

Posted by Morduan Zang 1 month, 1 week ago

Replace the custom gpd_ecram_read() and gpd_ecram_write() functions that
use direct I/O port access (inb/outb) with the kernel's standard ec_read()
and ec_write() functions. This provides better abstraction, improves code
maintainability, and ensures compatibility across different kernel
versions.

Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
---
 drivers/hwmon/gpd-fan.c | 37 +++----------------------------------
 1 file changed, 3 insertions(+), 34 deletions(-)

diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
index 1729729b135f..c44957b7fd91 100644
--- a/drivers/hwmon/gpd-fan.c
+++ b/drivers/hwmon/gpd-fan.c
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/acpi.h>
 
 #define DRIVER_NAME "gpdfan"
 #define GPD_PWM_CTR_OFFSET 0x1841
@@ -243,44 +244,12 @@ static const struct gpd_fan_drvdata *gpd_module_drvdata[] = {
 // Helper functions to handle EC read/write
 static void gpd_ecram_read(u16 offset, u8 *val)
 {
-	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
-	u16 data_port = gpd_driver_priv.drvdata->data_port;
-
-	outb(0x2E, addr_port);
-	outb(0x11, data_port);
-	outb(0x2F, addr_port);
-	outb((u8)((offset >> 8) & 0xFF), data_port);
-
-	outb(0x2E, addr_port);
-	outb(0x10, data_port);
-	outb(0x2F, addr_port);
-	outb((u8)(offset & 0xFF), data_port);
-
-	outb(0x2E, addr_port);
-	outb(0x12, data_port);
-	outb(0x2F, addr_port);
-	*val = inb(data_port);
+	*val = ec_read(offset, val);
 }
 
 static void gpd_ecram_write(u16 offset, u8 value)
 {
-	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
-	u16 data_port = gpd_driver_priv.drvdata->data_port;
-
-	outb(0x2E, addr_port);
-	outb(0x11, data_port);
-	outb(0x2F, addr_port);
-	outb((u8)((offset >> 8) & 0xFF), data_port);
-
-	outb(0x2E, addr_port);
-	outb(0x10, data_port);
-	outb(0x2F, addr_port);
-	outb((u8)(offset & 0xFF), data_port);
-
-	outb(0x2E, addr_port);
-	outb(0x12, data_port);
-	outb(0x2F, addr_port);
-	outb(value, data_port);
+	ec_write(offset, value);
 }
 
 static int gpd_generic_read_rpm(void)
-- 
2.50.1

Re: [PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write

Posted by kernel test robot 1 month, 1 week ago

Hi Morduan,

kernel test robot noticed the following build errors:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on linus/master v7.0-rc1 next-20260226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Morduan-Zang/hwmon-gpd-fan-replace-custom-EC-I-O-with-kernel-ec_read-ec_write/20260227-105255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/53C09CAECC90EB98%2B20260227024945.151198-1-zhangdandan%40uniontech.com
patch subject: [PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write
config: x86_64-randconfig-014-20260227 (https://download.01.org/0day-ci/archive/20260227/202602271850.wkrqotlH-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260227/202602271850.wkrqotlH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602271850.wkrqotlH-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "ec_read" [drivers/hwmon/gpd-fan.ko] undefined!
>> ERROR: modpost: "ec_write" [drivers/hwmon/gpd-fan.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write

Posted by kernel test robot 1 month, 1 week ago

Hi Morduan,

kernel test robot noticed the following build errors:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on linus/master v7.0-rc1 next-20260226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Morduan-Zang/hwmon-gpd-fan-replace-custom-EC-I-O-with-kernel-ec_read-ec_write/20260227-105255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/53C09CAECC90EB98%2B20260227024945.151198-1-zhangdandan%40uniontech.com
patch subject: [PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write
config: x86_64-buildonly-randconfig-004-20260227 (https://download.01.org/0day-ci/archive/20260227/202602271410.w0cX8IID-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260227/202602271410.w0cX8IID-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602271410.w0cX8IID-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/hwmon/gpd-fan.c: In function 'gpd_ecram_read':
>> drivers/hwmon/gpd-fan.c:255:16: error: implicit declaration of function 'ec_read'; did you mean 'up_read'? [-Wimplicit-function-declaration]
     255 |         *val = ec_read(offset, val);
         |                ^~~~~~~
         |                up_read
   drivers/hwmon/gpd-fan.c: In function 'gpd_ecram_write':
>> drivers/hwmon/gpd-fan.c:260:9: error: implicit declaration of function 'ec_write'; did you mean 'up_write'? [-Wimplicit-function-declaration]
     260 |         ec_write(offset, value);
         |         ^~~~~~~~
         |         up_write


vim +255 drivers/hwmon/gpd-fan.c

   251	
   252	// Helper functions to handle EC read/write
   253	static void gpd_ecram_read(u16 offset, u8 *val)
   254	{
 > 255		*val = ec_read(offset, val);
   256	}
   257	
   258	static void gpd_ecram_write(u16 offset, u8 value)
   259	{
 > 260		ec_write(offset, value);
   261	}
   262	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write

Posted by Guenter Roeck 1 month, 1 week ago

On 2/26/26 18:49, Morduan Zang wrote:
> Replace the custom gpd_ecram_read() and gpd_ecram_write() functions that
> use direct I/O port access (inb/outb) with the kernel's standard ec_read()
> and ec_write() functions. This provides better abstraction, improves code
> maintainability, and ensures compatibility across different kernel
> versions.
> 

It also adds dependency on ACPI. On top of that, it is quite obviously
buggy (see below).

> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> ---
>   drivers/hwmon/gpd-fan.c | 37 +++----------------------------------
>   1 file changed, 3 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
> index 1729729b135f..c44957b7fd91 100644
> --- a/drivers/hwmon/gpd-fan.c
> +++ b/drivers/hwmon/gpd-fan.c
> @@ -19,6 +19,7 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> +#include <linux/acpi.h>
>   
>   #define DRIVER_NAME "gpdfan"
>   #define GPD_PWM_CTR_OFFSET 0x1841
> @@ -243,44 +244,12 @@ static const struct gpd_fan_drvdata *gpd_module_drvdata[] = {
>   // Helper functions to handle EC read/write
>   static void gpd_ecram_read(u16 offset, u8 *val)
>   {
> -	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
> -	u16 data_port = gpd_driver_priv.drvdata->data_port;
> -
> -	outb(0x2E, addr_port);
> -	outb(0x11, data_port);
> -	outb(0x2F, addr_port);
> -	outb((u8)((offset >> 8) & 0xFF), data_port);
> -
> -	outb(0x2E, addr_port);
> -	outb(0x10, data_port);
> -	outb(0x2F, addr_port);
> -	outb((u8)(offset & 0xFF), data_port);
> -
> -	outb(0x2E, addr_port);
> -	outb(0x12, data_port);
> -	outb(0x2F, addr_port);
> -	*val = inb(data_port);
> +	*val = ec_read(offset, val);

ec_read() writes the return value into val, and returns an error code.
So this now returns the lower 8 bit of the error code as "value".

... which of course means that this code was not tested, and who knows
what the ec_read() and ec_write() functions actually do.

This code is so bad that I won't trust any subsequent versions. NACK.

Guenter

Re: [PATCH] hwmon: gpd-fan: replace custom EC I/O with kernel ec_read/ec_write

Posted by Cryolitia PukNgae 1 month, 1 week ago

Morduan Zang <zhangdandan@uniontech.com> 于2026年2月27日周五 10:50写道：
>
> Replace the custom gpd_ecram_read() and gpd_ecram_write() functions that
> use direct I/O port access (inb/outb) with the kernel's standard ec_read()
> and ec_write() functions. This provides better abstraction, improves code
> maintainability, and ensures compatibility across different kernel
> versions.
>
> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> ---
>  drivers/hwmon/gpd-fan.c | 37 +++----------------------------------
>  1 file changed, 3 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
> index 1729729b135f..c44957b7fd91 100644
> --- a/drivers/hwmon/gpd-fan.c
> +++ b/drivers/hwmon/gpd-fan.c
> @@ -19,6 +19,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/acpi.h>
>
>  #define DRIVER_NAME "gpdfan"
>  #define GPD_PWM_CTR_OFFSET 0x1841
> @@ -243,44 +244,12 @@ static const struct gpd_fan_drvdata *gpd_module_drvdata[] = {
>  // Helper functions to handle EC read/write
>  static void gpd_ecram_read(u16 offset, u8 *val)
>  {
> -       u16 addr_port = gpd_driver_priv.drvdata->addr_port;
> -       u16 data_port = gpd_driver_priv.drvdata->data_port;
> -
> -       outb(0x2E, addr_port);
> -       outb(0x11, data_port);
> -       outb(0x2F, addr_port);
> -       outb((u8)((offset >> 8) & 0xFF), data_port);
> -
> -       outb(0x2E, addr_port);
> -       outb(0x10, data_port);
> -       outb(0x2F, addr_port);
> -       outb((u8)(offset & 0xFF), data_port);
> -
> -       outb(0x2E, addr_port);
> -       outb(0x12, data_port);
> -       outb(0x2F, addr_port);
> -       *val = inb(data_port);
> +       *val = ec_read(offset, val);
>  }
>
>  static void gpd_ecram_write(u16 offset, u8 value)
>  {
> -       u16 addr_port = gpd_driver_priv.drvdata->addr_port;
> -       u16 data_port = gpd_driver_priv.drvdata->data_port;
> -
> -       outb(0x2E, addr_port);
> -       outb(0x11, data_port);
> -       outb(0x2F, addr_port);
> -       outb((u8)((offset >> 8) & 0xFF), data_port);
> -
> -       outb(0x2E, addr_port);
> -       outb(0x10, data_port);
> -       outb(0x2F, addr_port);
> -       outb((u8)(offset & 0xFF), data_port);
> -
> -       outb(0x2E, addr_port);
> -       outb(0x12, data_port);
> -       outb(0x2F, addr_port);
> -       outb(value, data_port);
> +       ec_write(offset, value);
>  }
>
>  static int gpd_generic_read_rpm(void)
> --
> 2.50.1
>

Why do you send it to my gmail, using uniontech.com which is in the
MAINTAINERS file please.
Have you tested it on a GPD device?

thanks,
Cryolitia PukNgae