Devicetree
 help / color / mirror / Atom feed
* Re: Devicetree Maintenance in barebox
From: Jon Loeliger @ 2014-02-09 17:58 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Sascha Hauer, Grant Likely, Ian Campbell,
	barebox-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140207141028.GT8533-u4khhh1J0LxI1Ri9qeTfzeTW4wlIGRCZ@public.gmane.org>

> Hi Sascha,
> 
> + Grant Likely, Ian Campbell, devicetree ML
> 
> Also, In the DT meeting earlier this week, Grant Likely said he has the
> request in to create a separate mailinglist for collaboration between
> the different devicetree users (BSD, Linux, etc).

...

> I think the proper solution will percolate out of the first
> cross-project discussions on the new ML.

...

> Definitely fodder for the new ML.
> 
> Grant, can you please add Sascha to the list of folks to notify when
> the new ML is ready?

I don't think there needs to be a different mailing list
in order to combine or discuss other OS's use of the device
tree compiler.  The  DTC is OS and Use-agnostic.  Discussions
of DTC needs for FreeBSD can happen right here as the orginal
purpose of this list was DTC discussion.

Are you, and Grant(?), suggesting that a separate list
should be created for FreeBSD use of DTS-file contents?
Or that DTS-file-content related discussions should be
separated from DTC discussions?


> imho, the goal is to not have any project tied to a specific version
> of the devicetree.
>
> iow, we don't break backwards compatibility in the
> devicetrees, and projects should revert to default behavior if new dt
> parameters are missing.  This means Linux and BSD shouldn't need to keep
> a current copy of the devicetree in their trees.  However, building the
> bootloader is a different animal.  It needs to provide the dt blob...

The devicetree source file format hasn't changed in years.
Yes, it is enhanced, but compatibly.  Or do you mean the
contents of the DTB for some specific platform?

Thanks,
jdl
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding
From: Guenter Roeck @ 2014-02-09 20:18 UTC (permalink / raw)
  To: Michal Simek, linux-kernel, monstr
  Cc: Wim Van Sebroeck, Grant Likely, Rob Herring, linux-watchdog,
	linux-arm-kernel, devicetree
In-Reply-To: <26d172891694e4041e73b7dae85a1848adf38034.1391177880.git.michal.simek@xilinx.com>

On 01/31/2014 06:18 AM, Michal Simek wrote:
> Use of_property_read_u32 functions to clean OF probing.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

^ permalink raw reply

* Re: [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Arnd Bergmann @ 2014-02-09 20:22 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: devicetree@vger.kernel.org, linaro-kernel, linux-pci, Will Deacon,
	LKML, Tanmay Inamdar, Catalin Marinas, Bjorn Helgaas, LAKML
In-Reply-To: <20140208142207.GR4993@e106497-lin.cambridge.arm.com>

On Saturday 08 February 2014, Liviu Dudau wrote:
> Yes, that parsing code of ranges is incorrect in light of the current discussion. I
> will try to propose a fix in the v2 series.
> 
> Regarding your PCI IO range: if your bus address starts at 0x8000_0000, would
> that not cause problems with devices that use less than 32bits for address
> decoding? It is a rather unusual setup, I believe the x86 world (even PCI spec?)
> mandates IO bus ranges in the first 16MB of address range?

Actually even less than that. The common location of the I/O aperture is
0x0000-0xffff in bus space, which comes from the way the x86 instruction
set handles it. You can have larger bus addresses, but you can easily get
into trouble that way. One limitation is that the total I/O space size for
all buses combined is 2MB on arm32 (to be extended to 16MB on arm64 as
it stands right now), the other is that we probably have code assuming that
io_offset is a non-negative number, i.e. the bus address is equal or less
than the logial I/O port number, which in turn is limited to a few MB.

	Arnd

^ permalink raw reply

* Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support
From: Jonathan Cameron @ 2014-02-09 21:48 UTC (permalink / raw)
  To: Matt Ranostay, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: matt.porter-QSEj5FYQhm4dnm+yROfE0A,
	pantelis.antoniou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1391670015-6551-3-git-send-email-mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 06/02/14 07:00, Matt Ranostay wrote:
> AS3935 chipset can detect lightning strikes and reports those back as
> events and the estimated distance to the storm.
>
> Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Matt,

Sorry for my somewhat late entry in the discussion of the interface for this
device, but I'm wondering if we don't have an opportunity to create a more
flexible interface for 'position' measurements.  Afterall sooner or later
we'll get a magetic position sensor or similar coming through.

So how about defining a new iio_chan_type IIO_POSITION and adding some
more modifiers to allow for polar coordinates (spherical coordinates I guess
given we are in 3D). For now we'd only need IIO_MOD_R though later I guess
we would have IIO_MOD_THETA and IIO_MOD_PHI.  That would in this case give
us.

in_position_r_raw with the unit after applying scaling being meters.

Clearly the ideal would have been to bring this suggestion up before we
had the proximity sensors but such is life.  Those tend to simply indicate
something is 'near' rather than at an explicity distance.  The difference
is clearly minor though.

What do people think of this approach?

Comments on the code inline. Note I review from the end (usually makes
more sense) so comments may only make sense in that direction!

Beware of the quirks of spi buses as highlighted inline.  You have to allow
for the buffers you provide being used directly for DMA transfers (unlike
say i2c which copies everything). As such there are some restrictions on
where these buffers must lie.

Jonathan
> ---
>   .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
>   drivers/iio/Kconfig                                |   1 +
>   drivers/iio/Makefile                               |   1 +
>   drivers/iio/proximity/Kconfig                      |  19 +
>   drivers/iio/proximity/Makefile                     |   6 +
>   drivers/iio/proximity/as3935.c                     | 437 +++++++++++++++++++++
>   6 files changed, 482 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>   create mode 100644 drivers/iio/proximity/Kconfig
>   create mode 100644 drivers/iio/proximity/Makefile
>   create mode 100644 drivers/iio/proximity/as3935.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935 b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
> new file mode 100644
> index 0000000..f6d9e6f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
> @@ -0,0 +1,18 @@
> +What		/sys/bus/iio/devices/iio:deviceX/in_proximity_raw
> +Date:		January 2014
> +KernelVersion:	3.15
> +Contact:	Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +Description:
> +		Get the current distance in kilometers of storm
> +		1    = storm overhead
> +		1-40 = distance in kilometers
> +		63   = out of range
This attribute should be a general purpose one given it will apply to lots
of other drivers over time.  Write it as such and distances should definitely
be in meters not kilometers.

> +
> +What		/sys/bus/iio/devices/iio:deviceX/gain_boost
> +Date:		January 2014
> +KernelVersion:	3.15
> +Contact:	Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +Description:
> +		Show or set the gain boost of the amp, from 0-31 range.
> +		18 = indoors (default)
> +		14 = outdoors
What does this gain boost actually mean?  I couldn't immediately tell from
the datasheet.  Does it effect the distance estimates, or is it about
sensitivity to detect them in the first place?  Even though you have defined
it just for this one device, we are always looking to generalize interfaces
so it is a good to have an idea of what it actually is.
   
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 5dd0e12..743485e 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -74,6 +74,7 @@ if IIO_TRIGGER
>      source "drivers/iio/trigger/Kconfig"
>   endif #IIO_TRIGGER
>   source "drivers/iio/pressure/Kconfig"
> +source "drivers/iio/proximity/Kconfig"
>   source "drivers/iio/temperature/Kconfig"
>
>   endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 887d390..698afc2 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -24,5 +24,6 @@ obj-y += light/
>   obj-y += magnetometer/
>   obj-y += orientation/
>   obj-y += pressure/
> +obj-y += proximity/
>   obj-y += temperature/
>   obj-y += trigger/
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> new file mode 100644
> index 0000000..0c8cdf5
> --- /dev/null
> +++ b/drivers/iio/proximity/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Proximity sensors
> +#
> +
> +menu "Lightning sensors"
> +
> +config AS3935
> +	tristate "AS3935 Franklin lightning sensor"
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	depends on SPI
> +	help
> +	  Say Y here to build SPI interface support for the Austrian
> +	  Microsystems AS3935 lightning detection sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called as3935
> +
> +endmenu
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> new file mode 100644
> index 0000000..743adee
> --- /dev/null
> +++ b/drivers/iio/proximity/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for IIO proximity sensors
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_AS3935)		+= as3935.o
> diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
> new file mode 100644
> index 0000000..109759e
> --- /dev/null
> +++ b/drivers/iio/proximity/as3935.c
> @@ -0,0 +1,437 @@
> +/*
> + * as3935.c - Support for AS3935 Franklin lightning sensor
> + *
> + * Copyright (C) 2014 Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * 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.
> + *
> + * 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/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/workqueue.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/of_gpio.h>
> +
> +
> +#define AS3935_AFE_GAIN		0x00
> +#define AS3935_AFE_MASK		0x3F
> +#define AS3935_AFE_GAIN_MAX	0x1F
> +#define AS3935_AFE_PWR_BIT	BIT(0)
> +
> +#define AS3935_INT		0x03
> +#define AS3935_INT_MASK		0x07
> +#define AS3935_EVENT_INT	BIT(3)
> +#define AS3935_NOISE_INT	BIT(1)
> +
> +#define AS3935_DATA		0x07
> +#define AS3935_DATA_MASK	0x3F
> +
> +#define AS3935_TUNE_CAP		0x08
> +#define AS3935_CALIBRATE	0x3D
> +
> +#define AS3935_WRITE_DATA	BIT(15)
> +#define AS3935_READ_DATA	BIT(14)
((x) << 8) to protect against strange elements being passed in as x.
Obviously you have this tightly controlled here, but who knows what others
might add in future, so best to do it right!
> +#define AS3935_ADDRESS(x)	(x<<8)
> +
> +struct as3935_state {
> +	struct spi_device *spi;
> +	struct iio_trigger *trig;
> +	struct mutex lock;
> +	struct delayed_work work;
> +
> +	u32 tune_cap;
> +};
> +
> +static const struct iio_chan_spec as3935_channels[] = {
> +	{
> +		.type           = IIO_PROXIMITY,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW),
> +		.scan_index     = 0,
> +		.scan_type = {
> +			.sign           = 'u',
> +			.realbits       = 6,
> +			.storagebits    = 8,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
This looks very similar to val = spi_w8r8(st->spi, reg);

> +static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
> +{
> +	u8 tx, rx;
Spi buffers should never be on the stack. You can't guarantee alignment and
they must be cache aligned.  spi_w8r8 deals with this by using a small
buffer allocated on the heap.  The alternative is to carefully allocate
a cache line aligned buffer in your state structure.  Here I'd definitely
use spi_w8r8 though!
> +	int ret;
> +
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = &tx,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		}, {
> +			.rx_buf = &rx,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		},
> +	};
> +	tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +	*val = rx;
> +
> +	return ret;
> +};
> +

> +static int as3935_write(struct as3935_state *st,
> +				unsigned int reg,
> +				unsigned int val)
> +{
> +	u8 buf[2];
You could use a C99 style asignment here (entirely optional!)
e.g.
	u8 buf[2] = { (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8,
	   	      val };
However, this buffer must be cacheline aligned seeing as spi_write
does not use a suitably aligned bounce buffer (unlike spi_w8r8 which does).
This can cause nasty, difficult to track down corruptions with spi controllers
that do DMA.
> +
> +	buf[0] = (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8;
> +	buf[1] = val;
> +
> +	return spi_write(st->spi, (u8 *) &buf, 2);
> +};
> +
> +static ssize_t as3935_gain_boost_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
> +	val = (val & AS3935_AFE_MASK) >> 1;
> +
> +	return sprintf(buf, "%d\n", val);
> +};
> +
> +static ssize_t as3935_gain_boost_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul((const char *) buf, 10, &val);
> +	if (ret)
> +		return -EINVAL;
> +
> +	if (val > AS3935_AFE_GAIN_MAX)
> +		return -EINVAL;
> +
> +	as3935_write(st, AS3935_AFE_GAIN, val << 1);
> +
> +	return len;
> +};
> +
> +static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
> +	as3935_gain_boost_show, as3935_gain_boost_store, 0);
> +
> +
> +static struct attribute *as3935_attributes[] = {
> +	&iio_dev_attr_gain_boost.dev_attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group as3935_attribute_group = {
> +	.attrs = as3935_attributes,
> +};
> +
> +static int as3935_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (m != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	*val2 = 0;
> +	ret = as3935_read(st, AS3935_DATA, val);
> +	if (ret)
> +		return ret;
> +	return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info as3935_info = {
> +	.driver_module = THIS_MODULE,
> +	.attrs = &as3935_attribute_group,
> +	.read_raw = &as3935_read_raw,
> +};
> +
> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
> +	.postenable = &iio_triggered_buffer_postenable,
> +	.predisable = &iio_triggered_buffer_predisable,
> +};
> +
> +static irqreturn_t as3935_trigger_handler(int irq, void *private)
> +{
> +	struct iio_poll_func *pf = private;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = as3935_read(st, AS3935_DATA, &val);
> +	if (ret)
> +		goto err_read;
> +	val &= AS3935_DATA_MASK;
This timestamp is probably rather late given I think the interrupt indicated
the actual point in time?
> +	iio_push_to_buffers_with_timestamp(indio_dev, &val, iio_get_time_ns());
> +err_read:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +};
> +
> +static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
> +	.owner = THIS_MODULE,
> +};
> +
> +static void as3935_event_work(struct work_struct *work)
> +{
> +	struct as3935_state *st;
> +	struct spi_device *spi;
> +	int val;
> +
> +	st = container_of(work, struct as3935_state, work.work);
> +	spi = st->spi;
Don't bother with the local variable for spi. It doesn't add any clarity.
> +
> +	as3935_read(st, AS3935_INT, &val);
> +	val &= AS3935_INT_MASK;
> +
> +	switch (val) {
> +	case AS3935_EVENT_INT:
You could save a timestamp in the interrupt handler then pass it on here.
> +		iio_trigger_poll(st->trig, 0);
> +		break;
> +	case AS3935_NOISE_INT:
Hmm. This brings me back to one of my favourite kernel questions,
'Why isn't there a better way of handling hardware errors?'
There isn't as far as I know!
> +		dev_warn(&spi->dev, "noise level is too high");
> +		break;
> +	}
> +};
> +
> +static irqreturn_t as3935_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
This is 'unusual' so can you add a comment explaining what is going on here.
My guess is that you want to avoid grabing new data for a short window after
a strike is detected?  The cancel is to avoid running the delayed work twice?

If so the reason a sleep in a threaded handler wouldn't work is that it would
not 'cancel' if new data arrived I guess.  You still have a window for problems
if the interrupt occurs whilst the work queue is actually running the
scheduled work.

I'm wondering if this is overcomplicating things and it is better to just
occasionally handle an extra interrupt and do this with a conventional
threaded interrupt handler.  Of course I may be missing something.
> +	cancel_delayed_work(&st->work);
I'm guessing this should be msecs_to_jiffies seeing as schedule_delayed_work
takes a value in jiffies.
> +	schedule_delayed_work(&st->work, jiffies_to_msecs(3));
blank line here.
> +	return IRQ_HANDLED;
> +}
> +
> +static void calibrate_as3935(struct as3935_state *st)
> +{
> +	mutex_lock(&st->lock);
> +
> +	/* mask disturber interrupt bit */
> +	as3935_write(st, AS3935_INT, 1 << 5);
> +
> +	as3935_write(st, AS3935_CALIBRATE, 0x96);
> +	as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
> +
> +	mdelay(2);
> +	as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
> +
> +	mutex_unlock(&st->lock);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	mutex_lock(&st->lock);
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
Mutex not released.

