Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2 1/3] MIPS: Loongson: Get host bridge information
To:     Jiaxun Yang <jiaxun.yang@flygoat.com>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Huacai Chen <chenhc@lemote.com>
References: <1584932355-3642-1-git-send-email-yangtiezhu@loongson.cn>
 <1584932355-3642-2-git-send-email-yangtiezhu@loongson.cn>
 <C251FD7C-87F0-454E-83D7-5198A6E060E2@flygoat.com>
Cc:     linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
        Xuefeng Li <lixuefeng@loongson.cn>
From:   Tiezhu Yang <yangtiezhu@loongson.cn>
Message-ID: <c8702862-f9a7-4776-68b9-cc17dba5d6f6@loongson.cn>
Date:   Tue, 24 Mar 2020 17:58:33 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <C251FD7C-87F0-454E-83D7-5198A6E060E2@flygoat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 03/24/2020 03:36 PM, Jiaxun Yang wrote:
>
> 于 2020年3月23日 GMT+08:00 上午10:59:13, Tiezhu Yang <yangtiezhu@loongson.cn> 写到:
>> 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@loongson.cn>
>> ---
>> 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’ 
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;
>> +	}
>> }