Re: Suspicious debounce handling code in pintctrl-baytrail

["Shevchenko, Andriy" <andriy.shevchenko@intel.com>]

On Thu, 2017-01-26 at 15:00 +0100, Jean Delvare wrote:
> Hi Mika, hello Andy,
> 
> On Thu, 26 Jan 2017 15:43:41 +0200, Mika Westerberg wrote:
> > On Thu, Jan 26, 2017 at 02:20:20PM +0100, Jean Delvare wrote:

> Thanks for the pointer, I wasn't aware of that patch. Looks overall
> good, but:
> 
> +			case 0:
> +				conf &= BYT_DEBOUNCE_EN;
> +				break;
> 
> looks suspicious to me. I think ~BYT_DEBOUNCE_EN was intended?

Yes!

> Furthermore I would expect conf |= BYT_DEBOUNCE_EN in the other cases,
> otherwise disabling debounce once will prevent you from enabling it
> forever.

Yes.

> Also Andy's patch doesn't seem to address my second point:
> 
> > Additionally, I suspect that in the "get" path, the following is
> > wrong:
> > 
> > 	case PIN_CONFIG_INPUT_DEBOUNCE:
> > 		if (!(conf & BYT_DEBOUNCE_EN))
> > 			return -EINVAL;
> > 
> > I'm not familiar with the pinctrl interface but my understanding of
> > the PIN_CONFIG_INPUT_DEBOUNCE documentation is that the driver must
> > return 0 if debouncing is disabled.
> 
> ?

I need to check this.

I will address your comments. Thanks for report!

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo