Re: [PATCH] pinctrl: intel: save HOSTSW_OWN register over suspend/resume

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Apr 01, 2019 at 06:41:57PM +0800, Chris Chiu wrote:
> On Mon, Apr 1, 2019 at 3:49 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Fri, Mar 29, 2019 at 04:38:20PM +0800, Chris Chiu wrote:

> > Sure I can but it probably does not happen until end of the week because
> > I'm currently busy with something else.
> 
> Thanks for your attention. I don't want to distract you so I'll try to
> refine the
> patch. It would be a great help if you can help review and give comments.
> 
> Don't know whether if the following patch still get the wrong idea about
> your thought.  It saves the hostsw_own when GPIO requested, check
> if the value differs in resume() and restore if necessary. Please kindly
> correct me if any. Thanks

Thanks for the patch.
My comments below.

> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index 8cda7b535b02..d1cfa5adef9b 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -77,6 +77,7 @@ struct intel_pad_context {
>         u32 padcfg0;
>         u32 padcfg1;
>         u32 padcfg2;

> +       u32 hostown;

This is wrong. We have one register per entire (*) group of pins to keep host
ownership. Basically it's a mask.

*) if it's <= 32, otherwise there are more registers. But in any case it's 1
bit per pin, and not 32 bits.

>         for (i = 0; i < pctrl->soc->npins; i++)

Thus, the actual actions should mimic what we do for interrupt mask.

-- 
With Best Regards,
Andy Shevchenko