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: <ff522b83-d84a-d76e-0317-e8ca55fb82b8@amd.com>
Date:   Tue, 16 May 2023 09:08:13 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:96.0) Gecko/20100101
 Thunderbird/96.0
Subject: Re: [PATCH v3 7/7] PCI: endpoint: Add PCI Endpoint function driver
 for MHI bus
Content-Language: en-US
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Kishon VijayAbraham I <kvijayab@amd.com>
Cc:     lpieralisi@kernel.org, kw@linux.com, kishon@kernel.org,
        bhelgaas@google.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
References: <20230314044623.10254-1-manivannan.sadhasivam@linaro.org>
 <20230314044623.10254-8-manivannan.sadhasivam@linaro.org>
 <d25fdf75-e1fe-b83b-cb9c-ec0fb62f9254@amd.com>
 <20230515113647.GA42703@thinkpad>
From:   Kishon Vijay Abraham I <Kishon.vijayabraham@amd.com>
In-Reply-To: <20230515113647.GA42703@thinkpad>
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?bUNmQWY5YmNPb2hnT3Rnb0RCNVF1MUx4cE1sTTAvbDZpSWVrVjA4VFRlcGtI?=
 =?utf-8?B?dVZBcEdJSGovbnRsTmVQTGNoTUZ2TkNHc3EyTzcvc1hjeE9IbGR4TlFqSEMw?=
 =?utf-8?B?TWhYMGdNRzdyeHRJR0RwMVZBUjQwQmFrV1p4anM2VFdDOEdzRWF1YnlOWGhF?=
 =?utf-8?B?a0NXNVF2NGs0REJNd28wNUNVNzFrUXcvU1hSaG05Z1JidVhhbHp3aFlIYzBx?=
 =?utf-8?B?RUxxU05zci9NN3ZRZURCUmNMZ0ZCVDluUy9pMVFyS3llcFJScGJCNk82NzBC?=
 =?utf-8?B?d3IrWGkxQUhaVXhBOHBOTENxSnZPTWJZemo5M3ExOTkrYlEwaWFjdEo1MkFa?=
 =?utf-8?B?MncvS1U3T1FyZU9UaUJHNHA1czFuSFhCWmpHcGplRlFCSTg5cnJrN0FXQXVU?=
 =?utf-8?B?NWlqVTloUEFWc3p4TGhWc29qcGdBek1xLy9ieG5xZjliOHhLaGtvUkxIVnpF?=
 =?utf-8?B?V3lZQWI2K0JoSHJJbG1YZUVsYlpjalZiWDBrNm5oZTZvdmRlazQwa1VUemJk?=
 =?utf-8?B?ZlRqZWdSMkVuQnowQUsxdFVRaXNwU2tyU2xmRkI3TkgwbVVueUtWWitianoz?=
 =?utf-8?B?aXhzTXgvTW01YXM1Uzc5Nk5LNEhoZUo2SHdDUGVhMldQZHRsNlU5NGdlNk9o?=
 =?utf-8?B?Z01qdjZUUHN3TnZiTmVwQ3VUYXlPb2dlTUY2N09SWUNCc2hWYkNxYlEyck0w?=
 =?utf-8?B?azNPczYxTnVBb0hxeFdGQWlVd2xuN0plampnMmNsZnpXbVhJZklWMkMwMVl2?=
 =?utf-8?B?RmNJT3FwWHU1aG9nTDkvRE1iaTBHZmtDSTlMYXN3SGVWY2dUekEzZ28ycjJl?=
 =?utf-8?B?QVFhOWdKQ0NwbDBaYmJjOE9pSlg5a3diRkdWbHdOTTdjYUZwNmszelBYUis3?=
 =?utf-8?B?U2tnVC96QVdBU1RoVlZhanp5QWFhRXZUcmZUU2ZzLzlKZ1dvTk1pYTFnUTVi?=
 =?utf-8?B?SkU1bXBXcXNRMnpiU2hjeFVRdXhYZHVZaEh4ZytvYjBGcXVjYkkzejVYTDFq?=
 =?utf-8?B?Z2t4T1dLeDRpd2ZBZFlpeHVWQlA4US9jQ1BXYXdXRS9UOGFYd3B5VVV5MWhK?=
 =?utf-8?B?VGIrai9FMzBWSDErMUlITzJZcDZvUDZCZ1VBSHJIQzV6NGlYK3BDeVhIUmhh?=
 =?utf-8?B?TTQ0eEVJc3M0VURqME5VRUU2SVVSK2ZPZzFSNmxzT0lXSVJ1dklHUDRNN2xa?=
 =?utf-8?B?dUVWMEVTemd6NHF6Y0FRTU9CZ1hxRFdBb1F4dWJlMjRnd282M2JwTWlwSy9L?=
 =?utf-8?B?STk1UWlHM1lDSE1obE9ycE0vVGRWY2tER2gxK1Q2OXZwOFN5dUdabFVxSkcz?=
 =?utf-8?B?RzU5TUE5ZlRuNGo3djd5NFBIMFlGUHpRQysvQVpSTUlFY1dqaTA1VzN6YjFD?=
 =?utf-8?B?S3ZiWWNVOERjTldGWmh1eUY0bjZ0YVFYbmJnQXJiZ0hOaTU3RG12SUdFV3Vs?=
 =?utf-8?B?QXg4K3VwV29ZTEZMQTNWV0dsS0djeGFmZFViOUJDckM2a0hyVjJCMzREeTM1?=
 =?utf-8?B?YTVCK20rcXlydGZDQlJib1JzV1BUdXJKVlBIYmswT0JpM2E2TXlqbTRVNmtz?=
 =?utf-8?B?ZDNNR1dHUDVsRHZwdnRFVDdlVWJlUXQxSGRVdzJwa1BIeTNDbmJtRWFTOFZ6?=
 =?utf-8?B?ZkhaRE5qNEZtWmdwZmVYZ1IwenN1dTQ3V0RWN0QraWxFN3VTYUJPTGtKeFI5?=
 =?utf-8?B?aHZMbkFYaldTYSsxSlJUNGtNVU5qNWc2MzFVblVJUnd6UU9kQXlybVZ6K3dH?=
 =?utf-8?B?bHJrbnNJMUwxSjMyK0g0WkxKQVpXeU1leDBNU2NKYTBMNFVSRnhHMHIxQ0xQ?=
 =?utf-8?B?TE9BSkNMc1Z5MEV5VVFvdVF1RUZ2UWlDb1NiYXYvTU9JcFdrd1lEai9oSXZE?=
 =?utf-8?B?cXpuSncvRXE2UXQ3dmoyQ0VvZVpDSWVnN1hUVGZ5MmNqR3c3cGpSS2FLemNN?=
 =?utf-8?B?Y0lVM2MzQnZVdWVzYVBsNVRVRGxpckVRUW5HaUZTdDQ3MTVldFZJRDR1V05D?=
 =?utf-8?B?NGhlMG9UTDRmUHh1RU1ydzZrS3VBWkVEZG5RNHhHMmZnZWFPS3IxLzBDRDVv?=
 =?utf-8?B?VXdaV3BQWVlqM1E3Z1NiaVpHdjlYeGhNb3V5RCtpdXMvQklZN2JvcHJ4YXFv?=
 =?utf-8?Q?w34FtOWf05RWqfXLXmvddZP4w?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 86052c3e-9c10-489f-9359-08db55bf0160
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5827.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2023 03:38:25.7301
 (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: bI6Tt8J8qLOdsiwJ3FVOEzTyb7vkW4usvxW61ixm50587il6YxTPJS4IdH7YuDYYJ4cg0okHQHxvy6W0McVsxg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB7484
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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 5/15/2023 5:06 PM, Manivannan Sadhasivam wrote:
> Hi Kishon,
> 
> On Mon, May 15, 2023 at 01:37:28PM +0530, Kishon VijayAbraham I wrote:
>> Hi Manivannan,
>>
>> On 3/14/2023 10:16 AM, Manivannan Sadhasivam wrote:
>>> Add PCI Endpoint driver for the Qualcomm MHI (Modem Host Interface) bus.
>>> The driver implements the MHI function over PCI in the endpoint device
>>> such as SDX55 modem. The MHI endpoint function driver acts as a
>>> controller driver for the MHI Endpoint stack and carries out all PCI
>>> related activities like mapping the host memory using iATU, triggering
>>> MSIs etc...
>>>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>> ---
>>>    drivers/pci/endpoint/functions/Kconfig       |  10 +
>>>    drivers/pci/endpoint/functions/Makefile      |   1 +
>>>    drivers/pci/endpoint/functions/pci-epf-mhi.c | 454 +++++++++++++++++++
>>>    3 files changed, 465 insertions(+)
>>>    create mode 100644 drivers/pci/endpoint/functions/pci-epf-mhi.c
>>>
>>> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
>>> index 9fd560886871..f5171b4fabbe 100644
>>> --- a/drivers/pci/endpoint/functions/Kconfig
>>> +++ b/drivers/pci/endpoint/functions/Kconfig
>>> @@ -37,3 +37,13 @@ config PCI_EPF_VNTB
>>>    	  between PCI Root Port and PCIe Endpoint.
>>>    	  If in doubt, say "N" to disable Endpoint NTB driver.
>>> +
>>> +config PCI_EPF_MHI
>>> +	tristate "PCI Endpoint driver for MHI bus"
>>> +	depends on PCI_ENDPOINT && MHI_BUS_EP
>>> +	help
>>> +	   Enable this configuration option to enable the PCI Endpoint
>>> +	   driver for Modem Host Interface (MHI) bus in Qualcomm Endpoint
>>> +	   devices such as SDX55.
>>> +
>>> +	   If in doubt, say "N" to disable Endpoint driver for MHI bus.
>>> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
>>> index 5c13001deaba..696473fce50e 100644
>>> --- a/drivers/pci/endpoint/functions/Makefile
>>> +++ b/drivers/pci/endpoint/functions/Makefile
>>> @@ -6,3 +6,4 @@
>>>    obj-$(CONFIG_PCI_EPF_TEST)		+= pci-epf-test.o
>>>    obj-$(CONFIG_PCI_EPF_NTB)		+= pci-epf-ntb.o
>>>    obj-$(CONFIG_PCI_EPF_VNTB) 		+= pci-epf-vntb.o
>>> +obj-$(CONFIG_PCI_EPF_MHI)		+= pci-epf-mhi.o
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
>>> new file mode 100644
>>> index 000000000000..03e7f42663b3
>>> --- /dev/null
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
>> .
>> .
>> <snip>
>> .
>> .
>>> +static int pci_epf_mhi_link_up(struct pci_epf *epf)
>>> +{
>>> +	struct pci_epf_mhi *epf_mhi = epf_get_drvdata(epf);
>>> +	const struct pci_epf_mhi_ep_info *info = epf_mhi->info;
>>> +	struct mhi_ep_cntrl *mhi_cntrl = &epf_mhi->mhi_cntrl;
>>> +	struct pci_epc *epc = epf->epc;
>>> +	struct device *dev = &epf->dev;
>>> +	int ret;
>>> +
>>> +	mhi_cntrl->mmio = epf_mhi->mmio;
>>> +	mhi_cntrl->irq = epf_mhi->irq;
>>> +	mhi_cntrl->mru = info->mru;
>>> +
>>> +	/* Assign the struct dev of PCI EP as MHI controller device */
>>> +	mhi_cntrl->cntrl_dev = epc->dev.parent;
>>> +	mhi_cntrl->raise_irq = pci_epf_mhi_raise_irq;
>>> +	mhi_cntrl->alloc_map = pci_epf_mhi_alloc_map;
>>> +	mhi_cntrl->unmap_free = pci_epf_mhi_unmap_free;
>>> +	mhi_cntrl->read_from_host = pci_epf_mhi_read_from_host;
>>> +	mhi_cntrl->write_to_host = pci_epf_mhi_write_to_host;
>>> +
>>> +	/* Register the MHI EP controller */
>>> +	ret = mhi_ep_register_controller(mhi_cntrl, info->config);
>>> +	if (ret) {
>>> +		dev_err(dev, "Failed to register MHI EP controller: %d\n", ret);
>>> +		return ret;
>>> +	}
>>
>> Any reason for delaying registration with MHI bus till link up? Since after
>> linkup, the host can start using the device, this should be doing minimal
>> stuff IMHO. Or is there further handshake between the host side MHI driver
>> and the endpoint side MHI driver?
>>
> 
> Yes, there are further handshakes required between ep and host before the host
> can start using MHI and this is done during mhi_ep_power_up(). Moreover,
> registering the controller during link_up event allows us to do cleanup properly
> when the link goes down.

Thank you for clarifying!

For the entire series:

Reviewed-by: Kishon Vijay Abraham I <kishon@kernel.org>
> 
> - Mani
>   
>> Thanks,
>> Kishon
>