* [PATCH V8 3/3] irqchip: qcom: Add IRQ combiner driver
From: Marc Zyngier @ 2016-12-13 15:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <78bfd90d47200347628f7dd98451122f@codeaurora.org>
On 13/12/16 15:23, Agustin Vega-Frias wrote:
> On 2016-12-07 13:16, Marc Zyngier wrote:
>> Hi Agustin,
>>
>> On 29/11/16 22:57, Agustin Vega-Frias wrote:
>>> Driver for interrupt combiners in the Top-level Control and Status
>>> Registers (TCSR) hardware block in Qualcomm Technologies chips.
>>>
>>> An interrupt combiner in this block combines a set of interrupts by
>>> OR'ing the individual interrupt signals into a summary interrupt
>>> signal routed to a parent interrupt controller, and provides read-
>>> only, 32-bit registers to query the status of individual interrupts.
>>> The status bit for IRQ n is bit (n % 32) within register (n / 32)
>>> of the given combiner. Thus, each combiner can be described as a set
>>> of register offsets and the number of IRQs managed.
>>>
>>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>>> ---
>>> drivers/irqchip/Kconfig | 8 +
>>> drivers/irqchip/Makefile | 1 +
>>> drivers/irqchip/qcom-irq-combiner.c | 337
>>> ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 346 insertions(+)
>>> create mode 100644 drivers/irqchip/qcom-irq-combiner.c
>>>
>>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>>> index bc0af33..610f902 100644
>>> --- a/drivers/irqchip/Kconfig
>>> +++ b/drivers/irqchip/Kconfig
>>> @@ -279,3 +279,11 @@ config EZNPS_GIC
>>> config STM32_EXTI
>>> bool
>>> select IRQ_DOMAIN
>>> +
>>> +config QCOM_IRQ_COMBINER
>>> + bool "QCOM IRQ combiner support"
>>> + depends on ARCH_QCOM
>>> + select IRQ_DOMAIN
>>> + help
>>> + Say yes here to add support for the IRQ combiner devices embedded
>>> + in Qualcomm Technologies chips.
>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>> index e4dbfc8..1818a0b 100644
>>> --- a/drivers/irqchip/Makefile
>>> +++ b/drivers/irqchip/Makefile
>>> @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
>>> obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
>>> obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
>>> obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
>>> +obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
>>> diff --git a/drivers/irqchip/qcom-irq-combiner.c
>>> b/drivers/irqchip/qcom-irq-combiner.c
>>> new file mode 100644
>>> index 0000000..fc25251
>>> --- /dev/null
>>> +++ b/drivers/irqchip/qcom-irq-combiner.c
>>> @@ -0,0 +1,337 @@
>>> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights
>>> reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> modify
>>> + * it under the terms of the GNU General Public License version 2 and
>>> + * only version 2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +/*
>>> + * Driver for interrupt combiners in the Top-level Control and Status
>>> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
>>> + * An interrupt combiner in this block combines a set of interrupts
>>> by
>>> + * OR'ing the individual interrupt signals into a summary interrupt
>>> + * signal routed to a parent interrupt controller, and provides read-
>>> + * only, 32-bit registers to query the status of individual
>>> interrupts.
>>> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
>>> + * of the given combiner. Thus, each combiner can be described as a
>>> set
>>> + * of register offsets and the number of IRQs managed.
>>> + */
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/irqchip/chained_irq.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define REG_SIZE 32
>>> +
>>> +struct combiner_reg {
>>> + void __iomem *addr;
>>> + unsigned long mask;
>>> +};
>>> +
>>> +struct combiner {
>>> + struct irq_chip irq_chip;
>>> + struct irq_domain *domain;
>>> + int parent_irq;
>>> + u32 nirqs;
>>> + u32 nregs;
>>> + struct combiner_reg regs[0];
>>> +};
>>> +
>>> +static inline u32 irq_register(int irq)
>>> +{
>>> + return irq / REG_SIZE;
>>> +}
>>> +
>>> +static inline u32 irq_bit(int irq)
>>> +{
>>> + return irq % REG_SIZE;
>>> +
>>> +}
>>> +
>>> +static inline int irq_nr(u32 reg, u32 bit)
>>> +{
>>> + return reg * REG_SIZE + bit;
>>> +}
>>> +
>>> +/*
>>> + * Handler for the cascaded IRQ.
>>> + */
>>> +static void combiner_handle_irq(struct irq_desc *desc)
>>> +{
>>> + struct combiner *combiner = irq_desc_get_handler_data(desc);
>>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>>> + u32 reg;
>>> +
>>> + chained_irq_enter(chip, desc);
>>> +
>>> + for (reg = 0; reg < combiner->nregs; reg++) {
>>> + int virq;
>>> + int hwirq;
>>> + u32 bit;
>>> + u32 status;
>>> +
>>> + if (combiner->regs[reg].mask == 0)
>>> + continue;
>>> +
>>> + status = readl_relaxed(combiner->regs[reg].addr);
>>> + status &= combiner->regs[reg].mask;
>>> +
>>> + while (status) {
>>> + bit = __ffs(status);
>>> + status &= ~(1 << bit);
>>> + hwirq = irq_nr(reg, bit);
>>> + virq = irq_find_mapping(combiner->domain, hwirq);
>>> + if (virq >= 0)
>>> + generic_handle_irq(virq);
>>> +
>>> + }
>>> + }
>>> +
>>> + chained_irq_exit(chip, desc);
>>> +}
>>> +
>>> +/*
>>> + * irqchip callbacks
>>> + */
>>> +
>>> +static void combiner_irq_chip_mask_irq(struct irq_data *data)
>>> +{
>>> + struct combiner *combiner = irq_data_get_irq_chip_data(data);
>>> + struct combiner_reg *reg = combiner->regs +
>>> irq_register(data->hwirq);
>>> +
>>> + clear_bit(irq_bit(data->hwirq), ®->mask);
>>> +}
>>> +
>>> +static void combiner_irq_chip_unmask_irq(struct irq_data *data)
>>> +{
>>> + struct combiner *combiner = irq_data_get_irq_chip_data(data);
>>> + struct combiner_reg *reg = combiner->regs +
>>> irq_register(data->hwirq);
>>> +
>>> + set_bit(irq_bit(data->hwirq), ®->mask);
>>> +}
>>> +
>>> +/*
>>> + * irq_domain_ops callbacks
>>> + */
>>> +
>>> +static int combiner_irq_map(struct irq_domain *domain, unsigned int
>>> irq,
>>> + irq_hw_number_t hwirq)
>>> +{
>>> + struct combiner *combiner = domain->host_data;
>>> +
>>> + if (hwirq >= combiner->nirqs)
>>> + return -EINVAL;
>>> +
>>> + irq_set_chip_and_handler(irq, &combiner->irq_chip,
>>> handle_level_irq);
>>> + irq_set_chip_data(irq, combiner);
>>> + irq_set_parent(irq, combiner->parent_irq);
>>
>> Do you really need this irq_set_parent? This only makes sense if you're
>> resending edge interrupts, and you seem to be level only.
>
> OK, I'll take a look.
>
>>
>>> + irq_set_noprobe(irq);
>>> + return 0;
>>> +}
>>> +
>>> +static void combiner_irq_unmap(struct irq_domain *domain, unsigned
>>> int irq)
>>> +{
>>> + irq_set_chip_and_handler(irq, NULL, NULL);
>>> + irq_set_chip_data(irq, NULL);
>>> + irq_set_parent(irq, -1);
>>
>> All of this should probably be replaced with a call to
>> irq_domain_reset_irq_data().
>
> Will do.
>
>>
>>> +}
>>> +
>>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>>
>> Is there any case where this is not valid?
>
> What do you mean? Are you asking why is this under a preprocessor
> conditional? If so I did it to be on the safe side since translate
> in struct irq_domain_ops under that conditional.
Since this code can only work when CONFIG_IRQ_DOMAIN_HIERARCHY is
selected, you might as well make the KConfig entry select (or depend on
- I'm always confused about which one should be used when) this
configuration symbol, and make the code more readable.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: PrasannaKumar Muralidharan @ 2016-12-13 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213141059.GB10647@Red>
> What do you think about those two solutions ?
I prefer the second solution's idea of using two files (/dev/hwrng and
/dev/hwprng). Upon having a quick glance it looks like (based on
current_rng == prng check) that your current implementation allows
only one rng device to be in use at a time. It would be better to have
both usable at the same time. So applications that need pseudo random
data at high speed can use /dev/prng while applications that require
true random number can use /dev/rng. Please feel free to correct if my
understanding of the code is incorrect. Along with this change I think
changing the algif_rng to use this code if this solution is going to
be used.
Regards,
PrasannaKumar
^ permalink raw reply
* [PATCH V8 3/3] irqchip: qcom: Add IRQ combiner driver
From: Agustin Vega-Frias @ 2016-12-13 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b009635d-8a10-5e22-4438-0019d79d01d3@arm.com>
On 2016-12-07 13:16, Marc Zyngier wrote:
> Hi Agustin,
>
> On 29/11/16 22:57, Agustin Vega-Frias wrote:
>> Driver for interrupt combiners in the Top-level Control and Status
>> Registers (TCSR) hardware block in Qualcomm Technologies chips.
>>
>> An interrupt combiner in this block combines a set of interrupts by
>> OR'ing the individual interrupt signals into a summary interrupt
>> signal routed to a parent interrupt controller, and provides read-
>> only, 32-bit registers to query the status of individual interrupts.
>> The status bit for IRQ n is bit (n % 32) within register (n / 32)
>> of the given combiner. Thus, each combiner can be described as a set
>> of register offsets and the number of IRQs managed.
>>
>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>> ---
>> drivers/irqchip/Kconfig | 8 +
>> drivers/irqchip/Makefile | 1 +
>> drivers/irqchip/qcom-irq-combiner.c | 337
>> ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 346 insertions(+)
>> create mode 100644 drivers/irqchip/qcom-irq-combiner.c
>>
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index bc0af33..610f902 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -279,3 +279,11 @@ config EZNPS_GIC
>> config STM32_EXTI
>> bool
>> select IRQ_DOMAIN
>> +
>> +config QCOM_IRQ_COMBINER
>> + bool "QCOM IRQ combiner support"
>> + depends on ARCH_QCOM
>> + select IRQ_DOMAIN
>> + help
>> + Say yes here to add support for the IRQ combiner devices embedded
>> + in Qualcomm Technologies chips.
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index e4dbfc8..1818a0b 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -74,3 +74,4 @@ obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
>> obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
>> obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
>> obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
>> +obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
>> diff --git a/drivers/irqchip/qcom-irq-combiner.c
>> b/drivers/irqchip/qcom-irq-combiner.c
>> new file mode 100644
>> index 0000000..fc25251
>> --- /dev/null
>> +++ b/drivers/irqchip/qcom-irq-combiner.c
>> @@ -0,0 +1,337 @@
>> +/* Copyright (c) 2015-2016, The Linux Foundation. All rights
>> reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +/*
>> + * Driver for interrupt combiners in the Top-level Control and Status
>> + * Registers (TCSR) hardware block in Qualcomm Technologies chips.
>> + * An interrupt combiner in this block combines a set of interrupts
>> by
>> + * OR'ing the individual interrupt signals into a summary interrupt
>> + * signal routed to a parent interrupt controller, and provides read-
>> + * only, 32-bit registers to query the status of individual
>> interrupts.
>> + * The status bit for IRQ n is bit (n % 32) within register (n / 32)
>> + * of the given combiner. Thus, each combiner can be described as a
>> set
>> + * of register offsets and the number of IRQs managed.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define REG_SIZE 32
>> +
>> +struct combiner_reg {
>> + void __iomem *addr;
>> + unsigned long mask;
>> +};
>> +
>> +struct combiner {
>> + struct irq_chip irq_chip;
>> + struct irq_domain *domain;
>> + int parent_irq;
>> + u32 nirqs;
>> + u32 nregs;
>> + struct combiner_reg regs[0];
>> +};
>> +
>> +static inline u32 irq_register(int irq)
>> +{
>> + return irq / REG_SIZE;
>> +}
>> +
>> +static inline u32 irq_bit(int irq)
>> +{
>> + return irq % REG_SIZE;
>> +
>> +}
>> +
>> +static inline int irq_nr(u32 reg, u32 bit)
>> +{
>> + return reg * REG_SIZE + bit;
>> +}
>> +
>> +/*
>> + * Handler for the cascaded IRQ.
>> + */
>> +static void combiner_handle_irq(struct irq_desc *desc)
>> +{
>> + struct combiner *combiner = irq_desc_get_handler_data(desc);
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + u32 reg;
>> +
>> + chained_irq_enter(chip, desc);
>> +
>> + for (reg = 0; reg < combiner->nregs; reg++) {
>> + int virq;
>> + int hwirq;
>> + u32 bit;
>> + u32 status;
>> +
>> + if (combiner->regs[reg].mask == 0)
>> + continue;
>> +
>> + status = readl_relaxed(combiner->regs[reg].addr);
>> + status &= combiner->regs[reg].mask;
>> +
>> + while (status) {
>> + bit = __ffs(status);
>> + status &= ~(1 << bit);
>> + hwirq = irq_nr(reg, bit);
>> + virq = irq_find_mapping(combiner->domain, hwirq);
>> + if (virq >= 0)
>> + generic_handle_irq(virq);
>> +
>> + }
>> + }
>> +
>> + chained_irq_exit(chip, desc);
>> +}
>> +
>> +/*
>> + * irqchip callbacks
>> + */
>> +
>> +static void combiner_irq_chip_mask_irq(struct irq_data *data)
>> +{
>> + struct combiner *combiner = irq_data_get_irq_chip_data(data);
>> + struct combiner_reg *reg = combiner->regs +
>> irq_register(data->hwirq);
>> +
>> + clear_bit(irq_bit(data->hwirq), ®->mask);
>> +}
>> +
>> +static void combiner_irq_chip_unmask_irq(struct irq_data *data)
>> +{
>> + struct combiner *combiner = irq_data_get_irq_chip_data(data);
>> + struct combiner_reg *reg = combiner->regs +
>> irq_register(data->hwirq);
>> +
>> + set_bit(irq_bit(data->hwirq), ®->mask);
>> +}
>> +
>> +/*
>> + * irq_domain_ops callbacks
>> + */
>> +
>> +static int combiner_irq_map(struct irq_domain *domain, unsigned int
>> irq,
>> + irq_hw_number_t hwirq)
>> +{
>> + struct combiner *combiner = domain->host_data;
>> +
>> + if (hwirq >= combiner->nirqs)
>> + return -EINVAL;
>> +
>> + irq_set_chip_and_handler(irq, &combiner->irq_chip,
>> handle_level_irq);
>> + irq_set_chip_data(irq, combiner);
>> + irq_set_parent(irq, combiner->parent_irq);
>
> Do you really need this irq_set_parent? This only makes sense if you're
> resending edge interrupts, and you seem to be level only.
OK, I'll take a look.
>
>> + irq_set_noprobe(irq);
>> + return 0;
>> +}
>> +
>> +static void combiner_irq_unmap(struct irq_domain *domain, unsigned
>> int irq)
>> +{
>> + irq_set_chip_and_handler(irq, NULL, NULL);
>> + irq_set_chip_data(irq, NULL);
>> + irq_set_parent(irq, -1);
>
> All of this should probably be replaced with a call to
> irq_domain_reset_irq_data().
Will do.
>
>> +}
>> +
>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>
> Is there any case where this is not valid?
What do you mean? Are you asking why is this under a preprocessor
conditional? If so I did it to be on the safe side since translate
in struct irq_domain_ops under that conditional.
>
>> +static int combiner_irq_translate(struct irq_domain *d, struct
>> irq_fwspec *fws,
>> + unsigned long *hwirq, unsigned int *type)
>> +{
>> + if (is_acpi_node(fws->fwnode)) {
>> + if (fws->param_count != 2)
>> + return -EINVAL;
>> +
>> + *hwirq = fws->param[0];
>> + *type = fws->param[1];
>
> Given that you're only handling level interrupts, shouldn't you abort
> if
> detecting an edge interrupt?
I'll add the check.
>
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +#endif
>> +
>> +static const struct irq_domain_ops domain_ops = {
>> + .map = combiner_irq_map,
>> + .unmap = combiner_irq_unmap,
>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> + .translate = combiner_irq_translate
>> +#endif
>> +};
>> +
>> +/*
>> + * Device probing
>> + */
>> +
>> +#ifdef CONFIG_ACPI
>> +
>> +static acpi_status count_registers_cb(struct acpi_resource *ares,
>> void *context)
>> +{
>> + int *count = context;
>> +
>> + if (ares->type == ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
>> + ++(*count);
>> + return AE_OK;
>> +}
>> +
>> +static int count_registers(struct platform_device *pdev)
>> +{
>> + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>> + acpi_status status;
>> + int count = 0;
>> +
>> + if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
>> + return -EINVAL;
>> +
>> + status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
>> + count_registers_cb, &count);
>> + if (ACPI_FAILURE(status))
>> + return -EINVAL;
>> + return count;
>> +}
>> +
>> +struct get_registers_context {
>> + struct device *dev;
>> + struct combiner *combiner;
>> + int err;
>> +};
>> +
>> +static acpi_status get_registers_cb(struct acpi_resource *ares, void
>> *context)
>> +{
>> + struct get_registers_context *ctx = context;
>> + struct acpi_resource_generic_register *reg;
>> + phys_addr_t paddr;
>> + void __iomem *vaddr;
>> +
>> + if (ares->type != ACPI_RESOURCE_TYPE_GENERIC_REGISTER)
>> + return AE_OK;
>> +
>> + reg = &ares->data.generic_reg;
>> + paddr = reg->address;
>> + if ((reg->space_id != ACPI_SPACE_MEM) ||
>> + (reg->bit_offset != 0) ||
>> + (reg->bit_width > REG_SIZE)) {
>> + dev_err(ctx->dev, "Bad register resource @%pa\n", &paddr);
>> + ctx->err = -EINVAL;
>> + return AE_ERROR;
>> + }
>> +
>> + vaddr = devm_ioremap(ctx->dev, reg->address, REG_SIZE);
>> + if (IS_ERR(vaddr)) {
>> + dev_err(ctx->dev, "Can't map register @%pa\n", &paddr);
>> + ctx->err = PTR_ERR(vaddr);
>> + return AE_ERROR;
>> + }
>> +
>> + ctx->combiner->regs[ctx->combiner->nregs].addr = vaddr;
>> + ctx->combiner->nirqs += reg->bit_width;
>> + ctx->combiner->nregs++;
>> + return AE_OK;
>> +}
>> +
>> +static int get_registers(struct platform_device *pdev, struct
>> combiner *comb)
>> +{
>> + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>> + acpi_status status;
>> + struct get_registers_context ctx;
>> +
>> + if (!acpi_has_method(adev->handle, METHOD_NAME__CRS))
>> + return -EINVAL;
>> +
>> + ctx.dev = &pdev->dev;
>> + ctx.combiner = comb;
>> + ctx.err = 0;
>> +
>> + status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
>> + get_registers_cb, &ctx);
>> + if (ACPI_FAILURE(status))
>> + return ctx.err;
>> + return 0;
>> +}
>> +
>> +#else /* !CONFIG_ACPI */
>> +
>> +static int count_registers(struct platform_device *pdev)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +static int get_registers(struct platform_device *pdev, struct
>> combiner *comb)
>> +{
>> + return -EINVAL;
>> +}
>
> So this driver is obviously ACPI only. Can't you just get rid of these,
> of the #ifdef CONFIG_ACPI, and simply make it depend on ACPI?
Good point, will do.
>
>> +
>> +#endif
>> +
>> +static int __init combiner_probe(struct platform_device *pdev)
>> +{
>> + struct combiner *combiner;
>> + size_t alloc_sz;
>> + u32 nregs;
>> + int err;
>> +
>> + nregs = count_registers(pdev);
>> + if (nregs <= 0) {
>> + dev_err(&pdev->dev, "Error reading register resources\n");
>> + return -EINVAL;
>> + }
>> +
>> + alloc_sz = sizeof(*combiner) + sizeof(struct combiner_reg) * nregs;
>> + combiner = devm_kzalloc(&pdev->dev, alloc_sz, GFP_KERNEL);
>> + if (!combiner)
>> + return -ENOMEM;
>> +
>> + err = get_registers(pdev, combiner);
>> + if (err < 0)
>> + return err;
>> +
>> + combiner->parent_irq = platform_get_irq(pdev, 0);
>> + if (combiner->parent_irq <= 0) {
>> + dev_err(&pdev->dev, "Error getting IRQ resource\n");
>> + return -EINVAL;
>
> Can you ever get in a situation where it'd be more sensible to return
> -EPROBE_DEFER? I'm thinking of a case where you'd have this irq chip
> cascaded into another one, which is not present yet?
Not in the current architecture, but I agree it would accommodate other
use cases. I will change it to return -EPROBE_DEFER.
>
>> + }
>> +
>> + combiner->domain = irq_domain_create_linear(
>> + pdev->dev.fwnode, combiner->nirqs, &domain_ops, combiner);
>
> On a single line, please. Do no listen to the checkpatch police that
> will tell you otherwise. It really hurt my eyes to see this opening
> bracket and *nothing* after that.
Will do.
>
>> + if (!combiner->domain)
>> + /* Errors printed by irq_domain_create_linear */
>> + return -ENODEV;
>> +
>> + irq_set_chained_handler_and_data(combiner->parent_irq,
>> + combiner_handle_irq, combiner);
>> + combiner->irq_chip.irq_mask = combiner_irq_chip_mask_irq;
>> + combiner->irq_chip.irq_unmask = combiner_irq_chip_unmask_irq;
>> + combiner->irq_chip.name = pdev->name;
>
> Arghh. Please don't do that. Once you've called irq_set_chained_*, the
> interrupt is live, and can be requested. Unlikely, but still. In
> general, feeding uninitialized data to a function is pretty bad.
>
> Also, do you really need to show pdev->name in /proc/cpuinfo? Just make
> the irq_chip structure static, outside of combiner, and have "QCOM80B1"
> (or something of your liking) as the name once and for all.
I'll look at this.
Thanks for the detailed feedback.
Agustin.
>
>> +
>> + dev_info(&pdev->dev, "Initialized with [p=%d,n=%d,r=%p]\n",
>> + combiner->parent_irq, combiner->nirqs, combiner->regs[0].addr);
>> + return 0;
>> +}
>> +
>> +static const struct acpi_device_id qcom_irq_combiner_acpi_match[] = {
>> + { "QCOM80B1", },
>> + { }
>> +};
>> +
>> +static struct platform_driver qcom_irq_combiner_probe = {
>> + .driver = {
>> + .name = "qcom-irq-combiner",
>> + .owner = THIS_MODULE,
>> + .acpi_match_table = ACPI_PTR(qcom_irq_combiner_acpi_match),
>> + },
>> + .probe = combiner_probe,
>> +};
>> +
>> +static int __init register_qcom_irq_combiner(void)
>> +{
>> + return platform_driver_register(&qcom_irq_combiner_probe);
>> +}
>> +device_initcall(register_qcom_irq_combiner);
>>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 6/6] ARM: dts: sun8i: raise the max voltage of DCDC2 in sun8i reference tablets
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213152252.53749-1-icenowy@aosc.xyz>
The "extremity_freq" frequency described in the original FEX files uses
a voltage of 1.46v, which is beyond the current maximum voltage value of
DCDC2 (Cortex-A7 supply) in the sun8i reference tablet DTSI file.
Raise the maximum value to 1.46v.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
index 7ac8bb4bc95a..325ca5bd67a5 100644
--- a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
+++ b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
@@ -180,7 +180,7 @@
®_dcdc2 {
regulator-always-on;
regulator-min-microvolt = <900000>;
- regulator-max-microvolt = <1400000>;
+ regulator-max-microvolt = <1460000>;
regulator-name = "vdd-sys";
};
--
2.11.0
^ permalink raw reply related
* [PATCH 5/6] ARM: dts: sun8i: set cpu-supply in reference tablet DTSI
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213152252.53749-1-icenowy@aosc.xyz>
All reference design A33 tablets uses DCDC2 of AXP223 as the power
supply of the Cortex-A7 cores.
Set the cpu-supply in the DTSI of sun8i reference tablets.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
index 08cd00143635..7ac8bb4bc95a 100644
--- a/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
+++ b/arch/arm/boot/dts/sun8i-reference-design-tablet.dtsi
@@ -213,6 +213,10 @@
regulator-name = "vcc-rtc";
};
+&cpu0 {
+ cpu-supply = <®_dcdc2>;
+};
+
&r_uart {
pinctrl-names = "default";
pinctrl-0 = <&r_uart_pins_a>;
--
2.11.0
^ permalink raw reply related
* [PATCH 4/6] ARM: dts: sun8i: add opp-v2 table for A33
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213152252.53749-1-icenowy@aosc.xyz>
An operating point table is needed for the cpu frequency adjusting to
work.
The operating point table is converted from the common value in
extracted script.fex from many A33 board/tablets.
1.344GHz is set as a turbo-mode operating point, as it's described as
"extremity_freq" in the FEX file. (the "max_freq" is 1.2GHz)
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
arch/arm/boot/dts/sun8i-a33.dtsi | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index 504996cbee29..035c058324b8 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -46,7 +46,45 @@
#include <dt-bindings/dma/sun4i-a10.h>
/ {
+ cpu0_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp at 648000000 {
+ opp-hz = /bits/ 64 <648000000>;
+ opp-microvolt = <1040000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp at 816000000 {
+ opp-hz = /bits/ 64 <816000000>;
+ opp-microvolt = <1100000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp at 1008000000 {
+ opp-hz = /bits/ 64 <1008000000>;
+ opp-microvolt = <1200000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp at 1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt = <1320000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ opp at 1344000000 {
+ opp-hz = /bits/ 64 <1344000000>;
+ opp-microvolt = <1460000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ turbo-mode;
+ };
+ };
+
cpus {
+ cpu0: cpu at 0 {
+ clocks = <&ccu CLK_CPUX>;
+ clock-names = "cpu";
+ operating-points-v2 = <&cpu0_opp_table>;
+ };
+
cpu at 2 {
compatible = "arm,cortex-a7";
device_type = "cpu";
--
2.11.0
^ permalink raw reply related
* [PATCH 3/6] ARM: dts: sun8i: add a cpu0 label to cpu@0 node on A23/33
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213152252.53749-1-icenowy@aosc.xyz>
A "cpu0" label is needed on cpu at 0 for cpufreq-dt to work.
Add such a label, in order to prepare for cpufreq support of A23/33.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 817747f41288..5931cc4d1567 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -84,7 +84,7 @@
#address-cells = <1>;
#size-cells = <0>;
- cpu at 0 {
+ cpu0: cpu at 0 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0>;
--
2.11.0
^ permalink raw reply related
* [PATCH 2/6] clk: sunxi-ng: set the parent rate when adjustin CPUX clock on A33
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213152252.53749-1-icenowy@aosc.xyz>
The CPUX clock on A33, which is for the Cortex-A7 cores, is designed to
be changeable by changing the rate of PLL_CPUX.
Add CLK_SET_RATE_PARENT flag to this clock.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
index 0f3e7d2dc19a..0d513d2674cb 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
@@ -170,7 +170,7 @@ static SUNXI_CCU_N_WITH_GATE_LOCK(pll_ddr1_clk, "pll-ddr1",
static const char * const cpux_parents[] = { "osc32k", "osc24M",
"pll-cpux" , "pll-cpux" };
static SUNXI_CCU_MUX(cpux_clk, "cpux", cpux_parents,
- 0x050, 16, 2, CLK_IS_CRITICAL);
+ 0x050, 16, 2, CLK_IS_CRITICAL | CLK_SET_RATE_PARENT);
static SUNXI_CCU_M(axi_clk, "axi", "cpux", 0x050, 0, 2, 0);
--
2.11.0
^ permalink raw reply related
* [PATCH 1/6] clk: sunxi-ng: fix PLL_CPUX adjusting on A33
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213152252.53749-1-icenowy@aosc.xyz>
When adjusting PLL_CPUX on A33, the PLL is temporarily driven too high,
and the system hangs.
Add a notifier to avoid this situation by temporarily switching to a
known stable 24 MHz oscillator.
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
index 3cd4190ccd59..0f3e7d2dc19a 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
@@ -752,6 +752,13 @@ static const struct sunxi_ccu_desc sun8i_a33_ccu_desc = {
.num_resets = ARRAY_SIZE(sun8i_a33_ccu_resets),
};
+static struct ccu_mux_nb sun8i_a33_cpu_nb = {
+ .common = &cpux_clk.common,
+ .cm = &cpux_clk.mux,
+ .delay_us = 1, /* > 8 clock cycles at 24 MHz */
+ .bypass_index = 1, /* index of 24 MHz oscillator */
+};
+
static void __init sun8i_a33_ccu_setup(struct device_node *node)
{
void __iomem *reg;
@@ -775,6 +782,9 @@ static void __init sun8i_a33_ccu_setup(struct device_node *node)
writel(val, reg + SUN8I_A33_PLL_MIPI_REG);
sunxi_ccu_probe(node, reg, &sun8i_a33_ccu_desc);
+
+ ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
+ &sun8i_a33_cpu_nb);
}
CLK_OF_DECLARE(sun8i_a33_ccu, "allwinner,sun8i-a33-ccu",
sun8i_a33_ccu_setup);
--
2.11.0
^ permalink raw reply related
* [PATCH 0/6] Allwinner A33 CPU frequency scaling support
From: Icenowy Zheng @ 2016-12-13 15:22 UTC (permalink / raw)
To: linux-arm-kernel
This series of patch adds frequency scaling support to Allwinner A33 SoC.
The first two patches fixes some bugs in the A33 CCU code.
The patch 3 and 4 is for enabling the cpufreq-dt driver to work.
The patch 5 is for enabling the voltage adjusting on reference design tablets.
The patch 6 is for enabling the "turbo-mode" of A33. (According to the
"extremity_freq" property in the FEX file. When I tested it with 3.4 BSP, it
really performs as a turbo mode.)
If there's any doubt of safety, the patch 6 can be ignored.
If there's any problem in patch 3, 4 and 5, they can also be temporarily
ignored, but finally we need them ;-)
Although there's now currently no thermal support for A33, many A33 devices
are tablets with battery, and it will be valuable to save some power energy,
so cpufreq support is also useful.
P.S.
Chen-Yu,
Do you want to test the CCU fix and the operating point table on A23?
Regards,
Icenowy
^ permalink raw reply
* [PATCH resend] ARM: dts: sun8i: Support DTB build for NanoPi M1
From: Maxime Ripard @ 2016-12-13 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161212231815.6114-1-woogyom.kim@gmail.com>
On Tue, Dec 13, 2016 at 08:18:15AM +0900, Milo Kim wrote:
> The commit 10efbf5f1633 ("ARM: dts: sun8i: Add dts file for NanoPi M1 SBC")
> introduced NanoPi M1 board but it's missing in Allwinner H3 DTB build.
>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
Applied, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161213/77e1b361/attachment.sig>
^ permalink raw reply
* [PATCH 4/4] s5p-cec: add hdmi-notifier support, move out of staging
From: Hans Verkuil @ 2016-12-13 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213150813.37966-1-hverkuil@xs4all.nl>
From: Hans Verkuil <hans.verkuil@cisco.com>
By using the HDMI notifier framework there is no longer any reason
to manually set the physical address. This was the one blocking
issue that prevented this driver from going out of staging, so do
this move as well.
Update the bindings documenting the new hdmi phandle and
update exynos4.dtsi accordingly.
Tested with my Odroid U3.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
.../devicetree/bindings/media/s5p-cec.txt | 2 ++
arch/arm/boot/dts/exynos4.dtsi | 1 +
drivers/media/platform/Kconfig | 18 +++++++++++
drivers/media/platform/Makefile | 1 +
.../media => media/platform}/s5p-cec/Makefile | 0
.../platform}/s5p-cec/exynos_hdmi_cec.h | 0
.../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0
.../media => media/platform}/s5p-cec/regs-cec.h | 0
.../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++++++++++++++----
.../media => media/platform}/s5p-cec/s5p_cec.h | 3 ++
drivers/staging/media/Kconfig | 2 --
drivers/staging/media/Makefile | 1 -
drivers/staging/media/s5p-cec/Kconfig | 9 ------
drivers/staging/media/s5p-cec/TODO | 7 -----
14 files changed, 55 insertions(+), 24 deletions(-)
rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
delete mode 100644 drivers/staging/media/s5p-cec/TODO
diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 925ab4d..710fc00 100644
--- a/Documentation/devicetree/bindings/media/s5p-cec.txt
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -15,6 +15,7 @@ Required properties:
- clock-names : from common clock binding: must contain "hdmicec",
corresponding to entry in the clocks property.
- samsung,syscon-phandle - phandle to the PMU system controller
+ - samsung,hdmi-phandle - phandle to the HDMI controller
Example:
@@ -25,6 +26,7 @@ hdmicec: cec at 100B0000 {
clocks = <&clock CLK_HDMI_CEC>;
clock-names = "hdmicec";
samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,hdmi-phandle = <&hdmi>;
pinctrl-names = "default";
pinctrl-0 = <&hdmi_cec>;
status = "okay";
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 5f034eb..48ab7fc 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -750,6 +750,7 @@
clocks = <&clock CLK_HDMI_CEC>;
clock-names = "hdmicec";
samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,hdmi-phandle = <&hdmi>;
pinctrl-names = "default";
pinctrl-0 = <&hdmi_cec>;
status = "disabled";
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index d944421..42f747a 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -392,6 +392,24 @@ config VIDEO_TI_SC
config VIDEO_TI_CSC
tristate
+menuconfig V4L_CEC_DRIVERS
+ bool "Platform HDMI CEC drivers"
+ depends on MEDIA_CEC_SUPPORT
+
+if V4L_CEC_DRIVERS
+
+config VIDEO_SAMSUNG_S5P_CEC
+ tristate "Samsung S5P CEC driver"
+ depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
+ select HDMI_NOTIFIERS
+ ---help---
+ This is a driver for Samsung S5P HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
+endif #V4L_CEC_DRIVERS
+
menuconfig V4L_TEST_DRIVERS
bool "Media test drivers"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 5b3cb27..ad3bf22 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D) += s5p-g2d/
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/
diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
similarity index 100%
rename from drivers/staging/media/s5p-cec/Makefile
rename to drivers/media/platform/s5p-cec/Makefile
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/regs-cec.h
rename to drivers/media/platform/s5p-cec/regs-cec.h
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
similarity index 89%
rename from drivers/staging/media/s5p-cec/s5p_cec.c
rename to drivers/media/platform/s5p-cec/s5p_cec.c
index 2a07968..565ee92 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.c
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -14,15 +14,18 @@
*/
#include <linux/clk.h>
+#include <linux/hdmi-notifier.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/timer.h>
#include <linux/workqueue.h>
+#include <media/cec-edid.h>
#include <media/cec.h>
#include "exynos_hdmi_cec.h"
@@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
static int s5p_cec_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct device_node *np;
+ struct platform_device *hdmi_dev;
struct resource *res;
struct s5p_cec_dev *cec;
int ret;
+ np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
+
+ if (!np) {
+ dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
+ return -ENODEV;
+ }
+ hdmi_dev = of_find_device_by_node(np);
+ if (hdmi_dev == NULL)
+ return -EPROBE_DEFER;
+
cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
if (!cec)
return -ENOMEM;
@@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
if (IS_ERR(cec->reg))
return PTR_ERR(cec->reg);
+ cec->notifier = hdmi_notifier_get(&hdmi_dev->dev);
+ if (cec->notifier == NULL)
+ return -ENOMEM;
+
cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
CEC_NAME,
- CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
+ CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
ret = PTR_ERR_OR_ZERO(cec->adap);
if (ret)
return ret;
+
ret = cec_register_adapter(cec->adap, &pdev->dev);
- if (ret) {
- cec_delete_adapter(cec->adap);
- return ret;
- }
+ if (ret)
+ goto err_delete_adapter;
+
+ cec_register_hdmi_notifier(cec->adap, cec->notifier);
platform_set_drvdata(pdev, cec);
pm_runtime_enable(dev);
dev_dbg(dev, "successfuly probed\n");
return 0;
+
+err_delete_adapter:
+ cec_delete_adapter(cec->adap);
+ return ret;
}
static int s5p_cec_remove(struct platform_device *pdev)
@@ -225,6 +249,7 @@ static int s5p_cec_remove(struct platform_device *pdev)
struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
cec_unregister_adapter(cec->adap);
+ hdmi_notifier_put(cec->notifier);
pm_runtime_disable(&pdev->dev);
return 0;
}
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.h b/drivers/media/platform/s5p-cec/s5p_cec.h
similarity index 97%
rename from drivers/staging/media/s5p-cec/s5p_cec.h
rename to drivers/media/platform/s5p-cec/s5p_cec.h
index 03732c1..69856b1 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.h
+++ b/drivers/media/platform/s5p-cec/s5p_cec.h
@@ -59,12 +59,15 @@ enum cec_state {
STATE_ERROR
};
+struct hdmi_notifier;
+
struct s5p_cec_dev {
struct cec_adapter *adap;
struct clk *clk;
struct device *dev;
struct mutex lock;
struct regmap *pmu;
+ struct hdmi_notifier *notifier;
int irq;
void __iomem *reg;
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index ffb8fa7..1b7804c 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -27,8 +27,6 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
source "drivers/staging/media/omap4iss/Kconfig"
-source "drivers/staging/media/s5p-cec/Kconfig"
-
# Keep LIRC at the end, as it has sub-menus
source "drivers/staging/media/lirc/Kconfig"
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index a28e82c..e11afbf 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -1,5 +1,4 @@
obj-$(CONFIG_I2C_BCM2048) += bcm2048/
-obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
obj-$(CONFIG_DVB_CXD2099) += cxd2099/
obj-$(CONFIG_LIRC_STAGING) += lirc/
obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/
diff --git a/drivers/staging/media/s5p-cec/Kconfig b/drivers/staging/media/s5p-cec/Kconfig
deleted file mode 100644
index ddfd955..0000000
--- a/drivers/staging/media/s5p-cec/Kconfig
+++ /dev/null
@@ -1,9 +0,0 @@
-config VIDEO_SAMSUNG_S5P_CEC
- tristate "Samsung S5P CEC driver"
- depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
- ---help---
- This is a driver for Samsung S5P HDMI CEC interface. It uses the
- generic CEC framework interface.
- CEC bus is present in the HDMI connector and enables communication
- between compatible devices.
-
diff --git a/drivers/staging/media/s5p-cec/TODO b/drivers/staging/media/s5p-cec/TODO
deleted file mode 100644
index 64f21ba..0000000
--- a/drivers/staging/media/s5p-cec/TODO
+++ /dev/null
@@ -1,7 +0,0 @@
-This driver requires that userspace sets the physical address.
-However, this should be passed on from the corresponding
-Samsung HDMI driver.
-
-We have to wait until the HDMI notifier framework has been merged
-in order to handle this gracefully, until that time this driver
-has to remain in staging.
--
2.10.2
^ permalink raw reply related
* [linux-sunxi] sunxi-ng clocks: leaving certain clocks alone?
From: Maxime Ripard @ 2016-12-13 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4e6ddb18-0686-1639-544c-c02371331d3c@arm.com>
Hi,
On Mon, Dec 12, 2016 at 12:16:07PM +0000, Andre Przywara wrote:
> Hi Chen-Yu,
>
> thanks for the answer!
>
> On 12/12/16 04:41, Chen-Yu Tsai wrote:
> > On Mon, Dec 12, 2016 at 7:54 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
> >> Hi,
> >>
> >> I was observing that the new sunxi-ng clock code apparently explicitly
> >> turns off _all_ clocks that are not used or needed. I find this rather
> >> unfortunate, as I wanted to use the THS temperature sensor from ARM
> >> Trusted Firmware to implement emergency shutdown or DVFS throttling.
> >> That works until the clock framework finds the clock (as enumerated in
> >> ccu-sun50i-a64.c) and obviously explicitly clears bit 31 in the THS mod
> >> clock register and bit 8 in the respective clock gate register.
> >> Turning them manually back on via /dev/mem or removing the THS clock
> >> from the sunxi-ng driver fixes this for me.
> >>
> >> This was not happening with the old Allwinner clocks, since the kernel
> >> wouldn't even know about it if there was no driver and the clock wasn't
> >> mentioned in the DT.
> >>
> >> Now with sunxi-ng even though the THS clock is not actually referenced
> >> or used in the DT, the kernel turns it off. I would expect that upon
> >> entering the kernel all unneeded clocks are turned off anyway, so there
> >> is no need to mess with clocks that have no user, but are enumerated in
> >> the ccu driver.
> >
> > I can't say that's absolutely true (wink).
> >
> >>
> >> I wonder if this kills simplefb as well, for instance, since I believe
> >> that U-Boot needs to turn on certain clocks and relies on them staying up.
> >
> > The simplefb bindings takes clocks and regulators expressly for the
> > purpose of keeping them enabled.
>
> Right, I should have checked this before ...
>
> >>
> >> So my questions:
> >> 1) Is this expected behaviour?
> >
> > Yes.
> >
> >> 2) If yes, should it really be?
> >> 3) If yes, shouldn't there be way to explicitly tell Linux to leave that
> >> clock alone, preferably via DT? Although the sunxi-ng clocks take
> >> control over the whole CCU unit, I wonder if it should really mess with
> >> clocks there are not referenced in the DT.
> >
> > As it owns the whole CCU unit, why not? And how would it know if some
> > clock is referenced or not, unless going through the whole device tree?
>
> I was hoping that it just provides clocks to any users (drivers) and
> wouldn't bother with tinkering with every clock unless explicitly being
> asked for (by a driver being pointed to a specific clock through DT).
> So it would need to iterate through anything - neither the whole DT nor
> it's own list of clocks.
>
> > Furthermore, nothing prevents another device driver from referencing
> > said clock and turning it off when it's not in use. Think THS driver
> > with runtime PM.
>
> I am totally OK with that: Any potential Linux THS driver can take over,
> if the DT references this device and the clock.
> My point is that atm there is no such driver and so the clocks framework
> shouldn't turn that clock off.
You could turn that exact argument the other way though. If there's no
user in the system, why should we waste power and leave it enabled?
> > Are you also mapping the THS to secure only? Otherwise nothing would
> > prevent Linux from also claiming it.
>
> Unfortunately the THS is always unsecure. And even if it could be
> switched, after a recent IRC discussion I came to believe that those
> secure peripherals features only works when the secure boot feature is
> used, which requires to blow an efuse and thus is not easily doable on
> most boards and also irreversible.
> Also I am not sure whether this security feature actually extends to the
> mod clocks and the bus reset and clock gates bits.
>
> But I was relying on that Linux doesn't touch hardware that's not
> referenced in the DT, so if firmware uses the THS, any Linux THS node
> would need to go - or the other way around: if there is a Linux THS
> node, firmware backs off.
It's not just about node though, but also based on the kernel
configuration. If the kernel didn't have a THS driver compiled (or not
loaded), then if you want to implement such a behaviour, you should
also keep the THS driver in the firmware.
> >> Maybe there is some way to reference those clocks via some dummy driver
> >> or DT node to avoid this behaviour? Is there any prior art in this respect?
> >
> > If you want a clock to not be disabled by anyone, adding CLK_IS_CRITICAL
> > to its flags is the proper option. This is done in the clk driver though.
>
> Yes, I was thinking about that, but it indeed sounds like a hack to
> follow this.
You also have the option to add a clock-critical property.
Keep in mind that just preventing it from shutting down at boot gives
no warranty that the clock will remain enabled. Other clocks in the
same sub-tree might do a reparenting or a disable that would lead to
that clock being modified or disabled too as a side effect.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161213/b593fc31/attachment.sig>
^ permalink raw reply
* [PATCH 3/4] cec: integrate HDMI notifier support
From: Hans Verkuil @ 2016-12-13 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213150813.37966-1-hverkuil@xs4all.nl>
From: Hans Verkuil <hans.verkuil@cisco.com>
Support the HDMI notifier framework, simplifying drivers that
depend on this.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/cec/cec-core.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
include/media/cec.h | 15 +++++++++++++
2 files changed, 65 insertions(+)
diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index aca3ab8..c620a4c 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -195,6 +195,52 @@ static void cec_devnode_unregister(struct cec_devnode *devnode)
put_device(&devnode->dev);
}
+#ifdef CONFIG_HDMI_NOTIFIERS
+static u16 parse_hdmi_addr(const struct edid *edid)
+{
+ if (!edid || edid->extensions == 0)
+ return CEC_PHYS_ADDR_INVALID;
+
+ return cec_get_edid_phys_addr((u8 *)edid,
+ EDID_LENGTH * (edid->extensions + 1), NULL);
+}
+
+static int cec_hdmi_notify(struct notifier_block *nb, unsigned long event,
+ void *data)
+{
+ struct cec_adapter *adap = container_of(nb, struct cec_adapter, nb);
+ struct hdmi_notifier *n = data;
+ unsigned int phys;
+
+ dprintk(1, "event %lu\n", event);
+
+ switch (event) {
+ case HDMI_DISCONNECTED:
+ cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
+ break;
+
+ case HDMI_NEW_EDID:
+ phys = parse_hdmi_addr(n->edid);
+ cec_s_phys_addr(adap, phys, false);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+void cec_register_hdmi_notifier(struct cec_adapter *adap,
+ struct hdmi_notifier *notifier)
+{
+ if (WARN_ON(!adap->devnode.registered))
+ return;
+
+ adap->nb.notifier_call = cec_hdmi_notify;
+ adap->notifier = notifier;
+ hdmi_notifier_register(adap->notifier, &adap->nb);
+}
+EXPORT_SYMBOL_GPL(cec_register_hdmi_notifier);
+#endif
+
struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
void *priv, const char *name, u32 caps,
u8 available_las)
@@ -344,6 +390,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
adap->rc = NULL;
#endif
debugfs_remove_recursive(adap->cec_dir);
+#ifdef CONFIG_HDMI_NOTIFIERS
+ if (adap->notifier)
+ hdmi_notifier_unregister(adap->notifier, &adap->nb);
+#endif
cec_devnode_unregister(&adap->devnode);
}
EXPORT_SYMBOL_GPL(cec_unregister_adapter);
diff --git a/include/media/cec.h b/include/media/cec.h
index 96a0aa7..3b4860d 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -28,6 +28,11 @@
#include <linux/kthread.h>
#include <linux/timer.h>
#include <linux/cec-funcs.h>
+#ifdef CONFIG_HDMI_NOTIFIERS
+#include <linux/notifier.h>
+#include <linux/hdmi-notifier.h>
+#include <drm/drm_edid.h>
+#endif
#include <media/rc-core.h>
#include <media/cec-edid.h>
@@ -173,6 +178,11 @@ struct cec_adapter {
bool passthrough;
struct cec_log_addrs log_addrs;
+#ifdef CONFIG_HDMI_NOTIFIERS
+ struct hdmi_notifier *notifier;
+ struct notifier_block nb;
+#endif
+
struct dentry *cec_dir;
struct dentry *status_file;
@@ -213,6 +223,11 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
+#ifdef CONFIG_HDMI_NOTIFIERS
+void cec_register_hdmi_notifier(struct cec_adapter *adap,
+ struct hdmi_notifier *notifier);
+#endif
+
#else
static inline int cec_register_adapter(struct cec_adapter *adap,
--
2.10.2
^ permalink raw reply related
* [PATCH 2/4] exynos_hdmi: add HDMI notifier support
From: Hans Verkuil @ 2016-12-13 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213150813.37966-1-hverkuil@xs4all.nl>
From: Hans Verkuil <hans.verkuil@cisco.com>
Implement the HDMI notifier support to allow CEC drivers to
be informed when there is a new EDID and when a connect or
disconnect happens.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/gpu/drm/exynos/Kconfig | 1 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 465d344f..34f3b6f 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
config DRM_EXYNOS_HDMI
bool "HDMI"
depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
+ select HDMI_NOTIFIERS
help
Choose this option if you want to use Exynos HDMI for DRM.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index e8fb6ef..2bfc411 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -31,6 +31,7 @@
#include <linux/pm_runtime.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
+#include <linux/hdmi-notifier.h>
#include <linux/regulator/consumer.h>
#include <linux/io.h>
#include <linux/of_address.h>
@@ -132,6 +133,7 @@ struct hdmi_context {
struct delayed_work hotplug_work;
struct drm_display_mode current_mode;
u8 cea_video_id;
+ struct hdmi_notifier *notifier;
const struct hdmi_driver_data *drv_data;
void __iomem *regs;
@@ -857,9 +859,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
{
struct hdmi_context *hdata = connector_to_hdmi(connector);
- if (gpiod_get_value(hdata->hpd_gpio))
+ if (gpiod_get_value(hdata->hpd_gpio)) {
+ hdmi_event_connect(hdata->notifier);
return connector_status_connected;
+ }
+ hdmi_event_disconnect(hdata->notifier);
return connector_status_disconnected;
}
@@ -898,6 +903,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
edid->width_cm, edid->height_cm);
drm_mode_connector_update_edid_property(connector, edid);
+ hdmi_event_connect(hdata->notifier);
+ hdmi_event_new_edid(hdata->notifier, edid,
+ EDID_LENGTH * (1 + edid->extensions));
ret = drm_add_edid_modes(connector, edid);
@@ -1544,6 +1552,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
if (funcs && funcs->disable)
(*funcs->disable)(crtc);
+ hdmi_event_disconnect(hdata->notifier);
cancel_delayed_work(&hdata->hotplug_work);
hdmiphy_disable(hdata);
@@ -1893,15 +1902,22 @@ static int hdmi_probe(struct platform_device *pdev)
}
}
+ hdata->notifier = hdmi_notifier_get(&pdev->dev);
+ if (hdata->notifier == NULL) {
+ ret = -ENOMEM;
+ goto err_hdmiphy;
+ }
+
pm_runtime_enable(dev);
ret = component_add(&pdev->dev, &hdmi_component_ops);
if (ret)
- goto err_disable_pm_runtime;
+ goto err_notifier_put;
return ret;
-err_disable_pm_runtime:
+err_notifier_put:
+ hdmi_notifier_put(hdata->notifier);
pm_runtime_disable(dev);
err_hdmiphy:
@@ -1918,9 +1934,11 @@ static int hdmi_remove(struct platform_device *pdev)
struct hdmi_context *hdata = platform_get_drvdata(pdev);
cancel_delayed_work_sync(&hdata->hotplug_work);
+ hdmi_event_disconnect(hdata->notifier);
component_del(&pdev->dev, &hdmi_component_ops);
+ hdmi_notifier_put(hdata->notifier);
pm_runtime_disable(&pdev->dev);
if (!IS_ERR(hdata->reg_hdmi_en))
--
2.10.2
^ permalink raw reply related
* [PATCH 1/4] video: add HDMI state notifier support
From: Hans Verkuil @ 2016-12-13 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161213150813.37966-1-hverkuil@xs4all.nl>
From: Hans Verkuil <hans.verkuil@cisco.com>
Add support for HDMI hotplug and EDID notifiers, which is used to convey
information from HDMI drivers to their CEC and audio counterparts.
Based on an earlier version from Russell King:
https://patchwork.kernel.org/patch/9277043/
The hdmi_notifier is a reference counted object containing the HDMI state
of an HDMI device.
When a new notifier is registered the current state will be reported to
that notifier at registration time.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/video/Kconfig | 3 +
drivers/video/Makefile | 1 +
drivers/video/hdmi-notifier.c | 134 ++++++++++++++++++++++++++++++++++++++++++
include/linux/hdmi-notifier.h | 109 ++++++++++++++++++++++++++++++++++
4 files changed, 247 insertions(+)
create mode 100644 drivers/video/hdmi-notifier.c
create mode 100644 include/linux/hdmi-notifier.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3c20af9..1ee7b9f 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
config HDMI
bool
+config HDMI_NOTIFIERS
+ bool
+
if VT
source "drivers/video/console/Kconfig"
endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9ad3c17..65f5649 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -1,5 +1,6 @@
obj-$(CONFIG_VGASTATE) += vgastate.o
obj-$(CONFIG_HDMI) += hdmi.o
+obj-$(CONFIG_HDMI_NOTIFIERS) += hdmi-notifier.o
obj-$(CONFIG_VT) += console/
obj-$(CONFIG_LOGO) += logo/
diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
new file mode 100644
index 0000000..29e4225
--- /dev/null
+++ b/drivers/video/hdmi-notifier.c
@@ -0,0 +1,134 @@
+#include <linux/export.h>
+#include <linux/hdmi-notifier.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+
+static LIST_HEAD(hdmi_notifiers);
+static DEFINE_MUTEX(hdmi_notifiers_lock);
+
+struct hdmi_notifier *hdmi_notifier_get(struct device *dev)
+{
+ struct hdmi_notifier *n;
+
+ mutex_lock(&hdmi_notifiers_lock);
+ list_for_each_entry(n, &hdmi_notifiers, head) {
+ if (n->dev == dev) {
+ mutex_unlock(&hdmi_notifiers_lock);
+ kref_get(&n->kref);
+ return n;
+ }
+ }
+ n = kzalloc(sizeof(*n), GFP_KERNEL);
+ if (!n)
+ goto unlock;
+ n->dev = dev;
+ mutex_init(&n->lock);
+ BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
+ kref_init(&n->kref);
+ list_add_tail(&n->head, &hdmi_notifiers);
+unlock:
+ mutex_unlock(&hdmi_notifiers_lock);
+ return n;
+}
+EXPORT_SYMBOL_GPL(hdmi_notifier_get);
+
+static void hdmi_notifier_release(struct kref *kref)
+{
+ struct hdmi_notifier *n =
+ container_of(kref, struct hdmi_notifier, kref);
+
+ mutex_lock(&hdmi_notifiers_lock);
+ list_del(&n->head);
+ mutex_unlock(&hdmi_notifiers_lock);
+ kfree(n->edid);
+ kfree(n);
+}
+
+void hdmi_notifier_put(struct hdmi_notifier *n)
+{
+ kref_put(&n->kref, hdmi_notifier_release);
+}
+EXPORT_SYMBOL_GPL(hdmi_notifier_put);
+
+int hdmi_notifier_register(struct hdmi_notifier *n, struct notifier_block *nb)
+{
+ int ret = blocking_notifier_chain_register(&n->notifiers, nb);
+
+ if (ret)
+ return ret;
+ kref_get(&n->kref);
+ mutex_lock(&n->lock);
+ if (n->connected) {
+ blocking_notifier_call_chain(&n->notifiers, HDMI_CONNECTED, n);
+ if (n->edid_size)
+ blocking_notifier_call_chain(&n->notifiers, HDMI_NEW_EDID, n);
+ if (n->has_eld)
+ blocking_notifier_call_chain(&n->notifiers, HDMI_NEW_ELD, n);
+ }
+ mutex_unlock(&n->lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(hdmi_notifier_register);
+
+int hdmi_notifier_unregister(struct hdmi_notifier *n, struct notifier_block *nb)
+{
+ int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
+
+ if (ret == 0)
+ hdmi_notifier_put(n);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(hdmi_notifier_unregister);
+
+void hdmi_event_connect(struct hdmi_notifier *n)
+{
+ mutex_lock(&n->lock);
+ n->connected = true;
+ blocking_notifier_call_chain(&n->notifiers, HDMI_CONNECTED, n);
+ mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_connect);
+
+void hdmi_event_disconnect(struct hdmi_notifier *n)
+{
+ mutex_lock(&n->lock);
+ n->connected = false;
+ n->has_eld = false;
+ n->edid_size = 0;
+ blocking_notifier_call_chain(&n->notifiers, HDMI_DISCONNECTED, n);
+ mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
+
+int hdmi_event_new_edid(struct hdmi_notifier *n, const void *edid, size_t size)
+{
+ mutex_lock(&n->lock);
+ if (n->edid_allocated_size < size) {
+ void *p = kmalloc(size, GFP_KERNEL);
+
+ if (p == NULL) {
+ mutex_unlock(&n->lock);
+ return -ENOMEM;
+ }
+ kfree(n->edid);
+ n->edid = p;
+ n->edid_allocated_size = size;
+ }
+ memcpy(n->edid, edid, size);
+ n->edid_size = size;
+ blocking_notifier_call_chain(&n->notifiers, HDMI_NEW_EDID, n);
+ mutex_unlock(&n->lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(hdmi_event_new_edid);
+
+void hdmi_event_new_eld(struct hdmi_notifier *n, const u8 eld[128])
+{
+ mutex_lock(&n->lock);
+ memcpy(n->eld, eld, sizeof(n->eld));
+ n->has_eld = true;
+ blocking_notifier_call_chain(&n->notifiers, HDMI_NEW_ELD, n);
+ mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_new_eld);
diff --git a/include/linux/hdmi-notifier.h b/include/linux/hdmi-notifier.h
new file mode 100644
index 0000000..1d88db0
--- /dev/null
+++ b/include/linux/hdmi-notifier.h
@@ -0,0 +1,109 @@
+/*
+ * hdmi-notifier.h - notify interested parties of (dis)connect and EDID
+ * events
+ *
+ * Copyright 2016 Russell King <rmk+kernel@arm.linux.org.uk>
+ * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+#ifndef LINUX_HDMI_NOTIFIER_H
+#define LINUX_HDMI_NOTIFIER_H
+
+#include <linux/types.h>
+#include <linux/notifier.h>
+#include <linux/kref.h>
+
+enum {
+ HDMI_CONNECTED,
+ HDMI_DISCONNECTED,
+ HDMI_NEW_EDID,
+ HDMI_NEW_ELD,
+};
+
+struct device;
+
+struct hdmi_notifier {
+ struct mutex lock;
+ struct list_head head;
+ struct kref kref;
+ struct blocking_notifier_head notifiers;
+ struct device *dev;
+
+ /* Current state */
+ bool connected;
+ bool has_eld;
+ unsigned char eld[128];
+ void *edid;
+ size_t edid_size;
+ size_t edid_allocated_size;
+};
+
+/**
+ * hdmi_notifier_get - find or create a new hdmi_notifier for the given device.
+ * @dev: device that sends the events.
+ *
+ * If a notifier for device @dev already exists, then increase the refcount
+ * and return that notifier.
+ *
+ * If it doesn't exist, then allocate a new notifier struct and return a
+ * pointer to that new struct.
+ *
+ * Return NULL if the memory could not be allocated.
+ */
+struct hdmi_notifier *hdmi_notifier_get(struct device *dev);
+
+/**
+ * hdmi_notifier_put - decrease refcount and delete when the refcount reaches 0.
+ * @n: notifier
+ */
+void hdmi_notifier_put(struct hdmi_notifier *n);
+
+/**
+ * hdmi_notifier_register - register the notifier with the notifier_block.
+ * @n: the HDMI notifier
+ * @nb: the notifier_block
+ */
+int hdmi_notifier_register(struct hdmi_notifier *n, struct notifier_block *nb);
+
+/**
+ * hdmi_notifier_unregister - unregister the notifier with the notifier_block.
+ * @n: the HDMI notifier
+ * @nb: the notifier_block
+ */
+int hdmi_notifier_unregister(struct hdmi_notifier *n, struct notifier_block *nb);
+
+/**
+ * hdmi_event_connect - send a connect event.
+ * @n: the HDMI notifier
+ *
+ * Send an HDMI_CONNECTED event to any registered parties.
+ */
+void hdmi_event_connect(struct hdmi_notifier *n);
+
+/**
+ * hdmi_event_disconnect - send a disconnect event.
+ * @n: the HDMI notifier
+ *
+ * Send an HDMI_DISCONNECTED event to any registered parties.
+ */
+void hdmi_event_disconnect(struct hdmi_notifier *n);
+
+/**
+ * hdmi_event_new_edid - send a new EDID event.
+ * @n: the HDMI notifier
+ *
+ * Send an HDMI_NEW_EDID event to any registered parties.
+ * This function will make a copy the EDID so it can return -ENOMEM if
+ * no memory could be allocated.
+ */
+int hdmi_event_new_edid(struct hdmi_notifier *n, const void *edid, size_t size);
+
+/**
+ * hdmi_event_new_eld - send a new ELD event.
+ * @n: the HDMI notifier
+ *
+ * Send an HDMI_NEW_ELD event to any registered parties.
+ */
+void hdmi_event_new_eld(struct hdmi_notifier *n, const u8 eld[128]);
+
+#endif
--
2.10.2
^ permalink raw reply related
* [PATCH 0/4] video/exynos/cec: add HDMI state notifier & use in s5p-cec
From: Hans Verkuil @ 2016-12-13 15:08 UTC (permalink / raw)
To: linux-arm-kernel
From: Hans Verkuil <hans.verkuil@cisco.com>
This patch series adds the HDMI notifier code, based on Russell's code:
https://patchwork.kernel.org/patch/9277043/
It adds support for it to the exynos_hdmi drm driver, adds support for
it to the CEC framework and finally adds support to the s5p-cec driver,
which now can be moved out of staging.
Tested with my Odroid U3 exynos4 devboard.
Comments are welcome. I'd like to get this in for the 4.11 kernel as
this is a missing piece needed to integrate CEC drivers.
Benjamin, can you look at doing the same notifier integration for your
st-cec driver as is done for s5p-cec? It would be good to be able to
move st-cec out of staging at the same time.
Regards,
Hans
Hans Verkuil (4):
video: add HDMI state notifier support
exynos_hdmi: add HDMI notifier support
cec: integrate HDMI notifier support
s5p-cec: add hdmi-notifier support, move out of staging
.../devicetree/bindings/media/s5p-cec.txt | 2 +
arch/arm/boot/dts/exynos4.dtsi | 1 +
drivers/gpu/drm/exynos/Kconfig | 1 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++-
drivers/media/cec/cec-core.c | 50 ++++++++
drivers/media/platform/Kconfig | 18 +++
drivers/media/platform/Makefile | 1 +
.../media => media/platform}/s5p-cec/Makefile | 0
.../platform}/s5p-cec/exynos_hdmi_cec.h | 0
.../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0
.../media => media/platform}/s5p-cec/regs-cec.h | 0
.../media => media/platform}/s5p-cec/s5p_cec.c | 35 +++++-
.../media => media/platform}/s5p-cec/s5p_cec.h | 3 +
drivers/staging/media/Kconfig | 2 -
drivers/staging/media/Makefile | 1 -
drivers/staging/media/s5p-cec/Kconfig | 9 --
drivers/staging/media/s5p-cec/TODO | 7 --
drivers/video/Kconfig | 3 +
drivers/video/Makefile | 1 +
drivers/video/hdmi-notifier.c | 134 +++++++++++++++++++++
include/linux/hdmi-notifier.h | 109 +++++++++++++++++
include/media/cec.h | 15 +++
22 files changed, 389 insertions(+), 27 deletions(-)
rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
delete mode 100644 drivers/staging/media/s5p-cec/TODO
create mode 100644 drivers/video/hdmi-notifier.c
create mode 100644 include/linux/hdmi-notifier.h
--
2.10.2
^ permalink raw reply
* [PATCH] drm: tilcdc: simplify the recovery from sync lost error on rev1
From: Bartosz Golaszewski @ 2016-12-13 15:07 UTC (permalink / raw)
To: linux-arm-kernel
Revision 2 of LCDC suffers from an issue where a SYNC_LOST error
caused by limited memory bandwidth may leave the picture shifted a
couple pixels to the right.
This issue has not been observed on revision 1, while the recovery
mechanism introduces a different issue, where the END_OF_FRAME
interrupt doesn't fire while drm is waiting for vblanks.
On rev1: recover from sync lost errors by simply clearing the
RASTER_ENABLE bit in the RASTER_CTRL register and re-enabling it
again as is suggested by the datasheet.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 9942b05..70e57a7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -921,17 +921,23 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost",
__func__, stat);
tilcdc_crtc->frame_intact = false;
- if (tilcdc_crtc->sync_lost_count++ >
- SYNC_LOST_COUNT_LIMIT) {
- dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, recovering", __func__, stat);
- queue_work(system_wq, &tilcdc_crtc->recover_work);
- if (priv->rev == 1)
- tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
- LCDC_V1_SYNC_LOST_INT_ENA);
- else
+ if (priv->rev == 1) {
+ tilcdc_clear(dev,
+ LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+ tilcdc_set(dev,
+ LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+ } else {
+ if (tilcdc_crtc->sync_lost_count++ >
+ SYNC_LOST_COUNT_LIMIT) {
+ dev_err(dev->dev,
+ "%s(0x%08x): Sync lost flood detected, recovering",
+ __func__, stat);
+ queue_work(system_wq,
+ &tilcdc_crtc->recover_work);
tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
LCDC_SYNC_LOST);
- tilcdc_crtc->sync_lost_count = 0;
+ tilcdc_crtc->sync_lost_count = 0;
+ }
}
}
--
2.9.3
^ permalink raw reply related
* [RFC v2 PATCH 0/3] Fix dma_alloc_coherent() and friends for NOMMU
From: Vladimir Murzin @ 2016-12-13 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1BE9400D-BACB-4C07-B6F0-1963D90A7029@esh.hu>
Hi,
On 13/12/16 14:33, Szemz? Andr?s wrote:
> Hi,
>
>> On 2016. Dec 13., at 14:45, Vladimir Murzin <vladimir.murzin@arm.com> wrote:
>>
>> Hi,
>>
>> It seem that addition of cache support for M-class cpus uncovered
>> latent bug in DMA usage. NOMMU memory model has been treated as being
>> always consistent; however, for R/M classes of cpu memory can be
>> covered by MPU which in turn might configure RAM as Normal
>> i.e. bufferable and cacheable. It breaks dma_alloc_coherent() and
>> friends, since data can stuck in caches now or be buffered.
>>
>> This patch set is trying to address the issue by providing region of
>> memory suitable for consistent DMA operations. It is supposed that such
>> region is marked by MPU as non-cacheable. Since we have MPU support in
>> Linux for R-class only and M-class setting MPU in bootloader, proposed
>> interface to advertise such memory is via "memdma=size at start" command
>> line option, to avoid clashing with normal memory (which usually comes
>> from dts) it'd be safer to use it together with "mem=" command line
>> option. Meanwhile, I'm open to suggestions for the better way telling
>> Linux of such memory.
>>
>
> I have tested these patches on my ATMEL SAME70 armv7m board.
>
> After setting the memory regions and attributes in the bootloader and providing the required
> command line parameters, the DMA issues fixed.
>
> I have tested an usart, sdcard, and ethernet driver with DMA enabled.
>
> Booting Linux on physical CPU 0x0
> Linux version 4.9.0 (root at debian) (gcc version 4.8.3 20140320 (prerelease) (Sourcery CodeBench Lite 2014.05-29) ) #4 Mon Dec 12 20:27:21 CET 201
> 6
> CPU: ARMv7-M [410fc271] revision 1 (ARMv7M), cr=00000000
> CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
> OF: fdt:Machine model: SAME70-sampione board
> Built 1 zonelists in Zone order, mobility grouping on. Total pages: 15748
> Kernel command line: console=ttyS1,115200 root=/dev/mmcblk0p2 rw init=/linuxrc rootwait mem=62M memdma=2M at 0x73e00000
>
> So you can add my Tested-by.
>
> Thanks for the patches!
Glad to hear it works for you! Thanks for reporting and testing!
Cheers
Vladimir
>
> Regards,
> Andras
>
>
>
^ permalink raw reply
* [PATCH V8 1/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Agustin Vega-Frias @ 2016-12-13 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161208110552.GA17398@red-moon>
Hi Lorenzo,
On 2016-12-08 06:05, Lorenzo Pieralisi wrote:
> Hi Agustin,
>
> please CC me for next version.
>
> On Tue, Nov 29, 2016 at 05:57:37PM -0500, Agustin Vega-Frias wrote:
>> When an Extended IRQ Resource contains a valid ResourceSource
>> use it to map the IRQ on the domain associated with the ACPI
>> device referenced.
>>
>> With this in place an irqchip driver can create its domain using
>> irq_domain_create_linear and pass the device fwnode to create
>> the domain mapping. When dependent devices are probed these
>> changes allow the ACPI core find the domain and map the IRQ.
>>
>> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
>> ---
>> drivers/acpi/Makefile | 2 +-
>> drivers/acpi/{gsi.c => irq.c} | 99
>> +++++++++++++++++++++++++++++++++++++------
>
> [...]
>
>> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>> +static void acpi_dev_get_irqresource(struct resource *res, u32 hwirq,
>> + struct fwnode_handle *source,
>> u8 triggering, u8 polarity, u8 shareable,
>> bool legacy)
>> {
>> int irq, p, t;
>>
>> - if (!valid_IRQ(gsi)) {
>> - acpi_dev_irqresource_disabled(res, gsi);
>> + if (!source && !valid_IRQ(hwirq)) {
>
> If you make source a struct acpi_resource_source pointer it could be:
>
> if (source || !valid_IRQ(hwirq))
>
> Actually we would not even need to pass the pointer, if we detect
> an acpi_resource_source dependency we can just disable the resource
> (without even looking-up the fwnode_handle, see below), it is a design
> choice we have to make.
>
>> + acpi_dev_irqresource_disabled(res, hwirq);
>> return;
>> }
>>
>> @@ -402,25 +403,25 @@ static void acpi_dev_get_irqresource(struct
>> resource *res, u32 gsi,
>> * using extended IRQ descriptors we take the IRQ configuration
>> * from _CRS directly.
>> */
>> - if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
>> + if (legacy && !acpi_get_override_irq(hwirq, &t, &p)) {
>> u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>> u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>>
>> if (triggering != trig || polarity != pol) {
>> - pr_warning("ACPI: IRQ %d override to %s, %s\n", gsi,
>> - t ? "level" : "edge", p ? "low" : "high");
>> + pr_warn("ACPI: IRQ %d override to %s, %s\n", hwirq,
>> + t ? "level" : "edge", p ? "low" : "high");
>> triggering = trig;
>> polarity = pol;
>> }
>> }
>>
>> res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
>> - irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>> + irq = acpi_register_irq(source, hwirq, triggering, polarity);
>> if (irq >= 0) {
>> res->start = irq;
>> res->end = irq;
>> } else {
>> - acpi_dev_irqresource_disabled(res, gsi);
>> + acpi_dev_irqresource_disabled(res, hwirq);
>> }
>> }
>>
>> @@ -448,6 +449,7 @@ bool acpi_dev_resource_interrupt(struct
>> acpi_resource *ares, int index,
>> {
>> struct acpi_resource_irq *irq;
>> struct acpi_resource_extended_irq *ext_irq;
>> + struct fwnode_handle *src;
>>
>> switch (ares->type) {
>> case ACPI_RESOURCE_TYPE_IRQ:
>> @@ -460,7 +462,7 @@ bool acpi_dev_resource_interrupt(struct
>> acpi_resource *ares, int index,
>> acpi_dev_irqresource_disabled(res, 0);
>> return false;
>> }
>> - acpi_dev_get_irqresource(res, irq->interrupts[index],
>> + acpi_dev_get_irqresource(res, irq->interrupts[index], NULL,
>> irq->triggering, irq->polarity,
>> irq->sharable, true);
>> break;
>> @@ -470,7 +472,8 @@ bool acpi_dev_resource_interrupt(struct
>> acpi_resource *ares, int index,
>> acpi_dev_irqresource_disabled(res, 0);
>> return false;
>> }
>> - acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
>> + src = acpi_get_irq_source_fwhandle(&ext_irq->resource_source);
>
> I think the only pending question on my side for this series is whether
> we still carry out the domain look-up here (as you are doing now), or,
> if we detect a resource_source dependency, we just disable the resource
> and leave the deferred probing mechanism to deal with it, this will
> completely decouple the current resource parsing from the deferred
> probe
> mechanism that you are introducing; basically this is equivalent to
> saying "if the IRQ resource has a dependency let's resolve it at
> acpi_irq_get() time, not now".
>
I'm also torn about this so I am going to leave most of this code as it
was originally and just ensure we don't attempt the mapping when we have
a resource source.
I other words bus scan only maps GSIs and other IRQs are mapped via
acpi_irq_get().
Thanks,
Agustin
> I am fine either way, I just think that leaving the domain look-up
> in the middle of the IRQ resource parsing is not really clean-cut.
>
> Thanks,
> Lorenzo
>
>> + acpi_dev_get_irqresource(res, ext_irq->interrupts[index], src,
>> ext_irq->triggering, ext_irq->polarity,
>> ext_irq->sharable, false);
>> break;
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 61a3d90..154e576 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -321,6 +321,25 @@ void acpi_set_irq_model(enum acpi_irq_model_id
>> model,
>> */
>> void acpi_unregister_gsi (u32 gsi);
>>
>> +#ifdef CONFIG_ACPI_GENERIC_GSI
>> +struct fwnode_handle *
>> +acpi_get_irq_source_fwhandle(const struct acpi_resource_source
>> *source);
>> +int acpi_register_irq(struct fwnode_handle *source, u32 hwirq, int
>> trigger,
>> + int polarity);
>> +void acpi_unregister_irq(struct fwnode_handle *source, u32 hwirq);
>> +#else
>> +#define acpi_get_irq_source_fwhandle(source) (NULL)
>> +static inline int acpi_register_irq(struct fwnode_handle *source, u32
>> hwirq,
>> + int trigger, int polarity)
>> +{
>> + return acpi_register_gsi(NULL, hwirq, trigger, polarity);
>> +}
>> +static inline void acpi_unregister_irq(struct fwnode_handle *source,
>> u32 hwirq)
>> +{
>> + acpi_unregister_gsi(hwirq);
>> +}
>> +#endif
>> +
>> struct pci_dev;
>>
>> int acpi_pci_irq_enable (struct pci_dev *dev);
>> --
>> Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
>> Technologies, Inc.
>> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
>> Linux Foundation Collaborative Project.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
>> in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a
Linux Foundation Collaborative Project.
^ permalink raw reply
* [RFC v2 PATCH 0/3] Fix dma_alloc_coherent() and friends for NOMMU
From: Vladimir Murzin @ 2016-12-13 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ed07f3ca-8ce7-fb1b-2292-238ecf6460da@arm.com>
On 13/12/16 14:25, Robin Murphy wrote:
> On 13/12/16 14:14, Vladimir Murzin wrote:
>> On 13/12/16 14:07, Russell King - ARM Linux wrote:
>>> On Tue, Dec 13, 2016 at 01:45:01PM +0000, Vladimir Murzin wrote:
>>>> This patch set is trying to address the issue by providing region of
>>>> memory suitable for consistent DMA operations. It is supposed that such
>>>> region is marked by MPU as non-cacheable. Since we have MPU support in
>>>> Linux for R-class only and M-class setting MPU in bootloader, proposed
>>>> interface to advertise such memory is via "memdma=size at start" command
>>>> line option, to avoid clashing with normal memory (which usually comes
>>>> from dts) it'd be safer to use it together with "mem=" command line
>>>> option. Meanwhile, I'm open to suggestions for the better way telling
>>>> Linux of such memory.
>>>
>>> For those nommu systems where the MPU is not used, how do they allocate
>>> DMA memory without setting aside a chunk of memory?
>>>
>>> >From what I understand of the current nommu code, it would just use
>>> the normal page allocator for DMA memory allocation, so now requiring
>>> everything to fit the "nommu has mpu" case seems like it's going to
>>> break older nommu.
>>>
>>
>> Probably, it'd be better if we just fallback to dma-noop operations if there
>> is no dma region, i.e. assume that platform is coherent. We still need a way
>> to tell user that absence of such region can be reason of broken DMA.
>
> As I mentioned internally, I think it would be worth trying to use CMA
> for this, because dma_map_ops are already wired to try that first, and
> from what I can see it seems already set up to do precisely this via a
> "shared-dma-pool" reserved memory region (see rmem_cma_setup() in
> drivers/base/dma-contiguous.c) - mandating that for cached v7-M systems
> whilst letting cache-less/non-MPU systems automatically fall back to the
> normal page allocator in its absence would seem to solve all 3 cases.
Unfortunately,
config DMA_CMA
bool "DMA Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && CMA
help
...
config CMA
bool "Contiguous Memory Allocator"
depends on HAVE_MEMBLOCK && MMU
select MIGRATION
and it blows up if I remove dependecy on MMU :(
Another option would be drivers/base/dma-coherent.c, but, IIUC, in this case
memory is reserved per device exclusively, so I'm in doubt if tiny M-class can
afford that...
>
> Other than the allocator issue, though, the rest of the refactoring does
> look nice.
Thanks for going through it!
Cheers
Vladimir
>
> Robin.
>
>>
>> Cheers
>> Vladimir
>>
>
>
^ permalink raw reply
* [PATCH v2 3/9] ARM: dts: dra72: Add separate dtsi for tps65917
From: Roger Quadros @ 2016-12-13 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <91b4075f-2dfb-0d17-592d-f99b91ace590@ti.com>
Lokesh,
On 13/12/16 15:08, Lokesh Vutla wrote:
>
>
> On Tuesday 13 December 2016 06:10 PM, Roger Quadros wrote:
>> +Tomi, Kishon, Carlos
>>
>> Hi,
>>
>> On 21/10/16 13:38, Lokesh Vutla wrote:
>>> dra72-evm-common.dtsi consolidates dra72-evm.dts and dra72-evm-revc.dts
>>> which also include tps65917 pmic support as both the evms uses the same
>>> pmic. But, dra71-evm has mostly similar features with a different pmic.
>>> In order to exploit dra72-evm-common.dtsi, creating a separate dtsi
>>> for tps65915 support and including it in respective board files.
>>>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>> arch/arm/boot/dts/dra72-evm-common.dtsi | 128 ----------------------------
>>> arch/arm/boot/dts/dra72-evm-revc.dts | 21 +++--
>>> arch/arm/boot/dts/dra72-evm-tps65917.dtsi | 134 ++++++++++++++++++++++++++++++
>>> arch/arm/boot/dts/dra72-evm.dts | 14 ++--
>>> 4 files changed, 154 insertions(+), 143 deletions(-)
>>> create mode 100644 arch/arm/boot/dts/dra72-evm-tps65917.dtsi
>>>
>>
>> This patch breaks USB XHCI and boot on dra72-evm (both revC and non revC)
>>
>> I'll explain why below.
>>
>> [ 13.625167] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
>> [ 13.632557] pgd = ede10000
>> [ 13.635390] [00000000] *pgd=00000000
>> [ 13.639145] Internal error: : 1406 [#1] SMP ARM
>> [ 13.643893] Modules linked in: xhci_plat_hcd(+) xhci_hcd usbcore omapfb dwc3 cfbfillrect snd_soc_davinci_mcasp cfbimgblt cfbcopyarea udc_core connector_hdmi encoder_tpd12s015 snd_soc_edma m25p80 snd_soc_simpe
>> [ 13.695557] CPU: 0 PID: 440 Comm: modprobe Not tainted 4.9.0-rc1 #1050
>> [ 13.702399] Hardware name: Generic DRA72X (Flattened Device Tree)
>> [ 13.708786] task: edb5c040 task.stack: edd10000
>> [ 13.713540] PC is at _raw_spin_unlock_irqrestore+0x0/0x44
>> [ 13.719219] LR is at xhci_hub_control+0xc2c/0x15e0 [xhci_hcd]
>> [ 13.725242] pc : [<c07df718>] lr : [<bf486300>] psr: a0000093
>> [ 13.725242] sp : edd118c0 ip : c0e306b4 fp : 00000000
>> [ 13.737278] r10: 00000000 r9 : 60000013 r8 : edf28218
>> [ 13.742753] r7 : edf28000 r6 : 00000000 r5 : 00000000 r4 : edf2a000
>> [ 13.749593] r3 : 00000000 r2 : 00000000 r1 : 60000013 r0 : edf28218
>
> Hmm.. Thanks for catching it. I remember it was booting when I tested,
> not sure how I missed this :(. usb2_phy1 & 2, mmc and dss supplies needs
> to be added in dra72-evm-tps65917.dtsi file.
OK.
>
> Tony,
> Do you want me to resend this patch or fixup patch on top of this?
In case you send a new patch. please add
Reported-by: Carlos Hernandez <ceh@ti.com>
Thanks.
cheers,
-roger
^ permalink raw reply
* [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
From: Sricharan @ 2016-12-13 14:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4e6ebf88-0138-afce-d752-56b66d69772f@arm.com>
Hi,
>
>On 13/12/16 14:38, Sricharan wrote:
>> Hi Robin,
>>
>>> -----Original Message-----
>>> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Robin Murphy
>>> Sent: Tuesday, December 13, 2016 7:33 PM
>>> To: Sricharan R <sricharan@codeaurora.org>; jcrouse at codeaurora.org; pdaly at codeaurora.org; jgebben at codeaurora.org;
>>> joro at 8bytes.org; linux-kernel at vger.kernel.org; pratikp at codeaurora.org; iommu at lists.linux-foundation.org;
>tzeng at codeaurora.org;
>>> linux-arm-kernel at lists.infradead.org; will.deacon at arm.com; mitchelh at codeaurora.org; vinod.koul at intel.com
>>> Cc: dan.j.williams at intel.com
>>> Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>>
>>> On 12/12/16 18:38, Sricharan R wrote:
>>>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>>
>>>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>>>> are only accessible to privileged DMA engines. Implement it in
>>>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>>>
>>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>> ---
>>>> arch/arm64/mm/dma-mapping.c | 6 +++---
>>>> drivers/iommu/dma-iommu.c | 10 ++++++++--
>>>> include/linux/dma-iommu.h | 3 ++-
>>>> 3 files changed, 13 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>>> index 401f79a..ae76ead 100644
>>>> --- a/arch/arm64/mm/dma-mapping.c
>>>> +++ b/arch/arm64/mm/dma-mapping.c
>>>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>>> unsigned long attrs)
>>>> {
>>>> bool coherent = is_device_dma_coherent(dev);
>>>> - int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>>>> + int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>>> size_t iosize = size;
>>>> void *addr;
>>>>
>>>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>>> unsigned long attrs)
>>>> {
>>>> bool coherent = is_device_dma_coherent(dev);
>>>> - int prot = dma_direction_to_prot(dir, coherent);
>>>> + int prot = dma_info_to_prot(dir, coherent, attrs);
>>>> dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>>>
>>>> if (!iommu_dma_mapping_error(dev, dev_addr) &&
>>>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>>> __iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>>>
>>>> return iommu_dma_map_sg(dev, sgl, nelems,
>>>> - dma_direction_to_prot(dir, coherent));
>>>> + dma_info_to_prot(dir, coherent, attrs));
>>>> }
>>>>
>>>> static void __iommu_unmap_sg_attrs(struct device *dev,
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index d2a7a46..756d5e0 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>>> EXPORT_SYMBOL(iommu_dma_init_domain);
>>>>
>>>> /**
>>>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>>>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>>>> + * page flags.
>>>> * @dir: Direction of DMA transfer
>>>> * @coherent: Is the DMA master cache-coherent?
>>>> + * @attrs: DMA attributes for the mapping
>>>> *
>>>> * Return: corresponding IOMMU API page protection flags
>>>> */
>>>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>>>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>>>> + unsigned long attrs)
>>>> {
>>>> int prot = coherent ? IOMMU_CACHE : 0;
>>>>
>>>> + if (attrs & DMA_ATTR_PRIVILEGED)
>>>> + prot |= IOMMU_PRIV;
>>>> +
>>>> switch (dir) {
>>>> case DMA_BIDIRECTIONAL:
>>>> return prot | IOMMU_READ | IOMMU_WRITE;
>>>
>>> ...and applying against -next now also needs this hunk:
>>>
>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>> *dev, phys_addr_t phys,
>>> size_t size, enum dma_data_direction dir, unsigned long attrs)
>>> {
>>> return __iommu_dma_map(dev, phys, size,
>>> - dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>> + dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>> }
>>>
>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>
>>> With those two issues fixed up, I've given the series (applied to
>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>
>>
>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>> 'checks out' you mean something is not working correct ?
>
>No, I mean it _is_ still correct - I guess that's more of an idiom than
>I thought :)
>
ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.
Regards,
Sricharan
^ permalink raw reply
* i.MX31 3DS board not booting after commit 1c2f87c22... ARM: 8025/1: Get rid of meminfo
From: Laura Abbott @ 2016-12-13 14:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <27bbaa4c-7c8d-cf76-c98f-e8edc89367db@gmail.com>
On 12/11/2016 02:34 AM, Magnus Lilja wrote:
> Hi,
>
> I'm trying to get current modern Linux kernels to boot on the i.MX31 PDK board (a.k.a MX31_3DS).
>
> One problem I've noticed is that I have to comment out the following code from mach-mx31_3ds.c, the reserve function is registered as mach_desc->reserve . If I leave this function as is the kernel stops very early in the boot (see end of the mail).
>
> static void __init mx31_3ds_reserve(void)
> {
> /* reserve MX31_3DS_CAMERA_BUF_SIZE bytes for mx3-camera */
> mx3_camera_base = arm_memblock_steal(MX31_3DS_CAMERA_BUF_SIZE,
> MX31_3DS_CAMERA_BUF_SIZE);
> }
>
> The commits that causes this problem (according to git bisect) is:
> [1c2f87c22566cd057bc8cde10c37ae9da1a1bb76] ARM: 8025/1: Get rid of meminfo
>
> (Yes, it's been a while since I last tried to boot a modern kernel on this board)
>
> I'm using the imx_v6_v7_defconfig.
>
> Kernel boot log using earlyprintk. (the arm_memblock_init NNN lines are my added printk's)
>
> Uncompressing Linux... done, booting the kernel.
> [ 0.000000] Booting Linux on physical CPU 0x0
> [ 0.000000] Linux version 4.9.0-rc7-dirty (lilja at xubuntu-14-04) (gcc version 4. 8.3 20140320 (prerelease) (Sourcery CodeBench Lite 2014.05-29) ) #150 SMP Sun Dec 11 11:12:32 CET 2016
> [ 0.000000] CPU: ARMv6-compatible processor [4107b364] revision 4 (ARMv6TEJ), cr=00c5387d
> [ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing instruction cache
> [ 0.000000] Machine: Freescale MX31PDK (3DS)
> [ 0.000000] bootconsole [earlycon0] enabled
> [ 0.000000] arm_memblock_init 268
> [ 0.000000] arm_memblock_init 273
> [ 0.000000] arm_memblock_init 277
> [ 0.000000] cma: Reserved 16 MiB at 0x86800000
> [ 0.000000] arm_memblock_init 281
> [ 0.000000] Memory policy: Data cache writeback
> [ 0.000000] CPU: All CPU(s) started in SVC mode.
> [ 0.000000] percpu: Embedded 14 pages/cpu @c64f0000 s26688 r8192 d22464 u57344
> [ 0.000000] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 30464
> [ 0.000000] Kernel command line: console=ttymxc0,115200 earlyprintk
> [ 0.000000] PID hash table entries: 512 (order: -1, 2048 bytes)
> [ 0.000000] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
> [ 0.000000] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
>
> This is where boot stops. If I comment out the arm_memblock_steal() call from mx31_3ds_reserve() the boot works and the next couple of lines are:
>
> [ 0.000000] Memory: 92564K/131072K available (8192K kernel code, 360K rwdata, 2 280K rodata, 1024K init, 8218K bss, 22124K reserved, 16384K cma-reserved, 0K highmem)
> [ 0.000000] Virtual kernel memory layout:
> [ 0.000000] vector : 0xffff0000 - 0xffff1000 ( 4 kB)
> [ 0.000000] fixmap : 0xffc00000 - 0xfff00000 (3072 kB)
> [ 0.000000] vmalloc : 0xc8800000 - 0xff800000 ( 880 MB)
> [ 0.000000] lowmem : 0xc0000000 - 0xc8000000 ( 128 MB)
> [ 0.000000] pkmap : 0xbfe00000 - 0xc0000000 ( 2 MB)
> [ 0.000000] modules : 0xbf000000 - 0xbfe00000 ( 14 MB)
> [ 0.000000] .text : 0xc0008000 - 0xc0900000 (9184 kB)
> [ 0.000000] .init : 0xc0c00000 - 0xc0d00000 (1024 kB)
> [ 0.000000] .data : 0xc0d00000 - 0xc0d5a120 ( 361 kB
>
>
> I have really no clue why that commit causes problems. There are not a lot of boards using mach_desc->reserve .
>
> Any ideas on how to fix or debug this?
>
> Regards, Magnus
I found the bug, it's another issue with removal and the lowmem boundary.
Given this has been present for 2.5 years without a complaint, I'm going
to take a little bit more time to think about a fix.
Thanks,
Laura
^ permalink raw reply
* [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
From: Robin Murphy @ 2016-12-13 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <002301d2554e$89112d20$9b338760$@codeaurora.org>
On 13/12/16 14:38, Sricharan wrote:
> Hi Robin,
>
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Robin Murphy
>> Sent: Tuesday, December 13, 2016 7:33 PM
>> To: Sricharan R <sricharan@codeaurora.org>; jcrouse at codeaurora.org; pdaly at codeaurora.org; jgebben at codeaurora.org;
>> joro at 8bytes.org; linux-kernel at vger.kernel.org; pratikp at codeaurora.org; iommu at lists.linux-foundation.org; tzeng at codeaurora.org;
>> linux-arm-kernel at lists.infradead.org; will.deacon at arm.com; mitchelh at codeaurora.org; vinod.koul at intel.com
>> Cc: dan.j.williams at intel.com
>> Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>
>> On 12/12/16 18:38, Sricharan R wrote:
>>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>
>>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>>> are only accessible to privileged DMA engines. Implement it in
>>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>>
>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>>> ---
>>> arch/arm64/mm/dma-mapping.c | 6 +++---
>>> drivers/iommu/dma-iommu.c | 10 ++++++++--
>>> include/linux/dma-iommu.h | 3 ++-
>>> 3 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>> index 401f79a..ae76ead 100644
>>> --- a/arch/arm64/mm/dma-mapping.c
>>> +++ b/arch/arm64/mm/dma-mapping.c
>>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>> unsigned long attrs)
>>> {
>>> bool coherent = is_device_dma_coherent(dev);
>>> - int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>>> + int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>> size_t iosize = size;
>>> void *addr;
>>>
>>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>> unsigned long attrs)
>>> {
>>> bool coherent = is_device_dma_coherent(dev);
>>> - int prot = dma_direction_to_prot(dir, coherent);
>>> + int prot = dma_info_to_prot(dir, coherent, attrs);
>>> dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>>
>>> if (!iommu_dma_mapping_error(dev, dev_addr) &&
>>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>> __iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>>
>>> return iommu_dma_map_sg(dev, sgl, nelems,
>>> - dma_direction_to_prot(dir, coherent));
>>> + dma_info_to_prot(dir, coherent, attrs));
>>> }
>>>
>>> static void __iommu_unmap_sg_attrs(struct device *dev,
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index d2a7a46..756d5e0 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>> EXPORT_SYMBOL(iommu_dma_init_domain);
>>>
>>> /**
>>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>>> + * page flags.
>>> * @dir: Direction of DMA transfer
>>> * @coherent: Is the DMA master cache-coherent?
>>> + * @attrs: DMA attributes for the mapping
>>> *
>>> * Return: corresponding IOMMU API page protection flags
>>> */
>>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>>> + unsigned long attrs)
>>> {
>>> int prot = coherent ? IOMMU_CACHE : 0;
>>>
>>> + if (attrs & DMA_ATTR_PRIVILEGED)
>>> + prot |= IOMMU_PRIV;
>>> +
>>> switch (dir) {
>>> case DMA_BIDIRECTIONAL:
>>> return prot | IOMMU_READ | IOMMU_WRITE;
>>
>> ...and applying against -next now also needs this hunk:
>>
>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>> *dev, phys_addr_t phys,
>> size_t size, enum dma_data_direction dir, unsigned long attrs)
>> {
>> return __iommu_dma_map(dev, phys, size,
>> - dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>> + dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>> }
>>
>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>
>> With those two issues fixed up, I've given the series (applied to
>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>
>
> oops, sorry that i missed this in rebase. I can repost now with this fixed,
> 'checks out' you mean something is not working correct ?
No, I mean it _is_ still correct - I guess that's more of an idiom than
I thought :)
Robin.
>
> Regards,
> Sricharan
>
^ 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