Received-SPF: pass (google.com: domain of linux-kernel+bounces-211428-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Paller, Kim Seer" <KimSeer.Paller@analog.com>
To: Jonathan Cameron <jic23@kernel.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
        "devicetree@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>,
        =?iso-8859-1?Q?Nuno_S=E1?=
	<noname.nuno@gmail.com>
Subject: RE: [PATCH v3 1/5] iio: ABI: Generalize ABI documentation for DAC
Thread-Topic: [PATCH v3 1/5] iio: ABI: Generalize ABI documentation for DAC
Thread-Index: AQHatVSHmwOhJ2StkkygP01nVjkJQbG9+RGAgAYJYgA=
Date: Wed, 12 Jun 2024 10:57:42 +0000
Message-ID: 
 <PH0PR03MB71416493AB2788638599CAE4F9C02@PH0PR03MB7141.namprd03.prod.outlook.com>
References: <20240603012200.16589-1-kimseer.paller@analog.com>
	<20240603012200.16589-2-kimseer.paller@analog.com>
 <20240608154053.1cf1097e@jic23-huawei>
In-Reply-To: <20240608154053.1cf1097e@jic23-huawei>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?iso-8859-1?Q?x0R55pb7gReyu0CQtzYDX/sBiVhW85zGIfafOYW6jeNrEO13f8I7pRQ4tK?=
 =?iso-8859-1?Q?/IZnfXSMxqk3wADdEO96AaYk9wypiNAj8OyckmHCm81FFDp5dQERwqy7EP?=
 =?iso-8859-1?Q?QblD4vNbY0TNmlb8Db3Lm5i3J8vyx9UtQH/SOAqO36yg2k2Se0RVNstEUm?=
 =?iso-8859-1?Q?lo/lxd7b2nMTTXbI196mA4i33JWg9ANgbYdn+9m/jjG3U914D8js6HcRbv?=
 =?iso-8859-1?Q?EvzzzYM7eX+tueArMLSpYyKX3eHi86bJnScNhyoKVyDAuOxnclhCRvDrVo?=
 =?iso-8859-1?Q?Y74dZ7mk+VGahNqaHmYHCngMQn9boE6bJqR6KKfp7gH6r1PLQ7tmRdBgE4?=
 =?iso-8859-1?Q?5gAnrG6KXNR9saFQ+C761SeFMfAEAV4Z3MM/szH5/FAAYFMvMk5Wgq8htd?=
 =?iso-8859-1?Q?0nVAIMyQbyt/KHQUnfQGMgtNhKXEHjy9Add8kK9ULtzgvzUqD7OAfnhN/u?=
 =?iso-8859-1?Q?JiazrqLSFMHbbDO9/ToVmNDdtw+pZy9Mg3BRlLZguEjzhU+As3Pch1nR67?=
 =?iso-8859-1?Q?2IWkDsFSEiR3sR1NUDaEQjJj2XJnya9g7CrELbNK/xxzbOZW5uenf58OHn?=
 =?iso-8859-1?Q?Fd5KFmQG+wztNovSTGF79nkK4ARVnV03OrLEo9F78cPQNljGpGntHRvPoD?=
 =?iso-8859-1?Q?eM5dItWOkhUoMWUU0u+DLqWRVIQ5eX2dEgP0SKqaxHvOmkqo9BvN3CxGMr?=
 =?iso-8859-1?Q?ACvRurqspMkvnSKHwBph7zfhQyp9JLmxy1z9K+EoV4qijktSUr+5SAjQ7D?=
 =?iso-8859-1?Q?B8QcetsWfZFOlWc34mhb88oi2K/icUYM0Xoi+DXWR3K7DnH3+jC4kdsa8o?=
 =?iso-8859-1?Q?E6GPIYvZ7WNHBlekwbCALkf3xUE3NL0tQQpl8Q0g1H34E7R8yPzgXJcC9R?=
 =?iso-8859-1?Q?c7m2SSKKQ8odOo/gTYanA2w27xrm+sChjYaCX2JYUHHcXO1zjSt0ZDlKe9?=
 =?iso-8859-1?Q?ZEsdFj2UKLO+d2rp1uiAwG4j17svaFrtcpAmR1cTC9/A569yfGSUo7pIPj?=
 =?iso-8859-1?Q?g3jYpifTbnRgBcNn+CkmAgiSB13BclNaRn/0bAeFOdyF83tKefdMrlGdHt?=
 =?iso-8859-1?Q?h743Mwy1vQWnjDbHI5x6z8nEuPKZX/u3AuU3OqFFD1/LVvvdHL2zjfpV8g?=
 =?iso-8859-1?Q?3kxrpRKYZ9UsIs5b0Tw/aq/hkdk9AFnhjyQWUYFyF50vIgRtoVcJL8cHg8?=
 =?iso-8859-1?Q?oquT+P7T7fCkhgNVeYbb5TZHgnj/10+fdyz7X0ww/SpsF0nfak/PpZYL/z?=
 =?iso-8859-1?Q?HGatlTy9wyofc102L8qKaS9HaRDUCeYm/hnU6eIXVf86HoPGP/X2qj3Vo8?=
 =?iso-8859-1?Q?LPxIILUmP1n1tShbanEk9uaK7hr1ycKT8krd5Fd1nLDv2J5elp465D4cBy?=
 =?iso-8859-1?Q?Kchvz0a8639ltpRDl/1cQi8sdiN7CaBbPHV61ex5mCrwavPa1LOZ5fPhh3?=
 =?iso-8859-1?Q?wtSXXhDqgeEaS32S0aATrV10EKXo3WDqEC9X67QvLPw9yHT90g53zial+D?=
 =?iso-8859-1?Q?5TsDf1vDJiIkdny5jUrl9HMUGDrTF6/ZCN7nehf3FaJrNZM4eJsSKeoXoB?=
 =?iso-8859-1?Q?TDd2oFxlZI4QaeRgwHnRY+CjUUpa+IykNFR6OBAtSM3jC2UsbCq5QFPi8J?=
 =?iso-8859-1?Q?fgCOAjry4P3I8LL8IJFt4Nfyhxudd8so0j?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: analog.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR03MB7141.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 29d58d56-9dd0-4066-4590-08dc8ace7b98
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jun 2024 10:57:42.0646
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: eaa689b4-8f87-40e0-9c6f-7228de4d754a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: xL0CehJrJATF06uxbcuaWB9Z7+jKGXU58uyr1vcswDGc7TrsdrARjOj29MiiJjBufOCgqmtxxl1io0JL0sbpBfFfgvOzem949DJuGmEE9Cw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR03MB6368
X-Proofpoint-GUID: Yfx9neBHmGi2F-c6liv3t9oHNrbWecD0
X-Proofpoint-ORIG-GUID: Yfx9neBHmGi2F-c6liv3t9oHNrbWecD0
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-12_06,2024-06-12_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxlogscore=999
 priorityscore=1501 bulkscore=0 clxscore=1011 spamscore=0 phishscore=0
 mlxscore=0 lowpriorityscore=0 malwarescore=0 suspectscore=0
 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.19.0-2405170001 definitions=main-2406120079


> -----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=E1 <noname.nuno@gmail.com>
> Subject: Re: [PATCH v3 1/5] iio: ABI: Generalize ABI documentation for DA=
C
>=20
> [External]
>=20
> On Mon, 3 Jun 2024 09:21:56 +0800
> Kim Seer Paller <kimseer.paller@analog.com> wrote:
>=20
> > 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>
>=20
> A few comments inline.
>=20
> I wondered if it made sense to combine voltage and current entries of eac=
h
> type
> in single block, but I think the docs would become too complicated with l=
ots
> of wild cards etc.  Hence I think the duplication is fine.
>=20
> Jonathan
>=20
> > ---
> >  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. T=
his
> Tab vs space issue - see below.
>=20
> > +		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 t=
he
> 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!
>=20
> > +
> > +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. T=
his
>=20
> Mix of spacing and tabs is inconsistent. Hence the odd indent in this rep=
ly
> version.
>=20
> > +		is useful when one wants to change the DAC output codes. The
> way
> > +		it should be done is:
>=20
> Hmm. Is this true?  If we are doing autonomous toggling on a clock or sim=
ilar
> than agreed.
> If we are using the out_current_symbol software control it would be commo=
n
> to switch
> to A, modify B, switch to B, modify A etc.
>=20
> I think our interface has probably evolved and so this might need an upda=
te.

I agree that the description could be clear about the differences between=20
autonomous and software toggling. If we were to change the description, wou=
ld=20
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 autono=
mous toggling, the way
        it should be done is:

        - disable toggle operation;
        - change out_currentY_rawN, where N is the integer value of the sym=
bol;
        - enable toggle operation.

For software toggling, one can switch to A, modify B, switch to B, modify A=
, etc.
>=20
> > +
> > +        	- disable toggle operation;
> > +        	- change out_voltageY_rawN, where N is the integer value of t=
he
> symbol;
> > +        	- enable toggle operation.