diff mbox series

[net] net: ethernet: marvell: octeontx2: nic: Add error pointer check in otx2_ethtool.c

Message ID 20240923113135.4366-1-kdipendra88@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net] net: ethernet: marvell: octeontx2: nic: Add error pointer check in otx2_ethtool.c | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7 this patch: 7
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 24 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-24--03-00 (tests: 762)

Commit Message

Dipendra Khadka Sept. 23, 2024, 11:31 a.m. UTC
Add error pointer check after calling otx2_mbox_get_rsp().

Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
---
 .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c    | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Simon Horman Sept. 24, 2024, 7:10 a.m. UTC | #1
On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> Add error pointer check after calling otx2_mbox_get_rsp().
> 

Hi Dipendra,

Please add a fixes tag here (no blank line between it and your
Signed-off-by line).
> Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>

As you have posted more than one patch for this driver, with very similar,
not overly complex or verbose changes, it might make sense to combine them
into a single patch. Or, if not, to bundle them up into a patch-set with a
cover letter.

Regarding the patch subject, looking at git history, I think
an appropriate prefix would be 'octeontx2-pf:'. I would go for
something like this:

  Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors

As for the code changes themselves, module the nits below, I agree the
error handling is consistent with that elsewhere in the same functions, and
is correct.

> ---
>  .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c    | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> index 0db62eb0dab3..36a08303752f 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> @@ -343,6 +343,12 @@ static void otx2_get_pauseparam(struct net_device *netdev,
>  	if (!otx2_sync_mbox_msg(&pfvf->mbox)) {
>  		rsp = (struct cgx_pause_frm_cfg *)
>  		       otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr);
> +

nit: No blank line here.

> +		if (IS_ERR(rsp)) {
> +			mutex_unlock(&pfvf->mbox.lock);
> +			return;
> +		}
> +
>  		pause->rx_pause = rsp->rx_pause;
>  		pause->tx_pause = rsp->tx_pause;
>  	}
> @@ -1074,6 +1080,12 @@ static int otx2_set_fecparam(struct net_device *netdev,
>  
>  	rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox,
>  						   0, &req->hdr);
> +

Ditto.

> +	if (IS_ERR(rsp)) {
> +		err = PTR_ERR(rsp);
> +		goto end;
> +	}
> +
>  	if (rsp->fec >= 0)
>  		pfvf->linfo.fec = rsp->fec;
>  	else
Dipendra Khadka Sept. 24, 2024, 2:54 p.m. UTC | #2
Hi Simon,

On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > Add error pointer check after calling otx2_mbox_get_rsp().
> >
>
> Hi Dipendra,
>
> Please add a fixes tag here (no blank line between it and your
> Signed-off-by line).
> > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
>
> As you have posted more than one patch for this driver, with very similar,
> not overly complex or verbose changes, it might make sense to combine them
> into a single patch. Or, if not, to bundle them up into a patch-set with a
> cover letter.
>
> Regarding the patch subject, looking at git history, I think
> an appropriate prefix would be 'octeontx2-pf:'. I would go for
> something like this:
>
>   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
>

If I bundle all the patches for the
drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
work? Or do I need to change anything? I don't know how to send the
patch-set with the cover letter.

> As for the code changes themselves, module the nits below, I agree the
> error handling is consistent with that elsewhere in the same functions, and
> is correct.
>
> > ---
> >  .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c    | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > index 0db62eb0dab3..36a08303752f 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > @@ -343,6 +343,12 @@ static void otx2_get_pauseparam(struct net_device *netdev,
> >       if (!otx2_sync_mbox_msg(&pfvf->mbox)) {
> >               rsp = (struct cgx_pause_frm_cfg *)
> >                      otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr);
> > +
>
> nit: No blank line here.
>
> > +             if (IS_ERR(rsp)) {
> > +                     mutex_unlock(&pfvf->mbox.lock);
> > +                     return;
> > +             }
> > +

If the above blank line after the check is ok or do I have to remove
this as well?

> >               pause->rx_pause = rsp->rx_pause;
> >               pause->tx_pause = rsp->tx_pause;
> >       }
> > @@ -1074,6 +1080,12 @@ static int otx2_set_fecparam(struct net_device *netdev,
> >
> >       rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox,
> >                                                  0, &req->hdr);
> > +
>
> Ditto.
>
> > +     if (IS_ERR(rsp)) {
> > +             err = PTR_ERR(rsp);
> > +             goto end;
> > +     }
> > +
> >       if (rsp->fec >= 0)
> >               pfvf->linfo.fec = rsp->fec;
> >       else
>
> --
> pw-bot: changes-requested