> +	val |= AS3935_AFE_PWR_BIT;
> +
> +	ret = as3935_write(st, AS3935_AFE_GAIN, val);
> +	mutex_unlock(&st->lock);
> +	return ret;
> +}
> +
> +static int as3935_resume(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	mutex_lock(&st->lock);
> +	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
> +	if (ret)
> +		return ret;
You haven't released the mutex in this error path.  This is a classic
case for using a goto rather than returning directly here.

> +	val &= ~AS3935_AFE_PWR_BIT;
> +	ret = as3935_write(st, AS3935_AFE_GAIN, val);
> +	mutex_unlock(&st->lock);
blank line before returns in simple cases like this.  Just makes things
ever so slightly easier to read ;)
> +	return ret;
> +}
> +#else
> +#define as3935_suspend	NULL
> +#define as3935_resume	NULL
> +#endif
> +
> +static int as3935_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct iio_trigger *trig;
> +	struct as3935_state *st;
> +	struct device_node *np = spi->dev.of_node;
> +	int ret;
> +
/* Make sure the lightning event interrupt is specified. */
You probably don't need to do this as the request will fail if supplied
with a 0. Even if you want to keep this, it would be cleaner to have it
down where the request for the irq is made rather than here at the start.
> +	/* Be sure lightning event interrupt */
> +	if (!spi->irq) {
> +		dev_err(&spi->dev, "unable to get event interrupt\n");
> +		return -EINVAL;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	st->tune_cap = 0;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	mutex_init(&st->lock);
> +	INIT_DELAYED_WORK(&st->work, as3935_event_work);
> +
> +	ret = of_property_read_u32(np, "ams,tune-cap", &st->tune_cap);
> +	if (ret) {
> +		st->tune_cap = 0;
> +		dev_warn(&spi->dev,
> +			"no tune-cap set, defaulting to %d", st->tune_cap);
> +	}
> +
> +	if (st->tune_cap > 15) {
> +		dev_err(&spi->dev,
> +			"wrong tune-cap setting of %d\n", st->tune_cap);
> +		return -EINVAL;
> +	}
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->channels = as3935_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &as3935_info;
> +
> +	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> +				      indio_dev->name, indio_dev->id);
> +
> +	if (!trig)
> +		return -ENOMEM;
> +
> +	st->trig = trig;
> +	trig->dev.parent = indio_dev->dev.parent;
> +	iio_trigger_set_drvdata(trig, indio_dev);
> +	trig->ops = &iio_interrupt_trigger_ops;
> +
> +	ret = iio_trigger_register(trig);
> +	if (ret) {
> +		dev_err(&spi->dev, "failed to register trigger\n");
> +		return ret;
> +	}
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					&as3935_trigger_handler,
> +					&iio_triggered_buffer_setup_ops);
> +
> +	if (ret) {
> +		dev_err(&spi->dev, "cannot setup iio trigger\n");
> +		goto unregister_trigger;
> +	}
> +
> +	calibrate_as3935(st);
> +
> +	ret = devm_request_irq(&spi->dev, spi->irq,
> +				&as3935_interrupt_handler,
> +				IRQF_TRIGGER_RISING,
> +				dev_name(&spi->dev),
> +				indio_dev);
> +
> +	if (ret) {
> +		dev_err(&spi->dev, "unable to request irq\n");
> +		goto unregister_trigger;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "unable to register device\n");
> +		goto unregister_trigger;
> +	}
> +	return 0;
> +
> +unregister_trigger:
> +	iio_trigger_unregister(st->trig);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	return ret;
> +};
> +
> +static int as3935_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct as3935_state *st = iio_priv(indio_dev);
> +
> +	iio_trigger_unregister(st->trig);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +	iio_device_unregister(indio_dev);
This should be in the reverse order of what goes on in the probe.
This helps both for review purposes and to avoid actual bugs. Here for
example, the buffer has been destroyed before the userspace interface
is removed in the device_unregister.

