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:

quoted

Hi Uwe,

On 12/23/2016 12:01 PM, Uwe Kleine-K?nig wrote:

quoted

On Fri, Dec 23, 2016 at 11:27:24AM +0200, Vladimir Zapolskiy wrote:

quoted

On 12/23/2016 10:32 AM, Uwe Kleine-K?nig wrote:

quoted

Hello,

On Fri, Dec 23, 2016 at 10:20:20AM +0200, Vladimir Zapolskiy wrote:

quoted

On 12/23/2016 03:55 AM, Guenter Roeck wrote:

quoted

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