| Message ID | 20201221165448.27312-3-uli+renesas@fpond.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | pinctrl: renesas: basic R8A779A0 (V3U) support | expand |
Hi Uli, On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote: > This patch adds config macros describing the voltage levels available on > a pin. The current default (3.3V/1.8V) maps to zero to avoid having to > change existing PFC implementations. > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl.c > +++ b/drivers/pinctrl/renesas/pinctrl.c > @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, > } > > case PIN_CONFIG_POWER_SOURCE: { > + int idx = sh_pfc_get_pin_index(pfc, _pin); I guess this cannot fail when we get here? > + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; > + int lower_voltage; unsigned int mV_low? > u32 pocctrl, val; > int bit; > > @@ -648,7 +651,10 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, > val = sh_pfc_read(pfc, pocctrl); > spin_unlock_irqrestore(&pfc->lock, flags); > > - arg = (val & BIT(bit)) ? 3300 : 1800; > + lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ? > + 2500 : 1800; > + > + arg = (val & BIT(bit)) ? 3300 : lower_voltage; > break; > } > > @@ -702,6 +708,9 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin, > > case PIN_CONFIG_POWER_SOURCE: { > unsigned int mV = pinconf_to_config_argument(configs[i]); > + int idx = sh_pfc_get_pin_index(pfc, _pin); > + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; > + int lower_voltage; unsigned int mV_low? > u32 pocctrl, val; > int bit; > Gr{oetje,eeting}s, Geert
Hi Uli, On Mon, Dec 21, 2020 at 5:55 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote: > This patch adds config macros describing the voltage levels available on > a pin. The current default (3.3V/1.8V) maps to zero to avoid having to > change existing PFC implementations. > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> > --- a/drivers/pinctrl/renesas/pinctrl.c > +++ b/drivers/pinctrl/renesas/pinctrl.c > @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, > } > > case PIN_CONFIG_POWER_SOURCE: { > + int idx = sh_pfc_get_pin_index(pfc, _pin); > + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; > + int lower_voltage; > u32 pocctrl, val; > int bit; > > @@ -648,7 +651,10 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, > val = sh_pfc_read(pfc, pocctrl); > spin_unlock_irqrestore(&pfc->lock, flags); > > - arg = (val & BIT(bit)) ? 3300 : 1800; > + lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ? > + 2500 : 1800; > + > + arg = (val & BIT(bit)) ? 3300 : lower_voltage; Alternatively, .pin_to_pocctrl() could return mV_low and mV_high? That would require updating all subdrivers, though. Gr{oetje,eeting}s, Geert
> On 12/22/2020 11:45 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > --- a/drivers/pinctrl/renesas/pinctrl.c > > +++ b/drivers/pinctrl/renesas/pinctrl.c > > @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, > > } > > > > case PIN_CONFIG_POWER_SOURCE: { > > + int idx = sh_pfc_get_pin_index(pfc, _pin); > > I guess this cannot fail when we get here? That would require a bug elsewhere, I think. > > + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; > > + int lower_voltage; > > unsigned int Fair enough... > mV_low? That, though, seems ambiguous to me because it could refer to the logical-zero voltage. (Are internal capital letters even permitted in identifiers?) CU Uli
> On 12/22/2020 12:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Alternatively, .pin_to_pocctrl() could return mV_low and mV_high? > That would require updating all subdrivers, though. Exactly. :) CU Uli
Hi Uli, On Tue, Dec 22, 2020 at 5:51 PM Ulrich Hecht <uli@fpond.eu> wrote: > > On 12/22/2020 11:45 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > --- a/drivers/pinctrl/renesas/pinctrl.c > > > +++ b/drivers/pinctrl/renesas/pinctrl.c > > > + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; > > > + int lower_voltage; > > > > unsigned int > > Fair enough... > > > mV_low? > > That, though, seems ambiguous to me because it could refer to the logical-zero voltage. True. > (Are internal capital letters even permitted in identifiers?) We already have "mV". Gr{oetje,eeting}s, Geert
diff --git a/drivers/pinctrl/renesas/pinctrl.c b/drivers/pinctrl/renesas/pinctrl.c index ac542d278a38..85a182191d7d 100644 --- a/drivers/pinctrl/renesas/pinctrl.c +++ b/drivers/pinctrl/renesas/pinctrl.c @@ -634,6 +634,9 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, } case PIN_CONFIG_POWER_SOURCE: { + int idx = sh_pfc_get_pin_index(pfc, _pin); + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; + int lower_voltage; u32 pocctrl, val; int bit; @@ -648,7 +651,10 @@ static int sh_pfc_pinconf_get(struct pinctrl_dev *pctldev, unsigned _pin, val = sh_pfc_read(pfc, pocctrl); spin_unlock_irqrestore(&pfc->lock, flags); - arg = (val & BIT(bit)) ? 3300 : 1800; + lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ? + 2500 : 1800; + + arg = (val & BIT(bit)) ? 3300 : lower_voltage; break; } @@ -702,6 +708,9 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin, case PIN_CONFIG_POWER_SOURCE: { unsigned int mV = pinconf_to_config_argument(configs[i]); + int idx = sh_pfc_get_pin_index(pfc, _pin); + const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; + int lower_voltage; u32 pocctrl, val; int bit; @@ -712,7 +721,10 @@ static int sh_pfc_pinconf_set(struct pinctrl_dev *pctldev, unsigned _pin, if (WARN(bit < 0, "invalid pin %#x", _pin)) return bit; - if (mV != 1800 && mV != 3300) + lower_voltage = (pin->configs & SH_PFC_PIN_VOLTAGE_25_33) ? + 2500 : 1800; + + if (mV != lower_voltage && mV != 3300) return -EINVAL; spin_lock_irqsave(&pfc->lock, flags); diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h index dc484c13f59c..00bfda90a7b7 100644 --- a/drivers/pinctrl/renesas/sh_pfc.h +++ b/drivers/pinctrl/renesas/sh_pfc.h @@ -31,6 +31,15 @@ enum { SH_PFC_PIN_CFG_PULL_DOWN) #define SH_PFC_PIN_CFG_IO_VOLTAGE (1 << 4) #define SH_PFC_PIN_CFG_DRIVE_STRENGTH (1 << 5) + +#define SH_PFC_PIN_VOLTAGE_18_33 (0 << 6) +#define SH_PFC_PIN_VOLTAGE_25_33 (1 << 6) + +#define SH_PFC_PIN_CFG_IO_VOLTAGE_18_33 (SH_PFC_PIN_CFG_IO_VOLTAGE | \ + SH_PFC_PIN_VOLTAGE_18_33) +#define SH_PFC_PIN_CFG_IO_VOLTAGE_25_33 (SH_PFC_PIN_CFG_IO_VOLTAGE | \ + SH_PFC_PIN_VOLTAGE_25_33) + #define SH_PFC_PIN_CFG_NO_GPIO (1 << 31) struct sh_pfc_pin {
This patch adds config macros describing the voltage levels available on a pin. The current default (3.3V/1.8V) maps to zero to avoid having to change existing PFC implementations. Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> --- drivers/pinctrl/renesas/pinctrl.c | 16 ++++++++++++++-- drivers/pinctrl/renesas/sh_pfc.h | 9 +++++++++ 2 files changed, 23 insertions(+), 2 deletions(-)