Best regards,
Dipendra Khadka
Simon Horman Sept. 24, 2024, 3:58 p.m. UTC | #3
On Tue, Sep 24, 2024 at 08:39:47PM +0545, Dipendra Khadka wrote:
> Hi Simon,
> 
> On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
> >
> > On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > > Add error pointer check after calling otx2_mbox_get_rsp().
> > >
> >
> > Hi Dipendra,
> >
> > Please add a fixes tag here (no blank line between it and your
> > Signed-off-by line).
> > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> >
> > As you have posted more than one patch for this driver, with very similar,
> > not overly complex or verbose changes, it might make sense to combine them
> > into a single patch. Or, if not, to bundle them up into a patch-set with a
> > cover letter.
> >
> > Regarding the patch subject, looking at git history, I think
> > an appropriate prefix would be 'octeontx2-pf:'. I would go for
> > something like this:
> >
> >   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
> >
> 
> If I bundle all the patches for the
> drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
> work? Or do I need to change anything? I don't know how to send the
> patch-set with the cover letter.

Given that one of the patches is already at v2, probably v3 is best.

If you use b4, it should send a cover letter if the series has more than 1
patch.  You can use various options to b4 prep to set the prefix
(net-next), version, and edit the cover (letter).  And you can use various
options to b4 send, such as -d, to test your submission before sending it
to the netdev ML.

Alternatively the following command will output 3 files: a cover letter and
a file for each of two patches, with v3 and net-next in the subject of each
file. You can edit these files and send them using git send-email.

git format-patch --cover-letter -2 -v3 --subject-prefix="PATCH net-next"

> 
> > As for the code changes themselves, module the nits below, I agree the
> > error handling is consistent with that elsewhere in the same functions, and
> > is correct.
> >
> > > ---
> > >  .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c    | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > > index 0db62eb0dab3..36a08303752f 100644
> > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > > @@ -343,6 +343,12 @@ static void otx2_get_pauseparam(struct net_device *netdev,
> > >       if (!otx2_sync_mbox_msg(&pfvf->mbox)) {
> > >               rsp = (struct cgx_pause_frm_cfg *)
> > >                      otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr);
> > > +
> >
> > nit: No blank line here.
> >
> > > +             if (IS_ERR(rsp)) {
> > > +                     mutex_unlock(&pfvf->mbox.lock);
> > > +                     return;
> > > +             }
> > > +
> 
> If the above blank line after the check is ok or do I have to remove
> this as well?

Please leave the blank line after the check (here).

> 
> > >               pause->rx_pause = rsp->rx_pause;
> > >               pause->tx_pause = rsp->tx_pause;
> > >       }
Dipendra Khadka Sept. 24, 2024, 5:57 p.m. UTC | #4
Hi Simon,

On Tue, 24 Sept 2024 at 21:43, Simon Horman <horms@kernel.org> wrote:
>
> On Tue, Sep 24, 2024 at 08:39:47PM +0545, Dipendra Khadka wrote:
> > Hi Simon,
> >
> > On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
> > >
> > > On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > > > Add error pointer check after calling otx2_mbox_get_rsp().
> > > >
> > >
> > > Hi Dipendra,
> > >
> > > Please add a fixes tag here (no blank line between it and your
> > > Signed-off-by line).
> > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > >
> > > As you have posted more than one patch for this driver, with very similar,
> > > not overly complex or verbose changes, it might make sense to combine them
> > > into a single patch. Or, if not, to bundle them up into a patch-set with a
> > > cover letter.
> > >
> > > Regarding the patch subject, looking at git history, I think
> > > an appropriate prefix would be 'octeontx2-pf:'. I would go for
> > > something like this:
> > >
> > >   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
> > >
> >
> > If I bundle all the patches for the
> > drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
> > work? Or do I need to change anything? I don't know how to send the
> > patch-set with the cover letter.
>
> Given that one of the patches is already at v2, probably v3 is best.
>
> If you use b4, it should send a cover letter if the series has more than 1
> patch.  You can use various options to b4 prep to set the prefix
> (net-next), version, and edit the cover (letter).  And you can use various
> options to b4 send, such as -d, to test your submission before sending it
> to the netdev ML.
>

