From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from sabertooth01.qualcomm.com ([65.197.215.72]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VWTSM-0004jK-Lu for ath10k@lists.infradead.org; Wed, 16 Oct 2013 15:56:19 +0000 From: Kalle Valo Subject: Re: [PATCH v2 6/8] ath10k: implement ath10k_pci_soc_read/write32() References: <20131016134503.25095.8044.stgit@localhost6.localdomain6> <20131016134611.25095.80204.stgit@localhost6.localdomain6> Date: Wed, 16 Oct 2013 18:55:50 +0300 In-Reply-To: (Michal Kazior's message of "Wed, 16 Oct 2013 08:38:43 -0700") Message-ID: <87txgh5hx5.fsf@kamboji.qca.qualcomm.com> MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Michal Kazior Cc: linux-wireless , ath10k@lists.infradead.org Michal Kazior writes: > On 16 October 2013 06:46, Kalle Valo wrote: >> To make it easier to access SOC registers. No functional >> changes. >> >> Signed-off-by: Kalle Valo >> --- >> drivers/net/wireless/ath/ath10k/pci.c | 3 +-- >> drivers/net/wireless/ath/ath10k/pci.h | 10 ++++++++++ >> 2 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c >> index d09f8a2..5c78383 100644 >> --- a/drivers/net/wireless/ath/ath10k/pci.c >> +++ b/drivers/net/wireless/ath/ath10k/pci.c >> @@ -2454,8 +2454,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev, >> return ret; >> } >> >> - chip_id = ath10k_pci_read32(ar, >> - RTC_SOC_BASE_ADDRESS + SOC_CHIP_ID_ADDRESS); >> + chip_id = ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS); >> >> ath10k_do_pci_sleep(ar); >> >> diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h >> index 52fb7b9..a304c33 100644 >> --- a/drivers/net/wireless/ath/ath10k/pci.h >> +++ b/drivers/net/wireless/ath/ath10k/pci.h >> @@ -318,6 +318,16 @@ static inline u32 ath10k_pci_read32(struct ath10k *ar, u32 offset) >> return ioread32(ar_pci->mem + offset); >> } >> >> +static inline u32 ath10k_pci_soc_read32(struct ath10k *ar, u32 addr) >> +{ >> + return ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS + addr); >> +} >> + >> +static inline void ath10k_pci_soc_write32(struct ath10k *ar, u32 addr, u32 val) >> +{ >> + ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + addr, val); >> +} >> + > > I'm not entirely sure about this. There are a couple of soc address > groups (RTC_SOC, RTC_WMAC, SOC_PCIE, SOC_CORE, ..). And then we can have different functions for each group. > I'd rather use just raw ioread/iowrite than have all those wrappers. Well, I again would prefer to have clean interfaces instead of doing address arithmetic in every call :) > I think the reason for having ath10k_pci_{read,write}32 was the HW v1 > workaround. No, it was also to make the code simple. > Do we really need to keep it? In my opinion yes. What is the negative side of keeping them? Why don't you like these wrappers? -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k