Message-ID: <CAD6G_RR+QnuK++KvztYo+68sb7yQqwM7qg-X7bybCYaBN+nAGQ@mail.gmail.com>
Date:	Thu, 27 Aug 2015 15:45:43 +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 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?

>
> 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.

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