RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs

Stuart Yoder
2014-09-22T15:01:26Z

> -----Original Message-----
> From: <gregkh@linuxfoundation.org> [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 22, 2014 9:58 AM
> To: Yoder Stuart-B08248
> Cc: Phillips Kim-R1AAHA; Alexander Graf; Rivera Jose-B46482; <arnd@arndb.de>; <linux-kernel@vger.kernel.org>;
> Wood Scott-B07421; <linuxppc-release@linux.freescale.net>
> Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
>
> On Mon, Sep 22, 2014 at 02:42:10PM +0000, Stuart Yoder wrote:
> >
> >
> > > -----Original Message-----
> > > From: Kim Phillips [mailto:kim.phillips@freescale.com]
> > > Sent: Friday, September 19, 2014 3:58 PM
> > > To: Yoder Stuart-B08248
> > > Cc: Alexander Graf; Rivera Jose-B46482; Phillips Kim-R1AAHA; <gregkh@linuxfoundation.org>;
> <arnd@arndb.de>;
> > > <linux-kernel@vger.kernel.org>; Wood Scott-B07421; <linuxppc-release@linux.freescale.net>
> > > Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
> > >
> > > On Fri, 19 Sep 2014 13:25:06 -0500
> > > Yoder Stuart-B08248 <stuart.yoder@freescale.com> wrote:
> > >
> > > > > From: Yoder Stuart-B08248
> > > > > Sent: Thursday, September 18, 2014 7:19 PM
> > > > >
> > > > > +/**
> > > > > > >>> + * @brief Disconnects one endpoint to remove its network link
> > > > > > >>> + *
> > > > > > >>> + * @param[in] mc_io Pointer to opaque I/O object
> > > > > > >>> + * @param[in] dprc_handle Handle to the DPRC object
> > > > > > >>> + * @param[in] endpoint Endpoint configuration parameters.
> > > > > > >>> + *
> > > > > > >>> + * @returns '0' on Success; Error code otherwise.
> > > > > > >>> + * */
> > > > > > >>> +int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle,
> > > > > > >>> + struct dprc_endpoint *endpoint);
> > > > > > >>> +
> > > > > > >>> +/*! @} */
> > > > > > >>
> > > > > > >> this entire file is riddled with non-kernel-doc comment markers: see
> > > > > > >> Documentation/kernel-doc-nano-HOWTO.txt on how to write function and
> > > > > > >> other types of comments in a kernel-doc compliant format.
> > > > > > > This is because this file is using doxygen comments, as it was developed
> > > > > > > by another team. Unless someone else has an objection, I will leave the doxygen comments alone
> and
> > > not
> > > > > make
> > > > > > any change here.
> > > > > >
> > > > > > Do you see any other source files in Linux using doxygen comments?
> > > > >
> > > > > Yes. Grep around a bit and you'll see examples of it. I grep'ed for some
> > > > > doxygen tags and found close to 200 source files with them.
> > >
> > > grepping for the one in this patch above - "! @}" - returns nothing.
> > >
> > > > > > Mixing different documentation styles can
> > > > > > easily become a big mess, because you can't generate external documentation consistently for the
> whole
> > > > > tree.
> > > > >
> > > > > As German mentioned elsewhere, this file is an interface to a hardware block,
> > > > > was written by another team targetting a wide variety of environments-- u-boot,
> > > > > Linux, user space, other OSes etc.
> > > > >
> > > > > We left the doxygen stuff there because while admitedly not used much, there
> > > > > are other examples of it in the kernel and the documentation seems useful.
> > > > > If it can't go into the kernel as is, we can just delete it.
> > > >
> > > > ...to be clear, we could just delete the doyxen tags. There is no scenario
> > > > where I would envision anyone generating documentation from these files in
> > > > the kernel. The tags are there because of where we grabbed the source from.
> > > > It certainly would benefit no one by conversion to kerneldoc.
> > >
> > > except kerneldoc users :)
> > >
> > > > However, just leaving the comments and doxygen tags alone as is would be nice.
> > >
> > > they are incompatible with kerneldoc.
> >
> > Seriously, no one is going to ever generate docs from this obscure bit of code
> > documenting an internal interface within this driver. Makes no sense to convert
> > the comments to kerneldoc.
> >
> > I completely get what you are saying with respect to comments actually documenting
> > kernel interfaces used by different kernel components or uapi interfaces.
> >
> > The doxygen tags are there because this code originated in a different project. That
> > code was intended to be Linux-style compliant and complies with checkpatch --strict.
> > However, we can sanitize the code and remove the tags if they are causing grief.
> > It's too bad though, because we'll have to fork this interface code from its origin.
>
> You "forked" the code when you asked for it be merged into the kernel
> tree. You shouldn't ever have to rely on the "old" version again, so
> please, remove the doxygen mess.

Will do.

Stuart