> -----Original Message----- > From: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> > Sent: Friday, December 20, 2024 5:32 PM > To: Carlos Song <carlos.song@xxxxxxx>; Oleksij Rempel > <o.rempel@xxxxxxxxxxxxxx> > Cc: Andi Shyti <andi.shyti@xxxxxxxxxx>; Frank Li <frank.li@xxxxxxx>; > kernel@xxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > festevam@xxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Clark Wang > <xiaoning.wang@xxxxxxx> > Subject: [EXT] Re: [PATCH v5] i2c: imx: support DMA defer probing > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > Hi Carlos, > > On 20.12.24 10:23, Carlos Song wrote: > >>> Hi, the ret is from i2c_imx_dma_request() and look like that ret has > >>> been converted by PTR_ERR, So the ret error has been human readable > >> version? > >> > >> I am not sure I understand the question. ERR_PTR() makes an error > >> pointer and %pe formats that pointer as error message. So you don't > >> need to change any function return types unless needed, just at the > >> end print it with %pe instead of %d (and after error pointer conversion if > needed). > > > > Sorry, I don't know if I understand it incorrectly. > > I review other driver code, most choose to return error value but not an error > pointer. > > Shouldn't error value be more readable than error pointers? > > When we see -110 we know TIMEOUT and we see -12 we know NO MEM. > > I know -110, but -12 I need to look up :) Both are cryptic to end users, which is > why %pe was added on top of the existing %p: > > If CONFIG_SYMBOLIC_ERRNAME is enabled %pe expands to an error string, e.g. > "ENOMEM" or "ETIMEDOUT". If it's disabled, you get the same error number > that was printed raw before. > > Cheers, > Ahmad > Wow! Looks so cool. Thank you very much for your patient explanation! I agree it. Also I will change the comment from your suggestion[1]: " /* * As we can always fall back to PIO, let's ignore the error setting up * DMA and see if we run into errors while setting up PIO mode. */ " In fact, other errors are also from DMA setting not from setting PIO mode. So can I comment simply like this? /* As we can always fall back to PIO, let's ignore the error setting up DMA. */ ret = i2c_imx_dma_request(i2c_imx, phy_addr); if (ret) { if (ret == -EPROBE_DEFER) goto clk_notifier_unregister; else if (ret == -ENODEV) dev_dbg(&pdev->dev, "Only use PIO mode\n"); else dev_err(&pdev->dev, "Failed to setup DMA (%pe), only use PIO mode\n", ERR_PTR(ret)); } [1]https://lore.kernel.org/imx/89a3b1c9-2be2-4e7f-a0c6-abbf8b88957b@xxxxxxxxxxxxxx/ BR Carlos > > > > i2c_imx_dma_request is using PTR_ERR to convert pointer to error value[1]. > > I don't know why need to use ERR_PTR to reconvert the value to pointer: > > > > dev_err(&pdev->dev, "Failed to setup DMA (%pe), only use PIO mode\n", > > ERR_PTR(ret)); > > > > Is there some strong reason? > > > > [1] > > https://lore/ > > .kernel.org%2Fimx%2FAM0PR0402MB3937419BBB58B75FB8F8DE2DE8072% > 40AM0PR04 > > > 02MB3937.eurprd04.prod.outlook.com%2F&data=05%7C02%7Ccarlos.song%4 > 0nxp > > .com%7C13a8e6532dd7435c302008dd20d92819%7C686ea1d3bc2b4c6fa92c > d99c5c30 > > > 1635%7C0%7C0%7C638702839227618754%7CUnknown%7CTWFpbGZsb3d8eyJ > FbXB0eU1h > > > cGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIj > > > oyfQ%3D%3D%7C0%7C%7C%7C&sdata=2A6qPlFEuW1SmlvOZ7OhtfO7VwqysM > cGhFL2G%2F > > ECNek%3D&reserved=0 > >> > >> -- > >> Pengutronix e.K. | > >> | > >> Steuerwalder Str. 21 | > >> http://www/. > >> > pen%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C13a8e6532dd7435c30 > 2008d > >> > d20d92819%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63870283 > 922763 > >> > 8077%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjA > uMDAw > >> > MCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&s > da > >> > ta=LasLtpe7GjsG5qVUKG%2FOmVro2JueGLfwUPALw7%2FB2fg%3D&reserved= > 0 > >> > gutronix.de%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C594497db1b5 > >> > 44e479a8f08dd20d1e88e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > >> %7C638702808104903131%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hc > Gki > >> > OnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyf > >> > Q %3D%3D%7C0%7C%7C%7C&sdata=EnhsIFlBooqjB%2FSRWF7uAqRHE3yN6rbd > D > >> 1yQueTrRus%3D&reserved=0 | > >> 31137 Hildesheim, Germany | Phone: > +49-5121-206917-0 > >> | > >> Amtsgericht Hildesheim, HRA 2686 | Fax: > >> +49-5121-206917-5555 | > > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > http://www.pen/ > gutronix.de%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C13a8e6532dd > 7435c302008dd20d92819%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > %7C638702839227652725%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGki > OnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ > %3D%3D%7C0%7C%7C%7C&sdata=Hm60QL5dJ2h3OCesOXK871ASnK7s0L4SPiY > bK0jtSOo%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |