[1/4] ath10k: snoc: remove 'wcn3990' from generic resource handling
diff mbox series

Message ID 20181013005504.46399-1-briannorris@chromium.org
State New
Headers show
Series
  • [1/4] ath10k: snoc: remove 'wcn3990' from generic resource handling
Related show

Commit Message

Brian Norris Oct. 13, 2018, 12:55 a.m. UTC
ath10k_wcn3990_clk_info and ath10k_wcn3990_vreg_info are not
WCN3990-specific structures. They hold generic data. So don't name them
with wcn3990 specifics.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/net/wireless/ath/ath10k/snoc.c | 34 +++++++++++++-------------
 drivers/net/wireless/ath/ath10k/snoc.h |  8 +++---
 2 files changed, 21 insertions(+), 21 deletions(-)

Comments

Douglas Anderson Oct. 16, 2018, 11:43 p.m. UTC | #1
Hi,

On Fri, Oct 12, 2018 at 5:55 PM Brian Norris <briannorris@chromium.org> wrote:
>
> ath10k_wcn3990_clk_info and ath10k_wcn3990_vreg_info are not
> WCN3990-specific structures. They hold generic data. So don't name them
> with wcn3990 specifics.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  drivers/net/wireless/ath/ath10k/snoc.c | 34 +++++++++++++-------------
>  drivers/net/wireless/ath/ath10k/snoc.h |  8 +++---
>  2 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
> index 8d3d9bca410f..c6254db17dab 100644
> --- a/drivers/net/wireless/ath/ath10k/snoc.c
> +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> @@ -46,14 +46,14 @@ static char *const ce_name[] = {
>         "WLAN_CE_11",
>  };
>
> -static struct ath10k_wcn3990_vreg_info vreg_cfg[] = {
> +static struct ath10k_vreg_info vreg_cfg[] = {

Ironically, you could sorta make the argument that this should be:

static struct ath10k_vreg_info wcn3990_vreg_cfg

AKA the "wcn3990" shouldn't be in the name of the structure (since all
snoc devices can have the concept of an array of regulators) but
wcn3990 could be in the name of the variable since it's possible that
different snoc devices could have different arrays.  However I'm OK w/
waiting to do that part until we actually see a different snoc device
with a different array.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Brian Norris Oct. 16, 2018, 11:47 p.m. UTC | #2
On Tue, Oct 16, 2018 at 4:43 PM Doug Anderson <dianders@chromium.org> wrote:
> On Fri, Oct 12, 2018 at 5:55 PM Brian Norris <briannorris@chromium.org> wrote:
> >
> > ath10k_wcn3990_clk_info and ath10k_wcn3990_vreg_info are not
> > WCN3990-specific structures. They hold generic data. So don't name them
> > with wcn3990 specifics.
> >

> > -static struct ath10k_wcn3990_vreg_info vreg_cfg[] = {
> > +static struct ath10k_vreg_info vreg_cfg[] = {
>
> Ironically, you could sorta make the argument that this should be:
>
> static struct ath10k_vreg_info wcn3990_vreg_cfg

Hehe, good point.

> AKA the "wcn3990" shouldn't be in the name of the structure (since all
> snoc devices can have the concept of an array of regulators) but
> wcn3990 could be in the name of the variable since it's possible that
> different snoc devices could have different arrays.  However I'm OK w/
> waiting to do that part until we actually see a different snoc device
> with a different array.

But I think that is a pretty reasonable conclusion.

There's still some other strange stuff in this driver too, like the
fact that this is NOT a const array -- we assign things dynamically to
the regulator fields within the array -- that we would probably want
to fix if this is really supposed to be a generic multi-IP driver.

> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks,
Brian
Kalle Valo Nov. 5, 2018, 1:04 p.m. UTC | #3
Brian Norris <briannorris@chromium.org> wrote:

> ath10k_wcn3990_clk_info and ath10k_wcn3990_vreg_info are not
> WCN3990-specific structures. They hold generic data. So don't name them
> with wcn3990 specifics.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

This patchset (I think it was with the first patch, but not sure) had some
conflicts but 3-way merge was able to automatically solve them. Please check
the end result anyway:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=77adda81e264e0c93af9570ac4ada82a5d0f5d99
Brian Norris Nov. 5, 2018, 9:17 p.m. UTC | #4
On Mon, Nov 5, 2018 at 5:04 AM Kalle Valo <kvalo@codeaurora.org> wrote:
> This patchset (I think it was with the first patch, but not sure) had some
> conflicts but 3-way merge was able to automatically solve them. Please check
> the end result anyway:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=77adda81e264e0c93af9570ac4ada82a5d0f5d99

I see the same. There were some unrelated changes to this file that
you merged to your pending tree in the meantime. The end result looks
fine to me. Thanks for the heads up.

Brian
Kalle Valo Nov. 6, 2018, 4:18 p.m. UTC | #5
Brian Norris <briannorris@chromium.org> wrote:

> ath10k_wcn3990_clk_info and ath10k_wcn3990_vreg_info are not
> WCN3990-specific structures. They hold generic data. So don't name them
> with wcn3990 specifics.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

4 patches applied to ath-next branch of ath.git, thanks.

887a3dcf5893 ath10k: snoc: remove 'wcn3990' from generic resource handling
1a1a0d5ccefc ath10k: snoc: fix unabalanced regulator error handling
bfe57a6ac75a ath10k: snoc: relax voltage requirements
82e60d920e8a ath10k: snoc: fix unbalanced clock error handling

Patch
diff mbox series

diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 8d3d9bca410f..c6254db17dab 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -46,14 +46,14 @@  static char *const ce_name[] = {
 	"WLAN_CE_11",
 };
 
-static struct ath10k_wcn3990_vreg_info vreg_cfg[] = {
+static struct ath10k_vreg_info vreg_cfg[] = {
 	{NULL, "vdd-0.8-cx-mx", 800000, 800000, 0, 0, false},
 	{NULL, "vdd-1.8-xo", 1800000, 1800000, 0, 0, false},
 	{NULL, "vdd-1.3-rfa", 1304000, 1304000, 0, 0, false},
 	{NULL, "vdd-3.3-ch0", 3312000, 3312000, 0, 0, false},
 };
 
-static struct ath10k_wcn3990_clk_info clk_cfg[] = {
+static struct ath10k_clk_info clk_cfg[] = {
 	{NULL, "cxo_ref_clk_pin", 0, false},
 };
 
@@ -1246,7 +1246,7 @@  static void ath10k_snoc_release_resource(struct ath10k *ar)
 }
 
 static int ath10k_get_vreg_info(struct ath10k *ar, struct device *dev,
-				struct ath10k_wcn3990_vreg_info *vreg_info)
+				struct ath10k_vreg_info *vreg_info)
 {
 	struct regulator *reg;
 	int ret = 0;
@@ -1284,7 +1284,7 @@  static int ath10k_get_vreg_info(struct ath10k *ar, struct device *dev,
 }
 
 static int ath10k_get_clk_info(struct ath10k *ar, struct device *dev,
-			       struct ath10k_wcn3990_clk_info *clk_info)
+			       struct ath10k_clk_info *clk_info)
 {
 	struct clk *handle;
 	int ret = 0;
@@ -1311,10 +1311,10 @@  static int ath10k_get_clk_info(struct ath10k *ar, struct device *dev,
 	return ret;
 }
 
-static int ath10k_wcn3990_vreg_on(struct ath10k *ar)
+static int ath10k_snoc_vreg_on(struct ath10k *ar)
 {
 	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
-	struct ath10k_wcn3990_vreg_info *vreg_info;
+	struct ath10k_vreg_info *vreg_info;
 	int ret = 0;
 	int i;
 
@@ -1376,10 +1376,10 @@  static int ath10k_wcn3990_vreg_on(struct ath10k *ar)
 	return ret;
 }
 
-static int ath10k_wcn3990_vreg_off(struct ath10k *ar)
+static int ath10k_snoc_vreg_off(struct ath10k *ar)
 {
 	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
-	struct ath10k_wcn3990_vreg_info *vreg_info;
+	struct ath10k_vreg_info *vreg_info;
 	int ret = 0;
 	int i;
 
@@ -1412,10 +1412,10 @@  static int ath10k_wcn3990_vreg_off(struct ath10k *ar)
 	return ret;
 }
 
-static int ath10k_wcn3990_clk_init(struct ath10k *ar)
+static int ath10k_snoc_clk_init(struct ath10k *ar)
 {
 	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
-	struct ath10k_wcn3990_clk_info *clk_info;
+	struct ath10k_clk_info *clk_info;
 	int ret = 0;
 	int i;
 
@@ -1461,10 +1461,10 @@  static int ath10k_wcn3990_clk_init(struct ath10k *ar)
 	return ret;
 }
 
-static int ath10k_wcn3990_clk_deinit(struct ath10k *ar)
+static int ath10k_snoc_clk_deinit(struct ath10k *ar)
 {
 	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
-	struct ath10k_wcn3990_clk_info *clk_info;
+	struct ath10k_clk_info *clk_info;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(clk_cfg); i++) {
@@ -1488,18 +1488,18 @@  static int ath10k_hw_power_on(struct ath10k *ar)
 
 	ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
 
-	ret = ath10k_wcn3990_vreg_on(ar);
+	ret = ath10k_snoc_vreg_on(ar);
 	if (ret)
 		return ret;
 
-	ret = ath10k_wcn3990_clk_init(ar);
+	ret = ath10k_snoc_clk_init(ar);
 	if (ret)
 		goto vreg_off;
 
 	return ret;
 
 vreg_off:
-	ath10k_wcn3990_vreg_off(ar);
+	ath10k_snoc_vreg_off(ar);
 	return ret;
 }
 
@@ -1509,9 +1509,9 @@  static int ath10k_hw_power_off(struct ath10k *ar)
 
 	ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
 
-	ath10k_wcn3990_clk_deinit(ar);
+	ath10k_snoc_clk_deinit(ar);
 
-	ret = ath10k_wcn3990_vreg_off(ar);
+	ret = ath10k_snoc_vreg_off(ar);
 
 	return ret;
 }
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index e1d2d6675556..4a3837b5c68d 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -53,7 +53,7 @@  struct ath10k_snoc_ce_irq {
 	u32 irq_line;
 };
 
-struct ath10k_wcn3990_vreg_info {
+struct ath10k_vreg_info {
 	struct regulator *reg;
 	const char *name;
 	u32 min_v;
@@ -63,7 +63,7 @@  struct ath10k_wcn3990_vreg_info {
 	bool required;
 };
 
-struct ath10k_wcn3990_clk_info {
+struct ath10k_clk_info {
 	struct clk *handle;
 	const char *name;
 	u32 freq;
@@ -81,8 +81,8 @@  struct ath10k_snoc {
 	struct ath10k_snoc_ce_irq ce_irqs[CE_COUNT_MAX];
 	struct ath10k_ce ce;
 	struct timer_list rx_post_retry;
-	struct ath10k_wcn3990_vreg_info *vreg;
-	struct ath10k_wcn3990_clk_info *clk;
+	struct ath10k_vreg_info *vreg;
+	struct ath10k_clk_info *clk;
 	struct ath10k_qmi *qmi;
 };