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