[PATCH v3 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[PATCH v3 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

replace the global state gpd_driver_priv with per-device private data
(struct gpd_fan_data) allocated in probe. This allows the driver to
support multiple instances in the future and aligns with kernel best
practices.

No functional change intended.
---
changes in v3:
1.Add v3 tag

changes in v2:
1.Platform_create_bundle pass a driver_data pointer
2.gpd_init_ec is needed before hwmon registration, submit as separate
bug fix.
---
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/hwmon/gpd-fan.c | 209 ++++++++++++++++++++++------------------
 1 file changed, 115 insertions(+), 94 deletions(-)

diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
index 80de5f20781e..7284babd4f5c 100644
--- a/drivers/hwmon/gpd-fan.c
+++ b/drivers/hwmon/gpd-fan.c
@@ -40,12 +40,11 @@ enum FAN_PWM_ENABLE {
 	AUTOMATIC	= 2,
 };
 
-static struct {
+struct gpd_fan_data {
 	enum FAN_PWM_ENABLE pwm_enable;
 	u8 pwm_value;
-
 	const struct gpd_fan_drvdata *drvdata;
-} gpd_driver_priv;
+};
 
 struct gpd_fan_drvdata {
 	const char *board_name; // Board name for module param comparison
@@ -249,10 +248,10 @@ 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)
+static void gpd_ecram_read(const struct gpd_fan_drvdata *drvdata, u16 offset, u8 *val)
 {
-	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
-	u16 data_port = gpd_driver_priv.drvdata->data_port;
+	u16 addr_port = drvdata->addr_port;
+	u16 data_port = drvdata->data_port;
 
 	outb(0x2E, addr_port);
 	outb(0x11, data_port);
@@ -270,10 +269,10 @@ static void gpd_ecram_read(u16 offset, u8 *val)
 	*val = inb(data_port);
 }
 
-static void gpd_ecram_write(u16 offset, u8 value)
+static void gpd_ecram_write(const struct gpd_fan_drvdata *drvdata, u16 offset, u8 value)
 {
-	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
-	u16 data_port = gpd_driver_priv.drvdata->data_port;
+	u16 addr_port = drvdata->addr_port;
+	u16 data_port = drvdata->data_port;
 
 	outb(0x2E, addr_port);
 	outb(0x11, data_port);
@@ -291,198 +290,198 @@ static void gpd_ecram_write(u16 offset, u8 value)
 	outb(value, data_port);
 }
 
-static int gpd_generic_read_rpm(void)
+static int gpd_generic_read_rpm(struct gpd_fan_data *data)
 {
-	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
 	u8 high, low;
 
-	gpd_ecram_read(drvdata->rpm_read, &high);
-	gpd_ecram_read(drvdata->rpm_read + 1, &low);
+	gpd_ecram_read(drvdata, drvdata->rpm_read, &high);
+	gpd_ecram_read(drvdata, drvdata->rpm_read + 1, &low);
 
 	return (u16)high << 8 | low;
 }
 
-static int gpd_wm2_read_rpm(void)
+static int gpd_wm2_read_rpm(struct gpd_fan_data *data)
 {
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
+
 	for (u16 pwm_ctr_offset = GPD_PWM_CTR_OFFSET;
 	     pwm_ctr_offset <= GPD_PWM_CTR_OFFSET + 2; pwm_ctr_offset++) {
 		u8 PWMCTR;
 
-		gpd_ecram_read(pwm_ctr_offset, &PWMCTR);
+		gpd_ecram_read(drvdata, pwm_ctr_offset, &PWMCTR);
 
 		if (PWMCTR != 0xB8)
-			gpd_ecram_write(pwm_ctr_offset, 0xB8);
+			gpd_ecram_write(drvdata, pwm_ctr_offset, 0xB8);
 	}
 
-	return gpd_generic_read_rpm();
+	return gpd_generic_read_rpm(data);
 }
 
 // Read value for fan1_input
-static int gpd_read_rpm(void)
+static int gpd_read_rpm(struct gpd_fan_data *data)
 {
-	switch (gpd_driver_priv.drvdata->board) {
+	switch (data->drvdata->board) {
 	case win4_6800u:
 	case win_mini:
 	case duo:
 	case mpc2:
-		return gpd_generic_read_rpm();
+		return gpd_generic_read_rpm(data);
 	case win_max_2:
-		return gpd_wm2_read_rpm();
+		return gpd_wm2_read_rpm(data);
 	}
 
 	return 0;
 }
 
-static int gpd_wm2_read_pwm(void)
+static int gpd_wm2_read_pwm(struct gpd_fan_data *data)
 {
-	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
 	u8 var;
 
-	gpd_ecram_read(drvdata->pwm_write, &var);
+	gpd_ecram_read(drvdata, drvdata->pwm_write, &var);
 
 	// Match gpd_generic_write_pwm(u8) below
 	return DIV_ROUND_CLOSEST((var - 1) * 255, (drvdata->pwm_max - 1));
 }
 
 // Read value for pwm1
-static int gpd_read_pwm(void)
+static int gpd_read_pwm(struct gpd_fan_data *data)
 {
-	switch (gpd_driver_priv.drvdata->board) {
+	switch (data->drvdata->board) {
 	case win_mini:
 	case duo:
 	case win4_6800u:
 	case mpc2:
-		switch (gpd_driver_priv.pwm_enable) {
+		switch (data->pwm_enable) {
 		case DISABLE:
 			return 255;
 		case MANUAL:
-			return gpd_driver_priv.pwm_value;
+			return data->pwm_value;
 		case AUTOMATIC:
 			return -EOPNOTSUPP;
 		}
 		break;
 	case win_max_2:
-		return gpd_wm2_read_pwm();
+		return gpd_wm2_read_pwm(data);
 	}
 	return 0;
 }
 
 // PWM value's range in EC is 1 - pwm_max, cast 0 - 255 to it.
-static inline u8 gpd_cast_pwm_range(u8 val)
+static inline u8 gpd_cast_pwm_range(const struct gpd_fan_drvdata *drvdata, u8 val)
 {
-	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
-
 	return DIV_ROUND_CLOSEST(val * (drvdata->pwm_max - 1), 255) + 1;
 }
 
-static void gpd_generic_write_pwm(u8 val)
+static void gpd_generic_write_pwm(struct gpd_fan_data *data, u8 val)
 {
-	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
 	u8 pwm_reg;
 
-	pwm_reg = gpd_cast_pwm_range(val);
-	gpd_ecram_write(drvdata->pwm_write, pwm_reg);
+	pwm_reg = gpd_cast_pwm_range(drvdata, val);
+	gpd_ecram_write(drvdata, drvdata->pwm_write, pwm_reg);
 }
 
-static void gpd_duo_write_pwm(u8 val)
+static void gpd_duo_write_pwm(struct gpd_fan_data *data, u8 val)
 {
-	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
 	u8 pwm_reg;
 
-	pwm_reg = gpd_cast_pwm_range(val);
-	gpd_ecram_write(drvdata->pwm_write, pwm_reg);
-	gpd_ecram_write(drvdata->pwm_write + 1, pwm_reg);
+	pwm_reg = gpd_cast_pwm_range(drvdata, val);
+	gpd_ecram_write(drvdata, drvdata->pwm_write, pwm_reg);
+	gpd_ecram_write(drvdata, drvdata->pwm_write + 1, pwm_reg);
 }
 
 // Write value for pwm1
-static int gpd_write_pwm(u8 val)
+static int gpd_write_pwm(struct gpd_fan_data *data, u8 val)
 {
-	if (gpd_driver_priv.pwm_enable != MANUAL)
+	if (data->pwm_enable != MANUAL)
 		return -EPERM;
 
-	switch (gpd_driver_priv.drvdata->board) {
+	switch (data->drvdata->board) {
 	case duo:
-		gpd_duo_write_pwm(val);
+		gpd_duo_write_pwm(data, val);
 		break;
 	case win_mini:
 	case win4_6800u:
 	case win_max_2:
 	case mpc2:
-		gpd_generic_write_pwm(val);
+		gpd_generic_write_pwm(data, val);
 		break;
 	}
 
 	return 0;
 }
 
-static void gpd_win_mini_set_pwm_enable(enum FAN_PWM_ENABLE pwm_enable)
+static void gpd_win_mini_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE pwm_enable)
 {
 	switch (pwm_enable) {
 	case DISABLE:
-		gpd_generic_write_pwm(255);
+		gpd_generic_write_pwm(data, 255);
 		break;
 	case MANUAL:
-		gpd_generic_write_pwm(gpd_driver_priv.pwm_value);
+		gpd_generic_write_pwm(data, data->pwm_value);
 		break;
 	case AUTOMATIC:
-		gpd_ecram_write(gpd_driver_priv.drvdata->pwm_write, 0);
+		gpd_ecram_write(data->drvdata, data->drvdata->pwm_write, 0);
 		break;
 	}
 }
 
-static void gpd_duo_set_pwm_enable(enum FAN_PWM_ENABLE pwm_enable)
+static void gpd_duo_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE pwm_enable)
 {
 	switch (pwm_enable) {
 	case DISABLE:
-		gpd_duo_write_pwm(255);
+		gpd_duo_write_pwm(data, 255);
 		break;
 	case MANUAL:
-		gpd_duo_write_pwm(gpd_driver_priv.pwm_value);
+		gpd_duo_write_pwm(data, data->pwm_value);
 		break;
 	case AUTOMATIC:
-		gpd_ecram_write(gpd_driver_priv.drvdata->pwm_write, 0);
+		gpd_ecram_write(data->drvdata, data->drvdata->pwm_write, 0);
 		break;
 	}
 }
 
-static void gpd_wm2_set_pwm_enable(enum FAN_PWM_ENABLE enable)
+static void gpd_wm2_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE enable)
 {
-	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
 
 	switch (enable) {
 	case DISABLE:
-		gpd_generic_write_pwm(255);
-		gpd_ecram_write(drvdata->manual_control_enable, 1);
+		gpd_generic_write_pwm(data, 255);
+		gpd_ecram_write(drvdata, drvdata->manual_control_enable, 1);
 		break;
 	case MANUAL:
-		gpd_generic_write_pwm(gpd_driver_priv.pwm_value);
-		gpd_ecram_write(drvdata->manual_control_enable, 1);
+		gpd_generic_write_pwm(data, data->pwm_value);
+		gpd_ecram_write(drvdata, drvdata->manual_control_enable, 1);
 		break;
 	case AUTOMATIC:
-		gpd_ecram_write(drvdata->manual_control_enable, 0);
+		gpd_ecram_write(drvdata, drvdata->manual_control_enable, 0);
 		break;
 	}
 }
 
 // Write value for pwm1_enable
-static void gpd_set_pwm_enable(enum FAN_PWM_ENABLE enable)
+static void gpd_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE enable)
 {
 	if (enable == MANUAL)
 		// Set pwm_value to max firstly when switching to manual mode, in
 		// consideration of device safety.
-		gpd_driver_priv.pwm_value = 255;
+		data->pwm_value = 255;
 
-	switch (gpd_driver_priv.drvdata->board) {
+	switch (data->drvdata->board) {
 	case win_mini:
 	case win4_6800u:
 	case mpc2:
-		gpd_win_mini_set_pwm_enable(enable);
+		gpd_win_mini_set_pwm_enable(data, enable);
 		break;
 	case duo:
-		gpd_duo_set_pwm_enable(enable);
+		gpd_duo_set_pwm_enable(data, enable);
 		break;
 	case win_max_2:
-		gpd_wm2_set_pwm_enable(enable);
+		gpd_wm2_set_pwm_enable(data, enable);
 		break;
 	}
 }
@@ -505,15 +504,16 @@ static umode_t gpd_fan_hwmon_is_visible(__always_unused const void *drvdata,
 	return 0;
 }
 
-static int gpd_fan_hwmon_read(__always_unused struct device *dev,
+static int gpd_fan_hwmon_read(struct device *dev,
 			      enum hwmon_sensor_types type, u32 attr,
 			      __always_unused int channel, long *val)
 {
+	struct gpd_fan_data *data = dev_get_drvdata(dev);
 	int ret;
 
 	if (type == hwmon_fan) {
 		if (attr == hwmon_fan_input) {
-			ret = gpd_read_rpm();
+			ret = gpd_read_rpm(data);
 
 			if (ret < 0)
 				return ret;
@@ -524,10 +524,10 @@ static int gpd_fan_hwmon_read(__always_unused struct device *dev,
 	} else if (type == hwmon_pwm) {
 		switch (attr) {
 		case hwmon_pwm_enable:
-			*val = gpd_driver_priv.pwm_enable;
+			*val = data->pwm_enable;
 			return 0;
 		case hwmon_pwm_input:
-			ret = gpd_read_pwm();
+			ret = gpd_read_pwm(data);
 
 			if (ret < 0)
 				return ret;
@@ -540,27 +540,29 @@ static int gpd_fan_hwmon_read(__always_unused struct device *dev,
 	return -EOPNOTSUPP;
 }
 
-static int gpd_fan_hwmon_write(__always_unused struct device *dev,
+static int gpd_fan_hwmon_write(struct device *dev,
 			       enum hwmon_sensor_types type, u32 attr,
 			       __always_unused int channel, long val)
 {
+	struct gpd_fan_data *data = dev_get_drvdata(dev);
+
 	if (type == hwmon_pwm) {
 		switch (attr) {
 		case hwmon_pwm_enable:
 			if (!in_range(val, 0, 3))
 				return -EINVAL;
 
-			gpd_driver_priv.pwm_enable = val;
+			data->pwm_enable = val;
 
-			gpd_set_pwm_enable(gpd_driver_priv.pwm_enable);
+			gpd_set_pwm_enable(data, data->pwm_enable);
 			return 0;
 		case hwmon_pwm_input:
 			if (!in_range(val, 0, 256))
 				return -EINVAL;
 
-			gpd_driver_priv.pwm_value = val;
+			data->pwm_value = val;
 
-			return gpd_write_pwm(val);
+			return gpd_write_pwm(data, val);
 		}
 	}
 
@@ -584,26 +586,27 @@ static struct hwmon_chip_info gpd_fan_chip_info = {
 	.info = gpd_fan_hwmon_channel_info
 };
 
-static void gpd_win4_init_ec(void)
+static void gpd_win4_init_ec(struct gpd_fan_data *data)
 {
+	const struct gpd_fan_drvdata *drvdata = data->drvdata;
 	u8 chip_id, chip_ver;
 
-	gpd_ecram_read(0x2000, &chip_id);
+	gpd_ecram_read(drvdata, 0x2000, &chip_id);
 
 	if (chip_id == 0x55) {
-		gpd_ecram_read(0x1060, &chip_ver);
-		gpd_ecram_write(0x1060, chip_ver | 0x80);
+		gpd_ecram_read(drvdata, 0x1060, &chip_ver);
+		gpd_ecram_write(drvdata, 0x1060, chip_ver | 0x80);
 	}
 }
 
-static void gpd_init_ec(void)
+static void gpd_init_ec(struct gpd_fan_data *data)
 {
 	// The buggy firmware won't initialize EC properly on boot.
 	// Before its initialization, reading RPM will always return 0,
 	// and writing PWM will have no effect.
 	// Initialize it manually on driver load.
-	if (gpd_driver_priv.drvdata->board == win4_6800u)
-		gpd_win4_init_ec();
+	if (data->drvdata->board == win4_6800u)
+		gpd_win4_init_ec(data);
 }
 
 static int gpd_fan_probe(struct platform_device *pdev)
@@ -611,7 +614,9 @@ static int gpd_fan_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const struct resource *region;
 	const struct resource *res;
-	const struct device *hwdev;
+	struct device *hwdev;
+	struct gpd_fan_data *data;
+	const struct gpd_fan_drvdata *match;
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
 	if (!res)
@@ -624,24 +629,42 @@ static int gpd_fan_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, -EBUSY,
 				     "Failed to request region\n");
 
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	match = dev_get_platdata(dev);
+	if (!match)
+		return -EINVAL;
+
+	data->drvdata = match;
+	data->pwm_enable = AUTOMATIC;
+	data->pwm_value = 255;
+
+	dev_set_drvdata(dev, data);
+
 	hwdev = devm_hwmon_device_register_with_info(dev,
 						     DRIVER_NAME,
-						     NULL,
+						     data,
 						     &gpd_fan_chip_info,
 						     NULL);
 	if (IS_ERR(hwdev))
 		return dev_err_probe(dev, PTR_ERR(hwdev),
 				     "Failed to register hwmon device\n");
 
-	gpd_init_ec();
+	gpd_init_ec(data);
 
 	return 0;
 }
 
-static void gpd_fan_remove(__always_unused struct platform_device *pdev)
+static void gpd_fan_remove(struct platform_device *pdev)
 {
-	gpd_driver_priv.pwm_enable = AUTOMATIC;
-	gpd_set_pwm_enable(AUTOMATIC);
+	struct gpd_fan_data *data = dev_get_drvdata(&pdev->dev);
+
+	if (data) {
+		data->pwm_enable = AUTOMATIC;
+		gpd_set_pwm_enable(data, AUTOMATIC);
+	}
 }
 
 static struct platform_driver gpd_fan_driver = {
@@ -668,6 +691,7 @@ static int __init gpd_fan_init(void)
 	if (!match) {
 		const struct dmi_system_id *dmi_match =
 			dmi_first_match(dmi_table);
+
 		if (dmi_match)
 			match = dmi_match->driver_data;
 	}
@@ -675,10 +699,6 @@ static int __init gpd_fan_init(void)
 	if (!match)
 		return -ENODEV;
 
-	gpd_driver_priv.pwm_enable = AUTOMATIC;
-	gpd_driver_priv.pwm_value = 255;
-	gpd_driver_priv.drvdata = match;
-
 	struct resource gpd_fan_resources[] = {
 		{
 			.start = match->addr_port,
@@ -690,7 +710,8 @@ static int __init gpd_fan_init(void)
 	gpd_fan_platform_device = platform_create_bundle(&gpd_fan_driver,
 							 gpd_fan_probe,
 							 gpd_fan_resources,
-							 1, NULL, 0);
+							 1,
+							 match, sizeof(*match));
 
 	if (IS_ERR(gpd_fan_platform_device)) {
 		pr_warn("Failed to create platform device\n");
-- 
2.25.1

Re: [PATCH v3 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[Guenter Roeck]

On 6/7/26 18:40, xiaopeitux@foxmail.com wrote:
> From: Pei Xiao <xiaopei01@kylinos.cn>
> 
> replace the global state gpd_driver_priv with per-device private data
> (struct gpd_fan_data) allocated in probe. This allows the driver to
> support multiple instances in the future and aligns with kernel best
> practices.
> 
> No functional change intended.
> ---

I got:

Commit

   beb0e2d1add1e2 ("hwmon: (gpd-fan): drop global driver data and use per-device allocation")

is missing a Signed-off-by from its author

Since it is the first patch of the series I had to drop the entire series.
Please resend with proper tags.

Guenter

Re: [PATCH v3 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[Pei Xiao]

在 2026/6/9 21:04, Guenter Roeck 写道:
> On 6/7/26 18:40, xiaopeitux@foxmail.com wrote:
>> From: Pei Xiao <xiaopei01@kylinos.cn>
>>
>> replace the global state gpd_driver_priv with per-device private data
>> (struct gpd_fan_data) allocated in probe. This allows the driver to
>> support multiple instances in the future and aligns with kernel best
>> practices.
>>
>> No functional change intended.
>> ---
>
> I got:
>
> Commit
>
>   beb0e2d1add1e2 ("hwmon: (gpd-fan): drop global driver data and use
> per-device allocation")
>
> is missing a Signed-off-by from its author 
>
hi Guenter,
    Sorry for wasting your time.
    I made a mistake. I thought the content between the
first |---| delimiter and the second delimiter 
was invisible to |git am|, and the content between the second and third |


---| delimiters was visible, 
so I lost the |sob|. I will resend version 4 with my |sob|.

Pei 

> Since it is the first patch of the series I had to drop the entire
> series.
> Please resend with proper tags.
>
> Guenter

Re: [PATCH v3 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[Cryolitia PukNgae]

On 6/8/26 01:40, xiaopeitux@foxmail.com wrote:

> From: Pei Xiao <xiaopei01@kylinos.cn>
>
> replace the global state gpd_driver_priv with per-device private data
> (struct gpd_fan_data) allocated in probe. This allows the driver to
> support multiple instances in the future and aligns with kernel best
> practices.
>
> No functional change intended.
> ---
> changes in v3:
> 1.Add v3 tag
>
> changes in v2:
> 1.Platform_create_bundle pass a driver_data pointer
> 2.gpd_init_ec is needed before hwmon registration, submit as separate
> bug fix.
> ---
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
>   drivers/hwmon/gpd-fan.c | 209 ++++++++++++++++++++++------------------
>   1 file changed, 115 insertions(+), 94 deletions(-)
>
> diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
> index 80de5f20781e..7284babd4f5c 100644
> --- a/drivers/hwmon/gpd-fan.c
> +++ b/drivers/hwmon/gpd-fan.c
> @@ -40,12 +40,11 @@ enum FAN_PWM_ENABLE {
>   	AUTOMATIC	= 2,
>   };
>   
> -static struct {
> +struct gpd_fan_data {
>   	enum FAN_PWM_ENABLE pwm_enable;
>   	u8 pwm_value;
> -
>   	const struct gpd_fan_drvdata *drvdata;
> -} gpd_driver_priv;
> +};
>   
>   struct gpd_fan_drvdata {
>   	const char *board_name; // Board name for module param comparison
> @@ -249,10 +248,10 @@ 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)
> +static void gpd_ecram_read(const struct gpd_fan_drvdata *drvdata, u16 offset, u8 *val)
>   {
> -	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
> -	u16 data_port = gpd_driver_priv.drvdata->data_port;
> +	u16 addr_port = drvdata->addr_port;
> +	u16 data_port = drvdata->data_port;
>   
>   	outb(0x2E, addr_port);
>   	outb(0x11, data_port);
> @@ -270,10 +269,10 @@ static void gpd_ecram_read(u16 offset, u8 *val)
>   	*val = inb(data_port);
>   }
>   
> -static void gpd_ecram_write(u16 offset, u8 value)
> +static void gpd_ecram_write(const struct gpd_fan_drvdata *drvdata, u16 offset, u8 value)
>   {
> -	u16 addr_port = gpd_driver_priv.drvdata->addr_port;
> -	u16 data_port = gpd_driver_priv.drvdata->data_port;
> +	u16 addr_port = drvdata->addr_port;
> +	u16 data_port = drvdata->data_port;
>   
>   	outb(0x2E, addr_port);
>   	outb(0x11, data_port);
> @@ -291,198 +290,198 @@ static void gpd_ecram_write(u16 offset, u8 value)
>   	outb(value, data_port);
>   }
>   
> -static int gpd_generic_read_rpm(void)
> +static int gpd_generic_read_rpm(struct gpd_fan_data *data)
>   {
> -	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
>   	u8 high, low;
>   
> -	gpd_ecram_read(drvdata->rpm_read, &high);
> -	gpd_ecram_read(drvdata->rpm_read + 1, &low);
> +	gpd_ecram_read(drvdata, drvdata->rpm_read, &high);
> +	gpd_ecram_read(drvdata, drvdata->rpm_read + 1, &low);
>   
>   	return (u16)high << 8 | low;
>   }
>   
> -static int gpd_wm2_read_rpm(void)
> +static int gpd_wm2_read_rpm(struct gpd_fan_data *data)
>   {
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
> +
>   	for (u16 pwm_ctr_offset = GPD_PWM_CTR_OFFSET;
>   	     pwm_ctr_offset <= GPD_PWM_CTR_OFFSET + 2; pwm_ctr_offset++) {
>   		u8 PWMCTR;
>   
> -		gpd_ecram_read(pwm_ctr_offset, &PWMCTR);
> +		gpd_ecram_read(drvdata, pwm_ctr_offset, &PWMCTR);
>   
>   		if (PWMCTR != 0xB8)
> -			gpd_ecram_write(pwm_ctr_offset, 0xB8);
> +			gpd_ecram_write(drvdata, pwm_ctr_offset, 0xB8);
>   	}
>   
> -	return gpd_generic_read_rpm();
> +	return gpd_generic_read_rpm(data);
>   }
>   
>   // Read value for fan1_input
> -static int gpd_read_rpm(void)
> +static int gpd_read_rpm(struct gpd_fan_data *data)
>   {
> -	switch (gpd_driver_priv.drvdata->board) {
> +	switch (data->drvdata->board) {
>   	case win4_6800u:
>   	case win_mini:
>   	case duo:
>   	case mpc2:
> -		return gpd_generic_read_rpm();
> +		return gpd_generic_read_rpm(data);
>   	case win_max_2:
> -		return gpd_wm2_read_rpm();
> +		return gpd_wm2_read_rpm(data);
>   	}
>   
>   	return 0;
>   }
>   
> -static int gpd_wm2_read_pwm(void)
> +static int gpd_wm2_read_pwm(struct gpd_fan_data *data)
>   {
> -	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
>   	u8 var;
>   
> -	gpd_ecram_read(drvdata->pwm_write, &var);
> +	gpd_ecram_read(drvdata, drvdata->pwm_write, &var);
>   
>   	// Match gpd_generic_write_pwm(u8) below
>   	return DIV_ROUND_CLOSEST((var - 1) * 255, (drvdata->pwm_max - 1));
>   }
>   
>   // Read value for pwm1
> -static int gpd_read_pwm(void)
> +static int gpd_read_pwm(struct gpd_fan_data *data)
>   {
> -	switch (gpd_driver_priv.drvdata->board) {
> +	switch (data->drvdata->board) {
>   	case win_mini:
>   	case duo:
>   	case win4_6800u:
>   	case mpc2:
> -		switch (gpd_driver_priv.pwm_enable) {
> +		switch (data->pwm_enable) {
>   		case DISABLE:
>   			return 255;
>   		case MANUAL:
> -			return gpd_driver_priv.pwm_value;
> +			return data->pwm_value;
>   		case AUTOMATIC:
>   			return -EOPNOTSUPP;
>   		}
>   		break;
>   	case win_max_2:
> -		return gpd_wm2_read_pwm();
> +		return gpd_wm2_read_pwm(data);
>   	}
>   	return 0;
>   }
>   
>   // PWM value's range in EC is 1 - pwm_max, cast 0 - 255 to it.
> -static inline u8 gpd_cast_pwm_range(u8 val)
> +static inline u8 gpd_cast_pwm_range(const struct gpd_fan_drvdata *drvdata, u8 val)
>   {
> -	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
> -
>   	return DIV_ROUND_CLOSEST(val * (drvdata->pwm_max - 1), 255) + 1;
>   }
>   
> -static void gpd_generic_write_pwm(u8 val)
> +static void gpd_generic_write_pwm(struct gpd_fan_data *data, u8 val)
>   {
> -	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
>   	u8 pwm_reg;
>   
> -	pwm_reg = gpd_cast_pwm_range(val);
> -	gpd_ecram_write(drvdata->pwm_write, pwm_reg);
> +	pwm_reg = gpd_cast_pwm_range(drvdata, val);
> +	gpd_ecram_write(drvdata, drvdata->pwm_write, pwm_reg);
>   }
>   
> -static void gpd_duo_write_pwm(u8 val)
> +static void gpd_duo_write_pwm(struct gpd_fan_data *data, u8 val)
>   {
> -	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
>   	u8 pwm_reg;
>   
> -	pwm_reg = gpd_cast_pwm_range(val);
> -	gpd_ecram_write(drvdata->pwm_write, pwm_reg);
> -	gpd_ecram_write(drvdata->pwm_write + 1, pwm_reg);
> +	pwm_reg = gpd_cast_pwm_range(drvdata, val);
> +	gpd_ecram_write(drvdata, drvdata->pwm_write, pwm_reg);
> +	gpd_ecram_write(drvdata, drvdata->pwm_write + 1, pwm_reg);
>   }
>   
>   // Write value for pwm1
> -static int gpd_write_pwm(u8 val)
> +static int gpd_write_pwm(struct gpd_fan_data *data, u8 val)
>   {
> -	if (gpd_driver_priv.pwm_enable != MANUAL)
> +	if (data->pwm_enable != MANUAL)
>   		return -EPERM;
>   
> -	switch (gpd_driver_priv.drvdata->board) {
> +	switch (data->drvdata->board) {
>   	case duo:
> -		gpd_duo_write_pwm(val);
> +		gpd_duo_write_pwm(data, val);
>   		break;
>   	case win_mini:
>   	case win4_6800u:
>   	case win_max_2:
>   	case mpc2:
> -		gpd_generic_write_pwm(val);
> +		gpd_generic_write_pwm(data, val);
>   		break;
>   	}
>   
>   	return 0;
>   }
>   
> -static void gpd_win_mini_set_pwm_enable(enum FAN_PWM_ENABLE pwm_enable)
> +static void gpd_win_mini_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE pwm_enable)
>   {
>   	switch (pwm_enable) {
>   	case DISABLE:
> -		gpd_generic_write_pwm(255);
> +		gpd_generic_write_pwm(data, 255);
>   		break;
>   	case MANUAL:
> -		gpd_generic_write_pwm(gpd_driver_priv.pwm_value);
> +		gpd_generic_write_pwm(data, data->pwm_value);
>   		break;
>   	case AUTOMATIC:
> -		gpd_ecram_write(gpd_driver_priv.drvdata->pwm_write, 0);
> +		gpd_ecram_write(data->drvdata, data->drvdata->pwm_write, 0);
>   		break;
>   	}
>   }
>   
> -static void gpd_duo_set_pwm_enable(enum FAN_PWM_ENABLE pwm_enable)
> +static void gpd_duo_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE pwm_enable)
>   {
>   	switch (pwm_enable) {
>   	case DISABLE:
> -		gpd_duo_write_pwm(255);
> +		gpd_duo_write_pwm(data, 255);
>   		break;
>   	case MANUAL:
> -		gpd_duo_write_pwm(gpd_driver_priv.pwm_value);
> +		gpd_duo_write_pwm(data, data->pwm_value);
>   		break;
>   	case AUTOMATIC:
> -		gpd_ecram_write(gpd_driver_priv.drvdata->pwm_write, 0);
> +		gpd_ecram_write(data->drvdata, data->drvdata->pwm_write, 0);
>   		break;
>   	}
>   }
>   
> -static void gpd_wm2_set_pwm_enable(enum FAN_PWM_ENABLE enable)
> +static void gpd_wm2_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE enable)
>   {
> -	const struct gpd_fan_drvdata *const drvdata = gpd_driver_priv.drvdata;
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
>   
>   	switch (enable) {
>   	case DISABLE:
> -		gpd_generic_write_pwm(255);
> -		gpd_ecram_write(drvdata->manual_control_enable, 1);
> +		gpd_generic_write_pwm(data, 255);
> +		gpd_ecram_write(drvdata, drvdata->manual_control_enable, 1);
>   		break;
>   	case MANUAL:
> -		gpd_generic_write_pwm(gpd_driver_priv.pwm_value);
> -		gpd_ecram_write(drvdata->manual_control_enable, 1);
> +		gpd_generic_write_pwm(data, data->pwm_value);
> +		gpd_ecram_write(drvdata, drvdata->manual_control_enable, 1);
>   		break;
>   	case AUTOMATIC:
> -		gpd_ecram_write(drvdata->manual_control_enable, 0);
> +		gpd_ecram_write(drvdata, drvdata->manual_control_enable, 0);
>   		break;
>   	}
>   }
>   
>   // Write value for pwm1_enable
> -static void gpd_set_pwm_enable(enum FAN_PWM_ENABLE enable)
> +static void gpd_set_pwm_enable(struct gpd_fan_data *data, enum FAN_PWM_ENABLE enable)
>   {
>   	if (enable == MANUAL)
>   		// Set pwm_value to max firstly when switching to manual mode, in
>   		// consideration of device safety.
> -		gpd_driver_priv.pwm_value = 255;
> +		data->pwm_value = 255;
>   
> -	switch (gpd_driver_priv.drvdata->board) {
> +	switch (data->drvdata->board) {
>   	case win_mini:
>   	case win4_6800u:
>   	case mpc2:
> -		gpd_win_mini_set_pwm_enable(enable);
> +		gpd_win_mini_set_pwm_enable(data, enable);
>   		break;
>   	case duo:
> -		gpd_duo_set_pwm_enable(enable);
> +		gpd_duo_set_pwm_enable(data, enable);
>   		break;
>   	case win_max_2:
> -		gpd_wm2_set_pwm_enable(enable);
> +		gpd_wm2_set_pwm_enable(data, enable);
>   		break;
>   	}
>   }
> @@ -505,15 +504,16 @@ static umode_t gpd_fan_hwmon_is_visible(__always_unused const void *drvdata,
>   	return 0;
>   }
>   
> -static int gpd_fan_hwmon_read(__always_unused struct device *dev,
> +static int gpd_fan_hwmon_read(struct device *dev,
>   			      enum hwmon_sensor_types type, u32 attr,
>   			      __always_unused int channel, long *val)
>   {
> +	struct gpd_fan_data *data = dev_get_drvdata(dev);
>   	int ret;
>   
>   	if (type == hwmon_fan) {
>   		if (attr == hwmon_fan_input) {
> -			ret = gpd_read_rpm();
> +			ret = gpd_read_rpm(data);
>   
>   			if (ret < 0)
>   				return ret;
> @@ -524,10 +524,10 @@ static int gpd_fan_hwmon_read(__always_unused struct device *dev,
>   	} else if (type == hwmon_pwm) {
>   		switch (attr) {
>   		case hwmon_pwm_enable:
> -			*val = gpd_driver_priv.pwm_enable;
> +			*val = data->pwm_enable;
>   			return 0;
>   		case hwmon_pwm_input:
> -			ret = gpd_read_pwm();
> +			ret = gpd_read_pwm(data);
>   
>   			if (ret < 0)
>   				return ret;
> @@ -540,27 +540,29 @@ static int gpd_fan_hwmon_read(__always_unused struct device *dev,
>   	return -EOPNOTSUPP;
>   }
>   
> -static int gpd_fan_hwmon_write(__always_unused struct device *dev,
> +static int gpd_fan_hwmon_write(struct device *dev,
>   			       enum hwmon_sensor_types type, u32 attr,
>   			       __always_unused int channel, long val)
>   {
> +	struct gpd_fan_data *data = dev_get_drvdata(dev);
> +
>   	if (type == hwmon_pwm) {
>   		switch (attr) {
>   		case hwmon_pwm_enable:
>   			if (!in_range(val, 0, 3))
>   				return -EINVAL;
>   
> -			gpd_driver_priv.pwm_enable = val;
> +			data->pwm_enable = val;
>   
> -			gpd_set_pwm_enable(gpd_driver_priv.pwm_enable);
> +			gpd_set_pwm_enable(data, data->pwm_enable);
>   			return 0;
>   		case hwmon_pwm_input:
>   			if (!in_range(val, 0, 256))
>   				return -EINVAL;
>   
> -			gpd_driver_priv.pwm_value = val;
> +			data->pwm_value = val;
>   
> -			return gpd_write_pwm(val);
> +			return gpd_write_pwm(data, val);
>   		}
>   	}
>   
> @@ -584,26 +586,27 @@ static struct hwmon_chip_info gpd_fan_chip_info = {
>   	.info = gpd_fan_hwmon_channel_info
>   };
>   
> -static void gpd_win4_init_ec(void)
> +static void gpd_win4_init_ec(struct gpd_fan_data *data)
>   {
> +	const struct gpd_fan_drvdata *drvdata = data->drvdata;
>   	u8 chip_id, chip_ver;
>   
> -	gpd_ecram_read(0x2000, &chip_id);
> +	gpd_ecram_read(drvdata, 0x2000, &chip_id);
>   
>   	if (chip_id == 0x55) {
> -		gpd_ecram_read(0x1060, &chip_ver);
> -		gpd_ecram_write(0x1060, chip_ver | 0x80);
> +		gpd_ecram_read(drvdata, 0x1060, &chip_ver);
> +		gpd_ecram_write(drvdata, 0x1060, chip_ver | 0x80);
>   	}
>   }
>   
> -static void gpd_init_ec(void)
> +static void gpd_init_ec(struct gpd_fan_data *data)
>   {
>   	// The buggy firmware won't initialize EC properly on boot.
>   	// Before its initialization, reading RPM will always return 0,
>   	// and writing PWM will have no effect.
>   	// Initialize it manually on driver load.
> -	if (gpd_driver_priv.drvdata->board == win4_6800u)
> -		gpd_win4_init_ec();
> +	if (data->drvdata->board == win4_6800u)
> +		gpd_win4_init_ec(data);
>   }
>   
>   static int gpd_fan_probe(struct platform_device *pdev)
> @@ -611,7 +614,9 @@ static int gpd_fan_probe(struct platform_device *pdev)
>   	struct device *dev = &pdev->dev;
>   	const struct resource *region;
>   	const struct resource *res;
> -	const struct device *hwdev;
> +	struct device *hwdev;
> +	struct gpd_fan_data *data;
> +	const struct gpd_fan_drvdata *match;
>   
>   	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
>   	if (!res)
> @@ -624,24 +629,42 @@ static int gpd_fan_probe(struct platform_device *pdev)
>   		return dev_err_probe(dev, -EBUSY,
>   				     "Failed to request region\n");
>   
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	match = dev_get_platdata(dev);
> +	if (!match)
> +		return -EINVAL;
> +
> +	data->drvdata = match;
> +	data->pwm_enable = AUTOMATIC;
> +	data->pwm_value = 255;
> +
> +	dev_set_drvdata(dev, data);
> +
>   	hwdev = devm_hwmon_device_register_with_info(dev,
>   						     DRIVER_NAME,
> -						     NULL,
> +						     data,
>   						     &gpd_fan_chip_info,
>   						     NULL);
>   	if (IS_ERR(hwdev))
>   		return dev_err_probe(dev, PTR_ERR(hwdev),
>   				     "Failed to register hwmon device\n");
>   
> -	gpd_init_ec();
> +	gpd_init_ec(data);
>   
>   	return 0;
>   }
>   
> -static void gpd_fan_remove(__always_unused struct platform_device *pdev)
> +static void gpd_fan_remove(struct platform_device *pdev)
>   {
> -	gpd_driver_priv.pwm_enable = AUTOMATIC;
> -	gpd_set_pwm_enable(AUTOMATIC);
> +	struct gpd_fan_data *data = dev_get_drvdata(&pdev->dev);
> +
> +	if (data) {
> +		data->pwm_enable = AUTOMATIC;
> +		gpd_set_pwm_enable(data, AUTOMATIC);
> +	}
>   }
>   
>   static struct platform_driver gpd_fan_driver = {
> @@ -668,6 +691,7 @@ static int __init gpd_fan_init(void)
>   	if (!match) {
>   		const struct dmi_system_id *dmi_match =
>   			dmi_first_match(dmi_table);
> +
>   		if (dmi_match)
>   			match = dmi_match->driver_data;
>   	}
> @@ -675,10 +699,6 @@ static int __init gpd_fan_init(void)
>   	if (!match)
>   		return -ENODEV;
>   
> -	gpd_driver_priv.pwm_enable = AUTOMATIC;
> -	gpd_driver_priv.pwm_value = 255;
> -	gpd_driver_priv.drvdata = match;
> -
>   	struct resource gpd_fan_resources[] = {
>   		{
>   			.start = match->addr_port,
> @@ -690,7 +710,8 @@ static int __init gpd_fan_init(void)
>   	gpd_fan_platform_device = platform_create_bundle(&gpd_fan_driver,
>   							 gpd_fan_probe,
>   							 gpd_fan_resources,
> -							 1, NULL, 0);
> +							 1,
> +							 match, sizeof(*match));
>   
>   	if (IS_ERR(gpd_fan_platform_device)) {
>   		pr_warn("Failed to create platform device\n");


I noticed you've been mixing different Latinization methods for your 
name and different email suffixes (foxmail and kylinos) in a short 
period of time. What could be the reason for this? Please keep it 
consistent.


thanks,

Cryolitia PukNgae

Re: [PATCH v3 1/4] hwmon: (gpd-fan): drop global driver data and use per-device allocation

[Pei Xiao]

在 2026/6/9 23:46, Cryolitia PukNgae 写道:
> On 6/8/26 01:40, xiaopeitux@foxmail.com wrote:
>
>> From: Pei Xiao <xiaopei01@kylinos.cn>
>>
>> replace the global state gpd_driver_priv with per-device private data
>> (struct gpd_fan_data) allocated in probe. This allows the driver to
>> support multiple instances in the future and aligns with kernel best
>> practices.
>>
>> No functional change intended.
>> ---
>> changes in v3:
>> 1.Add v3 tag
>>
>> changes in v2:
>> 1.Platform_create_bundle pass a driver_data pointer
>> 2.gpd_init_ec is needed before hwmon registration, submit as separate
>> bug fix.
>> ---
>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>> ---
>>   drivers/hwmon/gpd-fan.c | 209 ++++++++++++++++++++++------------------
>>   1 file changed, 115 insertions(+), 94 deletions(-)
>>
>> diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
>> index 80de5f20781e..7284babd4f5c 100644
>> --- a/drivers/hwmon/gpd-fan.c
>> +++ b/drivers/hwmon/gpd-fan.c
>> @@ -40,12 +40,11 @@ enum FAN_PWM_ENABLE {
>>       AUTOMATIC    = 2,
>>   };
>>   -static struct {
>> +struct gpd_fan_data {
>>       enum FAN_PWM_ENABLE pwm_enable;
>>       u8 pwm_value;
>> -
>>       const struct gpd_fan_drvdata *drvdata;
>> -} gpd_driver_priv;
>> +};
>>     struct gpd_fan_drvdata {
>>       const char *board_name; // Board name for module param comparison
>> @@ -249,10 +248,10 @@ 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)
>> +static void gpd_ecram_read(const struct gpd_fan_drvdata *drvdata,
>> u16 offset, u8 *val)
>>   {
>> -    u16 addr_port = gpd_driver_priv.drvdata->addr_port;
>> -    u16 data_port = gpd_driver_priv.drvdata->data_port;
>> +    u16 addr_port = drvdata->addr_port;
>> +    u16 data_port = drvdata->data_port;
>>         outb(0x2E, addr_port);
>>       outb(0x11, data_port);
>> @@ -270,10 +269,10 @@ static void gpd_ecram_read(u16 offset, u8 *val)
>>       *val = inb(data_port);
>>   }
>>   -static void gpd_ecram_write(u16 offset, u8 value)
>> +static void gpd_ecram_write(const struct gpd_fan_drvdata *drvdata,
>> u16 offset, u8 value)
>>   {
>> -    u16 addr_port = gpd_driver_priv.drvdata->addr_port;
>> -    u16 data_port = gpd_driver_priv.drvdata->data_port;
>> +    u16 addr_port = drvdata->addr_port;
>> +    u16 data_port = drvdata->data_port;
>>         outb(0x2E, addr_port);
>>       outb(0x11, data_port);
>> @@ -291,198 +290,198 @@ static void gpd_ecram_write(u16 offset, u8
>> value)
>>       outb(value, data_port);
>>   }
>>   -static int gpd_generic_read_rpm(void)
>> +static int gpd_generic_read_rpm(struct gpd_fan_data *data)
>>   {
>> -    const struct gpd_fan_drvdata *const drvdata =
>> gpd_driver_priv.drvdata;
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>>       u8 high, low;
>>   -    gpd_ecram_read(drvdata->rpm_read, &high);
>> -    gpd_ecram_read(drvdata->rpm_read + 1, &low);
>> +    gpd_ecram_read(drvdata, drvdata->rpm_read, &high);
>> +    gpd_ecram_read(drvdata, drvdata->rpm_read + 1, &low);
>>         return (u16)high << 8 | low;
>>   }
>>   -static int gpd_wm2_read_rpm(void)
>> +static int gpd_wm2_read_rpm(struct gpd_fan_data *data)
>>   {
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>> +
>>       for (u16 pwm_ctr_offset = GPD_PWM_CTR_OFFSET;
>>            pwm_ctr_offset <= GPD_PWM_CTR_OFFSET + 2; pwm_ctr_offset++) {
>>           u8 PWMCTR;
>>   -        gpd_ecram_read(pwm_ctr_offset, &PWMCTR);
>> +        gpd_ecram_read(drvdata, pwm_ctr_offset, &PWMCTR);
>>             if (PWMCTR != 0xB8)
>> -            gpd_ecram_write(pwm_ctr_offset, 0xB8);
>> +            gpd_ecram_write(drvdata, pwm_ctr_offset, 0xB8);
>>       }
>>   -    return gpd_generic_read_rpm();
>> +    return gpd_generic_read_rpm(data);
>>   }
>>     // Read value for fan1_input
>> -static int gpd_read_rpm(void)
>> +static int gpd_read_rpm(struct gpd_fan_data *data)
>>   {
>> -    switch (gpd_driver_priv.drvdata->board) {
>> +    switch (data->drvdata->board) {
>>       case win4_6800u:
>>       case win_mini:
>>       case duo:
>>       case mpc2:
>> -        return gpd_generic_read_rpm();
>> +        return gpd_generic_read_rpm(data);
>>       case win_max_2:
>> -        return gpd_wm2_read_rpm();
>> +        return gpd_wm2_read_rpm(data);
>>       }
>>         return 0;
>>   }
>>   -static int gpd_wm2_read_pwm(void)
>> +static int gpd_wm2_read_pwm(struct gpd_fan_data *data)
>>   {
>> -    const struct gpd_fan_drvdata *const drvdata =
>> gpd_driver_priv.drvdata;
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>>       u8 var;
>>   -    gpd_ecram_read(drvdata->pwm_write, &var);
>> +    gpd_ecram_read(drvdata, drvdata->pwm_write, &var);
>>         // Match gpd_generic_write_pwm(u8) below
>>       return DIV_ROUND_CLOSEST((var - 1) * 255, (drvdata->pwm_max - 1));
>>   }
>>     // Read value for pwm1
>> -static int gpd_read_pwm(void)
>> +static int gpd_read_pwm(struct gpd_fan_data *data)
>>   {
>> -    switch (gpd_driver_priv.drvdata->board) {
>> +    switch (data->drvdata->board) {
>>       case win_mini:
>>       case duo:
>>       case win4_6800u:
>>       case mpc2:
>> -        switch (gpd_driver_priv.pwm_enable) {
>> +        switch (data->pwm_enable) {
>>           case DISABLE:
>>               return 255;
>>           case MANUAL:
>> -            return gpd_driver_priv.pwm_value;
>> +            return data->pwm_value;
>>           case AUTOMATIC:
>>               return -EOPNOTSUPP;
>>           }
>>           break;
>>       case win_max_2:
>> -        return gpd_wm2_read_pwm();
>> +        return gpd_wm2_read_pwm(data);
>>       }
>>       return 0;
>>   }
>>     // PWM value's range in EC is 1 - pwm_max, cast 0 - 255 to it.
>> -static inline u8 gpd_cast_pwm_range(u8 val)
>> +static inline u8 gpd_cast_pwm_range(const struct gpd_fan_drvdata
>> *drvdata, u8 val)
>>   {
>> -    const struct gpd_fan_drvdata *const drvdata =
>> gpd_driver_priv.drvdata;
>> -
>>       return DIV_ROUND_CLOSEST(val * (drvdata->pwm_max - 1), 255) + 1;
>>   }
>>   -static void gpd_generic_write_pwm(u8 val)
>> +static void gpd_generic_write_pwm(struct gpd_fan_data *data, u8 val)
>>   {
>> -    const struct gpd_fan_drvdata *const drvdata =
>> gpd_driver_priv.drvdata;
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>>       u8 pwm_reg;
>>   -    pwm_reg = gpd_cast_pwm_range(val);
>> -    gpd_ecram_write(drvdata->pwm_write, pwm_reg);
>> +    pwm_reg = gpd_cast_pwm_range(drvdata, val);
>> +    gpd_ecram_write(drvdata, drvdata->pwm_write, pwm_reg);
>>   }
>>   -static void gpd_duo_write_pwm(u8 val)
>> +static void gpd_duo_write_pwm(struct gpd_fan_data *data, u8 val)
>>   {
>> -    const struct gpd_fan_drvdata *const drvdata =
>> gpd_driver_priv.drvdata;
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>>       u8 pwm_reg;
>>   -    pwm_reg = gpd_cast_pwm_range(val);
>> -    gpd_ecram_write(drvdata->pwm_write, pwm_reg);
>> -    gpd_ecram_write(drvdata->pwm_write + 1, pwm_reg);
>> +    pwm_reg = gpd_cast_pwm_range(drvdata, val);
>> +    gpd_ecram_write(drvdata, drvdata->pwm_write, pwm_reg);
>> +    gpd_ecram_write(drvdata, drvdata->pwm_write + 1, pwm_reg);
>>   }
>>     // Write value for pwm1
>> -static int gpd_write_pwm(u8 val)
>> +static int gpd_write_pwm(struct gpd_fan_data *data, u8 val)
>>   {
>> -    if (gpd_driver_priv.pwm_enable != MANUAL)
>> +    if (data->pwm_enable != MANUAL)
>>           return -EPERM;
>>   -    switch (gpd_driver_priv.drvdata->board) {
>> +    switch (data->drvdata->board) {
>>       case duo:
>> -        gpd_duo_write_pwm(val);
>> +        gpd_duo_write_pwm(data, val);
>>           break;
>>       case win_mini:
>>       case win4_6800u:
>>       case win_max_2:
>>       case mpc2:
>> -        gpd_generic_write_pwm(val);
>> +        gpd_generic_write_pwm(data, val);
>>           break;
>>       }
>>         return 0;
>>   }
>>   -static void gpd_win_mini_set_pwm_enable(enum FAN_PWM_ENABLE
>> pwm_enable)
>> +static void gpd_win_mini_set_pwm_enable(struct gpd_fan_data *data,
>> enum FAN_PWM_ENABLE pwm_enable)
>>   {
>>       switch (pwm_enable) {
>>       case DISABLE:
>> -        gpd_generic_write_pwm(255);
>> +        gpd_generic_write_pwm(data, 255);
>>           break;
>>       case MANUAL:
>> -        gpd_generic_write_pwm(gpd_driver_priv.pwm_value);
>> +        gpd_generic_write_pwm(data, data->pwm_value);
>>           break;
>>       case AUTOMATIC:
>> -        gpd_ecram_write(gpd_driver_priv.drvdata->pwm_write, 0);
>> +        gpd_ecram_write(data->drvdata, data->drvdata->pwm_write, 0);
>>           break;
>>       }
>>   }
>>   -static void gpd_duo_set_pwm_enable(enum FAN_PWM_ENABLE pwm_enable)
>> +static void gpd_duo_set_pwm_enable(struct gpd_fan_data *data, enum
>> FAN_PWM_ENABLE pwm_enable)
>>   {
>>       switch (pwm_enable) {
>>       case DISABLE:
>> -        gpd_duo_write_pwm(255);
>> +        gpd_duo_write_pwm(data, 255);
>>           break;
>>       case MANUAL:
>> -        gpd_duo_write_pwm(gpd_driver_priv.pwm_value);
>> +        gpd_duo_write_pwm(data, data->pwm_value);
>>           break;
>>       case AUTOMATIC:
>> -        gpd_ecram_write(gpd_driver_priv.drvdata->pwm_write, 0);
>> +        gpd_ecram_write(data->drvdata, data->drvdata->pwm_write, 0);
>>           break;
>>       }
>>   }
>>   -static void gpd_wm2_set_pwm_enable(enum FAN_PWM_ENABLE enable)
>> +static void gpd_wm2_set_pwm_enable(struct gpd_fan_data *data, enum
>> FAN_PWM_ENABLE enable)
>>   {
>> -    const struct gpd_fan_drvdata *const drvdata =
>> gpd_driver_priv.drvdata;
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>>         switch (enable) {
>>       case DISABLE:
>> -        gpd_generic_write_pwm(255);
>> -        gpd_ecram_write(drvdata->manual_control_enable, 1);
>> +        gpd_generic_write_pwm(data, 255);
>> +        gpd_ecram_write(drvdata, drvdata->manual_control_enable, 1);
>>           break;
>>       case MANUAL:
>> -        gpd_generic_write_pwm(gpd_driver_priv.pwm_value);
>> -        gpd_ecram_write(drvdata->manual_control_enable, 1);
>> +        gpd_generic_write_pwm(data, data->pwm_value);
>> +        gpd_ecram_write(drvdata, drvdata->manual_control_enable, 1);
>>           break;
>>       case AUTOMATIC:
>> -        gpd_ecram_write(drvdata->manual_control_enable, 0);
>> +        gpd_ecram_write(drvdata, drvdata->manual_control_enable, 0);
>>           break;
>>       }
>>   }
>>     // Write value for pwm1_enable
>> -static void gpd_set_pwm_enable(enum FAN_PWM_ENABLE enable)
>> +static void gpd_set_pwm_enable(struct gpd_fan_data *data, enum
>> FAN_PWM_ENABLE enable)
>>   {
>>       if (enable == MANUAL)
>>           // Set pwm_value to max firstly when switching to manual
>> mode, in
>>           // consideration of device safety.
>> -        gpd_driver_priv.pwm_value = 255;
>> +        data->pwm_value = 255;
>>   -    switch (gpd_driver_priv.drvdata->board) {
>> +    switch (data->drvdata->board) {
>>       case win_mini:
>>       case win4_6800u:
>>       case mpc2:
>> -        gpd_win_mini_set_pwm_enable(enable);
>> +        gpd_win_mini_set_pwm_enable(data, enable);
>>           break;
>>       case duo:
>> -        gpd_duo_set_pwm_enable(enable);
>> +        gpd_duo_set_pwm_enable(data, enable);
>>           break;
>>       case win_max_2:
>> -        gpd_wm2_set_pwm_enable(enable);
>> +        gpd_wm2_set_pwm_enable(data, enable);
>>           break;
>>       }
>>   }
>> @@ -505,15 +504,16 @@ static umode_t
>> gpd_fan_hwmon_is_visible(__always_unused const void *drvdata,
>>       return 0;
>>   }
>>   -static int gpd_fan_hwmon_read(__always_unused struct device *dev,
>> +static int gpd_fan_hwmon_read(struct device *dev,
>>                     enum hwmon_sensor_types type, u32 attr,
>>                     __always_unused int channel, long *val)
>>   {
>> +    struct gpd_fan_data *data = dev_get_drvdata(dev);
>>       int ret;
>>         if (type == hwmon_fan) {
>>           if (attr == hwmon_fan_input) {
>> -            ret = gpd_read_rpm();
>> +            ret = gpd_read_rpm(data);
>>                 if (ret < 0)
>>                   return ret;
>> @@ -524,10 +524,10 @@ static int gpd_fan_hwmon_read(__always_unused
>> struct device *dev,
>>       } else if (type == hwmon_pwm) {
>>           switch (attr) {
>>           case hwmon_pwm_enable:
>> -            *val = gpd_driver_priv.pwm_enable;
>> +            *val = data->pwm_enable;
>>               return 0;
>>           case hwmon_pwm_input:
>> -            ret = gpd_read_pwm();
>> +            ret = gpd_read_pwm(data);
>>                 if (ret < 0)
>>                   return ret;
>> @@ -540,27 +540,29 @@ static int gpd_fan_hwmon_read(__always_unused
>> struct device *dev,
>>       return -EOPNOTSUPP;
>>   }
>>   -static int gpd_fan_hwmon_write(__always_unused struct device *dev,
>> +static int gpd_fan_hwmon_write(struct device *dev,
>>                      enum hwmon_sensor_types type, u32 attr,
>>                      __always_unused int channel, long val)
>>   {
>> +    struct gpd_fan_data *data = dev_get_drvdata(dev);
>> +
>>       if (type == hwmon_pwm) {
>>           switch (attr) {
>>           case hwmon_pwm_enable:
>>               if (!in_range(val, 0, 3))
>>                   return -EINVAL;
>>   -            gpd_driver_priv.pwm_enable = val;
>> +            data->pwm_enable = val;
>>   -            gpd_set_pwm_enable(gpd_driver_priv.pwm_enable);
>> +            gpd_set_pwm_enable(data, data->pwm_enable);
>>               return 0;
>>           case hwmon_pwm_input:
>>               if (!in_range(val, 0, 256))
>>                   return -EINVAL;
>>   -            gpd_driver_priv.pwm_value = val;
>> +            data->pwm_value = val;
>>   -            return gpd_write_pwm(val);
>> +            return gpd_write_pwm(data, val);
>>           }
>>       }
>>   @@ -584,26 +586,27 @@ static struct hwmon_chip_info
>> gpd_fan_chip_info = {
>>       .info = gpd_fan_hwmon_channel_info
>>   };
>>   -static void gpd_win4_init_ec(void)
>> +static void gpd_win4_init_ec(struct gpd_fan_data *data)
>>   {
>> +    const struct gpd_fan_drvdata *drvdata = data->drvdata;
>>       u8 chip_id, chip_ver;
>>   -    gpd_ecram_read(0x2000, &chip_id);
>> +    gpd_ecram_read(drvdata, 0x2000, &chip_id);
>>         if (chip_id == 0x55) {
>> -        gpd_ecram_read(0x1060, &chip_ver);
>> -        gpd_ecram_write(0x1060, chip_ver | 0x80);
>> +        gpd_ecram_read(drvdata, 0x1060, &chip_ver);
>> +        gpd_ecram_write(drvdata, 0x1060, chip_ver | 0x80);
>>       }
>>   }
>>   -static void gpd_init_ec(void)
>> +static void gpd_init_ec(struct gpd_fan_data *data)
>>   {
>>       // The buggy firmware won't initialize EC properly on boot.
>>       // Before its initialization, reading RPM will always return 0,
>>       // and writing PWM will have no effect.
>>       // Initialize it manually on driver load.
>> -    if (gpd_driver_priv.drvdata->board == win4_6800u)
>> -        gpd_win4_init_ec();
>> +    if (data->drvdata->board == win4_6800u)
>> +        gpd_win4_init_ec(data);
>>   }
>>     static int gpd_fan_probe(struct platform_device *pdev)
>> @@ -611,7 +614,9 @@ static int gpd_fan_probe(struct platform_device
>> *pdev)
>>       struct device *dev = &pdev->dev;
>>       const struct resource *region;
>>       const struct resource *res;
>> -    const struct device *hwdev;
>> +    struct device *hwdev;
>> +    struct gpd_fan_data *data;
>> +    const struct gpd_fan_drvdata *match;
>>         res = platform_get_resource(pdev, IORESOURCE_IO, 0);
>>       if (!res)
>> @@ -624,24 +629,42 @@ static int gpd_fan_probe(struct platform_device
>> *pdev)
>>           return dev_err_probe(dev, -EBUSY,
>>                        "Failed to request region\n");
>>   +    data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +    if (!data)
>> +        return -ENOMEM;
>> +
>> +    match = dev_get_platdata(dev);
>> +    if (!match)
>> +        return -EINVAL;
>> +
>> +    data->drvdata = match;
>> +    data->pwm_enable = AUTOMATIC;
>> +    data->pwm_value = 255;
>> +
>> +    dev_set_drvdata(dev, data);
>> +
>>       hwdev = devm_hwmon_device_register_with_info(dev,
>>                                DRIVER_NAME,
>> -                             NULL,
>> +                             data,
>>                                &gpd_fan_chip_info,
>>                                NULL);
>>       if (IS_ERR(hwdev))
>>           return dev_err_probe(dev, PTR_ERR(hwdev),
>>                        "Failed to register hwmon device\n");
>>   -    gpd_init_ec();
>> +    gpd_init_ec(data);
>>         return 0;
>>   }
>>   -static void gpd_fan_remove(__always_unused struct platform_device
>> *pdev)
>> +static void gpd_fan_remove(struct platform_device *pdev)
>>   {
>> -    gpd_driver_priv.pwm_enable = AUTOMATIC;
>> -    gpd_set_pwm_enable(AUTOMATIC);
>> +    struct gpd_fan_data *data = dev_get_drvdata(&pdev->dev);
>> +
>> +    if (data) {
>> +        data->pwm_enable = AUTOMATIC;
>> +        gpd_set_pwm_enable(data, AUTOMATIC);
>> +    }
>>   }
>>     static struct platform_driver gpd_fan_driver = {
>> @@ -668,6 +691,7 @@ static int __init gpd_fan_init(void)
>>       if (!match) {
>>           const struct dmi_system_id *dmi_match =
>>               dmi_first_match(dmi_table);
>> +
>>           if (dmi_match)
>>               match = dmi_match->driver_data;
>>       }
>> @@ -675,10 +699,6 @@ static int __init gpd_fan_init(void)
>>       if (!match)
>>           return -ENODEV;
>>   -    gpd_driver_priv.pwm_enable = AUTOMATIC;
>> -    gpd_driver_priv.pwm_value = 255;
>> -    gpd_driver_priv.drvdata = match;
>> -
>>       struct resource gpd_fan_resources[] = {
>>           {
>>               .start = match->addr_port,
>> @@ -690,7 +710,8 @@ static int __init gpd_fan_init(void)
>>       gpd_fan_platform_device = platform_create_bundle(&gpd_fan_driver,
>>                                gpd_fan_probe,
>>                                gpd_fan_resources,
>> -                             1, NULL, 0);
>> +                             1,
>> +                             match, sizeof(*match));
>>         if (IS_ERR(gpd_fan_platform_device)) {
>>           pr_warn("Failed to create platform device\n");
>
>
> I noticed you've been mixing different Latinization methods for your
> name and different email suffixes (foxmail and kylinos) in a short
> period of time. What could be the reason for this? Please keep it
> consistent. 
>
Hi Cryolitia,
    My company's email is not very stable; it often fails to send emails
(especially when using |git sendmail|). 
The company's poor email isn't even as good as Tencent's email. 
When the company email is down, I use my personal email to send on its
behalf.

thanks for your reply！
>
> thanks,
>
> Cryolitia PukNgae

[PATCH v3 2/4] hwmon: (gpd-fan): Initialize EC before registering hwmon device

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

Move the gpd_init_ec() call to before devm_hwmon_device_register_with_info
in the probe function. With the previous ordering the hwmon device was
registered and exposed to userspace before the EC initialization
completes, creating a window where sysfs reads could return invalid values.

Some buggy firmware won't initialize EC properly on boot. Before its
initialization, reading RPM will always return 0, and writing PWM will have
no effect. So move gpd_init_ec to before hwmon device register.

Fixes: 0ab88e239439 ("hwmon: add GPD devices sensor driver")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v3:
1.Add v3 tag

changes in v2:
1.gpd_init_ec needed before hwmon registration, as separate bug fix.
---
 drivers/hwmon/gpd-fan.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
index 7284babd4f5c..745b3fb9e3a4 100644
--- a/drivers/hwmon/gpd-fan.c
+++ b/drivers/hwmon/gpd-fan.c
@@ -643,6 +643,7 @@ static int gpd_fan_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, data);
 
+	gpd_init_ec(data);
 	hwdev = devm_hwmon_device_register_with_info(dev,
 						     DRIVER_NAME,
 						     data,
@@ -651,9 +652,6 @@ static int gpd_fan_probe(struct platform_device *pdev)
 	if (IS_ERR(hwdev))
 		return dev_err_probe(dev, PTR_ERR(hwdev),
 				     "Failed to register hwmon device\n");
-
-	gpd_init_ec(data);
-
 	return 0;
 }
 
-- 
2.25.1

[PATCH v3 3/4] hwmon: (gpd-fan): upgrade log level from warn to err for platform device creation failure

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

When platform_create_bundle() fails, the error is fatal and prevents the
driver from loading. Use pr_err() instead of pr_warn() to clearly indicate
a critical failure.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v3:
1.Add v3 tag

changes in v2:
1.No changes.
---
 drivers/hwmon/gpd-fan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
index 745b3fb9e3a4..1b57a5add934 100644
--- a/drivers/hwmon/gpd-fan.c
+++ b/drivers/hwmon/gpd-fan.c
@@ -712,7 +712,7 @@ static int __init gpd_fan_init(void)
 							 match, sizeof(*match));
 
 	if (IS_ERR(gpd_fan_platform_device)) {
-		pr_warn("Failed to create platform device\n");
+		pr_err("Failed to create platform device\n");
 		return PTR_ERR(gpd_fan_platform_device);
 	}
 
-- 
2.25.1

[PATCH v3 4/4] hwmon: (gpd-fan): fix race condition between device removal and sysfs access

[xiaopeitux]

From: Pei Xiao <xiaopei01@kylinos.cn>

Replace the manual gpd_fan_remove() callback with a devres-managed
action using devm_add_action_or_reset(). The original remove hook
resets the fan to AUTOMATIC mode, but the hwmon sysfs interface
(registered with devm_hwmon_device_register_with_info()) remains
active until after the remove callback completes. This creates a
race window where a concurrent userspace sysfs access can interleave
with the EC I/O sequence, potentially corrupting EC registers.

Using devm_add_action_or_reset() registers the reset function as a
devres action. Due to the LIFO release order of devres, the hwmon
device is unregistered (sysfs removed) before the reset action
executes, eliminating the race condition.

Fixes: 0ab88e239439 ("hwmon: add GPD devices sensor driver")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v3:
1.Add v3 tag

changes in v2:
1.Fix a race condition between device removal and sysfs access report by 
sashiko-bot
---
 drivers/hwmon/gpd-fan.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
index 1b57a5add934..d1993cd645cb 100644
--- a/drivers/hwmon/gpd-fan.c
+++ b/drivers/hwmon/gpd-fan.c
@@ -609,6 +609,16 @@ static void gpd_init_ec(struct gpd_fan_data *data)
 		gpd_win4_init_ec(data);
 }
 
+static void gpd_fan_reset_hardware(void *pdata)
+{
+	struct gpd_fan_data *data = pdata;
+
+	if (data) {
+		data->pwm_enable = AUTOMATIC;
+		gpd_set_pwm_enable(data, AUTOMATIC);
+	}
+}
+
 static int gpd_fan_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -617,6 +627,7 @@ static int gpd_fan_probe(struct platform_device *pdev)
 	struct device *hwdev;
 	struct gpd_fan_data *data;
 	const struct gpd_fan_drvdata *match;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
 	if (!res)
@@ -644,6 +655,11 @@ static int gpd_fan_probe(struct platform_device *pdev)
 	dev_set_drvdata(dev, data);
 
 	gpd_init_ec(data);
+
+	ret = devm_add_action_or_reset(dev, gpd_fan_reset_hardware, data);
+	if (ret)
+		return ret;
+
 	hwdev = devm_hwmon_device_register_with_info(dev,
 						     DRIVER_NAME,
 						     data,
@@ -655,19 +671,8 @@ static int gpd_fan_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static void gpd_fan_remove(struct platform_device *pdev)
-{
-	struct gpd_fan_data *data = dev_get_drvdata(&pdev->dev);
-
-	if (data) {
-		data->pwm_enable = AUTOMATIC;
-		gpd_set_pwm_enable(data, AUTOMATIC);
-	}
-}
-
 static struct platform_driver gpd_fan_driver = {
 	.probe = gpd_fan_probe,
-	.remove = gpd_fan_remove,
 	.driver = {
 		.name = KBUILD_MODNAME,
 	},
-- 
2.25.1