* Re: [PATCH 3/6] dt-bindings: correct marvell orion MDIO binding document
From: Andrew Lunn @ 2017-04-10 19:28 UTC (permalink / raw)
To: Russell King
Cc: Marcin Wojtas, Stefan Chulski, Thomas Petazzoni, Rob Herring,
Mark Rutland, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <E1cxbEl-0006gK-1Q-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
On Mon, Apr 10, 2017 at 04:28:15PM +0100, Russell King wrote:
> Correct the Marvell Orion MDIO binding document to properly reflect the
> cases where an interrupt is present. Augment the examples to show this.
>
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
Andrew
--
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/2] leds: Add driver for Qualcomm LPG
From: Bjorn Andersson @ 2017-04-10 19:19 UTC (permalink / raw)
To: Pavel Machek
Cc: Jacek Anaszewski, Rob Herring, Richard Purdie, linux-kernel,
linux-leds, linux-arm-msm, Mark Rutland, devicetree
In-Reply-To: <20170408133904.GA9020@amd>
On Sat 08 Apr 06:39 PDT 2017, Pavel Machek wrote:
> Hi!
>
> > [..]
> > > > For the patterns I don't know how a trigger for this would look like,
> > > > how would setting the pattern of a trigger be propagated down to the
> > > > hardware?
> > >
> > > We'd need a new op and API similar to blink_set()/led_blink_set().
> > >
> >
> > I've tried to find different LED circuits with some sort of pattern
> > generator in an attempt to figure out how to design this interface, but
> > turned out to be quite hard to find examples; the three I can compare
> > are:
> >
> > * LP5xx series "implements" pattern generation by executing code.
> >
> > * Qualcomm LPG iterates over 2-64 brightness-values in a pattern, at a
> > fixed rate with knobs to configure what happens before starting and
> > after finishing iterating over the defined values. It does not support
> > smooth transitions between values.
> >
> > * AS3676 supports a pattern of 32 values controlling if the output
> > should be enabled or disabled for each 32.5ms (or 250ms) time period.
> > The delay before repeating the pattern can be configured. It support
> > smooth transitions between the states.
> >
> >
> > So, while I think I see how you would like to architect this interface I
> > am not sure how to figure out the details.
> >
> > The pattern definition would have to be expressive enough to support the
> > features of LP5xx and direct enough to support the limited AS3676. It
> > would likely have to express transitions, so that the LPG could generate
> > intermediate steps (and we will have to adapt the resolution of the
> > ramps based on the other LPGs in the system).
> >
> > How do we do with patterns that are implementable by the LP5xx but are
> > not with the LPG? Should we reject those or should we do some sort of
> > best-effort approach in the kernel?
>
> Lets say you get series of
>
> (red, green, blue, delta_t )
>
> points, meaning "in delta_t msec, change color to red, green,
> blue. Lets ignore other channels for now. delta_t of 0 would be step
> change. Would such interface work for you?
>
So I presume this would be input to the RGB trigger that we discussed.
But in my current device I have 6 LEDs, that are not in any RGB-like
configuration. So we would need to come up with an interface that looks
to be the same in both single-LED and RGB-LED setups.
This should be sufficient to describe a subset of the patterns I've seen
so far in products.
But let's consider the standard use case for an RGB LED on an Android
phone; continuously blinking (pulsing based on patterns) as you have
some notifications waiting. In this case you want the LED hardware to do
all the work, so that you can deep-idle the CPU. So we would need to
introduce a "repeat pattern"-command.
Then consider the fact that you want your patterns to have decent
resolution, but you have a limited amount of storage. So we either have
to be able to detect palindromes or have a way to represent this.
> Simple compiler from this to LP5XX code should not be hard to
> do.
It sounds fairly straight forward to convert a pattern to instructions,
but we do have an extremely limited amount of storage so it must be a
quite good implementation for people to be able to use it for anything
real.
We could implement some optimization steps where we try to detect slopes
and generate ramp-instructions instead of set-pwm + wait instructions,
use some variables to handle ramp up/down and we could probably generate
some jump instructions to implement loops.
But do we really want this logic in the kernel, for each LED chip
supporting patterns?
> AS3676 ... I'm not sure what to do, AFAICT it is too limited.
>
So out of the three examples I've looked at we're skipping one and we're
abstracting away most functionality from another.
I'm sorry for being pessimistic about this, but while I can see the
theoretical benefit of providing a uniform interface for this to user
space I see three very different pieces of hardware that would be used
in three different ways in products.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v4 3/9] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: jmondi @ 2017-04-10 19:19 UTC (permalink / raw)
To: Rob Herring
Cc: Jacopo Mondi, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170410181215.e6cihbv2rfljbm3b@rob-hp-laptop>
Hi Rob,
On Mon, Apr 10, 2017 at 01:12:15PM -0500, Rob Herring wrote:
> On Wed, Apr 05, 2017 at 04:07:21PM +0200, Jacopo Mondi wrote:
> > Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> > controller.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> > ---
> > .../bindings/pinctrl/renesas,rza1-pinctrl.txt | 218 +++++++++++++++++++++
> > 1 file changed, 218 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> > new file mode 100644
> > index 0000000..46584ef
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> > @@ -0,0 +1,218 @@
> > +Renesas RZ/A1 combined Pin and GPIO controller
> > +
> > +The Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller,
> > +named "Ports" in the hardware reference manual.
> > +Pin multiplexing and GPIO configuration is performed on a per-pin basis
> > +writing configuration values to per-port register sets.
> > +Each "port" features up to 16 pins, each of them configurable for GPIO
> > +function (port mode) or in alternate function mode.
> > +Up to 8 different alternate function modes exist for each single pin.
> > +
> > +Pin controller node
> > +-------------------
> > +
> > +Required properties:
> > + - compatible
> > + this shall be "renesas,r7s72100-ports".
> > +
> > + - reg
> > + address base and length of the memory area where pin controller
> > + hardware is mapped to.
> > +
> > +Example:
> > +Pin controller node for RZ/A1H SoC (r7s72100)
> > +
> > +pinctrl: pin-controller@fcfe3000 {
> > + compatible = "renesas,r7s72100-ports";
> > +
> > + reg = <0xfcfe3000 0x4230>;
> > +};
> > +
> > +Sub-nodes
> > +---------
> > +
> > +The child nodes of the pin controller node describe a pin multiplexing
> > +function or a gpio controller alternatively.
> > +
> > +- Pin multiplexing sub-nodes:
> > + A pin multiplexing sub-node describes how to configure a set of
> > + (or a single) pin in some desired alternate function mode.
> > + A single sub-node may define several pin configurations.
> > + Some alternate functions require special pin configuration flags to be
> > + supplied along with the alternate function configuration number.
> > + When hardware reference manual specifies a pin function to be either
> > + "bi-directional" or "software IO driven", use the generic properties from
> > + <include/linux/pinctrl/pinconf_generic.h> header file to instruct the
> > + pin controller to perform the desired pin configuration operations.
> > + Please refer to pinctrl-bindings.txt to get to know more on generic
> > + pin properties usage.
> > +
> > + The allowed generic formats for a pin multiplexing sub-node are the
> > + following ones:
> > +
> > + node-1 {
> > + pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > + GENERIC_PINCONFIG;
>
> What's GENERIC_PINCONFIG? I see this in some other binding docs, but not
> used anywhere. If this is a boolean property then get rid of the all
> caps. If this is a define, then don't use complex defines that expand to
> dts source.
GENERIC_PINCONF is a wildcard that identifies "generic" pin
configuration properties the pin controller framework defines.
Have a look at "enum pin_config_param" in
<include/linux/pinctrl/pinconf-generic.h>
Thanks
j
--
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
* Applied "regulator: Add driver for voltage controlled regulators" to the regulator tree
From: Mark Brown @ 2017-04-10 18:43 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: Rob Herring, Mark Brown, Liam Girdwood
In-Reply-To: <20170407195158.63546-1-mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
The patch
regulator: Add driver for voltage controlled regulators
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 9dee7a72d0c7cdfa2573c48b1e5f928c721d54d5 Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Date: Fri, 7 Apr 2017 12:51:58 -0700
Subject: [PATCH] regulator: Add driver for voltage controlled regulators
The output voltage of a voltage controlled regulator can be controlled
through the voltage of another regulator. The current version of this
driver assumes that the output voltage is a linear function of the control
voltage.
Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
.../devicetree/bindings/regulator/vctrl.txt | 49 ++
drivers/regulator/Kconfig | 7 +
drivers/regulator/Makefile | 1 +
drivers/regulator/vctrl-regulator.c | 546 +++++++++++++++++++++
4 files changed, 603 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/vctrl.txt
create mode 100644 drivers/regulator/vctrl-regulator.c
diff --git a/Documentation/devicetree/bindings/regulator/vctrl.txt b/Documentation/devicetree/bindings/regulator/vctrl.txt
new file mode 100644
index 000000000000..601328d7fdbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/vctrl.txt
@@ -0,0 +1,49 @@
+Bindings for Voltage controlled regulators
+==========================================
+
+Required properties:
+--------------------
+- compatible : must be "vctrl-regulator".
+- regulator-min-microvolt : smallest voltage consumers may set
+- regulator-max-microvolt : largest voltage consumers may set
+- ctrl-supply : The regulator supplying the control voltage.
+- ctrl-voltage-range : an array of two integer values describing the range
+ (min/max) of the control voltage. The values specify
+ the control voltage needed to generate the corresponding
+ regulator-min/max-microvolt output voltage.
+
+Optional properties:
+--------------------
+- ovp-threshold-percent : overvoltage protection (OVP) threshold of the
+ regulator in percent. Some regulators have an OVP
+ circuitry which shuts down the regulator when the
+ actual output voltage deviates beyond a certain
+ margin from the expected value for a given control
+ voltage. On larger voltage decreases this can occur
+ undesiredly since the output voltage does not adjust
+ inmediately to changes in the control voltage. To
+ avoid this situation the vctrl driver breaks down
+ larger voltage decreases into multiple steps, where
+ each step is within the OVP threshold.
+- min-slew-down-rate : Describes how slowly the regulator voltage will decay
+ down in the worst case (lightest expected load).
+ Specified in uV / us (like main regulator ramp rate).
+ This value is required when ovp-threshold-percent is
+ specified.
+
+Example:
+
+ vctrl-reg {
+ compatible = "vctrl-regulator";
+ regulator-name = "vctrl_reg";
+
+ ctrl-supply = <&ctrl_reg>;
+
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <1500000>;
+
+ ctrl-voltage-range = <200000 500000>;
+
+ min-slew-down-rate = <225>;
+ ovp-threshold-percent = <16>;
+ };
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 936f7ccc9736..da83a3abe288 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -843,6 +843,13 @@ config REGULATOR_TWL4030
This driver supports the voltage regulators provided by
this family of companion chips.
+config REGULATOR_VCTRL
+ tristate "Voltage controlled regulators"
+ depends on OF
+ help
+ This driver provides support for voltage regulators whose output
+ voltage is controlled by the voltage of another regulator.
+
config REGULATOR_VEXPRESS
tristate "Versatile Express regulators"
depends on VEXPRESS_CONFIG
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 14294692beb9..e246e148a7f9 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
obj-$(CONFIG_REGULATOR_TPS65912) += tps65912-regulator.o
obj-$(CONFIG_REGULATOR_TPS80031) += tps80031-regulator.o
obj-$(CONFIG_REGULATOR_TWL4030) += twl-regulator.o twl6030-regulator.o
+obj-$(CONFIG_REGULATOR_VCTRL) += vctrl-regulator.o
obj-$(CONFIG_REGULATOR_VEXPRESS) += vexpress-regulator.o
obj-$(CONFIG_REGULATOR_WM831X) += wm831x-dcdc.o
obj-$(CONFIG_REGULATOR_WM831X) += wm831x-isink.o
diff --git a/drivers/regulator/vctrl-regulator.c b/drivers/regulator/vctrl-regulator.c
new file mode 100644
index 000000000000..6baadef0ed74
--- /dev/null
+++ b/drivers/regulator/vctrl-regulator.c
@@ -0,0 +1,546 @@
+/*
+ * Driver for voltage controller regulators
+ *
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/sort.h>
+
+struct vctrl_voltage_range {
+ int min_uV;
+ int max_uV;
+};
+
+struct vctrl_voltage_ranges {
+ struct vctrl_voltage_range ctrl;
+ struct vctrl_voltage_range out;
+};
+
+struct vctrl_voltage_table {
+ int ctrl;
+ int out;
+ int ovp_min_sel;
+};
+
+struct vctrl_data {
+ struct regulator_dev *rdev;
+ struct regulator_desc desc;
+ struct regulator *ctrl_reg;
+ bool enabled;
+ unsigned int min_slew_down_rate;
+ unsigned int ovp_threshold;
+ struct vctrl_voltage_ranges vrange;
+ struct vctrl_voltage_table *vtable;
+ unsigned int sel;
+};
+
+static int vctrl_calc_ctrl_voltage(struct vctrl_data *vctrl, int out_uV)
+{
+ struct vctrl_voltage_range *ctrl = &vctrl->vrange.ctrl;
+ struct vctrl_voltage_range *out = &vctrl->vrange.out;
+
+ return ctrl->min_uV +
+ DIV_ROUND_CLOSEST_ULL((s64)(out_uV - out->min_uV) *
+ (ctrl->max_uV - ctrl->min_uV),
+ out->max_uV - out->min_uV);
+}
+
+static int vctrl_calc_output_voltage(struct vctrl_data *vctrl, int ctrl_uV)
+{
+ struct vctrl_voltage_range *ctrl = &vctrl->vrange.ctrl;
+ struct vctrl_voltage_range *out = &vctrl->vrange.out;
+
+ if (ctrl_uV < 0) {
+ pr_err("vctrl: failed to get control voltage\n");
+ return ctrl_uV;
+ }
+
+ if (ctrl_uV < ctrl->min_uV)
+ return out->min_uV;
+
+ if (ctrl_uV > ctrl->max_uV)
+ return out->max_uV;
+
+ return out->min_uV +
+ DIV_ROUND_CLOSEST_ULL((s64)(ctrl_uV - ctrl->min_uV) *
+ (out->max_uV - out->min_uV),
+ ctrl->max_uV - ctrl->min_uV);
+}
+
+static int vctrl_get_voltage(struct regulator_dev *rdev)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+ int ctrl_uV = regulator_get_voltage(vctrl->ctrl_reg);
+
+ return vctrl_calc_output_voltage(vctrl, ctrl_uV);
+}
+
+static int vctrl_set_voltage(struct regulator_dev *rdev,
+ int req_min_uV, int req_max_uV,
+ unsigned int *selector)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+ struct regulator *ctrl_reg = vctrl->ctrl_reg;
+ int orig_ctrl_uV = regulator_get_voltage(ctrl_reg);
+ int uV = vctrl_calc_output_voltage(vctrl, orig_ctrl_uV);
+ int ret;
+
+ if (req_min_uV >= uV || !vctrl->ovp_threshold)
+ /* voltage rising or no OVP */
+ return regulator_set_voltage(
+ ctrl_reg,
+ vctrl_calc_ctrl_voltage(vctrl, req_min_uV),
+ vctrl_calc_ctrl_voltage(vctrl, req_max_uV));
+
+ while (uV > req_min_uV) {
+ int max_drop_uV = (uV * vctrl->ovp_threshold) / 100;
+ int next_uV;
+ int next_ctrl_uV;
+ int delay;
+
+ /* Make sure no infinite loop even in crazy cases */
+ if (max_drop_uV == 0)
+ max_drop_uV = 1;
+
+ next_uV = max_t(int, req_min_uV, uV - max_drop_uV);
+ next_ctrl_uV = vctrl_calc_ctrl_voltage(vctrl, next_uV);
+
+ ret = regulator_set_voltage(ctrl_reg,
+ next_ctrl_uV,
+ next_ctrl_uV);
+ if (ret)
+ goto err;
+
+ delay = DIV_ROUND_UP(uV - next_uV, vctrl->min_slew_down_rate);
+ usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
+
+ uV = next_uV;
+ }
+
+ return 0;
+
+err:
+ /* Try to go back to original voltage */
+ regulator_set_voltage(ctrl_reg, orig_ctrl_uV, orig_ctrl_uV);
+
+ return ret;
+}
+
+static int vctrl_get_voltage_sel(struct regulator_dev *rdev)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+
+ return vctrl->sel;
+}
+
+static int vctrl_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int selector)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+ struct regulator *ctrl_reg = vctrl->ctrl_reg;
+ unsigned int orig_sel = vctrl->sel;
+ int ret;
+
+ if (selector >= rdev->desc->n_voltages)
+ return -EINVAL;
+
+ if (selector >= vctrl->sel || !vctrl->ovp_threshold) {
+ /* voltage rising or no OVP */
+ ret = regulator_set_voltage(ctrl_reg,
+ vctrl->vtable[selector].ctrl,
+ vctrl->vtable[selector].ctrl);
+ if (!ret)
+ vctrl->sel = selector;
+
+ return ret;
+ }
+
+ while (vctrl->sel != selector) {
+ unsigned int next_sel;
+ int delay;
+
+ if (selector >= vctrl->vtable[vctrl->sel].ovp_min_sel)
+ next_sel = selector;
+ else
+ next_sel = vctrl->vtable[vctrl->sel].ovp_min_sel;
+
+ ret = regulator_set_voltage(ctrl_reg,
+ vctrl->vtable[next_sel].ctrl,
+ vctrl->vtable[next_sel].ctrl);
+ if (ret) {
+ dev_err(&rdev->dev,
+ "failed to set control voltage to %duV\n",
+ vctrl->vtable[next_sel].ctrl);
+ goto err;
+ }
+ vctrl->sel = next_sel;
+
+ delay = DIV_ROUND_UP(vctrl->vtable[vctrl->sel].out -
+ vctrl->vtable[next_sel].out,
+ vctrl->min_slew_down_rate);
+ usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
+ }
+
+ return 0;
+
+err:
+ if (vctrl->sel != orig_sel) {
+ /* Try to go back to original voltage */
+ if (!regulator_set_voltage(ctrl_reg,
+ vctrl->vtable[orig_sel].ctrl,
+ vctrl->vtable[orig_sel].ctrl))
+ vctrl->sel = orig_sel;
+ else
+ dev_warn(&rdev->dev,
+ "failed to restore original voltage\n");
+ }
+
+ return ret;
+}
+
+static int vctrl_list_voltage(struct regulator_dev *rdev,
+ unsigned int selector)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+
+ if (selector >= rdev->desc->n_voltages)
+ return -EINVAL;
+
+ return vctrl->vtable[selector].out;
+}
+
+static int vctrl_parse_dt(struct platform_device *pdev,
+ struct vctrl_data *vctrl)
+{
+ int ret;
+ struct device_node *np = pdev->dev.of_node;
+ u32 pval;
+ u32 vrange_ctrl[2];
+
+ vctrl->ctrl_reg = devm_regulator_get(&pdev->dev, "ctrl");
+ if (IS_ERR(vctrl->ctrl_reg))
+ return PTR_ERR(vctrl->ctrl_reg);
+
+ ret = of_property_read_u32(np, "ovp-threshold-percent", &pval);
+ if (!ret) {
+ vctrl->ovp_threshold = pval;
+ if (vctrl->ovp_threshold > 100) {
+ dev_err(&pdev->dev,
+ "ovp-threshold-percent (%u) > 100\n",
+ vctrl->ovp_threshold);
+ return -EINVAL;
+ }
+ }
+
+ ret = of_property_read_u32(np, "min-slew-down-rate", &pval);
+ if (!ret) {
+ vctrl->min_slew_down_rate = pval;
+
+ /* We use the value as int and as divider; sanity check */
+ if (vctrl->min_slew_down_rate == 0) {
+ dev_err(&pdev->dev,
+ "min-slew-down-rate must not be 0\n");
+ return -EINVAL;
+ } else if (vctrl->min_slew_down_rate > INT_MAX) {
+ dev_err(&pdev->dev, "min-slew-down-rate (%u) too big\n",
+ vctrl->min_slew_down_rate);
+ return -EINVAL;
+ }
+ }
+
+ if (vctrl->ovp_threshold && !vctrl->min_slew_down_rate) {
+ dev_err(&pdev->dev,
+ "ovp-threshold-percent requires min-slew-down-rate\n");
+ return -EINVAL;
+ }
+
+ ret = of_property_read_u32(np, "regulator-min-microvolt", &pval);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed to read regulator-min-microvolt: %d\n", ret);
+ return ret;
+ }
+ vctrl->vrange.out.min_uV = pval;
+
+ ret = of_property_read_u32(np, "regulator-max-microvolt", &pval);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed to read regulator-max-microvolt: %d\n", ret);
+ return ret;
+ }
+ vctrl->vrange.out.max_uV = pval;
+
+ ret = of_property_read_u32_array(np, "ctrl-voltage-range", vrange_ctrl,
+ 2);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to read ctrl-voltage-range: %d\n",
+ ret);
+ return ret;
+ }
+
+ if (vrange_ctrl[0] >= vrange_ctrl[1]) {
+ dev_err(&pdev->dev, "ctrl-voltage-range is invalid: %d-%d\n",
+ vrange_ctrl[0], vrange_ctrl[1]);
+ return -EINVAL;
+ }
+
+ vctrl->vrange.ctrl.min_uV = vrange_ctrl[0];
+ vctrl->vrange.ctrl.max_uV = vrange_ctrl[1];
+
+ return 0;
+}
+
+static int vctrl_cmp_ctrl_uV(const void *a, const void *b)
+{
+ const struct vctrl_voltage_table *at = a;
+ const struct vctrl_voltage_table *bt = b;
+
+ return at->ctrl - bt->ctrl;
+}
+
+static int vctrl_init_vtable(struct platform_device *pdev)
+{
+ struct vctrl_data *vctrl = platform_get_drvdata(pdev);
+ struct regulator_desc *rdesc = &vctrl->desc;
+ struct regulator *ctrl_reg = vctrl->ctrl_reg;
+ struct vctrl_voltage_range *vrange_ctrl = &vctrl->vrange.ctrl;
+ int n_voltages;
+ int ctrl_uV;
+ int i, idx_vt;
+
+ n_voltages = regulator_count_voltages(ctrl_reg);
+
+ rdesc->n_voltages = n_voltages;
+
+ /* determine number of steps within the range of the vctrl regulator */
+ for (i = 0; i < n_voltages; i++) {
+ ctrl_uV = regulator_list_voltage(ctrl_reg, i);
+
+ if (ctrl_uV < vrange_ctrl->min_uV ||
+ ctrl_uV > vrange_ctrl->max_uV) {
+ rdesc->n_voltages--;
+ continue;
+ }
+ }
+
+ if (rdesc->n_voltages == 0) {
+ dev_err(&pdev->dev, "invalid configuration\n");
+ return -EINVAL;
+ }
+
+ vctrl->vtable = devm_kmalloc_array(
+ &pdev->dev, sizeof(struct vctrl_voltage_table),
+ rdesc->n_voltages, GFP_KERNEL | __GFP_ZERO);
+ if (!vctrl->vtable)
+ return -ENOMEM;
+
+ /* create mapping control <=> output voltage */
+ for (i = 0, idx_vt = 0; i < n_voltages; i++) {
+ ctrl_uV = regulator_list_voltage(ctrl_reg, i);
+
+ if (ctrl_uV < vrange_ctrl->min_uV ||
+ ctrl_uV > vrange_ctrl->max_uV)
+ continue;
+
+ vctrl->vtable[idx_vt].ctrl = ctrl_uV;
+ vctrl->vtable[idx_vt].out =
+ vctrl_calc_output_voltage(vctrl, ctrl_uV);
+ idx_vt++;
+ }
+
+ /* we rely on the table to be ordered by ascending voltage */
+ sort(vctrl->vtable, rdesc->n_voltages,
+ sizeof(struct vctrl_voltage_table), vctrl_cmp_ctrl_uV,
+ NULL);
+
+ /* pre-calculate OVP-safe downward transitions */
+ for (i = n_voltages - 1; i > 0; i--) {
+ int j;
+ int ovp_min_uV = (vctrl->vtable[i].out *
+ (100 - vctrl->ovp_threshold)) / 100;
+
+ for (j = 0; j < i; j++) {
+ if (vctrl->vtable[j].out >= ovp_min_uV) {
+ vctrl->vtable[i].ovp_min_sel = j;
+ break;
+ }
+ }
+
+ if (j == i) {
+ dev_warn(&pdev->dev, "switching down from %duV may cause OVP shutdown\n",
+ vctrl->vtable[i].out);
+ /* use next lowest voltage */
+ vctrl->vtable[i].ovp_min_sel = i - 1;
+ }
+ }
+
+ return 0;
+}
+
+static int vctrl_enable(struct regulator_dev *rdev)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+ int ret = regulator_enable(vctrl->ctrl_reg);
+
+ if (!ret)
+ vctrl->enabled = true;
+
+ return ret;
+}
+
+static int vctrl_disable(struct regulator_dev *rdev)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+ int ret = regulator_disable(vctrl->ctrl_reg);
+
+ if (!ret)
+ vctrl->enabled = false;
+
+ return ret;
+}
+
+static int vctrl_is_enabled(struct regulator_dev *rdev)
+{
+ struct vctrl_data *vctrl = rdev_get_drvdata(rdev);
+
+ return vctrl->enabled;
+}
+
+static const struct regulator_ops vctrl_ops_cont = {
+ .enable = vctrl_enable,
+ .disable = vctrl_disable,
+ .is_enabled = vctrl_is_enabled,
+ .get_voltage = vctrl_get_voltage,
+ .set_voltage = vctrl_set_voltage,
+};
+
+static const struct regulator_ops vctrl_ops_non_cont = {
+ .enable = vctrl_enable,
+ .disable = vctrl_disable,
+ .is_enabled = vctrl_is_enabled,
+ .set_voltage_sel = vctrl_set_voltage_sel,
+ .get_voltage_sel = vctrl_get_voltage_sel,
+ .list_voltage = vctrl_list_voltage,
+ .map_voltage = regulator_map_voltage_iterate,
+};
+
+static int vctrl_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct vctrl_data *vctrl;
+ const struct regulator_init_data *init_data;
+ struct regulator_desc *rdesc;
+ struct regulator_config cfg = { };
+ struct vctrl_voltage_range *vrange_ctrl;
+ int ctrl_uV;
+ int ret;
+
+ vctrl = devm_kzalloc(&pdev->dev, sizeof(struct vctrl_data),
+ GFP_KERNEL);
+ if (!vctrl)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, vctrl);
+
+ ret = vctrl_parse_dt(pdev, vctrl);
+ if (ret)
+ return ret;
+
+ vrange_ctrl = &vctrl->vrange.ctrl;
+
+ rdesc = &vctrl->desc;
+ rdesc->name = "vctrl";
+ rdesc->type = REGULATOR_VOLTAGE;
+ rdesc->owner = THIS_MODULE;
+
+ if ((regulator_get_linear_step(vctrl->ctrl_reg) == 1) ||
+ (regulator_count_voltages(vctrl->ctrl_reg) == -EINVAL)) {
+ rdesc->continuous_voltage_range = true;
+ rdesc->ops = &vctrl_ops_cont;
+ } else {
+ rdesc->ops = &vctrl_ops_non_cont;
+ }
+
+ init_data = of_get_regulator_init_data(&pdev->dev, np, rdesc);
+ if (!init_data)
+ return -ENOMEM;
+
+ cfg.of_node = np;
+ cfg.dev = &pdev->dev;
+ cfg.driver_data = vctrl;
+ cfg.init_data = init_data;
+
+ if (!rdesc->continuous_voltage_range) {
+ ret = vctrl_init_vtable(pdev);
+ if (ret)
+ return ret;
+
+ ctrl_uV = regulator_get_voltage(vctrl->ctrl_reg);
+ if (ctrl_uV < 0) {
+ dev_err(&pdev->dev, "failed to get control voltage\n");
+ return ctrl_uV;
+ }
+
+ /* determine current voltage selector from control voltage */
+ if (ctrl_uV < vrange_ctrl->min_uV) {
+ vctrl->sel = 0;
+ } else if (ctrl_uV > vrange_ctrl->max_uV) {
+ vctrl->sel = rdesc->n_voltages - 1;
+ } else {
+ int i;
+
+ for (i = 0; i < rdesc->n_voltages; i++) {
+ if (ctrl_uV == vctrl->vtable[i].ctrl) {
+ vctrl->sel = i;
+ break;
+ }
+ }
+ }
+ }
+
+ vctrl->rdev = devm_regulator_register(&pdev->dev, rdesc, &cfg);
+ if (IS_ERR(vctrl->rdev)) {
+ ret = PTR_ERR(vctrl->rdev);
+ dev_err(&pdev->dev, "failed to register regulator: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id vctrl_of_match[] = {
+ { .compatible = "vctrl-regulator", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, vctrl_of_match);
+
+static struct platform_driver vctrl_driver = {
+ .probe = vctrl_probe,
+ .driver = {
+ .name = "vctrl-regulator",
+ .of_match_table = of_match_ptr(vctrl_of_match),
+ },
+};
+
+module_platform_driver(vctrl_driver);
+
+MODULE_DESCRIPTION("Voltage Controlled Regulator Driver");
+MODULE_AUTHOR("Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
From: Vikas Manocha @ 2017-04-10 18:40 UTC (permalink / raw)
To: Alexandre Torgue, patrice.chotard-qxv4g6HH51o
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM PORT, open list, Mark Rutland, Maxime Coquelin,
Rob Herring, Russell King
In-Reply-To: <6ea617a3-94bc-870f-dd52-a45ad45c6d26-qxv4g6HH51o@public.gmane.org>
Thanks Alex,
On 04/10/2017 12:23 AM, Alexandre Torgue wrote:
> Hi
>
> On 04/08/2017 03:12 AM, Vikas Manocha wrote:
>> Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the major
>> spces of the two boards:
>>
>> stm32f769I discovery board:
>> - Cortex-M7 core @216MHz
>> - 2MB mcu internal flash
>> - 512KB internal sram
>> - 16MB sdram memory
>> - 64MB qspi flash memory
>> - 4 inch wvga LCD-TFT Display
>>
>> stm32f746 discovery board:
>> - Cortex-M7 core @216MHz
>> - 1MB mcu internal flash
>> - 320KB internal sram
>> - 8MB sdram memory
>> - 16MB qspi flash memory
>> - 4.3 inch 480x272 LCD-TFT display
>>
>> Signed-off-by: Vikas Manocha <vikas.manocha-qxv4g6HH51o@public.gmane.org>
>> ---
>> arch/arm/boot/dts/Makefile | 2 +
>> arch/arm/boot/dts/stm32f746-disco.dts | 101 ++++++++++++++++++++++++++++++++++
>> arch/arm/boot/dts/stm32f746.dtsi | 2 +-
>> arch/arm/boot/dts/stm32f769-disco.dts | 101 ++++++++++++++++++++++++++++++++++
>> 4 files changed, 205 insertions(+), 1 deletion(-)
>> create mode 100644 arch/arm/boot/dts/stm32f746-disco.dts
>> create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 0118084..a119f74 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
>> dtb-$(CONFIG_ARCH_STM32)+= \
>> stm32f429-disco.dtb \
>> stm32f469-disco.dtb \
>> + stm32f746-disco.dtb \
>> + stm32f769-disco.dtb \
>> stm32429i-eval.dtb \
>> stm32746g-eval.dtb
>> dtb-$(CONFIG_MACH_SUN4I) += \
>> diff --git a/arch/arm/boot/dts/stm32f746-disco.dts b/arch/arm/boot/dts/stm32f746-disco.dts
>> new file mode 100644
>> index 0000000..c0e313f
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/stm32f746-disco.dts
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + * a) This file 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; either version 2 of the
>> + * License, or (at your option) any later version.
>> + *
>> + * This file 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.
>> + *
>> + * Or, alternatively,
>> + *
>> + * b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +#include "stm32f746.dtsi"
>> +#include <dt-bindings/input/input.h>
>> +
>> +/ {
>> + model = "STMicroelectronics STM32F746-DISCO board";
>> + compatible = "st,stm32f746-disco", "st,stm32f746";
>> +
>> + chosen {
>> + bootargs = "root=/dev/ram";
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + memory {
>> + reg = <0xC0000000 0x800000>;
>> + };
>> +
>> + aliases {
>> + serial0 = &usart1;
>> + };
>> +
>> +};
>> +
>> +&clk_hse {
>> + clock-frequency = <25000000>;
>> +};
>> +
>> +&pinctrl {
>
>
> Pin muxing is not defined in board file. Please move it into SOC dtsi file.
Pin muxing used is different for different boards. e.g. usart1_rx pad is PA10 for stm32f769-disco board while it is PB7 for stm32f746-disco board.
The other possibilities for same pad (usart1_rx) is PB15. To make situation bit more complex, it is only available in f769 device.
Putting in SOC dtsi file means having lot of combinations for different pins in separate groups.
e.g. only for one instance of one ip (usart1), following groups might be required at one point of time:
usart1_pa10_pa9 {..}
usart1_pa10_pb14 {..}
usart1_pa10_pb6 {..}
usart1_pb7_pa9 {..}
usart1_pb7_pb14 {..}
usart1_pb7_pb6 {..}
usart1_pb15_pa9 {..}
usart1_pb15_pb14 {..}
usart1_pb15_pb6 {..}
In case of boards based on stm32f746 device, all the above mentioned groups with pb14 & pb15 will not be available.
One solution (to avoid using not available groups) could be to have separate dtsi (or separate pinmux.dtsi) for different devices of same family like one for stm32f746 & other for stm32f769. Still it does not resolve the need to have lot of groups combinations for each instance of every peripheral in dtsi as mentioned above.
It seems cleaner solution would be pin muxing in board dts file. Please let me know if there is some drawback of this approach. One point which i can think of is : duplication of pinmux groups in different board dts files.
Cheers,
Vikas
>> + usart1_pins: usart1@0 {
>> + pins1 {
>> + pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
>> + bias-disable;
>> + drive-push-pull;
>> + slew-rate = <2>;
>> + };
>> + pins2 {
>> + pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
>> + bias-disable;
>> + };
>> + };
>> +
>> + qspi_pins: qspi@0 {
>> + pins {
>> + pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
>> + <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
>> + <STM32F746_PD11_FUNC_QUADSPI_BK1_IO0>,
>> + <STM32F746_PD12_FUNC_QUADSPI_BK1_IO1>,
>> + <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
>> + <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
>> + slew-rate = <2>;
>> + };
>> + };
>> +};
>> +
>> +&usart1 {
>> + pinctrl-0 = <&usart1_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> +};
>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
>> index f321ffe..826700f 100644
>> --- a/arch/arm/boot/dts/stm32f746.dtsi
>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
>> @@ -178,7 +178,7 @@
>> interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
>> };
>>
>> - pin-controller {
>> + pinctrl: pin-controller {
>> #address-cells = <1>;
>> #size-cells = <1>;
>> compatible = "st,stm32f746-pinctrl";
>> diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
>> new file mode 100644
>> index 0000000..5f8558e
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + * a) This file 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; either version 2 of the
>> + * License, or (at your option) any later version.
>> + *
>> + * This file 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.
>> + *
>> + * Or, alternatively,
>> + *
>> + * b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +#include "stm32f746.dtsi"
>> +#include <dt-bindings/input/input.h>
>> +
>> +/ {
>> + model = "STMicroelectronics STM32F769-DISCO board";
>> + compatible = "st,stm32f769-disco", "st,stm32f7";
>> +
>> + chosen {
>> + bootargs = "root=/dev/ram";
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + memory {
>> + reg = <0xC0000000 0x1000000>;
>> + };
>> +
>> + aliases {
>> + serial0 = &usart1;
>> + };
>> +
>> +};
>> +
>> +&clk_hse {
>> + clock-frequency = <25000000>;
>> +};
>> +
>> +&pinctrl {
>
> same.
>
>> + usart1_pins: usart1@0 {
>> + pins1 {
>> + pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
>> + bias-disable;
>> + drive-push-pull;
>> + slew-rate = <2>;
>> + };
>> + pins2 {
>> + pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
>> + bias-disable;
>> + };
>> + };
>> +
>> + qspi_pins: qspi@0 {
>> + pins {
>> + pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
>> + <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
>> + <STM32F746_PC9_FUNC_QUADSPI_BK1_IO0>,
>> + <STM32F746_PC10_FUNC_QUADSPI_BK1_IO1>,
>> + <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
>> + <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
>> + slew-rate = <2>;
>> + };
>> + };
>> +};
>> +
>> +&usart1 {
>> + pinctrl-0 = <&usart1_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> +};
>>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/3] dt-bindings: Document Phytec phyCORE-RK3288 RDK
From: Rob Herring @ 2017-04-10 18:26 UTC (permalink / raw)
To: Wadim Egorov
Cc: mark.rutland-5wv7dgnIgG8, heiko-4mtYJXux2i+zQB+pC5nmwQ,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491483866-18368-3-git-send-email-w.egorov-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
On Thu, Apr 06, 2017 at 03:04:26PM +0200, Wadim Egorov wrote:
> Add documentation for the PCM-947 carrier board, a RK3288 based
> development board made by PHYTEC.
>
> Signed-off-by: Wadim Egorov <w.egorov-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> ---
> No changes in v2
> ---
> Documentation/devicetree/bindings/arm/rockchip.txt | 4 ++++
> 1 file changed, 4 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 v5 1/5] dt-bindings: watchdog: Document STM32 IWDG bindings
From: Rob Herring @ 2017-04-10 18:25 UTC (permalink / raw)
To: Yannick Fertre
Cc: Wim Van Sebroeck, Guenter Roeck, Alexandre TORGUE,
Benjamin Gaignard, Maxime Coquelin,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Philippe Cornu,
Gabriel FERNANDEZ, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1491481168-22213-2-git-send-email-yannick.fertre-qxv4g6HH51o@public.gmane.org>
On Thu, Apr 06, 2017 at 02:19:24PM +0200, Yannick Fertre wrote:
> This adds documentation of device tree bindings for the STM32 IWDG
> (Independent WatchDoG).
>
> Signed-off-by: Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>
> ---
> .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" 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: Add vendor prefix for Nordic Semiconductor
From: Rob Herring @ 2017-04-10 18:24 UTC (permalink / raw)
To: Kumar Gala; +Cc: Device Tree Mailing List, Mark Rutland
In-Reply-To: <20170406082927.31405-1-kumar.gala-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Thu, Apr 06, 2017 at 03:29:27AM -0500, Kumar Gala wrote:
> Nordic Semiconductor is a semiconductor company specializing in ARM
> Cortex-M based SoCs for low-power wireless communication, especially
> Bluetooth.
>
> Signed-off-by: Kumar Gala <kumar.gala-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)
Applied, thanks.
Rob
--
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/5] dt-bindings: media: Add document file for CXD2880 SPI I/F
From: Rob Herring @ 2017-04-10 18:23 UTC (permalink / raw)
To: Yasunari.Takiguchi
Cc: linux-kernel, linux-media, devicetree, tbird20d, frowand.list,
Masayuki Yamamoto, Hideki Nozawa, Kota Yonezawa,
Toshihiko Matsumoto, Satoshi Watanabe
In-Reply-To: <1491465273-9338-1-git-send-email-Yasunari.Takiguchi@sony.com>
On Thu, Apr 06, 2017 at 04:54:33PM +0900, Yasunari.Takiguchi@sony.com wrote:
> From: Yasunari Takiguchi <Yasunari.Takiguchi@sony.com>
>
> This is the document file for Sony CXD2880 DVB-T2/T tuner + demodulator.
> It contains the description of the SPI adapter binding.
>
> Signed-off-by: Yasunari Takiguchi <Yasunari.Takiguchi@sony.com>
> Signed-off-by: Masayuki Yamamoto <Masayuki.Yamamoto@sony.com>
> Signed-off-by: Hideki Nozawa <Hideki.Nozawa@sony.com>
> Signed-off-by: Kota Yonezawa <Kota.Yonezawa@sony.com>
> Signed-off-by: Toshihiko Matsumoto <Toshihiko.Matsumoto@sony.com>
> Signed-off-by: Satoshi Watanabe <Satoshi.C.Watanabe@sony.com>
> ---
> .../devicetree/bindings/media/spi/sony-cxd2880.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/spi/sony-cxd2880.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 3/3] nvmem: dt: document SNVS LPGPR binding
From: Rob Herring @ 2017-04-10 18:22 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Srinivas Kandagatla, Maxime Ripard, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170406073107.32445-3-o.rempel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Thu, Apr 06, 2017 at 09:31:07AM +0200, Oleksij Rempel wrote:
> Documenation bindings for the Low Power General Purpose Registe
s/Registe/Register/
> available on i.MX6 SoCs in the Secure Non-Volatile Storage.
>
> Signed-off-by: Oleksij Rempel <o.rempel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> Documentation/devicetree/bindings/nvmem/snvs-lpgpr.txt | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/snvs-lpgpr.txt
>
> diff --git a/Documentation/devicetree/bindings/nvmem/snvs-lpgpr.txt b/Documentation/devicetree/bindings/nvmem/snvs-lpgpr.txt
> new file mode 100644
> index 000000000000..9a8be1a2d12e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/snvs-lpgpr.txt
> @@ -0,0 +1,15 @@
> +Device tree bindings for Low Power General Purpose Registe found in i.MX6Q/D
> +Secure Non-Volatile Storage.
> +
> +Required properties:
> +- compatible: should be one of
> + "fsl,imx6q-snvs-lpgpr" (i.MX6Q/D/DL/S).
> +- offset: Should contain the offset relative to syscon parrent node.
typo
+- regmap: Should contain a phandle pointing to syscon.
> +
> +Example:
> + snvs_lpgpr: snvs-lpgpr {
> + compatible = "fsl,imx6q-snvs-lpgpr";
> + regmap = <&snvs>;
> + offset = <0x68>;
Why does this need to be in DT? Is something going to refer to this
node? If not, the &snvs node should be enough information for the OS.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 1/7] pinctrl: dt-bindings: Add documentation for Armada 37xx pin controllers
From: Rob Herring @ 2017-04-10 18:15 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Linus Walleij, linux-gpio, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, linux-arm-kernel,
devicetree, linux-kernel, Nadav Haklai, Victor Gu, Marcin Wojtas,
Wilson Ding, Hua Jing, Neta Zur Hershkovits
In-Reply-To: <941d03c9a3bdfd5e789aada29b35184ec9fed9fe.1491405475.git-series.gregory.clement@free-electrons.com>
On Wed, Apr 05, 2017 at 05:18:02PM +0200, Gregory CLEMENT wrote:
> Document the device tree binding for the pin controllers found on the
> Armada 37xx SoCs.
>
> Update the binding documention of the xtal clk which is a subnode of this
> syscon node.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt | 7 +--
> Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt | 183 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 187 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> index a88f1f05fbd6..4c0807f28cfa 100644
> --- a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> @@ -5,6 +5,7 @@ reading the gpio latch register.
>
> This node must be a subnode of the node exposing the register address
> of the GPIO block where the gpio latch is located.
> +See Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
>
> Required properties:
> - compatible : shall be one of the following:
> @@ -16,9 +17,9 @@ Optional properties:
> output names ("xtal")
>
> Example:
> -gpio1: gpio@13800 {
> - compatible = "marvell,armada-3700-gpio", "syscon", "simple-mfd";
> - reg = <0x13800 0x1000>;
> +pinctrl_nb: pinctrl-nb@13800 {
> + compatible = "armada3710-nb-pinctrl", "syscon", "simple-mfd";
> + reg = <0x13800 0x100>, <0x13C00 0x20>;
>
> xtalclk: xtal-clk {
> compatible = "marvell,armada-3700-xtal-clock";
> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
> new file mode 100644
> index 000000000000..2eda81e0bca1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
> @@ -0,0 +1,183 @@
> +* Marvell Armada 37xx SoC pin and gpio controller
> +
> +Each Armada 37xx SoC come with two pin and gpio controller one for the
> +south bridge and the other for the north bridge.
> +
> +Inside this set of register the gpio latch allows exposing some
> +configuration of the SoC and especially the clock frequency of the
> +xtal. Hence, this node is a represent as syscon allowing sharing the
> +register between multiple hardware block.
> +
> +GPIO and pin controller:
> +------------------------
> +
> +Main node:
> +
> +Refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning
> +of the phrase "pin configuration node".
> +
> +Required properties for pinctrl driver:
> +
> +- compatible: "marvell,armada3710-sb-pinctrl", "syscon, "simple-mfd"
> + for the south bridge
> + "marvell,armada3710-nb-pinctrl", "syscon, "simple-mfd"
> + for the north bridge
> +- reg: The first set of register are for pinctrl/gpio and the second
> + set for the interrupt controller
> +- interrupts: list of the interrupt use by the gpio
> +
> +Available groups and functions for the North bridge:
> +
> +group: jtag
> + - pins 20-24
> + - functions jtag, gpio
> +
> +group sdio0
> + - pins 8-10
> + - functions sdio, gpio
> +
> +group emmc_nb
> + - pins 27-35
> + - functions emmc, gpio
> +
> +group pwm0
> + - pin 11 (GPIO1-11)
> + - functions pwm, gpio
> +
> +group pwm1
> + - pin 12
> + - functions pwm, gpio
> +
> +group pwm2
> + - pin 13
> + - functions pwm, gpio
> +
> +group pwm3
> + - pin 14
> + - functions pwm, gpio
> +
> +group pmic1
> + - pin 17
> + - functions pmic, gpio
> +
> +group pmic0
> + - pin 16
> + - functions pmic, gpio
> +
> +group i2c2
> + - pins 2-3
> + - functions i2c, gpio
> +
> +group i2c1
> + - pins 0-1
> + - functions i2c, gpio
> +
> +group spi_cs1
> + - pin 17
> + - functions spi, gpio
> +
> +group spi_cs2
> + - pin 18
> + - functions spi, gpio
> +
> +group spi_cs3
> + - pin 19
> + - functions spi, gpio
> +
> +group onewire
> + - pin 4
> + - functions onewire, gpio
> +
> +group uart1
> + - pins 25-26
> + - functions uart, gpio
> +
> +group spi_quad
> + - pins 15-16
> + - functions spi, gpio
> +
> +group uart_2
> + - pins 9-10
> + - functions uart, gpio
> +
> +Available groups and functions for the South bridge:
> +
> +group usb32_drvvbus0
> + - pin 36
> + - functions drvbus, gpio
> +
> +group usb2_drvvbus1
> + - pin 37
> + - functions drvbus, gpio
> +
> +group sdio_sb
> + - pins 60-64
> + - functions sdio, gpio
> +
> +group rgmii
> + - pins 42-55
> + - functions mii, gpio
> +
> +group pcie1
> + - pins 39-40
> + - functions pcie, gpio
> +
> +group ptp
> + - pins 56-58
> + - functions ptp, gpio
> +
> +group ptp_clk
> + - pin 57
> + - functions ptp, mii
> +
> +group ptp_trig
> + - pin 58
> + - functions ptp, mii
> +
> +group mii_col
> + - pin 59
> + - functions mii, mii_err
> +
> +GPIO subnode:
> +
> +Please refer to gpio.txt in this directory for details of gpio-ranges property
> +and the common GPIO bindings used by client devices.
> +
> +Required properties for gpio driver under the gpio subnode:
> +- interrupts: List of interrupt specifier for the controllers interrupt.
> +- gpio-controller: Marks the device node as a gpio controller.
> +- #gpio-cells: Should be 2. The first cell is the GPIO number and the
> + second cell specifies GPIO flags, as defined in
> + <dt-bindings/gpio/gpio.h>. Only the GPIO_ACTIVE_HIGH and
> + GPIO_ACTIVE_LOW flags are supported.
> +- gpio-ranges: Range of pins managed by the GPIO controller.
> +
> +Xtal Clock bindings for Marvell Armada 37xx SoCs
> +------------------------------------------------
> +
> +see Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> +
> +
> +Example:
> +pinctrl_sb: pinctrl-sb@18800 {
Just pinctrl@...
> + compatible = "marvell,armada3710-sb-pinctrl", "syscon", "simple-mfd";
> + reg = <0x18800 0x100>, <0x18C00 0x20>;
> + gpiosb: gpiosb {
gpio {
With that,
Acked-by: Rob Herring <robh@kernel.org>
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl_sb 0 0 29>;
> + gpio-controller;
> + interrupts =
> + <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + rgmii_pins: mii-pins {
> + groups = "rgmii";
> + function = "mii";
> + };
> +
> +};
> --
> git-series 0.9.1
^ permalink raw reply
* Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port
From: Ralph Sennhauser @ 2017-04-10 18:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
Jason Cooper, devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Gregory Clement, Rob Herring,
Sebastian Hesselbarth
In-Reply-To: <20170405180808.4f8924cb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, 5 Apr 2017 18:08:08 +0200
Ralph Sennhauser <ralph.sennhauser-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Andrew,
>
> On Wed, 5 Apr 2017 17:50:32 +0200
> Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> > > > I would say, this is now too late. Changing an interface name
> > > > will break configuration scripts. We are stuck with it.
> > > >
> > >
> > > If it weren't for commit cb4f71c42988 that would have been obvious
> > > for me as well.
> >
> > Yes, that was not nice. But it was also very earlier in the life of
> > 38x, so it just affected a few developers with reference boards, not
> > real products out in the wild, as far as i remember.
> >
> > Andrew
>
> The mentioned commit bit me amongst others, see
> https://lkml.org/lkml/2016/8/21/62
>
> In fact I expect the impact to be much lower this time around, before
> 4.10 DSA was no contender for swconfig (out of tree switch driver as
> used by OpenWrt and others). On Mamba ~200Mbit was top speed. Now they
> perform roughly the same.
When I said much lower I left out the obvious, this only affects a
specific product line respectively a single device unlike the 38x
change.
I asked Chad McCue, principal developer of McDebian [1] (the only
"firmware" using DSA I know of / could make out) about this. McDebain
already uses "wan" across all devices in the series.
Ralph
[1] https://github.com/Chadster766/McDebian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 3/9] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: Rob Herring @ 2017-04-10 18:12 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491401247-7030-4-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On Wed, Apr 05, 2017 at 04:07:21PM +0200, Jacopo Mondi wrote:
> Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> controller.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/pinctrl/renesas,rza1-pinctrl.txt | 218 +++++++++++++++++++++
> 1 file changed, 218 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> new file mode 100644
> index 0000000..46584ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> @@ -0,0 +1,218 @@
> +Renesas RZ/A1 combined Pin and GPIO controller
> +
> +The Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller,
> +named "Ports" in the hardware reference manual.
> +Pin multiplexing and GPIO configuration is performed on a per-pin basis
> +writing configuration values to per-port register sets.
> +Each "port" features up to 16 pins, each of them configurable for GPIO
> +function (port mode) or in alternate function mode.
> +Up to 8 different alternate function modes exist for each single pin.
> +
> +Pin controller node
> +-------------------
> +
> +Required properties:
> + - compatible
> + this shall be "renesas,r7s72100-ports".
> +
> + - reg
> + address base and length of the memory area where pin controller
> + hardware is mapped to.
> +
> +Example:
> +Pin controller node for RZ/A1H SoC (r7s72100)
> +
> +pinctrl: pin-controller@fcfe3000 {
> + compatible = "renesas,r7s72100-ports";
> +
> + reg = <0xfcfe3000 0x4230>;
> +};
> +
> +Sub-nodes
> +---------
> +
> +The child nodes of the pin controller node describe a pin multiplexing
> +function or a gpio controller alternatively.
> +
> +- Pin multiplexing sub-nodes:
> + A pin multiplexing sub-node describes how to configure a set of
> + (or a single) pin in some desired alternate function mode.
> + A single sub-node may define several pin configurations.
> + Some alternate functions require special pin configuration flags to be
> + supplied along with the alternate function configuration number.
> + When hardware reference manual specifies a pin function to be either
> + "bi-directional" or "software IO driven", use the generic properties from
> + <include/linux/pinctrl/pinconf_generic.h> header file to instruct the
> + pin controller to perform the desired pin configuration operations.
> + Please refer to pinctrl-bindings.txt to get to know more on generic
> + pin properties usage.
> +
> + The allowed generic formats for a pin multiplexing sub-node are the
> + following ones:
> +
> + node-1 {
> + pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> + GENERIC_PINCONFIG;
What's GENERIC_PINCONFIG? I see this in some other binding docs, but not
used anywhere. If this is a boolean property then get rid of the all
caps. If this is a define, then don't use complex defines that expand to
dts source.
--
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 06/16] regulator: madera-ldo1: LDO1 driver for Cirrus Logic Madera codecs
From: Mark Brown @ 2017-04-10 18:11 UTC (permalink / raw)
To: Rob Herring
Cc: gnurou, alsa-devel, jason, devicetree, Richard Fitzgerald,
linus.walleij, patches, linux-kernel, linux-gpio, tglx, lee.jones
In-Reply-To: <20170410174917.dlklrvw2sraeq6xs@rob-hp-laptop>
[-- Attachment #1.1: Type: text/plain, Size: 322 bytes --]
On Mon, Apr 10, 2017 at 12:49:17PM -0500, Rob Herring wrote:
> Same question as Mark. Should this share bindings with arizona? The
> arizona one looks a bit strange, so not sure we'd want to just copy it.
In what way? Other than the -gpios stuff (which would just be a trivial
thing if we wanted to change it)?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/9] pinctrl: generic: Add bi-directional and output-enable
From: Rob Herring @ 2017-04-10 18:06 UTC (permalink / raw)
To: Jacopo Mondi
Cc: linus.walleij, geert+renesas, laurent.pinchart, chris.brandt,
mark.rutland, linux, linux-renesas-soc, linux-gpio, devicetree,
linux-kernel
In-Reply-To: <1491401247-7030-2-git-send-email-jacopo+renesas@jmondi.org>
On Wed, Apr 05, 2017 at 04:07:19PM +0200, Jacopo Mondi wrote:
> Add bi-directional and output-enable pin configuration properties.
>
> bi-directional allows to specify when a pin shall operate in input and
> output mode at the same time. This is particularly useful in platforms
> where input and output buffers have to be manually enabled.
>
> output-enable is just syntactic sugar to specify that a pin shall
> operate in output mode, ignoring the provided argument.
> This pairs with input-enable pin configuration option.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt | 2 ++
> drivers/pinctrl/pinconf-generic.c | 3 +++
> include/linux/pinctrl/pinconf-generic.h | 3 +++
> 3 files changed, 8 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 13/16] ASoC: madera: Add common support for Cirrus Logic Madera codecs
From: Rob Herring @ 2017-04-10 18:03 UTC (permalink / raw)
To: Richard Fitzgerald
Cc: linus.walleij, gnurou, tglx, jason, lee.jones, broonie,
alsa-devel, patches, linux-gpio, devicetree, linux-kernel
In-Reply-To: <1491386884-30689-14-git-send-email-rf@opensource.wolfsonmicro.com>
On Wed, Apr 05, 2017 at 11:08:01AM +0100, Richard Fitzgerald wrote:
> The Cirrus Logic Madera codecs are a family of related codecs with
> extensive digital and analogue I/O, digital mixing and routing,
> signal processing and programmable DSPs.
>
> This patch adds common support code shared by all Madera codecs.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Signed-off-by: Nariman Poushin <nariman@opensource.wolfsonmicro.com>
> Signed-off-by: Nikesh Oswal <Nikesh.Oswal@wolfsonmicro.com>
> Signed-off-by: Piotr Stankiewicz <piotrs@opensource.wolfsonmicro.com>
> Signed-off-by: Ajit Pandey <ajit.pandey@incubesol.com>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> ---
> Documentation/devicetree/bindings/sound/madera.txt | 63 +
> MAINTAINERS | 5 +
> include/dt-bindings/sound/madera.h | 18 +
Acked-by: Rob Herring <robh@kernel.org>
> include/sound/madera-pdata.h | 70 +
> sound/soc/codecs/Kconfig | 5 +
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/madera.c | 4430 ++++++++++++++++++++
> sound/soc/codecs/madera.h | 470 +++
> 8 files changed, 5063 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/madera.txt
> create mode 100644 include/dt-bindings/sound/madera.h
> create mode 100644 include/sound/madera-pdata.h
> create mode 100644 sound/soc/codecs/madera.c
> create mode 100644 sound/soc/codecs/madera.h
^ permalink raw reply
* Re: [PATCH 09/16] pinctrl: madera: Add driver for Cirrus Logic Madera codecs
From: Rob Herring @ 2017-04-10 17:56 UTC (permalink / raw)
To: Richard Fitzgerald
Cc: gnurou, alsa-devel, jason, devicetree, linus.walleij, patches,
linux-kernel, linux-gpio, broonie, tglx, lee.jones
In-Reply-To: <1491386884-30689-10-git-send-email-rf@opensource.wolfsonmicro.com>
On Wed, Apr 05, 2017 at 11:07:57AM +0100, Richard Fitzgerald wrote:
> These codecs have a variable number of I/O lines each of which
> is individually selectable to a wide range of possible functions.
>
> The functionality is slightly different from the traditional muxed
> GPIO since most of the functions can be mapped to any pin (and even
> the same function to multiple pins). Most pins have a dedicated
> "alternate" function that is only available on that pin. The
> alternate functions are usually a group of signals, though it is
> not always necessary to enable the full group, depending on the
> alternate function and how it is to be used. The mapping between
> alternate functions and GPIO pins varies between codecs depending
> on the number of alternate functions and available pins.
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> ---
> .../bindings/pinctrl/cirrus,madera-pinctrl.txt | 103 ++
As Linus said, separate patch is preferred. But I don't have any other
comments, so I'm not going to require it:
Acked-by: Rob Herring <robh@kernel.org>
> MAINTAINERS | 2 +
> drivers/pinctrl/Kconfig | 22 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-madera.c | 1092 ++++++++++++++++++++
> 5 files changed, 1220 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/cirrus,madera-pinctrl.txt
> create mode 100644 drivers/pinctrl/pinctrl-madera.c
^ permalink raw reply
* Re: [PATCH 08/16] irqchip: Add driver for Cirrus Logic Madera codecs
From: Rob Herring @ 2017-04-10 17:53 UTC (permalink / raw)
To: Richard Fitzgerald
Cc: gnurou, alsa-devel, jason, devicetree, linus.walleij, patches,
linux-kernel, linux-gpio, broonie, tglx, lee.jones
In-Reply-To: <1491386884-30689-9-git-send-email-rf@opensource.wolfsonmicro.com>
On Wed, Apr 05, 2017 at 11:07:56AM +0100, Richard Fitzgerald wrote:
> The Cirrus Logic Madera codecs (Cirrus Logic CS47L35/85/90/91 and WM1840)
> are highly complex devices containing up to 7 programmable DSPs and many
> other internal sources of interrupts plus a number of GPIOs that can be
> used as interrupt inputs. The large number (>150) of internal interrupt
> sources are managed by an on-board interrupt controller.
>
> This driver provides the handling for the interrupt controller. As the
> codec is accessed via regmap, we can make use of the generic IRQ
> functionality from regmap to do most of the work. Only around half of
> the possible interrupt source are currently of interest from the driver
> so only this subset is defined. Others can be added in future if needed.
>
> The KConfig options are not user-configurable because this driver is
> mandatory so is automatically included when the parent MFD driver is
> selected.
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> .../interrupt-controller/cirrus,madera.txt | 31 ++
> MAINTAINERS | 3 +
> drivers/irqchip/Kconfig | 5 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-madera.c | 349 +++++++++++++++++++++
> include/linux/irqchip/irq-madera-pdata.h | 19 ++
> include/linux/irqchip/irq-madera.h | 96 ++++++
> 7 files changed, 504 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/cirrus,madera.txt
> create mode 100644 drivers/irqchip/irq-madera.c
> create mode 100644 include/linux/irqchip/irq-madera-pdata.h
> create mode 100644 include/linux/irqchip/irq-madera.h
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/cirrus,madera.txt b/Documentation/devicetree/bindings/interrupt-controller/cirrus,madera.txt
> new file mode 100644
> index 0000000..4505315
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/cirrus,madera.txt
> @@ -0,0 +1,31 @@
> +Cirrus Logic Madera class audio codec IRQ driver
> +
> +The IRQ properties are members of the parent MFD node.
Just document them in the MFD binding.
> +
> +See also the core bindings for the parent MFD driver:
> +See Documentation/devicetree/bindings/mfd/madera.txt
> +
> +Required properties:
> + - interrupt-controller : Madera class devices contain interrupt controllers
> + and may provide interrupt services to other devices.
> +
> + - #interrupt-cells: the number of cells to describe an IRQ, this should be 2.
> + The first cell is the IRQ number.
> + The second cell is the flags, encoded as the trigger masks from
> + bindings/interrupt-controller/interrupts.txt
> +
> + - interrupts : The interrupt line the /IRQ signal for the device is
> + connected to.
> +
> + - interrupt-parent : The parent interrupt controller.
> +
> +Example:
> +
> +codec: cs47l85@0 {
> + compatible = "cirrus,cs47l85";
> +
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + interrupts = <&host_irq1>;
> + interrupt-parent = <&gic>;
> +};
^ permalink raw reply
* Re: [PATCH 06/16] regulator: madera-ldo1: LDO1 driver for Cirrus Logic Madera codecs
From: Rob Herring @ 2017-04-10 17:49 UTC (permalink / raw)
To: Richard Fitzgerald
Cc: linus.walleij, gnurou, tglx, jason, lee.jones, broonie,
alsa-devel, patches, linux-gpio, devicetree, linux-kernel
In-Reply-To: <1491386884-30689-7-git-send-email-rf@opensource.wolfsonmicro.com>
On Wed, Apr 05, 2017 at 11:07:54AM +0100, Richard Fitzgerald wrote:
> This patch adds a driver for the internal LDO1 regulator on
> some Cirrus Logic Madera class codecs.
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> .../devicetree/bindings/regulator/madera-ldo1.txt | 29 +++
cirrus-madera-ldo1.txt
Or perhaps a subdirectory. We should have done the same for arizona
bindings.
Same question as Mark. Should this share bindings with arizona? The
arizona one looks a bit strange, so not sure we'd want to just copy it.
> MAINTAINERS | 3 +
> drivers/regulator/Kconfig | 8 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/madera-ldo1.c | 198 +++++++++++++++++++++
> include/linux/regulator/madera-ldo1.h | 24 +++
> 6 files changed, 263 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/madera-ldo1.txt
> create mode 100644 drivers/regulator/madera-ldo1.c
> create mode 100644 include/linux/regulator/madera-ldo1.h
>
> diff --git a/Documentation/devicetree/bindings/regulator/madera-ldo1.txt b/Documentation/devicetree/bindings/regulator/madera-ldo1.txt
> new file mode 100644
> index 0000000..688f21d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/madera-ldo1.txt
> @@ -0,0 +1,29 @@
> +Cirrus Logic Madera class audio codecs LDO1 regulator driver
> +
> +Only required if you are using the codec internal LDO1 regulator.
> +This is a subnode of the parent mfd node.
> +
> +See also the core bindings for the parent MFD driver:
> +See Documentation/devicetree/bindings/mfd/madera.txt
> +
> +Required properties:
> + - compatible : must be "cirrus,madera-ldo1"
> + - LDOVDD-supply : Power supply for the LDO1 regulator.
> +
> + - enable-gpio : GPIO to use to enable/disable the regulator.
enable-gpios
And define whether active high or low.
> + As defined in bindings/gpio.txt.
> +
> +Optional subnodes:
> + Standard regulator bindings as described in bindings/regulator/regulator.txt
> +
> +Example:
> +
> +codec: cs47l85@0 {
> + compatible = "cirrus,cs47l85";
> +
> + ldo1 {
> + compatible = "cirrus,madera-ldo1";
> + LDOVDD-supply = <&pmic_vdd1>;
> + enable-gpio = <&gpio 0>;
> + };
> +};
^ permalink raw reply
* Re: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Sebastian Reichel @ 2017-04-10 17:10 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Samuel Thibault, Pavel Machek, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Andrey Smirnov
In-Reply-To: <CAL_JsqJCjob6NDfHe0r7X3dXPEniKh5vChXhBxfyHZRu7Fov7A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3810 bytes --]
Hi,
On Mon, Apr 10, 2017 at 11:12:39AM -0500, Rob Herring wrote:
> On Mon, Apr 10, 2017 at 9:03 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Apr 10, 2017 at 08:46:57AM -0500, Rob Herring wrote:
> >> On Sat, Apr 8, 2017 at 11:57 AM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >> > On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
> >> >> Add method, which waits until the transmission buffer has been sent.
> >> >> Note, that the change in ttyport_write_wakeup is related, since
> >> >> tty_wait_until_sent will hang without that change.
> >> >>
> >> >> Acked-by: Rob Herring <robh@kernel.org>
> >> >> Acked-by: Pavel Machek <pavel@ucw.cz>
> >> >> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> >> >> ---
> >> >> Changes since PATCHv2:
> >> >> * Avoid goto in ttyport_write_wakeup
> >> >> ---
> >> >> drivers/tty/serdev/core.c | 11 +++++++++++
> >> >> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
> >> >> include/linux/serdev.h | 3 +++
> >> >> 3 files changed, 28 insertions(+), 4 deletions(-)
> >> >>
> >> >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> >> >> index f4c6c90add78..a63b74031e22 100644
> >> >> --- a/drivers/tty/serdev/core.c
> >> >> +++ b/drivers/tty/serdev/core.c
> >> >> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> >> >> }
> >> >> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
> >> >>
> >> >> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> >> >> +{
> >> >> + struct serdev_controller *ctrl = serdev->ctrl;
> >> >> +
> >> >> + if (!ctrl || !ctrl->ops->wait_until_sent)
> >> >> + return;
> >> >> +
> >> >> + ctrl->ops->wait_until_sent(ctrl, timeout);
> >> >> +}
> >> >> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
> >> >
> >> > Is this still needed now that we have serdev_device_write() with an
> >> > unlimited timeout available?
> >>
> >> Yes, because only this waits until the data is on the wire.
> >
> > What "wire" is that? The serial wire? How do you know this? Many usb
> > to serial devices have no way to determine this, given that there is
> > another uart hanging off of the end of a USB connection.
>
> Okay, maybe it's just out of linux s/w buffers for h/w which you don't
> know. It is the same semantics as tty_wait_until_sent which is
> documented as: "Wait for characters pending in a tty driver to hit the
> wire, or for a timeout to occur (eg due to flow control)"
For embedded h/w it usually means the serial wire. tty_wait_until_sent()
first waits for the tty buffer to be empty and then calls wait_until_sent()
in the driver providing the tty. In case of serial-core that is
implemented by uart_wait_until_sent(), which waits for the serial
driver's tx_empty() operation becoming true (grepping for
".tx_empty" returned 81 hits for me). Also at least some of the usb to
serial adapters seem to support this using usb_serial_generic_wait_until_sent()
and ".tx_empty":
$ git grep "\.tx_empty"
cp210x.c: .tx_empty = cp210x_tx_empty,
f81534.c: .tx_empty = f81534_tx_empty,
ftdi_sio.c: .tx_empty = ftdi_tx_empty,
io_ti.c: .tx_empty = edge_tx_empty,
io_ti.c: .tx_empty = edge_tx_empty,
mxuport.c: .tx_empty = mxuport_tx_empty,
ti_usb_3410_5052.c: .tx_empty = ti_tx_empty,
ti_usb_3410_5052.c: .tx_empty = ti_tx_empty,
The other ones will only wait for empty s/w buffer, but that's
already the case for tty_wait_until_sent(), so IMHO a different
problem.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Ludovic BARRE @ 2017-04-10 16:52 UTC (permalink / raw)
To: Marek Vasut, Cyrille Pitchen
Cc: Boris Brezillon, Alexandre Torgue, devicetree, Richard Weinberger,
linux-kernel, Rob Herring, linux-mtd, Brian Norris,
David Woodhouse
In-Reply-To: <9a0c5f8e-01dc-3d2b-5ebb-069752594e8e@gmail.com>
hi Marek
tomorrow, I send a v3 with your/Rob reviews.
BR
Ludo
On 04/10/2017 06:15 PM, Marek Vasut wrote:
> On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
>> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> The quadspi is a specialized communication interface targeting single,
>>>> dual or quad SPI Flash memories.
>>>>
>>>> It can operate in any of the following modes:
>>>> -indirect mode: all the operations are performed using the quadspi
>>>> registers
>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>> microcontroller address space and is seen by the system as if it was
>>>> an internal memory
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>> drivers/mtd/spi-nor/Kconfig | 7 +
>>>> drivers/mtd/spi-nor/Makefile | 1 +
>>>> drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>>> ++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 698 insertions(+)
>>>> create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>>
>>> [...]
>>>
>>>> +struct stm32_qspi_flash {
>>>> + struct spi_nor nor;
>>>> + u32 cs;
>>>> + u32 fsize;
>>>> + u32 presc;
>>>> + struct stm32_qspi *qspi;
>>>> +};
>>> [...]
>>>
>>>> +struct stm32_qspi_cmd {
>>>> + struct {
>>>> + u8 addr_width;
>>>> + u8 dummy;
>>>> + u8 data;
>>>> + } conf;
>>> Is there any benefit in having this structure here or could you just
>>> make the struct stm32_qspi_cmd flat ?
>> no benefit, it was just to regroup, so I can do a flat structure
> Well, as you like, but I think it does make sense to just make it flat.
>
>>>> + u8 opcode;
>>>> + u32 framemode;
>>>> + u32 qspimode;
>>>> + u32 addr;
>>>> + size_t len;
>>>> + void *buf;
>>>> +};
>>> [...]
>>>
>>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>>> size_t len,
>>>> + u_char *buf)
>>>> +{
>>>> + struct stm32_qspi_flash *flash = nor->priv;
>>>> + struct stm32_qspi *qspi = flash->qspi;
>>>> + struct stm32_qspi_cmd cmd;
>>>> + int err;
>>>> +
>>>> + dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>>> + nor->read_opcode, buf, (u32)from, len);
>>>> +
>>>> + memset(&cmd, 0, sizeof(cmd));
>>>> + cmd.opcode = nor->read_opcode;
>>>> + cmd.conf.addr_width = nor->addr_width;
>>>> + cmd.addr = (u32)from;
>>> loff_t (from) can be 64bit ... how do we handle this ?
>> I'm surprise by the question,
>> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
>> So, the stm32 qspi controller has a 32 bit register for NOR address.
>> On the other hand the framework and other drivers used this variable
>> (from) like
>> a 32 bits.
> Hmmm, (rhetorical question) then why do we even use loff_t in the
> framework ?
>
> Anyway, this is no problem then.
In fact, the loff_t 64 bit come from mtd interface
(needed to address biggest device constraint) but not needed for spi-nor
devices.
>>>> + cmd.conf.data = 1;
>>>> + cmd.conf.dummy = nor->read_dummy;
>>>> + cmd.len = len;
>>>> + cmd.buf = buf;
>>>> + cmd.qspimode = qspi->read_mode;
>>>> +
>>>> + stm32_qspi_set_framemode(nor, &cmd, true);
>>>> + err = stm32_qspi_send(flash, &cmd);
>>>> +
>>>> + return err ? err : len;
>>>> +}
>>> [...]
>>>
>>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>>> +{
>>>> + struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>>> + u32 cr, sr, fcr = 0;
>>>> +
>>>> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>> + sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>>> +
>>>> + if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>>> + /* tx complete */
>>>> + fcr |= FCR_CTCF;
>>>> + complete(&qspi->cmd_completion);
>>>> + } else {
>>>> + dev_info(qspi->dev, "spurious interrupt\n");
>>> You probably want to ratelimit this one ...
>> yes it's better if there is an issue.
> Yep
>
>>>> + }
>>>> +
>>>> + writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>>> +
>>>> + return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>>> +{
>>>> + struct stm32_qspi_flash *flash = nor->priv;
>>>> + struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> + mutex_lock(&qspi->lock);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>>> ops)
>>>> +{
>>>> + struct stm32_qspi_flash *flash = nor->priv;
>>>> + struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> + mutex_unlock(&qspi->lock);
>>>> +}
>>>> +
>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>> + struct device_node *np)
>>>> +{
>>>> + u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>> + struct stm32_qspi_flash *flash;
>>>> + struct mtd_info *mtd;
>>>> + int ret;
>>>> +
>>>> + of_property_read_u32(np, "reg", &cs_num);
>>>> + if (cs_num >= STM32_MAX_NORCHIP)
>>>> + return -EINVAL;
>>>> +
>>>> + of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>> + if (!max_rate)
>>>> + return -EINVAL;
>>>> +
>>>> + presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>> +
>>>> + if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>> + width = 1;
>>>> +
>>>> + if (width == 4)
>>>> + flash_read = SPI_NOR_QUAD;
>>>> + else if (width == 2)
>>>> + flash_read = SPI_NOR_DUAL;
>>>> + else if (width == 1)
>>>> + flash_read = SPI_NOR_NORMAL;
>>>> + else
>>>> + return -EINVAL;
>>>> +
>>>> + flash = &qspi->flash[cs_num];
>>>> + flash->qspi = qspi;
>>>> + flash->cs = cs_num;
>>>> + flash->presc = presc;
>>>> +
>>>> + flash->nor.dev = qspi->dev;
>>>> + spi_nor_set_flash_node(&flash->nor, np);
>>>> + flash->nor.priv = flash;
>>>> + mtd = &flash->nor.mtd;
>>>> + mtd->priv = &flash->nor;
>>>> +
>>>> + flash->nor.read = stm32_qspi_read;
>>>> + flash->nor.write = stm32_qspi_write;
>>>> + flash->nor.erase = stm32_qspi_erase;
>>>> + flash->nor.read_reg = stm32_qspi_read_reg;
>>>> + flash->nor.write_reg = stm32_qspi_write_reg;
>>>> + flash->nor.prepare = stm32_qspi_prep;
>>>> + flash->nor.unprepare = stm32_qspi_unprep;
>>>> +
>>>> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>> +
>>>> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>>> + | CR_EN, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> + /*
>>>> + * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>>>> + * which define the size of nor flash.
>>>> + * if fsize is NULL, the controller can't sent spi-nor command.
>>>> + * set a temporary value just to discover the nor flash with
>>>> + * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>>> + */
>>> Is 25 the smallest value ? Use a macro for this ...
>> 25 is an arbitrary choice, I will define a smallest value
> Cool, thanks!
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator
From: Fabrice Gasnier @ 2017-04-10 16:43 UTC (permalink / raw)
To: Jonathan Cameron, linux, robh+dt, linux-arm-kernel, devicetree,
linux-kernel
Cc: mark.rutland, benjamin.gaignard, lars, alexandre.torgue,
linux-iio, pmeerw, mcoquelin.stm32, knaack.h, benjamin.gaignard
In-Reply-To: <8fa183fe-42f1-dc27-efd9-1f7d2e5b5057@kernel.org>
On 04/09/2017 11:34 AM, Jonathan Cameron wrote:
> On 06/04/17 17:11, Fabrice Gasnier wrote:
>> STM32 DAC has built-in noise or triangle waveform generator.
>> - "wavetype" extended attribute selects noise or triangle.
>> - "amplitude" extended attribute selects amplitude for waveform generator
>>
>> A DC offset can be added to waveform generator output. This can be done
>> using out_voltage[1/2]_offset
>>
>> Waveform generator requires a trigger to be configured, to increment /
>> decrement internal counter in case of triangle generator. Noise
>> generator is a bit different, but also requires a trigger to generate
>> samples.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>
> Various bits inline. Mostly I think the blockers on this will be
> making sure the ABI defined is generic enough to handle the more crazy
> DDS chips out there... (basically the ones doing frequency modulation).
>
> Jonathan
>> ---
>> Changes in v2:
>> - use _offset parameter to add DC offset to waveform generator
> Conceptually this offset is just the normal DAC output value (particularly
yes
> in the flat case) I guess we can paper over this by having the _raw
> and this always have th same value, but it's a little inelegant.
> Still people are going to expect _raw to control it when in DAC mode but
> that makes limited sense in DDS mode.
>
> Mind you nothing stops us defining all DDS channels as the sum of whatever
> the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
> purely through documentation. Think of the DDS as a modulation on top
> of the DAC...
>
>> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
>> out_voltage_wavetype_available, out_voltageY_amplitude,
>> out_voltage_amplitude_available
> Hmm. I'm thinking those amplitude values aren't nice and don't fit well
> with the more general ABI.
>
> I suggested (but didn't really expand upon) having standard defined types
> for each waveform then using scale to control the amplitude.
Do you mean _scale attribute ?
>
> Is that something that might work here?
I probably miss the point here...
>
> So say we have our triangle standard form having an amplitude of 1V Peak to
> Peak. Then we can use scale to make it whatever we actually have in this
> case? The docs for wave type will need to describe those standard forms
> though.
... scale is fixed here, in line with _raw attribute. In 'amplitude'
description for STM32 DAC here (e.g. from 1...4095), same scale is used.
Can you elaborate ?
>
> Hmm. Whether this is worth doing is unclear however as we'll still have
> to describe the 'frequency' in terms of the clock ticks (here the triggers)
Describing frequency may be an issue, not sure it makes senses in this
case: 'clock ticks', e.g. triggers here may be timers, but also an EXTI
(external...), or even software trig perhaps.
> So maybe amplitude is worth having. Again, looking for input from ADI lot
> on this... There are some really fiddly cases to describe were we are doing
> symbol encoding so have multiple waveforms that we are switching between based
> on some external signal. Any ABI needs to encompass that sort of usage.
> Parts like the AD9833 for example...
>
>> - Better explain trigger usage in case of waveform generator.
>> ---
>> Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 | 16 +++
>> drivers/iio/dac/stm32-dac-core.h | 4 +
>> drivers/iio/dac/stm32-dac.c | 158 +++++++++++++++++++++-
>> 3 files changed, 177 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> new file mode 100644
>> index 0000000..8f1fa009
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> Fair enough to initially introduced these for this part only, but I'd very
> much like to see us agree on these sufficiently to get them into the main
> docs asap so we have something to work with for getting the DDS chips out
> of staging...
>> @@ -0,0 +1,16 @@
>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
>> +KernelVersion: 4.12
>> +Contact: fabrice.gasnier@st.com
>> +Description:
>> + List and/or select waveform generation provided by STM32 DAC:
>> + - "flat": waveform generator disabled (default)
>> + - "noise": select noise waveform
>> + - "triangle": select triangle waveform
>> +
>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
>> +What: /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
>> +KernelVersion: 4.12
>> +Contact: fabrice.gasnier@st.com
>> +Description:
>> + List and/or select amplitude used for waveform generator
>> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
>> index e51a468..0f02975 100644
>> --- a/drivers/iio/dac/stm32-dac-core.h
>> +++ b/drivers/iio/dac/stm32-dac-core.h
>> @@ -37,8 +37,12 @@
>> #define STM32H7_DAC_CR_TEN1 BIT(1)
>> #define STM32H7_DAC_CR_TSEL1_SHIFT 2
>> #define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
>> +#define STM32_DAC_CR_WAVE1 GENMASK(7, 6)
>> +#define STM32_DAC_CR_MAMP1 GENMASK(11, 8)
>> #define STM32H7_DAC_CR_HFSEL BIT(15)
>> #define STM32_DAC_CR_EN2 BIT(16)
>> +#define STM32_DAC_CR_WAVE2 GENMASK(23, 22)
>> +#define STM32_DAC_CR_MAMP2 GENMASK(27, 24)
>>
>> /* STM32_DAC_SWTRIGR bit fields */
>> #define STM32_DAC_SWTRIGR_SWTRIG1 BIT(0)
>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>> index a7a078e..2ed75db 100644
>> --- a/drivers/iio/dac/stm32-dac.c
>> +++ b/drivers/iio/dac/stm32-dac.c
>> @@ -42,10 +42,12 @@
>> /**
>> * struct stm32_dac - private data of DAC driver
>> * @common: reference to DAC common data
>> + * @wavetype: waveform generator
>> * @swtrig: Using software trigger
>> */
>> struct stm32_dac {
>> struct stm32_dac_common *common;
>> + u32 wavetype;
>> bool swtrig;
>> };
>>
>> @@ -222,6 +224,29 @@ static int stm32_dac_set_value(struct stm32_dac *dac, int channel, int val)
>> return ret;
>> }
>>
>> +static int stm32_dac_get_offset(struct stm32_dac *dac, int channel, int *val)
>> +{
>> + int ret;
>> +
>> + /* Offset is only relevant in waveform generation mode. */
>> + if (!dac->wavetype) {
>> + *val = 0;
>> + return IIO_VAL_INT;
>> + }
>> +
>> + /*
>> + * In waveform generation mode, DC offset in DHR is added to waveform
>> + * generator output, then stored to DOR (data output register).
>> + * Read offset from DHR.
>> + */
> Just thinking what fun we could have if we do the fifo based output to push
> this that I was suggesting for the previous patch ;) triangles on top
> of fun general waveforms..
>
>> + if (STM32_DAC_IS_CHAN_1(channel))
>> + ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R1, val);
>> + else
>> + ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R2, val);
>> +
>> + return ret ? ret : IIO_VAL_INT;
>> +}
>> +
>> static int stm32_dac_read_raw(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *chan,
>> int *val, int *val2, long mask)
>> @@ -231,6 +256,8 @@ static int stm32_dac_read_raw(struct iio_dev *indio_dev,
>> switch (mask) {
>> case IIO_CHAN_INFO_RAW:
>> return stm32_dac_get_value(dac, chan->channel, val);
>> + case IIO_CHAN_INFO_OFFSET:
>> + return stm32_dac_get_offset(dac, chan->channel, val);
>> case IIO_CHAN_INFO_SCALE:
>> *val = dac->common->vref_mv;
>> *val2 = chan->scan_type.realbits;
>> @@ -247,8 +274,16 @@ static int stm32_dac_write_raw(struct iio_dev *indio_dev,
>> struct stm32_dac *dac = iio_priv(indio_dev);
>>
>> switch (mask) {
>> + case IIO_CHAN_INFO_OFFSET:
>> + /* Offset only makes sense in waveform generation mode */
>> + if (dac->wavetype)
>> + return stm32_dac_set_value(dac, chan->channel, val);
>> + return -EBUSY;
> Yeah, I think I sent you down a blind alley here. If people agree, lets
> just define DDS signals as always being the sum of _raw + the dds element.
> Then it's easy.
Ok, I can revert back to use _raw if this is fine ;-)
>> case IIO_CHAN_INFO_RAW:
>> - return stm32_dac_set_value(dac, chan->channel, val);
>> + if (!dac->wavetype)
>> + return stm32_dac_set_value(dac, chan->channel, val);
>> + /* raw value is read only in waveform generation mode */
>> + return -EBUSY;
>> default:
>> return -EINVAL;
>> }
>> @@ -334,6 +369,122 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
>> .set = stm32_dac_set_powerdown_mode,
>> };
>>
>> +/* waveform generator wave selection */
>> +static const char * const stm32_dac_wavetype_desc[] = {
>> + "flat",
>> + "noise",
>> + "triangle",
>> +};
>> +
>> +static int stm32_dac_set_wavetype(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + unsigned int wavetype)
>> +{
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + u32 mask, val;
>> + int ret;
>> +
>> + /*
>> + * Waveform generator requires a trigger to be configured, to increment
>> + * or decrement internal counter in case of triangle generator. Noise
>> + * generator is a bit different, but also requires a trigger to
>> + * generate samples.
>> + */
>> + if (wavetype && !indio_dev->trig)
>> + dev_dbg(&indio_dev->dev, "Wavegen requires a trigger\n");
>> +
>> + if (STM32_DAC_IS_CHAN_1(chan->channel)) {
>> + val = FIELD_PREP(STM32_DAC_CR_WAVE1, wavetype);
>> + mask = STM32_DAC_CR_WAVE1;
>> + } else {
>> + val = FIELD_PREP(STM32_DAC_CR_WAVE2, wavetype);
>> + mask = STM32_DAC_CR_WAVE2;
>> + }
>> +
>> + ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, mask, val);
>> + if (ret)
>> + return ret;
>> + dac->wavetype = wavetype;
>> +
>> + return 0;
>> +}
>> +
>> +static int stm32_dac_get_wavetype(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan)
>> +{
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + u32 val;
>> + int ret;
>> +
>> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (STM32_DAC_IS_CHAN_1(chan->channel))
>> + return FIELD_GET(STM32_DAC_CR_WAVE1, val);
>> + else
>> + return FIELD_GET(STM32_DAC_CR_WAVE2, val);
>> +}
>> +
>> +static const struct iio_enum stm32_dac_wavetype_enum = {
>> + .items = stm32_dac_wavetype_desc,
>> + .num_items = ARRAY_SIZE(stm32_dac_wavetype_desc),
>> + .get = stm32_dac_get_wavetype,
>> + .set = stm32_dac_set_wavetype,
>> +};
>> +
>> +/*
>> + * waveform generator mamp selection: mask/amplitude
>> + * - noise: LFSR mask (linear feedback shift register, umasks bit 0, [1:0]...)
> This needs breaking out into two attributes - for noise it isn't amplitude in
> any conventional sense... Keep the result device specific for now. I'm not
> even sure what type of pseudo random source that actually is...
Do you suggest to create specific attribute for this ? This will end-up
to write same register/bitfield as 'amplitude' for triangle generator.
Thanks & Regards,
Fabrice
> If anyone wants to figure it out it would be great to document it in a general
> form!
>
>> + * - triangle: amplitude (equal to 1, 3, 5, 7... 4095)
>> + */
>> +static const char * const stm32_dac_amplitude_desc[] = {
>> + "1", "3", "7", "15", "31", "63", "127", "255", "511", "1023", "2047",
>> + "4095",
>> +};
>> +
>> +static int stm32_dac_set_amplitude(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + unsigned int amplitude)
>> +{
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + u32 mask, val;
>> +
>> + if (STM32_DAC_IS_CHAN_1(chan->channel)) {
>> + val = FIELD_PREP(STM32_DAC_CR_MAMP1, amplitude);
>> + mask = STM32_DAC_CR_MAMP1;
>> + } else {
>> + val = FIELD_PREP(STM32_DAC_CR_MAMP2, amplitude);
>> + mask = STM32_DAC_CR_MAMP2;
>> + }
>> +
>> + return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, mask, val);
>> +}
>> +
>> +static int stm32_dac_get_amplitude(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan)
>> +{
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + u32 val;
>> + int ret;
>> +
>> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (STM32_DAC_IS_CHAN_1(chan->channel))
>> + return FIELD_GET(STM32_DAC_CR_MAMP1, val);
>> + else
>> + return FIELD_GET(STM32_DAC_CR_MAMP2, val);
>> +}
>> +
>> +static const struct iio_enum stm32_dac_amplitude_enum = {
>> + .items = stm32_dac_amplitude_desc,
>> + .num_items = ARRAY_SIZE(stm32_dac_amplitude_desc),
>> + .get = stm32_dac_get_amplitude,
>> + .set = stm32_dac_set_amplitude,
>> +};
>> +
>> static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
>> {
>> .name = "powerdown",
>> @@ -343,6 +494,10 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
>> },
>> IIO_ENUM("powerdown_mode", IIO_SEPARATE, &stm32_dac_powerdown_mode_en),
>> IIO_ENUM_AVAILABLE("powerdown_mode", &stm32_dac_powerdown_mode_en),
>> + IIO_ENUM("wavetype", IIO_SEPARATE, &stm32_dac_wavetype_enum),
>> + IIO_ENUM_AVAILABLE("wavetype", &stm32_dac_wavetype_enum),
>> + IIO_ENUM("amplitude", IIO_SEPARATE, &stm32_dac_amplitude_enum),
>> + IIO_ENUM_AVAILABLE("amplitude", &stm32_dac_amplitude_enum),
>> {},
>> };
>>
>> @@ -352,6 +507,7 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
>> .output = 1, \
>> .channel = chan, \
>> .info_mask_separate = \
>> + BIT(IIO_CHAN_INFO_OFFSET) | \
>> BIT(IIO_CHAN_INFO_RAW) | \
>> BIT(IIO_CHAN_INFO_SCALE), \
>> /* scan_index is always 0 as num_channels is 1 */ \
>>
>
^ permalink raw reply
* Re: [PATCH v2 4/5] iio: dac: stm32: add support for trigger events
From: Fabrice Gasnier @ 2017-04-10 16:37 UTC (permalink / raw)
To: Jonathan Cameron, linux, robh+dt, linux-arm-kernel, devicetree,
linux-kernel
Cc: mark.rutland, benjamin.gaignard, lars, alexandre.torgue,
linux-iio, pmeerw, mcoquelin.stm32, knaack.h, benjamin.gaignard
In-Reply-To: <14dfe2a9-d459-0074-532e-d44daf336406@kernel.org>
On 04/09/2017 11:04 AM, Jonathan Cameron wrote:
> On 06/04/17 17:11, Fabrice Gasnier wrote:
>> STM32 DAC supports triggers to synchronize conversions. When trigger
>> occurs, data is transferred from DHR (data holding register) to DOR
>> (data output register) so output voltage is updated.
>> Both hardware and software triggers are supported.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Hmm. I'm really not sure on how to handle this... The problem to my mind
> is knowing when it has triggered so we can know that it makes sense to
> put the next reading in.... Anyhow bear with me...
>
> I think the same arguement holds for this as equivalent input devices.
> There we have always argued that if you need multichannel synchronized
> capture (which is 'kind' of what we are looking at here) then you have
> to use the buffered interface.
>
> I think we need buffered output for this to fit nicely in the subsystem.
> It definitely isn't a correct use of the event triggers.
>
> That means we really need to figure out the ABI for buffered output and
> get that sorted. To my mind it shouldn't be too tricky and should look
> much like buffered input, just with us writing to a fifo from userspace
> rather than reading from it.
>
> The DMA side of that can come later, in a similar fashion to how it is
> added for the ADC side of things. We can also have 'providers'
> (equivalent of 'consumers' on the ADC side), perhaps giving a neat way
> of describing DDS devices (I'm not so sure on this yet).
>
> So to my mind, if you are not in buffered mode and do a sysfs write it
> should be 'instant'. If in buffered mode, then it will wait on the
> trigger.
>
> So the complex side of things is what we 'know' about the data flow.
> 1) Case you have here. We want to do direct write through to the device,
> but have no way of knowing (or do we?) that it has triggered and written
> the data to the output. So we have no way of knowing we can push the next
> value in from a fifo yet... In this case I guess the solution might be to
> have a fifo length of 0. That is data flows straight to hardware.
>
> 2) Simple stream case - always enough data in the fifo and we get an interrupt
> to signify that the previous trigger happened.
>
> 3) Case where we are only just keeping up. So we won't have data in the fifo
> until sometime after the previous trigger. In this case we need the fifo to
> push straight through if there isn't data ready to go.
>
> 4) Case where we are not pushing data fast enough. Just don't update?
>
> That last case 4 is nasty as the reason we typically want to do triggered
> DAC updates is to ensure we always have valid states in some control loop,
> but we might get a race here where one DAC has a value ready to go on a trigger
> and another one isn't quite ready. In this case we might want to hold off
> until all are ready... So there might need to be a sanity check that everyone
> on a given trigger is ready to go - an extra callback.
>
> So a bit fiddly and I'm not sure I like the representation of through flow as
> a fifo of 0 length... (can't think of a neater way though atm)
>
> Anyhow, time to sort output buffers out once and for all I think if we can
> get a reasonable group of people together who have the time.
>
> Sorry Fabrice that this has hit your driver! Perhaps we can figure
> enough out to be able to at least get the basics (i.e. patches 1,2) in as
> asap.
Hi Jonathan,
Thanks for sharing your view on this.
I sent patches 1,2 updated with your comments. I dropped following
patches for the time being, as it obviously require additions...
I agree with your analysis and concerns above.
I hope Lars or others can give some feedback or guidelines on output
buffer? Is there something already, we may start to work on ?
Thanks all in advance.
Best Regards,
Fabrice
>
> Jonathan
>> ---
>> Changes in v2:
>> - Fix issue with trigger, by using set_trigger callback
>> - trigger can now be assigned, no matters powerdown state
>> ---
>> drivers/iio/dac/Kconfig | 3 +
>> drivers/iio/dac/stm32-dac-core.h | 8 +++
>> drivers/iio/dac/stm32-dac.c | 127 ++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 137 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index 7198648..786c38b 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -278,6 +278,9 @@ config STM32_DAC
>> tristate "STMicroelectronics STM32 DAC"
>> depends on (ARCH_STM32 && OF) || COMPILE_TEST
>> depends on REGULATOR
>> + select IIO_TRIGGERED_EVENT
>> + select IIO_STM32_TIMER_TRIGGER
>> + select MFD_STM32_TIMERS
>> select STM32_DAC_CORE
>> help
>> Say yes here to build support for STMicroelectronics STM32 Digital
>> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
>> index daf0993..e51a468 100644
>> --- a/drivers/iio/dac/stm32-dac-core.h
>> +++ b/drivers/iio/dac/stm32-dac-core.h
>> @@ -26,6 +26,7 @@
>>
>> /* STM32 DAC registers */
>> #define STM32_DAC_CR 0x00
>> +#define STM32_DAC_SWTRIGR 0x04
>> #define STM32_DAC_DHR12R1 0x08
>> #define STM32_DAC_DHR12R2 0x14
>> #define STM32_DAC_DOR1 0x2C
>> @@ -33,9 +34,16 @@
>>
>> /* STM32_DAC_CR bit fields */
>> #define STM32_DAC_CR_EN1 BIT(0)
>> +#define STM32H7_DAC_CR_TEN1 BIT(1)
>> +#define STM32H7_DAC_CR_TSEL1_SHIFT 2
>> +#define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
>> #define STM32H7_DAC_CR_HFSEL BIT(15)
>> #define STM32_DAC_CR_EN2 BIT(16)
>>
>> +/* STM32_DAC_SWTRIGR bit fields */
>> +#define STM32_DAC_SWTRIGR_SWTRIG1 BIT(0)
>> +#define STM32_DAC_SWTRIGR_SWTRIG2 BIT(1)
>> +
>> /**
>> * struct stm32_dac_common - stm32 DAC driver common data (for all instances)
>> * @regmap: DAC registers shared via regmap
>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>> index c0d993a..a7a078e 100644
>> --- a/drivers/iio/dac/stm32-dac.c
>> +++ b/drivers/iio/dac/stm32-dac.c
>> @@ -23,6 +23,10 @@
>> #include <linux/bitfield.h>
>> #include <linux/delay.h>
>> #include <linux/iio/iio.h>
>> +#include <linux/iio/timer/stm32-timer-trigger.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_event.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> @@ -32,15 +36,113 @@
>> #define STM32_DAC_CHANNEL_1 1
>> #define STM32_DAC_CHANNEL_2 2
>> #define STM32_DAC_IS_CHAN_1(ch) ((ch) & STM32_DAC_CHANNEL_1)
>> +/* channel2 shift */
>> +#define STM32_DAC_CHAN2_SHIFT 16
>>
>> /**
>> * struct stm32_dac - private data of DAC driver
>> * @common: reference to DAC common data
>> + * @swtrig: Using software trigger
>> */
>> struct stm32_dac {
>> struct stm32_dac_common *common;
>> + bool swtrig;
>> };
>>
>> +/**
>> + * struct stm32_dac_trig_info - DAC trigger info
>> + * @name: name of the trigger, corresponding to its source
>> + * @tsel: trigger selection, value to be configured in DAC_CR.TSELx
>> + */
>> +struct stm32_dac_trig_info {
>> + const char *name;
>> + u32 tsel;
>> +};
>> +
>> +static const struct stm32_dac_trig_info stm32h7_dac_trinfo[] = {
>> + { "swtrig", 0 },
>> + { TIM1_TRGO, 1 },
>> + { TIM2_TRGO, 2 },
>> + { TIM4_TRGO, 3 },
>> + { TIM5_TRGO, 4 },
>> + { TIM6_TRGO, 5 },
>> + { TIM7_TRGO, 6 },
>> + { TIM8_TRGO, 7 },
>> + {},
>> +};
>> +
>> +static irqreturn_t stm32_dac_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + int channel = indio_dev->channels[0].channel;
>> +
>> + /* Using software trigger? Then, trigger it now */
> Can we get here otherwise?
> If not I'd prefer to either see an error on the other case
> (perhaps simply return IRQ_NONE)
>> + if (dac->swtrig) {
>> + u32 swtrig;
>> +
>> + if (STM32_DAC_IS_CHAN_1(channel))
>> + swtrig = STM32_DAC_SWTRIGR_SWTRIG1;
>> + else
>> + swtrig = STM32_DAC_SWTRIGR_SWTRIG2;
>> + regmap_update_bits(dac->common->regmap, STM32_DAC_SWTRIGR,
>> + swtrig, swtrig);
>> + }
>> +
>> + iio_trigger_notify_done(indio_dev->trig);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static unsigned int stm32_dac_get_trig_tsel(struct stm32_dac *dac,
>> + struct iio_trigger *trig)
>> +{
>> + unsigned int i;
>> +
>> + /* skip 1st trigger that should be swtrig */
>> + for (i = 1; stm32h7_dac_trinfo[i].name; i++) {
>> + /*
>> + * Checking both stm32 timer trigger type and trig name
>> + * should be safe against arbitrary trigger names.
>> + */
>> + if (is_stm32_timer_trigger(trig) &&
>> + !strcmp(stm32h7_dac_trinfo[i].name, trig->name)) {
>> + return stm32h7_dac_trinfo[i].tsel;
>> + }
>> + }
>> +
>> + /* When no trigger has been found, default to software trigger */
>> + dac->swtrig = true;
>> +
>> + return stm32h7_dac_trinfo[0].tsel;
>> +}
>> +
>> +static int stm32_dac_set_trigger(struct iio_dev *indio_dev,
>> + struct iio_trigger *trig)
>> +{
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + int channel = indio_dev->channels[0].channel;
>> + u32 shift = STM32_DAC_IS_CHAN_1(channel) ? 0 : STM32_DAC_CHAN2_SHIFT;
>> + u32 val = 0, tsel;
>> + u32 msk = (STM32H7_DAC_CR_TEN1 | STM32H7_DAC_CR_TSEL1) << shift;
>> +
>> + dac->swtrig = false;
>> + if (trig) {
>> + /* select & enable trigger (tsel / ten) */
>> + tsel = stm32_dac_get_trig_tsel(dac, trig);
>> + val = tsel << STM32H7_DAC_CR_TSEL1_SHIFT;
>> + val = (val | STM32H7_DAC_CR_TEN1) << shift;
>> + }
>> +
>> + if (trig)
>> + dev_dbg(&indio_dev->dev, "enable trigger: %s\n", trig->name);
>> + else
>> + dev_dbg(&indio_dev->dev, "disable trigger\n");
>> +
>> + return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, msk, val);
>> +}
>> +
>> static int stm32_dac_is_enabled(struct iio_dev *indio_dev, int channel)
>> {
>> struct stm32_dac *dac = iio_priv(indio_dev);
>> @@ -167,6 +269,7 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
>> static const struct iio_info stm32_dac_iio_info = {
>> .read_raw = stm32_dac_read_raw,
>> .write_raw = stm32_dac_write_raw,
>> + .set_trigger = stm32_dac_set_trigger,
>> .debugfs_reg_access = stm32_dac_debugfs_reg_access,
>> .driver_module = THIS_MODULE,
>> };
>> @@ -326,7 +429,28 @@ static int stm32_dac_probe(struct platform_device *pdev)
>> if (ret < 0)
>> return ret;
>>
>> - return devm_iio_device_register(&pdev->dev, indio_dev);
>> + ret = iio_triggered_event_setup(indio_dev, NULL,
>> + stm32_dac_trigger_handler);
>> + if (ret)
>> + return ret;
>> +
>> + ret = iio_device_register(indio_dev);
>> + if (ret) {
>> + iio_triggered_event_cleanup(indio_dev);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int stm32_dac_remove(struct platform_device *pdev)
>> +{
>> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> +
>> + iio_triggered_event_cleanup(indio_dev);
>> + iio_device_unregister(indio_dev);
>> +
>> + return 0;
>> }
>>
>> static const struct of_device_id stm32_dac_of_match[] = {
>> @@ -337,6 +461,7 @@ static int stm32_dac_probe(struct platform_device *pdev)
>>
>> static struct platform_driver stm32_dac_driver = {
>> .probe = stm32_dac_probe,
>> + .remove = stm32_dac_remove,
>> .driver = {
>> .name = "stm32-dac",
>> .of_match_table = stm32_dac_of_match,
>>
>
^ permalink raw reply
* Re: [PATCH v3 6/9] drivers: remove useless comment from base/arch_topology.c
From: Russell King - ARM Linux @ 2017-04-10 16:33 UTC (permalink / raw)
To: Juri Lelli
Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
vincent.guittot, robh+dt, mark.rutland, sudeep.holla,
lorenzo.pieralisi, catalin.marinas, will.deacon, morten.rasmussen,
dietmar.eggemann, broonie, gregkh
In-Reply-To: <20170410140214.GE30804@e106622-lin>
On Mon, Apr 10, 2017 at 03:02:14PM +0100, Juri Lelli wrote:
> Hi,
>
> On 10/04/17 14:51, Russell King - ARM Linux wrote:
> > On Mon, Mar 27, 2017 at 02:18:22PM +0100, Juri Lelli wrote:
> > > Printing out an error message when we failed to get the cpu device is
> > > not helping anyone. Remove it.
> >
> > (1) the subject line talks about removing a "comment" but you're
> > actually removing an error printk
> > (2) I don't think it's "not helping anyone", although the description
> > above doesn't say _why_ - it's reporting the lack of a missing CPU
> > device that we expect to be present. If it's not present, then
> > we're not going to end up with the cpu capacity attribute, and the
> > error message answers the "why is that sysfs file missing" question.
>
> That's the same I was thinking when I put the error message there in the
> first place. But, then Greg didn't seem to like it.
I don't think it was a case of "not liking it" - Greg asked what use it
was. Greg also pointed out the race with userspace.
I think dropping this patch is the quickest way to move forward.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH v2 2/2] dt-bindings: document: add firefly-rk3399 board support
From: Rob Herring @ 2017-04-10 16:25 UTC (permalink / raw)
To: Kever Yang
Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Jianqun Xu, Liang Chen,
Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Randy Li,
Andy Yan, Eddie Cai, Mark Rutland,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Xing Zheng
In-Reply-To: <1491384800-22412-2-git-send-email-kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Wed, Apr 05, 2017 at 05:33:20PM +0800, Kever Yang wrote:
> Use "firefly,firefly-rk3399" compatible string for firefly-rk3399 board.
>
> Signed-off-by: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>
> Changes in v2: None
>
> Documentation/devicetree/bindings/arm/rockchip.txt | 4 ++++
> 1 file changed, 4 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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