| Message ID | 20190314163725.7918-1-jbrunet@baylibre.com |
|---|---|
| Headers | show |
| Series | pinctrl: meson: add g12a drive strength support | expand |
On 14/03/2019 17:37, Jerome Brunet wrote: > From: Guillaume La Roque <glaroque@baylibre.com> > > drive-strength is a new feature needed for G12A SoC. > the default DS setting after boot is usually 0.5mA 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> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/pinctrl/meson/pinctrl-meson-g12a.c | 36 ++--- > drivers/pinctrl/meson/pinctrl-meson.c | 166 ++++++++++++++++----- > drivers/pinctrl/meson/pinctrl-meson.h | 20 ++- > 3 files changed, 162 insertions(+), 60 deletions(-) > > diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c > index d494492e98e9..3475cd7bd2af 100644 > --- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c > +++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c [...] > 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: > @@ -291,6 +373,10 @@ static int meson_pinconf_get(struct pinctrl_dev *pcdev, unsigned int pin, > else > return -EINVAL; > break; > + case PIN_CONFIG_DRIVE_STRENGTH: > + ret = meson_pinconf_get_drive_strength(pc, pin, &arg); > + if (ret) > + return ret; Missing break here ! Neil > default: > return -ENOTSUPP; > } > diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h > index 5eaab925f427..1a88103dcb9b 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,8 +157,12 @@ 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) > >
On Thu, 2019-03-14 at 17:37 +0100, Jerome Brunet wrote: > The purpose of this patchset is to add drive-strength support in meson pinconf > driver. This is a new feature that was added on the g12a. It is critical for us > to support this since many functions are failing with default pad drive-strength. > > Now the slightly annoying part :( > The value achievable by the SoC are 0.5mA, 2.5mA, 3mA and 4mA and the DT property > 'drive-strength' is expressed in mA. > > 1) Rounding down the value, we could be requesting a 0mA drive strength. > That would look weird. > 2) Rounding up, we can't distinguish between 2.5mA and 3mA > > To solve this issue in this in this v1, we chose to document that, on Amlogic, > drive-strength is expressed in uA instead of mA. > It works well and there is no impact on the other platforms but I'm not sure this > is really OK with the DT rules ? > > Linus, if this is not OK with you, here are 2 other options we are > considering. We would be very interested to get your opinion on the matter: > > 1) instead the generic 'drive-strength' property, we could add an amlogic > specific property, 'amlogic,drive-strength'. It would be expressed in uA > and parsed in amlogic specific code. > I think this option is kind of overkill. Expressing drive strength in uA is > not really amlogic specific so it does not make much sense, but it would > work ... > > 2) Add another generic property "drive-strength-uA". The change to do so > would be minimal and could be benefit to other platforms later on. Hi Linus, I know it has only been 10 days and you must be busy but I was wondering if we could get your view on the issue above ? Since the vast majority of SoC functions need a drive strength setting, DT patches are somehow blocked until we decide which binding to use for it. Sorry for this early ping. Jerome > > Cheers > Jerome > > Guillaume La Roque (2): > dt-bindings: pinctrl: meson: Add drive-strength property > pinctrl: meson: add support of drive-strength > > .../bindings/pinctrl/meson,pinctrl.txt | 3 + > drivers/pinctrl/meson/pinctrl-meson-g12a.c | 36 ++-- > drivers/pinctrl/meson/pinctrl-meson.c | 166 +++++++++++++----- > drivers/pinctrl/meson/pinctrl-meson.h | 20 ++- > 4 files changed, 165 insertions(+), 60 deletions(-) >
On Thu, Mar 14, 2019 at 11:37 PM Jerome Brunet <jbrunet@baylibre.com> wrote: > Now the slightly annoying part :( > The value achievable by the SoC are 0.5mA, 2.5mA, 3mA and 4mA and the DT property > 'drive-strength' is expressed in mA. > > 1) Rounding down the value, we could be requesting a 0mA drive strength. > That would look weird. > 2) Rounding up, we can't distinguish between 2.5mA and 3mA > > To solve this issue in this in this v1, we chose to document that, on Amlogic, > drive-strength is expressed in uA instead of mA. > It works well and there is no impact on the other platforms but I'm not sure this > is really OK with the DT rules ? I want the DT people to say what they think about this. > Linus, if this is not OK with you, here are 2 other options we are > considering. We would be very interested to get your opinion on the matter: > > 1) instead the generic 'drive-strength' property, we could add an amlogic > specific property, 'amlogic,drive-strength'. It would be expressed in uA > and parsed in amlogic specific code. > I think this option is kind of overkill. Expressing drive strength in uA is > not really amlogic specific so it does not make much sense, but it would > work ... > > 2) Add another generic property "drive-strength-uA". The change to do so > would be minimal and could be benefit to other platforms later on. I would go for 2). But we really need input from bindings people on this. Yours, Linus Walleij