| Message ID | 20181013005504.46399-1-briannorris@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] ath10k: snoc: remove 'wcn3990' from generic resource handling | expand |
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>
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
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
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
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
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; };
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(-)