On 26/07/2022 18:35, Krzysztof Kozlowski wrote: > On 26/07/2022 19:07, Conor Dooley wrote: >> From: Atul Khare <atulkhare@xxxxxxxxxxxx> >> >> Fixes Running device tree schema validation error messages like >> '... cache-sets:0:0: 1024 was expected'. >> >> The existing bindings had a single enumerated value of 1024, which >> trips up the dt-schema checks. The ISA permits any arbitrary power >> of two for the cache-sets value, but we decided to add the single >> additional value of 2048 because we couldn't spot an obvious way >> to express the constraint in the schema. > > There is no way to express "power of two" but enum for multiple values > would work. Is there a reason to limit only to 2048? Copy pasting from the cover: > I don't think that there's value in speculatively adding values to this > enum especially as (I think at least) the scala for this cache IP has > been released publicly: > https://github.com/sifive/block-inclusivecache-sifive/blob/master/design/craft/inclusivecache/src/Parameters.scala#L32 > > The two compatibles in the file match only against two specific cache > implemenations: the fu540's & the fu740's. I would seem to me that, it > would be better to lock this to a single value per compatible since the > 1024 applies to the fu540 & the new value of 2048 applies only to the > fu740. > > I have not made that change, I simply wanted to repackage this series > in a way that could be more easily applied & restart the discussion. TL;DR: I would limit it to 1024 & 2048 to match the only implementations although not in the way this patch did it. Hope that helps, Conor.
