Re: [PATCH v3 1/2] pinctrl: qcom: sc8180x: drop the use of tiles

[Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>]

On Thu 04 Mar 00:05 CST 2021, Shawn Guo wrote:

> To support both ACPI and DT, it makes more sense to not use tiles for
> pinctrl-sc8180x driver, as ACPI table describes TLMM block with one
> single memory resource.  Since DTS of SC8180X hasn't landed, there is
> still chance to align DT description with ACPI.
> 

I don't like the idea that we make up addresses to put in the DT to fit
what was put in the DSDT. It is 3 different memory regions, with things
in-between that Linux shouldn't touch.

Isn't it possible to during ACPI probe take reg 0 and register the 3
named regions instead?

Regards,
Bjorn

> Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxx>
> ---
>  drivers/pinctrl/qcom/pinctrl-sc8180x.c | 41 +++++++++-----------------
>  1 file changed, 14 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc8180x.c b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> index b765bf667574..66f76ed22200 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc8180x.c
> @@ -11,17 +11,9 @@
>  
>  #include "pinctrl-msm.h"
>  
> -static const char * const sc8180x_tiles[] = {
> -	"south",
> -	"east",
> -	"west"
> -};
> -
> -enum {
> -	SOUTH,
> -	EAST,
> -	WEST
> -};
> +#define WEST	0x00100000
> +#define EAST	0x00500000
> +#define SOUTH	0x00d00000
>  
>  #define FUNCTION(fname)					\
>  	[msm_mux_##fname] = {				\
> @@ -31,7 +23,7 @@ enum {
>  	}
>  
>  #define REG_SIZE 0x1000
> -#define PINGROUP_OFFSET(id, _tile, offset, f1, f2, f3, f4, f5, f6, f7, f8, f9)	\
> +#define PINGROUP_OFFSET(id, base, offset, f1, f2, f3, f4, f5, f6, f7, f8, f9)	\
>  	{						\
>  		.name = "gpio" #id,			\
>  		.pins = gpio##id##_pins,		\
> @@ -49,12 +41,11 @@ enum {
>  			msm_mux_##f9			\
>  		},					\
>  		.nfuncs = 10,				\
> -		.ctl_reg = REG_SIZE * id + offset,	\
> -		.io_reg = REG_SIZE * id + 0x4 + offset,	\
> -		.intr_cfg_reg = REG_SIZE * id + 0x8 + offset,	\
> -		.intr_status_reg = REG_SIZE * id + 0xc + offset,\
> -		.intr_target_reg = REG_SIZE * id + 0x8 + offset,\
> -		.tile = _tile,				\
> +		.ctl_reg = base + REG_SIZE * id + offset,		\
> +		.io_reg = base + REG_SIZE * id + 0x4 + offset,		\
> +		.intr_cfg_reg = base + REG_SIZE * id + 0x8 + offset,	\
> +		.intr_status_reg = base + REG_SIZE * id + 0xc + offset,	\
> +		.intr_target_reg = base + REG_SIZE * id + 0x8 + offset,	\
>  		.mux_bit = 2,				\
>  		.pull_bit = 0,				\
>  		.drv_bit = 6,				\
> @@ -71,20 +62,19 @@ enum {
>  		.intr_detection_width = 2,		\
>  	}
>  
> -#define PINGROUP(id, _tile, f1, f2, f3, f4, f5, f6, f7, f8, f9)	\
> -	PINGROUP_OFFSET(id, _tile, 0x0, f1, f2, f3, f4, f5, f6, f7, f8, f9)
> +#define PINGROUP(id, base, f1, f2, f3, f4, f5, f6, f7, f8, f9)	\
> +	PINGROUP_OFFSET(id, base, 0x0, f1, f2, f3, f4, f5, f6, f7, f8, f9)
>  
>  #define SDC_QDSD_PINGROUP(pg_name, ctl, pull, drv)	\
>  	{						\
>  		.name = #pg_name,			\
>  		.pins = pg_name##_pins,			\
>  		.npins = (unsigned int)ARRAY_SIZE(pg_name##_pins),	\
> -		.ctl_reg = ctl,				\
> +		.ctl_reg = EAST + ctl,			\
>  		.io_reg = 0,				\
>  		.intr_cfg_reg = 0,			\
>  		.intr_status_reg = 0,			\
>  		.intr_target_reg = 0,			\
> -		.tile = EAST,				\
>  		.mux_bit = -1,				\
>  		.pull_bit = pull,			\
>  		.drv_bit = drv,				\
> @@ -105,12 +95,11 @@ enum {
>  		.name = #pg_name,			\
>  		.pins = pg_name##_pins,			\
>  		.npins = (unsigned int)ARRAY_SIZE(pg_name##_pins),	\
> -		.ctl_reg = 0xb6000,			\
> -		.io_reg = 0xb6004,			\
> +		.ctl_reg = SOUTH + 0xb6000,		\
> +		.io_reg = SOUTH + 0xb6004,		\
>  		.intr_cfg_reg = 0,			\
>  		.intr_status_reg = 0,			\
>  		.intr_target_reg = 0,			\
> -		.tile = SOUTH,				\
>  		.mux_bit = -1,				\
>  		.pull_bit = 3,				\
>  		.drv_bit = 0,				\
> @@ -1575,8 +1564,6 @@ static const struct msm_gpio_wakeirq_map sc8180x_pdc_map[] = {
>  };
>  
>  static struct msm_pinctrl_soc_data sc8180x_pinctrl = {
> -	.tiles = sc8180x_tiles,
> -	.ntiles = ARRAY_SIZE(sc8180x_tiles),
>  	.pins = sc8180x_pins,
>  	.npins = ARRAY_SIZE(sc8180x_pins),
>  	.functions = sc8180x_functions,
> -- 
> 2.17.1
>