This patchset fixes some indentation- and style-related issues in qlge_main.c
reported by checkpatch.pl, such as:
WARNING: Avoid multiple line dereference
WARNING: line over 80 characters
WARNING: suspect code indent for conditional statements
v2:
- Addressed feedback from Joe Perches by unindenting
ql_set_mac_addr_reg and by replacing goto statements with break
statements in the function.
Rylan Dmello (7):
staging: qlge: Fix indentation in ql_set_mac_addr_reg
staging: qlge: Remove gotos from ql_set_mac_addr_reg
staging: qlge: Fix indentation in ql_get_mac_addr_reg
staging: qlge: Remove goto statements from ql_get_mac_addr_reg
staging: qlge: Remove multi-line dereference from ql_request_irq
staging: qlge: Fix suspect code indent warning in ql_init_device
staging: qlge: Fix function argument alignment warning in
ql_init_device
drivers/staging/qlge/qlge_main.c | 258 ++++++++++++++-----------------
1 file changed, 120 insertions(+), 138 deletions(-)
--
2.26.2
Based on Joe Perches' feedback, fix the indentation throughout
ql_set_mac_addr_reg. This helps fix several "line over 80 characters"
warnings along with the original "multiple line dereference" warning.
Fix checkpatch.pl warnings:
WARNING: Avoid multiple line dereference - prefer 'qdev->func'
WARNING: line over 80 characters
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 167 +++++++++++++++----------------
1 file changed, 78 insertions(+), 89 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index d7e4dfafc1a3..29610618c7c0 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -329,100 +329,89 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
int status = 0;
switch (type) {
- case MAC_ADDR_TYPE_MULTI_MAC:
- {
- u32 upper = (addr[0] << 8) | addr[1];
- u32 lower = (addr[2] << 24) | (addr[3] << 16) |
- (addr[4] << 8) | (addr[5]);
+ case MAC_ADDR_TYPE_MULTI_MAC: {
+ u32 upper = (addr[0] << 8) | addr[1];
+ u32 lower = (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) |
+ (addr[5]);
- status =
- ql_wait_reg_rdy(qdev,
- MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) |
- (index << MAC_ADDR_IDX_SHIFT) |
- type | MAC_ADDR_E);
- ql_write32(qdev, MAC_ADDR_DATA, lower);
- status =
- ql_wait_reg_rdy(qdev,
- MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) |
- (index << MAC_ADDR_IDX_SHIFT) |
- type | MAC_ADDR_E);
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | (index << MAC_ADDR_IDX_SHIFT) | type |
+ MAC_ADDR_E);
+ ql_write32(qdev, MAC_ADDR_DATA, lower);
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | (index << MAC_ADDR_IDX_SHIFT) | type |
+ MAC_ADDR_E);
- ql_write32(qdev, MAC_ADDR_DATA, upper);
- status =
- ql_wait_reg_rdy(qdev,
- MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- break;
- }
- case MAC_ADDR_TYPE_CAM_MAC:
- {
- u32 cam_output;
- u32 upper = (addr[0] << 8) | addr[1];
- u32 lower =
- (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) |
+ ql_write32(qdev, MAC_ADDR_DATA, upper);
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ break;
+ }
+ case MAC_ADDR_TYPE_CAM_MAC: {
+ u32 cam_output;
+ u32 upper = (addr[0] << 8) | addr[1];
+ u32 lower = (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) |
(addr[5]);
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) | /* offset */
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
- type); /* type */
- ql_write32(qdev, MAC_ADDR_DATA, lower);
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) | /* offset */
+ type); /* type */
+ ql_write32(qdev, MAC_ADDR_DATA, lower);
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
- type); /* type */
- ql_write32(qdev, MAC_ADDR_DATA, upper);
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset) | /* offset */
- (index << MAC_ADDR_IDX_SHIFT) | /* index */
- type); /* type */
- /* This field should also include the queue id
- * and possibly the function id. Right now we hardcode
- * the route field to NIC core.
- */
- cam_output = (CAM_OUT_ROUTE_NIC |
- (qdev->
- func << CAM_OUT_FUNC_SHIFT) |
- (0 << CAM_OUT_CQ_ID_SHIFT));
- if (qdev->ndev->features & NETIF_F_HW_VLAN_CTAG_RX)
- cam_output |= CAM_OUT_RV;
- /* route to NIC core */
- ql_write32(qdev, MAC_ADDR_DATA, cam_output);
- break;
- }
- case MAC_ADDR_TYPE_VLAN:
- {
- u32 enable_bit = *((u32 *) &addr[0]);
- /* For VLAN, the addr actually holds a bit that
- * either enables or disables the vlan id we are
- * addressing. It's either MAC_ADDR_E on or off.
- * That's bit-27 we're talking about.
- */
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, offset | /* offset */
- (index << MAC_ADDR_IDX_SHIFT) | /* index */
- type | /* type */
- enable_bit); /* enable/disable */
- break;
- }
+ type); /* type */
+ ql_write32(qdev, MAC_ADDR_DATA, upper);
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset) | /* offset */
+ (index << MAC_ADDR_IDX_SHIFT) | /* index */
+ type); /* type */
+ /* This field should also include the queue id
+ * and possibly the function id. Right now we hardcode
+ * the route field to NIC core.
+ */
+ cam_output = (CAM_OUT_ROUTE_NIC |
+ (qdev->func << CAM_OUT_FUNC_SHIFT) |
+ (0 << CAM_OUT_CQ_ID_SHIFT));
+ if (qdev->ndev->features & NETIF_F_HW_VLAN_CTAG_RX)
+ cam_output |= CAM_OUT_RV;
+ /* route to NIC core */
+ ql_write32(qdev, MAC_ADDR_DATA, cam_output);
+ break;
+ }
+ case MAC_ADDR_TYPE_VLAN: {
+ u32 enable_bit = *((u32 *)&addr[0]);
+ /* For VLAN, the addr actually holds a bit that
+ * either enables or disables the vlan id we are
+ * addressing. It's either MAC_ADDR_E on or off.
+ * That's bit-27 we're talking about.
+ */
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ offset | /* offset */
+ (index << MAC_ADDR_IDX_SHIFT) | /* index */
+ type | /* type */
+ enable_bit); /* enable/disable */
+ break;
+ }
case MAC_ADDR_TYPE_MULTI_FLTR:
default:
netif_crit(qdev, ifup, qdev->ndev,
--
2.26.2
As suggested by Joe Perches, this patch removes the 'exit' label
from the ql_set_mac_addr_reg function and replaces the goto
statements with break statements.
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 29610618c7c0..f2b4a54fc4c0 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -336,22 +336,20 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | (index << MAC_ADDR_IDX_SHIFT) | type |
MAC_ADDR_E);
ql_write32(qdev, MAC_ADDR_DATA, lower);
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | (index << MAC_ADDR_IDX_SHIFT) | type |
MAC_ADDR_E);
ql_write32(qdev, MAC_ADDR_DATA, upper);
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
break;
}
case MAC_ADDR_TYPE_CAM_MAC: {
@@ -361,7 +359,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
(addr[5]);
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
@@ -369,7 +367,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
ql_write32(qdev, MAC_ADDR_DATA, lower);
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
@@ -377,7 +375,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
ql_write32(qdev, MAC_ADDR_DATA, upper);
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
@@ -404,7 +402,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
*/
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
offset | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
@@ -418,7 +416,6 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
"Address type %d not yet supported.\n", type);
status = -EPERM;
}
-exit:
return status;
}
--
2.26.2
Similar to ql_set_mac_addr_reg, ql_get_mac_addr_reg also has several
goto statements that can be trivially replaced with a break statement.
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 844c2c6df38d..bb6c198a0130 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -265,7 +265,7 @@ int ql_get_mac_addr_reg(struct ql_adapter *qdev, u32 type, u16 index,
case MAC_ADDR_TYPE_CAM_MAC: {
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
@@ -273,11 +273,11 @@ int ql_get_mac_addr_reg(struct ql_adapter *qdev, u32 type, u16 index,
type); /* type */
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MR, 0);
if (status)
- goto exit;
+ break;
*value++ = ql_read32(qdev, MAC_ADDR_DATA);
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
@@ -285,13 +285,13 @@ int ql_get_mac_addr_reg(struct ql_adapter *qdev, u32 type, u16 index,
type); /* type */
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MR, 0);
if (status)
- goto exit;
+ break;
*value++ = ql_read32(qdev, MAC_ADDR_DATA);
if (type == MAC_ADDR_TYPE_CAM_MAC) {
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX,
MAC_ADDR_MW, 0);
if (status)
- goto exit;
+ break;
ql_write32(qdev, MAC_ADDR_IDX,
(offset++) | /* offset */
(index
@@ -301,7 +301,7 @@ int ql_get_mac_addr_reg(struct ql_adapter *qdev, u32 type, u16 index,
status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX,
MAC_ADDR_MR, 0);
if (status)
- goto exit;
+ break;
*value++ = ql_read32(qdev, MAC_ADDR_DATA);
}
break;
@@ -313,7 +313,6 @@ int ql_get_mac_addr_reg(struct ql_adapter *qdev, u32 type, u16 index,
"Address type %d not yet supported.\n", type);
status = -EPERM;
}
-exit:
return status;
}
--
2.26.2
Fix checkpatch.pl warning:
WARNING: Avoid multiple line dereference - prefer 'qdev->flags'
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index bb6c198a0130..9aa62d146d97 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -3415,9 +3415,9 @@ static int ql_request_irq(struct ql_adapter *qdev)
&qdev->rx_ring[0]);
status =
request_irq(pdev->irq, qlge_isr,
- test_bit(QL_MSI_ENABLED,
- &qdev->
- flags) ? 0 : IRQF_SHARED,
+ test_bit(QL_MSI_ENABLED, &qdev->flags)
+ ? 0
+ : IRQF_SHARED,
intr_context->name, &qdev->rx_ring[0]);
if (status)
goto err_irq;
--
2.26.2
Fix checkpatch.pl check:
CHECK: Alignment should match open parenthesis
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index fa708c722033..93df4f79b21d 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -4433,8 +4433,7 @@ static int ql_init_device(struct pci_dev *pdev, struct net_device *ndev,
pdev->needs_freset = 1;
pci_save_state(pdev);
qdev->reg_base =
- ioremap(pci_resource_start(pdev, 1),
- pci_resource_len(pdev, 1));
+ ioremap(pci_resource_start(pdev, 1), pci_resource_len(pdev, 1));
if (!qdev->reg_base) {
dev_err(&pdev->dev, "Register mapping failed.\n");
err = -ENOMEM;
@@ -4443,8 +4442,7 @@ static int ql_init_device(struct pci_dev *pdev, struct net_device *ndev,
qdev->doorbell_area_size = pci_resource_len(pdev, 3);
qdev->doorbell_area =
- ioremap(pci_resource_start(pdev, 3),
- pci_resource_len(pdev, 3));
+ ioremap(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3));
if (!qdev->doorbell_area) {
dev_err(&pdev->dev, "Doorbell register mapping failed.\n");
err = -ENOMEM;
--
2.26.2
This has similar indentation style issues as ql_set_mac_addr_reg, so I
thought I'd re-indent this too.
Fix several checkpatch.pl warnings:
WARNING: line over 80 characters
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 76 ++++++++++++++++----------------
1 file changed, 37 insertions(+), 39 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index f2b4a54fc4c0..844c2c6df38d 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -262,52 +262,50 @@ int ql_get_mac_addr_reg(struct ql_adapter *qdev, u32 type, u16 index,
switch (type) {
case MAC_ADDR_TYPE_MULTI_MAC:
- case MAC_ADDR_TYPE_CAM_MAC:
- {
- status =
- ql_wait_reg_rdy(qdev,
- MAC_ADDR_IDX, MAC_ADDR_MW, 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) | /* offset */
+ case MAC_ADDR_TYPE_CAM_MAC: {
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | /* offset */
(index << MAC_ADDR_IDX_SHIFT) | /* index */
- MAC_ADDR_ADR | MAC_ADDR_RS | type); /* type */
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MR, 0);
- if (status)
- goto exit;
- *value++ = ql_read32(qdev, MAC_ADDR_DATA);
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ MAC_ADDR_ADR | MAC_ADDR_RS |
+ type); /* type */
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MR, 0);
+ if (status)
+ goto exit;
+ *value++ = ql_read32(qdev, MAC_ADDR_DATA);
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
+ if (status)
+ goto exit;
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | /* offset */
+ (index << MAC_ADDR_IDX_SHIFT) | /* index */
+ MAC_ADDR_ADR | MAC_ADDR_RS |
+ type); /* type */
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MR, 0);
+ if (status)
+ goto exit;
+ *value++ = ql_read32(qdev, MAC_ADDR_DATA);
+ if (type == MAC_ADDR_TYPE_CAM_MAC) {
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX,
+ MAC_ADDR_MW, 0);
if (status)
goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) | /* offset */
- (index << MAC_ADDR_IDX_SHIFT) | /* index */
- MAC_ADDR_ADR | MAC_ADDR_RS | type); /* type */
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MR, 0);
+ ql_write32(qdev, MAC_ADDR_IDX,
+ (offset++) | /* offset */
+ (index
+ << MAC_ADDR_IDX_SHIFT) | /* index */
+ MAC_ADDR_ADR |
+ MAC_ADDR_RS | type); /* type */
+ status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX,
+ MAC_ADDR_MR, 0);
if (status)
goto exit;
*value++ = ql_read32(qdev, MAC_ADDR_DATA);
- if (type == MAC_ADDR_TYPE_CAM_MAC) {
- status =
- ql_wait_reg_rdy(qdev,
- MAC_ADDR_IDX, MAC_ADDR_MW,
- 0);
- if (status)
- goto exit;
- ql_write32(qdev, MAC_ADDR_IDX, (offset++) | /* offset */
- (index << MAC_ADDR_IDX_SHIFT) | /* index */
- MAC_ADDR_ADR | MAC_ADDR_RS | type); /* type */
- status =
- ql_wait_reg_rdy(qdev, MAC_ADDR_IDX,
- MAC_ADDR_MR, 0);
- if (status)
- goto exit;
- *value++ = ql_read32(qdev, MAC_ADDR_DATA);
- }
- break;
}
+ break;
+ }
case MAC_ADDR_TYPE_VLAN:
case MAC_ADDR_TYPE_MULTI_FLTR:
default:
--
2.26.2
Fix checkpatch.pl warnings:
WARNING: suspect code indent for conditional statements (16, 23)
WARNING: line over 80 characters
Signed-off-by: Rylan Dmello <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 9aa62d146d97..fa708c722033 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -4420,7 +4420,8 @@ static int ql_init_device(struct pci_dev *pdev, struct net_device *ndev,
} else {
err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
if (!err)
- err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+ err = dma_set_coherent_mask(&pdev->dev,
+ DMA_BIT_MASK(32));
}
if (err) {
--
2.26.2
On Wed, 2020-04-29 at 21:31 -0400, Rylan Dmello wrote:
> This patchset fixes some indentation- and style-related issues in qlge_main.c
> reported by checkpatch.pl, such as:
>
> WARNING: Avoid multiple line dereference
> WARNING: line over 80 characters
> WARNING: suspect code indent for conditional statements
All of this looks reasonable to me.
On Wed, Apr 29, 2020 at 09:33:04PM -0400, Rylan Dmello wrote:
> As suggested by Joe Perches, this patch removes the 'exit' label
> from the ql_set_mac_addr_reg function and replaces the goto
> statements with break statements.
>
> Signed-off-by: Rylan Dmello <[email protected]>
> ---
> drivers/staging/qlge/qlge_main.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
> index 29610618c7c0..f2b4a54fc4c0 100644
> --- a/drivers/staging/qlge/qlge_main.c
> +++ b/drivers/staging/qlge/qlge_main.c
> @@ -336,22 +336,20 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
>
> status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
> if (status)
> - goto exit;
> + break;
Just "return status". A direct return is immediately clear but with a
break statement then you have to look down a bit and then scroll back.
regards,
dan carpenter
On Thu, 2020-04-30 at 12:38 +0300, Dan Carpenter wrote:
> On Wed, Apr 29, 2020 at 09:33:04PM -0400, Rylan Dmello wrote:
> > As suggested by Joe Perches, this patch removes the 'exit' label
> > from the ql_set_mac_addr_reg function and replaces the goto
> > statements with break statements.
[]
> > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
[]
> > @@ -336,22 +336,20 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
> >
> > status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
> > if (status)
> > - goto exit;
> > + break;
>
> Just "return status". A direct return is immediately clear but with a
> break statement then you have to look down a bit and then scroll back.
To me, 6 of 1, half dozen of other as
all the case breaks could be returns.
So either form is fine with me.
The old form was poor through.
On Thu, Apr 30, 2020 at 03:03:07AM -0700, Joe Perches wrote:
> On Thu, 2020-04-30 at 12:38 +0300, Dan Carpenter wrote:
> > On Wed, Apr 29, 2020 at 09:33:04PM -0400, Rylan Dmello wrote:
> > > As suggested by Joe Perches, this patch removes the 'exit' label
> > > from the ql_set_mac_addr_reg function and replaces the goto
> > > statements with break statements.
> []
> > > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
> []
> > > @@ -336,22 +336,20 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
> > >
> > > status = ql_wait_reg_rdy(qdev, MAC_ADDR_IDX, MAC_ADDR_MW, 0);
> > > if (status)
> > > - goto exit;
> > > + break;
> >
> > Just "return status". A direct return is immediately clear but with a
> > break statement then you have to look down a bit and then scroll back.
>
> To me, 6 of 1, half dozen of other as
> all the case breaks could be returns.
>
> So either form is fine with me.
>
> The old form was poor through.
With a goto exit or a break you have to scroll down to exactly the same
place. There is no difference at all.
Anyway, I'm actually fine with this patch series as-is. It improves
a whole lot of stuff and doesn't cause any problems which weren't
there to begin with.
Reviewed-by: Dan Carpenter <[email protected]>
regards,
dan carpenter