> +
> +	return 0;
> +};
> +
> +static const struct spi_device_id as3935_id[] = {
> +	{"as3935", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, as3935_id);
> +
> +static struct spi_driver as3935_driver = {
> +	.driver = {
> +		.name	= "as3935",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= as3935_probe,
> +	.remove		= as3935_remove,
> +	.id_table	= as3935_id,
> +	.suspend	= as3935_suspend,
> +	.resume		= as3935_resume,
> +};
> +module_spi_driver(as3935_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("AS3935 lightning sensor");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("spi:as3935");
>

^ permalink raw reply

* Re: [PATCH 1/2] iio:as3935: Add DT binding docs for AS3935 driver
From: Jonathan Cameron @ 2014-02-09 21:54 UTC (permalink / raw)
  To: Matt Ranostay, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: matt.porter-QSEj5FYQhm4dnm+yROfE0A,
	pantelis.antoniou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1391670015-6551-2-git-send-email-mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 06/02/14 07:00, Matt Ranostay wrote:
> Document compatible string, required and optional DT properties for
> AS3935 chipset driver.
>
> Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   .../devicetree/bindings/iio/proximity/as3935.txt   | 28 ++++++++++++++++++++++
>   .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>   2 files changed, 29 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iio/proximity/as3935.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/proximity/as3935.txt b/Documentation/devicetree/bindings/iio/proximity/as3935.txt
> new file mode 100644
> index 0000000..574d49c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/as3935.txt
> @@ -0,0 +1,28 @@
> +Austrian Microsystems AS3935 Franklin lightning sensor device driver
> +
> +Required properties:
> +	- compatible: must be "ams,as3935"
> +	- reg: SPI chip select number for the device
> +	- spi-cpha: SPI Mode 1. Refer to spi/spi-bus.txt for generic SPI
> +	slave node bindings.
> +	- interrupt-parent : should be the phandle for the interrupt controller
> +	- interrupts : interrupt mapping for GPIO IRQ
Why does it need to be a GPIO IRQ?  As far as I can see any irq will do
(note there are devices that provide suitable interrupts without the ability
to read the level).  Oops, we have a couple of other IIO bindings stating
exactly this.  I doubt they need to be GPIOs either.

> +
> +	Refer to interrupt-controller/interrupts.txt for generic
> +	interrupt client node bindings.
> +
> +Optional properties:
> +	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 15.
> +	  Range of 0 to 120 pF, 8pF steps. This will require using the
> +	  calibration data from the manufacturer.
Why not have this as the actual value rather than a magic number?  Might
as well keep the device tree as human readable as possible.
> +
> +Example:
> +
> +as3935@0 {
> +	compatible = "ams,as3935";
> +	reg = <0>;
> +	spi-cpha;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <16 1>;
> +	ams,tune-cap = <10>;
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 3c31b40..9dd66ca 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -10,6 +10,7 @@ aeroflexgaisler	Aeroflex Gaisler AB
>   ak	Asahi Kasei Corp.
>   altr	Altera Corp.
>   amcc	Applied Micro Circuits Corporation (APM, formally AMCC)
> +ams	AMS AG
>   amstaos	AMS-Taos Inc.
>   apm	Applied Micro Circuits Corporation (APM)
>   arm	ARM Ltd.
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] iio:as3935: Add DT binding docs for AS3935 driver
From: Matt Ranostay @ 2014-02-09 23:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Matt Porter, Pantelis Antoniou
In-Reply-To: <52F7F92E.9040508-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Sun, Feb 9, 2014 at 1:54 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On 06/02/14 07:00, Matt Ranostay wrote:
>>
>> Document compatible string, required and optional DT properties for
>> AS3935 chipset driver.
>>
>> Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   .../devicetree/bindings/iio/proximity/as3935.txt   | 28
>> ++++++++++++++++++++++
>>   .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>>   2 files changed, 29 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/iio/proximity/as3935.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/proximity/as3935.txt
>> b/Documentation/devicetree/bindings/iio/proximity/as3935.txt
>> new file mode 100644
>> index 0000000..574d49c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/proximity/as3935.txt
>> @@ -0,0 +1,28 @@
>> +Austrian Microsystems AS3935 Franklin lightning sensor device driver
>> +
>> +Required properties:
>> +       - compatible: must be "ams,as3935"
>> +       - reg: SPI chip select number for the device
>> +       - spi-cpha: SPI Mode 1. Refer to spi/spi-bus.txt for generic SPI
>> +       slave node bindings.
>> +       - interrupt-parent : should be the phandle for the interrupt
>> controller
>> +       - interrupts : interrupt mapping for GPIO IRQ
>
> Why does it need to be a GPIO IRQ?  As far as I can see any irq will do
> (note there are devices that provide suitable interrupts without the ability
> to read the level).  Oops, we have a couple of other IIO bindings stating
> exactly this.  I doubt they need to be GPIOs either.
>
No it doesn't it can be any interrupt type... Will reword better


>
>> +
>> +       Refer to interrupt-controller/interrupts.txt for generic
>> +       interrupt client node bindings.
>> +
>> +Optional properties:
>> +       - ams,tune-cap: Calibration tuning capacitor stepping value 0 -
>> 15.
>> +         Range of 0 to 120 pF, 8pF steps. This will require using the
>> +         calibration data from the manufacturer.
>
> Why not have this as the actual value rather than a magic number?  Might
> as well keep the device tree as human readable as possible.
>

Makes sense.

>> +
>> +Example:
>> +
>> +as3935@0 {
>> +       compatible = "ams,as3935";
>> +       reg = <0>;
>> +       spi-cpha;
>> +       interrupt-parent = <&gpio1>;
>> +       interrupts = <16 1>;
>> +       ams,tune-cap = <10>;
>> +};
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index 3c31b40..9dd66ca 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -10,6 +10,7 @@ aeroflexgaisler       Aeroflex Gaisler AB
>>   ak    Asahi Kasei Corp.
>>   altr  Altera Corp.
>>   amcc  Applied Micro Circuits Corporation (APM, formally AMCC)
>> +ams    AMS AG
>>   amstaos       AMS-Taos Inc.
>>   apm   Applied Micro Circuits Corporation (APM)
>>   arm   ARM Ltd.
>>
>

^ permalink raw reply

* Re: [PATCH 2/2] iio: Add AS3935 lightning sensor support
From: Matt Ranostay @ 2014-02-09 23:32 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Matt Porter, Pantelis Antoniou
In-Reply-To: <52F7F79C.1010001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Sun, Feb 9, 2014 at 1:48 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On 06/02/14 07:00, Matt Ranostay wrote:
>>
>> AS3935 chipset can detect lightning strikes and reports those back as
>> events and the estimated distance to the storm.
>>
>> Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Hi Matt,
>
> Sorry for my somewhat late entry in the discussion of the interface for this
> device, but I'm wondering if we don't have an opportunity to create a more
> flexible interface for 'position' measurements.  Afterall sooner or later
> we'll get a magetic position sensor or similar coming through.
>
> So how about defining a new iio_chan_type IIO_POSITION and adding some
> more modifiers to allow for polar coordinates (spherical coordinates I guess
> given we are in 3D). For now we'd only need IIO_MOD_R though later I guess
> we would have IIO_MOD_THETA and IIO_MOD_PHI.  That would in this case give
> us.
>
> in_position_r_raw with the unit after applying scaling being meters.
>
> Clearly the ideal would have been to bring this suggestion up before we
> had the proximity sensors but such is life.  Those tend to simply indicate
> something is 'near' rather than at an explicity distance.  The difference
> is clearly minor though.
>
> What do people think of this approach?
>
> Comments on the code inline. Note I review from the end (usually makes
> more sense) so comments may only make sense in that direction!
>
> Beware of the quirks of spi buses as highlighted inline.  You have to allow
> for the buffers you provide being used directly for DMA transfers (unlike
> say i2c which copies everything). As such there are some restrictions on
> where these buffers must lie.
>
> Jonathan
>
>> ---
>>   .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
>>   drivers/iio/Kconfig                                |   1 +
>>   drivers/iio/Makefile                               |   1 +
>>   drivers/iio/proximity/Kconfig                      |  19 +
>>   drivers/iio/proximity/Makefile                     |   6 +
>>   drivers/iio/proximity/as3935.c                     | 437
>> +++++++++++++++++++++
>>   6 files changed, 482 insertions(+)
>>   create mode 100644
>> Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>>   create mode 100644 drivers/iio/proximity/Kconfig
>>   create mode 100644 drivers/iio/proximity/Makefile
>>   create mode 100644 drivers/iio/proximity/as3935.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>> b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>> new file mode 100644
>> index 0000000..f6d9e6f
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>> @@ -0,0 +1,18 @@
>> +What           /sys/bus/iio/devices/iio:deviceX/in_proximity_raw
>> +Date:          January 2014
>> +KernelVersion: 3.15
>> +Contact:       Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> +Description:
>> +               Get the current distance in kilometers of storm
>> +               1    = storm overhead
>> +               1-40 = distance in kilometers
>> +               63   = out of range
>
> This attribute should be a general purpose one given it will apply to lots
> of other drivers over time.  Write it as such and distances should
> definitely
> be in meters not kilometers.
>
>
>> +
>> +What           /sys/bus/iio/devices/iio:deviceX/gain_boost
>> +Date:          January 2014
>> +KernelVersion: 3.15
>> +Contact:       Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> +Description:
>> +               Show or set the gain boost of the amp, from 0-31 range.
>> +               18 = indoors (default)
>> +               14 = outdoors
>
> What does this gain boost actually mean?  I couldn't immediately tell from
> the datasheet.  Does it effect the distance estimates, or is it about
> sensitivity to detect them in the first place?  Even though you have defined
> it just for this one device, we are always looking to generalize interfaces
> so it is a good to have an idea of what it actually is.
>

Controls the goo

>
>>
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 5dd0e12..743485e 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -74,6 +74,7 @@ if IIO_TRIGGER
>>      source "drivers/iio/trigger/Kconfig"
>>   endif #IIO_TRIGGER
>>   source "drivers/iio/pressure/Kconfig"
>> +source "drivers/iio/proximity/Kconfig"
>>   source "drivers/iio/temperature/Kconfig"
>>
>>   endif # IIO
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 887d390..698afc2 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -24,5 +24,6 @@ obj-y += light/
>>   obj-y += magnetometer/
>>   obj-y += orientation/
>>   obj-y += pressure/
>> +obj-y += proximity/
>>   obj-y += temperature/
>>   obj-y += trigger/
>> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
>> new file mode 100644
>> index 0000000..0c8cdf5
>> --- /dev/null
>> +++ b/drivers/iio/proximity/Kconfig
>> @@ -0,0 +1,19 @@
>> +#
>> +# Proximity sensors
>> +#
>> +
>> +menu "Lightning sensors"
>> +
>> +config AS3935
>> +       tristate "AS3935 Franklin lightning sensor"
>> +       select IIO_BUFFER
>> +       select IIO_TRIGGERED_BUFFER
>> +       depends on SPI
>> +       help
>> +         Say Y here to build SPI interface support for the Austrian
>> +         Microsystems AS3935 lightning detection sensor.
>> +
>> +         To compile this driver as a module, choose M here: the
>> +         module will be called as3935
>> +
>> +endmenu
>> diff --git a/drivers/iio/proximity/Makefile
>> b/drivers/iio/proximity/Makefile
>> new file mode 100644
>> index 0000000..743adee
>> --- /dev/null
>> +++ b/drivers/iio/proximity/Makefile
>> @@ -0,0 +1,6 @@
>> +#
>> +# Makefile for IIO proximity sensors
>> +#
>> +
>> +# When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_AS3935)           += as3935.o
>> diff --git a/drivers/iio/proximity/as3935.c
>> b/drivers/iio/proximity/as3935.c
>> new file mode 100644
>> index 0000000..109759e
>> --- /dev/null
>> +++ b/drivers/iio/proximity/as3935.c
>> @@ -0,0 +1,437 @@
>> +/*
>> + * as3935.c - Support for AS3935 Franklin lightning sensor
>> + *
>> + * Copyright (C) 2014 Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> + *
>> + * 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.
>> + *
>> + * 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/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/irq.h>
>> +#include <linux/gpio.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/of_gpio.h>
>> +
>> +
>> +#define AS3935_AFE_GAIN                0x00
>> +#define AS3935_AFE_MASK                0x3F
>> +#define AS3935_AFE_GAIN_MAX    0x1F
>> +#define AS3935_AFE_PWR_BIT     BIT(0)
>> +
>> +#define AS3935_INT             0x03
>> +#define AS3935_INT_MASK                0x07
>> +#define AS3935_EVENT_INT       BIT(3)
>> +#define AS3935_NOISE_INT       BIT(1)
>> +
>> +#define AS3935_DATA            0x07
>> +#define AS3935_DATA_MASK       0x3F
>> +
>> +#define AS3935_TUNE_CAP                0x08
>> +#define AS3935_CALIBRATE       0x3D
>> +
>> +#define AS3935_WRITE_DATA      BIT(15)
>> +#define AS3935_READ_DATA       BIT(14)
>
> ((x) << 8) to protect against strange elements being passed in as x.
> Obviously you have this tightly controlled here, but who knows what others
> might add in future, so best to do it right!
>
>> +#define AS3935_ADDRESS(x)      (x<<8)
>> +
>> +struct as3935_state {
>> +       struct spi_device *spi;
>> +       struct iio_trigger *trig;
>> +       struct mutex lock;
>> +       struct delayed_work work;
>> +
>> +       u32 tune_cap;
>> +};
>> +
>> +static const struct iio_chan_spec as3935_channels[] = {
>> +       {
>> +               .type           = IIO_PROXIMITY,
>> +               .info_mask_separate =
>> +                       BIT(IIO_CHAN_INFO_RAW),
>> +               .scan_index     = 0,
>> +               .scan_type = {
>> +                       .sign           = 'u',
>> +                       .realbits       = 6,
>> +                       .storagebits    = 8,
>> +               },
>> +       },
>> +       IIO_CHAN_SOFT_TIMESTAMP(1),
>> +};
>> +
>
> This looks very similar to val = spi_w8r8(st->spi, reg);
>
>
>> +static int as3935_read(struct as3935_state *st, unsigned int reg, int
>> *val)
>> +{
>> +       u8 tx, rx;
>
> Spi buffers should never be on the stack. You can't guarantee alignment and
> they must be cache aligned.  spi_w8r8 deals with this by using a small
> buffer allocated on the heap.  The alternative is to carefully allocate
> a cache line aligned buffer in your state structure.  Here I'd definitely
> use spi_w8r8 though!
>

Noted will do the former.. having it the in the as3935_state struct
would make sense if the buffer was needed all over.

>> +       int ret;
>> +
>> +       struct spi_transfer xfers[] = {
>> +               {
>> +                       .tx_buf = &tx,
>> +                       .bits_per_word = 8,
>> +                       .len = 1,
>> +               }, {
>> +                       .rx_buf = &rx,
>> +                       .bits_per_word = 8,
>> +                       .len = 1,
>> +               },
>> +       };
>> +       tx = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
>> +
>> +       ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
>> +       *val = rx;
>> +
>> +       return ret;
>> +};
>> +
>
>
>> +static int as3935_write(struct as3935_state *st,
>> +                               unsigned int reg,
>> +                               unsigned int val)
>> +{
>> +       u8 buf[2];
>
> You could use a C99 style asignment here (entirely optional!)
> e.g.
>         u8 buf[2] = { (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8,
>                       val };
> However, this buffer must be cacheline aligned seeing as spi_write
> does not use a suitably aligned bounce buffer (unlike spi_w8r8 which does).
> This can cause nasty, difficult to track down corruptions with spi
> controllers
> that do DMA.
>
>> +
>> +       buf[0] = (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8;
>> +       buf[1] = val;
>> +
>> +       return spi_write(st->spi, (u8 *) &buf, 2);
>> +};
>> +
>> +static ssize_t as3935_gain_boost_show(struct device *dev,
>> +                               struct device_attribute *attr,
>> +                               char *buf)
>> +{
>> +       struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
>> +       int val, ret;
>> +
>> +       ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +       if (ret)
>> +               return ret;
>> +       val = (val & AS3935_AFE_MASK) >> 1;
>> +
>> +       return sprintf(buf, "%d\n", val);
>> +};
>> +
>> +static ssize_t as3935_gain_boost_store(struct device *dev,
>> +                                       struct device_attribute *attr,
>> +                                       const char *buf, size_t len)
>> +{
>> +       struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
>> +       unsigned long val;
>> +       int ret;
>> +
>> +       ret = kstrtoul((const char *) buf, 10, &val);
>> +       if (ret)
>> +               return -EINVAL;
>> +
>> +       if (val > AS3935_AFE_GAIN_MAX)
>> +               return -EINVAL;
>> +
>> +       as3935_write(st, AS3935_AFE_GAIN, val << 1);
>> +
>> +       return len;
>> +};
>> +
>> +static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
>> +       as3935_gain_boost_show, as3935_gain_boost_store, 0);
>> +
>> +
>> +static struct attribute *as3935_attributes[] = {
>> +       &iio_dev_attr_gain_boost.dev_attr.attr,
>> +       NULL,
>> +};
>> +
>> +static struct attribute_group as3935_attribute_group = {
>> +       .attrs = as3935_attributes,
>> +};
>> +
>> +static int as3935_read_raw(struct iio_dev *indio_dev,
>> +                          struct iio_chan_spec const *chan,
>> +                          int *val,
>> +                          int *val2,
>> +                          long m)
>> +{
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int ret;
>> +
>> +       if (m != IIO_CHAN_INFO_RAW)
>> +               return -EINVAL;
>> +
>> +       *val2 = 0;
>> +       ret = as3935_read(st, AS3935_DATA, val);
>> +       if (ret)
>> +               return ret;
>> +       return IIO_VAL_INT;
>> +}
>> +
>> +static const struct iio_info as3935_info = {
>> +       .driver_module = THIS_MODULE,
>> +       .attrs = &as3935_attribute_group,
>> +       .read_raw = &as3935_read_raw,
>> +};
>> +
>> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops =
>> {
>> +       .postenable = &iio_triggered_buffer_postenable,
>> +       .predisable = &iio_triggered_buffer_predisable,
>> +};
>> +
>> +static irqreturn_t as3935_trigger_handler(int irq, void *private)
>> +{
>> +       struct iio_poll_func *pf = private;
>> +       struct iio_dev *indio_dev = pf->indio_dev;
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int val, ret;
>> +
>> +       ret = as3935_read(st, AS3935_DATA, &val);
>> +       if (ret)
>> +               goto err_read;
>> +       val &= AS3935_DATA_MASK;
>
> This timestamp is probably rather late given I think the interrupt indicated
> the actual point in time?
>
>> +       iio_push_to_buffers_with_timestamp(indio_dev, &val,
>> iio_get_time_ns());
>> +err_read:
>> +       iio_trigger_notify_done(indio_dev->trig);
>> +
>> +       return IRQ_HANDLED;
>> +};
>> +
>> +static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
>> +       .owner = THIS_MODULE,
>> +};
>> +
>> +static void as3935_event_work(struct work_struct *work)
>> +{
>> +       struct as3935_state *st;
>> +       struct spi_device *spi;
>> +       int val;
>> +
>> +       st = container_of(work, struct as3935_state, work.work);
>> +       spi = st->spi;
>
> Don't bother with the local variable for spi. It doesn't add any clarity.
>
>> +
>> +       as3935_read(st, AS3935_INT, &val);
>> +       val &= AS3935_INT_MASK;
>> +
>> +       switch (val) {
>> +       case AS3935_EVENT_INT:
>
> You could save a timestamp in the interrupt handler then pass it on here.
>
Noted.

>> +               iio_trigger_poll(st->trig, 0);
>> +               break;
>> +       case AS3935_NOISE_INT:
>
> Hmm. This brings me back to one of my favourite kernel questions,
> 'Why isn't there a better way of handling hardware errors?'
> There isn't as far as I know!
>
>> +               dev_warn(&spi->dev, "noise level is too high");
>> +               break;
>> +       }
>> +};
>> +
>> +static irqreturn_t as3935_interrupt_handler(int irq, void *private)
>> +{
>> +       struct iio_dev *indio_dev = private;
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +
>
> This is 'unusual' so can you add a comment explaining what is going on here.
> My guess is that you want to avoid grabing new data for a short window after
> a strike is detected?  The cancel is to avoid running the delayed work
> twice?
>
> If so the reason a sleep in a threaded handler wouldn't work is that it
> would
> not 'cancel' if new data arrived I guess.  You still have a window for
> problems
> if the interrupt occurs whilst the work queue is actually running the
> scheduled work.
>
> I'm wondering if this is overcomplicating things and it is better to just
> occasionally handle an extra interrupt and do this with a conventional
> threaded interrupt handler.  Of course I may be missing something.
>>
>> +       cancel_delayed_work(&st->work);
>
> I'm guessing this should be msecs_to_jiffies seeing as schedule_delayed_work
> takes a value in jiffies.
>>
>> +       schedule_delayed_work(&st->work, jiffies_to_msecs(3));
>

Yes it should be msec_to_jiffies(3) :/
Noted the cancel_delayed_work() probably is verbose and will drop in
the next version.

Reason for this is the datasheet requires (suggests?) a 2+ millisecond
delay before reading
estimated distance value.

> blank line here.
>
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static void calibrate_as3935(struct as3935_state *st)
>> +{
>> +       mutex_lock(&st->lock);
>> +
>> +       /* mask disturber interrupt bit */
>> +       as3935_write(st, AS3935_INT, 1 << 5);
>> +
>> +       as3935_write(st, AS3935_CALIBRATE, 0x96);
>> +       as3935_write(st, AS3935_TUNE_CAP, 1 << 5 | st->tune_cap);
>> +
>> +       mdelay(2);
>> +       as3935_write(st, AS3935_TUNE_CAP, st->tune_cap);
>> +
>> +       mutex_unlock(&st->lock);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
>> +{
>> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int val, ret;
>> +
>> +       mutex_lock(&st->lock);
>> +       ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +       if (ret)
>> +               return ret;
>
> Mutex not released.
>
>
>> +       val |= AS3935_AFE_PWR_BIT;
>> +
>> +       ret = as3935_write(st, AS3935_AFE_GAIN, val);
>> +       mutex_unlock(&st->lock);
>> +       return ret;
>> +}
>> +
>> +static int as3935_resume(struct spi_device *spi)
>> +{
>> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +       int val, ret;
>> +
>> +       mutex_lock(&st->lock);
>> +       ret = as3935_read(st, AS3935_AFE_GAIN, &val);
>> +       if (ret)
>> +               return ret;
>
> You haven't released the mutex in this error path.  This is a classic
> case for using a goto rather than returning directly here.
>

Yeah this a rookie mistake on my behalf.

>
>> +       val &= ~AS3935_AFE_PWR_BIT;
>> +       ret = as3935_write(st, AS3935_AFE_GAIN, val);
>> +       mutex_unlock(&st->lock);
>
> blank line before returns in simple cases like this.  Just makes things
> ever so slightly easier to read ;)
>>
>> +       return ret;
>> +}
>>
>> +#else
>> +#define as3935_suspend NULL
>> +#define as3935_resume  NULL
>> +#endif
>> +
>> +static int as3935_probe(struct spi_device *spi)
>> +{
>> +       struct iio_dev *indio_dev;
>> +       struct iio_trigger *trig;
>> +       struct as3935_state *st;
>> +       struct device_node *np = spi->dev.of_node;
>> +       int ret;
>> +
>
> /* Make sure the lightning event interrupt is specified. */
> You probably don't need to do this as the request will fail if supplied
> with a 0. Even if you want to keep this, it would be cleaner to have it
> down where the request for the irq is made rather than here at the start.
>

Didn't know that.. all for no

>> +       /* Be sure lightning event interrupt */
>> +       if (!spi->irq) {
>> +               dev_err(&spi->dev, "unable to get event interrupt\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
>> +       if (!indio_dev)
>> +               return -ENOMEM;
>> +
>> +       st = iio_priv(indio_dev);
>> +       st->spi = spi;
>> +       st->tune_cap = 0;
>> +
>> +       spi_set_drvdata(spi, indio_dev);
>> +       mutex_init(&st->lock);
>> +       INIT_DELAYED_WORK(&st->work, as3935_event_work);
>> +
>> +       ret = of_property_read_u32(np, "ams,tune-cap", &st->tune_cap);
>> +       if (ret) {
>> +               st->tune_cap = 0;
>> +               dev_warn(&spi->dev,
>> +                       "no tune-cap set, defaulting to %d",
>> st->tune_cap);
>> +       }
>> +
>> +       if (st->tune_cap > 15) {
>> +               dev_err(&spi->dev,
>> +                       "wrong tune-cap setting of %d\n", st->tune_cap);
>> +               return -EINVAL;
>> +       }
>> +
>> +       indio_dev->dev.parent = &spi->dev;
>> +       indio_dev->name = spi_get_device_id(spi)->name;
>> +       indio_dev->channels = as3935_channels;
>> +       indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
>> +       indio_dev->modes = INDIO_DIRECT_MODE;
>> +       indio_dev->info = &as3935_info;
>> +
>> +       trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
>> +                                     indio_dev->name, indio_dev->id);
>> +
>> +       if (!trig)
>> +               return -ENOMEM;
>> +
>> +       st->trig = trig;
>> +       trig->dev.parent = indio_dev->dev.parent;
>> +       iio_trigger_set_drvdata(trig, indio_dev);
>> +       trig->ops = &iio_interrupt_trigger_ops;
>> +
>> +       ret = iio_trigger_register(trig);
>> +       if (ret) {
>> +               dev_err(&spi->dev, "failed to register trigger\n");
>> +               return ret;
>> +       }
>> +
>> +       ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +                                       &as3935_trigger_handler,
>> +                                       &iio_triggered_buffer_setup_ops);
>> +
>> +       if (ret) {
>> +               dev_err(&spi->dev, "cannot setup iio trigger\n");
>> +               goto unregister_trigger;
>> +       }
>> +
>> +       calibrate_as3935(st);
>> +
>> +       ret = devm_request_irq(&spi->dev, spi->irq,
>> +                               &as3935_interrupt_handler,
>> +                               IRQF_TRIGGER_RISING,
>> +                               dev_name(&spi->dev),
>> +                               indio_dev);
>> +
>> +       if (ret) {
>> +               dev_err(&spi->dev, "unable to request irq\n");
>> +               goto unregister_trigger;
>> +       }
>> +
>> +       ret = iio_device_register(indio_dev);
>> +       if (ret < 0) {
>> +               dev_err(&spi->dev, "unable to register device\n");
>> +               goto unregister_trigger;
>> +       }
>> +       return 0;
>> +
>> +unregister_trigger:
>> +       iio_trigger_unregister(st->trig);
>> +       iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +       return ret;
>> +};
>> +
>> +static int as3935_remove(struct spi_device *spi)
>> +{
>> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +       struct as3935_state *st = iio_priv(indio_dev);
>> +
>> +       iio_trigger_unregister(st->trig);
>> +       iio_triggered_buffer_cleanup(indio_dev);
>> +       iio_device_unregister(indio_dev);
>
> This should be in the reverse order of what goes on in the probe.
> This helps both for review purposes and to avoid actual bugs. Here for
> example, the buffer has been destroyed before the userspace interface
> is removed in the device_unregister.
>

Noted.

>> +
>> +       return 0;
>> +};
>> +
>> +static const struct spi_device_id as3935_id[] = {
>> +       {"as3935", 0},
>> +       {},
>> +};
>> +MODULE_DEVICE_TABLE(spi, as3935_id);
>> +
>> +static struct spi_driver as3935_driver = {
>> +       .driver = {
>> +               .name   = "as3935",
>> +               .owner  = THIS_MODULE,
>> +       },
>> +       .probe          = as3935_probe,
>> +       .remove         = as3935_remove,
>> +       .id_table       = as3935_id,
>> +       .suspend        = as3935_suspend,
>> +       .resume         = as3935_resume,
>> +};
>> +module_spi_driver(as3935_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
>> +MODULE_DESCRIPTION("AS3935 lightning sensor");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("spi:as3935");
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv2 1/2] net: stmmac: Add SOCFPGA glue driver
From: David Miller @ 2014-02-10  2:06 UTC (permalink / raw)
  To: dinguyen
  Cc: netdev, dinh.linux, devicetree, peppe.cavallaro, robh+dt,
	pawel.moll, mark.rutland, ijc+devicetree, galak, vbridgers2013
In-Reply-To: <1391728795-30435-1-git-send-email-dinguyen@altera.com>

From: <dinguyen@altera.com>
Date: Thu, 6 Feb 2014 17:19:54 -0600

> +MODULE_ALIAS("platform:socfpga-dwmac");
> +MODULE_AUTHOR("Dinh Nguyen <dinguyen@altera.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Altera SOCFPGA DWMAC Glue Layer");
> +

Do not add stray empty lines at the end of files.  Your second patch
has this problem is a few places too.

^ permalink raw reply

* Re: [PATCH] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Shawn Guo @ 2014-02-10  3:02 UTC (permalink / raw)
  To: Liu Ying; +Cc: devicetree, kernel, linux-arm-kernel
In-Reply-To: <1391522263-17877-1-git-send-email-Ying.Liu@freescale.com>

On Tue, Feb 04, 2014 at 09:57:42PM +0800, Liu Ying wrote:
> The D25 LED controlled by gpio on the i.MX51 babbage
> board is a diagnostic LED according to the board design.
> This patch adds the relevant device tree nodes to the
> i.MX51 babbage device tree file to support this LED.
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index be1407c..8d6a74b 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -81,6 +81,17 @@
>  		};
>  	};
>  
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&led_pin_gpio2_6>;
> +
> +		led-diagnostic {
> +			label = "diagnostic";
> +			gpios = <&gpio2 6 0>;

Just out of curiosity, how will you use/trigger the led?

> +		};
> +	};
> +
>  	sound {
>  		compatible = "fsl,imx51-babbage-sgtl5000",
>  			     "fsl,imx-audio-sgtl5000";
> @@ -280,6 +291,12 @@
>  				MX51_PAD_CSPI1_RDY__GPIO4_26 0x80000000
>  			>;
>  		};
> +
> +		led_pin_gpio2_6: led_gpio2_6 {

This might be copied from some existing file, but I would hope the name
can be more generic, something like the following

	pinctrl_gpio_leds: gpioledsgrp {

, so that when we have more gpio controlled leds to add, we can just add
more pins into the same group without concerning the name.

Shawn

> +			fsl,pins = <
> +				MX51_PAD_EIM_D22__GPIO2_6 0x80000000
> +			>;
> +		};
>  	};
>  };
>  
> -- 
> 1.7.9.5
> 
> 

^ permalink raw reply

* [PATCH RESEND] input: Add commonly used event types
From: Alexander Shiyan @ 2014-02-10  4:21 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, devicetree, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	Alexander Shiyan

Patch adds commonly used event types (EV_KEY and EV_SW) to
devicetree bindings header for input subsystem.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 include/dt-bindings/input/input.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/input/input.h b/include/dt-bindings/input/input.h
index 042e7b3..3a141d92 100644
--- a/include/dt-bindings/input/input.h
+++ b/include/dt-bindings/input/input.h
@@ -9,6 +9,9 @@
 #ifndef _DT_BINDINGS_INPUT_INPUT_H
 #define _DT_BINDINGS_INPUT_INPUT_H
 
+#define EV_KEY			0x01
+#define EV_SW			0x05
+
 #define KEY_RESERVED		0
 #define KEY_ESC			1
 #define KEY_1			2
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v2 2/2] input: clps711x-keypad: dts: Add bindings documentation
From: Alexander Shiyan @ 2014-02-10  4:22 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, devicetree, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	Alexander Shiyan

This patch adds the devicetree documentation for the Cirrus Logic
CLPS711X keypad.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/input/clps711x-keypad.txt  | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/clps711x-keypad.txt

diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
new file mode 100644
index 0000000..bcf5f02
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
@@ -0,0 +1,29 @@
+* Cirrus Logic CLPS711X matrix keypad device tree bindings
+
+Required Properties:
+- compatible:    Should be "cirrus,clps711x-keypad".
+- syscon:        phandle to syscon device node which control keypad (SYSCON1).
+- row-gpios:     List of GPIOs used as row lines.
+- poll-interval: Poll interval time in milliseconds.
+- linux,keymap:  The definition can be found at
+                 bindings/input/matrix-keymap.txt.
+
+Optional Properties:
+- autorepeat:    Enable autorepeat feature.
+
+Example:
+	keypad {
+		compatible = "cirrus,clps711x-keypad";
+		syscon = <&syscon1>;
+		autorepeat;
+		poll-interval = <120>;
+		row-gpios = <&porta 0 0>,
+			    <&porta 1 0>;
+
+		linux,keymap = <
+			MATRIX_KEY(0, 0, KEY_UP)
+			MATRIX_KEY(0, 1, KEY_DOWN)
+			MATRIX_KEY(1, 0, KEY_LEFT)
+			MATRIX_KEY(1, 1, KEY_RIGHT)
+		>;
+	};
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v6 0/2] AS3935 lightning sensor support
From: Matt Ranostay @ 2014-02-10  4:27 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: matt.porter-QSEj5FYQhm4dnm+yROfE0A,
	pantelis.antoniou-Re5JQEeQqe8AvxtiuMwx3w, Matt Ranostay

This series adds support for the AMS AS3935 lightning sensor that allows
reporting back estimated storm distance and strike events.

Changes from v5

* SPI write cache-aligned issues fixed 
* Fixed mutex_unlock's being missed
* Reports distance in meters instead of kilometers (1km steps)
* tune_cap is now in picofarads and not a register value

Matt Ranostay (2):
  iio:as3935: Add DT binding docs for AS3935 driver
  iio: Add AS3935 lightning sensor support

 .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
 .../devicetree/bindings/iio/proximity/as3935.txt   |  27 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/proximity/Kconfig                      |  19 +
 drivers/iio/proximity/Makefile                     |   6 +
 drivers/iio/proximity/as3935.c                     | 443 +++++++++++++++++++++
 8 files changed, 516 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/as3935.txt
 create mode 100644 drivers/iio/proximity/Kconfig
 create mode 100644 drivers/iio/proximity/Makefile
 create mode 100644 drivers/iio/proximity/as3935.c

-- 
1.8.3.2

^ permalink raw reply

* [PATCH v6 1/2] iio:as3935: Add DT binding docs for AS3935 driver
From: Matt Ranostay @ 2014-02-10  4:27 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: matt.porter-QSEj5FYQhm4dnm+yROfE0A,
	pantelis.antoniou-Re5JQEeQqe8AvxtiuMwx3w, Matt Ranostay
In-Reply-To: <1392006451-7818-1-git-send-email-mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Document compatible string, required and optional DT properties for
AS3935 chipset driver.

Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/iio/proximity/as3935.txt   | 27 ++++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 2 files changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/as3935.txt

diff --git a/Documentation/devicetree/bindings/iio/proximity/as3935.txt b/Documentation/devicetree/bindings/iio/proximity/as3935.txt
new file mode 100644
index 0000000..0453254
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/as3935.txt
@@ -0,0 +1,27 @@
+Austrian Microsystems AS3935 Franklin lightning sensor device driver
+
+Required properties:
+	- compatible: must be "ams,as3935"
+	- reg: SPI chip select number for the device
+	- spi-cpha: SPI Mode 1. Refer to spi/spi-bus.txt for generic SPI
+	slave node bindings.
+	- interrupt-parent : should be the phandle for the interrupt controller
+	- interrupts : interrupt mapping
+
+	Refer to interrupt-controller/interrupts.txt for generic
+	interrupt client node bindings.
+
+Optional properties:
+	- ams,tune-cap: Calibration tuning capacitor stepping value 0 - 120pF.
+	  This will require using the calibration data from the manufacturer.
+
+Example:
+
+as3935@0 {
+	compatible = "ams,as3935";
+	reg = <0>;
+	spi-cpha;
+	interrupt-parent = <&gpio1>;
+	interrupts = <16 1>;
+	ams,tune-cap = <80>;
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index e9d19e2..03e50ff 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -11,6 +11,7 @@ ak	Asahi Kasei Corp.
 allwinner	Allwinner Technology Co., Ltd.
 altr	Altera Corp.
 amcc	Applied Micro Circuits Corporation (APM, formally AMCC)
+ams	AMS AG
 amstaos	AMS-Taos Inc.
 apm	Applied Micro Circuits Corporation (APM)
 arm	ARM Ltd.
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v6 2/2] iio: Add AS3935 lightning sensor support
From: Matt Ranostay @ 2014-02-10  4:27 UTC (permalink / raw)
  To: linux-kernel, linux-iio, devicetree
  Cc: matt.porter, pantelis.antoniou, Matt Ranostay
In-Reply-To: <1392006451-7818-1-git-send-email-mranostay@gmail.com>

AS3935 chipset can detect lightning strikes and reports those back as
events and the estimated distance to the storm.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../ABI/testing/sysfs-bus-iio-proximity-as3935     |  18 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/proximity/Kconfig                      |  19 +
 drivers/iio/proximity/Makefile                     |   6 +
 drivers/iio/proximity/as3935.c                     | 443 +++++++++++++++++++++
 6 files changed, 488 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
 create mode 100644 drivers/iio/proximity/Kconfig
 create mode 100644 drivers/iio/proximity/Makefile
 create mode 100644 drivers/iio/proximity/as3935.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935 b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
