Re: [PATCH v3 5/6] pinctrl: meson: add support of drive-strength-microamp

[Martin Blumenstingl]

On Tue, May 7, 2019 at 1:57 PM Guillaume La Roque <glaroque@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
with the comments below addressed:
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>

> ---
>  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