I did not get this -d and testing? testing in net-next and sending to net?

> Alternatively the following command will output 3 files: a cover letter and
> a file for each of two patches, with v3 and net-next in the subject of each
> file. You can edit these files and send them using git send-email.
>
> git format-patch --cover-letter -2 -v3 --subject-prefix="PATCH net-next"
>

Should I send it to net-next or net?

Thank you so much for teaching me all these.

> >
> > > As for the code changes themselves, module the nits below, I agree the
> > > error handling is consistent with that elsewhere in the same functions, and
> > > is correct.
> > >
> > > > ---
> > > >  .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c    | 12 ++++++++++++
> > > >  1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > > > index 0db62eb0dab3..36a08303752f 100644
> > > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > > > @@ -343,6 +343,12 @@ static void otx2_get_pauseparam(struct net_device *netdev,
> > > >       if (!otx2_sync_mbox_msg(&pfvf->mbox)) {
> > > >               rsp = (struct cgx_pause_frm_cfg *)
> > > >                      otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr);
> > > > +
> > >
> > > nit: No blank line here.
> > >
> > > > +             if (IS_ERR(rsp)) {
> > > > +                     mutex_unlock(&pfvf->mbox.lock);
> > > > +                     return;
> > > > +             }
> > > > +
> >
> > If the above blank line after the check is ok or do I have to remove
> > this as well?
>
> Please leave the blank line after the check (here).
>
> >
> > > >               pause->rx_pause = rsp->rx_pause;
> > > >               pause->tx_pause = rsp->tx_pause;
> > > >       }

Best regards,
Dipendra Khadka
Simon Horman Sept. 24, 2024, 6:14 p.m. UTC | #5
On Tue, Sep 24, 2024 at 11:42:58PM +0545, Dipendra Khadka wrote:
> Hi Simon,
> 
> On Tue, 24 Sept 2024 at 21:43, Simon Horman <horms@kernel.org> wrote:
> >
> > On Tue, Sep 24, 2024 at 08:39:47PM +0545, Dipendra Khadka wrote:
> > > Hi Simon,
> > >
> > > On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
> > > >
> > > > On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > > > > Add error pointer check after calling otx2_mbox_get_rsp().
> > > > >
> > > >
> > > > Hi Dipendra,
> > > >
> > > > Please add a fixes tag here (no blank line between it and your
> > > > Signed-off-by line).
> > > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > >
> > > > As you have posted more than one patch for this driver, with very similar,
> > > > not overly complex or verbose changes, it might make sense to combine them
> > > > into a single patch. Or, if not, to bundle them up into a patch-set with a
> > > > cover letter.
> > > >
> > > > Regarding the patch subject, looking at git history, I think
> > > > an appropriate prefix would be 'octeontx2-pf:'. I would go for
> > > > something like this:
> > > >
> > > >   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
> > > >
> > >
> > > If I bundle all the patches for the
> > > drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
> > > work? Or do I need to change anything? I don't know how to send the
> > > patch-set with the cover letter.
> >
> > Given that one of the patches is already at v2, probably v3 is best.
> >
> > If you use b4, it should send a cover letter if the series has more than 1
> > patch.  You can use various options to b4 prep to set the prefix
> > (net-next), version, and edit the cover (letter).  And you can use various
> > options to b4 send, such as -d, to test your submission before sending it
> > to the netdev ML.
> >
> 
> I did not get this -d and testing? testing in net-next and sending to net?

I meant that b4 prep -d allows you to see the emails that would be sent
without actually sending them. I find this quite useful myself.

