* Re: [PATCH 1/6] regulator: helpers: Add regmap set_soft_start helper
From: kbuild test robot @ 2017-03-24 16:19 UTC (permalink / raw)
To: Charles Keepax
Cc: mark.rutland, devicetree, alsa-devel, patches, lgirdwood, robh+dt,
broonie, kbuild-all, lee.jones
In-Reply-To: <1490107539-23995-1-git-send-email-ckeepax@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 11088 bytes --]
Hi Charles,
[auto build test WARNING on asoc/for-next]
[also build test WARNING on v4.11-rc3 next-20170324]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Charles-Keepax/regulator-helpers-Add-regmap-set_soft_start-helper/20170324-223205
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/init.h:1: warning: no structured comments found
kernel/sched/core.c:2085: warning: No description found for parameter 'rf'
kernel/sched/core.c:2085: warning: Excess function parameter 'cookie' description in 'try_to_wake_up_local'
include/linux/kthread.h:26: warning: Excess function parameter '...' description in 'kthread_create'
kernel/sys.c:1: warning: no structured comments found
include/linux/device.h:969: warning: No description found for parameter 'dma_ops'
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
include/linux/iio/iio.h:597: warning: No description found for parameter 'trig_readonly'
include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev'
include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig'
include/linux/device.h:970: warning: No description found for parameter 'dma_ops'
>> include/linux/regulator/driver.h:358: warning: No description found for parameter 'soft_start_reg'
>> include/linux/regulator/driver.h:358: warning: No description found for parameter 'soft_start_mask'
>> include/linux/regulator/driver.h:358: warning: No description found for parameter 'soft_start_val_on'
drivers/regulator/core.c:1467: warning: Excess function parameter 'ret' description in 'regulator_dev_lookup'
include/drm/drm_drv.h:438: warning: No description found for parameter 'open'
include/drm/drm_drv.h:438: warning: No description found for parameter 'preclose'
include/drm/drm_drv.h:438: warning: No description found for parameter 'postclose'
include/drm/drm_drv.h:438: warning: No description found for parameter 'lastclose'
include/drm/drm_drv.h:438: warning: No description found for parameter 'set_busid'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_handler'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_preinstall'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_postinstall'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_uninstall'
include/drm/drm_drv.h:438: warning: No description found for parameter 'debugfs_init'
include/drm/drm_drv.h:438: warning: No description found for parameter 'debugfs_cleanup'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_open_object'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_close_object'
include/drm/drm_drv.h:438: warning: No description found for parameter 'prime_handle_to_fd'
include/drm/drm_drv.h:438: warning: No description found for parameter 'prime_fd_to_handle'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_export'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_import'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_pin'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_unpin'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_res_obj'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_get_sg_table'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_import_sg_table'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_vmap'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_vunmap'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_mmap'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_vm_ops'
include/drm/drm_drv.h:438: warning: No description found for parameter 'major'
include/drm/drm_drv.h:438: warning: No description found for parameter 'minor'
include/drm/drm_drv.h:438: warning: No description found for parameter 'patchlevel'
include/drm/drm_drv.h:438: warning: No description found for parameter 'name'
include/drm/drm_drv.h:438: warning: No description found for parameter 'desc'
include/drm/drm_drv.h:438: warning: No description found for parameter 'date'
include/drm/drm_drv.h:438: warning: No description found for parameter 'driver_features'
include/drm/drm_drv.h:438: warning: No description found for parameter 'ioctls'
include/drm/drm_drv.h:438: warning: No description found for parameter 'num_ioctls'
include/drm/drm_drv.h:438: warning: No description found for parameter 'fops'
include/drm/drm_color_mgmt.h:1: warning: no structured comments found
drivers/gpu/drm/drm_fb_cma_helper.c:557: warning: Excess function parameter 'num_crtc' description in 'drm_fbdev_cma_init'
drivers/gpu/drm/drm_fb_cma_helper.c:558: warning: Excess function parameter 'num_crtc' description in 'drm_fbdev_cma_init'
drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'pipe'
drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'dp_output'
drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'link_rate'
drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'pipe'
drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'dp_output'
drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'link_rate'
drivers/media/dvb-core/dvb_frontend.h:677: warning: No description found for parameter 'refcount'
Documentation/core-api/assoc_array.rst:13: WARNING: Enumerated list ends without a blank line; unexpected unindent.
Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown target name: "sphinx c domain".
kernel/sched/fair.c:7616: WARNING: Inline emphasis start-string without end-string.
kernel/time/timer.c:1200: ERROR: Unexpected indentation.
kernel/time/timer.c:1202: ERROR: Unexpected indentation.
kernel/time/timer.c:1203: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:122: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:125: ERROR: Unexpected indentation.
include/linux/wait.h:127: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/time/hrtimer.c:990: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/signal.c:322: WARNING: Inline literal start-string without end-string.
include/linux/iio/iio.h:219: ERROR: Unexpected indentation.
include/linux/iio/iio.h:220: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/iio/iio.h:226: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/iio/industrialio-core.c:639: ERROR: Unknown target name: "iio_val".
drivers/iio/industrialio-core.c:646: ERROR: Unknown target name: "iio_val".
drivers/message/fusion/mptbase.c:5051: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/tty/serial/serial_core.c:1898: WARNING: Definition list ends without a blank line; unexpected unindent.
include/linux/regulator/driver.h:271: ERROR: Unknown target name: "regulator_regmap_x_voltage".
include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
drivers/usb/core/message.c:478: ERROR: Unexpected indentation.
drivers/usb/core/message.c:479: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_type".
Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_dir".
Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_recip".
Documentation/driver-api/usb.rst:689: ERROR: Unknown target name: "usbdevfs_urb_type".
sound/soc/soc-core.c:2670: ERROR: Unknown target name: "snd_soc_daifmt".
sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn".
WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the imgmath_dvipng setting
vim +/soft_start_reg +358 include/linux/regulator/driver.h
ca5d1b35 Carlo Caione 2014-03-05 342 unsigned int bypass_val_on;
ca5d1b35 Carlo Caione 2014-03-05 343 unsigned int bypass_val_off;
354794da Laxman Dewangan 2016-03-02 344 unsigned int active_discharge_on;
354794da Laxman Dewangan 2016-03-02 345 unsigned int active_discharge_off;
354794da Laxman Dewangan 2016-03-02 346 unsigned int active_discharge_mask;
354794da Laxman Dewangan 2016-03-02 347 unsigned int active_discharge_reg;
8b45dea6 Charles Keepax 2017-03-21 348 unsigned int soft_start_reg;
8b45dea6 Charles Keepax 2017-03-21 349 unsigned int soft_start_mask;
8b45dea6 Charles Keepax 2017-03-21 350 unsigned int soft_start_val_on;
79511ed3 Mark Brown 2012-06-27 351
79511ed3 Mark Brown 2012-06-27 352 unsigned int enable_time;
871f5650 Guodong Xu 2014-08-13 353
871f5650 Guodong Xu 2014-08-13 354 unsigned int off_on_delay;
87e1e0f2 Javier Martinez Canillas 2014-11-10 355
87e1e0f2 Javier Martinez Canillas 2014-11-10 356 unsigned int (*of_map_mode)(unsigned int mode);
571a354b Liam Girdwood 2008-04-30 357 };
571a354b Liam Girdwood 2008-04-30 @358
c172708d Mark Brown 2012-04-04 359 /**
c172708d Mark Brown 2012-04-04 360 * struct regulator_config - Dynamic regulator descriptor
c172708d Mark Brown 2012-04-04 361 *
c172708d Mark Brown 2012-04-04 362 * Each regulator registered with the core is described with a
c172708d Mark Brown 2012-04-04 363 * structure of this type and a struct regulator_desc. This structure
c172708d Mark Brown 2012-04-04 364 * contains the runtime variable parts of the regulator description.
c172708d Mark Brown 2012-04-04 365 *
c172708d Mark Brown 2012-04-04 366 * @dev: struct device for the regulator
:::::: The code at line 358 was first introduced by commit
:::::: 571a354b1542a274d88617e1f6703f3fe7a517f1 regulator: regulator driver interface
:::::: TO: Liam Girdwood <lg@opensource.wolfsonmicro.com>
:::::: CC: Liam Girdwood <lg@opensource.wolfsonmicro.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6576 bytes --]
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Marc Zyngier @ 2017-03-24 16:22 UTC (permalink / raw)
To: Youlin Pei, Rob Herring, Matthias Brugger
Cc: Thomas Gleixner, Jason Cooper, Mark Rutland, Russell King,
linux-kernel, devicetree, linux-arm-kernel, linux-mediatek,
srv_heupstream, hongkun.cao, yong.wu, erin.lo
In-Reply-To: <1487040961-18319-3-git-send-email-youlin.pei@mediatek.com>
On 14/02/17 02:56, Youlin Pei wrote:
> In Mediatek SOCs, the CIRQ is a low power interrupt controller
> designed to works outside MCUSYS which comprises with Cortex-Ax
> cores,CCI and GIC.
>
> The CIRQ controller is integrated in between MCUSYS( include
> Cortex-Ax, CCI and GIC ) and interrupt sources as the second
> level interrupt controller. The external interrupts which outside
> MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
> all edge trigger interupts. When an edge interrupt is triggered,
> CIRQ can record the status and generate a pulse signal to GIC when
> flush command executed.
>
> When system enters sleep mode, MCUSYS will be turned off to improve
> power consumption, also GIC is power down. The edge trigger interrupts
> will be lost in this scenario without CIRQ.
>
> This commit provides the CIRQ irqchip implement.
>
> Signed-off-by: Youlin Pei <youlin.pei@mediatek.com>
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-mtk-cirq.c | 288 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 289 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-mtk-cirq.c
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 0e55d94..db9acd1 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -61,7 +61,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ) += irq-bcm7120-l2.o
> obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o
> obj-$(CONFIG_KEYSTONE_IRQ) += irq-keystone.o
> obj-$(CONFIG_MIPS_GIC) += irq-mips-gic.o
> -obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o
> +obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o irq-mtk-cirq.o
> obj-$(CONFIG_ARCH_DIGICOLOR) += irq-digicolor.o
> obj-$(CONFIG_RENESAS_H8300H_INTC) += irq-renesas-h8300h.o
> obj-$(CONFIG_RENESAS_H8S_INTC) += irq-renesas-h8s.o
> diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> new file mode 100644
> index 0000000..b5cf506
> --- /dev/null
> +++ b/drivers/irqchip/irq-mtk-cirq.c
> @@ -0,0 +1,288 @@
> +/*
> + * Copyright (c) 2016 MediaTek Inc.
> + * Author: Youlin.Pei <youlin.pei@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License 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.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/syscore_ops.h>
> +
> +#define CIRQ_ACK 0x40
> +#define CIRQ_MASK_SET 0xc0
> +#define CIRQ_MASK_CLR 0x100
> +#define CIRQ_SENS_SET 0x180
> +#define CIRQ_SENS_CLR 0x1c0
> +#define CIRQ_POL_SET 0x240
> +#define CIRQ_POL_CLR 0x280
> +#define CIRQ_CONTROL 0x300
> +
> +#define CIRQ_EN 0x1
> +#define CIRQ_EDGE 0x2
> +#define CIRQ_FLUSH 0x4
> +
> +struct mtk_cirq_chip_data {
> + void __iomem *base;
> + unsigned int ext_irq_start;
> + unsigned int ext_irq_end;
> + struct irq_domain *domain;
> +};
> +
> +static struct mtk_cirq_chip_data *cirq_data;
Question in relation to the sysirq patches that have been queued for
4.12: Are we going to see something similar with this driver, where
you're going to have to support multiple non-contiguous register sets?
I'd rather have this kind of thing from day one, instead of adding that
at a later time...
Specially given that the platform that is using this driver does also
have a sysirq controller.
> +
> +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
> +{
> + struct mtk_cirq_chip_data *chip_data = data->chip_data;
> + unsigned int cirq_num = data->hwirq;
> + u32 mask = 1 << (cirq_num % 32);
> +
> + writel_relaxed(mask, chip_data->base + offset + (cirq_num / 32) * 4);
> +}
> +
> +static void mtk_cirq_mask(struct irq_data *data)
> +{
> + mtk_cirq_write_mask(data, CIRQ_MASK_SET);
> + irq_chip_mask_parent(data);
> +}
> +
> +static void mtk_cirq_unmask(struct irq_data *data)
> +{
> + mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
> + irq_chip_unmask_parent(data);
> +}
> +
> +static int mtk_cirq_set_type(struct irq_data *data, unsigned int type)
> +{
> + int ret;
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_EDGE_FALLING:
> + mtk_cirq_write_mask(data, CIRQ_POL_CLR);
> + mtk_cirq_write_mask(data, CIRQ_SENS_CLR);
> + break;
> + case IRQ_TYPE_EDGE_RISING:
> + mtk_cirq_write_mask(data, CIRQ_POL_SET);
> + mtk_cirq_write_mask(data, CIRQ_SENS_CLR);
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + mtk_cirq_write_mask(data, CIRQ_POL_CLR);
> + mtk_cirq_write_mask(data, CIRQ_SENS_SET);
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + mtk_cirq_write_mask(data, CIRQ_POL_SET);
> + mtk_cirq_write_mask(data, CIRQ_SENS_SET);
> + break;
> + default:
> + break;
> + }
> +
> + data = data->parent_data;
> + ret = data->chip->irq_set_type(data, type);
> + return ret;
> +}
> +
> +static struct irq_chip mtk_cirq_chip = {
> + .name = "MT_CIRQ",
> + .irq_mask = mtk_cirq_mask,
> + .irq_unmask = mtk_cirq_unmask,
> + .irq_eoi = irq_chip_eoi_parent,
> + .irq_set_type = mtk_cirq_set_type,
> + .irq_retrigger = irq_chip_retrigger_hierarchy,
> +#ifdef CONFIG_SMP
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> +#endif
> +};
> +
> +static int mtk_cirq_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + if (is_of_node(fwspec->fwnode)) {
> + if (fwspec->param_count != 3)
> + return -EINVAL;
> +
> + /* No PPI should point to this domain */
> + if (fwspec->param[0] != 0)
> + return -EINVAL;
> +
> + /* cirq support irq number check */
> + if (fwspec->param[1] < cirq_data->ext_irq_start ||
> + fwspec->param[1] > cirq_data->ext_irq_end)
> + return -EINVAL;
> +
> + *hwirq = fwspec->param[1] - cirq_data->ext_irq_start;
> + *type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int mtk_cirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + int ret;
> + irq_hw_number_t hwirq;
> + unsigned int type;
> + struct irq_fwspec *fwspec = arg;
> + struct irq_fwspec parent_fwspec = *fwspec;
> +
> + ret = mtk_cirq_domain_translate(domain, fwspec, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + if (WARN_ON(nr_irqs != 1))
> + return -EINVAL;
> +
> + irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &mtk_cirq_chip,
> + domain->host_data);
> +
> + parent_fwspec.fwnode = domain->parent->fwnode;
> + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> + &parent_fwspec);
> +}
> +
> +static const struct irq_domain_ops cirq_domain_ops = {
> + .translate = mtk_cirq_domain_translate,
> + .alloc = mtk_cirq_domain_alloc,
> + .free = irq_domain_free_irqs_common,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mtk_cirq_suspend(void)
> +{
> + u32 value, mask;
> + unsigned int irq, hwirq_num;
> + bool pending, masked;
> + int i, pendret, maskret;
> +
> + hwirq_num = cirq_data->ext_irq_end - cirq_data->ext_irq_start + 1;
> + for (i = 0; i < hwirq_num; i++) {
> + irq = irq_find_mapping(cirq_data->domain, i);
> + if (irq) {
> + pendret = irq_get_irqchip_state(irq,
> + IRQCHIP_STATE_PENDING,
> + &pending);
> +
> + maskret = irq_get_irqchip_state(irq,
> + IRQCHIP_STATE_MASKED,
> + &masked);
> +
> + if (pendret == 0 && maskret == 0 &&
> + (pending && !masked))
> + continue;
> + }
> +
> + mask = 1 << (i % 32);
> + writel_relaxed(mask, cirq_data->base + CIRQ_ACK + (i / 32) * 4);
> + }
It would be worth documenting exactly what this is for. I had to go back
to our previous discussion in November to remember it.
> +
> + /* set edge_only mode, record edge-triggerd interrupts */
> + /* enable cirq */
> + value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> + value |= (CIRQ_EDGE | CIRQ_EN);
> + writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
> +
> + return 0;
> +}
> +
> +static void mtk_cirq_resume(void)
> +{
> + u32 value;
> +
> + /* flush recored interrupts, will send signals to parent controller */
> + value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> + writel_relaxed(value | CIRQ_FLUSH, cirq_data->base + CIRQ_CONTROL);
> +
> + /* disable cirq */
> + value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> + value &= ~(CIRQ_EDGE | CIRQ_EN);
> + writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
> +}
> +
> +static struct syscore_ops mtk_cirq_syscore_ops = {
> + .suspend = mtk_cirq_suspend,
> + .resume = mtk_cirq_resume,
> +};
> +
> +static void mtk_cirq_syscore_init(void)
> +{
> + register_syscore_ops(&mtk_cirq_syscore_ops);
> +}
> +#else
> +static inline void mtk_cirq_syscore_init(void) {}
> +#endif
> +
> +static int __init mtk_cirq_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + struct irq_domain *domain, *domain_parent;
> + unsigned int irq_num;
> + int ret;
> +
> + domain_parent = irq_find_host(parent);
> + if (!domain_parent) {
> + pr_err("mtk_cirq: interrupt-parent not found\n");
> + return -EINVAL;
> + }
> +
> + cirq_data = kzalloc(sizeof(*cirq_data), GFP_KERNEL);
> + if (!cirq_data)
> + return -ENOMEM;
> +
> + cirq_data->base = of_iomap(node, 0);
> + if (!cirq_data->base) {
> + pr_err("mtk_cirq: unable to map cirq register\n");
> + ret = -ENXIO;
> + goto out_free;
> + }
> +
> + ret = of_property_read_u32_index(node, "mediatek,ext-irq-range", 0,
> + &cirq_data->ext_irq_start);
> + if (ret)
> + goto out_unmap;
> +
> + ret = of_property_read_u32_index(node, "mediatek,ext-irq-range", 1,
> + &cirq_data->ext_irq_end);
> + if (ret)
> + goto out_unmap;
> +
> + irq_num = cirq_data->ext_irq_end - cirq_data->ext_irq_start + 1;
> + domain = irq_domain_add_hierarchy(domain_parent, 0,
> + irq_num, node,
> + &cirq_domain_ops, cirq_data);
> + if (!domain) {
> + ret = -ENOMEM;
> + goto out_unmap;
> + }
> + cirq_data->domain = domain;
> +
> + mtk_cirq_syscore_init();
> +
> + return 0;
> +
> +out_unmap:
> + iounmap(cirq_data->base);
> +out_free:
> + kfree(cirq_data);
> + return ret;
> +}
> +
> +IRQCHIP_DECLARE(mtk_cirq, "mediatek,mtk-cirq", mtk_cirq_of_init);
>
It otherwise looks much better than the previous iteration.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence
From: Rob Herring @ 2017-03-24 16:26 UTC (permalink / raw)
To: Piotr Sroka
Cc: linux-mmc, Adrian Hunter, Ulf Hansson, linux-kernel,
Masahiro Yamada, Mark Rutland, devicetree
In-Reply-To: <1490106781-3129-1-git-send-email-piotrs@cadence.com>
On Tue, Mar 21, 2017 at 02:33:01PM +0000, Piotr Sroka wrote:
> DTS properties are used instead of fixed data
> because PHY settings can be different for different chips/boards.
> Add description of new DLL PHY delays.
>
> Signed-off-by: Piotr Sroka <piotrs@cadence.com>
> ---
> Changes for v2:
> - file was created in v2. It was a part of driver source file patch.
> - most delays were moved from dts file
> to data associated with an SoC specific compatible
> - description of delays was updated to be more clearly
> ---
> Changes for v3:
> - move all delays back to dts because they are also boards dependent
> - prefix all of the Cadence-specific properties with cdns prefix
> ---
> Changes for v4:
> - change the beginning of the commit subject
> ---
> Changes for v5:
> - change name of property to be consistent with timing modes
> available in Linux
I don't see any change here...
> ---
> .../devicetree/bindings/mmc/sdhci-cadence.txt | 48 ++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 4/6] regulator: arizona-micbias: Add description of micbias binding
From: Rob Herring @ 2017-03-24 16:27 UTC (permalink / raw)
To: Charles Keepax
Cc: mark.rutland, devicetree, alsa-devel, patches, lgirdwood, broonie,
lee.jones
In-Reply-To: <1490107539-23995-4-git-send-email-ckeepax@opensource.wolfsonmicro.com>
On Tue, Mar 21, 2017 at 02:45:37PM +0000, Charles Keepax wrote:
> Add the new micbias regulators into the Arizona regulator binding
> document.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> Documentation/devicetree/bindings/regulator/arizona-regulator.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller
From: jacopo @ 2017-03-24 16:45 UTC (permalink / raw)
To: Linus Walleij
Cc: Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart, Chris Brandt,
Rob Herring, Mark Rutland, Russell King, Linux-Renesas,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdYjZJ7e0tJy6_jfpRT+UDFWqnu8ZX18-gaAHVZLgW-prA@mail.gmail.com>
Hi Linus,
On Fri, Mar 24, 2017 at 04:42:47PM +0100, Linus Walleij wrote:
> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>
> I assume Geert will queue this driver even if it is outside of sh-pfc?
>
> > Add combined gpio and pin controller driver for Renesas RZ/A1
> > r7s72100 SoC.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > drivers/pinctrl/Kconfig | 10 +
> > drivers/pinctrl/Makefile | 1 +
> > drivers/pinctrl/pinctrl-rza1.c | 961 +++++++++++++++++++++++++++++++++++++++++
>
> So this is very different from the SH-PFC family and should not
> be in drivers/pinctrl/sh-pfc?
>
Correct. The pin controller hardware in RZ/A(1) devices configures pin
functions per-pin and not per-group as the SH/R-Car family does.
There has been an attempt to support this platforms with the existing
sh-pfc/ infrastructure in the past (https://lwn.net/Articles/573222/)
which I re-proposed for v4.9, but that driver simply does not fit
this hardware which is different from the one found in R-Car
devices.
More on the background of this series in this email thread:
https://marc.info/?l=linux-gpio&m=148536779917834&w=2
I initially created a whole new sub-directory where all the Renesas
devices with this kind of pin controller would have gone
(drivers/pinctrl/rz-pfc) but since as of now the only available
hardware of that type is RZ/A1 and some of its variants, we decided to go
with a single driver supporting that platform only.
If more will come, we'll think about supporting them through this
driver if possible, or create a dedicated directory.
Are you ok with this?
Thanks
j
> > +config PINCTRL_RZA1
> > + bool "Renesas RZ/A1 gpio and pinctrl driver"
> > + depends on OF
> > + depends on ARCH_R7S72100 || COMPILE_TEST
> > + select GENERIC_PINCTRL_GROUPS
> > + select GENERIC_PINMUX_FUNCTIONS
> > + select GENERIC_PINCONF
>
> If it is also a GPIO driver I guess it should
> select GPIOLIB as well.
>
> This was not possible in the past, but it is possible nowadays.
>
> > +struct gpio_chip rza1_gpiochip_template = {
> > + .request = rza1_gpio_request,
> > + .free = rza1_gpio_free,
> > + .get_direction = rza1_gpio_get_direction,
> > + .direction_input = rza1_gpio_direction_input,
> > + .direction_output = rza1_gpio_direction_output,
> > + .get = rza1_gpio_get,
> > + .set = rza1_gpio_set,
> > +};
>
> We now also have .set_multiple() and more interestingly
> .set_config() which can be backed by pinctrl if you want
> to e.g. support debouncing and/or open drain/open source.
>
> Maybe this is stuff your pin controller can do, but not needed
> in the initial submission for sure.
>
> > +static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int selector,
> > + unsigned int group)
>
> Please name it rza1_set_mux() to correspond with the ops field.
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH 3/9] arm64: mm: install SError abort handler
From: Doug Berger @ 2017-03-24 16:48 UTC (permalink / raw)
To: Mark Rutland
Cc: catalin.marinas-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
will.deacon-5wv7dgnIgG8, computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, james.morse-5wv7dgnIgG8,
vladimir.murzin-5wv7dgnIgG8, panand-H+wXaHxf7aLQT0dZR+AlfA,
andre.przywara-5wv7dgnIgG8, cmetcalf-VPRAkNaXOzVWk0Htik3J/w,
mingo-DgEjT+Ai2ygdnm+yROfE0A,
sandeepa.s.prabhu-Re5JQEeQqe8AvxtiuMwx3w,
shijie.huang-5wv7dgnIgG8, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170324151654.GD29588@leverpostej>
On 03/24/2017 08:16 AM, Mark Rutland wrote:
> On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
>> This commit adds support for minimal handling of SError aborts and
>> allows them to be hooked by a driver or other part of the kernel to
>> install a custom SError abort handler. The hook function returns
>> the previously registered handler so that handlers may be chained if
>> desired.
>>
>> The handler should return the value 0 if the error has been handled,
>> otherwise the handler should either call the next handler in the
>> chain or return a non-zero value.
>
> ... so the order these get calls is completely dependent on probe
> order...
Yes, but this was an attempt to keep some flexibility in handling a
very ambiguous event.
>
>> Since the Instruction Specific Syndrome value for SError aborts is
>> implementation specific the registerred handlers must implement
>> their own parsing of the syndrome.
>
> ... and drivers have to be intimately familiar with the CPU, in order to
> be able to parse its IMPLEMENTATION DEFINED ESR_ELx.ISS value.
>
> Even then, there's no guarantee there's anything useful there, since it
> is IMPLEMENTATION DEFINED and could simply be RES0 or UNKNOWN in all
> cases.
>
> I do not think it is a good idea to allow arbitrary drivers to hook
> this fault in this manner.
>
I agree. It should really be resolved in the fault handling code like
it is for the ARM architecture, but the IMPLEMENTATION DEFINED nature of
the event for ARM64 makes this unmanageable but for the most specific
use cases, which is what is attempted here.
>> + .align 6
>> +el0_error:
>> + kernel_entry 0
>> +el0_error_naked:
>> + mrs x25, esr_el1 // read the syndrome register
>> + lsr x24, x25, #ESR_ELx_EC_SHIFT // exception class
>> + cmp x24, #ESR_ELx_EC_SERROR // SError exception in EL0
>> + b.ne el0_error_inv
>> +el0_serr:
>> + mrs x26, far_el1
>> + // enable interrupts before calling the main handler
>> + enable_dbg_and_irq
>
> ... why?
>
> We don't do this for inv_entry today.
>
Yes, my initial downstream implementation modified inv_entry, but after
commit 7d9e8f71b989 ("arm64: avoid returning from bad mode") added the
user abort handling for el0_inv I tried to follow that approach so user
mode errors (i.e. bad writes) wouldn't kill the kernel.
>> + ct_user_exit
>> + bic x0, x26, #(0xff << 56)
>> + mov x1, x25
>> + mov x2, sp
>> + bl do_serr_abort
>> + b ret_to_user
>> +el0_error_inv:
>> + enable_dbg
>> + mov x0, sp
>> + mov x1, #BAD_ERROR
>> + mov x2, x25
>> + b bad_mode
>> +ENDPROC(el0_error)
>
> Clearly you expect these to be delivered at arbitrary times during
> execution. What if a KVM guest is executing at the time the SError is
> delivered?
The timing isn't really arbitrary in our particular use case. It is
just after the bus interface has moved on from the failing transaction
so from the bus interfaces perspective it is asynchronous. The main
benefit is to help debug user mode code that accidentally maps a bad
address since we would never make such an egregious error in the kernel ;)
I'm afraid I'm not fully versed on the implications to KVM here.
>
> To be quite frank, I don't believe that we can reliably and safely
> handle this misfeature in the kernel, and this infrastructure only
> provides the illusion that we can.
>
> I do not think it makes sense to do this.
>
> Thanks,
> Mark.
>
I understand your position since this was the cleanest approach I came
up with and it is admittedly ugly. I would be happy to entertain any
better suggestion on how this could be handled more cleanly.
If you would consider an alternative implementation where we scrap the
SError handler (i.e. maintain the ugliness in our downstream kernel) in
favor of a more gentle user mode crash on SError that allows the kernel
the opportunity to service the interrupt for diagnostic purposes I could
try to repackage that.
Thanks for the review!
Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 01/15] ARM: dts: alpine: fix PCIe node name
From: Arnd Bergmann @ 2017-03-24 17:00 UTC (permalink / raw)
To: Antoine Tenart
Cc: Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, arm-soc,
Linux ARM, Tsahee Zidenberg
In-Reply-To: <20170322080137.st3osq5ckfis6uwz@kwain>
On Wed, Mar 22, 2017 at 9:01 AM, Antoine Tenart
<antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Rob,
>
> On Tue, Mar 21, 2017 at 09:02:59PM -0500, Rob Herring wrote:
>> PCIe bridges should have a node name of 'pcie'.
>>
>> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Acked-by: Antoine Tenart <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>
> This can go through arm-soc directly. If not I'll take it.
> Thanks!
I have a mild preference for this kind of patch to get picked up by
platform maintainers.
Rob, once replies stop coming in, please send a pull request for anything
that did not get picked up by anyone else.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/2] can: spi: hi311x: Add Holt HI-311x CAN driver
From: Akshay Bhat @ 2017-03-24 17:20 UTC (permalink / raw)
To: wg-5Yr1BZd7O62+XT7JhA+gdA, mkl-bIcnvbaLZ9MEGnE8C9+IrQ
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Akshay Bhat
In-Reply-To: <1489785040-26477-2-git-send-email-akshay.bhat-jEh4hwF5bVhBDgjK7y7TUQ@public.gmane.org>
Hi Marc,
On 03/17/2017 05:10 PM, Akshay Bhat wrote:
> This patch adds support for the Holt HI-311x CAN controller. The HI311x
> CAN controller is capable of transmitting and receiving standard data
> frames, extended data frames and remote frames. The HI311x interfaces
> with the host over SPI.
>
> Datasheet: www.holtic.com/documents/371-hi-3110_v-rev-jpdf.do
>
> Signed-off-by: Akshay Bhat <nodeax-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>
If there are no further review comments can this series be applied to
can-next or does it need to wait for the next kernel release cycle (4.13)?
Thanks,
Akshay
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support
From: Frank Rowand @ 2017-03-24 17:23 UTC (permalink / raw)
To: Stefan Wahren, kernel, Zhang Rui, Eduardo Valentin
Cc: Eric Anholt, Rob Herring, Florian Fainelli, linux-rpi-kernel,
linux-pm, devicetree
In-Reply-To: <872776681.149555.1490340443231@email.1und1.de>
On 03/24/17 00:27, Stefan Wahren wrote:
>
>> Frank Rowand <frowand.list@gmail.com> hat am 24. März 2017 um 00:32 geschrieben:
>>
>>
>> On 03/12/17 15:11, Stefan Wahren wrote:
>>> Use the new function of_property_read_s32_array() to prepare
>>> of-thermal for negative coefficients. These are used by
>>> the upcoming bcm2835_thermal driver.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> ---
>>> drivers/thermal/of-thermal.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>> index d04ec3b..491d58a 100644
>>> --- a/drivers/thermal/of-thermal.c
>>> +++ b/drivers/thermal/of-thermal.c
>>> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>>> struct device_node *child = NULL, *gchild;
>>> struct __thermal_zone *tz;
>>> int ret, i;
>>> - u32 prop, coef[2];
>>> + u32 prop;
>>> + s32 coef[2];
>>>
>>> if (!np) {
>>> pr_err("no thermal zone np\n");
>>> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>>> * one sensor per thermal zone. Thus, we are considering
>>> * only the first two values as slope and offset.
>>> */
>>> - ret = of_property_read_u32_array(np, "coefficients", coef, 2);
>>> + ret = of_property_read_s32_array(np, "coefficients", coef, 2);
>>> if (ret == 0) {
>>> tz->slope = coef[0];
>>> tz->offset = coef[1];
>>>
>>
>> Since you are doing so much work to fix reading the array of s32 property, you
>> might also want to do the same for the s32 properties, "polling-delay-passive"
>> and "polling-delay". Just change of_property_read_u32() to of_property_read_s32()
>> and change the type of prop to match.
>>
>
> The intension behind this patch series is adding a new thermal driver
> not fixing of-thermal. Since the initial version of this series was
> posted by Martin in May 2016 i do not want to wait much longer.
Not my driver, your choice, I was just pointing out some compile warnings
(without thinking enough about the context of the warnings - see below).
>
> Btw changing polling-delay-passive and polling-delay into a signed
> doesn't make any sense to me. Why do we need negative delays?
Good point. I did not actually look at what the meaning of the two
properties is, and whether it makes sense for them to have a negative
value. I also did not check the binding description (doing so now,
it clearly states that these property values are unsigned), I just
noted that there is a mismatch between the type of the properties
(u32) and the variables they are assigned to. As the compile warnings
below indicate, if the property value is large enough, it will be a
negative value after assignment to the int variable. The proper
answer is probably to change the variables to unsigned.
>
> I suggest to send a separate patch for this issue.
It would be good if someone did.
Not a critical issue, just a trap waiting to catch someone who puts
a very large value for one of those properties in a device tree source
file and does not realize it will be a negative number in the driver.
>
> Thanks for the review.
>
>>
>> drivers/thermal/of-thermal.c: In function 'thermal_of_build_thermal_zone':
>> drivers/thermal/of-thermal.c:841:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
>> drivers/thermal/of-thermal.c:848:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
>>
>>
>> 836 ret = of_property_read_u32(np, "polling-delay-passive", &prop);
>> 837 if (ret < 0) {
>> 838 pr_err("missing polling-delay-passive property\n");
>> 839 goto free_tz;
>> 840 }
>> 841 tz->passive_delay = prop;
>> 842
>> 843 ret = of_property_read_u32(np, "polling-delay", &prop);
>> 844 if (ret < 0) {
>> 845 pr_err("missing polling-delay property\n");
>> 846 goto free_tz;
>> 847 }
>> 848 tz->polling_delay = prop;
>>
>>
>> -Frank
>
^ permalink raw reply
* Re: [PATCH v3 1/2] doc: Add bindings document for Xilinx LogiCore PR Decoupler
From: Alan Tull @ 2017-03-24 17:25 UTC (permalink / raw)
To: Moritz Fischer
Cc: Mark Rutland, Rob Herring, devicetree@vger.kernel.org,
Greg Kroah-Hartman, linux-fpga, Michal Simek, linux-kernel,
Moritz Fischer, Sören Brinkmann,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170324152318.GA13089@tyrael.amer.corp.natinst.com>
On Fri, Mar 24, 2017 at 10:23 AM, Moritz Fischer
<moritz.fischer.private@gmail.com> wrote:
> On Fri, Mar 24, 2017 at 09:59:08AM -0500, Rob Herring wrote:
>> On Fri, Mar 17, 2017 at 01:11:12PM -0700, Moritz Fischer wrote:
>> > This adds the binding documentation for the Xilinx LogiCORE PR
>> > Decoupler soft core.
>> >
>> > Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> > Cc: Michal Simek <michal.simek@xilinx.com>
>> > Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
>> > Cc: linux-kernel@vger.kernel.org
>> > Cc: devicetree@vger.kernel.org
>> > ---
>> >
>> > Changes from v2:
>> > - Added refence to generic fpga-region bindings
>> > - Fixed up reg property in example
>> > - Added fallback to "xlnx,pr-decoupler" without version
>> >
>> > Changes from v1:
>> > - Added clock names & clock to example
>> > - Merged some of the description from Michal's version
>> >
>> > ---
>> > .../bindings/fpga/xilinx-pr-decoupler.txt | 35 ++++++++++++++++++++++
>> > 1 file changed, 35 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
>> > new file mode 100644
>> > index 0000000..16141bd
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
>> > @@ -0,0 +1,35 @@
>> > +Xilinx LogiCORE Partial Reconfig Decoupler Softcore
>> > +
>> > +The Xilinx LogiCORE Partial Reconfig Decoupler manages one or more
>> > +decouplers / fpga bridges.
>> > +The controller can decouple/disable the bridges which prevents signal
>> > +changes from passing through the bridge. The controller can also
>> > +couple / enable the bridges which allows traffic to pass through the
>> > +bridge normally.
>> > +
>> > +The Driver supports only MMIO handling. A PR region can have multiple
>> > +PR Decouples which can bhe handled independently or chaines via decouple/
>>
>> s/chaines/chains/
>
> Fixed in v4.
>
>> > +decouple_status signals.
>> > +
>> > +Required properties:
>> > +- compatible : Should contain "xlnx,pr-decoupler-1.00"
>> > +- regs : base address and size for decoupler module
>> > +- clocks : input clock to IP
>> > +- clock-names : should contain "aclk"
>> > +
>> > +Optional properties:
>> > +- bridge-enable : 0 if driver should disable bridge at startup
>> > + 1 if driver should enable bridge at startup
>> > + Default is to leave bridge in current state.
>>
>> This is common and should move into a common doc. Maybe fpga-region.txt
>> works?
>
> Ok will add patch for that to v5 series.
Arg, our emails criss-crossed. I've already sent v4 to Greg. I hope
we don't need v5 for this one thing. bridge-enable is common for the
fpga bridges (altera-fpga2sdram-bridge.txt, altera-freeze-bridge.txt,
altera-hps2fpga-bridge.txt, xilinx-pr-decoupler.txt). Probably we
need a new patch to move this common bridges binding from all the
above to fpga-region.txt or create a new fpga-bridges.txt. At first
blush, I prefer the later.
Alan
>
> Thanks for your feedback,
>
> Moritz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 3/9] arm64: mm: install SError abort handler
From: Mark Rutland @ 2017-03-24 17:35 UTC (permalink / raw)
To: Doug Berger
Cc: wangkefeng.wang, catalin.marinas, linus.walleij, will.deacon,
mingo, panand, f.fainelli, jonathanh, bgolaszewski,
bcm-kernel-feedback-list, shijie.huang, treding, devicetree,
vladimir.murzin, cmetcalf, suzuki.poulose, andre.przywara,
robh+dt, gregory.0xf0, sandeepa.s.prabhu, horms+renesas,
linux-arm-kernel, mirza.krak, linux-kernel, james.morse, olof,
computersforpeace
In-Reply-To: <58D54DE8.9020707@gmail.com>
On Fri, Mar 24, 2017 at 09:48:40AM -0700, Doug Berger wrote:
> On 03/24/2017 08:16 AM, Mark Rutland wrote:
> >On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
> If you would consider an alternative implementation where we scrap
> the SError handler (i.e. maintain the ugliness in our downstream
> kernel) in favor of a more gentle user mode crash on SError that
> allows the kernel the opportunity to service the interrupt for
> diagnostic purposes I could try to repackage that.
If this is just for diagnostic purposes, I believe you can register a
panic notifier, which can then read from the bus. The panic will occur,
but you'll have the opportunity to log some information to dmesg.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 3/9] arm64: mm: install SError abort handler
From: Florian Fainelli @ 2017-03-24 17:53 UTC (permalink / raw)
To: Mark Rutland, Doug Berger
Cc: wangkefeng.wang, catalin.marinas, linus.walleij, will.deacon,
mingo, panand, f.fainelli, jonathanh, bgolaszewski,
bcm-kernel-feedback-list, shijie.huang, treding, devicetree,
vladimir.murzin, cmetcalf, suzuki.poulose, andre.przywara,
robh+dt, gregory.0xf0, sandeepa.s.prabhu, horms+renesas,
linux-arm-kernel, mirza.krak, linux-kernel, james.morse, olof,
computersforpeace
In-Reply-To: <20170324173515.GB10746@leverpostej>
On 03/24/2017 10:35 AM, Mark Rutland wrote:
> On Fri, Mar 24, 2017 at 09:48:40AM -0700, Doug Berger wrote:
>> On 03/24/2017 08:16 AM, Mark Rutland wrote:
>>> On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
>
>> If you would consider an alternative implementation where we scrap
>> the SError handler (i.e. maintain the ugliness in our downstream
>> kernel) in favor of a more gentle user mode crash on SError that
>> allows the kernel the opportunity to service the interrupt for
>> diagnostic purposes I could try to repackage that.
>
> If this is just for diagnostic purposes, I believe you can register a
> panic notifier, which can then read from the bus. The panic will occur,
> but you'll have the opportunity to log some information to dmesg.
And crash the kernel? That sounds awful, FWIW the ARM/Linux kernel is
able to recover just fine from user-space accessing e.g: invalid
physical addresses in the GISB register space, bringing the same level
of functionality to ARM64/Linux sounds reasonable to me.
--
Florian
^ permalink raw reply
* Re: [PATCH 4/5] firmware: Add coreboot device tree binding documentation
From: Brian Norris @ 2017-03-24 17:57 UTC (permalink / raw)
To: Mark Rutland
Cc: Thierry Escande, Rob Herring, Greg Kroah-Hartman, Olof Johansson,
Stephen Warren, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Julius Werner
In-Reply-To: <20170324122146.GE22771@leverpostej>
Hi Mark,
Julius can correct me if I'm wrong on any of this, but a quick note that
probably should have prefaced this:
These bindings were submitted years go
https://patchwork.ozlabs.org/patch/359672/
[PATCH] firmware: Add device tree binding for coreboot
discussed, and resubmitted seemingly according to several reviewers'
suggestions here
https://patchwork.kernel.org/patch/4362661/
[PATCH v2] firmware: Add device tree binding for coreboot
after which point we heard no response.
So these bindings have been baked into our coreboot firmware ever since
and are impossible to change on existing devices. It's *possible* we
could make modifications for new products, but it's really a non-starter
for this binding to be rejected at this point. (Or at least, if you want
this stuff to work in mainline for any Chromebook made in the last
several years.) If it's rejected, Chromebooks will just have to continue
shipping with forked drivers in our downstream tree.
Again, IMO that doesn't mean that this binding can't be improved or
augmented, but I think it's worth very serious consideration that the
legacy binding be considered and documented, even if it's noted as
"deprecated" or something like that.
On Fri, Mar 24, 2017 at 12:21:46PM +0000, Mark Rutland wrote:
> On Thu, Mar 23, 2017 at 10:04:28PM +0100, Thierry Escande wrote:
> > From: Julius Werner <jwerner-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >
> > This patch adds documentation describing a device tree binding for the
> > coreboot firmware. It is meant to be dynamically added during boot and
> > contains address definitions for the coreboot table (a list of
> > variable-sized descriptors providing information about various compile-
> > and run-time generated firmware parameters) and the CBMEM area (the
> > structure containing most run-time resident memory regions set up by
> > coreboot).
> >
> > These definitions allow kernel drivers to easily access data contained
> > in and pointed to by these regions (such as coreboot's in-memory log).
> > (An example implementation can be seen in the following patch)
> >
> > Signed-off-by: Julius Werner <jwerner-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > Signed-off-by: Thierry Escande <thierry.escande-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> > ---
> > .../devicetree/bindings/firmware/coreboot.txt | 33 ++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/firmware/coreboot.txt
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/coreboot.txt b/Documentation/devicetree/bindings/firmware/coreboot.txt
> > new file mode 100644
> > index 0000000..4c95570
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/coreboot.txt
> > @@ -0,0 +1,33 @@
> > +COREBOOT firmware information
> > +
> > +The device tree node to communicate the location of coreboot's memory-resident
> > +bookkeeping structures to the kernel. Since coreboot itself cannot boot a
> > +device-tree-based kernel (yet), this node needs to be inserted by a
> > +second-stage bootloader (a coreboot "payload").
> > +
> > +Required properties:
> > + - compatible: Should be "coreboot"
>
> Devicetree bindings should be in vendor,prefix format. This doesn't
> represent every aspect of coreboot, so it needs a more descriptive
> string.
Any particular suggestion? I suppose this is Google's interpretation of
coreboot tables, so "google,coreboot"?
> > + - reg: Address and length of the following two memory regions, in order:
> > + 1.) The coreboot table. This is a list of variable-sized descriptors
> > + that contain various compile- and run-time generated firmware
> > + parameters. It is identified by the magic string "LBIO" in its first
> > + four bytes.
> > + See coreboot's src/commonlib/include/commonlib/coreboot_tables.h for
> > + details.
>
> Given this is a memory region, it should be described under the
> reserved-memory node.
We've painted this bikeshed before. I guess the result was either to use
/reserved-memory or /memreserve/. I believe we've been using
/memreserve/. I suppose we could document a method to use either, but
the main "agreement" was to use the /firmware/coreboot path instead of
/reserved-memory/coreboot.
(And yes, I suppose /memreserve/ should be noted here.)
Brian
> > + 2.) The CBMEM area. This is a downward-growing memory region used by
> > + coreboot to dynamically allocate data structures that remain resident.
> > + It may or may not include the coreboot table as one of its members. It
> > + is identified by a root node descriptor with the magic number
> > + 0xc0389481 that resides in the topmost 8 bytes of the area.
> > + See coreboot's src/include/imd.h for details.
>
> I beleive likewise here.
>
> Thanks,
> Mark.
>
> > +
> > +Example:
> > + firmware {
> > + ranges;
> > +
> > + coreboot {
> > + compatible = "coreboot";
> > + reg = <0xfdfea000 0x264>,
> > + <0xfdfea000 0x16000>;
> > + }
> > + };
> > --
> > 2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/6] regulator: helpers: Add regmap set_pull_down helper
From: kbuild test robot @ 2017-03-24 17:59 UTC (permalink / raw)
To: Charles Keepax
Cc: mark.rutland, devicetree, alsa-devel, patches, lgirdwood, robh+dt,
broonie, kbuild-all, lee.jones
In-Reply-To: <1490107539-23995-2-git-send-email-ckeepax@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 11391 bytes --]
Hi Charles,
[auto build test WARNING on asoc/for-next]
[also build test WARNING on v4.11-rc3 next-20170324]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Charles-Keepax/regulator-helpers-Add-regmap-set_soft_start-helper/20170324-223205
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/init.h:1: warning: no structured comments found
kernel/sched/core.c:2085: warning: No description found for parameter 'rf'
kernel/sched/core.c:2085: warning: Excess function parameter 'cookie' description in 'try_to_wake_up_local'
include/linux/kthread.h:26: warning: Excess function parameter '...' description in 'kthread_create'
kernel/sys.c:1: warning: no structured comments found
include/linux/device.h:969: warning: No description found for parameter 'dma_ops'
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
include/linux/iio/iio.h:597: warning: No description found for parameter 'trig_readonly'
include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev'
include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig'
include/linux/device.h:970: warning: No description found for parameter 'dma_ops'
include/linux/regulator/driver.h:361: warning: No description found for parameter 'soft_start_reg'
include/linux/regulator/driver.h:361: warning: No description found for parameter 'soft_start_mask'
include/linux/regulator/driver.h:361: warning: No description found for parameter 'soft_start_val_on'
>> include/linux/regulator/driver.h:361: warning: No description found for parameter 'pull_down_reg'
>> include/linux/regulator/driver.h:361: warning: No description found for parameter 'pull_down_mask'
>> include/linux/regulator/driver.h:361: warning: No description found for parameter 'pull_down_val_on'
drivers/regulator/core.c:1467: warning: Excess function parameter 'ret' description in 'regulator_dev_lookup'
include/drm/drm_drv.h:438: warning: No description found for parameter 'open'
include/drm/drm_drv.h:438: warning: No description found for parameter 'preclose'
include/drm/drm_drv.h:438: warning: No description found for parameter 'postclose'
include/drm/drm_drv.h:438: warning: No description found for parameter 'lastclose'
include/drm/drm_drv.h:438: warning: No description found for parameter 'set_busid'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_handler'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_preinstall'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_postinstall'
include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_uninstall'
include/drm/drm_drv.h:438: warning: No description found for parameter 'debugfs_init'
include/drm/drm_drv.h:438: warning: No description found for parameter 'debugfs_cleanup'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_open_object'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_close_object'
include/drm/drm_drv.h:438: warning: No description found for parameter 'prime_handle_to_fd'
include/drm/drm_drv.h:438: warning: No description found for parameter 'prime_fd_to_handle'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_export'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_import'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_pin'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_unpin'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_res_obj'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_get_sg_table'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_import_sg_table'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_vmap'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_vunmap'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_mmap'
include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_vm_ops'
include/drm/drm_drv.h:438: warning: No description found for parameter 'major'
include/drm/drm_drv.h:438: warning: No description found for parameter 'minor'
include/drm/drm_drv.h:438: warning: No description found for parameter 'patchlevel'
include/drm/drm_drv.h:438: warning: No description found for parameter 'name'
include/drm/drm_drv.h:438: warning: No description found for parameter 'desc'
include/drm/drm_drv.h:438: warning: No description found for parameter 'date'
include/drm/drm_drv.h:438: warning: No description found for parameter 'driver_features'
include/drm/drm_drv.h:438: warning: No description found for parameter 'ioctls'
include/drm/drm_drv.h:438: warning: No description found for parameter 'num_ioctls'
include/drm/drm_drv.h:438: warning: No description found for parameter 'fops'
include/drm/drm_color_mgmt.h:1: warning: no structured comments found
drivers/gpu/drm/drm_fb_cma_helper.c:557: warning: Excess function parameter 'num_crtc' description in 'drm_fbdev_cma_init'
drivers/gpu/drm/drm_fb_cma_helper.c:558: warning: Excess function parameter 'num_crtc' description in 'drm_fbdev_cma_init'
drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'pipe'
drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'dp_output'
drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'link_rate'
drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'pipe'
drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'dp_output'
drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'link_rate'
drivers/media/dvb-core/dvb_frontend.h:677: warning: No description found for parameter 'refcount'
Documentation/core-api/assoc_array.rst:13: WARNING: Enumerated list ends without a blank line; unexpected unindent.
Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown target name: "sphinx c domain".
kernel/sched/fair.c:7616: WARNING: Inline emphasis start-string without end-string.
kernel/time/timer.c:1200: ERROR: Unexpected indentation.
kernel/time/timer.c:1202: ERROR: Unexpected indentation.
kernel/time/timer.c:1203: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:122: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:125: ERROR: Unexpected indentation.
include/linux/wait.h:127: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/time/hrtimer.c:990: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/signal.c:322: WARNING: Inline literal start-string without end-string.
include/linux/iio/iio.h:219: ERROR: Unexpected indentation.
include/linux/iio/iio.h:220: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/iio/iio.h:226: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/iio/industrialio-core.c:639: ERROR: Unknown target name: "iio_val".
drivers/iio/industrialio-core.c:646: ERROR: Unknown target name: "iio_val".
drivers/message/fusion/mptbase.c:5051: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/tty/serial/serial_core.c:1898: WARNING: Definition list ends without a blank line; unexpected unindent.
include/linux/regulator/driver.h:271: ERROR: Unknown target name: "regulator_regmap_x_voltage".
include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
drivers/usb/core/message.c:478: ERROR: Unexpected indentation.
drivers/usb/core/message.c:479: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_type".
Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_dir".
Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_recip".
Documentation/driver-api/usb.rst:689: ERROR: Unknown target name: "usbdevfs_urb_type".
sound/soc/soc-core.c:2670: ERROR: Unknown target name: "snd_soc_daifmt".
sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn".
WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the imgmath_dvipng setting
vim +/pull_down_reg +361 include/linux/regulator/driver.h
354794da Laxman Dewangan 2016-03-02 345 unsigned int active_discharge_off;
354794da Laxman Dewangan 2016-03-02 346 unsigned int active_discharge_mask;
354794da Laxman Dewangan 2016-03-02 347 unsigned int active_discharge_reg;
8b45dea6 Charles Keepax 2017-03-21 348 unsigned int soft_start_reg;
8b45dea6 Charles Keepax 2017-03-21 349 unsigned int soft_start_mask;
8b45dea6 Charles Keepax 2017-03-21 350 unsigned int soft_start_val_on;
ce32c8a0 Charles Keepax 2017-03-21 351 unsigned int pull_down_reg;
ce32c8a0 Charles Keepax 2017-03-21 352 unsigned int pull_down_mask;
ce32c8a0 Charles Keepax 2017-03-21 353 unsigned int pull_down_val_on;
79511ed3 Mark Brown 2012-06-27 354
79511ed3 Mark Brown 2012-06-27 355 unsigned int enable_time;
871f5650 Guodong Xu 2014-08-13 356
871f5650 Guodong Xu 2014-08-13 357 unsigned int off_on_delay;
87e1e0f2 Javier Martinez Canillas 2014-11-10 358
87e1e0f2 Javier Martinez Canillas 2014-11-10 359 unsigned int (*of_map_mode)(unsigned int mode);
571a354b Liam Girdwood 2008-04-30 360 };
571a354b Liam Girdwood 2008-04-30 @361
c172708d Mark Brown 2012-04-04 362 /**
c172708d Mark Brown 2012-04-04 363 * struct regulator_config - Dynamic regulator descriptor
c172708d Mark Brown 2012-04-04 364 *
c172708d Mark Brown 2012-04-04 365 * Each regulator registered with the core is described with a
c172708d Mark Brown 2012-04-04 366 * structure of this type and a struct regulator_desc. This structure
c172708d Mark Brown 2012-04-04 367 * contains the runtime variable parts of the regulator description.
c172708d Mark Brown 2012-04-04 368 *
c172708d Mark Brown 2012-04-04 369 * @dev: struct device for the regulator
:::::: The code at line 361 was first introduced by commit
:::::: 571a354b1542a274d88617e1f6703f3fe7a517f1 regulator: regulator driver interface
:::::: TO: Liam Girdwood <lg@opensource.wolfsonmicro.com>
:::::: CC: Liam Girdwood <lg@opensource.wolfsonmicro.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6576 bytes --]
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support
From: Brian Norris @ 2017-03-24 18:00 UTC (permalink / raw)
To: Mark Rutland
Cc: Thierry Escande, Rob Herring, Greg Kroah-Hartman, Olof Johansson,
Stephen Warren, linux-kernel, devicetree, Julius Werner
In-Reply-To: <20170324122858.GF22771@leverpostej>
On Fri, Mar 24, 2017 at 12:28:59PM +0000, Mark Rutland wrote:
> On Thu, Mar 23, 2017 at 10:04:29PM +0100, Thierry Escande wrote:
> > +static const struct of_device_id coreboot_of_match[] = {
> > + { .compatible = "coreboot" },
> > + {},
> > +};
> > +
> > +static struct platform_driver coreboot_table_of_driver = {
> > + .probe = coreboot_table_of_probe,
> > + .remove = coreboot_table_of_remove,
> > + .driver = {
> > + .name = "coreboot_table_of",
> > + .of_match_table = coreboot_of_match,
> > + },
> > +};
> > +
> > +static int __init platform_coreboot_table_of_init(void)
> > +{
> > + struct platform_device *pdev;
> > + struct device_node *of_node;
> > +
> > + /* Limit device creation to the presence of /firmware/coreboot node */
> > + of_node = of_find_node_by_path("/firmware/coreboot");
> > + if (!of_node)
> > + return -ENODEV;
> > +
>
> I don't beleive that you need this module init function. Please use the
> usual DT probing infrastrucutre instead, e.g. add:
>
> MODULE_DEVICE_TABLE(of, coreboot_of_match);
> module_platform_driver(coreboot_table_of_driver);
That doesn't work. If this node is placed under /firmware, which isn't a
proper "bus", then we have to explicitly look for the sub-device.
Due to this, the MODULE_DEVICE_TABLE() also isn't useful, because the
/firmware/coreboot/ device won't be generated automatically, and so no
matching uevent will occur.
Brian
^ permalink raw reply
* Re: [PATCH v3 1/2] doc: Add bindings document for Xilinx LogiCore PR Decoupler
From: Moritz Fischer @ 2017-03-24 18:07 UTC (permalink / raw)
To: Alan Tull
Cc: Mark Rutland, Rob Herring, devicetree@vger.kernel.org,
Greg Kroah-Hartman, linux-fpga, Michal Simek, linux-kernel,
Moritz Fischer, Sören Brinkmann,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CANk1AXSHimnjsMsCJwbtwrkRppFCRxY7KQemFikgu+hqCTiO5Q@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3577 bytes --]
On Fri, Mar 24, 2017 at 12:25:25PM -0500, Alan Tull wrote:
> On Fri, Mar 24, 2017 at 10:23 AM, Moritz Fischer
> <moritz.fischer.private@gmail.com> wrote:
> > On Fri, Mar 24, 2017 at 09:59:08AM -0500, Rob Herring wrote:
> >> On Fri, Mar 17, 2017 at 01:11:12PM -0700, Moritz Fischer wrote:
> >> > This adds the binding documentation for the Xilinx LogiCORE PR
> >> > Decoupler soft core.
> >> >
> >> > Signed-off-by: Moritz Fischer <mdf@kernel.org>
> >> > Cc: Michal Simek <michal.simek@xilinx.com>
> >> > Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
> >> > Cc: linux-kernel@vger.kernel.org
> >> > Cc: devicetree@vger.kernel.org
> >> > ---
> >> >
> >> > Changes from v2:
> >> > - Added refence to generic fpga-region bindings
> >> > - Fixed up reg property in example
> >> > - Added fallback to "xlnx,pr-decoupler" without version
> >> >
> >> > Changes from v1:
> >> > - Added clock names & clock to example
> >> > - Merged some of the description from Michal's version
> >> >
> >> > ---
> >> > .../bindings/fpga/xilinx-pr-decoupler.txt | 35 ++++++++++++++++++++++
> >> > 1 file changed, 35 insertions(+)
> >> > create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
> >> > new file mode 100644
> >> > index 0000000..16141bd
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
> >> > @@ -0,0 +1,35 @@
> >> > +Xilinx LogiCORE Partial Reconfig Decoupler Softcore
> >> > +
> >> > +The Xilinx LogiCORE Partial Reconfig Decoupler manages one or more
> >> > +decouplers / fpga bridges.
> >> > +The controller can decouple/disable the bridges which prevents signal
> >> > +changes from passing through the bridge. The controller can also
> >> > +couple / enable the bridges which allows traffic to pass through the
> >> > +bridge normally.
> >> > +
> >> > +The Driver supports only MMIO handling. A PR region can have multiple
> >> > +PR Decouples which can bhe handled independently or chaines via decouple/
> >>
> >> s/chaines/chains/
> >
> > Fixed in v4.
> >
> >> > +decouple_status signals.
> >> > +
> >> > +Required properties:
> >> > +- compatible : Should contain "xlnx,pr-decoupler-1.00"
> >> > +- regs : base address and size for decoupler module
> >> > +- clocks : input clock to IP
> >> > +- clock-names : should contain "aclk"
> >> > +
> >> > +Optional properties:
> >> > +- bridge-enable : 0 if driver should disable bridge at startup
> >> > + 1 if driver should enable bridge at startup
> >> > + Default is to leave bridge in current state.
> >>
> >> This is common and should move into a common doc. Maybe fpga-region.txt
> >> works?
> >
> > Ok will add patch for that to v5 series.
>
> Arg, our emails criss-crossed. I've already sent v4 to Greg. I hope
> we don't need v5 for this one thing. bridge-enable is common for the
> fpga bridges (altera-fpga2sdram-bridge.txt, altera-freeze-bridge.txt,
> altera-hps2fpga-bridge.txt, xilinx-pr-decoupler.txt). Probably we
> need a new patch to move this common bridges binding from all the
> above to fpga-region.txt or create a new fpga-bridges.txt. At first
> blush, I prefer the later.
Yeah, I'll just send a follow-up patch, it's not that it breaks anything
the way it is.
- Moritz
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 3/7] coresight: refactor with function of_coresight_get_cpu
From: Stephen Boyd @ 2017-03-24 18:14 UTC (permalink / raw)
To: Leo Yan
Cc: Rob Herring, Mark Rutland, Wei Xu, Catalin Marinas, Will Deacon,
Andy Gross, David Brown, Michael Turquette, Mathieu Poirier,
Guodong Xu, John Stultz, Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
mike.leach-QSEj5FYQhm4dnm+yROfE0A, Suzuki.Poulose-5wv7dgnIgG8,
sudeep.holla-5wv7dgnIgG8
In-Reply-To: <1489762943-25849-4-git-send-email-leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 03/17, Leo Yan wrote:
> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> index 917ca39..7a60f2b 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -101,15 +101,38 @@ static int of_coresight_alloc_memory(struct device *dev,
> return 0;
> }
>
> +int of_coresight_get_cpu(struct device_node *node)
const node?
> +{
> + int cpu;
> + bool found;
> + struct device_node *dn, *np;
> +
> + dn = of_parse_phandle(node, "cpu", 0);
> +
> + /* Affinity defaults to CPU0 */
> + if (!dn)
> + return 0;
> +
> + for_each_possible_cpu(cpu) {
> + np = of_get_cpu_node(cpu, NULL);
of_cpu_device_node_get()?
> + found = (dn == np);
> + of_node_put(np);
> + if (found)
> + break;
> + }
> + of_node_put(dn);
> +
> + /* Affinity to CPU0 if no cpu nodes are found */
> + return found ? cpu : 0;
> +}
> +
> struct coresight_platform_data *of_get_coresight_platform_data(
> struct device *dev, struct device_node *node)
> {
> - int i = 0, ret = 0, cpu;
> + int i = 0, ret = 0;
> struct coresight_platform_data *pdata;
> struct of_endpoint endpoint, rendpoint;
> struct device *rdev;
> - bool found;
> - struct device_node *dn, *np;
> struct device_node *ep = NULL;
> struct device_node *rparent = NULL;
> struct device_node *rport = NULL;
> @@ -176,18 +199,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
> } while (ep);
> }
>
> - dn = of_parse_phandle(node, "cpu", 0);
> - for_each_possible_cpu(cpu) {
> - np = of_get_cpu_node(cpu, NULL);
> - found = (dn == np);
> - of_node_put(np);
> - if (found)
> - break;
> - }
> - of_node_put(dn);
> -
Ok yeah the code is moving, so perhaps another patch to make the
changes suggested above.
> - /* Affinity to CPU0 if no cpu nodes are found */
> - pdata->cpu = found ? cpu : 0;
> + pdata->cpu = of_coresight_get_cpu(node);
>
> return pdata;
> }
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 2a5982c..7b29743 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -263,9 +263,11 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
> #endif
>
> #ifdef CONFIG_OF
> +extern int of_coresight_get_cpu(struct device_node *node);
> extern struct coresight_platform_data *of_get_coresight_platform_data(
> struct device *dev, struct device_node *node);
> #else
> +static int of_coresight_get_cpu(struct device_node *node) { return 0; }
inline?
> static inline struct coresight_platform_data *of_get_coresight_platform_data(
> struct device *dev, struct device_node *node) { return NULL; }
> #endif
> --
> 2.7.4
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH linux v5 0/2] Support for ASPEED AST2400/AST2500 PWM and Fan Tach driver
From: Jaghathiswari Rankappagounder Natarajan @ 2017-03-24 18:17 UTC (permalink / raw)
To: joel, jdelvare, linux, linux-hwmon, linux-kernel, openbmc, corbet,
linux-doc, robh+dt, mark.rutland, devicetree
Cc: Jaghathiswari Rankappagounder Natarajan
Support for ASPEED AST2400/AST2500 PWM and Fan Tach driver.
Patches based on the upstream tag 4.9. Changes made in Version 4 are indicated
in the individual patches.
The AST2400/AST2500 PWM controller can support 8 PWM output ports.
The AST2400/AST2500 Fan Tach controller can support 16 tachometer inputs.
The hwmon driver provides sysfs entries through which the user can configure the
duty cycle for the particular PWM output port and read the fan rpm value for
the particular tachometer input.
Added devicetree binding documentation for AST2400/AST2500 PWM and Fan Tach
support.
Tested on Zaius board (which has AST2500 chip) and observed that when
duty cycle is lowered then the fan speed is lowered and lower fan rpm value(
corresponding to the duty cycle) is reported and when the duty cycle is
increased then the fan speed increases and higher fan rpm value(corresponding
to the duty cycle) is reported.
Jaghathiswari Rankappagounder Natarajan (2):
Documentation: dt-bindings: Document bindings for ASPEED
AST2400/AST2500 PWM and Fan tach controller device driver
drivers: hwmon: Support for ASPEED PWM/Fan tach
.../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 68 ++
Documentation/hwmon/aspeed-pwm-tacho | 22 +
drivers/hwmon/Kconfig | 13 +-
drivers/hwmon/Makefile | 1 +
drivers/hwmon/aspeed-pwm-tacho.c | 854 +++++++++++++++++++++
5 files changed, 956 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
create mode 100644 Documentation/hwmon/aspeed-pwm-tacho
create mode 100644 drivers/hwmon/aspeed-pwm-tacho.c
--
2.12.1.578.ge9c3154ca4-goog
^ permalink raw reply
* [PATCH linux v5 1/2] Documentation: dt-bindings: Document bindings for ASPEED AST2400/AST2500 PWM and Fan tach controller device driver
From: Jaghathiswari Rankappagounder Natarajan @ 2017-03-24 18:17 UTC (permalink / raw)
To: joel-U3u1mxZcP9KHXe+LvDLADg, jdelvare-IBi9RG/b67k,
linux-0h96xk9xTtrk1uMJSBkQmQ, linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, corbet-T1hC0tSOHrs,
linux-doc-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Jaghathiswari Rankappagounder Natarajan
In-Reply-To: <20170324181723.29379-1-jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
This binding provides interface for adding values related to ASPEED
AST2400/2500 PWM and Fan tach controller support.
The PWM controller can support upto 8 PWM output ports.
The Fan tach controller can support upto 16 tachometer inputs.
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
v5:
- Changed the naming scheme to aspeed,ast2400/2500-pwm-tacho
- Removed gpio pin muxing
- Added aspeed vendor prefix for fan-tach-ch
- Changed to fan@0/1
- Changed reg to 32 bits
v4:
- Used 'reg'
v3:
- Made the structure more common
v2:
- Removed '_' in node and property names
- Gave some explanation for the properties used
.../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 68 ++++++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
new file mode 100644
index 000000000000..f96d32bfcdfa
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
@@ -0,0 +1,68 @@
+ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver
+
+The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho
+controller can support upto 16 Fan tachometer inputs.
+
+There can be upto 8 fans supported. Each fan can have one PWM output and
+one/two Fan tach inputs.
+
+Required properties for pwm-tacho node:
+- #address-cells : should be 1.
+
+- #size-cells : should be 1.
+
+- reg : address and length of the register set for the device.
+
+- pinctrl-names : a pinctrl state named "default" must be defined.
+
+- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
+
+- compatible : should be "aspeed,ast2400-pwm-tacho" for AST2400 and
+ "aspeed,ast2500-pwm-tacho" for AST2500.
+
+- clocks : a fixed clock providing input clock frequency(PWM
+ and Fan Tach clock)
+
+fan subnode format:
+===================
+Under fan subnode there can upto 8 child nodes, with each child node
+representing a fan. If there are 8 fans each fan can have one PWM port and
+one/two Fan tach inputs.
+
+Required properties for each child node:
+- reg : should specify PWM source port.
+ integer value in the range 0 to 7 with 0 indicating PWM port A and
+ 7 indicating PWM port H.
+
+- aspeed-fan-tach-ch : should specify the Fan tach input channel.
+ integer value in the range 0 through 15, with 0 indicating
+ Fan tach channel 0 and 15 indicating Fan tach channel 15.
+ Atleast one Fan tach input channel is required.
+
+Examples:
+
+pwm_tacho_fixed_clk: fixedclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24000000>;
+}
+
+pwm_tacho: pwmtachocontroller@1e786000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x1E786000 0x1000>;
+ compatible = "aspeed,ast2500-pwm-tacho";
+ clocks = <&pwm_tacho_fixed_clk>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed-fan-tach-ch = /bits/ 8 <0x00>;
+ };
+
+ fan@1 {
+ reg = <0x01>;
+ aspeed-fan-tach-ch = /bits/ 8 <0x01 0x02>;
+ };
+};
--
2.12.1.578.ge9c3154ca4-goog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH linux v5 2/2] drivers: hwmon: Support for ASPEED PWM/Fan tach
From: Jaghathiswari Rankappagounder Natarajan @ 2017-03-24 18:17 UTC (permalink / raw)
To: joel, jdelvare, linux, linux-hwmon, linux-kernel, openbmc, corbet,
linux-doc, robh+dt, mark.rutland, devicetree
Cc: Jaghathiswari Rankappagounder Natarajan
In-Reply-To: <20170324181723.29379-1-jaghu@google.com>
The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
The ASPEED AST2400/2500 Fan tach controller supports 16 tachometer
inputs.
The device driver matches on the device tree node. The configuration
values
are read from the device tree and written to the respective registers.
The driver provides a sysfs entries through which the user can
configure the duty-cycle value (ranging from 0 to 100 percent) and read
the
fan tach rpm value.
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
v5:
- Changed the driver to suit the changes in the device tree documentation
v4:
- Modified this driver to suit the representation in the devicetree
v3:
- Only sent out device tree documentation; did not send this driver
v2:
- Used BIT()
- Used regmap
- Avoided division when raw data is 0
- Removed empty lines between declaration
- Removed macros; Used two attribute groups and used is_visible callback
- Returned error when properties are undefined
- Removed .owner field
- Used PTR_ERR_OR_ZERO
- Removed explicit of_node_put for child nodes
Documentation/hwmon/aspeed-pwm-tacho | 22 +
drivers/hwmon/Kconfig | 13 +-
drivers/hwmon/Makefile | 1 +
drivers/hwmon/aspeed-pwm-tacho.c | 854 +++++++++++++++++++++++++++++++++++
4 files changed, 888 insertions(+), 2 deletions(-)
create mode 100644 Documentation/hwmon/aspeed-pwm-tacho
create mode 100644 drivers/hwmon/aspeed-pwm-tacho.c
diff --git a/Documentation/hwmon/aspeed-pwm-tacho b/Documentation/hwmon/aspeed-pwm-tacho
new file mode 100644
index 000000000000..0e9ec6d5f900
--- /dev/null
+++ b/Documentation/hwmon/aspeed-pwm-tacho
@@ -0,0 +1,22 @@
+Kernel driver aspeed-pwm-tacho
+==============================
+
+Supported chips:
+ ASPEED AST2400/2500
+
+Authors:
+ <jaghu@google.com>
+
+Description:
+------------
+This driver implements support for ASPEED AST2400/2500 PWM and Fan Tacho
+controller. The PWM controller supports upto 8 PWM outputs. The Fan tacho
+controller supports upto 16 tachometer inputs.
+
+The driver provides the following sensor accesses in sysfs:
+
+fanX_input ro provide current fan rotation value in RPM as reported
+ by the fan to the device.
+
+pwmX rw get or set PWM fan control value. This is an integer
+ value between 0(off) and 255(full speed).
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 45cef3d2c75c..487110f21827 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -341,6 +341,15 @@ config SENSORS_ASB100
This driver can also be built as a module. If so, the module
will be called asb100.
+config SENSORS_ASPEED
+ tristate "ASPEED AST2400/AST2500 PWM and Fan tach driver"
+ help
+ This driver provides support for ASPEED AST2400/AST2500 PWM
+ and Fan Tacho controllers.
+
+ This driver can also be built as a module. If so, the module
+ will be called aspeed_pwm_tacho.
+
config SENSORS_ATXP1
tristate "Attansic ATXP1 VID controller"
depends on I2C
@@ -952,7 +961,7 @@ config SENSORS_LM70
help
If you say yes here you get support for the National Semiconductor
LM70, LM71, LM74 and Texas Instruments TMP121/TMP123 digital tempera-
- ture sensor chips.
+ true sensor chips.
This driver can also be built as a module. If so, the module
will be called lm70.
@@ -1506,7 +1515,7 @@ config SENSORS_ADS7871
config SENSORS_AMC6821
tristate "Texas Instruments AMC6821"
- depends on I2C
+ depends on I2C
help
If you say yes here you get support for the Texas Instruments
AMC6821 hardware monitoring chips.
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index aecf4ba17460..83025cc9bb45 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
+obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
new file mode 100644
index 000000000000..28888cc34555
--- /dev/null
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -0,0 +1,854 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+#include <linux/regmap.h>
+
+/* ASPEED PWM & FAN Tach Register Definition */
+#define ASPEED_PTCR_CTRL 0x00
+#define ASPEED_PTCR_CLK_CTRL 0x04
+#define ASPEED_PTCR_DUTY0_CTRL 0x08
+#define ASPEED_PTCR_DUTY1_CTRL 0x0c
+#define ASPEED_PTCR_TYPEM_CTRL 0x10
+#define ASPEED_PTCR_TYPEM_CTRL1 0x14
+#define ASPEED_PTCR_TYPEN_CTRL 0x18
+#define ASPEED_PTCR_TYPEN_CTRL1 0x1c
+#define ASPEED_PTCR_TACH_SOURCE 0x20
+#define ASPEED_PTCR_TRIGGER 0x28
+#define ASPEED_PTCR_RESULT 0x2c
+#define ASPEED_PTCR_INTR_CTRL 0x30
+#define ASPEED_PTCR_INTR_STS 0x34
+#define ASPEED_PTCR_TYPEM_LIMIT 0x38
+#define ASPEED_PTCR_TYPEN_LIMIT 0x3C
+#define ASPEED_PTCR_CTRL_EXT 0x40
+#define ASPEED_PTCR_CLK_CTRL_EXT 0x44
+#define ASPEED_PTCR_DUTY2_CTRL 0x48
+#define ASPEED_PTCR_DUTY3_CTRL 0x4c
+#define ASPEED_PTCR_TYPEO_CTRL 0x50
+#define ASPEED_PTCR_TYPEO_CTRL1 0x54
+#define ASPEED_PTCR_TACH_SOURCE_EXT 0x60
+#define ASPEED_PTCR_TYPEO_LIMIT 0x78
+
+/* ASPEED_PTCR_CTRL : 0x00 - General Control Register */
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1 15
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2 6
+#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK (BIT(7) | BIT(15))
+
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1 14
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2 5
+#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK (BIT(6) | BIT(14))
+
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1 13
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2 4
+#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK (BIT(5) | BIT(13))
+
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1 12
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2 3
+#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK (BIT(4) | BIT(12))
+
+#define ASPEED_PTCR_CTRL_FAN_NUM_EN(x) BIT(16 + (x))
+
+#define ASPEED_PTCR_CTRL_PWMD_EN BIT(11)
+#define ASPEED_PTCR_CTRL_PWMC_EN BIT(10)
+#define ASPEED_PTCR_CTRL_PWMB_EN BIT(9)
+#define ASPEED_PTCR_CTRL_PWMA_EN BIT(8)
+
+#define ASPEED_PTCR_CTRL_CLK_SRC BIT(1)
+#define ASPEED_PTCR_CTRL_CLK_EN BIT(0)
+
+/* ASPEED_PTCR_CLK_CTRL : 0x04 - Clock Control Register */
+/* TYPE N */
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_MASK GENMASK(31, 16)
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT 24
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_H 20
+#define ASPEED_PTCR_CLK_CTRL_TYPEN_L 16
+/* TYPE M */
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_MASK GENMASK(15, 0)
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT 8
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_H 4
+#define ASPEED_PTCR_CLK_CTRL_TYPEM_L 0
+
+/*
+ * ASPEED_PTCR_DUTY_CTRL/1/2/3 : 0x08/0x0C/0x48/0x4C - PWM-FAN duty control
+ * 0/1/2/3 register
+ */
+#define DUTY_CTRL_PWM2_FALL_POINT 24
+#define DUTY_CTRL_PWM2_RISE_POINT 16
+#define DUTY_CTRL_PWM2_RISE_FALL_MASK GENMASK(31, 16)
+#define DUTY_CTRL_PWM1_FALL_POINT 8
+#define DUTY_CTRL_PWM1_RISE_POINT 0
+#define DUTY_CTRL_PWM1_RISE_FALL_MASK GENMASK(15, 0)
+
+/* ASPEED_PTCR_TYPEM_CTRL : 0x10/0x18/0x50 - Type M/N/O Ctrl 0 Register */
+#define TYPE_CTRL_FAN_MASK (GENMASK(5, 1) | GENMASK(31, 16))
+#define TYPE_CTRL_FAN1_MASK GENMASK(31, 0)
+#define TYPE_CTRL_FAN_PERIOD 16
+#define TYPE_CTRL_FAN_MODE 4
+#define TYPE_CTRL_FAN_DIVISION 1
+#define TYPE_CTRL_FAN_TYPE_EN 1
+
+/* ASPEED_PTCR_TACH_SOURCE : 0x20/0x60 - Tach Source Register */
+/* bit [0,1] at 0x20, bit [2] at 0x60 */
+#define TACH_PWM_SOURCE_BIT01(x) ((x) * 2)
+#define TACH_PWM_SOURCE_BIT2(x) ((x) * 2)
+#define TACH_PWM_SOURCE_MASK_BIT01(x) (0x3 << ((x) * 2))
+#define TACH_PWM_SOURCE_MASK_BIT2(x) BIT((x) * 2)
+
+/* ASPEED_PTCR_RESULT : 0x2c - Result Register */
+#define RESULT_STATUS_MASK BIT(31)
+#define RESULT_VALUE_MASK 0xfffff
+
+/* ASPEED_PTCR_CTRL_EXT : 0x40 - General Control Extension #1 Register */
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1 15
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2 6
+#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK (BIT(7) | BIT(15))
+
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1 14
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2 5
+#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK (BIT(6) | BIT(14))
+
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1 13
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2 4
+#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK (BIT(5) | BIT(13))
+
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1 12
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2 3
+#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK (BIT(4) | BIT(12))
+
+#define ASPEED_PTCR_CTRL_PWMH_EN BIT(11)
+#define ASPEED_PTCR_CTRL_PWMG_EN BIT(10)
+#define ASPEED_PTCR_CTRL_PWMF_EN BIT(9)
+#define ASPEED_PTCR_CTRL_PWME_EN BIT(8)
+
+/* ASPEED_PTCR_CLK_EXT_CTRL : 0x44 - Clock Control Extension #1 Register */
+/* TYPE O */
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_MASK GENMASK(15, 0)
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT 8
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_H 4
+#define ASPEED_PTCR_CLK_CTRL_TYPEO_L 0
+
+#define PWM_MAX 255
+
+#define M_PWM_DIV_H 0x00
+#define M_PWM_DIV_L 0x05
+#define M_PWM_PERIOD 0x5F
+#define M_TACH_CLK_DIV 0x00
+#define M_TACH_MODE 0x00
+#define M_TACH_UNIT 0x1000
+#define INIT_FAN_CTRL 0xFF
+
+struct aspeed_pwm_tacho_regmap {
+ void __iomem *regs;
+};
+
+struct aspeed_pwm_tacho_data {
+ struct regmap *regmap;
+ unsigned long clk_freq;
+ bool pwm_present[8];
+ bool fan_tach_present[16];
+ u8 type_pwm_clock_unit[3];
+ u8 type_pwm_clock_division_h[3];
+ u8 type_pwm_clock_division_l[3];
+ u8 type_fan_tach_clock_division[3];
+ u16 type_fan_tach_unit[3];
+ u8 pwm_port_type[8];
+ u8 pwm_port_fan_ctrl[8];
+ u8 fan_tach_ch_source[16];
+ const struct attribute_group *groups[2];
+};
+
+enum type { TYPEM, TYPEN, TYPEO };
+
+struct type_params {
+ u32 l_value;
+ u32 h_value;
+ u32 unit_value;
+ u32 clk_ctrl_mask;
+ u32 clk_ctrl_reg;
+ u32 ctrl_reg;
+ u32 ctrl_reg1;
+};
+
+static const struct type_params type_params[] = {
+ [TYPEM] = {
+ .l_value = ASPEED_PTCR_CLK_CTRL_TYPEM_L,
+ .h_value = ASPEED_PTCR_CLK_CTRL_TYPEM_H,
+ .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT,
+ .clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_MASK,
+ .clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL,
+ .ctrl_reg = ASPEED_PTCR_TYPEM_CTRL,
+ .ctrl_reg1 = ASPEED_PTCR_TYPEM_CTRL1,
+ },
+ [TYPEN] = {
+ .l_value = ASPEED_PTCR_CLK_CTRL_TYPEN_L,
+ .h_value = ASPEED_PTCR_CLK_CTRL_TYPEN_H,
+ .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT,
+ .clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_MASK,
+ .clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL,
+ .ctrl_reg = ASPEED_PTCR_TYPEN_CTRL,
+ .ctrl_reg1 = ASPEED_PTCR_TYPEN_CTRL1,
+ },
+ [TYPEO] = {
+ .l_value = ASPEED_PTCR_CLK_CTRL_TYPEO_L,
+ .h_value = ASPEED_PTCR_CLK_CTRL_TYPEO_H,
+ .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT,
+ .clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_MASK,
+ .clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL_EXT,
+ .ctrl_reg = ASPEED_PTCR_TYPEO_CTRL,
+ .ctrl_reg1 = ASPEED_PTCR_TYPEO_CTRL1,
+ }
+};
+
+enum pwm_port { PWMA, PWMB, PWMC, PWMD, PWME, PWMF, PWMG, PWMH };
+
+struct pwm_port_params {
+ u32 pwm_en;
+ u32 ctrl_reg;
+ u32 type_part1;
+ u32 type_part2;
+ u32 type_mask;
+ u32 duty_ctrl_rise_point;
+ u32 duty_ctrl_fall_point;
+ u32 duty_ctrl_reg;
+ u32 duty_ctrl_rise_fall_mask;
+};
+
+static const struct pwm_port_params pwm_port_params[] = {
+ [PWMA] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMA_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY0_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+ },
+ [PWMB] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMB_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY0_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+ },
+ [PWMC] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMC_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY1_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+ },
+ [PWMD] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMD_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY1_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+ },
+ [PWME] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWME_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY2_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+ },
+ [PWMF] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMF_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY2_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+ },
+ [PWMG] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMG_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY3_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
+ },
+ [PWMH] = {
+ .pwm_en = ASPEED_PTCR_CTRL_PWMH_EN,
+ .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
+ .type_part1 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1,
+ .type_part2 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2,
+ .type_mask = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK,
+ .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
+ .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
+ .duty_ctrl_reg = ASPEED_PTCR_DUTY3_CTRL,
+ .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
+ }
+};
+
+static int regmap_aspeed_pwm_tacho_reg_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct aspeed_pwm_tacho_regmap *hregmap = context;
+
+ writel(val, hregmap->regs + reg);
+ return 0;
+}
+
+static int regmap_aspeed_pwm_tacho_reg_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct aspeed_pwm_tacho_regmap *hregmap = context;
+
+ *val = readl(hregmap->regs + reg);
+ return 0;
+}
+
+static const struct regmap_config aspeed_pwm_tacho_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = ASPEED_PTCR_TYPEO_LIMIT,
+ .reg_write = regmap_aspeed_pwm_tacho_reg_write,
+ .reg_read = regmap_aspeed_pwm_tacho_reg_read,
+ .fast_io = true,
+};
+
+static void aspeed_set_clock_enable(struct regmap *regmap, bool val)
+{
+ regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
+ ASPEED_PTCR_CTRL_CLK_EN,
+ val ? ASPEED_PTCR_CTRL_CLK_EN : 0);
+}
+
+static void aspeed_set_clock_source(struct regmap *regmap, int val)
+{
+ regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
+ ASPEED_PTCR_CTRL_CLK_SRC,
+ val ? ASPEED_PTCR_CTRL_CLK_SRC : 0);
+}
+
+static void aspeed_set_pwm_clock_values(struct regmap *regmap, u8 type,
+ u8 div_high, u8 div_low, u8 unit)
+{
+ u32 reg_value = ((div_high << type_params[type].h_value) |
+ (div_low << type_params[type].l_value) |
+ (unit << type_params[type].unit_value));
+
+ regmap_update_bits(regmap, type_params[type].clk_ctrl_reg,
+ type_params[type].clk_ctrl_mask, reg_value);
+}
+
+static void aspeed_set_pwm_port_enable(struct regmap *regmap, u8 pwm_port,
+ bool enable)
+{
+ regmap_update_bits(regmap, pwm_port_params[pwm_port].ctrl_reg,
+ pwm_port_params[pwm_port].pwm_en,
+ enable ? pwm_port_params[pwm_port].pwm_en : 0);
+}
+
+static void aspeed_set_pwm_port_type(struct regmap *regmap,
+ u8 pwm_port, u8 type)
+{
+ u32 reg_value = (type & 0x1) << pwm_port_params[pwm_port].type_part1;
+
+ reg_value |= (type & 0x2) << pwm_port_params[pwm_port].type_part2;
+
+ regmap_update_bits(regmap, pwm_port_params[pwm_port].ctrl_reg,
+ pwm_port_params[pwm_port].type_mask, reg_value);
+}
+
+static void aspeed_set_pwm_port_duty_rising_falling(struct regmap *regmap,
+ u8 pwm_port, u8 rising,
+ u8 falling)
+{
+ u32 reg_value = (rising <<
+ pwm_port_params[pwm_port].duty_ctrl_rise_point);
+ reg_value |= (falling <<
+ pwm_port_params[pwm_port].duty_ctrl_fall_point);
+
+ regmap_update_bits(regmap, pwm_port_params[pwm_port].duty_ctrl_reg,
+ pwm_port_params[pwm_port].duty_ctrl_rise_fall_mask,
+ reg_value);
+}
+
+static void aspeed_set_tacho_type_enable(struct regmap *regmap, u8 type,
+ bool enable)
+{
+ regmap_update_bits(regmap, type_params[type].ctrl_reg,
+ TYPE_CTRL_FAN_TYPE_EN,
+ enable ? TYPE_CTRL_FAN_TYPE_EN : 0);
+}
+
+static void aspeed_set_tacho_type_values(struct regmap *regmap, u8 type,
+ u8 mode, u16 unit, u8 division)
+{
+ u32 reg_value = ((mode << TYPE_CTRL_FAN_MODE) |
+ (unit << TYPE_CTRL_FAN_PERIOD) |
+ (division << TYPE_CTRL_FAN_DIVISION));
+
+ regmap_update_bits(regmap, type_params[type].ctrl_reg,
+ TYPE_CTRL_FAN_MASK, reg_value);
+ regmap_update_bits(regmap, type_params[type].ctrl_reg1,
+ TYPE_CTRL_FAN1_MASK, unit << 16);
+}
+
+static void aspeed_set_fan_tach_ch_enable(struct regmap *regmap, u8 fan_tach_ch,
+ bool enable)
+{
+ regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
+ ASPEED_PTCR_CTRL_FAN_NUM_EN(fan_tach_ch),
+ enable ?
+ ASPEED_PTCR_CTRL_FAN_NUM_EN(fan_tach_ch) : 0);
+}
+
+static void aspeed_set_fan_tach_ch_source(struct regmap *regmap, u8 fan_tach_ch,
+ u8 fan_tach_ch_source)
+{
+ u32 reg_value1 = ((fan_tach_ch_source & 0x3) <<
+ TACH_PWM_SOURCE_BIT01(fan_tach_ch));
+ u32 reg_value2 = (((fan_tach_ch_source & 0x4) >> 2) <<
+ TACH_PWM_SOURCE_BIT2(fan_tach_ch));
+
+ regmap_update_bits(regmap, ASPEED_PTCR_TACH_SOURCE,
+ TACH_PWM_SOURCE_MASK_BIT01(fan_tach_ch),
+ reg_value1);
+
+ regmap_update_bits(regmap, ASPEED_PTCR_TACH_SOURCE_EXT,
+ TACH_PWM_SOURCE_MASK_BIT2(fan_tach_ch),
+ reg_value2);
+}
+
+static void aspeed_set_pwm_port_fan_ctrl(struct aspeed_pwm_tacho_data *priv,
+ u8 index, u8 fan_ctrl)
+{
+ u16 period, dc_time_on;
+
+ period = priv->type_pwm_clock_unit[priv->pwm_port_type[index]];
+ period += 1;
+ dc_time_on = (fan_ctrl * period) / PWM_MAX;
+
+ if (dc_time_on == 0) {
+ aspeed_set_pwm_port_enable(priv->regmap, index, false);
+ } else {
+ if (dc_time_on == period)
+ dc_time_on = 0;
+
+ aspeed_set_pwm_port_duty_rising_falling(priv->regmap, index, 0,
+ dc_time_on);
+ aspeed_set_pwm_port_enable(priv->regmap, index, true);
+ }
+}
+
+static u32 aspeed_get_fan_tach_ch_measure_period(struct aspeed_pwm_tacho_data
+ *priv, u8 type)
+{
+ u32 clk;
+ u16 tacho_unit;
+ u8 clk_unit, div_h, div_l, tacho_div;
+
+ clk = priv->clk_freq;
+ clk_unit = priv->type_pwm_clock_unit[type];
+ div_h = priv->type_pwm_clock_division_h[type];
+ div_h = 0x1 << div_h;
+ div_l = priv->type_pwm_clock_division_l[type];
+ if (div_l == 0)
+ div_l = 1;
+ else
+ div_l = div_l * 2;
+
+ tacho_unit = priv->type_fan_tach_unit[type];
+ tacho_div = priv->type_fan_tach_clock_division[type];
+
+ tacho_div = 0x4 << (tacho_div * 2);
+ return clk / (clk_unit * div_h * div_l * tacho_div * tacho_unit);
+}
+
+static u32 aspeed_get_fan_tach_ch_rpm(struct aspeed_pwm_tacho_data *priv,
+ u8 fan_tach_ch)
+{
+ u32 raw_data, tach_div, clk_source, timeout = 0, sec, val;
+ u8 fan_tach_ch_source, type;
+
+ regmap_write(priv->regmap, ASPEED_PTCR_TRIGGER, 0);
+ regmap_write(priv->regmap, ASPEED_PTCR_TRIGGER, 0x1 << fan_tach_ch);
+
+ fan_tach_ch_source = priv->fan_tach_ch_source[fan_tach_ch];
+ type = priv->pwm_port_type[fan_tach_ch_source];
+
+ sec = (1000 / aspeed_get_fan_tach_ch_measure_period(priv, type));
+
+ msleep(sec);
+
+ regmap_read(priv->regmap, ASPEED_PTCR_RESULT, &val);
+ while (!(val & RESULT_STATUS_MASK)) {
+ timeout++;
+ if (timeout > 1)
+ return 0;
+ msleep(sec);
+ }
+
+ regmap_read(priv->regmap, ASPEED_PTCR_RESULT, &val);
+ raw_data = val & RESULT_VALUE_MASK;
+ tach_div = priv->type_fan_tach_clock_division[type];
+
+ tach_div = 0x4 << (tach_div * 2);
+ clk_source = priv->clk_freq;
+
+ if (raw_data == 0)
+ return 0;
+ else
+ return (clk_source * 60) / (2 * raw_data * tach_div);
+}
+
+static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+ int index = sensor_attr->index;
+ int ret;
+ struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+ long fan_ctrl;
+
+ ret = kstrtol(buf, 10, &fan_ctrl);
+ if (ret != 0)
+ return ret;
+
+ if (fan_ctrl < 0 || fan_ctrl > PWM_MAX)
+ return -EINVAL;
+
+ if (priv->pwm_port_fan_ctrl[index] == fan_ctrl)
+ return count;
+
+ priv->pwm_port_fan_ctrl[index] = fan_ctrl;
+ aspeed_set_pwm_port_fan_ctrl(priv, index, fan_ctrl);
+
+ return count;
+}
+
+static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+ int index = sensor_attr->index;
+ struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", priv->pwm_port_fan_ctrl[index]);
+}
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+ int index = sensor_attr->index;
+ u32 rpm;
+ struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+ rpm = aspeed_get_fan_tach_ch_rpm(priv, index);
+
+ return sprintf(buf, "%u\n", rpm);
+}
+
+static umode_t pwm_is_visible(struct kobject *kobj,
+ struct attribute *a, int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+ if (!priv->pwm_present[index])
+ return 0;
+ return a->mode;
+}
+
+static umode_t fan_dev_is_visible(struct kobject *kobj,
+ struct attribute *a, int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
+
+ if (!priv->fan_tach_present[index])
+ return 0;
+ return a->mode;
+}
+
+static SENSOR_DEVICE_ATTR(pwm0, 0644,
+ show_pwm, set_pwm, 0);
+static SENSOR_DEVICE_ATTR(pwm1, 0644,
+ show_pwm, set_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2, 0644,
+ show_pwm, set_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3, 0644,
+ show_pwm, set_pwm, 3);
+static SENSOR_DEVICE_ATTR(pwm4, 0644,
+ show_pwm, set_pwm, 4);
+static SENSOR_DEVICE_ATTR(pwm5, 0644,
+ show_pwm, set_pwm, 5);
+static SENSOR_DEVICE_ATTR(pwm6, 0644,
+ show_pwm, set_pwm, 6);
+static SENSOR_DEVICE_ATTR(pwm7, 0644,
+ show_pwm, set_pwm, 7);
+
+static struct attribute *pwm_dev_attrs[] = {
+ &sensor_dev_attr_pwm0.dev_attr.attr,
+ &sensor_dev_attr_pwm1.dev_attr.attr,
+ &sensor_dev_attr_pwm2.dev_attr.attr,
+ &sensor_dev_attr_pwm3.dev_attr.attr,
+ &sensor_dev_attr_pwm4.dev_attr.attr,
+ &sensor_dev_attr_pwm5.dev_attr.attr,
+ &sensor_dev_attr_pwm6.dev_attr.attr,
+ &sensor_dev_attr_pwm7.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group pwm_dev_group = {
+ .attrs = pwm_dev_attrs,
+ .is_visible = pwm_is_visible,
+};
+
+static SENSOR_DEVICE_ATTR(fan0_input, 0444,
+ show_rpm, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, 0444,
+ show_rpm, NULL, 1);
+static SENSOR_DEVICE_ATTR(fan2_input, 0444,
+ show_rpm, NULL, 2);
+static SENSOR_DEVICE_ATTR(fan3_input, 0444,
+ show_rpm, NULL, 3);
+static SENSOR_DEVICE_ATTR(fan4_input, 0444,
+ show_rpm, NULL, 4);
+static SENSOR_DEVICE_ATTR(fan5_input, 0444,
+ show_rpm, NULL, 5);
+static SENSOR_DEVICE_ATTR(fan6_input, 0444,
+ show_rpm, NULL, 6);
+static SENSOR_DEVICE_ATTR(fan7_input, 0444,
+ show_rpm, NULL, 7);
+static SENSOR_DEVICE_ATTR(fan8_input, 0444,
+ show_rpm, NULL, 8);
+static SENSOR_DEVICE_ATTR(fan9_input, 0444,
+ show_rpm, NULL, 9);
+static SENSOR_DEVICE_ATTR(fan10_input, 0444,
+ show_rpm, NULL, 10);
+static SENSOR_DEVICE_ATTR(fan11_input, 0444,
+ show_rpm, NULL, 11);
+static SENSOR_DEVICE_ATTR(fan12_input, 0444,
+ show_rpm, NULL, 12);
+static SENSOR_DEVICE_ATTR(fan13_input, 0444,
+ show_rpm, NULL, 13);
+static SENSOR_DEVICE_ATTR(fan14_input, 0444,
+ show_rpm, NULL, 14);
+static SENSOR_DEVICE_ATTR(fan15_input, 0444,
+ show_rpm, NULL, 15);
+
+static struct attribute *fan_dev_attrs[] = {
+ &sensor_dev_attr_fan0_input.dev_attr.attr,
+ &sensor_dev_attr_fan1_input.dev_attr.attr,
+ &sensor_dev_attr_fan2_input.dev_attr.attr,
+ &sensor_dev_attr_fan3_input.dev_attr.attr,
+ &sensor_dev_attr_fan4_input.dev_attr.attr,
+ &sensor_dev_attr_fan5_input.dev_attr.attr,
+ &sensor_dev_attr_fan6_input.dev_attr.attr,
+ &sensor_dev_attr_fan7_input.dev_attr.attr,
+ &sensor_dev_attr_fan8_input.dev_attr.attr,
+ &sensor_dev_attr_fan9_input.dev_attr.attr,
+ &sensor_dev_attr_fan10_input.dev_attr.attr,
+ &sensor_dev_attr_fan11_input.dev_attr.attr,
+ &sensor_dev_attr_fan12_input.dev_attr.attr,
+ &sensor_dev_attr_fan13_input.dev_attr.attr,
+ &sensor_dev_attr_fan14_input.dev_attr.attr,
+ &sensor_dev_attr_fan15_input.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group fan_dev_group = {
+ .attrs = fan_dev_attrs,
+ .is_visible = fan_dev_is_visible,
+};
+
+/*
+ * The clock type is type M :
+ * The PWM frequency = 24MHz / (type M clock division L bit *
+ * type M clock division H bit * (type M PWM period bit + 1))
+ */
+static void aspeed_create_type(struct aspeed_pwm_tacho_data *priv)
+{
+ priv->type_pwm_clock_division_h[TYPEM] = M_PWM_DIV_H;
+ priv->type_pwm_clock_division_l[TYPEM] = M_PWM_DIV_L;
+ priv->type_pwm_clock_unit[TYPEM] = M_PWM_PERIOD;
+ aspeed_set_pwm_clock_values(priv->regmap, TYPEM, M_PWM_DIV_H,
+ M_PWM_DIV_L, M_PWM_PERIOD);
+ aspeed_set_tacho_type_enable(priv->regmap, TYPEM, true);
+ priv->type_fan_tach_clock_division[TYPEM] = M_TACH_CLK_DIV;
+ priv->type_fan_tach_unit[TYPEM] = M_TACH_UNIT;
+ aspeed_set_tacho_type_values(priv->regmap, TYPEM, M_TACH_MODE,
+ M_TACH_UNIT, M_TACH_CLK_DIV);
+}
+
+static void aspeed_create_pwm_port(struct aspeed_pwm_tacho_data *priv,
+ u8 pwm_port)
+{
+ aspeed_set_pwm_port_enable(priv->regmap, pwm_port, true);
+ priv->pwm_present[pwm_port] = true;
+
+ priv->pwm_port_type[pwm_port] = TYPEM;
+ aspeed_set_pwm_port_type(priv->regmap, pwm_port, TYPEM);
+
+ priv->pwm_port_fan_ctrl[pwm_port] = INIT_FAN_CTRL;
+ aspeed_set_pwm_port_fan_ctrl(priv, pwm_port, INIT_FAN_CTRL);
+}
+
+static void aspeed_create_fan_tach_channel(struct aspeed_pwm_tacho_data *priv,
+ u8 *fan_tach_ch,
+ int count,
+ u8 pwm_source)
+{
+ u8 val, index;
+
+ for (val = 0; val < count; val++) {
+ index = fan_tach_ch[val];
+ aspeed_set_fan_tach_ch_enable(priv->regmap, index, true);
+ priv->fan_tach_present[index] = true;
+ priv->fan_tach_ch_source[index] = pwm_source;
+ aspeed_set_fan_tach_ch_source(priv->regmap, index, pwm_source);
+ }
+}
+
+static int aspeed_create_fan(struct device *dev,
+ struct device_node *child,
+ struct aspeed_pwm_tacho_data *priv)
+{
+ u8 *fan_tach_ch;
+ u32 pwm_port;
+ int ret, count;
+
+ ret = of_property_read_u32(child, "reg", &pwm_port);
+ if (ret)
+ return ret;
+ aspeed_create_pwm_port(priv, (u8)pwm_port);
+
+ count = of_property_count_u8_elems(child, "aspeed-fan-tach-ch");
+ if (count < 1)
+ return -EINVAL;
+ fan_tach_ch = devm_kzalloc(dev, sizeof(*fan_tach_ch) * count,
+ GFP_KERNEL);
+ if (!fan_tach_ch)
+ return -ENOMEM;
+ ret = of_property_read_u8_array(child, "aspeed-fan-tach-ch",
+ fan_tach_ch, count);
+ if (ret)
+ return ret;
+ aspeed_create_fan_tach_channel(priv, fan_tach_ch, count, pwm_port);
+
+ return 0;
+}
+
+static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np, *child;
+ struct aspeed_pwm_tacho_data *priv;
+ struct aspeed_pwm_tacho_regmap *hregmap;
+ struct resource *res;
+ struct device *hwmon;
+ struct clk *clk;
+ int ret;
+
+ np = dev->of_node;
+
+ hregmap = devm_kzalloc(dev, sizeof(*hregmap), GFP_KERNEL);
+ if (!hregmap)
+ return -ENOMEM;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENOENT;
+ hregmap->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(hregmap->regs))
+ return PTR_ERR(hregmap->regs);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ priv->regmap = devm_regmap_init(dev, NULL, hregmap,
+ &aspeed_pwm_tacho_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+ regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE, 0);
+ regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE_EXT, 0);
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ return -ENODEV;
+ priv->clk_freq = clk_get_rate(clk);
+ aspeed_set_clock_enable(priv->regmap, true);
+ aspeed_set_clock_source(priv->regmap, 0);
+
+ aspeed_create_type(priv);
+
+ for_each_child_of_node(np, child) {
+ ret = aspeed_create_fan(dev, child, priv);
+ of_node_put(child);
+ if (ret)
+ return ret;
+ }
+ of_node_put(np);
+
+ priv->groups[0] = &pwm_dev_group;
+ priv->groups[1] = &fan_dev_group;
+ hwmon = devm_hwmon_device_register_with_groups(dev,
+ "aspeed_pwm_tacho",
+ priv, priv->groups);
+
+ return PTR_ERR_OR_ZERO(hwmon);
+}
+
+static const struct of_device_id of_pwm_tacho_match_table[] = {
+ { .compatible = "aspeed,ast2400-pwm-tacho", },
+ { .compatible = "aspeed,ast2500-pwm-tacho", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_tacho_match_table);
+
+static struct platform_driver aspeed_pwm_tacho_driver = {
+ .probe = aspeed_pwm_tacho_probe,
+ .driver = {
+ .name = "aspeed_pwm_tacho",
+ .of_match_table = of_pwm_tacho_match_table,
+ },
+};
+
+module_platform_driver(aspeed_pwm_tacho_driver);
+
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("ASPEED PWM and Fan Tacho device driver");
+MODULE_LICENSE("GPL");
--
2.12.1.578.ge9c3154ca4-goog
^ permalink raw reply related
* Re: [PATCH 01/15] ARM: dts: alpine: fix PCIe node name
From: Antoine Tenart @ 2017-03-24 18:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Antoine Tenart, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
arm-soc, Linux ARM, Tsahee Zidenberg
In-Reply-To: <CAK8P3a2WOeNrz3J8vO+n+rhwn3N_vdOswZ0NMmYrdNvmkD8obQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 850 bytes --]
Hi Arnd,
On Fri, Mar 24, 2017 at 06:00:30PM +0100, Arnd Bergmann wrote:
> On Wed, Mar 22, 2017 at 9:01 AM, Antoine Tenart
> <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Tue, Mar 21, 2017 at 09:02:59PM -0500, Rob Herring wrote:
> >> PCIe bridges should have a node name of 'pcie'.
> >>
> >> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >
> > Acked-by: Antoine Tenart <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >
> > This can go through arm-soc directly. If not I'll take it.
> > Thanks!
>
> I have a mild preference for this kind of patch to get picked up by
> platform maintainers.
Sure. Applied to alpine/dt.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v4 2/4] dt-bindings: add the grf clock for dw-mipi-dsi
From: Rob Herring @ 2017-03-24 18:27 UTC (permalink / raw)
To: Chris Zhong
Cc: Mark Rutland, devicetree, linux-kernel, dri-devel, linux-rockchip,
linux-arm-kernel
In-Reply-To: <1490147691-4489-3-git-send-email-zyw@rock-chips.com>
On Wed, Mar 22, 2017 at 09:54:49AM +0800, Chris Zhong wrote:
> For RK3399, the grf clock should be controlled by dw-mipi-dsi driver,
> add the description for this clock.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
>
> Changes in v4:
> - remove "additional"
>
> Changes in v3: None
> Changes in v2: None
>
> .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 3/9] arm64: mm: install SError abort handler
From: Mark Rutland @ 2017-03-24 18:31 UTC (permalink / raw)
To: Florian Fainelli
Cc: Doug Berger, catalin.marinas-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, will.deacon-5wv7dgnIgG8,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, james.morse-5wv7dgnIgG8,
vladimir.murzin-5wv7dgnIgG8, panand-H+wXaHxf7aLQT0dZR+AlfA,
andre.przywara-5wv7dgnIgG8, cmetcalf-VPRAkNaXOzVWk0Htik3J/w,
mingo-DgEjT+Ai2ygdnm+yROfE0A,
sandeepa.s.prabhu-Re5JQEeQqe8AvxtiuMwx3w,
shijie.huang-5wv7dgnIgG8, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
marc.zyngier-5wv7dgnIgG8
In-Reply-To: <710c4142-ae20-9715-3e51-910a2073a64e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Florian,
On Fri, Mar 24, 2017 at 10:53:48AM -0700, Florian Fainelli wrote:
> On 03/24/2017 10:35 AM, Mark Rutland wrote:
> > On Fri, Mar 24, 2017 at 09:48:40AM -0700, Doug Berger wrote:
> >> On 03/24/2017 08:16 AM, Mark Rutland wrote:
> >>> On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
> >
> >> If you would consider an alternative implementation where we scrap
> >> the SError handler (i.e. maintain the ugliness in our downstream
> >> kernel) in favor of a more gentle user mode crash on SError that
> >> allows the kernel the opportunity to service the interrupt for
> >> diagnostic purposes I could try to repackage that.
> >
> > If this is just for diagnostic purposes, I believe you can register a
> > panic notifier, which can then read from the bus. The panic will occur,
> > but you'll have the opportunity to log some information to dmesg.
>
> And crash the kernel? That sounds awful, FWIW the ARM/Linux kernel is
> able to recover just fine from user-space accessing e.g: invalid
> physical addresses in the GISB register space, bringing the same level
> of functionality to ARM64/Linux sounds reasonable to me.
I disagree, given that:
(a) You cannot determine the (HW) origin of the SError in an
architecturally portable way. i.e. when you take an SError, you have
no way of determining what asynchronous event caused it.
(b) SError is effectively an edge-triggered interrupt for fatal system
errors (e.g. it may be triggered in resonse to ECC errors,
corruption detected in caches, etc). Even if you can determine that
the GISB triggered *an* SError, this does not tell you that this was
the *only* SError.
If you take an SError, something bad has already happened. Your data
may already have been corrupted, and worse, you don't know when or
where specifically this occurred (nor how many times).
(c) You cannot determine the (SW) origin of an SError without relying
upon implementation details. This cannot be written in a way that
does not rely on microarchitecture, integration, etc, and would need
to be updated for every future system with this misfeature.
(d) Even if you can determine the (SW) origin of an SError by relying on
IMPLEMENTATION DEFINED details, your handler needs to be intimately
familiar with the arch in question in order to attempt to recover.
For example, the existing code tries to skip an ARM instruction in
some cases. For arm64 there are three cases that would need to be
handled (AArch64 A64, AArch32 A32/ARM, AArch32 T32/Thumb).
Further, it appears to me that the existing code is broken given
that it doesn't handle Thumb, and given that it's skipping an
instruction in response to an asynchronous event -- i.e. some
arbitrary instruction after the one which triggered the abort.
For better or worse, SError *must* be treated as fatal.
As Doug stated:
The main benefit is to help debug user mode code that accidentally
maps a bad address since we would never make such an egregious error
in the kernel ;)
This is just one of many ways a userspace application with direct HW
access can bring down the system. I see no reason to treat it any
differently, especially given the above points.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] regulator: core: add regulator_has_continuous_voltage_range()
From: Mark Brown @ 2017-03-24 18:43 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Liam Girdwood, Rob Herring, Mark Rutland,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Anderson, Brian Norris,
Guenter Roeck, Dmitry Torokhov
In-Reply-To: <20170323214016.GA84219-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]
On Thu, Mar 23, 2017 at 02:40:16PM -0700, Matthias Kaehlcke wrote:
> El Mon, Mar 20, 2017 at 12:06:15PM +0000 Mark Brown ha dit:
> > On Fri, Mar 17, 2017 at 05:03:30PM -0700, Matthias Kaehlcke wrote:
> > > You are right that my case is very specialist, however I think it is
> > > a general problem that a consumer can't know whether the results of
> > > _list_voltage(), etc correspond to the regulator itself or to its
> > > supplies. E.g. a consumer might have a continuous reg which is
> > > supplied by a discrete reg, in this case _list_voltage() would return
> > > the steps of the supply reg, which is probably not what most consumers
> > > expect.
> > No, this is doesn't make much sense! Why should we be reporting
> > properties of the parent regulator when the child regulator is
> > regulating away all visibility of those properties?
> I am confused whether you are confirming that the current behavior
> makes no sense or if you think that what I'm saying is nonsense.
I'm saying that the current behaviour isn't good and that the
incoherence of what you're proposing should make this clear to you.
> > If it helps think of a continuous regulator as a discrete regulator with
> > a base voltage of 0 and steps of 1uV.
> Thanks, I understood that. What I didn't realize initially is that we
> can avoid iterating through all the voltages if the regulator has
> linear steps, which we can determine with regulator_get_linear_step().
> With that in mind I don't see concerns from the vctrl perspective.
OK, good. I keep meaning to look into retooling all the continuous
regulators to actually be linear regulators to simplify things.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 3/9] arm64: mm: install SError abort handler
From: Florian Fainelli @ 2017-03-24 19:02 UTC (permalink / raw)
To: Mark Rutland
Cc: Doug Berger, catalin.marinas-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, will.deacon-5wv7dgnIgG8,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, james.morse-5wv7dgnIgG8,
vladimir.murzin-5wv7dgnIgG8, panand-H+wXaHxf7aLQT0dZR+AlfA,
andre.przywara-5wv7dgnIgG8, cmetcalf-VPRAkNaXOzVWk0Htik3J/w,
mingo-DgEjT+Ai2ygdnm+yROfE0A,
sandeepa.s.prabhu-Re5JQEeQqe8AvxtiuMwx3w,
shijie.huang-5wv7dgnIgG8, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
marc.zyngier-5wv7dgnIgG8
In-Reply-To: <20170324183051.GD10746@leverpostej>
On 03/24/2017 11:31 AM, Mark Rutland wrote:
> Hi Florian,
>
> On Fri, Mar 24, 2017 at 10:53:48AM -0700, Florian Fainelli wrote:
>> On 03/24/2017 10:35 AM, Mark Rutland wrote:
>>> On Fri, Mar 24, 2017 at 09:48:40AM -0700, Doug Berger wrote:
>>>> On 03/24/2017 08:16 AM, Mark Rutland wrote:
>>>>> On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
>>>
>>>> If you would consider an alternative implementation where we scrap
>>>> the SError handler (i.e. maintain the ugliness in our downstream
>>>> kernel) in favor of a more gentle user mode crash on SError that
>>>> allows the kernel the opportunity to service the interrupt for
>>>> diagnostic purposes I could try to repackage that.
>>>
>>> If this is just for diagnostic purposes, I believe you can register a
>>> panic notifier, which can then read from the bus. The panic will occur,
>>> but you'll have the opportunity to log some information to dmesg.
>>
>> And crash the kernel? That sounds awful, FWIW the ARM/Linux kernel is
>> able to recover just fine from user-space accessing e.g: invalid
>> physical addresses in the GISB register space, bringing the same level
>> of functionality to ARM64/Linux sounds reasonable to me.
>
> I disagree, given that:
>
> (a) You cannot determine the (HW) origin of the SError in an
> architecturally portable way. i.e. when you take an SError, you have
> no way of determining what asynchronous event caused it.
>
> (b) SError is effectively an edge-triggered interrupt for fatal system
> errors (e.g. it may be triggered in resonse to ECC errors,
> corruption detected in caches, etc). Even if you can determine that
> the GISB triggered *an* SError, this does not tell you that this was
> the *only* SError.
Correct, which is why Doug's changes allow chaining of handlers.
>
> If you take an SError, something bad has already happened. Your data
> may already have been corrupted, and worse, you don't know when or
> where specifically this occurred (nor how many times).
Sure, but that still allows you to send the correct signal to a faulting
application (unless I am missing something here).
>
> (c) You cannot determine the (SW) origin of an SError without relying
> upon implementation details. This cannot be written in a way that
> does not rely on microarchitecture, integration, etc, and would need
> to be updated for every future system with this misfeature.
Which is exactly what is being done here, with the help of platform
specific information (we would not load brcmstb_gisb.c if we were not on
a platform where it makes sense to use that HW).
>
> (d) Even if you can determine the (SW) origin of an SError by relying on
> IMPLEMENTATION DEFINED details, your handler needs to be intimately
> familiar with the arch in question in order to attempt to recover.
>
> For example, the existing code tries to skip an ARM instruction in
> some cases. For arm64 there are three cases that would need to be
> handled (AArch64 A64, AArch32 A32/ARM, AArch32 T32/Thumb).
>
> Further, it appears to me that the existing code is broken given
> that it doesn't handle Thumb, and given that it's skipping an
> instruction in response to an asynchronous event -- i.e. some
> arbitrary instruction after the one which triggered the abort.
OK, that could presumably be fixed though.
>
> For better or worse, SError *must* be treated as fatal.
I disagree here, since this is a platform specific SError exception that
we can actually handle correctly there is a chance to actually not take
down the system on something that can be made non fatal and informative
at the same time.
>
> As Doug stated:
>
> The main benefit is to help debug user mode code that accidentally
> maps a bad address since we would never make such an egregious error
> in the kernel ;)
>
> This is just one of many ways a userspace application with direct HW
> access can bring down the system. I see no reason to treat it any
> differently, especially given the above points.
Partially disagree, in the absence of a way to specifically deal with
the exception, I would almost agree, but this is not the case here, we
have a piece of HW that can help us locate the problem, display an
informative message, and send a SIGBUS to the faulting application.
Anyway, I won't argue much further than that, but I certainly don't
think taking down an entire system is going to prove itself useful when
you need to deploy such a kernel to hundreds of people who have no clue
what so ever what their actual problem is in the first place. Taking a
SIGBUS and printing a message can at least allow us to say: read more
carefully, it say exactly what's wrong.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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