* [PATCH v2 2/5] Input: mpr121 - remove unused field in struct mpr121_touchkey
From: Akinobu Mita @ 2017-01-15 13:15 UTC (permalink / raw)
To: linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Akinobu Mita, Dmitry Torokhov
In-Reply-To: <1484486144-27947-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Remove unused key_val field in struct mpr121_touchkey.
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
* No changes from v1
drivers/input/keyboard/mpr121_touchkey.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index a3fd6e5..90be99d 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -59,7 +59,6 @@
struct mpr121_touchkey {
struct i2c_client *client;
struct input_dev *input_dev;
- unsigned int key_val;
unsigned int statusbits;
unsigned int keycount;
u16 keycodes[MPR121_MAX_KEY_COUNT];
--
2.7.4
--
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
* [PATCH v2 1/5] Input: mpr121 - annotate PM methods as __maybe_unused
From: Akinobu Mita @ 2017-01-15 13:15 UTC (permalink / raw)
To: linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Akinobu Mita, Dmitry Torokhov
In-Reply-To: <1484486144-27947-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Instead of using #ifdef, let's mark suspend and resume methods as
__maybe_unused to provide better compile coverage.
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Suggested-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
* Newly added patch from v2
drivers/input/keyboard/mpr121_touchkey.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index 0fd612d..a3fd6e5 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -266,8 +266,7 @@ static int mpr_touchkey_probe(struct i2c_client *client,
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-static int mpr_suspend(struct device *dev)
+static int __maybe_unused mpr_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
@@ -279,7 +278,7 @@ static int mpr_suspend(struct device *dev)
return 0;
}
-static int mpr_resume(struct device *dev)
+static int __maybe_unused mpr_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct mpr121_touchkey *mpr121 = i2c_get_clientdata(client);
@@ -292,7 +291,6 @@ static int mpr_resume(struct device *dev)
return 0;
}
-#endif
static SIMPLE_DEV_PM_OPS(mpr121_touchkey_pm_ops, mpr_suspend, mpr_resume);
--
2.7.4
--
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
* [PATCH v2 0/5] nput: mpr121 - switch to use device tree probe
From: Akinobu Mita @ 2017-01-15 13:15 UTC (permalink / raw)
To: linux-input, devicetree; +Cc: Akinobu Mita, Dmitry Torokhov, Rob Herring
This driver currently only supports legacy platform data probe. This
change adds device tree support and gets rid of platform data probe code
since no one is actually using mpr121 platform data in the mainline.
This series also contains miscellaneous cleanup and bug fixes that
mostly I found while playing with this driver.
* Changes since v1 (All changes are suggested by Dmitry Torokhov)
- Add patch 'annotate PM methods as __maybe_unused'
- Use for_each_set_bit() to search changed bit
- Use linux,keycodes property instead of using matrix keymap API
- Get rid of platform data
Akinobu Mita (5):
Input: mpr121 - annotate PM methods as __maybe_unused
Input: mpr121 - remove unused field in struct mpr121_touchkey
Input: mpr121 - set missing event capability
Input: mpr121 - handle multiple bits change of status register
Input: mpr121 - switch to device tree probe
.../devicetree/bindings/input/mpr121-touchkey.txt | 30 +++++
drivers/input/keyboard/mpr121_touchkey.c | 141 ++++++++++++++-------
include/linux/i2c/mpr121_touchkey.h | 20 ---
3 files changed, 127 insertions(+), 64 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/mpr121-touchkey.txt
delete mode 100644 include/linux/i2c/mpr121_touchkey.h
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Rob Herring <robh@kernel.org>
--
2.7.4
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger
From: Jonathan Cameron @ 2017-01-15 13:12 UTC (permalink / raw)
To: Andreas Klinger
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
mranostay-Re5JQEeQqe8AvxtiuMwx3w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170114174856.GA2351@andreas>
On 14/01/17 17:48, Andreas Klinger wrote:
> Hi Jonathan,
>
> see comments below.
>
> Andreas
>
>
> Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> schrieb am Sat, 14. Jan 12:17:
>> On 10/01/17 18:48, Andreas Klinger wrote:
>>> This is the IIO driver for devantech srf08 ultrasonic ranger which can be
>>> used to measure the distances to an object.
>>>
>>> The sensor supports I2C with some registers.
>>>
>>> Supported Features include:
>>> - read the distance in ranging mode in centimeters
>>> - output of the driver is directly the read value
>>> - together with the scale the driver delivers the distance in meters
>>> - only the first echo of the nearest object is delivered
>>> - set max gain register; userspace enters analogue value
>>> - set range registers; userspace enters range in millimeters in 43 mm steps
>>>
>>> Features not supported by this driver:
>>> - ranging mode in inches or in microseconds
>>> - ANN mode
>>> - change I2C address through this driver
>>> - light sensor
>>>
>>> The driver was added in the directory "proximity" of the iio subsystem
>>> in absence of another directory named "distance".
>>> There is also a new submenu "distance"
>> Hi Andreas,
>>
>> Sorry it took me a while to get to this!
>>
>> I'd not bother with the new submenu. Perhaps we should rename the
>> proximity menu to proximity/distance.
>>
>> We already the lightening detector in there which is definitely not
>> measuring proximity in the convetional sense!
>>
>> Anyhow, the actual code is fine, but we need to think about how the
>> userspace ABI fits within the wider IIO ABI. Naming and approaches
>> that make sense in a single class of drivers can end up meaining
>> very different things for other drivers. Various suggestions inline.
>>
>> Jonathan
>>>
>>> Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
>>> ---
>>> drivers/iio/proximity/Kconfig | 15 ++
>>> drivers/iio/proximity/Makefile | 1 +
>>> drivers/iio/proximity/srf08.c | 362 +++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 378 insertions(+)
>>> create mode 100644 drivers/iio/proximity/srf08.c
>>>
>>> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
>>> index ef4c73db5b53..7b10a137702b 100644
>>> --- a/drivers/iio/proximity/Kconfig
>>> +++ b/drivers/iio/proximity/Kconfig
>>> @@ -46,3 +46,18 @@ config SX9500
>>> module will be called sx9500.
>>>
>>> endmenu
>>> +
>>> +menu "Distance sensors"
>>> +
>>> +config SRF08
>>> + tristate "Devantech SRF08 ultrasonic ranger sensor"
>>> + depends on I2C
>>> + help
>>> + Say Y here to build a driver for Devantech SRF08 ultrasonic
>>> + ranger sensor. This driver can be used to measure the distance
>>> + of objects.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called srf08.
>>> +
>>> +endmenu
>>> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
>>> index 9aadd9a8ee99..e914c2a5dd49 100644
>>> --- a/drivers/iio/proximity/Makefile
>>> +++ b/drivers/iio/proximity/Makefile
>>> @@ -5,4 +5,5 @@
>>> # When adding new entries keep the list in alphabetical order
>>> obj-$(CONFIG_AS3935) += as3935.o
>>> obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
>>> +obj-$(CONFIG_SRF08) += srf08.o
>>> obj-$(CONFIG_SX9500) += sx9500.o
>>> diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
>>> new file mode 100644
>>> index 000000000000..f38c74ed0933
>>> --- /dev/null
>>> +++ b/drivers/iio/proximity/srf08.c
>>> @@ -0,0 +1,362 @@
>>> +/*
>>> + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
>>> + *
>>> + * Copyright (c) 2016 Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
>>> + *
>>> + * This file is subject to the terms and conditions of version 2 of
>>> + * the GNU General Public License. See the file COPYING in the main
>>> + * directory of this archive for more details.
>>> + *
>>> + * For details about the device see:
>>> + * http://www.robot-electronics.co.uk/htm/srf08tech.html
>>> + */
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/module.h>
>>> +#include <linux/bitops.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +
>>> +/* registers of SRF08 device */
>>> +#define SRF08_WRITE_COMMAND 0x00 /* Command Register */
>>> +#define SRF08_WRITE_MAX_GAIN 0x01 /* Max Gain Register: 0 .. 31 */
>>> +#define SRF08_WRITE_RANGE 0x02 /* Range Register: 0 .. 255 */
>>> +#define SRF08_READ_SW_REVISION 0x00 /* Software Revision */
>>> +#define SRF08_READ_LIGHT 0x01 /* Light Sensor during last echo */
>>> +#define SRF08_READ_ECHO_1_HIGH 0x02 /* Range of first echo received */
>>> +#define SRF08_READ_ECHO_1_LOW 0x03 /* Range of first echo received */
>>> +
>>> +#define SRF08_CMD_RANGING_CM 0x51 /* Ranging Mode - Result in cm */
>>> +
>>> +#define SRF08_DEFAULT_GAIN 1025 /* max. analogue value of Gain */
>>> +#define SRF08_DEFAULT_RANGE 11008 /* max. value of Range in mm */
>>> +
>>> +struct srf08_data {
>>> + struct i2c_client *client;
>>> + int gain; /* Max Gain */
>>> + int range_mm; /* Range in mm */
>>> + struct mutex lock;
>>> +};
>>> +
>>> +static const int srf08_gain[] = {
>>> + 94, 97, 100, 103, 107, 110, 114, 118,
>>> + 123, 128, 133, 139, 145, 152, 159, 168,
>>> + 177, 187, 199, 212, 227, 245, 265, 288,
>>> + 317, 352, 395, 450, 524, 626, 777, 1025 };
>>> +
>>> +static int srf08_read_ranging(struct srf08_data *data)
>>> +{
>>> + struct i2c_client *client = data->client;
>>> + int ret, i;
>>> +
>>> + mutex_lock(&data->lock);
>>> +
>>> + ret = i2c_smbus_write_byte_data(data->client,
>>> + SRF08_WRITE_COMMAND, SRF08_CMD_RANGING_CM);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "write command - err: %d\n", ret);
>>> + mutex_unlock(&data->lock);
>>> + return ret;
>>> + }
>>> +
>>> + /*
>>> + * normally after 65 ms the device should have the read value
>>> + * we round it up to 100 ms
>> I'd suggest this should be adapted so that it takes advantage of knowing
>> roughly how long it is going to take as the 'range' maximum is changed.
>> So perhaps in the basic case, sleep for 65 msecs, then poll at 5msec
>> intervals. If we know it's going to be a lot faster, then poll it from
>> an earlier time.
>>> + *
>>> + * we read here until a correct version number shows up as
>>> + * suggested by the documentation
>>> + */
>>> + for (i = 0; i < 5; i++) {
>>> + ret = i2c_smbus_read_byte_data(data->client,
>>> + SRF08_READ_SW_REVISION);
>>> +
>>> + /* check if a valid version number is read */
>>> + if (ret < 255 && ret > 0)
>>> + break;
>>> + msleep(20);
>>> + }
>>> +
>>> + if (ret >= 255 || ret <= 0) {
>>> + dev_err(&client->dev, "device not ready\n");
>>> + mutex_unlock(&data->lock);
>>> + return -EIO;
>>> + }
>>> +
>>> + ret = i2c_smbus_read_word_swapped(data->client,
>>> + SRF08_READ_ECHO_1_HIGH);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "cannot read distance: ret=%d\n", ret);
>>> + mutex_unlock(&data->lock);
>>> + return ret;
>>> + }
>>> +
>>> + mutex_unlock(&data->lock);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int srf08_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *channel, int *val,
>>> + int *val2, long mask)
>>> +{
>>> + struct srf08_data *data = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + if (channel->type != IIO_DISTANCE)
>>> + return -EINVAL;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + ret = srf08_read_ranging(data);
>>> + if (ret < 0)
>>> + return ret;
>>> + *val = ret;
>>> + return IIO_VAL_INT;
>>> + case IIO_CHAN_INFO_SCALE:
>>> + /* 1 LSB is 1 cm */
>>> + *val = 0;
>>> + *val2 = 10000;
>>> + return IIO_VAL_INT_PLUS_MICRO;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +}
>>> +
>>> +static ssize_t srf08_show_range_mm_available(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + int i, len = 0;
>>> +
>>> + for (i = 0; i < 256; i++)
>>> + len += scnprintf(buf + len, PAGE_SIZE - len,
>>> + "%d ", (i + 1) * 43);
>>> +
>>> + buf[len - 1] = '\n';
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(range_mm_available, S_IRUGO,
>>> + srf08_show_range_mm_available, NULL, 0);
>>> +
>>> +static ssize_t srf08_show_range_mm(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct srf08_data *data = iio_priv(indio_dev);
>>> +
>>> + return sprintf(buf, "%d\n", data->range_mm);
>>> +}
>>> +
>>> +/*
>>> + * set the range of the sensor to an even multiple of 43 mm
>>> + * which corresponds to 1 LSB in the register
>>> + *
>>> + * register value corresponding range
>>> + * 0x00 43 mm
>>> + * 0x01 86 mm
>>> + * 0x02 129 mm
>>> + * ...
>>> + * 0xFF 11008 mm
>>> + */
>>> +static ssize_t srf08_write_range_mm(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct srf08_data *data = iio_priv(indio_dev);
>>> + struct i2c_client *client = data->client;
>>> + int ret;
>>> + unsigned int val, mod;
>>> + u8 regval;
>>> +
>>> + ret = kstrtouint(buf, 10, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = val / 43 - 1;
>>> + mod = val % 43;
>>> +
>>> + if (mod || (ret < 0) || (ret > 255))
>>> + return -EINVAL;
>>> +
>>> + regval = ret;
>>> +
>>> + mutex_lock(&data->lock);
>>> +
>>> + ret = i2c_smbus_write_byte_data(data->client,
>>> + SRF08_WRITE_RANGE, regval);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "write_range - err: %d\n", ret);
>>> + mutex_unlock(&data->lock);
>>> + return ret;
>>> + }
>>> +
>>> + data->range_mm = val;
>>> +
>>> + mutex_unlock(&data->lock);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(range_mm, S_IRUGO | S_IWUSR,
>>> + srf08_show_range_mm, srf08_write_range_mm, 0);
>>> +
>>> +static ssize_t srf08_show_gain_available(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + int i, len = 0;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
>>> + len += sprintf(buf + len, "%d ", srf08_gain[i]);
>>> +
>>> + len += sprintf(buf + len, "\n");
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(gain_available, S_IRUGO,
>>> + srf08_show_gain_available, NULL, 0);
>>> +
>>> +static ssize_t srf08_show_gain(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct srf08_data *data = iio_priv(indio_dev);
>>> + int len;
>>> +
>>> + len = sprintf(buf, "%d\n", data->gain);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static ssize_t srf08_write_gain(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct srf08_data *data = iio_priv(indio_dev);
>>> + struct i2c_client *client = data->client;
>>> + int ret, i;
>>> + unsigned int val;
>>> + u8 regval;
>>> +
>>> + ret = kstrtouint(buf, 10, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
>>> + if (val == srf08_gain[i]) {
>>> + regval = i;
>>> + break;
>>> + }
>>> +
>>> + if (i >= ARRAY_SIZE(srf08_gain))
>>> + return -EINVAL;
>>> +
>>> + mutex_lock(&data->lock);
>>> +
>>> + ret = i2c_smbus_write_byte_data(data->client,
>>> + SRF08_WRITE_MAX_GAIN, regval);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "write_gain - err: %d\n", ret);
>>> + mutex_unlock(&data->lock);
>>> + return ret;
>>> + }
>>> +
>>> + data->gain = val;
>>> +
>>> + mutex_unlock(&data->lock);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(gain, S_IRUGO | S_IWUSR,
>>> + srf08_show_gain, srf08_write_gain, 0);
>>> +
>>> +static struct attribute *srf08_attributes[] = {
>>> + &iio_dev_attr_range_mm.dev_attr.attr,
>>> + &iio_dev_attr_range_mm_available.dev_attr.attr,
>>> + &iio_dev_attr_gain.dev_attr.attr,
>>> + &iio_dev_attr_gain_available.dev_attr.attr,
>> Hmm. Custom attributes always give us issues. The primary point of IIO
>> is to enforce (more or less) standard interfaces.
>>
>> If you do need to add something new then that is fine (and I do think
>> you need to here!).
>>
>> They need to be formally proposed as an addition to the ABI with
>> docs in /Documentation/ABI/testing/sysfs-bus-iio*
>>
>> Once we take one driver using it it becomes part of our ABI that
>> userspace will need to handle, hence we consider these very
>> carefully.
>>
>> My gut feeling would be that gain needs to be more specific as it's
>> a term that can mean very different things.. Here we are talking
>> about an amplifier on a signal that we are then looking at the timing
>> of. It might otherwise be interpretted as another term for what
>> we term 'scale' in IIO.
>>
>> So what to call it... Perhaps afegain for Analog front end gain?
>> We might want to add this to the core supported attrs, but lets
>> not do so until we see if we have this on a number of devices.
>>
>
> In /Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935 there is also a
> gain used in a similar situation and it's called there "sensor_sensitivity"
>
> What it we also use this name here?
It's not ideal as it's not linked to a particular channel or anything,
but lets go with that as at least we will be consistent between drivers.
>
>> The description would need to make it explicit that this gain is
>> for cases where we aren't measuring the magnitude of what is
>> being amplified.
>>
>> For the range, it's an interesting one. Again the term range could
>> mean too many things within the wider ABI. We need to make it more
>> specific.
>>
>> Actually reading the datasheet, I think this is fundamentally about the
>> maximum sampling frequency rather than directly about the range.
>> The only reason you'd reduce the range is to speed that up. It doesn't
>> improve the resolution, the device simply answers quicker.
>>
>> So I'd support this as sampling_frequency. You could then use
>> the the iio_info_mask_*_available and relevant callback to provide
>> info on what it then restricts the possible output values to
>> (rather than controlling it directly).
>>
>
> By changing the range one cannot influence the sampling frequency directly. I
> have seen on the oszilloscope that the telegrams arrive almost at the same time
> with different settings of range and the same gain.
>
> Only if the gain is also adjusted the sensor works faster and a higher frequency
> can be used. But the gain is also used to adjust the sensitivity of the sensor.
That's rather weird and not what the datasheet suggests. Ah well.
>
> What about calling it "sensor_domain" or "sensor_max_range"?
hmm. Not sure - propose that with appropriate Docs and we can think more on it.
>
>
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group srf08_attribute_group = {
>>> + .attrs = srf08_attributes,
>>> +};
>>> +
>>> +static const struct iio_chan_spec srf08_channels[] = {
>>> + {
>>> + .type = IIO_DISTANCE,
>>> + .info_mask_separate =
>>> + BIT(IIO_CHAN_INFO_RAW) |
>>> + BIT(IIO_CHAN_INFO_SCALE),
>>> + },
>>> +};
>>> +
>>> +static const struct iio_info srf08_info = {
>>> + .read_raw = srf08_read_raw,
>>> + .attrs = &srf08_attribute_group,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int srf08_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct iio_dev *indio_dev;
>>> + struct srf08_data *data;
>>> +
>>> + if (!i2c_check_functionality(client->adapter,
>>> + I2C_FUNC_SMBUS_READ_BYTE_DATA |
>>> + I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
>>> + I2C_FUNC_SMBUS_READ_WORD_DATA))
>>> + return -ENODEV;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>> + if (!indio_dev)
>>> + return -ENOMEM;
>>> +
>>> + data = iio_priv(indio_dev);
>>> + i2c_set_clientdata(client, indio_dev);
>>> + data->client = client;
>>> +
>>> + /*
>>> + * set default values of device here
>>> + * these values are already set on the hardware after power on
>>> + */
>>> + data->gain = SRF08_DEFAULT_GAIN;
>>> + data->range_mm = SRF08_DEFAULT_RANGE;
>> We should be a little careful with assumptions about the device having
>> just been powered on. The driver might simply have been removed and
>> reprobed. So I'd sugest rewriting them whatever to be sure we have
>> what we expect. Either that or if they can be read back, then just
>> always retrieve them from the device.
>
> You are right.
> Then i'll set the default value at the sensor, because it cannot be read.
>
>>> +
>>> + indio_dev->name = dev_name(&client->dev);
>>> + indio_dev->dev.parent = &client->dev;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &srf08_info;
>>> + indio_dev->channels = srf08_channels;
>>> + indio_dev->num_channels = ARRAY_SIZE(srf08_channels);
>>> +
>>> + mutex_init(&data->lock);
>>> +
>>> + return devm_iio_device_register(&client->dev, indio_dev);
>>> +}
>>> +
>>> +static const struct i2c_device_id srf08_id[] = {
>>> + { "srf08", 0 },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, srf08_id);
>>> +
>>> +static struct i2c_driver srf08_driver = {
>>> + .driver = {
>>> + .name = "srf08",
>>> + },
>>> + .probe = srf08_probe,
>>> + .id_table = srf08_id,
>>> +};
>>> +module_i2c_driver(srf08_driver);
>>> +
>>> +MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
>>> +MODULE_DESCRIPTION("Devantech SRF08 ultrasonic ranger driver");
>>> +MODULE_LICENSE("GPL");
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Jonathan Cameron @ 2017-01-15 13:09 UTC (permalink / raw)
To: Lars-Peter Clausen, Martin Blumenstingl, knaack.h, pmeerw,
robh+dt, mark.rutland, khilman, linux-iio, devicetree,
linux-amlogic, linux-clk
Cc: carlo, catalin.marinas, will.deacon, mturquette, sboyd,
narmstrong, linux-arm-kernel
In-Reply-To: <0bfa97d1-2ee3-9dd9-dce3-593e93771f03@metafoo.de>
On 15/01/17 12:26, Lars-Peter Clausen wrote:
> On 01/11/2017 06:43 PM, Martin Blumenstingl wrote:
>> + indio_dev->name = dev_name(&pdev->dev);
>
> The name is supposed to be the type of the device, e.g. part name, not the
> name of parent device instance. E.g. meson-gxbb-saradc or meson-gxl-saradc
> in this case.
Drat. I've been missing these again. Somehow, never manage to get them
onto those things my eyes pick up whilst reviewing.
Thanks Lars,
Jonathan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Jonathan Cameron @ 2017-01-15 13:08 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, khilman, linux-iio,
devicetree, linux-amlogic, linux-clk, carlo, catalin.marinas,
will.deacon, mturquette, sboyd, narmstrong, linux-arm-kernel,
Russell King
In-Reply-To: <CAFBinCB75JG+F-ZgeO4+q-S9AvrYKYn6AVEdgaDa1yv=cE=Ujw@mail.gmail.com>
On 14/01/17 17:44, Martin Blumenstingl wrote:
> Hi Jonathan,
>
> thank you for the review!
> (further comments from me inline)
> I think I'll send an updated version on Monday.
>
> On Sat, Jan 14, 2017 at 3:46 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 11/01/17 17:43, Martin Blumenstingl wrote:
>>> This adds support for the SAR (Successive Approximation Register) ADC
>>> on the Amlogic Meson SoCs.
>>>
>>> The code is based on the public S805 (Meson8b) and S905 (GXBB)
>>> datasheets, as well as by reading (various versions of) the vendor
>>> driver and by inspecting the registers on the vendor kernels of my
>>> testing-hardware.
>>>
>>> Currently the GXBB, GXL and GXM SoCs are supported. GXBB hardware has
>>> 10-bit ADC resolution, while GXL and GXM have 12-bit ADC resolution.
>>> The code was written to support older SoCs (Meson8 and Meson8b) as well,
>>> but due to lack of actual testing-hardware no of_device_id was added for
>>> these.
>>>
>>> Two "features" from the vendor driver are currently missing:
>>> - the vendor driver uses channel #7 for calibration (this improves the
>>> accuracy of the results - in my tests the results were less than 3%
>>> off without calibration compared to the vendor driver). Adding support
>>> for this should be easy, but is not required for most applications.
>>> - channel #6 is connected to the SoCs internal temperature sensor.
>>> Adding support for this is probably not so easy since (based on the
>>> u-boot sources) most SoC versions are using different registers and
>>> algorithms for the conversion from "ADC value" to temperature.
>>>
>>> Supported by the hardware but currently not supported by the driver:
>>> - reading multiple channels at the same time (the hardware has a FIFO
>>> buffer which stores multiple results)
>>> - continuous sampling (this would require a way to enable this
>>> individually because otherwise the ADC would be drawing power
>>> constantly)
>>> - interrupt support (similar to the vendor driver this new driver is
>>> polling the results. It is unclear if the IRQ-mode is supported on
>>> older (Meson6 or Meson8) hardware as well or if there are any errata)
>>>
>> Russell Cc'd for a quick question on the clk api.
> a quick side-note the clk API: my driver is a clock consumer and
> provider at the same time. This seems to be a recurring pattern in
> Amlogic hardware designs (as the MMC and DWMAC glue drivers are doing
> this also), see [0]
>
>> Ideally include a source for datasheets if available. Saves time googling and
>> perhaps getting the wrong thing!
> OK, will do this in v2
>
>> A few other minor comments inline. Pretty good V1.
> thanks :-)
>
>> Jonathan
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> ---
>>> drivers/iio/adc/Kconfig | 12 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/meson_saradc.c | 860 +++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 873 insertions(+)
>>> create mode 100644 drivers/iio/adc/meson_saradc.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 9c8b558ba19e..86059b9b91bf 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -371,6 +371,18 @@ config MEN_Z188_ADC
>>> This driver can also be built as a module. If so, the module will be
>>> called men_z188_adc.
>>>
>>> +config MESON_SARADC
>>> + tristate "Amlogic Meson SAR ADC driver"
>>> + default ARCH_MESON
>>> + depends on OF && COMMON_CLK && (ARCH_MESON || COMPILE_TEST)
>>> + select REGMAP_MMIO
>>> + help
>>> + Say yes here to build support for the SAR ADC found in Amlogic Meson
>>> + SoCs.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called meson_saradc.
>>> +
>>> config MXS_LRADC
>>> tristate "Freescale i.MX23/i.MX28 LRADC"
>>> depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index d36c4be8d1fc..de05b9e75f8f 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -36,6 +36,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o
>>> obj-$(CONFIG_MCP3422) += mcp3422.o
>>> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>>> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>>> +obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
>>> obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
>>> obj-$(CONFIG_NAU7802) += nau7802.o
>>> obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>>> new file mode 100644
>>> index 000000000000..06e8ac620385
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/meson_saradc.c
>>> @@ -0,0 +1,860 @@
>>> +/*
>>> + * Amlogic Meson Successive Approximation Register (SAR) A/D Converter
>>> + *
>>> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define SAR_ADC_REG0 0x00
>>> + #define SAR_ADC_REG0_PANEL_DETECT BIT(31)
>>> + #define SAR_ADC_REG0_BUSY_MASK GENMASK(30, 28)
>>> + #define SAR_ADC_REG0_DELTA_BUSY BIT(30)
>>> + #define SAR_ADC_REG0_AVG_BUSY BIT(29)
>>> + #define SAR_ADC_REG0_SAMPLE_BUSY BIT(28)
>>> + #define SAR_ADC_REG0_FIFO_FULL BIT(27)
>>> + #define SAR_ADC_REG0_FIFO_EMPTY BIT(26)
>>> + #define SAR_ADC_REG0_FIFO_COUNT_MASK GENMASK(25, 21)
>>> + #define SAR_ADC_REG0_ADC_BIAS_CTRL_MASK GENMASK(20, 19)
>>> + #define SAR_ADC_REG0_CURR_CHAN_ID_MASK GENMASK(18, 16)
>>> + #define SAR_ADC_REG0_ADC_TEMP_SEN_SEL BIT(15)
>>> + #define SAR_ADC_REG0_SAMPLING_STOP BIT(14)
>>> + #define SAR_ADC_REG0_CHAN_DELTA_EN_MASK GENMASK(13, 12)
>>> + #define SAR_ADC_REG0_DETECT_IRQ_POL BIT(10)
>>> + #define SAR_ADC_REG0_DETECT_IRQ_EN BIT(9)
>>> + #define SAR_ADC_REG0_FIFO_CNT_IRQ_MASK GENMASK(8, 4)
>>> + #define SAR_ADC_REG0_FIFO_IRQ_EN BIT(3)
>>> + #define SAR_ADC_REG0_SAMPLING_START BIT(2)
>>> + #define SAR_ADC_REG0_CONTINUOUS_EN BIT(1)
>>> + #define SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE BIT(0)
>>> +
>>> +#define SAR_ADC_CHAN_LIST 0x04
>>> + #define SAR_ADC_CHAN_LIST_MAX_INDEX_MASK GENMASK(26, 24)
>>> + #define SAR_ADC_CHAN_CHAN_ENTRY_MASK(_chan) \
>>> + (GENMASK(2, 0) << (_chan * 3))
>>> +
>>> +#define SAR_ADC_AVG_CNTL 0x08
>>> + #define SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(_chan) \
>>> + (16 + (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_AVG_MODE_MASK(_chan) \
>>> + (GENMASK(17, 16) << (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(_chan) \
>>> + (0 + (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(_chan) \
>>> + (GENMASK(1, 0) << (_chan * 2))
>>> +
>>> +#define SAR_ADC_REG3 0x0c
>>> + #define SAR_ADC_REG3_CNTL_USE_SC_DLY BIT(31)
>>> + #define SAR_ADC_REG3_CLK_EN BIT(30)
>>> + #define SAR_ADC_REG3_BL30_INITIALIZED BIT(28)
>>> + #define SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN BIT(27)
>>> + #define SAR_ADC_REG3_CTRL_SAMPLING_CLOCK_PHASE BIT(26)
>>> + #define SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_REG3_DETECT_EN BIT(22)
>>> + #define SAR_ADC_REG3_ADC_EN BIT(21)
>>> + #define SAR_ADC_REG3_PANEL_DETECT_COUNT_MASK GENMASK(20, 18)
>>> + #define SAR_ADC_REG3_PANEL_DETECT_FILTER_TB_MASK GENMASK(17, 16)
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_SHIFT 10
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_WIDTH 5
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_MASK GENMASK(15, 10)
>>> + #define SAR_ADC_REG3_BLOCK_DLY_SEL_MASK GENMASK(9, 8)
>>> + #define SAR_ADC_REG3_BLOCK_DLY_MASK GENMASK(7, 0)
>>> +
>>> +#define SAR_ADC_DELAY 0x10
>>> + #define SAR_ADC_DELAY_INPUT_DLY_SEL_MASK GENMASK(25, 24)
>>> + #define SAR_ADC_DELAY_BL30_BUSY BIT(15)
>>> + #define SAR_ADC_DELAY_KERNEL_BUSY BIT(14)
>>> + #define SAR_ADC_DELAY_INPUT_DLY_CNT_MASK GENMASK(23, 16)
>>> + #define SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK GENMASK(9, 8)
>>> + #define SAR_ADC_DELAY_SAMPLE_DLY_CNT_MASK GENMASK(7, 0)
>>> +
>>> +#define SAR_ADC_LAST_RD 0x14
>>> + #define SAR_ADC_LAST_RD_LAST_CHANNEL1_MASK GENMASK(23, 16)
>>> + #define SAR_ADC_LAST_RD_LAST_CHANNEL0_MASK GENMASK(9, 0)
>>> +
>>> +#define SAR_ADC_FIFO_RD 0x18
>>> + #define SAR_ADC_FIFO_RD_CHAN_ID_MASK GENMASK(14, 12)
>>> + #define SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK GENMASK(11, 0)
>>> +
>>> +#define SAR_ADC_AUX_SW 0x1c
>>> + #define SAR_ADC_AUX_SW_MUX_SEL_CHAN_MASK(_chan) \
>>> + (GENMASK(10, 8) << ((_chan - 2) * 2))
>>> + #define SAR_ADC_AUX_SW_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_AUX_SW_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_AUX_SW_MODE_SEL BIT(4)
>>> + #define SAR_ADC_AUX_SW_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_AUX_SW_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_AUX_SW_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_AUX_SW_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_CHAN_10_SW 0x20
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_MUX_SEL_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_VREF_P_MUX BIT(22)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_VREF_N_MUX BIT(21)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_MODE_SEL BIT(20)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_YP_DRIVE_SW BIT(19)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_XP_DRIVE_SW BIT(18)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_YM_DRIVE_SW BIT(17)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_XM_DRIVE_SW BIT(16)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_MUX_SEL_MASK GENMASK(9, 7)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_MODE_SEL BIT(4)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_DETECT_IDLE_SW 0x24
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_SW_EN BIT(26)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_VREF_P_MUX BIT(22)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_VREF_N_MUX BIT(21)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_SEL BIT(20)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_YP_DRIVE_SW BIT(19)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_XP_DRIVE_SW BIT(18)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_YM_DRIVE_SW BIT(17)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_XM_DRIVE_SW BIT(16)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK GENMASK(9, 7)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_SEL BIT(4)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_DELTA_10 0x28
>>> + #define SAR_ADC_DELTA_10_TEMP_SEL BIT(27)
>>> + #define SAR_ADC_DELTA_10_TS_REVE1 BIT(26)
>>> + #define SAR_ADC_DELTA_10_CHAN1_DELTA_VALUE_SHIFT 16
>>> + #define SAR_ADC_DELTA_10_CHAN1_DELTA_VALUE_MASK GENMASK(25, 16)
>>> + #define SAR_ADC_DELTA_10_TS_REVE0 BIT(15)
>>> + #define SAR_ADC_DELTA_10_TS_C_SHIFT 11
>>> + #define SAR_ADC_DELTA_10_TS_C_MASK GENMASK(14, 11)
>>> + #define SAR_ADC_DELTA_10_TS_VBG_EN BIT(10)
>>> + #define SAR_ADC_DELTA_10_CHAN0_DELTA_VALUE_SHIFT 0
>>> + #define SAR_ADC_DELTA_10_CHAN0_DELTA_VALUE_MASK GENMASK(9, 0)
>>> +
>>> +/* NOTE: registers from here are undocumented (the vendor Linux kernel driver
>>> + * and u-boot source served as reference). These only seem to be relevant on
>>> + * GXBB and newer.
>>> + */
>>> +#define SAR_ADC_REG11 0x2c
>>> + #define SAR_ADC_REG11_BANDGAP_EN BIT(13)
>>> +
>>> +#define SAR_ADC_REG13 0x34
>>> + #define SAR_ADC_REG13_12BIT_CALIBRATION_MASK GENMASK(13, 8)
>>> +
>>> +#define SAR_ADC_MAX_FIFO_SIZE 32
>>> +#define SAR_ADC_NUM_CHANNELS ARRAY_SIZE(meson_saradc_iio_channels)
>>> +#define SAR_ADC_VALUE_MASK(_priv) (BIT(_priv->resolution) - 1)
>>> +
>>> +#define MESON_SAR_ADC_CHAN(_chan, _type) { \
>>> + .type = _type, \
>>> + .indexed = true, \
>>> + .channel = _chan, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>> + BIT(IIO_CHAN_INFO_AVERAGE_RAW), \
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>>> + .datasheet_name = "SAR_ADC_CH"#_chan, \
>>> +}
>>> +
>>> +/* TODO: the hardware supports IIO_TEMP for channel 6 as well which is
>> Multline comment syntax.
> I got this wrong in 2 other places as well. will be fixed in v2, thanks!
>
>>> + * currently not supported by this driver.
>>> + */
>>> +static const struct iio_chan_spec meson_saradc_iio_channels[] = {
>>> + MESON_SAR_ADC_CHAN(0, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(1, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(2, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(3, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(4, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(5, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(6, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(7, IIO_VOLTAGE),
>>> + IIO_CHAN_SOFT_TIMESTAMP(8),
>>> +};
>>> +
>>> +enum meson_saradc_avg_mode {
>>> + NO_AVERAGING = 0x0,
>>> + MEAN_AVERAGING = 0x1,
>>> + MEDIAN_AVERAGING = 0x2,
>>> +};
>>> +
>>> +enum meson_saradc_num_samples {
>>> + ONE_SAMPLE = 0x0,
>>> + TWO_SAMPLES = 0x1,
>>> + FOUR_SAMPLES = 0x2,
>>> + EIGHT_SAMPLES = 0x3,
>>> +};
>>> +
>>> +enum meson_saradc_chan7_mux_sel {
>>> + CHAN7_MUX_VSS = 0x0,
>>> + CHAN7_MUX_VDD_DIV4 = 0x1,
>>> + CHAN7_MUX_VDD_DIV2 = 0x2,
>>> + CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
>>> + CHAN7_MUX_VDD = 0x4,
>>> + CHAN7_MUX_CH7_INPUT = 0x7,
>>> +};
>>> +
>>> +struct meson_saradc_priv {
>>> + struct regmap *regmap;
>>> + struct clk *clkin;
>>> + struct clk *core_clk;
>>> + struct clk *sana_clk;
>>> + struct clk *adc_sel_clk;
>>> + struct clk *adc_clk;
>>> + struct clk_gate clk_gate;
>>> + struct clk *adc_div_clk;
>>> + struct clk_divider clk_div;
>>> + struct regulator *vref;
>>> + struct completion completion;
>>> + u8 resolution;
>>> +};
>>> +
>>> +static const struct regmap_config meson_saradc_regmap_config = {
>>> + .reg_bits = 8,
>>> + .val_bits = 32,
>>> + .reg_stride = 4,
>>> + .max_register = SAR_ADC_REG13,
>>> +};
>>> +
>>> +static unsigned int meson_saradc_get_fifo_count(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> +
>>> + return FIELD_GET(SAR_ADC_REG0_FIFO_COUNT_MASK, regval);
>>> +}
>>> +
>>> +static int meson_saradc_wait_busy_clear(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int regval, timeout = 10000;
>>> +
>>> + do {
>>> + udelay(1);
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> + } while (FIELD_GET(SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
>>> +
>>> + if (timeout < 0)
>>> + return -ETIMEDOUT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_read_raw_sample(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
>>> +
>>> + ret = meson_saradc_wait_busy_clear(indio_dev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> +
>>> + while (meson_saradc_get_fifo_count(indio_dev) > 0 &&
>>> + count < SAR_ADC_MAX_FIFO_SIZE) {
>>> + regmap_read(priv->regmap, SAR_ADC_FIFO_RD, ®val);
>>> +
>>> + fifo_chan = FIELD_GET(SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>>> + if (fifo_chan == chan->channel) {
>>> + fifo_val = FIELD_GET(SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK,
>>> + regval) & SAR_ADC_VALUE_MASK(priv);
>>> + sum += fifo_val;
>>> + count++;
>>> + }
>>> + }
>>> +
>>> + if (!count)
>>> + return -ENOENT;
>>> +
>>> + *val = sum / count;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_saradc_set_averaging(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + enum meson_saradc_avg_mode mode,
>>> + enum meson_saradc_num_samples samples)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 val;
>>> +
>>> + val = samples << SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>> + SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(chan->channel),
>>> + val);
>>> +
>>> + val = mode << SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>> + SAR_ADC_AVG_CNTL_AVG_MODE_MASK(chan->channel), val);
>>> +}
>>> +
>>> +static void meson_saradc_enable_channel(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + /* the SAR ADC engine allows sampling multiple channels at the same
>>> + * time. to keep it simple we're only working with one *internal*
>>> + * channel, which starts counting at index 0 (which means: count = 1).
>>> + */
>>> + regval = FIELD_PREP(SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>> + SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, regval);
>>> +
>>> + /* map channel index 0 to the channel which we want to read */
>>> + regval = FIELD_PREP(SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>> + SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), regval);
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>> + chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>> + SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>> + regval);
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>> + chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>> + SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>> + regval);
>>> +
>>> + if (chan->channel == 6)
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELTA_10,
>>> + SAR_ADC_DELTA_10_TEMP_SEL, 0);
>>> +}
>>> +
>>> +static void meson_saradc_set_channel7_mux(struct iio_dev *indio_dev,
>>> + enum meson_saradc_chan7_mux_sel sel)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
>>> +
>>> + usleep_range(10, 20);
>>> +}
>>> +
>>> +static void meson_saradc_start_sample_engine(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLING_START,
>>> + SAR_ADC_REG0_SAMPLING_START);
>>> +}
>>> +
>>> +static void meson_saradc_stop_sample_engine(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLING_STOP,
>>> + SAR_ADC_REG0_SAMPLING_STOP);
>>> +
>>> + /* wait until all modules are stopped */
>>> + meson_saradc_wait_busy_clear(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE, 0);
>>> +}
>>> +
>>> +static void meson_saradc_lock(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int val;
>>> +
>>> + mutex_lock(&indio_dev->mlock);
>>> +
>>> + /* prevent BL30 from using the SAR ADC while we are using it */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY);
>>> +
>>> + /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
>>> + do {
>>> + udelay(1);
>>> + regmap_read(priv->regmap, SAR_ADC_DELAY, &val);
>>> + } while (val & SAR_ADC_DELAY_BL30_BUSY);
>>> +}
>>> +
>>> +static void meson_saradc_unlock(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + /* allow BL30 to use the SAR ADC again */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY, 0);
>>> +
>>> + mutex_unlock(&indio_dev->mlock);
>>> +}
>>> +
>>> +static int meson_saradc_get_sample(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + enum meson_saradc_avg_mode avg_mode,
>>> + enum meson_saradc_num_samples avg_samples,
>>> + int *val)
>>> +{
>>> + int ret, tmp;
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + /* clear old values from the FIFO buffer, ignoring errors */
>>> + meson_saradc_read_raw_sample(indio_dev, chan, &tmp);
>>> +
>>> + meson_saradc_set_averaging(indio_dev, chan, avg_mode, avg_samples);
>>> +
>>> + meson_saradc_enable_channel(indio_dev, chan);
>>> +
>>> + meson_saradc_start_sample_engine(indio_dev);
>>> + ret = meson_saradc_read_raw_sample(indio_dev, chan, val);
>>> + meson_saradc_stop_sample_engine(indio_dev);
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +
>>> + if (ret) {
>>> + dev_warn(&indio_dev->dev,
>>> + "failed to read sample for channel %d: %d\n",
>>> + chan->channel, ret);
>>> + return ret;
>>> + }
>>> +
>>> + return IIO_VAL_INT;
>>> +}
>>> +
>>> +static int meson_saradc_iio_info_read_raw(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val, int *val2, long mask)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + return meson_saradc_get_sample(indio_dev, chan, NO_AVERAGING,
>>> + ONE_SAMPLE, val);
>>> + break;
>>> +
>>> + case IIO_CHAN_INFO_AVERAGE_RAW:
>>> + return meson_saradc_get_sample(indio_dev, chan, MEAN_AVERAGING,
>>> + EIGHT_SAMPLES, val);
>>> + break;
>>> +
>>> + case IIO_CHAN_INFO_SCALE:
>>> + ret = regulator_get_voltage(priv->vref);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "failed to get vref voltage: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + *val = ret / 1000;
>>> + *val2 = priv->resolution;
>>> + return IIO_VAL_FRACTIONAL_LOG2;
>>> +
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +}
>>> +
>>> +static int meson_saradc_clk_init(struct iio_dev *indio_dev, void __iomem *base)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + struct clk_init_data init;
>>> + char clk_name[32];
>>> + const char *clk_parents[1];
>>> +
>>> + snprintf(clk_name, sizeof(clk_name), "%s#adc_div",
>>> + of_node_full_name(indio_dev->dev.of_node));
>>> + init.name = devm_kstrdup(&indio_dev->dev, clk_name, GFP_KERNEL);
>>> + init.flags = 0;
>>> + init.ops = &clk_divider_ops;
>>> + clk_parents[0] = __clk_get_name(priv->clkin);
>>> + init.parent_names = clk_parents;
>>> + init.num_parents = 1;
>>> +
>>> + priv->clk_div.reg = base + SAR_ADC_REG3;
>>> + priv->clk_div.shift = SAR_ADC_REG3_ADC_CLK_DIV_SHIFT;
>>> + priv->clk_div.width = SAR_ADC_REG3_ADC_CLK_DIV_WIDTH;
>>> + priv->clk_div.hw.init = &init;
>>> + priv->clk_div.flags = 0;
>>> +
>>> + priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
>>> + &priv->clk_div.hw);
>>> + if (WARN_ON(IS_ERR(priv->adc_div_clk)))
>>> + return PTR_ERR(priv->adc_div_clk);
>>> +
>>> + snprintf(clk_name, sizeof(clk_name), "%s#adc_en",
>>> + of_node_full_name(indio_dev->dev.of_node));
>>> + init.name = devm_kstrdup(&indio_dev->dev, clk_name, GFP_KERNEL);
>>> + init.flags = CLK_SET_RATE_PARENT;
>>> + init.ops = &clk_gate_ops;
>>> + clk_parents[0] = __clk_get_name(priv->adc_div_clk);
>>> + init.parent_names = clk_parents;
>>> + init.num_parents = 1;
>>> +
>>> + priv->clk_gate.reg = base + SAR_ADC_REG3;
>>> + priv->clk_gate.bit_idx = fls(SAR_ADC_REG3_CLK_EN);
>>> + priv->clk_gate.hw.init = &init;
>>> +
>>> + priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
>>> + if (WARN_ON(IS_ERR(priv->adc_clk)))
>>> + return PTR_ERR(priv->adc_clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_init(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int regval, ret;
>>> +
>>> + /* make sure we start at CH7 input */
>> why? Seems like a little more detail would be good here ;)
> I'll change this to "make sure we start at CH7 input since the other
> muxes are only used for internal calibration." in v2
>
>>> + meson_saradc_set_channel7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG3, ®val);
>>> + if (regval & SAR_ADC_REG3_BL30_INITIALIZED) {
>>> + dev_info(&indio_dev->dev, "already initialized by BL30\n");
>>> + return 0;
>>> + }
>>> +
>>> + dev_info(&indio_dev->dev, "initializing SAR ADC\n");
>> I'd argue this provides no useful info so should be dropped.
>> Useful for debugging no doubt, but just noise going forward.
> do you want me to remove them or should I turn them into dev_dbg() (so
> they can be enabled for debugging purposes)?
Drop it entirely.
>
>>> +
>>> + meson_saradc_stop_sample_engine(indio_dev);
>>> +
>>> + /* update the channel 6 MUX to select the temperature sensor */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_ADC_TEMP_SEN_SEL,
>>> + SAR_ADC_REG0_ADC_TEMP_SEN_SEL);
>>> +
>>> + /* disable all channels by default */
>>> + regmap_write(priv->regmap, SAR_ADC_CHAN_LIST, 0x0);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CTRL_SAMPLING_CLOCK_PHASE, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CNTL_USE_SC_DLY,
>>> + SAR_ADC_REG3_CNTL_USE_SC_DLY);
>>> +
>>> + /* delay between two samples = (10+1) * 1uS */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_CNT_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_SAMPLE_DLY_CNT_MASK, 10));
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK, 0));
>>> +
>>> + /* delay between two samples = (10+1) * 1uS */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_CNT_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_INPUT_DLY_CNT_MASK, 10));
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_SEL_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_INPUT_DLY_SEL_MASK, 1));
>>> +
>> Cool. I hadn't come across FIELD_PREP before. Neater and tidier than having
>> a shift and a mask for at least some usecases.
> I think these were introduced with v4.9. I like them because I tend
> use GENMASK() incorrectly and with those macros I get an error at
> compile-time (without having to debug my code at all)
>
>>> + ret = clk_set_parent(priv->adc_sel_clk, priv->clkin);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev,
>>> + "failed to set adc parent to clkin\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = clk_set_rate(priv->adc_clk, 1200000);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to set adc clock rate\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_hw_enable(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + ret = regulator_enable(priv->vref);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev, "failed to enable vref regulator\n");
>>> + goto err_vref;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->core_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable core clk\n");
>>> + goto err_core_clk;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->sana_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable sana clk\n");
>>> + goto err_sana_clk;
>>> + }
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG11,
>>> + SAR_ADC_REG11_BANDGAP_EN, SAR_ADC_REG11_BANDGAP_EN);
>> Is this controlling an offset for a bandgap or some actual electronics?
>> Not sure if it should be disabled on error and the datasheets I've found are
>> far from great! You disable it in the disable, so I'd expect it to be
>> unwound on error in here too.
> actually the bandgap is not documented at all :(
>
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3, SAR_ADC_REG3_ADC_EN,
>>> + SAR_ADC_REG3_ADC_EN);
>> The fact you turn this of in the disable suggests to me that on error
>> you should be doing it in here too.
> I will disable this along with SAR_ADC_REG11_BANDGAP_EN in the
> err_adc_clk label, thanks for spotting this.
>
>>> +
>>> + udelay(5);
>>> +
>>> + ret = clk_prepare_enable(priv->adc_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable adc_en clk\n");
>>> + goto err_adc_clk;
>>> + }
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +
>>> + return 0;
>>> +
>>> +err_adc_clk:
>>> + clk_disable_unprepare(priv->sana_clk);
>>> +err_sana_clk:
>>> + clk_disable_unprepare(priv->core_clk);
>>> +err_core_clk:
>>> + regulator_disable(priv->vref);
>>> +err_vref:
>>> + meson_saradc_unlock(indio_dev);
>>> + return ret;
>>> +}
>>> +
>>> +static void meson_saradc_hw_disable(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + clk_disable_unprepare(priv->adc_clk);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3, SAR_ADC_REG3_ADC_EN, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG11,
>>> + SAR_ADC_REG11_BANDGAP_EN, 0);
>>> +
>>> + clk_disable_unprepare(priv->sana_clk);
>>> + clk_disable_unprepare(priv->core_clk);
>>> +
>>> + regulator_disable(priv->vref);
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +}
>>> +
>>> +static const struct iio_info meson_saradc_iio_info = {
>>> + .read_raw = meson_saradc_iio_info_read_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static const struct of_device_id meson_saradc_of_match[] = {
>>> + {
>>> + .compatible = "amlogic,meson-gxbb-saradc",
>>> + .data = (void *)10,
>> Might have been worth having a structure array indexed from an enum.
>> For now it is overkill, but seems likely there are a few other differences
>> that aren't supported yet?
> what do you mean with "structure array indexed from an enum"? I can
> introduce some match-specific struct if you want (just like it's done
> in rockchip_saradc.c with "struct rockchip_saradc_data").
That works, or data can be an enum indexing an array of similar structures.
It doesn't really matter.
>
>>> + }, {
>>> + .compatible = "amlogic,meson-gxl-saradc",
>>> + .data = (void *)12,
>>> + },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, meson_saradc_of_match);
>>> +
>>> +static int meson_saradc_probe(struct platform_device *pdev)
>>> +{
>>> + struct meson_saradc_priv *priv;
>>> + struct iio_dev *indio_dev;
>>> + struct resource *res;
>>> + void __iomem *base;
>>> + const struct of_device_id *match;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
>>> + if (!indio_dev) {
>>> + dev_err(&pdev->dev, "failed allocating iio device\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + priv = iio_priv(indio_dev);
>>> +
>>> + match = of_match_device(meson_saradc_of_match, &pdev->dev);
>>> + priv->resolution = (unsigned long)match->data;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(base))
>>> + return PTR_ERR(base);
>>> +
>>> + priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>> + &meson_saradc_regmap_config);
>>> + if (IS_ERR(priv->regmap))
>>> + return PTR_ERR(priv->regmap);
>>> +
>>> + init_completion(&priv->completion);
>>> +
>>> + priv->clkin = devm_clk_get(&pdev->dev, "clkin");
>>> + if (IS_ERR(priv->clkin)) {
>>> + dev_err(&pdev->dev, "failed to get clkin\n");
>>> + return PTR_ERR(priv->clkin);
>>> + }
>>> +
>>> + priv->core_clk = devm_clk_get(&pdev->dev, "core");
>>> + if (IS_ERR(priv->core_clk)) {
>>> + dev_err(&pdev->dev, "failed to get core clk\n");
>>> + return PTR_ERR(priv->core_clk);
>>> + }
>>> +
>>> + priv->sana_clk = devm_clk_get(&pdev->dev, "sana");
>> Oh for a devm_clk_get_optional to handle this boiler plate neatly.
>> It's been suggested before, but nothing seems to have come of it.
> I guess quite a few drivers would benefit from that. maybe we should
> take this to the linux-clk list again?
Perhaps. I guess this has been proposed before but gone nowhere.
I haven't digged into why though.
>
>> Some array clk get functions might also clean this up a touch.
> yes, unfortunately in this case it's not that easy as it would have to
> allow a mix of mandatory and optional clocks. Additionally I cannot
> bulk-enable them unconditionally since some of these are simple gates,
> others need to be reparented and for some the rate has to be set.
>
>>> + if (IS_ERR(priv->sana_clk)) {
>>> + if (PTR_ERR(priv->sana_clk) == -ENOENT) {
>>> + priv->sana_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get sana clk\n");
>>> + return PTR_ERR(priv->sana_clk);
>>> + }
>>> + }
>>> +
>>> + priv->adc_clk = devm_clk_get(&pdev->dev, "adc_clk");
>>> + if (IS_ERR(priv->adc_clk)) {
>>> + if (PTR_ERR(priv->adc_clk) == -ENOENT) {
>>> + priv->adc_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get adc clk\n");
>>> + return PTR_ERR(priv->adc_clk);
>>> + }
>>> + }
>>> +
>>> + priv->adc_sel_clk = devm_clk_get(&pdev->dev, "adc_sel");
>>> + if (IS_ERR(priv->adc_sel_clk)) {
>>> + if (PTR_ERR(priv->adc_sel_clk) == -ENOENT) {
>>> + priv->adc_sel_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get adc_sel clk\n");
>>> + return PTR_ERR(priv->adc_sel_clk);
>>> + }
>>> + }
>>> +
>>> + /* on pre-GXBB SoCs the SAR ADC itself provides the ADC clock: */
>>> + if (!priv->adc_clk) {
>>> + ret = meson_saradc_clk_init(indio_dev, base);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + priv->vref = devm_regulator_get(&pdev->dev, "vref");
>>> + if (IS_ERR(priv->vref)) {
>>> + dev_err(&pdev->dev, "failed to get vref regulator\n");
>>> + return PTR_ERR(priv->vref);
>>> + }
>>> +
>>> + ret = meson_saradc_init(indio_dev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> + ret = meson_saradc_hw_enable(indio_dev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> + platform_set_drvdata(pdev, indio_dev);
>>> +
>>> + indio_dev->name = dev_name(&pdev->dev);
>>> + indio_dev->dev.parent = &pdev->dev;
>>> + indio_dev->dev.of_node = pdev->dev.of_node;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &meson_saradc_iio_info;
>>> +
>>> + indio_dev->channels = meson_saradc_iio_channels;
>>> + indio_dev->num_channels = SAR_ADC_NUM_CHANNELS;
>>> +
>>> + ret = iio_device_register(indio_dev);
>>> + if (ret)
>>> + goto err_hw;
>>> +
>>> + return 0;
>>> +
>>> +err_hw:
>>> + meson_saradc_hw_disable(indio_dev);
>>> +err:
>>> + return ret;
>>> +}
>>> +
>>> +static int meson_saradc_remove(struct platform_device *pdev)
>>> +{
>>> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>> +
>>> + meson_saradc_hw_disable(indio_dev);
>>> + iio_device_unregister(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int meson_saradc_suspend(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +
>>> + meson_saradc_hw_disable(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_resume(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +
>>> + return meson_saradc_hw_enable(indio_dev);
>>> +}
>>> +#endif /* CONFIG_PM_SLEEP */
>>> +
>>> +static SIMPLE_DEV_PM_OPS(meson_saradc_pm_ops,
>>> + meson_saradc_suspend, meson_saradc_resume);
>>> +
>>> +static struct platform_driver meson_saradc_driver = {
>>> + .probe = meson_saradc_probe,
>>> + .remove = meson_saradc_remove,
>>> + .driver = {
>>> + .name = "meson-saradc",
>>> + .of_match_table = meson_saradc_of_match,
>>> + .pm = &meson_saradc_pm_ops,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(meson_saradc_driver);
>>> +
>>> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
>>> +MODULE_DESCRIPTION("Amlogic Meson SAR ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>
>
> Regards,
> Martin
>
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000986.html
>
^ permalink raw reply
* Re: [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Lars-Peter Clausen @ 2017-01-15 12:26 UTC (permalink / raw)
To: Martin Blumenstingl, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
khilman-rdvid1DuHRBWk0Htik3J/w, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-clk-u79uwXL29TY76Z2rM5mHXA
Cc: carlo-KA+7E9HrN00dnm+yROfE0A, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, mturquette-rdvid1DuHRBWk0Htik3J/w,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, narmstrong-rdvid1DuHRBWk0Htik3J/w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170111174334.24343-4-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On 01/11/2017 06:43 PM, Martin Blumenstingl wrote:
> + indio_dev->name = dev_name(&pdev->dev);
The name is supposed to be the type of the device, e.g. part name, not the
name of parent device instance. E.g. meson-gxbb-saradc or meson-gxl-saradc
in this case.
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger
From: Lars-Peter Clausen @ 2017-01-15 12:24 UTC (permalink / raw)
To: Andreas Klinger, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
mranostay-Re5JQEeQqe8AvxtiuMwx3w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170110184815.GA15532@andreas>
On 01/10/2017 07:48 PM, Andreas Klinger wrote:
[...]
> + indio_dev->name = dev_name(&client->dev);
The name is supposed to be the type of the device, e.g. part name, not the
name of parent device instance. E.g. "srf08" in this case.
^ permalink raw reply
* Re: [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver
From: Lars-Peter Clausen @ 2017-01-15 12:18 UTC (permalink / raw)
To: Jacopo Mondi, wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w, geert-Td1EMuHUCqxL1ZNQvxDV9g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484301038-16386-2-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 01/13/2017 10:50 AM, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
>
> Add iio driver for Maxim MAX11100 single-channel ADC.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> Tested-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> ---
> drivers/iio/adc/Kconfig | 9 +++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 197 insertions(+)
> create mode 100644 drivers/iio/adc/max11100.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 38bc319..c32bc7a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -307,6 +307,15 @@ config MAX1027
> To compile this driver as a module, choose M here: the module will be
> called max1027.
>
> +config MAX11100
> + tristate "Maxim max11100 ADC driver"
> + depends on SPI_MASTER
> + help
> + Say yes here to build support for Maxim max11100 SPI ADC
> +
> + To compile this driver as a module, choose M here: the module will be
> + called max11100.
> +
> config MAX1363
> tristate "Maxim max1363 ADC driver"
> depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d36c4be..5684369 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
> obj-$(CONFIG_LTC2485) += ltc2485.o
> obj-$(CONFIG_MAX1027) += max1027.o
> +obj-$(CONFIG_MAX11100) += max11100.o
> obj-$(CONFIG_MAX1363) += max1363.o
> obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> new file mode 100644
> index 0000000..78e2a45
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,187 @@
> +/*
> + * iio/adc/max11100.c
> + * Maxim max11100 ADC Driver with IIO interface
> + *
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + * Copyright (C) 2016 Jacopo Mondi
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/driver.h>
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref_mv / 2 ^ 16)
> + *
> + * LSB is used to calculate analog voltage value
> + * from the number of ADC steps count
> + *
> + * Ain = (count * LSB)
> + */
> +#define MAX11100_LSB_DIV (1 << 16)
> +
> +struct max11100_state {
> + const struct max11100_chip_desc *desc;
> + struct regulator *vref_reg;
> + struct spi_device *spi;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> + { /* [0] */
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + },
> +};
> +
> +static struct max11100_chip_desc {
> + unsigned int num_chan;
> + const struct iio_chan_spec *channels;
> +} max11100_desc = {
> + .num_chan = ARRAY_SIZE(max11100_channels),
> + .channels = max11100_channels,
> +};
> +
> +static int max11100_read_single(struct iio_dev *indio_dev, int *val)
> +{
> + int ret;
> + struct max11100_state *state = iio_priv(indio_dev);
> + uint8_t buffer[3];
> +
> + ret = spi_read(state->spi, buffer, sizeof(buffer));
> + if (ret) {
> + dev_err(&indio_dev->dev, "SPI transfer failed\n");
> + return ret;
> + }
> +
> + /* the first 8 bits sent out from ADC must be 0s */
> + if (buffer[0]) {
> + dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
> + return -EINVAL;
> + }
> +
> + *val = (buffer[1] << 8) | buffer[2];
> +
> + return 0;
> +}
> +
> +static int max11100_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long info)
> +{
> + int ret, vref_uv;
> + struct max11100_state *state = iio_priv(indio_dev);
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + ret = max11100_read_single(indio_dev, val);
> + if (ret)
> + return ret;
> +
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + vref_uv = regulator_get_voltage(state->vref_reg);
> + if (vref_uv < 0)
> + /* dummy regulator "get_voltage" returns -EINVAL */
> + return -EINVAL;
> +
> + *val = vref_uv / 1000;
> + *val2 = MAX11100_LSB_DIV;
> + return IIO_VAL_FRACTIONAL;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_info max11100_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = max11100_read_raw,
> +};
> +
> +static int max11100_probe(struct spi_device *spi)
> +{
> + int ret;
> + struct iio_dev *indio_dev;
> + struct max11100_state *state;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + spi_set_drvdata(spi, indio_dev);
> +
> + state = iio_priv(indio_dev);
> + state->spi = spi;
> + state->desc = &max11100_desc;
> +
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->dev.of_node = spi->dev.of_node;
> + indio_dev->info = &max11100_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = state->desc->channels;
> + indio_dev->num_channels = state->desc->num_chan;
Consider setting indio_dev->name= "max11100"; so userspace can identify the
type of the device.
^ permalink raw reply
* [PATCH v2] Documentation: usb: fix wrong documentation paths
From: Yegor Yefremov @ 2017-01-15 12:14 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Yegor Yefremov
Fixes wrong spelled "pinctrl-bindings.txt" and "qcom-dwc3-usb-phy.txt"
file names as also wrong specified "mt8173-mtu3.txt" file name.
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Changes:
v1 -> v2: add changelog text (Greg Kroah-Hartman)
Documentation/devicetree/bindings/usb/dwc3-st.txt | 4 ++--
Documentation/devicetree/bindings/usb/ehci-st.txt | 2 +-
Documentation/devicetree/bindings/usb/mt8173-mtu3.txt | 2 +-
Documentation/devicetree/bindings/usb/mt8173-xhci.txt | 4 ++--
Documentation/devicetree/bindings/usb/qcom,dwc3.txt | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
index 01c71b1..50dee3b 100644
--- a/Documentation/devicetree/bindings/usb/dwc3-st.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
@@ -20,10 +20,10 @@ See: Documentation/devicetree/bindings/reset/reset.txt
with 'reg' property
- pinctl-names : A pinctrl state named "default" must be defined
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- pinctrl-0 : Pin control group
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- ranges : allows valid 1:1 translation between child's address space and
parent's address space
diff --git a/Documentation/devicetree/bindings/usb/ehci-st.txt b/Documentation/devicetree/bindings/usb/ehci-st.txt
index fb45fa5..410d922 100644
--- a/Documentation/devicetree/bindings/usb/ehci-st.txt
+++ b/Documentation/devicetree/bindings/usb/ehci-st.txt
@@ -7,7 +7,7 @@ Required properties:
- interrupts : one EHCI interrupt should be described here
- pinctrl-names : a pinctrl state named "default" must be defined
- pinctrl-0 : phandle referencing pin configuration of the USB controller
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- clocks : phandle list of usb clocks
- clock-names : should be "ic" for interconnect clock and "clk48"
See: Documentation/devicetree/bindings/clock/clock-bindings.txt
diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
index e049d19..718386a 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
@@ -30,7 +30,7 @@ Optional properties:
"id_float" and "id_ground" are optinal which depends on
"mediatek,enable-manual-drd"
- pinctrl-0 : pin control group
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- maximum-speed : valid arguments are "super-speed", "high-speed" and
"full-speed"; refer to usb/generic.txt
diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
index 2a930bd..7ea3971 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
@@ -37,7 +37,7 @@ Optional properties:
- usb3-lpm-capable : supports USB3.0 LPM
- pinctrl-names : a pinctrl state named "default" must be defined
- pinctrl-0 : pin control group
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
Example:
usb30: usb@11270000 {
@@ -67,7 +67,7 @@ usb30: usb@11270000 {
In the case, xhci is added as subnode to mtu3. An example and the DT binding
details of mtu3 can be found in:
-Documentation/devicetree/bindings/usb/mtu3.txt
+Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
Required properties:
- compatible : should contain "mediatek,mt8173-xhci"
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
index 39acb08..73cc096 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -18,7 +18,7 @@ A child node must exist to represent the core DWC3 IP block. The name of
the node is not important. The content of the node is defined in dwc3.txt.
Phy documentation is provided in the following places:
-Documentation/devicetree/bindings/phy/qcom,dwc3-usb-phy.txt
+Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
Example device nodes:
--
2.9.3
--
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] Documentation: usb: fix wrong documentation paths
From: Greg KH @ 2017-01-15 12:07 UTC (permalink / raw)
To: Yegor Yefremov
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170115120456.2942-1-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Sun, Jan 15, 2017 at 01:04:56PM +0100, Yegor Yefremov wrote:
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
I never take patches without any changelog text, and I doubt that other
maintainers do either :(
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] Documentation: usb: fix wrong documentation paths
From: Yegor Yefremov @ 2017-01-15 12:04 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Yegor Yefremov
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/usb/dwc3-st.txt | 4 ++--
Documentation/devicetree/bindings/usb/ehci-st.txt | 2 +-
Documentation/devicetree/bindings/usb/mt8173-mtu3.txt | 2 +-
Documentation/devicetree/bindings/usb/mt8173-xhci.txt | 4 ++--
Documentation/devicetree/bindings/usb/qcom,dwc3.txt | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3-st.txt b/Documentation/devicetree/bindings/usb/dwc3-st.txt
index 01c71b1..50dee3b 100644
--- a/Documentation/devicetree/bindings/usb/dwc3-st.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3-st.txt
@@ -20,10 +20,10 @@ See: Documentation/devicetree/bindings/reset/reset.txt
with 'reg' property
- pinctl-names : A pinctrl state named "default" must be defined
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- pinctrl-0 : Pin control group
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- ranges : allows valid 1:1 translation between child's address space and
parent's address space
diff --git a/Documentation/devicetree/bindings/usb/ehci-st.txt b/Documentation/devicetree/bindings/usb/ehci-st.txt
index fb45fa5..410d922 100644
--- a/Documentation/devicetree/bindings/usb/ehci-st.txt
+++ b/Documentation/devicetree/bindings/usb/ehci-st.txt
@@ -7,7 +7,7 @@ Required properties:
- interrupts : one EHCI interrupt should be described here
- pinctrl-names : a pinctrl state named "default" must be defined
- pinctrl-0 : phandle referencing pin configuration of the USB controller
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- clocks : phandle list of usb clocks
- clock-names : should be "ic" for interconnect clock and "clk48"
See: Documentation/devicetree/bindings/clock/clock-bindings.txt
diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
index e049d19..718386a 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
@@ -30,7 +30,7 @@ Optional properties:
"id_float" and "id_ground" are optinal which depends on
"mediatek,enable-manual-drd"
- pinctrl-0 : pin control group
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- maximum-speed : valid arguments are "super-speed", "high-speed" and
"full-speed"; refer to usb/generic.txt
diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
index 2a930bd..7ea3971 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
@@ -37,7 +37,7 @@ Optional properties:
- usb3-lpm-capable : supports USB3.0 LPM
- pinctrl-names : a pinctrl state named "default" must be defined
- pinctrl-0 : pin control group
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
Example:
usb30: usb@11270000 {
@@ -67,7 +67,7 @@ usb30: usb@11270000 {
In the case, xhci is added as subnode to mtu3. An example and the DT binding
details of mtu3 can be found in:
-Documentation/devicetree/bindings/usb/mtu3.txt
+Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
Required properties:
- compatible : should contain "mediatek,mt8173-xhci"
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
index 39acb08..73cc096 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -18,7 +18,7 @@ A child node must exist to represent the core DWC3 IP block. The name of
the node is not important. The content of the node is defined in dwc3.txt.
Phy documentation is provided in the following places:
-Documentation/devicetree/bindings/phy/qcom,dwc3-usb-phy.txt
+Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
Example device nodes:
--
2.9.3
--
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
* [PATCH] fbdev: ssd1307fb: allow reset-gpios is missing
From: Icenowy Zheng @ 2017-01-15 11:21 UTC (permalink / raw)
To: Rob Herring, David Airlie, Maxime Ripard
Cc: devicetree, linux-kernel, linux-fbdev, Icenowy Zheng
Currently some SSD1306 OLED modules are sold without a reset pin (only
VCC, GND, SCK, SDA four pins).
Add support for missing reset-gpios property.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
.../devicetree/bindings/display/ssd1307fb.txt | 2 +-
drivers/video/fbdev/ssd1307fb.c | 34 ++++++++++++----------
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index eb31ed47a283..a4541b13b706 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -8,13 +8,13 @@ Required properties:
0x3c or 0x3d
- pwm: Should contain the pwm to use according to the OF device tree PWM
specification [0]. Only required for the ssd1307.
- - reset-gpios: Should contain the GPIO used to reset the OLED display
- solomon,height: Height in pixel of the screen driven by the controller
- solomon,width: Width in pixel of the screen driven by the controller
- solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
mapped to.
Optional properties:
+ - reset-gpios: Should contain the GPIO used to reset the OLED display
- reset-active-low: Is the reset gpio is active on physical low?
- solomon,segment-no-remap: Display needs normal (non-inverted) data column
to segment mapping
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 2925d5ce8d3e..e2b51a091ed4 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -563,8 +563,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
par->reset = of_get_named_gpio(client->dev.of_node,
"reset-gpios", 0);
- if (!gpio_is_valid(par->reset)) {
- ret = -EINVAL;
+ if (!gpio_is_valid(par->reset) && par->reset != -ENOENT) {
+ ret = par->reset;
goto fb_alloc_error;
}
@@ -642,23 +642,27 @@ static int ssd1307fb_probe(struct i2c_client *client,
fb_deferred_io_init(info);
- ret = devm_gpio_request_one(&client->dev, par->reset,
- GPIOF_OUT_INIT_HIGH,
- "oled-reset");
- if (ret) {
- dev_err(&client->dev,
- "failed to request gpio %d: %d\n",
- par->reset, ret);
- goto reset_oled_error;
+ if (par->reset != -ENOENT) {
+ ret = devm_gpio_request_one(&client->dev, par->reset,
+ GPIOF_OUT_INIT_HIGH,
+ "oled-reset");
+ if (ret) {
+ dev_err(&client->dev,
+ "failed to request gpio %d: %d\n",
+ par->reset, ret);
+ goto reset_oled_error;
+ }
}
i2c_set_clientdata(client, info);
- /* Reset the screen */
- gpio_set_value(par->reset, 0);
- udelay(4);
- gpio_set_value(par->reset, 1);
- udelay(4);
+ if (par->reset != -ENOENT) {
+ /* Reset the screen */
+ gpio_set_value(par->reset, 0);
+ udelay(4);
+ gpio_set_value(par->reset, 1);
+ udelay(4);
+ }
ret = ssd1307fb_init(par);
if (ret)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v6 2/3] input: tm2-touchkey: Add touchkey driver support for TM2
From: Dmitry Torokhov @ 2017-01-15 7:11 UTC (permalink / raw)
To: Jaechul Lee
Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
Kukjin Kim, Krzysztof Kozlowski, Javier Martinez Canillas,
Andi Shyti, Chanwoo Choi, beomho.seo, galaxyra, linux-arm-kernel,
linux-input, devicetree, linux-kernel, linux-samsung-soc
In-Reply-To: <1483946535-4703-3-git-send-email-jcsing.lee@samsung.com>
Hi Jaechul,
On Mon, Jan 09, 2017 at 04:22:14PM +0900, Jaechul Lee wrote:
> +static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid)
> +{
> + struct tm2_touchkey_data *touchkey = devid;
> + u32 data;
> +
> + data = i2c_smbus_read_byte_data(touchkey->client,
> + TM2_TOUCHKEY_KEYCODE_REG);
> +
> + if (data < 0) {
You declared data as u32 so it will never be negative.
> + dev_err(&touchkey->client->dev, "Failed to read i2c data\n");
> + return IRQ_HANDLED;
> + }
> +
> + touchkey->keycode_type = data & TM2_TOUCHKEY_BIT_KEYCODE;
> + touchkey->pressed = !(data & TM2_TOUCHKEY_BIT_PRESS_EV);
There is no need to store this in touchkey structure as you are not
going to use it past this function.
Does the version of the patch below work for you?
Thanks.
--
Dmitry
Input: tm2-touchkey - add touchkey driver support for TM2
From: Jaechul Lee <jcsing.lee@samsung.com>
This patch adds support for the TM2 touch key and led functionality.
The driver interfaces with userspace through an input device and
reports KEY_PHONE and KEY_BACK event types. LED brightness can be
controlled by "/sys/class/leds/tm2-touchkey/brightness".
Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Patchwork-Id: 9504149
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/Kconfig | 11 +
drivers/input/keyboard/Makefile | 1
drivers/input/keyboard/tm2-touchkey.c | 286 +++++++++++++++++++++++++++++++++
3 files changed, 298 insertions(+)
create mode 100644 drivers/input/keyboard/tm2-touchkey.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index cbd75cf44739..97acd6524ad7 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -666,6 +666,17 @@ config KEYBOARD_TC3589X
To compile this driver as a module, choose M here: the
module will be called tc3589x-keypad.
+config KEYBOARD_TM2_TOUCHKEY
+ tristate "TM2 touchkey support"
+ depends on I2C
+ depends on LEDS_CLASS
+ help
+ Say Y here to enable device driver for tm2-touchkey with
+ LED control for the Exynos5433 TM2 board.
+
+ To compile this driver as a module, choose M here.
+ module will be called tm2-touchkey.
+
config KEYBOARD_TWL4030
tristate "TI TWL4030/TWL5030/TPS659x0 keypad support"
depends on TWL4030_CORE
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index d9f4cfcf3410..7d9acff819a7 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_KEYBOARD_SUN4I_LRADC) += sun4i-lradc-keys.o
obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
+obj-$(CONFIG_KEYBOARD_TM2_TOUCHKEY) += tm2-touchkey.o
obj-$(CONFIG_KEYBOARD_TWL4030) += twl4030_keypad.o
obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o
obj-$(CONFIG_KEYBOARD_W90P910) += w90p910_keypad.o
diff --git a/drivers/input/keyboard/tm2-touchkey.c b/drivers/input/keyboard/tm2-touchkey.c
new file mode 100644
index 000000000000..79bc2d2bd4b9
--- /dev/null
+++ b/drivers/input/keyboard/tm2-touchkey.c
@@ -0,0 +1,286 @@
+/*
+ * TM2 touchkey device driver
+ *
+ * Copyright 2005 Phil Blundell
+ * Copyright 2016 Samsung Electronics Co., Ltd.
+ *
+ * Author: Beomho Seo <beomho.seo@samsung.com>
+ * Author: Jaechul Lee <jcsing.lee@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+
+#define TM2_TOUCHKEY_DEV_NAME "tm2-touchkey"
+#define TM2_TOUCHKEY_KEYCODE_REG 0x03
+#define TM2_TOUCHKEY_BASE_REG 0x00
+#define TM2_TOUCHKEY_CMD_LED_ON 0x10
+#define TM2_TOUCHKEY_CMD_LED_OFF 0x20
+#define TM2_TOUCHKEY_BIT_PRESS_EV BIT(3)
+#define TM2_TOUCHKEY_BIT_KEYCODE GENMASK(2, 0)
+#define TM2_TOUCHKEY_LED_VOLTAGE_MIN 2500000
+#define TM2_TOUCHKEY_LED_VOLTAGE_MAX 3300000
+
+enum {
+ TM2_TOUCHKEY_KEY_MENU = 0x1,
+ TM2_TOUCHKEY_KEY_BACK,
+};
+
+struct tm2_touchkey_data {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ struct led_classdev led_dev;
+ struct regulator *vdd;
+ struct regulator_bulk_data regulators[2];
+};
+
+static void tm2_touchkey_led_brightness_set(struct led_classdev *led_dev,
+ enum led_brightness brightness)
+{
+ struct tm2_touchkey_data *touchkey =
+ container_of(led_dev, struct tm2_touchkey_data, led_dev);
+ u32 volt;
+ u8 data;
+
+ if (brightness == LED_OFF) {
+ volt = TM2_TOUCHKEY_LED_VOLTAGE_MIN;
+ data = TM2_TOUCHKEY_CMD_LED_OFF;
+ } else {
+ volt = TM2_TOUCHKEY_LED_VOLTAGE_MAX;
+ data = TM2_TOUCHKEY_CMD_LED_ON;
+ }
+
+ regulator_set_voltage(touchkey->vdd, volt, volt);
+ i2c_smbus_write_byte_data(touchkey->client,
+ TM2_TOUCHKEY_BASE_REG, data);
+}
+
+static int tm2_touchkey_power_enable(struct tm2_touchkey_data *touchkey)
+{
+ int error;
+
+ error = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
+ touchkey->regulators);
+ if (error)
+ return error;
+
+ /* waiting for device initialization, at least 150ms */
+ msleep(150);
+
+ return 0;
+}
+
+static void tm2_touchkey_power_disable(void *data)
+{
+ struct tm2_touchkey_data *touchkey = data;
+
+ regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators),
+ touchkey->regulators);
+}
+
+static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid)
+{
+ struct tm2_touchkey_data *touchkey = devid;
+ int data;
+ int key;
+
+ data = i2c_smbus_read_byte_data(touchkey->client,
+ TM2_TOUCHKEY_KEYCODE_REG);
+ if (data < 0) {
+ dev_err(&touchkey->client->dev,
+ "failed to read i2c data: %d\n", data);
+ goto out;
+ }
+
+ switch (data & TM2_TOUCHKEY_BIT_KEYCODE) {
+ case TM2_TOUCHKEY_KEY_MENU:
+ key = KEY_PHONE;
+ break;
+
+ case TM2_TOUCHKEY_KEY_BACK:
+ key = KEY_BACK;
+ break;
+
+ default:
+ dev_warn(&touchkey->client->dev,
+ "unhandled keycode, data %#02x\n", data);
+ goto out;
+ }
+
+ if (data & TM2_TOUCHKEY_BIT_PRESS_EV) {
+ input_report_key(touchkey->input_dev, key, 1);
+ } else {
+ input_report_key(touchkey->input_dev, KEY_PHONE, 0);
+ input_report_key(touchkey->input_dev, KEY_BACK, 0);
+ }
+
+ input_sync(touchkey->input_dev);
+
+out:
+ return IRQ_HANDLED;
+}
+
+static int tm2_touchkey_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct tm2_touchkey_data *touchkey;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA)) {
+ dev_err(&client->dev, "incompatible I2C adapter\n");
+ return -EIO;
+ }
+
+ touchkey = devm_kzalloc(&client->dev, sizeof(*touchkey), GFP_KERNEL);
+ if (!touchkey)
+ return -ENOMEM;
+
+ touchkey->client = client;
+ i2c_set_clientdata(client, touchkey);
+
+ touchkey->regulators[0].supply = "vcc";
+ touchkey->regulators[1].supply = "vdd";
+ error = devm_regulator_bulk_get(&client->dev,
+ ARRAY_SIZE(touchkey->regulators),
+ touchkey->regulators);
+ if (error) {
+ dev_err(&client->dev, "failed to get regulators: %d\n", error);
+ return error;
+ }
+
+ /* Save VDD for easy access */
+ touchkey->vdd = touchkey->regulators[1].consumer;
+
+ error = tm2_touchkey_power_enable(touchkey);
+ if (error) {
+ dev_err(&client->dev, "failed to power up device: %d\n", error);
+ return error;
+ }
+
+ error = devm_add_action_or_reset(&client->dev,
+ tm2_touchkey_power_disable, touchkey);
+ if (error) {
+ dev_err(&client->dev,
+ "failed to install poweroff handler: %d\n", error);
+ return error;
+ }
+
+ /* input device */
+ touchkey->input_dev = devm_input_allocate_device(&client->dev);
+ if (!touchkey->input_dev) {
+ dev_err(&client->dev, "failed to allocate input device\n");
+ return -ENOMEM;
+ }
+
+ touchkey->input_dev->name = TM2_TOUCHKEY_DEV_NAME;
+ touchkey->input_dev->id.bustype = BUS_I2C;
+
+ input_set_capability(touchkey->input_dev, EV_KEY, KEY_PHONE);
+ input_set_capability(touchkey->input_dev, EV_KEY, KEY_BACK);
+
+ input_set_drvdata(touchkey->input_dev, touchkey);
+
+ error = input_register_device(touchkey->input_dev);
+ if (error) {
+ dev_err(&client->dev,
+ "failed to register input device: %d\n", error);
+ return error;
+ }
+
+ error = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, tm2_touchkey_irq_handler,
+ IRQF_ONESHOT,
+ TM2_TOUCHKEY_DEV_NAME, touchkey);
+ if (error) {
+ dev_err(&client->dev,
+ "failed to request threaded irq: %d\n", error);
+ return error;
+ }
+
+ /* led device */
+ touchkey->led_dev.name = TM2_TOUCHKEY_DEV_NAME;
+ touchkey->led_dev.brightness = LED_FULL;
+ touchkey->led_dev.max_brightness = LED_FULL;
+ touchkey->led_dev.brightness_set = tm2_touchkey_led_brightness_set;
+
+ error = devm_led_classdev_register(&client->dev, &touchkey->led_dev);
+ if (error) {
+ dev_err(&client->dev,
+ "failed to register touchkey led: %d\n", error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused tm2_touchkey_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct tm2_touchkey_data *touchkey = i2c_get_clientdata(client);
+
+ disable_irq(client->irq);
+ tm2_touchkey_power_disable(touchkey);
+
+ return 0;
+}
+
+static int __maybe_unused tm2_touchkey_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct tm2_touchkey_data *touchkey = i2c_get_clientdata(client);
+ int ret;
+
+ enable_irq(client->irq);
+
+ ret = tm2_touchkey_power_enable(touchkey);
+ if (ret)
+ dev_err(dev, "failed to enable power: %d\n", ret);
+
+ return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(tm2_touchkey_pm_ops,
+ tm2_touchkey_suspend, tm2_touchkey_resume);
+
+static const struct i2c_device_id tm2_touchkey_id_table[] = {
+ { TM2_TOUCHKEY_DEV_NAME, 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, tm2_touchkey_id_table);
+
+static const struct of_device_id tm2_touchkey_of_match[] = {
+ { .compatible = "cypress,tm2-touchkey", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tm2_touchkey_of_match);
+
+static struct i2c_driver tm2_touchkey_driver = {
+ .driver = {
+ .name = TM2_TOUCHKEY_DEV_NAME,
+ .pm = &tm2_touchkey_pm_ops,
+ .of_match_table = of_match_ptr(tm2_touchkey_of_match),
+ },
+ .probe = tm2_touchkey_probe,
+ .id_table = tm2_touchkey_id_table,
+};
+module_i2c_driver(tm2_touchkey_driver);
+
+MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
+MODULE_AUTHOR("Jaechul Lee <jcsing.lee@samsung.com>");
+MODULE_DESCRIPTION("Samsung touchkey driver");
+MODULE_LICENSE("GPL v2");
^ permalink raw reply related
* Re: [alsa-devel] [PATCH 3/4] ASoC: rockchip: Add machine driver for ES8388 codecs
From: kbuild test robot @ 2017-01-15 5:21 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, Jaroslav Kysela, Takashi Iwai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Pawel Moll, Ian Campbell,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons,
Rob Herring, Kumar Gala, Romain Perier
In-Reply-To: <20170113161452.14313-4-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
Hi Romain,
[auto build test WARNING on rockchip/for-next]
[also build test WARNING on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Romain-Perier/Add-support-for-es8388-on-the-rock2/20170115-095611
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All warnings (new ones prefixed by >>):
warning: (SND_SOC_ROCKCHIP_ES8388) selects SND_SOC_ROCKCHIP_I2S which has unmet direct dependencies (SOUND && !M68K && !UML && SND && SND_SOC && CLKDEV_LOOKUP && SND_SOC_ROCKCHIP)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48885 bytes --]
^ permalink raw reply
* Re: [alsa-devel] [PATCH 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: kbuild test robot @ 2017-01-15 2:56 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, Jaroslav Kysela, Takashi Iwai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Pawel Moll, Ian Campbell,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons,
Rob Herring, Kumar Gala, Romain Perier
In-Reply-To: <20170113161452.14313-2-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]
Hi Romain,
[auto build test WARNING on rockchip/for-next]
[also build test WARNING on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Romain-Perier/Add-support-for-es8388-on-the-rock2/20170115-095611
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha
All warnings (new ones prefixed by >>):
>> sound/soc/codecs/es8328-i2c.c:22:14: warning: initialization makes integer from pointer without a cast [-Wint-conversion]
{ "es8328", "es8388", 0 },
^~~~~~~~
sound/soc/codecs/es8328-i2c.c:22:14: note: (near initialization for 'es8328_id[0].driver_data')
>> sound/soc/codecs/es8328-i2c.c:22:24: warning: excess elements in struct initializer
{ "es8328", "es8388", 0 },
^
sound/soc/codecs/es8328-i2c.c:22:24: note: (near initialization for 'es8328_id[0]')
vim +22 sound/soc/codecs/es8328-i2c.c
6 * Author: Sean Cross <xobs-nXMMniAx+RbQT0dZR+AlfA@public.gmane.org>
7 *
8 * This program is free software; you can redistribute it and/or modify
9 * it under the terms of the GNU General Public License version 2 as
10 * published by the Free Software Foundation.
11 */
12
13 #include <linux/module.h>
14 #include <linux/i2c.h>
15 #include <linux/regmap.h>
16
17 #include <sound/soc.h>
18
19 #include "es8328.h"
20
21 static const struct i2c_device_id es8328_id[] = {
> 22 { "es8328", "es8388", 0 },
23 { }
24 };
25 MODULE_DEVICE_TABLE(i2c, es8328_id);
26
27 static const struct of_device_id es8328_of_match[] = {
28 { .compatible = "everest,es8328", },
29 { .compatible = "everest,es8388", },
30 { }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48654 bytes --]
^ permalink raw reply
* [PATCH 2/2] ARM: BCM5301X: Add DT for Luxul XWR-1200
From: Dan Haab @ 2017-01-15 2:29 UTC (permalink / raw)
To: Florian Fainelli
Cc: Mark Rutland, devicetree, Hauke Mehrtens, Rafał Miłecki,
Russell King, Rob Herring, bcm-kernel-feedback-list, Dan Haab,
linux-arm-kernel
In-Reply-To: <1484447368-7622-1-git-send-email-dhaab@luxul.com>
Luxul XWR-1200 in a dual-band router based on BCM47081. It uses serial
flash (for bootloader and NVRAM) and NAND flash (for firmware).
Signed-off-by: Dan Haab <dhaab@luxul.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts | 107 +++++++++++++++++++++++++
2 files changed, 108 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 04fd8f0..8a403a2 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -84,6 +84,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
bcm47081-buffalo-wzr-600dhp2.dtb \
bcm47081-buffalo-wzr-900dhp.dtb \
bcm47081-luxul-xap-1410.dtb \
+ bcm47081-luxul-xwr-1200.dtb \
bcm4709-asus-rt-ac87u.dtb \
bcm4709-buffalo-wxr-1900dhp.dtb \
bcm4709-netgear-r7000.dtb \
diff --git a/arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts b/arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts
new file mode 100644
index 0000000..c544ab3
--- /dev/null
+++ b/arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dts
@@ -0,0 +1,107 @@
+/*
+ * Copyright 2017 Luxul Inc.
+ *
+ * Licensed under the ISC license.
+ */
+
+/dts-v1/;
+
+#include "bcm47081.dtsi"
+#include "bcm5301x-nand-cs0-bch4.dtsi"
+
+/ {
+ compatible = "luxul,xwr-1200v1", "brcm,bcm47081", "brcm,bcm4708";
+ model = "Luxul XWR-1200 V1";
+
+ chosen {
+ bootargs = "console=ttyS0,115200";
+ };
+
+ memory {
+ reg = <0x00000000 0x08000000>;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ power {
+ label = "bcm53xx:green:power";
+ gpios = <&chipcommon 0 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "default-on";
+ };
+
+ lan3 {
+ label = "bcm53xx:green:lan3";
+ gpios = <&chipcommon 1 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ lan4 {
+ label = "bcm53xx:green:lan4";
+ gpios = <&chipcommon 2 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ wan {
+ label = "bcm53xx:green:wan";
+ gpios = <&chipcommon 3 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ lan2 {
+ label = "bcm53xx:green:lan2";
+ gpios = <&chipcommon 6 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ usb {
+ label = "bcm53xx:green:usb";
+ gpios = <&chipcommon 8 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ status {
+ label = "bcm53xx:green:status";
+ gpios = <&chipcommon 10 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "timer";
+ };
+
+ 2ghz {
+ label = "bcm53xx:green:2ghz";
+ gpios = <&chipcommon 13 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ 5ghz {
+ label = "bcm53xx:green:5ghz";
+ gpios = <&chipcommon 14 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ lan1 {
+ label = "bcm53xx:green:lan1";
+ gpios = <&chipcommon 15 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ restart {
+ label = "Reset";
+ linux,code = <KEY_RESTART>;
+ gpios = <&chipcommon 11 GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&usb2 {
+ vcc-gpio = <&chipcommon 9 GPIO_ACTIVE_HIGH>;
+};
+
+&spi_nor {
+ status = "okay";
+};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] ARM: BCM5301X: Add DT for Luxul XAP-1410
From: Dan Haab @ 2017-01-15 2:29 UTC (permalink / raw)
To: Florian Fainelli
Cc: Mark Rutland, devicetree, Hauke Mehrtens, Rafał Miłecki,
Russell King, Rob Herring, bcm-kernel-feedback-list, Dan Haab,
linux-arm-kernel
Luxul XAP-1410 in a dual-band access point device based on BCM47081 with
serial flash. It has 3 LEDs and just one (reset) button.
Signed-off-by: Dan Haab <dhaab@luxul.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/bcm47081-luxul-xap-1410.dts | 60 +++++++++++++++++++++++++
2 files changed, 61 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm47081-luxul-xap-1410.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index cccdbcb..04fd8f0 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -83,6 +83,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \
bcm47081-asus-rt-n18u.dtb \
bcm47081-buffalo-wzr-600dhp2.dtb \
bcm47081-buffalo-wzr-900dhp.dtb \
+ bcm47081-luxul-xap-1410.dtb \
bcm4709-asus-rt-ac87u.dtb \
bcm4709-buffalo-wxr-1900dhp.dtb \
bcm4709-netgear-r7000.dtb \
diff --git a/arch/arm/boot/dts/bcm47081-luxul-xap-1410.dts b/arch/arm/boot/dts/bcm47081-luxul-xap-1410.dts
new file mode 100644
index 0000000..9b57598
--- /dev/null
+++ b/arch/arm/boot/dts/bcm47081-luxul-xap-1410.dts
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2017 Luxul Inc.
+ *
+ * Licensed under the ISC license.
+ */
+
+/dts-v1/;
+
+#include "bcm47081.dtsi"
+
+/ {
+ compatible = "luxul,xap-1410v1", "brcm,bcm47081", "brcm,bcm4708";
+ model = "Luxul XAP-1410 V1";
+
+ chosen {
+ bootargs = "console=ttyS0,115200";
+ };
+
+ memory {
+ reg = <0x00000000 0x08000000>;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ 5ghz {
+ label = "bcm53xx:blue:5ghz";
+ gpios = <&chipcommon 13 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ 2ghz {
+ label = "bcm53xx:blue:2ghz";
+ gpios = <&chipcommon 14 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "none";
+ };
+
+ status {
+ label = "bcm53xx:green:status";
+ gpios = <&chipcommon 15 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "timer";
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ restart {
+ label = "Reset";
+ linux,code = <KEY_RESTART>;
+ gpios = <&chipcommon 11 GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&spi_nor {
+ status = "okay";
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH RFC v2 4/4] ARM: dts: imx6*-hummingboard2: convert to more conventional vmmc-supply
From: Fabio Estevam @ 2017-01-15 2:11 UTC (permalink / raw)
To: Russell King
Cc: Fabio Estevam, Sascha Hauer, Shawn Guo, Jon Nettleton,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <E1cS36q-0002wW-C3-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
On Fri, Jan 13, 2017 at 12:45 PM, Russell King
<rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> + reg_usdhc2_vmmc: reg-usdhc2-vmmc {
> + compatible = "regulator-fixed";
> + gpio = <&gpio4 30 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_hummingboard2_vmmc>;
> + regulator-boot-on;
> + regulator-max-microvolt = <3300000>;
> + regulator-min-microvolt = <3300000>;
> + regulator-name = "usdhc2_vmmc";
> + startup-delay-us = <1000>;
> + };
> +
> sound-sgtl5000 {
> audio-codec = <&sgtl5000>;
> audio-routing =
> @@ -393,7 +400,6 @@
>
> pinctrl_hummingboard2_usdhc2_aux: hummingboard2-usdhc2-aux {
> fsl,pins = <
> - MX6QDL_PAD_GPIO_4__GPIO1_IO04 0x1f071
> MX6QDL_PAD_KEY_ROW1__SD2_VSELECT 0x1b071
> MX6QDL_PAD_DISP0_DAT9__GPIO4_IO30 0x1b0b0
> >;
> @@ -432,6 +438,12 @@
> >;
> };
>
> + pinctrl_hummingboard2_vmmc: hummingboard2-vmmc {
> + fsl,pins = <
> + MX6QDL_PAD_GPIO_4__GPIO1_IO04 0x1f071
Shouldn't this be gpio4 30 instead?
> + >;
> + };
> +
> pinctrl_hummingboard2_usdhc3: hummingboard2-usdhc3 {
> fsl,pins = <
> MX6QDL_PAD_SD3_CMD__SD3_CMD 0x17059
> @@ -519,7 +531,7 @@
> &pinctrl_hummingboard2_usdhc2_aux
> &pinctrl_hummingboard2_usdhc2_200mhz
> >;
> - mmc-pwrseq = <&usdhc2_pwrseq>;
> + vmmc-supply = <®_usdhc2_vmmc>;
> cd-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
as gpio1 4 seems to be used as card detect.
--
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 0/4] Drop drivers for Exynos4415
From: Kukjin Kim @ 2017-01-15 2:00 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sylwester Nawrocki, Tomasz Figa, Chanwoo Choi, Michael Turquette,
Stephen Boyd, Rob Herring, Mark Rutland, Kukjin Kim,
Javier Martinez Canillas, Inki Dae, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, Linus Walleij, linux-samsung-soc, linux-clk,
devicetree, linux-arm-kernel, linux-kernel, dri-devel, linux-gpio
In-Reply-To: <20170114123642.15581-1-krzk@kernel.org>
Hi Krzk,
2017. 1. 14. PM 8:36 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> Hi,
>
> Support for Exynos4415 is being removed because:
> 1. There are no upstream users,
> 2. There are no known downstream users,
> 3. Except compile testing, you cannot build working kernel
> for Exynos4415 anymore.
>
> Patches are rebased on current next and are independent.
> Please pick up as you wish.
>
> Best regards,
> Krzysztof
Agreed,
Acked-by: Kukjin Kim <kgene@kernel.org>
k-gene
>
> Krzysztof Kozlowski (4):
> ARM: EXYNOS: Remove Exynos4415 driver (SoC not supported anymore)
> clk: samsung: Remove Exynos4415 driver (SoC not supported anymore)
> pinctrl: samsung: Remove support for Exynos4415 (SoC not supported
> anymore)
> drm: exynos: Remove support for Exynos4415 (SoC not supported anymore)
>
> .../devicetree/bindings/clock/exynos4415-clock.txt | 38 -
> .../bindings/display/exynos/exynos_dsim.txt | 1 -
> .../bindings/display/exynos/samsung-fimd.txt | 1 -
> arch/arm/mach-exynos/Kconfig | 5 -
> arch/arm/mach-exynos/exynos.c | 1 -
> arch/arm/mach-exynos/suspend.c | 1 -
> drivers/clk/samsung/Makefile | 1 -
> drivers/clk/samsung/clk-exynos4415.c | 1022 --------------------
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 15 +-
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 18 +-
> drivers/pinctrl/samsung/pinctrl-exynos.c | 75 --
> drivers/pinctrl/samsung/pinctrl-samsung.c | 2 -
> drivers/pinctrl/samsung/pinctrl-samsung.h | 1 -
> include/dt-bindings/clock/exynos4415.h | 360 -------
> 14 files changed, 3 insertions(+), 1538 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/clock/exynos4415-clock.txt
> delete mode 100644 drivers/clk/samsung/clk-exynos4415.c
> delete mode 100644 include/dt-bindings/clock/exynos4415.h
>
> --
> 2.9.3
>
^ permalink raw reply
* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Marek Vasut @ 2017-01-15 0:23 UTC (permalink / raw)
To: Boris Brezillon
Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
Russell King, linux-mtd, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel
In-Reply-To: <20170114092958.022f2fc8@bbrezillon>
On 01/14/2017 09:29 AM, Boris Brezillon wrote:
> On Fri, 13 Jan 2017 18:33:40 +0100
> Marek Vasut <marek.vasut@gmail.com> wrote:
>
>> On 01/13/2017 05:56 PM, Boris Brezillon wrote:
>>> On Fri, 13 Jan 2017 17:44:12 +0100
>>> Marek Vasut <marek.vasut@gmail.com> wrote:
>>>
>>>> On 01/13/2017 05:28 PM, Boris Brezillon wrote:
>>>>> On Fri, 13 Jan 2017 17:13:55 +0100
>>>>> Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>
>>>>>> On 01/13/2017 04:12 PM, Matthias Brugger wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 13/01/17 15:17, Boris Brezillon wrote:
>>>>>>>> On Fri, 13 Jan 2017 15:13:29 +0800
>>>>>>>> Guochun Mao <guochun.mao@mediatek.com> wrote:
>>>>>>>>
>>>>>>>>> Add Mediatek nor flash node.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
>>>>>>>>> ---
>>>>>>>>> arch/arm/boot/dts/mt2701-evb.dts | 25 +++++++++++++++++++++++++
>>>>>>>>> arch/arm/boot/dts/mt2701.dtsi | 12 ++++++++++++
>>>>>>>>> 2 files changed, 37 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>> b/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>> index 082ca88..85e5ae8 100644
>>>>>>>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
>>>>>>>>> @@ -24,6 +24,31 @@
>>>>>>>>> };
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> +&nor_flash {
>>>>>>>>> + pinctrl-names = "default";
>>>>>>>>> + pinctrl-0 = <&nor_pins_default>;
>>>>>>>>> + status = "okay";
>>>>>>>>> + flash@0 {
>>>>>>>>> + compatible = "jedec,spi-nor";
>>>>>>>>> + reg = <0>;
>>>>>>>>> + };
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> +&pio {
>>>>>>>>> + nor_pins_default: nor {
>>>>>>>>> + pins1 {
>>>>>>>>> + pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
>>>>>>>>> + <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
>>>>>>>>> + <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
>>>>>>>>> + <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
>>>>>>>>> + <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
>>>>>>>>> + <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
>>>>>>>>> + drive-strength = <MTK_DRIVE_4mA>;
>>>>>>>>> + bias-pull-up;
>>>>>>>>> + };
>>>>>>>>> + };
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> &uart0 {
>>>>>>>>> status = "okay";
>>>>>>>>> };
>>>>>>>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>> b/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>> index bdf8954..1eefce4 100644
>>>>>>>>> --- a/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
>>>>>>>>> @@ -227,6 +227,18 @@
>>>>>>>>> status = "disabled";
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> + nor_flash: spi@11014000 {
>>>>>>>>> + compatible = "mediatek,mt2701-nor",
>>>>>>>>> + "mediatek,mt8173-nor";
>>>>>>>>
>>>>>>>> Why define both here? Is "mediatek,mt8173-nor" really providing a
>>>>>>>> subset of the features supported by "mediatek,mt2701-nor"?
>>>>>>>>
>>>>>>>
>>>>>>> I think even if the ip block is the same, we should provide both
>>>>>>> bindings, just in case in the future we find out that mt2701 has some
>>>>>>> hidden bug, feature or bug-feature. This way even if we update the
>>>>>>> driver, we stay compatible with older device tree blobs in the wild.
>>>>>>>
>>>>>>> We can drop the mt2701-nor in the bindings definition if you want.
>>>>>
>>>>> Oh, sorry, I misunderstood. What I meant is that if you want to
>>>>> list/support all possible compatibles, maybe you should just put one
>>>>> compatible in your DT and patch your driver (+ binding doc) to define
>>>>> all of them.
>>>>
>>>> Uh, what ? I lost you here :-)
>
> I mean adding a new entry in the mtk_nor_of_ids table (in
> mtk-quadspi.c) so that the mediatek,mt2701-nor compatible string can be
> matched directly, and you won't need to define 2 compatible strings in
> your device tree.
But then you grow the table in the driver, is that what we want if we
can avoid that ?
>>>>
>>>>>> This exactly. We should have a DT compat in the form:
>>>>>> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
>>>>>> Then if we find a problem in the future, we can match on the
>>>>>> "vendor,<soc>-block" and still support the old DTs.
>>>>>
>>>>> Not sure it's only in term of whose IP appeared first. My understanding
>>>>> is that it's a way to provide inheritance. For example:
>>>>>
>>>>> "<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
>>>>>
>>>>> or
>>>>>
>>>>> "<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
>>>>>
>>>>> BTW, which one is the oldest between mt8173 and mt2701? :-)
>>>>
>>>> And that's another thing and I agree with you, but I don't think that's
>>>> what we're discussing in this thread. But (!), OT, I think we should
>>>> codify the rules in Documentation/ . This discussion came up multiple
>>>> times recently.
>>>>
>>>> And my question still stands, what do we put into the DT here, IMO
>>>> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";
>>>
>>> I'd say
>>>
>>> compatible = "mediatek,mt8173-nor";
>>>
>>> because both compatible are referring to very specific IP version. It's
>>> not the same as
>>
>> But then you don't have the ability to handle a block in this particular
>> SoC in case there's a bug found in it in the future,
>> so IMO it should be:
>>
>> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";
>
> Sorry again, I meant
>
> compatible = "mediatek,mt2701-nor";
>
>>
>>> compatible = "mediatek,mt8173-nor", "mediatek,mt81xx-nor";
>>
>> This doesn't look right, since here we add two new compatibles ...
>
> That was just an example to describe how compatible inheritance works
> (at least that's my understanding of it), it does not apply to this
> particular use case.
Well this is OK I guess, but then you can also use "mediatek,mt8173-nor"
as the oldest supported compatible and be done with it, no ? It looks a
bit crappy though, I admit that ...
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH v3] ARM: dts: Add LEGO MINDSTORMS EV3 dts
From: kbuild test robot @ 2017-01-15 0:13 UTC (permalink / raw)
Cc: kbuild-all, devicetree, linux-arm-kernel, David Lechner,
Sekhar Nori, Kevin Hilman, Rob Herring, Mark Rutland,
linux-kernel
In-Reply-To: <1484253167-27568-1-git-send-email-david@lechnology.com>
[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]
Hi David,
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/David-Lechner/ARM-dts-Add-LEGO-MINDSTORMS-EV3-dts/20170114-145113
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-u300_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> Error: arch/arm/boot/dts/da850-lego-ev3.dts:310.1-6 Label or path usb1 not found
FATAL ERROR: Syntax error parsing input tree
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12462 bytes --]
^ permalink raw reply
* Re: [PATCH v3 16/24] media: Add i.MX media core driver
From: Steve Longerbeam @ 2017-01-14 22:46 UTC (permalink / raw)
To: Philipp Zabel
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1484320822.31475.96.camel@pengutronix.de>
(sorry sending again w/o html)
On 01/13/2017 07:20 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> +For image capture, the IPU contains the following internal subunits:
>> +
>> +- Image DMA Controller (IDMAC)
>> +- Camera Serial Interface (CSI)
>> +- Image Converter (IC)
>> +- Sensor Multi-FIFO Controller (SMFC)
>> +- Image Rotator (IRT)
>> +- Video De-Interlace Controller (VDIC)
> Nitpick: Video De-Interlacing or Combining Block (VDIC)
done.
>> +
>> +The IDMAC is the DMA controller for transfer of image frames to and from
>> +memory. Various dedicated DMA channels exist for both video capture and
>> +display paths.
>> +
>> +The CSI is the frontend capture unit that interfaces directly with
>> +capture sensors over Parallel, BT.656/1120, and MIPI CSI-2 busses.
>> +
>> +The IC handles color-space conversion, resizing, and rotation
>> +operations.
> And horizontal flipping.
done.
>> There are three independent "tasks" within the IC that can
>> +carry out conversions concurrently: pre-processing encoding,
>> +pre-processing preview, and post-processing.
> s/preview/viewfinder/ seems to be the commonly used name.
replaced everywhere in the doc.
> This paragraph could mention that a single hardware unit is used
> transparently time multiplexed by the three tasks at different
> granularity for the downsizing, main processing, and rotation sections.
> The downscale unit switches between tasks at 8-pixel burst granularity,
> the main processing unit at line granularity. The rotation units switch
> only at frame granularity.
I've added that info.
>> +The SMFC is composed of four independent channels that each can transfer
>> +captured frames from sensors directly to memory concurrently.
>> +
>> +The IRT carries out 90 and 270 degree image rotation operations.
> ... on 8x8 pixel blocks, supported by the IDMAC which handles block
> transfers, block reordering, and vertical flipping.
done.
>> +The VDIC handles the conversion of interlaced video to progressive, with
>> +support for different motion compensation modes (low, medium, and high
>> +motion). The deinterlaced output frames from the VDIC can be sent to the
>> +IC pre-process preview task for further conversions.
>> +
>> +In addition to the IPU internal subunits, there are also two units
>> +outside the IPU that are also involved in video capture on i.MX:
>> +
>> +- MIPI CSI-2 Receiver for camera sensors with the MIPI CSI-2 bus
>> + interface. This is a Synopsys DesignWare core.
>> +- A video multiplexer for selecting among multiple sensor inputs to
>> + send to a CSI.
> Two of them, actually.
done.
>> +
>> +- Includes a Frame Interval Monitor (FIM) that can correct vertical sync
>> + problems with the ADV718x video decoders. See below for a description
>> + of the FIM.
> Could this also be used to calculate more precise capture timestamps?
An input capture function could do that, triggered off a VSYNC or FIELD
signal such as on the ADV718x. The FIM is only used to calculate
frame intervals at this point, but its input capture method could be
used to also record more accurate timestamps.
>> +Capture Pipelines
>> +-----------------
>> +
>> +The following describe the various use-cases supported by the pipelines.
>> +
>> +The links shown do not include the frontend sensor, video mux, or mipi
>> +csi-2 receiver links. This depends on the type of sensor interface
>> +(parallel or mipi csi-2). So in all cases, these pipelines begin with:
>> +
>> +sensor -> ipu_csi_mux -> ipu_csi -> ...
>> +
>> +for parallel sensors, or:
>> +
>> +sensor -> imx-mipi-csi2 -> (ipu_csi_mux) -> ipu_csi -> ...
>> +
>> +for mipi csi-2 sensors. The imx-mipi-csi2 receiver may need to route
>> +to the video mux (ipu_csi_mux) before sending to the CSI, depending
>> +on the mipi csi-2 virtual channel, hence ipu_csi_mux is shown in
>> +parenthesis.
>> +
>> +Unprocessed Video Capture:
>> +--------------------------
>> +
>> +Send frames directly from sensor to camera interface, with no
>> +conversions:
>> +
>> +-> ipu_smfc -> camif
> I'd call this capture interface, this is not just for cameras. Or maybe
> idmac if you want to mirror hardware names?
Camif is so named because it is the V4L2 user interface for video
capture. I suppose it could be named "capif", but that doesn't role
off the tongue quite as well.
>> +Note the ipu_smfc can do pixel reordering within the same colorspace.
> That isn't a feature of the SMFC, but of the IDMAC (FCW & FCR).
yes, the doc is re-worded to make that more clear.
>> +For example, its sink pad can take UYVY2X8, but its source pad can
>> +output YUYV2X8.
> I don't think this is correct. Re-reading "37.4.3.7 Packing to memory"
> in the CSI chapter, for 8-bit per component data, the internal format
> between CSI, SMFC, and IDMAC is always some 32-bit RGBx/YUVx variant
> (or "bayer/generic data"). In either case, the internal format does not
> change along the way.
these are pixels in memory buffers, not the IPU internal formats.
>> +IC Direct Conversions:
>> +----------------------
>> +
>> +This pipeline uses the preprocess encode entity to route frames directly
>> +from the CSI to the IC (bypassing the SMFC), to carry out scaling up to
>> +1024x1024 resolution, CSC, and image rotation:
>> +
>> +-> ipu_ic_prpenc -> camif
>> +
>> +This can be a useful capture pipeline for heavily loaded memory bus
>> +traffic environments, since it has minimal IDMAC channel usage.
> Note that if rotation is enabled, transfers between IC processing and
> rotation still have to go through memory once.
yep.
>> +Post-Processing Conversions:
>> +----------------------------
>> +
>> +This pipeline routes frames from the SMFC to the post-processing
>> +entity.
> No, frames written by the CSI -> SMFC -> IDMAC path are read back into
> the post-processing entity.
that's true. The post-processing entity kicks off its read channels
to transfer those frames into the post-processor. Anyway this wording
will change after doing away with the SMFC entity.
>> + media-ctl -V "\"ipu1_csi0\":1 [fmt:YUYV2X8/640x480]"
>> + media-ctl -V "\"ipu1_smfc0\":0 [fmt:YUYV2X8/640x480]"
>> + media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/640x480]"
> I think the smfc entities should be dropped.
yes working on that.
>> + media-ctl -V "\"camif0\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"camif0\":1 [fmt:UYVY2X8/640x480]"
>> + # Configure pads for OV5640 pipeline
>> + media-ctl -V "\"ov5640_mipi 1-0040\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"imx-mipi-csi2\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"imx-mipi-csi2\":2 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"ipu1_csi1\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"ipu1_csi1\":1 [fmt:UYVY2X8/640x480]"
> [...]
>> + media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"
> I agree this looks very intuitive, but technically correct for the
> csi1:1 and camif1:0 pads would be a 32-bit YUV format.
> (MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).
>
> I think it would be better to use the correct format
I'm not sure I follow you here.
> as that will allow
> to chose the regular vs. companded packings in the future for formats
> with more than 8 bits per component.
>
>> + media-ctl -V "\"camif1\":1 [fmt:UYVY2X8/640x480]"
>> +
>> +Streaming can then begin independently on device nodes /dev/video0
>> +and /dev/video1.
>> +
>> +SabreAuto with ADV7180 decoder
>> +------------------------------
>> +
>> +On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>> +parallel bus input on the internal video mux to IPU1 CSI0.
>> +
>> +The following example configures a pipeline to capture from the ADV7180
>> +video decoder, assuming NTSC 720x480 input signals, with Motion
>> +Compensated de-interlacing (not shown: all pad field types should be set
>> +as indicated). $outputfmt can be any format supported by the
>> +ipu1_ic_prpvf entity at its output pad:
>> +
>> +.. code-block:: none
>> +
>> + # Setup links
>> + media-ctl -l '"adv7180 3-0021":0 -> "ipu1_csi0_mux":1[1]'
>> + media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
>> + media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
>> + media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
>> + media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
>> + media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
>> + # Configure pads
>> + # pad field types for below pads must be an interlaced type
>> + # such as "ALTERNATE"
> I think alternate should only extend as far as the CSI, since the CSI
> can only capture NTSC/PAL fields in a fixed order.
Agreed, I'm doing the translation from alternate to seq_bt/seq_tb depending
on sensor std in imx-vdic.c, but it should move upstream. Will fix.
>> + media-ctl -V "\"adv7180 3-0021\":0 [fmt:UYVY2X8/720x480]"
>> + media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
> From here the interlaced field type should be sequential in the correct
> order depending on NTSC/PAL.
right.
>> +
>> +Frame Interval Monitor
>> +----------------------
>> +
>> +The adv718x decoders can occasionally send corrupt fields during
>> +NTSC/PAL signal re-sync (too little or too many video lines). When
>> +this happens, the IPU triggers a mechanism to re-establish vertical
>> +sync by adding 1 dummy line every frame, which causes a rolling effect
>> +from image to image, and can last a long time before a stable image is
>> +recovered. Or sometimes the mechanism doesn't work at all, causing a
>> +permanent split image (one frame contains lines from two consecutive
>> +captured images).
> Is it only SabreAuto on which the FIM mechanism can be used due to the
> pad routing?
No, FIM can be used on any target, the fim child node of ipu_csi just
needs to be enabled via status property.
> [...]
>> +/*
>> + * DMA buffer ring handling
>> + */
>> +struct imx_media_dma_buf_ring {
>> + struct imx_media_dev *imxmd;
>> +
>> + /* the ring */
>> + struct imx_media_dma_buf buf[IMX_MEDIA_MAX_RING_BUFS];
>> + /* the scratch buffer for underruns */
>> + struct imx_media_dma_buf scratch;
>> +
>> + /* buffer generator */
>> + struct media_entity *src;
>> + /* buffer receiver */
>> + struct media_entity *sink;
>> +
>> + spinlock_t lock;
>> +
>> + int num_bufs;
>> + unsigned long last_seq;
>> +};
> I don't think this belongs in the capture driver at all.
> Memory-to-memory transfers should be handled at the videobuf2 level.
see below.
> [...]
>> +static struct imx_media_dma_buf *
>> +__dma_buf_queue(struct imx_media_dma_buf_ring *ring, int index)
>> +{
>> + struct imx_media_dma_buf *buf;
>> +
>> + if (index >= ring->num_bufs)
>> + return ERR_PTR(-EINVAL);
>> +
>> + buf = &ring->buf[index];
>> + if (WARN_ON(buf->state != IMX_MEDIA_BUF_STATUS_PREPARED))
>> + return ERR_PTR(-EINVAL);
>> +
>> + buf->state = IMX_MEDIA_BUF_STATUS_QUEUED;
>> + buf->seq = ring->last_seq++;
>> +
>> + return buf;
>> +}
> Is this a whole software buffer queue implementation? I thought the
> whole point of putting the custom mem2mem framework into the capture
> driver was to use the hardware FSU channel linking?
see below.
> What is the purpose of this if the sink should be triggered by the FSU?
Ok, here is where I need to make an admission.
The only FSU links I have attempted (and which currently have entries
in the fsu_link_info[] table), are the enc/vf/pp --> IRT links for rotation.
There does not appear to be support in the FSU for linking a write channel
to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
is support for the direct link from CSI (which I am using), but that's
not an
IDMAC channel link.
There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
0..2 (and 3? it's not clear, and not clear whether this includes channel 1).
But I think this links to channel 12, and not to channels 8,9,10 to the
VDIC.
Or will it? It's worth experimenting. It would have helped if FSL listed
which
IDMAC channels these FSU links correspond to, instead of making us guess
at it.
In any event, the docs are not clear enough to implement a real FSU
link to the VDIC read channels, if it's even possible. And trying to get
programming help from FSL can be difficult, and no coding examples
for this link AFAIK.
So I ended resorted to linking to VDIC channels 8,9,10 with a software
approach, instead of attempting a hardware FSU link.
The EOF interrupt handler for the SMFC channels informs the VDIC
entity via a v4l2_subdev_ioctl() call that a buffer is available. The
VDIC then manually kicks off its read channels to bring that buffer
(and a previous buffer for F(n-1) field) into the VDIC.
There is a small amount of extra overhead going this route compared
to a FSU hardware link: there is the EOF irq latency (a few usec), and
the CPU overhead for the VDIC to manually start the read channels,
which is also a few usec at most (see prepare_vdi_in_buffers() in
imx-vdic.c). So in total at most ~10 usec of extra overhead (CPU
use plus irq latency) under normal system load.
Of course, in order to implement this software link, I had to implement
a straightforward FIFO dma buffer ring. The sink (VDIC) allocates the ring
at stream on, and the source requests a pointer to this ring in its own
stream on. Passing buffers from source to sink then follows a
straightforward
FIFO queue/done/dequeue/queue model: sink queues buffers to src, src
grabs queued buffers and makes them active, src signals completed
buffers to sink, sink dequeues buffers in response, and sink queues
buffers back when it is finished with them.
> [...]
>> +/*
>> + * The subdevs have to be powered on/off, and streaming
>> + * enabled/disabled, in a specific sequence.
>> + */
>> +static const u32 stream_on_seq[] = {
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> + IMX_MEDIA_GRP_ID_CSI2,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_CSI,
>> +};
>> +
>> +static const u32 stream_off_seq[] = {
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_CSI,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_CSI2,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> +};
>> +
>> +#define NUM_STREAM_ENTITIES ARRAY_SIZE(stream_on_seq)
>> +
>> +static const u32 power_on_seq[] = {
>> + IMX_MEDIA_GRP_ID_CSI2,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_CSI,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> +};
>> +
>> +static const u32 power_off_seq[] = {
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_CSI,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> + IMX_MEDIA_GRP_ID_CSI2,
>> +};
> This seems somewhat arbitrary. Why is a power sequence needed?
The CSI-2 receiver must be powered up before the sensor, that's the
only requirement IIRC. The others have no s_power requirement. So I
can probably change this to power up in the frontend -> backend order
(IC_PP to sensor). And vice-versa for power off.
> [...]
>> +/*
>> + * Turn current pipeline power on/off starting from start_entity.
>> + * Must be called with mdev->graph_mutex held.
>> + */
>> +int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
>> + struct media_entity_graph *graph,
>> + struct media_entity *start_entity, bool on)
>> +{
>> + struct media_entity *entity;
>> + struct v4l2_subdev *sd;
>> + int i, ret = 0;
>> + u32 id;
>> +
>> + for (i = 0; i < NUM_POWER_ENTITIES; i++) {
>> + id = on ? power_on_seq[i] : power_off_seq[i];
>> + entity = find_pipeline_entity(imxmd, graph, start_entity, id);
>> + if (!entity)
>> + continue;
>> +
>> + sd = media_entity_to_v4l2_subdev(entity);
>> +
>> + ret = v4l2_subdev_call(sd, core, s_power, on);
>> + if (ret && ret != -ENOIOCTLCMD)
>> + break;
>> + }
>> +
>> + return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
>> +}
>> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
> This should really be handled by v4l2_pipeline_pm_use.
I thought about this earlier, but v4l2_pipeline_pm_use() seems to be
doing some other stuff that bothered me, at least that's what I remember.
I will revisit this.
>> +/*
>> + * Inherit the v4l2 controls from all entities in a pipeline
>> + * to the given video device.
>> + * Must be called with mdev->graph_mutex held.
>> + */
>> +int imx_media_inherit_controls(struct imx_media_dev *imxmd,
>> + struct video_device *vfd,
>> + struct media_entity *start_entity)
>> +{
>> + struct media_entity_graph graph;
>> + struct media_entity *entity;
>> + struct v4l2_subdev *sd;
>> + int ret;
>> +
>> + ret = media_entity_graph_walk_init(&graph, &imxmd->md);
>> + if (ret)
>> + return ret;
>> +
>> + media_entity_graph_walk_start(&graph, start_entity);
>> +
>> + while ((entity = media_entity_graph_walk_next(&graph))) {
>> + if (is_media_entity_v4l2_video_device(entity))
>> + continue;
>> +
>> + sd = media_entity_to_v4l2_subdev(entity);
>> +
>> + dev_dbg(imxmd->dev, "%s: adding controls from %s\n",
>> + __func__, sd->name);
>> +
>> + ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
>> + sd->ctrl_handler,
>> + NULL);
>> + if (ret)
>> + break;
>> + }
>> +
>> + media_entity_graph_walk_cleanup(&graph);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(imx_media_inherit_controls);
>> +
>> +MODULE_DESCRIPTION("i.MX5/6 v4l2 media controller driver");
>> +MODULE_AUTHOR("Steve Longerbeam<steve_longerbeam@mentor.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
>> new file mode 100644
>> index 0000000..357654d
>> --- /dev/null
>> +++ b/drivers/staging/media/imx/imx-media-dev.c
> This file is full of code that should live in the v4l2 core.
Agreed. Stuff like imx_media_inherit_controls() really should be widely
available.
>> +int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
>> + struct imx_media_subdev *csi[4])
>> +{
>> + int ret;
>> +
>> + /* there must be at least one CSI in first IPU */
> Why?
Well yeah, imx-media doesn't necessarily need a CSI if things
like the VDIC or post-processor are being used by an output
overlay pipeline, for example. I'll fix this.
>> +
>> +/* parse inputs property from a sensor node */
>> +static void of_parse_sensor_inputs(struct imx_media_dev *imxmd,
>> + struct imx_media_subdev *sensor,
>> + struct device_node *sensor_np)
>> +{
>> + struct imx_media_sensor_input *sinput = &sensor->input;
>> + int ret, i;
>> +
>> + for (i = 0; i < IMX_MEDIA_MAX_SENSOR_INPUTS; i++) {
>> + const char *input_name;
>> + u32 val;
>> +
>> + ret = of_property_read_u32_index(sensor_np, "inputs", i, &val);
>> + if (ret)
>> + break;
>> +
>> + sinput->value[i] = val;
>> +
>> + ret = of_property_read_string_index(sensor_np, "input-names",
>> + i, &input_name);
>> + /*
>> + * if input-names not provided, they will be set using
>> + * the subdev name once the sensor is known during
>> + * async bind
>> + */
>> + if (!ret)
>> + strncpy(sinput->name[i], input_name,
>> + sizeof(sinput->name[i]));
>> + }
>> +
>> + sinput->num = i;
>> +
>> + /* if no inputs provided just assume a single input */
>> + if (sinput->num == 0)
>> + sinput->num = 1;
>> +}
> This should be parsed by the sensor driver, not imx-media.
you're probably right. I'll submit a patch for adv7180.c.
>> +static void of_parse_sensor(struct imx_media_dev *imxmd,
>> + struct imx_media_subdev *sensor,
>> + struct device_node *sensor_np)
>> +{
>> + struct device_node *endpoint;
>> +
>> + of_parse_sensor_inputs(imxmd, sensor, sensor_np);
>> +
>> + endpoint = of_graph_get_next_endpoint(sensor_np, NULL);
>> + if (endpoint) {
>> + v4l2_of_parse_endpoint(endpoint, &sensor->sensor_ep);
>> + of_node_put(endpoint);
>> + }
>> +}
>> +
>> +static int of_get_port_count(const struct device_node *np)
>> +{
>> + struct device_node *child;
>> + int num = 0;
>> +
>> + /* if this node is itself a port, return 1 */
>> + if (of_node_cmp(np->name, "port") == 0)
>> + return 1;
>> +
>> + for_each_child_of_node(np, child)
>> + if (of_node_cmp(child->name, "port") == 0)
>> + num++;
>> +
>> + return num;
>> +}
> If this is extended to handle the ports subnode properly, it could be
> moved into drivers/of/base.c.
More that eventually needs exporting, agreed.
Steve
^ permalink raw reply
* Re: [PATCH v3 16/24] media: Add i.MX media core driver
From: Steve Longerbeam @ 2017-01-14 22:42 UTC (permalink / raw)
To: Philipp Zabel
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1484320822.31475.96.camel@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 20552 bytes --]
On 01/13/2017 07:20 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> +For image capture, the IPU contains the following internal subunits:
>> +
>> +- Image DMA Controller (IDMAC)
>> +- Camera Serial Interface (CSI)
>> +- Image Converter (IC)
>> +- Sensor Multi-FIFO Controller (SMFC)
>> +- Image Rotator (IRT)
>> +- Video De-Interlace Controller (VDIC)
> Nitpick: Video De-Interlacing or Combining Block (VDIC)
done.
>> +
>> +The IDMAC is the DMA controller for transfer of image frames to and from
>> +memory. Various dedicated DMA channels exist for both video capture and
>> +display paths.
>> +
>> +The CSI is the frontend capture unit that interfaces directly with
>> +capture sensors over Parallel, BT.656/1120, and MIPI CSI-2 busses.
>> +
>> +The IC handles color-space conversion, resizing, and rotation
>> +operations.
> And horizontal flipping.
done.
>> There are three independent "tasks" within the IC that can
>> +carry out conversions concurrently: pre-processing encoding,
>> +pre-processing preview, and post-processing.
> s/preview/viewfinder/ seems to be the commonly used name.
replaced everywhere in the doc.
> This paragraph could mention that a single hardware unit is used
> transparently time multiplexed by the three tasks at different
> granularity for the downsizing, main processing, and rotation sections.
> The downscale unit switches between tasks at 8-pixel burst granularity,
> the main processing unit at line granularity. The rotation units switch
> only at frame granularity.
I've added that info.
>> +The SMFC is composed of four independent channels that each can transfer
>> +captured frames from sensors directly to memory concurrently.
>> +
>> +The IRT carries out 90 and 270 degree image rotation operations.
> ... on 8x8 pixel blocks, supported by the IDMAC which handles block
> transfers, block reordering, and vertical flipping.
done.
>> +The VDIC handles the conversion of interlaced video to progressive, with
>> +support for different motion compensation modes (low, medium, and high
>> +motion). The deinterlaced output frames from the VDIC can be sent to the
>> +IC pre-process preview task for further conversions.
>> +
>> +In addition to the IPU internal subunits, there are also two units
>> +outside the IPU that are also involved in video capture on i.MX:
>> +
>> +- MIPI CSI-2 Receiver for camera sensors with the MIPI CSI-2 bus
>> + interface. This is a Synopsys DesignWare core.
>> +- A video multiplexer for selecting among multiple sensor inputs to
>> + send to a CSI.
> Two of them, actually.
done.
>> +
>> +- Includes a Frame Interval Monitor (FIM) that can correct vertical sync
>> + problems with the ADV718x video decoders. See below for a description
>> + of the FIM.
> Could this also be used to calculate more precise capture timestamps?
An input capture function could do that, triggered off a VSYNC or FIELD
signal such as on the ADV718x. The FIM is only used to calculate
frame intervals at this point, but its input capture method could be
used to also record more accurate timestamps.
>> +Capture Pipelines
>> +-----------------
>> +
>> +The following describe the various use-cases supported by the pipelines.
>> +
>> +The links shown do not include the frontend sensor, video mux, or mipi
>> +csi-2 receiver links. This depends on the type of sensor interface
>> +(parallel or mipi csi-2). So in all cases, these pipelines begin with:
>> +
>> +sensor -> ipu_csi_mux -> ipu_csi -> ...
>> +
>> +for parallel sensors, or:
>> +
>> +sensor -> imx-mipi-csi2 -> (ipu_csi_mux) -> ipu_csi -> ...
>> +
>> +for mipi csi-2 sensors. The imx-mipi-csi2 receiver may need to route
>> +to the video mux (ipu_csi_mux) before sending to the CSI, depending
>> +on the mipi csi-2 virtual channel, hence ipu_csi_mux is shown in
>> +parenthesis.
>> +
>> +Unprocessed Video Capture:
>> +--------------------------
>> +
>> +Send frames directly from sensor to camera interface, with no
>> +conversions:
>> +
>> +-> ipu_smfc -> camif
> I'd call this capture interface, this is not just for cameras. Or maybe
> idmac if you want to mirror hardware names?
Camif is so named because it is the V4L2 user interface for video
capture. I suppose it could be named "capif", but that doesn't role
off the tongue quite as well.
>> +Note the ipu_smfc can do pixel reordering within the same colorspace.
> That isn't a feature of the SMFC, but of the IDMAC (FCW & FCR).
yes, the doc is re-worded to make that more clear.
>> +For example, its sink pad can take UYVY2X8, but its source pad can
>> +output YUYV2X8.
> I don't think this is correct. Re-reading "37.4.3.7 Packing to memory"
> in the CSI chapter, for 8-bit per component data, the internal format
> between CSI, SMFC, and IDMAC is always some 32-bit RGBx/YUVx variant
> (or "bayer/generic data"). In either case, the internal format does not
> change along the way.
these are pixels in memory buffers, not the IPU internal formats.
>> +IC Direct Conversions:
>> +----------------------
>> +
>> +This pipeline uses the preprocess encode entity to route frames directly
>> +from the CSI to the IC (bypassing the SMFC), to carry out scaling up to
>> +1024x1024 resolution, CSC, and image rotation:
>> +
>> +-> ipu_ic_prpenc -> camif
>> +
>> +This can be a useful capture pipeline for heavily loaded memory bus
>> +traffic environments, since it has minimal IDMAC channel usage.
> Note that if rotation is enabled, transfers between IC processing and
> rotation still have to go through memory once.
yep.
>> +Post-Processing Conversions:
>> +----------------------------
>> +
>> +This pipeline routes frames from the SMFC to the post-processing
>> +entity.
> No, frames written by the CSI -> SMFC -> IDMAC path are read back into
> the post-processing entity.
that's true. The post-processing entity kicks off its read channels
to transfer those frames into the post-processor. Anyway this wording
will change after doing away with the SMFC entity.
>> + media-ctl -V "\"ipu1_csi0\":1 [fmt:YUYV2X8/640x480]"
>> + media-ctl -V "\"ipu1_smfc0\":0 [fmt:YUYV2X8/640x480]"
>> + media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/640x480]"
> I think the smfc entities should be dropped.
yes working on that.
>> + media-ctl -V "\"camif0\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"camif0\":1 [fmt:UYVY2X8/640x480]"
>> + # Configure pads for OV5640 pipeline
>> + media-ctl -V "\"ov5640_mipi 1-0040\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"imx-mipi-csi2\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"imx-mipi-csi2\":2 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"ipu1_csi1\":0 [fmt:UYVY2X8/640x480]"
>> + media-ctl -V "\"ipu1_csi1\":1 [fmt:UYVY2X8/640x480]"
> [...]
>> + media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"
> I agree this looks very intuitive, but technically correct for the
> csi1:1 and camif1:0 pads would be a 32-bit YUV format.
> (MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).
>
> I think it would be better to use the correct format
I'm not sure I follow you here.
> as that will allow
> to chose the regular vs. companded packings in the future for formats
> with more than 8 bits per component.
>
>> + media-ctl -V "\"camif1\":1 [fmt:UYVY2X8/640x480]"
>> +
>> +Streaming can then begin independently on device nodes /dev/video0
>> +and /dev/video1.
>> +
>> +SabreAuto with ADV7180 decoder
>> +------------------------------
>> +
>> +On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>> +parallel bus input on the internal video mux to IPU1 CSI0.
>> +
>> +The following example configures a pipeline to capture from the ADV7180
>> +video decoder, assuming NTSC 720x480 input signals, with Motion
>> +Compensated de-interlacing (not shown: all pad field types should be set
>> +as indicated). $outputfmt can be any format supported by the
>> +ipu1_ic_prpvf entity at its output pad:
>> +
>> +.. code-block:: none
>> +
>> + # Setup links
>> + media-ctl -l '"adv7180 3-0021":0 -> "ipu1_csi0_mux":1[1]'
>> + media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
>> + media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
>> + media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
>> + media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
>> + media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
>> + # Configure pads
>> + # pad field types for below pads must be an interlaced type
>> + # such as "ALTERNATE"
> I think alternate should only extend as far as the CSI, since the CSI
> can only capture NTSC/PAL fields in a fixed order.
Agreed, I'm doing the translation from alternate to seq_bt/seq_tb depending
on sensor std in imx-vdic.c, but it should move upstream. Will fix.
>> + media-ctl -V "\"adv7180 3-0021\":0 [fmt:UYVY2X8/720x480]"
>> + media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
> From here the interlaced field type should be sequential in the correct
> order depending on NTSC/PAL.
right.
>> +
>> +Frame Interval Monitor
>> +----------------------
>> +
>> +The adv718x decoders can occasionally send corrupt fields during
>> +NTSC/PAL signal re-sync (too little or too many video lines). When
>> +this happens, the IPU triggers a mechanism to re-establish vertical
>> +sync by adding 1 dummy line every frame, which causes a rolling effect
>> +from image to image, and can last a long time before a stable image is
>> +recovered. Or sometimes the mechanism doesn't work at all, causing a
>> +permanent split image (one frame contains lines from two consecutive
>> +captured images).
> Is it only SabreAuto on which the FIM mechanism can be used due to the
> pad routing?
No, FIM can be used on any target, the fim child node of ipu_csi just
needs to be enabled via status property.
> [...]
>> +/*
>> + * DMA buffer ring handling
>> + */
>> +struct imx_media_dma_buf_ring {
>> + struct imx_media_dev *imxmd;
>> +
>> + /* the ring */
>> + struct imx_media_dma_buf buf[IMX_MEDIA_MAX_RING_BUFS];
>> + /* the scratch buffer for underruns */
>> + struct imx_media_dma_buf scratch;
>> +
>> + /* buffer generator */
>> + struct media_entity *src;
>> + /* buffer receiver */
>> + struct media_entity *sink;
>> +
>> + spinlock_t lock;
>> +
>> + int num_bufs;
>> + unsigned long last_seq;
>> +};
> I don't think this belongs in the capture driver at all.
> Memory-to-memory transfers should be handled at the videobuf2 level.
see below.
> [...]
>> +static struct imx_media_dma_buf *
>> +__dma_buf_queue(struct imx_media_dma_buf_ring *ring, int index)
>> +{
>> + struct imx_media_dma_buf *buf;
>> +
>> + if (index >= ring->num_bufs)
>> + return ERR_PTR(-EINVAL);
>> +
>> + buf = &ring->buf[index];
>> + if (WARN_ON(buf->state != IMX_MEDIA_BUF_STATUS_PREPARED))
>> + return ERR_PTR(-EINVAL);
>> +
>> + buf->state = IMX_MEDIA_BUF_STATUS_QUEUED;
>> + buf->seq = ring->last_seq++;
>> +
>> + return buf;
>> +}
> Is this a whole software buffer queue implementation? I thought the
> whole point of putting the custom mem2mem framework into the capture
> driver was to use the hardware FSU channel linking?
see below.
> What is the purpose of this if the sink should be triggered by the FSU?
Ok, here is where I need to make an admission.
The only FSU links I have attempted (and which currently have entries
in the fsu_link_info[] table), are the enc/vf/pp --> IRT links for rotation.
There does not appear to be support in the FSU for linking a write channel
to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
is support for the direct link from CSI (which I am using), but that's
not an
IDMAC channel link.
There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
0..2 (and 3? it's not clear, and not clear whether this includes channel 1).
But I think this links to channel 12, and not to channels 8,9,10 to the
VDIC.
Or will it? It's worth experimenting. It would have helped if FSL listed
which
IDMAC channels these FSU links correspond to, instead of making us guess
at it.
In any event, the docs are not clear enough to implement a real FSU
link to the VDIC read channels, if it's even possible. And trying to get
programming help from FSL can be difficult, and no coding examples
for this link AFAIK.
So I ended resorted to linking to VDIC channels 8,9,10 with a software
approach, instead of attempting a hardware FSU link.
The EOF interrupt handler for the SMFC channels informs the VDIC
entity via a v4l2_subdev_ioctl() call that a buffer is available. The
VDIC then manually kicks off its read channels to bring that buffer
(and a previous buffer for F(n-1) field) into the VDIC.
There is a small amount of extra overhead going this route compared
to a FSU hardware link: there is the EOF irq latency (a few usec), and
the CPU overhead for the VDIC to manually start the read channels,
which is also a few usec at most (see prepare_vdi_in_buffers() in
imx-vdic.c). So in total at most ~10 usec of extra overhead (CPU
use plus irq latency) under normal system load.
Of course, in order to implement this software link, I had to implement
a straightforward FIFO dma buffer ring. The sink (VDIC) allocates the ring
at stream on, and the source requests a pointer to this ring in its own
stream on. Passing buffers from source to sink then follows a
straightforward
FIFO queue/done/dequeue/queue model: sink queues buffers to src, src
grabs queued buffers and makes them active, src signals completed
buffers to sink, sink dequeues buffers in response, and sink queues
buffers back when it is finished with them.
> [...]
>> +/*
>> + * The subdevs have to be powered on/off, and streaming
>> + * enabled/disabled, in a specific sequence.
>> + */
>> +static const u32 stream_on_seq[] = {
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> + IMX_MEDIA_GRP_ID_CSI2,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_CSI,
>> +};
>> +
>> +static const u32 stream_off_seq[] = {
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_CSI,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_CSI2,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> +};
>> +
>> +#define NUM_STREAM_ENTITIES ARRAY_SIZE(stream_on_seq)
>> +
>> +static const u32 power_on_seq[] = {
>> + IMX_MEDIA_GRP_ID_CSI2,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_CSI,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> +};
>> +
>> +static const u32 power_off_seq[] = {
>> + IMX_MEDIA_GRP_ID_IC_PP,
>> + IMX_MEDIA_GRP_ID_IC_PRPVF,
>> + IMX_MEDIA_GRP_ID_IC_PRPENC,
>> + IMX_MEDIA_GRP_ID_SMFC,
>> + IMX_MEDIA_GRP_ID_CSI,
>> + IMX_MEDIA_GRP_ID_VIDMUX,
>> + IMX_MEDIA_GRP_ID_SENSOR,
>> + IMX_MEDIA_GRP_ID_CSI2,
>> +};
> This seems somewhat arbitrary. Why is a power sequence needed?
The CSI-2 receiver must be powered up before the sensor, that's the
only requirement IIRC. The others have no s_power requirement. So I
can probably change this to power up in the frontend -> backend order
(IC_PP to sensor). And vice-versa for power off.
> [...]
>> +/*
>> + * Turn current pipeline power on/off starting from start_entity.
>> + * Must be called with mdev->graph_mutex held.
>> + */
>> +int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
>> + struct media_entity_graph *graph,
>> + struct media_entity *start_entity, bool on)
>> +{
>> + struct media_entity *entity;
>> + struct v4l2_subdev *sd;
>> + int i, ret = 0;
>> + u32 id;
>> +
>> + for (i = 0; i < NUM_POWER_ENTITIES; i++) {
>> + id = on ? power_on_seq[i] : power_off_seq[i];
>> + entity = find_pipeline_entity(imxmd, graph, start_entity, id);
>> + if (!entity)
>> + continue;
>> +
>> + sd = media_entity_to_v4l2_subdev(entity);
>> +
>> + ret = v4l2_subdev_call(sd, core, s_power, on);
>> + if (ret && ret != -ENOIOCTLCMD)
>> + break;
>> + }
>> +
>> + return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
>> +}
>> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
> This should really be handled by v4l2_pipeline_pm_use.
I thought about this earlier, but v4l2_pipeline_pm_use() seems to be
doing some other stuff that bothered me, at least that's what I remember.
I will revisit this.
>> +/*
>> + * Inherit the v4l2 controls from all entities in a pipeline
>> + * to the given video device.
>> + * Must be called with mdev->graph_mutex held.
>> + */
>> +int imx_media_inherit_controls(struct imx_media_dev *imxmd,
>> + struct video_device *vfd,
>> + struct media_entity *start_entity)
>> +{
>> + struct media_entity_graph graph;
>> + struct media_entity *entity;
>> + struct v4l2_subdev *sd;
>> + int ret;
>> +
>> + ret = media_entity_graph_walk_init(&graph, &imxmd->md);
>> + if (ret)
>> + return ret;
>> +
>> + media_entity_graph_walk_start(&graph, start_entity);
>> +
>> + while ((entity = media_entity_graph_walk_next(&graph))) {
>> + if (is_media_entity_v4l2_video_device(entity))
>> + continue;
>> +
>> + sd = media_entity_to_v4l2_subdev(entity);
>> +
>> + dev_dbg(imxmd->dev, "%s: adding controls from %s\n",
>> + __func__, sd->name);
>> +
>> + ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
>> + sd->ctrl_handler,
>> + NULL);
>> + if (ret)
>> + break;
>> + }
>> +
>> + media_entity_graph_walk_cleanup(&graph);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(imx_media_inherit_controls);
>> +
>> +MODULE_DESCRIPTION("i.MX5/6 v4l2 media controller driver");
>> +MODULE_AUTHOR("Steve Longerbeam<steve_longerbeam@mentor.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
>> new file mode 100644
>> index 0000000..357654d
>> --- /dev/null
>> +++ b/drivers/staging/media/imx/imx-media-dev.c
> This file is full of code that should live in the v4l2 core.
Agreed. Stuff like imx_media_inherit_controls() really should be widely
available.
>> +int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
>> + struct imx_media_subdev *csi[4])
>> +{
>> + int ret;
>> +
>> + /* there must be at least one CSI in first IPU */
> Why?
Well yeah, imx-media doesn't necessarily need a CSI if things
like the VDIC or post-processor are being used by an output
overlay pipeline, for example. I'll fix this.
>> +
>> +/* parse inputs property from a sensor node */
>> +static void of_parse_sensor_inputs(struct imx_media_dev *imxmd,
>> + struct imx_media_subdev *sensor,
>> + struct device_node *sensor_np)
>> +{
>> + struct imx_media_sensor_input *sinput = &sensor->input;
>> + int ret, i;
>> +
>> + for (i = 0; i < IMX_MEDIA_MAX_SENSOR_INPUTS; i++) {
>> + const char *input_name;
>> + u32 val;
>> +
>> + ret = of_property_read_u32_index(sensor_np, "inputs", i, &val);
>> + if (ret)
>> + break;
>> +
>> + sinput->value[i] = val;
>> +
>> + ret = of_property_read_string_index(sensor_np, "input-names",
>> + i, &input_name);
>> + /*
>> + * if input-names not provided, they will be set using
>> + * the subdev name once the sensor is known during
>> + * async bind
>> + */
>> + if (!ret)
>> + strncpy(sinput->name[i], input_name,
>> + sizeof(sinput->name[i]));
>> + }
>> +
>> + sinput->num = i;
>> +
>> + /* if no inputs provided just assume a single input */
>> + if (sinput->num == 0)
>> + sinput->num = 1;
>> +}
> This should be parsed by the sensor driver, not imx-media.
you're probably right. I'll submit a patch for adv7180.c.
>> +static void of_parse_sensor(struct imx_media_dev *imxmd,
>> + struct imx_media_subdev *sensor,
>> + struct device_node *sensor_np)
>> +{
>> + struct device_node *endpoint;
>> +
>> + of_parse_sensor_inputs(imxmd, sensor, sensor_np);
>> +
>> + endpoint = of_graph_get_next_endpoint(sensor_np, NULL);
>> + if (endpoint) {
>> + v4l2_of_parse_endpoint(endpoint, &sensor->sensor_ep);
>> + of_node_put(endpoint);
>> + }
>> +}
>> +
>> +static int of_get_port_count(const struct device_node *np)
>> +{
>> + struct device_node *child;
>> + int num = 0;
>> +
>> + /* if this node is itself a port, return 1 */
>> + if (of_node_cmp(np->name, "port") == 0)
>> + return 1;
>> +
>> + for_each_child_of_node(np, child)
>> + if (of_node_cmp(child->name, "port") == 0)
>> + num++;
>> +
>> + return num;
>> +}
> If this is extended to handle the ports subnode properly, it could be
> moved into drivers/of/base.c.
More that eventually needs exporting, agreed.
Steve
[-- Attachment #2: Type: text/html, Size: 27007 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] mmc: pwrseq: add support for Marvell SD8787 chip
From: kbuild test robot @ 2017-01-14 21:37 UTC (permalink / raw)
Cc: kbuild-all, linux-wireless, linux-kernel, linux-mmc, devicetree,
tony, Matt Ranostay, Ulf Hansson
In-Reply-To: <20170113052218.10534-3-matt@ranostay.consulting>
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
Hi Matt,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Matt-Ranostay/mmc-pwrseq-add-support-for-Marvell-SD8787-chip/20170115-030459
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> make[4]: *** No rule to make target 'drivers/mmc/core/pwrseq_sd8787.c', needed by 'drivers/mmc/core/pwrseq_sd8787.o'.
make[4]: Target '__build' not remade because of errors.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57927 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox