staging: xgifb: remove macros with hidden variable

PATCH Superseded Joshua Abraham 2 participants 3 messages Nov 30, 2017 – Dec 1, 2017 Staging

Share & tools

Joshua Abraham

author

2017-11-30 15:39 UTC #0 lore raw

PATCH v2 1/1 staging: xgifb: remove macros with hidden variable

This patch removes macros in XGI_main.h that contain a xgifb_info variable. These macros hurt readability by hiding said variable behind a define.

Signed-off-by Joshua Abraham <[email protected]>

---
 drivers/staging/xgifb/XGI_main.h    |  13 ---
 drivers/staging/xgifb/XGI_main_26.c | 227 +++++++++++++++++++++---------------
 2 files changed, 131 insertions(+), 109 deletions(-)

drivers/staging/xgifb/XGI_main.h +0 ~~-13~~

index a3af1cbbf8ee..e19a8291cb2a 100644
@@ -18,19 +18,6 @@ static const struct pci_device_id xgifb_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, xgifb_pci_table);
 
-/* To be included in fb.h */
-#define XGISR			  (xgifb_info->dev_info.P3c4)
-#define XGICR			  (xgifb_info->dev_info.P3d4)
-#define XGIDACA			  (xgifb_info->dev_info.P3c8)
-#define XGIDACD			  (xgifb_info->dev_info.P3c9)
-#define XGIPART1		  (xgifb_info->dev_info.Part1Port)
-#define XGIPART2		  (xgifb_info->dev_info.Part2Port)
-#define XGIPART3		  (xgifb_info->dev_info.Part3Port)
-#define XGIPART4		  (xgifb_info->dev_info.Part4Port)
-#define XGIPART5		  (xgifb_info->dev_info.Part5Port)
-#define XGIDAC2A                  XGIPART5
-#define XGIDAC2D                  (XGIPART5 + 1)
-
 #define IND_XGI_SCRATCH_REG_CR30  0x30  /* CRs */
 #define IND_XGI_SCRATCH_REG_CR31  0x31
 #define IND_XGI_SCRATCH_REG_CR32  0x32

drivers/staging/xgifb/XGI_main_26.c +131 ~~-96~~

497 lines not on this page · raw message

Signature · 1 line

2.15.1

Dan Carpenter

2017-12-01 07:13 UTC #1 lore raw

drivers/staging/xgifb/XGI_main_26.c static void dumpVGAReg(struct xgifb_video_info *xgifb_info) · line 37

Quoted 10 lines · Joshua Abraham

drivers/staging/xgifb/XGI_main_26.c +1 -1

index 6feecc55d2bc..6de66eaad96b 100644
@@ -34,16 +34,16 @@ static void dumpVGAReg(struct xgifb_video_info *xgifb_info)
 {
 	u8 i, reg;
 
-	xgifb_reg_set(XGISR, 0x05, 0x86);
+	xgifb_reg_set(xgifb_info->dev_info.P3c4, 0x05, 0x86);

This patch is OK, but it might be nicer to create a temporary variable so the lines are not so long:

	struct vb_device_info *vb = &xgifb_info->dev_info;
	u8 i, reg;

	xgifb_reg_set(vb->P3c4, 0x05, 0x86);

I chose "vb" based on the struct name... "dev" and "info" aren't very useful in a name because there are a lot of devices and lots of types of info.

regards,
dan carpneter

Josh Abraham

author

2017-12-01 13:15 UTC #2 lore raw

Quoted 20 lines · Dan Carpenter

drivers/staging/xgifb/XGI_main_26.c static void dumpVGAReg(struct xgifb_video_info *xgifb_info) · line 37
Quoted 10 lines · Joshua Abraham
drivers/staging/xgifb/XGI_main_26.c +1 -1
index 6feecc55d2bc..6de66eaad96b 100644
@@ -34,16 +34,16 @@ static void dumpVGAReg(struct xgifb_video_info *xgifb_info)
 {
 	u8 i, reg;
 
-	xgifb_reg_set(XGISR, 0x05, 0x86);
+	xgifb_reg_set(xgifb_info->dev_info.P3c4, 0x05, 0x86);
This patch is OK, but it might be nicer to create a temporary variable so the lines are not so long:
	struct vb_device_info *vb = &xgifb_info->dev_info;
	u8 i, reg;

	xgifb_reg_set(vb->P3c4, 0x05, 0x86);

Quoted 7 lines

I chose "vb" based on the struct name... "dev" and "info" aren't very useful in a name because there are a lot of devices and lots of types of info.
regards,
dan carpneter

That is indeed MUCH more readable. Thanks, I'll work on this today.

-Josh