2020-03-09 02:26:11

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH RFC 0/3] clean up misc device minor numbers

Some the misc device minor numbers definitions spread in different
local c files, specially some are duplicate number with different
name, some are same name with conflict numbers, some prefer dynamic
minors.

This patchset try to address all of them.

To be honest, I didn't try build on arm or sparc arch which some
drivers depend on as I have little experience on cross-compile.
But I still checked the patch carefully to ensure it builds
in theory. Appreciate if anyone willing to test build on those arch.

Zhenzhong Duan (3):
misc: cleanup minor number definitions in c file into miscdevice.h
misc: move FLASH_MINOR into miscdevice.h and fix conflicts
speakup: misc: Use dynamic minor numbers for speakup devices

arch/arm/include/asm/nwflash.h | 1 -
arch/um/drivers/random.c | 4 +---
drivers/auxdisplay/charlcd.c | 2 --
drivers/auxdisplay/panel.c | 2 --
drivers/char/applicom.c | 1 -
drivers/char/hw_random/core.c | 2 +-
drivers/char/nwbutton.h | 1 -
drivers/char/nwflash.c | 2 +-
drivers/char/toshiba.c | 2 --
drivers/macintosh/ans-lcd.c | 2 +-
drivers/macintosh/ans-lcd.h | 2 --
drivers/macintosh/via-pmu.c | 3 ---
drivers/sbus/char/envctrl.c | 2 --
drivers/sbus/char/flash.c | 4 +---
drivers/sbus/char/uctrl.c | 2 --
drivers/staging/speakup/devsynth.c | 10 +++-------
drivers/staging/speakup/speakup_soft.c | 14 +++++++-------
drivers/video/fbdev/pxa3xx-gcu.c | 7 +++----
include/linux/miscdevice.h | 14 +++++++++++++-
kernel/power/user.c | 2 --
20 files changed, 31 insertions(+), 48 deletions(-)

--
1.8.3.1


2020-03-09 02:27:50

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH RFC 1/3] misc: cleanup minor number definitions in c file into miscdevice.h

HWRNG_MINOR and RNG_MISCDEV_MINOR are duplicate definitions, use
unified RNG_MINOR instead and moved into miscdevice.h

ANSLCD_MINOR and LCD_MINOR are duplicate definitions, use unified
LCD_MINOR instead and moved into miscdevice.h

MISCDEV_MINOR is renamed to PXA3XXX_GCU_MINOR and moved into
miscdevice.h

Other definitions are just moved without any change.

Link: https://lore.kernel.org/lkml/[email protected]/t/
Suggested-by: Arnd Bergmann <[email protected]>
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
arch/um/drivers/random.c | 4 +---
drivers/auxdisplay/charlcd.c | 2 --
drivers/auxdisplay/panel.c | 2 --
drivers/char/applicom.c | 1 -
drivers/char/hw_random/core.c | 2 +-
drivers/char/nwbutton.h | 1 -
drivers/char/toshiba.c | 2 --
drivers/macintosh/ans-lcd.c | 2 +-
drivers/macintosh/ans-lcd.h | 2 --
drivers/macintosh/via-pmu.c | 3 ---
drivers/sbus/char/envctrl.c | 2 --
drivers/sbus/char/uctrl.c | 2 --
drivers/video/fbdev/pxa3xx-gcu.c | 7 +++----
include/linux/miscdevice.h | 12 +++++++++++-
kernel/power/user.c | 2 --
15 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/arch/um/drivers/random.c b/arch/um/drivers/random.c
index 1d5d305..687ae80 100644
--- a/arch/um/drivers/random.c
+++ b/arch/um/drivers/random.c
@@ -23,8 +23,6 @@
#define RNG_VERSION "1.0.0"
#define RNG_MODULE_NAME "hw_random"

-#define RNG_MISCDEV_MINOR 183 /* official */
-
/* Changed at init time, in the non-modular case, and at module load
* time, in the module case. Presumably, the module subsystem
* protects against a module being loaded twice at the same time.
@@ -104,7 +102,7 @@ static ssize_t rng_dev_read (struct file *filp, char __user *buf, size_t size,

/* rng_init shouldn't be called more than once at boot time */
static struct miscdevice rng_miscdev = {
- RNG_MISCDEV_MINOR,
+ RNG_MINOR,
RNG_MODULE_NAME,
&rng_chrdev_ops,
};
diff --git a/drivers/auxdisplay/charlcd.c b/drivers/auxdisplay/charlcd.c
index 874c259..e704865 100644
--- a/drivers/auxdisplay/charlcd.c
+++ b/drivers/auxdisplay/charlcd.c
@@ -22,8 +22,6 @@

#include "charlcd.h"

-#define LCD_MINOR 156
-
#define DEFAULT_LCD_BWIDTH 40
#define DEFAULT_LCD_HWIDTH 64

diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
index 85965953..99980aa 100644
--- a/drivers/auxdisplay/panel.c
+++ b/drivers/auxdisplay/panel.c
@@ -57,8 +57,6 @@

#include "charlcd.h"

-#define KEYPAD_MINOR 185
-
#define LCD_MAXBYTES 256 /* max burst write */

#define KEYPAD_BUFFER 64
diff --git a/drivers/char/applicom.c b/drivers/char/applicom.c
index 51121a4..14b2d80 100644
--- a/drivers/char/applicom.c
+++ b/drivers/char/applicom.c
@@ -53,7 +53,6 @@
#define MAX_BOARD 8 /* maximum of pc board possible */
#define MAX_ISA_BOARD 4
#define LEN_RAM_IO 0x800
-#define AC_MINOR 157

#ifndef PCI_VENDOR_ID_APPLICOM
#define PCI_VENDOR_ID_APPLICOM 0x1389
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index d2d7a42..f4c86b5 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -289,7 +289,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
static const struct attribute_group *rng_dev_groups[];

static struct miscdevice rng_miscdev = {
- .minor = HWRNG_MINOR,
+ .minor = RNG_MINOR,
.name = RNG_MODULE_NAME,
.nodename = "hwrng",
.fops = &rng_chrdev_ops,
diff --git a/drivers/char/nwbutton.h b/drivers/char/nwbutton.h
index 9dedfd7..f2b9fdc 100644
--- a/drivers/char/nwbutton.h
+++ b/drivers/char/nwbutton.h
@@ -14,7 +14,6 @@
#define NUM_PRESSES_REBOOT 2 /* How many presses to activate shutdown */
#define BUTTON_DELAY 30 /* How many jiffies for sequence to end */
#define VERSION "0.3" /* Driver version number */
-#define BUTTON_MINOR 158 /* Major 10, Minor 158, /dev/nwbutton */

/* Structure definitions: */

diff --git a/drivers/char/toshiba.c b/drivers/char/toshiba.c
index 98f3150..aff0a8e 100644
--- a/drivers/char/toshiba.c
+++ b/drivers/char/toshiba.c
@@ -61,8 +61,6 @@
#include <linux/mutex.h>
#include <linux/toshiba.h>

-#define TOSH_MINOR_DEV 181
-
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Jonathan Buzzard <[email protected]>");
MODULE_DESCRIPTION("Toshiba laptop SMM driver");
diff --git a/drivers/macintosh/ans-lcd.c b/drivers/macintosh/ans-lcd.c
index b1314d1..b4821c7 100644
--- a/drivers/macintosh/ans-lcd.c
+++ b/drivers/macintosh/ans-lcd.c
@@ -142,7 +142,7 @@
};

static struct miscdevice anslcd_dev = {
- ANSLCD_MINOR,
+ LCD_MINOR,
"anslcd",
&anslcd_fops
};
diff --git a/drivers/macintosh/ans-lcd.h b/drivers/macintosh/ans-lcd.h
index f0a6e4c..bca7d76 100644
--- a/drivers/macintosh/ans-lcd.h
+++ b/drivers/macintosh/ans-lcd.h
@@ -2,8 +2,6 @@
#ifndef _PPC_ANS_LCD_H
#define _PPC_ANS_LCD_H

