Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
From: Tom Psyborg <pozega.tomislav@gmail.com>
To: Christian Lamparter <chunkeey@gmail.com>
Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
	Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH] ath10k: restore QCA9880-AR1A (v1) detection
Date: Sun, 8 Sep 2019 08:32:35 +0200
Message-ID: <CAKR_QV+y9u_gP_+dZ=bFYJgANeq+W19v9Xx_SwydB5fe4ozhtg@mail.gmail.com> (raw)
In-Reply-To: <20190906215423.23589-1-chunkeey@gmail.com>

On 06/09/2019, Christian Lamparter <chunkeey@gmail.com> wrote:
> This patch restores the old behavior that read
> the chip_id on the QCA988x before resetting the
> chip. This needs to be done in this order since
> the unsupported QCA988x AR1A chips fall off the
> bus when resetted. Otherwise the next MMIO Op
> after the reset causes a BUS ERROR and panic.
>
> Cc: stable@vger.kernel.org
> Fixes: 1a7fecb766c8 ("ath10k: reset chip before reading chip_id in probe")
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/pci.c | 36 +++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c
> b/drivers/net/wireless/ath/ath10k/pci.c
> index a0b4d265c6eb..347bb92e4130 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -3490,7 +3490,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
>  	struct ath10k_pci *ar_pci;
>  	enum ath10k_hw_rev hw_rev;
>  	struct ath10k_bus_params bus_params = {};
> -	bool pci_ps;
> +	bool pci_ps, is_qca988x = false;
>  	int (*pci_soft_reset)(struct ath10k *ar);
>  	int (*pci_hard_reset)(struct ath10k *ar);
>  	u32 (*targ_cpu_to_ce_addr)(struct ath10k *ar, u32 addr);
> @@ -3500,6 +3500,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
>  	case QCA988X_2_0_DEVICE_ID:
>  		hw_rev = ATH10K_HW_QCA988X;
>  		pci_ps = false;
> +		is_qca988x = true;
>  		pci_soft_reset = ath10k_pci_warm_reset;
>  		pci_hard_reset = ath10k_pci_qca988x_chip_reset;
>  		targ_cpu_to_ce_addr = ath10k_pci_qca988x_targ_cpu_to_ce_addr;
> @@ -3619,25 +3620,34 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
>  		goto err_deinit_irq;
>  	}
>
> +	bus_params.dev_type = ATH10K_DEV_TYPE_LL;
> +	bus_params.link_can_suspend = true;
> +	/* Read CHIP_ID before reset to catch QCA9880-AR1A v1 devices that
> +	 * fall off the bus during chip_reset. These chips have the same pci
> +	 * device id as the QCA9880 BR4A or 2R4E. So that's why the check.
> +	 */
> +	if (is_qca988x) {
> +		bus_params.chip_id =
> +			ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS);
> +		if (bus_params.chip_id != 0xffffffff) {
> +			if (!ath10k_pci_chip_is_supported(pdev->device,
> +							  bus_params.chip_id))
> +				goto err_unsupported;
> +		}
> +	}
> +
>  	ret = ath10k_pci_chip_reset(ar);
>  	if (ret) {
>  		ath10k_err(ar, "failed to reset chip: %d\n", ret);
>  		goto err_free_irq;
>  	}
>
> -	bus_params.dev_type = ATH10K_DEV_TYPE_LL;
> -	bus_params.link_can_suspend = true;
>  	bus_params.chip_id = ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS);
> -	if (bus_params.chip_id == 0xffffffff) {
> -		ath10k_err(ar, "failed to get chip id\n");
> -		goto err_free_irq;
> -	}
> +	if (bus_params.chip_id == 0xffffffff)
> +		goto err_unsupported;
>
> -	if (!ath10k_pci_chip_is_supported(pdev->device, bus_params.chip_id)) {
> -		ath10k_err(ar, "device %04x with chip_id %08x isn't supported\n",
> -			   pdev->device, bus_params.chip_id);
> +	if (!ath10k_pci_chip_is_supported(pdev->device, bus_params.chip_id))
>  		goto err_free_irq;
> -	}
>
>  	ret = ath10k_core_register(ar, &bus_params);
>  	if (ret) {
> @@ -3647,6 +3657,10 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
>
>  	return 0;
>
> +err_unsupported:
> +	ath10k_err(ar, "device %04x with chip_id %08x isn't supported\n",
> +		   pdev->device, bus_params.chip_id);
> +
>  err_free_irq:
>  	ath10k_pci_free_irq(ar);
>  	ath10k_pci_rx_retry_sync(ar);
> --
> 2.23.0
>
>

Looks fine. For the time being. Have you looked any further to
actually support this chip? It seems warm reset is causing bus errors,
and cold reset goes through without crash.
Firmware gets loaded but is stuck at receiving control response, most
likely because of htc packet length or response message length.

  parent reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-06 21:54 Christian Lamparter
2019-09-07 21:43 ` Sasha Levin
2019-09-07 23:18   ` stable backports for "ath10k: restore QCA9880-AR1A (v1) detection" Christian Lamparter
2019-09-10  1:27   ` [PATCH] ath10k: restore QCA9880-AR1A (v1) detection Tom Psyborg
2019-09-10  7:21     ` Kalle Valo
2019-09-10 12:59       ` Tom Psyborg
2019-09-08  6:32 ` Tom Psyborg [this message]
2019-09-17  6:44 ` Kalle Valo
2019-09-18 21:30   ` Tom Psyborg
     [not found] ` <20190917064412.C2E0D61572@smtp.codeaurora.org>
2019-09-20 17:19   ` Christian Lamparter
2019-10-01 13:03     ` Kalle Valo
2019-10-01 14:17       ` Tom Psyborg
2019-10-02 17:19 ` Kalle Valo

Reply instructions:

You may reply publically 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='CAKR_QV+y9u_gP_+dZ=bFYJgANeq+W19v9Xx_SwydB5fe4ozhtg@mail.gmail.com' \
    --to=pozega.tomislav@gmail.com \
    --cc=ath10k@lists.infradead.org \
    --cc=chunkeey@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox