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.
> @@ -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.
> +
> 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.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
