* [PATCH v4] ARM64: dts: meson-gx: Add reserved memory zone and usable memory range
From: Kevin Hilman @ 2017-01-13 20:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484129414-23325-1-git-send-email-narmstrong@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
> this patch adds this reserved zone and redefines the usable memory range.
>
> The memory node is also moved from the dtsi files into the proper dts files
> to handle variants memory sizes.
>
> This patch also fixes the memory sizes for the following platforms :
> - gxl-s905x-p212 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
> - gxm-s912-q201 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
> - gxl-s905d-p231 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
> - gxl-nexbox-a95x : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Queued for v4.10-rc.
Kevin
^ permalink raw reply
* [PATCH 4/4] ARM64: dts: meson: meson-gx: add the SAR ADC
From: Heiner Kallweit @ 2017-01-13 20:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFBinCC_AYS0ogzdpGVRoeS2ETjcD6fdmzq3kmX_u=9heBu6oA@mail.gmail.com>
Am 13.01.2017 um 20:50 schrieb Martin Blumenstingl:
> Hi Heiner,
>
> On Fri, Jan 13, 2017 at 8:32 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Sorry, I'm not subscribed to the two mailing lists, therefore my reply
>> is outside the thread.
>>
>> I'm currently experimenting with an own rudimentary driver for SAR ADC
>> on a Odroid C2 (S905GXBB). So I have some remarks based on my experience.
> I hope that we haven't been duplicating too much work!
>
No, my driver doesn't include all the clock handling and relies on the
boot loader / firmware to do this. Also I don't support averaging mode.
Your driver is much more comprehensive and I would go with it.
I have some features like interrupt mode and calibration which are not
yet supported in your driver but they can be easily migrated and added
later.
Rgds, Heiner
>> Rgds, Heiner
>>
>>> Add the SAR ADC to meson-gxbb.dtsi and meson-gxl.dtsi. GXBB provides a
>>> 10-bit ADC while GXL (and GXM, which uses the same ADC as GXL) provides
>>> a 12-bit ADC.
>>> Some boards use resistor ladder buttons connected through one of the ADC
>>> channels. On newer devices (GXL and GXM) some boards use pull-ups/downs
>>> to change the resistance (and thus the ADC value) on of the ADC channels
>>> to indicate the board revision.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
>>> ---
>>> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 8 ++++++++
>>> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 10 ++++++++++
>>> arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 10 ++++++++++
>>> 3 files changed, 28 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>>> index cddad8c795ec..ed3bf29eb76a 100644
>>> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>>> @@ -237,6 +237,14 @@
>>> status = "disabled";
>>> };
>>>
>>> + saradc: adc at 8680 {
>>> + compatible = "amlogic,meson-saradc";
>>> + #io-channel-cells = <1>;
>>> + status = "disabled";
>>> + reg = <0x0 0x8680 0x0 0x34>;
>>> + interrupts = <GIC_SPI 9 IRQ_TYPE_EDGE_RISING>;
>> IRQ 9 dosn't work for me, where does this number come from?
>> With IRQ 73 interrupt mode is working fine here.
>> Of course I can't speak for all other Meson variants.
> I think I took the IRQ from the vendor's mesongxbb.dtsi, depending on
> whether I'll add IRQ support or not I'll remove or fix this. Thanks
> for spotting this!
>
>
> Regards,
> Martin
>
^ permalink raw reply
* [PATCH v5 05/12] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Rob Herring @ 2017-01-13 20:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <89c95d0084da6ad7132caa54ca66eb619d8a5090.1484154449.git-series.gregory.clement@free-electrons.com>
On Wed, Jan 11, 2017 at 06:19:08PM +0100, Gregory CLEMENT wrote:
> From: Hu Ziji <huziji@marvell.com>
>
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 197 +++++++-
> 1 file changed, 197 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> new file mode 100644
> index 000000000000..a3876d2cc616
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> @@ -0,0 +1,197 @@
> +Marvell Xenon SDHCI Controller device tree bindings
> +This file documents differences between the core mmc properties
> +described by mmc.txt and the properties used by the Xenon implementation.
> +
> +Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
> +Each SDHC is independent and owns independent resources, such as register sets,
> +clock and PHY.
> +Each SDHC should have an independent device tree node.
> +
> +Required Properties:
> +- compatible: should be one of the following
> + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SoC.
> + Must provide a second register area and marvell,pad-type.
> + - "marvell,armada-8k-sdhci": For controllers on Armada 7K/8K SoC
> +
> +- clocks:
> + Array of clocks required for SDHC.
> + Require at least input clock for Xenon IP core.
> +
> +- clock-names:
> + Array of names corresponding to clocks property.
> + The input clock for Xenon IP core should be named as "core".
> +
> +- reg:
> + * For "marvell,armada-3700-sdhci", two register areas.
> + The first one for Xenon IP register. The second one for the Armada 3700 SoC
> + PHY PAD Voltage Control register.
> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
> + in below.
> + Please also check property marvell,pad-type in below.
> +
> + * For other compatible strings, one register area for Xenon IP.
> +
> +Optional Properties:
> +- mmccard:
> + mmccard child node must be provided when current SDHC is for eMMC.
> + Xenon SDHC often can support both SD and eMMC. This child node indicates that
> + current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and
> + operations can be enabled prior to eMMC init sequence.
> + Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt.
> + This child node should not be set if current Xenon SDHC is for SD/SDIO.
> +
> +- bus-width:
> + When 8-bit data bus width is in use for eMMC, this property should be
> + explicitly provided and set as 8.
> + It is optional when data bus width is 4-bit or 1-bit.
> +
> +- mmc-ddr-1_8v:
> + Select this property when eMMC HS DDR is supported on SDHC side.
> +
> +- mmc-hs400-1_8v:
> + Select this property when eMMC HS400 is supported on SDHC side.
> +
> +- no-1-8-v:
> + Select this property when 1.8V signaling voltage supply is unavailable.
> + When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be
> + cleared.
> +
> +- marvell,xenon-sdhc-id:
> + Indicate the corresponding bit index of current SDHC in
> + SDHC System Operation Control Register Bit[7:0].
> + Set/clear the corresponding bit to enable/disable current SDHC.
> + If Xenon IP contains only one SDHC, this property is optional.
> +
> +- marvell,xenon-phy-type:
> + Xenon support mutilple types of PHYs.
> + To select eMMC 5.1 PHY, set:
> + marvell,xenon-phy-type = "emmc 5.1 phy"
> + eMMC 5.1 PHY is the default choice if this property is not provided.
> + To select eMMC 5.0 PHY, set:
> + marvell,xenon-phy-type = "emmc 5.0 phy"
> +
> + All those types of PHYs can support eMMC, SD and SDIO.
> + Please note that this property only presents the type of PHY.
> + It doesn't stand for the entire SDHC type or property.
> + For example, "emmc 5.1 phy" doesn't mean that this Xenon SDHC only supports
> + eMMC 5.1.
> +
> +- marvell,xenon-phy-znr:
> + Set PHY ZNR value.
> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> + Valid range = [0:0x1F].
> + ZNR is set as 0xF by default if this property is not provided.
> +
> +- marvell,xenon-phy-zpr:
> + Set PHY ZPR value.
> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> + Valid range = [0:0x1F].
> + ZPR is set as 0xF by default if this property is not provided.
> +
> +- marvell,xenon-phy-nr-success-tun:
> + Set the number of required consecutive successful sampling points used to
> + identify a valid sampling window, in tuning process.
> + Valid range = [1:7].
> + Set as 0x4 by default if this property is not provided.
> +
> +- marvell,xenon-phy-tun-step-divider:
> + Set the divider for calculating TUN_STEP.
> + Set as 64 by default if this property is not provided.
> +
> +- marvell,xenon-phy-slow-mode:
> + If this property is selected, transfers will bypass PHY.
> + Only available when bus frequency lower than 55MHz in SDR mde.
> + Disabled by default. Please only try this property if timing issues always
> + occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25, SD SDR50 mode.
> +
> +- marvell,xenon-tun-count:
> + Xenon SDHC SoC usually doesn't provide re-tuning counter in
> + Capabilities Register 3 Bit[11:8].
> + This property provides the re-tuning counter.
> + If this property is not set, default re-tuning counter will
> + be set as 0x9 in driver.
> +
> +- marvell,pad-type:
> + Type of Armada 3700 SoC PHY PAD Voltage Controller register.
> + Only valid when "marvell,armada-3700-sdhci" is selected.
> + Two types: "sd" and "fixed-1-8v".
> + If "sd" is slected, SoC PHY PAD is set as 3.3V at the beginning and is
> + switched to 1.8V when SD in UHS-I.
> + If "fixed-1-8v" is slected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
> + in below.
> +
> +Example:
> +- For eMMC:
> +
> + sdhci at aa0000 {
> + compatible = "marvell,armada-8k-sdhci";
> + reg = <0xaa0000 0x1000>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmc_clk>;
> + clock-names = "core";
> + bus-width = <8>;
> + mmc-ddr-1_8v;
> + mmc-hs400-1_8v;
> + marvell,xenon-sdhc-id = <0>;
Should be dropped? With that,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Heiner Kallweit @ 2017-01-13 20:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFBinCAD_EjYq9i1=vVNAh046q0jSSG38kc+UL_=6aeZRqsRHw@mail.gmail.com>
Am 13.01.2017 um 20:57 schrieb Martin Blumenstingl:
> On Fri, Jan 13, 2017 at 8:35 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>> This adds support for the SAR (Successive Approximation Register) ADC
>>> on the Amlogic Meson SoCs.
>>>
>>> The code is based on the public S805 (Meson8b) and S905 (GXBB)
>>> datasheets, as well as by reading (various versions of) the vendor
>>> driver and by inspecting the registers on the vendor kernels of my
>>> testing-hardware.
>>>
>>> Currently the GXBB, GXL and GXM SoCs are supported. GXBB hardware has
>>> 10-bit ADC resolution, while GXL and GXM have 12-bit ADC resolution.
>>> The code was written to support older SoCs (Meson8 and Meson8b) as well,
>>> but due to lack of actual testing-hardware no of_device_id was added for
>>> these.
>>>
>>> Two "features" from the vendor driver are currently missing:
>>> - the vendor driver uses channel #7 for calibration (this improves the
>>> accuracy of the results - in my tests the results were less than 3%
>>> off without calibration compared to the vendor driver). Adding support
>>> for this should be easy, but is not required for most applications.
>>> - channel #6 is connected to the SoCs internal temperature sensor.
>>> Adding support for this is probably not so easy since (based on the
>>> u-boot sources) most SoC versions are using different registers and
>>> algorithms for the conversion from "ADC value" to temperature.
>>>
>>> Supported by the hardware but currently not supported by the driver:
>>> - reading multiple channels at the same time (the hardware has a FIFO
>>> buffer which stores multiple results)
>>> - continuous sampling (this would require a way to enable this
>>> individually because otherwise the ADC would be drawing power
>>> constantly)
>>> - interrupt support (similar to the vendor driver this new driver is
>>> polling the results. It is unclear if the IRQ-mode is supported on
>>> older (Meson6 or Meson8) hardware as well or if there are any errata)
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
>>> ---
>>> drivers/iio/adc/Kconfig | 12 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/meson_saradc.c | 860 +++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 873 insertions(+)
>>> create mode 100644 drivers/iio/adc/meson_saradc.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 9c8b558ba19e..86059b9b91bf 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -371,6 +371,18 @@ config MEN_Z188_ADC
>>> This driver can also be built as a module. If so, the module will be
>>> called men_z188_adc.
>>>
>>> +config MESON_SARADC
>>> + tristate "Amlogic Meson SAR ADC driver"
>>> + default ARCH_MESON
>>> + depends on OF && COMMON_CLK && (ARCH_MESON || COMPILE_TEST)
>>> + select REGMAP_MMIO
>>> + help
>>> + Say yes here to build support for the SAR ADC found in Amlogic Meson
>>> + SoCs.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called meson_saradc.
>>> +
>>> config MXS_LRADC
>>> tristate "Freescale i.MX23/i.MX28 LRADC"
>>> depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index d36c4be8d1fc..de05b9e75f8f 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -36,6 +36,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o
>>> obj-$(CONFIG_MCP3422) += mcp3422.o
>>> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>>> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>>> +obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
>>> obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
>>> obj-$(CONFIG_NAU7802) += nau7802.o
>>> obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>>> new file mode 100644
>>> index 000000000000..06e8ac620385
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/meson_saradc.c
>>> @@ -0,0 +1,860 @@
>>> +/*
>>> + * Amlogic Meson Successive Approximation Register (SAR) A/D Converter
>>> + *
>>> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define SAR_ADC_REG0 0x00
>>> + #define SAR_ADC_REG0_PANEL_DETECT BIT(31)
>>> + #define SAR_ADC_REG0_BUSY_MASK GENMASK(30, 28)
>>> + #define SAR_ADC_REG0_DELTA_BUSY BIT(30)
>>> + #define SAR_ADC_REG0_AVG_BUSY BIT(29)
>>> + #define SAR_ADC_REG0_SAMPLE_BUSY BIT(28)
>>> + #define SAR_ADC_REG0_FIFO_FULL BIT(27)
>>> + #define SAR_ADC_REG0_FIFO_EMPTY BIT(26)
>>> + #define SAR_ADC_REG0_FIFO_COUNT_MASK GENMASK(25, 21)
>>> + #define SAR_ADC_REG0_ADC_BIAS_CTRL_MASK GENMASK(20, 19)
>>> + #define SAR_ADC_REG0_CURR_CHAN_ID_MASK GENMASK(18, 16)
>>> + #define SAR_ADC_REG0_ADC_TEMP_SEN_SEL BIT(15)
>>> + #define SAR_ADC_REG0_SAMPLING_STOP BIT(14)
>>> + #define SAR_ADC_REG0_CHAN_DELTA_EN_MASK GENMASK(13, 12)
>>> + #define SAR_ADC_REG0_DETECT_IRQ_POL BIT(10)
>>> + #define SAR_ADC_REG0_DETECT_IRQ_EN BIT(9)
>>> + #define SAR_ADC_REG0_FIFO_CNT_IRQ_MASK GENMASK(8, 4)
>>> + #define SAR_ADC_REG0_FIFO_IRQ_EN BIT(3)
>>> + #define SAR_ADC_REG0_SAMPLING_START BIT(2)
>>> + #define SAR_ADC_REG0_CONTINUOUS_EN BIT(1)
>>> + #define SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE BIT(0)
>>> +
>>> +#define SAR_ADC_CHAN_LIST 0x04
>>> + #define SAR_ADC_CHAN_LIST_MAX_INDEX_MASK GENMASK(26, 24)
>>> + #define SAR_ADC_CHAN_CHAN_ENTRY_MASK(_chan) \
>>> + (GENMASK(2, 0) << (_chan * 3))
>>> +
>>> +#define SAR_ADC_AVG_CNTL 0x08
>>> + #define SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(_chan) \
>>> + (16 + (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_AVG_MODE_MASK(_chan) \
>>> + (GENMASK(17, 16) << (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(_chan) \
>>> + (0 + (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(_chan) \
>>> + (GENMASK(1, 0) << (_chan * 2))
>>> +
>>> +#define SAR_ADC_REG3 0x0c
>>> + #define SAR_ADC_REG3_CNTL_USE_SC_DLY BIT(31)
>>> + #define SAR_ADC_REG3_CLK_EN BIT(30)
>>> + #define SAR_ADC_REG3_BL30_INITIALIZED BIT(28)
>>> + #define SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN BIT(27)
>>> + #define SAR_ADC_REG3_CTRL_SAMPLING_CLOCK_PHASE BIT(26)
>>> + #define SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_REG3_DETECT_EN BIT(22)
>>> + #define SAR_ADC_REG3_ADC_EN BIT(21)
>>> + #define SAR_ADC_REG3_PANEL_DETECT_COUNT_MASK GENMASK(20, 18)
>>> + #define SAR_ADC_REG3_PANEL_DETECT_FILTER_TB_MASK GENMASK(17, 16)
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_SHIFT 10
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_WIDTH 5
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_MASK GENMASK(15, 10)
>>> + #define SAR_ADC_REG3_BLOCK_DLY_SEL_MASK GENMASK(9, 8)
>>> + #define SAR_ADC_REG3_BLOCK_DLY_MASK GENMASK(7, 0)
>>> +
>>> +#define SAR_ADC_DELAY 0x10
>>> + #define SAR_ADC_DELAY_INPUT_DLY_SEL_MASK GENMASK(25, 24)
>>> + #define SAR_ADC_DELAY_BL30_BUSY BIT(15)
>>> + #define SAR_ADC_DELAY_KERNEL_BUSY BIT(14)
>>> + #define SAR_ADC_DELAY_INPUT_DLY_CNT_MASK GENMASK(23, 16)
>>> + #define SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK GENMASK(9, 8)
>>> + #define SAR_ADC_DELAY_SAMPLE_DLY_CNT_MASK GENMASK(7, 0)
>>> +
>>> +#define SAR_ADC_LAST_RD 0x14
>>> + #define SAR_ADC_LAST_RD_LAST_CHANNEL1_MASK GENMASK(23, 16)
>>> + #define SAR_ADC_LAST_RD_LAST_CHANNEL0_MASK GENMASK(9, 0)
>>> +
>>> +#define SAR_ADC_FIFO_RD 0x18
>>> + #define SAR_ADC_FIFO_RD_CHAN_ID_MASK GENMASK(14, 12)
>>> + #define SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK GENMASK(11, 0)
>>> +
>>> +#define SAR_ADC_AUX_SW 0x1c
>>> + #define SAR_ADC_AUX_SW_MUX_SEL_CHAN_MASK(_chan) \
>>> + (GENMASK(10, 8) << ((_chan - 2) * 2))
>>> + #define SAR_ADC_AUX_SW_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_AUX_SW_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_AUX_SW_MODE_SEL BIT(4)
>>> + #define SAR_ADC_AUX_SW_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_AUX_SW_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_AUX_SW_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_AUX_SW_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_CHAN_10_SW 0x20
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_MUX_SEL_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_VREF_P_MUX BIT(22)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_VREF_N_MUX BIT(21)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_MODE_SEL BIT(20)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_YP_DRIVE_SW BIT(19)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_XP_DRIVE_SW BIT(18)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_YM_DRIVE_SW BIT(17)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_XM_DRIVE_SW BIT(16)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_MUX_SEL_MASK GENMASK(9, 7)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_MODE_SEL BIT(4)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_DETECT_IDLE_SW 0x24
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_SW_EN BIT(26)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_VREF_P_MUX BIT(22)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_VREF_N_MUX BIT(21)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_SEL BIT(20)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_YP_DRIVE_SW BIT(19)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_XP_DRIVE_SW BIT(18)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_YM_DRIVE_SW BIT(17)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_XM_DRIVE_SW BIT(16)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK GENMASK(9, 7)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_SEL BIT(4)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_DELTA_10 0x28
>>> + #define SAR_ADC_DELTA_10_TEMP_SEL BIT(27)
>>> + #define SAR_ADC_DELTA_10_TS_REVE1 BIT(26)
>>> + #define SAR_ADC_DELTA_10_CHAN1_DELTA_VALUE_SHIFT 16
>>> + #define SAR_ADC_DELTA_10_CHAN1_DELTA_VALUE_MASK GENMASK(25, 16)
>>> + #define SAR_ADC_DELTA_10_TS_REVE0 BIT(15)
>>> + #define SAR_ADC_DELTA_10_TS_C_SHIFT 11
>>> + #define SAR_ADC_DELTA_10_TS_C_MASK GENMASK(14, 11)
>>> + #define SAR_ADC_DELTA_10_TS_VBG_EN BIT(10)
>>> + #define SAR_ADC_DELTA_10_CHAN0_DELTA_VALUE_SHIFT 0
>>> + #define SAR_ADC_DELTA_10_CHAN0_DELTA_VALUE_MASK GENMASK(9, 0)
>>> +
>>> +/* NOTE: registers from here are undocumented (the vendor Linux kernel driver
>>> + * and u-boot source served as reference). These only seem to be relevant on
>>> + * GXBB and newer.
>>> + */
>>> +#define SAR_ADC_REG11 0x2c
>>> + #define SAR_ADC_REG11_BANDGAP_EN BIT(13)
>>> +
>>> +#define SAR_ADC_REG13 0x34
>>> + #define SAR_ADC_REG13_12BIT_CALIBRATION_MASK GENMASK(13, 8)
>>> +
>>> +#define SAR_ADC_MAX_FIFO_SIZE 32
>>> +#define SAR_ADC_NUM_CHANNELS ARRAY_SIZE(meson_saradc_iio_channels)
>>> +#define SAR_ADC_VALUE_MASK(_priv) (BIT(_priv->resolution) - 1)
>>> +
>>> +#define MESON_SAR_ADC_CHAN(_chan, _type) { \
>>> + .type = _type, \
>>> + .indexed = true, \
>>> + .channel = _chan, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>> + BIT(IIO_CHAN_INFO_AVERAGE_RAW), \
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>>> + .datasheet_name = "SAR_ADC_CH"#_chan, \
>>> +}
>>> +
>>> +/* TODO: the hardware supports IIO_TEMP for channel 6 as well which is
>>> + * currently not supported by this driver.
>>> + */
>>> +static const struct iio_chan_spec meson_saradc_iio_channels[] = {
>>> + MESON_SAR_ADC_CHAN(0, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(1, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(2, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(3, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(4, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(5, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(6, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(7, IIO_VOLTAGE),
>>> + IIO_CHAN_SOFT_TIMESTAMP(8),
>>> +};
>>> +
>>> +enum meson_saradc_avg_mode {
>>> + NO_AVERAGING = 0x0,
>>> + MEAN_AVERAGING = 0x1,
>>> + MEDIAN_AVERAGING = 0x2,
>>> +};
>>> +
>>> +enum meson_saradc_num_samples {
>>> + ONE_SAMPLE = 0x0,
>>> + TWO_SAMPLES = 0x1,
>>> + FOUR_SAMPLES = 0x2,
>>> + EIGHT_SAMPLES = 0x3,
>>> +};
>>> +
>>> +enum meson_saradc_chan7_mux_sel {
>>> + CHAN7_MUX_VSS = 0x0,
>>> + CHAN7_MUX_VDD_DIV4 = 0x1,
>>> + CHAN7_MUX_VDD_DIV2 = 0x2,
>>> + CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
>>> + CHAN7_MUX_VDD = 0x4,
>>> + CHAN7_MUX_CH7_INPUT = 0x7,
>>> +};
>>> +
>>> +struct meson_saradc_priv {
>>> + struct regmap *regmap;
>>> + struct clk *clkin;
>>> + struct clk *core_clk;
>>> + struct clk *sana_clk;
>>> + struct clk *adc_sel_clk;
>>> + struct clk *adc_clk;
>>> + struct clk_gate clk_gate;
>>> + struct clk *adc_div_clk;
>>> + struct clk_divider clk_div;
>>> + struct regulator *vref;
>>> + struct completion completion;
>> This struct completion isn't used currently. Most likely it was meant
>> in peparation of using interrupt mode.
> indeed
>
>>> + u8 resolution;
>>> +};
>>> +
>>> +static const struct regmap_config meson_saradc_regmap_config = {
>>> + .reg_bits = 8,
>>> + .val_bits = 32,
>>> + .reg_stride = 4,
>>> + .max_register = SAR_ADC_REG13,
>>> +};
>>> +
>>> +static unsigned int meson_saradc_get_fifo_count(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> +
>>> + return FIELD_GET(SAR_ADC_REG0_FIFO_COUNT_MASK, regval);
>>> +}
>>> +
>>> +static int meson_saradc_wait_busy_clear(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int regval, timeout = 10000;
>>> +
>>> + do {
>>> + udelay(1);
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> + } while (FIELD_GET(SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
>>> +
>>> + if (timeout < 0)
>>> + return -ETIMEDOUT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_read_raw_sample(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
>>> +
>>> + ret = meson_saradc_wait_busy_clear(indio_dev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>> The resulting regval value isn't used, therefore this statement doesn't seem
>> to be needed.
> I can probably replace this with "0", good catch!
>
>> In the vendor driver there is such a dummy statement before reading the busy
>> flags in REG0 after starting sampling. Reason seems to be a potential race
>> when we try to read the busy flags before the sampling engine has set them.
>> This isn't needed in meson_saradc_wait_busy_clear here as an udelay(1) is
>> done first always.
> do you think it's worth adding a comment here that a do ... while loop
> is there on purpose?
>
Yes, a hint would be good that there's a potential race.
Else there's a good chance that a future refactoring introduces a regression.
>>> +
>>> + while (meson_saradc_get_fifo_count(indio_dev) > 0 &&
>> IMHO this loop isn't needed. When we come here the FIFO contains exactly
>> one element. This is true also in averaging mode as the averaging engine
>> writes only the resulting mean value to the FIFO.
>>
>> And we can't have multiple samples active in parallel due to the locking
>> done in meson_saradc_get_sample.
>>
>> By the way: I use an IRQ here to wake up when the FIFO contains one
>> element. But as you wrote: It's not clear whether this works on all
>> Meson systems.
> maybe I should switch to IRQ mode as even the old Meson6 vendor kernel
> sources indicate that the SAR ADC has IRQ support?
>
As you like. We can also add interrupt mode later (but leave polling intact)
and activate it only if an interrupt is set in DT.
This way we'd have a fallback in case there should be a problem with
interrupt mode on some system.
>>> + count < SAR_ADC_MAX_FIFO_SIZE) {
>>> + regmap_read(priv->regmap, SAR_ADC_FIFO_RD, ®val);
>>> +
>>> + fifo_chan = FIELD_GET(SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>>> + if (fifo_chan == chan->channel) {
>>> + fifo_val = FIELD_GET(SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK,
>>> + regval) & SAR_ADC_VALUE_MASK(priv);
>>> + sum += fifo_val;
>>> + count++;
>>> + }
>>> + }
>>> +
>>> + if (!count)
>>> + return -ENOENT;
>>> +
>>> + *val = sum / count;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_saradc_set_averaging(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + enum meson_saradc_avg_mode mode,
>>> + enum meson_saradc_num_samples samples)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 val;
>>> +
>>> + val = samples << SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>> + SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(chan->channel),
>>> + val);
>>> +
>>> + val = mode << SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>> + SAR_ADC_AVG_CNTL_AVG_MODE_MASK(chan->channel), val);
>>> +}
>>> +
>>> +static void meson_saradc_enable_channel(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + /* the SAR ADC engine allows sampling multiple channels at the same
>>> + * time. to keep it simple we're only working with one *internal*
>>> + * channel, which starts counting at index 0 (which means: count = 1).
>>> + */
>>> + regval = FIELD_PREP(SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>> + SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, regval);
>>> +
>>> + /* map channel index 0 to the channel which we want to read */
>>> + regval = FIELD_PREP(SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>> + SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), regval);
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>> + chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>> + SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>> + regval);
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>> + chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>> + SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>> + regval);
>>> +
>>> + if (chan->channel == 6)
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELTA_10,
>>> + SAR_ADC_DELTA_10_TEMP_SEL, 0);
>>> +}
>>> +
>>> +static void meson_saradc_set_channel7_mux(struct iio_dev *indio_dev,
>>> + enum meson_saradc_chan7_mux_sel sel)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
>>> +
>>> + usleep_range(10, 20);
>>> +}
>>> +
>>> +static void meson_saradc_start_sample_engine(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLING_START,
>>> + SAR_ADC_REG0_SAMPLING_START);
>>> +}
>>> +
>>> +static void meson_saradc_stop_sample_engine(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLING_STOP,
>>> + SAR_ADC_REG0_SAMPLING_STOP);
>>> +
>>> + /* wait until all modules are stopped */
>>> + meson_saradc_wait_busy_clear(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE, 0);
>>> +}
>>> +
>>> +static void meson_saradc_lock(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int val;
>>> +
>>> + mutex_lock(&indio_dev->mlock);
>>> +
>>> + /* prevent BL30 from using the SAR ADC while we are using it */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY);
>>> +
>>> + /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
>>> + do {
>>> + udelay(1);
>>> + regmap_read(priv->regmap, SAR_ADC_DELAY, &val);
>>> + } while (val & SAR_ADC_DELAY_BL30_BUSY);
>>> +}
>>> +
>>> +static void meson_saradc_unlock(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + /* allow BL30 to use the SAR ADC again */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY, 0);
>>> +
>>> + mutex_unlock(&indio_dev->mlock);
>>> +}
>>> +
>>> +static int meson_saradc_get_sample(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + enum meson_saradc_avg_mode avg_mode,
>>> + enum meson_saradc_num_samples avg_samples,
>>> + int *val)
>>> +{
>>> + int ret, tmp;
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + /* clear old values from the FIFO buffer, ignoring errors */
>>> + meson_saradc_read_raw_sample(indio_dev, chan, &tmp);
>>> +
>>> + meson_saradc_set_averaging(indio_dev, chan, avg_mode, avg_samples);
>>> +
>>> + meson_saradc_enable_channel(indio_dev, chan);
>>> +
>>> + meson_saradc_start_sample_engine(indio_dev);
>>> + ret = meson_saradc_read_raw_sample(indio_dev, chan, val);
>>> + meson_saradc_stop_sample_engine(indio_dev);
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +
>>> + if (ret) {
>>> + dev_warn(&indio_dev->dev,
>> Using the struct device in indio_dev results in IMHO ugly messages like
>> iio iio:device0: already initialized by BL30
>>
>> We should use the parent instead, this is more readable:
>> meson-saradc c1108680.adc: already initialized by BL30
>>
>> For this we need to move the assignment to indio_dev->dev.parent
>> in probe, else messages may be written when parent isn't set yet.
> indeed, I'll change this - thanks for the hint!
>
>>> + "failed to read sample for channel %d: %d\n",
>>> + chan->channel, ret);
>>> + return ret;
>>> + }
>>> +
>>> + return IIO_VAL_INT;
>>> +}
>>> +
>>> +static int meson_saradc_iio_info_read_raw(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val, int *val2, long mask)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + return meson_saradc_get_sample(indio_dev, chan, NO_AVERAGING,
>>> + ONE_SAMPLE, val);
>>> + break;
>>> +
>>> + case IIO_CHAN_INFO_AVERAGE_RAW:
>>> + return meson_saradc_get_sample(indio_dev, chan, MEAN_AVERAGING,
>>> + EIGHT_SAMPLES, val);
>>> + break;
>>> +
>>> + case IIO_CHAN_INFO_SCALE:
>>> + ret = regulator_get_voltage(priv->vref);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "failed to get vref voltage: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + *val = ret / 1000;
>>> + *val2 = priv->resolution;
>>> + return IIO_VAL_FRACTIONAL_LOG2;
>>> +
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +}
>>> +
>>> +static int meson_saradc_clk_init(struct iio_dev *indio_dev, void __iomem *base)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + struct clk_init_data init;
>>> + char clk_name[32];
>>> + const char *clk_parents[1];
>>> +
>>> + snprintf(clk_name, sizeof(clk_name), "%s#adc_div",
>>> + of_node_full_name(indio_dev->dev.of_node));
>>> + init.name = devm_kstrdup(&indio_dev->dev, clk_name, GFP_KERNEL);
>> Consider replacing snprintf + devm_kstrdup with devm_kasprintf
> I must have missed that, thanks for another good catch!
>
>>> + init.flags = 0;
>>> + init.ops = &clk_divider_ops;
>>> + clk_parents[0] = __clk_get_name(priv->clkin);
>>> + init.parent_names = clk_parents;
>>> + init.num_parents = 1;
>>> +
>>> + priv->clk_div.reg = base + SAR_ADC_REG3;
>>> + priv->clk_div.shift = SAR_ADC_REG3_ADC_CLK_DIV_SHIFT;
>>> + priv->clk_div.width = SAR_ADC_REG3_ADC_CLK_DIV_WIDTH;
>>> + priv->clk_div.hw.init = &init;
>>> + priv->clk_div.flags = 0;
>>> +
>>> + priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
>>> + &priv->clk_div.hw);
>>> + if (WARN_ON(IS_ERR(priv->adc_div_clk)))
>>> + return PTR_ERR(priv->adc_div_clk);
>>> +
>>> + snprintf(clk_name, sizeof(clk_name), "%s#adc_en",
>>> + of_node_full_name(indio_dev->dev.of_node));
>>> + init.name = devm_kstrdup(&indio_dev->dev, clk_name, GFP_KERNEL);
>>> + init.flags = CLK_SET_RATE_PARENT;
>>> + init.ops = &clk_gate_ops;
>>> + clk_parents[0] = __clk_get_name(priv->adc_div_clk);
>>> + init.parent_names = clk_parents;
>>> + init.num_parents = 1;
>>> +
>>> + priv->clk_gate.reg = base + SAR_ADC_REG3;
>>> + priv->clk_gate.bit_idx = fls(SAR_ADC_REG3_CLK_EN);
>>> + priv->clk_gate.hw.init = &init;
>>> +
>>> + priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
>>> + if (WARN_ON(IS_ERR(priv->adc_clk)))
>>> + return PTR_ERR(priv->adc_clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_init(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int regval, ret;
>>> +
>>> + /* make sure we start at CH7 input */
>>> + meson_saradc_set_channel7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG3, ®val);
>>> + if (regval & SAR_ADC_REG3_BL30_INITIALIZED) {
>>> + dev_info(&indio_dev->dev, "already initialized by BL30\n");
>>> + return 0;
>>> + }
>>> +
>>> + dev_info(&indio_dev->dev, "initializing SAR ADC\n");
>>> +
>>> + meson_saradc_stop_sample_engine(indio_dev);
>>> +
>>> + /* update the channel 6 MUX to select the temperature sensor */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_ADC_TEMP_SEN_SEL,
>>> + SAR_ADC_REG0_ADC_TEMP_SEN_SEL);
>>> +
>>> + /* disable all channels by default */
>>> + regmap_write(priv->regmap, SAR_ADC_CHAN_LIST, 0x0);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CTRL_SAMPLING_CLOCK_PHASE, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CNTL_USE_SC_DLY,
>>> + SAR_ADC_REG3_CNTL_USE_SC_DLY);
>>> +
>>> + /* delay between two samples = (10+1) * 1uS */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_CNT_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_SAMPLE_DLY_CNT_MASK, 10));
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK, 0));
>>> +
>>> + /* delay between two samples = (10+1) * 1uS */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_CNT_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_INPUT_DLY_CNT_MASK, 10));
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_SEL_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_INPUT_DLY_SEL_MASK, 1));
>>> +
>>> + ret = clk_set_parent(priv->adc_sel_clk, priv->clkin);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev,
>>> + "failed to set adc parent to clkin\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = clk_set_rate(priv->adc_clk, 1200000);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to set adc clock rate\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_hw_enable(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + ret = regulator_enable(priv->vref);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev, "failed to enable vref regulator\n");
>>> + goto err_vref;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->core_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable core clk\n");
>>> + goto err_core_clk;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->sana_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable sana clk\n");
>>> + goto err_sana_clk;
>>> + }
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG11,
>>> + SAR_ADC_REG11_BANDGAP_EN, SAR_ADC_REG11_BANDGAP_EN);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3, SAR_ADC_REG3_ADC_EN,
>>> + SAR_ADC_REG3_ADC_EN);
>>> +
>>> + udelay(5);
>>> +
>>> + ret = clk_prepare_enable(priv->adc_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable adc_en clk\n");
>>> + goto err_adc_clk;
>>> + }
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +
>>> + return 0;
>>> +
>>> +err_adc_clk:
>>> + clk_disable_unprepare(priv->sana_clk);
>>> +err_sana_clk:
>>> + clk_disable_unprepare(priv->core_clk);
>>> +err_core_clk:
>>> + regulator_disable(priv->vref);
>>> +err_vref:
>>> + meson_saradc_unlock(indio_dev);
>>> + return ret;
>>> +}
>>> +
>>> +static void meson_saradc_hw_disable(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + clk_disable_unprepare(priv->adc_clk);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3, SAR_ADC_REG3_ADC_EN, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG11,
>>> + SAR_ADC_REG11_BANDGAP_EN, 0);
>>> +
>>> + clk_disable_unprepare(priv->sana_clk);
>>> + clk_disable_unprepare(priv->core_clk);
>>> +
>>> + regulator_disable(priv->vref);
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +}
>>> +
>>> +static const struct iio_info meson_saradc_iio_info = {
>>> + .read_raw = meson_saradc_iio_info_read_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static const struct of_device_id meson_saradc_of_match[] = {
>>> + {
>>> + .compatible = "amlogic,meson-gxbb-saradc",
>>> + .data = (void *)10,
>>> + }, {
>>> + .compatible = "amlogic,meson-gxl-saradc",
>>> + .data = (void *)12,
>>> + },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, meson_saradc_of_match);
>>> +
>>> +static int meson_saradc_probe(struct platform_device *pdev)
>>> +{
>>> + struct meson_saradc_priv *priv;
>>> + struct iio_dev *indio_dev;
>>> + struct resource *res;
>>> + void __iomem *base;
>>> + const struct of_device_id *match;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
>>> + if (!indio_dev) {
>>> + dev_err(&pdev->dev, "failed allocating iio device\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + priv = iio_priv(indio_dev);
>>> +
>>> + match = of_match_device(meson_saradc_of_match, &pdev->dev);
>>> + priv->resolution = (unsigned long)match->data;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(base))
>>> + return PTR_ERR(base);
>>> +
>>> + priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>> + &meson_saradc_regmap_config);
>>> + if (IS_ERR(priv->regmap))
>>> + return PTR_ERR(priv->regmap);
>>> +
>>> + init_completion(&priv->completion);
>>> +
>>> + priv->clkin = devm_clk_get(&pdev->dev, "clkin");
>>> + if (IS_ERR(priv->clkin)) {
>>> + dev_err(&pdev->dev, "failed to get clkin\n");
>>> + return PTR_ERR(priv->clkin);
>>> + }
>>> +
>>> + priv->core_clk = devm_clk_get(&pdev->dev, "core");
>>> + if (IS_ERR(priv->core_clk)) {
>>> + dev_err(&pdev->dev, "failed to get core clk\n");
>>> + return PTR_ERR(priv->core_clk);
>>> + }
>>> +
>>> + priv->sana_clk = devm_clk_get(&pdev->dev, "sana");
>>> + if (IS_ERR(priv->sana_clk)) {
>>> + if (PTR_ERR(priv->sana_clk) == -ENOENT) {
>>> + priv->sana_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get sana clk\n");
>>> + return PTR_ERR(priv->sana_clk);
>>> + }
>>> + }
>>> +
>>> + priv->adc_clk = devm_clk_get(&pdev->dev, "adc_clk");
>>> + if (IS_ERR(priv->adc_clk)) {
>>> + if (PTR_ERR(priv->adc_clk) == -ENOENT) {
>>> + priv->adc_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get adc clk\n");
>>> + return PTR_ERR(priv->adc_clk);
>>> + }
>>> + }
>>> +
>>> + priv->adc_sel_clk = devm_clk_get(&pdev->dev, "adc_sel");
>>> + if (IS_ERR(priv->adc_sel_clk)) {
>>> + if (PTR_ERR(priv->adc_sel_clk) == -ENOENT) {
>>> + priv->adc_sel_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get adc_sel clk\n");
>>> + return PTR_ERR(priv->adc_sel_clk);
>>> + }
>>> + }
>>> +
>>> + /* on pre-GXBB SoCs the SAR ADC itself provides the ADC clock: */
>>> + if (!priv->adc_clk) {
>>> + ret = meson_saradc_clk_init(indio_dev, base);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + priv->vref = devm_regulator_get(&pdev->dev, "vref");
>>> + if (IS_ERR(priv->vref)) {
>>> + dev_err(&pdev->dev, "failed to get vref regulator\n");
>>> + return PTR_ERR(priv->vref);
>>> + }
>>> +
>>> + ret = meson_saradc_init(indio_dev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> + ret = meson_saradc_hw_enable(indio_dev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> + platform_set_drvdata(pdev, indio_dev);
>>> +
>>> + indio_dev->name = dev_name(&pdev->dev);
>>> + indio_dev->dev.parent = &pdev->dev;
>>> + indio_dev->dev.of_node = pdev->dev.of_node;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &meson_saradc_iio_info;
>>> +
>>> + indio_dev->channels = meson_saradc_iio_channels;
>>> + indio_dev->num_channels = SAR_ADC_NUM_CHANNELS;
>>> +
>>> + ret = iio_device_register(indio_dev);
>>> + if (ret)
>>> + goto err_hw;
>>> +
>>> + return 0;
>>> +
>>> +err_hw:
>>> + meson_saradc_hw_disable(indio_dev);
>>> +err:
>>> + return ret;
>>> +}
>>> +
>>> +static int meson_saradc_remove(struct platform_device *pdev)
>>> +{
>>> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>> +
>>> + meson_saradc_hw_disable(indio_dev);
>>> + iio_device_unregister(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int meson_saradc_suspend(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +
>>> + meson_saradc_hw_disable(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_resume(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +
>>> + return meson_saradc_hw_enable(indio_dev);
>>> +}
>>> +#endif /* CONFIG_PM_SLEEP */
>>> +
>>> +static SIMPLE_DEV_PM_OPS(meson_saradc_pm_ops,
>>> + meson_saradc_suspend, meson_saradc_resume);
>>> +
>>> +static struct platform_driver meson_saradc_driver = {
>>> + .probe = meson_saradc_probe,
>>> + .remove = meson_saradc_remove,
>>> + .driver = {
>>> + .name = "meson-saradc",
>>> + .of_match_table = meson_saradc_of_match,
>>> + .pm = &meson_saradc_pm_ops,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(meson_saradc_driver);
>>> +
>>> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>");
>>> +MODULE_DESCRIPTION("Amlogic Meson SAR ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>>
>
^ permalink raw reply
* [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2017-01-13 20:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqL+L1oHAOv7tLevzbtRFSKRs9J01SgF=HR_vNNqNV+4+Q@mail.gmail.com>
On 01/13/2017 01:25 PM, Rob Herring wrote:
> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Rob,
>>
>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>
>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>
>>>> Rob,
>>>>
>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>
>>>>>>
>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>> control device power states.
>>>>>>
>>>>>> Also, provide macros representing each device index as understood
>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>
>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>> ---
>>>>>> v2->v3:
>>>>>> Update k2g_pds node docs to show it should be a child of pmmc
>>>>>> node.
>>>>>> In early versions a phandle was used to point to pmmc and docs
>>>>>> still
>>>>>> incorrectly showed this.
>>>>>>
>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59 ++++++++++++++
>>>>>> MAINTAINERS | 2 +
>>>>>> include/dt-bindings/genpd/k2g.h | 90
>>>>>> ++++++++++++++++++++++
>>>>>> 3 files changed, 151 insertions(+)
>>>>>> create mode 100644
>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..4c9064e512cb
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> @@ -0,0 +1,59 @@
>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>> +---------------------------------------------
>>>>>> +
>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that
>>>>>> is
>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>> +Communication between the host processor running an OS and the system
>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>> domain
>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>> use
>>>>>> of
>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>> +
>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>> +
>>>>>> +PM Domain Node
>>>>>> +==============
>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>> PMMC,
>>>>>> +which in this case is the single implementation as documented by the
>>>>>> generic
>>>>>> +PM domain bindings in
>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>>> PMMC
>>>>>> it
>>>>>> +must be a child of the pmmc node.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +--------------------
>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>> +- #power-domain-cells: Must be 0.
>>>>>> +
>>>>>> +Example (K2G):
>>>>>> +-------------
>>>>>> + pmmc: pmmc {
>>>>>> + compatible = "ti,k2g-sci";
>>>>>> + ...
>>>>>> +
>>>>>> + k2g_pds: k2g_pds {
>>>>>> + compatible = "ti,sci-pm-domain";
>>>>>> + #power-domain-cells = <0>;
>>>>>> + };
>>>>>> + };
>>>>>> +
>>>>>> +PM Domain Consumers
>>>>>> +===================
>>>>>> +Hardware blocks that require SCI control over their state must provide
>>>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>>>> +specific ID that identifies the device.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +--------------------
>>>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>>>> + be used for device control.
>>>>>
>>>>>
>>>>>
>>>>> As I've already stated before, this goes in power-domain cells. When you
>>>>> have a single thing (i.e. node) that controls multiple things, then you
>>>>> you need to specify the ID for each of them in phandle args. This is how
>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>
>>>>
>>>>
>>>> You think the reasoning for doing it this way provided by both Ulf and
>>>> myself on v2 [1] is not valid then?
>>>>
>>>> From Ulf:
>>>>
>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>> in DT and assign to a device node. The only difference here, is that
>>>> we don't have common API to fetch the resource (like clk_get(),
>>>> regulator_get()), but instead we fetches the device's resource from
>>>> SoC specific code, via genpd's device ->attach() callback.
>>>
>>>
>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>> do with DT bindings.
>>>
>>>> From me:
>>>>
>>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>>> list
>>>> of genpds but rather identifies the device *within* a single genpd. It is
>>>> a
>>>> property specific to each device that resides in a ti-sci-genpd, not a
>>>> mapping describing which genpd the device belongs to. The generic power
>>>> domain binding is concerned with mapping the device to a specific genpd,
>>>> which is does fine for us, but we have a sub mapping for devices that
>>>> exist
>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>
>>>>
>>>> So to summarize, the genpd framework does interpret the phandle arg as an
>>>> index into multiple genpds, just as you've said other frameworks do, but
>>>> this is not what I am trying to do, we have multiple devices within this
>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>
>>>
>>> Fix the genpd framework rather than work around it in DT.
>>
>>
>> I still disagree that this has nothing to do with DT bindings, as the
>> current DT binding represents something different already. I am trying to
>> extend it to give me additional information needed for our platforms. Are
>> you saying that we should break what the current DT binding already
>> represents to mean something else?
>
> No idea because what's the current binding? From the patch, looks like
> a new binding to me.
Yes, ti,sci-id is a new binding. I am referring to the current meaning
of the "power-domains" binding, which is where you are asking this
property to be added, in "power-domains" cells. This is documented here
[1] in the kernel, although looking at it I must admit it is not very clear.
The power-domains cell represents an offset into an array of power
domains, if you choose to use it. That's what the genpd framework is
hard coded to interpret it as. This is correct, as it is an index into a
static list of power domains, used to identify which power domain a
device belongs to, which is exactly what the genpd framework itself is
concerned with. This is already how it is used in the kernel today.
My ti,sci-id is not an index into a list of power domains, so it should
not go in the power-domains cells and go against what the power-domains
binding says that the cell expects. We have one single power domain, and
the new ti,sci-id binding is not something the genpd framework itself is
concerned with as it's our property to identify a device inside a power
domain, not to identify which power domain it is associated with.
Regards,
Dave
[1] Documentation/devicetree/bindings/power/power_domain.txt
>
> Rob
>
^ permalink raw reply
* [PATCH] ARM: ux500: cut some platform data
From: Arnd Bergmann @ 2017-01-13 20:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113131648.24889-1-linus.walleij@linaro.org>
On Friday, January 13, 2017 2:16:48 PM CET Linus Walleij wrote:
> This platform data is revoked: the drivers are getting the DMA
> configuration from the device tree, it has been done like that
> since the DMA support was merged and this data has not been used
> since. The remaining auxdata is also unused.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> arch/arm/mach-ux500/board-mop500-audio.c | 77 -----------------------------
> arch/arm/mach-ux500/board-mop500.h | 17 -------
> arch/arm/mach-ux500/cpu-db8500.c | 16 ------
> arch/arm/mach-ux500/ste-dma40-db8500.h | 85 --------------------------------
>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero
From: Bin Liu @ 2017-01-13 21:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112173938.5am5njwrbs6p62br@lukather>
On Thu, Jan 12, 2017 at 06:39:38PM +0100, Maxime Ripard wrote:
> Hi Bin,
>
> On Thu, Jan 12, 2017 at 08:50:14AM -0600, Bin Liu wrote:
> > On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> > > On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > > > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > > > >
> > > > >
> > > > > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > > > >> ?Lichee Pi Zero features a USB OTG port.
> > > > > >>
> > > > > >> ?Add support for it.
> > > > > >>
> > > > > >> ?Note: in order to use the Host mode, the board must be powered via the
> > > > > >> ?+5V and GND pins.
> > > > > >>
> > > > > >> ?Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > > > >> ?---
> > > > > >> ??arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > > > > >> ??1 file changed, 10 insertions(+)
> > > > > >>
> > > > > >> ?diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> ?index 0099affc6ce3..3d9168cbaeca 100644
> > > > > >> ?--- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> ?+++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> ?@@ -71,3 +71,13 @@
> > > > > >> ??????????pinctrl-names = "default";
> > > > > >> ??????????status = "okay";
> > > > > >> ??};
> > > > > >> ?+
> > > > > >> ?+&usb_otg {
> > > > > >> ?+ dr_mode = "otg";
> > > > > >
> > > > > > Why not set this default mode in dtsi instead?
> > > > > >
> > > > > > Regards,
> > > > > > -Bin.
> > > > >
> > > > > There's possibly boards which do not have OTG functions.
> > > >
> > > > That is board specific.
> > >
> > > Exactly, and this is why it should be done in the board DT.
> >
> > I am just suggesting based on the common practice. If a .dtsi exists for
> > a family, the .dtsi describes the device and common properties for all
> > possible boards, and each board .dts adds or overrides its specific
> > implementation. Kernel has many devices/boards done in this way - define
> > the default dr_mode in .dtsi.
> >
> > In this case, I suggest to set the common dr_mode in .dtsi, then each
> > board .dts only overrides it if the implementation is different.
> >
> > >
> > > The controller in the Allwinner SoCs do not handle directly the ID pin
> > > and VBUS, but rather rely on a GPIO to do so.
> > >
> > > So boards with OTG will need setup anyway, at least to tell which
> > > GPIOs are used. There's no point in enforcing a default if it doesn't
> > > work by default.
> >
> > Then define a default which supposes to work for most boards.
> >
> > Why I suggest this, is because defining a default dr_mode which works
> > for most cases in dtsi could prevent a little surprise in MUSB function.
> > If someone designs a new board but forgets to define dr_mode in the new
> > board DT, the MUSB driver will default to org mode, which might not be
> > intended.
>
> The point is that there is no sensible default. Some boards don't have
> an ID pin and no VBUS (peripheral), some don't have an ID pin but VBUS
> (host), and some have an ID pin but no controllable VBUS, some have an
> ID pin and a controllable VBUS, but we have no idea which GPIOs are
> used.
>
> There's no way we can have something that works on most cases.
Ok, understood.
Regards,
-Bin.
^ permalink raw reply
* [PATCH net-next v2 08/10] net: dsa: Add support for platform data
From: Florian Fainelli @ 2017-01-13 22:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113140459.GH10203@lunn.ch>
On 01/13/2017 06:04 AM, Andrew Lunn wrote:
>> index cd91070b5467..d326fc4afad7 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -81,17 +81,23 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
>>
>> static bool dsa_port_is_valid(struct dsa_port *port)
>> {
>> - return !!port->dn;
>> + return !!(port->dn || port->name);
>> }
>
> Does this clash with Viviens recent change to make names optional and
> have the kernel assign it?
So there were two ways to look at this, one was that could check here
that ds->pd is assigned and port->name is assigned, which means that
platform data has to provide valid port name. We can also eliminate this
check entirely because we now support NULL names just fines.
>
> I suppose you could use an name of "eth%d"? Is it worth adding a
> comment to the platform data structure?
Humm, that could be done, maybe for simplicity we can just let
net/dsa/dsa2.c assign names either based on what platform data provided,
or by falling back to eth%d.
Thanks!
--
Florian
^ permalink raw reply
* [PATCH net-next v2 08/10] net: dsa: Add support for platform data
From: Florian Fainelli @ 2017-01-13 22:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113141110.GI10203@lunn.ch>
On 01/13/2017 06:11 AM, Andrew Lunn wrote:
>> static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
>> {
>> + struct dsa_chip_data *pdata = dev->platform_data;
>> struct device_node *np = dev->of_node;
>> struct dsa_switch_tree *dst;
>> struct device_node *ports;
>> u32 tree, index;
>> int i, err;
>>
>> - err = dsa_parse_member_dn(np, &tree, &index);
>> - if (err)
>> - return err;
>> + if (np) {
>> + err = dsa_parse_member_dn(np, &tree, &index);
>> + if (err)
>> + return err;
>>
>> - ports = dsa_get_ports(ds, np);
>> - if (IS_ERR(ports))
>> - return PTR_ERR(ports);
>> + ports = dsa_get_ports(ds, np);
>> + if (IS_ERR(ports))
>> + return PTR_ERR(ports);
>>
>> - err = dsa_parse_ports_dn(ports, ds);
>> - if (err)
>> - return err;
>> + err = dsa_parse_ports_dn(ports, ds);
>> + if (err)
>> + return err;
>> + } else {
>> + err = dsa_parse_member(pdata, &tree, &index);
>
Hello Andrew,
> Hi Florian
>
> Maybe it is hiding, but i don't see anywhere you check that pdata !=
> NULL.
You are right, there is not such a check, it should probably be added
early on.
>
> At least for x86 platforms, i don't expect we are booting using
> platform data like ARM systems used to do. I think it is more likely a
> glue module will be loaded. It looks up the MDIO bus and appends a
> platform data to an MDIO device. The switch driver then needs to load
> and use the platform data. But if things happen in a different order,
> it could be the switch driver probes before the glue driver, meaning
> pdata is NULL.
That's very valid, I will fix this, thanks!
>
> Do we even want to return -EPROBE_DEFERED?
I was trying to exercise that code path a little bit, but could not
quite make sense of what I was seeing, let me try again with more tracing.
--
Florian
^ permalink raw reply
* [PATCH v3 02/24] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
From: Steve Longerbeam @ 2017-01-13 22:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484308678.31475.24.camel@pengutronix.de>
On 01/13/2017 03:57 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> Add to the MIPI CSI2 receiver node: compatible string, interrupt sources,
>> clocks.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> arch/arm/boot/dts/imx6qdl.dtsi | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
>> index 53e6e63..42926e9 100644
>> --- a/arch/arm/boot/dts/imx6qdl.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
>> @@ -1125,7 +1125,14 @@
>> };
>>
>> mipi_csi: mipi at 021dc000 {
>> + compatible = "fsl,imx6-mipi-csi2";
>> reg = <0x021dc000 0x4000>;
>> + interrupts = <0 100 0x04>, <0 101 0x04>;
>> + clocks = <&clks IMX6QDL_CLK_HSI_TX>,
>> + <&clks IMX6QDL_CLK_VIDEO_27M>,
>> + <&clks IMX6QDL_CLK_EIM_SEL>;
> I think the latter should be EIM_PODF
done.
>
>> + clock-names = "dphy", "cfg", "pix";
> and I'm not sure dphy is the right name for this one. Is that the pll
> ref input?
I believe this naming came from FSL's mipi csi-2 driver. It is the "hsi_tx"
clock (presumably for the MIPI HSI controller) whose parents are selected
by the CDCDR register as PLL3_120M or PLL2_PFD2. I have no clue whether
this is indeed also used as the clock for the MIPI CSI-2 D-PHY, but
according
to FSL naming convention it might be.
Steve
^ permalink raw reply
* [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
From: Steve Longerbeam @ 2017-01-13 23:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484309021.31475.29.camel@pengutronix.de>
On 01/13/2017 04:03 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor.
>> Both hang off the same i2c2 bus, so they require different (and non-
>> default) i2c slave addresses.
>>
>> The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux.
>>
>> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
>> mipi_csi. It is set to transmit over MIPI virtual channel 1.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> arch/arm/boot/dts/imx6dl-sabrelite.dts | 5 ++
>> arch/arm/boot/dts/imx6q-sabrelite.dts | 6 ++
>> arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 118 +++++++++++++++++++++++++++++++
>> 3 files changed, 129 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6dl-sabrelite.dts b/arch/arm/boot/dts/imx6dl-sabrelite.dts
>> index 0f06ca5..fec2524 100644
>> --- a/arch/arm/boot/dts/imx6dl-sabrelite.dts
>> +++ b/arch/arm/boot/dts/imx6dl-sabrelite.dts
>> @@ -48,3 +48,8 @@
>> model = "Freescale i.MX6 DualLite SABRE Lite Board";
>> compatible = "fsl,imx6dl-sabrelite", "fsl,imx6dl";
>> };
>> +
>> +&ipu1_csi1_from_ipu1_csi1_mux {
>> + data-lanes = <0 1>;
>> + clock-lanes = <2>;
>> +};
>> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
>> index 66d10d8..9e2d26d 100644
>> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
>> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
>> @@ -52,3 +52,9 @@
>> &sata {
>> status = "okay";
>> };
>> +
>> +&ipu1_csi1_from_mipi_vc1 {
>> + data-lanes = <0 1>;
>> + clock-lanes = <2>;
>> +};
>> +
>> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> index 795b5a5..bca9fed 100644
>> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> @@ -39,6 +39,8 @@
>> * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> * OTHER DEALINGS IN THE SOFTWARE.
>> */
>> +
>> +#include <dt-bindings/clock/imx6qdl-clock.h>
>> #include <dt-bindings/gpio/gpio.h>
>> #include <dt-bindings/input/input.h>
>>
>> @@ -96,6 +98,15 @@
>> };
>> };
>>
>> + mipi_xclk: mipi_xclk {
>> + compatible = "pwm-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <22000000>;
>> + clock-output-names = "mipi_pwm3";
>> + pwms = <&pwm3 0 45>; /* 1 / 45 ns = 22 MHz */
>> + status = "okay";
>> + };
>> +
>> gpio-keys {
>> compatible = "gpio-keys";
>> pinctrl-names = "default";
>> @@ -220,6 +231,22 @@
>> };
>> };
>>
>> +&ipu1_csi0_from_ipu1_csi0_mux {
>> + bus-width = <8>;
>> + data-shift = <12>; /* Lines 19:12 used */
>> + hsync-active = <1>;
>> + vync-active = <1>;
>> +};
>> +
>> +&ipu1_csi0_mux_from_parallel_sensor {
>> + remote-endpoint = <&ov5642_to_ipu1_csi0_mux>;
>> +};
>> +
>> +&ipu1_csi0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_ipu1_csi0>;
>> +};
>> +
>> &audmux {
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_audmux>;
>> @@ -299,6 +326,52 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_i2c2>;
>> status = "okay";
>> +
>> + ov5640: camera at 40 {
>> + compatible = "ovti,ov5640";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_ov5640>;
>> + clocks = <&mipi_xclk>;
>> + clock-names = "xclk";
>> + reg = <0x40>;
>> + xclk = <22000000>;
> This is superfluous, you can use clk_get_rate on mipi_xclk.
This property is actually there to tell the driver what to set the
rate to, with clk_set_rate(). So you are saying it would be better
to set the rate in the device tree and the driver should only
retrieve the rate?
Steve
^ permalink raw reply
* [PATCH v8 16/18] vfio/type1: Allow transparent MSI IOVA allocation
From: Alex Williamson @ 2017-01-13 23:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484127714-3263-17-git-send-email-eric.auger@redhat.com>
On Wed, 11 Jan 2017 09:41:52 +0000
Eric Auger <eric.auger@redhat.com> wrote:
> When attaching a group to the container, check the group's
> reserved regions and test whether the IOMMU translates MSI
> transactions. If yes, we initialize an IOVA allocator through
> the iommu_get_msi_cookie API. This will allow the MSI IOVAs
> to be transparently allocated on MSI controller's compose().
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> v3 -> v4:
> - test region's type: IOMMU_RESV_MSI
> - restructure the code to prepare for safety assessment
> - reword title
> ---
> drivers/vfio/vfio_iommu_type1.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 9266271..5651faf 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -39,6 +39,7 @@
> #include <linux/pid_namespace.h>
> #include <linux/mdev.h>
> #include <linux/notifier.h>
> +#include <linux/dma-iommu.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -1181,6 +1182,28 @@ static struct vfio_group *find_iommu_group(struct vfio_domain *domain,
> return NULL;
> }
>
> +static bool vfio_iommu_has_resv_msi(struct iommu_group *group,
> + phys_addr_t *base)
> +{
> + struct list_head group_resv_regions;
> + struct iommu_resv_region *region, *next;
> + bool ret = false;
> +
> + INIT_LIST_HEAD(&group_resv_regions);
> + iommu_get_group_resv_regions(group, &group_resv_regions);
> + list_for_each_entry(region, &group_resv_regions, list) {
> + if (region->type & IOMMU_RESV_MSI) {
> + *base = region->start;
> + ret = true;
> + goto out;
> + }
> + }
> +out:
> + list_for_each_entry_safe(region, next, &group_resv_regions, list)
> + kfree(region);
> + return ret;
> +}
> +
> static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct iommu_group *iommu_group)
> {
> @@ -1189,6 +1212,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct vfio_domain *domain, *d;
> struct bus_type *bus = NULL, *mdev_bus;
> int ret;
> + bool resv_msi;
> + phys_addr_t resv_msi_base;
>
> mutex_lock(&iommu->lock);
>
> @@ -1258,6 +1283,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_domain;
>
> + resv_msi = vfio_iommu_has_resv_msi(iommu_group, &resv_msi_base);
> +
> INIT_LIST_HEAD(&domain->group_list);
> list_add(&group->next, &domain->group_list);
>
> @@ -1304,6 +1331,9 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_detach;
>
> + if (resv_msi && iommu_get_msi_cookie(domain->domain, resv_msi_base))
> + goto out_detach;
> +
> list_add(&domain->next, &iommu->domain_list);
>
> mutex_unlock(&iommu->lock);
^ permalink raw reply
* [PATCH v8 17/18] vfio/type1: Check MSI remapping at irq domain level
From: Alex Williamson @ 2017-01-13 23:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484127714-3263-18-git-send-email-eric.auger@redhat.com>
On Wed, 11 Jan 2017 09:41:53 +0000
Eric Auger <eric.auger@redhat.com> wrote:
> In case the IOMMU translates MSI transactions (typical case
> on ARM), we check MSI remapping capability at IRQ domain
> level. Otherwise it is checked at IOMMU level.
>
> At this stage the arm-smmu-(v3) still advertise the
> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> removed in subsequent patches.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> v6: rewrite test
> ---
> drivers/vfio/vfio_iommu_type1.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5651faf..ec903a0 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -40,6 +40,7 @@
> #include <linux/mdev.h>
> #include <linux/notifier.h>
> #include <linux/dma-iommu.h>
> +#include <linux/irqdomain.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -1212,7 +1213,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct vfio_domain *domain, *d;
> struct bus_type *bus = NULL, *mdev_bus;
> int ret;
> - bool resv_msi;
> + bool resv_msi, msi_remap;
> phys_addr_t resv_msi_base;
>
> mutex_lock(&iommu->lock);
> @@ -1288,8 +1289,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> INIT_LIST_HEAD(&domain->group_list);
> list_add(&group->next, &domain->group_list);
>
> - if (!allow_unsafe_interrupts &&
> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> + msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> + iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> +
> + if (!allow_unsafe_interrupts && !msi_remap) {
> pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
> __func__);
> ret = -EPERM;
^ permalink raw reply
* [PATCH v3 15/24] media: Add userspace header file for i.MX
From: Steve Longerbeam @ 2017-01-13 23:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484309143.31475.31.camel@pengutronix.de>
On 01/13/2017 04:05 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> This adds a header file for use by userspace programs wanting to interact
>> with the i.MX media driver. It defines custom v4l2 controls and events
>> generated by the i.MX v4l2 subdevices.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> include/uapi/media/Kbuild | 1 +
>> include/uapi/media/imx.h | 30 ++++++++++++++++++++++++++++++
>> 2 files changed, 31 insertions(+)
>> create mode 100644 include/uapi/media/imx.h
>>
>> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
>> index aafaa5a..fa78958 100644
>> --- a/include/uapi/media/Kbuild
>> +++ b/include/uapi/media/Kbuild
>> @@ -1 +1,2 @@
>> # UAPI Header export list
>> +header-y += imx.h
>> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
>> new file mode 100644
>> index 0000000..2421d9c
>> --- /dev/null
>> +++ b/include/uapi/media/imx.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the
>> + * License, or (at your option) any later version
>> + */
>> +
>> +#ifndef __UAPI_MEDIA_IMX_H__
>> +#define __UAPI_MEDIA_IMX_H__
>> +
>> +/*
>> + * events from the subdevs
>> + */
>> +#define V4L2_EVENT_IMX_CLASS V4L2_EVENT_PRIVATE_START
>> +#define V4L2_EVENT_IMX_NFB4EOF (V4L2_EVENT_IMX_CLASS + 1)
>> +#define V4L2_EVENT_IMX_EOF_TIMEOUT (V4L2_EVENT_IMX_CLASS + 2)
>> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
> Aren't these generic enough to warrant common events? I would think
> there have to be other capture IP cores that can signal aborted frames
> or frame timeouts.
Yes, agreed. A frame capture timeout, or frame interval error, are
both generic concepts. At some point it would be great to make the
Frame Interval Monitor generally available under v4l2-core. As for the
EOF timeout event, I'll look into moving that into a generic V4L2 event.
Steve
^ permalink raw reply
* [PATCH] clk: imx6: don't restrict LDB mux changes on QuadPlus
From: Fabio Estevam @ 2017-01-13 23:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113173943.23677-1-l.stach@pengutronix.de>
On Fri, Jan 13, 2017 at 3:39 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> The LDB mux/gate layout has been fixed on QuadPlus, so there is no need
> to restrict the LDB mux changes on this hardware, as the erratum
> preventing this from working properly is gone.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
That's correct: mx6qp does not suffer from the LDB clock glitch issue:
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Martin Blumenstingl @ 2017-01-13 23:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <055cc8fa-46c3-fab2-2d9e-be4e5416fc5a@gmail.com>
Hi Heiner,
On Fri, Jan 13, 2017 at 9:26 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
[snip]
>>>> +static int meson_saradc_read_raw_sample(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan,
>>>> + int *val)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
>>>> +
>>>> + ret = meson_saradc_wait_busy_clear(indio_dev);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> The resulting regval value isn't used, therefore this statement doesn't seem
>>> to be needed.
>> I can probably replace this with "0", good catch!
>>
>>> In the vendor driver there is such a dummy statement before reading the busy
>>> flags in REG0 after starting sampling. Reason seems to be a potential race
>>> when we try to read the busy flags before the sampling engine has set them.
>>> This isn't needed in meson_saradc_wait_busy_clear here as an udelay(1) is
>>> done first always.
>> do you think it's worth adding a comment here that a do ... while loop
>> is there on purpose?
>>
> Yes, a hint would be good that there's a potential race.
> Else there's a good chance that a future refactoring introduces a regression.
indeed, a comment will be part of v2
>>>> +
>>>> + while (meson_saradc_get_fifo_count(indio_dev) > 0 &&
>>> IMHO this loop isn't needed. When we come here the FIFO contains exactly
>>> one element. This is true also in averaging mode as the averaging engine
>>> writes only the resulting mean value to the FIFO.
>>>
>>> And we can't have multiple samples active in parallel due to the locking
>>> done in meson_saradc_get_sample.
>>>
>>> By the way: I use an IRQ here to wake up when the FIFO contains one
>>> element. But as you wrote: It's not clear whether this works on all
>>> Meson systems.
>> maybe I should switch to IRQ mode as even the old Meson6 vendor kernel
>> sources indicate that the SAR ADC has IRQ support?
>>
> As you like. We can also add interrupt mode later (but leave polling intact)
> and activate it only if an interrupt is set in DT.
> This way we'd have a fallback in case there should be a problem with
> interrupt mode on some system.
I'm fine with either way, my idea was to keep it simple for the start.
as a side-note: even if we get rid of polling mode we still need
something like meson_saradc_wait_busy_clear(), because it's required
to stop sampling (at least in some cases) according to the datasheet:
"To stop sampling, simply set This bit and wait for all processing
modules to no longer indicate that they are busy."
>>>> + count < SAR_ADC_MAX_FIFO_SIZE) {
>>>> + regmap_read(priv->regmap, SAR_ADC_FIFO_RD, ®val);
>>>> +
>>>> + fifo_chan = FIELD_GET(SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>>>> + if (fifo_chan == chan->channel) {
>>>> + fifo_val = FIELD_GET(SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK,
>>>> + regval) & SAR_ADC_VALUE_MASK(priv);
>>>> + sum += fifo_val;
>>>> + count++;
>>>> + }
>>>> + }
>>>> +
>>>> + if (!count)
>>>> + return -ENOENT;
>>>> +
>>>> + *val = sum / count;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void meson_saradc_set_averaging(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan,
>>>> + enum meson_saradc_avg_mode mode,
>>>> + enum meson_saradc_num_samples samples)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + u32 val;
>>>> +
>>>> + val = samples << SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>>> + SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(chan->channel),
>>>> + val);
>>>> +
>>>> + val = mode << SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>>> + SAR_ADC_AVG_CNTL_AVG_MODE_MASK(chan->channel), val);
>>>> +}
>>>> +
>>>> +static void meson_saradc_enable_channel(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + u32 regval;
>>>> +
>>>> + /* the SAR ADC engine allows sampling multiple channels at the same
>>>> + * time. to keep it simple we're only working with one *internal*
>>>> + * channel, which starts counting at index 0 (which means: count = 1).
>>>> + */
>>>> + regval = FIELD_PREP(SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, 0);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>>> + SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, regval);
>>>> +
>>>> + /* map channel index 0 to the channel which we want to read */
>>>> + regval = FIELD_PREP(SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>>> + SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), regval);
>>>> +
>>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>>> + chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>>> + SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>>> + regval);
>>>> +
>>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>>> + chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>>> + SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>>> + regval);
>>>> +
>>>> + if (chan->channel == 6)
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELTA_10,
>>>> + SAR_ADC_DELTA_10_TEMP_SEL, 0);
>>>> +}
>>>> +
>>>> +static void meson_saradc_set_channel7_mux(struct iio_dev *indio_dev,
>>>> + enum meson_saradc_chan7_mux_sel sel)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + u32 regval;
>>>> +
>>>> + regval = FIELD_PREP(SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>>> + SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
>>>> +
>>>> + usleep_range(10, 20);
>>>> +}
>>>> +
>>>> +static void meson_saradc_start_sample_engine(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
>>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLING_START,
>>>> + SAR_ADC_REG0_SAMPLING_START);
>>>> +}
>>>> +
>>>> +static void meson_saradc_stop_sample_engine(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLING_STOP,
>>>> + SAR_ADC_REG0_SAMPLING_STOP);
>>>> +
>>>> + /* wait until all modules are stopped */
>>>> + meson_saradc_wait_busy_clear(indio_dev);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE, 0);
>>>> +}
>>>> +
>>>> +static void meson_saradc_lock(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + int val;
>>>> +
>>>> + mutex_lock(&indio_dev->mlock);
>>>> +
>>>> + /* prevent BL30 from using the SAR ADC while we are using it */
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>>> + SAR_ADC_DELAY_KERNEL_BUSY,
>>>> + SAR_ADC_DELAY_KERNEL_BUSY);
>>>> +
>>>> + /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
>>>> + do {
>>>> + udelay(1);
>>>> + regmap_read(priv->regmap, SAR_ADC_DELAY, &val);
>>>> + } while (val & SAR_ADC_DELAY_BL30_BUSY);
>>>> +}
>>>> +
>>>> +static void meson_saradc_unlock(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> +
>>>> + /* allow BL30 to use the SAR ADC again */
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>>> + SAR_ADC_DELAY_KERNEL_BUSY, 0);
>>>> +
>>>> + mutex_unlock(&indio_dev->mlock);
>>>> +}
>>>> +
>>>> +static int meson_saradc_get_sample(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan,
>>>> + enum meson_saradc_avg_mode avg_mode,
>>>> + enum meson_saradc_num_samples avg_samples,
>>>> + int *val)
>>>> +{
>>>> + int ret, tmp;
>>>> +
>>>> + meson_saradc_lock(indio_dev);
>>>> +
>>>> + /* clear old values from the FIFO buffer, ignoring errors */
>>>> + meson_saradc_read_raw_sample(indio_dev, chan, &tmp);
>>>> +
>>>> + meson_saradc_set_averaging(indio_dev, chan, avg_mode, avg_samples);
>>>> +
>>>> + meson_saradc_enable_channel(indio_dev, chan);
>>>> +
>>>> + meson_saradc_start_sample_engine(indio_dev);
>>>> + ret = meson_saradc_read_raw_sample(indio_dev, chan, val);
>>>> + meson_saradc_stop_sample_engine(indio_dev);
>>>> +
>>>> + meson_saradc_unlock(indio_dev);
>>>> +
>>>> + if (ret) {
>>>> + dev_warn(&indio_dev->dev,
>>> Using the struct device in indio_dev results in IMHO ugly messages like
>>> iio iio:device0: already initialized by BL30
>>>
>>> We should use the parent instead, this is more readable:
>>> meson-saradc c1108680.adc: already initialized by BL30
>>>
>>> For this we need to move the assignment to indio_dev->dev.parent
>>> in probe, else messages may be written when parent isn't set yet.
>> indeed, I'll change this - thanks for the hint!
a little correction on this: I am all for setting the parent early.
However, I think that devm_iio_device_alloc() itself should take care
of building the device name correctly (to prevent code-duplication
across drivers in drivers/iio/*).
as an example: phy_create() does this (which includes the OF node
name): dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id);
iio_device_alloc does it like this (which is obviously missing the OF
node name): dev_set_name(&dev->dev, "iio:device%d", dev->id);
Regards,
Martin
^ permalink raw reply
* [PATCH 4/4] ARM64: dts: meson: meson-gx: add the SAR ADC
From: Martin Blumenstingl @ 2017-01-13 23:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <85c07462-c175-faf8-8dae-403a6291331e@gmail.com>
On Fri, Jan 13, 2017 at 9:14 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> Am 13.01.2017 um 20:50 schrieb Martin Blumenstingl:
>> Hi Heiner,
>>
>> On Fri, Jan 13, 2017 at 8:32 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>> Sorry, I'm not subscribed to the two mailing lists, therefore my reply
>>> is outside the thread.
>>>
>>> I'm currently experimenting with an own rudimentary driver for SAR ADC
>>> on a Odroid C2 (S905GXBB). So I have some remarks based on my experience.
>> I hope that we haven't been duplicating too much work!
>>
> No, my driver doesn't include all the clock handling and relies on the
> boot loader / firmware to do this. Also I don't support averaging mode.
> Your driver is much more comprehensive and I would go with it.
>
> I have some features like interrupt mode and calibration which are not
> yet supported in your driver but they can be easily migrated and added
> later.
indeed, that'll leave some fun for you as well :-)
^ permalink raw reply
* [PATCH] ARM: BCM5301X: Set 5 GHz wireless frequency limits on Netgear R8000
From: Rafał Miłecki @ 2017-01-13 23:58 UTC (permalink / raw)
To: linux-arm-kernel
From: Rafa? Mi?ecki <rafal@milecki.pl>
Netgear R8000 is a tri-band home router. It has three BCM43602 chipsets
two of them for 5 GHz band. Both seem the same and their firmwares
report the same set of channels. The problem is due to hardware / board
design there are extra limitations that should be respected.
First PHY should be used for U-NII-2 and U-NII-3. Third PHY should be
used for U-NII-1. Using them in a different way may result in wireless
not working or in noticeably reduced performance. Basic version of this
info was provided by Broadcom employee, then it has been verified by me
using original vendor firmware (which has limitations hardcoded in UI).
This patch uses recently introduced ieee80211-freq-limit property to
describe these limitations at DT level.
Referencing PCIe devices in DT required specifying all related bridges.
Below you can see (a bit complex) PCI tree from R8000 that explains all
entries that I needed to put in DT.
0000:00:00.0 14e4:8012 Bridge Device
?? 0000:01:00.0 14e4:aa52 Network Controller
0001:00:00.0 14e4:8012 Bridge Device
?? 0001:01:00.0 10b5:8603 Bridge Device
?? 0001:02:01.0 10b5:8603 Bridge Device
? ?? 0001:03:00.0 14e4:aa52 Network Controller
?? 0001:02:02.0 10b5:8603 Bridge Device
? ?? 0001:04:00.0 14e4:aa52 Network Controller
?? 0001:02:03.0 000d:0000 0x000000
?? 0001:02:04.0 000d:0000 0x000000
?? 0001:02:05.0 000d:0000 0x000000
?? 0001:02:06.0 000d:0000 0x000000
?? (...)
?? 0001:02:1d.0 000d:0000 0x000000
?? 0001:02:1e.0 000d:0000 0x000000
?? 0001:02:1f.0 000d:0000 0x000000
Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
arch/arm/boot/dts/bcm4709-netgear-r8000.dts | 48 +++++++++++++++++++++++++++++
arch/arm/boot/dts/bcm5301x.dtsi | 8 +++++
2 files changed, 56 insertions(+)
diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
index cd13534..8e39a84 100644
--- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
@@ -108,6 +108,54 @@
};
};
+&pcie0 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 0,0,0 {
+ reg = <0x0000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ wifi at 0,1,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-freq-limit = <5735000 5835000>;
+ };
+ };
+};
+
+&pcie1 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 1,0,0 {
+ reg = <0x0000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 1,1,0 {
+ reg = <0x0000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 1,2,2 {
+ reg = <0x1000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ wifi at 1,4,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-freq-limit = <5170000 5730000>;
+ };
+ };
+ };
+ };
+};
+
&usb2 {
vcc-gpio = <&chipcommon 0 GPIO_ACTIVE_HIGH>;
};
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index a4614c9..4fbb089 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -243,6 +243,14 @@
#gpio-cells = <2>;
};
+ pcie0: pcie at 12000 {
+ reg = <0x00012000 0x1000>;
+ };
+
+ pcie1: pcie at 13000 {
+ reg = <0x00013000 0x1000>;
+ };
+
usb2: usb2 at 21000 {
reg = <0x00021000 0x1000>;
--
2.10.1
^ permalink raw reply related
* [PATCH v7 00/15] ACPI platform MSI support and its example mbigen
From: Hanjun Guo @ 2017-01-14 1:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACVXFVO8n=Y+eN0Z_zMZJsirf+VNSr-y9g3uwwxQZzAxds=m4g@mail.gmail.com>
Hi Ming,
On 2017/1/13 18:23, Ming Lei wrote:
> On Wed, Jan 11, 2017 at 11:06 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>> With platform msi support landed in the kernel, and the introduction
>> of IORT for GICv3 ITS (PCI MSI) and SMMU, the framework for platform msi
>> is ready, this patch set add few patches to enable the ACPI platform
>> msi support.
>>
>> For platform device connecting to ITS on arm platform, we have IORT
>> table with the named componant node to describe the mappings of paltform
>> device and ITS, so we can retrieve the dev id and find its parent
>> irqdomain (ITS) from IORT table (simlar with the ACPI ITS support).
>>
>> v6 -> v7:
>> - Introduce iort_node_map_platform_id() to retrieve the
>> dev id for both NC (named component) -> ITS/SMMU and
>> NC -> SMMU -> ITS cases, suggested by Lorenzo;
>>
>> - Reorder the patches and rewrite some commit message;
>>
>> - Remove the test tags because it has major changes
>> to retrieve the dev id, Sinan, Majun, Xinwei, could
>> you please test them again on your platform?
>>
>> - rebased on top of 4.10-rc3 and Lorenzo's patch
>> https://patchwork.kernel.org/patch/9507041/
>>
>> - Tested against Agustin's patch [1-2/3] "[PATCH V9 0/3] irqchip: qcom:
>> Add IRQ combiner driver"
> Looks v7 works fine on D05, together with Lorenzo's
> fix([v2] ACPI/IORT: Fix iort_node_get_id() mapping entries indexing) and
> Agustin's patchset of "v9 irqchip: qcom: Add IRQ combiner driver".
>
> Tested-by: Ming Lei <ming.lei@canonical.com>
Thank you very much :)
Hanjun
^ permalink raw reply
* [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
From: Rob Herring @ 2017-01-14 2:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <84d7d49b-933b-8b26-f18a-3a5054738cb1@ti.com>
On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>
>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>
>>> Rob,
>>>
>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>
>>>>
>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>
>>>>>
>>>>> Rob,
>>>>>
>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>> control device power states.
>>>>>>>
>>>>>>> Also, provide macros representing each device index as understood
>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>
>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>> ---
>>>>>>> v2->v3:
>>>>>>> Update k2g_pds node docs to show it should be a child of pmmc
>>>>>>> node.
>>>>>>> In early versions a phandle was used to point to pmmc and
>>>>>>> docs
>>>>>>> still
>>>>>>> incorrectly showed this.
>>>>>>>
>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59
>>>>>>> ++++++++++++++
>>>>>>> MAINTAINERS | 2 +
>>>>>>> include/dt-bindings/genpd/k2g.h | 90
>>>>>>> ++++++++++++++++++++++
>>>>>>> 3 files changed, 151 insertions(+)
>>>>>>> create mode 100644
>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..4c9064e512cb
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> @@ -0,0 +1,59 @@
>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>> +---------------------------------------------
>>>>>>> +
>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>> that
>>>>>>> is
>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>> +Communication between the host processor running an OS and the
>>>>>>> system
>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>> domain
>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>> use
>>>>>>> of
>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>> +
>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>> +
>>>>>>> +PM Domain Node
>>>>>>> +==============
>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>> PMMC,
>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>> generic
>>>>>>> +PM domain bindings in
>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>>>> PMMC
>>>>>>> it
>>>>>>> +must be a child of the pmmc node.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>> +
>>>>>>> +Example (K2G):
>>>>>>> +-------------
>>>>>>> + pmmc: pmmc {
>>>>>>> + compatible = "ti,k2g-sci";
>>>>>>> + ...
>>>>>>> +
>>>>>>> + k2g_pds: k2g_pds {
>>>>>>> + compatible = "ti,sci-pm-domain";
>>>>>>> + #power-domain-cells = <0>;
>>>>>>> + };
>>>>>>> + };
>>>>>>> +
>>>>>>> +PM Domain Consumers
>>>>>>> +===================
>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>> provide
>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>> device
>>>>>>> +specific ID that identifies the device.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>> node.
>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>> to
>>>>>>> + be used for device control.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> As I've already stated before, this goes in power-domain cells. When
>>>>>> you
>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>> you
>>>>>> you need to specify the ID for each of them in phandle args. This is
>>>>>> how
>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> You think the reasoning for doing it this way provided by both Ulf and
>>>>> myself on v2 [1] is not valid then?
>>>>>
>>>>> From Ulf:
>>>>>
>>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>> in DT and assign to a device node. The only difference here, is that
>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>
>>>>
>>>>
>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>> do with DT bindings.
>>>>
>>>>> From me:
>>>>>
>>>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>>>> list
>>>>> of genpds but rather identifies the device *within* a single genpd. It
>>>>> is
>>>>> a
>>>>> property specific to each device that resides in a ti-sci-genpd, not a
>>>>> mapping describing which genpd the device belongs to. The generic power
>>>>> domain binding is concerned with mapping the device to a specific
>>>>> genpd,
>>>>> which is does fine for us, but we have a sub mapping for devices that
>>>>> exist
>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>
>>>>>
>>>>> So to summarize, the genpd framework does interpret the phandle arg as
>>>>> an
>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>> but
>>>>> this is not what I am trying to do, we have multiple devices within
>>>>> this
>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>
>>>>
>>>>
>>>> Fix the genpd framework rather than work around it in DT.
>>>
>>>
>>>
>>> I still disagree that this has nothing to do with DT bindings, as the
>>> current DT binding represents something different already. I am trying to
>>> extend it to give me additional information needed for our platforms. Are
>>> you saying that we should break what the current DT binding already
>>> represents to mean something else?
>>
>>
>> No idea because what's the current binding? From the patch, looks like
>> a new binding to me.
>
>
> Yes, ti,sci-id is a new binding. I am referring to the current meaning of
> the "power-domains" binding, which is where you are asking this property to
> be added, in "power-domains" cells. This is documented here [1] in the
> kernel, although looking at it I must admit it is not very clear.
>
> The power-domains cell represents an offset into an array of power domains,
> if you choose to use it. That's what the genpd framework is hard coded to
> interpret it as. This is correct, as it is an index into a static list of
> power domains, used to identify which power domain a device belongs to,
> which is exactly what the genpd framework itself is concerned with. This is
> already how it is used in the kernel today.
Strictly speaking, the cells are purely for the interpretation of the
phandle they are associated with. If some controller wants to have 20
cells, then it could assuming a good reason. The reality is we tend to
align the meaning of the cells. If genpd is interpreting the cells and
not letting the driver for the power domain controller interpret them,
then still, genpd needs to be fixed.
IIRC, initially it was said genpd required 0 cells, hence my confusion.
> My ti,sci-id is not an index into a list of power domains, so it should not
> go in the power-domains cells and go against what the power-domains binding
> says that the cell expects. We have one single power domain, and the new
> ti,sci-id binding is not something the genpd framework itself is concerned
> with as it's our property to identify a device inside a power domain, not to
> identify which power domain it is associated with.
What is the id used for? I can understand why you need to know what
power domain a device is in (as power-domains identifies), but not
what devices are in a power domain.
Rob
^ permalink raw reply
* [PATCH v2 1/2] of: base: add support to find the level of the last cache
From: Rob Herring @ 2017-01-14 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484245772-31511-1-git-send-email-sudeep.holla@arm.com>
On Thu, Jan 12, 2017 at 12:29 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> It is useful to have helper function just to get the number of cache
> levels for a given logical cpu. We can obtain the same by just checking
> the level at which the last cache is present. This patch adds support
> to find the level of the last cache for a given cpu.
>
> It will be used on ARM64 platform where the device tree provides the
> information for the additional non-architected/transparent/external
> last level caches that are not integrated with the processors.
>
> Suggested-by: Rob Herring <robh+dt@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/of/base.c | 27 +++++++++++++++++++++++++++
> include/linux/of.h | 1 +
> 2 files changed, 28 insertions(+)
>
> v1->v2:
> - Moved to using "cache-level" in the last level cache instead
> of counting through all the nodes as suggested by Rob
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d4bea3c797d6..c1128a077aea 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -25,6 +25,7 @@
> #include <linux/cpu.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/of_graph.h>
> #include <linux/spinlock.h>
> #include <linux/slab.h>
> @@ -2268,6 +2269,32 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
> }
>
> /**
> + * of_find_last_cache_level - Find the level at which the last cache is
> + * present for the given logical cpu
> + *
> + * @cpu: cpu number(logical index) for which the last cache level is needed
> + *
> + * Returns the the level at which the last cache is present. It is exactly
> + * same as the total number of cache levels for the given logical cpu.
> + */
> +int of_find_last_cache_level(unsigned int cpu)
> +{
> + int cache_level = 0;
> + struct device_node *prev = NULL, *np = of_cpu_device_node_get(cpu);
> +
> + while (np) {
> + prev = np;
> + of_node_put(np);
> + np = of_find_next_cache_node(np);
> + }
> +
> + if (prev)
Probably don't need this check. Otherwise,
Acked-by: Rob Herring <robh@kernel.org>
> + of_property_read_u32(prev, "cache-level", &cache_level);
> +
> + return cache_level;
> +}
> +
> +/**
> * of_graph_parse_endpoint() - parse common endpoint node properties
> * @node: pointer to endpoint device_node
> * @endpoint: pointer to the OF endpoint data structure
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d72f01009297..21e6323de0f3 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -280,6 +280,7 @@ extern struct device_node *of_get_child_by_name(const struct device_node *node,
>
> /* cache lookup */
> extern struct device_node *of_find_next_cache_node(const struct device_node *);
> +extern int of_find_last_cache_level(unsigned int cpu);
> extern struct device_node *of_find_node_with_property(
> struct device_node *from, const char *prop_name);
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v7 15/15] irqchip: mbigen: Add ACPI support
From: Hanjun Guo @ 2017-01-14 2:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113102104.GB20837@red-moon>
Hi Lorenzo,
On 2017/1/13 18:21, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:39PM +0800, Hanjun Guo wrote:
>> With the preparation of platform msi support and interrupt producer
>> in DSDT, we can add mbigen ACPI support now.
>>
>> We are using _PRS methd to indicate number of irq pins instead
>> of num_pins in DT to avoid _DSD usage in this case.
>>
>> For mbi-gen,
>> Device(MBI0) {
>> Name(_HID, "HISI0152")
>> Name(_UID, Zero)
>> Name(_CRS, ResourceTemplate() {
>> Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
>> })
>>
>> Name (_PRS, ResourceTemplate() {
>> Interrupt(ResourceProducer,...) {12,14,....}
> I still do not understand why you are using _PRS for this, I think
> the MBIgen configuration is static and if it is so the Interrupt
> resource should be part of the _CRS unless there is something I am
> missing here.
Sorry for not clear in the commit message. MBIgen is an interrupt producer
which produces irq resource to devices connecting to it, and MBIgen itself
don't consume wired interrupts.
Also devices connecting MBIgen may not consume all the interrupts produced
by MBIgen, for example, MBIgen may produce 128 interrupts but only half of
them are currently used, so _PRS here means "provide interrupt resources
may consumed by devices connecting to it".
Should I add this into the commit message?
Thanks
Hanjun
^ permalink raw reply
* [PATCH v7 12/15] msi: platform: make platform_msi_create_device_domain() ACPI aware
From: Hanjun Guo @ 2017-01-14 3:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113104524.GC20837@red-moon>
On 2017/1/13 18:45, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:36PM +0800, Hanjun Guo wrote:
>> platform_msi_create_device_domain() is used to ctreate
>> irqdomain for the device such as irqchip mbigen generating
>> the MSIs, it's almost ready for ACPI use except
>> of_node_to_fwnode() is for dt only, make it ACPI aware then
>> things will work in both DTS and ACPI.
> "The irqdomain creation carried out in:
>
> platform_msi_create_device_domain()
>
> relies on the fwnode_handle interrupt controller token to associate the
> interrupt controller with a specific irqdomain. Current code relies on
> the OF layer to retrieve a fwnode_handle for the device representing the
> interrupt controller from its device->of_node pointer. This makes
> platform_msi_create_device_domain() DT specific whilst it really is not
> because after the merge of commit f94277af03ea ("of/platform: Initialise
> dev->fwnode appropriately") the fwnode_handle can easily be retrieved
> from the dev->fwnode pointer in a firmware agnostic way.
>
> Update platform_msi_create_device_domain() to retrieve the interrupt
> controller fwnode_handle from the dev->fwnode pointer so that it can
> be used seamlessly in ACPI and DT systems".
Much better, I will update the patch.
>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Thanks
Hanjun
^ permalink raw reply
* [PATCH v7 08/15] ACPI: IORT: rename iort_node_map_rid() to make it generic
From: Hanjun Guo @ 2017-01-14 3:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113114747.GD20837@red-moon>
On 2017/1/13 19:47, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:32PM +0800, Hanjun Guo wrote:
>> iort_node_map_rid() was designed for both PCI and platform
>> device, but the rid means requester id is for ITS mappings,
> I do not understand what this means sorry.
>
>> rename iort_node_map_rid() to iort_node_map_id() and update
>> its argument names to make it more generic.
>>
> "iort_node_map_rid() was designed to take an input id (that is not
> necessarily a PCI requester id) and map it to an output id (eg an SMMU
> streamid or an ITS deviceid) according to the mappings provided by an
> IORT node mapping entries. This means that the iort_node_map_rid() input
> id is not always a PCI requester id as its name, parameters and local
> variables suggest, which is misleading.
>
> Apply the s/rid/id substitution to the iort_node_map_rid() mapping
> function and its users to make sure its intended usage is clearer."
Thank your patience, I will update the commit message.
Hanjun
^ permalink raw reply
* [PATCH v7 09/15] ACPI: platform-msi: retrieve dev id from IORT
From: Hanjun Guo @ 2017-01-14 4:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113121144.GE20837@red-moon>
Hi Lorenzo,
On 2017/1/13 20:11, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:33PM +0800, Hanjun Guo wrote:
>> For devices connecting to ITS, it needs dev id to identify itself, and
>> this dev id is represented in the IORT table in named component node
>> [1] for platform devices, so in this patch we will scan the IORT to
>> retrieve device's dev id.
>>
>> For named components we know that there are always two steps
>> involved (second optional):
>>
>> (1) Retrieve the initial id (this may well provide the final mapping)
>> (2) Map the id (optional if (1) represents the map type we need), this
>> is needed for use cases such as NC (named component) -> SMMU -> ITS
>> mappings.
>>
>> we have API iort_node_get_id() for step (1) above and
>> iort_node_map_rid() for step (2), so create a wrapper
>> iort_node_map_platform_id() to retrieve the dev id.
>>
>> [1]: https://static.docs.arm.com/den0049/b/DEN0049B_IO_Remapping_Table.pdf
> This patch should be split and IORT changes should be squashed with
> patch 10.
If split the changes for IORT and its platform msi, API introduced in IORT will
not be used in a single patch, seems violate the suggestion of "new introduced API
needs to be used in the same patch", did I miss something?
>
>> Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Suggested-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Sinan Kaya <okaya@codeaurora.org>
>> Cc: Tomasz Nowicki <tn@semihalf.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> drivers/acpi/arm64/iort.c | 56 +++++++++++++++++++++++++++
>> drivers/irqchip/irq-gic-v3-its-platform-msi.c | 4 +-
>> include/linux/acpi_iort.h | 8 ++++
>> 3 files changed, 67 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 069a690..95fd20b 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -30,6 +30,7 @@
>> #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
>> #define IORT_IOMMU_TYPE ((1 << ACPI_IORT_NODE_SMMU) | \
>> (1 << ACPI_IORT_NODE_SMMU_V3))
>> +#define IORT_TYPE_ANY (IORT_MSI_TYPE | IORT_IOMMU_TYPE)
>>
>> struct iort_its_msi_chip {
>> struct list_head list;
>> @@ -406,6 +407,34 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>> return NULL;
>> }
>>
>> +static
>> +struct acpi_iort_node *iort_node_map_platform_id(struct acpi_iort_node *node,
>> + u32 *id_out, u8 type_mask,
>> + int index)
>> +{
>> + struct acpi_iort_node *parent;
>> + u32 id;
>> +
>> + /* step 1: retrieve the initial dev id */
>> + parent = iort_node_get_id(node, &id, IORT_TYPE_ANY, index);
>> + if (!parent)
>> + return NULL;
>> +
>> + /*
>> + * optional step 2: map the initial dev id if its parent is not
>> + * the target type we wanted, map it again for the use cases such
>> + * as NC (named component) -> SMMU -> ITS. If the type is matched,
>> + * return the parent pointer directly.
>> + */
>> + if (!(IORT_TYPE_MASK(parent->type) & type_mask))
>> + parent = iort_node_map_id(parent, id, id_out, type_mask);
>> + else
>> + if (id_out)
> Remove this pointer check.
This was added because of NULL pointer reference, I passed NULL for id_out because I
only want to get its parent node, I think we have four options:
- Introduce a new API to get the parent only from the scratch, but it will duplicate the code
a lot;
- Don't check the id_out in iort_node_map_platform_id(), and introduce a wrapper and pass the
dummy id for iort_node_map_platform_id() :
static
struct acpi_iort_node *iort_node_get_platform_parent{struct device *dev, u8 type_mask}
{
struct acpi_iort_node *node, *parent = NULL;
int i;
u32 dummy_id;
node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
iort_match_node_callback, dev);
if (!node)
return NULL;
for (i = 0; i < node->mapping_count; i++) {
/* we just want to get the parent node */
parent = iort_node_map_platform_id(node, &dummy_id,
IORT_MSI_TYPE, i);
if (parent)
break;
}
return parent;
}
- Similar solution as above but don't introduce wrapper, just use dummy_id if
iort_node_map_platform_id() is called;
- Use the solution I proposed in this patch.
Please share you suggestion on this :)
>
>> + *id_out = id;
>> +
>> + return parent;
>> +}
>> +
>> static struct acpi_iort_node *iort_find_dev_node(struct device *dev)
>> {
>> struct pci_bus *pbus;
>> @@ -444,6 +473,33 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
>> }
>>
>> /**
>> + * iort_pmsi_get_dev_id() - Get the device id for a device
>> + * @dev: The device for which the mapping is to be done.
>> + * @dev_id: The device ID found.
>> + *
>> + * Returns: 0 for successful find a dev id, errors otherwise
> Nit: -ENODEV on error
>
>> + */
>> +int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>> +{
>> + int i;
>> + struct acpi_iort_node *node;
>> +
>> + if (!iort_table)
>> + return -ENODEV;
> I do not think this iort_table check is needed.
Agreed, it will be checked in iort_scan_node() and it's called
in iort_find_dev_node().
>
>> + node = iort_find_dev_node(dev);
>> + if (!node)
>> + return -ENODEV;
>> +
>> + for (i = 0; i < node->mapping_count; i++) {
>> + if(iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
> ^
>
> Nit: Missing a space.
I was on a flight when updating the patches, seems it's not a good place for coding :)
I will update the patch set when you are ok with the solutions I proposed, thank you
very much for the review.
Hanjun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox