| Message ID | 20240603012200.16589-2-kimseer.paller@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add driver for LTC2664 and LTC2672 | expand |
On Mon, 3 Jun 2024 09:21:56 +0800 Kim Seer Paller <kimseer.paller@analog.com> wrote: > Introduces a more generalized ABI documentation for DAC. Instead of > having separate ABI files for each DAC, we now have a single ABI file > that covers the common sysfs interface for all DAC. > > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> A few comments inline. I wondered if it made sense to combine voltage and current entries of each type in single block, but I think the docs would become too complicated with lots of wild cards etc. Hence I think the duplication is fine. Jonathan > --- > Documentation/ABI/testing/sysfs-bus-iio-dac | 61 +++++++++++++++++++ > .../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 ---------- > 2 files changed, 61 insertions(+), 31 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac > new file mode 100644 > index 000000000000..36d316bb75f6 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac > @@ -0,0 +1,61 @@ > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en > +KernelVersion: 5.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This Tab vs space issue - see below. > + is useful when one wants to change the DAC output codes. The way > + it should be done is: > + > + - disable toggle operation; > + - change out_currentY_rawN, where N is the integer value of the symbol; > + - enable toggle operation. Same question as below on whether this is accurate - Maybe it just needs to mention this scheme needs to be used for autonomous toggling (out of software control). It works for software toggling but may be overkill! > + > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN > +KernelVersion: 5.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute has the same meaning as out_currentY_raw. It is > + specific to toggle enabled channels and refers to the DAC output > + code in INPUT_N (_rawN), where N is the integer value of the symbol. > + The same scale and offset as in out_currentY_raw applies. > + > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol > +KernelVersion: 5.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + Performs a SW switch to a predefined output symbol. This attribute > + is specific to toggle enabled channels and allows switching between > + multiple predefined symbols. Each symbol corresponds to a different > + output, denoted as out_currentY_rawN, where N is the integer value > + of the symbol. Writing an integer value N will select out_currentY_rawN. > + > +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en > +KernelVersion: 5.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This Mix of spacing and tabs is inconsistent. Hence the odd indent in this reply version. > + is useful when one wants to change the DAC output codes. The way > + it should be done is: Hmm. Is this true? If we are doing autonomous toggling on a clock or similar than agreed. If we are using the out_current_symbol software control it would be common to switch to A, modify B, switch to B, modify A etc. I think our interface has probably evolved and so this might need an update. > + > + - disable toggle operation; > + - change out_voltageY_rawN, where N is the integer value of the symbol; > + - enable toggle operation.
> -----Original Message----- > From: Jonathan Cameron <jic23@kernel.org> > Sent: Saturday, June 8, 2024 10:41 PM > To: Paller, Kim Seer <KimSeer.Paller@analog.com> > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; David Lechner <dlechner@baylibre.com>; Lars- > Peter Clausen <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; > Mark Brown <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; > Krzysztof Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; > Conor Dooley <conor+dt@kernel.org>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com> > Subject: Re: [PATCH v3 1/5] iio: ABI: Generalize ABI documentation for DAC > > [External] > > On Mon, 3 Jun 2024 09:21:56 +0800 > Kim Seer Paller <kimseer.paller@analog.com> wrote: > > > Introduces a more generalized ABI documentation for DAC. Instead of > > having separate ABI files for each DAC, we now have a single ABI file > > that covers the common sysfs interface for all DAC. > > > > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > > A few comments inline. > > I wondered if it made sense to combine voltage and current entries of each > type > in single block, but I think the docs would become too complicated with lots > of wild cards etc. Hence I think the duplication is fine. > > Jonathan > > > --- > > Documentation/ABI/testing/sysfs-bus-iio-dac | 61 +++++++++++++++++++ > > .../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 ---------- > > 2 files changed, 61 insertions(+), 31 deletions(-) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac > b/Documentation/ABI/testing/sysfs-bus-iio-dac > > new file mode 100644 > > index 000000000000..36d316bb75f6 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac > > @@ -0,0 +1,61 @@ > > +What: > /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en > > +KernelVersion: 5.18 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This > Tab vs space issue - see below. > > > + is useful when one wants to change the DAC output codes. The > way > > + it should be done is: > > + > > + - disable toggle operation; > > + - change out_currentY_rawN, where N is the integer value of the > symbol; > > + - enable toggle operation. > Same question as below on whether this is accurate - Maybe it just needs to > mention > this scheme needs to be used for autonomous toggling (out of software > control). > It works for software toggling but may be overkill! > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN > > +KernelVersion: 5.18 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + This attribute has the same meaning as out_currentY_raw. It is > > + specific to toggle enabled channels and refers to the DAC > output > > + code in INPUT_N (_rawN), where N is the integer value of the > symbol. > > + The same scale and offset as in out_currentY_raw applies. > > + > > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol > > +KernelVersion: 5.18 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Performs a SW switch to a predefined output symbol. This > attribute > > + is specific to toggle enabled channels and allows switching > between > > + multiple predefined symbols. Each symbol corresponds to a > different > > + output, denoted as out_currentY_rawN, where N is the integer > value > > + of the symbol. Writing an integer value N will select > out_currentY_rawN. > > + > > +What: > /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en > > +KernelVersion: 5.18 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This > > Mix of spacing and tabs is inconsistent. Hence the odd indent in this reply > version. > > > + is useful when one wants to change the DAC output codes. The > way > > + it should be done is: > > Hmm. Is this true? If we are doing autonomous toggling on a clock or similar > than agreed. > If we are using the out_current_symbol software control it would be common > to switch > to A, modify B, switch to B, modify A etc. > > I think our interface has probably evolved and so this might need an update. I agree that the description could be clear about the differences between autonomous and software toggling. If we were to change the description, would this suffice? Description: Toggle enable. Write 1 to enable toggle or 0 to disable it. This is useful when one wants to change the DAC output codes. For autonomous toggling, the way it should be done is: - disable toggle operation; - change out_currentY_rawN, where N is the integer value of the symbol; - enable toggle operation. For software toggling, one can switch to A, modify B, switch to B, modify A, etc. > > > + > > + - disable toggle operation; > > + - change out_voltageY_rawN, where N is the integer value of the > symbol; > > + - enable toggle operation.
On Wed, 12 Jun 2024 10:57:42 +0000 "Paller, Kim Seer" <KimSeer.Paller@analog.com> wrote: > > -----Original Message----- > > From: Jonathan Cameron <jic23@kernel.org> > > Sent: Saturday, June 8, 2024 10:41 PM > > To: Paller, Kim Seer <KimSeer.Paller@analog.com> > > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > > devicetree@vger.kernel.org; David Lechner <dlechner@baylibre.com>; Lars- > > Peter Clausen <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; > > Mark Brown <broonie@kernel.org>; Dimitri Fedrau <dima.fedrau@gmail.com>; > > Krzysztof Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; > > Conor Dooley <conor+dt@kernel.org>; Hennerich, Michael > > <Michael.Hennerich@analog.com>; Nuno Sá <noname.nuno@gmail.com> > > Subject: Re: [PATCH v3 1/5] iio: ABI: Generalize ABI documentation for DAC > > > > [External] > > > > On Mon, 3 Jun 2024 09:21:56 +0800 > > Kim Seer Paller <kimseer.paller@analog.com> wrote: > > > > > Introduces a more generalized ABI documentation for DAC. Instead of > > > having separate ABI files for each DAC, we now have a single ABI file > > > that covers the common sysfs interface for all DAC. > > > > > > Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> > > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> > > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > > > > A few comments inline. > > > > I wondered if it made sense to combine voltage and current entries of each > > type > > in single block, but I think the docs would become too complicated with lots > > of wild cards etc. Hence I think the duplication is fine. > > > > Jonathan > > > > > --- > > > Documentation/ABI/testing/sysfs-bus-iio-dac | 61 +++++++++++++++++++ > > > .../ABI/testing/sysfs-bus-iio-dac-ltc2688 | 31 ---------- > > > 2 files changed, 61 insertions(+), 31 deletions(-) > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac > > b/Documentation/ABI/testing/sysfs-bus-iio-dac > > > new file mode 100644 > > > index 000000000000..36d316bb75f6 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac > > > @@ -0,0 +1,61 @@ > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en > > > +KernelVersion: 5.18 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This > > Tab vs space issue - see below. > > > > > + is useful when one wants to change the DAC output codes. The > > way > > > + it should be done is: > > > + > > > + - disable toggle operation; > > > + - change out_currentY_rawN, where N is the integer value of the > > symbol; > > > + - enable toggle operation. > > Same question as below on whether this is accurate - Maybe it just needs to > > mention > > this scheme needs to be used for autonomous toggling (out of software > > control). > > It works for software toggling but may be overkill! > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN > > > +KernelVersion: 5.18 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + This attribute has the same meaning as out_currentY_raw. It is > > > + specific to toggle enabled channels and refers to the DAC > > output > > > + code in INPUT_N (_rawN), where N is the integer value of the > > symbol. > > > + The same scale and offset as in out_currentY_raw applies. > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol > > > +KernelVersion: 5.18 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Performs a SW switch to a predefined output symbol. This > > attribute > > > + is specific to toggle enabled channels and allows switching > > between > > > + multiple predefined symbols. Each symbol corresponds to a > > different > > > + output, denoted as out_currentY_rawN, where N is the integer > > value > > > + of the symbol. Writing an integer value N will select > > out_currentY_rawN. > > > + > > > +What: > > /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en > > > +KernelVersion: 5.18 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Toggle enable. Write 1 to enable toggle or 0 to disable it. This > > > > Mix of spacing and tabs is inconsistent. Hence the odd indent in this reply > > version. > > > > > + is useful when one wants to change the DAC output codes. The > > way > > > + it should be done is: > > > > Hmm. Is this true? If we are doing autonomous toggling on a clock or similar > > than agreed. > > If we are using the out_current_symbol software control it would be common > > to switch > > to A, modify B, switch to B, modify A etc. > > > > I think our interface has probably evolved and so this might need an update. > > I agree that the description could be clear about the differences between > autonomous and software toggling. If we were to change the description, would > this suffice? > > Description: > Toggle enable. Write 1 to enable toggle or 0 to disable it. This > is useful when one wants to change the DAC output codes. For autonomous toggling, the way > it should be done is: > > - disable toggle operation; > - change out_currentY_rawN, where N is the integer value of the symbol; > - enable toggle operation. To here is good as focuses on the use case. > > For software toggling, one can switch to A, modify B, switch to B, modify A, etc. I'd not mention this part (not sure if you were intending to though given the formatting!) Jonathan > > > > > + > > > + - disable toggle operation; > > > + - change out_voltageY_rawN, where N is the integer value of the > > symbol; > > > + - enable toggle operation. > >
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac new file mode 100644 index 000000000000..36d316bb75f6 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac @@ -0,0 +1,61 @@ +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_toggle_en +KernelVersion: 5.18 +Contact: linux-iio@vger.kernel.org +Description: + Toggle enable. Write 1 to enable toggle or 0 to disable it. This + is useful when one wants to change the DAC output codes. The way + it should be done is: + + - disable toggle operation; + - change out_currentY_rawN, where N is the integer value of the symbol; + - enable toggle operation. + +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_rawN +KernelVersion: 5.18 +Contact: linux-iio@vger.kernel.org +Description: + This attribute has the same meaning as out_currentY_raw. It is + specific to toggle enabled channels and refers to the DAC output + code in INPUT_N (_rawN), where N is the integer value of the symbol. + The same scale and offset as in out_currentY_raw applies. + +What: /sys/bus/iio/devices/iio:deviceX/out_currentY_symbol +KernelVersion: 5.18 +Contact: linux-iio@vger.kernel.org +Description: + Performs a SW switch to a predefined output symbol. This attribute + is specific to toggle enabled channels and allows switching between + multiple predefined symbols. Each symbol corresponds to a different + output, denoted as out_currentY_rawN, where N is the integer value + of the symbol. Writing an integer value N will select out_currentY_rawN. + +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en +KernelVersion: 5.18 +Contact: linux-iio@vger.kernel.org +Description: + Toggle enable. Write 1 to enable toggle or 0 to disable it. This + is useful when one wants to change the DAC output codes. The way + it should be done is: + + - disable toggle operation; + - change out_voltageY_rawN, where N is the integer value of the symbol; + - enable toggle operation. + +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_rawN +KernelVersion: 5.18 +Contact: linux-iio@vger.kernel.org +Description: + This attribute has the same meaning as out_currentY_raw. It is + specific to toggle enabled channels and refers to the DAC output + code in INPUT_N (_rawN), where N is the integer value of the symbol. + The same scale and offset as in out_currentY_raw applies. + +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol +KernelVersion: 5.18 +Contact: linux-iio@vger.kernel.org +Description: + Performs a SW switch to a predefined output symbol. This attribute + is specific to toggle enabled channels and allows switching between + multiple predefined symbols. Each symbol corresponds to a different + output, denoted as out_voltageY_rawN, where N is the integer value + of the symbol. Writing an integer value N will select out_voltageY_rawN. diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 index 1c35971277ba..ae95a5477382 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ltc2688 @@ -53,34 +53,3 @@ KernelVersion: 5.18 Contact: linux-iio@vger.kernel.org Description: Returns the available values for the dither phase. - -What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_toggle_en -KernelVersion: 5.18 -Contact: linux-iio@vger.kernel.org -Description: - Toggle enable. Write 1 to enable toggle or 0 to disable it. This is - useful when one wants to change the DAC output codes. The way it should - be done is: - - - disable toggle operation; - - change out_voltageY_raw0 and out_voltageY_raw1; - - enable toggle operation. - -What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw0 -What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw1 -KernelVersion: 5.18 -Contact: linux-iio@vger.kernel.org -Description: - It has the same meaning as out_voltageY_raw. This attribute is - specific to toggle enabled channels and refers to the DAC output - code in INPUT_A (_raw0) and INPUT_B (_raw1). The same scale and offset - as in out_voltageY_raw applies. - -What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_symbol -KernelVersion: 5.18 -Contact: linux-iio@vger.kernel.org -Description: - Performs a SW toggle. This attribute is specific to toggle - enabled channels and allows to toggle between out_voltageY_raw0 - and out_voltageY_raw1 through software. Writing 0 will select - out_voltageY_raw0 while 1 selects out_voltageY_raw1.