Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <eab0487d7b4e68badbbe0505b2a7903b9d8931c4.camel@perches.com>
Subject: Re: [PATCH] docs: checkpatch: Document and segregate more
 checkpatch message types
From:   Joe Perches <joe@perches.com>
To:     Dwaipayan Ray <dwaipayanray1@gmail.com>, corbet@lwn.net,
        lukas.bulwahn@gmail.com
Cc:     linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Fri, 04 Jun 2021 23:26:35 -0700
In-Reply-To: <20210605055932.18393-1-dwaipayanray1@gmail.com>
References: <20210605055932.18393-1-dwaipayanray1@gmail.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.38.1-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Sat, 2021-06-05 at 11:29 +0530, Dwaipayan Ray wrote:
> Add and document more checkpatch message types. About 50% of all
> message types are documented now.
[]
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
[]
> +  **DEVICE_ATTR_FUNCTIONS**
> +    The function names used in DEVICE_ATTR is unusual.
> +    Typically, the store and show functions are named as name_store and
> +    name_show, where name is the device name.

No, it's the variable name of an attribute of a device, not the device name.

    Typically, the store and show functions are used with <attr>_store and
    <attr>_show, where <attr> is a named attribute variable of the device.

> +    Consider the following examples::
> +
> +      static DEVICE_ATTR(type, 0444, type_show, NULL);
> +      static DEVICE_ATTR(power, 0644, power_show, power_store);
> +
> +    The function names should preferably follow the above pattern.
> +
> +    See: https://www.kernel.org/doc/html/latest/driver-api/driver-model/device.html#attributes
> +
> +  **DEVICE_ATTR_RO**
> +    The DEVICE_ATTR_RO(name) helper macro can be used in place of
> +    DEVICE_ATTR(name, 0444, name_show, NULL);
> +
> +    Note that the macro automatically appends _show to the device name
> +    for the show method.

attribute, etc...

> +  **ENOSYS**
> +    ENOSYS means that a nonexistent system call was called.  We have a
> +    bad habit of using it for things like invalid operations on
> +    otherwise valid syscalls.  This should be avoided in new code.

Please do not use terms like "we".  Just use passive voice and not
any first person/collective words.

> +
> +    See: https://lore.kernel.org/lkml/5eb299021dec23c1a48fa7d9f2c8b794e967766d.1408730669.git.luto@amacapital.net/
> +
> +  **ENOTSUPP**
> +    ENOTSUPP is not a standard error code and should be avoided in new patches.
> +    EOPNOTSUPP should be used instead.

Better word choice is like this section above.