2024-02-15 23:49:33

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

From: Yang Xiwen <[email protected]>

These compatible strings are not found in any mainline dts, remove them.

Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/net/ethernet/hisilicon/hisi_femac.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
index d72160efff9a..6dabc62a00b7 100644
--- a/drivers/net/ethernet/hisilicon/hisi_femac.c
+++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
@@ -990,9 +990,6 @@ 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,hi3798mv200-femac",},
{},
};

--
2.43.0



2024-02-16 07:21:18

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

On 16/02/2024 00:48, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <[email protected]>
>
> These compatible strings are not found in any mainline dts, remove them.

That's not a real reason. What about all other users?

Best regards,
Krzysztof


2024-02-16 08:26:35

by Yang Xiwen

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

On 2/16/2024 3:20 PM, Krzysztof Kozlowski wrote:
> On 16/02/2024 00:48, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <[email protected]>
>>
>> These compatible strings are not found in any mainline dts, remove them.
> That's not a real reason. What about all other users?
The people who want their devices being supported should post a working
dts first. Having found the dts missing is strongly telling me that this
SoC(Hi3516) is orphan and EOL already. I can't even find it in git
commit logs. I'll argue that the old binding is simply wrong, and does
not describe the hardware properly. Who knows? Could anyone tell me if
the driver is still working for Hi3516 or not? I'm very willing to keep
the backward compatibility if someone can tell me the effort i paid to
maintain the old binding really makes sense. But the only things i found
in mainline kernel about Hi3516 is an CRG(clock) driver and this femac
driver. And it's been 8 years since last update for this SoC.
>
> Best regards,
> Krzysztof
>

--
Regards,
Yang Xiwen


2024-02-16 08:28:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

On 16/02/2024 09:21, Yang Xiwen wrote:
> On 2/16/2024 3:20 PM, Krzysztof Kozlowski wrote:
>> On 16/02/2024 00:48, Yang Xiwen via B4 Relay wrote:
>>> From: Yang Xiwen <[email protected]>
>>>
>>> These compatible strings are not found in any mainline dts, remove them.
>> That's not a real reason. What about all other users?
> The people who want their devices being supported should post a working
> dts first. Having found the dts missing is strongly telling me that this

Considering how poor HiSilicon contributions were - in numbers and
quality - that's kind of expected. :(


> SoC(Hi3516) is orphan and EOL already. I can't even find it in git
> commit logs. I'll argue that the old binding is simply wrong, and does
> not describe the hardware properly. Who knows? Could anyone tell me if
> the driver is still working for Hi3516 or not? I'm very willing to keep
> the backward compatibility if someone can tell me the effort i paid to
> maintain the old binding really makes sense. But the only things i found
> in mainline kernel about Hi3516 is an CRG(clock) driver and this femac
> driver. And it's been 8 years since last update for this SoC.

OK, that's fine with me, but please add parts of this explanation to the
commit msg (SoC is EOL, driver looks buggy and might not even work,
platform was upstreamed 8 years ago and no maintenance work happened on
it, thus it looks abandoned etc.).

Best regards,
Krzysztof


2024-02-16 08:41:53

by Yang Xiwen

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

On 2/16/2024 4:26 PM, Krzysztof Kozlowski wrote:
> On 16/02/2024 09:21, Yang Xiwen wrote:
>> On 2/16/2024 3:20 PM, Krzysztof Kozlowski wrote:
>>> On 16/02/2024 00:48, Yang Xiwen via B4 Relay wrote:
>>>> From: Yang Xiwen <[email protected]>
>>>>
>>>> These compatible strings are not found in any mainline dts, remove them.
>>> That's not a real reason. What about all other users?
>> The people who want their devices being supported should post a working
>> dts first. Having found the dts missing is strongly telling me that this
> Considering how poor HiSilicon contributions were - in numbers and
> quality - that's kind of expected. :(
>
>
>> SoC(Hi3516) is orphan and EOL already. I can't even find it in git
>> commit logs. I'll argue that the old binding is simply wrong, and does
>> not describe the hardware properly. Who knows? Could anyone tell me if
>> the driver is still working for Hi3516 or not? I'm very willing to keep
>> the backward compatibility if someone can tell me the effort i paid to
>> maintain the old binding really makes sense. But the only things i found
>> in mainline kernel about Hi3516 is an CRG(clock) driver and this femac
>> driver. And it's been 8 years since last update for this SoC.
> OK, that's fine with me, but please add parts of this explanation to the
> commit msg (SoC is EOL, driver looks buggy and might not even work,
> platform was upstreamed 8 years ago and no maintenance work happened on
> it, thus it looks abandoned etc.).

For me, it's a bit lucky to find a (partially) working driver in
mainline. It'll take me even more time if no mainline driver is
available. In fact, i wrote the driver for mainline u-boot from scratch
and it has been merged. So it's good to have this binding accepted
unmodified, or i'll have to modify u-boot side driver code to keep them
sync.

>
> Best regards,
> Krzysztof
>

--
Regards,
Yang Xiwen


2024-02-16 16:55:08

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

> For me, it's a bit lucky to find a (partially) working driver in mainline.
> It'll take me even more time if no mainline driver is available. In fact, i
> wrote the driver for mainline u-boot from scratch and it has been merged. So
> it's good to have this binding accepted unmodified, or i'll have to modify
> u-boot side driver code to keep them sync.

Sorry, but that is not how it works. If during review we decided it
needs to be modified, you will need to modify it.

I would suggest you first mainstream bindings to the kernel, because
it has active DT maintainers how really care about bindings. Then get
is merged to u-boot.

Andrew

2024-02-16 20:05:43

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

On Fri, Feb 16, 2024 at 02:01:08PM +0100, Andrew Lunn wrote:
> > For me, it's a bit lucky to find a (partially) working driver in mainline.
> > It'll take me even more time if no mainline driver is available. In fact, i
> > wrote the driver for mainline u-boot from scratch and it has been merged. So
> > it's good to have this binding accepted unmodified, or i'll have to modify
> > u-boot side driver code to keep them sync.
>
> Sorry, but that is not how it works. If during review we decided it
> needs to be modified, you will need to modify it.
>
> I would suggest you first mainstream bindings to the kernel, because
> it has active DT maintainers how really care about bindings. Then get
> is merged to u-boot.

Just to note, the U-Boot folk are currently working on a model where
they will be importing the kernel's dts files directly into their tree
along with the bindings. I think they're adding dtbs_check too.
Although that will be opt-in per board, it does point to an increased
desire for compliance there too, which is great.


Attachments:
(No filename) (1.04 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-16 20:09:53

by Yang Xiwen

[permalink] [raw]
Subject: Re: [PATCH 2/6] net: hisi_femac: remove unused compatible strings

On 2/17/2024 4:05 AM, Conor Dooley wrote:
> On Fri, Feb 16, 2024 at 02:01:08PM +0100, Andrew Lunn wrote:
>>> For me, it's a bit lucky to find a (partially) working driver in mainline.
>>> It'll take me even more time if no mainline driver is available. In fact, i
>>> wrote the driver for mainline u-boot from scratch and it has been merged. So
>>> it's good to have this binding accepted unmodified, or i'll have to modify
>>> u-boot side driver code to keep them sync.
>> Sorry, but that is not how it works. If during review we decided it
>> needs to be modified, you will need to modify it.
>>
>> I would suggest you first mainstream bindings to the kernel, because
>> it has active DT maintainers how really care about bindings. Then get
>> is merged to u-boot.
> Just to note, the U-Boot folk are currently working on a model where
> they will be importing the kernel's dts files directly into their tree
> along with the bindings. I think they're adding dtbs_check too.
> Although that will be opt-in per board, it does point to an increased
> desire for compliance there too, which is great.

Of course. I'll sync this stuff back to u-boot once this gets accepted
and merged. I begin working from u-boot simply because the Driver Model
of U-Boot is much simpler than Linux's. I wrote the driver for U-Boot
first to figure out how the hardware is working, then port it to Linux.

--
Regards,
Yang Xiwen