| Message ID | 20240307-net-v9-0-6e0cf3e6584d@outlook.com |
|---|---|
| Headers | show |
| Series | net: hisi-femac: add support for Hi3798MV200, remove unmaintained compatibles | expand |
On 07/03/2024 12:34, Yang Xiwen via B4 Relay wrote: > From: Yang Xiwen <forbidden405@outlook.com> > > Register the sub MDIO bus if it is found. Also implement the internal > PHY reset procedure as needed. ... > > @@ -946,6 +991,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev) > > static const struct of_device_id hisi_femac_match[] = { > {.compatible = "hisilicon,hi3516cv300-femac",}, > + {.compatible = "hisilicon,hi3798mv200-femac",}, Why do you keep growing this table? Best regards, Krzysztof
On 3/8/2024 4:02 PM, Krzysztof Kozlowski wrote: > On 07/03/2024 12:34, Yang Xiwen via B4 Relay wrote: >> From: Yang Xiwen <forbidden405@outlook.com> >> >> Register the sub MDIO bus if it is found. Also implement the internal >> PHY reset procedure as needed. > ... > >> >> @@ -946,6 +991,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev) >> >> static const struct of_device_id hisi_femac_match[] = { >> {.compatible = "hisilicon,hi3516cv300-femac",}, >> + {.compatible = "hisilicon,hi3798mv200-femac",}, > > Why do you keep growing this table? I'm completely confused. Don't I need to keep binding and driver compatible ids sync? The FEMAC cores on 2 SoCs are compatible afaik. That's why i want to add a generic "hisilicon,hisi-femac" compatible. Though i know nothing about the mysterious version numbers (v1, v2 etc..) documented in the old binding, so i want them to be removed. Instead only keep one generic fallback compatible. Do you mean that i broke the backward compatibility for "hisilicon,hi3516cv300-femac"? > > Best regards, > Krzysztof >
On 08/03/2024 09:07, Yang Xiwen wrote: > On 3/8/2024 4:02 PM, Krzysztof Kozlowski wrote: >> On 07/03/2024 12:34, Yang Xiwen via B4 Relay wrote: >>> From: Yang Xiwen <forbidden405@outlook.com> >>> >>> Register the sub MDIO bus if it is found. Also implement the internal >>> PHY reset procedure as needed. >> ... >> >>> >>> @@ -946,6 +991,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev) >>> >>> static const struct of_device_id hisi_femac_match[] = { >>> {.compatible = "hisilicon,hi3516cv300-femac",}, >>> + {.compatible = "hisilicon,hi3798mv200-femac",}, >> >> Why do you keep growing this table? > > > I'm completely confused. Don't I need to keep binding and driver > compatible ids sync? > > > The FEMAC cores on 2 SoCs are compatible afaik. That's why i want to add > a generic "hisilicon,hisi-femac" compatible. Though i know nothing about > the mysterious version numbers (v1, v2 etc..) documented in the old > binding, so i want them to be removed. Instead only keep one generic > fallback compatible. > > > Do you mean that i broke the backward compatibility for > "hisilicon,hi3516cv300-femac"? No. I meant, use one as fallback and only fallback needs to be in the device ID table. There are dozens if not hundreds of such examples in the tree. Best regards, Krzysztof
On 3/8/2024 4:09 PM, Krzysztof Kozlowski wrote: > On 08/03/2024 09:07, Yang Xiwen wrote: >> On 3/8/2024 4:02 PM, Krzysztof Kozlowski wrote: >>> On 07/03/2024 12:34, Yang Xiwen via B4 Relay wrote: >>>> From: Yang Xiwen <forbidden405@outlook.com> >>>> >>>> Register the sub MDIO bus if it is found. Also implement the internal >>>> PHY reset procedure as needed. >>> ... >>> >>>> >>>> @@ -946,6 +991,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev) >>>> >>>> static const struct of_device_id hisi_femac_match[] = { >>>> {.compatible = "hisilicon,hi3516cv300-femac",}, >>>> + {.compatible = "hisilicon,hi3798mv200-femac",}, >>> Why do you keep growing this table? >> >> I'm completely confused. Don't I need to keep binding and driver >> compatible ids sync? >> >> >> The FEMAC cores on 2 SoCs are compatible afaik. That's why i want to add >> a generic "hisilicon,hisi-femac" compatible. Though i know nothing about >> the mysterious version numbers (v1, v2 etc..) documented in the old >> binding, so i want them to be removed. Instead only keep one generic >> fallback compatible. >> >> >> Do you mean that i broke the backward compatibility for >> "hisilicon,hi3516cv300-femac"? > No. I meant, use one as fallback and only fallback needs to be in the > device ID table. There are dozens if not hundreds of such examples in > the tree. I don't think an arbitrary SoC compatible is a good name for a fallback compatible. Why can't we have "hisilicon,hisi-femac" instead of the odd "hisilicon,hi3516cv300-femac", If we are not going to keep backward compatibility? Hi3516CV300 is just an old and outdated ordinary SoC after all, but the FEMAC core is still being used in latest SoCs afaik. I can't see the reason to relate this core to some old SoC and keep the compatible forever. > > Best regards, > Krzysztof >
On 08/03/2024 09:18, Yang Xiwen wrote: > On 3/8/2024 4:09 PM, Krzysztof Kozlowski wrote: >> On 08/03/2024 09:07, Yang Xiwen wrote: >>> On 3/8/2024 4:02 PM, Krzysztof Kozlowski wrote: >>>> On 07/03/2024 12:34, Yang Xiwen via B4 Relay wrote: >>>>> From: Yang Xiwen <forbidden405@outlook.com> >>>>> >>>>> Register the sub MDIO bus if it is found. Also implement the internal >>>>> PHY reset procedure as needed. >>>> ... >>>> >>>>> >>>>> @@ -946,6 +991,7 @@ static int hisi_femac_drv_resume(struct platform_device *pdev) >>>>> >>>>> static const struct of_device_id hisi_femac_match[] = { >>>>> {.compatible = "hisilicon,hi3516cv300-femac",}, >>>>> + {.compatible = "hisilicon,hi3798mv200-femac",}, >>>> Why do you keep growing this table? >>> >>> I'm completely confused. Don't I need to keep binding and driver >>> compatible ids sync? >>> >>> >>> The FEMAC cores on 2 SoCs are compatible afaik. That's why i want to add >>> a generic "hisilicon,hisi-femac" compatible. Though i know nothing about >>> the mysterious version numbers (v1, v2 etc..) documented in the old >>> binding, so i want them to be removed. Instead only keep one generic >>> fallback compatible. >>> >>> >>> Do you mean that i broke the backward compatibility for >>> "hisilicon,hi3516cv300-femac"? >> No. I meant, use one as fallback and only fallback needs to be in the >> device ID table. There are dozens if not hundreds of such examples in >> the tree. > > > I don't think an arbitrary SoC compatible is a good name for a fallback > compatible. Why can't we have "hisilicon,hisi-femac" instead of the odd Why? Anyway, why rules for Hisilicon should be different than for everyone else? > "hisilicon,hi3516cv300-femac", If we are not going to keep backward > compatibility? Hi3516CV300 is just an old and outdated ordinary SoC > after all, but the FEMAC core is still being used in latest SoCs afaik. > I can't see the reason to relate this core to some old SoC and keep the > compatible forever. Why rules for Hisilicon should be different than for everyone else? Best regards, Krzysztof
Signed-off-by: Yang Xiwen <forbidden405@outlook.com> --- Changes in v9: - binding: remove generic fallback compatible as it's not used in driver - Link to v8: https://lore.kernel.org/r/20240305-net-v8-0-166aaeea2107@outlook.com Changes in v8: - remove MODULE_ALIAS: rewrite commit msg (Krzysztof Kozlowski) - driver: use only SoC compatibles (Krzysztof Kozlowski) - Link to v7: https://lore.kernel.org/r/20240301-net-v7-0-45823597d4d4@outlook.com Changes in v7: - dt-bindings: squash a bunch of patches to YAML conversion (Krzysztof Kozlowski) - dt-bindings: drop phy-mode->phy-connection-type conversion (Andrew Lunnm, Krzysztof Kozlowski) - driver: drop SoC compatibles - misc: some minor clean ups - driver: remove MODULE_ALIAS() - Link to v6: https://lore.kernel.org/r/20240228-net-v6-0-6d78d3d598c1@outlook.com Changes in v6: - add missing "not" in commit logs (Andrew) - rework binding changes, split it into several commits (Krzysztof Kozlowski) - Link to v5: https://lore.kernel.org/r/20240223-net-v5-0-43b22d39c013@outlook.com Changes in v5: - hisi-femac-mdio: remove clock completely (Krzysztof Kozlowski) - dt-bindings: mdio: apply comments from Krzysztof Kozlowski Changes in v4: - edit commit log to show why mdio bus clk is optional (Krzysztof Kozlowski) - add clk_bulk_disable_unprepare() during error path (Maxime Chevallier) - remove of_node_put() (Simon Horman) - remove histb-clock.h header in binding example as it's goign to be deprecated. - rearrange patches so binding comes before driver - Link to v3: https://lore.kernel.org/r/20240220-net-v3-0-b68e5b75e765@outlook.com Changes in v3: - rearrange patches to fix bot error. (Rob Herring) - rewrite commit logs (Andrew Lunn) - use clk_bulk_ APIs (Andrew Lunn) - fix uninitialization use of ret (assign to -ENODEV before goto) (Simon Horman) - Link to v2: https://lore.kernel.org/r/20240216-net-v2-0-89bd4b7065c2@outlook.com Changes in v2: - replace email. - hisi-femac: s/BUS/MACIF (Andrew Lunn) - hisi-femac: add "hisilicon,hisi-femac" compatible since the driver seems generic enough for various SoCs - hisi-femac-mdio: convert binding to YAML (Krzysztof Kozlowski) - rewrite commit logs (Krzysztof Kozlowski) - Link to v1: https://lore.kernel.org/r/20240216-net-v1-0-e0ad972cda99@outlook.com --- Yang Xiwen (9): dt-bindings: net: hisilicon-femac-mdio: convert to YAML dt-bindings: net: hisilicon,hisi-femac-mdio: remove clocks net: mdio: hisi-femac: remove clock dt-bindings: net: convert hisi-femac.txt to YAML dt-bindings: net: hisi-femac: add mandatory MDIO bus subnode dt-bindings: net: hisi-femac: add binding for Hi3798MV200 FEMAC core net: hisi_femac: remove unused compatible strings net: hisi_femac: add support for hisi_femac core on Hi3798MV200 net: hisi_femac: remove unneeded MODULE_ALIAS() .../bindings/net/hisilicon,hisi-femac-mdio.yaml | 39 +++++++ .../bindings/net/hisilicon,hisi-femac.yaml | 118 +++++++++++++++++++++ .../bindings/net/hisilicon-femac-mdio.txt | 22 ---- .../devicetree/bindings/net/hisilicon-femac.txt | 41 ------- drivers/net/ethernet/hisilicon/hisi_femac.c | 77 +++++++++++--- drivers/net/mdio/mdio-hisi-femac.c | 18 +--- 6 files changed, 218 insertions(+), 97 deletions(-) --- base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72 change-id: 20240216-net-9a208e17c40f Best regards,