new file mode 100644
index 0000000..fc92bce
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
@@ -0,0 +1,18 @@
+What		/sys/bus/iio/devices/iio:deviceX/in_proximity_raw
+Date:		January 2014
+KernelVersion:	3.15
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Get the current distance in meters of storm (1km steps)
+		1000       = storm overhead
+		1000-40000 = distance in meters
+		63000      = out of range
+
+What		/sys/bus/iio/devices/iio:deviceX/gain_boost
+Date:		January 2014
+KernelVersion:	3.15
+Contact:	Matt Ranostay <mranostay@gmail.com>
+Description:
+		Show or set the gain boost of the amp, from 0-31 range.
+		18 = indoors (default)
+		14 = outdoors
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 5dd0e12..743485e 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -74,6 +74,7 @@ if IIO_TRIGGER
    source "drivers/iio/trigger/Kconfig"
 endif #IIO_TRIGGER
 source "drivers/iio/pressure/Kconfig"
+source "drivers/iio/proximity/Kconfig"
 source "drivers/iio/temperature/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 887d390..698afc2 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -24,5 +24,6 @@ obj-y += light/
 obj-y += magnetometer/
 obj-y += orientation/
 obj-y += pressure/
+obj-y += proximity/
 obj-y += temperature/
 obj-y += trigger/
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
new file mode 100644
index 0000000..0c8cdf5
--- /dev/null
+++ b/drivers/iio/proximity/Kconfig
@@ -0,0 +1,19 @@
+#
+# Proximity sensors
+#
+
+menu "Lightning sensors"
+
+config AS3935
+	tristate "AS3935 Franklin lightning sensor"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	depends on SPI
+	help
+	  Say Y here to build SPI interface support for the Austrian
+	  Microsystems AS3935 lightning detection sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called as3935
+
+endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
new file mode 100644
index 0000000..743adee
--- /dev/null
+++ b/drivers/iio/proximity/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for IIO proximity sensors
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AS3935)		+= as3935.o
diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
new file mode 100644
index 0000000..34e8f61
--- /dev/null
+++ b/drivers/iio/proximity/as3935.c
@@ -0,0 +1,443 @@
+/*
+ * as3935.c - Support for AS3935 Franklin lightning sensor
+ *
+ * Copyright (C) 2014 Matt Ranostay <mranostay@gmail.com>
+ *
+ * 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.
+ *
+ * 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/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/workqueue.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/pm_runtime.h>
+#include <linux/of_gpio.h>
+
+
+#define AS3935_AFE_GAIN		0x00
+#define AS3935_AFE_MASK		0x3F
+#define AS3935_AFE_GAIN_MAX	0x1F
+#define AS3935_AFE_PWR_BIT	BIT(0)
+
+#define AS3935_INT		0x03
+#define AS3935_INT_MASK		0x07
+#define AS3935_EVENT_INT	BIT(3)
+#define AS3935_NOISE_INT	BIT(1)
+
+#define AS3935_DATA		0x07
+#define AS3935_DATA_MASK	0x3F
+
+#define AS3935_TUNE_CAP		0x08
+#define AS3935_CALIBRATE	0x3D
+
+#define AS3935_WRITE_DATA	BIT(15)
+#define AS3935_READ_DATA	BIT(14)
+#define AS3935_ADDRESS(x)	((x) << 8)
+
+#define MAX_PF_CAP		120
+#define TUNE_CAP_DIV		8
+
+struct as3935_state {
+	struct spi_device *spi;
+	struct iio_trigger *trig;
+	struct mutex lock;
+	struct delayed_work work;
+
+	u8 buf[2] ____cacheline_aligned;
+	u32 tune_cap;
+};
+
+static const struct iio_chan_spec as3935_channels[] = {
+	{
+		.type           = IIO_PROXIMITY,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW),
+		.scan_index     = 0,
+		.scan_type = {
+			.sign           = 'u',
+			.realbits       = 6,
+			.storagebits    = 8,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+static int as3935_read(struct as3935_state *st, unsigned int reg, int *val)
+{
+	u8 cmd;
+	int ret;
+
+	cmd = (AS3935_READ_DATA | AS3935_ADDRESS(reg)) >> 8;
+	ret = spi_w8r8(st->spi, cmd);
+	if (ret < 0)
+		return ret;
+	*val = ret;
+
+	return 0;
+};
+
+static int as3935_write(struct as3935_state *st,
+				unsigned int reg,
+				unsigned int val)
+{
+	u8 *buf = st->buf;
+
+	buf[0] = (AS3935_WRITE_DATA | AS3935_ADDRESS(reg)) >> 8;
+	buf[1] = val;
+
+	return spi_write(st->spi, (u8 *) buf, 2);
+};
+
+static ssize_t as3935_gain_boost_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		return ret;
+	val = (val & AS3935_AFE_MASK) >> 1;
+
+	return sprintf(buf, "%d\n", val);
+};
+
+static ssize_t as3935_gain_boost_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct as3935_state *st = iio_priv(dev_to_iio_dev(dev));
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul((const char *) buf, 10, &val);
+	if (ret)
+		return -EINVAL;
+
+	if (val > AS3935_AFE_GAIN_MAX)
+		return -EINVAL;
+
+	as3935_write(st, AS3935_AFE_GAIN, val << 1);
+
+	return len;
+};
+
+static IIO_DEVICE_ATTR(gain_boost, S_IRUGO | S_IWUSR,
+	as3935_gain_boost_show, as3935_gain_boost_store, 0);
+
+
+static struct attribute *as3935_attributes[] = {
+	&iio_dev_attr_gain_boost.dev_attr.attr,
+	NULL,
+};
+
+static struct attribute_group as3935_attribute_group = {
+	.attrs = as3935_attributes,
+};
+
+static int as3935_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct as3935_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (m != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	*val2 = 0;
+	ret = as3935_read(st, AS3935_DATA, val);
+	if (ret)
+		return ret;
+	*val *= 1000;
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_info as3935_info = {
+	.driver_module = THIS_MODULE,
+	.attrs = &as3935_attribute_group,
+	.read_raw = &as3935_read_raw,
+};
+
+static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
+	.postenable = &iio_triggered_buffer_postenable,
+	.predisable = &iio_triggered_buffer_predisable,
+};
+
+static irqreturn_t as3935_trigger_handler(int irq, void *private)
+{
+	struct iio_poll_func *pf = private;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = as3935_read(st, AS3935_DATA, &val);
+	if (ret)
+		goto err_read;
+	val &= AS3935_DATA_MASK;
+	val *= 1000;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &val, pf->timestamp);
+err_read:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+};
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static void as3935_event_work(struct work_struct *work)
+{
+	struct as3935_state *st;
+	int val;
+
+	st = container_of(work, struct as3935_state, work.work);
+
+	as3935_read(st, AS3935_INT, &val);
+	val &= AS3935_INT_MASK;
+
+	switch (val) {
+	case AS3935_EVENT_INT:
+		iio_trigger_poll(st->trig, iio_get_time_ns());
+		break;
+	case AS3935_NOISE_INT:
+		dev_warn(&st->spi->dev, "noise level is too high");
+		break;
+	}
+};
+
+static irqreturn_t as3935_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	/* Delay work for >2 milliseconds after an interrupt to allow
+	 * estimated distance to recalculated.
+	 */
+
+	schedule_delayed_work(&st->work, msecs_to_jiffies(3));
+
+	return IRQ_HANDLED;
+}
+
+static void calibrate_as3935(struct as3935_state *st)
+{
+	mutex_lock(&st->lock);
+
+	/* mask disturber interrupt bit */
+	as3935_write(st, AS3935_INT, 1<<5);
+
+	as3935_write(st, AS3935_CALIBRATE, 0x96);
+	as3935_write(st, AS3935_TUNE_CAP, 1<<5 | (st->tune_cap / TUNE_CAP_DIV));
+
+	mdelay(2);
+	as3935_write(st, AS3935_TUNE_CAP, (st->tune_cap / TUNE_CAP_DIV));
+
+	mutex_unlock(&st->lock);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int as3935_suspend(struct spi_device *spi, pm_message_t msg)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	mutex_lock(&st->lock);
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		goto err_suspend;
+	val |= AS3935_AFE_PWR_BIT;
+
+	ret = as3935_write(st, AS3935_AFE_GAIN, val);
+
+err_suspend:
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int as3935_resume(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	mutex_lock(&st->lock);
+	ret = as3935_read(st, AS3935_AFE_GAIN, &val);
+	if (ret)
+		goto err_resume;
+	val &= ~AS3935_AFE_PWR_BIT;
+	ret = as3935_write(st, AS3935_AFE_GAIN, val);
+
+err_resume:
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+#else
+#define as3935_suspend	NULL
+#define as3935_resume	NULL
+#endif
+
+static int as3935_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct iio_trigger *trig;
+	struct as3935_state *st;
+	struct device_node *np = spi->dev.of_node;
+	int ret;
+
+	/* Be sure lightning event interrupt */
+	if (!spi->irq) {
+		dev_err(&spi->dev, "unable to get event interrupt\n");
+		return -EINVAL;
+	}
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->tune_cap = 0;
+
+	spi_set_drvdata(spi, indio_dev);
+	mutex_init(&st->lock);
+	INIT_DELAYED_WORK(&st->work, as3935_event_work);
+
+	ret = of_property_read_u32(np, "ams,tune-cap", &st->tune_cap);
+	if (ret) {
+		st->tune_cap = 0;
+		dev_warn(&spi->dev,
+			"no tune-cap set, defaulting to %d", st->tune_cap);
+	}
+
+	if (st->tune_cap > MAX_PF_CAP) {
+		dev_err(&spi->dev,
+			"wrong tune-cap setting of %d\n", st->tune_cap);
+		return -EINVAL;
+	}
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->channels = as3935_channels;
+	indio_dev->num_channels = ARRAY_SIZE(as3935_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &as3935_info;
+
+	trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+				      indio_dev->name, indio_dev->id);
+
+	if (!trig)
+		return -ENOMEM;
+
+	st->trig = trig;
+	trig->dev.parent = indio_dev->dev.parent;
+	iio_trigger_set_drvdata(trig, indio_dev);
+	trig->ops = &iio_interrupt_trigger_ops;
+
+	ret = iio_trigger_register(trig);
+	if (ret) {
+		dev_err(&spi->dev, "failed to register trigger\n");
+		return ret;
+	}
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					&as3935_trigger_handler,
+					&iio_triggered_buffer_setup_ops);
+
+	if (ret) {
+		dev_err(&spi->dev, "cannot setup iio trigger\n");
+		goto unregister_trigger;
+	}
+
+	calibrate_as3935(st);
+
+	ret = devm_request_irq(&spi->dev, spi->irq,
+				&as3935_interrupt_handler,
+				IRQF_TRIGGER_RISING,
+				dev_name(&spi->dev),
+				indio_dev);
+
+	if (ret) {
+		dev_err(&spi->dev, "unable to request irq\n");
+		goto unregister_trigger;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&spi->dev, "unable to register device\n");
+		goto unregister_trigger;
+	}
+	return 0;
+
+unregister_trigger:
+	iio_trigger_unregister(st->trig);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return ret;
+};
+
+static int as3935_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct as3935_state *st = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	iio_trigger_unregister(st->trig);
+
+	return 0;
+};
+
+static const struct spi_device_id as3935_id[] = {
+	{"as3935", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, as3935_id);
+
+static struct spi_driver as3935_driver = {
+	.driver = {
+		.name	= "as3935",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= as3935_probe,
+	.remove		= as3935_remove,
+	.id_table	= as3935_id,
+	.suspend	= as3935_suspend,
+	.resume		= as3935_resume,
+};
+module_spi_driver(as3935_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("AS3935 lightning sensor");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:as3935");
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Alexander Shiyan @ 2014-02-10  4:29 UTC (permalink / raw)
  To: Shawn Guo; +Cc: devicetree, Liu Ying, linux-arm-kernel, kernel
In-Reply-To: <20140210030214.GB9628@S2101-09.ap.freescale.net>

Понедельник, 10 февраля 2014, 11:02 +08:00 от Shawn Guo <shawn.guo@linaro.org>:
> On Tue, Feb 04, 2014 at 09:57:42PM +0800, Liu Ying wrote:
> > The D25 LED controlled by gpio on the i.MX51 babbage
> > board is a diagnostic LED according to the board design.
> > This patch adds the relevant device tree nodes to the
> > i.MX51 babbage device tree file to support this LED.
> > 
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> > ---
> >  arch/arm/boot/dts/imx51-babbage.dts |   17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx51-babbage.dts
> b/arch/arm/boot/dts/imx51-babbage.dts
> > index be1407c..8d6a74b 100644
> > --- a/arch/arm/boot/dts/imx51-babbage.dts
> > +++ b/arch/arm/boot/dts/imx51-babbage.dts
> > @@ -81,6 +81,17 @@
> >  		};
> >  	};
> >  
> > +	leds {
> > +		compatible = "gpio-leds";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&led_pin_gpio2_6>;
> > +
> > +		led-diagnostic {
> > +			label = "diagnostic";
> > +			gpios = <&gpio2 6 0>;
> 
> Just out of curiosity, how will you use/trigger the led?

And GPIO bindings should be used to specify active level,
GPIO_ACTIVE_HIGH in this case.

---
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Liu Ying @ 2014-02-10  6:05 UTC (permalink / raw)
  To: Shawn Guo; +Cc: devicetree, kernel, linux-arm-kernel
In-Reply-To: <20140210030214.GB9628@S2101-09.ap.freescale.net>

On 02/10/2014 11:02 AM, Shawn Guo wrote:
> On Tue, Feb 04, 2014 at 09:57:42PM +0800, Liu Ying wrote:
>> The D25 LED controlled by gpio on the i.MX51 babbage
>> board is a diagnostic LED according to the board design.
>> This patch adds the relevant device tree nodes to the
>> i.MX51 babbage device tree file to support this LED.
>>
>> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>> ---
>>  arch/arm/boot/dts/imx51-babbage.dts |   17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
>> index be1407c..8d6a74b 100644
>> --- a/arch/arm/boot/dts/imx51-babbage.dts
>> +++ b/arch/arm/boot/dts/imx51-babbage.dts
>> @@ -81,6 +81,17 @@
>>  		};
>>  	};
>>  
>> +	leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&led_pin_gpio2_6>;
>> +
>> +		led-diagnostic {
>> +			label = "diagnostic";
>> +			gpios = <&gpio2 6 0>;
> 
> Just out of curiosity, how will you use/trigger the led?
> 

Switch on the LED:
echo 1 > /sys/class/leds/diagnostic/brightness

Switch off the LED:
echo 0 > /sys/class/leds/diagnostic/brightness

>> +		};
>> +	};
>> +
>>  	sound {
>>  		compatible = "fsl,imx51-babbage-sgtl5000",
>>  			     "fsl,imx-audio-sgtl5000";
>> @@ -280,6 +291,12 @@
>>  				MX51_PAD_CSPI1_RDY__GPIO4_26 0x80000000
>>  			>;
>>  		};
>> +
>> +		led_pin_gpio2_6: led_gpio2_6 {
> 
> This might be copied from some existing file, but I would hope the name
> can be more generic, something like the following
> 
> 	pinctrl_gpio_leds: gpioledsgrp {
> 
> , so that when we have more gpio controlled leds to add, we can just add
> more pins into the same group without concerning the name.
> 

Agree.  I will address this comment in patch v2.  Thanks!

Regards,
Liu Ying

> 
>> +			fsl,pins = <
>> +				MX51_PAD_EIM_D22__GPIO2_6 0x80000000
>> +			>;
>> +		};
>>  	};
>>  };
>>  
>> -- 
>> 1.7.9.5
>>
>>
> 

