Re: [PATCH] ARM: dts: ti/omap: gta04: fix pm issues caused by spi module

[Andreas Kemnade <andreas@kemnade.info>]

Am Mon, 18 Nov 2024 15:08:48 +0200
schrieb Roger Quadros <rogerq@kernel.org>:

> On 16/11/2024 22:27, Andreas Kemnade wrote:
> > Am Mon, 11 Nov 2024 23:46:04 +0100
> > schrieb Andreas Kemnade <andreas@kemnade.info>:
> >   
> >> Am Mon, 11 Nov 2024 19:31:17 +0100
> >> schrieb Andreas Kemnade <andreas@kemnade.info>:
> >>  
> >>> Am Mon, 11 Nov 2024 17:09:53 +0200
> >>> schrieb Tony Lindgren <tony@atomide.com>:
> >>>     
> >>>> * Andreas Kemnade <andreas@kemnade.info> [241108 17:41]:      
> >>>>> They are not used, if they are just disabled, kernel does not touch
> >>>>> them, so if it is there, the kernel can handle
> >>>>> pm. At least as long as it is not under ti,sysc.
> >>>>>
> >>>>> There are probably cleaner solutions for this, but for a CC: stable I
> >>>>> would prefer something less invasive.        
> >>>>
> >>>> For unused devices, it's best to configure things to use ti-sysc, and
> >>>> then set status disabled (or reserved) for the child devices only. This
> >>>> way the parent interconnect target module is PM runtime managed by
> >>>> Linux, and it's power domain gets properly idled for the unused devices
> >>>> too.
> >>>>       
> >>> Hmm, we also have omap_hwmod_setup_all() which is still called if
> >>> without device nodes being available.
> >>>
> >>> Converting mcspi to ti-sysc is more than 100 lines. So it does not
> >>> qualify for stable.
> >>>     
> >>>>> I can try a ti-sysc based fix in parallel.        
> >>>>
> >>>> Yeah that should be trivial hopefully :)
> >>>>       
> >>> I played around, got pm issues too, tried to force-enable things (via
> >>> power/control),
> >>> watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set
> >>> or reset.
> >>>
> >>> but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
> >>>
> >>> I test from initramfs, so no mmc activity involved
> >>>
> >>> removing status = "disabled" from mcspi3 solves things.
> >>> With and without ti-sysc conversion. removing status = "disabled" from
> >>> mcspi4 seems not to help.
> >>>
> >>> That all cannot be... I will retry tomorrow.
> >>>     
> >> well, I tried a bit further:
> >> I build the omap spi driver as module.
> >> and booted With mcspi3 not disabled and no module autoload.
> >>
> >> without module loaded: pm bad, same as with mcspi3 disabled
> >> with module loaded: core pm ok
> >> with module loaded and unloaded: core pm ok.
> >>
> >> so at least a trace.
> >>  
> > ok, I am a bit further.
> > mcspi is per default in slave mode, setting it to master solves issues.
> > And that happens when the driver is probed because its default is
> > master.
> > Having the pins muxed as mode 7 also helps or selecting a pulldown for
> > cs. (cs is active high per default!)
> > switching to pullup does not harm once the spi module is off, but having
> > active cs seems to prevent idling despite CM_IDLEST1_CORE
> > not showing it.
> > 
> > History: u-boot muxes McSPI3, because it can be available on an
> > optionally fitted pin header. But there is no user known (would need
> > a dtb overlay anyways). So I will rather mux to mode 7.  
> 
> I'm sorry I didn't fully understand the problem.
> 
> So, u-boot configures pinmux for McSPI3 and enables McSPI3 as well
> but fails to disable it properly?

At least it sets Pinmux.
 
> And because McSPI3 is in slave mode and CS is active it fails to
> transition to idle in Linux?

yes, slave mode is default.
> 
> So isn't this a u-boot issue?
> 
Just telling u-boot to not mux McSPI3 helps. So, yes u-boot should not
set it. But. I have no clear idea how bitrot the u-boot update process
is. I would prefer setting the pinmux also in kernel back to the
default.

Regards,
Andreas