Re: [PATCH 2/3] tmp/tpm_crb: fix Intel PTT hw bug during idle state

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Wed, Sep 07, 2016 at 09:21:34PM +0000, Winkler, Tomas wrote:
> 
> > 
> > On Wed, Sep 07, 2016 at 02:32:32PM +0300, Tomas Winkler wrote:
> > >  	INIT_LIST_HEAD(&resources);
> > > -	ret = acpi_dev_get_resources(device, &resources,
> > crb_check_resource,
> > > -				     &io_res);
> > > +	ret = acpi_dev_get_resources(device, &resources,
> > > +				     crb_check_resource, &io_res);
> > 
> > Do not randomly reflow unrelated text in patches
> 
> It wasn't random, who breaks code like that...

..  it has nothing to do with this patch.

., and I have no idea why you think that is better, the original is what
clang-format produces and it computes an optimal break point using
Knuth's algorithm...

> > > +	/*
> > > +	 * PTT HW bug w/a: wake up the device to access
> > > +	 * possibly not retained registers.
> > > +	 */
> > > +	ret = __crb_cmd_ready(dev, priv);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pa_high = ioread32(&priv->cca->cmd_pa_high);
> > > +	pa_low  = ioread32(&priv->cca->cmd_pa_low);
> > > +	cmd_pa = ((u64)pa_high << 32) | pa_low;
> > 
> > Why change from the original hunk?
> 
> This is where the bug is visible... I'll put the debug print back it might be useful. 

I don't get it, what is the difference? read ordering?

> >  -	cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) |
> >  -		  (u64) ioread32(&priv->cca->cmd_pa_low);

Jason

------------------------------------------------------------------------------