Re: [PATCH 1/2] staging: jnx: Juniper subsystem & board core APIs

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Oct 07, 2016 at 06:15:45PM +0300, Pantelis Antoniou wrote:
> From: Tom Kavanagh <tkavanagh@juniper.net>

Minor nit:

> +config JNX_DEVICES
> +	bool
> +	default n

n is always the default.

And how about a help entry?

> +if JNX_DEVICES
> +
> +menu "Juniper Devices and Infrastructure"
> +
> +config JNX_SYSTEM
> +	bool "Juniper System Infrastructure"
> +	default y

Only make something 'y' if you have to boot a bare-bones machine with
it.  That means almost no driver fits that category, and a staging
driver should never have this set either.

> +	help
> +	  This driver adds support for Juniper System Infrastructure. It creates
> +	  platform devices for the platform, chassis and cards and provides sysfs
> +	  attributes for these devices.
> +
> +	  This driver can not be compiled as a module.
> +
> +endmenu
> +
> +endif # JNX_DEVICES
> diff --git a/drivers/staging/jnx/Makefile b/drivers/staging/jnx/Makefile
> new file mode 100644
> index 0000000..52b8286
> --- /dev/null
> +++ b/drivers/staging/jnx/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for Juniper devices that really don't fit anywhere else.
> +#
> +
> +obj-$(CONFIG_JNX_SYSTEM)	+= jnx-subsys.o jnx-board-core.o
> diff --git a/drivers/staging/jnx/jnx-board-core.c b/drivers/staging/jnx/jnx-board-core.c
> new file mode 100644
> index 0000000..218a8b7
> --- /dev/null
> +++ b/drivers/staging/jnx/jnx-board-core.c
> @@ -0,0 +1,247 @@
> +/*
> + * Juniper Generic Board APIs
> + *
> + * Copyright (C) 2012, 2013, 2014 Juniper Networks. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.

Really "any later version"?  I have to ask.

> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/platform_data/at24.h>
> +#include <linux/slab.h>
> +#include <linux/jnx/jnx-subsys.h>
> +#include <linux/jnx/jnx-board-core.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/jnx-i2cs-core.h>
> +
> +#include "jnx-subsys-private.h"
> +
> +#define DRIVER_VERSION  "0.01.0"
> +#define DRIVER_DESC     "Board Generic HW"
> +
> +static LIST_HEAD(jnx_i2c_notify_list);
> +static DEFINE_MUTEX(jnx_i2c_notify_lock);
> +
> +static int jnx_i2c_adap_name_match(struct device *dev, void *data)
> +{
> +	struct i2c_adapter *adap = i2c_verify_adapter(dev);
> +	char *name = data;
> +
> +	if (!adap)
> +		return false;
> +
> +	return !strncmp(adap->name, name, strlen(name));
> +}
> +
> +struct i2c_adapter *jnx_i2c_find_adapter(char *name)
> +{
> +	struct device *dev;
> +	struct i2c_adapter *adap;
> +
> +	dev = bus_find_device(&i2c_bus_type, NULL, name,
> +			      jnx_i2c_adap_name_match);
> +	if (!dev)
> +		return NULL;
> +
> +	adap = i2c_verify_adapter(dev);
> +	if (!adap)
> +		put_device(dev);
> +
> +	return adap;
> +}
> +EXPORT_SYMBOL(jnx_i2c_find_adapter);

EXPORT_SYMBOL_GPL()?  Same for other exports.  I have to ask.

thanks,

greg k-h