Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751651AbbH1ENU (ORCPT ); Fri, 28 Aug 2015 00:13:20 -0400 Received: from s159.web-hosting.com ([68.65.121.203]:45730 "EHLO s159.web-hosting.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751452AbbH1ENJ (ORCPT ); Fri, 28 Aug 2015 00:13:09 -0400 MIME-Version: 1.0 In-Reply-To: References: <1440570367-22569-1-git-send-email-ranjit.waghmode@xilinx.com> Date: Fri, 28 Aug 2015 09:43:03 +0530 Message-ID: Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller From: Jagan Teki To: punnaiah choudary kalluri Cc: =?UTF-8?B?TWFyZWsgVmHFoXV0?= , harinik@xilinx.com, Ranjit Waghmode , Arnd Bergmann , Huang Shijie , "linux-kernel@vger.kernel.org" , zajec5@gmail.com, Michal Simek , linux-spi@vger.kernel.org, juhosg@openwrt.org, broonie@kernel.org, "linux-mtd@lists.infradead.org" , Soren Brinkmann , knut.wohlrab@de.bosch.com, Brian Norris , ben@decadent.org.uk, David Woodhouse , "linux-arm-kernel@lists.infradead.org" , beanhuo@micron.com Content-Type: text/plain; charset=UTF-8 X-OutGoing-Spam-Status: No, score=-2.9 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server159.web-hosting.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - openedev.com X-Get-Message-Sender-Via: server159.web-hosting.com: authenticated_id: jteki@openedev.com X-Source: X-Source-Args: X-Source-Dir: X-From-Rewrite: unmodified, already matched Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9490 Lines: 205 On 27 August 2015 at 17:19, punnaiah choudary kalluri wrote: > On Thu, Aug 27, 2015 at 3:45 PM, Jagan Teki wrote: >> On 27 August 2015 at 14:18, punnaiah choudary kalluri >> wrote: >>> On Thu, Aug 27, 2015 at 11:53 AM, Jagan Teki wrote: >>>> On 26 August 2015 at 21:02, punnaiah choudary kalluri >>>> wrote: >>>>> On Wed, Aug 26, 2015 at 5:49 PM, Jagan Teki wrote: >>>>>> On 26 August 2015 at 11:56, Ranjit Waghmode wrote: >>>>>>> This series adds dual parallel mode support for Zynq Ultrascale+ >>>>>>> MPSoC GQSPI controller driver. >>>>>>> >>>>>>> What is dual parallel mode? >>>>>>> --------------------------- >>>>>>> ZynqMP GQSPI controller supports Dual Parallel mode with following functionalities: >>>>>>> 1) Supporting two SPI flash memories operating in parallel. 8 I/O lines. >>>>>>> 2) Chip selects and clock are shared to both the flash devices >>>>>>> 3) This mode is targeted for faster read/write speed and also doubles the size >>>>>>> 4) Commands/data can be transmitted/received from both the devices(mirror), >>>>>>> or only upper or only lower flash memory devices. >>>>>>> 5) Data arrangement: >>>>>>> With stripe enabled, >>>>>>> Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus >>>>>>> Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus. >>> >>>>>> I don't find any previous discussion about way to inform flash >>>>>> dual-ness into spi-nor >>>>>> from spi drivers. >>>>>> >>>>>> Here is my idea, probably others may think same. >>>>>> Informing dual_flash from drivers/spi through flags or any other mode >>>>>> bits is not a better approach as dual flash feature is specific to >>>>>> spi-nor flash controller (controller specially designed for spi-nor >>>>>> flash not the generic spi controller). So if the driver sits on >>>>>> drivers/mtd/spi-nor/ (ex: fsl-quadspi.c), may be we can inform flash >>>>>> specific things to spi-nor as it's not touching generic spi stack in >>>>>> Linux. But there is a defined-drawback if the driver is moved to >>>>>> drivers/mtd/spi-nor ie it can't use spi core API's at-all. >>>>> >>>>> Xilinx GQSPI is a generic quad spi controller. The primary goal is to support >>>>> Generic/Future command sequences and Future NOR/NAND flash devices. >>>>> This core can also be used for legacy SPI devices. Due to the generic nature >>>>> of the core, software can generate any command sequence. It also has additional >>>>> features like parallel and stacked configurations to double the data >>>>> rate and size. >>>>> Accessing spi-nor flash device is one particular use case and like >>>>> that there will be >>>>> many. So, we decided to keep this driver in generic spi framework and >>>>> that is the ideal >>>>> thing to do for the GQSPI controller. >>>> >>>> Yes, I understand the generic nature of the GQSPI and it's good to >>>> have all-in-one like generic spi, spi-nor and spi-nand and more >>>> together, but Linux stacks were implemented in such a way to support >>>> the each type of controller with connected slaves separably for better >>>> handling. >>> >>> True and this is the reason we have controller drivers and protocol drivers. >>> GQSPI is the controller driver and spi-nor and spi-nand are the >>> protocol drivers. >>> >>>> >>>> Currently GQSPI driver is added in drivers/spi as it supports generic >>>> spi nature and now it enhanced more through flags for supporting >>>> spi-nor, what if we add spi-nand support for the same controller? do >>>> we add one more driver in spi-nand framework (drivers/mtd/spi-nand - >>>> an on going implementation)? My only observation here is even if the >>>> controller is more generic to support more number of device classes, >>>> and adding same driver and populate the slave stuff through flags or >>>> different kind of mechanism to different driver stack, this is not a >>>> better approach I thought. >>> >>> Just to clear, dual parallel( 2 CS and 8 IO lines) is not only specific >>> to flash parts, one can use for any other custom streaming protocols >>> I would say exporting dual parallel connection to protocol drivers is >>> something like depicting the spi bus topology to the protocol layer. >> >> So dual parallel may not used for spi-nor flash it can also used other >> spi slaves that's what your saying is it? > > Yes. As i said above, the main intention of this feature is to improve > the data rate with an overhead of few IO lines. > >> >>> >>> AFAIK, spi-nor and spi-nand are protocol drivers for accessing the >>> nor and nand flash devices sitting on the spi bus using the spi >>> controller driver. >> >> Yes, I do agree with your point, but though driver stacks are >> different with same kind of bus here, I'm trying to spit the GQSPI >> into 3 different controller drivers as Linux understand it and fit on >> to Linux stack with out disturbing the generic-ness. > > I feel this is not a nice idea. if there are 'n' functionalities and having > 'n' controller drivers doesn't seem good in any direction. Sorry, to be clear It doesn't depend on n-theory instead it divergent based on the how many Linux stacks that the GQSPI handle. And also I commented earlier on thread that it may not be a better solutions but it could be one of the good approach to fit into Linux-where-it's-not touching core stacks. Yes, we can do by adding spi bus driver and adding the generic-ness,but I'm feel it ended up talking to many stacks which is advisably not a good idea. > > Protocol driver can query the spi core about the bus topology and it is the > responsibility of the spi core and controller driver providing this information > to the upper layers. I agreed the protocol driver definition here,as per the spi-nor framework the drivers/mtd/spi-nor driver not only a protocol or slave or flash drivers but there are some controller driver as well ex: fsl-qspi-spi-nor.c OK, we both are in different directions - lets wait for any more comments from others. >> Assumption is GQSPI shall split to various platform_drivers (if each >> platform driver treated as a controller) thought it made up of spi >> bus. >> >>>> >>>> Based on the above comments, there is an approach to handle this >>>> support and I'm not 100% sure whether this fits or not but we >>>> implemented the same - this is "probing child devices from parent" >>>> (there was a discussion with Arnd earlier wrt this, but I'm unable to >>>> get the mailing thread) >>>> >>>> Added Arnd (probably will give more inputs or corrections) >>>> >>>> Let me explain how we implemented on our design. >>>> We have PCIe controller that support basic root complex handling, dma >>>> and controller hotplug (not in-build pcie hp) and ideally we need to >>>> write driver for handling root complex on drivers/pci/host and one >>>> hotplug driver in drivers/pci and one more driver in drivers/dma for >>>> handling pcie dma stuff. And some pcie calls need to navigate from >>>> root complex driver to dma and hotplug driver that means there is call >>>> transition from driver/pci to driver/dma which is absolutely not a >>>> good approach (spi to spi-nor and spi-nand transition - in GQSPI case) >>>> >>>> So the implementation we follow is like there is a pcie root complex >>>> driver(probably generic spi driver in drivers/spi/*) and inside probe >>>> we have register platform_device for hotplug (spi-nor) and dma >>>> (spi-nand) and the dma driver in drivers/dma and hotplug driver in >>>> driver/pci/ are platform drivers which is of legacy binding (not with >>>> dts) so there should be a common dts for root complex driver >>>> (drivers/spi/*) and individual child driver need to take those while >>>> registering platform_device. >>>> >>>> example pseudo: >>>> >>>> drivers/dma/dma-child2.c >>>> >>>> Legacy platform_driver binding and handling dma future as normal dma >>>> driver, spi-nand in your case >>>> >>>> drivers/pci/hotplug/hp-child1.c >>>> >>>> Legacy platform_driver binding and handling hotplug future as normal >>>> hotplug driver, spi-nor in your case. >>>> >>>> drivers/pci/host/rc-parent-pci.c >>>> >>>> static int rc_parent_pcie_probe_bridge(struct platform_device *pdev) >>>> { >>>> // Generic rc handling (genric spi stuff) >>>> >>>> // Hotplug handling (spi-nor) >>>> - platform_device_alloc >>>> - assign need resources >>>> - register pdev using platform_device_add >>>> >>>> // DMA handling (spi-nand) >>>> - same as above >>>> } >>>> >>>> static const struct of_device_id rc_parent_pcie_match_table[] = { >>>> {.compatible = "abc,rc-parent",}, >>>> {}, >>>> }; >>>> >>>> static struct platform_driver rc_parent_pcie_driver = { >>>> .driver = { >>>> .name = "rc-parent", >>>> .of_match_table = of_match_ptr(rc_parent_pcie_match_table), >>>> }, >>>> .probe = rc_parent_pcie_probe_bridge, >>>> }; >>>> module_platform_driver(rc_parent_pcie_driver); >>>> >>>> I couldn't find any driver mainlined wrt this design, think more on >>>> GQSPI front, whether this design fits well or not. thanks! -- Jagan | openedev. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/