2023-07-28 10:25:02

by Nikita Shubin

[permalink] [raw]
Subject: Re: [PATCH v3 07/42] soc: Add SoC driver for Cirrus ep93xx

On Fri, 2023-07-28 at 12:46 +0300, Andy Shevchenko wrote:
> On Fri, Jul 28, 2023 at 12:28 PM Nikita Shubin
> <[email protected]> wrote:
> >
> > Hello Andy!
> >
> > On Fri, 2023-07-21 at 17:13 +0300, Andy Shevchenko wrote:
> > > On Thu, Jul 20, 2023 at 02:29:07PM +0300, Nikita Shubin via B4
> > > Relay
> > > wrote:
> > > > From: Nikita Shubin <[email protected]>
> > > > +       spin_lock_irqsave(&ep93xx_swlock, flags);
> > > > +
> > > > +       regmap_read(map, EP93XX_SYSCON_DEVCFG, &val);
> > > > +       val &= ~clear_bits;
> > > > +       val |= set_bits;
> > > > +       regmap_write(map, EP93XX_SYSCON_SWLOCK,
> > > > EP93XX_SWLOCK_MAGICK);
> > > > +       regmap_write(map, EP93XX_SYSCON_DEVCFG, val);
> > >
> > > Is this sequence a must?
> > > I.o.w. can you first supply magic and then update devcfg?
> > >
> >
> > Unfortunately it is a must to write EP93XX_SYSCON_SWLOCK and only
> > then
> > the next write to swlocked registers will succeed.
>
> This doesn't answer my question. Can you first write a magic and then
> _update_ the other register (update means RMW op)?
>

I see your point now - citing docs:

"Logic safeguards are included to condition the control signals for
power connection to the matrix to prevent part damage. In addition, a
software lock register is included that must be written with 0xAA
before each register write to change the values of the four switch
matrix control registers."

So reading SHOULDN'T affect the lock.

But as we checked reading also breaks the lock, that's why this looks
so odd, it was done for purpose - i'll check it once again anyway.



2023-07-31 21:31:05

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 07/42] soc: Add SoC driver for Cirrus ep93xx

On Fri, Jul 28, 2023 at 01:01:11PM +0300, Nikita Shubin wrote:
> On Fri, 2023-07-28 at 12:46 +0300, Andy Shevchenko wrote:

...

> I see your point now - citing docs:
>
> "Logic safeguards are included to condition the control signals for
> power connection to the matrix to prevent part damage. In addition, a
> software lock register is included that must be written with 0xAA
> before each register write to change the values of the four switch
> matrix control registers."
>
> So reading SHOULDN'T affect the lock.
>
> But as we checked reading also breaks the lock, that's why this looks
> so odd, it was done for purpose - i'll check it once again anyway.

This is very interesting information! Please, document this somewhere in
the code.

--
With Best Regards,
Andy Shevchenko