On 10 September 2011 06:41, Joe Perches <joe@xxxxxxxxxxx> wrote:
> On Sat, 2011-09-03 at 12:49 +0100, Mark Einon wrote:
>> Some of my previous hacking attempts have not been following the rules.
>> All fixes either lines > 80 chars or whitespace corrections (spaces->tabs etc).
> []
>> diff --git a/drivers/staging/et131x/et1310_mac.c b/drivers/staging/et131x/et1310_mac.c
>> index ab85cb3..36f2168 100644
>> --- a/drivers/staging/et131x/et1310_mac.c
>> +++ b/drivers/staging/et131x/et1310_mac.c
>> @@ -191,7 +191,8 @@ void et1310_config_mac_regs2(struct et131x_adapter *adapter)
>> cfg1 |= CFG1_RX_ENABLE | CFG1_TX_ENABLE | CFG1_TX_FLOW;
>> /* Initialize loop back to off */
>> cfg1 &= ~(CFG1_LOOPBACK | CFG1_RX_FLOW);
>> - if (adapter->flowcontrol == FLOW_RXONLY || adapter->flowcontrol == FLOW_BOTH)
>> + if (adapter->flowcontrol == FLOW_RXONLY ||
>> + adapter->flowcontrol == FLOW_BOTH)
>
> Especially in cases like this, it helps readability
> to align to open parenthesis like:
>
> if (adapter->flowcontrol == FLOW_RXONLY ||
> adapter->flowcontrol == FLOW_BOTH)
>
Thanks, noted. I'll bear it in mind for next time...
> []
>> @@ -641,7 +641,8 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
>> * they point to
>> */
>> do {
>> - desc = (struct tx_desc *)(adapter->tx_ring.tx_desc_ring +
>> + desc = (struct tx_desc *)
>> + (adapter->tx_ring.tx_desc_ring +
>> INDEX10(tcb->index_start));
>
> Isn't this an unnecessary cast?
> Perhaps it's better as:
>
> u32 i10 = INDEX10(tcb->index_start);
>
> desc = &adapter->tx_ring.tx_desc_ring[i10];
That sounds like a good idea. I'll add it to the list when I go
through the tx/rx code for the driver.
Cheers,
Mark
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
