On Tue, Jan 10, 2023 at 07:04:00PM +0200, Ramon Fried wrote:

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

...

The cur_ptr variable is set to the start of the log buffer but then it's not used. Just remove the assignment altogether.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 6b36860187ca..66fcb48ebd55 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -1796,7 +1796,6 @@ static void mc_dump_log(void) if (size > bytes_end) { print_k_bytes(cur_ptr, &bytes_end);

•           cur_ptr = buf;
            size -= bytes_end;
    }
  
  

-- 2.25.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

Thanks for the reviews!

These kind of network patches go through the u-boot-net tree or is there another path?

Ioana

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

The break statement is just after a goto statement, thus it will not get executed. Just remove it.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 66fcb48ebd55..180e57e42266 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -1961,7 +1961,6 @@ static int do_fsl_mc(struct cmd_tbl *cmdtp, int flag, int argc, default: printf("Invalid option: %s\n", argv[1]); goto usage;

•           break;
    }
    return err;
  

  usage:

-- 2.25.1

It's possible that it's there only to silent dumb compiler who thinks that break was forgotten. Let's see if it introduces problems in the CI. meanwhile I'll approve. Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

The fsl-mc driver printed debug information which used the 0x prefix for decimal values. This only confuses anyone looking through the log. Because of this, just remove the prefix and use the "DPXY.<id>" notation which is the standard one for the DPAA2 objects.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 180e57e42266..440273a17562 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -1038,7 +1038,7 @@ static int dpio_init(void) }

#ifdef DEBUG

•   printf("Init: DPIO id=0x%d\n", dflt_dpio->dpio_id);
  

•   printf("Init: DPIO.%d\n", dflt_dpio->dpio_id);
  

#endif err = dpio_enable(dflt_mc_io, MC_CMD_NO_FLAGS, dflt_dpio->dpio_handle); if (err < 0) { @@ -1107,7 +1107,7 @@ static int dpio_exit(void) }

#ifdef DEBUG

•   printf("Exit: DPIO id=0x%d\n", dflt_dpio->dpio_id);
  

•   printf("Exit: DPIO.%d\n", dflt_dpio->dpio_id);
  

#endif

    if (dflt_dpio)

@@ -1312,7 +1312,7 @@ static int dpbp_init(void) }

#ifdef DEBUG

•   printf("Init: DPBP id=0x%x\n", dflt_dpbp->dpbp_attr.id);
  

•   printf("Init: DPBP.%d\n", dflt_dpbp->dpbp_attr.id);
  

#endif

    err = dpbp_close(dflt_mc_io, MC_CMD_NO_FLAGS, dflt_dpbp->dpbp_handle);

@@ -1351,7 +1351,7 @@ static int dpbp_exit(void) }

#ifdef DEBUG

•   printf("Exit: DPBP id=0x%d\n", dflt_dpbp->dpbp_attr.id);
  

•   printf("Exit: DPBP.%d\n", dflt_dpbp->dpbp_attr.id);
  

#endif

    if (dflt_dpbp)

@@ -1422,7 +1422,7 @@ static int dpni_init(void) }

#ifdef DEBUG

•   printf("Init: DPNI id=0x%d\n", dflt_dpni->dpni_id);
  

•   printf("Init: DPNI.%d\n", dflt_dpni->dpni_id);
  

#endif err = dpni_close(dflt_mc_io, MC_CMD_NO_FLAGS, dflt_dpni->dpni_handle); if (err < 0) { @@ -1459,7 +1459,7 @@ static int dpni_exit(void) }

#ifdef DEBUG

•   printf("Exit: DPNI id=0x%d\n", dflt_dpni->dpni_id);
  

•   printf("Exit: DPNI.%d\n", dflt_dpni->dpni_id);
  

#endif

    if (dflt_dpni)

-- 2.25.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

Some of the global variables were being initialised either to NULL or 0. This is not needed, just remove the it.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 440273a17562..001c77ba8355 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -57,14 +57,14 @@ static int mc_dpl_applied = -1; #ifdef CFG_SYS_LS_MC_DRAM_AIOP_IMG_OFFSET static int mc_aiop_applied = -1; #endif -struct fsl_mc_io *root_mc_io = NULL; -struct fsl_mc_io *dflt_mc_io = NULL; /* child container */ -uint16_t root_dprc_handle = 0; -uint16_t dflt_dprc_handle = 0; +struct fsl_mc_io *root_mc_io; +struct fsl_mc_io *dflt_mc_io; +uint16_t root_dprc_handle; +uint16_t dflt_dprc_handle; int child_dprc_id; -struct fsl_dpbp_obj *dflt_dpbp = NULL; -struct fsl_dpio_obj *dflt_dpio = NULL; -struct fsl_dpni_obj *dflt_dpni = NULL; +struct fsl_dpbp_obj *dflt_dpbp; +struct fsl_dpio_obj *dflt_dpio; +struct fsl_dpni_obj *dflt_dpni; static u64 mc_lazy_dpl_addr; static u32 dpsparser_obj_id; static u16 dpsparser_handle; -- 2.25.1

I don't believe it's a bad habit to initialize static variables. it's true that they will be initialized to zero by default. But I don't find this patch helpful in any way for understanding or decluttering code.

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

