* [v4,2/2] mailbox: add STMicroelectronics STM32 IPCC driver
From: Fabien DESSENNE @ 2018-05-14 15:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1523515661-8318-3-git-send-email-fabien.dessenne@st.com>
Hi Jassi,
Do you have any more comments or do you plan to have this patch part of
the 4.18 pull request ?
BR
Fabien
On 12/04/18 08:47, Fabien DESSENNE wrote:
> The STMicroelectronics STM32 Inter-Processor Communication Controller
> (IPCC) is used for communicating data between two processors.
> It provides a non blocking signaling mechanism to post and retrieve
> communication data in an atomic way.
>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/stm32-ipcc.c | 402 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 412 insertions(+)
> create mode 100644 drivers/mailbox/stm32-ipcc.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index ba2f152..d7581f0 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -171,4 +171,12 @@ config BCM_FLEXRM_MBOX
> Mailbox implementation of the Broadcom FlexRM ring manager,
> which provides access to various offload engines on Broadcom
> SoCs. Say Y here if you want to use the Broadcom FlexRM.
> +
> +config STM32_IPCC
> + tristate "STM32 IPCC Mailbox"
> + depends on MACH_STM32MP157
> + help
> + Mailbox implementation for STMicroelectonics STM32 family chips
> + with hardware for Inter-Processor Communication Controller (IPCC)
> + between processors. Say Y here if you want to have this support.
> endif
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index 4896f8d..7ea9654 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -36,3 +36,5 @@ obj-$(CONFIG_BCM_FLEXRM_MBOX) += bcm-flexrm-mailbox.o
> obj-$(CONFIG_QCOM_APCS_IPC) += qcom-apcs-ipc-mailbox.o
>
> obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o
> +
> +obj-$(CONFIG_STM32_IPCC) += stm32-ipcc.o
> diff --git a/drivers/mailbox/stm32-ipcc.c b/drivers/mailbox/stm32-ipcc.c
> new file mode 100644
> index 0000000..533b0da
> --- /dev/null
> +++ b/drivers/mailbox/stm32-ipcc.c
> @@ -0,0 +1,402 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
> + * Authors: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
> + * Fabien Dessenne <fabien.dessenne@st.com> for STMicroelectronics.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
> +
> +#define IPCC_XCR 0x000
> +#define XCR_RXOIE BIT(0)
> +#define XCR_TXOIE BIT(16)
> +
> +#define IPCC_XMR 0x004
> +#define IPCC_XSCR 0x008
> +#define IPCC_XTOYSR 0x00c
> +
> +#define IPCC_PROC_OFFST 0x010
> +
> +#define IPCC_HWCFGR 0x3f0
> +#define IPCFGR_CHAN_MASK GENMASK(7, 0)
> +
> +#define IPCC_VER 0x3f4
> +#define VER_MINREV_MASK GENMASK(3, 0)
> +#define VER_MAJREV_MASK GENMASK(7, 4)
> +
> +#define RX_BIT_MASK GENMASK(15, 0)
> +#define RX_BIT_CHAN(chan) BIT(chan)
> +#define TX_BIT_SHIFT 16
> +#define TX_BIT_MASK GENMASK(31, 16)
> +#define TX_BIT_CHAN(chan) BIT(TX_BIT_SHIFT + (chan))
> +
> +#define STM32_MAX_PROCS 2
> +
> +enum {
> + IPCC_IRQ_RX,
> + IPCC_IRQ_TX,
> + IPCC_IRQ_NUM,
> +};
> +
> +struct stm32_ipcc {
> + struct mbox_controller controller;
> + void __iomem *reg_base;
> + void __iomem *reg_proc;
> + struct clk *clk;
> + int irqs[IPCC_IRQ_NUM];
> + int wkp;
> + u32 proc_id;
> + u32 n_chans;
> + u32 xcr;
> + u32 xmr;
> +};
> +
> +static inline void stm32_ipcc_set_bits(void __iomem *reg, u32 mask)
> +{
> + writel_relaxed(readl_relaxed(reg) | mask, reg);
> +}
> +
> +static inline void stm32_ipcc_clr_bits(void __iomem *reg, u32 mask)
> +{
> + writel_relaxed(readl_relaxed(reg) & ~mask, reg);
> +}
> +
> +static irqreturn_t stm32_ipcc_rx_irq(int irq, void *data)
> +{
> + struct stm32_ipcc *ipcc = data;
> + struct device *dev = ipcc->controller.dev;
> + u32 status, mr, tosr, chan;
> + irqreturn_t ret = IRQ_NONE;
> + int proc_offset;
> +
> + /* read 'channel occupied' status from other proc */
> + proc_offset = ipcc->proc_id ? -IPCC_PROC_OFFST : IPCC_PROC_OFFST;
> + tosr = readl_relaxed(ipcc->reg_proc + proc_offset + IPCC_XTOYSR);
> + mr = readl_relaxed(ipcc->reg_proc + IPCC_XMR);
> +
> + /* search for unmasked 'channel occupied' */
> + status = tosr & FIELD_GET(RX_BIT_MASK, ~mr);
> +
> + for (chan = 0; chan < ipcc->n_chans; chan++) {
> + if (!(status & (1 << chan)))
> + continue;
> +
> + dev_dbg(dev, "%s: chan:%d rx\n", __func__, chan);
> +
> + mbox_chan_received_data(&ipcc->controller.chans[chan], NULL);
> +
> + stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XSCR,
> + RX_BIT_CHAN(chan));
> +
> + ret = IRQ_HANDLED;
> + }
> +
> + return ret;
> +}
> +
> +static irqreturn_t stm32_ipcc_tx_irq(int irq, void *data)
> +{
> + struct stm32_ipcc *ipcc = data;
> + struct device *dev = ipcc->controller.dev;
> + u32 status, mr, tosr, chan;
> + irqreturn_t ret = IRQ_NONE;
> +
> + tosr = readl_relaxed(ipcc->reg_proc + IPCC_XTOYSR);
> + mr = readl_relaxed(ipcc->reg_proc + IPCC_XMR);
> +
> + /* search for unmasked 'channel free' */
> + status = ~tosr & FIELD_GET(TX_BIT_MASK, ~mr);
> +
> + for (chan = 0; chan < ipcc->n_chans ; chan++) {
> + if (!(status & (1 << chan)))
> + continue;
> +
> + dev_dbg(dev, "%s: chan:%d tx\n", __func__, chan);
> +
> + /* mask 'tx channel free' interrupt */
> + stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XMR,
> + TX_BIT_CHAN(chan));
> +
> + mbox_chan_txdone(&ipcc->controller.chans[chan], 0);
> +
> + ret = IRQ_HANDLED;
> + }
> +
> + return ret;
> +}
> +
> +static int stm32_ipcc_send_data(struct mbox_chan *link, void *data)
> +{
> + unsigned int chan = (unsigned int)link->con_priv;
> + struct stm32_ipcc *ipcc = container_of(link->mbox, struct stm32_ipcc,
> + controller);
> +
> + dev_dbg(ipcc->controller.dev, "%s: chan:%d\n", __func__, chan);
> +
> + /* set channel n occupied */
> + stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XSCR, TX_BIT_CHAN(chan));
> +
> + /* unmask 'tx channel free' interrupt */
> + stm32_ipcc_clr_bits(ipcc->reg_proc + IPCC_XMR, TX_BIT_CHAN(chan));
> +
> + return 0;
> +}
> +
> +static int stm32_ipcc_startup(struct mbox_chan *link)
> +{
> + unsigned int chan = (unsigned int)link->con_priv;
> + struct stm32_ipcc *ipcc = container_of(link->mbox, struct stm32_ipcc,
> + controller);
> + int ret;
> +
> + ret = clk_prepare_enable(ipcc->clk);
> + if (ret) {
> + dev_err(ipcc->controller.dev, "can not enable the clock\n");
> + return ret;
> + }
> +
> + /* unmask 'rx channel occupied' interrupt */
> + stm32_ipcc_clr_bits(ipcc->reg_proc + IPCC_XMR, RX_BIT_CHAN(chan));
> +
> + return 0;
> +}
> +
> +static void stm32_ipcc_shutdown(struct mbox_chan *link)
> +{
> + unsigned int chan = (unsigned int)link->con_priv;
> + struct stm32_ipcc *ipcc = container_of(link->mbox, struct stm32_ipcc,
> + controller);
> +
> + /* mask rx/tx interrupt */
> + stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XMR,
> + RX_BIT_CHAN(chan) | TX_BIT_CHAN(chan));
> +
> + clk_disable_unprepare(ipcc->clk);
> +}
> +
> +static const struct mbox_chan_ops stm32_ipcc_ops = {
> + .send_data = stm32_ipcc_send_data,
> + .startup = stm32_ipcc_startup,
> + .shutdown = stm32_ipcc_shutdown,
> +};
> +
> +static int stm32_ipcc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct stm32_ipcc *ipcc;
> + struct resource *res;
> + unsigned int i;
> + int ret;
> + u32 ip_ver;
> + static const char * const irq_name[] = {"rx", "tx"};
> + irq_handler_t irq_thread[] = {stm32_ipcc_rx_irq, stm32_ipcc_tx_irq};
> +
> + if (!np) {
> + dev_err(dev, "No DT found\n");
> + return -ENODEV;
> + }
> +
> + ipcc = devm_kzalloc(dev, sizeof(*ipcc), GFP_KERNEL);
> + if (!ipcc)
> + return -ENOMEM;
> +
> + /* proc_id */
> + if (of_property_read_u32(np, "st,proc-id", &ipcc->proc_id)) {
> + dev_err(dev, "Missing st,proc-id\n");
> + return -ENODEV;
> + }
> +
> + if (ipcc->proc_id >= STM32_MAX_PROCS) {
> + dev_err(dev, "Invalid proc_id (%d)\n", ipcc->proc_id);
> + return -EINVAL;
> + }
> +
> + /* regs */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ipcc->reg_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ipcc->reg_base))
> + return PTR_ERR(ipcc->reg_base);
> +
> + ipcc->reg_proc = ipcc->reg_base + ipcc->proc_id * IPCC_PROC_OFFST;
> +
> + /* clock */
> + ipcc->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(ipcc->clk))
> + return PTR_ERR(ipcc->clk);
> +
> + ret = clk_prepare_enable(ipcc->clk);
> + if (ret) {
> + dev_err(dev, "can not enable the clock\n");
> + return ret;
> + }
> +
> + /* irq */
> + for (i = 0; i < IPCC_IRQ_NUM; i++) {
> + ipcc->irqs[i] = of_irq_get_byname(dev->of_node, irq_name[i]);
> + if (ipcc->irqs[i] < 0) {
> + dev_err(dev, "no IRQ specified %s\n", irq_name[i]);
> + ret = ipcc->irqs[i];
> + goto err_clk;
> + }
> +
> + ret = devm_request_threaded_irq(dev, ipcc->irqs[i], NULL,
> + irq_thread[i], IRQF_ONESHOT,
> + dev_name(dev), ipcc);
> + if (ret) {
> + dev_err(dev, "failed to request irq %d (%d)\n", i, ret);
> + goto err_clk;
> + }
> + }
> +
> + /* mask and enable rx/tx irq */
> + stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XMR,
> + RX_BIT_MASK | TX_BIT_MASK);
> + stm32_ipcc_set_bits(ipcc->reg_proc + IPCC_XCR, XCR_RXOIE | XCR_TXOIE);
> +
> + /* wakeup */
> + if (of_property_read_bool(np, "wakeup-source")) {
> + ipcc->wkp = of_irq_get_byname(dev->of_node, "wakeup");
> + if (ipcc->wkp < 0) {
> + dev_err(dev, "could not get wakeup IRQ\n");
> + ret = ipcc->wkp;
> + goto err_clk;
> + }
> +
> + device_init_wakeup(dev, true);
> + ret = dev_pm_set_dedicated_wake_irq(dev, ipcc->wkp);
> + if (ret) {
> + dev_err(dev, "Failed to set wake up irq\n");
> + goto err_init_wkp;
> + }
> + } else {
> + device_init_wakeup(dev, false);
> + }
> +
> + /* mailbox controller */
> + ipcc->n_chans = readl_relaxed(ipcc->reg_base + IPCC_HWCFGR);
> + ipcc->n_chans &= IPCFGR_CHAN_MASK;
> +
> + ipcc->controller.dev = dev;
> + ipcc->controller.txdone_irq = true;
> + ipcc->controller.ops = &stm32_ipcc_ops;
> + ipcc->controller.num_chans = ipcc->n_chans;
> + ipcc->controller.chans = devm_kcalloc(dev, ipcc->controller.num_chans,
> + sizeof(*ipcc->controller.chans),
> + GFP_KERNEL);
> + if (!ipcc->controller.chans) {
> + ret = -ENOMEM;
> + goto err_irq_wkp;
> + }
> +
> + for (i = 0; i < ipcc->controller.num_chans; i++)
> + ipcc->controller.chans[i].con_priv = (void *)i;
> +
> + ret = mbox_controller_register(&ipcc->controller);
> + if (ret)
> + goto err_irq_wkp;
> +
> + platform_set_drvdata(pdev, ipcc);
> +
> + ip_ver = readl_relaxed(ipcc->reg_base + IPCC_VER);
> +
> + dev_info(dev, "ipcc rev:%ld.%ld enabled, %d chans, proc %d\n",
> + FIELD_GET(VER_MAJREV_MASK, ip_ver),
> + FIELD_GET(VER_MINREV_MASK, ip_ver),
> + ipcc->controller.num_chans, ipcc->proc_id);
> +
> + clk_disable_unprepare(ipcc->clk);
> + return 0;
> +
> +err_irq_wkp:
> + if (ipcc->wkp)
> + dev_pm_clear_wake_irq(dev);
> +err_init_wkp:
> + device_init_wakeup(dev, false);
> +err_clk:
> + clk_disable_unprepare(ipcc->clk);
> + return ret;
> +}
> +
> +static int stm32_ipcc_remove(struct platform_device *pdev)
> +{
> + struct stm32_ipcc *ipcc = platform_get_drvdata(pdev);
> +
> + mbox_controller_unregister(&ipcc->controller);
> +
> + if (ipcc->wkp)
> + dev_pm_clear_wake_irq(&pdev->dev);
> +
> + device_init_wakeup(&pdev->dev, false);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static void stm32_ipcc_set_irq_wake(struct device *dev, bool enable)
> +{
> + struct stm32_ipcc *ipcc = dev_get_drvdata(dev);
> + unsigned int i;
> +
> + if (device_may_wakeup(dev))
> + for (i = 0; i < IPCC_IRQ_NUM; i++)
> + irq_set_irq_wake(ipcc->irqs[i], enable);
> +}
> +
> +static int stm32_ipcc_suspend(struct device *dev)
> +{
> + struct stm32_ipcc *ipcc = dev_get_drvdata(dev);
> +
> + ipcc->xmr = readl_relaxed(ipcc->reg_proc + IPCC_XMR);
> + ipcc->xcr = readl_relaxed(ipcc->reg_proc + IPCC_XCR);
> +
> + stm32_ipcc_set_irq_wake(dev, true);
> +
> + return 0;
> +}
> +
> +static int stm32_ipcc_resume(struct device *dev)
> +{
> + struct stm32_ipcc *ipcc = dev_get_drvdata(dev);
> +
> + stm32_ipcc_set_irq_wake(dev, false);
> +
> + writel_relaxed(ipcc->xmr, ipcc->reg_proc + IPCC_XMR);
> + writel_relaxed(ipcc->xcr, ipcc->reg_proc + IPCC_XCR);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(stm32_ipcc_pm_ops,
> + stm32_ipcc_suspend, stm32_ipcc_resume);
> +
> +static const struct of_device_id stm32_ipcc_of_match[] = {
> + { .compatible = "st,stm32mp1-ipcc" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_ipcc_of_match);
> +
> +static struct platform_driver stm32_ipcc_driver = {
> + .driver = {
> + .name = "stm32-ipcc",
> + .pm = &stm32_ipcc_pm_ops,
> + .of_match_table = stm32_ipcc_of_match,
> + },
> + .probe = stm32_ipcc_probe,
> + .remove = stm32_ipcc_remove,
> +};
> +
> +module_platform_driver(stm32_ipcc_driver);
> +
> +MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
> +MODULE_AUTHOR("Fabien Dessenne <fabien.dessenne@st.com>");
> +MODULE_DESCRIPTION("STM32 IPCC driver");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply
* [linux-sunxi] [PATCH v2 4/4] ARM: PWM: add allwinner sun8i pwm support.
From: Hao Zhang @ 2018-05-14 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8bd4247f-45fd-1385-3ba2-accd7a1e7eb9@arm.com>
2018-02-28 9:55 GMT+08:00 Andr? Przywara <andre.przywara@arm.com>:
> Hi,
>
> On 25/02/18 13:53, hao_zhang wrote:
>> This patch add allwinner sun8i pwm support.
>
> Again, the subject line is too generic. Mention the R40?
>
> Can you elaborate here a bit? Mention that is used on the R40, but not
> other sun8i SoCs, for instance. And mention that this is very different
> from the sun4i-pwm device, so justifies a new driver. Possibly mention
> some features? And that we for now just implement a subset of them.
Thanks for reviews, elaborate it next patch:)
>
>>
>> Signed-off-by: hao_zhang <hao5781286@gmail.com>
>> ---
>> drivers/pwm/Kconfig | 10 ++
>> drivers/pwm/Makefile | 1 +
>> drivers/pwm/pwm-sun8i.c | 401 ++++++++++++++++++++++++++++++++++++++++++++++++
>
> I am not too happy with this name, but I guess there are no better
> alternatives, so it's probably OK to keep it.
>
>> 3 files changed, 412 insertions(+)
>> create mode 100644 drivers/pwm/pwm-sun8i.c
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index 763ee50..7e68d0f 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -444,6 +444,16 @@ config PWM_SUN4I
>> To compile this driver as a module, choose M here: the module
>> will be called pwm-sun4i.
>>
>> +config PWM_SUN8I
>> + tristate "Allwinner PWM SUN8I support"
>> + depends on ARCH_SUNXI || COMPILE_TEST
>> + depends on HAS_IOMEM && COMMON_CLK
>> + help
>> + Generic PWM framework driver for Allwinner SoCs.
>
> Mmh, not really. So far there is only one SoC using this. Maybe:
> Driver for the enhanced PWM IP used in some newer Allwinner
> SoCs.
>
>> +
>> + To compile this driver as a module, choose M here: the module
>> + will be called pwm-sun8i.
>> +
>> config PWM_TEGRA
>> tristate "NVIDIA Tegra PWM support"
>> depends on ARCH_TEGRA
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index 0258a74..cd6bf40 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_STM32) += pwm-stm32.o
>> obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o
>> obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
>> obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
>> +obj-$(CONFIG_PWM_SUN8I) += pwm-sun8i.o
>> obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
>> obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
>> obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
>> diff --git a/drivers/pwm/pwm-sun8i.c b/drivers/pwm/pwm-sun8i.c
>> new file mode 100644
>> index 0000000..cf23b0a
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-sun8i.c
>> @@ -0,0 +1,401 @@
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/time.h>
>> +#include <linux/regmap.h>
>> +
>> +#define PWM_IRQ_ENABLE_REG 0x0000
>> +#define PCIE(ch) BIT(ch)
>
> Can you please align those:
> #define PWM_IRQ_ENABLE_REG 0x0000
> #define PCIE(ch) BIT(ch)
>
> And all those below as well? Which means you might want to insert
> another tab to cater for those longer symbols.
yep, align it is batter :-)
>
>> +
>> +#define PWM_IRQ_STATUS_REG 0x0004
>> +#define PIS(ch) BIT(ch)
>> +
>> +#define CAPTURE_IRQ_ENABLE_REG 0x0010
>> +#define CFIE(ch) BIT(ch << 1 + 1)
>> +#define CRIE(ch) BIT(ch << 1)
>> +
>> +#define CAPTURE_IRQ_STATUS_REG 0x0014
>> +#define CFIS(ch) BIT(ch << 1 + 1)
>> +#define CRIS(ch) BIT(ch << 1)
>> +
>> +#define CLK_CFG_REG(ch) (0x0020 + (ch >> 1) * 4)
>> +#define CLK_SRC BIT(7)
>> +#define CLK_SRC_BYPASS_SEC BIT(6)
>> +#define CLK_SRC_BYPASS_FIR BIT(5)
>> +#define CLK_GATING BIT(4)
>> +#define CLK_DIV_M GENMASK(3, 0)
>> +
>> +#define PWM_DZ_CTR_REG(ch) (0x0030 + (ch >> 1) * 4)
>> +#define PWM_DZ_INTV GENMASK(15, 8)
>> +#define PWM_DZ_EN BIT(0)
>> +
>> +#define PWM_ENABLE_REG 0x0040
>> +#define PWM_EN(ch) BIT(ch)
>> +
>> +#define CAPTURE_ENABLE_REG 0x0044
>> +#define CAP_EN(ch) BIT(ch)
>> +
>> +#define PWM_CTR_REG(ch) (0x0060 + ch * 0x20)
>> +#define PWM_PERIOD_RDY BIT(11)
>> +#define PWM_PUL_START BIT(10)
>> +#define PWM_MODE BIT(9)
>> +#define PWM_ACT_STA BIT(8)
>> +#define PWM_PRESCAL_K GENMASK(7, 0)
>> +
>> +#define PWM_PERIOD_REG(ch) (0x0064 + ch * 0x20)
>> +#define PWM_ENTIRE_CYCLE GENMASK(31, 16)
>> +#define PWM_ACT_CYCLE GENMASK(15, 0)
>> +
>> +#define PWM_CNT_REG(ch) (0x0068 + ch * 0x20)
>> +#define PWM_CNT_VAL GENMASK(15, 0)
>> +
>> +#define CAPTURE_CTR_REG(ch) (0x006c + ch * 0x20)
>> +#define CAPTURE_CRLF BIT(2)
>> +#define CAPTURE_CFLF BIT(1)
>> +#define CAPINV BIT(0)
>> +
>> +#define CAPTURE_RISE_REG(ch) (0x0070 + ch * 0x20)
>> +#define CAPTURE_CRLR GENMASK(15, 0)
>> +
>> +#define CAPTURE_FALL_REG(ch) (0x0074 + ch * 0x20)
>> +#define CAPTURE_CFLR GENMASK(15, 0)
>> +
>> +struct sun8i_pwm_data {
>> + bool has_prescaler_bypass;
>> + bool has_rdy;
>> + unsigned int npwm;
>> +};
>
> I believe you don't need this structure. See below.
yep, clock will output directly while bypass has been set,
and equivalent to 50% duty cycles...
>
>> +
>> +struct sun8i_pwm_chip {
>> + struct pwm_chip chip;
>> + struct clk *clk;
>> + void __iomem *base;
>> + spinlock_t ctrl_lock;
>> + const struct sun8i_pwm_data *data;
>> + struct regmap *regmap;
>> +};
>> +
>> +static const u16 div_m_table[] = {
>> + 1,
>> + 2,
>> + 4,
>> + 8,
>> + 16,
>> + 32,
>> + 64,
>> + 128,
>> + 256
>> +};
>
> That looks very much like: "1U << x" to me.
uhmm, i think using table is more explicit and extended...
>
>> +
>> +static inline struct sun8i_pwm_chip *to_sun8i_pwm_chip(struct pwm_chip *chip)
>
> No need for "inline", the compiler knows better. static is enough.
okey :-)
>
>> +{
>> + return container_of(chip, struct sun8i_pwm_chip, chip);
>> +}
>> +
>> +static u32 sun8i_pwm_read(struct sun8i_pwm_chip *sun8i_pwm,
>> + unsigned long offset)
>
> Can you please align those continuation lines properly? The first
> character in the new line should be aligned to the first character of
> the first argument. Use tabs first, then fill up with spaces:
Align it next :-)
>
> static u32 sun8i_pwm_read(struct sun8i_pwm_chip *sun8i_pwm,
> unsigned long offset)
>
> This applies to the rest of the file as well.
>
>> +{
>> + u32 val;
>> +
>> + regmap_read(sun8i_pwm->regmap, offset, &val);
>> +
>> + return val;
>> +}
>> +
>> +static inline void sun8i_pwm_set_bit(struct sun8i_pwm_chip *sun8i_pwm,
>
> no inline (for those below as well)
>
>> + unsigned long reg, u32 bit)
>> +{
>> + regmap_update_bits(sun8i_pwm->regmap, reg, bit, bit);
>> +}
>> +
>> +static inline void sun8i_pwm_clear_bit(struct sun8i_pwm_chip *sun8i_pwm,
>> + unsigned long reg, u32 bit)
>> +{
>> + regmap_update_bits(sun8i_pwm->regmap, reg, bit, 0);
>> +}
>> +
>> +static inline void sun8i_pwm_set_value(struct sun8i_pwm_chip *sun8i_pwm,
>> + unsigned long reg, u32 mask, u32 val)
>> +{
>> + regmap_update_bits(sun8i_pwm->regmap, reg, mask, val);
>> +}
>> +
>> +static void sun8i_pwm_set_polarity(struct sun8i_pwm_chip *chip, u32 ch,
>> + enum pwm_polarity polarity)
>> +{
>> + if (polarity == PWM_POLARITY_NORMAL)
>> + sun8i_pwm_set_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
>> + else
>> + sun8i_pwm_clear_bit(chip, PWM_CTR_REG(ch), PWM_ACT_STA);
>> +}
>> +
>> +static int sun8i_pwm_config(struct sun8i_pwm_chip *sun8i_pwm, u8 ch,
>> + struct pwm_state *state)
>> +{
>> + u64 clk_rate, clk_div, val;
>> + u16 prescaler = 0;
>> + u8 id = 0;
>> +
>> + clk_rate = clk_get_rate(sun8i_pwm->clk);
>> +
>> + if (clk_rate == 24000000)
>> + sun8i_pwm_clear_bit(sun8i_pwm, CLK_CFG_REG(ch), CLK_SRC);
>> + else
>> + sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch), CLK_SRC);
>
> This hardcoded 24MHz looks slightly dodgy and should be replaced with
> some proper code to select the best matching clock, out of a number of
> them given in the DT (see the DT binding mail).
> Without thinking too deeply about it, I guess we try which clocks gives
> the least error for the given configuration. The frequency alone might
> be a good first guide.
> If you can't be bothered with coding this, we might just go ahead with
> the first specified clock and always use this, for now.
Dose the framework support parse 2 or more clk from DT ?
yep, It is better to set the clk automatically
>
>> +
>> + if (sun8i_pwm->data->has_prescaler_bypass) {
>
> What is this about? I think this is a misunderstanding:
> The bypass bits allows to directly pass on the input clock to the output
> pin, without any actual PWM properties. So if one channel is (by
> chance?) configured for a 50% duty cycle and the same frequency as one
> of the input clocks, you might want to use the bypass bit instead. But I
> don't see many advantages in doing so, so I guess we can ignore it in a
> generic PWM driver.
> Anyway using some hardcoded value from the "data" structure looks just
> wrong to me. I guess you can just remove this, along with the
> has_prescaler_bypass variable from the sun8i_pwm_data structure.
Agree to remove it.
>
>> + /* pwm output bypass */
>> + if (ch % 2)
>> + sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch),
>> + CLK_SRC_BYPASS_FIR);
>> + else
>> + sun8i_pwm_set_bit(sun8i_pwm, CLK_CFG_REG(ch),
>> + CLK_SRC_BYPASS_SEC);
>> + return 0;
>> + }
>> +
>> + val = state->period * clk_rate;
>> + do_div(val, NSEC_PER_SEC);
>> + if (val < 1) {
>> + dev_err(sun8i_pwm->chip.dev,
>> + "Period expects a larger value\n");
>
> Alignment.
> And you might want to hook in here to select a higher frequency input clock.
>
>> + return -EINVAL;
>> + }
>> +
>> + /* calculate and set prescalar, div table, pwn entrie cycle */
>
> prescaler PWM entire
>
> though I believe this "entire cycle" term is an Allwinner invention.
> Wouldn't period be a better term here, also matching the framework?
It seem no...
referent the manual, "entire cycle" seem means the count of
prescaler_clk(divide by prescaler),
you shoule multiply Tprescaler_clk, then is Tperiod.
>
>> + clk_div = val;
>> +
>> + while (clk_div > 65535) {
>> + prescaler++;
>> + clk_div = val;
>> + do_div(clk_div, prescaler + 1);
>> + do_div(clk_div, div_m_table[id]);
>
> 1U << id
>
>> +
>> + if (prescaler == 255) {
>> + prescaler = 0;
>> + id++;
>> + if (id == 9)
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + sun8i_pwm_set_value(sun8i_pwm, PWM_PERIOD_REG(ch),
>> + PWM_ENTIRE_CYCLE, clk_div << 16);
>> + sun8i_pwm_set_value(sun8i_pwm, PWM_CTR_REG(ch),
>> + PWM_PRESCAL_K, prescaler << 0);
>> + sun8i_pwm_set_value(sun8i_pwm, CLK_CFG_REG(ch),
>> + CLK_DIV_M, id << 0);
>> +
>> + /* set duty cycle */
>> + val = (prescaler + 1) * div_m_table[id] * clk_div;
>
> (1U << id)
>
> You might want to check for the range, though.
Yep :-)
>
>> + val = state->period;
>> + do_div(val, clk_div);
>> + clk_div = state->duty_cycle;
>> + do_div(clk_div, val);
>> +
>> + sun8i_pwm_set_value(sun8i_pwm, PWM_PERIOD_REG(ch),
>> + PWM_ACT_CYCLE, clk_div << 0);
>> +
>> + return 0;
>> +}
>> +
>> +static int sun8i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>> + struct pwm_state *state)
>> +{
>> + int ret;
>> + struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
>> + struct pwm_state cstate;
>> +
>> + pwm_get_state(pwm, &cstate);
>> + if (!cstate.enabled) {
>> + ret = clk_prepare_enable(sun8i_pwm->clk);
>> + if (ret) {
>> + dev_err(chip->dev, "Failed to enable PWM clock\n");
>> + return ret;
>> + }
>> + }
>> +
>> + spin_lock(&sun8i_pwm->ctrl_lock);
>> +
>> + if ((cstate.period != state->period) ||
>> + (cstate.duty_cycle != state->duty_cycle)) {
>> + ret = sun8i_pwm_config(sun8i_pwm, pwm->hwpwm, state);
>> + if (ret) {
>> + spin_unlock(&sun8i_pwm->ctrl_lock);
>> + dev_err(chip->dev, "Failed to config PWM\n");
>> + return ret;
>> + }
>> + }
>> +
>> + if (state->polarity != cstate.polarity)
>> + sun8i_pwm_set_polarity(sun8i_pwm, pwm->hwpwm, state->polarity);
>> +
>> + if (state->enabled) {
>> + sun8i_pwm_set_bit(sun8i_pwm,
>> + CLK_CFG_REG(pwm->hwpwm), CLK_GATING);
>> +
>> + sun8i_pwm_set_bit(sun8i_pwm,
>> + PWM_ENABLE_REG, PWM_EN(pwm->hwpwm));
>> + } else {
>> + sun8i_pwm_clear_bit(sun8i_pwm,
>> + CLK_CFG_REG(pwm->hwpwm), CLK_GATING);
>> +
>> + sun8i_pwm_clear_bit(sun8i_pwm,
>> + PWM_ENABLE_REG, PWM_EN(pwm->hwpwm));
>> + }
>> +
>> + spin_unlock(&sun8i_pwm->ctrl_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static void sun8i_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>> + struct pwm_state *state)
>> +{
>> + struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
>> + u64 clk_rate, tmp;
>> + u32 val;
>> + u16 clk_div, act_cycle;
>> + u8 prescal, id;
>
> You might want to add a channel variable to increase readability:
> int channel = pwm->hwpwm;
>
Okey
>> +
>> + clk_rate = clk_get_rate(sun8i_pwm->clk);
>> +
>> + val = sun8i_pwm_read(sun8i_pwm, PWM_CTR_REG(pwm->hwpwm));
>> + if (PWM_ACT_STA & val)
>> + state->polarity = PWM_POLARITY_NORMAL;
>> + else
>> + state->polarity = PWM_POLARITY_INVERSED;
>> +
>> + prescal = PWM_PRESCAL_K & val;
>> +
>> + val = sun8i_pwm_read(sun8i_pwm, PWM_ENABLE_REG);
>> + if (PWM_EN(pwm->hwpwm) & val)
>> + state->enabled = true;
>> + else
>> + state->enabled = false;
>> +
>> + val = sun8i_pwm_read(sun8i_pwm, PWM_PERIOD_REG(pwm->hwpwm));
>> + act_cycle = PWM_ACT_CYCLE & val;
>> + clk_div = val >> 16;
>> +
>> + val = sun8i_pwm_read(sun8i_pwm, CLK_CFG_REG(pwm->hwpwm));
>> + id = CLK_DIV_M & val;
>> +
>> + tmp = act_cycle * prescal * div_m_table[id] * NSEC_PER_SEC;
>> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
>> + tmp = clk_div * prescal * div_m_table[id] * NSEC_PER_SEC;
>> + state->period = DIV_ROUND_CLOSEST_ULL(tmp, clk_rate);
>> +}
>> +
>> +static const struct regmap_config sun8i_pwm_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = CAPTURE_FALL_REG(7),
>> +};
>> +
>> +static const struct pwm_ops sun8i_pwm_ops = {
>> + .apply = sun8i_pwm_apply,
>> + .get_state = sun8i_pwm_get_state,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static const struct sun8i_pwm_data sun8i_pwm_data_r40 = {
>> + .has_prescaler_bypass = false,
>
> This is not needed (see my comment above).
yep.
>
>> + .has_rdy = true,
>
> And this is not used. Copied from sun4i? Where it interestingly isn't
> used either ;-)
>
>> + .npwm = 8,
>
> I would really love to see this being moved to the DT (see my other mail
> to Thierry about the generic property).
>
> This would mean you don't need a SoC specific structure at all.
okey.
>
>> +};
>> +
>> +static const struct of_device_id sun8i_pwm_dt_ids[] = {
>> + {
>> + .compatible = "allwinner,sun8i-r40-pwm",
>> + .data = &sun8i_pwm_data_r40,
>> + },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sun8i_pwm_dt_ids);
>> +
>> +static int sun8i_pwm_probe(struct platform_device *pdev)
>> +{
>> + struct sun8i_pwm_chip *pwm;
>> + struct resource *res;
>> + int ret;
>> + const struct of_device_id *match;
>> +
>> + match = of_match_device(sun8i_pwm_dt_ids, &pdev->dev);
>> + if (!match) {
>> + dev_err(&pdev->dev, "Error: No device match found\n");
>> + return -ENODEV;
>> + }
>> +
>> + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
>> + if (!pwm)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + pwm->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(pwm->base))
>> + return PTR_ERR(pwm->base);
>> +
>> + pwm->regmap = devm_regmap_init_mmio(&pdev->dev, pwm->base,
>> + &sun8i_pwm_regmap_config);
>> + if (IS_ERR(pwm->regmap)) {
>> + dev_err(&pdev->dev, "Failed to create regmap\n");
>> + return PTR_ERR(pwm->regmap);
>> + }
>> +
>> + pwm->clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(pwm->clk))
>> + return PTR_ERR(pwm->clk);
>
> This would need to be extended to get multiple clocks.
okey.
>
>> +
>> + pwm->data = match->data;
>> + pwm->chip.dev = &pdev->dev;
>> + pwm->chip.ops = &sun8i_pwm_ops;
>> + pwm->chip.base = -1;
>> + pwm->chip.npwm = pwm->data->npwm;
>
> It should be fairly easy to initialise this from some DT property.
>
> That's it for the my first review round. Haven't checked the actual
> algorithm and bit assignments yet.
> Did you manage to test this?
Sure :-)
All has been tested on my T3 board (compatible V40, R40)
PWM signal is work well observe from oscilloscope.
>
> Cheers,
> Andre.
>
>> + pwm->chip.of_xlate = of_pwm_xlate_with_flags;
>> + pwm->chip.of_pwm_n_cells = 3;
>> +
>> + spin_lock_init(&pwm->ctrl_lock);
>> +
>> + ret = pwmchip_add(&pwm->chip);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Failed to add PWM chip: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, pwm);
>> +
>> + return 0;
>> +}
>> +
>> +static int sun8i_pwm_remove(struct platform_device *pdev)
>> +{
>> + struct sun8i_pwm_chip *pwm = platform_get_drvdata(pdev);
>> +
>> + return pwmchip_remove(&pwm->chip);
>> +}
>> +
>> +static struct platform_driver sun8i_pwm_driver = {
>> + .driver = {
>> + .name = "sun8i-pwm",
>> + .of_match_table = sun8i_pwm_dt_ids,
>> + },
>> + .probe = sun8i_pwm_probe,
>> + .remove = sun8i_pwm_remove,
>> +};
>> +module_platform_driver(sun8i_pwm_driver);
>> +
>> +MODULE_ALIAS("platform: sun8i-pwm");
>> +MODULE_AUTHOR("Hao Zhang <hao5781286@gmail.com>");
>> +MODULE_DESCRIPTION("Allwinner sun8i PWM driver");
>> +MODULE_LICENSE("GPL v2");
>>
>
^ permalink raw reply
* [PATCH] ARM: dts: socfpga: Fix NAND controller node compatible for Arria10
From: Dinh Nguyen @ 2018-05-14 15:30 UTC (permalink / raw)
To: linux-arm-kernel
The NAND compatible "denali,denal-nand-dt" property has never been used and
is obsolete. Remove it.
Cc: stable at vger.kernel.org
Fixes: f549af06e9b6("ARM: dts: socfpga: Add NAND device tree for Arria10")
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
arch/arm/boot/dts/socfpga_arria10.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index bead79e4b2aa..d8b1aa309f76 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -633,7 +633,7 @@
nand: nand at ffb90000 {
#address-cells = <1>;
#size-cells = <1>;
- compatible = "denali,denali-nand-dt", "altr,socfpga-denali-nand";
+ compatible = "altr,socfpga-denali-nand";
reg = <0xffb90000 0x72000>,
<0xffb80000 0x10000>;
reg-names = "nand_data", "denali_reg";
--
2.17.0.391.g1f1cddd
^ permalink raw reply related
* [PATCH] ARM: dts: socfpga: Fix NAND controller node compatible
From: Dinh Nguyen @ 2018-05-14 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73h8nak8y3.fsf@pengutronix.de>
On 05/14/2018 02:11 AM, Steffen Trumtrar wrote:
>
> Marek Vasut <marex@denx.de> writes:
>
>> The compatible string for the Denali NAND controller is incorrect,
>> fix it by replacing it with one matching the DT bindings and the
>> driver.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Fixes: d837a80d19 ("ARM: dts: socfpga: add nand controller nodes")
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> ?arch/arm/boot/dts/socfpga.dtsi | 2 +-
>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi
>> b/arch/arm/boot/dts/socfpga.dtsi
>> index 7e24dc8e82d4..d697f5062624 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -744,7 +744,7 @@
>> ???????? nand0: nand at ff900000 {
>> ???????????? #address-cells = <0x1>;
>> ???????????? #size-cells = <0x1>;
>> -??????????? compatible = "denali,denali-nand-dt";
>> +??????????? compatible = "altr,socfpga-denali-nand";
>> ???????????? reg = <0xff900000 0x100000>,
>> ?????????????????? <0xffb80000 0x10000>;
>> ???????????? reg-names = "nand_data", "denali_reg";
>
> Ack. As the binding was changed after d837a80d19 this is obviously
> correct.
>
> @Dinh: Maybe you also want to cleanup the socfpga_arria10.dtsi? It has
> both compatibles, but the "denali,denali-nand-dt" seems to be obsolete.
>
Applied and cc'd stable.
Will fix up for Arria10.
Thanks
Dinh
^ permalink raw reply
* [PATCH] ARM: dts: socfpga: Fix NAND controller node compatible
From: Dinh Nguyen @ 2018-05-14 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73h8nak8y3.fsf@pengutronix.de>
On 05/14/2018 02:11 AM, Steffen Trumtrar wrote:
>
> Marek Vasut <marex@denx.de> writes:
>
>> The compatible string for the Denali NAND controller is incorrect,
>> fix it by replacing it with one matching the DT bindings and the
>> driver.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Fixes: d837a80d19 ("ARM: dts: socfpga: add nand controller nodes")
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> ?arch/arm/boot/dts/socfpga.dtsi | 2 +-
>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi
>> b/arch/arm/boot/dts/socfpga.dtsi
>> index 7e24dc8e82d4..d697f5062624 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -744,7 +744,7 @@
>> ???????? nand0: nand at ff900000 {
>> ???????????? #address-cells = <0x1>;
>> ???????????? #size-cells = <0x1>;
>> -??????????? compatible = "denali,denali-nand-dt";
>> +??????????? compatible = "altr,socfpga-denali-nand";
>> ???????????? reg = <0xff900000 0x100000>,
>> ?????????????????? <0xffb80000 0x10000>;
>> ???????????? reg-names = "nand_data", "denali_reg";
>
> Ack. As the binding was changed after d837a80d19 this is obviously
> correct.
>
> @Dinh: Maybe you also want to cleanup the socfpga_arria10.dtsi? It has
> both compatibles, but the "denali,denali-nand-dt" seems to be obsolete.
>
Applied and cc'd stable.
Will fix up for Arria10.
Thanks
Dinh
^ permalink raw reply
* [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks
From: David Lechner @ 2018-05-14 15:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7cd9d990-26d5-f7d4-291b-a4e4e905cabc@ti.com>
On 05/14/2018 04:49 AM, Sekhar Nori wrote:
> Hi David,
>
> On Saturday 12 May 2018 07:12 AM, David Lechner wrote:
>> On 05/11/2018 09:10 AM, Sekhar Nori wrote:
>>> Fix parent of emac and voice codec PSC clocks. This now matches
>>> existing implementation in arch/arm/mach-davinci/dm365.c
>>>
>>> Also, there is only one power domain on DM365. Fix the power
>>> domain of voice codec and vpss dac modules.
>>>
>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>>> ---
>>> ? drivers/clk/davinci/psc-dm365.c | 6 +++---
>>> ? 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/davinci/psc-dm365.c
>>> b/drivers/clk/davinci/psc-dm365.c
>>> index 5b5b55b0b59a..beeda10fd2f0 100644
>>> --- a/drivers/clk/davinci/psc-dm365.c
>>> +++ b/drivers/clk/davinci/psc-dm365.c
>>> @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info
>>> dm365_psc_info[] = {
>>> ????? LPSC(31, 0, arm,???????? pll2_sysclk2, NULL,
>>> LPSC_ALWAYS_ENABLED),
>>> ????? LPSC(38, 0, spi3,??????? pll1_sysclk4, spi3_clkdev,??????? 0),
>>> ????? LPSC(39, 0, spi4,??????? pll1_auxclk,? spi4_clkdev,??????? 0),
>>> -??? LPSC(40, 0, emac,??????? pll2_sysclk4, emac_clkdev,??????? 0),
>>> -??? LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0),
>>> -??? LPSC(46, 1, vpss_dac,??? pll1_sysclk3, vpss_dac_clkdev,??? 0),
>>> +??? LPSC(40, 0, emac,??????? pll1_sysclk4, emac_clkdev,??????? 0),
>>> +??? LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0),
>>> +??? LPSC(46, 0, vpss_dac,??? pll1_sysclk3, vpss_dac_clkdev,??? 0),
>>> ????? LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0),
>>> ????? LPSC(50, 0, mjcp,??????? pll1_sysclk3, NULL,?????????????? 0),
>>> ????? { }
>>>
>>
>> Slightly off topic...
>>
>> Hmm... Looking at the TRM, I see that there are a bunch of mux clocks
>> that we
>> have not implemented for the DM365 (all of the clocks in the
>> VPSS_CLK_CTRL and
>> PERI_CLKCTL registers). I'm wondering if we should be creating drivers for
>> those like the DA8XX CFGCHIP clock driver.
>
> Yes, but lets leave that to after the current version is merged. That
> way we have one kernel version which is just replacing the private clock
> implementation and that will be good to bisect any regressions.
> VPSS_CLK_CTRL for example is being set directly by the VPSS driver. That
> driver will have to change too once we model the mux as a proper clock.
>
>>
>> Back on topic...
>>
>> The emac fix here looks good.
>
> Okay.
>
>>
>> The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the
>> PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5,
>> It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however
>> this dependency has probably gone unnoticed because so many other devices
>> also use that same clock, it will pretty much always be on. In Figure 38,
>> on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec.
>>
>> So, I am thinking that pll1_sysclk4 is the correct parent for the
>> voice_codec clock here since it is the only one listed in Figure 38
>> (which is in the PSC section of the TRM). The pll2_sysclk4 clock should
>> probably be used by the video codec device driver directly (well, the
>> DIV2 clock really, but we don't have a driver for that yet).
>
> Well the main thing I went with here is compatibility with existing code
> which uses pll2_sysclk4. You are right that pll1_sysclk4 is probably a
> better parent clock to model for the PSC. But, this will break existing
> functionality as no one will be enabling pll2_sysclk4 then. And I
> suspect that will break voice codec driver. The documentation for voice
> codec does not seem to do a good job of explaining what the two clocks
> are for. And without some thorough testing of voice codec (I have never
> used it myself), I would just keep the functionality as-is.
>
> I can add a comment on how the parent was arrived at though, so there is
> some reference to when we look at this again.
>
>>
>> The vpss_dac clock in not very clear in the TRM. In Table 39, the name
>> for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure
>> 37, however, there is not a node that matches exactly. There is "VIDEO
>> DAC", which I take to be the same clock though. This has parent clocks
>> of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6
>> should be the parent here rather than pll1_sysclk3. According to Figure
>> 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the
>> same as the video DAC since they are also listed separately in Figure 5.
>
> The HDVICP is a subsystem in itself, so I am not so sure about that. The
> problem here is compounded by the fact that some of these video
> subsystems are not publicly documented. Here too, I think the safer
> approach is to be compliant with existing code at least for one kernel
> version. Otherwise, we run into the issue of too many things changing at
> the same time making it tough to nail regressions.
>
> we can add a comment here too on how the parent was arrived at (refer
> back to existing code).
>
> Thanks,
> Sekhar
>
Making it just match the existing clock code sounds like a good plan to
me. And it would be nice to add the suggested comments since you will be
doing a v2 for the other patch anyway.
^ permalink raw reply
* [PATCH 07/11] driver core: Respect all error codes from dev_pm_domain_attach()
From: Tony Lindgren @ 2018-05-14 15:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1524732790-2234-8-git-send-email-ulf.hansson@linaro.org>
Ulf,
* Ulf Hansson <ulf.hansson@linaro.org> [180426 09:01]:
> The limitation of being able to check only for -EPROBE_DEFER from
> dev_pm_domain_attach() has been removed. Hence let's respect all error
> codes and bail out accordingly.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/platform.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 8075ddc..9460139 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -572,17 +572,16 @@ static int platform_drv_probe(struct device *_dev)
> return ret;
>
> ret = dev_pm_domain_attach(_dev, true);
> - if (ret != -EPROBE_DEFER) {
> - if (drv->probe) {
> - ret = drv->probe(dev);
> - if (ret)
> - dev_pm_domain_detach(_dev, true);
> - } else {
> - /* don't fail if just dev_pm_domain_attach failed */
> - ret = 0;
> - }
> + if (ret)
> + goto out;
> +
> + if (drv->probe) {
> + ret = drv->probe(dev);
> + if (ret)
> + dev_pm_domain_detach(_dev, true);
> }
>
> +out:
> if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> dev_warn(_dev, "probe deferral not supported\n");
> ret = -ENXIO;
> --
Looks like this causes Linux next to not boot for me with device
probes failing with error -17. So that's at least omaps, looks
like kernelci has others failing too.
Reverting for 8c123c14bbba ("driver core: Respect all error codes from
dev_pm_domain_attach()") fixes the issue for me.
Sounds like something is missing, any ideas?
Regards,
Tony
^ permalink raw reply
* [PATCH v3 4/4] drm/rockchip: support dp training outside dp firmware
From: Sean Paul @ 2018-05-14 15:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526291635-31122-4-git-send-email-hl@rock-chips.com>
On Mon, May 14, 2018 at 05:53:55PM +0800, Lin Huang wrote:
> DP firmware uses fixed phy config values to do training, but some
> boards need to adjust these values to fit for their unique hardware
> design. So if the phy is using custom config values, do software
> link training instead of relying on firmware, if software training
> fail, keep firmware training as a fallback if sw training fails.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - use variable fw_training instead sw_training_success
> - base on DP SPCE, if training fail use lower link rate to retry training
>
> drivers/gpu/drm/rockchip/Makefile | 3 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 24 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +
> drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 416 ++++++++++++++++++++++++
> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 31 +-
> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 38 ++-
> 6 files changed, 501 insertions(+), 13 deletions(-)
> create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index a314e21..b932f62 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>
> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
> + cdn-dp-link-training.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index cce64c1..d9d0d4d 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
> goto out;
> }
> }
> -
> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
> - goto out;
> + if (dp->use_fw_training == true) {
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to idle video %d\n", ret);
> + goto out;
> + }
> }
>
> ret = cdn_dp_config_video(dp);
> @@ -642,11 +644,15 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
> goto out;
> }
>
> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret);
> - goto out;
> + if (dp->use_fw_training == true) {
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to valid video %d\n", ret);
> + goto out;
> + }
> }
> +
> out:
> mutex_unlock(&dp->lock);
> }
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index 46159b2..77a9793 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -84,6 +84,7 @@ struct cdn_dp_device {
> bool connected;
> bool active;
> bool suspended;
> + bool use_fw_training;
>
> const struct firmware *fw; /* cdn dp firmware */
> unsigned int fw_version; /* cdn fw version */
> @@ -106,6 +107,7 @@ struct cdn_dp_device {
> u8 ports;
> u8 lanes;
> int active_port;
> + u8 train_set[4];
>
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> bool sink_has_audio;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> new file mode 100644
> index 0000000..b8fd5bc
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
/snip
> +static int cdn_dp_get_lower_link_rate(struct cdn_dp_device *dp)
> +{
> + if (dp->link.rate == DP_LINK_BW_1_62)
> + return -EINVAL;
> + else if (dp->link.rate == DP_LINK_BW_2_7)
> + dp->link.rate = DP_LINK_BW_1_62;
Extra indent
> + else
> + dp->link.rate = DP_LINK_BW_2_7;
This is better expressed as a switch statement:
switch (dp->link.rate) {
case DP_LINK_BW_1_62:
return -EINVAL;
case DP_LINK_BW_2_7:
dp->link.rate = DP_LINK_BW_1_62:
break;
default:
dp->link.rate = DP_LINK_BW_2_7:
break;
}
You might also consider adding an additional case since there are rates higher
than 5.4GHz. ie:
case DP_LINK_BW_5_4:
dp->link.rate = DP_LINK_BW_2_7:
break;
default:
dp->link.rate = DP_LINK_BW_5_4:
break;
> +
> + return 0;
> +}
> +
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
> +{
> + int ret, stop_err;
> + u8 link_config[2];
> + u32 rate, sink_max, source_max;
> +
> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> + sizeof(dp->dpcd));
> + if (ret < 0) {
> + DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> + return ret;
> + }
> +
> + source_max = dp->lanes;
> + sink_max = drm_dp_max_lane_count(dp->dpcd);
> + dp->link.num_lanes = min(source_max, sink_max);
> +
> + source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
> + sink_max = drm_dp_max_link_rate(dp->dpcd);
> + rate = min(source_max, sink_max);
> + dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
> +
> +retry:
> + /* Write the link configuration data */
> + link_config[0] = dp->link.rate;
> + link_config[1] = dp->link.num_lanes;
> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
> + link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> + drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config, 2);
> +
> + link_config[0] = 0;
> + link_config[1] = 0;
> + if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
> + link_config[1] = DP_SET_ANSI_8B10B;
> + drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
> +
> + ret = cdn_dp_link_training_clock_recovery(dp);
> + if (ret) {
> + if (cdn_dp_get_lower_link_rate(dp)) {
> + DRM_ERROR("training clock recovery fail, err: %d\n",
> + ret);
> + goto stop_training;
> + }
> +
> + /* use lower link rate to retraining */
> + goto retry;
> + }
> +
> + ret = cdn_dp_link_training_channel_equalization(dp);
> + if (ret) {
> + if (cdn_dp_get_lower_link_rate(dp)) {
> + DRM_ERROR("training channel equalization fail, err: %d\n",
> + ret);
> + goto stop_training;
> + }
> +
> + /* use lower link rate to retraining */
> + goto retry;
> + }
> +
> +stop_training:
> + stop_err = cdn_dp_stop_link_train(dp);
> + if (stop_err) {
> + DRM_ERROR("stop training fail, error: %d\n", stop_err);
> + return stop_err;
> + }
> +
> + return ret;
Using labels to do loops reduces readability, it seems like you can also pull
out the downspread control write.
link_config[0] = 0;
link_config[1] = 0;
if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
link_config[1] = DP_SET_ANSI_8B10B;
drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
while (true) {
/* Write the link configuration data */
link_config[0] = dp->link.rate;
link_config[1] = dp->link.num_lanes;
if (drm_dp_enhanced_frame_cap(dp->dpcd))
link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config, 2);
ret = cdn_dp_link_training_clock_recovery(dp);
if (ret) {
if (!cdn_dp_get_lower_link_rate(dp))
continue;
DRM_ERROR("training clock recovery failed: %d\n", ret);
break;
}
ret = cdn_dp_link_training_channel_equalization(dp);
if (ret) {
if (!cdn_dp_get_lower_link_rate(dp))
continue;
DRM_ERROR("training channel eq failed: %d\n", ret);
break;
}
return 0;
}
stop_err = cdn_dp_stop_link_train(dp);
if (stop_err) {
DRM_ERROR("stop training fail, error: %d\n", stop_err);
return stop_err;
}
return ret;
> +
> +stop_training:
> + stop_err = cdn_dp_stop_link_train(dp);
> + if (stop_err) {
> + DRM_ERROR("stop training fail, error: %d\n", stop_err);
> + return stop_err;
> + }
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index 979355d..e1273e6 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -17,7 +17,9 @@
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
> #include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>
> #include "cdn-dp-core.h"
> #include "cdn-dp-reg.h"
> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device *dp, u8 module_id,
> return 0;
> }
>
> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> {
> u8 msg[6];
>
> @@ -609,6 +611,31 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
> {
> int ret;
>
> + /*
> + * DP firmware uses fixed phy config values to do training, but some
> + * boards need to adjust these values to fit for their unique hardware
> + * design. So if the phy is using custom config values, do software
> + * link training instead of relying on firmware, if software training
This comment is no longer accurate.
> + * fail, keep firmware training as a fallback if sw training fails.
> + */
> + ret = cdn_dp_software_train_link(dp);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to do software training %d\n", ret);
> + goto do_fw_training;
> + }
> + ret = cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to write SOURCE_HDTX_CAR register %d\n", ret);
> + goto do_fw_training;
> + }
> + dp->use_fw_training = false;
> + return 0;
> +
> +do_fw_training:
> + dp->use_fw_training = true;
> + DRM_DEV_DEBUG_KMS(dp->dev, "use fw training\n");
> ret = cdn_dp_training_start(dp);
> if (ret) {
> DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n", ret);
> @@ -623,7 +650,7 @@ int cdn_dp_train_link(struct cdn_dp_device *dp)
>
> DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n", dp->link.rate,
> dp->link.num_lanes);
> - return ret;
> + return 0;
> }
>
> int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
>
/snip
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply
* [PATCH 4/4] firmware: arm_sdei: use common SMCCC_CONDUIT_*
From: James Morse @ 2018-05-14 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180503170330.5591-5-mark.rutland@arm.com>
Hi Mark,
On 03/05/18 18:03, Mark Rutland wrote:
> Now that we have common definitions for SMCCC conduits, move the SDEI
> code over to them, and remove the SDEI-specific definitions.
>
> There should be no functional change as a result of this patch.
Thanks for clearing this up!
Acked-by: James Morse <james.morse@arm.com>
James
^ permalink raw reply
* [PATCH] ghes_edac: enable HIP08 platform edac driver
From: James Morse @ 2018-05-14 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514094709.GC23049@pd.tnic>
Hi Borislav,
(CC: +linux-arm list, just in case there is wider discussion)
On 14/05/18 10:47, Borislav Petkov wrote:
> On Mon, May 14, 2018 at 12:11:28PM +0800, Zhengqiang wrote:
>> In ARM64 defconfig, ghes_edac is default load. memory error report to
>> user space rasdaemon tool through function ghes_edac_report_mem_error,
>> we need it.
>
> So depending on whether there will be an ARM64 edac driver, we can do
I'm afraid there could be a mix: The v8.2 CPU RAS Extensions mean the kernel can
do kernel first. (I agree for those systems there should only be one edac driver).
For systems without the v8.2 CPU RAS Extensions firmware-first is the only way
of doing it.
> the platform whitelisting on x86 only if ARM prefers to do the reporting
> through ghes_edac. James?
I'm afraid I'd like to keep both doors open. Kernel-first handling will require
some ACPI-table/DT property as some aspects of the CPU extensions aren't
discover-able. Can't we use this to pick up whether the platform supports
firmware-first (HEST and GHES entries) or kernel-first via some as-yet-undefined
HEST bits?
Without GHES entries this code would never be run. So we 'just' need to catch
systems that are describing both. (which can be the platform specific kernel
first bits problem to do)
Thanks,
James
^ permalink raw reply
* [PATCH] ARM: dts: socfpga: Fix NAND controller node compatible
From: Dinh Nguyen @ 2018-05-14 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73h8nak8y3.fsf@pengutronix.de>
On 05/14/2018 02:11 AM, Steffen Trumtrar wrote:
>
> Marek Vasut <marex@denx.de> writes:
>
>> The compatible string for the Denali NAND controller is incorrect,
>> fix it by replacing it with one matching the DT bindings and the
>> driver.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Fixes: d837a80d19 ("ARM: dts: socfpga: add nand controller nodes")
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> ?arch/arm/boot/dts/socfpga.dtsi | 2 +-
>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi
>> b/arch/arm/boot/dts/socfpga.dtsi
>> index 7e24dc8e82d4..d697f5062624 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -744,7 +744,7 @@
>> ???????? nand0: nand at ff900000 {
>> ???????????? #address-cells = <0x1>;
>> ???????????? #size-cells = <0x1>;
>> -??????????? compatible = "denali,denali-nand-dt";
>> +??????????? compatible = "altr,socfpga-denali-nand";
>> ???????????? reg = <0xff900000 0x100000>,
>> ?????????????????? <0xffb80000 0x10000>;
>> ???????????? reg-names = "nand_data", "denali_reg";
>
> Ack. As the binding was changed after d837a80d19 this is obviously
> correct.
>
> @Dinh: Maybe you also want to cleanup the socfpga_arria10.dtsi? It has
> both compatibles, but the "denali,denali-nand-dt" seems to be obsolete.
>
Patch applied and cc'd stable kernel.
Will clean up arria10.
Thanks,
Dinh
^ permalink raw reply
* [PATCH] ARM: dts: socfpga: Fix NAND controller node compatible
From: Dinh Nguyen @ 2018-05-14 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73h8nak8y3.fsf@pengutronix.de>
On 05/14/2018 02:11 AM, Steffen Trumtrar wrote:
>
> Marek Vasut <marex@denx.de> writes:
>
>> The compatible string for the Denali NAND controller is incorrect,
>> fix it by replacing it with one matching the DT bindings and the
>> driver.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Fixes: d837a80d19 ("ARM: dts: socfpga: add nand controller nodes")
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> ?arch/arm/boot/dts/socfpga.dtsi | 2 +-
>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi
>> b/arch/arm/boot/dts/socfpga.dtsi
>> index 7e24dc8e82d4..d697f5062624 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -744,7 +744,7 @@
>> ???????? nand0: nand at ff900000 {
>> ???????????? #address-cells = <0x1>;
>> ???????????? #size-cells = <0x1>;
>> -??????????? compatible = "denali,denali-nand-dt";
>> +??????????? compatible = "altr,socfpga-denali-nand";
>> ???????????? reg = <0xff900000 0x100000>,
>> ?????????????????? <0xffb80000 0x10000>;
>> ???????????? reg-names = "nand_data", "denali_reg";
>
> Ack. As the binding was changed after d837a80d19 this is obviously
> correct.
>
> @Dinh: Maybe you also want to cleanup the socfpga_arria10.dtsi? It has
> both compatibles, but the "denali,denali-nand-dt" seems to be obsolete.
>
Patch applied and cc'd stable kernel.
Will clean up arria10.
Thanks,
Dinh
^ permalink raw reply
* [PATCH] ARM: dts: socfpga: Fix NAND controller node compatible
From: Dinh Nguyen @ 2018-05-14 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73h8nak8y3.fsf@pengutronix.de>
On 05/14/2018 02:11 AM, Steffen Trumtrar wrote:
>
> Marek Vasut <marex@denx.de> writes:
>
>> The compatible string for the Denali NAND controller is incorrect,
>> fix it by replacing it with one matching the DT bindings and the
>> driver.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Fixes: d837a80d19 ("ARM: dts: socfpga: add nand controller nodes")
>> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> ?arch/arm/boot/dts/socfpga.dtsi | 2 +-
>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/socfpga.dtsi
>> b/arch/arm/boot/dts/socfpga.dtsi
>> index 7e24dc8e82d4..d697f5062624 100644
>> --- a/arch/arm/boot/dts/socfpga.dtsi
>> +++ b/arch/arm/boot/dts/socfpga.dtsi
>> @@ -744,7 +744,7 @@
>> ???????? nand0: nand at ff900000 {
>> ???????????? #address-cells = <0x1>;
>> ???????????? #size-cells = <0x1>;
>> -??????????? compatible = "denali,denali-nand-dt";
>> +??????????? compatible = "altr,socfpga-denali-nand";
>> ???????????? reg = <0xff900000 0x100000>,
>> ?????????????????? <0xffb80000 0x10000>;
>> ???????????? reg-names = "nand_data", "denali_reg";
>
> Ack. As the binding was changed after d837a80d19 this is obviously
> correct.
>
> @Dinh: Maybe you also want to cleanup the socfpga_arria10.dtsi? It has
> both compatibles, but the "denali,denali-nand-dt" seems to be obsolete.
>
Patch applied and cc'd stable kernel.
Will clean up arria10.
Thanks,
Dinh
^ permalink raw reply
* [PATCH] arm64: allowing mmap to be traced
From: Christian Hansen (chansen3) @ 2018-05-14 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526308362-45443-1-git-send-email-chansen3@cisco.com>
Indeed, you have the quicker draw. I concede to your patch.
?On 2018-05-14, 10:32 AM, "Christian Hansen (chansen3)" <chansen3@cisco.com> wrote:
Adding missing macro which is present all other system calls to
to mmap declaration for ARM. This allows it to appear as a kernel
tracing target.
Signed-off-by: Christian Hansen <chansen3@cisco.com>
---
arch/arm64/kernel/sys.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
index 72981ba..1ccdbfc 100644
--- a/arch/arm64/kernel/sys.c
+++ b/arch/arm64/kernel/sys.c
@@ -27,9 +27,9 @@
#include <linux/syscalls.h>
#include <asm/cpufeature.h>
-asmlinkage long sys_mmap(unsigned long addr, unsigned long len,
- unsigned long prot, unsigned long flags,
- unsigned long fd, off_t off)
+SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
+ unsigned long, prot, unsigned long, flags,
+ unsigned long, fd, off_t, off)
{
if (offset_in_page(off) != 0)
return -EINVAL;
--
2.5.0
^ permalink raw reply
* [PATCH] ARM: dts: socfpga: Fix NAND controller clock supply
From: Dinh Nguyen @ 2018-05-14 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180510143726.8047-1-marex@denx.de>
On 05/10/2018 09:37 AM, Marek Vasut wrote:
> The Denali NAND x-clock should be supplied by nand_x_clk, not by
> nand_clk. Fix this, otherwise the Denali driver gets incorrect
> clock frequency information and incorrectly configures the NAND
> timing.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Fixes: d837a80d19 ("ARM: dts: socfpga: add nand controller nodes")
> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> ---
> arch/arm/boot/dts/socfpga.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index d697f5062624..e719c20a0e65 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -750,7 +750,7 @@
> reg-names = "nand_data", "denali_reg";
> interrupts = <0x0 0x90 0x4>;
> dma-mask = <0xffffffff>;
> - clocks = <&nand_clk>;
> + clocks = <&nand_x_clk>;
> status = "disabled";
> };
>
>
Applied and cc'ed stable kernel.
Thanks,
Dinh
^ permalink raw reply
* [PATCH 10/18] arm64: convert native/compat syscall entry to C
From: Mark Rutland @ 2018-05-14 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514144331.GL7753@e103592.cambridge.arm.com>
On Mon, May 14, 2018 at 03:43:36PM +0100, Dave Martin wrote:
> On Mon, May 14, 2018 at 12:58:05PM +0100, Mark Rutland wrote:
> > On Mon, May 14, 2018 at 12:07:30PM +0100, Dave Martin wrote:
> > > On Mon, May 14, 2018 at 10:46:32AM +0100, Mark Rutland wrote:
> > > > +{
> > > > + if (!system_supports_sve())
> > > > + return;
> > > > +
> > > > + /*
> > > > + * task_fpsimd_load() won't be called to update CPACR_EL1 in
> > > > + * ret_to_user unless TIF_FOREIGN_FPSTATE is still set, which only
> > > > + * happens if a context switch or kernel_neon_begin() or context
> > > > + * modification (sigreturn, ptrace) intervenes.
> > > > + * So, ensure that CPACR_EL1 is already correct for the fast-path case.
> > > > + */
> > > > + if (test_and_clear_thread_flag(TIF_SVE))
> > > > + sve_user_disable();
> > >
> > > sve_user_disable() is already inline, and incorporates the if()
> > > internally via sysreg_clear_set().
> > >
> > > So, should this just be
> > >
> > > clear_thread_flag(TIF_SVE);
> > > sve_user_disable();
> >
> > Sure. That does mean we'll unconditionally read cpacr_el1, but I assume
> > you're happy with that. I'll note the difference in the commit message.
>
> This is what the code does today, conditioned no system_supports_sve().
>
> I'm assuming that reading CPACR_EL1 is cheap ... or have you come across
> counterexamples to that?
I have no data either way. :)
> > > > +}
> > > > +
> > > > +extern syscall_fn_t sys_call_table[];
> > > > +
> > > > +asmlinkage void el0_svc_handler(struct pt_regs *regs)
> > > > +{
> > >
> > > if (system_supports_sve()) ?
> > >
> > > > + sve_user_disable();
> > >
> > > Or should this be replaced by a call to sve_user_reset()?
> > >
> > > I suspect the latter, since we do want to be clearing TIF_SVE here too.
> >
> > Yes, this was mean to be sve_user_reset().
>
> OK. Just to be clear, I think there should be a system_supports_sve()
> check here (in case that wasn't obvious from my previous reply).
I understood that; the check is inside sve_user_reset(), which I had
mean to call here.
With your above comments, I now have the following:
static inline void sve_user_reset(void)
{
if (!system_supports_sve())
return;
/*
* task_fpsimd_load() won't be called to update CPACR_EL1 in
* ret_to_user unless TIF_FOREIGN_FPSTATE is still set, which only
* happens if a context switch or kernel_neon_begin() or context
* modification (sigreturn, ptrace) intervenes.
* So, ensure that CPACR_EL1 is already correct for the fast-path case.
*/
clear_thread_flag(TIF_SVE);
sve_user_disable();
}
asmlinkage void el0_svc_handler(struct pt_regs *regs)
{
sve_user_reset();
el0_svc_common(regs, regs->regs[8], __NR_syscalls, sys_call_table);
}
... which I think alleviates that concern?
Thanks,
Mark.
^ permalink raw reply
* [PATCH] arm64: defconfig: enable the Armada thermal driver
From: Antoine Tenart @ 2018-05-14 14:58 UTC (permalink / raw)
To: linux-arm-kernel
This patch enables the Armada thermal driver to support thermal
management on Marvell EBU Armada SoCs (7K,8K).
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index ecf613761e78..8f0707bcc54d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -343,6 +343,7 @@ CONFIG_SENSORS_INA2XX=m
CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
CONFIG_CPU_THERMAL=y
CONFIG_THERMAL_EMULATION=y
+CONFIG_ARMADA_THERMAL=y
CONFIG_BRCMSTB_THERMAL=m
CONFIG_EXYNOS_THERMAL=y
CONFIG_RCAR_GEN3_THERMAL=y
--
2.17.0
^ permalink raw reply related
* [PATCH v3 3/4] phy: rockchip-typec: support variable phy config value
From: Sean Paul @ 2018-05-14 14:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526291635-31122-3-git-send-email-hl@rock-chips.com>
On Mon, May 14, 2018 at 05:53:54PM +0800, Lin Huang wrote:
> the phy config values used to fix in dp firmware, but some boards
> need change these values to do training and get the better eye diagram
> result. So support that in phy driver.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - delete need_software_training variable
> - add default phy config value, if dts do not define phy config value, use these value
>
> drivers/phy/rockchip/phy-rockchip-typec.c | 305 ++++++++++++++++++++----------
> include/soc/rockchip/rockchip_phy_typec.h | 63 ++++++
> 2 files changed, 270 insertions(+), 98 deletions(-)
> create mode 100644 include/soc/rockchip/rockchip_phy_typec.h
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 76a4b58..10253ad 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
/snip
>
> +/* default phy config */
> +struct phy_config configs[3][4] = {
static const
Also, configs isn't a good name. How about tcphy_default_config?
> + {{ 0x2a, 0x00 },
Can you please expand the assignment for all of these, ie:
{ .swing = 0x2a, .pe = 0x00 },
> + { 0x1f, 0x15 },
> + { 0x14, 0x22 },
> + { 0x02, 0x2b } },
> +
> + {{ 0x21, 0x00 },
> + { 0x12, 0x15 },
> + { 0x02, 0x22 },
> + { 0, 0 } },
> +
> + {{ 0x15, 0x00 },
> + { 0x00, 0x15 },
> + { 0, 0 },
> + { 0, 0 } },
> +};
> +
/snip
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply
* [PATCH v3 3/3] arm64: Force swiotlb bounce buffering for non-coherent DMA with large CWG
From: Catalin Marinas @ 2018-05-14 14:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180512123829.GA8024@lst.de>
On Sat, May 12, 2018 at 02:38:29PM +0200, Christoph Hellwig wrote:
> On Fri, May 11, 2018 at 02:55:47PM +0100, Catalin Marinas wrote:
> > On systems with a Cache Writeback Granule (CTR_EL0.CWG) greater than
> > ARCH_DMA_MINALIGN, DMA cache maintenance on sub-CWG ranges is not safe,
> > leading to data corruption. If such configuration is detected, the
> > kernel will force swiotlb bounce buffering for all non-coherent devices.
>
> Per the previous discussion I understand that so far this is a
> purely theoretical condition.
That's what we think, at least for publicly available hardware.
> Given that I'd rather avoid commiting this patch and just refuse too
> boot in this case.
I'll keep it to a WARN_TAINT() for now. Given that the warn triggers
only when cache_line_size() > ARCH_DMA_MINALIGN and we keep this
constant unchanged (128), it shouldn't be much different from our
current assumptions and no-one complained of DMA corruption so far.
> In a merge window or two I plan to have a noncoherent flag in struct
> device, at which point we can handle this entirely in common code.
Sounds ok, looking forward to this.
Thanks.
--
Catalin
^ permalink raw reply
* [PATCH 20/21] i2c: stu300: make use of i2c_8bit_addr_from_msg
From: Peter Rosin @ 2018-05-14 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514145330.4857-1-peda@axentia.se>
Because it looks neater.
Also restructure debug output for resends, since that code as a
result is only handling debug output.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/busses/i2c-stu300.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
index dc63236b45b2..e866c481bfc3 100644
--- a/drivers/i2c/busses/i2c-stu300.c
+++ b/drivers/i2c/busses/i2c-stu300.c
@@ -602,20 +602,24 @@ static int stu300_send_address(struct stu300_dev *dev,
u32 val;
int ret;
- if (msg->flags & I2C_M_TEN)
+ if (msg->flags & I2C_M_TEN) {
/* This is probably how 10 bit addresses look */
val = (0xf0 | (((u32) msg->addr & 0x300) >> 7)) &
I2C_DR_D_MASK;
- else
- val = ((msg->addr << 1) & I2C_DR_D_MASK);
+ if (msg->flags & I2C_M_RD)
+ /* This is the direction bit */
+ val |= 0x01;
+ } else {
+ val = i2c_8bit_addr_from_msg(msg);
+ }
- if (msg->flags & I2C_M_RD) {
- /* This is the direction bit */
- val |= 0x01;
- if (resend)
+ if (resend) {
+ if (msg->flags & I2C_M_RD)
dev_dbg(&dev->pdev->dev, "read resend\n");
- } else if (resend)
- dev_dbg(&dev->pdev->dev, "write resend\n");
+ else
+ dev_dbg(&dev->pdev->dev, "write resend\n");
+ }
+
stu300_wr8(val, dev->virtbase + I2C_DR);
/* For 10bit addressing, await 10bit request (EVENT 9) */
--
2.11.0
^ permalink raw reply related
* [PATCH 07/21] i2c: efm32: make use of i2c_8bit_addr_from_msg
From: Peter Rosin @ 2018-05-14 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514145330.4857-1-peda@axentia.se>
Because it looks neater.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/busses/i2c-efm32.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-efm32.c b/drivers/i2c/busses/i2c-efm32.c
index aa336ba89aa3..5f2bab878b2c 100644
--- a/drivers/i2c/busses/i2c-efm32.c
+++ b/drivers/i2c/busses/i2c-efm32.c
@@ -144,8 +144,7 @@ static void efm32_i2c_send_next_msg(struct efm32_i2c_ddata *ddata)
struct i2c_msg *cur_msg = &ddata->msgs[ddata->current_msg];
efm32_i2c_write32(ddata, REG_CMD, REG_CMD_START);
- efm32_i2c_write32(ddata, REG_TXDATA, cur_msg->addr << 1 |
- (cur_msg->flags & I2C_M_RD ? 1 : 0));
+ efm32_i2c_write32(ddata, REG_TXDATA, i2c_8bit_addr_from_msg(cur_msg));
}
static void efm32_i2c_send_next_byte(struct efm32_i2c_ddata *ddata)
--
2.11.0
^ permalink raw reply related
* [PATCH 04/21] i2c: aspeed: make use of i2c_8bit_addr_from_msg
From: Peter Rosin @ 2018-05-14 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514145330.4857-1-peda@axentia.se>
Because it looks neater.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/busses/i2c-aspeed.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 7d4aeb4465b3..60e4d0e939a3 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -335,13 +335,12 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
{
u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
- u8 slave_addr = msg->addr << 1;
+ u8 slave_addr = i2c_8bit_addr_from_msg(msg);
bus->master_state = ASPEED_I2C_MASTER_START;
bus->buf_index = 0;
if (msg->flags & I2C_M_RD) {
- slave_addr |= 1;
command |= ASPEED_I2CD_M_RX_CMD;
/* Need to let the hardware know to NACK after RX. */
if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
--
2.11.0
^ permalink raw reply related
* [PATCH 00/21] i2c: make use of i2c_8bit_addr_from_msg
From: Peter Rosin @ 2018-05-14 14:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi!
The nice little inline i2c_8bit_addr_from_msg is not getting
enough use. This series improves the situation and drops a
bunch of lines in the process.
I have only compile-tested (that part fine, at least over here).
Cheers,
Peter
Peter Rosin (21):
i2c: algo: bit: make use of i2c_8bit_addr_from_msg
i2c: algo: pca: make use of i2c_8bit_addr_from_msg
i2c: algo: pcf: make use of i2c_8bit_addr_from_msg
i2c: aspeed: make use of i2c_8bit_addr_from_msg
i2c: axxia: make use of i2c_8bit_addr_from_msg
i2c: diolan: make use of i2c_8bit_addr_from_msg
i2c: efm32: make use of i2c_8bit_addr_from_msg
i2c: eg20t: make use of i2c_8bit_addr_from_msg
i2c: emev2: make use of i2c_8bit_addr_from_msg
i2c: hix5hd2: make use of i2c_8bit_addr_from_msg
i2c: imx-lpi2c: make use of i2c_8bit_addr_from_msg
i2c: imx: make use of i2c_8bit_addr_from_msg
i2c: kempld: make use of i2c_8bit_addr_from_msg
i2c: mxs: make use of i2c_8bit_addr_from_msg
i2c: ocores: make use of i2c_8bit_addr_from_msg
i2c: pasemi: make use of i2c_8bit_addr_from_msg
i2c: qup: make use of i2c_8bit_addr_from_msg
i2c: rcar: make use of i2c_8bit_addr_from_msg
i2c: riic: make use of i2c_8bit_addr_from_msg
i2c: stu300: make use of i2c_8bit_addr_from_msg
i2c: xiic: make use of i2c_8bit_addr_from_msg
drivers/i2c/algos/i2c-algo-bit.c | 4 +---
drivers/i2c/algos/i2c-algo-pca.c | 5 +----
drivers/i2c/algos/i2c-algo-pcf.c | 5 +----
drivers/i2c/busses/i2c-aspeed.c | 3 +--
drivers/i2c/busses/i2c-axxia.c | 5 +++--
drivers/i2c/busses/i2c-diolan-u2c.c | 11 ++++-------
drivers/i2c/busses/i2c-efm32.c | 3 +--
drivers/i2c/busses/i2c-eg20t.c | 5 ++---
drivers/i2c/busses/i2c-emev2.c | 2 +-
drivers/i2c/busses/i2c-hix5hd2.c | 9 ++-------
drivers/i2c/busses/i2c-imx-lpi2c.c | 4 +---
drivers/i2c/busses/i2c-imx.c | 10 +++++-----
drivers/i2c/busses/i2c-kempld.c | 7 +++----
drivers/i2c/busses/i2c-mxs.c | 9 +++------
drivers/i2c/busses/i2c-ocores.c | 5 +----
drivers/i2c/busses/i2c-pasemi.c | 2 +-
drivers/i2c/busses/i2c-qup.c | 2 +-
drivers/i2c/busses/i2c-rcar.c | 2 +-
drivers/i2c/busses/i2c-riic.c | 5 ++---
drivers/i2c/busses/i2c-stu300.c | 22 +++++++++++++---------
drivers/i2c/busses/i2c-xiic.c | 11 ++---------
21 files changed, 50 insertions(+), 81 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH] PM / Domains: Don't return -EEXIST at attach when PM domain exists
From: Ulf Hansson @ 2018-05-14 14:52 UTC (permalink / raw)
To: linux-arm-kernel
As dev_pm_domain_attach() isn't the only way to assign PM domain pointers
to devices, clearly we must allow a device to have the pointer already
being assigned. For this reason, return 0 instead of -EEXIST.
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Tested-by: Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Krzysztof reported problems for an Exynos5 board, where some devices are added
to their PM domains (genpd) via calling of_genpd_add_device(). As also pointed
out by Sylvester, this leads to probe failure when dev_pm_domain_attach()
returns -EXISTS.
Rafael, potentially this change could be squashed with the recently queued
patch: "PM / Domains: Check for existing PM domain in dev_pm_domain_attach()",
but perhaps its too late for that and the fix is better applied on top!?
---
drivers/base/power/common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index 5e4b481..390868c 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -106,7 +106,7 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
int ret;
if (dev->pm_domain)
- return -EEXIST;
+ return 0;
ret = acpi_dev_pm_attach(dev, power_on);
if (!ret)
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/4] ARM: PWM: add allwinner sun8i pwm support.
From: Hao Zhang @ 2018-05-14 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180226090038.etk5q4pd4rl5dvf6@flea.lan>
2018-02-26 17:00 GMT+08:00 Maxime Ripard <maxime.ripard@bootlin.com>:
> Hi,
>
> Thanks for respinning this serie. It looks mostly good, but you still
> have a quite significant number of checkpatch (--strict) warnings that
> you should address.
Thanks for reviews :) ,i'm sorry for that, it will be fixed next time.
and, besides, in what situation were the checkpatch warning can be ignore?
>
> On Sun, Feb 25, 2018 at 09:53:08PM +0800, hao_zhang wrote:
>> +#define CAPTURE_IRQ_ENABLE_REG 0x0010
>> +#define CFIE(ch) BIT(ch << 1 + 1)
>> +#define CRIE(ch) BIT(ch << 1)
>
> You should also put your argument between parentheses here (and in all
> your other macros).
Do you mean like this ?
#define CFIE(ch) BIT((ch) << 1 + 1)
#define CRIE(ch) BIT((ch) << 1)
>
>> +static const u16 div_m_table[] = {
>> + 1,
>> + 2,
>> + 4,
>> + 8,
>> + 16,
>> + 32,
>> + 64,
>> + 128,
>> + 256
>> +};
>
> If this is just a power of two, you can use either the power of two /
> ilog2 to switch back and forth, instead of using that table.
I think using table is more explicit and extended...
>
>> +static int sun8i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>> + struct pwm_state *state)
>> +{
>> + int ret;
>> + struct sun8i_pwm_chip *sun8i_pwm = to_sun8i_pwm_chip(chip);
>> + struct pwm_state cstate;
>> +
>> + pwm_get_state(pwm, &cstate);
>> + if (!cstate.enabled) {
>> + ret = clk_prepare_enable(sun8i_pwm->clk);
>> + if (ret) {
>> + dev_err(chip->dev, "Failed to enable PWM clock\n");
>> + return ret;
>> + }
>> + }
>> +
>> + spin_lock(&sun8i_pwm->ctrl_lock);
>
> What do you need that spinlock for? Can you use a mutex instead?
It should be remove.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox