Re: [PATCH] PCI: imx6: Clean up comments and whitespace

[Frank Li <Frank.li@nxp.com>]

On Sat, Jan 18, 2025 at 03:07:27PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> For readability, fix typos and comments that needlessly exceed 80 columns.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 41 ++++++++++++++-------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 06d22f23c6b3..d70e6c427976 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -236,11 +236,11 @@ static void imx_pcie_configure_type(struct imx_pcie *imx_pcie)
>
>  	id = imx_pcie->controller_id;
>
> -	/* If mode_mask is 0, then generic PHY driver is used to set the mode */
> +	/* If mode_mask is 0, generic PHY driver is used to set the mode */
>  	if (!drvdata->mode_mask[0])
>  		return;
>
> -	/* If mode_mask[id] is zero, means each controller have its individual gpr */
> +	/* If mode_mask[id] is 0, each controller has its individual GPR */
>  	if (!drvdata->mode_mask[id])
>  		id = 0;
>
> @@ -377,14 +377,15 @@ static int pcie_phy_write(struct imx_pcie *imx_pcie, int addr, u16 data)
>
>  static int imx8mq_pcie_init_phy(struct imx_pcie *imx_pcie)
>  {
> -	/* TODO: Currently this code assumes external oscillator is being used */
> +	/* TODO: This code assumes external oscillator is being used */
>  	regmap_update_bits(imx_pcie->iomuxc_gpr,
>  			   imx_pcie_grp_offset(imx_pcie),
>  			   IMX8MQ_GPR_PCIE_REF_USE_PAD,
>  			   IMX8MQ_GPR_PCIE_REF_USE_PAD);
>  	/*
> -	 * Regarding the datasheet, the PCIE_VPH is suggested to be 1.8V. If the PCIE_VPH is
> -	 * supplied by 3.3V, the VREG_BYPASS should be cleared to zero.
> +	 * Per the datasheet, the PCIE_VPH is suggested to be 1.8V.  If the
> +	 * PCIE_VPH is supplied by 3.3V, the VREG_BYPASS should be cleared
> +	 * to zero.
>  	 */
>  	if (imx_pcie->vph && regulator_get_voltage(imx_pcie->vph) > 3000000)
>  		regmap_update_bits(imx_pcie->iomuxc_gpr,
> @@ -571,7 +572,7 @@ static int imx_pcie_attach_pd(struct device *dev)
>  			DL_FLAG_PM_RUNTIME |
>  			DL_FLAG_RPM_ACTIVE);
>  	if (!link) {
> -		dev_err(dev, "Failed to add device_link to pcie pd.\n");
> +		dev_err(dev, "Failed to add device_link to pcie pd\n");
>  		return -EINVAL;
>  	}
>
> @@ -584,7 +585,7 @@ static int imx_pcie_attach_pd(struct device *dev)
>  			DL_FLAG_PM_RUNTIME |
>  			DL_FLAG_RPM_ACTIVE);
>  	if (!link) {
> -		dev_err(dev, "Failed to add device_link to pcie_phy pd.\n");
> +		dev_err(dev, "Failed to add device_link to pcie_phy pd\n");
>  		return -EINVAL;
>  	}
>
> @@ -605,10 +606,10 @@ static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
>  		/* power up core phy and enable ref clock */
>  		regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD);
>  		/*
> -		 * the async reset input need ref clock to sync internally,
> +		 * The async reset input need ref clock to sync internally,
>  		 * when the ref clock comes after reset, internal synced
>  		 * reset time is too short, cannot meet the requirement.
> -		 * add one ~10us delay here.
> +		 * Add a ~10us delay here.
>  		 */
>  		usleep_range(10, 100);
>  		regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN);
> @@ -880,6 +881,7 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>
>  		if (imx_pcie->drvdata->flags &
>  		    IMX_PCIE_FLAG_IMX_SPEED_CHANGE) {
> +
>  			/*
>  			 * On i.MX7, DIRECT_SPEED_CHANGE behaves differently
>  			 * from i.MX6 family when no link speed transition
> @@ -888,7 +890,6 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>  			 * which will cause the following code to report false
>  			 * failure.
>  			 */
> -
>  			ret = imx_pcie_wait_for_speed_change(imx_pcie);
>  			if (ret) {
>  				dev_err(dev, "Failed to bring link up!\n");
> @@ -1091,15 +1092,16 @@ static const struct pci_epc_features imx8q_pcie_epc_features = {
>  };
>
>  /*
> - * BAR#	| Default BAR enable	| Default BAR Type	| Default BAR Size	| BAR Sizing Scheme
> - * ================================================================================================
> - * BAR0	| Enable		| 64-bit		| 1 MB			| Programmable Size
> - * BAR1	| Disable		| 32-bit		| 64 KB			| Fixed Size
> - *        BAR1 should be disabled if BAR0 is 64bit.
> - * BAR2	| Enable		| 32-bit		| 1 MB			| Programmable Size
> - * BAR3	| Enable		| 32-bit		| 64 KB			| Programmable Size
> - * BAR4	| Enable		| 32-bit		| 1M			| Programmable Size
> - * BAR5	| Enable		| 32-bit		| 64 KB			| Programmable Size
> + *     	| Default  | Default | Default | BAR Sizing
> + * BAR#	| Enable?  | Type    | Size    | Scheme
> + * =======================================================
> + * BAR0	| Enable   | 64-bit  |  1 MB   | Programmable Size
> + * BAR1	| Disable  | 32-bit  | 64 KB   | Fixed Size
> + *       (BAR1 should be disabled if BAR0 is 64-bit)
> + * BAR2	| Enable   | 32-bit  |  1 MB   | Programmable Size
> + * BAR3	| Enable   | 32-bit  | 64 KB   | Programmable Size
> + * BAR4	| Enable   | 32-bit  |  1 MB   | Programmable Size
> + * BAR5	| Enable   | 32-bit  | 64 KB   | Programmable Size
>   */
>  static const struct pci_epc_features imx95_pcie_epc_features = {
>  	.msi_capable = true,
> @@ -1260,6 +1262,7 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  		ret = imx_pcie_deassert_core_reset(imx_pcie);
>  		if (ret)
>  			return ret;
> +
>  		/*
>  		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
>  		 * root complex. This is why we have to setup the rc again and
> --
> 2.34.1
>