[PATCH 2/2] pinctrl: at91-pio4: add support for slew-rate

[Eugen.Hristev at microchip.com <Eugen.Hristev@microchip.com>]

On 27.01.2021 15:00, Claudiu Beznea wrote:
> SAMA7G5 supports slew rate configuration. Adapt the driver for this.
> For switching frequencies lower than 50MHz the slew rate needs to
> be enabled. Since most of the pins on SAMA7G5 fall into this category
> enabled the slew rate by default.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>   arch/arm/mach-at91/include/mach/atmel_pio4.h |  1 +
>   drivers/pinctrl/pinctrl-at91-pio4.c          | 26 +++++++++++++++++++++++---
>   2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/include/mach/atmel_pio4.h b/arch/arm/mach-at91/include/mach/atmel_pio4.h
> index 35ac7b2d40e1..c3bd9140dfef 100644
> --- a/arch/arm/mach-at91/include/mach/atmel_pio4.h
> +++ b/arch/arm/mach-at91/include/mach/atmel_pio4.h
> @@ -44,6 +44,7 @@ struct atmel_pio4_port {
>   #define ATMEL_PIO_DIR_MASK		BIT(8)
>   #define ATMEL_PIO_PUEN_MASK		BIT(9)
>   #define ATMEL_PIO_PDEN_MASK		BIT(10)
> +#define ATMEL_PIO_SR			BIT(11)
>   #define ATMEL_PIO_IFEN_MASK		BIT(12)
>   #define ATMEL_PIO_IFSCEN_MASK		BIT(13)
>   #define ATMEL_PIO_OPD_MASK		BIT(14)
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
> index 3a5143adc381..5c6ece745ab0 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -24,6 +24,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   
>   struct atmel_pio4_plat {
>   	struct atmel_pio4_port *reg_base;
> +	unsigned int slew_rate_support;
>   };
>   
>   static const struct pinconf_param conf_params[] = {
> @@ -35,9 +36,11 @@ static const struct pinconf_param conf_params[] = {
>   	{ "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
>   	{ "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 },
>   	{ "atmel,drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 },
> +	{ "slew-rate", PIN_CONFIG_SLEW_RATE, 0},
Hi Claudiu,

If slew rate is enabled by default, we should have here 0 or 1 ?

>   };
>   
> -static u32 atmel_pinctrl_get_pinconf(struct udevice *config)
> +static u32 atmel_pinctrl_get_pinconf(struct udevice *config,
> +				     struct atmel_pio4_plat *plat)
>   {
>   	const struct pinconf_param *params;
>   	u32 param, arg, conf = 0;
> @@ -52,6 +55,10 @@ static u32 atmel_pinctrl_get_pinconf(struct udevice *config)
>   		param = params->param;
>   		arg = params->default_value;
>   
> +		/* Keep slew rate enabled by default. */
> +		if (plat->slew_rate_support)
> +			conf |= ATMEL_PIO_SR;
> +
>   		switch (param) {
>   		case PIN_CONFIG_BIAS_DISABLE:
>   			conf &= (~ATMEL_PIO_PUEN_MASK);
> @@ -90,6 +97,15 @@ static u32 atmel_pinctrl_get_pinconf(struct udevice *config)
>   			conf |= (val << ATMEL_PIO_DRVSTR_OFFSET)
>   				& ATMEL_PIO_DRVSTR_MASK;
>   			break;
> +		case PIN_CONFIG_SLEW_RATE:
> +			if (!plat->slew_rate_support)
> +				break;
> +
> +			dev_read_u32(config, params->property, &val);
> +			/* And disable it if requested. */
> +			if (val == 0)
> +				conf &= ~ATMEL_PIO_SR;
> +			break;
>   		default:
>   			printf("%s: Unsupported configuration parameter: %u\n",
>   			       __func__, param);
> @@ -115,6 +131,7 @@ static inline struct atmel_pio4_port *atmel_pio4_bank_base(struct udevice *dev,
>   
>   static int atmel_pinctrl_set_state(struct udevice *dev, struct udevice *config)
>   {
> +	struct atmel_pio4_plat *plat = dev_get_plat(dev);
>   	struct atmel_pio4_port *bank_base;
>   	const void *blob = gd->fdt_blob;
>   	int node = dev_of_offset(config);
> @@ -123,7 +140,7 @@ static int atmel_pinctrl_set_state(struct udevice *dev, struct udevice *config)
>   	u32 i, conf;
>   	int count;
>   
> -	conf = atmel_pinctrl_get_pinconf(config);
> +	conf = atmel_pinctrl_get_pinconf(config, plat);
>   
>   	count = fdtdec_get_int_array_count(blob, node, "pinmux",
>   					   cells, ARRAY_SIZE(cells));
> @@ -163,6 +180,7 @@ const struct pinctrl_ops atmel_pinctrl_ops  = {
>   static int atmel_pinctrl_probe(struct udevice *dev)
>   {
>   	struct atmel_pio4_plat *plat = dev_get_plat(dev);
> +	ulong priv = dev_get_driver_data(dev);
>   	fdt_addr_t addr_base;
>   
>   	dev = dev_get_parent(dev);
> @@ -171,13 +189,15 @@ static int atmel_pinctrl_probe(struct udevice *dev)
>   		return -EINVAL;
>   
>   	plat->reg_base = (struct atmel_pio4_port *)addr_base;
> +	plat->slew_rate_support = priv;
>   
>   	return 0;
>   }
>   
>   static const struct udevice_id atmel_pinctrl_match[] = {
>   	{ .compatible = "atmel,sama5d2-pinctrl" },
> -	{ .compatible = "microchip,sama7g5-pinctrl" },
> +	{ .compatible = "microchip,sama7g5-pinctrl",
> +	  .data = (ulong)1, },


Is this fine to have the data converted to an ulong, a hardcoded '1', 
and then in the plat struct we have an unsigned int that takes this value ?

Maybe it would be better to have a config struct as we have in the macb 
driver for example ? And this struct would have a field that makes more 
sense, like 'has_slew_rate' or something.

Eugen

>   	{}
>   };
>   
>