Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5f3fb325-cecd-5a98-656d-c4fbe5e9e03b@vivo.com>
Date:   Fri, 7 Jul 2023 15:39:13 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH 3/4] PCI: v3-semi: Use
 devm_platform_get_and_ioremap_resource()
To:     =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= <kw@linux.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Rob Herring <robh@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, Yang Li <yang.lee@linux.alibaba.com>
References: <20230706092047.18599-1-frank.li@vivo.com>
 <20230706092047.18599-3-frank.li@vivo.com>
 <20230706185511.GA3380331@rocinante>
From:   Yangtao Li <frank.li@vivo.com>
In-Reply-To: <20230706185511.GA3380331@rocinante>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?b2EyTnhrVzcvRzQwd2pFR3ZhMytPbzk5MTdaaHB6eG1KZU92NG5FaEk5ZHVr?=
 =?utf-8?B?Y0d3QlNDMjVDVUlPMjVmT1JaWHBtQURqL1VHaGdPbS9BUTVSeDJnQlhOYlNw?=
 =?utf-8?B?WWplSUU0K2hRNGxUYy95WEgyRDFZNjdJRTQ0U0FnMmNteGc3QStlMXlPaTFm?=
 =?utf-8?B?ZDh5WXl6QS8vUkg3NHB3OFNjMTNMWDB3V1NyYVBEeWU5ZWRSOVZRUHlJa2xn?=
 =?utf-8?B?VEFhb3FCSE5LZys5aEJ6YnNCdGhra3ZQN2gwNzhmSzV1TG5CL0NTVnQ3d3VJ?=
 =?utf-8?B?VGs0NWlrbkJTNFZ1SkdnQ3hYb2NIOElxaTBMQXJYR2k3TTllVkdzRFdMTEJZ?=
 =?utf-8?B?cmFpcSt1RUlmVTdYc3JFd216U09ZcVlBdy95NlpNRWMwcFdITnhnK2NoamZw?=
 =?utf-8?B?Yis2TGNXd3lLWDZTRkFrVmxoV0c5dTJZbi9tN01kZS9pQWdTV2ZXcnpZVDlE?=
 =?utf-8?B?ZVcxQ1NiY3BodWV4UVRoWUhYSEVrTXVwQmM0MlJ6N0VudnVSZ053Tk4vditk?=
 =?utf-8?B?QWJkalJSOU9mOWFwL3Vra0pYVitGOGVyZGdNKzJaUWJCVmMyRlo5eWN6elNW?=
 =?utf-8?B?RG5McVJsZFdXR2dBRU9VNVp6QlgveUJ0aERCaGNNa0dtRy85Mktia3dhNVdt?=
 =?utf-8?B?ZE05d1Q5L0JIemdpaUVLa3lYTFZCajd5VSt3dDFJeUlDV0V5YWVCVFp5YkQ1?=
 =?utf-8?B?cm1DV3BXUk1ERHN1aXo1Sm9hSGpmMUk3UU9WUXJWQ1F2ejNyWTV2WlhLWmxQ?=
 =?utf-8?B?dERLN2dnM0ZMZERTU2VkTnI2K1puZWQwZGFuU2JhRjhkVGdZS0NmdVgvbG8y?=
 =?utf-8?B?Mm5YTGxKNHlZSzZCMkZqdnhIZEYrNHR4V3pIZmpnMmQvTUFxV0tZU2ZVTWdp?=
 =?utf-8?B?eUxoZ3dqOTBQeEFIa1M4Vy9iVnF5NHAyaGVGdm02aVk5U3Q5VW81VmRLeWNw?=
 =?utf-8?B?bjI4Q29rTTBhOEV5OGVkcXNqcmM4M3l5ajMxQzhqMUlmTHNRM1cyaU11MnNI?=
 =?utf-8?B?WkV4RDB1ZFFaWmx6OGk0aXMxZm5sWUVvbWtXTkUyUTZwbGNESjdST2w0OEk4?=
 =?utf-8?B?eFFVaVlsYzJhYnI5Y3VHcjlTaEtVVmFlUjE0WXZQaHkwdDhlU29yY0VMUHR6?=
 =?utf-8?B?a1E0MFdsOFJYTFd4T0RlZ1ZYZ3FocGIzTkh6SmdQN0pnZ3ZwOUs4cEozdzNa?=
 =?utf-8?B?OEJLWUF0cGM4SjY5TXh4SzZPUW1ja0gyclhxNENGWjVJMFhic2I0eEE5RmJp?=
 =?utf-8?B?bytGb3R1TXRhM1RZNWRIMXRFV0VWN3R2SGhEYkljQ0wvYWJSWVAvM0V6b1Ju?=
 =?utf-8?B?VHdOT2h2TnhpUGE0MmUwdGlSSXZnbTJaOHRwMmE5cGQ0ZkJ6VDFEMHBRNzVp?=
 =?utf-8?B?NEZMQTB0aktrLzRETldwMVI5OCtJVUZLZStKOGFKVloyR2oxWitjS3RMTjJm?=
 =?utf-8?B?VmVaUTBXcVhQUmk4QUFEUkVYdHdYa3E5cVZlaXBFUnpvQkE5Q2pyVUZ2R0tQ?=
 =?utf-8?B?SjFkTG5FWG5oUzBOS00yTUxLbTdnVlhtdXU4T0NSZis3VE5HUDdZQjYyUkNs?=
 =?utf-8?B?N2JJYnJPZzE1dHNnMGc4cTcwYitZbnFTSUc5TzFMUGx4eUozM1d4b09EdGJN?=
 =?utf-8?B?eW9aSnEray9JdGVHRzhTTVoxU0l4REJJOUpQd0tudmx3SEZjcTArM1RXRldm?=
 =?utf-8?B?MmZrQVpjSkJ1NHBOellyRUhZWDU0QVhDNkdkc1ZaYmVhRkM1V0w5dHpPcVJz?=
 =?utf-8?B?VllhdXVGcDc3d01IRW8rOTlqajYzMlF5dlZSdXNOVkFqS0dxbmZud0NlUlI1?=
 =?utf-8?B?eEZKbzdzbFdFbElWQ3Q5b1ExUDFYQUd4ZXU2ZFVPL1F2aGkwWitONmNFZ1dN?=
 =?utf-8?B?SU83bFBjaTR6aTlOWHNiUWNTQXdVSUFMNHh2L24rQnUvVmI2dGcwd29OTzJ3?=
 =?utf-8?B?aFVnTVJXWCswYkhqRE9uYTZ1ejF0Z2dzQkoyVi9yWmoyT1ArakI2Nm4rZ3Fv?=
 =?utf-8?B?cFp3M2YwdlNsQXY3WDBuYXl6N2VZTG5YZnZwK2Q5T3l2aXVLa0ZOOXlZUTNV?=
 =?utf-8?B?eGd0OUUrRmJpYjV3dGI0bUdGa1hGajh6WCtoWmVKck1xVVY3OVJ6aC9hVER2?=
 =?utf-8?Q?k+NgrKBYrC0DIlEdP3GVFK4pN?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a31b5625-7f5e-4f5c-3a8c-08db7ebd4595
