RE: [PATCH v7 3/5] Documentation: ABI: sysfs-bus-counter: add cascade_enable and external_input_phase_clock_select

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> >




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux