diff mbox series

[v3,3/3] iio:Documentation: Add documentation for hinge sensor channels

Message ID 20201203035352.13918-4-xiang.ye@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] HID: hid-sensor-custom: Add custom sensor iio support | expand

Commit Message

Ye Xiang Dec. 3, 2020, 3:53 a.m. UTC
Add channel description for hinge sensor, including channel label
attribute and raw data description.

Signed-off-by: Ye Xiang <xiang.ye@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Srinivas Pandruvada Dec. 4, 2020, 11:40 a.m. UTC | #1
On Thu, 2020-12-03 at 11:53 +0800, Ye Xiang wrote:
> Add channel description for hinge sensor, including channel label
> attribute and raw data description.
> 
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
> b/Documentation/ABI/testing/sysfs-bus-iio
> index df42bed09f25..82303b1bdff0 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1802,3 +1802,20 @@ Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Unscaled light intensity according to CIE 1931/DIN 5033
> color space.
>  		Units after application of scale are nano nanowatts per
> square meter.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
Channel "Y" doesn't sound to be a variable. Why not just say "*"

Thanks,
Srinivas

> +KernelVersion:	5.12
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Angle of rotation for channel Y. Units after
> application of scale
> +		and offset are radians.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
> +KernelVersion:	5.12
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Optional symbolic label for channel Y.
> +		For Intel hid hinge sensor, the label values are:
> +		hinge, keyboard, screen. It means the three channels
> +		each correspond respectively to hinge angle, keyboard
> angle,
> +		and screen angle.
Jonathan Cameron Dec. 5, 2020, 3:52 p.m. UTC | #2
On Fri, 04 Dec 2020 03:40:15 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Thu, 2020-12-03 at 11:53 +0800, Ye Xiang wrote:
> > Add channel description for hinge sensor, including channel label
> > attribute and raw data description.
> > 
> > Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio
> > b/Documentation/ABI/testing/sysfs-bus-iio
> > index df42bed09f25..82303b1bdff0 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -1802,3 +1802,20 @@ Contact:	linux-iio@vger.kernel.org
> >  Description:
> >  		Unscaled light intensity according to CIE 1931/DIN 5033
> > color space.
> >  		Units after application of scale are nano nanowatts per
> > square meter.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw  
> Channel "Y" doesn't sound to be a variable. Why not just say "*"

Long term convention.  More generally you need a way to refer to the
individual wild cards. Sometimes there are more than the two we have
here.

Not entirely sure where the convention comes from but it is used
throughout this file and I doubt I invented it.

Jonathan


> 
> Thanks,
> Srinivas
> 
> > +KernelVersion:	5.12
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Angle of rotation for channel Y. Units after
> > application of scale
> > +		and offset are radians.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
> > +KernelVersion:	5.12
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Optional symbolic label for channel Y.
> > +		For Intel hid hinge sensor, the label values are:
> > +		hinge, keyboard, screen. It means the three channels
> > +		each correspond respectively to hinge angle, keyboard
> > angle,
> > +		and screen angle.  
>
Jonathan Cameron Dec. 5, 2020, 4:05 p.m. UTC | #3
On Thu,  3 Dec 2020 11:53:52 +0800
Ye Xiang <xiang.ye@intel.com> wrote:

> Add channel description for hinge sensor, including channel label
> attribute and raw data description.
> 
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index df42bed09f25..82303b1bdff0 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1802,3 +1802,20 @@ Contact:	linux-iio@vger.kernel.org
>  Description:
>  		Unscaled light intensity according to CIE 1931/DIN 5033 color space.
>  		Units after application of scale are nano nanowatts per square meter.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
> +KernelVersion:	5.12
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Angle of rotation for channel Y. Units after application of scale
> +		and offset are radians.

This entry is already mostly in the doc around line 200, just without the index.
Please just add the What: line to that block to reduce repetition.
If you want to add... "Where present, Y indexes the channel." or something like
that feel free.


> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
> +KernelVersion:	5.12
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Optional symbolic label for channel Y.
> +		For Intel hid hinge sensor, the label values are:
> +		hinge, keyboard, screen. It means the three channels
> +		each correspond respectively to hinge angle, keyboard angle,
> +		and screen angle.

Makes sense to keep this block separate given the additional info provided.
Alternative would be to add it to the one with in_voltageY_label which would
be odd given what follows!

Thanks,

Jonathan
Ye Xiang Dec. 7, 2020, 9:18 a.m. UTC | #4
Hi Jonathan

Thanks for review and comments.

On Sat, Dec 05, 2020 at 04:05:40PM +0000, Jonathan Cameron wrote:
> On Thu,  3 Dec 2020 11:53:52 +0800
> Ye Xiang <xiang.ye@intel.com> wrote:
> 
> > Add channel description for hinge sensor, including channel label
> > attribute and raw data description.
> > 
> > Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index df42bed09f25..82303b1bdff0 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -1802,3 +1802,20 @@ Contact:	linux-iio@vger.kernel.org
> >  Description:
> >  		Unscaled light intensity according to CIE 1931/DIN 5033 color space.
> >  		Units after application of scale are nano nanowatts per square meter.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
> > +KernelVersion:	5.12
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Angle of rotation for channel Y. Units after application of scale
> > +		and offset are radians.
> 
> This entry is already mostly in the doc around line 200, just without the index.
> Please just add the What: line to that block to reduce repetition.
> If you want to add... "Where present, Y indexes the channel." or something like
> that feel free.
When adding in_anglY_raw to in_angl_raw block, Should I update the KernelVersion form
4.17 to 5.12? Like blow:

What:		/sys/bus/iio/devices/iio:deviceX/in_angl_raw
What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
KernelVersion:	5.12
Contact:	linux-iio@vger.kernel.org
Description:
		Angle of rotation. Units after application of scale and offset
		are radians. Where present, Y indexes the channel.

> 
> 
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
> > +KernelVersion:	5.12
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Optional symbolic label for channel Y.
> > +		For Intel hid hinge sensor, the label values are:
> > +		hinge, keyboard, screen. It means the three channels
> > +		each correspond respectively to hinge angle, keyboard angle,
> > +		and screen angle.
> 
> Makes sense to keep this block separate given the additional info provided.
> Alternative would be to add it to the one with in_voltageY_label which would
> be odd given what follows!
Ok, Then I will keep the in_anglY_label block here. BTW, I didnot see in_voltageY_label
in sysfs-bus-iio in current kernel master branch.

> 
> Thanks,
> 
> Jonathan
> 
Thanks
Ye, Xiang
>
Jonathan Cameron Dec. 13, 2020, 2:15 p.m. UTC | #5
On Mon, 7 Dec 2020 17:18:18 +0800
"Ye, Xiang" <xiang.ye@intel.com> wrote:

> Hi Jonathan
> 
> Thanks for review and comments.
> 
> On Sat, Dec 05, 2020 at 04:05:40PM +0000, Jonathan Cameron wrote:
> > On Thu,  3 Dec 2020 11:53:52 +0800
> > Ye Xiang <xiang.ye@intel.com> wrote:
> >   
> > > Add channel description for hinge sensor, including channel label
> > > attribute and raw data description.
> > > 
> > > Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > > index df42bed09f25..82303b1bdff0 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > > @@ -1802,3 +1802,20 @@ Contact:	linux-iio@vger.kernel.org
> > >  Description:
> > >  		Unscaled light intensity according to CIE 1931/DIN 5033 color space.
> > >  		Units after application of scale are nano nanowatts per square meter.
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
> > > +KernelVersion:	5.12
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Angle of rotation for channel Y. Units after application of scale
> > > +		and offset are radians.  
> > 
> > This entry is already mostly in the doc around line 200, just without the index.
> > Please just add the What: line to that block to reduce repetition.
> > If you want to add... "Where present, Y indexes the channel." or something like
> > that feel free.  
> When adding in_anglY_raw to in_angl_raw block, Should I update the KernelVersion form
> 4.17 to 5.12? Like blow:
> 
> What:		/sys/bus/iio/devices/iio:deviceX/in_angl_raw
> What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
> KernelVersion:	5.12
> Contact:	linux-iio@vger.kernel.org
> Description:
> 		Angle of rotation. Units after application of scale and offset
> 		are radians. Where present, Y indexes the channel.

No. This is an unfortunate limitation of these files unless we keep duplicating
nearly identical text.

I'm not that worried about documenting an optional bit of ABI as apparently
being in an earlier kernel than it actually was.  That way any userspace
will work.  The other way around is more likely to cause problems.

> 
> > 
> >   
> > > +
> > > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
> > > +KernelVersion:	5.12
> > > +Contact:	linux-iio@vger.kernel.org
> > > +Description:
> > > +		Optional symbolic label for channel Y.
> > > +		For Intel hid hinge sensor, the label values are:
> > > +		hinge, keyboard, screen. It means the three channels
> > > +		each correspond respectively to hinge angle, keyboard angle,
> > > +		and screen angle.  
> > 
> > Makes sense to keep this block separate given the additional info provided.
> > Alternative would be to add it to the one with in_voltageY_label which would
> > be odd given what follows!  
> Ok, Then I will keep the in_anglY_label block here. BTW, I didnot see in_voltageY_label
> in sysfs-bus-iio in current kernel master branch.
It was part of the series that added the support for per channel labels to the core.
3079188f821c ("iio:Documentation: Add documentation for label channel attribute")
so will only going into current master in the merge window that will open shortly.

Thanks,

Jonathan

> 
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> Thanks
> Ye, Xiang
> >
Ye Xiang Dec. 14, 2020, 6:16 a.m. UTC | #6
On Sun, Dec 13, 2020 at 02:15:45PM +0000, Jonathan Cameron wrote:
> On Mon, 7 Dec 2020 17:18:18 +0800
> "Ye, Xiang" <xiang.ye@intel.com> wrote:
> 
> > Hi Jonathan
> > 
> > Thanks for review and comments.
> > 
> > On Sat, Dec 05, 2020 at 04:05:40PM +0000, Jonathan Cameron wrote:
> > > On Thu,  3 Dec 2020 11:53:52 +0800
> > > Ye Xiang <xiang.ye@intel.com> wrote:
> > >   
> > > > Add channel description for hinge sensor, including channel label
> > > > attribute and raw data description.
> > > > 
> > > > Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-bus-iio | 17 +++++++++++++++++
> > > >  1 file changed, 17 insertions(+)
> > > > 
> > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > > > index df42bed09f25..82303b1bdff0 100644
> > > > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > > > @@ -1802,3 +1802,20 @@ Contact:	linux-iio@vger.kernel.org
> > > >  Description:
> > > >  		Unscaled light intensity according to CIE 1931/DIN 5033 color space.
> > > >  		Units after application of scale are nano nanowatts per square meter.
> > > > +
> > > > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
> > > > +KernelVersion:	5.12
> > > > +Contact:	linux-iio@vger.kernel.org
> > > > +Description:
> > > > +		Angle of rotation for channel Y. Units after application of scale
> > > > +		and offset are radians.  
> > > 
> > > This entry is already mostly in the doc around line 200, just without the index.
> > > Please just add the What: line to that block to reduce repetition.
> > > If you want to add... "Where present, Y indexes the channel." or something like
> > > that feel free.  
> > When adding in_anglY_raw to in_angl_raw block, Should I update the KernelVersion form
> > 4.17 to 5.12? Like blow:
> > 
> > What:		/sys/bus/iio/devices/iio:deviceX/in_angl_raw
> > What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
> > KernelVersion:	5.12
> > Contact:	linux-iio@vger.kernel.org
> > Description:
> > 		Angle of rotation. Units after application of scale and offset
> > 		are radians. Where present, Y indexes the channel.
> 
> No. This is an unfortunate limitation of these files unless we keep duplicating
> nearly identical text.
> 
> I'm not that worried about documenting an optional bit of ABI as apparently
> being in an earlier kernel than it actually was.  That way any userspace
> will work.  The other way around is more likely to cause problems.
Got it, then I will keep KernelVersion 4.17. will send the next version out
for review soon. Thanks again for the review.
> 
> > 
> > > 
> > >   
> > > > +
> > > > +What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
> > > > +KernelVersion:	5.12
> > > > +Contact:	linux-iio@vger.kernel.org
> > > > +Description:
> > > > +		Optional symbolic label for channel Y.
> > > > +		For Intel hid hinge sensor, the label values are:
> > > > +		hinge, keyboard, screen. It means the three channels
> > > > +		each correspond respectively to hinge angle, keyboard angle,
> > > > +		and screen angle.  
> > > 
> > > Makes sense to keep this block separate given the additional info provided.
> > > Alternative would be to add it to the one with in_voltageY_label which would
> > > be odd given what follows!  
> > Ok, Then I will keep the in_anglY_label block here. BTW, I didnot see in_voltageY_label
> > in sysfs-bus-iio in current kernel master branch.
> It was part of the series that added the support for per channel labels to the core.
> 3079188f821c ("iio:Documentation: Add documentation for label channel attribute")
> so will only going into current master in the merge window that will open shortly.
got it.
> 

Thanks
Ye, Xiang
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index df42bed09f25..82303b1bdff0 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1802,3 +1802,20 @@  Contact:	linux-iio@vger.kernel.org
 Description:
 		Unscaled light intensity according to CIE 1931/DIN 5033 color space.
 		Units after application of scale are nano nanowatts per square meter.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_raw
+KernelVersion:	5.12
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Angle of rotation for channel Y. Units after application of scale
+		and offset are radians.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_anglY_label
+KernelVersion:	5.12
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Optional symbolic label for channel Y.
+		For Intel hid hinge sensor, the label values are:
+		hinge, keyboard, screen. It means the three channels
+		each correspond respectively to hinge angle, keyboard angle,
+		and screen angle.