[bug report] powerpc/powernv: Support EEH reset for VF PE

Hello Wei Yang,

The patch 9312bc5bab59: "powerpc/powernv: Support EEH reset for VF
PE" from Mar 4, 2016, leads to the following static checker warning:

	arch/powerpc/platforms/powernv/eeh-powernv.c:1033 pnv_eeh_reset_vf_pe()
	info: return a literal instead of 'ret'

arch/powerpc/platforms/powernv/eeh-powernv.c
  1019  static int pnv_eeh_reset_vf_pe(struct eeh_pe *pe, int option)
  1020  {
  1021          struct eeh_dev *edev;
  1022          struct pci_dn *pdn;
  1023          int ret;
  1024  
  1025          /* The VF PE should have only one child device */
  1026          edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev, list);
  1027          pdn = eeh_dev_to_pdn(edev);
  1028          if (!pdn)
  1029                  return -ENXIO;
  1030  
  1031          ret = pnv_eeh_do_flr(pdn, option);
  1032          if (!ret)
  1033                  return ret;

Presumably if pnv_eeh_do_flr() succeeds then we don't need to continue?
pnv_eeh_do_af_flr() is a fall back option?  Doing a:

			return 0;

Makes this more deliberate looking.  Sometimes people get their if
statements reversed is the other option.

  1034  
  1035          return pnv_eeh_do_af_flr(pdn, option);
  1036  }

regards,
dan carpenter

On Tue, 2017-02-14 at 16:39 +0300, Dan Carpenter wrote:

Hello Wei Yang,

The patch 9312bc5bab59: "powerpc/powernv: Support EEH reset for VF
PE" from Mar 4, 2016, leads to the following static checker warning:

	arch/powerpc/platforms/powernv/eeh-powernv.c:1033 pnv_eeh_reset_vf_pe()
	info: return a literal instead of 'ret'

Hey Dan,

Out of curiosity, what static analysis tool are you using?

arch/powerpc/platforms/powernv/eeh-powernv.c
  1019  static int pnv_eeh_reset_vf_pe(struct eeh_pe *pe, int option)
  1020  {
  1021          struct eeh_dev *edev;
  1022          struct pci_dn *pdn;
  1023          int ret;
  1024  
  1025          /* The VF PE should have only one child device */
  1026          edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev,
list);
  1027          pdn = eeh_dev_to_pdn(edev);
  1028          if (!pdn)
  1029                  return -ENXIO;
  1030  
  1031          ret = pnv_eeh_do_flr(pdn, option);
  1032          if (!ret)
  1033                  return ret;

Presumably if pnv_eeh_do_flr() succeeds then we don't need to continue?
pnv_eeh_do_af_flr() is a fall back option?  Doing a:

			return 0;

Makes this more deliberate looking.  Sometimes people get their if
statements reversed is the other option.

I can send a patch for this.

  1034  
  1035          return pnv_eeh_do_af_flr(pdn, option);
  1036  }

regards,
dan carpenter