Date:   Wed, 6 Feb 2019 11:24:06 +0100
From:   Ludovic Desroches <ludovic.desroches@...rochip.com>
To:     <Claudiu.Beznea@...rochip.com>
CC:     <linus.walleij@...aro.org>, <robh+dt@...nel.org>,
        <mark.rutland@....com>, <Nicolas.Ferre@...rochip.com>,
        <alexandre.belloni@...tlin.com>, <linux-gpio@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/7] pinctrl: at91: add option to use drive strength
 bits

On Wed, Feb 06, 2019 at 09:46:28AM +0000, Claudiu.Beznea@...rochip.com wrote:
> 
> 
> On 06.02.2019 10:13, Ludovic Desroches wrote:
> > On Thu, Jan 31, 2019 at 05:18:04PM +0100, Claudiu Beznea - M18063 wrote:
> >> From: Claudiu Beznea <claudiu.beznea@...rochip.com>
> >>
> >> SAM9X60 uses high and low drive strengths. To implement this, in
> >> at91_pinctrl_mux_ops::set_drivestrength and
> >> at91_pinctrl_mux_ops::get_drivestrength we need bit numbers of
> >> drive strengths (1 for low, 2 for high), thus change the code to
> >> allow the usage of drive strength bit numbers.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@...rochip.com>
> >> ---
> >>  drivers/pinctrl/pinctrl-at91.c | 32 ++++++++++++++++++++------------
> >>  1 file changed, 20 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >> index 3d49bbbcdbc7..31f06dafca2e 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -72,10 +72,15 @@ static int gpio_banks;
> >>   * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the drive
> >>   * strength when there is no dt config for it.
> >>   */
> >> -#define DRIVE_STRENGTH_DEFAULT		(0 << DRIVE_STRENGTH_SHIFT)
> >> -#define DRIVE_STRENGTH_LOW          (1 << DRIVE_STRENGTH_SHIFT)
> >> -#define DRIVE_STRENGTH_MED          (2 << DRIVE_STRENGTH_SHIFT)
> >> -#define DRIVE_STRENGTH_HI           (3 << DRIVE_STRENGTH_SHIFT)
> >> +enum drive_strength_bit {
> >> +	DRIVE_STRENGTH_BIT_DEF,
> >> +	DRIVE_STRENGTH_BIT_LOW,
> >> +	DRIVE_STRENGTH_BIT_MED,
> >> +	DRIVE_STRENGTH_BIT_HI,
> >> +};
> >> +
> >> +#define DRIVE_STRENGTH_BIT_MSK(name)	(DRIVE_STRENGTH_BIT_##name << \
> >> +					 DRIVE_STRENGTH_SHIFT)
> >>  
> >>  /**
> >>   * struct at91_pmx_func - describes AT91 pinmux functions
> >> @@ -551,7 +556,7 @@ static unsigned at91_mux_sama5d3_get_drivestrength(void __iomem *pio,
> >>  	/* SAMA5 strength is 1:1 with our defines,
> >>  	 * except 0 is equivalent to low per datasheet */
> >>  	if (!tmp)
> >> -		tmp = DRIVE_STRENGTH_LOW;
> >> +		tmp = DRIVE_STRENGTH_BIT_MSK(LOW);
> >>  
> >>  	return tmp;
> >>  }
> >> @@ -564,7 +569,7 @@ static unsigned at91_mux_sam9x5_get_drivestrength(void __iomem *pio,
> >>  
> >>  	/* strength is inverse in SAM9x5s hardware with the pinctrl defines
> >>  	 * hardware: 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> >> -	tmp = DRIVE_STRENGTH_HI - tmp;
> >> +	tmp = DRIVE_STRENGTH_BIT_MSK(HI) - tmp;
> >>  
> >>  	return tmp;
> >>  }
> >> @@ -600,7 +605,7 @@ static void at91_mux_sam9x5_set_drivestrength(void __iomem *pio, unsigned pin,
> >>  
> >>  	/* strength is inverse on SAM9x5s with our defines
> >>  	 * 0 = hi, 1 = med, 2 = low, 3 = rsvd */
> >> -	setting = DRIVE_STRENGTH_HI - setting;
> >> +	setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting;
> >>  
> >>  	set_drive_strength(pio + at91sam9x5_get_drive_register(pin), pin,
> >>  				setting);
> >> @@ -959,11 +964,11 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> >>  	}					\
> >>  } while (0)
> >>  
> >> -#define DBG_SHOW_FLAG_MASKED(mask,flag) do {	\
> >> +#define DBG_SHOW_FLAG_MASKED(mask,flag,name) do {	\
> > 
> > checkpatch error: space required
> 
> I'm aware of that but I added it in the way it was previously (were the
> checkpatch error was still present). Please let me know if you want me to
> send a new version fixing this.

I think it's the opportunity to fix it. If you can resend a new version
including my ack.

Regards

Ludovic

> 
> Thank you,
> Claudiu Beznea
> 
> > 
> >>  	if ((config & mask) == flag) {		\
> >>  		if (num_conf)			\
> >>  			seq_puts(s, "|");	\
> >> -		seq_puts(s, #flag);		\
> >> +		seq_puts(s, #name);		\
> >>  		num_conf++;			\
> >>  	}					\
> >>  } while (0)
> >> @@ -981,9 +986,12 @@ static void at91_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> >>  	DBG_SHOW_FLAG(PULL_DOWN);
> >>  	DBG_SHOW_FLAG(DIS_SCHMIT);
> >>  	DBG_SHOW_FLAG(DEGLITCH);
> >> -	DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_LOW);
> >> -	DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_MED);
> >> -	DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_HI);
> >> +	DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(LOW),
> >> +			     DRIVE_STRENGTH_LOW);
> >> +	DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(MED),
> >> +			     DRIVE_STRENGTH_MED);
> >> +	DBG_SHOW_FLAG_MASKED(DRIVE_STRENGTH, DRIVE_STRENGTH_BIT_MSK(HI),
> >> +			     DRIVE_STRENGTH_HI);
> >>  	DBG_SHOW_FLAG(DEBOUNCE);
> >>  	if (config & DEBOUNCE) {
> >>  		val = config >> DEBOUNCE_VAL_SHIFT;
> >> -- 
> >> 2.7.4
> >>
> > 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Powered by blists - more mailing lists