Hi William Breathitt Gray, Thanks for the feedback. > Subject: Re: [PATCH v7 3/5] Documentation: ABI: sysfs-bus-counter: add > cascade_enable and external_input_phase_clock_select > > On Thu, Nov 24, 2022 at 05:00:16PM +0000, Biju Das wrote: > > This commit adds cascade_enable and external_input_phase_clock_ select > > items to counter ABI file. > > (e.g. for Renesas MTU3 hardware used for phase counting). > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > I have a few comments below left for this patch. Assuming these are > resolved, then I expect to ack this patch in the next submission. OK. > > > --- > > v6->v7: > > * Replaced long_word_access_ctrl_mode->cascade_enable > > * Updated Kernel version > > v5->v6: > > * No change > > v5: > > * New patch > > --- > > Documentation/ABI/testing/sysfs-bus-counter | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-counter > > b/Documentation/ABI/testing/sysfs-bus-counter > > index ff83320b4255..abc691b13b0f 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-counter > > +++ b/Documentation/ABI/testing/sysfs-bus-counter > > @@ -215,6 +215,22 @@ Contact: linux-iio@xxxxxxxxxxxxxxx > > Description: > > This attribute indicates the number of overflows of count Y. > > > > +What: /sys/bus/counter/devices/counterX/cascade_enable > > It's possible that in the future we might cascading other things as well, > so let's make this name more specific: "cascade_counts_enable". Agreed. > > > +KernelVersion: 6.3 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + This attribute indicates the cascading of counts on > > + counter X. > > Add a line stating this is a boolean attribute: "Valid attribute values > are boolean." Agreed, will add this to a new line. + Valid attribute values are boolean. > > > + > > +What: > /sys/bus/counter/devices/counterX/external_input_phase_clock_select > > +KernelVersion: 6.3 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + This attribute selects the external clock pin for phase > > + counting mode of counter X. > > This is a driver-specific enum attribute so it needs a corresponding > *_available entry. Take a look at the count_mode_available entry in this > file and use that as a template to create a new entry block for > external_input_phase_clock_select_available. Thanks. Will create new entry block for external_input_phase_clock_select_available > > > + > > +What: /sys/bus/counter/devices/counterX/cascade_enable > > +What: > /sys/bus/counter/devices/counterX/external_input_phase_clock_select > > These two lines are missing the '_id' suffix: "cascade_enable_id" and > "external_input_phase_clock_select_id". OK, will add _id suffix for both these entries. Cheers, Biju > > > What: > /sys/bus/counter/devices/counterX/countY/capture_component_id > > What: > /sys/bus/counter/devices/counterX/countY/ceiling_component_id > > What: > /sys/bus/counter/devices/counterX/countY/floor_component_id > > -- > > 2.25.1 > >