> 
> > Alternatively the following command will output 3 files: a cover letter and
> > a file for each of two patches, with v3 and net-next in the subject of each
> > file. You can edit these files and send them using git send-email.
> >
> > git format-patch --cover-letter -2 -v3 --subject-prefix="PATCH net-next"
> >
> 
> Should I send it to net-next or net?

Sorry for the confusion. I wrote net-next in my example,
but I think this patch-set would be for net.

...
Dipendra Khadka Sept. 25, 2024, 6:22 a.m. UTC | #6
On Wed, 25 Sept 2024 at 00:00, Simon Horman <horms@kernel.org> wrote:
>
> On Tue, Sep 24, 2024 at 11:42:58PM +0545, Dipendra Khadka wrote:
> > Hi Simon,
> >
> > On Tue, 24 Sept 2024 at 21:43, Simon Horman <horms@kernel.org> wrote:
> > >
> > > On Tue, Sep 24, 2024 at 08:39:47PM +0545, Dipendra Khadka wrote:
> > > > Hi Simon,
> > > >
> > > > On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
> > > > >
> > > > > On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > > > > > Add error pointer check after calling otx2_mbox_get_rsp().
> > > > > >
> > > > >
> > > > > Hi Dipendra,
> > > > >
> > > > > Please add a fixes tag here (no blank line between it and your
> > > > > Signed-off-by line).
> > > > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > > >
> > > > > As you have posted more than one patch for this driver, with very similar,
> > > > > not overly complex or verbose changes, it might make sense to combine them
> > > > > into a single patch. Or, if not, to bundle them up into a patch-set with a
> > > > > cover letter.
> > > > >
> > > > > Regarding the patch subject, looking at git history, I think
> > > > > an appropriate prefix would be 'octeontx2-pf:'. I would go for
> > > > > something like this:
> > > > >
> > > > >   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
> > > > >
> > > >
> > > > If I bundle all the patches for the
> > > > drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
> > > > work? Or do I need to change anything? I don't know how to send the
> > > > patch-set with the cover letter.
> > >
> > > Given that one of the patches is already at v2, probably v3 is best.
> > >
> > > If you use b4, it should send a cover letter if the series has more than 1
> > > patch.  You can use various options to b4 prep to set the prefix
> > > (net-next), version, and edit the cover (letter).  And you can use various
> > > options to b4 send, such as -d, to test your submission before sending it
> > > to the netdev ML.
> > >
> >
> > I did not get this -d and testing? testing in net-next and sending to net?
>
> I meant that b4 prep -d allows you to see the emails that would be sent
> without actually sending them. I find this quite useful myself.
>
> >
> > > Alternatively the following command will output 3 files: a cover letter and
> > > a file for each of two patches, with v3 and net-next in the subject of each
> > > file. You can edit these files and send them using git send-email.
> > >
> > > git format-patch --cover-letter -2 -v3 --subject-prefix="PATCH net-next"
> > >
> >
> > Should I send it to net-next or net?
>
> Sorry for the confusion. I wrote net-next in my example,
> but I think this patch-set would be for net.
>
> ...

Thank you Simon for everything.

Best regards,
Dipendra
Dipendra Khadka Sept. 26, 2024, 5:42 a.m. UTC | #7
Hi Simon,

