Re: [PATCH v1] pinctrl: intel: Allow to request locked pins

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Aug 06, 2019 at 02:01:26PM +0300, Mika Westerberg wrote:
> On Fri, Jul 26, 2019 at 11:08:30PM +0300, Andy Shevchenko wrote:
> > Some firmwares would like to protect pins from being modified by OS
> > and at the same time provide them to OS as a resource. So, the driver
> > in such circumstances may request pin and may not change its state.
> 
> This is definitely good idea.

Thanks for review, my answers below.

> >  	 * the pad is considered unlocked. Any other case means that it is
> >  	 * either fully or partially locked and we don't touch it.
> 
> I think you should update the above comment as well.

Will do for v2.

> >  	raw_spin_lock_irqsave(&pctrl->lock, flags);
> >  
> > -	if (!intel_pad_usable(pctrl, pin)) {
> > +	if (!intel_pad_owned_by_host(pctrl, pin)) {
> >  		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> >  		return -EBUSY;
> >  	}
> >  
> > +	if (!intel_pad_is_unlocked(pctrl, pin)) {
> > +		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > +		return 0;
> 
> Hmm, if I'm reading this right it still does not allow requesting locked
> pins. What I'm missing here?

We do not return an error code here, so pin is left requested but pad
configuration register untouched.

> > +	}
> > +
> >  	padcfg0 = intel_get_padcfg(pctrl, pin, PADCFG0);
> >  	intel_gpio_set_gpio_mode(padcfg0);

-- 
With Best Regards,
Andy Shevchenko