Re: [PATCH] can: mscan: replace in_8/out_8 with ioread8/iowrite8

[Vincent Mailhol <mailhol@kernel.org>]

On 03/06/2026 at 02:43, Rosen Penev wrote:
> On Tue, Jun 2, 2026 at 5:29 PM Vincent Mailhol <mailhol@kernel.org> wrote:
>>
>> On 03/06/2026 at 00:34, Rosen Penev wrote:
>>> Mechanical conversion of the ppc4xx-specific in_8/out_8 accessors and
>>> the setbits8/clrbits8 macros to the generic ioread8/iowrite8 helpers
>>> for portability.
>>>
>>> Add HAS_IOMEM as these functions need it.
>>>
>>> Add COMPILE_TEST as a result. This can be built anywhere now.
>>>
>>> Assisted-by: opencode:big-pickle
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>
>> I left a comment for a potential extension of the patch, but it is ok as-is.
>>
>> Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
>>
>>> ---
>>>  drivers/net/can/mscan/Kconfig |   3 +-
>>>  drivers/net/can/mscan/mscan.c | 143 +++++++++++++++++-----------------
>>>  2 files changed, 73 insertions(+), 73 deletions(-)
>>>
>>> diff --git a/drivers/net/can/mscan/Kconfig b/drivers/net/can/mscan/Kconfig
>>> index dfe6bd9947bb..ef3a99b3d3db 100644
>>> --- a/drivers/net/can/mscan/Kconfig
>>> +++ b/drivers/net/can/mscan/Kconfig
>>> @@ -1,6 +1,7 @@
>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>  config CAN_MSCAN
>>> -     depends on PPC
>>> +     depends on PPC || COMPILE_TEST
>>> +     depends on HAS_IOMEM
>>
>> It seems to me that following your changes, it should also now become
>> easy to add COMPILE_TEST to config CAN_MPC5XXX.
> I'll look into this.
>>
>> mpc5xxx_can.c has a couple of unused headers, after removing those, I
>> could compile test it!
>>
>> These are my local changes:
>>
>> ----8<----
>> diff --git a/drivers/net/can/mscan/Kconfig b/drivers/net/can/mscan/Kconfig
>> index ef3a99b3d3db..9bffd91ea418 100644
>> --- a/drivers/net/can/mscan/Kconfig
>> +++ b/drivers/net/can/mscan/Kconfig
>> @@ -13,7 +13,7 @@ if CAN_MSCAN
>>
>>  config CAN_MPC5XXX
>>         tristate "Freescale MPC5xxx onboard CAN controller"
>> -       depends on (PPC_MPC52xx || PPC_MPC512x)
>> +       depends on PPC_MPC52xx || PPC_MPC512x || COMPILE_TEST
>>         help
>>           If you say yes here you get support for Freescale's MPC5xxx
>>           onboard CAN controller. Currently, the MPC5200, MPC5200B and
>> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
>> index 0080c39ee182..759efb71d843 100644
>> --- a/drivers/net/can/mscan/mpc5xxx_can.c
>> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
>> @@ -9,8 +9,6 @@
>>   */
>>
>>  #include <linux/kernel.h>
>> -#include <linux/module.h>
>> -#include <linux/interrupt.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/property.h>
>>  #include <linux/netdevice.h>
>> @@ -18,11 +16,6 @@
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>> -#include <linux/of_platform.h>
>> -#include <sysdev/fsl_soc.h>
>> -#include <linux/clk.h>
>> -#include <linux/io.h>
>> -#include <asm/mpc52xx.h>
>>
>>  #include "mscan.h"
>> ----8<----
>>
>> Et voilà ! Both CAN_MSCAN and CAN_MPC5XXX can now be compile tested!
> drivers/net/can/mscan/mpc5xxx_can.c:22:10: fatal error:
> 'sysdev/fsl_soc.h' file not found 22 | #include <sysdev/fsl_soc.h> |
> ^~~~~~~~~~~~~~~~~~
> 
> drivers/net/can/mscan/mpc5xxx_can.c:376:2: error: call to undeclared
> function '_memcpy_fromio'; ISO C99 and later do not support implicit
> function declarations [-Wimplicit-function-declaration] 376 |
> _memcpy_fromio(&saved_regs, regs, sizeof(*regs)); | ^
> 
> needs more work.

Ah! I only compiled the objects but didn't link them and thus missed
this error. Well, this is what I get for answering the mailing list in
the middle of the night.

Regardless, for PPC, memcpy_toio() and _memcpy_toio() are the exact
same function and memcpy_fromio() is implemented as:

	#ifdef CONFIG_EEH
	#define __do_memcpy_fromio(dst, src, n)	\
					eeh_memcpy_fromio(dst, src, n)
	#else /* CONFIG_EEH */
	#define __do_memcpy_fromio(dst, src, n)	\
					_memcpy_fromio(dst, src, n)
	#endif /* !CONFIG_EEH */
	
	static inline void memcpy_fromio(void *d, const volatile void __iomem *s, unsigned long n)
	{
		__do_memcpy_fromio(d, s, n);
	}

So also the same as _memcpy_fromio() if CONFIG_EEH is not selected. And
if CONFIG_EEH is selected, using _memcpy_fromio() instead of
eeh_memcpy_fromio() seems incorrect.

I think that the final nail in the curtain is that mpc5xxx is the only
driver in the full tree still using _memcpy_fromio() and
_memcpy_toio().

After replacing both with their memcpy_fromio() and memcpy_toio()
equivalent, I can now build and link without any problem on non
PPC architectures.


>>>       tristate "Support for Freescale MSCAN based chips"
>>>       help
>>>         The Motorola Scalable Controller Area Network (MSCAN) definition

 
Yours sincerely,
Vincent Mailhol