| Message ID | 20240423175900.702640-11-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add PFC support for Renesas RZ/V2H(P) SoC | expand |
Hi Prabhakar, On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > properties of the pin. > > Two new function pointers get_bias_param() and get_bias_val() are > introduced as the values in PUPD register differ when compared to > RZ/G2L family and RZ/V2H(P) SoC, > > Value | RZ/G2L | RZ/V2H > --------------------------------- > 00b: | Bias Disabled | Pull up/down disabled > 01b: | Pull-up | Pull up/down disabled > 10b: | Pull-down | Pull-down > 11b: | Prohibited | Pull-up > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > RFC->v2 > - New patch Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK); > break; > > + case PIN_CONFIG_BIAS_DISABLE: > + case PIN_CONFIG_BIAS_PULL_UP: > + case PIN_CONFIG_BIAS_PULL_DOWN: { > + if (!(cfg & PIN_CFG_PUPD)) > + return -EINVAL; > + > + ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl, > + PUPD(off), > + bit, PUPD_MASK)); This is rather long, so please split it in two parts, like is done in other cases in this function: arg = rzg2l_read_pin_config(pctrl, PUPD(off), bit, PUPD_MASK); ret = pctrl->data->get_bias_param(arg); > + if (ret < 0) > + return ret; > + > + if (ret != param) > + return -EINVAL; > + /* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */ > + arg = 1; > + break; > + } > + The rest LGTM. Gr{oetje,eeting}s, Geert
On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > properties of the pin. > > Two new function pointers get_bias_param() and get_bias_val() are > introduced as the values in PUPD register differ when compared to > RZ/G2L family and RZ/V2H(P) SoC, > > Value | RZ/G2L | RZ/V2H > --------------------------------- > 00b: | Bias Disabled | Pull up/down disabled > 01b: | Pull-up | Pull up/down disabled > 10b: | Pull-down | Pull-down > 11b: | Prohibited | Pull-up > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > RFC->v2 > - New patch > --- > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++ > 1 file changed, 73 insertions(+) > > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > index 102fa75c71d3..c144bf43522b 100644 > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -122,6 +122,7 @@ > #define IOLH(off) (0x1000 + (off) * 8) > #define SR(off) (0x1400 + (off) * 8) > #define IEN(off) (0x1800 + (off) * 8) > +#define PUPD(off) (0x1C00 + (off) * 8) > #define ISEL(off) (0x2C00 + (off) * 8) > #define SD_CH(off, ch) ((off) + (ch) * 4) > #define ETH_POC(off, ch) ((off) + (ch) * 4) > @@ -140,6 +141,7 @@ > #define IEN_MASK 0x01 > #define IOLH_MASK 0x03 > #define SR_MASK 0x01 > +#define PUPD_MASK 0x03 > > #define PM_INPUT 0x1 > #define PM_OUTPUT 0x2 > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data { > void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr); > u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); > int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); > + int (*get_bias_param)(u8 val); > + int (*get_bias_val)(enum pin_config_param param); Please use consistent naming: "pmc_writeb" uses <noun>_<verb> ordering, "get_bias_pararm" uses <verb>_<noun> ordering. Perhaps "hw_to_bias_param()" and "bias_param_to_hw()?" Gr{oetje,eeting}s, Geert
Hi Geert, Thank you for the review. On Wed, May 22, 2024 at 2:14 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > > properties of the pin. > > > > Two new function pointers get_bias_param() and get_bias_val() are > > introduced as the values in PUPD register differ when compared to > > RZ/G2L family and RZ/V2H(P) SoC, > > > > Value | RZ/G2L | RZ/V2H > > --------------------------------- > > 00b: | Bias Disabled | Pull up/down disabled > > 01b: | Pull-up | Pull up/down disabled > > 10b: | Pull-down | Pull-down > > 11b: | Prohibited | Pull-up > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > RFC->v2 > > - New patch > > Thanks for your patch! > > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > > arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK); > > break; > > > > + case PIN_CONFIG_BIAS_DISABLE: > > + case PIN_CONFIG_BIAS_PULL_UP: > > + case PIN_CONFIG_BIAS_PULL_DOWN: { > > + if (!(cfg & PIN_CFG_PUPD)) > > + return -EINVAL; > > + > > + ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl, > > + PUPD(off), > > + bit, PUPD_MASK)); > > This is rather long, so please split it in two parts, like is done in > other cases in this function: > > arg = rzg2l_read_pin_config(pctrl, PUPD(off), bit, PUPD_MASK); > ret = pctrl->data->get_bias_param(arg); > Agreed, I'll update it as per your suggestion. Cheers, Prabhakar
Hi Geert, Thank you for the review. On Wed, May 22, 2024 at 2:26 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Tue, Apr 23, 2024 at 7:59 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > > properties of the pin. > > > > Two new function pointers get_bias_param() and get_bias_val() are > > introduced as the values in PUPD register differ when compared to > > RZ/G2L family and RZ/V2H(P) SoC, > > > > Value | RZ/G2L | RZ/V2H > > --------------------------------- > > 00b: | Bias Disabled | Pull up/down disabled > > 01b: | Pull-up | Pull up/down disabled > > 10b: | Pull-down | Pull-down > > 11b: | Prohibited | Pull-up > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > RFC->v2 > > - New patch > > --- > > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++ > > 1 file changed, 73 insertions(+) > > > > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > index 102fa75c71d3..c144bf43522b 100644 > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > @@ -122,6 +122,7 @@ > > #define IOLH(off) (0x1000 + (off) * 8) > > #define SR(off) (0x1400 + (off) * 8) > > #define IEN(off) (0x1800 + (off) * 8) > > +#define PUPD(off) (0x1C00 + (off) * 8) > > #define ISEL(off) (0x2C00 + (off) * 8) > > #define SD_CH(off, ch) ((off) + (ch) * 4) > > #define ETH_POC(off, ch) ((off) + (ch) * 4) > > @@ -140,6 +141,7 @@ > > #define IEN_MASK 0x01 > > #define IOLH_MASK 0x03 > > #define SR_MASK 0x01 > > +#define PUPD_MASK 0x03 > > > > #define PM_INPUT 0x1 > > #define PM_OUTPUT 0x2 > > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data { > > void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr); > > u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); > > int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); > > + int (*get_bias_param)(u8 val); > > + int (*get_bias_val)(enum pin_config_param param); > > Please use consistent naming: "pmc_writeb" uses <noun>_<verb> ordering, > "get_bias_pararm" uses <verb>_<noun> ordering. > > Perhaps "hw_to_bias_param()" and "bias_param_to_hw()?" > Ok, I'll rename as suggested above. Cheers, Prabhakar
Hi, Prabhakar, On 23.04.2024 20:58, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > properties of the pin. > > Two new function pointers get_bias_param() and get_bias_val() are > introduced as the values in PUPD register differ when compared to > RZ/G2L family and RZ/V2H(P) SoC, > > Value | RZ/G2L | RZ/V2H > --------------------------------- > 00b: | Bias Disabled | Pull up/down disabled > 01b: | Pull-up | Pull up/down disabled > 10b: | Pull-down | Pull-down > 11b: | Prohibited | Pull-up > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > RFC->v2 > - New patch > --- > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++ > 1 file changed, 73 insertions(+) > > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > index 102fa75c71d3..c144bf43522b 100644 > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -122,6 +122,7 @@ > #define IOLH(off) (0x1000 + (off) * 8) > #define SR(off) (0x1400 + (off) * 8) > #define IEN(off) (0x1800 + (off) * 8) > +#define PUPD(off) (0x1C00 + (off) * 8) > #define ISEL(off) (0x2C00 + (off) * 8) > #define SD_CH(off, ch) ((off) + (ch) * 4) > #define ETH_POC(off, ch) ((off) + (ch) * 4) > @@ -140,6 +141,7 @@ > #define IEN_MASK 0x01 > #define IOLH_MASK 0x03 > #define SR_MASK 0x01 > +#define PUPD_MASK 0x03 > > #define PM_INPUT 0x1 > #define PM_OUTPUT 0x2 > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data { > void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr); > u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); > int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); > + int (*get_bias_param)(u8 val); > + int (*get_bias_val)(enum pin_config_param param); > }; > > /** > @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 > return 0; > } > > +static int rzg2l_get_bias_param(u8 val) > +{ > + switch (val) { > + case 0: > + return PIN_CONFIG_BIAS_DISABLE; > + case 1: > + return PIN_CONFIG_BIAS_PULL_UP; > + case 2: > + return PIN_CONFIG_BIAS_PULL_DOWN; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int rzg2l_get_bias_val(enum pin_config_param param) > +{ > + switch (param) { > + case PIN_CONFIG_BIAS_DISABLE: > + return 0; > + case PIN_CONFIG_BIAS_PULL_UP: > + return 1; > + case PIN_CONFIG_BIAS_PULL_DOWN: > + return 2; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > unsigned int _pin, > unsigned long *config) > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK); > break; > > + case PIN_CONFIG_BIAS_DISABLE: > + case PIN_CONFIG_BIAS_PULL_UP: > + case PIN_CONFIG_BIAS_PULL_DOWN: { Block { } can be removed here. > + if (!(cfg & PIN_CFG_PUPD)) > + return -EINVAL; > + > + ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl, > + PUPD(off), > + bit, PUPD_MASK)); > + if (ret < 0) > + return ret; > + > + if (ret != param) > + return -EINVAL; Can this happen? Otherwise it can be removed. > + /* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */ What about bias disable? I haven't checked in detail, is it OK to do arg = 1 here? > + arg = 1; > + break; > + } > + > case PIN_CONFIG_DRIVE_STRENGTH: { > unsigned int index; > > @@ -1254,6 +1309,20 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, > rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg); > break; > > + case PIN_CONFIG_BIAS_DISABLE: > + case PIN_CONFIG_BIAS_PULL_UP: > + case PIN_CONFIG_BIAS_PULL_DOWN: { Block { } can be removed in this case. > + if (!(cfg & PIN_CFG_PUPD)) > + return -EINVAL; > + > + ret = pctrl->data->get_bias_val(param); > + if (ret < 0) > + return ret; > + > + rzg2l_rmw_pin_config(pctrl, PUPD(off), bit, PUPD_MASK, ret); > + break; > + } > + > case PIN_CONFIG_DRIVE_STRENGTH: > arg = pinconf_to_config_argument(_configs[i]); > > @@ -2746,6 +2815,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = { > .pmc_writeb = &rzg2l_pmc_writeb, > .read_oen = &rzg2l_read_oen, > .write_oen = &rzg2l_write_oen, > + .get_bias_param = &rzg2l_get_bias_param, > + .get_bias_val = &rzg2l_get_bias_val, > }; > > static struct rzg2l_pinctrl_data r9a08g045_data = { > @@ -2761,6 +2832,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = { > .pmc_writeb = &rzg2l_pmc_writeb, > .read_oen = &rzg2l_read_oen, > .write_oen = &rzg2l_write_oen, > + .get_bias_param = &rzg2l_get_bias_param, > + .get_bias_val = &rzg2l_get_bias_val, > }; > > static const struct of_device_id rzg2l_pinctrl_of_table[] = {
Hi Claudiu, Thank you for the review. On Thu, May 30, 2024 at 8:48 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > > Hi, Prabhakar, > > On 23.04.2024 20:58, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > > properties of the pin. > > > > Two new function pointers get_bias_param() and get_bias_val() are > > introduced as the values in PUPD register differ when compared to > > RZ/G2L family and RZ/V2H(P) SoC, > > > > Value | RZ/G2L | RZ/V2H > > --------------------------------- > > 00b: | Bias Disabled | Pull up/down disabled > > 01b: | Pull-up | Pull up/down disabled > > 10b: | Pull-down | Pull-down > > 11b: | Prohibited | Pull-up > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > RFC->v2 > > - New patch > > --- > > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++ > > 1 file changed, 73 insertions(+) > > > > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > index 102fa75c71d3..c144bf43522b 100644 > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > @@ -122,6 +122,7 @@ > > #define IOLH(off) (0x1000 + (off) * 8) > > #define SR(off) (0x1400 + (off) * 8) > > #define IEN(off) (0x1800 + (off) * 8) > > +#define PUPD(off) (0x1C00 + (off) * 8) > > #define ISEL(off) (0x2C00 + (off) * 8) > > #define SD_CH(off, ch) ((off) + (ch) * 4) > > #define ETH_POC(off, ch) ((off) + (ch) * 4) > > @@ -140,6 +141,7 @@ > > #define IEN_MASK 0x01 > > #define IOLH_MASK 0x03 > > #define SR_MASK 0x01 > > +#define PUPD_MASK 0x03 > > > > #define PM_INPUT 0x1 > > #define PM_OUTPUT 0x2 > > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data { > > void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr); > > u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); > > int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); > > + int (*get_bias_param)(u8 val); > > + int (*get_bias_val)(enum pin_config_param param); > > }; > > > > /** > > @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 > > return 0; > > } > > > > +static int rzg2l_get_bias_param(u8 val) > > +{ > > + switch (val) { > > + case 0: > > + return PIN_CONFIG_BIAS_DISABLE; > > + case 1: > > + return PIN_CONFIG_BIAS_PULL_UP; > > + case 2: > > + return PIN_CONFIG_BIAS_PULL_DOWN; > > + default: > > + break; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int rzg2l_get_bias_val(enum pin_config_param param) > > +{ > > + switch (param) { > > + case PIN_CONFIG_BIAS_DISABLE: > > + return 0; > > + case PIN_CONFIG_BIAS_PULL_UP: > > + return 1; > > + case PIN_CONFIG_BIAS_PULL_DOWN: > > + return 2; > > + default: > > + break; > > + } > > + > > + return -EINVAL; > > +} > > + > > static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > > unsigned int _pin, > > unsigned long *config) > > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > > arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK); > > break; > > > > + case PIN_CONFIG_BIAS_DISABLE: > > + case PIN_CONFIG_BIAS_PULL_UP: > > + case PIN_CONFIG_BIAS_PULL_DOWN: { > > Block { } can be removed here. > Agreed, I will drop it. > > + if (!(cfg & PIN_CFG_PUPD)) > > + return -EINVAL; > > + > > + ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl, > > + PUPD(off), > > + bit, PUPD_MASK)); > > + if (ret < 0) > > + return ret; > > + > > + if (ret != param) > > + return -EINVAL; > > Can this happen? Otherwise it can be removed. > > > + /* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */ > > What about bias disable? I haven't checked in detail, is it OK to do > arg = 1 here? > For BIAS_DISABLE config there isn't any argument, hence the above comment mentions only for UP/DOWN. Passing arg = 1 for BIAS_DISABLE has no effect. > > + arg = 1; > > + break; > > + } > > + > > case PIN_CONFIG_DRIVE_STRENGTH: { > > unsigned int index; > > > > @@ -1254,6 +1309,20 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, > > rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg); > > break; > > > > + case PIN_CONFIG_BIAS_DISABLE: > > + case PIN_CONFIG_BIAS_PULL_UP: > > + case PIN_CONFIG_BIAS_PULL_DOWN: { > > Block { } can be removed in this case. > Agreed, I will drop it. Cheers, Prabhakar
On Thu, May 30, 2024 at 8:48 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > > Hi, Prabhakar, > > On 23.04.2024 20:58, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Add support to configure bias-disable, bias-pull-up and bias-pull-down > > properties of the pin. > > > > Two new function pointers get_bias_param() and get_bias_val() are > > introduced as the values in PUPD register differ when compared to > > RZ/G2L family and RZ/V2H(P) SoC, > > > > Value | RZ/G2L | RZ/V2H > > --------------------------------- > > 00b: | Bias Disabled | Pull up/down disabled > > 01b: | Pull-up | Pull up/down disabled > > 10b: | Pull-down | Pull-down > > 11b: | Prohibited | Pull-up > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > RFC->v2 > > - New patch > > --- > > drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++++ > > 1 file changed, 73 insertions(+) > > > > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > index 102fa75c71d3..c144bf43522b 100644 > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > > @@ -122,6 +122,7 @@ > > #define IOLH(off) (0x1000 + (off) * 8) > > #define SR(off) (0x1400 + (off) * 8) > > #define IEN(off) (0x1800 + (off) * 8) > > +#define PUPD(off) (0x1C00 + (off) * 8) > > #define ISEL(off) (0x2C00 + (off) * 8) > > #define SD_CH(off, ch) ((off) + (ch) * 4) > > #define ETH_POC(off, ch) ((off) + (ch) * 4) > > @@ -140,6 +141,7 @@ > > #define IEN_MASK 0x01 > > #define IOLH_MASK 0x03 > > #define SR_MASK 0x01 > > +#define PUPD_MASK 0x03 > > > > #define PM_INPUT 0x1 > > #define PM_OUTPUT 0x2 > > @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data { > > void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr); > > u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); > > int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); > > + int (*get_bias_param)(u8 val); > > + int (*get_bias_val)(enum pin_config_param param); > > }; > > > > /** > > @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 > > return 0; > > } > > > > +static int rzg2l_get_bias_param(u8 val) > > +{ > > + switch (val) { > > + case 0: > > + return PIN_CONFIG_BIAS_DISABLE; > > + case 1: > > + return PIN_CONFIG_BIAS_PULL_UP; > > + case 2: > > + return PIN_CONFIG_BIAS_PULL_DOWN; > > + default: > > + break; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int rzg2l_get_bias_val(enum pin_config_param param) > > +{ > > + switch (param) { > > + case PIN_CONFIG_BIAS_DISABLE: > > + return 0; > > + case PIN_CONFIG_BIAS_PULL_UP: > > + return 1; > > + case PIN_CONFIG_BIAS_PULL_DOWN: > > + return 2; > > + default: > > + break; > > + } > > + > > + return -EINVAL; > > +} > > + > > static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > > unsigned int _pin, > > unsigned long *config) > > @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, > > arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK); > > break; > > > > + case PIN_CONFIG_BIAS_DISABLE: > > + case PIN_CONFIG_BIAS_PULL_UP: > > + case PIN_CONFIG_BIAS_PULL_DOWN: { > > Block { } can be removed here. > > > + if (!(cfg & PIN_CFG_PUPD)) > > + return -EINVAL; > > + > > + ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl, > > + PUPD(off), > > + bit, PUPD_MASK)); > > + if (ret < 0) > > + return ret; > > + > > + if (ret != param) > > + return -EINVAL; > > Can this happen? Otherwise it can be removed. > Yes this can happen (and is needed) as we want to report only the current BIAS setting of the pin. For example without this condition I get the below for ET1_RXD3 pin: pin 173 (ET1_RXD3): input bias disabled, input bias pull down (0x1 ohms), input bias pull up (0x1 ohms) with the check included: pin 173 (ET1_RXD3): input bias disabled Cheers, Prabhakar
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 102fa75c71d3..c144bf43522b 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -122,6 +122,7 @@ #define IOLH(off) (0x1000 + (off) * 8) #define SR(off) (0x1400 + (off) * 8) #define IEN(off) (0x1800 + (off) * 8) +#define PUPD(off) (0x1C00 + (off) * 8) #define ISEL(off) (0x2C00 + (off) * 8) #define SD_CH(off, ch) ((off) + (ch) * 4) #define ETH_POC(off, ch) ((off) + (ch) * 4) @@ -140,6 +141,7 @@ #define IEN_MASK 0x01 #define IOLH_MASK 0x03 #define SR_MASK 0x01 +#define PUPD_MASK 0x03 #define PM_INPUT 0x1 #define PM_OUTPUT 0x2 @@ -265,6 +267,8 @@ struct rzg2l_pinctrl_data { void (*pmc_writeb)(struct rzg2l_pinctrl *pctrl, u8 val, void __iomem *addr); u32 (*read_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin); int (*write_oen)(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen); + int (*get_bias_param)(u8 val); + int (*get_bias_val)(enum pin_config_param param); }; /** @@ -1081,6 +1085,38 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 return 0; } +static int rzg2l_get_bias_param(u8 val) +{ + switch (val) { + case 0: + return PIN_CONFIG_BIAS_DISABLE; + case 1: + return PIN_CONFIG_BIAS_PULL_UP; + case 2: + return PIN_CONFIG_BIAS_PULL_DOWN; + default: + break; + } + + return -EINVAL; +} + +static int rzg2l_get_bias_val(enum pin_config_param param) +{ + switch (param) { + case PIN_CONFIG_BIAS_DISABLE: + return 0; + case PIN_CONFIG_BIAS_PULL_UP: + return 1; + case PIN_CONFIG_BIAS_PULL_DOWN: + return 2; + default: + break; + } + + return -EINVAL; +} + static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, unsigned int _pin, unsigned long *config) @@ -1139,6 +1175,25 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, arg = rzg2l_read_pin_config(pctrl, SR(off), bit, SR_MASK); break; + case PIN_CONFIG_BIAS_DISABLE: + case PIN_CONFIG_BIAS_PULL_UP: + case PIN_CONFIG_BIAS_PULL_DOWN: { + if (!(cfg & PIN_CFG_PUPD)) + return -EINVAL; + + ret = pctrl->data->get_bias_param(rzg2l_read_pin_config(pctrl, + PUPD(off), + bit, PUPD_MASK)); + if (ret < 0) + return ret; + + if (ret != param) + return -EINVAL; + /* for PIN_CONFIG_BIAS_PULL_UP/DOWN when enabled we just return 1 */ + arg = 1; + break; + } + case PIN_CONFIG_DRIVE_STRENGTH: { unsigned int index; @@ -1254,6 +1309,20 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, rzg2l_rmw_pin_config(pctrl, SR(off), bit, SR_MASK, arg); break; + case PIN_CONFIG_BIAS_DISABLE: + case PIN_CONFIG_BIAS_PULL_UP: + case PIN_CONFIG_BIAS_PULL_DOWN: { + if (!(cfg & PIN_CFG_PUPD)) + return -EINVAL; + + ret = pctrl->data->get_bias_val(param); + if (ret < 0) + return ret; + + rzg2l_rmw_pin_config(pctrl, PUPD(off), bit, PUPD_MASK, ret); + break; + } + case PIN_CONFIG_DRIVE_STRENGTH: arg = pinconf_to_config_argument(_configs[i]); @@ -2746,6 +2815,8 @@ static struct rzg2l_pinctrl_data r9a07g044_data = { .pmc_writeb = &rzg2l_pmc_writeb, .read_oen = &rzg2l_read_oen, .write_oen = &rzg2l_write_oen, + .get_bias_param = &rzg2l_get_bias_param, + .get_bias_val = &rzg2l_get_bias_val, }; static struct rzg2l_pinctrl_data r9a08g045_data = { @@ -2761,6 +2832,8 @@ static struct rzg2l_pinctrl_data r9a08g045_data = { .pmc_writeb = &rzg2l_pmc_writeb, .read_oen = &rzg2l_read_oen, .write_oen = &rzg2l_write_oen, + .get_bias_param = &rzg2l_get_bias_param, + .get_bias_val = &rzg2l_get_bias_val, }; static const struct of_device_id rzg2l_pinctrl_of_table[] = {