Remove all the explicit casts from the void* returned by calloc. With this we also improve a bit the length of those lines and there is no need to split the assignment.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 001c77ba8355..5ad47e3414b5 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -812,7 +812,7 @@ int mc_init(u64 mc_fw_addr, u64 mc_dpc_addr) * Initialize the global default MC portal * And check that the MC firmware is responding portal commands: */

•   root_mc_io = (struct fsl_mc_io *)calloc(sizeof(struct fsl_mc_io), 1);
  

•   root_mc_io = calloc(sizeof(struct fsl_mc_io), 1);
    if (!root_mc_io) {
            printf(" No memory: calloc() failed\n");
            return -ENOMEM;
  

@@ -979,8 +979,7 @@ static int dpio_init(void) int err = 0; uint16_t major_ver, minor_ver;

•   dflt_dpio = (struct fsl_dpio_obj *)calloc(
  

•                                   sizeof(struct fsl_dpio_obj), 1);
  

•   dflt_dpio = calloc(sizeof(struct fsl_dpio_obj), 1);
    if (!dflt_dpio) {
            printf("No memory: calloc() failed\n");
            err = -ENOMEM;
  

@@ -1168,7 +1167,7 @@ static int dprc_init(void) goto err_create; }

•   dflt_mc_io = (struct fsl_mc_io *)calloc(sizeof(struct fsl_mc_io), 1);
  

•   dflt_mc_io = calloc(sizeof(struct fsl_mc_io), 1);
    if (!dflt_mc_io) {
            err  = -ENOMEM;
            printf(" No memory: calloc() failed\n");
  

@@ -1250,8 +1249,7 @@ static int dpbp_init(void) struct dpbp_cfg dpbp_cfg; uint16_t major_ver, minor_ver;

•   dflt_dpbp = (struct fsl_dpbp_obj *)calloc(
  

•                                   sizeof(struct fsl_dpbp_obj), 1);
  

•   dflt_dpbp = calloc(sizeof(struct fsl_dpbp_obj), 1);
    if (!dflt_dpbp) {
            printf("No memory: calloc() failed\n");
            err = -ENOMEM;
  

@@ -1369,8 +1367,7 @@ static int dpni_init(void) struct dpni_cfg dpni_cfg; uint16_t major_ver, minor_ver;

•   dflt_dpni = (struct fsl_dpni_obj *)calloc(
  

•                                   sizeof(struct fsl_dpni_obj), 1);
  

•   dflt_dpni = calloc(sizeof(struct fsl_dpni_obj), 1);
    if (!dflt_dpni) {
            printf("No memory: calloc() failed\n");
            err = -ENOMEM;
  

-- 2.25.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

There were some cases in which the function parameters were not aligned to the open paranthesis. Fix those instances.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 5ad47e3414b5..531835fbd503 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -356,8 +356,7 @@ static int mc_fixup_dpc_mac_addr(void *blob, int dpmac_id, if (noff < 0) { err = fdt_increase_size(blob, 200); if (err) {

•                   printf("fdt_increase_size: err=%s\n",
  

•                           fdt_strerror(err));
  

•                   printf("fdt_increase_size: err=%s\n", fdt_strerror(err));
                    return err;
            }
  
  

@@ -373,7 +372,7 @@ static int mc_fixup_dpc_mac_addr(void *blob, int dpmac_id, "link_type", link_type_mode); if (err) { printf("fdt_appendprop_string: err=%s\n",

•                           fdt_strerror(err));
  

•                          fdt_strerror(err));
                    return err;
            }
    }
  

@@ -1158,10 +1157,9 @@ static int dprc_init(void) cfg.icid = DPRC_GET_ICID_FROM_POOL; cfg.portal_id = DPRC_GET_PORTAL_ID_FROM_POOL; err = dprc_create_container(root_mc_io, MC_CMD_NO_FLAGS,

•                   root_dprc_handle,
  

•                   &cfg,
  

•                   &child_dprc_id,
  

•                   &mc_portal_offset);
  

•                               root_dprc_handle, &cfg,
  

•                               &child_dprc_id,
  

•                               &mc_portal_offset);
    if (err < 0) {
            printf("dprc_create_container() failed: %d\n", err);
            goto err_create;
  

-- 2.25.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Thu, Jan 5, 2023 at 5:03 PM Ioana Ciornei ioana.ciornei@nxp.com wrote:

Remove the instances in which we have multiple blank lines.

Signed-off-by: Ioana Ciornei ioana.ciornei@nxp.com

drivers/net/fsl-mc/mc.c | 2 -- 1 file changed, 2 deletions(-)

diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c index 531835fbd503..f78f070aec1c 100644 --- a/drivers/net/fsl-mc/mc.c +++ b/drivers/net/fsl-mc/mc.c @@ -526,7 +526,6 @@ static int load_mc_dpc(u64 mc_ram_addr, size_t mc_ram_size, u64 mc_dpc_addr) return 0; }

static int mc_fixup_dpl(u64 dpl_addr) { void *blob = (void *)dpl_addr; @@ -698,7 +697,6 @@ static int wait_for_mc(bool booting_mc, u32 *final_reg_gsr) printf("SUCCESS\n"); }

•   *final_reg_gsr = reg_gsr;
    return 0;
  

}

2.25.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com