[PATCH v2,RESEND] MIPS: Scan the DMI system information

[PATCH v2,RESEND] MIPS: Scan the DMI system information

[Tiezhu Yang]

Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
(dmi_system_id) for printing it out on task dumps and prepares DIMM entry
information (dmi_memdev_info) from the SMBIOS table. With this patch, the
driver can easily match various of mainboards.

In the SMBIOS reference specification, the table anchor string "_SM_" is
present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
but there exists a special case for Loongson platform, when call function
dmi_early_remap, it should specify the start address to 0xFFFE000 due to
it is reserved for SMBIOS and can be normally access in the BIOS.

This patch works fine on the Loongson 3A3000 platform which belongs to
MIPS architecture and has no influence on the other architectures such
as x86 and ARM.

Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
[jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Reviewed-by: Huacai Chen <chenhc@lemote.com>
---

v2:
  - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
  - refine definitions and Kconfig by Jiaxun

 arch/mips/Kconfig           | 10 ++++++++++
 arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
 arch/mips/kernel/setup.c    |  2 ++
 drivers/firmware/dmi_scan.c |  6 +++++-
 4 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/include/asm/dmi.h

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 4b83507..c097f78 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2773,6 +2773,16 @@ config HW_PERF_EVENTS
 	  Enable hardware performance counter support for perf events. If
 	  disabled, perf events will use software events only.
 
+config DMI
+	default y if MACH_LOONGSON64
+	select DMI_SCAN_MACHINE_NON_EFI_FALLBACK
+	bool "Enable DMI scanning"
+	help
+	  Enabled scanning of DMI to identify machine quirks. Say Y
+	  here unless you have verified that your setup is not
+	  affected by entries in the DMI blacklist. Required by PNP
+	  BIOS code.
+
 config SMP
 	bool "Multi-Processing support"
 	depends on SYS_SUPPORTS_SMP
diff --git a/arch/mips/include/asm/dmi.h b/arch/mips/include/asm/dmi.h
new file mode 100644
index 0000000..27415a2
--- /dev/null
+++ b/arch/mips/include/asm/dmi.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_DMI_H
+#define _ASM_DMI_H
+
+#include <linux/io.h>
+#include <linux/memblock.h>
+
+#define dmi_early_remap(x, l)		ioremap_cache(x, l)
+#define dmi_early_unmap(x, l)		iounmap(x)
+#define dmi_remap(x, l)			ioremap_cache(x, l)
+#define dmi_unmap(x)			iounmap(x)
+
+/* MIPS initialize DMI scan before SLAB is ready, so we use memblock here */
+#define dmi_alloc(l)			memblock_alloc_low(l, PAGE_SIZE)
+
+#if defined(CONFIG_MACH_LOONGSON64)
+#define SMBIOS_ENTRY_POINT_SCAN_START	0xFFFE000
+#endif
+
+#endif /* _ASM_DMI_H */
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 701f4bc..d9bd841 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -28,6 +28,7 @@
 #include <linux/decompress/generic.h>
 #include <linux/of_fdt.h>
 #include <linux/of_reserved_mem.h>
+#include <linux/dmi.h>
 
 #include <asm/addrspace.h>
 #include <asm/bootinfo.h>
@@ -800,6 +801,7 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	arch_mem_init(cmdline_p);
+	dmi_setup();
 
 	resource_init();
 	plat_smp_setup();
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 2045566..f59163c 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -11,6 +11,10 @@
 #include <asm/dmi.h>
 #include <asm/unaligned.h>
 
+#ifndef SMBIOS_ENTRY_POINT_SCAN_START
+#define SMBIOS_ENTRY_POINT_SCAN_START 0xF0000
+#endif
+
 struct kobject *dmi_kobj;
 EXPORT_SYMBOL_GPL(dmi_kobj);
 
@@ -663,7 +667,7 @@ static void __init dmi_scan_machine(void)
 			return;
 		}
 	} else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
-		p = dmi_early_remap(0xF0000, 0x10000);
+		p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
 		if (p == NULL)
 			goto error;
 
-- 
2.1.0

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Tiezhu Yang]

On 1/16/20 9:26 PM, Tiezhu Yang wrote:
> Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
> (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
> information (dmi_memdev_info) from the SMBIOS table. With this patch, the
> driver can easily match various of mainboards.
>
> In the SMBIOS reference specification, the table anchor string "_SM_" is
> present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
> but there exists a special case for Loongson platform, when call function
> dmi_early_remap, it should specify the start address to 0xFFFE000 due to
> it is reserved for SMBIOS and can be normally access in the BIOS.
>
> This patch works fine on the Loongson 3A3000 platform which belongs to
> MIPS architecture and has no influence on the other architectures such
> as x86 and ARM.
>
> Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
> Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
> [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> Reviewed-by: Huacai Chen <chenhc@lemote.com>
> ---
>
> v2:
>    - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
>    - refine definitions and Kconfig by Jiaxun
>
>   arch/mips/Kconfig           | 10 ++++++++++
>   arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
>   arch/mips/kernel/setup.c    |  2 ++
>   drivers/firmware/dmi_scan.c |  6 +++++-
>   4 files changed, 37 insertions(+), 1 deletion(-)
>   create mode 100644 arch/mips/include/asm/dmi.h


Hi Paul and Jean,

How do you think this patch?

Should I split it into the following two patches?
[PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
[PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)

The first patch is only related with the common dmi code
drivers/firmware/dmi_scan.c, the other patch is only related
with the mips code under arch/mips.

If you have any questions or suggestions, please let me know.
I am looking forward to your early reply.

Thanks,

Tiezhu Yang


>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 4b83507..c097f78 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2773,6 +2773,16 @@ config HW_PERF_EVENTS
>   	  Enable hardware performance counter support for perf events. If
>   	  disabled, perf events will use software events only.
>   
> +config DMI
> +	default y if MACH_LOONGSON64
> +	select DMI_SCAN_MACHINE_NON_EFI_FALLBACK
> +	bool "Enable DMI scanning"
> +	help
> +	  Enabled scanning of DMI to identify machine quirks. Say Y
> +	  here unless you have verified that your setup is not
> +	  affected by entries in the DMI blacklist. Required by PNP
> +	  BIOS code.
> +
>   config SMP
>   	bool "Multi-Processing support"
>   	depends on SYS_SUPPORTS_SMP
> diff --git a/arch/mips/include/asm/dmi.h b/arch/mips/include/asm/dmi.h
> new file mode 100644
> index 0000000..27415a2
> --- /dev/null
> +++ b/arch/mips/include/asm/dmi.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_DMI_H
> +#define _ASM_DMI_H
> +
> +#include <linux/io.h>
> +#include <linux/memblock.h>
> +
> +#define dmi_early_remap(x, l)		ioremap_cache(x, l)
> +#define dmi_early_unmap(x, l)		iounmap(x)
> +#define dmi_remap(x, l)			ioremap_cache(x, l)
> +#define dmi_unmap(x)			iounmap(x)
> +
> +/* MIPS initialize DMI scan before SLAB is ready, so we use memblock here */
> +#define dmi_alloc(l)			memblock_alloc_low(l, PAGE_SIZE)
> +
> +#if defined(CONFIG_MACH_LOONGSON64)
> +#define SMBIOS_ENTRY_POINT_SCAN_START	0xFFFE000
> +#endif
> +
> +#endif /* _ASM_DMI_H */
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 701f4bc..d9bd841 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -28,6 +28,7 @@
>   #include <linux/decompress/generic.h>
>   #include <linux/of_fdt.h>
>   #include <linux/of_reserved_mem.h>
> +#include <linux/dmi.h>
>   
>   #include <asm/addrspace.h>
>   #include <asm/bootinfo.h>
> @@ -800,6 +801,7 @@ void __init setup_arch(char **cmdline_p)
>   #endif
>   
>   	arch_mem_init(cmdline_p);
> +	dmi_setup();
>   
>   	resource_init();
>   	plat_smp_setup();
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 2045566..f59163c 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -11,6 +11,10 @@
>   #include <asm/dmi.h>
>   #include <asm/unaligned.h>
>   
> +#ifndef SMBIOS_ENTRY_POINT_SCAN_START
> +#define SMBIOS_ENTRY_POINT_SCAN_START 0xF0000
> +#endif
> +
>   struct kobject *dmi_kobj;
>   EXPORT_SYMBOL_GPL(dmi_kobj);
>   
> @@ -663,7 +667,7 @@ static void __init dmi_scan_machine(void)
>   			return;
>   		}
>   	} else if (IS_ENABLED(CONFIG_DMI_SCAN_MACHINE_NON_EFI_FALLBACK)) {
> -		p = dmi_early_remap(0xF0000, 0x10000);
> +		p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
>   		if (p == NULL)
>   			goto error;
>   

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Jean Delvare]

Hi Tiezhun,

On Mon, 3 Feb 2020 16:32:03 +0800, Tiezhu Yang wrote:
> On 1/16/20 9:26 PM, Tiezhu Yang wrote:
> > Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
> > (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
> > information (dmi_memdev_info) from the SMBIOS table. With this patch, the
> > driver can easily match various of mainboards.
> >
> > In the SMBIOS reference specification, the table anchor string "_SM_" is
> > present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
> > but there exists a special case for Loongson platform, when call function
> > dmi_early_remap, it should specify the start address to 0xFFFE000 due to
> > it is reserved for SMBIOS and can be normally access in the BIOS.
> >
> > This patch works fine on the Loongson 3A3000 platform which belongs to
> > MIPS architecture and has no influence on the other architectures such
> > as x86 and ARM.
> >
> > Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
> > Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
> > [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
> > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > Reviewed-by: Huacai Chen <chenhc@lemote.com>
> > ---
> >
> > v2:
> >    - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
> >    - refine definitions and Kconfig by Jiaxun
> >
> >   arch/mips/Kconfig           | 10 ++++++++++
> >   arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
> >   arch/mips/kernel/setup.c    |  2 ++
> >   drivers/firmware/dmi_scan.c |  6 +++++-
> >   4 files changed, 37 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/mips/include/asm/dmi.h  
> 
> 
> Hi Paul and Jean,
> 
> How do you think this patch?

Looks good to me and you can add:

Reviewed-by: Jean Delvare <jdelvare@suse.de>

for the dmi subsystem part.

> 
> Should I split it into the following two patches?
> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)
> 
> The first patch is only related with the common dmi code
> drivers/firmware/dmi_scan.c, the other patch is only related
> with the mips code under arch/mips.
> 
> If you have any questions or suggestions, please let me know.
> I am looking forward to your early reply.

I'm fine either way. I you do not split it, as most changes are in the
mips arch files and I do not expect any conflict in the dmi subsystem
part, I believe that the patch should be merged by the mips arch
maintainer.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Jonas Gorski]

Hi,

On Thu, 16 Jan 2020 at 14:28, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
> (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
> information (dmi_memdev_info) from the SMBIOS table. With this patch, the
> driver can easily match various of mainboards.
>
> In the SMBIOS reference specification, the table anchor string "_SM_" is
> present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
> but there exists a special case for Loongson platform, when call function
> dmi_early_remap, it should specify the start address to 0xFFFE000 due to
> it is reserved for SMBIOS and can be normally access in the BIOS.
>
> This patch works fine on the Loongson 3A3000 platform which belongs to
> MIPS architecture and has no influence on the other architectures such
> as x86 and ARM.
>
> Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
> Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
> [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> Reviewed-by: Huacai Chen <chenhc@lemote.com>
> ---
>
> v2:
>   - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
>   - refine definitions and Kconfig by Jiaxun
>
>  arch/mips/Kconfig           | 10 ++++++++++
>  arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
>  arch/mips/kernel/setup.c    |  2 ++
>  drivers/firmware/dmi_scan.c |  6 +++++-
>  4 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100644 arch/mips/include/asm/dmi.h
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 4b83507..c097f78 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2773,6 +2773,16 @@ config HW_PERF_EVENTS
>           Enable hardware performance counter support for perf events. If
>           disabled, perf events will use software events only.
>
> +config DMI
> +       default y if MACH_LOONGSON64
> +       select DMI_SCAN_MACHINE_NON_EFI_FALLBACK
> +       bool "Enable DMI scanning"

Is this option harmless to enable for other MIPS platforms? What
happens if it is enabled on devices where PHYS_OFFSET isn't 0, so
0xF0000 is likely not backed by anything and accessing it might hang
the system?

This probably should depend on MACH_LOONGSON64.

Regards

Jonas

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Tiezhu Yang]

On 2/3/20 10:46 PM, Jonas Gorski wrote:
> Hi,
>
> On Thu, 16 Jan 2020 at 14:28, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>> Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
>> (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
>> information (dmi_memdev_info) from the SMBIOS table. With this patch, the
>> driver can easily match various of mainboards.
>>
>> In the SMBIOS reference specification, the table anchor string "_SM_" is
>> present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
>> but there exists a special case for Loongson platform, when call function
>> dmi_early_remap, it should specify the start address to 0xFFFE000 due to
>> it is reserved for SMBIOS and can be normally access in the BIOS.
>>
>> This patch works fine on the Loongson 3A3000 platform which belongs to
>> MIPS architecture and has no influence on the other architectures such
>> as x86 and ARM.
>>
>> Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
>> Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
>> [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> Reviewed-by: Huacai Chen <chenhc@lemote.com>
>> ---
>>
>> v2:
>>    - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
>>    - refine definitions and Kconfig by Jiaxun
>>
>>   arch/mips/Kconfig           | 10 ++++++++++
>>   arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
>>   arch/mips/kernel/setup.c    |  2 ++
>>   drivers/firmware/dmi_scan.c |  6 +++++-
>>   4 files changed, 37 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/mips/include/asm/dmi.h
>>
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index 4b83507..c097f78 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -2773,6 +2773,16 @@ config HW_PERF_EVENTS
>>            Enable hardware performance counter support for perf events. If
>>            disabled, perf events will use software events only.
>>
>> +config DMI
>> +       default y if MACH_LOONGSON64
>> +       select DMI_SCAN_MACHINE_NON_EFI_FALLBACK
>> +       bool "Enable DMI scanning"
> Is this option harmless to enable for other MIPS platforms? What
> happens if it is enabled on devices where PHYS_OFFSET isn't 0, so
> 0xF0000 is likely not backed by anything and accessing it might hang
> the system?
>
> This probably should depend on MACH_LOONGSON64.


Hi Jonas,

Thanks for your reply and suggestion.

With the current patch, on the MIPS architecture, the DMI option is
default y only if MACH_LOONGSON64 is set, otherwise it is not set.
When enable DMI manually on the MIPS platform which is not LOONGSON64,
the SMBIOS_ENTRY_POINT_SCAN_START is 0xF0000, dmi_early_remap() will
return NULL and goto err to print "DMI not present or invalid."

static void __init dmi_scan_machine(void)
{
...
         p = dmi_early_remap(SMBIOS_ENTRY_POINT_SCAN_START, 0x10000);
         if (p == NULL)
             goto error;
...
  error:
     pr_info("DMI not present or invalid.\n");
}

It seems harmless and the system can boot successful. However, in order
to avoid the unknown risks on the mips platform which is not LOONGSON64,
I think it is better to depend on MACH_LOONGSON64.

config DMI
     bool "Enable DMI scanning"
     depends on MACH_LOONGSON64
     select DMI_SCAN_MACHINE_NON_EFI_FALLBACK
     default y

If other mips platform also needs this DMI feature in the future,
the "depends on" conditon can be modified.

I will do it in the v3 patch.


Thanks,


Tiezhu Yang


>
> Regards
>
> Jonas

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Tiezhu Yang]

On 2/3/20 8:14 PM, Jean Delvare wrote:
> Hi Tiezhun,
>
> On Mon, 3 Feb 2020 16:32:03 +0800, Tiezhu Yang wrote:
>> On 1/16/20 9:26 PM, Tiezhu Yang wrote:
>>> Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
>>> (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
>>> information (dmi_memdev_info) from the SMBIOS table. With this patch, the
>>> driver can easily match various of mainboards.
>>>
>>> In the SMBIOS reference specification, the table anchor string "_SM_" is
>>> present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
>>> but there exists a special case for Loongson platform, when call function
>>> dmi_early_remap, it should specify the start address to 0xFFFE000 due to
>>> it is reserved for SMBIOS and can be normally access in the BIOS.
>>>
>>> This patch works fine on the Loongson 3A3000 platform which belongs to
>>> MIPS architecture and has no influence on the other architectures such
>>> as x86 and ARM.
>>>
>>> Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
>>> Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
>>> [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>> Reviewed-by: Huacai Chen <chenhc@lemote.com>
>>> ---
>>>
>>> v2:
>>>     - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
>>>     - refine definitions and Kconfig by Jiaxun
>>>
>>>    arch/mips/Kconfig           | 10 ++++++++++
>>>    arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
>>>    arch/mips/kernel/setup.c    |  2 ++
>>>    drivers/firmware/dmi_scan.c |  6 +++++-
>>>    4 files changed, 37 insertions(+), 1 deletion(-)
>>>    create mode 100644 arch/mips/include/asm/dmi.h
>>
>> Hi Paul and Jean,
>>
>> How do you think this patch?
> Looks good to me and you can add:
>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>
>
> for the dmi subsystem part.
>
>> Should I split it into the following two patches?
>> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
>> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)
>>
>> The first patch is only related with the common dmi code
>> drivers/firmware/dmi_scan.c, the other patch is only related
>> with the mips code under arch/mips.
>>
>> If you have any questions or suggestions, please let me know.
>> I am looking forward to your early reply.
> I'm fine either way. I you do not split it, as most changes are in the
> mips arch files and I do not expect any conflict in the dmi subsystem
> part, I believe that the patch should be merged by the mips arch
> maintainer.


Hi Jean,


Thanks very much for your review.


As described in another mail [1] by MIPS maintainer Paul Burton:

"So if unmerged arch/mips/ patches are holding you up, ping me, but
preferrably make sure code being added actually belongs under arch/mips/
first."

I think it is better to split it into the following two patches?
[PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
[PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)


I will send v3 patch as soon as possible.


Thanks,


Tiezhu Yang


[1] 
https://lore.kernel.org/linux-mips/20190208200852.wcywd7yfcq7zwzve@pburton-laptop/


>
> Thanks,

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Jiaxun Yang]

> On 2/3/20 8:14 PM, Jean Delvare wrote:
> 
>> Hi Tiezhun,
>>
>> On Mon, 3 Feb 2020 16:32:03 +0800, Tiezhu Yang wrote:
>>
>>> On 1/16/20 9:26 PM, Tiezhu Yang wrote:
>>>
>>>> Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
>>>> (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
>>>> information (dmi_memdev_info) from the SMBIOS table. With this patch, the
>>>> driver can easily match various of mainboards.
>>>>
>>>> In the SMBIOS reference specification, the table anchor string "_SM_" is
>>>> present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
>>>> but there exists a special case for Loongson platform, when call function
>>>> dmi_early_remap, it should specify the start address to 0xFFFE000 due to
>>>> it is reserved for SMBIOS and can be normally access in the BIOS.
>>>>
>>>> This patch works fine on the Loongson 3A3000 platform which belongs to
>>>> MIPS architecture and has no influence on the other architectures such
>>>> as x86 and ARM.
>>>>
>>>> Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
>>>> Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
>>>> [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>>> Reviewed-by: Huacai Chen <chenhc@lemote.com>
>>>> ---
>>>>
>>>> v2:
>>>> - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
>>>> - refine definitions and Kconfig by Jiaxun
>>>>
>>>> arch/mips/Kconfig | 10 ++++++++++
>>>> arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
>>>> arch/mips/kernel/setup.c | 2 ++
>>>> drivers/firmware/dmi_scan.c | 6 +++++-
>>>> 4 files changed, 37 insertions(+), 1 deletion(-)
>>>> create mode 100644 arch/mips/include/asm/dmi.h
>>>
>>> Hi Paul and Jean,
>>>
>>> How do you think this patch?
>>
>> Looks good to me and you can add:
>>
>> Reviewed-by: Jean Delvare <jdelvare@suse.de>
>>
>> for the dmi subsystem part.
>>
>>> Should I split it into the following two patches?
>>> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
>>> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)
>>>
>>> The first patch is only related with the common dmi code
>>> drivers/firmware/dmi_scan.c, the other patch is only related
>>> with the mips code under arch/mips.
>>>
>>> If you have any questions or suggestions, please let me know.
>>> I am looking forward to your early reply.
>>
>> I'm fine either way. I you do not split it, as most changes are in the
>> mips arch files and I do not expect any conflict in the dmi subsystem
>> part, I believe that the patch should be merged by the mips arch
>> maintainer.
> 
> Hi Jean,
> 
> Thanks very much for your review.
> 
> As described in another mail [1] by MIPS maintainer Paul Burton:
> 
> "So if unmerged arch/mips/ patches are holding you up, ping me, but
> preferrably make sure code being added actually belongs under arch/mips/
> first."
> 
> I think it is better to split it into the following two patches?
> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)

Hi Tiezhu,

That way will break bisect. In this case, I think the patch should still go through MIPS
tree after grab review or ack from DMI maintainer.

There is a grey margin for tree-wide patches, we should discuss how to deal with them case by
case, for the email you mentioned, the main focus is platform driver. 

Thanks.

--

Jiaxun Yang

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Tiezhu Yang]

On 2/5/20 10:55 AM, Jiaxun Yang wrote:
>
>> On 2/3/20 8:14 PM, Jean Delvare wrote:
>>
>>> Hi Tiezhun,
>>>
>>> On Mon, 3 Feb 2020 16:32:03 +0800, Tiezhu Yang wrote:
>>>
>>>> On 1/16/20 9:26 PM, Tiezhu Yang wrote:
>>>>
>>>>> Enable DMI scanning on the MIPS architecture, this setups DMI identifiers
>>>>> (dmi_system_id) for printing it out on task dumps and prepares DIMM entry
>>>>> information (dmi_memdev_info) from the SMBIOS table. With this patch, the
>>>>> driver can easily match various of mainboards.
>>>>>
>>>>> In the SMBIOS reference specification, the table anchor string "_SM_" is
>>>>> present in the address range 0xF0000 to 0xFFFFF on a 16-byte boundary,
>>>>> but there exists a special case for Loongson platform, when call function
>>>>> dmi_early_remap, it should specify the start address to 0xFFFE000 due to
>>>>> it is reserved for SMBIOS and can be normally access in the BIOS.
>>>>>
>>>>> This patch works fine on the Loongson 3A3000 platform which belongs to
>>>>> MIPS architecture and has no influence on the other architectures such
>>>>> as x86 and ARM.
>>>>>
>>>>> Co-developed-by: Yinglu Yang <yangyinglu@loongson.cn>
>>>>> Signed-off-by: Yinglu Yang <yangyinglu@loongson.cn>
>>>>> [jiaxun.yang@flygoat.com: Refine definitions and Kconfig]
>>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>>>> Reviewed-by: Huacai Chen <chenhc@lemote.com>
>>>>> ---
>>>>>
>>>>> v2:
>>>>> - add SMBIOS_ENTRY_POINT_SCAN_START suggested by Jean
>>>>> - refine definitions and Kconfig by Jiaxun
>>>>>
>>>>> arch/mips/Kconfig | 10 ++++++++++
>>>>> arch/mips/include/asm/dmi.h | 20 ++++++++++++++++++++
>>>>> arch/mips/kernel/setup.c | 2 ++
>>>>> drivers/firmware/dmi_scan.c | 6 +++++-
>>>>> 4 files changed, 37 insertions(+), 1 deletion(-)
>>>>> create mode 100644 arch/mips/include/asm/dmi.h
>>>> Hi Paul and Jean,
>>>>
>>>> How do you think this patch?
>>> Looks good to me and you can add:
>>>
>>> Reviewed-by: Jean Delvare <jdelvare@suse.de>
>>>
>>> for the dmi subsystem part.
>>>
>>>> Should I split it into the following two patches?
>>>> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
>>>> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)
>>>>
>>>> The first patch is only related with the common dmi code
>>>> drivers/firmware/dmi_scan.c, the other patch is only related
>>>> with the mips code under arch/mips.
>>>>
>>>> If you have any questions or suggestions, please let me know.
>>>> I am looking forward to your early reply.
>>> I'm fine either way. I you do not split it, as most changes are in the
>>> mips arch files and I do not expect any conflict in the dmi subsystem
>>> part, I believe that the patch should be merged by the mips arch
>>> maintainer.
>> Hi Jean,
>>
>> Thanks very much for your review.
>>
>> As described in another mail [1] by MIPS maintainer Paul Burton:
>>
>> "So if unmerged arch/mips/ patches are holding you up, ping me, but
>> preferrably make sure code being added actually belongs under arch/mips/
>> first."
>>
>> I think it is better to split it into the following two patches?
>> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
>> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)
> Hi Tiezhu,
>
> That way will break bisect. In this case, I think the patch should still go through MIPS
> tree after grab review or ack from DMI maintainer.
>
> There is a grey margin for tree-wide patches, we should discuss how to deal with them case by
> case, for the email you mentioned, the main focus is platform driver.


Hi Jiaxun,


Sorry for the late reply.

I just saw your email a moment ago, and already sent the split v3 patch.

https://lore.kernel.org/patchwork/patch/1189713/

https://lore.kernel.org/patchwork/patch/1189714/

Let us wait and hear the opinion of Paul.


Hi Paul,

How do you think this case? One patch or two patches?

If one patch is better, I will send a v4 patch to make MIPS DMI config

depend on MACH_LOONGSON64.


Hi Jean,

Please do not merge the following patch until  we hear the opinion of Paul:

[PATCH v3,1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START

https://lore.kernel.org/patchwork/patch/1189713/


Thanks,


Tiezhu Yang


>
> Thanks.
>
> --
>
> Jiaxun Yang

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Jean Delvare]

On Wed, 05 Feb 2020 10:55:38 +0800, Jiaxun Yang wrote:
> > I think it is better to split it into the following two patches?
> > [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
> > [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)  
> 
> That way will break bisect.

Are you sure? As far as I can see, each patch builds individually. The
dmi patch is a no-op alone. The mips patch will not work alone,
obviously, however according to Tiezhu dmi_scan_machine() will fail
with a harmless error message if the base address is 0xF0000. If that's
correct then it's not breaking bisect.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2,RESEND] MIPS: Scan the DMI system information

[Jiaxun Yang]

> On Wed, 05 Feb 2020 10:55:38 +0800, Jiaxun Yang wrote:
> 
>>> I think it is better to split it into the following two patches?
>>> [PATCH v3 1/2] firmware: dmi: Add macro SMBIOS_ENTRY_POINT_SCAN_START
>>> [PATCH v3 2/2] MIPS: Add support for Desktop Management Interface (DMI)
>>
>> That way will break bisect.
> 
> Are you sure? As far as I can see, each patch builds individually. The
> dmi patch is a no-op alone. The mips patch will not work alone,
> obviously, however according to Tiezhu dmi_scan_machine() will fail
> with a harmless error message if the base address is 0xF0000. If that's
> correct then it's not breaking bisect.

Sorry, I even forgot that it's my modification :-)
Just don't want to trouble maintainers so much.

> 
> --
> Jean Delvare
> SUSE L3 Support
--
Jiaxun Yang