[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160209015759.GA21846@kroah.com>
Date: Mon, 8 Feb 2016 17:57:59 -0800
From: Greg KH <gregkh@...uxfoundation.org>
To: Lijun Pan <Lijun.Pan@...escale.com>
Cc: arnd@...db.de, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org, bhamciu1@...escale.com,
Lijun.Pan2000@...il.com, bhupesh.sharma@...escale.com,
german.rivera@...escale.com, agraf@...e.de,
stuart.yoder@...escale.com, nir.erez@...escale.com,
itai.katz@...escale.com, scottwood@...escale.com,
leoli@...escale.com, R89243@...escale.com,
dan.carpenter@...cle.com, richard.schmitt@...escale.com
Subject: Re: [PATCH v3 7/8] staging: fsl-mc: update TODO and README for
restool driver
On Mon, Feb 08, 2016 at 05:40:17PM -0600, Lijun Pan wrote:
> Add more introduction of restool driver and state why
> restool driver is needed in helping moving fsl-mc bus
> out of staging tree.
>
> Signed-off-by: Lijun Pan <Lijun.Pan@...escale.com>
> ---
> drivers/staging/fsl-mc/README.txt | 11 ++++++++++-
> drivers/staging/fsl-mc/TODO | 18 ++++++++++++++++--
> 2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/fsl-mc/README.txt b/drivers/staging/fsl-mc/README.txt
> index 8214102..e9ec507 100644
> --- a/drivers/staging/fsl-mc/README.txt
> +++ b/drivers/staging/fsl-mc/README.txt
> @@ -130,7 +130,16 @@ the objects involved in creating a network interfaces.
> via a config file passed to the MC when firmware starts
> it. There is also a Linux user space tool called "restool"
> that can be used to create/destroy containers and objects
> - dynamically.
> + dynamically. The kernel side restool driver communicates with
> + user space restool via ioctl. Restool relies on allocator driver
> + to allocate dpmcp resources, enumerates fsl-mc bus to find root dprc
> + objects of interest. When the user space restool program sends a request
> + to restool driver to create a dp* objects in MC firmware, an interrupt
> + will be triggered by MC firmware and the dprc driver's interrupt handler
> + shall process the interrupt (synchronizing the objects in MC firmware and
> + objects in Linux kernel). Though small, restool driver helps verify all
> + the functionality of fsl-mc bus, dprc driver, allocator driver,
> + and MC flib API.
>
> -DPAA2 Objects for an Ethernet Network Interface
>
> diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
> index 5065821..4892eb6 100644
> --- a/drivers/staging/fsl-mc/TODO
> +++ b/drivers/staging/fsl-mc/TODO
> @@ -5,10 +5,24 @@
> fsl-mc bus out of staging.
>
> * Decide if multiple root fsl-mc buses will be supported per Linux instance,
> - and if so add support for this.
> + and if so add support for this. No matter fsl-mc bus support multiple root
> + dprc or not, restool driver is designed to support multiple root if fsl-mc
> + bus is ready some day later. If there is only one root dprc, restool driver
> + works fine.
The english here doesn't make much sense, and also, it seems that you
all can't agree what should happen here.
Why are you adding new functionality to this driver instead of working
to get it out of the staging tree properly? What is this patch series
doing toward _REMOVING_ items from the TODO list? It seems like you are
adding more stuff to sort out at some unknown later time.
> +* Add at least one driver utilizing fsl-mc bus. Restool driver is a very
> + small and simple driver, which interacts with fsl-mc bus, dprc driver,
> + allocator driver. Restool relies on allocator driver to allocate
> + dpmcp resources, enumerates fsl-mc bus to find root dprc objects of interest.
> + When the user space restool program sends a request to restool driver to
> + create a dp* objects in MC firmware, an interrupt will be triggered by
> + MC firmware and the dprc driver's interrupt handler shall process the
> + interrupt. Though small, restool driver helps verify all the functionality
> + of fsl-mc bus, dprc driver, allocator driver, and MC flib API. Restool
> + driver helps in moving fsl-mc bus out of staging branch.
Again, this doesn't make much sense. Why is this a TODO item?
> * Add at least one device driver for a DPAA2 object (child device of the
> - fsl-mc bus). Most likely candidate for this is adding DPAA2 Ethernet
> + fsl-mc bus). Most likely candidate for this is adding DPAA2 Ethernet
Unneeded changed.
Why has the maintainers for this code not signed-off on these changes?
thanks,
greg k-h
Powered by blists - more mailing lists