* Re: [PATCH] of: introduce event tracepoints for dynamic device_node lifecyle
From: Michael Ellerman @ 2017-04-19 1:31 UTC (permalink / raw)
To: Frank Rowand, Tyrel Datwyler, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <58F6AA35.2040902-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> On 04/17/17 17:32, Tyrel Datwyler wrote:
>> This patch introduces event tracepoints for tracking a device_nodes
>> reference cycle as well as reconfig notifications generated in response
>> to node/property manipulations.
>>
>> With the recent upstreaming of the refcount API several device_node
>> underflows and leaks have come to my attention in the pseries (DLPAR) dynamic
>> logical partitioning code (ie. POWER speak for hotplugging virtual and physcial
>> resources at runtime such as cpus or IOAs). These tracepoints provide a
>> easy and quick mechanism for validating the reference counting of
>> device_nodes during their lifetime.
>>
>> Further, when pseries lpars are migrated to a different machine we
>> perform a live update of our device tree to bring it into alignment with the
>> configuration of the new machine. The of_reconfig_notify trace point
>> provides a mechanism that can be turned for debuging the device tree
>> modifications with out having to build a custom kernel to get at the
>> DEBUG code introduced by commit 00aa3720.
>
> I do not like changing individual (or small groups of) printk() style
> debugging information to tracepoint style.
I'm not quite sure which printks() you're referring to.
The only printks that are removed in this series are under #ifdef DEBUG,
and so are essentially not there unless you build a custom kernel.
They also only cover the reconfig case, which is actually less
interesting than the much more common and bug-prone get/put logic.
> As far as I know, there is no easy way to combine trace data and printk()
> style data to create a single chronology of events. If some of the
> information needed to debug an issue is trace data and some is printk()
> style data then it becomes more difficult to understand the overall
> situation.
If you enable CONFIG_PRINTK_TIME then you should be able to just sort
the trace and the printk output by the timestamp. If you're really
trying to correlate the two then you should probably just be using
trace_printk().
But IMO this level of detail, tracing every get/put, does not belong in
printk. Trace points are absolutely the right solution for this type of
debugging.
cheers
--
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] ARM: dts: rockchip: reuse firefly dtsi
From: Eddie Cai @ 2017-04-19 0:42 UTC (permalink / raw)
To: Heiko Stuebner
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <746291609.ssvjv54Vin@phil>
2017-04-19 6:20 GMT+08:00 Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>:
> Hi Eddie,
>
> Am Dienstag, 18. April 2017, 20:15:27 CEST schrieb Eddie Cai:
>> firefly reload is very similar with firefly board, so reuse firefly dtsi
>>
>> Signed-off-by: Eddie Cai <eddie.cai.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 310 ------------------
>> arch/arm/boot/dts/rk3288-firefly-reload.dts | 368 ++--------------------
>
> I would disagree and remember having a similar discussion when the reload-
> support was initially submitted. Please keep in mind that the firefly-
> reload is a som+baseboard combination, so somebody could (or maybe
> already has) create a completely different baseboard for the som that
> does not have any similarities with the original firefly.
> The previous firefly being a real single board.
>
> We also don't combine rock2 and firefly and other boards following the
> general rk3288 design guidelines :-) and the original firefly and reload are
> very different boards if you look at them.
OK, i think the real similar part is the core board and firefly. how
about reuse the
firefly code in the core board?
>
>
> Heiko
>
--
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] ARM: dts: rockchip: reuse firefly dtsi
From: Eddie Cai @ 2017-04-19 0:40 UTC (permalink / raw)
To: Heiko Stuebner
Cc: robh+dt, mark.rutland, linux, linux-arm-kernel, linux-rockchip,
devicetree, linux-kernel
In-Reply-To: <746291609.ssvjv54Vin@phil>
[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]
2017-04-19 6:20 GMT+08:00 Heiko Stuebner <heiko@sntech.de>:
> Hi Eddie,
>
> Am Dienstag, 18. April 2017, 20:15:27 CEST schrieb Eddie Cai:
> > firefly reload is very similar with firefly board, so reuse firefly dtsi
> >
> > Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> > ---
> > arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 310
> ------------------
> > arch/arm/boot/dts/rk3288-firefly-reload.dts | 368
> ++--------------------
>
> I would disagree and remember having a similar discussion when the reload-
> support was initially submitted. Please keep in mind that the firefly-
> reload is a som+baseboard combination, so somebody could (or maybe
> already has) create a completely different baseboard for the som that
> does not have any similarities with the original firefly.
> The previous firefly being a real single board.
> We also don't combine rock2 and firefly and other boards following the
> general rk3288 design guidelines :-) and the original firefly and reload
> are
> very different boards if you look at them.
>
OK, i think the real similar part is the core board and firefly. how about
reuse the
firefly code in the core board?
>
>
> Heiko
>
>
[-- Attachment #2: Type: text/html, Size: 2047 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE
From: Florian Fainelli @ 2017-04-19 0:26 UTC (permalink / raw)
To: Markus Mayer, Guenter Roeck
Cc: Jean Delvare, Rob Herring, Mark Rutland, Markus Mayer,
Broadcom Kernel List, Linux HWMON List, Device Tree List,
ARM Kernel List, Linux Kernel Mailing List
In-Reply-To: <CANEuBv5uTBurgpoXBh143T0sB+GCj8oymDxh5+2CqXk5A4dYqw@mail.gmail.com>
On 04/18/2017 05:15 PM, Markus Mayer wrote:
> On 18 April 2017 at 15:47, Guenter Roeck <linux@roeck-us.net> wrote:
>> Hi Florian,
>>
>> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>>> Hey Guenter,
>>>
>>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>>>> Hi Markus,
>>>>
>>>> On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>>>>> From: Markus Mayer <mmayer@broadcom.com>
>>>>>
>>>>> This driver allows access to DRAM properties, such as the refresh rate,
>>>>> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>>>>> used as indirect indicator of the DRAM temperature.
>>>>>
>>>>> The driver also allows setting of the sampling interval.
>>>>>
>>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>>> ---
>>>> [ ... ]
>>>>
>>>>> +
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>>>>> + 1000);
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>>>>> +static struct attribute *dpfe_attrs[] = {
>>>>> + &sensor_dev_attr_dpfe_info.dev_attr.attr,
>>>>> + &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>>>>> + &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>>>>> + NULL
>>>>> +};
>>>>> +ATTRIBUTE_GROUPS(dpfe);
>>>>> +
>>>>
>>>> There is not a single standard hwmon attribute. I don't know how
>>>> to classify this driver, and where it should reside, but it is not
>>>> a hwmon driver.
>>>
>>> This is a driver that talks to an embedded CPU running firmware which is
>>> capable of giving various informations about the DRAM chip being
>>> populated, including a temperature trend (hotter or cooler). We thought
>>> initially we would be able to expose the actual temperature, but this in
>>> turn required a lot more knowledge about the DRAM chip that we wish we
>>> knew about. That is sort of where and why this driver was proposed for
>>> hwmon.
>>>
>>> Which subsystem do you think would be best for this driver drivers/misc/
>>> or drivers/soc/bcm/brcmstb/ maybe?
>>
>> Both should work. I would probably try misc first and let Greg tell me
>> which way to go ;-).
>
> Thanks for the tip. As Florian said, it was not the idea to submit a
> completely non-standard hwmon driver. I was sure I would be able to
> report at least some standard hwmon data also.
Good thing with drivers/soc/bcm/brcmstb/ is that I can take such patches
directly through ARM SoC pull requests, and since this is inherently
Broadcom STB specific, that sounds like the most ideal location IMHO.
--
Florian
^ permalink raw reply
* Re: [PATCH v6 6/8] coresight: add support for CPU debug module
From: Leo Yan @ 2017-04-19 0:18 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Suzuki K Poulose, Stephen Boyd, linux-doc, linux-kernel,
devicetree, linux-arm-kernel, linux-arm-msm, linux-soc,
Mike Leach, Sudeep Holla
In-Reply-To: <20170418174050.GC22806@linaro.org>
On Tue, Apr 18, 2017 at 11:40:50AM -0600, Mathieu Poirier wrote:
> On Thu, Apr 06, 2017 at 09:30:59PM +0800, Leo Yan wrote:
> > Coresight includes debug module and usually the module connects with CPU
> > debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
> > description for related info in "Part H: External Debug".
> >
> > Chapter H7 "The Sample-based Profiling Extension" introduces several
> > sampling registers, e.g. we can check program counter value with
> > combined CPU exception level, secure state, etc. So this is helpful for
> > analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
> > loop with IRQ disabled. In this case the CPU cannot switch context and
> > handle any interrupt (including IPIs), as the result it cannot handle
> > SMP call for stack dump.
> >
> > This patch is to enable coresight debug module, so firstly this driver
> > is to bind apb clock for debug module and this is to ensure the debug
> > module can be accessed from program or external debugger. And the driver
> > uses sample-based registers for debug purpose, e.g. when system triggers
> > panic, the driver will dump program counter and combined context
> > registers (EDCIDSR, EDVIDSR); by parsing context registers so can
> > quickly get to know CPU secure state, exception level, etc.
> >
> > Some of the debug module registers are located in CPU power domain, so
> > this requires the CPU power domain stays on when access related debug
> > registers, but the power management for CPU power domain is quite
> > dependent on SoC integration for power management. For the platforms
> > which with sane power controller implementations, this driver follows
> > the method to set EDPRCR to try to pull the CPU out of low power state
> > and then set 'no power down request' bit so the CPU has no chance to
> > lose power.
> >
> > If the SoC has not followed up this design well for power management
> > controller, the user should use the command line parameter or sysfs
> > to constrain all or partial idle states to ensure the CPU power
> > domain is enabled and access coresight CPU debug component safely.
> >
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>
> This is coming along well - a few comment below. In your next revision please
> add GKH to the 'To' list.
Sure. Will send out in this week and add Greg in 'To' list.
> > ---
> > drivers/hwtracing/coresight/Kconfig | 14 +
> > drivers/hwtracing/coresight/Makefile | 1 +
> > drivers/hwtracing/coresight/coresight-cpu-debug.c | 667 ++++++++++++++++++++++
> > 3 files changed, 682 insertions(+)
> > create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c
> >
> > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> > index 130cb21..8d55d6d 100644
> > --- a/drivers/hwtracing/coresight/Kconfig
> > +++ b/drivers/hwtracing/coresight/Kconfig
> > @@ -89,4 +89,18 @@ config CORESIGHT_STM
> > logging useful software events or data coming from various entities
> > in the system, possibly running different OSs
> >
> > +config CORESIGHT_CPU_DEBUG
> > + tristate "CoreSight CPU Debug driver"
> > + depends on ARM || ARM64
> > + depends on DEBUG_FS
> > + help
> > + This driver provides support for coresight debugging module. This
> > + is primarily used to dump sample-based profiling registers when
> > + system triggers panic, the driver will parse context registers so
> > + can quickly get to know program counter (PC), secure state,
> > + exception level, etc. Before use debugging functionality, platform
> > + needs to ensure the clock domain and power domain are enabled
> > + properly, please refer Documentation/trace/coresight-cpu-debug.txt
> > + for detailed description and the example for usage.
> > +
> > endif
> > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> > index af480d9..433d590 100644
> > --- a/drivers/hwtracing/coresight/Makefile
> > +++ b/drivers/hwtracing/coresight/Makefile
> > @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
> > coresight-etm4x-sysfs.o
> > obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
> > obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> > +obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> > diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> > new file mode 100644
> > index 0000000..8470e31
> > --- /dev/null
> > +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> > @@ -0,0 +1,667 @@
> > +/*
> > + * Copyright (c) 2017 Linaro Limited. All rights reserved.
> > + *
> > + * Author: Leo Yan <leo.yan@linaro.org>
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + *
> > + */
> > +#include <linux/amba/bus.h>
> > +#include <linux/coresight.h>
> > +#include <linux/cpu.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/pm_qos.h>
> > +#include <linux/slab.h>
> > +#include <linux/smp.h>
> > +#include <linux/types.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include "coresight-priv.h"
> > +
> > +#define EDPCSR 0x0A0
> > +#define EDCIDSR 0x0A4
> > +#define EDVIDSR 0x0A8
> > +#define EDPCSR_HI 0x0AC
> > +#define EDOSLAR 0x300
> > +#define EDPRCR 0x310
> > +#define EDPRSR 0x314
> > +#define EDDEVID1 0xFC4
> > +#define EDDEVID 0xFC8
> > +
> > +#define EDPCSR_PROHIBITED 0xFFFFFFFF
> > +
> > +/* bits definition for EDPCSR */
> > +#define EDPCSR_THUMB BIT(0)
> > +#define EDPCSR_ARM_INST_MASK GENMASK(31, 2)
> > +#define EDPCSR_THUMB_INST_MASK GENMASK(31, 1)
> > +
> > +/* bits definition for EDPRCR */
> > +#define EDPRCR_COREPURQ BIT(3)
> > +#define EDPRCR_CORENPDRQ BIT(0)
> > +
> > +/* bits definition for EDPRSR */
> > +#define EDPRSR_DLK BIT(6)
> > +#define EDPRSR_PU BIT(0)
> > +
> > +/* bits definition for EDVIDSR */
> > +#define EDVIDSR_NS BIT(31)
> > +#define EDVIDSR_E2 BIT(30)
> > +#define EDVIDSR_E3 BIT(29)
> > +#define EDVIDSR_HV BIT(28)
> > +#define EDVIDSR_VMID GENMASK(7, 0)
> > +
> > +/*
> > + * bits definition for EDDEVID1:PSCROffset
> > + *
> > + * NOTE: armv8 and armv7 have different definition for the register,
> > + * so consolidate the bits definition as below:
> > + *
> > + * 0b0000 - Sample offset applies based on the instruction state, we
> > + * rely on EDDEVID to check if EDPCSR is implemented or not
> > + * 0b0001 - No offset applies.
> > + * 0b0010 - No offset applies, but do not use in AArch32 mode
> > + *
> > + */
> > +#define EDDEVID1_PCSR_OFFSET_MASK GENMASK(3, 0)
> > +#define EDDEVID1_PCSR_OFFSET_INS_SET (0x0)
> > +#define EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32 (0x2)
> > +
> > +/* bits definition for EDDEVID */
> > +#define EDDEVID_PCSAMPLE_MODE GENMASK(3, 0)
> > +#define EDDEVID_IMPL_EDPCSR (0x1)
> > +#define EDDEVID_IMPL_EDPCSR_EDCIDSR (0x2)
> > +#define EDDEVID_IMPL_FULL (0x3)
> > +
> > +#define DEBUG_WAIT_SLEEP 1000
> > +#define DEBUG_WAIT_TIMEOUT 32000
> > +
> > +struct debug_drvdata {
> > + void __iomem *base;
> > e struct device *dev;
> > + int cpu;
> > +
> > + bool edpcsr_present;
> > + bool edcidsr_present;
> > + bool edvidsr_present;
> > + bool pc_has_offset;
> > +
> > + u32 edpcsr;
> > + u32 edpcsr_hi;
> > + u32 edprsr;
> > + u32 edvidsr;
> > + u32 edcidsr;
> > +};
> > +
> > +static DEFINE_MUTEX(debug_lock);
> > +static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
> > +static int debug_count;
> > +static struct dentry *debug_debugfs_dir;
> > +
> > +static bool debug_enable;
> > +module_param_named(enable, debug_enable, bool, 0600);
> > +MODULE_PARM_DESC(enable, "Knob to enable debug functionality "
> > + "(default is 0, which means is disabled by default)");
>
> For this driver we have a debugFS interface so I question the validity of a
> kernel module parameter. Other than adding complexity to the code it offers no
> real added value. If a user is to insmod a module, it is just as easy to switch
> on the functionality using debugFS in a second step.
This module parameter can be used for kernel command line, so
it's useful when user wants to dynamically turn on/off the
functionality at boot up time.
Does this make sense for you? Removing this parameter is okay for
me, but this means users need to decide if use it by Kernel config
with static building in. This is a bit contradictory with before's
discussion.
> > +static void debug_os_unlock(struct debug_drvdata *drvdata)
> > +{
> > + /* Unlocks the debug registers */
> > + writel_relaxed(0x0, drvdata->base + EDOSLAR);
>
> Here the wmb is to make sure reordering (either at compile time or in the CPU)
> doesn't happend. You need a comment here otherwise checkpatch will complain.
> Speaking of which, did you run this through checkpatch?
Right. Everytime I run checkpatch before sending patch and it has
complain for this. Will add comment. Sorry for imprecise working.
> > + wmb();
> > +}
> > +
> > +/*
> > + * According to ARM DDI 0487A.k, before access external debug
> > + * registers should firstly check the access permission; if any
> > + * below condition has been met then cannot access debug
> > + * registers to avoid lockup issue:
> > + *
> > + * - CPU power domain is powered off;
> > + * - The OS Double Lock is locked;
> > + *
> > + * By checking EDPRSR can get to know if meet these conditions.
> > + */
> > +static bool debug_access_permitted(struct debug_drvdata *drvdata)
> > +{
> > + /* CPU is powered off */
> > + if (!(drvdata->edprsr & EDPRSR_PU))
> > + return false;
> > +
> > + /* The OS Double Lock is locked */
> > + if (drvdata->edprsr & EDPRSR_DLK)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static void debug_force_cpu_powered_up(struct debug_drvdata *drvdata)
> > +{
> > + bool retried = false;
> > + u32 edprcr;
> > +
> > +try_again:
> > +
> > + /*
> > + * Send request to power management controller and assert
> > + * DBGPWRUPREQ signal; if power management controller has
> > + * sane implementation, it should enable CPU power domain
> > + * in case CPU is in low power state.
> > + */
> > + edprcr = readl_relaxed(drvdata->base + EDPRCR);
> > + edprcr |= EDPRCR_COREPURQ;
> > + writel_relaxed(edprcr, drvdata->base + EDPRCR);
> > +
> > + /* Wait for CPU to be powered up (timeout~=32ms) */
> > + if (readx_poll_timeout_atomic(readl_relaxed, drvdata->base + EDPRSR,
> > + drvdata->edprsr, (drvdata->edprsr & EDPRSR_PU),
> > + DEBUG_WAIT_SLEEP, DEBUG_WAIT_TIMEOUT)) {
> > + /*
> > + * Unfortunately the CPU cannot be powered up, so return
> > + * back and later has no permission to access other
> > + * registers. For this case, should disable CPU low power
> > + * states to ensure CPU power domain is enabled!
> > + */
> > + pr_err("%s: power up request for CPU%d failed\n",
> > + __func__, drvdata->cpu);
> > + return;
> > + }
> > +
> > + /*
> > + * At this point the CPU is powered up, so set the no powerdown
> > + * request bit so we don't lose power and emulate power down.
> > + */
> > + edprcr = readl_relaxed(drvdata->base + EDPRCR);
> > + edprcr |= EDPRCR_COREPURQ | EDPRCR_CORENPDRQ;
> > + writel_relaxed(edprcr, drvdata->base + EDPRCR);
> > +
> > + drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> > +
> > + /* Bail out if CPU is powered up */
> > + if (likely(drvdata->edprsr & EDPRSR_PU))
> > + return;
>
> /* The core power domain got switched off on use, try again */
> if (unlikely(!drvdata->edprsr & EDPRSR_PU))
> goto try_again;
>
> I understand you don't want to introduce a infinite loop but if that happens
> here, something else has gone very wrong. The above readx_poll_timeout_atomic
> loop should take care of bailing out in case of problems. That way you also get
> to rid of the retried variable and the code is more simple.
Okay. I will follow your method. Thinking the duration of CPU power
on/off is not same magnitude with this piece code, the infinite loop
will not happen.
> > +
> > + /*
> > + * Handle race condition if CPU has been waken up but it sleeps
> > + * again if EDPRCR_CORENPDRQ has been flipped, so try to run
> > + * waken flow one more time.
> > + */
> > + if (!retried) {
> > + retried = true;
> > + goto try_again;
> > + }
> > +}
> > +
> > +static void debug_read_regs(struct debug_drvdata *drvdata)
> > +{
> > + u32 save_edprcr;
> > +
> > + CS_UNLOCK(drvdata->base);
> > +
> > + /* Unlock os lock */
> > + debug_os_unlock(drvdata);
> > +
> > + /* Save EDPRCR register */
> > + save_edprcr = readl_relaxed(drvdata->base + EDPRCR);
> > +
> > + /*
> > + * Ensure CPU power domain is enabled to let registers
> > + * are accessiable.
> > + */
> > + debug_force_cpu_powered_up(drvdata);
> > +
> > + if (!debug_access_permitted(drvdata))
> > + goto out;
> > +
> > + drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
> > +
> > + /*
> > + * As described in ARM DDI 0487A.k, if the processing
> > + * element (PE) is in debug state, or sample-based
> > + * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
> > + * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
> > + * UNKNOWN state. So directly bail out for this case.
> > + */
> > + if (drvdata->edpcsr == EDPCSR_PROHIBITED)
> > + goto out;
> > +
> > + /*
> > + * A read of the EDPCSR normally has the side-effect of
> > + * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
> > + * at this point it's safe to read value from them.
> > + */
> > + if (IS_ENABLED(CONFIG_64BIT))
> > + drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> > +
> > + if (drvdata->edcidsr_present)
> > + drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
> > +
> > + if (drvdata->edvidsr_present)
> > + drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
> > +
> > +out:
> > + /* Restore EDPRCR register */
> > + writel_relaxed(save_edprcr, drvdata->base + EDPRCR);
> > +
> > + CS_LOCK(drvdata->base);
> > +}
> > +
> > +static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata)
> > +{
> > + unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
> > + unsigned long pc;
> > +
> > + if (IS_ENABLED(CONFIG_64BIT))
> > + return (unsigned long)drvdata->edpcsr_hi << 32 |
> > + (unsigned long)drvdata->edpcsr;
> > +
> > + pc = (unsigned long)drvdata->edpcsr;
> > +
> > + if (drvdata->pc_has_offset) {
> > + arm_inst_offset = 8;
> > + thumb_inst_offset = 4;
> > + }
> > +
> > + /* Handle thumb instruction */
> > + if (pc & EDPCSR_THUMB) {
> > + pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
> > + return pc;
> > + }
> > +
> > + /*
> > + * Handle arm instruction offset, if the arm instruction
> > + * is not 4 byte alignment then it's possible the case
> > + * for implementation defined; keep original value for this
> > + * case and print info for notice.
> > + */
> > + if (pc & BIT(1))
> > + pr_emerg("Instruction offset is implementation defined\n");
> > + else
> > + pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
> > +
> > + return pc;
> > +}
> > +
> > +static void debug_dump_regs(struct debug_drvdata *drvdata)
> > +{
> > + unsigned long pc;
> > +
> > + pr_emerg("\tEDPRSR: %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
> > + drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
> > + drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
> > +
> > + if (!debug_access_permitted(drvdata)) {
> > + pr_emerg("No permission to access debug registers!\n");
> > + return;
> > + }
> > +
> > + if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
> > + pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
> > + return;
> > + }
> > +
> > + pc = debug_adjust_pc(drvdata);
> > + pr_emerg("\tEDPCSR: [<%p>] %pS\n", (void *)pc, (void *)pc);
> > +
> > + if (drvdata->edcidsr_present)
> > + pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
> > +
> > + if (drvdata->edvidsr_present)
> > + pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%dbits VMID:%x)\n",
> > + drvdata->edvidsr,
> > + drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
> > + drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
> > + (drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
> > + drvdata->edvidsr & EDVIDSR_HV ? 64 : 32,
> > + drvdata->edvidsr & (u32)EDVIDSR_VMID);
> > +}
> > +
> > +static void debug_init_arch_data(void *info)
> > +{
> > + struct debug_drvdata *drvdata = info;
> > + u32 mode, pcsr_offset;
> > + u32 eddevid, eddevid1;
> > +
> > + CS_UNLOCK(drvdata->base);
> > +
> > + /* Read device info */
> > + eddevid = readl_relaxed(drvdata->base + EDDEVID);
> > + eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
> > +
> > + CS_LOCK(drvdata->base);
> > +
> > + /* Parse implementation feature */
> > + mode = eddevid & EDDEVID_PCSAMPLE_MODE;
> > + pcsr_offset = eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
> > +
> > + drvdata->edpcsr_present = false;
> > + drvdata->edcidsr_present = false;
> > + drvdata->edvidsr_present = false;
> > + drvdata->pc_has_offset = false;
> > +
> > + switch (mode) {
> > + case EDDEVID_IMPL_FULL:
> > + drvdata->edvidsr_present = true;
> > + /* Fall through */
> > + case EDDEVID_IMPL_EDPCSR_EDCIDSR:
> > + drvdata->edcidsr_present = true;
> > + /* Fall through */
> > + case EDDEVID_IMPL_EDPCSR:
> > + /*
> > + * In ARM DDI 0487A.k, the EDDEVID1.PCSROffset is used to
> > + * define if has the offset for PC sampling value; if read
> > + * back EDDEVID1.PCSROffset == 0x2, then this means the debug
> > + * module does not sample the instruction set state when
> > + * armv8 CPU in AArch32 state.
> > + */
> > + drvdata->edpcsr_present = (IS_ENABLED(CONFIG_64BIT) ||
> > + (pcsr_offset != EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32));
> > +
> > + drvdata->pc_has_offset =
> > + (pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +/*
> > + * Dump out information on panic.
> > + */
> > +static int debug_notifier_call(struct notifier_block *self,
> > + unsigned long v, void *p)
> > +{
> > + int cpu;
> > + struct debug_drvdata *drvdata;
> > +
> > + pr_emerg("ARM external debug module:\n");
> > +
> > + for_each_possible_cpu(cpu) {
> > + drvdata = per_cpu(debug_drvdata, cpu);
> > + if (!drvdata)
> > + continue;
> > +
> > + pr_emerg("CPU[%d]:\n", drvdata->cpu);
> > +
> > + debug_read_regs(drvdata);
> > + debug_dump_regs(drvdata);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct notifier_block debug_notifier = {
> > + .notifier_call = debug_notifier_call,
> > +};
> > +
> > +static int debug_enable_func(void)
> > +{
> > + struct debug_drvdata *drvdata;
> > + int cpu;
> > +
> > + for_each_possible_cpu(cpu) {
> > + drvdata = per_cpu(debug_drvdata, cpu);
> > + if (!drvdata)
> > + continue;
> > +
> > + pm_runtime_get_sync(drvdata->dev);
> > + }
> > +
> > + return atomic_notifier_chain_register(&panic_notifier_list,
> > + &debug_notifier);
> > +}
> > +
> > +static int debug_disable_func(void)
> > +{
> > + struct debug_drvdata *drvdata;
> > + int cpu;
> > +
> > + for_each_possible_cpu(cpu) {
> > + drvdata = per_cpu(debug_drvdata, cpu);
> > + if (!drvdata)
> > + continue;
> > +
> > + pm_runtime_put(drvdata->dev);
> > + }
> > +
> > + return atomic_notifier_chain_unregister(&panic_notifier_list,
> > + &debug_notifier);
> > +}
> > +
> > +static ssize_t debug_func_knob_write(struct file *f,
> > + const char __user *buf, size_t count, loff_t *ppos)
> > +{
> > + u8 val;
> > + int ret;
> > +
> > + ret = kstrtou8_from_user(buf, count, 2, &val);
> > + if (ret)
> > + return ret;
> > +
> > + mutex_lock(&debug_lock);
> > +
> > + if (val == debug_enable)
> > + goto out;
> > +
> > + if (val)
> > + ret = debug_enable_func();
> > + else
> > + ret = debug_disable_func();
>
> I don't think you need to install the handler every time the functionality is
> switched on/off. I suggest to install the handler at boot time (or module
> insertion time) and in the notifier handler, check the debug_enable flag before
> moving on with the output.
Will do.
> > +
> > + if (ret) {
> > + pr_err("%s: unable to %s debug function: %d\n",
> > + __func__, val ? "enable" : "disable", ret);
> > + goto err;
> > + }
> > +
> > + debug_enable = val;
>
> Using a true/false value is probably better here. That way you don't end up
> with miscellaneous values in debugFS.
>From my testing here will not assign other values rather than 0 or 1.
This is controlled by function kstrtou8_from_user(), the 3rd parameter
'2' is to define the value range [0..1], so other values will report
error after return back from kstrtou8_from_user().
> > +out:
> > + ret = count;
> > +err:
> > + mutex_unlock(&debug_lock);
> > + return ret;
> > +}
> > +
> > +static ssize_t debug_func_knob_read(struct file *f,
> > + char __user *ubuf, size_t count, loff_t *ppos)
> > +{
> > + ssize_t ret;
> > + char buf[2];
> > +
> > + mutex_lock(&debug_lock);
> > +
> > + buf[0] = '0' + debug_enable;
> > + buf[1] = '\n';
>
> snprintf()
Will fix.
> > + ret = simple_read_from_buffer(ubuf, count, ppos, buf, sizeof(buf));
> > +
> > + mutex_unlock(&debug_lock);
> > + return ret;
> > +}
> > +
> > +static const struct file_operations debug_func_knob_fops = {
> > + .open = simple_open,
> > + .read = debug_func_knob_read,
> > + .write = debug_func_knob_write,
> > +};
> > +
> > +static int debug_func_init(void)
> > +{
> > + struct dentry *file;
> > + int ret;
> > +
> > + /* Create debugfs node */
> > + debug_debugfs_dir = debugfs_create_dir("coresight_cpu_debug", NULL);
> > + if (!debug_debugfs_dir) {
> > + pr_err("%s: unable to create debugfs directory\n", __func__);
> > + return -ENOMEM;
> > + }
> > +
> > + file = debugfs_create_file("enable", S_IRUGO | S_IWUSR,
> > + debug_debugfs_dir, NULL, &debug_func_knob_fops);
>
> Please align this properly.
Will fix. Thanks a lot for detailed reviewing.
[...]
Thanks,
Leo Yan
^ permalink raw reply
* Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE
From: Markus Mayer @ 2017-04-19 0:15 UTC (permalink / raw)
To: Guenter Roeck
Cc: Florian Fainelli, Markus Mayer, Jean Delvare, Rob Herring,
Mark Rutland, Markus Mayer, Broadcom Kernel List,
Linux HWMON List, Device Tree List, ARM Kernel List,
Linux Kernel Mailing List
In-Reply-To: <20170418224739.GA24376-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
On 18 April 2017 at 15:47, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
> Hi Florian,
>
> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>> Hey Guenter,
>>
>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>> > Hi Markus,
>> >
>> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> >> From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >>
>> >> This driver allows access to DRAM properties, such as the refresh rate,
>> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> >> used as indirect indicator of the DRAM temperature.
>> >>
>> >> The driver also allows setting of the sampling interval.
>> >>
>> >> Signed-off-by: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >> ---
>> > [ ... ]
>> >
>> >> +
>> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> >> + 1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> >> +static struct attribute *dpfe_attrs[] = {
>> >> + &sensor_dev_attr_dpfe_info.dev_attr.attr,
>> >> + &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> >> + &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> >> + NULL
>> >> +};
>> >> +ATTRIBUTE_GROUPS(dpfe);
>> >> +
>> >
>> > There is not a single standard hwmon attribute. I don't know how
>> > to classify this driver, and where it should reside, but it is not
>> > a hwmon driver.
>>
>> This is a driver that talks to an embedded CPU running firmware which is
>> capable of giving various informations about the DRAM chip being
>> populated, including a temperature trend (hotter or cooler). We thought
>> initially we would be able to expose the actual temperature, but this in
>> turn required a lot more knowledge about the DRAM chip that we wish we
>> knew about. That is sort of where and why this driver was proposed for
>> hwmon.
>>
>> Which subsystem do you think would be best for this driver drivers/misc/
>> or drivers/soc/bcm/brcmstb/ maybe?
>
> Both should work. I would probably try misc first and let Greg tell me
> which way to go ;-).
Thanks for the tip. As Florian said, it was not the idea to submit a
completely non-standard hwmon driver. I was sure I would be able to
report at least some standard hwmon data also.
Regards,
-Markus
--
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] of: introduce event tracepoints for dynamic device_node lifecyle
From: Frank Rowand @ 2017-04-19 0:07 UTC (permalink / raw)
To: Tyrel Datwyler, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
nfont-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
mpe-Gsx/Oe8HsFggBc27wqDAHg, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1492475525-10827-1-git-send-email-tyreld-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On 04/17/17 17:32, Tyrel Datwyler wrote:
> This patch introduces event tracepoints for tracking a device_nodes
> reference cycle as well as reconfig notifications generated in response
> to node/property manipulations.
>
> With the recent upstreaming of the refcount API several device_node
> underflows and leaks have come to my attention in the pseries (DLPAR) dynamic
> logical partitioning code (ie. POWER speak for hotplugging virtual and physcial
> resources at runtime such as cpus or IOAs). These tracepoints provide a
> easy and quick mechanism for validating the reference counting of
> device_nodes during their lifetime.
>
> Further, when pseries lpars are migrated to a different machine we
> perform a live update of our device tree to bring it into alignment with the
> configuration of the new machine. The of_reconfig_notify trace point
> provides a mechanism that can be turned for debuging the device tree
> modifications with out having to build a custom kernel to get at the
> DEBUG code introduced by commit 00aa3720.
I do not like changing individual (or small groups of) printk() style
debugging information to tracepoint style.
As far as I know, there is no easy way to combine trace data and printk()
style data to create a single chronology of events. If some of the
information needed to debug an issue is trace data and some is printk()
style data then it becomes more difficult to understand the overall
situation.
If Rob wants to convert printk() style data to trace data (and I can't
convince him otherwise) then I will have further comments on this specific
patch.
-Frank
>
> The following trace events are provided: of_node_get, of_node_put,
> of_node_release, and of_reconfig_notify. These trace points require a kernel
> built with ftrace support to be enabled. In a typical environment where
> debugfs is mounted at /sys/kernel/debug the entire set of tracepoints
> can be set with the following:
>
> echo "of:*" > /sys/kernel/debug/tracing/set_event
>
> or
>
> echo 1 > /sys/kernel/debug/tracing/of/enable
>
> The following shows the trace point data from a DLPAR remove of a cpu
> from a pseries lpar:
>
> cat /sys/kernel/debug/tracing/trace | grep "POWER8@10"
>
> cpuhp/23-147 [023] .... 128.324827:
> of_node_put: refcount=5, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147 [023] .... 128.324829:
> of_node_put: refcount=4, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147 [023] .... 128.324829:
> of_node_put: refcount=3, dn->full_name=/cpus/PowerPC,POWER8@10
> cpuhp/23-147 [023] .... 128.324831:
> of_node_put: refcount=2, dn->full_name=/cpus/PowerPC,POWER8@10
> drmgr-7284 [009] .... 128.439000:
> of_node_put: refcount=1, dn->full_name=/cpus/PowerPC,POWER8@10
> drmgr-7284 [009] .... 128.439002:
> of_reconfig_notify: action=DETACH_NODE, dn->full_name=/cpus/PowerPC,POWER8@10,
> prop->name=null, old_prop->name=null
> drmgr-7284 [009] .... 128.439015:
> of_node_put: refcount=0, dn->full_name=/cpus/PowerPC,POWER8@10
> drmgr-7284 [009] .... 128.439016:
> of_node_release: dn->full_name=/cpus/PowerPC,POWER8@10, dn->_flags=4
>
> Signed-off-by: Tyrel Datwyler <tyreld-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
> drivers/of/dynamic.c | 30 ++++++---------
> include/trace/events/of.h | 93 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 105 insertions(+), 18 deletions(-)
> create mode 100644 include/trace/events/of.h
>
< snip >
--
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 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
From: Eric Anholt @ 2017-04-19 0:02 UTC (permalink / raw)
To: Florian Fainelli, dri-devel, Rob Herring, Mark Rutland,
devicetree
Cc: linux-kernel
In-Reply-To: <7906db2f-cfb8-e2e6-5869-b6e829dd8c6f@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 703 bytes --]
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 04/18/2017 04:38 PM, Eric Anholt wrote:
>> For the Raspberry Pi's bindings, the power domain also implicitly
>> turns on the clock and deasserts reset, but for the new Cygnus port we
>> start representing the clock in the devicetree.
>>
>> v2: Document the clock-names property, check for -ENOENT for no clock
>> in DT.
>>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>
>> + if (v3d->clk)
>> + clk_disable_unprepare(v3d->clk);
>
> The clock API allows you to pass a NULL clk and do nothing in these
> cases which is what you seem to have done a few lines below, you could
> simplify these checks?
Sounds good.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
From: Eric Anholt @ 2017-04-19 0:02 UTC (permalink / raw)
To: Florian Fainelli, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <7906db2f-cfb8-e2e6-5869-b6e829dd8c6f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> On 04/18/2017 04:38 PM, Eric Anholt wrote:
>> For the Raspberry Pi's bindings, the power domain also implicitly
>> turns on the clock and deasserts reset, but for the new Cygnus port we
>> start representing the clock in the devicetree.
>>
>> v2: Document the clock-names property, check for -ENOENT for no clock
>> in DT.
>>
>> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>> ---
>
>> + if (v3d->clk)
>> + clk_disable_unprepare(v3d->clk);
>
> The clock API allows you to pass a NULL clk and do nothing in these
> cases which is what you seem to have done a few lines below, you could
> simplify these checks?
Sounds good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
From: Florian Fainelli @ 2017-04-18 23:48 UTC (permalink / raw)
To: Eric Anholt, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170418233805.15767-1-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
On 04/18/2017 04:38 PM, Eric Anholt wrote:
> For the Raspberry Pi's bindings, the power domain also implicitly
> turns on the clock and deasserts reset, but for the new Cygnus port we
> start representing the clock in the devicetree.
>
> v2: Document the clock-names property, check for -ENOENT for no clock
> in DT.
>
> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> ---
> + if (v3d->clk)
> + clk_disable_unprepare(v3d->clk);
The clock API allows you to pass a NULL clk and do nothing in these
cases which is what you seem to have done a few lines below, you could
simplify these checks?
> +
> return 0;
> }
>
> @@ -318,6 +322,13 @@ static int vc4_v3d_runtime_resume(struct device *dev)
> if (ret)
> return ret;
>
> + if (v3d->clk) {
> + int ret = clk_prepare_enable(v3d->clk);
> +
> + if (ret != 0)
> + return ret;
> + }
> +
> vc4_v3d_init_hw(vc4->dev);
> vc4_irq_postinstall(vc4->dev);
>
> @@ -348,15 +359,40 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
> vc4->v3d = v3d;
> v3d->vc4 = vc4;
>
> + v3d->clk = devm_clk_get(dev, "v3d_clk");
> + if (IS_ERR(v3d->clk)) {
> + int ret = PTR_ERR(v3d->clk);
> +
> + if (ret == -ENOENT) {
> + /* bcm2835 didn't have a clock reference in the DT. */
> + ret = 0;
> + v3d->clk = NULL;
> + } else {
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Failed to get V3D clock: %d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> if (V3D_READ(V3D_IDENT0) != V3D_EXPECTED_IDENT0) {
> DRM_ERROR("V3D_IDENT0 read 0x%08x instead of 0x%08x\n",
> V3D_READ(V3D_IDENT0), V3D_EXPECTED_IDENT0);
> return -EINVAL;
> }
>
> + if (v3d->clk) {
> + ret = clk_prepare_enable(v3d->clk);
> + if (ret != 0)
> + return ret;
> + }
> +
> ret = vc4_allocate_bin_bo(drm);
> - if (ret)
> + if (ret) {
> + if (v3d->clk)
> + clk_disable_unprepare(v3d->clk);
> return ret;
> + }
>
> /* Reset the binner overflow address/size at setup, to be sure
> * we don't reuse an old one.
>
--
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
* [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
From: Eric Anholt @ 2017-04-18 23:38 UTC (permalink / raw)
To: dri-devel, Rob Herring, Mark Rutland, devicetree; +Cc: linux-kernel
In-Reply-To: <20170418191157.18517-1-eric@anholt.net>
For the Raspberry Pi's bindings, the power domain also implicitly
turns on the clock and deasserts reset, but for the new Cygnus port we
start representing the clock in the devicetree.
v2: Document the clock-names property, check for -ENOENT for no clock
in DT.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
.../devicetree/bindings/display/brcm,bcm-vc4.txt | 4 +++
drivers/gpu/drm/vc4/vc4_drv.h | 1 +
drivers/gpu/drm/vc4/vc4_v3d.c | 38 +++++++++++++++++++++-
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
index ca02d3e4db91..2318266f6481 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
+++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
@@ -59,6 +59,10 @@ Required properties for V3D:
- interrupts: The interrupt number
See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+Optional properties for V3D:
+- clocks: The clock the unit runs on
+- clock-names: Must be "v3d_clk"
+
Required properties for DSI:
- compatible: Should be "brcm,bcm2835-dsi0" or "brcm,bcm2835-dsi1"
- reg: Physical base address and length of the DSI block's registers
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index b0967e2f7e88..92eb7d811bf2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -200,6 +200,7 @@ struct vc4_v3d {
struct vc4_dev *vc4;
struct platform_device *pdev;
void __iomem *regs;
+ struct clk *clk;
};
struct vc4_hvs {
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index a88078d7c9d1..ca987d2800c6 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -16,6 +16,7 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include "linux/clk.h"
#include "linux/component.h"
#include "linux/pm_runtime.h"
#include "vc4_drv.h"
@@ -305,6 +306,9 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
drm_gem_object_put_unlocked(&vc4->bin_bo->base.base);
vc4->bin_bo = NULL;
+ if (v3d->clk)
+ clk_disable_unprepare(v3d->clk);
+
return 0;
}
@@ -318,6 +322,13 @@ static int vc4_v3d_runtime_resume(struct device *dev)
if (ret)
return ret;
+ if (v3d->clk) {
+ int ret = clk_prepare_enable(v3d->clk);
+
+ if (ret != 0)
+ return ret;
+ }
+
vc4_v3d_init_hw(vc4->dev);
vc4_irq_postinstall(vc4->dev);
@@ -348,15 +359,40 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
vc4->v3d = v3d;
v3d->vc4 = vc4;
+ v3d->clk = devm_clk_get(dev, "v3d_clk");
+ if (IS_ERR(v3d->clk)) {
+ int ret = PTR_ERR(v3d->clk);
+
+ if (ret == -ENOENT) {
+ /* bcm2835 didn't have a clock reference in the DT. */
+ ret = 0;
+ v3d->clk = NULL;
+ } else {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get V3D clock: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
if (V3D_READ(V3D_IDENT0) != V3D_EXPECTED_IDENT0) {
DRM_ERROR("V3D_IDENT0 read 0x%08x instead of 0x%08x\n",
V3D_READ(V3D_IDENT0), V3D_EXPECTED_IDENT0);
return -EINVAL;
}
+ if (v3d->clk) {
+ ret = clk_prepare_enable(v3d->clk);
+ if (ret != 0)
+ return ret;
+ }
+
ret = vc4_allocate_bin_bo(drm);
- if (ret)
+ if (ret) {
+ if (v3d->clk)
+ clk_disable_unprepare(v3d->clk);
return ret;
+ }
/* Reset the binner overflow address/size at setup, to be sure
* we don't reuse an old one.
--
2.11.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH] ARM: dts: bcm-cygnus: Add 911360's V3D device.
From: Eric Anholt @ 2017-04-18 23:32 UTC (permalink / raw)
To: Florian Fainelli, Rob Herring, Mark Rutland, devicetree
Cc: linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list, Ray Jui,
Scott Branden, Jon Mason, Eric Anholt
This loads the VC4 driver on the 911360_entphn platform (with the
corresponding series sent to dri-devel), which is supported by master
of the Mesa tree.
Signed-off-by: Eric Anholt <eric@anholt.net>
---
arch/arm/boot/dts/bcm-cygnus.dtsi | 13 +++++++++++++
arch/arm/boot/dts/bcm911360_entphn.dts | 8 ++++++++
2 files changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index f52750c6d3ed..907a5e843364 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -406,6 +406,19 @@
status = "disabled";
};
+ v3d: v3d@180a2000 {
+ compatible = "brcm,cygnus-v3d";
+ reg = <0x180a2000 0x1000>;
+ clocks = <&mipipll BCM_CYGNUS_MIPIPLL_CH2_V3D>;
+ clock-names = "v3d_clk";
+ interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
+
+ vc4: gpu {
+ compatible = "brcm,cygnus-vc4";
+ };
+
adc: adc@180a6000 {
compatible = "brcm,iproc-static-adc";
#io-channel-cells = <1>;
diff --git a/arch/arm/boot/dts/bcm911360_entphn.dts b/arch/arm/boot/dts/bcm911360_entphn.dts
index 8b3800f46288..037621c13290 100644
--- a/arch/arm/boot/dts/bcm911360_entphn.dts
+++ b/arch/arm/boot/dts/bcm911360_entphn.dts
@@ -57,6 +57,14 @@
};
};
+&v3d {
+ assigned-clocks =
+ <&mipipll BCM_CYGNUS_MIPIPLL>,
+ <&mipipll BCM_CYGNUS_MIPIPLL_CH2_V3D>;
+ assigned-clock-rates = <525000000>, <300000000>;
+ status = "okay";
+};
+
&uart3 {
status = "okay";
};
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v6 5/8] coresight: use const for device_node structures
From: Leo Yan @ 2017-04-18 23:13 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Suzuki K Poulose, Stephen Boyd, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA, Mike Leach, Sudeep Holla
In-Reply-To: <20170418152447.GB22806-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Hi Mathieu,
On Tue, Apr 18, 2017 at 09:24:47AM -0600, Mathieu Poirier wrote:
> On Thu, Apr 06, 2017 at 09:30:58PM +0800, Leo Yan wrote:
> > Almost low level functions from open firmware have used const to
> > qualify device_node structures, so add const for device_node
> > parameters in of_coresight related functions.
> >
> > Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> I agree with these changes but the patch needs to be split up - please see
> below.
>
> > ---
> > drivers/hwtracing/coresight/of_coresight.c | 6 +++---
> > include/linux/coresight.h | 8 ++++----
> > 2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> > index 78d2399..46eec0f 100644
> > --- a/drivers/hwtracing/coresight/of_coresight.c
> > +++ b/drivers/hwtracing/coresight/of_coresight.c
> > @@ -52,7 +52,7 @@ of_coresight_get_endpoint_device(struct device_node *endpoint)
> > endpoint, of_dev_node_match);
> > }
> >
> > -static void of_coresight_get_ports(struct device_node *node,
> > +static void of_coresight_get_ports(const struct device_node *node,
> > int *nr_inport, int *nr_outport)
>
> Move this to a patch by itself as it is not related to this driver.
>
> > {
> > struct device_node *ep = NULL;
> > @@ -101,7 +101,7 @@ static int of_coresight_alloc_memory(struct device *dev,
> > return 0;
> > }
> >
> > -int of_coresight_get_cpu(struct device_node *node)
> > +int of_coresight_get_cpu(const struct device_node *node)
>
> Move this to the previous patch in this set. There is not need to undo what you
> just did there.
>
> > {
> > int cpu;
> > bool found;
> > @@ -128,7 +128,7 @@ int of_coresight_get_cpu(struct device_node *node)
> > EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
> >
> > struct coresight_platform_data *of_get_coresight_platform_data(
> > - struct device *dev, struct device_node *node)
> > + struct device *dev, const struct device_node *node)
>
> Same here, move this to a new patch (the same one as of_coresight_get_ports()).
Yeah, this is better than my form. Will change.
Thanks,
Leo Yan
--
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/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE
From: Guenter Roeck @ 2017-04-18 22:47 UTC (permalink / raw)
To: Florian Fainelli
Cc: Markus Mayer, Jean Delvare, Rob Herring, Mark Rutland,
Markus Mayer, Broadcom Kernel List, Linux HWMON List,
Device Tree List, ARM Kernel List, Linux Kernel Mailing List
In-Reply-To: <f8c99c6f-8c53-0d0e-0712-d001c1e45f75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Florian,
On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
> Hey Guenter,
>
> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> > Hi Markus,
> >
> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> >> From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >>
> >> This driver allows access to DRAM properties, such as the refresh rate,
> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> >> used as indirect indicator of the DRAM temperature.
> >>
> >> The driver also allows setting of the sampling interval.
> >>
> >> Signed-off-by: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >> ---
> > [ ... ]
> >
> >> +
> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> >> + 1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> >> +static struct attribute *dpfe_attrs[] = {
> >> + &sensor_dev_attr_dpfe_info.dev_attr.attr,
> >> + &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> >> + &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> >> + NULL
> >> +};
> >> +ATTRIBUTE_GROUPS(dpfe);
> >> +
> >
> > There is not a single standard hwmon attribute. I don't know how
> > to classify this driver, and where it should reside, but it is not
> > a hwmon driver.
>
> This is a driver that talks to an embedded CPU running firmware which is
> capable of giving various informations about the DRAM chip being
> populated, including a temperature trend (hotter or cooler). We thought
> initially we would be able to expose the actual temperature, but this in
> turn required a lot more knowledge about the DRAM chip that we wish we
> knew about. That is sort of where and why this driver was proposed for
> hwmon.
>
> Which subsystem do you think would be best for this driver drivers/misc/
> or drivers/soc/bcm/brcmstb/ maybe?
Both should work. I would probably try misc first and let Greg tell me
which way to go ;-).
Thanks,
Guenter
--
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/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE
From: Florian Fainelli @ 2017-04-18 22:29 UTC (permalink / raw)
To: Guenter Roeck, Markus Mayer
Cc: Jean Delvare, Rob Herring, Mark Rutland, Markus Mayer,
Broadcom Kernel List, Linux HWMON List, Device Tree List,
ARM Kernel List, Linux Kernel Mailing List
In-Reply-To: <20170418205839.GA3554@roeck-us.net>
Hey Guenter,
On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> Hi Markus,
>
> On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> From: Markus Mayer <mmayer@broadcom.com>
>>
>> This driver allows access to DRAM properties, such as the refresh rate,
>> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> used as indirect indicator of the DRAM temperature.
>>
>> The driver also allows setting of the sampling interval.
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
> [ ... ]
>
>> +
>> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> + 1000);
>> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> +static struct attribute *dpfe_attrs[] = {
>> + &sensor_dev_attr_dpfe_info.dev_attr.attr,
>> + &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> + &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> + NULL
>> +};
>> +ATTRIBUTE_GROUPS(dpfe);
>> +
>
> There is not a single standard hwmon attribute. I don't know how
> to classify this driver, and where it should reside, but it is not
> a hwmon driver.
This is a driver that talks to an embedded CPU running firmware which is
capable of giving various informations about the DRAM chip being
populated, including a temperature trend (hotter or cooler). We thought
initially we would be able to expose the actual temperature, but this in
turn required a lot more knowledge about the DRAM chip that we wish we
knew about. That is sort of where and why this driver was proposed for
hwmon.
Which subsystem do you think would be best for this driver drivers/misc/
or drivers/soc/bcm/brcmstb/ maybe?
--
Florian
^ permalink raw reply
* Re: [PATCH v3 0/7] NFC: trf7970a: Fixups & convert to desc-based GPIO
From: Mark Greer @ 2017-04-18 22:26 UTC (permalink / raw)
To: Samuel Ortiz, Lauro Ramos Venancio, Aloisio Almeida Jr
Cc: linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Greer
In-Reply-To: <20170120191745.29255-1-mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>
On Fri, Jan 20, 2017 at 12:17:38PM -0700, Mark Greer wrote:
> These trf7970a driver patches do the following things:
> - a couple minor fixups
> - allow EN2 to not be managed by the driver (e.g., when its tied low by
> hardware
> - convert the driver to use the descriptor-based GPIO interface
> - remove support for 'vin-voltage-override' DT property
> - change the DTS example to indicate that EN and EN2 are active high GPIOs
> - add Mark Greer as the maintainer of the trf7970a driver
>
> Based on k.o. 44b4b46 (Merge tag 'armsoc-fixes' of
> git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc)
>
> v2->v3:
> - Removed "[PATCH v2 5/7] NFC: trf7970a: Clean up coding style issues"
> because it will make merging patches from Geoff Lansberry and others
> hard to apply. I will resubmit once those patches have been merged
> or rejected.
> - Added a patch to remove 'vin-voltage-override' DT property support as
> proper DT regulator set up makes it unnecessary.
>
> v1->v2:
> - Commit description fixups only; no functional changes.
>
> Mark Greer (7):
> NFC: trf7970a: Don't de-assert EN2 unless it was asserted
> NFC: trf7970a: Don't manage EN2 when not specified in DT
> NFC: trf7970a: Convert to descriptor based GPIO interface
> NFC: trf7970a: Remove useless comment
> NFC: trf7970a: Remove support for 'vin-voltage-override' DT property
> NFC: trf7970a: Enable pins are active high not active low
> MAINTAINERS: NFC: trf7970a: Add Mark Greer as maintainer
>
> .../devicetree/bindings/net/nfc/trf7970a.txt | 6 +-
> MAINTAINERS | 7 ++
> drivers/nfc/Kconfig | 2 +-
> drivers/nfc/trf7970a.c | 75 +++++++++-------------
> 4 files changed, 40 insertions(+), 50 deletions(-)
>
> --
> 2.11.0
Ping?
--
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] ARM: dts: rockchip: reuse firefly dtsi
From: Heiko Stuebner @ 2017-04-18 22:20 UTC (permalink / raw)
To: Eddie Cai
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170418121527.3155-1-eddie.cai.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Eddie,
Am Dienstag, 18. April 2017, 20:15:27 CEST schrieb Eddie Cai:
> firefly reload is very similar with firefly board, so reuse firefly dtsi
>
> Signed-off-by: Eddie Cai <eddie.cai.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 310 ------------------
> arch/arm/boot/dts/rk3288-firefly-reload.dts | 368 ++--------------------
I would disagree and remember having a similar discussion when the reload-
support was initially submitted. Please keep in mind that the firefly-
reload is a som+baseboard combination, so somebody could (or maybe
already has) create a completely different baseboard for the som that
does not have any similarities with the original firefly.
The previous firefly being a real single board.
We also don't combine rock2 and firefly and other boards following the
general rk3288 design guidelines :-) and the original firefly and reload are
very different boards if you look at them.
Heiko
--
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 v13 03/10] mux: minimal mux subsystem and gpio-based mux controller
From: Peter Rosin @ 2017-04-18 21:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wolfram Sang, Rob Herring, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
linux-iio, linux-doc, Andrew Morton, Colin Ian King,
Paul Gortmaker, Philipp Zabel, kernel
In-Reply-To: <20170418114430.GA1496@kroah.com>
On 2017-04-18 13:44, Greg Kroah-Hartman wrote:
> On Tue, Apr 18, 2017 at 12:59:50PM +0200, Peter Rosin wrote:
>> On 2017-04-18 10:51, Greg Kroah-Hartman wrote:
>>> On Thu, Apr 13, 2017 at 06:43:07PM +0200, Peter Rosin wrote:
>>>> +config MUX_GPIO
>>>> + tristate "GPIO-controlled Multiplexer"
>>>> + depends on OF && GPIOLIB
>>>
>>> Why have the gpio and mux core in the same patch?
>>
>> One is not usable w/o the other. I can split them if you want to?
>
> Then why are they two different config options? Add the core code in
> one patch, and then add the gpio controled mutiplexer in a separate
> patch.
Ah, I meant when there are not yet any other mux drivers. I'll just
split the patch.
>>>> +static struct class mux_class = {
>>>> + .name = "mux",
>>>> + .owner = THIS_MODULE,
>>>> +};
>>>
>>> No Documentation/ABI/ update for your sysfs files? Please do so.
>>
>> Ok I'll look into it. Wasn't even aware that I added any. But there's the
>> new class of course...
>
> Hint, you have files, the devices that belong to the class :)
Right.
>>>> +static int __init mux_init(void)
>>>> +{
>>>> + return class_register(&mux_class);
>>>> +}
>>>> +
>>>> +static DEFINE_IDA(mux_ida);
>>>
>>> When your module is unloaded, you forgot to clean this structure up with
>>> what was done with it.
>>
>> I was under the impression that not providing an exit function for modules
>> made the module infrastructure prevent unloading (by bumping some reference
>> counter). Maybe that is a misconception?
>
> Ah, messy, don't do that. Make it so you can unload your module please,
> why wouldn't you want that to happen?
What made me do it was the worry that mux drivers might be left dangling
w/o the core. But that can probably only happen if someone is very
deliberately trying to break stuff by forcing unloads??
>>>> + mux_chip = kzalloc(sizeof(*mux_chip) +
>>>> + controllers * sizeof(*mux_chip->mux) +
>>>> + sizeof_priv, GFP_KERNEL);
>>>> + if (!mux_chip)
>>>> + return NULL;
>>>
>>> You don't return PTR_ERR(-ENOMEM)? Ok, why not? (I'm not arguing for
>>> it, just curious...)
>>
>> There's no particular reason. Do you think I should change it?
>
> What does the caller do with an error? Pass it up to where? Who gets
> it? Don't you want the caller to know you are out of memory?
The current callers return -ENOMEM when NULL is returned here. Looks
like I'm going to be doing some fairly major changes anyway so I'll
just change this too while at it...
>>>> +
>>>> + device_initialize(&mux_chip->dev);
>>>
>>> Why are you not registering the device here as well? Why have this be a
>>> two step process?
>>
>> Because of idle state handling. The drivers are expected to fill in
>> the desired idle state(s) after allocating the mux controller(s).
>> Then, when registering, the desired idle state is activated (if the
>> idle state is not idle-as-is, of course) and as a last step the mux
>> is "advertised".
>
> Ok, is that documented in the functions somewhere?
I'll make sure to add it if it's missing.
>>>> + ret = device_add(&mux_chip->dev);
>>>> + if (ret < 0)
>>>> + dev_err(&mux_chip->dev,
>>>> + "device_add failed in mux_chip_register: %d\n", ret);
>>>
>>> Did you run checkpatch.pl in strict mode on this new file? Please do so :)
>>
>> I did, and did it again just to be sure, and I do not get any complaints.
>> So, what's wrong?
>
> You list the function name in the printk string, it should complain
> that __func__ should be used. Oh well, it's just a perl script, it
> doesn't always catch everything.
> isn't always correct :)
Ah, ok.
>>>> +EXPORT_SYMBOL_GPL(devm_mux_chip_alloc);
>>>
>>>
>>> Having devm functions that create/destroy other struct devices worries
>>> me, do we have other examples of this happening today? Are you sure you
>>> got the reference counting all correct?
>>
>> drivers/iio/industrialio_core.c:devm_iio_device_alloc
>>
>> Or is the iio case different in some subtle way that I'm missing?
>
> I don't know, hopefully you got it all correct, I haven't audited that
> code path in a long time :)
Looks as ok to me as it did before. Moving on... :-)
>>>> +
>>>> +static int devm_mux_chip_match(struct device *dev, void *res, void *data)
>>>> +{
>>>> + struct mux_chip **r = res;
>>>> +
>>>> + if (WARN_ON(!r || !*r))
>>>
>>> How can this happen?
>>
>> It shouldn't. I copied the pattern from the iio subsystem.
>
> Then it should be removed there too...
Ok, I'll see if I can find time to suggest some patch(es) to Jonathan.
>>>> +void devm_mux_chip_free(struct device *dev, struct mux_chip *mux_chip)
>>>> +{
>>>> + WARN_ON(devres_release(dev, devm_mux_chip_release,
>>>> + devm_mux_chip_match, mux_chip));
>>>
>>> What can someone do with these WARN_ON() splats in the kernel log?
>>
>> Don't know. Again, I copied the pattern from the iio subsystem.
>
> If you don't know what it should be used for, don't copy it!
>
> Cargo-cult coding is horrible, please no.
Ok, I'll just drop those WARNs...
>>>> + /* ...or it's just contended. */
>>>> + down_write(&mux->lock);
>>>
>>> Why use a read/write lock at all? Have you tested this to verify it
>>> really is faster and needed?
>>
>> For one of the HW configuration that drove the development, the same mux
>> controller is used to mux both an I2C channel and a couple of ADC lines.
>>
>> If there is no kind of reader/writer locking going on, there is no way to
>> do ADC readings concurrently with an I2C transfer even when the consumers
>> want the mux in the same position. With an ordinary mutex controlling the
>> mux position, the consumers will unconditionally get serialized, which
>> seems like a waste to me. Or maybe I'm missing something?
>
> Why is serializing things a "waste"? Again, a rw semaphore is slower,
> takes more logic to get correct, and is very complex at times. If you
> are not SURE you need it, and that it matters, don't use it. And if you
> do use it, document the heck out of it how you need it and why.
It's a waste of time because two independent mux consumers of the same
mux controller can't do things concurrently even if they want the same
thing from the mux. Let's say that one mux consumer is an iio-mux and
one is an i2c-mux. Also, let's say that you for some reason need to get
a lot of samples at a determined rate through the iio-mux. With a mutex,
that can't happen if there is an access through the i2c-mux taking
"forever" in the eyes of the ADC/iio-mux, even if they both want the
shared mux controller to be in the same position. The ADC/iio-mux and
the i2c-mux would be waiting for each other for no purpose at all.
>>>> +
>>>> + if (mux->cached_state == state) {
>>>> + /*
>>>> + * Hmmm, someone else changed the mux to my liking.
>>>> + * That makes me wonder how long I waited for nothing?
>>>> + */
>>>> + downgrade_write(&mux->lock);
>>>
>>> Oh that always scares me... Are you _sure_ this is correct? And
>>> needed?
>>
>> It might not be needed, and it would probably work ok to just fall
>> through and call mux_control_set unconditionally. What is it that
>> always scares you exactly? Relying on cached state to be correct?
>> Downgrading writer locks?
>
> downgrading a writer lock scares me, especially for something as
> "simple" as this type of interface. Again, don't use it unless you
> _have_ to. Simple is good, start with that always.
Some kind of lock needs to be grabbed in mux_control_select and
released in mux_control_deselect that fixes the mux state while the
mux consumer goes about its business. For the reasons stated above I
went with a reader/writer lock instead of the mutex I had originally.
Agreed, the code in mux_control_select is a few more lines than I
like, but I suspected the big issue to be holding *any* lock over
a pair of "global" functions. Changing from holding a rw-lock as
reader to instead holding a mutex changes very little in my eyes.
mux_control_select is simply not *that* complicated...
>>>> + if (mux->idle_state != MUX_IDLE_AS_IS &&
>>>> + mux->idle_state != mux->cached_state)
>>>> + ret = mux_control_set(mux, mux->idle_state);
>>>> +
>>>> + up_read(&mux->lock);
>>>
>>> You require a lock to be held for a "global" function? Without
>>> documentation? Or even a sparse marking? That's asking for trouble...
>>
>> Documentation I can handle, but where should I look to understand how I
>> should add sparse markings?
>
> Run sparse on the code and see what it says :)
Will do.
>> The mux needs to be locked somehow. But as I stated in the cover letter
>> the rwsem isn't a perfect fit.
>>
>> I'm using an rwsem to lock a mux, but that isn't really a
>> perfect fit. Is there a better locking primitive that I don't
>> know about that fits better? I had a mutex at one point, but
>> that didn't allow any concurrent accesses at all. At least
>> the rwsem allows concurrent access as long as all users
>> agree on the mux state, but I suspect that the rwsem will
>> degrade to the mutex situation pretty quickly if there is
>> any contention.
>>
>> Also, the lock doesn't add anything if there is only one consumer of
>> a mux controller. Maybe there should be some mechanism for shortcutting
>> the locking for the (more common?) single-consumer case?
>>
>> But again, I need the locking for my multi-consumer use case.
>
> Go back to a mutex, and having a function that requires it to be held
> is, icky.
But how do you propose that the ickyness is avoided? It's a requirement
that any waiters are released when the mux is available...
*thinking/coding for a bit*
I'm going to experiment with the below (untested) code which AFAICT
has the issues that starvation is possible, that it isn't first-come
first-serve, and that once there is contention the waiters may wait
for a longer time than needed...
On the positive side there are no actual locks held from select over
to deselect and there's no rwsem. But a missing (or an extra) deselect
still messes things up pretty fatally, since the below is really just
some kind of open coded locking thingy that I thought was one of the
things I should stay away from. But maybe you like it better?
(In the below, mux->wait_sem is assumed to be initialized to zero.)
int mux_control_select(struct mux_control *mux, int state)
{
int ret = 0;
again:
mutex_lock(&mux->lock);
if (mux->cached_state == state) {
/* The mux is already correct, just bump the user count. */
++mux->users;
goto done;
}
if (mux->users) {
/* The mux needs updating but is in use, wait... */
++mux->waiters;
mutex_unlock(&mux->lock);
down(&mux->wait_sem);
goto again;
}
/* The mux needs updating and is unused. */
ret = mux_control_set(mux, state);
if (ret >= 0) {
++mux->users;
goto done;
}
/* The mux update failed, try to revert if appropriate... */
if (mux->idle_state != MUX_IDLE_AS_IS)
mux_control_set(mux, mux->idle_state);
/* ...and release a waiter if there is one. */
if (mux->waiters) {
--mux->waiters;
up(&mux->wait_sem);
}
done:
mutex_unlock(&mux->lock);
return ret;
}
int mux_control_deselect(struct mux_control *mux)
{
int ret = 0;
mutex_lock(&mux->lock);
if (--mux->users)
goto done;
/* This was the last user, idle the mux if appropriate... */
if (mux->idle_state != MUX_IDLE_AS_IS &&
mux->idle_state != mux->cached_state)
ret = mux_control_set(mux, mux->idle_state);
/* ...and release a waiter if there is one. */
if (mux->waiters) {
--mux->waiters;
up(&mux->wait_sem);
}
done:
mutex_unlock(&mux->lock);
return ret;
}
>>>> +/*
>>>> + * Using subsys_initcall instead of module_init here to ensure - for the
>>>> + * non-modular case - that the subsystem is initialized when mux consumers
>>>> + * and mux controllers start to use it /without/ relying on link order.
>>>> + * For the modular case, the ordering is ensured with module dependencies.
>>>> + */
>>>> +subsys_initcall(mux_init);
>>>
>>> Even with subsys_initcall you are relying on link order, you do realize
>>> that? What about other subsystems that rely on this? :)
>>
>> Yes, that is true, but if others start relying on this, that's their problem,
>> right? :-)
>
> Yes, but no need to document something that isn't true. You are relying
> on link order here.
Well, am I? If I change it to module_init I do get runtime errors for a
non-modular build (when mux consumers get initialized before the mux core).
With subsys_init the mux core get initialized before all (current)
consumers. I don't really see how the link order *currently* matters?
So, just making sure that we are on the same page, the thing that relies on
link order are any mux consumers/drivers that in the future may be added as
part of some other subsys_init call (or earlier). Right?
Hmmm, or are you perhaps referring to the fact that the mux core depends on
other subsystems being initialized first?
>>>> +struct mux_control_ops {
>>>> + int (*set)(struct mux_control *mux, int state);
>>>> +};
>>>> +
>>>> +/* These defines match the constants from the dt-bindings. On purpose. */
>>>
>>> Why on purpose?
>>
>> I sure wasn't an accident? :-)
>>
>> Want me to remove it?
>
> At least explain _why_ you are doing this, that would help, right?
Should I perhaps just #include <linux/dt-bindings/mux/mux.h> instead?
Is that an OK thing to do? I didn't because I feared it might come back
to haunt me at some point if the bindings header needed stuff in the
future that made it incompatible...
It's also not terribly common to include bindings from an include...
Cheers,
peda
^ permalink raw reply
* Re: [PATCH 0/4] of: remove *phandle properties from expanded device tree
From: Frank Rowand @ 2017-04-18 21:08 UTC (permalink / raw)
To: Rob Herring, Stephen Boyd
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <58F19A75.80500-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Rob,
Please do not apply this patch series.
The more context I look at, the less this approach seems good.
I hope to have a simpler version completed quickly.
Thanks,
- Frank
On 04/14/17 20:58, Frank Rowand wrote:
> Hi Stephen,
>
> I left you off the distribution list, sorry...
>
> On 04/14/17 20:55, frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>
>> Remove "phandle" and "linux,phandle" properties from the internal
>> device tree. The phandle will still be in the struct device_node
>> phandle field.
>>
>> This is to resolve the issue found by Stephen Boyd [1] when he changed
>> the type of struct property.value from void * to const void *. As
>> a result of the type change, the overlay code had compile errors
>> where the resolver updates phandle values.
>>
>> [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>>
>> Patch 1 is the phandle related changes.
>>
>> Patches 2 - 4 are minor fixups for issues that became visible
>> while implementing patch 1.
>>
>> Frank Rowand (4):
>> of: remove *phandle properties from expanded device tree
>> of: make __of_attach_node() static
>> of: be consistent in form of file mode
>> of: detect invalid phandle in overlay
>>
>> drivers/of/base.c | 53 +++++++++++++++++++++++++++++++++++++++++++++----
>> drivers/of/dynamic.c | 31 ++++++++++++++++-------------
>> drivers/of/fdt.c | 40 ++++++++++++++++++++++---------------
>> drivers/of/of_private.h | 1 -
>> drivers/of/overlay.c | 8 +++++---
>> drivers/of/resolver.c | 23 +--------------------
>> include/linux/of.h | 1 +
>> 7 files changed, 97 insertions(+), 60 deletions(-)
--
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/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE
From: Guenter Roeck @ 2017-04-18 20:58 UTC (permalink / raw)
To: Markus Mayer
Cc: Jean Delvare, Rob Herring, Mark Rutland, Florian Fainelli,
Markus Mayer, Broadcom Kernel List, Linux HWMON List,
Device Tree List, ARM Kernel List, Linux Kernel Mailing List
In-Reply-To: <20170418201702.57019-3-code@mmayer.net>
Hi Markus,
On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
>
> This driver allows access to DRAM properties, such as the refresh rate,
> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> used as indirect indicator of the DRAM temperature.
>
> The driver also allows setting of the sampling interval.
>
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
[ ... ]
> +
> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> + 1000);
> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> +static struct attribute *dpfe_attrs[] = {
> + &sensor_dev_attr_dpfe_info.dev_attr.attr,
> + &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> + &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(dpfe);
> +
There is not a single standard hwmon attribute. I don't know how
to classify this driver, and where it should reside, but it is not
a hwmon driver.
Thanks,
Guenter
^ permalink raw reply
* [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE
From: Markus Mayer @ 2017-04-18 20:17 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland,
Florian Fainelli
Cc: Markus Mayer, Broadcom Kernel List, Linux HWMON List,
Device Tree List, ARM Kernel List, Linux Kernel Mailing List
In-Reply-To: <20170418201702.57019-1-code@mmayer.net>
From: Markus Mayer <mmayer@broadcom.com>
This driver allows access to DRAM properties, such as the refresh rate,
via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
used as indirect indicator of the DRAM temperature.
The driver also allows setting of the sampling interval.
Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
MAINTAINERS | 8 +
drivers/hwmon/Kconfig | 13 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/brcmstb-dpfe.c | 689 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 711 insertions(+)
create mode 100644 drivers/hwmon/brcmstb-dpfe.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 676c139..d2d0495 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2825,6 +2825,14 @@ S: Maintained
F: Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt
F: drivers/cpufreq/brcmstb*
+BROADCOM STB DPFE HWMON DRIVER
+M: Markus Mayer <mmayer@broadcom.com>
+M: bcm-kernel-feedback-list@broadcom.com
+L: linux-hwmon@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/hwmon/brcmstb*
+F: drivers/hwmon/brcmstb*
+
BROADCOM SPECIFIC AMBA DRIVER (BCMA)
M: Rafał Miłecki <zajec5@gmail.com>
L: linux-wireless@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0649d53f3..54bb55c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -355,6 +355,19 @@ config SENSORS_ATXP1
This driver can also be built as a module. If so, the module
will be called atxp1.
+config SENSORS_BRCMSTB_DPFE
+ tristate "BRCMSTB DPFE driver"
+ depends on ARCH_BRCMSTB
+ default y
+ help
+ If you say yes here you get support for the Broadcom set-top box
+ DDR PHY Front End (DPFE) interface.
+
+ The driver allows one to query the DRAM refresh rate, which in turn,
+ provides an indirect means for deriving the DRAM temperature.
+
+ If in doubt, say N.
+
config SENSORS_DS620
tristate "Dallas Semiconductor DS620"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5509edf..cec13ec 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_ATK0110) += asus_atk0110.o
# Native drivers
# asb100, then w83781d go first, as they can override other drivers' addresses.
obj-$(CONFIG_SENSORS_ASB100) += asb100.o
+obj-$(CONFIG_SENSORS_BRCMSTB_DPFE) += brcmstb-dpfe.o
obj-$(CONFIG_SENSORS_W83627HF) += w83627hf.o
obj-$(CONFIG_SENSORS_W83792D) += w83792d.o
obj-$(CONFIG_SENSORS_W83793) += w83793.o
diff --git a/drivers/hwmon/brcmstb-dpfe.c b/drivers/hwmon/brcmstb-dpfe.c
new file mode 100644
index 0000000..01abea8
--- /dev/null
+++ b/drivers/hwmon/brcmstb-dpfe.c
@@ -0,0 +1,689 @@
+/*
+ * Temperature sensor driver for Broadcom set top box SoCs
+ *
+ * Copyright (c) 2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+#define DRVNAME "brcmstb-dpfe"
+#define FIRMWARE_NAME "dpfe.bin"
+#define DT_COMPAT_DMEM "brcm,dpfe-dmem"
+#define DT_COMPAT_IMEM "brcm,dpfe-imem"
+
+/* DCPU register offsets */
+#define REG_DCPU_RESET 0x0
+#define REG_TO_DCPU_MBOX 0x10
+#define REG_TO_HOST_MBOX 0x14
+
+/* Message RAM */
+#define DCPU_MSG_RAM(x) (0x100 + (x) * sizeof(u32))
+
+/* DRAM Info Offsets & Masks */
+#define DRAM_INFO_INTERVAL 0x0
+#define DRAM_INFO_MR4 0x4
+#define DRAM_INFO_ERROR 0x8
+#define DRAM_INFO_MASK 0xff
+
+/* DRAM MR4 Offsets & Masks */
+#define DRAM_MR4_REFRESH 0x0 /* Refresh rate */
+#define DRAM_MR4_SR_ABORT 0x3 /* Self Refresh Abort */
+#define DRAM_MR4_PPRE 0x4 /* Post-package repair entry/exit */
+#define DRAM_MR4_TH_OFFS 0x5 /* Thermal Offset; vendor specific */
+#define DRAM_MR4_TUF 0x7 /* Temperature Update Flag */
+
+#define DRAM_MR4_REFRESH_MASK 0x7
+#define DRAM_MR4_SR_ABORT_MASK 0x1
+#define DRAM_MR4_PPRE_MASK 0x1
+#define DRAM_MR4_TH_OFFS_MASK 0x3
+#define DRAM_MR4_TUF_MASK 0x1
+
+/* DRAM Vendor Offsets & Masks */
+#define DRAM_VENDOR_MR5 0x0
+#define DRAM_VENDOR_MR6 0x4
+#define DRAM_VENDOR_MR7 0x8
+#define DRAM_VENDOR_MR8 0xc
+#define DRAM_VENDOR_ERROR 0x10
+#define DRAM_VENDOR_MASK 0xff
+
+/* Reset register bits & masks */
+#define DCPU_RESET_SHIFT 0x0
+#define DCPU_RESET_MASK 0x1
+#define DCPU_CLK_DISABLE_SHIFT 0x2
+
+/* DCPU return codes */
+#define DCPU_RET_SUCCESS 0x00000001
+#define DCPU_RET_ERR_HEADER 0x80000001
+#define DCPU_RET_ERR_INVAL 0x80000002
+#define DCPU_RET_ERR_CHKSUM 0x80000004
+#define DCPU_RET_ERR_OTHER 0x80000008
+
+/* Firmware magic */
+#define DPFE_BE_MAGIC 0xfe1010fe
+#define DPFE_LE_MAGIC 0xfe0101fe
+
+/* Error codes */
+#define ERR_INVALID_MAGIC -1
+#define ERR_INVALID_SIZE -2
+#define ERR_INVALID_CHKSUM -3
+
+/* Message types */
+#define DPFE_MSG_TYPE_COMMAND 1
+#define DPFE_MSG_TYPE_RESPONSE 2
+
+#define DELAY_LOOP_MAX 200000
+
+enum dpfe_msg_fields {
+ MSG_HEADER,
+ MSG_COMMAND,
+ MSG_ARG_COUNT,
+ MSG_ARG0,
+ MSG_CHKSUM,
+ MSG_FIELD_MAX /* Last entry */
+};
+
+enum dpfe_commands {
+ DPFE_CMD_GET_INFO,
+ DPFE_CMD_GET_REFRESH,
+ DPFE_CMD_GET_VENDOR,
+ DPFE_CMD_MAX /* Last entry */
+};
+
+struct dpfe_msg {
+ u32 header;
+ u32 command;
+ u32 arg_count;
+ u32 arg0;
+ u32 chksum; /* This is the sum of all other entries. */
+};
+
+/*
+ * Format of the binary firmware file:
+ *
+ * entry
+ * 0 header
+ * value: 0xfe0101fe <== little endian
+ * 0xfe1010fe <== big endian
+ * 1 sequence:
+ * [31:16] total segments on this build
+ * [15:0] this segment sequence.
+ * 2 FW version
+ * 3 IMEM byte size
+ * 4 DMEM byte size
+ * IMEM
+ * DMEM
+ * last checksum ==> sum of everything
+ */
+struct dpfe_firmware_header {
+ u32 magic;
+ u32 sequence;
+ u32 version;
+ u32 imem_size;
+ u32 dmem_size;
+};
+
+/* Things we only need during initialization. */
+struct init_data {
+ void __iomem *dmem;
+ void __iomem *imem;
+ unsigned int dmem_len;
+ unsigned int imem_len;
+ unsigned int chksum;
+ bool is_big_endian;
+};
+
+/* Things we need for as long as we are active. */
+struct private_data {
+ void __iomem *regs;
+ void __iomem *dmem;
+ struct mutex lock;
+};
+
+/* List of supported firmware commands */
+const u32 dpfe_commands[DPFE_CMD_MAX][MSG_FIELD_MAX] = {
+ [DPFE_CMD_GET_INFO] = {
+ [MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+ [MSG_COMMAND] = 1,
+ [MSG_ARG_COUNT] = 1,
+ [MSG_ARG0] = 1,
+ [MSG_CHKSUM] = 4,
+ },
+ [DPFE_CMD_GET_REFRESH] = {
+ [MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+ [MSG_COMMAND] = 2,
+ [MSG_ARG_COUNT] = 1,
+ [MSG_ARG0] = 1,
+ [MSG_CHKSUM] = 5,
+ },
+ [DPFE_CMD_GET_VENDOR] = {
+ [MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+ [MSG_COMMAND] = 2,
+ [MSG_ARG_COUNT] = 1,
+ [MSG_ARG0] = 2,
+ [MSG_CHKSUM] = 6,
+ },
+};
+
+static u32 dpfe_readl(const void __iomem *addr)
+{
+ return le32_to_cpu(readl_relaxed(addr));
+}
+
+static void dpfe_writel(u32 value, void __iomem *addr)
+{
+ writel_relaxed(cpu_to_le32(value), addr);
+}
+
+static void __iomem *__map_region(const char *name)
+{
+ struct device_node *np;
+ void __iomem *ptr;
+
+ np = of_find_compatible_node(NULL, NULL, name);
+ if (!np)
+ return NULL;
+
+ ptr = of_iomap(np, 0);
+ of_node_put(np);
+
+ return ptr;
+}
+
+static void __disable_dcpu(void __iomem *regs)
+{
+ u32 val;
+
+ /* Check if DCPU is running */
+ val = dpfe_readl(regs + REG_DCPU_RESET);
+ if (!(val & DCPU_RESET_MASK)) {
+ /* Put DCPU in reset */
+ val |= (1 << DCPU_RESET_SHIFT);
+ dpfe_writel(val, regs + REG_DCPU_RESET);
+ }
+}
+
+static void __enable_dcpu(void __iomem *regs)
+{
+ u32 val;
+
+ /* Clear mailbox registers. */
+ dpfe_writel(0, regs + REG_TO_DCPU_MBOX);
+ dpfe_writel(0, regs + REG_TO_HOST_MBOX);
+
+ /* Disable DCPU clock gating */
+ val = dpfe_readl(regs + REG_DCPU_RESET);
+ val &= ~(1 << DCPU_CLK_DISABLE_SHIFT);
+ dpfe_writel(val, regs + REG_DCPU_RESET);
+
+ /* Take DCPU out of reset */
+ val = dpfe_readl(regs + REG_DCPU_RESET);
+ val &= ~(1 << DCPU_RESET_SHIFT);
+ dpfe_writel(val, regs + REG_DCPU_RESET);
+}
+
+static unsigned int get_msg_chksum(const u32 msg[])
+{
+ unsigned int sum = 0;
+ unsigned int i;
+
+ /* Don't include the last field in the checksum. */
+ for (i = 0; i < MSG_FIELD_MAX - 1; i++)
+ sum += msg[i];
+
+ return sum;
+}
+
+static int __send_command(struct private_data *priv, unsigned int cmd,
+ u32 result[])
+{
+ const u32 *msg = dpfe_commands[cmd];
+ void __iomem *regs = priv->regs;
+ unsigned int i, chksum;
+ int ret = 0;
+ u32 resp;
+
+ if (cmd >= DPFE_CMD_MAX)
+ return -1;
+
+ mutex_lock(&priv->lock);
+
+ /* Write command and arguments to message area */
+ for (i = 0; i < MSG_FIELD_MAX; i++)
+ dpfe_writel(msg[i], regs + DCPU_MSG_RAM(i));
+
+ /* Tell DCPU there is a command waiting */
+ dpfe_writel(1, regs + REG_TO_DCPU_MBOX);
+
+ /* Wait for DCPU to process the command */
+ for (i = 0; i < DELAY_LOOP_MAX; i++) {
+ /* Read response code */
+ resp = dpfe_readl(regs + REG_TO_HOST_MBOX);
+ if (resp > 0)
+ break;
+ udelay(5);
+ }
+ if (i == DELAY_LOOP_MAX)
+ ret = -ETIMEDOUT;
+
+ /* Read response data */
+ for (i = 0; i < MSG_FIELD_MAX; i++)
+ result[i] = dpfe_readl(regs + DCPU_MSG_RAM(i));
+
+ /* Tell DCPU we are done */
+ dpfe_writel(0, regs + REG_TO_HOST_MBOX);
+
+ mutex_unlock(&priv->lock);
+
+ if (!ret) {
+ /* Verify response */
+ chksum = get_msg_chksum(result);
+ if (chksum != result[MSG_CHKSUM])
+ ret = -1;
+ }
+
+ if (!ret) {
+ switch (resp) {
+ case DCPU_RET_SUCCESS:
+ break;
+ case DCPU_RET_ERR_HEADER:
+ case DCPU_RET_ERR_INVAL:
+ case DCPU_RET_ERR_CHKSUM:
+ case DCPU_RET_ERR_OTHER:
+ ret = -1;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+/* Ensure that the firmware file loaded meets all the requirements. */
+static int __verify_firmware(struct init_data *init,
+ const struct firmware *fw)
+{
+ const struct dpfe_firmware_header *header = (void *)fw->data;
+ unsigned int dmem_size, imem_size, total_size;
+ bool is_big_endian = false;
+ const u32 *chksum;
+
+ if (header->magic == DPFE_BE_MAGIC)
+ is_big_endian = true;
+ else if (header->magic != DPFE_LE_MAGIC)
+ return ERR_INVALID_MAGIC;
+
+ if (is_big_endian) {
+ dmem_size = be32_to_cpu(header->dmem_size);
+ imem_size = be32_to_cpu(header->imem_size);
+ } else {
+ dmem_size = header->dmem_size;
+ imem_size = header->imem_size;
+ }
+
+ /* Data and instruction sections are 32 bit words. */
+ if ((dmem_size % sizeof(u32)) != 0 || (imem_size % sizeof(u32)) != 0)
+ return ERR_INVALID_SIZE;
+
+ /*
+ * The header + the data section + the instruction section + the
+ * checksum must be equal to the total firmware size.
+ */
+ total_size = dmem_size + imem_size + sizeof(*header) + sizeof(*chksum);
+ if (total_size != fw->size)
+ return ERR_INVALID_SIZE;
+
+ /* The checksum comes at the very end. */
+ chksum = (void *)fw->data + sizeof(*header) + dmem_size + imem_size;
+
+ init->is_big_endian = is_big_endian;
+ init->dmem_len = dmem_size;
+ init->imem_len = imem_size;
+ init->chksum = (is_big_endian) ? be32_to_cpu(*chksum) : *chksum;
+
+ return 0;
+}
+
+/* Verify checksum by reading back the firmware from co-processor RAM. */
+static int __verify_fw_checksum(struct init_data *init,
+ const struct dpfe_firmware_header *header,
+ u32 checksum)
+{
+ u32 magic, sequence, version, sum;
+ u32 __iomem *dmem = init->dmem;
+ u32 __iomem *imem = init->imem;
+ unsigned int i;
+
+ if (init->is_big_endian) {
+ magic = be32_to_cpu(header->magic);
+ sequence = be32_to_cpu(header->sequence);
+ version = be32_to_cpu(header->version);
+ } else {
+ magic = header->magic;
+ sequence = header->sequence;
+ version = header->version;
+ }
+
+ sum = magic + sequence + version + init->dmem_len + init->imem_len;
+
+ for (i = 0; i < init->dmem_len / sizeof(u32); i++)
+ sum += dpfe_readl(dmem + i);
+
+ for (i = 0; i < init->imem_len / sizeof(u32); i++)
+ sum += dpfe_readl(imem + i);
+
+ return (sum == checksum) ? 0 : -1;
+}
+
+static int __write_firmware(u32 __iomem *mem, const u32 *fw,
+ unsigned int size, bool is_big_endian)
+{
+ unsigned int i;
+
+ /* Convert size to 32-bit words. */
+ size /= sizeof(u32);
+
+ /* It is recommended to clear the firmware area first. */
+ for (i = 0; i < size; i++)
+ dpfe_writel(0, mem + i);
+
+ /* Now copy it. */
+ if (is_big_endian) {
+ for (i = 0; i < size; i++)
+ dpfe_writel(be32_to_cpu(fw[i]), mem + i);
+ } else {
+ for (i = 0; i < size; i++)
+ dpfe_writel(fw[i], mem + i);
+ }
+
+ return 0;
+}
+
+static int brcmstb_hwmon_download_firwmare(struct platform_device *pdev,
+ struct init_data *init)
+{
+ const struct dpfe_firmware_header *header;
+ unsigned int dmem_size, imem_size;
+ struct device *dev = &pdev->dev;
+ bool is_big_endian = false;
+ struct private_data *priv;
+ const struct firmware *fw;
+ const u32 *dmem, *imem;
+ const void *fw_blob;
+ int ret;
+
+ ret = request_firmware(&fw, FIRMWARE_NAME, dev);
+ /* request_firmware() prints its own error messages. */
+ if (ret)
+ return ret;
+
+ priv = platform_get_drvdata(pdev);
+
+ ret = __verify_firmware(init, fw);
+ if (ret)
+ return -EFAULT;
+
+ __disable_dcpu(priv->regs);
+
+ is_big_endian = init->is_big_endian;
+ dmem_size = init->dmem_len;
+ imem_size = init->imem_len;
+
+ /* At the beginning of the firmware blob is a header. */
+ header = (struct dpfe_firmware_header *)fw->data;
+ /* Void pointer to the beginning of the actual firmware. */
+ fw_blob = fw->data + sizeof(*header);
+ /* IMEM comes right after the header. */
+ imem = fw_blob;
+ /* DMEM follows after IMEM. */
+ dmem = fw_blob + imem_size;
+
+ ret = __write_firmware(init->dmem, dmem, dmem_size, is_big_endian);
+ if (ret)
+ return ret;
+ ret = __write_firmware(init->imem, imem, imem_size, is_big_endian);
+ if (ret)
+ return ret;
+
+ ret = __verify_fw_checksum(init, header, init->chksum);
+ if (ret)
+ return ret;
+
+ __enable_dcpu(priv->regs);
+
+ return 0;
+}
+
+static ssize_t generic_show(unsigned int command, u32 response[],
+ struct device *dev, char *buf)
+{
+ struct private_data *priv;
+ int ret;
+
+ priv = dev_get_drvdata(dev);
+
+ ret = __send_command(priv, command, response);
+ if (ret)
+ return sprintf(buf, "error %d\n", ret);
+
+ return 0;
+}
+
+static ssize_t show_info(struct device *dev, struct device_attribute *devattr,
+ char *buf)
+{
+ u32 response[MSG_FIELD_MAX];
+ unsigned int info;
+ int ret;
+
+ ret = generic_show(DPFE_CMD_GET_INFO, response, dev, buf);
+ if (ret)
+ return ret;
+
+ info = response[MSG_ARG0];
+
+ return sprintf(buf, "%u.%u.%u.%u\n",
+ (info >> 24) & 0xff,
+ (info >> 16) & 0xff,
+ (info >> 8) & 0xff,
+ info & 0xff);
+}
+
+static ssize_t show_refresh(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ u32 response[MSG_FIELD_MAX];
+ void __iomem *info;
+ struct private_data *priv;
+ unsigned int offset;
+ u8 refresh, sr_abort, ppre, thermal_offs, tuf;
+ u32 mr4;
+ int ret;
+
+ ret = generic_show(DPFE_CMD_GET_REFRESH, response, dev, buf);
+ if (ret)
+ return ret;
+
+ priv = dev_get_drvdata(dev);
+ offset = response[MSG_ARG0];
+ info = priv->dmem + offset;
+
+ mr4 = dpfe_readl(info + DRAM_INFO_MR4) & DRAM_INFO_MASK;
+
+ refresh = (mr4 >> DRAM_MR4_REFRESH) & DRAM_MR4_REFRESH_MASK;
+ sr_abort = (mr4 >> DRAM_MR4_SR_ABORT) & DRAM_MR4_SR_ABORT_MASK;
+ ppre = (mr4 >> DRAM_MR4_PPRE) & DRAM_MR4_PPRE_MASK;
+ thermal_offs = (mr4 >> DRAM_MR4_TH_OFFS) & DRAM_MR4_TH_OFFS_MASK;
+ tuf = (mr4 >> DRAM_MR4_TUF) & DRAM_MR4_TUF_MASK;
+
+ return sprintf(buf, "%#x %#x %#x %#x %#x %#x %#x\n",
+ dpfe_readl(info + DRAM_INFO_INTERVAL),
+ refresh, sr_abort, ppre, thermal_offs, tuf,
+ dpfe_readl(info + DRAM_INFO_ERROR));
+}
+
+static ssize_t store_refresh(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u32 response[MSG_FIELD_MAX];
+ struct private_data *priv;
+ void __iomem *info;
+ unsigned int offset;
+ unsigned long val;
+ int ret;
+
+ if (kstrtoul(buf, 0, &val) < 0)
+ return -EINVAL;
+
+ priv = dev_get_drvdata(dev);
+
+ ret = __send_command(priv, DPFE_CMD_GET_REFRESH, response);
+ if (ret)
+ return ret;
+
+ offset = response[MSG_ARG0];
+ info = priv->dmem + offset + DRAM_MR4_REFRESH;
+ dpfe_writel(val, info);
+
+ return count;
+}
+
+
+static ssize_t show_vendor(struct device *dev, struct device_attribute *devattr,
+ char *buf)
+{
+ u32 response[MSG_FIELD_MAX];
+ struct private_data *priv;
+ void __iomem *info;
+ unsigned int offset;
+ int ret;
+
+ ret = generic_show(DPFE_CMD_GET_VENDOR, response, dev, buf);
+ if (ret)
+ return ret;
+
+ offset = response[MSG_ARG0];
+ priv = dev_get_drvdata(dev);
+ info = priv->dmem + offset;
+
+ return sprintf(buf, "%#x %#x %#x %#x %#x\n",
+ dpfe_readl(info + DRAM_VENDOR_MR5) & DRAM_VENDOR_MASK,
+ dpfe_readl(info + DRAM_VENDOR_MR6) & DRAM_VENDOR_MASK,
+ dpfe_readl(info + DRAM_VENDOR_MR7) & DRAM_VENDOR_MASK,
+ dpfe_readl(info + DRAM_VENDOR_MR8) & DRAM_VENDOR_MASK,
+ dpfe_readl(info + DRAM_VENDOR_ERROR));
+}
+
+static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
+static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
+ 1000);
+static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
+static struct attribute *dpfe_attrs[] = {
+ &sensor_dev_attr_dpfe_info.dev_attr.attr,
+ &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
+ &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(dpfe);
+
+static int brcmstb_hwmon_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device *hwmon_dev = NULL;
+ struct private_data *priv;
+ struct init_data init;
+ struct resource *res;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mutex_init(&priv->lock);
+ platform_set_drvdata(pdev, priv);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(priv->regs)) {
+ dev_err(dev, "couldn't map DT entry brcm,dpfe-cpu\n");
+ return -ENODEV;
+ }
+
+ init.dmem = __map_region(DT_COMPAT_DMEM);
+ if (!init.dmem) {
+ dev_err(dev, "Couldn't map %s\n", DT_COMPAT_DMEM);
+ return -ENOENT;
+ }
+ init.imem = __map_region(DT_COMPAT_IMEM);
+ if (init.imem) {
+ ret = brcmstb_hwmon_download_firwmare(pdev, &init);
+ } else {
+ ret = -ENOENT;
+ dev_err(dev, "Couldn't map %s\n", DT_COMPAT_IMEM);
+ }
+
+ /* We don't need IMEM after initialization. */
+ iounmap(init.imem);
+
+ if (!ret) {
+ hwmon_dev = devm_hwmon_device_register_with_groups(dev,
+ "brcmstb_dpfe", priv, dpfe_groups);
+ if (IS_ERR(hwmon_dev))
+ ret = PTR_ERR(hwmon_dev);
+ }
+
+ if (ret) {
+ iounmap(init.dmem);
+ dev_err(dev, "failed to initialize -- error %d\n", ret);
+ } else {
+ priv->dmem = init.dmem;
+ dev_info(dev, "registered.\n");
+ }
+
+ return ret;
+}
+
+static int brcmstb_hwmon_remove(struct platform_device *pdev)
+{
+ struct private_data *priv;
+
+ priv = platform_get_drvdata(pdev);
+ iounmap(priv->dmem);
+
+ return 0;
+}
+
+static const struct of_device_id brcmstb_hwmon_of_match[] = {
+ { .compatible = "brcm,dpfe-cpu", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, brcmstb_hwmon_of_match);
+
+static struct platform_driver brcmstb_hwmon_driver = {
+ .driver = {
+ .name = DRVNAME,
+ .of_match_table = brcmstb_hwmon_of_match,
+ },
+ .probe = brcmstb_hwmon_probe,
+ .remove = brcmstb_hwmon_remove,
+};
+
+module_platform_driver(brcmstb_hwmon_driver);
+
+MODULE_AUTHOR("Markus Mayer <mmayer@broadcom.com>");
+MODULE_DESCRIPTION("BRCMSTB Hardware Monitoring");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors
From: Markus Mayer @ 2017-04-18 20:17 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland,
Florian Fainelli
Cc: Markus Mayer, Broadcom Kernel List, Linux HWMON List,
Device Tree List, ARM Kernel List, Linux Kernel Mailing List
In-Reply-To: <20170418201702.57019-1-code@mmayer.net>
From: Markus Mayer <mmayer@broadcom.com>
Provide bindings for the Broadcom STB DDR PHY Front End (DPFE).
Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
.../devicetree/bindings/hwmon/brcmstb-dpfe.txt | 68 ++++++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
diff --git a/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
new file mode 100644
index 0000000..3519197
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
@@ -0,0 +1,68 @@
+DDR PHY Front End (DPFE) for Broadcom STB
+=========================================
+
+DPFE and the DPFE firmware provide an interface for the host CPU to
+communicate with the DCPU, which resides inside the DDR PHY.
+
+There are three memory regions for interacting with the DCPU.
+
+The DCPU Register Space
+-----------------------
+
+Required properties:
+ - compatible: must be one of brcm,bcm7271-dpfe-cpu, brcm,dpfe-cpu-v12.0.0.0
+ or brcm,dpfe-cpu
+ - reg: must reference the start address and length of the DCPU register
+ space
+
+Optional properties:
+ - cell-index: the index of the DPFE instance; will default to 0 if not set
+
+Example:
+ dpfe_cpu0: dpfe-cpu@f1132000 {
+ compatible = "brcm,bcm7271-dpfe-cpu",
+ "brcm,dpfe-cpu-v12.0.0.0",
+ "brcm,dpfe-cpu";
+ reg = <0xf1132000 0x180>;
+ cell-index = <0>;
+ };
+
+The DCPU Data Memory Space
+--------------------------
+
+Required properties:
+ - compatible: must be one of brcm,bcm7271-dpfe-dmem, brcm,dpfe-dmem-v12.0.0.0
+ or brcm,dpfe-dmem
+ - reg: must reference the start address and length of the DCPU DMEM space
+
+Optional properties:
+ - cell-index: the index of the DPFE instance; will default to 0 if not set
+
+Example:
+ dpfe_dmem0: dpfe-dmem@f1134000 {
+ compatible = "brcm,bcm7271-dpfe-dmem",
+ "brcm,dpfe-dmem-v12.0.0.0",
+ "brcm,dpfe-dmem";
+ reg = <0xf1134000 0x1000>;
+ cell-index = <0>;
+ };
+
+The DCPU Instruction Memory Space
+---------------------------------
+
+Required properties:
+ - compatible: must be one of brcm,bcm7271-dpfe-imem, brcm,dpfe-imem-v12.0.0.0
+ or brcm,dpfe-imem
+ - reg: must reference the start address and length of the DCPU IMEM space
+
+Optional properties:
+ - cell-index: the index of the DPFE instance; will default to 0 if not set
+
+Example:
+ dpfe_imem0: dpfe-imem@f1138000 {
+ compatible = "brcm,bcm7271-dpfe-imem",
+ "brcm,dpfe-imem-v12.0.0.0",
+ "brcm,dpfe-imem";
+ reg = <0xf1138000 0x4000>;
+ cell-index = <0>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] HWMON driver for Broadcom STB DPFE
From: Markus Mayer @ 2017-04-18 20:17 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland,
Florian Fainelli
Cc: Markus Mayer, Broadcom Kernel List, Linux HWMON List,
Device Tree List, ARM Kernel List, Linux Kernel Mailing List
From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
This series introduces a driver to interact with the Broadcom STB DDR
PHY Front End (DPFE), specifically to communicate with the DCPU that is
part of the DDR PHY and which is running its own firmware.
The DCPU provides information such as DRAM refresh rate, which can be
used as indirect indicator for the DRAM temperature (the higher the
refresh rate, the hotter the RAM).
Markus Mayer (2):
dt/bindings: Add bindings for Broadcom STB DRAM Sensors
hwmon: (brcmstb) Add driver for Broadcom STB DPFE
.../devicetree/bindings/hwmon/brcmstb-dpfe.txt | 68 ++
MAINTAINERS | 8 +
drivers/hwmon/Kconfig | 13 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/brcmstb-dpfe.c | 689 +++++++++++++++++++++
5 files changed, 779 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
create mode 100644 drivers/hwmon/brcmstb-dpfe.c
--
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] arm64: dts: allwinner: a64: Add UART2 pin nodes
From: Andreas Färber @ 2017-04-18 19:32 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chen-Yu Tsai,
Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170418053458.phcbp4da3vatejpg@lukather>
Am 18.04.2017 um 07:34 schrieb Maxime Ripard:
> On Fri, Apr 14, 2017 at 07:13:20PM +0200, Andreas Färber wrote:
>> UART2 is exposed on the Pi connector of Pine64. Make a pinctrl node
>> available at the SoC level, to simplify enabling UART2 via DT overlay.
>>
>> Signed-off-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
>
> Applied, thanks!
Thanks, that was fast.
> Could you also create the node in the pine64 DTS (with an explicit
> status = "disabled" in there) so that the overlay would be even easier
> to make ?
Makes sense. I went ahead and did it for all UART nodes then - careful,
final patch is neither build- nor runtime-tested yet.
Cheers,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
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 v3] arm64: dts: rk3399: add support for firefly-rk3399 board
From: Marc Zyngier @ 2017-04-18 19:31 UTC (permalink / raw)
To: Kever Yang
Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, Brian Norris,
Will Deacon, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Matthias Brugger, Andy Yan, Jianqun Xu, Liang Chen
In-Reply-To: <1491796213-4115-1-git-send-email-kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Mon, 10 Apr 2017 11:50:13 +0800
Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Firefly-rk3399 is a bord from T-Firefly, you can find detail about
> it here:
> http://en.t-firefly.com/en/firenow/Firefly_RK3399/
>
> This patch add basic node for the board and make it able to bring
> up.
>
> Peripheral works:
> - usb hub which connect to ehci controller;
> - UART2 debug
> - eMMC
> - PCIe
>
> Not work:
> - USB 3.0 HOST, type-C port
> - sdio, sd-card
>
> Not test for other peripheral:
[...]
> - Ethernet
[...]
FWIW, I've tested the Ethernet port, which seems to work nicely.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
--
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