-#define ANSLCD_MINOR 156
-
#define ANSLCD_CLEAR 0x01
#define ANSLCD_SENDCTRL 0x02
#define ANSLCD_SETSHORTDELAY 0x03
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index d38fb78..83eb05b 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -75,9 +75,6 @@
/* Some compile options */
#undef DEBUG_SLEEP

-/* Misc minor number allocated for /dev/pmu */
-#define PMU_MINOR 154
-
/* How many iterations between battery polls */
#define BATTERY_POLLING_COUNT 2

diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c
index 12d66aa..843e830 100644
--- a/drivers/sbus/char/envctrl.c
+++ b/drivers/sbus/char/envctrl.c
@@ -37,8 +37,6 @@
#define DRIVER_NAME "envctrl"
#define PFX DRIVER_NAME ": "

-#define ENVCTRL_MINOR 162
-
#define PCF8584_ADDRESS 0x55

#define CONTROL_PIN 0x80
diff --git a/drivers/sbus/char/uctrl.c b/drivers/sbus/char/uctrl.c
index 7173a2e..37d252f2 100644
--- a/drivers/sbus/char/uctrl.c
+++ b/drivers/sbus/char/uctrl.c
@@ -23,8 +23,6 @@
#include <asm/io.h>
#include <asm/pgtable.h>

-#define UCTRL_MINOR 174
-
#define DEBUG 1
#ifdef DEBUG
#define dprintk(x) printk x
diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c
index 74ffb44..cb2eaab 100644
--- a/drivers/video/fbdev/pxa3xx-gcu.c
+++ b/drivers/video/fbdev/pxa3xx-gcu.c
@@ -36,7 +36,6 @@
#include "pxa3xx-gcu.h"

#define DRV_NAME "pxa3xx-gcu"
-#define MISCDEV_MINOR 197

#define REG_GCCR 0x00
#define GCCR_SYNC_CLR (1 << 9)
@@ -595,7 +594,7 @@ static int pxa3xx_gcu_probe(struct platform_device *pdev)
* container_of(). This isn't really necessary as we have a fixed minor
* number anyway, but this is to avoid statics. */

- priv->misc_dev.minor = MISCDEV_MINOR,
+ priv->misc_dev.minor = PXA3XXX_GCU_MINOR,
priv->misc_dev.name = DRV_NAME,
priv->misc_dev.fops = &pxa3xx_gcu_miscdev_fops;

@@ -638,7 +637,7 @@ static int pxa3xx_gcu_probe(struct platform_device *pdev)
ret = misc_register(&priv->misc_dev);
if (ret < 0) {
dev_err(dev, "misc_register() for minor %d failed\n",
- MISCDEV_MINOR);
+ PXA3XXX_GCU_MINOR);
goto err_free_dma;
}

@@ -714,7 +713,7 @@ static int pxa3xx_gcu_remove(struct platform_device *pdev)

MODULE_DESCRIPTION("PXA3xx graphics controller unit driver");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(MISCDEV_MINOR);
+MODULE_ALIAS_MISCDEV(PXA3XXX_GCU_MINOR);
MODULE_AUTHOR("Janine Kropp <[email protected]>, "
"Denis Oliver Kropp <[email protected]>, "
"Daniel Mack <[email protected]>");
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index becde69..35294c6 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -31,14 +31,23 @@
#define DMAPI_MINOR 140 /* unused */
#define NVRAM_MINOR 144
#define SGI_MMTIMER 153
+#define PMU_MINOR 154
#define STORE_QUEUE_MINOR 155 /* unused */
+#define LCD_MINOR 156
+#define AC_MINOR 157
+#define BUTTON_MINOR 158 /* Major 10, Minor 158, /dev/nwbutton */
+#define ENVCTRL_MINOR 162
#define I2O_MINOR 166
+#define UCTRL_MINOR 174
#define AGPGART_MINOR 175
-#define HWRNG_MINOR 183
+#define TOSH_MINOR_DEV 181
+#define RNG_MINOR 183
#define MICROCODE_MINOR 184
+#define KEYPAD_MINOR 185
#define IRNET_MINOR 187
#define D7S_MINOR 193
#define VFIO_MINOR 196
+#define PXA3XXX_GCU_MINOR 197
#define TUN_MINOR 200
#define CUSE_MINOR 203
#define MWAVE_MINOR 219 /* ACP/Mwave Modem */
@@ -49,6 +58,7 @@
#define MISC_MCELOG_MINOR 227
#define HPET_MINOR 228
#define FUSE_MINOR 229
+#define SNAPSHOT_MINOR 231
#define KVM_MINOR 232
#define BTRFS_MINOR 234
#define AUTOFS_MINOR 235
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 7743895..98fb659 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -27,8 +27,6 @@
#include "power.h"


-#define SNAPSHOT_MINOR 231
-
static struct snapshot_data {
struct snapshot_handle handle;
int swap;
--
1.8.3.1

2020-03-09 02:28:59

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH RFC 2/3] misc: move FLASH_MINOR into miscdevice.h and fix conflicts

FLASH_MINOR is used in both drivers/char/nwflash.c and
drivers/sbus/char/flash.c with conflict minor numbers.

Move all the definitions of FLASH_MINOR into miscdevice.h.
Rename FLASH_MINOR for drivers/char/nwflash.c to NWFLASH_MINOR
and FLASH_MINOR for drivers/sbus/char/flash.c to SBUS_FLASH_MINOR.

Link: https://lore.kernel.org/lkml/[email protected]/t/
Suggested-by: Arnd Bergmann <[email protected]>
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Russell King <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
arch/arm/include/asm/nwflash.h | 1 -
drivers/char/nwflash.c | 2 +-
drivers/sbus/char/flash.c | 4 +---
include/linux/miscdevice.h | 2 ++
4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/nwflash.h b/arch/arm/include/asm/nwflash.h
index 0ec6f07..66b7e68 100644
--- a/arch/arm/include/asm/nwflash.h
+++ b/arch/arm/include/asm/nwflash.h
@@ -2,7 +2,6 @@
#ifndef _FLASH_H
#define _FLASH_H

-#define FLASH_MINOR 160 /* MAJOR is 10 - miscdevice */
#define CMD_WRITE_DISABLE 0
#define CMD_WRITE_ENABLE 0x28
#define CMD_WRITE_BASE64K_ENABLE 0x47
diff --git a/drivers/char/nwflash.c b/drivers/char/nwflash.c
index a4a0797d..0973c2c 100644
--- a/drivers/char/nwflash.c
+++ b/drivers/char/nwflash.c
@@ -576,7 +576,7 @@ static void kick_open(void)

static struct miscdevice flash_miscdev =
{
- FLASH_MINOR,
+ NWFLASH_MINOR,
"nwflash",
&flash_fops
};
diff --git a/drivers/sbus/char/flash.c b/drivers/sbus/char/flash.c
index e85a05a..4147d22 100644
--- a/drivers/sbus/char/flash.c
+++ b/drivers/sbus/char/flash.c
@@ -31,8 +31,6 @@
unsigned long busy; /* In use? */
} flash;

-#define FLASH_MINOR 152
-
static int
flash_mmap(struct file *file, struct vm_area_struct *vma)
{
@@ -157,7 +155,7 @@
.release = flash_release,
};

-static struct miscdevice flash_dev = { FLASH_MINOR, "flash", &flash_fops };
+static struct miscdevice flash_dev = { SBUS_FLASH_MINOR, "flash", &flash_fops };

static int flash_probe(struct platform_device *op)
{
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 35294c6..41dc10b 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -30,12 +30,14 @@
#define SUN_OPENPROM_MINOR 139
#define DMAPI_MINOR 140 /* unused */
#define NVRAM_MINOR 144
+#define SBUS_FLASH_MINOR 152
#define SGI_MMTIMER 153
#define PMU_MINOR 154
#define STORE_QUEUE_MINOR 155 /* unused */
#define LCD_MINOR 156
#define AC_MINOR 157
#define BUTTON_MINOR 158 /* Major 10, Minor 158, /dev/nwbutton */
+#define NWFLASH_MINOR 160 /* MAJOR is 10 - miscdevice */
#define ENVCTRL_MINOR 162
#define I2O_MINOR 166
#define UCTRL_MINOR 174
--
1.8.3.1

2020-03-09 02:29:32

by Zhenzhong Duan

[permalink] [raw]
Subject: [PATCH RFC 3/3] speakup: misc: Use dynamic minor numbers for speakup devices

Arnd notes in the link:
| To clarify: the only numbers that I think should be changed to dynamic
| allocation are for drivers/staging/speakup. While this is a fairly old
| subsystem, I would expect that it being staging means we can be a
| little more progressive with the changes.

This releases misc device minor numbers 25-27 for dynamic usage.

Link: https://lore.kernel.org/lkml/[email protected]/t/
Suggested-by: Arnd Bergmann <[email protected]>
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: William Hubbs <[email protected]>
Cc: Chris Brannon <[email protected]>
Cc: Kirk Reiser <[email protected]>
Cc: Samuel Thibault <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
drivers/staging/speakup/devsynth.c | 10 +++-------
drivers/staging/speakup/speakup_soft.c | 14 +++++++-------
2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/speakup/devsynth.c b/drivers/staging/speakup/devsynth.c
index d920256..d305716 100644
--- a/drivers/staging/speakup/devsynth.c
+++ b/drivers/staging/speakup/devsynth.c
@@ -1,16 +1,12 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/errno.h>
-#include <linux/miscdevice.h> /* for misc_register, and SYNTH_MINOR */
+#include <linux/miscdevice.h> /* for misc_register, and MISC_DYNAMIC_MINOR */
#include <linux/types.h>
#include <linux/uaccess.h>

#include "speakup.h"
#include "spk_priv.h"

-#ifndef SYNTH_MINOR
-#define SYNTH_MINOR 25
-#endif
-
static int misc_registered;
static int dev_opened;

@@ -67,7 +63,7 @@ static int speakup_file_release(struct inode *ip, struct file *fp)
};

static struct miscdevice synth_device = {
- .minor = SYNTH_MINOR,
+ .minor = MISC_DYNAMIC_MINOR,
.name = "synth",
.fops = &synth_fops,
};
@@ -81,7 +77,7 @@ void speakup_register_devsynth(void)
pr_warn("Couldn't initialize miscdevice /dev/synth.\n");
} else {
pr_info("initialized device: /dev/synth, node (MAJOR %d, MINOR %d)\n",
- MISC_MAJOR, SYNTH_MINOR);
+ MISC_MAJOR, synth_device.minor);
misc_registered = 1;
}
}
diff --git a/drivers/staging/speakup/speakup_soft.c b/drivers/staging/speakup/speakup_soft.c
index 9d85a3a..eed246f 100644
--- a/drivers/staging/speakup/speakup_soft.c
+++ b/drivers/staging/speakup/speakup_soft.c
@@ -10,7 +10,7 @@
*/

#include <linux/unistd.h>
-#include <linux/miscdevice.h> /* for misc_register, and SYNTH_MINOR */
+#include <linux/miscdevice.h> /* for misc_register, and MISC_DYNAMIC_MINOR */
#include <linux/poll.h> /* for poll_wait() */

/* schedule(), signal_pending(), TASK_INTERRUPTIBLE */
@@ -20,8 +20,6 @@
#include "speakup.h"

#define DRV_VERSION "2.6"
-#define SOFTSYNTH_MINOR 26 /* might as well give it one more than /dev/synth */
-#define SOFTSYNTHU_MINOR 27 /* might as well give it one more than /dev/synth */
#define PROCSPEECH 0x0d
#define CLEAR_SYNTH 0x18

@@ -375,7 +373,7 @@ static int softsynth_probe(struct spk_synth *synth)
if (misc_registered != 0)
return 0;
memset(&synth_device, 0, sizeof(synth_device));
- synth_device.minor = SOFTSYNTH_MINOR;
+ synth_device.minor = MISC_DYNAMIC_MINOR;
synth_device.name = "softsynth";
synth_device.fops = &softsynth_fops;
if (misc_register(&synth_device)) {
@@ -384,7 +382,7 @@ static int softsynth_probe(struct spk_synth *synth)
}

memset(&synthu_device, 0, sizeof(synthu_device));
- synthu_device.minor = SOFTSYNTHU_MINOR;
+ synthu_device.minor = MISC_DYNAMIC_MINOR;
synthu_device.name = "softsynthu";
synthu_device.fops = &softsynthu_fops;
if (misc_register(&synthu_device)) {
@@ -393,8 +391,10 @@ static int softsynth_probe(struct spk_synth *synth)
}

misc_registered = 1;
- pr_info("initialized device: /dev/softsynth, node (MAJOR 10, MINOR 26)\n");
- pr_info("initialized device: /dev/softsynthu, node (MAJOR 10, MINOR 27)\n");
+ pr_info("initialized device: /dev/softsynth, node (MAJOR 10, MINOR %d)\n",
+ synth_device.minor);
+ pr_info("initialized device: /dev/softsynthu, node (MAJOR 10, MINOR %d)\n",
+ synthu_device.minor);
return 0;
}

--
1.8.3.1

2020-03-09 07:14:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] clean up misc device minor numbers

On Mon, Mar 09, 2020 at 10:17:44AM +0800, Zhenzhong Duan wrote:
> Some the misc device minor numbers definitions spread in different
> local c files, specially some are duplicate number with different
> name, some are same name with conflict numbers, some prefer dynamic
> minors.
>
> This patchset try to address all of them.
>
> To be honest, I didn't try build on arm or sparc arch which some
> drivers depend on as I have little experience on cross-compile.
> But I still checked the patch carefully to ensure it builds
> in theory. Appreciate if anyone willing to test build on those arch.
>
> Zhenzhong Duan (3):
> misc: cleanup minor number definitions in c file into miscdevice.h
> misc: move FLASH_MINOR into miscdevice.h and fix conflicts
> speakup: misc: Use dynamic minor numbers for speakup devices

Many thanks for this work, I think they all look sane except maybe the
last one, I'll respond there to that.

thanks,

greg k-h

2020-03-09 07:16:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH RFC 3/3] speakup: misc: Use dynamic minor numbers for speakup devices

On Mon, Mar 09, 2020 at 10:17:47AM +0800, Zhenzhong Duan wrote:
> Arnd notes in the link:
> | To clarify: the only numbers that I think should be changed to dynamic
> | allocation are for drivers/staging/speakup. While this is a fairly old
> | subsystem, I would expect that it being staging means we can be a
> | little more progressive with the changes.
>
> This releases misc device minor numbers 25-27 for dynamic usage.
>
> Link: https://lore.kernel.org/lkml/[email protected]/t/
> Suggested-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Cc: William Hubbs <[email protected]>
> Cc: Chris Brannon <[email protected]>
> Cc: Kirk Reiser <[email protected]>
> Cc: Samuel Thibault <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/staging/speakup/devsynth.c | 10 +++-------
> drivers/staging/speakup/speakup_soft.c | 14 +++++++-------
> 2 files changed, 10 insertions(+), 14 deletions(-)

speakup, while being in staging, has been around for a very long time,
so we might break things if we change their minor numbers.

I'd need an ACK from the speakup maintainers/developers before I can
take this as I don't have any way to verify what their systems look
like.

thanks,

greg k-h

2020-03-09 08:55:50

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] misc: cleanup minor number definitions in c file into miscdevice.h

On Mon, Mar 09, 2020 at 10:17:45AM +0800, Zhenzhong Duan wrote:
> HWRNG_MINOR and RNG_MISCDEV_MINOR are duplicate definitions, use
> unified RNG_MINOR instead and moved into miscdevice.h

Please keep the HWRNG_MINOR name, RNG_MINOR could cause confusion.

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2020-03-09 10:18:06

by Willy TARREAU

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] clean up misc device minor numbers

On Mon, Mar 09, 2020 at 10:17:44AM +0800, Zhenzhong Duan wrote:
> Some the misc device minor numbers definitions spread in different
> local c files, specially some are duplicate number with different
> name, some are same name with conflict numbers, some prefer dynamic
> minors.
>
> This patchset try to address all of them.

Thanks for this! When I initially created panel.c about 20 years ago
I didn't even realize there was a miscdevice.h to centralize all this.
It's definitely cleaner this way.

> To be honest, I didn't try build on arm or sparc arch which some
> drivers depend on as I have little experience on cross-compile.
> But I still checked the patch carefully to ensure it builds
> in theory. Appreciate if anyone willing to test build on those arch.

So I've built for ARM to check, I could enable and successfully build
these modules that you touched: charlcd, panel, applicom, devsynth,
speakup_soft. The other ones might require less obvious configs to be
build-tested.

Willy

2020-03-09 11:37:48

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] clean up misc device minor numbers

On Mon, Mar 9, 2020 at 10:51 AM Willy TARREAU <[email protected]> wrote:
>
> Thanks for this! When I initially created panel.c about 20 years ago
> I didn't even realize there was a miscdevice.h to centralize all this.
> It's definitely cleaner this way.

+1

> So I've built for ARM to check, I could enable and successfully build
> these modules that you touched: charlcd

Thanks for checking Willy! Also compiled here for both arm and arm64;
will send the ack.

Cheers,
Miguel

2020-03-09 11:38:38

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] misc: cleanup minor number definitions in c file into miscdevice.h

On Mon, Mar 9, 2020 at 3:18 AM Zhenzhong Duan <[email protected]> wrote:
>
> drivers/auxdisplay/charlcd.c | 2 --
> drivers/auxdisplay/panel.c | 2 --

Compile-tested these two for arm and arm64.

Acked-by: Miguel Ojeda <[email protected]>

Cheers,
Miguel

2020-03-09 13:07:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH RFC 2/3] misc: move FLASH_MINOR into miscdevice.h and fix conflicts

On Mon, Mar 9, 2020 at 3:18 AM Zhenzhong Duan <[email protected]> wrote:
>
> FLASH_MINOR is used in both drivers/char/nwflash.c and
> drivers/sbus/char/flash.c with conflict minor numbers.
>
> Move all the definitions of FLASH_MINOR into miscdevice.h.
> Rename FLASH_MINOR for drivers/char/nwflash.c to NWFLASH_MINOR
> and FLASH_MINOR for drivers/sbus/char/flash.c to SBUS_FLASH_MINOR.
>
> Link: https://lore.kernel.org/lkml/[email protected]/t/
> Suggested-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: "David S. Miller" <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2020-03-09 13:09:49

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] misc: cleanup minor number definitions in c file into miscdevice.h

On Mon, Mar 9, 2020 at 3:18 AM Zhenzhong Duan <[email protected]> wrote:
>
> HWRNG_MINOR and RNG_MISCDEV_MINOR are duplicate definitions, use
> unified RNG_MINOR instead and moved into miscdevice.h
>
> ANSLCD_MINOR and LCD_MINOR are duplicate definitions, use unified
> LCD_MINOR instead and moved into miscdevice.h
>
> MISCDEV_MINOR is renamed to PXA3XXX_GCU_MINOR and moved into
> miscdevice.h

This should be PXA3XX_GCU_MINOR (with 2 X, not 3).

With that (and the other comments) addressed

Acked-by: Arnd Bergmann <[email protected]>

2020-03-11 02:58:16

by Zhenzhong Duan

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] misc: cleanup minor number definitions in c file into miscdevice.h