^ permalink raw reply

* [PATCH v3 0/2] ASoC: atmel_ssc_dai: add option to choose clock
From: Bo Shen @ 2014-02-10  6:09 UTC (permalink / raw)
  To: broonie
  Cc: Mark Rutland, alsa-devel, Stephen Warren, linux-doc, Takashi Iwai,
	nicolas.ferre, Bo Shen, Chas Williams, Lars-Peter Clausen,
	Pawel Moll, linux-atm-general, plagnioj, devicetree,
	Arnd Bergmann, Ian Campbell, linux-sound, Rob Herring,
	linux-arm-kernel, Richard Genoud, Greg Kroah-Hartman,
	Liam Girdwood, linux-kernel, Rob Landley, netdev, Mark Brown

When SSC work in slave mode, the clock can come from TK pin and also
can come from RK pin, this is hardware design decided. So, make it
available to choose where the clock from.

Changes in v3:
  - Move the property from card to ssc device
Series-changes: 2
  - using "-" replace "_" in binding document

Bo Shen (2):
  ASoC: atmel_ssc_dai: make option to choose clock
  Binding: atmel-ssc: add option to choose clock

 Documentation/devicetree/bindings/misc/atmel-ssc.txt |  8 ++++++++
 drivers/misc/atmel-ssc.c                             |  6 ++++++
 include/linux/atmel-ssc.h                            |  1 +
 sound/soc/atmel/atmel_ssc_dai.c                      | 13 +++++++++----
 4 files changed, 24 insertions(+), 4 deletions(-)

-- 
1.8.5.2

^ permalink raw reply

* [PATCH v3 2/2] Binding: atmel-ssc: add option to choose clock
From: Bo Shen @ 2014-02-10  6:09 UTC (permalink / raw)
  To: broonie
  Cc: Mark Rutland, devicetree, alsa-devel, Rob Landley, Pawel Moll,
	Stephen Warren, Richard Genoud, linux-kernel, Ian Campbell,
	nicolas.ferre, Mark Brown, linux-sound, linux-doc, Rob Herring,
	Bo Shen, plagnioj, linux-arm-kernel
In-Reply-To: <1392012586-30790-1-git-send-email-voice.shen@atmel.com>

Add the option to choose clock on which pin input to SSC (as slave).
Default is on TK pin to SSC, add "atmel,clk-from-rk-pin" option to
specify the clock is on RK pin to SSC.

Signed-off-by: Bo Shen <voice.shen@atmel.com>
---
Changes in v3:
  - None
Series-changes: 2
  - using "-" replace "_" in binding document

 Documentation/devicetree/bindings/misc/atmel-ssc.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/misc/atmel-ssc.txt b/Documentation/devicetree/bindings/misc/atmel-ssc.txt
index a45ae08..5c1e14e 100644
--- a/Documentation/devicetree/bindings/misc/atmel-ssc.txt
+++ b/Documentation/devicetree/bindings/misc/atmel-ssc.txt
@@ -14,6 +14,14 @@ Required properties for devices compatible with "atmel,at91sam9g45-ssc":
   See Documentation/devicetree/bindings/dma/atmel-dma.txt for details.
 - dma-names: Must be "tx", "rx".
 
+Optional properties:
+  - atmel,clk-from-rk-pin: bool property.
+     - When SSC works in slave mode, according to the hardware design, the
+       clock can get from TK pin, and also can get from RK pin. So, add
+       this parameter to choose where the clock from.
+     - By default the clock is from TK pin, if the clock from RK pin, this
+       property is needed.
+
 Examples:
 - PDC transfer:
 ssc0: ssc@fffbc000 {
-- 
1.8.5.2

^ permalink raw reply related

* Re: [PATCH] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Liu Ying @ 2014-02-10  6:24 UTC (permalink / raw)
  To: Alexander Shiyan; +Cc: devicetree, Shawn Guo, linux-arm-kernel, kernel
In-Reply-To: <1392006579.703394606@f357.i.mail.ru>

On 02/10/2014 12:29 PM, Alexander Shiyan wrote:
> Понедельник, 10 февраля 2014, 11:02 +08:00 от Shawn Guo <shawn.guo@linaro.org>:
>> On Tue, Feb 04, 2014 at 09:57:42PM +0800, Liu Ying wrote:
>>> The D25 LED controlled by gpio on the i.MX51 babbage
>>> board is a diagnostic LED according to the board design.
>>> This patch adds the relevant device tree nodes to the
>>> i.MX51 babbage device tree file to support this LED.
>>>
>>> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>>> ---
>>>  arch/arm/boot/dts/imx51-babbage.dts |   17 +++++++++++++++++
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/imx51-babbage.dts
>> b/arch/arm/boot/dts/imx51-babbage.dts
>>> index be1407c..8d6a74b 100644
>>> --- a/arch/arm/boot/dts/imx51-babbage.dts
>>> +++ b/arch/arm/boot/dts/imx51-babbage.dts
>>> @@ -81,6 +81,17 @@
>>>  		};
>>>  	};
>>>  
>>> +	leds {
>>> +		compatible = "gpio-leds";
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&led_pin_gpio2_6>;
>>> +
>>> +		led-diagnostic {
>>> +			label = "diagnostic";
>>> +			gpios = <&gpio2 6 0>;
>>
>> Just out of curiosity, how will you use/trigger the led?
> 
> And GPIO bindings should be used to specify active level,
> GPIO_ACTIVE_HIGH in this case.

Based on Linux 3.14-rc2, this gives me no single output:
grep GPIO_ACTIVE arch/arm/boot/dts/ -nr | grep imx

So, this patch follows the old approach.

I agree to use GPIO_ACTIVE_HIGH/GPIO_ACTIVE_LOW macros.
How about generating dedicated patch sets for this purpose?
Actually, I had one set for i.MX51 platforms.

Regards,
Liu Ying


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Shawn Guo @ 2014-02-10  6:31 UTC (permalink / raw)
  To: Liu Ying
  Cc: Alexander Shiyan, devicetree-u79uwXL29TY76Z2rM5mHXA,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <52F87087.4000908-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

On Mon, Feb 10, 2014 at 02:24:07PM +0800, Liu Ying wrote:
> >>> @@ -81,6 +81,17 @@
> >>>  		};
> >>>  	};
> >>>  
> >>> +	leds {
> >>> +		compatible = "gpio-leds";
> >>> +		pinctrl-names = "default";
> >>> +		pinctrl-0 = <&led_pin_gpio2_6>;
> >>> +
> >>> +		led-diagnostic {
> >>> +			label = "diagnostic";
> >>> +			gpios = <&gpio2 6 0>;
> >>
> >> Just out of curiosity, how will you use/trigger the led?
> > 
> > And GPIO bindings should be used to specify active level,
> > GPIO_ACTIVE_HIGH in this case.
> 
> Based on Linux 3.14-rc2, this gives me no single output:
> grep GPIO_ACTIVE arch/arm/boot/dts/ -nr | grep imx

If you grep linux-next, you will find quite a few.

> 
> So, this patch follows the old approach.
> 
> I agree to use GPIO_ACTIVE_HIGH/GPIO_ACTIVE_LOW macros.
> How about generating dedicated patch sets for this purpose?
> Actually, I had one set for i.MX51 platforms.

I prefer to have this change in the same patch.

Shawn

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Liu Ying @ 2014-02-10  7:05 UTC (permalink / raw)
  To: shawn.guo-QSEj5FYQhm4dnm+yROfE0A
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, shc_work-JGs/UdohzUI

The D25 LED controlled by gpio on the i.MX51 babbage
board is a diagnostic LED according to the board design.
This patch adds the relevant device tree nodes to the
i.MX51 babbage device tree file to support this LED.

Signed-off-by: Liu Ying <Ying.Liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
v1->v2:
* Rebase the patch from 3.14-rc2 to linux-next(d371ca32be).
* Rename the pinctrl name for the LED to be more generic.
* Switch to use the standard GPIO flag GPIO_ACTIVE_HIGH.

 arch/arm/boot/dts/imx51-babbage.dts |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index ea5c67e..121dadd 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -81,6 +81,17 @@
 		};
 	};
 
