From: Drew Fustini <drew@beagleboard.org>
To: Linus Walleij <linus.walleij@linaro.org>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Lindgren <tony@atomide.com>,
Andy Shevchenko <andy.shevchenko@gmail.com>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
Jason Kridner <jkridner@beagleboard.org>,
Robert Nelson <robertcnelson@beagleboard.org>,
Joe Perches <joe@perches.com>,
Dan Carpenter <dan.carpenter@oracle.com>,
Jonathan Corbet <corbet@lwn.net>,
linux-doc@vger.kernel.org
Subject: Re: [PATCH v8 0/3] pinctrl: pinmux: Add pinmux-select debugfs file
Date: Fri, 26 Feb 2021 14:21:22 -0800 [thread overview]
Message-ID: <20210226222122.GA725305@x1> (raw)
In-Reply-To: <20210220202750.117421-1-drew@beagleboard.org>
On Sat, Feb 20, 2021 at 12:27:47PM -0800, Drew Fustini wrote:
> This series first converts the debugfs files in the pinctrl subsystem to
> octal permissions and then adds a new debugfs file "pinmux-select".
>
> Group name and function name can be written to "pinmux-select" which
> will cause the pin function for the specified group to be activated on
> the pin controller.
>
> The final patch in this series documents the debugfs files for pinctrl.
>
> Notes for PATCH v8:
> - add 'Reviewed-by:' from Geert Uytterhoeven for pinmux-select patch
> - add 'Tested-by:' from Geert Uytterhoeven for pinmux-select patch
> - change pinmux-select format to '<group-name function-name>' based on
> feedback from Geert
> - rephrase parts of documentation per Geert's comments
>
> Notes for PATCH v7:
> - add 'Reviewed-by:' from Andy Shevchenko for pinmux-select patch
> - add 'Reviewed-by:' from Andy Shevchenko for documentation patch
> - add 'Reviewed-by:' from Tony Lindgren to all patches
> - change order of '#include <linux/ctype.h>' per Andy's suggestion
> - change PINMUX_SELECT_MAX back to 128 as I had accidentally changed it
> to 50 and Andy pointed this out.
> - grammer fixes as suggested by Andy
> - rework assignment of fsel and ret from pinmux_func_name_to_selector()
> - rework assignment of gsel and ret from pinctrl_get_group_selector()
>
> Notes for PATCH v6:
> - add 'Suggested-by:' for Joe Perches to octal permissions patch
> - add 'Reviewed-by:' from Andy and Geert to octal permissions patch
> - reword example in the pinmux-select patch per Andy's advice
> - indent the example output per Andy's advice
> - remove usage error messages as Andy advised it is too verbose
> - return -ENOMEM when write is too big for the input buffer per Andy's advice
> - handle whitespace before, in between, and after the function name and
> group name as suggested by Andy
> - rename free_buf to exit_free_buf per Andy's advice
> - add documentation patch to series which documents the debugfs files
> for the pinctrl subsystem including the new pinmux-select file
>
> Notes for PATCH v5:
> - convert permissions from symbolic to octal for debugfs_create_file()
> calls in core.c that Joe Perches pointed out I had missed
> - Linus W: please let me know if I should break this series apart as you
> already applied an earlier version of octal conversion patch today [1]
> - switch from sscanf() to just pointing to function name and group name
> inside of the buffer. This also avoids having to allocate additional
> buffers for fname and gname. Geert and Andy highlighted this security
> issue and Andy suggested code to use instead of sscanf().
> - switch from devm_kfree() to kfree() after Dan Carpenter warned me
> - remove .read from pinmux_select_ops per Geert since it is write only
> - add usage format to error when unable find fname or gname in buffer
>
> Notes for PATCH v4:
> - correct the commit message in the second patch to reference function
> and group name instead of integer selectors. Apologies for not fixing
> that in v3
> - fix typos in cover letter
>
> Notes for PATCH v3:
> - add Suggested-by: Andy Shevchenko to the "pinctrl: use to octal
> permissions for debugfs files" patch
> - change the octal permissions from 0400 to 0444 to correctly match the
> symbolic permissions (thanks to Joe Perches and Geert Uytterhoeven)
> - note that S_IFREG flag is added to the mode in __debugfs_create_file()
> (thanks to Andy for highlighting this and Joe for suggesting I should
> add a note to the commit message)
> - fix order of the goto labels so that the buffers are freed correctly
> as suggested by Dan Carpenter
> - move from devm_kzalloc() to kzalloc() as the buffers are only used
> inside the pinmux_select() function and not related to the lifetime
> of the pin controller device (thanks to Andy for pointing this out)
> - correct the pinmux-select example in commit message to use the
> function and group name instead of selector (thanks to Geert)
>
> Notes for PATCH v2:
> - create patch series that includes patch to switch all the debugfs
> files in pinctrl subsystem over to octal permission
> - write function name and group name, instead of error-prone selector
> numbers, to the 'pinmux-select' file
> - switch from static to dynamic allocation for the kernel buffer filled
> by strncpy_from_user()
> - look up function selector from function name using
> pinmux_func_name_to_selector()
> - validate group name with get_function_groups() and match_string()
> - look up selector for group name with pinctrl_get_group_selector()
>
> Notes for PATCH v1:
> - posted seperate patch to switch all the debugfs files in pinctrl
> subsystem over to octal permission
> - there is no existing documentation for any of the debugfs enteries for
> pinctrl, so it seemed to have a bigger scope than just this patch. I
> also noticed that rst documentation is confusingly named "pinctl" (no
> 'r') and started thread about that [2]. Linus suggested chaning that
> to 'pin-control'. Thus I am planning a seperate documentation patch
> series where the file is renamed, references changed and a section on
> the pinctrl debugfs files is added.
>
> Notes for RFC v2 [3]:
> - rename debugfs file "pinmux-set" to "pinmux-select"
> - renmae pinmux_set_write() to pinmux_select()
> - switch from memdup_user_nul() to strncpy_from_user()
> - switch from pr_warn() to dev_err()
>
> [1] https://lore.kernel.org/linux-gpio/20210126044742.87602-1-drew@beagleboard.org/
> [2] https://lore.kernel.org/linux-gpio/20210126050817.GA187797@x1/
> [3] https://lore.kernel.org/linux-gpio/20210123064909.466225-1-drew@beagleboard.org/
>
> Drew Fustini (3):
> pinctrl: use to octal permissions for debugfs files
> pinctrl: pinmux: Add pinmux-select debugfs file
> docs/pinctrl: document debugfs files
>
> Documentation/driver-api/pinctl.rst | 37 ++++++++++
> drivers/pinctrl/core.c | 12 ++--
> drivers/pinctrl/pinconf.c | 4 +-
> drivers/pinctrl/pinmux.c | 106 +++++++++++++++++++++++++++-
> 4 files changed, 149 insertions(+), 10 deletions(-)
>
> --
> 2.25.1
>
Does any have any additional feedback for any of the patches in this
series?
Thank you!
DRew
prev parent reply other threads:[~2021-02-26 22:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-20 20:27 [PATCH v8 0/3] pinctrl: pinmux: Add pinmux-select debugfs file Drew Fustini
2021-02-20 20:27 ` [PATCH v8 1/3] pinctrl: use to octal permissions for debugfs files Drew Fustini
2021-02-20 20:27 ` [PATCH v8 2/3] pinctrl: pinmux: Add pinmux-select debugfs file Drew Fustini
2021-02-20 20:27 ` [PATCH v8 3/3] docs/pinctrl: document debugfs files Drew Fustini
2021-02-26 22:21 ` Drew Fustini [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210226222122.GA725305@x1 \
--to=drew@beagleboard.org \
--cc=alexandre.belloni@bootlin.com \
--cc=andy.shevchenko@gmail.com \
--cc=corbet@lwn.net \
--cc=dan.carpenter@oracle.com \
--cc=geert@linux-m68k.org \
--cc=jkridner@beagleboard.org \
--cc=joe@perches.com \
--cc=linus.walleij@linaro.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pantelis.antoniou@konsulko.com \
--cc=robertcnelson@beagleboard.org \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).