Duplicated ABI entries - Was: Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output

From: Mauro Carvalho Chehab
Date: Tue Nov 10 2020 - 02:27:30 EST

Next message: Zong Li: "[PATCH v2 0/3] clk: add driver for the SiFive FU740"
Previous message: rentao . bupt: "[PATCH] ARM: dts: aspeed: minipack: Fixup I2C tree"
In reply to: Jonathan Cameron: "Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output"
Next in thread: Randy Dunlap: "Re: Duplicated ABI entries - Was: Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Jonathan,

Em Sun, 8 Nov 2020 16:56:21 +0000
Jonathan Cameron <jic23@xxxxxxxxxx> escreveu:

> > PS.: the IIO subsystem is the one that currently has more duplicated
> > ABI entries:
> > $ ./scripts/get_abi.pl validate 2>&1|grep iio
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_accel_x_calibbias is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-icm42600:0 Documentation/ABI/testing/sysfs-bus-iio:394
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_accel_y_calibbias is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-icm42600:1 Documentation/ABI/testing/sysfs-bus-iio:395
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_accel_z_calibbias is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-icm42600:2 Documentation/ABI/testing/sysfs-bus-iio:396
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_anglvel_x_calibbias is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-icm42600:3 Documentation/ABI/testing/sysfs-bus-iio:397
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_anglvel_y_calibbias is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-icm42600:4 Documentation/ABI/testing/sysfs-bus-iio:398
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_anglvel_z_calibbias is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-icm42600:5 Documentation/ABI/testing/sysfs-bus-iio:399
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_count0_preset is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:100 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:0
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:117 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:14
> > Warning: /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available is defined 3 times: Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8:2 Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:111 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:8
> > Warning: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371:0 Documentation/ABI/testing/sysfs-bus-iio:599
> > Warning: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_powerdown is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371:36 Documentation/ABI/testing/sysfs-bus-iio:588
> > Warning: /sys/bus/iio/devices/iio:deviceX/out_currentY_raw is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-light-lm3533-als:43 Documentation/ABI/testing/sysfs-bus-iio-health-afe440x:38
> > Warning: /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010:0 Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc100x:0
> > Warning: /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw_available is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010:1 Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc100x:1
> > Warning: /sys/bus/iio/devices/iio:deviceX/sensor_sensitivity is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-distance-srf08:0 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935:8
> > Warning: /sys/bus/iio/devices/triggerX/sampling_frequency is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:92 Documentation/ABI/testing/sysfs-bus-iio:45

>
> That was intentional. Often these provide more information on the
> ABI for a particular device than is present in the base ABI doc.

FYI, right now, there are 20 duplicated entries, being 16 of them
from IIO, on those files:

$ ./scripts/get_abi.pl validate 2>&1|perl -ne 'if (m,(Documentation/\S+)\:,g) { print "$1\n" }'|sort|uniq
Documentation/ABI/stable/sysfs-driver-w1_ds28e04
Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
Documentation/ABI/testing/sysfs-bus-iio-distance-srf08
Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371
Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010
Documentation/ABI/testing/sysfs-bus-iio-icm42600
Documentation/ABI/testing/sysfs-bus-iio-light-lm3533-als
Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
Documentation/ABI/testing/sysfs-class-backlight-adp8860
Documentation/ABI/testing/sysfs-class-led-trigger-pattern
Documentation/ABI/testing/sysfs-kernel-iommu_groups

>
> A bit like when we have additional description for dt binding properties
> for a particular device, even though they are standard properties.
>
> Often a standard property allows for more values than the specific
> one for a particular device. There can also be obscuring coupling
> between sysfs attributes due to hardware restrictions that we would
> like to provide some explanatory info on.
>
> I suppose we could add all this information to the parent doc but
> that is pretty ugly and will make that doc very nasty to read.

I understand what you meant to do, but right now, it is is actually
a lot uglier than merging into a single entry ;-)

Let's view ABI from the PoV of a system admin that doesn't know
yet about a certain ABI symbol.

He'll try to seek for the symbol, more likely using the HTML
documentation. Only very senior system admins might try to take
a look at the Kernel.

This is what happens when one would seek for a duplicated symbol
via command line:

$ ./scripts/get_abi.pl search /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency$

/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency
----------------------------------------------------------

Kernel version: 3.4.0
Contact: linux-iio@xxxxxxxxxxxxxxx
Defined on file(s): Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371 Documentation/ABI/testing/sysfs-bus-iio

Description:

Stores the PLL frequency in Hz for channel Y.
Reading returns the actual frequency in Hz.
The ADF4371 has an integrated VCO with fundamendal output
frequency ranging from 4000000000 Hz 8000000000 Hz.

out_altvoltage0_frequency:
A divide by 1, 2, 4, 8, 16, 32 or circuit generates
frequencies from 62500000 Hz to 8000000000 Hz.
out_altvoltage1_frequency:
This channel duplicates the channel 0 frequency
out_altvoltage2_frequency:
A frequency doubler generates frequencies from
8000000000 Hz to 16000000000 Hz.
out_altvoltage3_frequency:
A frequency quadrupler generates frequencies from
16000000000 Hz to 32000000000 Hz.

Note: writes to one of the channels will affect the frequency of
all the other channels, since it involves changing the VCO
fundamental output frequency.

Output frequency for channel Y in Hz. The number must always be
specified and unique if the output corresponds to a single
channel.

As the "What:" field is identical on both sysfs-bus-iio-frequency-adf4371
and sysfs-bus-iio, those entries are merged, which produces an ABI
documentation mixing both the generic one and the board specific one
into a single output.

Worse than that, the "generic" content is at the end.

The same happens when generating the HTML output.

See, entries at the HTML output are ordered by the What: field,
which is considered within the script as an unique key, as it is
unique (except for IIO and a couple of other cases).

-

As I commented on an e-mail I sent to Greg, I see a few ways
to solve it.

The most trivial one (which I used to solve a few conflicts on
other places), is to place driver-specific details on a separate
file under Documentation/driver-api, and mention it at the
generic entries. The docs building system will generate cross
references for Documentation/.../foo.rst files, so, everything
should be OK.

The second alternative that I also used on a couple of places
is to modify the generic entry for it to contain the generic
definition first, followed by per-device details.

There is a third possible alternative: add a new optional field
(something like Scope:) which would be part of the unique key,
if present. Implementing support for it could be tricky, as the
produced output would likely need to create cross-references
between the generic field (if present) and the per-device details.

Thanks,
Mauro

PS.: I'm taking a few days of PTO during this week. So, it
could take a while for me to reply again to this thread.

Next message: Zong Li: "[PATCH v2 0/3] clk: add driver for the SiFive FU740"
Previous message: rentao . bupt: "[PATCH] ARM: dts: aspeed: minipack: Fixup I2C tree"
In reply to: Jonathan Cameron: "Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output"
Next in thread: Randy Dunlap: "Re: Duplicated ABI entries - Was: Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]