From: zhangfei <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> To: David Laight <David.Laight-ZS65k/vG3HxXrIkS9f7CXA@public.gmane.org>, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> Cc: "linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org" <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>, "arnd-r2nGTMty4D4@public.gmane.org" <arnd-r2nGTMty4D4@public.gmane.org>, "f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org" <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>, "mark.rutland-5wv7dgnIgG8@public.gmane.org" <mark.rutland-5wv7dgnIgG8@public.gmane.org>, "eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>, "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH 3/3] net: hisilicon: new hip04 ethernet driver Date: Tue, 08 Apr 2014 22:47:25 +0800 [thread overview] Message-ID: <53440BFD.3000805@linaro.org> (raw) In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6F1434-VkEWCZq2GCInGFn1LkZF6NBPR1lH4CV8@public.gmane.org> Dear David, On 04/08/2014 04:30 PM, David Laight wrote: > From: zhangfei [mailto:zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org] >> On 04/08/2014 02:53 AM, David Miller wrote: >>> From: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >>> Date: Sat, 5 Apr 2014 12:35:06 +0800 >>> >>>> +struct tx_desc { >>>> + u32 send_addr; >>>> + u16 reserved_16; >>>> + u16 send_size; > > The above doesn't look right for endianness independence. > I'd guess the hardware spec shows a 32bit word with the 'send size' > in one half - that is what you need to define. > > Since this is a tx descriptor (and written by the host) you > can't have 'reserved' field - the host has to write it. > probably these are 'must be zero' fields. Yes, it is not endianness independence. In fact, we have switched the layout since it is u16 for doing the switch endianness. The reserved_16 is the part not used. So it is simpler to define u32 here. If upper 16 bits also need to be set, usually we still use the u32, and organize dynamically, right? > >>>> + u32 reserved_32; >>>> + u32 cfg; >>>> + u32 wb_addr; >>>> +} ____cacheline_aligned; >>> >>> I do not think that ____cacheline_aligned is appropriate at all here. >>> >>> First of all, this is a hardware descriptor, so it has a fixed layout >>> and therefore size. > > The structure also isn't even a multiple of a power of two. > So there will be implicit padding at the end. > > Since there isn't a 'pointer to next' I presume the hardware accesses > the descriptors from adjacent physical addresses. > So you need to explicitly pad to that size. > If the cache line size were 128 byte the above wouldn't work at all. Yes, __aligned(64) can be used here, when I though directly use 64 is not good. The requirement is desc address should be align to 0x40, since desc phys is send to register whose [31:6] is used. > >>> >>> Secondly, unless you declare this object statically in the data section >>> of the object file, the alignment doesn't matter. These descriptors >>> are always dynamically allocated, rather than instantiated in the >>> kernel/driver image. >> >> The ____cacheline_aligned used here is only for the requirement of >> alignment, and use dma_alloc_coherent, while at first dma_pool is used >> for the requirement of alignment. >> Otherwise desc[1] is not aligned and can not be used directly, the >> structure is smaller. > > It sounds like you should be explicitly padding the structure > to 32 bytes - whether or not that is the cache line size. Got it, understand now. > > ... >> I am sorry, but unfortunately this series really does NOT have TX done >> interrupt after checked with hardware guy many times. >> And next series will add TX done interrupt according to the feedback. >> >> There are two reasons of removing the TX done interrupt when the chip is >> designed. >> 1. The specific product does not care the latency, only care the throughput. >> 2. When doing many experiment, the tx done interrupt will impact the >> throughput, as a result reclaim is moved to xmit as one of >> optimizations, then finally tx done interrupt is removed at all. >> >> Is it acceptable of removing timer as well as latency handling, or any >> other work around of this kind of hardware? > > If you don't have a global 'TX done' interrupt, you need a per > descriptor one. > Otherwise you cannot send at maximum rate in the absence of > receive traffic. > Global 'TX done' interrupt means interrupt for desc chain (several desc link together), right? There is no interrupt for either desc chain or single desc. By the way, if single desc interrupt, is it can be optimized like napi, disable the interrupt and re-enable the interrupt until all buffers are reclaimed? Thanks -- 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
WARNING: multiple messages have this Message-ID (diff)
From: zhangfei.gao@linaro.org (zhangfei) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 3/3] net: hisilicon: new hip04 ethernet driver Date: Tue, 08 Apr 2014 22:47:25 +0800 [thread overview] Message-ID: <53440BFD.3000805@linaro.org> (raw) In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6F1434@AcuExch.aculab.com> Dear David, On 04/08/2014 04:30 PM, David Laight wrote: > From: zhangfei [mailto:zhangfei.gao at linaro.org] >> On 04/08/2014 02:53 AM, David Miller wrote: >>> From: Zhangfei Gao <zhangfei.gao@linaro.org> >>> Date: Sat, 5 Apr 2014 12:35:06 +0800 >>> >>>> +struct tx_desc { >>>> + u32 send_addr; >>>> + u16 reserved_16; >>>> + u16 send_size; > > The above doesn't look right for endianness independence. > I'd guess the hardware spec shows a 32bit word with the 'send size' > in one half - that is what you need to define. > > Since this is a tx descriptor (and written by the host) you > can't have 'reserved' field - the host has to write it. > probably these are 'must be zero' fields. Yes, it is not endianness independence. In fact, we have switched the layout since it is u16 for doing the switch endianness. The reserved_16 is the part not used. So it is simpler to define u32 here. If upper 16 bits also need to be set, usually we still use the u32, and organize dynamically, right? > >>>> + u32 reserved_32; >>>> + u32 cfg; >>>> + u32 wb_addr; >>>> +} ____cacheline_aligned; >>> >>> I do not think that ____cacheline_aligned is appropriate at all here. >>> >>> First of all, this is a hardware descriptor, so it has a fixed layout >>> and therefore size. > > The structure also isn't even a multiple of a power of two. > So there will be implicit padding at the end. > > Since there isn't a 'pointer to next' I presume the hardware accesses > the descriptors from adjacent physical addresses. > So you need to explicitly pad to that size. > If the cache line size were 128 byte the above wouldn't work at all. Yes, __aligned(64) can be used here, when I though directly use 64 is not good. The requirement is desc address should be align to 0x40, since desc phys is send to register whose [31:6] is used. > >>> >>> Secondly, unless you declare this object statically in the data section >>> of the object file, the alignment doesn't matter. These descriptors >>> are always dynamically allocated, rather than instantiated in the >>> kernel/driver image. >> >> The ____cacheline_aligned used here is only for the requirement of >> alignment, and use dma_alloc_coherent, while at first dma_pool is used >> for the requirement of alignment. >> Otherwise desc[1] is not aligned and can not be used directly, the >> structure is smaller. > > It sounds like you should be explicitly padding the structure > to 32 bytes - whether or not that is the cache line size. Got it, understand now. > > ... >> I am sorry, but unfortunately this series really does NOT have TX done >> interrupt after checked with hardware guy many times. >> And next series will add TX done interrupt according to the feedback. >> >> There are two reasons of removing the TX done interrupt when the chip is >> designed. >> 1. The specific product does not care the latency, only care the throughput. >> 2. When doing many experiment, the tx done interrupt will impact the >> throughput, as a result reclaim is moved to xmit as one of >> optimizations, then finally tx done interrupt is removed at all. >> >> Is it acceptable of removing timer as well as latency handling, or any >> other work around of this kind of hardware? > > If you don't have a global 'TX done' interrupt, you need a per > descriptor one. > Otherwise you cannot send at maximum rate in the absence of > receive traffic. > Global 'TX done' interrupt means interrupt for desc chain (several desc link together), right? There is no interrupt for either desc chain or single desc. By the way, if single desc interrupt, is it can be optimized like napi, disable the interrupt and re-enable the interrupt until all buffers are reclaimed? Thanks
next prev parent reply other threads:[~2014-04-08 14:47 UTC|newest] Thread overview: 128+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-04-05 4:35 [PATCH v7 0/3] add hisilicon hip04 ethernet driver Zhangfei Gao 2014-04-05 4:35 ` Zhangfei Gao 2014-04-05 4:35 ` [PATCH 1/3] Documentation: add Device tree bindings for Hisilicon hip04 ethernet Zhangfei Gao 2014-04-05 4:35 ` Zhangfei Gao 2014-04-05 4:35 ` [PATCH 2/3] net: hisilicon: new hip04 MDIO driver Zhangfei Gao 2014-04-05 4:35 ` Zhangfei Gao 2014-04-07 18:47 ` David Miller 2014-04-07 18:47 ` David Miller 2014-04-05 4:35 ` [PATCH 3/3] net: hisilicon: new hip04 ethernet driver Zhangfei Gao 2014-04-05 4:35 ` Zhangfei Gao 2014-04-07 18:53 ` David Miller 2014-04-07 18:53 ` David Miller 2014-04-08 8:07 ` zhangfei 2014-04-08 8:07 ` zhangfei 2014-04-08 8:30 ` David Laight 2014-04-08 8:30 ` David Laight [not found] ` <063D6719AE5E284EB5DD2968C1650D6D0F6F1434-VkEWCZq2GCInGFn1LkZF6NBPR1lH4CV8@public.gmane.org> 2014-04-08 9:42 ` Arnd Bergmann 2014-04-08 9:42 ` Arnd Bergmann 2014-04-08 14:47 ` zhangfei [this message] 2014-04-08 14:47 ` zhangfei 2014-04-18 13:17 ` zhangfei 2014-04-18 13:17 ` zhangfei 2014-04-07 18:56 ` David Miller 2014-04-07 18:56 ` David Miller -- strict thread matches above, loose matches on Subject: below -- 2014-04-04 15:16 [PATCH v6 0/3] add hisilicon " Zhangfei Gao [not found] ` <1396624597-390-1-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-04-04 15:16 ` [PATCH 3/3] net: hisilicon: new " Zhangfei Gao 2014-04-04 15:16 ` Zhangfei Gao 2014-04-01 13:27 [PATCH v5 0/3] add hisilicon " Zhangfei Gao 2014-04-01 13:27 ` [PATCH 3/3] net: hisilicon: new " Zhangfei Gao 2014-04-01 13:27 ` Zhangfei Gao [not found] ` <1396358832-15828-4-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-04-02 9:21 ` Arnd Bergmann 2014-04-02 9:21 ` Arnd Bergmann 2014-04-02 9:51 ` zhangfei 2014-04-02 9:51 ` zhangfei 2014-04-02 15:24 ` Arnd Bergmann 2014-04-02 15:24 ` Arnd Bergmann 2014-04-02 10:04 ` David Laight 2014-04-02 10:04 ` David Laight 2014-04-02 15:49 ` Arnd Bergmann 2014-04-02 15:49 ` Arnd Bergmann 2014-04-03 6:24 ` Zhangfei Gao 2014-04-03 6:24 ` Zhangfei Gao [not found] ` <CAMj5BkgfwE1hHpVeqH9WRitwCB30x3c4w0qw7sXT3PiOV-QcPQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-04-03 8:35 ` Arnd Bergmann 2014-04-03 8:35 ` Arnd Bergmann 2014-04-03 15:22 ` David Miller 2014-04-03 15:22 ` David Miller 2014-04-03 15:38 ` zhangfei 2014-04-03 15:38 ` zhangfei 2014-04-03 15:27 ` Russell King - ARM Linux 2014-04-03 15:27 ` Russell King - ARM Linux 2014-04-03 15:42 ` David Laight 2014-04-03 15:42 ` David Laight 2014-04-03 15:50 ` Russell King - ARM Linux 2014-04-03 15:50 ` Russell King - ARM Linux 2014-04-03 17:57 ` Arnd Bergmann 2014-04-03 17:57 ` Arnd Bergmann 2014-04-04 6:52 ` Zhangfei Gao 2014-04-04 6:52 ` Zhangfei Gao 2014-03-28 15:35 [PATCH v4 0/3] add hisilicon " Zhangfei Gao 2014-03-28 15:36 ` [PATCH 3/3] net: hisilicon: new " Zhangfei Gao 2014-03-28 15:36 ` Zhangfei Gao 2014-03-24 14:14 [PATCH v3 0/3] add hisilicon " Zhangfei Gao 2014-03-24 14:14 ` [PATCH 3/3] net: hisilicon: new " Zhangfei Gao 2014-03-24 14:14 ` Zhangfei Gao [not found] ` <1395670496-17381-4-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-03-24 15:18 ` Arnd Bergmann 2014-03-24 15:18 ` Arnd Bergmann 2014-03-25 4:06 ` Zhangfei Gao 2014-03-25 4:06 ` Zhangfei Gao 2014-03-25 8:12 ` Arnd Bergmann 2014-03-25 8:12 ` Arnd Bergmann 2014-03-25 17:00 ` Florian Fainelli 2014-03-25 17:00 ` Florian Fainelli 2014-03-25 17:05 ` Arnd Bergmann 2014-03-25 17:05 ` Arnd Bergmann 2014-03-25 17:16 ` Florian Fainelli 2014-03-25 17:16 ` Florian Fainelli 2014-03-25 17:57 ` Arnd Bergmann 2014-03-25 17:57 ` Arnd Bergmann 2014-03-26 9:55 ` David Laight 2014-03-26 9:55 ` David Laight 2014-03-25 17:17 ` David Laight 2014-03-25 17:17 ` David Laight 2014-03-25 17:21 ` Eric Dumazet 2014-03-25 17:21 ` Eric Dumazet 2014-03-25 17:54 ` Arnd Bergmann 2014-03-25 17:54 ` Arnd Bergmann 2014-03-27 12:53 ` zhangfei 2014-03-27 12:53 ` zhangfei 2014-03-24 16:32 ` Florian Fainelli 2014-03-24 16:32 ` Florian Fainelli 2014-03-24 17:23 ` Arnd Bergmann 2014-03-24 17:23 ` Arnd Bergmann 2014-03-24 17:35 ` Florian Fainelli 2014-03-24 17:35 ` Florian Fainelli 2014-03-27 6:27 ` Zhangfei Gao 2014-03-27 6:27 ` Zhangfei Gao 2014-03-21 15:09 [PATCH v2 0/3] add hisilicon " Zhangfei Gao 2014-03-21 15:09 ` [PATCH 3/3] net: hisilicon: new " Zhangfei Gao 2014-03-21 15:09 ` Zhangfei Gao 2014-03-21 15:27 ` Arnd Bergmann 2014-03-21 15:27 ` Arnd Bergmann 2014-03-22 1:18 ` zhangfei 2014-03-22 1:18 ` zhangfei 2014-03-22 8:08 ` Arnd Bergmann 2014-03-22 8:08 ` Arnd Bergmann 2014-03-18 8:40 [PATCH 0/3] add hisilicon " Zhangfei Gao [not found] ` <1395132017-15928-1-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-03-18 8:40 ` [PATCH 3/3] net: hisilicon: new " Zhangfei Gao 2014-03-18 8:40 ` Zhangfei Gao [not found] ` <1395132017-15928-4-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-03-18 10:46 ` Russell King - ARM Linux 2014-03-18 10:46 ` Russell King - ARM Linux 2014-03-20 9:51 ` Zhangfei Gao 2014-03-20 9:51 ` Zhangfei Gao 2014-03-24 14:17 ` Rob Herring 2014-03-24 14:17 ` Rob Herring 2014-03-26 14:22 ` Zhangfei Gao 2014-03-26 14:22 ` Zhangfei Gao 2014-03-18 11:25 ` Arnd Bergmann 2014-03-18 11:25 ` Arnd Bergmann 2014-03-20 14:00 ` Zhangfei Gao 2014-03-20 14:00 ` Zhangfei Gao 2014-03-20 14:31 ` Arnd Bergmann 2014-03-20 14:31 ` Arnd Bergmann [not found] ` <201403201531.20416.arnd-r2nGTMty4D4@public.gmane.org> 2014-03-21 5:19 ` Zhangfei Gao 2014-03-21 5:19 ` Zhangfei Gao 2014-03-21 7:37 ` Arnd Bergmann 2014-03-21 7:37 ` Arnd Bergmann 2014-03-21 7:56 ` Zhangfei Gao 2014-03-21 7:56 ` Zhangfei Gao 2014-03-24 8:17 ` Zhangfei Gao 2014-03-24 8:17 ` Zhangfei Gao 2014-03-24 10:02 ` Arnd Bergmann 2014-03-24 10:02 ` Arnd Bergmann 2014-03-24 13:23 ` Zhangfei Gao 2014-03-24 13:23 ` Zhangfei Gao
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=53440BFD.3000805@linaro.org \ --to=zhangfei.gao-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \ --cc=David.Laight-ZS65k/vG3HxXrIkS9f7CXA@public.gmane.org \ --cc=arnd-r2nGTMty4D4@public.gmane.org \ --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \ --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@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: 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.