[PATCH 3/3] coresight etm4x: Add 32-bit read/write option to split 64-bit words

[Leo Yan]

On Thu, Mar 09, 2023 at 12:46:55PM -0800, Steve Clevenger wrote:

[...]

> > #include <linux/io-64-nonatomic-hi-lo.h>
> > 
> > static inline u64 etm4x_relaxed_read64(struct csdev_access *csa, unsigned int offset)
> > {
> > 	if (csa->io_mem) {
> > 		if (csa->no_quad_mmio)
> >                         return hi_lo_readq_relaxed(csa->base + offset);
> > 		else
> > 			return readq_relaxed(csa->base + offset);
> > 	} else {
> > 		return read_etm4x_sysreg_offset(offset, true);
> >         }
> > }
> > 
> > I'm not saying to play magic on {writeq|readq}_relaxed(), would it be
> > fine to directly call hi_lo_readq_relaxed()?
> 
> I can do this, and it would work fine. Without compile protection, one
> concern is the silent inclusion of io-64-nonatomic-hi-lo.h definitions
> for readq_relaxed/writeq_relaxed if the include hierarchy gets changed.
> The only (minor) risk I see is to performance.

Yeah, we could include io.h ahead io-64-nonatomic-hi-lo.h:

  #include <linux/io.h>
  #include <linux/io-64-nonatomic-lo-hi.h>

Thus we can get the definitions for {readq|writeq}_relaxed() from
"io.h".

> Second, it seems to me the hi_lo and lo_hi macros were intended to deal
> with endianness. IMO, the implementation I offered is agnostic without a
> hint to endianness or atomicity.

Seems to me, kernel functions with explict endianness naming is not bad
thing :)

> The CoreSight maintainer audience is
> limited and already aware of these issues, so I'll make the change for you.

Thanks!  Suzuki, Mike, I think it's good to get your opinion before
Steve can proceed for updating patch.

Leo