Re: [PATCH v3 2/2] net/ps3_gelic: Cleanups, improve logging
Geoff Levand a écrit :
General source cleanups and improved logging messages.
Describe a bit more what you do to cleanup and improve.
Some of your changes are not cleanup , they increase the mess.
You should read kernel coding style
Signed-off-by: Geoff Levand
---
drivers/net/ethernet/toshiba/ps3_gelic_net.c | 395 ++-
1 file changed, 216 insertions(+), 179 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index e01938128882..9dbcb7c4ec80 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -44,17 +44,17 @@ MODULE_AUTHOR("SCE Inc.");
MODULE_DESCRIPTION("Gelic Network driver");
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) {
No { } for single lines
+ dev_err(dev, "%s:%d failed: %d\n", __func__, __LINE__, status);
+ }
+
return status;
}
@@ -63,6 +63,7 @@ static void gelic_card_rx_irq_on(struct gelic_card *card)
card->irq_mask |= GELIC_CARD_RXINT;
gelic_card_set_irq_mask(card, card->irq_mask);
}
+
static void gelic_card_rx_irq_off(struct gelic_card *card)
{
card->irq_mask &= ~GELIC_CARD_RXINT;
@@ -70,15 +71,14 @@ static void gelic_card_rx_irq_off(struct
gelic_card *card)
}
static void gelic_card_get_ether_port_status(struct gelic_card *card,
-int inform)
+ int inform)
Bad indent
{
u64 v2;
struct net_device *ether_netdev;
lv1_net_control(bus_id(card), dev_id(card),
- GELIC_LV1_GET_ETH_PORT_STATUS,
- GELIC_LV1_VLAN_TX_ETHERNET_0, 0, 0,
- >ether_port_status, );
+ GELIC_LV1_GET_ETH_PORT_STATUS, GELIC_LV1_VLAN_TX_ETHERNET_0, 0,
+ 0, >ether_port_status, );
Bad indent
if (inform) {
ether_netdev = card->netdev[GELIC_PORT_ETHERNET_0];
@@ -105,15 +105,17 @@ 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;
status = lv1_net_control(bus_id(card), dev_id(card),
-GELIC_LV1_SET_NEGOTIATION_MODE,
-GELIC_LV1_PHY_ETHERNET_0, mode, 0, , );
+ 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;
}
@@ -130,13 +132,16 @@ 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, "%s:%d: lv1_net_stop_tx_dma failed: %d\n",
+ __func__, __LINE__, status);
+ }
}
/**
@@ -187,13 +192,16 @@ 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, "%s:%d: lv1_net_stop_rx_dma failed: %d\n",
+ __func__, __LINE__, status);
+ }
}
/**
@@ -216,6 +224,7 @@ static void gelic_descr_set_status(struct
gelic_descr *descr,
* Usually caller of this function wants to inform that to the
* hardware, so we assure here the hardware sees the change.
*/
+
Bad blank line, it separates the comment from the commentee
wmb();
}
@@ -229,8 +238,7 @@ static void gelic_descr_set_status(struct
gelic_descr *descr,
* and re-initialize the hardware chain
[PATCH v3 2/2] net/ps3_gelic: Cleanups, improve logging
General source cleanups and improved logging messages.
Signed-off-by: Geoff Levand
---
drivers/net/ethernet/toshiba/ps3_gelic_net.c | 395 ++-
1 file changed, 216 insertions(+), 179 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index e01938128882..9dbcb7c4ec80 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -44,17 +44,17 @@ MODULE_AUTHOR("SCE Inc.");
MODULE_DESCRIPTION("Gelic Network driver");
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;
}
@@ -63,6 +63,7 @@ static void gelic_card_rx_irq_on(struct gelic_card *card)
card->irq_mask |= GELIC_CARD_RXINT;
gelic_card_set_irq_mask(card, card->irq_mask);
}
+
static void gelic_card_rx_irq_off(struct gelic_card *card)
{
card->irq_mask &= ~GELIC_CARD_RXINT;
@@ -70,15 +71,14 @@ static void gelic_card_rx_irq_off(struct gelic_card *card)
}
static void gelic_card_get_ether_port_status(struct gelic_card *card,
-int inform)
+ int inform)
{
u64 v2;
struct net_device *ether_netdev;
lv1_net_control(bus_id(card), dev_id(card),
- GELIC_LV1_GET_ETH_PORT_STATUS,
- GELIC_LV1_VLAN_TX_ETHERNET_0, 0, 0,
- >ether_port_status, );
+ GELIC_LV1_GET_ETH_PORT_STATUS, GELIC_LV1_VLAN_TX_ETHERNET_0, 0,
+ 0, >ether_port_status, );
if (inform) {
ether_netdev = card->netdev[GELIC_PORT_ETHERNET_0];
@@ -105,15 +105,17 @@ 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;
status = lv1_net_control(bus_id(card), dev_id(card),
-GELIC_LV1_SET_NEGOTIATION_MODE,
-GELIC_LV1_PHY_ETHERNET_0, mode, 0, , );
+ 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;
}
@@ -130,13 +132,16 @@ 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, "%s:%d: lv1_net_stop_tx_dma failed: %d\n",
+ __func__, __LINE__, status);
+ }
}
/**
@@ -187,13 +192,16 @@ 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, "%s:%d: lv1_net_stop_rx_dma failed: %d\n",
+ __func__, __LINE__, status);
+ }
}
/**
@@ -216,6 +224,7 @@ static void gelic_descr_set_status(struct gelic_descr
*descr,
* Usually caller of this function wants to inform that to the
* hardware, so we assure here the hardware sees the change.
*/
+
wmb();
}
@@ -229,8 +238,7 @@ static void gelic_descr_set_status(struct gelic_descr
*descr,
* and re-initialize the hardware chain for later use
*/
static void gelic_card_reset_chain(struct gelic_card *card,
- struct gelic_descr_chain *chain,
- struct gelic_descr *start_descr)
+ struct gelic_descr_chain *chain, struct gelic_descr *start_descr)
{
struct
