Hi Robin, > From: Robin Murphy, Sent: Wednesday, January 25, 2023 7:42 PM > > On 2023-01-25 08:54, Geert Uytterhoeven wrote: > > Hi Shimoda-san, > > > > On Wed, Jan 25, 2023 at 1:49 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > >>> From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM > >>> On Mon, Jan 23, 2023 at 2:35 AM Yoshihiro Shimoda > >>> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > >>>> Since R-Car Gen4 doens't have the main IPMMU IMSSTR register, but > >>>> each cache IPMMU has own module id. So, update descriptions of > >>>> renesas,ipmmu-main property for R-Car Gen4. > >>>> > >>>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > > >>>> --- > >>>> The old R-Car S4-8 datasheet had described IPMMU IMSSTR register, but > >>>> the latest datasheet undocumented the register. So, update the propeties > >>>> description. Note that the second argument is not used on the driver. > >>> > >>> DT describes hardware, not software policy. > >> > >> I think so. > >> > >>>> So no behavior change. > >>> > >>> So where do we get the module id numbers to use, if they are no longer > >>> documented in the Hardware Manual? > >> > >> If so, we cannot get the module id numbers. So, should we use other > >> information which is completely fixed instead? I have some ideas: > >> 1) Just 0 (or other fixed value) if the IMSSTR register doesn't exist. > >> 2) Sequential numbers from register base offset. > >> In R-Car S4: ipmmu_rt0 is the first node from register base offset, > >> and ipmmu_rt1 is the second one. > >> So, ipmmu_rt0 is 0, ipmmu_rt1 is 1, ipmmu_ds0 is 2 and ipmmu_hc is 3. > >> 3) Using base address upper 16-bits. > >> In R-Car S4: ipmmu_rt0 is 0xee480000. So, the value is 0xee48. > >> > >> Perhaps, the option 1) is reasonable, I think. But, what do you think? > > > > I would not make up numbers, as that would cause confusion with SoCs > > where the numbers do match the hardware. > > As the driver doesn't use the module id number (it already loops > > over all domains, instead of checking IMSSTR, probably because of > > historical (R-Car Gen2) reasons?), what about dropping it from the > > property? I.e. add "minItems: 1", possibly only when compatible with > > renesas,rcar-gen4-ipmmu-vmsa? > > Right, if there really is no meaningful ID for this model then its > binding should not require one. Thank you for your comment. I got it. > > BTW, the related IMCTR register is still documented, and the driver > > does enable the interrupt bit (IMCTR_INTEN), so I'm wondering how the > > hardware (documentation) people intend this to be used... > > Perhaps IMCTR_INTEN will be removed/undocumented, too? > > Or perhaps the removal/undocumentation of IMSSTR was a mistake? > > I guess it should be pretty straightforward to just try reading the > expected IMSSTR register locations on this SoC to double-check whether > anything is there. As I mentioned on other email [1], we should not access IMSSTR to avoid a potential issue... [1] https://lore.kernel.org/all/TYBPR01MB5341F8DC36EAD659209A2BDDD8CF9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ Best regards, Yoshihiro Shimoda > Thanks, > Robin.
