Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
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
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




      reply index

Thread overview: 12+ 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]

Reply instructions:

You may reply publically 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

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org
	public-inbox-index linux-gpio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git