* [PATCH v4 3/4] dmaengine: mediatek-cqdma: fix compatible
From: EastL @ 2020-05-28 9:57 UTC (permalink / raw)
To: Sean Wang
Cc: mark.rutland, devicetree, wsd_upstream, linux-kernel, EastL,
dmaengine, vkoul, robh+dt, linux-mediatek, matthias.bgg,
linux-arm-kernel
In-Reply-To: <1590659832-31476-1-git-send-email-EastL.Lee@mediatek.com>
This patch fixes mediatek-cqdma compatible to common.
Signed-off-by: EastL <EastL.Lee@mediatek.com>
---
drivers/dma/mediatek/mtk-cqdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 905bbcb..bca7118 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -544,7 +544,7 @@ static void mtk_cqdma_hw_deinit(struct mtk_cqdma_device *cqdma)
}
static const struct of_device_id mtk_cqdma_match[] = {
- { .compatible = "mediatek,mt6765-cqdma" },
+ { .compatible = "mediatek,cqdma" },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mtk_cqdma_match);
--
1.9.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH] irqchip/irq-mtk-sysirq: replace spinlock with raw_spinlock
From: Bartosz Golaszewski @ 2020-05-28 9:35 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier, Matthias Brugger
Cc: Stephane Le Provost, Bartosz Golaszewski, Pedro Tsai,
linux-kernel, Fabien Parent, linux-mediatek, Andrew Perepech,
linux-arm-kernel
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This driver may take a regular spinlock when a raw spinlock
(irq_desc->lock) is already taken which results in the following
lockdep splat:
=============================
[ BUG: Invalid wait context ]
5.7.0-rc7 #1 Not tainted
-----------------------------
swapper/0/0 is trying to lock:
ffffff800303b798 (&chip_data->lock){....}-{3:3}, at: mtk_sysirq_set_type+0x48/0xc0
other info that might help us debug this:
context-{5:5}
2 locks held by swapper/0/0:
#0: ffffff800302ee68 (&desc->request_mutex){....}-{4:4}, at: __setup_irq+0xc4/0x8a0
#1: ffffff800302ecf0 (&irq_desc_lock_class){....}-{2:2}, at: __setup_irq+0xe4/0x8a0
stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7 #1
Hardware name: Pumpkin MT8516 (DT)
Call trace:
dump_backtrace+0x0/0x180
show_stack+0x14/0x20
dump_stack+0xd0/0x118
__lock_acquire+0x8c8/0x2270
lock_acquire+0xf8/0x470
_raw_spin_lock_irqsave+0x50/0x78
mtk_sysirq_set_type+0x48/0xc0
__irq_set_trigger+0x58/0x170
__setup_irq+0x420/0x8a0
request_threaded_irq+0xd8/0x190
timer_of_init+0x1e8/0x2c4
mtk_gpt_init+0x5c/0x1dc
timer_probe+0x74/0xf4
time_init+0x14/0x44
start_kernel+0x394/0x4f0
Replace the spinlock_t with raw_spinlock_t to avoid this warning.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/irqchip/irq-mtk-sysirq.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
index 73eae5966a40..6ff98b87e5c0 100644
--- a/drivers/irqchip/irq-mtk-sysirq.c
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -15,7 +15,7 @@
#include <linux/spinlock.h>
struct mtk_sysirq_chip_data {
- spinlock_t lock;
+ raw_spinlock_t lock;
u32 nr_intpol_bases;
void __iomem **intpol_bases;
u32 *intpol_words;
@@ -37,7 +37,7 @@ static int mtk_sysirq_set_type(struct irq_data *data, unsigned int type)
reg_index = chip_data->which_word[hwirq];
offset = hwirq & 0x1f;
- spin_lock_irqsave(&chip_data->lock, flags);
+ raw_spin_lock_irqsave(&chip_data->lock, flags);
value = readl_relaxed(base + reg_index * 4);
if (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_EDGE_FALLING) {
if (type == IRQ_TYPE_LEVEL_LOW)
@@ -53,7 +53,7 @@ static int mtk_sysirq_set_type(struct irq_data *data, unsigned int type)
data = data->parent_data;
ret = data->chip->irq_set_type(data, type);
- spin_unlock_irqrestore(&chip_data->lock, flags);
+ raw_spin_unlock_irqrestore(&chip_data->lock, flags);
return ret;
}
@@ -212,7 +212,7 @@ static int __init mtk_sysirq_of_init(struct device_node *node,
ret = -ENOMEM;
goto out_free_which_word;
}
- spin_lock_init(&chip_data->lock);
+ raw_spin_lock_init(&chip_data->lock);
return 0;
--
2.25.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH v10 3/3] configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
From: Marco Felsch @ 2020-05-28 9:26 UTC (permalink / raw)
To: Fengping Yu
Cc: Dmitry Torokhov, linux-mediatek, linux-input, Yingjoe Chen,
Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200528084143.36482-4-fengping.yu@mediatek.com>
On 20-05-28 16:41, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> Add Mediatek matrix keypad support in defconfig.
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 24e534d85045..112ced090b21 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -349,6 +349,7 @@ CONFIG_KEYBOARD_GPIO=y
> CONFIG_KEYBOARD_SNVS_PWRKEY=m
> CONFIG_KEYBOARD_IMX_SC_KEY=m
> CONFIG_KEYBOARD_CROS_EC=y
> +CONFIG_KEYBOARD_MTK_KPD=m
> CONFIG_INPUT_TOUCHSCREEN=y
> CONFIG_TOUCHSCREEN_ATMEL_MXT=m
> CONFIG_INPUT_MISC=y
> --
> 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 v10 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Marco Felsch @ 2020-05-28 9:26 UTC (permalink / raw)
To: Fengping Yu
Cc: Dmitry Torokhov, linux-mediatek, linux-input, Yingjoe Chen,
Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200528084143.36482-3-fengping.yu@mediatek.com>
On 20-05-28 16:41, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
>
> This adds matrix keypad support for Mediatek SoCs.
>
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> drivers/input/keyboard/Kconfig | 11 ++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 206 +++++++++++++++++++++++++++++++
> 3 files changed, 218 insertions(+)
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 28de965a08d5..c55230c4c344 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -782,6 +782,17 @@ config KEYBOARD_BCM
> To compile this driver as a module, choose M here: the
> module will be called bcm-keypad.
>
> +config KEYBOARD_MTK_KPD
> + tristate "MediaTek Keypad Support"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + select CONFIG_REGMAP_MMIO
> + select INPUT_MATRIXKMAP
> + help
> + Say Y here if you want to use the keypad on MediaTek SoCs.
> + If unsure, say N.
> + To compile this driver as a module, choose M here: the
> + module will be called mtk-kpd.
> +
> config KEYBOARD_MTK_PMIC
> tristate "MediaTek PMIC keys support"
> depends on MFD_MT6397
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 1d689fdd5c00..6c9d852c377e 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
> obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
> obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
> +obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
> obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
> obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
> obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
> diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
> new file mode 100644
> index 000000000000..2900487c8f60
> --- /dev/null
> +++ b/drivers/input/keyboard/mtk-kpd.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + * Author Terry Chang <terry.chang@mediatek.com>
> + */
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MTK_KPD_NAME "mtk-kpd"
> +#define MTK_KPD_MEM 0x0004
> +#define MTK_KPD_DEBOUNCE 0x0018
> +#define MTK_KPD_DEBOUNCE_MASK GENMASK(13, 0)
> +#define MTK_KPD_DEBOUNCE_MAX_US 256000
> +#define MTK_KPD_NUM_MEMS 5
> +#define MTK_KPD_NUM_BITS 136 /* 4*32+8 MEM5 only use 8 BITS */
> +
> +struct mtk_keypad {
> + struct regmap *regmap;
> + struct input_dev *input_dev;
> + struct clk *clk;
> + void __iomem *base;
> + u32 n_rows;
> + u32 n_cols;
> + DECLARE_BITMAP(keymap_state, MTK_KPD_NUM_BITS);
> +};
> +
> +static const struct regmap_config keypad_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 36,
> +};
> +
> +static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
> +{
> + struct mtk_keypad *keypad = dev_id;
> + unsigned short *keycode = keypad->input_dev->keycode;
> + DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
> + DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
> + int bit_nr;
> + int pressed;
> + unsigned short code;
> +
> + regmap_raw_read(keypad->regmap, MTK_KPD_MEM,
> + new_state, MTK_KPD_NUM_MEMS);
> +
> + bitmap_xor(change, new_state, keypad->keymap_state, MTK_KPD_NUM_BITS);
> +
> + for_each_set_bit(bit_nr, change, MTK_KPD_NUM_BITS) {
> + /* 1: not pressed, 0: pressed */
> + pressed = !test_bit(bit_nr, new_state);
> + dev_dbg(&keypad->input_dev->dev, "%s",
> + pressed ? "pressed" : "released");
> +
> + /* 32bit register only use low 16bit as keypad mem register */
> + code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
> +
> + input_report_key(keypad->input_dev, code, pressed);
> + input_sync(keypad->input_dev);
> +
> + dev_dbg(&keypad->input_dev->dev,
> + "report Linux keycode = %d\n", code);
> + }
> +
> + bitmap_copy(keypad->keymap_state, new_state, MTK_KPD_NUM_BITS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void kpd_clk_disable(void *data)
> +{
> + clk_disable_unprepare(data);
> +}
> +
> +static int kpd_pdrv_probe(struct platform_device *pdev)
> +{
> + struct mtk_keypad *keypad;
> + unsigned int irq;
> + u32 debounce;
> + bool wakeup;
> + int ret;
> +
> + keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> + if (!keypad)
> + return -ENOMEM;
> +
> + keypad->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(keypad->base))
> + return PTR_ERR(keypad->base);
> +
> + keypad->regmap = devm_regmap_init_mmio(&pdev->dev,
> + keypad->base,
> + &keypad_regmap_cfg);
> + if (IS_ERR(keypad->regmap)) {
> + dev_err(&pdev->dev,
> + "regmap init failed:%ld\n", PTR_ERR(keypad->regmap));
> + return PTR_ERR(keypad->regmap);
> + }
> +
> + bitmap_fill(keypad->keymap_state, MTK_KPD_NUM_BITS);
> +
> + keypad->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!keypad->input_dev) {
> + dev_err(&pdev->dev, "Failed to allocate input dev\n");
> + return -ENOMEM;
> + }
> +
> + keypad->input_dev->name = MTK_KPD_NAME;
> + keypad->input_dev->id.bustype = BUS_HOST;
> +
> + ret = matrix_keypad_parse_properties(&pdev->dev, &keypad->n_rows,
> + &keypad->n_cols);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to parse keypad params\n");
> + return ret;
> + }
> +
> + if (device_property_read_u32(&pdev->dev, "mediatek,debounce-us",
> + &debounce))
> + debounce = 16000;
> +
> + if (debounce > MTK_KPD_DEBOUNCE_MAX_US) {
> + dev_err(&pdev->dev, "Debounce time exceeds the maximum allowed time %dus\n",
> + MTK_KPD_DEBOUNCE_MAX_US);
> + return -EINVAL;
> + }
> +
> + wakeup = device_property_read_bool(&pdev->dev, "wakeup-source");
> +
> + dev_dbg(&pdev->dev, "n_row=%d n_col=%d debounce=%d\n",
> + keypad->n_rows, keypad->n_cols, debounce);
> +
> + ret = matrix_keypad_build_keymap(NULL, NULL,
> + keypad->n_rows,
> + keypad->n_cols,
> + NULL,
> + keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to build keymap\n");
> + return ret;
> + }
> +
> + regmap_write(keypad->regmap, MTK_KPD_DEBOUNCE,
> + debounce * 32 / 1000 & MTK_KPD_DEBOUNCE_MASK);
> +
> + keypad->clk = devm_clk_get(&pdev->dev, "kpd");
> + if (IS_ERR(keypad->clk))
> + return keypad->clk;
> +
> + ret = clk_prepare_enable(keypad->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(&pdev->dev, kpd_clk_disable,
> + keypad->clk);
> + if (ret)
> + return ret;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq,
> + NULL, kpd_irq_handler, 0,
> + MTK_KPD_NAME, keypad);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> + irq, ret);
> + return ret;
> + }
> +
> + ret = input_register_device(keypad->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register device\n");
> + return ret;
> + }
> +
> + return device_init_wakeup(&pdev->dev, wakeup);
> +}
> +
> +static const struct of_device_id kpd_of_match[] = {
> + { .compatible = "mediatek,mt6779-keypad" },
> + { .compatible = "mediatek,mt6873-keypad" },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver kpd_pdrv = {
> + .probe = kpd_pdrv_probe,
> + .driver = {
> + .name = MTK_KPD_NAME,
> + .of_match_table = kpd_of_match,
> + },
> +};
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.18.0
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v10 1/3] dt-bindings: Add keypad devicetree documentation
From: Marco Felsch @ 2020-05-28 9:24 UTC (permalink / raw)
To: Fengping Yu
Cc: Dmitry Torokhov, linux-mediatek, linux-input, Yingjoe Chen,
Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200528084143.36482-2-fengping.yu@mediatek.com>
On 20-05-28 16:41, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
...
> + mediatek,debounce-us:
> + description: Debounce interval in microseconds
> + maximum: 256000
Nit, I would mention that 16000 is the default value which is applied if
not given.
Appart of this feel free to add my:
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Regards,
Marco
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [RESEND PATCH v11 3/3] configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
From: Fengping Yu @ 2020-05-28 9:01 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528090144.54033-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
Add Mediatek matrix keypad support in defconfig.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 24e534d85045..112ced090b21 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -349,6 +349,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
CONFIG_KEYBOARD_CROS_EC=y
+CONFIG_KEYBOARD_MTK_KPD=m
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ATMEL_MXT=m
CONFIG_INPUT_MISC=y
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [RESEND PATCH v11] Add matrix keypad driver support for Mediatek SoCs
From: Fengping Yu @ 2020-05-28 9:01 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: linux-mediatek, linux-arm-kernel, linux-input
Change since v10:
- add wakeup source setting in probe function
fengping.yu (3):
dt-bindings: Add keypad devicetree documentation
drivers: input: keyboard: Add mtk keypad driver
configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
.../devicetree/bindings/input/mtk-kpd.yaml | 95 ++++++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 11 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 206 ++++++++++++++++++
5 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [RESEND PATCH v11] Add matrix keypad driver support for Mediatek SoCs
From: Fengping Yu @ 2020-05-28 9:07 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: linux-mediatek, linux-arm-kernel, linux-input
Change since v10:
- add wakeup source setting in probe function
fengping.yu (3):
dt-bindings: Add keypad devicetree documentation
drivers: input: keyboard: Add mtk keypad driver
configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
.../devicetree/bindings/input/mtk-kpd.yaml | 95 ++++++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 11 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 206 ++++++++++++++++++
5 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [RESEND PATCH v11 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Fengping Yu @ 2020-05-28 9:01 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528090144.54033-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
This adds matrix keypad support for Mediatek SoCs.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
drivers/input/keyboard/Kconfig | 11 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 206 +++++++++++++++++++++++++++++++
3 files changed, 218 insertions(+)
create mode 100644 drivers/input/keyboard/mtk-kpd.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 28de965a08d5..c55230c4c344 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -782,6 +782,17 @@ config KEYBOARD_BCM
To compile this driver as a module, choose M here: the
module will be called bcm-keypad.
+config KEYBOARD_MTK_KPD
+ tristate "MediaTek Keypad Support"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select CONFIG_REGMAP_MMIO
+ select INPUT_MATRIXKMAP
+ help
+ Say Y here if you want to use the keypad on MediaTek SoCs.
+ If unsure, say N.
+ To compile this driver as a module, choose M here: the
+ module will be called mtk-kpd.
+
config KEYBOARD_MTK_PMIC
tristate "MediaTek PMIC keys support"
depends on MFD_MT6397
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d689fdd5c00..6c9d852c377e 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
new file mode 100644
index 000000000000..2900487c8f60
--- /dev/null
+++ b/drivers/input/keyboard/mtk-kpd.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 MediaTek Inc.
+ * Author Terry Chang <terry.chang@mediatek.com>
+ */
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MTK_KPD_NAME "mtk-kpd"
+#define MTK_KPD_MEM 0x0004
+#define MTK_KPD_DEBOUNCE 0x0018
+#define MTK_KPD_DEBOUNCE_MASK GENMASK(13, 0)
+#define MTK_KPD_DEBOUNCE_MAX_US 256000
+#define MTK_KPD_NUM_MEMS 5
+#define MTK_KPD_NUM_BITS 136 /* 4*32+8 MEM5 only use 8 BITS */
+
+struct mtk_keypad {
+ struct regmap *regmap;
+ struct input_dev *input_dev;
+ struct clk *clk;
+ void __iomem *base;
+ u32 n_rows;
+ u32 n_cols;
+ DECLARE_BITMAP(keymap_state, MTK_KPD_NUM_BITS);
+};
+
+static const struct regmap_config keypad_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = sizeof(u32),
+ .max_register = 36,
+};
+
+static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
+{
+ struct mtk_keypad *keypad = dev_id;
+ unsigned short *keycode = keypad->input_dev->keycode;
+ DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
+ DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
+ int bit_nr;
+ int pressed;
+ unsigned short code;
+
+ regmap_raw_read(keypad->regmap, MTK_KPD_MEM,
+ new_state, MTK_KPD_NUM_MEMS);
+
+ bitmap_xor(change, new_state, keypad->keymap_state, MTK_KPD_NUM_BITS);
+
+ for_each_set_bit(bit_nr, change, MTK_KPD_NUM_BITS) {
+ /* 1: not pressed, 0: pressed */
+ pressed = !test_bit(bit_nr, new_state);
+ dev_dbg(&keypad->input_dev->dev, "%s",
+ pressed ? "pressed" : "released");
+
+ /* 32bit register only use low 16bit as keypad mem register */
+ code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
+
+ input_report_key(keypad->input_dev, code, pressed);
+ input_sync(keypad->input_dev);
+
+ dev_dbg(&keypad->input_dev->dev,
+ "report Linux keycode = %d\n", code);
+ }
+
+ bitmap_copy(keypad->keymap_state, new_state, MTK_KPD_NUM_BITS);
+
+ return IRQ_HANDLED;
+}
+
+static void kpd_clk_disable(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
+static int kpd_pdrv_probe(struct platform_device *pdev)
+{
+ struct mtk_keypad *keypad;
+ unsigned int irq;
+ u32 debounce;
+ bool wakeup;
+ int ret;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ keypad->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(keypad->base))
+ return PTR_ERR(keypad->base);
+
+ keypad->regmap = devm_regmap_init_mmio(&pdev->dev,
+ keypad->base,
+ &keypad_regmap_cfg);
+ if (IS_ERR(keypad->regmap)) {
+ dev_err(&pdev->dev,
+ "regmap init failed:%ld\n", PTR_ERR(keypad->regmap));
+ return PTR_ERR(keypad->regmap);
+ }
+
+ bitmap_fill(keypad->keymap_state, MTK_KPD_NUM_BITS);
+
+ keypad->input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!keypad->input_dev) {
+ dev_err(&pdev->dev, "Failed to allocate input dev\n");
+ return -ENOMEM;
+ }
+
+ keypad->input_dev->name = MTK_KPD_NAME;
+ keypad->input_dev->id.bustype = BUS_HOST;
+
+ ret = matrix_keypad_parse_properties(&pdev->dev, &keypad->n_rows,
+ &keypad->n_cols);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to parse keypad params\n");
+ return ret;
+ }
+
+ if (device_property_read_u32(&pdev->dev, "mediatek,debounce-us",
+ &debounce))
+ debounce = 16000;
+
+ if (debounce > MTK_KPD_DEBOUNCE_MAX_US) {
+ dev_err(&pdev->dev, "Debounce time exceeds the maximum allowed time %dus\n",
+ MTK_KPD_DEBOUNCE_MAX_US);
+ return -EINVAL;
+ }
+
+ wakeup = device_property_read_bool(&pdev->dev, "wakeup-source");
+
+ dev_dbg(&pdev->dev, "n_row=%d n_col=%d debounce=%d\n",
+ keypad->n_rows, keypad->n_cols, debounce);
+
+ ret = matrix_keypad_build_keymap(NULL, NULL,
+ keypad->n_rows,
+ keypad->n_cols,
+ NULL,
+ keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to build keymap\n");
+ return ret;
+ }
+
+ regmap_write(keypad->regmap, MTK_KPD_DEBOUNCE,
+ debounce * 32 / 1000 & MTK_KPD_DEBOUNCE_MASK);
+
+ keypad->clk = devm_clk_get(&pdev->dev, "kpd");
+ if (IS_ERR(keypad->clk))
+ return keypad->clk;
+
+ ret = clk_prepare_enable(keypad->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(&pdev->dev, kpd_clk_disable,
+ keypad->clk);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq,
+ NULL, kpd_irq_handler, 0,
+ MTK_KPD_NAME, keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
+ irq, ret);
+ return ret;
+ }
+
+ ret = input_register_device(keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register device\n");
+ return ret;
+ }
+
+ return device_init_wakeup(&pdev->dev, wakeup);
+}
+
+static const struct of_device_id kpd_of_match[] = {
+ { .compatible = "mediatek,mt6779-keypad" },
+ { .compatible = "mediatek,mt6873-keypad" },
+ { /* sentinel */ }
+};
+
+static struct platform_driver kpd_pdrv = {
+ .probe = kpd_pdrv_probe,
+ .driver = {
+ .name = MTK_KPD_NAME,
+ .of_match_table = kpd_of_match,
+ },
+};
+module_platform_driver(kpd_pdrv);
+
+MODULE_AUTHOR("Mediatek Corporation");
+MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
+MODULE_LICENSE("GPL");
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [RESEND PATCH v11 1/3] dt-bindings: Add keypad devicetree documentation
From: Fengping Yu @ 2020-05-28 9:01 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528090144.54033-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
Add Mediatek matrix keypad dt-bindings doc as yaml schema.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
.../devicetree/bindings/input/mtk-kpd.yaml | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.yaml b/Documentation/devicetree/bindings/input/mtk-kpd.yaml
new file mode 100644
index 000000000000..586cd196dd00
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mtk-kpd.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+version: 1
+
+$id: http://devicetree.org/schemas/input/mtk-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek's Keypad Controller device tree bindings
+
+maintainer:
+ - Fengping Yu <fengping.yu@mediatek.com>
+
+description: |
+ Mediatek's Keypad controller is used to interface a SoC with a matrix-type
+ keypad device. The keypad controller supports multiple row and column lines.
+ A key can be placed at each intersection of a unique row and a unique column.
+ The keypad controller can sense a key-press and key-release and report the
+ event using a interrupt to the cpu.
+
+properties:
+ compatible:
+ oneOf:
+ - const: "mediatek,mt6779-keypad"
+ - const: "mediatek,mt6873-keypad"
+
+ clock-names:
+ description: Names of the clocks listed in clocks property in the same order
+ maxItems: 1
+
+ clocks:
+ description: Must contain one entry, for the module clock
+ refs: devicetree/bindings/clocks/clock-bindings.txt for details.
+
+ interrupts:
+ description: A single interrupt specifier
+ maxItems: 1
+
+ linux,keymap:
+ description: The keymap for keys as described in the binding document
+ refs: devicetree/bindings/input/matrix-keymap.txt
+ minItems: 1
+ maxItems: 16
+
+ pinctrl-0:
+ description: Specify pin control groups used for this controller
+ refs: devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+ pinctrl-names:
+ description: Names for optional pin modes
+ maxItems: 1
+
+ reg:
+ description: The base address of the Keypad register bank
+ maxItems: 1
+
+ wakeup-source:
+ description: use any event on keypad as wakeup event
+ type: boolean
+
+ keypad,num-columns:
+ description: Number of column lines connected to the keypad controller,
+ it is not equal to PCB columns number, instead you should add required value
+ for each IC
+
+ keypad,num-rows:
+ description: Number of row lines connected to the keypad controller, it is
+ not equal to PCB rows number, instead you should add required value for each IC
+
+ mediatek,debounce-us:
+ description: Debounce interval in microseconds
+ maximum: 256000
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - linux,keymap
+ - pinctrl
+ - clocks
+ - clock-names
+
+examples:
+ - |
+
+ keypad: kp@10010000 {
+ compatible = "mediatek,mt6779-keypad";
+ reg = <0 0x10010000 0 0x1000>;
+ linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
+ clocks = <&clk26m>;
+ clock-names = "kpd";
+ pinctrl-names = "default";
+ pinctrl-0 = <&kpd_gpios_def_cfg>;
+ };
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH v10 3/3] configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
From: Fengping Yu @ 2020-05-28 8:41 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528084143.36482-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
Add Mediatek matrix keypad support in defconfig.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 24e534d85045..112ced090b21 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -349,6 +349,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
CONFIG_KEYBOARD_CROS_EC=y
+CONFIG_KEYBOARD_MTK_KPD=m
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ATMEL_MXT=m
CONFIG_INPUT_MISC=y
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH v10 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Fengping Yu @ 2020-05-28 8:41 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528084143.36482-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
This adds matrix keypad support for Mediatek SoCs.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
drivers/input/keyboard/Kconfig | 11 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 206 +++++++++++++++++++++++++++++++
3 files changed, 218 insertions(+)
create mode 100644 drivers/input/keyboard/mtk-kpd.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 28de965a08d5..c55230c4c344 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -782,6 +782,17 @@ config KEYBOARD_BCM
To compile this driver as a module, choose M here: the
module will be called bcm-keypad.
+config KEYBOARD_MTK_KPD
+ tristate "MediaTek Keypad Support"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select CONFIG_REGMAP_MMIO
+ select INPUT_MATRIXKMAP
+ help
+ Say Y here if you want to use the keypad on MediaTek SoCs.
+ If unsure, say N.
+ To compile this driver as a module, choose M here: the
+ module will be called mtk-kpd.
+
config KEYBOARD_MTK_PMIC
tristate "MediaTek PMIC keys support"
depends on MFD_MT6397
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d689fdd5c00..6c9d852c377e 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
new file mode 100644
index 000000000000..2900487c8f60
--- /dev/null
+++ b/drivers/input/keyboard/mtk-kpd.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 MediaTek Inc.
+ * Author Terry Chang <terry.chang@mediatek.com>
+ */
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MTK_KPD_NAME "mtk-kpd"
+#define MTK_KPD_MEM 0x0004
+#define MTK_KPD_DEBOUNCE 0x0018
+#define MTK_KPD_DEBOUNCE_MASK GENMASK(13, 0)
+#define MTK_KPD_DEBOUNCE_MAX_US 256000
+#define MTK_KPD_NUM_MEMS 5
+#define MTK_KPD_NUM_BITS 136 /* 4*32+8 MEM5 only use 8 BITS */
+
+struct mtk_keypad {
+ struct regmap *regmap;
+ struct input_dev *input_dev;
+ struct clk *clk;
+ void __iomem *base;
+ u32 n_rows;
+ u32 n_cols;
+ DECLARE_BITMAP(keymap_state, MTK_KPD_NUM_BITS);
+};
+
+static const struct regmap_config keypad_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = sizeof(u32),
+ .max_register = 36,
+};
+
+static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
+{
+ struct mtk_keypad *keypad = dev_id;
+ unsigned short *keycode = keypad->input_dev->keycode;
+ DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
+ DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
+ int bit_nr;
+ int pressed;
+ unsigned short code;
+
+ regmap_raw_read(keypad->regmap, MTK_KPD_MEM,
+ new_state, MTK_KPD_NUM_MEMS);
+
+ bitmap_xor(change, new_state, keypad->keymap_state, MTK_KPD_NUM_BITS);
+
+ for_each_set_bit(bit_nr, change, MTK_KPD_NUM_BITS) {
+ /* 1: not pressed, 0: pressed */
+ pressed = !test_bit(bit_nr, new_state);
+ dev_dbg(&keypad->input_dev->dev, "%s",
+ pressed ? "pressed" : "released");
+
+ /* 32bit register only use low 16bit as keypad mem register */
+ code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
+
+ input_report_key(keypad->input_dev, code, pressed);
+ input_sync(keypad->input_dev);
+
+ dev_dbg(&keypad->input_dev->dev,
+ "report Linux keycode = %d\n", code);
+ }
+
+ bitmap_copy(keypad->keymap_state, new_state, MTK_KPD_NUM_BITS);
+
+ return IRQ_HANDLED;
+}
+
+static void kpd_clk_disable(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
+static int kpd_pdrv_probe(struct platform_device *pdev)
+{
+ struct mtk_keypad *keypad;
+ unsigned int irq;
+ u32 debounce;
+ bool wakeup;
+ int ret;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ keypad->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(keypad->base))
+ return PTR_ERR(keypad->base);
+
+ keypad->regmap = devm_regmap_init_mmio(&pdev->dev,
+ keypad->base,
+ &keypad_regmap_cfg);
+ if (IS_ERR(keypad->regmap)) {
+ dev_err(&pdev->dev,
+ "regmap init failed:%ld\n", PTR_ERR(keypad->regmap));
+ return PTR_ERR(keypad->regmap);
+ }
+
+ bitmap_fill(keypad->keymap_state, MTK_KPD_NUM_BITS);
+
+ keypad->input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!keypad->input_dev) {
+ dev_err(&pdev->dev, "Failed to allocate input dev\n");
+ return -ENOMEM;
+ }
+
+ keypad->input_dev->name = MTK_KPD_NAME;
+ keypad->input_dev->id.bustype = BUS_HOST;
+
+ ret = matrix_keypad_parse_properties(&pdev->dev, &keypad->n_rows,
+ &keypad->n_cols);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to parse keypad params\n");
+ return ret;
+ }
+
+ if (device_property_read_u32(&pdev->dev, "mediatek,debounce-us",
+ &debounce))
+ debounce = 16000;
+
+ if (debounce > MTK_KPD_DEBOUNCE_MAX_US) {
+ dev_err(&pdev->dev, "Debounce time exceeds the maximum allowed time %dus\n",
+ MTK_KPD_DEBOUNCE_MAX_US);
+ return -EINVAL;
+ }
+
+ wakeup = device_property_read_bool(&pdev->dev, "wakeup-source");
+
+ dev_dbg(&pdev->dev, "n_row=%d n_col=%d debounce=%d\n",
+ keypad->n_rows, keypad->n_cols, debounce);
+
+ ret = matrix_keypad_build_keymap(NULL, NULL,
+ keypad->n_rows,
+ keypad->n_cols,
+ NULL,
+ keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to build keymap\n");
+ return ret;
+ }
+
+ regmap_write(keypad->regmap, MTK_KPD_DEBOUNCE,
+ debounce * 32 / 1000 & MTK_KPD_DEBOUNCE_MASK);
+
+ keypad->clk = devm_clk_get(&pdev->dev, "kpd");
+ if (IS_ERR(keypad->clk))
+ return keypad->clk;
+
+ ret = clk_prepare_enable(keypad->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(&pdev->dev, kpd_clk_disable,
+ keypad->clk);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq,
+ NULL, kpd_irq_handler, 0,
+ MTK_KPD_NAME, keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
+ irq, ret);
+ return ret;
+ }
+
+ ret = input_register_device(keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register device\n");
+ return ret;
+ }
+
+ return device_init_wakeup(&pdev->dev, wakeup);
+}
+
+static const struct of_device_id kpd_of_match[] = {
+ { .compatible = "mediatek,mt6779-keypad" },
+ { .compatible = "mediatek,mt6873-keypad" },
+ { /* sentinel */ }
+};
+
+static struct platform_driver kpd_pdrv = {
+ .probe = kpd_pdrv_probe,
+ .driver = {
+ .name = MTK_KPD_NAME,
+ .of_match_table = kpd_of_match,
+ },
+};
+module_platform_driver(kpd_pdrv);
+
+MODULE_AUTHOR("Mediatek Corporation");
+MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
+MODULE_LICENSE("GPL");
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH v10 1/3] dt-bindings: Add keypad devicetree documentation
From: Fengping Yu @ 2020-05-28 8:41 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528084143.36482-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
Add Mediatek matrix keypad dt-bindings doc as yaml schema.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
.../devicetree/bindings/input/mtk-kpd.yaml | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.yaml b/Documentation/devicetree/bindings/input/mtk-kpd.yaml
new file mode 100644
index 000000000000..586cd196dd00
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mtk-kpd.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+version: 1
+
+$id: http://devicetree.org/schemas/input/mtk-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek's Keypad Controller device tree bindings
+
+maintainer:
+ - Fengping Yu <fengping.yu@mediatek.com>
+
+description: |
+ Mediatek's Keypad controller is used to interface a SoC with a matrix-type
+ keypad device. The keypad controller supports multiple row and column lines.
+ A key can be placed at each intersection of a unique row and a unique column.
+ The keypad controller can sense a key-press and key-release and report the
+ event using a interrupt to the cpu.
+
+properties:
+ compatible:
+ oneOf:
+ - const: "mediatek,mt6779-keypad"
+ - const: "mediatek,mt6873-keypad"
+
+ clock-names:
+ description: Names of the clocks listed in clocks property in the same order
+ maxItems: 1
+
+ clocks:
+ description: Must contain one entry, for the module clock
+ refs: devicetree/bindings/clocks/clock-bindings.txt for details.
+
+ interrupts:
+ description: A single interrupt specifier
+ maxItems: 1
+
+ linux,keymap:
+ description: The keymap for keys as described in the binding document
+ refs: devicetree/bindings/input/matrix-keymap.txt
+ minItems: 1
+ maxItems: 16
+
+ pinctrl-0:
+ description: Specify pin control groups used for this controller
+ refs: devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+ pinctrl-names:
+ description: Names for optional pin modes
+ maxItems: 1
+
+ reg:
+ description: The base address of the Keypad register bank
+ maxItems: 1
+
+ wakeup-source:
+ description: use any event on keypad as wakeup event
+ type: boolean
+
+ keypad,num-columns:
+ description: Number of column lines connected to the keypad controller,
+ it is not equal to PCB columns number, instead you should add required value
+ for each IC
+
+ keypad,num-rows:
+ description: Number of row lines connected to the keypad controller, it is
+ not equal to PCB rows number, instead you should add required value for each IC
+
+ mediatek,debounce-us:
+ description: Debounce interval in microseconds
+ maximum: 256000
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - linux,keymap
+ - pinctrl
+ - clocks
+ - clock-names
+
+examples:
+ - |
+
+ keypad: kp@10010000 {
+ compatible = "mediatek,mt6779-keypad";
+ reg = <0 0x10010000 0 0x1000>;
+ linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
+ clocks = <&clk26m>;
+ clock-names = "kpd";
+ pinctrl-names = "default";
+ pinctrl-0 = <&kpd_gpios_def_cfg>;
+ };
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH v11] Add matrix keypad driver support for Mediatek SoCs
From: Fengping Yu @ 2020-05-28 8:41 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: linux-mediatek, linux-arm-kernel, linux-input
Change since v10:
- add wakeup source setting in probe function
fengping.yu (3):
dt-bindings: Add keypad devicetree documentation
drivers: input: keyboard: Add mtk keypad driver
configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
.../devicetree/bindings/input/mtk-kpd.yaml | 95 ++++++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 11 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 206 ++++++++++++++++++
5 files changed, 314 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
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 v10] Add matrix keypad driver support for Mediatek SoCs
From: fengping.yu @ 2020-05-28 8:21 UTC (permalink / raw)
To: Marco Felsch
Cc: Dmitry Torokhov, linux-mediatek, linux-input, Yingjoe Chen,
Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200528071916.2zxch46lbimxet2u@pengutronix.de>
Hi Marco:
I am sorry, I think I misunderstand your suggestion about wakeup member
in patch v9.
Keypad is a wakeup source, so I mis-remove wakeup source setting in the
probe function.
I will modify this in the next patch.
Thanks.
On Thu, 2020-05-28 at 09:19 +0200, Marco Felsch wrote:
> Hi,
>
> this version looks nice to me =) One last question.
>
> On 20-05-28 13:33, Fengping Yu wrote:
> >
> > Change since v9:
> > - modify KEYBOARD_MTK_KPD config dependent item
> > - remove wakeup member of mtk_keypad struct
>
> You also removed the device wakeup capability completely, was this
> intended? What I mean is that we don't need that member within the
> driver state struct.
>
> Regards,
> Marco
>
> > - remove default pinctrl state set
> > - modify request irq failed return value
> > - add space of kepad matching table
> > - modify align issue
> >
> > fengping.yu (3):
> > dt-bindings: Add keypad devicetree documentation
> > drivers: input: keyboard: Add mtk keypad driver
> > configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
> >
> > .../devicetree/bindings/input/mtk-kpd.yaml | 95 +++++++++
> > arch/arm64/configs/defconfig | 1 +
> > drivers/input/keyboard/Kconfig | 11 +
> > drivers/input/keyboard/Makefile | 1 +
> > drivers/input/keyboard/mtk-kpd.c | 201 ++++++++++++++++++
> > 5 files changed, 309 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
> > create mode 100644 drivers/input/keyboard/mtk-kpd.c
> >
> > --
> > 2.18.0
> >
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
_______________________________________________
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-05-28 8:04 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Rob Herring, Andy Shevchenko, srv_heupstream,
devicetree, Linus Walleij,
Shengnan Wang (王圣男), Tomasz Figa,
Bartosz Golaszewski, Sj Huang, Nicolas Boichat,
moderated list:ARM/Mediatek SoC support, 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: <20200528072332.GW7618@paasikivi.fi.intel.com>
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.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [BUG] Cannot boot on MT8173 if Mediatek thermal is enabled
From: Enric Balletbo i Serra @ 2020-05-28 8:08 UTC (permalink / raw)
To: Michael Kao
Cc: drinkcat@chromium.org, roger.lu, renze,
moderated list:ARM/Mediatek SoC support, Hsin-Yi Wang,
Matthias Brugger
In-Reply-To: <1590634780.22554.1.camel@mtksdccf07>
Hi Michael,
On 28/5/20 4:59, Michael Kao wrote:
> On Wed, 2020-05-20 at 18:12 +0200, Enric Balletbo i Serra wrote:
>> Hi Matthias et all,
>>
>> On 20/5/20 17:25, Enric Balletbo i Serra wrote:
>>>
>>>
>>> On 20/5/20 17:21, Matthias Brugger wrote:
>>>>
>>>>
>>>> On 20/05/2020 17:09, Enric Balletbo i Serra wrote:
>>>>> Dear all,
>>>>>
>>>>> I've been testing the Acer Chromebook R 13 (elm - MT8173) for a while. Today I
>>>>> enabled the Mediatek thermal driver (CONFIG_MTK_THERMAL=y) and I started to get
>>>>> that hang [1]
>>>>>
>>>>
>>>> Did you try to bisect to find out what broke it?
>>>>
>>>
>>> I don't even know if this worked at some point, I was running/testing my kernels
>>> with CONFIG_MTK_THERMAL disabled. From the log doesn't seem to have a lot of
>>> changes so I suspect this issue is there since long time.
>>>
>>
>> So the commit that introduces the problem is:
>>
>> commit eb9aecd90d1a39601e91cd08b90d5fee51d321a6
>> Author: Michael Kao <michael.kao@mediatek.com>
>> Date: Fri Feb 1 15:38:07 2019 +0800
>>
>> thermal: mediatek: fix register index error
>>
>> The index of msr and adcpnp should match the sensor
>> which belongs to the selected bank in the for loop.
>>
>> Fixes: b7cf0053738c ("thermal: Add Mediatek thermal driver for mt2701.")
>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>> Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
>>
>>
>>>
>>>> Regards,
>>>> Matthias
>>>>
>>>>> The stacktrace points point to this function:
>>>>>
>>>>> static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>>>>>
>>>>> More precisely to this call:
>>>>>
>>>>> raw = readl(mt->thermal_base +
>>>>> conf->msr[conf->bank_data[bank->id].sensors[i]]);
>>>>>
>>>>> this call, is in a loop and ends trying to access to conf->msr[4]
>>>>> (conf->msr[MT8173_TSABB]) which doesn't exist as per the following struct
>>>>>
>>>>> static const int mt8173_msr[MT8173_NUM_SENSORS_PER_ZONE] = {
>>>>> TEMP_MSR0, TEMP_MSR1, TEMP_MSR2, TEMP_MSR3
>>>>> };
>>>>>
>>>>> I think the datasheet will help here to clarify what is happening but is not
>>>>> public, so I can really check. Anyway seems that or the mt8173_msr struct is
>>>>> wrong or the mt8173_bank_data is wrong or there is something else.
>>>>>
>>>>> Could anyone with the information or with this hardwware knowledge take a look,
>>>>> please.
>>>>>
>>>>> Thanks,
>>>>> Enric
>>>>>
>>>>>
>>>>> [1]
>>>>> [ 2.222488] Unable to handle kernel paging request at virtual address
>>>>> ffff8000125f5001
>>>>> [ 2.230421] Mem abort info:
>>>>> [ 2.233207] ESR = 0x96000021
>>>>> [ 2.236261] EC = 0x25: DABT (current EL), IL = 32 bits
>>>>> [ 2.241571] SET = 0, FnV = 0
>>>>> [ 2.244623] EA = 0, S1PTW = 0
>>>>> [ 2.247762] Data abort info:
>>>>> [ 2.250640] ISV = 0, ISS = 0x00000021
>>>>> [ 2.254473] CM = 0, WnR = 0
>>>>> [ 2.257544] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000041850000
>>>>> [ 2.264251] [ffff8000125f5001] pgd=000000013ffff003, pud=000000013fffe003,
>>>>> pmd=000000013fff9003, pte=006800001100b707
>>>>> [ 2.274867] Internal error: Oops: 96000021 [#1] PREEMPT SMP
>>>>> [ 2.280432] Modules linked in:
>>>>> [ 2.283483] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc6+ #162
>>>>> [ 2.289914] Hardware name: Google Elm (DT)
>>>>> [ 2.294003] pstate: 20000005 (nzCv daif -PAN -UAO)
>>>>> [ 2.298792] pc : mtk_read_temp+0xb8/0x1c8
>>>>> [ 2.302793] lr : mtk_read_temp+0x7c/0x1c8
>>>>> [ 2.306794] sp : ffff80001003b930
>>>>> [ 2.310100] x29: ffff80001003b930 x28: 0000000000000000
>>>>> [ 2.315404] x27: 0000000000000002 x26: ffff0000f9550b10
>>>>> [ 2.320709] x25: ffff0000f9550a80 x24: 0000000000000090
>>>>> [ 2.326014] x23: ffff80001003ba24 x22: 00000000610344c0
>>>>> [ 2.331318] x21: 0000000000002710 x20: 00000000000001f4
>>>>> [ 2.336622] x19: 0000000000030d40 x18: ffff800011742ec0
>>>>> [ 2.341926] x17: 0000000000000001 x16: 0000000000000001
>>>>> [ 2.347230] x15: ffffffffffffffff x14: ffffff0000000000
>>>>> [ 2.352535] x13: ffffffffffffffff x12: 0000000000000028
>>>>> [ 2.357839] x11: 0000000000000003 x10: ffff800011295ec8
>>>>> [ 2.363143] x9 : 000000000000291b x8 : 0000000000000002
>>>>> [ 2.368447] x7 : 00000000000000a8 x6 : 0000000000000004
>>>>> [ 2.373751] x5 : 0000000000000000 x4 : ffff800011295cb0
>>>>> [ 2.379055] x3 : 0000000000000002 x2 : ffff8000125f5001
>>>>> [ 2.384359] x1 : 0000000000000001 x0 : ffff0000f9550a80
>>>>> [ 2.389665] Call trace:
>>>>> [ 2.392105] mtk_read_temp+0xb8/0x1c8
>>>>> [ 2.395760] of_thermal_get_temp+0x2c/0x40
>>>>> [ 2.399849] thermal_zone_get_temp+0x78/0x160
>>>>> [ 2.404198] thermal_zone_device_update.part.0+0x3c/0x1f8
>>>>> [ 2.409589] thermal_zone_device_update+0x34/0x48
>>>>> [ 2.414286] of_thermal_set_mode+0x58/0x88
>>>>> [ 2.418375] thermal_zone_of_sensor_register+0x1a8/0x1d8
>>>>> [ 2.423679] devm_thermal_zone_of_sensor_register+0x64/0xb0
>>>>> [ 2.429242] mtk_thermal_probe+0x690/0x7d0
>>>>> [ 2.433333] platform_drv_probe+0x5c/0xb0
>>>>> [ 2.437335] really_probe+0xe4/0x448
>>>>> [ 2.440901] driver_probe_device+0xe8/0x140
>>>>> [ 2.445077] device_driver_attach+0x7c/0x88
>>>>> [ 2.449252] __driver_attach+0xac/0x178
>>>>> [ 2.453082] bus_for_each_dev+0x78/0xc8
>>>>> [ 2.456909] driver_attach+0x2c/0x38
>>>>> [ 2.460476] bus_add_driver+0x14c/0x230
>>>>> [ 2.464304] driver_register+0x6c/0x128
>>>>> [ 2.468131] __platform_driver_register+0x50/0x60
>>>>> [ 2.472831] mtk_thermal_driver_init+0x24/0x30
>>>>> [ 2.477268] do_one_initcall+0x50/0x298
>>>>> [ 2.481098] kernel_init_freeable+0x1ec/0x264
>>>>> [ 2.485450] kernel_init+0x1c/0x110
>>>>> [ 2.488931] ret_from_fork+0x10/0x1c
>>>>> [ 2.492502] Code: f9401081 f9400402 b8a67821 8b010042 (b9400042)
>>>>> [ 2.498599] ---[ end trace e43e3105ed27dc99 ]---
>>>>> [ 2.503367] Kernel panic - not syncing: Attempted to kill init!
>>>>> exitcode=0x0000000b
>>>>> [ 2.511020] SMP: stopping secondary CPUs
>>>>> [ 2.514941] Kernel Offset: disabled
>>>>> [ 2.518421] CPU features: 0x090002,25006005
>>>>> [ 2.522595] Memory Limit: none
>>>>> [ 2.525644] ---[ end Kernel panic - not syncing: Attempted to kill init!
>>>>> exitcode=0x0000000b ]---
>>>>>
>
>> Hi Enric,
> I will help to fix this crash. If it is urgent, you can revert the patch
> locally first for mt8173.
>
Thanks, the reverted patch is what I am carrying ;-)
It'd be nice have this fixed for next MR or during the upcoming release cycle,
it will probably start next week. During the next merge window will land the
support for MT8173 Elm and Hana, so those boards will be affected by this and
will break. Actually, there is no much boards supported in mainline using MT8183
(only EVB), so in the worse case I can send a revert or a partial revert of the
patch.
Thanks,
Enric
> The last sensor in mt8173_bank_data[2] is MT8173_TSABB.
> Its index is 4. But there is not index 4 in mt8173_msr and
> mt8173_adcpnp.
> That is the root cause for your reference.
>
> static const struct mtk_thermal_data mt8173_thermal_data = {
> .auxadc_channel = MT8173_TEMP_AUXADC_CHANNEL,
> .num_banks = MT8173_NUM_ZONES,
> .num_sensors = MT8173_NUM_SENSORS,
> .vts_index = mt8173_vts_index,
> .cali_val = MT8173_CALIBRATION,
> .num_controller = MT8173_NUM_CONTROLLER,
> .controller_offset = mt8173_tc_offset,
> .need_switch_bank = true,
> .bank_data = {
> {
> .num_sensors = 2,
> .sensors = mt8173_bank_data[0],
> }, {
> .num_sensors = 2,
> .sensors = mt8173_bank_data[1],
> }, {
> .num_sensors = 3,
> .sensors = mt8173_bank_data[2],
> }, {
> .num_sensors = 1,
> .sensors = mt8173_bank_data[3],
> },
> },
> .msr = mt8173_msr,
> .adcpnp = mt8173_adcpnp,
> .sensor_mux_values = mt8173_mux_values,
> };
>
>
>
> /* MT8173 thermal sensor data */
> static const int mt8173_bank_data[MT8173_NUM_ZONES][3] = {
> { MT8173_TS2, MT8173_TS3 },
> { MT8173_TS2, MT8173_TS4 },
> { MT8173_TS1, MT8173_TS2, MT8173_TSABB },
> { MT8173_TS2 },
> };
>
> static const int mt8173_msr[MT8173_NUM_SENSORS_PER_ZONE] = {
> TEMP_MSR0, TEMP_MSR1, TEMP_MSR2, TEMP_MSR3
> };
>
> static const int mt8173_adcpnp[MT8173_NUM_SENSORS_PER_ZONE] = {
> TEMP_ADCPNP0, TEMP_ADCPNP1, TEMP_ADCPNP2, TEMP_ADCPNP3
> };
>
> /* MT8173 thermal sensors */
> #define MT8173_TS1 0
> #define MT8173_TS2 1
> #define MT8173_TS3 2
> #define MT8173_TS4 3
> #define MT8173_TSABB 4
>
> Best Regards,
> Michael
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH 09/12] devfreq: add mediatek cci devfreq
From: Chanwoo Choi @ 2020-05-28 8:00 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, linux-arm-kernel
In-Reply-To: <c39e4f30-805a-78c4-b1c4-e55a03e2408e@samsung.com>
Hi Andrew-sh.Cheng,
On 5/28/20 4:35 PM, Chanwoo Choi wrote:
> Hi Andrew-sh.Cheng,
>
> On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
>> This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
>> of the Mediatek MT8183.
>>
>> On the MT8183 the CCI is supplied by the same regulator as the LITTLE
>> cores. The driver is notified when the regulator voltage changes
>> (driven by cpufreq) and adjusts the CCI frequency to the maximum
>> possible value.
I understood that the mt8183-cci.c and mt8183 cpufreq driver (ARM_MEDIATEK_CPUFREQ)
shared the same regulator. So, when mt8183 cpufreq driver
changes the CPU frequency and voltage, the mt8183-cci.c
changes the CCI frequency according to the new mt8183 frequency
with passive governor.
I think that mt8183-cci.c don't need to change the voltage
because already mt8183 cpufreq changed the voltage of shared regulator.
Why do you change the voltage in this driver?
>>
>> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
>> ---
>> drivers/devfreq/Kconfig | 10 ++
>> drivers/devfreq/Makefile | 1 +
>> drivers/devfreq/mt8183-cci-devfreq.c | 206 +++++++++++++++++++++++++++++++++++
>
> The mt8183-cci.c is enough for driver name.
>
>> 3 files changed, 217 insertions(+)
>> create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index d9067950af6a..4ed7116271ee 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -103,6 +103,16 @@ config ARM_IMX8M_DDRC_DEVFREQ
>> This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>> adjusting DRAM frequency.
>>
>> +config ARM_MT8183_CCI_DEVFREQ
>> + tristate "MT8183 CCI DEVFREQ Driver"
>> + depends on ARM_MEDIATEK_CPUFREQ
>> + help
>> + This adds a devfreq driver for Cache Coherent Interconnect
>> + of Mediatek MT8183, which is shared the same regulator
>> + with cpu cluster.
>> + It can track buck voltage and update a proper cci frequency.
>
> s/cci/CCI
>
>> + Use notification to get regulator status.
>> +
>> config ARM_TEGRA_DEVFREQ
>> tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>> depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 3eb4d5e6635c..5b1b670c954d 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
>> # DEVFREQ Drivers
>> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
>> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o
>> +obj-$(CONFIG_ARM_MT8183_CCI_DEVFREQ) += mt8183-cci-devfreq.o
>> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
>> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o
>> obj-$(CONFIG_ARM_TEGRA20_DEVFREQ) += tegra20-devfreq.o
>> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
>> new file mode 100644
>> index 000000000000..cd7929a83bf8
>> --- /dev/null
>> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
>> @@ -0,0 +1,206 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>
> s/2019/2020
>
>> +
>> + * Author: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/time.h>
>> +
>> +#include "governor.h"
>
> It is not needed. Please remove it.
>
>> +
>> +#define MAX_VOLT_LIMIT (1150000)
>> +
>> +struct cci_devfreq {
>> + struct devfreq *devfreq;
>> + struct regulator *proc_reg;
>
> 'proc' means the 'processor'?
> Instead of 'proc_reg', you better to use 'cpu_reg'.
>
>> + struct clk *cci_clk;
>> + int old_vproc;
>> + unsigned long old_freq;
>> +};
>> +
>> +static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
>> +{
>> + int ret;
>> +
>> + ret = regulator_set_voltage(cci_df->proc_reg, vproc,
>> + MAX_VOLT_LIMIT);
>> + if (!ret)
>> + cci_df->old_vproc = vproc;
>> + return ret;
>> +}
>> +
>> +static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
>> + u32 flags)
>> +{
>> + int ret;
>> + struct cci_devfreq *cci_df = dev_get_drvdata(dev);
>> + struct dev_pm_opp *opp;
>> + unsigned long opp_rate, opp_voltage, old_voltage;
>> +
>> + if (!cci_df)
>> + return -EINVAL;
>> +
>> + if (cci_df->old_freq == *freq)
>> + return 0;
>> +
>> + opp_rate = *freq;
>> + opp = dev_pm_opp_find_freq_floor(dev, &opp_rate);
>> + opp_voltage = dev_pm_opp_get_voltage(opp);
>> + dev_pm_opp_put(opp);
>
>
> You can use the helper function for getting the rate
> with devfreq_recommended_opp(). You can refer the following code
> in drivers/devfreq/exynos-bus.c
>
> opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(opp)) {
> dev_err(dev, "failed to get recommended opp instance\n");
> return PTR_ERR(opp);
> }
> dev_pm_opp_put(opp);
>
>> +
>> + old_voltage = cci_df->old_vproc;
>> + if (old_voltage == 0)
>> + old_voltage = regulator_get_voltage(cci_df->proc_reg);
>> +
>> + // scale up: set voltage first then freq
>> + if (opp_voltage > old_voltage) {
>> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
>> + if (ret) {
>> + pr_err("cci: failed to scale up voltage\n");
>> + return ret;
>> + }
>> + }
>> +
>> + ret = clk_set_rate(cci_df->cci_clk, *freq);
>> + if (ret) {
>> + pr_err("%s: failed cci to set rate: %d\n", __func__,
>> + ret);
>> + mtk_cci_set_voltage(cci_df, old_voltage);
>> + return ret;
>> + }
>> +
>> + // scale down: set freq first then voltage
>> + if (opp_voltage < old_voltage) {
>> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
>> + if (ret) {
>> + pr_err("cci: failed to scale down voltage\n");
>> + clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
>> + return ret;
>> + }
>> + }
>
>
> I recommend that dev_pm_opp_set_rate() and dev_pm_opp_set_regulator()
> instead of 'clk_set_rate' and 'regulator_set_voltage'.
> In the dev_pm_opp_set_rate(), handle the these sequence.
> You can refer the merged patch[1].
>
> [1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
> - PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()
>
>
>> +
>> + cci_df->old_freq = *freq;
>> +
>> + return 0;
>> +}
>> +
>> +static struct devfreq_dev_profile cci_devfreq_profile = {
>> + .target = mtk_cci_devfreq_target,
>
> Need to add '.exit' for calling dev_pm_opp_of_remove_table().
> You can refer the merged devfreq patches like exynos_bus.c, imx-bus.c.
>
>> +};
>> +
>> +static int mtk_cci_devfreq_probe(struct platform_device *pdev)
>> +{
>> + struct device *cci_dev = &pdev->dev;
>> + struct cci_devfreq *cci_df;
>> + struct devfreq_passive_data *passive_data;
>> + int ret;
>> +
>> + cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
>> + if (!cci_df)
>> + return -ENOMEM;
>> +
>> + cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
>> + ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
>> + if (ret) {
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(cci_dev, "failed to get clock for CCI: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + cci_df->proc_reg = devm_regulator_get_optional(cci_dev, "proc");
>> + ret = PTR_ERR_OR_ZERO(cci_df->proc_reg);
>> + if (ret) {
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
>> + ret);
>> + return ret;
>> + }
>
> I recommend that use dev_pm_opp_set_regulators.
> You can refer the merged patch[1].
>
> [1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
> - PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()
>
>
>> + ret = regulator_enable(cci_df->proc_reg);
>> + if (ret) {
>> + pr_warn("enable buck for cci fail\n");
>
> Use dev_err instead of 'pr_warn'.
>
>> + return ret;
>> + }
>> +
>> + ret = dev_pm_opp_of_add_table(cci_dev);
>> + if (ret) {
>> + dev_err(cci_dev, "Fail to init CCI OPP table: %d\n", ret);
>
> How about changing the error log as following
> because in this driver, use the 'failed to' sentence for error handling?
>
> failed to get OPP table for CCI:L %d
>
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, cci_df);
>> +
>> + passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
>> + if (!passive_data)
>> + return -ENOMEM;
>
> On this error case, you have to call dev_pm_opp_of_remove_table().
> You better to make the 'err_opp' jump lable and then add 'goto err_opp'.
>
>> +
>> + passive_data->parent_type = CPUFREQ_PARENT_DEV;
>> +
>> + cci_df->devfreq = devm_devfreq_add_device(cci_dev,
>> + &cci_devfreq_profile,
>> + DEVFREQ_GOV_PASSIVE,
>> + passive_data);
>> + if (IS_ERR(cci_df->devfreq)) {
>> + ret = PTR_ERR(cci_df->devfreq);
>> + dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
>> + dev_pm_opp_of_remove_table(cci_dev);
>
> Instead of direct call, use 'goto err_opp'.
>
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_cci_devfreq_remove(struct platform_device *pdev)
>> +{
>> + struct device *cci_dev = &pdev->dev;
>> + struct cci_devfreq *cci_df;
>> + struct notifier_block *opp_nb;
>> +
>> + cci_df = platform_get_drvdata(pdev);
>> + opp_nb = &cci_df->opp_nb;
>> +
>> + dev_pm_opp_unregister_notifier(cci_dev, opp_nb);
>
> This patch doesn't call the dev_pm_opp_register_notifier.
> Please remove it.
>
>> + devm_devfreq_remove_device(cci_dev, cci_df->devfreq);
>
> Don't need to call this function because you used devm_devfreq_add_device().
>
>> + dev_pm_opp_of_remove_table(cci_dev)> + regulator_disable(cci_df->proc_reg);
>> +
>> + return 0;
>> +}
>> +
>> +static const __maybe_unused struct of_device_id
>> + mediatek_cci_devfreq_of_match[] = {
>
> Make it on one line and remove '__maybe_unused' keyword.
> - mediatek_cci_devfreq_of_match-> mediatek_cci_of_match
>
>> + { .compatible = "mediatek,mt8183-cci" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, mediatek_cci_devfreq_of_match);
>
> ditto.
>
>> +
>> +static struct platform_driver cci_devfreq_driver = {
>> + .probe = mtk_cci_devfreq_probe,
>> + .remove = mtk_cci_devfreq_remove,
>> + .driver = {
>> + .name = "mediatek-cci-devfreq",
>> + .of_match_table = of_match_ptr(mediatek_cci_devfreq_of_match),
>
> ditto.
>
>> + },
>> +};
>> +
>> +static int __init mtk_cci_devfreq_init(void)
>> +{
>> + return platform_driver_register(&cci_devfreq_driver);
>> +}
>> +module_init(mtk_cci_devfreq_init)
>> +
>> +static void __exit mtk_cci_devfreq_exit(void)
>> +{
>> + platform_driver_unregister(&cci_devfreq_driver);
>> +}
>> +module_exit(mtk_cci_devfreq_exit)
>
> Use 'module_platform_driver' instead of module_init and module_exit.
>
>> +
>> +MODULE_DESCRIPTION("Mediatek CCI devfreq driver");
>> +MODULE_AUTHOR("Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>");
>> +MODULE_LICENSE("GPL v2");
>>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [PATCH] mt76: mt7615: Use kmemdup in mt7615_queue_key_update()
From: YueHaibing @ 2020-05-28 7:48 UTC (permalink / raw)
To: Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Kalle Valo,
Jakub Kicinski, Matthias Brugger, Sean Wang
Cc: YueHaibing, kernel-janitors, linux-wireless, linux-mediatek,
netdev, linux-arm-kernel
Use kmemdup rather than duplicating its implementation
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/mediatek/mt76/mt7615/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index 2e9e9d3519d7..c32f06c85f0f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -289,12 +289,11 @@ mt7615_queue_key_update(struct mt7615_dev *dev, enum set_key_cmd cmd,
wd->type = MT7615_WTBL_KEY_DESC;
wd->sta = msta;
- wd->key.key = kzalloc(key->keylen, GFP_KERNEL);
+ wd->key.key = kmemdup(key->key, key->keylen, GFP_KERNEL);
if (!wd->key.key) {
kfree(wd);
return -ENOMEM;
}
- memcpy(wd->key.key, key->key, key->keylen);
wd->key.cipher = key->cipher;
wd->key.keyidx = key->keyidx;
wd->key.keylen = key->keylen;
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH 08/12] dt-bindings: devfreq: add compatible for mt8183 cci devfreq
From: Chanwoo Choi @ 2020-05-28 7:42 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, linux-arm-kernel
In-Reply-To: <20200520034307.20435-9-andrew-sh.cheng@mediatek.com>
Hi,
On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> This adds dt-binding documentation of cci devfreq
> for Mediatek MT8183 SoC platform.
>
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
> .../devicetree/bindings/devfreq/mt8183-cci.yaml | 51 ++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
>
> diff --git a/Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml b/Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
> new file mode 100644
> index 000000000000..a7341fd94097
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: https://protect2.fireeye.com/url?k=33f1f15d-6e23ea05-33f07a12-0cc47a31c8b4-91b3f8aeecce95dc&q=1&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fdevfreq%2Fmt8183-cci.yaml%23
> +$schema: https://protect2.fireeye.com/url?k=fc7d9089-a1af8bd1-fc7c1bc6-0cc47a31c8b4-b46f5afc59faf86d&q=1&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> +
> +title: CCI_DEVFREQ driver for MT8183.
> +
> +maintainers:
> + - Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> +
> +description: |
> + This module is used to create CCI DEVFREQ.
> + The performance will depend on both CCI frequency and CPU frequency.
> + For MT8183, CCI co-buck with Little core.
> + Contain CCI opp table for voltage and frequency scaling.
> +
> +properties:
> + compatible:
> + const: "mediatek,mt8183-cci"
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: "cci"
> +
> + operating-points-v2: true
> + opp-table: true
> +
> + proc-supply:
> + description:
> + Phandle of the regulator that provides the supply voltage.
> +
> +required:
> + - compatible
> + - clocks
> + - clock-names
> + - proc-supply
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mt8183-clk.h>
> + cci: cci {
> + compatible = "mediatek,mt8183-cci";
> + clocks = <&apmixedsys CLK_APMIXED_CCIPLL>;
> + clock-names = "cci";
> + operating-points-v2 = <&cci_opp>;
> + proc-supply = <&mt6358_vproc12_reg>;
> + };
> +
>
I recommend that add the more detailed example
with OPP table with CPU node.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH 09/12] devfreq: add mediatek cci devfreq
From: Chanwoo Choi @ 2020-05-28 7:35 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, linux-arm-kernel
In-Reply-To: <20200520034307.20435-10-andrew-sh.cheng@mediatek.com>
Hi Andrew-sh.Cheng,
On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
> of the Mediatek MT8183.
>
> On the MT8183 the CCI is supplied by the same regulator as the LITTLE
> cores. The driver is notified when the regulator voltage changes
> (driven by cpufreq) and adjusts the CCI frequency to the maximum
> possible value.
>
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
> drivers/devfreq/Kconfig | 10 ++
> drivers/devfreq/Makefile | 1 +
> drivers/devfreq/mt8183-cci-devfreq.c | 206 +++++++++++++++++++++++++++++++++++
The mt8183-cci.c is enough for driver name.
> 3 files changed, 217 insertions(+)
> create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index d9067950af6a..4ed7116271ee 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -103,6 +103,16 @@ config ARM_IMX8M_DDRC_DEVFREQ
> This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
> adjusting DRAM frequency.
>
> +config ARM_MT8183_CCI_DEVFREQ
> + tristate "MT8183 CCI DEVFREQ Driver"
> + depends on ARM_MEDIATEK_CPUFREQ
> + help
> + This adds a devfreq driver for Cache Coherent Interconnect
> + of Mediatek MT8183, which is shared the same regulator
> + with cpu cluster.
> + It can track buck voltage and update a proper cci frequency.
s/cci/CCI
> + Use notification to get regulator status.
> +
> config ARM_TEGRA_DEVFREQ
> tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
> depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 3eb4d5e6635c..5b1b670c954d 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
> # DEVFREQ Drivers
> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o
> +obj-$(CONFIG_ARM_MT8183_CCI_DEVFREQ) += mt8183-cci-devfreq.o
> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o
> obj-$(CONFIG_ARM_TEGRA20_DEVFREQ) += tegra20-devfreq.o
> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
> new file mode 100644
> index 000000000000..cd7929a83bf8
> --- /dev/null
> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
s/2019/2020
> +
> + * Author: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/devfreq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/time.h>
> +
> +#include "governor.h"
It is not needed. Please remove it.
> +
> +#define MAX_VOLT_LIMIT (1150000)
> +
> +struct cci_devfreq {
> + struct devfreq *devfreq;
> + struct regulator *proc_reg;
'proc' means the 'processor'?
Instead of 'proc_reg', you better to use 'cpu_reg'.
> + struct clk *cci_clk;
> + int old_vproc;
> + unsigned long old_freq;
> +};
> +
> +static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
> +{
> + int ret;
> +
> + ret = regulator_set_voltage(cci_df->proc_reg, vproc,
> + MAX_VOLT_LIMIT);
> + if (!ret)
> + cci_df->old_vproc = vproc;
> + return ret;
> +}
> +
> +static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
> + u32 flags)
> +{
> + int ret;
> + struct cci_devfreq *cci_df = dev_get_drvdata(dev);
> + struct dev_pm_opp *opp;
> + unsigned long opp_rate, opp_voltage, old_voltage;
> +
> + if (!cci_df)
> + return -EINVAL;
> +
> + if (cci_df->old_freq == *freq)
> + return 0;
> +
> + opp_rate = *freq;
> + opp = dev_pm_opp_find_freq_floor(dev, &opp_rate);
> + opp_voltage = dev_pm_opp_get_voltage(opp);
> + dev_pm_opp_put(opp);
You can use the helper function for getting the rate
with devfreq_recommended_opp(). You can refer the following code
in drivers/devfreq/exynos-bus.c
opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(opp)) {
dev_err(dev, "failed to get recommended opp instance\n");
return PTR_ERR(opp);
}
dev_pm_opp_put(opp);
> +
> + old_voltage = cci_df->old_vproc;
> + if (old_voltage == 0)
> + old_voltage = regulator_get_voltage(cci_df->proc_reg);
> +
> + // scale up: set voltage first then freq
> + if (opp_voltage > old_voltage) {
> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
> + if (ret) {
> + pr_err("cci: failed to scale up voltage\n");
> + return ret;
> + }
> + }
> +
> + ret = clk_set_rate(cci_df->cci_clk, *freq);
> + if (ret) {
> + pr_err("%s: failed cci to set rate: %d\n", __func__,
> + ret);
> + mtk_cci_set_voltage(cci_df, old_voltage);
> + return ret;
> + }
> +
> + // scale down: set freq first then voltage
> + if (opp_voltage < old_voltage) {
> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
> + if (ret) {
> + pr_err("cci: failed to scale down voltage\n");
> + clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
> + return ret;
> + }
> + }
I recommend that dev_pm_opp_set_rate() and dev_pm_opp_set_regulator()
instead of 'clk_set_rate' and 'regulator_set_voltage'.
In the dev_pm_opp_set_rate(), handle the these sequence.
You can refer the merged patch[1].
[1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
- PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()
> +
> + cci_df->old_freq = *freq;
> +
> + return 0;
> +}
> +
> +static struct devfreq_dev_profile cci_devfreq_profile = {
> + .target = mtk_cci_devfreq_target,
Need to add '.exit' for calling dev_pm_opp_of_remove_table().
You can refer the merged devfreq patches like exynos_bus.c, imx-bus.c.
> +};
> +
> +static int mtk_cci_devfreq_probe(struct platform_device *pdev)
> +{
> + struct device *cci_dev = &pdev->dev;
> + struct cci_devfreq *cci_df;
> + struct devfreq_passive_data *passive_data;
> + int ret;
> +
> + cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
> + if (!cci_df)
> + return -ENOMEM;
> +
> + cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
> + ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(cci_dev, "failed to get clock for CCI: %d\n",
> + ret);
> + return ret;
> + }
> + cci_df->proc_reg = devm_regulator_get_optional(cci_dev, "proc");
> + ret = PTR_ERR_OR_ZERO(cci_df->proc_reg);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
> + ret);
> + return ret;
> + }
I recommend that use dev_pm_opp_set_regulators.
You can refer the merged patch[1].
[1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
- PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()
> + ret = regulator_enable(cci_df->proc_reg);
> + if (ret) {
> + pr_warn("enable buck for cci fail\n");
Use dev_err instead of 'pr_warn'.
> + return ret;
> + }
> +
> + ret = dev_pm_opp_of_add_table(cci_dev);
> + if (ret) {
> + dev_err(cci_dev, "Fail to init CCI OPP table: %d\n", ret);
How about changing the error log as following
because in this driver, use the 'failed to' sentence for error handling?
failed to get OPP table for CCI:L %d
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, cci_df);
> +
> + passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
> + if (!passive_data)
> + return -ENOMEM;
On this error case, you have to call dev_pm_opp_of_remove_table().
You better to make the 'err_opp' jump lable and then add 'goto err_opp'.
> +
> + passive_data->parent_type = CPUFREQ_PARENT_DEV;
> +
> + cci_df->devfreq = devm_devfreq_add_device(cci_dev,
> + &cci_devfreq_profile,
> + DEVFREQ_GOV_PASSIVE,
> + passive_data);
> + if (IS_ERR(cci_df->devfreq)) {
> + ret = PTR_ERR(cci_df->devfreq);
> + dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
> + dev_pm_opp_of_remove_table(cci_dev);
Instead of direct call, use 'goto err_opp'.
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_cci_devfreq_remove(struct platform_device *pdev)
> +{
> + struct device *cci_dev = &pdev->dev;
> + struct cci_devfreq *cci_df;
> + struct notifier_block *opp_nb;
> +
> + cci_df = platform_get_drvdata(pdev);
> + opp_nb = &cci_df->opp_nb;
> +
> + dev_pm_opp_unregister_notifier(cci_dev, opp_nb);
This patch doesn't call the dev_pm_opp_register_notifier.
Please remove it.
> + devm_devfreq_remove_device(cci_dev, cci_df->devfreq);
Don't need to call this function because you used devm_devfreq_add_device().
> + dev_pm_opp_of_remove_table(cci_dev)> + regulator_disable(cci_df->proc_reg);
> +
> + return 0;
> +}
> +
> +static const __maybe_unused struct of_device_id
> + mediatek_cci_devfreq_of_match[] = {
Make it on one line and remove '__maybe_unused' keyword.
- mediatek_cci_devfreq_of_match-> mediatek_cci_of_match
> + { .compatible = "mediatek,mt8183-cci" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, mediatek_cci_devfreq_of_match);
ditto.
> +
> +static struct platform_driver cci_devfreq_driver = {
> + .probe = mtk_cci_devfreq_probe,
> + .remove = mtk_cci_devfreq_remove,
> + .driver = {
> + .name = "mediatek-cci-devfreq",
> + .of_match_table = of_match_ptr(mediatek_cci_devfreq_of_match),
ditto.
> + },
> +};
> +
> +static int __init mtk_cci_devfreq_init(void)
> +{
> + return platform_driver_register(&cci_devfreq_driver);
> +}
> +module_init(mtk_cci_devfreq_init)
> +
> +static void __exit mtk_cci_devfreq_exit(void)
> +{
> + platform_driver_unregister(&cci_devfreq_driver);
> +}
> +module_exit(mtk_cci_devfreq_exit)
Use 'module_platform_driver' instead of module_init and module_exit.
> +
> +MODULE_DESCRIPTION("Mediatek CCI devfreq driver");
> +MODULE_AUTHOR("Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
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: Sakari Ailus @ 2020-05-28 7:23 UTC (permalink / raw)
To: Dongchun Zhu
Cc: Mark Rutland, Rob Herring, Andy Shevchenko, srv_heupstream,
devicetree, Linus Walleij,
Shengnan Wang (王圣男), Tomasz Figa,
Bartosz Golaszewski, Sj Huang, Nicolas Boichat,
moderated list:ARM/Mediatek SoC support, 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: <1590636882.8804.474.camel@mhfsdcap03>
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??
--
Sakari Ailus
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v10] Add matrix keypad driver support for Mediatek SoCs
From: Marco Felsch @ 2020-05-28 7:19 UTC (permalink / raw)
To: Fengping Yu
Cc: Dmitry Torokhov, linux-mediatek, linux-input, Yingjoe Chen,
Andy Shevchenko, linux-arm-kernel
In-Reply-To: <20200528053344.25936-1-fengping.yu@mediatek.com>
Hi,
this version looks nice to me =) One last question.
On 20-05-28 13:33, Fengping Yu wrote:
>
> Change since v9:
> - modify KEYBOARD_MTK_KPD config dependent item
> - remove wakeup member of mtk_keypad struct
You also removed the device wakeup capability completely, was this
intended? What I mean is that we don't need that member within the
driver state struct.
Regards,
Marco
> - remove default pinctrl state set
> - modify request irq failed return value
> - add space of kepad matching table
> - modify align issue
>
> fengping.yu (3):
> dt-bindings: Add keypad devicetree documentation
> drivers: input: keyboard: Add mtk keypad driver
> configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
>
> .../devicetree/bindings/input/mtk-kpd.yaml | 95 +++++++++
> arch/arm64/configs/defconfig | 1 +
> drivers/input/keyboard/Kconfig | 11 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/mtk-kpd.c | 201 ++++++++++++++++++
> 5 files changed, 309 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
> create mode 100644 drivers/input/keyboard/mtk-kpd.c
>
> --
> 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 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: Chanwoo Choi @ 2020-05-28 7:17 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <64b81eea-641c-811d-9830-718b28db4188@samsung.com>
Hi Andrew-sh.Cheng,
The exynos-bus.c used the passive governor.
Even if don't make the problem because DEVFREQ_PARENT_DEV is zero,
you need to initialize the parent_type with DEVFREQ_PARENT_DEV as following:
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 8fa8eb541373..1c71c47bc2ac 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -369,6 +369,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
return -ENOMEM;
passive_data->parent = parent_devfreq;
+ passive_data->parent_type = DEVFREQ_PARENT_DEV;
/* Add devfreq device for exynos bus with passive governor */
bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
On 5/28/20 3:14 PM, Chanwoo Choi wrote:
> Hi Andrew-sh.Cheng,
>
> Thanks for your posting. I like this approach absolutely.
> I think that it is necessary. When I developed the embedded product,
> I needed this feature always.
>
> I add the comments on below.
>
>
> And the following email is not valid. So, I dropped this email
> from Cc list.
> Saravana Kannan <skannan@codeaurora.org>
>
>
> On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
>> From: Saravana Kannan <skannan@codeaurora.org>
>>
>> Many CPU architectures have caches that can scale independent of the
>> CPUs. Frequency scaling of the caches is necessary to make sure that the
>> cache is not a performance bottleneck that leads to poor performance and
>> power. The same idea applies for RAM/DDR.
>>
>> To achieve this, this patch adds support for cpu based scaling to the
>> passive governor. This is accomplished by taking the current frequency
>> of each CPU frequency domain and then adjust the frequency of the cache
>> (or any devfreq device) based on the frequency of the CPUs. It listens
>> to CPU frequency transition notifiers to keep itself up to date on the
>> current CPU frequency.
>>
>> To decide the frequency of the device, the governor does one of the
>> following:
>> * Derives the optimal devfreq device opp from required-opps property of
>> the parent cpu opp_table.
>>
>> * Scales the device frequency in proportion to the CPU frequency. So, if
>> the CPUs are running at their max frequency, the device runs at its
>> max frequency. If the CPUs are running at their min frequency, the
>> device runs at its min frequency. It is interpolated for frequencies
>> in between.
>>
>> Andrew-sh.Cheng change
>> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
>> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
>> for kernel-5.7
>>
>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
>> [Sibi: Integrated cpu-freqmap governor into passive_governor]
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
>> ---
>> drivers/devfreq/Kconfig | 2 +
>> drivers/devfreq/governor_passive.c | 278 ++++++++++++++++++++++++++++++++++---
>> include/linux/devfreq.h | 40 +++++-
>> 3 files changed, 299 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 0b1df12e0f21..d9067950af6a 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
>> device. This governor does not change the frequency by itself
>> through sysfs entries. The passive governor recommends that
>> devfreq device uses the OPP table to get the frequency/voltage.
>> + Alternatively the governor can also be chosen to scale based on
>> + the online CPUs current frequency.
>>
>> comment "DEVFREQ Drivers"
>>
>> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
>> index 2d67d6c12dce..7dcda02a5bb7 100644
>> --- a/drivers/devfreq/governor_passive.c
>> +++ b/drivers/devfreq/governor_passive.c
>> @@ -8,11 +8,89 @@
>> */
>>
>> #include <linux/module.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpufreq.h>
>> +#include <linux/cpumask.h>
>> #include <linux/device.h>
>> #include <linux/devfreq.h>
>> +#include <linux/slab.h>
>> #include "governor.h"
>>
>> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>> +static unsigned int xlate_cpufreq_to_devfreq(struct devfreq_passive_data *data,
>
> Need to change 'unsigned int' to 'unsigned long'.
>
>> + unsigned int cpu)
>> +{
>> + unsigned int cpu_min, cpu_max, dev_min, dev_max, cpu_percent, max_state;
>
> Better to define them separately as following and then need to rename
> the variable. Usually, use the 'min_freq' and 'max_freq' word for
> the minimum/maximum frequency.
>
> unsigned int cpu_min_freq, cpu_max_freq, cpu_curr_freq, cpu_percent;
> unsigned long dev_min_freq, dev_max_freq, dev_max_state,
>
> The devfreq used 'unsigned long'. The cpufreq used 'unsigned long'
> and 'unsigned int'. You need to handle them properly.
>
>
>> + struct devfreq_cpu_state *cpu_state = data->cpu_state[cpu];
>> + struct devfreq *devfreq = (struct devfreq *)data->this;
>> + unsigned long *freq_table = devfreq->profile->freq_table;
>
> In this function, use 'cpu' work for cpufreq and use 'dev' for devfreq.
> So, I think 'dev_freq_table' is proper name instead of 'freq_table'
> for the readability.
>
> freq_table -> dev_freq_table
>
>> + struct dev_pm_opp *opp = NULL, *cpu_opp = NULL;
>
> In the get_target_freq_with_devfreq(), use 'p_opp' indicating
> the OPP of parent device. For the consistency, I think that
> use 'p_opp' instead of 'cpu_opp'.
>
>> + unsigned long cpu_freq, freq;
>
> Define the 'cpu_freq' on above with cpu_min_freq/cpu_max_freq definition.
> cpu_freq -> cpu_curr_freq.
>
>> +
>> + if (!cpu_state || cpu_state->first_cpu != cpu ||
>> + !cpu_state->opp_table || !devfreq->opp_table)
>> + return 0;
>> +
>> + cpu_freq = cpu_state->freq * 1000;
>> + cpu_opp = devfreq_recommended_opp(cpu_state->dev, &cpu_freq, 0);
>> + if (IS_ERR(cpu_opp))
>> + return 0;
>> +
>> + opp = dev_pm_opp_xlate_required_opp(cpu_state->opp_table,
>> + devfreq->opp_table, cpu_opp);
>> + dev_pm_opp_put(cpu_opp);
>> +
>> + if (!IS_ERR(opp)) {
>> + freq = dev_pm_opp_get_freq(opp);
>> + dev_pm_opp_put(opp);
>
> Better to add the 'out' goto statement.
> If you use 'goto out', you can reduce the one indentation
> without 'else' statement.
>
>
>> + } else {
>
> As I commented, when dev_pm_opp_xlate_required_opp() return successfully
> , use 'goto out'. We can remove 'else' and then reduce the unneeded indentation.
>
>
>> + /* Use Interpolation if required opps is not available */
>> + cpu_min = cpu_state->min_freq;
>> + cpu_max = cpu_state->max_freq;
>> + cpu_freq = cpu_state->freq;
>> +
>> + if (freq_table) {
>> + /* Get minimum frequency according to sorting order */
>> + max_state = freq_table[devfreq->profile->max_state - 1];
>> + if (freq_table[0] < max_state) {
>> + dev_min = freq_table[0];
>> + dev_max = max_state;
>> + } else {
>> + dev_min = max_state;
>> + dev_max = freq_table[0];
>> + }
>> + } else {
>> + if (devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value
>> + <= devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value)
>> + return 0;
>> + dev_min =
>> + devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value;
>> + dev_max =
>> + devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value;
>
> I think it is not proper to access the variable of pm_qos structure directly.
> Instead of direct access, you have to use the exported PM QoS function such as
> - pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY);
> - pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY);
>
>> + }
>> + cpu_percent = ((cpu_freq - cpu_min) * 100) / cpu_max - cpu_min;
>> + freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
>> + }
>
>
> I think that you better to add 'out' jump label as following:
>
> out:
>
>> +
>> + return freq;
>> +}
>> +
>> +static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
>> + unsigned long *freq)
>> +{
>> + struct devfreq_passive_data *p_data =
>> + (struct devfreq_passive_data *)devfreq->data;
>> + unsigned int cpu, target_freq = 0;
>
> Need to define 'target_freq' with 'unsigned long' type.
>
>> +
>> + for_each_online_cpu(cpu)
>> + target_freq = max(target_freq,
>> + xlate_cpufreq_to_devfreq(p_data, cpu));
>> +
>> + *freq = target_freq;
>> +
>> + return 0;
>> +}
>> +
>> +static int get_target_freq_with_devfreq(struct devfreq *devfreq,
>> unsigned long *freq)
>> {
>> struct devfreq_passive_data *p_data
>> @@ -23,16 +101,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>> int i, count, ret = 0;
>>
>> /*
>> - * If the devfreq device with passive governor has the specific method
>> - * to determine the next frequency, should use the get_target_freq()
>> - * of struct devfreq_passive_data.
>> - */
>> - if (p_data->get_target_freq) {
>> - ret = p_data->get_target_freq(devfreq, freq);
>> - goto out;
>> - }
>> -
>> - /*
>> * If the parent and passive devfreq device uses the OPP table,
>> * get the next frequency by using the OPP table.
>> */
>> @@ -102,6 +170,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>> return ret;
>> }
>>
>> +static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
>> + unsigned long *freq)
>> +{
>> + struct devfreq_passive_data *p_data =
>> + (struct devfreq_passive_data *)devfreq->data;
>> + int ret;
>> +
>> + /*
>> + * If the devfreq device with passive governor has the specific method
>> + * to determine the next frequency, should use the get_target_freq()
>> + * of struct devfreq_passive_data.
>> + */
>> + if (p_data->get_target_freq)
>> + return p_data->get_target_freq(devfreq, freq);
>> +
>> + switch (p_data->parent_type) {
>> + case DEVFREQ_PARENT_DEV:
>> + ret = get_target_freq_with_devfreq(devfreq, freq);
>> + break;
>> + case CPUFREQ_PARENT_DEV:
>> + ret = get_target_freq_with_cpufreq(devfreq, freq);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + dev_err(&devfreq->dev, "Invalid parent type\n");
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
>> {
>> int ret;
>> @@ -156,6 +255,140 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb,
>> return NOTIFY_DONE;
>> }
>>
>> +static int cpufreq_passive_notifier_call(struct notifier_block *nb,
>> + unsigned long event, void *ptr)
>> +{
>> + struct devfreq_passive_data *data =
>> + container_of(nb, struct devfreq_passive_data, nb);
>> + struct devfreq *devfreq = (struct devfreq *)data->this;
>> + struct devfreq_cpu_state *cpu_state;
>> + struct cpufreq_freqs *freq = ptr;
>
> How about changing 'freq' to 'cpu_freqs'?
>
> In the drivers/cpufreq/cpufreq.c, use 'freqs' name indicating
> the instance of 'struct cpufreq_freqs'. And in order to
> identfy, how about adding 'cpu_' prefix for variable name?
>
>> + unsigned int current_freq;
>
> Need to define curr_freq with 'unsigned long' type
> and better to use 'curr_freq' variable name.
>
>> + int ret;
>> +
>> + if (event != CPUFREQ_POSTCHANGE || !freq ||
>> + !data->cpu_state[freq->policy->cpu])
>> + return 0;
>> +
>> + cpu_state = data->cpu_state[freq->policy->cpu];
>> + if (cpu_state->freq == freq->new)
>> + return 0;
>> +
>> + /* Backup current freq and pre-update cpu state freq*/
>> + current_freq = cpu_state->freq;
>> + cpu_state->freq = freq->new;
>> +
>> + mutex_lock(&devfreq->lock);
>> + ret = update_devfreq(devfreq);
>> + mutex_unlock(&devfreq->lock);
>> + if (ret) {
>> + cpu_state->freq = current_freq;
>> + dev_err(&devfreq->dev, "Couldn't update the frequency.\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int cpufreq_passive_register(struct devfreq_passive_data **p_data)
>> +{
>> + struct devfreq_passive_data *data = *p_data;
>> + struct devfreq *devfreq = (struct devfreq *)data->this;
>> + struct device *dev = devfreq->dev.parent;
>> + struct opp_table *opp_table = NULL;
>> + struct devfreq_cpu_state *state;
>
> For the readability, I thinkt 'cpu_state' is proper instead of 'state'.
>
>> + struct cpufreq_policy *policy;
>> + struct device *cpu_dev;
>> + unsigned int cpu;
>> + int ret;
>> +
>> + get_online_cpus();
>
> Add blank line.
>
>> + data->nb.notifier_call = cpufreq_passive_notifier_call;
>> + ret = cpufreq_register_notifier(&data->nb,
>> + CPUFREQ_TRANSITION_NOTIFIER);
>> + if (ret) {
>> + dev_err(dev, "Couldn't register cpufreq notifier.\n");
>> + data->nb.notifier_call = NULL;
>> + goto out;
>> + }
>> +
>> + /* Populate devfreq_cpu_state */
>> + for_each_online_cpu(cpu) {
>> + if (data->cpu_state[cpu])
>> + continue;
>> +
>> + policy = cpufreq_cpu_get(cpu);
>
> cpufreq_cpu_get() might return 'NULL'. I think you need to handle
> return value as following:
>
> if (!policy) {
> ret = -EINVAL;
> goto out;
> } else if (PTR_ERR(policy) == -EPROBE_DEFER) {
> goto out;
> } else if (IS_ERR(policy) {
> ret = PTR_ERR(policy);
> dev_err(dev, "Couldn't get the cpufreq_poliy.\n");
> goto out;
> }
>
> If cpufreq_cpu_get() return successfully, to do next.
> It reduces the one indentaion.
>
>
>
>> + if (policy) {
>> + state = kzalloc(sizeof(*state), GFP_KERNEL);
>> + if (!state) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + cpu_dev = get_cpu_device(cpu);
>> + if (!cpu_dev) {
>> + dev_err(dev, "Couldn't get cpu device.\n");
>> + ret = -ENODEV;
>> + goto out;
>> + }
>> +
>> + opp_table = dev_pm_opp_get_opp_table(cpu_dev);
>> + if (IS_ERR(devfreq->opp_table)) {
>> + ret = PTR_ERR(opp_table);
>> + goto out;
>> + }
>> +
>> + state->dev = cpu_dev;
>> + state->opp_table = opp_table;
>> + state->first_cpu = cpumask_first(policy->related_cpus);
>> + state->freq = policy->cur;
>> + state->min_freq = policy->cpuinfo.min_freq;
>> + state->max_freq = policy->cpuinfo.max_freq;
>> + data->cpu_state[cpu] = state;
>
> Add blank line.
>
>> + cpufreq_cpu_put(policy);
>> + } else {
>> + ret = -EPROBE_DEFER;
>> + goto out;
>> + }
>> + }
>
> Add blank line.
>
>> +out:
>> + put_online_cpus();
>> + if (ret)
>> + return ret;
>> +
>> + /* Update devfreq */
>> + mutex_lock(&devfreq->lock);
>> + ret = update_devfreq(devfreq);
>> + mutex_unlock(&devfreq->lock);
>> + if (ret)
>> + dev_err(dev, "Couldn't update the frequency.\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int cpufreq_passive_unregister(struct devfreq_passive_data **p_data)
>> +{
>> + struct devfreq_passive_data *data = *p_data;
>> + struct devfreq_cpu_state *cpu_state;
>> + int cpu;
>> +
>> + if (data->nb.notifier_call)
>> + cpufreq_unregister_notifier(&data->nb,
>> + CPUFREQ_TRANSITION_NOTIFIER);
>> +
>> + for_each_possible_cpu(cpu) {
>> + cpu_state = data->cpu_state[cpu];
>> + if (cpu_state) {
>> + if (cpu_state->opp_table)
>> + dev_pm_opp_put_opp_table(cpu_state->opp_table);
>> + kfree(cpu_state);
>> + cpu_state = NULL;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int devfreq_passive_event_handler(struct devfreq *devfreq,
>> unsigned int event, void *data)
>> {
>> @@ -165,7 +398,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
>> struct notifier_block *nb = &p_data->nb;
>> int ret = 0;
>>
>> - if (!parent)
>> + if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
>> return -EPROBE_DEFER;
>
> If you modify the devfreq_passive_event_handler() as following,
> you can move this condition for DEVFREQ_PARENT_DEV into
> (register|unregister)_parent_dev_notifier.
>
> switch (event) {
> case DEVFREQ_GOV_START:
> ret = register_parent_dev_notifier(p_data);
> break;
> case DEVFREQ_GOV_STOP:
> ret = unregister_parent_dev_notifier(p_data);
> break;
> default:
> ret = -EINVAL;
> break;
> }
>
> return ret;
>
>>
>> switch (event) {
>> @@ -173,13 +406,24 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
>> if (!p_data->this)
>> p_data->this = devfreq;
>>
>> - nb->notifier_call = devfreq_passive_notifier_call;
>> - ret = devfreq_register_notifier(parent, nb,
>> - DEVFREQ_TRANSITION_NOTIFIER);
>> + if (p_data->parent_type == DEVFREQ_PARENT_DEV) {
>> + nb->notifier_call = devfreq_passive_notifier_call;
>> + ret = devfreq_register_notifier(parent, nb,
>> + DEVFREQ_TRANSITION_NOTIFIER);
>> + } else if (p_data->parent_type == CPUFREQ_PARENT_DEV) {
>> + ret = cpufreq_passive_register(&p_data);
>
> I think that we better to collect the code related to notifier registration
> into one function like devfreq_pass_register_notifier() instead of
> cpufreq_passive_register() as following: I think it is more simple and readable.
>
> If you have more proper function name of register_parent_dev_notifier,
> please give your opinion.
>
>
> int register_parent_dev_notifier(struct devfreq_passive_data **p_data)
> switch (p_data->parent_type) {
> case DEVFREQ_PARENT_DEV:
> nb->notifier_call = devfreq_passive_notifier_call;
> ret = devfreq_register_notifier(parent, nb,
> break;
> case CPUFREQ_PARENT_DEV:
> cpufreq_register_notifier(...)
> ...
> break;
> }
>
>
>> + } else {
>> + ret = -EINVAL;
>> + }
>> break;
>> case DEVFREQ_GOV_STOP:
>> - WARN_ON(devfreq_unregister_notifier(parent, nb,
>> - DEVFREQ_TRANSITION_NOTIFIER));
>> + if (p_data->parent_type == DEVFREQ_PARENT_DEV)
>> + WARN_ON(devfreq_unregister_notifier(parent, nb,
>> + DEVFREQ_TRANSITION_NOTIFIER));
>> + else if (p_data->parent_type == CPUFREQ_PARENT_DEV)
>> + cpufreq_passive_unregister(&p_data);
>> + else
>> + ret = -EINVAL;
>
> ditto. unregister_parent_dev_notifier(struct devfreq_passive_data **p_data)
>
>> break;
>> default:
>> break;
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index a4b19d593151..04ce576fd6f1 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -278,6 +278,32 @@ struct devfreq_simple_ondemand_data {
>>
>> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
>> /**
>> + * struct devfreq_cpu_state - holds the per-cpu state
>> + * @freq: the current frequency of the cpu.
>> + * @min_freq: the min frequency of the cpu.
>> + * @max_freq: the max frequency of the cpu.
>> + * @first_cpu: the cpumask of the first cpu of a policy.
>> + * @dev: reference to cpu device.
>> + * @opp_table: reference to cpu opp table.
>> + *
>> + * This structure stores the required cpu_state of a cpu.
>> + * This is auto-populated by the governor.
>> + */
>> +struct devfreq_cpu_state {> + unsigned int freq;
>
> It is better to change from 'freq' to 'curr_freq'
> for more correct expression.
>
>> + unsigned int min_freq;
>> + unsigned int max_freq;
>> + unsigned int first_cpu;
>> + struct device *dev;
>
> How about changing the name 'dev' to 'cpu_dev'?
>
>
>> + struct opp_table *opp_table;
>> +};
>
> devfreq_cpu_state is only handled by within driver/devfreq/governor_passive.c.
>
> So, you can move it into drivers/devfreq/governor_passive.c
> and just add the definition into include/linux/devfreq.h as following:
> It is able to prevent the access of variable of 'struct devfreq_cpu_state'
> outside.
>
> struct devfreq_cpu_state;
>
>> +
>> +enum devfreq_parent_dev_type {
>> + DEVFREQ_PARENT_DEV,
>> + CPUFREQ_PARENT_DEV,
>> +};
>> +
>> +/**
>> * struct devfreq_passive_data - ``void *data`` fed to struct devfreq
>> * and devfreq_add_device
>> * @parent: the devfreq instance of parent device.
>> @@ -288,13 +314,15 @@ struct devfreq_simple_ondemand_data {
>> * using governors except for passive governor.
>> * If the devfreq device has the specific method to decide
>> * the next frequency, should use this callback.
>> - * @this: the devfreq instance of own device.
>> - * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
>> + * @parent_type parent type of the device
>
> Need to add ':' at the end of word. -> "parent_type:".
>
>> + * @this: the devfreq instance of own device.
>> + * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
>
> I knew that you make them with same indentation.
> But, actually, it is not related to this patch like clean-up code.
> Even if it is not pretty, you better to don't touch 'this' and 'nb' indentaion.
>
>> + * @cpu_state: the state min/max/current frequency of all online cpu's
>> *
>> * The devfreq_passive_data have to set the devfreq instance of parent
>> * device with governors except for the passive governor. But, don't need to
>> - * initialize the 'this' and 'nb' field because the devfreq core will handle
>> - * them.
>> + * initialize the 'this', 'nb' and 'cpu_state' field because the devfreq core
>> + * will handle them.
>> */
>> struct devfreq_passive_data {
>> /* Should set the devfreq instance of parent device */
>> @@ -303,9 +331,13 @@ struct devfreq_passive_data {
>> /* Optional callback to decide the next frequency of passvice device */
>> int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
>>
>> + /* Should set the type of parent device */
>> + enum devfreq_parent_dev_type parent_type;
>> +
>> /* For passive governor's internal use. Don't need to set them */
>> struct devfreq *this;
>> struct notifier_block nb;
>> + struct devfreq_cpu_state *cpu_state[NR_CPUS];
>> };
>> #endif
>>
>>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* [PATCH v2 4/4] ARM: dts: mediatek: Update mt7629 PCIe node
From: chuanjia.liu @ 2020-05-28 6:16 UTC (permalink / raw)
To: robh+dt, ryder.lee, matthias.bgg
Cc: devicetree, lorenzo.pieralisi, srv_heupstream, chuanjia.liu,
linux-pci, linux-kernel, jianjun.wang, linux-mediatek, yong.wu,
bhelgaas, linux-arm-kernel, amurray
In-Reply-To: <20200528061648.32078-1-chuanjia.liu@mediatek.com>
From: "chuanjia.liu" <Chuanjia.Liu@mediatek.com>
Remove unused property and add pciecfg node.
Signed-off-by: chuanjia.liu <Chuanjia.Liu@mediatek.com>
---
arch/arm/boot/dts/mt7629-rfb.dts | 3 ++-
arch/arm/boot/dts/mt7629.dtsi | 23 +++++++++++++----------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/arm/boot/dts/mt7629-rfb.dts b/arch/arm/boot/dts/mt7629-rfb.dts
index 9980c10c6e29..eb536cbebd9b 100644
--- a/arch/arm/boot/dts/mt7629-rfb.dts
+++ b/arch/arm/boot/dts/mt7629-rfb.dts
@@ -140,9 +140,10 @@
};
};
-&pcie {
+&pcie1 {
pinctrl-names = "default";
pinctrl-0 = <&pcie_pins>;
+ status = "okay";
};
&pciephy1 {
diff --git a/arch/arm/boot/dts/mt7629.dtsi b/arch/arm/boot/dts/mt7629.dtsi
index 5cbb3d244c75..94567307b842 100644
--- a/arch/arm/boot/dts/mt7629.dtsi
+++ b/arch/arm/boot/dts/mt7629.dtsi
@@ -360,16 +360,21 @@
#reset-cells = <1>;
};
- pcie: pcie@1a140000 {
+ pciecfg: pciecfg@1a140000 {
+ compatible = "mediatek,mt7629-pciecfg", "syscon";
+ reg = <0x1a140000 0x1000>;
+ };
+
+ pcie1: pcie@1a145000 {
compatible = "mediatek,mt7629-pcie";
device_type = "pci";
- reg = <0x1a140000 0x1000>,
- <0x1a145000 0x1000>;
- reg-names = "subsys","port1";
+ reg = <0x1a145000 0x1000>;
+ reg-names = "port1";
+ mediatek,pcie-cfg = <&pciecfg>;
#address-cells = <3>;
#size-cells = <2>;
- interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_LOW>,
- <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "pcie_irq";
clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
<&pciesys CLK_PCIE_P0_AHB_EN>,
<&pciesys CLK_PCIE_P1_AUX_EN>,
@@ -390,21 +395,19 @@
power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
bus-range = <0x00 0xff>;
ranges = <0x82000000 0 0x20000000 0x20000000 0 0x10000000>;
+ status = "disabled";
- pcie1: pcie@1,0 {
- device_type = "pci";
+ slot1: pcie@1,0 {
reg = <0x0800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
- num-lanes = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc1 0>,
<0 0 0 2 &pcie_intc1 1>,
<0 0 0 3 &pcie_intc1 2>,
<0 0 0 4 &pcie_intc1 3>;
-
pcie_intc1: interrupt-controller {
interrupt-controller;
#address-cells = <0>;
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox