From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Rasmus Villemoes <rasmus.villemoes@prevas.dk>, netdev@vger.kernel.org
Cc: Li Yang <leoyang.li@nxp.com>,
"David S . Miller" <davem@davemloft.net>,
Zhao Qiang <qiang.zhao@nxp.com>, Andrew Lunn <andrew@lunn.ch>,
Jakub Kicinski <kuba@kernel.org>,
Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Subject: Re: [PATCH net-next v2 08/17] ethernet: ucc_geth: remove {rx,tx}_glbl_pram_offset from struct ucc_geth_private
Date: Wed, 20 Jan 2021 07:57:03 +0100 [thread overview]
Message-ID: <26c7c3cc-2dec-c86e-2e9d-63dc1a7ddba1@csgroup.eu> (raw)
In-Reply-To: <20210119150802.19997-9-rasmus.villemoes@prevas.dk>
Le 19/01/2021 à 16:07, Rasmus Villemoes a écrit :
> These fields are only used within ucc_geth_startup(), so they might as
> well be local variables in that function rather than being stashed in
> struct ucc_geth_private.
>
> Aside from making that struct a tiny bit smaller, it also shortens
> some lines (getting rid of pointless casts while here), and fixes the
> problems with using IS_ERR_VALUE() on a u32 as explained in commit
> 800cd6fb76f0 ("soc: fsl: qe: change return type of cpm_muram_alloc()
> to s32").
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> drivers/net/ethernet/freescale/ucc_geth.c | 21 +++++++++------------
> drivers/net/ethernet/freescale/ucc_geth.h | 2 --
> 2 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index 74ee2ed2fbbb..75466489bf9a 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -2351,6 +2351,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> u8 function_code = 0;
> u8 __iomem *endOfRing;
> u8 numThreadsRxNumerical, numThreadsTxNumerical;
> + s32 rx_glbl_pram_offset, tx_glbl_pram_offset;
That's still a quite long name for a local variable. Kernel Codying Style says:
LOCAL variable names should be short, and to the point. If you have some random integer loop
counter, it should probably be called i. Calling it loop_counter is non-productive, if there is no
chance of it being mis-understood. Similarly, tmp can be just about any type of variable that is
used to hold a temporary value.
If you are afraid to mix up your local variable names, you have another problem, which is called the
function-growth-hormone-imbalance syndrome. See chapter 6 (Functions).
>
> ugeth_vdbg("%s: IN", __func__);
> uccf = ugeth->uccf;
> @@ -2495,17 +2496,15 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> */
> /* Tx global PRAM */
> /* Allocate global tx parameter RAM page */
> - ugeth->tx_glbl_pram_offset =
> + tx_glbl_pram_offset =
> qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
> UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
> - if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
> + if (tx_glbl_pram_offset < 0) {
> if (netif_msg_ifup(ugeth))
> pr_err("Can not allocate DPRAM memory for p_tx_glbl_pram\n");
> return -ENOMEM;
> }
> - ugeth->p_tx_glbl_pram =
> - (struct ucc_geth_tx_global_pram __iomem *) qe_muram_addr(ugeth->
> - tx_glbl_pram_offset);
> + ugeth->p_tx_glbl_pram = qe_muram_addr(tx_glbl_pram_offset);
> /* Fill global PRAM */
>
> /* TQPTR */
> @@ -2656,17 +2655,15 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
>
> /* Rx global PRAM */
> /* Allocate global rx parameter RAM page */
> - ugeth->rx_glbl_pram_offset =
> + rx_glbl_pram_offset =
> qe_muram_alloc(sizeof(struct ucc_geth_rx_global_pram),
> UCC_GETH_RX_GLOBAL_PRAM_ALIGNMENT);
> - if (IS_ERR_VALUE(ugeth->rx_glbl_pram_offset)) {
> + if (rx_glbl_pram_offset < 0) {
> if (netif_msg_ifup(ugeth))
> pr_err("Can not allocate DPRAM memory for p_rx_glbl_pram\n");
> return -ENOMEM;
> }
> - ugeth->p_rx_glbl_pram =
> - (struct ucc_geth_rx_global_pram __iomem *) qe_muram_addr(ugeth->
> - rx_glbl_pram_offset);
> + ugeth->p_rx_glbl_pram = qe_muram_addr(rx_glbl_pram_offset);
> /* Fill global PRAM */
>
> /* RQPTR */
> @@ -2928,7 +2925,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> ((u32) ug_info->numThreadsTx) << ENET_INIT_PARAM_TGF_SHIFT;
>
> ugeth->p_init_enet_param_shadow->rgftgfrxglobal |=
> - ugeth->rx_glbl_pram_offset | ug_info->riscRx;
> + rx_glbl_pram_offset | ug_info->riscRx;
> if ((ug_info->largestexternallookupkeysize !=
> QE_FLTR_LARGEST_EXTERNAL_TABLE_LOOKUP_KEY_SIZE_NONE) &&
> (ug_info->largestexternallookupkeysize !=
> @@ -2966,7 +2963,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> }
>
> ugeth->p_init_enet_param_shadow->txglobal =
> - ugeth->tx_glbl_pram_offset | ug_info->riscTx;
> + tx_glbl_pram_offset | ug_info->riscTx;
> if ((ret_val =
> fill_init_enet_entries(ugeth,
> &(ugeth->p_init_enet_param_shadow->
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
> index c80bed2c995c..be47fa8ced15 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.h
> +++ b/drivers/net/ethernet/freescale/ucc_geth.h
> @@ -1166,9 +1166,7 @@ struct ucc_geth_private {
> struct ucc_geth_exf_global_pram __iomem *p_exf_glbl_param;
> u32 exf_glbl_param_offset;
> struct ucc_geth_rx_global_pram __iomem *p_rx_glbl_pram;
> - u32 rx_glbl_pram_offset;
> struct ucc_geth_tx_global_pram __iomem *p_tx_glbl_pram;
> - u32 tx_glbl_pram_offset;
> struct ucc_geth_send_queue_mem_region __iomem *p_send_q_mem_reg;
> u32 send_q_mem_reg_offset;
> struct ucc_geth_thread_data_tx __iomem *p_thread_data_tx;
>
next prev parent reply other threads:[~2021-01-20 6:58 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-19 15:07 [PATCH net-next v2 00/17] ucc_geth improvements Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 01/17] ethernet: ucc_geth: remove unused read of temoder field Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 02/17] soc: fsl: qe: make cpm_muram_offset take a const void* argument Rasmus Villemoes
2021-01-19 18:42 ` Leo Li
2021-01-19 15:07 ` [PATCH net-next v2 03/17] soc: fsl: qe: store muram_vbase as a void pointer instead of u8 Rasmus Villemoes
2021-01-19 18:45 ` Li Yang
2021-01-19 15:07 ` [PATCH net-next v2 04/17] soc: fsl: qe: add cpm_muram_free_addr() helper Rasmus Villemoes
2021-01-19 18:46 ` Li Yang
2021-01-19 15:07 ` [PATCH net-next v2 05/17] ethernet: ucc_geth: use qe_muram_free_addr() Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 06/17] ethernet: ucc_geth: remove unnecessary memset_io() calls Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 07/17] ethernet: ucc_geth: replace kmalloc+memset by kzalloc Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 08/17] ethernet: ucc_geth: remove {rx,tx}_glbl_pram_offset from struct ucc_geth_private Rasmus Villemoes
2021-01-20 6:57 ` Christophe Leroy [this message]
2021-01-20 11:26 ` Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 09/17] ethernet: ucc_geth: factor out parsing of {rx,tx}-clock{,-name} properties Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 10/17] ethernet: ucc_geth: constify ugeth_primary_info Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 11/17] ethernet: ucc_geth: don't statically allocate eight ucc_geth_info Rasmus Villemoes
2021-01-20 7:02 ` Christophe Leroy
2021-01-20 11:25 ` Rasmus Villemoes
2021-01-20 11:30 ` Christophe Leroy
2021-01-19 15:07 ` [PATCH net-next v2 12/17] ethernet: ucc_geth: use UCC_GETH_{RX,TX}_BD_RING_ALIGNMENT macros directly Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 13/17] ethernet: ucc_geth: remove bd_mem_part and all associated code Rasmus Villemoes
2021-01-20 7:17 ` Christophe Leroy
2021-01-20 11:33 ` Rasmus Villemoes
2021-01-19 15:07 ` [PATCH net-next v2 14/17] ethernet: ucc_geth: replace kmalloc_array()+for loop by kcalloc() Rasmus Villemoes
2021-01-20 7:19 ` Christophe Leroy
2021-01-19 15:08 ` [PATCH net-next v2 15/17] ethernet: ucc_geth: add helper to replace repeated switch statements Rasmus Villemoes
2021-01-19 15:08 ` [PATCH net-next v2 16/17] ethernet: ucc_geth: inform the compiler that numQueues is always 1 Rasmus Villemoes
2021-01-19 15:08 ` [PATCH net-next v2 17/17] ethernet: ucc_geth: simplify rx/tx allocations Rasmus Villemoes
2021-01-19 17:52 ` [PATCH net-next v2 00/17] ucc_geth improvements Leo Li
2021-01-21 20:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=26c7c3cc-2dec-c86e-2e9d-63dc1a7ddba1@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=Joakim.Tjernlund@infinera.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=leoyang.li@nxp.com \
--cc=netdev@vger.kernel.org \
--cc=qiang.zhao@nxp.com \
--cc=rasmus.villemoes@prevas.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).