Re: [PATCH] net: dsa: mt7530: increase reset hold time

[Justin Swartz]

On 2024-03-13 14:06, Arınç ÜNAL wrote:
> On 13.03.2024 14:52, Justin Swartz wrote:
> > On 2024-03-13 10:59, Arınç ÜNAL wrote:
> > > This ship has sailed anyway. Now the changes the first patch did must 
> > > be
> > > reverted too. I will deal with this from now on, you can stop sending
> > > patches regarding this.
> >
> > At least if the first patch isn't reverted, the approach used is
> > less likely to result in the problem occuring, IMHO.
>
> I disagree that the previous approach is less likely to result in the
> problem occurring. If you don't like the delay amount we agreed on, 
> feel
> free to express a higher amount.

I created and tested a patch to entertain your input about what you
thought could be a suitable hold period to address the problem, and it
appeared to work. The criteria being that the crystal frequency 
selection
was correct over 20 tests.

So if only the reset hold period is going to change, I'm good with what
you had suggested: 5000 - 5100 usec.

Ultimately the selection of this period should be guided by the timing
information provided in a datasheet or design guide from the 
manufacturer.

If you, or anyone else, has such a document that provides this 
information
and is able to confirm or deny speculation about any/all timing periods
related to reset, please do so.


> I also disagree on introducing a solution that is in addition to 
> another
> solution, both of which fix the same problem.

I'm not sure I understand why you say that. These patches were intended
to be applied exclusively, or in other words: in isolation - not 
together.

Although if they were applied together, it wouldn't really matter.

For the record, I've only continued to keep this thread alive in the
hope that some solution to this problem will make it into mainline
eventually.

I don't care if it was my original patch, the subsequent patch, or a
later patch provided by you or someone else. :)


> > The delay between deliberately switching the LEDs off, instead of
> > only waiting on chip reset logic to handle that, and the reset
> > assertion could be considered a "reset setup" period to complement
> > the original reset hold period.
> >
> > Increasing the hold period to what should be 5000 - 5100 usec,
> > definitely made the problem go away my testing, but the previous
> > approach is (if nothing else) more explicit in its intent.
>
> I don't want any unnecessary complications on the code I'm maintaining. 
> I
> already gave a clear intent on the patch log that introduces a simpler 
> and
> more efficient approach, it doesn't need to be on the code.
>
> Arınç


Kind Regards
Justin