Re: [PATCH 2/5] net: macb: Fix coding style warnings

[Nicolas Ferre]

Le 14/03/2016 21:53, Michal Simek a écrit :
> On 13.3.2016 20:10, Moritz Fischer wrote:
>> This commit takes care of the coding style warnings
>> that are mostly due to a different comment style and
>> lines over 80 chars, as well as a dangling else.
>>
>> Signed-off-by: Moritz Fischer 
>> ---
>>  drivers/net/ethernet/cadence/macb.c | 101 
>> +++-
>>  1 file changed, 43 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c 
>> b/drivers/net/ethernet/cadence/macb.c
>> index 4370f37..c2d31c5 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -58,8 +58,7 @@
>>  
>>  #define GEM_MTU_MIN_SIZE68
>>  
>> -/*
>> - * Graceful stop timeouts in us. We should allow up to
>> +/* Graceful stop timeouts in us. We should allow up to
>>   * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
>>   */
>>  #define MACB_HALT_TIMEOUT   1230
>> @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 
>> value)
>>  writel_relaxed(value, bp->regs + offset);
>>  }
>>  
>> -/*
>> - * Find the CPU endianness by using the loopback bit of NCR register. When 
>> the
>> +/* Find the CPU endianness by using the loopback bit of NCR register. When 
>> the
> 
> TBH: I would rather see this converting to kernel-doc format instead of
> using this networking block.

As there is hardly any kernel-doc comments in this driver, I won't force
Moritz to move this one to it.

I would advice, if someone want to move to kernel-doc for some function
comments, to do it in a separate patch (series).


> Also splitting this to more patches will be better. Just by categories
> but that's just my opinion.

Well, yes... but I won't be too picky for such a patch. So here is my:
Acked-by: Nicolas Ferre 

Thank for your feedback, bye,
-- 
Nicolas Ferre

Re: [PATCH 2/5] net: macb: Fix coding style warnings

[Michal Simek]

On 13.3.2016 20:10, Moritz Fischer wrote:
> This commit takes care of the coding style warnings
> that are mostly due to a different comment style and
> lines over 80 chars, as well as a dangling else.
> 
> Signed-off-by: Moritz Fischer 
> ---
>  drivers/net/ethernet/cadence/macb.c | 101 
> +++-
>  1 file changed, 43 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c 
> b/drivers/net/ethernet/cadence/macb.c
> index 4370f37..c2d31c5 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -58,8 +58,7 @@
>  
>  #define GEM_MTU_MIN_SIZE 68
>  
> -/*
> - * Graceful stop timeouts in us. We should allow up to
> +/* Graceful stop timeouts in us. We should allow up to
>   * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
>   */
>  #define MACB_HALT_TIMEOUT1230
> @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 
> value)
>   writel_relaxed(value, bp->regs + offset);
>  }
>  
> -/*
> - * Find the CPU endianness by using the loopback bit of NCR register. When 
> the
> +/* Find the CPU endianness by using the loopback bit of NCR register. When 
> the

TBH: I would rather see this converting to kernel-doc format instead of
using this networking block.

Also splitting this to more patches will be better. Just by categories
but that's just my opinion.

Thanks,
Michal

[PATCH 2/5] net: macb: Fix coding style warnings

[Moritz Fischer]

This commit takes care of the coding style warnings
that are mostly due to a different comment style and
lines over 80 chars, as well as a dangling else.

Signed-off-by: Moritz Fischer 
---
 drivers/net/ethernet/cadence/macb.c | 101 +++-
 1 file changed, 43 insertions(+), 58 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index 4370f37..c2d31c5 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -58,8 +58,7 @@
 
 #define GEM_MTU_MIN_SIZE   68
 
-/*
- * Graceful stop timeouts in us. We should allow up to
+/* Graceful stop timeouts in us. We should allow up to
  * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
  */
 #define MACB_HALT_TIMEOUT  1230
@@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 
value)
writel_relaxed(value, bp->regs + offset);
 }
 
-/*
- * Find the CPU endianness by using the loopback bit of NCR register. When the
+/* Find the CPU endianness by using the loopback bit of NCR register. When the
  * CPU is in big endian we need to program swaped mode for management
  * descriptor access.
  */
@@ -383,7 +381,8 @@ static int macb_mii_probe(struct net_device *dev)
 
pdata = dev_get_platdata(>pdev->dev);
if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
-   ret = devm_gpio_request(>pdev->dev, pdata->phy_irq_pin, 
"phy int");
+   ret = devm_gpio_request(>pdev->dev, pdata->phy_irq_pin,
+   "phy int");
if (!ret) {
phy_irq = gpio_to_irq(pdata->phy_irq_pin);
phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
@@ -449,7 +448,8 @@ static int macb_mii_init(struct macb *bp)
err = of_mdiobus_register(bp->mii_bus, np);
 
/* fallback to standard phy registration if no phy were
-  found during dt phy registration */
+* found during dt phy registration
+*/
if (!err && !phy_find_first(bp->mii_bus)) {
for (i = 0; i < PHY_MAX_ADDR; i++) {
struct phy_device *phydev;
@@ -564,8 +564,7 @@ static void macb_tx_error_task(struct work_struct *work)
/* Make sure nobody is trying to queue up new packets */
netif_tx_stop_all_queues(bp->dev);
 
-   /*
-* Stop transmission now
+   /* Stop transmission now
 * (in case we have just queued new packets)
 * macb/gem must be halted to write TBQP register
 */
@@ -573,8 +572,7 @@ static void macb_tx_error_task(struct work_struct *work)
/* Just complain for now, reinitializing TX path can be good */
netdev_err(bp->dev, "BUG: halt tx timed out\n");
 
-   /*
-* Treat frames in TX queue including the ones that caused the error.
+   /* Treat frames in TX queue including the ones that caused the error.
 * Free transmit buffers in upper layer.
 */
for (tail = queue->tx_tail; tail != queue->tx_head; tail++) {
@@ -604,10 +602,9 @@ static void macb_tx_error_task(struct work_struct *work)
bp->stats.tx_bytes += skb->len;
}
} else {
-   /*
-* "Buffers exhausted mid-frame" errors may only happen
-* if the driver is buggy, so complain loudly about 
those.
-* Statistics are updated by hardware.
+   /* "Buffers exhausted mid-frame" errors may only happen
+* if the driver is buggy, so complain loudly about
+* those. Statistics are updated by hardware.
 */
if (ctrl & MACB_BIT(TX_BUF_EXHAUSTED))
netdev_err(bp->dev,
@@ -719,7 +716,8 @@ static void gem_rx_refill(struct macb *bp)
struct sk_buff  *skb;
dma_addr_t  paddr;
 
-   while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail, RX_RING_SIZE) > 0) 
{
+   while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail,
+ RX_RING_SIZE) > 0) {
entry = macb_rx_ring_wrap(bp->rx_prepared_head);
 
/* Make hw descriptor updates visible to CPU */
@@ -738,7 +736,8 @@ static void gem_rx_refill(struct macb *bp)
 
/* now fill corresponding descriptor entry */
paddr = dma_map_single(>pdev->dev, skb->data,
-  bp->rx_buffer_size, 
DMA_FROM_DEVICE);
+  bp->rx_buffer_size,
+  DMA_FROM_DEVICE);
if (dma_mapping_error(>pdev->dev, paddr)) {