On Wed, 25 Sept 2024 at 12:07, Dipendra Khadka <kdipendra88@gmail.com> wrote:
>
> On Wed, 25 Sept 2024 at 00:00, Simon Horman <horms@kernel.org> wrote:
> >
> > On Tue, Sep 24, 2024 at 11:42:58PM +0545, Dipendra Khadka wrote:
> > > Hi Simon,
> > >
> > > On Tue, 24 Sept 2024 at 21:43, Simon Horman <horms@kernel.org> wrote:
> > > >
> > > > On Tue, Sep 24, 2024 at 08:39:47PM +0545, Dipendra Khadka wrote:
> > > > > Hi Simon,
> > > > >
> > > > > On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
> > > > > >
> > > > > > On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > > > > > > Add error pointer check after calling otx2_mbox_get_rsp().
> > > > > > >
> > > > > >
> > > > > > Hi Dipendra,
> > > > > >
> > > > > > Please add a fixes tag here (no blank line between it and your
> > > > > > Signed-off-by line).
> > > > > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > > > >
> > > > > > As you have posted more than one patch for this driver, with very similar,
> > > > > > not overly complex or verbose changes, it might make sense to combine them
> > > > > > into a single patch. Or, if not, to bundle them up into a patch-set with a
> > > > > > cover letter.
> > > > > >
> > > > > > Regarding the patch subject, looking at git history, I think
> > > > > > an appropriate prefix would be 'octeontx2-pf:'. I would go for
> > > > > > something like this:
> > > > > >
> > > > > >   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
> > > > > >
> > > > >
> > > > > If I bundle all the patches for the
> > > > > drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
> > > > > work? Or do I need to change anything? I don't know how to send the
> > > > > patch-set with the cover letter.
> > > >
> > > > Given that one of the patches is already at v2, probably v3 is best.
> > > >
> > > > If you use b4, it should send a cover letter if the series has more than 1
> > > > patch.  You can use various options to b4 prep to set the prefix
> > > > (net-next), version, and edit the cover (letter).  And you can use various
> > > > options to b4 send, such as -d, to test your submission before sending it
> > > > to the netdev ML.
> > > >
> > >
> > > I did not get this -d and testing? testing in net-next and sending to net?
> >
> > I meant that b4 prep -d allows you to see the emails that would be sent
> > without actually sending them. I find this quite useful myself.
> >
> > >
> > > > Alternatively the following command will output 3 files: a cover letter and
> > > > a file for each of two patches, with v3 and net-next in the subject of each
> > > > file. You can edit these files and send them using git send-email.
> > > >
> > > > git format-patch --cover-letter -2 -v3 --subject-prefix="PATCH net-next"
> > > >

Do I need to maintain patch history below  Signed-off-by for each
patch when I send them in the patch set? If so, what to do with those
which have v1 but no v2 but the patch-set in v3?

> > >
> > > Should I send it to net-next or net?
> >
> > Sorry for the confusion. I wrote net-next in my example,
> > but I think this patch-set would be for net.
> >
> > ...
>
> Thank you Simon for everything.
>
> Best regards,
> Dipendra

Best regards,
Dipendra
Dipendra Khadka Sept. 30, 2024, 6:12 p.m. UTC | #8
Hi ,

On Thu, 26 Sept 2024 at 11:27, Dipendra Khadka <kdipendra88@gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, 25 Sept 2024 at 12:07, Dipendra Khadka <kdipendra88@gmail.com> wrote:
> >
> > On Wed, 25 Sept 2024 at 00:00, Simon Horman <horms@kernel.org> wrote:
> > >
> > > On Tue, Sep 24, 2024 at 11:42:58PM +0545, Dipendra Khadka wrote:
> > > > Hi Simon,
> > > >
> > > > On Tue, 24 Sept 2024 at 21:43, Simon Horman <horms@kernel.org> wrote:
> > > > >
> > > > > On Tue, Sep 24, 2024 at 08:39:47PM +0545, Dipendra Khadka wrote:
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Tue, 24 Sept 2024 at 12:55, Simon Horman <horms@kernel.org> wrote:
> > > > > > >
> > > > > > > On Mon, Sep 23, 2024 at 11:31:34AM +0000, Dipendra Khadka wrote:
> > > > > > > > Add error pointer check after calling otx2_mbox_get_rsp().
> > > > > > > >
> > > > > > >
> > > > > > > Hi Dipendra,
> > > > > > >
> > > > > > > Please add a fixes tag here (no blank line between it and your
> > > > > > > Signed-off-by line).
> > > > > > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > > > > >
> > > > > > > As you have posted more than one patch for this driver, with very similar,
> > > > > > > not overly complex or verbose changes, it might make sense to combine them
> > > > > > > into a single patch. Or, if not, to bundle them up into a patch-set with a
> > > > > > > cover letter.
> > > > > > >
> > > > > > > Regarding the patch subject, looking at git history, I think
> > > > > > > an appropriate prefix would be 'octeontx2-pf:'. I would go for
> > > > > > > something like this:
> > > > > > >
> > > > > > >   Subject: [PATCH net v2] octeontx2-pf: handle otx2_mbox_get_rsp errors
> > > > > > >
> > > > > >
> > > > > > If I bundle all the patches for the
> > > > > > drivers/net/ethernet/marvell/octeontx2/ , will this subject without v2
> > > > > > work? Or do I need to change anything? I don't know how to send the
> > > > > > patch-set with the cover letter.
> > > > >
> > > > > Given that one of the patches is already at v2, probably v3 is best.
> > > > >
> > > > > If you use b4, it should send a cover letter if the series has more than 1
> > > > > patch.  You can use various options to b4 prep to set the prefix
> > > > > (net-next), version, and edit the cover (letter).  And you can use various
> > > > > options to b4 send, such as -d, to test your submission before sending it
> > > > > to the netdev ML.
> > > > >
> > > >
> > > > I did not get this -d and testing? testing in net-next and sending to net?
> > >
> > > I meant that b4 prep -d allows you to see the emails that would be sent
> > > without actually sending them. I find this quite useful myself.
> > >
> > > >
> > > > > Alternatively the following command will output 3 files: a cover letter and
> > > > > a file for each of two patches, with v3 and net-next in the subject of each
> > > > > file. You can edit these files and send them using git send-email.
> > > > >
> > > > > git format-patch --cover-letter -2 -v3 --subject-prefix="PATCH net-next"
> > > > >
>
> Do I need to maintain patch history below  Signed-off-by for each
> patch when I send them in the patch set? If so, what to do with those
> which have v1 but no v2 but the patch-set in v3?
>
> > > >
> > > > Should I send it to net-next or net?
> > >
> > > Sorry for the confusion. I wrote net-next in my example,
> > > but I think this patch-set would be for net.
> > >
> > > ...
> >
> > Thank you Simon for everything.
> >
> > Best regards,
> > Dipendra
>
> Best regards,
> Dipendra

Are we accepting any changes related to the error pointer handling for
the driver octeontx2?

Best Regards,
Dipendra Khadka
Simon Horman Oct. 1, 2024, 1:42 p.m. UTC | #9
On Mon, Sep 30, 2024 at 11:57:02PM +0545, Dipendra Khadka wrote:

...

> Are we accepting any changes related to the error pointer handling for
> the driver octeontx2?

Sorry, I think I'm missing some context.
Could you explain in a bit more detail?
Dipendra Khadka Oct. 1, 2024, 3:11 p.m. UTC | #10
Hi Simon,

On Tue, 1 Oct 2024 at 19:27, Simon Horman <horms@kernel.org> wrote:
>
> On Mon, Sep 30, 2024 at 11:57:02PM +0545, Dipendra Khadka wrote:
>
> ...
>
> > Are we accepting any changes related to the error pointer handling for
> > the driver octeontx2?
>
> Sorry, I think I'm missing some context.
> Could you explain in a bit more detail?

We did not accept the patch where Vladimir replied. So , I thought if
there is not anything like that there, then only I will send a
patch-set.
Hence, I asked this question.

Best regards,
Dipendra Khadka
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index 0db62eb0dab3..36a08303752f 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -343,6 +343,12 @@  static void otx2_get_pauseparam(struct net_device *netdev,
 	if (!otx2_sync_mbox_msg(&pfvf->mbox)) {
 		rsp = (struct cgx_pause_frm_cfg *)
 		       otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr);
+
+		if (IS_ERR(rsp)) {
+			mutex_unlock(&pfvf->mbox.lock);
+			return;
+		}
+
 		pause->rx_pause = rsp->rx_pause;
 		pause->tx_pause = rsp->tx_pause;
 	}
@@ -1074,6 +1080,12 @@  static int otx2_set_fecparam(struct net_device *netdev,
 
 	rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox,
 						   0, &req->hdr);
+
+	if (IS_ERR(rsp)) {
+		err = PTR_ERR(rsp);
+		goto end;
+	}
+
 	if (rsp->fec >= 0)
 		pfvf->linfo.fec = rsp->fec;
 	else