* Re: [PATCH v8 09/14] media: rkisp1: add rockchip isp1 core driver
From: Sakari Ailus @ 2019-08-07 15:27 UTC (permalink / raw)
To: Helen Koike
Cc: linux-rockchip, devicetree, eddie.cai.linux, mchehab, heiko,
jacob2.chen, jeffy.chen, zyc, linux-kernel, tfiga, hans.verkuil,
laurent.pinchart, kernel, ezequiel, linux-media, linux-arm-kernel,
zhengsq, Jacob Chen, Allon Huang
In-Reply-To: <20190730184256.30338-10-helen.koike@collabora.com>
Hi Helen,
On Tue, Jul 30, 2019 at 03:42:51PM -0300, Helen Koike wrote:
> From: Jacob Chen <jacob2.chen@rock-chips.com>
>
> Add the core driver for rockchip isp1.
>
> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Signed-off-by: Yichong Zhong <zyc@rock-chips.com>
> Signed-off-by: Jacob Chen <cc@rock-chips.com>
> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Signed-off-by: Allon Huang <allon.huang@rock-chips.com>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> [fixed compilation and run time errors regarding new v4l2 async API]
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> [Add missing module device table]
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> [update for upstream]
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
>
> Changes in v8: None
> Changes in v7:
> - VIDEO_ROCKCHIP_ISP1 selects VIDEOBUF2_VMALLOC
> - add PHY_ROCKCHIP_DPHY as a dependency for VIDEO_ROCKCHIP_ISP1
> - Fix compilation and runtime errors due to bitrotting
> The code has bit-rotten since March 2018, fix compilation errors.
> The new V4L2 async notifier API requires notifiers to be initialized by
> a call to v4l2_async_notifier_init() before being used, do so.
> - Add missing module device table
> - use clk_bulk framework
> - add missing notifiers cleanups
> - s/strlcpy/strscpy
> - normalize bus_info name
> - fix s_stream error path, stream_cnt wans't being decremented properly
> - use devm_platform_ioremap_resource() helper
> - s/deice/device
> - redesign: remove mipi/csi subdevice, sensors connect directly to the
> isp subdevice in the media topology now.
> - remove "saved_state" member from rkisp1_stream struct
> - Reverse the order of MIs
> - Simplify MI interrupt handling
> Rather than adding unnecessary indirection, just use stream index to
> handle MI interrupt enable/disable/clear, since the stream index matches
> the order of bits now, thanks to previous patch. While at it, remove
> some dead code.
> - code styling and checkpatch fixes
>
> drivers/media/platform/Kconfig | 12 +
> drivers/media/platform/Makefile | 1 +
> drivers/media/platform/rockchip/isp1/Makefile | 7 +
> drivers/media/platform/rockchip/isp1/common.h | 101 +++
> drivers/media/platform/rockchip/isp1/dev.c | 675 ++++++++++++++++++
> drivers/media/platform/rockchip/isp1/dev.h | 97 +++
> 6 files changed, 893 insertions(+)
> create mode 100644 drivers/media/platform/rockchip/isp1/Makefile
> create mode 100644 drivers/media/platform/rockchip/isp1/common.h
> create mode 100644 drivers/media/platform/rockchip/isp1/dev.c
> create mode 100644 drivers/media/platform/rockchip/isp1/dev.h
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 89555f9a813f..e0e98937c565 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -106,6 +106,18 @@ config VIDEO_QCOM_CAMSS
> select VIDEOBUF2_DMA_SG
> select V4L2_FWNODE
>
> +config VIDEO_ROCKCHIP_ISP1
> + tristate "Rockchip Image Signal Processing v1 Unit driver"
> + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on ARCH_ROCKCHIP || COMPILE_TEST
> + select VIDEOBUF2_DMA_CONTIG
> + select VIDEOBUF2_VMALLOC
> + select V4L2_FWNODE
> + select PHY_ROCKCHIP_DPHY
> + default n
> + ---help---
> + Support for ISP1 on the rockchip SoC.
> +
> config VIDEO_S3C_CAMIF
> tristate "Samsung S3C24XX/S3C64XX SoC Camera Interface driver"
> depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 7cbbd925124c..f9fcf8e7c513 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
> obj-$(CONFIG_VIDEO_RENESAS_JPU) += rcar_jpu.o
> obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1/
>
> +obj-$(CONFIG_VIDEO_ROCKCHIP_ISP1) += rockchip/isp1/
> obj-$(CONFIG_VIDEO_ROCKCHIP_RGA) += rockchip/rga/
>
> obj-y += omap/
> diff --git a/drivers/media/platform/rockchip/isp1/Makefile b/drivers/media/platform/rockchip/isp1/Makefile
> new file mode 100644
> index 000000000000..72706e80fc8b
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/isp1/Makefile
> @@ -0,0 +1,7 @@
> +obj-$(CONFIG_VIDEO_ROCKCHIP_ISP1) += rockchip-isp1.o
> +rockchip-isp1-objs += rkisp1.o \
> + dev.o \
> + regs.o \
> + isp_stats.o \
> + isp_params.o \
> + capture.o
> diff --git a/drivers/media/platform/rockchip/isp1/common.h b/drivers/media/platform/rockchip/isp1/common.h
> new file mode 100644
> index 000000000000..606ce2793546
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/isp1/common.h
> @@ -0,0 +1,101 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Rockchip isp1 driver
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + */
> +
> +#ifndef _RKISP1_COMMON_H
> +#define _RKISP1_COMMON_H
> +
> +#include <linux/mutex.h>
> +#include <media/media-device.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#define RKISP1_DEFAULT_WIDTH 800
> +#define RKISP1_DEFAULT_HEIGHT 600
> +
> +#define RKISP1_MAX_STREAM 2
> +#define RKISP1_STREAM_MP 0
> +#define RKISP1_STREAM_SP 1
> +
> +#define RKISP1_PLANE_Y 0
> +#define RKISP1_PLANE_CB 1
> +#define RKISP1_PLANE_CR 2
> +
> +enum rkisp1_sd_type {
> + RKISP1_SD_SENSOR,
> + RKISP1_SD_PHY_CSI,
> + RKISP1_SD_VCM,
> + RKISP1_SD_FLASH,
> + RKISP1_SD_MAX,
> +};
I wonder if this is a leftover from the driver development time. Same goes
for the subdevs field in struct rkisp1_device.
> +
> +/* One structure per video node */
> +struct rkisp1_vdev_node {
> + struct vb2_queue buf_queue;
> + /* vfd lock */
> + struct mutex vlock;
> + struct video_device vdev;
> + struct media_pad pad;
> +};
> +
> +enum rkisp1_fmt_pix_type {
> + FMT_YUV,
> + FMT_RGB,
> + FMT_BAYER,
> + FMT_JPEG,
> + FMT_MAX
> +};
> +
> +enum rkisp1_fmt_raw_pat_type {
> + RAW_RGGB = 0,
> + RAW_GRBG,
> + RAW_GBRG,
> + RAW_BGGR,
> +};
> +
> +struct rkisp1_buffer {
> + struct vb2_v4l2_buffer vb;
> + struct list_head queue;
> + union {
> + u32 buff_addr[VIDEO_MAX_PLANES];
> + void *vaddr[VIDEO_MAX_PLANES];
> + };
> +};
> +
> +struct rkisp1_dummy_buffer {
> + void *vaddr;
> + dma_addr_t dma_addr;
> + u32 size;
> +};
> +
> +extern int rkisp1_debug;
> +
> +static inline
> +struct rkisp1_vdev_node *vdev_to_node(struct video_device *vdev)
> +{
> + return container_of(vdev, struct rkisp1_vdev_node, vdev);
> +}
> +
> +static inline struct rkisp1_vdev_node *queue_to_node(struct vb2_queue *q)
> +{
> + return container_of(q, struct rkisp1_vdev_node, buf_queue);
> +}
> +
> +static inline struct rkisp1_buffer *to_rkisp1_buffer(struct vb2_v4l2_buffer *vb)
> +{
> + return container_of(vb, struct rkisp1_buffer, vb);
> +}
> +
> +static inline struct vb2_queue *to_vb2_queue(struct file *file)
> +{
> + struct rkisp1_vdev_node *vnode = video_drvdata(file);
> +
> + return &vnode->buf_queue;
> +}
> +
> +#endif /* _RKISP1_COMMON_H */
> diff --git a/drivers/media/platform/rockchip/isp1/dev.c b/drivers/media/platform/rockchip/isp1/dev.c
> new file mode 100644
> index 000000000000..2b4a67e1a3b5
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/isp1/dev.c
> @@ -0,0 +1,675 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Rockchip isp1 driver
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/phy-mipi-dphy.h>
> +
> +#include "common.h"
> +#include "regs.h"
> +
> +struct isp_match_data {
> + const char * const *clks;
> + int size;
unsigned int
> +};
> +
> +struct sensor_async_subdev {
> + struct v4l2_async_subdev asd;
> + struct v4l2_mbus_config mbus;
> + unsigned int lanes;
> +};
> +
> +int rkisp1_debug;
> +module_param_named(debug, rkisp1_debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Debug level (0-1)");
Have you thought of using dynamic debug instead?
> +
> +/**************************** pipeline operations******************************/
> +
> +static int __isp_pipeline_prepare(struct rkisp1_pipeline *p,
> + struct media_entity *me)
> +{
> + struct rkisp1_device *dev = container_of(p, struct rkisp1_device, pipe);
> + struct v4l2_subdev *sd;
> + unsigned int i;
> +
> + p->num_subdevs = 0;
> + memset(p->subdevs, 0, sizeof(p->subdevs));
> +
> + while (1) {
> + struct media_pad *pad = NULL;
> +
> + /* Find remote source pad */
> + for (i = 0; i < me->num_pads; i++) {
> + struct media_pad *spad = &me->pads[i];
> +
> + if (!(spad->flags & MEDIA_PAD_FL_SINK))
> + continue;
> + pad = media_entity_remote_pad(spad);
> + if (pad)
> + break;
> + }
> +
> + if (!pad)
> + break;
> +
> + sd = media_entity_to_v4l2_subdev(pad->entity);
> + if (sd != &dev->isp_sdev.sd)
> + p->subdevs[p->num_subdevs++] = sd;
How do you make sure you don't overrun the array?
Instead, I'd avoid maintaining the array in the first place --- the same
information is available from the MC framework data structures --- see e.g.
the omap3isp driver.
> +
> + me = &sd->entity;
> + if (me->num_pads == 1)
> + break;
> + }
> + return 0;
> +}
> +
> +static int __subdev_set_power(struct v4l2_subdev *sd, int on)
> +{
> + int ret;
> +
> + if (!sd)
> + return -ENXIO;
> +
> + ret = v4l2_subdev_call(sd, core, s_power, on);
> +
> + return ret != -ENOIOCTLCMD ? ret : 0;
> +}
> +
> +static int __isp_pipeline_s_power(struct rkisp1_pipeline *p, bool on)
Could you instead use v4l2_pipeline_pm_use()?
> +{
> + struct rkisp1_device *dev = container_of(p, struct rkisp1_device, pipe);
> + int i, ret;
> +
> + if (on) {
> + __subdev_set_power(&dev->isp_sdev.sd, true);
> +
> + for (i = p->num_subdevs - 1; i >= 0; --i) {
> + ret = __subdev_set_power(p->subdevs[i], true);
> + if (ret < 0 && ret != -ENXIO)
> + goto err_power_off;
> + }
> + } else {
> + for (i = 0; i < p->num_subdevs; ++i)
> + __subdev_set_power(p->subdevs[i], false);
> +
> + __subdev_set_power(&dev->isp_sdev.sd, false);
> + }
> +
> + return 0;
> +
> +err_power_off:
> + for (++i; i < p->num_subdevs; ++i)
> + __subdev_set_power(p->subdevs[i], false);
> + __subdev_set_power(&dev->isp_sdev.sd, true);
> + return ret;
> +}
> +
> +static int rkisp1_pipeline_open(struct rkisp1_pipeline *p,
> + struct media_entity *me,
> + bool prepare)
> +{
> + int ret;
> +
> + if (WARN_ON(!p || !me))
> + return -EINVAL;
> + if (atomic_inc_return(&p->power_cnt) > 1)
> + return 0;
> +
> + /* go through media graphic and get subdevs */
> + if (prepare)
> + __isp_pipeline_prepare(p, me);
> +
> + if (!p->num_subdevs)
> + return -EINVAL;
> +
> + ret = __isp_pipeline_s_power(p, 1);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int rkisp1_pipeline_close(struct rkisp1_pipeline *p)
> +{
> + int ret;
> +
> + if (atomic_dec_return(&p->power_cnt) > 0)
> + return 0;
> + ret = __isp_pipeline_s_power(p, 0);
> +
> + return ret == -ENXIO ? 0 : ret;
> +}
> +
> +/*
> + * stream-on order: isp_subdev, mipi dphy, sensor
> + * stream-off order: mipi dphy, sensor, isp_subdev
> + */
> +static int rkisp1_pipeline_set_stream(struct rkisp1_pipeline *p, bool on)
> +{
> + struct rkisp1_device *dev = container_of(p, struct rkisp1_device, pipe);
> + int i, ret;
> +
> + if ((on && atomic_inc_return(&p->stream_cnt) > 1) ||
> + (!on && atomic_dec_return(&p->stream_cnt) > 0))
> + return 0;
> +
> + if (on) {
> + ret = v4l2_subdev_call(&dev->isp_sdev.sd, video, s_stream,
> + true);
> + if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
> + v4l2_err(&dev->v4l2_dev,
> + "s_stream failed on subdevice %s (%d)\n",
> + dev->isp_sdev.sd.name,
> + ret);
> + atomic_dec(&p->stream_cnt);
> + return ret;
> + }
> + }
> +
> + /* phy -> sensor */
> + for (i = 0; i < p->num_subdevs; ++i) {
> + ret = v4l2_subdev_call(p->subdevs[i], video, s_stream, on);
> + if (on && ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> + goto err_stream_off;
You should stop after the first external sub-device.
It seems even the omap3isp driver doesn't do that. It's not easy to spot
such issues indeed.
> + }
> +
> + if (!on)
> + v4l2_subdev_call(&dev->isp_sdev.sd, video, s_stream, false);
> +
> + return 0;
> +
> +err_stream_off:
> + for (--i; i >= 0; --i)
> + v4l2_subdev_call(p->subdevs[i], video, s_stream, false);
> + v4l2_subdev_call(&dev->isp_sdev.sd, video, s_stream, false);
> + atomic_dec(&p->stream_cnt);
> + return ret;
> +}
> +
> +/***************************** media controller *******************************/
> +/* See http://opensource.rock-chips.com/wiki_Rockchip-isp1 for Topology */
The host appears to be down, or there's a routing problem. Unless this is
fixed, having such a URL here doesn't do much good. :-I
> +
> +static int rkisp1_create_links(struct rkisp1_device *dev)
> +{
> + struct media_entity *source, *sink;
> + struct rkisp1_sensor *sensor;
> + unsigned int flags, pad;
> + int ret;
> +
> + /* sensor links(or mipi-phy) */
> + list_for_each_entry(sensor, &dev->sensors, list) {
> + for (pad = 0; pad < sensor->sd->entity.num_pads; pad++)
> + if (sensor->sd->entity.pads[pad].flags &
> + MEDIA_PAD_FL_SOURCE)
> + break;
Could you use media_entity_get_fwnode_pad() instead?
> +
> + if (pad == sensor->sd->entity.num_pads) {
> + dev_err(dev->dev,
> + "failed to find src pad for %s\n",
> + sensor->sd->name);
> +
> + return -ENXIO;
> + }
> +
> + ret = media_create_pad_link(
> + &sensor->sd->entity, pad,
> + &dev->isp_sdev.sd.entity,
> + RKISP1_ISP_PAD_SINK,
> + list_is_first(&sensor->list, &dev->sensors) ?
> + MEDIA_LNK_FL_ENABLED : 0);
> + if (ret) {
> + dev_err(dev->dev,
> + "failed to create link for %s\n",
> + sensor->sd->name);
> + return ret;
> + }
> + }
> +
> + /* params links */
> + source = &dev->params_vdev.vnode.vdev.entity;
> + sink = &dev->isp_sdev.sd.entity;
> + flags = MEDIA_LNK_FL_ENABLED;
> + ret = media_create_pad_link(source, 0, sink,
> + RKISP1_ISP_PAD_SINK_PARAMS, flags);
> + if (ret < 0)
> + return ret;
> +
> + /* create isp internal links */
> + /* SP links */
> + source = &dev->isp_sdev.sd.entity;
> + sink = &dev->stream[RKISP1_STREAM_SP].vnode.vdev.entity;
> + ret = media_create_pad_link(source, RKISP1_ISP_PAD_SOURCE_PATH,
> + sink, 0, flags);
> + if (ret < 0)
> + return ret;
> +
> + /* MP links */
> + source = &dev->isp_sdev.sd.entity;
> + sink = &dev->stream[RKISP1_STREAM_MP].vnode.vdev.entity;
> + ret = media_create_pad_link(source, RKISP1_ISP_PAD_SOURCE_PATH,
> + sink, 0, flags);
> + if (ret < 0)
> + return ret;
> +
> + /* 3A stats links */
> + source = &dev->isp_sdev.sd.entity;
> + sink = &dev->stats_vdev.vnode.vdev.entity;
> + return media_create_pad_link(source, RKISP1_ISP_PAD_SOURCE_STATS,
> + sink, 0, flags);
Indentation. Same for the calls to the same function above.
> +}
> +
> +static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *sd,
> + struct v4l2_async_subdev *asd)
> +{
> + struct rkisp1_device *isp_dev = container_of(notifier,
> + struct rkisp1_device,
> + notifier);
> + struct sensor_async_subdev *s_asd = container_of(asd,
> + struct sensor_async_subdev, asd);
> + struct rkisp1_sensor *sensor;
> +
> + sensor = devm_kzalloc(isp_dev->dev, sizeof(*sensor), GFP_KERNEL);
> + if (!sensor)
> + return -ENOMEM;
> +
> + sensor->lanes = s_asd->lanes;
> + sensor->mbus = s_asd->mbus;
> + sensor->sd = sd;
> + sensor->dphy = devm_phy_get(isp_dev->dev, "dphy");
> + if (IS_ERR(sensor->dphy)) {
> + if (PTR_ERR(sensor->dphy) != -EPROBE_DEFER)
> + dev_err(isp_dev->dev, "Couldn't get the MIPI D-PHY\n");
> + return PTR_ERR(sensor->dphy);
> + }
> + phy_init(sensor->dphy);
> +
> + list_add(&sensor->list, &isp_dev->sensors);
In general, maintaining the information on the external subdevs on your own
adds complexity to the driver. You can get the information when you need it
from the data structures maintained by MC (see e.g. the omap3isp driver for
examples).
> +
> + return 0;
> +}
> +
> +static struct rkisp1_sensor *sd_to_sensor(struct rkisp1_device *dev,
> + struct v4l2_subdev *sd)
> +{
> + struct rkisp1_sensor *sensor;
> +
> + list_for_each_entry(sensor, &dev->sensors, list)
> + if (sensor->sd == sd)
> + return sensor;
> +
> + return NULL;
> +}
> +
> +static void subdev_notifier_unbind(struct v4l2_async_notifier *notifier,
> + struct v4l2_subdev *sd,
> + struct v4l2_async_subdev *asd)
> +{
> + struct rkisp1_device *isp_dev = container_of(notifier,
> + struct rkisp1_device,
> + notifier);
> + struct rkisp1_sensor *sensor = sd_to_sensor(isp_dev, sd);
> +
> + /* TODO: check if a lock is required here */
> + list_del(&sensor->list);
> +
> + phy_exit(sensor->dphy);
> +}
> +
> +static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)
> +{
> + struct rkisp1_device *dev = container_of(notifier, struct rkisp1_device,
> + notifier);
> + int ret;
> +
> + mutex_lock(&dev->media_dev.graph_mutex);
> + ret = rkisp1_create_links(dev);
> + if (ret < 0)
> + goto unlock;
> + ret = v4l2_device_register_subdev_nodes(&dev->v4l2_dev);
> + if (ret < 0)
> + goto unlock;
> +
> + v4l2_info(&dev->v4l2_dev, "Async subdev notifier completed\n");
> +
> +unlock:
> + mutex_unlock(&dev->media_dev.graph_mutex);
> + return ret;
> +}
> +
> +static int rkisp1_fwnode_parse(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd)
> +{
> + struct sensor_async_subdev *s_asd =
> + container_of(asd, struct sensor_async_subdev, asd);
> +
> + if (vep->bus_type != V4L2_MBUS_CSI2_DPHY) {
> + dev_err(dev, "Only CSI2 bus type is currently supported\n");
> + return -EINVAL;
> + }
> +
> + if (vep->base.port != 0) {
> + dev_err(dev, "The ISP has only port 0\n");
> + return -EINVAL;
> + }
> +
> + s_asd->mbus.type = vep->bus_type;
> + s_asd->mbus.flags = vep->bus.mipi_csi2.flags;
> + s_asd->lanes = vep->bus.mipi_csi2.num_data_lanes;
> +
> + switch (vep->bus.mipi_csi2.num_data_lanes) {
> + case 1:
> + s_asd->mbus.flags |= V4L2_MBUS_CSI2_1_LANE;
> + break;
> + case 2:
> + s_asd->mbus.flags |= V4L2_MBUS_CSI2_2_LANE;
> + break;
> + case 3:
> + s_asd->mbus.flags |= V4L2_MBUS_CSI2_3_LANE;
> + break;
> + case 4:
> + s_asd->mbus.flags |= V4L2_MBUS_CSI2_4_LANE;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct v4l2_async_notifier_operations subdev_notifier_ops = {
> + .bound = subdev_notifier_bound,
> + .unbind = subdev_notifier_unbind,
> + .complete = subdev_notifier_complete,
> +};
> +
> +static int isp_subdev_notifier(struct rkisp1_device *isp_dev)
> +{
> + struct v4l2_async_notifier *ntf = &isp_dev->notifier;
> + struct device *dev = isp_dev->dev;
> + int ret;
> +
> + v4l2_async_notifier_init(ntf);
> +
> + ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> + dev, ntf, sizeof(struct sensor_async_subdev), 0,
> + rkisp1_fwnode_parse);
> + if (ret < 0)
> + return ret;
> +
> + if (list_empty(&ntf->asd_list))
> + return -ENODEV; /* no endpoint */
> +
> + ntf->ops = &subdev_notifier_ops;
> +
> + return v4l2_async_notifier_register(&isp_dev->v4l2_dev, ntf);
> +}
> +
> +/***************************** platform device *******************************/
> +
> +static int rkisp1_register_platform_subdevs(struct rkisp1_device *dev)
> +{
> + int ret;
> +
> + ret = rkisp1_register_isp_subdev(dev, &dev->v4l2_dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = rkisp1_register_stream_vdevs(dev);
> + if (ret < 0)
> + goto err_unreg_isp_subdev;
> +
> + ret = rkisp1_register_stats_vdev(&dev->stats_vdev, &dev->v4l2_dev, dev);
> + if (ret < 0)
> + goto err_unreg_stream_vdev;
> +
> + ret = rkisp1_register_params_vdev(&dev->params_vdev, &dev->v4l2_dev,
> + dev);
> + if (ret < 0)
> + goto err_unreg_stats_vdev;
> +
> + ret = isp_subdev_notifier(dev);
> + if (ret < 0) {
> + v4l2_err(&dev->v4l2_dev,
> + "Failed to register subdev notifier(%d)\n", ret);
> + goto err_unreg_params_vdev;
> + }
> +
> + return 0;
> +err_unreg_params_vdev:
> + rkisp1_unregister_params_vdev(&dev->params_vdev);
> +err_unreg_stats_vdev:
> + rkisp1_unregister_stats_vdev(&dev->stats_vdev);
> +err_unreg_stream_vdev:
> + rkisp1_unregister_stream_vdevs(dev);
> +err_unreg_isp_subdev:
> + rkisp1_unregister_isp_subdev(dev);
> + return ret;
> +}
> +
> +static const char * const rk3399_isp_clks[] = {
> + "clk_isp",
> + "aclk_isp",
> + "hclk_isp",
> + "aclk_isp_wrap",
> + "hclk_isp_wrap",
> +};
> +
> +static const char * const rk3288_isp_clks[] = {
> + "clk_isp",
> + "aclk_isp",
> + "hclk_isp",
> + "pclk_isp_in",
> + "sclk_isp_jpe",
> +};
> +
> +static const struct isp_match_data rk3288_isp_clk_data = {
> + .clks = rk3288_isp_clks,
> + .size = ARRAY_SIZE(rk3288_isp_clks),
> +};
> +
> +static const struct isp_match_data rk3399_isp_clk_data = {
> + .clks = rk3399_isp_clks,
> + .size = ARRAY_SIZE(rk3399_isp_clks),
> +};
> +
> +static const struct of_device_id rkisp1_plat_of_match[] = {
> + {
> + .compatible = "rockchip,rk3288-cif-isp",
> + .data = &rk3288_isp_clk_data,
> + }, {
> + .compatible = "rockchip,rk3399-cif-isp",
> + .data = &rk3399_isp_clk_data,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, rkisp1_plat_of_match);
> +
> +static irqreturn_t rkisp1_irq_handler(int irq, void *ctx)
> +{
> + struct device *dev = ctx;
> + struct rkisp1_device *rkisp1_dev = dev_get_drvdata(dev);
> + unsigned int mis_val;
> +
> + mis_val = readl(rkisp1_dev->base_addr + CIF_ISP_MIS);
> + if (mis_val)
> + rkisp1_isp_isr(mis_val, rkisp1_dev);
> +
> + mis_val = readl(rkisp1_dev->base_addr + CIF_MIPI_MIS);
> + if (mis_val)
> + rkisp1_mipi_isr(mis_val, rkisp1_dev);
> +
> + mis_val = readl(rkisp1_dev->base_addr + CIF_MI_MIS);
> + if (mis_val)
> + rkisp1_mi_isr(mis_val, rkisp1_dev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int rkisp1_plat_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + const struct isp_match_data *clk_data;
> + const struct of_device_id *match;
> + struct device *dev = &pdev->dev;
> + struct rkisp1_device *isp_dev;
> + struct v4l2_device *v4l2_dev;
> + unsigned int i;
> + int ret, irq;
> +
> + match = of_match_node(rkisp1_plat_of_match, node);
> + isp_dev = devm_kzalloc(dev, sizeof(*isp_dev), GFP_KERNEL);
> + if (!isp_dev)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&isp_dev->sensors);
> +
> + dev_set_drvdata(dev, isp_dev);
> + isp_dev->dev = dev;
> +
> + isp_dev->base_addr = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(isp_dev->base_addr))
> + return PTR_ERR(isp_dev->base_addr);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_irq(dev, irq, rkisp1_irq_handler, IRQF_SHARED,
> + dev_driver_string(dev), dev);
> + if (ret < 0) {
> + dev_err(dev, "request irq failed: %d\n", ret);
> + return ret;
> + }
> +
> + isp_dev->irq = irq;
> + clk_data = match->data;
> +
> + for (i = 0; i < clk_data->size; i++)
> + isp_dev->clks[i].id = clk_data->clks[i];
> + ret = devm_clk_bulk_get(dev, clk_data->size, isp_dev->clks);
> + if (ret)
> + return ret;
> + isp_dev->clk_size = clk_data->size;
> +
> + atomic_set(&isp_dev->pipe.power_cnt, 0);
> + atomic_set(&isp_dev->pipe.stream_cnt, 0);
> + isp_dev->pipe.open = rkisp1_pipeline_open;
> + isp_dev->pipe.close = rkisp1_pipeline_close;
> + isp_dev->pipe.set_stream = rkisp1_pipeline_set_stream;
> +
> + rkisp1_stream_init(isp_dev, RKISP1_STREAM_SP);
> + rkisp1_stream_init(isp_dev, RKISP1_STREAM_MP);
> +
> + strscpy(isp_dev->media_dev.model, "rkisp1",
> + sizeof(isp_dev->media_dev.model));
> + isp_dev->media_dev.dev = &pdev->dev;
> + strscpy(isp_dev->media_dev.bus_info,
> + "platform: " DRIVER_NAME, sizeof(isp_dev->media_dev.bus_info));
> + media_device_init(&isp_dev->media_dev);
> +
> + v4l2_dev = &isp_dev->v4l2_dev;
> + v4l2_dev->mdev = &isp_dev->media_dev;
> + strscpy(v4l2_dev->name, "rkisp1", sizeof(v4l2_dev->name));
> + v4l2_ctrl_handler_init(&isp_dev->ctrl_handler, 5);
> + v4l2_dev->ctrl_handler = &isp_dev->ctrl_handler;
> +
> + ret = v4l2_device_register(isp_dev->dev, &isp_dev->v4l2_dev);
> + if (ret < 0)
Once you've initialised the control handler, you'll need to free it in case
of an error. I.e. add one more label for that purpose near the end.
> + return ret;
> +
> + ret = media_device_register(&isp_dev->media_dev);
> + if (ret < 0) {
> + v4l2_err(v4l2_dev, "Failed to register media device: %d\n",
> + ret);
> + goto err_unreg_v4l2_dev;
> + }
> +
> + /* create & register platefom subdev (from of_node) */
> + ret = rkisp1_register_platform_subdevs(isp_dev);
> + if (ret < 0)
> + goto err_unreg_media_dev;
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + return 0;
> +
> +err_unreg_media_dev:
> + media_device_unregister(&isp_dev->media_dev);
> +err_unreg_v4l2_dev:
> + v4l2_device_unregister(&isp_dev->v4l2_dev);
> + return ret;
> +}
> +
> +static int rkisp1_plat_remove(struct platform_device *pdev)
> +{
> + struct rkisp1_device *isp_dev = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + media_device_unregister(&isp_dev->media_dev);
> + v4l2_async_notifier_unregister(&isp_dev->notifier);
> + v4l2_async_notifier_cleanup(&isp_dev->notifier);
> + v4l2_device_unregister(&isp_dev->v4l2_dev);
> + rkisp1_unregister_params_vdev(&isp_dev->params_vdev);
> + rkisp1_unregister_stats_vdev(&isp_dev->stats_vdev);
> + rkisp1_unregister_stream_vdevs(isp_dev);
> + rkisp1_unregister_isp_subdev(isp_dev);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused rkisp1_runtime_suspend(struct device *dev)
> +{
> + struct rkisp1_device *isp_dev = dev_get_drvdata(dev);
> +
> + clk_bulk_disable_unprepare(isp_dev->clk_size, isp_dev->clks);
> + return pinctrl_pm_select_sleep_state(dev);
> +}
> +
> +static int __maybe_unused rkisp1_runtime_resume(struct device *dev)
> +{
> + struct rkisp1_device *isp_dev = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = pinctrl_pm_select_default_state(dev);
> + if (ret < 0)
> + return ret;
> + ret = clk_bulk_prepare_enable(isp_dev->clk_size, isp_dev->clks);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops rkisp1_plat_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(rkisp1_runtime_suspend, rkisp1_runtime_resume, NULL)
> +};
> +
> +static struct platform_driver rkisp1_plat_drv = {
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = of_match_ptr(rkisp1_plat_of_match),
> + .pm = &rkisp1_plat_pm_ops,
> + },
> + .probe = rkisp1_plat_probe,
> + .remove = rkisp1_plat_remove,
> +};
> +
> +module_platform_driver(rkisp1_plat_drv);
> +MODULE_AUTHOR("Rockchip Camera/ISP team");
> +MODULE_DESCRIPTION("Rockchip ISP1 platform driver");
> +MODULE_LICENSE("Dual BSD/GPL");
BSD or MIT?
> diff --git a/drivers/media/platform/rockchip/isp1/dev.h b/drivers/media/platform/rockchip/isp1/dev.h
> new file mode 100644
> index 000000000000..f7cbee316523
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/isp1/dev.h
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Rockchip isp1 driver
> + *
> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> + */
> +
> +#ifndef _RKISP1_DEV_H
> +#define _RKISP1_DEV_H
> +
> +#include <linux/clk.h>
> +
> +#include "capture.h"
> +#include "rkisp1.h"
> +#include "isp_params.h"
> +#include "isp_stats.h"
> +
> +#define DRIVER_NAME "rkisp1"
> +#define ISP_VDEV_NAME DRIVER_NAME "_ispdev"
> +#define SP_VDEV_NAME DRIVER_NAME "_selfpath"
> +#define MP_VDEV_NAME DRIVER_NAME "_mainpath"
> +#define DMA_VDEV_NAME DRIVER_NAME "_dmapath"
> +
> +#define GRP_ID_SENSOR BIT(0)
> +#define GRP_ID_MIPIPHY BIT(1)
> +#define GRP_ID_ISP BIT(2)
> +#define GRP_ID_ISP_MP BIT(3)
> +#define GRP_ID_ISP_SP BIT(4)
> +
> +#define RKISP1_MAX_BUS_CLK 8
> +#define RKISP1_MAX_SENSOR 2
> +#define RKISP1_MAX_PIPELINE 4
> +
> +/*
> + * struct rkisp1_pipeline - An ISP hardware pipeline
> + *
> + * Capture device call other devices via pipeline
> + *
> + * @num_subdevs: number of linked subdevs
> + * @power_cnt: pipeline power count
> + * @stream_cnt: stream power count
> + */
> +struct rkisp1_pipeline {
> + struct media_pipeline pipe;
> + int num_subdevs;
> + atomic_t power_cnt;
> + atomic_t stream_cnt;
> + struct v4l2_subdev *subdevs[RKISP1_MAX_PIPELINE];
> + int (*open)(struct rkisp1_pipeline *p,
> + struct media_entity *me, bool prepare);
> + int (*close)(struct rkisp1_pipeline *p);
> + int (*set_stream)(struct rkisp1_pipeline *p, bool on);
> +};
> +
> +/*
> + * struct rkisp1_sensor - Sensor information
> + * @mbus: media bus configuration
> + */
> +struct rkisp1_sensor {
> + struct v4l2_subdev *sd;
> + struct v4l2_mbus_config mbus;
> + unsigned int lanes;
> + struct phy *dphy;
> + struct list_head list;
> +};
You seem to also have struct sensor_async_subdev that appears to contain
similar information. Would it be possible to unify the two?
This would appear to allow you getting rid of functions such as
sd_to_sensor, for instance.
> +
> +/*
> + * struct rkisp1_device - ISP platform device
> + * @base_addr: base register address
> + * @active_sensor: sensor in-use, set when streaming on
> + * @isp_sdev: ISP sub-device
> + * @rkisp1_stream: capture video device
> + * @stats_vdev: ISP statistics output device
> + * @params_vdev: ISP input parameters device
> + */
> +struct rkisp1_device {
> + void __iomem *base_addr;
> + int irq;
> + struct device *dev;
> + unsigned int clk_size;
> + struct clk_bulk_data clks[RKISP1_MAX_BUS_CLK];
> + struct v4l2_device v4l2_dev;
> + struct v4l2_ctrl_handler ctrl_handler;
> + struct media_device media_dev;
> + struct v4l2_async_notifier notifier;
> + struct v4l2_subdev *subdevs[RKISP1_SD_MAX];
> + struct rkisp1_sensor *active_sensor;
> + struct list_head sensors;
> + struct rkisp1_isp_subdev isp_sdev;
> + struct rkisp1_stream stream[RKISP1_MAX_STREAM];
> + struct rkisp1_isp_stats_vdev stats_vdev;
> + struct rkisp1_isp_params_vdev params_vdev;
> + struct rkisp1_pipeline pipe;
> + struct vb2_alloc_ctx *alloc_ctx;
> +};
> +
> +#endif
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
From: Pierre-Louis Bossart @ 2019-08-07 15:22 UTC (permalink / raw)
To: Daniel Baluta
Cc: Daniel Baluta, Marco Felsch, Shawn Guo, Mark Rutland,
Aisheng Dong, Peng Fan, Anson Huang, Devicetree List, S.j. Wang,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Fabio Estevam,
linux-arm-kernel, sound-open-firmware
In-Reply-To: <CAEnQRZDctjdzQ2RjJXhQh+s=d0y_j3Taa51hDaR4bqJ62C=7iQ@mail.gmail.com>
>>>> +static int sof_dt_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct device *dev = &pdev->dev;
>>>> + const struct sof_dev_desc *desc;
>>>> + /*TODO: create a generic snd_soc_xxx_mach */
>>>> + struct snd_soc_acpi_mach *mach;
>>>
>>> I wonder if you really need to use the same structures. For Intel we get
>>> absolutely zero info from the firmware so rely on an ACPI codec ID as a
>>> key to find information on which firmware and topology to use, and which
>>> machine driver to load. You could have all this information in a DT blob?
>>
>> Yes. I see your point. I will still need to make a generic structure for
>> snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
>>
>> Maybe something like this:
>>
>> struct snd_soc_mach {
>> union {
>> struct snd_soc_acpi_mach acpi_mach;
>> struct snd_soc_of_mach of_mach;
>> }
>> };
>>
>> and then change the prototype of sof_nocodec_setup.
>
> Hi Pierre,
>
> I fixed all the comments except the one above. Replacing snd_soc_acpi_mach
> with a generic snd_soc_mach is not trivial task.
>
> I wonder if it is acceptable to get the initial patches as they are
> now and later switch to
> generic ACPI/OF abstraction.
>
> Asking this because for the moment on the i.MX side I have only
> implemented no codec
> version and we don't probe any of the machine drivers we have.
>
> So, there is this only one member of snd_soc_acpi_mach that imx
> version is making use of:
>
> mach->drv_name = "sof-nocodec";
>
> That's all.
>
> I think the change as it is now is very clean and non-intrusive. Later
> we will get options to
> read firmware name and stuff from DT.
>
> Anyhow, I don't think we can get rid of snd_dev_desc structure on
> i.MX. This will be used
> to store the information read from DT:
>
> static struct sof_dev_desc sof_of_imx8qxp_desc = {
> » .default_fw_path = "imx/sof",
> » .default_tplg_path = "imx/sof-tplg",
> » .nocodec_fw_filename = "sof-imx8.ri",
> » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> » .ops = &sof_imx8_ops,
> };
>
> For the moment we will only use the default values.
Yes, that's fine for now. If you don't have a real machine driver then
there's nothing urgent to change.
Is the new version on GitHub?
Thanks
-Pierre
^ permalink raw reply
* [PATCH] arm64: dts: rockchip: add rk3328 VPU node
From: Jonas Karlman @ 2019-08-07 15:20 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Rob Herring, Arnaud Pouliquen, linux-rockchip@lists.infradead.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Jonas Karlman
This patch add a VPU device node for rk3328.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index e9fefd8a7e02..4a175fff2861 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -278,6 +278,7 @@
};
pd_vpu@RK3328_PD_VPU {
reg = <RK3328_PD_VPU>;
+ clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>;
};
};
@@ -596,6 +597,17 @@
status = "disabled";
};
+ vpu: video-codec@ff350000 {
+ compatible = "rockchip,rk3328-vpu";
+ reg = <0x0 0xff350000 0x0 0x800>;
+ interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "vdpu";
+ clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>;
+ clock-names = "aclk", "hclk";
+ iommus = <&vpu_mmu>;
+ power-domains = <&power RK3328_PD_VPU>;
+ };
+
vpu_mmu: iommu@ff350800 {
compatible = "rockchip,iommu";
reg = <0x0 0xff350800 0x0 0x40>;
@@ -604,7 +616,7 @@
clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>;
clock-names = "aclk", "iface";
#iommu-cells = <0>;
- status = "disabled";
+ power-domains = <&power RK3328_PD_VPU>;
};
rkvdec_mmu: iommu@ff360480 {
--
2.17.1
^ permalink raw reply related
* Re: [RFCv3 2/3] interconnect: Add imx core driver
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
To: Leonard Crestez, Rob Herring, Artur Świgoń,
Alexandre Bailon, Viresh Kumar
Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
linux-arm-kernel
In-Reply-To: <2b8905754d9a3fa6f4dc7b73b45649c85aa3e80a.1565088423.git.leonard.crestez@nxp.com>
Hi Leonard,
On 8/6/19 13:55, Leonard Crestez wrote:
> This adds support for i.MX SoC family to interconnect framework.
>
> Platform drivers can describe their interconnect graph and
> several adjustment knobs where an icc node bandwith converted to a
s/bandwith/bandwidth/
> clk_min_rate request.
>
> All adjustable nodes are assumed to be independent.
>
> Based on an earlier work by Alexandre Bailon but greatly reduced to drop
> "platform opp" support.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Your Signed-off-by should be below Alexandre's.
> ---
> drivers/interconnect/Kconfig | 1 +
> drivers/interconnect/Makefile | 1 +
> drivers/interconnect/imx/Kconfig | 5 +
> drivers/interconnect/imx/Makefile | 1 +
> drivers/interconnect/imx/imx.c | 258 ++++++++++++++++++++++++++++++
> drivers/interconnect/imx/imx.h | 62 +++++++
> 6 files changed, 328 insertions(+)
> create mode 100644 drivers/interconnect/imx/Kconfig
> create mode 100644 drivers/interconnect/imx/Makefile
> create mode 100644 drivers/interconnect/imx/imx.c
> create mode 100644 drivers/interconnect/imx/imx.h
>
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index bfa4ca3ab7a9..e61802230f90 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -10,7 +10,8 @@ menuconfig INTERCONNECT
> If unsure, say no.
>
> if INTERCONNECT
>
> source "drivers/interconnect/qcom/Kconfig"
> +source "drivers/interconnect/imx/Kconfig"
>
> endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index 28f2ab0824d5..20a13b7eb37f 100644
> --- a/drivers/interconnect/Makefile
> +++ b/drivers/interconnect/Makefile
> @@ -2,5 +2,6 @@
>
> icc-core-objs := core.o
>
> obj-$(CONFIG_INTERCONNECT) += icc-core.o
> obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/
> +obj-$(CONFIG_INTERCONNECT_IMX) += imx/
> diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
> new file mode 100644
> index 000000000000..45fbae7007af
> --- /dev/null
> +++ b/drivers/interconnect/imx/Kconfig
> @@ -0,0 +1,5 @@
> +config INTERCONNECT_IMX
> + bool "i.MX interconnect drivers"
> + depends on ARCH_MXC || ARCH_MXC_ARM64 || COMPILE_TEST
> + help
> + Generic interconnect driver for i.MX SOCs
> diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
> new file mode 100644
> index 000000000000..bb92fd9fe4a5
> --- /dev/null
> +++ b/drivers/interconnect/imx/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_INTERCONNECT_IMX) += imx.o
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> new file mode 100644
> index 000000000000..cc838e40419e
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.c
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
> +#include <linux/devfreq.h>
Please sort alphabetically.
> +#include <linux/of.h>
> +
> +#include "imx.h"
> +
> +/* private icc_provider data */
> +struct imx_icc_provider {
> + struct device *dev;
> +};
> +
> +/* private icc_node data */
> +struct imx_icc_node {
> + const struct imx_icc_node_desc *desc;
> + struct devfreq *devfreq;
> + struct dev_pm_qos_request qos_req;
> +};
> +
> +static int imx_icc_aggregate(struct icc_node *node, u32 avg_bw,
> + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> +{
> + *agg_avg += avg_bw;
> + *agg_peak = max(*agg_peak, peak_bw);
> +
> + return 0;
> +}
> +
> +static struct icc_node* imx_icc_xlate(struct of_phandle_args *spec, void *data)
> +{
> + struct imx_icc_provider *desc = data;
> + struct icc_provider *provider = dev_get_drvdata(desc->dev);
> + unsigned int id = spec->args[0];
> + struct icc_node *node;
> +
> + list_for_each_entry (node, &provider->nodes, node_list)
> + if (node->id == id)
> + return node;
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static int imx_icc_node_set(struct icc_node *node)
> +{
> + struct device *dev = node->provider->dev;
> + struct imx_icc_node *node_data = node->data;
> + unsigned long freq;
> +
> + if (!node_data->devfreq)
> + return 0;
> +
> + freq = (node->avg_bw + node->peak_bw) * node_data->desc->adj->bw_mul;
> + do_div(freq, node_data->desc->adj->bw_div);
> + if (freq > INT_MAX) {
> + dev_err(dev, "%s can't request more INT_MAX freq\n",
> + node->name);
> + return -ERANGE;
> + }
> +
> + dev_dbg(dev, "%s avg_bw %u peak_bw %u min_freq %lu\n",
> + node->name, node->avg_bw, node->peak_bw, freq);
> +
> + dev_pm_qos_update_request(&node_data->qos_req, freq);
> +
> + return 0;
> +}
> +
> +static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> + return imx_icc_node_set(dst);
> +}
> +
> +static int imx_icc_node_init_devfreq(struct device *dev,
> + struct icc_node *node)
> +{
> + struct imx_icc_node *node_data = node->data;
> + const struct imx_icc_node_desc *node_desc = node_data->desc;
> + int index;
> + int ret;
> +
> + index = of_property_match_string(dev->of_node,
> + "devfreq-names", node_desc->adj->devfreq_name);
> + if (index < 0) {
> + dev_err(dev, "failed to match devfreq-names %s: %d\n",
> + node_desc->adj->devfreq_name, index);
> + return index;
> + }
> +
> + node_data->devfreq = devfreq_get_devfreq_by_phandle(dev, index);
> + if (IS_ERR(node_data->devfreq)) {
> + ret = PTR_ERR(node_data->devfreq);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "failed to fetch devfreq %d %s: %d\n",
> + index, node_desc->adj->devfreq_name, ret);
> + return ret;
> + }
> +
> + return dev_pm_qos_add_request(node_data->devfreq->dev.parent,
> + &node_data->qos_req,
> + DEV_PM_QOS_MIN_FREQUENCY, 0);
> +}
> +
> +static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
> + const struct imx_icc_node_desc *node_desc)
> +{
> + struct imx_icc_provider *provider_data = provider->data;
> + struct device *dev = provider_data->dev;
> + struct imx_icc_node *node_data;
> + struct icc_node *node;
> + int ret;
> +
> + node = icc_node_create(node_desc->id);
> + if (IS_ERR(node)) {
> + dev_err(dev, "failed to create node %d\n", node_desc->id);
> + return node;
> + }
> +
> + if (node->data) {
> + dev_err(dev, "already created node %s id=%d\n",
> + node_desc->name, node_desc->id);
> + return ERR_PTR(-EEXIST);
> + }
> +
> + node_data = devm_kzalloc(dev, sizeof(*node_data), GFP_KERNEL);
> + if (!node_data) {
> + icc_node_destroy(node->id);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + node->name = node_desc->name;
> + node->data = node_data;
> + node_data->desc = node_desc;
> + if (node_desc->adj) {
> + ret = imx_icc_node_init_devfreq(dev, node);
> + if (ret < 0) {
> + icc_node_destroy(node->id);
> + return ERR_PTR(ret);
> + }
> + }
> +
> + icc_node_add(node, provider);
> +
> + return node;
> +}
> +
> +static void imx_icc_unregister_nodes(struct icc_provider *provider)
> +{
> + struct icc_node *node, *tmp;
> +
> + list_for_each_entry_safe(node, tmp, &provider->nodes, node_list) {
> + struct imx_icc_node *node_data = node->data;
> +
> + icc_node_del(node);
> + icc_node_destroy(node->id);
> + if (dev_pm_qos_request_active(&node_data->qos_req))
> + dev_pm_qos_remove_request(&node_data->qos_req);
> + }
> +}
> +
> +static int imx_icc_register_nodes(struct icc_provider *provider,
> + const struct imx_icc_node_desc *descs,
> + int count)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + struct icc_node *node;
> + const struct imx_icc_node_desc *node_desc = &descs[i];
> + size_t j;
> +
> + node = imx_icc_node_add(provider, node_desc);
> + if (IS_ERR(node)) {
> + ret = PTR_ERR(node);
> + if (ret != -EPROBE_DEFER)
> + dev_err(provider->dev, "failed to add %s: %d\n",
> + node_desc->name, ret);
> + goto err;
> + }
> +
> + for (j = 0; j < node_desc->num_links; j++)
> + icc_link_create(node, node_desc->links[j]);
> + }
> +
> + return 0;
> +
> +err:
> + imx_icc_unregister_nodes(provider);
> +
> + return ret;
> +}
> +
> +int imx_icc_register(struct platform_device *pdev,
> + struct imx_icc_node_desc *nodes, int nodes_count)
> +{
> + struct device *dev = &pdev->dev;
> + struct imx_icc_provider *desc;
> + struct icc_provider *provider;
> + int ret;
> +
> + desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
> + if (!desc)
> + return -ENOMEM;
> + desc->dev = dev;
> +
> + provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> + if (!provider)
> + return -ENOMEM;
> + provider->set = imx_icc_set;
> + provider->aggregate = imx_icc_aggregate;
> + provider->xlate = imx_icc_xlate;
> + provider->data = desc;
> + provider->dev = dev;
> + platform_set_drvdata(pdev, provider);
> +
> + ret = icc_provider_add(provider);
> + if (ret) {
> + dev_err(dev, "error adding interconnect provider\n");
> + return ret;
> + }
> +
> + ret = imx_icc_register_nodes(provider, nodes, nodes_count);
> + if (ret) {
> + dev_err(dev, "error adding interconnect nodes\n");
> + goto provider_del;
> + }
> +
> + return 0;
> +
> +provider_del:
> + icc_provider_del(provider);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_register);
> +
> +int imx_icc_unregister(struct platform_device *pdev)
> +{
> + struct icc_provider *provider = platform_get_drvdata(pdev);
> +
> + icc_provider_del(provider);
> + imx_icc_unregister_nodes(provider);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_icc_unregister);
> diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
> new file mode 100644
> index 000000000000..ab191eb89616
> --- /dev/null
> +++ b/drivers/interconnect/imx/imx.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright (c) 2019, BayLibre
> + * Copyright (c) 2019, NXP
> + * Author: Alexandre Bailon <abailon@baylibre.com>
> + * Author: Leonard Crestez <leonard.crestez@nxp.com>
> + */
> +#ifndef __BUSFREQ_H
> +#define __BUSFREQ_H
Maybe __DRIVERS_INTERCONNECT_IMX_IMX_H to match with the path and filename.
> +
> +#include <linux/kernel.h>
> +
> +#define IMX_ICC_MAX_LINKS 32
2 seems enough?
> +#define IMX_ICC_UNDEFINED_BW 0xffffffff
Is this used?
> +
> +/*
> + * struct imx_icc_node_adj - Describe an dynamic adjustment knob
s/an/a/
> + */
> +struct imx_icc_node_adj_desc {
> + const char *devfreq_name;
> + unsigned int bw_mul, bw_div;
> +};
> +
> +/*
> + * struct imx_icc_node - Describe an interconnect node
> + * @name: name of the node
> + * @id: an unique id to identify the node
> + * @links: an array of slaves' node id
> + * @num_links: number of id defined in links
> + */
> +struct imx_icc_node_desc {
> + const char *name;
> + u16 id;
> + u16 links[IMX_ICC_MAX_LINKS];
> + u16 num_links;
> +
> + const struct imx_icc_node_adj_desc *adj;
> +};
> +
> +#define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, _numlinks, ...) \
You can remove the _numlinks...
> + { \
> + .id = _id, \
> + .name = _name, \
> + .adj = _adj, \
> + .num_links = _numlinks, \
...and calculate the number of links automatically with:
.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })), \
> + .links = { __VA_ARGS__ }, \
> + }
> +
> +#define DEFINE_BUS_MASTER(_name, _id, _dest_id) \
> + DEFINE_BUS_INTERCONNECT(_name, _id, NULL, 1, _dest_id)
> +
> +#define DEFINE_BUS_SLAVE(_name, _id, _adj) \
> + DEFINE_BUS_INTERCONNECT(_name, _id, _adj, 0)
> +
> +int imx_icc_register(struct platform_device *pdev,
> + struct imx_icc_node_desc *nodes,
> + int nodes_count);
> +int imx_icc_unregister(struct platform_device *pdev);
> +
> +#endif /* __BUSFREQ_H */
Thanks,
Georgi
^ permalink raw reply
* Re: [RFCv3 1/3] dt-bindings: interconnect: Add bindings for imx
From: Georgi Djakov @ 2019-08-07 15:16 UTC (permalink / raw)
To: Leonard Crestez, Rob Herring, Artur Świgoń,
Alexandre Bailon, Viresh Kumar
Cc: Mark Rutland, Dong Aisheng, Jacky Bai, Saravana Kannan,
Anson Huang, Stephen Boyd, Michael Turquette, linux-pm,
Krzysztof Kozlowski, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
linux-imx, kernel, Fabio Estevam, Shawn Guo, devicetree,
linux-arm-kernel
In-Reply-To: <90561b14af66655ca859d387b3808a84641eea4e.1565088423.git.leonard.crestez@nxp.com>
Hi Leonard,
On 8/6/19 13:55, Leonard Crestez wrote:
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
Please put some commit text.
> .../devicetree/bindings/interconnect/imx.yaml | 38 +++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interconnect/imx.yaml
>
> diff --git a/Documentation/devicetree/bindings/interconnect/imx.yaml b/Documentation/devicetree/bindings/interconnect/imx.yaml
> new file mode 100644
> index 000000000000..c6f173b38f4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/imx.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX interconnect device
> +
> +maintainers:
> + - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8mm-interconnect
Maybe fsl,imx8mm-busfreq? I thought it's called busfreq in downstream, but it's
up to you.
> + "#interconnect-cells":
> + const: 1
> + devfreq-names:
> + description: Names of devfreq instances for adjustable nodes
> + devfreq:
> + description: List of phandle pointing to devfreq instances
> +
> +required:
> + - compatible
> + - "#interconnect-cells"
> + - "devfreq-names"
> + - "devfreq"
> +
> +examples:
> + - |
> + #include <dt-bindings/interconnect/imx8mm.h>
> + icc: interconnect {
> + compatible = "fsl,imx8mm-interconnect";
> + #interconnect-cells = <1>;
> + devfreq-names = "dram", "noc", "axi";
> + devfreq = <&ddrc>, <&noc>, <&pl301_main>;
> + };
>
Thanks,
Georgi
^ permalink raw reply
* Re: [PATCH v5 2/2] EDAC: add EDAC driver for DMC520
From: Borislav Petkov @ 2019-08-07 14:40 UTC (permalink / raw)
To: Lei Wang
Cc: james.morse@arm.com, robh+dt@kernel.org, mark.rutland@arm.com,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
mchehab@kernel.org, linux-edac@vger.kernel.org, sashal@kernel.org,
hangl@microsoft.com, lewan@microsoft.com, ruizhao@microsoft.com
In-Reply-To: <BN6PR04MB1107CE3C2D666A806E62851B86C10@BN6PR04MB1107.namprd04.prod.outlook.com>
On Thu, Jul 25, 2019 at 12:49:27AM +0000, Lei Wang wrote:
> New driver supports error detection and correction on the devices with ARM
> DMC-520 memory controller.
>
> Signed-off-by: Lei Wang <leiwang_git@outlook.com>
> Reviewed-by: James Morse <james.morse@arm.com>
> ---
> Changes in v5:
> - When enable configured interrupts, do not potentially disable
> other irrelevant interrupts.
> - fix return type of dmc520_get_scrub_type to enum scrub_type
> - Use capital letters for enum
> - Retrieve memory width from dmc register format_control
> - use cheaper spin_lock API.
> - Remove compatible line for "brcm,dmc-520"
> - Add James' Reviewed-by
> ---
> MAINTAINERS | 6 +
> drivers/edac/Kconfig | 7 +
> drivers/edac/Makefile | 1 +
> drivers/edac/dmc520_edac.c | 632 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 646 insertions(+)
> create mode 100644 drivers/edac/dmc520_edac.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 77eae44bf5de..7c1ac8bc8ea1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5703,6 +5703,12 @@ F: Documentation/driver-api/edac.rst
> F: drivers/edac/
> F: include/linux/edac.h
>
> +EDAC-DMC520
> +M: Lei Wang <lewan@microsoft.com>
> +L: linux-edac@vger.kernel.org
> +S: Supported
> +F: drivers/edac/dmc520_edac.c
> +
> EDAC-E752X
> M: Mark Gross <mark.gross@intel.com>
> L: linux-edac@vger.kernel.org
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 200c04ce5b0e..7fde5aea0c1a 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -510,4 +510,11 @@ config EDAC_ASPEED
> First, ECC must be configured in the bootloader. Then, this driver
> will expose error counters via the EDAC kernel framework.
>
> +config EDAC_DMC520
> + tristate "ARM DMC-520 ECC"
> + depends on ARM64
> + help
> + Support for error detection and correction on the
> + SoCs with ARM DMC-520 DRAM controller.
> +
> endif # EDAC
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 165ca65e1a3a..e0d98f5b2045 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -85,3 +85,4 @@ obj-$(CONFIG_EDAC_XGENE) += xgene_edac.o
> obj-$(CONFIG_EDAC_TI) += ti_edac.o
> obj-$(CONFIG_EDAC_QCOM) += qcom_edac.o
> obj-$(CONFIG_EDAC_ASPEED) += aspeed_edac.o
> +obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o
> diff --git a/drivers/edac/dmc520_edac.c b/drivers/edac/dmc520_edac.c
> new file mode 100644
> index 000000000000..aeb8d84405cf
> --- /dev/null
> +++ b/drivers/edac/dmc520_edac.c
> @@ -0,0 +1,632 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* EDAC driver for DMC-520 */
That comment is kinda useless.
> +#include <linux/bitfield.h>
> +#include <linux/edac.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include "edac_mc.h"
> +
> +/* DMC-520 registers */
> +#define REG_OFFSET_FEATURE_CONFIG 0x130
> +#define REG_OFFSET_ECC_ERRC_COUNT_31_00 0x158
> +#define REG_OFFSET_ECC_ERRC_COUNT_63_32 0x15C
> +#define REG_OFFSET_ECC_ERRD_COUNT_31_00 0x160
> +#define REG_OFFSET_ECC_ERRD_COUNT_63_32 0x164
> +#define REG_OFFSET_INTERRUPT_CONTROL 0x500
> +#define REG_OFFSET_INTERRUPT_CLR 0x508
> +#define REG_OFFSET_INTERRUPT_STATUS 0x510
> +#define REG_OFFSET_DRAM_ECC_ERRC_INT_INFO_31_00 0x528
> +#define REG_OFFSET_DRAM_ECC_ERRC_INT_INFO_63_32 0x52C
> +#define REG_OFFSET_DRAM_ECC_ERRD_INT_INFO_31_00 0x530
> +#define REG_OFFSET_DRAM_ECC_ERRD_INT_INFO_63_32 0x534
> +#define REG_OFFSET_ADDRESS_CONTROL_NOW 0x1010
> +#define REG_OFFSET_MEMORY_TYPE_NOW 0x1128
> +#define REG_OFFSET_SCRUB_CONTROL0_NOW 0x1170
> +#define REG_OFFSET_FORMAT_CONTROL 0x18
> +
> +/* DMC-520 types, masks and bitfields */
> +#define DRAM_ECC_INT_CE_BIT BIT(2)
> +#define DRAM_ECC_INT_UE_BIT BIT(3)
> +#define ALL_INT_MASK GENMASK(9, 0)
> +#define MEMORY_WIDTH_MASK GENMASK(1, 0)
> +#define SCRUB_TRIGGER0_NEXT_MASK GENMASK(1, 0)
> +#define REG_FIELD_DRAM_ECC_ENABLED GENMASK(1, 0)
> +#define REG_FIELD_MEMORY_TYPE GENMASK(2, 0)
> +#define REG_FIELD_DEVICE_WIDTH GENMASK(9, 8)
> +#define REG_FIELD_ADDRESS_CONTROL_COL GENMASK(2, 0)
> +#define REG_FIELD_ADDRESS_CONTROL_ROW GENMASK(10, 8)
> +#define REG_FIELD_ADDRESS_CONTROL_BANK GENMASK(18, 16)
> +#define REG_FIELD_ADDRESS_CONTROL_RANK GENMASK(25, 24)
> +#define REG_FIELD_ERR_INFO_LOW_VALID BIT(0)
> +#define REG_FIELD_ERR_INFO_LOW_COL GENMASK(10, 1)
> +#define REG_FIELD_ERR_INFO_LOW_ROW GENMASK(28, 11)
> +#define REG_FIELD_ERR_INFO_LOW_RANK GENMASK(31, 29)
> +#define REG_FIELD_ERR_INFO_HIGH_BANK GENMASK(3, 0)
> +#define REG_FIELD_ERR_INFO_HIGH_VALID BIT(31)
> +
> +#define DRAM_ADDRESS_CONTROL_MIN_COL_BITS 8
> +#define DRAM_ADDRESS_CONTROL_MIN_ROW_BITS 11
> +
> +#define DMC520_SCRUB_TRIGGER_ERR_DETECT 2
> +#define DMC520_SCRUB_TRIGGER_IDLE 3
One tab too many. Keep them all vertically aligned.
> +
> +/* Driver settings */
> +/* The max-length message would be: "rank:7 bank:15 row:262143 col:1023".
> + * Max length is 34. Using a 40-size buffer is enough.
> + */
> +#define EDAC_MSG_BUF_SIZE 40
That define is driver-local only. Prefix it with "DMC520_" or so, not
with "EDAC_".
> +#define EDAC_MOD_NAME "dmc520-edac"
> +#define EDAC_CTL_NAME "dmc520"
> +
> +/* the data bus width for the attached memory chips. */
> +enum dmc520_mem_width {
> + MEM_WIDTH_X32 = 2,
> + MEM_WIDTH_X64 = 3
> +};
> +
> +/* memory type */
> +enum dmc520_mem_type {
> + MEM_TYPE_DDR3 = 1,
> + MEM_TYPE_DDR4 = 2
> +};
> +
> +/* memory device width */
> +enum dmc520_dev_width {
> + DEV_WIDTH_X4 = 0,
> + DEV_WIDTH_X8 = 1,
> + DEV_WIDTH_X16 = 2
> +};
> +
> +struct ecc_error_info {
> + u32 col;
> + u32 row;
> + u32 bank;
> + u32 rank;
> +};
> +
> +/* The EDAC driver private data */
> +struct dmc520_edac {
> + void __iomem *reg_base;
> + u32 nintr;
> + u32 interrupt_mask_all;
> + spinlock_t ecc_lock;
> + u32 interrupt_masks[0];
> +};
> +
> +static int dmc520_mc_idx;
> +
> +static irqreturn_t
> +dmc520_edac_dram_all_isr(int irq, struct mem_ctl_info *mci, u32 interrupt_mask);
> +
> +#define DECLARE_ISR(index) \
> +static irqreturn_t dmc520_isr_##index (int irq, void *data) \
Please integrate scripts/checkpatch.pl into your patch creation
workflow. Some of the warnings/errors *actually* make sense.
WARNING: space prohibited between function name and open parenthesis '('
#200: FILE: drivers/edac/dmc520_edac.c:108:
+static irqreturn_t dmc520_isr_##index (int irq, void *data) \
> +{ \
> + struct mem_ctl_info *mci; \
> + struct dmc520_edac *edac; \
> + mci = data; \
> + edac = mci->pvt_info; \
> + return dmc520_edac_dram_all_isr(irq, mci, edac->interrupt_masks[index]); \
> +}
> +
> +DECLARE_ISR(0)
> +DECLARE_ISR(1)
> +/* More DECLARE_ISR(index) can be added to support more interrupt lines. */
> +
> +irq_handler_t dmc520_isr_array[] = {
> + dmc520_isr_0,
> + dmc520_isr_1
> + /* More dmc520_isr_index can be added to support more interrupt lines. */
What are those comments really for?
> +};
> +
> +static u32 dmc520_read_reg(struct dmc520_edac *edac, u32 offset)
> +{
> + return readl(edac->reg_base + offset);
> +}
> +
> +static void dmc520_write_reg(struct dmc520_edac *edac, u32 val, u32 offset)
> +{
> + writel(val, edac->reg_base + offset);
> +}
> +
> +static u32 dmc520_calc_dram_ecc_error(u32 value)
> +{
> + u32 total = 0;
> +
> + /* Each rank's error counter takes one byte */
> + while (value > 0) {
> + total += (value & 0xFF);
> + value >>= 8;
> + }
> + return total;
> +}
> +
> +static u32 dmc520_get_dram_ecc_error_count(struct dmc520_edac *edac,
> + bool is_ce)
> +{
> + u32 reg_offset_low, reg_offset_high;
> + u32 err_low, err_high;
> + u32 err_count;
> +
> + reg_offset_low = is_ce ? REG_OFFSET_ECC_ERRC_COUNT_31_00 :
> + REG_OFFSET_ECC_ERRD_COUNT_31_00;
> + reg_offset_high = is_ce ? REG_OFFSET_ECC_ERRC_COUNT_63_32 :
> + REG_OFFSET_ECC_ERRD_COUNT_63_32;
> +
> + err_low = dmc520_read_reg(edac, reg_offset_low);
> + err_high = dmc520_read_reg(edac, reg_offset_high);
> + /* Reset error counters */
> + dmc520_write_reg(edac, 0, reg_offset_low);
> + dmc520_write_reg(edac, 0, reg_offset_high);
> +
> + err_count = dmc520_calc_dram_ecc_error(err_low) +
> + dmc520_calc_dram_ecc_error(err_high);
> +
> + return err_count;
> +}
> +
> +static void dmc520_get_dram_ecc_error_info(struct dmc520_edac *edac,
> + bool is_ce,
> + struct ecc_error_info *info)
> +{
> + u32 reg_offset_low, reg_offset_high;
> + u32 reg_val_low, reg_val_high;
> + bool valid;
> +
> + reg_offset_low = is_ce ? REG_OFFSET_DRAM_ECC_ERRC_INT_INFO_31_00 :
> + REG_OFFSET_DRAM_ECC_ERRD_INT_INFO_31_00;
> + reg_offset_high = is_ce ? REG_OFFSET_DRAM_ECC_ERRC_INT_INFO_63_32 :
> + REG_OFFSET_DRAM_ECC_ERRD_INT_INFO_63_32;
> +
> + reg_val_low = dmc520_read_reg(edac, reg_offset_low);
> + reg_val_high = dmc520_read_reg(edac, reg_offset_high);
> +
> + valid = (FIELD_GET(REG_FIELD_ERR_INFO_LOW_VALID, reg_val_low) != 0) &&
> + (FIELD_GET(REG_FIELD_ERR_INFO_HIGH_VALID, reg_val_high) != 0);
> +
> + if (valid) {
> + info->col = FIELD_GET(REG_FIELD_ERR_INFO_LOW_COL, reg_val_low);
> + info->row = FIELD_GET(REG_FIELD_ERR_INFO_LOW_ROW, reg_val_low);
> + info->rank = FIELD_GET(REG_FIELD_ERR_INFO_LOW_RANK, reg_val_low);
> + info->bank = FIELD_GET(REG_FIELD_ERR_INFO_HIGH_BANK, reg_val_high);
> + } else {
> + memset(info, 0, sizeof(struct ecc_error_info));
> + }
> +}
> +
> +static bool dmc520_is_ecc_enabled(void __iomem *reg_base)
> +{
> + u32 reg_val = readl(reg_base + REG_OFFSET_FEATURE_CONFIG);
> +
> + return (FIELD_GET(REG_FIELD_DRAM_ECC_ENABLED, reg_val) != 0);
No need for that " != 0" at the end.
> +}
> +
> +static enum scrub_type dmc520_get_scrub_type(struct dmc520_edac *edac)
> +{
> + enum scrub_type type = SCRUB_NONE;
> + u32 reg_val, scrub_cfg;
> +
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_SCRUB_CONTROL0_NOW);
> + scrub_cfg = FIELD_GET(SCRUB_TRIGGER0_NEXT_MASK, reg_val);
> +
> + if (DMC520_SCRUB_TRIGGER_ERR_DETECT == scrub_cfg ||
> + DMC520_SCRUB_TRIGGER_IDLE == scrub_cfg)
> + type = SCRUB_HW_PROG;
More from checkpatch:
WARNING: Comparisons should place the constant on the right side of the test
#309: FILE: drivers/edac/dmc520_edac.c:217:
+ if (DMC520_SCRUB_TRIGGER_ERR_DETECT == scrub_cfg ||
WARNING: Comparisons should place the constant on the right side of the test
#310: FILE: drivers/edac/dmc520_edac.c:218:
+ DMC520_SCRUB_TRIGGER_IDLE == scrub_cfg)
> +
> + return type;
> +}
> +
> +/* Get the memory data bus width, in number of bytes. */
> +static u32 dmc520_get_memory_width(struct dmc520_edac *edac)
> +{
> + static u32 mem_width;
> + u32 reg_val;
> + enum dmc520_mem_width mem_width_field;
Please sort function local variables declaration in a reverse christmas
tree order:
<type A> longest_variable_name;
<type B> shorter_var_name;
<type C> even_shorter;
<type D> i;
Here and in the other functions.
> +
> + if (mem_width == 0) {
Flip that check and save yourself an indentation level:
if (mem_width)
return;
reg_val = ...
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_FORMAT_CONTROL);
> + mem_width_field = FIELD_GET(MEMORY_WIDTH_MASK, reg_val);
> +
> + if (mem_width_field == MEM_WIDTH_X32)
> + mem_width = 4; /* 32-bits, 4 bytes */
> + else if (mem_width_field == MEM_WIDTH_X64)
> + mem_width = 8; /* 64-bits, 8 bytes */
Please no side-comments. And those you can directly remove as they're
useless - your defines and variables should be named properly so that
there is no need for a comment here additionally.
> + }
> +
> + return mem_width;
> +}
> +
> +static enum mem_type dmc520_get_mtype(struct dmc520_edac *edac)
> +{
> + enum mem_type mt = MEM_UNKNOWN;
> + u32 reg_val;
> + enum dmc520_mem_type type;
> +
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_MEMORY_TYPE_NOW);
> + type = FIELD_GET(REG_FIELD_MEMORY_TYPE, reg_val);
> +
> + switch (type) {
> + case MEM_TYPE_DDR3:
> + mt = MEM_DDR3;
> + break;
> +
> + case MEM_TYPE_DDR4:
> + mt = MEM_DDR4;
> + break;
> + }
> +
> + return mt;
> +}
> +
> +static enum dev_type dmc520_get_dtype(struct dmc520_edac *edac)
> +{
> + enum dev_type dt = DEV_UNKNOWN;
> + u32 reg_val;
> + enum dmc520_dev_width device_width;
> +
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_MEMORY_TYPE_NOW);
> + device_width = FIELD_GET(REG_FIELD_DEVICE_WIDTH, reg_val);
> +
> + switch (device_width) {
> + case DEV_WIDTH_X4:
> + dt = DEV_X4;
> + break;
> +
> + case DEV_WIDTH_X8:
> + dt = DEV_X8;
> + break;
> +
> + case DEV_WIDTH_X16:
> + dt = DEV_X16;
> + break;
> + }
> +
> + return dt;
> +}
> +
> +static u32 dmc520_get_rank_count(void __iomem *reg_base)
> +{
> + u32 reg_val, rank_bits;
> +
> + reg_val = readl(reg_base + REG_OFFSET_ADDRESS_CONTROL_NOW);
> + rank_bits = FIELD_GET(REG_FIELD_ADDRESS_CONTROL_RANK, reg_val);
> +
> + return (1 << rank_bits);
return BIT(rank_bits);
> +}
> +
> +static u64 dmc520_get_rank_size(struct dmc520_edac *edac)
> +{
> + u32 reg_val, col_bits, row_bits, bank_bits;
> +
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_ADDRESS_CONTROL_NOW);
> +
> + col_bits = FIELD_GET(REG_FIELD_ADDRESS_CONTROL_COL, reg_val) +
> + DRAM_ADDRESS_CONTROL_MIN_COL_BITS;
> + row_bits = FIELD_GET(REG_FIELD_ADDRESS_CONTROL_ROW, reg_val) +
> + DRAM_ADDRESS_CONTROL_MIN_ROW_BITS;
> + bank_bits = FIELD_GET(REG_FIELD_ADDRESS_CONTROL_BANK, reg_val);
> +
> + return (u64)dmc520_get_memory_width(edac) << (col_bits + row_bits + bank_bits);
> +}
> +
> +static void dmc520_handle_dram_ecc_errors(struct mem_ctl_info *mci,
> + bool is_ce)
> +{
> + struct ecc_error_info info;
> + struct dmc520_edac *edac;
> + u32 cnt;
> + char message[EDAC_MSG_BUF_SIZE];
> +
> + edac = mci->pvt_info;
> + dmc520_get_dram_ecc_error_info(edac, is_ce, &info);
> +
> + cnt = dmc520_get_dram_ecc_error_count(edac, is_ce);
> +
> + if (cnt > 0) {
Save an indentation level:
if (!cnt)
return;
snprintf...
> + snprintf(message, ARRAY_SIZE(message),
> + "rank:%d bank:%d row:%d col:%d",
> + info.rank, info.bank,
> + info.row, info.col);
> +
> + spin_lock(&edac->ecc_lock);
> + edac_mc_handle_error((is_ce ? HW_EVENT_ERR_CORRECTED :
> + HW_EVENT_ERR_UNCORRECTED),
> + mci, cnt, 0, 0, 0, info.rank, -1, -1,
> + message, "");
> + spin_unlock(&edac->ecc_lock);
> + }
> +}
> +
> +static irqreturn_t dmc520_edac_dram_ecc_isr(int irq, struct mem_ctl_info *mci, bool is_ce)
> +{
> + u32 i_mask;
> + struct dmc520_edac *edac;
> +
> + edac = mci->pvt_info;
> +
> + i_mask = is_ce ? DRAM_ECC_INT_CE_BIT : DRAM_ECC_INT_UE_BIT;
> +
> + dmc520_handle_dram_ecc_errors(mci, is_ce);
> +
> + dmc520_write_reg(edac, i_mask, REG_OFFSET_INTERRUPT_CLR);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t
> +dmc520_edac_dram_all_isr(int irq, struct mem_ctl_info *mci, u32 interrupt_mask)
> +{
> + struct dmc520_edac *edac;
> + u32 status;
> + irqreturn_t irq_ret = IRQ_NONE;
> +
> + edac = mci->pvt_info;
> +
> + status = dmc520_read_reg(edac, REG_OFFSET_INTERRUPT_STATUS);
> +
> + if ((interrupt_mask & DRAM_ECC_INT_CE_BIT) &&
> + (status & DRAM_ECC_INT_CE_BIT))
> + irq_ret = dmc520_edac_dram_ecc_isr(irq, mci, true);
> +
> + if ((interrupt_mask & DRAM_ECC_INT_UE_BIT) &&
> + (status & DRAM_ECC_INT_UE_BIT))
> + irq_ret = dmc520_edac_dram_ecc_isr(irq, mci, false);
> +
> + /* If in the future there are more supported interrupts in a different
> + * platform, more condition statements can be added here for each
> + * interrupt flag, together with its corresponding isr implementations.
> + */
This comment above is useless.
Also, kernel comments style is:
/*
* A sentence ending with a full-stop.
* Another sentence. ...
* More sentences. ...
*/
> +
> + return irq_ret;
> +}
> +
> +static void dmc520_init_csrow(struct mem_ctl_info *mci)
> +{
> + struct csrow_info *csi;
> + struct dimm_info *dimm;
> + int row, ch;
> + enum dev_type dt;
> + enum mem_type mt;
> + u64 rs;
> + u32 pages_per_rank;
> + struct dmc520_edac *edac = mci->pvt_info;
> +
> + dt = dmc520_get_dtype(edac);
> + mt = dmc520_get_mtype(edac);
> + rs = dmc520_get_rank_size(edac);
> + pages_per_rank = rs >> PAGE_SHIFT;
> +
> + for (row = 0; row < mci->nr_csrows; row++) {
> + csi = mci->csrows[row];
> +
> + for (ch = 0; ch < csi->nr_channels; ch++) {
> + dimm = csi->channels[ch]->dimm;
> + dimm->grain = dmc520_get_memory_width(edac);
> + dimm->dtype = dt;
> + dimm->mtype = mt;
> + dimm->edac_mode = EDAC_FLAG_SECDED;
> + dimm->nr_pages = pages_per_rank / csi->nr_channels;
> + }
> + }
> +}
> +
> +static int dmc520_edac_probe(struct platform_device *pdev)
> +{
> + struct device *dev;
> + struct dmc520_edac *edac;
> + struct mem_ctl_info *mci;
> + struct edac_mc_layer layers[1];
> + int ret, intr_index, nintr, nintr_registered = 0;
> + struct resource *res;
> + void __iomem *reg_base;
> + u32 reg_val;
> +
> + /* Parsing the device node */
> + dev = &pdev->dev;
> +
> + nintr = of_property_count_u32_elems(dev->of_node, "interrupt-config");
> + if (nintr <= 0) {
> + edac_printk(KERN_ERR, EDAC_MOD_NAME,
> + "Invalid device node configuration: at least one interrupt "
> + "line & config is expected.\n");
More from checkpatch:
WARNING: quoted string split across lines
#528: FILE: drivers/edac/dmc520_edac.c:436:
+ "Invalid device node configuration: at least one interrupt "
+ "line & config is expected.\n");
WARNING: quoted string split across lines
#535: FILE: drivers/edac/dmc520_edac.c:443:
+ "Invalid device node configuration: # of interrupt config "
+ "elements (%d) can not exceed %ld.\n",
WARNING: quoted string split across lines
#579: FILE: drivers/edac/dmc520_edac.c:487:
+ "interrupt-config error: "
+ "element %d's interrupt mask %d has overlap.\n",
WARNING: quoted string split across lines
#590: FILE: drivers/edac/dmc520_edac.c:498:
+ "interrupt-config warning: "
+ "interrupt mask (0x%x) is not supported by dmc520 (0x%lx).\n",
> + return -EINVAL;
> + }
> +
> + if (nintr > ARRAY_SIZE(dmc520_isr_array)) {
> + edac_printk(KERN_ERR, EDAC_MOD_NAME,
> + "Invalid device node configuration: # of interrupt config "
> + "elements (%d) can not exceed %ld.\n",
> + nintr, ARRAY_SIZE(dmc520_isr_array));
> + return -EINVAL;
> + }
> +
> + /* Initialize dmc520 edac */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reg_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(reg_base))
> + return PTR_ERR(reg_base);
> +
> + if (!dmc520_is_ecc_enabled(reg_base))
> + return -ENXIO;
> +
> + layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
> + layers[0].size = dmc520_get_rank_count(reg_base);
> + layers[0].is_virt_csrow = true;
> +
> + mci = edac_mc_alloc(dmc520_mc_idx++, ARRAY_SIZE(layers), layers,
> + sizeof(struct dmc520_edac) + sizeof(u32) * nintr);
> + if (!mci) {
> + edac_printk(KERN_ERR, EDAC_MOD_NAME,
> + "Failed to allocate memory for mc instance\n");
> + return -ENOMEM;
> + }
> +
> + edac = mci->pvt_info;
> + edac->reg_base = reg_base;
> + edac->nintr = nintr;
> + edac->interrupt_mask_all = 0;
> + spin_lock_init(&edac->ecc_lock);
Move that checking...
> +
> + ret = of_property_read_u32_array(dev->of_node, "interrupt-config",
> + edac->interrupt_masks, nintr);
> + if (ret) {
> + edac_printk(KERN_ERR, EDAC_MOD_NAME,
> + "Failed to get interrupt-config arrays.\n");
> + goto err_free_mc;
> + }
> +
> + for (intr_index = 0; intr_index < nintr; ++intr_index) {
> + if (edac->interrupt_mask_all & edac->interrupt_masks[intr_index]) {
> + edac_printk(KERN_ERR, EDAC_MC,
> + "interrupt-config error: "
> + "element %d's interrupt mask %d has overlap.\n",
> + intr_index, edac->interrupt_masks[intr_index]);
> + goto err_free_mc;
> + }
> +
> + edac->interrupt_mask_all |= edac->interrupt_masks[intr_index];
> + }
> +
> + if ((edac->interrupt_mask_all | ALL_INT_MASK) != ALL_INT_MASK) {
> + edac_printk(KERN_WARNING, EDAC_MOD_NAME,
> + "interrupt-config warning: "
> + "interrupt mask (0x%x) is not supported by dmc520 (0x%lx).\n",
> + edac->interrupt_mask_all, ALL_INT_MASK);
> + }
> + edac->interrupt_mask_all &= ALL_INT_MASK;
> +
> + platform_set_drvdata(pdev, mci);
... up-to-here before the edac_mc_alloc() call so that you don't have to
needlessly goto err_free_mc and free the memory you just allocated if
any of those properties reading fails. IOW, do all your hw init first
and then call edac_mc_alloc(), when you have everything needed.
> +
> + mci->pdev = dev;
> + mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR4;
> + mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
> + mci->edac_cap = EDAC_FLAG_SECDED;
> + mci->scrub_cap = SCRUB_FLAG_HW_SRC;
> + mci->scrub_mode = dmc520_get_scrub_type(edac);
> + mci->ctl_name = EDAC_CTL_NAME;
> + mci->dev_name = dev_name(mci->pdev);
> + mci->mod_name = EDAC_MOD_NAME;
> + mci->ctl_page_to_phys = NULL;
> +
> + edac_op_state = EDAC_OPSTATE_INT;
> +
> + dmc520_init_csrow(mci);
> +
> + ret = edac_mc_add_mc(mci);
> + if (ret) {
> + edac_printk(KERN_ERR, EDAC_MOD_NAME,
> + "Failed to register with EDAC core\n");
> + goto err_free_mc;
> + }
> +
> + /* Clear interrupts, not affecting other unrelated interrupts */
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_INTERRUPT_CONTROL);
> + dmc520_write_reg(edac, reg_val & (~(edac->interrupt_mask_all)),
> + REG_OFFSET_INTERRUPT_CONTROL);
> + dmc520_write_reg(edac, edac->interrupt_mask_all,
> + REG_OFFSET_INTERRUPT_CLR);
> +
> + for (intr_index = 0; intr_index < nintr; ++intr_index) {
> + int irq_id = platform_get_irq(pdev, intr_index);
> +
> + if (irq_id < 0) {
> + edac_printk(KERN_ERR, EDAC_MC,
> + "Failed to get irq #%d\n", intr_index);
> + ret = -ENODEV;
> + goto err_free_irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq_id,
> + dmc520_isr_array[intr_index], IRQF_SHARED,
> + dev_name(&pdev->dev), mci);
> + if (ret < 0) {
> + edac_printk(KERN_ERR, EDAC_MC,
> + "Failed to request irq %d\n", irq_id);
> + goto err_free_irq;
> + }
> +
> + ++nintr_registered;
> + }
The same with this IRQ requesting and you'll save yourself the err_free_irq too.
> +
> + /* Reset DRAM CE/UE counters */
> + if (edac->interrupt_mask_all & DRAM_ECC_INT_CE_BIT)
> + dmc520_get_dram_ecc_error_count(edac, true);
> +
> + if (edac->interrupt_mask_all & DRAM_ECC_INT_UE_BIT)
> + dmc520_get_dram_ecc_error_count(edac, false);
> +
> + /* Enable interrupts, not affecting other unrelated interrupts */
> + dmc520_write_reg(edac,
> + reg_val | edac->interrupt_mask_all,
> + REG_OFFSET_INTERRUPT_CONTROL);
Align at opening brace:
dmc520_write_reg(edac, reg_val | edac->interrupt_mask_all,
REG_OFFSET_INTERRUPT_CONTROL);
> +
> + return 0;
> +
> +err_free_irq:
> + for (intr_index = 0; intr_index < nintr_registered; ++intr_index) {
> + int irq_id = platform_get_irq(pdev, intr_index);
> +
> + devm_free_irq(&pdev->dev, irq_id, mci);
> + }
> + edac_mc_del_mc(&pdev->dev);
> +err_free_mc:
> + edac_mc_free(mci);
> +
> + return ret;
> +}
> +
> +static int dmc520_edac_remove(struct platform_device *pdev)
> +{
> + struct dmc520_edac *edac;
> + struct mem_ctl_info *mci;
> + u32 reg_val, intr_index;
> +
> + mci = platform_get_drvdata(pdev);
> + edac = mci->pvt_info;
> +
> + /* Disable interrupts */
> + reg_val = dmc520_read_reg(edac, REG_OFFSET_INTERRUPT_CONTROL);
> + dmc520_write_reg(edac, reg_val & (~(edac->interrupt_mask_all)),
> + REG_OFFSET_INTERRUPT_CONTROL);
> +
> + /* free irq's */
> + for (intr_index = 0; intr_index < edac->nintr; ++intr_index) {
> + int irq_id = platform_get_irq(pdev, intr_index);
> +
> + devm_free_irq(&pdev->dev, irq_id, mci);
> + }
> +
> + edac_mc_del_mc(&pdev->dev);
> + edac_mc_free(mci);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dmc520_edac_driver_id[] = {
> + { .compatible = "arm,dmc-520", },
> + { /* end of table */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, dmc520_edac_driver_id);
> +
> +static struct platform_driver dmc520_edac_driver = {
> + .driver = {
> + .name = "dmc520",
> + .of_match_table = dmc520_edac_driver_id,
> + },
> +
> + .probe = dmc520_edac_probe,
> + .remove = dmc520_edac_remove
> +};
> +
> +module_platform_driver(dmc520_edac_driver);
> +
> +MODULE_AUTHOR(
No linebreaks like that - let it stick out over 80 cols.
> + "Rui Zhao <ruizhao@microsoft.com>, Lei Wang <lewan@microsoft.com>");
> +MODULE_DESCRIPTION("DMC-520 ECC driver");
> +MODULE_LICENSE("GPL v2");
> --
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
From: Daniel Baluta @ 2019-08-07 14:39 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Daniel Baluta, Marco Felsch, Shawn Guo, Mark Rutland,
Aisheng Dong, Peng Fan, Anson Huang, Devicetree List, S.j. Wang,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Fabio Estevam,
linux-arm-kernel, sound-open-firmware
In-Reply-To: <CAEnQRZDr+gj_eiESLNbVUVy1rreRE1nnDgtb3g=CjaRF5Aq9Vw@mail.gmail.com>
On Wed, Jul 24, 2019 at 10:04 AM Daniel Baluta <daniel.baluta@gmail.com> wrote:
>
> On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
> >
> >
> > > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
> > > index 61b97fc55bb2..2aa3a1cdf60c 100644
> > > --- a/sound/soc/sof/Kconfig
> > > +++ b/sound/soc/sof/Kconfig
> > > @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
> > > Say Y if you need this option
> > > If unsure select "N".
> > >
> > > +config SND_SOC_SOF_DT
> > > + tristate "SOF DT enumeration support"
> > > + select SND_SOC_SOF
> > > + select SND_SOC_SOF_OPTIONS
> > > + help
> > > + This adds support for Device Tree enumeration. This option is
> > > + required to enable i.MX8 devices.
> > > + Say Y if you need this option. If unsure select "N".
> > > +
> >
> > [snip]
> >
> > > diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> > > index fff64a9970f0..fa35994a79c4 100644
> > > --- a/sound/soc/sof/imx/Kconfig
> > > +++ b/sound/soc/sof/imx/Kconfig
> > > @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
> > >
> > > config SND_SOC_SOF_IMX8
> > > tristate "SOF support for i.MX8"
> > > + select SND_SOC_SOF_DT
> >
> > This looks upside down. You should select SOF_DT first then include the
> > NXP stuff.
> >
> > > help
> > > This adds support for Sound Open Firmware for NXP i.MX8 platforms
> > > Say Y if you have such a device.
> > > diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c
> > > new file mode 100644
> > > index 000000000000..31429bbb5c7e
> > > --- /dev/null
> > > +++ b/sound/soc/sof/sof-dt-dev.c
> > > @@ -0,0 +1,159 @@
> > > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > > +//
> > > +// Copyright 2019 NXP
> > > +//
> > > +// Author: Daniel Baluta <daniel.baluta@nxp.com>
> > > +//
> > > +
> > > +#include <linux/firmware.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <sound/sof.h>
> > > +
> > > +#include "ops.h"
> > > +
> > > +extern struct snd_sof_dsp_ops sof_imx8_ops;
> > > +
> > > +static char *fw_path;
> > > +module_param(fw_path, charp, 0444);
> > > +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
> > > +
> > > +static char *tplg_path;
> > > +module_param(tplg_path, charp, 0444);
> > > +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
> > > +
> > > +/* platform specific devices */
> > > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
> > > +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
> > > + .default_fw_path = "imx/sof",
> > > + .default_tplg_path = "imx/sof-tplg",
> > > + .nocodec_fw_filename = "sof-imx8.ri",
> > > + .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> > > + .ops = &sof_imx8_ops,
> > > +};
> > > +#endif
> > > +
> > > +static const struct dev_pm_ops sof_dt_pm = {
> > > + SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
> > > + SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
> > > + NULL)
> > > +};
> > > +
> > > +static void sof_dt_probe_complete(struct device *dev)
> > > +{
> > > + /* allow runtime_pm */
> > > + pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
> > > + pm_runtime_use_autosuspend(dev);
> > > + pm_runtime_enable(dev);
> > > +}
> > > +
> > > +static int sof_dt_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + const struct sof_dev_desc *desc;
> > > + /*TODO: create a generic snd_soc_xxx_mach */
> > > + struct snd_soc_acpi_mach *mach;
> >
> > I wonder if you really need to use the same structures. For Intel we get
> > absolutely zero info from the firmware so rely on an ACPI codec ID as a
> > key to find information on which firmware and topology to use, and which
> > machine driver to load. You could have all this information in a DT blob?
>
> Yes. I see your point. I will still need to make a generic structure for
> snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
>
> Maybe something like this:
>
> struct snd_soc_mach {
> union {
> struct snd_soc_acpi_mach acpi_mach;
> struct snd_soc_of_mach of_mach;
> }
> };
>
> and then change the prototype of sof_nocodec_setup.
Hi Pierre,
I fixed all the comments except the one above. Replacing snd_soc_acpi_mach
with a generic snd_soc_mach is not trivial task.
I wonder if it is acceptable to get the initial patches as they are
now and later switch to
generic ACPI/OF abstraction.
Asking this because for the moment on the i.MX side I have only
implemented no codec
version and we don't probe any of the machine drivers we have.
So, there is this only one member of snd_soc_acpi_mach that imx
version is making use of:
mach->drv_name = "sof-nocodec";
That's all.
I think the change as it is now is very clean and non-intrusive. Later
we will get options to
read firmware name and stuff from DT.
Anyhow, I don't think we can get rid of snd_dev_desc structure on
i.MX. This will be used
to store the information read from DT:
static struct sof_dev_desc sof_of_imx8qxp_desc = {
» .default_fw_path = "imx/sof",
» .default_tplg_path = "imx/sof-tplg",
» .nocodec_fw_filename = "sof-imx8.ri",
» .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
» .ops = &sof_imx8_ops,
};
For the moment we will only use the default values.
thanks,
Daniel.
^ permalink raw reply
* Re: [RFCv2 1/9] dt-bindings: mailbox: meson-mhu: convert to yaml
From: Maxime Ripard @ 2019-08-07 14:34 UTC (permalink / raw)
To: Neil Armstrong
Cc: linux-amlogic, robh+dt, jassisinghbrar, linux-arm-kernel,
devicetree
In-Reply-To: <20190805120320.32282-2-narmstrong@baylibre.com>
Hi,
On Mon, Aug 05, 2019 at 02:03:12PM +0200, Neil Armstrong wrote:
> Now that we have the DT validation in place, let's convert the device tree
> bindings for the Amlogic MHU controller over to a YAML schemas.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../mailbox/amlogic,meson-gxbb-mhu.yaml | 53 +++++++++++++++++++
> .../devicetree/bindings/mailbox/meson-mhu.txt | 34 ------------
> 2 files changed, 53 insertions(+), 34 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mailbox/amlogic,meson-gxbb-mhu.yaml
> delete mode 100644 Documentation/devicetree/bindings/mailbox/meson-mhu.txt
>
> diff --git a/Documentation/devicetree/bindings/mailbox/amlogic,meson-gxbb-mhu.yaml b/Documentation/devicetree/bindings/mailbox/amlogic,meson-gxbb-mhu.yaml
> new file mode 100644
> index 000000000000..2536a0082cff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/amlogic,meson-gxbb-mhu.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 BayLibre, SAS
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mailbox/amlogic,meson-gxbb-mhu.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Amlogic Meson Message-Handling-Unit Controller
> +
> +maintainers:
> + - Neil Armstrong <narmstrong@baylibre.com>
> +
> +description: |
> + The Amlogic's Meson SoCs Message-Handling-Unit (MHU) is a mailbox controller
> + that has 3 independent channels/links to communicate with remote processor(s).
> + MHU links are hardwired on a platform. A link raises interrupt for any
> + received data. However, there is no specified way of knowing if the sent
> + data has been read by the remote. This driver assumes the sender polls
> + STAT register and the remote clears it after having read the data.
> +
> +properties:
> + compatible:
> + enum:
> + - amlogic,meson-gxbb-mhu
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 3
> + maxItems: 3
You don't need to specify both here. If one is missing, the tools will
fill it automatically with the other's value.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Georgi Djakov @ 2019-08-07 14:21 UTC (permalink / raw)
To: Artur Świgoń, devicetree, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-pm, dri-devel
Cc: krzk, cw00.choi, myungjoo.ham, inki.dae, sw0312.kim, m.szyprowski,
Bartłomiej Żołnierkiewicz
In-Reply-To: <62557522be4924a01d3822d4734c30f2965c608b.camel@partner.samsung.com>
Hi Artur,
On 8/1/19 10:59, Artur Świgoń wrote:
> Hi Georgi,
>
> On Fri, 2019-07-26 at 11:05 +0300, Georgi Djakov wrote:
>> Hi Artur,
>>
>> On 7/23/19 15:20, Artur Świgoń wrote:
>>> This patch adds interconnect functionality to the exynos-bus devfreq
>>> driver.
>>>
>>> The SoC topology is a graph (or, more specifically, a tree) and most of its
>>> edges are taken from the devfreq parent-child hierarchy (cf.
>>> Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
>>> patch adds missing edges to the DT (under the name 'parent'). Due to
>>> unspecified relative probing order, -EPROBE_DEFER may be propagated to
>>> guarantee that a child is probed before its parent.
>>>
>>> Each bus is now an interconnect provider and an interconnect node as well
>>> (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers
>>> itself as a node. Node IDs are not hardcoded but rather assigned at
>>> runtime, in probing order (subject to the above-mentioned exception
>>> regarding relative order). This approach allows for using this driver with
>>> various Exynos SoCs.
>>
>> I am not familiar with the Exynos bus topology, but it seems to me that it's not
>> represented correctly. An interconnect provider with just a single node (port)
>> is odd. I would expect that each provider consists of multiple master and slave
>> nodes. This data would be used by a framework to understand what are the links
>> and how the traffic flows between the IP blocks and through which buses.
>
> To summarize the exynos-bus topology[1] used by the devfreq driver: There are
> many data buses for data transfer in Samsung Exynos SoC. Every bus has its own
> clock. Buses often share power lines, in which case one of the buses on the
> power line is referred to as 'parent' (or as 'devfreq' in the DT). In the
> particular case of Exynos4412[1][2], the topology can be expressed as follows:
>
> bus_dmc
> -- bus_acp
> -- bus_c2c
>
> bus_leftbus
> -- bus_rightbus
> -- bus_display
> -- bus_fsys
> -- bus_peri
> -- bus_mfc
>
> Where bus_dmc and bus_leftbus probably could be referred to as masters, and the
> following indented nodes as slaves. Patch 08/11 of this RFC additionally adds
> the following to the DT:
>
> bus_dmc
> -- bus_leftbus
>
> Which makes the topology a valid tree.
>
> The exynos-bus concept in devfreq[3] is designed in such a way that every bus is
> probed separately as a platform device, and is a largely independent entity.
>
> This RFC proposes an extension to the existing devfreq driver that basically
> provides a simple QoS to ensure minimum clock frequency for selected buses
> (possibly overriding devfreq governor calculations) using the interconnect
> framework.
>
> The hierarchy is modelled in such a way that every bus is an interconnect node.
> On the other hand, what is considered an interconnect provider here is quite
> arbitrary, but for the reasons mentioned in the above paragraph, this RFC
> assumes that every bus is a provider of itself as a node. Using an alternative
IIUC, in case we want to transfer data between the display and the memory
controller, the path would look like this:
display --> bus_display --> bus_leftbus --> bus_dmc --> memory
But the bus_display for example would have not one, but two nodes (ports),
right? One representing the link to the display controller and another one
representing the link to bus_leftbus? So i think that all the buses should
have at least two nodes, to represent each end of the wire.
> singleton provider approach was deemed more complicated since the 'dev' field in
> 'struct icc_provider' has to be set to something meaningful and we are tied to
> the 'samsung,exynos-bus' compatible string in the driver (and multiple instances
> of exynos-bus probed in indeterminate relative order).
>
Sure, the rest makes sense to me.
Thanks,
Georgi
^ permalink raw reply
* Re: [PATCH v3 07/21] ARM: dts: imx7-colibri: fix 1.8V/UHS support
From: Philippe Schenker @ 2019-08-07 13:41 UTC (permalink / raw)
To: festevam@gmail.com
Cc: s.hauer@pengutronix.de, stefan@agner.ch, Marcel Ziswiler,
kernel@pengutronix.de, Max Krummenacher, mark.rutland@arm.com,
devicetree@vger.kernel.org, michal.vokac@ysoft.com,
shawnguo@kernel.org, Stefan Agner, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, linux-imx@nxp.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAOMZO5CdWmVKXmNSLdbsmnU6_ZKwbeVArtOQzuTg_gtqTUnVag@mail.gmail.com>
On Wed, 2019-08-07 at 08:19 -0300, Fabio Estevam wrote:
> Hi Philippe,
>
> On Wed, Aug 7, 2019 at 5:26 AM Philippe Schenker
> <philippe.schenker@toradex.com> wrote:
> > From: Stefan Agner <stefan.agner@toradex.com>
> >
> > Add pinmuxing and do not specify voltage restrictions for the usdhc
> > instance available on the modules edge connector. This allows to use
> > SD-cards with higher transfer modes if supported by the carrier
> > board.
> >
> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> >
> > ---
> >
> > Changes in v3:
> > - Add new commit message from Stefan's proposal on ML
>
> The commit message has been improved, but there is also another point
> I mentioned earlier:
>
> > Changes in v2: None
> >
> > arch/arm/boot/dts/imx7-colibri.dtsi | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi
> > b/arch/arm/boot/dts/imx7-colibri.dtsi
> > index 16d1a1ed1aff..67f5e0c87fdc 100644
> > --- a/arch/arm/boot/dts/imx7-colibri.dtsi
> > +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
> > @@ -326,7 +326,6 @@
> > &usdhc1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_cd_usdhc1>;
> > - no-1-8-v;
> > cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
> > disable-wp;
> > vqmmc-supply = <®_LDO2>;
> > @@ -671,6 +670,28 @@
> > >;
> > };
> >
> > + pinctrl_usdhc1_100mhz: usdhc1grp_100mhz {
>
> This new entry has been added and it is not referenced.
Sorry, I probably could have mentioned that in this commit. I want, if
possible, to let this commmit as is. That's why I added another on top
of this patchset. Please see patch 21/21 that makes use of this change.
>
> > + fsl,pins = <
> > + MX7D_PAD_SD1_CMD__SD1_CMD 0x5a
> > + MX7D_PAD_SD1_CLK__SD1_CLK 0x1a
> > + MX7D_PAD_SD1_DATA0__SD1_DATA0 0x5a
> > + MX7D_PAD_SD1_DATA1__SD1_DATA1 0x5a
> > + MX7D_PAD_SD1_DATA2__SD1_DATA2 0x5a
> > + MX7D_PAD_SD1_DATA3__SD1_DATA3 0x5a
> > + >;
> > + };
> > +
> > + pinctrl_usdhc1_200mhz: usdhc1grp_200mhz {
>
> Same here.
^ permalink raw reply
* [PATCH v5 4/4] dt-bindings: devfreq: exynos-bus: remove unused property
From: k.konieczny @ 2019-08-07 13:38 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190807133838.14678-1-k.konieczny@partner.samsung.com>
Remove unused DT property "exynos,voltage-tolerance".
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
index f8e946471a58..e71f752cc18f 100644
--- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
+++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
@@ -50,8 +50,6 @@ Required properties only for passive bus device:
Optional properties only for parent bus device:
- exynos,saturation-ratio: the percentage value which is used to calibrate
the performance count against total cycle count.
-- exynos,voltage-tolerance: the percentage value for bus voltage tolerance
- which is used to calculate the max voltage.
Detailed correlation between sub-blocks and power line according to Exynos SoC:
- In case of Exynos3250, there are two power line as following:
--
2.22.0
^ permalink raw reply related
* [PATCH v5 3/4] ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800
From: k.konieczny @ 2019-08-07 13:38 UTC (permalink / raw)
To: k.konieczny
Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190807133838.14678-1-k.konieczny@partner.samsung.com>
From: Marek Szyprowski <m.szyprowski@samsung.com>
Declare Exynos5422/5800 voltage ranges for opp points for big cpu core and
bus wcore and couple their voltage supllies as vdd_arm and vdd_int should
be in 300mV range.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
[k.konieczny: add missing patch description]
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
arch/arm/boot/dts/exynos5420.dtsi | 34 +++++++++----------
arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 +++
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +++
arch/arm/boot/dts/exynos5800.dtsi | 32 ++++++++---------
4 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 5fb2326875dc..0cbf74750553 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -48,62 +48,62 @@
opp-shared;
opp-1800000000 {
opp-hz = /bits/ 64 <1800000000>;
- opp-microvolt = <1250000>;
+ opp-microvolt = <1250000 1250000 1500000>;
clock-latency-ns = <140000>;
};
opp-1700000000 {
opp-hz = /bits/ 64 <1700000000>;
- opp-microvolt = <1212500>;
+ opp-microvolt = <1212500 1212500 1500000>;
clock-latency-ns = <140000>;
};
opp-1600000000 {
opp-hz = /bits/ 64 <1600000000>;
- opp-microvolt = <1175000>;
+ opp-microvolt = <1175000 1175000 1500000>;
clock-latency-ns = <140000>;
};
opp-1500000000 {
opp-hz = /bits/ 64 <1500000000>;
- opp-microvolt = <1137500>;
+ opp-microvolt = <1137500 1137500 1500000>;
clock-latency-ns = <140000>;
};
opp-1400000000 {
opp-hz = /bits/ 64 <1400000000>;
- opp-microvolt = <1112500>;
+ opp-microvolt = <1112500 1112500 1500000>;
clock-latency-ns = <140000>;
};
opp-1300000000 {
opp-hz = /bits/ 64 <1300000000>;
- opp-microvolt = <1062500>;
+ opp-microvolt = <1062500 1062500 1500000>;
clock-latency-ns = <140000>;
};
opp-1200000000 {
opp-hz = /bits/ 64 <1200000000>;
- opp-microvolt = <1037500>;
+ opp-microvolt = <1037500 1037500 1500000>;
clock-latency-ns = <140000>;
};
opp-1100000000 {
opp-hz = /bits/ 64 <1100000000>;
- opp-microvolt = <1012500>;
+ opp-microvolt = <1012500 1012500 1500000>;
clock-latency-ns = <140000>;
};
opp-1000000000 {
opp-hz = /bits/ 64 <1000000000>;
- opp-microvolt = < 987500>;
+ opp-microvolt = < 987500 987500 1500000>;
clock-latency-ns = <140000>;
};
opp-900000000 {
opp-hz = /bits/ 64 <900000000>;
- opp-microvolt = < 962500>;
+ opp-microvolt = < 962500 962500 1500000>;
clock-latency-ns = <140000>;
};
opp-800000000 {
opp-hz = /bits/ 64 <800000000>;
- opp-microvolt = < 937500>;
+ opp-microvolt = < 937500 937500 1500000>;
clock-latency-ns = <140000>;
};
opp-700000000 {
opp-hz = /bits/ 64 <700000000>;
- opp-microvolt = < 912500>;
+ opp-microvolt = < 912500 912500 1500000>;
clock-latency-ns = <140000>;
};
};
@@ -1100,23 +1100,23 @@
opp00 {
opp-hz = /bits/ 64 <84000000>;
- opp-microvolt = <925000>;
+ opp-microvolt = <925000 925000 1400000>;
};
opp01 {
opp-hz = /bits/ 64 <111000000>;
- opp-microvolt = <950000>;
+ opp-microvolt = <950000 950000 1400000>;
};
opp02 {
opp-hz = /bits/ 64 <222000000>;
- opp-microvolt = <950000>;
+ opp-microvolt = <950000 950000 1400000>;
};
opp03 {
opp-hz = /bits/ 64 <333000000>;
- opp-microvolt = <950000>;
+ opp-microvolt = <950000 950000 1400000>;
};
opp04 {
opp-hz = /bits/ 64 <400000000>;
- opp-microvolt = <987500>;
+ opp-microvolt = <987500 987500 1400000>;
};
};
diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
index 25d95de15c9b..65d094256b54 100644
--- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
@@ -428,6 +428,8 @@
regulator-max-microvolt = <1500000>;
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&buck3_reg>;
+ regulator-coupled-max-spread = <300000>;
};
buck3_reg: BUCK3 {
@@ -436,6 +438,8 @@
regulator-max-microvolt = <1400000>;
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&buck2_reg>;
+ regulator-coupled-max-spread = <300000>;
};
buck4_reg: BUCK4 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e0f470fe54c8..5c1e965ed7e9 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -257,6 +257,8 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-coupled-with = <&buck3_reg>;
+ regulator-coupled-max-spread = <300000>;
regulator-state-mem {
regulator-off-in-suspend;
};
@@ -269,6 +271,8 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-coupled-with = <&buck2_reg>;
+ regulator-coupled-max-spread = <300000>;
regulator-state-mem {
regulator-off-in-suspend;
};
diff --git a/arch/arm/boot/dts/exynos5800.dtsi b/arch/arm/boot/dts/exynos5800.dtsi
index 57d3b319fd65..2a74735d161c 100644
--- a/arch/arm/boot/dts/exynos5800.dtsi
+++ b/arch/arm/boot/dts/exynos5800.dtsi
@@ -22,61 +22,61 @@
&cluster_a15_opp_table {
opp-1700000000 {
- opp-microvolt = <1250000>;
+ opp-microvolt = <1250000 1250000 1500000>;
};
opp-1600000000 {
- opp-microvolt = <1250000>;
+ opp-microvolt = <1250000 1250000 1500000>;
};
opp-1500000000 {
- opp-microvolt = <1100000>;
+ opp-microvolt = <1100000 1100000 1500000>;
};
opp-1400000000 {
- opp-microvolt = <1100000>;
+ opp-microvolt = <1100000 1100000 1500000>;
};
opp-1300000000 {
- opp-microvolt = <1100000>;
+ opp-microvolt = <1100000 1100000 1500000>;
};
opp-1200000000 {
- opp-microvolt = <1000000>;
+ opp-microvolt = <1000000 1000000 1500000>;
};
opp-1100000000 {
- opp-microvolt = <1000000>;
+ opp-microvolt = <1000000 1000000 1500000>;
};
opp-1000000000 {
- opp-microvolt = <1000000>;
+ opp-microvolt = <1000000 1000000 1500000>;
};
opp-900000000 {
- opp-microvolt = <1000000>;
+ opp-microvolt = <1000000 1000000 1500000>;
};
opp-800000000 {
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
};
opp-700000000 {
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
};
opp-600000000 {
opp-hz = /bits/ 64 <600000000>;
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
clock-latency-ns = <140000>;
};
opp-500000000 {
opp-hz = /bits/ 64 <500000000>;
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
clock-latency-ns = <140000>;
};
opp-400000000 {
opp-hz = /bits/ 64 <400000000>;
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
clock-latency-ns = <140000>;
};
opp-300000000 {
opp-hz = /bits/ 64 <300000000>;
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
clock-latency-ns = <140000>;
};
opp-200000000 {
opp-hz = /bits/ 64 <200000000>;
- opp-microvolt = <900000>;
+ opp-microvolt = <900000 900000 1500000>;
clock-latency-ns = <140000>;
};
};
--
2.22.0
^ permalink raw reply related
* [PATCH v5 2/4] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: k.konieczny @ 2019-08-07 13:38 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190807133838.14678-1-k.konieczny@partner.samsung.com>
Reuse opp core code for setting bus clock and voltage. As a side
effect this allow usage of coupled regulators feature (required
for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
uses regulator_set_voltage_triplet() for setting regulator voltage
while the old code used regulator_set_voltage_tol() with fixed
tolerance. This patch also removes no longer needed parsing of DT
property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
it). After applying changes both functions exynos_bus_passive_target()
and exynos_bus_target() have the same code, so remove
exynos_bus_passive_target(). In exynos_bus_probe() replace it with
exynos_bus_target.
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
Changes:
v5:
- squashed last patch into this one, as suggested by Chanwoo Choi
v4:
- remove unrelated changes, add newline before comment
---
drivers/devfreq/exynos-bus.c | 130 +++++++----------------------------
1 file changed, 24 insertions(+), 106 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index f34fa26f00d0..2aeb6cc07960 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -25,7 +25,6 @@
#include <linux/slab.h>
#define DEFAULT_SATURATION_RATIO 40
-#define DEFAULT_VOLTAGE_TOLERANCE 2
struct exynos_bus {
struct device *dev;
@@ -37,9 +36,8 @@ struct exynos_bus {
unsigned long curr_freq;
- struct regulator *regulator;
+ struct opp_table *opp_table;
struct clk *clk;
- unsigned int voltage_tolerance;
unsigned int ratio;
};
@@ -93,62 +91,29 @@ static int exynos_bus_get_event(struct exynos_bus *bus,
}
/*
- * Must necessary function for devfreq simple-ondemand governor
+ * devfreq function for both simple-ondemand and passive governor
*/
static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
{
struct exynos_bus *bus = dev_get_drvdata(dev);
struct dev_pm_opp *new_opp;
- unsigned long old_freq, new_freq, new_volt, tol;
int ret = 0;
- /* Get new opp-bus instance according to new bus clock */
+ /* Get correct frequency for bus. */
new_opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(new_opp)) {
dev_err(dev, "failed to get recommended opp instance\n");
return PTR_ERR(new_opp);
}
- new_freq = dev_pm_opp_get_freq(new_opp);
- new_volt = dev_pm_opp_get_voltage(new_opp);
dev_pm_opp_put(new_opp);
- old_freq = bus->curr_freq;
-
- if (old_freq == new_freq)
- return 0;
- tol = new_volt * bus->voltage_tolerance / 100;
-
/* Change voltage and frequency according to new OPP level */
mutex_lock(&bus->lock);
+ ret = dev_pm_opp_set_rate(dev, *freq);
+ if (!ret)
+ bus->curr_freq = *freq;
- if (old_freq < new_freq) {
- ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
- if (ret < 0) {
- dev_err(bus->dev, "failed to set voltage\n");
- goto out;
- }
- }
-
- ret = clk_set_rate(bus->clk, new_freq);
- if (ret < 0) {
- dev_err(dev, "failed to change clock of bus\n");
- clk_set_rate(bus->clk, old_freq);
- goto out;
- }
-
- if (old_freq > new_freq) {
- ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
- if (ret < 0) {
- dev_err(bus->dev, "failed to set voltage\n");
- goto out;
- }
- }
- bus->curr_freq = new_freq;
-
- dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
- old_freq, new_freq, clk_get_rate(bus->clk));
-out:
mutex_unlock(&bus->lock);
return ret;
@@ -196,54 +161,10 @@ static void exynos_bus_exit(struct device *dev)
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
- if (bus->regulator)
- regulator_disable(bus->regulator);
-}
-
-/*
- * Must necessary function for devfreq passive governor
- */
-static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
- u32 flags)
-{
- struct exynos_bus *bus = dev_get_drvdata(dev);
- struct dev_pm_opp *new_opp;
- unsigned long old_freq, new_freq;
- int ret = 0;
-
- /* Get new opp-bus instance according to new bus clock */
- new_opp = devfreq_recommended_opp(dev, freq, flags);
- if (IS_ERR(new_opp)) {
- dev_err(dev, "failed to get recommended opp instance\n");
- return PTR_ERR(new_opp);
- }
-
- new_freq = dev_pm_opp_get_freq(new_opp);
- dev_pm_opp_put(new_opp);
-
- old_freq = bus->curr_freq;
-
- if (old_freq == new_freq)
- return 0;
-
- /* Change the frequency according to new OPP level */
- mutex_lock(&bus->lock);
-
- ret = clk_set_rate(bus->clk, new_freq);
- if (ret < 0) {
- dev_err(dev, "failed to set the clock of bus\n");
- goto out;
+ if (bus->opp_table) {
+ dev_pm_opp_put_regulators(bus->opp_table);
+ bus->opp_table = NULL;
}
-
- *freq = new_freq;
- bus->curr_freq = new_freq;
-
- dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
- old_freq, new_freq, clk_get_rate(bus->clk));
-out:
- mutex_unlock(&bus->lock);
-
- return ret;
}
static void exynos_bus_passive_exit(struct device *dev)
@@ -258,21 +179,19 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
struct exynos_bus *bus)
{
struct device *dev = bus->dev;
+ struct opp_table *opp_table;
+ const char *vdd = "vdd";
int i, ret, count, size;
- /* Get the regulator to provide each bus with the power */
- bus->regulator = devm_regulator_get(dev, "vdd");
- if (IS_ERR(bus->regulator)) {
- dev_err(dev, "failed to get VDD regulator\n");
- return PTR_ERR(bus->regulator);
- }
-
- ret = regulator_enable(bus->regulator);
- if (ret < 0) {
- dev_err(dev, "failed to enable VDD regulator\n");
+ opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ dev_err(dev, "failed to set regulators %d\n", ret);
return ret;
}
+ bus->opp_table = opp_table;
+
/*
* Get the devfreq-event devices to get the current utilization of
* buses. This raw data will be used in devfreq ondemand governor.
@@ -313,14 +232,11 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
bus->ratio = DEFAULT_SATURATION_RATIO;
- if (of_property_read_u32(np, "exynos,voltage-tolerance",
- &bus->voltage_tolerance))
- bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
-
return 0;
err_regulator:
- regulator_disable(bus->regulator);
+ dev_pm_opp_put_regulators(bus->opp_table);
+ bus->opp_table = NULL;
return ret;
}
@@ -471,7 +387,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto out;
passive:
/* Initialize the struct profile and governor data for passive device */
- profile->target = exynos_bus_passive_target;
+ profile->target = exynos_bus_target;
profile->exit = exynos_bus_passive_exit;
/* Get the instance of parent devfreq device */
@@ -511,8 +427,10 @@ static int exynos_bus_probe(struct platform_device *pdev)
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
err_reg:
- if (!passive)
- regulator_disable(bus->regulator);
+ if (!passive) {
+ dev_pm_opp_put_regulators(bus->opp_table);
+ bus->opp_table = NULL;
+ }
return ret;
}
--
2.22.0
^ permalink raw reply related
* [PATCH v5 1/4] devfreq: exynos-bus: correct clock enable sequence
From: k.konieczny @ 2019-08-07 13:38 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190807133838.14678-1-k.konieczny@partner.samsung.com>
Regulators should be enabled before clocks to avoid h/w hang. This
require change in exynos_bus_probe() to move exynos_bus_parse_of()
after exynos_bus_parent_parse_of() and change in error handling.
Similar change is needed in exynos_bus_exit() where clock should be
disabled before regulators.
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes:
v5:
- added Acked-by tag
v4:
- move regulator disable after clock disable
- remove unrelated changes
- add disabling regulators in error path in exynos_bus_probe()
---
drivers/devfreq/exynos-bus.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 486cc5b422f1..f34fa26f00d0 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -194,11 +194,10 @@ static void exynos_bus_exit(struct device *dev)
if (ret < 0)
dev_warn(dev, "failed to disable the devfreq-event devices\n");
- if (bus->regulator)
- regulator_disable(bus->regulator);
-
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
+ if (bus->regulator)
+ regulator_disable(bus->regulator);
}
/*
@@ -386,6 +385,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
struct exynos_bus *bus;
int ret, max_state;
unsigned long min_freq, max_freq;
+ bool passive = false;
if (!np) {
dev_err(dev, "failed to find devicetree node\n");
@@ -399,27 +399,27 @@ static int exynos_bus_probe(struct platform_device *pdev)
bus->dev = &pdev->dev;
platform_set_drvdata(pdev, bus);
- /* Parse the device-tree to get the resource information */
- ret = exynos_bus_parse_of(np, bus);
- if (ret < 0)
- return ret;
-
profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
- if (!profile) {
- ret = -ENOMEM;
- goto err;
- }
+ if (!profile)
+ return -ENOMEM;
node = of_parse_phandle(dev->of_node, "devfreq", 0);
if (node) {
of_node_put(node);
- goto passive;
+ passive = true;
} else {
ret = exynos_bus_parent_parse_of(np, bus);
+ if (ret < 0)
+ return ret;
}
+ /* Parse the device-tree to get the resource information */
+ ret = exynos_bus_parse_of(np, bus);
if (ret < 0)
- goto err;
+ goto err_reg;
+
+ if (passive)
+ goto passive;
/* Initialize the struct profile and governor data for parent device */
profile->polling_ms = 50;
@@ -510,6 +510,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
err:
dev_pm_opp_of_remove_table(dev);
clk_disable_unprepare(bus->clk);
+err_reg:
+ if (!passive)
+ regulator_disable(bus->regulator);
return ret;
}
--
2.22.0
^ permalink raw reply related
* [PATCH v5 0/4] add coupled regulators for Exynos5422/5800
From: k.konieczny @ 2019-08-07 13:38 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <CGME20190807133855eucas1p1cab425b791262e8dee1b17cbe8b1b3da@eucas1p1.samsung.com>
Hi,
The main purpose of this patch series is to add coupled regulators for
Exynos5422/5800 to keep constrain on voltage difference between vdd_arm
and vdd_int to be at most 300mV. In exynos-bus instead of using
regulator_set_voltage_tol() with default voltage tolerance it should be
used regulator_set_voltage_triplet() with volatege range, and this is
already present in opp/core.c code, so it can be reused. While at this,
move setting regulators into opp/core.
This patchset was tested on Odroid XU3.
The DTS coupled regulators patch depends on previous patches.
Changes:
v5:
- squashed last patch "remove exynos_bus_passive_target()" into second
- added Acked-by to patch "correct clock enable sequence"
v4:
- removed "opp: core: add regulators enable and disable" from patchset
as it was applied by Viresh Kumar and changed cover letter
- fix patch "devfreq: exynos-bus: correct clock enable sequence" to
correct order of enable/disable
- removed unrelated changes in "devfreq: exynos-bus: convert to use
dev_pm_opp_set_rate()"
- added new patch "devfreq: exynos-bus: remove exynos_bus_passive_target()"
as suggested by Chanwoo Choi
v3:
- added new exynos-bus patch to correct clock and regulator enabling
and disabling sequence as suggested by Chanwoo Choi
- corrected error path in enable and improved commit message in opp/core
- improve comment in devfreq/exynos-bus.c before devfreq_recommended_opp()
- change cover letter as there is new patch
- added note before Signed-off-by in 4th patch
v2:
- improve regulators enable/disable code in opp/core as suggested by
Viresh Kumar
- add new patch for remove unused dt-bindings as suggested by Krzysztof
Kozlowski
Kamil Konieczny (3):
devfreq: exynos-bus: correct clock enable sequence
devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
dt-bindings: devfreq: exynos-bus: remove unused property
Marek Szyprowski (1):
ARM: dts: exynos: add initial data for coupled regulators for
Exynos5422/5800
.../bindings/devfreq/exynos-bus.txt | 2 -
arch/arm/boot/dts/exynos5420.dtsi | 34 ++--
arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 +
arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +
arch/arm/boot/dts/exynos5800.dtsi | 32 ++--
drivers/devfreq/exynos-bus.c | 153 +++++-------------
6 files changed, 78 insertions(+), 151 deletions(-)
--
2.22.0
^ permalink raw reply
* Re: [PATCH v8 04/14] media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
From: Helen Koike @ 2019-08-07 13:37 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-rockchip, devicetree, eddie.cai.linux, mchehab, heiko,
jacob2.chen, jeffy.chen, zyc, linux-kernel, tfiga, hans.verkuil,
laurent.pinchart, sakari.ailus, kernel, ezequiel, linux-media,
linux-arm-kernel, zhengsq
In-Reply-To: <20190807130558.GF822@valkosipuli.retiisi.org.uk>
Hi Sakari,
thanks for your review,
On 8/7/19 10:05 AM, Sakari Ailus wrote:
> Hi Helen,
>
> Thanks for the patchset.
>
> On Tue, Jul 30, 2019 at 03:42:46PM -0300, Helen Koike wrote:
>> From: Jacob Chen <jacob2.chen@rock-chips.com>
>>
>> This commit adds a subdev driver for Rockchip MIPI Synopsys DPHY driver
>>
>> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> [migrate to phy framework]
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> [update for upstream]
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>
>> ---
>>
>> Changes in v8:
>> - Remove boiler plate license text
>>
>> Changes in v7:
>> - Migrate dphy specific code from
>> drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
>> to drivers/phy/rockchip/phy-rockchip-dphy.c
>> - Drop support for rk3288
>> - Drop support for dphy txrx
>> - code styling and checkpatch fixes
>>
>> drivers/phy/rockchip/Kconfig | 8 +
>> drivers/phy/rockchip/Makefile | 1 +
>> drivers/phy/rockchip/phy-rockchip-dphy.c | 408 +++++++++++++++++++++++
>> 3 files changed, 417 insertions(+)
>> create mode 100644 drivers/phy/rockchip/phy-rockchip-dphy.c
>>
>> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
>> index c454c90cd99e..afd072f135e6 100644
>> --- a/drivers/phy/rockchip/Kconfig
>> +++ b/drivers/phy/rockchip/Kconfig
>> @@ -9,6 +9,14 @@ config PHY_ROCKCHIP_DP
>> help
>> Enable this to support the Rockchip Display Port PHY.
>>
>> +config PHY_ROCKCHIP_DPHY
>> + tristate "Rockchip MIPI Synopsys DPHY driver"
>> + depends on ARCH_ROCKCHIP && OF
>
> How about (...) || COMPILE_TEST ?
>
>> + select GENERIC_PHY_MIPI_DPHY
>> + select GENERIC_PHY
>> + help
>> + Enable this to support the Rockchip MIPI Synopsys DPHY.
>> +
>> config PHY_ROCKCHIP_EMMC
>> tristate "Rockchip EMMC PHY Driver"
>> depends on ARCH_ROCKCHIP && OF
>> diff --git a/drivers/phy/rockchip/Makefile b/drivers/phy/rockchip/Makefile
>> index fd21cbaf40dd..f62e9010bcaf 100644
>> --- a/drivers/phy/rockchip/Makefile
>> +++ b/drivers/phy/rockchip/Makefile
>> @@ -1,5 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>> obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
>> +obj-$(CONFIG_PHY_ROCKCHIP_DPHY) += phy-rockchip-dphy.o
>> obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
>> obj-$(CONFIG_PHY_ROCKCHIP_INNO_HDMI) += phy-rockchip-inno-hdmi.o
>> obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
>> diff --git a/drivers/phy/rockchip/phy-rockchip-dphy.c b/drivers/phy/rockchip/phy-rockchip-dphy.c
>> new file mode 100644
>> index 000000000000..3a29976c2dff
>> --- /dev/null
>> +++ b/drivers/phy/rockchip/phy-rockchip-dphy.c
>> @@ -0,0 +1,408 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Rockchip MIPI Synopsys DPHY driver
>> + *
>> + * Based on:
>> + *
>> + * Copyright (C) 2016 FuZhou Rockchip Co., Ltd.
>> + * Author: Yakir Yang <ykk@@rock-chips.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/phy/phy-mipi-dphy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#define RK3399_GRF_SOC_CON9 0x6224
>> +#define RK3399_GRF_SOC_CON21 0x6254
>> +#define RK3399_GRF_SOC_CON22 0x6258
>> +#define RK3399_GRF_SOC_CON23 0x625c
>> +#define RK3399_GRF_SOC_CON24 0x6260
>> +#define RK3399_GRF_SOC_CON25 0x6264
>> +#define RK3399_GRF_SOC_STATUS1 0xe2a4
>> +
>> +#define CLOCK_LANE_HS_RX_CONTROL 0x34
>> +#define LANE0_HS_RX_CONTROL 0x44
>> +#define LANE1_HS_RX_CONTROL 0x54
>> +#define LANE2_HS_RX_CONTROL 0x84
>> +#define LANE3_HS_RX_CONTROL 0x94
>> +#define HS_RX_DATA_LANES_THS_SETTLE_CONTROL 0x75
>> +
>> +#define MAX_DPHY_CLK 8
>> +
>> +#define PHY_TESTEN_ADDR (0x1 << 16)
>> +#define PHY_TESTEN_DATA (0x0 << 16)
>> +#define PHY_TESTCLK (0x1 << 1)
>> +#define PHY_TESTCLR (0x1 << 0)
>> +#define THS_SETTLE_COUNTER_THRESHOLD 0x04
>> +
>> +#define HIWORD_UPDATE(val, mask, shift) \
>> + ((val) << (shift) | (mask) << ((shift) + 16))
>> +
>> +#define GRF_SOC_CON12 0x0274
>> +
>> +#define GRF_EDP_REF_CLK_SEL_INTER_HIWORD_MASK BIT(20)
>> +#define GRF_EDP_REF_CLK_SEL_INTER BIT(4)
>> +
>> +#define GRF_EDP_PHY_SIDDQ_HIWORD_MASK BIT(21)
>> +#define GRF_EDP_PHY_SIDDQ_ON 0
>> +#define GRF_EDP_PHY_SIDDQ_OFF BIT(5)
>> +
>> +struct hsfreq_range {
>> + u32 range_h;
>> + u8 cfg_bit;
>> +};
>> +
>> +static const struct hsfreq_range rk3399_mipidphy_hsfreq_ranges[] = {
>> + { 89, 0x00}, { 99, 0x10}, { 109, 0x20}, { 129, 0x01},
>> + { 139, 0x11}, { 149, 0x21}, { 169, 0x02}, { 179, 0x12},
>> + { 199, 0x22}, { 219, 0x03}, { 239, 0x13}, { 249, 0x23},
>> + { 269, 0x04}, { 299, 0x14}, { 329, 0x05}, { 359, 0x15},
>> + { 399, 0x25}, { 449, 0x06}, { 499, 0x16}, { 549, 0x07},
>> + { 599, 0x17}, { 649, 0x08}, { 699, 0x18}, { 749, 0x09},
>> + { 799, 0x19}, { 849, 0x29}, { 899, 0x39}, { 949, 0x0a},
>> + { 999, 0x1a}, {1049, 0x2a}, {1099, 0x3a}, {1149, 0x0b},
>> + {1199, 0x1b}, {1249, 0x2b}, {1299, 0x3b}, {1349, 0x0c},
>> + {1399, 0x1c}, {1449, 0x2c}, {1500, 0x3c}
>> +};
>> +
>> +static const char * const rk3399_mipidphy_clks[] = {
>> + "dphy-ref",
>> + "dphy-cfg",
>> + "grf",
>> +};
>> +
>> +enum dphy_reg_id {
>> + GRF_DPHY_RX0_TURNDISABLE = 0,
>> + GRF_DPHY_RX0_FORCERXMODE,
>> + GRF_DPHY_RX0_FORCETXSTOPMODE,
>> + GRF_DPHY_RX0_ENABLE,
>> + GRF_DPHY_RX0_TESTCLR,
>> + GRF_DPHY_RX0_TESTCLK,
>> + GRF_DPHY_RX0_TESTEN,
>> + GRF_DPHY_RX0_TESTDIN,
>> + GRF_DPHY_RX0_TURNREQUEST,
>> + GRF_DPHY_RX0_TESTDOUT,
>> + GRF_DPHY_TX0_TURNDISABLE,
>> + GRF_DPHY_TX0_FORCERXMODE,
>> + GRF_DPHY_TX0_FORCETXSTOPMODE,
>> + GRF_DPHY_TX0_TURNREQUEST,
>> + GRF_DPHY_TX1RX1_TURNDISABLE,
>> + GRF_DPHY_TX1RX1_FORCERXMODE,
>> + GRF_DPHY_TX1RX1_FORCETXSTOPMODE,
>> + GRF_DPHY_TX1RX1_ENABLE,
>> + GRF_DPHY_TX1RX1_MASTERSLAVEZ,
>> + GRF_DPHY_TX1RX1_BASEDIR,
>> + GRF_DPHY_TX1RX1_ENABLECLK,
>> + GRF_DPHY_TX1RX1_TURNREQUEST,
>> + GRF_DPHY_RX1_SRC_SEL,
>> + /* rk3288 only */
>> + GRF_CON_DISABLE_ISP,
>> + GRF_CON_ISP_DPHY_SEL,
>> + GRF_DSI_CSI_TESTBUS_SEL,
>> + GRF_DVP_V18SEL,
>> + /* below is for rk3399 only */
>> + GRF_DPHY_RX0_CLK_INV_SEL,
>> + GRF_DPHY_RX1_CLK_INV_SEL,
>> +};
>> +
>> +struct dphy_reg {
>> + u32 offset;
>> + u32 mask;
>> + u32 shift;
>> +};
>> +
>> +#define PHY_REG(_offset, _width, _shift) \
>> + { .offset = _offset, .mask = BIT(_width) - 1, .shift = _shift, }
>> +
>> +static const struct dphy_reg rk3399_grf_dphy_regs[] = {
>> + [GRF_DPHY_RX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON9, 4, 0),
>> + [GRF_DPHY_RX0_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 10),
>> + [GRF_DPHY_RX1_CLK_INV_SEL] = PHY_REG(RK3399_GRF_SOC_CON9, 1, 11),
>> + [GRF_DPHY_RX0_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 0),
>> + [GRF_DPHY_RX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 4),
>> + [GRF_DPHY_RX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 8),
>> + [GRF_DPHY_RX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON21, 4, 12),
>> + [GRF_DPHY_TX0_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 0),
>> + [GRF_DPHY_TX0_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 4),
>> + [GRF_DPHY_TX0_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 8),
>> + [GRF_DPHY_TX0_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON22, 4, 12),
>> + [GRF_DPHY_TX1RX1_ENABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 0),
>> + [GRF_DPHY_TX1RX1_FORCERXMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 4),
>> + [GRF_DPHY_TX1RX1_FORCETXSTOPMODE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 8),
>> + [GRF_DPHY_TX1RX1_TURNDISABLE] = PHY_REG(RK3399_GRF_SOC_CON23, 4, 12),
>> + [GRF_DPHY_TX1RX1_TURNREQUEST] = PHY_REG(RK3399_GRF_SOC_CON24, 4, 0),
>> + [GRF_DPHY_RX1_SRC_SEL] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 4),
>> + [GRF_DPHY_TX1RX1_BASEDIR] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 5),
>> + [GRF_DPHY_TX1RX1_ENABLECLK] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 6),
>> + [GRF_DPHY_TX1RX1_MASTERSLAVEZ] = PHY_REG(RK3399_GRF_SOC_CON24, 1, 7),
>> + [GRF_DPHY_RX0_TESTDIN] = PHY_REG(RK3399_GRF_SOC_CON25, 8, 0),
>> + [GRF_DPHY_RX0_TESTEN] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 8),
>> + [GRF_DPHY_RX0_TESTCLK] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 9),
>> + [GRF_DPHY_RX0_TESTCLR] = PHY_REG(RK3399_GRF_SOC_CON25, 1, 10),
>> + [GRF_DPHY_RX0_TESTDOUT] = PHY_REG(RK3399_GRF_SOC_STATUS1, 8, 0),
>> +};
>> +
>> +struct dphy_drv_data {
>> + const char * const *clks;
>> + int num_clks;
>> + const struct hsfreq_range *hsfreq_ranges;
>> + int num_hsfreq_ranges;
>> + const struct dphy_reg *regs;
>> +};
>> +
>> +struct rockchip_dphy {
>> + struct device *dev;
>> + struct regmap *grf;
>> + const struct dphy_reg *grf_regs;
>> + struct clk_bulk_data clks[MAX_DPHY_CLK];
>> +
>> + const struct dphy_drv_data *drv_data;
>> + struct phy_configure_opts_mipi_dphy config;
>> +};
>> +
>> +static inline void write_grf_reg(struct rockchip_dphy *priv,
>> + int index, u8 value)
>> +{
>> + const struct dphy_reg *reg = &priv->grf_regs[index];
>> + unsigned int val = HIWORD_UPDATE(value, reg->mask, reg->shift);
>> +
>> + WARN_ON(!reg->offset);
>> + regmap_write(priv->grf, reg->offset, val);
>> +}
>> +
>> +static void mipidphy0_wr_reg(struct rockchip_dphy *priv,
>> + u8 test_code, u8 test_data)
>> +{
>> + /*
>> + * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
>> + * is latched internally as the current test code. Test data is
>> + * programmed internally by rising edge on TESTCLK.
>> + */
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_code);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 1);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 0);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTEN, 0);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTDIN, test_data);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
>> +}
>> +
>> +/* should be move to power_on */
>> +static int mipidphy_rx_stream_on(struct rockchip_dphy *priv)
>> +{
>> + const struct dphy_drv_data *drv_data = priv->drv_data;
>> + const struct hsfreq_range *hsfreq_ranges = drv_data->hsfreq_ranges;
>> + struct phy_configure_opts_mipi_dphy *config = &priv->config;
>> + unsigned int i, hsfreq = 0, data_rate_mbps = config->hs_clk_rate;
>> + int num_hsfreq_ranges = drv_data->num_hsfreq_ranges;
>> +
>> + do_div(data_rate_mbps, 1000 * 1000);
>> +
>> + dev_dbg(priv->dev, "%s: lanes %d - data_rate_mbps %u\n",
>> + __func__, config->lanes, data_rate_mbps);
>> +
>> + for (i = 0; i < num_hsfreq_ranges; i++) {
>> + if (hsfreq_ranges[i].range_h >= data_rate_mbps) {
>> + hsfreq = hsfreq_ranges[i].cfg_bit;
>> + break;
>> + }
>> + }
>> +
>> + write_grf_reg(priv, GRF_DPHY_RX0_FORCERXMODE, 0);
>> + write_grf_reg(priv, GRF_DPHY_RX0_FORCETXSTOPMODE, 0);
>> +
>> + /* Disable lan turn around, which is ignored in receive mode */
>> + write_grf_reg(priv, GRF_DPHY_RX0_TURNREQUEST, 0);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TURNDISABLE, 0xf);
>> +
>> + write_grf_reg(priv, GRF_DPHY_RX0_ENABLE, GENMASK(config->lanes - 1, 0));
>> +
>> + /* dphy start */
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLK, 1);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 1);
>> + usleep_range(100, 150);
>> + write_grf_reg(priv, GRF_DPHY_RX0_TESTCLR, 0);
>> + usleep_range(100, 150);
>> +
>> + /* set clock lane */
>> + /* HS hsfreq_range & lane 0 settle bypass */
>> + mipidphy0_wr_reg(priv, CLOCK_LANE_HS_RX_CONTROL, 0);
>> + /* HS RX Control of lane0 */
>> + mipidphy0_wr_reg(priv, LANE0_HS_RX_CONTROL, hsfreq << 1);
>> + /* HS RX Control of lane1 */
>> + mipidphy0_wr_reg(priv, LANE1_HS_RX_CONTROL, 0);
>> + /* HS RX Control of lane2 */
>> + mipidphy0_wr_reg(priv, LANE2_HS_RX_CONTROL, 0);
>> + /* HS RX Control of lane3 */
>> + mipidphy0_wr_reg(priv, LANE3_HS_RX_CONTROL, 0);
>> + /* HS RX Data Lanes Settle State Time Control */
>> + mipidphy0_wr_reg(priv, HS_RX_DATA_LANES_THS_SETTLE_CONTROL,
>> + THS_SETTLE_COUNTER_THRESHOLD);
>> +
>> + /* Normal operation */
>> + mipidphy0_wr_reg(priv, 0x0, 0);
>> +
>> + return 0;
>> +}
>> +
>> +static int rockchip_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>> +{
>> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
>> + int ret;
>> +
>> + /* pass with phy_mipi_dphy_get_default_config (with pixel rate?) */
>> + ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
>> + if (ret)
>> + return ret;
>> +
>> + memcpy(&priv->config, opts, sizeof(priv->config));
>
> You could to:
>
> priv->config = *opts;
>
> Up to you. Some people like memcpy(). :-)
your way is better thanks!
>
>> +
>> + return 0;
>> +}
>> +
>> +static int rockchip_dphy_power_on(struct phy *phy)
>> +{
>> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
>> + int ret;
>> +
>> + ret = clk_bulk_enable(priv->drv_data->num_clks, priv->clks);
>> + if (ret)
>> + return ret;
>> +
>> + return mipidphy_rx_stream_on(priv);
>> +}
>> +
>> +static int rockchip_dphy_power_off(struct phy *phy)
>> +{
>> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
>> +
>> + clk_bulk_disable(priv->drv_data->num_clks, priv->clks);
>> + return 0;
>> +}
>> +
>> +static int rockchip_dphy_init(struct phy *phy)
>> +{
>> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
>> + int ret;
>> +
>> + ret = clk_bulk_prepare(priv->drv_data->num_clks, priv->clks);
>
> return ...;
>
>> + if (ret)
>> + return ret;
>> + return 0;
>> +}
>> +
>> +static int rockchip_dphy_exit(struct phy *phy)
>> +{
>> + struct rockchip_dphy *priv = phy_get_drvdata(phy);
>> +
>> + clk_bulk_unprepare(priv->drv_data->num_clks, priv->clks);
>> + return 0;
>> +}
>> +
>> +static const struct phy_ops rockchip_dphy_ops = {
>> + .power_on = rockchip_dphy_power_on,
>> + .power_off = rockchip_dphy_power_off,
>> + .init = rockchip_dphy_init,
>> + .exit = rockchip_dphy_exit,
>> + .configure = rockchip_dphy_configure,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static const struct dphy_drv_data rk3399_mipidphy_drv_data = {
>> + .clks = rk3399_mipidphy_clks,
>> + .num_clks = ARRAY_SIZE(rk3399_mipidphy_clks),
>> + .hsfreq_ranges = rk3399_mipidphy_hsfreq_ranges,
>> + .num_hsfreq_ranges = ARRAY_SIZE(rk3399_mipidphy_hsfreq_ranges),
>> + .regs = rk3399_grf_dphy_regs,
>
> Do you expect to support more of the similar PHY(s) --- are there such? If
> not, you could put these in the code that uses them.
Yes, for rk3288 in the future.
Regards,
Helen
>
>> +};
>> +
>> +static const struct of_device_id rockchip_dphy_dt_ids[] = {
>> + {
>> + .compatible = "rockchip,rk3399-mipi-dphy",
>> + .data = &rk3399_mipidphy_drv_data,
>> + },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, rockchip_dphy_dt_ids);
>> +
>> +static int rockchip_dphy_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + const struct dphy_drv_data *drv_data;
>> + struct phy_provider *phy_provider;
>> + const struct of_device_id *of_id;
>> + struct rockchip_dphy *priv;
>> + struct regmap *grf;
>> + struct phy *phy;
>> + unsigned int i;
>> + int ret;
>> +
>> + if (!dev->parent || !dev->parent->of_node)
>> + return -ENODEV;
>> +
>> + if (platform_get_resource(pdev, IORESOURCE_MEM, 0)) {
>> + dev_err(&pdev->dev, "Rockchip DPHY driver only suports rx\n");
>> + return -EINVAL;
>> + }
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> + priv->dev = dev;
>> +
>> + grf = syscon_node_to_regmap(dev->parent->of_node);
>> + if (IS_ERR(grf)) {
>> + grf = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "rockchip,grf");
>> + if (IS_ERR(grf)) {
>> + dev_err(dev, "Can't find GRF syscon\n");
>> + return -ENODEV;
>> + }
>> + }
>> + priv->grf = grf;
>> +
>> + of_id = of_match_device(rockchip_dphy_dt_ids, dev);
>> + if (!of_id)
>> + return -EINVAL;
>> +
>> + drv_data = of_id->data;
>> + priv->grf_regs = drv_data->regs;
>> + priv->drv_data = drv_data;
>> + for (i = 0; i < drv_data->num_clks; i++)
>> + priv->clks[i].id = drv_data->clks[i];
>> + ret = devm_clk_bulk_get(&pdev->dev, drv_data->num_clks, priv->clks);
>> + if (ret)
>> + return ret;
>> +
>> + phy = devm_phy_create(dev, np, &rockchip_dphy_ops);
>> + if (IS_ERR(phy)) {
>> + dev_err(dev, "failed to create phy\n");
>> + return PTR_ERR(phy);
>> + }
>> + phy_set_drvdata(phy, priv);
>> +
>> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> + return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static struct platform_driver rockchip_dphy_driver = {
>> + .probe = rockchip_dphy_probe,
>> + .driver = {
>> + .name = "rockchip-mipi-dphy",
>> + .of_match_table = rockchip_dphy_dt_ids,
>> + },
>> +};
>> +module_platform_driver(rockchip_dphy_driver);
>> +
>> +MODULE_AUTHOR("Ezequiel Garcia <ezequiel@collabora.com>");
>> +MODULE_DESCRIPTION("Rockchip MIPI Synopsys DPHY driver");
>> +MODULE_LICENSE("Dual MIT/GPL");
>
^ permalink raw reply
* [PATCH v2 4/4] dt-bindings: iio: adc: Add AD7606B ADC documentation
From: Beniamin Bia @ 2019-08-07 13:31 UTC (permalink / raw)
To: jic23
Cc: devel, mark.rutland, lars, biabeniamin, Michael.Hennerich,
devicetree, linux-iio, gregkh, linus.walleij, linux-kernel,
nicolas.ferre, robh+dt, pmeerw, knaack.h, mchehab+samsung,
paulmck, Beniamin Bia
In-Reply-To: <20190807133137.11185-1-beniamin.bia@analog.com>
Documentation for AD7606B Analog to Digital Converter and software
mode was added.
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
Changes in v2:
-nothing changed
Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
index 509dbe9c84d2..2afe31747a70 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -13,6 +13,7 @@ maintainers:
description: |
Analog Devices AD7606 Simultaneous Sampling ADC
https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
properties:
@@ -22,6 +23,7 @@ properties:
- adi,ad7606-8
- adi,ad7606-6
- adi,ad7606-4
+ - adi,ad7606b
- adi,ad7616
reg:
@@ -87,7 +89,7 @@ properties:
adi,sw-mode:
description:
- Software mode of operation, so far available only for ad7616.
+ Software mode of operation, so far available only for ad7616 and ad7606B.
It is enabled when all three oversampling mode pins are connected to
high level. The device is configured by the corresponding registers. If the
adi,oversampling-ratio-gpios property is defined, then the driver will set the
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/4] dt-bindings: iio: adc: Migrate AD7606 documentation to yaml
From: Beniamin Bia @ 2019-08-07 13:31 UTC (permalink / raw)
To: jic23
Cc: devel, mark.rutland, lars, biabeniamin, Michael.Hennerich,
devicetree, linux-iio, gregkh, linus.walleij, linux-kernel,
nicolas.ferre, robh+dt, pmeerw, knaack.h, mchehab+samsung,
paulmck, Beniamin Bia
In-Reply-To: <20190807133137.11185-1-beniamin.bia@analog.com>
The documentation for ad7606 was migrated to yaml.
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
Changes in v2:
-old txt file was deleted
.../bindings/iio/adc/adi,ad7606.txt | 66 ---------
.../bindings/iio/adc/adi,ad7606.yaml | 134 ++++++++++++++++++
MAINTAINERS | 2 +-
3 files changed, 135 insertions(+), 67 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
deleted file mode 100644
index d8652460198e..000000000000
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-Analog Devices AD7606 Simultaneous Sampling ADC
-
-Required properties for the AD7606:
-
-- compatible: Must be one of
- * "adi,ad7605-4"
- * "adi,ad7606-8"
- * "adi,ad7606-6"
- * "adi,ad7606-4"
- * "adi,ad7616"
-- reg: SPI chip select number for the device
-- spi-max-frequency: Max SPI frequency to use
- see: Documentation/devicetree/bindings/spi/spi-bus.txt
-- spi-cpha: See Documentation/devicetree/bindings/spi/spi-bus.txt
-- avcc-supply: phandle to the Avcc power supply
-- interrupts: IRQ line for the ADC
- see: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
-- adi,conversion-start-gpios: must be the device tree identifier of the CONVST pin.
- This logic input is used to initiate conversions on the analog
- input channels. As the line is active high, it should be marked
- GPIO_ACTIVE_HIGH.
-
-Optional properties:
-
-- reset-gpios: must be the device tree identifier of the RESET pin. If specified,
- it will be asserted during driver probe. As the line is active high,
- it should be marked GPIO_ACTIVE_HIGH.
-- standby-gpios: must be the device tree identifier of the STBY pin. This pin is used
- to place the AD7606 into one of two power-down modes, Standby mode or
- Shutdown mode. As the line is active low, it should be marked
- GPIO_ACTIVE_LOW.
-- adi,first-data-gpios: must be the device tree identifier of the FRSTDATA pin.
- The FRSTDATA output indicates when the first channel, V1, is
- being read back on either the parallel, byte or serial interface.
- As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
-- adi,range-gpios: must be the device tree identifier of the RANGE pin. The polarity on
- this pin determines the input range of the analog input channels. If
- this pin is tied to a logic high, the analog input range is ±10V for
- all channels. If this pin is tied to a logic low, the analog input range
- is ±5V for all channels. As the line is active high, it should be marked
- GPIO_ACTIVE_HIGH.
-- adi,oversampling-ratio-gpios: must be the device tree identifier of the over-sampling
- mode pins. As the line is active high, it should be marked
- GPIO_ACTIVE_HIGH.
-
-Example:
-
- adc@0 {
- compatible = "adi,ad7606-8";
- reg = <0>;
- spi-max-frequency = <1000000>;
- spi-cpol;
-
- avcc-supply = <&adc_vref>;
-
- interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- interrupt-parent = <&gpio>;
-
- adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
- reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
- adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
- adi,oversampling-ratio-gpios = <&gpio 18 GPIO_ACTIVE_HIGH
- &gpio 23 GPIO_ACTIVE_HIGH
- &gpio 26 GPIO_ACTIVE_HIGH>;
- standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
- };
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
new file mode 100644
index 000000000000..509dbe9c84d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7606 Simultaneous Sampling ADC
+
+maintainers:
+ - Beniamin Bia <beniamin.bia@analog.com>
+ - Stefan Popa <stefan.popa@analog.com>
+
+description: |
+ Analog Devices AD7606 Simultaneous Sampling ADC
+ https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7605-4
+ - adi,ad7606-8
+ - adi,ad7606-6
+ - adi,ad7606-4
+ - adi,ad7616
+
+ reg:
+ maxItems: 1
+
+ spi-cpha: true
+
+ avcc-supply:
+ description:
+ Phandle to the Avcc power supply
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ adi,conversion-start-gpios:
+ description:
+ Must be the device tree identifier of the CONVST pin.
+ This logic input is used to initiate conversions on the analog
+ input channels. As the line is active high, it should be marked
+ GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ Must be the device tree identifier of the RESET pin. If specified,
+ it will be asserted during driver probe. As the line is active high,
+ it should be marked GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ standby-gpios:
+ description:
+ Must be the device tree identifier of the STBY pin. This pin is used
+ to place the AD7606 into one of two power-down modes, Standby mode or
+ Shutdown mode. As the line is active low, it should be marked
+ GPIO_ACTIVE_LOW.
+ maxItems: 1
+
+ adi,first-data-gpios:
+ description:
+ Must be the device tree identifier of the FRSTDATA pin.
+ The FRSTDATA output indicates when the first channel, V1, is
+ being read back on either the parallel, byte or serial interface.
+ As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ adi,range-gpios:
+ description:
+ Must be the device tree identifier of the RANGE pin. The polarity on
+ this pin determines the input range of the analog input channels. If
+ this pin is tied to a logic high, the analog input range is ±10V for
+ all channels. If this pin is tied to a logic low, the analog input range
+ is ±5V for all channels. As the line is active high, it should be marked
+ GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ adi,oversampling-ratio-gpios:
+ description:
+ Must be the device tree identifier of the over-sampling
+ mode pins. As the line is active high, it should be marked
+ GPIO_ACTIVE_HIGH.
+ maxItems: 1
+
+ adi,sw-mode:
+ description:
+ Software mode of operation, so far available only for ad7616.
+ It is enabled when all three oversampling mode pins are connected to
+ high level. The device is configured by the corresponding registers. If the
+ adi,oversampling-ratio-gpios property is defined, then the driver will set the
+ oversampling gpios to high. Otherwise, it is assumed that the pins are hardwired
+ to VDD.
+ maxItems: 1
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - spi-cpha
+ - avcc-supply
+ - interrupts
+ - adi,conversion-start-gpios
+
+examples:
+ - |
+ spi0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad7606-8";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpol;
+
+ avcc-supply = <&adc_vref>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+
+ adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
+ adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
+ adi,oversampling-ratio-gpios = <&gpio 18 GPIO_ACTIVE_HIGH
+ &gpio 23 GPIO_ACTIVE_HIGH
+ &gpio 26 GPIO_ACTIVE_HIGH>;
+ standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
+ adi,sw-mode;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 052d7a8591fb..d2e465772071 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -900,7 +900,7 @@ L: linux-iio@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
F: drivers/iio/adc/ad7606.c
-F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
+F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
ANALOG DEVICES INC AD7768-1 DRIVER
M: Stefan Popa <stefan.popa@analog.com>
--
2.17.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related
* [PATCH v2 2/4] MAINTAINERS: Add Beniamin Bia for AD7606 driver
From: Beniamin Bia @ 2019-08-07 13:31 UTC (permalink / raw)
To: jic23
Cc: devel, mark.rutland, lars, biabeniamin, Michael.Hennerich,
devicetree, linux-iio, gregkh, linus.walleij, linux-kernel,
nicolas.ferre, robh+dt, pmeerw, knaack.h, mchehab+samsung,
paulmck, Beniamin Bia
In-Reply-To: <20190807133137.11185-1-beniamin.bia@analog.com>
Add Beniamin Bia as maintainer for AD7606 driver.
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
Changes in v2:
-nothing changed
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ad498428b38c..052d7a8591fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -895,6 +895,7 @@ F: Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
ANALOG DEVICES INC AD7606 DRIVER
M: Stefan Popa <stefan.popa@analog.com>
+M: Beniamin Bia <beniamin.bia@analog.com>
L: linux-iio@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/4] iio: adc: ad7606: Add support for AD7606B ADC
From: Beniamin Bia @ 2019-08-07 13:31 UTC (permalink / raw)
To: jic23
Cc: devel, mark.rutland, lars, biabeniamin, Stefan Popa,
Michael.Hennerich, devicetree, linux-iio, gregkh, linus.walleij,
linux-kernel, nicolas.ferre, robh+dt, pmeerw, knaack.h,
mchehab+samsung, paulmck, Beniamin Bia
From: Stefan Popa <stefan.popa@analog.com>
The AD7606B is a 16-bit ADC that supports simultaneous sampling of 8
channels. It is pin compatible to AD7606, but adds extra modes by
writing to the register map.
The AD7606B can be configured to work in software mode by setting all
oversampling pins to high. This mode is selected by default.
The oversampling ratio is configured from the OS_MODE register (address
0x08) with the addition of OS=128 and OS=256 that were not available in
hardware mode.
The device is configured to output data on a single spi channel, but this
configuration must be done right after restart. That is why the delay was
removed for devices which doesn't require it.
Moreover, in software mode, the range gpio has no longer its function.
Instead, the scale can be configured individually for each channel from
the RANGE_CH registers (address 0x03 to 0x06). Besides the already
supported ±10 V and ±5 V ranges, software mode can also accommodate the
±2.5 V range.
Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Co-developed-by: Beniamin Bia <beniamin.bia@analog.com>
Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
---
Changes in v2:
-nothing changed
drivers/iio/adc/ad7606.c | 13 ++++-
drivers/iio/adc/ad7606.h | 4 ++
drivers/iio/adc/ad7606_spi.c | 107 +++++++++++++++++++++++++++++++++++
3 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index ed2d08437e5d..f5ba94c03a8d 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -410,12 +410,19 @@ static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
.oversampling_avail = ad7606_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
},
+ [ID_AD7606B] = {
+ .channels = ad7606_channels,
+ .num_channels = 9,
+ .oversampling_avail = ad7606_oversampling_avail,
+ .oversampling_num = ARRAY_SIZE(ad7606_oversampling_avail),
+ },
[ID_AD7616] = {
.channels = ad7616_channels,
.num_channels = 17,
.oversampling_avail = ad7616_oversampling_avail,
.oversampling_num = ARRAY_SIZE(ad7616_oversampling_avail),
.os_req_reset = true,
+ .init_delay_ms = 15,
},
};
@@ -631,8 +638,10 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
/* AD7616 requires al least 15ms to reconfigure after a reset */
- if (msleep_interruptible(15))
- return -ERESTARTSYS;
+ if (st->chip_info->init_delay_ms) {
+ if (msleep_interruptible(st->chip_info->init_delay_ms))
+ return -ERESTARTSYS;
+ }
st->write_scale = ad7606_write_scale_hw;
st->write_os = ad7606_write_os_hw;
diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index eeaaa8b905db..9350ef1f63b5 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -46,6 +46,8 @@
* oversampling ratios.
* @oversampling_num number of elements stored in oversampling_avail array
* @os_req_reset some devices require a reset to update oversampling
+ * @init_delay_ms required delay in miliseconds for initialization
+ * after a restart
*/
struct ad7606_chip_info {
const struct iio_chan_spec *channels;
@@ -53,6 +55,7 @@ struct ad7606_chip_info {
const unsigned int *oversampling_avail;
unsigned int oversampling_num;
bool os_req_reset;
+ unsigned long init_delay_ms;
};
/**
@@ -155,6 +158,7 @@ enum ad7606_supported_device_ids {
ID_AD7606_8,
ID_AD7606_6,
ID_AD7606_4,
+ ID_AD7606B,
ID_AD7616,
};
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index 98ed52b74507..070ee7e31e2c 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -31,6 +31,20 @@
/* The range of the channel is stored on 2 bits*/
#define AD7616_RANGE_CH_MSK(ch) (0b11 << (((ch) & 0b11) * 2))
#define AD7616_RANGE_CH_MODE(ch, mode) ((mode) << ((((ch) & 0b11)) * 2))
+
+#define AD7606_CONFIGURATION_REGISTER 0x02
+#define AD7606_SINGLE_DOUT 0x0
+
+/*
+ * Range for AD7606B channels are stored in registers starting with address 0x3.
+ * Each register stores range for 2 channels(4 bits per channel).
+ */
+#define AD7606_RANGE_CH_MSK(ch) (GENMASK(3, 0) << (4 * ((ch) & 0x1)))
+#define AD7606_RANGE_CH_MODE(ch, mode) \
+ ((GENMASK(3, 0) & mode) << (4 * ((ch) & 0x1)))
+#define AD7606_RANGE_CH_ADDR(ch) (0x03 + ((ch) >> 1))
+#define AD7606_OS_MODE 0x08
+
static const struct iio_chan_spec ad7616_sw_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(16),
AD7616_CHANNEL(0),
@@ -51,6 +65,22 @@ static const struct iio_chan_spec ad7616_sw_channels[] = {
AD7616_CHANNEL(15),
};
+static const struct iio_chan_spec ad7606B_sw_channels[] = {
+ IIO_CHAN_SOFT_TIMESTAMP(8),
+ AD7616_CHANNEL(0),
+ AD7616_CHANNEL(1),
+ AD7616_CHANNEL(2),
+ AD7616_CHANNEL(3),
+ AD7616_CHANNEL(4),
+ AD7616_CHANNEL(5),
+ AD7616_CHANNEL(6),
+ AD7616_CHANNEL(7),
+};
+
+static const unsigned int ad7606B_oversampling_avail[9] = {
+ 1, 2, 4, 8, 16, 32, 64, 128, 256
+};
+
static u16 ad7616_spi_rd_wr_cmd(int addr, char isWriteOp)
{
/*
@@ -60,6 +90,16 @@ static u16 ad7616_spi_rd_wr_cmd(int addr, char isWriteOp)
return ((addr & 0x7F) << 1) | ((isWriteOp & 0x1) << 7);
}
+static u16 ad7606B_spi_rd_wr_cmd(int addr, char isWriteOp)
+{
+ /*
+ * The address of register consists of one bit which
+ * specifies a read command placed bit 6, followed by
+ * 6 bits of address.
+ */
+ return (addr & 0x3F) | (((~isWriteOp) & 0x1) << 6);
+}
+
static int ad7606_spi_read_block(struct device *dev,
int count, void *buf)
{
@@ -169,6 +209,23 @@ static int ad7616_write_os_sw(struct iio_dev *indio_dev, int val)
AD7616_OS_MASK, val << 2);
}
+static int ad7606_write_scale_sw(struct iio_dev *indio_dev, int ch, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return ad7606_spi_write_mask(st,
+ AD7606_RANGE_CH_ADDR(ch),
+ AD7606_RANGE_CH_MSK(ch),
+ AD7606_RANGE_CH_MODE(ch, val));
+}
+
+static int ad7606_write_os_sw(struct iio_dev *indio_dev, int val)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ return ad7606_spi_reg_write(st, AD7606_OS_MODE, val);
+}
+
static int ad7616_sw_mode_config(struct iio_dev *indio_dev)
{
struct ad7606_state *st = iio_priv(indio_dev);
@@ -189,6 +246,42 @@ static int ad7616_sw_mode_config(struct iio_dev *indio_dev)
AD7616_BURST_MODE | AD7616_SEQEN_MODE);
}
+static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+ unsigned long os[3] = {1};
+
+ /*
+ * Software mode is enabled when all three oversampling
+ * pins are set to high. If oversampling gpios are defined
+ * in the device tree, then they need to be set to high,
+ * otherwise, they must be hardwired to VDD
+ */
+ if (st->gpio_os) {
+ gpiod_set_array_value(ARRAY_SIZE(os),
+ st->gpio_os->desc, st->gpio_os->info, os);
+ }
+ /* OS of 128 and 256 are available only in software mode */
+ st->oversampling_avail = ad7606B_oversampling_avail;
+ st->num_os_ratios = ARRAY_SIZE(ad7606B_oversampling_avail);
+
+ st->write_scale = ad7606_write_scale_sw;
+ st->write_os = &ad7606_write_os_sw;
+
+ /* Configure device spi to output on a single channel */
+ st->bops->reg_write(st,
+ AD7606_CONFIGURATION_REGISTER,
+ AD7606_SINGLE_DOUT);
+
+ /*
+ * Scale can be configured individually for each channel
+ * in software mode.
+ */
+ indio_dev->channels = ad7606B_sw_channels;
+
+ return 0;
+}
+
static const struct ad7606_bus_ops ad7606_spi_bops = {
.read_block = ad7606_spi_read_block,
};
@@ -202,6 +295,15 @@ static const struct ad7606_bus_ops ad7616_spi_bops = {
.sw_mode_config = ad7616_sw_mode_config,
};
+static const struct ad7606_bus_ops ad7606B_spi_bops = {
+ .read_block = ad7606_spi_read_block,
+ .reg_read = ad7606_spi_reg_read,
+ .reg_write = ad7606_spi_reg_write,
+ .write_mask = ad7606_spi_write_mask,
+ .rd_wr_cmd = ad7606B_spi_rd_wr_cmd,
+ .sw_mode_config = ad7606B_sw_mode_config,
+};
+
static int ad7606_spi_probe(struct spi_device *spi)
{
const struct spi_device_id *id = spi_get_device_id(spi);
@@ -211,6 +313,9 @@ static int ad7606_spi_probe(struct spi_device *spi)
case ID_AD7616:
bops = &ad7616_spi_bops;
break;
+ case ID_AD7606B:
+ bops = &ad7606B_spi_bops;
+ break;
default:
bops = &ad7606_spi_bops;
break;
@@ -226,6 +331,7 @@ static const struct spi_device_id ad7606_id_table[] = {
{ "ad7606-4", ID_AD7606_4 },
{ "ad7606-6", ID_AD7606_6 },
{ "ad7606-8", ID_AD7606_8 },
+ { "ad7606b", ID_AD7606B },
{ "ad7616", ID_AD7616 },
{}
};
@@ -236,6 +342,7 @@ static const struct of_device_id ad7606_of_match[] = {
{ .compatible = "adi,ad7606-4" },
{ .compatible = "adi,ad7606-6" },
{ .compatible = "adi,ad7606-8" },
+ { .compatible = "adi,ad7606b" },
{ .compatible = "adi,ad7616" },
{ },
};
--
2.17.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related
* Re: [PATCH 5/5] serial: lantiq: Add support for Lightning Mountain SoC
From: Andy Shevchenko @ 2019-08-07 13:31 UTC (permalink / raw)
To: Rahul Tanwar
Cc: linux-serial, devicetree, gregkh, linux-kernel, jslaby, robh+dt,
mark.rutland, qi-ming.wu, cheol.yong.kim, rahul.tanwar
In-Reply-To: <a947355d6cf0ab71205e81779e1549f42f3f945a.1565160764.git.rahul.tanwar@linux.intel.com>
On Wed, Aug 07, 2019 at 05:21:35PM +0800, Rahul Tanwar wrote:
> This patch adds IRQ & ISR support in the driver for Lightning Mountain SoC.
> +#define ASC_IRNCR_MASK 0x7
GENMASK() ?
> +static irqreturn_t lqasc_irq(int irq, void *p)
> +{
> + unsigned long flags;
> + u32 stat;
> + struct uart_port *port = p;
> + struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
> +
> + spin_lock_irqsave(<q_port->lock, flags);
> + stat = readl(port->membase + LTQ_ASC_IRNCR);
> + if (!(stat & ASC_IRNCR_MASK)) {
> + spin_unlock_irqrestore(<q_port->lock, flags);
> + return IRQ_NONE;
> + }
> + spin_unlock_irqrestore(<q_port->lock, flags);
Are you sure the below does not need a serialization?
If it's not the case, you may unlock the lock immediately after readl().
> +
> + if (stat & ASC_IRNCR_TIR)
> + lqasc_tx_int(irq, p);
> +
> + if (stat & ASC_IRNCR_RIR)
> + lqasc_rx_int(irq, p);
> +
> + if (stat & ASC_IRNCR_EIR)
> + lqasc_err_int(irq, p);
> +
> + return IRQ_HANDLED;
> +}
> +static int fetch_irq_intel(struct platform_device *pdev,
> + struct ltq_uart_port *ltq_port)
> +{
> + struct uart_port *port = <q_port->port;
> + int ret;
> +
> + ret = of_irq_get(pdev->dev.of_node, 0);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "failed to fetch IRQ for serial port\n");
> + return -ENODEV;
return ret;
> + }
> + ltq_port->common_irq = ret;
> + port->irq = ret;
> +
> + return ret;
Same as per patch 3, i.e.
return 0;
> +}
> +static int request_irq_intel(struct uart_port *port)
> +{
> + struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
> + int retval;
> +
> + retval = request_irq(ltq_port->common_irq, lqasc_irq, 0,
> + "asc_irq", port);
> + if (retval) {
> + dev_err(port->dev, "failed to request asc_irq\n");
> + return retval;
> + }
> +
> + return 0;
if (retval)
dev_err();
return retval;
> +}
> +
> +
One blank line is enough.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Applied "ASoC: dt-bindings: Introduce compatible strings for 7ULP and 8MQ" to the asoc tree
From: Mark Brown @ 2019-08-07 13:30 UTC (permalink / raw)
To: Daniel Baluta
Cc: alsa-devel, angus, broonie, devicetree, festevam, kernel,
linux-imx, linux-kernel, l.stach
In-Reply-To: <20190806151214.6783-6-daniel.baluta@nxp.com>
The patch
ASoC: dt-bindings: Introduce compatible strings for 7ULP and 8MQ
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 371be51a925a619f1fb149b8d7707e353d9c9f86 Mon Sep 17 00:00:00 2001
From: Daniel Baluta <daniel.baluta@nxp.com>
Date: Tue, 6 Aug 2019 18:12:14 +0300
Subject: [PATCH] ASoC: dt-bindings: Introduce compatible strings for 7ULP and
8MQ
For i.MX7ULP and i.MX8MQ register map is changed. Add two new compatbile
strings to differentiate this.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Link: https://lore.kernel.org/r/20190806151214.6783-6-daniel.baluta@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/sound/fsl-sai.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
index 2e726b983845..e61c0dc1fc0b 100644
--- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
@@ -8,7 +8,8 @@ codec/DSP interfaces.
Required properties:
- compatible : Compatible list, contains "fsl,vf610-sai",
- "fsl,imx6sx-sai" or "fsl,imx6ul-sai"
+ "fsl,imx6sx-sai", "fsl,imx6ul-sai",
+ "fsl,imx7ulp-sai" or "fsl,imx8mq-sai".
- reg : Offset and length of the register set for the device.
--
2.20.1
^ permalink raw reply related
* Applied "ASoC: fsl_sai: Add registers definition for multiple datalines" to the asoc tree
From: Mark Brown @ 2019-08-07 13:30 UTC (permalink / raw)
To: Daniel Baluta
Cc: alsa-devel, angus, broonie, devicetree, festevam, kernel,
linux-imx, linux-kernel, l.stach
In-Reply-To: <20190806151214.6783-2-daniel.baluta@nxp.com>
The patch
ASoC: fsl_sai: Add registers definition for multiple datalines
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 5f0ac20ed6db1d6da2eea8b862cf3d54fdfb5830 Mon Sep 17 00:00:00 2001
From: Daniel Baluta <daniel.baluta@nxp.com>
Date: Tue, 6 Aug 2019 18:12:10 +0300
Subject: [PATCH] ASoC: fsl_sai: Add registers definition for multiple
datalines
SAI IP supports up to 8 data lines. The configuration of
supported number of data lines is decided at SoC integration
time.
This patch adds definitions for all related data TX/RX registers:
* TDR0..7, Transmit data register
* TFR0..7, Transmit FIFO register
* RDR0..7, Receive data register
* RFR0..7, Receive FIFO register
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Link: https://lore.kernel.org/r/20190806151214.6783-2-daniel.baluta@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/fsl/fsl_sai.c | 76 +++++++++++++++++++++++++++++++++++------
sound/soc/fsl/fsl_sai.h | 36 ++++++++++++++++---
2 files changed, 98 insertions(+), 14 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 8f4d9fa95599..e4221f2a5ee3 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -685,7 +685,14 @@ static struct reg_default fsl_sai_reg_defaults[] = {
{FSL_SAI_TCR3, 0},
{FSL_SAI_TCR4, 0},
{FSL_SAI_TCR5, 0},
- {FSL_SAI_TDR, 0},
+ {FSL_SAI_TDR0, 0},
+ {FSL_SAI_TDR1, 0},
+ {FSL_SAI_TDR2, 0},
+ {FSL_SAI_TDR3, 0},
+ {FSL_SAI_TDR4, 0},
+ {FSL_SAI_TDR5, 0},
+ {FSL_SAI_TDR6, 0},
+ {FSL_SAI_TDR7, 0},
{FSL_SAI_TMR, 0},
{FSL_SAI_RCR1, 0},
{FSL_SAI_RCR2, 0},
@@ -704,7 +711,14 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_TCR3:
case FSL_SAI_TCR4:
case FSL_SAI_TCR5:
- case FSL_SAI_TFR:
+ case FSL_SAI_TFR0:
+ case FSL_SAI_TFR1:
+ case FSL_SAI_TFR2:
+ case FSL_SAI_TFR3:
+ case FSL_SAI_TFR4:
+ case FSL_SAI_TFR5:
+ case FSL_SAI_TFR6:
+ case FSL_SAI_TFR7:
case FSL_SAI_TMR:
case FSL_SAI_RCSR:
case FSL_SAI_RCR1:
@@ -712,8 +726,22 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_RCR3:
case FSL_SAI_RCR4:
case FSL_SAI_RCR5:
- case FSL_SAI_RDR:
- case FSL_SAI_RFR:
+ case FSL_SAI_RDR0:
+ case FSL_SAI_RDR1:
+ case FSL_SAI_RDR2:
+ case FSL_SAI_RDR3:
+ case FSL_SAI_RDR4:
+ case FSL_SAI_RDR5:
+ case FSL_SAI_RDR6:
+ case FSL_SAI_RDR7:
+ case FSL_SAI_RFR0:
+ case FSL_SAI_RFR1:
+ case FSL_SAI_RFR2:
+ case FSL_SAI_RFR3:
+ case FSL_SAI_RFR4:
+ case FSL_SAI_RFR5:
+ case FSL_SAI_RFR6:
+ case FSL_SAI_RFR7:
case FSL_SAI_RMR:
return true;
default:
@@ -726,9 +754,30 @@ static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
switch (reg) {
case FSL_SAI_TCSR:
case FSL_SAI_RCSR:
- case FSL_SAI_TFR:
- case FSL_SAI_RFR:
- case FSL_SAI_RDR:
+ case FSL_SAI_TFR0:
+ case FSL_SAI_TFR1:
+ case FSL_SAI_TFR2:
+ case FSL_SAI_TFR3:
+ case FSL_SAI_TFR4:
+ case FSL_SAI_TFR5:
+ case FSL_SAI_TFR6:
+ case FSL_SAI_TFR7:
+ case FSL_SAI_RFR0:
+ case FSL_SAI_RFR1:
+ case FSL_SAI_RFR2:
+ case FSL_SAI_RFR3:
+ case FSL_SAI_RFR4:
+ case FSL_SAI_RFR5:
+ case FSL_SAI_RFR6:
+ case FSL_SAI_RFR7:
+ case FSL_SAI_RDR0:
+ case FSL_SAI_RDR1:
+ case FSL_SAI_RDR2:
+ case FSL_SAI_RDR3:
+ case FSL_SAI_RDR4:
+ case FSL_SAI_RDR5:
+ case FSL_SAI_RDR6:
+ case FSL_SAI_RDR7:
return true;
default:
return false;
@@ -744,7 +793,14 @@ static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_TCR3:
case FSL_SAI_TCR4:
case FSL_SAI_TCR5:
- case FSL_SAI_TDR:
+ case FSL_SAI_TDR0:
+ case FSL_SAI_TDR1:
+ case FSL_SAI_TDR2:
+ case FSL_SAI_TDR3:
+ case FSL_SAI_TDR4:
+ case FSL_SAI_TDR5:
+ case FSL_SAI_TDR6:
+ case FSL_SAI_TDR7:
case FSL_SAI_TMR:
case FSL_SAI_RCSR:
case FSL_SAI_RCR1:
@@ -883,8 +939,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
MCLK_DIR(index));
}
- sai->dma_params_rx.addr = res->start + FSL_SAI_RDR;
- sai->dma_params_tx.addr = res->start + FSL_SAI_TDR;
+ sai->dma_params_rx.addr = res->start + FSL_SAI_RDR0;
+ sai->dma_params_tx.addr = res->start + FSL_SAI_TDR0;
sai->dma_params_rx.maxburst = FSL_SAI_MAXBURST_RX;
sai->dma_params_tx.maxburst = FSL_SAI_MAXBURST_TX;
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 7c1ef671da28..4bb478041d67 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -20,8 +20,22 @@
#define FSL_SAI_TCR3 0x0c /* SAI Transmit Configuration 3 */
#define FSL_SAI_TCR4 0x10 /* SAI Transmit Configuration 4 */
#define FSL_SAI_TCR5 0x14 /* SAI Transmit Configuration 5 */
-#define FSL_SAI_TDR 0x20 /* SAI Transmit Data */
-#define FSL_SAI_TFR 0x40 /* SAI Transmit FIFO */
+#define FSL_SAI_TDR0 0x20 /* SAI Transmit Data 0 */
+#define FSL_SAI_TDR1 0x24 /* SAI Transmit Data 1 */
+#define FSL_SAI_TDR2 0x28 /* SAI Transmit Data 2 */
+#define FSL_SAI_TDR3 0x2C /* SAI Transmit Data 3 */
+#define FSL_SAI_TDR4 0x30 /* SAI Transmit Data 4 */
+#define FSL_SAI_TDR5 0x34 /* SAI Transmit Data 5 */
+#define FSL_SAI_TDR6 0x38 /* SAI Transmit Data 6 */
+#define FSL_SAI_TDR7 0x3C /* SAI Transmit Data 7 */
+#define FSL_SAI_TFR0 0x40 /* SAI Transmit FIFO 0 */
+#define FSL_SAI_TFR1 0x44 /* SAI Transmit FIFO 1 */
+#define FSL_SAI_TFR2 0x48 /* SAI Transmit FIFO 2 */
+#define FSL_SAI_TFR3 0x4C /* SAI Transmit FIFO 3 */
+#define FSL_SAI_TFR4 0x50 /* SAI Transmit FIFO 4 */
+#define FSL_SAI_TFR5 0x54 /* SAI Transmit FIFO 5 */
+#define FSL_SAI_TFR6 0x58 /* SAI Transmit FIFO 6 */
+#define FSL_SAI_TFR7 0x5C /* SAI Transmit FIFO 7 */
#define FSL_SAI_TMR 0x60 /* SAI Transmit Mask */
#define FSL_SAI_RCSR 0x80 /* SAI Receive Control */
#define FSL_SAI_RCR1 0x84 /* SAI Receive Configuration 1 */
@@ -29,8 +43,22 @@
#define FSL_SAI_RCR3 0x8c /* SAI Receive Configuration 3 */
#define FSL_SAI_RCR4 0x90 /* SAI Receive Configuration 4 */
#define FSL_SAI_RCR5 0x94 /* SAI Receive Configuration 5 */
-#define FSL_SAI_RDR 0xa0 /* SAI Receive Data */
-#define FSL_SAI_RFR 0xc0 /* SAI Receive FIFO */
+#define FSL_SAI_RDR0 0xa0 /* SAI Receive Data 0 */
+#define FSL_SAI_RDR1 0xa4 /* SAI Receive Data 1 */
+#define FSL_SAI_RDR2 0xa8 /* SAI Receive Data 2 */
+#define FSL_SAI_RDR3 0xac /* SAI Receive Data 3 */
+#define FSL_SAI_RDR4 0xb0 /* SAI Receive Data 4 */
+#define FSL_SAI_RDR5 0xb4 /* SAI Receive Data 5 */
+#define FSL_SAI_RDR6 0xb8 /* SAI Receive Data 6 */
+#define FSL_SAI_RDR7 0xbc /* SAI Receive Data 7 */
+#define FSL_SAI_RFR0 0xc0 /* SAI Receive FIFO 0 */
+#define FSL_SAI_RFR1 0xc4 /* SAI Receive FIFO 1 */
+#define FSL_SAI_RFR2 0xc8 /* SAI Receive FIFO 2 */
+#define FSL_SAI_RFR3 0xcc /* SAI Receive FIFO 3 */
+#define FSL_SAI_RFR4 0xd0 /* SAI Receive FIFO 4 */
+#define FSL_SAI_RFR5 0xd4 /* SAI Receive FIFO 5 */
+#define FSL_SAI_RFR6 0xd8 /* SAI Receive FIFO 6 */
+#define FSL_SAI_RFR7 0xdc /* SAI Receive FIFO 7 */
#define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */
#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
--
2.20.1
^ permalink raw reply related
* Applied "ASoC: fsl_sai: Add support for SAI new version" to the asoc tree
From: Mark Brown @ 2019-08-07 13:30 UTC (permalink / raw)
To: Daniel Baluta
Cc: devicetree, alsa-devel, timur, robh, Shengjiu Wang, angus,
linux-kernel, tiwai, Nicolin Chen, Mark Brown, linux-imx, kernel,
Mihai Serban, festevam, mihai.serban, l.stach
In-Reply-To: <20190806151214.6783-4-daniel.baluta@nxp.com>
The patch
ASoC: fsl_sai: Add support for SAI new version
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 4f7a0728b5305e2d865f543fbcffd617e03c7674 Mon Sep 17 00:00:00 2001
From: Daniel Baluta <daniel.baluta@nxp.com>
Date: Tue, 6 Aug 2019 18:12:12 +0300
Subject: [PATCH] ASoC: fsl_sai: Add support for SAI new version
New IP version introduces Version ID and Parameter registers
and optionally added Timestamp feature.
VERID and PARAM registers are placed at the top of registers
address space and some registers are shifted according to
the following table:
Tx/Rx data registers and Tx/Rx FIFO registers keep their
addresses, all other registers are shifted by 8.
SAI Memory map is described in chapter 13.10.4.1.1 I2S Memory map
of the Reference Manual [1].
In order to make as less changes as possible we attach an offset
to each register offset to each changed register definition. The
offset is read from each board private data.
[1]https://cache.nxp.com/secured/assets/documents/en/reference-manual/IMX8MDQLQRM.pdf?__gda__=1563728701_38bea7f0f726472cc675cb141b91bec7&fileExt=.pdf
Signed-off-by: Mihai Serban <mihai.serban@nxp.com>
[initial coding in the NXP internal tree]
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
[bugfixing and cleanups]
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
[adapted to linux-next]
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Link: https://lore.kernel.org/r/20190806151214.6783-4-daniel.baluta@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/fsl/fsl_sai.c | 228 ++++++++++++++++++++++++----------------
sound/soc/fsl/fsl_sai.h | 41 ++++----
2 files changed, 156 insertions(+), 113 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index f2698c94c9fe..0c5452927c04 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -40,6 +40,7 @@ static const struct snd_pcm_hw_constraint_list fsl_sai_rate_constraints = {
static irqreturn_t fsl_sai_isr(int irq, void *devid)
{
struct fsl_sai *sai = (struct fsl_sai *)devid;
+ unsigned int ofs = sai->soc_data->reg_offset;
struct device *dev = &sai->pdev->dev;
u32 flags, xcsr, mask;
bool irq_none = true;
@@ -52,7 +53,7 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
mask = (FSL_SAI_FLAGS >> FSL_SAI_CSR_xIE_SHIFT) << FSL_SAI_CSR_xF_SHIFT;
/* Tx IRQ */
- regmap_read(sai->regmap, FSL_SAI_TCSR, &xcsr);
+ regmap_read(sai->regmap, FSL_SAI_TCSR(ofs), &xcsr);
flags = xcsr & mask;
if (flags)
@@ -82,11 +83,11 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
xcsr &= ~FSL_SAI_CSR_xF_MASK;
if (flags)
- regmap_write(sai->regmap, FSL_SAI_TCSR, flags | xcsr);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), flags | xcsr);
irq_rx:
/* Rx IRQ */
- regmap_read(sai->regmap, FSL_SAI_RCSR, &xcsr);
+ regmap_read(sai->regmap, FSL_SAI_RCSR(ofs), &xcsr);
flags = xcsr & mask;
if (flags)
@@ -116,7 +117,7 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
xcsr &= ~FSL_SAI_CSR_xF_MASK;
if (flags)
- regmap_write(sai->regmap, FSL_SAI_RCSR, flags | xcsr);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), flags | xcsr);
out:
if (irq_none)
@@ -140,6 +141,7 @@ static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
int clk_id, unsigned int freq, int fsl_dir)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
u32 val_cr2 = 0;
@@ -160,7 +162,7 @@ static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
return -EINVAL;
}
- regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx, ofs),
FSL_SAI_CR2_MSEL_MASK, val_cr2);
return 0;
@@ -193,6 +195,7 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
unsigned int fmt, int fsl_dir)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
bool tx = fsl_dir == FSL_FMT_TRANSMITTER;
u32 val_cr2 = 0, val_cr4 = 0;
@@ -287,9 +290,9 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
return -EINVAL;
}
- regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx, ofs),
FSL_SAI_CR2_BCP | FSL_SAI_CR2_BCD_MSTR, val_cr2);
- regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
FSL_SAI_CR4_MF | FSL_SAI_CR4_FSE |
FSL_SAI_CR4_FSP | FSL_SAI_CR4_FSD_MSTR, val_cr4);
@@ -316,6 +319,7 @@ static int fsl_sai_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
unsigned long clk_rate;
u32 savediv = 0, ratio, savesub = freq;
u32 id;
@@ -378,17 +382,17 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
*/
if ((sai->synchronous[TX] && !sai->synchronous[RX]) ||
(!tx && !sai->synchronous[RX])) {
- regmap_update_bits(sai->regmap, FSL_SAI_RCR2,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs),
FSL_SAI_CR2_MSEL_MASK,
FSL_SAI_CR2_MSEL(sai->mclk_id[tx]));
- regmap_update_bits(sai->regmap, FSL_SAI_RCR2,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs),
FSL_SAI_CR2_DIV_MASK, savediv - 1);
} else if ((sai->synchronous[RX] && !sai->synchronous[TX]) ||
(tx && !sai->synchronous[TX])) {
- regmap_update_bits(sai->regmap, FSL_SAI_TCR2,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs),
FSL_SAI_CR2_MSEL_MASK,
FSL_SAI_CR2_MSEL(sai->mclk_id[tx]));
- regmap_update_bits(sai->regmap, FSL_SAI_TCR2,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs),
FSL_SAI_CR2_DIV_MASK, savediv - 1);
}
@@ -403,6 +407,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
unsigned int channels = params_channels(params);
u32 word_width = params_width(params);
@@ -455,19 +460,19 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
if (!sai->is_slave_mode) {
if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {
- regmap_update_bits(sai->regmap, FSL_SAI_TCR4,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCR4(ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
val_cr4);
- regmap_update_bits(sai->regmap, FSL_SAI_TCR5,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
regmap_write(sai->regmap, FSL_SAI_TMR,
~0UL - ((1 << channels) - 1));
} else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
- regmap_update_bits(sai->regmap, FSL_SAI_RCR4,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCR4(ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
val_cr4);
- regmap_update_bits(sai->regmap, FSL_SAI_RCR5,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
regmap_write(sai->regmap, FSL_SAI_RMR,
@@ -475,10 +480,10 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
}
}
- regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
val_cr4);
- regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - 1));
@@ -506,6 +511,8 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *cpu_dai)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
+
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
u32 xcsr, count = 100;
@@ -514,9 +521,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
* Rx sync with Tx clocks: Clear SYNC for Tx, set it for Rx.
* Tx sync with Rx clocks: Clear SYNC for Rx, set it for Tx.
*/
- regmap_update_bits(sai->regmap, FSL_SAI_TCR2, FSL_SAI_CR2_SYNC,
- sai->synchronous[TX] ? FSL_SAI_CR2_SYNC : 0);
- regmap_update_bits(sai->regmap, FSL_SAI_RCR2, FSL_SAI_CR2_SYNC,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCR2(ofs), FSL_SAI_CR2_SYNC,
+ sai->synchronous[TX] ? FSL_SAI_CR2_SYNC : 0);
+ regmap_update_bits(sai->regmap, FSL_SAI_RCR2(ofs), FSL_SAI_CR2_SYNC,
sai->synchronous[RX] ? FSL_SAI_CR2_SYNC : 0);
/*
@@ -527,43 +534,44 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
- regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
- regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
break;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
- regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_FRDE, 0);
- regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_xIE_MASK, 0);
/* Check if the opposite FRDE is also disabled */
- regmap_read(sai->regmap, FSL_SAI_xCSR(!tx), &xcsr);
+ regmap_read(sai->regmap, FSL_SAI_xCSR(!tx, ofs), &xcsr);
if (!(xcsr & FSL_SAI_CSR_FRDE)) {
/* Disable both directions and reset their FIFOs */
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
FSL_SAI_CSR_TERE, 0);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
FSL_SAI_CSR_TERE, 0);
/* TERE will remain set till the end of current frame */
do {
udelay(10);
- regmap_read(sai->regmap, FSL_SAI_xCSR(tx), &xcsr);
+ regmap_read(sai->regmap,
+ FSL_SAI_xCSR(tx, ofs), &xcsr);
} while (--count && xcsr & FSL_SAI_CSR_TERE);
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+ regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
/*
@@ -575,13 +583,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
*/
if (!sai->is_slave_mode) {
/* Software Reset for both Tx and Rx */
- regmap_write(sai->regmap,
- FSL_SAI_TCSR, FSL_SAI_CSR_SR);
- regmap_write(sai->regmap,
- FSL_SAI_RCSR, FSL_SAI_CSR_SR);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
+ FSL_SAI_CSR_SR);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs),
+ FSL_SAI_CSR_SR);
/* Clear SR bit to finish the reset */
- regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
- regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
}
}
break;
@@ -596,10 +604,11 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
int ret;
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
FSL_SAI_CR3_TRCE_MASK,
FSL_SAI_CR3_TRCE);
@@ -613,9 +622,10 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+ unsigned int ofs = sai->soc_data->reg_offset;
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
FSL_SAI_CR3_TRCE_MASK, 0);
}
@@ -633,18 +643,20 @@ static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
{
struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
+ unsigned int ofs = sai->soc_data->reg_offset;
/* Software Reset for both Tx and Rx */
- regmap_write(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_SR);
- regmap_write(sai->regmap, FSL_SAI_RCSR, FSL_SAI_CSR_SR);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), FSL_SAI_CSR_SR);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), FSL_SAI_CSR_SR);
/* Clear SR bit to finish the reset */
- regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
- regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
- regmap_update_bits(sai->regmap, FSL_SAI_TCR1, FSL_SAI_CR1_RFW_MASK,
+ regmap_update_bits(sai->regmap, FSL_SAI_TCR1(ofs),
+ FSL_SAI_CR1_RFW_MASK,
sai->soc_data->fifo_depth - FSL_SAI_MAXBURST_TX);
- regmap_update_bits(sai->regmap, FSL_SAI_RCR1, FSL_SAI_CR1_RFW_MASK,
- FSL_SAI_MAXBURST_RX - 1);
+ regmap_update_bits(sai->regmap, FSL_SAI_RCR1(ofs),
+ FSL_SAI_CR1_RFW_MASK, FSL_SAI_MAXBURST_RX - 1);
snd_soc_dai_init_dma_data(cpu_dai, &sai->dma_params_tx,
&sai->dma_params_rx);
@@ -681,12 +693,12 @@ static const struct snd_soc_component_driver fsl_component = {
.name = "fsl-sai",
};
-static struct reg_default fsl_sai_reg_defaults[] = {
- {FSL_SAI_TCR1, 0},
- {FSL_SAI_TCR2, 0},
- {FSL_SAI_TCR3, 0},
- {FSL_SAI_TCR4, 0},
- {FSL_SAI_TCR5, 0},
+static struct reg_default fsl_sai_reg_defaults_ofs0[] = {
+ {FSL_SAI_TCR1(0), 0},
+ {FSL_SAI_TCR2(0), 0},
+ {FSL_SAI_TCR3(0), 0},
+ {FSL_SAI_TCR4(0), 0},
+ {FSL_SAI_TCR5(0), 0},
{FSL_SAI_TDR0, 0},
{FSL_SAI_TDR1, 0},
{FSL_SAI_TDR2, 0},
@@ -695,24 +707,50 @@ static struct reg_default fsl_sai_reg_defaults[] = {
{FSL_SAI_TDR5, 0},
{FSL_SAI_TDR6, 0},
{FSL_SAI_TDR7, 0},
- {FSL_SAI_TMR, 0},
- {FSL_SAI_RCR1, 0},
- {FSL_SAI_RCR2, 0},
- {FSL_SAI_RCR3, 0},
- {FSL_SAI_RCR4, 0},
- {FSL_SAI_RCR5, 0},
- {FSL_SAI_RMR, 0},
+ {FSL_SAI_TMR, 0},
+ {FSL_SAI_RCR1(0), 0},
+ {FSL_SAI_RCR2(0), 0},
+ {FSL_SAI_RCR3(0), 0},
+ {FSL_SAI_RCR4(0), 0},
+ {FSL_SAI_RCR5(0), 0},
+ {FSL_SAI_RMR, 0},
+};
+
+static struct reg_default fsl_sai_reg_defaults_ofs8[] = {
+ {FSL_SAI_TCR1(8), 0},
+ {FSL_SAI_TCR2(8), 0},
+ {FSL_SAI_TCR3(8), 0},
+ {FSL_SAI_TCR4(8), 0},
+ {FSL_SAI_TCR5(8), 0},
+ {FSL_SAI_TDR0, 0},
+ {FSL_SAI_TDR1, 0},
+ {FSL_SAI_TDR2, 0},
+ {FSL_SAI_TDR3, 0},
+ {FSL_SAI_TDR4, 0},
+ {FSL_SAI_TDR5, 0},
+ {FSL_SAI_TDR6, 0},
+ {FSL_SAI_TDR7, 0},
+ {FSL_SAI_TMR, 0},
+ {FSL_SAI_RCR1(8), 0},
+ {FSL_SAI_RCR2(8), 0},
+ {FSL_SAI_RCR3(8), 0},
+ {FSL_SAI_RCR4(8), 0},
+ {FSL_SAI_RCR5(8), 0},
+ {FSL_SAI_RMR, 0},
};
static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
{
+ struct fsl_sai *sai = dev_get_drvdata(dev);
+ unsigned int ofs = sai->soc_data->reg_offset;
+
+ if (reg >= FSL_SAI_TCSR(ofs) && reg <= FSL_SAI_TCR5(ofs))
+ return true;
+
+ if (reg >= FSL_SAI_RCSR(ofs) && reg <= FSL_SAI_RCR5(ofs))
+ return true;
+
switch (reg) {
- case FSL_SAI_TCSR:
- case FSL_SAI_TCR1:
- case FSL_SAI_TCR2:
- case FSL_SAI_TCR3:
- case FSL_SAI_TCR4:
- case FSL_SAI_TCR5:
case FSL_SAI_TFR0:
case FSL_SAI_TFR1:
case FSL_SAI_TFR2:
@@ -722,12 +760,6 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_TFR6:
case FSL_SAI_TFR7:
case FSL_SAI_TMR:
- case FSL_SAI_RCSR:
- case FSL_SAI_RCR1:
- case FSL_SAI_RCR2:
- case FSL_SAI_RCR3:
- case FSL_SAI_RCR4:
- case FSL_SAI_RCR5:
case FSL_SAI_RDR0:
case FSL_SAI_RDR1:
case FSL_SAI_RDR2:
@@ -753,9 +785,13 @@ static bool fsl_sai_readable_reg(struct device *dev, unsigned int reg)
static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
{
+ struct fsl_sai *sai = dev_get_drvdata(dev);
+ unsigned int ofs = sai->soc_data->reg_offset;
+
+ if (reg == FSL_SAI_TCSR(ofs) || reg == FSL_SAI_RCSR(ofs))
+ return true;
+
switch (reg) {
- case FSL_SAI_TCSR:
- case FSL_SAI_RCSR:
case FSL_SAI_TFR0:
case FSL_SAI_TFR1:
case FSL_SAI_TFR2:
@@ -788,13 +824,16 @@ static bool fsl_sai_volatile_reg(struct device *dev, unsigned int reg)
static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
{
+ struct fsl_sai *sai = dev_get_drvdata(dev);
+ unsigned int ofs = sai->soc_data->reg_offset;
+
+ if (reg >= FSL_SAI_TCSR(ofs) && reg <= FSL_SAI_TCR5(ofs))
+ return true;
+
+ if (reg >= FSL_SAI_RCSR(ofs) && reg <= FSL_SAI_RCR5(ofs))
+ return true;
+
switch (reg) {
- case FSL_SAI_TCSR:
- case FSL_SAI_TCR1:
- case FSL_SAI_TCR2:
- case FSL_SAI_TCR3:
- case FSL_SAI_TCR4:
- case FSL_SAI_TCR5:
case FSL_SAI_TDR0:
case FSL_SAI_TDR1:
case FSL_SAI_TDR2:
@@ -804,12 +843,6 @@ static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
case FSL_SAI_TDR6:
case FSL_SAI_TDR7:
case FSL_SAI_TMR:
- case FSL_SAI_RCSR:
- case FSL_SAI_RCR1:
- case FSL_SAI_RCR2:
- case FSL_SAI_RCR3:
- case FSL_SAI_RCR4:
- case FSL_SAI_RCR5:
case FSL_SAI_RMR:
return true;
default:
@@ -817,15 +850,15 @@ static bool fsl_sai_writeable_reg(struct device *dev, unsigned int reg)
}
}
-static const struct regmap_config fsl_sai_regmap_config = {
+static struct regmap_config fsl_sai_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
.val_bits = 32,
.fast_io = true,
.max_register = FSL_SAI_RMR,
- .reg_defaults = fsl_sai_reg_defaults,
- .num_reg_defaults = ARRAY_SIZE(fsl_sai_reg_defaults),
+ .reg_defaults = fsl_sai_reg_defaults_ofs0,
+ .num_reg_defaults = ARRAY_SIZE(fsl_sai_reg_defaults_ofs0),
.readable_reg = fsl_sai_readable_reg,
.volatile_reg = fsl_sai_volatile_reg,
.writeable_reg = fsl_sai_writeable_reg,
@@ -857,6 +890,12 @@ static int fsl_sai_probe(struct platform_device *pdev)
if (IS_ERR(base))
return PTR_ERR(base);
+ if (sai->soc_data->reg_offset == 8) {
+ fsl_sai_regmap_config.reg_defaults = fsl_sai_reg_defaults_ofs8;
+ fsl_sai_regmap_config.num_reg_defaults =
+ ARRAY_SIZE(fsl_sai_reg_defaults_ofs8);
+ }
+
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
"bus", base, &fsl_sai_regmap_config);
@@ -971,11 +1010,13 @@ static int fsl_sai_remove(struct platform_device *pdev)
static const struct fsl_sai_soc_data fsl_sai_vf610_data = {
.use_imx_pcm = false,
.fifo_depth = 32,
+ .reg_offset = 0,
};
static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
.use_imx_pcm = true,
.fifo_depth = 32,
+ .reg_offset = 0,
};
static const struct of_device_id fsl_sai_ids[] = {
@@ -1008,6 +1049,7 @@ static int fsl_sai_runtime_suspend(struct device *dev)
static int fsl_sai_runtime_resume(struct device *dev)
{
struct fsl_sai *sai = dev_get_drvdata(dev);
+ unsigned int ofs = sai->soc_data->reg_offset;
int ret;
ret = clk_prepare_enable(sai->bus_clk);
@@ -1029,11 +1071,11 @@ static int fsl_sai_runtime_resume(struct device *dev)
}
regcache_cache_only(sai->regmap, false);
- regmap_write(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_SR);
- regmap_write(sai->regmap, FSL_SAI_RCSR, FSL_SAI_CSR_SR);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), FSL_SAI_CSR_SR);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), FSL_SAI_CSR_SR);
usleep_range(1000, 2000);
- regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
- regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
+ regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
+ regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
ret = regcache_sync(sai->regmap);
if (ret)
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 20c5b9b1e8bc..b89b0ca26053 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -14,12 +14,12 @@
SNDRV_PCM_FMTBIT_S32_LE)
/* SAI Register Map Register */
-#define FSL_SAI_TCSR 0x00 /* SAI Transmit Control */
-#define FSL_SAI_TCR1 0x04 /* SAI Transmit Configuration 1 */
-#define FSL_SAI_TCR2 0x08 /* SAI Transmit Configuration 2 */
-#define FSL_SAI_TCR3 0x0c /* SAI Transmit Configuration 3 */
-#define FSL_SAI_TCR4 0x10 /* SAI Transmit Configuration 4 */
-#define FSL_SAI_TCR5 0x14 /* SAI Transmit Configuration 5 */
+#define FSL_SAI_TCSR(ofs) (0x00 + ofs) /* SAI Transmit Control */
+#define FSL_SAI_TCR1(ofs) (0x04 + ofs) /* SAI Transmit Configuration 1 */
+#define FSL_SAI_TCR2(ofs) (0x08 + ofs) /* SAI Transmit Configuration 2 */
+#define FSL_SAI_TCR3(ofs) (0x0c + ofs) /* SAI Transmit Configuration 3 */
+#define FSL_SAI_TCR4(ofs) (0x10 + ofs) /* SAI Transmit Configuration 4 */
+#define FSL_SAI_TCR5(ofs) (0x14 + ofs) /* SAI Transmit Configuration 5 */
#define FSL_SAI_TDR0 0x20 /* SAI Transmit Data 0 */
#define FSL_SAI_TDR1 0x24 /* SAI Transmit Data 1 */
#define FSL_SAI_TDR2 0x28 /* SAI Transmit Data 2 */
@@ -37,12 +37,12 @@
#define FSL_SAI_TFR6 0x58 /* SAI Transmit FIFO 6 */
#define FSL_SAI_TFR7 0x5C /* SAI Transmit FIFO 7 */
#define FSL_SAI_TMR 0x60 /* SAI Transmit Mask */
-#define FSL_SAI_RCSR 0x80 /* SAI Receive Control */
-#define FSL_SAI_RCR1 0x84 /* SAI Receive Configuration 1 */
-#define FSL_SAI_RCR2 0x88 /* SAI Receive Configuration 2 */
-#define FSL_SAI_RCR3 0x8c /* SAI Receive Configuration 3 */
-#define FSL_SAI_RCR4 0x90 /* SAI Receive Configuration 4 */
-#define FSL_SAI_RCR5 0x94 /* SAI Receive Configuration 5 */
+#define FSL_SAI_RCSR(ofs) (0x80 + ofs) /* SAI Receive Control */
+#define FSL_SAI_RCR1(ofs) (0x84 + ofs)/* SAI Receive Configuration 1 */
+#define FSL_SAI_RCR2(ofs) (0x88 + ofs) /* SAI Receive Configuration 2 */
+#define FSL_SAI_RCR3(ofs) (0x8c + ofs) /* SAI Receive Configuration 3 */
+#define FSL_SAI_RCR4(ofs) (0x90 + ofs) /* SAI Receive Configuration 4 */
+#define FSL_SAI_RCR5(ofs) (0x94 + ofs) /* SAI Receive Configuration 5 */
#define FSL_SAI_RDR0 0xa0 /* SAI Receive Data 0 */
#define FSL_SAI_RDR1 0xa4 /* SAI Receive Data 1 */
#define FSL_SAI_RDR2 0xa8 /* SAI Receive Data 2 */
@@ -61,14 +61,14 @@
#define FSL_SAI_RFR7 0xdc /* SAI Receive FIFO 7 */
#define FSL_SAI_RMR 0xe0 /* SAI Receive Mask */
-#define FSL_SAI_xCSR(tx) (tx ? FSL_SAI_TCSR : FSL_SAI_RCSR)
-#define FSL_SAI_xCR1(tx) (tx ? FSL_SAI_TCR1 : FSL_SAI_RCR1)
-#define FSL_SAI_xCR2(tx) (tx ? FSL_SAI_TCR2 : FSL_SAI_RCR2)
-#define FSL_SAI_xCR3(tx) (tx ? FSL_SAI_TCR3 : FSL_SAI_RCR3)
-#define FSL_SAI_xCR4(tx) (tx ? FSL_SAI_TCR4 : FSL_SAI_RCR4)
-#define FSL_SAI_xCR5(tx) (tx ? FSL_SAI_TCR5 : FSL_SAI_RCR5)
-#define FSL_SAI_xDR(tx) (tx ? FSL_SAI_TDR : FSL_SAI_RDR)
-#define FSL_SAI_xFR(tx) (tx ? FSL_SAI_TFR : FSL_SAI_RFR)
+#define FSL_SAI_xCSR(tx, ofs) (tx ? FSL_SAI_TCSR(ofs) : FSL_SAI_RCSR(ofs))
+#define FSL_SAI_xCR1(tx, ofs) (tx ? FSL_SAI_TCR1(ofs) : FSL_SAI_RCR1(ofs))
+#define FSL_SAI_xCR2(tx, ofs) (tx ? FSL_SAI_TCR2(ofs) : FSL_SAI_RCR2(ofs))
+#define FSL_SAI_xCR3(tx, ofs) (tx ? FSL_SAI_TCR3(ofs) : FSL_SAI_RCR3(ofs))
+#define FSL_SAI_xCR4(tx, ofs) (tx ? FSL_SAI_TCR4(ofs) : FSL_SAI_RCR4(ofs))
+#define FSL_SAI_xCR5(tx, ofs) (tx ? FSL_SAI_TCR5(ofs) : FSL_SAI_RCR5(ofs))
+#define FSL_SAI_xDR(tx, ofs) (tx ? FSL_SAI_TDR(ofs) : FSL_SAI_RDR(ofs))
+#define FSL_SAI_xFR(tx, ofs) (tx ? FSL_SAI_TFR(ofs) : FSL_SAI_RFR(ofs))
#define FSL_SAI_xMR(tx) (tx ? FSL_SAI_TMR : FSL_SAI_RMR)
/* SAI Transmit/Receive Control Register */
@@ -158,6 +158,7 @@
struct fsl_sai_soc_data {
bool use_imx_pcm;
unsigned int fifo_depth;
+ unsigned int reg_offset;
};
struct fsl_sai {
--
2.20.1
^ permalink raw reply related
* Applied "ASoC: fsl_sai: Update Tx/Rx channel enable mask" to the asoc tree
From: Mark Brown @ 2019-08-07 13:30 UTC (permalink / raw)
To: Daniel Baluta
Cc: alsa-devel, angus, broonie, devicetree, festevam, kernel,
linux-imx, linux-kernel, l.stach
In-Reply-To: <20190806151214.6783-3-daniel.baluta@nxp.com>
The patch
ASoC: fsl_sai: Update Tx/Rx channel enable mask
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From b84f50b0fcb497a62068926fca793d2d213c7dbd Mon Sep 17 00:00:00 2001
From: Daniel Baluta <daniel.baluta@nxp.com>
Date: Tue, 6 Aug 2019 18:12:11 +0300
Subject: [PATCH] ASoC: fsl_sai: Update Tx/Rx channel enable mask
Tx channel enable (TCE) / Rx channel enable (RCE) bits
enable corresponding data channel for Tx/Rx operation.
Because SAI supports up the 8 channels TCE/RCE occupy
up the 8 bits inside TCR3/RCR3 registers we need to extend
the mask to reflect this.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Link: https://lore.kernel.org/r/20190806151214.6783-3-daniel.baluta@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/fsl/fsl_sai.c | 6 ++++--
sound/soc/fsl/fsl_sai.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index e4221f2a5ee3..f2698c94c9fe 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -599,7 +599,8 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
int ret;
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE,
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+ FSL_SAI_CR3_TRCE_MASK,
FSL_SAI_CR3_TRCE);
ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
@@ -614,7 +615,8 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE, 0);
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx),
+ FSL_SAI_CR3_TRCE_MASK, 0);
}
static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 4bb478041d67..20c5b9b1e8bc 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -110,6 +110,7 @@
/* SAI Transmit and Receive Configuration 3 Register */
#define FSL_SAI_CR3_TRCE BIT(16)
+#define FSL_SAI_CR3_TRCE_MASK GENMASK(23, 16)
#define FSL_SAI_CR3_WDFL(x) (x)
#define FSL_SAI_CR3_WDFL_MASK 0x1f
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox