* Re: [PATCH 1/2] kbuild: create built-in.o automatically if parent directory wants it
From: Masahiro Yamada @ 2017-11-18 4:06 UTC (permalink / raw)
To: Linux Kbuild mailing list, Sam Ravnborg
Cc: Rob Herring, Mark Rutland, Pantelis Antoniou, devicetree,
Arnd Bergmann, Linux-MIPS, Linux Kernel Mailing List,
Michal Marek, Masahiro Yamada, Michal Marek
In-Reply-To: <1510072307-16819-2-git-send-email-yamada.masahiro@socionext.com>
2017-11-08 1:31 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> "obj-y += foo/" syntax requires Kbuild to visit the "foo" subdirectory
> and link built-in.o from that directory. This means foo/Makefile is
> responsible for creating built-in.o even if there is no object to
> link (in this case, built-in.o is an empty archive).
>
> We have had several fixups like commit 4b024242e8a4 ("kbuild: Fix
> linking error built-in.o no such file or directory"), then ended up
> with a complex condition as follows:
>
> ifneq ($(strip $(obj-y) $(obj-m) $(obj-) $(subdir-m) $(lib-target)),)
> builtin-target := $(obj)/built-in.o
> endif
>
> We still have more cases not covered by the above, so we need to add
> obj- := dummy.o
> in several places just for creating empty built-in.o.
>
> A key point is, the parent Makefile knows whether built-in.o is needed
> or not. If a subdirectory needs to create built-in.o, its parent can
> tell the fact when Kbuild descends into it.
>
> If non-empty $(need-builtin) flag is passed from the parent, built-in.o
> should be created. $(obj-y) should be still checked to support the
> single target "%/". All of ugly tricks will go away.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
Applied to linux-kbuild/kbuild.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 2/2] kbuild: remove all dummy assignments to obj-
From: Masahiro Yamada @ 2017-11-18 4:08 UTC (permalink / raw)
To: Linux Kbuild mailing list
Cc: Rob Herring, Mark Rutland, Pantelis Antoniou, devicetree,
Arnd Bergmann, Linux-MIPS, Linux Kernel Mailing List,
Michal Marek, Masahiro Yamada, Ralf Baechle, Alexei Starovoitov,
netdev, Russell King, Daniel Borkmann, Sam Ravnborg,
linux-arm-kernel
In-Reply-To: <1510072307-16819-3-git-send-email-yamada.masahiro@socionext.com>
2017-11-08 1:31 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> Now kbuild core scripts create empty built-in.o where necessary.
> Remove "obj- := dummy.o" tricks.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
Applied to linux-kbuild/kbuild.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] dt-bindings: i2c: eeprom: Add Renesas R1EX24128
From: Wolfram Sang @ 2017-11-18 7:38 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring, Mark Rutland, Bartosz Golaszewski, devicetree,
linux-i2c, linux-renesas-soc
In-Reply-To: <1510839994-12473-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 372 bytes --]
On Thu, Nov 16, 2017 at 02:46:34PM +0100, Geert Uytterhoeven wrote:
> Document the compatible value for the Renesas R1EX24128ASAS0A two-wire
> serial interface EEPROM, so it can be used in DTS files without causing
> checkpatch warnings.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example
From: Jacek Anaszewski @ 2017-11-18 13:38 UTC (permalink / raw)
To: Rob Herring
Cc: Pavel Machek, Sakari Ailus, Sakari Ailus, linux-leds, linux-media,
devicetree, linux-kernel@vger.kernel.org, Dan Murphy
In-Reply-To: <4e69932a-18af-416a-b280-16236fc4c2b3@gmail.com>
Hi Rob,
Any thoughts on my analysis? Do you think that LED function naming
standardization would really make sense having the abundance
of LED functions categories present in the mainline dts files?
The list of standard LED function names would have to be
continuously extended as people would be adding new boards.
Limiting users to only existing set of LED functions wouldn't
be practical IMHO.
I'd propose only to modify 'label' property description in LED common
bindings, so that it would explicitly state that it should contain only
"colour:functon" segments. It would be driver's responsibility to add
"devicename:" prefix to the label and use the whole string for a LED
class device name. Some drivers do that already. All DT bindings, dts
files and LED class drivers that don't adhere to this rule would have
to be updated accordingly.
Regarding colour - the "devicename:colour:function" LED device naming
convention defined in Documentation/leds/leds-class.txt is around for
a long time. Since LED colour can vary from board two board and is
independent of a driver, we have to have a means for defining it in
DT. Would you see providing separate 'colour' DT property as a solution?
Best regards,
Jacek Anaszewski
On 09/23/2017 11:12 PM, Jacek Anaszewski wrote:
> On 09/22/2017 11:07 PM, Jacek Anaszewski wrote:
>> On 09/20/2017 10:53 PM, Rob Herring wrote:
>>> On Tue, Sep 19, 2017 at 11:01:02PM +0200, Jacek Anaszewski wrote:
>>>> Hi Pavel,
>>>>
>>>> On 09/18/2017 10:54 PM, Pavel Machek wrote:
>>>>> On Mon 2017-09-18 17:49:23, Sakari Ailus wrote:
>>>>>> Hi Pavel,
>>>>>>
>>>>>> On Mon, Sep 18, 2017 at 12:56:55PM +0200, Pavel Machek wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>>> Specify the exact label used if the label property is omitted in DT, as
>>>>>>>> well as use label in the example that conforms to LED device naming.
>>>>>>>>
>>>>>>>> @@ -69,11 +73,11 @@ Example
>>>>>>>> flash-max-microamp = <320000>;
>>>>>>>> led-max-microamp = <60000>;
>>>>>>>> ams,input-max-microamp = <1750000>;
>>>>>>>> - label = "as3645a:flash";
>>>>>>>> + label = "as3645a:white:flash";
>>>>>>>> };
>>>>>>>> indicator@1 {
>>>>>>>> reg = <0x1>;
>>>>>>>> led-max-microamp = <10000>;
>>>>>>>> - label = "as3645a:indicator";
>>>>>>>> + label = "as3645a:red:indicator";
>>>>>>>> };
>>>>>>>> };
>>>>>>>
>>>>>>> Ok, but userspace still has no chance to determine if this is flash
>>>>>>> from main camera or flash for front camera; todays smartphones have
>>>>>>> flashes on both cameras.
>>>>>>>
>>>>>>> So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
>>>>>>> ....?
>>>>>>
>>>>>> If there's just a single one in the device, could you use that?
>>>>>>
>>>>>> Even if we name this so for N9 (and N900), the application still would only
>>>>>> work with the two devices.
>>>>>
>>>>> Well, I'd plan to name it on other devices, too.
>>>>>
>>>>>> My suggestion would be to look for a flash LED, and perhaps the maximum
>>>>>> current as well. That should generally work better than assumptions on the
>>>>>> label.
>>>>>
>>>>> If you just look for flash LED, you don't know if it is front one or
>>>>> back one. Its true that if you have just one flash it is usually on
>>>>> the back camera, but you can't know if maybe driver is not available
>>>>> for the main flash.
>>>>>
>>>>> Lets get this right, please "main_camera_flash" is 12 bytes more than
>>>>> "flash", and it saves application logic.. more than 12 bytes, I'm sure.
>>>>
>>>> What you are trying to introduce is yet another level of LED class
>>>> device naming standard, one level below devicename:colour:function.
>>>> It seems you want also to come up with the set of standarized LED
>>>> function names. This would certainly have to be covered for consistency.
>>>
>>> I really dislike how this naming convention is used for label. label is
>>> supposed to be the phyically identifiable name. Having the devicename
>>> defeats that. Perhaps color, too. We'd be better off with a color
>>> property. It seems we're overloading the naming with too many things.
>>> Now we're adding device association.
>>
>> Regarding devicename - there is indeed inconsistency in the way how LED
>> DT bindings use label, as some of them use it for defining full LED
>> class device name, and the rest fill only colour and function, leaving
>> addition of a devicename to the driver.
>>
>> The problem is also in current definition of label in LED common
>> bindings documentation, which says:
>>
>> "It has to uniquely identify a device, i.e. no other LED class device
>> can be assigned the same label."
>>
>> In view of your above words this is not true, and we probably should
>> remove this sentence (it doesn't have DT maintainer ack btw).
>>
>>> I do want to see standard names though. On 96boards for example, there
>>> are defined LEDs and locations. The function on some are defined (e.g.
>>> WiFi/BT) and somewhat undefined on others (user{1-4}). I'd like to see
>>> the same label across all boards.
>>
>> Currently we have following LED functions (obtained with
>> grep label Documentation/devicetree/bindings/leds/* | sed
>> s'/^.*label/label/g' | awk -F"=" '{print $2}' | sed '/^$/d' | sed
>> s'/.*:\(.*\)";/\1/' | sed '/^\s\{1,\}/d' | sort -u)
>>
>> 0
>> 1
>> 2
>> 2g
>> 3
>> 4
>> 5
>> 6
>> 7
>> adsl
>> alarm
>> alive
>> aux
>> broadband
>> chrg
>> dsl
>> flash
>> green
>> indicator
>> inet
>> keypad
>> phone
>> power
>> red
>> sata
>> sata0
>> sata1
>> tel
>> tv
>> upgrading
>> usb
>> usr0
>> usr1
>> usr35
>> wan
>> white
>> wireless
>> wps
>> yellow
>>
>> By extracting numerical pattern names and replacing numbers with N
>> we're getting something like this:
>>
>> N
>> Ng
>> colour
>> adsl
>> alarm
>> alive
>> aux
>> broadband
>> chrg
>> dsl
>> flash
>> indicator
>> inet
>> keypad
>> phone
>> power
>> sataN
>> tel
>> tv
>> upgrading
>> usb
>> usrN
>> wan
>> wireless
>> wps
>>
>> Is this list something you'd like to see as a base of standard LED
>> functions? It seems that this list would have to be continuously
>> supplemented with new positions.
>>
>
> Even better option is to grep through all *.dts* files in the arch
> directory. A slightly modified command chain. which removes numerical
> postfixes (there are some not covered corner cases though)
>
> find arch -name "*.dts*" | xargs grep label | sed s'/^.*label/label/g' |
> awk -F"=" '{print $2}' | sed s'/.*:\(.*\)";/\1/' | sed '/^\s\{1,\}/d' |
> sed s'/\([^0-9]*\)\([0-9]*\)/\1/' | sed '/^$/d' | sort -u
>
> gives following 135 positions (~5 colours percolated due
> to some non-covered corner cases), and some of them don't belong
> to LED DT nodes unfortunately, as e.g. gpio-keys use also ":" as
> a delimiter in their labels, but the whole set gives reasonable
> overview I think:
>
> active
> activity_led
> adsl
> alarm
> alive
> all
> amber
> app
> aux
> backup
> backup_led
> bl
> blue
> bluetooth
> boot
> bottom
> brick-status
> bt
> CEL
> chrg
> COM
> copy
> core_module
> cpu
> D
> debug
> DIA
> disk
> disk_led
> down
> dsl
> enocean
> enter
> err
> error
> esata
> ethernet-status
> fail
> fault
> front
> func
> function
> g
> ghz
> ghz-1
> ghz-2
> gpio
> green
> gsm
> HD
> hdd
> hdderr
> health
> health_led
> heart
> heartbeat
> home
> inet
> info
> internet
> keypad
> L
> lan
> led
> ledb
> left
> l_hdd
> live
> logo
> microSD
> misc
> mmc
> nand
> network
> on
> orange
> os
> panel
> pmu_stat
> power
> power_led
> programming
> proximitysensor
> pulse
> pwr
> qss
> rebuild_led
> red
> r_hdd
> right
> router
> rs
> rx
> sata
> sata-l
> sata-r
> sd
> SD
> sleep
> standby
> stat
> state
> status
> Status
> sw
> sys
> system
> system-status
> tel
> top
> tv
> tx
> up
> usb
> USB
> usb_1
> usb_2
> usb_copy
> usb-port1
> usb-port2
> user
> USER
> usr
> wan
> white
> wifi
> wifi_ap
> wifi-status
> wireless
> wlan
> wlan_g
> wmode
> wps
> WPS
> yellow
>
^ permalink raw reply
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver
From: Jacek Anaszewski @ 2017-11-18 14:19 UTC (permalink / raw)
To: Dan Murphy, robh+dt, mark.rutland, rpurdie, pavel
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <20171115194203.13572-1-dmurphy@ti.com>
Hi Dan,
On 11/15/2017 08:42 PM, Dan Murphy wrote:
> This adds the devicetree bindings for the LM3692x
> I2C LED string driver.
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v3 - No changes
> v2 - No changes - https://patchwork.kernel.org/patch/10056677/
>
> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> new file mode 100644
> index 000000000000..cfef67bd4100
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> @@ -0,0 +1,28 @@
> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver
> +
> +The LM36922 is an ultra-compact, highly efficient,
> +two string white-LED driver designed for LCD display
> +backlighting.
> +
> +Required properties:
> + - compatible:
> + "ti,lm3692x"
> + - reg - I2C slave address
> +
> +Optional properties:
> + - label - Used for naming LEDs
> + - enable-gpio - gpio pin to enable/disable the device.
> + - supply - "vled" - LED supply
> +
> +Example:
> +
> +leds: leds@6 {
> + compatible = "ti,lm3692x";
> + reg = <0x36>;
> + label = "backlight_cluster";
Documentation/devicetree/bindings/leds/common.txt states that
each LED component has to be represented as a child node
(for consistency it should apply also to the arrangements with one
LED).
> + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> + vled-supply = <&vbatt>;
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Jacek Anaszewski @ 2017-11-18 14:19 UTC (permalink / raw)
To: Dan Murphy, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
pavel-+ZI9xUNit7I
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171115194203.13572-2-dmurphy-l0cyMroinI0@public.gmane.org>
Hi Dan,
Please find my comments below.
On 11/15/2017 08:42 PM, Dan Murphy wrote:
> Introducing the LM3692x Dual-String white LED driver.
>
> Data sheet is located
> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
>
> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
> ---
>
> v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/
>
> v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init
> routine, return on fault_check failure, updated brightness calculation and
> fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/
>
> drivers/leds/Kconfig | 7 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 398 insertions(+)
> create mode 100644 drivers/leds/leds-lm3692x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 318a28fd58fe..1363e77a7d5f 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -137,6 +137,13 @@ config LEDS_LM3642
> converter plus 1.5A constant current driver for a high-current
> white LED.
>
> +config LEDS_LM3692X
> + tristate "LED support for LM3692x Chips"
> + depends on LEDS_CLASS && I2C
> + select REGMAP_I2C
> + help
> + This option enables support for the TI LM3692x family
> + of LED drivers.
>
> config LEDS_LOCOMO
> tristate "LED Support for Locomo device"
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index a2a6b5a4f86d..987884a5b9a5 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o
> obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o
> obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
> obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
> +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
> new file mode 100644
> index 000000000000..9fa7f5a20466
> --- /dev/null
> +++ b/drivers/leds/leds-lm3692x.c
> @@ -0,0 +1,390 @@
> +/*
> + * TI lm3692x LED Driver
> + *
> + * Copyright (C) 2017 Texas Instruments
> + *
> + * Author: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * Data sheet is located
> + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/slab.h>
> +
> +#define LM3692X_LED_NAME "lm3692x_led"
> +
> +#define LM3692X_REV 0x0
> +#define LM3692X_RESET 0x1
> +#define LM3692X_EN 0x10
> +#define LM3692X_BRT_CTRL 0x11
> +#define LM3692X_PWM_CTRL 0x12
> +#define LM3692X_BOOST_CTRL 0x13
> +#define LM3692X_AUTO_FREQ_HI 0x15
> +#define LM3692X_AUTO_FREQ_LO 0x16
> +#define LM3692X_BL_ADJ_THRESH 0x17
> +#define LM3692X_BRT_LSB 0x18
> +#define LM3692X_BRT_MSB 0x19
> +#define LM3692X_FAULT_CTRL 0x1e
> +#define LM3692X_FAULT_FLAGS 0x1f
> +
> +#define LM3692X_SW_RESET BIT(0)
> +#define LM3692X_DEVICE_EN BIT(0)
> +#define LM3692X_LED1_EN BIT(1)
> +#define LM3692X_LED2_EN BIT(2)
> +
> +/* Brightness Control Bits */
> +#define LM3692X_BL_ADJ_POL BIT(0)
> +#define LM3692X_RAMP_RATE_125us 0x00
> +#define LM3692X_RAMP_RATE_250us BIT(1)
> +#define LM3692X_RAMP_RATE_500us BIT(2)
> +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2))
> +#define LM3692X_RAMP_RATE_2ms BIT(3)
> +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1))
> +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3))
> +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3))
> +#define LM3692X_RAMP_EN BIT(4)
> +#define LM3692X_BRHT_MODE_REG 0x00
> +#define LM3692X_BRHT_MODE_PWM BIT(5)
> +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6)
> +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6))
> +#define LM3692X_MAP_MODE_EXP BIT(7)
> +
> +/* PWM Register Bits */
> +#define LM3692X_PWM_FILTER_100 BIT(0)
> +#define LM3692X_PWM_FILTER_150 BIT(1)
> +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1))
> +#define LM3692X_PWM_HYSTER_1LSB BIT(2)
> +#define LM3692X_PWM_HYSTER_2LSB BIT(3)
> +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2))
> +#define LM3692X_PWM_HYSTER_4LSB BIT(4)
> +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2))
> +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3))
> +#define LM3692X_PWM_POLARITY BIT(5)
> +#define LM3692X_PWM_SAMP_4MHZ BIT(6)
> +#define LM3692X_PWM_SAMP_24MHZ BIT(7)
> +
> +/* Boost Control Bits */
> +#define LM3692X_OCP_PROT_1A BIT(0)
> +#define LM3692X_OCP_PROT_1_25A BIT(1)
> +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1))
> +#define LM3692X_OVP_21V BIT(2)
> +#define LM3692X_OVP_25V BIT(3)
> +#define LM3692X_OVP_29V (BIT(2) | BIT(3))
> +#define LM3692X_MIN_IND_22UH BIT(4)
> +#define LM3692X_BOOST_SW_1MHZ BIT(5)
> +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6)
> +
> +/* Fault Control Bits */
> +#define LM3692X_FAULT_CTRL_OVP BIT(0)
> +#define LM3692X_FAULT_CTRL_OCP BIT(1)
> +#define LM3692X_FAULT_CTRL_TSD BIT(2)
> +#define LM3692X_FAULT_CTRL_OPEN BIT(3)
> +
> +/* Fault Flag Bits */
> +#define LM3692X_FAULT_FLAG_OVP BIT(0)
> +#define LM3692X_FAULT_FLAG_OCP BIT(1)
> +#define LM3692X_FAULT_FLAG_TSD BIT(2)
> +#define LM3692X_FAULT_FLAG_SHRT BIT(3)
> +#define LM3692X_FAULT_FLAG_OPEN BIT(4)
> +
> +/**
> + * struct lm3692x_led -
> + * @lock - Lock for reading/writing the device
> + * @client - Pointer to the I2C client
> + * @led_dev - led class device pointer
> + * @regmap - Devices register map
> + * @enable_gpio - VDDIO/EN gpio to enable communication interface
> + * @regulator - LED supply regulator pointer
> + * @label - LED label
> + */
> +struct lm3692x_led {
> + struct mutex lock;
> + struct i2c_client *client;
> + struct led_classdev led_dev;
> + struct regmap *regmap;
> + struct gpio_desc *enable_gpio;
> + struct regulator *regulator;
> + const char *label;
> +};
> +
> +static const struct reg_default lm3692x_reg_defs[] = {
> + {LM3692X_EN, 0xf},
> + {LM3692X_BRT_CTRL, 0x61},
> + {LM3692X_PWM_CTRL, 0x73},
> + {LM3692X_BOOST_CTRL, 0x6f},
> + {LM3692X_AUTO_FREQ_HI, 0x0},
> + {LM3692X_AUTO_FREQ_LO, 0x0},
> + {LM3692X_BL_ADJ_THRESH, 0x0},
> + {LM3692X_BRT_LSB, 0x7},
> + {LM3692X_BRT_MSB, 0xff},
> + {LM3692X_FAULT_CTRL, 0x7},
> +};
> +
> +static const struct regmap_config lm3692x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = LM3692X_FAULT_FLAGS,
> + .reg_defaults = lm3692x_reg_defs,
> + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs),
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int lm3692x_fault_check(struct lm3692x_led *led)
> +{
> + int ret, fault;
> + unsigned int read_buf;
> +
> + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf);
> + if (ret)
> + return ret;
> +
> + fault = read_buf;
> +
> + if (fault)
> + dev_err(&led->client->dev, "Detected a fault 0x%X\n",
> + fault);
> +
> + return ret;
> +}
> +
> +static int lm3692x_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brt_val)
> +{
> + struct lm3692x_led *led =
> + container_of(led_cdev, struct lm3692x_led, led_dev);
> + int ret;
> + int led_brightness_lsb = (brt_val >> 5);
> +
> + mutex_lock(&led->lock);
> +
> + ret = lm3692x_fault_check(led);
> + if (ret) {
> + dev_err(&led->client->dev, "Cannot read/clear faults\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val);
> + if (ret) {
> + dev_err(&led->client->dev, "Cannot write MSB\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb);
> + if (ret) {
> + dev_err(&led->client->dev, "Cannot write LSB\n");
> + goto out;
> + }
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3692x_init(struct lm3692x_led *led)
> +{
> + int ret;
> +
> + if (led->regulator) {
> + ret = regulator_enable(led->regulator);
> + if (ret)
> + dev_err(&led->client->dev,
> + "Failed to enable regulator\n");
> + goto out;
> + }
> +
> + if (led->enable_gpio)
> + gpiod_direction_output(led->enable_gpio, 1);
> +
> + ret = lm3692x_fault_check(led);
> + if (ret) {
> + dev_err(&led->client->dev, "Cannot read/clear faults\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00);
> + if (ret) {
> + dev_err(&led->client->dev, "Fail writing BRT CTRL\n");
> + goto out;
> + }
> +
> + /*
> + * For glitch free operation, the following data should
> + * only be written while device enable bit is 0
> + * per Section 7.5.14 of the data sheet
> + */
> + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
> + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
> + if (ret) {
> + dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
> + LM3692X_BRHT_MODE_RAMP_MULTI |
> + LM3692X_BL_ADJ_POL |
> + LM3692X_RAMP_RATE_250us);
> + if (ret) {
> + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
> + if (ret) {
> + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00);
> + if (ret) {
> + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00);
> + if (ret) {
> + dev_err(&led->client->dev, "Failed programming BL Threshold\n");
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL,
> + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB);
> + if (ret)
> + dev_err(&led->client->dev, "Failed programming BRT CTRL\n");
> +
> + return ret;
> +out:
> +
> + if (led->enable_gpio)
> + gpiod_direction_output(led->enable_gpio, 0);
> +
> + if (led->regulator) {
> + ret = regulator_disable(led->regulator);
> + if (ret)
> + dev_err(&led->client->dev,
> + "Failed to disable regulator\n");
> + }
> +
> + return ret;
> +}
> +
> +static int lm3692x_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret;
> + struct lm3692x_led *led;
> + struct device_node *np = client->dev.of_node;
> +
> + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + if (client->dev.of_node) {
We should fail in the opposite case.
> + ret = of_property_read_string(np, "label", &led->label);
> + if (ret)
> + led->label = LM3692X_LED_NAME;
LED label should be taken from the child node name if label property is
not present. See Documentation/devicetree/bindings/leds/common.txt.
We have some mess in this area, but I'm going to sort it out soon
(added you on CC to the related discussion).
Generally LED class device name should be composed by the LED class
driver by taking label and prefixing it with "[devicename]:" string.
> + }
> +
> + led->enable_gpio = devm_gpiod_get_optional(&client->dev,
> + "enable", GPIOD_OUT_LOW);
> + if (IS_ERR(led->enable_gpio)) {
> + ret = PTR_ERR(led->enable_gpio);
> + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret);
> + return ret;
> + }
> +
> + led->regulator = devm_regulator_get(&client->dev, "vled");
> + if (IS_ERR(led->regulator))
> + led->regulator = NULL;
> +
> + led->client = client;
> + led->led_dev.name = led->label;
> + led->led_dev.max_brightness = LED_FULL;
You can skip this line, it will be initialized to LED_FULL
in of_led_classdev_register in this case.
> + led->led_dev.brightness_set_blocking = lm3692x_brightness_set;
> +
> + mutex_init(&led->lock);
> +
> + i2c_set_clientdata(client, led);
> +
> + led->regmap = devm_regmap_init_i2c(client, &lm3692x_regmap_config);
> + if (IS_ERR(led->regmap)) {
> + ret = PTR_ERR(led->regmap);
> + dev_err(&client->dev, "Failed to allocate register map: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = lm3692x_init(led);
> + if (ret)
> + return ret;
> +
> + ret = led_classdev_register(&client->dev, &led->led_dev);
Please use devm_* prefixed version.
> + if (ret) {
> + dev_err(&client->dev, "led register err: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int lm3692x_remove(struct i2c_client *client)
> +{
> + struct lm3692x_led *led = i2c_get_clientdata(client);
> + int ret;
> +
> + led_classdev_unregister(&led->led_dev);
> +
> + if (led->enable_gpio)
> + gpiod_direction_output(led->enable_gpio, 0);
> +
> + if (led->regulator) {
> + ret = regulator_disable(led->regulator);
> + if (ret)
> + dev_err(&led->client->dev,
> + "Failed to disable regulator\n");
> + }
mutex_destroy(&led->lock)
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id lm3692x_id[] = {
> + { "lm3692x", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, lm3692x_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id of_lm3692x_leds_match[] = {
> + { .compatible = "ti,lm3692x", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match);
> +#endif
> +
> +static struct i2c_driver lm3692x_driver = {
> + .driver = {
> + .name = "lm3692x",
> + .of_match_table = of_match_ptr(of_lm3692x_leds_match),
> + },
> + .probe = lm3692x_probe,
> + .remove = lm3692x_remove,
> + .id_table = lm3692x_id,
> +};
> +module_i2c_driver(lm3692x_driver);
> +
> +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver");
> +MODULE_AUTHOR("Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>");
> +MODULE_LICENSE("GPL");
According to your license info it should be "GPL v2"
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] pwm: imx: Allow keeping the PWM active during suspend
From: Fabio Estevam @ 2017-11-18 14:36 UTC (permalink / raw)
To: thierry.reding; +Cc: robh+dt, linux, linux-pwm, devicetree, Fabio Estevam
In some systems it is desirable to keep the PWM active during system
suspend.
One use case is the imx6q-cubox-i board, which has an LED driven by PWM.
When the system goes into suspend the PWM block is disabled by default,
the PWM pin goes to zero and turn on the LED during suspend, which is
not really the behaviour we want to see.
By keeping the PWM enabled during suspend the pwm-leds driver sets
the brightness to zero in suspend and then the LED is turned off as
expected.
Introduce the 'fsl,active-in-suspend' property to indicate that the
PWM will stay active during suspend.
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Changes since v1:
- Rename the property to 'fsl,active-in-suspend' to better describe
its purpose.
Documentation/devicetree/bindings/pwm/imx-pwm.txt | 5 +++++
drivers/pwm/pwm-imx.c | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
index c61bdf8..ec2a84c 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -14,6 +14,11 @@ See the clock consumer binding,
Documentation/devicetree/bindings/clock/clock-bindings.txt
- interrupts: The interrupt for the pwm controller
+Optional properties:
+- fsl,active-in-suspend: Boolean property that indicates that the PWM block
+ stays active during system suspend. If not present,
+ the PWM block stays inactive during suspend.
+
Example:
pwm1: pwm@53fb4000 {
diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 2ba5c3a..afa1b20 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -35,6 +35,7 @@
#define MX3_PWMSAR 0x0C /* PWM Sample Register */
#define MX3_PWMPR 0x10 /* PWM Period Register */
#define MX3_PWMCR_PRESCALER(x) ((((x) - 1) & 0xFFF) << 4)
+#define MX3_PWMCR_STOPEN (1 << 25)
#define MX3_PWMCR_DOZEEN (1 << 24)
#define MX3_PWMCR_WAITEN (1 << 23)
#define MX3_PWMCR_DBGEN (1 << 22)
@@ -54,6 +55,7 @@ struct imx_chip {
void __iomem *mmio_base;
struct pwm_chip chip;
+ bool active;
};
#define to_imx_chip(chip) container_of(chip, struct imx_chip, chip)
@@ -217,6 +219,9 @@ static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
if (state->polarity == PWM_POLARITY_INVERSED)
cr |= MX3_PWMCR_POUTC;
+ if (imx->active)
+ cr |= MX3_PWMCR_STOPEN;
+
writel(cr, imx->mmio_base + MX3_PWMCR);
} else if (cstate.enabled) {
writel(0, imx->mmio_base + MX3_PWMCR);
@@ -264,6 +269,7 @@ static int imx_pwm_probe(struct platform_device *pdev)
{
const struct of_device_id *of_id =
of_match_device(imx_pwm_dt_ids, &pdev->dev);
+ struct device_node *np = pdev->dev.of_node;
const struct imx_pwm_data *data;
struct imx_chip *imx;
struct resource *r;
@@ -301,6 +307,8 @@ static int imx_pwm_probe(struct platform_device *pdev)
if (IS_ERR(imx->mmio_base))
return PTR_ERR(imx->mmio_base);
+ imx->active = of_property_read_bool(np, "fsl,active-in-suspend");
+
ret = pwmchip_add(&imx->chip);
if (ret < 0)
return ret;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 1/4] dt-bindings: iio: at91-sama5d2_adc: add optional dma property
From: Jonathan Cameron @ 2017-11-18 16:07 UTC (permalink / raw)
To: Rob Herring
Cc: Eugen Hristev, nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA, lars-Qo5EllUWu/uELgA04lAiVw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA
In-Reply-To: <20171115152746.w4giwt7kdqm4i3yl@rob-hp-laptop>
On Wed, 15 Nov 2017 09:27:46 -0600
Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Nov 15, 2017 at 02:56:45PM +0200, Eugen Hristev wrote:
> > Added property for DMA configuration of the device.
> >
> > Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> > ---
> > Changes in v3:
> > None, but we discussed on the ML about whether we should have "dma-names"
> > present in the binding even if it's only one.
> > The helpers in the kernel to retrieve the channel info rely on the
> > presence of this property, so I am resending the patch based on this.
> > If another solution is better, please advise and I can try it and
> > resend the patch.
>
> Really the kernel APIs should accept a NULL name and return the DMA
> channel when there is only one. This is how the clk_get API works for
> example.
>
> > Documentation/devicetree/bindings/iio/adc/at91-sama5d2_adc.txt | 7 +++++++
> > 1 file changed, 7 insertions(+)
>
> In any case, not really a big deal.
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" 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 v3 3/4] iio: adc: at91-sama5d2_adc: add support for DMA
From: Jonathan Cameron @ 2017-11-18 16:07 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA, lars-Qo5EllUWu/uELgA04lAiVw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA
In-Reply-To: <1510750608-8697-4-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Wed, 15 Nov 2017 14:56:47 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> Added support for DMA transfers. The implementation uses the user watermark
> to decide whether DMA will be used or not. For watermark 1, DMA will not be
> used. If watermark is bigger, DMA will be used.
> Sysfs attributes are created to indicate whether the DMA is used,
> with hwfifo_enabled, and the current DMA watermark is readable
> in hwfifo_watermark. Minimum and maximum values are in hwfifo_watermark_min
> and hwfifo_watermark_max.
>
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
A tiny nitpick inline but otherwise looks fine to me.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> Changes in v3:
> - Remove misleaded dev_info message when DMA was not enabled at probe
> - Rebased patch on top of the
> [PATCH] iio: adc: at91-sama5d2_adc: fix probe error on missing trigger property
> Which is already upstreamed in 4.14
> - Fixed the bug introduced in v2, with buffer size
> - added extra check when enabling DMA, to have hw trigger present.
> This is because now, we can have the driver with software trigger only (if no
> hw trigger in device tree, start as software only)
>
> Changes in v2:
> - No longer add last timestamp to all samples. Now, compute an interval
> between samples w.r.t. start and end time of the transfer and number
> of samples. Then distribute them each in the time interval.
> - Add warning for conversion overrun. This helps user identify cases
> when the watermark needs adjustment : the software is too slow in reading
> data from the ADC.
> - Protection around watermark is not needed, changing of the watermark
> cannot be done while the buffer is enabled. When buffer is disabled, all
> DMA resources are freed anyway.
> - Added validation on trigger to be used by own device
> - Best sample rate I could obtain using the low frequency clock was about
> 4k samples/second, with a watermark of 100. To get up to 50k samples/second
> the ADC frequency must be increased to max.
> - Fixed computation of DMA buffer size
> - Addressed other comments from mailing list review. Feedback is appreciated
>
> drivers/iio/adc/Kconfig | 1 +
> drivers/iio/adc/at91-sama5d2_adc.c | 453 +++++++++++++++++++++++++++++++++++--
> 2 files changed, 434 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 1d13bf0..1a3a8e3 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -158,6 +158,7 @@ config AT91_SAMA5D2_ADC
> tristate "Atmel AT91 SAMA5D2 ADC"
> depends on ARCH_AT91 || COMPILE_TEST
> depends on HAS_IOMEM
> + depends on HAS_DMA
> select IIO_TRIGGERED_BUFFER
> help
> Say yes here to build support for Atmel SAMA5D2 ADC which is
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index a70ef7f..11d34a8 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -16,6 +16,8 @@
>
> #include <linux/bitops.h>
> #include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/module.h>
> @@ -100,6 +102,8 @@
> #define AT91_SAMA5D2_LCDR 0x20
> /* Interrupt Enable Register */
> #define AT91_SAMA5D2_IER 0x24
> +/* Interrupt Enable Register - general overrun error */
> +#define AT91_SAMA5D2_IER_GOVRE BIT(25)
> /* Interrupt Disable Register */
> #define AT91_SAMA5D2_IDR 0x28
> /* Interrupt Mask Register */
> @@ -167,13 +171,19 @@
>
> /*
> * Maximum number of bytes to hold conversion from all channels
> - * plus the timestamp
> + * without the timestamp.
> */
> -#define AT91_BUFFER_MAX_BYTES ((AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> - AT91_SAMA5D2_DIFF_CHAN_CNT) * 2 + 8)
> +#define AT91_BUFFER_MAX_CONVERSION_BYTES ((AT91_SAMA5D2_SINGLE_CHAN_CNT + \
> + AT91_SAMA5D2_DIFF_CHAN_CNT) * 2)
> +
> +/* This total must also include the timestamp */
> +#define AT91_BUFFER_MAX_BYTES (AT91_BUFFER_MAX_CONVERSION_BYTES + 8)
>
> #define AT91_BUFFER_MAX_HWORDS (AT91_BUFFER_MAX_BYTES / 2)
>
> +#define AT91_HWFIFO_MAX_SIZE_STR "128"
> +#define AT91_HWFIFO_MAX_SIZE 128
> +
> #define AT91_SAMA5D2_CHAN_SINGLE(num, addr) \
> { \
> .type = IIO_VOLTAGE, \
> @@ -228,6 +238,28 @@ struct at91_adc_trigger {
> bool hw_trig;
> };
>
> +/**
> + * at91_adc_dma - at91-sama5d2 dma information struct
> + * @dma_chan: the dma channel acquired
> + * @rx_buf: dma coherent allocated area
> + * @rx_dma_buf: dma handler for the buffer
> + * @phys_addr: physical address of the ADC base register
> + * @buf_idx: index inside the dma buffer where reading was last done
> + * @rx_buf_sz: size of buffer used by DMA operation
> + * @watermark: number of conversions to copy before DMA triggers irq
> + * @dma_ts: hold the start timestamp of dma operation
> + */
> +struct at91_adc_dma {
> + struct dma_chan *dma_chan;
> + u8 *rx_buf;
> + dma_addr_t rx_dma_buf;
> + phys_addr_t phys_addr;
> + int buf_idx;
> + int rx_buf_sz;
> + int watermark;
> + s64 dma_ts;
> +};
> +
> struct at91_adc_state {
> void __iomem *base;
> int irq;
> @@ -242,6 +274,7 @@ struct at91_adc_state {
> u32 conversion_value;
> struct at91_adc_soc_info soc_info;
> wait_queue_head_t wq_data_available;
> + struct at91_adc_dma dma_st;
> u16 buffer[AT91_BUFFER_MAX_HWORDS];
> /*
> * lock to prevent concurrent 'single conversion' requests through
> @@ -322,11 +355,17 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
> if (state) {
> at91_adc_writel(st, AT91_SAMA5D2_CHER,
> BIT(chan->channel));
> - at91_adc_writel(st, AT91_SAMA5D2_IER,
> - BIT(chan->channel));
> + /* enable irq only if not using DMA */
> + if (!st->dma_st.dma_chan) {
> + at91_adc_writel(st, AT91_SAMA5D2_IER,
> + BIT(chan->channel));
> + }
> } else {
> - at91_adc_writel(st, AT91_SAMA5D2_IDR,
> - BIT(chan->channel));
> + /* disable irq only if not using DMA */
> + if (!st->dma_st.dma_chan) {
> + at91_adc_writel(st, AT91_SAMA5D2_IDR,
> + BIT(chan->channel));
> + }
> at91_adc_writel(st, AT91_SAMA5D2_CHDR,
> BIT(chan->channel));
> }
> @@ -340,6 +379,10 @@ static int at91_adc_reenable_trigger(struct iio_trigger *trig)
> struct iio_dev *indio = iio_trigger_get_drvdata(trig);
> struct at91_adc_state *st = iio_priv(indio);
>
> + /* if we are using DMA, we must not reenable irq after each trigger */
> + if (st->dma_st.dma_chan)
> + return 0;
> +
> enable_irq(st->irq);
>
> /* Needed to ACK the DRDY interruption */
> @@ -351,6 +394,153 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
> .owner = THIS_MODULE,
> .set_trigger_state = &at91_adc_configure_trigger,
> .try_reenable = &at91_adc_reenable_trigger,
> + .validate_device = iio_trigger_validate_own_device,
> +};
> +
> +static int at91_adc_dma_size_done(struct at91_adc_state *st)
> +{
> + struct dma_tx_state state;
> + enum dma_status status;
> + int i, size;
> +
> + status = dmaengine_tx_status(st->dma_st.dma_chan,
> + st->dma_st.dma_chan->cookie,
> + &state);
> + if (status != DMA_IN_PROGRESS)
> + return 0;
> +
> + /* Transferred length is size in bytes from end of buffer */
> + i = st->dma_st.rx_buf_sz - state.residue;
> +
> + /* Return available bytes */
> + if (i >= st->dma_st.buf_idx)
> + size = i - st->dma_st.buf_idx;
> + else
> + size = st->dma_st.rx_buf_sz + i - st->dma_st.buf_idx;
> + return size;
> +}
> +
> +static void at91_dma_buffer_done(void *data)
> +{
> + struct iio_dev *indio_dev = data;
> +
> + iio_trigger_poll_chained(indio_dev->trig);
> +}
> +
> +static int at91_adc_dma_start(struct iio_dev *indio_dev)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + struct dma_async_tx_descriptor *desc;
> + dma_cookie_t cookie;
> + int ret;
> + u8 bit;
> +
> + if (!st->dma_st.dma_chan)
> + return 0;
> +
> + /* we start a new DMA, so set buffer index to start */
> + st->dma_st.buf_idx = 0;
> +
> + /*
> + * compute buffer size w.r.t. watermark and enabled channels.
> + * scan_bytes is aligned so we need an exact size for DMA
> + */
> + st->dma_st.rx_buf_sz = 0;
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->num_channels) {
> + struct iio_chan_spec const *chan = indio_dev->channels + bit;
> +
> + st->dma_st.rx_buf_sz += chan->scan_type.storagebits / 8;
> + }
> + st->dma_st.rx_buf_sz *= st->dma_st.watermark;
> +
> + /* Prepare a DMA cyclic transaction */
> + desc = dmaengine_prep_dma_cyclic(st->dma_st.dma_chan,
> + st->dma_st.rx_dma_buf,
> + st->dma_st.rx_buf_sz,
> + st->dma_st.rx_buf_sz / 2,
> + DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
> +
> + if (!desc) {
> + dev_err(&indio_dev->dev, "cannot prepare DMA cyclic\n");
> + return -EBUSY;
> + }
> +
> + desc->callback = at91_dma_buffer_done;
> + desc->callback_param = indio_dev;
> +
> + cookie = dmaengine_submit(desc);
> + ret = dma_submit_error(cookie);
> + if (ret) {
> + dev_err(&indio_dev->dev, "cannot submit DMA cyclic\n");
> + dmaengine_terminate_async(st->dma_st.dma_chan);
> + return ret;
> + }
> +
> + /* enable general overrun error signaling */
> + at91_adc_writel(st, AT91_SAMA5D2_IER, AT91_SAMA5D2_IER_GOVRE);
> + /* Issue pending DMA requests */
> + dma_async_issue_pending(st->dma_st.dma_chan);
> +
> + /* consider current time as DMA start time for timestamps */
> + st->dma_st.dma_ts = iio_get_time_ns(indio_dev);
> +
> + dev_dbg(&indio_dev->dev, "DMA cyclic started\n");
> +
> + return 0;
> +}
> +
> +static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + int ret;
> +
> + ret = at91_adc_dma_start(indio_dev);
> + if (ret) {
> + dev_err(&indio_dev->dev, "buffer postenable failed\n");
> + return ret;
> + }
> +
> + return iio_triggered_buffer_postenable(indio_dev);
> +}
> +
> +static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + int ret;
> + u8 bit;
> +
> + ret = iio_triggered_buffer_predisable(indio_dev);
> + if (ret < 0)
> + dev_err(&indio_dev->dev, "buffer predisable failed\n");
> +
> + if (!st->dma_st.dma_chan)
> + return ret;
> +
> + /* if we are using DMA we must clear registers and end DMA */
> + dmaengine_terminate_sync(st->dma_st.dma_chan);
> +
> + /*
> + * For each enabled channel we must read the last converted value
> + * to clear EOC status and not get a possible interrupt later.
> + * This value is being read by DMA from LCDR anyway
> + */
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->num_channels) {
> + struct iio_chan_spec const *chan = indio_dev->channels + bit;
> +
> + if (st->dma_st.dma_chan)
> + at91_adc_readl(st, chan->address);
> + }
> +
> + /* read overflow register to clear possible overflow status */
> + at91_adc_readl(st, AT91_SAMA5D2_OVER);
> + return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
> + .postenable = &at91_adc_buffer_postenable,
> + .predisable = &at91_adc_buffer_predisable,
> };
>
> static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
> @@ -389,24 +579,77 @@ static int at91_adc_trigger_init(struct iio_dev *indio)
> return 0;
> }
>
> -static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
> +static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
> + struct iio_poll_func *pf)
> {
> - struct iio_poll_func *pf = p;
> - struct iio_dev *indio = pf->indio_dev;
> - struct at91_adc_state *st = iio_priv(indio);
> + struct at91_adc_state *st = iio_priv(indio_dev);
> int i = 0;
> u8 bit;
>
> - for_each_set_bit(bit, indio->active_scan_mask, indio->num_channels) {
> - struct iio_chan_spec const *chan = indio->channels + bit;
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->num_channels) {
> + struct iio_chan_spec const *chan = indio_dev->channels + bit;
>
> st->buffer[i] = at91_adc_readl(st, chan->address);
> i++;
> }
> + iio_push_to_buffers_with_timestamp(indio_dev, st->buffer,
> + pf->timestamp);
> +}
> +
> +static void at91_adc_trigger_handler_dma(struct iio_dev *indio_dev)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + int transferred_len = at91_adc_dma_size_done(st);
> + s64 ns = iio_get_time_ns(indio_dev);
> + s64 interval;
> + int sample_index = 0, sample_count, sample_size;
> +
> + u32 status = at91_adc_readl(st, AT91_SAMA5D2_ISR);
> + /* if we reached this point, we cannot sample faster */
> + if (status & AT91_SAMA5D2_IER_GOVRE)
> + pr_info_ratelimited("%s: conversion overrun detected\n",
> + indio_dev->name);
> +
> + sample_size = div_s64(st->dma_st.rx_buf_sz, st->dma_st.watermark);
> +
> + sample_count = div_s64(transferred_len, sample_size);
> +
> + /*
> + * interval between samples is total time since last transfer handling
> + * divided by the number of samples (total size divided by sample size)
> + */
> + interval = div_s64((ns - st->dma_st.dma_ts), sample_count);
> +
> + while (transferred_len >= sample_size) {
> + iio_push_to_buffers_with_timestamp(indio_dev,
> + (st->dma_st.rx_buf + st->dma_st.buf_idx),
> + (st->dma_st.dma_ts + interval * sample_index));
> + /* adjust remaining length */
> + transferred_len -= sample_size;
> + /* adjust buffer index */
> + st->dma_st.buf_idx += sample_size;
> + /* in case of reaching end of buffer, reset index */
> + if (st->dma_st.buf_idx >= st->dma_st.rx_buf_sz)
> + st->dma_st.buf_idx = 0;
> + sample_index++;
> + }
> + /* adjust saved time for next transfer handling */
> + st->dma_st.dma_ts = iio_get_time_ns(indio_dev);
> +}
> +
> +static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct at91_adc_state *st = iio_priv(indio_dev);
>
> - iio_push_to_buffers_with_timestamp(indio, st->buffer, pf->timestamp);
> + if (st->dma_st.dma_chan)
> + at91_adc_trigger_handler_dma(indio_dev);
> + else
> + at91_adc_trigger_handler_nodma(indio_dev, pf);
>
> - iio_trigger_notify_done(indio->trig);
> + iio_trigger_notify_done(indio_dev->trig);
>
> return IRQ_HANDLED;
> }
> @@ -415,7 +658,7 @@ static int at91_adc_buffer_init(struct iio_dev *indio)
> {
> return devm_iio_triggered_buffer_setup(&indio->dev, indio,
> &iio_pollfunc_store_time,
> - &at91_adc_trigger_handler, NULL);
> + &at91_adc_trigger_handler, &at91_buffer_setup_ops);
> }
>
> static unsigned at91_adc_startup_time(unsigned startup_time_min,
> @@ -486,10 +729,13 @@ static irqreturn_t at91_adc_interrupt(int irq, void *private)
> if (!(status & imr))
> return IRQ_NONE;
>
> - if (iio_buffer_enabled(indio)) {
> + if (iio_buffer_enabled(indio) && !st->dma_st.dma_chan) {
> disable_irq_nosync(irq);
> iio_trigger_poll(indio->trig);
> - } else {
> + } else if (iio_buffer_enabled(indio) && st->dma_st.dma_chan) {
> + disable_irq_nosync(irq);
> + WARN(true, "Unexpected irq occurred\n");
> + } else if (!iio_buffer_enabled(indio)) {
> st->conversion_value = at91_adc_readl(st, st->chan->address);
> st->conversion_done = true;
> wake_up_interruptible(&st->wq_data_available);
> @@ -511,7 +757,6 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
> ret = iio_device_claim_direct_mode(indio_dev);
> if (ret)
> return ret;
> -
Real nitpick, but this is an unrelated change... Should not be here.
> mutex_lock(&st->lock);
>
> st->chan = chan;
> @@ -581,9 +826,123 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev,
> return 0;
> }
>
> +static void at91_adc_dma_init(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + struct dma_slave_config config = {0};
> + /*
> + * We make the buffer double the size of the fifo,
> + * such that DMA uses one half of the buffer (full fifo size)
> + * and the software uses the other half to read/write.
> + */
> + unsigned int pages = DIV_ROUND_UP(AT91_HWFIFO_MAX_SIZE *
> + AT91_BUFFER_MAX_CONVERSION_BYTES * 2,
> + PAGE_SIZE);
> +
> + if (st->dma_st.dma_chan)
> + return;
> +
> + st->dma_st.dma_chan = dma_request_slave_channel(&pdev->dev, "rx");
> +
> + if (!st->dma_st.dma_chan) {
> + dev_info(&pdev->dev, "can't get DMA channel\n");
> + goto dma_exit;
> + }
> +
> + st->dma_st.rx_buf = dma_alloc_coherent(st->dma_st.dma_chan->device->dev,
> + pages * PAGE_SIZE,
> + &st->dma_st.rx_dma_buf,
> + GFP_KERNEL);
> + if (!st->dma_st.rx_buf) {
> + dev_info(&pdev->dev, "can't allocate coherent DMA area\n");
> + goto dma_chan_disable;
> + }
> +
> + /* Configure DMA channel to read data register */
> + config.direction = DMA_DEV_TO_MEM;
> + config.src_addr = (phys_addr_t)(st->dma_st.phys_addr
> + + AT91_SAMA5D2_LCDR);
> + config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + config.src_maxburst = 1;
> + config.dst_maxburst = 1;
> +
> + if (dmaengine_slave_config(st->dma_st.dma_chan, &config)) {
> + dev_info(&pdev->dev, "can't configure DMA slave\n");
> + goto dma_free_area;
> + }
> +
> + dev_info(&pdev->dev, "using %s for rx DMA transfers\n",
> + dma_chan_name(st->dma_st.dma_chan));
> +
> + return;
> +
> +dma_free_area:
> + dma_free_coherent(st->dma_st.dma_chan->device->dev, pages * PAGE_SIZE,
> + st->dma_st.rx_buf, st->dma_st.rx_dma_buf);
> +dma_chan_disable:
> + dma_release_channel(st->dma_st.dma_chan);
> + st->dma_st.dma_chan = 0;
> +dma_exit:
> + dev_info(&pdev->dev, "continuing without DMA support\n");
> +}
> +
> +static void at91_adc_dma_disable(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct at91_adc_state *st = iio_priv(indio_dev);
> + unsigned int pages = DIV_ROUND_UP(AT91_HWFIFO_MAX_SIZE *
> + AT91_BUFFER_MAX_CONVERSION_BYTES * 2,
> + PAGE_SIZE);
> +
> + /* if we are not using DMA, just return */
> + if (!st->dma_st.dma_chan)
> + return;
> +
> + /* wait for all transactions to be terminated first*/
> + dmaengine_terminate_sync(st->dma_st.dma_chan);
> +
> + dma_free_coherent(st->dma_st.dma_chan->device->dev, pages * PAGE_SIZE,
> + st->dma_st.rx_buf, st->dma_st.rx_dma_buf);
> + dma_release_channel(st->dma_st.dma_chan);
> + st->dma_st.dma_chan = 0;
> +
> + dev_info(&pdev->dev, "continuing without DMA support\n");
> +}
> +
> +static int at91_adc_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> +{
> + struct at91_adc_state *st = iio_priv(indio_dev);
> +
> + if (val > AT91_HWFIFO_MAX_SIZE)
> + return -EINVAL;
> +
> + if (!st->selected_trig->hw_trig) {
> + dev_dbg(&indio_dev->dev, "we need hw trigger for DMA\n");
> + return 0;
> + }
> +
> + dev_dbg(&indio_dev->dev, "new watermark is %u\n", val);
> + st->dma_st.watermark = val;
> +
> + /*
> + * The logic here is: if we have watermark 1, it means we do
> + * each conversion with it's own IRQ, thus we don't need DMA.
> + * If the watermark is higher, we do DMA to do all the transfers in bulk
> + */
> +
> + if (val == 1)
> + at91_adc_dma_disable(to_platform_device(&indio_dev->dev));
> + else if (val > 1)
> + at91_adc_dma_init(to_platform_device(&indio_dev->dev));
> +
> + return 0;
> +}
> +
> static const struct iio_info at91_adc_info = {
> .read_raw = &at91_adc_read_raw,
> .write_raw = &at91_adc_write_raw,
> + .hwfifo_set_watermark = &at91_adc_set_watermark,
> .driver_module = THIS_MODULE,
> };
>
> @@ -601,6 +960,42 @@ static void at91_adc_hw_init(struct at91_adc_state *st)
> at91_adc_setup_samp_freq(st, st->soc_info.min_sample_rate);
> }
>
> +static ssize_t at91_adc_get_fifo_state(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iio_dev *indio_dev =
> + platform_get_drvdata(to_platform_device(dev));
> + struct at91_adc_state *st = iio_priv(indio_dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", !!st->dma_st.dma_chan);
> +}
> +
> +static ssize_t at91_adc_get_watermark(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iio_dev *indio_dev =
> + platform_get_drvdata(to_platform_device(dev));
> + struct at91_adc_state *st = iio_priv(indio_dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", st->dma_st.watermark);
> +}
> +
> +static IIO_DEVICE_ATTR(hwfifo_enabled, 0444,
> + at91_adc_get_fifo_state, NULL, 0);
> +static IIO_DEVICE_ATTR(hwfifo_watermark, 0444,
> + at91_adc_get_watermark, NULL, 0);
> +
> +static IIO_CONST_ATTR(hwfifo_watermark_min, "2");
> +static IIO_CONST_ATTR(hwfifo_watermark_max, AT91_HWFIFO_MAX_SIZE_STR);
> +
> +static const struct attribute *at91_adc_fifo_attributes[] = {
> + &iio_const_attr_hwfifo_watermark_min.dev_attr.attr,
> + &iio_const_attr_hwfifo_watermark_max.dev_attr.attr,
> + &iio_dev_attr_hwfifo_watermark.dev_attr.attr,
> + &iio_dev_attr_hwfifo_enabled.dev_attr.attr,
> + NULL,
> +};
> +
> static int at91_adc_probe(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev;
> @@ -676,6 +1071,9 @@ static int at91_adc_probe(struct platform_device *pdev)
> if (!res)
> return -EINVAL;
>
> + /* if we plan to use DMA, we need the physical address of the regs */
> + st->dma_st.phys_addr = res->start;
> +
> st->base = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(st->base))
> return PTR_ERR(st->base);
> @@ -739,11 +1137,22 @@ static int at91_adc_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "couldn't setup the triggers.\n");
> goto per_clk_disable_unprepare;
> }
> + /*
> + * Initially the iio buffer has a length of 2 and
> + * a watermark of 1
> + */
> + st->dma_st.watermark = 1;
> +
> + iio_buffer_set_attrs(indio_dev->buffer,
> + at91_adc_fifo_attributes);
> }
>
> + if (dma_coerce_mask_and_coherent(&indio_dev->dev, DMA_BIT_MASK(32)))
> + dev_info(&pdev->dev, "cannot set DMA mask to 32-bit\n");
> +
> ret = iio_device_register(indio_dev);
> if (ret < 0)
> - goto per_clk_disable_unprepare;
> + goto dma_disable;
>
> if (st->selected_trig->hw_trig)
> dev_info(&pdev->dev, "setting up trigger as %s\n",
> @@ -754,6 +1163,8 @@ static int at91_adc_probe(struct platform_device *pdev)
>
> return 0;
>
> +dma_disable:
> + at91_adc_dma_disable(pdev);
> per_clk_disable_unprepare:
> clk_disable_unprepare(st->per_clk);
> vref_disable:
> @@ -770,6 +1181,8 @@ static int at91_adc_remove(struct platform_device *pdev)
>
> iio_device_unregister(indio_dev);
>
> + at91_adc_dma_disable(pdev);
> +
> clk_disable_unprepare(st->per_clk);
>
> regulator_disable(st->vref);
--
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: [v4,2/3] drivers: hwmon: Add W83773G driver
From: Guenter Roeck @ 2017-11-18 16:15 UTC (permalink / raw)
To: Lei YU
Cc: Mark Rutland, Jean Delvare, Jonathan Corbet, Jiri Kosina,
devicetree, linux-kernel, linux-hwmon, Joel Stanley,
Andrew Jeffery
In-Reply-To: <1510543655-27059-3-git-send-email-mine260309@gmail.com>
On Mon, Nov 13, 2017 at 11:27:33AM +0800, Lei YU wrote:
> Nuvoton W83773G is a hardware monitor IC providing one local
> temperature and two remote temperature sensors.
>
> Signed-off-by: Lei YU <mine260309@gmail.com>
Applied to hwmon-next.
Thanks,
Guenter
> ---
> v2:
> - Rewrite the driver using regmap
> - Add offset and update_interval
> v3:
> - Use devm_hwmon_device_register_with_info() with is_visible/read/write
> functions.
> v4:
> - Fix review comments.
> ---
> drivers/hwmon/Kconfig | 10 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/w83773g.c | 329 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 340 insertions(+)
> create mode 100644 drivers/hwmon/w83773g.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index d654314..11c6248 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1710,6 +1710,16 @@ config SENSORS_VT8231
> This driver can also be built as a module. If so, the module
> will be called vt8231.
>
> +config SENSORS_W83773G
> + tristate "Nuvoton W83773G"
> + depends on I2C
> + help
> + If you say yes here you get support for the Nuvoton W83773G hardware
> + monitoring chip.
> +
> + This driver can also be built as a module. If so, the module
> + will be called w83773g.
> +
> config SENSORS_W83781D
> tristate "Winbond W83781D, W83782D, W83783S, Asus AS99127F"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index c84d978..0649ad8 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SENSORS_ATK0110) += asus_atk0110.o
> # asb100, then w83781d go first, as they can override other drivers' addresses.
> obj-$(CONFIG_SENSORS_ASB100) += asb100.o
> obj-$(CONFIG_SENSORS_W83627HF) += w83627hf.o
> +obj-$(CONFIG_SENSORS_W83773G) += w83773g.o
> obj-$(CONFIG_SENSORS_W83792D) += w83792d.o
> obj-$(CONFIG_SENSORS_W83793) += w83793.o
> obj-$(CONFIG_SENSORS_W83795) += w83795.o
> diff --git a/drivers/hwmon/w83773g.c b/drivers/hwmon/w83773g.c
> new file mode 100644
> index 0000000..0b97c28
> --- /dev/null
> +++ b/drivers/hwmon/w83773g.c
> @@ -0,0 +1,329 @@
> +/*
> + * Copyright (C) 2017 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Driver for the Nuvoton W83773G SMBus temperature sensor IC.
> + * Supported models: W83773G
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +/* W83773 has 3 channels */
> +#define W83773_CHANNELS 3
> +
> +/* The W83773 registers */
> +#define W83773_CONVERSION_RATE_REG_READ 0x04
> +#define W83773_CONVERSION_RATE_REG_WRITE 0x0A
> +#define W83773_MANUFACTURER_ID_REG 0xFE
> +#define W83773_LOCAL_TEMP 0x00
> +
> +static const u8 W83773_STATUS[2] = { 0x02, 0x17 };
> +
> +static const u8 W83773_TEMP_LSB[2] = { 0x10, 0x25 };
> +static const u8 W83773_TEMP_MSB[2] = { 0x01, 0x24 };
> +
> +static const u8 W83773_OFFSET_LSB[2] = { 0x12, 0x16 };
> +static const u8 W83773_OFFSET_MSB[2] = { 0x11, 0x15 };
> +
> +/* this is the number of sensors in the device */
> +static const struct i2c_device_id w83773_id[] = {
> + { "w83773g" },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, w83773_id);
> +
> +static const struct of_device_id w83773_of_match[] = {
> + {
> + .compatible = "nuvoton,w83773g"
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, w83773_of_match);
> +
> +static inline long temp_of_local(s8 reg)
> +{
> + return reg * 1000;
> +}
> +
> +static inline long temp_of_remote(s8 hb, u8 lb)
> +{
> + return (hb << 3 | lb >> 5) * 125;
> +}
> +
> +static int get_local_temp(struct regmap *regmap, long *val)
> +{
> + unsigned int regval;
> + int ret;
> +
> + ret = regmap_read(regmap, W83773_LOCAL_TEMP, ®val);
> + if (ret < 0)
> + return ret;
> +
> + *val = temp_of_local(regval);
> + return 0;
> +}
> +
> +static int get_remote_temp(struct regmap *regmap, int index, long *val)
> +{
> + unsigned int regval_high;
> + unsigned int regval_low;
> + int ret;
> +
> + ret = regmap_read(regmap, W83773_TEMP_MSB[index], ®val_high);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(regmap, W83773_TEMP_LSB[index], ®val_low);
> + if (ret < 0)
> + return ret;
> +
> + *val = temp_of_remote(regval_high, regval_low);
> + return 0;
> +}
> +
> +static int get_fault(struct regmap *regmap, int index, long *val)
> +{
> + unsigned int regval;
> + int ret;
> +
> + ret = regmap_read(regmap, W83773_STATUS[index], ®val);
> + if (ret < 0)
> + return ret;
> +
> + *val = (u8)regval & 0x04 >> 2;
> + return 0;
> +}
> +
> +static int get_offset(struct regmap *regmap, int index, long *val)
> +{
> + unsigned int regval_high;
> + unsigned int regval_low;
> + int ret;
> +
> + ret = regmap_read(regmap, W83773_OFFSET_MSB[index], ®val_high);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(regmap, W83773_OFFSET_LSB[index], ®val_low);
> + if (ret < 0)
> + return ret;
> +
> + *val = temp_of_remote(regval_high, regval_low);
> + return 0;
> +}
> +
> +static int set_offset(struct regmap *regmap, int index, long val)
> +{
> + int ret;
> + u8 high_byte;
> + u8 low_byte;
> +
> + val = clamp_val(val, -127825, 127825);
> + /* offset value equals to (high_byte << 3 | low_byte >> 5) * 125 */
> + val /= 125;
> + high_byte = val >> 3;
> + low_byte = (val & 0x07) << 5;
> +
> + ret = regmap_write(regmap, W83773_OFFSET_MSB[index], high_byte);
> + if (ret < 0)
> + return ret;
> +
> + return regmap_write(regmap, W83773_OFFSET_LSB[index], low_byte);
> +}
> +
> +static int get_update_interval(struct regmap *regmap, long *val)
> +{
> + unsigned int regval;
> + int ret;
> +
> + ret = regmap_read(regmap, W83773_CONVERSION_RATE_REG_READ, ®val);
> + if (ret < 0)
> + return ret;
> +
> + *val = 16000 >> regval;
> + return 0;
> +}
> +
> +static int set_update_interval(struct regmap *regmap, long val)
> +{
> + int rate;
> +
> + /*
> + * For valid rates, interval can be calculated as
> + * interval = (1 << (8 - rate)) * 62.5;
> + * Rounded rate is therefore
> + * rate = 8 - __fls(interval * 8 / (62.5 * 7));
> + * Use clamp_val() to avoid overflows, and to ensure valid input
> + * for __fls.
> + */
> + val = clamp_val(val, 62, 16000) * 10;
> + rate = 8 - __fls((val * 8 / (625 * 7)));
> + return regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, rate);
> +}
> +
> +static int w83773_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct regmap *regmap = dev_get_drvdata(dev);
> +
> + if (type == hwmon_chip) {
> + if (attr == hwmon_chip_update_interval)
> + return get_update_interval(regmap, val);
> + return -EOPNOTSUPP;
> + }
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + if (channel == 0)
> + return get_local_temp(regmap, val);
> + return get_remote_temp(regmap, channel - 1, val);
> + case hwmon_temp_fault:
> + return get_fault(regmap, channel - 1, val);
> + case hwmon_temp_offset:
> + return get_offset(regmap, channel - 1, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int w83773_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + struct regmap *regmap = dev_get_drvdata(dev);
> +
> + if (type == hwmon_chip && attr == hwmon_chip_update_interval)
> + return set_update_interval(regmap, val);
> +
> + if (type == hwmon_temp && attr == hwmon_temp_offset)
> + return set_offset(regmap, channel - 1, val);
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static umode_t w83773_is_visible(const void *data, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_chip:
> + switch (attr) {
> + case hwmon_chip_update_interval:
> + return 0644;
> + }
> + break;
> + case hwmon_temp:
> + switch (attr) {
> + case hwmon_temp_input:
> + case hwmon_temp_fault:
> + return 0444;
> + case hwmon_temp_offset:
> + return 0644;
> + }
> + break;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> +static const u32 w83773_chip_config[] = {
> + HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL,
> + 0
> +};
> +
> +static const struct hwmon_channel_info w83773_chip = {
> + .type = hwmon_chip,
> + .config = w83773_chip_config,
> +};
> +
> +static const u32 w83773_temp_config[] = {
> + HWMON_T_INPUT,
> + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET,
> + HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_OFFSET,
> + 0
> +};
> +
> +static const struct hwmon_channel_info w83773_temp = {
> + .type = hwmon_temp,
> + .config = w83773_temp_config,
> +};
> +
> +static const struct hwmon_channel_info *w83773_info[] = {
> + &w83773_chip,
> + &w83773_temp,
> + NULL
> +};
> +
> +static const struct hwmon_ops w83773_ops = {
> + .is_visible = w83773_is_visible,
> + .read = w83773_read,
> + .write = w83773_write,
> +};
> +
> +static const struct hwmon_chip_info w83773_chip_info = {
> + .ops = &w83773_ops,
> + .info = w83773_info,
> +};
> +
> +static const struct regmap_config w83773_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int w83773_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct device *hwmon_dev;
> + struct regmap *regmap;
> + int ret;
> +
> + regmap = devm_regmap_init_i2c(client, &w83773_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(dev, "failed to allocate register map\n");
> + return PTR_ERR(regmap);
> + }
> +
> + /* Set the conversion rate to 2 Hz */
> + ret = regmap_write(regmap, W83773_CONVERSION_RATE_REG_WRITE, 0x05);
> + if (ret < 0) {
> + dev_err(&client->dev, "error writing config rate register\n");
> + return ret;
> + }
> +
> + i2c_set_clientdata(client, regmap);
> +
> + hwmon_dev = devm_hwmon_device_register_with_info(dev,
> + client->name,
> + regmap,
> + &w83773_chip_info,
> + NULL);
> + return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +static struct i2c_driver w83773_driver = {
> + .class = I2C_CLASS_HWMON,
> + .driver = {
> + .name = "w83773g",
> + .of_match_table = of_match_ptr(w83773_of_match),
> + },
> + .probe = w83773_probe,
> + .id_table = w83773_id,
> +};
> +
> +module_i2c_driver(w83773_driver);
> +
> +MODULE_AUTHOR("Lei YU <mine260309@gmail.com>");
> +MODULE_DESCRIPTION("W83773G temperature sensor driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [v4,3/3] hwmon: (w83773g) Add documentation
From: Guenter Roeck @ 2017-11-18 16:16 UTC (permalink / raw)
To: Lei YU
Cc: Mark Rutland, Jean Delvare, Jonathan Corbet, Jiri Kosina,
devicetree, linux-kernel, linux-hwmon, Joel Stanley,
Andrew Jeffery
In-Reply-To: <1510543655-27059-4-git-send-email-mine260309@gmail.com>
On Mon, Nov 13, 2017 at 11:27:34AM +0800, Lei YU wrote:
> Add documentation for the w83773g driver.
>
> Signed-off-by: Lei YU <mine260309@gmail.com>
Applied to hwmon-next.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
From: Stefan Wahren @ 2017-11-18 20:21 UTC (permalink / raw)
To: John Youn, Eric Anholt, Minas Harutyunyan
Cc: Phil Elwell, linux-usb-u79uwXL29TY76Z2rM5mHXA, Florian Fainelli,
Rob Herring, Mark Rutland,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <410670D7E743164D87FA6160E7907A560113A3F8F0-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
Hi Minas,
> Minas Harutyunyan <Minas.Harutyunyan-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> hat am 17. November 2017 um 08:48 geschrieben:
>
>
> On 11/17/2017 2:35 AM, Stefan Wahren wrote:
> > Hi Eric,
> >
> >> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 31. Oktober 2017 um 09:43 geschrieben:
> >>
> >>
> >> Hi Eric,
> >>
> >>> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 31. Oktober 2017 um 01:40 geschrieben:
> >>>
> >>>
> >>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
> >>>
> >>>>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
> >>>>>
> >>>>>
> >>>>> In case the RPi Zero has at least a device connected to the OTG port
> >>>>> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
> >>>>> also reduced to 512 bytes. So fix this accordingly.
> >>>>>
> >>>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >>>>> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
> >>>>
> >>>> gentle ping ...
> >>>
> >>> I've tried to make sense of this a couple of times, but I don't get it:
> >>> why does EP 6/7 get reduced to 512 bytes in this case?
> >>
> >> i cannot give you an answer for this specific case.
> >>
> >> Since the dwc2 databook isn't public, i started a thread on linux-usb [1] about proper fifo size configuration. But i didn't get any reply.
> >>
> >> The problem here is there different contraints:
> >> * the sum of all fifo values must not exceed 3776 bytes
> >> * each slot have its individual upper limit (available in the BCM2835 datasheet)
> >>
> >> During my tests for OTG mode i missed the specific case above. Now my determined limits of 512 for EP 6 and 7 are contrary to the BCM2835 datasheet. Maybe the Synopsys guys have an answer?
> >>
> >> Btw the values in the downstream tree also violate the contraints.
> >>
> >> [1] - https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dusb_msg157200.html&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=O2ngUGajgx3MfHk4IQKp5iRlwOOpG62gkji4R2gE64k&s=8ClOBwWN5B69mL1TDjnN3l78JBrvd1YHunRdzei-xrA&e=
> >
> > still concerns about this patch, because it's not included in dt-fixes?
> >
>
> Hi Stefan,
>
> According BCM2835 datasheet Total Data FIFO RAM Depth is 4096. I assume
> that in datasheet assumed 4096 dwords, not a bytes. DFIFO depth in
> dwords stored GHWCFG3[31:16].
>
> So, for buffer DMA mode max space in dwords which available to allocate
> to FIFO's is 4096-8*2=4080, where 8 EP count including EP0 and 2 for
> both directions of EP's. Based on bcm283x-rpi-usb-otg.dtsi:
> g-rx-fifo-size + g-np-tx-fifo-size + g-tx-fifo-size[1..7] =
> 256+32+256+256+512+512+512+768+768 = 3872 < 4080.
> So, I don't see any reason to change EP 6,7 TxFIFO sizes.
that's a nice calculation, but not relevant in my case.
Let's step back to the original problem. If i try to boot the Raspberry Pi Zero (Linux 4.14, U-Boot 2017-07) in OTG mode with at least 1 USB device connected, i will get the following warnings:
[ 2.997476] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[6]=768
[ 3.015754] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[7]=768
These warnings doesn't appear if there are no devices connected during boot.
In order to understand how i came to the "real" constraints 3776 bytes and to the new values for EP 6,7 i add some debug:
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -462,6 +462,8 @@ static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
for (fifo = 1; fifo <= fifo_count; fifo++)
total += hsotg->params.g_tx_fifo_size[fifo];
+ dev_warn(hsotg->dev, "%s: g-tx-fifo-size: %d\n", __func__, dwc2_hsotg_tx_fifo_total_depth(hsotg));
+
if (total > dwc2_hsotg_tx_fifo_total_depth(hsotg) || !total) {
dev_warn(hsotg->dev, "%s: Invalid parameter g-tx-fifo-size, setting to default average\n",
__func__);
@@ -472,6 +474,8 @@ static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
dptxfszn = (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) &
FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
+ dev_warn(hsotg->dev, "%s: g_tx_fifo_size[%d]=%d\n", __func__, fifo, dptxfszn);
+
if (hsotg->params.g_tx_fifo_size[fifo] < min ||
hsotg->params.g_tx_fifo_size[fifo] > dptxfszn) {
dev_warn(hsotg->dev, "%s: Invalid parameter g_tx_fifo_size[%d]=%d\n",
After applying the patch i will get the following output if 1 device is connected during boot:
[ 2.939493] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g-tx-fifo-size: 3776
[ 2.947650] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[1]=512
[ 2.955972] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[2]=512
[ 2.964294] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[3]=512
[ 2.972630] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[4]=512
[ 2.980949] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[5]=512
[ 2.989174] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[6]=512
[ 2.997476] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[6]=768
[ 3.007439] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[7]=512
[ 3.015754] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[7]=768
[ 3.025763] dwc2 20980000.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
[ 3.034534] dwc2 20980000.usb: DWC OTG Controller
[ 3.039802] dwc2 20980000.usb: new USB bus registered, assigned bus number 1
[ 3.047399] dwc2 20980000.usb: irq 33, io mem 0x20980000
As can see from dwc2 perspective the sum of EP 1 - 7 must be 3776 bytes and the upper limits for EP 6 + 7 changed to 512 instead of 768. Datasheets are nice, but as long as they apply only in specific cases they aren't helpful.
This might be not the right fix, but as long as i didn't get any reply to my mails this is my best solution. Sorry but as BCM2835 maintainer the dwc2 driver is a real problem child to us.
Best regards
Stefan
>
> More probably comment in dtsi not correct:
> "* fifo sizes shouldn't exceed 3776 bytes."
> Please check FIFO depth in GHWCFG3 and if it 4096 then update the comment.
>
> Thanks,
> Minas
>
>
>
>
--
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] dt-bindings: trivial-devices: Remove fsl,mc13892
From: Fabio Estevam @ 2017-11-18 22:04 UTC (permalink / raw)
To: Jonathan Neuschäfer
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel,
Rob Herring, Mark Rutland
In-Reply-To: <20171118022232.16835-1-j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>
On Sat, Nov 18, 2017 at 12:22 AM, Jonathan Neuschäfer
<j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org> wrote:
> This device's bindings are not trivial: Additional properties are
> documented in in Documentation/devicetree/bindings/mfd/mc13xxx.txt.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer-hi6Y0CQ0nG0@public.gmane.org>
Reviewed-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 0/3] iio: Add Intersil isl76683 light sensor support
From: Christoph Fritz @ 2017-11-18 23:20 UTC (permalink / raw)
To: Jonathan Cameron, Peter Meerwald-Stadler, Rob Herring
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
This patchset adds initial support for isl76683 light sensor.
Changes since v1:
- introduce dt property suffix -kilo-ohms
- rename optinal dt property to isil,external-resistor-kilo-ohms
- purge dynamic buffer in favour of simple static 16 byte IIO_LE
- use iio_device_claim/release_direct_mode() instead of flag variable
- return with IRQ_HANDLED in all cases in isl76683_trigger_handler()
- simplify return in isl76683_set_config
- rename isl76683_read_lux() to isl76683_get_sensordata()
- use IIO_CHAN_INFO_SCALE instead of IIO_CHAN_INFO_HARDWAREGAIN
- rename macro ISL76683_REG_SENSOR_L/M to ISL76683_REG_SENSOR_LSB/MSB
- rename marcro ISL76683_CHANNEL_2 to ISL76683_CHANNEL
- use GENMASK for ISL76683_PHOTOD_MASK
Christoph Fritz (3):
iio: light: Add support for Intersil isl76683 sensor
dt-bindings: iio: add Intersil isl76683 light sensor bindings
iio: light: isl76683 add way to adjust irq threshold
.../ABI/testing/sysfs-bus-iio-light-isl76683 | 17 +
.../devicetree/bindings/iio/light/isl76683.txt | 26 +
.../devicetree/bindings/property-units.txt | 1 +
drivers/iio/light/Kconfig | 12 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/isl76683.c | 714 +++++++++++++++++++++
6 files changed, 771 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
create mode 100644 Documentation/devicetree/bindings/iio/light/isl76683.txt
create mode 100644 drivers/iio/light/isl76683.c
--
2.1.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
* [PATCH v2 1/3] iio: light: Add support for Intersil isl76683 sensor
From: Christoph Fritz @ 2017-11-18 23:20 UTC (permalink / raw)
To: Jonathan Cameron, Peter Meerwald-Stadler, Rob Herring
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1511047230-7021-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch adds support for Intersil isl76683 light sensor.
Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/iio/light/Kconfig | 12 +
drivers/iio/light/Makefile | 1 +
drivers/iio/light/isl76683.c | 663 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 676 insertions(+)
create mode 100644 drivers/iio/light/isl76683.c
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 2356ed9..4f0882c 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -194,6 +194,18 @@ config ISL29125
To compile this driver as a module, choose M here: the module will be
called isl29125.
+config ISL76683
+ tristate "Intersil ISL76683 light sensor"
+ depends on I2C
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say Y here if you want to build a driver for the Intersil ISL76683
+ light sensor for I2C.
+
+ To compile this driver as a module, choose M here: the module will be
+ called isl76683.
+
config HID_SENSOR_ALS
depends on HID_SENSOR_HUB
select IIO_BUFFER
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index fa32fa4..886a51f 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o
obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o
obj-$(CONFIG_ISL29125) += isl29125.o
+obj-$(CONFIG_ISL76683) += isl76683.o
obj-$(CONFIG_JSA1212) += jsa1212.o
obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
obj-$(CONFIG_LTR501) += ltr501.o
diff --git a/drivers/iio/light/isl76683.c b/drivers/iio/light/isl76683.c
new file mode 100644
index 0000000..4d158f6
--- /dev/null
+++ b/drivers/iio/light/isl76683.c
@@ -0,0 +1,663 @@
+/*
+ * IIO driver for the light sensor ISL76683.
+ *
+ * Copyright (c) 2017 Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * Datasheet:
+ * http://www.intersil.com/content/dam/Intersil/documents/isl7/isl76683.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/util_macros.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define ISL76683_REG_CMD 0x00
+#define ISL76683_REG_CTRL 0x01
+#define ISL76683_REG_THR_HI 0x02
+#define ISL76683_REG_THR_LO 0x03
+#define ISL76683_REG_SENSOR_LSB 0x04
+#define ISL76683_REG_SENSOR_MSB 0x05
+#define ISL76683_REG_CLEAR_INT 0x40
+#define ISL76683_REGMAP_MAX 0x40
+
+#define ISL76683_CMD_ENABLE BIT(7)
+#define ISL76683_CMD_PWRDWN BIT(6)
+#define ISL76683_WIDTH_MASK 0x3
+#define ISL76683_PHOTOD_SHFT 2
+#define ISL76683_PHOTOD_MASK GENMASK(3, ISL76683_PHOTOD_SHFT)
+#define ISL76683_INTPERS_MASK 0x3
+#define ISL76683_LUXRANGE_SHFT 2
+#define ISL76683_LUXRANGE_MASK (0x3 << ISL76683_LUXRANGE_SHFT)
+#define ISL76683_LUXRANGE_STR "1000 4000 16000 64000"
+
+enum isl76683_dmode {
+ ISL76683_DIODE_0 = 0,
+ ISL76683_DIODE_IR,
+ ISL76683_DIODE_DIFF,
+};
+
+enum isl76683_lux_range {
+ ISL76683_LUX_1000 = 0,
+ ISL76683_LUX_4000,
+ ISL76683_LUX_16000,
+ ISL76683_LUX_64000,
+};
+
+static const int isl76683_lux_ranges_available[] = {
+ 1000, 4000, 16000, 64000};
+
+#define ISL76683_LUX_RANGE_DEFAULT ISL76683_LUX_1000
+#define ISL76683_DIODE_MAX ISL76683_DIODE_DIFF
+#define ISL76683_DIODE_DEFAULT ISL76683_DIODE_0
+#define ISL76683_WIDTH_DEFAULT 0x0
+#define ISL76683_RESOLUTION_DEFAULT 16
+#define ISL76683_EXT_RESISTOR_DEFAULT 100
+#define ISL76683_KOHM_MIN 1
+#define ISL76683_KOHM_MAX 1000
+#define ISL76683_INTPERS_DEFAULT 0x0
+#define ISL76683_THR_DEFAULT 0x7f
+
+struct isl76683_chip {
+ enum isl76683_lux_range luxrange;
+ int external_resistor;
+ enum isl76683_dmode photodiode;
+ struct i2c_client *client;
+ struct regmap *rmp;
+ struct completion irq_complete;
+ struct iio_trigger *trig;
+ bool trig_enabled;
+ struct mutex lock;
+ s64 time_ns;
+};
+
+static bool isl76683_readable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case ISL76683_REG_CMD:
+ case ISL76683_REG_CTRL:
+ case ISL76683_REG_THR_HI:
+ case ISL76683_REG_THR_LO:
+ case ISL76683_REG_SENSOR_LSB:
+ case ISL76683_REG_SENSOR_MSB:
+ case ISL76683_REG_CLEAR_INT:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool isl76683_writeable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case ISL76683_REG_CMD:
+ case ISL76683_REG_CTRL:
+ case ISL76683_REG_THR_HI:
+ case ISL76683_REG_THR_LO:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool isl76683_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case ISL76683_REG_SENSOR_LSB:
+ case ISL76683_REG_SENSOR_MSB:
+ case ISL76683_REG_CLEAR_INT:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static const struct regmap_config isl76683_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = ISL76683_REGMAP_MAX,
+ .readable_reg = isl76683_readable_reg,
+ .writeable_reg = isl76683_writeable_reg,
+ .volatile_reg = isl76683_is_volatile_reg,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int isl76683_set_config(struct isl76683_chip *chip)
+{
+ int ret;
+
+ ret = regmap_update_bits(chip->rmp, ISL76683_REG_CTRL,
+ ISL76683_LUXRANGE_MASK | ISL76683_INTPERS_MASK,
+ (chip->luxrange << ISL76683_LUXRANGE_SHFT) |
+ ISL76683_INTPERS_DEFAULT);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(chip->rmp, ISL76683_REG_CMD,
+ ISL76683_PHOTOD_MASK | ISL76683_WIDTH_MASK,
+ (chip->photodiode << ISL76683_PHOTOD_SHFT) |
+ ISL76683_WIDTH_DEFAULT);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(chip->rmp, ISL76683_REG_THR_HI,
+ ISL76683_THR_DEFAULT);
+ if (ret < 0)
+ return ret;
+
+ return regmap_write(chip->rmp, ISL76683_REG_THR_LO,
+ ISL76683_THR_DEFAULT);
+}
+
+static int isl76683_power(struct isl76683_chip *chip, bool on)
+{
+ int ret;
+
+ ret = regmap_update_bits(chip->rmp, ISL76683_REG_CMD,
+ ISL76683_CMD_ENABLE | ISL76683_CMD_PWRDWN,
+ 0x0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(chip->rmp, ISL76683_REG_CMD,
+ ISL76683_CMD_ENABLE | ISL76683_CMD_PWRDWN,
+ on ? ISL76683_CMD_ENABLE : ISL76683_CMD_PWRDWN);
+ if (ret < 0)
+ return ret;
+
+ return on ? isl76683_set_config(chip) : 0;
+}
+
+static int isl76683_reset(struct isl76683_chip *chip)
+{
+ int ret;
+
+ ret = isl76683_power(chip, false);
+ if (ret < 0)
+ return ret;
+
+ return isl76683_power(chip, true);
+}
+
+static int isl76683_get_sensordata(struct isl76683_chip *chip,
+ bool is_processed, int *val)
+{
+ unsigned int sensor_data, range, fsr;
+ __le16 sensor_raw;
+ int ret;
+
+ ret = regmap_bulk_read(chip->rmp, ISL76683_REG_SENSOR_LSB,
+ &sensor_raw, sizeof(sensor_raw));
+ if (ret)
+ return ret;
+
+ sensor_data = le16_to_cpu(sensor_raw);
+
+ if (!is_processed) {
+ *val = sensor_data;
+ return 0;
+ }
+
+ /* range values taken from datasheet (table 9) */
+ if (chip->luxrange == ISL76683_LUX_1000)
+ range = 973;
+ else if (chip->luxrange == ISL76683_LUX_4000)
+ range = 3892;
+ else if (chip->luxrange == ISL76683_LUX_16000)
+ range = 15568;
+ else if (chip->luxrange == ISL76683_LUX_64000)
+ range = 62272;
+ else
+ return -EINVAL;
+
+ /* equations from datasheet (EQ.3 and EQ.4) */
+ fsr = (100 * range) / chip->external_resistor;
+ *val = (fsr * sensor_data) / (1 << ISL76683_RESOLUTION_DEFAULT);
+
+ return 0;
+}
+
+static irqreturn_t isl76683_interrupt_handler(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct isl76683_chip *chip = iio_priv(indio_dev);
+
+ chip->time_ns = iio_get_time_ns(indio_dev);
+
+ if (chip->trig_enabled)
+ iio_trigger_poll(chip->trig);
+
+ if (!completion_done(&chip->irq_complete))
+ complete(&chip->irq_complete);
+
+ return IRQ_HANDLED;
+}
+
+static inline int isl76683_start_measurement(struct isl76683_chip *chip)
+{
+ int dummy;
+
+ /* dummy read is clearing irq and triggers new measurement */
+ return regmap_read(chip->rmp, ISL76683_REG_CLEAR_INT, &dummy);
+}
+
+static int isl76683_singleshot_conversion(struct isl76683_chip *chip,
+ const struct iio_chan_spec *chan,
+ bool is_processed, int *val)
+{
+ long timeout;
+ int ret;
+
+ switch (chan->channel2) {
+ case IIO_MOD_LIGHT_BOTH:
+ chip->photodiode = ISL76683_DIODE_DIFF;
+ break;
+ case IIO_MOD_LIGHT_IR:
+ chip->photodiode = ISL76683_DIODE_IR;
+ break;
+ default:
+ chip->photodiode = ISL76683_DIODE_0;
+ }
+
+ ret = isl76683_set_config(chip);
+ if (ret)
+ return ret;
+
+ ret = isl76683_start_measurement(chip);
+ if (ret < 0)
+ return ret;
+
+ /* wait for measurement to complete */
+ timeout = wait_for_completion_interruptible_timeout(
+ &chip->irq_complete,
+ msecs_to_jiffies(5000));
+ if (timeout == 0) {
+ dev_err(&chip->client->dev, "measurement timed out\n");
+ return -ETIMEDOUT;
+ } else if (timeout < 0) {
+ dev_err(&chip->client->dev, "wait for measurement failed\n");
+ return -EINTR;
+ }
+
+ ret = isl76683_get_sensordata(chip, is_processed, val);
+ if (ret) {
+ dev_err(&chip->client->dev, "%s: Error %d reading lux\n",
+ __func__, ret);
+ return ret;
+ }
+
+ return IIO_VAL_INT;
+}
+
+static irqreturn_t isl76683_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct isl76683_chip *chip = iio_priv(indio_dev);
+ u8 data[16] = { }; /* 2x 8-bit data + padding + 8 bytes timestamp */
+ __le16 buf;
+ int ret;
+
+ ret = regmap_bulk_read(chip->rmp, ISL76683_REG_SENSOR_LSB, &buf,
+ sizeof(buf));
+ if (ret)
+ return IRQ_HANDLED;
+
+ data[0] = buf;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, data, chip->time_ns);
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ ret = isl76683_start_measurement(chip);
+ if (ret < 0)
+ return IRQ_HANDLED;
+
+ return IRQ_HANDLED;
+}
+
+static int isl76683_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct isl76683_chip *chip = iio_priv(indio_dev);
+ int ret;
+
+ chip->photodiode = indio_dev->channels[0].channel2;
+ ret = isl76683_set_config(chip);
+
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops isl76683_buffer_setup_ops = {
+ .preenable = &isl76683_buffer_preenable,
+ .predisable = iio_triggered_buffer_predisable,
+ .postenable = iio_triggered_buffer_postenable,
+ .validate_scan_mask = &iio_validate_scan_mask_onehot,
+};
+
+static int isl76683_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct isl76683_chip *chip = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+ ret = isl76683_singleshot_conversion(chip, chan, false, val);
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ case IIO_CHAN_INFO_PROCESSED:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+ ret = isl76683_singleshot_conversion(chip, chan, true, val);
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ case IIO_CHAN_INFO_SCALE:
+ *val = isl76683_lux_ranges_available[chip->luxrange];
+ return IIO_VAL_INT;
+ }
+
+ return -EINVAL;
+}
+
+static int isl76683_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct isl76683_chip *chip = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ mutex_lock(&chip->lock);
+ chip->luxrange = find_closest(val,
+ isl76683_lux_ranges_available,
+ ARRAY_SIZE(isl76683_lux_ranges_available));
+ ret = isl76683_set_config(chip);
+ mutex_unlock(&chip->lock);
+ return ret;
+ }
+
+ return -EINVAL;
+}
+
+static IIO_CONST_ATTR(in_illuminance_scale_available,
+ ISL76683_LUXRANGE_STR);
+
+static struct attribute *isl76683_attributes[] = {
+ &iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group isl76683_attribute_group = {
+ .attrs = isl76683_attributes,
+};
+
+#define ISL76683_CHANNEL(_ch2, _si) { \
+ .type = IIO_LIGHT, \
+ .modified = 1, \
+ .channel2 = _ch2, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_PROCESSED), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_index = _si, \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 16, \
+ .storagebits = 16, \
+ .endianness = IIO_LE, \
+ }, \
+}
+
+static const struct iio_chan_spec isl76683_channels[] = {
+ {
+ .type = IIO_LIGHT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 16,
+ .storagebits = 16,
+ .endianness = IIO_LE,
+ },
+ },
+ ISL76683_CHANNEL(IIO_MOD_LIGHT_IR, 1),
+ ISL76683_CHANNEL(IIO_MOD_LIGHT_BOTH, 2),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const struct iio_info isl76683_info = {
+ .read_raw = isl76683_read_raw,
+ .write_raw = isl76683_write_raw,
+ .attrs = &isl76683_attribute_group,
+};
+
+static int isl76683_set_trigger_state(struct iio_trigger *trig, bool enable)
+{
+ struct isl76683_chip *chip = iio_trigger_get_drvdata(trig);
+ int ret;
+
+ if (enable) {
+ chip->trig_enabled = true;
+ ret = isl76683_start_measurement(chip);
+ if (ret < 0)
+ return ret;
+ } else
+ chip->trig_enabled = false;
+
+ return 0;
+}
+
+static const struct iio_trigger_ops isl76683_trigger_ops = {
+ .owner = THIS_MODULE,
+ .set_trigger_state = isl76683_set_trigger_state,
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static int isl76683_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct isl76683_chip *chip;
+ struct iio_dev *indio_dev;
+ struct device_node *np = client->dev.of_node;
+ int rs = ISL76683_EXT_RESISTOR_DEFAULT;
+ int v, ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ chip = iio_priv(indio_dev);
+
+ i2c_set_clientdata(client, indio_dev);
+ chip->client = client;
+
+ if (np) {
+ ret = of_property_read_u32(np,
+ "isil,external-resistor-kilo-ohms", &v);
+ if (ret || v < ISL76683_KOHM_MIN || v > ISL76683_KOHM_MAX)
+ dev_warn(&client->dev,
+ "assuming %i kOhm resistor\n", rs);
+ else
+ rs = v;
+ }
+
+ chip->luxrange = ISL76683_LUX_RANGE_DEFAULT;
+ chip->external_resistor = rs;
+ chip->photodiode = ISL76683_DIODE_DEFAULT;
+
+ chip->rmp = devm_regmap_init_i2c(client, &isl76683_regmap_config);
+ if (IS_ERR(chip->rmp)) {
+ ret = PTR_ERR(chip->rmp);
+ dev_err(&client->dev, "%s: Error %d initializing regmap\n",
+ __func__, ret);
+ return ret;
+ }
+
+ mutex_init(&chip->lock);
+
+ init_completion(&chip->irq_complete);
+
+ if (!client->irq) {
+ dev_err(&client->dev, "no interrupt configured\n");
+ return -EINVAL;
+ }
+
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->info = &isl76683_info;
+ indio_dev->channels = isl76683_channels;
+ indio_dev->num_channels = ARRAY_SIZE(isl76683_channels);
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ chip->trig_enabled = false;
+ chip->trig = devm_iio_trigger_alloc(&client->dev, "%s-dev%d",
+ indio_dev->name, indio_dev->id);
+ if (!chip->trig)
+ return -ENOMEM;
+
+ chip->trig->ops = &isl76683_trigger_ops;
+ chip->trig->dev.parent = &client->dev;
+ iio_trigger_set_drvdata(chip->trig, chip);
+
+ ret = devm_request_irq(&client->dev, client->irq,
+ isl76683_interrupt_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "isl76683_event", indio_dev);
+ if (ret) {
+ dev_err(&client->dev, "irq request error\n");
+ return ret;
+ }
+
+ ret = devm_iio_trigger_register(&client->dev, chip->trig);
+ if (ret) {
+ dev_err(&client->dev, "iio_trigger register error\n");
+ return ret;
+ }
+
+ ret = isl76683_reset(chip);
+ if (ret) {
+ dev_err(&client->dev, "reset failed\n");
+ return ret;
+ }
+
+ ret = isl76683_get_sensordata(chip, false, &v);
+ if (ret) {
+ dev_err(&client->dev, "initial dummy readout failed\n");
+ return ret;
+ }
+
+ ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
+ &isl76683_trigger_handler, &isl76683_buffer_setup_ops);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_device_register(&client->dev, indio_dev);
+ if (ret) {
+ dev_err(&client->dev,
+ "%s(): iio registration failed with error %d\n",
+ __func__, ret);
+ }
+
+ return ret;
+}
+
+static int isl76683_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct isl76683_chip *chip = iio_priv(indio_dev);
+
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ isl76683_power(chip, false);
+
+ return 0;
+}
+
+#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM)
+static int isl76683_runtime_suspend(struct device *dev)
+{
+ struct isl76683_chip *chip =
+ iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+ int ret;
+
+ mutex_lock(&chip->lock);
+ ret = isl76683_power(chip, false);
+ regcache_mark_dirty(chip->rmp);
+ mutex_unlock(&chip->lock);
+
+ return ret;
+}
+
+static int isl76683_runtime_resume(struct device *dev)
+{
+ struct isl76683_chip *chip =
+ iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+ int ret;
+
+ mutex_lock(&chip->lock);
+ ret = isl76683_power(chip, true);
+ mutex_unlock(&chip->lock);
+
+ return ret;
+}
+#endif
+
+static const struct dev_pm_ops isl76683_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(isl76683_runtime_suspend,
+ isl76683_runtime_resume, NULL)
+};
+
+static const struct i2c_device_id isl76683_id[] = {
+ {"isl76683", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, isl76683_id);
+
+static const struct of_device_id isl76683_of_match[] = {
+ { .compatible = "isil,isl76683", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, isl76683_of_match);
+
+static struct i2c_driver isl76683_driver = {
+ .driver = {
+ .name = "isl76683",
+ .of_match_table = isl76683_of_match,
+ .pm = &isl76683_pm_ops,
+ },
+ .probe = isl76683_probe,
+ .remove = isl76683_remove,
+ .id_table = isl76683_id,
+};
+
+module_i2c_driver(isl76683_driver);
+
+MODULE_DESCRIPTION("ISL76683 Ambient Light Sensor driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>");
--
2.1.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 2/3] dt-bindings: iio: add Intersil isl76683 light sensor bindings
From: Christoph Fritz @ 2017-11-18 23:20 UTC (permalink / raw)
To: Jonathan Cameron, Peter Meerwald-Stadler, Rob Herring
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1511047230-7021-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch adds documentation of device tree bindings for Intersil
isl76683 light sensor.
Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
.../devicetree/bindings/iio/light/isl76683.txt | 26 ++++++++++++++++++++++
.../devicetree/bindings/property-units.txt | 1 +
2 files changed, 27 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/light/isl76683.txt
diff --git a/Documentation/devicetree/bindings/iio/light/isl76683.txt b/Documentation/devicetree/bindings/iio/light/isl76683.txt
new file mode 100644
index 0000000..b1a8a67
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/isl76683.txt
@@ -0,0 +1,26 @@
+* ISL76683 ambient light sensor
+
+http://www.intersil.com/content/dam/Intersil/documents/isl7/isl76683.pdf
+
+Required properties:
+
+ - compatible: must be "isil,isl76683"
+ - reg: the I2C address of the sensor
+ - interrupt-parent: should be the phandle for the interrupt controller
+ - interrupts: interrupt mapping for GPIO IRQ
+
+Optional properties:
+
+ - isil,external-resistor-kilo-ohms: integer in kilo Ohms of external resistor
+ R_ext. Valid values are from 1 to 1000. If not supplied, 100 kilo Ohms
+ will be assumed.
+
+Example:
+
+isl76683@44 {
+ compatible = "isil,isl76683";
+ reg = <0x44>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
+ isil,external-resistor-kilo-ohms = <50>;
+};
diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index 45ce054..5f9c71a 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -28,6 +28,7 @@ Electricity
-microamp-hours : micro amp-hours
-ohms : Ohms
-micro-ohms : micro Ohms
+-kilo-ohms : kilo Ohms
-microwatt-hours: micro Watt-hours
-microvolt : micro volts
-picofarads : picofarads
--
2.1.4
^ permalink raw reply related
* [PATCH v2 3/3] iio: light: isl76683 add way to adjust irq threshold
From: Christoph Fritz @ 2017-11-18 23:20 UTC (permalink / raw)
To: Jonathan Cameron, Peter Meerwald-Stadler, Rob Herring
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1511047230-7021-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch adds sysfs read/write support for upper and lower irq
thresholds. So it's possible that only on certain lux ranges the
irq triggered measurement happens.
Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
.../ABI/testing/sysfs-bus-iio-light-isl76683 | 17 +++++++
drivers/iio/light/isl76683.c | 57 ++++++++++++++++++++--
2 files changed, 71 insertions(+), 3 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-isl76683 b/Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
new file mode 100644
index 0000000..c7e08d7
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
@@ -0,0 +1,17 @@
+What: /sys/bus/iio/devices/iio:deviceX/in_illuminance_threshold_low
+Date: November 2017
+KernelVersion: 4.15.0
+Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+ Raw value of lower threshold for the interrupt.
+ Reading returns 8-bit MSB data of a 16-bit threshold.
+ Writing 0...255 represents 8-bit MSB data of a 16-bit threshold.
+
+What: /sys/bus/iio/devices/iio:deviceX/in_illuminance_threshold_up
+Date: November 2017
+KernelVersion: 4.15.0
+Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+ Raw value of upper threshold for the interrupt.
+ Reading returns 8-bit MSB data of a 16-bit threshold.
+ Writing 0...255 represents 8-bit MSB data of a 16-bit threshold.
diff --git a/drivers/iio/light/isl76683.c b/drivers/iio/light/isl76683.c
index 4d158f6..1507158 100644
--- a/drivers/iio/light/isl76683.c
+++ b/drivers/iio/light/isl76683.c
@@ -74,11 +74,14 @@ static const int isl76683_lux_ranges_available[] = {
#define ISL76683_KOHM_MAX 1000
#define ISL76683_INTPERS_DEFAULT 0x0
#define ISL76683_THR_DEFAULT 0x7f
+#define ISL76683_THR_MAX 0xFF
struct isl76683_chip {
enum isl76683_lux_range luxrange;
int external_resistor;
enum isl76683_dmode photodiode;
+ int threshold_up;
+ int threshold_low;
struct i2c_client *client;
struct regmap *rmp;
struct completion irq_complete;
@@ -157,13 +160,12 @@ static int isl76683_set_config(struct isl76683_chip *chip)
if (ret < 0)
return ret;
- ret = regmap_write(chip->rmp, ISL76683_REG_THR_HI,
- ISL76683_THR_DEFAULT);
+ ret = regmap_write(chip->rmp, ISL76683_REG_THR_HI, chip->threshold_up);
if (ret < 0)
return ret;
return regmap_write(chip->rmp, ISL76683_REG_THR_LO,
- ISL76683_THR_DEFAULT);
+ chip->threshold_low);
}
static int isl76683_power(struct isl76683_chip *chip, bool on)
@@ -402,11 +404,58 @@ static int isl76683_write_raw(struct iio_dev *indio_dev,
return -EINVAL;
}
+#define ISL76683_SYSFS_STORE(ident, _max) \
+static ssize_t in_illuminance_##ident##_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t len) \
+{ \
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev); \
+ struct isl76683_chip *chip = iio_priv(indio_dev); \
+ unsigned int val; \
+ int ret; \
+ \
+ if (kstrtouint(buf, 10, &val)) \
+ return -EINVAL; \
+ \
+ if (val > _max) \
+ return -EINVAL; \
+ \
+ mutex_lock(&chip->lock); \
+ chip->ident = val; \
+ ret = isl76683_set_config(chip); \
+ mutex_unlock(&chip->lock); \
+ \
+ if (ret) \
+ return -EIO; \
+ \
+ return len; \
+}
+
+ISL76683_SYSFS_STORE(threshold_low, ISL76683_THR_MAX)
+ISL76683_SYSFS_STORE(threshold_up, ISL76683_THR_MAX)
+
+#define ISL76683_SYSFS_SHOW(ident, show_val) \
+static ssize_t in_illuminance_##ident##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev); \
+ struct isl76683_chip *chip = iio_priv(indio_dev); \
+ \
+ return snprintf(buf, PAGE_SIZE, "%i\n", show_val); \
+}
+
+ISL76683_SYSFS_SHOW(threshold_up, chip->threshold_up)
+ISL76683_SYSFS_SHOW(threshold_low, chip->threshold_low)
+
static IIO_CONST_ATTR(in_illuminance_scale_available,
ISL76683_LUXRANGE_STR);
+static IIO_DEVICE_ATTR_RW(in_illuminance_threshold_up, 0);
+static IIO_DEVICE_ATTR_RW(in_illuminance_threshold_low, 0);
static struct attribute *isl76683_attributes[] = {
&iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
+ &iio_dev_attr_in_illuminance_threshold_up.dev_attr.attr,
+ &iio_dev_attr_in_illuminance_threshold_low.dev_attr.attr,
NULL
};
@@ -508,6 +557,8 @@ static int isl76683_probe(struct i2c_client *client,
chip->luxrange = ISL76683_LUX_RANGE_DEFAULT;
chip->external_resistor = rs;
chip->photodiode = ISL76683_DIODE_DEFAULT;
+ chip->threshold_up = ISL76683_THR_DEFAULT;
+ chip->threshold_low = ISL76683_THR_DEFAULT;
chip->rmp = devm_regmap_init_i2c(client, &isl76683_regmap_config);
if (IS_ERR(chip->rmp)) {
--
2.1.4
^ permalink raw reply related
* Re: [RFC v1 7/8] Bluetooth: hci_serdev: remove the HCI_UART_INIT_PENDING check
From: Marcel Holtmann @ 2017-11-19 8:21 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Rob Herring, devicetree, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
Gustavo F. Padovan, Johan Hedberg, Greg Kroah-Hartman, Jiri Slaby,
Johan Hovold, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ
In-Reply-To: <20171117223543.32429-8-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Hi Martin,
> The three-wire (H5) protocol is the only protocol which uses
> HCI_UART_INIT_PENDING.
> Unfortunately the protocol implementation never receives data with this
> check still in place. For the H5 protocol this means that the
> initialization never completes and thus the firmware download never
> starts. Even if the initialization would succeed later on the drivers
> would call hci_uart_init_ready() which schedules the registration which
> is currently not implemented by hci_serdev.c.
>
> Removing the HCI_UART_INIT_PENDING check makes the code easier to read
> and also fixes the initalization of devices (implemented with the serdev
> library) which use the H5 protocol.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> drivers/bluetooth/hci_serdev.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
> index e0e6461b9200..fe67eb6d4278 100644
> --- a/drivers/bluetooth/hci_serdev.c
> +++ b/drivers/bluetooth/hci_serdev.c
> @@ -333,9 +333,6 @@ int hci_uart_register_device(struct hci_uart *hu,
> else
> hdev->dev_type = HCI_PRIMARY;
>
> - if (test_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags))
> - return 0;
> -
then lets also remove the flag definition itself. Or if that is still needed for some legacy operation, then describe this. For example I also see it used in hci_serdev.c and main question would be if it is used there as well or some legacy cruft.
Regards
Marcel
--
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: [RFC v1 5/8] Bluetooth: btrtl: add support for the RTL8723BS and RTL8723DS chips
From: Marcel Holtmann @ 2017-11-19 8:25 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
Gustavo F. Padovan, Johan Hedberg,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
johan-DgEjT+Ai2ygdnm+yROfE0A, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ
In-Reply-To: <20171117223543.32429-6-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Hi Martin,
> The Realtek RTL8723BS and RTL8723DS chipsets are SDIO wifi chips. They
> also contain a Bluetooth module which is connected via UART to the host.
>
> Realtek's userspace initialization tool (rtk_hciattach) differentiates
> these two via the HCI version and revision returned by the
> HCI_OP_READ_LOCAL_VERSION command.
> Additionally we apply these checks only the for UART devices. Everything
> else is assumed to be a "RTL8723B" which was originally supported by the
> driver (communicating via USB).
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> drivers/bluetooth/btrtl.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 45b872f5ad22..d896f9421250 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -418,9 +418,33 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev)
> has_rom_version = false;
> break;
> case RTL_ROM_LMP_8723B:
> - fw_name = "rtl_bt/rtl8723b_fw.bin";
> - cfg_name = "rtl_bt/rtl8723b_config.bin";
> + /* all variants support reading the ROM version */
> has_rom_version = true;
> +
> + /*
> + * RTL8723 devices exist in different variants:
> + * - RTL8723BS (SDIO chip with UART Bluetooth)
> + * - RTL8723DS (SDIO chip with UART Bluetooth)
> + * - for backwards-compatibility everything else is assumed to
> + * be an RTL8723B communicating over USB
> + *
> + * Only UART devices really need the config because that
> + * contains the UART speed / flow control settings.
> + */
> + if (hdev->bus == HCI_UART && resp->hci_ver == 6 &&
> + le16_to_cpu(resp->hci_rev) == 0xb) {
> + fw_name = "rtl_bt/rtl8723bs_fw.bin";
> + cfg_name = "rtl_bt/rtl8723bs_config.bin";
> + cfg_needed = true;
> + } else if (hdev->bus == HCI_UART && resp->hci_ver == 8 &&
> + le16_to_cpu(resp->hci_rev) == 0xd) {
> + fw_name = "rtl_bt/rtl8723ds_fw.bin";
> + cfg_name = "rtl_bt/rtl872ds_config.bin";
> + cfg_needed = true;
> + } else {
> + fw_name = "rtl_bt/rtl8723b_fw.bin";
> + cfg_name = "rtl_bt/rtl8723b_config.bin";
> + }
so the main question is why is this a config file in the first place? So far all other drivers have expressed UART settings via DT (or even via ACPI). If we are just getting the baudrates, then better have this in DT. Since otherwise we end up with board specific filesystems again where different config files need to be installed. That is really not useful in the end.
What is actually in these config files? Can we have some parsing and friendly display tool in bluez.git like we have for other manufactures.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v1 6/8] Bluetooth: hci_h5: add support for Realtek UART Bluetooth modules
From: Marcel Holtmann @ 2017-11-19 8:29 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
Gustavo F. Padovan, Johan Hedberg,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
johan-DgEjT+Ai2ygdnm+yROfE0A, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ
In-Reply-To: <20171117223543.32429-7-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Hi Martin,
> Realtek RTL8723BS and RTL8723DS are SDIO wifi chips with an embedded
> Bluetooth controller which connects to the host via UART.
> The H5 protocol is used for communication between host and device.
>
> The Realtek "rtl8723bs_bt" and "rtl8723ds_bt" userspace Bluetooth UART
> initialization tools (rtk_hciattach) use the following sequence:
> 1) send H5 sync pattern (already supported by hci_h5)
> 2) get LMP version (already supported by btrtl)
> 3) get ROM version (already supported by btrtl)
> 4) load the firmware and config for the current chipset (already
> supported by btrtl)
> 5) read UART settings from the config blob (already supported by btrtl)
> 6) send UART settings via a vendor command to the device (which changes
> the baudrate of the device and enables or disables flow control
> depending on the config)
> 7) change the baudrate and flow control settings on the host
> 8) send the firmware and config blob to the device (already supported by
> btrtl)
>
> This uses the serdev library as well as the existing btrtl driver to
> initialize the Bluetooth functionality, which consists of:
> - identifying the device and loading the corresponding firmware and
> config blobs (steps #2, #3 and #4)
> - configuring the baudrate and flow control (steps #6 and #7)
> - uploading the firmware to the device (step #8)
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> drivers/bluetooth/Kconfig | 1 +
> drivers/bluetooth/hci_h5.c | 195 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 195 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 60e1c7d6986d..3001f1200c72 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -146,6 +146,7 @@ config BT_HCIUART_LL
> config BT_HCIUART_3WIRE
> bool "Three-wire UART (H5) protocol support"
> depends on BT_HCIUART
> + select BT_RTL if SERIAL_DEV_BUS
> help
> The HCI Three-wire UART Transport Layer makes it possible to
> user the Bluetooth HCI over a serial port interface. The HCI
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 6a8d0d06aba7..7d584e5928bf 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -28,7 +28,14 @@
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/serdev.h>
> +
> #include "hci_uart.h"
> +#include "btrtl.h"
>
> #define HCI_3WIRE_ACK_PKT 0
> #define HCI_3WIRE_LINK_PKT 15
> @@ -97,6 +104,13 @@ struct h5 {
> } sleep;
> };
>
> +struct h5_device {
> + struct hci_uart hu;
> + struct gpio_desc *disable_gpio;
> + struct gpio_desc *reset_gpio;
> + int (*vendor_setup)(struct h5_device *h5_dev);
> +};
> +
> static void h5_reset_rx(struct h5 *h5);
>
> static void h5_link_control(struct hci_uart *hu, const void *data, size_t len)
> @@ -190,6 +204,7 @@ static int h5_open(struct hci_uart *hu)
> {
> struct h5 *h5;
> const unsigned char sync[] = { 0x01, 0x7e };
> + int err;
>
> BT_DBG("hu %p", hu);
>
> @@ -210,6 +225,14 @@ static int h5_open(struct hci_uart *hu)
>
> h5->tx_win = H5_TX_WIN_MAX;
>
> + if (hu->serdev) {
> + err = serdev_device_open(hu->serdev);
> + if (err) {
> + bt_dev_err(hu->hdev, "failed to open serdev: %d", err);
> + return err;
> + }
> + }
> +
> set_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags);
>
> /* Send initial sync request */
> @@ -219,6 +242,23 @@ static int h5_open(struct hci_uart *hu)
> return 0;
> }
>
> +static int h5_setup(struct hci_uart *hu)
> +{
> + int err;
> + struct h5_device *h5_dev;
> +
> + if (!hu->serdev)
> + return 0;
> +
> + h5_dev = serdev_device_get_drvdata(hu->serdev);
> +
> + err = h5_dev->vendor_setup(h5_dev);
> + if (err)
> + return err;
if (h5_dev->vendor_setup)
return h5_dev->vendor_setup(h5_dev);
> +
> + return 0;
> +}
> +
> static int h5_close(struct hci_uart *hu)
> {
> struct h5 *h5 = hu->priv;
> @@ -229,6 +269,15 @@ static int h5_close(struct hci_uart *hu)
> skb_queue_purge(&h5->rel);
> skb_queue_purge(&h5->unrel);
>
> + if (hu->serdev) {
> + struct h5_device *h5_dev;
> +
> + h5_dev = serdev_device_get_drvdata(hu->serdev);
> + gpiod_set_value_cansleep(h5_dev->disable_gpio, 1);
> +
> + serdev_device_close(hu->serdev);
> + }
> +
> kfree(h5);
>
> return 0;
> @@ -316,7 +365,10 @@ static void h5_handle_internal_rx(struct hci_uart *hu)
> h5->tx_win = (data[2] & 0x07);
> BT_DBG("Three-wire init complete. tx_win %u", h5->tx_win);
> h5->state = H5_ACTIVE;
> - hci_uart_init_ready(hu);
> +
> + /* serdev does not support the "init_ready" signal */
> + if (!hu->serdev)
> + hci_uart_init_ready(hu);
> return;
> } else if (memcmp(data, sleep_req, 2) == 0) {
> BT_DBG("Peer went to sleep");
> @@ -739,10 +791,147 @@ static int h5_flush(struct hci_uart *hu)
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
> +static int h5_setup_realtek(struct h5_device *h5_dev)
> +{
> + struct hci_uart *hu = &h5_dev->hu;
> + int err = 0, retry = 3;
> + struct sk_buff *skb;
> + struct btrtl_device_info *btrtl_dev;
> + __le32 baudrate_data;
> + u32 device_baudrate;
> + unsigned int controller_baudrate;
> + bool flow_control;
> +
> + /* devices always start with flow control disabled and even parity */
> + serdev_device_set_flow_control(hu->serdev, false);
> + serdev_device_set_parity(hu->serdev, true, false);
> +
> + do {
> + /* Configure BT_DISn and BT_RST_N to LOW state */
> + gpiod_set_value_cansleep(h5_dev->reset_gpio, 1);
> + gpiod_set_value_cansleep(h5_dev->disable_gpio, 1);
> + msleep(500);
> + gpiod_set_value_cansleep(h5_dev->reset_gpio, 0);
> + gpiod_set_value_cansleep(h5_dev->disable_gpio, 0);
> + msleep(500);
I really hate random msleep() without comments. Explain in the comment block why this specific wait is good.
> +
> + btrtl_dev = btrtl_initialize(hu->hdev);
> + if (!IS_ERR(btrtl_dev))
> + break;
> +
> + /* Toggle BT_DISn and retry */
> + } while (retry--);
> +
> + if (IS_ERR(btrtl_dev))
> + return PTR_ERR(btrtl_dev);
> +
> + err = btrtl_get_uart_settings(hu->hdev, btrtl_dev,
> + &controller_baudrate, &device_baudrate,
> + &flow_control);
> + if (err)
> + goto out_free;
> +
> + baudrate_data = cpu_to_le32(device_baudrate);
> + skb = __hci_cmd_sync(hu->hdev, 0xfc17, sizeof(baudrate_data),
> + &baudrate_data, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hu->hdev, "set baud rate command failed");
> + err = -PTR_ERR(skb);
> + goto out_free;
> + } else {
> + kfree_skb(skb);
> + }
> +
> + msleep(500);
Same here, explain why this time is the right time to wait.
> +
> + serdev_device_set_baudrate(hu->serdev, controller_baudrate);
> + serdev_device_set_flow_control(hu->serdev, flow_control);
> +
> + err = btrtl_download_firmware(hu->hdev, btrtl_dev);
> +
> +out_free:
> + btrtl_free(btrtl_dev);
> +
> + return err;
> +}
> +
> +static const struct hci_uart_proto h5p;
> +
> +static int hci_h5_probe(struct serdev_device *serdev)
> +{
> + struct hci_uart *hu;
> + struct h5_device *h5_dev;
> +
> + h5_dev = devm_kzalloc(&serdev->dev, sizeof(*h5_dev), GFP_KERNEL);
> + if (!h5_dev)
> + return -ENOMEM;
> +
> + hu = &h5_dev->hu;
> + hu->serdev = serdev;
> +
> + serdev_device_set_drvdata(serdev, h5_dev);
> +
> + h5_dev->vendor_setup = of_device_get_match_data(&serdev->dev);
> +
> + h5_dev->disable_gpio = devm_gpiod_get_optional(&serdev->dev, "disable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(h5_dev->disable_gpio))
> + return PTR_ERR(h5_dev->disable_gpio);
> +
> + h5_dev->reset_gpio = devm_gpiod_get_optional(&serdev->dev, "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(h5_dev->reset_gpio))
> + return PTR_ERR(h5_dev->reset_gpio);
> +
> + hci_uart_set_speeds(hu, 115200, 0);
> +
> + return hci_uart_register_device(hu, &h5p);
> +}
> +
> +static void hci_h5_remove(struct serdev_device *serdev)
> +{
> + struct h5_device *h5_dev = serdev_device_get_drvdata(serdev);
> + struct hci_uart *hu = &h5_dev->hu;
> + struct hci_dev *hdev = hu->hdev;
> +
> + cancel_work_sync(&hu->write_work);
> +
> + hci_unregister_dev(hdev);
> + hci_free_dev(hdev);
> + hu->proto->close(hu);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id hci_h5_of_match[] = {
> + {
> + .compatible = "realtek,rtl8723bs-bluetooth",
> + .data = h5_setup_realtek
> + },
> + {
> + .compatible = "realtek,rtl8723ds-bluetooth",
> + .data = h5_setup_realtek
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, hci_h5_of_match);
> +#endif
> +
> +static struct serdev_device_driver hci_h5_drv = {
> + .driver = {
> + .name = "hci-h5",
> + .of_match_table = of_match_ptr(hci_h5_of_match),
> + },
> + .probe = hci_h5_probe,
> + .remove = hci_h5_remove,
> +};
> +#endif
> +
> static const struct hci_uart_proto h5p = {
> .id = HCI_UART_3WIRE,
> .name = "Three-wire (H5)",
> .open = h5_open,
> + .setup = h5_setup,
> .close = h5_close,
> .recv = h5_recv,
> .enqueue = h5_enqueue,
> @@ -752,10 +941,14 @@ static const struct hci_uart_proto h5p = {
>
> int __init h5_init(void)
> {
> + serdev_device_driver_register(&hci_h5_drv);
> +
> return hci_uart_register_proto(&h5p);
> }
>
> int __exit h5_deinit(void)
> {
> + serdev_device_driver_unregister(&hci_h5_drv);
> +
> return hci_uart_unregister_proto(&h5p);
> }
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 1/3] iio: light: Add support for Intersil isl76683 sensor
From: Jonathan Cameron @ 2017-11-19 11:10 UTC (permalink / raw)
To: Christoph Fritz
Cc: Peter Meerwald-Stadler, Rob Herring,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1511047230-7021-2-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Sun, 19 Nov 2017 00:20:28 +0100
Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> This patch adds support for Intersil isl76683 light sensor.
>
> Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Hi Christoph,
Sorry my review on V1 was so superficial. I missed some bigger stuff.
Anyhow, comments inline.
Main one is that you are hand spinning a tight loop trigger here.
There is no need to do so given we have one already that does this cleaner
(I wrote it as a response to the last time we had a driver doing what you
have here). Use that and you'll have a much more generic result whilst
achieving much the same result as you have here.
Jonathan
> ---
> drivers/iio/light/Kconfig | 12 +
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/isl76683.c | 663 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 676 insertions(+)
> create mode 100644 drivers/iio/light/isl76683.c
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 2356ed9..4f0882c 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -194,6 +194,18 @@ config ISL29125
> To compile this driver as a module, choose M here: the module will be
> called isl29125.
>
> +config ISL76683
> + tristate "Intersil ISL76683 light sensor"
> + depends on I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say Y here if you want to build a driver for the Intersil ISL76683
> + light sensor for I2C.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called isl76683.
> +
> config HID_SENSOR_ALS
> depends on HID_SENSOR_HUB
> select IIO_BUFFER
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index fa32fa4..886a51f 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_HID_SENSOR_PROX) += hid-sensor-prox.o
> obj-$(CONFIG_SENSORS_ISL29018) += isl29018.o
> obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o
> obj-$(CONFIG_ISL29125) += isl29125.o
> +obj-$(CONFIG_ISL76683) += isl76683.o
> obj-$(CONFIG_JSA1212) += jsa1212.o
> obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
> obj-$(CONFIG_LTR501) += ltr501.o
> diff --git a/drivers/iio/light/isl76683.c b/drivers/iio/light/isl76683.c
> new file mode 100644
> index 0000000..4d158f6
> --- /dev/null
> +++ b/drivers/iio/light/isl76683.c
> @@ -0,0 +1,663 @@
> +/*
> + * IIO driver for the light sensor ISL76683.
> + *
> + * Copyright (c) 2017 Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * Datasheet:
> + * http://www.intersil.com/content/dam/Intersil/documents/isl7/isl76683.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/util_macros.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +
> +#define ISL76683_REG_CMD 0x00
> +#define ISL76683_REG_CTRL 0x01
> +#define ISL76683_REG_THR_HI 0x02
> +#define ISL76683_REG_THR_LO 0x03
> +#define ISL76683_REG_SENSOR_LSB 0x04
> +#define ISL76683_REG_SENSOR_MSB 0x05
> +#define ISL76683_REG_CLEAR_INT 0x40
> +#define ISL76683_REGMAP_MAX 0x40
> +
> +#define ISL76683_CMD_ENABLE BIT(7)
> +#define ISL76683_CMD_PWRDWN BIT(6)
> +#define ISL76683_WIDTH_MASK 0x3
> +#define ISL76683_PHOTOD_SHFT 2
> +#define ISL76683_PHOTOD_MASK GENMASK(3, ISL76683_PHOTOD_SHFT)
> +#define ISL76683_INTPERS_MASK 0x3
> +#define ISL76683_LUXRANGE_SHFT 2
> +#define ISL76683_LUXRANGE_MASK (0x3 << ISL76683_LUXRANGE_SHFT)
> +#define ISL76683_LUXRANGE_STR "1000 4000 16000 64000"
> +
> +enum isl76683_dmode {
> + ISL76683_DIODE_0 = 0,
> + ISL76683_DIODE_IR,
> + ISL76683_DIODE_DIFF,
> +};
> +
> +enum isl76683_lux_range {
> + ISL76683_LUX_1000 = 0,
> + ISL76683_LUX_4000,
> + ISL76683_LUX_16000,
> + ISL76683_LUX_64000,
> +};
> +
> +static const int isl76683_lux_ranges_available[] = {
> + 1000, 4000, 16000, 64000};
> +
> +#define ISL76683_LUX_RANGE_DEFAULT ISL76683_LUX_1000
> +#define ISL76683_DIODE_MAX ISL76683_DIODE_DIFF
> +#define ISL76683_DIODE_DEFAULT ISL76683_DIODE_0
> +#define ISL76683_WIDTH_DEFAULT 0x0
> +#define ISL76683_RESOLUTION_DEFAULT 16
> +#define ISL76683_EXT_RESISTOR_DEFAULT 100
> +#define ISL76683_KOHM_MIN 1
> +#define ISL76683_KOHM_MAX 1000
> +#define ISL76683_INTPERS_DEFAULT 0x0
> +#define ISL76683_THR_DEFAULT 0x7f
> +
> +struct isl76683_chip {
> + enum isl76683_lux_range luxrange;
> + int external_resistor;
> + enum isl76683_dmode photodiode;
> + struct i2c_client *client;
> + struct regmap *rmp;
> + struct completion irq_complete;
> + struct iio_trigger *trig;
> + bool trig_enabled;
> + struct mutex lock;
> + s64 time_ns;
> +};
> +
> +static bool isl76683_readable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case ISL76683_REG_CMD:
> + case ISL76683_REG_CTRL:
> + case ISL76683_REG_THR_HI:
> + case ISL76683_REG_THR_LO:
> + case ISL76683_REG_SENSOR_LSB:
> + case ISL76683_REG_SENSOR_MSB:
> + case ISL76683_REG_CLEAR_INT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool isl76683_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case ISL76683_REG_CMD:
> + case ISL76683_REG_CTRL:
> + case ISL76683_REG_THR_HI:
> + case ISL76683_REG_THR_LO:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static bool isl76683_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case ISL76683_REG_SENSOR_LSB:
> + case ISL76683_REG_SENSOR_MSB:
> + case ISL76683_REG_CLEAR_INT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config isl76683_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = ISL76683_REGMAP_MAX,
> + .readable_reg = isl76683_readable_reg,
> + .writeable_reg = isl76683_writeable_reg,
> + .volatile_reg = isl76683_is_volatile_reg,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int isl76683_set_config(struct isl76683_chip *chip)
> +{
> + int ret;
> +
> + ret = regmap_update_bits(chip->rmp, ISL76683_REG_CTRL,
> + ISL76683_LUXRANGE_MASK | ISL76683_INTPERS_MASK,
> + (chip->luxrange << ISL76683_LUXRANGE_SHFT) |
> + ISL76683_INTPERS_DEFAULT);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_update_bits(chip->rmp, ISL76683_REG_CMD,
> + ISL76683_PHOTOD_MASK | ISL76683_WIDTH_MASK,
> + (chip->photodiode << ISL76683_PHOTOD_SHFT) |
> + ISL76683_WIDTH_DEFAULT);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(chip->rmp, ISL76683_REG_THR_HI,
> + ISL76683_THR_DEFAULT);
> + if (ret < 0)
> + return ret;
> +
> + return regmap_write(chip->rmp, ISL76683_REG_THR_LO,
> + ISL76683_THR_DEFAULT);
> +}
> +
> +static int isl76683_power(struct isl76683_chip *chip, bool on)
> +{
> + int ret;
> +
> + ret = regmap_update_bits(chip->rmp, ISL76683_REG_CMD,
> + ISL76683_CMD_ENABLE | ISL76683_CMD_PWRDWN,
> + 0x0);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_update_bits(chip->rmp, ISL76683_REG_CMD,
> + ISL76683_CMD_ENABLE | ISL76683_CMD_PWRDWN,
> + on ? ISL76683_CMD_ENABLE : ISL76683_CMD_PWRDWN);
> + if (ret < 0)
> + return ret;
> +
> + return on ? isl76683_set_config(chip) : 0;
> +}
> +
> +static int isl76683_reset(struct isl76683_chip *chip)
> +{
> + int ret;
> +
> + ret = isl76683_power(chip, false);
> + if (ret < 0)
> + return ret;
> +
> + return isl76683_power(chip, true);
> +}
> +
> +static int isl76683_get_sensordata(struct isl76683_chip *chip,
> + bool is_processed, int *val)
> +{
> + unsigned int sensor_data, range, fsr;
> + __le16 sensor_raw;
> + int ret;
> +
> + ret = regmap_bulk_read(chip->rmp, ISL76683_REG_SENSOR_LSB,
> + &sensor_raw, sizeof(sensor_raw));
> + if (ret)
> + return ret;
> +
> + sensor_data = le16_to_cpu(sensor_raw);
> +
> + if (!is_processed) {
> + *val = sensor_data;
> + return 0;
> + }
> +
> + /* range values taken from datasheet (table 9) */
> + if (chip->luxrange == ISL76683_LUX_1000)
> + range = 973;
> + else if (chip->luxrange == ISL76683_LUX_4000)
> + range = 3892;
> + else if (chip->luxrange == ISL76683_LUX_16000)
> + range = 15568;
> + else if (chip->luxrange == ISL76683_LUX_64000)
> + range = 62272;
> + else
> + return -EINVAL;
> +
> + /* equations from datasheet (EQ.3 and EQ.4) */
> + fsr = (100 * range) / chip->external_resistor;
> + *val = (fsr * sensor_data) / (1 << ISL76683_RESOLUTION_DEFAULT);
Why is it necessary to do this processing in kernel?
> +
> + return 0;
> +}
> +
> +static irqreturn_t isl76683_interrupt_handler(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct isl76683_chip *chip = iio_priv(indio_dev);
> +
> + chip->time_ns = iio_get_time_ns(indio_dev);
> +
> + if (chip->trig_enabled)
> + iio_trigger_poll(chip->trig);
> +
> + if (!completion_done(&chip->irq_complete))
> + complete(&chip->irq_complete);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static inline int isl76683_start_measurement(struct isl76683_chip *chip)
> +{
> + int dummy;
> +
> + /* dummy read is clearing irq and triggers new measurement */
> + return regmap_read(chip->rmp, ISL76683_REG_CLEAR_INT, &dummy);
> +}
> +
> +static int isl76683_singleshot_conversion(struct isl76683_chip *chip,
> + const struct iio_chan_spec *chan,
> + bool is_processed, int *val)
> +{
> + long timeout;
> + int ret;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_LIGHT_BOTH:
> + chip->photodiode = ISL76683_DIODE_DIFF;
> + break;
> + case IIO_MOD_LIGHT_IR:
> + chip->photodiode = ISL76683_DIODE_IR;
> + break;
> + default:
> + chip->photodiode = ISL76683_DIODE_0;
> + }
> +
> + ret = isl76683_set_config(chip);
> + if (ret)
> + return ret;
> +
> + ret = isl76683_start_measurement(chip);
> + if (ret < 0)
> + return ret;
> +
> + /* wait for measurement to complete */
> + timeout = wait_for_completion_interruptible_timeout(
> + &chip->irq_complete,
> + msecs_to_jiffies(5000));
> + if (timeout == 0) {
> + dev_err(&chip->client->dev, "measurement timed out\n");
> + return -ETIMEDOUT;
> + } else if (timeout < 0) {
> + dev_err(&chip->client->dev, "wait for measurement failed\n");
> + return -EINTR;
> + }
> +
> + ret = isl76683_get_sensordata(chip, is_processed, val);
> + if (ret) {
> + dev_err(&chip->client->dev, "%s: Error %d reading lux\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + return IIO_VAL_INT;
> +}
> +
> +static irqreturn_t isl76683_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct isl76683_chip *chip = iio_priv(indio_dev);
> + u8 data[16] = { }; /* 2x 8-bit data + padding + 8 bytes timestamp */
> + __le16 buf;
> + int ret;
> +
> + ret = regmap_bulk_read(chip->rmp, ISL76683_REG_SENSOR_LSB, &buf,
> + sizeof(buf));
> + if (ret)
> + return IRQ_HANDLED;
> +
> + data[0] = buf;
Only the bottom 8 bits?
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, data, chip->time_ns);
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + ret = isl76683_start_measurement(chip);
> + if (ret < 0)
> + return IRQ_HANDLED;
Not really worth checking ret :) Anyhow, this will go away I think given
comments below.
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int isl76683_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct isl76683_chip *chip = iio_priv(indio_dev);
> + int ret;
> +
> + chip->photodiode = indio_dev->channels[0].channel2;
> + ret = isl76683_set_config(chip);
This is configuring the device to match the scan mode requested.
Right callback for that is update scan_mode in struct iio_info.
> +
> + return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops isl76683_buffer_setup_ops = {
> + .preenable = &isl76683_buffer_preenable,
> + .predisable = iio_triggered_buffer_predisable,
> + .postenable = iio_triggered_buffer_postenable,
> + .validate_scan_mask = &iio_validate_scan_mask_onehot,
> +};
> +
> +static int isl76683_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct isl76683_chip *chip = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> + ret = isl76683_singleshot_conversion(chip, chan, false, val);
> + iio_device_release_direct_mode(indio_dev);
> + return ret;
> + case IIO_CHAN_INFO_PROCESSED:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> + ret = isl76683_singleshot_conversion(chip, chan, true, val);
> + iio_device_release_direct_mode(indio_dev);
> + return ret;
> + case IIO_CHAN_INFO_SCALE:
> + *val = isl76683_lux_ranges_available[chip->luxrange];
> + return IIO_VAL_INT;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int isl76683_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct isl76683_chip *chip = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + mutex_lock(&chip->lock);
> + chip->luxrange = find_closest(val,
> + isl76683_lux_ranges_available,
> + ARRAY_SIZE(isl76683_lux_ranges_available));
> + ret = isl76683_set_config(chip);
> + mutex_unlock(&chip->lock);
> + return ret;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static IIO_CONST_ATTR(in_illuminance_scale_available,
> + ISL76683_LUXRANGE_STR);
> +
> +static struct attribute *isl76683_attributes[] = {
> + &iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group isl76683_attribute_group = {
> + .attrs = isl76683_attributes,
> +};
> +
> +#define ISL76683_CHANNEL(_ch2, _si) { \
> + .type = IIO_LIGHT, \
> + .modified = 1, \
> + .channel2 = _ch2, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_PROCESSED), \
Channels should not be provided in both raw and processed forms.
The only time to provide processed data is when there is a reason
we can't provide the raw data (usually annoying non linear scaling etc).
So just provide the raw form here.
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_index = _si, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 16, \
> + .storagebits = 16, \
> + .endianness = IIO_LE, \
> + }, \
> +}
> +
> +static const struct iio_chan_spec isl76683_channels[] = {
> + {
> + .type = IIO_LIGHT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_PROCESSED),
Again, provide raw only. Conversion up to userspace (which has
the advantage of floating point maths!)
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> + .scan_index = 0,
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + ISL76683_CHANNEL(IIO_MOD_LIGHT_IR, 1),
> + ISL76683_CHANNEL(IIO_MOD_LIGHT_BOTH, 2),
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const struct iio_info isl76683_info = {
> + .read_raw = isl76683_read_raw,
> + .write_raw = isl76683_write_raw,
> + .attrs = &isl76683_attribute_group,
> +};
> +
> +static int isl76683_set_trigger_state(struct iio_trigger *trig, bool enable)
> +{
> + struct isl76683_chip *chip = iio_trigger_get_drvdata(trig);
> + int ret;
> +
> + if (enable) {
> + chip->trig_enabled = true;
This is a little unusual.
Superficially it looks the device doesn't really have an internal sequencer, but
rather is triggered on demand and you are doing that in a fairly tight loop?
For that we have the generic iio-trig-loop. Please look at whether
that can meet your usecase. It basically relies on using a thread to call
the irq thread of a device in a tight loop.
So the threaded part of the irq handler would do:
1. isl76683_start_measurement
2. Wait on completion
3. (completion from the actual interrupt handler)
4. grab data to buffer and return - then the loop trigger would restart
the sequence.
This makes it compatible with other triggers as well so a high resolution timer
trigger of sysfs trigger would work fine as well.
> + ret = isl76683_start_measurement(chip);
> + if (ret < 0)
> + return ret;
> + } else
> + chip->trig_enabled = false;
> +
> + return 0;
> +}
> +
> +static const struct iio_trigger_ops isl76683_trigger_ops = {
> + .owner = THIS_MODULE,
This field is gone. So drop the .owner from here. If not I
can clean up when applying. Note we dropped this in favour
of doing the equivalent using some macro magic.
> + .set_trigger_state = isl76683_set_trigger_state,
> + .validate_device = iio_trigger_validate_own_device,
> +};
> +
> +static int isl76683_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct isl76683_chip *chip;
> + struct iio_dev *indio_dev;
> + struct device_node *np = client->dev.of_node;
> + int rs = ISL76683_EXT_RESISTOR_DEFAULT;
> + int v, ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + chip = iio_priv(indio_dev);
> +
> + i2c_set_clientdata(client, indio_dev);
> + chip->client = client;
> +
> + if (np) {
> + ret = of_property_read_u32(np,
> + "isil,external-resistor-kilo-ohms", &v);
> + if (ret || v < ISL76683_KOHM_MIN || v > ISL76683_KOHM_MAX)
> + dev_warn(&client->dev,
> + "assuming %i kOhm resistor\n", rs);
> + else
> + rs = v;
> + }
> +
> + chip->luxrange = ISL76683_LUX_RANGE_DEFAULT;
> + chip->external_resistor = rs;
> + chip->photodiode = ISL76683_DIODE_DEFAULT;
> +
> + chip->rmp = devm_regmap_init_i2c(client, &isl76683_regmap_config);
> + if (IS_ERR(chip->rmp)) {
> + ret = PTR_ERR(chip->rmp);
> + dev_err(&client->dev, "%s: Error %d initializing regmap\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + mutex_init(&chip->lock);
> +
> + init_completion(&chip->irq_complete);
> +
> + if (!client->irq) {
> + dev_err(&client->dev, "no interrupt configured\n");
> + return -EINVAL;
> + }
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &isl76683_info;
> + indio_dev->channels = isl76683_channels;
> + indio_dev->num_channels = ARRAY_SIZE(isl76683_channels);
> + indio_dev->name = id->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + chip->trig_enabled = false;
> + chip->trig = devm_iio_trigger_alloc(&client->dev, "%s-dev%d",
> + indio_dev->name, indio_dev->id);
> + if (!chip->trig)
> + return -ENOMEM;
> +
> + chip->trig->ops = &isl76683_trigger_ops;
> + chip->trig->dev.parent = &client->dev;
> + iio_trigger_set_drvdata(chip->trig, chip);
> +
> + ret = devm_request_irq(&client->dev, client->irq,
> + isl76683_interrupt_handler,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + "isl76683_event", indio_dev);
> + if (ret) {
> + dev_err(&client->dev, "irq request error\n");
> + return ret;
> + }
> +
> + ret = devm_iio_trigger_register(&client->dev, chip->trig);
> + if (ret) {
> + dev_err(&client->dev, "iio_trigger register error\n");
> + return ret;
> + }
> +
> + ret = isl76683_reset(chip);
> + if (ret) {
> + dev_err(&client->dev, "reset failed\n");
> + return ret;
> + }
> +
> + ret = isl76683_get_sensordata(chip, false, &v);
> + if (ret) {
> + dev_err(&client->dev, "initial dummy readout failed\n");
> + return ret;
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
> + &isl76683_trigger_handler, &isl76683_buffer_setup_ops);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_device_register(&client->dev, indio_dev);
This always rings alarm bells. The moment we have a devm register
(which exposes the device to both in kernel and userspace consumers)
and we still have a remove function, that almost always means we
are looking at potential races.
The key thing is that the unregister for this will occur after the
remove function below. Hence everything should still be working until
that point... Given you turn the chip off in remove - that seems
rather unlikely!
Upshot, you can't use managed device registration, but need to
do use iio_device_register and iio_device_unregister to ensure the
ordering is correct.
> + if (ret) {
> + dev_err(&client->dev,
> + "%s(): iio registration failed with error %d\n",
> + __func__, ret);
> + }
> +
> + return ret;
> +}
> +
> +static int isl76683_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct isl76683_chip *chip = iio_priv(indio_dev);
> +
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
So what form of pm runtime are you aiming for? I'm guessing
that the intent was to do a power down on idle? That needs
to be set up, otherwise this won't do runtime pm at all..
If the intent is just to allow for that support later, then
I'd prefer you actually just did standard suspend and
resume for now and then made the change to the runtime forms
when that support is ready.
> + isl76683_power(chip, false);
> +
> + return 0;
> +}
> +
> +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM)
On this the growing movement in the kernel is not to bother
with the ifdef fun, but instead mark these functions
as __maybe_unused. The compiler will happily drop them
anyway as they aren't used.
> +static int isl76683_runtime_suspend(struct device *dev)
> +{
> + struct isl76683_chip *chip =
> + iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> + int ret;
> +
> + mutex_lock(&chip->lock);
> + ret = isl76683_power(chip, false);
> + regcache_mark_dirty(chip->rmp);
> + mutex_unlock(&chip->lock);
> +
> + return ret;
> +}
> +
> +static int isl76683_runtime_resume(struct device *dev)
> +{
> + struct isl76683_chip *chip =
> + iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> + int ret;
> +
> + mutex_lock(&chip->lock);
> + ret = isl76683_power(chip, true);
> + mutex_unlock(&chip->lock);
> +
> + return ret;
> +}
> +#endif
> +
> +static const struct dev_pm_ops isl76683_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(isl76683_runtime_suspend,
> + isl76683_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id isl76683_id[] = {
> + {"isl76683", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, isl76683_id);
> +
> +static const struct of_device_id isl76683_of_match[] = {
> + { .compatible = "isil,isl76683", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, isl76683_of_match);
> +
> +static struct i2c_driver isl76683_driver = {
> + .driver = {
> + .name = "isl76683",
> + .of_match_table = isl76683_of_match,
> + .pm = &isl76683_pm_ops,
> + },
> + .probe = isl76683_probe,
> + .remove = isl76683_remove,
> + .id_table = isl76683_id,
> +};
> +
> +module_i2c_driver(isl76683_driver);
> +
> +MODULE_DESCRIPTION("ISL76683 Ambient Light Sensor driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>");
^ permalink raw reply
* Re: [PATCH v2 3/3] iio: light: isl76683 add way to adjust irq threshold
From: Jonathan Cameron @ 2017-11-19 11:26 UTC (permalink / raw)
To: Christoph Fritz
Cc: Peter Meerwald-Stadler, Rob Herring,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1511047230-7021-4-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Sun, 19 Nov 2017 00:20:30 +0100
Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> This patch adds sysfs read/write support for upper and lower irq
> thresholds. So it's possible that only on certain lux ranges the
> irq triggered measurement happens.
>
> Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
hmm, this is 'unusual' to say the least...
>From the datasheet it initially looks like a straight forward threshold
interrupt - which should be supported as an IIO event.
However, there seems to not be a generic monitoring mode, but rather the
device has to be polled? (which makes this a 'funny' sort of interrupt..)
So I think you are ultimately using this threshold interrupt to provide
a dataready signal when there isn't a real one provided?
That's horrible and makes it very hard to fit this device into standard
frameworks. My gut feeling would be to:
* stop using the interrupt for data ready at all, but dead reckon
that with a timer delay.
* use this 'interrupt' (actually a hardware threshold signal rather than
an interrupt really) for event detection and handle it
as an event with all the standard infrastructure that is in place
for that.
I can see the hardware designers logic that you might only want to read
the values back when the light level has changed from your expected value,
but given you have to manually trigger readings, the utility of this is
somewhat limited...
Jonathan
> ---
> .../ABI/testing/sysfs-bus-iio-light-isl76683 | 17 +++++++
> drivers/iio/light/isl76683.c | 57 ++++++++++++++++++++--
> 2 files changed, 71 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-isl76683 b/Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
> new file mode 100644
> index 0000000..c7e08d7
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-light-isl76683
> @@ -0,0 +1,17 @@
> +What: /sys/bus/iio/devices/iio:deviceX/in_illuminance_threshold_low
> +Date: November 2017
> +KernelVersion: 4.15.0
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> + Raw value of lower threshold for the interrupt.
> + Reading returns 8-bit MSB data of a 16-bit threshold.
> + Writing 0...255 represents 8-bit MSB data of a 16-bit threshold.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_illuminance_threshold_up
> +Date: November 2017
> +KernelVersion: 4.15.0
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> + Raw value of upper threshold for the interrupt.
> + Reading returns 8-bit MSB data of a 16-bit threshold.
> + Writing 0...255 represents 8-bit MSB data of a 16-bit threshold.
> diff --git a/drivers/iio/light/isl76683.c b/drivers/iio/light/isl76683.c
> index 4d158f6..1507158 100644
> --- a/drivers/iio/light/isl76683.c
> +++ b/drivers/iio/light/isl76683.c
> @@ -74,11 +74,14 @@ static const int isl76683_lux_ranges_available[] = {
> #define ISL76683_KOHM_MAX 1000
> #define ISL76683_INTPERS_DEFAULT 0x0
> #define ISL76683_THR_DEFAULT 0x7f
> +#define ISL76683_THR_MAX 0xFF
>
> struct isl76683_chip {
> enum isl76683_lux_range luxrange;
> int external_resistor;
> enum isl76683_dmode photodiode;
> + int threshold_up;
> + int threshold_low;
> struct i2c_client *client;
> struct regmap *rmp;
> struct completion irq_complete;
> @@ -157,13 +160,12 @@ static int isl76683_set_config(struct isl76683_chip *chip)
> if (ret < 0)
> return ret;
>
> - ret = regmap_write(chip->rmp, ISL76683_REG_THR_HI,
> - ISL76683_THR_DEFAULT);
> + ret = regmap_write(chip->rmp, ISL76683_REG_THR_HI, chip->threshold_up);
> if (ret < 0)
> return ret;
>
> return regmap_write(chip->rmp, ISL76683_REG_THR_LO,
> - ISL76683_THR_DEFAULT);
> + chip->threshold_low);
> }
>
> static int isl76683_power(struct isl76683_chip *chip, bool on)
> @@ -402,11 +404,58 @@ static int isl76683_write_raw(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> +#define ISL76683_SYSFS_STORE(ident, _max) \
> +static ssize_t in_illuminance_##ident##_store(struct device *dev, \
> + struct device_attribute *attr, \
> + const char *buf, size_t len) \
> +{ \
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); \
> + struct isl76683_chip *chip = iio_priv(indio_dev); \
> + unsigned int val; \
> + int ret; \
> + \
> + if (kstrtouint(buf, 10, &val)) \
> + return -EINVAL; \
> + \
> + if (val > _max) \
> + return -EINVAL; \
> + \
> + mutex_lock(&chip->lock); \
> + chip->ident = val; \
> + ret = isl76683_set_config(chip); \
> + mutex_unlock(&chip->lock); \
> + \
> + if (ret) \
> + return -EIO; \
> + \
> + return len; \
> +}
> +
> +ISL76683_SYSFS_STORE(threshold_low, ISL76683_THR_MAX)
> +ISL76683_SYSFS_STORE(threshold_up, ISL76683_THR_MAX)
> +
> +#define ISL76683_SYSFS_SHOW(ident, show_val) \
> +static ssize_t in_illuminance_##ident##_show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev); \
> + struct isl76683_chip *chip = iio_priv(indio_dev); \
> + \
> + return snprintf(buf, PAGE_SIZE, "%i\n", show_val); \
> +}
> +
> +ISL76683_SYSFS_SHOW(threshold_up, chip->threshold_up)
> +ISL76683_SYSFS_SHOW(threshold_low, chip->threshold_low)
> +
> static IIO_CONST_ATTR(in_illuminance_scale_available,
> ISL76683_LUXRANGE_STR);
> +static IIO_DEVICE_ATTR_RW(in_illuminance_threshold_up, 0);
> +static IIO_DEVICE_ATTR_RW(in_illuminance_threshold_low, 0);
>
> static struct attribute *isl76683_attributes[] = {
> &iio_const_attr_in_illuminance_scale_available.dev_attr.attr,
> + &iio_dev_attr_in_illuminance_threshold_up.dev_attr.attr,
> + &iio_dev_attr_in_illuminance_threshold_low.dev_attr.attr,
> NULL
> };
>
> @@ -508,6 +557,8 @@ static int isl76683_probe(struct i2c_client *client,
> chip->luxrange = ISL76683_LUX_RANGE_DEFAULT;
> chip->external_resistor = rs;
> chip->photodiode = ISL76683_DIODE_DEFAULT;
> + chip->threshold_up = ISL76683_THR_DEFAULT;
> + chip->threshold_low = ISL76683_THR_DEFAULT;
>
> chip->rmp = devm_regmap_init_i2c(client, &isl76683_regmap_config);
> if (IS_ERR(chip->rmp)) {
^ permalink raw reply
* Re: [PATCH v3 4/4] iio: adc: at91-sama5d2_adc: ack DRDY irq in direct mode
From: Jonathan Cameron @ 2017-11-19 11:28 UTC (permalink / raw)
To: Eugen Hristev
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA, lars-Qo5EllUWu/uELgA04lAiVw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA
In-Reply-To: <1510750608-8697-5-git-send-email-eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
On Wed, 15 Nov 2017 14:56:48 +0200
Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
> Need to acknowledge DRDY irq in direct mode/ software
> triggered mode. Otherwise, on the next conversion, overrun
> flag will be raised, which is not a correct state.
> This doesn't affect the functionality, but will generate
> possible incorrect overrun reports.
>
> Signed-off-by: Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
Applied. Thanks.
> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 11d34a8..274cb5e 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -787,6 +787,9 @@ static int at91_adc_read_raw(struct iio_dev *indio_dev,
> at91_adc_writel(st, AT91_SAMA5D2_IDR, BIT(chan->channel));
> at91_adc_writel(st, AT91_SAMA5D2_CHDR, BIT(chan->channel));
>
> + /* Needed to ACK the DRDY interruption */
> + at91_adc_readl(st, AT91_SAMA5D2_LCDR);
> +
> mutex_unlock(&st->lock);
>
> iio_device_release_direct_mode(indio_dev);
--
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 v4 02/12] docs: driver-api: add iio hw consumer section
From: Jonathan Cameron @ 2017-11-19 12:31 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Rob Herring, Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Maxime Coquelin,
Alexandre Torgue
In-Reply-To: <1510222354-15290-3-git-send-email-arnaud.pouliquen-qxv4g6HH51o@public.gmane.org>
On Thu, 9 Nov 2017 11:12:24 +0100
Arnaud Pouliquen <arnaud.pouliquen-qxv4g6HH51o@public.gmane.org> wrote:
> This adds a section about the Hardware consumer
> Api of the IIO subsystem to the driver API
> documentation.
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen-qxv4g6HH51o@public.gmane.org>
Good little bit of docs. A couple of minor formatting
comments inline.
Jonathan
> ---
> Documentation/driver-api/iio/hw-consumer.rst | 50 ++++++++++++++++++++++++++++
> Documentation/driver-api/iio/index.rst | 1 +
> 2 files changed, 51 insertions(+)
> create mode 100644 Documentation/driver-api/iio/hw-consumer.rst
>
> diff --git a/Documentation/driver-api/iio/hw-consumer.rst b/Documentation/driver-api/iio/hw-consumer.rst
> new file mode 100644
> index 0000000..b777133
> --- /dev/null
> +++ b/Documentation/driver-api/iio/hw-consumer.rst
> @@ -0,0 +1,50 @@
> +===========
> +HW consumer
> +===========
> +An IIO device can be directly connected to another device in hardware. in this
> +case the buffers between IIO provider and IIO consumer are handled by hardware.
> +The Industrial I/O hw consumer offers a way to bond these IIO devices without
> +software buffer for data. The implementation can be found under
> +:file:`drivers/iio/buffer/hw-consumer.c`
> +
> +
> +* struct :c:type:`iio_hw_consumer` — Hardware consumer structure
> +* :c:func:`iio_hw_consumer_alloc` — Allocate IIO hardware consumer
> +* :c:func:`iio_hw_consumer_free` — Free IIO hardware consumer
> +* :c:func:`iio_hw_consumer_enable` — Enable IIO hardware consumer
> +* :c:func:`iio_hw_consumer_disable` — Disable IIO hardware consumer
> +
> +
> +HW consumer setup
> +=================
> +
> +As standard IIO device the implementation is based on IIO provider/consumer.
> +A typical IIO Hw conumer setup looks like this::
HW
> +
> + static struct iio_hw_consumer *hwc;
> +
> + static const struct iio_info adc_info = {
> + .read_raw = adc_read_raw,
> + };
> +
> + static int adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> + {
> + ret = iio_hw_consumer_enable(hwc);
> +
> + /* Acquire data */
Blank line here to make it clear the Acquire data doesn't apply to
the line below but is representing a missing block.
> + ret = iio_hw_consumer_disable(hwc);
> + }
> +
> + static int adc_probe(struct platform_device *pdev)
> + {
> + hwc = devm_iio_hw_consumer_alloc(&iio->dev);
> + }
> +
> +More details
> +============
> +.. kernel-doc:: include/linux/iio/hw-consumer.h
> +.. kernel-doc:: drivers/iio/buffer/industrialio-hw-consumer.c
> + :export:
> +
> diff --git a/Documentation/driver-api/iio/index.rst b/Documentation/driver-api/iio/index.rst
> index e5c3922..7fba341 100644
> --- a/Documentation/driver-api/iio/index.rst
> +++ b/Documentation/driver-api/iio/index.rst
> @@ -15,3 +15,4 @@ Contents:
> buffers
> triggers
> triggered-buffers
> + hw-consumer
--
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 v4 01/12] iio: Add hardware consumer buffer support
From: Jonathan Cameron @ 2017-11-19 12:32 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Rob Herring, Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Maxime Coquelin,
Alexandre Torgue
In-Reply-To: <1510222354-15290-2-git-send-email-arnaud.pouliquen-qxv4g6HH51o@public.gmane.org>
On Thu, 9 Nov 2017 11:12:23 +0100
Arnaud Pouliquen <arnaud.pouliquen-qxv4g6HH51o@public.gmane.org> wrote:
> From: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
>
> Hardware consumer interface can be used when one IIO device has
> a direct connection to another device in hardware.
>
> Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen-qxv4g6HH51o@public.gmane.org>
I think this is fine though it is something we will have to
keep an eye on to make sure it is appropriately used...
Jonathan
> ---
> drivers/iio/buffer/Kconfig | 10 ++
> drivers/iio/buffer/Makefile | 1 +
> drivers/iio/buffer/industrialio-hw-consumer.c | 182 ++++++++++++++++++++++++++
> include/linux/iio/hw-consumer.h | 20 +++
> 4 files changed, 213 insertions(+)
> create mode 100644 drivers/iio/buffer/industrialio-hw-consumer.c
> create mode 100644 include/linux/iio/hw-consumer.h
>
> diff --git a/drivers/iio/buffer/Kconfig b/drivers/iio/buffer/Kconfig
> index 4ffd3db..338774c 100644
> --- a/drivers/iio/buffer/Kconfig
> +++ b/drivers/iio/buffer/Kconfig
> @@ -29,6 +29,16 @@ config IIO_BUFFER_DMAENGINE
>
> Should be selected by drivers that want to use this functionality.
>
> +config IIO_BUFFER_HW_CONSUMER
> + tristate "Industrial I/O HW buffering"
> + help
> + Provides a way to bonding when an IIO device has a direct connection
> + to another device in hardware. In this case buffers for data transfers
> + are handled by hardware.
> +
> + Should be selected by drivers that want to use the generic Hw consumer
> + interface.
> +
> config IIO_KFIFO_BUF
> tristate "Industrial I/O buffering based on kfifo"
> help
> diff --git a/drivers/iio/buffer/Makefile b/drivers/iio/buffer/Makefile
> index 85beaae..324a36b 100644
> --- a/drivers/iio/buffer/Makefile
> +++ b/drivers/iio/buffer/Makefile
> @@ -6,5 +6,6 @@
> obj-$(CONFIG_IIO_BUFFER_CB) += industrialio-buffer-cb.o
> obj-$(CONFIG_IIO_BUFFER_DMA) += industrialio-buffer-dma.o
> obj-$(CONFIG_IIO_BUFFER_DMAENGINE) += industrialio-buffer-dmaengine.o
> +obj-$(CONFIG_IIO_BUFFER_HW_CONSUMER) += industrialio-hw-consumer.o
> obj-$(CONFIG_IIO_TRIGGERED_BUFFER) += industrialio-triggered-buffer.o
> obj-$(CONFIG_IIO_KFIFO_BUF) += kfifo_buf.o
> diff --git a/drivers/iio/buffer/industrialio-hw-consumer.c b/drivers/iio/buffer/industrialio-hw-consumer.c
> new file mode 100644
> index 0000000..7d4d800
> --- /dev/null
> +++ b/drivers/iio/buffer/industrialio-hw-consumer.c
> @@ -0,0 +1,182 @@
> +/*
> + * Copyright 2017 Analog Devices Inc.
> + * Author: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/hw-consumer.h>
> +#include <linux/iio/buffer_impl.h>
> +
> +/**
> + * struct iio_hw_consumer - IIO hw consumer block
> + * @buffers: hardware buffers list head.
> + * @channels: IIO provider channels.
> + */
> +struct iio_hw_consumer {
> + struct list_head buffers;
> + struct iio_channel *channels;
> +};
> +
> +struct hw_consumer_buffer {
> + struct list_head head;
> + struct iio_dev *indio_dev;
> + struct iio_buffer buffer;
> + long scan_mask[];
> +};
> +
> +static struct hw_consumer_buffer *iio_buffer_to_hw_consumer_buffer(
> + struct iio_buffer *buffer)
> +{
> + return container_of(buffer, struct hw_consumer_buffer, buffer);
> +}
> +
> +static void iio_hw_buf_release(struct iio_buffer *buffer)
> +{
> + struct hw_consumer_buffer *hw_buf =
> + iio_buffer_to_hw_consumer_buffer(buffer);
> + kfree(hw_buf);
> +}
> +
> +static const struct iio_buffer_access_funcs iio_hw_buf_access = {
> + .release = &iio_hw_buf_release,
> + .modes = INDIO_BUFFER_HARDWARE,
> +};
> +
> +static struct hw_consumer_buffer *iio_hw_consumer_get_buffer(
> + struct iio_hw_consumer *hwc, struct iio_dev *indio_dev)
> +{
> + size_t mask_size = BITS_TO_LONGS(indio_dev->masklength) * sizeof(long);
> + struct hw_consumer_buffer *buf;
> +
> + list_for_each_entry(buf, &hwc->buffers, head) {
> + if (buf->indio_dev == indio_dev)
> + return buf;
> + }
> +
> + buf = kzalloc(sizeof(*buf) + mask_size, GFP_KERNEL);
> + if (!buf)
> + return NULL;
> +
> + buf->buffer.access = &iio_hw_buf_access;
> + buf->indio_dev = indio_dev;
> + buf->buffer.scan_mask = buf->scan_mask;
> +
> + iio_buffer_init(&buf->buffer);
> + list_add_tail(&buf->head, &hwc->buffers);
> +
> + return buf;
> +}
> +
> +/**
> + * iio_hw_consumer_alloc() - Allocate IIO hardware consumer
> + * @dev: Pointer to consumer device.
> + *
> + * Returns a valid iio_hw_consumer on success or a ERR_PTR() on failure.
> + */
> +struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev)
> +{
> + struct hw_consumer_buffer *buf;
> + struct iio_hw_consumer *hwc;
> + struct iio_channel *chan;
> + int ret;
> +
> + hwc = kzalloc(sizeof(*hwc), GFP_KERNEL);
> + if (!hwc)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(&hwc->buffers);
> +
> + hwc->channels = iio_channel_get_all(dev);
> + if (IS_ERR(hwc->channels)) {
> + ret = PTR_ERR(hwc->channels);
> + goto err_free_hwc;
> + }
> +
> + chan = &hwc->channels[0];
> + while (chan->indio_dev) {
> + buf = iio_hw_consumer_get_buffer(hwc, chan->indio_dev);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto err_put_buffers;
> + }
> + set_bit(chan->channel->scan_index, buf->buffer.scan_mask);
> + chan++;
> + }
> +
> + return hwc;
> +
> +err_put_buffers:
> + list_for_each_entry(buf, &hwc->buffers, head)
> + iio_buffer_put(&buf->buffer);
> + iio_channel_release_all(hwc->channels);
> +err_free_hwc:
> + kfree(hwc);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(iio_hw_consumer_alloc);
> +
> +/**
> + * iio_hw_consumer_free() - Free IIO hardware consumer
> + * @hwc: hw consumer to free.
> + */
> +void iio_hw_consumer_free(struct iio_hw_consumer *hwc)
> +{
> + struct hw_consumer_buffer *buf;
> +
> + iio_channel_release_all(hwc->channels);
> + list_for_each_entry(buf, &hwc->buffers, head)
> + iio_buffer_put(&buf->buffer);
> + kfree(hwc);
> +}
> +EXPORT_SYMBOL_GPL(iio_hw_consumer_free);
> +
> +/**
> + * iio_hw_consumer_enable() - Enable IIO hardware consumer
> + * @hwc: iio_hw_consumer to enable.
> + *
> + * Returns 0 on success.
> + */
> +int iio_hw_consumer_enable(struct iio_hw_consumer *hwc)
> +{
> + struct hw_consumer_buffer *buf;
> + int ret;
> +
> + list_for_each_entry(buf, &hwc->buffers, head) {
> + ret = iio_update_buffers(buf->indio_dev, &buf->buffer, NULL);
> + if (ret)
> + goto err_disable_buffers;
> + }
> +
> + return 0;
> +
> +err_disable_buffers:
> + list_for_each_entry_continue_reverse(buf, &hwc->buffers, head)
> + iio_update_buffers(buf->indio_dev, NULL, &buf->buffer);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_hw_consumer_enable);
> +
> +/**
> + * iio_hw_consumer_disable() - Disable IIO hardware consumer
> + * @hwc: iio_hw_consumer to disable.
> + */
> +void iio_hw_consumer_disable(struct iio_hw_consumer *hwc)
> +{
> + struct hw_consumer_buffer *buf;
> +
> + list_for_each_entry(buf, &hwc->buffers, head)
> + iio_update_buffers(buf->indio_dev, NULL, &buf->buffer);
> +}
> +EXPORT_SYMBOL_GPL(iio_hw_consumer_disable);
> +
> +MODULE_AUTHOR("Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>");
> +MODULE_DESCRIPTION("Hardware consumer buffer the IIO framework");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/hw-consumer.h b/include/linux/iio/hw-consumer.h
> new file mode 100644
> index 0000000..f16791b
> --- /dev/null
> +++ b/include/linux/iio/hw-consumer.h
> @@ -0,0 +1,20 @@
> +/*
> + * Industrial I/O in kernel hardware consumer interface
> + *
> + * Copyright 2017 Analog Devices Inc.
> + * Author: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#ifndef LINUX_IIO_HW_CONSUMER_H
> +#define LINUX_IIO_HW_CONSUMER_H
> +
> +struct iio_hw_consumer;
> +
> +struct iio_hw_consumer *iio_hw_consumer_alloc(struct device *dev);
> +void iio_hw_consumer_free(struct iio_hw_consumer *hwc);
> +int iio_hw_consumer_enable(struct iio_hw_consumer *hwc);
> +void iio_hw_consumer_disable(struct iio_hw_consumer *hwc);
> +
> +#endif
^ 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