From 7b34206e3fada9c86c5737955f9af9699f34c2b4 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:46 -0600 Subject: [PATCH 1/7] pinctrl: exynos: Improve coding style Style commit, no functional change. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 3 ++- drivers/pinctrl/exynos/pinctrl-exynos.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index 898185479b..995a3a0ee5 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -57,7 +57,8 @@ static unsigned long pin_to_bank_base(struct udevice *dev, const char *pin_name, /* lookup the pin bank data using the pin bank name */ while (true) { - const struct samsung_pin_ctrl *pin_ctrl = &pin_ctrl_array[pin_ctrl_idx]; + const struct samsung_pin_ctrl *pin_ctrl = + &pin_ctrl_array[pin_ctrl_idx]; nr_banks = pin_ctrl->nr_banks; if (!nr_banks) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.h b/drivers/pinctrl/exynos/pinctrl-exynos.h index cbc5174b48..6b19f196bc 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.h +++ b/drivers/pinctrl/exynos/pinctrl-exynos.h @@ -27,7 +27,7 @@ struct samsung_pin_bank_data { #define EXYNOS_PIN_BANK(pins, reg, id) \ { \ - .offset = reg, \ + .offset = reg, \ .nr_pins = pins, \ .name = id \ } From 58e84bf3d7fac6300790940176c6edf457c8fd88 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:47 -0600 Subject: [PATCH 2/7] pinctrl: exynos: Extract pin parsing code into a separate function Next commits are going to re-design the pin_to_bank_base() function and its usage in a way that the pin parsing code will be called separately. Extract it into a separate function first, as a refactoring commit. No functional change. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 27 ++++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index 995a3a0ee5..2d194ba0a4 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -34,6 +34,22 @@ void exynos_pinctrl_setup_peri(struct exynos_pinctrl_config_data *conf, } } +static void parse_pin(const char *pin_name, u32 *pin, char *bank_name) +{ + u32 idx = 0; + + /* + * The format of the pin name is -. + * Example: gpa0-4 (gpa0 is the bank_name name and 4 is the pin number. + */ + while (pin_name[idx] != '-') { + bank_name[idx] = pin_name[idx]; + idx++; + } + bank_name[idx] = '\0'; + *pin = pin_name[++idx] - '0'; +} + /* given a pin-name, return the address of pin config registers */ static unsigned long pin_to_bank_base(struct udevice *dev, const char *pin_name, u32 *pin) @@ -44,16 +60,7 @@ static unsigned long pin_to_bank_base(struct udevice *dev, const char *pin_name, u32 nr_banks, pin_ctrl_idx = 0, idx = 0, bank_base; char bank[10]; - /* - * The format of the pin name is -. - * Example: gpa0-4 (gpa0 is the bank name and 4 is the pin number. - */ - while (pin_name[idx] != '-') { - bank[idx] = pin_name[idx]; - idx++; - } - bank[idx] = '\0'; - *pin = pin_name[++idx] - '0'; + parse_pin(pin_name, pin, bank); /* lookup the pin bank data using the pin bank name */ while (true) { From 2dfcb250d03a1014ce6a2648985b8216bd1d5207 Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:48 -0600 Subject: [PATCH 3/7] pinctrl: exynos: Rework pin_to_bank_base() to obtain data by name Rework pin_to_bank_base() function to obtain bank data structure by bank name instead of getting bank base address by pin name, and rename it to get_bank() to reflect this change. This in turn leads to the extraction of parse_pin(), so the caller has to use it before calling get_bank(). No functional change. This is a refactoring commit which prepares pinctrl driver code for handling different sizes of register fields, which will be added next. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 27 +++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index 2d194ba0a4..d908927135 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -50,17 +50,14 @@ static void parse_pin(const char *pin_name, u32 *pin, char *bank_name) *pin = pin_name[++idx] - '0'; } -/* given a pin-name, return the address of pin config registers */ -static unsigned long pin_to_bank_base(struct udevice *dev, const char *pin_name, - u32 *pin) +/* given a bank name, find out the pin bank structure */ +static const struct samsung_pin_bank_data *get_bank(struct udevice *dev, + const char *bank_name) { struct exynos_pinctrl_priv *priv = dev_get_priv(dev); const struct samsung_pin_ctrl *pin_ctrl_array = priv->pin_ctrl; const struct samsung_pin_bank_data *bank_data; - u32 nr_banks, pin_ctrl_idx = 0, idx = 0, bank_base; - char bank[10]; - - parse_pin(pin_name, pin, bank); + u32 nr_banks, pin_ctrl_idx = 0, idx = 0; /* lookup the pin bank data using the pin bank name */ while (true) { @@ -75,15 +72,13 @@ static unsigned long pin_to_bank_base(struct udevice *dev, const char *pin_name, for (idx = 0; idx < nr_banks; idx++) { debug("pinctrl[%d] bank_data[%d] name is: %s\n", pin_ctrl_idx, idx, bank_data[idx].name); - if (!strcmp(bank, bank_data[idx].name)) { - bank_base = priv->base + bank_data[idx].offset; - break; - } + if (!strcmp(bank_name, bank_data[idx].name)) + return &bank_data[idx]; } pin_ctrl_idx++; } - return bank_base; + return NULL; } /** @@ -93,6 +88,7 @@ static unsigned long pin_to_bank_base(struct udevice *dev, const char *pin_name, */ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) { + struct exynos_pinctrl_priv *priv = dev_get_priv(dev); const void *fdt = gd->fdt_blob; int node = dev_of_offset(config); unsigned int count, idx, pin_num; @@ -113,10 +109,15 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) pindrv = fdtdec_get_int(fdt, node, "samsung,pin-drv", -1); for (idx = 0; idx < count; idx++) { + const struct samsung_pin_bank_data *bank; + char bank_name[10]; + name = fdt_stringlist_get(fdt, node, "samsung,pins", idx, NULL); if (!name) continue; - reg = pin_to_bank_base(dev, name, &pin_num); + parse_pin(name, &pin_num, bank_name); + bank = get_bank(dev, bank_name); + reg = priv->base + bank->offset; if (pinfunc != -1) { value = readl(reg + PIN_CON); From 2ed4ba83fb8a1e32a91f4d289069a33d390f532b Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:49 -0600 Subject: [PATCH 4/7] pinctrl: exynos: Support different register types in pin banks Get rid of hard-coded register offsets and widths. Instead provide a way for pinctrl drivers to specify different pin bank register offsets and widths. This in turn makes it possible to add support for new SoCs that have registers with offset/width values different than generic ones already available in pinctrl-exynos driver. Offset constants (now unused in pinctrl-exynos.c) are moved to pinctrl-exynos7420 driver, which is the single user of those constants. The design of this patch follows Linux kernel pinctrl-exynos driver design, in terms of added data structures and types. This patch doesn't add support for any new SoCs and shouldn't introduce any functional changes. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 42 ++++++++++++++------- drivers/pinctrl/exynos/pinctrl-exynos.h | 34 +++++++++++++++-- drivers/pinctrl/exynos/pinctrl-exynos7420.c | 2 + 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index d908927135..9a51653be8 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -15,6 +15,12 @@ DECLARE_GLOBAL_DATA_PTR; +/* CON, DAT, PUD, DRV */ +const struct samsung_pin_bank_type bank_type_alive = { + .fld_width = { 4, 1, 2, 2, }, + .reg_offset = { 0x00, 0x04, 0x08, 0x0c, }, +}; + /** * exynos_pinctrl_setup_peri: setup pinctrl for a peripheral. * conf: soc specific pin configuration data array @@ -81,6 +87,22 @@ static const struct samsung_pin_bank_data *get_bank(struct udevice *dev, return NULL; } +static void exynos_pinctrl_set_pincfg(unsigned long reg_base, u32 pin_num, + u32 val, enum pincfg_type pincfg, + const struct samsung_pin_bank_type *type) +{ + u32 width = type->fld_width[pincfg]; + u32 reg_offset = type->reg_offset[pincfg]; + u32 mask = (1 << width) - 1; + u32 shift = pin_num * width; + u32 data; + + data = readl(reg_base + reg_offset); + data &= ~(mask << shift); + data |= val << shift; + writel(data, reg_base + reg_offset); +} + /** * exynos_pinctrl_set_state: configure a pin state. * dev: the pinctrl device to be configured. @@ -93,7 +115,7 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) int node = dev_of_offset(config); unsigned int count, idx, pin_num; unsigned int pinfunc, pinpud, pindrv; - unsigned long reg, value; + unsigned long reg; const char *name; /* @@ -120,24 +142,18 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) reg = priv->base + bank->offset; if (pinfunc != -1) { - value = readl(reg + PIN_CON); - value &= ~(0xf << (pin_num << 2)); - value |= (pinfunc << (pin_num << 2)); - writel(value, reg + PIN_CON); + exynos_pinctrl_set_pincfg(reg, pin_num, pinfunc, + PINCFG_TYPE_FUNC, bank->type); } if (pinpud != -1) { - value = readl(reg + PIN_PUD); - value &= ~(0x3 << (pin_num << 1)); - value |= (pinpud << (pin_num << 1)); - writel(value, reg + PIN_PUD); + exynos_pinctrl_set_pincfg(reg, pin_num, pinpud, + PINCFG_TYPE_PUD, bank->type); } if (pindrv != -1) { - value = readl(reg + PIN_DRV); - value &= ~(0x3 << (pin_num << 1)); - value |= (pindrv << (pin_num << 1)); - writel(value, reg + PIN_DRV); + exynos_pinctrl_set_pincfg(reg, pin_num, pindrv, + PINCFG_TYPE_DRV, bank->type); } } diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.h b/drivers/pinctrl/exynos/pinctrl-exynos.h index 6b19f196bc..743bb55730 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.h +++ b/drivers/pinctrl/exynos/pinctrl-exynos.h @@ -8,25 +8,51 @@ #ifndef __PINCTRL_EXYNOS_H_ #define __PINCTRL_EXYNOS_H_ -#define PIN_CON 0x00 /* Offset of pin function register */ -#define PIN_DAT 0x04 /* Offset of pin data register */ -#define PIN_PUD 0x08 /* Offset of pin pull up/down config register */ -#define PIN_DRV 0x0C /* Offset of pin drive strength register */ +/** + * enum pincfg_type - possible pin configuration types supported. + * @PINCFG_TYPE_FUNC: Function configuration. + * @PINCFG_TYPE_DAT: Pin value configuration. + * @PINCFG_TYPE_PUD: Pull up/down configuration. + * @PINCFG_TYPE_DRV: Drive strength configuration. + */ +enum pincfg_type { + PINCFG_TYPE_FUNC, + PINCFG_TYPE_DAT, + PINCFG_TYPE_PUD, + PINCFG_TYPE_DRV, + + PINCFG_TYPE_NUM +}; + +/** + * struct samsung_pin_bank_type: pin bank type description + * @fld_width: widths of configuration bitfields (0 if unavailable) + * @reg_offset: offsets of configuration registers (don't care of width is 0) + */ +struct samsung_pin_bank_type { + u8 fld_width[PINCFG_TYPE_NUM]; + u8 reg_offset[PINCFG_TYPE_NUM]; +}; /** * struct samsung_pin_bank_data: represent a controller pin-bank data. + * @type: type of the bank (register offsets and bitfield widths) * @offset: starting offset of the pin-bank registers. * @nr_pins: number of pins included in this bank. * @name: name to be prefixed for each pin in this pin bank. */ struct samsung_pin_bank_data { + const struct samsung_pin_bank_type *type; u32 offset; u8 nr_pins; const char *name; }; +extern const struct samsung_pin_bank_type bank_type_alive; + #define EXYNOS_PIN_BANK(pins, reg, id) \ { \ + .type = &bank_type_alive, \ .offset = reg, \ .nr_pins = pins, \ .name = id \ diff --git a/drivers/pinctrl/exynos/pinctrl-exynos7420.c b/drivers/pinctrl/exynos/pinctrl-exynos7420.c index 07870b7f51..77d510d8f6 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos7420.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos7420.c @@ -16,6 +16,8 @@ #include "pinctrl-exynos.h" #define GPD1_OFFSET 0xc0 +#define PIN_CON 0x00 /* Offset of pin function register */ +#define PIN_PUD 0x08 /* Offset of pin pull up/down config register */ static struct exynos_pinctrl_config_data serial2_conf[] = { { From aad0f6abf113a691b0c0472a26fef6b560d30abf Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:50 -0600 Subject: [PATCH 5/7] pinctrl: exynos: Refactor handling the pin related dt properties All pin related dt properties (pin-function, pin-pud and pin-drv) are handled in a very similar way. Get rid of that code duplication by extracting the corresponding data knowledge into an actual data structure (array), and then just iterating over it. No functional change, it's a refactoring commit. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 35 ++++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index 9a51653be8..e79ce5113d 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -21,6 +21,13 @@ const struct samsung_pin_bank_type bank_type_alive = { .reg_offset = { 0x00, 0x04, 0x08, 0x0c, }, }; +static const char * const exynos_pinctrl_props[PINCFG_TYPE_NUM] = { + [PINCFG_TYPE_FUNC] = "samsung,pin-function", + [PINCFG_TYPE_DAT] = "samsung,pin-val", + [PINCFG_TYPE_PUD] = "samsung,pin-pud", + [PINCFG_TYPE_DRV] = "samsung,pin-drv", +}; + /** * exynos_pinctrl_setup_peri: setup pinctrl for a peripheral. * conf: soc specific pin configuration data array @@ -114,7 +121,7 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) const void *fdt = gd->fdt_blob; int node = dev_of_offset(config); unsigned int count, idx, pin_num; - unsigned int pinfunc, pinpud, pindrv; + unsigned int pinvals[PINCFG_TYPE_NUM]; unsigned long reg; const char *name; @@ -126,13 +133,16 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) if (count <= 0) return -EINVAL; - pinfunc = fdtdec_get_int(fdt, node, "samsung,pin-function", -1); - pinpud = fdtdec_get_int(fdt, node, "samsung,pin-pud", -1); - pindrv = fdtdec_get_int(fdt, node, "samsung,pin-drv", -1); + for (idx = 0; idx < PINCFG_TYPE_NUM; ++idx) { + pinvals[idx] = fdtdec_get_int(fdt, node, + exynos_pinctrl_props[idx], -1); + } + pinvals[PINCFG_TYPE_DAT] = -1; /* ignore GPIO data register */ for (idx = 0; idx < count; idx++) { const struct samsung_pin_bank_data *bank; char bank_name[10]; + int pincfg; name = fdt_stringlist_get(fdt, node, "samsung,pins", idx, NULL); if (!name) @@ -141,19 +151,12 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) bank = get_bank(dev, bank_name); reg = priv->base + bank->offset; - if (pinfunc != -1) { - exynos_pinctrl_set_pincfg(reg, pin_num, pinfunc, - PINCFG_TYPE_FUNC, bank->type); - } + for (pincfg = 0; pincfg < PINCFG_TYPE_NUM; ++pincfg) { + unsigned int val = pinvals[pincfg]; - if (pinpud != -1) { - exynos_pinctrl_set_pincfg(reg, pin_num, pinpud, - PINCFG_TYPE_PUD, bank->type); - } - - if (pindrv != -1) { - exynos_pinctrl_set_pincfg(reg, pin_num, pindrv, - PINCFG_TYPE_DRV, bank->type); + if (val != -1) + exynos_pinctrl_set_pincfg(reg, pin_num, val, + pincfg, bank->type); } } From da06fefcef7bf1d985338f08810e90c7f504bfad Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:51 -0600 Subject: [PATCH 6/7] pinctrl: exynos: Reduce variables scope Pull some variables declared in exynos_pinctrl_set_state() into its loop, to reduce their scope. Style commit, no functional change. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index e79ce5113d..b6af3befbf 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -120,10 +120,8 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) struct exynos_pinctrl_priv *priv = dev_get_priv(dev); const void *fdt = gd->fdt_blob; int node = dev_of_offset(config); - unsigned int count, idx, pin_num; + unsigned int count, idx; unsigned int pinvals[PINCFG_TYPE_NUM]; - unsigned long reg; - const char *name; /* * refer to the following document for the pinctrl bindings @@ -141,7 +139,10 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) for (idx = 0; idx < count; idx++) { const struct samsung_pin_bank_data *bank; + unsigned int pin_num; char bank_name[10]; + unsigned long reg; + const char *name; int pincfg; name = fdt_stringlist_get(fdt, node, "samsung,pins", idx, NULL); From 5bf111b77c8d819fd5e30734b72c5f70518d690d Mon Sep 17 00:00:00 2001 From: Sam Protsenko Date: Thu, 30 Nov 2023 14:13:52 -0600 Subject: [PATCH 7/7] pinctrl: exynos: Convert to use livetree API for fdt access Use counterpart dev_read_* functions instead of fdt* ones. It fixes checkpatch warnings like this: WARNING: Use the livetree API (dev_read_...) #54: FILE: drivers/pinctrl/exynos/pinctrl-exynos.c:137: pinvals[idx] = fdtdec_get_int(fdt, node, and also makes it possible to avoid using the global data pointer in the driver. No functional change. Signed-off-by: Sam Protsenko Signed-off-by: Minkyu Kang --- drivers/pinctrl/exynos/pinctrl-exynos.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/pinctrl/exynos/pinctrl-exynos.c b/drivers/pinctrl/exynos/pinctrl-exynos.c index b6af3befbf..8a045cdf7a 100644 --- a/drivers/pinctrl/exynos/pinctrl-exynos.c +++ b/drivers/pinctrl/exynos/pinctrl-exynos.c @@ -9,12 +9,9 @@ #include #include #include -#include #include #include "pinctrl-exynos.h" -DECLARE_GLOBAL_DATA_PTR; - /* CON, DAT, PUD, DRV */ const struct samsung_pin_bank_type bank_type_alive = { .fld_width = { 4, 1, 2, 2, }, @@ -118,8 +115,6 @@ static void exynos_pinctrl_set_pincfg(unsigned long reg_base, u32 pin_num, int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) { struct exynos_pinctrl_priv *priv = dev_get_priv(dev); - const void *fdt = gd->fdt_blob; - int node = dev_of_offset(config); unsigned int count, idx; unsigned int pinvals[PINCFG_TYPE_NUM]; @@ -127,13 +122,13 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) * refer to the following document for the pinctrl bindings * linux/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt */ - count = fdt_stringlist_count(fdt, node, "samsung,pins"); + count = dev_read_string_count(config, "samsung,pins"); if (count <= 0) return -EINVAL; for (idx = 0; idx < PINCFG_TYPE_NUM; ++idx) { - pinvals[idx] = fdtdec_get_int(fdt, node, - exynos_pinctrl_props[idx], -1); + pinvals[idx] = dev_read_u32_default(config, + exynos_pinctrl_props[idx], -1); } pinvals[PINCFG_TYPE_DAT] = -1; /* ignore GPIO data register */ @@ -142,12 +137,13 @@ int exynos_pinctrl_set_state(struct udevice *dev, struct udevice *config) unsigned int pin_num; char bank_name[10]; unsigned long reg; - const char *name; - int pincfg; + const char *name = NULL; + int pincfg, err; - name = fdt_stringlist_get(fdt, node, "samsung,pins", idx, NULL); - if (!name) + err = dev_read_string_index(config, "samsung,pins", idx, &name); + if (err || !name) continue; + parse_pin(name, &pin_num, bank_name); bank = get_bank(dev, bank_name); reg = priv->base + bank->offset;