Re: [PATCH v4 02/10] net/ps3_gelic: Use local dev variable
Le 23/07/2021 à 22:31, Geoff Levand a écrit :
In an effort to make the PS3 gelic driver easier to maintain, add a
local variable dev to those routines that use the device structure that
makes the use the device structure more consistent.
Signed-off-by: Geoff Levand
Running checkpatch script provides the following feedback:
WARNING:BRACES: braces {} are not necessary for single statement blocks
#31: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:56:
+ if (status) {
+ dev_err(dev, "%s:%d failed: %d\n", __func__, __LINE__, status);
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#70: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:140:
+ if (status) {
+ dev_err(dev, "lv1_net_stop_tx_dma failed, status=%d\n", status);
+ }
WARNING:BRACES: braces {} are not necessary for single statement blocks
#111: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:191:
+ if (status) {
+ dev_err(dev, "lv1_net_stop_rx_dma failed, %d\n", status);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#170: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:301:
+ dma_unmap_single(dev, descr->link.cpu_addr, descr->link.size,
+ DMA_BIDIRECTIONAL);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#190: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:335:
+ dma_map_single(dev, descr, descr->link.size,
+ DMA_BIDIRECTIONAL);
WARNING:BRACES: braces {} are not necessary for single statement blocks
#213: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:387:
+ if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) {
+ dev_err(dev, "%s:%d: ERROR status\n", __func__, __LINE__);
+ }
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#226: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:413:
+ descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(dev,
descr->skb->data,
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#281: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:570:
+dev_info_ratelimited(dev,
+"%s:%d: forcing end of tx descriptor with
status %x\n",
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#414: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:843:
+ dev_info(dev, "%s:%d: lv1_net_start_txdma failed: %d\n",
+ __func__, __LINE__, status);
WARNING:VSPRINTF_SPECIFIER_PX: Using vsprintf specifier '%px' potentially exposes the kernel memory
layout, if you don't really need the address please consider using '%p'.
#480: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1006:
+ dev_dbg(dev, "%s:%d: dormant descr? %px\n", __func__, __LINE__,
+ descr);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#491: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1024:
+ dev_info(dev, "%s:%d: unknown packet vid=%x\n",
+ __func__, __LINE__, vid);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#502: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1034:
+ dev_info(dev, "%s:%d: dropping RX descriptor with state %x\n",
+ __func__, __LINE__, status);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#687: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1535:
+ dev_info(dev, "%s:%d: %s MAC addr %pxM\n", __func__, __LINE__,
+ netdev->name, netdev->dev_addr);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#797: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1710:
+ dev_info(dev, "%s:%d: gelic_net_alloc_card failed.\n", __func__,
+ __LINE__);
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#824: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1737:
+ result = ps3_sb_event_receive_port_setup(sb_dev, PS3_BINDING_CPU_ANY,
>irq);
WARNING:VSPRINTF_SPECIFIER_PX: Using vsprintf specifier '%px' potentially exposes the kernel memory
layout, if you don't really need the address please consider using '%p'.
#854: FILE: drivers/net/ethernet/toshiba/ps3_gelic_net.c:1773:
+ dev_dbg(dev, "%s:%d: descr rx %px, tx %px, size %#lx, num %#x\n",
+ __func__, __LINE__, card->rx_top, card->tx_top,
+ sizeof(struct gelic_descr), GELIC_NET_RX_DESCRIPTORS);
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 244665a969da ("net/ps3_gelic: Use local dev variable") has style
problems,
Re: [PATCH v4 02/10] net/ps3_gelic: Use local dev variable
Geoff Levand a écrit :
In an effort to make the PS3 gelic driver easier to maintain, add a
local variable dev to those routines that use the device structure that
makes the use the device structure more consistent.
Signed-off-by: Geoff Levand
---
drivers/net/ethernet/toshiba/ps3_gelic_net.c | 340 +++
1 file changed, 191 insertions(+), 149 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index cb45571573d7..ba008a98928a 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -48,13 +48,15 @@ MODULE_LICENSE("GPL");
/* set irq_mask */
int gelic_card_set_irq_mask(struct gelic_card *card, u64 mask)
{
+ struct device *dev = ctodev(card);
int status;
status = lv1_net_set_interrupt_mask(bus_id(card), dev_id(card),
mask, 0);
- if (status)
- dev_info(ctodev(card),
-"%s failed %d\n", __func__, status);
+ if (status) {
There shall be no { } for single line statements. And anyway this
change is not part of this patch as it is unrelated to the use of
local dev variable.
+ dev_err(dev, "%s:%d failed: %d\n", __func__, __LINE__, status);
+ }
+
return status;
}
@@ -103,6 +105,7 @@ gelic_descr_get_status(struct gelic_descr *descr)
static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
{
+ struct device *dev = ctodev(card);
int status;
u64 v1, v2;
@@ -110,8 +113,8 @@ static int gelic_card_set_link_mode(struct
gelic_card *card, int mode)
GELIC_LV1_SET_NEGOTIATION_MODE,
GELIC_LV1_PHY_ETHERNET_0, mode, 0, , );
if (status) {
- pr_info("%s: failed setting negotiation mode %d\n", __func__,
- status);
+ dev_err(dev, "%s:%d: Failed setting negotiation mode: %d\n",
+ __func__, __LINE__, status);
Changing from pr_info to dev_err is unrelated to the use of a dev local var
return -EBUSY;
}
@@ -128,13 +131,15 @@ static int gelic_card_set_link_mode(struct
gelic_card *card, int mode)
*/
static void gelic_card_disable_txdmac(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
int status;
/* this hvc blocks until the DMA in progress really stopped */
status = lv1_net_stop_tx_dma(bus_id(card), dev_id(card));
- if (status)
- dev_err(ctodev(card),
- "lv1_net_stop_tx_dma failed, status=%d\n", status);
+
+ if (status) {
+ dev_err(dev, "lv1_net_stop_tx_dma failed, status=%d\n", status);
+ }
}
/**
@@ -146,6 +151,7 @@ static void gelic_card_disable_txdmac(struct
gelic_card *card)
*/
static void gelic_card_enable_rxdmac(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
int status;
#ifdef DEBUG
@@ -161,9 +167,10 @@ static void gelic_card_enable_rxdmac(struct
gelic_card *card)
#endif
status = lv1_net_start_rx_dma(bus_id(card), dev_id(card),
card->rx_chain.head->link.cpu_addr, 0);
- if (status)
- dev_info(ctodev(card),
-"lv1_net_start_rx_dma failed, status=%d\n", status);
+ if (status) {
+ dev_err(dev, "lv1_net_start_rx_dma failed, status=%d\n",
+ status);
+ }
}
/**
@@ -175,13 +182,15 @@ static void gelic_card_enable_rxdmac(struct
gelic_card *card)
*/
static void gelic_card_disable_rxdmac(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
int status;
/* this hvc blocks until the DMA in progress really stopped */
status = lv1_net_stop_rx_dma(bus_id(card), dev_id(card));
- if (status)
- dev_err(ctodev(card),
- "lv1_net_stop_rx_dma failed, %d\n", status);
+
+ if (status) {
+ dev_err(dev, "lv1_net_stop_rx_dma failed, %d\n", status);
+ }
}
/**
@@ -235,10 +244,11 @@ static void gelic_card_reset_chain(struct
gelic_card *card,
void gelic_card_up(struct gelic_card *card)
{
- pr_debug("%s: called\n", __func__);
+ struct device *dev = ctodev(card);
+
mutex_lock(>updown_lock);
if (atomic_inc_return(>users) == 1) {
- pr_debug("%s: real do\n", __func__);
+ dev_dbg(dev, "%s:%d: Starting...\n", __func__, __LINE__);
/* enable irq */
gelic_card_set_irq_mask(card, card->irq_mask);
/* start rx */
@@ -247,16 +257,16 @@ void gelic_card_up(struct gelic_card *card)
napi_enable(>napi);
}
mutex_unlock(>updown_lock);
- pr_debug("%s: done\n", __func__);
}
void gelic_card_down(struct gelic_card *card)
[PATCH v4 02/10] net/ps3_gelic: Use local dev variable
In an effort to make the PS3 gelic driver easier to maintain, add a
local variable dev to those routines that use the device structure that
makes the use the device structure more consistent.
Signed-off-by: Geoff Levand
---
drivers/net/ethernet/toshiba/ps3_gelic_net.c | 340 +++
1 file changed, 191 insertions(+), 149 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index cb45571573d7..ba008a98928a 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -48,13 +48,15 @@ MODULE_LICENSE("GPL");
/* set irq_mask */
int gelic_card_set_irq_mask(struct gelic_card *card, u64 mask)
{
+ struct device *dev = ctodev(card);
int status;
status = lv1_net_set_interrupt_mask(bus_id(card), dev_id(card),
mask, 0);
- if (status)
- dev_info(ctodev(card),
-"%s failed %d\n", __func__, status);
+ if (status) {
+ dev_err(dev, "%s:%d failed: %d\n", __func__, __LINE__, status);
+ }
+
return status;
}
@@ -103,6 +105,7 @@ gelic_descr_get_status(struct gelic_descr *descr)
static int gelic_card_set_link_mode(struct gelic_card *card, int mode)
{
+ struct device *dev = ctodev(card);
int status;
u64 v1, v2;
@@ -110,8 +113,8 @@ static int gelic_card_set_link_mode(struct gelic_card
*card, int mode)
GELIC_LV1_SET_NEGOTIATION_MODE,
GELIC_LV1_PHY_ETHERNET_0, mode, 0, , );
if (status) {
- pr_info("%s: failed setting negotiation mode %d\n", __func__,
- status);
+ dev_err(dev, "%s:%d: Failed setting negotiation mode: %d\n",
+ __func__, __LINE__, status);
return -EBUSY;
}
@@ -128,13 +131,15 @@ static int gelic_card_set_link_mode(struct gelic_card
*card, int mode)
*/
static void gelic_card_disable_txdmac(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
int status;
/* this hvc blocks until the DMA in progress really stopped */
status = lv1_net_stop_tx_dma(bus_id(card), dev_id(card));
- if (status)
- dev_err(ctodev(card),
- "lv1_net_stop_tx_dma failed, status=%d\n", status);
+
+ if (status) {
+ dev_err(dev, "lv1_net_stop_tx_dma failed, status=%d\n", status);
+ }
}
/**
@@ -146,6 +151,7 @@ static void gelic_card_disable_txdmac(struct gelic_card
*card)
*/
static void gelic_card_enable_rxdmac(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
int status;
#ifdef DEBUG
@@ -161,9 +167,10 @@ static void gelic_card_enable_rxdmac(struct gelic_card
*card)
#endif
status = lv1_net_start_rx_dma(bus_id(card), dev_id(card),
card->rx_chain.head->link.cpu_addr, 0);
- if (status)
- dev_info(ctodev(card),
-"lv1_net_start_rx_dma failed, status=%d\n", status);
+ if (status) {
+ dev_err(dev, "lv1_net_start_rx_dma failed, status=%d\n",
+ status);
+ }
}
/**
@@ -175,13 +182,15 @@ static void gelic_card_enable_rxdmac(struct gelic_card
*card)
*/
static void gelic_card_disable_rxdmac(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
int status;
/* this hvc blocks until the DMA in progress really stopped */
status = lv1_net_stop_rx_dma(bus_id(card), dev_id(card));
- if (status)
- dev_err(ctodev(card),
- "lv1_net_stop_rx_dma failed, %d\n", status);
+
+ if (status) {
+ dev_err(dev, "lv1_net_stop_rx_dma failed, %d\n", status);
+ }
}
/**
@@ -235,10 +244,11 @@ static void gelic_card_reset_chain(struct gelic_card
*card,
void gelic_card_up(struct gelic_card *card)
{
- pr_debug("%s: called\n", __func__);
+ struct device *dev = ctodev(card);
+
mutex_lock(>updown_lock);
if (atomic_inc_return(>users) == 1) {
- pr_debug("%s: real do\n", __func__);
+ dev_dbg(dev, "%s:%d: Starting...\n", __func__, __LINE__);
/* enable irq */
gelic_card_set_irq_mask(card, card->irq_mask);
/* start rx */
@@ -247,16 +257,16 @@ void gelic_card_up(struct gelic_card *card)
napi_enable(>napi);
}
mutex_unlock(>updown_lock);
- pr_debug("%s: done\n", __func__);
}
void gelic_card_down(struct gelic_card *card)
{
+ struct device *dev = ctodev(card);
u64 mask;
- pr_debug("%s: called\n", __func__);
+
mutex_lock(>updown_lock);
if (atomic_dec_if_positive(>users) == 0) {
- pr_debug("%s: real do\n", __func__);
+
