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: <6a1bde36-fbf9-9ddd-e47f-f3c21a3528f7@vivo.com>
Date:   Wed, 5 Jul 2023 17:30:01 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH 4/4] phy: mediatek: tphy: Convert to
 devm_platform_ioremap_resource()
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Chunfeng Yun <chunfeng.yun@mediatek.com>,
        Vinod Koul <vkoul@kernel.org>,
        Kishon Vijay Abraham I <kishon@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>
Cc:     linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-phy@lists.infradead.org,
        linux-kernel@vger.kernel.org
References: <20230705090126.26854-1-frank.li@vivo.com>
 <20230705090126.26854-4-frank.li@vivo.com>
 <ddc05f70-7075-97ab-1c94-cb3d5839ef2b@collabora.com>
From:   Yangtao Li <frank.li@vivo.com>
In-Reply-To: <ddc05f70-7075-97ab-1c94-cb3d5839ef2b@collabora.com>
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?cnB6K0FSUWVJb2xWTjdwL0dkOTVXc0V5ZFR5TWs2cGplT1NOR0hFUW0wRmJN?=
 =?utf-8?B?WEZlZjlYZ0Z4QVlYOC9zVTk1NUQvVG1hcU9CYjQ1SGJyeldGa3RpVXMreDk1?=
 =?utf-8?B?TjhESU9pVXpaVXZoTDA4NDlOK1FCbTJVK3FPMUhHVXJ0aTNXQi9vZ3NHbE9Z?=
 =?utf-8?B?VzRQT25sa3hRaWhzemI3azZ0M3N0azF2UE1uUC9VdEptcU1CTGRmalc2NE96?=
 =?utf-8?B?NDJhMnA2K3pORDhhd1pvRFh3Q05JTkV1U25GQWpRdXN1Y294YTJWSVpJUHFG?=
 =?utf-8?B?R1g2RmR2VGIzU2tCbm9ieG1VakZ4ekNVait0NFNvQkVVdHBSdk1yZXBjM1hr?=
 =?utf-8?B?NGtEQXFIT1BvUG1XUHRXZnFRZlEvMVZ5Zlp5YVpyRkpHeklabjVoblprM1Fp?=
 =?utf-8?B?Z1pIS1dCT2dBQ2ZwbTZpaVV2amRtWVE4aXYyQUptbTJZdFI2VHl6cXV2bFZp?=
 =?utf-8?B?L3UvZ0J3MmZSY1dxc1dDVTVZeEp5V1orTGF6ckNWcHNxeE9VTU8reERHRnR1?=
 =?utf-8?B?bStWOTFEMjBZWE5sZFpUM1IxenZpa1VHN1ZGTUp3QzJmdlpFTXgrS3RYTTdN?=
 =?utf-8?B?Q3B5UGtOMjJjNUxsTXF1aTdkalYwNkJ5UlZMQXIyeS9Rb2twM2tXc01JSEJm?=
 =?utf-8?B?cEh2QkF4N2dIMkFSeDVPQkpvQS9jYjNXS3VFUklmMUNGcEpyQWxqb2lOc05s?=
 =?utf-8?B?cElIbTFkZ0ZyZnJoRDdlbGhiTTZiMTBORmQ5R1ZYYndBbVYzaGFkTldNcmlo?=
 =?utf-8?B?MVRRbkYyYUJ1L005UjJCamgxbmptMUFCS2hiZ2p5bVVHWk41RllVV2ZldlpQ?=
 =?utf-8?B?Um83amNCT3pQWUJiaWlHbGNTVWdBTGpFZFJNRHBRT3I2V09xVEpyNDhUaitJ?=
 =?utf-8?B?Tk1uV3k3N3VYRzJYdnpJdVVleUtrNml0WU91UkkxZVlxRUV6ZGhxNHRzNEgr?=
 =?utf-8?B?dnV6eElGcWU0Sk5VbEYrWjYwLzlQVGJGYjZERjY1OHdiZGlJak9heXFjMmZr?=
 =?utf-8?B?QmJVZ0s1YnVINEM1QUdRNTZEUCtjcTJzcTFPV0J1ak9HM3VRQ3NJTllZL1NT?=
 =?utf-8?B?c0J5UkgwVWFncS9ZMFFmd3Rkc25sL2xhejNpeFFXRkFFRjdFZFNlZ0NlQWhk?=
 =?utf-8?B?NTV0cVpYTlNlN0txN1dKMEhTNzRSdkFmMnhCMldoUUtuTGIxK0RaVFQ5YlFY?=
 =?utf-8?B?ZGVkMThCeVFxOE56cy9xVThZc21MbG10ZEtETXJjeUszd0E5b0pMYzlYTUNF?=
 =?utf-8?B?dllMaTZCZU1mbUVFb2dVNnExTHcwakVlUllBY3IyYW0wQS9HaG8yWlFsVUNm?=
 =?utf-8?B?R050ZGEySTN3b2dLeG8vZmVtZ2dJeWxpVEhYL2Nad2RKdU91aGsza21kcDF6?=
 =?utf-8?B?VE9LSFFYeVdwbm5hZ2d3WE1PUktsUTZ0S3R4WmdLY2tIVHBLc3dMeVhiZ1Zx?=
 =?utf-8?B?NnpaajFFKzdBeU9VSWV0N0s5cmE2aHh0MUs0TWFlNkFzVnJ3VVpMZVpDd2dR?=
 =?utf-8?B?UmFsSm52aGpCSEY1NmcwVlEwVDFRRlF1R25qeGdsdFMyQjZXYnRKL3lQZCsw?=
 =?utf-8?B?YWI5OXVMVEVuaXc0WkZNS2pnRjJEbFh0U0hrT2ZVYXFybWdJOW5PNDgvU25Z?=
 =?utf-8?B?L0R6Y2hmaUZIN09QaWlqSFRoVzA4K29XekhwaDRSUGszV3hlYmc1T2puZ1dp?=
 =?utf-8?B?NFRIOHVWZXlSamNzc0x2VXVKNjIxbksydFFyRzhnTzV3RHJNV2xPNUxCQk4v?=
 =?utf-8?B?dDVLQUpiaGZ3bjBUL3MxV1FqRWVxMG1vL0kzRnZ4dE1GaEk2NUFDeTZXZ0Ji?=
 =?utf-8?B?cVFKOE9uUEZFNktPbVV2OG90bVRNWHc1ZFFyczYyZ1l6RWE1OG5yMjQwaWpM?=
 =?utf-8?B?aStZcFhBMkZEaFNYMDRNSnd6clowemFEZG1QTmIwSDlaN0ZJSTFhV1F2RXZF?=
 =?utf-8?B?cmNmMUpYNXNmbTlsOUVENUdjdjVmZlF5N3BZY01YeFB6Y1lZMGYvbnk5cGZ2?=
 =?utf-8?B?M2V3MWY0eXdEZE1La3RNNVlsQnVoaCs2S3JtNTN6YkJzcW10ZGR5K0luNWc0?=
 =?utf-8?B?azdxWVNRZFZHQkEybEUzcisyc2V2ZEF5ZU1ScGsrQ3hMUEhGbmw3akZZNEZU?=
 =?utf-8?Q?iDH1xW8mE/h51qcYYcXTumTxd?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 19500207-10d3-489a-96fa-08db7d3a6b24
