Re: [PATCH v1 1/2] soc: loongson: add GUTS driver for loongson2 platforms

[Arnd Bergmann]

On Mon, Oct 24, 2022, at 12:58, Yinbo Zhu wrote:
> The global utilities block controls PCIE device enabling, alternate
> function selection for multiplexed signals, consistency of HDA, USB
> and PCIE, configuration of memory controller, rtc controller, lio
> controller, and clock control.


> This patch adds a driver to manage and access global utilities block.
> Initially only reading SVR and registering soc device are supported.

Is this for MIPS, Loongarch or both?

It looks like the driver only deals with identifying the SoC, but
is at the moment unrelated to the other functionality you mention
in the changelog text.

Please explain in the text whether you plan to add more functionality
in this driver later, of if this is just meant to be set up by
bootloader/firmware.

> Signed-off-by: Yinbo Zhu <zhuyinbo@xxxxxxxxxxx>
> ---
>  MAINTAINERS                             |   7 +
>  drivers/soc/Kconfig                     |   1 +
>  drivers/soc/Makefile                    |   1 +
>  drivers/soc/loongson/Kconfig            |  17 +++
>  drivers/soc/loongson/Makefile           |   6 +
>  drivers/soc/loongson/loongson2_guts.c   | 168 ++++++++++++++++++++++++
>  include/linux/loongson/loongson2_guts.h |  35 +++++

Is there a need to include the header from another driver?
If not, just fold it into the driver file itself.

> +
> +menu "Loongson2 series SoC drivers"
> +
> +config LOONGSON2_GUTS
> +	tristate "LOONGSON2 GUTS"
> +	select SOC_BUS

Please limit this to to the appropriate targets like

        depends on MACH_LOONGSON64 || LOONGARCH || COMPILE_TEST

so this does not show up in 'make oldconfig' for users of
other architectures.

> +struct scfg_guts {
> +   u32 svr; /* Version Register */
> +   u8 res0[4];
> +   u16 feature; /* Feature Register */
> +   u32 vendor; /* Vendor Register */
> +   u8 res1[6];
> +   u32 id;
> +   u8 res2[0x3ff8 - 0x18];
> +   u32 chip;
> +} __packed;

This should not be marked as __packed, since MMIO
registers have to be accessed atomically while packed
registers have to be accessed byte-wise on CPUs without
unaligned access.

    Arnd