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]

 



On 2023-01-31 08:20, Geert Uytterhoeven wrote:
Hi Rob,

On Mon, Jan 30, 2023 at 8:36 PM Rob Herring <robh@xxxxxxxxxx> wrote:
On Wed, Jan 25, 2023 at 10:42:13AM +0000, Robin Murphy wrote:
On 2023-01-25 08:54, Geert Uytterhoeven wrote:
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.

I agree, however that makes parsing the property a pain (for both the
schema and driver). This property is a matrix. The number of entries is
already variable. If both dimensions are variable, we have to then look
at the compatible to know how to parse it. I would go with option 1.

But it does not have to be two-dimensional.
The second dimension was added in commit 39bd2b6a3783b899 ("dt-bindings
  Improve phandle-array schemas"), but is not needed.
Can this be simplified again?

Maybe the answer is that it's a cell-array of one item which contains a bare phandle and an (optional) integer that happens to look a bit like a phandle argument but isn't?

Robin.

The driver doesn't care, it just checks for the presence of the
property, i.e. treats it as a boolean flag.

A 4th option is a new property.

If all else fails, a new boolean flag would work...

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