From patchwork Mon Jan 18 22:05:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arend van Spriel X-Patchwork-Id: 8057361 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1BBFD9F96D for ; Mon, 18 Jan 2016 22:05:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0D08C204A2 for ; Mon, 18 Jan 2016 22:05:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B40E120465 for ; Mon, 18 Jan 2016 22:05:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756728AbcARWFw (ORCPT ); Mon, 18 Jan 2016 17:05:52 -0500 Received: from mail-gw3-out.broadcom.com ([216.31.210.64]:38245 "EHLO mail-gw3-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754871AbcARWFt (ORCPT ); Mon, 18 Jan 2016 17:05:49 -0500 X-IronPort-AV: E=Sophos;i="5.22,314,1449561600"; d="scan'208";a="85604606" Received: from irvexchcas07.broadcom.com (HELO IRVEXCHCAS07.corp.ad.broadcom.com) ([10.9.208.55]) by mail-gw3-out.broadcom.com with ESMTP; 18 Jan 2016 14:45:00 -0800 Received: from IRVEXCHSMTP2.corp.ad.broadcom.com (10.9.207.52) by IRVEXCHCAS07.corp.ad.broadcom.com (10.9.208.55) with Microsoft SMTP Server (TLS) id 14.3.235.1; Mon, 18 Jan 2016 14:05:49 -0800 Received: from mail-sj1-12.sj.broadcom.com (10.10.10.20) by IRVEXCHSMTP2.corp.ad.broadcom.com (10.9.207.52) with Microsoft SMTP Server id 14.3.235.1; Mon, 18 Jan 2016 14:05:49 -0800 Received: from [10.177.253.85] (unknown [10.177.253.85]) by mail-sj1-12.sj.broadcom.com (Postfix) with ESMTP id 5FED227A83; Mon, 18 Jan 2016 14:05:47 -0800 (PST) Subject: Re: [GIT] Networking To: Marc Zyngier , Kalle Valo , Hante Meuleman References: <20160112.215136.850414372508130298.davem@davemloft.net> <56978094.4050302@arm.com> <87k2n97n5e.fsf@kamboji.qca.qualcomm.com> <569CCCD2.4040105@arm.com> CC: David Miller , , , From: Arend van Spriel X-Enigmail-Draft-Status: N1110 Message-ID: <569D61BA.8020701@broadcom.com> Date: Mon, 18 Jan 2016 23:05:46 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <569CCCD2.4040105@arm.com> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 18-1-2016 12:30, Marc Zyngier wrote: > Hi Kalle, > > On 16/01/16 11:57, Kalle Valo wrote: >> Marc Zyngier writes: >> >>> David, Hante, >>> >>> On 13/01/16 02:51, David Miller wrote: >>> >>> [...] >>> >>>> Hante Meuleman (33): >>> [...] >>>> brcmfmac: Move all module parameters to one place >> >> As a reminder to myself this is the commit id: >> >> 7d34b0560567 brcmfmac: Move all module parameters to one place >> >>> This particular patch breaks one of my boxes in a spectacular way: >>> >>> [ 3.602155] Unable to handle kernel paging request at virtual address 000027e4 >>> [ 3.602160] pgd = c0003000 >>> [ 3.602169] [000027e4] *pgd=80000040004003, *pmd=00000000 >>> [ 3.602181] Internal error: Oops: 206 [#1] PREEMPT SMP ARM >> >> [...] >> >>> This is caused by this hunk: >>> >>> @@ -890,7 +887,8 @@ static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev) >>> if (!sdiodev->sg_support) >>> return; >>> >>> - nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE, brcmf_sdiod_txglomsz); >>> + nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE, >>> + sdiodev->bus_if->drvr->settings->sdiod_txglomsz); >>> nents += (nents >> 4) + 1; >>> >>> WARN_ON(nents > sdiodev->max_segment_count); >>> >>> were drvr->settings is NULL (as the settings allocation seems to be done >>> much later). The fix is not completely obvious to me (probably requires >>> pushing the call to brcmf_mp_device_attach() down into the various bus >>> specific functions). An alternative would be to restore the txglomsz >>> parameter as it was before and not rely on settings being allocated. >> >> Should we revert the patch or can you Hante fix this? The revert doesn't >> seem to be trivial so I would appreciate if someone could send a patch. > > I've worked out a partial revert (see below) that allows my system to > boot, but I'd rather see a proper fix from the maintainer of this code. Hi Marc, Thanks for the patch, but Hante has created a different patch basically deferring the allocation of the sgtable. Feel free to give it a spin on your box and share the results. I am looking for a arm64 platform to add to my test machines so if you can recommend one (with PCIe). Regards, Arend From f7d015866f24cfad5af0c35b1ab60c567835c5e8 Mon Sep 17 00:00:00 2001 From: Hante Meuleman Date: Mon, 18 Jan 2016 15:19:45 +0100 Subject: [PATCH] brcmfmac: fix sdio sg table alloc crash With the patch to move all module paramaters to one place a bug was introduced causing a null pointer exception. This patch fixes the bug by initializing the sg table till after the settings have been initialized. Reviewed-by: Arend Van Spriel Reviewed-by: Franky (Zhenhui) Lin Reviewed-by: Pieter-Paul Giesberts Signed-off-by: Hante Meuleman Signed-off-by: Arend van Spriel Reported-by: Marc Zyngier Tested-by: Marc Zyngier --- .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 40 ++++++++-------------- .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 5 +++ .../wireless/broadcom/brcm80211/brcmfmac/sdio.h | 1 + 3 files changed, 21 insertions(+), 25 deletions(-) #ifdef CONFIG_PM_SLEEP diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 5363739..b98db8a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -879,11 +879,24 @@ int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn) return 0; } -static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev) +void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev) { + struct sdio_func *func; + struct mmc_host *host; + uint max_blocks; uint nents; int err; + func = sdiodev->func[2]; + host = func->card->host; + sdiodev->sg_support = host->max_segs > 1; + max_blocks = min_t(uint, host->max_blk_count, 511u); + sdiodev->max_request_size = min_t(uint, host->max_req_size, + max_blocks * func->cur_blksize); + sdiodev->max_segment_count = min_t(uint, host->max_segs, + SG_MAX_SINGLE_ALLOC); + sdiodev->max_segment_size = host->max_seg_size; + if (!sdiodev->sg_support) return; @@ -1021,9 +1034,6 @@ static void brcmf_sdiod_host_fixup(struct mmc_host *host) static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev) { - struct sdio_func *func; - struct mmc_host *host; - uint max_blocks; int ret = 0; sdiodev->num_funcs = 2; @@ -1054,26 +1064,6 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev) goto out; } - /* - * determine host related variables after brcmf_sdiod_probe() - * as func->cur_blksize is properly set and F2 init has been - * completed successfully. - */ - func = sdiodev->func[2]; - host = func->card->host; - sdiodev->sg_support = host->max_segs > 1; - max_blocks = min_t(uint, host->max_blk_count, 511u); - sdiodev->max_request_size = min_t(uint, host->max_req_size, - max_blocks * func->cur_blksize); - sdiodev->max_segment_count = min_t(uint, host->max_segs, - SG_MAX_SINGLE_ALLOC); - sdiodev->max_segment_size = host->max_seg_size; - - /* allocate scatter-gather table. sg support - * will be disabled upon allocation failure. - */ - brcmf_sdiod_sgtable_alloc(sdiodev); - ret = brcmf_sdiod_freezer_attach(sdiodev); if (ret) goto out; @@ -1084,7 +1074,7 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev) ret = -ENODEV; goto out; } - brcmf_sdiod_host_fixup(host); + brcmf_sdiod_host_fixup(sdiodev->func[2]->card->host); out: if (ret) brcmf_sdiod_remove(sdiodev); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index dd66143..a14d9d9d 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -4114,6 +4114,11 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev) goto fail; } + /* allocate scatter-gather table. sg support + * will be disabled upon allocation failure. + */ + brcmf_sdiod_sgtable_alloc(bus->sdiodev); + /* Query the F2 block size, set roundup accordingly */ bus->blocksize = bus->sdiodev->func[2]->cur_blksize; bus->roundup = min(max_roundup, bus->blocksize); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h index 5ec7a6d..23f2231 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h @@ -342,6 +342,7 @@ int brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address, /* Issue an abort to the specified function */ int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn); +void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev); void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev, enum brcmf_sdiod_state state);