On Thu, Dec 14, 2023 at 12:12:42PM -0600, Rob Herring wrote: > On Thu, Dec 14, 2023 at 10:23 AM Conor Dooley <conor@xxxxxxxxxx> wrote: > > > > On Wed, Dec 13, 2023 at 05:24:55PM -0600, Rob Herring wrote: > > > Defining the size of register regions is not really in scope of what > > > bindings need to cover. The schema for this is also not completely correct > > > as a reg entry can be variable number of cells for the address and size, > > > but the schema assumes 1 cell. > > > > > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx> > > > > Does this not also remove restrictions on what the number in the reg > > entry is actually allowed to be? > > Yes, that's what I mean with the first sentence. We don't do this > anywhere else with the exception of some I2C devices with fixed > addresses. Keying off of the interrupt property also seems > questionable. If the register size is different, that should be a > different compatible. Reading the code, it appears the hardware always supported interrupts, however the first version of the driver never used them. It seems like some DT blobs had the register space cover just the needed registers for polling, and excluded the interrupt control register. When interrupt support was added, all in-tree DT files were updated with the extended register space, but to allow backwards compatibility, the driver checks the length of the register space and will not enable interrupts if its too small. I'm guessing that since the hardware did not change, a new compatible was not used when adding interrupt support. And the yaml is there to help when old out of tree .dts files are merged into the tree and have the old register space. This is and old driver, and its usage of DT is from long before many of the current best practices where determined, or yaml was even an idea. So i'm not surprised it has a few odd quirks. I don't see a reason not to remove these constraints, as i said, the driver should do the right thing if the register space it too small and YAML does not warn about it. Andrew