Message-ID: <CAD6G_RS6J3t7YPwBz+c9VduVRmRRN_DL0RUEMx713M4Km2oDHA@mail.gmail.com>
Date:	Fri, 28 Aug 2015 09:43:03 +0530
From:	Jagan Teki <jteki@...nedev.com>
To:	punnaiah choudary kalluri <punnaia@...inx.com>
Cc:	Marek Vašut <marex@...x.de>, harinik@...inx.com,
	Ranjit Waghmode <ranjit.waghmode@...inx.com>,
	Arnd Bergmann <arnd@...db.de>,
	Huang Shijie <b32955@...escale.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	zajec5@...il.com, Michal Simek <michal.simek@...inx.com>,
	linux-spi@...r.kernel.org, juhosg@...nwrt.org, broonie@...nel.org,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	Soren Brinkmann <soren.brinkmann@...inx.com>,
	knut.wohlrab@...bosch.com,
	Brian Norris <computersforpeace@...il.com>,
	ben@...adent.org.uk, David Woodhouse <dwmw2@...radead.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>, beanhuo@...ron.com
Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq
 MPSoC GQSPI controller

On 27 August 2015 at 17:19, punnaiah choudary kalluri
<punnaia@...inx.com> wrote:
> On Thu, Aug 27, 2015 at 3:45 PM, Jagan Teki <jteki@...nedev.com> wrote:
>> On 27 August 2015 at 14:18, punnaiah choudary kalluri
>> <punnaia@...inx.com> wrote:
>>> On Thu, Aug 27, 2015 at 11:53 AM, Jagan Teki <jteki@...nedev.com> wrote:
>>>> On 26 August 2015 at 21:02, punnaiah choudary kalluri
>>>> <punnaia@...inx.com> wrote:
>>>>> On Wed, Aug 26, 2015 at 5:49 PM, Jagan Teki <jteki@...nedev.com> wrote:
>>>>>> On 26 August 2015 at 11:56, Ranjit Waghmode <ranjit.waghmode@...inx.com> 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.
>>> <snip>
>>>>>> 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@...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