Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9F3F0C433CA
From:   Kalle Valo <kvalo@codeaurora.org>
To:     John Crispin <john@phrozen.org>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless@vger.kernel.org, ath11k@lists.infradead.org
Subject: Re: [PATCH 3/3] ath11k: add support for BSS coloring
References: <20200617150021.4183253-1-john@phrozen.org>
        <20200617150021.4183253-3-john@phrozen.org>
Date:   Mon, 22 Jun 2020 17:45:31 +0300
In-Reply-To: <20200617150021.4183253-3-john@phrozen.org> (John Crispin's
        message of "Wed, 17 Jun 2020 17:00:21 +0200")
Message-ID: <87h7v3f9bo.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

John Crispin <john@phrozen.org> writes:

> Whenever the MAC detects a color collision or any of its associated station
> detects one the FW will send out an event. Add the code to parse and handle
> this event. and pass the data up to mac80211.
>
> The FW does not provide an offload future such as the one used for CSA. The
> CCA process is hence triggered via the beacon offload tx completion events
> sent out by the FW.
>
> Signed-off-by: John Crispin <john@phrozen.org>

On what hardware and firmware did you test this? I'll add that to the
commit log.

> +	if (ev->obss_color_bitmap & BIT(arvif->vif->bss_conf.he_bss_color.color)) {
> +		/* FW sometime sends out stray events. Ignore them if our color is free */
> +		ieeee80211_obss_color_collision_notify(arvif->vif, ev->obss_color_bitmap);
> +
> +		ath11k_info(ab, "OBSS color collision detected vdev:%d, event:%d, bitmap:%08llx\n",
> +			    ev->vdev_id, ev->evt_type, ev->obss_color_bitmap);
> +	}

Is this really something so important that we should print it to the
log? What should the user do after seeing this warning?

And warnings should be printed with ath11k_warn(), but maybe this should
be a debug message?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches