* Re: [PATCH v25 01/16] dt: bindings: Add multicolor class dt bindings documention
From: Pavel Machek @ 2020-06-05 12:11 UTC (permalink / raw)
To: Rob Herring
Cc: Dan Murphy, Jacek Anaszewski, devicetree, Linux LED Subsystem,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqJ1XOYXyqj_VO2cFigVT=k5NTX3BO6RsDqQ-+pDBNJsrw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]
> > > There's no risk because you are supposed to apply both patches. I
> > > don't apply binding patches that are a part of a series like this.
> >
> > Yes, this is always guaranteed to happen, because "git bisect"
> > understand patch series. Oh, wait.
>
> What!? If the binding patch with the header comes first, how would
> bisect build the driver change without the header?
The driver is already in tree, and includes array of strings. When you
change the define, you need to update the array, too, because you
don't want to have invalid value in there.
> > Patches are supposed to be correct on their own. If your repository
> > filtering can not handle that, you need to fix that...
>
> I'm just asking you to follow the process that *everyone* else is
> following and works. It's not really about the repository filtering.
> That doesn't care. A binding ABI is defined by the schema and any
> defines it has. That is the logical unit that stands on its own.
It does not work in this case.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-05 12:20 UTC (permalink / raw)
To: linux-kernel, Florian Fainelli
Cc: Ray Jui, open list:SPI SUBSYSTEM, lukas, Rob Herring,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
Martin Sperl, Scott Branden, Nicolas Saenz Julienne,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <159135564425.14579.13716287498736798458.b4-ty@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 295 bytes --]
On Fri, Jun 05, 2020 at 12:14:07PM +0100, Mark Brown wrote:
> [1/1] spi: bcm2835: Enable shared interrupt support
> commit: ecfbd3cf3b8bb73ac6a80ddf430b5912fd4402a6
Eh, sorry - this was me fat fingering another fix. At the very least
this needs to wait for the end of the merge window.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Sakari Ailus @ 2020-06-05 12:14 UTC (permalink / raw)
To: Dongchun Zhu
Cc: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, drinkcat, tfiga, matthias.bgg, bingbu.cao,
srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang
In-Reply-To: <20200605105412.18813-3-dongchun.zhu@mediatek.com>
Hi Dongchun,
Thank you for the update.
On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> control to set the desired focus via IIC serial interface.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 13 ++
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 581 insertions(+)
> create mode 100644 drivers/media/i2c/dw9768.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d72c41..c92dc99 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5157,6 +5157,7 @@ L: linux-media@vger.kernel.org
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> +F: drivers/media/i2c/dw9768.c
>
> DONGWOON DW9807 LENS VOICE COIL DRIVER
> M: Sakari Ailus <sakari.ailus@linux.intel.com>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 125d596..afdf994 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1040,6 +1040,19 @@ config VIDEO_DW9714
> capability. This is designed for linear control of
> voice coil motors, controlled via I2C serial interface.
>
> +config VIDEO_DW9768
> + tristate "DW9768 lens voice coil support"
> + depends on I2C && VIDEO_V4L2
> + depends on PM
> + select MEDIA_CONTROLLER
> + select VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
> + help
> + This is a driver for the DW9768 camera lens voice coil.
> + DW9768 is a 10 bit DAC with 100mA output current sink
> + capability. This is designed for linear control of
> + voice coil motors, controlled via I2C serial interface.
> +
> config VIDEO_DW9807_VCM
> tristate "DW9807 lens voice coil support"
> depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 77bf7d0..4057476 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
> obj-$(CONFIG_VIDEO_AD5820) += ad5820.o
> obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
> obj-$(CONFIG_VIDEO_DW9714) += dw9714.o
> +obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
> obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c
> new file mode 100644
> index 0000000..f34a8ed
> --- /dev/null
> +++ b/drivers/media/i2c/dw9768.c
> @@ -0,0 +1,566 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 MediaTek Inc.
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fwnode.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define DW9768_NAME "dw9768"
> +#define DW9768_MAX_FOCUS_POS (1024 - 1)
> +/*
> + * This sets the minimum granularity for the focus positions.
> + * A value of 1 gives maximum accuracy for a desired focus position
> + */
> +#define DW9768_FOCUS_STEPS 1
> +
> +/*
> + * Ring control and Power control register
> + * Bit[1] RING_EN
> + * 0: Direct mode
> + * 1: AAC mode (ringing control mode)
> + * Bit[0] PD
> + * 0: Normal operation mode
> + * 1: Power down mode
> + * DW9768 requires waiting time of Topr after PD reset takes place.
> + */
> +#define DW9768_RING_PD_CONTROL_REG 0x02
> +#define DW9768_PD_MODE_OFF 0x00
> +#define DW9768_PD_MODE_EN BIT(0)
> +#define DW9768_AAC_MODE_EN BIT(1)
> +
> +/*
> + * DW9768 separates two registers to control the VCM position.
> + * One for MSB value, another is LSB value.
> + * DAC_MSB: D[9:8] (ADD: 0x03)
> + * DAC_LSB: D[7:0] (ADD: 0x04)
> + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> + */
> +#define DW9768_MSB_ADDR 0x03
> +#define DW9768_LSB_ADDR 0x04
> +#define DW9768_STATUS_ADDR 0x05
> +
> +/*
> + * AAC mode control & prescale register
> + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> + * 001 AAC2 0.48 x Tvib
> + * 010 AAC3 0.70 x Tvib
> + * 011 AAC4 0.75 x Tvib
> + * 101 AAC8 1.13 x Tvib
> + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> + * 000 2
> + * 001 1
> + * 010 1/2
> + * 011 1/4
> + * 100 8
> + * 101 4
> + */
> +#define DW9768_AAC_PRESC_REG 0x06
> +#define DW9768_AAC_MODE_SEL_MASK GENMASK(7, 5)
> +#define DW9768_CLOCK_PRE_SCALE_SEL_MASK GENMASK(2, 0)
> +
> +/*
> + * VCM period of vibration register
> + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> + * Dividing Rate is the internal clock dividing rate that is defined at
> + * PRESCALE register (ADD: 0x06)
> + */
> +#define DW9768_AAC_TIME_REG 0x07
> +
> +/*
> + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> + * or in the case of PD reset taking place.
> + */
> +#define DW9768_T_OPR_US 1000
> +#define DW9768_Tvib_MS_BASE10 (64 - 1)
> +#define DW9768_AAC_MODE_DEFAULT 2
> +#define DW9768_AAC_TIME_DEFAULT 0x20
> +#define DW9768_CLOCK_PRE_SCALE_DEFAULT 1
> +
> +/*
> + * This acts as the minimum granularity of lens movement.
> + * Keep this value power of 2, so the control steps can be
> + * uniformly adjusted for gradual lens movement, with desired
> + * number of control steps.
> + */
> +#define DW9768_MOVE_STEPS 16
> +
> +static const char * const dw9768_supply_names[] = {
> + "vin", /* Digital I/O power */
> + "vdd", /* Digital core power */
> +};
> +
> +/* dw9768 device structure */
> +struct dw9768 {
> + struct regulator_bulk_data supplies[ARRAY_SIZE(dw9768_supply_names)];
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *focus;
> + struct v4l2_subdev sd;
> +
> + u32 aac_mode;
> + u32 aac_timing;
> + u32 clock_presc;
> +};
> +
> +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> +{
> + return container_of(subdev, struct dw9768, sd);
> +}
> +
> +struct regval_list {
> + u8 reg_num;
> + u8 value;
> +};
> +
> +struct dw9768_aac_mode_ot_multi {
> + u32 aac_mode_enum;
> + u32 ot_multi_base100;
> +};
> +
> +struct dw9768_clk_presc_dividing_rate {
> + u32 clk_presc_enum;
> + u32 dividing_rate_base100;
> +};
> +
> +static const struct dw9768_aac_mode_ot_multi aac_mode_ot_multi[] = {
> + {1, 48},
> + {2, 70},
> + {3, 75},
> + {5, 113},
> +};
> +
> +static const struct dw9768_clk_presc_dividing_rate presc_dividing_rate[] = {
> + {0, 200},
> + {1, 100},
> + {2, 50},
> + {3, 25},
> + {4, 800},
> + {5, 400},
> +};
> +
> +static u32 dw9768_find_ot_multi(u32 aac_mode_param)
> +{
> + u32 cur_ot_multi_base100 = 70;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(aac_mode_ot_multi); i++) {
> + if (aac_mode_ot_multi[i].aac_mode_enum == aac_mode_param) {
> + cur_ot_multi_base100 =
> + aac_mode_ot_multi[i].ot_multi_base100;
> + }
> + }
> +
> + return cur_ot_multi_base100;
> +}
> +
> +static u32 dw9768_find_dividing_rate(u32 presc_param)
> +{
> + u32 cur_clk_dividing_rate_base100 = 100;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(presc_dividing_rate); i++) {
> + if (presc_dividing_rate[i].clk_presc_enum == presc_param) {
> + cur_clk_dividing_rate_base100 =
> + presc_dividing_rate[i].dividing_rate_base100;
> + }
> + }
> +
> + return cur_clk_dividing_rate_base100;
> +}
> +
> +/*
> + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> + * For current VCM mode: AAC3, Operation Time would be 0.70 x Tvib.
> + * Tvib = (6.3ms + AACT[5:0] * 0.1MS) * Dividing Rate.
> + * Below is calculation of the operation delay for each step.
> + */
> +static inline u32 dw9768_cal_move_delay(u32 aac_mode_param, u32 presc_param,
> + u32 aac_timing_param)
> +{
> + u32 Tvib_us;
> + u32 ot_multi_base100;
> + u32 clk_dividing_rate_base100;
> +
> + ot_multi_base100 = dw9768_find_ot_multi(aac_mode_param);
> +
> + clk_dividing_rate_base100 = dw9768_find_dividing_rate(presc_param);
> +
> + Tvib_us = (DW9768_Tvib_MS_BASE10 + aac_timing_param) *
> + clk_dividing_rate_base100;
> +
> + return Tvib_us * ot_multi_base100;
> +}
> +
> +static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + val = ((unsigned char)ret & ~mask) | (val & mask);
> +
> + return i2c_smbus_write_byte_data(client, reg, val);
> +}
> +
> +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> +
> + /* Write VCM position to registers */
> + return i2c_smbus_write_word_swapped(client, DW9768_MSB_ADDR, val);
> +}
> +
> +static int dw9768_init(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + u32 move_delay_us;
> + int ret, val;
> +
> + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_OFF);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + /* Set DW9768_RING_PD_CONTROL_REG to DW9768_AAC_MODE_EN(0x01) */
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_AAC_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /* Set AAC mode */
> + ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
> + DW9768_AAC_MODE_SEL_MASK,
> + dw9768->aac_mode << 5);
> + if (ret < 0)
> + return ret;
> +
> + /* Set clock presc */
> + if (dw9768->clock_presc != DW9768_CLOCK_PRE_SCALE_DEFAULT) {
> + ret = dw9768_mod_reg(dw9768, DW9768_AAC_PRESC_REG,
> + DW9768_CLOCK_PRE_SCALE_SEL_MASK,
> + dw9768->clock_presc);
> + if (ret < 0)
> + return ret;
> + }
> +
> + /* Set AAC Timing */
> + if (dw9768->aac_timing != DW9768_AAC_TIME_DEFAULT) {
> + ret = i2c_smbus_write_byte_data(client, DW9768_AAC_TIME_REG,
> + dw9768->aac_timing);
> + if (ret < 0)
> + return ret;
> + }
> +
> + move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> + dw9768->clock_presc,
> + dw9768->aac_timing) / 100;
> +
> + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> + val <= dw9768->focus->val;
> + val += DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
> + return ret;
> + }
> + usleep_range(move_delay_us, move_delay_us + 1000);
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_release(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> + dw9768->clock_presc,
> + dw9768->aac_timing) / 100;
> + int ret, val;
> +
> + val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> + for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "I2C write fail: %d", ret);
> + return ret;
> + }
> + usleep_range(move_delay_us, move_delay_us + 1000);
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + return 0;
> +}
> +
> +static int dw9768_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + dw9768_release(dw9768);
> + regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> +
> + return 0;
> +}
> +
> +static int dw9768_runtime_resume(struct device *dev)
__maybe_unused for this and the suspend callback.
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /*
> + * The datasheet refers to t_OPR that needs to be waited before sending
> + * I2C commands after power-up.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + ret = dw9768_init(dw9768);
> + if (ret < 0)
> + goto disable_regulator;
> +
> + return 0;
> +
> +disable_regulator:
> + regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> +
> + return ret;
> +}
> +
> +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct dw9768 *dw9768 = container_of(ctrl->handler,
> + struct dw9768, ctrls);
> +
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> + return dw9768_set_dac(dw9768, ctrl->val);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> + .s_ctrl = dw9768_set_ctrl,
> +};
> +
> +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + int ret;
> +
> + ret = pm_runtime_get_sync(sd->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(sd->dev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + pm_runtime_put(sd->dev);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> + .open = dw9768_open,
> + .close = dw9768_close,
> +};
> +
> +static const struct v4l2_subdev_ops dw9768_ops = { };
> +
> +static int dw9768_init_controls(struct dw9768 *dw9768)
> +{
> + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> +
> + v4l2_ctrl_handler_init(hdl, 1);
> +
> + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE, 0,
> + DW9768_MAX_FOCUS_POS,
> + DW9768_FOCUS_STEPS, 0);
> +
> + if (hdl->error)
> + return hdl->error;
> +
> + dw9768->sd.ctrl_handler = hdl;
> +
> + return 0;
> +}
> +
> +static int dw9768_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct dw9768 *dw9768;
> + u32 aac_mode_select;
> + u32 aac_timing_select;
> + u32 clock_presc_select;
> + unsigned int i;
> + int ret;
> +
> + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> + if (!dw9768)
> + return -ENOMEM;
> +
> + /* Initialize subdev */
> + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> +
> + dw9768->aac_mode = DW9768_AAC_MODE_DEFAULT;
> + dw9768->aac_timing = DW9768_AAC_TIME_DEFAULT;
> + dw9768->clock_presc = DW9768_CLOCK_PRE_SCALE_DEFAULT;
> +
> + /* Optional indication of AAC mode select */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-mode",
> + &aac_mode_select);
> +
> + if (!ret)
> + dw9768->aac_mode = aac_mode_select;
> +
> + /* Optional indication of clock pre-scale select */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,clock-presc",
> + &clock_presc_select);
> +
> + if (!ret)
> + dw9768->clock_presc = clock_presc_select;
> +
> + /* Optional indication of AAC Timing */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-timing",
> + &aac_timing_select);
> +
> + if (!ret)
> + dw9768->aac_timing = aac_timing_select;
You can assign the defaults to the dw9768 struct and use the fwnode
property API to read the properties into the same fields. No return values
need to be checked.
> +
> + for (i = 0; i < ARRAY_SIZE(dw9768_supply_names); i++)
> + dw9768->supplies[i].supply = dw9768_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dw9768_supply_names),
> + dw9768->supplies);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + /* Initialize controls */
> + ret = dw9768_init_controls(dw9768);
> + if (ret)
> + goto err_free_handler;
> +
> + /* Initialize subdev */
> + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + dw9768->sd.internal_ops = &dw9768_int_ops;
> +
> + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> + if (ret < 0)
> + goto err_free_handler;
> +
> + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> + pm_runtime_enable(dev);
> + if (!pm_runtime_enabled(dev)) {
> + ret = dw9768_runtime_resume(dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to power on: %d\n", ret);
> + goto err_clean_entity;
> + }
> + }
> +
> + ret = v4l2_async_register_subdev(&dw9768->sd);
> + if (ret < 0) {
> + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> + goto error_async_register;
> + }
> +
> + return 0;
> +
> +error_async_register:
> + if (!pm_runtime_enabled(dev))
> + dw9768_runtime_suspend(dev);
> +err_clean_entity:
> + media_entity_cleanup(&dw9768->sd.entity);
> +err_free_handler:
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> +
> + return ret;
> +}
> +
> +static int dw9768_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + v4l2_async_unregister_subdev(&dw9768->sd);
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> + media_entity_cleanup(&dw9768->sd.entity);
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + dw9768_runtime_suspend(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dw9768_of_table[] = {
> + { .compatible = "dongwoon,dw9768" },
> + { .compatible = "giantec,gt9769" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> +
> +static const struct dev_pm_ops dw9768_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> +};
> +
> +static struct i2c_driver dw9768_i2c_driver = {
> + .driver = {
> + .name = DW9768_NAME,
> + .pm = &dw9768_pm_ops,
> + .of_match_table = dw9768_of_table,
> + },
> + .probe_new = dw9768_probe,
> + .remove = dw9768_remove,
> +};
> +module_i2c_driver(dw9768_i2c_driver);
> +
> +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> +MODULE_DESCRIPTION("DW9768 VCM driver");
> +MODULE_LICENSE("GPL v2");
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH] ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add device tree for camera DB
From: Geert Uytterhoeven @ 2020-06-05 12:27 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Rob Herring, Magnus Damm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Linux-Renesas, Prabhakar
In-Reply-To: <1590586141-21006-1-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi Prabhakar,
On Wed, May 27, 2020 at 3:29 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add support for the camera daughter board which is connected to
> iWave's RZ/G1H Qseven carrier board. Also enable ttySC[0135] and
> ethernet1 interfaces.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- /dev/null
> +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
> +&scifb1 {
> + pinctrl-0 = <&scifb1_pins>;
> + pinctrl-names = "default";
> + status = "okay";
Before I queue this in renesas-devel for v5.9, I have on question:
As this port carries RTS/CTS signals, perhaps you want to add
rts-gpios = <&gpio4 21 GPIO_ACTIVE_LOW>;
cts-gpios = <&gpio4 17 GPIO_ACTIVE_LOW>;
?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v26 01/15] dt: bindings: Add multicolor class dt bindings documention
From: Dan Murphy @ 2020-06-05 12:42 UTC (permalink / raw)
To: Rob Herring; +Cc: jacek.anaszewski, pavel, devicetree, linux-leds, linux-kernel
In-Reply-To: <20200604224026.GA4153787@bogus>
Rob
On 6/4/20 5:40 PM, Rob Herring wrote:
> On Thu, Jun 04, 2020 at 07:04:50AM -0500, Dan Murphy wrote:
>> Add DT bindings for the LEDs multicolor class framework.
>> Add multicolor ID to the color ID list for device tree bindings.
>>
>> CC: Rob Herring <robh@kernel.org>
>> Acked-by: Pavel Machek <pavel@ucw.cz>
>> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> .../bindings/leds/leds-class-multicolor.yaml | 39 +++++++++++++++++++
>> include/dt-bindings/leds/common.h | 3 +-
>> 2 files changed, 41 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
>> new file mode 100644
>> index 000000000000..6cab2a1405e1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.yaml
>> @@ -0,0 +1,39 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/leds/leds-class-multicolor.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Common properties for the multicolor LED class.
>> +
>> +maintainers:
>> + - Dan Murphy <dmurphy@ti.com>
>> +
>> +description: |
>> + Bindings for multi color LEDs show how to describe current outputs of
>> + either integrated multi-color LED elements (like RGB, RGBW, RGBWA-UV
>> + etc.) or standalone LEDs, to achieve logically grouped multi-color LED
>> + modules. This is achieved by adding multi-led nodes layer to the
>> + monochrome LED bindings.
>> + The nodes and properties defined in this document are unique to the multicolor
>> + LED class. Common LED nodes and properties are inherited from the common.txt
>> + within this documentation directory.
>> +
>> +patternProperties:
>> + "^multi-led@([0-9a-f])$":
>> + type: object
>> + description: Represents the LEDs that are to be grouped.
>> + properties:
>> + #allOf:
>> + #- $ref: "common.yaml#"
> Why is this commented out? Other than it is wrong. Uncommented, this
> would be defining a DT property called 'allOf'.
>
> You can drop 'allOf' now. '$ref' should be at the level of 'properties'.
I used the example from the rohm,bd71828-leds.yaml where these lines appear.
So that binding is wrong as well.
>> +
>> + color:
>> + $ref: /schemas/types.yaml#definitions/uint32
> common.yaml already defines the type, so drop this.
OK
Dan
^ permalink raw reply
* Re: [PATCH v4 06/11] gpio: add support for the sl28cpld GPIO controller
From: Michael Walle @ 2020-06-05 12:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: open list:GPIO SUBSYSTEM, devicetree, Linux Kernel Mailing List,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm Mailing List,
Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
Andy Shevchenko
In-Reply-To: <CAHp75VfRhL1f-XD=PMbqd3BLeJQzQMFAupSzqAvx0g7-X_2VhQ@mail.gmail.com>
Am 2020-06-05 14:00, schrieb Andy Shevchenko:
> On Fri, Jun 5, 2020 at 12:14 AM Michael Walle <michael@walle.cc> wrote:
>
>> Add support for the GPIO controller of the sl28 board management
>> controller. This driver is part of a multi-function device.
>>
>> A controller has 8 lines. There are three different flavors:
>> full-featured GPIO with interrupt support, input-only and output-only.
>
> ...
>
>> +#include <linux/of_device.h>
>
> I think also not needed.
> But wait...
>
>> + return devm_regmap_add_irq_chip_np(dev, dev_of_node(dev),
>> regmap,
>
> It seems regmap needs to be converted to use fwnode.
Mhh, this _np functions was actually part of this series in the
beginning.
>> + irq, IRQF_SHARED |
>> IRQF_ONESHOT, 0,
>> + irq_chip, &gpio->irq_data);
>
> ...
>
>> + if (!pdev->dev.parent)
>> + return -ENODEV;
>
> Are we expecting to get shot into foot? I mean why we need this check?
Can we be sure, that we always have a parent node? You are the first
which complains about this ;) There were some other comments to move
this to the beginning of the function.
>
>> + dev_id = platform_get_device_id(pdev);
>> + if (dev_id)
>> + type = dev_id->driver_data;
>
> Oh, no. In new code we don't need this. We have facilities to provide
> platform data in a form of fwnode.
Ok I'll look into that.
But I already have a question, so there are of_property_read_xx(), which
seems to be the old functions, then there is device_property_read_xx()
and
fwnode_property_read_xx(). What is the difference between the latter
two?
>
>> + else
>> + type =
>> (uintptr_t)of_device_get_match_data(&pdev->dev);
>
> So does this. device_get_match_data().
ok
>
>> + if (!type)
>> + return -ENODEV;
>
> ...
>
>> + if (irq_support &&
>
> Why do you need this flag? Can't simple IRQ number be sufficient?
I want to make sure, the is no misconfiguration. Eg. only GPIO
flavors which has irq_support set, have the additional interrupt
registers.
>
>> + device_property_read_bool(&pdev->dev,
>> "interrupt-controller")) {
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0)
>> + return irq;
>> +
>> + ret = sl28cpld_gpio_irq_init(&pdev->dev, gpio, regmap,
>> + base, irq);
>> + if (ret)
>> + return ret;
>> +
>> + config.irq_domain =
>> regmap_irq_get_domain(gpio->irq_data);
>> + }
>
> ...
>
>> +static const struct of_device_id sl28cpld_gpio_of_match[] = {
>
>> + { .compatible = "kontron,sl28cpld-gpio",
>> + .data = (void *)SL28CPLD_GPIO },
>> + { .compatible = "kontron,sl28cpld-gpi",
>> + .data = (void *)SL28CPLD_GPI },
>> + { .compatible = "kontron,sl28cpld-gpo",
>> + .data = (void *)SL28CPLD_GPO },
>
> All above can be twice less LOCs.
They are longer than 80 chars. Or do I miss something?
>
>> + {},
>
> No comma.
ok
>> +};
>
>
> ...
>
>> + .name = KBUILD_MODNAME,
>
> This actually not good idea in long term. File name can change and
> break an ABI.
Ahh an explanation, why this is bad. Ok makes sense, although to be
fair,
.id_table should be used for the driver name matching. I'm not sure if
this is used somewhere else, though.
--
-michael
^ permalink raw reply
* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Andy Shevchenko @ 2020-06-05 12:46 UTC (permalink / raw)
To: Dongchun Zhu
Cc: linus.walleij, bgolaszewski, mchehab, robh+dt, mark.rutland,
sakari.ailus, drinkcat, tfiga, matthias.bgg, bingbu.cao,
srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang
In-Reply-To: <20200605105412.18813-3-dongchun.zhu@mediatek.com>
On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> control to set the desired focus via IIC serial interface.
...
> +config VIDEO_DW9768
> + tristate "DW9768 lens voice coil support"
> + depends on I2C && VIDEO_V4L2
No compile test?
> + depends on PM
This is very strange dependency for ordinary driver.
> + select MEDIA_CONTROLLER
> + select VIDEO_V4L2_SUBDEV_API
> + select V4L2_FWNODE
...
> +/*
> + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> + * or in the case of PD reset taking place.
> + */
> +#define DW9768_T_OPR_US 1000
> +#define DW9768_Tvib_MS_BASE10 (64 - 1)
> +#define DW9768_AAC_MODE_DEFAULT 2
> +#define DW9768_AAC_TIME_DEFAULT 0x20
Hex? Why not decimal?
> +#define DW9768_CLOCK_PRE_SCALE_DEFAULT 1
...
> +static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(client, reg);
> + if (ret < 0)
> + return ret;
> +
> + val = ((unsigned char)ret & ~mask) | (val & mask);
This cast is weird.
> +
> + return i2c_smbus_write_byte_data(client, reg, val);
> +}
...
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
One line?
...
> +static int dw9768_release(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> + dw9768->clock_presc,
> + dw9768->aac_timing) / 100;
> + int ret, val;
> +
> + val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> + for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "I2C write fail: %d", ret);
> + return ret;
> + }
> + usleep_range(move_delay_us, move_delay_us + 1000);
> + }
It will look more naturally in the multiplier kind of value.
unsigned int steps = DIV_ROUND_UP(...);
while (steps--) {
...(..., steps * ..._MOVE_STEPS);
...
}
but double check arithmetics.
> + return 0;
> +}
Also it seems we need to have writex_poll_timeout() implementation (see
iopoll.h).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 10/10] ARM: dts: r8a7742-iwg21d-q7: Add support for iWave G21D-Q7 board based on RZ/G1H
From: Geert Uytterhoeven @ 2020-06-05 12:51 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Magnus Damm, Rob Herring, Vinod Koul, Ulf Hansson, Linus Walleij,
Greg Kroah-Hartman, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, dmaengine, Linux MMC List,
open list:GPIO SUBSYSTEM, open list:SERIAL DRIVERS, Prabhakar
In-Reply-To: <1588542414-14826-11-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi Prabhakar,
On Sun, May 3, 2020 at 11:47 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add support for iWave RainboW-G21D-Qseven board based on RZ/G1H.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> --- /dev/null
> +++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7.dts
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the iWave-RZ/G1H Qseven board
> + *
> + * Copyright (C) 2020 Renesas Electronics Corp.
> + */
> +
> +/dts-v1/;
> +#include "r8a7742-iwg21m.dtsi"
> +
> +/ {
> + model = "iWave Systems RainboW-G21D-Qseven board based on RZ/G1H";
> + compatible = "iwave,g21d", "iwave,g21m", "renesas,r8a7742";
> +
> + aliases {
> + serial2 = &scifa2;
> + };
> +
> + chosen {
> + bootargs = "ignore_loglevel root=/dev/mmcblk0p1 rw rootwait";
> + stdout-path = "serial2:115200n8";
> + };
> +};
> +
> +&pfc {
> + scifa2_pins: scifa2 {
> + groups = "scifa2_data_c";
Upon second look, I think this group is wrong. While labeled SCIFA2 in
the SOM schematics, these signals seem to be connected to a debugging
interface.
The real UART2 seems to be present on the camera daughter board. Those
signals are labeled "SCIFA2" in the camera board schematics, but "SCIF2"
in the SOM schematics. This is OK, as "scif2_data" and "scifa2_data"
share the same pins, so you can choose either SCIF2 or SCIFA2 to drive
them.
If I'm right, please change the group, and move all serial2 descriptions
to the camera board DTS.
> + function = "scifa2";
> + };
> +};
> +
> +&scifa2 {
> + pinctrl-0 = <&scifa2_pins>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Tomasz Figa @ 2020-06-05 13:02 UTC (permalink / raw)
To: Sakari Ailus, Dongchun Zhu
Cc: Dongchun Zhu, linus.walleij, bgolaszewski, mchehab,
andriy.shevchenko, robh+dt, mark.rutland, drinkcat, matthias.bgg,
bingbu.cao, srv_heupstream, linux-mediatek, linux-arm-kernel,
sj.huang, linux-media, devicetree, louis.kuo, shengnan.wang
In-Reply-To: <20200605121459.GS16711@paasikivi.fi.intel.com>
On Fri, Jun 05, 2020 at 03:14:59PM +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> Thank you for the update.
>
> On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> > Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> > control to set the desired focus via IIC serial interface.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/media/i2c/Kconfig | 13 ++
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 581 insertions(+)
> > create mode 100644 drivers/media/i2c/dw9768.c
[snip]
> > +static int dw9768_runtime_suspend(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > +
> > + dw9768_release(dw9768);
> > + regulator_bulk_disable(ARRAY_SIZE(dw9768_supply_names),
> > + dw9768->supplies);
> > +
> > + return 0;
> > +}
> > +
> > +static int dw9768_runtime_resume(struct device *dev)
>
> __maybe_unused for this and the suspend callback.
>
These are always used. If runtime PM is disabled, they are called
explicitly in probe and remove.
Best regards,
Tomasz
^ permalink raw reply
* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Tomasz Figa @ 2020-06-05 13:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dongchun Zhu, linus.walleij, bgolaszewski, mchehab, robh+dt,
mark.rutland, sakari.ailus, drinkcat, matthias.bgg, bingbu.cao,
srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang
In-Reply-To: <20200605124643.GG2428291@smile.fi.intel.com>
Hi Andy,
On Fri, Jun 05, 2020 at 03:46:43PM +0300, Andy Shevchenko wrote:
> On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> > Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> > control to set the desired focus via IIC serial interface.
>
> ...
>
> > +config VIDEO_DW9768
> > + tristate "DW9768 lens voice coil support"
> > + depends on I2C && VIDEO_V4L2
>
> No compile test?
>
> > + depends on PM
>
> This is very strange dependency for ordinary driver.
>
> > + select MEDIA_CONTROLLER
> > + select VIDEO_V4L2_SUBDEV_API
> > + select V4L2_FWNODE
>
> ...
>
> > +/*
> > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > + * or in the case of PD reset taking place.
> > + */
> > +#define DW9768_T_OPR_US 1000
> > +#define DW9768_Tvib_MS_BASE10 (64 - 1)
> > +#define DW9768_AAC_MODE_DEFAULT 2
>
> > +#define DW9768_AAC_TIME_DEFAULT 0x20
>
> Hex? Why not decimal?
>
> > +#define DW9768_CLOCK_PRE_SCALE_DEFAULT 1
>
> ...
>
> > +static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + int ret;
> > +
> > + ret = i2c_smbus_read_byte_data(client, reg);
> > + if (ret < 0)
> > + return ret;
> > +
>
> > + val = ((unsigned char)ret & ~mask) | (val & mask);
>
> This cast is weird.
>
> > +
> > + return i2c_smbus_write_byte_data(client, reg, val);
> > +}
>
> ...
>
> > + dev_err(&client->dev, "%s I2C failure: %d",
> > + __func__, ret);
>
> One line?
>
> ...
>
> > +static int dw9768_release(struct dw9768 *dw9768)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> > + dw9768->clock_presc,
> > + dw9768->aac_timing) / 100;
> > + int ret, val;
> > +
> > + val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > + for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
> > + ret = dw9768_set_dac(dw9768, val);
> > + if (ret) {
> > + dev_err(&client->dev, "I2C write fail: %d", ret);
> > + return ret;
> > + }
> > + usleep_range(move_delay_us, move_delay_us + 1000);
> > + }
>
>
> It will look more naturally in the multiplier kind of value.
>
> unsigned int steps = DIV_ROUND_UP(...);
>
> while (steps--) {
> ...(..., steps * ..._MOVE_STEPS);
> ...
> }
>
> but double check arithmetics.
>
First of all, thank for the review!
As for this particular change suggestion, I suspect this could be a
subjective thing, because for me the current code looks more naturally
and it's what the other VCM drivers do.
> > + return 0;
> > +}
>
>
> Also it seems we need to have writex_poll_timeout() implementation (see
> iopoll.h).
What would it be supposed to do? readx_poll_timeout() repeats reading
the same registers and sleeping until a condition becomes true, which is
basically "polling". In this case we're not polling anything and we're
not writing the same value, but it's an explicit algorithm of this
driver to power down the VCM corectly.
However, given that it's quite common among VCMs to require this kind of
phased power down - we could indeed provide some V4L2 VCM helpers for open
and release.
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH v4 06/11] gpio: add support for the sl28cpld GPIO controller
From: Andy Shevchenko @ 2020-06-05 13:15 UTC (permalink / raw)
To: Michael Walle
Cc: open list:GPIO SUBSYSTEM, devicetree, Linux Kernel Mailing List,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm Mailing List,
Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman
In-Reply-To: <216db3154b46bd80202873df055bb3f3@walle.cc>
On Fri, Jun 05, 2020 at 02:42:53PM +0200, Michael Walle wrote:
> Am 2020-06-05 14:00, schrieb Andy Shevchenko:
> > On Fri, Jun 5, 2020 at 12:14 AM Michael Walle <michael@walle.cc> wrote:
> > > + return devm_regmap_add_irq_chip_np(dev, dev_of_node(dev),
> > > regmap,
> >
> > It seems regmap needs to be converted to use fwnode.
>
> Mhh, this _np functions was actually part of this series in the
> beginning.
Then, please, make them fwnode aware rather than OF centric.
> > > IRQF_ONESHOT, 0,
> > > + irq_chip, &gpio->irq_data);
...
> > > + dev_id = platform_get_device_id(pdev);
> > > + if (dev_id)
> > > + type = dev_id->driver_data;
> >
> > Oh, no. In new code we don't need this. We have facilities to provide
> > platform data in a form of fwnode.
>
> Ok I'll look into that.
>
> But I already have a question, so there are of_property_read_xx(), which
> seems to be the old functions, then there is device_property_read_xx() and
> fwnode_property_read_xx(). What is the difference between the latter two?
It's easy. device_*() requires struct device to be established for this, so,
operates only against devices, while the fwnode_*() operates on pure data which
might or might not be related to any devices. If you understand OF examples
better, consider device node vs. child of such node.
...
> > > + if (irq_support &&
> >
> > Why do you need this flag? Can't simple IRQ number be sufficient?
>
> I want to make sure, the is no misconfiguration. Eg. only GPIO
> flavors which has irq_support set, have the additional interrupt
> registers.
In gpio-dwapb, for example, we simple check two things: a) hardware limitation
(if IRQ is assigned to a proper port) and b) if there is any IRQ comes from DT,
ACPI, etc.
> > > + device_property_read_bool(&pdev->dev,
> > > "interrupt-controller")) {
> > > + irq = platform_get_irq(pdev, 0);
> > > + if (irq < 0)
> > > + return irq;
> > > +
> > > + ret = sl28cpld_gpio_irq_init(&pdev->dev, gpio, regmap,
> > > + base, irq);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + config.irq_domain =
> > > regmap_irq_get_domain(gpio->irq_data);
> > > + }
...
> > > + { .compatible = "kontron,sl28cpld-gpio",
> > > + .data = (void *)SL28CPLD_GPIO },
> > > + { .compatible = "kontron,sl28cpld-gpi",
> > > + .data = (void *)SL28CPLD_GPI },
> > > + { .compatible = "kontron,sl28cpld-gpo",
> > > + .data = (void *)SL28CPLD_GPO },
> >
> > All above can be twice less LOCs.
>
> They are longer than 80 chars. Or do I miss something?
We have 100 :-)
...
> > > + .name = KBUILD_MODNAME,
> >
> > This actually not good idea in long term. File name can change and break
> > an ABI.
>
> Ahh an explanation, why this is bad. Ok makes sense, although to be fair,
> .id_table should be used for the driver name matching. I'm not sure if
> this is used somewhere else, though.
I saw in my practice chain of renames for a driver. Now, if somebody
somewhere would like to instantiate a platform driver by its name...
Oops, ABI breakage.
And of course using platform data for such device makes less sense.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-05 13:20 UTC (permalink / raw)
To: Robin Murphy
Cc: Florian Fainelli, linux-kernel,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, lukas, Ray Jui, Rob Herring,
open list:SPI SUBSYSTEM,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, Nicolas Saenz Julienne
In-Reply-To: <142d48ae-2725-1368-3e11-658449662371@arm.com>
[-- Attachment #1: Type: text/plain, Size: 817 bytes --]
On Fri, Jun 05, 2020 at 12:34:36PM +0100, Robin Murphy wrote:
> On 2020-06-04 22:28, Florian Fainelli wrote:
> > For the BCM2835 case which is deemed performance critical, we would like
> > to continue using an interrupt handler which does not have the extra
> > comparison on BCM2835_SPI_CS_INTR.
> FWIW, if I'm reading the patch correctly, then with sensible codegen that
> "overhead" should amount to a bit test on a live register plus a not-taken
> conditional branch - according to the 1176 TRM that should add up to a
> whopping 2 cycles. If that's really significant then I'd have to wonder
> whether you want to be at the mercy of the whole generic IRQ stack at all,
> and should perhaps consider using FIQ instead.
Yes, and indeed the compiler does seem to manage that. It *is* non-zero
overhead though.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-bindings: irqchip: renesas-rza1-irqc: Convert to json-schema
From: Niklas Söderlund @ 2020-06-05 13:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring, Chris Brandt, Thomas Gleixner, Jason Cooper,
Marc Zyngier, devicetree, linux-kernel, linux-renesas-soc
In-Reply-To: <20200528132853.1751-1-geert+renesas@glider.be>
Hi Geert,
Thanks for your work.
On 2020-05-28 15:28:53 +0200, Geert Uytterhoeven wrote:
> Convert the Renesas RZ/A1 Interrupt Controller Device Tree binding
> documentation to json-schema.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> Validation depends on "[PATCH dt-schema] Fix interrupt controllers with
> interrupt-map".
> http://lore.kernel.org/r/20200528132323.30288-1-geert+renesas@glider.be
> ---
> .../renesas,rza1-irqc.txt | 43 ----------
> .../renesas,rza1-irqc.yaml | 80 +++++++++++++++++++
> 2 files changed, 80 insertions(+), 43 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.txt
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.txt
> deleted file mode 100644
> index 727b7e4cd6e01110..0000000000000000
> --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -DT bindings for the Renesas RZ/A1 Interrupt Controller
> -
> -The RZ/A1 Interrupt Controller is a front-end for the GIC found on Renesas
> -RZ/A1 and RZ/A2 SoCs:
> - - IRQ sense select for 8 external interrupts, 1:1-mapped to 8 GIC SPI
> - interrupts,
> - - NMI edge select.
> -
> -Required properties:
> - - compatible: Must be "renesas,<soctype>-irqc", and "renesas,rza1-irqc" as
> - fallback.
> - Examples with soctypes are:
> - - "renesas,r7s72100-irqc" (RZ/A1H)
> - - "renesas,r7s9210-irqc" (RZ/A2M)
> - - #interrupt-cells: Must be 2 (an interrupt index and flags, as defined
> - in interrupts.txt in this directory)
> - - #address-cells: Must be zero
> - - interrupt-controller: Marks the device as an interrupt controller
> - - reg: Base address and length of the memory resource used by the interrupt
> - controller
> - - interrupt-map: Specifies the mapping from external interrupts to GIC
> - interrupts
> - - interrupt-map-mask: Must be <7 0>
> -
> -Example:
> -
> - irqc: interrupt-controller@fcfef800 {
> - compatible = "renesas,r7s72100-irqc", "renesas,rza1-irqc";
> - #interrupt-cells = <2>;
> - #address-cells = <0>;
> - interrupt-controller;
> - reg = <0xfcfef800 0x6>;
> - interrupt-map =
> - <0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> - <1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> - <2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> - <3 0 &gic GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> - <4 0 &gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> - <5 0 &gic GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
> - <6 0 &gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> - <7 0 &gic GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-map-mask = <7 0>;
> - };
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.yaml
> new file mode 100644
> index 0000000000000000..755cdfabfcd06c85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rza1-irqc.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rza1-irqc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/A1 Interrupt Controller
> +
> +maintainers:
> + - Chris Brandt <chris.brandt@renesas.com>
> + - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +description: |
> + The RZ/A1 Interrupt Controller is a front-end for the GIC found on Renesas RZ/A1 and
> + RZ/A2 SoCs:
> + - IRQ sense select for 8 external interrupts, 1:1-mapped to 8 GIC SPI interrupts,
> + - NMI edge select.
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,r7s72100-irqc # RZ/A1H
> + - renesas,r7s9210-irqc # RZ/A2M
> + - const: renesas,rza1-irqc
> +
> + '#interrupt-cells':
> + const: 2
> +
> + '#address-cells':
> + const: 0
> +
> + interrupt-controller: true
> +
> + reg:
> + maxItems: 1
> +
> + interrupt-map:
> + maxItems: 8
> + description: Specifies the mapping from external interrupts to GIC interrupts.
> +
> + interrupt-map-mask:
> + items:
> + - const: 7
> + - const: 0
> +
> +required:
> + - compatible
> + - '#interrupt-cells'
> + - '#address-cells'
> + - interrupt-controller
> + - reg
> + - interrupt-map
> + - interrupt-map-mask
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + irqc: interrupt-controller@fcfef800 {
> + compatible = "renesas,r7s72100-irqc", "renesas,rza1-irqc";
> + #interrupt-cells = <2>;
> + #address-cells = <0>;
> + interrupt-controller;
> + reg = <0xfcfef800 0x6>;
> + interrupt-map =
> + <0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> + <1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> + <2 0 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> + <3 0 &gic GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> + <4 0 &gic GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> + <5 0 &gic GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
> + <6 0 &gic GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> + <7 0 &gic GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-map-mask = <7 0>;
> + };
> --
> 2.17.1
>
--
Regards,
Niklas Söderlund
^ permalink raw reply
* [PATCH v9 00/14] add ecspi ERR009165 for i.mx6/7 soc family
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO
transfer to be send twice in DMA mode. Please get more information from:
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf. The workaround is adding
new sdma ram script which works in XCH mode as PIO inside sdma instead
of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0. The issue should be
exist on all legacy i.mx6/7 soc family before i.mx6ul.
NXP fix this design issue from i.mx6ul, so newer chips including i.mx6ul/
6ull/6sll do not need this workaroud anymore. All other i.mx6/7/8 chips
still need this workaroud. This patch set add new 'fsl,imx6ul-ecspi'
for ecspi driver and 'ecspi_fixed' in sdma driver to choose if need errata
or not.
The first two reverted patches should be the same issue, though, it
seems 'fixed' by changing to other shp script. Hope Sean or Sascha could
have the chance to test this patch set if could fix their issues.
Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not work
on i.mx8mm because the event id is zero.
PS:
Please get sdma firmware from below linux-firmware and copy it to your
local rootfs /lib/firmware/imx/sdma.
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/imx/sdma
v2:
1.Add commit log for reverted patches.
2.Add comment for 'ecspi_fixed' in sdma driver.
3.Add 'fsl,imx6sll-ecspi' compatible instead of 'fsl,imx6ul-ecspi'
rather than remove.
v3:
1.Confirm with design team make sure ERR009165 fixed on i.mx6ul/i.mx6ull
/i.mx6sll, not fixed on i.mx8m/8mm and other i.mx6/7 legacy chips.
Correct dts related dts patch in v2.
2.Clean eratta information in binding doc and new 'tx_glitch_fixed' flag
in spi-imx driver to state ERR009165 fixed or not.
3.Enlarge burst size to fifo size for tx since tx_wml set to 0 in the
errata workaroud, thus improve performance as possible.
v4:
1.Add Ack tag from Mark and Vinod
2.Remove checking 'event_id1' zero as 'event_id0'.
v5:
1.Add the last patch for compatible with the current uart driver which
using rom script, so both uart ram script and rom script supported
in latest firmware, by default uart rom script used. UART driver
will be broken without this patch.
v6:
1.Resend after rebase the latest next branch.
2.Remove below No.13~No.15 patches of v5 because they were mergered.
ARM: dts: imx6ul: add dma support on ecspi
ARM: dts: imx6sll: correct sdma compatible
arm64: defconfig: Enable SDMA on i.mx8mq/8mm
3.Revert "dmaengine: imx-sdma: fix context cache" since
'context_loaded' removed.
v7:
1.Put the last patch 13/13 'Revert "dmaengine: imx-sdma: fix context
cache"' to the ahead of 03/13 'Revert "dmaengine: imx-sdma: refine
to load context only once" so that no building waring during comes out
during bisect.
2.Address Sascha's comments, including eliminating any i.mx6sx in this
series, adding new 'is_imx6ul_ecspi()' instead imx in imx51 and taking
care SMC bit for PIO.
3.Add back missing 'Reviewed-by' tag on 08/15(v5):09/13(v7)
'spi: imx: add new i.mx6ul compatible name in binding doc'
v8:
1.remove 0003-Revert-dmaengine-imx-sdma-fix-context-cache.patch and merge
it into 04/13 of v7
2.add 0005-spi-imx-fallback-to-PIO-if-dma-setup-failure.patch for no any
ecspi function broken even if sdma firmware not updated.
3.merge 'tx.dst_maxburst' changes in the two continous patches into one
patch to avoid confusion.
4.fix typo 'duplicated'.
v9:
1. add "spi: imx: add dma_sync_sg_for_device after fallback from dma"
to fix the potential issue brought by commit bcd8e7761ec9("spi: imx:
fallback to PIO if dma setup failure") which is the only one patch
of v8 merged. Thanks Matthias for reporting:
https://lore.kernel.org/linux-arm-kernel/5d246dd81607bb6e5cb9af86ad4e53f7a7a99c50.camel@ew.tq-group.com/
Robin Gong (14):
Revert "ARM: dts: imx6q: Use correct SDMA script for SPI5 core"
Revert "ARM: dts: imx6: Use correct SDMA script for SPI cores"
Revert "dmaengine: imx-sdma: refine to load context only once"
dmaengine: imx-sdma: remove duplicated sdma_load_context
spi: imx: fallback to PIO if dma setup failure
spi: imx: add dma_sync_sg_for_device after fallback from dma
dmaengine: imx-sdma: add mcu_2_ecspi script
spi: imx: fix ERR009165
spi: imx: remove ERR009165 workaround on i.mx6ul
spi: imx: add new i.mx6ul compatible name in binding doc
dmaengine: imx-sdma: remove ERR009165 on i.mx6ul
dma: imx-sdma: add i.mx6ul compatible name
dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
dmaengine: imx-sdma: add uart rom script
.../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
.../devicetree/bindings/spi/fsl-imx-cspi.txt | 1 +
arch/arm/boot/dts/imx6q.dtsi | 2 +-
arch/arm/boot/dts/imx6qdl.dtsi | 8 +-
drivers/dma/imx-sdma.c | 67 ++++++++-----
drivers/spi/spi-imx.c | 104 ++++++++++++++++++---
include/linux/platform_data/dma-imx-sdma.h | 8 +-
7 files changed, 147 insertions(+), 44 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v9 01/14] Revert "ARM: dts: imx6q: Use correct SDMA script for SPI5 core"
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
There are two ways for SDMA accessing SPBA devices: one is SDMA->AIPS
->SPBA(masterA port), another is SDMA->SPBA(masterC port). Please refer
to the 'Figure 58-1. i.MX 6Dual/6Quad SPBA connectivity' of i.mx6DQ
Reference Manual. SDMA provide the corresponding app_2_mcu/mcu_2_app and
shp_2_mcu/mcu_2_shp script for such two options. So both AIPS and SPBA
scripts should keep the same behaviour, the issue only caught in AIPS
script sounds not solide.
The issue is more likely as the ecspi errata
ERR009165(http://www.nxp.com/docs/en/errata/IMX6DQCE.pdf):
eCSPI: TXFIFO empty flag glitch can cause the current FIFO transfer to
be sent twice
So revert commit 'df07101e1c4a' firstly.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
arch/arm/boot/dts/imx6q.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 78a4d64..afdd9eb 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -177,7 +177,7 @@
clocks = <&clks IMX6Q_CLK_ECSPI5>,
<&clks IMX6Q_CLK_ECSPI5>;
clock-names = "ipg", "per";
- dmas = <&sdma 11 8 1>, <&sdma 12 8 2>;
+ dmas = <&sdma 11 7 1>, <&sdma 12 7 2>;
dma-names = "rx", "tx";
status = "disabled";
};
--
2.7.4
^ permalink raw reply related
* [PATCH v9 02/14] Revert "ARM: dts: imx6: Use correct SDMA script for SPI cores"
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
There are two ways for SDMA accessing SPBA devices: one is SDMA->AIPS
->SPBA(masterA port), another is SDMA->SPBA(masterC port). Please refer
to the 'Figure 58-1. i.MX 6Dual/6Quad SPBA connectivity' of i.mx6DQ
Reference Manual. SDMA provide the corresponding app_2_mcu/mcu_2_app and
shp_2_mcu/mcu_2_shp script for such two options. So both AIPS and SPBA
scripts should keep the same behaviour, the issue only caught in AIPS
script sounds not solide.
The issue is more likely as the ecspi errata
ERR009165(http://www.nxp.com/docs/en/errata/IMX6DQCE.pdf):
eCSPI: TXFIFO empty flag glitch can cause the current FIFO transfer to
be sent twice
So revert commit 'dd4b487b32a3' firstly.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
arch/arm/boot/dts/imx6qdl.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 98da446..4a50331 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -337,7 +337,7 @@
clocks = <&clks IMX6QDL_CLK_ECSPI1>,
<&clks IMX6QDL_CLK_ECSPI1>;
clock-names = "ipg", "per";
- dmas = <&sdma 3 8 1>, <&sdma 4 8 2>;
+ dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
dma-names = "rx", "tx";
status = "disabled";
};
@@ -351,7 +351,7 @@
clocks = <&clks IMX6QDL_CLK_ECSPI2>,
<&clks IMX6QDL_CLK_ECSPI2>;
clock-names = "ipg", "per";
- dmas = <&sdma 5 8 1>, <&sdma 6 8 2>;
+ dmas = <&sdma 5 7 1>, <&sdma 6 7 2>;
dma-names = "rx", "tx";
status = "disabled";
};
@@ -365,7 +365,7 @@
clocks = <&clks IMX6QDL_CLK_ECSPI3>,
<&clks IMX6QDL_CLK_ECSPI3>;
clock-names = "ipg", "per";
- dmas = <&sdma 7 8 1>, <&sdma 8 8 2>;
+ dmas = <&sdma 7 7 1>, <&sdma 8 7 2>;
dma-names = "rx", "tx";
status = "disabled";
};
@@ -379,7 +379,7 @@
clocks = <&clks IMX6QDL_CLK_ECSPI4>,
<&clks IMX6QDL_CLK_ECSPI4>;
clock-names = "ipg", "per";
- dmas = <&sdma 9 8 1>, <&sdma 10 8 2>;
+ dmas = <&sdma 9 7 1>, <&sdma 10 7 2>;
dma-names = "rx", "tx";
status = "disabled";
};
--
2.7.4
^ permalink raw reply related
* [PATCH v9 03/14] Revert "dmaengine: imx-sdma: refine to load context only once"
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
This reverts commit ad0d92d7ba6aecbe2705907c38ff8d8be4da1e9c, because
in spi-imx case, burst length may be changed dynamically.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/dma/imx-sdma.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 9177403..b1f61eb 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -377,7 +377,6 @@ struct sdma_channel {
unsigned long watermark_level;
u32 shp_addr, per_addr;
enum dma_status status;
- bool context_loaded;
struct imx_dma_data data;
struct work_struct terminate_worker;
};
@@ -984,9 +983,6 @@ static int sdma_load_context(struct sdma_channel *sdmac)
int ret;
unsigned long flags;
- if (sdmac->context_loaded)
- return 0;
-
if (sdmac->direction == DMA_DEV_TO_MEM)
load_address = sdmac->pc_from_device;
else if (sdmac->direction == DMA_DEV_TO_DEV)
@@ -1029,8 +1025,6 @@ static int sdma_load_context(struct sdma_channel *sdmac)
spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
- sdmac->context_loaded = true;
-
return ret;
}
@@ -1069,7 +1063,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
vchan_get_all_descriptors(&sdmac->vc, &head);
spin_unlock_irqrestore(&sdmac->vc.lock, flags);
vchan_dma_desc_free_list(&sdmac->vc, &head);
- sdmac->context_loaded = false;
}
static int sdma_terminate_all(struct dma_chan *chan)
@@ -1338,7 +1331,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
sdmac->event_id0 = 0;
sdmac->event_id1 = 0;
- sdmac->context_loaded = false;
sdma_set_channel_priority(sdmac, 0);
--
2.7.4
^ permalink raw reply related
* [PATCH v9 04/14] dmaengine: imx-sdma: remove duplicated sdma_load_context
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
Since sdma_transfer_init() will do sdma_load_context before any
sdma transfer, no need once more in sdma_config_channel().
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
---
drivers/dma/imx-sdma.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b1f61eb..4440ddb 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1137,7 +1137,6 @@ static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
static int sdma_config_channel(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
- int ret;
sdma_disable_channel(chan);
@@ -1177,9 +1176,7 @@ static int sdma_config_channel(struct dma_chan *chan)
sdmac->watermark_level = 0; /* FIXME: M3_BASE_ADDRESS */
}
- ret = sdma_load_context(sdmac);
-
- return ret;
+ return 0;
}
static int sdma_set_channel_priority(struct sdma_channel *sdmac,
--
2.7.4
^ permalink raw reply related
* [PATCH v9 05/14] spi: imx: fallback to PIO if dma setup failure
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
Fallback to PIO in case dma setup failed. For example, sdma firmware not
updated but ERR009165 workaroud added in kernel.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/spi/spi-imx.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index f4f28a4..b7a85e3 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -71,6 +71,7 @@ struct spi_imx_devtype_data {
void (*reset)(struct spi_imx_data *);
void (*setup_wml)(struct spi_imx_data *);
void (*disable)(struct spi_imx_data *);
+ void (*disable_dma)(struct spi_imx_data *);
bool has_dmamode;
bool has_slavemode;
unsigned int fifo_size;
@@ -485,6 +486,11 @@ static void mx51_ecspi_trigger(struct spi_imx_data *spi_imx)
writel(reg, spi_imx->base + MX51_ECSPI_CTRL);
}
+static void mx51_disable_dma(struct spi_imx_data *spi_imx)
+{
+ writel(0, spi_imx->base + MX51_ECSPI_DMA);
+}
+
static void mx51_ecspi_disable(struct spi_imx_data *spi_imx)
{
u32 ctrl;
@@ -987,6 +993,7 @@ static struct spi_imx_devtype_data imx51_ecspi_devtype_data = {
.rx_available = mx51_ecspi_rx_available,
.reset = mx51_ecspi_reset,
.setup_wml = mx51_setup_wml,
+ .disable_dma = mx51_disable_dma,
.fifo_size = 64,
.has_dmamode = true,
.dynamic_burst = true,
@@ -1001,6 +1008,7 @@ static struct spi_imx_devtype_data imx53_ecspi_devtype_data = {
.prepare_transfer = mx51_ecspi_prepare_transfer,
.trigger = mx51_ecspi_trigger,
.rx_available = mx51_ecspi_rx_available,
+ .disable_dma = mx51_disable_dma,
.reset = mx51_ecspi_reset,
.fifo_size = 64,
.has_dmamode = true,
@@ -1385,6 +1393,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!desc_tx) {
dmaengine_terminate_all(master->dma_tx);
+ dmaengine_terminate_all(master->dma_rx);
return -EINVAL;
}
@@ -1498,6 +1507,7 @@ static int spi_imx_transfer(struct spi_device *spi,
struct spi_transfer *transfer)
{
struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master);
+ int ret;
/* flush rxfifo before transfer */
while (spi_imx->devtype_data->rx_available(spi_imx))
@@ -1506,10 +1516,23 @@ static int spi_imx_transfer(struct spi_device *spi,
if (spi_imx->slave_mode)
return spi_imx_pio_transfer_slave(spi, transfer);
- if (spi_imx->usedma)
- return spi_imx_dma_transfer(spi_imx, transfer);
- else
- return spi_imx_pio_transfer(spi, transfer);
+ /*
+ * fallback PIO mode if dma setup error happen, for example sdma
+ * firmware may not be updated as ERR009165 required.
+ */
+ if (spi_imx->usedma) {
+ ret = spi_imx_dma_transfer(spi_imx, transfer);
+ if (ret != -EINVAL)
+ return ret;
+
+ spi_imx->devtype_data->disable_dma(spi_imx);
+
+ spi_imx->usedma = false;
+ spi_imx->dynamic_burst = spi_imx->devtype_data->dynamic_burst;
+ dev_dbg(&spi->dev, "Fallback to PIO mode\n");
+ }
+
+ return spi_imx_pio_transfer(spi, transfer);
}
static int spi_imx_setup(struct spi_device *spi)
--
2.7.4
^ permalink raw reply related
* [PATCH v9 06/14] spi: imx: add dma_sync_sg_for_device after fallback from dma
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
In case dma transfer failed and fallback to pio, tx_buf/rx_buf need to be
taken care cache since they have already been maintained by spi.c
Fixes: bcd8e7761ec9("spi: imx: fallback to PIO if dma setup failure")
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Reported-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Link: https://lore.kernel.org/linux-arm-kernel/5d246dd81607bb6e5cb9af86ad4e53f7a7a99c50.camel@ew.tq-group.com/
---
drivers/spi/spi-imx.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index b7a85e3..c51cd3a 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1456,6 +1456,13 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
return -ETIMEDOUT;
}
+ if (transfer->rx_sg.sgl) {
+ struct device *rx_dev = spi->controller->dma_rx->device->dev;
+
+ dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
+ transfer->rx_sg.nents, DMA_TO_DEVICE);
+ }
+
return transfer->len;
}
@@ -1521,10 +1528,15 @@ static int spi_imx_transfer(struct spi_device *spi,
* firmware may not be updated as ERR009165 required.
*/
if (spi_imx->usedma) {
+ struct device *tx_dev = spi->controller->dma_tx->device->dev;
+
ret = spi_imx_dma_transfer(spi_imx, transfer);
if (ret != -EINVAL)
return ret;
+ dma_sync_sg_for_cpu(tx_dev, transfer->tx_sg.sgl,
+ transfer->tx_sg.nents, DMA_FROM_DEVICE);
+
spi_imx->devtype_data->disable_dma(spi_imx);
spi_imx->usedma = false;
--
2.7.4
^ permalink raw reply related
* [PATCH v9 07/14] dmaengine: imx-sdma: add mcu_2_ecspi script
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
Add mcu_2_ecspi script to fix ecspi errata ERR009165.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
---
drivers/dma/imx-sdma.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 4440ddb..db4132f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -920,6 +920,9 @@ static void sdma_get_pc(struct sdma_channel *sdmac,
emi_2_per = sdma->script_addrs->mcu_2_ata_addr;
break;
case IMX_DMATYPE_CSPI:
+ per_2_emi = sdma->script_addrs->app_2_mcu_addr;
+ emi_2_per = sdma->script_addrs->mcu_2_ecspi_addr;
+ break;
case IMX_DMATYPE_EXT:
case IMX_DMATYPE_SSI:
case IMX_DMATYPE_SAI:
--
2.7.4
^ permalink raw reply related
* [PATCH v9 08/14] spi: imx: fix ERR009165
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
Change to XCH mode even in dma mode, please refer to the below
errata:
https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-imx.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index c51cd3a..f82d63f 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -591,8 +591,8 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk);
spi_imx->spi_bus_clk = clk;
- if (spi_imx->usedma)
- ctrl |= MX51_ECSPI_CTRL_SMC;
+ /* ERR009165: work in XHC mode as PIO */
+ ctrl &= ~MX51_ECSPI_CTRL_SMC;
writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
@@ -623,7 +623,7 @@ static void mx51_setup_wml(struct spi_imx_data *spi_imx)
* and enable DMA request.
*/
writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) |
- MX51_ECSPI_DMA_TX_WML(spi_imx->wml) |
+ MX51_ECSPI_DMA_TX_WML(0) |
MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) |
MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN |
MX51_ECSPI_DMA_RXTDEN, spi_imx->base + MX51_ECSPI_DMA);
@@ -1273,10 +1273,6 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx,
{
int ret;
- /* use pio mode for i.mx6dl chip TKT238285 */
- if (of_machine_is_compatible("fsl,imx6dl"))
- return 0;
-
spi_imx->wml = spi_imx->devtype_data->fifo_size / 2;
/* Prepare for TX DMA: */
--
2.7.4
^ permalink raw reply related
* [PATCH v9 09/14] spi: imx: remove ERR009165 workaround on i.mx6ul
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
ERR009165 fixed on i.mx6ul/6ull/6sll. All other i.mx6/7 and
i.mx8m/8mm still need this errata. Please refer to nxp official
errata document from https://www.nxp.com/ .
For removing workaround on those chips. Add new i.mx6ul type.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-imx.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 54 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index f82d63f..9067c95 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -57,6 +57,7 @@ enum spi_imx_devtype {
IMX35_CSPI, /* CSPI on all i.mx except above */
IMX51_ECSPI, /* ECSPI on i.mx51 */
IMX53_ECSPI, /* ECSPI on i.mx53 and later */
+ IMX6UL_ECSPI, /* ERR009165 fix from i.mx6ul */
};
struct spi_imx_data;
@@ -76,6 +77,11 @@ struct spi_imx_devtype_data {
bool has_slavemode;
unsigned int fifo_size;
bool dynamic_burst;
+ /*
+ * ERR009165 fixed or not:
+ * https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf
+ */
+ bool tx_glitch_fixed;
enum spi_imx_devtype devtype;
};
@@ -132,6 +138,11 @@ static inline int is_imx51_ecspi(struct spi_imx_data *d)
return d->devtype_data->devtype == IMX51_ECSPI;
}
+static inline int is_imx6ul_ecspi(struct spi_imx_data *d)
+{
+ return d->devtype_data->devtype == IMX6UL_ECSPI;
+}
+
static inline int is_imx53_ecspi(struct spi_imx_data *d)
{
return d->devtype_data->devtype == IMX53_ECSPI;
@@ -591,8 +602,14 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk);
spi_imx->spi_bus_clk = clk;
- /* ERR009165: work in XHC mode as PIO */
- ctrl &= ~MX51_ECSPI_CTRL_SMC;
+ /*
+ * ERR009165: work in XHC mode instead of SMC as PIO on the chips
+ * before i.mx6ul.
+ */
+ if (spi_imx->usedma && spi_imx->devtype_data->tx_glitch_fixed)
+ ctrl |= MX51_ECSPI_CTRL_SMC;
+ else
+ ctrl &= ~MX51_ECSPI_CTRL_SMC;
writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
@@ -618,12 +635,16 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
static void mx51_setup_wml(struct spi_imx_data *spi_imx)
{
+ u32 tx_wml = 0;
+
+ if (spi_imx->devtype_data->tx_glitch_fixed)
+ tx_wml = spi_imx->wml;
/*
* Configure the DMA register: setup the watermark
* and enable DMA request.
*/
writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) |
- MX51_ECSPI_DMA_TX_WML(0) |
+ MX51_ECSPI_DMA_TX_WML(tx_wml) |
MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) |
MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN |
MX51_ECSPI_DMA_RXTDEN, spi_imx->base + MX51_ECSPI_DMA);
@@ -1017,6 +1038,23 @@ static struct spi_imx_devtype_data imx53_ecspi_devtype_data = {
.devtype = IMX53_ECSPI,
};
+static struct spi_imx_devtype_data imx6ul_ecspi_devtype_data = {
+ .intctrl = mx51_ecspi_intctrl,
+ .prepare_message = mx51_ecspi_prepare_message,
+ .prepare_transfer = mx51_ecspi_prepare_transfer,
+ .trigger = mx51_ecspi_trigger,
+ .rx_available = mx51_ecspi_rx_available,
+ .reset = mx51_ecspi_reset,
+ .setup_wml = mx51_setup_wml,
+ .fifo_size = 64,
+ .has_dmamode = true,
+ .dynamic_burst = true,
+ .has_slavemode = true,
+ .tx_glitch_fixed = true,
+ .disable = mx51_ecspi_disable,
+ .devtype = IMX6UL_ECSPI,
+};
+
static const struct platform_device_id spi_imx_devtype[] = {
{
.name = "imx1-cspi",
@@ -1040,6 +1078,9 @@ static const struct platform_device_id spi_imx_devtype[] = {
.name = "imx53-ecspi",
.driver_data = (kernel_ulong_t) &imx53_ecspi_devtype_data,
}, {
+ .name = "imx6ul-ecspi",
+ .driver_data = (kernel_ulong_t) &imx6ul_ecspi_devtype_data,
+ }, {
/* sentinel */
}
};
@@ -1052,6 +1093,7 @@ static const struct of_device_id spi_imx_dt_ids[] = {
{ .compatible = "fsl,imx35-cspi", .data = &imx35_cspi_devtype_data, },
{ .compatible = "fsl,imx51-ecspi", .data = &imx51_ecspi_devtype_data, },
{ .compatible = "fsl,imx53-ecspi", .data = &imx53_ecspi_devtype_data, },
+ { .compatible = "fsl,imx6ul-ecspi", .data = &imx6ul_ecspi_devtype_data, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, spi_imx_dt_ids);
@@ -1179,7 +1221,14 @@ static int spi_imx_dma_configure(struct spi_master *master)
tx.direction = DMA_MEM_TO_DEV;
tx.dst_addr = spi_imx->base_phys + MXC_CSPITXDATA;
tx.dst_addr_width = buswidth;
- tx.dst_maxburst = spi_imx->wml;
+ /*
+ * For ERR009165 with TX_THRESHOLD=0 could enlarge burst size to fifo
+ * size to speed up fifo filling as possible.
+ */
+ if (spi_imx->devtype_data->tx_glitch_fixed)
+ tx.dst_maxburst = spi_imx->wml;
+ else
+ tx.dst_maxburst = spi_imx->devtype_data->fifo_size;
ret = dmaengine_slave_config(master->dma_tx, &tx);
if (ret) {
dev_err(spi_imx->dev, "TX dma configuration failed with %d\n", ret);
@@ -1690,7 +1739,7 @@ static int spi_imx_probe(struct platform_device *pdev)
spi_imx->bitbang.master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH \
| SPI_NO_CS;
if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx) ||
- is_imx53_ecspi(spi_imx))
+ is_imx53_ecspi(spi_imx) || is_imx6ul_ecspi(spi_imx))
spi_imx->bitbang.master->mode_bits |= SPI_LOOP | SPI_READY;
spi_imx->spi_drctl = spi_drctl;
--
2.7.4
^ permalink raw reply related
* [PATCH v9 10/14] spi: imx: add new i.mx6ul compatible name in binding doc
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
ERR009165 fixed from i.mx6ul, add its compatible name in binding doc.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
index 33bc58f..0a529ba 100644
--- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
@@ -10,6 +10,7 @@ Required properties:
- "fsl,imx35-cspi" for SPI compatible with the one integrated on i.MX35
- "fsl,imx51-ecspi" for SPI compatible with the one integrated on i.MX51
- "fsl,imx53-ecspi" for SPI compatible with the one integrated on i.MX53 and later Soc
+ - "fsl,imx6ul-ecspi" for SPI compatible with the one integrated on i.MX6UL and later Soc
- "fsl,imx8mq-ecspi" for SPI compatible with the one integrated on i.MX8MQ
- "fsl,imx8mm-ecspi" for SPI compatible with the one integrated on i.MX8MM
- "fsl,imx8mn-ecspi" for SPI compatible with the one integrated on i.MX8MN
--
2.7.4
^ permalink raw reply related
* [PATCH v9 11/14] dmaengine: imx-sdma: remove ERR009165 on i.mx6ul
From: Robin Gong @ 2020-06-05 21:32 UTC (permalink / raw)
To: mark.rutland, broonie, robh+dt, catalin.marinas, vkoul,
will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, matthias.schiffer
Cc: linux-spi, linux-kernel, devicetree, linux-arm-kernel, kernel,
linux-imx, dmaengine
In-Reply-To: <1591392755-19136-1-git-send-email-yibin.gong@nxp.com>
ECSPI issue fixed from i.mx6ul at hardware level, no need
ERR009165 anymore on those chips such as i.mx8mq.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
---
drivers/dma/imx-sdma.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index db4132f..be86ae8 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -419,6 +419,13 @@ struct sdma_driver_data {
int num_events;
struct sdma_script_start_addrs *script_addrs;
bool check_ratio;
+ /*
+ * ecspi ERR009165 fixed should be done in sdma script
+ * and it has been fixed in soc from i.mx6ul.
+ * please get more information from the below link:
+ * https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf
+ */
+ bool ecspi_fixed;
};
struct sdma_engine {
@@ -539,6 +546,13 @@ static struct sdma_driver_data sdma_imx6q = {
.script_addrs = &sdma_script_imx6q,
};
+static struct sdma_driver_data sdma_imx6ul = {
+ .chnenbl0 = SDMA_CHNENBL0_IMX35,
+ .num_events = 48,
+ .script_addrs = &sdma_script_imx6q,
+ .ecspi_fixed = true,
+};
+
static struct sdma_script_start_addrs sdma_script_imx7d = {
.ap_2_ap_addr = 644,
.uart_2_mcu_addr = 819,
@@ -587,6 +601,9 @@ static const struct platform_device_id sdma_devtypes[] = {
.name = "imx7d-sdma",
.driver_data = (unsigned long)&sdma_imx7d,
}, {
+ .name = "imx6ul-sdma",
+ .driver_data = (unsigned long)&sdma_imx6ul,
+ }, {
.name = "imx8mq-sdma",
.driver_data = (unsigned long)&sdma_imx8mq,
}, {
@@ -603,6 +620,7 @@ static const struct of_device_id sdma_dt_ids[] = {
{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
{ .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
{ .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
+ { .compatible = "fsl,imx6ul-sdma", .data = &sdma_imx6ul, },
{ .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx8mq, },
{ /* sentinel */ }
};
@@ -1169,8 +1187,17 @@ static int sdma_config_channel(struct dma_chan *chan)
if (sdmac->peripheral_type == IMX_DMATYPE_ASRC_SP ||
sdmac->peripheral_type == IMX_DMATYPE_ASRC)
sdma_set_watermarklevel_for_p2p(sdmac);
- } else
+ } else {
+ /*
+ * ERR009165 fixed from i.mx6ul, no errata need,
+ * set bit31 to let sdma script skip the errata.
+ */
+ if (sdmac->peripheral_type == IMX_DMATYPE_CSPI &&
+ sdmac->direction == DMA_MEM_TO_DEV &&
+ sdmac->sdma->drvdata->ecspi_fixed)
+ __set_bit(31, &sdmac->watermark_level);
__set_bit(sdmac->event_id0, sdmac->event_mask);
+ }
/* Address */
sdmac->shp_addr = sdmac->per_address;
--
2.7.4
^ 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