* Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support
From: Nicolas Saenz Julienne @ 2020-06-05 10:58 UTC (permalink / raw)
To: Florian Fainelli, linux-kernel
Cc: 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, Mark Brown,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl
In-Reply-To: <f728f55fe6266718b5041b6f3b1864a673991129.camel@suse.de>
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
On Fri, 2020-06-05 at 10:46 +0200, Nicolas Saenz Julienne wrote:
> Hi Florian,
> Thanks for taking over this!
>
> On Thu, 2020-06-04 at 14:28 -0700, Florian Fainelli wrote:
> > The 4 SPI controller instances added in BCM2711 and BCM7211 SoCs (SPI3,
> > SPI4, SPI5 and SPI6) share the same interrupt line with SPI0.
>
> I think this isn't 100% correct. SPI0 has its own interrupt, but SPI[3-6]
> share
> the same interrupt.
I'm wrong here, I missed this in bcm2711.dtsi:
&spi {
interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
};
Sorry for the noise.
Regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
bingbu.cao
Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200605105412.18813-1-dongchun.zhu@mediatek.com>
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)
+{
+ 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;
+
+ 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");
--
2.9.2
^ permalink raw reply related
* [V7, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
bingbu.cao
Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200605105412.18813-1-dongchun.zhu@mediatek.com>
Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
coil actuator.
Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
.../bindings/media/i2c/dongwoon,dw9768.yaml | 100 +++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 107 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
new file mode 100644
index 0000000..cb96e95
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) 2020 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
+
+maintainers:
+ - Dongchun Zhu <dongchun.zhu@mediatek.com>
+
+description: |-
+ The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
+ with 100 mA output current sink capability. VCM current is controlled with
+ a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
+ serial interface that operates at clock rates up to 1MHz. This chip
+ integrates Advanced Actuator Control (AAC) technology and is intended for
+ driving voice coil lenses in camera modules.
+
+properties:
+ compatible:
+ enum:
+ - dongwoon,dw9768 # for DW9768 VCM
+ - giantec,gt9769 # for GT9769 VCM
+
+ reg:
+ maxItems: 1
+
+ vin-supply:
+ description:
+ Definition of the regulator used as Digital I/O voltage supply.
+
+ vdd-supply:
+ description:
+ Definition of the regulator used as Digital core voltage supply.
+
+ dongwoon,aac-mode:
+ description:
+ Indication of AAC mode select.
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/uint32"
+ - enum:
+ - 1 # AAC2 mode(operation time# 0.48 x Tvib)
+ - 2 # AAC3 mode(operation time# 0.70 x Tvib)
+ - 3 # AAC4 mode(operation time# 0.75 x Tvib)
+ - 5 # AAC8 mode(operation time# 1.13 x Tvib)
+ default: 2
+
+ dongwoon,aac-timing:
+ description:
+ Number of AAC Timing count that controlled by one 6-bit period of
+ vibration register AACT[5:0], the unit of which is 100 us.
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/uint32"
+ - default: 0x20
+ minimum: 0x00
+ maximum: 0x3f
+
+ dongwoon,clock-presc:
+ description:
+ Indication of VCM internal clock dividing rate select, as one multiple
+ factor to calculate VCM ring periodic time Tvib.
+ allOf:
+ - $ref: "/schemas/types.yaml#/definitions/uint32"
+ - enum:
+ - 0 # Dividing Rate - 2
+ - 1 # Dividing Rate - 1
+ - 2 # Dividing Rate - 1/2
+ - 3 # Dividing Rate - 1/4
+ - 4 # Dividing Rate - 8
+ - 5 # Dividing Rate - 4
+ default: 1
+
+required:
+ - compatible
+ - reg
+ - vin-supply
+ - vdd-supply
+
+additionalProperties: false
+
+examples:
+ - |
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dw9768: camera-lens@c {
+ compatible = "dongwoon,dw9768";
+ reg = <0x0c>;
+
+ vin-supply = <&mt6358_vcamio_reg>;
+ vdd-supply = <&mt6358_vcama2_reg>;
+ dongwoon,aac-timing = <0x39>;
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e64e5db..8d72c41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5151,6 +5151,13 @@ T: git git://linuxtv.org/media_tree.git
F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
F: drivers/media/i2c/dw9714.c
+DONGWOON DW9768 LENS VOICE COIL DRIVER
+M: Dongchun Zhu <dongchun.zhu@mediatek.com>
+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
+
DONGWOON DW9807 LENS VOICE COIL DRIVER
M: Sakari Ailus <sakari.ailus@linux.intel.com>
L: linux-media@vger.kernel.org
--
2.9.2
^ permalink raw reply related
* [V7, 0/2] media: i2c: Add support for DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 10:54 UTC (permalink / raw)
To: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, sakari.ailus, drinkcat, tfiga, matthias.bgg,
bingbu.cao
Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
Hello,
This series adds DT bindings and V4L2 sub-device driver for Dongwoon's DW9768,
which is a 10-bit DAC with 100mA output current sink capability.
The driver controls the position with 10-bit DAC data D[9:0] and seperates
two 8-bit registers to control the VCM position as belows.
DAC_MSB: D[9:8](ADDR: 0x03):
+---+---+---+---+---+---+---+---+
|---|---|---|---|---|---|D09|D08|
+---+---+---+---+---+---+---+---+
DAC_LSB: D[7:0](ADDR: 0x04):
+---+---+---+---+---+---+---+---+
|D07|D06|D05|D04|D03|D02|D01|D00|
+---+---+---+---+---+---+---+---+
This driver supports:
- set DW9768 to standby mode once suspend and turn it back to active if resume
- set the desired focus via V4L2_CID_FOCUS_ABSOLUTE ctrl
Previous versions of this patch-set can be found here:
v6: https://lore.kernel.org/linux-media/20200518132731.20855-1-dongchun.zhu@mediatek.com/
v5: https://lore.kernel.org/linux-media/20200502161727.30463-1-dongchun.zhu@mediatek.com/
v4: https://lore.kernel.org/linux-media/20200330123634.363-1-dongchun.zhu@mediatek.com/
v3: https://lore.kernel.org/linux-media/20200228155958.20657-1-dongchun.zhu@mediatek.com/
v2: https://lore.kernel.org/linux-media/20190905072142.14606-1-dongchun.zhu@mediatek.com/
v1: https://lore.kernel.org/linux-media/20190708100641.2702-1-dongchun.zhu@mediatek.com/
Mainly changes of v7 are addressing comments from Rob, Sakari, Tomasz.
Compared to v6:
- Refine DT bindings
- Use i2c_smbus_read_byte_data() directly to replace of dw9768_read_smbus()
- Calculate operation time based on the configured board-specific DT settings
- Abstract async register error handling case
- Fix other review comments in v6
Please review.
Thanks.
Dongchun Zhu (2):
media: dt-bindings: media: i2c: Document DW9768 bindings
media: i2c: dw9768: Add DW9768 VCM driver
.../bindings/media/i2c/dongwoon,dw9768.yaml | 100 ++++
MAINTAINERS | 8 +
drivers/media/i2c/Kconfig | 13 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++
5 files changed, 688 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
create mode 100644 drivers/media/i2c/dw9768.c
--
2.9.2
^ permalink raw reply
* Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-05 10:52 UTC (permalink / raw)
To: Nicolas Saenz Julienne
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
In-Reply-To: <f728f55fe6266718b5041b6f3b1864a673991129.camel@suse.de>
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
On Fri, Jun 05, 2020 at 10:46:57AM +0200, Nicolas Saenz Julienne wrote:
> > -static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
> > +static inline irqreturn_t bcm2835_spi_interrupt_common(struct spi_controller
> > *ctlr,
> > + u32 cs)
> Keep in mind the new 100 character limit.
That's more about stopping people doing awful contortions to shut
checkpatch up than saying that it's a particularly good idea to lengthen
lines.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support
From: Lukas Wunner @ 2020-06-05 10:51 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Mark Brown, Rob Herring, Nicolas Saenz Julienne,
Ray Jui, Scott Branden,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
open list:SPI SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl
In-Reply-To: <20200604212819.715-1-f.fainelli@gmail.com>
On Thu, Jun 04, 2020 at 02:28:19PM -0700, Florian Fainelli wrote:
> The 4 SPI controller instances added in BCM2711 and BCM7211 SoCs (SPI3,
> SPI4, SPI5 and SPI6) share the same interrupt line with SPI0.
>
> 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.
>
> To support that requirement the common interrupt handling code between
> the shared and non-shared interrupt paths is split into a
> bcm2835_spi_interrupt_common() and both bcm2835_spi_interrupt() as well
> as bcm2835_spi_shared_interrupt() make use of it.
>
> During probe, we determine if there is at least another instance of this
> SPI controller, and if there is, then we install a shared interrupt
> handler.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
^ permalink raw reply
* Re: [PATCH v4 04/11] watchdog: add support for sl28cpld watchdog
From: Andy Shevchenko @ 2020-06-05 10:50 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,
Andy Shevchenko
In-Reply-To: <8f042c2442852c29519c381833f3d289@walle.cc>
On Fri, Jun 5, 2020 at 1:24 PM Michael Walle <michael@walle.cc> wrote:
> Am 2020-06-05 10:14, schrieb Andy Shevchenko:
> > On Fri, Jun 5, 2020 at 12:14 AM Michael Walle <michael@walle.cc> wrote:
...
> >> +static bool nowayout = WATCHDOG_NOWAYOUT;
> >> +module_param(nowayout, bool, 0);
> >> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started
> >> (default="
> >> + __MODULE_STRING(WATCHDOG_NOWAYOUT)
> >> ")");
> >> +
> >> +static int timeout;
> >> +module_param(timeout, int, 0);
> >> +MODULE_PARM_DESC(timeout, "Initial watchdog timeout in seconds");
> >
> > Guenter ACKed this, but I'm wondering why we still need module
> > parameters...
>
> How would a user change the nowayout or the timeout? For the latter
> there is
> a device tree entry, but thats not easy changable by the user.
Yes, it's more question to VIm and Guenter than to you.
...
> >> + if (status & WDT_CTRL_EN) {
> >> + sl28cpld_wdt_start(wdd);
> >
> >> + set_bit(WDOG_HW_RUNNING, &wdd->status);
> >
> > Do you need atomic op here? Why?
>
> I'd say consistency, all watchdog drivers do it like that. I just
> had a look at where this is used, but it looks like access from
> userspace is protected by a lock.
Okay then.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller
From: Mark Brown @ 2020-06-05 10:50 UTC (permalink / raw)
To: Lee Jones
Cc: Michael Walle, linux-gpio, devicetree, linux-kernel, linux-hwmon,
linux-pwm, linux-watchdog, linux-arm-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Greg Kroah-Hartman, Andy Shevchenko
In-Reply-To: <20200605065709.GD3714@dell>
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
On Fri, Jun 05, 2020 at 07:57:09AM +0100, Lee Jones wrote:
> On Thu, 04 Jun 2020, Michael Walle wrote:
> > + sl28cpld->regmap = devm_regmap_init_i2c(i2c, &sl28cpld_regmap_config);
> > + if (IS_ERR(sl28cpld->regmap))
> > + return PTR_ERR(sl28cpld->regmap);
> This is now a shared memory allocator and not an MFD at all.
> I'm clamping down on these type of drivers!
> Please find a better way to accomplish this.
What is the concern with this? Looking at the patch I'm guessing the
concern would be that the driver isn't instantiating any MFD children
and instead requiring them to be put in the DT?
> Potentially using "simple-mfd" and "simple-regmap".
> The former already exists and does what you want. The latter doesn't
> yet exist, but could solve your and lots of other contributor's
> issues.
I have no idea what you are thinking of when you say "simple-regmap" so
it is difficult to comment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller
From: Andy Shevchenko @ 2020-06-05 10:48 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,
Andy Shevchenko
In-Reply-To: <8ed988b3e0bc48ea9219d0847c1b1b8e@walle.cc>
On Fri, Jun 5, 2020 at 1:09 PM Michael Walle <michael@walle.cc> wrote:
> Am 2020-06-05 10:01, schrieb Andy Shevchenko:
> > On Fri, Jun 5, 2020 at 12:16 AM Michael Walle <michael@walle.cc> wrote:
...
> >> + bool "Kontron sl28 core driver"
> >> + depends on I2C=y
> >
> > Why not module?
>
> There are users of the interupt lines provided by the interrupt
> controller.
> For example, the gpio-button driver. If this is compiled into the kernel
> (which it is by default in the arm64 defconfig), probing will fail
> because
> the interrupt is not found. Is there a better way for that? I guess the
> same
> is true for the GPIO driver.
And GPIO nicely handles this via deferred probe mechanism. Why it
can't be used here?
So, we really need to have a strong argument to limit module nowadays
to be only builtin.
...
> >> + depends on OF
> >
> > I didn't find an evidence this is needed.
> >> +#include <linux/of_platform.h>
> >
> > No evidence of user of this.
> > I think you meant mod_devicetable.h.
>
> devm_of_platform_populate(), so I need CONFIG_OF, too right?
Ah, this explains header, thanks!
But it doesn't explain depends OF.
So, perhaps,
depends OF || COMPILE_TEST will be more informative, i.e.
tells "okay, this driver can be compiled w/o OF, but won't be functional".
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 3/3] spi: bcm2835: Enable shared interrupt support
From: Mark Brown @ 2020-06-05 10:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Rob Herring, Nicolas Saenz Julienne, Ray Jui,
Scott Branden,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
open list:SPI SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, lukas
In-Reply-To: <21772111-fa1f-7a50-aa92-e44b09cff4eb@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]
On Thu, Jun 04, 2020 at 09:05:46AM -0700, Florian Fainelli wrote:
> On 6/4/2020 5:32 AM, Mark Brown wrote:
> > This feels hacky - it's essentially using the compatible string to set a
> > boolean flag which isn't really about the IP but rather the platform
> > integration. It might cause problems if we do end up having to quirk
> > this version of the IP for some other reason.
> I am not sure why it would be a problem, when you describe a piece of
> hardware with Device Tree, even with the IP block being strictly the
> same, its very integration into a new SoC (with details like shared
> interrupt lines) do warrant a different compatible string. Maybe this is
> more of a philosophical question.
The big concern here is trying to support things going forwards - if it
turns out that any quirks are required by this version of the IP then it
gets very confusing and hard to keep DTs stable if you've already
quirked something that clearly isn't the IP version with the compatible
string. Conversely if we start putting flags into the binding for every
feature that might be changed in a given IP that gets complex as we
can't ever learn new things about an existing IP version without
updating all the DTs which is also bad.
> Instead of counting the number of SPI devices we culd request the
> interrupt first with flags = IRQF_PROBE_SHARED, if this works, good we
> have a single SPI master enabled, if it returns -EBUSY, try again with
> flags = IRQF_SHARED and set-up the bcm2835_spi_sh_interrupt interrupt
> handler to manage the sharing.
Like you said in a followup patch that doesn't work as the first device
to probe will think the interrupt isn't shared. You'd need a callback
to change to shared mode from genirq which feels... inelegant.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 04/11] watchdog: add support for sl28cpld watchdog
From: Michael Walle @ 2020-06-05 10:24 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: <CAHp75VdeD6zDc--R4NPHsiqQerzfNGwUikLN+WHMiZZVsQ8QSA@mail.gmail.com>
Am 2020-06-05 10:14, schrieb Andy Shevchenko:
> On Fri, Jun 5, 2020 at 12:14 AM Michael Walle <michael@walle.cc> wrote:
>>
>> Add support for the watchdog of the sl28cpld board management
>> controller. This is part of a multi-function device driver.
>
> ...
>
>> +#include <linux/of_device.h>
>
> Didn't find a user of this.
I guess mod_devicetable.h then.
>
> ...
>
>> +static bool nowayout = WATCHDOG_NOWAYOUT;
>> +module_param(nowayout, bool, 0);
>> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started
>> (default="
>> + __MODULE_STRING(WATCHDOG_NOWAYOUT)
>> ")");
>> +
>> +static int timeout;
>> +module_param(timeout, int, 0);
>> +MODULE_PARM_DESC(timeout, "Initial watchdog timeout in seconds");
>
> Guenter ACKed this, but I'm wondering why we still need module
> parameters...
How would a user change the nowayout or the timeout? For the latter
there is
a device tree entry, but thats not easy changable by the user.
>
> ...
>
>> + int ret;
>> +
>> + ret = regmap_read(wdt->regmap, wdt->offset + WDT_COUNT, &val);
>> +
>> + return (ret < 0) ? 0 : val;
>
> Besides extra parentheses and questionable ' < 0' part, the following
> would look better I think
>
> ret = ...
> if (ret)
> return 0;
>
> return val;
yes, you're right.
>
> ...
>
>> + int ret;
>> +
>> + ret = regmap_write(wdt->regmap, wdt->offset + WDT_TIMEOUT,
>> timeout);
>> + if (!ret)
>> + wdd->timeout = timeout;
>> +
>> + return ret;
>
> Similar story here:
>
> ret = ...
> if (ret)
> return ret;
>
> wdd->... = ...
> return 0;
>
> ...
ok
>
>> + ret = regmap_read(wdt->regmap, wdt->offset + WDT_CTRL,
>> &status);
>
>> + if (ret < 0)
>
> What ' < 0' means? Do we have some positive return values?
> Ditto for all your code.
probably not, I'll go over all return values and change them.
>> + return ret;
>
> ...
>
>> + if (status & WDT_CTRL_EN) {
>> + sl28cpld_wdt_start(wdd);
>
>> + set_bit(WDOG_HW_RUNNING, &wdd->status);
>
> Do you need atomic op here? Why?
I'd say consistency, all watchdog drivers do it like that. I just
had a look at where this is used, but it looks like access from
userspace is protected by a lock.
>
>> + }
>
> ...
>
>> +static const struct of_device_id sl28cpld_wdt_of_match[] = {
>> + { .compatible = "kontron,sl28cpld-wdt" },
>
>> + {},
>
> No comma.
yeah ;)
--
-michael
^ permalink raw reply
* Re: [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller
From: Michael Walle @ 2020-06-05 10:09 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: <CAHp75Vd-R3yqhq88-whY6vdDhESpzvFCsbi-ygSTjfXfUzOrtg@mail.gmail.com>
Hi Andy,
Am 2020-06-05 10:01, schrieb Andy Shevchenko:
> On Fri, Jun 5, 2020 at 12:16 AM Michael Walle <michael@walle.cc> wrote:
>>
>> Add the core support for the board management controller found on the
>> SMARC-sAL28 board. It consists of the following functions:
>> - watchdog
>> - GPIO controller
>> - PWM controller
>> - fan sensor
>> - interrupt controller
>>
>> At the moment, this controller is used on the Kontron SMARC-sAL28
>> board.
>>
>> Please note that the MFD driver is defined as bool in the Kconfig
>> because the next patch will add interrupt support.
>
> ...
>
>> +config MFD_SL28CPLD
>> + bool "Kontron sl28 core driver"
>> + depends on I2C=y
>
> Why not module?
There are users of the interupt lines provided by the interrupt
controller.
For example, the gpio-button driver. If this is compiled into the kernel
(which it is by default in the arm64 defconfig), probing will fail
because
the interrupt is not found. Is there a better way for that? I guess the
same
is true for the GPIO driver.
>
>> + depends on OF
>
> I didn't find an evidence this is needed.
see below.
>
> No Compile Test?
ok
>> + select REGMAP_I2C
>> + select MFD_CORE
>
> ...
>
>> +#include <linux/of_platform.h>
>
> No evidence of user of this.
> I think you meant mod_devicetable.h.
devm_of_platform_populate(), so I need CONFIG_OF, too right?
>> +static struct i2c_driver sl28cpld_driver = {
>> + .probe_new = sl28cpld_probe,
>> + .driver = {
>> + .name = "sl28cpld",
>> + .of_match_table = of_match_ptr(sl28cpld_of_match),
>
> Drop of_match_ptr(). It has a little sense in this context (depends
> OF).
> It will have a little sense even if you drop depends OF b/c you will
> introduce a compiler warning.
ok
>
>> + },
>> +};
--
-michael
^ permalink raw reply
* [PATCH v1 2/2] ARM: dts: aspeed: mihawk: Add 8 tmp401 thermal sensor
From: Ben Pai @ 2020-06-05 10:06 UTC (permalink / raw)
To: joel, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel
Cc: claire_ku, Ben Pai
Signed-off-by: Ben Pai <Ben_Pai@wistron.com>
---
arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts | 40 +++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
index 25ffe65fbdc0..5bf1f13dda3b 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
@@ -834,6 +834,11 @@
line-name = "smbus0";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus9_mux232: i2c@1 {
@@ -854,6 +859,11 @@
line-name = "smbus1";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus9_mux233: i2c@2 {
@@ -897,6 +907,11 @@
line-name = "smbus2";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus9_mux236: i2c@1 {
@@ -917,6 +932,11 @@
line-name = "smbus3";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus9_mux237: i2c@2 {
@@ -979,6 +999,11 @@
line-name = "smbus4";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus10_mux240: i2c@1 {
@@ -999,6 +1024,11 @@
line-name = "smbus5";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus10_mux241: i2c@2 {
@@ -1042,6 +1072,11 @@
line-name = "smbus6";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus10_mux244: i2c@1 {
@@ -1062,6 +1097,11 @@
line-name = "smbus7";
};
};
+
+ tmp431@4c {
+ compatible = "ti,tmp401";
+ reg = <0x4c>;
+ };
};
bus10_mux245: i2c@2 {
--
2.17.1
---------------------------------------------------------------------------------------------------------------------------------------------------------------
This email contains confidential or legally privileged information and is for the sole use of its intended recipient.
Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited.
If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately.
---------------------------------------------------------------------------------------------------------------------------------------------------------------
^ permalink raw reply related
* [PATCH v1 1/2] ARM: dts: aspeed: mihawk: IO expander uses TCA9554 driver
From: Ben Pai @ 2020-06-05 10:06 UTC (permalink / raw)
To: joel, devicetree, linux-arm-kernel, linux-aspeed, linux-kernel
Cc: claire_ku, Ben Pai
Set smbus_en of IO expander to 1 in order to be able to read sensor.
Signed-off-by: Ben Pai <Ben_Pai@wistron.com>
---
arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts | 112 ++++++++++++++++++++
1 file changed, 112 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
index eac6ed2bbc67..25ffe65fbdc0 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-mihawk.dts
@@ -820,12 +820,40 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus0 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus0";
+ };
+ };
};
bus9_mux232: i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus1 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus1";
+ };
+ };
};
bus9_mux233: i2c@2 {
@@ -855,12 +883,40 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus2 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus2";
+ };
+ };
};
bus9_mux236: i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus3 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus3";
+ };
+ };
};
bus9_mux237: i2c@2 {
@@ -909,12 +965,40 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus4 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus4";
+ };
+ };
};
bus10_mux240: i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus5 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus5";
+ };
+ };
};
bus10_mux241: i2c@2 {
@@ -944,12 +1028,40 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus6 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus6";
+ };
+ };
};
bus10_mux244: i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
+
+ tca9554@39 {
+ compatible = "ti,tca9554";
+ reg = <0x39>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ smbus7 {
+ gpio-hog;
+ gpios = <4 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "smbus7";
+ };
+ };
};
bus10_mux245: i2c@2 {
--
2.17.1
---------------------------------------------------------------------------------------------------------------------------------------------------------------
This email contains confidential or legally privileged information and is for the sole use of its intended recipient.
Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited.
If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately.
---------------------------------------------------------------------------------------------------------------------------------------------------------------
^ permalink raw reply related
* Re: [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller
From: Michael Walle @ 2020-06-05 9:51 UTC (permalink / raw)
To: Lee Jones
Cc: broonie, linux-gpio, devicetree, linux-kernel, linux-hwmon,
linux-pwm, linux-watchdog, linux-arm-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Jean Delvare, Guenter Roeck,
Thierry Reding, Uwe Kleine-König, Wim Van Sebroeck,
Shawn Guo, Li Yang, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Greg Kroah-Hartman, Andy Shevchenko
In-Reply-To: <20200605065709.GD3714@dell>
Hi Lee,
thanks for the review.
Am 2020-06-05 08:57, schrieb Lee Jones:
> Mark, what do you think?
>
> On Thu, 04 Jun 2020, Michael Walle wrote:
>
>> Add the core support for the board management controller found on the
>> SMARC-sAL28 board. It consists of the following functions:
>> - watchdog
>> - GPIO controller
>> - PWM controller
>> - fan sensor
>> - interrupt controller
>>
>> At the moment, this controller is used on the Kontron SMARC-sAL28
>> board.
>>
>> Please note that the MFD driver is defined as bool in the Kconfig
>> because the next patch will add interrupt support.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/mfd/Kconfig | 19 ++++++++++
>> drivers/mfd/Makefile | 2 ++
>> drivers/mfd/sl28cpld.c | 79
>> ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 100 insertions(+)
>> create mode 100644 drivers/mfd/sl28cpld.c
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 4f8b73d92df3..5c0cd514d197 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -2109,5 +2109,24 @@ config SGI_MFD_IOC3
>> If you have an SGI Origin, Octane, or a PCI IOC3 card,
>> then say Y. Otherwise say N.
>>
>> +config MFD_SL28CPLD
>> + bool "Kontron sl28 core driver"
>
> "Kontron SL28 Core Driver"
ok
>
>> + depends on I2C=y
>> + depends on OF
>> + select REGMAP_I2C
>> + select MFD_CORE
>> + help
>> + This option enables support for the board management controller
>> + found on the Kontron sl28 CPLD. You have to select individual
>
> I can't find any reference to the "Kontron sl28 CPLD" online.
>
> Is there a datasheet?
Unfortunately not.
>> + functions, such as watchdog, GPIO, etc, under the corresponding
>> menus
>> + in order to enable them.
>> +
>> + Currently supported boards are:
>> +
>> + Kontron SMARC-sAL28
>> +
>> + To compile this driver as a module, choose M here: the module will
>> be
>> + called sl28cpld.
>> +
>> endmenu
>> endif
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 9367a92f795a..be59fb40aa28 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -264,3 +264,5 @@ obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
>> obj-$(CONFIG_MFD_STMFX) += stmfx.o
>>
>> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
>> +
>> +obj-$(CONFIG_MFD_SL28CPLD) += sl28cpld.o
>> diff --git a/drivers/mfd/sl28cpld.c b/drivers/mfd/sl28cpld.c
>> new file mode 100644
>> index 000000000000..a23194bb6efa
>> --- /dev/null
>> +++ b/drivers/mfd/sl28cpld.c
>> @@ -0,0 +1,79 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * MFD core for the sl28cpld.
>
> Ideally this would match the Kconfig subject line.
ok
>
>> + * Copyright 2019 Kontron Europe GmbH
>
> This is out of date.
ok
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/regmap.h>
>> +
>> +#define SL28CPLD_VERSION 0x03
>> +#define SL28CPLD_MIN_REQ_VERSION 14
>> +
>> +struct sl28cpld {
>> + struct device *dev;
>> + struct regmap *regmap;
>> +};
>
> Why do you need this structure?
see below
>> +static const struct regmap_config sl28cpld_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .reg_stride = 1,
>> +};
>> +
>> +static int sl28cpld_probe(struct i2c_client *i2c)
>> +{
>> + struct sl28cpld *sl28cpld;
>> + struct device *dev = &i2c->dev;
>> + unsigned int cpld_version;
>> + int ret;
>> +
>> + sl28cpld = devm_kzalloc(dev, sizeof(*sl28cpld), GFP_KERNEL);
>> + if (!sl28cpld)
>> + return -ENOMEM;
>> +
>> + sl28cpld->regmap = devm_regmap_init_i2c(i2c,
>> &sl28cpld_regmap_config);
>> + if (IS_ERR(sl28cpld->regmap))
>> + return PTR_ERR(sl28cpld->regmap);
>
> This is now a shared memory allocator and not an MFD at all.
>
> I'm clamping down on these type of drivers!
>
> Please find a better way to accomplish this.
>
> Potentially using "simple-mfd" and "simple-regmap".
>
> The former already exists and does what you want. The latter doesn't
> yet exist, but could solve your and lots of other contributor's
> issues.
Mh, the former doesn't provide a regmap, correct? Most MMIO drivers
won't
need it, but this is an I2C device. So yes, I could come up with a
"simple-regmap" proposal. I guess that should go into drivers/mfd/
because
it is more than just adding one line, i.e. you would have to configure
the
regmap and its different interfaces.
But how would you model that version check below for example? (Not that
I'm
very keen of it.)
>
> Heck maybe I'll implement it myself if this keeps happening.
>
>> + ret = regmap_read(sl28cpld->regmap, SL28CPLD_VERSION,
>> &cpld_version);
>> + if (ret)
>> + return ret;
>> +
>> + if (cpld_version < SL28CPLD_MIN_REQ_VERSION) {
>> + dev_err(dev, "unsupported CPLD version %d\n", cpld_version);
>> + return -ENODEV;
>> + }
>> +
>> + sl28cpld->dev = dev;
>> + i2c_set_clientdata(i2c, sl28cpld);
>
> If the struct definition is in here, how do you use it elsewhere?
I wanted to store the regmap somewhere, but because there are no users,
I'll
drop that.
>> + dev_info(dev, "successfully probed. CPLD version %d\n",
>> cpld_version);
>> +
>> + return devm_of_platform_populate(&i2c->dev);
>> +}
>> +
>> +static const struct of_device_id sl28cpld_of_match[] = {
>> + { .compatible = "kontron,sl28cpld-r1", },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, sl28cpld_of_match);
>> +
>> +static struct i2c_driver sl28cpld_driver = {
>> + .probe_new = sl28cpld_probe,
>> + .driver = {
>> + .name = "sl28cpld",
>> + .of_match_table = of_match_ptr(sl28cpld_of_match),
>> + },
>> +};
>> +module_i2c_driver(sl28cpld_driver);
>> +
>> +MODULE_DESCRIPTION("sl28cpld MFD Core Driver");
>> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
>> +MODULE_LICENSE("GPL");
--
-michael
^ permalink raw reply
* [PATCH v4 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591350276-15816-1-git-send-email-hayashi.kunihiko@socionext.com>
The misc interrupts consisting of PME, AER, and Link event, is handled
by INTx handler, however, these interrupts should be also handled by
MSI handler.
This adds the function uniphier_pcie_misc_isr() that handles misc
interrupts, which is called from both INTx and MSI handlers.
This function detects PME and AER interrupts with the status register,
and invoke PME and AER drivers related to MSI.
And this sets the mask for misc interrupts from INTx if MSI is enabled
and sets the mask for misc interrupts from MSI if MSI is disabled.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-uniphier.c | 57 ++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index a5401a0..5ce2479 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -44,7 +44,9 @@
#define PCL_SYS_AUX_PWR_DET BIT(8)
#define PCL_RCV_INT 0x8108
+#define PCL_RCV_INT_ALL_INT_MASK GENMASK(28, 25)
#define PCL_RCV_INT_ALL_ENABLE GENMASK(20, 17)
+#define PCL_RCV_INT_ALL_MSI_MASK GENMASK(12, 9)
#define PCL_CFG_BW_MGT_STATUS BIT(4)
#define PCL_CFG_LINK_AUTO_BW_STATUS BIT(3)
#define PCL_CFG_AER_RC_ERR_MSI_STATUS BIT(2)
@@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci)
static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv)
{
- writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
+ u32 val;
+
+ val = PCL_RCV_INT_ALL_ENABLE;
+ if (pci_msi_enabled())
+ val |= PCL_RCV_INT_ALL_INT_MASK;
+ else
+ val |= PCL_RCV_INT_ALL_MSI_MASK;
+
+ writel(val, priv->base + PCL_RCV_INT);
writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
}
@@ -231,32 +241,56 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = {
.map = uniphier_pcie_intx_map,
};
-static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+static void uniphier_pcie_misc_isr(struct pcie_port *pp, bool is_msi)
{
- struct pcie_port *pp = irq_desc_get_handler_data(desc);
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
- struct irq_chip *chip = irq_desc_get_chip(desc);
- unsigned long reg;
- u32 val, bit, virq;
+ u32 val, virq;
- /* INT for debug */
val = readl(priv->base + PCL_RCV_INT);
if (val & PCL_CFG_BW_MGT_STATUS)
dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
+
if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
- if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
- dev_dbg(pci->dev, "Root Error\n");
- if (val & PCL_CFG_PME_MSI_STATUS)
- dev_dbg(pci->dev, "PME Interrupt\n");
+
+ if (is_msi) {
+ if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
+ dev_dbg(pci->dev, "Root Error Status\n");
+
+ if (val & PCL_CFG_PME_MSI_STATUS)
+ dev_dbg(pci->dev, "PME Interrupt\n");
+
+ if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
+ PCL_CFG_PME_MSI_STATUS)) {
+ virq = irq_linear_revmap(pp->irq_domain, 0);
+ generic_handle_irq(virq);
+ }
+ }
writel(val, priv->base + PCL_RCV_INT);
+}
+
+static void uniphier_pcie_msi_host_isr(struct pcie_port *pp)
+{
+ uniphier_pcie_misc_isr(pp, true);
+}
+
+static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+{
+ struct pcie_port *pp = irq_desc_get_handler_data(desc);
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long reg;
+ u32 val, bit, virq;
/* INTx */
chained_irq_enter(chip, desc);
+ uniphier_pcie_misc_isr(pp, false);
+
val = readl(priv->base + PCL_RCV_INTX);
reg = FIELD_GET(PCL_RCV_INTX_ALL_STATUS, val);
@@ -330,6 +364,7 @@ static int uniphier_pcie_host_init(struct pcie_port *pp)
static const struct dw_pcie_host_ops uniphier_pcie_host_ops = {
.host_init = uniphier_pcie_host_init,
+ .msi_host_isr = uniphier_pcie_msi_host_isr,
};
static int uniphier_add_pcie_port(struct uniphier_pcie_priv *priv,
--
2.7.4
^ permalink raw reply related
* [PATCH v4 5/6] PCI: uniphier: Add error message when failed to get phy
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591350276-15816-1-git-send-email-hayashi.kunihiko@socionext.com>
Even if phy driver doesn't probe, the error message can't be distinguished
from other errors. This displays error message caused by the phy driver
explicitly.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-uniphier.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index c37a968..8356dd3 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -470,8 +470,12 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
return PTR_ERR(priv->rst);
priv->phy = devm_phy_optional_get(dev, "pcie-phy");
- if (IS_ERR(priv->phy))
- return PTR_ERR(priv->phy);
+ if (IS_ERR(priv->phy)) {
+ ret = PTR_ERR(priv->phy);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get phy (%d)\n", ret);
+ return ret;
+ }
platform_set_drvdata(pdev, priv);
--
2.7.4
^ permalink raw reply related
* [PATCH v4 6/6] PCI: uniphier: Use devm_platform_ioremap_resource_byname()
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591350276-15816-1-git-send-email-hayashi.kunihiko@socionext.com>
Use devm_platform_ioremap_resource_byname() to simplify the code a bit.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-uniphier.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 8356dd3..233d624 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -456,8 +456,7 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
if (IS_ERR(priv->pci.atu_base))
priv->pci.atu_base = NULL;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "link");
- priv->base = devm_ioremap_resource(dev, res);
+ priv->base = devm_platform_ioremap_resource_byname(pdev, "link");
if (IS_ERR(priv->base))
return PTR_ERR(priv->base);
--
2.7.4
^ permalink raw reply related
* [PATCH v4 3/6] dt-bindings: PCI: uniphier: Add iATU register description
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591350276-15816-1-git-send-email-hayashi.kunihiko@socionext.com>
In the dt-bindings, "atu" reg-names is required to get the register space
for iATU in Synopsys DWC version 4.80 or later.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/pci/uniphier-pcie.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/pci/uniphier-pcie.txt b/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
index 1fa2c59..c4b7381 100644
--- a/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
@@ -16,6 +16,7 @@ Required properties:
"dbi" - controller configuration registers
"link" - SoC-specific glue layer registers
"config" - PCIe configuration space
+ "atu" - iATU registers for DWC version 4.80 or later
- clocks: A phandle to the clock gate for PCIe glue layer including
the host controller.
- resets: A phandle to the reset line for PCIe glue layer including
--
2.7.4
^ permalink raw reply related
* [PATCH v4 4/6] PCI: uniphier: Add iATU register support
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591350276-15816-1-git-send-email-hayashi.kunihiko@socionext.com>
This gets iATU register area from reg property. In Synopsys DWC version
4.80 or later, since iATU register area is separated from core register
area, this area is necessary to get from DT independently.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-uniphier.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 5ce2479..c37a968 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -451,6 +451,11 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
if (IS_ERR(priv->pci.dbi_base))
return PTR_ERR(priv->pci.dbi_base);
+ priv->pci.atu_base =
+ devm_platform_ioremap_resource_byname(pdev, "atu");
+ if (IS_ERR(priv->pci.atu_base))
+ priv->pci.atu_base = NULL;
+
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "link");
priv->base = devm_ioremap_resource(dev, res);
if (IS_ERR(priv->base))
--
2.7.4
^ permalink raw reply related
* [PATCH v4 1/6] PCI: dwc: Add msi_host_isr() callback
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591350276-15816-1-git-send-email-hayashi.kunihiko@socionext.com>
This adds msi_host_isr() callback function support to describe
SoC-dependent service triggered by MSI.
For example, when AER interrupt is triggered by MSI, the callback function
reads SoC-dependent registers and detects that the interrupt is from AER,
and invoke AER interrupts related to MSI.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 3 +++
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 0a4a5aa..026edb1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -83,6 +83,9 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
u32 status, num_ctrls;
irqreturn_t ret = IRQ_NONE;
+ if (pp->ops->msi_host_isr)
+ pp->ops->msi_host_isr(pp);
+
num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
for (i = 0; i < num_ctrls; i++) {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 656e00f..e741967 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -170,6 +170,7 @@ struct dw_pcie_host_ops {
void (*scan_bus)(struct pcie_port *pp);
void (*set_num_vectors)(struct pcie_port *pp);
int (*msi_host_init)(struct pcie_port *pp);
+ void (*msi_host_isr)(struct pcie_port *pp);
};
struct pcie_port {
--
2.7.4
^ permalink raw reply related
* [PATCH v4 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller
From: Kunihiko Hayashi @ 2020-06-05 9:44 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada, Marc Zyngier
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
This series adds some features for UniPhier PCIe host controller.
- Add support for PME and AER invoked by MSI interrupt
- Add iATU register view support for PCIe version >= 4.80
- Add an error message when failing to get phy driver
This adds a new function called by MSI handler in DesignWare PCIe framework,
that invokes PME and AER funcions to detect the factor from SoC-dependent
registers.
Changes since v3:
- Move msi_host_isr() call into dw_handle_msi_irq()
- Move uniphier_pcie_misc_isr() call into the guard of chained_irq
- Use a bool argument is_msi instead of pci_msi_enabled()
- Consolidate handler calls for the same interrupt
- Fix typos in commit messages
Changes since v2:
- Avoid printing phy error message in case of EPROBE_DEFER
- Fix iATU register mapping method
- dt-bindings: Add Acked-by: line
- Fix typos in commit messages
- Use devm_platform_ioremap_resource_byname()
Changes since v1:
- Add check if struct resource is NULL
- Fix warning in the type of dev_err() argument
Kunihiko Hayashi (6):
PCI: dwc: Add msi_host_isr() callback
PCI: uniphier: Add misc interrupt handler to invoke PME and AER
dt-bindings: PCI: uniphier: Add iATU register description
PCI: uniphier: Add iATU register support
PCI: uniphier: Add error message when failed to get phy
PCI: uniphier: Use devm_platform_ioremap_resource_byname()
.../devicetree/bindings/pci/uniphier-pcie.txt | 1 +
drivers/pci/controller/dwc/pcie-designware-host.c | 3 +
drivers/pci/controller/dwc/pcie-designware.h | 1 +
drivers/pci/controller/dwc/pcie-uniphier.c | 73 +++++++++++++++++-----
4 files changed, 63 insertions(+), 15 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v4 05/11] pwm: add support for sl28cpld PWM controller
From: Andy Shevchenko @ 2020-06-05 9:33 UTC (permalink / raw)
To: Lee Jones
Cc: Michael Walle, 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, 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: <20200605084915.GE3714@dell>
On Fri, Jun 5, 2020 at 11:51 AM Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 04 Jun 2020, Michael Walle wrote:
...
> > + cycle = state->duty_cycle * config->max_duty_cycle;
> > + do_div(cycle, state->period);
>
> Forgive my ignorance (I'm new here!), but what are these 2 lines
> doing? Here we are multiplying the current duty_cycle with the
> maximum value, then dividing by the period.
>
> So in the case of PWM_MODE_1KHZ with a 50% duty cycle, you'd have:
>
> (500000 * 0x20[16]) / 1000000 = [0x10]16
>
> Thus, the above gives as a proportional representation of the maximum
> valid value for placement into the cycle control register(s), right?
>
> Either way (whether I'm correct or not), I think it would be nice to
> mention this in a comment. Maybe even clarify with a simple example.
IIRC PWM has a helper for that (to calc period based on PWM state and
new duty cycle %).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* =?y?q?=5BPATCH=C2=A0V3=200/2=5D=20Add=20SDHC=20interconnect=20bandwidth=20scaling=20?=
From: Pradeep P V K @ 2020-06-05 9:30 UTC (permalink / raw)
To: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
sboyd, georgi.djakov, mka
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
linux-mmc-owner, rnayak, sibis, matthias, Pradeep P V K
In-Reply-To: <1591269283-24084-1-git-send-email-ppvk@codeaurora.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1200 bytes --]
Add interconnect bandwidths for SDHC driver using OPP framework that
is required by SDHC driver based on the clock frequency and bus width
of the card. Otherwise, the system clocks may run at minimum clock
speed and thus affecting the performance.
This change is based on
[1] [Patch v8] Introduce OPP bandwidth bindings
(https://lkml.org/lkml/2020/5/12/493)
[2] [Patch v3 09/17] mmc: sdhci-msm: Fix error handling
for dev_pm_opp_of_add_table()
(https://lkml.org/lkml/2020/5/5/491)
[3] [RFC v6 2/2] dt-bindings: mmc: sdhci-msm: Add interconnect BW
scaling strings
(https://lkml.org/lkml/2020/3/23/409)
as there were no extra changes made on [3], retaining the Acked-by and
Reviewed-by sign-off from [3].
Changes since V2:
- Removed debug error prints on icc path ready check.
changes since V1:
- Added dev_pm_opp_find_icc_paths() to check for icc paths.
Pradeep P V K (2):
mmc: sdhci-msm: Add interconnect bandwidth scaling support
dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings
Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
drivers/mmc/host/sdhci-msm.c | 8 ++++++++
2 files changed, 26 insertions(+)
--
1.9.1
^ permalink raw reply
* =?y?q?=5BPATCH=C2=A0V3=201/2=5D=20mmc=3A=20sdhci-msm=3A=20Add=20interconnect=20bandwidth=20scaling=20support?=
From: Pradeep P V K @ 2020-06-05 9:30 UTC (permalink / raw)
To: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
sboyd, georgi.djakov, mka
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
linux-mmc-owner, rnayak, sibis, matthias, Pradeep P V K
In-Reply-To: <1591349427-27004-1-git-send-email-ppvk@codeaurora.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1376 bytes --]
Interconnect bandwidth scaling support is now added as a
part of OPP [1]. So, make sure interconnect driver is ready
before handling interconnect scaling.
This change is based on
[1] [Patch v8] Introduce OPP bandwidth bindings
(https://lkml.org/lkml/2020/5/12/493)
[2] [Patch v3] mmc: sdhci-msm: Fix error handling
for dev_pm_opp_of_add_table()
(https://lkml.org/lkml/2020/5/5/491)
Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
---
drivers/mmc/host/sdhci-msm.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index b277dd7..a945e84 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -14,6 +14,7 @@
#include <linux/slab.h>
#include <linux/iopoll.h>
#include <linux/regulator/consumer.h>
+#include <linux/interconnect.h>
#include "sdhci-pltfm.h"
#include "cqhci.h"
@@ -2070,6 +2071,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
}
msm_host->bulk_clks[0].clk = clk;
+ /* Make sure that ICC driver is ready for interconnect bandwdith
+ * scaling before registering the device for OPP.
+ */
+ ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
+ if (ret)
+ goto bus_clk_disable;
+
msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
if (IS_ERR(msm_host->opp_table)) {
ret = PTR_ERR(msm_host->opp_table);
--
1.9.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