* Re: [PATCH 2/2] drm/panel: simple: Add support for KOE TX26D202VM0BWA panel
From: Emil Velikov @ 2020-06-02 12:46 UTC (permalink / raw)
To: Liu Ying
Cc: ML dri-devel, devicetree, Thierry Reding, Sam Ravnborg,
NXP Linux Team
In-Reply-To: <1590991880-24273-1-git-send-email-victor.liu@nxp.com>
On Tue, 2 Jun 2020 at 08:17, Liu Ying <victor.liu@nxp.com> wrote:
>
> This patch adds support for Kaohsiung Opto-Electronics Inc.
> 10.1" TX26D202VM0BWA WUXGA(1920x1200) TFT LCD panel with LVDS interface.
> The panel has dual LVDS channels.
>
> My panel is manufactured by US Micro Products(USMP). There is a tag at
> the back of the panel, which indicates the panel type is 'TX26D202VM0BWA'
> and it's made by KOE in Taiwan.
>
> The panel spec from USMP can be found at:
> https://www.usmicroproducts.com/sites/default/files/datasheets/USMP-T101-192120NDU-A0.pdf
>
> The below panel spec from KOE is basically the same to the one from USMP.
> However, the panel type 'TX26D202VM0BAA' is a little bit different.
> It looks that the two types of panel are compatible with each other.
> http://www.koe.j-display.com/upload/product/TX26D202VM0BAA.pdf
>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
> drivers/gpu/drm/panel/panel-simple.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index b6ecd15..7c222ec 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2200,6 +2200,37 @@ static const struct panel_desc koe_tx14d24vm1bpa = {
> },
> };
>
> +static const struct display_timing koe_tx26d202vm0bwa_timing = {
> + .pixelclock = { 151820000, 156720000, 159780000 },
> + .hactive = { 1920, 1920, 1920 },
> + .hfront_porch = { 105, 130, 142 },
> + .hback_porch = { 45, 70, 82 },
> + .hsync_len = { 30, 30, 30 },
> + .vactive = { 1200, 1200, 1200},
> + .vfront_porch = { 3, 5, 10 },
> + .vback_porch = { 2, 5, 10 },
> + .vsync_len = { 5, 5, 5 },
> +};
> +
> +static const struct panel_desc koe_tx26d202vm0bwa = {
> + .timings = &koe_tx26d202vm0bwa_timing,
> + .num_timings = 1,
> + .bpc = 8,
> + .size = {
> + .width = 217,
> + .height = 136,
> + },
> + .delay = {
> + .prepare = 1000,
> + .enable = 1000,
> + .unprepare = 1000,
> + .disable = 1000,
Ouch 1s for each delay is huge. Nevertheless it matches the specs so,
the series is:
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Sam, Thierry I assume you'll merge the series. Let me know if I should
pick it up.
-Emil
^ permalink raw reply
* Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check
From: Emil Velikov @ 2020-06-02 12:53 UTC (permalink / raw)
To: Adrian Ratiu
Cc: Philippe CORNU, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org,
Laurent Pinchart, Jernej Skrabec, Adrian Pop, Jonas Karlman,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Yannick FERTRE, Andrzej Hajda, linux-imx@nxp.com,
kernel@collabora.com, linux-stm32@st-md-mailman.stormreply.com,
Arnaud Ferraris, Benjamin GAIGNARD
In-Reply-To: <87blm387vt.fsf@iwork.i-did-not-set--mail-host-address--so-tickle-me>
Hi Adrian,
On Mon, 1 Jun 2020 at 10:14, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>
> On Fri, 29 May 2020, Philippe CORNU <philippe.cornu@st.com> wrote:
> > Hi Adrian, and thank you very much for the patchset. Thank you
> > also for having tested it on STM32F769 and STM32MP1. Sorry for
> > the late response, Yannick and I will review it as soon as
> > possible and we will keep you posted. Note: Do not hesitate to
> > put us in copy for the next version (philippe.cornu@st.com,
> > yannick.fertre@st.com) Regards, Philippe :-)
>
> Hi Philippe,
>
> Thank you very much for your previous and future STM testing,
> really appreciate it! I've CC'd Yannick until now but I'll also CC
> you sure :)
>
> It's been over a month since I posted v8 and I was just gearing up
> to address all feedback, rebase & retest to prepare v9 but I'll
> wait a little longer, no problem, it's no rush.
>
Small idea, pardon for joining so late:
Might be a good idea to add inline comment, why the clocks are disabled so late.
Effectively a 2 line version of the commit summary.
Feel free to make that a separate/follow-up patch.
-Emil
^ permalink raw reply
* Re: [PATCH v2 09/12] iio: imu: inv_icm42600: add buffer support in iio devices
From: Jean-Baptiste Maneyrol @ 2020-06-02 12:57 UTC (permalink / raw)
To: Jonathan Cameron
Cc: robh+dt@kernel.org, robh@kernel.org, mchehab+huawei@kernel.org,
davem@davemloft.net, gregkh@linuxfoundation.org,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20200531135615.7938fb96@archlinux>
Hi Jonathan,
I agree that this multiplexed watermark computation value is anything except simple and clear to understand.
I will add more documentation about it. And it also triggered a kbuild robot issue, because it is using 64 bits modulo without using math64 macros.
For buffer preenable/postenable/..., the sequence I am using currently is:
- preenable -> turn chip on (pm_runtime_get)
- update_scan_mode -> set FIFO en bits configuration (which sensor data is going into the fifo)
- hwfifo_watermark -> compute and set watermark value
- postenable -> turn FIFO on (and multiplexed with a FIFO on counter since used by accel & gyro)
- predisable -> turn FIFO off (multiplexed with counter)
- postdisable -> turn chip off (pm_runtime_put)
This setting is working well. Good to note that if there is an error when enabling the buffer, postdisable will always get called after preenable. So it ensures pm_runtime reference counter to be always OK.
Another way would be to only store configuration in internal state with update_scan_mode and hwfifo_watermark, and do everything in postenable/predisable. This is a possibility, but makes things a little more complex.
For hwfifo flush, this is an interesting feature when there is a need to have data immediately. Or when there is a need to do a clean change of configuration. In Android systems, Android framework is mainly using FIFO flush to change the sensor configuration (ODR, watermark) in a clean way. For our case with the FIFO interleaved this is a not an issue. If there are samples from the 2 sensors, it means the 2 buffers are enabled. And if data is coming to the iio buffer sooner than expected, that should not be a problem. The limitation I see when the 2 sensors are runnings, is that we will return less data than should have been possible. I limit FIFO reading to the provided n bytes, so we could read less than n samples of 1 sensor.
Something I have in mind, that would be really interesting to be able to set/change watermark value when the buffer is enabled. Otherwise, we are always loosing events by turning sensor off when we want to change the value. Is there any limitation to work this way, or should it be possible to implement this feature in the future ?
Thanks,
JB
From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, May 31, 2020 14:56
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Cc: robh+dt@kernel.org <robh+dt@kernel.org>; robh@kernel.org <robh@kernel.org>; mchehab+huawei@kernel.org <mchehab+huawei@kernel.org>; davem@davemloft.net <davem@davemloft.net>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 09/12] iio: imu: inv_icm42600: add buffer support in iio devices
CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
On Wed, 27 May 2020 20:57:08 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Add all FIFO parsing and reading functions. Add accel and gyro
> kfifo buffer and FIFO data parsing. Use device interrupt for
> reading data FIFO and launching accel and gyro parsing.
>
> Support hwfifo watermark by multiplexing gyro and accel settings.
> Support hwfifo flush.
Both of these are complex given the interactions of the two sensors
types and to be honest I couldn't figure out exactly what the intent was.
Needs more docs!
Thanks,
Jonathan
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
> drivers/iio/imu/inv_icm42600/Kconfig | 1 +
> drivers/iio/imu/inv_icm42600/Makefile | 1 +
> drivers/iio/imu/inv_icm42600/inv_icm42600.h | 8 +
> .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 160 ++++-
> .../imu/inv_icm42600/inv_icm42600_buffer.c | 555 ++++++++++++++++++
> .../imu/inv_icm42600/inv_icm42600_buffer.h | 98 ++++
> .../iio/imu/inv_icm42600/inv_icm42600_core.c | 30 +
> .../iio/imu/inv_icm42600/inv_icm42600_gyro.c | 160 ++++-
> 8 files changed, 1011 insertions(+), 2 deletions(-)
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
>
> diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig
> index 22390a72f0a3..50cbcfcb6cf1 100644
> --- a/drivers/iio/imu/inv_icm42600/Kconfig
> +++ b/drivers/iio/imu/inv_icm42600/Kconfig
> @@ -2,6 +2,7 @@
>
> config INV_ICM42600
> tristate
> + select IIO_BUFFER
>
> config INV_ICM42600_I2C
> tristate "InvenSense ICM-426xx I2C driver"
> diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
> index 48965824f00c..0f49f6df3647 100644
> --- a/drivers/iio/imu/inv_icm42600/Makefile
> +++ b/drivers/iio/imu/inv_icm42600/Makefile
> @@ -5,6 +5,7 @@ inv-icm42600-y += inv_icm42600_core.o
> inv-icm42600-y += inv_icm42600_gyro.o
> inv-icm42600-y += inv_icm42600_accel.o
> inv-icm42600-y += inv_icm42600_temp.o
> +inv-icm42600-y += inv_icm42600_buffer.o
>
> obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
> inv-icm42600-i2c-y += inv_icm42600_i2c.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> index 43749f56426c..4d5811562a61 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> @@ -14,6 +14,8 @@
> #include <linux/pm.h>
> #include <linux/iio/iio.h>
>
> +#include "inv_icm42600_buffer.h"
> +
> enum inv_icm42600_chip {
> INV_CHIP_ICM42600,
> INV_CHIP_ICM42602,
> @@ -123,6 +125,7 @@ struct inv_icm42600_suspended {
> * @indio_gyro: gyroscope IIO device.
> * @indio_accel: accelerometer IIO device.
> * @buffer: data transfer buffer aligned for DMA.
> + * @fifo: FIFO management structure.
> */
> struct inv_icm42600_state {
> struct mutex lock;
> @@ -137,6 +140,7 @@ struct inv_icm42600_state {
> struct iio_dev *indio_gyro;
> struct iio_dev *indio_accel;
> uint8_t buffer[2] ____cacheline_aligned;
> + struct inv_icm42600_fifo fifo;
> };
>
> /* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
> @@ -377,6 +381,10 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
>
> int inv_icm42600_gyro_init(struct inv_icm42600_state *st);
>
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev);
> +
> int inv_icm42600_accel_init(struct inv_icm42600_state *st);
>
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev);
> +
> #endif
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> index 6a615d7ffb24..c73ce204efc6 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> @@ -11,9 +11,12 @@
> #include <linux/delay.h>
> #include <linux/math64.h>
> #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
>
> #include "inv_icm42600.h"
> #include "inv_icm42600_temp.h"
> +#include "inv_icm42600_buffer.h"
>
> #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info) \
> { \
> @@ -64,6 +67,76 @@ static const struct iio_chan_spec inv_icm42600_accel_channels[] = {
> INV_ICM42600_TEMP_CHAN(INV_ICM42600_ACCEL_SCAN_TEMP),
> };
>
> +/* IIO buffer data: 8 bytes */
> +struct inv_icm42600_accel_buffer {
> + struct inv_icm42600_fifo_sensor_data accel;
> + int8_t temp;
> + uint8_t padding;
> +};
> +
> +#define INV_ICM42600_SCAN_MASK_ACCEL_3AXIS \
> + (BIT(INV_ICM42600_ACCEL_SCAN_X) | \
> + BIT(INV_ICM42600_ACCEL_SCAN_Y) | \
> + BIT(INV_ICM42600_ACCEL_SCAN_Z))
> +
> +#define INV_ICM42600_SCAN_MASK_TEMP BIT(INV_ICM42600_ACCEL_SCAN_TEMP)
> +
> +static const unsigned long inv_icm42600_accel_scan_masks[] = {
> + /* 3-axis accel + temperature */
> + INV_ICM42600_SCAN_MASK_ACCEL_3AXIS | INV_ICM42600_SCAN_MASK_TEMP,
> + 0,
> +};
> +
> +/* enable accelerometer sensor and FIFO write */
> +static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> + unsigned int fifo_en = 0;
> + unsigned int sleep_temp = 0;
> + unsigned int sleep_accel = 0;
> + unsigned int sleep;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + if (*scan_mask & INV_ICM42600_SCAN_MASK_TEMP) {
> + /* enable temp sensor */
> + ret = inv_icm42600_set_temp_conf(st, true, &sleep_temp);
> + if (ret)
> + goto out_unlock;
> + fifo_en |= INV_ICM42600_SENSOR_TEMP;
> + }
> +
> + if (*scan_mask & INV_ICM42600_SCAN_MASK_ACCEL_3AXIS) {
> + /* enable accel sensor */
> + conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
> + ret = inv_icm42600_set_accel_conf(st, &conf, &sleep_accel);
> + if (ret)
> + goto out_unlock;
> + fifo_en |= INV_ICM42600_SENSOR_ACCEL;
> + }
> +
> + /* update data FIFO write */
> + ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> + if (ret)
> + goto out_unlock;
> +
> + ret = inv_icm42600_buffer_update_watermark(st);
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> + /* sleep maximum required time */
> + if (sleep_accel > sleep_temp)
> + sleep = sleep_accel;
> + else
> + sleep = sleep_temp;
> + if (sleep)
> + msleep(sleep);
> + return ret;
> +}
> +
> static int inv_icm42600_accel_read_sensor(struct inv_icm42600_state *st,
> struct iio_chan_spec const *chan,
> int16_t *val)
> @@ -248,7 +321,12 @@ static int inv_icm42600_accel_write_odr(struct inv_icm42600_state *st,
> mutex_lock(&st->lock);
>
> ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
> + if (ret)
> + goto out_unlock;
> + inv_icm42600_buffer_update_fifo_period(st);
> + inv_icm42600_buffer_update_watermark(st);
>
> +out_unlock:
> mutex_unlock(&st->lock);
> pm_runtime_mark_last_busy(dev);
> pm_runtime_put_autosuspend(dev);
> @@ -563,12 +641,51 @@ static int inv_icm42600_accel_write_raw_get_fmt(struct iio_dev *indio_dev,
> }
> }
>
> +static int inv_icm42600_accel_hwfifo_set_watermark(struct iio_dev *indio_dev,
> + unsigned int val)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + st->fifo.watermark.accel = val;
> + ret = inv_icm42600_buffer_update_watermark(st);
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int inv_icm42600_accel_hwfifo_flush(struct iio_dev *indio_dev,
> + unsigned int count)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + if (count == 0)
> + return 0;
> +
> + mutex_lock(&st->lock);
> +
> + ret = inv_icm42600_buffer_hwfifo_flush(st, count);
> + if (!ret)
> + ret = st->fifo.nb.accel;
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> static const struct iio_info inv_icm42600_accel_info = {
> .read_raw = inv_icm42600_accel_read_raw,
> .read_avail = inv_icm42600_accel_read_avail,
> .write_raw = inv_icm42600_accel_write_raw,
> .write_raw_get_fmt = inv_icm42600_accel_write_raw_get_fmt,
> .debugfs_reg_access = inv_icm42600_debugfs_reg,
> + .update_scan_mode = inv_icm42600_accel_update_scan_mode,
> + .hwfifo_set_watermark = inv_icm42600_accel_hwfifo_set_watermark,
> + .hwfifo_flush_to_buffer = inv_icm42600_accel_hwfifo_flush,
> };
>
> int inv_icm42600_accel_init(struct inv_icm42600_state *st)
> @@ -576,6 +693,7 @@ int inv_icm42600_accel_init(struct inv_icm42600_state *st)
> struct device *dev = regmap_get_device(st->map);
> const char *name;
> struct iio_dev *indio_dev;
> + struct iio_buffer *buffer;
>
> name = devm_kasprintf(dev, GFP_KERNEL, "%s-accel", st->name);
> if (!name)
> @@ -585,14 +703,54 @@ int inv_icm42600_accel_init(struct inv_icm42600_state *st)
> if (!indio_dev)
> return -ENOMEM;
>
> + buffer = devm_iio_kfifo_allocate(dev);
> + if (!buffer)
> + return -ENOMEM;
> +
> iio_device_set_drvdata(indio_dev, st);
> indio_dev->dev.parent = dev;
> indio_dev->name = name;
> indio_dev->info = &inv_icm42600_accel_info;
> - indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> indio_dev->channels = inv_icm42600_accel_channels;
> indio_dev->num_channels = ARRAY_SIZE(inv_icm42600_accel_channels);
> + indio_dev->available_scan_masks = inv_icm42600_accel_scan_masks;
> + indio_dev->setup_ops = &inv_icm42600_buffer_ops;
> +
> + iio_device_attach_buffer(indio_dev, buffer);
>
> st->indio_accel = indio_dev;
> return devm_iio_device_register(dev, st->indio_accel);
> }
> +
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + ssize_t i, size;
> + const void *accel, *gyro, *timestamp;
> + const int8_t *temp;
> + unsigned int odr;
> + struct inv_icm42600_accel_buffer buffer = {
> + .padding = 0,
> + };
> +
> + /* parse all fifo packets */
> + for (i = 0; i < st->fifo.count; i += size) {
> + size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> + &accel, &gyro, &temp, ×tamp, &odr);
> + /* quit if error or FIFO is empty */
> + if (size <= 0)
> + return size;
> +
> + /* skip packet if no accel data or data is invalid */
> + if (accel == NULL || !inv_icm42600_fifo_is_data_valid(accel))
> + continue;
> +
> + /* fill and push data buffer */
> + memcpy(&buffer.accel, accel, sizeof(buffer.accel));
> + buffer.temp = temp ? *temp : 0;
> + iio_push_to_buffers(indio_dev, &buffer);
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> new file mode 100644
> index 000000000000..c91075f62231
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -0,0 +1,555 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/delay.h>
> +#include <linux/math64.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +
> +#include "inv_icm42600.h"
> +#include "inv_icm42600_buffer.h"
> +
> +/* FIFO header: 1 byte */
> +#define INV_ICM42600_FIFO_HEADER_MSG BIT(7)
> +#define INV_ICM42600_FIFO_HEADER_ACCEL BIT(6)
> +#define INV_ICM42600_FIFO_HEADER_GYRO BIT(5)
> +#define INV_ICM42600_FIFO_HEADER_TMST_FSYNC GENMASK(3, 2)
> +#define INV_ICM42600_FIFO_HEADER_ODR_ACCEL BIT(1)
> +#define INV_ICM42600_FIFO_HEADER_ODR_GYRO BIT(0)
> +
> +struct inv_icm42600_fifo_1sensor_packet {
> + uint8_t header;
> + struct inv_icm42600_fifo_sensor_data data;
> + int8_t temp;
> +} __packed;
> +#define INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE 8
> +
> +struct inv_icm42600_fifo_2sensors_packet {
> + uint8_t header;
> + struct inv_icm42600_fifo_sensor_data accel;
> + struct inv_icm42600_fifo_sensor_data gyro;
> + int8_t temp;
> + __be16 timestamp;
> +} __packed;
> +#define INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE 16
> +
> +ssize_t inv_icm42600_fifo_decode_packet(const void *packet, const void **accel,
> + const void **gyro, const int8_t **temp,
> + const void **timestamp, unsigned int *odr)
> +{
> + const struct inv_icm42600_fifo_1sensor_packet *pack1 = packet;
> + const struct inv_icm42600_fifo_2sensors_packet *pack2 = packet;
> + uint8_t header = *((const uint8_t *)packet);
> +
> + /* FIFO empty */
> + if (header & INV_ICM42600_FIFO_HEADER_MSG) {
> + *accel = NULL;
> + *gyro = NULL;
> + *temp = NULL;
> + *timestamp = NULL;
> + *odr = 0;
> + return 0;
> + }
> +
> + /* handle odr flags */
> + *odr = 0;
> + if (header & INV_ICM42600_FIFO_HEADER_ODR_GYRO)
> + *odr |= INV_ICM42600_SENSOR_GYRO;
> + if (header & INV_ICM42600_FIFO_HEADER_ODR_ACCEL)
> + *odr |= INV_ICM42600_SENSOR_ACCEL;
> +
> + /* accel + gyro */
> + if ((header & INV_ICM42600_FIFO_HEADER_ACCEL) &&
> + (header & INV_ICM42600_FIFO_HEADER_GYRO)) {
> + *accel = &pack2->accel;
> + *gyro = &pack2->gyro;
> + *temp = &pack2->temp;
> + *timestamp = &pack2->timestamp;
> + return INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
> + }
> +
> + /* accel only */
> + if (header & INV_ICM42600_FIFO_HEADER_ACCEL) {
> + *accel = &pack1->data;
> + *gyro = NULL;
> + *temp = &pack1->temp;
> + *timestamp = NULL;
> + return INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE;
> + }
> +
> + /* gyro only */
> + if (header & INV_ICM42600_FIFO_HEADER_GYRO) {
> + *accel = NULL;
> + *gyro = &pack1->data;
> + *temp = &pack1->temp;
> + *timestamp = NULL;
> + return INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE;
> + }
> +
> + /* invalid packet if here */
> + return -EINVAL;
> +}
> +
> +void inv_icm42600_buffer_update_fifo_period(struct inv_icm42600_state *st)
> +{
> + uint32_t period_gyro, period_accel, period;
> +
> + if (st->fifo.en & INV_ICM42600_SENSOR_GYRO)
> + period_gyro = inv_icm42600_odr_to_period(st->conf.gyro.odr);
> + else
> + period_gyro = U32_MAX;
> +
> + if (st->fifo.en & INV_ICM42600_SENSOR_ACCEL)
> + period_accel = inv_icm42600_odr_to_period(st->conf.accel.odr);
> + else
> + period_accel = U32_MAX;
> +
> + if (period_gyro <= period_accel)
> + period = period_gyro;
> + else
> + period = period_accel;
> +
> + st->fifo.period = period;
> +}
> +
> +int inv_icm42600_buffer_set_fifo_en(struct inv_icm42600_state *st,
> + unsigned int fifo_en)
> +{
> + unsigned int mask, val;
> + int ret;
> +
> + /* update only FIFO EN bits */
> + mask = INV_ICM42600_FIFO_CONFIG1_TMST_FSYNC_EN |
> + INV_ICM42600_FIFO_CONFIG1_TEMP_EN |
> + INV_ICM42600_FIFO_CONFIG1_GYRO_EN |
> + INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
> +
> + val = 0;
> + if (fifo_en & INV_ICM42600_SENSOR_GYRO)
> + val |= INV_ICM42600_FIFO_CONFIG1_GYRO_EN;
> + if (fifo_en & INV_ICM42600_SENSOR_ACCEL)
> + val |= INV_ICM42600_FIFO_CONFIG1_ACCEL_EN;
> + if (fifo_en & INV_ICM42600_SENSOR_TEMP)
> + val |= INV_ICM42600_FIFO_CONFIG1_TEMP_EN;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_FIFO_CONFIG1,
> + mask, val);
> + if (ret)
> + return ret;
> +
> + st->fifo.en = fifo_en;
> + inv_icm42600_buffer_update_fifo_period(st);
> +
> + return 0;
> +}
> +
> +static size_t inv_icm42600_get_packet_size(unsigned int fifo_en)
> +{
> + size_t packet_size;
> +
> + if ((fifo_en & INV_ICM42600_SENSOR_GYRO) &&
> + (fifo_en & INV_ICM42600_SENSOR_ACCEL))
> + packet_size = INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
> + else
> + packet_size = INV_ICM42600_FIFO_1SENSOR_PACKET_SIZE;
> +
> + return packet_size;
> +}
> +
> +static unsigned int inv_icm42600_wm_truncate(unsigned int watermark,
> + size_t packet_size)
> +{
> + size_t wm_size;
> + unsigned int wm;
> +
> + wm_size = watermark * packet_size;
> + if (wm_size > INV_ICM42600_FIFO_WATERMARK_MAX)
> + wm_size = INV_ICM42600_FIFO_WATERMARK_MAX;
> +
> + wm = wm_size / packet_size;
> +
> + return wm;
> +}
> +
I think some overview docs on how this is working would be good.
Set out the aims for the watermark selected and how it is achieved.
> +int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st)
> +{
> + size_t packet_size, wm_size;
> + unsigned int wm_gyro, wm_accel, watermark;
> + uint32_t period_gyro, period_accel, period;
> + int64_t latency_gyro, latency_accel, latency;
> + bool restore;
> + __le16 raw_wm;
> + int ret;
> +
> + packet_size = inv_icm42600_get_packet_size(st->fifo.en);
> +
> + /* get minimal latency, depending on sensor watermark and odr */
> + wm_gyro = inv_icm42600_wm_truncate(st->fifo.watermark.gyro,
> + packet_size);
> + wm_accel = inv_icm42600_wm_truncate(st->fifo.watermark.accel,
> + packet_size);
> + period_gyro = inv_icm42600_odr_to_period(st->conf.gyro.odr);
> + period_accel = inv_icm42600_odr_to_period(st->conf.accel.odr);
> + latency_gyro = (int64_t)period_gyro * (int64_t)wm_gyro;
> + latency_accel = (int64_t)period_accel * (int64_t)wm_accel;
> + if (latency_gyro == 0) {
> + latency = latency_accel;
> + watermark = wm_accel;
> + } else if (latency_accel == 0) {
> + latency = latency_gyro;
> + watermark = wm_gyro;
> + } else {
> + /* compute the smallest latency that is a multiple of both */
> + if (latency_gyro <= latency_accel) {
> + latency = latency_gyro;
> + latency -= latency_accel % latency_gyro;
> + } else {
> + latency = latency_accel;
> + latency -= latency_gyro % latency_accel;
> + }
> + /* use the shortest period */
> + if (period_gyro <= period_accel)
> + period = period_gyro;
> + else
> + period = period_accel;
> + /* all this works because periods are multiple of each others */
> + watermark = div_s64(latency, period);
> + if (watermark < 1)
> + watermark = 1;
> + }
> + wm_size = watermark * packet_size;
> +
> + /* changing FIFO watermark requires to turn off watermark interrupt */
> + ret = regmap_update_bits_check(st->map, INV_ICM42600_REG_INT_SOURCE0,
> + INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN,
> + 0, &restore);
> + if (ret)
> + return ret;
> +
> + raw_wm = INV_ICM42600_FIFO_WATERMARK_VAL(wm_size);
> + memcpy(st->buffer, &raw_wm, sizeof(raw_wm));
> + ret = regmap_bulk_write(st->map, INV_ICM42600_REG_FIFO_WATERMARK,
> + st->buffer, sizeof(raw_wm));
> + if (ret)
> + return ret;
> +
> + /* restore watermark interrupt */
> + if (restore) {
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
> + INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN,
> + INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct device *dev = regmap_get_device(st->map);
> +
> + pm_runtime_get_sync(dev);
> +
> + return 0;
> +}
> +
> +/*
> + * update_scan_mode callback is turning sensors on and setting data FIFO enable
> + * bits.
> + */
> +static int inv_icm42600_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + /* exit if FIFO is already on */
> + if (st->fifo.on) {
> + ret = 0;
> + goto out_on;
> + }
> +
> + /* set FIFO threshold interrupt */
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
> + INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN,
> + INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN);
> + if (ret)
> + goto out_unlock;
> +
> + /* flush FIFO data */
> + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
> + INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH);
> + if (ret)
> + goto out_unlock;
> +
> + /* set FIFO in streaming mode */
> + ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> + INV_ICM42600_FIFO_CONFIG_STREAM);
> + if (ret)
> + goto out_unlock;
> +
> + /* workaround: first read of FIFO count after reset is always 0 */
> + ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT, st->buffer, 2);
> + if (ret)
> + goto out_unlock;
> +
> +out_on:
> + /* increase FIFO on counter */
> + st->fifo.on++;
> +out_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int inv_icm42600_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + /* exit if there are several sensors using the FIFO */
> + if (st->fifo.on > 1) {
> + ret = 0;
> + goto out_off;
> + }
> +
> + /* set FIFO in bypass mode */
> + ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> + INV_ICM42600_FIFO_CONFIG_BYPASS);
> + if (ret)
> + goto out_unlock;
> +
> + /* flush FIFO data */
> + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
> + INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH);
> + if (ret)
> + goto out_unlock;
> +
> + /* disable FIFO threshold interrupt */
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INT_SOURCE0,
> + INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN, 0);
> + if (ret)
> + goto out_unlock;
> +
> +out_off:
> + /* decrease FIFO on counter */
> + st->fifo.on--;
> +out_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct device *dev = regmap_get_device(st->map);
> + unsigned int sensor;
> + unsigned int *watermark;
> + struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> + unsigned int sleep_temp = 0;
> + unsigned int sleep_sensor = 0;
> + unsigned int sleep;
> + int ret;
> +
> + if (indio_dev == st->indio_gyro) {
> + sensor = INV_ICM42600_SENSOR_GYRO;
> + watermark = &st->fifo.watermark.gyro;
> + } else if (indio_dev == st->indio_accel) {
> + sensor = INV_ICM42600_SENSOR_ACCEL;
> + watermark = &st->fifo.watermark.accel;
> + } else {
> + return -EINVAL;
> + }
> +
> + mutex_lock(&st->lock);
> +
> + ret = inv_icm42600_buffer_set_fifo_en(st, st->fifo.en & ~sensor);
> + if (ret)
> + goto out_unlock;
> +
> + *watermark = 0;
> + ret = inv_icm42600_buffer_update_watermark(st);
> + if (ret)
> + goto out_unlock;
> +
> + conf.mode = INV_ICM42600_SENSOR_MODE_OFF;
> + if (sensor == INV_ICM42600_SENSOR_GYRO)
> + ret = inv_icm42600_set_gyro_conf(st, &conf, &sleep_sensor);
> + else
> + ret = inv_icm42600_set_accel_conf(st, &conf, &sleep_sensor);
> + if (ret)
> + goto out_unlock;
> +
> + /* if FIFO is off, turn temperature off */
> + if (!st->fifo.on)
> + ret = inv_icm42600_set_temp_conf(st, false, &sleep_temp);
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> +
> + /* sleep maximum required time */
> + if (sleep_sensor > sleep_temp)
> + sleep = sleep_sensor;
> + else
> + sleep = sleep_temp;
> + if (sleep)
> + msleep(sleep);
> +
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> + return ret;
> +}
> +
> +const struct iio_buffer_setup_ops inv_icm42600_buffer_ops = {
> + .preenable = inv_icm42600_buffer_preenable,
> + .postenable = inv_icm42600_buffer_postenable,
We've been slowly eroding the difference between preenable and posteenable.
Would be good to understand why you need to define both?
> + .predisable = inv_icm42600_buffer_predisable,
> + .postdisable = inv_icm42600_buffer_postdisable,
> +};
> +
> +int inv_icm42600_buffer_fifo_read(struct inv_icm42600_state *st,
> + unsigned int max)
> +{
> + size_t max_count;
> + __be16 *raw_fifo_count;
> + ssize_t i, size;
> + const void *accel, *gyro, *timestamp;
> + const int8_t *temp;
> + unsigned int odr;
> + int ret;
> +
> + /* reset all samples counters */
> + st->fifo.count = 0;
> + st->fifo.nb.gyro = 0;
> + st->fifo.nb.accel = 0;
> + st->fifo.nb.total = 0;
> +
> + /* compute maximum FIFO read size */
> + if (max == 0)
> + max_count = sizeof(st->fifo.data);
> + else
> + max_count = max * inv_icm42600_get_packet_size(st->fifo.en);
> +
> + /* read FIFO count value */
> + raw_fifo_count = (__be16 *)st->buffer;
> + ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT,
> + raw_fifo_count, sizeof(*raw_fifo_count));
> + if (ret)
> + return ret;
> + st->fifo.count = be16_to_cpup(raw_fifo_count);
> +
> + /* check and clamp FIFO count value */
> + if (st->fifo.count == 0)
> + return 0;
> + if (st->fifo.count > max_count)
> + st->fifo.count = max_count;
> +
> + /* read all FIFO data in internal buffer */
> + ret = regmap_noinc_read(st->map, INV_ICM42600_REG_FIFO_DATA,
> + st->fifo.data, st->fifo.count);
> + if (ret)
> + return ret;
> +
> + /* compute number of samples for each sensor */
> + for (i = 0; i < st->fifo.count; i += size) {
> + size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> + &accel, &gyro, &temp, ×tamp, &odr);
> + if (size <= 0)
> + break;
> + if (gyro != NULL && inv_icm42600_fifo_is_data_valid(gyro))
> + st->fifo.nb.gyro++;
> + if (accel != NULL && inv_icm42600_fifo_is_data_valid(accel))
> + st->fifo.nb.accel++;
> + st->fifo.nb.total++;
> + }
> +
> + return 0;
> +}
> +
> +int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
> +{
> + int ret;
> +
> + if (st->fifo.nb.total == 0)
> + return 0;
> +
> + if (st->fifo.nb.gyro > 0) {
> + ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
> + if (ret)
> + return ret;
> + }
> +
> + if (st->fifo.nb.accel > 0) {
> + ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> + unsigned int count)
> +{
> + int ret;
> +
> + ret = inv_icm42600_buffer_fifo_read(st, count);
> + if (ret)
> + return ret;
Definitely searching my memory for how this works in the core, so
I may have it wrong.
This is a bit unusual (I think). The intent of the flush
is to read up to 'n' bytes because someone just did a read on the buffer
or select, and there was data in the hwfifo capable of satisfying the read
even though we haven't yet reached the watermark.
Given both sensor types are coming from one buffer, do we have a potential
issue here or under serving even though data is available?
The case I worry may be served late is when an poll / select
is waiting for sufficient data.
So what should we be doing? We want to guarantee to provide data
for each sensor type if it's in the hwfifo. As such we could keep reading
until we have enough, but that could cause some issues if the two data rates
are very different (overflow on the other kfifo)
Maybe what you have here is the best we can do.
I'm assuming the watermark level has a similar problem. One value represents
the sum of the two types of data.
> +
> + if (st->fifo.nb.total == 0)
> + return 0;
> +
> + if (st->fifo.nb.gyro > 0) {
> + ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
> + if (ret)
> + return ret;
> + }
> +
> + if (st->fifo.nb.accel > 0) {
> + ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int inv_icm42600_buffer_init(struct inv_icm42600_state *st)
> +{
> + unsigned int val;
> + int ret;
> +
> + /*
> + * Default FIFO configuration (bits 7 to 5)
> + * - use invalid value
> + * - FIFO count in bytes
> + * - FIFO count in big endian
> + */
> + val = INV_ICM42600_INTF_CONFIG0_FIFO_COUNT_ENDIAN;
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG0,
> + GENMASK(7, 5), val);
> + if (ret)
> + return ret;
> +
> + /*
> + * Enable FIFO partial read and continuous watermark interrupt.
> + * Disable all FIFO EN bits.
> + */
> + val = INV_ICM42600_FIFO_CONFIG1_RESUME_PARTIAL_RD |
> + INV_ICM42600_FIFO_CONFIG1_WM_GT_TH;
> + return regmap_update_bits(st->map, INV_ICM42600_REG_FIFO_CONFIG1,
> + GENMASK(6, 5) | GENMASK(3, 0), val);
> +}
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> new file mode 100644
> index 000000000000..de2a3949dcc7
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#ifndef INV_ICM42600_BUFFER_H_
> +#define INV_ICM42600_BUFFER_H_
> +
> +#include <linux/kernel.h>
> +#include <linux/bits.h>
> +
> +struct inv_icm42600_state;
> +
> +#define INV_ICM42600_SENSOR_GYRO BIT(0)
> +#define INV_ICM42600_SENSOR_ACCEL BIT(1)
> +#define INV_ICM42600_SENSOR_TEMP BIT(2)
> +
> +/**
> + * struct inv_icm42600_fifo - FIFO state variables
> + * @on: reference counter for FIFO on.
> + * @en: bits field of INV_ICM42600_SENSOR_* for FIFO EN bits.
> + * @period: FIFO internal period.
> + * @watermark: watermark configuration values for accel and gyro.
> + * @count: number of bytes in the FIFO data buffer.
> + * @nb: gyro, accel and total samples in the FIFO data buffer.
> + * @data: FIFO data buffer aligned for DMA (2kB + 32 bytes of read cache).
> + */
> +struct inv_icm42600_fifo {
> + unsigned int on;
> + unsigned int en;
> + uint32_t period;
> + struct {
> + unsigned int gyro;
> + unsigned int accel;
> + } watermark;
> + size_t count;
> + struct {
> + size_t gyro;
> + size_t accel;
> + size_t total;
> + } nb;
> + uint8_t data[2080] ____cacheline_aligned;
> +};
> +
> +/* FIFO data packet */
> +struct inv_icm42600_fifo_sensor_data {
> + __be16 x;
> + __be16 y;
> + __be16 z;
> +} __packed;
Why packed? Should be anyway I think.
> +#define INV_ICM42600_FIFO_DATA_INVALID -32768
> +
> +static inline int16_t inv_icm42600_fifo_get_sensor_data(__be16 d)
> +{
> + return be16_to_cpu(d);
> +}
> +
> +static inline bool
> +inv_icm42600_fifo_is_data_valid(const struct inv_icm42600_fifo_sensor_data *s)
> +{
> + int16_t x, y, z;
> +
> + x = inv_icm42600_fifo_get_sensor_data(s->x);
> + y = inv_icm42600_fifo_get_sensor_data(s->y);
> + z = inv_icm42600_fifo_get_sensor_data(s->z);
> +
> + if (x == INV_ICM42600_FIFO_DATA_INVALID &&
> + y == INV_ICM42600_FIFO_DATA_INVALID &&
> + z == INV_ICM42600_FIFO_DATA_INVALID)
> + return false;
> +
> + return true;
> +}
> +
> +ssize_t inv_icm42600_fifo_decode_packet(const void *packet, const void **accel,
> + const void **gyro, const int8_t **temp,
> + const void **timestamp, unsigned int *odr);
> +
> +extern const struct iio_buffer_setup_ops inv_icm42600_buffer_ops;
> +
> +int inv_icm42600_buffer_init(struct inv_icm42600_state *st);
> +
> +void inv_icm42600_buffer_update_fifo_period(struct inv_icm42600_state *st);
> +
> +int inv_icm42600_buffer_set_fifo_en(struct inv_icm42600_state *st,
> + unsigned int fifo_en);
> +
> +int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st);
> +
> +int inv_icm42600_buffer_fifo_read(struct inv_icm42600_state *st,
> + unsigned int max);
> +
> +int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st);
> +
> +int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> + unsigned int count);
> +
> +#endif
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index 246c1eb52231..6f1c1eb83953 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -18,6 +18,7 @@
> #include <linux/iio/iio.h>
>
> #include "inv_icm42600.h"
> +#include "inv_icm42600_buffer.h"
>
> static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
> {
> @@ -429,6 +430,18 @@ static irqreturn_t inv_icm42600_irq_handler(int irq, void *_data)
> if (status & INV_ICM42600_INT_STATUS_FIFO_FULL)
> dev_warn(dev, "FIFO full data lost!\n");
>
> + /* FIFO threshold reached */
> + if (status & INV_ICM42600_INT_STATUS_FIFO_THS) {
> + ret = inv_icm42600_buffer_fifo_read(st, 0);
> + if (ret) {
> + dev_err(dev, "FIFO read error %d\n", ret);
> + goto out_unlock;
> + }
> + ret = inv_icm42600_buffer_fifo_parse(st);
> + if (ret)
> + dev_err(dev, "FIFO parsing error %d\n", ret);
> + }
> +
> out_unlock:
> mutex_unlock(&st->lock);
> return IRQ_HANDLED;
> @@ -600,6 +613,10 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> if (ret)
> return ret;
>
> + ret = inv_icm42600_buffer_init(st);
> + if (ret)
> + return ret;
> +
> ret = inv_icm42600_gyro_init(st);
> if (ret)
> return ret;
> @@ -645,6 +662,14 @@ static int __maybe_unused inv_icm42600_suspend(struct device *dev)
> goto out_unlock;
> }
>
> + /* disable FIFO data streaming */
> + if (st->fifo.on) {
> + ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> + INV_ICM42600_FIFO_CONFIG_BYPASS);
> + if (ret)
> + goto out_unlock;
> + }
> +
> ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
> INV_ICM42600_SENSOR_MODE_OFF, false,
> NULL);
> @@ -684,6 +709,11 @@ static int __maybe_unused inv_icm42600_resume(struct device *dev)
> if (ret)
> goto out_unlock;
>
> + /* restore FIFO data streaming */
> + if (st->fifo.on)
> + ret = regmap_write(st->map, INV_ICM42600_REG_FIFO_CONFIG,
> + INV_ICM42600_FIFO_CONFIG_STREAM);
> +
> out_unlock:
> mutex_unlock(&st->lock);
> return ret;
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> index 38654e0d217b..b05c33876b8d 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> @@ -11,9 +11,12 @@
> #include <linux/delay.h>
> #include <linux/math64.h>
> #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
>
> #include "inv_icm42600.h"
> #include "inv_icm42600_temp.h"
> +#include "inv_icm42600_buffer.h"
>
> #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info) \
> { \
> @@ -64,6 +67,76 @@ static const struct iio_chan_spec inv_icm42600_gyro_channels[] = {
> INV_ICM42600_TEMP_CHAN(INV_ICM42600_GYRO_SCAN_TEMP),
> };
>
> +/* IIO buffer data: 8 bytes */
> +struct inv_icm42600_gyro_buffer {
> + struct inv_icm42600_fifo_sensor_data gyro;
> + int8_t temp;
> + uint8_t padding;
> +};
> +
> +#define INV_ICM42600_SCAN_MASK_GYRO_3AXIS \
> + (BIT(INV_ICM42600_GYRO_SCAN_X) | \
> + BIT(INV_ICM42600_GYRO_SCAN_Y) | \
> + BIT(INV_ICM42600_GYRO_SCAN_Z))
> +
> +#define INV_ICM42600_SCAN_MASK_TEMP BIT(INV_ICM42600_GYRO_SCAN_TEMP)
> +
> +static const unsigned long inv_icm42600_gyro_scan_masks[] = {
> + /* 3-axis gyro + temperature */
> + INV_ICM42600_SCAN_MASK_GYRO_3AXIS | INV_ICM42600_SCAN_MASK_TEMP,
> + 0,
> +};
> +
> +/* enable gyroscope sensor and FIFO write */
> +static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> + unsigned int fifo_en = 0;
> + unsigned int sleep_gyro = 0;
> + unsigned int sleep_temp = 0;
> + unsigned int sleep;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + if (*scan_mask & INV_ICM42600_SCAN_MASK_TEMP) {
> + /* enable temp sensor */
> + ret = inv_icm42600_set_temp_conf(st, true, &sleep_temp);
> + if (ret)
> + goto out_unlock;
> + fifo_en |= INV_ICM42600_SENSOR_TEMP;
> + }
> +
> + if (*scan_mask & INV_ICM42600_SCAN_MASK_GYRO_3AXIS) {
> + /* enable gyro sensor */
> + conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
> + ret = inv_icm42600_set_gyro_conf(st, &conf, &sleep_gyro);
> + if (ret)
> + goto out_unlock;
> + fifo_en |= INV_ICM42600_SENSOR_GYRO;
> + }
> +
> + /* update data FIFO write */
> + ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> + if (ret)
> + goto out_unlock;
> +
> + ret = inv_icm42600_buffer_update_watermark(st);
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> + /* sleep maximum required time */
> + if (sleep_gyro > sleep_temp)
> + sleep = sleep_gyro;
> + else
> + sleep = sleep_temp;
> + if (sleep)
> + msleep(sleep);
> + return ret;
> +}
> +
> static int inv_icm42600_gyro_read_sensor(struct inv_icm42600_state *st,
> struct iio_chan_spec const *chan,
> int16_t *val)
> @@ -260,7 +333,12 @@ static int inv_icm42600_gyro_write_odr(struct inv_icm42600_state *st,
> mutex_lock(&st->lock);
>
> ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
> + if (ret)
> + goto out_unlock;
> + inv_icm42600_buffer_update_fifo_period(st);
> + inv_icm42600_buffer_update_watermark(st);
>
> +out_unlock:
> mutex_unlock(&st->lock);
> pm_runtime_mark_last_busy(dev);
> pm_runtime_put_autosuspend(dev);
> @@ -574,12 +652,51 @@ static int inv_icm42600_gyro_write_raw_get_fmt(struct iio_dev *indio_dev,
> }
> }
>
> +static int inv_icm42600_gyro_hwfifo_set_watermark(struct iio_dev *indio_dev,
> + unsigned int val)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + st->fifo.watermark.gyro = val;
> + ret = inv_icm42600_buffer_update_watermark(st);
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int inv_icm42600_gyro_hwfifo_flush(struct iio_dev *indio_dev,
> + unsigned int count)
> +{
Nothing to do with this patch, but I realised reading this that we have
some 'unusual' use of the word flush here. It's a straight forward
read function so not sure why we called it flush.
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + if (count == 0)
> + return 0;
> +
> + mutex_lock(&st->lock);
> +
> + ret = inv_icm42600_buffer_hwfifo_flush(st, count);
> + if (!ret)
> + ret = st->fifo.nb.gyro;
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> static const struct iio_info inv_icm42600_gyro_info = {
> .read_raw = inv_icm42600_gyro_read_raw,
> .read_avail = inv_icm42600_gyro_read_avail,
> .write_raw = inv_icm42600_gyro_write_raw,
> .write_raw_get_fmt = inv_icm42600_gyro_write_raw_get_fmt,
> .debugfs_reg_access = inv_icm42600_debugfs_reg,
> + .update_scan_mode = inv_icm42600_gyro_update_scan_mode,
> + .hwfifo_set_watermark = inv_icm42600_gyro_hwfifo_set_watermark,
> + .hwfifo_flush_to_buffer = inv_icm42600_gyro_hwfifo_flush,
> };
>
> int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
> @@ -587,6 +704,7 @@ int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
> struct device *dev = regmap_get_device(st->map);
> const char *name;
> struct iio_dev *indio_dev;
> + struct iio_buffer *buffer;
>
> name = devm_kasprintf(dev, GFP_KERNEL, "%s-gyro", st->name);
> if (!name)
> @@ -596,14 +714,54 @@ int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
> if (!indio_dev)
> return -ENOMEM;
>
> + buffer = devm_iio_kfifo_allocate(dev);
> + if (!buffer)
> + return -ENOMEM;
> +
> iio_device_set_drvdata(indio_dev, st);
> indio_dev->dev.parent = dev;
> indio_dev->name = name;
> indio_dev->info = &inv_icm42600_gyro_info;
> - indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> indio_dev->channels = inv_icm42600_gyro_channels;
> indio_dev->num_channels = ARRAY_SIZE(inv_icm42600_gyro_channels);
> + indio_dev->available_scan_masks = inv_icm42600_gyro_scan_masks;
> + indio_dev->setup_ops = &inv_icm42600_buffer_ops;
> +
> + iio_device_attach_buffer(indio_dev, buffer);
>
> st->indio_gyro = indio_dev;
> return devm_iio_device_register(dev, st->indio_gyro);
> }
> +
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + ssize_t i, size;
> + const void *accel, *gyro, *timestamp;
> + const int8_t *temp;
> + unsigned int odr;
> + struct inv_icm42600_gyro_buffer buffer = {
> + .padding = 0,
Might be worth a comment here or where the structure is defined
on why we make padding explicit.
> + };
> +
> + /* parse all fifo packets */
> + for (i = 0; i < st->fifo.count; i += size) {
> + size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> + &accel, &gyro, &temp, ×tamp, &odr);
> + /* quit if error or FIFO is empty */
> + if (size <= 0)
> + return size;
> +
> + /* skip packet if no gyro data or data is invalid */
> + if (gyro == NULL || !inv_icm42600_fifo_is_data_valid(gyro))
> + continue;
> +
> + /* fill and push data buffer */
> + memcpy(&buffer.gyro, gyro, sizeof(buffer.gyro));
> + buffer.temp = temp ? *temp : 0;
> + iio_push_to_buffers(indio_dev, &buffer);
> + }
> +
> + return 0;
> +}
^ permalink raw reply
* Re: Security Random Number Generator support
From: Marc Zyngier @ 2020-06-02 13:02 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Neal Liu,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman, Sean Wang, lkml,
wsd_upstream, Rob Herring, linux-mediatek,
Linux Crypto Mailing List, Matt Mackall, Matthias Brugger,
Crystal Guo, Linux ARM
In-Reply-To: <CAMj1kXHjAdk5=-uSh_=S9j5cz42zr3h6t+YYGy+obevuQDp0fg@mail.gmail.com>
On 2020-06-02 13:14, Ard Biesheuvel wrote:
> On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
>>
>> These patch series introduce a security random number generator
>> which provides a generic interface to get hardware rnd from Secure
>> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
>> Execution Environment(TEE), or even EL2 hypervisor.
>>
>> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
>> For security awareness SoCs on ARMv8 with TrustZone enabled,
>> peripherals like entropy sources is not accessible from normal world
>> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
>> This driver aims to provide a generic interface to Arm Trusted
>> Firmware or Hypervisor rng service.
>>
>>
>> changes since v1:
>> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can
>> reuse
>> this driver.
>> - refine coding style and unnecessary check.
>>
>> changes since v2:
>> - remove unused comments.
>> - remove redundant variable.
>>
>> changes since v3:
>> - add dt-bindings for MediaTek rng with TrustZone enabled.
>> - revise HWRNG SMC call fid.
>>
>> changes since v4:
>> - move bindings to the arm/firmware directory.
>> - revise driver init flow to check more property.
>>
>> changes since v5:
>> - refactor to more generic security rng driver which
>> is not platform specific.
>>
>> *** BLURB HERE ***
>>
>> Neal Liu (2):
>> dt-bindings: rng: add bindings for sec-rng
>> hwrng: add sec-rng driver
>>
>
> There is no reason to model a SMC call as a driver, and represent it
> via a DT node like this.
+1.
> It would be much better if this SMC interface is made truly generic,
> and wired into the arch_get_random() interface, which can be used much
> earlier.
Wasn't there a plan to standardize a SMC call to rule them all?
M.
--
Who you jivin' with that Cosmik Debris?
^ permalink raw reply
* [PATCH] ARM: dts: AM33xx-l4: add gpio-ranges
From: Drew Fustini @ 2020-06-02 13:14 UTC (permalink / raw)
To: Tony Lindgren, linux-omap, linux-kernel, Benoît Cousson,
Rob Herring, devicetree
Cc: Santosh Shilimkar, Suman Anna, Grygorii Strashko, Haojian Zhuang,
Linus Walleij, linux-gpio
Add gpio-ranges properties to the gpio controller nodes.
These gpio-ranges were created based on "Table 9-10. CONTROL_MODULE
REGISTERS" in the "AM335x Technical Reference Manual" [0] and "Table
4-2. Pin Attributes" in the "AM335x Sitara Processor datasheet" [1].
A csv file with this data is available for reference [2].
[0] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf
[1] http://www.ti.com/lit/ds/symlink/am3358.pdf
[2] https://gist.github.com/pdp7/6ffaddc8867973c1c3e8612cfaf72020
Signed-off-by: Drew Fustini <drew@beagleboard.org>
---
Notes:
There was previous dicussion relevant to this patch:
https://lore.kernel.org/linux-gpio/20200525131731.GA948395@x1/
Here is the list I compiled which shows the mapping between a gpioline
and the pin number including the memory address for the pin control
register
gpiochip,gpio-line,pinctrl-PIN,pinctrl-address
0,0,82,44e10948
0,1,83,44e1094c
0,2,84,44e10950
0,3,85,44e10954
0,4,86,44e10958
0,5,87,44e1095c
0,6,88,44e10960
0,7,89,44e10964
0,8,52,44e108d0
0,9,53,44e108d4
0,10,54,44e108d8
0,11,55,44e108dc
0,12,94,44e10978
0,13,95,44e1097c
0,14,96,44e10980
0,15,97,44e10984
0,16,71,44e1091c
0,17,72,44e10920
0,18,135,44e10a1c
0,19,108,44e109b0
0,20,109,44e109b4
0,21,73,44e10924
0,22,8,44e10820
0,23,9,44e10824
0,26,10,44e10828
0,27,11,44e1082c
0,28,74,44e10928
0,29,81,44e10944
0,30,28,44e10870
0,31,29,44e10874
1,0,0,44e10800
1,1,1,44e10804
1,2,2,44e10808
1,3,3,44e1080c
1,4,4,44e10810
1,5,5,44e10814
1,6,6,44e10818
1,7,7,44e1081c
1,8,90,44e10968
1,9,91,44e1096c
1,10,92,44e10970
1,11,93,44e10974
1,12,12,44e10830
1,13,13,44e10834
1,14,14,44e10838
1,15,15,44e1083c
1,16,16,44e10840
1,17,17,44e10844
1,18,18,44e10848
1,19,19,44e1084c
1,20,20,44e10850
1,21,21,44e10854
1,22,22,44e10858
1,23,23,44e1085c
1,24,24,44e10860
1,25,25,44e10864
1,26,26,44e10868
1,27,27,44e1086c
1,28,30,44e10878
1,29,31,44e1087c
1,30,32,44e10880
1,31,33,44e10884
2,0,34,44e10888
2,1,35,44e1088c
2,2,36,44e10890
2,3,37,44e10894
2,4,38,44e10898
2,5,39,44e1089c
2,6,40,44e108a0
2,7,41,44e108a4
2,8,42,44e108a8
2,9,43,44e108ac
2,10,44,44e108b0
2,11,45,44e108b4
2,12,46,44e108b8
2,13,47,44e108bc
2,14,48,44e108c0
2,15,49,44e108c4
2,16,50,44e108c8
2,17,51,44e108cc
2,18,77,44e10934
2,19,78,44e10938
2,20,79,44e1093c
2,21,80,44e10940
2,22,56,44e108e0
2,23,57,44e108e4
2,24,58,44e108e8
2,25,59,44e108ec
2,26,60,44e108f0
2,27,61,44e108f4
2,28,62,44e108f8
2,29,63,44e108fc
2,30,64,44e10900
2,31,65,44e10904
3,0,66,44e10908
3,1,67,44e1090c
3,2,68,44e10910
3,3,69,44e10914
3,4,70,44e10918
3,5,98,44e10988
3,6,99,44e1098c
3,9,75,44e1092c
3,10,76,44e10930
3,13,141,44e10a34
3,14,100,44e10990
3,15,101,44e10994
3,16,102,44e10998
3,17,103,44e1099c
3,18,104,44e109a0
3,19,105,44e109a4
3,20,106,44e109a8
3,21,107,44e109ac
On a BeagleBlack Black board (AM3358) with this patch:
cat /sys/kernel/debug/pinctrl/44e10800.pinmux-pinctrl-single/gpio-ranges
GPIO ranges handled:
0: gpio-0-31 GPIOS [0 - 7] PINS [82 - 89]
8: gpio-0-31 GPIOS [8 - 11] PINS [52 - 55]
12: gpio-0-31 GPIOS [12 - 15] PINS [94 - 97]
16: gpio-0-31 GPIOS [16 - 17] PINS [71 - 72]
18: gpio-0-31 GPIOS [18 - 18] PINS [135 - 135]
19: gpio-0-31 GPIOS [19 - 20] PINS [108 - 109]
21: gpio-0-31 GPIOS [21 - 21] PINS [73 - 73]
22: gpio-0-31 GPIOS [22 - 23] PINS [8 - 9]
26: gpio-0-31 GPIOS [26 - 27] PINS [10 - 11]
28: gpio-0-31 GPIOS [28 - 28] PINS [74 - 74]
29: gpio-0-31 GPIOS [29 - 29] PINS [81 - 81]
30: gpio-0-31 GPIOS [30 - 31] PINS [28 - 29]
0: gpio-32-63 GPIOS [32 - 39] PINS [0 - 7]
8: gpio-32-63 GPIOS [40 - 43] PINS [90 - 93]
12: gpio-32-63 GPIOS [44 - 59] PINS [12 - 27]
28: gpio-32-63 GPIOS [60 - 63] PINS [30 - 33]
0: gpio-64-95 GPIOS [64 - 81] PINS [34 - 51]
18: gpio-64-95 GPIOS [82 - 85] PINS [77 - 80]
22: gpio-64-95 GPIOS [86 - 95] PINS [56 - 65]
0: gpio-96-127 GPIOS [96 - 100] PINS [66 - 70]
5: gpio-96-127 GPIOS [101 - 102] PINS [98 - 99]
7: gpio-96-127 GPIOS [103 - 104] PINS [75 - 76]
13: gpio-96-127 GPIOS [109 - 109] PINS [141 - 141]
14: gpio-96-127 GPIOS [110 - 117] PINS [100 - 107]
arch/arm/boot/dts/am33xx-l4.dtsi | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
index 5ed7f3c58c0f..340ea331e54d 100644
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -151,6 +151,18 @@ SYSC_OMAP2_SOFTRESET |
gpio0: gpio@0 {
compatible = "ti,omap4-gpio";
+ gpio-ranges = <&am33xx_pinmux 0 82 8>,
+ <&am33xx_pinmux 8 52 4>,
+ <&am33xx_pinmux 12 94 4>,
+ <&am33xx_pinmux 16 71 2>,
+ <&am33xx_pinmux 18 135 1>,
+ <&am33xx_pinmux 19 108 2>,
+ <&am33xx_pinmux 21 73 1>,
+ <&am33xx_pinmux 22 8 2>,
+ <&am33xx_pinmux 26 10 2>,
+ <&am33xx_pinmux 28 74 1>,
+ <&am33xx_pinmux 29 81 1>,
+ <&am33xx_pinmux 30 28 2>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
@@ -1298,6 +1310,10 @@ SYSC_OMAP2_SOFTRESET |
gpio1: gpio@0 {
compatible = "ti,omap4-gpio";
+ gpio-ranges = <&am33xx_pinmux 0 0 8>,
+ <&am33xx_pinmux 8 90 4>,
+ <&am33xx_pinmux 12 12 16>,
+ <&am33xx_pinmux 28 30 4>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
@@ -1700,6 +1716,9 @@ SYSC_OMAP2_SOFTRESET |
gpio2: gpio@0 {
compatible = "ti,omap4-gpio";
+ gpio-ranges = <&am33xx_pinmux 0 34 18>,
+ <&am33xx_pinmux 18 77 4>,
+ <&am33xx_pinmux 22 56 10>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
@@ -1733,6 +1752,11 @@ SYSC_OMAP2_SOFTRESET |
gpio3: gpio@0 {
compatible = "ti,omap4-gpio";
+ gpio-ranges = <&am33xx_pinmux 0 66 5>,
+ <&am33xx_pinmux 5 98 2>,
+ <&am33xx_pinmux 7 75 2>,
+ <&am33xx_pinmux 13 141 1>,
+ <&am33xx_pinmux 14 100 8>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
--
2.25.1
^ permalink raw reply related
* [PATCH] dt-bindings: clock: Convert imx7ulp clock to json-schema
From: Anson Huang @ 2020-06-02 13:16 UTC (permalink / raw)
To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
aisheng.dong, linux-clk, devicetree, linux-arm-kernel,
linux-kernel
Cc: Linux-imx
Convert the i.MX7ULP clock binding to DT schema format using json-schema,
the original binding doc is actually for two clock modules(SCG and PCC),
so split it to two binding docs, and the MPLL(mipi PLL) is NOT supposed
to be in clock module, so remove it from binding doc as well.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
.../devicetree/bindings/clock/imx7ulp-clock.txt | 103 ------------------
.../bindings/clock/imx7ulp-pcc-clock.yaml | 119 +++++++++++++++++++++
.../bindings/clock/imx7ulp-scg-clock.yaml | 97 +++++++++++++++++
3 files changed, 216 insertions(+), 103 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml
create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml
diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
deleted file mode 100644
index 93d89ad..0000000
--- a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
+++ /dev/null
@@ -1,103 +0,0 @@
-* Clock bindings for Freescale i.MX7ULP
-
-i.MX7ULP Clock functions are under joint control of the System
-Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
-modules, and Core Mode Controller (CMC)1 blocks
-
-The clocking scheme provides clear separation between M4 domain
-and A7 domain. Except for a few clock sources shared between two
-domains, such as the System Oscillator clock, the Slow IRC (SIRC),
-and and the Fast IRC clock (FIRCLK), clock sources and clock
-management are separated and contained within each domain.
-
-M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
-A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
-
-Note: this binding doc is only for A7 clock domain.
-
-System Clock Generation (SCG) modules:
----------------------------------------------------------------------
-The System Clock Generation (SCG) is responsible for clock generation
-and distribution across this device. Functions performed by the SCG
-include: clock reference selection, generation of clock used to derive
-processor, system, peripheral bus and external memory interface clocks,
-source selection for peripheral clocks and control of power saving
-clock gating mode.
-
-Required properties:
-
-- compatible: Should be "fsl,imx7ulp-scg1".
-- reg : Should contain registers location and length.
-- #clock-cells: Should be <1>.
-- clocks: Should contain the fixed input clocks.
-- clock-names: Should contain the following clock names:
- "rosc", "sosc", "sirc", "firc", "upll", "mpll".
-
-Peripheral Clock Control (PCC) modules:
----------------------------------------------------------------------
-The Peripheral Clock Control (PCC) is responsible for clock selection,
-optional division and clock gating mode for peripherals in their
-respected power domain
-
-Required properties:
-- compatible: Should be one of:
- "fsl,imx7ulp-pcc2",
- "fsl,imx7ulp-pcc3".
-- reg : Should contain registers location and length.
-- #clock-cells: Should be <1>.
-- clocks: Should contain the fixed input clocks.
-- clock-names: Should contain the following clock names:
- "nic1_bus_clk", "nic1_clk", "ddr_clk", "apll_pfd2",
- "apll_pfd1", "apll_pfd0", "upll", "sosc_bus_clk",
- "mpll", "firc_bus_clk", "rosc", "spll_bus_clk";
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell.
-See include/dt-bindings/clock/imx7ulp-clock.h
-for the full list of i.MX7ULP clock IDs of each module.
-
-Examples:
-
-#include <dt-bindings/clock/imx7ulp-clock.h>
-
-scg1: scg1@403e0000 {
- compatible = "fsl,imx7ulp-scg1;
- reg = <0x403e0000 0x10000>;
- clocks = <&rosc>, <&sosc>, <&sirc>,
- <&firc>, <&upll>, <&mpll>;
- clock-names = "rosc", "sosc", "sirc",
- "firc", "upll", "mpll";
- #clock-cells = <1>;
-};
-
-pcc2: pcc2@403f0000 {
- compatible = "fsl,imx7ulp-pcc2";
- reg = <0x403f0000 0x10000>;
- #clock-cells = <1>;
- clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
- <&scg1 IMX7ULP_CLK_NIC1_DIV>,
- <&scg1 IMX7ULP_CLK_DDR_DIV>,
- <&scg1 IMX7ULP_CLK_APLL_PFD2>,
- <&scg1 IMX7ULP_CLK_APLL_PFD1>,
- <&scg1 IMX7ULP_CLK_APLL_PFD0>,
- <&scg1 IMX7ULP_CLK_UPLL>,
- <&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
- <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
- <&scg1 IMX7ULP_CLK_ROSC>,
- <&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
- clock-names = "nic1_bus_clk", "nic1_clk", "ddr_clk",
- "apll_pfd2", "apll_pfd1", "apll_pfd0",
- "upll", "sosc_bus_clk", "mpll",
- "firc_bus_clk", "rosc", "spll_bus_clk";
-};
-
-usdhc1: usdhc@40380000 {
- compatible = "fsl,imx7ulp-usdhc";
- reg = <0x40380000 0x10000>;
- interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
- <&scg1 IMX7ULP_CLK_NIC1_DIV>,
- <&pcc2 IMX7ULP_CLK_USDHC1>;
- clock-names ="ipg", "ahb", "per";
- bus-width = <4>;
-};
diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml b/Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml
new file mode 100644
index 0000000..d5fd286
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-pcc-clock.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx7ulp-pcc-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX7ULP Peripheral Clock Control (PCC) modules
+
+maintainers:
+ - A.s. Dong <aisheng.dong@nxp.com>
+
+description: |
+ i.MX7ULP Clock functions are under joint control of the System
+ Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
+ modules, and Core Mode Controller (CMC)1 blocks
+
+ The clocking scheme provides clear separation between M4 domain
+ and A7 domain. Except for a few clock sources shared between two
+ domains, such as the System Oscillator clock, the Slow IRC (SIRC),
+ and and the Fast IRC clock (FIRCLK), clock sources and clock
+ management are separated and contained within each domain.
+
+ M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
+ A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
+
+ Note: this binding doc is only for A7 clock domain.
+
+ The Peripheral Clock Control (PCC) is responsible for clock selection,
+ optional division and clock gating mode for peripherals in their
+ respected power domain.
+
+ The clock consumer should specify the desired clock by having the clock
+ ID in its "clocks" phandle cell.
+ See include/dt-bindings/clock/imx7ulp-clock.h for the full list of
+ i.MX7ULP clock IDs of each module.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx7ulp-pcc2
+ - fsl,imx7ulp-pcc3
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: nic1 bus clock
+ - description: nic1 clock
+ - description: ddr clock
+ - description: apll pfd2
+ - description: apll pfd1
+ - description: apll pfd0
+ - description: usb pll
+ - description: system osc bus clock
+ - description: fast internal reference clock bus
+ - description: rtc osc
+ - description: system pll bus clock
+
+ clock-names:
+ items:
+ - const: nic1_bus_clk
+ - const: nic1_clk
+ - const: ddr_clk
+ - const: apll_pfd2
+ - const: apll_pfd1
+ - const: apll_pfd0
+ - const: upll
+ - const: sosc_bus_clk
+ - const: firc_bus_clk
+ - const: rosc
+ - const: spll_bus_clk
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx7ulp-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ clock-controller@403f0000 {
+ compatible = "fsl,imx7ulp-pcc2";
+ reg = <0x403f0000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
+ <&scg1 IMX7ULP_CLK_NIC1_DIV>,
+ <&scg1 IMX7ULP_CLK_DDR_DIV>,
+ <&scg1 IMX7ULP_CLK_APLL_PFD2>,
+ <&scg1 IMX7ULP_CLK_APLL_PFD1>,
+ <&scg1 IMX7ULP_CLK_APLL_PFD0>,
+ <&scg1 IMX7ULP_CLK_UPLL>,
+ <&scg1 IMX7ULP_CLK_SOSC_BUS_CLK>,
+ <&scg1 IMX7ULP_CLK_FIRC_BUS_CLK>,
+ <&scg1 IMX7ULP_CLK_ROSC>,
+ <&scg1 IMX7ULP_CLK_SPLL_BUS_CLK>;
+ clock-names = "nic1_bus_clk", "nic1_clk", "ddr_clk",
+ "apll_pfd2", "apll_pfd1", "apll_pfd0",
+ "upll", "sosc_bus_clk", "firc_bus_clk",
+ "rosc", "spll_bus_clk";
+ };
+
+ mmc@40380000 {
+ compatible = "fsl,imx7ulp-usdhc";
+ reg = <0x40380000 0x10000>;
+ interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
+ <&scg1 IMX7ULP_CLK_NIC1_DIV>,
+ <&pcc2 IMX7ULP_CLK_USDHC1>;
+ clock-names ="ipg", "ahb", "per";
+ bus-width = <4>;
+ };
diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml b/Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml
new file mode 100644
index 0000000..ae2a648
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-scg-clock.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx7ulp-scg-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX7ULP System Clock Generation (SCG) modules
+
+maintainers:
+ - A.s. Dong <aisheng.dong@nxp.com>
+
+description: |
+ i.MX7ULP Clock functions are under joint control of the System
+ Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
+ modules, and Core Mode Controller (CMC)1 blocks
+
+ The clocking scheme provides clear separation between M4 domain
+ and A7 domain. Except for a few clock sources shared between two
+ domains, such as the System Oscillator clock, the Slow IRC (SIRC),
+ and and the Fast IRC clock (FIRCLK), clock sources and clock
+ management are separated and contained within each domain.
+
+ M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
+ A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
+
+ Note: this binding doc is only for A7 clock domain.
+
+ The System Clock Generation (SCG) is responsible for clock generation
+ and distribution across this device. Functions performed by the SCG
+ include: clock reference selection, generation of clock used to derive
+ processor, system, peripheral bus and external memory interface clocks,
+ source selection for peripheral clocks and control of power saving
+ clock gating mode.
+
+ The clock consumer should specify the desired clock by having the clock
+ ID in its "clocks" phandle cell.
+ See include/dt-bindings/clock/imx7ulp-clock.h for the full list of
+ i.MX7ULP clock IDs of each module.
+
+properties:
+ compatible:
+ const: fsl,imx7ulp-scg1
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: rtc osc
+ - description: system osc
+ - description: slow internal reference clock
+ - description: fast internal reference clock
+ - description: usb PLL
+
+ clock-names:
+ items:
+ - const: rosc
+ - const: sosc
+ - const: sirc
+ - const: firc
+ - const: upll
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx7ulp-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ clock-controller@403e0000 {
+ compatible = "fsl,imx7ulp-scg1";
+ reg = <0x403e0000 0x10000>;
+ clocks = <&rosc>, <&sosc>, <&sirc>,
+ <&firc>, <&upll>;
+ clock-names = "rosc", "sosc", "sirc",
+ "firc", "upll";
+ #clock-cells = <1>;
+ };
+
+ mmc@40380000 {
+ compatible = "fsl,imx7ulp-usdhc";
+ reg = <0x40380000 0x10000>;
+ interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
+ <&scg1 IMX7ULP_CLK_NIC1_DIV>,
+ <&pcc2 IMX7ULP_CLK_USDHC1>;
+ clock-names ="ipg", "ahb", "per";
+ bus-width = <4>;
+ };
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] ARM: dts: AM33xx-l4: add gpio-ranges
From: Grygorii Strashko @ 2020-06-02 13:44 UTC (permalink / raw)
To: Drew Fustini, Tony Lindgren, linux-omap, linux-kernel,
Benoît Cousson, Rob Herring, devicetree
Cc: Santosh Shilimkar, Suman Anna, Haojian Zhuang, Linus Walleij,
linux-gpio
In-Reply-To: <20200602131428.GA496390@x1>
On 02/06/2020 16:14, Drew Fustini wrote:
> Add gpio-ranges properties to the gpio controller nodes.
>
> These gpio-ranges were created based on "Table 9-10. CONTROL_MODULE
> REGISTERS" in the "AM335x Technical Reference Manual" [0] and "Table
> 4-2. Pin Attributes" in the "AM335x Sitara Processor datasheet" [1].
> A csv file with this data is available for reference [2].
It will be good if you can explain not only "what" is changed, but
also "why" it's needed in commit message.
>
> [0] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf
> [1] http://www.ti.com/lit/ds/symlink/am3358.pdf
> [2] https://gist.github.com/pdp7/6ffaddc8867973c1c3e8612cfaf72020
>
> Signed-off-by: Drew Fustini <drew@beagleboard.org>
> ---
> Notes:
> There was previous dicussion relevant to this patch:
> https://lore.kernel.org/linux-gpio/20200525131731.GA948395@x1/
>
> Here is the list I compiled which shows the mapping between a gpioline
> and the pin number including the memory address for the pin control
> register
>
> gpiochip,gpio-line,pinctrl-PIN,pinctrl-address
> 0,0,82,44e10948
> 0,1,83,44e1094c
> 0,2,84,44e10950
> 0,3,85,44e10954
> 0,4,86,44e10958
> 0,5,87,44e1095c
...
> On a BeagleBlack Black board (AM3358) with this patch:
> cat /sys/kernel/debug/pinctrl/44e10800.pinmux-pinctrl-single/gpio-ranges
>
> GPIO ranges handled:
> 0: gpio-0-31 GPIOS [0 - 7] PINS [82 - 89]
> 8: gpio-0-31 GPIOS [8 - 11] PINS [52 - 55]
> 12: gpio-0-31 GPIOS [12 - 15] PINS [94 - 97]
> 16: gpio-0-31 GPIOS [16 - 17] PINS [71 - 72]
> 18: gpio-0-31 GPIOS [18 - 18] PINS [135 - 135]
> 19: gpio-0-31 GPIOS [19 - 20] PINS [108 - 109]
> 21: gpio-0-31 GPIOS [21 - 21] PINS [73 - 73]
> 22: gpio-0-31 GPIOS [22 - 23] PINS [8 - 9]
> 26: gpio-0-31 GPIOS [26 - 27] PINS [10 - 11]
> 28: gpio-0-31 GPIOS [28 - 28] PINS [74 - 74]
> 29: gpio-0-31 GPIOS [29 - 29] PINS [81 - 81]
> 30: gpio-0-31 GPIOS [30 - 31] PINS [28 - 29]
> 0: gpio-32-63 GPIOS [32 - 39] PINS [0 - 7]
> 8: gpio-32-63 GPIOS [40 - 43] PINS [90 - 93]
> 12: gpio-32-63 GPIOS [44 - 59] PINS [12 - 27]
> 28: gpio-32-63 GPIOS [60 - 63] PINS [30 - 33]
> 0: gpio-64-95 GPIOS [64 - 81] PINS [34 - 51]
> 18: gpio-64-95 GPIOS [82 - 85] PINS [77 - 80]
> 22: gpio-64-95 GPIOS [86 - 95] PINS [56 - 65]
> 0: gpio-96-127 GPIOS [96 - 100] PINS [66 - 70]
> 5: gpio-96-127 GPIOS [101 - 102] PINS [98 - 99]
> 7: gpio-96-127 GPIOS [103 - 104] PINS [75 - 76]
> 13: gpio-96-127 GPIOS [109 - 109] PINS [141 - 141]
> 14: gpio-96-127 GPIOS [110 - 117] PINS [100 - 107]
>
> arch/arm/boot/dts/am33xx-l4.dtsi | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
> index 5ed7f3c58c0f..340ea331e54d 100644
> --- a/arch/arm/boot/dts/am33xx-l4.dtsi
> +++ b/arch/arm/boot/dts/am33xx-l4.dtsi
> @@ -151,6 +151,18 @@ SYSC_OMAP2_SOFTRESET |
>
> gpio0: gpio@0 {
> compatible = "ti,omap4-gpio";
> + gpio-ranges = <&am33xx_pinmux 0 82 8>,
> + <&am33xx_pinmux 8 52 4>,
> + <&am33xx_pinmux 12 94 4>,
> + <&am33xx_pinmux 16 71 2>,
> + <&am33xx_pinmux 18 135 1>,
> + <&am33xx_pinmux 19 108 2>,
> + <&am33xx_pinmux 21 73 1>,
> + <&am33xx_pinmux 22 8 2>,
> + <&am33xx_pinmux 26 10 2>,
> + <&am33xx_pinmux 28 74 1>,
> + <&am33xx_pinmux 29 81 1>,
> + <&am33xx_pinmux 30 28 2>;
> gpio-controller;
> #gpio-cells = <2>;
> interrupt-controller;
> @@ -1298,6 +1310,10 @@ SYSC_OMAP2_SOFTRESET |
>
> gpio1: gpio@0 {
> compatible = "ti,omap4-gpio";
> + gpio-ranges = <&am33xx_pinmux 0 0 8>,
> + <&am33xx_pinmux 8 90 4>,
> + <&am33xx_pinmux 12 12 16>,
> + <&am33xx_pinmux 28 30 4>;
> gpio-controller;
> #gpio-cells = <2>;
> interrupt-controller;
> @@ -1700,6 +1716,9 @@ SYSC_OMAP2_SOFTRESET |
>
> gpio2: gpio@0 {
> compatible = "ti,omap4-gpio";
> + gpio-ranges = <&am33xx_pinmux 0 34 18>,
> + <&am33xx_pinmux 18 77 4>,
> + <&am33xx_pinmux 22 56 10>;
> gpio-controller;
> #gpio-cells = <2>;
> interrupt-controller;
> @@ -1733,6 +1752,11 @@ SYSC_OMAP2_SOFTRESET |
>
> gpio3: gpio@0 {
> compatible = "ti,omap4-gpio";
> + gpio-ranges = <&am33xx_pinmux 0 66 5>,
> + <&am33xx_pinmux 5 98 2>,
> + <&am33xx_pinmux 7 75 2>,
> + <&am33xx_pinmux 13 141 1>,
> + <&am33xx_pinmux 14 100 8>;
> gpio-controller;
> #gpio-cells = <2>;
> interrupt-controller;
>
--
Best regards,
grygorii
^ permalink raw reply
* Re: [PATCH] ARM: dts: AM33xx-l4: add gpio-ranges
From: Tony Lindgren @ 2020-06-02 13:51 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Drew Fustini, linux-omap, linux-kernel, Benoît Cousson,
Rob Herring, devicetree, Santosh Shilimkar, Suman Anna,
Haojian Zhuang, Linus Walleij, linux-gpio
In-Reply-To: <803e2d78-28ba-0816-dbb5-d441d7659a91@ti.com>
* Grygorii Strashko <grygorii.strashko@ti.com> [200602 13:44]:
>
>
> On 02/06/2020 16:14, Drew Fustini wrote:
> > Add gpio-ranges properties to the gpio controller nodes.
> >
> > These gpio-ranges were created based on "Table 9-10. CONTROL_MODULE
> > REGISTERS" in the "AM335x Technical Reference Manual" [0] and "Table
> > 4-2. Pin Attributes" in the "AM335x Sitara Processor datasheet" [1].
> > A csv file with this data is available for reference [2].
>
> It will be good if you can explain not only "what" is changed, but
> also "why" it's needed in commit message.
Also, please check (again) that this is the same for all the am3
variants. For omap3, we had different pad assignments even between
SoC revisions. Different pad routings should be easy to deal with
in the dts if needed though.
Regards,
Tony
^ permalink raw reply
* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Emil Velikov @ 2020-06-02 14:13 UTC (permalink / raw)
To: Krishna Manikandan
Cc: ML dri-devel, linux-arm-msm, freedreno, devicetree,
Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
Kristian H . Kristensen, mka
In-Reply-To: <1591009402-681-1-git-send-email-mkrishn@codeaurora.org>
Hi Krishna,
On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan <mkrishn@codeaurora.org> wrote:
>
> Define shutdown callback for display drm driver,
> so as to disable all the CRTCS when shutdown
> notification is received by the driver.
>
> This change will turn off the timing engine so
> that no display transactions are requested
> while mmu translations are getting disabled
> during reboot sequence.
>
> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
>
AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
msm_drm_ops::unbind.
Are you saying that unbind never triggers? If so, then we should
really fix that instead, since this patch seems more like a
workaround.
-Emil
^ permalink raw reply
* Re: [PATCH V2 1/2] hwmon: pwm-fan: Add profile support and add remove module support
From: Guenter Roeck @ 2020-06-02 14:28 UTC (permalink / raw)
To: Sandipan Patra
Cc: treding, jonathanh, kamil, jdelvare, robh+dt, u.kleine-koenig,
bbasu, bbiswas, kyarlagadda, linux-pwm, linux-hwmon, devicetree,
linux-tegra, linux-kernel
In-Reply-To: <1590992354-12623-1-git-send-email-spatra@nvidia.com>
On Mon, Jun 01, 2020 at 11:49:13AM +0530, Sandipan Patra wrote:
> Add support for profiles mode settings.
> This allows different fan settings for trip point temp/hyst/pwm.
> Tegra194 has multiple fan-profiles support.
>
> Signed-off-by: Sandipan Patra <spatra@nvidia.com>
The subject says "remove module support". What is this supposed to be
about ?
The code adds support for multiple cooling "profiles" but, unless I am
really missing something, no means to actually select a profile.
This adds a lot of complexity to the code with zero value.
Either case, and I may have mentioned this before, functionality like this
should really reside in the thermal core and not in individual drivers.
> ---
>
> PATCH V2:
> Cleaned pwm_fan_remove support as it is not required.
>
> drivers/hwmon/pwm-fan.c | 92 ++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 80 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 30b7b3e..1d2a416 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -3,8 +3,10 @@
> * pwm-fan.c - Hwmon driver for fans connected to PWM lines.
> *
> * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Copyright (c) 2020, NVIDIA Corporation.
> *
> * Author: Kamil Debski <k.debski@samsung.com>
> + * Author: Sandipan Patra <spatra@nvidia.com>
You can not claim authorship for a driver by adding a few lines of code
to it.
> */
>
> #include <linux/hwmon.h>
> @@ -21,6 +23,8 @@
> #include <linux/timer.h>
>
> #define MAX_PWM 255
> +/* Based on OF max device tree node name length */
> +#define MAX_PROFILE_NAME_LENGTH 31
>
> struct pwm_fan_ctx {
> struct mutex lock;
> @@ -38,6 +42,12 @@ struct pwm_fan_ctx {
> unsigned int pwm_fan_state;
> unsigned int pwm_fan_max_state;
> unsigned int *pwm_fan_cooling_levels;
> +
> + unsigned int pwm_fan_profiles;
> + const char **fan_profile_names;
> + unsigned int **fan_profile_cooling_levels;
> + unsigned int fan_current_profile;
> +
> struct thermal_cooling_device *cdev;
> };
>
> @@ -227,28 +237,86 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
> struct pwm_fan_ctx *ctx)
> {
> struct device_node *np = dev->of_node;
> + struct device_node *base_profile = NULL;
> + struct device_node *profile_np = NULL;
> + const char *default_profile = NULL;
> int num, i, ret;
>
> - if (!of_find_property(np, "cooling-levels", NULL))
> - return 0;
> + num = of_property_count_u32_elems(np, "cooling-levels");
> + if (num <= 0) {
> + base_profile = of_get_child_by_name(np, "profiles");
You can not just add new properties like this without documenting it
and getting approval by devicetree maintainers.
Guenter
> + if (!base_profile) {
> + dev_err(dev, "Wrong Data\n");
> + return -EINVAL;
> + }
> + }
> +
> + if (base_profile) {
> + ctx->pwm_fan_profiles =
> + of_get_available_child_count(base_profile);
>
> - ret = of_property_count_u32_elems(np, "cooling-levels");
> - if (ret <= 0) {
> - dev_err(dev, "Wrong data!\n");
> - return ret ? : -EINVAL;
> + if (ctx->pwm_fan_profiles <= 0) {
> + dev_err(dev, "Profiles used but not defined\n");
> + return -EINVAL;
> + }
> +
> + ctx->fan_profile_names = devm_kzalloc(dev,
> + sizeof(const char *) * ctx->pwm_fan_profiles,
> + GFP_KERNEL);
> + ctx->fan_profile_cooling_levels = devm_kzalloc(dev,
> + sizeof(int *) * ctx->pwm_fan_profiles,
> + GFP_KERNEL);
> +
> + if (!ctx->fan_profile_names
> + || !ctx->fan_profile_cooling_levels)
> + return -ENOMEM;
> +
> + ctx->fan_current_profile = 0;
> + i = 0;
> + for_each_available_child_of_node(base_profile, profile_np) {
> + num = of_property_count_u32_elems(profile_np,
> + "cooling-levels");
> + if (num <= 0) {
> + dev_err(dev, "No data in cooling-levels inside profile node!\n");
> + return -EINVAL;
> + }
> +
> + of_property_read_string(profile_np, "name",
> + &ctx->fan_profile_names[i]);
> + if (default_profile &&
> + !strncmp(default_profile,
> + ctx->fan_profile_names[i],
> + MAX_PROFILE_NAME_LENGTH))
> + ctx->fan_current_profile = i;
> +
> + ctx->fan_profile_cooling_levels[i] =
> + devm_kzalloc(dev, sizeof(int) * num,
> + GFP_KERNEL);
> + if (!ctx->fan_profile_cooling_levels[i])
> + return -ENOMEM;
> +
> + of_property_read_u32_array(profile_np, "cooling-levels",
> + ctx->fan_profile_cooling_levels[i], num);
> + i++;
> + }
> }
>
> - num = ret;
> ctx->pwm_fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
> GFP_KERNEL);
> if (!ctx->pwm_fan_cooling_levels)
> return -ENOMEM;
>
> - ret = of_property_read_u32_array(np, "cooling-levels",
> - ctx->pwm_fan_cooling_levels, num);
> - if (ret) {
> - dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> - return ret;
> + if (base_profile) {
> + memcpy(ctx->pwm_fan_cooling_levels,
> + ctx->fan_profile_cooling_levels[ctx->fan_current_profile],
> + num);
> + } else {
> + ret = of_property_read_u32_array(np, "cooling-levels",
> + ctx->pwm_fan_cooling_levels, num);
> + if (ret) {
> + dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> + return -EINVAL;
> + }
> }
>
> for (i = 0; i < num; i++) {
^ permalink raw reply
* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: John Donnelly @ 2020-06-02 14:41 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: Bhupesh Sharma, Chen Zhou, Simon Horman, Devicetree List,
Baoquan He, Will Deacon, Linux Doc Mailing List, Catalin Marinas,
kexec mailing list, Linux Kernel Mailing List, Rob Herring,
Ingo Molnar, Arnd Bergmann, guohanjun, James Morse,
Thomas Gleixner, Prabhakar Kushwaha, RuiRui Yang,
linux-arm-kernel
In-Reply-To: <CAJ2QiJJE-jeRL1HPUZCwi1LtV9CBMmYrsOaS6vX1R1sJ6Z1t8g@mail.gmail.com>
> On Jun 2, 2020, at 12:38 AM, Prabhakar Kushwaha <prabhakar.pkin@gmail.com> wrote:
>
> On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <john.p.donnelly@oracle.com> wrote:
>>
>> Hi . See below !
>>
>>> On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>>
>>> Hi John,
>>>
>>> On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <John.P.donnelly@oracle.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
>>>>> Hi Chen,
>>>>>
>>>>> On Thu, May 21, 2020 at 3:05 PM Chen Zhou <chenzhou10@huawei.com> wrote:
>>>>>> This patch series enable reserving crashkernel above 4G in arm64.
>>>>>>
>>>>>> There are following issues in arm64 kdump:
>>>>>> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
>>>>>> when there is no enough low memory.
>>>>>> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
>>>>>> in this case, if swiotlb or DMA buffers are required, crash dump kernel
>>>>>> will boot failure because there is no low memory available for allocation.
>>>>>>
>>>>>> To solve these issues, introduce crashkernel=X,low to reserve specified
>>>>>> size low memory.
>>>>>> Crashkernel=X tries to reserve memory for the crash dump kernel under
>>>>>> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
>>>>>> size low memory for crash kdump kernel devices firstly and then reserve
>>>>>> memory above 4G.
>>>>>>
>>>>>> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
>>>>>> is specified simultaneously, kernel should reserve specified size low memory
>>>>>> for crash dump kernel devices. So there may be two crash kernel regions, one
>>>>>> is below 4G, the other is above 4G.
>>>>>> In order to distinct from the high region and make no effect to the use of
>>>>>> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
>>>>>> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
>>>>>>
>>>>>> Besides, we need to modify kexec-tools:
>>>>>> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
>>>>>>
>>>>>> The previous changes and discussions can be retrieved from:
>>>>>>
>>>>>> Changes since [v7]
>>>>>> - Move x86 CRASH_ALIGN to 2M
>>>>>> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
>>>>>> - Update Documentation/devicetree/bindings/chosen.txt
>>>>>> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
>>>>>> - Add Tested-by from Jhon and pk
>>>>>>
>>>>>> Changes since [v6]
>>>>>> - Fix build errors reported by kbuild test robot.
>>>>>>
>>>>>> Changes since [v5]
>>>>>> - Move reserve_crashkernel_low() into kernel/crash_core.c.
>>>>>> - Delete crashkernel=X,high.
>>>>>> - Modify crashkernel=X,low.
>>>>>> If crashkernel=X,low is specified simultaneously, reserve spcified size low
>>>>>> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
>>>>>> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
>>>>>> pass to crash dump kernel by DT property "linux,low-memory-range".
>>>>>> - Update Documentation/admin-guide/kdump/kdump.rst.
>>>>>>
>>>>>> Changes since [v4]
>>>>>> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
>>>>>>
>>>>>> Changes since [v3]
>>>>>> - Add memblock_cap_memory_ranges back for multiple ranges.
>>>>>> - Fix some compiling warnings.
>>>>>>
>>>>>> Changes since [v2]
>>>>>> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
>>>>>> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
>>>>>> patch.
>>>>>>
>>>>>> Changes since [v1]:
>>>>>> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
>>>>>> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
>>>>>> in fdt_enforce_memory_region().
>>>>>> There are at most two crash kernel regions, for two crash kernel regions
>>>>>> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
>>>>>> and then remove the memory range in the middle.
>>>>>>
>>>>>> [1]: https://urldefense.com/v3/__http://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbvpn1uM1$
>>>>>> [v1]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/2/1174__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbt0xN9PE$
>>>>>> [v2]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/9/86__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbub7yUQH$
>>>>>> [v3]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/9/306__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbnc4zPPV$
>>>>>> [v4]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/4/15/273__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbvsAsZLu$
>>>>>> [v5]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/5/6/1360__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbl24n-79$
>>>>>> [v6]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/8/30/142__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbs7r8G2a$
>>>>>> [v7]: https://urldefense.com/v3/__https://lkml.org/lkml/2019/12/23/411__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiFUH90G$
>>>>>>
>>>>>> Chen Zhou (5):
>>>>>> x86: kdump: move reserve_crashkernel_low() into crash_core.c
>>>>>> arm64: kdump: reserve crashkenel above 4G for crash dump kernel
>>>>>> arm64: kdump: add memory for devices by DT property, low-memory-range
>>>>>> kdump: update Documentation about crashkernel on arm64
>>>>>> dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
>>>>>>
>>>>> We are getting "warn_alloc" [1] warning during boot of kdump kernel
>>>>> with bootargs as [2] of primary kernel.
>>>>> This error observed on ThunderX2 ARM64 platform.
>>>>>
>>>>> It is observed with latest upstream tag (v5.7-rc3) with this patch set
>>>>> and https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
>>>>> Also **without** this patch-set
>>>>> "https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$"
>>>>>
>>>>> This issue comes whenever crashkernel memory is reserved after 0xc000_0000.
>>>>> More details discussed earlier in
>>>>> https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$ without any
>>>>> solution
>>>>>
>>>>> This patch-set is expected to solve similar kind of issue.
>>>>> i.e. low memory is only targeted for DMA, swiotlb; So above mentioned
>>>>> observation should be considered/fixed. .
>>>>>
>>>>> --pk
>>>>>
>>>>> [1]
>>>>> [ 30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>> [ 30.367696] NET: Registered protocol family 16
>>>>> [ 30.369973] swapper/0: page allocation failure: order:6,
>>>>> mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
>>>>> [ 30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc3+ #121
>>>>> [ 30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
>>>>> TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
>>>>> [ 30.369984] Call trace:
>>>>> [ 30.369989] dump_backtrace+0x0/0x1f8
>>>>> [ 30.369991] show_stack+0x20/0x30
>>>>> [ 30.369997] dump_stack+0xc0/0x10c
>>>>> [ 30.370001] warn_alloc+0x10c/0x178
>>>>> [ 30.370004] __alloc_pages_slowpath.constprop.111+0xb10/0xb50
>>>>> [ 30.370006] __alloc_pages_nodemask+0x2b4/0x300
>>>>> [ 30.370008] alloc_page_interleave+0x24/0x98
>>>>> [ 30.370011] alloc_pages_current+0xe4/0x108
>>>>> [ 30.370017] dma_atomic_pool_init+0x44/0x1a4
>>>>> [ 30.370020] do_one_initcall+0x54/0x228
>>>>> [ 30.370027] kernel_init_freeable+0x228/0x2cc
>>>>> [ 30.370031] kernel_init+0x1c/0x110
>>>>> [ 30.370034] ret_from_fork+0x10/0x18
>>>>> [ 30.370036] Mem-Info:
>>>>> [ 30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
>>>>> [ 30.370064] active_file:0 inactive_file:0 isolated_file:0
>>>>> [ 30.370064] unevictable:0 dirty:0 writeback:0 unstable:0
>>>>> [ 30.370064] slab_reclaimable:34 slab_unreclaimable:4438
>>>>> [ 30.370064] mapped:0 shmem:0 pagetables:14 bounce:0
>>>>> [ 30.370064] free:1537719 free_pcp:219 free_cma:0
>>>>> [ 30.370070] Node 0 active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>> unstable:0kB all_unreclaimable? no
>>>>> [ 30.370073] Node 1 active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
>>>>> isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
>>>>> shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
>>>>> unstable:0kB all_unreclaimable? no
>>>>> [ 30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
>>>>> reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>> present:128kB managed:0kB mlocked:0kB kernel_stack:0kB pagetables:0kB
>>>>> bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>> [ 30.370084] lowmem_reserve[]: 0 250 6063 6063
>>>>> [ 30.370090] Node 0 DMA32 free:256000kB min:408kB low:664kB
>>>>> high:920kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>> present:269700kB managed:256000kB mlocked:0kB kernel_stack:0kB
>>>>> pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
>>>>> [ 30.370094] lowmem_reserve[]: 0 0 5813 5813
>>>>> [ 30.370100] Node 0 Normal free:5894876kB min:9552kB low:15504kB
>>>>> high:21456kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
>>>>> active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
>>>>> present:8388608kB managed:5953112kB mlocked:0kB kernel_stack:21672kB
>>>>> pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB free_cma:0kB
>>>>> [ 30.370104] lowmem_reserve[]: 0 0 0 0
>>>>> [ 30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>> 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
>>>>> [ 30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
>>>>> 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) = 256000kB
>>>>> [ 30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB (UE) 3*32kB
>>>>> (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME) 3*1024kB (ME)
>>>>> 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
>>>>> [ 30.370129] Node 0 hugepages_total=0 hugepages_free=0
>>>>> hugepages_surp=0 hugepages_size=1048576kB
>>>>> [ 30.370130] 0 total pagecache pages
>>>>> [ 30.370132] 0 pages in swap cache
>>>>> [ 30.370134] Swap cache stats: add 0, delete 0, find 0/0
>>>>> [ 30.370135] Free swap = 0kB
>>>>> [ 30.370136] Total swap = 0kB
>>>>> [ 30.370137] 2164609 pages RAM
>>>>> [ 30.370139] 0 pages HighMem/MovableOnly
>>>>> [ 30.370140] 612331 pages reserved
>>>>> [ 30.370141] 0 pages hwpoisoned
>>>>> [ 30.370143] DMA: failed to allocate 256 KiB pool for atomic
>>>>> coherent allocation
>>>>
>>>>
>>>> During my testing I saw the same error and Chen's solution corrected it .
>>>
>>> Which combination you are using on your side? I am using Prabhakar's
>>> suggested environment and can reproduce the issue
>>> with or without Chen's crashkernel support above 4G patchset.
>>>
>>> I am also using a ThunderX2 platform with latest makedumpfile code and
>>> kexec-tools (with the suggested patch
>>> <https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$ >).
>>>
>>> Thanks,
>>> Bhupesh
>>
>>
>> I did this activity 5 months ago and I have moved on to other activities. My DMA failures were related to PCI devices that could not be enumerated because low-DMA space was not available when crashkernel was moved above 4G; I don’t recall the exact platform.
>>
>>
>>
>> For this failure ,
>>
>>>>> DMA: failed to allocate 256 KiB pool for atomic
>>>>> coherent allocation
>>
>>
>> Is due to :
>>
>>
>> 3618082c
>> ("arm64 use both ZONE_DMA and ZONE_DMA32")
>>
>> With the introduction of ZONE_DMA to support the Raspberry DMA
>> region below 1G, the crashkernel is placed in the upper 4G
>> ZONE_DMA_32 region. Since the crashkernel does not have access
>> to the ZONE_DMA region, it prints out call trace during bootup.
>>
>> It is due to having this CONFIG item ON :
>>
>>
>> CONFIG_ZONE_DMA=y
>>
>> Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
>> use the device tree to specify memory below 1G.
>>
>>
>
> Disabling ZONE_DMA is temporary solution. We may need proper solution
Perhaps the Raspberry platform configuration dependencies need separated from “server class” Arm equipment ? Or auto-configured on boot ? Consult an expert ;-)
>
>> I would like to see Chen’s feature added , perhaps as EXPERIMENTAL, so we can get some configuration testing done on it. It corrects having a DMA zone in low memory while crash-kernel is above 4GB. This has been going on for a year now.
>
> I will also like this patch to be added in Linux as early as possible.
>
> Issue mentioned by me happens with or without this patch.
>
> This patch-set can consider fixing because it uses low memory for DMA
> & swiotlb only.
> We can consider restricting crashkernel within the required range like below
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 7f9e5a6dc48c..bd67b90d35bd 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
> return 0;
> }
>
> - low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
> + low_base = memblock_find_in_range(0,0xc0000000, low_size, CRASH_ALIGN);
> if (!low_base) {
> pr_err("Cannot reserve %ldMB crashkernel low memory,
> please try smaller size.\n",
> (unsigned long)(low_size >> 20));
>
>
I suspect 0xc0000000 would need to be a CONFIG item and not hard-coded.
> Similar change can be considered for scenario "without" this patch.
> But it will decrease memory availability for crashkernel.
> Hence increase the failure probability of crashkernel reservation.
>
> --pk
^ permalink raw reply
* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Sai Prakash Ranjan @ 2020-06-02 14:49 UTC (permalink / raw)
To: Emil Velikov
Cc: Krishna Manikandan, ML dri-devel, linux-arm-msm, freedreno,
devicetree, Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
Kristian H . Kristensen, mka, devicetree-owner
In-Reply-To: <CACvgo50eb5_jp_6B5tkV9cX_X2_y2Xnavu+wvUUhHN5FsV9hiw@mail.gmail.com>
Hi Emil,
On 2020-06-02 19:43, Emil Velikov wrote:
> Hi Krishna,
>
> On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan
> <mkrishn@codeaurora.org> wrote:
>>
>> Define shutdown callback for display drm driver,
>> so as to disable all the CRTCS when shutdown
>> notification is received by the driver.
>>
>> This change will turn off the timing engine so
>> that no display transactions are requested
>> while mmu translations are getting disabled
>> during reboot sequence.
>>
>> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
>>
> AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
> msm_drm_ops::unbind.
>
> Are you saying that unbind never triggers? If so, then we should
> really fix that instead, since this patch seems more like a
> workaround.
>
Which path do you suppose that the unbind should be called from, remove
callback? Here we are talking about the drivers which are builtin, where
remove callbacks are not called from the driver core during
reboot/shutdown,
instead shutdown callbacks are called which needs to be defined in order
to
trigger unbind. So AFAICS there is nothing to be fixed.
msm_pdev_shutdown()
platform_drv_shutdown()
device_shutdown()
kernel_restart_prepare()
kernel_restart()
__arm64_sys_reboot()
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH v1 2/6] drm: panel-simple: add Seiko 70WVW2T 7" simple panel
From: Emil Velikov @ 2020-06-02 14:55 UTC (permalink / raw)
To: Doug Anderson
Cc: Sam Ravnborg,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Sebastian Reichel, dri-devel, Bjorn Andersson, Thierry Reding,
Søren Andersen
In-Reply-To: <CAD=FV=VSyODjtVtEe6H46U6xNraD2LUUi+xt8cxraaqXom=64g@mail.gmail.com>
On Tue, 2 Jun 2020 at 01:31, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jun 1, 2020 at 1:33 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > The Seiko 70WVW2T is a discontinued product, but may be used somewhere.
> > Tested on a proprietary product.
> >
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Søren Andersen <san@skov.dk>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > ---
> > drivers/gpu/drm/panel/panel-simple.c | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index b067f66cea0e..8624bb80108c 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -3194,6 +3194,31 @@ static const struct panel_desc shelly_sca07010_bfn_lnn = {
> > .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > };
> >
> > +static const struct drm_display_mode sii_70wvw2t_mode = {
> > + .clock = 33000,
> > + .hdisplay = 800,
> > + .hsync_start = 800 + 256,
> > + .hsync_end = 800 + 256 + 0,
> > + .htotal = 800 + 256 + 0 + 0,
> > + .vdisplay = 480,
> > + .vsync_start = 480 + 0,
> > + .vsync_end = 480 + 0 + 0,
> > + .vtotal = 480 + 0 + 0 + 45,
>
> Important to have a "vrefresh"?
>
Ville posted a series (most of which already landed) getting removing
vrefresh all together. The overall idea is to compute it, in the rare
case it's needed.
>
> > + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
> > +};
> > +
> > +static const struct panel_desc sii_70wvw2t = {
> > + .modes = &sii_70wvw2t_mode,
> > + .num_modes = 1,
>
> Do we want "bpc = 6"?
>
The largest user of bpc is userspace - the data gets copied via the ioctls.
A secondary, and quite limited, user are drivers exposing the "max
bpc" connector property. From a quick look: amdgpu, the synopsys
dw-hdmi bridge and i915 do so. In case the data missing, atomics
assume a max 8 bpc.
>
> > + .size = {
> > + .width = 152,
> > + .height = 91,
> > + },
> > + .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>
> Should this be a 666 format? Random internet-found data sheet says
> 262K colors...
Good catch. Would be nice to have a spec sheet link (even if random)
in the commit message.
HTH
-Emil
^ permalink raw reply
* Re: [PATCH v3 006/105] dt-bindings: display: Convert VC4 bindings to schemas
From: Maxime Ripard @ 2020-06-02 15:00 UTC (permalink / raw)
To: Rob Herring
Cc: Nicolas Saenz Julienne, Eric Anholt, dri-devel, linux-rpi-kernel,
bcm-kernel-feedback-list, linux-arm-kernel, linux-kernel,
Dave Stevenson, Tim Gover, Phil Elwell, devicetree
In-Reply-To: <20200527191211.GA2559189@bogus>
[-- Attachment #1: Type: text/plain, Size: 4929 bytes --]
Hi Rob,
On Wed, May 27, 2020 at 01:12:11PM -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 05:47:36PM +0200, Maxime Ripard wrote:
> > The BCM283x SoCs have a display pipeline composed of several controllers
> > with device tree bindings that are supported by Linux.
> >
> > Now that we have the DT validation in place, let's split into separate
> > files and convert the device tree bindings for those controllers to
> > schemas.
> >
> > This is just a 1:1 conversion though, and some bindings were incomplete so
> > it results in example validation warnings that are going to be addressed in
> > the following patches.
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> > Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | 174 +------------------------------------------------------------------------
> > Documentation/devicetree/bindings/display/brcm,bcm2835-dpi.yaml | 66 +++++++++++++++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml | 73 ++++++++++++++++++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-hdmi.yaml | 75 +++++++++++++++++++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-hvs.yaml | 37 +++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-pixelvalve0.yaml | 40 +++++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-txp.yaml | 37 +++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml | 42 +++++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-vc4.yaml | 34 ++++++++++++++-
> > Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml | 44 ++++++++++++++++++-
> > MAINTAINERS | 2 +-
> > 11 files changed, 449 insertions(+), 175 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-dpi.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-hdmi.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-hvs.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-pixelvalve0.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-txp.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-vc4.yaml
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml
>
>
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml
> > new file mode 100644
> > index 000000000000..3887675f844e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/brcm,bcm2835-dsi0.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom VC4 (VideoCore4) DSI Controller
> > +
> > +maintainers:
> > + - Eric Anholt <eric@anholt.net>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - brcm,bcm2835-dsi0
> > + - brcm,bcm2835-dsi1
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: The DSI PLL clock feeding the DSI analog PHY
> > + - description: The DSI ESC clock
> > + - description: The DSI pixel clock
> > +
> > + clock-output-names: true
> > + # FIXME: The meta-schemas don't seem to allow it for now
> > + # items:
> > + # - description: The DSI byte clock for the PHY
> > + # - description: The DSI DDR2 clock
> > + # - description: The DSI DDR clock
>
> Doesn't pattern work for you?
>
> pattern: '^dsi[0-1]_byte$'
That's not really what I was trying to achieve. I don't think
clock-output-names should hardcode the values it expect, since the whole
point is to let the "user" (ie the DT) control the clock names. If these
were to be fixed, it wouldn't even be here in the first place.
I just wanted to have a description of the clocks to provide a name for,
but it looks like clock-output-names can't have an items below. I looked
at why, couldn't really find a reason, and forgot to tell you about it,
sorry
> Either way,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 104/105] dt-bindings: display: vc4: hdmi: Add BCM2711 HDMI controllers bindings
From: Maxime Ripard @ 2020-06-02 15:08 UTC (permalink / raw)
To: Rob Herring
Cc: Nicolas Saenz Julienne, Eric Anholt, dri-devel, linux-rpi-kernel,
bcm-kernel-feedback-list, linux-arm-kernel, linux-kernel,
Dave Stevenson, Tim Gover, Phil Elwell, devicetree
In-Reply-To: <20200529181833.GA2685451@bogus>
[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]
Hi Rob,
On Fri, May 29, 2020 at 12:18:33PM -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 05:49:14PM +0200, Maxime Ripard wrote:
> > The HDMI controllers found in the BCM2711 SoC need some adjustments to the
> > bindings, especially since the registers have been shuffled around in more
> > register ranges.
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> > Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 109 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > new file mode 100644
> > index 000000000000..6091fe3d315b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
>
> Dual license...
>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> > +
> > +maintainers:
> > + - Eric Anholt <eric@anholt.net>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - brcm,bcm2711-hdmi0
> > + - brcm,bcm2711-hdmi1
>
> What's the difference between the 2 blocks?
The register layout and the lane mapping in the PHY change a bit.
> > +
> > + reg:
> > + items:
> > + - description: HDMI controller register range
> > + - description: DVP register range
> > + - description: HDMI PHY register range
> > + - description: Rate Manager register range
> > + - description: Packet RAM register range
> > + - description: Metadata RAM register range
> > + - description: CSC register range
> > + - description: CEC register range
> > + - description: HD register range
> > +
> > + reg-names:
> > + items:
> > + - const: hdmi
> > + - const: dvp
> > + - const: phy
> > + - const: rm
> > + - const: packet
> > + - const: metadata
> > + - const: csc
> > + - const: cec
> > + - const: hd
> > +
> > + clocks:
> > + description: The HDMI state machine clock
> > +
> > + clock-names:
> > + const: hdmi
> > +
> > + ddc:
> > + allOf:
> > + - $ref: /schemas/types.yaml#/definitions/phandle
> > + description: >
> > + Phandle of the I2C controller used for DDC EDID probing
>
> Goes in the connector.
>
> And isn't the standard name ddc-i2c-bus?
>
> > +
> > + hpd-gpios:
> > + description: >
> > + The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> > + as an interrupt/status bit in the HDMI controller itself)
>
> Goes in the connector.
If this was an entirely new binding, I would agree, but this is not
really the case here.
We discussed it already for the v2, and this binding is essentially the
same one than the bcm2835 HDMI controller.
I initially sent a patch adding conditionnals for the clocks and regs
differences too, and you asked to split the binding into a separate file
to simplify it a bit.
Supporting both the old binding, and the new one based on the connector
is going to make the code significantly more complicated, and I'm not
really sure why we would here.
Thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH resend 0/2] dts: keystone-k2g-evm: Display support
From: santosh.shilimkar @ 2020-06-02 15:18 UTC (permalink / raw)
To: Tomi Valkeinen, Jyri Sarha, dri-devel, ssantosh, linux-arm-kernel,
devicetree
Cc: mark.rutland, praneeth, robh+dt, peter.ujfalusi, laurent.pinchart
In-Reply-To: <973b69f2-bbe1-3c1b-615f-751bb8d5d83e@ti.com>
On 6/2/20 1:13 AM, Tomi Valkeinen wrote:
> Hi Santosh,
>
> On 14/02/2020 19:40, santosh.shilimkar@oracle.com wrote:
>> On 2/14/20 1:22 AM, Jyri Sarha wrote:
>>> Resend because the earlier recipient list was wrong.
>>>
>>> Now that drm/tidss is queued for mainline, lets add display support for
>>> k2g-evm. There is no hurry since tidss is out only in v5.7, but it
>>> should not harm to have the dts changes in place before that.
>>>
>>> Jyri Sarha (2):
>>> ARM: dts: keystone-k2g: Add DSS node
>>> ARM: dts: keystone-k2g-evm: add HDMI video support
>>>
>>> arch/arm/boot/dts/keystone-k2g-evm.dts | 101 +++++++++++++++++++++++++
>>> arch/arm/boot/dts/keystone-k2g.dtsi | 22 ++++++
>>> 2 files changed, 123 insertions(+)
>>>
>> Ok. Will add this to the next queue.
>
> What happened with this one? It used to be in linux-next, but now I
> don't see it anymore.
>
Pull request [1] was sent during last merge window. Thought it was
picked up but doesn't seems to be. Have sent question to arm-soc
maintainers.
Regards,
Santosh
[1] https://www.spinics.net/lists/arm-kernel/msg791224.html
^ permalink raw reply
* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Emil Velikov @ 2020-06-02 15:39 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: Krishna Manikandan, ML dri-devel, linux-arm-msm, freedreno,
devicetree, Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
Kristian H . Kristensen, mka, devicetree-owner
In-Reply-To: <cd61dd742e73b89794fc1b812d9fdcd9@codeaurora.org>
On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Hi Emil,
>
> On 2020-06-02 19:43, Emil Velikov wrote:
> > Hi Krishna,
> >
> > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan
> > <mkrishn@codeaurora.org> wrote:
> >>
> >> Define shutdown callback for display drm driver,
> >> so as to disable all the CRTCS when shutdown
> >> notification is received by the driver.
> >>
> >> This change will turn off the timing engine so
> >> that no display transactions are requested
> >> while mmu translations are getting disabled
> >> during reboot sequence.
> >>
> >> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
> >>
> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
> > msm_drm_ops::unbind.
> >
> > Are you saying that unbind never triggers? If so, then we should
> > really fix that instead, since this patch seems more like a
> > workaround.
> >
>
> Which path do you suppose that the unbind should be called from, remove
> callback? Here we are talking about the drivers which are builtin, where
> remove callbacks are not called from the driver core during
> reboot/shutdown,
> instead shutdown callbacks are called which needs to be defined in order
> to
> trigger unbind. So AFAICS there is nothing to be fixed.
>
Interesting - in drm effectively only drm panels implement .shutdown.
So my naive assumption was that .remove was used implicitly by core,
as part of the shutdown process. Yet that's not the case, so it seems
that many drivers could use some fixes.
Then again, that's an existing problem which is irrelevant for msm.
-Emil
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: Martin Kepplinger @ 2020-06-02 15:44 UTC (permalink / raw)
To: Marco Felsch
Cc: robh, kernel, shawnguo, s.hauer, kernel, festevam, linux-imx,
mchehab, Anson.Huang, agx, angus, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20200527093538.xqtdoybl5hx27ccv@pengutronix.de>
Hi Marco,
On 27.05.20 11:35, Marco Felsch wrote:
> Hi Martin,
>
> On 20-05-14 17:57, Martin Kepplinger wrote:
>> From: "Angus Ainslie (Purism)" <angus@akkea.ca>
>>
>> Add a devicetree description for the Librem 5 phone. The early batches
>> that have been sold are supported as well as the mass-produced device
>> available later this year, see https://puri.sm/products/librem-5/
>>
>> This boots to a working console with working WWAN modem, wifi usdhc,
>> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
>> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
>> Signed-off-by: Guido Günther <agx@sigxcpu.org>
>> ---
>> arch/arm64/boot/dts/freescale/Makefile | 1 +
>> .../boot/dts/freescale/imx8mq-librem5.dts | 1174 +++++++++++++++++
>> 2 files changed, 1175 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>>
>> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
>> index cd38d04da5a7..342579121f98 100644
>> --- a/arch/arm64/boot/dts/freescale/Makefile
>> +++ b/arch/arm64/boot/dts/freescale/Makefile
>> @@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
>> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
>> dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> new file mode 100644
>> index 000000000000..95c105b4c120
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>> @@ -0,0 +1,1174 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2018-2020 Purism SPC
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "dt-bindings/input/input.h"
>> +#include "dt-bindings/pwm/pwm.h"
>> +#include "dt-bindings/usb/pd.h"
>> +#include "imx8mq.dtsi"
>> +
>> +/ {
>> + model = "Purism Librem 5";
>> + compatible = "purism,librem5", "fsl,imx8mq";
>> +
>> + backlight_dsi: backlight-dsi {
>> + compatible = "led-backlight";
>> + leds = <&led_backlight>;
>> + brightness-levels = <255>;
>> + default-brightness-level = <100>;
>> + };
>> +
>> + bm818_codec: sound-wwan-codec {
>> + compatible = "broadmobi,bm818", "option,gtm601";
>> + #sound-dai-cells = <0>;
>> + };
>
> Please sort the node names alpabetical.
>
>> +
>> + chosen {
>> + stdout-path = &uart1;
>> + };
>> +
>> + gpio-keys {
>> + compatible = "gpio-keys";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_keys>, <&pinctrl_hp>;
>> +
>> + hp-det {
>> + label = "HP_DET";
>> + gpios = <&gpio3 9 GPIO_ACTIVE_HIGH>;
>> + wakeup-source;
>> + linux,code = <KEY_HP>;
>
> Nit: I would add the wakeup-source behind the linux,code.
>
>> + };
>> +
>> + vol-down {
>> + label = "VOL_DOWN";
>> + gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
>> + linux,code = <KEY_VOLUMEDOWN>;
>> + };
>> +
>> + vol-up {
>> + label = "VOL_UP";
>> + gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
>> + linux,code = <KEY_VOLUMEUP>;
>> + };
>> + };
>> +
>> + pwmleds {
>> + compatible = "pwm-leds";
>> +
>> + blue {
>> + label = "phone:blue:front";
>> + max-brightness = <248>;
>> + pwms = <&pwm2 0 50000>;
>> + };
>> +
>> + green {
>> + label = "phone:green:front";
>> + max-brightness = <248>;
>> + pwms = <&pwm4 0 50000>;
>> + };
>> +
>> + red {
>> + label = "phone:red:front";
>> + max-brightness = <248>;
>> + pwms = <&pwm3 0 50000>;
>> + };
>> + };
>> +
>> + pmic_osc: clock-pmic {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <32768>;
>> + clock-output-names = "pmic_osc";
>> + };
>
> Please sort nodes alphabetical.
>
>> +
>> + reg_audio_pwr_en: regulator-audio-pwr-en {
>> + compatible = "regulator-fixed";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_audiopwr>;
>> + regulator-name = "AUDIO_PWR_EN";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + regulator-always-on;
>
> Why should this regulator be always on? The wm8962.c driver can handle
> the regualtor enable/disable.
It can indeed be handled by the driver here. We keep it always on
because of issues with the display stack that are not yet part of this
description.
>
>> + };
>> +
>> + reg_aud_1v8: regulator-audio-v1v8 {
> ^
> regulator-audio-1v8?
>
>> + compatible = "regulator-fixed";
>> + regulator-name = "aud_1v8";
>
> Is it intended to use capitalized and no-capitalized regulator-name's?
>
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + vin-supply = <®_audio_pwr_en>;
>> + };
>
> Can we squash regulator-audio-pwr-en and regulator-audio-v1v8?
I think we can.
>
>> +
>> + reg_gnss: regulator-gnss {
>> + compatible = "regulator-fixed";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_gnsspwr>;
>> + regulator-name = "GNSS";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + gpio = <&gpio3 12 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + };
>> +
>> + reg_hub: regulator-hub {
>> + compatible = "regulator-fixed";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_hub_pwr>;
>> + regulator-name = "HUB";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + };
>> +
>> + reg_lcd_1v8: regulator-lcd-1v8 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "lcd_1v8";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_dsien>;
>> + vin-supply = <®_vdd_1v8>;
>> + enable-active-high;
>> + gpio = <&gpio1 5 GPIO_ACTIVE_HIGH>;
>> + };
>
> This regulator is never used.
>
>> +
>> + reg_lcd_3v4: regulator-lcd-3v4 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "lcd_3v4";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_dsibiasen>;
>> + vin-supply = <®_vsys_3v4>;
>> + enable-active-high;
>> + gpio = <&gpio1 20 GPIO_ACTIVE_HIGH>;
>> + };
>> +
>> + reg_vdd_sen: regulator-vdd-sen {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vdd_sen";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> +
>> + reg_vdd_3v3: regulator-vdd-3v3 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vdd_3v3";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> +
>> + reg_vdd_1v8: regulator-vdd-1v8 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vdd_1v8";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + };
>> +
>> + reg_vsys_3v4: regulator-vsys-3v4 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vsys_3v4";
>> + regulator-min-microvolt = <3400000>;
>> + regulator-max-microvolt = <3400000>;
>> + regulator-always-on;
>> + };
>> +
>> + reg_3v3_wifi: regulator-3v3-wifi {
> ^
> reg_wifi_3v3: regulator-wifi-3v3?
>
>> + compatible = "regulator-fixed";
>> + regulator-name = "3v3_wifi";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> +
>> + sound {
>> + compatible = "simple-audio-card";
>> + simple-audio-card,name = "wm8962";
>> + simple-audio-card,format = "i2s";
>> + simple-audio-card,widgets =
>> + "Headphone", "Headphone",
>> + "Microphone", "Headset Mic",
>> + "Microphone", "Digital Mic",
>> + "Speaker", "Speaker";
>> + simple-audio-card,routing =
>> + "Headphone", "HPOUTL",
>> + "Headphone", "HPOUTR",
>> + "Speaker", "SPKOUTL",
>> + "Speaker", "SPKOUTR",
>> + "Headset Mic", "MICBIAS",
>> + "IN3R", "Headset Mic",
>> + "DMICDAT", "Digital Mic";
>> + simple-audio-card,cpu {
>> + sound-dai = <&sai2>;
>> + };
>> + simple-audio-card,codec {
>> + sound-dai = <&codec>;
>> + clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
>> + frame-master;
>> + bitclock-master;
>> + };
>> + };
>> +
>> + sound-wwan {
>> + compatible = "simple-audio-card";
>> + simple-audio-card,name = "MODEM";
>> + simple-audio-card,format = "i2s";
>> +
>> + simple-audio-card,cpu {
>> + sound-dai = <&sai6>;
>> + frame-inversion;
>> + };
>> +
>> + telephony_link_master: simple-audio-card,codec {
> ^
> useless phandle?
>> + sound-dai = <&bm818_codec>;
>> + frame-master;
>> + bitclock-master;
>> + };
>> + };
>> +
>> + vibrator {
>> + compatible = "pwm-vibrator";
>> + pwms = <&pwm1 0 1000000000 0>;
>> + pwm-names = "enable";
>> + vcc-supply = <®_vdd_3v3>;
>> + };
>> +};
>> +
>> +&A53_0 {
>> + cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&A53_1 {
>> + cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&A53_2 {
>> + cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&A53_3 {
>> + cpu-supply = <&buck2_reg>;
>> +};
>> +
>> +&clk {
>> + assigned-clocks = <&clk IMX8MQ_AUDIO_PLL1>, <&clk IMX8MQ_AUDIO_PLL2>;
>> + assigned-clock-rates = <786432000>, <722534400>;
>> +};
>
> Either I would bundle all clock settings here or within the sai nodes.
You're right, I'll try to move them.
>
>> +
>> +&ddrc {
>> + operating-points-v2 = <&ddrc_opp_table>;
>> +
>> + ddrc_opp_table: ddrc-opp-table {
>> + compatible = "operating-points-v2";
>> +
>> + opp-25M {
>> + opp-hz = /bits/ 64 <25000000>;
>> + };
>> + opp-100M {
>> + opp-hz = /bits/ 64 <100000000>;
>> + };
>> + opp-800M {
>> + opp-hz = /bits/ 64 <800000000>;
>> + };
>> + };
>> +};
>> +
>> +&dphy {
>> + status = "okay";
>> +};
>> +
>> +&ecspi1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_ecspi1>;
>> + cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>;
>
> Missmatch with the pinctrl_ecspi1?
>
>> + status = "okay";
>
> Status is always the last property.
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + nor_flash: flash@0 {
>> + compatible = "jedec,spi-nor";
>> + spi-max-frequency = <1000000>;
>> + reg = <0>;
>> + };
>> +};
>> +
>> +&gpio1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_pmic_5v>;
>> +
>> + pmic-5v {
>> + gpio-hog;
>> + gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
>> + input;
>> + };
>> +};
>> +
>> +&iomuxc {
>> + pinctrl_audiopwr: audiopwrgrp {
>> + fsl,pins = <
>> + /* AUDIO_POWER_EN_3V3 */
>> + MX8MQ_IOMUXC_GPIO1_IO04_GPIO1_IO4 0x83
>> + >;
>> + };
>> +
>> + pinctrl_bl: blgrp {
>> + fsl,pins = <
>> + /* BACKLINGE_EN */
>> + MX8MQ_IOMUXC_NAND_DQS_GPIO3_IO14 0x83
>> + >;
>> + };
>> +
>> + pinctrl_charger_in: chargeringrp {
>> + fsl,pins = <
>> + /* CHRG_INT */
>> + MX8MQ_IOMUXC_NAND_CE2_B_GPIO3_IO3 0x80
>> + /* CHG_STATUS_B */
>> + MX8MQ_IOMUXC_NAND_ALE_GPIO3_IO0 0x80
>> + >;
>> + };
>> +
>> + pinctrl_dsibiasen: dsibiasengrp {
>> + fsl,pins = <
>> + /* DSI_BIAS_EN */
>> + MX8MQ_IOMUXC_ENET_TD1_GPIO1_IO20 0x83
>> + >;
>> + };
>> +
>> + pinctrl_dsien: dsiengrp {
>> + fsl,pins = <
>> + /* DSI_EN_3V3 */
>> + MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5 0x83
>> + >;
>> + };
>> +
>> + pinctrl_ecspi1: spi1grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_ECSPI1_MOSI_ECSPI1_MOSI 0x83
>> + MX8MQ_IOMUXC_ECSPI1_MISO_ECSPI1_MISO 0x83
>> + MX8MQ_IOMUXC_ECSPI1_SS0_GPIO5_IO9 0x19
>> + MX8MQ_IOMUXC_ECSPI1_SCLK_ECSPI1_SCLK 0x83
>> + /* SPI_SS1 */
>> + MX8MQ_IOMUXC_UART4_RXD_GPIO5_IO28 0x19
>> + >;
>> + };
>> +
>> + pinctrl_gauge: gaugegrp {
>> + fsl,pins = <
>> + /* BAT_LOW */
>> + MX8MQ_IOMUXC_SAI5_RXC_GPIO3_IO20 0x80
>> + >;
>> + };
>> +
>> + pinctrl_gnsspwr: gnsspwrgrp {
>> + fsl,pins = <
>> + /* GPS3V3_EN */
>> + MX8MQ_IOMUXC_NAND_DATA06_GPIO3_IO12 0x83
>> + >;
>> + };
>> +
>> + pinctrl_haptic: hapticgrp {
>> + fsl,pins = <
>> + /* MOTO */
>> + MX8MQ_IOMUXC_SPDIF_EXT_CLK_PWM1_OUT 0x83
>> + >;
>> + };
>> +
>> + pinctrl_hp: hpgrp {
>> + fsl,pins = <
>> + /* HEADPHONE_DET_1V8 */
>> + MX8MQ_IOMUXC_NAND_DATA03_GPIO3_IO9 0x180
>> + >;
>> + };
>> +
>> + pinctrl_hub_pwr: hubpwrgrp {
>> + fsl,pins = <
>> + /* HUB_PWR_3V3_EN */
>> + MX8MQ_IOMUXC_GPIO1_IO14_GPIO1_IO14 0x83
>> + >;
>> + };
>> +
>> + pinctrl_i2c1: i2c1grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL 0x40000026
>> + MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA 0x40000026
>> + >;
>> + };
>> +
>> + pinctrl_i2c2: i2c2grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_I2C2_SCL_I2C2_SCL 0x40000026
>> + MX8MQ_IOMUXC_I2C2_SDA_I2C2_SDA 0x40000026
>> + >;
>> + };
>> +
>> + pinctrl_i2c3: i2c3grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_I2C3_SCL_I2C3_SCL 0x40000026
>> + MX8MQ_IOMUXC_I2C3_SDA_I2C3_SDA 0x40000026
>> + >;
>> + };
>> +
>> + pinctrl_i2c4: i2c4grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL 0x40000026
>> + MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA 0x40000026
>> + >;
>> + };
>> +
>> + pinctrl_keys: keysgrp {
>> + fsl,pins = <
>> + /* 4G_WAKE */
>> + MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15 0x80
>> + /* PWR_KEY */
>> + MX8MQ_IOMUXC_NAND_CLE_GPIO3_IO5 0x01C0
>
> gpio3 5/15 are never used was this intended?
>
>> + /* VOL- */
>> + MX8MQ_IOMUXC_ENET_MDIO_GPIO1_IO17 0x01C0
>> + /* VOL+ */
>> + MX8MQ_IOMUXC_ENET_MDC_GPIO1_IO16 0x01C0
>> + >;
>> + };
>> +
>> + pinctrl_led_b: ledbgrp {
>> + fsl,pins = <
>> + /* LED_B */
>> + MX8MQ_IOMUXC_GPIO1_IO13_PWM2_OUT 0x06
>> + >;
>> + };
>> +
>> + pinctrl_led_g: ledggrp {
>> + fsl,pins = <
>> + /* LED_G */
>> + MX8MQ_IOMUXC_SAI3_MCLK_PWM4_OUT 0x06
>> + >;
>> + };
>> +
>> + pinctrl_led_r: ledrgrp {
>> + fsl,pins = <
>> + /* LED_R */
>> + MX8MQ_IOMUXC_SPDIF_TX_PWM3_OUT 0x06
>> + >;
>> + };
>> +
>> + pinctrl_mag: maggrp {
>> + fsl,pins = <
>> + /* INT_MAG */
>> + MX8MQ_IOMUXC_SAI5_RXD1_GPIO3_IO22 0x80
>> + >;
>> + };
>> +
>> + pinctrl_pmic: pmicgrp {
>> + fsl,pins = <
>> + /* PMIC_NINT */
>> + MX8MQ_IOMUXC_GPIO1_IO07_GPIO1_IO7 0x80
>> + >;
>> + };
>> +
>> + pinctrl_pmic_5v: pmic5vgrp {
>> + fsl,pins = <
>> + /* PMIC_5V */
>> + MX8MQ_IOMUXC_GPIO1_IO01_GPIO1_IO1 0x80
>> + >;
>> + };
>> +
>> + pinctrl_prox: proxgrp {
>> + fsl,pins = <
>> + /* INT_LIGHT */
>> + MX8MQ_IOMUXC_NAND_DATA01_GPIO3_IO7 0x80
>> + >;
>> + };
>> +
>> + pinctrl_rtc: rtcgrp {
>> + fsl,pins = <
>> + /* RTC_INT */
>> + MX8MQ_IOMUXC_GPIO1_IO09_GPIO1_IO9 0x80
>> + >;
>> + };
>> +
>> + pinctrl_sai2: sai2grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SAI2_TXD0_SAI2_TX_DATA0 0xd6
>> + MX8MQ_IOMUXC_SAI2_TXFS_SAI2_TX_SYNC 0xd6
>> + MX8MQ_IOMUXC_SAI2_MCLK_SAI2_MCLK 0xd6
>> + MX8MQ_IOMUXC_SAI2_RXD0_SAI2_RX_DATA0 0xd6
>> + MX8MQ_IOMUXC_SAI2_TXC_SAI2_TX_BCLK 0xd6
>> + >;
>> + };
>> +
>> + pinctrl_sai6: sai6grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SAI1_RXD5_SAI6_RX_DATA0 0xd6
>> + MX8MQ_IOMUXC_SAI1_RXD6_SAI6_RX_SYNC 0xd6
>> + MX8MQ_IOMUXC_SAI1_TXD4_SAI6_RX_BCLK 0xd6
>> + MX8MQ_IOMUXC_SAI1_TXD5_SAI6_TX_DATA0 0xd6
>> + >;
>> + };
>> +
>> + pinctrl_tcpc: tcpcgrp {
>> + fsl,pins = <
>> + /* TCPC_INT */
>> + MX8MQ_IOMUXC_GPIO1_IO10_GPIO1_IO10 0x01C0
>> + >;
>> + };
>> +
>> + pinctrl_typec: typecgrp {
>> + fsl,pins = <
>> + /* TYPEC_MUX_EN */
>> + MX8MQ_IOMUXC_GPIO1_IO11_GPIO1_IO11 0x83
>> + >;
>> + };
>> +
>> + pinctrl_uart1: uart1grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_UART1_RXD_UART1_DCE_RX 0x49
>> + MX8MQ_IOMUXC_UART1_TXD_UART1_DCE_TX 0x49
>> + >;
>> + };
>> +
>> + pinctrl_uart2: uart2grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_UART2_TXD_UART2_DCE_TX 0x49
>> + MX8MQ_IOMUXC_UART2_RXD_UART2_DCE_RX 0x49
>> + >;
>> + };
>> +
>> + pinctrl_uart3: uart3grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_UART3_RXD_UART3_DCE_RX 0x49
>> + MX8MQ_IOMUXC_UART3_TXD_UART3_DCE_TX 0x49
>> + >;
>> + };
>> +
>> + pinctrl_uart4: uart4grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_ECSPI2_SCLK_UART4_DCE_RX 0x49
>> + MX8MQ_IOMUXC_ECSPI2_MOSI_UART4_DCE_TX 0x49
>> + MX8MQ_IOMUXC_ECSPI2_MISO_UART4_DCE_CTS_B 0x49
>> + MX8MQ_IOMUXC_ECSPI2_SS0_UART4_DCE_RTS_B 0x49
>> + >;
>> + };
>> +
>> + pinctrl_usdhc1: usdhc1grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK 0x83
>> + MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6 0xc3
>> + MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7 0xc3
>> + MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE 0x83
>> + MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B 0xc1
>> + >;
>> + };
>> +
>> + pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK 0x8d
>> + MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6 0xcd
>> + MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7 0xcd
>> + MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE 0x8d
>> + MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B 0xc1
>> + >;
>> + };
>> +
>> + pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK 0x9f
>> + MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6 0xdf
>> + MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7 0xdf
>> + MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE 0x9f
>> + MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B 0xc1
>> + >;
>> + };
>> +
>> + pinctrl_usdhc2: usdhc2grp {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B 0x80
>> + MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK 0x83
>> + MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD 0xc3
>> + MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0 0xc3
>> + MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1 0xc3
>> + MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2 0xc3
>> + MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3 0xc3
>> + MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
>> + >;
>> + };
>> +
>> + pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B 0x80
>> + MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK 0x8d
>> + MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD 0xcd
>> + MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0 0xcd
>> + MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1 0xcd
>> + MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2 0xcd
>> + MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3 0xcd
>> + MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
>> + >;
>> + };
>> +
>> + pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
>> + fsl,pins = <
>> + MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B 0x80
>> + MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK 0x9f
>> + MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD 0xcf
>> + MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0 0xcf
>> + MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1 0xcf
>> + MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2 0xcf
>> + MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3 0xcf
>> + MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
>> + >;
>> + };
>> +
>> + pinctrl_wdog: wdoggrp {
>> + fsl,pins = <
>> + /* nWDOG */
>> + MX8MQ_IOMUXC_GPIO1_IO02_WDOG1_WDOG_B 0x1f
>> + >;
>> + };
>> +};
>> +
>> +&i2c1 {
>> + clock-frequency = <387000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_i2c1>;
>> + status = "okay";
>> +
>> + typec_pd: usb-pd@3f {
>> + compatible = "ti,tps6598x";
>> + reg = <0x3f>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_typec>, <&pinctrl_tcpc>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
>> +
>> + connector {
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> +
>> + usb_con_hs: endpoint {
>> + remote-endpoint = <&typec_hs>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> +
>> + usb_con_ss: endpoint {
>> + remote-endpoint = <&typec_ss>;
>> + };
>> + };
>> + };
>> + };
>> + };
>> +
>> + pmic: pmic@4b {
>> + compatible = "rohm,bd71837";
>> + reg = <0x4b>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_pmic>;
>> + clocks = <&pmic_osc>;
>> + clock-names = "osc";
>> + clock-output-names = "pmic_clk";
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <7 GPIO_ACTIVE_LOW>;
>> + interrupt-names = "irq";
>> + rohm,reset-snvs-powered;
>> +
>> + regulators {
>> + buck1_reg: BUCK1 {
>> + regulator-name = "buck1";
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <1300000>;
>> + regulator-ramp-delay = <1250>;
>> + rohm,dvs-run-voltage = <900000>;
>> + rohm,dvs-idle-voltage = <850000>;
>> + rohm,dvs-suspend-voltage = <800000>;
>> + regulator-always-on;
>> + };
>> +
>> + buck2_reg: BUCK2 {
>> + regulator-name = "buck2";
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <1300000>;
>> + regulator-ramp-delay = <1250>;
>> + rohm,dvs-run-voltage = <1000000>;
>> + rohm,dvs-idle-voltage = <900000>;
>> + regulator-always-on;
>> + };
>> +
>> + buck3_reg: BUCK3 {
>> + regulator-name = "buck3";
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <1300000>;
>> + rohm,dvs-run-voltage = <900000>;
>> + regulator-always-on;
>> + };
>> +
>> + buck4_reg: BUCK4 {
>> + regulator-name = "buck4";
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <1300000>;
>> + rohm,dvs-run-voltage = <1000000>;
>> + };
>> +
>> + buck5_reg: BUCK5 {
>> + regulator-name = "buck5";
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <1350000>;
>> + regulator-always-on;
>> + };
>> +
>> + buck6_reg: BUCK6 {
>> + regulator-name = "buck6";
>> + regulator-min-microvolt = <3000000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-always-on;
>> + };
>> +
>> + buck7_reg: BUCK7 {
>> + regulator-name = "buck7";
>> + regulator-min-microvolt = <1605000>;
>> + regulator-max-microvolt = <1995000>;
>> + regulator-always-on;
>> + };
>> +
>> + buck8_reg: BUCK8 {
>> + regulator-name = "buck8";
>> + regulator-min-microvolt = <800000>;
>> + regulator-max-microvolt = <1400000>;
>> + regulator-always-on;
>> + };
>> +
>> + ldo1_reg: LDO1 {
>> + regulator-name = "ldo1";
>> + regulator-min-microvolt = <3000000>;
>> + regulator-max-microvolt = <3300000>;
>> + /* leave on for snvs power button */
>> + regulator-always-on;
>> + };
>> +
>> + ldo2_reg: LDO2 {
>> + regulator-name = "ldo2";
>> + regulator-min-microvolt = <900000>;
>> + regulator-max-microvolt = <900000>;
>> + /* leave on for snvs power button */
>> + regulator-always-on;
>> + };
>> +
>> + ldo3_reg: LDO3 {
>> + regulator-name = "ldo3";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-always-on;
>> + };
>> +
>> + ldo4_reg: LDO4 {
>> + regulator-name = "ldo4";
>> + regulator-min-microvolt = <900000>;
>> + regulator-max-microvolt = <1800000>;
>> + regulator-always-on;
>> + };
>> +
>> + ldo5_reg: LDO5 {
>> + /* VDD_PHY_0V9 - MIPI and HDMI domains */
>> + regulator-name = "ldo5";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-always-on;
>> + };
>> +
>> + ldo6_reg: LDO6 {
>> + /* VDD_PHY_0V9 - MIPI, HDMI and USB domains */
>> + regulator-name = "ldo6";
>> + regulator-min-microvolt = <900000>;
>> + regulator-max-microvolt = <1800000>;
>> + regulator-always-on;
>> + };
>> +
>> + ldo7_reg: LDO7 {
>> + /* VDD_PHY_3V3 - USB domain */
>> + regulator-name = "ldo7";
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-always-on;
>> + };
>
> Out of curiosity, why did you marked all regulators as
> regulator-always-on? I thought the librem5 is a smartphone.
It is. But we just currently see an unstable system in case we allow
turning most of them off. We do for the VPU, and should (afaik already)
be able to do so for the GPU. After more work in the drivers the mipi
regulator(s) should be able to be turned off. So we'll look at more and
more of these over time.
>
>> + };
>> + };
>> +
>> + rtc@68 {
>> + compatible = "microcrystal,rv4162";
>> + reg = <0x68>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_rtc>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
>> + };
>> +};
>> +
>> +&i2c2 {
>> + clock-frequency = <387000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_i2c2>;
>> + status = "okay";
>> +
>> + magnetometer@1e {
>> + compatible = "st,lsm9ds1-magn";
>> + reg = <0x1e>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_mag>;
>> + interrupt-parent = <&gpio3>;
>> + interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
>> + vdd-supply = <®_vdd_sen>;
>> + vddio-supply = <®_vdd_1v8>;
>> + };
>> +
>> + regulator@3e {
>> + compatible = "tps65132";
>> + reg = <0x3e>;
>> + reg_lcd_avdd: outp {
>> + regulator-name = "lcd_avdd";
>> + vin-supply = <®_lcd_3v4>;
>> + };
>> +
>> + reg_lcd_avee: outn {
>> + regulator-name = "lcd_avee";
>> + vin-supply = <®_lcd_3v4>;
>> + };
> both phandles are not used.
>> + };
>> +
>> + flash@53 {
>> + compatible = "lm3560";
>> + reg = <0x53>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + flash@0 {
>> + reg = <0x0>;
>> + flash-timeout-us = <150000>;
>> + flash-max-microamp = <320000>;
>> + led-max-microamp = <60000>;
>> + label = "lm3560:flash";
>> + };
>> +
>> + torch@1 {
>> + reg = <0x1>;
>> + led-max-microamp = <10000>;
>> + label = "lm3560:torch";
>> + };
>> +
>> + };
>> +
>> + prox@60 {
>> + compatible = "vishay,vcnl4040";
>> + reg = <0x60>;
>> + pinctrl-0 = <&pinctrl_prox>;
>> + interrupt-parent = <&gpio3>;
>> + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
>> + };
>> +
>> + accel-gyro@6a {
>> + compatible = "st,lsm9ds1-imu";
>> + reg = <0x6a>;
>> + vdd-supply = <®_vdd_sen>;
>> + vddio-supply = <®_vdd_1v8>;
>> + mount-matrix = "1", "0", "0",
>> + "0", "1", "0",
>> + "0", "0", "-1";
>> + };
>> +};
>> +
>> +&i2c3 {
>> + clock-frequency = <387000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_i2c3>;
>> + status = "okay";
>> +
>> + codec: wm8962@1a {
>
> Please use generic names.
>
>> + compatible = "wlf,wm8962";
>> + reg = <0x1a>; // 0x4a is the test address
>> + clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
>> + assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
>> + assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
>> + assigned-clock-rates = <24576000>;
>> + #sound-dai-cells = <0>;
>> + mic-cfg = <0x200>;
>> + DCVDD-supply = <®_aud_1v8>;
>> + DBVDD-supply = <®_aud_1v8>;
>> + AVDD-supply = <®_aud_1v8>;
>> + CPVDD-supply = <®_aud_1v8>;
>> + MICVDD-supply = <®_aud_1v8>;
>> + PLLVDD-supply = <®_aud_1v8>;
>> + SPKVDD1-supply = <®_vsys_3v4>;
>> + SPKVDD2-supply = <®_vsys_3v4>;
>> + gpio-cfg = <
>> + 0x0000 /* n/c */
>> + 0x0001 /* gpio2, 1: default */
>> + 0x0013 /* gpio3, 2: dmicclk */
>> + 0x0000 /* n/c, 3: default */
>> + 0x8014 /* gpio5, 4: dmic_dat */
>> + 0x0000 /* gpio6, 5: default */
>> + >;
>> + status = "okay";
>
> status can be dropped
>
>> + };
>> +
>> + backlight@36 {
>> + compatible = "ti,lm36922";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_bl>;
>> + reg = <0x36>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + enable-gpios = <&gpio3 14 GPIO_ACTIVE_HIGH>;
>> + vled-supply = <®_vsys_3v4>;
>> + ti,ovp-microvolt = <25000000>;
>> +
>> + led_backlight: led@0 {
>> + reg = <0>;
>> + label = "white:backlight_cluster";
>> + linux,default-trigger = "backlight";
>> + led-max-microamp = <20000>;
>> + };
>> + };
>> +
>> + touchscreen@38 {
>> + compatible = "edt,edt-ft5506";
>> + reg = <0x38>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <27 IRQ_TYPE_EDGE_FALLING>;
>
> You need to mux the irq gpio.
>
>> + irq-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
>
> irq-gpios is not supported by the driver. We only have a
> wake/reset-gpio.
>
>> + touchscreen-size-x = <720>;
>> + touchscreen-size-y = <1440>;
>> + };
>> +};
>> +
>> +&i2c4 {
>> + clock-frequency = <387000>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_i2c4>;
>> + status = "okay";
>> +
>> + bat: fuel-gauge@36 {
>> + compatible = "maxim,max17055";
>> + interrupt-parent = <&gpio3>;
>> + interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_gauge>;
>> + reg = <0x36>;
>
> Please check that "reg" is always the 2nd property after the
> "compatible".
>
>> + maxim,over-heat-temp = <700>;
>> + maxim,over-volt = <4500>;
>> + maxim,rsns-microohm = <5000>;
>> + };
>> +
>> + charger@6a { /* bq25895 */
>> + compatible = "ti,bq25890";
>
> The compatible should be "ti,bq25895" if it is a bq25895. So we can drop
> the comment too.
>
>> + reg = <0x6a>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_charger_in>;
>> + interrupt-parent = <&gpio3>;
>> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
>> + phys = <&usb3_phy0>;
>> + ti,battery-regulation-voltage = <4192000>; /* 4.192V */
>> + ti,charge-current = <1600000>; /* 1.6A */
>> + ti,termination-current = <66000>; /* 66mA */
>> + ti,precharge-current = <130000>; /* 130mA */
>> + ti,minimum-sys-voltage = <3700000>; /* 3.7V */
>> + ti,boost-voltage = <5000000>; /* 5V */
>> + ti,boost-max-current = <50000>; /* 50mA */
>> + ti,use-vinmin-threshold = <1>; /* enable VINDPM */
>> + ti,vinmin-threshold = <3900000>; /* 3.9V */
>
> I would only mention the units within a comment because comments like
> this begin to divergence after you fix something.
>
> Regards,
> Marco
>
Basically I try to follow all your suggestions that I haven't commented
here, too.
thank you very much for this in-depth review. That helps a lot and I try
to put together a second revision soon.
martin
^ permalink raw reply
* Re: [v2] drm/msm: add shutdown support for display platform_driver
From: Sai Prakash Ranjan @ 2020-06-02 16:10 UTC (permalink / raw)
To: Emil Velikov
Cc: Krishna Manikandan, ML dri-devel, linux-arm-msm, freedreno,
devicetree, Linux-Kernel@Vger. Kernel. Org, Sean Paul, kalyan_t,
Kristian H . Kristensen, mka, devicetree-owner
In-Reply-To: <CACvgo50b+m2+onak=AZfgihkBXEP9POjMR52087v==-puLdkQQ@mail.gmail.com>
Hi Emil,
On 2020-06-02 21:09, Emil Velikov wrote:
> On Tue, 2 Jun 2020 at 15:49, Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>>
>> Hi Emil,
>>
>> On 2020-06-02 19:43, Emil Velikov wrote:
>> > Hi Krishna,
>> >
>> > On Tue, 2 Jun 2020 at 08:17, Krishna Manikandan
>> > <mkrishn@codeaurora.org> wrote:
>> >>
>> >> Define shutdown callback for display drm driver,
>> >> so as to disable all the CRTCS when shutdown
>> >> notification is received by the driver.
>> >>
>> >> This change will turn off the timing engine so
>> >> that no display transactions are requested
>> >> while mmu translations are getting disabled
>> >> during reboot sequence.
>> >>
>> >> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
>> >>
>> > AFAICT atomics is setup in msm_drm_ops::bind and shutdown in
>> > msm_drm_ops::unbind.
>> >
>> > Are you saying that unbind never triggers? If so, then we should
>> > really fix that instead, since this patch seems more like a
>> > workaround.
>> >
>>
>> Which path do you suppose that the unbind should be called from,
>> remove
>> callback? Here we are talking about the drivers which are builtin,
>> where
>> remove callbacks are not called from the driver core during
>> reboot/shutdown,
>> instead shutdown callbacks are called which needs to be defined in
>> order
>> to
>> trigger unbind. So AFAICS there is nothing to be fixed.
>>
> Interesting - in drm effectively only drm panels implement .shutdown.
> So my naive assumption was that .remove was used implicitly by core,
> as part of the shutdown process. Yet that's not the case, so it seems
> that many drivers could use some fixes.
>
> Then again, that's an existing problem which is irrelevant for msm.
> -Emil
To give more context, we are actually targeting the clients/consumers
of SMMU/IOMMU here because we have to make sure that before the supplier
(SMMU) shuts down, its consumers/clients need to be shutdown properly.
Now the ordering of this is taken care in the SMMU driver via
device_link
which makes sure that consumer shutdown callbacks are called first, but
we
need to define shutdown callbacks for all its consumers to make sure we
actually shutdown the clients or else it would invite the crashes during
reboot
which in this case was seen for display.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* [PATCH 1/3] ARM: dts: NSP: Add common bindings for Meraki MX64/65
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel
In-Reply-To: <cover.1591114003.git.mnhagan88@gmail.com>
Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
arch/arm/boot/dts/bcm958625-mx6x-common.dtsi | 172 +++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm958625-mx6x-common.dtsi
diff --git a/arch/arm/boot/dts/bcm958625-mx6x-common.dtsi b/arch/arm/boot/dts/bcm958625-mx6x-common.dtsi
new file mode 100644
index 000000000000..1e253dd0941a
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx6x-common.dtsi
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Common Bindings for Cisco Meraki MX64 (Kingpin) and MX65 (Alamo) devices.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm-nsp.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x60000000 0x80000000>;
+ };
+
+ pwm-leds {
+ compatible = "pwm-leds";
+
+ red {
+ label = "pwm:led:red";
+ pwms = <&pwm 1 50000>;
+ };
+
+ green {
+ label = "pwm:led:green";
+ pwms = <&pwm 2 50000>;
+ };
+
+ blue {
+ label = "pwm:led:blue";
+ pwms = <&pwm 3 50000>;
+ };
+ };
+};
+
+&L2 {
+ arm,io-coherent;
+ prefetch-data = <1>;
+ prefetch-instr = <1>;
+};
+
+&uart0 {
+ clock-frequency = <62500000>;
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+ eeprom: at24@50 {
+ compatible = "atmel,24c64";
+ pagesize = <32>;
+ reg = <0x50>;
+ };
+};
+
+&amac2 {
+ status = "okay";
+};
+
+&nand {
+ nandcs@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ nand-on-flash-bbt;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ nand-ecc-strength = <24>;
+ nand-ecc-step-size = <1024>;
+
+ brcm,nand-oob-sector-size = <27>;
+
+ partition@0 {
+ label = "U-boot";
+ reg = <0x00 0x80000>;
+ read-only;
+ };
+
+ partition@80000 {
+ label = "Shmoo";
+ reg = <0x80000 0x80000>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "bootkernel1";
+ reg = <0x100000 0x300000>;
+ };
+
+ partition@400000 {
+ label = "senao_nvram";
+ reg = <0x400000 0x100000>;
+ };
+
+ partition@500000 {
+ label = "bootkernel2";
+ reg = <0x500000 0x300000>;
+ };
+
+ partition@800000 {
+ label = "ubi";
+ reg = <0x800000 0x3f700000>;
+ };
+ };
+};
+
+&ehci0 {
+ status = "okay";
+};
+
+&ohci0 {
+ status = "okay";
+};
+
+&pwm {
+ status = "okay";
+ #pwm-cells = <2>;
+ chan0 {
+ channel = <1>;
+ active_low = <1>;
+ };
+ chan1 {
+ channel = <2>;
+ active_low = <1>;
+ };
+ chan2 {
+ channel = <3>;
+ active_low = <1>;
+ };
+};
+
+&ccbtimer1 {
+ status = "disabled";
+};
+
+&pinctrl {
+ pinctrl-names = "default";
+ pinctrl-0 = <&nand_sel>, <&gpiobs>, <&pwmc>;
+
+ nand_sel: nand_sel {
+ function = "nand";
+ groups = "nand_grp";
+ };
+
+ gpiobs: gpiobs {
+ function = "gpio_b";
+ groups = "gpio_b_0_grp", "gpio_b_1_grp", "gpio_b_2_grp",
+ "gpio_b_3_grp";
+ };
+
+ pwmc: pwmc {
+ function = "pwm";
+ groups = "pwm0_grp", "pwm1_grp", "pwm2_grp", "pwm3_grp";
+ };
+
+ i2c_sel: i2c {
+ function = "i2c";
+ groups = "i2c_grp";
+ };
+};
+
+&sata_phy {
+ status = "disabled";
+};
--
2.25.4
^ permalink raw reply related
* [PATCH 2/3] ARM: dts: NSP: Add support for Cisco Meraki MX64(W)
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel
In-Reply-To: <cover.1591114003.git.mnhagan88@gmail.com>
Hardware Info
-------------
Processor - Broadcom BCM58625 dual-core @ 1.2 GHz
DDR3 RAM - 2GB (4x SK Hynix H5TC4G83CFR)
Flash - 1GB (Micron MT29F8G08ABACA)
Switch - Broadcom BCM58625
Wireless(MX64W) - Broadcom BCM43520KMLG (2x)
Ports - 5 Ports
Serial Port - 115200 8n1
USB - 1x 2.0
Tested with Kernel 5.4. PCIe is inactive on the non-wireless MX64.
Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
arch/arm/boot/dts/bcm958625-mx64.dts | 15 +++
arch/arm/boot/dts/bcm958625-mx64w.dts | 23 +++++
arch/arm/boot/dts/bcm958625-mx64x.dtsi | 136 +++++++++++++++++++++++++
3 files changed, 174 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm958625-mx64.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx64w.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx64x.dtsi
diff --git a/arch/arm/boot/dts/bcm958625-mx64.dts b/arch/arm/boot/dts/bcm958625-mx64.dts
new file mode 100644
index 000000000000..ec1017b8bf68
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx64.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX64.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx64x.dtsi"
+
+/ {
+ model = "Cisco Meraki MX64";
+ compatible = "meraki,mx64", "brcm,bcm58625", "brcm,nsp";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx64w.dts b/arch/arm/boot/dts/bcm958625-mx64w.dts
new file mode 100644
index 000000000000..a3fbf0fed218
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx64w.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX64W.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx64x.dtsi"
+
+/ {
+ model = "Cisco Meraki MX64W";
+ compatible = "meraki,mx64w", "brcm,bcm58625", "brcm,nsp";
+};
+
+&pcie0 {
+ status = "okay";
+};
+
+&pcie1 {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx64x.dtsi b/arch/arm/boot/dts/bcm958625-mx64x.dtsi
new file mode 100644
index 000000000000..4be3dd314beb
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx64x.dtsi
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX64 series (Kingpin).
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx6x-common.dtsi"
+
+#include <dt-bindings/input/input.h>
+
+/ {
+ leds {
+ compatible = "gpio-leds";
+
+ power_orange {
+ label = "power:orange";
+ gpios = <&gpioa 0 GPIO_ACTIVE_LOW>;
+ default-state = "on";
+ };
+
+ lan1_right {
+ label = "lan1:right";
+ gpios = <&gpioa 18 GPIO_ACTIVE_LOW>;
+ };
+
+ lan1_left {
+ label = "lan1:left";
+ gpios = <&gpioa 19 GPIO_ACTIVE_LOW>;
+ };
+
+ lan2_right {
+ label = "lan2:right";
+ gpios = <&gpioa 20 GPIO_ACTIVE_LOW>;
+ };
+
+ lan2_left {
+ label = "lan2:left";
+ gpios = <&gpioa 24 GPIO_ACTIVE_LOW>;
+ };
+
+ lan3_right {
+ label = "lan3:right";
+ gpios = <&gpioa 25 GPIO_ACTIVE_LOW>;
+ };
+
+ lan3_left {
+ label = "lan3:left";
+ gpios = <&gpioa 26 GPIO_ACTIVE_LOW>;
+ };
+
+ lan4_right {
+ label = "lan4:right";
+ gpios = <&gpioa 27 GPIO_ACTIVE_LOW>;
+ };
+
+ lan4_left {
+ label = "lan4:left";
+ gpios = <&gpioa 28 GPIO_ACTIVE_LOW>;
+ };
+
+ wan0_right {
+ label = "wan0:right";
+ gpios = <&gpioa 29 GPIO_ACTIVE_LOW>;
+ };
+
+ wan0_left {
+ label = "wan0:left";
+ gpios = <&gpioa 30 GPIO_ACTIVE_LOW>;
+ };
+
+ power_white {
+ label = "power:white";
+ gpios = <&gpioa 31 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ gpio-buttons {
+ compatible = "gpio-keys-polled";
+ autorepeat;
+ poll-interval = <20>;
+
+ reset {
+ label = "reset";
+ linux,code = <KEY_RESTART>;
+ gpios = <&gpioa 6 GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&srab {
+ compatible = "brcm,bcm58625-srab", "brcm,nsp-srab";
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ label = "lan1";
+ reg = <0>;
+ };
+
+ port@1 {
+ label = "lan2";
+ reg = <1>;
+ };
+
+ port@2 {
+ label = "lan3";
+ reg = <2>;
+ };
+
+ port@3 {
+ label = "lan4";
+ reg = <3>;
+ };
+
+ port@4 {
+ label = "wan0";
+ reg = <4>;
+ };
+
+ port@8 {
+ ethernet = <&amac2>;
+ label = "cpu";
+ reg = <8>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+};
--
2.25.4
^ permalink raw reply related
* [PATCH 0/3] ARM: dts: NSP: Add support for Cisco Meraki NSP devices
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel
This patch set adds support for the Meraki MX64(W) and MX65(W) security
devices. There are four devices in total, all using the same basic hardware.
The MX64 series has five ethernet ports connected to the BCM SRAB. The MX65
series has two ports conected to the SRAB, and two QCA8337 switches connected
by SGMII to SRAB ports 4 and 5, each providing five additional ports.
The W variants of these devices have two BCM43520s on the PCIe bus. On the
non-wireless variants PCIe is inactive, hence separate dts files.
1/3 contains common bindings for both Meraki devices.
2/3 contains MX64 specific bindings.
3/3 contains MX65 specific bindings.
Note that Chris Packham's "[PATCH 2/2] ARM: dts: NSP: avoid unnecessary probe
deferrals" is also necessary.
Thanks,
Matthew
Matthew Hagan (3):
ARM: dts: NSP: Add common bindings for Meraki MX64/65
ARM: dts: NSP: Add support for Cisco Meraki MX64(W)
ARM: dts: NSP: Add support for Cisco Meraki MX65(W)
arch/arm/boot/dts/bcm958625-mx64.dts | 15 +
arch/arm/boot/dts/bcm958625-mx64w.dts | 23 ++
arch/arm/boot/dts/bcm958625-mx64x.dtsi | 136 ++++++++
arch/arm/boot/dts/bcm958625-mx65.dts | 15 +
arch/arm/boot/dts/bcm958625-mx65w.dts | 23 ++
arch/arm/boot/dts/bcm958625-mx65x.dtsi | 321 +++++++++++++++++++
arch/arm/boot/dts/bcm958625-mx6x-common.dtsi | 172 ++++++++++
7 files changed, 705 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm958625-mx64.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx64w.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx64x.dtsi
create mode 100644 arch/arm/boot/dts/bcm958625-mx65.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx65w.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx65x.dtsi
create mode 100644 arch/arm/boot/dts/bcm958625-mx6x-common.dtsi
--
2.25.4
^ permalink raw reply
* [PATCH 3/3] ARM: dts: NSP: Add support for Cisco Meraki MX65(W)
From: Matthew Hagan @ 2020-06-02 16:11 UTC (permalink / raw)
Cc: Matthew Hagan, Florian Fainelli, Rob Herring, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, devicetree,
linux-arm-kernel
In-Reply-To: <cover.1591114003.git.mnhagan88@gmail.com>
Hardware Info
-------------
Processor - Broadcom BCM58625 dual-core @ 1.2 GHz
DDR3 RAM - 2GB (4x SK Hynix H5TC4G83CFR)
Flash - 1GB (Micron MT29F8G08ABACA)
Switches - Broadcom BCM58625, 2x Qualcomm Atheros QCA8337
Ports - 12 Ports
Wireless(MX65W) - Broadcom BCM43520KMLG (2X)
Serial Port - 115200 8n1
USB - 1x 2.0
Tested with Kernel 5.4. PCIe is inactive on non-wireless MX65.
Note: The QCA8337 switches are connected to ports 4 and 5 of the BCM58625
SRAB, which need be set to SGMII mode.
Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
---
arch/arm/boot/dts/bcm958625-mx65.dts | 15 ++
arch/arm/boot/dts/bcm958625-mx65w.dts | 23 ++
arch/arm/boot/dts/bcm958625-mx65x.dtsi | 321 +++++++++++++++++++++++++
3 files changed, 359 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm958625-mx65.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx65w.dts
create mode 100644 arch/arm/boot/dts/bcm958625-mx65x.dtsi
diff --git a/arch/arm/boot/dts/bcm958625-mx65.dts b/arch/arm/boot/dts/bcm958625-mx65.dts
new file mode 100644
index 000000000000..af161d268824
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx65.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX65.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx65x.dtsi"
+
+/ {
+ model = "Cisco Meraki MX65";
+ compatible = "meraki,mx65", "brcm,bcm58625", "brcm,nsp";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx65w.dts b/arch/arm/boot/dts/bcm958625-mx65w.dts
new file mode 100644
index 000000000000..67933ca7b598
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx65w.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX65W.
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx65x.dtsi"
+
+/ {
+ model = "Cisco Meraki MX65W";
+ compatible = "meraki,mx65w", "brcm,bcm58625", "brcm,nsp";
+};
+
+&pcie0 {
+ status = "okay";
+};
+
+&pcie1 {
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm958625-mx65x.dtsi b/arch/arm/boot/dts/bcm958625-mx65x.dtsi
new file mode 100644
index 000000000000..f69949be501e
--- /dev/null
+++ b/arch/arm/boot/dts/bcm958625-mx65x.dtsi
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Device Tree Bindings for Cisco Meraki MX65 series (Alamo).
+ *
+ * Copyright (C) 2020 Matthew Hagan <mnhagan88@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "bcm958625-mx6x-common.dtsi"
+
+#include <dt-bindings/input/input.h>
+
+/ {
+ aliases {
+ mdio-mux-mmio = &mdiomux0;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ power_orange {
+ label = "power:orange";
+ gpios = <&gpioa 3 GPIO_ACTIVE_HIGH>;
+ default-state = "on";
+ };
+
+ lan_leds {
+ label = "lan:leds";
+ gpios = <&gpioa 12 GPIO_ACTIVE_HIGH>;
+ };
+
+ wan1_right {
+ label = "wan1:right";
+ gpios = <&gpioa 24 GPIO_ACTIVE_LOW>;
+ };
+
+ wan1_left {
+ label = "wan1:left";
+ gpios = <&gpioa 25 GPIO_ACTIVE_LOW>;
+ };
+
+ wan2_right {
+ label = "wan2:right";
+ gpios = <&gpioa 26 GPIO_ACTIVE_LOW>;
+ };
+
+ wan2_left {
+ label = "wan2:left";
+ gpios = <&gpioa 27 GPIO_ACTIVE_LOW>;
+ };
+
+ power_white {
+ label = "power:white";
+ gpios = <&gpioa 31 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ gpio-buttons {
+ compatible = "gpio-keys-polled";
+ autorepeat;
+ poll-interval = <20>;
+
+ reset {
+ label = "reset";
+ linux,code = <KEY_RESTART>;
+ gpios = <&gpioa 8 GPIO_ACTIVE_LOW>;
+ };
+ };
+
+ mdio: mdio@18032000 {
+ compatible = "brcm,iproc-mdio";
+ reg = <0x18032000 0x8>;
+ #size-cells = <0>;
+ #address-cells = <1>;
+ };
+
+ mdiomux0: mdio-mux {
+ compatible = "mdio-mux-mmioreg";
+ reg = <0x18032000 0x4>;
+ mux-mask = <0x200>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mdio-parent-bus = <&mdio>;
+
+ mdio_int: mdio@0 {
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ };
+ mdio_ext: mdio@200 {
+ reg = <0x200>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ mdio-mii-mux {
+ compatible = "mdio-mux-mmioreg";
+ reg = <0x1803f1c0 0x4>;
+ mux-mask = <0x2000>;
+ mdio-parent-bus = <&mdio_ext>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mdio@0 {
+ reg = <0x0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_port6: phy@0 {
+ reg = <0>;
+ };
+
+ phy_port7: phy@1 {
+ reg = <1>;
+ };
+
+ phy_port8: phy@2 {
+ reg = <2>;
+ };
+
+ phy_port9: phy@3 {
+ reg = <3>;
+ };
+
+ phy_port10: phy@4 {
+ reg = <4>;
+ };
+
+ switch@10 {
+ compatible = "qca,qca8337";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10>;
+ dsa,member = <1 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ label = "cpu";
+ ethernet = <&sgmii1>;
+ phy-mode = "sgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan8";
+ phy-handle = <&phy_port6>;
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan9";
+ phy-handle = <&phy_port7>;
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan10";
+ phy-handle = <&phy_port8>;
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "lan11";
+ phy-handle = <&phy_port9>;
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "lan12";
+ phy-handle = <&phy_port10>;
+ };
+ };
+ };
+ };
+
+ mdio-mii@2000 {
+ reg = <0x2000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_port1: phy@0 {
+ reg = <0>;
+ };
+
+ phy_port2: phy@1 {
+ reg = <1>;
+ };
+
+ phy_port3: phy@2 {
+ reg = <2>;
+ };
+
+ phy_port4: phy@3 {
+ reg = <3>;
+ };
+
+ phy_port5: phy@4 {
+ reg = <4>;
+ };
+
+ switch@10 {
+ compatible = "qca,qca8337";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10>;
+ dsa,member = <2 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ label = "cpu";
+ ethernet = <&sgmii0>;
+ phy-mode = "sgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan3";
+ phy-handle = <&phy_port1>;
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan4";
+ phy-handle = <&phy_port2>;
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan5";
+ phy-handle = <&phy_port3>;
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "lan6";
+ phy-handle = <&phy_port4>;
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "lan7";
+ phy-handle = <&phy_port5>;
+ };
+ };
+ };
+ };
+ };
+};
+
+&srab {
+ compatible = "brcm,bcm58625-srab", "brcm,nsp-srab";
+ status = "okay";
+ dsa,member = <0 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ label = "wan1";
+ reg = <0>;
+ };
+
+ port@1 {
+ label = "wan2";
+ reg = <1>;
+ };
+
+ sgmii0: port@4 {
+ label = "sw0";
+ reg = <4>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ sgmii1: port@5 {
+ label = "sw1";
+ reg = <5>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ port@8 {
+ ethernet = <&amac2>;
+ label = "cpu";
+ reg = <8>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+};
+
+&pinctrl {
+ mdio_sel: mdio {
+ function = "mdio";
+ groups = "mdio_grp";
+ };
+};
--
2.25.4
^ permalink raw reply related
* Re: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
From: Bjorn Helgaas @ 2020-06-02 16:32 UTC (permalink / raw)
To: Ansuel Smith
Cc: Rob Herring, Sham Muthayyan, Rob Herring, Andy Gross,
Bjorn Andersson, Bjorn Helgaas, Mark Rutland, Stanimir Varbanov,
Lorenzo Pieralisi, Andrew Murray, Philipp Zabel, linux-arm-msm,
linux-pci, devicetree, linux-kernel
In-Reply-To: <20200602115353.20143-12-ansuelsmth@gmail.com>
On Tue, Jun 02, 2020 at 01:53:52PM +0200, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
>
> Add Force GEN1 support needed in some ipq8064 board that needs to limit
> some PCIe line to gen1 for some hardware limitation. This is set by the
> max-link-speed binding and needed by some soc based on ipq8064. (for
> example Netgear R7800 router)
>
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 259b627bf890..0ce15d53c46e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -27,6 +27,7 @@
> #include <linux/slab.h>
> #include <linux/types.h>
>
> +#include "../../pci.h"
> #include "pcie-designware.h"
>
> #define PCIE20_PARF_SYS_CTRL 0x00
> @@ -99,6 +100,8 @@
> #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE 0x358
> #define SLV_ADDR_SPACE_SZ 0x10000000
>
> +#define PCIE20_LNK_CONTROL2_LINK_STATUS2 0xa0
> +
> #define DEVICE_TYPE_RC 0x4
>
> #define QCOM_PCIE_2_1_0_MAX_SUPPLY 3
> @@ -195,6 +198,7 @@ struct qcom_pcie {
> struct phy *phy;
> struct gpio_desc *reset;
> const struct qcom_pcie_ops *ops;
> + int gen;
> };
>
> #define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
> @@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
> /* wait for clock acquisition */
> usleep_range(1000, 1500);
>
> + if (pcie->gen == 1) {
> + val = readl(pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
> + val |= 1;
Is this the same bit that's visible in config space as
PCI_EXP_LNKSTA_CLS_2_5GB? Why not use that #define?
There are a bunch of other #defines in this file that look like
redefinitions of standard things:
#define PCIE20_COMMAND_STATUS 0x04
Looks like PCI_COMMAND
#define CMD_BME_VAL 0x4
Looks like PCI_COMMAND_MASTER
#define PCIE20_DEVICE_CONTROL2_STATUS2 0x98
Looks like (PCIE20_CAP + PCI_EXP_DEVCTL2)
#define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
Looks like PCI_EXP_DEVCTL2_COMP_TMOUT_DIS
#define PCIE20_CAP 0x70
This one is obviously device-specific
#define PCIE20_CAP_LINK_CAPABILITIES (PCIE20_CAP + 0xC)
Looks like (PCIE20_CAP + PCI_EXP_LNKCAP)
#define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) | BIT(11))
Looks like PCI_EXP_LNKCAP_ASPMS
#define PCIE20_CAP_LINK_1 (PCIE20_CAP + 0x14)
#define PCIE_CAP_LINK1_VAL 0x2FD7F
This looks like PCIE20_CAP_LINK_1 should be (PCIE20_CAP +
PCI_EXP_SLTCAP), but "CAP_LINK_1" doesn't sound like the Slot
Capabilities register, and I don't know what PCIE_CAP_LINK1_VAL
means.
> + writel(val, pci->dbi_base + PCIE20_LNK_CONTROL2_LINK_STATUS2);
> + }
>
> /* Set the Max TLP size to 2K, instead of using default of 4K */
> writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
> @@ -1397,6 +1406,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> goto err_pm_runtime_put;
> }
>
> + pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
> + if (pcie->gen < 0)
> + pcie->gen = 2;
> +
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
> pcie->parf = devm_ioremap_resource(dev, res);
> if (IS_ERR(pcie->parf)) {
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH] ARM: dts: AM33xx-l4: add gpio-ranges
From: Drew Fustini @ 2020-06-02 16:34 UTC (permalink / raw)
To: Tony Lindgren
Cc: Grygorii Strashko, linux-omap, linux-kernel, Benoît Cousson,
Rob Herring, devicetree, Santosh Shilimkar, Suman Anna,
Haojian Zhuang, Linus Walleij, linux-gpio, jkridner,
robertcnelson
In-Reply-To: <20200602135155.GE37466@atomide.com>
On Tue, Jun 02, 2020 at 06:51:55AM -0700, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [200602 13:44]:
> >
> >
> > On 02/06/2020 16:14, Drew Fustini wrote:
> > > Add gpio-ranges properties to the gpio controller nodes.
> > >
> > > These gpio-ranges were created based on "Table 9-10. CONTROL_MODULE
> > > REGISTERS" in the "AM335x Technical Reference Manual" [0] and "Table
> > > 4-2. Pin Attributes" in the "AM335x Sitara Processor datasheet" [1].
> > > A csv file with this data is available for reference [2].
> >
> > It will be good if you can explain not only "what" is changed, but
> > also "why" it's needed in commit message.
>
> Also, please check (again) that this is the same for all the am3
> variants. For omap3, we had different pad assignments even between
> SoC revisions. Different pad routings should be easy to deal with
> in the dts if needed though.
>
> Regards,
>
> Tony
It appears that the only usage of am33xx-l4.dtsi is for am335x for which
specific parts mentioned in those dtsi files are 3352, 3358, and 3359.
$ git grep am33xx-l4.dtsi
arch/arm/boot/dts/am33xx.dtsi:#include "am33xx-l4.dtsi"
$ git grep -l '#include "am33xx.dtsi"' arch/ |wc -l
27
$ git grep -l '#include "am33xx.dtsi"' arch/ |grep -v am335x |wc -l
0
Also, it appears that the only AM33xx parts that actually exist are [0]:
AM3351, AM3352, AM3354, AM3356, AM3357, AM3358, AM3359
I clicked on the datasheet link for each product page and while the URL
has the specific part number in it [1], they all end up loading the
exact same PDF. The header states:
"AM3359, AM3358, AM3357, AM3356, AM3354, AM3352, AM3351
SPRS717L – OCTOBER 2011 – REVISED MARCH 2020"
Thus, I do believe all SoC's using am33xx-l4.dtsi would have the same
memory map for the pin control registers and the same relationshop from
pin to gpio line. For example, GPMC_A0 has mode 7 and it is labeled
gpio1_16. conf_gpmc_a0 is at offset 840h which makes it pin 16.
Maybe am33xx-l4.dtsi should have actually been named am335x-l4.dtsi?
Though I suppose there is no point in changing that now.
thanks,
drew
[0] http://www.ti.com/processors/sitara-arm/am335x-cortex-a8/overview.html
[1] https://www.ti.com/lit/ds/symlink/am3359.pdf
^ 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