+	leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_gpio_leds>;
+
+		led-diagnostic {
+			label = "diagnostic";
+			gpios = <&gpio2 6 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
 	sound {
 		compatible = "fsl,imx51-babbage-sgtl5000",
 			     "fsl,imx-audio-sgtl5000";
@@ -344,6 +355,12 @@
 			>;
 		};
 
+		pinctrl_gpio_leds: gpioledsgrp {
+			fsl,pins = <
+				MX51_PAD_EIM_D22__GPIO2_6		0x80000000
+			>;
+		};
+
 		pinctrl_i2c2: i2c2grp {
 			fsl,pins = <
 				MX51_PAD_KEY_COL4__I2C2_SCL		0x400001ed
-- 
1.7.9.5


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
From: Lukasz Majewski @ 2014-02-10  7:10 UTC (permalink / raw)
  To: Thomas Abraham, Sudeep Holla
  Cc: Nishanth Menon, devicetree@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org,
	viresh.kumar@linaro.org, t.figa@samsung.com, rjw@rjwysocki.net,
	kgene.kim@samsung.com, thomas.ab@samsung.com,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAJuA9agT4PC6YOmhCwKQ=58qaGEr1qSuhTpX57ro1pEuk++Miw@mail.gmail.com>

Hi Thomas, Sudeep

> On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com>
> wrote:
> > On 07/02/14 15:19, Thomas Abraham wrote:
> >> From: Thomas Abraham <thomas.ab@samsung.com>
> >>
> >> Commit 6f19efc0 ("cpufreq: Add boost frequency support in core")
> >> adds support for CPU boost mode. This patch adds support for
> >> finding available boost frequencies from device tree and marking
> >> them as usable in boost mode.
> >>
> >> Cc: Nishanth Menon <nm@ti.com>
> >> Cc: Lukasz Majewski <l.majewski@samsung.com>
> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> >> ---
> >>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
> >>  1 file changed, 33 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> >> index fa41874..b636826 100644
> >> --- a/drivers/base/power/opp.c
> >> +++ b/drivers/base/power/opp.c
> >> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct
> >> device *dev, struct device_opp *dev_opp;
> >>       struct dev_pm_opp *opp;
> >>       struct cpufreq_frequency_table *freq_table;
> >> -     int i = 0;
> >> +     int i = 0, j, len, ret;
> >> +     u32 *boost_freqs = NULL;
> >>
> >>       /* Pretend as if I am an updater */
> >>       mutex_lock(&dev_opp_list_lock);
> >> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct
> >> device *dev, return -ENOMEM;
> >>       }
> >>
> >> +     if (of_find_property(dev->of_node, "boost-frequency", &len))
> >> {
> >> +             if (len == 0 || (len & (sizeof(u32) - 1)) != 0) {
> >> +                     dev_err(dev, "%s: invalid boost
> >> frequency\n", __func__);
> >> +                     ret = -EINVAL;
> >> +                     goto err_boost;
> >> +             }
> >> +
> >> +             boost_freqs = kzalloc(len, GFP_KERNEL);
> >> +             if (!boost_freqs) {
> >> +                     dev_warn(dev, "%s: no memory for boost freq
> >> table\n",
> >> +                                     __func__);
> >> +                     ret = -ENOMEM;
> >> +                     goto err_boost;
> >> +             }
> >> +             of_property_read_u32_array(dev->of_node,
> >> "boost-frequency",
> >> +                     boost_freqs, len / sizeof(u32));
> >> +     }
> >> +
> >>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
> >>               if (opp->available) {
> >>                       freq_table[i].driver_data = i;
> >>                       freq_table[i].frequency = opp->rate / 1000;
> >> +                     for (j = 0; j < len / sizeof(u32) &&
> >> boost_freqs; j++) {
> >> +                             if (boost_freqs[j] ==
> >> freq_table[i].frequency) {
> >> +                                     freq_table[i].driver_data =
> >> +
> >> CPUFREQ_BOOST_FREQ;
> >> +                                     break;
> >> +                             }
> >> +                     }
> >>                       i++;
> >>               }
> >>       }
> > IIRC you had mentioned that the boost-opp was not limited to be a
> > cpufreq, but this change seems to be cpufreq only.
> 
> Yes, but as you have initiated the discussion on extending the OPP
> binding, this has been limited to cpufreq only. If the new OPP library
> has support for listing boost frequency, this can be migrated to the
> new OPP libaray.

I agree here with Thomas. Since we are fixing regression here, we shall
use the existing OPP API. When agreement for the future OPPs format is
in place, we will port the boost OPPs to it.

> 
> Thanks,
> Thomas.
> 
> >
> > Regards,
> > Sudeep
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
From: Lukasz Majewski @ 2014-02-10  7:38 UTC (permalink / raw)
  To: Thomas Abraham, Sudeep Holla
  Cc: Nishanth Menon, linux-pm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	rjw@rjwysocki.net, linux-samsung-soc@vger.kernel.org,
	kgene.kim@samsung.com, t.figa@samsung.com,
	viresh.kumar@linaro.org, thomas.ab@samsung.com, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala
In-Reply-To: <CAJuA9agcpbKYocaQUBr2-VCXcXAo3r3urFF=+G_+ib2jCZqvwg@mail.gmail.com>

Hi Thomas, Sudeep,

> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com>
> wrote:
> > On 07/02/14 17:37, Nishanth Menon wrote:
> >> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
> >> <Sudeep.Holla@arm.com> wrote:
> >
> > [...]
> >
> >>> Yes I think its counter-intuitive as it's visible to the
> >>> userspace(list of frequencies and the boost parameters are
> >>> exposed through sysfs)
> >>
> >> That will be a different problem -> as currently every single
> >> frequency in the cpufreq list has ability to be marked as boost
> >> frequency - if userspace does not maintain that, then, IMHO, fix
> >> the userspace :D
> >>
> >
> > /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> > gives the list of frequencies based on the state of the boost
> > feature at anytime.
> 
> The list of frequencies in
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> does not change based in the state of the boost feature (enabled or
> disabled). But the scaling_max_frequency and scaling_min_frequency are
> updated based on the set of available + boost frequencies available.

With boost intended behavior is as follow:

/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies [1]

shows the non boost frequencies no matter if boost is enabled or not.
Those are the "normal" frequencies.

When boost is supported (by enabling the CONFIG_CPU_FREQ_BOOST_SW)
extra sysfs attribute shows up:

/sys/devices/system/cpu/cpu0/cpufreq/scaling_boost_frequencies [2]
in which are listed only the boost frequencies.

If the dts doesn't provide any values or as it is now no
CPUFREQ_BOOST_FREQ flag is specified (at exynos4x12-cpufreq.c), then
this list is empty.


When boost is disabled the max freq (cpuinfo_max_freq) is in the range
listed at [1].

When boost is enabled this value is the sum of freqs represented at [1]
and [2].

By adding an extra attribute [2] we:
1. Indicate that boost is supported
2. List provided BOOST frequencies
3. Separate boost and non boost freqs


> 
> >
> > Intuitively the list without boost shouldn't have any frequency
> > above the range when it's enabled :), that's what I was referring
> > to. So I am not talking about any issue with user-space maintenance.
> >
> > Regards,
> > Sudeep
> >



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
From: Lukasz Majewski @ 2014-02-10  7:53 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: Nishanth Menon, Mark Rutland, devicetree@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, Pawel Moll,
	linux-pm@vger.kernel.org, viresh.kumar@linaro.org,
	t.figa@samsung.com, Ian Campbell, rjw@rjwysocki.net, Rob Herring,
	kgene.kim@samsung.com, thomas.ab@samsung.com, Sudeep Holla,
	Kumar Gala, linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAJuA9aj-LsdBiS2msOVmNRSe6mNj9QJsys9KjHZAk=9hrZOLzA@mail.gmail.com>

Hi Thomas, Sudeep,

> On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
> > On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla
> > <Sudeep.Holla@arm.com> wrote:
> >> On 07/02/14 17:37, Nishanth Menon wrote:
> >>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
> >>> <Sudeep.Holla@arm.com> wrote:
> >>
> >> [...]
> >>
> >>>> Yes I think its counter-intuitive as it's visible to the
> >>>> userspace(list of frequencies and the boost parameters are
> >>>> exposed through sysfs)
> >>>
> >>> That will be a different problem -> as currently every single
> >>> frequency in the cpufreq list has ability to be marked as boost
> >>> frequency - if userspace does not maintain that, then, IMHO, fix
> >>> the userspace :D
> >>>
> >>
> >> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> >> gives the list of frequencies based on the state of the boost
> >> feature at anytime.
> >>
> >> Intuitively the list without boost shouldn't have any frequency
> >> above the range when it's enabled :), that's what I was referring
> >> to. So I am not talking about any issue with user-space
> >> maintenance.
> > Fair enough - but i still think it has nothing to do with dt binding
> > itself -> and i think the discussion we've had should be good for
> > the binding provided in this patch.. I hope.. if documentation
> > needs a bit of better explanation to prevent a repeat of the same
> > discussion at a later point of time, now might be a good time to
> > add it in.
> 
> The term boost and over-clocking have been described in the bindings
> document as being the same. Since the term over-clocking refers to
> running a CPU beyond normal operating frequencies, I tend to agree
> with Sudeep that it is not intuitive if a normal operating frequency
> is greater than a boost mode frequency.
> 
> Otherwise, when userspace does "echo 1 >
> /sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
> think the original intent of boost mode patches was to allow CPU to
> operate at frequencies greater than the normal operating frequencies.
> 
> Lukasz, how would you want this to be handled?

Please consider an example:

normal-freqs: 1400000, 1200000, 1000000, 800000, 600000, 400000, 200000
[1]
boost-freqs: 1700000, 1600000, 1500000. [2]

All those freqs shall be represented as OPPs freq and voltage tuple.

Best would be to specify all the boost-freqs as:
boost-freqs = <1700000 1600000 1500000> to be prepared for future
quirks or problems (or special cases which might show up latter).
If anybody can foresee any potential threads - like platform on which
boost freqs are 1700000 and 1500000, but not 1600000, then please share
this information.

However, I think that it would be also acceptable to specify only:
boost-freq = <1500000> and mark all freqs greater or equal to it as
CPUFREQ_BOOST_FREQ.

If there aren't any potential problems, then I think the second option
would be a good solution (we would have only one BOOST attribute stored
at CPUs DTS node).

> 
> Thanks,
> Thomas.
> 
> >
> > Regards,
> > Nishanth Menon
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply

* Re: [PATCH v2] ARM: dts: i.MX51 babbage: Support diagnostic LED
From: Shawn Guo @ 2014-02-10  8:11 UTC (permalink / raw)
  To: Liu Ying; +Cc: devicetree, shc_work, linux-arm-kernel, kernel
In-Reply-To: <1392015946-22419-1-git-send-email-Ying.Liu@freescale.com>

On Mon, Feb 10, 2014 at 03:05:46PM +0800, Liu Ying wrote:
> The D25 LED controlled by gpio on the i.MX51 babbage
> board is a diagnostic LED according to the board design.
> This patch adds the relevant device tree nodes to the
> i.MX51 babbage device tree file to support this LED.
> 
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>

Applied, thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox