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?
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?
> > > --- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
> > > +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
> > > @@ -76,14 +76,15 @@ properties:
> > > items:
> > > - items:
> > > - description: phandle to main IPMMU
> > > - - description: the interrupt bit number associated with the particular
> > > - cache IPMMU device. The interrupt bit number needs to match the main
> > > - IPMMU IMSSTR register. Only used by cache IPMMU instances.
> > > + - description: The interrupt bit number or module id associated with
> > > + the particular cache IPMMU device. The interrupt bit number needs
> > > + to match the main IPMMU IMSSTR register. Only used by cache IPMMU
> > > + instances.
> > > description:
> > > Reference to the main IPMMU phandle plus 1 cell. The cell is
> > > - the interrupt bit number associated with the particular cache IPMMU
> > > - device. The interrupt bit number needs to match the main IPMMU IMSSTR
> > > - register. Only used by cache IPMMU instances.
> > > + the interrupt bit number or module id associated with the particular
> > > + cache IPMMU device. The interrupt bit number needs to match the main
> > > + IPMMU IMSSTR register. Only used by cache IPMMU instances.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
