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 Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, January 24, 2023 11:35 PM
> 
> Hi Shimoda-san,
> 
> 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>
> 
> Thanks for your patch!

Thank you for your review!

> > ---
> >  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?

Best regards,
Yoshihiro Shimoda

> > --- 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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux