Jonathan Cameron wrote: > > > > > > * If for instance the generalized ABI unit is going to be Ohms, > > > should I still > > > remove the entry from the pac1934 even though it would not be fully > > > compliant > > > with the generalized ABI? > > > > > > * To cover the current exposed attributes, the "What" fields would > > > look like: > > > from max9611: > > > What: /sys/.../iio:deviceX/in_current_shunt_resistor > > > What: /sys/.../iio:deviceX/in_power_shunt_resistor > > > from ina2xx: > > > What: /sys/.../iio:deviceX/in_shunt_resistor > > > from pac1934: > > > What: /sys/.../iio:deviceX/in_shunt_resistorY > > This one is a bit odd in that it describes it if it were a measurable > channel in of itself but we probably couldn't figure out a better way > to scope it to a specific channel. > > > > Does this look correct? I think that for the first two drivers the > > > shunt_resistor can be considered as a channel info property, shared > > > by type for > > > max9611 case and shared by direction for ina2xx case (maybe better to > > > remove > > > "in_" from the What field if the type is not specified?). > > Keep it consistent. It's valid to provide the in_ and in general > over restrict channel attributes, even if not strictly necessary. > > > > What seems odd to me is the pac1934 case, since it doesn't fit in the > > > format > > > <type>[Y_]shunt_resistor referred in many other attributes (where I > > > assume > > > <type> is actually [dir_][type_]?). > > > Doesn't it look like pac1934 is exposing additional input channels, > > > that are > > > also writeable? Maybe such case would more clear if the shunt > > > resistor would be > > > an info property of specific channels? For example: > > > in_currentY_shunt_resistor, > > > in_powerY_shunt_resistor and in_engergyY_shunt_resitor. > > > > > > > > I don't think it will be a good idea to duplicate the same information > > into multiple attributes like: in_currentY_shunt_resistor, > > in_powerY_shunt_resistor and in_engergyY_shunt_resitor. > > > > The pac1934 device could be viewed like 4 devices that have only one > > measurement hardware. Changing the shunt for a hardware channel will > > impact multiple software measurements for that particular channel. > Yup. You've Sorry Jonathan, is there anything missing in this sentence? Looks like unintentionally truncated: You've ... > > > > For example "sampling_frequency" is only one property per device and > > not one property per channel. > > Not necessarily. If it varies per channel it is > in_voltageX_sampling_frequency etc > That is rare, but we have specific text to cover it in the ABI docs. > > What: /sys/bus/iio/devices/iio:deviceX/in_voltageX_sampling_frequency > What: /sys/bus/iio/devices/iio:deviceX/in_powerY_sampling_frequency > What: /sys/bus/iio/devices/iio:deviceX/in_currentZ_sampling_frequency > KernelVersion: 5.20 > Contact: linux-iio@xxxxxxxxxxxxxxx > Description: > Some devices have separate controls of sampling frequency for > individual channels. If multiple channels are enabled in a scan, > then the sampling_frequency of the scan may be computed from the > per channel sampling frequencies. > > > > > Also I'm not felling comfortable to remove the [dir_] from the name, > > because this value is dependent of the hardware and we can't have a > > "available" properties for it. > Removing the dir is unnecessary. Just leave that in place. > Note we can't change existing ABI of drivers for this sort of thing > that wasn't standardized (as we can't argue they break ABI) so > they are stuck as they stand. > > Unfortunately the most consistent path is probably to treat it as a > normal attribute, even if that generates a bunch of silly duplication > if there is more than one shunt_resistance. > I agree it's ugly but it's not the only case of this sort of duplication. > It happens for that sampling_frequency case in a few corners were there is > on channel that is sampled different from all the others. > > So I think > in_powerY_shut_resistor and in_energyY_shunt_resistor is > most consistent with existing 'standard' ABI. > > This is one where I didn't do a great job in review unfortunately > so the one with the index on the end got through. > > I'm not hugely worried about this mess though as runtime shunt resistor > calibration is not that common. If people want good measurements they > tend to build their circuit with good components / PCB tracks etc. >