* Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register
From: Chen-Yu Tsai @ 2018-03-20 7:15 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mark Rutland, devicetree, Stephen Boyd, netdev, Michael Turquette,
Rob Herring, Corentin Labbe, Mark Brown, Giuseppe Cavallaro,
linux-clk, linux-arm-kernel, Icenowy Zheng
In-Reply-To: <20180318213129.ucwslzvwq6khxrcd@flea>
On Mon, Mar 19, 2018 at 5:31 AM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Sat, Mar 17, 2018 at 05:28:47PM +0800, Chen-Yu Tsai wrote:
>> From: Icenowy Zheng <icenowy@aosc.io>
>>
>> There's a GMAC configuration register, which exists on A64/A83T/H3/H5 in
>> the syscon part, in the CCU of R40 SoC.
>>
>> Export a regmap of the CCU.
>>
>> Read access is not restricted to all registers, but only the GMAC
>> register is allowed to be written.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> Gah, this is crazy. I'm really starting to regret letting that syscon
> in in the first place...
IMHO syscon is really a better fit. It's part of the glue layer and
most other dwmac user platforms treat it as such and use a syscon.
Plus the controls encompass delays (phase), inverters (polarity),
and even signal routing. It's not really just a group of clock controls,
like what we poorly modeled for A20/A31. I think that was really a
mistake.
As I mentioned in the cover letter, a slightly saner approach would
be to let drivers add custom syscon entries, which would then require
less custom plumbing.
> And I'm not really looking forward the time where SCPI et al. will be
> mature and we'll have the clock controller completely outside of our
> control.
I don't think it's going to happen for any of the older SoCs. The R40
only stands out because the GMAC controls are in the clock controller
address space, presumably to be like the A20.
ChenYu
^ permalink raw reply
* Re: [bpf-next V3 PATCH 13/15] mlx5: use page_pool for xdp_return_frame call
From: Tariq Toukan @ 2018-03-20 7:43 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: netdev, BjörnTöpel, magnus.karlsson, eugenia,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed, galp,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180319141217.416d269a@redhat.com>
On 19/03/2018 3:12 PM, Jesper Dangaard Brouer wrote:
> On Mon, 12 Mar 2018 15:20:06 +0200 Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> On 12/03/2018 12:16 PM, Tariq Toukan wrote:
>>>
>>> On 12/03/2018 12:08 PM, Tariq Toukan wrote:
>>>>
>>>> On 09/03/2018 10:56 PM, Jesper Dangaard Brouer wrote:
>>>>> This patch shows how it is possible to have both the driver local page
>>>>> cache, which uses elevated refcnt for "catching"/avoiding SKB
>>>>> put_page. And at the same time, have pages getting returned to the
>>>>> page_pool from ndp_xdp_xmit DMA completion.
>>>>>
> [...]
>>>>>
>>>>> Before this patch: single flow performance was 6Mpps, and if I started
>>>>> two flows the collective performance drop to 4Mpps, because we hit the
>>>>> page allocator lock (further negative scaling occurs).
>>>>>
>>>>> V2: Adjustments requested by Tariq
>>>>> - Changed page_pool_create return codes not return NULL, only
>>>>> ERR_PTR, as this simplifies err handling in drivers.
>>>>> - Save a branch in mlx5e_page_release
>>>>> - Correct page_pool size calc for MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ
>>>>>
>>>>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>>>> ---
>>>>
>>>> I am running perf tests with your series. I sense a drastic
>>>> degradation in regular TCP flows, I'm double checking the numbers now...
>>>
>>> Well, there's a huge performance degradation indeed, whenever the
>>> regular flows (non-XDP) use the new page pool. Cannot merge before
>>> fixing this.
>>>
>>> If I disable the local page-cache, numbers get as low as 100's of Mbps
>>> in TCP stream tests.
>>
>> It seems that the page-pool doesn't fit as a general fallback (when page
>> in local rx cache is busy), as the refcnt is elevated/changing:
>
> I see the issue. I have to go over the details in the driver, but I
> think it should be sufficient to remove the WARN(). When the page_pool
> was integrated with the MM-layer, being invoked from the put_page()
> call itself, this would indicate a likely API misuse. But now, with
> the page refcnt based recycle tricks, it is the norm (for non-XDP) that
> put_page is called without the knowledge of page_pool.
>
I see, I'll remove the WARN and test.
>
>> [ 7343.086102] ------------[ cut here ]------------
>> [ 7343.086103] __page_pool_put_page() violating page_pool invariance refcnt:0
>> [ 7343.086114] WARNING: CPU: 1 PID: 17 at net/core/page_pool.c:291 __page_pool_put_page+0x7c/0xa0
>
> Here page_pool actually catch the page refcnt race correctly, and does
> the proper handling of returning it to the page allocator (via __put_page).
>
> I do notice (in the page_pool code) that in case page_pool handles DMA
> mapping (which isn't the case, yet), that I'm missing a DMA unmap
> release in the code.
>
I didn't get this one. Both DMA map/unmap do not exist yet in page pool, no?
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Alexey Roslyakov @ 2018-03-20 7:58 UTC (permalink / raw)
To: Arend van Spriel
Cc: Florian Fainelli, Andrew Lunn, Kalle Valo, robh+dt, mark.rutland,
franky.lin, hante.meuleman, chi-hsien.lin, wright.feng, netdev,
linux-wireless, devicetree, linux-kernel, brcm80211-dev-list.pdl,
brcm80211-dev-list, Ulf Hansson
In-Reply-To: <5AB044C0.9060701@broadcom.com>
Arend,
>Also I am not sure if the broken-sg-support is still needed. We added that for omap_hsmmc, but that has since changed to scatter-gather emulation so it might not be needed anymore.
I can confirm it doesn't impact wifi performance in case of rk3288+ap6335.
But I still have to set settings->bus.sdio.sd_head_align = 4 and
settings->bus.sdio.sd_sgentry_align = 512, otherwise
brcmf_sdiod_sglist_rw fails:
974.888452] net_ratelimit: 8 callbacks suppressed
[ 974.888457] brcmfmac: brcmf_sdiod_sglist_rw: CMD53 sg block read failed -84
[ 974.901527] brcmfmac: brcmf_sdio_rxglom: glom read of 6144 bytes failed: -5
[ 974.910248] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame
[ 975.018041] brcmfmac: brcmf_sdiod_sglist_rw: CMD53 sg block read failed -84
[ 975.025833] brcmfmac: brcmf_sdio_rxglom: glom read of 6144 bytes failed: -5
[ 975.033634] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame
[ 975.033924] dwmmc_rockchip ff0d0000.dwmmc: Unexpected command
timeout, state 0
[ 975.049209] brcmfmac: brcmf_sdio_readframes: RXHEADER FAILED: -84
[ 975.056034] brcmfmac: brcmf_sdio_rxfail: abort command, terminate
frame, send NAK
[ 975.068367] brcmfmac: brcmf_sdio_hdparse: HW header length too long
[ 975.075379] brcmfmac: brcmf_sdio_rxfail: terminate frame
Regards,
Alex
On 20 March 2018 at 06:16, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> + Uffe
>
> On 3/19/2018 6:55 PM, Florian Fainelli wrote:
>>
>> On 03/19/2018 07:10 AM, Alexey Roslyakov wrote:
>>>
>>> Hi Arend,
>>> I appreciate your response. In my opinion, it has nothing to do with
>>> SDIO host, because it defines "quirks" in the driver itself.
>>
>>
>> It is not clear to me from your patch series whether the problem is that:
>>
>> - the SDIO device has a specific alignment requirements, which would be
>> either a SDIO device driver limitation/issue or maybe the underlying
>> hardware device/firmware requiring that
>>
>> - the SDIO host controller used is not capable of coping nicely with
>> these said limitations
>>
>> It seems to me like what you are doing here is a) applicable to possibly
>> more SDIO devices and host combinations, and b) should likely be done at
>> the layer between the host and device, such that it is available to more
>> combinations.
>
>
> Indeed. That was my thought exactly and I can not imagine Uffe would push
> back on that reasoning.
>
>>> If I get it right, you mean something like this:
>>>
>>> mmc3: mmc@1c12000 {
>>> ...
>>> broken-sg-support;
>>> sd-head-align = 4;
>>> sd-sgentry-align = 512;
>>>
>>> brcmf: wifi@1 {
>>> ...
>>> };
>>> };
>>>
>>> Where dt: bindings documentation for these entries should reside?
>>> In generic MMC bindings? Well, this is the very special case and
>>> mmc-linux maintainer will unlikely to accept these changes.
>>> Also, extra kernel code modification might be required. It could make
>>> quite trivial change much more complex.
>>
>>
>> If the MMC maintainers are not copied on this patch series, it will
>> likely be hard for them to identify this patch series and chime in...
>
>
> The main question is whether this is indeed a "very special case" as Alexey
> claims it to be or that it is likely to be applicable to other device and
> host combinations as you are suggesting.
>
> If these properties are imposed by the host or host controller it would make
> sense to have these in the mmc bindings.
>
>>>
>>>> Also I am not sure if the broken-sg-support is still needed. We added
>>>> that for omap_hsmmc, but that has since changed to scatter-gather emulation
>>>> so it might not be needed anymore.
>>>
>>>
>>> I've experienced the problem with rk3288 (dw-mmc host) and sdio
>>> settings like above solved it.
>>> Frankly, I haven't investigated any deeper which one of the settings
>>> helped in my case yet...
>>> I will try to get rid of broken-sg-support first and let you know if
>>> it does make any difference.
>
>
> Are you using some chromebook. I have some lying around here so I could also
> look into it. What broadcom chipset do you have?
>
> Regards,
> Arend
>
>
>>> All the best,
>>> Alex.
>>>
>>> On 19 March 2018 at 16:31, Arend van Spriel
>>> <arend.vanspriel@broadcom.com> wrote:
>>>>
>>>> On 3/19/2018 2:40 AM, Alexey Roslyakov wrote:
>>>>>
>>>>>
>>>>> In case if the host has higher align requirements for SG items, allow
>>>>> setting device-specific aligns for scatterlist items.
>>>>>
>>>>> Signed-off-by: Alexey Roslyakov <alexey.roslyakov@gmail.com>
>>>>> ---
>>>>> Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> | 5
>>>>> +++++
>>>>> 1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> index 86602f264dce..187b8c1b52a7 100644
>>>>> ---
>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>> @@ -17,6 +17,11 @@ Optional properties:
>>>>> When not specified the device will use in-band SDIO
>>>>> interrupts.
>>>>> - interrupt-names : name of the out-of-band interrupt, which must
>>>>> be
>>>>> set
>>>>> to "host-wake".
>>>>> + - brcm,broken-sg-support : boolean flag to indicate that the SDIO
>>>>> host
>>>>> + controller has higher align requirement than 32 bytes for each
>>>>> + scatterlist item.
>>>>> + - brcm,sd-head-align : alignment requirement for start of data
>>>>> buffer.
>>>>> + - brcm,sd-sgentry-align : length alignment requirement for each sg
>>>>> entry.
>>>>
>>>>
>>>>
>>>> Hi Alexey,
>>>>
>>>> Thanks for the patch. However, the problem with these is that they are
>>>> characterizing the host controller and not the wireless device. So from
>>>> device tree perspective , which is to describe the hardware, these
>>>> properties should be SDIO host controller properties. Also I am not sure
>>>> if
>>>> the broken-sg-support is still needed. We added that for omap_hsmmc, but
>>>> that has since changed to scatter-gather emulation so it might not be
>>>> needed
>>>> anymore.
>>>>
>>>> Regards,
>>>> Arend
>>>
>>>
>>>
>>>
>>
>>
>
--
With best regards,
Alexey Roslyakov
Email: alexey.roslyakov@gmail.com
^ permalink raw reply
* Re: linux-next on x60: network manager often complains "network is disabled" after resume
From: Pavel Machek @ 2018-03-20 8:03 UTC (permalink / raw)
To: Dan Williams
Cc: Woody Suwalski, Rafael J. Wysocki, kernel list,
Linux-pm mailing list, Netdev list
In-Reply-To: <1521481549.20208.8.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2149 bytes --]
On Mon 2018-03-19 12:45:49, Dan Williams wrote:
> On Mon, 2018-03-19 at 18:33 +0100, Pavel Machek wrote:
> > On Mon 2018-03-19 10:40:08, Dan Williams wrote:
> > > On Mon, 2018-03-19 at 10:21 +0100, Pavel Machek wrote:
> > > > On Mon 2018-03-19 05:17:45, Woody Suwalski wrote:
> > > > > Pavel Machek wrote:
> > > > > > Hi!
> > > > > >
> > > > > > With recent linux-next, after resume networkmanager often
> > > > > > claims
> > > > > > that
> > > > > > "network is disabled". Sometimes suspend/resume clears that.
> > > > > >
> > > > > > Any ideas? Does it work for you?
> > > > > >
> > > > > >
> > > > > > Pavel
> > > > >
> > > > > Tried the 4.16-rc6 with nm 1.4.4 - I do not see the issue.
> > > >
> > > > Thanks for testing... but yes, 4.16 should be ok. If not fixed,
> > > > problem will appear in 4.17-rc1.
> > >
> > > Where does the complaint occur? In the GUI, or with nmcli, or
> > > somewhere else? Also, what's the output of "nmcli dev" after
> > > resume?
> >
> > In the GUI. I click in place where I'd select access point, and menu
> > does not show up, telling me that "network is disabled".
>
> Ok, what does 'nmcli dev' and 'nmcli radio' show?
Broken state.
pavel@amd:~$ nmcli dev
DEVICE TYPE STATE CONNECTION
eth1 ethernet unavailable --
lo loopback unmanaged --
wlan0 wifi unmanaged --
pavel@amd:~$ nmcli radio
WIFI-HW WIFI WWAN-HW WWAN
enabled enabled enabled enabled
pavel@amd:~$ uname -a
Linux amd 4.16.0-rc5-next-20180314+ #31 SMP Thu Mar 15 14:27:19 CET
2018 i686 GNU/Linux
Let me suspend/resume. I was lucky and got it into working state:
pavel@amd:~$ nmcli dev
DEVICE TYPE STATE CONNECTION
wlan0 wifi connected openwireless.org
eth1 ethernet unavailable --
lo loopback unmanaged --
pavel@amd:~$ nmcli radio
WIFI-HW WIFI WWAN-HW WWAN
enabled enabled enabled enabled
pavel@amd:~$
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [bpf-next V3 PATCH 13/15] mlx5: use page_pool for xdp_return_frame call
From: Tariq Toukan @ 2018-03-20 8:18 UTC (permalink / raw)
To: Tariq Toukan, Jesper Dangaard Brouer
Cc: netdev, BjörnTöpel, magnus.karlsson, eugenia,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed, galp,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <66f0da5a-388d-5ddc-4bb7-441f6df4af96@mellanox.com>
>> I see the issue. I have to go over the details in the driver, but I
>> think it should be sufficient to remove the WARN(). When the page_pool
>> was integrated with the MM-layer, being invoked from the put_page()
>> call itself, this would indicate a likely API misuse. But now, with
>> the page refcnt based recycle tricks, it is the norm (for non-XDP) that
>> put_page is called without the knowledge of page_pool.
>>
> I see, I'll remove the WARN and test.
>
Verified, it works. Please fix in next patchset.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH 0/3] kernel: add support for 256-bit IO access
From: Ingo Molnar @ 2018-03-20 8:26 UTC (permalink / raw)
To: Thomas Gleixner
Cc: David Laight, 'Rahul Lakkireddy', x86@kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mingo@redhat.com, hpa@zytor.com, davem@davemloft.net,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
ganeshgr@chelsio.com, nirranjan@chelsio.com, indranil@chelsio.com,
Andy Lutomirski, Peter Zijlstra, Thomas Gleixner, Fenghua Yu
In-Reply-To: <alpine.DEB.2.21.1803191625080.2010@nanos.tec.linutronix.de>
* Thomas Gleixner <tglx@linutronix.de> wrote:
> > Useful also for code that needs AVX-like registers to do things like CRCs.
>
> x86/crypto/ has a lot of AVX optimized code.
Yeah, that's true, but the crypto code is processing fundamentally bigger blocks
of data, which amortizes the cost of using kernel_fpu_begin()/_end().
kernel_fpu_begin()/_end() is a pretty heavy operation because it does a full FPU
save/restore via the XSAVE[S] and XRSTOR[S] instructions, which can easily copy a
thousand bytes around! So kernel_fpu_begin()/_end() is probably a non-starter for
something small, like a single 256-bit or 512-bit word access.
But there's actually a new thing in modern kernels: we got rid of (most of) lazy
save/restore FPU code, our new x86 FPU model is very "direct" with no FPU faults
taken normally.
So assuming the target driver will only load on modern FPUs I *think* it should
actually be possible to do something like (pseudocode):
vmovdqa %ymm0, 40(%rsp)
vmovdqa %ymm1, 80(%rsp)
...
# use ymm0 and ymm1
...
vmovdqa 80(%rsp), %ymm1
vmovdqa 40(%rsp), %ymm0
... without using the heavy XSAVE/XRSTOR instructions.
Note that preemption probably still needs to be disabled and possibly there are
other details as well, but there should be no 'heavy' FPU operations.
I think this should still preserve all user-space FPU state and shouldn't muck up
any 'weird' user-space FPU state (such as pending exceptions, legacy x87 running
code, NaN registers or weird FPU control word settings) we might have interrupted
either.
But I could be wrong, it should be checked whether this sequence is safe.
Worst-case we might have to save/restore the FPU control and tag words - but those
operations should still be much faster than a full XSAVE/XRSTOR pair.
So I do think we could do more in this area to improve driver performance, if the
code is correct and if there's actual benchmarks that are showing real benefits.
Thanks,
Ingo
^ permalink raw reply
* Re: [RFC PATCH 0/3] kernel: add support for 256-bit IO access
From: Thomas Gleixner @ 2018-03-20 8:38 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Laight, 'Rahul Lakkireddy', x86@kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mingo@redhat.com, hpa@zytor.com, davem@davemloft.net,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
ganeshgr@chelsio.com, nirranjan@chelsio.com, indranil@chelsio.com,
Andy Lutomirski, Peter Zijlstra, Fenghua Yu, Eric Biggers
In-Reply-To: <20180320082651.jmxvvii2xvmpyr2s@gmail.com>
On Tue, 20 Mar 2018, Ingo Molnar wrote:
> * Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > > Useful also for code that needs AVX-like registers to do things like CRCs.
> >
> > x86/crypto/ has a lot of AVX optimized code.
>
> Yeah, that's true, but the crypto code is processing fundamentally bigger blocks
> of data, which amortizes the cost of using kernel_fpu_begin()/_end().
Correct.
> So assuming the target driver will only load on modern FPUs I *think* it should
> actually be possible to do something like (pseudocode):
>
> vmovdqa %ymm0, 40(%rsp)
> vmovdqa %ymm1, 80(%rsp)
>
> ...
> # use ymm0 and ymm1
> ...
>
> vmovdqa 80(%rsp), %ymm1
> vmovdqa 40(%rsp), %ymm0
>
> ... without using the heavy XSAVE/XRSTOR instructions.
>
> Note that preemption probably still needs to be disabled and possibly there are
> other details as well, but there should be no 'heavy' FPU operations.
Emphasis on should :)
> I think this should still preserve all user-space FPU state and shouldn't muck up
> any 'weird' user-space FPU state (such as pending exceptions, legacy x87 running
> code, NaN registers or weird FPU control word settings) we might have interrupted
> either.
>
> But I could be wrong, it should be checked whether this sequence is safe.
> Worst-case we might have to save/restore the FPU control and tag words - but those
> operations should still be much faster than a full XSAVE/XRSTOR pair.
Fair enough.
> So I do think we could do more in this area to improve driver performance, if the
> code is correct and if there's actual benchmarks that are showing real benefits.
If it's about hotpath performance I'm all for it, but the use case here is
a debug facility...
And if we go down that road then we want a AVX based memcpy()
implementation which is runtime conditional on the feature bit(s) and
length dependent. Just slapping a readqq() at it and use it in a loop does
not make any sense.
Thanks,
tglx
^ permalink raw reply
* Re: pull request: bluetooth 2018-03-16
From: Marcel Holtmann @ 2018-03-20 8:44 UTC (permalink / raw)
To: Johan Hedberg, David S. Miller; +Cc: linux-bluetooth, Network Development
In-Reply-To: <20180316182319.GA4401@x1c>
Hi Dave,
> Here are a few more important Bluetooth driver fixes for the 4.16
> kernel.
>
> Please let me know if there are any issues pulling. Thanks.
>
> Johan
>
> ---
> The following changes since commit 3d502067599f0db12e74e6646aee8728efe3e5be:
>
> net/smc: simplify wait when closing listen socket (2018-03-15 09:49:13 -0400)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git for-upstream
>
> for you to fetch changes up to e07c99b07ae85255d5c5bc2480fbd4c4e77f71bc:
>
> Bluetooth: hci_bcm: Set pulsed_host_wake flag in sleep parameters (2018-03-15 19:39:37 +0100)
>
> ----------------------------------------------------------------
> Hans de Goede (2):
> Revert "Bluetooth: hci_bcm: Streamline runtime PM code"
> Bluetooth: hci_bcm: Set pulsed_host_wake flag in sleep parameters
>
> Takashi Iwai (1):
> Bluetooth: btusb: Fix quirk for Atheros 1525/QCA6174
>
> drivers/bluetooth/btusb.c | 2 +-
> drivers/bluetooth/hci_bcm.c | 13 +++++++++----
> 2 files changed, 10 insertions(+), 5 deletions(-)
any issue with this pull request? I ask since it seems to have disappeared from patchwork.
Regards
Marcel
^ permalink raw reply
* Re: HW question: i210 vs. BCM5461S over SGMII: no response from PHY to MDIO requests?
From: Frantisek Rysanek @ 2018-03-20 9:02 UTC (permalink / raw)
To: Andrew Lunn, netdev
In-Reply-To: <20180318144053.GA1188@lunn.ch>
[-- Attachment #1: Mail message body --]
[-- Type: text/plain, Size: 6415 bytes --]
On 18 Mar 2018 at 15:40, Andrew Lunn wrote:
> > I'm not getting an ACK from the SFP, probably because I've got the
> > address and offset wrong and because I'd better use indirect access.
> > There's some more work awaiting me...
>
> Try address 0x50.
>
> i2detect will probe all addresses for you, if you have a standard
> Linux i2c bus.
>
Thanks for that pointer, that has helped :-)
Not sure if the HW-driven i2c in the i210 works right or not.
It seems to read some garbage instead of MII Dev ID,
and even if I hack that in as a recognized ID, it then
fails at some later stage of init. I have to get back to this later.
After I got the bit-banging I2c to work (see below), the few values
that I see coming out of the HW-driven i2c are possibly "obtained
correctly" too.
I've managed to get far enough in the init to make the netdevice
surface in the OS.
And, at the end of the process, the bit-banging i2c interface also
appears as /dev/i2c-8 (in my case - after all the graphics DDC busses
and whatnot).
And, I've modded the existing bit-banging callbacks in igb_main.c,
supposedly written for the i350. I'm wondering if they've ever worked
right in the first place... I seem to have confirmed my earlier vague
impression, that the "set" functions would drive the output hard to
log.1, instead of letting it "float up" based on the pull up resistor
(go high-Z).
[CODE SNIPPET]
static void igb_set_i2c_data(void *data, int state)
{
struct igb_adapter *adapter = (struct igb_adapter *)data;
struct e1000_hw *hw = &adapter->hw;
s32 i2cctl = rd32(E1000_I2CPARAMS);
/*
// original version
if (state)
i2cctl |= E1000_I2C_DATA_OUT;
else
i2cctl &= ~E1000_I2C_DATA_OUT;
i2cctl &= ~E1000_I2C_DATA_OE_N;
i2cctl |= E1000_I2C_CLK_OE_N;
*/
// my replacement
if (state) {
i2cctl |= E1000_I2C_DATA_OE_N; // high Z for SDA
i2cctl |= E1000_I2C_DATA_OUT; // redundant, but ahh well
}
else {
i2cctl &= ~E1000_I2C_DATA_OE_N; // SDA output active
i2cctl &= ~E1000_I2C_DATA_OUT; // log.0 on SDA output
}
// this stays the same:
wr32(E1000_I2CPARAMS, i2cctl);
wrfl();
}
[/CODE SNIPPET]
Same thing for the SCL pin.
The master must drive the bus using an open collector, rather than a
full totem - so that the slave can have her say (ACK, return data,
extend the clock maybe). Not sure if the i210's HW i2c master does
this right, and if it supports all the needed transaction formats
etc.
After this mod, the user-space i2c progies started to work on
/dev/i2c-8. I mean - I'd seen activity on the 'scope earlier,
but it was all failures.
I should probably turn this into a patch, but the same is true about
the rest of what I'm doing to the driver code, and currently it's not
nice :-) Once I get to the bottom of it (if I ever do) I'll have to
start over from scratch = from pristine original IGB code, insert the
needed mods (observing coding style), comment them properly and
*then* take a diff...
And I got a little further.
I'm attaching two text files, containing the dumps of i2cdetect and
eeprog (modified by me *not* to exit on error, just try another
byte).
The i2cdetect succeeds even with a simple "read byte" transaction
(just two bytes on the bus).
The "eeprog" uses an SMbus-style indirect read with an 8bit address.
This consists of two low-level i2c transactions:
"write byte" followed by a "read byte".
The first transaction sets the offset inside the slave,
the second transaction reads back actual data.
Again I have two SFP's, both of them "3rd-party Chinese", one sold as
Huawei-compatible, the other one as Cisco-compatible. Not sure to
what extent this is true :-)
i2cdetect has found three i2c slaves (identical layout in both SFP's)
at addresses 0x50, 0x51 and 0x56.
What are they? EEPROM, DDM and "MDIO over i2c" ?
The SFP's likely lack a proper SFP MSA data structure.
The second SFP ("Cisco-compatible") behaves somewhat special at 0x56.
eeprog just calls the "SMbus read 1 byte from offset at slave"
iteratively across the "space of offsets".
I.e. every offset gets an independent transaction, 1 byte long.
Now... if I unleash this iteration unto i2c slave 0x56, then every
other byte (every odd byte = every odd offset) ends up in a
"transaction error".
I had to hack eeprog to just ignore these errors and print XX instead
of data in the hex dump.
I went on to investigate further - I tried reading single bytes.
If I start at an even offset, such as 0 or 8, and I read that single
offset (byte) twice, i.e. two separate and complete SMbus
transactions, that succeeds, the two values returned are different
from each other, and I can read another offset without an error.
But, if I read an offset just once, and then try a different offset,
I get an error on the second offset. And even/odd offsets don't
really matter - I can read an odd offset just fine, as long as I read
any/every offset *twice*.
And, it gets more interesting if I look at the 'scope.
I'm attaching some screenshots of the failed transaction.
As already briefly mentioned, the healthy SMbus "read byte"
transaction consists of two separate i2c sub-transactions:
[START][7bit slave addr][WRITE][ACK][REPST][8bit offset][ACK][STOP]
[START][7bit slave addr][READ][ACK][REPST][8bit data][NAK][STOP]
The error (if I break the rule that I have to read every offset
twice) looks like this:
[START][7bit slave addr][WRITE][ACK] [8bit offset][NAK][STOP]
(and the second sub-transaction is not attempted by the master).
= already the first sub-transaction gets NACKed.
This kind of rings a bell, in that the MII PHY registers are 16 bits
each. Is this a plausible correlation?
This is making me wonder if I should use some other transaction
format. Read one more byte? There's a transaction called "SMBus read
word" which does exactly that.
Except that "read word" wouldn't do, I cannot read another byte after
a NAK. Same thing with "SMBus read block data". Apparently the
appropriate transaction style is hardwired in the slave, the master
cannot "force" the slave to use a particular transaction style.
Not within that one transaction (maybe somehow "out of band").
Does this make sense as some sort of proprietary standard
(Cisco, if I should take the vendor's label at face value),
or is this likely some "3rd-party innovation thinko", not worth
implementing?
Frank
[-- Attachment #2: Attachment information. --]
[-- Type: text/plain, Size: 468 bytes --]
The following section of this message contains a file attachment
prepared for transmission using the Internet MIME message format.
If you are using Pegasus Mail, or any other MIME-compliant system,
you should be able to save it or view it from within your mailer.
If you cannot, please ask your system administrator for assistance.
---- File information -----------
File: i2c-detect.txt
Date: 20 Mar 2018, 6:50
Size: 1459 bytes.
Type: Text
[-- Attachment #3: i2c-detect.txt --]
[-- Type: Application/Octet-stream, Size: 1459 bytes --]
SFP#1 sold as Huawei-compliant
root@debian:/usr/src/eeprog-0.7.6# i2cdetect -a 8 0 0x7f
WARNING! This program can confuse your I2C bus, cause data loss and worse!
I will probe file /dev/i2c-8 using read byte commands.
I will probe address range 0x00-0x7f.
Continue? [Y/n]
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: 50 51 -- -- -- -- 56 -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
SFP#2 sold as Cisco-compliant
root@debian:/usr/src/eeprog-0.7.6# i2cdetect -a 8 0 0x7f
WARNING! This program can confuse your I2C bus, cause data loss and worse!
I will probe file /dev/i2c-8.
I will probe address range 0x00-0x7f.
Continue? [Y/n]
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: 50 51 -- -- -- -- 56 -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
[-- Attachment #4: Attachment information. --]
[-- Type: text/plain, Size: 464 bytes --]
The following section of this message contains a file attachment
prepared for transmission using the Internet MIME message format.
If you are using Pegasus Mail, or any other MIME-compliant system,
you should be able to save it or view it from within your mailer.
If you cannot, please ask your system administrator for assistance.
---- File information -----------
File: eeprog.txt
Date: 20 Mar 2018, 6:49
Size: 7631 bytes.
Type: Text
[-- Attachment #5: eeprog.txt --]
[-- Type: Application/Octet-stream, Size: 7631 bytes --]
EEPROG is using the smbus-style indirect access.
The -8 arg means "use 8-bit addresses",
otherwise useful with 24c0x...24C16.
EEPROG can also do 16bit addresses for the larger members
of the 24cXX family, but these are likely no use here,
given that the SFP I2c slaves seem to have some data
in the first 128 B only (corresponding to a 24c01).
######################
SFP#1 Huawei-compliant
######################
root@debian:/usr/src/eeprog-0.7.6# ./eeprog -x -f -8 -r 0:256 /dev/i2c-8 0x50
eeprog 0.7.6, a 24Cxx EEPROM reader/writer
Copyright (c) 2003-2004 by Stefano Barbato - All rights reserved.
Bus: /dev/i2c-8, Address: 0x50, Mode: 8bit
Reading 256 bytes from 0x0
0000| 00 00 07 00 10 01 20 42 00 0c 00 05 02 00 00 00
0010| c8 64 00 00 46 69 62 65 72 53 74 6f 72 65 20 20
0020| 20 20 20 20 00 00 90 65 53 46 50 2d 47 45 2d 31
0030| 30 30 46 58 20 20 20 20 41 20 20 20 05 1e 00 ac
0040| 00 1a 00 00 45 41 31 33 32 35 30 30 30 35 30 37
0050| 37 33 20 20 31 38 30 33 30 32 20 20 68 90 01 a8
0060| 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0070| 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0080| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0090| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00a0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00b0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00c0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00d0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00e0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00f0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
root@debian:/usr/src/eeprog-0.7.6# ./eeprog -x -f -8 -r 0:256 /dev/i2c-8 0x51
eeprog 0.7.6, a 24Cxx EEPROM reader/writer
Copyright (c) 2003-2004 by Stefano Barbato - All rights reserved.
Bus: /dev/i2c-8, Address: 0x51, Mode: 8bit
Reading 256 bytes from 0x0
0000| 7f ff 80 00 7f ff 80 00 ff ff 00 00 ff ff 00 00
0010| ff ff 00 00 ff ff 00 00 ff ff ff 80 84 ff ff ff
0020| ff ff 00 00 ff ff 00 00 00 00 00 00 00 00 00 00
0030| 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0040| 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0050| 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0060| 3a a0 7d 30 00 00 00 00 00 00 00 00 00 00 00 f8
0070| 01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00
0080| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0090| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00a0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00b0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00c0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00d0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00e0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00f0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
root@debian:/usr/src/eeprog-0.7.6# ./eeprog -x -f -8 -r 0:256 /dev/i2c-8 0x56
eeprog 0.7.6, a 24Cxx EEPROM reader/writer
Copyright (c) 2003-2004 by Stefano Barbato - All rights reserved.
Bus: /dev/i2c-8, Address: 0x56, Mode: 8bit
Reading 256 bytes from 0x0
0000| 00 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
0010| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
0020| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
0030| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
0040| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
0050| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
0060| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
0070| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
0080| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
0090| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
00a0| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
00b0| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
00c0| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
00d0| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
00e0| 21 79 00 60 01 00 00 20 00 00 00 00 00 00 00 30
00f0| 00 00 00 00 00 00 00 00 04 05 00 ff 00 00 00 00
#####################
SFP#2 Cisco-compliant
#####################
root@debian:/usr/src/eeprog-0.7.6# ./eeprog -x -f -8 -r 0:256 /dev/i2c-8 0x50
eeprog 0.7.6, a 24Cxx EEPROM reader/writer
Copyright (c) 2003-2004 by Stefano Barbato - All rights reserved.
Bus: /dev/i2c-8, Address: 0x50, Mode: 8bit
Reading 256 bytes from 0x0
0000| 00 00 07 00 00 00 00 00 00 00 00 02 01 00 0a 00
0010| 00 00 00 00 4f 45 4d 20 20 20 20 20 20 20 20 20
0020| 20 20 20 20 00 20 20 20 46 45 2d 31 30 30 4c 58
0030| 20 20 20 20 20 20 20 20 31 2e 30 20 05 1e 00 bb
0040| 00 1a 00 00 46 41 39 36 30 39 46 45 4c 30 34 20
0050| 20 20 20 20 30 39 30 36 31 36 20 20 00 00 00 ca
0060| 2c 00 08 ae aa 68 43 55 90 d3 69 ac 2b c0 85 a8
0070| 6b aa ee 00 00 00 00 00 00 00 00 00 a5 4d 36 da
0080| 31 30 31 30 32 32 2d 4d 33 37 20 20 20 20 20 20
0090| 32 30 31 30 31 30 32 36 20 20 20 20 20 20 20 20
00a0| 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
00b0| 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
00c0| 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
00d0| 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
00e0| 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20
00f0| 76 76 76 76 76 76 76 76 76 76 76 76 76 76 76 76
root@debian:/usr/src/eeprog-0.7.6# ./eeprog -x -f -8 -r 0:256 /dev/i2c-8 0x51
eeprog 0.7.6, a 24Cxx EEPROM reader/writer
Copyright (c) 2003-2004 by Stefano Barbato - All rights reserved.
Bus: /dev/i2c-8, Address: 0x51, Mode: 8bit
Reading 256 bytes from 0x0
0000| f7 ff ff ff ff ff ff ff ff fe ff ff ff ff ff ff
0010| ff ff ff ff ff ff ff ff ff ff ff 80 84 ff ff ff
0020| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0030| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0040| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0050| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0060| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0070| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0080| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0090| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00a0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00b0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00c0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00d0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00e0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
00f0| ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
root@debian:/usr/src/eeprog-0.7.6# ./eeprog -x -f -8 -r 0:256 /dev/i2c-8 0x56
eeprog 0.7.6, a 24Cxx EEPROM reader/writer
Copyright (c) 2003-2004 by Stefano Barbato - All rights reserved.
Bus: /dev/i2c-8, Address: 0x56, Mode: 8bit
Reading 256 bytes from 0x0
0000| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
0010| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
0020| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
0030| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
0040| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
0050| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
0060| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
0070| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
0080| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
0090| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
00a0| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
00b0| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
00c0| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
00d0| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
00e0| 21 XX 01 XX 00 XX 00 XX 00 XX 00 XX 00 XX 00 XX
00f0| 00 XX 00 XX 00 XX 00 XX 00 XX 80 XX 00 XX 00 XX
[-- Attachment #6: Attachment information. --]
[-- Type: text/plain, Size: 483 bytes --]
The following section of this message contains a file attachment
prepared for transmission using the Internet MIME message format.
If you are using Pegasus Mail, or any other MIME-compliant system,
you should be able to save it or view it from within your mailer.
If you cannot, please ask your system administrator for assistance.
---- File information -----------
File: SFP2_1st_subtrans_bad.png
Date: 20 Mar 2018, 9:32
Size: 65532 bytes.
Type: Unknown
[-- Attachment #7: SFP2_1st_subtrans_bad.png --]
[-- Type: Application/Octet-stream, Size: 65532 bytes --]
[-- Attachment #8: Attachment information. --]
[-- Type: text/plain, Size: 482 bytes --]
The following section of this message contains a file attachment
prepared for transmission using the Internet MIME message format.
If you are using Pegasus Mail, or any other MIME-compliant system,
you should be able to save it or view it from within your mailer.
If you cannot, please ask your system administrator for assistance.
---- File information -----------
File: SFP2_1st_subtrans_OK.png
Date: 20 Mar 2018, 9:29
Size: 63141 bytes.
Type: Unknown
[-- Attachment #9: SFP2_1st_subtrans_OK.png --]
[-- Type: Application/Octet-stream, Size: 63141 bytes --]
[-- Attachment #10: Attachment information. --]
[-- Type: text/plain, Size: 482 bytes --]
The following section of this message contains a file attachment
prepared for transmission using the Internet MIME message format.
If you are using Pegasus Mail, or any other MIME-compliant system,
you should be able to save it or view it from within your mailer.
If you cannot, please ask your system administrator for assistance.
---- File information -----------
File: SFP2_whole_trans_bad.png
Date: 20 Mar 2018, 9:25
Size: 65050 bytes.
Type: Unknown
[-- Attachment #11: SFP2_whole_trans_bad.png --]
[-- Type: Application/Octet-stream, Size: 65050 bytes --]
[-- Attachment #12: Attachment information. --]
[-- Type: text/plain, Size: 481 bytes --]
The following section of this message contains a file attachment
prepared for transmission using the Internet MIME message format.
If you are using Pegasus Mail, or any other MIME-compliant system,
you should be able to save it or view it from within your mailer.
If you cannot, please ask your system administrator for assistance.
---- File information -----------
File: SFP2_whole_trans_OK.png
Date: 20 Mar 2018, 9:23
Size: 68852 bytes.
Type: Unknown
[-- Attachment #13: SFP2_whole_trans_OK.png --]
[-- Type: Application/Octet-stream, Size: 68852 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Arend van Spriel @ 2018-03-20 9:03 UTC (permalink / raw)
To: Alexey Roslyakov
Cc: Florian Fainelli, Andrew Lunn, Kalle Valo, robh+dt, mark.rutland,
franky.lin, hante.meuleman, chi-hsien.lin, wright.feng, netdev,
linux-wireless, devicetree, linux-kernel, brcm80211-dev-list.pdl,
brcm80211-dev-list, Ulf Hansson
In-Reply-To: <CALFoz4aaEshpmyCxzut1t6cqSOSee4YP6vsi5G4Eei9m8hkZ3A@mail.gmail.com>
On 3/20/2018 8:58 AM, Alexey Roslyakov wrote:
> Arend,
>
>> Also I am not sure if the broken-sg-support is still needed. We added that for omap_hsmmc, but that has since changed to scatter-gather emulation so it might not be needed anymore.
> I can confirm it doesn't impact wifi performance in case of rk3288+ap6335.
>
> But I still have to set settings->bus.sdio.sd_head_align = 4 and
> settings->bus.sdio.sd_sgentry_align = 512, otherwise
> brcmf_sdiod_sglist_rw fails:
>
> 974.888452] net_ratelimit: 8 callbacks suppressed
> [ 974.888457] brcmfmac: brcmf_sdiod_sglist_rw: CMD53 sg block read failed -84
> [ 974.901527] brcmfmac: brcmf_sdio_rxglom: glom read of 6144 bytes failed: -5
> [ 974.910248] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame
> [ 975.018041] brcmfmac: brcmf_sdiod_sglist_rw: CMD53 sg block read failed -84
> [ 975.025833] brcmfmac: brcmf_sdio_rxglom: glom read of 6144 bytes failed: -5
> [ 975.033634] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame
> [ 975.033924] dwmmc_rockchip ff0d0000.dwmmc: Unexpected command
> timeout, state 0
> [ 975.049209] brcmfmac: brcmf_sdio_readframes: RXHEADER FAILED: -84
> [ 975.056034] brcmfmac: brcmf_sdio_rxfail: abort command, terminate
> frame, send NAK
> [ 975.068367] brcmfmac: brcmf_sdio_hdparse: HW header length too long
> [ 975.075379] brcmfmac: brcmf_sdio_rxfail: terminate frame
Hi Alex,
Thanks for checking. In your case I think you do not need sd_head_align
as it will default to either 4 or 8:
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
#define ALIGNMENT 8
#else
#define ALIGNMENT 4
#endif
I am not saying you should not be needing this. When it comes to DT
people are often tempted to accommodate a driver solution especially
when such a solution is already in place. However, DT is a hardware
description and these do not describe the wifi device. They are
applicable to the wifi device because it is a limitation of the host
controller and as such should be described in the DT binding of the host
controller.
Regards,
Arend
> Regards,
> Alex
>
> On 20 March 2018 at 06:16, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> + Uffe
>>
>> On 3/19/2018 6:55 PM, Florian Fainelli wrote:
>>>
>>> On 03/19/2018 07:10 AM, Alexey Roslyakov wrote:
>>>>
>>>> Hi Arend,
>>>> I appreciate your response. In my opinion, it has nothing to do with
>>>> SDIO host, because it defines "quirks" in the driver itself.
>>>
>>>
>>> It is not clear to me from your patch series whether the problem is that:
>>>
>>> - the SDIO device has a specific alignment requirements, which would be
>>> either a SDIO device driver limitation/issue or maybe the underlying
>>> hardware device/firmware requiring that
>>>
>>> - the SDIO host controller used is not capable of coping nicely with
>>> these said limitations
>>>
>>> It seems to me like what you are doing here is a) applicable to possibly
>>> more SDIO devices and host combinations, and b) should likely be done at
>>> the layer between the host and device, such that it is available to more
>>> combinations.
>>
>>
>> Indeed. That was my thought exactly and I can not imagine Uffe would push
>> back on that reasoning.
>>
>>>> If I get it right, you mean something like this:
>>>>
>>>> mmc3: mmc@1c12000 {
>>>> ...
>>>> broken-sg-support;
>>>> sd-head-align = 4;
>>>> sd-sgentry-align = 512;
>>>>
>>>> brcmf: wifi@1 {
>>>> ...
>>>> };
>>>> };
>>>>
>>>> Where dt: bindings documentation for these entries should reside?
>>>> In generic MMC bindings? Well, this is the very special case and
>>>> mmc-linux maintainer will unlikely to accept these changes.
>>>> Also, extra kernel code modification might be required. It could make
>>>> quite trivial change much more complex.
>>>
>>>
>>> If the MMC maintainers are not copied on this patch series, it will
>>> likely be hard for them to identify this patch series and chime in...
>>
>>
>> The main question is whether this is indeed a "very special case" as Alexey
>> claims it to be or that it is likely to be applicable to other device and
>> host combinations as you are suggesting.
>>
>> If these properties are imposed by the host or host controller it would make
>> sense to have these in the mmc bindings.
>>
>>>>
>>>>> Also I am not sure if the broken-sg-support is still needed. We added
>>>>> that for omap_hsmmc, but that has since changed to scatter-gather emulation
>>>>> so it might not be needed anymore.
>>>>
>>>>
>>>> I've experienced the problem with rk3288 (dw-mmc host) and sdio
>>>> settings like above solved it.
>>>> Frankly, I haven't investigated any deeper which one of the settings
>>>> helped in my case yet...
>>>> I will try to get rid of broken-sg-support first and let you know if
>>>> it does make any difference.
>>
>>
>> Are you using some chromebook. I have some lying around here so I could also
>> look into it. What broadcom chipset do you have?
>>
>> Regards,
>> Arend
>>
>>
>>>> All the best,
>>>> Alex.
>>>>
>>>> On 19 March 2018 at 16:31, Arend van Spriel
>>>> <arend.vanspriel@broadcom.com> wrote:
>>>>>
>>>>> On 3/19/2018 2:40 AM, Alexey Roslyakov wrote:
>>>>>>
>>>>>>
>>>>>> In case if the host has higher align requirements for SG items, allow
>>>>>> setting device-specific aligns for scatterlist items.
>>>>>>
>>>>>> Signed-off-by: Alexey Roslyakov <alexey.roslyakov@gmail.com>
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> | 5
>>>>>> +++++
>>>>>> 1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> index 86602f264dce..187b8c1b52a7 100644
>>>>>> ---
>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> +++
>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>> @@ -17,6 +17,11 @@ Optional properties:
>>>>>> When not specified the device will use in-band SDIO
>>>>>> interrupts.
>>>>>> - interrupt-names : name of the out-of-band interrupt, which must
>>>>>> be
>>>>>> set
>>>>>> to "host-wake".
>>>>>> + - brcm,broken-sg-support : boolean flag to indicate that the SDIO
>>>>>> host
>>>>>> + controller has higher align requirement than 32 bytes for each
>>>>>> + scatterlist item.
>>>>>> + - brcm,sd-head-align : alignment requirement for start of data
>>>>>> buffer.
>>>>>> + - brcm,sd-sgentry-align : length alignment requirement for each sg
>>>>>> entry.
>>>>>
>>>>>
>>>>>
>>>>> Hi Alexey,
>>>>>
>>>>> Thanks for the patch. However, the problem with these is that they are
>>>>> characterizing the host controller and not the wireless device. So from
>>>>> device tree perspective , which is to describe the hardware, these
>>>>> properties should be SDIO host controller properties. Also I am not sure
>>>>> if
>>>>> the broken-sg-support is still needed. We added that for omap_hsmmc, but
>>>>> that has since changed to scatter-gather emulation so it might not be
>>>>> needed
>>>>> anymore.
>>>>>
>>>>> Regards,
>>>>> Arend
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/3] kernel: add support for 256-bit IO access
From: Ingo Molnar @ 2018-03-20 9:08 UTC (permalink / raw)
To: Thomas Gleixner
Cc: David Laight, 'Rahul Lakkireddy', x86@kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mingo@redhat.com, hpa@zytor.com, davem@davemloft.net,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
ganeshgr@chelsio.com, nirranjan@chelsio.com, indranil@chelsio.com,
Andy Lutomirski, Peter Zijlstra, Fenghua Yu, Eric Biggers
In-Reply-To: <alpine.DEB.2.21.1803200933320.6506@nanos.tec.linutronix.de>
* Thomas Gleixner <tglx@linutronix.de> wrote:
> > So I do think we could do more in this area to improve driver performance, if the
> > code is correct and if there's actual benchmarks that are showing real benefits.
>
> If it's about hotpath performance I'm all for it, but the use case here is
> a debug facility...
>
> And if we go down that road then we want a AVX based memcpy()
> implementation which is runtime conditional on the feature bit(s) and
> length dependent. Just slapping a readqq() at it and use it in a loop does
> not make any sense.
Yeah, so generic memcpy() replacement is only feasible I think if the most
optimistic implementation is actually correct:
- if no preempt disable()/enable() is required
- if direct access to the AVX[2] registers does not disturb legacy FPU state in
any fashion
- if direct access to the AVX[2] registers cannot raise weird exceptions or have
weird behavior if the FPU control word is modified to non-standard values by
untrusted user-space
If we have to touch the FPU tag or control words then it's probably only good for
a specialized API.
Thanks,
Ingo
^ permalink raw reply
* Recall: DTS for our Configuration
From: Alayev Michael @ 2018-03-20 9:18 UTC (permalink / raw)
To: 'Andrew Lunn'
Cc: 'netdev@vger.kernel.org', Efter Yoram, Dror Alon
Alayev Michael would like to recall the message, "DTS for our Configuration".
***********************************************************************************************
Please consider the environment before printing this email !
The information contained in this communication is proprietary to Israel Aerospace Industries Ltd. and/or third parties, may contain confidential or privileged information, and is intended only for the use of the intended addressee thereof.
If you are not the intended addressee, please be aware that any use, disclosure, distribution and/or copying of this communication is strictly prohibited. If you receive this communication in error, please notify the sender immediately and delete it from your computer.
Thank you.
Visit us at: www.iai.co.il
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Use the DT IRQ trigger mode
From: Uwe Kleine-König @ 2018-03-20 9:20 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev
In-Reply-To: <1521494181-4653-2-git-send-email-andrew@lunn.ch>
On Mon, Mar 19, 2018 at 10:16:20PM +0100, Andrew Lunn wrote:
> By calling request_threaded_irq() with the flag IRQF_TRIGGER_FALLING
> we override the trigger mode provided in device tree. And the
> interrupt is actually active low, which is what all the current device
> tree descriptions use.
>
> Suggested-by: Uwe Kleine-K�nig <u.kleine-koenig@pengutronix.de>
Your locale setting seems broken. Can this please be fixed up during
commit?
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index fe46b40195fa..84e6febaf881 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -425,7 +425,7 @@ static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
>
> err = request_threaded_irq(chip->irq, NULL,
> mv88e6xxx_g1_irq_thread_fn,
> - IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
> + IRQF_ONESHOT,
> dev_name(chip->dev), chip);
You could join the shortend line with the next here.
Other than that:
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 0/4] net: dsa: mv88e6xxx: novice fixes and irq handling
From: Uwe Kleine-König @ 2018-03-20 9:30 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, Marc Zyngier, Thomas Gleixner, kernel,
Florian Fainelli, netdev, Gregory CLEMENT
In-Reply-To: <20180319163713.GH26039@lunn.ch>
Hello Andrew,
On Mon, Mar 19, 2018 at 05:37:13PM +0100, Andrew Lunn wrote:
> > When I tested the second change however the driver still failed
> > because the gpio controller doesn't support level sensitive
> > irqs. :-|
>
> Do you have documentation for the SoC? Is it a hardware limitation, or
> just missing from the pinctrl driver?
I didn't check yet. This is an Armada 3720, the hardware specification
is available from Marvell. I'll take a look after resending my series
with your Reviewed-by tags.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K�nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Alexey Roslyakov @ 2018-03-20 9:35 UTC (permalink / raw)
To: Arend van Spriel
Cc: Florian Fainelli, Andrew Lunn, Kalle Valo, robh+dt, mark.rutland,
franky.lin, hante.meuleman, chi-hsien.lin, wright.feng, netdev,
linux-wireless, devicetree, linux-kernel, brcm80211-dev-list.pdl,
brcm80211-dev-list, Ulf Hansson
In-Reply-To: <5AB0CE6B.2050109@broadcom.com>
Arend,
Agreed. Let's dismiss these patches.
Now I'm curious if I can get the information about DMA SG limitations
from MMC layer, I'll try to figure out something.
BTW, my specific setup (with default alignments) triggers kernel panic
(I see brcm_* in backtrace). It's better if I create separate email
chain for the issue.
Thanks,
Alex.
On 20 March 2018 at 16:03, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 3/20/2018 8:58 AM, Alexey Roslyakov wrote:
>>
>> Arend,
>>
>>> Also I am not sure if the broken-sg-support is still needed. We added
>>> that for omap_hsmmc, but that has since changed to scatter-gather emulation
>>> so it might not be needed anymore.
>>
>> I can confirm it doesn't impact wifi performance in case of rk3288+ap6335.
>>
>> But I still have to set settings->bus.sdio.sd_head_align = 4 and
>> settings->bus.sdio.sd_sgentry_align = 512, otherwise
>> brcmf_sdiod_sglist_rw fails:
>>
>> 974.888452] net_ratelimit: 8 callbacks suppressed
>> [ 974.888457] brcmfmac: brcmf_sdiod_sglist_rw: CMD53 sg block read failed
>> -84
>> [ 974.901527] brcmfmac: brcmf_sdio_rxglom: glom read of 6144 bytes
>> failed: -5
>> [ 974.910248] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame
>> [ 975.018041] brcmfmac: brcmf_sdiod_sglist_rw: CMD53 sg block read failed
>> -84
>> [ 975.025833] brcmfmac: brcmf_sdio_rxglom: glom read of 6144 bytes
>> failed: -5
>> [ 975.033634] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame
>> [ 975.033924] dwmmc_rockchip ff0d0000.dwmmc: Unexpected command
>> timeout, state 0
>> [ 975.049209] brcmfmac: brcmf_sdio_readframes: RXHEADER FAILED: -84
>> [ 975.056034] brcmfmac: brcmf_sdio_rxfail: abort command, terminate
>> frame, send NAK
>> [ 975.068367] brcmfmac: brcmf_sdio_hdparse: HW header length too long
>> [ 975.075379] brcmfmac: brcmf_sdio_rxfail: terminate frame
>
>
> Hi Alex,
>
> Thanks for checking. In your case I think you do not need sd_head_align as
> it will default to either 4 or 8:
>
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> #define ALIGNMENT 8
> #else
> #define ALIGNMENT 4
> #endif
>
> I am not saying you should not be needing this. When it comes to DT people
> are often tempted to accommodate a driver solution especially when such a
> solution is already in place. However, DT is a hardware description and
> these do not describe the wifi device. They are applicable to the wifi
> device because it is a limitation of the host controller and as such should
> be described in the DT binding of the host controller.
>
> Regards,
> Arend
>
>
>> Regards,
>> Alex
>>
>> On 20 March 2018 at 06:16, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>>
>>> + Uffe
>>>
>>> On 3/19/2018 6:55 PM, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 03/19/2018 07:10 AM, Alexey Roslyakov wrote:
>>>>>
>>>>>
>>>>> Hi Arend,
>>>>> I appreciate your response. In my opinion, it has nothing to do with
>>>>> SDIO host, because it defines "quirks" in the driver itself.
>>>>
>>>>
>>>>
>>>> It is not clear to me from your patch series whether the problem is
>>>> that:
>>>>
>>>> - the SDIO device has a specific alignment requirements, which would be
>>>> either a SDIO device driver limitation/issue or maybe the underlying
>>>> hardware device/firmware requiring that
>>>>
>>>> - the SDIO host controller used is not capable of coping nicely with
>>>> these said limitations
>>>>
>>>> It seems to me like what you are doing here is a) applicable to possibly
>>>> more SDIO devices and host combinations, and b) should likely be done at
>>>> the layer between the host and device, such that it is available to more
>>>> combinations.
>>>
>>>
>>>
>>> Indeed. That was my thought exactly and I can not imagine Uffe would push
>>> back on that reasoning.
>>>
>>>>> If I get it right, you mean something like this:
>>>>>
>>>>> mmc3: mmc@1c12000 {
>>>>> ...
>>>>> broken-sg-support;
>>>>> sd-head-align = 4;
>>>>> sd-sgentry-align = 512;
>>>>>
>>>>> brcmf: wifi@1 {
>>>>> ...
>>>>> };
>>>>> };
>>>>>
>>>>> Where dt: bindings documentation for these entries should reside?
>>>>> In generic MMC bindings? Well, this is the very special case and
>>>>> mmc-linux maintainer will unlikely to accept these changes.
>>>>> Also, extra kernel code modification might be required. It could make
>>>>> quite trivial change much more complex.
>>>>
>>>>
>>>>
>>>> If the MMC maintainers are not copied on this patch series, it will
>>>> likely be hard for them to identify this patch series and chime in...
>>>
>>>
>>>
>>> The main question is whether this is indeed a "very special case" as
>>> Alexey
>>> claims it to be or that it is likely to be applicable to other device and
>>> host combinations as you are suggesting.
>>>
>>> If these properties are imposed by the host or host controller it would
>>> make
>>> sense to have these in the mmc bindings.
>>>
>>>>>
>>>>>> Also I am not sure if the broken-sg-support is still needed. We added
>>>>>> that for omap_hsmmc, but that has since changed to scatter-gather
>>>>>> emulation
>>>>>> so it might not be needed anymore.
>>>>>
>>>>>
>>>>>
>>>>> I've experienced the problem with rk3288 (dw-mmc host) and sdio
>>>>> settings like above solved it.
>>>>> Frankly, I haven't investigated any deeper which one of the settings
>>>>> helped in my case yet...
>>>>> I will try to get rid of broken-sg-support first and let you know if
>>>>> it does make any difference.
>>>
>>>
>>>
>>> Are you using some chromebook. I have some lying around here so I could
>>> also
>>> look into it. What broadcom chipset do you have?
>>>
>>> Regards,
>>> Arend
>>>
>>>
>>>>> All the best,
>>>>> Alex.
>>>>>
>>>>> On 19 March 2018 at 16:31, Arend van Spriel
>>>>> <arend.vanspriel@broadcom.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 3/19/2018 2:40 AM, Alexey Roslyakov wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> In case if the host has higher align requirements for SG items, allow
>>>>>>> setting device-specific aligns for scatterlist items.
>>>>>>>
>>>>>>> Signed-off-by: Alexey Roslyakov <alexey.roslyakov@gmail.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>>> | 5
>>>>>>> +++++
>>>>>>> 1 file changed, 5 insertions(+)
>>>>>>>
>>>>>>> diff --git
>>>>>>>
>>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>>>
>>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>>> index 86602f264dce..187b8c1b52a7 100644
>>>>>>> ---
>>>>>>>
>>>>>>> a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>>> +++
>>>>>>>
>>>>>>> b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>>>>>>> @@ -17,6 +17,11 @@ Optional properties:
>>>>>>> When not specified the device will use in-band SDIO
>>>>>>> interrupts.
>>>>>>> - interrupt-names : name of the out-of-band interrupt, which
>>>>>>> must
>>>>>>> be
>>>>>>> set
>>>>>>> to "host-wake".
>>>>>>> + - brcm,broken-sg-support : boolean flag to indicate that the SDIO
>>>>>>> host
>>>>>>> + controller has higher align requirement than 32 bytes for
>>>>>>> each
>>>>>>> + scatterlist item.
>>>>>>> + - brcm,sd-head-align : alignment requirement for start of data
>>>>>>> buffer.
>>>>>>> + - brcm,sd-sgentry-align : length alignment requirement for each sg
>>>>>>> entry.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Alexey,
>>>>>>
>>>>>> Thanks for the patch. However, the problem with these is that they are
>>>>>> characterizing the host controller and not the wireless device. So
>>>>>> from
>>>>>> device tree perspective , which is to describe the hardware, these
>>>>>> properties should be SDIO host controller properties. Also I am not
>>>>>> sure
>>>>>> if
>>>>>> the broken-sg-support is still needed. We added that for omap_hsmmc,
>>>>>> but
>>>>>> that has since changed to scatter-gather emulation so it might not be
>>>>>> needed
>>>>>> anymore.
>>>>>>
>>>>>> Regards,
>>>>>> Arend
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
--
With best regards,
Alexey Roslyakov
Email: alexey.roslyakov@gmail.com
^ permalink raw reply
* Re: [PATCH bpf] bpf: fix crash due to inode i_op mismatch with clang/llvm
From: Daniel Borkmann @ 2018-03-20 9:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alexei Starovoitov, psodagud, fengc, Network Development
In-Reply-To: <CA+55aFxxXYuuReoQYJxnGX4UTVLbZ-jDh3r1t0NpEN-tqFMxzw@mail.gmail.com>
On 03/20/2018 03:06 AM, Linus Torvalds wrote:
> On Mon, Mar 19, 2018 at 6:50 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Add it to everything. If it's an invalid optimization, it shouldn't be on.
>
> IOW, why isn't this just something like
>
> diff --git a/Makefile b/Makefile
> index d65e2e229017..01abedc2e79f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -826,6 +826,9 @@ KBUILD_CFLAGS += $(call cc-disable-warning, pointer-sign)
> # disable invalid "can't wrap" optimizations for signed / pointers
> KBUILD_CFLAGS += $(call cc-option,-fno-strict-overflow)
>
> +# disable invalid optimization on clang
> +KBUILD_CFLAGS += $(call cc-option,-fno-merge-all-constants)
> +
> # Make sure -fstack-check isn't enabled (like gentoo apparently did)
> KBUILD_CFLAGS += $(call cc-option,-fno-stack-check,)
>
> (whitespace-damaged, but you get the gist of it).
>
> We disable some optimizations that are technically _valid_, because
> they are too dangerous and a bad idea.
>
> Disabling an optimization that isn't valid EVEN IN THEORY is an
> absolute no-brainer, particularly if it has already shown itself to
> cause problems.
>
> We have other situations where we generate multiple static structures
> and expect them to be unique. I'm not sure any of them would trigger
> the clang rules, but the clang rules are obviously complete garbage
> anyway, so who knows?
>
> That optimization seems to teuly be pure and utter garbage. Clang can
> even *see* the address comparison happening in that file.
>
> Some clang person needs to be publicly shamed for enabling this kind
> of garbage by default, particularly since they apparently _knew_ it
> was invalid.
Yeah, agree with all the above. I'll respin it to disable globally.
Thanks, Linus!
^ permalink raw reply
* Re: [RFC PATCH 0/3] kernel: add support for 256-bit IO access
From: Thomas Gleixner @ 2018-03-20 9:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Laight, 'Rahul Lakkireddy', x86@kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mingo@redhat.com, hpa@zytor.com, davem@davemloft.net,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
ganeshgr@chelsio.com, nirranjan@chelsio.com, indranil@chelsio.com,
Andy Lutomirski, Peter Zijlstra, Fenghua Yu, Eric Biggers
In-Reply-To: <20180320090802.qw4tqjmhy6yfd6sf@gmail.com>
On Tue, 20 Mar 2018, Ingo Molnar wrote:
> * Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > > So I do think we could do more in this area to improve driver performance, if the
> > > code is correct and if there's actual benchmarks that are showing real benefits.
> >
> > If it's about hotpath performance I'm all for it, but the use case here is
> > a debug facility...
> >
> > And if we go down that road then we want a AVX based memcpy()
> > implementation which is runtime conditional on the feature bit(s) and
> > length dependent. Just slapping a readqq() at it and use it in a loop does
> > not make any sense.
>
> Yeah, so generic memcpy() replacement is only feasible I think if the most
> optimistic implementation is actually correct:
>
> - if no preempt disable()/enable() is required
>
> - if direct access to the AVX[2] registers does not disturb legacy FPU state in
> any fashion
>
> - if direct access to the AVX[2] registers cannot raise weird exceptions or have
> weird behavior if the FPU control word is modified to non-standard values by
> untrusted user-space
>
> If we have to touch the FPU tag or control words then it's probably only good for
> a specialized API.
I did not mean to have a general memcpy replacement. Rather something like
magic_memcpy() which falls back to memcpy when AVX is not usable or the
length does not justify the AVX stuff at all.
Thanks,
tglx
^ permalink raw reply
* [PATCH v2 1/3] net: dsa: mv88e6xxx: Fix name of switch 88E6141
From: Uwe Kleine-König @ 2018-03-20 9:44 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, Vivien Didelot, kernel, Gregory CLEMENT, netdev,
Florian Fainelli
In-Reply-To: <20180320094442.18368-1-u.kleine-koenig@pengutronix.de>
The switch name is emitted in the kernel log, so having the right name
there is nice.
Fixes: 1558727a1c1b ("net: dsa: mv88e6xxx: Add support for ethernet switch 88E6141")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index fe46b40195fa..6439f7d6c4d6 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3427,7 +3427,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
[MV88E6141] = {
.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6141,
.family = MV88E6XXX_FAMILY_6341,
- .name = "Marvell 88E6341",
+ .name = "Marvell 88E6141",
.num_databases = 4096,
.num_ports = 6,
.num_internal_phys = 5,
--
2.16.2
^ permalink raw reply related
* [PATCH v2 0/3] net: dsa: mv88e6xxx: some fixes
From: Uwe Kleine-König @ 2018-03-20 9:44 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, Vivien Didelot, kernel, Gregory CLEMENT, netdev,
Florian Fainelli
Hello,
these patches target net-next and got approved by Andrew Lunn.
Compared to (implicit) v1, I dropped the patch that I didn't know if it
was right because of missing documentation on my side. But Andrew
already cared for that in a patch that is now adfccf118211 in net-next.
Best regards
Uwe
Uwe Kleine-König (3):
net: dsa: mv88e6xxx: Fix name of switch 88E6141
net: dsa: mv88e6xxx: Fix typo in a comment
net: dsa: mv88e6xxx: Fix interrupt name for g2 irq
drivers/net/dsa/mv88e6xxx/chip.c | 4 ++--
drivers/net/dsa/mv88e6xxx/global2.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.16.2
^ permalink raw reply
* [PATCH v2 3/3] net: dsa: mv88e6xxx: Fix interrupt name for g2 irq
From: Uwe Kleine-König @ 2018-03-20 9:44 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, Vivien Didelot, kernel, Gregory CLEMENT, netdev,
Florian Fainelli
In-Reply-To: <20180320094442.18368-1-u.kleine-koenig@pengutronix.de>
This changes the respective line in /proc/interrupts from
49: x x mv88e6xxx-g1 7 Edge mv88e6xxx-g1
to
49: x x mv88e6xxx-g1 7 Edge mv88e6xxx-g2
which makes more sense.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/net/dsa/mv88e6xxx/global2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 6c620974fef3..0ce627fded48 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -1090,7 +1090,7 @@ int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip)
err = request_threaded_irq(chip->device_irq, NULL,
mv88e6xxx_g2_irq_thread_fn,
- IRQF_ONESHOT, "mv88e6xxx-g1", chip);
+ IRQF_ONESHOT, "mv88e6xxx-g2", chip);
if (err)
goto out;
--
2.16.2
^ permalink raw reply related
* [PATCH v2 2/3] net: dsa: mv88e6xxx: Fix typo in a comment
From: Uwe Kleine-König @ 2018-03-20 9:44 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, Vivien Didelot, kernel, Gregory CLEMENT, netdev,
Florian Fainelli
In-Reply-To: <20180320094442.18368-1-u.kleine-koenig@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 6439f7d6c4d6..fd78378ad6b1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4175,7 +4175,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
}
/* Has to be performed before the MDIO bus is created, because
- * the PHYs will link there interrupts to these interrupt
+ * the PHYs will link their interrupts to these interrupt
* controllers
*/
mutex_lock(&chip->reg_lock);
--
2.16.2
^ permalink raw reply related
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Kalle Valo @ 2018-03-20 9:55 UTC (permalink / raw)
To: Arend van Spriel
Cc: Florian Fainelli, Alexey Roslyakov, Andrew Lunn, robh+dt,
mark.rutland, franky.lin, hante.meuleman, chi-hsien.lin,
wright.feng, netdev, linux-wireless, devicetree, linux-kernel,
brcm80211-dev-list.pdl, brcm80211-dev-list, Ulf Hansson
In-Reply-To: <5AB044C0.9060701@broadcom.com>
Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>> If I get it right, you mean something like this:
>>>
>>> mmc3: mmc@1c12000 {
>>> ...
>>> broken-sg-support;
>>> sd-head-align = 4;
>>> sd-sgentry-align = 512;
>>>
>>> brcmf: wifi@1 {
>>> ...
>>> };
>>> };
>>>
>>> Where dt: bindings documentation for these entries should reside?
>>> In generic MMC bindings? Well, this is the very special case and
>>> mmc-linux maintainer will unlikely to accept these changes.
>>> Also, extra kernel code modification might be required. It could make
>>> quite trivial change much more complex.
>>
>> If the MMC maintainers are not copied on this patch series, it will
>> likely be hard for them to identify this patch series and chime in...
>
> The main question is whether this is indeed a "very special case" as
> Alexey claims it to be or that it is likely to be applicable to other
> device and host combinations as you are suggesting.
>
> If these properties are imposed by the host or host controller it
> would make sense to have these in the mmc bindings.
BTW, last year we were discussing something similar (I mean related to
alignment requirements) with ath10k SDIO patches and at the time the
patch submitter was proposing to have a bounce buffer in ath10k to
workaround that. I don't remember the details anymore, they are on the
ath10k mailing list archive if anyone is curious to know, but I would
not be surprised if they are similar as here. So there might be a need
to solve this in a generic way (but not sure of course as I haven't
checked the details).
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2 00/21] Allow compile-testing NO_DMA (drivers)
From: Geert Uytterhoeven @ 2018-03-20 9:57 UTC (permalink / raw)
To: Wolfram Sang
Cc: Ulf Hansson, linux-iio, linux-fpga, linux-remoteproc,
ALSA Development Mailing List, Bjorn Andersson, Eric Anholt,
netdev, MTD Maling List, Linux I2C, linux1394-devel,
Christoph Hellwig, Marek Szyprowski, Stefan Wahren,
Boris Brezillon, James E . J . Bottomley, Herbert Xu, scsi,
Richard Weinberger, Joerg Roedel, Jassi Brar, Marek Vasut,
linux-seria
In-Reply-To: <20180316212317.7d5rri3p25hpmx44@ninjato>
Hi Wolfram,
On Fri, Mar 16, 2018 at 10:23 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> To avoid allmodconfig/allyesconfig regressions on NO_DMA=y platforms,
>> this (drivers) series should be applied after the previous (core)
>> series (but not many people may notice/care ;-)
>
> I still don't get if there is a dependency on the core patches. I.e.
> shall I apply the subsystem patch now by myself or do you want to push
> the series after the core patch and need my ack here?
Yes, there is a dependency. Without the core patches, enabling COMPILE_TEST,
and compile-testing drivers irrelevant on obscure NO_DMA=y platforms will
cause build failures.
To play it safe, you want to postpone the subsystem patches until the core
part has landed upstream. I will rebase and resubmit after v4.17-rc1.
Thanks, and sorry for being unclear.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* RE: [RFC PATCH 0/3] kernel: add support for 256-bit IO access
From: David Laight @ 2018-03-20 9:59 UTC (permalink / raw)
To: 'Thomas Gleixner', Ingo Molnar
Cc: 'Rahul Lakkireddy', x86@kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
mingo@redhat.com, hpa@zytor.com, davem@davemloft.net,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
ganeshgr@chelsio.com, nirranjan@chelsio.com, indranil@chelsio.com,
Andy Lutomirski, Peter Zijlstra, Fenghua Yu, Eric Biggers
In-Reply-To: <alpine.DEB.2.21.1803201039460.6506@nanos.tec.linutronix.de>
From: Thomas Gleixner
> Sent: 20 March 2018 09:41
> On Tue, 20 Mar 2018, Ingo Molnar wrote:
> > * Thomas Gleixner <tglx@linutronix.de> wrote:
...
> > > And if we go down that road then we want a AVX based memcpy()
> > > implementation which is runtime conditional on the feature bit(s) and
> > > length dependent. Just slapping a readqq() at it and use it in a loop does
> > > not make any sense.
> >
> > Yeah, so generic memcpy() replacement is only feasible I think if the most
> > optimistic implementation is actually correct:
> >
> > - if no preempt disable()/enable() is required
> >
> > - if direct access to the AVX[2] registers does not disturb legacy FPU state in
> > any fashion
> >
> > - if direct access to the AVX[2] registers cannot raise weird exceptions or have
> > weird behavior if the FPU control word is modified to non-standard values by
> > untrusted user-space
> >
> > If we have to touch the FPU tag or control words then it's probably only good for
> > a specialized API.
>
> I did not mean to have a general memcpy replacement. Rather something like
> magic_memcpy() which falls back to memcpy when AVX is not usable or the
> length does not justify the AVX stuff at all.
There is probably no point for memcpy().
Where it would make a big difference is memcpy_fromio() for PCIe devices
(where longer TLP make a big difference).
But any code belongs in its implementation not in every driver.
The implementation of memcpy_toio() is nothing like as critical.
If might be the code would need to fallback to 64bit accesses
if the AVX(2) registers can't currently be accessed - maybe some
obscure state....
However memcpy_to/fromio() are both horrid at the moment because
they result in byte copies!
David
^ permalink raw reply
* Re: [PATCH v2 00/21] Allow compile-testing NO_DMA (drivers)
From: Wolfram Sang @ 2018-03-20 10:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Ulf Hansson, linux-iio, linux-fpga, linux-remoteproc,
ALSA Development Mailing List, Bjorn Andersson, Eric Anholt,
netdev, MTD Maling List, Linux I2C, linux1394-devel,
Christoph Hellwig, Marek Szyprowski, Stefan Wahren,
Boris Brezillon, James E . J . Bottomley, Herbert Xu, scsi,
Richard Weinberger, Joerg Roedel, Jassi Brar, Marek Vasut,
linux-seria
In-Reply-To: <CAMuHMdUUKMngcyajm_s==KbhL3ZjMNHVxhNNGkPWkpnePE2rMw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 247 bytes --]
> To play it safe, you want to postpone the subsystem patches until the core
> part has landed upstream. I will rebase and resubmit after v4.17-rc1.
Thanks for the heads up. I'll wait for the rebased patch then and apply
it after rc1 for 4.17.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 144 bytes --]
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox