Messages in this thread | | | From | Laurentiu Tudor <> | Subject | RE: [PATCH 3/3][v4] staging: fsl-mc: move bus driver out of staging | Date | Mon, 22 May 2017 09:15:20 +0000 |
| |
> -----Original Message----- > From: Marc Zyngier [mailto:marc.zyngier@arm.com] > Sent: Monday, May 22, 2017 12:06 PM > To: Laurentiu Tudor <laurentiu.tudor@nxp.com> > > On 22/05/17 09:42, Laurentiu Tudor wrote: > > Hi Marc, > > > >> -----Original Message----- > >> From: Marc Zyngier [mailto:marc.zyngier@arm.com] > >> Sent: Monday, May 22, 2017 10:41 AM > >> > >> On Mon, May 22 2017 at 7:12:39 am GMT, Laurentiu Tudor > >> <laurentiu.tudor@nxp.com> wrote: > >> > >> Hi Laurentiu, > >> > >>> Hi Marc, > >>> > >>>> -----Original Message----- > >>>> From: Marc Zyngier [mailto:marc.zyngier@arm.com] > >>>> Sent: Saturday, May 20, 2017 9:43 AM > >>>> To: Matthias Brugger <matthias.bgg@gmail.com> > >>>> > >>>> On Fri, May 19 2017 at 02:41:43 PM, Matthias Brugger > >>>> <matthias.bgg@gmail.com> wrote: > >>>>> On 19/05/17 15:13, laurentiu.tudor@nxp.com wrote: > >>>>>> From: Stuart Yoder <stuart.yoder@nxp.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
| |