Re: [PATCH v6 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support

[Andy Shevchenko]

On Tue, Apr 27, 2021 at 10:38 AM Michal Simek <michal.simek@xxxxxxxxxx> wrote:
> On 4/27/21 9:31 AM, Andy Shevchenko wrote:
> > On Tue, Apr 27, 2021 at 10:23 AM Michal Simek <michal.simek@xxxxxxxxxx> wrote:
> >> On 4/26/21 4:04 PM, Andy Shevchenko wrote:
> >>> On Mon, Apr 26, 2021 at 4:20 PM Sai Krishna Potthuri
> >>> <lakshmis@xxxxxxxxxx> wrote:
> >>>>> From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> >>>>> Sent: Friday, April 23, 2021 9:24 PM
> >>>>> On Thu, Apr 22, 2021 at 11:31 AM Sai Krishna Potthuri
> >>>>> <lakshmi.sai.krishna.potthuri@xxxxxxxxxx> wrote:

...

> >>>>>> +       help
> >>>>>> +         This selects the pinctrl driver for Xilinx ZynqMP platform.
> >>>>>> +         This driver will query the pin information from the firmware
> >>>>>> +         and allow configuring the pins.
> >>>>>> +         Configuration can include the mux function to select on those
> >>>>>> +         pin(s)/group(s), and various pin configuration parameters
> >>>>>> +         such as pull-up, slew rate, etc.
> >>>>>
> >>>>> Missed module name.
> >>>> Is this (module name) a configuration option in Kconfig?
> >>>
> >>> It's a text in a free form that sheds light on how the module will be
> >>> named in case the user will choose "m".
> >>
> >> Is this described somewhere in documentation that module name should be
> >> the part of symbol description? I was looking at pinctrl Kconfig and I
> >> can't see any description like this there that's why I want to double
> >> check.
> >
> > I dunno if it is described, the group of maintainers require that for some time.
> > I personally found this as a good practice.
>
> I don't think it is a big deal to add it but it is a question if this
> information is useful because module names should correspond target in
> Makefile which can be considered as additional information.

For you as a *developer* — yes, for me as a *user* — no. You are
telling me something like "hey, if you want to know more you must dig
into kernel sources". No, this is not how we should treat users,
should we?


> >> Also if this is a rule checkpatch should be extended to checking this.
> >
> > There was a discussion at some point to add a check that help
> > description shouldn't be less than 3 lines. Not sure what the outcome
> > of it.
>
> This check is likely there because I have definitely seen these messages
> coming but never seen any name checking.

Yeah, it was about insisting developers to be more verbose in the help
descriptions, but the name is, as I said, an activity "de facto"
rather than "de jure". Just look around for the latest new driver
contributions (I follow IIO, I2C, SPI, GPIO, pin control) for how they
provide their help descriptions (I admit that not everybody follows
that practice).

-- 
With Best Regards,
Andy Shevchenko