Re: [PATCH v18 17/20] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Tue, Aug 01, 2023 at 04:06:32AM +0300, Serge Semin wrote:
> On Mon, Jul 24, 2023 at 05:58:20PM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Jul 21, 2023 at 04:44:49PM +0900, Yoshihiro Shimoda wrote:
> > > Add R-Car Gen4 PCIe Host support. This controller is based on
> > > Synopsys DesignWare PCIe, but this controller has vendor-specific
> > > registers so that requires initialization code like mode setting
> > > and retraining and so on.
> > > 
> > > To reduce code delta, adds some helper functions which are used by
> > > both the host driver and the endpoint driver (which is added
> > > immediately afterwards) into a separate file.
> > > 
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > >  drivers/pci/controller/dwc/Kconfig            |   9 +
> > >  drivers/pci/controller/dwc/Makefile           |   2 +
> > >  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 149 +++++++++++++
> > >  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 200 ++++++++++++++++++
> > >  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  44 ++++
> > >  5 files changed, 404 insertions(+)
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > > 

[...]

> > 
> > > +		return err;
> > > +	}
> > > +
> > > +	err = rcar_gen4_pcie_prepare(rcar);
> > > +	if (err < 0)
> > > +		return err;
> > > +
> 
> > > +	err = rcar_gen4_add_dw_pcie_rp(rcar);
> > > +	if (err < 0)
> > > +		goto err_add;
> > > +
> > > +	return 0;
> > > +
> > > +err_add:
> > 
> > err_prepare
> 
> IMO either "err_unprepare" or "err_add_rp". First option seems better
> since unlike the second version it would look correct in case of
> having multiple gotos to the same label.
> 
> "err_prepare" doesn't indicate neither the target code nor the source
> of the jump. So the name doesn't sound descriptive if not to say
> misleading.
> 

I just blindly went with the function name. Yes, "err_unprepare" would be the
correct label name.

- Mani

-- 
மணிவண்ணன் சதாசிவம்