Re: [PATCH v5 7/8] i2c: amd-asf: Clear remote IRR bit to get successive interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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). 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).

-- 
With Best Regards,
Andy Shevchenko






[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux