Arend van Spriel <arend.vanspr...@broadcom.com> writes:

> On 7/30/2018 4:06 PM, Kalle Valo wrote:
>> Sergey Matyukevich <sergey.matyukevich...@quantenna.com> writes:
>>
>>> From: Andrey Shevchenko <ashevche...@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.

That's a very good point.

-- 
Kalle Valo

Reply via email to