Re: [PATCH v2 6/6] MIPS: cm: Probe GCR address from DeviceTree

[Serge Semin]

On Tue, Sep 10, 2024 at 08:23:25PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年9月10日九月 下午1:36，Serge Semin写道：
> [...]
> >
> > This causes the kernel boot-up procedure to crash/hang-up because the
> > CM GCR base address is supposed to be redefined by means of the
> > already mapped CM GCR address space by accessing the
> > CM_GCR_BASE_GCRBASE register:
> > change_gcr_base()
> > +-> read_gcr_base()
> >     +-> addr_gcr_base()
> >         +-> return mips_gcr_base + CM_GCR_BASE_GCRBASE
> >
> > By the time of the change_gcr_base() call in mips_cm_phys_base(), the
> > mips_gcr_base variable hasn't been defined. So the IO operations
> > performed in the change_gcr_base() method would be accessing the
> > NULL-based memory space. That's why the kernel crash/hanging-up.
> 
> Thanks for the analysis!
> This path was not taken on my audience hardware, so I didn't catch this,
> will fix in next version.
> 
> >
> > In order to fix this we have to first map the CM GCR block at the
> > default base-address, then update the CM GCR-base CSR and after that
> > remap the CM GCR-space.
> >
> > Please also note, the GCR_BASE field might be RO. It depends on the
> > IP-core configuration. So it's possible that the CM_GCR_BASE_GCRBASE
> > field update won't work. Although that will be detected a bit later in
> > the mips_cm_probe() method by comparing the address returned from
> > mips_cm_phys_base() and retrieved from the CM GCR-base CSR.
> 

> Hmm, I just checked RTL and RDL for CM2 and CM3 and I didn't see it as a
> configurable option. It's possible to change hardware reset value but not make it RO.

Both MIPS P5600 and P6600 databooks define the GCR_BASE field as
optionally R/W:

GCR_BASE 31:15 This field sets the base address of the 32KB          R or R/W
               GCR block of the P5600 MPS.                           (IP Config-
               This register has a fixed value after reset if         uration)
               configured as Read-Only (an IP Configuration Option).

> 
> Maybe it was possible on earlier IP release,

I found the text above in the latest MIPS Warrior P-class software
manuals downloaded from the ImagTech site. Not sure why your RTL code
doesn't have such configs.

> in this case it's always
> user's responsibility to write correct address in DeviceTree :-)

Right. The system just won't work if the CM GCR base address couldn't
be updated.

-Serge(y)

> 
> Thanks
> 
> >
> > -Serge(y)
> 
> -- 
> - Jiaxun