On a very quick search: $ ./scripts/get_abi.pl search "\bmin.*max" I can't see any place using min and max at the same devnode. $ ./scripts/get_abi.pl search "\b(min|max)"|grep /sys/ |wc -l 234 So, it sounds to me that merging those into a single devnode is an API violation. > > There was another branch of the thread where Boris mentioned this as an > option. It isn't bad to deal with and an easy change to the code, > but I have an open question on what choice we make for representing > unknown min / max. For separate files the absence of the file > indicates we don't have any information. > > > > > > > > > > So in conclusion, I think the proposed multiple sysfs attribute style > > > with them reading back the most recent value written is the least bad > > > solution to a complex control interface. > > > > > > > > > > > echo ... > > > > > > > > then writes in the bank. > > > > > > > > > ... so we would propose we do not add max_ and min_ for now and see how the > > > > > use cases evolve. > > > > > > > > Yes, you should apply that same methodology to the rest of the new features > > > > you're adding: only add functionality for the stuff that is actually being > > > > used now. You can always extend it later. > > > > > > > > Changing an already user-visible API is a whole different story and a lot lot > > > > harder, even impossible. > > > > > > > > So I'd suggest you prune the EDAC patches from all the hypothetical usage and > > > > then send only what remains so that I can try to queue them. > > > > > > Sure. In this case the addition of min/max was perhaps a wrong response to > > > your request for a way to those ranges rather than just rejecting a write > > > of something out of range as earlier version did. > > > > > > We can revisit in future if range discovery becomes necessary. Personally > > > I don't think it is given we are only taking these actions in response error > > > records that give us precisely what to write and hence are always in range. > > > > For RO devnodes, there's no need for ranges, but those are likely needed for > > RW, as otherwise userspace may try to write invalid requests and/or have > > backward-compatibility issues. > > Given these parameters are only meaningfully written with values coming > ultimately from error records, userspace should never consider writing > something that is out of range except during testing. > > I don't mind presenting the range where known (in CXL case it is not > discoverable for most of them) but I wouldn't expect tooling to ever > read it as known correct values to write come from the error records. > Checking those values against provided limits seems an unnecessary step > given an invalid parameter that slips through will be rejected by the > hardware anyway. I'm fine starting without min/max if there's no current usecase, provided that: 1. when needed, we add min/max as separate devnodes; 2. there won't be any backward issues when min/max gets added. Regards, Mauro
