Re: [PATCH] tpm_tis: fix stall after iowrite*()s

[Haris Okanovic]

On 08/15/2017 01:11 AM, Alexander Stein wrote:
> On Monday 14 August 2017 17:53:47, Haris Okanovic wrote:
> > --- a/drivers/char/tpm/tpm_tis.c
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -52,6 +52,22 @@ static inline struct tpm_tis_tcg_phy
> > *to_tpm_tis_tcg_phy(struct tpm_tis_data *da return container_of(data,
> > struct tpm_tis_tcg_phy, priv);
> >   }
> >
> > +static inline void tpm_tis_iowrite8(u8 b, void __iomem *iobase, u32 addr)
> > +{
> > +	iowrite8(b, iobase + addr);
> > +#ifdef CONFIG_PREEMPT_RT_FULL
> > +	ioread8(iobase + TPM_ACCESS(0));
> > +#endif
> > +}
>
> Maybe add some comment why an iorad8 is actually requried after each write on
> RT. Currently it is rather obvious why this additional read is necessary. But
> is this still the case in a year?

Sure. I re-added the tpm_tis_flush() function with comments to explain 
what's going. Calling it from tpm_tis_iowrite8() and tpm_tis_iowrite32().

Will post a PATCH v2 shortly.

> > +static inline void tpm_tis_iowrite32(u32 b, void __iomem *iobase, u32 addr)
> > +{
> > +	iowrite32(b, iobase + addr);
> > +#ifdef CONFIG_PREEMPT_RT_FULL
> > +	ioread8(iobase + TPM_ACCESS(0));
> > +#endif
> > +}
>
> Same applies here. Or add a comment above both functions describing their
> purpose.
>
> Just my 2 cents
>
> Best regards,
> Alexander