Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7f2dc633-23a2-16dc-8a55-0611e2cd6b7c@amd.com>
Date:   Fri, 7 Oct 2022 15:45:17 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH RFC 0/2] Generate device tree node for pci devices
To:     Rob Herring <robh@kernel.org>
Cc:     Frank Rowand <frowand.list@gmail.com>,
        Lizhi Hou <lizhi.hou@amd.com>, linux-pci@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        helgaas@kernel.org, clement.leger@bootlin.com, max.zhen@amd.com,
        larry.liu@amd.com, brian.xu@amd.com, stefano.stabellini@xilinx.com,
        trix@redhat.com
References: <1661809417-11370-1-git-send-email-lizhi.hou@amd.com>
 <f831f62b-004b-4f73-2a66-de9d675c44b6@gmail.com>
 <CAL_JsqJn=i=TT9NArHK25g1NkZN_G1GjN3EGEeTAvyW_PUhgcw@mail.gmail.com>
 <8ea70992-d4e9-8bbd-0fca-d5700f84e071@amd.com>
 <CAL_JsqJA8K0nKO=O2QjyHdue6=EHYNqCTw6pKUsBKwrdrYGePA@mail.gmail.com>
Content-Language: en-US
From:   Sonal Santan <sonal.santan@amd.com>
In-Reply-To: <CAL_JsqJA8K0nKO=O2QjyHdue6=EHYNqCTw6pKUsBKwrdrYGePA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aHB5Y3d0aUtrRzFtajlmNWk3YVBIdUJxK1pHQVFKV2R4Nm51aVJJWnZlRVZH?=
 =?utf-8?B?aC9DYlU5RjFwOW1SbmJJSUpXMjl2SjdNVVNzL1dGdTg5eWNNNElKTVpyU3NO?=
 =?utf-8?B?d1Bsd29YN2lEUmJjK1gzRzc2RUkrbUlMR0NOTVJpQkxMc2xUQ3hlQVdWN3RW?=
 =?utf-8?B?TW9hNkxzVlR2WXR1Wko1MkdQaTR5TElXc3A4eExlRE9wSHMyeDAzd01GS1oz?=
 =?utf-8?B?NDVVb0NqSFZQOHdUYUhPUVlwekR1VlhKOHZodXpWQ3Zob0FkMWhaUFE2aFNE?=
 =?utf-8?B?RFpyT2QvZ1hJWXQ0SXV1WDBFRFE4QkdDaXZyN3RrOFlveElZZ3JISU9lVHo3?=
 =?utf-8?B?eGlBcHhaR29FNjZ2ZmdSWHlnYmUzREdDMW9GWEI2MWVMd3hncFZXY0o3ZTh0?=
 =?utf-8?B?bGQ2TU5weDBMRW9BSmkzYmQ5MVJ4MUJwVkZ0aDlxb0RmVytiU0R2YTJiMmtU?=
 =?utf-8?B?U2FIWFdYK1pST0QrQ1VYcmtVUXdGaWhiQytzWnNyNDFCSHZPZWxMV2tic2I3?=
 =?utf-8?B?czRXV3ljZXJEcVVwWWZ6bEllRW5mcDJDVHJhSUNSMVZnRVR3cG5neVBjZEcv?=
 =?utf-8?B?Zzk2bXdVRHVvaWVBTmRZdkFldUVFTkxDa242bENrVXM1MDUrL25jQm9oQkdJ?=
 =?utf-8?B?aFVZUWxnK0d6d3ZrdTNNS0ZYQjltaGZmYkVuWHJVMG9SVDBVdGtoSmFYL2t5?=
 =?utf-8?B?TDFYZG0zL0xzeFpmMGVhTzFPa3BOWWQ0aEUxOXRVbHR5VytBa2VuT2ZTNHRR?=
 =?utf-8?B?endBTVM3TG5MM0JyaTY1WGJYNjBsZFA0NkFlTnFiS3EzUWMvTE1peThQZnhQ?=
 =?utf-8?B?RHM0b2ZlQktBOVhCa0ZEWUJ3M0J1dStXZ0FLM2o3SlBVMitUSlBvcHVLZzRz?=
 =?utf-8?B?OE9JM0U3WDRURjNFWW9IM2hXWG1pcGlCbU1PSm1xZGIrd01Ta0hyZ2JIQ0pp?=
 =?utf-8?B?ZXRpekd0ZFgrRWFIZW5ONHRReGFSMkJOZUg3U3RvVThxVUJxVVVWYi9CS1Z2?=
 =?utf-8?B?NThMYklYZnlqZm9NUVhlM1ErenVmMzdZMVdaNmw1dkdjN3NUM3JBNThEQXRB?=
 =?utf-8?B?b2dGY1VEWVhxK1VHRHpzZ3ZUbmQ1NytjVzZpdUtDUFo4MkdKVjYwb1dGYm41?=
 =?utf-8?B?UXR0b2pwVmRzMnducDVHNndSNGFlWlArOWppcHBqcFUrUzMwa0xtM202cDhn?=
 =?utf-8?B?elFoaFlYNldYWW1mUm5LVGFzdU1EVjgrQVY2eWxYV3oxTStjbS9DVFZoRXVN?=
 =?utf-8?B?cWU0UjVHTG56TWsxcFVyZDJZU0dsdmdUY09qS09neElVeEtjSjNqYTAvYlha?=
 =?utf-8?B?S25UZjJwMXhzU1VLQ051VDhtRG1tWEg3NExMQ1h6R0FGOG1vUW5oWWxmeURN?=
 =?utf-8?B?bldMcEtnYVJHWXczakJrY3VZZ2VNYTcySCtubSszUmgxTDRKeS9JcEhGMkZY?=
 =?utf-8?B?Yy9FZVM3UVdVWU96L21kZWNJK3U1SUFXMUVLOFJXbEhESXZlYXdwcXg4WCtB?=
 =?utf-8?B?NmpJckpEMGg0aThITGd2aTMvWjAwY3Eyc2x0YjdQMUhYTkErUHBiaEV1YWY5?=
 =?utf-8?B?SVNQRDA1Qnl0UWlPME5GTEVuemVUbWE2R2F0ZGR5dHUraDFsVHRZamhRV0p6?=
 =?utf-8?B?azZLSTRlc0xBYVdYblNiaW5LVlJjdElSdlc2eVhDWVdvWUlLWVEzMVV0aTRt?=
 =?utf-8?B?L01EY3NRWmpQTHU5anlvdjVzQUNGYWRFWEd4cHBTSnE1L2VoclJEYU4yZGtk?=
 =?utf-8?B?ZmJBRkhaR3pnbHZzQUd5OHVUMCtwRW1LSVdWT2VPVkNHdFBrTTROOHh0WnFp?=
 =?utf-8?B?ay82OW9xU1hmTklMaS96K205ZW9KZWNSTFRKemxacFVXbmdhWm9nMnE4cWNT?=
 =?utf-8?B?bFpLREgvajYxQWozRW5lR0FnaFR6NnJoMFA2OG9MR1lrdVgrMzh3TUxzSFUv?=
 =?utf-8?B?SGNMayt3UlcvNkxObXJ3SkpFbGtxd3JmSEh5bTlGSitRdUVmRVFKWldYM21U?=
 =?utf-8?B?N21WVThZQW9ldlU2bnlHWXRtaUhaWWgwZUlsTWVGcWQ2N2xiNzloWWJkcjRC?=
 =?utf-8?B?My81UklLQ0thQmtRQTJwdzQ3UWhGeFB2L2E2dUhLZVFoNWlxTkxoZ0E0YTNq?=
 =?utf-8?Q?7UENIK4NX5dwseNrZpkzxpfLN?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 44f36692-b2c3-4e43-8139-08daa8b59d4c
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4502.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Oct 2022 22:45:21.0273
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: i0wUKkk28QrwIJhLkATGaZ0U4GWABor0tmbUD4ZdEgvovWzCgAFYm9gwLgYTZpYp
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5953
X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/6/22 08:10, Rob Herring wrote:
> On Fri, Sep 30, 2022 at 2:29 PM Sonal Santan <sonal.santan@amd.com> wrote:
>>
>> On 9/26/22 15:44, Rob Herring wrote:
>>> On Fri, Sep 16, 2022 at 6:15 PM Frank Rowand <frowand.list@gmail.com> wrote:
>>>>
>>>> On 8/29/22 16:43, Lizhi Hou wrote:
>>>>> This patch series introduces OF overlay support for PCI devices which
>>>>> primarily addresses two use cases. First, it provides a data driven method
>>>>> to describe hardware peripherals that are present in a PCI endpoint and
>>>>> hence can be accessed by the PCI host. An example device is Xilinx/AMD
>>>>> Alveo PCIe accelerators. Second, it allows reuse of a OF compatible
>>>>> driver -- often used in SoC platforms -- in a PCI host based system. An
>>>>> example device is Microchip LAN9662 Ethernet Controller.
>>>>>
>>>>> This patch series consolidates previous efforts to define such an
>>>>> infrastructure:
>>>>> https://lore.kernel.org/lkml/20220305052304.726050-1-lizhi.hou@xilinx.com/
>>>>> https://lore.kernel.org/lkml/20220427094502.456111-1-clement.leger@bootlin.com/
>>>>>
>>>>> Normally, the PCI core discovers PCI devices and their BARs using the
>>>>> PCI enumeration process. However, the process does not provide a way to
>>>>> discover the hardware peripherals that are present in a PCI device, and
>>>>> which can be accessed through the PCI BARs. Also, the enumeration process
>>>>> does not provide a way to associate MSI-X vectors of a PCI device with the
>>>>> hardware peripherals that are present in the device. PCI device drivers
>>>>> often use header files to describe the hardware peripherals and their
>>>>> resources as there is no standard data driven way to do so. This patch> series proposes to use flattened device tree blob to describe the
>>>>> peripherals in a data driven way.
>>>>
>>>>> Based on previous discussion, using
>>>>> device tree overlay is the best way to unflatten the blob and populate
>>>>> platform devices.
>>>>
>>>> I still do not agree with this statement.  The device tree overlay
>>>> implementation is very incomplete and should not be used until it
>>>> becomes more complete.  No need to debate this right now, but I don't want
>>>> to let this go unchallenged.
>>>
>>> Then we should remove overlay support. The only way it becomes more
>>> complete is having actual users.
>>>
>>> But really, whether this is the right solution to the problem is
>>> independent of the state of kernel overlay support.
>>>
>>>> If there is no base system device tree on an ACPI based system, then I
>>>> am not convinced that a mixed ACPI / device tree implementation is
>>>> good architecture.
>>>
>>> Most/all of this series is needed for a DT system in which the PCI
>>> devices are not populated in the DT.
>>>
>>>>    I might be more supportive of using a device tree
>>>> description of a PCI device in a detached device tree (not linked to
>>>> the system device tree, but instead freestanding).  Unfortunately the
>>>> device tree functions assume a single system devicetree, with no concept
>>>> of a freestanding tree (eg, if a NULL device tree node is provided to
>>>> a function or macro, it often defaults to the root of the system device
>>>> tree).  I need to go look at whether the flag OF_DETACHED handles this,
>>>> or if it could be leveraged to do so.
>>>
>>> Instead of worrying about a theoretical problem, we should see if
>>> there is an actual problem for a user.
>>>
>>> I'm not so worried about DT functions themselves, but places which
>>> have 'if ACPI ... else (DT) ...' paths.
>>>
>>
>> Bringing this thread back into focus. Any thoughts on how to move forward?
> 
> Reviewers raise concerns/issues and the submitters work to address
> them or explain why they aren't an issue. The submitter has to push
> things forward. That's how the process works.
> 
We are working on updating the patch set to address the feedback. The 
design is still based on device tree overlay infrastructure.

> As I noted, much of this is needed on a DT system with PCI device not
> described in DT. So you could split out any ACPI system support to
> avoid that concern for example. Enabling others to exercise these
> patches may help too. Perhaps use QEMU to create some imaginary
> device.
To verify this patch set, in addition to a x86_64/ACPI based system, we 
also have an AARCH64/DT QEMU setup where we have attached a physical 
Alveo device. We haven't run into any ACPI or DTO issues so far.

Perhaps we can introduce this feature in a phased manner where we first 
enable DT based platforms and then enable ACPI based platforms?

-Sonal
> 
> Rob