| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 17 Jun 2015 13:17:20 -0000 From: "Dov Levenglick" <dovl@...eaurora.org> To: "Rob Herring" <robherring2@...il.com> Cc: "Dov Levenglick" <dovl@...eaurora.org>, "Yaniv Gardi" <ygardi@...eaurora.org>, "Akinobu Mita" <akinobu.mita@...il.com>, "Jej B" <james.bottomley@...senpartnership.com>, "LKML" <linux-kernel@...r.kernel.org>, linux-scsi@...r.kernel.org, "linux-arm-msm" <linux-arm-msm@...r.kernel.org>, "Santosh Y" <santoshsy@...il.com>, linux-scsi-owner@...r.kernel.org, "Subhash Jadavani" <subhashj@...eaurora.org>, "Paul Bolle" <pebolle@...cali.nl>, "Gilad Broner" <gbroner@...eaurora.org>, "Rob Herring" <robh+dt@...nel.org>, "Pawel Moll" <pawel.moll@....com>, "Mark Rutland" <mark.rutland@....com>, "Ian Campbell" <ijc+devicetree@...lion.org.uk>, "Kumar Gala" <galak@...eaurora.org>, "Vinayak Holikatti" <vinholikatti@...il.com>, "James E.J. Bottomley" <jbottomley@...n.com>, "Dolev Raviv" <draviv@...eaurora.org>, "Christoph Hellwig" <hch@....de>, "Sujit Reddy Thumma" <sthumma@...eaurora.org>, "Raviv Shvili" <rshvili@...eaurora.org>, "Sahitya Tummala" <stummala@...eaurora.org>, "open list:OPEN FIRMWARE AND..." <devicetree@...r.kernel.org> Subject: Re: [PATCH v2 4/4] scsi: ufs: probe and init of variant driver from the platform device > On Wed, Jun 17, 2015 at 2:42 AM, Dov Levenglick <dovl@...eaurora.org> > wrote: >>> On Tue, Jun 9, 2015 at 12:53 AM, Dov Levenglick <dovl@...eaurora.org> >>> wrote: >>>>> On Sun, Jun 7, 2015 at 10:32 AM, <ygardi@...eaurora.org> wrote: >>>>>>> 2015-06-05 5:53 GMT+09:00 <ygardi@...eaurora.org>: >>> >>> [...] >>> >>>>>> If ufshcd-pltfrm driver is loaded before ufs-qcom, (what actually >>>>>> happens >>>>>> always), then the calling to of_platform_populate() which is added, >>>>>> guarantees that ufs-qcom probe will be called and finish, before >>>>>> ufshcd_pltfrm probe continues. >>>>>> so ufs_variant device is always there, and ready. >>>>>> I think it means we are safe - since either way, we make sure >>>>>> ufs-qcom >>>>>> probe will be called and finish before dealing with ufs_variant >>>>>> device >>>>>> in >>>>>> ufshcd_pltfrm probe. >>>>> >>>>> This is due to the fact that you have 2 platform drivers. You should >>>>> only have 1 (and 1 node). If you really think you need 2, then you >>>>> should do like many other common *HCIs do and make the base UFS >>>>> driver >>>>> a set of library functions that drivers can use or call. Look at >>>>> EHCI, >>>>> AHCI, SDHCI, etc. for inspiration. >>>> >>>> Hi Rob, >>>> We did look at SDHCI and decided to go with this design due to its >>>> simplicity and lack of library functions. Yaniv described the proper >>>> flow >>>> of probing and, as we understand things, it is guaranteed to work as >>>> designed. >>>> >>>> Furthermore, the design of having a subcore in the dts is used in the >>>> Linux kernel. Please have a look at drivers/usb/dwc3 where - as an >>>> example >>>> - both dwc3-msm and dwc3-exynox invoke the probing function in core.c >>>> (i.e. the shared underlying Synopsys USB dwc3 core) by calling >>>> of_platform_populate(). >>> >>> That binding has the same problem. Please don't propagate that. There >>> is no point in a sub-node in this case. >>> >>>> Do you see a benefit in the SDHCi implementation? >>> >>> Yes, it does not let the kernel driver design dictate the hardware >>> description. >>> >>> Rob >>> >> >> Hi Rob, >> We appear to be having a philosophical disagreement on the practicality >> of >> designing the ufshcd variant's implementation - in other words, we >> disagree on the proper design pattern to follow here. >> If I understand correctly, you are concerned with a design pattern >> wherein >> a generic implementation is wrapped - at the device-tree level - in a >> variant implementation. The main reason for your concern is that you >> don't >> want the "kernel driver design dictate the hardware description". >> >> We considered this point when we suggested our implementation (both >> before >> and after you raised it) and reached the conclusion that - while an >> important consideration - it should not be the prevailing one. I believe >> that you will agree once you read the reasoning. What guided us was the >> following: >> 1. Keep our change minimal. >> 2. Keep our patch in line with known design patterns in the kernel. >> 3. Have our patch extend the existing solution rather than reinvent it. >> >> It is the 3rd point that is most important to this discussion, since UFS >> has already been deployed by various vendors and is used by OEM. >> Changing >> ufshcd to a set of library functions that would be called by variants >> would necessarily introduce a significant change to the code flow in >> many >> places and would pose a backward compatibility issue. By using the tried >> and tested pattern of subnodes in the dts we were able to keep the >> change >> simple, succinct, understandable, maintainable and backward compatible. >> In >> fact, the entire logic tying of the generic implementation to the >> variant >> takes ~20 lines of code - both short and elegant. > > The DWC3 binding does this and nothing else that I'm aware of. This > hardly makes for a common pattern. If you really want to split this to > 2 devices, you can create platform devices without having a DT node. > > If you want to convince me this is the right approach for the binding > then you need to convince me the h/w is actually split this way and > there is functionality separate from the licensed IP. > > Rob > I don't understand the challenge that you just posed. It is clear from our implementation that there is the standard and variants thereof. I know this to be a fact on the processors that we are working on. Furthermore, although I didn't check each and every result in the search, of_platform_populate is used in more locations than dwc3 and at least a few of them seem have be using the same paradigm as ours (http://lxr.free-electrons.com/ident?i=of_platform_populate). QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists