On 6/13/22 03:37, Sergei Shtylyov wrote: > On 6/12/22 3:57 PM, Li Qiong wrote: > >> As the possible failure of the devm_ioremap(), the return value >> could be NULL. Therefore it should be better to check it and >> print error message, return '-ENOMEM' error code. This error is very unlikely. So unless you are seeing actual problems in the field, I do not think it is worth fixing. >> >> Signed-off-by: Li Qiong <liqiong@xxxxxxxxxxxx> >> Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx> >> --- >> v2: >> - add driver's name (pata_pxa) to subject. >> --- >> drivers/ata/pata_pxa.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/ata/pata_pxa.c b/drivers/ata/pata_pxa.c >> index 985f42c4fd70..cd1a8f37f920 100644 >> --- a/drivers/ata/pata_pxa.c >> +++ b/drivers/ata/pata_pxa.c >> @@ -228,6 +228,11 @@ static int pxa_ata_probe(struct platform_device *pdev) >> ap->ioaddr.bmdma_addr = devm_ioremap(&pdev->dev, dma_res->start, >> resource_size(dma_res)); > > Looking again into this driver, this statement doesn't make sense: dma_res > points to a DMA resource, calling devm_ioremap() on it is just wrong... and Yes, having to do an ioremap of an IORESOURCE_DMA resource is rather unusual. dmaengine_slave_config() should be doing anything that is required for that resource. > 'ap->ioaddr.bmdma_addr' doesn;t seem to be used anyways... It is used in lbata-sff.c. A much cleaner fix would be to use devm_platform_get_and_ioremap_resource() or devm_platform_ioremap_resource() which will also remove the call to platform_get_resource((). But as mentioned above, unless this is fixing an actual bug in production, I do not think this is worth it. > > MBR, Sergey -- Damien Le Moal Western Digital Research
