From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh jain Subject: Re: [PATCH v1 28/28] ether: support SoC device/driver Date: Thu, 7 Jul 2016 15:59:41 +0530 Message-ID: <577E2F15.7070607@nxp.com> References: <1462542490-15556-1-git-send-email-viktorin@rehivetech.com> <1462542490-15556-29-git-send-email-viktorin@rehivetech.com> <577397EF.2080300@nxp.com> <20160704150451.1a61fbbd@pcviktorin.fit.vutbr.cz> <577A7245.700@nxp.com> <20160704163646.45d1f0f8@pcviktorin.fit.vutbr.cz> <577B3ABE.4040709@nxp.com> <20160705051630.5861459.88497.5797@rehivetech.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Cc: , Thomas Monjalon To: Jan Viktorin Return-path: Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on0065.outbound.protection.outlook.com [104.47.36.65]) by dpdk.org (Postfix) with ESMTP id A43AF2A5D for ; Thu, 7 Jul 2016 12:29:05 +0200 (CEST) In-Reply-To: <20160705051630.5861459.88497.5797@rehivetech.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jan, Apologies for delay in my response. On Tuesday 05 July 2016 10:46 AM, Jan Viktorin wrote: > Hello Shreyansh, > =E2=80=8E >> On Monday 04 July 2016 08:06 PM, Jan Viktorin wrote: >>> On Mon, 4 Jul 2016 19:57:18 +0530 >>> Shreyansh jain wrote: >>> >>> [...] >>> >>>>>>> @@ -1431,7 +1524,7 @@ >rte_eth_dev_info_get(uint8_t port_id, stru= ct >rte_eth_dev_info *dev_info) >>>>>>> >>>>>>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->>dev_infos_get); >>>>>>> (*dev->dev_ops->dev_infos_get)(dev, dev_info); >>>>>>> - dev_info->pci_dev =3D dev->pci_dev; >>>>>>> + dev_info->soc_dev =3D dev->soc_dev;=20 >>>>>> >>>>>> I think both the members, pci_dev and soc_dev, should be updated b= y this call. >>>>>> Is there some specific reason why soc_dev is the only one which is= getting updated?=20 >>>>> >>>>> Yes, looks like a mistake. Thanks! And sorry for delayed reply.=20 >>>> >>>> No problems - thanks for confirmation. >>>> I have gone through almost complete series and as and when you rebas= e it, it would have my ACK. >>> >>> OK, thanks. That's what I am playing with right now. I've rebased on = v3 of this patch. There will >>> be some more tests in my v2. >>> >>>> rte_driver patchset which I sent last are broken - I will publish an= updated version very soon. >>> >>> I am surprised that you've changed the args to RTE_EAL_PCI_REGISTER..= . Are you sure about this step? >>> I wrote that I'll change it myself for v2 for SoC to accept name and = pointer as it was originally for PCI... >> >> Really? Then probably I understood it wrong. I don't have any issues w= ith the first one as well but just for slightly cleaner approach I though= t of going with your suggest (or, suggestion as understood by me). > =E2=80=8E> >> Anyways the patch is broken and doesn't apply on master. I will push a= new version (with revert EAL_PCI_REGISTER arguments) within today. >=20 > Ok. I am away for few days this week but I will continue as soon as pos= sible on the soc patchset and also on the rte_device/driver issue. We hav= e to cut this as soon as possible. I think the best would be to post a sm= all patchset on top of this one introducing the change. I am anyway working on a driver for NXP's SoC platform which I am basing = over rte_device + SoC patchset. I plan to release a draft of this driver soon. It might help validate bot= h the patchsets in a limited manner. >=20 > I think, there should be a single list of rte_device and a list for rte= _driver while preserving lists for each infra, so also rte_pci_device wou= ld have a separate list. Then, it's possible to iterate over all PCI devi= ces easily and iterate over all devices generically at the same time. I have similar understanding. Separate pci/soc lists, and unified rte_dri= ver and rte_device lists. This, in my opinion, would help keep the interf= aces clean (free of unnecessary checks). >=20 > What I am not sure about are addr and id things. It's quite difficult t= o generalize them. The addr needs a conpare function but how to compare p= ci addr to another type of addr? Do we need this? If so, I'll work on thi= s. I am not sure why we need to compare the PCI addresses with non-PCI (or a= ny other, SoC) address set? Can you elaborate? >=20 > Another thing - resources. Do we want to have a kind of a generic rte_r= esource instead of rte_pci_resource? I think this is clearly possible. I = am about to do this. The idea sounds good but I don't know whether it would be feasible or not= . My understanding is that resources have special characteristics. Genera= lizing them would involve creating opaque objects in rte_resource{..} whi= ch in turn beats the purpose of generalization. Probably, I will wait for your next patchset version to understand your a= pproach. >=20 > I am afraid we are unable to change devargs significantly in this relea= se :/ (due to time and lack of a discussion here).=E2=80=8E What I really= like to see is the virtual device conversion and pmd_type removal. Are y= ou able to look at this? pmd_type removal is my todo as well. This would anyway impact the vdevs. I haven't given devargs much thought, yet. >=20 > Any other objections? I was looking at various discussions [1],[2] that have happened in past. = >>From that, the summary I have is: 1) Generalize devices to rte_device/rte_driver `-- Cleaner interfaces/difference for 'bus', 'driver' and 'device' `-- Moving the init/deinit functions into rte_device/rte_driver layer `-- hierarchical device structure (as explained by David in [1])=20 2) Do away with device type specific initializations (registrations) `-- No more pdev/vdev distinction `-- standardizing devargs for accepting device specific strings. 3) Hotplug support Most of work of (1) David has already done. What remains is completing (2= ) and probably (3) which I haven't verified yet. [1] http://dpdk.org/ml/archives/dev/2016-January/031390.html [2] http://dpdk.org/ml/archives/dev/2016-January/030975.html >=20 > Jan Viktorin > RehiveTech > Sent from a mobile device=E2=80=8E >=20 >>> >>> Jan >>> =20 >> >> - >> Shreyansh >=20 - Shreyansh =20