Skip to contentSkip to conversation

ata: ahci: ceva: Update the driver to support xilinx GT phy

bug report Dan Carpenter 1 participant 1 message May 3, 2021 libata

lore
Share & tools
lore
#0 lore raw

Hello Piyush Mehta,

The patch 9a9d3abe24bb: "ata: ahci: ceva: Update the driver to support xilinx GT phy" from Feb 8, 2021, leads to the following static checker warning:

	drivers/ata/ahci_ceva.c:213 ceva_ahci_probe()
	warn: pointer error 'PTR_ERR(cevapriv->rst)' not handled

drivers/ata/ahci_ceva.c
192 static int ceva_ahci_probe(struct platform_device *pdev) 193 {

   194          struct device_node *np = pdev->dev.of_node;
   195          struct device *dev = &pdev->dev;
   196          struct ahci_host_priv *hpriv;
   197          struct ceva_ahci_priv *cevapriv;
   198          enum dev_dma_attr attr;
   199          int rc;

200
201 cevapriv = devm_kzalloc(dev, sizeof(*cevapriv), GFP_KERNEL);
202 if (!cevapriv)
203 return -ENOMEM;
204
205 cevapriv->ahci_pdev = pdev;
206
207 cevapriv->rst = devm_reset_control_get_optional_exclusive(&pdev->dev,
208 NULL);
209 if (IS_ERR(cevapriv->rst))
210 dev_err_probe(&pdev->dev, PTR_ERR(cevapriv->rst),
211 "failed to get reset\n");

The function just continues if "cevapriv->rst" is an error pointer. We should probably return an error, because the user asked us to do something and we failed. Another option is to set it to NULL.

   212  
   213          hpriv = ahci_platform_get_resources(pdev, 0);
   214          if (IS_ERR(hpriv))
   215                  return PTR_ERR(hpriv);
   216  
   217          if (!cevapriv->rst) {

It's an error pointer so it's not NULL

218 rc = ahci_platform_enable_resources(hpriv);
219 if (rc)
220 return rc;
221 } else {
222 int i;
223
224 rc = ahci_platform_enable_clks(hpriv);
225 if (rc)
226 return rc;
227 /* Assert the controller reset */

   228                  reset_control_assert(cevapriv->rst);
                                             ^^^^^^^^^^^^^

This will trigger a WARN_ON().

   229  
   230                  for (i = 0; i < hpriv->nports; i++) {
   231                          rc = phy_init(hpriv->phys[i]);

regards,
dan carpenter