Re: [PATCH v2 1/3] MIPS: Loongson: Get host bridge information

[Tiezhu Yang]

On 03/24/2020 06:02 PM, Jiaxun Yang wrote:
> ä 2020å3æ24æ GMT+08:00 äå5:58:33, Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> åå:
> > On 03/24/2020 03:36 PM, Jiaxun Yang wrote:
> > > ä 2020å3æ23æ GMT+08:00 äå10:59:13, Tiezhu Yang
> > <yangtiezhu@xxxxxxxxxxx> åå:
> > > > Read the address of HyperTransport Configuration Space to get the
> > > > vendor ID
> > > > and device ID of bridge chip, and then we can distinguish various
> > types
> > > > of
> > > > bridge chip such as Loongson 7A1000 or AMD RS780E.
> > > >
> > > > Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> > > > ---
> > > > arch/mips/include/asm/mach-loongson64/boot_param.h |  6 ++++++
> > > > arch/mips/loongson64/env.c                         | 16
> > > > ++++++++++++++++
> > > > 2 files changed, 22 insertions(+)
> > > >
> > > > diff --git a/arch/mips/include/asm/mach-loongson64/boot_param.h
> > > > b/arch/mips/include/asm/mach-loongson64/boot_param.h
> > > > index 8c286be..5e8c70d 100644
> > > > --- a/arch/mips/include/asm/mach-loongson64/boot_param.h
> > > > +++ b/arch/mips/include/asm/mach-loongson64/boot_param.h
> > > > @@ -190,6 +190,11 @@ struct boot_params {
> > > > 	struct efi_reset_system_t reset_system;
> > > > };
> > > >
> > > > +enum loongson_bridge_type {
> > > > +	RS780E = 1,
> > > > +	LS7A1000 = 2
> > > > +};
> > > > +
> > > > struct loongson_system_configuration {
> > > > 	u32 nr_cpus;
> > > > 	u32 nr_nodes;
> > > > @@ -198,6 +203,7 @@ struct loongson_system_configuration {
> > > > 	u16 boot_cpu_id;
> > > > 	u16 reserved_cpus_mask;
> > > > 	enum loongson_cpu_type cputype;
> > > > +	enum loongson_bridge_type bridgetype;
> > > > 	u64 ht_control_base;
> > > > 	u64 pci_mem_start_addr;
> > > > 	u64 pci_mem_end_addr;
> > > > diff --git a/arch/mips/loongson64/env.c b/arch/mips/loongson64/env.c
> > > > index 0daeb7b..42542c7 100644
> > > > --- a/arch/mips/loongson64/env.c
> > > > +++ b/arch/mips/loongson64/env.c
> > > > @@ -19,6 +19,8 @@
> > > > #include <boot_param.h>
> > > > #include <workarounds.h>
> > > >
> > > > +#define HT1_LO_BUS_CONFIG_BASE	0x90000efdfe000000
> > > > +
> > > > u32 cpu_clock_freq;
> > > > EXPORT_SYMBOL(cpu_clock_freq);
> > > > struct efi_memory_map_loongson *loongson_memmap;
> > > > @@ -42,6 +44,8 @@ void __init prom_init_env(void)
> > > > 	struct system_loongson *esys;
> > > > 	struct efi_cpuinfo_loongson *ecpu;
> > > > 	struct irq_source_routing_table *eirq_source;
> > > > +	u32 id;
> > > > +	u16 vendor, device;
> > > >
> > > > 	/* firmware arguments are initialized in head.S */
> > > > 	boot_p = (struct boot_params *)fw_arg2;
> > > > @@ -155,4 +159,16 @@ void __init prom_init_env(void)
> > > > 		memcpy(loongson_sysconf.sensors, esys->sensors,
> > > > 			sizeof(struct sensor_device) * loongson_sysconf.nr_sensors);
> > > > 	pr_info("CpuClock = %u\n", cpu_clock_freq);
> > > > +
> > > > +	id = readl((u32 *)HT1_LO_BUS_CONFIG_BASE);
> > > Is that pointer cast required?
> > Yes, otherwise it will compile failed:
> > arch/mips/loongson64/env.c:163:13: error: passing argument 1 of âreadlâ
> I don't like it through....
> Can we cast it in macro?
> At least it should be void __iomem *.

OK, I will modify the code to make it easy to read.

> > makes pointer from integer without a cast [-Werror]
> >    id = readl(HT1_LO_BUS_CONFIG_BASE);
> >               ^
> > > And you'd better add a comment here like
> > > /* Read HT PCI 00:00:00 Host Bridge's ID to detect bridge model */
> > > Otherwise it's hard for others to understand what you are doing.
> > OK, I will do it.
> >
> > > > +	vendor = id & 0xffff;
> > > > +	device = (id >> 16) & 0xffff;
> > > > +
> > > > +	if (vendor == 0x0014 && device == 0x7a00) {
> > > Please confirm with hardware design guys that bridge chip in future
> > will have different dev ID.
> >
> > Yes, it maybe have a new device ID for the new bridge chip.
> >
> > > Thanks.
> > >
> > > > +		pr_info("The bridge chip is Loongson 7A1000\n");
> > > > +		loongson_sysconf.bridgetype = LS7A1000;
> > > > +	} else {
> > > > +		pr_info("The bridge chip is AMD RS780E or SR5690\n");
> > > > +		loongson_sysconf.bridgetype = RS780E;
> > > > +	}
> > > > }