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

Stuart Yoder
2014-09-19T01:14:00Z

> -----Original Message-----
> From: Kim Phillips [mailto:kim.phillips@freescale.com]
> Sent: Monday, September 15, 2014 6:45 PM
> To: Rivera Jose-B46482
> Cc: gregkh@linuxfoundation.org; arnd@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Phillips
> Kim-R1AAHA; Wood Scott-B07421; agraf@suse.de; linuxppc-release@linux.freescale.net
> Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
>
> On Thu, 11 Sep 2014 12:34:21 -0500
> "J. German Rivera" <German.Rivera@freescale.com> wrote:
>
> > From: "J. German Rivera" <German.Rivera@freescale.com>
> >
> > APIs to access the Management Complex (MC) hardware
> > module of Freescale LS2 SoCs. This patch includes
> > APIs to check the MC firmware version and to manipulate
> > DPRC objects in the MC.
> >
> > Signed-off-by: J. German Rivera <German.Rivera@freescale.com>
> > Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com>
> > ---
> > drivers/bus/fsl-mc/dpmng.c | 93 +++++
> > drivers/bus/fsl-mc/dprc.c | 504 +++++++++++++++++++++++
> > drivers/bus/fsl-mc/fsl_dpmng_cmd.h | 83 ++++
> > drivers/bus/fsl-mc/fsl_dprc_cmd.h | 545 +++++++++++++++++++++++++
> > drivers/bus/fsl-mc/fsl_mc_sys.c | 237 +++++++++++
> > include/linux/fsl_dpmng.h | 120 ++++++
> > include/linux/fsl_dprc.h | 790 ++++++++++++++++++++++++++++++++++++
> > include/linux/fsl_mc_cmd.h | 182 +++++++++
> > include/linux/fsl_mc_sys.h | 81 ++++
> > 9 files changed, 2635 insertions(+)
> > create mode 100644 drivers/bus/fsl-mc/dpmng.c
> > create mode 100644 drivers/bus/fsl-mc/dprc.c
> > create mode 100644 drivers/bus/fsl-mc/fsl_dpmng_cmd.h
> > create mode 100644 drivers/bus/fsl-mc/fsl_dprc_cmd.h
> > create mode 100644 drivers/bus/fsl-mc/fsl_mc_sys.c
> > create mode 100644 include/linux/fsl_dpmng.h
> > create mode 100644 include/linux/fsl_dprc.h
> > create mode 100644 include/linux/fsl_mc_cmd.h
> > create mode 100644 include/linux/fsl_mc_sys.h
>
> the fsl prefix in the filename fsl_dpmng_cmd.h is redundant with
> its directory name fsl-mc/. Note that I find dashes ('-') in
> filenames make them easier to type: is there a reason we're using
> underscores here?
>
> Also, any reason why these and future include files aren't being put
> in include/linux/fsl/, so as to not pollute the top level
> include/linux/? That way, we can also remove the fsl- prefix from
> those filenames, too..

There is nothing in include/linux/fsl except 1 driver from 2 years ago.
Why is that?

I'd also prefer to see these headers is include/linux/fsl, but given that
the directory is essentially unused it seemed that include/linux was more
conventional.

> > diff --git a/drivers/bus/fsl-mc/dpmng.c b/drivers/bus/fsl-mc/dpmng.c
> > new file mode 100644
> > index 0000000..c6ed27c
> > --- /dev/null
> > +++ b/drivers/bus/fsl-mc/dpmng.c
> > @@ -0,0 +1,93 @@
> > +/* Copyright 2013-2014 Freescale Semiconductor Inc.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are met:
> > + * * Redistributions of source code must retain the above copyright
> > + * notice, this list of conditions and the following disclaimer.
> > + * * Redistributions in binary form must reproduce the above copyright
> > + * notice, this list of conditions and the following disclaimer in the
> > + * documentation and/or other materials provided with the distribution.
> > + * * Neither the name of Freescale Semiconductor nor the
> > + * names of its contributors may be used to endorse or promote products
> > + * derived from this software without specific prior written permission.
> > + *
> > + *
> > + * ALTERNATIVELY, this software may be distributed under the terms of the
> > + * GNU General Public License ("GPL") as published by the Free Software
> > + * Foundation, either version 2 of that License or (at your option) any
> > + * later version.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
>
> interesting, normally this text reads:
>
> "THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS"
>
> ...does that mean we're excluding non-Freescale copyright holders
> and contributors from this warranty statement? That doesn't seem
> appropriate for an upstream kernel submission.

This is a standard license dual license used by Freescale, wording
most likely set by the legal dept.

There are many instances of it in the kernel (mostly in device trees).

git grep 'THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor' | wc -l
180

Unless it is condition of being accepted into the kernel I want
to leave it as is.

Stuart