* Re: [PATCH 1/5] iio: xoadc: augment DT bindings a bit
From: Bjorn Andersson @ 2017-03-26 4:57 UTC (permalink / raw)
To: Linus Walleij
Cc: Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170318133358.22314-1-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Sat 18 Mar 06:33 PDT 2017, Linus Walleij wrote:
> In order to accommodate in a logical manner for the premuxed channels
> in PM8921 and the similarly addressed channels in later PMICs, we
> need a twocell arrangement with premux and analog mux setting as
> a tuple to uniquely identify a hardware channel.
>
> These bindings are not yet in use, so it should be find to augment
s/find/fine?
> them before we actually start using it in drivers and device trees.
>
> This scheme came out of lengthy discussions and reverse-engineering
> and reading of the few information sources we have.
>
As there's no documentation of how this works I find it quite important
to make something that is intuitive - so more than you and I can use
this binding. So I like this.
> Suggested-by: Björn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/iio/adc/qcom,pm8xxx-xoadc.txt | 117 ++++++++++++---------
> 1 file changed, 69 insertions(+), 48 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> index 53cd146d8096..680bb7a29dd5 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> @@ -19,32 +19,42 @@ Required properties:
> with PMIC variant but is typically something like 2.2 or 1.8V.
>
> The following required properties are standard for IO channels, see
> -iio-bindings.txt for more details:
> +iio-bindings.txt for more details, but noitice that this particular
> +ADC has a special adressing scheme that require two cells for
> +identifying each ADC channel:
>
> -- #address-cells: should be set to <1>
> +- #address-cells: should be set to <2>, the first cell is the
> + prescaler (on PM8058) or premux (on PM8921) with two valid bits
> + so legal values are 0x00, 0x01 or 0x02. The second cell
> + is the main analog mux setting (0x00..0x0f). The combination
> + of prescaler/premux and analog mux uniquely addresses a hardware
> + channel on all systems.
>
> - #size-cells: should be set to <0>
>
> -- #io-channel-cells: should be set to <1>
> +- #io-channel-cells: should be set to <2>, again the cells are
> + precaler or premux followed by the analog muxing line.
>
> - interrupts: should refer to the parent PMIC interrupt controller
> and reference the proper ADC interrupt.
>
> Required subnodes:
>
> -The ADC channels are configured as subnodes of the ADC. Since some of
> -them are used for calibrating the ADC, these nodes are compulsory:
> +The ADC channels are configured as subnodes of the ADC.
>
> -adc-channel@c {
> - reg = <0x0c>;
> +Since some of them are used for calibrating the ADC, these nodes are
> +compulsory:
> +
> +adc-channel@0c {
Shouldn't the @ be followed by the value of the first cell in "reg"?
(Which will collide if we name them all "adc-channel").
> + reg = <0x00 0x0c>;
> };
>
> -adc-channel@d {
> - reg = <0x0d>;
> +adc-channel@0d {
> + reg = <0x00 0x0d>;
> };
>
> -adc-channel@f {
> - reg = <0x0f>;
> +adc-channel@0f {
> + reg = <0x00 0x0f>;
> };
>
> These three nodes are used for absolute and ratiometric calibration
> @@ -52,13 +62,26 @@ and only need to have these reg values: they are by hardware definition
> 1:1 ratio converters that sample 625, 1250 and 0 milliV and create
> an interpolation calibration for all other ADCs.
>
> -Optional subnodes: any channels other than channel 0x0c, 0x0d and
> -0x0f are optional.
> +Optional subnodes: any channels other than channels [0x00 0x0c],
> +[0x00 0x0d] and [0x00 0x0f] are optional.
>
> Required channel node properties:
>
> - reg: should contain the hardware channel number in the range
> - 0 .. 0x0f (4 bits). The hardware only supports 16 channels.
> + 0 .. 0xff (8 bits).
> +
> + On PM8058 the hardware only supports 16 channels, but we get the same
> + channels repeating with its input divided down by 1 or 3. Channels 00,
> + 10, 20, ... f0 are the raw values, 04, 14, 24 .. f4 are "unity" channels
> + divided by 1, and 08, 18, 28 .. f8 are channels divided by 3. Bits 0
> + and 1 of the channel index should always be 0.
> +
> + On PM8921 the hardware supports more than 16 channels through a complex
> + routing matrix using a premux, so 00, 10, 20 .. f0 are the basic raw
> + channels while another set of channels appear for 04, 14, 24 .. f4,
> + and again some of the same channels appear again divided down by 3
> + in 08, 18, 28 .. f8. Again bits 0 and 1 of the channel index should
> + always be 0.
While I believe documenting this is a good thing I do not think it adds
value to this binding document (other than showing Rob the absurdness of
the addressing scheme). Please consider moving it to the driver (haven't
checked how you commented it yet) and drop it from here.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v2 1/2] soc: qcom: Add device tree binding for GLINK RPM
From: Bjorn Andersson @ 2017-03-26 4:31 UTC (permalink / raw)
To: Rob Herring
Cc: Andy Gross, Mark Rutland, Ohad Ben-Cohen, linux-arm-msm,
linux-soc, devicetree, linux-kernel, linux-remoteproc
In-Reply-To: <20170324160731.caxbdgpqm7u35po6@rob-hp-laptop>
On Fri 24 Mar 09:07 PDT 2017, Rob Herring wrote:
> On Mon, Mar 20, 2017 at 02:09:55PM -0700, Bjorn Andersson wrote:
> > Add device tree binding documentation for the Qualcomm GLINK RPM, used
> > for communication with the Resource Power Management subsystem in
> > various Qualcomm SoCs.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v1:
> > - None
> >
> > .../devicetree/bindings/soc/qcom/qcom,glink.txt | 73 ++++++++++++++++++++++
> > 1 file changed, 73 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
> >
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
> > new file mode 100644
> > index 000000000000..da92c4f787f3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
> > @@ -0,0 +1,73 @@
> > +Qualcomm RPM GLINK binding
> > +
> > +This binding describes the Qualcomm RPM GLINK, a fifo based mechanism for
> > +communication with the Resource Power Management system on various Qualcomm
> > +platforms.
> > +
> > +- compatible:
> > + Usage: required
> > + Value type: <stringlist>
> > + Definition: must be "qcom,glink-rpm"
>
> SoC specific compatibles please.
>
In addition to the generic qcom,glink-rpm I presume?
> > +
> > +- interrupts:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: should specify the IRQ used by the remote processor to
> > + signal this processor about communication related events
> > +
> > +- qcom,rpm-msg-ram:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: handle to RPM message memory resource
> > +
> > +- qcom,ipc:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: three entries specifying the outgoing ipc bit used for
> > + signaling the remote processor:
> > + - phandle to a syscon node representing the apcs registers
> > + - u32 representing offset to the register within the syscon
> > + - u32 representing the ipc bit within the register
> > +
> > += GLINK DEVICES
> > +Each subnode of the GLINK node represent function tied to a virtual
> > +communication channel. The name of the nodes are not important. The properties
> > +of these nodes are defined by the individual bindings for the specific function
> > +- but must contain the following property:
> > +
> > +- qcom,glink-channels:
> > + Usage: required
> > + Value type: <stringlist>
> > + Definition: a list of channels tied to this function, used for matching
> > + the function to a set of virtual channels
> > +
> > += EXAMPLE
> > +The following example represents the GLINK RPM node on a MSM8996 device, with
> > +the function for the "rpm_request" channel defined, which is used for
> > +regualtors and root clocks.
> > +
> > + apcs: syscon@f9011000 {
> > + compatible = "syscon";
>
> syscon alone is not valid.
>
This is equivalent to how we have done this on all previous platforms.
On previous platforms this generally maps to one of the "Krait Processor
SubSystem" blocks, so I'm quite worried with coming up with a compatible
for this block will not be compatible with any future description of
this block.
> > + reg = <0xf9011000 0x1000>;
> > + };
> > +
> > + rpm_msg_ram: memory@68000 {
> > + compatible = "qcom,rpm-msg-ram";
>
> Is this more than just mmio-sram?
>
> Or with a fixed use could be part of another node?
>
It represents one of the RAM modules of one of the other ARM cores in
the SoC, used for shared memory communication with this ARM core. So the
hardware is essentially mmio-sram. But it has none of the properties
that a mmio-sram seem to have.
The reason I did not put it in the rpm-glink node is that in previous
Qualcomm platforms we have exactly the same system design, but the data
structure of the RAM was different, so it felt natural to just keep the
same description of the hardware.
> > + reg = <0x68000 0x6000>;
> > + };
> > +
> > + rpm-glink {
> > + compatible = "qcom,glink-rpm";
> > +
> > + interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> > +
> > + qcom,rpm-msg-ram = <&rpm_msg_ram>;
> > + qcom,ipc = <&apcs 0x10 0>;
> > +
> > + rpm-requests {
> > + compatible = "qcom,rpm-msm8996";
> > + qcom,glink-channels = "rpm_requests";
> > +
> > + ...
> > + };
> > + };
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support
From: kbuild test robot @ 2017-03-26 1:41 UTC (permalink / raw)
To: Thierry Escande
Cc: kbuild-all, Rob Herring, Greg Kroah-Hartman, Olof Johansson,
Stephen Warren, linux-kernel, devicetree, Julius Werner,
Brian Norris
In-Reply-To: <1490303069-13230-6-git-send-email-thierry.escande@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 3267 bytes --]
Hi Thierry,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Thierry-Escande/firmware-google-memconsole/20170326-080828
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All errors (new ones prefixed by >>):
drivers/firmware/google/gsmi.c: In function 'gsmi_exec.constprop':
>> drivers/firmware/google/gsmi.c:198:3: error: inconsistent operand constraints in an 'asm'
asm volatile (
^~~
drivers/firmware/google/gsmi.c:213:3: error: inconsistent operand constraints in an 'asm'
asm volatile (
^~~
drivers/firmware/google/gsmi.c:229:3: error: inconsistent operand constraints in an 'asm'
asm volatile (
^~~
vim +/asm +198 drivers/firmware/google/gsmi.c
74c5b31c Mike Waychison 2011-04-29 182 /*
74c5b31c Mike Waychison 2011-04-29 183 * AH : Subfunction number
74c5b31c Mike Waychison 2011-04-29 184 * AL : Function number
74c5b31c Mike Waychison 2011-04-29 185 * EBX : Parameter block address
74c5b31c Mike Waychison 2011-04-29 186 * DX : SMI command port
74c5b31c Mike Waychison 2011-04-29 187 *
74c5b31c Mike Waychison 2011-04-29 188 * Three protocols here. See also the comment in gsmi_init().
74c5b31c Mike Waychison 2011-04-29 189 */
74c5b31c Mike Waychison 2011-04-29 190 if (gsmi_dev.handshake_type == GSMI_HANDSHAKE_CF) {
74c5b31c Mike Waychison 2011-04-29 191 /*
74c5b31c Mike Waychison 2011-04-29 192 * If handshake_type == HANDSHAKE_CF then set CF on the
74c5b31c Mike Waychison 2011-04-29 193 * way in and wait for the handler to clear it; this avoids
74c5b31c Mike Waychison 2011-04-29 194 * corrupting register state on those chipsets which have
74c5b31c Mike Waychison 2011-04-29 195 * a delay between writing the SMI trigger register and
74c5b31c Mike Waychison 2011-04-29 196 * entering SMM.
74c5b31c Mike Waychison 2011-04-29 197 */
74c5b31c Mike Waychison 2011-04-29 @198 asm volatile (
74c5b31c Mike Waychison 2011-04-29 199 "stc\n"
74c5b31c Mike Waychison 2011-04-29 200 "outb %%al, %%dx\n"
74c5b31c Mike Waychison 2011-04-29 201 "1: jc 1b\n"
74c5b31c Mike Waychison 2011-04-29 202 : "=a" (result)
74c5b31c Mike Waychison 2011-04-29 203 : "0" (cmd),
74c5b31c Mike Waychison 2011-04-29 204 "d" (gsmi_dev.smi_cmd),
74c5b31c Mike Waychison 2011-04-29 205 "b" (gsmi_dev.param_buf->address)
74c5b31c Mike Waychison 2011-04-29 206 : "memory", "cc"
:::::: The code at line 198 was first introduced by commit
:::::: 74c5b31c6618f01079212332b2e5f6c42f2d6307 driver: Google EFI SMI
:::::: TO: Mike Waychison <mikew@google.com>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46805 bytes --]
^ permalink raw reply
* Re: [PATCHv2] phy: cpcap-usb: Add CPCAP PMIC USB support
From: Tony Lindgren @ 2017-03-25 22:10 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Kishon Vijay Abraham I, linux-kernel, linux-usb,
linux-omap, devicetree, Marcel Partap, Michael Scott
In-Reply-To: <201703260432.1EcS6tcy%fengguang.wu@intel.com>
* kbuild test robot <lkp@intel.com> [170325 13:45]:
> Hi Tony,
>
> [auto build test ERROR on phy/next]
> [also build test ERROR on v4.11-rc3 next-20170324]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/phy-cpcap-usb-Add-CPCAP-PMIC-USB-support/20170324-061112
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
> config: x86_64-randconfig-a0-03260238 (attached as .config)
> compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/built-in.o: In function `cpcap_usb_detect':
> >> phy-cpcap-usb.c:(.text+0x18ad): undefined reference to `iio_read_channel_processed'
> drivers/built-in.o: In function `cpcap_usb_phy_probe':
> >> phy-cpcap-usb.c:(.text+0x1c5e): undefined reference to `devm_iio_channel_get'
> >> phy-cpcap-usb.c:(.text+0x1c92): undefined reference to `iio_get_channel_type'
OK thanks for the report, we need IIO for VBUS level, here's
a patch to fix it.
Regards,
Tony
8< -----------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Sat, 25 Mar 2017 15:04:37 -0700
Subject: [PATCH] phy: cpcap-usb: Fix build by adding a dependency to
CONFIG_IIO
We need the ADC output provided by IIO to detect VBUS level.
Otherwise we can get:
drivers/built-in.o: In function `cpcap_usb_detect':
phy-cpcap-usb.c:(.text+0x18ad): undefined reference to
`iio_read_channel_processed'
drivers/built-in.o: In function `cpcap_usb_phy_probe':
phy-cpcap-usb.c:(.text+0x1c5e): undefined reference to
`devm_iio_channel_get'
phy-cpcap-usb.c:(.text+0x1c92): undefined reference to`iio_get_channel_type'
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/phy/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -49,7 +49,7 @@ config PHY_BERLIN_SATA
config PHY_CPCAP_USB
tristate "CPCAP USB PHY driver"
- depends on USB_SUPPORT
+ depends on USB_SUPPORT && IIO
select GENERIC_PHY
select USB_PHY
help
--
2.12.1
^ permalink raw reply
* Re: [PATCHv2] phy: cpcap-usb: Add CPCAP PMIC USB support
From: kbuild test robot @ 2017-03-25 20:42 UTC (permalink / raw)
To: Tony Lindgren
Cc: kbuild-all-JC7UmRfGjtg, Kishon Vijay Abraham I,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Marcel Partap, Michael Scott
In-Reply-To: <20170322234602.5888-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]
Hi Tony,
[auto build test ERROR on phy/next]
[also build test ERROR on v4.11-rc3 next-20170324]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/phy-cpcap-usb-Add-CPCAP-PMIC-USB-support/20170324-061112
base: https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
config: x86_64-randconfig-a0-03260238 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `cpcap_usb_detect':
>> phy-cpcap-usb.c:(.text+0x18ad): undefined reference to `iio_read_channel_processed'
drivers/built-in.o: In function `cpcap_usb_phy_probe':
>> phy-cpcap-usb.c:(.text+0x1c5e): undefined reference to `devm_iio_channel_get'
>> phy-cpcap-usb.c:(.text+0x1c92): undefined reference to `iio_get_channel_type'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25513 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] iio: adc: meson-saradc: add Meson8b SoC compatibility
From: Jonathan Cameron @ 2017-03-25 18:31 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-iio-u79uwXL29TY76Z2rM5mHXA,
carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAFBinCDVLZdBEX=UnGKFCkRF02YYx4hoRw54+AcwAku9zGjvaQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 25/03/17 18:28, Martin Blumenstingl wrote:
> On Sat, Mar 25, 2017 at 7:13 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On 25/03/17 16:29, Martin Blumenstingl wrote:
>>> Meson GX SoCs however use some magic bits to prevent simultaneous (=
>>> conflicting, because only consumer should use the FIFO buffer with the
>>> ADC results) usage by the Linux kernel and the bootloader (the BL30
>>> bootloader uses the SAR ADC to read the CPU temperature).
>>> This patch changes guards all BL30 functionality so it is skipped on
>>> SoCs which don't have it. Since the hardware itself doesn't know whether
>>> BL30 is available the internal meson_sar_adc_data is extended so this
>>> information can be provided per of_device_id.data inside the driver.
>>>
>>> Additionally the clocks "adc_clk" and "adc_sel" are not provided by the
>>> clock-controller itself. "adc_sel" is not available at all. "adc_clk"
>>> is provided by the SAR ADC IP block itself on Meson8b (and earlier).
>>> This is already supported by the meson_saradc driver.
>>>
>>> Finally a new of_device_id for the Meson8b SoC is added so it can be
>>> wired up in the corresponding DT.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> Looks sensible to me. Just fix those bits from Peter and we'll let it sit
>> a few days on the list to see if anyone else wants to chip in.
> OK, we'll have to wait for one of the DT-maintainers ACK (on the other
> patch) anyways, so I'll re-spin this in a week or so and fix
> everything that has come up until then
There is some flexibility in 'trivial' dt bindings such as this so as
not to waste their time. But we should allow them the opportunity before
applying it anyway ;)
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
>
--
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 2/2] iio: adc: meson-saradc: add Meson8b SoC compatibility
From: Martin Blumenstingl @ 2017-03-25 18:28 UTC (permalink / raw)
To: Jonathan Cameron
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-iio-u79uwXL29TY76Z2rM5mHXA,
carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <231f1fef-e947-3461-2e5b-99d80abfa73a-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Sat, Mar 25, 2017 at 7:13 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On 25/03/17 16:29, Martin Blumenstingl wrote:
>> Meson GX SoCs however use some magic bits to prevent simultaneous (=
>> conflicting, because only consumer should use the FIFO buffer with the
>> ADC results) usage by the Linux kernel and the bootloader (the BL30
>> bootloader uses the SAR ADC to read the CPU temperature).
>> This patch changes guards all BL30 functionality so it is skipped on
>> SoCs which don't have it. Since the hardware itself doesn't know whether
>> BL30 is available the internal meson_sar_adc_data is extended so this
>> information can be provided per of_device_id.data inside the driver.
>>
>> Additionally the clocks "adc_clk" and "adc_sel" are not provided by the
>> clock-controller itself. "adc_sel" is not available at all. "adc_clk"
>> is provided by the SAR ADC IP block itself on Meson8b (and earlier).
>> This is already supported by the meson_saradc driver.
>>
>> Finally a new of_device_id for the Meson8b SoC is added so it can be
>> wired up in the corresponding DT.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Looks sensible to me. Just fix those bits from Peter and we'll let it sit
> a few days on the list to see if anyone else wants to chip in.
OK, we'll have to wait for one of the DT-maintainers ACK (on the other
patch) anyways, so I'll re-spin this in a week or so and fix
everything that has come up until then
^ permalink raw reply
* [PATCH v5 9/9] arm64: dts: qcom: msm8916: Add debug unit
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
mike.leach-QSEj5FYQhm4dnm+yROfE0A, Suzuki.Poulose-5wv7dgnIgG8,
sudeep.holla-5wv7dgnIgG8
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Add debug unit on Qualcomm msm8916 based platforms, including the
DragonBoard 410c board.
Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 68a8e67..3af814b 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1104,6 +1104,38 @@
};
};
+ debug@850000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0x850000 0x1000>;
+ clocks = <&rpmcc RPM_QDSS_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&CPU0>;
+ };
+
+ debug@852000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0x852000 0x1000>;
+ clocks = <&rpmcc RPM_QDSS_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&CPU1>;
+ };
+
+ debug@854000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0x854000 0x1000>;
+ clocks = <&rpmcc RPM_QDSS_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&CPU2>;
+ };
+
+ debug@856000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0x856000 0x1000>;
+ clocks = <&rpmcc RPM_QDSS_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&CPU3>;
+ };
+
etm@85c000 {
compatible = "arm,coresight-etm4x", "arm,primecell";
reg = <0x85c000 0x1000>;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 8/9] arm64: dts: hi6220: register debug module
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
Bind debug module driver for Hi6220.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 64 +++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 470461d..467aa15 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -913,5 +913,69 @@
};
};
};
+
+ debug@f6590000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf6590000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu0>;
+ };
+
+ debug@f6592000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf6592000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu1>;
+ };
+
+ debug@f6594000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf6594000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu2>;
+ };
+
+ debug@f6596000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf6596000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu3>;
+ };
+
+ debug@f65d0000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf65d0000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu4>;
+ };
+
+ debug@f65d2000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf65d2000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu5>;
+ };
+
+ debug@f65d4000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf65d4000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu6>;
+ };
+
+ debug@f65d6000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf65d6000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu7>;
+ };
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH v5 7/9] clk: hi6220: add debug APB clock
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
The debug APB clock is absent in hi6220 driver, so this patch is to add
support for it.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
drivers/clk/hisilicon/clk-hi6220.c | 1 +
include/dt-bindings/clock/hi6220-clock.h | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index c0e8e1f..2ae151c 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c
@@ -134,6 +134,7 @@ static struct hisi_gate_clock hi6220_separated_gate_clks_sys[] __initdata = {
{ HI6220_UART4_PCLK, "uart4_pclk", "uart4_src", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 8, 0, },
{ HI6220_SPI_CLK, "spi_clk", "clk_150m", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 9, 0, },
{ HI6220_TSENSOR_CLK, "tsensor_clk", "clk_bus", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x230, 12, 0, },
+ { HI6220_DAPB_CLK, "dapb_clk", "cs_dapb", CLK_SET_RATE_PARENT|CLK_IS_CRITICAL, 0x230, 18, 0, },
{ HI6220_MMU_CLK, "mmu_clk", "ddrc_axi1", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x240, 11, 0, },
{ HI6220_HIFI_SEL, "hifi_sel", "hifi_src", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x270, 0, 0, },
{ HI6220_MMC0_SYSPLL, "mmc0_syspll", "syspll", CLK_SET_RATE_PARENT|CLK_IGNORE_UNUSED, 0x270, 1, 0, },
diff --git a/include/dt-bindings/clock/hi6220-clock.h b/include/dt-bindings/clock/hi6220-clock.h
index 6b03c84..b8ba665 100644
--- a/include/dt-bindings/clock/hi6220-clock.h
+++ b/include/dt-bindings/clock/hi6220-clock.h
@@ -124,7 +124,10 @@
#define HI6220_CS_DAPB 57
#define HI6220_CS_ATB_DIV 58
-#define HI6220_SYS_NR_CLKS 59
+/* gate clock */
+#define HI6220_DAPB_CLK 59
+
+#define HI6220_SYS_NR_CLKS 60
/* clk in Hi6220 media controller */
/* gate clocks */
--
2.7.4
^ permalink raw reply related
* [PATCH v5 6/9] coresight: add support for CPU debug module
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
Coresight includes debug module and usually the module connects with CPU
debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
description for related info in "Part H: External Debug".
Chapter H7 "The Sample-based Profiling Extension" introduces several
sampling registers, e.g. we can check program counter value with
combined CPU exception level, secure state, etc. So this is helpful for
analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
loop with IRQ disabled. In this case the CPU cannot switch context and
handle any interrupt (including IPIs), as the result it cannot handle
SMP call for stack dump.
This patch is to enable coresight debug module, so firstly this driver
is to bind apb clock for debug module and this is to ensure the debug
module can be accessed from program or external debugger. And the driver
uses sample-based registers for debug purpose, e.g. when system triggers
panic, the driver will dump program counter and combined context
registers (EDCIDSR, EDVIDSR); by parsing context registers so can
quickly get to know CPU secure state, exception level, etc.
Some of the debug module registers are located in CPU power domain, so
this requires the CPU power domain stays on when access related debug
registers, but the power management for CPU power domain is quite
dependent on SoC integration for power management. For the platforms
which with sane power controller implementations, this driver follows
the method to set EDPRCR to try to pull the CPU out of low power state
and then set 'no power down request' bit so the CPU has no chance to
lose power.
If the SoC has not followed up this design well for power management
controller, the driver introduces module parameter "idle_constraint".
Setting this parameter for latency requirement in microseconds, finally
we can constrain all or partial idle states to ensure the CPU power
domain is enabled, this is a backup method to access coresight CPU
debug component safely.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
drivers/hwtracing/coresight/Kconfig | 11 +
drivers/hwtracing/coresight/Makefile | 1 +
drivers/hwtracing/coresight/coresight-cpu-debug.c | 704 ++++++++++++++++++++++
3 files changed, 716 insertions(+)
create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 130cb21..18d7931 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -89,4 +89,15 @@ config CORESIGHT_STM
logging useful software events or data coming from various entities
in the system, possibly running different OSs
+config CORESIGHT_CPU_DEBUG
+ tristate "CoreSight CPU Debug driver"
+ depends on ARM || ARM64
+ depends on DEBUG_FS
+ help
+ This driver provides support for coresight debugging module. This
+ is primarily used to dump sample-based profiling registers when
+ system triggers panic, the driver will parse context registers so
+ can quickly get to know program counter (PC), secure state,
+ exception level, etc.
+
endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index af480d9..433d590 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
coresight-etm4x-sysfs.o
obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
+obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c
new file mode 100644
index 0000000..fbec1d1
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
@@ -0,0 +1,704 @@
+/*
+ * Copyright (c) 2017 Linaro Limited. All rights reserved.
+ *
+ * Author: Leo Yan <leo.yan@linaro.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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#include <linux/amba/bus.h>
+#include <linux/coresight.h>
+#include <linux/cpu.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/pm_qos.h>
+#include <linux/slab.h>
+#include <linux/smp.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "coresight-priv.h"
+
+#define EDPCSR 0x0A0
+#define EDCIDSR 0x0A4
+#define EDVIDSR 0x0A8
+#define EDPCSR_HI 0x0AC
+#define EDOSLAR 0x300
+#define EDPRCR 0x310
+#define EDPRSR 0x314
+#define EDDEVID1 0xFC4
+#define EDDEVID 0xFC8
+
+#define EDPCSR_PROHIBITED 0xFFFFFFFF
+
+/* bits definition for EDPCSR */
+#ifndef CONFIG_64BIT
+#define EDPCSR_THUMB BIT(0)
+#define EDPCSR_ARM_INST_MASK GENMASK(31, 2)
+#define EDPCSR_THUMB_INST_MASK GENMASK(31, 1)
+#endif
+
+/* bits definition for EDPRCR */
+#define EDPRCR_COREPURQ BIT(3)
+#define EDPRCR_CORENPDRQ BIT(0)
+
+/* bits definition for EDPRSR */
+#define EDPRSR_DLK BIT(6)
+#define EDPRSR_PU BIT(0)
+
+/* bits definition for EDVIDSR */
+#define EDVIDSR_NS BIT(31)
+#define EDVIDSR_E2 BIT(30)
+#define EDVIDSR_E3 BIT(29)
+#define EDVIDSR_HV BIT(28)
+#define EDVIDSR_VMID GENMASK(7, 0)
+
+/*
+ * bits definition for EDDEVID1:PSCROffset
+ *
+ * NOTE: armv8 and armv7 have different definition for the register,
+ * so consolidate the bits definition as below:
+ *
+ * 0b0000 - Sample offset applies based on the instruction state, we
+ * rely on EDDEVID to check if EDPCSR is implemented or not
+ * 0b0001 - No offset applies.
+ * 0b0010 - No offset applies, but do not use in AArch32 mode
+ *
+ */
+#define EDDEVID1_PCSR_OFFSET_MASK GENMASK(3, 0)
+#define EDDEVID1_PCSR_OFFSET_INS_SET (0x0)
+#define EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32 (0x2)
+
+/* bits definition for EDDEVID */
+#define EDDEVID_PCSAMPLE_MODE GENMASK(3, 0)
+#define EDDEVID_IMPL_NONE (0x0)
+#define EDDEVID_IMPL_EDPCSR (0x1)
+#define EDDEVID_IMPL_EDPCSR_EDCIDSR (0x2)
+#define EDDEVID_IMPL_FULL (0x3)
+
+#define DEBUG_WAIT_TIMEOUT 32
+
+struct debug_drvdata {
+ void __iomem *base;
+ struct device *dev;
+ int cpu;
+
+ bool edpcsr_present;
+ bool edcidsr_present;
+ bool edvidsr_present;
+ bool pc_has_offset;
+
+ u32 eddevid;
+ u32 eddevid1;
+
+ u32 edpcsr;
+ u32 edpcsr_hi;
+ u32 edprcr;
+ u32 edprsr;
+ u32 edvidsr;
+ u32 edcidsr;
+};
+
+static DEFINE_MUTEX(debug_lock);
+static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
+static int debug_count;
+static struct dentry *debug_debugfs_dir;
+
+static struct pm_qos_request debug_qos_req;
+static int idle_constraint = PM_QOS_DEFAULT_VALUE;
+module_param(idle_constraint, int, 0600);
+MODULE_PARM_DESC(idle_constraint, "Latency requirement in microseconds for CPU "
+ "idle states (default is -1, which means have no limiation "
+ "to CPU idle states; 0 means disabling all idle states; user "
+ "can choose other platform dependent values so can disable "
+ "specific idle states for the platform)");
+
+static bool debug_enable;
+module_param_named(enable, debug_enable, bool, 0600);
+MODULE_PARM_DESC(enable, "Knob to enable debug functionality "
+ "(default is 0, which means is disabled by default)");
+
+static void debug_os_unlock(struct debug_drvdata *drvdata)
+{
+ /* Unlocks the debug registers */
+ writel_relaxed(0x0, drvdata->base + EDOSLAR);
+ wmb();
+}
+
+/*
+ * According to ARM DDI 0487A.k, before access external debug
+ * registers should firstly check the access permission; if any
+ * below condition has been met then cannot access debug
+ * registers to avoid lockup issue:
+ *
+ * - CPU power domain is powered off;
+ * - The OS Double Lock is locked;
+ *
+ * By checking EDPRSR can get to know if meet these conditions.
+ */
+static bool debug_access_permitted(struct debug_drvdata *drvdata)
+{
+ /* CPU is powered off */
+ if (!(drvdata->edprsr & EDPRSR_PU))
+ return false;
+
+ /* The OS Double Lock is locked */
+ if (drvdata->edprsr & EDPRSR_DLK)
+ return false;
+
+ return true;
+}
+
+static void debug_force_cpu_powered_up(struct debug_drvdata *drvdata)
+{
+ int timeout = DEBUG_WAIT_TIMEOUT;
+
+ drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
+
+ CS_UNLOCK(drvdata->base);
+
+ /* Bail out if CPU is powered up yet */
+ if (drvdata->edprsr & EDPRSR_PU)
+ goto out_powered_up;
+
+ /*
+ * Send request to power management controller and assert
+ * DBGPWRUPREQ signal; if power management controller has
+ * sane implementation, it should enable CPU power domain
+ * in case CPU is in low power state.
+ */
+ drvdata->edprsr = readl(drvdata->base + EDPRCR);
+ drvdata->edprsr |= EDPRCR_COREPURQ;
+ writel(drvdata->edprsr, drvdata->base + EDPRCR);
+
+ /* Wait for CPU to be powered up (timeout~=32ms) */
+ while (timeout--) {
+ drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
+ if (drvdata->edprsr & EDPRSR_PU)
+ break;
+
+ usleep_range(1000, 2000);
+ }
+
+ /*
+ * Unfortunately the CPU cannot be powered up, so return
+ * back and later has no permission to access other
+ * registers. For this case, should set 'idle_constraint'
+ * to ensure CPU power domain is enabled!
+ */
+ if (!(drvdata->edprsr & EDPRSR_PU)) {
+ pr_err("%s: power up request for CPU%d failed\n",
+ __func__, drvdata->cpu);
+ goto out;
+ }
+
+out_powered_up:
+ debug_os_unlock(drvdata);
+
+ /*
+ * At this point the CPU is powered up, so set the no powerdown
+ * request bit so we don't lose power and emulate power down.
+ */
+ drvdata->edprsr = readl(drvdata->base + EDPRCR);
+ drvdata->edprsr |= EDPRCR_COREPURQ | EDPRCR_CORENPDRQ;
+ writel(drvdata->edprsr, drvdata->base + EDPRCR);
+
+out:
+ CS_LOCK(drvdata->base);
+}
+
+static void debug_read_regs(struct debug_drvdata *drvdata)
+{
+ /*
+ * Ensure CPU power domain is enabled to let registers
+ * are accessiable.
+ */
+ debug_force_cpu_powered_up(drvdata);
+
+ if (!debug_access_permitted(drvdata))
+ return;
+
+ CS_UNLOCK(drvdata->base);
+
+ debug_os_unlock(drvdata);
+
+ drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
+
+ /*
+ * As described in ARM DDI 0487A.k, if the processing
+ * element (PE) is in debug state, or sample-based
+ * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
+ * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
+ * UNKNOWN state. So directly bail out for this case.
+ */
+ if (drvdata->edpcsr == EDPCSR_PROHIBITED)
+ goto out;
+
+ /*
+ * A read of the EDPCSR normally has the side-effect of
+ * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
+ * at this point it's safe to read value from them.
+ */
+ if (IS_ENABLED(CONFIG_64BIT))
+ drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
+
+ if (drvdata->edcidsr_present)
+ drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
+
+ if (drvdata->edvidsr_present)
+ drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
+
+out:
+ CS_LOCK(drvdata->base);
+}
+
+#ifndef CONFIG_64BIT
+static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata,
+ unsigned long pc)
+{
+ unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
+
+ if (drvdata->pc_has_offset) {
+ arm_inst_offset = 8;
+ thumb_inst_offset = 4;
+ }
+
+ /* Handle thumb instruction */
+ if (pc & EDPCSR_THUMB) {
+ pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
+ return pc;
+ }
+
+ /*
+ * Handle arm instruction offset, if the arm instruction
+ * is not 4 byte alignment then it's possible the case
+ * for implementation defined; keep original value for this
+ * case and print info for notice.
+ */
+ if (pc & BIT(1))
+ pr_emerg("Instruction offset is implementation defined\n");
+ else
+ pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
+
+ return pc;
+}
+#endif
+
+static void debug_dump_regs(struct debug_drvdata *drvdata)
+{
+ unsigned long pc;
+
+ pr_emerg("\tEDPRSR: %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
+ drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
+ drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
+
+ if (!debug_access_permitted(drvdata)) {
+ pr_emerg("No permission to access debug registers!\n");
+ return;
+ }
+
+ if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
+ pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
+ return;
+ }
+
+#ifdef CONFIG_64BIT
+ pc = (unsigned long)drvdata->edpcsr_hi << 32 |
+ (unsigned long)drvdata->edpcsr;
+#else
+ pc = debug_adjust_pc(drvdata, (unsigned long)drvdata->edpcsr);
+#endif
+
+ pr_emerg("\tEDPCSR: [<%p>] %pS\n", (void *)pc, (void *)pc);
+
+ if (drvdata->edcidsr_present)
+ pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
+
+ if (drvdata->edvidsr_present)
+ pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%dbits VMID:%x)\n",
+ drvdata->edvidsr,
+ drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
+ drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
+ (drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
+ drvdata->edvidsr & EDVIDSR_HV ? 64 : 32,
+ drvdata->edvidsr & (u32)EDVIDSR_VMID);
+}
+
+static void debug_init_arch_data(void *info)
+{
+ struct debug_drvdata *drvdata = info;
+ u32 mode, pcsr_offset;
+
+ CS_UNLOCK(drvdata->base);
+
+ debug_os_unlock(drvdata);
+
+ /* Read device info */
+ drvdata->eddevid = readl_relaxed(drvdata->base + EDDEVID);
+ drvdata->eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
+
+ CS_LOCK(drvdata->base);
+
+ /* Parse implementation feature */
+ mode = drvdata->eddevid & EDDEVID_PCSAMPLE_MODE;
+ pcsr_offset = drvdata->eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
+
+ if (mode == EDDEVID_IMPL_NONE) {
+ drvdata->edpcsr_present = false;
+ drvdata->edcidsr_present = false;
+ drvdata->edvidsr_present = false;
+ } else if (mode == EDDEVID_IMPL_EDPCSR) {
+ drvdata->edpcsr_present = true;
+ drvdata->edcidsr_present = false;
+ drvdata->edvidsr_present = false;
+ } else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
+ /*
+ * In ARM DDI 0487A.k, the EDDEVID1.PCSROffset is used to
+ * define if has the offset for PC sampling value; if read
+ * back EDDEVID1.PCSROffset == 0x2, then this means the debug
+ * module does not sample the instruction set state when
+ * armv8 CPU in AArch32 state.
+ */
+ if (!IS_ENABLED(CONFIG_64BIT) &&
+ (pcsr_offset == EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32))
+ drvdata->edpcsr_present = false;
+ else
+ drvdata->edpcsr_present = true;
+
+ drvdata->edcidsr_present = true;
+ drvdata->edvidsr_present = false;
+ } else if (mode == EDDEVID_IMPL_FULL) {
+ drvdata->edpcsr_present = true;
+ drvdata->edcidsr_present = true;
+ drvdata->edvidsr_present = true;
+ }
+
+ if (IS_ENABLED(CONFIG_64BIT))
+ drvdata->pc_has_offset = false;
+ else
+ drvdata->pc_has_offset =
+ (pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
+
+ return;
+}
+
+/*
+ * Dump out information on panic.
+ */
+static int debug_notifier_call(struct notifier_block *self,
+ unsigned long v, void *p)
+{
+ int cpu;
+ struct debug_drvdata *drvdata;
+
+ pr_emerg("ARM external debug module:\n");
+
+ for_each_possible_cpu(cpu) {
+ drvdata = per_cpu(debug_drvdata, cpu);
+ if (!drvdata)
+ continue;
+
+ pr_emerg("CPU[%d]:\n", drvdata->cpu);
+
+ debug_read_regs(drvdata);
+ debug_dump_regs(drvdata);
+ }
+
+ return 0;
+}
+
+static struct notifier_block debug_notifier = {
+ .notifier_call = debug_notifier_call,
+};
+
+static int debug_enable_func(void)
+{
+ int ret;
+
+ pm_qos_add_request(&debug_qos_req,
+ PM_QOS_CPU_DMA_LATENCY, idle_constraint);
+
+ ret = atomic_notifier_chain_register(&panic_notifier_list,
+ &debug_notifier);
+ if (ret)
+ goto err;
+
+ return 0;
+
+err:
+ pm_qos_remove_request(&debug_qos_req);
+ return ret;
+}
+
+static void debug_disable_func(void)
+{
+ atomic_notifier_chain_unregister(&panic_notifier_list,
+ &debug_notifier);
+ pm_qos_remove_request(&debug_qos_req);
+}
+
+static ssize_t debug_func_knob_write(struct file *f,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ u8 on;
+ int ret;
+
+ ret = kstrtou8_from_user(buf, count, 2, &on);
+ if (ret)
+ return ret;
+
+ mutex_lock(&debug_lock);
+
+ if (!on ^ debug_enable)
+ goto out;
+
+ if (on) {
+ ret = debug_enable_func();
+ if (ret) {
+ pr_err("%s: unable to disable debug function: %d\n",
+ __func__, ret);
+ goto err;
+ }
+ } else
+ debug_disable_func();
+
+ debug_enable = on;
+
+out:
+ ret = count;
+err:
+ mutex_unlock(&debug_lock);
+ return ret;
+}
+
+static ssize_t debug_func_knob_read(struct file *f,
+ char __user *ubuf, size_t count, loff_t *ppos)
+{
+ char val[] = { '0' + debug_enable, '\n' };
+
+ return simple_read_from_buffer(ubuf, count, ppos, val, sizeof(val));
+}
+
+static ssize_t debug_idle_constraint_write(struct file *f,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ int val;
+ ssize_t ret;
+
+ ret = kstrtoint_from_user(buf, count, 10, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&debug_lock);
+ idle_constraint = val;
+
+ if (debug_enable)
+ pm_qos_update_request(&debug_qos_req, idle_constraint);
+
+ mutex_unlock(&debug_lock);
+ return count;
+}
+
+static ssize_t debug_idle_constraint_read(struct file *f,
+ char __user *ubuf, size_t count, loff_t *ppos)
+{
+ char buf[32];
+ int len;
+
+ if (*ppos)
+ return 0;
+
+ len = sprintf(buf, "%d\n", idle_constraint);
+ return simple_read_from_buffer(ubuf, count, ppos, buf, len);
+}
+
+static const struct file_operations debug_func_knob_fops = {
+ .open = simple_open,
+ .read = debug_func_knob_read,
+ .write = debug_func_knob_write,
+};
+
+static const struct file_operations debug_idle_constraint_fops = {
+ .open = simple_open,
+ .read = debug_idle_constraint_read,
+ .write = debug_idle_constraint_write,
+};
+
+static int debug_func_init(void)
+{
+ struct dentry *file;
+ int ret;
+
+ /* Create debugfs node */
+ debug_debugfs_dir = debugfs_create_dir("coresight_cpu_debug", NULL);
+ if (!debug_debugfs_dir) {
+ pr_err("%s: unable to create debugfs directory\n", __func__);
+ return -ENOMEM;
+ }
+
+ file = debugfs_create_file("enable", S_IRUGO | S_IWUSR,
+ debug_debugfs_dir, NULL, &debug_func_knob_fops);
+ if (!file) {
+ pr_err("%s: unable to create enable knob file\n", __func__);
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ file = debugfs_create_file("idle_constraint", S_IRUGO | S_IWUSR,
+ debug_debugfs_dir, NULL, &debug_idle_constraint_fops);
+ if (!file) {
+ pr_err("%s: unable to create idle constraint file\n", __func__);
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ /* Use sysfs node to enable functionality */
+ if (!debug_enable)
+ return 0;
+
+ /* Enable debug module at boot time */
+ ret = debug_enable_func();
+ if (ret) {
+ pr_err("%s: unable to disable debug function: %d\n",
+ __func__, ret);
+ goto err;
+ }
+
+ return 0;
+
+err:
+ debugfs_remove_recursive(debug_debugfs_dir);
+ return ret;
+}
+
+static void debug_func_exit(void)
+{
+ debugfs_remove_recursive(debug_debugfs_dir);
+
+ /* Disable functionality if has enabled */
+ if (debug_enable)
+ debug_disable_func();
+}
+
+static int debug_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ void __iomem *base;
+ struct device *dev = &adev->dev;
+ struct debug_drvdata *drvdata;
+ struct resource *res = &adev->res;
+ struct device_node *np = adev->dev.of_node;
+ int ret;
+
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
+ if (per_cpu(debug_drvdata, drvdata->cpu)) {
+ dev_err(dev, "CPU's drvdata has been initialized\n");
+ return -EBUSY;
+ }
+
+ drvdata->dev = &adev->dev;
+ amba_set_drvdata(adev, drvdata);
+
+ /* Validity for the resource is already checked by the AMBA core */
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ drvdata->base = base;
+
+ get_online_cpus();
+ per_cpu(debug_drvdata, drvdata->cpu) = drvdata;
+ ret = smp_call_function_single(drvdata->cpu,
+ debug_init_arch_data, drvdata, 1);
+ put_online_cpus();
+
+ if (ret) {
+ dev_err(dev, "Debug arch init failed\n");
+ goto err;
+ }
+
+ if (!drvdata->edpcsr_present) {
+ ret = -ENXIO;
+ dev_err(dev, "Sample-based profiling is not implemented\n");
+ goto err;
+ }
+
+ if (!debug_count++) {
+ ret = debug_func_init();
+ if (ret)
+ goto err_func_init;
+ }
+
+ dev_info(dev, "Coresight debug-CPU%d initialized\n", drvdata->cpu);
+ return 0;
+
+err_func_init:
+ debug_count--;
+err:
+ per_cpu(debug_drvdata, drvdata->cpu) = NULL;
+ return ret;
+}
+
+static int debug_remove(struct amba_device *adev)
+{
+ struct debug_drvdata *drvdata = amba_get_drvdata(adev);
+
+ per_cpu(debug_drvdata, drvdata->cpu) = NULL;
+
+ if (!--debug_count)
+ debug_func_exit();
+
+ return 0;
+}
+
+static struct amba_id debug_ids[] = {
+ { /* Debug for Cortex-A53 */
+ .id = 0x000bbd03,
+ .mask = 0x000fffff,
+ },
+ { /* Debug for Cortex-A57 */
+ .id = 0x000bbd07,
+ .mask = 0x000fffff,
+ },
+ { /* Debug for Cortex-A72 */
+ .id = 0x000bbd08,
+ .mask = 0x000fffff,
+ },
+ { 0, 0 },
+};
+
+static struct amba_driver debug_driver = {
+ .drv = {
+ .name = "coresight-cpu-debug",
+ .suppress_bind_attrs = true,
+ },
+ .probe = debug_probe,
+ .remove = debug_remove,
+ .id_table = debug_ids,
+};
+
+module_amba_driver(debug_driver);
+
+MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
+MODULE_DESCRIPTION("ARM Coresight CPU Debug Driver");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* [PATCH v5 5/9] coresight: use const for device_node structures
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
mike.leach-QSEj5FYQhm4dnm+yROfE0A, Suzuki.Poulose-5wv7dgnIgG8,
sudeep.holla-5wv7dgnIgG8
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Almost low level functions from open firmware have used const to
qualify device_node structures, so add const for device_node
parameters in of_coresight related functions.
Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/hwtracing/coresight/of_coresight.c | 6 +++---
include/linux/coresight.h | 8 ++++----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index 78d2399..46eec0f 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -52,7 +52,7 @@ of_coresight_get_endpoint_device(struct device_node *endpoint)
endpoint, of_dev_node_match);
}
-static void of_coresight_get_ports(struct device_node *node,
+static void of_coresight_get_ports(const struct device_node *node,
int *nr_inport, int *nr_outport)
{
struct device_node *ep = NULL;
@@ -101,7 +101,7 @@ static int of_coresight_alloc_memory(struct device *dev,
return 0;
}
-int of_coresight_get_cpu(struct device_node *node)
+int of_coresight_get_cpu(const struct device_node *node)
{
int cpu;
bool found;
@@ -128,7 +128,7 @@ int of_coresight_get_cpu(struct device_node *node)
EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
struct coresight_platform_data *of_get_coresight_platform_data(
- struct device *dev, struct device_node *node)
+ struct device *dev, const struct device_node *node)
{
int i = 0, ret = 0;
struct coresight_platform_data *pdata;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index bf96678..4915254 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -263,13 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
#endif
#ifdef CONFIG_OF
-extern int of_coresight_get_cpu(struct device_node *node);
+extern int of_coresight_get_cpu(const struct device_node *node);
extern struct coresight_platform_data *of_get_coresight_platform_data(
- struct device *dev, struct device_node *node);
+ struct device *dev, const struct device_node *node);
#else
-static inline int of_coresight_get_cpu(struct device_node *node) { return 0; }
+static inline int of_coresight_get_cpu(const struct device_node *node) { return 0; }
static inline struct coresight_platform_data *of_get_coresight_platform_data(
- struct device *dev, struct device_node *node) { return NULL; }
+ struct device *dev, const struct device_node *node) { return NULL; }
#endif
#ifdef CONFIG_PID_NS
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 4/9] coresight: refactor with function of_coresight_get_cpu
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
This is refactor to add function of_coresight_get_cpu(), so it's used to
retrieve CPU id for coresight component. Finally can use it as a common
function for multiple places.
Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
drivers/hwtracing/coresight/of_coresight.c | 43 +++++++++++++++++++-----------
include/linux/coresight.h | 2 ++
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index 1a77280..78d2399 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -101,15 +101,39 @@ static int of_coresight_alloc_memory(struct device *dev,
return 0;
}
+int of_coresight_get_cpu(struct device_node *node)
+{
+ int cpu;
+ bool found;
+ struct device_node *dn, *np;
+
+ dn = of_parse_phandle(node, "cpu", 0);
+
+ /* Affinity defaults to CPU0 */
+ if (!dn)
+ return 0;
+
+ for_each_possible_cpu(cpu) {
+ np = of_cpu_device_node_get(cpu);
+ found = (dn == np);
+ of_node_put(np);
+ if (found)
+ break;
+ }
+ of_node_put(dn);
+
+ /* Affinity to CPU0 if no cpu nodes are found */
+ return found ? cpu : 0;
+}
+EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+
struct coresight_platform_data *of_get_coresight_platform_data(
struct device *dev, struct device_node *node)
{
- int i = 0, ret = 0, cpu;
+ int i = 0, ret = 0;
struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev;
- bool found;
- struct device_node *dn, *np;
struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
@@ -176,18 +200,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
} while (ep);
}
- dn = of_parse_phandle(node, "cpu", 0);
- for_each_possible_cpu(cpu) {
- np = of_cpu_device_node_get(cpu);
- found = (dn == np);
- of_node_put(np);
- if (found)
- break;
- }
- of_node_put(dn);
-
- /* Affinity to CPU0 if no cpu nodes are found */
- pdata->cpu = found ? cpu : 0;
+ pdata->cpu = of_coresight_get_cpu(node);
return pdata;
}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 2a5982c..bf96678 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -263,9 +263,11 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
#endif
#ifdef CONFIG_OF
+extern int of_coresight_get_cpu(struct device_node *node);
extern struct coresight_platform_data *of_get_coresight_platform_data(
struct device *dev, struct device_node *node);
#else
+static inline int of_coresight_get_cpu(struct device_node *node) { return 0; }
static inline struct coresight_platform_data *of_get_coresight_platform_data(
struct device *dev, struct device_node *node) { return NULL; }
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH v5 3/9] coresight: of_get_coresight_platform_data: Add missing of_node_put
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Suzuki K Poulose, Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
From: Suzuki K Poulose <suzuki.poulose@arm.com>
The of_get_coresight_platform_data iterates over the possible CPU nodes
to find a given cpu phandle. However it does not drop the reference
to the node pointer returned by the of_get_coresight_platform_data.
This patch also introduces another minor fix is to use
of_cpu_device_node_get() to replace of_get_cpu_node().
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
[Leo: minor tweaks for of_get_coresight_platform_data]
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
drivers/hwtracing/coresight/of_coresight.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index 629e031..1a77280 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -108,7 +108,8 @@ struct coresight_platform_data *of_get_coresight_platform_data(
struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev;
- struct device_node *dn;
+ bool found;
+ struct device_node *dn, *np;
struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
@@ -175,17 +176,19 @@ struct coresight_platform_data *of_get_coresight_platform_data(
} while (ep);
}
- /* Affinity defaults to CPU0 */
- pdata->cpu = 0;
dn = of_parse_phandle(node, "cpu", 0);
- for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
- if (dn == of_get_cpu_node(cpu, NULL)) {
- pdata->cpu = cpu;
+ for_each_possible_cpu(cpu) {
+ np = of_cpu_device_node_get(cpu);
+ found = (dn == np);
+ of_node_put(np);
+ if (found)
break;
- }
}
of_node_put(dn);
+ /* Affinity to CPU0 if no cpu nodes are found */
+ pdata->cpu = found ? cpu : 0;
+
return pdata;
}
EXPORT_SYMBOL_GPL(of_get_coresight_platform_data);
--
2.7.4
^ permalink raw reply related
* [PATCH v5 2/9] doc: Add documentation for Coresight CPU debug
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
Update kernel-parameters.txt to add two new parameters:
- coresight_cpu_debug.enable is a knob to enable debugging at boot time.
- coresight_cpu_debug.idle_constraint is used to constrain idle states
to ensure Coresight CPU debug component can be accessible.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
Documentation/admin-guide/kernel-parameters.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2ba45ca..6ed57d9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -650,6 +650,27 @@
/proc/<pid>/coredump_filter.
See also Documentation/filesystems/proc.txt.
+ coresight_cpu_debug.enable
+ [ARM,ARM64]
+ Format: <bool>
+ Enable/disable the CPU sampling based debugging.
+ 0: default value, disable debugging
+ 1: enable debugging at boot time
+
+ coresight_cpu_debug.idle_constraint
+ [ARM,ARM64]
+ Format: <int>
+ Some platforms have designed the idle states to disable
+ CPU power domain and need manually set constraint so
+ can access coresight CPU debug component safely. Setting
+ this parameter for latency requirement in
+ microseconds, finally we can constraint all or partial
+ idle states to ensure the CPU power domain is enabled.
+ Default is -1, which means no limiation to CPU idle
+ states; if set to 0, this means disabling all idle
+ states; user can choose other platform dependent values
+ so can disable specific idle states for the platform.
+
cpuidle.off=1 [CPU_IDLE]
disable the cpuidle sub-system
--
2.7.4
^ permalink raw reply related
* [PATCH v5 1/9] coresight: bindings for CPU debug module
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
In-Reply-To: <1490466197-29163-1-git-send-email-leo.yan@linaro.org>
According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
Chapter 'Part H: External debug', the CPU can integrate debug module
and it can support self-hosted debug and external debug. Especially
for supporting self-hosted debug, this means the program can access
the debug module from mmio region; and usually the mmio region is
integrated with coresight.
So add document for binding debug component, includes binding to APB
clock; and also need specify the CPU node which the debug module is
dedicated to specific CPU.
Suggested-by: Mike Leach <mike.leach@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
.../bindings/arm/coresight-cpu-debug.txt | 48 ++++++++++++++++++++++
1 file changed, 48 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
diff --git a/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
new file mode 100644
index 0000000..7ef3824
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
@@ -0,0 +1,48 @@
+* CoreSight CPU Debug Component:
+
+CoreSight CPU debug component are compliant with the ARMv8 architecture
+reference manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The
+external debug module is mainly used for two modes: self-hosted debug and
+external debug, and it can be accessed from mmio region from Coresight
+and eventually the debug module connects with CPU for debugging. And the
+debug module provides sample-based profiling extension, which can be used
+to sample CPU program counter, secure state and exception level, etc;
+usually every CPU has one dedicated debug module to be connected.
+
+Required properties:
+
+- compatible : should be "arm,coresight-cpu-debug"; supplemented with
+ "arm,primecell" since this driver is using the AMBA bus
+ interface.
+
+- reg : physical base address and length of the register set.
+
+- clocks : the clock associated to this component.
+
+- clock-names : the name of the clock referenced by the code. Since we are
+ using the AMBA framework, the name of the clock providing
+ the interconnect should be "apb_pclk" and the clock is
+ mandatory. The interface between the debug logic and the
+ processor core is clocked by the internal CPU clock, so it
+ is enabled with CPU clock by default.
+
+- cpu : the CPU phandle the debug module is affined to. When omitted
+ the module is considered to belong to CPU0.
+
+Optional properties:
+
+- power-domains: a phandle to the debug power domain. We use "power-domains"
+ binding to turn on the debug logic if it has own dedicated
+ power domain and if necessary to use "idle_constraint" in
+ kernel command line or debugfs node to constrain idle states
+ to ensure registers in the CPU power domain are accessible.
+
+Example:
+
+ debug@f6590000 {
+ compatible = "arm,coresight-cpu-debug","arm,primecell";
+ reg = <0 0xf6590000 0 0x1000>;
+ clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+ clock-names = "apb_pclk";
+ cpu = <&cpu0>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v5 0/9] coresight: enable debug module
From: Leo Yan @ 2017-03-25 18:23 UTC (permalink / raw)
To: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
Suzuki.Poulose, sudeep.holla
Cc: Leo Yan
ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.
This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.
Patch 0001 is to document the dt binding; patch 0002 is to document
boot parameters used in kernel command line.
Patch 0003 is used to fix the func of_get_coresight_platform_data()
doesn't properly drop the reference to the CPU node pointer; and
patch 0004 is refactor to add new function of_coresight_get_cpu().
Patch 0005 is to add const quality to structure device_node.
Patch 0006 is the driver for CPU debug module.
Patches 0007/0008 in this series are to enable debug unit on 96boards
Hikey, patch 0007 is to add apb clock for debug unit and patch 0006
is to add DT nodes for debug unit. Patch 0009 is to enable debug on
96boards DB410c. Have verified on both two boards.
We can enable debugging with two method, adding parameters into kernel
command line for build-in module:
coresight_cpu_debug.enable=1
coresight_cpu_debug.idle_constraint=0
Or we can wait the system has booted up to use debugfs nodes to enable
debugging and set idle constraints:
# echo 1 > /sys/kernel/debug/coresight_cpu_debug/enable
# echo 0 > /sys/kernel/debug/coresight_cpu_debug/idle_constraint
As result we can get below log after input command:
echo c > /proc/sysrq-trigger:
ARM external debug module:
CPU[0]:
EDPRSR: 0000000b (Power:On DLK:Unlock)
EDPCSR: [<ffff00000808eb54>] handle_IPI+0xe4/0x150
EDCIDSR: 00000000
EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
CPU[1]:
EDPRSR: 0000000b (Power:On DLK:Unlock)
EDPCSR: [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
EDCIDSR: 00000000
EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
[...]
Changes from v4:
* This version is mainly credit to ARM colleagues many contribution
ideas for better quality (Thanks a lot Suzuki, Mike and Sudeep!).
* According to Suzuki suggestion, refined debug module driver to avoid
memory leak for drvdata struct, handle PCSAMPLE_MODE=1, use flag
drvdata.pc_has_offset to indicate if PCSR has offset, minor fixes.
* According to Mathieu suggestion, refined dt binding description.
* Changed driver to support module mode;
* According to Mike suggestion and very appreciate the pseudo code,
added support to force CPU powered up with register EDPRCR;
* According to discussions, added command line and debugfs nodes to
support enabling debugging for boot time, or later can dynamically
enable/disable debugging by debugfs.
* According to Rob Herring suggestion, one minor fixes in DT binding.
* According to Stephen Boyd suggestion, add const quality to structure
device_node. And used use of_cpu_device_node_get() to replace
of_get_cpu_node() in patch 0003.
Changes from v3:
* Added Suzuki K Poulose's patch to fix issue for the func
of_get_coresight_platform_data() doesn't properly drop the reference
to the CPU node pointer.
* According to Suzuki suggestion, added code to handl the corner case
for ARMv8 CPU with aarch32 mode.
* According to Suzuki suggestion, changed compatible string to
"arm,coresight-cpu-debug".
* According to Mathieu suggestion, added "power-domains" as optional
properties.
Changes from v2:
* According to Mathieu Poirier suggestion, applied some minor fixes.
* Added two extra patches for enabling debug module on Hikey.
Changes from v1:
* According to Mike Leach suggestion, removed the binding for debug
module clocks which have been directly provided by CPU clocks.
* According to Mathieu Poirier suggestion, added function
of_coresight_get_cpu() and some minor refactors for debug module
driver.
Changes from RFC:
* According to Mike Leach suggestion, added check for EDPRSR to avoid
lockup; added supporting EDVIDSR and EDCIDSR registers.
* According to Mark Rutland and Mathieu Poirier suggestion, rewrote
the documentation for DT binding.
* According to Mark and Mathieu suggestion, refined debug driver.
Leo Yan (8):
coresight: bindings for CPU debug module
doc: Add documentation for Coresight CPU debug
coresight: refactor with function of_coresight_get_cpu
coresight: use const for device_node structures
coresight: add support for CPU debug module
clk: hi6220: add debug APB clock
arm64: dts: hi6220: register debug module
arm64: dts: qcom: msm8916: Add debug unit
Suzuki K Poulose (1):
coresight: of_get_coresight_platform_data: Add missing of_node_put
Documentation/admin-guide/kernel-parameters.txt | 21 +
.../bindings/arm/coresight-cpu-debug.txt | 48 ++
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 64 ++
arch/arm64/boot/dts/qcom/msm8916.dtsi | 32 +
drivers/clk/hisilicon/clk-hi6220.c | 1 +
drivers/hwtracing/coresight/Kconfig | 11 +
drivers/hwtracing/coresight/Makefile | 1 +
drivers/hwtracing/coresight/coresight-cpu-debug.c | 704 +++++++++++++++++++++
drivers/hwtracing/coresight/of_coresight.c | 44 +-
include/dt-bindings/clock/hi6220-clock.h | 5 +-
include/linux/coresight.h | 6 +-
11 files changed, 920 insertions(+), 17 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt
create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c
--
2.7.4
^ permalink raw reply
* Re: [PATCH 2/2] iio: adc: meson-saradc: add Meson8b SoC compatibility
From: Jonathan Cameron @ 2017-03-25 18:13 UTC (permalink / raw)
To: Martin Blumenstingl, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170325162938.28659-3-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On 25/03/17 16:29, Martin Blumenstingl wrote:
> Meson GX SoCs however use some magic bits to prevent simultaneous (=
> conflicting, because only consumer should use the FIFO buffer with the
> ADC results) usage by the Linux kernel and the bootloader (the BL30
> bootloader uses the SAR ADC to read the CPU temperature).
> This patch changes guards all BL30 functionality so it is skipped on
> SoCs which don't have it. Since the hardware itself doesn't know whether
> BL30 is available the internal meson_sar_adc_data is extended so this
> information can be provided per of_device_id.data inside the driver.
>
> Additionally the clocks "adc_clk" and "adc_sel" are not provided by the
> clock-controller itself. "adc_sel" is not available at all. "adc_clk"
> is provided by the SAR ADC IP block itself on Meson8b (and earlier).
> This is already supported by the meson_saradc driver.
>
> Finally a new of_device_id for the Meson8b SoC is added so it can be
> wired up in the corresponding DT.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Looks sensible to me. Just fix those bits from Peter and we'll let it sit
a few days on the list to see if anyone else wants to chip in.
Thanks,
Jonathan
> ---
> drivers/iio/adc/meson_saradc.c | 70 +++++++++++++++++++++++++++---------------
> 1 file changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index dd4190b50df6..f78fe66e0ec8 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -220,6 +220,7 @@ enum meson_sar_adc_chan7_mux_sel {
> };
>
> struct meson_sar_adc_data {
> + char has_bl30_integration;
> unsigned int resolution;
> const char *name;
> };
> @@ -437,19 +438,24 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
>
> mutex_lock(&indio_dev->mlock);
>
> - /* prevent BL30 from using the SAR ADC while we are using it */
> - regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
> - MESON_SAR_ADC_DELAY_KERNEL_BUSY,
> - MESON_SAR_ADC_DELAY_KERNEL_BUSY);
> -
> - /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
> - do {
> - udelay(1);
> - regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
> - } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
> -
> - if (timeout < 0)
> - return -ETIMEDOUT;
> + if (priv->data->has_bl30_integration) {
> + /* prevent BL30 from using the SAR ADC while we are using it */
> + regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
> + MESON_SAR_ADC_DELAY_KERNEL_BUSY,
> + MESON_SAR_ADC_DELAY_KERNEL_BUSY);
> +
> + /*
> + * wait until BL30 releases it's lock (so we can use the SAR
> + * ADC)
> + */
> + do {
> + udelay(1);
> + regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
> + } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
> +
> + if (timeout < 0)
> + return -ETIMEDOUT;
> + }
>
> return 0;
> }
> @@ -458,9 +464,10 @@ static void meson_sar_adc_unlock(struct iio_dev *indio_dev)
> {
> struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
>
> - /* allow BL30 to use the SAR ADC again */
> - regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
> - MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
> + if (priv->data->has_bl30_integration)
> + /* allow BL30 to use the SAR ADC again */
> + regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
> + MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
>
> mutex_unlock(&indio_dev->mlock);
> }
> @@ -614,14 +621,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
> */
> meson_sar_adc_set_chan7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
>
> - /*
> - * leave sampling delay and the input clocks as configured by BL30 to
> - * make sure BL30 gets the values it expects when reading the
> - * temperature sensor.
> - */
> - regmap_read(priv->regmap, MESON_SAR_ADC_REG3, ®val);
> - if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
> - return 0;
> + if (priv->data->has_bl30_integration) {
> + /*
> + * leave sampling delay and the input clocks as configured by
> + * BL30 to make sure BL30 gets the values it expects when
> + * reading the temperature sensor.
> + */
> + regmap_read(priv->regmap, MESON_SAR_ADC_REG3, ®val);
> + if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
> + return 0;
> + }
>
> meson_sar_adc_stop_sample_engine(indio_dev);
>
> @@ -834,23 +843,36 @@ static const struct iio_info meson_sar_adc_iio_info = {
> .driver_module = THIS_MODULE,
> };
>
> +struct meson_sar_adc_data meson_sar_adc_meson8b_data = {
> + .has_bl30_integration = 0,
> + .resolution = 10,
> + .name = "meson-meson8b-saradc",
> +};
> +
> struct meson_sar_adc_data meson_sar_adc_gxbb_data = {
> + .has_bl30_integration = 1,
> .resolution = 10,
> .name = "meson-gxbb-saradc",
> };
>
> struct meson_sar_adc_data meson_sar_adc_gxl_data = {
> + .has_bl30_integration = 1,
> .resolution = 12,
> .name = "meson-gxl-saradc",
> };
>
> struct meson_sar_adc_data meson_sar_adc_gxm_data = {
> + .has_bl30_integration = 1,
> .resolution = 12,
> .name = "meson-gxm-saradc",
> };
>
> static const struct of_device_id meson_sar_adc_of_match[] = {
> {
> + .compatible = "amlogic,meson8b-saradc",
> + .data = &meson_sar_adc_meson8b_data,
> + },
> + {
> .compatible = "amlogic,meson-gxbb-saradc",
> .data = &meson_sar_adc_gxbb_data,
> }, {
>
^ permalink raw reply
* Re: [PATCHv3] iio: adc: cpcap: Add minimal support for CPCAP PMIC ADC
From: Jonathan Cameron @ 2017-03-25 18:10 UTC (permalink / raw)
To: Tony Lindgren
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Marcel Partap, Michael Scott,
Sebastian Reichel
In-Reply-To: <20170324033842.12541-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
On 24/03/17 03:38, Tony Lindgren wrote:
> On Motorola phones like droid 4 there is a custom CPCAP PMIC. This PMIC
> has ADCs that are used for battery charging and USB PHY VBUS and ID pin
> detection.
>
> Unfortunately the only documentation for this ADC seems to be the
> Motorola mapphone Linux kernel tree. I have tested that reading raw and
> scaled values works, but I have not used the timed sampling that the ADC
> seems to support.
>
> Let's add a minimal support for it so we can eventually provide IIO
> channels for the related battery charging and USB PHY drivers.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Marcel Partap <mpartap-hi6Y0CQ0nG0@public.gmane.org>
> Cc: Michael Scott <michael.scott-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Looks good to me. It's complex enough that we should let it sit for a few
more days though to see if anyone else picks up on anything.
If it looks like I've dropped this after say two weeks, do poke me as I've
been known to lose drivers from time to time!
Jonathan
> ---
>
> Changes since v2:
>
> - Simplify things further by using channel based indexing and
> remove more drivers specific enumerations
>
> Changes since v1:
>
> - Fix numerous issues pointed out by Peter Meerwald-Stadler
> <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org> and Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> - Simplify things further by limiting read functions to a
> single channel instead of a bank and got rid of the driver
> specific cpcap_adc_conv_type in favor of IIO generic types
>
> ---
> .../devicetree/bindings/iio/adc/cpcap-adc.txt | 18 +
> drivers/iio/adc/Kconfig | 11 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/cpcap-adc.c | 1007 ++++++++++++++++++++
> 4 files changed, 1037 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/cpcap-adc.txt
> create mode 100644 drivers/iio/adc/cpcap-adc.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/cpcap-adc.txt b/Documentation/devicetree/bindings/iio/adc/cpcap-adc.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/cpcap-adc.txt
> @@ -0,0 +1,18 @@
> +Motorola CPCAP PMIC ADC binding
> +
> +Required properties:
> +- compatible: Should be "motorola,cpcap-adc" or "motorola,mapphone-cpcap-adc"
> +- interrupt-parent: The interrupt controller
> +- interrupts: The interrupt number for the ADC device
> +- interrupt-names: Should be "adcdone"
> +- #io-channel-cells: Number of cells in an IIO specifier
> +
> +Example:
> +
> +cpcap_adc: adc {
> + compatible = "motorola,mapphone-cpcap-adc";
> + interrupt-parent = <&cpcap>;
> + interrupts = <8 IRQ_TYPE_NONE>;
> + interrupt-names = "adcdone";
> + #io-channel-cells = <1>;
> +};
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -195,6 +195,17 @@ config CC10001_ADC
> This driver can also be built as a module. If so, the module will be
> called cc10001_adc.
>
> +config CPCAP_ADC
> + tristate "Motorola CPCAP PMIC ADC driver"
> + depends on MFD_CPCAP
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to build support for Motorola CPCAP PMIC ADC.
> +
> + This driver can also be built as a module. If so, the module will be
> + called cpcap-adc.
> +
> config DA9150_GPADC
> tristate "Dialog DA9150 GPADC driver support"
> depends on MFD_DA9150
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
> obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
> obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
> obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
> +obj-$(CONFIG_CPCAP_ADC) += cpcap-adc.o
> obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
> obj-$(CONFIG_ENVELOPE_DETECTOR) += envelope-detector.o
> obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
> diff --git a/drivers/iio/adc/cpcap-adc.c b/drivers/iio/adc/cpcap-adc.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/adc/cpcap-adc.c
> @@ -0,0 +1,1007 @@
> +/*
> + * Copyright (C) 2017 Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> + *
> + * Rewritten for Linux IIO framework with some code based on
> + * earlier driver found in the Motorola Linux kernel:
> + *
> + * Copyright (C) 2009-2010 Motorola, Inc.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/driver.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/mfd/motorola-cpcap.h>
> +
> +/* Register CPCAP_REG_ADCC1 bits */
> +#define CPCAP_BIT_ADEN_AUTO_CLR BIT(15) /* Currently unused */
> +#define CPCAP_BIT_CAL_MODE BIT(14) /* Set with BIT_RAND0 */
> +#define CPCAP_BIT_ADC_CLK_SEL1 BIT(13) /* Currently unused */
> +#define CPCAP_BIT_ADC_CLK_SEL0 BIT(12) /* Currently unused */
> +#define CPCAP_BIT_ATOX BIT(11)
> +#define CPCAP_BIT_ATO3 BIT(10)
> +#define CPCAP_BIT_ATO2 BIT(9)
> +#define CPCAP_BIT_ATO1 BIT(8)
> +#define CPCAP_BIT_ATO0 BIT(7)
> +#define CPCAP_BIT_ADA2 BIT(6)
> +#define CPCAP_BIT_ADA1 BIT(5)
> +#define CPCAP_BIT_ADA0 BIT(4)
> +#define CPCAP_BIT_AD_SEL1 BIT(3) /* Set for bank1 */
> +#define CPCAP_BIT_RAND1 BIT(2) /* Set for channel 16 & 17 */
> +#define CPCAP_BIT_RAND0 BIT(1) /* Set with CAL_MODE */
> +#define CPCAP_BIT_ADEN BIT(0) /* Currently unused */
> +
> +/* Register CPCAP_REG_ADCC2 bits */
> +#define CPCAP_BIT_CAL_FACTOR_ENABLE BIT(15) /* Currently unused */
> +#define CPCAP_BIT_BATDETB_EN BIT(14) /* Currently unused */
> +#define CPCAP_BIT_ADTRIG_ONESHOT BIT(13) /* Set for !TIMING_IMM */
> +#define CPCAP_BIT_ASC BIT(12) /* Set for TIMING_IMM */
> +#define CPCAP_BIT_ATOX_PS_FACTOR BIT(11)
> +#define CPCAP_BIT_ADC_PS_FACTOR1 BIT(10)
> +#define CPCAP_BIT_ADC_PS_FACTOR0 BIT(9)
> +#define CPCAP_BIT_AD4_SELECT BIT(8) /* Currently unused */
> +#define CPCAP_BIT_ADC_BUSY BIT(7) /* Currently unused */
> +#define CPCAP_BIT_THERMBIAS_EN BIT(6) /* Currently unused */
> +#define CPCAP_BIT_ADTRIG_DIS BIT(5) /* Disable interrupt */
> +#define CPCAP_BIT_LIADC BIT(4) /* Currently unused */
> +#define CPCAP_BIT_TS_REFEN BIT(3) /* Currently unused */
> +#define CPCAP_BIT_TS_M2 BIT(2) /* Currently unused */
> +#define CPCAP_BIT_TS_M1 BIT(1) /* Currently unused */
> +#define CPCAP_BIT_TS_M0 BIT(0) /* Currently unused */
> +
> +#define CPCAP_MAX_TEMP_LVL 27
> +#define CPCAP_FOUR_POINT_TWO_ADC 801
> +#define ST_ADC_CAL_CHRGI_HIGH_THRESHOLD 530
> +#define ST_ADC_CAL_CHRGI_LOW_THRESHOLD 494
> +#define ST_ADC_CAL_BATTI_HIGH_THRESHOLD 530
> +#define ST_ADC_CAL_BATTI_LOW_THRESHOLD 494
> +#define ST_ADC_CALIBRATE_DIFF_THRESHOLD 3
> +
> +#define CPCAP_ADC_MAX_RETRIES 5 /* Calibration and quirk */
> +
> +/**
> + * struct cpcap_adc_ato - timing settings for cpcap adc
> + *
> + * Unfortunately no cpcap documentation available, please document when
> + * using these.
> + */
> +struct cpcap_adc_ato {
> + unsigned short ato_in;
> + unsigned short atox_in;
> + unsigned short adc_ps_factor_in;
> + unsigned short atox_ps_factor_in;
> + unsigned short ato_out;
> + unsigned short atox_out;
> + unsigned short adc_ps_factor_out;
> + unsigned short atox_ps_factor_out;
> +};
> +
> +/**
> + * struct cpcap-adc - cpcap adc device driver data
> + * @reg: cpcap regmap
> + * @dev: struct device
> + * @vendor: cpcap vendor
> + * @irq: interrupt
> + * @lock: mutex
> + * @ato: request timings
> + * @wq_data_avail: work queue
> + * @done: work done
> + */
> +struct cpcap_adc {
> + struct regmap *reg;
> + struct device *dev;
> + u16 vendor;
> + int irq;
> + struct mutex lock; /* ADC register access lock */
> + const struct cpcap_adc_ato *ato;
> + wait_queue_head_t wq_data_avail;
> + bool done;
> +};
> +
> +/**
> + * enum cpcap_adc_channel - cpcap adc channels
> + */
> +enum cpcap_adc_channel {
> + /* Bank0 channels */
> + CPCAP_ADC_AD0_BATTDETB, /* Battery detection */
> + CPCAP_ADC_BATTP, /* Battery voltage */
> + CPCAP_ADC_VBUS, /* USB VBUS voltage */
> + CPCAP_ADC_AD3, /* Battery temperature when charging */
> + CPCAP_ADC_BPLUS_AD4, /* Another battery or system voltage */
> + CPCAP_ADC_CHG_ISENSE, /* Calibrated charge current */
> + CPCAP_ADC_BATTI, /* Calibrated system current */
> + CPCAP_ADC_USB_ID, /* USB OTG ID, unused on droid 4? */
> +
> + /* Bank1 channels */
> + CPCAP_ADC_AD8, /* Seems unused */
> + CPCAP_ADC_AD9, /* Seems unused */
> + CPCAP_ADC_LICELL, /* Maybe system voltage? Always 3V */
> + CPCAP_ADC_HV_BATTP, /* Another battery detection? */
> + CPCAP_ADC_TSX1_AD12, /* Seems unused, for touchscreen? */
> + CPCAP_ADC_TSX2_AD13, /* Seems unused, for touchscreen? */
> + CPCAP_ADC_TSY1_AD14, /* Seems unused, for touchscreen? */
> + CPCAP_ADC_TSY2_AD15, /* Seems unused, for touchscreen? */
> +
> + /* Remuxed channels using bank0 entries */
> + CPCAP_ADC_BATTP_PI16, /* Alternative mux mode for BATTP */
> + CPCAP_ADC_BATTI_PI17, /* Alternative mux mode for BATTI */
> +
> + CPCAP_ADC_CHANNEL_NUM,
> +};
> +
> +/**
> + * enum cpcap_adc_timing - cpcap adc timing options
> + *
> + * CPCAP_ADC_TIMING_IMM seems to be immediate with no timings.
> + * Please document when using.
> + */
> +enum cpcap_adc_timing {
> + CPCAP_ADC_TIMING_IMM,
> + CPCAP_ADC_TIMING_IN,
> + CPCAP_ADC_TIMING_OUT,
> +};
> +
> +/**
> + * struct cpcap_adc_phasing_tbl - cpcap phasing table
> + * @offset: offset in the phasing table
> + * @multiplier: multiplier in the phasing table
> + * @divider: divider in the phasing table
> + * @min: minimum value
> + * @max: maximum value
> + */
> +struct cpcap_adc_phasing_tbl {
> + short offset;
> + unsigned short multiplier;
> + unsigned short divider;
> + short min;
> + short max;
> +};
> +
> +/**
> + * struct cpcap_adc_conversion_tbl - cpcap conversion table
> + * @conv_type: conversion type
> + * @align_offset: align offset
> + * @conv_offset: conversion offset
> + * @cal_offset: calibration offset
> + * @multiplier: conversion multiplier
> + * @divider: conversion divider
> + */
> +struct cpcap_adc_conversion_tbl {
> + enum iio_chan_info_enum conv_type;
> + int align_offset;
> + int conv_offset;
> + int cal_offset;
> + int multiplier;
> + int divider;
> +};
> +
> +/**
> + * struct cpcap_adc_request - cpcap adc request
> + * @channel: request channel
> + * @phase_tbl: channel phasing table
> + * @conv_tbl: channel conversion table
> + * @bank_index: channel index within the bank
> + * @timing: timing settings
> + * @result: result
> + */
> +struct cpcap_adc_request {
> + int channel;
> + const struct cpcap_adc_phasing_tbl *phase_tbl;
> + const struct cpcap_adc_conversion_tbl *conv_tbl;
> + int bank_index;
> + enum cpcap_adc_timing timing;
> + int result;
> +};
> +
> +/* Phasing table for channels. Note that channels 16 & 17 use BATTP and BATTI */
> +static const struct cpcap_adc_phasing_tbl bank_phasing[] = {
> + /* Bank0 */
> + [CPCAP_ADC_AD0_BATTDETB] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_BATTP] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_VBUS] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_AD3] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_BPLUS_AD4] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_CHG_ISENSE] = {0, 0x80, 0x80, -512, 511},
> + [CPCAP_ADC_BATTI] = {0, 0x80, 0x80, -512, 511},
> + [CPCAP_ADC_USB_ID] = {0, 0x80, 0x80, 0, 1023},
> +
> + /* Bank1 */
> + [CPCAP_ADC_AD8] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_AD9] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_LICELL] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_HV_BATTP] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_TSX1_AD12] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_TSX2_AD13] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_TSY1_AD14] = {0, 0x80, 0x80, 0, 1023},
> + [CPCAP_ADC_TSY2_AD15] = {0, 0x80, 0x80, 0, 1023},
> +};
> +
> +/*
> + * Conversion table for channels. Updated during init based on calibration.
> + * Here too channels 16 & 17 use BATTP and BATTI.
> + */
> +static struct cpcap_adc_conversion_tbl bank_conversion[] = {
> + /* Bank0 */
> + [CPCAP_ADC_AD0_BATTDETB] = {
> + IIO_CHAN_INFO_PROCESSED, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_BATTP] = {
> + IIO_CHAN_INFO_PROCESSED, 0, 2400, 0, 2300, 1023,
> + },
> + [CPCAP_ADC_VBUS] = {
> + IIO_CHAN_INFO_PROCESSED, 0, 0, 0, 10000, 1023,
> + },
> + [CPCAP_ADC_AD3] = {
> + IIO_CHAN_INFO_PROCESSED, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_BPLUS_AD4] = {
> + IIO_CHAN_INFO_PROCESSED, 0, 2400, 0, 2300, 1023,
> + },
> + [CPCAP_ADC_CHG_ISENSE] = {
> + IIO_CHAN_INFO_PROCESSED, -512, 2, 0, 5000, 1023,
> + },
> + [CPCAP_ADC_BATTI] = {
> + IIO_CHAN_INFO_PROCESSED, -512, 2, 0, 5000, 1023,
> + },
> + [CPCAP_ADC_USB_ID] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> +
> + /* Bank1 */
> + [CPCAP_ADC_AD8] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_AD9] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_LICELL] = {
> + IIO_CHAN_INFO_PROCESSED, 0, 0, 0, 3400, 1023,
> + },
> + [CPCAP_ADC_HV_BATTP] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_TSX1_AD12] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_TSX2_AD13] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_TSY1_AD14] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> + [CPCAP_ADC_TSY2_AD15] = {
> + IIO_CHAN_INFO_RAW, 0, 0, 0, 1, 1,
> + },
> +};
> +
> +/*
> + * Temperature lookup table of register values to milliCelcius.
> + * REVISIT: Check the duplicate 0x3ff entry in a freezer
> + */
> +static const int temp_map[CPCAP_MAX_TEMP_LVL][2] = {
> + { 0x03ff, -40000 },
> + { 0x03ff, -35000 },
> + { 0x03ef, -30000 },
> + { 0x03b2, -25000 },
> + { 0x036c, -20000 },
> + { 0x0320, -15000 },
> + { 0x02d0, -10000 },
> + { 0x027f, -5000 },
> + { 0x022f, 0 },
> + { 0x01e4, 5000 },
> + { 0x019f, 10000 },
> + { 0x0161, 15000 },
> + { 0x012b, 20000 },
> + { 0x00fc, 25000 },
> + { 0x00d4, 30000 },
> + { 0x00b2, 35000 },
> + { 0x0095, 40000 },
> + { 0x007d, 45000 },
> + { 0x0069, 50000 },
> + { 0x0059, 55000 },
> + { 0x004b, 60000 },
> + { 0x003f, 65000 },
> + { 0x0036, 70000 },
> + { 0x002e, 75000 },
> + { 0x0027, 80000 },
> + { 0x0022, 85000 },
> + { 0x001d, 90000 },
> +};
> +
> +#define CPCAP_CHAN(_type, _index, _address, _datasheet_name) { \
> + .type = (_type), \
> + .address = (_address), \
> + .indexed = 1, \
> + .channel = (_index), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_PROCESSED), \
> + .scan_index = (_index), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 10, \
> + .storagebits = 16, \
> + .endianness = IIO_CPU, \
> + }, \
> + .datasheet_name = (_datasheet_name), \
> +}
> +
> +/*
> + * The datasheet names are from Motorola mapphone Linux kernel except
> + * for the last two which might be uncalibrated charge voltage and
> + * current.
> + */
> +static const struct iio_chan_spec cpcap_adc_channels[] = {
> + /* Bank0 */
> + CPCAP_CHAN(IIO_TEMP, 0, CPCAP_REG_ADCD0, "battdetb"),
> + CPCAP_CHAN(IIO_VOLTAGE, 1, CPCAP_REG_ADCD1, "battp"),
> + CPCAP_CHAN(IIO_VOLTAGE, 2, CPCAP_REG_ADCD2, "vbus"),
> + CPCAP_CHAN(IIO_TEMP, 3, CPCAP_REG_ADCD3, "ad3"),
> + CPCAP_CHAN(IIO_VOLTAGE, 4, CPCAP_REG_ADCD4, "ad4"),
> + CPCAP_CHAN(IIO_CURRENT, 5, CPCAP_REG_ADCD5, "chg_isense"),
> + CPCAP_CHAN(IIO_CURRENT, 6, CPCAP_REG_ADCD6, "batti"),
> + CPCAP_CHAN(IIO_VOLTAGE, 7, CPCAP_REG_ADCD7, "usb_id"),
> +
> + /* Bank1 */
> + CPCAP_CHAN(IIO_CURRENT, 8, CPCAP_REG_ADCD0, "ad8"),
> + CPCAP_CHAN(IIO_VOLTAGE, 9, CPCAP_REG_ADCD1, "ad9"),
> + CPCAP_CHAN(IIO_VOLTAGE, 10, CPCAP_REG_ADCD2, "licell"),
> + CPCAP_CHAN(IIO_VOLTAGE, 11, CPCAP_REG_ADCD3, "hv_battp"),
> + CPCAP_CHAN(IIO_VOLTAGE, 12, CPCAP_REG_ADCD4, "tsx1_ad12"),
> + CPCAP_CHAN(IIO_VOLTAGE, 13, CPCAP_REG_ADCD5, "tsx2_ad13"),
> + CPCAP_CHAN(IIO_VOLTAGE, 14, CPCAP_REG_ADCD6, "tsy1_ad14"),
> + CPCAP_CHAN(IIO_VOLTAGE, 15, CPCAP_REG_ADCD7, "tsy2_ad15"),
> +
> + /* There are two registers with multiplexed functionality */
> + CPCAP_CHAN(IIO_VOLTAGE, 16, CPCAP_REG_ADCD0, "chg_vsense"),
> + CPCAP_CHAN(IIO_CURRENT, 17, CPCAP_REG_ADCD1, "batti2"),
> +};
> +
> +static irqreturn_t cpcap_adc_irq_thread(int irq, void *data)
> +{
> + struct iio_dev *indio_dev = data;
> + struct cpcap_adc *ddata = iio_priv(indio_dev);
> + int error;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ADTRIG_DIS,
> + CPCAP_BIT_ADTRIG_DIS);
> + if (error)
> + return IRQ_NONE;
> +
> + ddata->done = true;
> + wake_up_interruptible(&ddata->wq_data_avail);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/* ADC calibration functions */
> +static void cpcap_adc_setup_calibrate(struct cpcap_adc *ddata,
> + enum cpcap_adc_channel chan)
> +{
> + unsigned int value = 0;
> + unsigned long timeout = jiffies + msecs_to_jiffies(3000);
> + int error;
> +
> + if ((chan != CPCAP_ADC_CHG_ISENSE) &&
> + (chan != CPCAP_ADC_BATTI))
> + return;
> +
> + value |= CPCAP_BIT_CAL_MODE | CPCAP_BIT_RAND0;
> + value |= ((chan << 4) &
> + (CPCAP_BIT_ADA2 | CPCAP_BIT_ADA1 | CPCAP_BIT_ADA0));
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC1,
> + CPCAP_BIT_CAL_MODE | CPCAP_BIT_ATOX |
> + CPCAP_BIT_ATO3 | CPCAP_BIT_ATO2 |
> + CPCAP_BIT_ATO1 | CPCAP_BIT_ATO0 |
> + CPCAP_BIT_ADA2 | CPCAP_BIT_ADA1 |
> + CPCAP_BIT_ADA0 | CPCAP_BIT_AD_SEL1 |
> + CPCAP_BIT_RAND1 | CPCAP_BIT_RAND0,
> + value);
> + if (error)
> + return;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ATOX_PS_FACTOR |
> + CPCAP_BIT_ADC_PS_FACTOR1 |
> + CPCAP_BIT_ADC_PS_FACTOR0,
> + 0);
> + if (error)
> + return;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ADTRIG_DIS,
> + CPCAP_BIT_ADTRIG_DIS);
> + if (error)
> + return;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ASC,
> + CPCAP_BIT_ASC);
> + if (error)
> + return;
> +
> + do {
> + schedule_timeout_uninterruptible(1);
> + error = regmap_read(ddata->reg, CPCAP_REG_ADCC2, &value);
> + if (error)
> + return;
> + } while ((value & CPCAP_BIT_ASC) && time_before(jiffies, timeout));
> +
> + if (value & CPCAP_BIT_ASC)
> + dev_err(ddata->dev,
> + "Timeout waiting for calibration to complete\n");
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC1,
> + CPCAP_BIT_CAL_MODE, 0);
> + if (error)
> + return;
> +}
> +
> +static int cpcap_adc_calibrate_one(struct cpcap_adc *ddata,
> + int channel,
> + u16 calibration_register,
> + int lower_threshold,
> + int upper_threshold)
> +{
> + unsigned int calibration_data[2];
> + unsigned short cal_data_diff;
> + int i, error;
> +
> + for (i = 0; i < CPCAP_ADC_MAX_RETRIES; i++) {
> + calibration_data[0] = 0;
> + calibration_data[1] = 0;
> + cal_data_diff = 0;
> + cpcap_adc_setup_calibrate(ddata, channel);
> + error = regmap_read(ddata->reg, calibration_register,
> + &calibration_data[0]);
> + if (error)
> + return error;
> + cpcap_adc_setup_calibrate(ddata, channel);
> + error = regmap_read(ddata->reg, calibration_register,
> + &calibration_data[1]);
> + if (error)
> + return error;
> +
> + if (calibration_data[0] > calibration_data[1])
> + cal_data_diff =
> + calibration_data[0] - calibration_data[1];
> + else
> + cal_data_diff =
> + calibration_data[1] - calibration_data[0];
> +
> + if (((calibration_data[1] >= lower_threshold) &&
> + (calibration_data[1] <= upper_threshold) &&
> + (cal_data_diff <= ST_ADC_CALIBRATE_DIFF_THRESHOLD)) ||
> + (ddata->vendor == CPCAP_VENDOR_TI)) {
> + bank_conversion[channel].cal_offset =
> + ((short)calibration_data[1] * -1) + 512;
> + dev_dbg(ddata->dev, "ch%i calibration complete: %i\n",
> + channel, bank_conversion[channel].cal_offset);
> + break;
> + }
> + usleep_range(5000, 10000);
> + }
> +
> + return 0;
> +}
> +
> +static int cpcap_adc_calibrate(struct cpcap_adc *ddata)
> +{
> + int error;
> +
> + error = cpcap_adc_calibrate_one(ddata, CPCAP_ADC_CHG_ISENSE,
> + CPCAP_REG_ADCAL1,
> + ST_ADC_CAL_CHRGI_LOW_THRESHOLD,
> + ST_ADC_CAL_CHRGI_HIGH_THRESHOLD);
> + if (error)
> + return error;
> +
> + error = cpcap_adc_calibrate_one(ddata, CPCAP_ADC_BATTI,
> + CPCAP_REG_ADCAL2,
> + ST_ADC_CAL_BATTI_LOW_THRESHOLD,
> + ST_ADC_CAL_BATTI_HIGH_THRESHOLD);
> + if (error)
> + return error;
> +
> + return 0;
> +}
> +
> +/* ADC setup, read and scale functions */
> +static void cpcap_adc_setup_bank(struct cpcap_adc *ddata,
> + struct cpcap_adc_request *req)
> +{
> + const struct cpcap_adc_ato *ato = ddata->ato;
> + unsigned short value1 = 0;
> + unsigned short value2 = 0;
> + int error;
> +
> + if (!ato)
> + return;
> +
> + switch (req->channel) {
> + case CPCAP_ADC_AD8 ... CPCAP_ADC_TSY2_AD15:
> + value1 |= CPCAP_BIT_AD_SEL1;
> + break;
> + case CPCAP_ADC_BATTP_PI16 ... CPCAP_ADC_BATTI_PI17:
> + value1 |= CPCAP_BIT_RAND1;
> + default:
> + break;
> + }
> +
> + switch (req->timing) {
> + case CPCAP_ADC_TIMING_IN:
> + value1 |= ato->ato_in;
> + value1 |= ato->atox_in;
> + value2 |= ato->adc_ps_factor_in;
> + value2 |= ato->atox_ps_factor_in;
> + break;
> + case CPCAP_ADC_TIMING_OUT:
> + value1 |= ato->ato_out;
> + value1 |= ato->atox_out;
> + value2 |= ato->adc_ps_factor_out;
> + value2 |= ato->atox_ps_factor_out;
> + break;
> +
> + case CPCAP_ADC_TIMING_IMM:
> + default:
> + break;
> + }
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC1,
> + CPCAP_BIT_CAL_MODE | CPCAP_BIT_ATOX |
> + CPCAP_BIT_ATO3 | CPCAP_BIT_ATO2 |
> + CPCAP_BIT_ATO1 | CPCAP_BIT_ATO0 |
> + CPCAP_BIT_ADA2 | CPCAP_BIT_ADA1 |
> + CPCAP_BIT_ADA0 | CPCAP_BIT_AD_SEL1 |
> + CPCAP_BIT_RAND1 | CPCAP_BIT_RAND0,
> + value1);
> + if (error)
> + return;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ATOX_PS_FACTOR |
> + CPCAP_BIT_ADC_PS_FACTOR1 |
> + CPCAP_BIT_ADC_PS_FACTOR0,
> + value2);
> + if (error)
> + return;
> +
> + if (req->timing == CPCAP_ADC_TIMING_IMM) {
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ADTRIG_DIS,
> + CPCAP_BIT_ADTRIG_DIS);
> + if (error)
> + return;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ASC,
> + CPCAP_BIT_ASC);
> + if (error)
> + return;
> + } else {
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ADTRIG_ONESHOT,
> + CPCAP_BIT_ADTRIG_ONESHOT);
> + if (error)
> + return;
> +
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ADTRIG_DIS, 0);
> + if (error)
> + return;
> + }
> +}
> +
> +/*
> + * Occasionally the ADC does not seem to start and there will be no
> + * interrupt. Let's re-init interrupt to prevent the ADC from hanging
> + * for the next request. It is unclear why this happens, but the next
> + * request will usually work after doing this.
> + */
> +static void cpcap_adc_quirk_reset_lost_irq(struct cpcap_adc *ddata)
> +{
> + int error;
> +
> + dev_info(ddata->dev, "lost ADC irq, attempting to reinit\n");
> + disable_irq(ddata->irq);
> + error = regmap_update_bits(ddata->reg, CPCAP_REG_ADCC2,
> + CPCAP_BIT_ADTRIG_DIS,
> + CPCAP_BIT_ADTRIG_DIS);
> + if (error)
> + dev_warn(ddata->dev, "%s reset failed: %i\n",
> + __func__, error);
> + enable_irq(ddata->irq);
> +}
> +
> +static int cpcap_adc_start_bank(struct cpcap_adc *ddata,
> + struct cpcap_adc_request *req)
> +{
> + int i, error;
> +
> + req->timing = CPCAP_ADC_TIMING_IMM;
> + ddata->done = false;
> +
> + for (i = 0; i < CPCAP_ADC_MAX_RETRIES; i++) {
> + cpcap_adc_setup_bank(ddata, req);
> + error = wait_event_interruptible_timeout(ddata->wq_data_avail,
> + ddata->done,
> + msecs_to_jiffies(50));
> + if (error > 0)
> + return 0;
> +
> + if (error == 0) {
> + cpcap_adc_quirk_reset_lost_irq(ddata);
> + error = -ETIMEDOUT;
> + continue;
> + }
> +
> + if (error < 0)
> + return error;
> + }
> +
> + return error;
> +}
> +
> +static void cpcap_adc_phase(struct cpcap_adc_request *req)
> +{
> + const struct cpcap_adc_conversion_tbl *conv_tbl = req->conv_tbl;
> + const struct cpcap_adc_phasing_tbl *phase_tbl = req->phase_tbl;
> + int index = req->channel;
> +
> + /* Remuxed channels 16 and 17 use BATTP and BATTI entries */
> + switch (req->channel) {
> + case CPCAP_ADC_BATTP:
> + case CPCAP_ADC_BATTP_PI16:
> + index = req->bank_index;
> + req->result -= phase_tbl[index].offset;
> + req->result -= CPCAP_FOUR_POINT_TWO_ADC;
> + req->result *= phase_tbl[index].multiplier;
> + if (phase_tbl[index].divider == 0)
> + return;
> + req->result /= phase_tbl[index].divider;
> + req->result += CPCAP_FOUR_POINT_TWO_ADC;
> + break;
> + case CPCAP_ADC_BATTI_PI17:
> + index = req->bank_index;
> + /* fallthrough */
> + default:
> + req->result += conv_tbl[index].cal_offset;
> + req->result += conv_tbl[index].align_offset;
> + req->result *= phase_tbl[index].multiplier;
> + if (phase_tbl[index].divider == 0)
> + return;
> + req->result /= phase_tbl[index].divider;
> + req->result += phase_tbl[index].offset;
> + break;
> + }
> +
> + if (req->result < phase_tbl[index].min)
> + req->result = phase_tbl[index].min;
> + else if (req->result > phase_tbl[index].max)
> + req->result = phase_tbl[index].max;
> +}
> +
> +/* Looks up temperatures in a table and calculates averages if needed */
> +static int cpcap_adc_table_to_millicelcius(unsigned short value)
> +{
> + int i, result = 0, alpha;
> +
> + if (value <= temp_map[CPCAP_MAX_TEMP_LVL - 1][0])
> + return temp_map[CPCAP_MAX_TEMP_LVL - 1][1];
> +
> + if (value >= temp_map[0][0])
> + return temp_map[0][1];
> +
> + for (i = 0; i < CPCAP_MAX_TEMP_LVL - 1; i++) {
> + if ((value <= temp_map[i][0]) &&
> + (value >= temp_map[i + 1][0])) {
> + if (value == temp_map[i][0]) {
> + result = temp_map[i][1];
> + } else if (value == temp_map[i + 1][0]) {
> + result = temp_map[i + 1][1];
> + } else {
> + alpha = ((value - temp_map[i][0]) * 1000) /
> + (temp_map[i + 1][0] - temp_map[i][0]);
> +
> + result = temp_map[i][1] +
> + ((alpha * (temp_map[i + 1][1] -
> + temp_map[i][1])) / 1000);
> + }
> + break;
> + }
> + }
> +
> + return result;
> +}
> +
> +static void cpcap_adc_convert(struct cpcap_adc_request *req)
> +{
> + const struct cpcap_adc_conversion_tbl *conv_tbl = req->conv_tbl;
> + int index = req->channel;
> +
> + /* Remuxed channels 16 and 17 use BATTP and BATTI entries */
> + switch (req->channel) {
> + case CPCAP_ADC_BATTP_PI16:
> + index = CPCAP_ADC_BATTP;
> + break;
> + case CPCAP_ADC_BATTI_PI17:
> + index = CPCAP_ADC_BATTI;
> + break;
> + default:
> + break;
> + }
> +
> + /* No conversion for raw channels */
> + if (conv_tbl[index].conv_type == IIO_CHAN_INFO_RAW)
> + return;
> +
> + /* Temperatures use a lookup table instead of conversion table */
> + if ((req->channel == CPCAP_ADC_AD0_BATTDETB) ||
> + (req->channel == CPCAP_ADC_AD3)) {
> + req->result =
> + cpcap_adc_table_to_millicelcius(req->result);
> +
> + return;
> + }
> +
> + /* All processed channels use a conversion table */
> + req->result *= conv_tbl[index].multiplier;
> + if (conv_tbl[index].divider == 0)
> + return;
> + req->result /= conv_tbl[index].divider;
> + req->result += conv_tbl[index].conv_offset;
> +}
> +
> +/*
> + * REVISIT: Check if timed sampling can use multiple channels at the
> + * same time. If not, replace channel_mask with just channel.
> + */
> +static int cpcap_adc_read_bank_scaled(struct cpcap_adc *ddata,
> + struct cpcap_adc_request *req)
> +{
> + int calibration_data, error, addr;
> +
> + if (ddata->vendor == CPCAP_VENDOR_TI) {
> + error = regmap_read(ddata->reg, CPCAP_REG_ADCAL1,
> + &calibration_data);
> + if (error)
> + return error;
> + bank_conversion[CPCAP_ADC_CHG_ISENSE].cal_offset =
> + ((short)calibration_data * -1) + 512;
> +
> + error = regmap_read(ddata->reg, CPCAP_REG_ADCAL2,
> + &calibration_data);
> + if (error)
> + return error;
> + bank_conversion[CPCAP_ADC_BATTI].cal_offset =
> + ((short)calibration_data * -1) + 512;
> + }
> +
> + addr = CPCAP_REG_ADCD0 + req->bank_index * 4;
> +
> + error = regmap_read(ddata->reg, addr, &req->result);
> + if (error)
> + return error;
> +
> + req->result &= 0x3ff;
> + cpcap_adc_phase(req);
> + cpcap_adc_convert(req);
> +
> + return 0;
> +}
> +
> +static int cpcap_adc_init_request(struct cpcap_adc_request *req,
> + int channel)
> +{
> + req->channel = channel;
> + req->phase_tbl = bank_phasing;
> + req->conv_tbl = bank_conversion;
> +
> + switch (channel) {
> + case CPCAP_ADC_AD0_BATTDETB ... CPCAP_ADC_USB_ID:
> + req->bank_index = channel;
> + break;
> + case CPCAP_ADC_AD8 ... CPCAP_ADC_TSY2_AD15:
> + req->bank_index = channel - 8;
> + break;
> + case CPCAP_ADC_BATTP_PI16:
> + req->bank_index = CPCAP_ADC_BATTP;
> + break;
> + case CPCAP_ADC_BATTI_PI17:
> + req->bank_index = CPCAP_ADC_BATTI;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int cpcap_adc_read(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct cpcap_adc *ddata = iio_priv(indio_dev);
> + struct cpcap_adc_request req;
> + int error;
> +
> + error = cpcap_adc_init_request(&req, chan->channel);
> + if (error)
> + return error;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&ddata->lock);
> + error = cpcap_adc_start_bank(ddata, &req);
> + if (error)
> + goto err_unlock;
> + error = regmap_read(ddata->reg, chan->address, val);
> + if (error)
> + goto err_unlock;
> + mutex_unlock(&ddata->lock);
> + break;
> + case IIO_CHAN_INFO_PROCESSED:
> + mutex_lock(&ddata->lock);
> + error = cpcap_adc_start_bank(ddata, &req);
> + if (error)
> + goto err_unlock;
> + error = cpcap_adc_read_bank_scaled(ddata, &req);
> + if (error)
> + goto err_unlock;
> + mutex_unlock(&ddata->lock);
> + *val = req.result;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT;
> +
> +err_unlock:
> + mutex_unlock(&ddata->lock);
> + dev_err(ddata->dev, "error reading ADC: %i\n", error);
> +
> + return error;
> +}
> +
> +static const struct iio_info cpcap_adc_info = {
> + .read_raw = &cpcap_adc_read,
> + .driver_module = THIS_MODULE,
> +};
> +
> +/*
> + * Configuration for Motorola mapphone series such as droid 4.
> + * Copied from the Motorola mapphone kernel tree.
> + */
> +static const struct cpcap_adc_ato mapphone_adc = {
> + .ato_in = 0x0480,
> + .atox_in = 0,
> + .adc_ps_factor_in = 0x0200,
> + .atox_ps_factor_in = 0,
> + .ato_out = 0,
> + .atox_out = 0,
> + .adc_ps_factor_out = 0,
> + .atox_ps_factor_out = 0,
> +};
> +
> +static const struct of_device_id cpcap_adc_id_table[] = {
> + {
> + .compatible = "motorola,cpcap-adc",
> + },
> + {
> + .compatible = "motorola,mapphone-cpcap-adc",
> + .data = &mapphone_adc,
> + },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_adc_id_table);
> +
> +static int cpcap_adc_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *match;
> + struct cpcap_adc *ddata;
> + struct iio_dev *indio_dev;
> + int error;
> +
> + match = of_match_device(of_match_ptr(cpcap_adc_id_table),
> + &pdev->dev);
> + if (!match)
> + return -EINVAL;
> +
> + if (!match->data) {
> + dev_err(&pdev->dev, "no configuration data found\n");
> +
> + return -ENODEV;
> + }
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ddata));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed to allocate iio device\n");
> +
> + return -ENOMEM;
> + }
> + ddata = iio_priv(indio_dev);
> + ddata->ato = match->data;
> + ddata->dev = &pdev->dev;
> +
> + mutex_init(&ddata->lock);
> + init_waitqueue_head(&ddata->wq_data_avail);
> +
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->channels = cpcap_adc_channels;
> + indio_dev->num_channels = ARRAY_SIZE(cpcap_adc_channels);
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->info = &cpcap_adc_info;
> +
> + ddata->reg = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!ddata->reg)
> + return -ENODEV;
> +
> + error = cpcap_get_vendor(ddata->dev, ddata->reg, &ddata->vendor);
> + if (error)
> + return error;
> +
> + platform_set_drvdata(pdev, indio_dev);
> +
> + ddata->irq = platform_get_irq_byname(pdev, "adcdone");
> + if (!ddata->irq)
> + return -ENODEV;
> +
> + error = devm_request_threaded_irq(&pdev->dev, ddata->irq, NULL,
> + cpcap_adc_irq_thread,
> + IRQF_TRIGGER_NONE,
> + "cpcap-adc", indio_dev);
> + if (error) {
> + dev_err(&pdev->dev, "could not get irq: %i\n",
> + error);
> +
> + return error;
> + }
> +
> + error = cpcap_adc_calibrate(ddata);
> + if (error)
> + return error;
> +
> + dev_info(&pdev->dev, "CPCAP ADC device probed\n");
> +
> + return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
> +
> +static struct platform_driver cpcap_adc_driver = {
> + .driver = {
> + .name = "cpcap_adc",
> + .of_match_table = of_match_ptr(cpcap_adc_id_table),
> + },
> + .probe = cpcap_adc_probe,
> +};
> +
> +module_platform_driver(cpcap_adc_driver);
> +
> +MODULE_ALIAS("platform:cpcap_adc");
> +MODULE_DESCRIPTION("CPCAP ADC driver");
> +MODULE_AUTHOR("Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: ata: add DT bindings for ahci-dm816 SATA controller
From: Bartosz Golaszewski @ 2017-03-25 18:05 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, linux-devicetree,
Sergei Shtylyov, Patrick Titiano, Tony Lindgren, Kevin Hilman,
Michael Turquette, Neil Armstrong, LKML, Bartosz Golaszewski,
Rob Herring
In-Reply-To: <1489489491-14195-2-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2017-03-14 12:04 GMT+01:00 Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>:
> Add DT bindings for the onboard SATA controller present on the DM816x
> SoCs.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> .../devicetree/bindings/ata/ahci-dm816.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/ata/ahci-dm816.txt
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-dm816.txt b/Documentation/devicetree/bindings/ata/ahci-dm816.txt
> new file mode 100644
> index 0000000..f8c535f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-dm816.txt
> @@ -0,0 +1,21 @@
> +Device tree binding for the TI DM816 AHCI SATA Controller
> +---------------------------------------------------------
> +
> +Required properties:
> + - compatible: must be "ti,dm816-ahci"
> + - reg: physical base address and size of the register region used by
> + the controller (as defined by the AHCI 1.1 standard)
> + - interrupts: interrupt specifier (refer to the interrupt binding)
> + - clocks: list of phandle and clock specifier pairs (or only
> + phandles for clock providers with '0' defined for
> + #clock-cells); two clocks must be specified: the functional
> + clock and an external reference clock
> +
> +Example:
> +
> + sata: sata@4a140000 {
> + compatible = "ti,dm816-ahci";
> + reg = <0x4a140000 0x10000>;
> + interrupts = <16>;
> + clocks = <&sysclk5_ck>, <&sata_refclk>;
> + };
> --
> 2.9.3
>
Hi Tejun,
can you pick this one up for your tree as well?
Thanks,
Bartosz
--
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 v3 07/11] iio: adc: sun4i-gpadc-iio: add support for A33 thermal sensor
From: Jonathan Cameron @ 2017-03-25 17:48 UTC (permalink / raw)
To: Quentin Schulz, dmitry.torokhov, robh+dt, mark.rutland,
maxime.ripard, wens, lee.jones, linux, knaack.h, lars, pmeerw
Cc: thomas.petazzoni, devicetree, linux-iio, linux-kernel,
linux-sunxi, icenowy, linux-input, linux-arm-kernel
In-Reply-To: <20170321153611.16228-8-quentin.schulz@free-electrons.com>
On 21/03/17 15:36, Quentin Schulz wrote:
> This adds support for the Allwinner A33 thermal sensor.
>
> Unlike the A10, A13 and A31, the Allwinner A33 only has one channel
> which is dedicated to the thermal sensor. Moreover, its thermal sensor
> does not generate interruptions, thus we only need to directly read the
> register storing the temperature value.
>
> The MFD used by the A10, A13 and A31, was created to avoid breaking the
> DT binding, but since the nodes for the ADC weren't there for the A33,
> it is not needed.
>
> Though the A33 does not have an internal ADC, it has a thermal sensor
> which shares the same registers with GPADC of the already supported SoCs
> and almost the same bits, for the same purpose (thermal sensor).
>
> The thermal sensor behaves exactly the same (except the presence of
> interrupts or not) on the different SoCs.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Jonathan Cameron <jic23@kernel.org>
I'm assuming this is going through your tree Lee, let me know if you want
to do it differently.
Jonathan
> ---
>
> v3:
> - switched compatible from allwinner,sun8i-a33-gpadc-iio to
> allwinner,sun8i-a33-ths to better reflect the datasheet's name,
> - fixed the non-working if (!IS_ENABLED(THERMAL_OF)) by prefixing it with
> CONFIG,
>
> v2:
> - removed added comments in Kconfig,
> - simplified Kconfig depends on condition,
> - removed THERMAL_OF requirement for sun8i,
> - renamed sun8i_gpadc_channels to sun8i_a33_gpadc_channels,
> - renamed use_dt boolean in no_irq as it reflects better why we need it,
> - removed spurious/unneeded modifications done in v1,
>
> drivers/iio/adc/Kconfig | 2 +-
> drivers/iio/adc/sun4i-gpadc-iio.c | 100 ++++++++++++++++++++++++++++++++++++--
> include/linux/mfd/sun4i-gpadc.h | 4 ++
> 3 files changed, 102 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9f8b4b1..8c8ead6 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -562,7 +562,7 @@ config STX104
> config SUN4I_GPADC
> tristate "Support for the Allwinner SoCs GPADC"
> depends on IIO
> - depends on MFD_SUN4I_GPADC
> + depends on MFD_SUN4I_GPADC || MACH_SUN8I
> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> GPADC. This ADC provides 4 channels which can be used as an ADC or as
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 7cb997a..74705aa 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -85,6 +85,12 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
> };
>
> +static const struct gpadc_data sun8i_a33_gpadc_data = {
> + .temp_offset = -1662,
> + .temp_scale = 162,
> + .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
> +};
> +
> struct sun4i_gpadc_iio {
> struct iio_dev *indio_dev;
> struct completion completion;
> @@ -96,6 +102,7 @@ struct sun4i_gpadc_iio {
> unsigned int temp_data_irq;
> atomic_t ignore_temp_data_irq;
> const struct gpadc_data *data;
> + bool no_irq;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> };
> @@ -138,6 +145,23 @@ static const struct iio_chan_spec sun4i_gpadc_channels_no_temp[] = {
> SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
> };
>
> +static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> + .datasheet_name = "temp_adc",
> + },
> +};
> +
> +static const struct regmap_config sun4i_gpadc_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> +};
> +
> static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
> unsigned int irq)
> {
> @@ -247,6 +271,17 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> {
> struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> + if (info->no_irq) {
> + pm_runtime_get_sync(indio_dev->dev.parent);
> +
> + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> +
> + pm_runtime_mark_last_busy(indio_dev->dev.parent);
> + pm_runtime_put_autosuspend(indio_dev->dev.parent);
> +
> + return 0;
> + }
> +
> return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
> }
>
> @@ -454,6 +489,58 @@ static int sun4i_irq_init(struct platform_device *pdev, const char *name,
> return 0;
> }
>
> +static const struct of_device_id sun4i_gpadc_of_id[] = {
> + {
> + .compatible = "allwinner,sun8i-a33-ths",
> + .data = &sun8i_a33_gpadc_data,
> + },
> + { /* sentinel */ }
> +};
> +
> +static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> + struct iio_dev *indio_dev)
> +{
> + struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
> + const struct of_device_id *of_dev;
> + struct thermal_zone_device *tzd;
> + struct resource *mem;
> + void __iomem *base;
> + int ret;
> +
> + of_dev = of_match_device(sun4i_gpadc_of_id, &pdev->dev);
> + if (!of_dev)
> + return -ENODEV;
> +
> + info->no_irq = true;
> + info->data = (struct gpadc_data *)of_dev->data;
> + indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
> + indio_dev->channels = sun8i_a33_gpadc_channels;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun4i_gpadc_regmap_config);
> + if (IS_ERR(info->regmap)) {
> + ret = PTR_ERR(info->regmap);
> + dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
> + return ret;
> + }
> +
> + if (!IS_ENABLED(CONFIG_THERMAL_OF))
> + return 0;
> +
> + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, info,
> + &sun4i_ts_tz_ops);
> + if (IS_ERR(tzd))
> + dev_err(&pdev->dev, "could not register thermal sensor: %ld\n",
> + PTR_ERR(tzd));
> +
> + return PTR_ERR_OR_ZERO(tzd);
> +}
> +
> static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
> struct iio_dev *indio_dev)
> {
> @@ -462,6 +549,7 @@ static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
> dev_get_drvdata(pdev->dev.parent);
> int ret;
>
> + info->no_irq = false;
> info->regmap = sun4i_gpadc_dev->regmap;
>
> indio_dev->num_channels = ARRAY_SIZE(sun4i_gpadc_channels);
> @@ -561,7 +649,11 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> indio_dev->info = &sun4i_gpadc_iio_info;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> - ret = sun4i_gpadc_probe_mfd(pdev, indio_dev);
> + if (pdev->dev.of_node)
> + ret = sun4i_gpadc_probe_dt(pdev, indio_dev);
> + else
> + ret = sun4i_gpadc_probe_mfd(pdev, indio_dev);
> +
> if (ret)
> return ret;
>
> @@ -580,7 +672,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> return 0;
>
> err_map:
> - if (IS_ENABLED(CONFIG_THERMAL_OF))
> + if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> iio_map_array_unregister(indio_dev);
>
> pm_runtime_put(&pdev->dev);
> @@ -592,10 +684,11 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> static int sun4i_gpadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> - if (IS_ENABLED(CONFIG_THERMAL_OF))
> + if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> iio_map_array_unregister(indio_dev);
>
> return 0;
> @@ -611,6 +704,7 @@ static const struct platform_device_id sun4i_gpadc_id[] = {
> static struct platform_driver sun4i_gpadc_driver = {
> .driver = {
> .name = "sun4i-gpadc-iio",
> + .of_match_table = sun4i_gpadc_of_id,
> .pm = &sun4i_gpadc_pm_ops,
> },
> .id_table = sun4i_gpadc_id,
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 509e736..139872c 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -38,6 +38,10 @@
> #define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x) (GENMASK(3, 0) & BIT(x))
> #define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK GENMASK(3, 0)
>
> +/* TP_CTRL1 bits for sun8i SoCs */
> +#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
> +#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
> +
> #define SUN4I_GPADC_CTRL2 0x08
>
> #define SUN4I_GPADC_CTRL2_TP_SENSITIVE_ADJUST(x) ((GENMASK(3, 0) & (x)) << 28)
>
^ permalink raw reply
* Re: [PATCH v3 3/4] iio: adc: Add Maxim max9611 ADC driver
From: Jonathan Cameron @ 2017-03-25 17:37 UTC (permalink / raw)
To: jacopo
Cc: Jacopo Mondi, geert-Td1EMuHUCqxL1ZNQvxDV9g,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170325171735.GA5981@w540>
On 25/03/17 17:21, jacopo wrote:
> Hi Jonathan,
> thanks for review
>
> On Sat, Mar 25, 2017 at 03:45:05PM +0000, Jonathan Cameron wrote:
>> On 24/03/17 15:28, Jacopo Mondi wrote:
>>> Add iio driver for Maxim max9611 and max9612 current-sense amplifiers
>>> with 12-bits ADC interface.
>>>
>>> Datasheet publicly available at:
>>> https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
>> A few more little things inline. Coming together nicely.
>>
>> The channel set here is just odd enough that it might aid review to have
>> a quick listing of the resulting sysfs entries. One or two authors do
>> this an it is always useful for a quick sanity check.
>>
>> Perhaps even a set of typical values. Put this below the --- as we don't
>> need it in the git history, only to assist lazy reviewers like me ;)
>>
>
> I included the output of iio_info in the cover letter, I can add some
> more info here for sure!
Just goes to show I don't always remember to check cover letters ;)
>
>> Thanks,
>>
>> Jonathan
>>> ---
>>> drivers/iio/adc/Kconfig | 10 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/max9611.c | 590 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 601 insertions(+)
>>> create mode 100644 drivers/iio/adc/max9611.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index dedae7a..82f2e7b8 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -354,6 +354,16 @@ config MAX1363
>>> To compile this driver as a module, choose M here: the module will be
>>> called max1363.
>>>
>>> +config MAX9611
>>> + tristate "Maxim max9611/max9612 ADC driver"
>>> + depends on I2C
>>> + help
>>> + Say yes here to build support for Maxim max9611/max9612 current sense
>>> + amplifier with 12-bits ADC interface.
>>> +
>>> + To compile this driver as a module, choose M here: the module will be
>>> + called max9611.
>>> +
>>> config MCP320X
>>> tristate "Microchip Technology MCP3x01/02/04/08"
>>> depends on SPI
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index d001262..149f979 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -34,6 +34,7 @@ obj-$(CONFIG_LTC2485) += ltc2485.o
>>> obj-$(CONFIG_MAX1027) += max1027.o
>>> obj-$(CONFIG_MAX11100) += max11100.o
>>> obj-$(CONFIG_MAX1363) += max1363.o
>>> +obj-$(CONFIG_MAX9611) += max9611.o
>>> obj-$(CONFIG_MCP320X) += mcp320x.o
>>> obj-$(CONFIG_MCP3422) += mcp3422.o
>>> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>>> diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c
>>> new file mode 100644
>>> index 0000000..61566ec
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/max9611.c
>>> @@ -0,0 +1,590 @@
>>> +/*
>>> + * iio/adc/max9611.c
>>> + *
>>> + * Maxim max9611/max9612 high side current sense amplifier with
>>> + * 12-bit ADC interface.
>>> + *
>>> + * Copyright (C) 2017 Jacopo Mondi
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +/*
>>> + * This driver supports input common-mode voltage, current-sense
>>> + * amplifier with programmable gains and die temperature reading from
>>> + * Maxim max9611/max9612.
>>> + *
>>> + * Op-amp, analog comparator, and watchdog functionalities are not
>>> + * supported by this driver.
>>> + */
>>> +
>>> +#include <linux/delay.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +#include <linux/module.h>
>>> +
>>> +#define DRIVER_NAME "max9611"
>>> +
>>> +/* max9611 register addresses */
>>> +#define MAX9611_REG_CSA_DATA 0x00
>>> +#define MAX9611_REG_RS_DATA 0x02
>>> +#define MAX9611_REG_TEMP_DATA 0x08
>>> +#define MAX9611_REG_CTRL1 0x0a
>>> +#define MAX9611_REG_CTRL2 0x0b
>>> +
>>> +/* max9611 REG1 mux configuration options */
>>> +#define MAX9611_MUX_MASK 0x07
>>> +#define MAX9611_MUX_SENSE_1x 0x00
>>> +#define MAX9611_MUX_SENSE_4x 0x01
>>> +#define MAX9611_MUX_SENSE_8x 0x02
>>> +#define MAX9611_INPUT_VOLT 0x03
>>> +#define MAX9611_MUX_TEMP 0x06
>>> +
>>> +/* max9611 voltage (both csa and input) helper macros */
>>> +#define MAX9611_VOLTAGE_SHIFT 0x04
>>> +#define MAX9611_VOLTAGE_RAW(_r) ((_r) >> MAX9611_VOLTAGE_SHIFT)
>>> +
>>> +/*
>>> + * max9611 current sense amplifier voltage output:
>>> + * LSB and offset values depends on selected gain (1x, 4x, 8x)
>>> + *
>>> + * GAIN LSB (nV) OFFSET (LSB steps)
>>> + * 1x 107500 1
>>> + * 4x 26880 1
>>> + * 8x 13440 3
>>> + *
>>> + * The complete formula to calculate current sense voltage is:
>>> + * (((adc_read >> 4) - offset) / ((1 / LSB) * 10^-3)
>>> + */
>>> +#define CSA_VOLT_1x_LSB_nV 107500
>>> +#define CSA_VOLT_4x_LSB_nV 26880
>>> +#define CSA_VOLT_8x_LSB_nV 13440
>>> +
>>> +#define CSA_VOLT_1x_OFFS_RAW 1
>>> +#define CSA_VOLT_4x_OFFS_RAW 1
>>> +#define CSA_VOLT_8x_OFFS_RAW 3
>>> +
>>> +/*
>>> + * max9611 common input mode (CIM): LSB is 14mV, with 14mV offset at 25 C
>>> + *
>>> + * The complete formula to calculate input common voltage is:
>>> + * (((adc_read >> 4) * 1000) - offset) / (1 / 14 * 1000)
>>> + */
>>> +#define CIM_VOLTAGE_LSB_mV 14
>>> +#define CIM_VOLTAGE_OFFSET_RAW 1
>>> +
>>> +/*
>>> + * max9611 temperature reading: LSB is 0.48 degrees Celsius
>>> + *
>>> + * The complete formula to calculate temperature is:
>>> + * ((adc_read >> 7) * 1000) / (1 / 0.48 * 1000)
>>> + */
>> I'd prefer these defines to be prefixed with MAX9611_
>> Easiest to just do the lot. Some of these are 'standard' enough
>> the might well clash with something that turns up in an included header
>> at somepoint in the future.
>>
>>> +#define TEMP_SHIFT 0x07
>>> +#define TEMP_MAX_RAW_POS 0x7f80
>>> +#define TEMP_MAX_RAW_NEG 0xff80
>>> +#define TEMP_MIN_RAW_NEG 0xd980
>>> +#define TEMP_MASK ((1 << TEMP_SHIFT) - 1)
>>> +#define TEMP_RAW(_r) ((_r) >> TEMP_SHIFT)
>>> +#define TEMP_LSB_mC 480
>>> +#define TEMP_SCALE_NUM 1000
>>> +#define TEMP_SCALE_DIV 2083
>>> +
>>> +struct max9611_dev {
>>> + struct device *dev;
>>> + struct i2c_client *i2c_client;
>>> + struct mutex lock;
>>> + unsigned int shunt_resistor_uohm;
>>> +};
>>> +
>>> +enum max9611_conf_ids {
>>> + CONF_SENSE_1x,
>>> + CONF_SENSE_4x,
>>> + CONF_SENSE_8x,
>>> + CONF_IN_VOLT,
>>> + CONF_TEMP,
>>> +};
>>> +
>>> +/**
>>> + * max9611_mux_conf - associate ADC mux configuration with register address
>>> + * where data shall be read from
>>> + */
>>> +static unsigned int max9611_mux_conf[][2] = {
>>> + /* CONF_SENSE_1x */
>>> + { MAX9611_MUX_SENSE_1x, MAX9611_REG_CSA_DATA },
>>> + /* CONF_SENSE_4x */
>>> + { MAX9611_MUX_SENSE_4x, MAX9611_REG_CSA_DATA },
>>> + /* CONF_SENSE_8x */
>>> + { MAX9611_MUX_SENSE_8x, MAX9611_REG_CSA_DATA },
>>> + /* CONF_IN_VOLT */
>>> + { MAX9611_INPUT_VOLT, MAX9611_REG_RS_DATA },
>>> + /* CONF_TEMP */
>>> + { MAX9611_MUX_TEMP, MAX9611_REG_TEMP_DATA },
>>> +};
>>> +
>>> +enum max9611_csa_gain {
>>> + CSA_GAIN_1x,
>>> + CSA_GAIN_4x,
>>> + CSA_GAIN_8x,
>>> +};
>>> +
>>> +enum max9611_csa_gain_params {
>>> + CSA_GAIN_LSB_nV,
>>> + CSA_GAIN_OFFS_RAW,
>>> +};
>>> +
>>> +/**
>>> + * max9611_csa_gain_conf - associate gain multiplier with LSB and
>>> + * offset values.
>>> + *
>>> + * Group together parameters associated with configurable gain
>>> + * on current sense amplifier path to ADC interface.
>>> + * Current sense read routine adjusts gain until it gets a meaningful
>>> + * value; use this structure to retrieve the correct LSB and offset values.
>>> + */
>>> +static unsigned int max9611_gain_conf[][2] = {
>>> + { /* [0] CSA_GAIN_1x */
>>> + CSA_VOLT_1x_LSB_nV,
>>> + CSA_VOLT_1x_OFFS_RAW,
>>> + },
>>> + { /* [1] CSA_GAIN_4x */
>>> + CSA_VOLT_4x_LSB_nV,
>>> + CSA_VOLT_4x_OFFS_RAW,
>>> + },
>>> + { /* [2] CSA_GAIN_8x */
>>> + CSA_VOLT_8x_LSB_nV,
>>> + CSA_VOLT_8x_OFFS_RAW,
>>> + },
>>> +};
>>> +
>>> +enum max9611_chan_addrs {
>>> + MAX9611_CHAN_VOLTAGE_INPUT,
>>> + MAX9611_CHAN_VOLTAGE_SENSE,
>>> + MAX9611_CHAN_TEMPERATURE,
>>> + MAX9611_CHAN_CURRENT_LOAD,
>>> + MAX9611_CHAN_POWER_LOAD,
>>> +};
>>> +
>>> +static struct iio_chan_spec max9611_channels[] = {
>>> + {
>>> + .type = IIO_TEMP,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> + BIT(IIO_CHAN_INFO_SCALE),
>>> + .address = MAX9611_CHAN_TEMPERATURE,
>>> + },
>>> + {
>>> + .type = IIO_VOLTAGE,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> + BIT(IIO_CHAN_INFO_SCALE) |
>>> + BIT(IIO_CHAN_INFO_OFFSET),
>>> + .address = MAX9611_CHAN_VOLTAGE_INPUT,
>>> + .indexed = 1,
>>> + .channel = 1,
>>> + },
>>> + {
>>> + .type = IIO_VOLTAGE,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>>> + .address = MAX9611_CHAN_VOLTAGE_SENSE,
>>> + .indexed = 1,
>>> + .channel = 0,
>> Unusual to have the channels in here other than in channel order...
>>> + },
>>> + {
>>> + .type = IIO_CURRENT,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>>> + .address = MAX9611_CHAN_CURRENT_LOAD,
>>> + },
>>> + {
>>> + .type = IIO_POWER,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>>> + .address = MAX9611_CHAN_POWER_LOAD
>>> + },
>>> +};
>>> +
>>> +/**
>>> + * max9611_read_single() - read a single vale from ADC interface
>> value
>>> + *
>>> + * Data registers are 16 bit long, spread between two 8 bit registers
>>> + * with consecutive addresses.
>>> + * Configure ADC mux first, then read register at address "reg_addr".
>>> + * The smbus_read_word routine asks for 16 bits and the ADC is kind enough
>>> + * to return values from "reg_addr" and "reg_addr + 1" consecutively.
>>> + *
>>> + * @max9611: max9611 device
>>> + * @selector: index for mux and register configuration
>>> + * @raw_val: the value returned from ADC
>>> + */
>>> +static int max9611_read_single(struct max9611_dev *max9611,
>>> + enum max9611_conf_ids selector,
>>> + u16 *raw_val)
>>> +{
>>> + int ret;
>>> +
>>> + u8 mux_conf = max9611_mux_conf[selector][0] & MAX9611_MUX_MASK;
>>> + u8 reg_addr = max9611_mux_conf[selector][1];
>>> +
>>> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
>>> + MAX9611_REG_CTRL1, mux_conf);
>>> + if (ret) {
>>> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
>>> + MAX9611_REG_CTRL1, mux_conf);
>>> + return ret;
>>> + }
>>> +
>>> + /*
>>> + * need a delay here to make register configuration
>>> + * stabilize. 1 msec at least, from empirical testing.
>>> + */
>>> + usleep_range(1000, 2000);
>>> +
>>> + ret = i2c_smbus_read_word_swapped(max9611->i2c_client, reg_addr);
>>> + if (ret < 0) {
>>> + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
>>> + reg_addr);
>>> + return ret;
>>> + }
>>> + *raw_val = ret;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * max9611_read_csa_voltage() - read current sense amplifier output voltage
>>> + *
>>> + * Current sense amplifier output voltage is read through a configurable
>>> + * 1x, 4x or 8x gain.
>>> + * Start with plain 1x gain, and adjust gain control properly until a
>>> + * meaningful value is read from ADC output.
>>> + *
>>> + * @max9611: max9611 device
>>> + * @adc_raw: raw value read from ADC output
>>> + * @csa_gain: gain configuration option selector
>>> + */
>>> +static int max9611_read_csa_voltage(struct max9611_dev *max9611,
>>> + u16 *adc_raw,
>>> + enum max9611_csa_gain *csa_gain)
>>> +{
>>> + enum max9611_conf_ids gain_selectors[] = {
>>> + CONF_SENSE_1x,
>>> + CONF_SENSE_4x,
>>> + CONF_SENSE_8x
>>> + };
>>> + unsigned int i;
>>> + int ret;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(gain_selectors); ++i) {
>>> + ret = max9611_read_single(max9611, gain_selectors[i], adc_raw);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (*adc_raw > 0) {
>>> + *csa_gain = gain_selectors[i];
>>> + return 0;
>>> + }
>>> + }
>>> +
>>> + return -EIO;
>>> +}
>>> +
>>> +static int max9611_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int *val, int *val2, long mask)
>>> +{
>>> + struct max9611_dev *dev = iio_priv(indio_dev);
>>> + enum max9611_csa_gain gain_selector;
>>> + unsigned int *csa_gain;
>>> + u16 adc_data;
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + mutex_lock(&dev->lock);
>>> +
>>> + switch (chan->address) {
>>> + case MAX9611_CHAN_TEMPERATURE:
>>> + ret = max9611_read_single(dev, CONF_TEMP,
>>> + &adc_data);
>>> + if (ret)
>> I'm not personally keen on jumping out of deep indentations like this
>> just save on repeating one line. I'd pull the unlock back here and return
>> directly as I feel it'll improve readability.
>
> I'll change it, with this and Peter's suggestion on this function in
> his review.
>
>> Actually come to think of it, why does the lock need to be held for
>> the next line anyway? adc_data is on the stack so doesn't matter if we
>> have concurrent readers, once the i2c transaction is finished.
>> Just unlock before checking ret.
>
> I naively overvalued 'style' (ret assignment and check in two
> consecutive lines) over practicality... I'll change this
>
>>> + goto unlock_fail;
>>> +
>>> + *val = TEMP_RAW(adc_data);
>>> +
>>> + mutex_unlock(&dev->lock);
>>> + return IIO_VAL_INT;
>>> +
>>> + case MAX9611_CHAN_VOLTAGE_INPUT:
>>> + ret = max9611_read_single(dev, CONF_IN_VOLT,
>>> + &adc_data);
>>> + if (ret)
>>> + goto unlock_fail;
>>> +
>>> + *val = MAX9611_VOLTAGE_RAW(adc_data);
>>> +
>>> + mutex_unlock(&dev->lock);
>>> + return IIO_VAL_INT;
>>> + }
>>> +
>>> + case IIO_CHAN_INFO_OFFSET:
>>> + switch (chan->address) {
>>> + case MAX9611_CHAN_VOLTAGE_INPUT:
>>> + *val = CIM_VOLTAGE_OFFSET_RAW;
>>> +
>>> + return IIO_VAL_INT;
>>> + }
>>> +
>>> + case IIO_CHAN_INFO_SCALE:
>>> + switch (chan->address) {
>>> + case MAX9611_CHAN_TEMPERATURE:
>>> + *val = TEMP_SCALE_NUM;
>>> + *val2 = TEMP_SCALE_DIV;
>>> +
>>> + return IIO_VAL_FRACTIONAL;
>>> +
>>> + case MAX9611_CHAN_VOLTAGE_INPUT:
>>> + *val = CIM_VOLTAGE_LSB_mV;
>>> + return IIO_VAL_INT;
>>> + }
>>> +
>>> + case IIO_CHAN_INFO_PROCESSED:
>>> + mutex_lock(&dev->lock);
>>> +
>>> + switch (chan->address) {
>>> + case MAX9611_CHAN_VOLTAGE_SENSE:
>>> + /*
>>> + * processed (mV): (raw - offset) * LSB (nV) / 10^6
>>> + *
>>> + * Even if max9611 can output raw csa voltage readings,
>>> + * use a produced value as scale depends on gain.
>>> + */
>>> + ret = max9611_read_csa_voltage(dev, &adc_data,
>>> + &gain_selector);
>>> + if (ret)
>>> + goto unlock_fail;
>>> +
>>> + csa_gain = max9611_gain_conf[gain_selector];
>>> +
>>> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
>>> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
>>> + csa_gain[CSA_GAIN_LSB_nV];
>>> + *val2 = 1000000;
>>> +
>>> + mutex_unlock(&dev->lock);
>>> + return IIO_VAL_FRACTIONAL;
>>> +
>>> + case MAX9611_CHAN_CURRENT_LOAD:
>>> + /* processed (mA): Vcsa (nV) / Rshunt (uOhm) */
>>> + ret = max9611_read_csa_voltage(dev, &adc_data,
>>> + &gain_selector);
>>> + if (ret)
>>> + goto unlock_fail;
>>> +
>>> + csa_gain = max9611_gain_conf[gain_selector];
>>> +
>>> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
>>> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
>>> + csa_gain[CSA_GAIN_LSB_nV];
>>> + *val2 = dev->shunt_resistor_uohm;
>>> +
>>> + mutex_unlock(&dev->lock);
>>> + return IIO_VAL_FRACTIONAL;
>>> +
>>> + case MAX9611_CHAN_POWER_LOAD:
>>> + /*
>>> + * processed (mW): Vin (mV) * Vcsa (uV) /
>>> + * Rshunt (uOhm)
>>> + */
>>> + ret = max9611_read_single(dev, CONF_IN_VOLT,
>>> + &adc_data);
>>> + if (ret)
>>> + goto unlock_fail;
>>> +
>>> + adc_data -= CIM_VOLTAGE_OFFSET_RAW;
>>> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
>>> + CIM_VOLTAGE_LSB_mV;
>>> +
>>> + ret = max9611_read_csa_voltage(dev, &adc_data,
>>> + &gain_selector);
>>> + if (ret)
>>> + goto unlock_fail;
>>> +
>>> + csa_gain = max9611_gain_conf[gain_selector];
>>> +
>>> + /* divide by 10^3 here to avoid 32bit overflow */
>>> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
>>> + *val *= MAX9611_VOLTAGE_RAW(adc_data) *
>>> + csa_gain[CSA_GAIN_LSB_nV] / 1000;
>>> + *val2 = dev->shunt_resistor_uohm;
>>> +
>>> + mutex_unlock(&dev->lock);
>>> + return IIO_VAL_FRACTIONAL;
>>> + }
>>> + }
>>> +
>>> + ret = -EINVAL;
>>> +
>>> +unlock_fail:
>>> + mutex_unlock(&dev->lock);
>>> + return ret;
>>> +
>>> +}
>>> +
>>> +static ssize_t max9611_shunt_resistor_show(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct max9611_dev *max9611 = iio_priv(dev_to_iio_dev(dev));
>>> +
>>> + return sprintf(buf, "%d\n", max9611->shunt_resistor_uohm);
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(in_shunt_resistor_power, 0444,
>>> + max9611_shunt_resistor_show, NULL, 0);
>>> +static IIO_DEVICE_ATTR(in_shunt_resistor_current, 0444,
>>> + max9611_shunt_resistor_show, NULL, 0);
>>> +
>>> +static struct attribute *max9611_attributes[] = {
>>> + &iio_dev_attr_in_shunt_resistor_power.dev_attr.attr,
>>> + &iio_dev_attr_in_shunt_resistor_current.dev_attr.attr,
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group max9611_attribute_group = {
>>> + .attrs = max9611_attributes,
>>> +};
>>> +
>>> +static const struct iio_info indio_info = {
>>> + .driver_module = THIS_MODULE,
>>> + .read_raw = max9611_read_raw,
>>> + .attrs = &max9611_attribute_group,
>>> +};
>>> +
>>> +static int max9611_init(struct max9611_dev *max9611)
>>> +{
>>> + struct i2c_client *client = max9611->i2c_client;
>>> + u16 regval;
>>> + int ret;
>>> +
>>> + if (!i2c_check_functionality(client->adapter,
>>> + I2C_FUNC_SMBUS_WRITE_BYTE |
>>> + I2C_FUNC_SMBUS_READ_WORD_DATA)) {
>>> + dev_err(max9611->dev,
>>> + "No smbus support in I2c adapter: aborting probe.\n");
>> This isn't necessarily an accurate message. I2c adapter might support
>> smbus_read_byte only rather than word reads for example.
>>
>> Maybe make it more explict as to what we need?
>
> Chopped away some details to make it fit in 80 columns.. I'll make it
> more verbose...
>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* Configure MUX to read temperature */
>>> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
>>> + MAX9611_REG_CTRL1, MAX9611_MUX_TEMP);
>>> + if (ret) {
>>> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
>>> + MAX9611_REG_CTRL1, MAX9611_MUX_TEMP);
>>> + return ret;
>>> + }
>>> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
>>> + MAX9611_REG_CTRL2, 0);
>>> + if (ret) {
>>> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
>>> + MAX9611_REG_CTRL2, 0);
>>> + return ret;
>>> + }
>>> + usleep_range(1000, 2000);
>>> +
>>> + /* Make sure die temperature is in range to test communications. */
>>> + ret = i2c_smbus_read_word_swapped(max9611->i2c_client,
>>> + MAX9611_REG_TEMP_DATA);
>>> + if (ret < 0) {
>>> + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
>>> + MAX9611_REG_TEMP_DATA);
>>> + return ret;
>>> + }
>>> + regval = ret & ~TEMP_MASK;
>>> +
>>> + if ((regval > TEMP_MAX_RAW_POS &&
>>> + regval < TEMP_MIN_RAW_NEG) ||
>>> + regval > TEMP_MAX_RAW_NEG) {
>>> + dev_err(max9611->dev,
>>> + "Invalid value received from ADC 0x%4x: aborting\n",
>>> + regval);
>>> + return -EIO;
>>> + }
>>> +
>>> + /* Mux shall be zeroed back before applying other configurations */
>>> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
>>> + MAX9611_REG_CTRL1, 0);
>>> + if (ret) {
>>> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
>>> + MAX9611_REG_CTRL1, 0);
>>> + return ret;
>>> + }
>>> + usleep_range(1000, 2000);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int max9611_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + const char * const shunt_res_prop = "shunt-resistor-uohm";
>>> + struct device_node *of_node = client->dev.of_node;
>>> + struct max9611_dev *max9611;
>>> + struct iio_dev *indio_dev;
>>> + unsigned int of_shunt;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*max9611));
>>> + if (IS_ERR(indio_dev))
>>> + return PTR_ERR(indio_dev);
>>> +
>>> + i2c_set_clientdata(client, indio_dev);
>>> +
>>> + max9611 = iio_priv(indio_dev);
>>> + max9611->dev = &client->dev;
>>> + max9611->i2c_client = client;
>>> + mutex_init(&max9611->lock);
>>> +
>>> + ret = of_property_read_u32(of_node, shunt_res_prop, &of_shunt);
>>> + if (ret) {
>>> + dev_err(&client->dev,
>>> + "Missing %s property for %s node\n",
>>> + shunt_res_prop, of_node->full_name);
>>> + return ret;
>>> + }
>>> + max9611->shunt_resistor_uohm = of_shunt;
>>> +
>>> + ret = max9611_init(max9611);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + indio_dev->dev.parent = &client->dev;
>>> + indio_dev->dev.of_node = client->dev.of_node;
>>> + indio_dev->name = client->dev.of_node->name;
>> What's this going to give for the name? Name in IIO is supposed to
>> be an indication of the part rather than anything more explicit.
>> That's not easily obtained from device tree directly...
>>
>
> I used the one coming from device tree as otherwise device entries
> have the same name, and I wanted to have it to inclued the unit
> address (eg. adc@7c and not just adc)
> But from you comment I guess it's fine just adc, so I'll change this
> back to v1).
Should be the part number - so max9611 or similar..
You can query the device node details directly if you need to identify
which is which.
>
> Thanks
> j
>
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &indio_info;
>>> + indio_dev->channels = max9611_channels;
>>> + indio_dev->num_channels = ARRAY_SIZE(max9611_channels);
>>> +
>>> + return devm_iio_device_register(&client->dev, indio_dev);
>>> +}
>>> +
>>> +static const struct of_device_id max9611_of_table[] = {
>>> + {.compatible = "maxim,max9611"},
>>> + {.compatible = "maxim,max9612"},
>>> + { },
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(of, max9611_of_table);
>>> +
>>> +static struct i2c_driver max9611_driver = {
>>> + .driver = {
>>> + .name = DRIVER_NAME,
>>> + .owner = THIS_MODULE,
>>> + .of_match_table = max9611_of_table,
>>> + },
>>> + .probe = max9611_probe,
>>> +};
>>> +module_i2c_driver(max9611_driver);
>>> +
>>> +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
>>> +MODULE_DESCRIPTION("Maxim max9611/12 current sense amplifier with 12bit ADC");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
^ permalink raw reply
* Re: [PATCH v3 3/4] iio: adc: Add Maxim max9611 ADC driver
From: jacopo @ 2017-03-25 17:21 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jacopo Mondi, geert-Td1EMuHUCqxL1ZNQvxDV9g,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <a911f5d1-e019-772f-0889-849cf42e6827-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Hi Jonathan,
thanks for review
On Sat, Mar 25, 2017 at 03:45:05PM +0000, Jonathan Cameron wrote:
> On 24/03/17 15:28, Jacopo Mondi wrote:
> > Add iio driver for Maxim max9611 and max9612 current-sense amplifiers
> > with 12-bits ADC interface.
> >
> > Datasheet publicly available at:
> > https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> A few more little things inline. Coming together nicely.
>
> The channel set here is just odd enough that it might aid review to have
> a quick listing of the resulting sysfs entries. One or two authors do
> this an it is always useful for a quick sanity check.
>
> Perhaps even a set of typical values. Put this below the --- as we don't
> need it in the git history, only to assist lazy reviewers like me ;)
>
I included the output of iio_info in the cover letter, I can add some
more info here for sure!
> Thanks,
>
> Jonathan
> > ---
> > drivers/iio/adc/Kconfig | 10 +
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/max9611.c | 590 ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 601 insertions(+)
> > create mode 100644 drivers/iio/adc/max9611.c
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index dedae7a..82f2e7b8 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -354,6 +354,16 @@ config MAX1363
> > To compile this driver as a module, choose M here: the module will be
> > called max1363.
> >
> > +config MAX9611
> > + tristate "Maxim max9611/max9612 ADC driver"
> > + depends on I2C
> > + help
> > + Say yes here to build support for Maxim max9611/max9612 current sense
> > + amplifier with 12-bits ADC interface.
> > +
> > + To compile this driver as a module, choose M here: the module will be
> > + called max9611.
> > +
> > config MCP320X
> > tristate "Microchip Technology MCP3x01/02/04/08"
> > depends on SPI
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index d001262..149f979 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -34,6 +34,7 @@ obj-$(CONFIG_LTC2485) += ltc2485.o
> > obj-$(CONFIG_MAX1027) += max1027.o
> > obj-$(CONFIG_MAX11100) += max11100.o
> > obj-$(CONFIG_MAX1363) += max1363.o
> > +obj-$(CONFIG_MAX9611) += max9611.o
> > obj-$(CONFIG_MCP320X) += mcp320x.o
> > obj-$(CONFIG_MCP3422) += mcp3422.o
> > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
> > diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c
> > new file mode 100644
> > index 0000000..61566ec
> > --- /dev/null
> > +++ b/drivers/iio/adc/max9611.c
> > @@ -0,0 +1,590 @@
> > +/*
> > + * iio/adc/max9611.c
> > + *
> > + * Maxim max9611/max9612 high side current sense amplifier with
> > + * 12-bit ADC interface.
> > + *
> > + * Copyright (C) 2017 Jacopo Mondi
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +/*
> > + * This driver supports input common-mode voltage, current-sense
> > + * amplifier with programmable gains and die temperature reading from
> > + * Maxim max9611/max9612.
> > + *
> > + * Op-amp, analog comparator, and watchdog functionalities are not
> > + * supported by this driver.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/module.h>
> > +
> > +#define DRIVER_NAME "max9611"
> > +
> > +/* max9611 register addresses */
> > +#define MAX9611_REG_CSA_DATA 0x00
> > +#define MAX9611_REG_RS_DATA 0x02
> > +#define MAX9611_REG_TEMP_DATA 0x08
> > +#define MAX9611_REG_CTRL1 0x0a
> > +#define MAX9611_REG_CTRL2 0x0b
> > +
> > +/* max9611 REG1 mux configuration options */
> > +#define MAX9611_MUX_MASK 0x07
> > +#define MAX9611_MUX_SENSE_1x 0x00
> > +#define MAX9611_MUX_SENSE_4x 0x01
> > +#define MAX9611_MUX_SENSE_8x 0x02
> > +#define MAX9611_INPUT_VOLT 0x03
> > +#define MAX9611_MUX_TEMP 0x06
> > +
> > +/* max9611 voltage (both csa and input) helper macros */
> > +#define MAX9611_VOLTAGE_SHIFT 0x04
> > +#define MAX9611_VOLTAGE_RAW(_r) ((_r) >> MAX9611_VOLTAGE_SHIFT)
> > +
> > +/*
> > + * max9611 current sense amplifier voltage output:
> > + * LSB and offset values depends on selected gain (1x, 4x, 8x)
> > + *
> > + * GAIN LSB (nV) OFFSET (LSB steps)
> > + * 1x 107500 1
> > + * 4x 26880 1
> > + * 8x 13440 3
> > + *
> > + * The complete formula to calculate current sense voltage is:
> > + * (((adc_read >> 4) - offset) / ((1 / LSB) * 10^-3)
> > + */
> > +#define CSA_VOLT_1x_LSB_nV 107500
> > +#define CSA_VOLT_4x_LSB_nV 26880
> > +#define CSA_VOLT_8x_LSB_nV 13440
> > +
> > +#define CSA_VOLT_1x_OFFS_RAW 1
> > +#define CSA_VOLT_4x_OFFS_RAW 1
> > +#define CSA_VOLT_8x_OFFS_RAW 3
> > +
> > +/*
> > + * max9611 common input mode (CIM): LSB is 14mV, with 14mV offset at 25 C
> > + *
> > + * The complete formula to calculate input common voltage is:
> > + * (((adc_read >> 4) * 1000) - offset) / (1 / 14 * 1000)
> > + */
> > +#define CIM_VOLTAGE_LSB_mV 14
> > +#define CIM_VOLTAGE_OFFSET_RAW 1
> > +
> > +/*
> > + * max9611 temperature reading: LSB is 0.48 degrees Celsius
> > + *
> > + * The complete formula to calculate temperature is:
> > + * ((adc_read >> 7) * 1000) / (1 / 0.48 * 1000)
> > + */
> I'd prefer these defines to be prefixed with MAX9611_
> Easiest to just do the lot. Some of these are 'standard' enough
> the might well clash with something that turns up in an included header
> at somepoint in the future.
>
> > +#define TEMP_SHIFT 0x07
> > +#define TEMP_MAX_RAW_POS 0x7f80
> > +#define TEMP_MAX_RAW_NEG 0xff80
> > +#define TEMP_MIN_RAW_NEG 0xd980
> > +#define TEMP_MASK ((1 << TEMP_SHIFT) - 1)
> > +#define TEMP_RAW(_r) ((_r) >> TEMP_SHIFT)
> > +#define TEMP_LSB_mC 480
> > +#define TEMP_SCALE_NUM 1000
> > +#define TEMP_SCALE_DIV 2083
> > +
> > +struct max9611_dev {
> > + struct device *dev;
> > + struct i2c_client *i2c_client;
> > + struct mutex lock;
> > + unsigned int shunt_resistor_uohm;
> > +};
> > +
> > +enum max9611_conf_ids {
> > + CONF_SENSE_1x,
> > + CONF_SENSE_4x,
> > + CONF_SENSE_8x,
> > + CONF_IN_VOLT,
> > + CONF_TEMP,
> > +};
> > +
> > +/**
> > + * max9611_mux_conf - associate ADC mux configuration with register address
> > + * where data shall be read from
> > + */
> > +static unsigned int max9611_mux_conf[][2] = {
> > + /* CONF_SENSE_1x */
> > + { MAX9611_MUX_SENSE_1x, MAX9611_REG_CSA_DATA },
> > + /* CONF_SENSE_4x */
> > + { MAX9611_MUX_SENSE_4x, MAX9611_REG_CSA_DATA },
> > + /* CONF_SENSE_8x */
> > + { MAX9611_MUX_SENSE_8x, MAX9611_REG_CSA_DATA },
> > + /* CONF_IN_VOLT */
> > + { MAX9611_INPUT_VOLT, MAX9611_REG_RS_DATA },
> > + /* CONF_TEMP */
> > + { MAX9611_MUX_TEMP, MAX9611_REG_TEMP_DATA },
> > +};
> > +
> > +enum max9611_csa_gain {
> > + CSA_GAIN_1x,
> > + CSA_GAIN_4x,
> > + CSA_GAIN_8x,
> > +};
> > +
> > +enum max9611_csa_gain_params {
> > + CSA_GAIN_LSB_nV,
> > + CSA_GAIN_OFFS_RAW,
> > +};
> > +
> > +/**
> > + * max9611_csa_gain_conf - associate gain multiplier with LSB and
> > + * offset values.
> > + *
> > + * Group together parameters associated with configurable gain
> > + * on current sense amplifier path to ADC interface.
> > + * Current sense read routine adjusts gain until it gets a meaningful
> > + * value; use this structure to retrieve the correct LSB and offset values.
> > + */
> > +static unsigned int max9611_gain_conf[][2] = {
> > + { /* [0] CSA_GAIN_1x */
> > + CSA_VOLT_1x_LSB_nV,
> > + CSA_VOLT_1x_OFFS_RAW,
> > + },
> > + { /* [1] CSA_GAIN_4x */
> > + CSA_VOLT_4x_LSB_nV,
> > + CSA_VOLT_4x_OFFS_RAW,
> > + },
> > + { /* [2] CSA_GAIN_8x */
> > + CSA_VOLT_8x_LSB_nV,
> > + CSA_VOLT_8x_OFFS_RAW,
> > + },
> > +};
> > +
> > +enum max9611_chan_addrs {
> > + MAX9611_CHAN_VOLTAGE_INPUT,
> > + MAX9611_CHAN_VOLTAGE_SENSE,
> > + MAX9611_CHAN_TEMPERATURE,
> > + MAX9611_CHAN_CURRENT_LOAD,
> > + MAX9611_CHAN_POWER_LOAD,
> > +};
> > +
> > +static struct iio_chan_spec max9611_channels[] = {
> > + {
> > + .type = IIO_TEMP,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_SCALE),
> > + .address = MAX9611_CHAN_TEMPERATURE,
> > + },
> > + {
> > + .type = IIO_VOLTAGE,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_SCALE) |
> > + BIT(IIO_CHAN_INFO_OFFSET),
> > + .address = MAX9611_CHAN_VOLTAGE_INPUT,
> > + .indexed = 1,
> > + .channel = 1,
> > + },
> > + {
> > + .type = IIO_VOLTAGE,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> > + .address = MAX9611_CHAN_VOLTAGE_SENSE,
> > + .indexed = 1,
> > + .channel = 0,
> Unusual to have the channels in here other than in channel order...
> > + },
> > + {
> > + .type = IIO_CURRENT,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> > + .address = MAX9611_CHAN_CURRENT_LOAD,
> > + },
> > + {
> > + .type = IIO_POWER,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> > + .address = MAX9611_CHAN_POWER_LOAD
> > + },
> > +};
> > +
> > +/**
> > + * max9611_read_single() - read a single vale from ADC interface
> value
> > + *
> > + * Data registers are 16 bit long, spread between two 8 bit registers
> > + * with consecutive addresses.
> > + * Configure ADC mux first, then read register at address "reg_addr".
> > + * The smbus_read_word routine asks for 16 bits and the ADC is kind enough
> > + * to return values from "reg_addr" and "reg_addr + 1" consecutively.
> > + *
> > + * @max9611: max9611 device
> > + * @selector: index for mux and register configuration
> > + * @raw_val: the value returned from ADC
> > + */
> > +static int max9611_read_single(struct max9611_dev *max9611,
> > + enum max9611_conf_ids selector,
> > + u16 *raw_val)
> > +{
> > + int ret;
> > +
> > + u8 mux_conf = max9611_mux_conf[selector][0] & MAX9611_MUX_MASK;
> > + u8 reg_addr = max9611_mux_conf[selector][1];
> > +
> > + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> > + MAX9611_REG_CTRL1, mux_conf);
> > + if (ret) {
> > + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> > + MAX9611_REG_CTRL1, mux_conf);
> > + return ret;
> > + }
> > +
> > + /*
> > + * need a delay here to make register configuration
> > + * stabilize. 1 msec at least, from empirical testing.
> > + */
> > + usleep_range(1000, 2000);
> > +
> > + ret = i2c_smbus_read_word_swapped(max9611->i2c_client, reg_addr);
> > + if (ret < 0) {
> > + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
> > + reg_addr);
> > + return ret;
> > + }
> > + *raw_val = ret;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * max9611_read_csa_voltage() - read current sense amplifier output voltage
> > + *
> > + * Current sense amplifier output voltage is read through a configurable
> > + * 1x, 4x or 8x gain.
> > + * Start with plain 1x gain, and adjust gain control properly until a
> > + * meaningful value is read from ADC output.
> > + *
> > + * @max9611: max9611 device
> > + * @adc_raw: raw value read from ADC output
> > + * @csa_gain: gain configuration option selector
> > + */
> > +static int max9611_read_csa_voltage(struct max9611_dev *max9611,
> > + u16 *adc_raw,
> > + enum max9611_csa_gain *csa_gain)
> > +{
> > + enum max9611_conf_ids gain_selectors[] = {
> > + CONF_SENSE_1x,
> > + CONF_SENSE_4x,
> > + CONF_SENSE_8x
> > + };
> > + unsigned int i;
> > + int ret;
> > +
> > + for (i = 0; i < ARRAY_SIZE(gain_selectors); ++i) {
> > + ret = max9611_read_single(max9611, gain_selectors[i], adc_raw);
> > + if (ret)
> > + return ret;
> > +
> > + if (*adc_raw > 0) {
> > + *csa_gain = gain_selectors[i];
> > + return 0;
> > + }
> > + }
> > +
> > + return -EIO;
> > +}
> > +
> > +static int max9611_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + struct max9611_dev *dev = iio_priv(indio_dev);
> > + enum max9611_csa_gain gain_selector;
> > + unsigned int *csa_gain;
> > + u16 adc_data;
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + mutex_lock(&dev->lock);
> > +
> > + switch (chan->address) {
> > + case MAX9611_CHAN_TEMPERATURE:
> > + ret = max9611_read_single(dev, CONF_TEMP,
> > + &adc_data);
> > + if (ret)
> I'm not personally keen on jumping out of deep indentations like this
> just save on repeating one line. I'd pull the unlock back here and return
> directly as I feel it'll improve readability.
I'll change it, with this and Peter's suggestion on this function in
his review.
> Actually come to think of it, why does the lock need to be held for
> the next line anyway? adc_data is on the stack so doesn't matter if we
> have concurrent readers, once the i2c transaction is finished.
> Just unlock before checking ret.
I naively overvalued 'style' (ret assignment and check in two
consecutive lines) over practicality... I'll change this
> > + goto unlock_fail;
> > +
> > + *val = TEMP_RAW(adc_data);
> > +
> > + mutex_unlock(&dev->lock);
> > + return IIO_VAL_INT;
> > +
> > + case MAX9611_CHAN_VOLTAGE_INPUT:
> > + ret = max9611_read_single(dev, CONF_IN_VOLT,
> > + &adc_data);
> > + if (ret)
> > + goto unlock_fail;
> > +
> > + *val = MAX9611_VOLTAGE_RAW(adc_data);
> > +
> > + mutex_unlock(&dev->lock);
> > + return IIO_VAL_INT;
> > + }
> > +
> > + case IIO_CHAN_INFO_OFFSET:
> > + switch (chan->address) {
> > + case MAX9611_CHAN_VOLTAGE_INPUT:
> > + *val = CIM_VOLTAGE_OFFSET_RAW;
> > +
> > + return IIO_VAL_INT;
> > + }
> > +
> > + case IIO_CHAN_INFO_SCALE:
> > + switch (chan->address) {
> > + case MAX9611_CHAN_TEMPERATURE:
> > + *val = TEMP_SCALE_NUM;
> > + *val2 = TEMP_SCALE_DIV;
> > +
> > + return IIO_VAL_FRACTIONAL;
> > +
> > + case MAX9611_CHAN_VOLTAGE_INPUT:
> > + *val = CIM_VOLTAGE_LSB_mV;
> > + return IIO_VAL_INT;
> > + }
> > +
> > + case IIO_CHAN_INFO_PROCESSED:
> > + mutex_lock(&dev->lock);
> > +
> > + switch (chan->address) {
> > + case MAX9611_CHAN_VOLTAGE_SENSE:
> > + /*
> > + * processed (mV): (raw - offset) * LSB (nV) / 10^6
> > + *
> > + * Even if max9611 can output raw csa voltage readings,
> > + * use a produced value as scale depends on gain.
> > + */
> > + ret = max9611_read_csa_voltage(dev, &adc_data,
> > + &gain_selector);
> > + if (ret)
> > + goto unlock_fail;
> > +
> > + csa_gain = max9611_gain_conf[gain_selector];
> > +
> > + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> > + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> > + csa_gain[CSA_GAIN_LSB_nV];
> > + *val2 = 1000000;
> > +
> > + mutex_unlock(&dev->lock);
> > + return IIO_VAL_FRACTIONAL;
> > +
> > + case MAX9611_CHAN_CURRENT_LOAD:
> > + /* processed (mA): Vcsa (nV) / Rshunt (uOhm) */
> > + ret = max9611_read_csa_voltage(dev, &adc_data,
> > + &gain_selector);
> > + if (ret)
> > + goto unlock_fail;
> > +
> > + csa_gain = max9611_gain_conf[gain_selector];
> > +
> > + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> > + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> > + csa_gain[CSA_GAIN_LSB_nV];
> > + *val2 = dev->shunt_resistor_uohm;
> > +
> > + mutex_unlock(&dev->lock);
> > + return IIO_VAL_FRACTIONAL;
> > +
> > + case MAX9611_CHAN_POWER_LOAD:
> > + /*
> > + * processed (mW): Vin (mV) * Vcsa (uV) /
> > + * Rshunt (uOhm)
> > + */
> > + ret = max9611_read_single(dev, CONF_IN_VOLT,
> > + &adc_data);
> > + if (ret)
> > + goto unlock_fail;
> > +
> > + adc_data -= CIM_VOLTAGE_OFFSET_RAW;
> > + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> > + CIM_VOLTAGE_LSB_mV;
> > +
> > + ret = max9611_read_csa_voltage(dev, &adc_data,
> > + &gain_selector);
> > + if (ret)
> > + goto unlock_fail;
> > +
> > + csa_gain = max9611_gain_conf[gain_selector];
> > +
> > + /* divide by 10^3 here to avoid 32bit overflow */
> > + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> > + *val *= MAX9611_VOLTAGE_RAW(adc_data) *
> > + csa_gain[CSA_GAIN_LSB_nV] / 1000;
> > + *val2 = dev->shunt_resistor_uohm;
> > +
> > + mutex_unlock(&dev->lock);
> > + return IIO_VAL_FRACTIONAL;
> > + }
> > + }
> > +
> > + ret = -EINVAL;
> > +
> > +unlock_fail:
> > + mutex_unlock(&dev->lock);
> > + return ret;
> > +
> > +}
> > +
> > +static ssize_t max9611_shunt_resistor_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct max9611_dev *max9611 = iio_priv(dev_to_iio_dev(dev));
> > +
> > + return sprintf(buf, "%d\n", max9611->shunt_resistor_uohm);
> > +}
> > +
> > +static IIO_DEVICE_ATTR(in_shunt_resistor_power, 0444,
> > + max9611_shunt_resistor_show, NULL, 0);
> > +static IIO_DEVICE_ATTR(in_shunt_resistor_current, 0444,
> > + max9611_shunt_resistor_show, NULL, 0);
> > +
> > +static struct attribute *max9611_attributes[] = {
> > + &iio_dev_attr_in_shunt_resistor_power.dev_attr.attr,
> > + &iio_dev_attr_in_shunt_resistor_current.dev_attr.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group max9611_attribute_group = {
> > + .attrs = max9611_attributes,
> > +};
> > +
> > +static const struct iio_info indio_info = {
> > + .driver_module = THIS_MODULE,
> > + .read_raw = max9611_read_raw,
> > + .attrs = &max9611_attribute_group,
> > +};
> > +
> > +static int max9611_init(struct max9611_dev *max9611)
> > +{
> > + struct i2c_client *client = max9611->i2c_client;
> > + u16 regval;
> > + int ret;
> > +
> > + if (!i2c_check_functionality(client->adapter,
> > + I2C_FUNC_SMBUS_WRITE_BYTE |
> > + I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> > + dev_err(max9611->dev,
> > + "No smbus support in I2c adapter: aborting probe.\n");
> This isn't necessarily an accurate message. I2c adapter might support
> smbus_read_byte only rather than word reads for example.
>
> Maybe make it more explict as to what we need?
Chopped away some details to make it fit in 80 columns.. I'll make it
more verbose...
> > + return -EINVAL;
> > + }
> > +
> > + /* Configure MUX to read temperature */
> > + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> > + MAX9611_REG_CTRL1, MAX9611_MUX_TEMP);
> > + if (ret) {
> > + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> > + MAX9611_REG_CTRL1, MAX9611_MUX_TEMP);
> > + return ret;
> > + }
> > + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> > + MAX9611_REG_CTRL2, 0);
> > + if (ret) {
> > + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> > + MAX9611_REG_CTRL2, 0);
> > + return ret;
> > + }
> > + usleep_range(1000, 2000);
> > +
> > + /* Make sure die temperature is in range to test communications. */
> > + ret = i2c_smbus_read_word_swapped(max9611->i2c_client,
> > + MAX9611_REG_TEMP_DATA);
> > + if (ret < 0) {
> > + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
> > + MAX9611_REG_TEMP_DATA);
> > + return ret;
> > + }
> > + regval = ret & ~TEMP_MASK;
> > +
> > + if ((regval > TEMP_MAX_RAW_POS &&
> > + regval < TEMP_MIN_RAW_NEG) ||
> > + regval > TEMP_MAX_RAW_NEG) {
> > + dev_err(max9611->dev,
> > + "Invalid value received from ADC 0x%4x: aborting\n",
> > + regval);
> > + return -EIO;
> > + }
> > +
> > + /* Mux shall be zeroed back before applying other configurations */
> > + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> > + MAX9611_REG_CTRL1, 0);
> > + if (ret) {
> > + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> > + MAX9611_REG_CTRL1, 0);
> > + return ret;
> > + }
> > + usleep_range(1000, 2000);
> > +
> > + return 0;
> > +}
> > +
> > +static int max9611_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + const char * const shunt_res_prop = "shunt-resistor-uohm";
> > + struct device_node *of_node = client->dev.of_node;
> > + struct max9611_dev *max9611;
> > + struct iio_dev *indio_dev;
> > + unsigned int of_shunt;
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*max9611));
> > + if (IS_ERR(indio_dev))
> > + return PTR_ERR(indio_dev);
> > +
> > + i2c_set_clientdata(client, indio_dev);
> > +
> > + max9611 = iio_priv(indio_dev);
> > + max9611->dev = &client->dev;
> > + max9611->i2c_client = client;
> > + mutex_init(&max9611->lock);
> > +
> > + ret = of_property_read_u32(of_node, shunt_res_prop, &of_shunt);
> > + if (ret) {
> > + dev_err(&client->dev,
> > + "Missing %s property for %s node\n",
> > + shunt_res_prop, of_node->full_name);
> > + return ret;
> > + }
> > + max9611->shunt_resistor_uohm = of_shunt;
> > +
> > + ret = max9611_init(max9611);
> > + if (ret)
> > + return ret;
> > +
> > + indio_dev->dev.parent = &client->dev;
> > + indio_dev->dev.of_node = client->dev.of_node;
> > + indio_dev->name = client->dev.of_node->name;
> What's this going to give for the name? Name in IIO is supposed to
> be an indication of the part rather than anything more explicit.
> That's not easily obtained from device tree directly...
>
I used the one coming from device tree as otherwise device entries
have the same name, and I wanted to have it to inclued the unit
address (eg. adc@7c and not just adc)
But from you comment I guess it's fine just adc, so I'll change this
back to v1).
Thanks
j
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->info = &indio_info;
> > + indio_dev->channels = max9611_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(max9611_channels);
> > +
> > + return devm_iio_device_register(&client->dev, indio_dev);
> > +}
> > +
> > +static const struct of_device_id max9611_of_table[] = {
> > + {.compatible = "maxim,max9611"},
> > + {.compatible = "maxim,max9612"},
> > + { },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, max9611_of_table);
> > +
> > +static struct i2c_driver max9611_driver = {
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .owner = THIS_MODULE,
> > + .of_match_table = max9611_of_table,
> > + },
> > + .probe = max9611_probe,
> > +};
> > +module_i2c_driver(max9611_driver);
> > +
> > +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
> > +MODULE_DESCRIPTION("Maxim max9611/12 current sense amplifier with 12bit ADC");
> > +MODULE_LICENSE("GPL v2");
> >
>
^ permalink raw reply
* Re: [PATCH v3 2/4] iio: Documentation: Add max9611 sysfs documentation
From: Jonathan Cameron @ 2017-03-25 16:32 UTC (permalink / raw)
To: Jacopo Mondi, geert-Td1EMuHUCqxL1ZNQvxDV9g,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490369323-13866-3-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 24/03/17 15:28, Jacopo Mondi wrote:
> Add documentation for max9611 driver.
> Document attributes describing value of shunt resistor installed between
> RS+ and RS- voltage sense inputs.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> ---
> Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
> new file mode 100644
> index 0000000..9c60824
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
> @@ -0,0 +1,16 @@
> +What: /sys/bus/iio/devices/iio:deviceX/in_shunt_resistor_power
> +Date: March 2017
> +KernelVersion: 4.12
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description: The value of the shunt resistor used to compute power drain on
> + common input voltage pin (RS+). In micro Ohms.
Unfortunately this has ended up with a name that made me initially think it was
the power rating of the resistor in question (why we'd want to know that I've no
idea). We might need to make this even more explicit to avoid idiots like me ;)
It's the shunt resistor for the power measurement so I think it should probably be...
in_power_shunt_resistance
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_shunt_resistor_current
> +Date: March 2017
> +KernelVersion: 4.12
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description: The value of the shunt resistor used to compute current flowing
> + between RS+ and RS- voltage sense inputs. In micro Ohms.
> + This attributes describe a single physical component, exposed
> + as two distinct attributes as it is used to calculate two
> + different values.
>
Same as above...
Jonathan
^ permalink raw reply
* [PATCH 2/2] iio: adc: meson-saradc: add Meson8b SoC compatibility
From: Martin Blumenstingl @ 2017-03-25 16:29 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Martin Blumenstingl
In-Reply-To: <20170325162938.28659-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Meson GX SoCs however use some magic bits to prevent simultaneous (=
conflicting, because only consumer should use the FIFO buffer with the
ADC results) usage by the Linux kernel and the bootloader (the BL30
bootloader uses the SAR ADC to read the CPU temperature).
This patch changes guards all BL30 functionality so it is skipped on
SoCs which don't have it. Since the hardware itself doesn't know whether
BL30 is available the internal meson_sar_adc_data is extended so this
information can be provided per of_device_id.data inside the driver.
Additionally the clocks "adc_clk" and "adc_sel" are not provided by the
clock-controller itself. "adc_sel" is not available at all. "adc_clk"
is provided by the SAR ADC IP block itself on Meson8b (and earlier).
This is already supported by the meson_saradc driver.
Finally a new of_device_id for the Meson8b SoC is added so it can be
wired up in the corresponding DT.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/iio/adc/meson_saradc.c | 70 +++++++++++++++++++++++++++---------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index dd4190b50df6..f78fe66e0ec8 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -220,6 +220,7 @@ enum meson_sar_adc_chan7_mux_sel {
};
struct meson_sar_adc_data {
+ char has_bl30_integration;
unsigned int resolution;
const char *name;
};
@@ -437,19 +438,24 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
mutex_lock(&indio_dev->mlock);
- /* prevent BL30 from using the SAR ADC while we are using it */
- regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
- MESON_SAR_ADC_DELAY_KERNEL_BUSY,
- MESON_SAR_ADC_DELAY_KERNEL_BUSY);
-
- /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
- do {
- udelay(1);
- regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
- } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
-
- if (timeout < 0)
- return -ETIMEDOUT;
+ if (priv->data->has_bl30_integration) {
+ /* prevent BL30 from using the SAR ADC while we are using it */
+ regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
+ MESON_SAR_ADC_DELAY_KERNEL_BUSY,
+ MESON_SAR_ADC_DELAY_KERNEL_BUSY);
+
+ /*
+ * wait until BL30 releases it's lock (so we can use the SAR
+ * ADC)
+ */
+ do {
+ udelay(1);
+ regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
+ } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
+
+ if (timeout < 0)
+ return -ETIMEDOUT;
+ }
return 0;
}
@@ -458,9 +464,10 @@ static void meson_sar_adc_unlock(struct iio_dev *indio_dev)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
- /* allow BL30 to use the SAR ADC again */
- regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
- MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
+ if (priv->data->has_bl30_integration)
+ /* allow BL30 to use the SAR ADC again */
+ regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
+ MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
mutex_unlock(&indio_dev->mlock);
}
@@ -614,14 +621,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
*/
meson_sar_adc_set_chan7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
- /*
- * leave sampling delay and the input clocks as configured by BL30 to
- * make sure BL30 gets the values it expects when reading the
- * temperature sensor.
- */
- regmap_read(priv->regmap, MESON_SAR_ADC_REG3, ®val);
- if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
- return 0;
+ if (priv->data->has_bl30_integration) {
+ /*
+ * leave sampling delay and the input clocks as configured by
+ * BL30 to make sure BL30 gets the values it expects when
+ * reading the temperature sensor.
+ */
+ regmap_read(priv->regmap, MESON_SAR_ADC_REG3, ®val);
+ if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
+ return 0;
+ }
meson_sar_adc_stop_sample_engine(indio_dev);
@@ -834,23 +843,36 @@ static const struct iio_info meson_sar_adc_iio_info = {
.driver_module = THIS_MODULE,
};
+struct meson_sar_adc_data meson_sar_adc_meson8b_data = {
+ .has_bl30_integration = 0,
+ .resolution = 10,
+ .name = "meson-meson8b-saradc",
+};
+
struct meson_sar_adc_data meson_sar_adc_gxbb_data = {
+ .has_bl30_integration = 1,
.resolution = 10,
.name = "meson-gxbb-saradc",
};
struct meson_sar_adc_data meson_sar_adc_gxl_data = {
+ .has_bl30_integration = 1,
.resolution = 12,
.name = "meson-gxl-saradc",
};
struct meson_sar_adc_data meson_sar_adc_gxm_data = {
+ .has_bl30_integration = 1,
.resolution = 12,
.name = "meson-gxm-saradc",
};
static const struct of_device_id meson_sar_adc_of_match[] = {
{
+ .compatible = "amlogic,meson8b-saradc",
+ .data = &meson_sar_adc_meson8b_data,
+ },
+ {
.compatible = "amlogic,meson-gxbb-saradc",
.data = &meson_sar_adc_gxbb_data,
}, {
--
2.12.1
^ permalink raw reply related
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