On Wed, Dec 20, 2017 at 07:26:03PM +0100, Javier Martinez Canillas wrote:

On 12/20/2017 07:10 PM, Jason Gunthorpe wrote:

quoted

On Wed, Dec 20, 2017 at 03:19:19PM +0000, Shaikh, Azhar wrote:

quoted

quoted

This flag is only used to warn if CLKRUN_EN wasn't disabled on Braswell
systems, but the only way this can happen is if the code is not correct.

So it's an unnecessary check that just makes the code harder to read.

This code was implemented as a suggestion from Jason on the previous patches. 
https://www.spinics.net/lists/linux-integrity/msg00827.html

The concept was to be like ASSERT_RTNL, maybe it just needs a suitably
named static inline to addrress Javier's readability concerns?

I really think is not worth it and pollutes all the tpm_tcg_{read,write}
functions with those is_bsw() and flags checks. Your example is different
since is a core API used by in lot of places in the kernel, but it's not
the case here.

But I don't have a strong opinion either, it was Jarkko who questioned
the value of the flag so I can drop this patch too if you disagree with
the change. I'm mostly interested in PATCH 4/4 that's the actual fix.

Not going to fight over this one. I would apply the patch but if there
is strong opposition I can reconsider.

/Jarkko