Re: [PATCH] i2c: i801: Fix resume bug

[Jean Delvare <jdelvare@suse.de>]

On Mon, 31 Aug 2020 20:29:29 +0530, Vaibhav Gupta wrote:
> On Fri, Aug 28, 2020 at 10:57:53AM +0200, Wolfram Sang wrote:
> > On Tue, Aug 25, 2020 at 07:13:03PM +0200, Jean Delvare wrote:  
> > > @@ -1961,8 +1965,11 @@ static int i801_suspend(struct device *d
> > >  
> > >  static int i801_resume(struct device *dev)
> > >  {
> > > -	struct i801_priv *priv = dev_get_drvdata(dev);
> > > +	struct pci_dev *pci_dev = to_pci_dev(dev);
> > > +	struct i801_priv *priv = pci_get_drvdata(pci_dev);
> > > +	unsigned char hstcfg = i801_setup_hstcfg(priv->original_hstcfg);
> > >  
> > > +	pci_write_config_byte(pci_dev, SMBHSTCFG, hstcfg);  
> > 
> > And on top of that, we could skip the 'hstcfg' variable here.
>
> Add extra parameter of "struct pci_dev*" type in i801_setup_hstcfg().

That was my first approach, yes. But then I tried passing the struct
i801_priv * instead, and it looks even neater to me (but I'm biased
because I did it). I'll post that for review shortly.

> We can
> also skip the introduction of "struct pci_dev*" type variable in .resume()
> then. Just use to_pci_dev(dev) .

I don't really mind keeping the pci_dev variable, for symmetry with
.suspend(), and the compiler will optimize it away anyway. There's a
minor cleanup possible in this area though, to which I'll give a try.

Thanks,
-- 
Jean Delvare
SUSE L3 Support