Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

[Arend van Spriel <arend.vanspriel@broadcom.com>]

On 7/30/2018 4:06 PM, Kalle Valo wrote:
> Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> writes:
>
>> From: Andrey Shevchenko <ashevchenko@quantenna.com>
>>
>> Implement support for PTA (Packet Traffic Arbitration) configuration.
>> The PTA mechanism is used to coordinate sharing of the medium between
>> WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
>>
>> This patch implements core infrastructure and vendor specific commands
>> to control PTA functionality in firmware.
>
> And no description of the actual interface which would have helped with
> the review.
>
> Anyway, the vendor commands are pain and they just make me grumpy. The
> original idea was that upstream drivers should not support them at all,
> later we flexed the rules so that low level hardware specific interfaces
> might be ok, for example we added one in wil6210.
>
> If I would even consider applying a patch which adds a vendor command it
> needs a really good commit log with a proper description of the actual
> interface and good justifications why a generic nl80211 command won't
> work. I don't see anything even remotely close here.
>
> Sorry for being grumpy, I just hate these vendor commands. Especially
> when I see that a generic nl80211 command has not even be consired at
> all.

For what it is worth, looking at part of the patch:

+/**
+ * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
+ *
+ * @QLINK_PTA_MODE_DISABLED: PTA is disabled
+ * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
+ */
+enum qlink_pta_mode {
+	QLINK_PTA_MODE_DISABLED = 0,
+	QLINK_PTA_MODE_2_WIRE = 2
+};
+

it smells very much like low-level btcoex. The question is whether this 
needs to be conveyed from user-space or should these be device 
configuration, eg. like DT properties.

Regards,
Arend