On Mon, Mar 9, 2020 at 9:08 PM Arnd Bergmann <[email protected]> wrote:
>
> On Mon, Mar 9, 2020 at 3:18 AM Zhenzhong Duan <[email protected]> wrote:
> >
> > HWRNG_MINOR and RNG_MISCDEV_MINOR are duplicate definitions, use
> > unified RNG_MINOR instead and moved into miscdevice.h
> >
> > ANSLCD_MINOR and LCD_MINOR are duplicate definitions, use unified
> > LCD_MINOR instead and moved into miscdevice.h
> >
> > MISCDEV_MINOR is renamed to PXA3XXX_GCU_MINOR and moved into
> > miscdevice.h
>
> This should be PXA3XX_GCU_MINOR (with 2 X, not 3).

Will fix in next version, thanks for pointing out.

Zhenzhong

2020-03-11 03:09:12

by Zhenzhong Duan

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] misc: cleanup minor number definitions in c file into miscdevice.h

On Mon, Mar 9, 2020 at 4:55 PM Herbert Xu <[email protected]> wrote:
>
> On Mon, Mar 09, 2020 at 10:17:45AM +0800, Zhenzhong Duan wrote:
> > HWRNG_MINOR and RNG_MISCDEV_MINOR are duplicate definitions, use
> > unified RNG_MINOR instead and moved into miscdevice.h
>
> Please keep the HWRNG_MINOR name, RNG_MINOR could cause confusion.

Thanks for your suggestion, I'll use HWRNG_MINOR as the unique.

Zhenzhong

2020-03-11 03:15:47

by Zhenzhong Duan

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] clean up misc device minor numbers

On Mon, Mar 9, 2020 at 7:36 PM Miguel Ojeda
<[email protected]> wrote:
>
> On Mon, Mar 9, 2020 at 10:51 AM Willy TARREAU <[email protected]> wrote:
> >
> > Thanks for this! When I initially created panel.c about 20 years ago
> > I didn't even realize there was a miscdevice.h to centralize all this.
> > It's definitely cleaner this way.
>
> +1
>
> > So I've built for ARM to check, I could enable and successfully build
> > these modules that you touched: charlcd
>
> Thanks for checking Willy! Also compiled here for both arm and arm64;
> will send the ack.

Thanks Willy and Miguel, much appreciated!

Zhenzhong

2020-03-15 01:46:20

by Samuel Thibault

[permalink] [raw]
Subject: Re: [PATCH RFC 3/3] speakup: misc: Use dynamic minor numbers for speakup devices

Hello,

Greg KH, le lun. 09 mars 2020 08:15:06 +0100, a ecrit:
> On Mon, Mar 09, 2020 at 10:17:47AM +0800, Zhenzhong Duan wrote:
> > Arnd notes in the link:
> > | To clarify: the only numbers that I think should be changed to dynamic
> > | allocation are for drivers/staging/speakup. While this is a fairly old
> > | subsystem, I would expect that it being staging means we can be a
> > | little more progressive with the changes.
> >
> > This releases misc device minor numbers 25-27 for dynamic usage.
> >
> > Link: https://lore.kernel.org/lkml/[email protected]/t/
> > Suggested-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Zhenzhong Duan <[email protected]>

Acked-by: Samuel Thibault <[email protected]>

> > Cc: William Hubbs <[email protected]>
> > Cc: Chris Brannon <[email protected]>
> > Cc: Kirk Reiser <[email protected]>
> > Cc: Samuel Thibault <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > ---
> > drivers/staging/speakup/devsynth.c | 10 +++-------
> > drivers/staging/speakup/speakup_soft.c | 14 +++++++-------
> > 2 files changed, 10 insertions(+), 14 deletions(-)
>
> speakup, while being in staging, has been around for a very long time,
> so we might break things if we change their minor numbers.
>
> I'd need an ACK from the speakup maintainers/developers before I can
> take this as I don't have any way to verify what their systems look
> like.

I believe it will be fine to use dynamic minor numbers, since the /dev
entries are autocreated nowadays, and the espeakup and speechd-up don't
use hardcoded minor values.

Thanks for making sure,
Samuel