Re: [PATCH v3 2/4] ata: ahci_imx: Clean up code by using i.MX8Q HSIO PHY driver

On Tue, Jul 16, 2024 at 11:18:13AM +0800, Richard Zhu wrote:

Clean up code by using PHY interface.

Signed-off-by: Richard Zhu hongxing.zhu@nxp.com

drivers/ata/ahci_imx.c | 396 ++++++++++------------------------------- 1 file changed, 98 insertions(+), 298 deletions(-)

diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index cb768f66f0a70..e94c0fdea2260 100644 --- a/drivers/ata/ahci_imx.c +++ b/drivers/ata/ahci_imx.c @@ -986,65 +827,22 @@ static const struct scsi_host_template ahci_platform_sht = { static int imx8_sata_probe(struct device *dev, struct imx_ahci_priv *imxpriv) {

• struct resource *phy_res;
• struct platform_device *pdev = imxpriv->ahci_pdev;
• struct device_node *np = dev->of_node;
• if (of_property_read_u32(np, "fsl,phy-imp", &imxpriv->imped_ratio))

• imxpriv->imped_ratio = IMX8QM_SATA_PHY_IMPED_RATIO_85OHM;
  

• phy_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
• if (phy_res) {

• imxpriv->phy_base = devm_ioremap(dev, phy_res->start,
  

• 			resource_size(phy_res));
  

• if (!imxpriv->phy_base) {
  

• 	dev_err(dev, "error with ioremap\n");
  

• 	return -ENOMEM;
  

• }
  

• } else {

• dev_err(dev, "missing *phy* reg region.\n");
  

• return -ENOMEM;
  

• }
• imxpriv->gpr =

•  syscon_regmap_lookup_by_phandle(np, "hsio");
  

• if (IS_ERR(imxpriv->gpr)) {

• dev_err(dev, "unable to find gpr registers\n");
  

• return PTR_ERR(imxpriv->gpr);
  

• }
• imxpriv->epcs_tx_clk = devm_clk_get(dev, "epcs_tx");
• if (IS_ERR(imxpriv->epcs_tx_clk)) {

• dev_err(dev, "can't get epcs_tx_clk clock.\n");
  

• return PTR_ERR(imxpriv->epcs_tx_clk);
  

• }
• imxpriv->epcs_rx_clk = devm_clk_get(dev, "epcs_rx");
• if (IS_ERR(imxpriv->epcs_rx_clk)) {

• dev_err(dev, "can't get epcs_rx_clk clock.\n");
  

• return PTR_ERR(imxpriv->epcs_rx_clk);
  

• }
• imxpriv->phy_pclk0 = devm_clk_get(dev, "phy_pclk0");
• if (IS_ERR(imxpriv->phy_pclk0)) {

• dev_err(dev, "can't get phy_pclk0 clock.\n");
  

• return PTR_ERR(imxpriv->phy_pclk0);
  

• }
• imxpriv->phy_pclk1 = devm_clk_get(dev, "phy_pclk1");
• if (IS_ERR(imxpriv->phy_pclk1)) {

• dev_err(dev, "can't get phy_pclk1 clock.\n");
  

• return PTR_ERR(imxpriv->phy_pclk1);
  

• }
• imxpriv->phy_apbclk = devm_clk_get(dev, "phy_apbclk");
• if (IS_ERR(imxpriv->phy_apbclk)) {

• dev_err(dev, "can't get phy_apbclk clock.\n");
  

• return PTR_ERR(imxpriv->phy_apbclk);
  

• }
• /* Fetch GPIO, then enable the external OSC */
• imxpriv->clkreq_gpiod = devm_gpiod_get_optional(dev, "clkreq",

• 		GPIOD_OUT_LOW | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
  

• if (IS_ERR(imxpriv->clkreq_gpiod))

• return PTR_ERR(imxpriv->clkreq_gpiod);
  

• if (imxpriv->clkreq_gpiod)

• gpiod_set_consumer_name(imxpriv->clkreq_gpiod, "SATA CLKREQ");
  

• if (!(dev->bus_dma_limit))

• dev->bus_dma_limit = DMA_BIT_MASK(32);
  

These two lines look like a unrelated change, should be in a separate commit with a proper commit message.

Kind regards, Niklas