* [PATCH] leds: mt6360: Add LED driver for MT6360
From: Gene Chen @ 2020-06-04 6:26 UTC (permalink / raw)
To: jacek.anaszewski, pavel, matthias.bgg
Cc: gene_chen, linux-kernel, cy_huang, benjamin.chao, linux-mediatek,
dmurphy, linux-leds, Wilma.Wu, linux-arm-kernel, shufan_lee
From: Gene Chen <gene_chen@richtek.com>
Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
and 3-channel RGB LED support Register/Flash/Breath Mode
Signed-off-by: Gene Chen <gene_chen@richtek.com>
base-commit: 098c4adf249c198519a4abebe482b1e6b8c50e47
---
drivers/leds/Kconfig | 11 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-mt6360.c | 1061 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/mt6360.h | 6 +-
4 files changed, 1078 insertions(+), 1 deletion(-)
create mode 100644 drivers/leds/leds-mt6360.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index c664d84..c47be91 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -229,6 +229,17 @@ config LEDS_MT6323
This option enables support for on-chip LED drivers found on
Mediatek MT6323 PMIC.
+config LEDS_MT6360
+ tristate "LED Support for Mediatek MT6360 PMIC"
+ depends on LEDS_CLASS_FLASH && OF
+ depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
+ depends on MFD_MT6360
+ help
+ This option enables support for dual Flash LED drivers found on
+ Mediatek MT6360 PMIC.
+ Support Torch and Strobe mode independently current source.
+ Include Low-VF and short protection.
+
config LEDS_S3C24XX
tristate "LED Support for Samsung S3C24XX GPIO LEDs"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 45235d5..2883b4d 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o
obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o
obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
+obj-$(CONFIG_LEDS_MT6360) += leds-mt6360.o
obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o
obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
diff --git a/drivers/leds/leds-mt6360.c b/drivers/leds/leds-mt6360.c
new file mode 100644
index 0000000..3e62547
--- /dev/null
+++ b/drivers/leds/leds-mt6360.c
@@ -0,0 +1,1061 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ *
+ * Author: Gene Chen <gene_chen@richtek.com>
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/regmap.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/led-class-flash.h>
+#include <media/v4l2-flash-led-class.h>
+
+#include <linux/mfd/mt6360.h>
+
+enum {
+ MT6360_LED_ISINK1 = 0,
+ MT6360_LED_ISINK2,
+ MT6360_LED_ISINK3,
+ MT6360_LED_ISINK4,
+ MT6360_LED_MAX,
+};
+
+enum {
+ MT6360_LEDMODE_PWM = 0,
+ MT6360_LEDMODE_BREATH,
+ MT6360_LEDMODE_CC,
+ MT6360_LEDMODE_MAX,
+};
+
+enum {
+ MT6360_FLED_CH1 = 0,
+ MT6360_FLED_CH2,
+ MT6360_FLED_MAX,
+};
+
+/* ILED setting/reg */
+#define MT6360_SINKCUR_MAX1 (0x0d)
+#define MT6360_SINKCUR_MAX2 (0x0d)
+#define MT6360_SINKCUR_MAX3 (0x0d)
+#define MT6360_SINKCUR_MAX4 (0x1f)
+#define MT6360_CURRSEL_REG1 (MT6360_PMU_RGB1_ISNK)
+#define MT6360_CURRSEL_REG2 (MT6360_PMU_RGB2_ISNK)
+#define MT6360_CURRSEL_REG3 (MT6360_PMU_RGB3_ISNK)
+#define MT6360_CURRSEL_REG4 (MT6360_PMU_RGB_ML_ISNK)
+#define MT6360_CURRSEL_MASK1 (0x0f)
+#define MT6360_CURRSEL_MASK2 (0x0f)
+#define MT6360_CURRSEL_MASK3 (0x0f)
+#define MT6360_CURRSEL_MASK4 (0x1f)
+#define MT6360_LEDMODE_REG1 (MT6360_PMU_RGB1_ISNK)
+#define MT6360_LEDMODE_REG2 (MT6360_PMU_RGB2_ISNK)
+#define MT6360_LEDMODE_REG3 (MT6360_PMU_RGB3_ISNK)
+#define MT6360_LEDMODE_REG4 (0)
+#define MT6360_LEDMODE_MASK1 (0xc0)
+#define MT6360_LEDMODE_MASK2 (0xc0)
+#define MT6360_LEDMODE_MASK3 (0xc0)
+#define MT6360_LEDMODE_MASK4 (0)
+#define MT6360_PWMDUTY_REG1 (MT6360_PMU_RGB1_DIM)
+#define MT6360_PWMDUTY_REG2 (MT6360_PMU_RGB2_DIM)
+#define MT6360_PWMDUTY_REG3 (MT6360_PMU_RGB3_DIM)
+#define MT6360_PWMDUTY_REG4 (0)
+#define MT6360_PWMDUTY_MASK1 (0xff)
+#define MT6360_PWMDUTY_MASK2 (0xff)
+#define MT6360_PWMDUTY_MASK3 (0xff)
+#define MT6360_PWMDUTY_MASK4 (0)
+#define MT6360_PWMFREQ_REG1 (MT6360_PMU_RGB12_Freq)
+#define MT6360_PWMFREQ_REG2 (MT6360_PMU_RGB12_Freq)
+#define MT6360_PWMFREQ_REG3 (MT6360_PMU_RGB34_Freq)
+#define MT6360_PWMFREQ_REG4 (0)
+#define MT6360_PWMFREQ_MASK1 (0xe0)
+#define MT6360_PWMFREQ_MASK2 (0x1c)
+#define MT6360_PWMFREQ_MASK3 (0xe0)
+#define MT6360_PWMFREQ_MASK4 (0)
+#define MT6360_BREATH_REGBASE1 (MT6360_PMU_RGB1_Tr)
+#define MT6360_BREATH_REGBASE2 (MT6360_PMU_RGB2_Tr)
+#define MT6360_BREATH_REGBASE3 (MT6360_PMU_RGB3_Tr)
+#define MT6360_BREATH_REGBASE4 (0)
+#define MT6360_LEDEN_MASK1 (0x80)
+#define MT6360_LEDEN_MASK2 (0x40)
+#define MT6360_LEDEN_MASK3 (0x20)
+#define MT6360_LEDEN_MASK4 (0x10)
+#define MT6360_LEDEN_REG (MT6360_PMU_RGB_EN)
+#define MT6360_LEDALLEN_MASK (0xf0)
+
+#define MT6360_CHRIND_MASK (0x08)
+
+/* pattern order -> toff, tr1, tr2, ton, tf1, tf2 */
+#define MT6360_BRPATTERN_NUM (6)
+#define MT6360_BREATHREG_NUM (3)
+
+/* FLED setting */
+#define MT6360_CSENABLE_REG1 (MT6360_PMU_FLED_EN)
+#define MT6360_CSENABLE_MASK1 (0x02)
+#define MT6360_CSENABLE_REG2 (MT6360_PMU_FLED_EN)
+#define MT6360_CSENABLE_MASK2 (0x01)
+#define MT6360_TORBRIGHT_MAX1 (0x1f)
+#define MT6360_TORBRIGHT_MAX2 (0x1f)
+#define MT6360_TORBRIGHT_REG1 (MT6360_PMU_FLED1_TOR_CTRL)
+#define MT6360_TORBRIGHT_MASK1 (0x1f)
+#define MT6360_STRBRIGHT_REG1 (MT6360_PMU_FLED1_STRB_CTRL2)
+#define MT6360_STRBRIGHT_MASK1 (0x7f)
+#define MT6360_TORBRIGHT_REG2 (MT6360_PMU_FLED2_TOR_CTRL)
+#define MT6360_TORBRIGHT_MASK2 (0x1f)
+#define MT6360_STRBRIGHT_REG2 (MT6360_PMU_FLED2_STRB_CTRL2)
+#define MT6360_STRBRIGHT_MASK2 (0x7f)
+#define MT6360_TORENABLE_REG1 (MT6360_PMU_FLED_EN)
+#define MT6360_TORENABLE_MASK1 (0x08)
+#define MT6360_TORENABLE_REG2 (MT6360_PMU_FLED_EN)
+#define MT6360_TORENABLE_MASK2 (0x08)
+#define MT6360_STRBENABLE_REG1 (MT6360_PMU_FLED_EN)
+#define MT6360_STRBENABLE_MASK1 (0x06)
+#define MT6360_STRBENABLE_REG2 (MT6360_PMU_FLED_EN)
+#define MT6360_STRBENABLE_MASK2 (0x04)
+#define MT6360_STRBTIMEOUT_REG (MT6360_PMU_FLED_STRB_CTRL)
+#define MT6360_STRBTIMEOUT_MASK (0x7f)
+#define MT6360_TORCHCUR_MIN (25000)
+#define MT6360_TORCHCUR_STEP (12500)
+#define MT6360_TORCHCUR_MAX (400000)
+#define MT6360_STROBECUR_MIN (50000)
+#define MT6360_STROBECUR_STEP (12500)
+#define MT6360_STROBECUR_MAX (1500000)
+#define MT6360_STRBTIMEOUT_MIN (64000)
+#define MT6360_STRBTIMEOUT_STEP (32000)
+#define MT6360_STRBTIMEOUT_MAX (2432000)
+
+#define MT6360_MASK_ULTRA_STROBE (0x80)
+#define MT6360_SHIFT_ULTRA_STROBE (7)
+
+#define MT6360_FLEDSUPPORT_FAULTS (LED_FAULT_UNDER_VOLTAGE |\
+ LED_FAULT_SHORT_CIRCUIT |\
+ LED_FAULT_INPUT_VOLTAGE |\
+ LED_FAULT_TIMEOUT)
+
+struct mt6360_led_platform_data {
+ u32 rgbon_sync;
+ u32 fled1_ultraistrb;
+ u32 fled2_ultraistrb;
+};
+
+struct breath_element_cfg {
+ /* base, step in ms */
+ unsigned int base;
+ unsigned int step;
+ unsigned int maxval;
+ unsigned int reg_offset;
+ unsigned int reg_mask;
+};
+
+struct mt6360_led_classdev {
+ struct led_classdev cdev;
+ int index;
+ struct device_node *np;
+ unsigned int currsel_reg;
+ unsigned int currsel_mask;
+ unsigned int enable_mask;
+ unsigned int mode_reg;
+ unsigned int mode_mask;
+ unsigned int pwmduty_reg;
+ unsigned int pwmduty_mask;
+ unsigned int pwmfreq_reg;
+ unsigned int pwmfreq_mask;
+ unsigned int breath_regbase;
+};
+
+struct mt6360_fled_classdev {
+ struct led_classdev_flash fl_cdev;
+ int index;
+ struct v4l2_flash *v4l2_flash;
+ struct device_node *np;
+ unsigned int cs_enable_reg;
+ unsigned int cs_enable_mask;
+ unsigned int torch_bright_reg;
+ unsigned int torch_bright_mask;
+ unsigned int torch_enable_reg;
+ unsigned int torch_enable_mask;
+ unsigned int strobe_bright_reg;
+ unsigned int strobe_bright_mask;
+ unsigned int strobe_enable_reg;
+ unsigned int strobe_enable_mask;
+ unsigned int strobe_external_reg;
+ unsigned int strobe_external_mask;
+ u32 faults;
+};
+
+struct mt6360_led_data {
+ struct device *dev;
+ struct mt6360_led_platform_data *pdata;
+ struct regmap *regmap;
+ struct mt6360_led_classdev mtled_cdev[MT6360_LED_MAX];
+ struct mt6360_fled_classdev mtfled_cdev[MT6360_FLED_MAX];
+ unsigned long fl_torch_flags;
+ unsigned long fl_strobe_flags;
+};
+
+static const struct mt6360_led_platform_data def_platform_data = {
+ .rgbon_sync = 0,
+ .fled1_ultraistrb = 1,
+ .fled2_ultraistrb = 1,
+};
+
+static int mt6360_led_brightness_set(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct mt6360_led_classdev *mtled_cdev =
+ (struct mt6360_led_classdev *)cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
+ int shift, sync_regval = 0, ret;
+
+ /* if isink1 user control, set chrind function to sw mode */
+ if (mtled_cdev->index == MT6360_LED_ISINK1) {
+ ret = regmap_update_bits(mld->regmap,
+ MT6360_PMU_RGB_EN, MT6360_CHRIND_MASK, 0xff);
+ if (ret < 0)
+ dev_err(cdev->dev, "disable chrind func fail\n");
+ }
+ if (brightness == LED_OFF) {
+ ret = regmap_update_bits(mld->regmap,
+ MT6360_LEDEN_REG, mtled_cdev->enable_mask, 0);
+ if (ret < 0)
+ return ret;
+ if (mtled_cdev->mode_reg == 0)
+ goto out_bright_set;
+ /* if off, force config to cc_mode */
+ shift = ffs(mtled_cdev->mode_mask) - 1;
+ ret = regmap_update_bits(mld->regmap, mtled_cdev->mode_reg,
+ mtled_cdev->mode_mask, MT6360_LEDMODE_CC << shift);
+ if (ret < 0)
+ dev_err(cdev->dev, "config cc mode fail\n");
+ goto out_bright_set;
+ }
+ shift = ffs(mtled_cdev->currsel_mask) - 1;
+ brightness -= 1;
+ ret = regmap_update_bits(mld->regmap, mtled_cdev->currsel_reg,
+ mtled_cdev->currsel_mask, brightness << shift);
+ if (ret < 0)
+ return ret;
+ if (mld->pdata->rgbon_sync) {
+ ret = regmap_read(mld->regmap, MT6360_LEDEN_REG, &sync_regval);
+ if (ret < 0)
+ goto out_bright_set;
+ ret = regmap_update_bits(mld->regmap,
+ MT6360_LEDEN_REG, MT6360_LEDALLEN_MASK, 0);
+ if (ret < 0)
+ goto out_bright_set;
+ sync_regval |= mtled_cdev->enable_mask;
+ ret = regmap_update_bits(mld->regmap, MT6360_LEDEN_REG,
+ MT6360_LEDALLEN_MASK, sync_regval);
+ } else {
+ ret = regmap_update_bits(mld->regmap, MT6360_LEDEN_REG,
+ mtled_cdev->enable_mask, 0xff);
+ }
+out_bright_set:
+ return ret;
+}
+
+static enum led_brightness mt6360_led_brightness_get(struct led_classdev *cdev)
+{
+ struct mt6360_led_classdev *mtled_cdev =
+ (struct mt6360_led_classdev *)cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
+ unsigned int regval = 0;
+ int shift = ffs(mtled_cdev->currsel_mask) - 1, ret;
+
+ ret = regmap_read(mld->regmap, MT6360_LEDEN_REG, ®val);
+ if (ret < 0) {
+ dev_err(cdev->dev, "%s: get enable fail\n", __func__);
+ return LED_OFF;
+ }
+ if (!(regval & mtled_cdev->enable_mask))
+ return LED_OFF;
+ ret = regmap_read(mld->regmap, mtled_cdev->currsel_reg, ®val);
+ if (ret < 0) {
+ dev_err(cdev->dev, "%s: get isink fail\n", __func__);
+ return LED_OFF;
+ }
+ regval &= mtled_cdev->currsel_mask;
+ regval >>= shift;
+ return (regval + 1);
+}
+
+static const unsigned int dim_freqs[] = {
+ 4, 8, 250, 500, 1000, 2000, 4000, 8000,
+};
+
+static int mt6360_led_blink_set(struct led_classdev *cdev,
+ unsigned long *delay_on, unsigned long *delay_off)
+{
+ struct mt6360_led_classdev *mtled_cdev =
+ (struct mt6360_led_classdev *)cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
+ int freq, duty, shift, sum, ret;
+
+ if (mtled_cdev->mode_reg == 0)
+ return -ENOTSUPP;
+ if (*delay_on == 0 && *delay_off == 0)
+ *delay_on = *delay_off = 500;
+ sum = *delay_on + *delay_off;
+ for (freq = 0; freq < ARRAY_SIZE(dim_freqs); freq++) {
+ if (sum <= dim_freqs[freq])
+ break;
+ }
+ if (freq == ARRAY_SIZE(dim_freqs)) {
+ dev_err(cdev->dev, "exceed pwm frequency max\n");
+ return -EINVAL;
+ }
+ /* invert */
+ freq = ARRAY_SIZE(dim_freqs) - 1 - freq;
+ shift = ffs(mtled_cdev->pwmfreq_mask) - 1;
+ ret = regmap_update_bits(mld->regmap, mtled_cdev->pwmfreq_reg,
+ mtled_cdev->pwmfreq_mask, freq << shift);
+ if (ret < 0) {
+ dev_err(cdev->dev, "Failed to set pwmfreq\n");
+ return ret;
+ }
+ duty = 255 * (*delay_on) / sum;
+ shift = ffs(mtled_cdev->pwmduty_mask) - 1;
+ ret = regmap_update_bits(mld->regmap, mtled_cdev->pwmduty_reg,
+ mtled_cdev->pwmduty_mask, duty << shift);
+ if (ret < 0) {
+ dev_err(cdev->dev, "Failed to set pwmduty\n");
+ return ret;
+ }
+ shift = ffs(mtled_cdev->mode_mask) - 1;
+ ret = regmap_update_bits(mld->regmap, mtled_cdev->mode_reg,
+ mtled_cdev->mode_mask, MT6360_LEDMODE_PWM << shift);
+ return ret;
+}
+
+#define MT6360_LED_DESC(_id) { \
+ .cdev = { \
+ .name = "mt6360_isink" #_id, \
+ .max_brightness = MT6360_SINKCUR_MAX##_id, \
+ .brightness_set_blocking = mt6360_led_brightness_set, \
+ .brightness_get = mt6360_led_brightness_get, \
+ .blink_set = mt6360_led_blink_set, \
+ }, \
+ .index = MT6360_LED_ISINK##_id, \
+ .currsel_reg = MT6360_CURRSEL_REG##_id, \
+ .currsel_mask = MT6360_CURRSEL_MASK##_id, \
+ .enable_mask = MT6360_LEDEN_MASK##_id, \
+ .mode_reg = MT6360_LEDMODE_REG##_id, \
+ .mode_mask = MT6360_LEDMODE_MASK##_id, \
+ .pwmduty_reg = MT6360_PWMDUTY_REG##_id, \
+ .pwmduty_mask = MT6360_PWMDUTY_MASK##_id, \
+ .pwmfreq_reg = MT6360_PWMFREQ_REG##_id, \
+ .pwmfreq_mask = MT6360_PWMFREQ_MASK##_id, \
+ .breath_regbase = MT6360_BREATH_REGBASE##_id, \
+}
+
+/* ISINK 1/2/3 for RGBLED, ISINK4 for MoonLight */
+static const struct mt6360_led_classdev def_led_classdev[MT6360_LED_MAX] = {
+ MT6360_LED_DESC(1),
+ MT6360_LED_DESC(2),
+ MT6360_LED_DESC(3),
+ MT6360_LED_DESC(4),
+};
+
+static inline bool mt6360_fled_check_flags_if_any(unsigned long *flags)
+{
+ return (*flags) ? true : false;
+}
+
+static int mt6360_fled_strobe_brightness_set(
+ struct led_classdev_flash *fled_cdev, u32 brightness)
+{
+ struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct led_flash_setting *fs = &fled_cdev->brightness;
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+ int shift;
+ u32 val;
+
+ val = brightness;
+ val = (val - fs->min) / fs->step;
+ shift = ffs(mtfled_cdev->strobe_bright_mask) - 1;
+ return regmap_update_bits(mld->regmap, mtfled_cdev->strobe_bright_reg,
+ mtfled_cdev->strobe_bright_mask, val << shift);
+}
+
+static int mt6360_fled_strobe_brightness_get(
+ struct led_classdev_flash *fled_cdev, u32 *brightness)
+{
+ struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct led_flash_setting *fs = &fled_cdev->brightness;
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+ int shift, ret;
+ u32 regval = 0;
+
+ ret = regmap_read(mld->regmap, mtfled_cdev->strobe_bright_reg, ®val);
+ if (ret < 0)
+ return ret;
+ regval &= mtfled_cdev->strobe_bright_mask;
+ shift = ffs(mtfled_cdev->strobe_bright_mask) - 1;
+ regval >>= shift;
+ /* convert to microamp value */
+ *brightness = regval * fs->step + fs->min;
+ return 0;
+}
+
+static int mt6360_fled_strobe_set(
+ struct led_classdev_flash *fled_cdev, bool state)
+{
+ struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+ int id = mtfled_cdev->index, ret;
+
+ if (!(state ^ test_bit(id, &mld->fl_strobe_flags)))
+ return 0;
+ if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
+ dev_err(led_cdev->dev,
+ "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
+ return -EINVAL;
+ }
+ ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
+ mtfled_cdev->cs_enable_mask, state ? 0xff : 0);
+ if (ret < 0) {
+ dev_err(led_cdev->dev, "Fail to set cs enable [%d]\n", state);
+ goto out_strobe_set;
+ }
+ ret = regmap_update_bits(mld->regmap, mtfled_cdev->strobe_enable_reg,
+ mtfled_cdev->strobe_enable_mask, state ? 0xff : 0);
+ if (ret < 0) {
+ dev_err(led_cdev->dev, "Fail to set strb enable [%d]\n", state);
+ goto out_strobe_set;
+ }
+ if (state) {
+ if (!mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags))
+ usleep_range(5000, 6000);
+ set_bit(id, &mld->fl_strobe_flags);
+ mtfled_cdev->faults = 0;
+ } else {
+ clear_bit(id, &mld->fl_strobe_flags);
+ if (!mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags))
+ usleep_range(400, 500);
+ }
+out_strobe_set:
+ return ret;
+}
+
+static int mt6360_fled_strobe_get(
+ struct led_classdev_flash *fled_cdev, bool *state)
+{
+ struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+ int id = mtfled_cdev->index;
+
+ *state = test_bit(id, &mld->fl_strobe_flags) ? true : false;
+ return 0;
+}
+
+static int mt6360_fled_strobe_timeout_set(
+ struct led_classdev_flash *fled_cdev, u32 timeout)
+{
+ struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct led_flash_setting *ts = &fled_cdev->timeout;
+ int shift, ret;
+ u32 regval;
+
+ regval = (timeout - ts->min) / ts->step;
+ shift = ffs(MT6360_STRBTIMEOUT_MASK) - 1;
+ ret = regmap_update_bits(mld->regmap, MT6360_STRBTIMEOUT_REG,
+ MT6360_STRBTIMEOUT_MASK, regval << shift);
+ return ret;
+}
+
+static int mt6360_fled_strobe_fault_get(
+ struct led_classdev_flash *fled_cdev, u32 *fault)
+{
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
+
+ *fault = mtfled_cdev->faults;
+ return 0;
+}
+
+static const struct led_flash_ops mt6360_fled_ops = {
+ .flash_brightness_set = mt6360_fled_strobe_brightness_set,
+ .flash_brightness_get = mt6360_fled_strobe_brightness_get,
+ .strobe_set = mt6360_fled_strobe_set,
+ .strobe_get = mt6360_fled_strobe_get,
+ .timeout_set = mt6360_fled_strobe_timeout_set,
+ .fault_get = mt6360_fled_strobe_fault_get,
+};
+
+static int mt6360_fled_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
+ int id = mtfled_cdev->index, shift, keep, ret;
+
+ if (mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags)) {
+ dev_err(led_cdev->dev,
+ "Disable all leds strobe [%lu]\n", mld->fl_strobe_flags);
+ return -EINVAL;
+ }
+ if (brightness == LED_OFF) {
+ clear_bit(id, &mld->fl_torch_flags);
+ keep = mt6360_fled_check_flags_if_any(&mld->fl_torch_flags);
+ ret = regmap_update_bits(mld->regmap,
+ mtfled_cdev->torch_enable_reg,
+ mtfled_cdev->torch_enable_mask,
+ keep ? 0xff : 0);
+ if (ret < 0) {
+ dev_err(led_cdev->dev, "Fail to set torch disable\n");
+ goto out_bright_set;
+ }
+ ret = regmap_update_bits(mld->regmap,
+ mtfled_cdev->cs_enable_reg,
+ mtfled_cdev->cs_enable_mask, 0);
+ if (ret < 0)
+ dev_err(led_cdev->dev, "Fail to set torch disable\n");
+ goto out_bright_set;
+ }
+ shift = ffs(mtfled_cdev->torch_bright_mask) - 1;
+ brightness -= 1;
+ ret = regmap_update_bits(mld->regmap, mtfled_cdev->torch_bright_reg,
+ mtfled_cdev->torch_bright_mask, brightness << shift);
+ if (ret < 0) {
+ dev_err(led_cdev->dev,
+ "Fail to set torch bright [%d]\n", brightness);
+ goto out_bright_set;
+ }
+ ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
+ mtfled_cdev->cs_enable_mask, 0xff);
+ if (ret < 0) {
+ dev_err(led_cdev->dev, "Fail to set cs enable\n");
+ goto out_bright_set;
+ }
+ ret = regmap_update_bits(mld->regmap, mtfled_cdev->torch_enable_reg,
+ mtfled_cdev->torch_enable_mask, 0xff);
+ set_bit(id, &mld->fl_torch_flags);
+out_bright_set:
+ return ret;
+}
+
+static enum led_brightness mt6360_fled_brightness_get(
+ struct led_classdev *led_cdev)
+{
+ struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
+ int id = mtfled_cdev->index, shift, ret;
+ u32 regval = 0;
+
+ if (!test_bit(id, &mld->fl_torch_flags))
+ return LED_OFF;
+ ret = regmap_read(mld->regmap, mtfled_cdev->torch_bright_reg, ®val);
+ if (ret < 0) {
+ dev_err(led_cdev->dev, "%s: Fail to get torb reg\n", __func__);
+ return LED_OFF;
+ }
+ shift = ffs(mtfled_cdev->torch_bright_mask) - 1;
+ regval &= mtfled_cdev->torch_bright_mask;
+ regval >>= shift;
+ return (regval + 1);
+}
+
+#define MT6360_FLED_DESC(_id) { \
+ .fl_cdev = { \
+ .led_cdev = { \
+ .name = "mt6360_fled_ch" #_id, \
+ .max_brightness = MT6360_TORBRIGHT_MAX##_id, \
+ .brightness_set_blocking = mt6360_fled_brightness_set, \
+ .brightness_get = mt6360_fled_brightness_get, \
+ .flags = LED_DEV_CAP_FLASH, \
+ }, \
+ .brightness = { \
+ .min = MT6360_STROBECUR_MIN, \
+ .step = MT6360_STROBECUR_STEP, \
+ .max = MT6360_STROBECUR_MAX, \
+ .val = MT6360_STROBECUR_MIN, \
+ }, \
+ .timeout = { \
+ .min = MT6360_STRBTIMEOUT_MIN, \
+ .step = MT6360_STRBTIMEOUT_STEP, \
+ .max = MT6360_STRBTIMEOUT_MAX, \
+ .val = MT6360_STRBTIMEOUT_MIN, \
+ }, \
+ .ops = &mt6360_fled_ops, \
+ }, \
+ .index = MT6360_FLED_CH##_id, \
+ .cs_enable_reg = MT6360_CSENABLE_REG##_id, \
+ .cs_enable_mask = MT6360_CSENABLE_MASK##_id, \
+ .torch_bright_reg = MT6360_TORBRIGHT_REG##_id, \
+ .torch_bright_mask = MT6360_TORBRIGHT_MASK##_id, \
+ .torch_enable_reg = MT6360_TORENABLE_REG##_id, \
+ .torch_enable_mask = MT6360_TORENABLE_MASK##_id, \
+ .strobe_bright_reg = MT6360_STRBRIGHT_REG##_id, \
+ .strobe_bright_mask = MT6360_STRBRIGHT_MASK##_id, \
+ .strobe_enable_reg = MT6360_STRBENABLE_REG##_id, \
+ .strobe_enable_mask = MT6360_STRBENABLE_MASK##_id, \
+}
+
+static const struct mt6360_fled_classdev def_fled_classdev[MT6360_FLED_MAX] = {
+ MT6360_FLED_DESC(1),
+ MT6360_FLED_DESC(2),
+};
+
+#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
+static int mt6360_fled_external_strobe_set(
+ struct v4l2_flash *v4l2_flash, bool enable)
+{
+ struct led_classdev_flash *lcf = v4l2_flash->fled_cdev;
+ struct led_classdev *led_cdev = &lcf->led_cdev;
+ struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
+ struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
+ int id = mtfled_cdev->index, ret;
+
+ if (!(enable ^ test_bit(id, &mld->fl_strobe_flags)))
+ return 0;
+ if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
+ dev_err(led_cdev->dev,
+ "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
+ return -EINVAL;
+ }
+ ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
+ mtfled_cdev->cs_enable_mask, enable ? 0xff : 0);
+ if (enable) {
+ set_bit(id, &mld->fl_strobe_flags);
+ mtfled_cdev->faults = 0;
+ } else
+ clear_bit(id, &mld->fl_strobe_flags);
+ return ret;
+}
+
+static const struct v4l2_flash_ops v4l2_flash_ops = {
+ .external_strobe_set = mt6360_fled_external_strobe_set,
+};
+
+static void mt6360_init_v4l2_flash_config(
+ struct mt6360_fled_classdev *mtfled_cdev,
+ struct v4l2_flash_config *config)
+{
+ struct led_flash_setting *torch_intensity = &config->intensity;
+ struct led_classdev *led_cdev = &(mtfled_cdev->fl_cdev.led_cdev);
+ s32 val;
+
+ snprintf(config->dev_name, sizeof(config->dev_name),
+ "%s", mtfled_cdev->fl_cdev.led_cdev.name);
+ torch_intensity->min = MT6360_TORCHCUR_MIN;
+ torch_intensity->step = MT6360_TORCHCUR_STEP;
+ val = MT6360_TORCHCUR_MIN;
+ val += ((led_cdev->max_brightness - 1) * MT6360_TORCHCUR_STEP);
+ torch_intensity->val = torch_intensity->max = val;
+ config->flash_faults |= MT6360_FLEDSUPPORT_FAULTS;
+ config->has_external_strobe = 1;
+}
+#else
+static const struct v4l2_flash_ops v4l2_flash_ops;
+
+static void mt6360_init_v4l2_flash_config(
+ struct mt6360_fled_classdev *mtfled_cdev,
+ struct v4l2_flash_config *config)
+{
+}
+#endif /* IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS) */
+
+static irqreturn_t mt6360_pmu_fled_lvf_evt_handler(int irq, void *data)
+{
+ struct mt6360_led_data *mld = data;
+
+ dev_err(mld->dev, "%s\n", __func__);
+ mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_UNDER_VOLTAGE;
+ mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_UNDER_VOLTAGE;
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled2_short_evt_handler(int irq, void *data)
+{
+ struct mt6360_led_data *mld = data;
+
+ dev_err(mld->dev, "%s\n", __func__);
+ mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_SHORT_CIRCUIT;
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled1_short_evt_handler(int irq, void *data)
+{
+ struct mt6360_led_data *mld = data;
+
+ dev_err(mld->dev, "%s\n", __func__);
+ mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_SHORT_CIRCUIT;
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled2_strb_to_evt_handler(int irq, void *data)
+{
+ struct mt6360_led_data *mld = data;
+
+ mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_TIMEOUT;
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled1_strb_to_evt_handler(int irq, void *data)
+{
+ struct mt6360_led_data *mld = data;
+
+ mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_TIMEOUT;
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t mt6360_pmu_fled_chg_vinovp_evt_handler(int irq, void *data)
+{
+ struct mt6360_led_data *mld = data;
+
+ dev_warn(mld->dev, "%s\n", __func__);
+ mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_INPUT_VOLTAGE;
+ mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_INPUT_VOLTAGE;
+ return IRQ_HANDLED;
+}
+
+static struct mt6360_pmu_irq_desc mt6360_pmu_fled_irq_desc[] = {
+ { "fled_chg_vinovp_evt", mt6360_pmu_fled_chg_vinovp_evt_handler },
+ { "fled_lvf_evt", mt6360_pmu_fled_lvf_evt_handler },
+ { "fled2_short_evt", mt6360_pmu_fled2_short_evt_handler },
+ { "fled1_short_evt", mt6360_pmu_fled1_short_evt_handler },
+ { "fled2_strb_to_evt", mt6360_pmu_fled2_strb_to_evt_handler },
+ { "fled1_strb_to_evt", mt6360_pmu_fled1_strb_to_evt_handler },
+};
+
+static int mt6360_fled_irq_register(struct platform_device *pdev)
+{
+ struct mt6360_pmu_irq_desc *irq_desc;
+ int i, irq, ret;
+
+ for (i = 0; i < ARRAY_SIZE(mt6360_pmu_fled_irq_desc); i++) {
+ irq_desc = mt6360_pmu_fled_irq_desc + i;
+ if (unlikely(!irq_desc->name))
+ continue;
+ irq = platform_get_irq_byname(pdev, irq_desc->name);
+ if (irq < 0)
+ continue;
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ irq_desc->irq_handler,
+ IRQF_TRIGGER_FALLING,
+ irq_desc->name,
+ platform_get_drvdata(pdev));
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "request %s irq fail\n", irq_desc->name);
+ return ret;
+ }
+ }
+ return 0;
+}
+
+static int mt6360_iled_parse_dt(struct device *dev,
+ struct mt6360_led_data *mld)
+{
+ struct device_node *iled_np, *child;
+ struct mt6360_led_classdev *mtled_cdev;
+ u32 val;
+ int ret;
+
+ if (!dev->of_node)
+ return 0;
+ iled_np = of_find_node_by_name(dev->of_node, "iled");
+ if (!iled_np)
+ return 0;
+ for_each_available_child_of_node(iled_np, child) {
+ ret = of_property_read_u32(child, "reg", &val);
+ if (ret) {
+ dev_err(dev, "Fail to read reg property\n");
+ continue;
+ }
+ if (val >= MT6360_LED_MAX) {
+ dev_err(dev, "Invalid iled reg [%u]\n", val);
+ ret = -EINVAL;
+ goto out_iled_dt;
+ }
+ mtled_cdev = mld->mtled_cdev + val;
+
+ of_property_read_string(child,
+ "label", &(mtled_cdev->cdev.name));
+ of_property_read_string(child, "linux,default-trigger",
+ &(mtled_cdev->cdev.default_trigger));
+ mtled_cdev->np = child;
+ }
+ return 0;
+out_iled_dt:
+ of_node_put(child);
+ return ret;
+}
+
+static int mt6360_fled_parse_dt(struct device *dev,
+ struct mt6360_led_data *mld)
+{
+ struct device_node *fled_np, *child;
+ struct mt6360_fled_classdev *mtfled_cdev;
+ struct led_classdev *led_cdev;
+ struct led_flash_setting *fs;
+ u32 val;
+ int ret;
+
+ if (!dev->of_node)
+ return 0;
+ fled_np = of_find_node_by_name(dev->of_node, "fled");
+ if (!fled_np)
+ return 0;
+ for_each_available_child_of_node(fled_np, child) {
+ ret = of_property_read_u32(child, "reg", &val);
+ if (ret) {
+ dev_err(dev, "Fail to read reg property\n");
+ continue;
+ }
+ if (val >= MT6360_FLED_MAX) {
+ dev_err(dev, "Invalid fled reg [%u]\n", val);
+ ret = -EINVAL;
+ goto out_fled_dt;
+ }
+ mtfled_cdev = mld->mtfled_cdev + val;
+
+ of_property_read_string(child, "label",
+ &(mtfled_cdev->fl_cdev.led_cdev.name));
+ ret = of_property_read_u32(child, "led-max-microamp", &val);
+ if (ret) {
+ dev_warn(dev, "led-max-microamp property missing\n");
+ val = MT6360_TORCHCUR_MIN;
+ }
+ if (val < MT6360_TORCHCUR_MIN)
+ val = MT6360_TORCHCUR_MIN;
+ val = (val - MT6360_TORCHCUR_MIN) / MT6360_TORCHCUR_STEP + 1;
+ led_cdev = &(mtfled_cdev->fl_cdev.led_cdev);
+ led_cdev->max_brightness = min(led_cdev->max_brightness, val);
+ ret = of_property_read_u32(child, "flash-max-microamp", &val);
+ if (ret) {
+ dev_warn(dev, "flash-max-microamp property missing\n");
+ val = MT6360_STROBECUR_MIN;
+ }
+ if (val < MT6360_STROBECUR_MIN)
+ val = MT6360_STROBECUR_MIN;
+ fs = &(mtfled_cdev->fl_cdev.brightness);
+ fs->val = fs->max = min(fs->max, val);
+ ret = of_property_read_u32(child, "flash-max-timeout", &val);
+ if (ret) {
+ dev_warn(dev, "flash-max-timeout property missing\n");
+ val = MT6360_STRBTIMEOUT_MIN;
+ }
+ if (val < MT6360_STRBTIMEOUT_MIN)
+ val = MT6360_STRBTIMEOUT_MIN;
+ fs = &(mtfled_cdev->fl_cdev.timeout);
+ fs->val = fs->max = min(fs->max, val);
+ mtfled_cdev->np = child;
+ }
+ return 0;
+out_fled_dt:
+ of_node_put(child);
+ return ret;
+}
+
+static void mt6360_led_parse_dt_data(struct device *dev,
+ struct mt6360_led_platform_data *pdata)
+{
+ struct device_node *np = dev->of_node;
+ int i, ret;
+ const struct {
+ const char *name;
+ u32 *val_ptr;
+ } u32_opts[] = {
+ { "rgbon_sync", &pdata->rgbon_sync },
+ { "fled1_ultraistrb", &pdata->fled1_ultraistrb },
+ { "fled2_ultraistrb", &pdata->fled2_ultraistrb },
+ };
+
+ memcpy(pdata, &def_platform_data, sizeof(*pdata));
+ for (i = 0; i < ARRAY_SIZE(u32_opts); i++) {
+ ret = of_property_read_u32(np, u32_opts[i].name,
+ u32_opts[i].val_ptr);
+ if (ret)
+ dev_err(dev, "error reading '%s'\n", u32_opts[i].name);
+ }
+}
+
+static int mt6360_led_apply_pdata(struct mt6360_led_data *mld,
+ struct mt6360_led_platform_data *pdata)
+{
+ int i, ret;
+ const struct {
+ u32 *val_ptr;
+ u8 reg;
+ u8 mask;
+ u8 shift;
+ } sel_props[] = {
+ {
+ &pdata->fled1_ultraistrb, MT6360_PMU_FLED1_STRB_CTRL2,
+ MT6360_MASK_ULTRA_STROBE, MT6360_SHIFT_ULTRA_STROBE,
+ },
+ {
+ &pdata->fled2_ultraistrb, MT6360_PMU_FLED2_STRB_CTRL2,
+ MT6360_MASK_ULTRA_STROBE, MT6360_SHIFT_ULTRA_STROBE,
+ },
+ };
+
+ for (i = 0; i < ARRAY_SIZE(sel_props); i++) {
+ ret = regmap_update_bits(mld->regmap,
+ sel_props[i].reg,
+ sel_props[i].mask,
+ *sel_props[i].val_ptr <<
+ sel_props[i].shift);
+ if (ret < 0)
+ return ret;
+ }
+ return 0;
+}
+
+static int mt6360_led_probe(struct platform_device *pdev)
+{
+ struct mt6360_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct mt6360_led_data *mld;
+ struct mt6360_led_classdev *mtled_cdev;
+ struct mt6360_fled_classdev *mtfled_cdev;
+ struct v4l2_flash_config v4l2_config;
+ int i, ret;
+
+ mld = devm_kzalloc(&pdev->dev, sizeof(*mld), GFP_KERNEL);
+ if (!mld)
+ return -ENOMEM;
+
+ if (pdev->dev.of_node) {
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ mt6360_led_parse_dt_data(&pdev->dev, pdata);
+ }
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "no platform data specified\n");
+ return -EINVAL;
+ }
+
+ mld->dev = &pdev->dev;
+ mld->pdata = pdata;
+ platform_set_drvdata(pdev, mld);
+
+ mld->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!mld->regmap) {
+ dev_err(&pdev->dev, "Failed to get parent regmap\n");
+ return -ENODEV;
+ }
+
+ ret = mt6360_led_apply_pdata(mld, pdata);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "apply pdata fail\n");
+ return ret;
+ }
+
+ memcpy(mld->mtled_cdev, def_led_classdev, sizeof(def_led_classdev));
+ ret = mt6360_iled_parse_dt(&pdev->dev, mld);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to parse iled dt\n");
+ return ret;
+ }
+
+ for (i = 0; i < MT6360_LED_MAX; i++) {
+ mtled_cdev = mld->mtled_cdev + i;
+ ret = devm_led_classdev_register(&pdev->dev,
+ &(mtled_cdev->cdev));
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register led[%d]\n", i);
+ return ret;
+ }
+ mtled_cdev->cdev.dev->of_node = mtled_cdev->np;
+ }
+
+ memcpy(mld->mtfled_cdev, def_fled_classdev, sizeof(def_fled_classdev));
+ ret = mt6360_fled_parse_dt(&pdev->dev, mld);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Fail to parse fled dt\n");
+ return ret;
+ }
+
+ for (i = 0; i < MT6360_FLED_MAX; i++) {
+ mtfled_cdev = mld->mtfled_cdev + i;
+ ret = led_classdev_flash_register(&pdev->dev,
+ &mtfled_cdev->fl_cdev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register fled[%d]\n", i);
+ goto out_fled_cdev;
+ }
+ }
+
+ for (i = 0; i < MT6360_FLED_MAX; i++) {
+ mtfled_cdev = mld->mtfled_cdev + i;
+ memset(&v4l2_config, 0, sizeof(v4l2_config));
+ mt6360_init_v4l2_flash_config(mtfled_cdev, &v4l2_config);
+ mtfled_cdev->v4l2_flash = v4l2_flash_init(&pdev->dev,
+ of_fwnode_handle(mtfled_cdev->np),
+ &mtfled_cdev->fl_cdev,
+ &v4l2_flash_ops, &v4l2_config);
+ if (IS_ERR(mtfled_cdev->v4l2_flash)) {
+ dev_err(&pdev->dev, "Failed to register v4l2_sd\n");
+ ret = PTR_ERR(mtfled_cdev->v4l2_flash);
+ goto out_v4l2_sd;
+ }
+ }
+
+ ret = mt6360_fled_irq_register(pdev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register irqs\n");
+ goto out_v4l2_sd;
+ }
+
+ return 0;
+out_v4l2_sd:
+ while (--i >= 0) {
+ mtfled_cdev = mld->mtfled_cdev + i;
+ v4l2_flash_release(mtfled_cdev->v4l2_flash);
+ }
+ i = MT6360_FLED_MAX;
+out_fled_cdev:
+ while (--i >= 0) {
+ mtfled_cdev = mld->mtfled_cdev + i;
+ led_classdev_flash_unregister(&mtfled_cdev->fl_cdev);
+ }
+ return ret;
+}
+
+static int mt6360_led_remove(struct platform_device *pdev)
+{
+ struct mt6360_led_data *mld = platform_get_drvdata(pdev);
+ struct mt6360_fled_classdev *mtfled_cdev;
+ int i;
+
+ for (i = 0; i < MT6360_FLED_MAX; i++) {
+ mtfled_cdev = mld->mtfled_cdev + i;
+ v4l2_flash_release(mtfled_cdev->v4l2_flash);
+ led_classdev_flash_unregister(&mtfled_cdev->fl_cdev);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id __maybe_unused mt6360_led_of_id[] = {
+ { .compatible = "mediatek,mt6360_led", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6360_led_of_id);
+
+static struct platform_driver mt6360_led_driver = {
+ .driver = {
+ .name = "mt6360_led",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mt6360_led_of_id),
+ },
+ .probe = mt6360_led_probe,
+ .remove = mt6360_led_remove,
+};
+module_platform_driver(mt6360_led_driver);
+
+MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>");
+MODULE_DESCRIPTION("MT6360 Led Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/mt6360.h b/include/linux/mfd/mt6360.h
index ea13040..2b81ab5 100644
--- a/include/linux/mfd/mt6360.h
+++ b/include/linux/mfd/mt6360.h
@@ -29,6 +29,11 @@ struct mt6360_pmu_data {
unsigned int chip_rev;
};
+struct mt6360_pmu_irq_desc {
+ const char *name;
+ irq_handler_t irq_handler;
+};
+
/* PMU register defininition */
#define MT6360_PMU_DEV_INFO (0x00)
#define MT6360_PMU_CORE_CTRL1 (0x01)
@@ -236,5 +241,4 @@ struct mt6360_pmu_data {
#define CHIP_VEN_MASK (0xF0)
#define CHIP_VEN_MT6360 (0x50)
#define CHIP_REV_MASK (0x0F)
-
#endif /* __MT6360_H__ */
--
2.7.4
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH] mm/memblock: export max_pfn for kernel modules
From: Miles Chen @ 2020-06-04 3:39 UTC (permalink / raw)
To: Mike Rapoport
Cc: wsd_upstream, linux-kernel, linux-mm, linux-mediatek,
Matthias Brugger, Andrew Morton, linux-arm-kernel
In-Reply-To: <20200603170624.GA202648@linux.ibm.com>
On Wed, 2020-06-03 at 20:06 +0300, Mike Rapoport wrote:
> On Thu, Jun 04, 2020 at 12:11:32AM +0800, Miles Chen wrote:
> > max_pfn is uesd to get the highest pfn in the system. Drivers like
> > drivers/iommu/mtk_iommu.c checks max_pfn to see if it should enable
> > its "4GB mode".
> >
> > This patch exports the max_pfn symbol, so we can build the driver as
> > a kernel module.
>
> You can use totalram_pages(), like e.g.
> drivers/media/platform/mtk-vpu/mtk_vpu.c:
>
> $ git grep -np totalram_pages drivers/medhttps://lkml.org/lkml/2020/5/30/123ia/
> drivers/media/platform/mtk-vpu/mtk_vpu.c=779=static int mtk_vpu_probe(struct platform_device *pdev)
> drivers/media/platform/mtk-vpu/mtk_vpu.c:861: vpu->enable_4GB = !!(totalram_pages() > (SZ_2G >> PAGE_SHIFT));
>
Thanks for the suggestion.
totalram_pages() works and I can follow this example.
Miles
>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> > mm/memblock.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index c79ba6f9920c..3b2b21ecebb6 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -99,6 +99,7 @@ EXPORT_SYMBOL(contig_page_data);
> > unsigned long max_low_pfn;
> > unsigned long min_low_pfn;
> > unsigned long max_pfn;
> > +EXPORT_SYMBOL(max_pfn);
> > unsigned long long max_possible_pfn;
> >
> > static struct memblock_region memblock_memory_init_regions[INIT_MEMBLOCK_REGIONS] __initdata_memblock;
> > --
> > 2.18.0
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] mm/memblock: export max_pfn for kernel modules
From: Miles Chen @ 2020-06-04 3:39 UTC (permalink / raw)
To: David Hildenbrand
Cc: wsd_upstream, linux-kernel, Mike Rapoport, linux-mm,
linux-mediatek, Matthias Brugger, Andrew Morton, linux-arm-kernel
In-Reply-To: <23a9a2cc-3461-52bb-4285-b063a01bd7c6@redhat.com>
On Wed, 2020-06-03 at 18:16 +0200, David Hildenbrand wrote:
> On 03.06.20 18:11, Miles Chen wrote:
> > max_pfn is uesd to get the highest pfn in the system. Drivers like
> > drivers/iommu/mtk_iommu.c checks max_pfn to see if it should enable
> > its "4GB mode".
> >
> > This patch exports the max_pfn symbol, so we can build the driver as
> > a kernel module.
>
> Please add that change to the respective user patch (and cc MM-people
> for that patch), so we have the actual user right along the change and
> can figure out if this is the right thing to do.
>
Thanks for the comment.
Mike points out another alternative way to do this by totalram_pages().
I will use that approach so we don't have to export max_pfn here.
https://lkml.org/lkml/2020/6/3/771
Miles
> >
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> > mm/memblock.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index c79ba6f9920c..3b2b21ecebb6 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -99,6 +99,7 @@ EXPORT_SYMBOL(contig_page_data);
> > unsigned long max_low_pfn;
> > unsigned long min_low_pfn;
> > unsigned long max_pfn;
> > +EXPORT_SYMBOL(max_pfn);
> > unsigned long long max_possible_pfn;
> >
> > static struct memblock_region memblock_memory_init_regions[INIT_MEMBLOCK_REGIONS] __initdata_memblock;
> >
>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [PATCH v4] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
From: Macpaul Lin @ 2020-06-04 3:01 UTC (permalink / raw)
To: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger
Cc: Mediatek WSD Upstream, linux-usb, linux-kernel, stable,
linux-mediatek, Macpaul Lin, Macpaul Lin, linux-arm-kernel
In-Reply-To: <1591189767-21988-1-git-send-email-macpaul.lin@mediatek.com>
When runtime suspend was enabled, runtime suspend might happen
when xhci is removing hcd. This might cause kernel panic when hcd
has been freed but runtime pm suspend related handle need to
reference it.
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: stable@vger.kernel.org
---
Changes for v3:
- Replace better sequence for disabling the pm_runtime suspend.
Changes for v4:
- Thanks for Sergei's review, typo in commit description has been corrected.
drivers/usb/host/xhci-mtk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bfbdb3c..641d24e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct usb_hcd *shared_hcd = xhci->shared_hcd;
+ pm_runtime_put_noidle(&dev->dev);
+ pm_runtime_disable(&dev->dev);
+
usb_remove_hcd(shared_hcd);
xhci->shared_hcd = NULL;
device_init_wakeup(&dev->dev, false);
@@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
xhci_mtk_sch_exit(mtk);
xhci_mtk_clks_disable(mtk);
xhci_mtk_ldos_disable(mtk);
- pm_runtime_put_sync(&dev->dev);
- pm_runtime_disable(&dev->dev);
return 0;
}
--
1.7.9.5
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH] iio: adc: mt6360: Add ADC driver for MT6360
From: Gene Chen @ 2020-06-04 3:00 UTC (permalink / raw)
To: jic23, matthias.bgg
Cc: gene_chen, lars, linux-iio, linux-kernel, cy_huang, benjamin.chao,
linux-mediatek, pmeerw, knaack.h, Wilma.Wu, linux-arm-kernel,
shufan_lee
From: Gene Chen <gene_chen@richtek.com>
Add MT6360 ADC driver include Charger Current, Voltage, and
Temperature.
Signed-off-by: Gene Chen <gene_chen@richtek.com>
base-commit: 098c4adf249c198519a4abebe482b1e6b8c50e47
---
drivers/iio/adc/Kconfig | 11 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mt6360-adc.c | 419 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 431 insertions(+)
create mode 100644 drivers/iio/adc/mt6360-adc.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 12bb8b7..a9736ec 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -657,6 +657,17 @@ config MCP3911
This driver can also be built as a module. If so, the module will be
called mcp3911.
+config MEDIATEK_MT6360_ADC
+ tristate "Mediatek MT6360 ADC Part"
+ depends on MFD_MT6360
+ select IIO_BUFFER
+ select IIO_KFIFO_BUF
+ help
+ Say Y here to enable MT6360 ADC Part.
+ Integrated for System Monitoring include
+ Charger and Battery Current, Voltage and
+ Terperature
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 6378078..4209776 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
obj-$(CONFIG_MCP3911) += mcp3911.o
+obj-$(CONFIG_MEDIATEK_MT6360_ADC) += mt6360-adc.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c
new file mode 100644
index 0000000..bc9c488
--- /dev/null
+++ b/drivers/iio/adc/mt6360-adc.c
@@ -0,0 +1,419 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ *
+ * Author: Gene Chen <gene_chen@richtek.com>
+ */
+
+#include <linux/completion.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/ktime.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/mt6360.h>
+
+/* CHG_CTRL3 0x13 */
+#define MT6360_AICR_MASK (0xFC)
+#define MT6360_AICR_SHFT (2)
+#define MT6360_AICR_400MA (0x6)
+/* ADC_CONFIG 0x56 */
+#define MT6360_ADCEN_SHFT (7)
+/* ADC_RPT_1 0x5A */
+#define MT6360_PREFERCH_MASK (0xF0)
+#define MT6360_PREFERCH_SHFT (4)
+#define MT6360_RPTCH_MASK (0x0F)
+
+enum {
+ MT6360_CHAN_USBID = 0,
+ MT6360_CHAN_VBUSDIV5,
+ MT6360_CHAN_VBUSDIV2,
+ MT6360_CHAN_VSYS,
+ MT6360_CHAN_VBAT,
+ MT6360_CHAN_IBUS,
+ MT6360_CHAN_IBAT,
+ MT6360_CHAN_CHG_VDDP,
+ MT6360_CHAN_TEMP_JC,
+ MT6360_CHAN_VREF_TS,
+ MT6360_CHAN_TS,
+ MT6360_CHAN_MAX,
+};
+
+struct mt6360_adc_data {
+ struct device *dev;
+ struct regmap *regmap;
+ struct task_struct *scan_task;
+ struct completion adc_complete;
+ struct mutex adc_lock;
+ ktime_t last_off_timestamps[MT6360_CHAN_MAX];
+ int irq;
+};
+
+static inline int mt6360_adc_val_converte(int val, int multiplier,
+ int offset, int divisor)
+{
+ return ((val * multiplier) + offset) / divisor;
+}
+
+static int mt6360_adc_get_process_val(struct mt6360_adc_data *info,
+ int chan_idx, int *val)
+{
+ unsigned int regval = 0;
+ int ret;
+ const struct converter {
+ int multiplier;
+ int offset;
+ int divisor;
+ } adc_converter[MT6360_CHAN_MAX] = {
+ { 1250, 0, 1}, /* USBID */
+ { 6250, 0, 1}, /* VBUSDIV5 */
+ { 2500, 0, 1}, /* VBUSDIV2 */
+ { 1250, 0, 1}, /* VSYS */
+ { 1250, 0, 1}, /* VBAT */
+ { 2500, 0, 1}, /* IBUS */
+ { 2500, 0, 1}, /* IBAT */
+ { 1250, 0, 1}, /* CHG_VDDP */
+ { 105, -8000, 100}, /* TEMP_JC */
+ { 1250, 0, 1}, /* VREF_TS */
+ { 1250, 0, 1}, /* TS */
+ }, sp_ibus_adc_converter = { 1900, 0, 1 }, *sel_converter;
+
+ if (chan_idx < 0 || chan_idx >= MT6360_CHAN_MAX)
+ return -EINVAL;
+ sel_converter = adc_converter + chan_idx;
+ if (chan_idx == MT6360_CHAN_IBUS) {
+ /* ibus chan will be affected by aicr config */
+ /* if aicr < 400, apply the special ibus converter */
+ ret = regmap_read(info->regmap, MT6360_PMU_CHG_CTRL3, ®val);
+ if (ret < 0)
+ return ret;
+ regval = (regval & MT6360_AICR_MASK) >> MT6360_AICR_SHFT;
+ if (regval < MT6360_AICR_400MA)
+ sel_converter = &sp_ibus_adc_converter;
+ }
+ *val = mt6360_adc_val_converte(*val, sel_converter->multiplier,
+ sel_converter->offset, sel_converter->divisor);
+ return 0;
+}
+
+static int mt6360_adc_read_raw(struct iio_dev *iio_dev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long mask)
+{
+ struct mt6360_adc_data *mad = iio_priv(iio_dev);
+ long timeout;
+ u8 tmp[2], rpt[3];
+ ktime_t start_t, predict_end_t;
+ int ret;
+
+ mutex_lock(&mad->adc_lock);
+ /* select preferred channel that we want */
+ ret = regmap_update_bits(mad->regmap,
+ MT6360_PMU_ADC_RPT_1, MT6360_PREFERCH_MASK,
+ chan->channel << MT6360_PREFERCH_SHFT);
+ if (ret < 0)
+ goto err_adc_init;
+ /* enable adc channel we want and adc_en */
+ memset(tmp, 0, sizeof(tmp));
+ tmp[0] |= BIT(MT6360_ADCEN_SHFT);
+ tmp[(chan->channel / 8) ? 0 : 1] |= BIT(chan->channel % 8);
+ ret = regmap_bulk_write(mad->regmap,
+ MT6360_PMU_ADC_CONFIG, tmp, sizeof(tmp));
+ if (ret < 0)
+ goto err_adc_init;
+ start_t = ktime_get();
+ predict_end_t = ktime_add_ms(
+ mad->last_off_timestamps[chan->channel], 50);
+ if (ktime_after(start_t, predict_end_t))
+ predict_end_t = ktime_add_ms(start_t, 25);
+ else
+ predict_end_t = ktime_add_ms(start_t, 75);
+ enable_irq(mad->irq);
+retry:
+ reinit_completion(&mad->adc_complete);
+ /* wait for conversion to complete */
+ timeout = wait_for_completion_interruptible_timeout(
+ &mad->adc_complete, msecs_to_jiffies(200));
+ if (timeout == 0) {
+ ret = -ETIMEDOUT;
+ goto err_adc_conv;
+ } else if (timeout < 0) {
+ ret = -EINTR;
+ goto err_adc_conv;
+ }
+ memset(rpt, 0, sizeof(rpt));
+ ret = regmap_bulk_read(mad->regmap,
+ MT6360_PMU_ADC_RPT_1, rpt, sizeof(rpt));
+ if (ret < 0)
+ goto err_adc_conv;
+ /* get report channel */
+ if ((rpt[0] & MT6360_RPTCH_MASK) != chan->channel) {
+ dev_dbg(&iio_dev->dev,
+ "not wanted channel report [%02x]\n", rpt[0]);
+ goto retry;
+ }
+ if (!ktime_after(ktime_get(), predict_end_t)) {
+ dev_dbg(&iio_dev->dev, "time is not after 26ms chan_time\n");
+ goto retry;
+ }
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = (rpt[1] << 8) | rpt[2];
+ break;
+ case IIO_CHAN_INFO_PROCESSED:
+ *val = (rpt[1] << 8) | rpt[2];
+ ret = mt6360_adc_get_process_val(mad, chan->channel, val);
+ if (ret < 0)
+ goto err_adc_conv;
+ break;
+ default:
+ break;
+ }
+ ret = IIO_VAL_INT;
+err_adc_conv:
+ disable_irq(mad->irq);
+ /* whatever disable all channel and keep adc_en*/
+ memset(tmp, 0, sizeof(tmp));
+ tmp[0] |= BIT(MT6360_ADCEN_SHFT);
+ regmap_bulk_write(mad->regmap, MT6360_PMU_ADC_CONFIG, tmp, sizeof(tmp));
+ mad->last_off_timestamps[chan->channel] = ktime_get();
+ /* set prefer channel to 0xf */
+ regmap_update_bits(mad->regmap, MT6360_PMU_ADC_RPT_1,
+ MT6360_PREFERCH_MASK, 0xF << MT6360_PREFERCH_SHFT);
+err_adc_init:
+ mutex_unlock(&mad->adc_lock);
+ return ret;
+}
+
+static const struct iio_info mt6360_adc_iio_info = {
+ .read_raw = mt6360_adc_read_raw,
+};
+
+#define MT6360_ADC_CHAN(_idx, _type) { \
+ .type = _type, \
+ .channel = MT6360_CHAN_##_idx, \
+ .scan_index = MT6360_CHAN_##_idx, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 32, \
+ .storagebits = 32, \
+ .shift = 0, \
+ .endianness = IIO_CPU, \
+ }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_PROCESSED), \
+ .datasheet_name = #_idx, \
+ .indexed = 1, \
+}
+
+static const struct iio_chan_spec mt6360_adc_channels[] = {
+ MT6360_ADC_CHAN(USBID, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(VBUSDIV5, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(VBUSDIV2, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(VSYS, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(VBAT, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(IBUS, IIO_CURRENT),
+ MT6360_ADC_CHAN(IBAT, IIO_CURRENT),
+ MT6360_ADC_CHAN(CHG_VDDP, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(TEMP_JC, IIO_TEMP),
+ MT6360_ADC_CHAN(VREF_TS, IIO_VOLTAGE),
+ MT6360_ADC_CHAN(TS, IIO_VOLTAGE),
+ IIO_CHAN_SOFT_TIMESTAMP(MT6360_CHAN_MAX),
+};
+
+static irqreturn_t mt6360_pmu_adc_donei_handler(int irq, void *data)
+{
+ struct mt6360_adc_data *mad = iio_priv(data);
+
+ complete(&mad->adc_complete);
+ return IRQ_HANDLED;
+}
+
+static int mt6360_adc_scan_task_threadfn(void *data)
+{
+ struct mt6360_adc_data *mad = data;
+ struct iio_dev *indio_dev = iio_priv_to_dev(mad);
+ int channel_vals[MT6360_CHAN_MAX];
+ int i, bit, var = 0;
+ int ret;
+
+ while (!kthread_should_stop()) {
+ memset(channel_vals, 0, sizeof(channel_vals));
+ i = 0;
+ for_each_set_bit(bit, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ ret = mt6360_adc_read_raw(indio_dev,
+ mt6360_adc_channels + bit,
+ &var, NULL,
+ IIO_CHAN_INFO_PROCESSED);
+ if (ret < 0)
+ dev_err(mad->dev, "get adc[%d] fail\n", bit);
+ channel_vals[i++] = var;
+ if (kthread_should_stop())
+ goto out;
+ }
+ iio_push_to_buffers_with_timestamp(indio_dev, channel_vals,
+ iio_get_time_ns(indio_dev));
+ }
+out:
+ do_exit(0);
+ return 0;
+}
+
+static int mt6360_adc_iio_post_enable(struct iio_dev *iio_dev)
+{
+ struct mt6360_adc_data *mad = iio_priv(iio_dev);
+
+ mad->scan_task = kthread_run(mt6360_adc_scan_task_threadfn, mad,
+ "scan_thread.%s", dev_name(&iio_dev->dev));
+ return PTR_ERR_OR_ZERO(mad->scan_task);
+}
+
+static int mt6360_adc_iio_pre_disable(struct iio_dev *iio_dev)
+{
+ struct mt6360_adc_data *mad = iio_priv(iio_dev);
+
+ if (mad->scan_task) {
+ kthread_stop(mad->scan_task);
+ mad->scan_task = NULL;
+ }
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops mt6360_adc_iio_setup_ops = {
+ .postenable = mt6360_adc_iio_post_enable,
+ .predisable = mt6360_adc_iio_pre_disable,
+};
+
+static int mt6360_adc_iio_device_register(struct iio_dev *indio_dev)
+{
+ struct mt6360_adc_data *mad = iio_priv(indio_dev);
+ struct iio_buffer *buffer;
+ int ret;
+
+ indio_dev->name = dev_name(mad->dev);
+ indio_dev->dev.parent = mad->dev;
+ indio_dev->dev.of_node = mad->dev->of_node;
+ indio_dev->info = &mt6360_adc_iio_info;
+ indio_dev->channels = mt6360_adc_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mt6360_adc_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+ indio_dev->setup_ops = &mt6360_adc_iio_setup_ops;
+ buffer = devm_iio_kfifo_allocate(mad->dev);
+ if (!buffer)
+ return -ENOMEM;
+ iio_device_attach_buffer(indio_dev, buffer);
+ ret = devm_iio_device_register(mad->dev, indio_dev);
+ if (ret < 0) {
+ dev_err(mad->dev, "Failed to register iio device\n");
+ return ret;
+ }
+ return 0;
+}
+
+static inline int mt6360_adc_reset(struct mt6360_adc_data *info)
+{
+ u8 tmp[3] = {0x80, 0, 0};
+ ktime_t all_off_time;
+ int i;
+
+ all_off_time = ktime_get();
+ for (i = 0; i < MT6360_CHAN_MAX; i++)
+ info->last_off_timestamps[i] = all_off_time;
+ /* enable adc_en, clear adc_chn_en/zcv/en/adc_wait_t/adc_idle_t */
+ return regmap_bulk_write(info->regmap,
+ MT6360_PMU_ADC_CONFIG, tmp, sizeof(tmp));
+}
+
+static int mt6360_adc_probe(struct platform_device *pdev)
+{
+ struct mt6360_adc_data *mad;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*mad));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ mad = iio_priv(indio_dev);
+ mad->dev = &pdev->dev;
+ init_completion(&mad->adc_complete);
+ mutex_init(&mad->adc_lock);
+ platform_set_drvdata(pdev, indio_dev);
+
+ mad->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!mad->regmap) {
+ dev_err(&pdev->dev, "Failed to get parent regmap\n");
+ return -ENODEV;
+ }
+
+ ret = mt6360_adc_reset(mad);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to reset adc\n");
+ return ret;
+ }
+
+ ret = mt6360_adc_iio_device_register(indio_dev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register iio device\n");
+ return ret;
+ }
+
+ mad->irq = platform_get_irq_byname(pdev, "adc_donei");
+ if (mad->irq < 0) {
+ dev_err(&pdev->dev, "Failed to get adc_done irq\n");
+ return mad->irq;
+ }
+
+ irq_set_status_flags(mad->irq, IRQ_NOAUTOEN);
+ ret = devm_request_threaded_irq(&pdev->dev, mad->irq, NULL,
+ mt6360_pmu_adc_donei_handler,
+ IRQF_TRIGGER_FALLING, "adc_donei",
+ platform_get_drvdata(pdev));
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to register adc_done irq\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int mt6360_adc_remove(struct platform_device *pdev)
+{
+ struct mt6360_adc_data *mad = platform_get_drvdata(pdev);
+
+ if (mad->scan_task)
+ kthread_stop(mad->scan_task);
+
+ return 0;
+}
+
+static const struct of_device_id __maybe_unused mt6360_adc_of_id[] = {
+ { .compatible = "mediatek,mt6360_adc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6360_adc_of_id);
+
+static struct platform_driver mt6360_adc_driver = {
+ .driver = {
+ .name = "mt6360_adc",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mt6360_adc_of_id),
+ },
+ .probe = mt6360_adc_probe,
+ .remove = mt6360_adc_remove,
+};
+module_platform_driver(mt6360_adc_driver);
+
+MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>");
+MODULE_DESCRIPTION("MT6360 ADC Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [V6, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-04 2:33 UTC (permalink / raw)
To: Tomasz Figa
Cc: Mark Rutland, Nicolas Boichat, Andy Shevchenko, srv_heupstream,
linux-devicetree, Linus Walleij,
Shengnan Wang (王圣男), Louis Kuo,
Bartosz Golaszewski, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, dongchun.zhu,
Sakari Ailus, Matthias Brugger, Cao Bing Bu,
Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
In-Reply-To: <CAAFQd5Dgboh8om68546ADELX3g-0y40rdBxY+H3WsX5xAD1_FQ@mail.gmail.com>
Hi Tomasz,
On Mon, 2020-06-01 at 20:47 +0200, Tomasz Figa wrote:
> On Wed, May 27, 2020 at 11:03 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > On Mon, 2020-05-25 at 13:45 +0200, Tomasz Figa wrote:
> > > On Fri, May 22, 2020 at 11:27 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > Thanks for the review. My replies are as below.
> > > >
> > > > On Thu, 2020-05-21 at 19:51 +0000, Tomasz Figa wrote:
> > > > > Hi Dongchun, Sakari,
> > > > >
> > > > > On Mon, May 18, 2020 at 09:27:31PM +0800, Dongchun Zhu wrote:
> > > [snip]
> > > > > > + 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 entity_cleanup;
> > > > > > + }
> > > > > > + }
> > > > > > +
> > > > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > > > + if (ret < 0)
> > > > > > + goto entity_cleanup;
> > > > > > +
> > > > > > + return 0;
> > > > > > +
> > > > > > +entity_cleanup:
> > > > >
> > > > > Need to power off if the code above powered on.
> > > > >
> > > >
> > > > Thanks for the reminder.
> > > > If there is something wrong with runtime PM, actuator is to be powered
> > > > on via dw9768_runtime_resume() API.
> > > > When actuator sub-device is powered on completely and async registered
> > > > successfully, we shall power off it afterwards.
> > > >
> > >
> > > The code above calls dw9768_runtime_resume() if
> > > !pm_runtime_enabled(dev), but the clean-up code below the
> > > entity_cleanup label doesn't have the corresponding
> > > dw9768_runtime_suspend() call.
> > >
> >
> > Did you mean the 'entity_cleanup' after v4l2_async_register_subdev()?
>
> Yes.
>
> > Actually I made some changes for OV02A V9, according to this comment.
> > Could you help review that change? Thanks.
>
> Sure, I will take a look.
>
Thanks.
Sorry, I just wanna make sure the change is okay for next release.
May we use the check like OV02A V9 did?
ret = v4l2_async_register_subdev(&dw9768->sd);
if (ret < 0) {
if (!pm_runtime_enabled(dev))
dw9768_runtime_suspend(dev);
goto entity_cleanup;
}
> Best regards,
> Tomasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v15 06/11] soc: mediatek: Add subsys clock control for bus protection
From: Nicolas Boichat @ 2020-06-04 2:29 UTC (permalink / raw)
To: Weiyi Lu
Cc: James Liao, srv_heupstream, Rob Herring, Enric Balletbo Serra,
lkml, Fan Chen, Devicetree List,
moderated list:ARM/Mediatek SoC support, Sascha Hauer,
Matthias Brugger, linux-arm Mailing List
In-Reply-To: <1590051985-29149-7-git-send-email-weiyi.lu@mediatek.com>
On Thu, May 21, 2020 at 5:06 PM Weiyi Lu <weiyi.lu@mediatek.com> wrote:
>
> For the bus protection operations, some subsys clocks need to be enabled
> before releasing the protection, and vice versa.
> But those subsys clocks could only be controlled once its corresponding
> power domain is turned on first.
> In this patch, we add the subsys clock control into its relevant steps.
>
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 62 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index 59a525a..ef2c668 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> [snip]
> val |= PWR_ISO_BIT;
> @@ -498,6 +511,39 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> return ret;
> }
>
> +static int init_subsys_clks(struct platform_device *pdev,
> + const char *prefix, struct clk **clk)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + u32 prefix_len, sub_clk_cnt = 0;
> + struct property *prop;
> + const char *clk_name;
> +
> + prefix_len = strlen(prefix);
> +
> + of_property_for_each_string(node, "clock-names", prop, clk_name) {
> + if (!strncmp(clk_name, prefix, prefix_len) &&
> + (clk_name[prefix_len] == '-')) {
> + if (sub_clk_cnt >= MAX_SUBSYS_CLKS) {
> + dev_err(&pdev->dev,
> + "subsys clk out of range %d\n",
> + sub_clk_cnt);
> + return -EINVAL;
> + }
> +
> + clk[sub_clk_cnt] = devm_clk_get(&pdev->dev,
> + clk_name);
> +
> + if (IS_ERR(clk[sub_clk_cnt]))
> + return PTR_ERR(clk[sub_clk_cnt]);
> +
> + sub_clk_cnt++;
> + }
> + }
> +
> + return sub_clk_cnt;
> +}
> +
> static int init_basic_clks(struct platform_device *pdev, struct clk **clk,
> const char * const *name)
> {
> @@ -596,6 +642,18 @@ static struct scp *init_scp(struct platform_device *pdev,
> if (ret)
> return ERR_PTR(ret);
>
> + if (data->subsys_clk_prefix) {
> + ret = init_subsys_clks(pdev,
> + data->subsys_clk_prefix,
> + scpd->subsys_clk);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "%s: subsys clk unavailable\n",
> + data->name);
init_subsys_clks should already have printed an error (directly or
indirectly), so this is not needed.
> + return ERR_PTR(ret);
> + }
> + }
> +
> genpd->name = data->name;
> genpd->power_off = scpsys_power_off;
> genpd->power_on = scpsys_power_on;
> --
> 1.8.1.1.dirty
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [V9, 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings
From: Dongchun Zhu @ 2020-06-04 2:20 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Rob Herring, Andy Shevchenko, srv_heupstream,
linux-devicetree, Linus Walleij,
Shengnan Wang (王圣男), Tomasz Figa,
Bartosz Golaszewski, Sj Huang, Nicolas Boichat,
moderated list:ARM/Mediatek SoC support, dongchun.zhu, Louis Kuo,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Linux Media Mailing List
In-Reply-To: <20200602095654.GD29325@paasikivi.fi.intel.com>
Hi Sakari,
On Tue, 2020-06-02 at 12:56 +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> On Tue, Jun 02, 2020 at 02:15:01PM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari,
> >
> > On Mon, 2020-06-01 at 20:18 +0200, Tomasz Figa wrote:
> > > On Mon, Jun 1, 2020 at 4:35 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Fri, 2020-05-29 at 15:43 +0200, Tomasz Figa wrote:
> > > > > On Thu, May 28, 2020 at 10:06 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Sakari,
> > > > > >
> > > > > > On Thu, 2020-05-28 at 10:23 +0300, Sakari Ailus wrote:
> > > > > > > Hi Dongchun,
> > > > > > >
> > > > > > > On Thu, May 28, 2020 at 11:34:42AM +0800, Dongchun Zhu wrote:
> > > > > > > > Hi Sakari, Rob,
> > > > > > > >
> > > > > > > > On Thu, 2020-05-28 at 00:16 +0300, Sakari Ailus wrote:
> > > > > > > > > Hi Rob, Dongchun,
> > > > > > > > >
> > > > > > > > > On Wed, May 27, 2020 at 09:27:22AM -0600, Rob Herring wrote:
> > > > > > > > > > > > > + properties:
> > > > > > > > > > > > > + endpoint:
> > > > > > > > > > > > > + type: object
> > > > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + properties:
> > > > > > > > > > >
> > > > > > > > > > > Actually I wonder whether we need to declare 'clock-lanes' here?
> > > > > > > > > >
> > > > > > > > > > Yes, if you are using it.
> > > > > > > > >
> > > > > > > > > Dongchun, can you confirm the chip has a single data and a single clock
> > > > > > > > > lane and that it does not support lane reordering?
> > > > > > > > >
> > > > > > > >
> > > > > > > > From the datasheet, 'MIPI inside the OV02A10 provides one single
> > > > > > > > uni-directional clock lane and one bi-directional data lane solution for
> > > > > > > > communication links between components inside a mobile device.
> > > > > > > > The data lane has full support for HS(uni-directional) and
> > > > > > > > LP(bi-directional) data transfer mode.'
> > > > > > > >
> > > > > > > > The sensor doesn't support lane reordering, so 'clock-lanes' property
> > > > > > > > would not be added in next release.
> > > > > > > >
> > > > > > > > > So if there's nothing to convey to the driver, also the data-lanes should
> > > > > > > > > be removed IMO.
> > > > > > > > >
> > > > > > > >
> > > > > > > > However, 'data-lanes' property may still be required.
> > > > > > > > It is known that either data-lanes or clock-lanes is an array of
> > > > > > > > physical data lane indexes. Position of an entry determines the logical
> > > > > > > > lane number, while the value of an entry indicates physical lane, e.g.,
> > > > > > > > for 1-lane MIPI CSI-2 bus we could have "data-lanes = <1>;", assuming
> > > > > > > > the clock lane is on hardware lane 0.
> > > > > > > >
> > > > > > > > As mentioned earlier, the OV02A10 sensor supports only 1C1D and does not
> > > > > > > > support lane reordering, so here we shall use 'data-lanes = <1>' as
> > > > > > > > there is only a clock lane for OV02A10.
> > > > > > > >
> > > > > > > > Reminder:
> > > > > > > > If 'data-lanes' property is not present, the driver would assume
> > > > > > > > four-lane operation. This means for one-lane or two-lane operation, this
> > > > > > > > property must be present and set to the right physical lane indexes.
> > > > > > > > If the hardware does not support lane reordering, monotonically
> > > > > > > > incremented values shall be used from 0 or 1 onwards, depending on
> > > > > > > > whether or not there is also a clock lane.
> > > > > > >
> > > > > > > How can the driver use four lanes, considering the device only supports a
> > > > > > > single lane??
> > > > > > >
> > > > > >
> > > > > > I understood your meaning.
> > > > > > If we omit the property 'data-lanes', the sensor should work still.
> > > > > > But then what's the meaning of the existence of 'data-lanes'?
> > > > > > If this property 'data-lanes' is always optional, then why dt-bindings
> > > > > > provide the interface?
> > > > > >
> > > > > > In the meantime, if omitting 'data-lanes' for one sensor(transmitter)
> > > > > > that has only one physical data lane, MIPI receiver(e.g., MIPI CSI-2)
> > > > > > shall enable four-lane configuration, which may increase consumption of
> > > > > > both power and resource in the process of IIC communication.
> > > > >
> > > > > Wouldn't the receiver still have the data-lanes property under its
> > > > > endpoint node, telling it how many lanes and in which order should be
> > > > > used?
> > > > >
> > > >
> > > > The MIPI receiver(RX) shall use
> > > > v4l2_async_notifier_add_fwnode_remote_subdev() API to parse the property
> > > > "data-lanes" under sensor output port.
> > >
> > > That's not true. The MIPI receiver driver parses its own port node
> > > corresponding to the sensor. Also quoting the documentation [1]:
> > >
> > > "An endpoint subnode of a device contains all properties needed for
> > > _configuration of this device_ for data exchange with other device. In most
> > > cases properties at the peer 'endpoint' nodes will be identical, however they
> > > might need to be different when there is any signal modifications on the bus
> > > between two devices, e.g. there are logic signal inverters on the lines."
> > >
> > > In this case, there is such a signal modification if the sensor has a
> > > 1-lane bus and the receiver more lines, so the data-lanes properties
> > > would be different on both sides.
> > >
> > > [1] https://elixir.bootlin.com/linux/v5.7/source/Documentation/devicetree/bindings/media/video-interfaces.txt
> > >
> >
> > Sorry for the misunderstanding.
> > After doing some experiments about the data-lanes property under sensor
> > i2c node, we found the API
> > v4l2_async_notifier_add_fwnode_remote_subdev() that MIPI receiver driver
> > used indeed parses the data-lanes under its own port node.
> >
> > Sorry make a mistake for the use case of sensor data-lanes previously.
> > Now We may encounter one new question for this patch.
> > In practice we haven't used the data-lanes under sensor i2c node
> > anywhere, if sensor driver itself doesn't parse that.
> >
> > But there is still one reason to keep the exactly right data-lanes in
> > DT. That is, the data-lanes under sensor i2c node could be used as a
> > reference for MIPI receiver driver.
> > Just as Tomasz said, 'The MIPI receiver driver parses its own port node
> > corresponding to the sensor'.
> >
> > Sakari, Tomasz, what's your opinions about the present of data-lanes
> > under sensor node or not?
>
> The receiver driver doesn't parse the properties in the sensor
> (transmitter) device's endpoint. If that property provides no information
> to the receiver, as is the case here, it should be omitted.
>
Understood.
Fixed in next release.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Dongchun Zhu @ 2020-06-04 2:14 UTC (permalink / raw)
To: linus.walleij
Cc: mark.rutland, drinkcat, andriy.shevchenko, louis.kuo,
srv_heupstream, devicetree, shengnan.wang, tfiga, bgolaszewski,
sj.huang, robh+dt, linux-mediatek, dongchun.zhu, sakari.ailus,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20200523084103.31276-3-dongchun.zhu@mediatek.com>
Hi Tomasz, Sakari, and sirs,
Could anyone help to review this patch?
On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> Add a V4L2 sub-device driver for OV02A10 image sensor.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 13 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 1040 insertions(+)
> create mode 100644 drivers/media/i2c/ov02a10.c
>
[snip]
> +static int ov02a10_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct ov02a10 *ov02a10;
> + unsigned int rotation;
> + unsigned int clock_lane_tx_speed;
> + unsigned int i;
> + int ret;
> +
> + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> + if (!ov02a10)
> + return -ENOMEM;
> +
> + ret = ov02a10_check_hwcfg(dev, ov02a10);
> + if (ret) {
> + dev_err(dev, "failed to check HW configuration: %d", ret);
> + return ret;
> + }
> +
> + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> + ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +
> + /* Optional indication of physical rotation of sensor */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> + if (!ret && rotation == 180) {
> + ov02a10->upside_down = true;
> + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> + }
> +
> + /* Optional indication of mipi TX speed */
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> + &clock_lane_tx_speed);
> +
> + if (!ret)
> + ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> +
> + /* Get system clock (eclk) */
> + ov02a10->eclk = devm_clk_get(dev, "eclk");
> + if (IS_ERR(ov02a10->eclk)) {
> + ret = PTR_ERR(ov02a10->eclk);
> + dev_err(dev, "failed to get eclk %d\n", ret);
> + return ret;
> + }
> +
> + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> + &ov02a10->eclk_freq);
> + if (ret) {
> + dev_err(dev, "failed to get eclk frequency\n");
> + return ret;
> + }
> +
> + ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> + if (ret) {
> + dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> + return ret;
> + }
> +
> + if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> + dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> + ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> + return -EINVAL;
> + }
> +
> + ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> + if (IS_ERR(ov02a10->pd_gpio)) {
> + ret = PTR_ERR(ov02a10->pd_gpio);
> + dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> + return ret;
> + }
> +
> + ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ov02a10->n_rst_gpio)) {
> + ret = PTR_ERR(ov02a10->n_rst_gpio);
> + dev_err(dev, "failed to get reset-gpios %d\n", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> + ov02a10->supplies);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + mutex_init(&ov02a10->mutex);
> + ov02a10->cur_mode = &supported_modes[0];
> + ret = ov02a10_initialize_controls(ov02a10);
> + if (ret) {
> + dev_err(dev, "failed to initialize controls\n");
> + goto err_destroy_mutex;
> + }
> +
> + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> + if (ret < 0) {
> + dev_err(dev, "failed to init entity pads: %d", ret);
> + goto err_free_handler;
> + }
> +
> + pm_runtime_enable(dev);
> + if (!pm_runtime_enabled(dev)) {
> + ret = ov02a10_power_on(dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to power on: %d\n", ret);
> + goto err_free_handler;
> + }
> + }
> +
> + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> + if (ret) {
> + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> + if (!pm_runtime_enabled(dev))
> + ov02a10_power_off(dev);
> + goto err_clean_entity;
> + }
Tomasz, Sakari, is this ok?
or coding like this:
ret = v4l2_async_register_subdev(&ov02a10->subdev);
if (!pm_runtime_enabled(dev))
ov02a10_power_off(dev);
if (ret) {
dev_err(dev, "failed to register V4L2 subdev: %d", ret);
goto err_clean_entity;
}
What's your opinions about the change?
> +
> + return 0;
> +
> +err_clean_entity:
> + media_entity_cleanup(&ov02a10->subdev.entity);
> +err_free_handler:
> + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> +err_destroy_mutex:
> + mutex_destroy(&ov02a10->mutex);
> +
> + return ret;
> +}
> +
> +static int ov02a10_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_ctrl_handler_free(sd->ctrl_handler);
> + pm_runtime_disable(&client->dev);
> + if (!pm_runtime_status_suspended(&client->dev))
> + ov02a10_power_off(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + mutex_destroy(&ov02a10->mutex);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ov02a10_of_match[] = {
> + { .compatible = "ovti,ov02a10" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> +
> +static struct i2c_driver ov02a10_i2c_driver = {
> + .driver = {
> + .name = "ov02a10",
> + .pm = &ov02a10_pm_ops,
> + .of_match_table = ov02a10_of_match,
> + },
> + .probe_new = &ov02a10_probe,
> + .remove = &ov02a10_remove,
> +};
> +
> +module_i2c_driver(ov02a10_i2c_driver);
> +
> +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> +MODULE_LICENSE("GPL v2");
> +
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v3] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
From: Sergei Shtylyov @ 2020-06-03 17:33 UTC (permalink / raw)
To: Macpaul Lin, Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman,
Matthias Brugger, stable
Cc: Mediatek WSD Upstream, linux-usb, linux-kernel, linux-mediatek,
Macpaul Lin, linux-arm-kernel
In-Reply-To: <1591189767-21988-1-git-send-email-macpaul.lin@mediatek.com>
Hello.
On 03.06.2020 16:09, Macpaul Lin wrote:
> When runtime suspend was enabled, runtime suspend might happened
Happen.
> when xhci is removing hcd. This might cause kernel panic when hcd
> has been freed but runtime pm suspend related handle need to
> reference it.
>
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
[...]
MBR, Sergei
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] mm/memblock: export max_pfn for kernel modules
From: Mike Rapoport @ 2020-06-03 17:06 UTC (permalink / raw)
To: Miles Chen
Cc: wsd_upstream, linux-kernel, linux-mm, linux-mediatek,
Matthias Brugger, Andrew Morton, linux-arm-kernel
In-Reply-To: <20200603161132.2441-1-miles.chen@mediatek.com>
On Thu, Jun 04, 2020 at 12:11:32AM +0800, Miles Chen wrote:
> max_pfn is uesd to get the highest pfn in the system. Drivers like
> drivers/iommu/mtk_iommu.c checks max_pfn to see if it should enable
> its "4GB mode".
>
> This patch exports the max_pfn symbol, so we can build the driver as
> a kernel module.
You can use totalram_pages(), like e.g.
drivers/media/platform/mtk-vpu/mtk_vpu.c:
$ git grep -np totalram_pages drivers/media/
drivers/media/platform/mtk-vpu/mtk_vpu.c=779=static int mtk_vpu_probe(struct platform_device *pdev)
drivers/media/platform/mtk-vpu/mtk_vpu.c:861: vpu->enable_4GB = !!(totalram_pages() > (SZ_2G >> PAGE_SHIFT));
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
> mm/memblock.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/mm/memblock.c b/mm/memblock.c
> index c79ba6f9920c..3b2b21ecebb6 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -99,6 +99,7 @@ EXPORT_SYMBOL(contig_page_data);
> unsigned long max_low_pfn;
> unsigned long min_low_pfn;
> unsigned long max_pfn;
> +EXPORT_SYMBOL(max_pfn);
> unsigned long long max_possible_pfn;
>
> static struct memblock_region memblock_memory_init_regions[INIT_MEMBLOCK_REGIONS] __initdata_memblock;
> --
> 2.18.0
--
Sincerely yours,
Mike.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
From: Lukasz Luba @ 2020-06-03 16:45 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Nishanth Menon, Juri Lelli, Peter Zijlstra, Viresh Kumar,
Liviu Dudau, dri-devel, Bjorn Andersson, Benjamin Segall,
alyssa.rosenzweig, Fabio Estevam, Matthias Kaehlcke, Rob Herring,
Amit Kucheria, Lorenzo Pieralisi, Vincent Guittot, Kevin Hilman,
Andy Gross, Daniel Lezcano, steven.price, Chanwoo Choi,
Ingo Molnar, dl-linux-imx, Zhang, Rui, Mel Gorman, orjan.eide,
Daniel Vetter, Linux PM, linux-arm-msm, Sascha Hauer,
Steven Rostedt, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Linux OMAP Mailing List, Dietmar Eggemann,
Linux ARM, David Airlie, Tomeu Vizoso, Quentin Perret,
Stephen Boyd, Randy Dunlap, Rafael J. Wysocki,
Linux Kernel Mailing List, Bartlomiej Zolnierkiewicz,
Sascha Hauer, Sudeep Holla, patrick.bellasi, Shawn Guo
In-Reply-To: <CAJZ5v0jL0+TXDGXaO=WfYg6QM3=B83LLZ90xtc2HtX70jdoiYQ@mail.gmail.com>
On 6/3/20 5:22 PM, Rafael J. Wysocki wrote:
> On Wed, Jun 3, 2020 at 6:12 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>>
>>
>> On 6/3/20 4:40 PM, Rafael J. Wysocki wrote:
>>> On Wed, Jun 3, 2020 at 5:26 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 6/3/20 4:13 PM, Rafael J. Wysocki wrote:
>>>>> On Tue, Jun 2, 2020 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>>>>
>>>>>> Hi Daniel,
>>>>>>
>>>>>> On 6/1/20 10:44 PM, Daniel Lezcano wrote:
>>>>>>> On 27/05/2020 11:58, Lukasz Luba wrote:
>>>>>>>> Add support for other devices than CPUs. The registration function
>>>>>>>> does not require a valid cpumask pointer and is ready to handle new
>>>>>>>> devices. Some of the internal structures has been reorganized in order to
>>>>>>>> keep consistent view (like removing per_cpu pd pointers).
>>>>>>>>
>>>>>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>>>>>> ---
>>>>>>>
>>>>>>> [ ... ]
>>>>>>>
>>>>>>>> }
>>>>>>>> EXPORT_SYMBOL_GPL(em_register_perf_domain);
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
>>>>>>>> + * @dev : Device for which the EM is registered
>>>>>>>> + *
>>>>>>>> + * Try to unregister the EM for the specified device (but not a CPU).
>>>>>>>> + */
>>>>>>>> +void em_dev_unregister_perf_domain(struct device *dev)
>>>>>>>> +{
>>>>>>>> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
>>>>>>>> + return;
>>>>>>>> +
>>>>>>>> + if (_is_cpu_device(dev))
>>>>>>>> + return;
>>>>>>>> +
>>>>>>>> + mutex_lock(&em_pd_mutex);
>>>>>>>
>>>>>>> Is the mutex really needed?
>>>>>>
>>>>>> I just wanted to align this unregister code with register. Since there
>>>>>> is debugfs dir lookup and the device's EM existence checks I thought it
>>>>>> wouldn't harm just to lock for a while and make sure the registration
>>>>>> path is not used. These two paths shouldn't affect each other, but with
>>>>>> modules loading/unloading I wanted to play safe.
>>>>>>
>>>>>> I can change it maybe to just dmb() and the end of the function if it's
>>>>>> a big performance problem in this unloading path. What do you think?
>>>>>
>>>>> I would rather leave the mutex locking as is.
>>>>>
>>>>> However, the question to ask is what exactly may go wrong without that
>>>>> locking in place? Is there any specific race condition that you are
>>>>> concerned about?
>>>>>
>>>>
>>>> I tried to test this with module loading & unloading with panfrost
>>>> driver and CPU hotplug (which should bail out quickly) and was OK.
>>>> I don't see any particular race. I don't too much about the
>>>> debugfs code, though. That's why I tried to protect from some
>>>> scripts/services which try to re-load the driver.
>>>>
>>>> Apart from that, maybe just this dev->em = NULL to be populated to all
>>>> CPUs, which mutex_unlock synchronizes for free here.
>>>
>>> If it may run concurrently with the registration for the same device,
>>> the locking is necessary, but in that case the !dev->em_pd check needs
>>> to go under the mutex too IMO, or you may end up leaking the pd if the
>>> registration can run between that check and the point at which the
>>> mutex is taken.
>>
>> They don't run concurrently for the same device and users of that EM are
>> already gone.
>> I just wanted to be sure that everything is cleaned and synced properly.
>> Here is some example of the directories under
>> /sys/kernel/debug/energy_model
>> cpu0, cpu4, gpu, dsp, etc
>>
>> The only worry that I had was the debugfs dir name, which is a
>> string from dev_name() and will be the same for the next registration
>> if module is re-loaded.
>
> OK, so that needs to be explained in a comment.
OK, I will add it.
>
>> So the 'name' is reused and debugfs_create_dir()
>> and debugfs_remove_recursive() uses this fsnotify, but they are
>> operating under inode_lock/unlock() on the parent dir 'energy_model'.
>> Then there is also this debugfs_lookup() which is slightly different.
>>
>> That's why I put a mutex to separate all registration and unregistration
>> for all devices.
>> It should work without the mutex in unregister path, but I think it does
>> not harm to take
>
> Well, fair enough, but I still think that the !dev->em_pd check should
> be done under the mutex or it will be confusing.
>
>> it just in case and also have the CPU variable sync for free.
>
> I'm not sure what you mean by the last part here?
The mutex_unlock for me also means dmb() took place. ARM has slightly
different memory model than x86 and I just wanted to be sure that
this new values reach memory and become visible to other cores.
mutex_unlock just guaranties this for me.
>
>>>
>>> Apart from this your kerneldoc comments might me improved IMO.
>>>
>>> First of all, you can use @dev inside of a kerneldoc (if @dev
>>> represents an argument of the documented function) and that will
>>> produce the right output automatically.
>>
>> OK
>>
>>>
>>> Second, it is better to avoid saying things like "Try to unregister
>>> ..." in kerneldoc comments (the "Try to" part is redundant). Simply
>>> say "Unregister ..." instead.
>>
>> Good point, thanks, I will use "Unregister ..." then.
>>
>>>
>>> Thanks!
>>>
>>
>> Shell I send a 'resend patch' which changes these @dev and
>> 'unregister' comments?
>
> Yes, please, but see the comments above too.
Saw it
>
>> Or wait for you to finish reviewing the other patches and send v9?
>
> That is not necessary, unless you want to make kerneldoc improvements
> in the other patches.
I will check them, but if they are OKish then I will just resend this
one.
Thank you for the review.
Regards,
Lukasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
From: Rafael J. Wysocki @ 2020-06-03 16:22 UTC (permalink / raw)
To: Lukasz Luba
Cc: Nishanth Menon, Juri Lelli, Rafael J. Wysocki, Peter Zijlstra,
Viresh Kumar, Liviu Dudau, dri-devel, Bjorn Andersson,
Benjamin Segall, alyssa.rosenzweig, Fabio Estevam,
Matthias Kaehlcke, Rob Herring, Amit Kucheria, Lorenzo Pieralisi,
Vincent Guittot, Kevin Hilman, Andy Gross, Daniel Lezcano,
steven.price, Chanwoo Choi, Ingo Molnar, dl-linux-imx, Zhang, Rui,
Mel Gorman, orjan.eide, Daniel Vetter, Linux PM, linux-arm-msm,
Sascha Hauer, Steven Rostedt, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Linux OMAP Mailing List, Dietmar Eggemann,
Linux ARM, David Airlie, Tomeu Vizoso, Quentin Perret,
Stephen Boyd, Randy Dunlap, Rafael J. Wysocki,
Linux Kernel Mailing List, Bartlomiej Zolnierkiewicz,
Sascha Hauer, Sudeep Holla, patrick.bellasi, Shawn Guo
In-Reply-To: <d0894383-1362-fdea-f74c-7dd8ecdc33ca@arm.com>
On Wed, Jun 3, 2020 at 6:12 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 6/3/20 4:40 PM, Rafael J. Wysocki wrote:
> > On Wed, Jun 3, 2020 at 5:26 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
> >>
> >>
> >>
> >> On 6/3/20 4:13 PM, Rafael J. Wysocki wrote:
> >>> On Tue, Jun 2, 2020 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
> >>>>
> >>>> Hi Daniel,
> >>>>
> >>>> On 6/1/20 10:44 PM, Daniel Lezcano wrote:
> >>>>> On 27/05/2020 11:58, Lukasz Luba wrote:
> >>>>>> Add support for other devices than CPUs. The registration function
> >>>>>> does not require a valid cpumask pointer and is ready to handle new
> >>>>>> devices. Some of the internal structures has been reorganized in order to
> >>>>>> keep consistent view (like removing per_cpu pd pointers).
> >>>>>>
> >>>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> >>>>>> ---
> >>>>>
> >>>>> [ ... ]
> >>>>>
> >>>>>> }
> >>>>>> EXPORT_SYMBOL_GPL(em_register_perf_domain);
> >>>>>> +
> >>>>>> +/**
> >>>>>> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
> >>>>>> + * @dev : Device for which the EM is registered
> >>>>>> + *
> >>>>>> + * Try to unregister the EM for the specified device (but not a CPU).
> >>>>>> + */
> >>>>>> +void em_dev_unregister_perf_domain(struct device *dev)
> >>>>>> +{
> >>>>>> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
> >>>>>> + return;
> >>>>>> +
> >>>>>> + if (_is_cpu_device(dev))
> >>>>>> + return;
> >>>>>> +
> >>>>>> + mutex_lock(&em_pd_mutex);
> >>>>>
> >>>>> Is the mutex really needed?
> >>>>
> >>>> I just wanted to align this unregister code with register. Since there
> >>>> is debugfs dir lookup and the device's EM existence checks I thought it
> >>>> wouldn't harm just to lock for a while and make sure the registration
> >>>> path is not used. These two paths shouldn't affect each other, but with
> >>>> modules loading/unloading I wanted to play safe.
> >>>>
> >>>> I can change it maybe to just dmb() and the end of the function if it's
> >>>> a big performance problem in this unloading path. What do you think?
> >>>
> >>> I would rather leave the mutex locking as is.
> >>>
> >>> However, the question to ask is what exactly may go wrong without that
> >>> locking in place? Is there any specific race condition that you are
> >>> concerned about?
> >>>
> >>
> >> I tried to test this with module loading & unloading with panfrost
> >> driver and CPU hotplug (which should bail out quickly) and was OK.
> >> I don't see any particular race. I don't too much about the
> >> debugfs code, though. That's why I tried to protect from some
> >> scripts/services which try to re-load the driver.
> >>
> >> Apart from that, maybe just this dev->em = NULL to be populated to all
> >> CPUs, which mutex_unlock synchronizes for free here.
> >
> > If it may run concurrently with the registration for the same device,
> > the locking is necessary, but in that case the !dev->em_pd check needs
> > to go under the mutex too IMO, or you may end up leaking the pd if the
> > registration can run between that check and the point at which the
> > mutex is taken.
>
> They don't run concurrently for the same device and users of that EM are
> already gone.
> I just wanted to be sure that everything is cleaned and synced properly.
> Here is some example of the directories under
> /sys/kernel/debug/energy_model
> cpu0, cpu4, gpu, dsp, etc
>
> The only worry that I had was the debugfs dir name, which is a
> string from dev_name() and will be the same for the next registration
> if module is re-loaded.
OK, so that needs to be explained in a comment.
> So the 'name' is reused and debugfs_create_dir()
> and debugfs_remove_recursive() uses this fsnotify, but they are
> operating under inode_lock/unlock() on the parent dir 'energy_model'.
> Then there is also this debugfs_lookup() which is slightly different.
>
> That's why I put a mutex to separate all registration and unregistration
> for all devices.
> It should work without the mutex in unregister path, but I think it does
> not harm to take
Well, fair enough, but I still think that the !dev->em_pd check should
be done under the mutex or it will be confusing.
> it just in case and also have the CPU variable sync for free.
I'm not sure what you mean by the last part here?
> >
> > Apart from this your kerneldoc comments might me improved IMO.
> >
> > First of all, you can use @dev inside of a kerneldoc (if @dev
> > represents an argument of the documented function) and that will
> > produce the right output automatically.
>
> OK
>
> >
> > Second, it is better to avoid saying things like "Try to unregister
> > ..." in kerneldoc comments (the "Try to" part is redundant). Simply
> > say "Unregister ..." instead.
>
> Good point, thanks, I will use "Unregister ..." then.
>
> >
> > Thanks!
> >
>
> Shell I send a 'resend patch' which changes these @dev and
> 'unregister' comments?
Yes, please, but see the comments above too.
> Or wait for you to finish reviewing the other patches and send v9?
That is not necessary, unless you want to make kerneldoc improvements
in the other patches.
Thanks!
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] mm/memblock: export max_pfn for kernel modules
From: David Hildenbrand @ 2020-06-03 16:16 UTC (permalink / raw)
To: Miles Chen, Mike Rapoport, Andrew Morton, Matthias Brugger
Cc: linux-mm, linux-mediatek, linux-kernel, linux-arm-kernel,
wsd_upstream
In-Reply-To: <20200603161132.2441-1-miles.chen@mediatek.com>
On 03.06.20 18:11, Miles Chen wrote:
> max_pfn is uesd to get the highest pfn in the system. Drivers like
> drivers/iommu/mtk_iommu.c checks max_pfn to see if it should enable
> its "4GB mode".
>
> This patch exports the max_pfn symbol, so we can build the driver as
> a kernel module.
Please add that change to the respective user patch (and cc MM-people
for that patch), so we have the actual user right along the change and
can figure out if this is the right thing to do.
>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
> mm/memblock.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/mm/memblock.c b/mm/memblock.c
> index c79ba6f9920c..3b2b21ecebb6 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -99,6 +99,7 @@ EXPORT_SYMBOL(contig_page_data);
> unsigned long max_low_pfn;
> unsigned long min_low_pfn;
> unsigned long max_pfn;
> +EXPORT_SYMBOL(max_pfn);
> unsigned long long max_possible_pfn;
>
> static struct memblock_region memblock_memory_init_regions[INIT_MEMBLOCK_REGIONS] __initdata_memblock;
>
--
Thanks,
David / dhildenb
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
From: Lukasz Luba @ 2020-06-03 16:12 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Nishanth Menon, Juri Lelli, Peter Zijlstra, Viresh Kumar,
Liviu Dudau, dri-devel, Bjorn Andersson, Benjamin Segall,
alyssa.rosenzweig, Fabio Estevam, Matthias Kaehlcke, Rob Herring,
Amit Kucheria, Lorenzo Pieralisi, Vincent Guittot, Kevin Hilman,
Andy Gross, Daniel Lezcano, steven.price, Chanwoo Choi,
Ingo Molnar, dl-linux-imx, Zhang, Rui, Mel Gorman, orjan.eide,
Daniel Vetter, Linux PM, linux-arm-msm, Sascha Hauer,
Steven Rostedt, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Linux OMAP Mailing List, Dietmar Eggemann,
Linux ARM, David Airlie, Tomeu Vizoso, Quentin Perret,
Stephen Boyd, Randy Dunlap, Rafael J. Wysocki,
Linux Kernel Mailing List, Bartlomiej Zolnierkiewicz,
Sascha Hauer, Sudeep Holla, patrick.bellasi, Shawn Guo
In-Reply-To: <CAJZ5v0iDNH7tZmKsYgW1xp-g3WmOod+Wo-AzJmszXuv_wztwwA@mail.gmail.com>
On 6/3/20 4:40 PM, Rafael J. Wysocki wrote:
> On Wed, Jun 3, 2020 at 5:26 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>>
>>
>> On 6/3/20 4:13 PM, Rafael J. Wysocki wrote:
>>> On Tue, Jun 2, 2020 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>>>
>>>> Hi Daniel,
>>>>
>>>> On 6/1/20 10:44 PM, Daniel Lezcano wrote:
>>>>> On 27/05/2020 11:58, Lukasz Luba wrote:
>>>>>> Add support for other devices than CPUs. The registration function
>>>>>> does not require a valid cpumask pointer and is ready to handle new
>>>>>> devices. Some of the internal structures has been reorganized in order to
>>>>>> keep consistent view (like removing per_cpu pd pointers).
>>>>>>
>>>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>>>> ---
>>>>>
>>>>> [ ... ]
>>>>>
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(em_register_perf_domain);
>>>>>> +
>>>>>> +/**
>>>>>> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
>>>>>> + * @dev : Device for which the EM is registered
>>>>>> + *
>>>>>> + * Try to unregister the EM for the specified device (but not a CPU).
>>>>>> + */
>>>>>> +void em_dev_unregister_perf_domain(struct device *dev)
>>>>>> +{
>>>>>> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
>>>>>> + return;
>>>>>> +
>>>>>> + if (_is_cpu_device(dev))
>>>>>> + return;
>>>>>> +
>>>>>> + mutex_lock(&em_pd_mutex);
>>>>>
>>>>> Is the mutex really needed?
>>>>
>>>> I just wanted to align this unregister code with register. Since there
>>>> is debugfs dir lookup and the device's EM existence checks I thought it
>>>> wouldn't harm just to lock for a while and make sure the registration
>>>> path is not used. These two paths shouldn't affect each other, but with
>>>> modules loading/unloading I wanted to play safe.
>>>>
>>>> I can change it maybe to just dmb() and the end of the function if it's
>>>> a big performance problem in this unloading path. What do you think?
>>>
>>> I would rather leave the mutex locking as is.
>>>
>>> However, the question to ask is what exactly may go wrong without that
>>> locking in place? Is there any specific race condition that you are
>>> concerned about?
>>>
>>
>> I tried to test this with module loading & unloading with panfrost
>> driver and CPU hotplug (which should bail out quickly) and was OK.
>> I don't see any particular race. I don't too much about the
>> debugfs code, though. That's why I tried to protect from some
>> scripts/services which try to re-load the driver.
>>
>> Apart from that, maybe just this dev->em = NULL to be populated to all
>> CPUs, which mutex_unlock synchronizes for free here.
>
> If it may run concurrently with the registration for the same device,
> the locking is necessary, but in that case the !dev->em_pd check needs
> to go under the mutex too IMO, or you may end up leaking the pd if the
> registration can run between that check and the point at which the
> mutex is taken.
They don't run concurrently for the same device and users of that EM are
already gone.
I just wanted to be sure that everything is cleaned and synced properly.
Here is some example of the directories under
/sys/kernel/debug/energy_model
cpu0, cpu4, gpu, dsp, etc
The only worry that I had was the debugfs dir name, which is a
string from dev_name() and will be the same for the next registration
if module is re-loaded. So the 'name' is reused and debugfs_create_dir()
and debugfs_remove_recursive() uses this fsnotify, but they are
operating under inode_lock/unlock() on the parent dir 'energy_model'.
Then there is also this debugfs_lookup() which is slightly different.
That's why I put a mutex to separate all registration and unregistration
for all devices.
It should work without the mutex in unregister path, but I think it does
not harm to take it just in case and also have the CPU variable sync for
free.
>
> Apart from this your kerneldoc comments might me improved IMO.
>
> First of all, you can use @dev inside of a kerneldoc (if @dev
> represents an argument of the documented function) and that will
> produce the right output automatically.
OK
>
> Second, it is better to avoid saying things like "Try to unregister
> ..." in kerneldoc comments (the "Try to" part is redundant). Simply
> say "Unregister ..." instead.
Good point, thanks, I will use "Unregister ..." then.
>
> Thanks!
>
Shell I send a 'resend patch' which changes these @dev and
'unregister' comments?
Or wait for you to finish reviewing the other patches and send v9?
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [PATCH] mm/memblock: export max_pfn for kernel modules
From: Miles Chen @ 2020-06-03 16:11 UTC (permalink / raw)
To: Mike Rapoport, Andrew Morton, Matthias Brugger
Cc: wsd_upstream, linux-kernel, linux-mm, Miles Chen, linux-mediatek,
linux-arm-kernel
max_pfn is uesd to get the highest pfn in the system. Drivers like
drivers/iommu/mtk_iommu.c checks max_pfn to see if it should enable
its "4GB mode".
This patch exports the max_pfn symbol, so we can build the driver as
a kernel module.
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
---
mm/memblock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mm/memblock.c b/mm/memblock.c
index c79ba6f9920c..3b2b21ecebb6 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -99,6 +99,7 @@ EXPORT_SYMBOL(contig_page_data);
unsigned long max_low_pfn;
unsigned long min_low_pfn;
unsigned long max_pfn;
+EXPORT_SYMBOL(max_pfn);
unsigned long long max_possible_pfn;
static struct memblock_region memblock_memory_init_regions[INIT_MEMBLOCK_REGIONS] __initdata_memblock;
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
From: Rafael J. Wysocki @ 2020-06-03 15:40 UTC (permalink / raw)
To: Lukasz Luba
Cc: Nishanth Menon, Juri Lelli, Rafael J. Wysocki, Peter Zijlstra,
Viresh Kumar, Liviu Dudau, dri-devel, Bjorn Andersson,
Benjamin Segall, alyssa.rosenzweig, Fabio Estevam,
Matthias Kaehlcke, Rob Herring, Amit Kucheria, Lorenzo Pieralisi,
Vincent Guittot, Kevin Hilman, Andy Gross, Daniel Lezcano,
steven.price, Chanwoo Choi, Ingo Molnar, dl-linux-imx, Zhang, Rui,
Mel Gorman, orjan.eide, Daniel Vetter, Linux PM, linux-arm-msm,
Sascha Hauer, Steven Rostedt, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Linux OMAP Mailing List, Dietmar Eggemann,
Linux ARM, David Airlie, Tomeu Vizoso, Quentin Perret,
Stephen Boyd, Randy Dunlap, Rafael J. Wysocki,
Linux Kernel Mailing List, Bartlomiej Zolnierkiewicz,
Sascha Hauer, Sudeep Holla, patrick.bellasi, Shawn Guo
In-Reply-To: <d6a0d345-53ef-523c-836d-3bc4ea4c6e66@arm.com>
On Wed, Jun 3, 2020 at 5:26 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 6/3/20 4:13 PM, Rafael J. Wysocki wrote:
> > On Tue, Jun 2, 2020 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
> >>
> >> Hi Daniel,
> >>
> >> On 6/1/20 10:44 PM, Daniel Lezcano wrote:
> >>> On 27/05/2020 11:58, Lukasz Luba wrote:
> >>>> Add support for other devices than CPUs. The registration function
> >>>> does not require a valid cpumask pointer and is ready to handle new
> >>>> devices. Some of the internal structures has been reorganized in order to
> >>>> keep consistent view (like removing per_cpu pd pointers).
> >>>>
> >>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> >>>> ---
> >>>
> >>> [ ... ]
> >>>
> >>>> }
> >>>> EXPORT_SYMBOL_GPL(em_register_perf_domain);
> >>>> +
> >>>> +/**
> >>>> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
> >>>> + * @dev : Device for which the EM is registered
> >>>> + *
> >>>> + * Try to unregister the EM for the specified device (but not a CPU).
> >>>> + */
> >>>> +void em_dev_unregister_perf_domain(struct device *dev)
> >>>> +{
> >>>> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
> >>>> + return;
> >>>> +
> >>>> + if (_is_cpu_device(dev))
> >>>> + return;
> >>>> +
> >>>> + mutex_lock(&em_pd_mutex);
> >>>
> >>> Is the mutex really needed?
> >>
> >> I just wanted to align this unregister code with register. Since there
> >> is debugfs dir lookup and the device's EM existence checks I thought it
> >> wouldn't harm just to lock for a while and make sure the registration
> >> path is not used. These two paths shouldn't affect each other, but with
> >> modules loading/unloading I wanted to play safe.
> >>
> >> I can change it maybe to just dmb() and the end of the function if it's
> >> a big performance problem in this unloading path. What do you think?
> >
> > I would rather leave the mutex locking as is.
> >
> > However, the question to ask is what exactly may go wrong without that
> > locking in place? Is there any specific race condition that you are
> > concerned about?
> >
>
> I tried to test this with module loading & unloading with panfrost
> driver and CPU hotplug (which should bail out quickly) and was OK.
> I don't see any particular race. I don't too much about the
> debugfs code, though. That's why I tried to protect from some
> scripts/services which try to re-load the driver.
>
> Apart from that, maybe just this dev->em = NULL to be populated to all
> CPUs, which mutex_unlock synchronizes for free here.
If it may run concurrently with the registration for the same device,
the locking is necessary, but in that case the !dev->em_pd check needs
to go under the mutex too IMO, or you may end up leaking the pd if the
registration can run between that check and the point at which the
mutex is taken.
Apart from this your kerneldoc comments might me improved IMO.
First of all, you can use @dev inside of a kerneldoc (if @dev
represents an argument of the documented function) and that will
produce the right output automatically.
Second, it is better to avoid saying things like "Try to unregister
..." in kerneldoc comments (the "Try to" part is redundant). Simply
say "Unregister ..." instead.
Thanks!
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
From: Lukasz Luba @ 2020-06-03 15:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Nishanth Menon, Juri Lelli, Peter Zijlstra, Viresh Kumar,
Liviu Dudau, dri-devel, Bjorn Andersson, Benjamin Segall,
alyssa.rosenzweig, Fabio Estevam, Matthias Kaehlcke, Rob Herring,
Amit Kucheria, Lorenzo Pieralisi, Vincent Guittot, Kevin Hilman,
Andy Gross, Daniel Lezcano, steven.price, Chanwoo Choi,
Ingo Molnar, dl-linux-imx, Zhang, Rui, Mel Gorman, orjan.eide,
Daniel Vetter, Linux PM, linux-arm-msm, Sascha Hauer,
Steven Rostedt, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Linux OMAP Mailing List, Dietmar Eggemann,
Linux ARM, David Airlie, Tomeu Vizoso, Quentin Perret,
Stephen Boyd, Randy Dunlap, Rafael J. Wysocki,
Linux Kernel Mailing List, Bartlomiej Zolnierkiewicz,
Sascha Hauer, Sudeep Holla, patrick.bellasi, Shawn Guo
In-Reply-To: <CAJZ5v0jwoNSYOz3nGqNshd=5btsLxOp-di-Dot+cHqAQZEQVRw@mail.gmail.com>
On 6/3/20 4:13 PM, Rafael J. Wysocki wrote:
> On Tue, Jun 2, 2020 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> Hi Daniel,
>>
>> On 6/1/20 10:44 PM, Daniel Lezcano wrote:
>>> On 27/05/2020 11:58, Lukasz Luba wrote:
>>>> Add support for other devices than CPUs. The registration function
>>>> does not require a valid cpumask pointer and is ready to handle new
>>>> devices. Some of the internal structures has been reorganized in order to
>>>> keep consistent view (like removing per_cpu pd pointers).
>>>>
>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>> ---
>>>
>>> [ ... ]
>>>
>>>> }
>>>> EXPORT_SYMBOL_GPL(em_register_perf_domain);
>>>> +
>>>> +/**
>>>> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
>>>> + * @dev : Device for which the EM is registered
>>>> + *
>>>> + * Try to unregister the EM for the specified device (but not a CPU).
>>>> + */
>>>> +void em_dev_unregister_perf_domain(struct device *dev)
>>>> +{
>>>> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
>>>> + return;
>>>> +
>>>> + if (_is_cpu_device(dev))
>>>> + return;
>>>> +
>>>> + mutex_lock(&em_pd_mutex);
>>>
>>> Is the mutex really needed?
>>
>> I just wanted to align this unregister code with register. Since there
>> is debugfs dir lookup and the device's EM existence checks I thought it
>> wouldn't harm just to lock for a while and make sure the registration
>> path is not used. These two paths shouldn't affect each other, but with
>> modules loading/unloading I wanted to play safe.
>>
>> I can change it maybe to just dmb() and the end of the function if it's
>> a big performance problem in this unloading path. What do you think?
>
> I would rather leave the mutex locking as is.
>
> However, the question to ask is what exactly may go wrong without that
> locking in place? Is there any specific race condition that you are
> concerned about?
>
I tried to test this with module loading & unloading with panfrost
driver and CPU hotplug (which should bail out quickly) and was OK.
I don't see any particular race. I don't too much about the
debugfs code, though. That's why I tried to protect from some
scripts/services which try to re-load the driver.
Apart from that, maybe just this dev->em = NULL to be populated to all
CPUs, which mutex_unlock synchronizes for free here.
Regards,
Lukasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model
From: Rafael J. Wysocki @ 2020-06-03 15:13 UTC (permalink / raw)
To: Lukasz Luba
Cc: Nishanth Menon, Juri Lelli, Peter Zijlstra, Viresh Kumar,
Liviu Dudau, dri-devel, Bjorn Andersson, Benjamin Segall,
alyssa.rosenzweig, Fabio Estevam, Matthias Kaehlcke, Rob Herring,
Amit Kucheria, Lorenzo Pieralisi, Vincent Guittot, Kevin Hilman,
Andy Gross, Daniel Lezcano, steven.price, Chanwoo Choi,
Ingo Molnar, dl-linux-imx, Zhang, Rui, Mel Gorman, orjan.eide,
Daniel Vetter, Linux PM, linux-arm-msm, Sascha Hauer,
Steven Rostedt, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Linux OMAP Mailing List, Dietmar Eggemann,
Linux ARM, David Airlie, Tomeu Vizoso, Quentin Perret,
Stephen Boyd, Randy Dunlap, Rafael J. Wysocki,
Linux Kernel Mailing List, Bartlomiej Zolnierkiewicz,
Sascha Hauer, Sudeep Holla, patrick.bellasi, Shawn Guo
In-Reply-To: <7201e161-6952-6e28-4036-bd0f0353ec30@arm.com>
On Tue, Jun 2, 2020 at 1:31 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi Daniel,
>
> On 6/1/20 10:44 PM, Daniel Lezcano wrote:
> > On 27/05/2020 11:58, Lukasz Luba wrote:
> >> Add support for other devices than CPUs. The registration function
> >> does not require a valid cpumask pointer and is ready to handle new
> >> devices. Some of the internal structures has been reorganized in order to
> >> keep consistent view (like removing per_cpu pd pointers).
> >>
> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> >> ---
> >
> > [ ... ]
> >
> >> }
> >> EXPORT_SYMBOL_GPL(em_register_perf_domain);
> >> +
> >> +/**
> >> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device
> >> + * @dev : Device for which the EM is registered
> >> + *
> >> + * Try to unregister the EM for the specified device (but not a CPU).
> >> + */
> >> +void em_dev_unregister_perf_domain(struct device *dev)
> >> +{
> >> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd)
> >> + return;
> >> +
> >> + if (_is_cpu_device(dev))
> >> + return;
> >> +
> >> + mutex_lock(&em_pd_mutex);
> >
> > Is the mutex really needed?
>
> I just wanted to align this unregister code with register. Since there
> is debugfs dir lookup and the device's EM existence checks I thought it
> wouldn't harm just to lock for a while and make sure the registration
> path is not used. These two paths shouldn't affect each other, but with
> modules loading/unloading I wanted to play safe.
>
> I can change it maybe to just dmb() and the end of the function if it's
> a big performance problem in this unloading path. What do you think?
I would rather leave the mutex locking as is.
However, the question to ask is what exactly may go wrong without that
locking in place? Is there any specific race condition that you are
concerned about?
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] mtd: rawnand: mtk: Convert the driver to exec_op()
From: Miquel Raynal @ 2020-06-03 14:23 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, linux-mtd,
Matthias Brugger, linux-mediatek, Xiaolei Li
In-Reply-To: <20200518170321.321697-1-boris.brezillon@collabora.com>
Hello,
Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 18 May
2020 19:03:21 +0200:
> Let's convert the driver to exec_op() to have one less driver relying
> on the legacy interface.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/mtk_nand.c | 116 +++++++++++++++++++-------------
> 1 file changed, 71 insertions(+), 45 deletions(-)
Anyone to test this series?
If not I will apply it as soon as v5.8-rc1 is released.
Thanks,
Miquèl
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend
From: Macpaul Lin @ 2020-06-03 13:50 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, Szabolcs Szőke, Mediatek WSD Upstream,
Greg Kroah-Hartman, linux-usb, Takashi Iwai, stable, linux-kernel,
Hui Wang, Alexander Tsoy, linux-mediatek, Matthias Brugger,
Johan Hovold, Jaroslav Kysela, Macpaul Lin, linux-arm-kernel
In-Reply-To: <s5hr1uwco4c.wl-tiwai@suse.de>
On Wed, 2020-06-03 at 14:47 +0200, Takashi Iwai wrote:
> On Wed, 03 Jun 2020 14:39:24 +0200,
> Macpaul Lin wrote:
> >
> > On Wed, 2020-06-03 at 10:45 +0200, Takashi Iwai wrote:
> > > On Wed, 03 Jun 2020 08:54:51 +0200,
> > > Takashi Iwai wrote:
> > > >
> > > > On Wed, 03 Jun 2020 08:28:09 +0200,
> > > > Takashi Iwai wrote:
> > > > >
> > > > > And, the most suspicious case is the last one,
> > > > > chip->num_suspended-intf. It means that the device has multiple
> > > > > USB interfaces and they went to suspend, while the resume isn't
> > > > > performed for the all suspended interfaces in return.
> > > >
> > > > If this is the cause, a patch like below might help.
> > > > It gets/puts the all assigned interfaced instead of only the primary
> > > > one.
> > >
> > > ... and considering of the problem again, rather the patch below might
> > > be the right answer. Now the driver tries to remember at which state
> > > it entered into the system-suspend. Upon resume, in return, when the
> > > state reaches back to that point, set the card state to D0.
> > >
> > > The previous patch can be applied on the top, too, and it might be
> > > worth to apply both.
> > >
> > > Let me know if any of those actually helps.
> > >
> > >
> > > Takashi
> >
> > Thanks for your response so quickly.
> > I've just test this patch since it looks like enough for the issue.
>
> Good to hear!
>
> > This patch worked since the flag system_suspend will be set at the same
> > time when power state has been changed. I have 2 interface with the head
> > set. But actually the problem happened when primary one is suspended.
>
> Currently the autosuspend is set only to the primary interface; IOW,
> the other interfaces will never get autosuspend, and the another
> suspend-all-intf patch should improve that situation. But it won't
> fix your actual bug, obviously :)
>
> > So I didn't test the earlier patch "suspend all interface instead of
> > only the primary one."
>
> Could you try it one on top of the last patch? At least I'd like to
> see whether it causes any regression.
I've tried both of these 2 patches together, and it looks okay.
> > Will you resend this patch officially later? I think this solution is
> > required to send to stable, too. It's better to have it for other stable
> > kernel versions include android's.
>
> Yes, that's a general bug and worth to be merged quickly.
> I'm going to submit a proper patch soon later.
>
>
> thanks,
>
> Takashi
>
Thanks!
Macpaul Lin
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [PATCH v3] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
From: Macpaul Lin @ 2020-06-03 13:09 UTC (permalink / raw)
To: Chunfeng Yun, Mathias Nyman, Greg Kroah-Hartman, Matthias Brugger,
stable
Cc: Mediatek WSD Upstream, linux-usb, linux-kernel, linux-mediatek,
Macpaul Lin, Macpaul Lin, linux-arm-kernel
In-Reply-To: <ebd32a2b-c4ba-8891-b13e-f6c641a94276@linux.intel.com>
When runtime suspend was enabled, runtime suspend might happened
when xhci is removing hcd. This might cause kernel panic when hcd
has been freed but runtime pm suspend related handle need to
reference it.
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
Changes for v3:
- Replace better sequence for disabling the pm_runtime suspend.
drivers/usb/host/xhci-mtk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index bfbdb3c..641d24e 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct usb_hcd *shared_hcd = xhci->shared_hcd;
+ pm_runtime_put_noidle(&dev->dev);
+ pm_runtime_disable(&dev->dev);
+
usb_remove_hcd(shared_hcd);
xhci->shared_hcd = NULL;
device_init_wakeup(&dev->dev, false);
@@ -597,8 +600,6 @@ static int xhci_mtk_remove(struct platform_device *dev)
xhci_mtk_sch_exit(mtk);
xhci_mtk_clks_disable(mtk);
xhci_mtk_ldos_disable(mtk);
- pm_runtime_put_sync(&dev->dev);
- pm_runtime_disable(&dev->dev);
return 0;
}
--
1.7.9.5
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH] usb: host: xhci-mtk: avoid runtime suspend when removing hcd
From: Macpaul Lin @ 2020-06-03 13:01 UTC (permalink / raw)
To: Mathias Nyman
Cc: Mathias Nyman, Mediatek WSD Upstream, Greg Kroah-Hartman,
linux-usb, linux-kernel, Matthias Brugger, linux-mediatek,
Chunfeng Yun, Macpaul Lin, linux-arm-kernel
In-Reply-To: <ebd32a2b-c4ba-8891-b13e-f6c641a94276@linux.intel.com>
On Wed, 2020-06-03 at 14:47 +0300, Mathias Nyman wrote:
> On 29.5.2020 7.29, Macpaul Lin wrote:
> > When runtime suspend was enabled, runtime suspend might happened
> > when xhci is removing hcd. This might cause kernel panic when hcd
> > has been freed but runtime pm suspend related handle need to
> > reference it.
> >
> > Change-Id: I70a5dc8006207caeecbac6955ce8e5345dcc70e6
> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> > ---
> > drivers/usb/host/xhci-mtk.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index bfbdb3c..641d24e 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -587,6 +587,9 @@ static int xhci_mtk_remove(struct platform_device *dev)
> > struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > struct usb_hcd *shared_hcd = xhci->shared_hcd;
> >
> > + pm_runtime_put_sync(&dev->dev);
>
> Might runtime suspend here.
> It's a lot better than before, no panic as hcd isn't released, but a bit unnecessary.
>
> how about this sequence instead:
> pm_runtime_disable()
> pm_runtime_put_noidle()
>
> > + pm_runtime_disable(&dev->dev);
> > +
>
> -Mathias
Thanks for your suggestion!
Will it better to put no idle before disable?
pm_runtime_put_noidle()
pm_runtime_disable()
I've found pm_runtime_put_noidle is called in pm_runtime_disable() when
there is a pending request.
I will send patch v3 as noidle() called earlier than disable(). Please
help to comment it if disable() should go before.
Thanks!
Macpaul Lin
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend
From: Macpaul Lin @ 2020-06-03 12:39 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, Szabolcs Szőke, Mediatek WSD Upstream,
Greg Kroah-Hartman, linux-usb, Takashi Iwai, stable, linux-kernel,
Hui Wang, Alexander Tsoy, linux-mediatek, Matthias Brugger,
Johan Hovold, Jaroslav Kysela, Macpaul Lin, linux-arm-kernel
In-Reply-To: <s5h367cfsga.wl-tiwai@suse.de>
On Wed, 2020-06-03 at 10:45 +0200, Takashi Iwai wrote:
> On Wed, 03 Jun 2020 08:54:51 +0200,
> Takashi Iwai wrote:
> >
> > On Wed, 03 Jun 2020 08:28:09 +0200,
> > Takashi Iwai wrote:
> > >
> > > And, the most suspicious case is the last one,
> > > chip->num_suspended-intf. It means that the device has multiple
> > > USB interfaces and they went to suspend, while the resume isn't
> > > performed for the all suspended interfaces in return.
> >
> > If this is the cause, a patch like below might help.
> > It gets/puts the all assigned interfaced instead of only the primary
> > one.
>
> ... and considering of the problem again, rather the patch below might
> be the right answer. Now the driver tries to remember at which state
> it entered into the system-suspend. Upon resume, in return, when the
> state reaches back to that point, set the card state to D0.
>
> The previous patch can be applied on the top, too, and it might be
> worth to apply both.
>
> Let me know if any of those actually helps.
>
>
> Takashi
Thanks for your response so quickly.
I've just test this patch since it looks like enough for the issue.
This patch worked since the flag system_suspend will be set at the same
time when power state has been changed. I have 2 interface with the head
set. But actually the problem happened when primary one is suspended.
So I didn't test the earlier patch "suspend all interface instead of
only the primary one."
Will you resend this patch officially later? I think this solution is
required to send to stable, too. It's better to have it for other stable
kernel versions include android's.
> ---
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -843,9 +843,6 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
> if (chip == (void *)-1L)
> return 0;
>
> - chip->autosuspended = !!PMSG_IS_AUTO(message);
> - if (!chip->autosuspended)
> - snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
> if (!chip->num_suspended_intf++) {
> list_for_each_entry(as, &chip->pcm_list, list) {
> snd_usb_pcm_suspend(as);
> @@ -858,6 +855,11 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
> snd_usb_mixer_suspend(mixer);
> }
>
> + if (!PMSG_IS_AUTO(message) && !chip->system_suspend) {
> + snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
> + chip->system_suspend = chip->num_suspended_intf;
> + }
> +
> return 0;
> }
>
> @@ -871,10 +873,10 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
>
> if (chip == (void *)-1L)
> return 0;
> - if (--chip->num_suspended_intf)
> - return 0;
>
> atomic_inc(&chip->active); /* avoid autopm */
> + if (chip->num_suspended_intf > 1)
> + goto out;
>
> list_for_each_entry(as, &chip->pcm_list, list) {
> err = snd_usb_pcm_resume(as);
> @@ -896,9 +898,12 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
> snd_usbmidi_resume(p);
> }
>
> - if (!chip->autosuspended)
> + out:
> + if (chip->num_suspended_intf == chip->system_suspend) {
> snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0);
> - chip->autosuspended = 0;
> + chip->system_suspend = 0;
> + }
> + chip->num_suspended_intf--;
>
> err_out:
> atomic_dec(&chip->active); /* allow autopm after this point */
> diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> index 1c892c7f14d7..e0ebfb25fbd5 100644
> --- a/sound/usb/usbaudio.h
> +++ b/sound/usb/usbaudio.h
> @@ -26,7 +26,7 @@ struct snd_usb_audio {
> struct usb_interface *pm_intf;
> u32 usb_id;
> struct mutex mutex;
> - unsigned int autosuspended:1;
> + unsigned int system_suspend;
> atomic_t active;
> atomic_t shutdown;
> atomic_t usage_count;
>
> _______________________________________________
Thank you very much!
Best regards,
Macpaul Lin
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend
From: Takashi Iwai @ 2020-06-03 12:47 UTC (permalink / raw)
To: Macpaul Lin
Cc: alsa-devel, Szabolcs Szőke, Mediatek WSD Upstream,
Greg Kroah-Hartman, linux-usb, Takashi Iwai, stable, linux-kernel,
Hui Wang, Alexander Tsoy, linux-mediatek, Matthias Brugger,
Johan Hovold, Jaroslav Kysela, Macpaul Lin, linux-arm-kernel
In-Reply-To: <1591187964.23525.61.camel@mtkswgap22>
On Wed, 03 Jun 2020 14:39:24 +0200,
Macpaul Lin wrote:
>
> On Wed, 2020-06-03 at 10:45 +0200, Takashi Iwai wrote:
> > On Wed, 03 Jun 2020 08:54:51 +0200,
> > Takashi Iwai wrote:
> > >
> > > On Wed, 03 Jun 2020 08:28:09 +0200,
> > > Takashi Iwai wrote:
> > > >
> > > > And, the most suspicious case is the last one,
> > > > chip->num_suspended-intf. It means that the device has multiple
> > > > USB interfaces and they went to suspend, while the resume isn't
> > > > performed for the all suspended interfaces in return.
> > >
> > > If this is the cause, a patch like below might help.
> > > It gets/puts the all assigned interfaced instead of only the primary
> > > one.
> >
> > ... and considering of the problem again, rather the patch below might
> > be the right answer. Now the driver tries to remember at which state
> > it entered into the system-suspend. Upon resume, in return, when the
> > state reaches back to that point, set the card state to D0.
> >
> > The previous patch can be applied on the top, too, and it might be
> > worth to apply both.
> >
> > Let me know if any of those actually helps.
> >
> >
> > Takashi
>
> Thanks for your response so quickly.
> I've just test this patch since it looks like enough for the issue.
Good to hear!
> This patch worked since the flag system_suspend will be set at the same
> time when power state has been changed. I have 2 interface with the head
> set. But actually the problem happened when primary one is suspended.
Currently the autosuspend is set only to the primary interface; IOW,
the other interfaces will never get autosuspend, and the another
suspend-all-intf patch should improve that situation. But it won't
fix your actual bug, obviously :)
> So I didn't test the earlier patch "suspend all interface instead of
> only the primary one."
Could you try it one on top of the last patch? At least I'd like to
see whether it causes any regression.
> Will you resend this patch officially later? I think this solution is
> required to send to stable, too. It's better to have it for other stable
> kernel versions include android's.
Yes, that's a general bug and worth to be merged quickly.
I'm going to submit a proper patch soon later.
thanks,
Takashi
>
> > ---
> > diff --git a/sound/usb/card.c b/sound/usb/card.c
> > --- a/sound/usb/card.c
> > +++ b/sound/usb/card.c
> > @@ -843,9 +843,6 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
> > if (chip == (void *)-1L)
> > return 0;
> >
> > - chip->autosuspended = !!PMSG_IS_AUTO(message);
> > - if (!chip->autosuspended)
> > - snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
> > if (!chip->num_suspended_intf++) {
> > list_for_each_entry(as, &chip->pcm_list, list) {
> > snd_usb_pcm_suspend(as);
> > @@ -858,6 +855,11 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
> > snd_usb_mixer_suspend(mixer);
> > }
> >
> > + if (!PMSG_IS_AUTO(message) && !chip->system_suspend) {
> > + snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot);
> > + chip->system_suspend = chip->num_suspended_intf;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -871,10 +873,10 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
> >
> > if (chip == (void *)-1L)
> > return 0;
> > - if (--chip->num_suspended_intf)
> > - return 0;
> >
> > atomic_inc(&chip->active); /* avoid autopm */
> > + if (chip->num_suspended_intf > 1)
> > + goto out;
> >
> > list_for_each_entry(as, &chip->pcm_list, list) {
> > err = snd_usb_pcm_resume(as);
> > @@ -896,9 +898,12 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
> > snd_usbmidi_resume(p);
> > }
> >
> > - if (!chip->autosuspended)
> > + out:
> > + if (chip->num_suspended_intf == chip->system_suspend) {
> > snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0);
> > - chip->autosuspended = 0;
> > + chip->system_suspend = 0;
> > + }
> > + chip->num_suspended_intf--;
> >
> > err_out:
> > atomic_dec(&chip->active); /* allow autopm after this point */
> > diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> > index 1c892c7f14d7..e0ebfb25fbd5 100644
> > --- a/sound/usb/usbaudio.h
> > +++ b/sound/usb/usbaudio.h
> > @@ -26,7 +26,7 @@ struct snd_usb_audio {
> > struct usb_interface *pm_intf;
> > u32 usb_id;
> > struct mutex mutex;
> > - unsigned int autosuspended:1;
> > + unsigned int system_suspend;
> > atomic_t active;
> > atomic_t shutdown;
> > atomic_t usage_count;
> >
> > _______________________________________________
>
> Thank you very much!
>
> Best regards,
> Macpaul Lin
>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox