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
>
