| Message ID | 1479141222-8493-11-git-send-email-erik.stromdahl@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromdahl@gmail.com> wrote: > Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com> > --- > drivers/net/wireless/ath/ath10k/hw.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h > index 46142e9..ef45ecf 100644 > --- a/drivers/net/wireless/ath/ath10k/hw.h > +++ b/drivers/net/wireless/ath/ath10k/hw.h > @@ -224,6 +224,7 @@ enum ath10k_hw_rev { > ATH10K_HW_QCA9377, > ATH10K_HW_QCA4019, > ATH10K_HW_QCA9887, > + ATH10K_HW_QCA65XX, Are you 100% positive that you're supporting all QCA65XX chips? The rule of thumb is to avoid Xs as much as possible unless totally perfectly completely sure. Michał
Michal Kazior <michal.kazior@tieto.com> writes: > On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromdahl@gmail.com> wrote: >> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com> >> --- >> drivers/net/wireless/ath/ath10k/hw.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h >> index 46142e9..ef45ecf 100644 >> --- a/drivers/net/wireless/ath/ath10k/hw.h >> +++ b/drivers/net/wireless/ath/ath10k/hw.h >> @@ -224,6 +224,7 @@ enum ath10k_hw_rev { >> ATH10K_HW_QCA9377, >> ATH10K_HW_QCA4019, >> ATH10K_HW_QCA9887, >> + ATH10K_HW_QCA65XX, > > Are you 100% positive that you're supporting all QCA65XX chips? The > rule of thumb is to avoid Xs as much as possible unless totally > perfectly completely sure. But the thing is that nobody can't be absolutely sure as we never know what marketing comes up within few years again. So I would say that XX in chip names should be completely avoided to avoid any confusion. We already suffer from this in ath10k with QCA988X and QCA9888 which are different designs but the names overlap. I haven't reviewed Erik's patches yet but I'm surprised why even a new enum value is needed here. I was assuming we could use ATH10K_HW_QCA6174 because AFAIK they share the same design. Any particular reason for this?
On 11/15/2016 11:54 AM, Valo, Kalle wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromdahl@gmail.com> wrote: >>> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com> >>> --- >>> drivers/net/wireless/ath/ath10k/hw.h | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h >>> index 46142e9..ef45ecf 100644 >>> --- a/drivers/net/wireless/ath/ath10k/hw.h >>> +++ b/drivers/net/wireless/ath/ath10k/hw.h >>> @@ -224,6 +224,7 @@ enum ath10k_hw_rev { >>> ATH10K_HW_QCA9377, >>> ATH10K_HW_QCA4019, >>> ATH10K_HW_QCA9887, >>> + ATH10K_HW_QCA65XX, >> >> Are you 100% positive that you're supporting all QCA65XX chips? The >> rule of thumb is to avoid Xs as much as possible unless totally >> perfectly completely sure. > I admit that I am definitely not totally perfectly completely sure about this :) In fact, I don't have a clue what does numbers really mean. All I know is that the chipset I am using is called QCA6584 which I think is an automotive version of another chipses called QCA6574. I have tried to google these numbers up, but it is really hard to find anything useful. So I thought, hey, why don't I just add a few X'es in there and I have support for both! > But the thing is that nobody can't be absolutely sure as we never know > what marketing comes up within few years again. So I would say that XX > in chip names should be completely avoided to avoid any confusion. We > already suffer from this in ath10k with QCA988X and QCA9888 which are > different designs but the names overlap. > > I haven't reviewed Erik's patches yet but I'm surprised why even a new > enum value is needed here. I was assuming we could use ATH10K_HW_QCA6174 > because AFAIK they share the same design. Any particular reason for > this? > The reason for this was that the switch(hw_rev)-statement in ath10k_core_create assigns ar->regs and ar->hw_values to structures containing values that are not applicable for SDIO. I though that I would potentially need other structures, but after investigating the qca6174_regs struct, it seems that the values that are applicable for SDIO are the same. I will remove this enum and use ATH10K_HW_QCA6174 instead as you propose. If for some reason I would need other regs and hw_values structs in the future, we can figure out appropriate names then.
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h index 46142e9..ef45ecf 100644 --- a/drivers/net/wireless/ath/ath10k/hw.h +++ b/drivers/net/wireless/ath/ath10k/hw.h @@ -224,6 +224,7 @@ enum ath10k_hw_rev { ATH10K_HW_QCA9377, ATH10K_HW_QCA4019, ATH10K_HW_QCA9887, + ATH10K_HW_QCA65XX, }; struct ath10k_hw_regs {
Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com> --- drivers/net/wireless/ath/ath10k/hw.h | 1 + 1 file changed, 1 insertion(+)