RE: [PATCH] dt-bindings: iommu: renesas,ipmmu-vmsa: Update descriptions for R-Car Gen4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux