On Tue, Sep 09, 2025 at 07:52:21PM +0530, Anwar, Md Danish wrote:
> On 9/9/2025 5:39 PM, Dong Yibo wrote:
> > Add fundamental mailbox (MBX) communication operations between PF (Physical
> > Function) and firmware for n500/n210 chips
> >
> > Signed-off-by: Dong Yibo <dong100@xxxxxxxxx>
> > ---
> > drivers/net/ethernet/mucse/rnpgbe/Makefile | 4 +-
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 25 ++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 70 +++
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 7 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 5 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 425 ++++++++++++++++++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 20 +
> > 7 files changed, 555 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> >
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > index 9df536f0d04c..5fc878ada4b1 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -5,4 +5,6 @@
> > #
>
> [ ... ]
>
> > +
> > +/**
> > + * rnpgbe_init_hw - Setup hw info according to board_type
> > + * @hw: hw information structure
> > + * @board_type: board type
> > + *
> > + * rnpgbe_init_hw initializes all hw data
> > + *
> > + * Return: 0 on success, negative errno on failure
> > + **/
> > +int rnpgbe_init_hw(struct mucse_hw *hw, int board_type)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + mbx->pf2fw_mbx_ctrl = MUCSE_GBE_PFFW_MBX_CTRL_OFFSET;
> > + mbx->fwpf_mbx_mask = MUCSE_GBE_FWPF_MBX_MASK_OFFSET;
> > +
> > + switch (board_type) {
> > + case board_n500:
> > + rnpgbe_init_n500(hw);
> > + break;
> > + case board_n210:
> > + rnpgbe_init_n210(hw);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
>
> The indentation of this switch block seems off to me.
>
> As per the coding guidlines
> https://www.kernel.org/doc/html/v4.14/process/coding-style.html#indentation
>
> Break statements should be at the same indentation level as the case
> code. The current indentation has the "break" statements at the same
> level as the case labels, which is inconsistent.
>
> This should be like,
>
> switch (board_type) {
> case board_n500:
> rnpgbe_init_n500(hw);
> break;
> case board_n210:
> rnpgbe_init_n210(hw);
> break;
> default:
> return -EINVAL;
> }
>
Sorry for the bad code style. I will fix this in next version.
> > + /* init_params with mbx base */
> > + mucse_init_mbx_params_pf(hw);
> > +
> > + return 0;
> > +}
>
[ ... ]
>
>
>
> --
> Thanks and Regards,
> Md Danish Anwar
>
>
Thanks for your feedback.
