linux-kernel - Re: [PATCH v1 1/1] soc/aspeed: Add host side BMC device driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6cea8ee7-f845-6ef3-631f-3f252ff5e949@linux.vnet.ibm.com>
Date:   Tue, 22 Aug 2023 11:14:15 -0500
From:   Ninad Palsule <ninad@...ux.vnet.ibm.com>
To:     Andrew Lunn <andrew@...n.ch>, Ninad Palsule <ninad@...ux.ibm.com>
Cc:     joel@....id.au, andrew@...id.au, eajames@...ux.ibm.com,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-aspeed@...ts.ozlabs.org
Subject: Re: [PATCH v1 1/1] soc/aspeed: Add host side BMC device driver

Hello Andrew,

Thanks for the review.

On 8/21/23 2:29 PM, Andrew Lunn wrote:
>> Testing:
>>    - This is tested on IBM rainier system with BMC. It requires BMC side
>>      BMC device driver which is available in the ASPEED's 5.15 SDK
>>      kernel.
> How relevant is that? To the host side, it just appears to be an
> 16550A. Is the SDK emulating an 16550A? If you where to use a
> different kernel, is it still guaranteed to be an 16550A? I also
> notice there is a mainline
> drivers/tty/serial/8250/8250_aspeed_vuart.c. Could that be used on the
> BMC? That would be a better testing target than the vendor kernel.

This is just to indicate how I tested my code.

Yes, aspeed chip (in this case ast2600) is compatible with 16550 UART.

I am guessing it should work with different kernel too as 16550 standard 
is used.

The 8250_aspeed_vuart.c is a BMC side driver for accessing VUART over 
LPC bus and

this is a host side driver to access VUART over PCIe bus.

>> +config ASPEED_HOST_BMC_DEV
>> +	bool "ASPEED SoC Host BMC device driver"
>> +	default ARCH_ASPEED
>> +	select SOC_BUS
>> +	default ARCH_ASPEED
> same default twice?
Removed.
>
>> +static int __init aspeed_host_bmc_device_init(void)
>> +{
>> +	int ret;
>> +
>> +	/* register pci driver */
>> +	ret = pci_register_driver(&aspeed_host_bmc_dev_driver);
>> +	if (ret < 0) {
>> +		pr_err("pci-driver: can't register pci driver\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +
>> +}
>> +
>> +static void aspeed_host_bmc_device_exit(void)
>> +{
>> +	/* unregister pci driver */
>> +	pci_unregister_driver(&aspeed_host_bmc_dev_driver);
>> +}
>> +
>> +late_initcall(aspeed_host_bmc_device_init);
>> +module_exit(aspeed_host_bmc_device_exit);
> It looks like you can use module_pci_driver() ?
yes, It should work unless the late initcall is important. I will test 
it and see.

Thanks & Regards,

Ninad Palsule