X-MS-Exchange-CrossTenant-AuthSource: SEZPR06MB5269.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jul 2023 07:39:18.6806
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: QppljdE5mc2zjaUb2cXUrtn/DgmXPDSCNEl5Y9mF8FZOAq5ObBhO93kzo1ln45UPDduOitTiHOIDvAXztAKv2g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: KL1PR0601MB4228
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2023/7/7 2:55, Krzysztof Wilczyński wrote:

> [+Cc Yang]
>
> Hello,
>
>> Convert platform_get_resource(), devm_ioremap_resource() to a single
>> call to devm_platform_get_and_ioremap_resource(), as this is exactly
>> what this function does.
> A very similar series was posted a while ago.  Patches 1, 2 and 4 in this
> series are identical, except for this patch, which adds one more very nice
> refactoring,
>
> The other series:
>
>    https://lore.kernel.org/linux-pci/20230626110226.GA569947@rocinante
>
> What I can do would be to pick this patch and squash it with the other ones
> that are already applied.  I will then attribute both you and Yang Li as
> the authors.  Let me know if this is fine with you, Yangtao and Yang.


Acked-by: Yangtao Li <frank.li@vivo.com>

Thx,
Yangtao


>
> That said...
>
>> diff --git a/drivers/pci/controller/pci-v3-semi.c b/drivers/pci/controller/pci-v3-semi.c
>> index ca44b0c83d1b..0f993328dba2 100644
>> --- a/drivers/pci/controller/pci-v3-semi.c
>> +++ b/drivers/pci/controller/pci-v3-semi.c
>> @@ -736,8 +736,7 @@ static int v3_pci_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> -	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	v3->base = devm_ioremap_resource(dev, regs);
>> +	v3->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> Are you sure about the variable name here? This might be a small copy-and-paste
> mistake that snuck up here.  Have a closer look, or better yet, try to compile
> and test this. :)
>
>> -	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	v3->config_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
> Same as above.
>
>> +	if (IS_ERR(v3->config_base))
>> +		return PTR_ERR(v3->config_base);
>>   	if (resource_size(regs) != SZ_16M) {
>>   		dev_err(dev, "config mem is not 16MB!\n");
>>   		return -EINVAL;
>>   	}
>>   	v3->config_mem = regs->start;
>> -	v3->config_base = devm_ioremap_resource(dev, regs);
>> -	if (IS_ERR(v3->config_base))
>> -		return PTR_ERR(v3->config_base);
> This could be made such that the assignments are together.
>
> 	Krzysztof