From: Paul Gortmaker <paul.gortmaker@windriver.com> To: Mahesh Bandewar <maheshb@google.com> Cc: Noam Camus <noamc@ezchip.com>, linux-netdev <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <Alexey.Brodkin@synopsys.com>, <vgupta@synopsys.com>, <giladb@ezchip.com>, <cmetcalf@ezchip.com>, Tal Zilcer <talz@ezchip.com> Subject: Re: [PATCH v5] NET: Add ezchip ethernet driver Date: Mon, 22 Jun 2015 19:47:51 -0400 [thread overview] Message-ID: <20150622234749.GA21581@windriver.com> (raw) In-Reply-To: <CAF2d9jgQRatz_wL5+xk7fH=gAXTixb1yNVZGnenmKwR9RoSQLQ@mail.gmail.com> [Re: [PATCH v5] NET: Add ezchip ethernet driver] On 22/06/2015 (Mon 10:45) Mahesh Bandewar wrote: > On Tue, Jun 16, 2015 at 7:35 AM, Noam Camus <noamc@ezchip.com> wrote: > > > > From: Noam Camus <noamc@ezchip.com> > > > > Simple LAN device for debug or management purposes. > > Device supports interrupts for RX and TX(completion). > > Device does not have DMA ability. > > > > Signed-off-by: Noam Camus <noamc@ezchip.com> > > Signed-off-by: Tal Zilcer <talz@ezchip.com> > > Acked-by: Alexey Brodkin <abrodkin@synopsys.com> > > --- > > Change log for v5: > > Basically its all based on Florian comments. > > Main items are: > > 1) Move all interrupt chore to bottom-half > > 2) use memcpy_toio/fromio > > 3) dev_kfree_skb() moved to bottom-half > > 4) add set_rx_mode callback > > 5) use platform api toward non-DT platforms > > --- [...] > > diff --git a/drivers/net/ethernet/ezchip/Kconfig b/drivers/net/ethernet/ezchip/Kconfig > > new file mode 100644 > > index 0000000..d031177 > > --- /dev/null > > +++ b/drivers/net/ethernet/ezchip/Kconfig > > @@ -0,0 +1,27 @@ > > +# > > +# EZchip network device configuration > > +# > > + > > +config NET_VENDOR_EZCHIP > > + bool "EZchip devices" > > + default y > > > Why this has to be included in the default build? Shouldn't it be > either "m" or "N" by default? > Mahesh, in the future please don't quote 1200 lines of code in its entirety just to add these two lines of comment, which in this case happen to also be incorrect. It is a NET_VENDOR_<name> option which is just the lead in as it clearly states below if we go look... > > + ---help--- > > + If you have a network (Ethernet) device belonging to this class, say Y > > + and read the Ethernet-HOWTO, available from > > + <http://www.tldp.org/docs.html#howto>. Noam, since you are unlikely to get this in final from before net-next closes (happening any time now), please delete the Howto reference when you resubmit in two weeks, assuming that my cleanup here: https://patchwork.ozlabs.org/patch/487080/ gets applied to this release. > > + > > + Note that the answer to this question doesn't directly affect the > > + kernel: saying N will just cause the configurator to skip all > > + the questions about EZchip devices. If you say Y, you will be asked for > > + your specific device in the following questions. Mahesh, here above. So he's not doing a "default y" on an actual driver (which, if he were, yes that would be wrong.) What worries me more though, is that ... > > + > > +if NET_VENDOR_EZCHIP > > + > > +config EZCHIP_NPS_MANAGEMENT_ENET > > + tristate "EZchip NPS management enet support" ...there is no dependency line here. None. Noam - this implicitly means that you expect this to build on all arch for all combinations of .config files. At a quick glance I see a dts fragment, and I also see a call to of_get_mac_address() so I'm thinking there needs to be some dependency here, on OF or OF_NET, or... ? Paul. -- > > + ---help--- > > + Simple LAN device for debug or management purposes. > > + Device supports interrupts for RX and TX(completion). > > + Device does not have DMA ability. > > + > > +endif > > diff --git a/drivers/net/ethernet/ezchip/Makefile b/drivers/net/ethernet/ezchip/Makefile > > new file mode 100644 > > index 0000000..e490176 > > --- /dev/null > > +++ b/drivers/net/ethernet/ezchip/Makefile [snip] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/
WARNING: multiple messages have this Message-ID (diff)
From: Paul Gortmaker <paul.gortmaker@windriver.com> To: Mahesh Bandewar <maheshb@google.com> Cc: Noam Camus <noamc@ezchip.com>, linux-netdev <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <Alexey.Brodkin@synopsys.com>, <vgupta@synopsys.com>, <giladb@ezchip.com>, <cmetcalf@ezchip.com>, Tal Zilcer <talz@ezchip.com> Subject: Re: [PATCH v5] NET: Add ezchip ethernet driver Date: Mon, 22 Jun 2015 19:47:51 -0400 [thread overview] Message-ID: <20150622234749.GA21581@windriver.com> (raw) In-Reply-To: <CAF2d9jgQRatz_wL5+xk7fH=gAXTixb1yNVZGnenmKwR9RoSQLQ@mail.gmail.com> [Re: [PATCH v5] NET: Add ezchip ethernet driver] On 22/06/2015 (Mon 10:45) Mahesh Bandewar wrote: > On Tue, Jun 16, 2015 at 7:35 AM, Noam Camus <noamc@ezchip.com> wrote: > > > > From: Noam Camus <noamc@ezchip.com> > > > > Simple LAN device for debug or management purposes. > > Device supports interrupts for RX and TX(completion). > > Device does not have DMA ability. > > > > Signed-off-by: Noam Camus <noamc@ezchip.com> > > Signed-off-by: Tal Zilcer <talz@ezchip.com> > > Acked-by: Alexey Brodkin <abrodkin@synopsys.com> > > --- > > Change log for v5: > > Basically its all based on Florian comments. > > Main items are: > > 1) Move all interrupt chore to bottom-half > > 2) use memcpy_toio/fromio > > 3) dev_kfree_skb() moved to bottom-half > > 4) add set_rx_mode callback > > 5) use platform api toward non-DT platforms > > --- [...] > > diff --git a/drivers/net/ethernet/ezchip/Kconfig b/drivers/net/ethernet/ezchip/Kconfig > > new file mode 100644 > > index 0000000..d031177 > > --- /dev/null > > +++ b/drivers/net/ethernet/ezchip/Kconfig > > @@ -0,0 +1,27 @@ > > +# > > +# EZchip network device configuration > > +# > > + > > +config NET_VENDOR_EZCHIP > > + bool "EZchip devices" > > + default y > > > Why this has to be included in the default build? Shouldn't it be > either "m" or "N" by default? > Mahesh, in the future please don't quote 1200 lines of code in its entirety just to add these two lines of comment, which in this case happen to also be incorrect. It is a NET_VENDOR_<name> option which is just the lead in as it clearly states below if we go look... > > + ---help--- > > + If you have a network (Ethernet) device belonging to this class, say Y > > + and read the Ethernet-HOWTO, available from > > + <http://www.tldp.org/docs.html#howto>. Noam, since you are unlikely to get this in final from before net-next closes (happening any time now), please delete the Howto reference when you resubmit in two weeks, assuming that my cleanup here: https://patchwork.ozlabs.org/patch/487080/ gets applied to this release. > > + > > + Note that the answer to this question doesn't directly affect the > > + kernel: saying N will just cause the configurator to skip all > > + the questions about EZchip devices. If you say Y, you will be asked for > > + your specific device in the following questions. Mahesh, here above. So he's not doing a "default y" on an actual driver (which, if he were, yes that would be wrong.) What worries me more though, is that ... > > + > > +if NET_VENDOR_EZCHIP > > + > > +config EZCHIP_NPS_MANAGEMENT_ENET > > + tristate "EZchip NPS management enet support" ...there is no dependency line here. None. Noam - this implicitly means that you expect this to build on all arch for all combinations of .config files. At a quick glance I see a dts fragment, and I also see a call to of_get_mac_address() so I'm thinking there needs to be some dependency here, on OF or OF_NET, or... ? Paul. -- > > + ---help--- > > + Simple LAN device for debug or management purposes. > > + Device supports interrupts for RX and TX(completion). > > + Device does not have DMA ability. > > + > > +endif > > diff --git a/drivers/net/ethernet/ezchip/Makefile b/drivers/net/ethernet/ezchip/Makefile > > new file mode 100644 > > index 0000000..e490176 > > --- /dev/null > > +++ b/drivers/net/ethernet/ezchip/Makefile [snip]
next prev parent reply other threads:[~2015-06-22 23:50 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-06-09 12:44 [PATCH] NET: Add ezchip ethernet driver Noam Camus 2015-06-09 14:11 ` Alexey Brodkin 2015-06-10 7:54 ` Paul Bolle 2015-06-11 8:33 ` [PATCH v2] " Noam Camus 2015-06-11 17:41 ` [PATCH v3] " Noam Camus 2015-06-14 6:26 ` [PATCH v4] " Noam Camus 2015-06-14 20:25 ` Florian Fainelli 2015-06-16 14:35 ` [PATCH v5] " Noam Camus 2015-06-21 16:22 ` David Miller 2015-06-21 16:22 ` David Miller 2015-06-22 14:52 ` Noam Camus 2015-06-22 14:52 ` Noam Camus 2015-06-22 17:45 ` Mahesh Bandewar 2015-06-22 17:45 ` Mahesh Bandewar 2015-06-22 23:47 ` Paul Gortmaker [this message] 2015-06-22 23:47 ` Paul Gortmaker 2015-06-23 6:05 ` Noam Camus 2015-06-23 6:05 ` Noam Camus 2015-06-23 7:31 ` David Miller 2015-06-23 7:31 ` David Miller 2015-06-22 20:51 ` [PATCH v6] " Noam Camus 2015-06-22 20:51 ` Noam Camus 2015-06-22 21:04 ` Rami Rosen 2015-06-22 21:04 ` Rami Rosen 2015-06-23 8:43 ` [PATCH v7] " Noam Camus 2015-06-23 14:17 ` David Miller 2015-06-24 3:40 ` Paul Gortmaker 2015-06-11 22:43 ` [PATCH v2] " David Miller
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=20150622234749.GA21581@windriver.com \ --to=paul.gortmaker@windriver.com \ --cc=Alexey.Brodkin@synopsys.com \ --cc=cmetcalf@ezchip.com \ --cc=giladb@ezchip.com \ --cc=linux-kernel@vger.kernel.org \ --cc=maheshb@google.com \ --cc=netdev@vger.kernel.org \ --cc=noamc@ezchip.com \ --cc=talz@ezchip.com \ --cc=vgupta@synopsys.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.