On 9/18/2024 15:33, Andy Shevchenko wrote: > On Wed, Sep 18, 2024 at 12:01:19AM +0530, Shyam Sundar S K wrote: >> On 9/14/2024 00:49, Andy Shevchenko wrote: >>> On Fri, Sep 13, 2024 at 05:41:09PM +0530, Shyam Sundar S K wrote: > > ... > >>>> + eoi_addr = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> + if (!eoi_addr) >>>> + return dev_err_probe(&pdev->dev, -EINVAL, "missing MEM resources\n"); >>>> + >>>> + asf_dev->eoi_base = devm_ioremap(&pdev->dev, eoi_addr->start, resource_size(eoi_addr)); >>>> + if (!asf_dev->eoi_base) >>>> + return dev_err_probe(&pdev->dev, -EBUSY, "failed mapping IO region\n"); >>> >>> Home grown copy of devm_platform_ioremap_resource(). >> >> devm_platform_ioremap_resource() internally calls >> devm_platform_get_and_ioremap_resource(), performing two main actions: >> >> It uses platform_get_resource(). >> It then calls devm_ioremap_resource(). >> >> However, there's an issue. >> >> devm_ioremap_resource() invokes devm_request_mem_region() followed by >> __devm_ioremap(). In this driver, the resource obtained via ASL might >> not actually belong to the ASF device address space. Instead, it could >> be within other IP blocks of the ASIC, which are crucial for >> generating subsequent interrupts (the main focus of this patch). As a >> result, devm_request_mem_region() fails, preventing __devm_ioremap() >> from being executed. >> >> TL;DR, it’s more appropriate to call platform_get_resource() and >> devm_ioremap() separately in this scenario. > > Okay, at bare minimum this must be commented in the code (like the above > summary). Okay, I will leave a comment. > Ideally it should be done differently as the resource regions > should not be shared (it's an exceptional case and usually shows bad design > of the driver). If you can't really split, regmap APIs help with that > (and they also may provide an adequate serialisation to IO). > Unfortunately, this is the only way to get subsequent interrupts from the ASF IP block (based on the AMD ASF databook). Thanks, Shyam
