[PATCH v5 13/33] pinctrl: Add support for Kendryte K210 FPIOA

Sean Anderson
2020-03-04T15:00:20Z

On 3/4/20 1:47 AM, Rick Chen wrote:
> Hi Sean
>
>> The Fully-Programmable Input/Output Array (FPIOA) device controls pin
>> multiplexing on the K210. The FPIOA can remap any supported function to any
>> multifunctional IO pin. It can also perform basic GPIO functions, such as
>> reading the current value of a pin.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>> Changes in v5:
>> - New
>>
>> MAINTAINERS | 2 +
>> .../pinctrl/kendryte,k210-fpioa.txt | 116 +++
>> drivers/pinctrl/Kconfig | 1 +
>> drivers/pinctrl/Makefile | 1 +
>> drivers/pinctrl/kendryte/Kconfig | 7 +
>> drivers/pinctrl/kendryte/Makefile | 1 +
>> drivers/pinctrl/kendryte/pinctrl.c | 663 ++++++++++++++++++
>> drivers/pinctrl/kendryte/pinctrl.h | 325 +++++++++
>> include/dt-bindings/pinctrl/k210-pinctrl.h | 12 +
>> 9 files changed, 1128 insertions(+)
>> create mode 100644 doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
>> create mode 100644 drivers/pinctrl/kendryte/Kconfig
>> create mode 100644 drivers/pinctrl/kendryte/Makefile
>> create mode 100644 drivers/pinctrl/kendryte/pinctrl.c
>> create mode 100644 drivers/pinctrl/kendryte/pinctrl.h
>> create mode 100644 include/dt-bindings/pinctrl/k210-pinctrl.h
>>
>
> Please checkpatch and fix
>
> total: 3 errors, 13 warnings, 5 checks, 1147 lines checked
>
> Thanks
> Rick

Here is the output of checkpatch.

drivers/pinctrl/kendryte/pinctrl.c:25: error: space prohibited before open square bracket '['

This is from using macros in the style

#define FOO(x) [FOO_##x] = {\
...
}

I think this is more elegant than putting the [] on a separate line,
but it can of course be changed.

drivers/pinctrl/kendryte/pinctrl.c:76: check: Please use a blank line after function/struct/union/enum declarations

This is from using "local" macros, e.g.

#define FOO(x) FOO_##x
enum foo {
FOO(bar),
FOO(baz)
};
#undef FOO

I think keeping the undefinition of FOO close to where it is last used
aids readability. This can of course be changed (perhaps by moving the
(un)definition inside the enum).

drivers/pinctrl/kendryte/pinctrl.c:83: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:106: warning: Prefer 'unsigned int' to bare use of 'unsigned'

All these warnings are due to following the function definitions as
defined in pinctrl_ops.

drivers/pinctrl/kendryte/pinctrl.c:122: error: space prohibited before open square bracket '['
drivers/pinctrl/kendryte/pinctrl.c:133: check: Please use a blank line after function/struct/union/enum declarations
drivers/pinctrl/kendryte/pinctrl.c:141: error: space prohibited before open square bracket '['
drivers/pinctrl/kendryte/pinctrl.c:145: check: Please use a blank line after function/struct/union/enum declarations
drivers/pinctrl/kendryte/pinctrl.c:403: check: Please use a blank line after function/struct/union/enum declarations
drivers/pinctrl/kendryte/pinctrl.c:411: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:416: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:417: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:442: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:453: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:454: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:454: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:529: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:529: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:530: warning: Prefer 'unsigned int' to bare use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:612: warning: ENOSYS means 'invalid syscall nr' and nothing else
drivers/pinctrl/kendryte/pinctrl.h:281: check: Prefer using the BIT macro

This is from the line

#define K210_PC_DRIVE_1 (1 << K210_PC_DRIVE_SHIFT)

Where there are several succeeding and preceding lines which have
different values instead of 1.

--Sean