[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5c918888-6933-7661-45f0-32ae4521aa2c@linux.vnet.ibm.com>
Date: Wed, 23 Aug 2023 12:32:35 -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,
On 8/22/23 11:14 AM, Ninad Palsule wrote:
> 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.
I will not be able to use module_pci_driver() as it doesn't support late
initcall which is required otherwise
8250 registration fails. So I am not making this change.
>
> Thanks & Regards,
>
> Ninad Palsule
>
Powered by blists - more mailing lists