From: Heiko Stuebner <heiko@sntech.de>
To: Jianqun Xu <jay.xu@rock-chips.com>
Cc: linus.walleij@linaro.org, david.wu@rock-chips.com,
kever.yang@rock-chips.com, linux-kernel@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-rockchip@lists.infradead.org
Subject: Re: [PATCH v4 2/2] pinctrl: rockchip: split rockchip pinctrl driver by SoC type
Date: Sat, 15 Feb 2020 12:11:19 +0100 [thread overview]
Message-ID: <6201612.znBgJCgWHB@phil> (raw)
In-Reply-To: <20200117081358.5772-3-jay.xu@rock-chips.com>
Hi Jay,
Am Freitag, 17. Januar 2020, 09:13:58 CET schrieb Jianqun Xu:
> The pinctrl-rockchip driver grows larger by adding support for
> each new SoC, that make the kernel Image size too large since
> it only under one config named PINCTRL_ROCKCHIP.
>
> This patch split driver in the form of core driver + soc driver,
> - pinctrl-rockchip.c defined an platform probe register function
> - pinctrl-rkxxxx.c init module by matching compatible name
>
> For rockchip_defconfig, it needs to select all PINCTRL_RKxxxx to
> keep same with old driver.
>
> For some special defconfig, it can only select one PINCTRL_RKxxxx.
>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com>
> ---
> changes since v3:
> - add base patch with directory change only, suggested by Robin
> - rebase patch
>
> changes since v2:
> - remove rockchip_pinctrl_remove
> - rename rockchip_pinctrl_* to rockchip_pctrl_*
> - redule arguments for get_soc_data
> - add module author for each new driver files
> - add copyright for new driver files
>
> changes since v1:
> - add rockchip_pinctrl_remove
> - remove unused head files in pinctrl-rockchip.h
>
> drivers/pinctrl/rockchip/Kconfig | 114 +
> drivers/pinctrl/rockchip/Makefile | 14 +
> drivers/pinctrl/rockchip/pinctrl-px30.c | 224 ++
> drivers/pinctrl/rockchip/pinctrl-rk2928.c | 70 +
> drivers/pinctrl/rockchip/pinctrl-rk3036.c | 69 +
> drivers/pinctrl/rockchip/pinctrl-rk3066a.c | 72 +
> drivers/pinctrl/rockchip/pinctrl-rk3066b.c | 51 +
> drivers/pinctrl/rockchip/pinctrl-rk3128.c | 161 ++
> drivers/pinctrl/rockchip/pinctrl-rk3188.c | 147 ++
> drivers/pinctrl/rockchip/pinctrl-rk3228.c | 225 ++
> drivers/pinctrl/rockchip/pinctrl-rk3288.c | 210 ++
> drivers/pinctrl/rockchip/pinctrl-rk3308.c | 420 +++
> drivers/pinctrl/rockchip/pinctrl-rk3328.c | 272 ++
> drivers/pinctrl/rockchip/pinctrl-rk3368.c | 125 +
> drivers/pinctrl/rockchip/pinctrl-rk3399.c | 195 ++
> drivers/pinctrl/rockchip/pinctrl-rockchip.c | 2547 ++-----------------
> drivers/pinctrl/rockchip/pinctrl-rockchip.h | 388 +++
> drivers/pinctrl/rockchip/pinctrl-rv1108.c | 214 ++
> 18 files changed, 3149 insertions(+), 2369 deletions(-)
What Robin suggested, was doing this incrementally. So keep your patch1
but then do
- patch2: split out px30-pinctrl
- patch3: split out rk3288 pinctrl
- etc
Because even my mail client chokes on this massive 6000 line patch, so a
real review is actually very difficult.
> diff --git a/drivers/pinctrl/rockchip/Kconfig b/drivers/pinctrl/rockchip/Kconfig
> index 7a0077ca32dd..4873a05108f8 100644
> --- a/drivers/pinctrl/rockchip/Kconfig
> +++ b/drivers/pinctrl/rockchip/Kconfig
> @@ -5,8 +5,122 @@ if (ARCH_ROCKCHIP || COMPILE_TEST)
> config PINCTRL_ROCKCHIP
> bool
> select PINMUX
> + select PINCONF
> select GENERIC_PINCONF
> + select GPIOLIB_IRQCHIP
> select GENERIC_IRQ_CHIP
> select MFD_SYSCON
>
> +config PINCTRL_PX30
> + tristate "PX30 pin controller driver"
> + depends on GPIOLIB && OF
> + select PINCTRL_ROCKCHIP
you might want to add a
default y if ARM64
here
(similar default y if ARM for arm32 pinctrl drivers)
Because otherwise you're breaking peoples kernel configs and also
the default is to build a somewhat unified kernel in the default defconfigs,
so we want all matching pinctrl drivers by default and people then can
disable drivers if they really want to build a slimmed down kernel.
With the "if ARM" / "if ARM64" parts you even save some space by
default as well, as you build only the relevant drivers.
Heiko
next prev parent reply other threads:[~2020-02-15 11:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 9:40 [PATCH] pinctrl/rockchip: splite soc data to separated driver Jianqun Xu
2020-01-10 16:19 ` kbuild test robot
2020-01-13 1:16 ` [PATCH v2] " Jianqun Xu
2020-01-15 12:51 ` Linus Walleij
2020-01-16 3:44 ` Kever Yang
2020-01-16 7:14 ` [PATCH v3] pinctrl: rockchip: split rockchip pinctrl driver by SoC type Jianqun Xu
2020-01-16 7:47 ` [PATCH v3 RESEND] " Jianqun Xu
2020-01-16 10:34 ` Robin Murphy
2020-01-17 8:13 ` [PATCH 0/2] pinctrl: rockchip: codingstyle for pinctrl-rockchip Jianqun Xu
2020-01-17 8:13 ` [PATCH 1/2] pinctrl: rockchip: new rockchip dir " Jianqun Xu
2020-01-17 8:13 ` [PATCH v4 2/2] pinctrl: rockchip: split rockchip pinctrl driver by SoC type Jianqun Xu
2020-02-15 11:11 ` Heiko Stuebner [this message]
2020-09-12 11:27 ` [PATCH 0/2] pinctrl: rockchip: codingstyle for pinctrl-rockchip Linus Walleij
2020-09-12 12:02 ` Heiko Stübner
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=6201612.znBgJCgWHB@phil \
--to=heiko@sntech.de \
--cc=david.wu@rock-chips.com \
--cc=jay.xu@rock-chips.com \
--cc=kever.yang@rock-chips.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
/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 NNTP newsgroup(s).