Re: m68k: coldfire: create internal register access defines

Hi Arnd,

On 30/4/26 17:39, Arnd Bergmann wrote:
> On Thu, Apr 30, 2026, at 07:19, Greg Ungerer wrote:
>> The readx/writex family of IO access functions for ColdFire are
>> non-standard. They return native endian data for multi-byte access,
>> which on m68k/ColdFire is big-endian byte order. A number of places
>> need fixing to change readx/writex to be more standard - a lot of
>> the CPU/SoC architecture specific code and a number of drivers.
>>
>> The existing ColdFire CPU/SoC architecture code is inconsistent in
>> how it accesses SoC hardware module registers. Both readx/writex
>> and __raw_readx/__raw_writex are used across the code base. To clean
>> this up this set of patches moves to an internal set of access
>> defines for the CPU/SoC architecture code. The following are defined
>> and used for all internal register access:
>>
>>      8 bit read  -->  mcf_read8
>>      8 bit write -->  mcf_write8
>>     16 bit read  -->  mcf_read16
>>     16 bit write -->  mcf_write16
>>     32 bit read  -->  mcf_read32
>>     32 bit write -->  mcf_write32
>>
>> To ease review I have broken the changes up a little, grouping major
>> blocks together instead of one huge patch.
> 
> These all look good to me,
> 
> Reviewed-by: Arnd Bergmann <[email protected]>

Thanks.


> There are still open questions about how to continue from here
> to change the existing readl() and ioread32be() style helpers
> to have normal endianess and type characteristics without
> breaking things, but this is a good step in that direction.

I have been playing with one idea. I have been working through the affected
drivers and changing them to use raw primitives only for their IO access
on ColdFire (so only the __raw_readx/__raw_writex macros). My thinking is
that these can then be pushed via driver subsystem maintainers if and when
they are ready. When all are affected drivers are fixed then we can correct
the definitions in arch/m68k/include/asm/io_no.h. At a later time if we choose
we can then make changes to effected drivers again to use the now corrected
readx/writex family.

At least this way we can keep everything working and avoid a single patch that
makes changes to io_no.h and drivers across multiple subsystems in one go.
What do you think?

So far I have modified the fec and smc91x ethernet drivers in this way.
That was strait forward, though I may get a little resistance in the fec
driver since I abstracted the IO access from readl/writel to local internal
functions to keep it clean and avoid redefining readl/wrtel. But the change
is simple in concept. I am working through the handful of other drivers
that we identified.

Regards
Greg