| Message ID | 20190507115726.23714-6-glaroque@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add drive-strength in Meson pinctrl driver | expand |
On Tue, May 7, 2019 at 1:57 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > > drive-strength-microamp is a new feature needed for G12A SoC. > the default DS setting after boot is usually 500uA and it is not enough for > many functions. We need to be able to set the drive strength to reliably > enable things like MMC, I2C, etc ... > > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> with the comments below addressed: Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/pinctrl/meson/pinctrl-meson.c | 102 ++++++++++++++++++++++++++ > drivers/pinctrl/meson/pinctrl-meson.h | 18 ++++- > 2 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c > index a216a7537564..3da867c13f47 100644 > --- a/drivers/pinctrl/meson/pinctrl-meson.c > +++ b/drivers/pinctrl/meson/pinctrl-meson.c > @@ -219,12 +219,56 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, > return 0; > } > > +static int meson_pinconf_set_drive_strength(struct meson_pinctrl *pc, > + unsigned int pin, > + u16 drive_strength_ua) > +{ > + struct meson_bank *bank; > + unsigned int reg, bit; > + unsigned int ds_val; you can move ds_val to the line above [...] > + if (!pc->reg_ds) { > + dev_err(pc->dev, "drive-strength not supported\n"); I'm getting this on one of my Meson8m2 boards: $ # cat /sys/kernel/debug/pinctrl/c1109880.pinctrl-pinctrl-meson/pinconf-pins ... [ 874.748531] meson8-pinctrl c1109880.pinctrl: drive-strength not supported [ 874.755278] meson8-pinctrl c1109880.pinctrl: drive-strength not supported [ 874.762086] meson8-pinctrl c1109880.pinctrl: drive-strength not supported Pin config settings per pin Format: pin (name): configs pin 0 (GPIOX_0): input bias disabled pin 1 (GPIOX_1): input bias disabled pin 2 (GPIOX_2): input bias disabled ... I believe we are not supposed to complain when getting the drive-strength when reg_ds is absent. all pre-G12A SoCs don't have reg_ds, so we don't need to error-out in that case (because that's perfectly valid) [...] > +static int meson_pinconf_get_drive_strength(struct meson_pinctrl *pc, > + unsigned int pin, > + u16 *drive_strength_ua) > +{ > + struct meson_bank *bank; > + unsigned int reg, bit; > + unsigned int val; > + int ret; > + > + if (!pc->reg_ds) { > + dev_err(pc->dev, "drive-strength not supported\n"); based on your previous explanation (that you want to inform the .dts author that he's doing something wrong) I'm happy with this error if Linus W. doesn't veto this. Regards Martin
hi martin, thanks for review, i will do a new series to remove err log on get_drive_strength and integrate your comment On 5/7/19 8:18 PM, Martin Blumenstingl wrote: > On Tue, May 7, 2019 at 1:57 PM Guillaume La Roque <glaroque@baylibre.com> wrote: >> drive-strength-microamp is a new feature needed for G12A SoC. >> the default DS setting after boot is usually 500uA and it is not enough for >> many functions. We need to be able to set the drive strength to reliably >> enable things like MMC, I2C, etc ... >> >> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> > with the comments below addressed: > Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > >> --- >> drivers/pinctrl/meson/pinctrl-meson.c | 102 ++++++++++++++++++++++++++ >> drivers/pinctrl/meson/pinctrl-meson.h | 18 ++++- >> 2 files changed, 119 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c >> index a216a7537564..3da867c13f47 100644 >> --- a/drivers/pinctrl/meson/pinctrl-meson.c >> +++ b/drivers/pinctrl/meson/pinctrl-meson.c >> @@ -219,12 +219,56 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, >> return 0; >> } >> >> +static int meson_pinconf_set_drive_strength(struct meson_pinctrl *pc, >> + unsigned int pin, >> + u16 drive_strength_ua) >> +{ >> + struct meson_bank *bank; >> + unsigned int reg, bit; >> + unsigned int ds_val; > you can move ds_val to the line above > > [...] >> + if (!pc->reg_ds) { >> + dev_err(pc->dev, "drive-strength not supported\n"); > I'm getting this on one of my Meson8m2 boards: > $ # cat /sys/kernel/debug/pinctrl/c1109880.pinctrl-pinctrl-meson/pinconf-pins > ... > [ 874.748531] meson8-pinctrl c1109880.pinctrl: drive-strength not supported > [ 874.755278] meson8-pinctrl c1109880.pinctrl: drive-strength not supported > [ 874.762086] meson8-pinctrl c1109880.pinctrl: drive-strength not supported > Pin config settings per pin > Format: pin (name): configs > pin 0 (GPIOX_0): input bias disabled > pin 1 (GPIOX_1): input bias disabled > pin 2 (GPIOX_2): input bias disabled > ... > > I believe we are not supposed to complain when getting the > drive-strength when reg_ds is absent. > all pre-G12A SoCs don't have reg_ds, so we don't need to error-out in > that case (because that's perfectly valid) > > [...] >> +static int meson_pinconf_get_drive_strength(struct meson_pinctrl *pc, >> + unsigned int pin, >> + u16 *drive_strength_ua) >> +{ >> + struct meson_bank *bank; >> + unsigned int reg, bit; >> + unsigned int val; >> + int ret; >> + >> + if (!pc->reg_ds) { >> + dev_err(pc->dev, "drive-strength not supported\n"); > based on your previous explanation (that you want to inform the .dts > author that he's doing something wrong) I'm happy with this error if > Linus W. doesn't veto this. > > > Regards > Martin Guillaume
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c index a216a7537564..3da867c13f47 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.c +++ b/drivers/pinctrl/meson/pinctrl-meson.c @@ -219,12 +219,56 @@ static int meson_pinconf_enable_bias(struct meson_pinctrl *pc, unsigned int pin, return 0; } +static int meson_pinconf_set_drive_strength(struct meson_pinctrl *pc, + unsigned int pin, + u16 drive_strength_ua) +{ + struct meson_bank *bank; + unsigned int reg, bit; + unsigned int ds_val; + int ret; + + if (!pc->reg_ds) { + dev_err(pc->dev, "drive-strength not supported\n"); + return -ENOTSUPP; + } + + ret = meson_get_bank(pc, pin, &bank); + if (ret) + return ret; + + meson_calc_reg_and_bit(bank, pin, REG_DS, ®, &bit); + bit = bit << 1; + + if (drive_strength_ua <= 500) { + ds_val = MESON_PINCONF_DRV_500UA; + } else if (drive_strength_ua <= 2500) { + ds_val = MESON_PINCONF_DRV_2500UA; + } else if (drive_strength_ua <= 3000) { + ds_val = MESON_PINCONF_DRV_3000UA; + } else if (drive_strength_ua <= 4000) { + ds_val = MESON_PINCONF_DRV_4000UA; + } else { + dev_warn_once(pc->dev, + "pin %u: invalid drive-strength : %d , default to 4mA\n", + pin, drive_strength_ua); + ds_val = MESON_PINCONF_DRV_4000UA; + } + + ret = regmap_update_bits(pc->reg_ds, reg, 0x3 << bit, ds_val << bit); + if (ret) + return ret; + + return 0; +} + static int meson_pinconf_set(struct pinctrl_dev *pcdev, unsigned int pin, unsigned long *configs, unsigned num_configs) { struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); struct meson_bank *bank; enum pin_config_param param; + unsigned int drive_strength_ua; int i, ret; ret = meson_get_bank(pc, pin, &bank); @@ -250,6 +294,14 @@ static int meson_pinconf_set(struct pinctrl_dev *pcdev, unsigned int pin, if (ret) return ret; break; + case PIN_CONFIG_DRIVE_STRENGTH_UA: + drive_strength_ua = + pinconf_to_config_argument(configs[i]); + ret = meson_pinconf_set_drive_strength + (pc, pin, drive_strength_ua); + if (ret) + return ret; + break; default: return -ENOTSUPP; } @@ -292,12 +344,57 @@ static int meson_pinconf_get_pull(struct meson_pinctrl *pc, unsigned int pin) return conf; } +static int meson_pinconf_get_drive_strength(struct meson_pinctrl *pc, + unsigned int pin, + u16 *drive_strength_ua) +{ + struct meson_bank *bank; + unsigned int reg, bit; + unsigned int val; + int ret; + + if (!pc->reg_ds) { + dev_err(pc->dev, "drive-strength not supported\n"); + return -ENOTSUPP; + } + + ret = meson_get_bank(pc, pin, &bank); + if (ret) + return ret; + + meson_calc_reg_and_bit(bank, pin, REG_DS, ®, &bit); + + ret = regmap_read(pc->reg_ds, reg, &val); + if (ret) + return ret; + + switch ((val >> bit) & 0x3) { + case MESON_PINCONF_DRV_500UA: + *drive_strength_ua = 500; + break; + case MESON_PINCONF_DRV_2500UA: + *drive_strength_ua = 2500; + break; + case MESON_PINCONF_DRV_3000UA: + *drive_strength_ua = 3000; + break; + case MESON_PINCONF_DRV_4000UA: + *drive_strength_ua = 4000; + break; + default: + return -EINVAL; + } + + return 0; +} + static int meson_pinconf_get(struct pinctrl_dev *pcdev, unsigned int pin, unsigned long *config) { struct meson_pinctrl *pc = pinctrl_dev_get_drvdata(pcdev); enum pin_config_param param = pinconf_to_config_param(*config); u16 arg; + int ret; switch (param) { case PIN_CONFIG_BIAS_DISABLE: @@ -308,6 +405,11 @@ static int meson_pinconf_get(struct pinctrl_dev *pcdev, unsigned int pin, else return -EINVAL; break; + case PIN_CONFIG_DRIVE_STRENGTH_UA: + ret = meson_pinconf_get_drive_strength(pc, pin, &arg); + if (ret) + return ret; + break; default: return -ENOTSUPP; } diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h index 5eaab925f427..cd955fb7c2ce 100644 --- a/drivers/pinctrl/meson/pinctrl-meson.h +++ b/drivers/pinctrl/meson/pinctrl-meson.h @@ -71,9 +71,20 @@ enum meson_reg_type { REG_DIR, REG_OUT, REG_IN, + REG_DS, NUM_REG, }; +/** + * enum meson_pinconf_drv - value of drive-strength supported + */ +enum meson_pinconf_drv { + MESON_PINCONF_DRV_500UA, + MESON_PINCONF_DRV_2500UA, + MESON_PINCONF_DRV_3000UA, + MESON_PINCONF_DRV_4000UA, +}; + /** * struct meson bank * @@ -132,7 +143,8 @@ struct meson_pinctrl { .num_groups = ARRAY_SIZE(fn ## _groups), \ } -#define BANK(n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ +#define BANK_DS(n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib, \ + dsr, dsb) \ { \ .name = n, \ .first = f, \ @@ -145,9 +157,13 @@ struct meson_pinctrl { [REG_DIR] = { dr, db }, \ [REG_OUT] = { or, ob }, \ [REG_IN] = { ir, ib }, \ + [REG_DS] = { dsr, dsb }, \ }, \ } +#define BANK(n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ + BANK_DS(n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib, 0, 0) + #define MESON_PIN(x) PINCTRL_PIN(x, #x) /* Common pmx functions */
drive-strength-microamp is a new feature needed for G12A SoC. the default DS setting after boot is usually 500uA and it is not enough for many functions. We need to be able to set the drive strength to reliably enable things like MMC, I2C, etc ... Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> --- drivers/pinctrl/meson/pinctrl-meson.c | 102 ++++++++++++++++++++++++++ drivers/pinctrl/meson/pinctrl-meson.h | 18 ++++- 2 files changed, 119 insertions(+), 1 deletion(-)