[PATCH v2 7/9] char: misc: Allocate 4 more fixed minors for watchdog

Zijun Hu posted 9 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v2 7/9] char: misc: Allocate 4 more fixed minors for watchdog
Posted by Zijun Hu 3 months, 1 week ago
From: Zijun Hu <zijun.hu@oss.qualcomm.com>

There are drivers which needs more fixed minors for watchdog, but
watchdog only has one fixed minor currently, it causes hardcoded and
unregistered fixed minors are used by these drivers.

Allocate 4 more fixed minors and apply for these drivers.
---
 drivers/hwmon/fschmd.c     | 3 ++-
 drivers/hwmon/w83793.c     | 3 ++-
 drivers/watchdog/cpwd.c    | 6 +++---
 include/linux/miscdevice.h | 4 ++++
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/fschmd.c b/drivers/hwmon/fschmd.c
index a303959879efddd9bfc02e8c25766e67b1a4af8e..7ffaec12eea8473a7dd17d7477db9157230e7f31 100644
--- a/drivers/hwmon/fschmd.c
+++ b/drivers/hwmon/fschmd.c
@@ -1084,7 +1084,8 @@ static int fschmd_probe(struct i2c_client *client)
 	struct fschmd_data *data;
 	static const char * const names[7] = { "Poseidon", "Hermes", "Scylla",
 				"Heracles", "Heimdall", "Hades", "Syleus" };
-	static const int watchdog_minors[] = { WATCHDOG_MINOR, 212, 213, 214, 215 };
+	static const int watchdog_minors[] = { WATCHDOG_MINOR, WATCHDOG_MINOR_1,
+		WATCHDOG_MINOR_2, WATCHDOG_MINOR_3, WATCHDOG_MINOR_4 };
 	int i, err;
 	enum chips kind = (uintptr_t)i2c_get_match_data(client);
 
diff --git a/drivers/hwmon/w83793.c b/drivers/hwmon/w83793.c
index 67728f60333fe351715ccc03097f9aa2e135f384..fb4d76c87ea26366586f6f6bc2bee4f76a430880 100644
--- a/drivers/hwmon/w83793.c
+++ b/drivers/hwmon/w83793.c
@@ -1642,7 +1642,8 @@ static int w83793_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	static const int watchdog_minors[] = {
-		WATCHDOG_MINOR, 212, 213, 214, 215
+		WATCHDOG_MINOR, WATCHDOG_MINOR_1, WATCHDOG_MINOR_2,
+		WATCHDOG_MINOR_3, WATCHDOG_MINOR_4
 	};
 	struct w83793_data *data;
 	int i, tmp, val, err;
diff --git a/drivers/watchdog/cpwd.c b/drivers/watchdog/cpwd.c
index 13a4d47e68cd82572f103c37ec066384cc5f7cd7..7891ea8f87451ddfb377e90706310a73324fb0ce 100644
--- a/drivers/watchdog/cpwd.c
+++ b/drivers/watchdog/cpwd.c
@@ -44,9 +44,9 @@
 #define WD_BTIMEOUT	(jiffies + (HZ * 1000))
 #define WD_BLIMIT	0xFFFF
 
-#define WD0_MINOR	212
-#define WD1_MINOR	213
-#define WD2_MINOR	214
+#define WD0_MINOR	WATCHDOG_MINOR_1
+#define WD1_MINOR	WATCHDOG_MINOR_2
+#define WD2_MINOR	WATCHDOG_MINOR_3
 
 /* Internal driver definitions.  */
 #define WD0_ID			0
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 565b88efeb23d02b7d91df1cd7df4bdcf2898224..c33839df8bd261bf1ab841b7e50afe3f8b0b007e 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -52,6 +52,10 @@
 #define PXA3XX_GCU_MINOR	197
 #define TUN_MINOR		200
 #define CUSE_MINOR		203
+#define WATCHDOG_MINOR_1	212
+#define WATCHDOG_MINOR_2	213
+#define WATCHDOG_MINOR_3	214
+#define WATCHDOG_MINOR_4	215
 #define MWAVE_MINOR		219	/* ACP/Mwave Modem */
 #define MPT_MINOR		220
 #define MPT2SAS_MINOR		221

-- 
2.34.1
Re: [PATCH v2 7/9] char: misc: Allocate 4 more fixed minors for watchdog
Posted by Arnd Bergmann 3 months, 1 week ago
On Tue, Jul 1, 2025, at 17:24, Zijun Hu wrote:
> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
>
> There are drivers which needs more fixed minors for watchdog, but
> watchdog only has one fixed minor currently, it causes hardcoded and
> unregistered fixed minors are used by these drivers.
>
> Allocate 4 more fixed minors and apply for these drivers.

Missing signoff?


I don't think this is the right fix here, these drivers implement
the normal watchdog API, so they should not even call misc_register
but should instead call watchdog_dev_register().

Obviously doing this right is a bigger change, so maybe the simpler
answer is to use dynamic minors instead of the nonstandard ones.

FWIW, I double-checked to see whether there are any in-tree
references to these two drivers, and there is one each:

drivers/i2c/busses/i2c-i801.c:	{ "Hermes", DMI_DEV_TYPE_OTHER, 0x73, "fscher" },
drivers/i2c/busses/i2c-i801.c:	{ "Hades",  DMI_DEV_TYPE_OTHER, 0x73, "fschds" },
drivers/i2c/busses/i2c-i801.c:	{ "Syleus", DMI_DEV_TYPE_OTHER, 0x73, "fscsyl" },
arch/powerpc/boot/dts/fsl/t4240rdb.dts:                         compatible = "winbond,w83793";

These were added in 2009 and 2016, respectively, so the hardware
is probably around somewhere but quite hard to find.

    Arnd
Re: [PATCH v2 7/9] char: misc: Allocate 4 more fixed minors for watchdog
Posted by Zijun Hu 3 months, 1 week ago
On 2025/7/2 02:45, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 17:24, Zijun Hu wrote:
>> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
>>
>> There are drivers which needs more fixed minors for watchdog, but
>> watchdog only has one fixed minor currently, it causes hardcoded and
>> unregistered fixed minors are used by these drivers.
>>
>> Allocate 4 more fixed minors and apply for these drivers.
> 
> Missing signoff?

my mistake. thank you for pointing out.

> 
> 
> I don't think this is the right fix here, these drivers implement
> the normal watchdog API, so they should not even call misc_register
> but should instead call watchdog_dev_register().
> 

agree

Documentation/watchdog/convert_drivers_to_kernel_api.rst indeed suggests
convert old watchdog implementation to new one using watchdog framework.

> Obviously doing this right is a bigger change, so maybe the simpler
> answer is to use dynamic minors instead of the nonstandard ones.
> 
yes. it is feasible.

> FWIW, I double-checked to see whether there are any in-tree
> references to these two drivers, and there is one each:
> 
> drivers/i2c/busses/i2c-i801.c:	{ "Hermes", DMI_DEV_TYPE_OTHER, 0x73, "fscher" },
> drivers/i2c/busses/i2c-i801.c:	{ "Hades",  DMI_DEV_TYPE_OTHER, 0x73, "fschds" },
> drivers/i2c/busses/i2c-i801.c:	{ "Syleus", DMI_DEV_TYPE_OTHER, 0x73, "fscsyl" },
> arch/powerpc/boot/dts/fsl/t4240rdb.dts:                         compatible = "winbond,w83793";
> 
> These were added in 2009 and 2016, respectively, so the hardware
> is probably around somewhere but quite hard to find.
> 

let me drop this change in next revision.
may try to convert them by following the guidance at my spare time.

>     Arnd