diff mbox series

parisc: Use signed char for hardware path in pdc.h

Message ID 20221021072038.83248-1-deller@gmx.de (mailing list archive)
State Accepted, archived
Headers show
Series parisc: Use signed char for hardware path in pdc.h | expand

Commit Message

Helge Deller Oct. 21, 2022, 7:20 a.m. UTC
Clean up the struct for hardware_path and drop the struct device_path
with a proper assignment of bc[] and mod members as signed chars.

This patch prepares for the kbuild change from Jason A. Donenfeld to
treat char as always unsigned.

Signed-off-by: Helge Deller <deller@gmx.de>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/parisc/include/uapi/asm/pdc.h | 36 +++++++++++-------------------
 drivers/parisc/pdc_stable.c        | 34 ++++++++++++++--------------
 2 files changed, 30 insertions(+), 40 deletions(-)

--
2.37.3

Comments

Rolf Eike Beer Oct. 22, 2022, 8:19 a.m. UTC | #1
Am Freitag, 21. Oktober 2022, 09:20:38 CEST schrieb Helge Deller:
> Clean up the struct for hardware_path and drop the struct device_path
> with a proper assignment of bc[] and mod members as signed chars.
> 
> This patch prepares for the kbuild change from Jason A. Donenfeld to
> treat char as always unsigned.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  arch/parisc/include/uapi/asm/pdc.h | 36 +++++++++++-------------------
>  drivers/parisc/pdc_stable.c        | 34 ++++++++++++++--------------
>  2 files changed, 30 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/parisc/include/uapi/asm/pdc.h
> b/arch/parisc/include/uapi/asm/pdc.h index e794e143ec5f..7a90070136e8
> 100644
> --- a/arch/parisc/include/uapi/asm/pdc.h
> +++ b/arch/parisc/include/uapi/asm/pdc.h
> @@ -363,20 +363,25 @@
> 
>  #if !defined(__ASSEMBLY__)
> 
> -/* flags of the device_path */
> +/* flags for hardware_path */
>  #define	PF_AUTOBOOT	0x80
>  #define	PF_AUTOSEARCH	0x40
>  #define	PF_TIMER	0x0F
> 
> -struct device_path {		/* page 1-69 */
> -	unsigned char flags;	/* flags see above! */
> -	unsigned char bc[6];	/* bus converter routing info */
> -	unsigned char mod;
> -	unsigned int  layers[6];/* device-specific layer-info */
> -} __attribute__((aligned(8))) ;
> +struct hardware_path {
> +	unsigned char flags;	/* see bit definitions below */
> +	signed   char bc[6];	/* Bus Converter routing info to a specific */
> +				/* I/O adaptor (< 0 means none, > 63 resvd) */
> +	signed   char mod;	/* fixed field of specified module */
> +};
> +
> +struct pdc_module_path {	/* page 1-69 */
> +	struct hardware_path path;
> +	unsigned int layers[6]; /* device-specific info (ctlr #, unit # ...) */
> +} __attribute__((aligned(8)));
> 
>  struct pz_device {
> -	struct	device_path dp;	/* see above */
> +	struct pdc_module_path dp;	/* see above */
>  	/* struct	iomod *hpa; */
>  	unsigned int hpa;	/* HPA base address */
>  	/* char	*spa; */
> @@ -611,21 +616,6 @@ struct pdc_initiator { /* PDC_INITIATOR */
>  	int mode;
>  };
> 
> -struct hardware_path {
> -	char  flags;	/* see bit definitions below */
> -	char  bc[6];	/* Bus Converter routing info to a specific */
> -			/* I/O adaptor (< 0 means none, > 63 resvd) */
> -	char  mod;	/* fixed field of specified module */
> -};
> -
> -/*
> - * Device path specifications used by PDC.
> - */
> -struct pdc_module_path {
> -	struct hardware_path path;
> -	unsigned int layers[6]; /* device-specific info (ctlr #, unit # ...) */
> -};
> -
>  /* Only used on some pre-PA2.0 boxes */
>  struct pdc_memory_map {		/* PDC_MEMORY_MAP */
>  	unsigned long hpa;	/* mod's register set address */
> diff --git a/drivers/parisc/pdc_stable.c b/drivers/parisc/pdc_stable.c
> index d9e51036a4fa..d6af5726ddf3 100644
> --- a/drivers/parisc/pdc_stable.c
> +++ b/drivers/parisc/pdc_stable.c
> @@ -88,7 +88,7 @@ struct pdcspath_entry {
>  	short ready;			/* entry record is valid if != 0 */
>  	unsigned long addr;		/* entry address in stable storage */
>  	char *name;			/* entry name */
> -	struct device_path devpath;	/* device path in parisc representation */
> +	struct pdc_module_path devpath;	/* device path in parisc representation */
> struct device *dev;		/* corresponding device */
>  	struct kobject kobj;
>  };
> @@ -138,7 +138,7 @@ struct pdcspath_attribute paths_attr_##_name = { \
>  static int
>  pdcspath_fetch(struct pdcspath_entry *entry)
>  {
> -	struct device_path *devpath;
> +	struct pdc_module_path *devpath;
> 
>  	if (!entry)
>  		return -EINVAL;
> @@ -153,7 +153,7 @@ pdcspath_fetch(struct pdcspath_entry *entry)
>  		return -EIO;
> 
>  	/* Find the matching device.
> -	   NOTE: hardware_path overlays with device_path, so the nice cast can
> +	   NOTE: hardware_path overlays with pdc_module_path, so the nice cast can
> be used */
>  	entry->dev = hwpath_to_device((struct hardware_path *)devpath);

Maybe just use &devpath->path instead and scrap the comment?

Regards,

Eike
Jason A. Donenfeld Oct. 24, 2022, 3:22 p.m. UTC | #2
Hi Helge,

I assume this takes care of the cases that Dan found in:

https://lore.kernel.org/lkml/Y1ZZyP4ZRBIbv+Kg@kili/ ?

By the way, I would queue this up in my unsigned-char tree -- Linus
asked me to keep the fixes for the fallout together -- but your patch
clearly applies over in-development parisc code, so it makes sense to
keep it with the parisc code, and I'll make note of this one.

Applied here, right?
https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-next&id=50f19697dd768d8b072cf7f12c0c99c7d31b67d8

Jason
diff mbox series

Patch

diff --git a/arch/parisc/include/uapi/asm/pdc.h b/arch/parisc/include/uapi/asm/pdc.h
index e794e143ec5f..7a90070136e8 100644
--- a/arch/parisc/include/uapi/asm/pdc.h
+++ b/arch/parisc/include/uapi/asm/pdc.h
@@ -363,20 +363,25 @@ 

 #if !defined(__ASSEMBLY__)

-/* flags of the device_path */
+/* flags for hardware_path */
 #define	PF_AUTOBOOT	0x80
 #define	PF_AUTOSEARCH	0x40
 #define	PF_TIMER	0x0F

-struct device_path {		/* page 1-69 */
-	unsigned char flags;	/* flags see above! */
-	unsigned char bc[6];	/* bus converter routing info */
-	unsigned char mod;
-	unsigned int  layers[6];/* device-specific layer-info */
-} __attribute__((aligned(8))) ;
+struct hardware_path {
+	unsigned char flags;	/* see bit definitions below */
+	signed   char bc[6];	/* Bus Converter routing info to a specific */
+				/* I/O adaptor (< 0 means none, > 63 resvd) */
+	signed   char mod;	/* fixed field of specified module */
+};
+
+struct pdc_module_path {	/* page 1-69 */
+	struct hardware_path path;
+	unsigned int layers[6]; /* device-specific info (ctlr #, unit # ...) */
+} __attribute__((aligned(8)));

 struct pz_device {
-	struct	device_path dp;	/* see above */
+	struct pdc_module_path dp;	/* see above */
 	/* struct	iomod *hpa; */
 	unsigned int hpa;	/* HPA base address */
 	/* char	*spa; */
@@ -611,21 +616,6 @@  struct pdc_initiator { /* PDC_INITIATOR */
 	int mode;
 };

-struct hardware_path {
-	char  flags;	/* see bit definitions below */
-	char  bc[6];	/* Bus Converter routing info to a specific */
-			/* I/O adaptor (< 0 means none, > 63 resvd) */
-	char  mod;	/* fixed field of specified module */
-};
-
-/*
- * Device path specifications used by PDC.
- */
-struct pdc_module_path {
-	struct hardware_path path;
-	unsigned int layers[6]; /* device-specific info (ctlr #, unit # ...) */
-};
-
 /* Only used on some pre-PA2.0 boxes */
 struct pdc_memory_map {		/* PDC_MEMORY_MAP */
 	unsigned long hpa;	/* mod's register set address */
diff --git a/drivers/parisc/pdc_stable.c b/drivers/parisc/pdc_stable.c
index d9e51036a4fa..d6af5726ddf3 100644
--- a/drivers/parisc/pdc_stable.c
+++ b/drivers/parisc/pdc_stable.c
@@ -14,7 +14,7 @@ 
  *    all) PA-RISC machines should have them. Anyway, for safety reasons, the
  *    following code can deal with just 96 bytes of Stable Storage, and all
  *    sizes between 96 and 192 bytes (provided they are multiple of struct
- *    device_path size, eg: 128, 160 and 192) to provide full information.
+ *    pdc_module_path size, eg: 128, 160 and 192) to provide full information.
  *    One last word: there's one path we can always count on: the primary path.
  *    Anything above 224 bytes is used for 'osdep2' OS-dependent storage area.
  *
@@ -88,7 +88,7 @@  struct pdcspath_entry {
 	short ready;			/* entry record is valid if != 0 */
 	unsigned long addr;		/* entry address in stable storage */
 	char *name;			/* entry name */
-	struct device_path devpath;	/* device path in parisc representation */
+	struct pdc_module_path devpath;	/* device path in parisc representation */
 	struct device *dev;		/* corresponding device */
 	struct kobject kobj;
 };
@@ -138,7 +138,7 @@  struct pdcspath_attribute paths_attr_##_name = { \
 static int
 pdcspath_fetch(struct pdcspath_entry *entry)
 {
-	struct device_path *devpath;
+	struct pdc_module_path *devpath;

 	if (!entry)
 		return -EINVAL;
@@ -153,7 +153,7 @@  pdcspath_fetch(struct pdcspath_entry *entry)
 		return -EIO;

 	/* Find the matching device.
-	   NOTE: hardware_path overlays with device_path, so the nice cast can
+	   NOTE: hardware_path overlays with pdc_module_path, so the nice cast can
 	   be used */
 	entry->dev = hwpath_to_device((struct hardware_path *)devpath);

@@ -179,7 +179,7 @@  pdcspath_fetch(struct pdcspath_entry *entry)
 static void
 pdcspath_store(struct pdcspath_entry *entry)
 {
-	struct device_path *devpath;
+	struct pdc_module_path *devpath;

 	BUG_ON(!entry);

@@ -221,7 +221,7 @@  static ssize_t
 pdcspath_hwpath_read(struct pdcspath_entry *entry, char *buf)
 {
 	char *out = buf;
-	struct device_path *devpath;
+	struct pdc_module_path *devpath;
 	short i;

 	if (!entry || !buf)
@@ -236,11 +236,11 @@  pdcspath_hwpath_read(struct pdcspath_entry *entry, char *buf)
 		return -ENODATA;

 	for (i = 0; i < 6; i++) {
-		if (devpath->bc[i] >= 128)
+		if (devpath->path.bc[i] < 0)
 			continue;
-		out += sprintf(out, "%u/", (unsigned char)devpath->bc[i]);
+		out += sprintf(out, "%d/", devpath->path.bc[i]);
 	}
-	out += sprintf(out, "%u\n", (unsigned char)devpath->mod);
+	out += sprintf(out, "%u\n", (unsigned char)devpath->path.mod);

 	return out - buf;
 }
@@ -296,12 +296,12 @@  pdcspath_hwpath_write(struct pdcspath_entry *entry, const char *buf, size_t coun
 	for (i=5; ((temp = strrchr(in, '/'))) && (temp-in > 0) && (likely(i)); i--) {
 		hwpath.bc[i] = simple_strtoul(temp+1, NULL, 10);
 		in[temp-in] = '\0';
-		DPRINTK("%s: bc[%d]: %d\n", __func__, i, hwpath.bc[i]);
+		DPRINTK("%s: bc[%d]: %d\n", __func__, i, hwpath.path.bc[i]);
 	}

 	/* Store the final field */
 	hwpath.bc[i] = simple_strtoul(in, NULL, 10);
-	DPRINTK("%s: bc[%d]: %d\n", __func__, i, hwpath.bc[i]);
+	DPRINTK("%s: bc[%d]: %d\n", __func__, i, hwpath.path.bc[i]);

 	/* Now we check that the user isn't trying to lure us */
 	if (!(dev = hwpath_to_device((struct hardware_path *)&hwpath))) {
@@ -342,7 +342,7 @@  static ssize_t
 pdcspath_layer_read(struct pdcspath_entry *entry, char *buf)
 {
 	char *out = buf;
-	struct device_path *devpath;
+	struct pdc_module_path *devpath;
 	short i;

 	if (!entry || !buf)
@@ -547,7 +547,7 @@  static ssize_t pdcs_auto_read(struct kobject *kobj,
 	pathentry = &pdcspath_entry_primary;

 	read_lock(&pathentry->rw_lock);
-	out += sprintf(out, "%s\n", (pathentry->devpath.flags & knob) ?
+	out += sprintf(out, "%s\n", (pathentry->devpath.path.flags & knob) ?
 					"On" : "Off");
 	read_unlock(&pathentry->rw_lock);

@@ -594,8 +594,8 @@  static ssize_t pdcs_timer_read(struct kobject *kobj,

 	/* print the timer value in seconds */
 	read_lock(&pathentry->rw_lock);
-	out += sprintf(out, "%u\n", (pathentry->devpath.flags & PF_TIMER) ?
-				(1 << (pathentry->devpath.flags & PF_TIMER)) : 0);
+	out += sprintf(out, "%u\n", (pathentry->devpath.path.flags & PF_TIMER) ?
+				(1 << (pathentry->devpath.path.flags & PF_TIMER)) : 0);
 	read_unlock(&pathentry->rw_lock);

 	return out - buf;
@@ -764,7 +764,7 @@  static ssize_t pdcs_auto_write(struct kobject *kobj,

 	/* Be nice to the existing flag record */
 	read_lock(&pathentry->rw_lock);
-	flags = pathentry->devpath.flags;
+	flags = pathentry->devpath.path.flags;
 	read_unlock(&pathentry->rw_lock);

 	DPRINTK("%s: flags before: 0x%X\n", __func__, flags);
@@ -785,7 +785,7 @@  static ssize_t pdcs_auto_write(struct kobject *kobj,
 	write_lock(&pathentry->rw_lock);

 	/* Change the path entry flags first */
-	pathentry->devpath.flags = flags;
+	pathentry->devpath.path.flags = flags;

 	/* Now, dive in. Write back to the hardware */
 	pdcspath_store(pathentry);