lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 27 Aug 2016 15:42:42 -0500
From:   Timur Tabi <timur@...eaurora.org>
To:     Rami Rosen <roszenrami@...il.com>
Cc:     Netdev <netdev@...r.kernel.org>, devicetree@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, sdharia@...eaurora.org,
        shankerd@...eaurora.org, vikrams@...eaurora.org,
        cov@...eaurora.org, gavidov@...eaurora.org, robh+dt@...nel.org,
        andrew@...n.ch, bjorn.andersson@...aro.org, mlangsdo@...hat.com,
        jcm@...hat.com, agross@...eaurora.org,
        David Miller <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>, LinoSanfilippo@....de
Subject: Re: [PATCH] [v9] net: emac: emac gigabit ethernet controller driver

Rami Rosen wrote:
> Hi,
>
> Shouldn't it be obj-$(CONFIG_QCOM_EMAC) instead obj-y?

Yes, I noticed this soon after I posted this patch.  I will fix it in v10.

> The return type of the method emac_tx_tpd_create()  should be void, as
> it always returns true.

Will fix, thanks.

> Seems that there are several unused members in the emac_stats struct:
>
>> +struct emac_stats {
> ...
> ...
> Both rx_bcast_byte_cnt and rx_mcast_byte_cnt are not used anywhere/
>> +       u64 rx_bcast_byte_cnt;  /* broadcast packets byte count (without FCS) */
>> +       u64 rx_mcast_byte_cnt;  /* multicast packets byte count (without FCS) */
> ...
> rx_err_addr is not used
>> +       u64 rx_err_addr;        /* packets dropped due to address filtering */

I'll go through the structure and remove the unused fields.

>
> Typo: it is jabbers and not jubbers; greppping for "jubber" under
> drivers/net/ethernet gives
> 0 results, whereas:
> /net-next/drivers/net/ethernet$ grep -R jabber | wc -l
> gives 241 results.
>
>> +       u64 rx_jubbers;         /* jubbers */

Thanks.  I'll fix that in v10.

>
>
> ...
> ..
> tx_rd_eop and tx_len_err are not used
>> +       u64 tx_rd_eop;          /* count of reads beyond EOP */
>> +       u64 tx_len_err;         /* packets with length mismatch */
>
> Both tx_bcast_byte and tx_mcast_byte are not used.
>> +       u64 tx_bcast_byte;      /* broadcast packets byte count (without FCS) */
>> +       u64 tx_mcast_byte;      /* multicast packets byte count (without FCS) */
>
> Thanks for your good work, it seems that the driver is almost done!

I'm grateful for the opportunity to improve this driver over the crappy 
one that's been bouncing around internally for years, and for all the 
people who helped me along the way.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Powered by blists - more mailing lists