RE: [EXT] Re: [PATCH v2] bus: add platform bus

[Tomasz Duszynski <tduszynski@marvell.com>]

Hi David, 

>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Monday, February 6, 2023 3:29 PM
>To: Tomasz Duszynski <tduszynski@marvell.com>
>Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; chenbo.xia@intel.com; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>; stephen@networkplumber.org
>Subject: [EXT] Re: [PATCH v2] bus: add platform bus
>
>External Email
>
>----------------------------------------------------------------------
>On Sun, Feb 5, 2023 at 7:39 PM Tomasz Duszynski <tduszynski@marvell.com> wrote:
>>
>> Platform bus is a software bus under Linux that manages devices which
>> generally do not have built-in discovery mechanisms. Linux normally
>> learns about platform devices directly from device-tree during boot-up
>> phase.
>>
>> Up to this point, whenever some userspace app needed control over
>> platform device or a range of thereof some sort of driver being a
>> mixture of vdev/rawdev was required.
>>
>> In order to simplify this task, provide an auto-probe experience and
>> separate bus logic from the driver itself, add platform bus support.
>>
>> Currently devices backed up by vfio-platform kernel driver are
>> supported.
>>
>> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
>> ---
>> v2:
>> - take a step back and do not add eal changes that simplify parsing
>>   sysfs string. That should be moved to a more generic library and
>>   submitted separately.
>> - compile code conditionally based on VFIO_PRESENT
>
>I must be missing something, but in the form proposed in v2, this won't work, see below.
>
>[snip]
>

No you did not miss anything. Prior to submission I realized that if dpdk uses pretty
old kernel there might be no vfio uapi header present hence added that define. 
Yeah complete no-brainer. Will fix that in v3. 

>
>> diff --git a/drivers/bus/platform/platform.c
>> b/drivers/bus/platform/platform.c new file mode 100644 index
>> 0000000000..9f06327b77
>> --- /dev/null
>> +++ b/drivers/bus/platform/platform.c
>> @@ -0,0 +1,632 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(C) 2023 Marvell.
>> + */
>> +
>> +#ifdef VFIO_PRESENT
>> +
>
>VFIO_PRESENT is defined in rte_vfio.h and nothing else defines it.
>So the rest of this file is a noop.
>What did I miss?
>
>So either you need to include <rte_vfio.h> first, or we could move the VFIO_PRESENT detection stuff
>from this EAL header, to meson.
>
>
>I did not look further into this patch, since this added code is probably not tested (since not
>compiled).
>
>
>--
>David Marchand