> Then I would assume that all lpddr properties can differ between ranks, > including the timings. But probably some SDRAM memory expert should > clarify that. Right, so that's what my proposal does -- separate timings nodes per rank (and channel). > > That really doesn't work for our use case, we can't generate a > > specific compatible string for each part number. This may work when > > your board is only using a single memory part and you can hardcode > > that in the DTB blob bundled with the kernel, but we are trying to do > > runtime identification between dozens of different parts on our > > boards. The whole point of us wanting to add these bindings is that we > > want to have the firmware inject the raw values it can read from mode > > registers into the device tree (with just the compatible string > > "jedec,lpddr3"), > > You cannot have jedec,lpddr3 alone. You need specific compatible. Sorry, what do you mean we cannot? Why not? That's the way we need to do it for our use case. Why shouldn't it work that way? As far as I understand the binding definition, this is one of the legal compatible strings for it. (I'm not saying other platforms can't register and provide specific compatible strings if they want to, of course, but for our situation that really doesn't work.) > > so that we can then delegate the task of matching > > those values to part numbers to a userspace process. > > Constructing a vendor from mode registers is like 10 lines of C code, so > this is not a problem. Trouble would be with device part of compatible. There's potentially 255 different manufacturer codes, and the assignments may be different for different LPDDR versions. That's a big string table that we don't want to have to fit in our firmware flash. Besides, as you said, that still only gives you the vendor... so then should we use "micron,lpddr3" or "elpida,lpddr3" instead of "jedec,lpddr3"? Where's the advantage in that? > > Can we please revert that deprecation and at least keep the property > > around as optional? > > Yes, we can. You still would need to generate the compatible according > to the current bindings. Whether we can change it I am not sure. I think > it depends how much customization is possible per vendor, according to > JEDEC spec. If we never ever have to identify specific part, because > JEDEC spec and registers tell us everything, then we could skip it, > similarly to lpddr2 and jedec,spi-nor. Shouldn't that be decided per use case? In general LPDDR is a pretty rigid set of standards and memory controllers are generally compatible with any vendor without hardcoding vendor-specific behavior, so I don't anticipate that this would be likely (particularly since there is no "real" kernel device driver that needs to initialize the full memory controller, after all, these bindings are mostly informational). Of course there may always be mistakes and broken devices that need custom handling, and if someone has a platform with such a case I of course don't want to preclude them from tying special behavior to a custom compatible string. But why would that mean we need to make this mandatory for all platforms even if it's not relevant (and not practically feasible) for them? Why not allow both? > > We need to be able to report the information that's currently encoded > > in the "jedec,lpddr2" binding separately for each channel+rank > > combination, and we need to be able to tell how many LPDDR chips are > > combined under a single memory channel. > > Who and why needs that information? > > To me it's not a very useful information without knowing how memory > ranges are mapped to the chips and then only kernel drivers should be > able to utilize that info in a meaningful way. What driver are we > talking about? We're using this for diagnostic purposes, to be able to accurately report the installed memory configuration to the user or in automated error reporting. We're planning to just read it from userspace (via /proc/device-tree) and not actually add a kernel driver for it, but since it needs to come from the firmware through the device tree it should have a standardized binding all the same.
