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
