netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Timur Tabi <timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
To: Rami Rosen <roszenrami-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Netdev <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	sdharia-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	vikrams-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	cov-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	gavidov-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	andrew-g2DYL2Zd6BY@public.gmane.org,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	mlangsdo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	Florian Fainelli
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	LinoSanfilippo-Mmb7MZpHnFY@public.gmane.org
Subject: Re: [PATCH] [v9] net: emac: emac gigabit ethernet controller driver
Date: Sat, 27 Aug 2016 15:42:42 -0500	[thread overview]
Message-ID: <57C1FB42.104@codeaurora.org> (raw)
In-Reply-To: <CAKoUAr=Z56VxzW=zHu7r9ossdXHQ6xpeYdR5jwhv38zkV_eKFQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2016-08-27 20:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-25 21:39 [PATCH] [v9] net: emac: emac gigabit ethernet controller driver Timur Tabi
2016-08-27  4:29 ` David Miller
2016-08-27 12:26   ` Timur Tabi
2016-08-27 17:58     ` Florian Fainelli
2016-08-28  4:50     ` David Miller
2016-08-27 20:26 ` Rami Rosen
     [not found]   ` <CAKoUAr=Z56VxzW=zHu7r9ossdXHQ6xpeYdR5jwhv38zkV_eKFQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-08-27 20:42     ` Timur Tabi [this message]
2016-08-31 18:57       ` Timur Tabi
2016-08-31 19:15         ` Florian Fainelli
     [not found]           ` <60d13549-f9ea-694b-1030-0c610e0d9722-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-08-31 19:19             ` Timur Tabi
     [not found] ` <1472161143-26417-1-git-send-email-timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-08-31 15:00   ` Rob Herring
2016-08-31 15:11     ` Timur Tabi
2016-08-31 20:46       ` Rob Herring
     [not found]         ` <CAL_Jsq+BtwBtB7Vd8HB+vhucfUmn_0bxRRXmyO13GoCM82LgFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-08-31 21:00           ` Timur Tabi
     [not found]             ` <57C74562.3050203-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-08-31 21:16               ` Andrew Lunn
2016-09-01 22:49             ` Rob Herring
2016-08-31 15:24     ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57C1FB42.104@codeaurora.org \
    --to=timur-sgv2jx0feol9jmxxk+q4oq@public.gmane.org \
    --cc=LinoSanfilippo-Mmb7MZpHnFY@public.gmane.org \
    --cc=agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=andrew-g2DYL2Zd6BY@public.gmane.org \
    --cc=bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=cov-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=gavidov-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mlangsdo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=roszenrami-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=sdharia-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=vikrams-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).