Hi Alagu,
On 2017-10-02 09:02, Alagu Sankar wrote:
> Hi Steve,
>
> On 2017-10-02 04:17, Steve deRosier wrote:
>> Hi Alagu,
>>
>>
>> On Sat, Sep 30, 2017 at 10:37 AM, <silexcommon at gmail.com> wrote:
>>>
>>> From: Alagu Sankar <alagusankar at silex-india.com>
>>>
>>> The QCA9377-3 WB396 sdio reference card does not get initialized
>>> due to the conflict in uart gpio pins. This fix is not required
>>> for other QCA9377 sdio cards.
>>>
>>> Signed-off-by: Alagu Sankar <alagusankar at silex-india.com>
>>> ---
>>> drivers/net/wireless/ath/ath10k/core.c | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
>>> index b4f66cd..86247c8 100644
>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>> @@ -1708,8 +1708,15 @@ static int ath10k_init_uart(struct ath10k *ar)
>>> return ret;
>>> }
>>>
>>> - if (!uart_print)
>>> + if (!uart_print) {
>>> + /* Hack: override dbg TX pin to avoid side effects of default
>>> + * GPIO_6 in QCA9377 WB396 reference card
>>> + */
>>> + if (ar->hif.bus == ATH10K_BUS_SDIO)
>>> + ath10k_bmi_write32(ar, hi_dbg_uart_txpin,
>>> + ar->hw_params.uart_pin);
>>
>> If it is indeed a "hack", then I don't think the maintainer should
>> accept this upstream. If you want it upstream you need a clean enough
>> implementation that doesn't need to be labeled a "hack".
>
> It is a hack as per the qcacld reference driver.
>
>> Your commit message states that this is only needed for a very
>> specific card and not for other QCA9377 sdio cards. Yet, you're doing
>> this for all ATH10K_BUS_SDIO devices. Not good. I think that it's a
>> quirk and it's limited to a particular implementation of the device.
>> My suggestion: if it can be automatically determined, then do so
>> explicitly. If not, then it needs to be a DT setting or a module
>> parameter or something like that so the platform maker can decide to
>> do it. Having it affect all users of a SDIO QCA9377 when it doesn't
>> apply doesn't seem like a good idea to me.
>>
>>
>> - Steve
>
> Got it. The qcacld reference driver had it for all the QCA9377 sdio cards.
> But we found it to be a problem only for the WB396 reference card. Will
> have this checked again and release a v2 patch accordingly.
>
While you are at it, you might as well change the commit comments to:
"ath10k: sdio: <description>"
or perhaps just:
"ath10k: <description>"
> Best Regards,
> Alagu Sankar
>
> _______________________________________________
> ath10k mailing list
> ath10k at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k