在 2023/3/24 上午6:33, Ilkka Koskinen 写道:
>
> Hi Jing,
>
> On Thu, 23 Mar 2023, Jing Zhang wrote:
>> ACPI companion devices call insert_resource() in platform_device_add()
>> to claim the device resources. If the resources are claimed again before
>> ioremap(), and the addresses of multiple resources overlap, it will
>> return -BUSY, causing the driver to fail to load.
>>
>> For example, the CMN700 on my machine is set with two resources similar
>> to CMN600, and the overlap of resource addresses makes the CMN driver
>> unable to match my CMN700. The error log:
>
> I thought that the two address ranges were only needed for CMN-600?
> I guess, the specification doesn't explicitly forbid giving two ranges for CMN700 although the example shows just one.
>
Hi Ilkka,
Yes, on our platform, CMN700 is designed with two resources which describe PERIPHBASE and ROOTNODEBASE respectively.
>>
>> [ 12.016837] arm-cmn ARMHC700:00: can't request region for resource [mem 0x40000000-0x4fffffff]
>> [ 12.028230] arm-cmn: probe of ARMHC700:00 failed with error -16
>> [ 12.036832] arm-cmn ARMHC700:01: can't request region for resource [mem 0x40040000000-0x4004fffffff]
>> [ 12.051289] arm-cmn: probe of ARMHC700:01 failed with error -16
>>
>> So let ACPI companion devices call arm_cmn_acpi_probe() and not claim
>> resource again. In addition, the arm_cmn_acpi_probe() and
>> arm_cmn_of_probe() functions are refactored to make them compatible
>> with both CMN600 and CMN-ANY.
>>
>> Fixes: 61ec1d875812 ("perf/arm-cmn: Demarcate CMN-600 specifics")
>> Signed-off-by: Jing Zhang <renyu.zj@xxxxxxxxxxxxxxxxx>
>> ---
>> drivers/perf/arm-cmn.c | 57 ++++++++++++++++++++++++++++++++------------------
>> 1 file changed, 37 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index 1deb61b..beb3b37 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c
>> @@ -2206,7 +2206,7 @@ static int arm_cmn_discover(struct arm_cmn *cmn, unsigned int rgn_offset)
>> return 0;
>> }
>>
>> -static int arm_cmn600_acpi_probe(struct platform_device *pdev, struct arm_cmn *cmn)
>> +static int arm_cmn_acpi_probe(struct platform_device *pdev, struct arm_cmn *cmn)
>> {
>> struct resource *cfg, *root;
>>
>> @@ -2214,12 +2214,21 @@ static int arm_cmn600_acpi_probe(struct platform_device *pdev, struct arm_cmn *c
>> if (!cfg)
>> return -EINVAL;
>>
>> - root = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> - if (!root)
>> - return -EINVAL;
>> + /* If ACPI defines more than one resource, such as cmn-600, then there may be
>> + * a deviation between ROOTNODEBASE and PERIPHBASE, and ROOTNODEBASE can
>> + * be obtained from the second resource. Otherwise, it can be considered that
>> + * ROOT NODE BASE is PERIPHBASE. This is compatible with cmn-600 and cmn-any.
>> + */
>> + if (pdev->num_resources > 1) {
>> + root = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + if (!root)
>> + return -EINVAL;
>>
>> - if (!resource_contains(cfg, root))
>> - swap(cfg, root);
>> + if (!resource_contains(cfg, root))
>> + swap(cfg, root);
>> + } else {
>> + root = cfg;
>> + }
>> /*
>> * Note that devm_ioremap_resource() is dumb and won't let the platform
>> * device claim cfg when the ACPI companion device has already claimed
>> @@ -2227,17 +2236,30 @@ static int arm_cmn600_acpi_probe(struct platform_device *pdev, struct arm_cmn *c
>> * appropriate name, we don't really need to do it again here anyway.
>> */
>> cmn->base = devm_ioremap(cmn->dev, cfg->start, resource_size(cfg));
>> - if (!cmn->base)
>> - return -ENOMEM;
>> + if (IS_ERR(cmn->base))
>> + return PTR_ERR(cmn->base);
>
> I believe, devm_ioremap() returns NULL in case of error
>
Yes, you are right.
Thanks,
Jing
