Messages in this thread | | | Subject | Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs | From | Scott Wood <> | Date | Thu, 18 Sep 2014 18:03:12 -0500 |
| |
On Thu, 2014-09-18 at 15:22 -0500, Kim Phillips wrote: > On Thu, 18 Sep 2014 15:14:03 +0200 > Alexander Graf <agraf@suse.de> wrote: > > > > Am 18.09.2014 um 06:17 schrieb German Rivera <German.Rivera@freescale.com>: > > > > > >> On 09/15/2014 06:44 PM, Kim Phillips wrote: > > >> 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? > > > This is a convention that we decided early on '-' for directory names > > > and '_' for file names. > > based on what?
I agree with Kim here.
> > >>> +/** > > >>> + * @brief Management Complex firmware version information > > >>> + */ > > >>> +#define MC_VER_MAJOR 2 > > >>> +#define MC_VER_MINOR 0 > > >> > > >> code should be adjusted to run on all *compatible* versions of h/w, > > >> not strictly the one set in these defines. > > > This comment is not precise enough be actionable. > > > What exactly you want to be changed here? > > > > > > I think the easy thing to do is to convert the exact version check > > into a ranged version check: have minimum and maximum versions you > > support. Or a list of exact versions you support. Or not check for the > > version at all - or only for the major version and guarantee that the > > major version indicates backwards compatibility. > > > yes, this was my point: elsewhere I noticed the code denies to run > iff those defines are not matched exactly: that code should change > to run as Alex describes.
Also keep in mind that in the future we may need to support multiple versions that behave differently.
-Scott
| |