Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 13/12/16 12:37, Jacopo Mondi wrote:
> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 
Nothing significant to add, but a few comments inline.

Jonathan
> ---
> 
> v1 -> v2:
> - incorporated pmeerw's review comments
> - retrieve vref from dts and use that to convert read_raw result
>   to mV
> - add device tree bindings documentation
> 
> ---
>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>  drivers/iio/adc/Kconfig|   9 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 166 
> +
>  4 files changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> new file mode 100644
> index 000..6877c11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> @@ -0,0 +1,17 @@
> +* Maxim max11100 Analog to Digital Converter (ADC)
> +
> +Required properties:
> +  - compatible: Should be "maxim,max11100"
> +  - vref-supply: phandle to the regulator that provides reference voltage
> +
> +Optional properties:
> +  - spi-max-frequency: SPI maximum frequency
> +
> +Example:
> +
> +adc0: max11100@0 {
> +compatible = "maxim,max11100";
> +vref-supply = <_vref>;
> +spi-max-frequency = <24>;
> +};
> +
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..a909484 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -285,6 +285,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
> + 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 7a40c04..1463044 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,166 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV (1 << 16)
> +#define MAX11100_LSB(vref)   (vref / MAX11100_LSB_DIV)
> +
> +struct max11100_state {
> + const struct max11100_chip_desc *desc;
> + struct spi_device *spi;
> + int vref_uv;
> + struct mutex lock;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> + { /* [0] */
> + .type = IIO_VOLTAGE,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 24,
> + .shift = 8,
> + .repeat = 1,
> + .endianness = IIO_BE,
> + },
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + },
> +};
> +
> +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_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int *val, int *val2, long mask)
> +{
> + int ret;
> + struct max11100_state *state = iio_priv(indio_dev);
> + uint8_t buffer[3];
> +
> + mutex_lock(>lock);
> +
> + ret = spi_read(state->spi, buffer, sizeof(buffer));
> +

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 14/12/16 12:00, jac...@jmondi.org wrote:
> Hello Peter,
>thanks for review
> 
> On 13/12/2016 21:33, Peter Meerwald-Stadler wrote:
>>
>>> Add IIO driver for Maxim MAX11100 single-channel ADC.
>>> Add DT bindings documentation.
>>
>> some more comments
>>
>>> Signed-off-by: Jacopo Mondi 
>>> ---
>>>
>>> v1 -> v2:
>>> - incorporated pmeerw's review comments
>>> - retrieve vref from dts and use that to convert read_raw result
>>>   to mV
>>> - add device tree bindings documentation
>>>
>>> ---
>>>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>>>  drivers/iio/adc/Kconfig|   9 ++
>>>  drivers/iio/adc/Makefile   |   1 +
>>>  drivers/iio/adc/max11100.c | 166 
>>> +
>>>  4 files changed, 193 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>>>  create mode 100644 drivers/iio/adc/max11100.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
>>> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
>>> new file mode 100644
>>> index 000..6877c11
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
>>> @@ -0,0 +1,17 @@
>>> +* Maxim max11100 Analog to Digital Converter (ADC)
>>> +
>>> +Required properties:
>>> +  - compatible: Should be "maxim,max11100"
>>> +  - vref-supply: phandle to the regulator that provides reference voltage
>>> +
>>> +Optional properties:
>>> +  - spi-max-frequency: SPI maximum frequency
>>> +
>>> +Example:
>>> +
>>> +adc0: max11100@0 {
>>> +compatible = "maxim,max11100";
>>> +vref-supply = <_vref>;
>>> +spi-max-frequency = <24>;
>>> +};
>>> +
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 99c0514..a909484 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -285,6 +285,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
>>
>> SPI_MASTER is more precise I think
>>
>>> +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 7a40c04..1463044 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -29,6 +29,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 000..f372ad8
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/max11100.c
>>> @@ -0,0 +1,166 @@
>>> +/*
>>> + * 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 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +/*
>>> + * LSB is the ADC single digital step
>>> + * 1 LSB = (vref / 2 ^ 16)
>>> + * AIN = (DIN * LSB)
>>> + */
>>> +#define MAX11100_LSB_DIV(1 << 16)
>>> +#define MAX11100_LSB(vref) (vref / MAX11100_LSB_DIV)
>>
>> maybe parenthesis around vref
>>
>>> +
>>> +struct max11100_state {
>>> +const struct max11100_chip_desc *desc;
>>> +struct spi_device *spi;
>>> +int vref_uv;
>>> +struct mutex lock;
>>> +};
>>> +
>>> +static struct iio_chan_spec max11100_channels[] = {
>>> +{ /* [0] */
>>> +.type = IIO_VOLTAGE,
>>> +.scan_type = {
>>
>> scan_type not needed since driver does not support buffered reads
>>
>>> +.sign = 'u',
>>> +.realbits = 16,
>>> +.storagebits = 24,
>>> +.shift = 8,
>>> +.repeat = 1,
>>> +.endianness = IIO_BE,
>>> +},
>>> +.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>> +},
>>> +};
>>> +
>>> +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,
>>> +};

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 13/12/16 15:53, Wolfram Sang wrote:
> 
>>> +struct max11100_state {
>>> +   const struct max11100_chip_desc *desc;
>>> +   struct spi_device *spi;
>>> +   int vref_uv;
>>
>> unsi
>> It's good practice to move the smaller members to the end of the structure,
>> to avoid gaps due to alignment rules (struct mutex needs to be 64-bit aligned
>> on 64-bit platforms).
> 
> One option. Another idea is to put the most used members to the front to
> increase chances of being in the same cacheline.
> 
Or more cynically, just put them in the order that makes most logical sense
as that gives you fewest grumpy reviewers.
We aren't really talking high performance stuff here!

Jonathan

Re: [PATCHv2] iio: adc: Add Maxim MAX11100 driver

2016-12-30 Thread Jonathan Cameron 
On 13/12/16 12:37, Jacopo Mondi wrote:
> Add IIO driver for Maxim MAX11100 single-channel ADC.
> Add DT bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 
A quick process related thing.  Please don't post a new
version of a driver as a reply to an old version. It very rapidly
leads to deep and difficult to follow threads.

Much easier to just start a new thread.

> ---
> 
> v1 -> v2:
> - incorporated pmeerw's review comments
> - retrieve vref from dts and use that to convert read_raw result
>   to mV
> - add device tree bindings documentation
> 
> ---
>  .../devicetree/bindings/iio/adc/max11100.txt   |  17 +++
>  drivers/iio/adc/Kconfig|   9 ++
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max11100.c | 166 
> +
>  4 files changed, 193 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
>  create mode 100644 drivers/iio/adc/max11100.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt 
> b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> new file mode 100644
> index 000..6877c11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
> @@ -0,0 +1,17 @@
> +* Maxim max11100 Analog to Digital Converter (ADC)
> +
> +Required properties:
> +  - compatible: Should be "maxim,max11100"
> +  - vref-supply: phandle to the regulator that provides reference voltage
> +
> +Optional properties:
> +  - spi-max-frequency: SPI maximum frequency
> +
> +Example:
> +
> +adc0: max11100@0 {
> +compatible = "maxim,max11100";
> +vref-supply = <_vref>;
> +spi-max-frequency = <24>;
> +};
> +
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..a909484 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -285,6 +285,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
> + 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 7a40c04..1463044 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -29,6 +29,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 000..f372ad8
> --- /dev/null
> +++ b/drivers/iio/adc/max11100.c
> @@ -0,0 +1,166 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * LSB is the ADC single digital step
> + * 1 LSB = (vref / 2 ^ 16)
> + * AIN = (DIN * LSB)
> + */
> +#define MAX11100_LSB_DIV (1 << 16)
> +#define MAX11100_LSB(vref)   (vref / MAX11100_LSB_DIV)
> +
> +struct max11100_state {
> + const struct max11100_chip_desc *desc;
> + struct spi_device *spi;
> + int vref_uv;
> + struct mutex lock;
> +};
> +
> +static struct iio_chan_spec max11100_channels[] = {
> + { /* [0] */
> + .type = IIO_VOLTAGE,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 24,
> + .shift = 8,
> + .repeat = 1,
> + .endianness = IIO_BE,
> + },
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + },
> +};
> +
> +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_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int *val, int *val2, long mask)
> +{
> + int ret;
> + struct max11100_state *state =

Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

2016-12-30 Thread Sergei Shtylyov 

On 12/30/2016 12:46 PM, Sergei Shtylyov wrote:


It is possible that PCI device supports 64-bit DMA addressing, and thus
it's driver sets device's dma_mask to DMA_BIT_MASK(64), however PCI host


   Its.


bridge has limitations on inbound transactions addressing. Example of
such setup is NVME


   Isn't it called NVMe?


SSD device connected to RCAR PCIe controller.


   R=Car.


   Sorry, R-Car. :-)

[...]

MBR, Sergei