Re: [PATCH v3 3/4] LoongArch: Extract IOCSR definitions to standalone header

[Jiaxun Yang]

在2024年9月14日九月 上午9:56，maobibo写道：
> Hi jiaxun,
Hi Bibo,

Thanks for your comments.

[...]
>> +
>> +#define LOONGSON_IOCSR_FEATURES	0x8
>> +#define  IOCSRF_TEMP			BIT_ULL(0)
>> +#define  IOCSRF_NODECNT			BIT_ULL(1)
>> +#define  IOCSRF_MSI			BIT_ULL(2)
>> +#define  IOCSRF_EXTIOI			BIT_ULL(3)
>> +#define  IOCSRF_CSRIPI			BIT_ULL(4)
>> +#define  IOCSRF_FREQCSR			BIT_ULL(5)
>> +#define  IOCSRF_FREQSCALE		BIT_ULL(6)
>> +#define  IOCSRF_DVFSV1			BIT_ULL(7)
>> +#define  IOCSRF_EIODECODE		BIT_ULL(9)
>> +#define  IOCSRF_FLATMODE		BIT_ULL(10)
>> +#define  IOCSRF_VM			BIT_ULL(11)
>> +#define  IOCSRF_AVEC			BIT_ULL(15)
> Is these definiton the same between Loongson3 mips and LoongArch machine 
> such as IOCSRF_FLATMODE/IOCSRF_AVEC?

Those bits are only defined on LoongArch based loongson3, and will be zero
on MIPS loongson3 systems. Zero means hardware doesn't have such feature,
this is still semantically correct.

Given that there will be no more future MIPS loongson3 it makes sense to
just define them in common header.

>
[...]
>> +
>> +/* PerCore CSR, only accessible by local cores */
>> +#define LOONGSON_IOCSR_IPI_STATUS	0x1000
>> +#define LOONGSON_IOCSR_IPI_EN		0x1004
>> +#define LOONGSON_IOCSR_IPI_SET		0x1008
>> +#define LOONGSON_IOCSR_IPI_CLEAR	0x100c
>> +#define LOONGSON_IOCSR_MBUF0		0x1020
>> +#define LOONGSON_IOCSR_MBUF1		0x1028
>> +#define LOONGSON_IOCSR_MBUF2		0x1030
>> +#define LOONGSON_IOCSR_MBUF3		0x1038
> It seems that it is only used with arch/loongarch/kernel/smp.c,
> and file smp.c is arch specific. No generic driver uses this.

Yes, but arch/mips/loongson64/smp.c are using those bits as well,
and will be converted in next patch.

Just FYI, I'm now trying to implement a common SMP IPI driver for
MIPS/LoongArch Loongson chips, there is a preview [1].

>
>> +
[...]
>> +
>> +/* Register offset and bit definition for CSR access */
>> +#define LOONGSON_IOCSR_TIMER_CFG       0x1060
>> +#define LOONGSON_IOCSR_TIMER_TICK      0x1070
>> +#define  IOCSR_TIMER_CFG_RESERVED       (_ULCAST_(1) << 63)
>> +#define  IOCSR_TIMER_CFG_PERIODIC       (_ULCAST_(1) << 62)
>> +#define  IOCSR_TIMER_CFG_EN             (_ULCAST_(1) << 61)
>> +#define  IOCSR_TIMER_MASK		0x0ffffffffffffULL
>> +#define  IOCSR_TIMER_INITVAL_RST        (_ULCAST_(0xffff) << 48)
> I do not find any use about IOCSR_TIMER macro, which does ip driver use 
> this?

So IOCSR timer is obsoleted by LoongArch's architecture timer, but still
present on at least 3A5000 for compatibility reason. We may write a driver
for it at some point.

It is common practice that hardware headers contains those bits that are not used
by drivers yet to serve as a document to the hardware.

>
>> +
>> +#define LOONGSON_IOCSR_EXTIOI_NODEMAP_BASE	0x14a0
>> +#define LOONGSON_IOCSR_EXTIOI_IPMAP_BASE	0x14c0
>> +#define LOONGSON_IOCSR_EXTIOI_EN_BASE		0x1600
>> +#define LOONGSON_IOCSR_EXTIOI_BOUNCE_BASE	0x1680
>> +#define LOONGSON_IOCSR_EXTIOI_ISR_BASE		0x1800
>> +#define LOONGSON_IOCSR_EXTIOI_ROUTE_BASE	0x1c00
>> +#define IOCSR_EXTIOI_VECTOR_NUM			256
> Is it better to define these macro in common header file or in extioi 
> driver?  It seems that only extioi use it.

Maybe :-) This is a little bit out of scope of this series but I can try
later.

Thanks
[...]

[1]: https://github.com/FlyGoat/linux/commit/0a5ee611529d359b4d22eaae22c2b9835c63c63f

-- 
- Jiaxun