diff mbox series

[net-next,v4,6/6] net: hisi_femac: remove unused compatible strings

Message ID 20240222-net-v4-6-eea68f93f090@outlook.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: hisi-femac: add support for Hi3798MV200, remove unmaintained compatibles | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 940 this patch: 940
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 957 this patch: 957
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 957 this patch: 957
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 10 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-02-23--03-00 (tests: 1457)

Commit Message

Yang Xiwen via B4 Relay Feb. 22, 2024, 12:43 p.m. UTC
From: Yang Xiwen <forbidden405@outlook.com>

The only documented SoC Hi3516DV300 does not receive any updates from 8
years ago. With the recent driver changes, it unlikely works for this
SoC anymore. Remove the binding for this SoC.

Also it's hard to get the version number and it's unknown how the
version can be used. Remove them until it's really needed.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Krzysztof Kozlowski Feb. 26, 2024, 7:55 a.m. UTC | #1
On 22/02/2024 13:43, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
> 
> The only documented SoC Hi3516DV300 does not receive any updates from 8
> years ago. With the recent driver changes, it unlikely works for this
> SoC anymore. Remove the binding for this SoC.
> 
> Also it's hard to get the version number and it's unknown how the
> version can be used. Remove them until it's really needed.
> 
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
>  drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
> index eab91e011d11..9466ca9da2bb 100644
> --- a/drivers/net/ethernet/hisilicon/hisi_femac.c
> +++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
> @@ -990,9 +990,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev)
>  #endif
>  
>  static const struct of_device_id hisi_femac_match[] = {
> -	{.compatible = "hisilicon,hisi-femac-v1",},
> -	{.compatible = "hisilicon,hisi-femac-v2",},
> -	{.compatible = "hisilicon,hi3516cv300-femac",},
> +	{.compatible = "hisilicon,hisi-femac",},

What is happening here? Removal could be justified, but then order of
your patches is totally wrong. But that hisi-femac is a no-go or provide
proper rationale.

Best regards,
Krzysztof
Yang Xiwen Feb. 27, 2024, 1:51 a.m. UTC | #2
On 2/26/2024 3:55 PM, Krzysztof Kozlowski wrote:
> On 22/02/2024 13:43, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
>>
>> The only documented SoC Hi3516DV300 does not receive any updates from 8
>> years ago. With the recent driver changes, it unlikely works for this
>> SoC anymore. Remove the binding for this SoC.
>>
>> Also it's hard to get the version number and it's unknown how the
>> version can be used. Remove them until it's really needed.
>>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>>  drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
>> index eab91e011d11..9466ca9da2bb 100644
>> --- a/drivers/net/ethernet/hisilicon/hisi_femac.c
>> +++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
>> @@ -990,9 +990,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev)
>>  #endif
>>  
>>  static const struct of_device_id hisi_femac_match[] = {
>> -	{.compatible = "hisilicon,hisi-femac-v1",},
>> -	{.compatible = "hisilicon,hisi-femac-v2",},
>> -	{.compatible = "hisilicon,hi3516cv300-femac",},
>> +	{.compatible = "hisilicon,hisi-femac",},
> 
> What is happening here? Removal could be justified, but then order of
> your patches is totally wrong. But that hisi-femac is a no-go or provide
> proper rationale.

I don't understand exactly... In dts, we commonly have a SoC specific
compatible string first, generic compatible string the second. e.g.

compatible = "hisilicon,hi3798mv200-perictrl", "syscon", "simple-mfd".

So i think this is recommended. Or does it mean we only need them in
dts, not in the driver? The generic driver only needs a generic
compatible "hisilicon,hisi-femac" in all?

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 27, 2024, 6:53 a.m. UTC | #3
On 27/02/2024 02:51, Yang Xiwen wrote:
> On 2/26/2024 3:55 PM, Krzysztof Kozlowski wrote:
>> On 22/02/2024 13:43, Yang Xiwen via B4 Relay wrote:
>>> From: Yang Xiwen <forbidden405@outlook.com>
>>>
>>> The only documented SoC Hi3516DV300 does not receive any updates from 8
>>> years ago. With the recent driver changes, it unlikely works for this
>>> SoC anymore. Remove the binding for this SoC.
>>>
>>> Also it's hard to get the version number and it's unknown how the
>>> version can be used. Remove them until it's really needed.
>>>
>>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>>> ---
>>>  drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
>>> index eab91e011d11..9466ca9da2bb 100644
>>> --- a/drivers/net/ethernet/hisilicon/hisi_femac.c
>>> +++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
>>> @@ -990,9 +990,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev)
>>>  #endif
>>>  
>>>  static const struct of_device_id hisi_femac_match[] = {
>>> -	{.compatible = "hisilicon,hisi-femac-v1",},
>>> -	{.compatible = "hisilicon,hisi-femac-v2",},
>>> -	{.compatible = "hisilicon,hi3516cv300-femac",},
>>> +	{.compatible = "hisilicon,hisi-femac",},
>>
>> What is happening here? Removal could be justified, but then order of
>> your patches is totally wrong. But that hisi-femac is a no-go or provide
>> proper rationale.
> 
> I don't understand exactly... In dts, we commonly have a SoC specific
> compatible string first, generic compatible string the second. e.g.
> 
> compatible = "hisilicon,hi3798mv200-perictrl", "syscon", "simple-mfd".

There is no generic compatible here. hi3798mv200 is soc.

> 
> So i think this is recommended. Or does it mean we only need them in

It is allowed for certain cases and recommended for even fewer ones. Do
you want to say you have fully discoverable features here and you do not
need any properties? Or you want to say that all possible hardware will
have exactly the same programming interface (registers etc)?

Best regards,
Krzysztof
Yang Xiwen Feb. 27, 2024, 7:36 a.m. UTC | #4
On 2/27/2024 2:53 PM, Krzysztof Kozlowski wrote:
> On 27/02/2024 02:51, Yang Xiwen wrote:
>> On 2/26/2024 3:55 PM, Krzysztof Kozlowski wrote:
>>> On 22/02/2024 13:43, Yang Xiwen via B4 Relay wrote:
>>>> From: Yang Xiwen <forbidden405@outlook.com>
>>>>
>>>> The only documented SoC Hi3516DV300 does not receive any updates from 8
>>>> years ago. With the recent driver changes, it unlikely works for this
>>>> SoC anymore. Remove the binding for this SoC.
>>>>
>>>> Also it's hard to get the version number and it's unknown how the
>>>> version can be used. Remove them until it's really needed.
>>>>
>>>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>>>> ---
>>>>  drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +---
>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
>>>> index eab91e011d11..9466ca9da2bb 100644
>>>> --- a/drivers/net/ethernet/hisilicon/hisi_femac.c
>>>> +++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
>>>> @@ -990,9 +990,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev)
>>>>  #endif
>>>>  
>>>>  static const struct of_device_id hisi_femac_match[] = {
>>>> -	{.compatible = "hisilicon,hisi-femac-v1",},
>>>> -	{.compatible = "hisilicon,hisi-femac-v2",},
>>>> -	{.compatible = "hisilicon,hi3516cv300-femac",},
>>>> +	{.compatible = "hisilicon,hisi-femac",},
>>>
>>> What is happening here? Removal could be justified, but then order of
>>> your patches is totally wrong. But that hisi-femac is a no-go or provide
>>> proper rationale.
>>
>> I don't understand exactly... In dts, we commonly have a SoC specific
>> compatible string first, generic compatible string the second. e.g.
>>
>> compatible = "hisilicon,hi3798mv200-perictrl", "syscon", "simple-mfd".
> 
> There is no generic compatible here. hi3798mv200 is soc.
> 
>>
>> So i think this is recommended. Or does it mean we only need them in
> 
> It is allowed for certain cases and recommended for even fewer ones. Do
> you want to say you have fully discoverable features here and you do not
> need any properties? Or you want to say that all possible hardware will
> have exactly the same programming interface (registers etc)?

Of course not. Take FEMAC for example. The FEMAC core on Hi3516 and
Hi3798 can programmed in the same way. They use the same
resources(resets and clocks). Though still a bit different in hardware,
basically the fifo depth etc..

Hi3516 FEMAC is not special enough to have a new compatible string, nor
do hi3798 FEMAC. Nor do i think we need those undocumented version
numbers, i.e. "hisilicon,hisi-femac-v1/2". As i observed, this driver is
generic enough to handle all the FEMAC cores i know, no matter which
version nor which SoC. This can also be concluded from the driver, the
driver defined 3 compatibles but they are all treated the same.

Should I remove all of them, and only leave a generic
"hisilicon,hisi-femac" instead?
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 27, 2024, 7:48 a.m. UTC | #5
On 27/02/2024 08:36, Yang Xiwen wrote:
> On 2/27/2024 2:53 PM, Krzysztof Kozlowski wrote:
>> On 27/02/2024 02:51, Yang Xiwen wrote:
>>> On 2/26/2024 3:55 PM, Krzysztof Kozlowski wrote:
>>>> On 22/02/2024 13:43, Yang Xiwen via B4 Relay wrote:
>>>>> From: Yang Xiwen <forbidden405@outlook.com>
>>>>>
>>>>> The only documented SoC Hi3516DV300 does not receive any updates from 8
>>>>> years ago. With the recent driver changes, it unlikely works for this
>>>>> SoC anymore. Remove the binding for this SoC.
>>>>>
>>>>> Also it's hard to get the version number and it's unknown how the
>>>>> version can be used. Remove them until it's really needed.
>>>>>
>>>>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>>>>> ---
>>>>>  drivers/net/ethernet/hisilicon/hisi_femac.c | 4 +---
>>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
>>>>> index eab91e011d11..9466ca9da2bb 100644
>>>>> --- a/drivers/net/ethernet/hisilicon/hisi_femac.c
>>>>> +++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
>>>>> @@ -990,9 +990,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev)
>>>>>  #endif
>>>>>  
>>>>>  static const struct of_device_id hisi_femac_match[] = {
>>>>> -	{.compatible = "hisilicon,hisi-femac-v1",},
>>>>> -	{.compatible = "hisilicon,hisi-femac-v2",},
>>>>> -	{.compatible = "hisilicon,hi3516cv300-femac",},
>>>>> +	{.compatible = "hisilicon,hisi-femac",},
>>>>
>>>> What is happening here? Removal could be justified, but then order of
>>>> your patches is totally wrong. But that hisi-femac is a no-go or provide
>>>> proper rationale.
>>>
>>> I don't understand exactly... In dts, we commonly have a SoC specific
>>> compatible string first, generic compatible string the second. e.g.
>>>
>>> compatible = "hisilicon,hi3798mv200-perictrl", "syscon", "simple-mfd".
>>
>> There is no generic compatible here. hi3798mv200 is soc.
>>
>>>
>>> So i think this is recommended. Or does it mean we only need them in
>>
>> It is allowed for certain cases and recommended for even fewer ones. Do
>> you want to say you have fully discoverable features here and you do not
>> need any properties? Or you want to say that all possible hardware will
>> have exactly the same programming interface (registers etc)?
> 
> Of course not. Take FEMAC for example. The FEMAC core on Hi3516 and

If they have different programming interface then you cannot use generic
compatible as fallback.

> Hi3798 can programmed in the same way. They use the same
> resources(resets and clocks). Though still a bit different in hardware,
> basically the fifo depth etc..
> 
> Hi3516 FEMAC is not special enough to have a new compatible string, nor
> do hi3798 FEMAC. Nor do i think we need those undocumented version
> numbers, i.e. "hisilicon,hisi-femac-v1/2". As i observed, this driver is
> generic enough to handle all the FEMAC cores i know, no matter which
> version nor which SoC. This can also be concluded from the driver, the
> driver defined 3 compatibles but they are all treated the same.
> 
> Should I remove all of them, and only leave a generic
> "hisilicon,hisi-femac" instead?

No.

Use one SoC specific compatible as fallback. That's what we ask almost
every time...

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
index eab91e011d11..9466ca9da2bb 100644
--- a/drivers/net/ethernet/hisilicon/hisi_femac.c
+++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
@@ -990,9 +990,7 @@  static int hisi_femac_drv_resume(struct platform_device *pdev)
 #endif
 
 static const struct of_device_id hisi_femac_match[] = {
-	{.compatible = "hisilicon,hisi-femac-v1",},
-	{.compatible = "hisilicon,hisi-femac-v2",},
-	{.compatible = "hisilicon,hi3516cv300-femac",},
+	{.compatible = "hisilicon,hisi-femac",},
 	{.compatible = "hisilicon,hi3798mv200-femac",},
 	{},
 };