X-MS-Exchange-CrossTenant-AuthSource: SEZPR06MB5269.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2023 09:30:06.3932
 (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: Qt3dpfOcKarp4uxx2THMqJZLrKAnMt+I+nt7DfvMYCzrBtxv9nrtBKrA8PBip+Xb1swHZKaISTbafXhBnYKfBA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYZPR06MB4416
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

HI AngeloGioacchino,

On 2023/7/5 17:22, AngeloGioacchino Del Regno wrote:
> Il 05/07/23 11:01, Yangtao Li ha scritto:
>> Use devm_platform_ioremap_resource() to simplify code.
>>
>> Signed-off-by: Yangtao Li <frank.li@vivo.com>
>> ---
>>   drivers/phy/mediatek/phy-mtk-tphy.c | 10 +++-------
>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c 
>> b/drivers/phy/mediatek/phy-mtk-tphy.c
>> index 0d110e50bbfd..cb7a4e6ea017 100644
>> --- a/drivers/phy/mediatek/phy-mtk-tphy.c
>> +++ b/drivers/phy/mediatek/phy-mtk-tphy.c
>> @@ -1554,7 +1554,6 @@ static int mtk_tphy_probe(struct 
>> platform_device *pdev)
>>       struct device_node *np = dev->of_node;
>>       struct device_node *child_np;
>>       struct phy_provider *provider;
>> -    struct resource *sif_res;
>>       struct mtk_tphy *tphy;
>>       struct resource res;
>>       int port, retval;
>> @@ -1576,15 +1575,12 @@ static int mtk_tphy_probe(struct 
>> platform_device *pdev)
>>       tphy->dev = dev;
>>       platform_set_drvdata(pdev, tphy);
>>   -    sif_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       /* SATA phy of V1 needn't it if not shared with PCIe or USB */
>> -    if (sif_res && tphy->pdata->version == MTK_PHY_V1) {
>> +    if (tphy->pdata->version == MTK_PHY_V1) {
>>           /* get banks shared by multiple phys */
>> -        tphy->sif_base = devm_ioremap_resource(dev, sif_res);
>> -        if (IS_ERR(tphy->sif_base)) {
>> -            dev_err(dev, "failed to remap sif regs\n");
>> +        tphy->sif_base = devm_platform_ioremap_resource(pdev, 0);
>> +        if (IS_ERR(tphy->sif_base) && PTR_ERR(tphy->sif_base) != 
>> -EINVAL)
>
> That's wrong. You want to return any error unconditionally, as the V1 
> PHY *needs*
> the sif iospace and there's nothing afterwards retrying this.


When sif_res is not configured, an IOMEM_ERR_PTR(-EINVAL) error will be 
returned in __devm_ioremap_resource.

This kind of error is ignored in the newly added code, and the driving 
probe process will not be interrupted at this time.

What did I miss?

Thx,

Yangtao


>
> Please fix.
>
> Regards,
> Angelo
>
>>               return PTR_ERR(tphy->sif_base);
>> -        }
>>       }
>>         if (tphy->pdata->version < MTK_PHY_V3) {
>
>