Message-ID: <VI1PR0401MB1856584067178ECA1B1AA983ECF80@VI1PR0401MB1856.eurprd04.prod.outlook.com>
Date:   Mon, 22 May 2017 09:15:20 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Marc Zyngier <marc.zyngier@....com>
CC:     Matthias Brugger <matthias.bgg@...il.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "stuyoder@...il.com" <stuyoder@...il.com>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "arnd@...db.de" <arnd@...db.de>,
        Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>,
        "Roy Pledge" <roy.pledge@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "agraf@...e.de" <agraf@...e.de>,
        "Catalin Horghidan" <catalin.horghidan@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Leo Li <leoyang.li@....com>,
        "Bharat Bhushan" <bharat.bhushan@....com>,
        Jason Cooper <jason@...edaemon.net>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Rob Herring <robh+dt@...nel.org>,
        Bogdan Vlad <bogdan.vlad@....com>
Subject: RE: [PATCH 3/3][v4] staging: fsl-mc: move bus driver out of staging



> -----Original Message-----
> From: Marc Zyngier [mailto:marc.zyngier@....com]
> Sent: Monday, May 22, 2017 12:06 PM
> To: Laurentiu Tudor <laurentiu.tudor@....com>
> 
> On 22/05/17 09:42, Laurentiu Tudor wrote:
> > Hi Marc,
> >
> >> -----Original Message-----
> >> From: Marc Zyngier [mailto:marc.zyngier@....com]
> >> Sent: Monday, May 22, 2017 10:41 AM
> >>
> >> On Mon, May 22 2017 at  7:12:39 am GMT, Laurentiu Tudor
> >> <laurentiu.tudor@....com> wrote:
> >>
> >> Hi Laurentiu,
> >>
> >>> Hi Marc,
> >>>
> >>>> -----Original Message-----
> >>>> From: Marc Zyngier [mailto:marc.zyngier@....com]
> >>>> Sent: Saturday, May 20, 2017 9:43 AM
> >>>> To: Matthias Brugger <matthias.bgg@...il.com>
> >>>>
> >>>> On Fri, May 19 2017 at 02:41:43 PM, Matthias Brugger
> >>>> <matthias.bgg@...il.com> wrote:
> >>>>> On 19/05/17 15:13, laurentiu.tudor@....com wrote:
> >>>>>> From: Stuart Yoder <stuart.yoder@....com>
> >>>>>>
> >>>>>> Move the source files out of staging into their final locations:
> >>>>>>    -include files in drivers/staging/fsl-mc/include go to include/linux/fsl
> >>>>>>    -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >>>>>
> >>>>> This driver has as compatible "arm,gic-v3-its". I wonder if this
> >>>>> is correct and if it should be moved like this out of staging.
> >>>>
> >>>> This is no different from the way we handle *any* bus that uses the
> >>>> GICv3 ITS as an MSI controller. Each bus provides its glue code
> >>>> that latches onto the ITS node, and calls into the generic code.
> >>>>
> >>>> Now, when it comes to moving this out of staging, here is my concern:
> >>>> There is mention of a userspace tool (restool) used to control the
> >>>> HW. Where is this tool? Where is the user ABI documented?
> >>>
> >>> The tool is published here:

[snip]

> >>> There are two ways of configuring the mc-bus:
> >>>  - a static one, through a FDT based configuration file (we call it
> >>> DPL), documented in the refman linked below, chapter 22.
> >>>  - a dynamic one, using this restool utility.
> >>> Please note the usage of restool is optional.
> >>
> >> Optional or not, it still is a userspace ABI, and while I can see
> >> restool issuing ioctl system calls to configure the HW, I cannot see
> >> the corresponding code in the kernel tree. So how does it work?
> >> If the syscall interface is not present in the mainline kernel, drop
> >> the reference to it in the documentation. If it is there (and I
> >> obviously missed it), document it, and get it reviewed.
> >
> > Our original plan was to first get the bus out of staging and after that submit
> the restool support ASAP (patches are done - so I'm thinking at few days
> timeframe).
> > if this is not acceptable, I can drop the restool reference from the README
> and resubmit the patch series. We'll re-add the reference together with the
> restool support patches.
> 
> I think it would make a lot more sense to drop anything that is not implemented
> by the current code. Once you have patches that implement this userspace
> interface, they can be reviewed together with the corresponding
> documentation.

Ok, sounds good. I'll respin the patch series. 

> >> If there are associated DT bindings to the kernel code, they must be
> >> documented (and reviewed) as part of the device-tree documentation,
> >> and not in some obscure, hard to access document.
> >
> > There's only one binding involved and it's already accepted [1].
> 
> Ah, I missed it. It would be good to mention it in the documentation as well.

Good point. I'll add a reference in the next respin.

> > [snip]
> >
> >>>
> >>> We're also working on publishing the docs on github so that they're
> >>> more accessible.
> >>
> >> That'd be great, because the way the registration process is
> >> presented, I'd have to agree to the Access Agreement *before* having
> >> a chance to read it. Not going to happen...
> >
> > Sorry about that. Not much I can do. :-(
> 
> I understand this is not your job ;-). But maybe making people inside NXP aware
> of the issue would help... Oh well.

I'll make sure your comments will reach our guys and in the meantime push to get the docs on github.

---
Thanks & Best Regards, Laurentiu

Powered by blists - more mailing lists