Hello Uwe, On 12/23/2016 04:11 PM, Uwe Kleine-König wrote: > On Fri, Dec 23, 2016 at 01:21:20PM +0200, Vladimir Zapolskiy wrote: >> Hi Uwe, >> >> On 12/23/2016 12:01 PM, Uwe Kleine-König wrote: >>> On Fri, Dec 23, 2016 at 11:27:24AM +0200, Vladimir Zapolskiy wrote: >>>> On 12/23/2016 10:32 AM, Uwe Kleine-König wrote: >>>>> Hello, >>>>> >>>>> On Fri, Dec 23, 2016 at 10:20:20AM +0200, Vladimir Zapolskiy wrote: >>>>>> On 12/23/2016 03:55 AM, Guenter Roeck wrote: >>>>>>> What is the ultimate conclusion of this exchange ? >>>>>>> >>>>>>> Are we going to get another version of the patch, or did everyone agree that >>>>>>> the patch is good as it is and does not require further changes ? >>>>>>> >>>>>> >>>>>> I can not imagine a different fix. >>>>> >>>>> my preferred fix would be: >>>>> >>>>> - add an imx35 compatible to all newer dtsi >>>>> - update the driver to only write the wmcr on imx35 compatible devices >>>>> adding only imx35. >>>>> >>>> >>>> It breaks old DTS vs. new kernel compatibility, I've already mentioned this. >>> >>> Correct, and I didn't deny that. In my mail I pointed out the problem >>> this imposes and I think it's small enough to not justify the complexity >>> introduced in your proposed change. >>> >> >> I can not statistically estimate well the severity of the problem which was >> fixed by commit 5fe65ce7cc (all boards with a similar change not found in >> a bootloader will be affected, I believe) multiplied by the rate of users, >> who don't update DTB. > > I think most people updating the kernel will update the dtb at the same > time. And for those who don't it should be quickly obvious that > something is wrong during development if the machine resets > unexpectedly. Plus I think that most users are not affected anyhow > (either because their bootloader fixes up accordingly or because most > machines don't reset when the timer expires because the hardware isn't > designed accordingly). After all between introduction of i.MX35/i.MX25 > (not later than Thu Jun 4 11:32:12 2009 +0200, commit > 8c25c36f33157a2e2a1fcd60b6dc00feace80631) and the broken commit > 5fe65ce7cc (Mon Sep 8 09:14:07 2014 +0200) it seems it wasn't an issue. > > With my suggestion the situation on an affected machine with old dtb and > new kernel is just as it was in these 5 years where nobody complained. Please feel free to send the revert of 5fe65ce7cc, if you think that the commit is unneeded. > > It's fine to target a bugfree system, and if you want to target that > that's applaudable. But then please make it easy for others after you to > not undo your good and add a big comment to the compatible array of the > driver explaining why they are all listed explicitly and how to prevent > to have to expand the list further. Please clarify, what do you mean here by "undo my good"? Revert my fix and break boot on i.MX31 again or something else? > > So yes, my suggestion has a risk, but I think when weighting that > against the overhead that is introduced in the driver, I'd go the > simpler way. What overhead do you mean here? Prolonged time in a loop while finding a compatible by looking at 10 more values? That's the price of the accepted commit 5fe65ce7cc and overlooked incompatibilities in hardware while writing DTSI files for i.MX SoCs. > That's of course something personal and as it's you who > seems to have in interest in fixing the driver, it's your way that > matters more. And if you document your way good enough, I won't stop > you. > We're going round in circles. And I don't understand what do you mean by "documenting my way" here. For clarity I do NOT object against changes in DTS, I do NOT object to shrink the list of compatibles, I object to mix up the requested DTS changes for practically every i.MX powered board, bug fix and a new potential bug in one single change. The series of commits is generally good, but it lacks atomicity property of a fix, and in the middle of the series users have to update DTB, it is an overkill. Is it possible to change DTS *only* and fix kernel boot problem? NO. Is it possible to change driver *only* and fix kernel boot problem? YES. Conclusion: the problem is within the driver, to solve the problem it is sufficient to fix the driver. As I see it what you propose with involving DTS changes is a solution to *another* problem (boot time performance optimisation? amount of driver's lines of code? DTS beautification?). And obviously it makes no sense to send a DTS change plus error-prone change plus the actual fix to linux-stable then, even if they are split by commits. Uwe, I'm disappointed that we still did not come to a conclusion, would you agree to a compromise? I'll add a comment to v2: /* * The list of compatibles is added to achieve backward compatibility * between DTS and kernel while fixing the incompatibility between * i.MX21/i.MX27/i.MX31 and i.MX25/i.MX35/etc. watchdog controllers. * * Please do *NOT* extend the list by adding a compatible value for * a controller, which is compatible with one of the already listed. * * You may consider to shrink the list at your own risk, but this may * break backward compatibility and users may have to update their DTB, * which is good eventually. */ And you send the removal of the comment and compatibles as a *separate* change, if you find such a long list of compatibles redundant. -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
