Received-SPF: pass (google.com: domain of linux-kernel+bounces-73324-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID:
 <SEZPR06MB69594CBF0625989A5C54DC9096502@SEZPR06MB6959.apcprd06.prod.outlook.com>
Date: Wed, 21 Feb 2024 00:19:05 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC v2 1/5] dt-bindings: clock: histb-clock: Add missing
 common clock and Hi3798MV200 specific clock definition
Content-Language: en-US
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Michael Turquette <mturquette@baylibre.com>, Stephen Boyd
 <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>
Cc: David Yang <mmyangfl@gmail.com>, linux-clk@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240217-clk-mv200-v2-0-b782e4eb66f7@outlook.com>
 <20240217-clk-mv200-v2-1-b782e4eb66f7@outlook.com>
 <875b706f-801a-4a4c-8806-411a67c5a5e7@linaro.org>
 <SEZPR06MB6959456E59D84C15F0C1B88396502@SEZPR06MB6959.apcprd06.prod.outlook.com>
 <90e0dc10-8514-4827-998f-15b4d45d874e@linaro.org>
From: Yang Xiwen <forbidden405@outlook.com>
In-Reply-To: <90e0dc10-8514-4827-998f-15b4d45d874e@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: SEZPR06MB6959:EE_|PUZPR06MB5433:EE_
X-MS-Office365-Filtering-Correlation-Id: 188da492-4605-4da1-3b7b-08dc322faaae
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info:
	7GcMrDNw3/EquY1lTVAJo2ceALnj2qFlnp/BiYB1ONyNKGZZ+wygmK/FIe4qjndv/wrAmGpbDQ9euKlkf/9BfL1Gt5rg9CG6Jbo2irvgeC+FJUfuvobVwq1Xz3r2h3DL2sBifUlongU0c56m6DUE2UYkqH2+oUEcCCBirW1WfS7Mxb+ardm89yAY2a5xgaTKIHlhEKmNR76Mxt9wQZ+GJXiTUb5+VeotCrp4Tyfd2zXOmsh9nPiRHm86bD68nXOm8f3Q3E66fmdLY/t38pnqwqe2HACG13t+YqO2TPvvXR9WvCHY+RUie5TVJgQeUhOC6toJtxmXGzV6NgS/yRBUQ7odSKe5z3ivDykus0qrHxg0tB/hvfSwq3DCRwjzuk2vebuJgYLiY5BeK5vLspCxgL00FrhLZoqh+iJZXr208gKf5bAugtn0+WnSkWqoH9qZs9EgemdNB/hToWvYd9L4lDqljFymCznZ47cMjulJDb7W2DV5NnepxSA/J2BVCH86l1C2bC0ILFOAE7XuBS1WBUbAMkUtw6kkKYjVH/yWlImf4ixYzPa13idn8q7VwEKT
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?aDE0QWd5Z2xXWWpUcGRHRDVGVlVGMGF0NG5vQWRFZmZVcHUrN0pVckw1TWtm?=
 =?utf-8?B?RVZzVnlPUkpPQWpPUmhDSStFQW45NW9NUlFUKzNwckx2elk3ZjJqVWw2c2Fy?=
 =?utf-8?B?czJVSUFIUE5MZEw5ZEN5bnozSlJ0SkhIS1gyWWlqaGZMYkNIZzdxWFRsd3I0?=
 =?utf-8?B?UDVSTWwxTUJtem9WMmhnZlpmKzFZQkNlL24yblVLSzR0d2ZoeWtDdW13M2hE?=
 =?utf-8?B?T0grYk1JM3BvTC9YeGJpaWpOTUZaQ2d6aFd3M21jazcvSWtDeHh0MVhBT2x5?=
 =?utf-8?B?dkIrWnB5Z3VSd2lNRG9kbEI1azlBL2ZvSW8veXhhK0pybHJwc2d2YkU1VFc2?=
 =?utf-8?B?L1paQW9aVnRlZ2I5Y1NDVHp5bWh3K2pMUWJEM0xrdGplMjNERDRscFU0d1Z2?=
 =?utf-8?B?Mng4L0xaZ1RZeTNKc2p3bXR1OTU0REtlNTF2SzVFZjBEdlJGTzdsRGczTE1y?=
 =?utf-8?B?UWs1aTdCMlVrSnFzL3FRYmVwWjNMZHhvbmJRYzJMbElWaDVIaVNkYVh4QU1U?=
 =?utf-8?B?UHVwd2JVTlZ4V2JuWjFQOVpFUEJweTBod2ZHVFg5VEpMWStlMmVrWUk5TURQ?=
 =?utf-8?B?NXVpZFdkNlR5NHFTVTlXRXJnVWZWM3V1Q0ZIM2MzSmJDYUN2Uk51cjFxa2Zp?=
 =?utf-8?B?aDZNUDE2K2xxa1pSSEw4Q3RQcFRQSXV3cS9rTzcrOHFUUmUzMkZ6ZmFjdnc2?=
 =?utf-8?B?Y3lxcUVUdjlqRU1jQWJTeG5VN0orOXVVOXFjL3BOYm9GeGRLbGJLRFhiZlNF?=
 =?utf-8?B?NUxodnhUc0xabmt4a3VnODBCTTFFR0E2MTNuRldjNlBYUGZIMk9Ud3U4STZR?=
 =?utf-8?B?WHNrcEpNUUthQjZ6OW9yR2phbjZFM09zSkxEVFgzTTRsV1FwR1hxckx2ZDR3?=
 =?utf-8?B?S01jUGNaWVZBTFBjcEF5bUxPTnlqbDZpMkpvdmgrNzJtRHZJN3BVaVBNa2dn?=
 =?utf-8?B?dGNIaW1OOENNRWVqdFF2ajg0YXVpcGQwS0VqYmFSb0c4bENLbGxzQ21sNVMr?=
 =?utf-8?B?WVlZTlVDNWJrVHVOeVFldVMvTVV4aGcwSGJJMklVMTNTdGlsYUN3TElWZUtj?=
 =?utf-8?B?WWNNdE9JVnpIbjZjdjk3ZFd2S1RGSkJNeUJmQWwwTkJqVHBQVWZ1T1E2cm9M?=
 =?utf-8?B?bHhCL2l3K0QvVjZUNjVFWGg1RVIveE9CVkE0bGtmWWNQaFg1Sm1QSFlPL2Ro?=
 =?utf-8?B?TVc2dnZuZmR5aWszSHpUc3JhU2pTbHdMYlFMbUdFcFF0Z2l0RjZtMTI2S1Qw?=
 =?utf-8?B?RWp1ZVBvaWh1eHJiMzFmWVFRLzRxRktDK3NWY2J0RWIzZGR0TEhwN3d2S0hO?=
 =?utf-8?B?TDF6S1FhSTVmQklKbitIb1I0dnNSd3FNdHo2a2tKem14bGVLcHNzWW51bkdS?=
 =?utf-8?B?V3oxT0hublp6VEVRWmk1R2FuVElFZUU2cXBocE5LZjcrQUhpclMvNjZvdmVK?=
 =?utf-8?B?QUNIZzUzZW9la1hBNGh3WEphSDZ2bU1DZVB6Mnc1Z2k2azZ4dWk3ZFpjaWZs?=
 =?utf-8?B?cWdWeVVhWnJNdDQ4OTdVM21Yb0dJU2UrMGk3S1dYWmhKYytjSlF0c3FOblIv?=
 =?utf-8?B?QzFQK2tUd1Jvc0V1OVF4My84ZnFZVCtQMk05eGR5VHRaQ2hNUWVJQ0FEVXly?=
 =?utf-8?B?S3ZBcE5ocHByT3hOY0xmOTJzc2ttMmd6Q29GclFqYlNYdEtpZHFOUkxBQU9N?=
 =?utf-8?B?SHFrNWJyKzdMRDRpK05DcWFEdlErNFVPYWMvOHRqSElsWHZqT2VRWklUZzRW?=
 =?utf-8?Q?U5dLNU+2V0cyG1pkbVGwGxsHwjq3OI43FglRY8q?=
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 188da492-4605-4da1-3b7b-08dc322faaae
X-MS-Exchange-CrossTenant-AuthSource: SEZPR06MB6959.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2024 16:19:08.9512
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg:
	00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PUZPR06MB5433

On 2/21/2024 12:13 AM, Krzysztof Kozlowski wrote:
> On 20/02/2024 15:06, Yang Xiwen wrote:
>> On 2/20/2024 6:10 PM, Krzysztof Kozlowski wrote:
>>> On 17/02/2024 13:52, Yang Xiwen via B4 Relay wrote:
>>>> From: Yang Xiwen <forbidden405@outlook.com>
>>>>
>>>> According to the datasheet, some clocks are missing, add their
>>>> definitions first.
>>>>
>>>> Some aliases for hi3798mv200 are also introduced.
>>>>
>>>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>>>> ---
>>>>    include/dt-bindings/clock/histb-clock.h | 21 +++++++++++++++++++++
>>>>    1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/include/dt-bindings/clock/histb-clock.h b/include/dt-bindings/clock/histb-clock.h
>>>> index e64e5770ada6..68a53053586a 100644
>>>> --- a/include/dt-bindings/clock/histb-clock.h
>>>> +++ b/include/dt-bindings/clock/histb-clock.h
>>>> @@ -58,6 +58,27 @@
>>>>    #define HISTB_USB3_UTMI_CLK1		48
>>>>    #define HISTB_USB3_PIPE_CLK1		49
>>>>    #define HISTB_USB3_SUSPEND_CLK1		50
>>>> +#define HISTB_SDIO1_BIU_CLK		51
>>>> +#define HISTB_SDIO1_CIU_CLK		52
>>>> +#define HISTB_SDIO1_DRV_CLK		53
>>>> +#define HISTB_SDIO1_SAMPLE_CLK		54
>>>> +#define HISTB_ETH0_PHY_CLK		55
>>>> +#define HISTB_ETH1_PHY_CLK		56
>>>> +#define HISTB_WDG0_CLK			57
>>>> +#define HISTB_USB2_UTMI0_CLK		HISTB_USB2_UTMI_CLK
>>> Why? It's anyway placed oddly, the entries are ordered by number/value.
>>
>> So this is somewhat broken at the beginning. It named after
>> histb-clock.h but actually they are all clocks for Hi3798CV200 SoC. For
>> Hi3798MV200(also a HiSTB SoC), there is one additional UTMI clock.
>>
>>
>> What solution do you prefer? rename UTMI_CLK to UTMI0_CLK, add UTMI1_CLK
>> after it and increment all the indexes after it? Then the diff would be
>> very ugly.
> I still don't understand what is the problem you are trying to solve
> here. Your commit msg says add missing ID, but that ID -
> HISTB_USB2_UTMI_CLK - is already there.
>
> I also do not get why there is a need to rename anything.


Because there are two USB2_UTMI_CLKs in total, at least for Hi3798MV200. 
UTMI1 is missing here. For other HiSTB SoCs, there could be even more.


If we add USB2_UTMI1_CLK, it looks silly to keep USB2_UTMI_CLK without 
renaming it to UTMI0. Just like all the other clocks. E.g. 
I2Cn_CLK(n=0,1,2,3,4) etc.., so the same for USB2_UTMI_CLK.


>
>
>
> Best regards,
> Krzysztof
>

-- 
Regards,
Yang Xiwen