* [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Lars-Peter Clausen @ 2017-01-15 12:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170111174334.24343-4-martin.blumenstingl@googlemail.com>
On 01/11/2017 06:43 PM, Martin Blumenstingl wrote:
> + indio_dev->name = dev_name(&pdev->dev);
The name is supposed to be the type of the device, e.g. part name, not the
name of parent device instance. E.g. meson-gxbb-saradc or meson-gxl-saradc
in this case.
^ permalink raw reply
* [PATCH] arm64: defconfig: disable CONFIG_DEVMEM
From: Leif Lindholm @ 2017-01-15 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113114801.GD3253@arm.com>
On Fri, Jan 13, 2017 at 11:48:01AM +0000, Will Deacon wrote:
> On Fri, Jan 13, 2017 at 11:37:34AM +0000, Leif Lindholm wrote:
> > /dev/mem is the opposite of what an operating system is for.
> > Additionally, on arm* it opens up for denial-of-service attacks from
> > userspace. So leave it disabled by default, requiring people who need
> > it to enable it explicitly.
>
> I really like the idea, but are we sure that nothing common breaks without
> this? For example, does Debian still boot nicely with this patch applied?
Getting distros to not have to shop around for config fragments in
order to be able to a stable system is one of my main purposes of this
patch.
Since Debian just published a 4.9 kernel, I gave that a spin (both DT
and ACPI). No issues.
> Just trying to get a feel for how much userspace this has seen (particularly
> on ACPI-based systems, which I seem to remember like poking about in here).
There are various tools that let you figure out various things about
the system (a bit like running dtc on a dump of the active
device-tree), but nothing actually used for booting a system (much
like dtc).
/dev/mem has been used for things like dmidecode and acpidump in the
past, but we fixed those tools back in 2015 to use /sys interfaces
instead of blindly groping around and hoping for the best.
Stretch version of dmidecode operates as expected on both DT and ACPI
boot, and acpidump does in the ACPI case (no tables in /sys otherwise).
There may be other tools that will also break if not implementing
support for access via /sys, but none critical for system operation
(and currently a denial-of-service waiting to happen anyway).
Systemd will try to access /dev/mem to extract boot timestamp stuff,
but handles a failure gracefully (i.e. not even a warning message).
On a side note, comparing the resulting configs, there is a
semi-broken config dependency in lib/Kconfig.debug, meaning
CONFIG_*STRICT_DEVMEM get set even if CONFIG_DEVMEM is not.
But I'll send that out as a separate patch.
/
Leif
^ permalink raw reply
* [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Jonathan Cameron @ 2017-01-15 13:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFBinCB75JG+F-ZgeO4+q-S9AvrYKYn6AVEdgaDa1yv=cE=Ujw@mail.gmail.com>
On 14/01/17 17:44, Martin Blumenstingl wrote:
> Hi Jonathan,
>
> thank you for the review!
> (further comments from me inline)
> I think I'll send an updated version on Monday.
>
> On Sat, Jan 14, 2017 at 3:46 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 11/01/17 17:43, Martin Blumenstingl wrote:
>>> This adds support for the SAR (Successive Approximation Register) ADC
>>> on the Amlogic Meson SoCs.
>>>
>>> The code is based on the public S805 (Meson8b) and S905 (GXBB)
>>> datasheets, as well as by reading (various versions of) the vendor
>>> driver and by inspecting the registers on the vendor kernels of my
>>> testing-hardware.
>>>
>>> Currently the GXBB, GXL and GXM SoCs are supported. GXBB hardware has
>>> 10-bit ADC resolution, while GXL and GXM have 12-bit ADC resolution.
>>> The code was written to support older SoCs (Meson8 and Meson8b) as well,
>>> but due to lack of actual testing-hardware no of_device_id was added for
>>> these.
>>>
>>> Two "features" from the vendor driver are currently missing:
>>> - the vendor driver uses channel #7 for calibration (this improves the
>>> accuracy of the results - in my tests the results were less than 3%
>>> off without calibration compared to the vendor driver). Adding support
>>> for this should be easy, but is not required for most applications.
>>> - channel #6 is connected to the SoCs internal temperature sensor.
>>> Adding support for this is probably not so easy since (based on the
>>> u-boot sources) most SoC versions are using different registers and
>>> algorithms for the conversion from "ADC value" to temperature.
>>>
>>> Supported by the hardware but currently not supported by the driver:
>>> - reading multiple channels at the same time (the hardware has a FIFO
>>> buffer which stores multiple results)
>>> - continuous sampling (this would require a way to enable this
>>> individually because otherwise the ADC would be drawing power
>>> constantly)
>>> - interrupt support (similar to the vendor driver this new driver is
>>> polling the results. It is unclear if the IRQ-mode is supported on
>>> older (Meson6 or Meson8) hardware as well or if there are any errata)
>>>
>> Russell Cc'd for a quick question on the clk api.
> a quick side-note the clk API: my driver is a clock consumer and
> provider at the same time. This seems to be a recurring pattern in
> Amlogic hardware designs (as the MMC and DWMAC glue drivers are doing
> this also), see [0]
>
>> Ideally include a source for datasheets if available. Saves time googling and
>> perhaps getting the wrong thing!
> OK, will do this in v2
>
>> A few other minor comments inline. Pretty good V1.
> thanks :-)
>
>> Jonathan
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> ---
>>> drivers/iio/adc/Kconfig | 12 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/meson_saradc.c | 860 +++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 873 insertions(+)
>>> create mode 100644 drivers/iio/adc/meson_saradc.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 9c8b558ba19e..86059b9b91bf 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -371,6 +371,18 @@ config MEN_Z188_ADC
>>> This driver can also be built as a module. If so, the module will be
>>> called men_z188_adc.
>>>
>>> +config MESON_SARADC
>>> + tristate "Amlogic Meson SAR ADC driver"
>>> + default ARCH_MESON
>>> + depends on OF && COMMON_CLK && (ARCH_MESON || COMPILE_TEST)
>>> + select REGMAP_MMIO
>>> + help
>>> + Say yes here to build support for the SAR ADC found in Amlogic Meson
>>> + SoCs.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called meson_saradc.
>>> +
>>> config MXS_LRADC
>>> tristate "Freescale i.MX23/i.MX28 LRADC"
>>> depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index d36c4be8d1fc..de05b9e75f8f 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -36,6 +36,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o
>>> obj-$(CONFIG_MCP3422) += mcp3422.o
>>> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
>>> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
>>> +obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
>>> obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
>>> obj-$(CONFIG_NAU7802) += nau7802.o
>>> obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>>> new file mode 100644
>>> index 000000000000..06e8ac620385
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/meson_saradc.c
>>> @@ -0,0 +1,860 @@
>>> +/*
>>> + * Amlogic Meson Successive Approximation Register (SAR) A/D Converter
>>> + *
>>> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define SAR_ADC_REG0 0x00
>>> + #define SAR_ADC_REG0_PANEL_DETECT BIT(31)
>>> + #define SAR_ADC_REG0_BUSY_MASK GENMASK(30, 28)
>>> + #define SAR_ADC_REG0_DELTA_BUSY BIT(30)
>>> + #define SAR_ADC_REG0_AVG_BUSY BIT(29)
>>> + #define SAR_ADC_REG0_SAMPLE_BUSY BIT(28)
>>> + #define SAR_ADC_REG0_FIFO_FULL BIT(27)
>>> + #define SAR_ADC_REG0_FIFO_EMPTY BIT(26)
>>> + #define SAR_ADC_REG0_FIFO_COUNT_MASK GENMASK(25, 21)
>>> + #define SAR_ADC_REG0_ADC_BIAS_CTRL_MASK GENMASK(20, 19)
>>> + #define SAR_ADC_REG0_CURR_CHAN_ID_MASK GENMASK(18, 16)
>>> + #define SAR_ADC_REG0_ADC_TEMP_SEN_SEL BIT(15)
>>> + #define SAR_ADC_REG0_SAMPLING_STOP BIT(14)
>>> + #define SAR_ADC_REG0_CHAN_DELTA_EN_MASK GENMASK(13, 12)
>>> + #define SAR_ADC_REG0_DETECT_IRQ_POL BIT(10)
>>> + #define SAR_ADC_REG0_DETECT_IRQ_EN BIT(9)
>>> + #define SAR_ADC_REG0_FIFO_CNT_IRQ_MASK GENMASK(8, 4)
>>> + #define SAR_ADC_REG0_FIFO_IRQ_EN BIT(3)
>>> + #define SAR_ADC_REG0_SAMPLING_START BIT(2)
>>> + #define SAR_ADC_REG0_CONTINUOUS_EN BIT(1)
>>> + #define SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE BIT(0)
>>> +
>>> +#define SAR_ADC_CHAN_LIST 0x04
>>> + #define SAR_ADC_CHAN_LIST_MAX_INDEX_MASK GENMASK(26, 24)
>>> + #define SAR_ADC_CHAN_CHAN_ENTRY_MASK(_chan) \
>>> + (GENMASK(2, 0) << (_chan * 3))
>>> +
>>> +#define SAR_ADC_AVG_CNTL 0x08
>>> + #define SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(_chan) \
>>> + (16 + (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_AVG_MODE_MASK(_chan) \
>>> + (GENMASK(17, 16) << (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(_chan) \
>>> + (0 + (_chan * 2))
>>> + #define SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(_chan) \
>>> + (GENMASK(1, 0) << (_chan * 2))
>>> +
>>> +#define SAR_ADC_REG3 0x0c
>>> + #define SAR_ADC_REG3_CNTL_USE_SC_DLY BIT(31)
>>> + #define SAR_ADC_REG3_CLK_EN BIT(30)
>>> + #define SAR_ADC_REG3_BL30_INITIALIZED BIT(28)
>>> + #define SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN BIT(27)
>>> + #define SAR_ADC_REG3_CTRL_SAMPLING_CLOCK_PHASE BIT(26)
>>> + #define SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_REG3_DETECT_EN BIT(22)
>>> + #define SAR_ADC_REG3_ADC_EN BIT(21)
>>> + #define SAR_ADC_REG3_PANEL_DETECT_COUNT_MASK GENMASK(20, 18)
>>> + #define SAR_ADC_REG3_PANEL_DETECT_FILTER_TB_MASK GENMASK(17, 16)
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_SHIFT 10
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_WIDTH 5
>>> + #define SAR_ADC_REG3_ADC_CLK_DIV_MASK GENMASK(15, 10)
>>> + #define SAR_ADC_REG3_BLOCK_DLY_SEL_MASK GENMASK(9, 8)
>>> + #define SAR_ADC_REG3_BLOCK_DLY_MASK GENMASK(7, 0)
>>> +
>>> +#define SAR_ADC_DELAY 0x10
>>> + #define SAR_ADC_DELAY_INPUT_DLY_SEL_MASK GENMASK(25, 24)
>>> + #define SAR_ADC_DELAY_BL30_BUSY BIT(15)
>>> + #define SAR_ADC_DELAY_KERNEL_BUSY BIT(14)
>>> + #define SAR_ADC_DELAY_INPUT_DLY_CNT_MASK GENMASK(23, 16)
>>> + #define SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK GENMASK(9, 8)
>>> + #define SAR_ADC_DELAY_SAMPLE_DLY_CNT_MASK GENMASK(7, 0)
>>> +
>>> +#define SAR_ADC_LAST_RD 0x14
>>> + #define SAR_ADC_LAST_RD_LAST_CHANNEL1_MASK GENMASK(23, 16)
>>> + #define SAR_ADC_LAST_RD_LAST_CHANNEL0_MASK GENMASK(9, 0)
>>> +
>>> +#define SAR_ADC_FIFO_RD 0x18
>>> + #define SAR_ADC_FIFO_RD_CHAN_ID_MASK GENMASK(14, 12)
>>> + #define SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK GENMASK(11, 0)
>>> +
>>> +#define SAR_ADC_AUX_SW 0x1c
>>> + #define SAR_ADC_AUX_SW_MUX_SEL_CHAN_MASK(_chan) \
>>> + (GENMASK(10, 8) << ((_chan - 2) * 2))
>>> + #define SAR_ADC_AUX_SW_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_AUX_SW_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_AUX_SW_MODE_SEL BIT(4)
>>> + #define SAR_ADC_AUX_SW_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_AUX_SW_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_AUX_SW_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_AUX_SW_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_CHAN_10_SW 0x20
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_MUX_SEL_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_VREF_P_MUX BIT(22)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_VREF_N_MUX BIT(21)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_MODE_SEL BIT(20)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_YP_DRIVE_SW BIT(19)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_XP_DRIVE_SW BIT(18)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_YM_DRIVE_SW BIT(17)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN1_XM_DRIVE_SW BIT(16)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_MUX_SEL_MASK GENMASK(9, 7)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_MODE_SEL BIT(4)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_CHAN_10_SW_CHAN0_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_DETECT_IDLE_SW 0x24
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_SW_EN BIT(26)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK GENMASK(25, 23)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_VREF_P_MUX BIT(22)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_VREF_N_MUX BIT(21)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_SEL BIT(20)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_YP_DRIVE_SW BIT(19)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_XP_DRIVE_SW BIT(18)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_YM_DRIVE_SW BIT(17)
>>> + #define SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_XM_DRIVE_SW BIT(16)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK GENMASK(9, 7)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_VREF_P_MUX BIT(6)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_VREF_N_MUX BIT(5)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_SEL BIT(4)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_YP_DRIVE_SW BIT(3)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_XP_DRIVE_SW BIT(2)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_YM_DRIVE_SW BIT(1)
>>> + #define SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_XM_DRIVE_SW BIT(0)
>>> +
>>> +#define SAR_ADC_DELTA_10 0x28
>>> + #define SAR_ADC_DELTA_10_TEMP_SEL BIT(27)
>>> + #define SAR_ADC_DELTA_10_TS_REVE1 BIT(26)
>>> + #define SAR_ADC_DELTA_10_CHAN1_DELTA_VALUE_SHIFT 16
>>> + #define SAR_ADC_DELTA_10_CHAN1_DELTA_VALUE_MASK GENMASK(25, 16)
>>> + #define SAR_ADC_DELTA_10_TS_REVE0 BIT(15)
>>> + #define SAR_ADC_DELTA_10_TS_C_SHIFT 11
>>> + #define SAR_ADC_DELTA_10_TS_C_MASK GENMASK(14, 11)
>>> + #define SAR_ADC_DELTA_10_TS_VBG_EN BIT(10)
>>> + #define SAR_ADC_DELTA_10_CHAN0_DELTA_VALUE_SHIFT 0
>>> + #define SAR_ADC_DELTA_10_CHAN0_DELTA_VALUE_MASK GENMASK(9, 0)
>>> +
>>> +/* NOTE: registers from here are undocumented (the vendor Linux kernel driver
>>> + * and u-boot source served as reference). These only seem to be relevant on
>>> + * GXBB and newer.
>>> + */
>>> +#define SAR_ADC_REG11 0x2c
>>> + #define SAR_ADC_REG11_BANDGAP_EN BIT(13)
>>> +
>>> +#define SAR_ADC_REG13 0x34
>>> + #define SAR_ADC_REG13_12BIT_CALIBRATION_MASK GENMASK(13, 8)
>>> +
>>> +#define SAR_ADC_MAX_FIFO_SIZE 32
>>> +#define SAR_ADC_NUM_CHANNELS ARRAY_SIZE(meson_saradc_iio_channels)
>>> +#define SAR_ADC_VALUE_MASK(_priv) (BIT(_priv->resolution) - 1)
>>> +
>>> +#define MESON_SAR_ADC_CHAN(_chan, _type) { \
>>> + .type = _type, \
>>> + .indexed = true, \
>>> + .channel = _chan, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>> + BIT(IIO_CHAN_INFO_AVERAGE_RAW), \
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>>> + .datasheet_name = "SAR_ADC_CH"#_chan, \
>>> +}
>>> +
>>> +/* TODO: the hardware supports IIO_TEMP for channel 6 as well which is
>> Multline comment syntax.
> I got this wrong in 2 other places as well. will be fixed in v2, thanks!
>
>>> + * currently not supported by this driver.
>>> + */
>>> +static const struct iio_chan_spec meson_saradc_iio_channels[] = {
>>> + MESON_SAR_ADC_CHAN(0, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(1, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(2, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(3, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(4, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(5, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(6, IIO_VOLTAGE),
>>> + MESON_SAR_ADC_CHAN(7, IIO_VOLTAGE),
>>> + IIO_CHAN_SOFT_TIMESTAMP(8),
>>> +};
>>> +
>>> +enum meson_saradc_avg_mode {
>>> + NO_AVERAGING = 0x0,
>>> + MEAN_AVERAGING = 0x1,
>>> + MEDIAN_AVERAGING = 0x2,
>>> +};
>>> +
>>> +enum meson_saradc_num_samples {
>>> + ONE_SAMPLE = 0x0,
>>> + TWO_SAMPLES = 0x1,
>>> + FOUR_SAMPLES = 0x2,
>>> + EIGHT_SAMPLES = 0x3,
>>> +};
>>> +
>>> +enum meson_saradc_chan7_mux_sel {
>>> + CHAN7_MUX_VSS = 0x0,
>>> + CHAN7_MUX_VDD_DIV4 = 0x1,
>>> + CHAN7_MUX_VDD_DIV2 = 0x2,
>>> + CHAN7_MUX_VDD_MUL3_DIV4 = 0x3,
>>> + CHAN7_MUX_VDD = 0x4,
>>> + CHAN7_MUX_CH7_INPUT = 0x7,
>>> +};
>>> +
>>> +struct meson_saradc_priv {
>>> + struct regmap *regmap;
>>> + struct clk *clkin;
>>> + struct clk *core_clk;
>>> + struct clk *sana_clk;
>>> + struct clk *adc_sel_clk;
>>> + struct clk *adc_clk;
>>> + struct clk_gate clk_gate;
>>> + struct clk *adc_div_clk;
>>> + struct clk_divider clk_div;
>>> + struct regulator *vref;
>>> + struct completion completion;
>>> + u8 resolution;
>>> +};
>>> +
>>> +static const struct regmap_config meson_saradc_regmap_config = {
>>> + .reg_bits = 8,
>>> + .val_bits = 32,
>>> + .reg_stride = 4,
>>> + .max_register = SAR_ADC_REG13,
>>> +};
>>> +
>>> +static unsigned int meson_saradc_get_fifo_count(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> +
>>> + return FIELD_GET(SAR_ADC_REG0_FIFO_COUNT_MASK, regval);
>>> +}
>>> +
>>> +static int meson_saradc_wait_busy_clear(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int regval, timeout = 10000;
>>> +
>>> + do {
>>> + udelay(1);
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> + } while (FIELD_GET(SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
>>> +
>>> + if (timeout < 0)
>>> + return -ETIMEDOUT;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_read_raw_sample(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
>>> +
>>> + ret = meson_saradc_wait_busy_clear(indio_dev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> +
>>> + while (meson_saradc_get_fifo_count(indio_dev) > 0 &&
>>> + count < SAR_ADC_MAX_FIFO_SIZE) {
>>> + regmap_read(priv->regmap, SAR_ADC_FIFO_RD, ®val);
>>> +
>>> + fifo_chan = FIELD_GET(SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>>> + if (fifo_chan == chan->channel) {
>>> + fifo_val = FIELD_GET(SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK,
>>> + regval) & SAR_ADC_VALUE_MASK(priv);
>>> + sum += fifo_val;
>>> + count++;
>>> + }
>>> + }
>>> +
>>> + if (!count)
>>> + return -ENOENT;
>>> +
>>> + *val = sum / count;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_saradc_set_averaging(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + enum meson_saradc_avg_mode mode,
>>> + enum meson_saradc_num_samples samples)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 val;
>>> +
>>> + val = samples << SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>> + SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(chan->channel),
>>> + val);
>>> +
>>> + val = mode << SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>> + SAR_ADC_AVG_CNTL_AVG_MODE_MASK(chan->channel), val);
>>> +}
>>> +
>>> +static void meson_saradc_enable_channel(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + /* the SAR ADC engine allows sampling multiple channels at the same
>>> + * time. to keep it simple we're only working with one *internal*
>>> + * channel, which starts counting at index 0 (which means: count = 1).
>>> + */
>>> + regval = FIELD_PREP(SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>> + SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, regval);
>>> +
>>> + /* map channel index 0 to the channel which we want to read */
>>> + regval = FIELD_PREP(SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>> + SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), regval);
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>> + chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>> + SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>> + regval);
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>> + chan->channel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>> + SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>> + regval);
>>> +
>>> + if (chan->channel == 6)
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELTA_10,
>>> + SAR_ADC_DELTA_10_TEMP_SEL, 0);
>>> +}
>>> +
>>> +static void meson_saradc_set_channel7_mux(struct iio_dev *indio_dev,
>>> + enum meson_saradc_chan7_mux_sel sel)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + u32 regval;
>>> +
>>> + regval = FIELD_PREP(SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
>>> +
>>> + usleep_range(10, 20);
>>> +}
>>> +
>>> +static void meson_saradc_start_sample_engine(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLING_START,
>>> + SAR_ADC_REG0_SAMPLING_START);
>>> +}
>>> +
>>> +static void meson_saradc_stop_sample_engine(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLING_STOP,
>>> + SAR_ADC_REG0_SAMPLING_STOP);
>>> +
>>> + /* wait until all modules are stopped */
>>> + meson_saradc_wait_busy_clear(indio_dev);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE, 0);
>>> +}
>>> +
>>> +static void meson_saradc_lock(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int val;
>>> +
>>> + mutex_lock(&indio_dev->mlock);
>>> +
>>> + /* prevent BL30 from using the SAR ADC while we are using it */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY);
>>> +
>>> + /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
>>> + do {
>>> + udelay(1);
>>> + regmap_read(priv->regmap, SAR_ADC_DELAY, &val);
>>> + } while (val & SAR_ADC_DELAY_BL30_BUSY);
>>> +}
>>> +
>>> +static void meson_saradc_unlock(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + /* allow BL30 to use the SAR ADC again */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_KERNEL_BUSY, 0);
>>> +
>>> + mutex_unlock(&indio_dev->mlock);
>>> +}
>>> +
>>> +static int meson_saradc_get_sample(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + enum meson_saradc_avg_mode avg_mode,
>>> + enum meson_saradc_num_samples avg_samples,
>>> + int *val)
>>> +{
>>> + int ret, tmp;
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + /* clear old values from the FIFO buffer, ignoring errors */
>>> + meson_saradc_read_raw_sample(indio_dev, chan, &tmp);
>>> +
>>> + meson_saradc_set_averaging(indio_dev, chan, avg_mode, avg_samples);
>>> +
>>> + meson_saradc_enable_channel(indio_dev, chan);
>>> +
>>> + meson_saradc_start_sample_engine(indio_dev);
>>> + ret = meson_saradc_read_raw_sample(indio_dev, chan, val);
>>> + meson_saradc_stop_sample_engine(indio_dev);
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +
>>> + if (ret) {
>>> + dev_warn(&indio_dev->dev,
>>> + "failed to read sample for channel %d: %d\n",
>>> + chan->channel, ret);
>>> + return ret;
>>> + }
>>> +
>>> + return IIO_VAL_INT;
>>> +}
>>> +
>>> +static int meson_saradc_iio_info_read_raw(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan,
>>> + int *val, int *val2, long mask)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + return meson_saradc_get_sample(indio_dev, chan, NO_AVERAGING,
>>> + ONE_SAMPLE, val);
>>> + break;
>>> +
>>> + case IIO_CHAN_INFO_AVERAGE_RAW:
>>> + return meson_saradc_get_sample(indio_dev, chan, MEAN_AVERAGING,
>>> + EIGHT_SAMPLES, val);
>>> + break;
>>> +
>>> + case IIO_CHAN_INFO_SCALE:
>>> + ret = regulator_get_voltage(priv->vref);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "failed to get vref voltage: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + *val = ret / 1000;
>>> + *val2 = priv->resolution;
>>> + return IIO_VAL_FRACTIONAL_LOG2;
>>> +
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +}
>>> +
>>> +static int meson_saradc_clk_init(struct iio_dev *indio_dev, void __iomem *base)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + struct clk_init_data init;
>>> + char clk_name[32];
>>> + const char *clk_parents[1];
>>> +
>>> + snprintf(clk_name, sizeof(clk_name), "%s#adc_div",
>>> + of_node_full_name(indio_dev->dev.of_node));
>>> + init.name = devm_kstrdup(&indio_dev->dev, clk_name, GFP_KERNEL);
>>> + init.flags = 0;
>>> + init.ops = &clk_divider_ops;
>>> + clk_parents[0] = __clk_get_name(priv->clkin);
>>> + init.parent_names = clk_parents;
>>> + init.num_parents = 1;
>>> +
>>> + priv->clk_div.reg = base + SAR_ADC_REG3;
>>> + priv->clk_div.shift = SAR_ADC_REG3_ADC_CLK_DIV_SHIFT;
>>> + priv->clk_div.width = SAR_ADC_REG3_ADC_CLK_DIV_WIDTH;
>>> + priv->clk_div.hw.init = &init;
>>> + priv->clk_div.flags = 0;
>>> +
>>> + priv->adc_div_clk = devm_clk_register(&indio_dev->dev,
>>> + &priv->clk_div.hw);
>>> + if (WARN_ON(IS_ERR(priv->adc_div_clk)))
>>> + return PTR_ERR(priv->adc_div_clk);
>>> +
>>> + snprintf(clk_name, sizeof(clk_name), "%s#adc_en",
>>> + of_node_full_name(indio_dev->dev.of_node));
>>> + init.name = devm_kstrdup(&indio_dev->dev, clk_name, GFP_KERNEL);
>>> + init.flags = CLK_SET_RATE_PARENT;
>>> + init.ops = &clk_gate_ops;
>>> + clk_parents[0] = __clk_get_name(priv->adc_div_clk);
>>> + init.parent_names = clk_parents;
>>> + init.num_parents = 1;
>>> +
>>> + priv->clk_gate.reg = base + SAR_ADC_REG3;
>>> + priv->clk_gate.bit_idx = fls(SAR_ADC_REG3_CLK_EN);
>>> + priv->clk_gate.hw.init = &init;
>>> +
>>> + priv->adc_clk = devm_clk_register(&indio_dev->dev, &priv->clk_gate.hw);
>>> + if (WARN_ON(IS_ERR(priv->adc_clk)))
>>> + return PTR_ERR(priv->adc_clk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_init(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int regval, ret;
>>> +
>>> + /* make sure we start at CH7 input */
>> why? Seems like a little more detail would be good here ;)
> I'll change this to "make sure we start at CH7 input since the other
> muxes are only used for internal calibration." in v2
>
>>> + meson_saradc_set_channel7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
>>> +
>>> + regmap_read(priv->regmap, SAR_ADC_REG3, ®val);
>>> + if (regval & SAR_ADC_REG3_BL30_INITIALIZED) {
>>> + dev_info(&indio_dev->dev, "already initialized by BL30\n");
>>> + return 0;
>>> + }
>>> +
>>> + dev_info(&indio_dev->dev, "initializing SAR ADC\n");
>> I'd argue this provides no useful info so should be dropped.
>> Useful for debugging no doubt, but just noise going forward.
> do you want me to remove them or should I turn them into dev_dbg() (so
> they can be enabled for debugging purposes)?
Drop it entirely.
>
>>> +
>>> + meson_saradc_stop_sample_engine(indio_dev);
>>> +
>>> + /* update the channel 6 MUX to select the temperature sensor */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>> + SAR_ADC_REG0_ADC_TEMP_SEN_SEL,
>>> + SAR_ADC_REG0_ADC_TEMP_SEN_SEL);
>>> +
>>> + /* disable all channels by default */
>>> + regmap_write(priv->regmap, SAR_ADC_CHAN_LIST, 0x0);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CTRL_SAMPLING_CLOCK_PHASE, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>> + SAR_ADC_REG3_CNTL_USE_SC_DLY,
>>> + SAR_ADC_REG3_CNTL_USE_SC_DLY);
>>> +
>>> + /* delay between two samples = (10+1) * 1uS */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_CNT_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_SAMPLE_DLY_CNT_MASK, 10));
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_SAMPLE_DLY_SEL_MASK, 0));
>>> +
>>> + /* delay between two samples = (10+1) * 1uS */
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_CNT_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_INPUT_DLY_CNT_MASK, 10));
>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>> + SAR_ADC_DELAY_INPUT_DLY_SEL_MASK,
>>> + FIELD_PREP(SAR_ADC_DELAY_INPUT_DLY_SEL_MASK, 1));
>>> +
>> Cool. I hadn't come across FIELD_PREP before. Neater and tidier than having
>> a shift and a mask for at least some usecases.
> I think these were introduced with v4.9. I like them because I tend
> use GENMASK() incorrectly and with those macros I get an error at
> compile-time (without having to debug my code at all)
>
>>> + ret = clk_set_parent(priv->adc_sel_clk, priv->clkin);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev,
>>> + "failed to set adc parent to clkin\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = clk_set_rate(priv->adc_clk, 1200000);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to set adc clock rate\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_hw_enable(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + ret = regulator_enable(priv->vref);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev, "failed to enable vref regulator\n");
>>> + goto err_vref;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->core_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable core clk\n");
>>> + goto err_core_clk;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->sana_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable sana clk\n");
>>> + goto err_sana_clk;
>>> + }
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG11,
>>> + SAR_ADC_REG11_BANDGAP_EN, SAR_ADC_REG11_BANDGAP_EN);
>> Is this controlling an offset for a bandgap or some actual electronics?
>> Not sure if it should be disabled on error and the datasheets I've found are
>> far from great! You disable it in the disable, so I'd expect it to be
>> unwound on error in here too.
> actually the bandgap is not documented at all :(
>
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3, SAR_ADC_REG3_ADC_EN,
>>> + SAR_ADC_REG3_ADC_EN);
>> The fact you turn this of in the disable suggests to me that on error
>> you should be doing it in here too.
> I will disable this along with SAR_ADC_REG11_BANDGAP_EN in the
> err_adc_clk label, thanks for spotting this.
>
>>> +
>>> + udelay(5);
>>> +
>>> + ret = clk_prepare_enable(priv->adc_clk);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "failed to enable adc_en clk\n");
>>> + goto err_adc_clk;
>>> + }
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +
>>> + return 0;
>>> +
>>> +err_adc_clk:
>>> + clk_disable_unprepare(priv->sana_clk);
>>> +err_sana_clk:
>>> + clk_disable_unprepare(priv->core_clk);
>>> +err_core_clk:
>>> + regulator_disable(priv->vref);
>>> +err_vref:
>>> + meson_saradc_unlock(indio_dev);
>>> + return ret;
>>> +}
>>> +
>>> +static void meson_saradc_hw_disable(struct iio_dev *indio_dev)
>>> +{
>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>> +
>>> + meson_saradc_lock(indio_dev);
>>> +
>>> + clk_disable_unprepare(priv->adc_clk);
>>> +
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3, SAR_ADC_REG3_ADC_EN, 0);
>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG11,
>>> + SAR_ADC_REG11_BANDGAP_EN, 0);
>>> +
>>> + clk_disable_unprepare(priv->sana_clk);
>>> + clk_disable_unprepare(priv->core_clk);
>>> +
>>> + regulator_disable(priv->vref);
>>> +
>>> + meson_saradc_unlock(indio_dev);
>>> +}
>>> +
>>> +static const struct iio_info meson_saradc_iio_info = {
>>> + .read_raw = meson_saradc_iio_info_read_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static const struct of_device_id meson_saradc_of_match[] = {
>>> + {
>>> + .compatible = "amlogic,meson-gxbb-saradc",
>>> + .data = (void *)10,
>> Might have been worth having a structure array indexed from an enum.
>> For now it is overkill, but seems likely there are a few other differences
>> that aren't supported yet?
> what do you mean with "structure array indexed from an enum"? I can
> introduce some match-specific struct if you want (just like it's done
> in rockchip_saradc.c with "struct rockchip_saradc_data").
That works, or data can be an enum indexing an array of similar structures.
It doesn't really matter.
>
>>> + }, {
>>> + .compatible = "amlogic,meson-gxl-saradc",
>>> + .data = (void *)12,
>>> + },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, meson_saradc_of_match);
>>> +
>>> +static int meson_saradc_probe(struct platform_device *pdev)
>>> +{
>>> + struct meson_saradc_priv *priv;
>>> + struct iio_dev *indio_dev;
>>> + struct resource *res;
>>> + void __iomem *base;
>>> + const struct of_device_id *match;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
>>> + if (!indio_dev) {
>>> + dev_err(&pdev->dev, "failed allocating iio device\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + priv = iio_priv(indio_dev);
>>> +
>>> + match = of_match_device(meson_saradc_of_match, &pdev->dev);
>>> + priv->resolution = (unsigned long)match->data;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(base))
>>> + return PTR_ERR(base);
>>> +
>>> + priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>> + &meson_saradc_regmap_config);
>>> + if (IS_ERR(priv->regmap))
>>> + return PTR_ERR(priv->regmap);
>>> +
>>> + init_completion(&priv->completion);
>>> +
>>> + priv->clkin = devm_clk_get(&pdev->dev, "clkin");
>>> + if (IS_ERR(priv->clkin)) {
>>> + dev_err(&pdev->dev, "failed to get clkin\n");
>>> + return PTR_ERR(priv->clkin);
>>> + }
>>> +
>>> + priv->core_clk = devm_clk_get(&pdev->dev, "core");
>>> + if (IS_ERR(priv->core_clk)) {
>>> + dev_err(&pdev->dev, "failed to get core clk\n");
>>> + return PTR_ERR(priv->core_clk);
>>> + }
>>> +
>>> + priv->sana_clk = devm_clk_get(&pdev->dev, "sana");
>> Oh for a devm_clk_get_optional to handle this boiler plate neatly.
>> It's been suggested before, but nothing seems to have come of it.
> I guess quite a few drivers would benefit from that. maybe we should
> take this to the linux-clk list again?
Perhaps. I guess this has been proposed before but gone nowhere.
I haven't digged into why though.
>
>> Some array clk get functions might also clean this up a touch.
> yes, unfortunately in this case it's not that easy as it would have to
> allow a mix of mandatory and optional clocks. Additionally I cannot
> bulk-enable them unconditionally since some of these are simple gates,
> others need to be reparented and for some the rate has to be set.
>
>>> + if (IS_ERR(priv->sana_clk)) {
>>> + if (PTR_ERR(priv->sana_clk) == -ENOENT) {
>>> + priv->sana_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get sana clk\n");
>>> + return PTR_ERR(priv->sana_clk);
>>> + }
>>> + }
>>> +
>>> + priv->adc_clk = devm_clk_get(&pdev->dev, "adc_clk");
>>> + if (IS_ERR(priv->adc_clk)) {
>>> + if (PTR_ERR(priv->adc_clk) == -ENOENT) {
>>> + priv->adc_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get adc clk\n");
>>> + return PTR_ERR(priv->adc_clk);
>>> + }
>>> + }
>>> +
>>> + priv->adc_sel_clk = devm_clk_get(&pdev->dev, "adc_sel");
>>> + if (IS_ERR(priv->adc_sel_clk)) {
>>> + if (PTR_ERR(priv->adc_sel_clk) == -ENOENT) {
>>> + priv->adc_sel_clk = NULL;
>>> + } else {
>>> + dev_err(&pdev->dev, "failed to get adc_sel clk\n");
>>> + return PTR_ERR(priv->adc_sel_clk);
>>> + }
>>> + }
>>> +
>>> + /* on pre-GXBB SoCs the SAR ADC itself provides the ADC clock: */
>>> + if (!priv->adc_clk) {
>>> + ret = meson_saradc_clk_init(indio_dev, base);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + priv->vref = devm_regulator_get(&pdev->dev, "vref");
>>> + if (IS_ERR(priv->vref)) {
>>> + dev_err(&pdev->dev, "failed to get vref regulator\n");
>>> + return PTR_ERR(priv->vref);
>>> + }
>>> +
>>> + ret = meson_saradc_init(indio_dev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> + ret = meson_saradc_hw_enable(indio_dev);
>>> + if (ret)
>>> + goto err;
>>> +
>>> + platform_set_drvdata(pdev, indio_dev);
>>> +
>>> + indio_dev->name = dev_name(&pdev->dev);
>>> + indio_dev->dev.parent = &pdev->dev;
>>> + indio_dev->dev.of_node = pdev->dev.of_node;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->info = &meson_saradc_iio_info;
>>> +
>>> + indio_dev->channels = meson_saradc_iio_channels;
>>> + indio_dev->num_channels = SAR_ADC_NUM_CHANNELS;
>>> +
>>> + ret = iio_device_register(indio_dev);
>>> + if (ret)
>>> + goto err_hw;
>>> +
>>> + return 0;
>>> +
>>> +err_hw:
>>> + meson_saradc_hw_disable(indio_dev);
>>> +err:
>>> + return ret;
>>> +}
>>> +
>>> +static int meson_saradc_remove(struct platform_device *pdev)
>>> +{
>>> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>> +
>>> + meson_saradc_hw_disable(indio_dev);
>>> + iio_device_unregister(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int meson_saradc_suspend(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +
>>> + meson_saradc_hw_disable(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_saradc_resume(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>>> +
>>> + return meson_saradc_hw_enable(indio_dev);
>>> +}
>>> +#endif /* CONFIG_PM_SLEEP */
>>> +
>>> +static SIMPLE_DEV_PM_OPS(meson_saradc_pm_ops,
>>> + meson_saradc_suspend, meson_saradc_resume);
>>> +
>>> +static struct platform_driver meson_saradc_driver = {
>>> + .probe = meson_saradc_probe,
>>> + .remove = meson_saradc_remove,
>>> + .driver = {
>>> + .name = "meson-saradc",
>>> + .of_match_table = meson_saradc_of_match,
>>> + .pm = &meson_saradc_pm_ops,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(meson_saradc_driver);
>>> +
>>> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
>>> +MODULE_DESCRIPTION("Amlogic Meson SAR ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>
>
> Regards,
> Martin
>
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2016-August/000986.html
>
^ permalink raw reply
* [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Jonathan Cameron @ 2017-01-15 13:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0bfa97d1-2ee3-9dd9-dce3-593e93771f03@metafoo.de>
On 15/01/17 12:26, Lars-Peter Clausen wrote:
> On 01/11/2017 06:43 PM, Martin Blumenstingl wrote:
>> + indio_dev->name = dev_name(&pdev->dev);
>
> The name is supposed to be the type of the device, e.g. part name, not the
> name of parent device instance. E.g. meson-gxbb-saradc or meson-gxl-saradc
> in this case.
Drat. I've been missing these again. Somehow, never manage to get them
onto those things my eyes pick up whilst reviewing.
Thanks Lars,
Jonathan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH v4] ARM64: dts: meson-gx: Add reserved memory zone and usable memory range
From: Andreas Färber @ 2017-01-15 14:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <m21sw66828.fsf@baylibre.com>
Am 13.01.2017 um 21:03 schrieb Kevin Hilman:
> 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.
What is the motivation for this change? I have a local U-Boot patch to
detect the amount of memory available as done downstream, but U-Boot
only updates the reg property that you seem to be abandoning here...
So for devices that come in multiple RAM configurations - like R-Box Pro
- this would require separate .dts files now! This looks very wrong to
me, especially since I am not aware of other platforms doing the same.
Instead, there's memory reservations for top and bottom done in U-Boot
for reg, plus reserved-memory nodes for anything in the middle.
Another thing to consider is that uEFI boot (bootefi) handles memory
reservation differently yet again, on the bootloader level. I have had
that working fine on Odroid-C2 and Vega S95.
So if there's no bug this is fixing (none mentioned in commit message) I
strongly object to this patch.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply
* [RFT PATCH] ARM64: dts: meson-gxbb: Add reserved memory zone and usable memory range
From: Andreas Färber @ 2017-01-15 15:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <90d5b5a3-6dd8-aab2-8701-112a10e3364c@gmx.de>
Am 23.12.2016 um 10:42 schrieb Heinrich Schuchardt:
> it really makes a difference if we write
>
> memory at 0 {
> device_type = "memory";
> linux,usable-memory = <0x0 0x1000000 0x0 0x7f000000>;
> };
>
> or
>
> memory at 0 {
> device_type = "memory";
> reg = <0x0 0x1000000 0x0 0x7f000000>;
> };
>
> The second version leads to failure of the Odroid C2.
>
> When I looked at /sys/firmware/fdt I saw this difference:
>
> --- fails
> +++ works
>
> memory at 0 {
> - device_type = "memory";
> reg = <0x0 0x0 0x0 0x78000000>;
> + device_type = "memory";
> + linux,usable-memory = <0x0 0x1000000 0x0 0x7f000000>;
> };
>
> I found the following sentence in the NXP forum:
> In case you want to overwrite the memory usage passed from u-boot, you
> can use "linux,usable-memory".
> https://community.nxp.com/thread/382284
The Odroid-C2 is in mainline U-Boot. Please submit a patch to U-Boot
instead of forcing the creation of unnecessary new .dts files onto
everyone due to hardcoded linux,usable-memory properties. In fact, it
already reserves 0x1000000, so it seems you are merely using an older
U-Boot.
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-meson/board.c;h=f159cbf849f75ab046e6f3a025bbc97c0bcfd59d;hb=HEAD#l39
I would bet that the upper limit is unrelated here.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply
* Dropping device tree pinmux nodes for GPIO usage (Was: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes)
From: Rask Ingemann Lambertsen @ 2017-01-15 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v65GJFTnQZtWHCS7CAtA+Dry94o77aakPLr938BpQksLog@mail.gmail.com>
On Tue, Jan 03, 2017 at 10:52:12AM +0800, Chen-Yu Tsai wrote:
> On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara <andre.przywara@arm.com> wrote:
> > +
> > + mmc0_default_cd_pin: mmc0_cd_pin at 0 {
> > + pins = "PF6";
> > + function = "gpio_in";
> > + bias-pull-up;
> > + };
>
> We are starting to drop pinmux nodes for gpio usage.
How do we get the equivalent of bias-pull-up/down and drive-strength if we
run across a pin that needs it?
--
Rask Ingemann Lambertsen
^ permalink raw reply
* [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core
From: Florian Fainelli @ 2017-01-15 17:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115110753.GG26374@kroah.com>
On 01/15/2017 03:07 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
>> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
>> it going to be used by net/dsa/dsa2.c and the namespace of the function
>> justifies making it available to other users potentially.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> include/linux/netdevice.h | 2 ++
>> net/core/dev.c | 19 +++++++++++++++++++
>> net/dsa/dsa.c | 18 ------------------
>> 3 files changed, 21 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 97ae0ac513ee..6d021c37b774 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -4390,4 +4390,6 @@ do { \
>> #define PTYPE_HASH_SIZE (16)
>> #define PTYPE_HASH_MASK (PTYPE_HASH_SIZE - 1)
>>
>> +struct net_device *dev_to_net_device(struct device *dev);
>> +
>> #endif /* _LINUX_NETDEVICE_H */
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index ad5959e56116..7547e2ccc06b 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
>> return empty;
>> }
>>
>> +struct net_device *dev_to_net_device(struct device *dev)
>> +{
>> + struct device *d;
>> +
>> + d = device_find_class(dev, "net");
>> + if (d) {
>> + struct net_device *nd;
>> +
>> + nd = to_net_dev(d);
>> + dev_hold(nd);
>> + put_device(d);
>> +
>> + return nd;
>> + }
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(dev_to_net_device);
>
> This really isn't just a "struct device to net device cast" type
> function, (otherwise a simple container_of() would work). You are
> walking the device tree and assuming it is in a specific order so that
> this function works. You better document the hell out of this,
> otherwise people are going to try to use this and get very confused,
> very quickly...
Fair enough. Does that make it clearer how the device_find_class() is
used though? Maybe device_find_class() should be named
device_find_by_class_name() instead?
--
Florian
^ permalink raw reply
* [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Florian Fainelli @ 2017-01-15 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115110628.GF26374@kroah.com>
On 01/15/2017 03:06 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
>> Now that the base device driver code provides an identical
>> implementation of dev_find_class() utilize device_find_class() instead
>> of our own version of it.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> net/dsa/dsa.c | 22 ++--------------------
>> 1 file changed, 2 insertions(+), 20 deletions(-)
>>
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 2306d1b87c83..77fa4c4f5828 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>> #endif
>>
>> /* platform driver init and cleanup *****************************************/
>> -static int dev_is_class(struct device *dev, void *class)
>> -{
>> - if (dev->class != NULL && !strcmp(dev->class->name, class))
>> - return 1;
>> -
>> - return 0;
>> -}
>> -
>> -static struct device *dev_find_class(struct device *parent, char *class)
>> -{
>> - if (dev_is_class(parent, class)) {
>> - get_device(parent);
>> - return parent;
>> - }
>> -
>> - return device_find_child(parent, class, dev_is_class);
>> -}
>> -
>> struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>> {
>> struct device *d;
>>
>> - d = dev_find_class(dev, "mdio_bus");
>> + d = device_find_class(dev, "mdio_bus");
>> if (d != NULL) {
>> struct mii_bus *bus;
>
> You want a peer of your device on a specific class? What is this for?
It's not a peer of our device, it's a separate device reference from the
one looked up in the "net" class. In the classic, and now deprecated DSA
device driver model, a "dsa" platform device would represent one or more
Ethernet switches, connected via a MDIO bus (this reference above), and
one Ethernet device (the CPU/host/management interface). This was
completely violating the Linux device driver model and imposed
limitations on what bus would be used, and we did not have proper struct
device references (therefore no adequate hierarchy either).
Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
PCI, platform and drivers that allow us to register with DSA as a
specialized kind of device (so we are now finally using the right Linux
Device Driver model). What we still need though, in order to our switch
to the networking stack is a reference to the master/host network device
since we mangle packets in and out of it.
>
>> @@ -495,7 +477,7 @@ static struct net_device *dev_to_net_device(struct device *dev)
>> {
>> struct device *d;
>>
>> - d = dev_find_class(dev, "net");
>> + d = device_find_class(dev, "net");
>> if (d != NULL) {
>> struct net_device *nd;
>
> Again, huh? What is the device heirachy here that is so odd that this
> type of function is needed?
An Ethernet switch managed by DSA needs to have one ore more references
to a host/CPU/management network interface, this is what this struct
device reference is here for.
--
Florian
^ permalink raw reply
* [PATCH net-next v3 05/10] drivers: base: Add device_find_class()
From: Florian Fainelli @ 2017-01-15 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115110455.GE26374@kroah.com>
On 01/15/2017 03:04 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:08PM -0800, Florian Fainelli wrote:
>> Add a helper function to lookup a device reference given a class name.
>> This is a preliminary patch to remove adhoc code from net/dsa/dsa.c and
>> make it more generic.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/base/core.c | 19 +++++++++++++++++++
>> include/linux/device.h | 1 +
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 020ea7f05520..3dd6047c10d8 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -2065,6 +2065,25 @@ struct device *device_find_child(struct device *parent, void *data,
>> }
>> EXPORT_SYMBOL_GPL(device_find_child);
>>
>> +static int dev_is_class(struct device *dev, void *class)
>> +{
>> + if (dev->class != NULL && !strcmp(dev->class->name, class))
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>> +struct device *device_find_class(struct device *parent, char *class)
>
> Why are you using the char * for a class, and not just a pointer to
> "struct class"? That seems to be the most logical one, no need to rely
> on string comparisons here.
A more reflective name of what that does would probably be
device_find_by_class_name() or something alike.
>
> Also, what is this being used for? You aren't trying to walk up the
> device heirachy to find a specific "type" of device, are you? If so,
> ugh, I ranted about this in the past when the hyperv driver was trying
> to do such a thing...
What's a better way to do that though?
>
>> +{
>> + if (dev_is_class(parent, class)) {
>> + get_device(parent);
>> + return parent;
>> + }
>> +
>> + return device_find_child(parent, class, dev_is_class);
>
> You are trying to find a peer device with the same parent that belongs
> to a specific class?
Correct, network devices, and MDIO bus devices usually (always?) set
dev.parent.
>
> Again, what is this being used for?
See my other replies in patches 6, 7 and how it is used in patches 8 and
10 for instance.
>
> And all exported driver core functions should have full kerneldoc
> information for them so that people know how to use them, and what the
> constraints are (see device_find_child() as an example.) Please do that
> here as well because you are returning a pointer to a structure with the
> reference count incremented, callers need to know that.
Sure.
--
Florian
^ permalink raw reply
* [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Greg KH @ 2017-01-15 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fc247aeb-a8d6-5ba9-c26d-cb057a5ac34d@gmail.com>
On Sun, Jan 15, 2017 at 09:27:22AM -0800, Florian Fainelli wrote:
>
>
> On 01/15/2017 03:06 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
> >> Now that the base device driver code provides an identical
> >> implementation of dev_find_class() utilize device_find_class() instead
> >> of our own version of it.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >> net/dsa/dsa.c | 22 ++--------------------
> >> 1 file changed, 2 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> >> index 2306d1b87c83..77fa4c4f5828 100644
> >> --- a/net/dsa/dsa.c
> >> +++ b/net/dsa/dsa.c
> >> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
> >> #endif
> >>
> >> /* platform driver init and cleanup *****************************************/
> >> -static int dev_is_class(struct device *dev, void *class)
> >> -{
> >> - if (dev->class != NULL && !strcmp(dev->class->name, class))
> >> - return 1;
> >> -
> >> - return 0;
> >> -}
> >> -
> >> -static struct device *dev_find_class(struct device *parent, char *class)
> >> -{
> >> - if (dev_is_class(parent, class)) {
> >> - get_device(parent);
> >> - return parent;
> >> - }
> >> -
> >> - return device_find_child(parent, class, dev_is_class);
> >> -}
> >> -
> >> struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
> >> {
> >> struct device *d;
> >>
> >> - d = dev_find_class(dev, "mdio_bus");
> >> + d = device_find_class(dev, "mdio_bus");
> >> if (d != NULL) {
> >> struct mii_bus *bus;
> >
> > You want a peer of your device on a specific class? What is this for?
>
> It's not a peer of our device, it's a separate device reference from the
> one looked up in the "net" class. In the classic, and now deprecated DSA
> device driver model, a "dsa" platform device would represent one or more
> Ethernet switches, connected via a MDIO bus (this reference above), and
> one Ethernet device (the CPU/host/management interface). This was
> completely violating the Linux device driver model and imposed
> limitations on what bus would be used, and we did not have proper struct
> device references (therefore no adequate hierarchy either).
>
> Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
> PCI, platform and drivers that allow us to register with DSA as a
> specialized kind of device (so we are now finally using the right Linux
> Device Driver model). What we still need though, in order to our switch
> to the networking stack is a reference to the master/host network device
> since we mangle packets in and out of it.
Ok, but where in the tree are you trying to find this other device? Are
you just going to randomly find any device that happens to be of the
specific class type? That seems really fragile and broken :(
You should NEVER have to walk the device tree to find stuff. Why can't
you have a pointer to the device you need to talk to some other way?
What exactly is the relationship between these devices (a ascii-art tree
or sysfs tree output might be nice) so I can try to understand what is
going on here.
thanks,
greg k-h
^ permalink raw reply
* [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2
From: Florian Fainelli @ 2017-01-15 17:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115110831.GH26374@kroah.com>
On 01/15/2017 03:08 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
>> Hi all,
>>
>> This is not exactly new, and was sent before, although back then, I did not
>> have an user of the pre-declared MDIO board information, but now we do. Note
>> that I have additional changes queued up to have b53 register platform data for
>> MIPS bcm47xx and bcm63xx.
>>
>> Yes I know that we should have the Orion platforms eventually be converted to
>> Device Tree, but until that happens, I don't want any remaining users of the
>> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
>> and, there will be platforms out there that most likely won't never see DT
>> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
>> future).
>>
>> We would probably want the whole series to be merged via David Miller's tree
>> to simplify things.
>>
>> Greg, can you Ack/Nack patch 5 since it touched the core LDD?
>
> I've NAKed them for now, you need to describe what you are trying to do
> here, as it doesn't make any sense to me at the moment.
For one, this is moving *existing* code from net/dsa/dsa.c part into the
device core for device_find_class() and part into the network device
core for dev_to_net_device(). Patch 8 is where this actually gets used.
See my individual replies for more details.
Even though the existing code is there in net/dsa/dsa.c, at this point,
and for the sake of getting these patches merged via David, I can
probably just keep it where it is (like what patch series v1 did) and
just namespace it with dsa_. Later on, if this is deemed valuable to
other parts of the kernel, I can try to relocate it to the device core,
does that sound acceptable?
--
Florian
^ permalink raw reply
* [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core
From: Greg KH @ 2017-01-15 17:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <621be9fe-0869-2d0a-fd49-97468c2d4972@gmail.com>
On Sun, Jan 15, 2017 at 09:20:06AM -0800, Florian Fainelli wrote:
>
>
> On 01/15/2017 03:07 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
> >> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
> >> it going to be used by net/dsa/dsa2.c and the namespace of the function
> >> justifies making it available to other users potentially.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >> include/linux/netdevice.h | 2 ++
> >> net/core/dev.c | 19 +++++++++++++++++++
> >> net/dsa/dsa.c | 18 ------------------
> >> 3 files changed, 21 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 97ae0ac513ee..6d021c37b774 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -4390,4 +4390,6 @@ do { \
> >> #define PTYPE_HASH_SIZE (16)
> >> #define PTYPE_HASH_MASK (PTYPE_HASH_SIZE - 1)
> >>
> >> +struct net_device *dev_to_net_device(struct device *dev);
> >> +
> >> #endif /* _LINUX_NETDEVICE_H */
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index ad5959e56116..7547e2ccc06b 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
> >> return empty;
> >> }
> >>
> >> +struct net_device *dev_to_net_device(struct device *dev)
> >> +{
> >> + struct device *d;
> >> +
> >> + d = device_find_class(dev, "net");
> >> + if (d) {
> >> + struct net_device *nd;
> >> +
> >> + nd = to_net_dev(d);
> >> + dev_hold(nd);
> >> + put_device(d);
> >> +
> >> + return nd;
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dev_to_net_device);
> >
> > This really isn't just a "struct device to net device cast" type
> > function, (otherwise a simple container_of() would work). You are
> > walking the device tree and assuming it is in a specific order so that
> > this function works. You better document the hell out of this,
> > otherwise people are going to try to use this and get very confused,
> > very quickly...
>
> Fair enough. Does that make it clearer how the device_find_class() is
> used though? Maybe device_find_class() should be named
> device_find_by_class_name() instead?
Better, but you are just poking around randomly in the device tree and
"hoping" you get it right. What happens if devices move around? You
are assuming some sort of heirachy here that I don't understand at
all...
thanks,
greg k-h
^ permalink raw reply
* [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2
From: Greg KH @ 2017-01-15 17:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bc0d9ea7-17ee-abc3-b2aa-4d241413b9a3@gmail.com>
On Sun, Jan 15, 2017 at 09:40:24AM -0800, Florian Fainelli wrote:
> On 01/15/2017 03:08 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> This is not exactly new, and was sent before, although back then, I did not
> >> have an user of the pre-declared MDIO board information, but now we do. Note
> >> that I have additional changes queued up to have b53 register platform data for
> >> MIPS bcm47xx and bcm63xx.
> >>
> >> Yes I know that we should have the Orion platforms eventually be converted to
> >> Device Tree, but until that happens, I don't want any remaining users of the
> >> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
> >> and, there will be platforms out there that most likely won't never see DT
> >> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
> >> future).
> >>
> >> We would probably want the whole series to be merged via David Miller's tree
> >> to simplify things.
> >>
> >> Greg, can you Ack/Nack patch 5 since it touched the core LDD?
> >
> > I've NAKed them for now, you need to describe what you are trying to do
> > here, as it doesn't make any sense to me at the moment.
>
> For one, this is moving *existing* code from net/dsa/dsa.c part into the
> device core for device_find_class() and part into the network device
> core for dev_to_net_device(). Patch 8 is where this actually gets used.
> See my individual replies for more details.
>
> Even though the existing code is there in net/dsa/dsa.c, at this point,
> and for the sake of getting these patches merged via David, I can
> probably just keep it where it is (like what patch series v1 did) and
> just namespace it with dsa_. Later on, if this is deemed valuable to
> other parts of the kernel, I can try to relocate it to the device core,
> does that sound acceptable?
Nope!
I really want to try to understand what you all are doing with the
device tree that you feel that blindly walking it actually comes up with
a valid result.
See my other email about wanting to see a tree, we can take it from that
thread to try to consolidate all of these different ones.
And sorry, I know you are just trying to move code around, but this
isn't the first time this has come up, and I think it needs to be
resolved properly.
thanks,
greg k-h
^ permalink raw reply
* [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Florian Fainelli @ 2017-01-15 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115173949.GA19268@kroah.com>
On 01/15/2017 09:39 AM, Greg KH wrote:
> On Sun, Jan 15, 2017 at 09:27:22AM -0800, Florian Fainelli wrote:
>>
>>
>> On 01/15/2017 03:06 AM, Greg KH wrote:
>>> On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
>>>> Now that the base device driver code provides an identical
>>>> implementation of dev_find_class() utilize device_find_class() instead
>>>> of our own version of it.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> ---
>>>> net/dsa/dsa.c | 22 ++--------------------
>>>> 1 file changed, 2 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>>>> index 2306d1b87c83..77fa4c4f5828 100644
>>>> --- a/net/dsa/dsa.c
>>>> +++ b/net/dsa/dsa.c
>>>> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>>>> #endif
>>>>
>>>> /* platform driver init and cleanup *****************************************/
>>>> -static int dev_is_class(struct device *dev, void *class)
>>>> -{
>>>> - if (dev->class != NULL && !strcmp(dev->class->name, class))
>>>> - return 1;
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static struct device *dev_find_class(struct device *parent, char *class)
>>>> -{
>>>> - if (dev_is_class(parent, class)) {
>>>> - get_device(parent);
>>>> - return parent;
>>>> - }
>>>> -
>>>> - return device_find_child(parent, class, dev_is_class);
>>>> -}
>>>> -
>>>> struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>>>> {
>>>> struct device *d;
>>>>
>>>> - d = dev_find_class(dev, "mdio_bus");
>>>> + d = device_find_class(dev, "mdio_bus");
>>>> if (d != NULL) {
>>>> struct mii_bus *bus;
>>>
>>> You want a peer of your device on a specific class? What is this for?
>>
>> It's not a peer of our device, it's a separate device reference from the
>> one looked up in the "net" class. In the classic, and now deprecated DSA
>> device driver model, a "dsa" platform device would represent one or more
>> Ethernet switches, connected via a MDIO bus (this reference above), and
>> one Ethernet device (the CPU/host/management interface). This was
>> completely violating the Linux device driver model and imposed
>> limitations on what bus would be used, and we did not have proper struct
>> device references (therefore no adequate hierarchy either).
>>
>> Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
>> PCI, platform and drivers that allow us to register with DSA as a
>> specialized kind of device (so we are now finally using the right Linux
>> Device Driver model). What we still need though, in order to our switch
>> to the networking stack is a reference to the master/host network device
>> since we mangle packets in and out of it.
>
> Ok, but where in the tree are you trying to find this other device? Are
> you just going to randomly find any device that happens to be of the
> specific class type? That seems really fragile and broken :(
The search is not really random, since the platform data for the DSA
switch we want to register gives us a reference to a device registered
via any bus type, and we know (by contract it has to) that it has to
provide a network device of some kind, we do a specific search in the
"net" class for that purpose. I agree this is not great.
>
> You should NEVER have to walk the device tree to find stuff. Why can't
> you have a pointer to the device you need to talk to some other way?
We do, I think the existing code just tries to be extra careful here and
make sure that the device reference we got is actually part of the "net"
class, indicating that the reference passed is indeed pointing to a
network device, and not a random device.
>
> What exactly is the relationship between these devices (a ascii-art tree
> or sysfs tree output might be nice) so I can try to understand what is
> going on here.
OK, let me try and let's take the case of only one Ethernet switch in
the system for all simplicity:
- switch device drivers get probed by any kind of bus allocate and
register a dsa_switch with dsa_register_switch()
- this switch device has platform data (struct dsa_chip_data) that
describes its port layout (number of ports mostly) and has one struct
device references to which Ethernet network interface these ports connect to
- a dsa_switch_tree is created, this struct dsa_switch is attached to
the dsa_switch_tree at position 0 in the tree, we invoke a bunch of
switch driver operations
- we conclude with attaching this dsa_switch_tree to the network device
we looked up and assigning the tree to the dsa_ptr member
Documentation/networking/dsa/dsa.txt has some ascii art drawing that
sort of capture what is going on.
--
Florian
^ permalink raw reply
* Nokia N900: mixers changed between 4.9 and 4.10-rc3, no longer can use in-call speaker
From: Jarkko Nikula @ 2017-01-15 19:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115100533.GA15816@amd>
On 01/15/2017 12:05 PM, Pavel Machek wrote:
>>> So regression seems to be between v4.9 and v4.10. Any ideas?
>>
>> Interesting... seems there are no sound relevant changes after v4.9.
>>
>> Looks like there are only three commits after v4.9 for sound/soc which
>> are built for Nokia N900:
>>
>> e411b0b5eb9b65257a050eac333d181d6e00e2c6
>> e7aa450fe17890e59db7d3c2d8eff5b6b41fc531
>> 63c3194b82530bd71fd49db84eb7ab656b8d404a
>>
>> Maybe something not related to sound/soc could broke it?
>
> Lets see.
>
> a9042defa29a01cc538b742eab047848e9b5ae14 -- works ok.
> ce38207f161513ee3d2bd3860489f07ebe65bc78 --
>
> alsactl: set_control:1328: failed to obtain info for control #229 (No
> such file or directory)
>
I'm unable to test older kernels at the moment since modules don't load
by some reason (bluetooth: Unknown symbol _GLOBAL_OFFSET_TABLE_ (err
0)). Probably some recent regression in my debian/testing installation
since the same configs were working before.
However v4.9.4 works and with it I can see there is an issue with these
aic3x controls:
amixer: Unable to find simple control 'Left DAC_L1 Mixer Line',0
amixer: Unable to find simple control 'Right DAC_R1 Mixer Line',0
At quick look I don't see why above commits could cause it. Maybe
regression happened earlier between v4.6 and v4.9?
--
Jarkko
^ permalink raw reply
* [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Andrew Lunn @ 2017-01-15 19:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a9ea1f45-b9a5-d840-68e0-9bf9bc7a6598@gmail.com>
> > What exactly is the relationship between these devices (a ascii-art tree
> > or sysfs tree output might be nice) so I can try to understand what is
> > going on here.
Hi Greg, Florian
A few diagrams and trees which might help understand what is going on.
The first diagram comes from the 2008 patch which added all this code:
+-----------+ +-----------+
| | RGMII | |
| +-------+ +------ 1000baseT MDI ("WAN")
| | | 6-port +------ 1000baseT MDI ("LAN1")
| CPU | | ethernet +------ 1000baseT MDI ("LAN2")
| |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
| +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
| | | |
+-----------+ +-----------+
We have an ethernet switch and a host CPU. The switch is connected to
the CPU in two different ways. RGMII allows us to get Ethernet frames
from the CPU into the switch. MIImgmt, is the management bus normally
used for Ethernet PHYs, but Marvell switches also use it for Managing
switches.
The diagram above is the simplest setup. You can have multiple
Ethernet switches, connected together via switch ports. Each switch
has its own MIImgmt connect to the CPU, but there is only one RGMII
link.
When this code was designed back in 2008, it was decided to represent
this is a platform device, and it has a platform_data, which i have
slightly edited to keep it simple:
struct dsa_platform_data {
/*
* Reference to a Linux network interface that connects
* to the root switch chip of the tree.
*/
struct device *netdev;
/*
* Info structs describing each of the switch chips
* connected via this network interface.
*/
int nr_chips;
struct dsa_chip_data *chip;
};
This netdev is the CPU side of the RGMII interface.
Each switch has a dsa_chip_data, again edited:
struct dsa_chip_data {
/*
* How to access the switch configuration registers.
*/
struct device *host_dev;
int sw_addr;
...
}
The host_dev is the CPU side of the MIImgmt, and we have the address
the switch is using on the bus.
During probe of this platform device, we need to get from the
struct device *netdev to a struct net_device *dev.
So the code looks in the device net class to find the device
| | | |-- f1074000.ethernet
| | | | |-- deferred_probe
| | | | |-- driver -> ../../../../../bus/platform/drivers/mvneta
| | | | |-- driver_override
| | | | |-- modalias
| | | | |-- net
| | | | | `-- eth1
| | | | | |-- addr_assign_type
| | | | | |-- address
| | | | | |-- addr_len
| | | | | |-- broadcast
| | | | | |-- carrier
| | | | | |-- carrier_changes
| | | | | |-- deferred_probe
| | | | | |-- device -> ../../../f1074000.ethernet
and then use container_of() to get the net_device.
Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.
| | | |-- f1072004.mdio
| | | | |-- deferred_probe
| | | | |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
| | | | |-- driver_override
| | | | |-- mdio_bus
| | | | | `-- f1072004.mdio-mi
| | | | | |-- deferred_probe
| | | | | |-- device -> ../../../f1072004.mdio
Andrew
^ permalink raw reply
* mmc: core: complete/wait_for_completion performance
From: Stefan Wahren @ 2017-01-15 21:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484512950.2026.4.camel@embedded.rocks>
Hi J?rg,
> J?rg Krause <joerg.krause@embedded.rocks> hat am 15. Januar 2017 um 21:42 geschrieben:
>
>
> Hi Stefan,
>
> On Tue, 2016-12-27 at 00:03 +0100, Stefan Wahren wrote:
> > Hi J?rg,
> > ...
> > I also rebased an old patch from Shawn Guo [2] with pre_req and
> > post_req support, tried to call the DMA channel callback from the
> > interrupt context instead of scheduling the tasklet within the DMA
> > engine driver and implement CMD23 support [3]. But none of them show
> > any measurable performance improvement.
>
> I tested the three patches after disabling any debugging options in the
> config. There is no performance gain, but the timings have changed.
> Please have a look at the attached graphs. The time between complete()
> and wait_for_completion() is reduced to 15us whereas the time from
> return from wait_for_completion() to return to sdio_readsb() increases
> to 23us (from maybe 2us before).
that confirms my results. I think you are searching on the wrong layer. You better step "back" and take a look at the whole transfer instead of single blocks. Maybe there are some bigger delays.
Stefan
>
> J?rg
^ permalink raw reply
* [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
From: Laurent Pinchart @ 2017-01-15 21:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0e5c066f-ddf6-30ce-392e-79bd0f401a18@arm.com>
Hi Robin,
On Friday 13 Jan 2017 12:17:24 Robin Murphy wrote:
> On 13/01/17 11:59, Geert Uytterhoeven wrote:
> > On Fri, Jan 13, 2017 at 12:32 PM, Robin Murphy wrote:
> >> On 13/01/17 11:07, Geert Uytterhoeven wrote:
> >>> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
> >>> This allows to allocate physically contiguous DMA buffers on arm64
> >>> systems with an IOMMU.
> >>
> >> Can anyone explain what this attribute is actually used for? I've never
> >> quite figured it out.
> >
> > My understanding is that DMA_ATTR_FORCE_CONTIGUOUS is needed when using
> > an IOMMU but wanting the buffers to be both contiguous in IOVA space and
> > physically contiguous to allow passing to devices without IOMMU.
> >
> > Main users are graphic and remote processors.
>
> Sure, I assumed it must be to do with buffer sharing, but the systems
> I'm aware of which have IOMMUs in their media subsystems tend to have
> them in front of every IP block involved, so I was curious as to what
> bit of non-IOMMU hardware wanted to play too. The lone in-tree use in
> the Exynos DRM driver was never very revealing, and the new one I see in
> the Qualcomm PIL driver frankly looks redundant to me.
If two (or more) devices with different memory requirements are involved in
buffer sharing, we need to either allocate and export buffers from the device
with the strictest requirements, or to implement a central buffer allocator.
In any case, I don't think DMA_ATTR_FORCE_CONTIGUOUS is the right solution to
that problem.
Forcing contiguous allocation can however help with performance optimization,
as mapping physically contiguous memory through IOMMUs can make use of larger
page sizes.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 1/2] ARM: dts: imx6ul-isiot: Add eMMC node
From: Jagan Teki @ 2017-01-15 21:50 UTC (permalink / raw)
To: linux-arm-kernel
From: Jagan Teki <jagan@amarulasolutions.com>
Add usdhc2 node, which is eMMC for Engicam Is.IoT MX6UL modules.
dmesg:
-----
mmc1: SDHCI controller on 2194000.usdhc [2194000.usdhc] using ADMA
mmc1: new DDR MMC card at address 0001
mmcblk1: mmc1:0001 M62704 3.53 GiB
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Signed-off-by: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
arch/arm/boot/dts/imx6ul-isiot.dts | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ul-isiot.dts b/arch/arm/boot/dts/imx6ul-isiot.dts
index 077bc26..acb97bd 100644
--- a/arch/arm/boot/dts/imx6ul-isiot.dts
+++ b/arch/arm/boot/dts/imx6ul-isiot.dts
@@ -76,6 +76,15 @@
status = "okay";
};
+&usdhc2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usdhc2>;
+ cd-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>;
+ bus-width = <8>;
+ no-1-8-v;
+ status = "okay";
+};
+
&iomuxc {
pinctrl_uart1: uart1grp {
fsl,pins = <
@@ -116,4 +125,20 @@
MX6UL_PAD_SD1_DATA3__USDHC1_DATA3 0x170f9
>;
};
+
+ pinctrl_usdhc2: usdhc2grp {
+ fsl,pins = <
+ MX6UL_PAD_NAND_RE_B__USDHC2_CLK 0x17070
+ MX6UL_PAD_NAND_WE_B__USDHC2_CMD 0x10070
+ MX6UL_PAD_NAND_DATA00__USDHC2_DATA0 0x17070
+ MX6UL_PAD_NAND_DATA01__USDHC2_DATA1 0x17070
+ MX6UL_PAD_NAND_DATA02__USDHC2_DATA2 0x17070
+ MX6UL_PAD_NAND_DATA03__USDHC2_DATA3 0x17070
+ MX6UL_PAD_NAND_DATA04__USDHC2_DATA4 0x17070
+ MX6UL_PAD_NAND_DATA05__USDHC2_DATA5 0x17070
+ MX6UL_PAD_NAND_DATA06__USDHC2_DATA6 0x17070
+ MX6UL_PAD_NAND_DATA07__USDHC2_DATA7 0x17070
+ MX6UL_PAD_NAND_ALE__USDHC2_RESET_B 0x17070
+ >;
+ };
};
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: imx6qdl: Fix "ERROR: code indent should use tabs where possible"
From: Jagan Teki @ 2017-01-15 21:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484517012-13321-1-git-send-email-jagan@openedev.com>
From: Jagan Teki <jagan@amarulasolutions.com>
Fixed code indent tabs in respetcive imx6qdl dtsi files and
also add space on imx6qdl-icore-rqs.dtsi on usdhc bus-width nodes.
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 2 +-
arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 2 +-
arch/arm/boot/dts/imx6qdl-gw552x.dtsi | 2 +-
arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index b6078b1..91991d6 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -235,7 +235,7 @@
/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
reg_1p8v: sw2 {
- regulator-name = "vdd1p8";
+ regulator-name = "vdd1p8";
regulator-min-microvolt = <1033310>;
regulator-max-microvolt = <2004000>;
lltc,fb-voltage-divider = <301000 200000>;
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 19cf036..a208e7e 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -227,7 +227,7 @@
/* VDD_1P8 (1+R1/R2 = 2.505): GPS/VideoIn/ENET-PHY */
reg_1p8v: sw2 {
- regulator-name = "vdd1p8";
+ regulator-name = "vdd1p8";
regulator-min-microvolt = <1033310>;
regulator-max-microvolt = <2004000>;
lltc,fb-voltage-divider = <301000 200000>;
diff --git a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
index 90200fa..67613dd 100644
--- a/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw552x.dtsi
@@ -162,7 +162,7 @@
/* VDD_1P8 (1+R1/R2 = 2.505): ENET-PHY */
reg_1p8v: sw2 {
- regulator-name = "vdd1p8";
+ regulator-name = "vdd1p8";
regulator-min-microvolt = <1033310>;
regulator-max-microvolt = <2004000>;
lltc,fb-voltage-divider = <301000 200000>;
diff --git a/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi b/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi
index d5c3aa8..6e29d8b 100644
--- a/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-icore-rqs.dtsi
@@ -223,7 +223,7 @@
pinctrl-2 = <&pinctrl_usdhc3_200mhz>;
vmcc-supply = <®_sd3_vmmc>;
cd-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
- bus-witdh=<4>;
+ bus-witdh = <4>;
no-1-8-v;
status = "okay";
};
@@ -234,7 +234,7 @@
pinctrl-1 = <&pinctrl_usdhc4_100mhz>;
pinctrl-2 = <&pinctrl_usdhc4_200mhz>;
vmcc-supply = <®_sd4_vmmc>;
- bus-witdh=<8>;
+ bus-witdh = <8>;
no-1-8-v;
non-removable;
status = "okay";
--
1.9.1
^ permalink raw reply related
* [PATCH 0/2] add support for uart_AO_B
From: Martin Blumenstingl @ 2017-01-15 22:20 UTC (permalink / raw)
To: linux-arm-kernel
This adds the uart_AO_B serial port and the related pinctrl nodes to
meson-gx.
Due to a bug in the current GXBB and GXL/GXM pinctrl code uart_AO_B
could not be used when uart_AO(_A) was enabled at the same time. The
reason for this is that uart_AO_A and uart_AO_B were both trying to
request the same pin as their RX pin (GPIOAO_1). uart_AO_B also
requests a second pin for it's RX (GPIOAO_5), which does not make any
sense.
Thus uart_AO_B is changed to only use GPIOAO_5 for RX and GPIOAO_4
for TX, which is also what the Amlogic reference kernel does.
Adding uart_AO_B is preparation work for adding support for the
Khadas VIM and VIM Pro boards, as these are both exposing uart_AO_B
on the pin-header.
Martin Blumenstingl (2):
pinctrl: meson: fix uart_ao_b for GXBB and GXL/GXM
ARM64: dts: meson-gx: add the missing uart_AO_B
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 8 ++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 7 +++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 7 +++++++
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 7 +++----
drivers/pinctrl/meson/pinctrl-meson-gxl.c | 7 +++----
5 files changed, 28 insertions(+), 8 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH 1/2] pinctrl: meson: fix uart_ao_b for GXBB and GXL/GXM
From: Martin Blumenstingl @ 2017-01-15 22:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115222029.8271-1-martin.blumenstingl@googlemail.com>
The GXBB and GXL/GXM pinctrl drivers had a configuration which conflicts
with uart_ao_a. According to the GXBB ("S905") datasheet the AO UART
functions are:
- GPIOAO_0: Func1 = UART_TX_AO_A (bit 12), Func2 = UART_TX_AO_B (bit 26)
- GPIOAO_1: Func1 = UART_RX_AO_A (bit 11), Func2 = UART_RX_AO_B (bit 25)
- GPIOAO_4: Func2 = UART_TX_AO_B (bit 24)
- GPIOAO_5: Func2 = UART_RX_AO_B (bit 25)
The existing definition for uart_AO_A already uses GPIOAO_0 and GPIOAO_1.
The old definition of uart_AO_B however was broken, as it used GPIOAO_0
for TX (which would be fine) and two pins (GPIOAO_1 and GPIOAO_5) for RX
(which does not make any sense).
This fixes the uart_AO_B configuration by moving it to GPIOAO_4 and
GPIOAO_5 (it would be possible to use GPIOAO_0 and GPIOAO_1 in theory,
but all existing hardware uses uart_AO_A there).
The fix for GXBB and GXL/GXM is identical since it seems that these
specific pins are identical on both SoC variants.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 7 +++----
drivers/pinctrl/meson/pinctrl-meson-gxl.c | 7 +++----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index c3928aa3fefa..e0bca4df2a2f 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -253,9 +253,8 @@ static const unsigned int uart_tx_ao_a_pins[] = { PIN(GPIOAO_0, 0) };
static const unsigned int uart_rx_ao_a_pins[] = { PIN(GPIOAO_1, 0) };
static const unsigned int uart_cts_ao_a_pins[] = { PIN(GPIOAO_2, 0) };
static const unsigned int uart_rts_ao_a_pins[] = { PIN(GPIOAO_3, 0) };
-static const unsigned int uart_tx_ao_b_pins[] = { PIN(GPIOAO_0, 0) };
-static const unsigned int uart_rx_ao_b_pins[] = { PIN(GPIOAO_1, 0),
- PIN(GPIOAO_5, 0) };
+static const unsigned int uart_tx_ao_b_pins[] = { PIN(GPIOAO_4, 0) };
+static const unsigned int uart_rx_ao_b_pins[] = { PIN(GPIOAO_5, 0) };
static const unsigned int uart_cts_ao_b_pins[] = { PIN(GPIOAO_2, 0) };
static const unsigned int uart_rts_ao_b_pins[] = { PIN(GPIOAO_3, 0) };
@@ -498,7 +497,7 @@ static struct meson_pmx_group meson_gxbb_aobus_groups[] = {
GPIO_GROUP(GPIOAO_13, 0),
/* bank AO */
- GROUP(uart_tx_ao_b, 0, 26),
+ GROUP(uart_tx_ao_b, 0, 24),
GROUP(uart_rx_ao_b, 0, 25),
GROUP(uart_tx_ao_a, 0, 12),
GROUP(uart_rx_ao_a, 0, 11),
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
index 25694f7094c7..b69743b07a1d 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
@@ -214,9 +214,8 @@ static const unsigned int uart_tx_ao_a_pins[] = { PIN(GPIOAO_0, 0) };
static const unsigned int uart_rx_ao_a_pins[] = { PIN(GPIOAO_1, 0) };
static const unsigned int uart_cts_ao_a_pins[] = { PIN(GPIOAO_2, 0) };
static const unsigned int uart_rts_ao_a_pins[] = { PIN(GPIOAO_3, 0) };
-static const unsigned int uart_tx_ao_b_pins[] = { PIN(GPIOAO_0, 0) };
-static const unsigned int uart_rx_ao_b_pins[] = { PIN(GPIOAO_1, 0),
- PIN(GPIOAO_5, 0) };
+static const unsigned int uart_tx_ao_b_pins[] = { PIN(GPIOAO_4, 0) };
+static const unsigned int uart_rx_ao_b_pins[] = { PIN(GPIOAO_5, 0) };
static const unsigned int uart_cts_ao_b_pins[] = { PIN(GPIOAO_2, 0) };
static const unsigned int uart_rts_ao_b_pins[] = { PIN(GPIOAO_3, 0) };
@@ -409,7 +408,7 @@ static struct meson_pmx_group meson_gxl_aobus_groups[] = {
GPIO_GROUP(GPIOAO_9, 0),
/* bank AO */
- GROUP(uart_tx_ao_b, 0, 26),
+ GROUP(uart_tx_ao_b, 0, 24),
GROUP(uart_rx_ao_b, 0, 25),
GROUP(uart_tx_ao_a, 0, 12),
GROUP(uart_rx_ao_a, 0, 11),
--
2.11.0
^ permalink raw reply related
* [PATCH 2/2] ARM64: dts: meson-gx: add the missing uart_AO_B
From: Martin Blumenstingl @ 2017-01-15 22:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170115222029.8271-1-martin.blumenstingl@googlemail.com>
This adds the missing node for the uart_AO_B port to the meson-gx.dtsi
(as this is supported by GXBB, GXL and GXM) along with the required
pinctrl pins. This is required as some boards are using it (the boards
from the Khadas VIM series for example have it exposed on the pin
headers).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 8 ++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 7 +++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 7 +++++++
3 files changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index cddad8c795ec..5ece505dca71 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -324,6 +324,14 @@
status = "disabled";
};
+ uart_AO_B: serial at 4e0 {
+ compatible = "amlogic,meson-uart";
+ reg = <0x0 0x004e0 0x0 0x14>;
+ interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>;
+ status = "disabled";
+ };
+
ir: ir at 580 {
compatible = "amlogic,meson-gxbb-ir";
reg = <0x0 0x00580 0x0 0x40>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 5d686334f692..474435e21759 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -138,6 +138,13 @@
};
};
+ uart_ao_b_pins: uart_ao_b {
+ mux {
+ groups = "uart_tx_ao_b", "uart_rx_ao_b";
+ function = "uart_ao_b";
+ };
+ };
+
remote_input_ao_pins: remote_input_ao {
mux {
groups = "remote_input_ao";
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index bb2842f8a08f..53ed7a5f50ab 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -82,6 +82,13 @@
};
};
+ uart_ao_b_pins: uart_ao_b {
+ mux {
+ groups = "uart_tx_ao_b", "uart_rx_ao_b";
+ function = "uart_ao_b";
+ };
+ };
+
remote_input_ao_pins: remote_input_ao {
mux {
groups = "remote_input_ao";
--
2.11.0
^ permalink raw reply related
* [PATCH 0/4] Bluetooth support for GXBB/GXL/GXM based devices
From: Martin Blumenstingl @ 2017-01-15 22:32 UTC (permalink / raw)
To: linux-arm-kernel
This adds the missing kernel bits for Bluetooth support on the
Tronsmart Vega S95 (GXBB based) boards as well as for the GXL
P230/P231 and GXM Q200/Q201 reference boards.
The Bluetooth functionality on these boards is provided by the
SDIO wifi/Bluetooth combo-chip (Broadcom bcm43xx based). The
Bluetooth module on that combo-chip has to be taken out of reset,
which is taken care of the GPIO in the sdio_pwrseq.
Once the module is taken out of reset it can be set up from userspace
using the "hciattach" tool from bluez, which talks to the Bluetooth
module which is connected to one of the serial ports (in our case
uart_A). To get the Bluetooth module initialized within the timeout
defined by "hciattach" (and to achieve usable speeds for Bluetooth
transfers) the communication uses a speed of 2000000 baud, which was
not supported by meson_uart before.
NOTE: The .dts-changes from this series depends on my previous series
"add support for uart_AO_B" - see [0]
[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001982.html
Martin Blumenstingl (4):
tty: serial: meson: allow baud-rates higher than 115200
ARM64: dts: meson-gx: add the serial CTS and RTS pin groups
ARM64: dts: meson-gx-p23x-q20x: enable the Bluetooth module
ARM64: dts: meson-gxbb-vega-s95: enable the Bluetooth module
.../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 12 ++++++-
.../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 9 +++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 40 ++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 40 ++++++++++++++++++++++
drivers/tty/serial/meson_uart.c | 2 +-
5 files changed, 101 insertions(+), 2 deletions(-)
--
2.11.0
^ 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