arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
From: Frank Wunderlich <frank-w@public-files.de>
After commit 868ff5f4944a
("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
the mt7531 switch on Bananapi-R64 was not detected.
mt7530-mdio mdio-bus:00: reset timeout
mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
Fix this by adding phy address in devicetree.
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 224bb289660c..811b227d6f50 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -149,9 +149,9 @@ mdio: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;
- switch@0 {
+ switch@1f {
compatible = "mediatek,mt7531";
- reg = <0>;
+ reg = <0x1f>;
interrupt-controller;
#interrupt-cells = <1>;
interrupts-extended = <&pio 53 IRQ_TYPE_LEVEL_HIGH>;
--
2.34.1
On Thu, 16 May 2024 22:48:47 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> After commit 868ff5f4944a
> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
> the mt7531 switch on Bananapi-R64 was not detected.
>
> mt7530-mdio mdio-bus:00: reset timeout
> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>
> Fix this by adding phy address in devicetree.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
> arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
My bot found new DTB warnings on the .dts files added or changed in this
series.
Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.
If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:
pip3 install dtschema --upgrade
New warnings running 'make CHECK_DTBS=y mediatek/mt7622-bananapi-bpi-r64.dtb' for 20240516204847.171029-1-linux@fw-web.de:
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dtb: switch@1f: 'interrupts' is a dependency of 'interrupt-controller'
from schema $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#
On 16/05/2024 23:48, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> After commit 868ff5f4944a
> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
> the mt7531 switch on Bananapi-R64 was not detected.
>
> mt7530-mdio mdio-bus:00: reset timeout
> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>
> Fix this by adding phy address in devicetree.
>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
I don't like the mention of the Linux kernel driver on the patch log. What
you're fixing is the incorrect description of the switch's PHY address on
the DTS file. Whether or not any driver from any project is actually
reading it from the DTS file is irrelevant to this patch. That said, I
already have a patch series I've been meaning to send the next version of
that already addresses this. Please wait for that.
Arınç
Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>On 16/05/2024 23:48, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@public-files.de>
>>
>> After commit 868ff5f4944a
>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>> the mt7531 switch on Bananapi-R64 was not detected.
>>
>> mt7530-mdio mdio-bus:00: reset timeout
>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>
>> Fix this by adding phy address in devicetree.
>>
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>
>I don't like the mention of the Linux kernel driver on the patch log. What
>you're fixing is the incorrect description of the switch's PHY address on
>the DTS file. Whether or not any driver from any project is actually
>reading it from the DTS file is irrelevant to this patch. That said, I
>already have a patch series I've been meaning to send the next version of
>that already addresses this. Please wait for that.
>
>Arınç
Hi arinc,
From my PoV it is a regression in next/6.10 because the driver change was merged (without "broadcast" fallback) and the dts patch [1] is not.
I agree that my patch can be dropped because there was already one.
regards Frank
[1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240314-for-mediatek-mt7531-phy-address-v1-1-52f58db01acd@arinc9.com/
On 17/05/2024 09.27, Frank Wunderlich wrote:
> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> After commit 868ff5f4944a
>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>
>>> mt7530-mdio mdio-bus:00: reset timeout
>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>>
>>> Fix this by adding phy address in devicetree.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>
>> I don't like the mention of the Linux kernel driver on the patch log. What
>> you're fixing is the incorrect description of the switch's PHY address on
>> the DTS file. Whether or not any driver from any project is actually
>> reading it from the DTS file is irrelevant to this patch. That said, I
>> already have a patch series I've been meaning to send the next version of
>> that already addresses this. Please wait for that.
>>
>> Arınç
>
> Hi arinc,
>
> From my PoV it is a regression in next/6.10 because the driver change was merged (without "broadcast" fallback) and the dts patch [1] is not.
What is a broadcast fallback? 0x1f is just another PHY address.
Arınç
Am 31. Mai 2024 08:12:06 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>On 17/05/2024 09.27, Frank Wunderlich wrote:
>> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>
>>>> After commit 868ff5f4944a
>>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>>
>>>> mt7530-mdio mdio-bus:00: reset timeout
>>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>>>
>>>> Fix this by adding phy address in devicetree.
>>>>
>>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> I don't like the mention of the Linux kernel driver on the patch log. What
>>> you're fixing is the incorrect description of the switch's PHY address on
>>> the DTS file. Whether or not any driver from any project is actually
>>> reading it from the DTS file is irrelevant to this patch. That said, I
>>> already have a patch series I've been meaning to send the next version of
>>> that already addresses this. Please wait for that.
>>>
>>> Arınç
>>
>> Hi arinc,
>>
>> From my PoV it is a regression in next/6.10 because the driver change was merged (without "broadcast" fallback) and the dts patch [1] is not.
>
>What is a broadcast fallback? 0x1f is just another PHY address.
Afaik 0x0 is some kind of broadcast address if real phy address is not known. The driver change seems not allow this 0x0 adress and forces devicetree to have the real address.
Thats what i mean with broadcast fallback. Maybe the naming is wrong.
>Arınç
@thorsten i have not tested again,but i have not seen any further fix for it.
regards Frank
On 31/05/2024 09.18, Frank Wunderlich wrote:
> Am 31. Mai 2024 08:12:06 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>> On 17/05/2024 09.27, Frank Wunderlich wrote:
>>> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>>>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>>
>>>>> After commit 868ff5f4944a
>>>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>>>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>>>
>>>>> mt7530-mdio mdio-bus:00: reset timeout
>>>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>>>>
>>>>> Fix this by adding phy address in devicetree.
>>>>>
>>>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>>>
>>>> I don't like the mention of the Linux kernel driver on the patch log. What
>>>> you're fixing is the incorrect description of the switch's PHY address on
>>>> the DTS file. Whether or not any driver from any project is actually
>>>> reading it from the DTS file is irrelevant to this patch. That said, I
>>>> already have a patch series I've been meaning to send the next version of
>>>> that already addresses this. Please wait for that.
>>>>
>>>> Arınç
>>>
>>> Hi arinc,
>>>
>>> From my PoV it is a regression in next/6.10 because the driver change was merged (without "broadcast" fallback) and the dts patch [1] is not.
>>
>> What is a broadcast fallback? 0x1f is just another PHY address.
>
> Afaik 0x0 is some kind of broadcast address if real phy address is not known. The driver change seems not allow this 0x0 adress and forces devicetree to have the real address.
That's not true. 0x0 is just another PHY address. The driver change
actually allows reading 0x0 from the device tree and using that PHY address
to control the switch registers, instead of using 0x1f which was hardcoded
on the driver. But on the hardware, the switch actually listens on 0x1f.
Arınç
[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]
On 17.05.24 08:27, Frank Wunderlich wrote:
> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> After commit 868ff5f4944a
>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>
>>> mt7530-mdio mdio-bus:00: reset timeout
>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>>
>>> Fix this by adding phy address in devicetree.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>
>> I don't like the mention of the Linux kernel driver on the patch log. What
>> you're fixing is the incorrect description of the switch's PHY address on
>> the DTS file. Whether or not any driver from any project is actually
>> reading it from the DTS file is irrelevant to this patch. That said, I
>> already have a patch series I've been meaning to send the next version of
>> that already addresses this. Please wait for that.
Did you sent this? Because from what I see with my limited experience in
this subsystem...
> From my PoV it is a regression in next/6.10
...I have to agree with Frank here. So it would be good to get this
fixed before -rc1 is out to prevent more people from running into this.
> because the driver change was merged (without "broadcast" fallback) and the dts patch [1] is not.
>
> I agree that my patch can be dropped because there was already one.
>
> regards Frank
>
> [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240314-for-mediatek-mt7531-phy-address-v1-1-52f58db01acd@arinc9.com/
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
[adding Paolo, who committed the culprit]
On 23.05.24 12:44, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 17.05.24 08:27, Frank Wunderlich wrote:
>> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>
>>>> After commit 868ff5f4944a
>>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>>
>>>> mt7530-mdio mdio-bus:00: reset timeout
>>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>>>
>>>> Fix this by adding phy address in devicetree.
>>>>
Frank, am I right assuming the regression is still present in mainline?
As from here it looks like for two weeks now there was no progress at
all to fix this (or I missed it, which is quite possible).
Makes me wonder if the maintainers should revert the culprit or if the
arm64 dts folks should accept your fix despite Arınç ÜNAL's (who from a
quick look on lore hasn't posted anything for two weeks now) comment.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
>>> I don't like the mention of the Linux kernel driver on the patch log. What
>>> you're fixing is the incorrect description of the switch's PHY address on
>>> the DTS file. Whether or not any driver from any project is actually
>>> reading it from the DTS file is irrelevant to this patch. That said, I
>>> already have a patch series I've been meaning to send the next version of
>>> that already addresses this. Please wait for that.
>
> Did you sent this? Because from what I see with my limited experience in
> this subsystem...
>
>> From my PoV it is a regression in next/6.10
>
> ...I have to agree with Frank here. So it would be good to get this
> fixed before -rc1 is out to prevent more people from running into this.
>
>> because the driver change was merged (without "broadcast" fallback) and the dts patch [1] is not.
>>
>> I agree that my patch can be dropped because there was already one.
>>
>> regards Frank
>>
>> [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240314-for-mediatek-mt7531-phy-address-v1-1-52f58db01acd@arinc9.com/
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
On 31/05/2024 08.40, Thorsten Leemhuis wrote:
> [adding Paolo, who committed the culprit]
>
> On 23.05.24 12:44, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 17.05.24 08:27, Frank Wunderlich wrote:
>>> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL" <arinc.unal@arinc9.com>:
>>>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>>
>>>>> After commit 868ff5f4944a
>>>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device tree")
>>>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>>>
>>>>> mt7530-mdio mdio-bus:00: reset timeout
>>>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with error -110
>>>>>
>>>>> Fix this by adding phy address in devicetree.
>>>>>
>
> Frank, am I right assuming the regression is still present in mainline?
> As from here it looks like for two weeks now there was no progress at
> all to fix this (or I missed it, which is quite possible).
>
> Makes me wonder if the maintainers should revert the culprit or if the
> arm64 dts folks should accept your fix despite Arınç ÜNAL's (who from a
> quick look on lore hasn't posted anything for two weeks now) comment.
I'm not against the patch. I'm against the logic it entails on the patch
log. I had already submitted a patch series that would've prevented this
issue back in 14 March 2024 [1]. I've asked numerous times for the patch
series to be applied [2][3][4][5].
Eventually Daniel asked for some changes [6]. But I won't have time to do
that anytime soon and I think the patch series is good enough to be applied
as is.
[1] https://lore.kernel.org/all/20240314-for-mediatek-mt7531-phy-address-v1-0-52f58db01acd@arinc9.com/
[2] https://lore.kernel.org/all/ff196055-ecd8-4563-bc01-ff2533a07109@arinc9.com/
[3] https://lore.kernel.org/all/a60fc16d-4236-427c-b4a8-ec6fdf62d9f0@arinc9.com/
[4] https://lore.kernel.org/all/facb8204-c2b3-4084-a2e3-4fbf3a3fdc9d@arinc9.com/
[5] https://lore.kernel.org/all/44e366ea-964a-4515-9027-2a2edfe12512@arinc9.com/
[6] https://lore.kernel.org/all/ZixU287DdhvRyZBe@makrotopia.org/
Arınç
On 31.05.24 08:10, Arınç ÜNAL wrote:
> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>> [adding Paolo, who committed the culprit]
/me slowly wonders if the culprit should be reverted for now (see below)
and should be reapplied later together with the matching changes from
Arınç ÜNAL.
>> On 23.05.24 12:44, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 17.05.24 08:27, Frank Wunderlich wrote:
>>>> Am 17. Mai 2024 04:17:47 MESZ schrieb "Arınç ÜNAL"
>>>> <arinc.unal@arinc9.com>:
>>>>> On 16/05/2024 23:48, Frank Wunderlich wrote:
>>>>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>>>>
>>>>>> After commit 868ff5f4944a
>>>>>> ("net: dsa: mt7530-mdio: read PHY address of switch from device
>>>>>> tree")
>>>>>> the mt7531 switch on Bananapi-R64 was not detected.
>>>>>>
>>>>>> mt7530-mdio mdio-bus:00: reset timeout
>>>>>> mt7530-mdio mdio-bus:00: probe with driver mt7530-mdio failed with
>>>>>> error -110
>>>>>>
>>>>>> Fix this by adding phy address in devicetree.
>>
>> Frank, am I right assuming the regression is still present in mainline?
>> As from here it looks like for two weeks now there was no progress at
>> all to fix this (or I missed it, which is quite possible).
>>
>> Makes me wonder if the maintainers should revert the culprit or if the
>> arm64 dts folks should accept your fix despite Arınç ÜNAL's (who from a
>> quick look on lore hasn't posted anything for two weeks now) comment.
>
> I'm not against the patch. I'm against the logic it entails on the patch
> log.
In that case: can you maybe help Frank with writing something better or
submit something based on this patch to resolve this and make everyone
happy?
> I had already submitted a patch series that would've prevented this
> issue back in 14 March 2024 [1]. I've asked numerous times for the patch
> series to be applied [2][3][4][5].
>> Eventually Daniel asked for some changes [6]. But I won't have time to do
> that anytime soon and I think the patch series is good enough to be applied
> as is.
Then I guess we need some other way to resolve this in mainline to unfix
Frank's device. The two obvious options are afaics:
* revert the culprit (868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY
address of switch from device tree")) and reapply it in a later cycle
* apply Frank's patch (or an improved one) in this thread (and if needed
revert it when some better changes emerge.
Arınç ÜNAL, could you please comment on that and ideally handle the
necessary tasks, as it's your patch that causes the regression.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
> [1]
> https://lore.kernel.org/all/20240314-for-mediatek-mt7531-phy-address-v1-0-52f58db01acd@arinc9.com/
> [2]
> https://lore.kernel.org/all/ff196055-ecd8-4563-bc01-ff2533a07109@arinc9.com/
> [3]
> https://lore.kernel.org/all/a60fc16d-4236-427c-b4a8-ec6fdf62d9f0@arinc9.com/
> [4]
> https://lore.kernel.org/all/facb8204-c2b3-4084-a2e3-4fbf3a3fdc9d@arinc9.com/
> [5]
> https://lore.kernel.org/all/44e366ea-964a-4515-9027-2a2edfe12512@arinc9.com/
> [6] https://lore.kernel.org/all/ZixU287DdhvRyZBe@makrotopia.org/
>
> Arınç
>
>
On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote: > On 31.05.24 08:10, Arınç ÜNAL wrote: > > On 31/05/2024 08.40, Thorsten Leemhuis wrote: > > > [adding Paolo, who committed the culprit] > > /me slowly wonders if the culprit should be reverted for now (see below) > and should be reapplied later together with the matching changes from > Arınç ÜNAL. FWIS I think a revert should be avoided, given that a fix is available and nicely small. Thanks, Paolo
On 07.06.24 16:03, Paolo Abeni wrote: > On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote: >> On 31.05.24 08:10, Arınç ÜNAL wrote: >>> On 31/05/2024 08.40, Thorsten Leemhuis wrote: >>>> [adding Paolo, who committed the culprit] >> >> /me slowly wonders if the culprit should be reverted for now (see below) >> and should be reapplied later together with the matching changes from >> Arınç ÜNAL. > > FWIS I think a revert should be avoided, given that a fix is available > and nicely small. Yeah, on one hand I agree; but on the other it seems that the maintainers that would have to take care of the dt changes to fix this until now remained silent in this thread, apart from Rob who sent the mail regarding the warnings. I put those maintainers in the To: field of this mail, maybe that might lead to some reaction. Ciao, Thorsten
On 07.06.24 16:15, Thorsten Leemhuis wrote: > On 07.06.24 16:03, Paolo Abeni wrote: >> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote: >>> On 31.05.24 08:10, Arınç ÜNAL wrote: >>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote: >>>>> [adding Paolo, who committed the culprit] >>> >>> /me slowly wonders if the culprit should be reverted for now (see below) >>> and should be reapplied later together with the matching changes from >>> Arınç ÜNAL. >> >> FWIS I think a revert should be avoided, given that a fix is available >> and nicely small. > > Yeah, on one hand I agree; but on the other it seems that the > maintainers that would have to take care of the dt changes to fix this > until now remained silent in this thread, apart from Rob who sent the > mail regarding the warnings. > > I put those maintainers in the To: field of this mail, maybe that might > lead to some reaction. Still no reply from the DRS folks or any other progress I noticed. Guess that means I will soon have no other choice than to get Linus involved, as this looks stuck. :-( #sigh Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
On 11/06/2024 14:30, Thorsten Leemhuis wrote: > On 07.06.24 16:15, Thorsten Leemhuis wrote: >> On 07.06.24 16:03, Paolo Abeni wrote: >>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote: >>>> On 31.05.24 08:10, Arınç ÜNAL wrote: >>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote: >>>>>> [adding Paolo, who committed the culprit] >>>> >>>> /me slowly wonders if the culprit should be reverted for now (see below) >>>> and should be reapplied later together with the matching changes from >>>> Arınç ÜNAL. >>> >>> FWIS I think a revert should be avoided, given that a fix is available >>> and nicely small. >> >> Yeah, on one hand I agree; but on the other it seems that the >> maintainers that would have to take care of the dt changes to fix this >> until now remained silent in this thread, apart from Rob who sent the >> mail regarding the warnings. >> >> I put those maintainers in the To: field of this mail, maybe that might >> lead to some reaction. > > Still no reply from the DRS folks or any other progress I noticed. Guess > that means I will soon have no other choice than to get Linus involved, > as this looks stuck. :-( #sigh Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY address of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there any other ARM maintainers that can apply the fix to their tree? Arınç
Il 11/06/24 13:38, Arınç ÜNAL ha scritto:
> On 11/06/2024 14:30, Thorsten Leemhuis wrote:
>> On 07.06.24 16:15, Thorsten Leemhuis wrote:
>>> On 07.06.24 16:03, Paolo Abeni wrote:
>>>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote:
>>>>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>>>>>>> [adding Paolo, who committed the culprit]
>>>>>
>>>>> /me slowly wonders if the culprit should be reverted for now (see below)
>>>>> and should be reapplied later together with the matching changes from
>>>>> Arınç ÜNAL.
>>>>
>>>> FWIS I think a revert should be avoided, given that a fix is available
>>>> and nicely small.
>>>
>>> Yeah, on one hand I agree; but on the other it seems that the
>>> maintainers that would have to take care of the dt changes to fix this
>>> until now remained silent in this thread, apart from Rob who sent the
>>> mail regarding the warnings.
>>>
>>> I put those maintainers in the To: field of this mail, maybe that might
>>> lead to some reaction.
>>
>> Still no reply from the DRS folks or any other progress I noticed. Guess
>> that means I will soon have no other choice than to get Linus involved,
>> as this looks stuck. :-( #sigh
>
> Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY address
> of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there any other
> ARM maintainers that can apply the fix to their tree?
>
> Arınç
You have feedback from two people on the series that you mentioned, and noone
is going to apply something that needs to be fixed.
I'm giving you the possibility of addressing the comments in your patch, but
I don't want to see any mention of the driver previously ignoring this or that
as this is irrelevant for a hardware description. Devicetree only describes HW.
Adding up, in commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY address of
switch from device tree"),
you have created a regression.
Regressions should be fixed - as in - if the driver did work before with the old
devicetrees, it shall still work. You can't break ABI. Any changes that you do
to your driver must not break functionality with old devicetrees.
So...
------> Fix the driver that you broke <------
After you've fixed it - and I repeat - only after, *and* after someone (Frank?)
validates that the old devicetrees do work with the fixed driver, I will take
the device tree fixes for that MDIO address (as those are, again, fixing a
description of the hardware on those boards, so I agree that those must be fixed
AS WELL).
Regards,
Angelo
On 11/06/2024 15:28, AngeloGioacchino Del Regno wrote:
> Il 11/06/24 13:38, Arınç ÜNAL ha scritto:
>> On 11/06/2024 14:30, Thorsten Leemhuis wrote:
>>> On 07.06.24 16:15, Thorsten Leemhuis wrote:
>>>> On 07.06.24 16:03, Paolo Abeni wrote:
>>>>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote:
>>>>>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>>>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>>>>>>>> [adding Paolo, who committed the culprit]
>>>>>>
>>>>>> /me slowly wonders if the culprit should be reverted for now (see below)
>>>>>> and should be reapplied later together with the matching changes from
>>>>>> Arınç ÜNAL.
>>>>>
>>>>> FWIS I think a revert should be avoided, given that a fix is available
>>>>> and nicely small.
>>>>
>>>> Yeah, on one hand I agree; but on the other it seems that the
>>>> maintainers that would have to take care of the dt changes to fix this
>>>> until now remained silent in this thread, apart from Rob who sent the
>>>> mail regarding the warnings.
>>>>
>>>> I put those maintainers in the To: field of this mail, maybe that might
>>>> lead to some reaction.
>>>
>>> Still no reply from the DRS folks or any other progress I noticed. Guess
>>> that means I will soon have no other choice than to get Linus involved,
>>> as this looks stuck. :-( #sigh
>>
>> Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY address
>> of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there any other
>> ARM maintainers that can apply the fix to their tree?
>>
>> Arınç
>
> You have feedback from two people on the series that you mentioned, and noone
> is going to apply something that needs to be fixed.
>
> I'm giving you the possibility of addressing the comments in your patch, but
> I don't want to see any mention of the driver previously ignoring this or that
> as this is irrelevant for a hardware description. Devicetree only describes HW.
>
> Adding up, in commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY address of switch from device tree"),
> you have created a regression.
>
> Regressions should be fixed - as in - if the driver did work before with the old
> devicetrees, it shall still work. You can't break ABI. Any changes that you do
> to your driver must not break functionality with old devicetrees.
>
> So...
>
> ------> Fix the driver that you broke <------
The device tree ABI before the change on the driver:
The reg value represents the PHY address of the switch.
The device tree ABI after the change on the driver:
The reg value represents the PHY address of the switch.
I see no device tree ABI breakage. What I see instead is the driver
starting enforcing the device tree ABI. No change had been made on the
device tree ABI so any non-Linux driver that controls this switch continues
to work.
These old device tree source files in question did not abide by the device
tree ABI in the first place, which is why they don't work anymore as the
Linux driver now enforces the ABI. Device tree source files not conforming
to the ABI is not something to maintain but to fix. The patch series that
fixes them are already submitted.
>
> After you've fixed it - and I repeat - only after, *and* after someone (Frank?)
> validates that the old devicetrees do work with the fixed driver, I will take
> the device tree fixes for that MDIO address (as those are, again, fixing a
> description of the hardware on those boards, so I agree that those must be fixed
> AS WELL).
Sorry, this approach makes no sense to me.
Arınç
Il 11/06/24 14:56, Arınç ÜNAL ha scritto:
> On 11/06/2024 15:28, AngeloGioacchino Del Regno wrote:
>> Il 11/06/24 13:38, Arınç ÜNAL ha scritto:
>>> On 11/06/2024 14:30, Thorsten Leemhuis wrote:
>>>> On 07.06.24 16:15, Thorsten Leemhuis wrote:
>>>>> On 07.06.24 16:03, Paolo Abeni wrote:
>>>>>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote:
>>>>>>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>>>>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>>>>>>>>> [adding Paolo, who committed the culprit]
>>>>>>>
>>>>>>> /me slowly wonders if the culprit should be reverted for now (see below)
>>>>>>> and should be reapplied later together with the matching changes from
>>>>>>> Arınç ÜNAL.
>>>>>>
>>>>>> FWIS I think a revert should be avoided, given that a fix is available
>>>>>> and nicely small.
>>>>>
>>>>> Yeah, on one hand I agree; but on the other it seems that the
>>>>> maintainers that would have to take care of the dt changes to fix this
>>>>> until now remained silent in this thread, apart from Rob who sent the
>>>>> mail regarding the warnings.
>>>>>
>>>>> I put those maintainers in the To: field of this mail, maybe that might
>>>>> lead to some reaction.
>>>>
>>>> Still no reply from the DRS folks or any other progress I noticed. Guess
>>>> that means I will soon have no other choice than to get Linus involved,
>>>> as this looks stuck. :-( #sigh
>>>
>>> Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY address
>>> of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there any other
>>> ARM maintainers that can apply the fix to their tree?
>>>
>>> Arınç
>>
>> You have feedback from two people on the series that you mentioned, and noone
>> is going to apply something that needs to be fixed.
>>
>> I'm giving you the possibility of addressing the comments in your patch, but
>> I don't want to see any mention of the driver previously ignoring this or that
>> as this is irrelevant for a hardware description. Devicetree only describes HW.
>>
>> Adding up, in commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY address of
>> switch from device tree"),
>> you have created a regression.
>>
>> Regressions should be fixed - as in - if the driver did work before with the old
>> devicetrees, it shall still work. You can't break ABI. Any changes that you do
>> to your driver must not break functionality with old devicetrees.
>>
>> So...
>>
>> ------> Fix the driver that you broke <------
>
> The device tree ABI before the change on the driver:
>
> The reg value represents the PHY address of the switch.
>
> The device tree ABI after the change on the driver:
>
> The reg value represents the PHY address of the switch.
>
> I see no device tree ABI breakage. What I see instead is the driver
> starting enforcing the device tree ABI. No change had been made on the
> device tree ABI so any non-Linux driver that controls this switch continues
> to work.
>
> These old device tree source files in question did not abide by the device
> tree ABI in the first place, which is why they don't work anymore as the
> Linux driver now enforces the ABI. Device tree source files not conforming
> to the ABI is not something to maintain but to fix. The patch series that
> fixes them are already submitted.
As I said, the devicetree MUST describe the hardware correctly, and on that I do
agree, and I, again, said that I want to take the devicetree fix.
However, the driver regressed, and this broke functionality with old device trees.
Old device trees might have been wrong (and they are, yes), but functionality was
there and the switch was working.
I repeat, driver changes MUST be retro-compatible with older device trees, and your
driver changes ARE NOT; otherwise, this wouldn't be called *regression*.
Again, please fix the driver to be retro-compatible with old device trees.
Regards,
Angelo
On 11/06/2024 16:03, AngeloGioacchino Del Regno wrote:
> Il 11/06/24 14:56, Arınç ÜNAL ha scritto:
>> On 11/06/2024 15:28, AngeloGioacchino Del Regno wrote:
>>> Il 11/06/24 13:38, Arınç ÜNAL ha scritto:
>>>> On 11/06/2024 14:30, Thorsten Leemhuis wrote:
>>>>> On 07.06.24 16:15, Thorsten Leemhuis wrote:
>>>>>> On 07.06.24 16:03, Paolo Abeni wrote:
>>>>>>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote:
>>>>>>>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>>>>>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>>>>>>>>>> [adding Paolo, who committed the culprit]
>>>>>>>>
>>>>>>>> /me slowly wonders if the culprit should be reverted for now (see below)
>>>>>>>> and should be reapplied later together with the matching changes from
>>>>>>>> Arınç ÜNAL.
>>>>>>>
>>>>>>> FWIS I think a revert should be avoided, given that a fix is available
>>>>>>> and nicely small.
>>>>>>
>>>>>> Yeah, on one hand I agree; but on the other it seems that the
>>>>>> maintainers that would have to take care of the dt changes to fix this
>>>>>> until now remained silent in this thread, apart from Rob who sent the
>>>>>> mail regarding the warnings.
>>>>>>
>>>>>> I put those maintainers in the To: field of this mail, maybe that might
>>>>>> lead to some reaction.
>>>>>
>>>>> Still no reply from the DRS folks or any other progress I noticed. Guess
>>>>> that means I will soon have no other choice than to get Linus involved,
>>>>> as this looks stuck. :-( #sigh
>>>>
>>>> Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY address
>>>> of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there any other
>>>> ARM maintainers that can apply the fix to their tree?
>>>>
>>>> Arınç
>>>
>>> You have feedback from two people on the series that you mentioned, and noone
>>> is going to apply something that needs to be fixed.
>>>
>>> I'm giving you the possibility of addressing the comments in your patch, but
>>> I don't want to see any mention of the driver previously ignoring this or that
>>> as this is irrelevant for a hardware description. Devicetree only describes HW.
>>>
>>> Adding up, in commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY address of switch from device tree"),
>>> you have created a regression.
>>>
>>> Regressions should be fixed - as in - if the driver did work before with the old
>>> devicetrees, it shall still work. You can't break ABI. Any changes that you do
>>> to your driver must not break functionality with old devicetrees.
>>>
>>> So...
>>>
>>> ------> Fix the driver that you broke <------
>>
>> The device tree ABI before the change on the driver:
>>
>> The reg value represents the PHY address of the switch.
>>
>> The device tree ABI after the change on the driver:
>>
>> The reg value represents the PHY address of the switch.
>>
>> I see no device tree ABI breakage. What I see instead is the driver
>> starting enforcing the device tree ABI. No change had been made on the
>> device tree ABI so any non-Linux driver that controls this switch continues
>> to work.
>>
>> These old device tree source files in question did not abide by the device
>> tree ABI in the first place, which is why they don't work anymore as the
>> Linux driver now enforces the ABI. Device tree source files not conforming
>> to the ABI is not something to maintain but to fix. The patch series that
>> fixes them are already submitted.
>
> As I said, the devicetree MUST describe the hardware correctly, and on that I do
> agree, and I, again, said that I want to take the devicetree fix.
>
> However, the driver regressed, and this broke functionality with old device trees.
> Old device trees might have been wrong (and they are, yes), but functionality was
> there and the switch was working.
>
> I repeat, driver changes MUST be retro-compatible with older device trees, and your
> driver changes ARE NOT; otherwise, this wouldn't be called *regression*.
I'm going to argue that what caused the regression is the broken device
tree. The recent change on the driver only worked towards exposing the
broken device tree. The device tree files hosted on the Linux repository is
not only for use with the Linux drivers. Other projects use these device
tree files as well, as hardware description is not supposed to differ by
project. And for any non-Linux driver that would use this broken device
tree, there would be a regression.
So I don't understand why you demand a change on a Linux driver to be made
before applying the fix for a broken device tree.
That said, I don't understand the old device tree sentiment here. The
driver, after the change, still does support old device trees. Never in the
existence of this switch bindings, the PHY address was supposed to be
described a value other than the PHY address the switch listens on. Yes,
the driver now doesn't work with old and broken device trees. Which is why
we're fixing the said device trees. I don't see why it is necessary to make
the driver support broken device trees just because they used to work for a
certain range of time. This isn't about preserving ABI.
Arınç
This thread/fixing the regressions looks stalled again, let me jump in
here with a further comment below.
On 11.06.24 20:15, Arınç ÜNAL wrote:
> On 11/06/2024 16:03, AngeloGioacchino Del Regno wrote:
>> Il 11/06/24 14:56, Arınç ÜNAL ha scritto:
>>> On 11/06/2024 15:28, AngeloGioacchino Del Regno wrote:
>>>> Il 11/06/24 13:38, Arınç ÜNAL ha scritto:
>>>>> On 11/06/2024 14:30, Thorsten Leemhuis wrote:
>>>>>> On 07.06.24 16:15, Thorsten Leemhuis wrote:
>>>>>>> On 07.06.24 16:03, Paolo Abeni wrote:
>>>>>>>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote:
>>>>>>>>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>>>>>>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>>>>>>>>>>> [adding Paolo, who committed the culprit]
>>>>>>>>>
>>>>>>>>> /me slowly wonders if the culprit should be reverted for now
>>>>>>>>> (see below)
>>>>>>>>> and should be reapplied later together with the matching
>>>>>>>>> changes from
>>>>>>>>> Arınç ÜNAL.
>>>>>>>>
>>>>>>>> FWIS I think a revert should be avoided, given that a fix is
>>>>>>>> available
>>>>>>>> and nicely small.
>>>>>>>
>>>>>>> Yeah, on one hand I agree; but on the other it seems that the
>>>>>>> maintainers that would have to take care of the dt changes to fix
>>>>>>> this
>>>>>>> until now remained silent in this thread, apart from Rob who sent
>>>>>>> the
>>>>>>> mail regarding the warnings.
>>>>>>>
>>>>>>> I put those maintainers in the To: field of this mail, maybe that
>>>>>>> might
>>>>>>> lead to some reaction.
>>>>>>
>>>>>> Still no reply from the DRS folks or any other progress I noticed.
>>>>>> Guess
>>>>>> that means I will soon have no other choice than to get Linus
>>>>>> involved,
>>>>>> as this looks stuck. :-( #sigh
>>>>>
>>>>> Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY
>>>>> address
>>>>> of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there
>>>>> any other
>>>>> ARM maintainers that can apply the fix to their tree?
>>>>>
>>>>> Arınç
>>>>
>>>> You have feedback from two people on the series that you mentioned,
>>>> and noone
>>>> is going to apply something that needs to be fixed.
>>>>
>>>> I'm giving you the possibility of addressing the comments in your
>>>> patch, but
>>>> I don't want to see any mention of the driver previously ignoring
>>>> this or that
>>>> as this is irrelevant for a hardware description. Devicetree only
>>>> describes HW.
>>>>
>>>> Adding up, in commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY
>>>> address of switch from device tree"),
>>>> you have created a regression.
>>>>
>>>> Regressions should be fixed - as in - if the driver did work before
>>>> with the old
>>>> devicetrees, it shall still work. You can't break ABI. Any changes
>>>> that you do
>>>> to your driver must not break functionality with old devicetrees.
>>>>
>>>> So...
>>>>
>>>> ------> Fix the driver that you broke <------
>>>
>>> The device tree ABI before the change on the driver:
>>>
>>> The reg value represents the PHY address of the switch.
>>>
>>> The device tree ABI after the change on the driver:
>>>
>>> The reg value represents the PHY address of the switch.
>>>
>>> I see no device tree ABI breakage. What I see instead is the driver
>>> starting enforcing the device tree ABI. No change had been made on the
>>> device tree ABI so any non-Linux driver that controls this switch
>>> continues
>>> to work.
>>>
>>> These old device tree source files in question did not abide by the
>>> device
>>> tree ABI in the first place, which is why they don't work anymore as the
>>> Linux driver now enforces the ABI. Device tree source files not
>>> conforming
>>> to the ABI is not something to maintain but to fix. The patch series
>>> that
>>> fixes them are already submitted.
>>
>> As I said, the devicetree MUST describe the hardware correctly, and on
>> that I do
>> agree, and I, again, said that I want to take the devicetree fix.
>>
>> However, the driver regressed, and this broke functionality with old
>> device trees.
>> Old device trees might have been wrong (and they are, yes), but
>> functionality was
>> there and the switch was working.
>>
>> I repeat, driver changes MUST be retro-compatible with older device
>> trees, and your
>> driver changes ARE NOT; otherwise, this wouldn't be called *regression*.
>
> I'm going to argue that what caused the regression is the broken device
> tree. The recent change on the driver only worked towards exposing the
> broken device tree.
Well, for the kernel that does not matter much: due to our "no
regressions" rule and how Linus handles it the author of that "recent
change" (e.g. you) is responsible to fix regressions a change exposed --
or that change is reverted (I might be wrong, but I think there are
quotes from Linus in
https://docs.kernel.org/process/handling-regressions.html to back this
up). So in the end a revert in a week or two is likely the outcome,
unless you or someone else fixes it in a way that pleases
AngeloGioacchino et. at.
Ciao, Thorsten
On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis) wrote:
> This thread/fixing the regressions looks stalled again, let me jump in
> here with a further comment below.
>
> On 11.06.24 20:15, Arınç ÜNAL wrote:
>> On 11/06/2024 16:03, AngeloGioacchino Del Regno wrote:
>>> Il 11/06/24 14:56, Arınç ÜNAL ha scritto:
>>>> On 11/06/2024 15:28, AngeloGioacchino Del Regno wrote:
>>>>> Il 11/06/24 13:38, Arınç ÜNAL ha scritto:
>>>>>> On 11/06/2024 14:30, Thorsten Leemhuis wrote:
>>>>>>> On 07.06.24 16:15, Thorsten Leemhuis wrote:
>>>>>>>> On 07.06.24 16:03, Paolo Abeni wrote:
>>>>>>>>> On Thu, 2024-06-06 at 10:26 +0200, Thorsten Leemhuis wrote:
>>>>>>>>>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>>>>>>>>>> On 31/05/2024 08.40, Thorsten Leemhuis wrote:
>>>>>>>>>>>> [adding Paolo, who committed the culprit]
>>>>>>>>>>
>>>>>>>>>> /me slowly wonders if the culprit should be reverted for now
>>>>>>>>>> (see below)
>>>>>>>>>> and should be reapplied later together with the matching
>>>>>>>>>> changes from
>>>>>>>>>> Arınç ÜNAL.
>>>>>>>>>
>>>>>>>>> FWIS I think a revert should be avoided, given that a fix is
>>>>>>>>> available
>>>>>>>>> and nicely small.
>>>>>>>>
>>>>>>>> Yeah, on one hand I agree; but on the other it seems that the
>>>>>>>> maintainers that would have to take care of the dt changes to fix
>>>>>>>> this
>>>>>>>> until now remained silent in this thread, apart from Rob who sent
>>>>>>>> the
>>>>>>>> mail regarding the warnings.
>>>>>>>>
>>>>>>>> I put those maintainers in the To: field of this mail, maybe that
>>>>>>>> might
>>>>>>>> lead to some reaction.
>>>>>>>
>>>>>>> Still no reply from the DRS folks or any other progress I noticed.
>>>>>>> Guess
>>>>>>> that means I will soon have no other choice than to get Linus
>>>>>>> involved,
>>>>>>> as this looks stuck. :-( #sigh
>>>>>>
>>>>>> Does it have to be Linus that needs to apply "[PATCH 0/2] Set PHY
>>>>>> address
>>>>>> of MT7531 switch to 0x1f on MediaTek arm64 boards"? Aren't there
>>>>>> any other
>>>>>> ARM maintainers that can apply the fix to their tree?
>>>>>>
>>>>>> Arınç
>>>>>
>>>>> You have feedback from two people on the series that you mentioned,
>>>>> and noone
>>>>> is going to apply something that needs to be fixed.
>>>>>
>>>>> I'm giving you the possibility of addressing the comments in your
>>>>> patch, but
>>>>> I don't want to see any mention of the driver previously ignoring
>>>>> this or that
>>>>> as this is irrelevant for a hardware description. Devicetree only
>>>>> describes HW.
>>>>>
>>>>> Adding up, in commit 868ff5f4944a ("net: dsa: mt7530-mdio: read PHY
>>>>> address of switch from device tree"),
>>>>> you have created a regression.
>>>>>
>>>>> Regressions should be fixed - as in - if the driver did work before
>>>>> with the old
>>>>> devicetrees, it shall still work. You can't break ABI. Any changes
>>>>> that you do
>>>>> to your driver must not break functionality with old devicetrees.
>>>>>
>>>>> So...
>>>>>
>>>>> ------> Fix the driver that you broke <------
>>>>
>>>> The device tree ABI before the change on the driver:
>>>>
>>>> The reg value represents the PHY address of the switch.
>>>>
>>>> The device tree ABI after the change on the driver:
>>>>
>>>> The reg value represents the PHY address of the switch.
>>>>
>>>> I see no device tree ABI breakage. What I see instead is the driver
>>>> starting enforcing the device tree ABI. No change had been made on the
>>>> device tree ABI so any non-Linux driver that controls this switch
>>>> continues
>>>> to work.
>>>>
>>>> These old device tree source files in question did not abide by the
>>>> device
>>>> tree ABI in the first place, which is why they don't work anymore as the
>>>> Linux driver now enforces the ABI. Device tree source files not
>>>> conforming
>>>> to the ABI is not something to maintain but to fix. The patch series
>>>> that
>>>> fixes them are already submitted.
>>>
>>> As I said, the devicetree MUST describe the hardware correctly, and on
>>> that I do
>>> agree, and I, again, said that I want to take the devicetree fix.
>>>
>>> However, the driver regressed, and this broke functionality with old
>>> device trees.
>>> Old device trees might have been wrong (and they are, yes), but
>>> functionality was
>>> there and the switch was working.
>>>
>>> I repeat, driver changes MUST be retro-compatible with older device
>>> trees, and your
>>> driver changes ARE NOT; otherwise, this wouldn't be called *regression*.
>>
>> I'm going to argue that what caused the regression is the broken device
>> tree. The recent change on the driver only worked towards exposing the
>> broken device tree.
>
> Well, for the kernel that does not matter much: due to our "no
> regressions" rule and how Linus handles it the author of that "recent
> change" (e.g. you) is responsible to fix regressions a change exposed --
> or that change is reverted (I might be wrong, but I think there are
> quotes from Linus in
> https://docs.kernel.org/process/handling-regressions.html to back this
> up). So in the end a revert in a week or two is likely the outcome,
> unless you or someone else fixes it in a way that pleases
> AngeloGioacchino et. at.
I've submitted a patch series that fixes the regression. Angelo argued
against the way the regression is fixed. I've very clearly argued back why
I find Angelo's approach wrong. There's been no response back. I don't
understand why reverting the patch is the likely outcome whilst the
standing argument points towards applying the said patch series. If a
revert happens before this discussion with Angelo finalises, this will set
a precedent that will tell maintainers that they can have their way by just
not replying to the ongoing discussions.
That said, the decision of resolving the regression by either reverting the
patch or applying the patch series shall not depend on whether or not
Angelo is pleased but rather there're no counter-arguments left on the
points brought, meaning the decision shall be made depending on the
argument that stands.
Therefore, I suggest that unless Angelo responds back with a
counter-argument in the window of a week or two, as you've described, my
patch series shall be applied.
Arınç
On 17.06.24 13:08, Arınç ÜNAL wrote:
> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis) wrote:
> [...]
> I've submitted a patch series that fixes the regression. Angelo argued
> against the way the regression is fixed. I've very clearly argued back why
> I find Angelo's approach wrong. There's been no response back. I don't
> understand why reverting the patch is the likely outcome
Long story short: because that how things like that are handled in the
Linux kernel project, as Linus wants it like that. See some of the
quotes from https://docs.kernel.org/process/handling-regressions.html
for details.
> whilst the
> standing argument points towards applying the said patch series. If a
> revert happens before this discussion with Angelo finalises, this will set
> a precedent that will tell maintainers that they can have their way by just
> not replying to the ongoing discussions.
>
> That said, the decision of resolving the regression by either reverting the
> patch or applying the patch series shall not depend on whether or not
> Angelo is pleased but rather there're no counter-arguments left on the
> points brought, meaning the decision shall be made depending on the
> argument that stands.
>
> Therefore, I suggest that unless Angelo responds back with a
> counter-argument in the window of a week or two, as you've described, my
> patch series shall be applied.
It looks more and more like we are stuck here (or was there progress and
I just missed it?) while the 6.10 final is slowly getting closer. Hence:
AngeloGioacchino, should we ask the net maintainers to revert
868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
device tree") for now to resolve this regression? Reminder, there is
nothing wrong with that commit per se afaik, it just exposes a problem
that needs to be fixed first before it can be reapplied.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha scritto:
> On 17.06.24 13:08, Arınç ÜNAL wrote:
>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [...]
>> I've submitted a patch series that fixes the regression. Angelo argued
>> against the way the regression is fixed. I've very clearly argued back why
>> I find Angelo's approach wrong. There's been no response back. I don't
>> understand why reverting the patch is the likely outcome
>
> Long story short: because that how things like that are handled in the
> Linux kernel project, as Linus wants it like that. See some of the
> quotes from https://docs.kernel.org/process/handling-regressions.html
> for details.
>
>> whilst the
>> standing argument points towards applying the said patch series. If a
>> revert happens before this discussion with Angelo finalises, this will set
>> a precedent that will tell maintainers that they can have their way by just
>> not replying to the ongoing discussions.
>>
>> That said, the decision of resolving the regression by either reverting the
>> patch or applying the patch series shall not depend on whether or not
>> Angelo is pleased but rather there're no counter-arguments left on the
>> points brought, meaning the decision shall be made depending on the
>> argument that stands.
>>
>> Therefore, I suggest that unless Angelo responds back with a
>> counter-argument in the window of a week or two, as you've described, my
>> patch series shall be applied.
>
> It looks more and more like we are stuck here (or was there progress and
> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>
> AngeloGioacchino, should we ask the net maintainers to revert
> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
> device tree") for now to resolve this regression? Reminder, there is
> nothing wrong with that commit per se afaik, it just exposes a problem
> that needs to be fixed first before it can be reapplied.
>
To be clear on this: I asked for the commit to be fixed such that it guarantees
backwards compatibility with older device trees.
If no fix comes, then I guess that we should ask them to revert this commit
until a fix is available.
I don't like this situation, either, btw.
Ciao!
Angelo
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
[CCing the other net maintainers]
On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha
> scritto:
>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>> wrote:
>>> [...]
>> It looks more and more like we are stuck here (or was there progress and
>> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>>
>> AngeloGioacchino, should we ask the net maintainers to revert
>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>> device tree") for now to resolve this regression? Reminder, there is
>> nothing wrong with that commit per se afaik, it just exposes a problem
>> that needs to be fixed first before it can be reapplied.
>
> To be clear on this: I asked for the commit to be fixed such that it
> guarantees
> backwards compatibility with older device trees.
>
> If no fix comes,
I haven't see any since that mail, did you? If not, I think...
> then I guess that we should ask them to revert this commit
> until a fix is available.
...it's time to ask them for the revert to resolve this for -rc7 (and
avoid a last minute revert), or what do you think?
> I don't like this situation, either, btw.
I guess none of us does. :-/
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On 01/07/2024 09:16, Linux regression tracking (Thorsten Leemhuis) wrote:
> [CCing the other net maintainers]
>
> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
>> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha
>> scritto:
>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>> wrote:
>>>> [...]
>>> It looks more and more like we are stuck here (or was there progress and
>>> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>>>
>>> AngeloGioacchino, should we ask the net maintainers to revert
>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>> device tree") for now to resolve this regression? Reminder, there is
>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>> that needs to be fixed first before it can be reapplied.
>>
>> To be clear on this: I asked for the commit to be fixed such that it
>> guarantees
>> backwards compatibility with older device trees.
>>
>> If no fix comes,
>
> I haven't see any since that mail, did you? If not, I think...
>
>> then I guess that we should ask them to revert this commit
>> until a fix is available.
>
> ...it's time to ask them for the revert to resolve this for -rc7 (and
> avoid a last minute revert), or what do you think?
This is quite frustrating. I absolutely won't consent to a revert. I've
spent a great amount of time and effort explaining why this is neither
necessary nor a good approach in this email thread. I'm not going to accept
a revert due to the other side's failure to communicate, which will create
unnecessary work for me to do. It is ridiculous to demand a change in a
Linux driver before accepting a device tree patch.
Arınç
On 01.07.24 09:44, Arınç ÜNAL wrote:
> On 01/07/2024 09:16, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [CCing the other net maintainers]
>>
>> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
>>> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha
>>> scritto:
>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>>> wrote:
>>>>> [...]
>>>> It looks more and more like we are stuck here (or was there progress
>>>> and
>>>> I just missed it?) while the 6.10 final is slowly getting closer.
>>>> Hence:
>>>>
>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>>> device tree") for now to resolve this regression? Reminder, there is
>>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>>> that needs to be fixed first before it can be reapplied.
>>>
>>> To be clear on this: I asked for the commit to be fixed such that it
>>> guarantees
>>> backwards compatibility with older device trees.
>>>
>>> If no fix comes,
>>
>> I haven't see any since that mail, did you? If not, I think...
>>
>>> then I guess that we should ask them to revert this commit
>>> until a fix is available.
>>
>> ...it's time to ask them for the revert to resolve this for -rc7 (and
>> avoid a last minute revert), or what do you think?
>
> This is quite frustrating. I absolutely won't consent to a revert. [...]
Reminder: try to not see a revert as a bad thing. It's just means "not
ready yet, revert and we'll try again later" -- that's actually
something Linus wrote just a few hours ago:
https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/
Ciao, Thorsten
On 01/07/2024 11:04, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 01.07.24 09:44, Arınç ÜNAL wrote:
>> On 01/07/2024 09:16, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> [CCing the other net maintainers]
>>>
>>> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
>>>> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha
>>>> scritto:
>>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>>>> wrote:
>>>>>> [...]
>>>>> It looks more and more like we are stuck here (or was there progress
>>>>> and
>>>>> I just missed it?) while the 6.10 final is slowly getting closer.
>>>>> Hence:
>>>>>
>>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>>>> device tree") for now to resolve this regression? Reminder, there is
>>>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>>>> that needs to be fixed first before it can be reapplied.
>>>>
>>>> To be clear on this: I asked for the commit to be fixed such that it
>>>> guarantees
>>>> backwards compatibility with older device trees.
>>>>
>>>> If no fix comes,
>>>
>>> I haven't see any since that mail, did you? If not, I think...
>>>
>>>> then I guess that we should ask them to revert this commit
>>>> until a fix is available.
>>>
>>> ...it's time to ask them for the revert to resolve this for -rc7 (and
>>> avoid a last minute revert), or what do you think?
>>
>> This is quite frustrating. I absolutely won't consent to a revert. [...]
>
> Reminder: try to not see a revert as a bad thing. It's just means "not
> ready yet, revert and we'll try again later" -- that's actually
> something Linus wrote just a few hours ago:
> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/
Except it is ready and trying again is my responsibility, which means
unnecessary work for me to do. I've already got a ton of things to do.
Applying the device tree patch resolves this regression; no reverts needed.
And then there's the patch in the works by Daniel that will address all the
remaining cases outside of the reported regression.
Arınç
Il 01/07/24 10:15, Arınç ÜNAL ha scritto:
> On 01/07/2024 11:04, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 01.07.24 09:44, Arınç ÜNAL wrote:
>>> On 01/07/2024 09:16, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>> [CCing the other net maintainers]
>>>>
>>>> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
>>>>> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha
>>>>> scritto:
>>>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>>>>> wrote:
>>>>>>> [...]
>>>>>> It looks more and more like we are stuck here (or was there progress
>>>>>> and
>>>>>> I just missed it?) while the 6.10 final is slowly getting closer.
>>>>>> Hence:
>>>>>>
>>>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>>>>> device tree") for now to resolve this regression? Reminder, there is
>>>>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>>>>> that needs to be fixed first before it can be reapplied.
>>>>>
>>>>> To be clear on this: I asked for the commit to be fixed such that it
>>>>> guarantees
>>>>> backwards compatibility with older device trees.
>>>>>
>>>>> If no fix comes,
>>>>
>>>> I haven't see any since that mail, did you? If not, I think...
>>>>
>>>>> then I guess that we should ask them to revert this commit
>>>>> until a fix is available.
>>>>
>>>> ...it's time to ask them for the revert to resolve this for -rc7 (and
>>>> avoid a last minute revert), or what do you think?
>>>
>>> This is quite frustrating. I absolutely won't consent to a revert. [...]
>>
>> Reminder: try to not see a revert as a bad thing. It's just means "not
>> ready yet, revert and we'll try again later" -- that's actually
>> something Linus wrote just a few hours ago:
>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/
>
> Except it is ready and trying again is my responsibility, which means
> unnecessary work for me to do. I've already got a ton of things to do.
> Applying the device tree patch resolves this regression; no reverts needed.
> And then there's the patch in the works by Daniel that will address all the
> remaining cases outside of the reported regression.
>
The commit that fixes your breakage in a way that does *not* please me
(because of older devicetrees being still broken with the new driver) was
picked and it is in v6.11-rc1.
I had to do this because I value the community (in this case, the users) much
more than trying to make an arrogant developer to act in a community-friendly
manner while leaving things completely broken.
That said, remembering that we're humans and, as such, it's normal to get something
wrong during the development process, I want to remind you that:
1. A series that creates regressions is *not* good and *not* ready to be
upstreamed; and
2. As a maintainer, you have the responsibility of not breaking the kernel,
not breaking devices and not breaking currently working functionality; and
3. Devicetrees being wrong (but working) since day 0 is not an excuse to break
functionality; and
4. Blaming the one who introduced the devicetree (you're doing that, since you
are blaming the devicetree being wrong) isn't solving anything and will not
magically make your code acceptable or good; and
5. If you push a wrong commit, there's nothing to be ashamed of; and
6. If you make a mistake, you should recognize that and find a way to
make things right, that should be done for the community, not for
yourself; and
7. You shall respect the community: in this case, with your arrogant behavior
you did *not* respect the users.
P.S.: The right way of making such change is to:
1. Avoid breaking currently working devices by making sure that their DT
still works with the new driver; and
2. If breakage is unavoidable, make it so one kernel version has a fix that
works with both old and new driver, and the next version introduces the
breakage. N.2 should ideally never happen, anyway.
Let's wrap up this matter now - I don't want to spend any more word, nor time,
on this, as I really have nothing else to say.
Best regards,
Angelo
On 2024-07-30 12:41, AngeloGioacchino Del Regno wrote:
> Il 01/07/24 10:15, Arınç ÜNAL ha scritto:
>> On 01/07/2024 11:04, Linux regression tracking (Thorsten Leemhuis)
>> wrote:
>>> On 01.07.24 09:44, Arınç ÜNAL wrote:
>>>> On 01/07/2024 09:16, Linux regression tracking (Thorsten Leemhuis)
>>>> wrote:
>>>>> [CCing the other net maintainers]
>>>>>
>>>>> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
>>>>>> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis)
>>>>>> ha
>>>>>> scritto:
>>>>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten
>>>>>>>> Leemhuis)
>>>>>>>> wrote:
>>>>>>>> [...]
>>>>>>> It looks more and more like we are stuck here (or was there
>>>>>>> progress
>>>>>>> and
>>>>>>> I just missed it?) while the 6.10 final is slowly getting closer.
>>>>>>> Hence:
>>>>>>>
>>>>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of
>>>>>>> switch from
>>>>>>> device tree") for now to resolve this regression? Reminder, there
>>>>>>> is
>>>>>>> nothing wrong with that commit per se afaik, it just exposes a
>>>>>>> problem
>>>>>>> that needs to be fixed first before it can be reapplied.
>>>>>>
>>>>>> To be clear on this: I asked for the commit to be fixed such that
>>>>>> it
>>>>>> guarantees
>>>>>> backwards compatibility with older device trees.
>>>>>>
>>>>>> If no fix comes,
>>>>>
>>>>> I haven't see any since that mail, did you? If not, I think...
>>>>>
>>>>>> then I guess that we should ask them to revert this commit
>>>>>> until a fix is available.
>>>>>
>>>>> ...it's time to ask them for the revert to resolve this for -rc7
>>>>> (and
>>>>> avoid a last minute revert), or what do you think?
>>>>
>>>> This is quite frustrating. I absolutely won't consent to a revert.
>>>> [...]
>>>
>>> Reminder: try to not see a revert as a bad thing. It's just means
>>> "not
>>> ready yet, revert and we'll try again later" -- that's actually
>>> something Linus wrote just a few hours ago:
>>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/
>>
>> Except it is ready and trying again is my responsibility, which means
>> unnecessary work for me to do. I've already got a ton of things to do.
>> Applying the device tree patch resolves this regression; no reverts
>> needed.
>> And then there's the patch in the works by Daniel that will address
>> all the
>> remaining cases outside of the reported regression.
>>
>
> The commit that fixes your breakage in a way that does *not* please me
> (because of older devicetrees being still broken with the new driver)
> was
> picked and it is in v6.11-rc1.
>
> I had to do this because I value the community (in this case, the
> users) much
> more than trying to make an arrogant developer to act in a
> community-friendly
> manner while leaving things completely broken.
>
> That said, remembering that we're humans and, as such, it's normal to
> get something
> wrong during the development process, I want to remind you that:
>
> 1. A series that creates regressions is *not* good and *not* ready to
> be
> upstreamed; and
> 2. As a maintainer, you have the responsibility of not breaking the
> kernel,
> not breaking devices and not breaking currently working
> functionality; and
> 3. Devicetrees being wrong (but working) since day 0 is not an excuse
> to break
> functionality; and
> 4. Blaming the one who introduced the devicetree (you're doing that,
> since you
> are blaming the devicetree being wrong) isn't solving anything and
> will not
> magically make your code acceptable or good; and
> 5. If you push a wrong commit, there's nothing to be ashamed of; and
> 6. If you make a mistake, you should recognize that and find a way to
> make things right, that should be done for the community, not for
> yourself; and
> 7. You shall respect the community: in this case, with your arrogant
> behavior
> you did *not* respect the users.
>
> P.S.: The right way of making such change is to:
> 1. Avoid breaking currently working devices by making sure that
> their DT
> still works with the new driver; and
> 2. If breakage is unavoidable, make it so one kernel version has
> a fix that
> works with both old and new driver, and the next version
> introduces the
> breakage. N.2 should ideally never happen, anyway.
>
> Let's wrap up this matter now - I don't want to spend any more word,
> nor time,
> on this, as I really have nothing else to say.
>
> Best regards,
> Angelo
To be clear, I only became aware that my patch was picked by reading
this
email. It is clear that we have different views. To that extend, all of
what you have written above can be answered to by reading what I have
already written in this thread. Therefore, I don't see any wrongdoing
from
my side and invite everyone to fully read this thread to draw their own
conclusions; something you seem not to have done. And I'm not the one,
calling people names here. I can only offer my respect for hard working
people.
In my view, your behaviour of not applying a devicetree patch before a
Linux driver patch was applied, and then not replying to any arguments
whatsoever, was keeping the devicetree files hostage until your demands
were met. What I see is that, you failed as a maintainer to attend to my
points against this practice. It's no surprise that, after not having
heard
back from you with an argument against my points, my patch was
eventually
taken in by someone else.
Arınç
On 30/07/2024 13:22, arinc.unal@arinc9.com wrote: >>>> >>>> Reminder: try to not see a revert as a bad thing. It's just means >>>> "not >>>> ready yet, revert and we'll try again later" -- that's actually >>>> something Linus wrote just a few hours ago: >>>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/ >>> >>> Except it is ready and trying again is my responsibility, which means >>> unnecessary work for me to do. I've already got a ton of things to do. >>> Applying the device tree patch resolves this regression; no reverts >>> needed. >>> And then there's the patch in the works by Daniel that will address >>> all the >>> remaining cases outside of the reported regression. >>> >> >> The commit that fixes your breakage in a way that does *not* please me >> (because of older devicetrees being still broken with the new driver) >> was >> picked and it is in v6.11-rc1. >> >> I had to do this because I value the community (in this case, the >> users) much >> more than trying to make an arrogant developer to act in a >> community-friendly >> manner while leaving things completely broken. >> >> That said, remembering that we're humans and, as such, it's normal to >> get something >> wrong during the development process, I want to remind you that: >> >> 1. A series that creates regressions is *not* good and *not* ready to >> be >> upstreamed; and >> 2. As a maintainer, you have the responsibility of not breaking the >> kernel, >> not breaking devices and not breaking currently working >> functionality; and >> 3. Devicetrees being wrong (but working) since day 0 is not an excuse >> to break >> functionality; and >> 4. Blaming the one who introduced the devicetree (you're doing that, >> since you >> are blaming the devicetree being wrong) isn't solving anything and >> will not >> magically make your code acceptable or good; and >> 5. If you push a wrong commit, there's nothing to be ashamed of; and >> 6. If you make a mistake, you should recognize that and find a way to >> make things right, that should be done for the community, not for >> yourself; and >> 7. You shall respect the community: in this case, with your arrogant >> behavior >> you did *not* respect the users. >> >> P.S.: The right way of making such change is to: >> 1. Avoid breaking currently working devices by making sure that >> their DT >> still works with the new driver; and >> 2. If breakage is unavoidable, make it so one kernel version has >> a fix that >> works with both old and new driver, and the next version >> introduces the >> breakage. N.2 should ideally never happen, anyway. >> >> Let's wrap up this matter now - I don't want to spend any more word, >> nor time, >> on this, as I really have nothing else to say. >> >> Best regards, >> Angelo > > To be clear, I only became aware that my patch was picked by reading > this > email. It is clear that we have different views. To that extend, all of > what you have written above can be answered to by reading what I have > already written in this thread. Therefore, I don't see any wrongdoing > from > my side and invite everyone to fully read this thread to draw their own > conclusions; something you seem not to have done. And I'm not the one, > calling people names here. I can only offer my respect for hard working > people. > > In my view, your behaviour of not applying a devicetree patch before a > Linux driver patch was applied, and then not replying to any arguments > whatsoever, was keeping the devicetree files hostage until your demands Hm, why ever DTS patch should be applied before driver patch is? This clearly suggests ABI break. You proposed to fix ABI issue by fixing DTS, which is not the way, because it literally fixes nothing. You got comments - fix the driver to be backwards compatible. > were met. What I see is that, you failed as a maintainer to attend to my > points against this practice. It's no surprise that, after not having > heard > back from you with an argument against my points, my patch was > eventually > taken in by someone else. Best regards, Krzysztof
On 30/07/2024 19:04, Krzysztof Kozlowski wrote: > On 30/07/2024 13:22, arinc.unal@arinc9.com wrote: >>>>> >>>>> Reminder: try to not see a revert as a bad thing. It's just means >>>>> "not >>>>> ready yet, revert and we'll try again later" -- that's actually >>>>> something Linus wrote just a few hours ago: >>>>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/ >>>> >>>> Except it is ready and trying again is my responsibility, which means >>>> unnecessary work for me to do. I've already got a ton of things to do. >>>> Applying the device tree patch resolves this regression; no reverts >>>> needed. >>>> And then there's the patch in the works by Daniel that will address >>>> all the >>>> remaining cases outside of the reported regression. >>>> >>> >>> The commit that fixes your breakage in a way that does *not* please me >>> (because of older devicetrees being still broken with the new driver) >>> was >>> picked and it is in v6.11-rc1. >>> >>> I had to do this because I value the community (in this case, the >>> users) much >>> more than trying to make an arrogant developer to act in a >>> community-friendly >>> manner while leaving things completely broken. >>> >>> That said, remembering that we're humans and, as such, it's normal to >>> get something >>> wrong during the development process, I want to remind you that: >>> >>> 1. A series that creates regressions is *not* good and *not* ready to >>> be >>> upstreamed; and >>> 2. As a maintainer, you have the responsibility of not breaking the >>> kernel, >>> not breaking devices and not breaking currently working >>> functionality; and >>> 3. Devicetrees being wrong (but working) since day 0 is not an excuse >>> to break >>> functionality; and >>> 4. Blaming the one who introduced the devicetree (you're doing that, >>> since you >>> are blaming the devicetree being wrong) isn't solving anything and >>> will not >>> magically make your code acceptable or good; and >>> 5. If you push a wrong commit, there's nothing to be ashamed of; and >>> 6. If you make a mistake, you should recognize that and find a way to >>> make things right, that should be done for the community, not for >>> yourself; and >>> 7. You shall respect the community: in this case, with your arrogant >>> behavior >>> you did *not* respect the users. >>> >>> P.S.: The right way of making such change is to: >>> 1. Avoid breaking currently working devices by making sure that >>> their DT >>> still works with the new driver; and >>> 2. If breakage is unavoidable, make it so one kernel version has >>> a fix that >>> works with both old and new driver, and the next version >>> introduces the >>> breakage. N.2 should ideally never happen, anyway. >>> >>> Let's wrap up this matter now - I don't want to spend any more word, >>> nor time, >>> on this, as I really have nothing else to say. >>> >>> Best regards, >>> Angelo >> >> To be clear, I only became aware that my patch was picked by reading >> this >> email. It is clear that we have different views. To that extend, all of >> what you have written above can be answered to by reading what I have >> already written in this thread. Therefore, I don't see any wrongdoing >> from >> my side and invite everyone to fully read this thread to draw their own >> conclusions; something you seem not to have done. And I'm not the one, >> calling people names here. I can only offer my respect for hard working >> people. >> >> In my view, your behaviour of not applying a devicetree patch before a >> Linux driver patch was applied, and then not replying to any arguments >> whatsoever, was keeping the devicetree files hostage until your demands > > Hm, why ever DTS patch should be applied before driver patch is? This > clearly suggests ABI break. You proposed to fix ABI issue by fixing DTS, > which is not the way, because it literally fixes nothing. You got > comments - fix the driver to be backwards compatible. As I argued in this thread, I see no ABI issue here. I proposed to fix broken devicetrees, nothing more. Please read the full thread to understand where I'm coming from. Arınç
On 30/07/2024 18:38, Arınç ÜNAL wrote: > On 30/07/2024 19:04, Krzysztof Kozlowski wrote: >> On 30/07/2024 13:22, arinc.unal@arinc9.com wrote: >>>>>> >>>>>> Reminder: try to not see a revert as a bad thing. It's just means >>>>>> "not >>>>>> ready yet, revert and we'll try again later" -- that's actually >>>>>> something Linus wrote just a few hours ago: >>>>>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/ >>>>> >>>>> Except it is ready and trying again is my responsibility, which means >>>>> unnecessary work for me to do. I've already got a ton of things to do. >>>>> Applying the device tree patch resolves this regression; no reverts >>>>> needed. >>>>> And then there's the patch in the works by Daniel that will address >>>>> all the >>>>> remaining cases outside of the reported regression. >>>>> >>>> >>>> The commit that fixes your breakage in a way that does *not* please me >>>> (because of older devicetrees being still broken with the new driver) >>>> was >>>> picked and it is in v6.11-rc1. >>>> >>>> I had to do this because I value the community (in this case, the >>>> users) much >>>> more than trying to make an arrogant developer to act in a >>>> community-friendly >>>> manner while leaving things completely broken. >>>> >>>> That said, remembering that we're humans and, as such, it's normal to >>>> get something >>>> wrong during the development process, I want to remind you that: >>>> >>>> 1. A series that creates regressions is *not* good and *not* ready to >>>> be >>>> upstreamed; and >>>> 2. As a maintainer, you have the responsibility of not breaking the >>>> kernel, >>>> not breaking devices and not breaking currently working >>>> functionality; and >>>> 3. Devicetrees being wrong (but working) since day 0 is not an excuse >>>> to break >>>> functionality; and >>>> 4. Blaming the one who introduced the devicetree (you're doing that, >>>> since you >>>> are blaming the devicetree being wrong) isn't solving anything and >>>> will not >>>> magically make your code acceptable or good; and >>>> 5. If you push a wrong commit, there's nothing to be ashamed of; and >>>> 6. If you make a mistake, you should recognize that and find a way to >>>> make things right, that should be done for the community, not for >>>> yourself; and >>>> 7. You shall respect the community: in this case, with your arrogant >>>> behavior >>>> you did *not* respect the users. >>>> >>>> P.S.: The right way of making such change is to: >>>> 1. Avoid breaking currently working devices by making sure that >>>> their DT >>>> still works with the new driver; and >>>> 2. If breakage is unavoidable, make it so one kernel version has >>>> a fix that >>>> works with both old and new driver, and the next version >>>> introduces the >>>> breakage. N.2 should ideally never happen, anyway. >>>> >>>> Let's wrap up this matter now - I don't want to spend any more word, >>>> nor time, >>>> on this, as I really have nothing else to say. >>>> >>>> Best regards, >>>> Angelo >>> >>> To be clear, I only became aware that my patch was picked by reading >>> this >>> email. It is clear that we have different views. To that extend, all of >>> what you have written above can be answered to by reading what I have >>> already written in this thread. Therefore, I don't see any wrongdoing >>> from >>> my side and invite everyone to fully read this thread to draw their own >>> conclusions; something you seem not to have done. And I'm not the one, >>> calling people names here. I can only offer my respect for hard working >>> people. >>> >>> In my view, your behaviour of not applying a devicetree patch before a >>> Linux driver patch was applied, and then not replying to any arguments >>> whatsoever, was keeping the devicetree files hostage until your demands >> >> Hm, why ever DTS patch should be applied before driver patch is? This >> clearly suggests ABI break. You proposed to fix ABI issue by fixing DTS, >> which is not the way, because it literally fixes nothing. You got >> comments - fix the driver to be backwards compatible. > > As I argued in this thread, I see no ABI issue here. I proposed to fix > broken devicetrees, nothing more. Please read the full thread to understand > where I'm coming from. I read most of it and it does suggest ABI break. But even if you do not focus on that aspect, your suggestion that DTS should be applied before driver patch is just wrong. It was never like this and it must not be like this. Best regards, Krzysztof
On 31/07/2024 08:29, Krzysztof Kozlowski wrote: > On 30/07/2024 18:38, Arınç ÜNAL wrote: >> On 30/07/2024 19:04, Krzysztof Kozlowski wrote: >>> On 30/07/2024 13:22, arinc.unal@arinc9.com wrote: >>>>>>> >>>>>>> Reminder: try to not see a revert as a bad thing. It's just means >>>>>>> "not >>>>>>> ready yet, revert and we'll try again later" -- that's actually >>>>>>> something Linus wrote just a few hours ago: >>>>>>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/ >>>>>> >>>>>> Except it is ready and trying again is my responsibility, which means >>>>>> unnecessary work for me to do. I've already got a ton of things to do. >>>>>> Applying the device tree patch resolves this regression; no reverts >>>>>> needed. >>>>>> And then there's the patch in the works by Daniel that will address >>>>>> all the >>>>>> remaining cases outside of the reported regression. >>>>>> >>>>> >>>>> The commit that fixes your breakage in a way that does *not* please me >>>>> (because of older devicetrees being still broken with the new driver) >>>>> was >>>>> picked and it is in v6.11-rc1. >>>>> >>>>> I had to do this because I value the community (in this case, the >>>>> users) much >>>>> more than trying to make an arrogant developer to act in a >>>>> community-friendly >>>>> manner while leaving things completely broken. >>>>> >>>>> That said, remembering that we're humans and, as such, it's normal to >>>>> get something >>>>> wrong during the development process, I want to remind you that: >>>>> >>>>> 1. A series that creates regressions is *not* good and *not* ready to >>>>> be >>>>> upstreamed; and >>>>> 2. As a maintainer, you have the responsibility of not breaking the >>>>> kernel, >>>>> not breaking devices and not breaking currently working >>>>> functionality; and >>>>> 3. Devicetrees being wrong (but working) since day 0 is not an excuse >>>>> to break >>>>> functionality; and >>>>> 4. Blaming the one who introduced the devicetree (you're doing that, >>>>> since you >>>>> are blaming the devicetree being wrong) isn't solving anything and >>>>> will not >>>>> magically make your code acceptable or good; and >>>>> 5. If you push a wrong commit, there's nothing to be ashamed of; and >>>>> 6. If you make a mistake, you should recognize that and find a way to >>>>> make things right, that should be done for the community, not for >>>>> yourself; and >>>>> 7. You shall respect the community: in this case, with your arrogant >>>>> behavior >>>>> you did *not* respect the users. >>>>> >>>>> P.S.: The right way of making such change is to: >>>>> 1. Avoid breaking currently working devices by making sure that >>>>> their DT >>>>> still works with the new driver; and >>>>> 2. If breakage is unavoidable, make it so one kernel version has >>>>> a fix that >>>>> works with both old and new driver, and the next version >>>>> introduces the >>>>> breakage. N.2 should ideally never happen, anyway. >>>>> >>>>> Let's wrap up this matter now - I don't want to spend any more word, >>>>> nor time, >>>>> on this, as I really have nothing else to say. >>>>> >>>>> Best regards, >>>>> Angelo >>>> >>>> To be clear, I only became aware that my patch was picked by reading >>>> this >>>> email. It is clear that we have different views. To that extend, all of >>>> what you have written above can be answered to by reading what I have >>>> already written in this thread. Therefore, I don't see any wrongdoing >>>> from >>>> my side and invite everyone to fully read this thread to draw their own >>>> conclusions; something you seem not to have done. And I'm not the one, >>>> calling people names here. I can only offer my respect for hard working >>>> people. >>>> >>>> In my view, your behaviour of not applying a devicetree patch before a >>>> Linux driver patch was applied, and then not replying to any arguments >>>> whatsoever, was keeping the devicetree files hostage until your demands >>> >>> Hm, why ever DTS patch should be applied before driver patch is? This >>> clearly suggests ABI break. You proposed to fix ABI issue by fixing DTS, >>> which is not the way, because it literally fixes nothing. You got >>> comments - fix the driver to be backwards compatible. >> >> As I argued in this thread, I see no ABI issue here. I proposed to fix >> broken devicetrees, nothing more. Please read the full thread to understand >> where I'm coming from. > > I read most of it and it does suggest ABI break. But even if you do not > focus on that aspect, your suggestion that DTS should be applied before > driver patch is just wrong. It was never like this and it must not be > like this. > > Best regards, > Krzysztof I will use the ABI, bindings, and dt-bindings terms synonymously here. Let's discuss the Devicetree Specification Releasev0.4. In 2.1, it is stated that DTSpec specifies a construct called a devicetree to describe system hardware. A boot program loads a devicetree into a client program’s memory and passes a pointer to the devicetree to the client. A devicetree is a tree data structure with nodes that describe the devices in a system. Each node has property/value pairs that describe the characteristics of the device being represented. Each node has exactly one parent except for the root node, which has no parent. A device in this context may be an actual hardware device, such as a UART. It may be part of a hardware device, such as the random-number generator in a TPM. It may also be a device provided through virtualisation, such as a protocol providing access to an I2C device attached to a remote CPU. A device may include functions implemented by firmware running in higher privilege levels or remote processors. There is no requirement that nodes in a device tree be a physical hardware device, but generally they have some correlation to physical hardware devices. Nodes should not be designed for OS- or project- specific purposes. They should describe something which can be implemented by any OS or project. As read here, devicetree design is not to be influenced by any project. That would mean that bindings and devicetree source files are described first, then the implementation is made by any OS or project. To be specific to the case here, it makes no sense to hold a patch that fixes a devicetree source file until a Linux driver patch is taken in. I think the dt-bindings and the DT source files being hosted on the Linux repository greatly contributes to this false impression that Linux drivers have any influence over the design of bindings or fixing DT source files that did not comply with the bindings. That is why I'm looking forward to the efforts to take dt-bindings and DT source files out of the Linux repository into its own, separate repository. This would be a great step in addressing all the project-dependent bindings of Linux, U-Boot, OpenWrt, and all other projects, to have a single, unified repository to describe all the hardware that exists in the world. One of the concepts of the devicetree architecture is that a boot program can describe and communicate system hardware information to a client program, thus eliminating the need for the client program to have hard-coded descriptions of system hardware. Not only eliminate the need of that, but allow hardware with a different value for the description than what was hard-coded. A driver change cannot possibly break ABI because it's the implementation being changed, not the bindings. The implementation change can be so that it breaks compliance with the bindings. Which is not the case with the change made in the Linux driver in question. I have eliminated the hard-coded description from the Linux driver. That did not break compliance with the bindings. Instead, the implementation change made the driver compliant with the bindings; specifically, the description where the reg value represents the PHY address of the switch. It is important to note that old devicetrees that abided to the bindings still work after the Linux driver change. This is also, of course, the case for any DT source files hosted out of the Linux repository. Therefore, the correct path forward was to correct the DT source files that include values for descriptions that do not reflect the hardware it describes. Which is eventually what happened here. To be more specific, we fixed the incorrect description of the switch's PHY address on the DT source file as, on the hardware, the switch listens on PHY address 0x1f. Arınç
Hi, On Tue, Jul 30, 2024 at 06:04:12PM +0200, Krzysztof Kozlowski wrote: > On 30/07/2024 13:22, arinc.unal@arinc9.com wrote: > > [...] > > In my view, your behaviour of not applying a devicetree patch before a > > Linux driver patch was applied, and then not replying to any arguments > > whatsoever, was keeping the devicetree files hostage until your demands > > Hm, why ever DTS patch should be applied before driver patch is? This > clearly suggests ABI break. You proposed to fix ABI issue by fixing DTS, > which is not the way, because it literally fixes nothing. You got > comments - fix the driver to be backwards compatible. I've suggested a fix for the driver here: https://patchwork.kernel.org/project/netdevbpf/patch/f485d1d4f7b34cc2ebf3d60030d1c67b4016af3c.1720107535.git.daniel@makrotopia.org/ It has been tested and acknowledged by Arinc, hence a possible solution would be to apply it and there by restore compatibility with existing (broken) device trees.
On 2024-07-30 14:22, arinc.unal@arinc9.com wrote:
> On 2024-07-30 12:41, AngeloGioacchino Del Regno wrote:
>> Il 01/07/24 10:15, Arınç ÜNAL ha scritto:
>>> On 01/07/2024 11:04, Linux regression tracking (Thorsten Leemhuis)
>>> wrote:
>>>> On 01.07.24 09:44, Arınç ÜNAL wrote:
>>>>> On 01/07/2024 09:16, Linux regression tracking (Thorsten Leemhuis)
>>>>> wrote:
>>>>>> [CCing the other net maintainers]
>>>>>>
>>>>>> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
>>>>>>> Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis)
>>>>>>> ha
>>>>>>> scritto:
>>>>>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten
>>>>>>>>> Leemhuis)
>>>>>>>>> wrote:
>>>>>>>>> [...]
>>>>>>>> It looks more and more like we are stuck here (or was there
>>>>>>>> progress
>>>>>>>> and
>>>>>>>> I just missed it?) while the 6.10 final is slowly getting
>>>>>>>> closer.
>>>>>>>> Hence:
>>>>>>>>
>>>>>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>>>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of
>>>>>>>> switch from
>>>>>>>> device tree") for now to resolve this regression? Reminder,
>>>>>>>> there is
>>>>>>>> nothing wrong with that commit per se afaik, it just exposes a
>>>>>>>> problem
>>>>>>>> that needs to be fixed first before it can be reapplied.
>>>>>>>
>>>>>>> To be clear on this: I asked for the commit to be fixed such that
>>>>>>> it
>>>>>>> guarantees
>>>>>>> backwards compatibility with older device trees.
>>>>>>>
>>>>>>> If no fix comes,
>>>>>>
>>>>>> I haven't see any since that mail, did you? If not, I think...
>>>>>>
>>>>>>> then I guess that we should ask them to revert this commit
>>>>>>> until a fix is available.
>>>>>>
>>>>>> ...it's time to ask them for the revert to resolve this for -rc7
>>>>>> (and
>>>>>> avoid a last minute revert), or what do you think?
>>>>>
>>>>> This is quite frustrating. I absolutely won't consent to a revert.
>>>>> [...]
>>>>
>>>> Reminder: try to not see a revert as a bad thing. It's just means
>>>> "not
>>>> ready yet, revert and we'll try again later" -- that's actually
>>>> something Linus wrote just a few hours ago:
>>>> https://lore.kernel.org/lkml/CAHk-=wgQMOscLeeA3QXOs97xOz_CTxdqJjpC20tJ-7bUdHWtSA@mail.gmail.com/
>>>
>>> Except it is ready and trying again is my responsibility, which means
>>> unnecessary work for me to do. I've already got a ton of things to
>>> do.
>>> Applying the device tree patch resolves this regression; no reverts
>>> needed.
>>> And then there's the patch in the works by Daniel that will address
>>> all the
>>> remaining cases outside of the reported regression.
>>>
>>
>> The commit that fixes your breakage in a way that does *not* please me
>> (because of older devicetrees being still broken with the new driver)
>> was
>> picked and it is in v6.11-rc1.
>>
>> I had to do this because I value the community (in this case, the
>> users) much
>> more than trying to make an arrogant developer to act in a
>> community-friendly
>> manner while leaving things completely broken.
>>
>> That said, remembering that we're humans and, as such, it's normal to
>> get something
>> wrong during the development process, I want to remind you that:
>>
>> 1. A series that creates regressions is *not* good and *not* ready to
>> be
>> upstreamed; and
>> 2. As a maintainer, you have the responsibility of not breaking the
>> kernel,
>> not breaking devices and not breaking currently working
>> functionality; and
>> 3. Devicetrees being wrong (but working) since day 0 is not an excuse
>> to break
>> functionality; and
>> 4. Blaming the one who introduced the devicetree (you're doing that,
>> since you
>> are blaming the devicetree being wrong) isn't solving anything and
>> will not
>> magically make your code acceptable or good; and
>> 5. If you push a wrong commit, there's nothing to be ashamed of; and
>> 6. If you make a mistake, you should recognize that and find a way to
>> make things right, that should be done for the community, not for
>> yourself; and
>> 7. You shall respect the community: in this case, with your arrogant
>> behavior
>> you did *not* respect the users.
>>
>> P.S.: The right way of making such change is to:
>> 1. Avoid breaking currently working devices by making sure that
>> their DT
>> still works with the new driver; and
>> 2. If breakage is unavoidable, make it so one kernel version has
>> a fix that
>> works with both old and new driver, and the next version
>> introduces the
>> breakage. N.2 should ideally never happen, anyway.
>>
>> Let's wrap up this matter now - I don't want to spend any more word,
>> nor time,
>> on this, as I really have nothing else to say.
>>
>> Best regards,
>> Angelo
>
> To be clear, I only became aware that my patch was picked by reading
> this
> email. It is clear that we have different views. To that extend, all of
> what you have written above can be answered to by reading what I have
> already written in this thread. Therefore, I don't see any wrongdoing
> from
> my side and invite everyone to fully read this thread to draw their own
> conclusions; something you seem not to have done. And I'm not the one,
> calling people names here. I can only offer my respect for hard working
> people.
>
> In my view, your behaviour of not applying a devicetree patch before a
> Linux driver patch was applied, and then not replying to any arguments
> whatsoever, was keeping the devicetree files hostage until your demands
> were met. What I see is that, you failed as a maintainer to attend to
> my
> points against this practice. It's no surprise that, after not having
> heard
> back from you with an argument against my points, my patch was
> eventually
> taken in by someone else.
>
> Arınç
Funny. It turns out it was not even my patch that was picked but rather
the
patch that spawned this thread. Picked by you, no less. An improper
patch
whose log was loaded with incorrect logic, and the patch itself not
fixing
all the remaining broken trees. Now someone needs to submit another
patch
to close the remaining hole,
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts.
I shall do that.
Arınç
Hi,
i can confirm that daniels patch pointed by arinc fixes driver for the wrong devicetree on BPI-R64.
but there are concerns from the netdev/dsa maintainers in its current state
https://patchwork.kernel.org/project/linux-mediatek/patch/11f5f127d0350e72569c36f9060b6e642dfaddbb.1714514208.git.daniel@makrotopia.org/
regards Frank
> Gesendet: Montag, 01. Juli 2024 um 08:16 Uhr
> Von: "Linux regression tracking (Thorsten Leemhuis)" <regressions@leemhuis.info>
> An: "Paolo Abeni" <pabeni@redhat.com>, "David S. Miller" <davem@davemloft.net>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>, "Jakub Kicinski" <kuba@kernel.org>
> Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, "Daniel Golle" <daniel@makrotopia.org>, frank-w@public-files.de, "Frank Wunderlich" <linux@fw-web.de>, "Arınç ÜNAL" <arinc.unal@arinc9.com>, "Rob Herring" <robh@kernel.org>, "Conor Dooley" <conor+dt@kernel.org>, "Krzysztof Kozlowski" <krzk+dt@kernel.org>, "Matthias Brugger" <matthias.bgg@gmail.com>, "Linux regressions mailing list" <regressions@lists.linux.dev>
> Betreff: Re: [PATCH] arm64: dts: mt7622: fix switch probe on bananapi-r64
>
> [CCing the other net maintainers]
>
> On 25.06.24 10:51, AngeloGioacchino Del Regno wrote:
> > Il 25/06/24 07:56, Linux regression tracking (Thorsten Leemhuis) ha
> > scritto:
> >> On 17.06.24 13:08, Arınç ÜNAL wrote:
> >>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
> >>> wrote:
> >>> [...]
> >> It looks more and more like we are stuck here (or was there progress and
> >> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
> >>
> >> AngeloGioacchino, should we ask the net maintainers to revert
> >> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
> >> device tree") for now to resolve this regression? Reminder, there is
> >> nothing wrong with that commit per se afaik, it just exposes a problem
> >> that needs to be fixed first before it can be reapplied.
> >
> > To be clear on this: I asked for the commit to be fixed such that it
> > guarantees
> > backwards compatibility with older device trees.
> >
> > If no fix comes,
>
> I haven't see any since that mail, did you? If not, I think...
>
> > then I guess that we should ask them to revert this commit
> > until a fix is available.
>
> ...it's time to ask them for the revert to resolve this for -rc7 (and
> avoid a last minute revert), or what do you think?
>
> > I don't like this situation, either, btw.
>
> I guess none of us does. :-/
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
On 25/06/2024 08.56, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 17.06.24 13:08, Arınç ÜNAL wrote:
>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis) wrote:
>> [...]
>> I've submitted a patch series that fixes the regression. Angelo argued
>> against the way the regression is fixed. I've very clearly argued back why
>> I find Angelo's approach wrong. There's been no response back. I don't
>> understand why reverting the patch is the likely outcome
>
> Long story short: because that how things like that are handled in the
> Linux kernel project, as Linus wants it like that. See some of the
> quotes from https://docs.kernel.org/process/handling-regressions.html
> for details.
>
>> whilst the
>> standing argument points towards applying the said patch series. If a
>> revert happens before this discussion with Angelo finalises, this will set
>> a precedent that will tell maintainers that they can have their way by just
>> not replying to the ongoing discussions.
>>
>> That said, the decision of resolving the regression by either reverting the
>> patch or applying the patch series shall not depend on whether or not
>> Angelo is pleased but rather there're no counter-arguments left on the
>> points brought, meaning the decision shall be made depending on the
>> argument that stands.
>>
>> Therefore, I suggest that unless Angelo responds back with a
>> counter-argument in the window of a week or two, as you've described, my
>> patch series shall be applied.
>
> It looks more and more like we are stuck here (or was there progress and
> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
There hasn't been progress at all. I believe I have clearly described the
way out of this issue.
>
> AngeloGioacchino, should we ask the net maintainers to revert
> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
> device tree") for now to resolve this regression? Reminder, there is
> nothing wrong with that commit per se afaik, it just exposes a problem
> that needs to be fixed first before it can be reapplied.
Are you suggesting the patch shall be reverted first, then the DT patch
applied, then the reverted patch applied back? If only one of the first two
steps were done, it would fix the regression so I don't understand why go
through this tedious process when we can quite simply apply the DT patch to
resolve the regression.
Keep in mind that I maintain the MT7530 DSA subdriver and the company I
work with has got boards that uses the functionality the commit
868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
device tree") brings.
Arınç
On 25.06.24 08:17, Arınç ÜNAL wrote:
> On 25/06/2024 08.56, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>> wrote:
>>> [...]
>>> I've submitted a patch series that fixes the regression. Angelo argued
>>> against the way the regression is fixed. I've very clearly argued
>>> back why
>>> I find Angelo's approach wrong. There's been no response back. I don't
>>> understand why reverting the patch is the likely outcome
>>
>> Long story short: because that how things like that are handled in the
>> Linux kernel project, as Linus wants it like that. See some of the
>> quotes from https://docs.kernel.org/process/handling-regressions.html
>> for details.
>>
>>> whilst the
>>> standing argument points towards applying the said patch series. If a
>>> revert happens before this discussion with Angelo finalises, this
>>> will set
>>> a precedent that will tell maintainers that they can have their way
>>> by just
>>> not replying to the ongoing discussions.
>>>
>>> That said, the decision of resolving the regression by either
>>> reverting the
>>> patch or applying the patch series shall not depend on whether or not
>>> Angelo is pleased but rather there're no counter-arguments left on the
>>> points brought, meaning the decision shall be made depending on the
>>> argument that stands.
>>>
>>> Therefore, I suggest that unless Angelo responds back with a
>>> counter-argument in the window of a week or two, as you've described, my
>>> patch series shall be applied.
>>
>> It looks more and more like we are stuck here (or was there progress and
>> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>
> There hasn't been progress at all. I believe I have clearly described the
> way out of this issue.
>
>> AngeloGioacchino, should we ask the net maintainers to revert
>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>> device tree") for now to resolve this regression? Reminder, there is
>> nothing wrong with that commit per se afaik, it just exposes a problem
>> that needs to be fixed first before it can be reapplied.
>
> Are you suggesting the patch shall be reverted first, then the DT patch
> applied, then the reverted patch applied back?
Yeah.
> If only one of the first two
> steps were done, it would fix the regression so I don't understand why go
> through this tedious process when we can quite simply apply the DT patch to
> resolve the regression.
Which DT patch do you mean here? Your series or the one from Frank at
the start of the thread (the one you seems to be unhappy about iirc, but
I might be wrong there)?
Anyway, to answer the statement: because the maintainers that would have
to accept the DT patch to resolve the problem apparently are not happy
with it -- and nobody seems to be working on providing patches that make
them happy which are also acceptable at this point of the devel cycle;
so as it looks like currently to prevent the regression from entering
6.10 reverting the net change is the only option left.
> Keep in mind that I maintain the MT7530 DSA subdriver and the company I
> work with has got boards that uses the functionality the commit
> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
> device tree") brings.
Don't see a revert as setback at all, that's just normal for the kernel.
I'm not the one that will decide about this anyway. And everyone
involved afaics would like to prevent a revert. But it seems more and
more likely that we are not getting there in time for the 6.10 release
(or ideally -rc6 or -rc7 to allow some testing, as last-minute reverts
can cause new problems).
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
On 25/06/2024 09.57, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 25.06.24 08:17, Arınç ÜNAL wrote:
>> On 25/06/2024 08.56, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>> wrote:
>>>> [...]
>>>> I've submitted a patch series that fixes the regression. Angelo argued
>>>> against the way the regression is fixed. I've very clearly argued
>>>> back why
>>>> I find Angelo's approach wrong. There's been no response back. I don't
>>>> understand why reverting the patch is the likely outcome
>>>
>>> Long story short: because that how things like that are handled in the
>>> Linux kernel project, as Linus wants it like that. See some of the
>>> quotes from https://docs.kernel.org/process/handling-regressions.html
>>> for details.
>>>
>>>> whilst the
>>>> standing argument points towards applying the said patch series. If a
>>>> revert happens before this discussion with Angelo finalises, this
>>>> will set
>>>> a precedent that will tell maintainers that they can have their way
>>>> by just
>>>> not replying to the ongoing discussions.
>>>>
>>>> That said, the decision of resolving the regression by either
>>>> reverting the
>>>> patch or applying the patch series shall not depend on whether or not
>>>> Angelo is pleased but rather there're no counter-arguments left on the
>>>> points brought, meaning the decision shall be made depending on the
>>>> argument that stands.
>>>>
>>>> Therefore, I suggest that unless Angelo responds back with a
>>>> counter-argument in the window of a week or two, as you've described, my
>>>> patch series shall be applied.
>>>
>>> It looks more and more like we are stuck here (or was there progress and
>>> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>>
>> There hasn't been progress at all. I believe I have clearly described the
>> way out of this issue.
>>
>>> AngeloGioacchino, should we ask the net maintainers to revert
>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>> device tree") for now to resolve this regression? Reminder, there is
>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>> that needs to be fixed first before it can be reapplied.
>>
>> Are you suggesting the patch shall be reverted first, then the DT patch
>> applied, then the reverted patch applied back?
>
> Yeah.
>
>> If only one of the first two
>> steps were done, it would fix the regression so I don't understand why go
>> through this tedious process when we can quite simply apply the DT patch to
>> resolve the regression.
>
> Which DT patch do you mean here? Your series or the one from Frank at
> the start of the thread (the one you seems to be unhappy about iirc, but
> I might be wrong there)?
My series, as arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts needs to be
addressed too to resolve the regression.
>
> Anyway, to answer the statement: because the maintainers that would have
> to accept the DT patch to resolve the problem apparently are not happy
> with it -- and nobody seems to be working on providing patches that make
> them happy which are also acceptable at this point of the devel cycle;
> so as it looks like currently to prevent the regression from entering
> 6.10 reverting the net change is the only option left.
I've already made my case regarding the situation with the DT patch. I
can't provide new patches because Angelo did not acknowledge my points yet.
I maintain the net driver and I too won't be happy with a revert on the
driver.
>
>> Keep in mind that I maintain the MT7530 DSA subdriver and the company I
>> work with has got boards that uses the functionality the commit
>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>> device tree") brings.
>
> Don't see a revert as setback at all, that's just normal for the kernel.
> I'm not the one that will decide about this anyway. And everyone
> involved afaics would like to prevent a revert. But it seems more and
> more likely that we are not getting there in time for the 6.10 release
> (or ideally -rc6 or -rc7 to allow some testing, as last-minute reverts
> can cause new problems).
I am still calling for the simple procedure of applying the DT patch to
resolve the regression.
Arınç
Il 25/06/24 10:17, Arınç ÜNAL ha scritto:
> On 25/06/2024 09.57, Linux regression tracking (Thorsten Leemhuis) wrote:
>> On 25.06.24 08:17, Arınç ÜNAL wrote:
>>> On 25/06/2024 08.56, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>>> wrote:
>>>>> [...]
>>>>> I've submitted a patch series that fixes the regression. Angelo argued
>>>>> against the way the regression is fixed. I've very clearly argued
>>>>> back why
>>>>> I find Angelo's approach wrong. There's been no response back. I don't
>>>>> understand why reverting the patch is the likely outcome
>>>>
>>>> Long story short: because that how things like that are handled in the
>>>> Linux kernel project, as Linus wants it like that. See some of the
>>>> quotes from https://docs.kernel.org/process/handling-regressions.html
>>>> for details.
>>>>
>>>>> whilst the
>>>>> standing argument points towards applying the said patch series. If a
>>>>> revert happens before this discussion with Angelo finalises, this
>>>>> will set
>>>>> a precedent that will tell maintainers that they can have their way
>>>>> by just
>>>>> not replying to the ongoing discussions.
>>>>>
>>>>> That said, the decision of resolving the regression by either
>>>>> reverting the
>>>>> patch or applying the patch series shall not depend on whether or not
>>>>> Angelo is pleased but rather there're no counter-arguments left on the
>>>>> points brought, meaning the decision shall be made depending on the
>>>>> argument that stands.
>>>>>
>>>>> Therefore, I suggest that unless Angelo responds back with a
>>>>> counter-argument in the window of a week or two, as you've described, my
>>>>> patch series shall be applied.
>>>>
>>>> It looks more and more like we are stuck here (or was there progress and
>>>> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>>>
>>> There hasn't been progress at all. I believe I have clearly described the
>>> way out of this issue.
>>>
>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>>> device tree") for now to resolve this regression? Reminder, there is
>>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>>> that needs to be fixed first before it can be reapplied.
>>>
>>> Are you suggesting the patch shall be reverted first, then the DT patch
>>> applied, then the reverted patch applied back?
>>
>> Yeah.
>>
Sorry, I've lost your reply in the long stack of emails that I get every day.
I'm not suggesting to revert the patch, but to fix it such that it does not
break devices using old devicetrees, as it was the case before.
Even though, in a way, when you update the kernel, you do also update the
devicetrees with it just because it's almost effortless to do so, doing that
is not mandatory.
...and that's why the driver, which was - in a way - faulty before, getting
the switch to work even though the devicetree node was wrong, must still be
compatible.
I do want to apply the devicetree fix, but I also do *not* want to see *driver*
changes that go against the backward compatibility rule of devicetree when this
is not strictly necessary (when it is - it's okay to make an exception)...
...and this is not one of the cases in which it's strictly necessary.
>>> If only one of the first two
>>> steps were done, it would fix the regression so I don't understand why go
>>> through this tedious process when we can quite simply apply the DT patch to
>>> resolve the regression.
>>
>> Which DT patch do you mean here? Your series or the one from Frank at
>> the start of the thread (the one you seems to be unhappy about iirc, but
>> I might be wrong there)?
>
> My series, as arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts needs to be
> addressed too to resolve the regression.
>
>>
>> Anyway, to answer the statement: because the maintainers that would have
>> to accept the DT patch to resolve the problem apparently are not happy
>> with it -- and nobody seems to be working on providing patches that make
>> them happy which are also acceptable at this point of the devel cycle;
>> so as it looks like currently to prevent the regression from entering
>> 6.10 reverting the net change is the only option left.
>
> I've already made my case regarding the situation with the DT patch. I
> can't provide new patches because Angelo did not acknowledge my points yet.
> I maintain the net driver and I too won't be happy with a revert on the
> driver.
>
And again, I wouldn't be happy to see a revert either; just fix it so that the
old devicetree still works with the driver code.
Regards,
Angelo
On 25/06/2024 11.49, AngeloGioacchino Del Regno wrote:
> Il 25/06/24 10:17, Arınç ÜNAL ha scritto:
>> On 25/06/2024 09.57, Linux regression tracking (Thorsten Leemhuis) wrote:
>>> On 25.06.24 08:17, Arınç ÜNAL wrote:
>>>> On 25/06/2024 08.56, Linux regression tracking (Thorsten Leemhuis) wrote:
>>>>> On 17.06.24 13:08, Arınç ÜNAL wrote:
>>>>>> On 17/06/2024 11:33, Linux regression tracking (Thorsten Leemhuis)
>>>>>> wrote:
>>>>>> [...]
>>>>>> I've submitted a patch series that fixes the regression. Angelo argued
>>>>>> against the way the regression is fixed. I've very clearly argued
>>>>>> back why
>>>>>> I find Angelo's approach wrong. There's been no response back. I don't
>>>>>> understand why reverting the patch is the likely outcome
>>>>>
>>>>> Long story short: because that how things like that are handled in the
>>>>> Linux kernel project, as Linus wants it like that. See some of the
>>>>> quotes from https://docs.kernel.org/process/handling-regressions.html
>>>>> for details.
>>>>>
>>>>>> whilst the
>>>>>> standing argument points towards applying the said patch series. If a
>>>>>> revert happens before this discussion with Angelo finalises, this
>>>>>> will set
>>>>>> a precedent that will tell maintainers that they can have their way
>>>>>> by just
>>>>>> not replying to the ongoing discussions.
>>>>>>
>>>>>> That said, the decision of resolving the regression by either
>>>>>> reverting the
>>>>>> patch or applying the patch series shall not depend on whether or not
>>>>>> Angelo is pleased but rather there're no counter-arguments left on the
>>>>>> points brought, meaning the decision shall be made depending on the
>>>>>> argument that stands.
>>>>>>
>>>>>> Therefore, I suggest that unless Angelo responds back with a
>>>>>> counter-argument in the window of a week or two, as you've described, my
>>>>>> patch series shall be applied.
>>>>>
>>>>> It looks more and more like we are stuck here (or was there progress and
>>>>> I just missed it?) while the 6.10 final is slowly getting closer. Hence:
>>>>
>>>> There hasn't been progress at all. I believe I have clearly described the
>>>> way out of this issue.
>>>>
>>>>> AngeloGioacchino, should we ask the net maintainers to revert
>>>>> 868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY address of switch from
>>>>> device tree") for now to resolve this regression? Reminder, there is
>>>>> nothing wrong with that commit per se afaik, it just exposes a problem
>>>>> that needs to be fixed first before it can be reapplied.
>>>>
>>>> Are you suggesting the patch shall be reverted first, then the DT patch
>>>> applied, then the reverted patch applied back?
>>>
>>> Yeah.
>>>
>
> Sorry, I've lost your reply in the long stack of emails that I get every day.
>
> I'm not suggesting to revert the patch, but to fix it such that it does not
> break devices using old devicetrees, as it was the case before.
>
> Even though, in a way, when you update the kernel, you do also update the
> devicetrees with it just because it's almost effortless to do so, doing that
> is not mandatory.
>
> ...and that's why the driver, which was - in a way - faulty before, getting
> the switch to work even though the devicetree node was wrong, must still be
> compatible.
>
> I do want to apply the devicetree fix, but I also do *not* want to see *driver*
> changes that go against the backward compatibility rule of devicetree when this
> is not strictly necessary (when it is - it's okay to make an exception)...
>
> ...and this is not one of the cases in which it's strictly necessary.
I understand that you receive emails often. It seems you've not read my
point of view in the previous emails because of it, so it is preventing us
from having a proper discussion to come to a mutual agreement. I don't
intend to repeat myself for it to be lost in your inbox again.
I'm hoping that you manage to read this; Daniel has been working on a patch
[1] to make old device trees not hosted on the Linux repository with PHY
address wrongfully described as "0" work.
As I see the extend of the reported regression is limited to the BPI-R64
board, fixing the device tree source file for it - and mt7622-rfb1.dts as
the only remaining DTS file with wrong PHY address described in the Linux
repository - is enough to resolve the regression. Then Daniel's patch can
come in to address the device tree source files I've described above.
Although I don't find that patch necessary, I won't stand against it.
So I suggest to apply my DT patch series [2] without taking any other steps
to resolve the reported regression.
[1] https://lore.kernel.org/all/11f5f127d0350e72569c36f9060b6e642dfaddbb.1714514208.git.daniel@makrotopia.org/
[2] https://lore.kernel.org/all/20240314-for-mediatek-mt7531-phy-address-v1-0-52f58db01acd@arinc9.com/
Arınç
On 06/06/2024 11.26, Thorsten Leemhuis wrote:
> On 31.05.24 08:10, Arınç ÜNAL wrote:
>> I had already submitted a patch series that would've prevented this
>> issue back in 14 March 2024 [1]. I've asked numerous times for the patch
>> series to be applied [2][3][4][5].
>>> Eventually Daniel asked for some changes [6]. But I won't have time to do
>> that anytime soon and I think the patch series is good enough to be applied
>> as is.
>
> Then I guess we need some other way to resolve this in mainline to unfix
I don't believe we need another way to resolve it. I've already told that
the patch series is good enough to be applied as is and I don't see any
responses with reasons against this here.
> Frank's device. The two obvious options are afaics:
>
> * revert the culprit (868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY
> address of switch from device tree")) and reapply it in a later cycle
Sorry, no. There's nothing wrong with that patch. The actual cause of this
issue is the patch that introduced this device tree source file with the
wrong PHY address.
> * apply Frank's patch (or an improved one) in this thread (and if needed
> revert it when some better changes emerge.
>
> Arınç ÜNAL, could you please comment on that and ideally handle the
> necessary tasks, as it's your patch that causes the regression.
I don't see any necessary tasks for me to handle. AngeloGioacchino Del
Regno whom is the only person I know that maintains these device tree
source files can simply apply the patch series that I have already
submitted and we can all move on. I haven't heard from them whilst the
patch had been waiting since March. So I'm not sure who's going to apply
this patch, and to which tree.
Arınç
On 06.06.24 11:01, Arınç ÜNAL wrote:
> On 06/06/2024 11.26, Thorsten Leemhuis wrote:
>> On 31.05.24 08:10, Arınç ÜNAL wrote:
>>> I had already submitted a patch series that would've prevented this
>>> issue back in 14 March 2024 [1]. I've asked numerous times for the patch
>>> series to be applied [2][3][4][5].
>>>> Eventually Daniel asked for some changes [6]. But I won't have time
>>>> to do
>>> that anytime soon and I think the patch series is good enough to be
>>> applied
>>> as is.
>>
>> Then I guess we need some other way to resolve this in mainline to unfix
>
> I don't believe we need another way to resolve it. I've already told that
> the patch series is good enough to be applied as is and I don't see any
> responses with reasons against this here.
>
>> Frank's device. The two obvious options are afaics:
>>
>> * revert the culprit (868ff5f4944aa9 ("net: dsa: mt7530-mdio: read PHY
>> address of switch from device tree")) and reapply it in a later cycle
>
> Sorry, no. There's nothing wrong with that patch. The actual cause of this
> issue is the patch that introduced this device tree source file with the
> wrong PHY address.
Was that also merged for 6.10? Because if not, then what matters here
afaics is what patch exposed the problem. Of course ideally we wound fix
that problem -- but if nobody takes care of that any time soon it might
come down to a revert of the patch that exposed the problem. At least
that how Linus handles these things afaics.
>> * apply Frank's patch (or an improved one) in this thread (and if needed
>> revert it when some better changes emerge.
>>
>> Arınç ÜNAL, could you please comment on that and ideally handle the
>> necessary tasks, as it's your patch that causes the regression.
>
> I don't see any necessary tasks for me to handle. AngeloGioacchino Del
> Regno whom is the only person I know that maintains these device tree
> source files can simply apply the patch series that I have already
> submitted and we can all move on. I haven't heard from them whilst the
> patch had been waiting since March. So I'm not sure who's going to apply
> this patch, and to which tree.
AngeloGioacchino Del Regno, if you have a minute, could you please
comment if merging those changes for 6.10 is an option?
Ciao, Thorsten
© 2016 - 2024 Red Hat, Inc.