diff mbox series

[v3,net-next,4/5] net: ipa: introduce ipa_table_hash_support()

Message ID 20210212143402.2691-5-elder@linaro.org
State New
Headers show
Series net: ipa: some more cleanup | expand

Commit Message

Alex Elder Feb. 12, 2021, 2:34 p.m. UTC 
Introduce a new function to abstract the knowledge of whether hashed
routing and filter tables are supported for a given IPA instance.

IPA v4.2 is the only one that doesn't support hashed tables (now
and for the foreseeable future), but the name of the helper function
is better for explaining what's going on.

Signed-off-by: Alex Elder <elder@linaro.org>

---
v2: - Update copyrights.

 drivers/net/ipa/ipa_cmd.c   |  2 +-
 drivers/net/ipa/ipa_table.c | 16 +++++++++-------
 drivers/net/ipa/ipa_table.h |  8 +++++++-
 3 files changed, 17 insertions(+), 9 deletions(-)

-- 
2.20.1

Comments

Alexander Duyck Feb. 13, 2021, 1:05 a.m. UTC | #1 
On Fri, Feb 12, 2021 at 6:40 AM Alex Elder <elder@linaro.org> wrote:
>

> Introduce a new function to abstract the knowledge of whether hashed

> routing and filter tables are supported for a given IPA instance.

>

> IPA v4.2 is the only one that doesn't support hashed tables (now

> and for the foreseeable future), but the name of the helper function

> is better for explaining what's going on.

>

> Signed-off-by: Alex Elder <elder@linaro.org>

> ---

> v2: - Update copyrights.

>

>  drivers/net/ipa/ipa_cmd.c   |  2 +-

>  drivers/net/ipa/ipa_table.c | 16 +++++++++-------

>  drivers/net/ipa/ipa_table.h |  8 +++++++-

>  3 files changed, 17 insertions(+), 9 deletions(-)

>

> diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c

> index fd8bf6468d313..35e35852c25c5 100644

> --- a/drivers/net/ipa/ipa_cmd.c

> +++ b/drivers/net/ipa/ipa_cmd.c

> @@ -268,7 +268,7 @@ static bool ipa_cmd_register_write_valid(struct ipa *ipa)

>         /* If hashed tables are supported, ensure the hash flush register

>          * offset will fit in a register write IPA immediate command.

>          */

> -       if (ipa->version != IPA_VERSION_4_2) {

> +       if (ipa_table_hash_support(ipa)) {

>                 offset = ipa_reg_filt_rout_hash_flush_offset(ipa->version);

>                 name = "filter/route hash flush";

>                 if (!ipa_cmd_register_write_offset_valid(ipa, name, offset))

> diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c

> index 32e2d3e052d55..baaab3dd0e63c 100644

> --- a/drivers/net/ipa/ipa_table.c

> +++ b/drivers/net/ipa/ipa_table.c

> @@ -1,7 +1,7 @@

>  // SPDX-License-Identifier: GPL-2.0

>

>  /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.

> - * Copyright (C) 2018-2020 Linaro Ltd.

> + * Copyright (C) 2018-2021 Linaro Ltd.

>   */

>

>  #include <linux/types.h>

> @@ -239,6 +239,11 @@ static void ipa_table_validate_build(void)

>

>  #endif /* !IPA_VALIDATE */

>

> +bool ipa_table_hash_support(struct ipa *ipa)

> +{

> +       return ipa->version != IPA_VERSION_4_2;

> +}

> +


Since this is only a single comparison it might make more sense to
make this a static inline and place it in ipa.h. Otherwise you are
just bloating the code up to jump to such a small function.

>  /* Zero entry count means no table, so just return a 0 address */

>  static dma_addr_t ipa_table_addr(struct ipa *ipa, bool filter_mask, u16 count)

>  {

> @@ -412,8 +417,7 @@ int ipa_table_hash_flush(struct ipa *ipa)

>         struct gsi_trans *trans;

>         u32 val;

>

> -       /* IPA version 4.2 does not support hashed tables */

> -       if (ipa->version == IPA_VERSION_4_2)

> +       if (!ipa_table_hash_support(ipa))

>                 return 0;

>

>         trans = ipa_cmd_trans_alloc(ipa, 1);

> @@ -531,8 +535,7 @@ static void ipa_filter_config(struct ipa *ipa, bool modem)

>         enum gsi_ee_id ee_id = modem ? GSI_EE_MODEM : GSI_EE_AP;

>         u32 ep_mask = ipa->filter_map;

>

> -       /* IPA version 4.2 has no hashed route tables */

> -       if (ipa->version == IPA_VERSION_4_2)

> +       if (!ipa_table_hash_support(ipa))

>                 return;

>

>         while (ep_mask) {

> @@ -582,8 +585,7 @@ static void ipa_route_config(struct ipa *ipa, bool modem)

>  {

>         u32 route_id;

>

> -       /* IPA version 4.2 has no hashed route tables */

> -       if (ipa->version == IPA_VERSION_4_2)

> +       if (!ipa_table_hash_support(ipa))

>                 return;

>

>         for (route_id = 0; route_id < IPA_ROUTE_COUNT_MAX; route_id++)

> diff --git a/drivers/net/ipa/ipa_table.h b/drivers/net/ipa/ipa_table.h

> index 78038d14fcea9..1a68d20f19d6a 100644

> --- a/drivers/net/ipa/ipa_table.h

> +++ b/drivers/net/ipa/ipa_table.h

> @@ -1,7 +1,7 @@

>  /* SPDX-License-Identifier: GPL-2.0 */

>

>  /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.

> - * Copyright (C) 2019-2020 Linaro Ltd.

> + * Copyright (C) 2019-2021 Linaro Ltd.

>   */

>  #ifndef _IPA_TABLE_H_

>  #define _IPA_TABLE_H_

> @@ -51,6 +51,12 @@ static inline bool ipa_filter_map_valid(struct ipa *ipa, u32 filter_mask)

>

>  #endif /* !IPA_VALIDATE */

>

> +/**

> + * ipa_table_hash_support() - Return true if hashed tables are supported

> + * @ipa:       IPA pointer

> + */

> +bool ipa_table_hash_support(struct ipa *ipa);

> +

>  /**

>   * ipa_table_reset() - Reset filter and route tables entries to "none"

>   * @ipa:       IPA pointer


Just define the function here and make it a static inline.
Alex Elder Feb. 13, 2021, 1:35 a.m. UTC | #2 
On 2/12/21 7:05 PM, Alexander Duyck wrote:
> On Fri, Feb 12, 2021 at 6:40 AM Alex Elder <elder@linaro.org> wrote:

>>

>> Introduce a new function to abstract the knowledge of whether hashed

>> routing and filter tables are supported for a given IPA instance.

>>

>> IPA v4.2 is the only one that doesn't support hashed tables (now

>> and for the foreseeable future), but the name of the helper function

>> is better for explaining what's going on.

>>

>> Signed-off-by: Alex Elder <elder@linaro.org>

>> ---

>> v2: - Update copyrights.

>>

>>   drivers/net/ipa/ipa_cmd.c   |  2 +-

>>   drivers/net/ipa/ipa_table.c | 16 +++++++++-------

>>   drivers/net/ipa/ipa_table.h |  8 +++++++-

>>   3 files changed, 17 insertions(+), 9 deletions(-)

>>

>> diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c

>> index fd8bf6468d313..35e35852c25c5 100644

>> --- a/drivers/net/ipa/ipa_cmd.c

>> +++ b/drivers/net/ipa/ipa_cmd.c

>> @@ -268,7 +268,7 @@ static bool ipa_cmd_register_write_valid(struct ipa *ipa)

>>          /* If hashed tables are supported, ensure the hash flush register

>>           * offset will fit in a register write IPA immediate command.

>>           */

>> -       if (ipa->version != IPA_VERSION_4_2) {

>> +       if (ipa_table_hash_support(ipa)) {

>>                  offset = ipa_reg_filt_rout_hash_flush_offset(ipa->version);

>>                  name = "filter/route hash flush";

>>                  if (!ipa_cmd_register_write_offset_valid(ipa, name, offset))

>> diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c

>> index 32e2d3e052d55..baaab3dd0e63c 100644

>> --- a/drivers/net/ipa/ipa_table.c

>> +++ b/drivers/net/ipa/ipa_table.c

>> @@ -1,7 +1,7 @@

>>   // SPDX-License-Identifier: GPL-2.0

>>

>>   /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.

>> - * Copyright (C) 2018-2020 Linaro Ltd.

>> + * Copyright (C) 2018-2021 Linaro Ltd.

>>    */

>>

>>   #include <linux/types.h>

>> @@ -239,6 +239,11 @@ static void ipa_table_validate_build(void)

>>

>>   #endif /* !IPA_VALIDATE */

>>

>> +bool ipa_table_hash_support(struct ipa *ipa)

>> +{

>> +       return ipa->version != IPA_VERSION_4_2;

>> +}

>> +

> 

> Since this is only a single comparison it might make more sense to

> make this a static inline and place it in ipa.h. Otherwise you are

> just bloating the code up to jump to such a small function.


Static inline will duplicate the function everywhere also,
"bloating" it in another way (but it's a fairly trivial
amount of code either way).

Nevertheless I agree with your sentiment.  It's moot now
because the series was just accepted, but I'll keep it
in mind.

Thanks for your input.

					-Alex

>>   /* Zero entry count means no table, so just return a 0 address */

>>   static dma_addr_t ipa_table_addr(struct ipa *ipa, bool filter_mask, u16 count)

>>   {

>> @@ -412,8 +417,7 @@ int ipa_table_hash_flush(struct ipa *ipa)

>>          struct gsi_trans *trans;

>>          u32 val;

>>

>> -       /* IPA version 4.2 does not support hashed tables */

>> -       if (ipa->version == IPA_VERSION_4_2)

>> +       if (!ipa_table_hash_support(ipa))

>>                  return 0;

>>

>>          trans = ipa_cmd_trans_alloc(ipa, 1);

>> @@ -531,8 +535,7 @@ static void ipa_filter_config(struct ipa *ipa, bool modem)

>>          enum gsi_ee_id ee_id = modem ? GSI_EE_MODEM : GSI_EE_AP;

>>          u32 ep_mask = ipa->filter_map;

>>

>> -       /* IPA version 4.2 has no hashed route tables */

>> -       if (ipa->version == IPA_VERSION_4_2)

>> +       if (!ipa_table_hash_support(ipa))

>>                  return;

>>

>>          while (ep_mask) {

>> @@ -582,8 +585,7 @@ static void ipa_route_config(struct ipa *ipa, bool modem)

>>   {

>>          u32 route_id;

>>

>> -       /* IPA version 4.2 has no hashed route tables */

>> -       if (ipa->version == IPA_VERSION_4_2)

>> +       if (!ipa_table_hash_support(ipa))

>>                  return;

>>

>>          for (route_id = 0; route_id < IPA_ROUTE_COUNT_MAX; route_id++)

>> diff --git a/drivers/net/ipa/ipa_table.h b/drivers/net/ipa/ipa_table.h

>> index 78038d14fcea9..1a68d20f19d6a 100644

>> --- a/drivers/net/ipa/ipa_table.h

>> +++ b/drivers/net/ipa/ipa_table.h

>> @@ -1,7 +1,7 @@

>>   /* SPDX-License-Identifier: GPL-2.0 */

>>

>>   /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.

>> - * Copyright (C) 2019-2020 Linaro Ltd.

>> + * Copyright (C) 2019-2021 Linaro Ltd.

>>    */

>>   #ifndef _IPA_TABLE_H_

>>   #define _IPA_TABLE_H_

>> @@ -51,6 +51,12 @@ static inline bool ipa_filter_map_valid(struct ipa *ipa, u32 filter_mask)

>>

>>   #endif /* !IPA_VALIDATE */

>>

>> +/**

>> + * ipa_table_hash_support() - Return true if hashed tables are supported

>> + * @ipa:       IPA pointer

>> + */

>> +bool ipa_table_hash_support(struct ipa *ipa);

>> +

>>   /**

>>    * ipa_table_reset() - Reset filter and route tables entries to "none"

>>    * @ipa:       IPA pointer

> 

> Just define the function here and make it a static inline.

>
diff mbox series

Patch

diff --git a/drivers/net/ipa/ipa_cmd.c b/drivers/net/ipa/ipa_cmd.c
index fd8bf6468d313..35e35852c25c5 100644
--- a/drivers/net/ipa/ipa_cmd.c
+++ b/drivers/net/ipa/ipa_cmd.c
@@ -268,7 +268,7 @@  static bool ipa_cmd_register_write_valid(struct ipa *ipa)
 	/* If hashed tables are supported, ensure the hash flush register
 	 * offset will fit in a register write IPA immediate command.
 	 */
-	if (ipa->version != IPA_VERSION_4_2) {
+	if (ipa_table_hash_support(ipa)) {
 		offset = ipa_reg_filt_rout_hash_flush_offset(ipa->version);
 		name = "filter/route hash flush";
 		if (!ipa_cmd_register_write_offset_valid(ipa, name, offset))
diff --git a/drivers/net/ipa/ipa_table.c b/drivers/net/ipa/ipa_table.c
index 32e2d3e052d55..baaab3dd0e63c 100644
--- a/drivers/net/ipa/ipa_table.c
+++ b/drivers/net/ipa/ipa_table.c
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 
 /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
- * Copyright (C) 2018-2020 Linaro Ltd.
+ * Copyright (C) 2018-2021 Linaro Ltd.
  */
 
 #include <linux/types.h>
@@ -239,6 +239,11 @@  static void ipa_table_validate_build(void)
 
 #endif /* !IPA_VALIDATE */
 
+bool ipa_table_hash_support(struct ipa *ipa)
+{
+	return ipa->version != IPA_VERSION_4_2;
+}
+
 /* Zero entry count means no table, so just return a 0 address */
 static dma_addr_t ipa_table_addr(struct ipa *ipa, bool filter_mask, u16 count)
 {
@@ -412,8 +417,7 @@  int ipa_table_hash_flush(struct ipa *ipa)
 	struct gsi_trans *trans;
 	u32 val;
 
-	/* IPA version 4.2 does not support hashed tables */
-	if (ipa->version == IPA_VERSION_4_2)
+	if (!ipa_table_hash_support(ipa))
 		return 0;
 
 	trans = ipa_cmd_trans_alloc(ipa, 1);
@@ -531,8 +535,7 @@  static void ipa_filter_config(struct ipa *ipa, bool modem)
 	enum gsi_ee_id ee_id = modem ? GSI_EE_MODEM : GSI_EE_AP;
 	u32 ep_mask = ipa->filter_map;
 
-	/* IPA version 4.2 has no hashed route tables */
-	if (ipa->version == IPA_VERSION_4_2)
+	if (!ipa_table_hash_support(ipa))
 		return;
 
 	while (ep_mask) {
@@ -582,8 +585,7 @@  static void ipa_route_config(struct ipa *ipa, bool modem)
 {
 	u32 route_id;
 
-	/* IPA version 4.2 has no hashed route tables */
-	if (ipa->version == IPA_VERSION_4_2)
+	if (!ipa_table_hash_support(ipa))
 		return;
 
 	for (route_id = 0; route_id < IPA_ROUTE_COUNT_MAX; route_id++)
diff --git a/drivers/net/ipa/ipa_table.h b/drivers/net/ipa/ipa_table.h
index 78038d14fcea9..1a68d20f19d6a 100644
--- a/drivers/net/ipa/ipa_table.h
+++ b/drivers/net/ipa/ipa_table.h
@@ -1,7 +1,7 @@ 
 /* SPDX-License-Identifier: GPL-2.0 */
 
 /* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
- * Copyright (C) 2019-2020 Linaro Ltd.
+ * Copyright (C) 2019-2021 Linaro Ltd.
  */
 #ifndef _IPA_TABLE_H_
 #define _IPA_TABLE_H_
@@ -51,6 +51,12 @@  static inline bool ipa_filter_map_valid(struct ipa *ipa, u32 filter_mask)
 
 #endif /* !IPA_VALIDATE */
 
+/**
+ * ipa_table_hash_support() - Return true if hashed tables are supported
+ * @ipa:	IPA pointer
+ */
+bool ipa_table_hash_support(struct ipa *ipa);
+
 /**
  * ipa_table_reset() - Reset filter and route tables entries to "none"
  * @ipa:	IPA pointer