On Fri, Nov 22, 2013 at 4:08 PM, Dan Carpenter <[email protected]> wrote:
> On Fri, Nov 22, 2013 at 10:54:27AM +0800, ZHAO Gang wrote:
>> 1. change function name: et1310_phy_power_down -> et1310_phy_power_switch
>> change function name to better describe its functionality.
>>
>> 2. as TODO file suggested, do this sort of things to reduce split lines
>> struct fbr_lookup *fbr;
>> fbr = rx_local->fbr[id];
>> Then replace all the instances of "rx_local->fbr[id]" with fbr.
>>
>> 3. delete unnecessary variable in function et131x_init
>> variable u32 numrfd is not necessary in this function.
>>
>> 4. some code style change
>
> This should be 4 patches. One for each. The first 3 versions of this
> patch weren't sent to the list or we would have said that already.
I can split this patch to four, since this is the first patch of a
patch set, I will resend the entire patch set.
>
>> @@ -1822,6 +1822,9 @@ static void et131x_config_rx_dma_regs(struct
>> et131x_adapter *adapter)
>> u32 __iomem *base_hi;
>> u32 __iomem *base_lo;
>>
>> + struct fbr_lookup *fbr;
>> + fbr = rx_local->fbr[id];
>
> The blank lines are in the wrong place. The variable declarations go in
> one block with a blank after.
I will fix this.
>
>> +
>> if (id == 0) {
>> num_des = &rx_dma->fbr0_num_des;
>> full_offset = &rx_dma->fbr0_full_offset;
>
> Otherwise this patch seems nice but it needs to be split up and resent.
By the way, this mailing list is surely an open list, but I can't find
how to subscribe it - the website at driverdev.osuosl.org said it has
no publicly-advertised Mailman mailing lists.
Another question: if I send patches to this list, should I also cc
linux-kernel mailing list?
Thanks for your review.
>
> regards,
> dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
